From 7e9db1fb71994ced7e760947cf6351816c85b88b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 07:30:58 +0000 Subject: [PATCH 1/5] test(cli,plugin-dev): ledger the i18n auto-detect's option-B loss, RED first (#15232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The by-shape sweep found this site, not the #15004 pin, so `OPTION_B_LOSSES` carries no row for it. This commit adds the row and nothing else: the reader is deliberately still top-level-only, so the pin goes RED naming a subsystem that loses a collection the ledger does not carry. The fix, and the deletion of the row, land in the next commit — a row that never existed is a row nobody can check the fix against. The row calls the SHIPPED decision rather than re-reading `stack.translations`: `devI18nPluginOptions` is extracted from `DevPlugin.init`'s 3b block, verbatim in behaviour, and exported so the probe measures what the plugin itself decides. `packages/cli` reaches it through an anchored vitest alias to source (the sanctioned remedy — `KNOWN_UNALIASED_TEST_IMPORTS` is shrink-only) and a star-less `paths` rule, so the run and the type verdict agree about which artifact is under test. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- packages/cli/package.json | 1 + .../test/fixtures/option-b-reader-probe.ts | 19 ++++- packages/cli/tsconfig.test.json | 7 ++ packages/cli/vitest.config.ts | 17 ++++ packages/plugins/plugin-dev/src/dev-i18n.ts | 83 +++++++++++++++++++ packages/plugins/plugin-dev/src/dev-plugin.ts | 22 ++--- packages/plugins/plugin-dev/src/index.ts | 9 ++ pnpm-lock.yaml | 3 + 8 files changed, 150 insertions(+), 11 deletions(-) create mode 100644 packages/plugins/plugin-dev/src/dev-i18n.ts diff --git a/packages/cli/package.json b/packages/cli/package.json index 84bef8d86e..2eb9ef0c7a 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -129,6 +129,7 @@ }, "devDependencies": { "@objectstack/driver-turso": "workspace:*", + "@objectstack/plugin-dev": "workspace:*", "@oclif/plugin-help": "^6.2.58", "@oclif/plugin-plugins": "^5.4.87", "@types/better-sqlite3": "^7.6.13", diff --git a/packages/cli/test/fixtures/option-b-reader-probe.ts b/packages/cli/test/fixtures/option-b-reader-probe.ts index 7bbbbe8dc6..7ef50019a0 100644 --- a/packages/cli/test/fixtures/option-b-reader-probe.ts +++ b/packages/cli/test/fixtures/option-b-reader-probe.ts @@ -14,7 +14,8 @@ * * - invokes a reader this repo SHIPS (`collectBundleActions`, * `resolveStandaloneDatabase`, `createStandaloneStack`, - * `appSecurityPluginOptions`, …) and reports its return value, or + * `appSecurityPluginOptions`, `devI18nPluginOptions`, …) and reports its + * return value, or * - boots a real kernel carrying the real `AppPlugin` and reports what that * plugin HANDED to a subsystem (a job scheduled, a datasource connected, a * mapping set, an i18n service registered, a seed dataset merged). @@ -62,6 +63,7 @@ import { resolveStandaloneDatabase, } from '@objectstack/runtime'; import { appSecurityPluginOptions } from '@objectstack/plugin-security'; +import { devI18nPluginOptions } from '@objectstack/plugin-dev'; import { ObjectStackDefinitionSchema, normalizeStackInput } from '@objectstack/spec'; // The lowering itself, not a copy of it — reached as SOURCE, by relative path, @@ -350,6 +352,21 @@ export async function measureShape(project: unknown, projectRoot: string): Promi // loader — it is in the readers each of them then drives, which is what // these rows are. + // `DevPlugin` takes its stack from a CALLER-SUPPLIED object + // (`new DevPlugin({ stack: config })`, the documented construction), so there + // is no load boundary between the composed config and this reader — the same + // object `os dev` boots from source is handed straight to the plugin. The row + // calls the SHIPPED decision (`devI18nPluginOptions`), which is what the + // plugin itself calls to decide whether to register `I18nServicePlugin`; a + // row that re-read `stack.translations` here would be a second copy of the + // read the reader program changes and would stay red after it was fixed. + const devI18n = devI18nPluginOptions(project); + rows.push(row( + 'B2 · plugin-dev I18nServicePlugin auto-detect over the caller-supplied stack · translations', + devI18n ? `I18nServicePlugin(fallbackLocale=${devI18n.fallbackLocale})` : undefined, + devI18n === undefined, + )); + const fromSourceProfile = appSecurityPluginOptions(project)?.fallbackPermissionSet; rows.push(row( 'B2 · plugin-security appSecurityPluginOptions over the from-source config (default permission set) · permissions', diff --git a/packages/cli/tsconfig.test.json b/packages/cli/tsconfig.test.json index e87ce6c578..bb77c382da 100644 --- a/packages/cli/tsconfig.test.json +++ b/packages/cli/tsconfig.test.json @@ -151,6 +151,13 @@ // into this program needs no widening. "paths": { "@objectstack/objectql": ["../objectql/src/index.ts"], + // [#15232] The fourth, added for the same reason and under the same + // no-star rule: the option-B probe calls `@objectstack/plugin-dev`'s + // shipped i18n auto-detect decision, and this package's vitest config + // aliases that same bare specifier to the same source entry, so the type + // verdict and the run agree about which artifact is under test. The + // package publishes only `"."`. + "@objectstack/plugin-dev": ["../plugins/plugin-dev/src/index.ts"], "@objectstack/plugin-security": ["../plugins/plugin-security/src/index.ts"], "@objectstack/runtime": ["../runtime/src/index.ts"] } diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index a142e4758f..312c087abd 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -618,6 +618,23 @@ export default defineConfig({ find: /^@objectstack\/plugin-auth$/, replacement: path.resolve(__dirname, '../plugins/plugin-auth/src/index.ts'), }, + // `test/fixtures/option-b-reader-probe.ts` (#15232) calls + // `@objectstack/plugin-dev`'s shipped i18n auto-detect decision — the + // option-B acceptance pin (#15004) measures readers by CALLING them, and + // a reader resolved through `exports` to plugin-dev's **dist** would make + // that row a verdict about the last build rather than about the reader + // this card changes. The registry in `check-test-source-alias.mjs` is + // SHRINK-ONLY, so widening `KNOWN_UNALIASED_TEST_IMPORTS` was never an + // option; this entry is the sanctioned remedy, in the same anchored form + // as its neighbours (plugin-dev publishes only `"."`, and the anchor is + // what keeps that true if a subpath is ever added). Measured before and + // after: the gate reports the same required set for this package in both + // directions — crossing into `plugin-dev/src` adds no unaliased artifact + // import it did not already carry. + { + find: /^@objectstack\/plugin-dev$/, + replacement: path.resolve(__dirname, '../plugins/plugin-dev/src/index.ts'), + }, // `src/utils/protocol-version-gap.test.ts` (#13860) exercises the upgrade // advisory, whose verdict comes from `checkProtocolCompat` — the platform's // single reader of `engines.protocol`. The advisory is a thin direction diff --git a/packages/plugins/plugin-dev/src/dev-i18n.ts b/packages/plugins/plugin-dev/src/dev-i18n.ts new file mode 100644 index 0000000000..05d978b592 --- /dev/null +++ b/packages/plugins/plugin-dev/src/dev-i18n.ts @@ -0,0 +1,83 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The options `DevPlugin` hands `I18nServicePlugin` when it auto-registers it. + * + * `defaultLocale` stays optional because the detected stack may declare + * translations without declaring an i18n config at all — that is the common + * case, and `I18nServicePlugin`'s own default is what should apply then. + */ +export interface DevI18nPluginOptions { + defaultLocale?: string; + fallbackLocale: string; +} + +const asBag = (value: unknown): Record | undefined => + (value && typeof value === 'object' ? value as Record : undefined); + +const declaresTranslationArray = (body: unknown): boolean => { + const declared = asBag(body)?.translations; + return Array.isArray(declared) && declared.length > 0; +}; + +/** + * Does this stack DECLARE translations? + * + * ⚠️ TOP LEVEL ONLY, which is the defect #15232 exists to fix — recorded here + * so this intermediate commit is not mistaken for the fix. A multi-package + * artifact under ADR-0130 D4's option-B shape carries `translations` inside + * `packages[]` and nothing at the top level, so this answers `false` and the + * dev server keeps the in-memory i18n fallback with nothing thrown and nothing + * logged. The probe row landing in the same commit ledgers exactly that loss. + */ +export function stackDeclaresTranslations(stack: unknown): boolean { + return declaresTranslationArray(stack); +} + +/** + * [#15232] The `I18nServicePlugin` options a stack implies — the ONE decision + * `DevPlugin`'s i18n auto-detect makes, resolved in one place. + * + * `undefined` means "this stack declares no i18n content", i.e. do not register + * the file-based service and leave the slot to the core in-memory fallback. + * Returning the options rather than a boolean is what keeps the decision and + * the values it derives from the same read — a caller cannot pair a `true` with + * a locale resolved from somewhere else. + * + * Three declarations trigger it, exactly as they have since the auto-detect was + * written: a `translations` collection (read through + * {@link stackDeclaresTranslations}), an `i18n` config on the stack or its + * manifest, or `manifest.translations` — the authoring manifest's glob + * patterns, which are a declaration of intent even before a bundle is + * assembled. + * + * Exported because the #15004 option-B acceptance probe measures this decision + * by CALLING it. A probe that re-implemented the read would be a second copy of + * the code the reader program changes, and would stay red after the reader + * beside it was fixed. + */ +export function devI18nPluginOptions(stack: unknown): DevI18nPluginOptions | undefined { + const bag = asBag(stack); + if (!bag) return undefined; + + const manifest = asBag(bag.manifest); + const hasTranslations = stackDeclaresTranslations(stack); + const hasI18nConfig = !!(bag.i18n || manifest?.i18n); + const hasManifestTranslations = !!( + manifest && Array.isArray(manifest.translations) && manifest.translations.length > 0 + ); + + if (!hasTranslations && !hasI18nConfig && !hasManifestTranslations) return undefined; + + // `stack.i18n || stack.manifest.i18n || {}`, the original expression: the + // stack's own config wins, the manifest's is the fallback, and neither being + // present means the service plugin's own defaults apply. + const i18nConfig = (bag.i18n || manifest?.i18n || {}) as { + defaultLocale?: string; + fallbackLocale?: string; + }; + return { + defaultLocale: i18nConfig.defaultLocale, + fallbackLocale: i18nConfig.fallbackLocale || i18nConfig.defaultLocale || 'en', + }; +} diff --git a/packages/plugins/plugin-dev/src/dev-plugin.ts b/packages/plugins/plugin-dev/src/dev-plugin.ts index e72638e0b7..dd6b4bad93 100644 --- a/packages/plugins/plugin-dev/src/dev-plugin.ts +++ b/packages/plugins/plugin-dev/src/dev-plugin.ts @@ -4,6 +4,8 @@ import { Plugin, PluginContext } from '@objectstack/core'; import { resolveAllowDegradedTenancy, resolveAllowDevPlugin, resolveTenancyPosture } from '@objectstack/types'; import { postureEnforcesWall } from '@objectstack/spec/security'; +import { devI18nPluginOptions } from './dev-i18n.js'; + /** * Dev Plugin Options * @@ -522,19 +524,19 @@ export class DevPlugin implements Plugin { // file-based i18n. Falls back to the core in-memory i18n fallback // (with locale resolution) if the package is not installed. if (enabled('i18n') && this.options.stack) { - const stack = this.options.stack; - const hasTranslations = Array.isArray(stack.translations) && stack.translations.length > 0; - const hasI18nConfig = !!(stack.i18n || (stack.manifest && stack.manifest.i18n)); - const hasManifestTranslations = !!(stack.manifest && Array.isArray(stack.manifest.translations) && stack.manifest.translations.length > 0); + // [#15232] The detection AND the locales it derives are one decision, + // resolved in `dev-i18n.ts` — which reads `translations` at the flattened + // top level first and then through `resolveArtifactPackageOrder`, so a + // multi-package app under ADR-0130 D4's option-B shape is detected + // instead of silently falling back to the in-memory i18n. The dynamic + // import and its degradation stay HERE, because they are about the + // optional PACKAGE being installed, not about what the stack declares. + const i18nOptions = devI18nPluginOptions(this.options.stack); - if (hasTranslations || hasI18nConfig || hasManifestTranslations) { + if (i18nOptions) { try { const { I18nServicePlugin } = await import('@objectstack/service-i18n') as any; - const i18nConfig = stack.i18n || (stack.manifest || stack)?.i18n || {}; - const i18nPlugin = new I18nServicePlugin({ - defaultLocale: i18nConfig.defaultLocale, - fallbackLocale: i18nConfig.fallbackLocale || i18nConfig.defaultLocale || 'en', - }); + const i18nPlugin = new I18nServicePlugin(i18nOptions); this.childPlugins.push(i18nPlugin); ctx.logger.info(' ✔ I18nServicePlugin auto-registered (translations detected in stack)'); } catch (err) { diff --git a/packages/plugins/plugin-dev/src/index.ts b/packages/plugins/plugin-dev/src/index.ts index 515059d00f..ba3d2fb0d6 100644 --- a/packages/plugins/plugin-dev/src/index.ts +++ b/packages/plugins/plugin-dev/src/index.ts @@ -39,3 +39,12 @@ export { DevPlugin } from './dev-plugin.js'; export type { DevPluginOptions } from './dev-plugin.js'; + +/** + * [#15232] The i18n auto-detect's decision, exported so it can be MEASURED + * rather than re-implemented — the #15004 option-B acceptance pin calls it. + * `stackDeclaresTranslations` stays module-private on purpose: the published + * surface is the decision, not its limbs. + */ +export { devI18nPluginOptions } from './dev-i18n.js'; +export type { DevI18nPluginOptions } from './dev-i18n.js'; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a615c9086e..76c024e842 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -609,6 +609,9 @@ importers: '@objectstack/driver-turso': specifier: workspace:* version: link:../drivers/driver-turso + '@objectstack/plugin-dev': + specifier: workspace:* + version: link:../plugins/plugin-dev '@oclif/plugin-help': specifier: ^6.2.58 version: 6.2.58 From 64906c62057eb0e210c46b022912fcb6a88d2a66 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 07:45:50 +0000 Subject: [PATCH 2/5] test(cli): ledger the plugin-dev i18n loss so the fix has something to check against (#15232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The row added in the previous commit went RED naming a subsystem the ledger does not carry — the exact failure #15004 exists to make loud: B2 · plugin-dev I18nServicePlugin auto-detect over the caller-supplied stack · translations This records it, which turns the pin green at 25 rows and makes the loss a measured fact rather than an argument. The next commit fixes the reader and deletes this line again, so `OPTION_B_LOSSES` ends this branch exactly as it started it — the ledger never grows. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- packages/cli/test/option-b-reader-acceptance.pin.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cli/test/option-b-reader-acceptance.pin.test.ts b/packages/cli/test/option-b-reader-acceptance.pin.test.ts index 62ae5777d1..898c80a92f 100644 --- a/packages/cli/test/option-b-reader-acceptance.pin.test.ts +++ b/packages/cli/test/option-b-reader-acceptance.pin.test.ts @@ -143,6 +143,10 @@ const OPTION_B_LOSSES: readonly string[] = [ 'B2 · AppPlugin ql.setDatasourceMapping (object routing) (from source) · datasourceMapping', 'B2 · AppPlugin seed datasets merged (from source) · data', 'B2 · AppPlugin translation loading into the i18n service (from source) · translations', + // [#15232] The by-shape sweep (#15210) found this site, not this pin — so it + // is LEDGERED here first, red, before the reader beside it is touched. The + // fix and this line's deletion land in the next commit. + 'B2 · plugin-dev I18nServicePlugin auto-detect over the caller-supplied stack · translations', 'B2 · plugin-security appSecurityPluginOptions over the from-source config (default permission set) · permissions', 'B2 · runtime collectBundleActions over the from-source config · actions + objects[].actions', 'B2 · runtime collectBundleFunctionEntries over the from-source config · functions', From e3aca15e2ece0abeea759a84f2fb1eb88dad52ba Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 07:48:00 +0000 Subject: [PATCH 3/5] fix(plugin-dev): the i18n auto-detect resolves `translations` from `packages[]` (#15232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DevPlugin.init`'s 3b block read `options.stack.translations` alone, so a multi-package app under ADR-0130 D4's option-B shape — every definition carried once inside `packages[]`, no flattened top level — was read as declaring no copy at all. `I18nServicePlugin` was never registered and `os dev` served message keys, or last release's strings, from the core in-memory fallback. Nothing threw and nothing logged. The detection now reads the flattened top level FIRST and consults each package body only where that came back falsy, in the order `resolveArtifactPackageOrder` (`@objectstack/core`, ADR-0130 D4+D5) registers them. Today's additive artifact answers bit-identically: the original expression `Array.isArray(t) && t.length > 0` is preserved rather than re-expressed, and it short-circuits before `packages[]` is touched. A stack with no `packages` key never reaches the traversal at all, so its `translations` is still read exactly once. A malformed `packages` raises the same ADR-0112 envelope the registration path raises for it. The ledger row added two commits ago goes green and is deleted, so `OPTION_B_LOSSES` ends this branch at the 24 rows it started with. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- .../plugin-dev-i18n-detect-packages-reader.md | 50 ++++ .../option-b-reader-acceptance.pin.test.ts | 4 - .../src/dev-i18n-packages-reader.test.ts | 264 ++++++++++++++++++ packages/plugins/plugin-dev/src/dev-i18n.ts | 107 ++++++- 4 files changed, 407 insertions(+), 18 deletions(-) create mode 100644 .changeset/plugin-dev-i18n-detect-packages-reader.md create mode 100644 packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts diff --git a/.changeset/plugin-dev-i18n-detect-packages-reader.md b/.changeset/plugin-dev-i18n-detect-packages-reader.md new file mode 100644 index 0000000000..40375119b5 --- /dev/null +++ b/.changeset/plugin-dev-i18n-detect-packages-reader.md @@ -0,0 +1,50 @@ +--- +"@objectstack/plugin-dev": patch +--- + +fix(plugin-dev): the i18n auto-detect resolves `translations` from `packages[]`, not only the flattened top level (#15232) + +`DevPlugin.init`'s 3b block read `options.stack.translations` and nothing else. +For a multi-package app under the ADR-0130 D4 option-B shape — where +`packages[]` carries each definition exactly once and the flattened top-level +copy is gone — that read returns `undefined`, the detection concludes "this app +declared no copy", and the boot continues. Nothing throws and nothing logs. + +What the developer gets instead is the wrong strings. `I18nServicePlugin` +(`@objectstack/service-i18n`) is never registered, so the `i18n` slot keeps the +core in-memory fallback: `os dev` serves message KEYS, or last release's copy, +for an app that declared real translations. It reads as "the translations are +broken", not as "a collection went missing", which is why it is a reader fix +rather than a footnote. + +The detection now reads the flattened top level FIRST and then each package +body, in the order `resolveArtifactPackageOrder` (`@objectstack/core`, +ADR-0130 D4+D5) registers them: + +- **Every artifact the platform emits today answers bit-identically.** The + flattened level still answers first and short-circuits, so the `packages[]` + pass can only supply a declaration the top level did not have. This is the + reader half of the ruled order (readers first, emitter last, the artifact + additive throughout), so it lands with no change to what any command emits. +- **The caller's original expression is preserved, not re-expressed.** + `Array.isArray(t) && t.length > 0` still decides the top level, per package + body as well — re-expressing a gate as a resolved-and-counted traversal is + what silently changes the verdict for a stack that declares the key empty. +- **⛔ `stack.packages` is not iterated directly.** + `resolveArtifactPackageOrder` is the platform's one traversal and also the + GATE that parses each entry, so a second traversal would disagree with the + load path about which artifacts are loadable. An artifact with no `packages` + key is left entirely on the old path — the key's absence is checked before + the call, because D4's second branch would otherwise hand the caller's own + object back and read the same `translations` twice. +- **A malformed `packages` is refused, not skipped.** A non-array `packages`, + an entry inlined instead of wrapped under `manifest:`, or a duplicate package + id raises the same ADR-0112 envelope (`code` + `status: 422`) that + `ObjectQL.registerApp` raises for the same object later in the same boot. + +The decision — detection plus the locales it derives — is now one exported +function, `devI18nPluginOptions`, so the #15004 option-B acceptance pin +measures it by CALLING it rather than re-implementing the read. `DevPlugin` +keeps the dynamic import and its degradation: those are about the optional +package being installed, which is a different question from what the stack +declares. diff --git a/packages/cli/test/option-b-reader-acceptance.pin.test.ts b/packages/cli/test/option-b-reader-acceptance.pin.test.ts index 898c80a92f..62ae5777d1 100644 --- a/packages/cli/test/option-b-reader-acceptance.pin.test.ts +++ b/packages/cli/test/option-b-reader-acceptance.pin.test.ts @@ -143,10 +143,6 @@ const OPTION_B_LOSSES: readonly string[] = [ 'B2 · AppPlugin ql.setDatasourceMapping (object routing) (from source) · datasourceMapping', 'B2 · AppPlugin seed datasets merged (from source) · data', 'B2 · AppPlugin translation loading into the i18n service (from source) · translations', - // [#15232] The by-shape sweep (#15210) found this site, not this pin — so it - // is LEDGERED here first, red, before the reader beside it is touched. The - // fix and this line's deletion land in the next commit. - 'B2 · plugin-dev I18nServicePlugin auto-detect over the caller-supplied stack · translations', 'B2 · plugin-security appSecurityPluginOptions over the from-source config (default permission set) · permissions', 'B2 · runtime collectBundleActions over the from-source config · actions + objects[].actions', 'B2 · runtime collectBundleFunctionEntries over the from-source config · functions', diff --git a/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts b/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts new file mode 100644 index 0000000000..686cf89fb2 --- /dev/null +++ b/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts @@ -0,0 +1,264 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #15232 — the i18n auto-detect reads `translations` from `packages[]` too. +// +// ── The defect ───────────────────────────────────────────────────────────── +// +// `DevPlugin`'s 3b block decided whether to register the file-based +// `I18nServicePlugin` from `stack.translations` alone. A multi-package artifact +// under ADR-0130 D4's option-B shape carries each definition once, inside +// `packages[]`, with no flattened top level — so the read returned `undefined`, +// the detection said "this app declares no copy", and `os dev` booted on the +// core in-memory i18n fallback. Nothing throws. Nothing logs. The developer +// sees message keys, or last release's strings, where the app declared real +// translations. +// +// ── What is pinned here, and in which direction ──────────────────────────── +// +// Both directions, because only the pair is a discrimination: +// +// - today's ADDITIVE artifact answers exactly as it did before (the flattened +// level answers FIRST and short-circuits — the caller's original expression +// is preserved, not re-expressed, which is the trap #15006 measured); +// - the option-B artifact, whose ONLY copy is under `packages[]`, is now +// detected — the row this card added to the #15004 ledger and then deleted. +// +// The last case boots the real `DevPlugin` rather than only calling the +// decision, because what a developer experiences is the SERVICE: the plugin has +// to reach `new I18nServicePlugin(...)` with the locales the detection derived. +// `@objectstack/service-i18n` is mocked to a recording double for that arm +// (present and constructible), and every other optional package is mocked +// ABSENT — the #3060 convention in this package's sibling tests, which keeps a +// dev-assembly boot off the vite transform hot path. + +import { describe, it, expect, vi } from 'vitest'; +import { composeStacks, defineStack, type ObjectStackDefinition } from '@objectstack/spec'; + +import { devI18nPluginOptions } from './dev-i18n'; +import { DevPlugin } from './dev-plugin'; + +const absent = (name: string): Error => + Object.assign(new Error(`Cannot find package '${name}'`), { code: 'ERR_MODULE_NOT_FOUND' }); + +/** The one package that must be PRESENT: the arm under test constructs it. */ +const i18nConstructions = vi.hoisted(() => [] as unknown[]); +vi.mock('@objectstack/service-i18n', () => ({ + I18nServicePlugin: class { + name = 'com.objectstack.service.i18n'; + type = 'service' as const; + version = '1.0.0'; + constructor(options: unknown) { i18nConstructions.push(options); } + async init(): Promise { /* the recorder needs no behaviour */ } + }, +})); + +vi.mock('@objectstack/objectql', () => { throw absent('@objectstack/objectql'); }); +vi.mock('@objectstack/runtime', () => { throw absent('@objectstack/runtime'); }); +vi.mock('@objectstack/driver-memory', () => { throw absent('@objectstack/driver-memory'); }); +vi.mock('@objectstack/service-storage', () => { throw absent('@objectstack/service-storage'); }); +vi.mock('@objectstack/service-realtime', () => { throw absent('@objectstack/service-realtime'); }); +vi.mock('@objectstack/plugin-auth', () => { throw absent('@objectstack/plugin-auth'); }); +vi.mock('@objectstack/plugin-security', () => { throw absent('@objectstack/plugin-security'); }); +vi.mock('@objectstack/plugin-hono-server', () => { throw absent('@objectstack/plugin-hono-server'); }); +vi.mock('@objectstack/rest', () => { throw absent('@objectstack/rest'); }); +vi.mock('@objectstack/setup', () => { throw absent('@objectstack/setup'); }); +vi.mock('@objectstack/account', () => { throw absent('@objectstack/account'); }); + +// ─── The two-package fixture, in both shapes ──────────────────────────────── + +const CORE_ID = 'com.example.i18n.core'; +const MODULE_ID = 'com.example.i18n.orders'; + +const corePackage = (): ObjectStackDefinition => + defineStack({ + manifest: { + id: CORE_ID, + name: 'I18n Probe Core', + namespace: 'i18nprobe', + version: '1.0.0', + type: 'app', + }, + objects: [ + { + name: 'i18nprobe_account', + label: 'Account', + pluralLabel: 'Accounts', + sharingModel: 'private', + fields: { name: { name: 'name', type: 'text', label: 'Name', required: true } }, + }, + ], + // The whole point: the app's declared COPY lives in a package. + translations: [ + { en: { objects: { i18nprobe_account: { label: 'Account (translated)' } } } }, + ], + }); + +const modulePackage = (): ObjectStackDefinition => + defineStack({ + manifest: { + id: MODULE_ID, + name: 'I18n Probe Orders', + namespace: 'i18nprobe', + version: '1.0.0', + type: 'module', + dependencies: { [CORE_ID]: '^1.0.0' }, + }, + objects: [ + { + name: 'i18nprobe_order', + label: 'Order', + pluralLabel: 'Orders', + sharingModel: 'private', + fields: { name: { name: 'name', type: 'text', label: 'Number', required: true } }, + }, + ], + }); + +/** Today's emitted shape: flattened top level PLUS `packages[]`. */ +const additiveProject = (): Record => + composeStacks([modulePackage(), corePackage()], { manifest: 'preserve' }) as unknown as Record; + +/** The ruled option-B shape, for the one collection this reader reads. */ +const optionBProject = (): Record => { + const composed = additiveProject(); + delete composed.translations; + return composed; +}; + +const mockCtx = () => { + const registered = new Map(); + const info: string[] = []; + const ctx = { + logger: { + info: (line: unknown) => { if (typeof line === 'string') info.push(line); }, + debug: () => undefined, + warn: () => undefined, + error: () => undefined, + }, + getService: (name: string) => { + if (registered.has(name)) return registered.get(name); + throw new Error(`service not found: ${name}`); + }, + getServices: () => new Map(), + registerService: (name: string, svc: unknown) => { registered.set(name, svc); }, + hook: () => undefined, + trigger: () => undefined, + getKernel: () => undefined, + }; + return { ctx, info }; +}; + +describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', () => { + it('CONTROL — the fixture really carries the translations under `packages[]`', () => { + // Anti-vacuity: every "detected" below is a READER resolving `packages[]`, + // never a fixture that quietly kept a flattened copy. + const optionB = optionBProject(); + expect(optionB.translations).toBeUndefined(); + const bodies = (optionB.packages as Array<{ manifest?: { id?: string; translations?: unknown[] } }>); + expect(bodies.map((p) => p.manifest?.id).sort()).toEqual([CORE_ID, MODULE_ID]); + expect(bodies.find((p) => p.manifest?.id === CORE_ID)?.manifest?.translations).toHaveLength(1); + }); + + it("BASELINE — today's additive artifact answers exactly as it did before", () => { + const additive = additiveProject(); + expect(Array.isArray(additive.translations)).toBe(true); + expect(devI18nPluginOptions(additive)).toEqual({ defaultLocale: undefined, fallbackLocale: 'en' }); + }); + + it('THE FIX — the option-B artifact is detected through `packages[]`', () => { + expect(devI18nPluginOptions(optionBProject())).toEqual({ defaultLocale: undefined, fallbackLocale: 'en' }); + }); + + it('the flattened level answers FIRST — `packages[]` is not even traversed', () => { + // Two things at once, and the malformed `packages` is what proves the + // first: the original expression short-circuits, so today's additive + // artifact cannot start refusing anything it accepted before. + let reads = 0; + const stack = { + manifest: { id: CORE_ID, name: 'x', version: '1.0.0', type: 'app' }, + get translations() { reads += 1; return [{ en: { objects: {} } }]; }, + packages: 'not an array — this would be refused if it were reached', + }; + expect(devI18nPluginOptions(stack)).toEqual({ defaultLocale: undefined, fallbackLocale: 'en' }); + expect(reads).toBe(1); + }); + + it('a single-package stack reads its `translations` ONCE, and never throws', () => { + // D4's second branch returns the CALLER'S OWN OBJECT as the single package + // body, so an unguarded walk would read the same key a second time. The + // guard is what keeps every single-package stack on exactly the old path. + let reads = 0; + const stack = { + manifest: { id: CORE_ID, name: 'x', version: '1.0.0', type: 'app' }, + get translations() { reads += 1; return []; }, + }; + expect(devI18nPluginOptions(stack)).toBeUndefined(); + expect(reads).toBe(1); + }); + + it('an EMPTY top-level `translations` is not an answer — `packages[]` supplies it', () => { + // `[]` is falsy for the original expression (`length > 0`), so this is the + // case where `packages[]` legitimately supplies what the top level lacks. + const stack = { ...optionBProject(), translations: [] }; + expect(devI18nPluginOptions(stack)).toEqual({ defaultLocale: undefined, fallbackLocale: 'en' }); + }); + + it('locales still come from the stack `i18n` config, which option B does not move', () => { + // `i18n` is an artifact ENVELOPE key, not a package-owned collection, so it + // stays at the top level in both shapes and this limb loses nothing. + const stack = { ...optionBProject(), i18n: { defaultLocale: 'zh-CN', fallbackLocale: 'en-US' } }; + expect(devI18nPluginOptions(stack)).toEqual({ defaultLocale: 'zh-CN', fallbackLocale: 'en-US' }); + }); + + it('a malformed `packages[]` is REFUSED with an ADR-0112 envelope, not skipped', () => { + // The gate travels with the read: `resolveArtifactPackageOrder` is also + // what refuses this artifact at registration, so swallowing it here would + // resolve an i18n posture out of a package list nothing else accepts. + const stack = { + manifest: { id: CORE_ID, name: 'x', version: '1.0.0', type: 'app' }, + // An entry inlined instead of wrapped as `{ manifest: { … } }`. + packages: [{ id: CORE_ID, name: 'x', version: '1.0.0', type: 'app' }], + }; + let caught: (Error & { code?: string; status?: number }) | undefined; + try { + devI18nPluginOptions(stack); + } catch (err) { + caught = err as Error & { code?: string; status?: number }; + } + expect(caught?.code).toBe('INVALID_ARTIFACT_PACKAGE_ENTRY'); + expect(caught?.status).toBe(422); + expect(caught?.message).toContain('packages[0]'); + }); + + // ── What the developer actually gets: the SERVICE ───────────────────────── + + const bootWith = async (stack: Record | undefined) => { + i18nConstructions.length = 0; + const { ctx, info } = mockCtx(); + await new DevPlugin({ + seedAdminUser: false, + stack, + services: { + objectql: false, driver: false, auth: false, setup: false, server: false, + rest: false, dispatcher: false, security: false, storage: false, + 'file-storage': false, realtime: false, + }, + }).init(ctx as never); + return { constructions: [...i18nConstructions], info }; + }; + + it('BOOT — a multi-package app under option B gets the file-based I18nServicePlugin', async () => { + const { constructions, info } = await bootWith(optionBProject()); + expect(constructions).toEqual([{ defaultLocale: undefined, fallbackLocale: 'en' }]); + expect(info.some((l) => l.includes('I18nServicePlugin auto-registered'))).toBe(true); + }); + + it('BOOT — a stack declaring no copy at all still gets no I18nServicePlugin', async () => { + // The negative control. Without it the assertion above would pass for a + // detection that fires unconditionally. + const bare = { manifest: { id: CORE_ID, name: 'x', version: '1.0.0', type: 'app' } }; + const { constructions, info } = await bootWith(bare); + expect(constructions).toEqual([]); + expect(info.some((l) => l.includes('I18nServicePlugin auto-registered'))).toBe(false); + }); +}); diff --git a/packages/plugins/plugin-dev/src/dev-i18n.ts b/packages/plugins/plugin-dev/src/dev-i18n.ts index 05d978b592..8ecb11ff79 100644 --- a/packages/plugins/plugin-dev/src/dev-i18n.ts +++ b/packages/plugins/plugin-dev/src/dev-i18n.ts @@ -1,5 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +import { resolveArtifactPackageOrder } from '@objectstack/core'; + /** * The options `DevPlugin` hands `I18nServicePlugin` when it auto-registers it. * @@ -21,17 +23,94 @@ const declaresTranslationArray = (body: unknown): boolean => { }; /** - * Does this stack DECLARE translations? - * - * ⚠️ TOP LEVEL ONLY, which is the defect #15232 exists to fix — recorded here - * so this intermediate commit is not mistaken for the fix. A multi-package - * artifact under ADR-0130 D4's option-B shape carries `translations` inside - * `packages[]` and nothing at the top level, so this answers `false` and the - * dev server keeps the in-memory i18n fallback with nothing thrown and nothing - * logged. The probe row landing in the same commit ledgers exactly that loss. + * [ADR-0130 D4, #15232] Does this stack DECLARE translations — at the flattened + * top level, or inside `packages[]`? + * + * ## What this exists to stop + * + * A multi-package artifact carries each definition twice today: flattened onto + * the top level, and again inside `packages[i].manifest`. Option B (the + * ADR-0130 D4 ruling on #14512) removes the flattened copy, so `packages[]` + * carries it once. A reader that only ever looked at the top level does not + * fail when that happens — `stack.translations` is simply `undefined`, the + * detection answers "this app declared no copy", and the boot continues. + * + * For THIS reader the consequence is a dev server that serves the wrong + * strings. `I18nServicePlugin` (`@objectstack/service-i18n`) is never + * registered, so the `i18n` slot keeps the core in-memory fallback and the + * developer sees message KEYS, or last release's copy, where the app declared + * real translations. Nothing throws and nothing logs — the failure reads as + * "the translations are wrong", which is why it needs a reader fix rather than + * a footnote. + * + * ## Top level FIRST, `packages[]` only where it came back falsy + * + * The reader half of the program lands while the artifact is still ADDITIVE, so + * this has to be a superset of the old read rather than a replacement for it: + * every artifact the platform emits today still answers on the flattened level, + * bit-identically, and the `packages[]` pass can only supply a declaration the + * top level did not have — which is precisely the option-B shape. Keeping the + * caller's original expression as the first answer is also what stops the + * measured trap #15006 recorded: re-expressing a gate as a resolved-and-counted + * traversal silently changes the verdict for a stack that declares the key + * empty. Here the original expression is `Array.isArray(t) && t.length > 0` and + * it is preserved verbatim, both at the top level and per package body. + * + * ## The order is `resolveArtifactPackageOrder`'s, not the array's + * + * `resolveArtifactPackageOrder` (`@objectstack/core`, ADR-0130 D4+D5, #14643) + * is the ONE place that turns an artifact into its ordered package list, and it + * is also the GATE that parses each entry. ⛔ Do not iterate `stack.packages` + * directly: a second traversal is a second ordering, and this reader would then + * disagree with the load path about which artifacts are even loadable. + * + * ⚠️ Stated so the next reader does not mistake the reason: the ORDER itself is + * not observable through a boolean — any package declaring translations answers + * the same question. What is observable is the GATE. A hand-rolled loop over + * `stack.packages` would accept a duplicate package id, an unwrapped entry or a + * body carrying authoring-time globs and answer "this app declares copy" for an + * artifact the registration path refuses moments later. That is why the one + * traversal is used even where its ordering does not show. + * + * The `packages` guard above it is not an optimisation. D4's second branch + * makes `resolveArtifactPackageOrder` return the CALLER'S OWN OBJECT as the + * single package body when the key is absent, so walking it unguarded would + * read the top-level `translations` a second time — the same answer, reached + * twice, for every single-package stack the platform has ever emitted. + * + * ## A malformed `packages[]` is refused, not skipped + * + * A non-array `packages`, an entry inlined instead of wrapped under `manifest:` + * or a duplicate package id raises an ADR-0112 envelope (`code` + + * `status: 422`) out of this call, and it is deliberately not caught. It is the + * same refusal `ObjectQL.registerApp` raises for the same object later in the + * same boot (the registration path IS reached from `AppPlugin.start`, measured + * on both shapes in #14512 comment 5523603341), so catching it here would + * resolve an i18n posture out of a package list nothing else will accept — the + * gate travels with the read. ⚠️ It can only be reached at all when the top + * level declares no translations, because the flattened answer returns first. + * + * ## `i18n` is NOT read from `packages[]`, and that is not an omission + * + * `i18n` is an artifact ENVELOPE key, not a package-owned collection — derived, + * not asserted: the package-owned set is `ObjectStackDefinitionSchema` ∩ + * `AssembledPackageBodySchema`, and `i18n` is in the seven-key complement + * (`ARTIFACT_ENVELOPE_KEYS`, pinned by #15004). An option-B artifact therefore + * still carries `stack.i18n` and `stack.manifest` exactly where they are today, + * so those two limbs of the detection lose nothing and are left untouched. + * `translations` is the one limb the strip moves. */ export function stackDeclaresTranslations(stack: unknown): boolean { - return declaresTranslationArray(stack); + // The caller's ORIGINAL expression, first and unchanged. + if (declaresTranslationArray(stack)) return true; + + const packages = asBag(stack)?.packages; + if (packages === undefined || packages === null) return false; + + for (const body of resolveArtifactPackageOrder(stack)) { + if (declaresTranslationArray(body)) return true; + } + return false; } /** @@ -45,11 +124,11 @@ export function stackDeclaresTranslations(stack: unknown): boolean { * a locale resolved from somewhere else. * * Three declarations trigger it, exactly as they have since the auto-detect was - * written: a `translations` collection (read through - * {@link stackDeclaresTranslations}), an `i18n` config on the stack or its - * manifest, or `manifest.translations` — the authoring manifest's glob - * patterns, which are a declaration of intent even before a bundle is - * assembled. + * written: a `translations` collection (now read through + * {@link stackDeclaresTranslations}, so `packages[]` counts), an `i18n` config + * on the stack or its manifest, or `manifest.translations` — the authoring + * manifest's glob patterns, which are a declaration of intent even before a + * bundle is assembled. * * Exported because the #15004 option-B acceptance probe measures this decision * by CALLING it. A probe that re-implemented the read would be a second copy of From c60f2b44d7ed992ea0a0e4bb379823025e3886be Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 08:53:20 +0000 Subject: [PATCH 4/5] test(plugin-dev): drop the redundant driver-memory mock the census ledger locks (#15232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:driver-memory-census` counts `vi.mock` as a DECLARATION, and that package's consumer set is locked by maintainer ruling (#5499 froze investment, #5704 / #6664 ruled each remaining consumer at two). The mock copied in from this package's sibling harnesses made this file a third consumer. ⛔ Not ledgered — MIGRATED, which here means deleted: the line was redundant. `dev-plugin.ts` has exactly one `import('@objectstack/driver-memory')` and it sits inside `if (enabled('driver'))`, while both boots in this file pass `services: { driver: false }`, so the specifier was never reached. Measured rather than argued: with the line gone the suite is unchanged at `7 passed (7) / 68 passed (68)`, both BOOT cases green (5ms / 1ms), and the census gate returns to `OK — 2 ruled consumers` with its ledger untouched. A comment stands where the mock was, so the next author copying the sibling harness does not re-add it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- .../plugin-dev/src/dev-i18n-packages-reader.test.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts b/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts index 686cf89fb2..aa4d81ae2c 100644 --- a/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts +++ b/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts @@ -54,7 +54,17 @@ vi.mock('@objectstack/service-i18n', () => ({ vi.mock('@objectstack/objectql', () => { throw absent('@objectstack/objectql'); }); vi.mock('@objectstack/runtime', () => { throw absent('@objectstack/runtime'); }); -vi.mock('@objectstack/driver-memory', () => { throw absent('@objectstack/driver-memory'); }); +// ⛔ NO `@objectstack/driver-memory` mock here, deliberately — do not copy one in +// from the sibling harnesses. `vi.mock` counts as a DECLARATION to +// `scripts/check-driver-memory-census.mjs`, and that package's consumer set is +// locked by maintainer ruling (#5499 froze investment, #5704 / #6664 ruled each +// remaining consumer). A third test consumer is the #6664 defect itself, not a +// bookkeeping chore. Nothing here needs it: every boot below passes +// `services: { driver: false }`, and `dev-plugin.ts`'s ONE +// `import('@objectstack/driver-memory')` sits inside `if (enabled('driver'))`, +// so the specifier is never reached. Measured, not assumed: with the line gone +// this suite is unchanged at 68 passed, both BOOT cases below green in 5ms and +// 1ms — timings a real `import()` of that package would not fit in. vi.mock('@objectstack/service-storage', () => { throw absent('@objectstack/service-storage'); }); vi.mock('@objectstack/service-realtime', () => { throw absent('@objectstack/service-realtime'); }); vi.mock('@objectstack/plugin-auth', () => { throw absent('@objectstack/plugin-auth'); }); From c108823e9473b8a229263f42aa20945e64466ce5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 09:12:10 +0000 Subject: [PATCH 5/5] fix(plugin-dev): degrade on a refused package list, ask the cheap limbs first, document what throws (#15232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contract review (#15282) measured three things wrong with the first cut, and this is all three: 1. REACHABILITY. "Today's artifacts never reach the packages[] pass" was FALSE. The flattened read short-circuits only when `translations` is non-empty, so every multi-package stack that does not translate reaches the gate on every boot. The claim is corrected wherever it appeared and pinned by a case that counts the reads (2 when the gate is reached, 0 when it short-circuits). 2. A REPRODUCED REGRESSION. A package manifest still carrying authoring glob `objects` is refused by `ArtifactPackageSchema` by design; such a project boots today and would have stopped booting on this reader — thrown from the block whose only job is deciding whether to register a translation service, while `new AppPlugin(...)` twenty lines above degrades the very same refusal to a log line. That inversion is indefensible, so `DevPlugin` now catches, prints its own line naming the METADATA defect and carrying the envelope verbatim, and boots on the in-memory fallback. ⛔ Not `reportOptionalLoadFailure` (it names a PACKAGE — the #7926 mis-attribution) and ⛔ never silent. `dev-plugin.ts`'s AppPlugin try/catch is untouched: whether DevPlugin should refuse malformed metadata at all is a separate maintainer question. 3. EVALUATION ORDER. The three limbs are now asked cheapest-first, so a stack that already declares its locales in `i18n` is no longer refused over a `packages` list its answer never needed. `||` is commutative — the answer is unchanged, only what can throw is. Both functions gain `@throws`, including the BARE `Error` (no code, no status) `resolvePluginOrder` raises for a dependency cycle, now asserted by a case. The guard divergence with the sibling reader (`Array.isArray` vs absent-key) is recorded as a program-level split rather than unilaterally aligned. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- .../src/dev-i18n-packages-reader.test.ts | 142 +++++++++++++++++- packages/plugins/plugin-dev/src/dev-i18n.ts | 103 ++++++++++++- packages/plugins/plugin-dev/src/dev-plugin.ts | 40 ++++- 3 files changed, 267 insertions(+), 18 deletions(-) diff --git a/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts b/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts index aa4d81ae2c..bd106b235b 100644 --- a/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts +++ b/packages/plugins/plugin-dev/src/dev-i18n-packages-reader.test.ts @@ -23,6 +23,16 @@ // - the option-B artifact, whose ONLY copy is under `packages[]`, is now // detected — the row this card added to the #15004 ledger and then deleted. // +// Two more, added after an adversarial contract review measured the first draft +// of this file claiming more than it pinned: +// +// - a composed multi-package stack with NO i18n anywhere DOES reach +// `resolveArtifactPackageOrder` (counted, not argued). The short-circuit is +// real only for a stack whose flattened `translations` is non-empty. +// - therefore the gate's refusals are reachable on the ORDINARY path, so a +// project the gate refuses — one whose package manifest still carries +// authoring globs — must keep booting. It does, loudly. +// // The last case boots the real `DevPlugin` rather than only calling the // decision, because what a developer experiences is the SERVICE: the plugin has // to reach `new I18nServicePlugin(...)` with the locales the detection derived. @@ -128,6 +138,24 @@ const modulePackage = (): ObjectStackDefinition => const additiveProject = (): Record => composeStacks([modulePackage(), corePackage()], { manifest: 'preserve' }) as unknown as Record; +/** + * The SAME composition with no i18n anywhere — no `translations` at any level, + * no `i18n` config, no `manifest.translations`. This is the ordinary + * multi-package app that simply does not translate, and it is the shape the + * reachability claim turns on. + */ +const additiveNoI18nProject = (): Record => { + const composed = composeStacks( + [modulePackage(), { ...corePackage(), translations: undefined } as ObjectStackDefinition], + { manifest: 'preserve' }, + ) as unknown as Record; + delete composed.translations; + for (const entry of composed.packages as Array<{ manifest?: Record }>) { + delete entry.manifest?.translations; + } + return composed; +}; + /** The ruled option-B shape, for the one collection this reader reads. */ const optionBProject = (): Record => { const composed = additiveProject(); @@ -138,12 +166,13 @@ const optionBProject = (): Record => { const mockCtx = () => { const registered = new Map(); const info: string[] = []; + const errors: string[] = []; const ctx = { logger: { info: (line: unknown) => { if (typeof line === 'string') info.push(line); }, debug: () => undefined, warn: () => undefined, - error: () => undefined, + error: (line: unknown) => { if (typeof line === 'string') errors.push(line); }, }, getService: (name: string) => { if (registered.has(name)) return registered.get(name); @@ -155,7 +184,7 @@ const mockCtx = () => { trigger: () => undefined, getKernel: () => undefined, }; - return { ctx, info }; + return { ctx, info, errors }; }; describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', () => { @@ -179,10 +208,11 @@ describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', () expect(devI18nPluginOptions(optionBProject())).toEqual({ defaultLocale: undefined, fallbackLocale: 'en' }); }); - it('the flattened level answers FIRST — `packages[]` is not even traversed', () => { - // Two things at once, and the malformed `packages` is what proves the - // first: the original expression short-circuits, so today's additive - // artifact cannot start refusing anything it accepted before. + it('the flattened level answers FIRST **when it has something to say** — `packages[]` is not traversed then', () => { + // ⚠️ Scope, stated because an earlier draft of this file read this case as + // proof of something wider: it pins the short-circuit for a stack whose top + // level ALREADY declares translations. It says nothing about a stack that + // declares none — that case is the one below, and it reaches the gate. let reads = 0; const stack = { manifest: { id: CORE_ID, name: 'x', version: '1.0.0', type: 'app' }, @@ -206,6 +236,69 @@ describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', () expect(reads).toBe(1); }); + it('a composed multi-package stack with NO i18n DOES reach the artifact gate — and answers undefined without throwing', () => { + // The measurement that falsified this PR's first draft ("for every artifact + // the platform produces today the packages[] pass is not even reached"). + // It is reached, on the ordinary path, for every multi-package app that + // does not translate — so the walk is real work and its refusals are + // reachable in ordinary use, which is why `DevPlugin` degrades on them. + // + // `packages` is read TWICE when the gate is reached and ZERO times when the + // flattened level short-circuits: once by this reader's own absent-key + // guard, once inside `resolveArtifactPackageOrder`. That second read is the + // discriminator, so the assertion is on it and not on "at least one". + let packagesReads = 0; + const project = additiveNoI18nProject(); + const counted = new Proxy(project, { + get(target, key, recv) { + if (key === 'packages') packagesReads += 1; + return Reflect.get(target, key, recv); + }, + }); + + expect(project.translations).toBeUndefined(); + expect((project.packages as unknown[]).length).toBe(2); + expect(() => devI18nPluginOptions(counted)).not.toThrow(); + expect(devI18nPluginOptions(counted)).toBeUndefined(); + expect(packagesReads).toBeGreaterThanOrEqual(2); + }); + + it('a stack that already declares its locales is answered WITHOUT walking `packages[]`', () => { + // The limbs are asked cheapest-first: an `i18n` config answers the question + // on its own, so a `packages` list that answer never needed cannot refuse + // it. Before the reorder this threw INVALID_ARTIFACT_PACKAGES. + const stack = { + ...additiveNoI18nProject(), + i18n: { defaultLocale: 'zh-CN' }, + packages: 'not an array — would be refused if this limb were reached', + }; + expect(devI18nPluginOptions(stack)).toEqual({ defaultLocale: 'zh-CN', fallbackLocale: 'zh-CN' }); + }); + + it('a dependency CYCLE between two packages throws a BARE Error — no `code`, no `status`', () => { + // Documented under @throws because it is the one refusal here that does not + // carry the ADR-0112 envelope: it comes from `resolvePluginOrder`, the + // platform's one topological sorter, not from the artifact gate. A caller + // matching on `code` alone would miss it — `DevPlugin`'s catch does not. + const cyclic = { + manifest: { id: 'a', name: 'A', version: '1.0.0', type: 'app' }, + packages: [ + { manifest: { id: 'a', name: 'A', version: '1.0.0', type: 'app', dependencies: { b: '^1.0.0' } } }, + { manifest: { id: 'b', name: 'B', version: '1.0.0', type: 'module', dependencies: { a: '^1.0.0' } } }, + ], + }; + let caught: (Error & { code?: unknown; status?: unknown }) | undefined; + try { + devI18nPluginOptions(cyclic); + } catch (err) { + caught = err as Error & { code?: unknown; status?: unknown }; + } + expect(caught).toBeInstanceOf(Error); + expect(caught?.message).toContain('Circular dependency detected'); + expect(caught?.code).toBeUndefined(); + expect(caught?.status).toBeUndefined(); + }); + it('an EMPTY top-level `translations` is not an answer — `packages[]` supplies it', () => { // `[]` is falsy for the original expression (`length > 0`), so this is the // case where `packages[]` legitimately supplies what the top level lacks. @@ -244,7 +337,7 @@ describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', () const bootWith = async (stack: Record | undefined) => { i18nConstructions.length = 0; - const { ctx, info } = mockCtx(); + const { ctx, info, errors } = mockCtx(); await new DevPlugin({ seedAdminUser: false, stack, @@ -254,7 +347,7 @@ describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', () 'file-storage': false, realtime: false, }, }).init(ctx as never); - return { constructions: [...i18nConstructions], info }; + return { constructions: [...i18nConstructions], info, errors }; }; it('BOOT — a multi-package app under option B gets the file-based I18nServicePlugin', async () => { @@ -263,6 +356,39 @@ describe('#15232 — DevPlugin i18n auto-detect over a multi-package stack', () expect(info.some((l) => l.includes('I18nServicePlugin auto-registered'))).toBe(true); }); + it('BOOT — a project the ADR-0130 D4 gate REFUSES still boots, loudly, on the fallback', async () => { + // The regression this posture exists to prevent, reproduced: one package's + // authoring manifest still declares glob `objects` (`ManifestSchema`'s + // written form, and the repo's own CONFIG_GLOBS fixture in + // packages/cli/test/build-multi-package-artifact.e2e.test.ts). Such a body + // is refused by `ArtifactPackageSchema` BY DESIGN + // (packages/spec/src/assembled-package-body.test.ts). That project boots + // today; a reader that threw here would have stopped it booting — and from + // the block whose only job is deciding whether to register a translation + // service, while `new AppPlugin(...)` twenty lines above degrades the very + // same refusal to a log line. + const refused = additiveNoI18nProject(); + (refused.packages as Array<{ manifest: Record }>)[0] + .manifest.objects = ['./src/objects/*.object.ts']; + + // The reader itself still refuses — the gate travels with the read. + expect(() => devI18nPluginOptions(refused)).toThrow(); + + // The PLUGIN does not. It boots, says exactly what is wrong, and registers + // nothing. + const { constructions, info, errors } = await bootWith(refused); + expect(constructions).toEqual([]); + expect(info.some((l) => l.includes('I18nServicePlugin auto-registered'))).toBe(false); + const line = errors.find((l) => l.includes('i18n auto-detect could not read')); + expect(line, `no diagnosis line; errors were:\n${errors.join('\n')}`).toBeDefined(); + // ⛔ Never silent, and never mis-attributed to a missing package (#7926): + // the line names the metadata defect and carries the refusal verbatim. + expect(line).toContain('PACKAGE LIST is malformed'); + expect(line).toContain('INVALID_ARTIFACT_PACKAGE_ENTRY'); + expect(line).toContain('packages[0]'); + expect(line).not.toContain('not installed'); + }); + it('BOOT — a stack declaring no copy at all still gets no I18nServicePlugin', async () => { // The negative control. Without it the assertion above would pass for a // detection that fires unconditionally. diff --git a/packages/plugins/plugin-dev/src/dev-i18n.ts b/packages/plugins/plugin-dev/src/dev-i18n.ts index 8ecb11ff79..ef5e89819a 100644 --- a/packages/plugins/plugin-dev/src/dev-i18n.ts +++ b/packages/plugins/plugin-dev/src/dev-i18n.ts @@ -47,8 +47,10 @@ const declaresTranslationArray = (body: unknown): boolean => { * * The reader half of the program lands while the artifact is still ADDITIVE, so * this has to be a superset of the old read rather than a replacement for it: - * every artifact the platform emits today still answers on the flattened level, - * bit-identically, and the `packages[]` pass can only supply a declaration the + * the ANSWER for every artifact the platform emits today is bit-identical, + * because `composeStacks` merges `translations` with `'concat'` + * (`stack.zod.ts`), so a package that declares copy always leaves a non-empty + * flattened array too. The `packages[]` pass can only supply a declaration the * top level did not have — which is precisely the option-B shape. Keeping the * caller's original expression as the first answer is also what stops the * measured trap #15006 recorded: re-expressing a gate as a resolved-and-counted @@ -56,6 +58,21 @@ const declaresTranslationArray = (body: unknown): boolean => { * empty. Here the original expression is `Array.isArray(t) && t.length > 0` and * it is preserved verbatim, both at the top level and per package body. * + * ⚠️ **The same answer is NOT the same work, and an earlier draft of this file + * said it was.** The flattened read returns early only when `translations` is + * present and NON-EMPTY. So every stack that carries `packages[]` and declares + * no i18n at all — no `i18n` config, no `manifest.translations`, no non-empty + * top-level `translations`, i.e. the ordinary multi-package app that simply + * does not translate — DOES reach `resolveArtifactPackageOrder`, on every + * `DevPlugin.init`, and pays a full ADR-0130 D4 parse of every package body. + * Measured with a counting proxy on a real `composeStacks(…, 'preserve')` + * output, not reasoned about; the case is pinned below. Two things follow, and + * both are load-bearing: {@link devI18nPluginOptions} asks the cheap limbs + * FIRST so a stack that already declares its locales never pays this, and the + * refusals documented under `@throws` are reachable in ORDINARY use rather than + * only for exotic input — which is why `DevPlugin` degrades on them instead of + * dying. + * * ## The order is `resolveArtifactPackageOrder`'s, not the array's * * `resolveArtifactPackageOrder` (`@objectstack/core`, ADR-0130 D4+D5, #14643) @@ -85,10 +102,35 @@ const declaresTranslationArray = (body: unknown): boolean => { * `status: 422`) out of this call, and it is deliberately not caught. It is the * same refusal `ObjectQL.registerApp` raises for the same object later in the * same boot (the registration path IS reached from `AppPlugin.start`, measured - * on both shapes in #14512 comment 5523603341), so catching it here would - * resolve an i18n posture out of a package list nothing else will accept — the - * gate travels with the read. ⚠️ It can only be reached at all when the top - * level declares no translations, because the flattened answer returns first. + * on both shapes in #14512 comment 5523603341), so answering out of a package + * list nothing else will accept is not something this function does — the gate + * travels with the read. + * + * ⚠️ What that means for a CALLER is a separate question, and `DevPlugin` + * answers it the other way: it catches, says so loudly and boots anyway. The + * reason is not comfort — it is consistency with what already ships. Twenty + * lines above the i18n block, `new AppPlugin(this.options.stack)` parses the + * same object and its refusal is degraded to one log line, so a detector for + * "should I register a translation service" must not refuse harder than the + * gate for "should I register this app's metadata at all". The measured + * regression that made this concrete: a project whose package manifest still + * carries authoring-time glob `objects` is refused by `ArtifactPackageSchema` + * BY DESIGN, boots today, and would have stopped booting on this reader alone. + * ⛔ Whether `DevPlugin` should refuse malformed metadata outright is a + * maintainer question filed separately; it is not decided here, and this + * function's own semantics are unchanged by it. + * + * ## The guard divergence with `@objectstack/core`, recorded rather than fixed + * + * This reader treats only an ABSENT `packages` key (`undefined` / `null`) as + * "single package"; anything else goes to the gate, so `packages: {}` is + * REFUSED. `resolveArtifactPackageOrder`'s own second branch is spelled + * `declared === undefined || declared === null` too, but the sibling reader in + * `@objectstack/metadata` guards with `Array.isArray`, which silently accepts a + * non-array. Two readers in one program answering the same input differently is + * a program-level split, not this file's to settle (#15226 spells it as this + * file does). Recorded here so the next author does not "fix" one side into + * agreement without ruling the other. * * ## `i18n` is NOT read from `packages[]`, and that is not an omission * @@ -99,6 +141,24 @@ const declaresTranslationArray = (body: unknown): boolean => { * still carries `stack.i18n` and `stack.manifest` exactly where they are today, * so those two limbs of the detection lose nothing and are left untouched. * `translations` is the one limb the strip moves. + * + * @param stack - Any value. A non-object (`null`, a primitive, a function) and + * an object with no `translations` and no `packages` both answer `false` + * without reaching the gate. + * @returns Whether any level of this stack declares a non-empty `translations` + * array. + * @throws An ADR-0112 envelope (`Error & { code, status: 422 }`) from + * `resolveArtifactPackageOrder` when `packages` is present but not loadable: + * `INVALID_ARTIFACT_PACKAGES` (not an array), `INVALID_ARTIFACT_PACKAGE_ENTRY` + * (an entry that is not `{ manifest: … }`, a body carrying authoring-time + * globs where definitions belong, or a manifest with no usable id) or + * `DUPLICATE_ARTIFACT_PACKAGE`. + * @throws A **bare** `Error` — no `code`, no `status` — from `resolvePluginOrder` + * when two packages in `packages[]` depend on each other + * (`[Kernel] Circular dependency detected: …`). Documented because it is the + * one failure here that does NOT carry the envelope every other refusal in + * this repo does, so a caller matching on `code` alone will miss it. Measured, + * not inferred from the sorter's prose. */ export function stackDeclaresTranslations(stack: unknown): boolean { // The caller's ORIGINAL expression, first and unchanged. @@ -130,23 +190,50 @@ export function stackDeclaresTranslations(stack: unknown): boolean { * manifest's glob patterns, which are a declaration of intent even before a * bundle is assembled. * + * ## The three limbs are asked CHEAPEST FIRST, and that ordering is a fix + * + * `||` is commutative, so the ANSWER does not depend on the order — but which + * inputs can make this call throw does. The `translations` limb is the only one + * that can reach `resolveArtifactPackageOrder`, so asking it first meant a + * stack that had already stated its locales in `i18n` could still be refused + * over a `packages` list that answer never needed. The envelope limbs are pure + * property reads on the stack and its manifest; they are asked first, and only + * a stack that answers neither of them pays the package walk. + * * Exported because the #15004 option-B acceptance probe measures this decision * by CALLING it. A probe that re-implemented the read would be a second copy of * the code the reader program changes, and would stay red after the reader * beside it was fixed. + * + * @param stack - The caller-supplied stack (`new DevPlugin({ stack })`). + * @returns The options to construct `I18nServicePlugin` with, or `undefined` + * when this stack declares no i18n content at all. + * @throws Everything {@link stackDeclaresTranslations} throws, and only from + * that limb: the ADR-0112 envelopes (`code` + `status: 422`) for a `packages` + * list that is not loadable, and the **bare** `Error` (no `code`, no + * `status`) for a dependency cycle between two packages. ⚠️ A stack whose + * `i18n` / `manifest.i18n` / `manifest.translations` answers the question + * never reaches that limb and therefore never throws. `DevPlugin` catches + * both classes, reports the metadata defect and boots on the in-memory + * fallback — see `dev-plugin.ts`'s 3b block for why degrading is the + * consistent posture there. */ export function devI18nPluginOptions(stack: unknown): DevI18nPluginOptions | undefined { const bag = asBag(stack); if (!bag) return undefined; const manifest = asBag(bag.manifest); - const hasTranslations = stackDeclaresTranslations(stack); + // Cheapest first — see the docblock. These two are property reads that cannot + // throw; `stackDeclaresTranslations` is the limb that can reach the artifact + // gate, so it is asked LAST and only when the others answered no. const hasI18nConfig = !!(bag.i18n || manifest?.i18n); const hasManifestTranslations = !!( manifest && Array.isArray(manifest.translations) && manifest.translations.length > 0 ); - if (!hasTranslations && !hasI18nConfig && !hasManifestTranslations) return undefined; + if (!hasI18nConfig && !hasManifestTranslations && !stackDeclaresTranslations(stack)) { + return undefined; + } // `stack.i18n || stack.manifest.i18n || {}`, the original expression: the // stack's own config wins, the manifest's is the fallback, and neither being diff --git a/packages/plugins/plugin-dev/src/dev-plugin.ts b/packages/plugins/plugin-dev/src/dev-plugin.ts index dd6b4bad93..dce6439a0c 100644 --- a/packages/plugins/plugin-dev/src/dev-plugin.ts +++ b/packages/plugins/plugin-dev/src/dev-plugin.ts @@ -4,7 +4,7 @@ import { Plugin, PluginContext } from '@objectstack/core'; import { resolveAllowDegradedTenancy, resolveAllowDevPlugin, resolveTenancyPosture } from '@objectstack/types'; import { postureEnforcesWall } from '@objectstack/spec/security'; -import { devI18nPluginOptions } from './dev-i18n.js'; +import { devI18nPluginOptions, type DevI18nPluginOptions } from './dev-i18n.js'; /** * Dev Plugin Options @@ -531,7 +531,43 @@ export class DevPlugin implements Plugin { // instead of silently falling back to the in-memory i18n. The dynamic // import and its degradation stay HERE, because they are about the // optional PACKAGE being installed, not about what the stack declares. - const i18nOptions = devI18nPluginOptions(this.options.stack); + let i18nOptions: DevI18nPluginOptions | undefined; + try { + i18nOptions = devI18nPluginOptions(this.options.stack); + } catch (err) { + // [#15232] A metadata-SHAPE defect, degraded — deliberately, and NOT + // through `reportOptionalLoadFailure`. + // + // Two things are wrong with refusing here, and both were measured. The + // reach: this is not exotic input. A stack carrying `packages[]` that + // declares no i18n at all walks the whole package list on every boot, + // so an artifact the ADR-0130 D4 gate refuses — a package body still + // carrying authoring-time glob `objects`, for instance, which + // `ArtifactPackageSchema` rejects by design — arrives here on the + // ordinary path. The inversion: twenty lines above, `new AppPlugin(...)` + // parses the SAME object and its refusal is degraded to a log line, so + // refusing here would make "should I register a translation service?" + // a harder gate than "should I register this app's metadata at all?". + // A project like that boots today; it must keep booting. + // + // ⛔ Not `reportOptionalLoadFailure`: its text says the PACKAGE is + // installed but failed to initialize, and naming a package for a + // metadata-shape defect is exactly the mis-attribution #7926 removed + // from this file. ⛔ And not a silent skip either — silence is the + // failure class this whole change exists to remove. Its own line, its + // own diagnosis, carrying the refusal verbatim. + const code = (err as { code?: unknown })?.code; + ctx.logger.error( + ' ✘ the i18n auto-detect could not read this stack\'s `packages[]` — skipping ' + + 'I18nServicePlugin and continuing on the core in-memory i18n fallback. This is NOT ' + + 'a missing-package problem and installing anything will not help: the stack\'s ' + + 'PACKAGE LIST is malformed (ADR-0130 D4), and `os build` refuses the same project ' + + 'with its own compile diagnostic. Translations this app declares will not be ' + + 'served until it is fixed. The artifact reader reported (verbatim — the framework ' + + `does not interpret it): ${typeof code === 'string' ? `${code}: ` : ''}` + + `${err instanceof Error ? err.message : String(err)}`, + ); + } if (i18nOptions) { try {