Skip to content

Commit 88a35c2

Browse files
hotlongclaude
andauthored
fix(plugin-dev): the i18n auto-detect resolves translations from packages[] (#15232) (#15282)
* test(cli,plugin-dev): ledger the i18n auto-detect's option-B loss, RED first (#15232) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m * test(cli): ledger the plugin-dev i18n loss so the fix has something to check against (#15232) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m * fix(plugin-dev): the i18n auto-detect resolves `translations` from `packages[]` (#15232) `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m * test(plugin-dev): drop the redundant driver-memory mock the census ledger locks (#15232) `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m * fix(plugin-dev): degrade on a refused package list, ask the cheap limbs first, document what throws (#15232) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8a1bad8 commit 88a35c2

10 files changed

Lines changed: 802 additions & 11 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/plugin-dev": patch
3+
---
4+
5+
fix(plugin-dev): the i18n auto-detect resolves `translations` from `packages[]`, not only the flattened top level (#15232)
6+
7+
`DevPlugin.init`'s 3b block read `options.stack.translations` and nothing else.
8+
For a multi-package app under the ADR-0130 D4 option-B shape — where
9+
`packages[]` carries each definition exactly once and the flattened top-level
10+
copy is gone — that read returns `undefined`, the detection concludes "this app
11+
declared no copy", and the boot continues. Nothing throws and nothing logs.
12+
13+
What the developer gets instead is the wrong strings. `I18nServicePlugin`
14+
(`@objectstack/service-i18n`) is never registered, so the `i18n` slot keeps the
15+
core in-memory fallback: `os dev` serves message KEYS, or last release's copy,
16+
for an app that declared real translations. It reads as "the translations are
17+
broken", not as "a collection went missing", which is why it is a reader fix
18+
rather than a footnote.
19+
20+
The detection now reads the flattened top level FIRST and then each package
21+
body, in the order `resolveArtifactPackageOrder` (`@objectstack/core`,
22+
ADR-0130 D4+D5) registers them:
23+
24+
- **Every artifact the platform emits today answers bit-identically.** The
25+
flattened level still answers first and short-circuits, so the `packages[]`
26+
pass can only supply a declaration the top level did not have. This is the
27+
reader half of the ruled order (readers first, emitter last, the artifact
28+
additive throughout), so it lands with no change to what any command emits.
29+
- **The caller's original expression is preserved, not re-expressed.**
30+
`Array.isArray(t) && t.length > 0` still decides the top level, per package
31+
body as well — re-expressing a gate as a resolved-and-counted traversal is
32+
what silently changes the verdict for a stack that declares the key empty.
33+
- **`stack.packages` is not iterated directly.**
34+
`resolveArtifactPackageOrder` is the platform's one traversal and also the
35+
GATE that parses each entry, so a second traversal would disagree with the
36+
load path about which artifacts are loadable. An artifact with no `packages`
37+
key is left entirely on the old path — the key's absence is checked before
38+
the call, because D4's second branch would otherwise hand the caller's own
39+
object back and read the same `translations` twice.
40+
- **A malformed `packages` is refused, not skipped.** A non-array `packages`,
41+
an entry inlined instead of wrapped under `manifest:`, or a duplicate package
42+
id raises the same ADR-0112 envelope (`code` + `status: 422`) that
43+
`ObjectQL.registerApp` raises for the same object later in the same boot.
44+
45+
The decision — detection plus the locales it derives — is now one exported
46+
function, `devI18nPluginOptions`, so the #15004 option-B acceptance pin
47+
measures it by CALLING it rather than re-implementing the read. `DevPlugin`
48+
keeps the dynamic import and its degradation: those are about the optional
49+
package being installed, which is a different question from what the stack
50+
declares.

packages/cli/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@
129129
},
130130
"devDependencies": {
131131
"@objectstack/driver-turso": "workspace:*",
132+
"@objectstack/plugin-dev": "workspace:*",
132133
"@oclif/plugin-help": "^6.2.58",
133134
"@oclif/plugin-plugins": "^5.4.87",
134135
"@types/better-sqlite3": "^7.6.13",

packages/cli/test/fixtures/option-b-reader-probe.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
*
1515
* - invokes a reader this repo SHIPS (`collectBundleActions`,
1616
* `resolveStandaloneDatabase`, `createStandaloneStack`,
17-
* `appSecurityPluginOptions`, …) and reports its return value, or
17+
* `appSecurityPluginOptions`, `devI18nPluginOptions`, …) and reports its
18+
* return value, or
1819
* - boots a real kernel carrying the real `AppPlugin` and reports what that
1920
* plugin HANDED to a subsystem (a job scheduled, a datasource connected, a
2021
* mapping set, an i18n service registered, a seed dataset merged).
@@ -62,6 +63,7 @@ import {
6263
resolveStandaloneDatabase,
6364
} from '@objectstack/runtime';
6465
import { appSecurityPluginOptions } from '@objectstack/plugin-security';
66+
import { devI18nPluginOptions } from '@objectstack/plugin-dev';
6567
import { ObjectStackDefinitionSchema, normalizeStackInput } from '@objectstack/spec';
6668

6769
// The lowering itself, not a copy of it — reached as SOURCE, by relative path,
@@ -364,6 +366,21 @@ export async function measureShape(project: unknown, projectRoot: string): Promi
364366
// loader — it is in the readers each of them then drives, which is what
365367
// these rows are.
366368

369+
// `DevPlugin` takes its stack from a CALLER-SUPPLIED object
370+
// (`new DevPlugin({ stack: config })`, the documented construction), so there
371+
// is no load boundary between the composed config and this reader — the same
372+
// object `os dev` boots from source is handed straight to the plugin. The row
373+
// calls the SHIPPED decision (`devI18nPluginOptions`), which is what the
374+
// plugin itself calls to decide whether to register `I18nServicePlugin`; a
375+
// row that re-read `stack.translations` here would be a second copy of the
376+
// read the reader program changes and would stay red after it was fixed.
377+
const devI18n = devI18nPluginOptions(project);
378+
rows.push(row(
379+
'B2 · plugin-dev I18nServicePlugin auto-detect over the caller-supplied stack · translations',
380+
devI18n ? `I18nServicePlugin(fallbackLocale=${devI18n.fallbackLocale})` : undefined,
381+
devI18n === undefined,
382+
));
383+
367384
const fromSourceProfile = appSecurityPluginOptions(project)?.fallbackPermissionSet;
368385
rows.push(row(
369386
'B2 · plugin-security appSecurityPluginOptions over the from-source config (default permission set) · permissions',

packages/cli/tsconfig.test.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,13 @@
151151
// into this program needs no widening.
152152
"paths": {
153153
"@objectstack/objectql": ["../objectql/src/index.ts"],
154+
// [#15232] The fourth, added for the same reason and under the same
155+
// no-star rule: the option-B probe calls `@objectstack/plugin-dev`'s
156+
// shipped i18n auto-detect decision, and this package's vitest config
157+
// aliases that same bare specifier to the same source entry, so the type
158+
// verdict and the run agree about which artifact is under test. The
159+
// package publishes only `"."`.
160+
"@objectstack/plugin-dev": ["../plugins/plugin-dev/src/index.ts"],
154161
"@objectstack/plugin-security": ["../plugins/plugin-security/src/index.ts"],
155162
"@objectstack/runtime": ["../runtime/src/index.ts"]
156163
}

packages/cli/vitest.config.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,23 @@ export default defineConfig({
618618
find: /^@objectstack\/plugin-auth$/,
619619
replacement: path.resolve(__dirname, '../plugins/plugin-auth/src/index.ts'),
620620
},
621+
// `test/fixtures/option-b-reader-probe.ts` (#15232) calls
622+
// `@objectstack/plugin-dev`'s shipped i18n auto-detect decision — the
623+
// option-B acceptance pin (#15004) measures readers by CALLING them, and
624+
// a reader resolved through `exports` to plugin-dev's **dist** would make
625+
// that row a verdict about the last build rather than about the reader
626+
// this card changes. The registry in `check-test-source-alias.mjs` is
627+
// SHRINK-ONLY, so widening `KNOWN_UNALIASED_TEST_IMPORTS` was never an
628+
// option; this entry is the sanctioned remedy, in the same anchored form
629+
// as its neighbours (plugin-dev publishes only `"."`, and the anchor is
630+
// what keeps that true if a subpath is ever added). Measured before and
631+
// after: the gate reports the same required set for this package in both
632+
// directions — crossing into `plugin-dev/src` adds no unaliased artifact
633+
// import it did not already carry.
634+
{
635+
find: /^@objectstack\/plugin-dev$/,
636+
replacement: path.resolve(__dirname, '../plugins/plugin-dev/src/index.ts'),
637+
},
621638
// `src/utils/protocol-version-gap.test.ts` (#13860) exercises the upgrade
622639
// advisory, whose verdict comes from `checkProtocolCompat` — the platform's
623640
// single reader of `engines.protocol`. The advisory is a thin direction

0 commit comments

Comments
 (0)