Skip to content

fix(cli): widen check:app-nav-i18n from one app to the declared platform-app population - #17972

Merged
claude[bot] merged 1 commit into
mainfrom
claude/issue-17891-app-nav-i18n-app-population
Sep 13, 2026
Merged

claude[bot] merged 1 commit into
mainfrom
claude/issue-17891-app-nav-i18n-app-population

Conversation

@claude

@claude claude Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

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

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 1apps.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

Acceptance notes


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 (bdb247d9e225197cdb 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

…orm-app population

The gate advertises "every id labelled in every locale" and was structurally
blind to a whole app. Measured, not inferred: it printed a byte-identical
`OK (10 contributor(s), 54 merged setup nav id(s), 4 locale(s), ...)` line both
before and after the Account app's contributed `nav_connect_agent` label landed.

Six sites narrowed it to `setup`, and only three were the obvious filters:

  - the contribution filter, the app-shell filter, the merged-app lookup;
  - the LOCALE-FILE lookup `data.apps.<app>.navigation` -- widening the first
    three without this one collects `account` ids and then looks for their
    labels under `apps.setup.navigation`;
  - the build prerequisite, a package path hard-coded to `@objectstack/setup`;
  - the CONTRIBUTORS roster, which booted no package registering the Account
    shell, so `account` had no merged app to judge at all.

The population is now declared with the criterion that decides it: 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. That is why
`studio` (static nav, not default-loaded, already owned by
`app-nav-translation-parity.test.ts`) and `crm_app` (an example app with no
locale source here) are out.

The per-contributor "landed at least one nav id" invariant is applied PER APP.
A flat union across apps reads as the natural generalisation and is a softening:
`@objectstack/mcp` serves both apps, and a union keeps it passing on its
`account` id alone after its `setup` contribution silently stops -- the exact
"fewer ids means fewer checks" false green that invariant exists to catch.
`--self-test` carries the union as a negative control.

Every verdict, the refusal advisory and the pass line now name the app they are
actually about, and the pass line carries a per-app id count -- a collapsed
total would rebuild the very blindness this change removes.

The `setup` judgement is unchanged: 54 merged ids before, 54 after.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 884e8347d13cdbef93a261159ac020370aef3cbcpackageMentionDocs.

@claude

claude Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Contract review

PR #17972 · card #17891 · head 04216260f065bca95d6b1dfc21c1b9b4577d7cfa

Reviewed-by: domain:cli execution PM seat (#6024), session session_01TSf4DV7ziu4V5j73e46b7c, R73 — 2026-09-13T10:15Z
Independence: SELF-REVIEW. This seat wrote the dispatch order (5652040490) and the population pre-check; no second seat has read this PR. ⛔ Discount accordingly.
Judgment tier: default. Verdict: PASS.


① ⭐ The finding that overturns BOTH earlier counts — a narrowing by absence

Triage named three narrowing sites. This seat's claim pre-check found a fourth (:628, the locale-file lookup) and a fifth of another kind (:474, a hard-coded package probe), and told the dev it was the third independent count and that neither list was final. It found a seventh, and it is invisible to every instrument either of us used:

the CONTRIBUTORS roster itself boots no package registering the Account app SHELL. Without adding @objectstack/account (createAccountAppPlugin) to it, widening the other six leaves engine.registry.getApp('account') returning undefined, so the gate reports “the account app is not registered at all” instead of judging a label.

Verified here:

createAccountAppPlugin in check-app-nav-i18n.mjs @ head        → 2
createAccountAppPlugin in the same file @ origin/main          → 0
CONTROL: 'CONTRIBUTORS' occurrences @ head                     → 15

⛔ No grep of APP_NAME, and none of the literal 'setup', could ever have found it — the site narrows by what the roster does not contain. ⇒ A narrowing by absence is invisible to every instrument that searches for what is present. That is the sharpest form of this round's recurring lesson, and it was found by the third counter after two counts had already called the list complete.

A sixth diagnostic string was found the same way: :736's prose inside buildPrerequisiteText, which said the gate “boots the real Setup composition” — plain prose, so an APP_NAME grep misses it, and after widening it would have described a Setup-only run while refusing on a missing Account build. It now reads “Setup and Account” (control: the phrase exists on main too, so this is a rewrite rather than an addition).

② The population is now decided by a written criterion, ⛔ not by a list

The order's acceptance item 1 demanded the criterion, not just the sites. Delivered at :121:135: an app is judged iff the ADR-0048 platform-app loop in packages/cli/src/commands/serve.ts registers its shell by default AND at least one package contributes navigation into it at runtime. That criterion is what excludes studio (fails both halves; already owned by the static parity test) and crm_app (fails the first, and has no locale source). ⇒ A future app is admitted or refused by reading the rule, ⛔ not by editing a list.

③ ⭐ The trap it guarded, which the order did not name

The per-contributor “landed at least one nav id” invariant had to stay per app. A flat union across apps reads as the obvious generalisation and is a softening of the existing setup judgement: @objectstack/mcp serves both apps, so a union keeps it passing on its account id alone after its setup contribution silently stops.

The delivery does not merely avoid it — it constructs the union as a negative control in --self-test (:215:222, :506:508). ⇒ The wrong generalisation is now pinned as wrong, so a later “tidy-up” cannot quietly make it.

④ Two directions, with the one moving number declared rather than smoothed

BEFORE (pristine gate, restored from HEAD into a gitignored copy so it resolves identically)
  OK (10 contributor(s), 54 merged `setup` nav id(s), 4 locale(s), every id labelled in every locale)   exit 0
AFTER
  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)                                                                exit 0

⭐ The setup count is 54 then 54 — unmoved, which is acceptance item 2's “must not over-fire”. The contributor count moves 10 → 11 because the Account shell is a new roster entry, and the dev names it as the one number the change necessarily moves rather than letting a reader discover it.

Must-catch (relayed): delete one of #17759's four account keys, rebuild, and the widened gate exits 1 with a verdict naming the id, the contributing package and the author's literal. ⭐ And the leg that makes it a measurement rather than a demo: under the same mutation and the same rebuilt dist, the pre-fix gate exits 0 with output byte-identical to its unmutated run (cmp exit 0, same sha256). ⇒ Structural blindness proven, ⛔ not inferred from a missed run — which is exactly what the card claimed and what triage could not verify.

⚠️ Reported as void rather than hidden: a first ablation attempt had no cd, so pnpm --filter ran in the shared checkout and failed with tsup: not found; the dist was never rebuilt, both gates stayed green, and ablation-dist-preflight refused that reading. ⇒ The false green the preflight exists to stop, caught by it, and declared. Nothing was written to the shared checkout (verified empty git status --porcelain).

⑤ Fences — measured

packages/platform-objects/   touched → False     (that half is #17930, domain:engine)
.claude/                     touched → False
content/docs/                touched → False     ⇒ content/docs/releases/** trivially untouched
CONTROL packages/cli/        touched → True
files: 2   — the gate script (+327/−47) and its changeset (+23)

⛔ No new translation key in any locale. ⛔ The existing setup judgement is not narrowed or softened anywhere.

⑥ Declared gaps, ⛔ not smoothed

⭐ The one worth repeating: packages/cli's typecheck does not reach the changed filetsconfig.json declares include: ['src'] with no allowJs, so tsc never compiles scripts/check-app-nav-i18n.mjs. Its green says nothing about this diff; what actually covers the file is node --check, the script's own --self-test, and the real run. ⇒ A green gate that structurally cannot see the diff is the same species as the defect this card fixes, and the dev caught it in its own verification.

Also declared: the integration tier (no spawn entry, no bin/, no kernel boot path — CI's), the 5 path-scheduled jobs, the 11 wide-population families, the 50 artifact-roster families, and that origin/main moved during the run without a rebase (the queue's to validate). Gate families: 50 derived, 50 run, 0 NOT-MEASURED, 0 UNRUN, all exit 0; check:dual-build-cjs-loads first answered exit 3 PREREQUISITE NOT MET and was re-run to a real verdict after a full build rather than recorded as a pass it never was.


Verdict and what remains

PASS. The card asked for one gate's population to be widened; what landed is a gate that states the rule deciding its own population, names the app it is actually failing on, and pins the wrong generalisation as wrong.

⚠️ CI: 29 success / 3 skipped / 2 failures, and ⛔ neither failure is this PR's. Both are Auto Label and Check PR Size — the two label-writing jobs — caught in a platform incident this seat has since bounded by measurement: first failure 08:50:49Z, last 09:04:27Z, first recovery 09:08:20Z (PR #17977), and every run since is green (#17982 · #17983 · #17986 · #17988). Filed as #17981. ⛔ No commit explains it; ⛔ it is not a size threshold (2 files / +350−47 is size/m against thresholds xs<10 · s<100 · m<500 · l<1000).

⇒ This seat cannot re-run themPOST /actions/runs/{id}/rerun-failed-jobs answers 403 Resource not accessible by integration, and branches/main/protection answers 403 too, so ⛔ this seat also cannot read which contexts are required. Per the standing rule for a failure judged not-this-PR's with no means to re-run: said once, here. The flip to ready re-fires the pull_request workflows, which is the legitimate re-run this seat does have — ⛔ not an empty commit and ⛔ not a close-and-reopen.

domain:cli execution PM seat · #6024 · session session_01TSf4DV7ziu4V5j73e46b7c · R73 · contract review of record · Independence: SELF-REVIEW


Generated by Claude Code

@claude
claude Bot marked this pull request as ready for review September 13, 2026 10:14
@claude
claude Bot added this pull request to the merge queue Sep 13, 2026
Merged via the queue into main with commit ca9d9d3 Sep 13, 2026
34 of 36 checks passed
@claude
claude Bot deleted the claude/issue-17891-app-nav-i18n-app-population branch September 13, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant