Skip to content

Commit ca9d9d3

Browse files
claude[bot]claude
andauthored
fix(cli): widen check:app-nav-i18n from one app to the declared platform-app population (#17972)
Fixes #17891 Clause-②: no `packages/cli/scripts/check-app-nav-i18n.mjs` is named "every id labelled in every locale" and was structurally blind to an entire app. It now judges the **declared platform-app population** — Setup **and Account** — with the criterion that decides that population written down beside it. --- ## 1. Acceptance item 1 — the population, declared before any edit This is the **third** independent count. Triage named 3 sites plus the declaration; the claim comment's pre-check found 2 more. Re-derived here on `origin/main` `bdb247d9e`, over the 662-line file. `APP_NAME` occurs **10×** (`grep -c APP_NAME` = 10), and the full census of narrowing is wider than the spelling. ### Behavioural — these decide what is measured | site | what it is | named by triage | named by the claim pre-check | |:--|:--|:--:|:--:| | `:109` | `const APP_NAME = 'setup'` — the declaration every filter below reads | yes | yes | | `:581` | `if (contribution?.app !== APP_NAME) continue` — the contribution filter | yes | yes | | `:590` | `if (app?.name !== APP_NAME) continue` — the app-shell filter | yes | yes | | `:601` | `engine.registry.getApp(APP_NAME)` — the merged-app lookup | yes | yes | | `:628` | `data?.apps?.[APP_NAME]?.navigation` — the **locale-file lookup** | no | yes | | `:474` | `join(CLI_ROOT,'node_modules','@objectstack','setup','dist','index.mjs')` — a hard-coded **package** probe, not spelled `APP_NAME` at all | no | yes | | ⭐ `:173`–`:243` | the **`CONTRIBUTORS` roster itself** — ten entries, none of which registers the **Account app shell**. Narrowing **by absence**: no `setup` literal, no `APP_NAME`, and no grep of either finds it | **no** | **no** | ⭐ The roster is a real seventh site, not a refinement of the others. Widening `:581`/`:590`/`:601`/`:628`/`:474` alone leaves `engine.registry.getApp('account')` returning `undefined`, so the gate reports *"the `account` app is not registered at all"* instead of judging a single label. The shell ships in `@objectstack/account` (`createAccountAppPlugin`), which `os serve` loads in the ADR-0048 platform-app loop (`packages/cli/src/commands/serve.ts:4001`) and which `packages/cli` already depends on. ### Diagnostic — these decide which app the gate *reports* on `:162` · `:614` · `:621` · `:652` · `:660` are the five `APP_NAME`-interpolated strings the claim pre-check listed. ⭐ There is a **sixth** neither count named: `:496`, inside `buildPrerequisiteText` — *"This gate boots the real **Setup** composition"*. It is plain prose rather than an interpolation, so a grep for `APP_NAME` misses it, and after widening it would have described a Setup-only run while refusing on a missing **Account** build. All six now name the app they are actually about. ### Checked and rejected as sites — stated so the next reader does not re-open them - `:519` `SetupAppTranslations` — a setup-**named** export that in fact carries every app: `en.ts` declares `apps.account` and `apps.setup` side by side. A naming hazard, **not** a narrowing; the locale source was already app-agnostic and `:628`'s subscript was the whole of it. Renaming it would touch `packages/platform-objects/**`, which is fenced (#17930). - `:262` · `:290`–`:320` — self-test fixtures named for Setup that exercise `collectNavIds` / `missingLabels`, both of which take the tree and the locale subtree as arguments. Already app-agnostic. - `:364` · `:412` — the refusal fixture pins that the advisory names *the path it was handed*, deliberately not a re-derived one. ### The criterion — stated, not defaulted An app is judged by this gate **iff both halves hold**: - **(a)** the composition root registers its shell **by default** — it is one of the app packages the ADR-0048 platform-app loop loads in `packages/cli/src/commands/serve.ts`; and - **(b)** at least one package contributes navigation into it **at runtime** (`manifest.navigationContributions[].app`), so a static walk cannot see every label it renders. Today that resolves to exactly `setup` and `account`. ⛔ **"All apps" is not the criterion**, and each half excludes something real: - `studio` fails **both**. The loop deliberately does not load it (*"@objectstack/studio is intentionally NOT default-loaded"*), and `STUDIO_APP` declares its whole navigation statically with nothing contributing into it — `app-nav-translation-parity.test.ts` already owns that verdict by a static walk. Adding it here would be a second owner for one route and would drag in a package the composition root never boots. - `crm_app` fails **(a)** and has no locale source here at all: it is an `examples/` app and `SetupAppTranslations` carries no `apps.crm_app` subtree to compare against. `account` satisfied both halves all along. It was invisible anyway. --- ## 2. Acceptance item 2 — both directions fire Every exit code below was captured **before any pipe** (`cmd > log 2>&1; EXIT=$?`). **The mutation**: delete #17759's `en` Account `nav_connect_agent` entry from `packages/platform-objects/src/apps/translations/en.ts`, rebuild `@objectstack/platform-objects`, and prove the deletion reached the artifact the gate consumes. It was reverted; the diff of this PR does not touch that file. | leg | reading | |:--|:--| | on-disk mutation proof | `nav_connect_agent` occurrences in the source `3 → 1`; unique marker `1 → 0`; `git diff --stat` = 1 file, 10 deletions | | rebuild (mutate leg) | `os-verify-lock … VERDICT command-exit 0 · held the lock 27s` | | dist proof (mutate leg) | `ablation-dist-preflight … --absent` → exit **0**, *"marker absent from all 66 built files"* | | **must catch** — widened gate | exit **1** — ``apps.account.navigation — locale `en` has no label for 1 runtime-merged nav id(s): nav_connect_agent contributed by @objectstack/mcp (CONNECT_AGENT_UI_BUNDLE) — author's literal "Connect an Agent"`` | | **premise 1** — the pre-fix gate, same mutation, same rebuilt dist | exit **0**, and its line is **byte-identical** to the unmutated run: `cmp` exit **0**, sha256 `60a5f61d…` on both. That line is ``check-app-nav-i18n: OK (10 contributor(s), 54 merged `setup` nav id(s), 4 locale(s), every id labelled in every locale).`` — the card's reading, reproduced here rather than quoted | | restore leg | `git checkout HEAD --` → blob `da6690b9…` equals the HEAD blob; `git diff HEAD` empty; **whole-tree** `git status --porcelain` empty | | rebuild + dist proof (restore leg) | `VERDICT command-exit 0 · held the lock 28s`; preflight (present mode) exit **0**, *"marker present in 6 built files"*; ⛔ not skipped — a marker left in `dist/` would have kept the mutation live for every later run in this worktree | **Must not over-fire**, unmutated tree: ``` before check-app-nav-i18n: OK (10 contributor(s), 54 merged `setup` nav id(s), 4 locale(s), every id labelled in every locale). after check-app-nav-i18n: OK (11 contributor(s), 4 locale(s), 2 app(s) — setup: 54 merged nav id(s), account: 12 merged nav id(s) — every id labelled in every locale). ``` The `setup` verdict is unchanged and its count is unmoved: **54 → 54**, exit 0 → exit 0. The contributor count moves `10 → 11` because the Account **shell** is a new roster entry — that is the one number this change necessarily moves, and it is stated rather than smoothed. `account: 12` is the eleven ids `ACCOUNT_APP` declares plus the one `@objectstack/mcp` contributes. --- ## 3. The trap in widening, and the guard against it The per-contributor *"landed at least one nav id"* invariant is the anti-false-green half of this gate. Widening it into **one flat union across apps** reads as the obvious generalisation and is a **softening**: `@objectstack/mcp` serves both apps, so a union keeps it passing on its `account` id alone after its `setup` contribution has silently stopped — the exact *"fewer ids means fewer checks"* false green the invariant exists to catch, restated one app wider. So the invariant is applied **per app**: each `CONTRIBUTORS` entry declares its `apps`, and `contributorsWithNoNavIds` is called once per app over `contributionsForApp(...)` — the original helper is untouched. `--self-test` carries the union as a **negative control**: it constructs the union projection and requires it to be the spelling that goes green, so the pin cannot pass by tautology. New `--self-test` coverage, all with their own negative controls: the union softening; a verdict that names the wrong `apps` subtree; a pass line that cannot notice an app leaving the population; and three reachability pins tying `APPS`, `CONTRIBUTORS` and the shell packages together. --- ## 4. Published surface — measured, not assumed Widening added `contributionsForApp` and `summaryText` as module exports of the **script**. Measured against `packages/cli/package.json`: `files` is `["dist","README.md","CHANGELOG.md"]` and `exports` names only `.`, `./console`, `./hook-body` and `./package.json`. `packages/cli/scripts/**` is **not shipped and is not an entry point**, and the file already exported `collectNavIds`, `missingLabels` and `contributorsWithNoNavIds` before this PR. ⇒ **No export was added to a published entry point**, so the re-declaration the dispatch order reserved to the seat is not triggered. The changeset is written because the dispatch order requires one for `@objectstack/cli`; by the published-surface test alone nothing shipped moves. --- ## 5. Fences observed - ⛔ `packages/platform-objects/**` is **not** in this diff (#17930 owns mechanism two). It was mutated and restored for the ablation only, proven byte-identical by blob hash and a whole-tree `git status`. - ⛔ No new translation key, in any locale, for any app. #17759 already supplied the correct form; this is the last step of the standard repair order. - ⛔ `.claude/**`, `content/docs/releases/**`: untouched. - ⛔ The existing `setup` judgement is never narrowed, skipped or softened — see §2 and §3. ## Acceptance notes - `SetupAppTranslations` is a setup-named export carrying every app's subtree. Noted, not filed: the rename lands in `packages/platform-objects/**`, which this card is fenced out of, and it is a naming observation rather than a defect — nothing resolves wrongly because of it. Carrier: none today; #17930 is the next PR in that file's neighbourhood if a maintainer wants it swept. - The gate header's bound #2 ("No reverse direction") still holds and still holds for `account`: one booted composition cannot distinguish a gated-off contribution from a dead key, so the reverse direction for the new app stays outside this gate exactly as it does for Setup. - ⚠️ Two sibling devs are live in `packages/cli` (#17821 in `src/commands/`, #17853 in `vitest.config.ts`). No file overlap. The vitest tier population is derived at config load, so tier file counts may move under this branch; ⛔ not reconciled here. --- ## 6. Verification record Every exit code captured **before any pipe**. All readings taken in the dedicated worktree at `04216260f`, branched from `origin/main` `bdb247d9e`. | what | command | exit | |:--|:--|:--:| | the gate, as CI spells it | `pnpm --filter @objectstack/cli run check:app-nav-i18n` (self-test **and** real run) | **0** | | dependency closure build | `pnpm --workspace-concurrency=2 --filter '@objectstack/cli^...' build`, under the shared heavy-verify lock, slot `dev-17891` | **0** (`VERDICT command-exit 0 · held the lock 405s`) | | whole-repo build | `pnpm build --concurrency=2`, under the lock | **0** (`VERDICT command-exit 0 · held the lock 473s`; 73/73 tasks) | | affected package, unit tier | `pnpm --filter @objectstack/cli exec vitest run --project unit --maxWorkers=2`, under the lock | **0** — 204 files / 2935 tests passed | | affected package, typecheck | `pnpm --filter @objectstack/cli typecheck`, under the lock | **0** | | repo-wide style authority | `pnpm lint` (`eslint . --no-inline-config`), whole repo, not narrowed | **0** | | derived gate families | all **50** commands from `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, each run separately with its own exit code recorded | **49 × 0**, plus the one below | | reconciliation | `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran …` over that record | *"50 derived, 50 run, 0 NOT-MEASURED, 0 UNRUN — a DERIVED zero: all 50 recorded an exit code and none of them is 3"* | | control characters | `pnpm check:nul-bytes` plus a hand sweep of both changed files for every non-tab/newline C0 byte and DEL | **0** / no hits | `pnpm check:dual-build-cjs-loads` returned **exit 3 — `PREREQUISITE NOT MET`** on its first run: it reads built output for **every** package, and only the `@objectstack/cli` dependency closure was built at that point. ⛔ That is not a red gate and was not recorded as one — nothing was measured. The whole repo was then built and the family **re-run to a real verdict: exit 0**, 104 published require entry points across 67 packages. ### NOT MEASURED — declared, not smoothed over - **`packages/cli`'s `typecheck` does not reach the changed file.** `packages/cli/tsconfig.json` declares `include: ["src"]` with no `allowJs`, so `tsc --noEmit` never compiles `packages/cli/scripts/check-app-nav-i18n.mjs`. Its green says nothing about this diff. What does cover the file: `node --check`, the script's own `--self-test` (which the `check:app-nav-i18n` script runs before the real pass), and the real run itself. - **`packages/cli`'s `integration` tier was not run locally.** This diff touches no spawn entry point, no `bin/`, no `test/helpers/serve-process.ts` and no driver or kernel boot path, so by the tier rule only `unit` is owed here. CI's `pnpm test` runs both. - **The five CI jobs scheduled by these paths**, the 11 wide-population families, the 50 artifact-roster families and the always-runs tail are each outside the derived 50 and were not run locally — `dispatch-gates` names every one of them, and CI is their authority. - **`origin/main` moved during this run** (`bdb247d9e` → `225197cdb` at the time of the derivation). This branch is not rebased onto it; the merge is CI's and the queue's to validate. --- _Generated by [Claude Code](https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9fca8eb commit ca9d9d3

2 files changed

Lines changed: 350 additions & 47 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
`check:app-nav-i18n` now judges the PLATFORM APPS' navigation — Setup **and Account** — instead of narrowing to `setup` at every site.
6+
7+
The gate is named "every id labelled in every locale" and was structurally blind to one whole app: it printed a byte-identical `OK (10 contributor(s), 54 merged setup nav id(s), 4 locale(s), …)` line before and after the Account app's contributed `nav_connect_agent` label landed, so nothing it printed could tell you it had skipped an app.
8+
9+
Six sites narrowed it, only three of which were the obvious filters:
10+
11+
- the contribution filter, the app-shell filter and the merged-app lookup;
12+
- the **locale-file lookup** (`data.apps.<app>.navigation`) — widening the first three without this one yields a gate that collects `account` ids and then hunts for their labels under `apps.setup.navigation`;
13+
- the **build prerequisite**, a package path hard-coded to `@objectstack/setup`;
14+
- the **contributor roster**, which booted no package that registers the Account shell — so `account` had no merged app to judge at all.
15+
16+
Behaviour now:
17+
18+
- the population is declared with its criterion (an app is judged iff the ADR-0048 platform-app loop registers its shell by default **and** at least one package contributes navigation into it at runtime), which is why `studio` and `crm_app` are out;
19+
- the per-contributor "landed at least one nav id" invariant is applied **per app**, never over a union across apps — a union would let a contributor serving two apps keep passing on one of them after the other silently stopped;
20+
- every verdict, the refusal advisory and the pass line name the app they are actually about, and the pass line carries a per-app id count;
21+
- `--self-test` gains negative controls for the union softening, for a verdict that names the wrong app subtree, and for a pass line that cannot notice an app leaving the population.
22+
23+
The `setup` judgement is unchanged: the same 54 merged ids, the same verdict, and the same count in the pass line.

0 commit comments

Comments
 (0)