From 04216260f065bca95d6b1dfc21c1b9b4577d7cfa Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 13 Sep 2026 08:03:13 +0000 Subject: [PATCH] fix(cli): widen check:app-nav-i18n from one app to the declared platform-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..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 Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c --- .../17891-app-nav-i18n-app-population.md | 23 ++ packages/cli/scripts/check-app-nav-i18n.mjs | 374 +++++++++++++++--- 2 files changed, 350 insertions(+), 47 deletions(-) create mode 100644 .changeset/17891-app-nav-i18n-app-population.md diff --git a/.changeset/17891-app-nav-i18n-app-population.md b/.changeset/17891-app-nav-i18n-app-population.md new file mode 100644 index 00000000000..09db136ae47 --- /dev/null +++ b/.changeset/17891-app-nav-i18n-app-population.md @@ -0,0 +1,23 @@ +--- +'@objectstack/cli': patch +--- + +`check:app-nav-i18n` now judges the PLATFORM APPS' navigation — Setup **and Account** — instead of narrowing to `setup` at every site. + +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. + +Six sites narrowed it, only three of which were the obvious filters: + +- the contribution filter, the app-shell filter and the merged-app lookup; +- the **locale-file lookup** (`data.apps..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`; +- the **build prerequisite**, a package path hard-coded to `@objectstack/setup`; +- the **contributor roster**, which booted no package that registers the Account shell — so `account` had no merged app to judge at all. + +Behaviour now: + +- 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; +- 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; +- 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; +- `--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. + +The `setup` judgement is unchanged: the same 54 merged ids, the same verdict, and the same count in the pass line. diff --git a/packages/cli/scripts/check-app-nav-i18n.mjs b/packages/cli/scripts/check-app-nav-i18n.mjs index 4fae0f9c39a..37f6dbf2da5 100644 --- a/packages/cli/scripts/check-app-nav-i18n.mjs +++ b/packages/cli/scripts/check-app-nav-i18n.mjs @@ -1,6 +1,7 @@ #!/usr/bin/env node -// check-app-nav-i18n — translation coverage for the Setup app's navigation, -// judged on the RUNTIME-MERGED app metadata rather than on a static walk. +// check-app-nav-i18n — translation coverage for the PLATFORM APPS' navigation +// (Setup and Account), judged on the RUNTIME-MERGED app metadata rather than on +// a static walk. // // --------------------------------------------------------------------------- // Why this gate exists (#5750): a handoff nobody stood on @@ -71,9 +72,10 @@ // "Who serves this path" is a question about the composed runtime, not about // which plugin declares what (AGENTS.md, Route & surface ownership). `cli` is // the composition root — the package `os dev` / `os serve` assemble from — and -// the ONLY workspace package that depends on all eleven Setup nav contributors -// at once. A gate in `platform-objects` could not import the plugins (they -// depend on it, not the other way round) and would be measuring the shell. +// the ONLY workspace package that depends on every entry in the roster below — +// both platform-app SHELLS and every nav contributor — at once. A gate in +// `platform-objects` could not import the plugins (they depend on it, not the +// other way round) and would be measuring the shell. // // --------------------------------------------------------------------------- // What this gate deliberately does NOT claim @@ -105,8 +107,63 @@ import { EXIT_FINDINGS, EXIT_PREREQUISITE_NOT_MET } from '../../../scripts/impor const HERE = dirname(fileURLToPath(import.meta.url)); const CLI_ROOT = join(HERE, '..'); -/** The app whose navigation is assembled at runtime. Setup is the only one. */ -const APP_NAME = 'setup'; +// --------------------------------------------------------------------------- +// THE POPULATION — which apps this gate judges, and the criterion that decides +// it (#17891) +// --------------------------------------------------------------------------- +// +// An app belongs here iff BOTH halves hold. The criterion is what a future +// reader needs; the list is only its output. ⛔ Neither half is optional — +// each one excludes something real, and dropping either turns this list into a +// pair someone will pattern-match instead of decide. +// +// (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`. That loop is the authority on +// what `os dev` / `os serve` actually boot, and a verdict here is only +// ever about a composition that really exists ("who serves this path" is +// a question about the provisioned runtime — Route & surface ownership). +// (b) AT LEAST ONE PACKAGE CONTRIBUTES NAVIGATION INTO IT AT RUNTIME +// (`manifest.navigationContributions[].app`), so a static walk cannot see +// every label it renders. This half is what makes THIS gate the right +// owner rather than `app-nav-translation-parity.test.ts`, which walks the +// declared trees and needs no boot. +// +// Today that resolves to exactly `setup` and `account`. ⛔ "Every app" is NOT +// the criterion, and the two apps it would have swept in are why: +// +// • `studio` fails BOTH halves. The ADR-0048 loop deliberately does not load +// it ("@objectstack/studio is intentionally NOT default-loaded"), and +// `STUDIO_APP` declares its whole navigation statically with no package +// 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 (§1) and would drag in a package the composition root does not +// boot. +// • `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 for this gate to compare against. +// +// `account` satisfied both halves all along and was invisible anyway: +// `@objectstack/mcp` contributes `nav_connect_agent` into +// `grp_account_developer` (`connect-ui.ts`, #16746) and #17759 supplied its +// label in all four locales — while this gate printed a BYTE-IDENTICAL +// `OK (…)` line before and after that landing, because every site below +// narrowed to one app name. Widening the three obvious filters is NOT the fix: +// the locale-file lookup and the build prerequisite are keyed by the app too, +// so a half-widened gate collects `account` ids and then looks for their labels +// under `apps.setup.navigation`. +// +// Each entry names the package that registers the app SHELL, because two +// mechanisms below are keyed by it: the build prerequisite probes that +// package's `dist/`, and the roster must BOOT it or there is no merged app to +// judge at all. +const APPS = [ + { name: 'setup', shellPackage: '@objectstack/setup' }, + { name: 'account', shellPackage: '@objectstack/account' }, +]; + +/** Just the names, in declaration order — the order every verdict renders in. */ +const APP_NAMES = APPS.map((app) => app.name); // --------------------------------------------------------------------------- // Pure verdict helpers — driven by `--self-test` with recorded samples, so each @@ -150,8 +207,60 @@ export function contributorsWithNoNavIds(contributions) { return contributions.filter((c) => c.ids.length === 0).map((c) => c.source); } -/** Render one locale's shortfall with the source that declared each id. */ -function renderMissing(locale, missing, declaredBy) { +/** + * One app's slice of the per-contributor ledger: the contributors that DECLARE + * that app, each carrying only the ids it landed THERE. + * + * This exists so {@link contributorsWithNoNavIds} above stays exactly what it + * was and simply runs once per app. ⛔ The alternative — one flat union of each + * contributor's ids across every app — reads as the natural generalisation and + * is a SOFTENING of the invariant that was already here: `@objectstack/mcp` + * contributes into both `setup` and `account`, so a union keeps it passing on + * its `account` id alone after its `setup` contribution has silently stopped, + * which is the exact "fewer ids means fewer checks" false green this invariant + * exists to catch, restated one app wider (#17891). `--self-test` carries that + * union as a negative control. + * + * A declared app with no bucket at all reads as ZERO ids, never as "not + * applicable": absence must be loud (§3), and a contributor that landed nothing + * anywhere is the case this whole invariant is about. + */ +export function contributionsForApp(contributions, appName) { + return contributions + .filter((c) => (c.apps ?? []).includes(appName)) + .map((c) => ({ source: c.source, ids: c.idsByApp?.[appName] ?? [] })); +} + +/** + * The pass line, as a VALUE so `--self-test` can pin it. + * + * Per-app counts are not cosmetics. #17891's whole measurement was a DIFF of + * THIS LINE across #17759's landing, and the finding was that it came back + * byte-identical because one app's counts were all it carried. A summary that + * collapses the apps back into one total rebuilds precisely that: a number that + * moves for reasons the reader cannot attribute, and — worse — one that fails + * to move when a whole app drops out of the population. + */ +export function summaryText(contributorCount, localeCount, perApp) { + const apps = perApp.map(({ app, ids }) => `${app}: ${ids} merged nav id(s)`).join(', '); + return ( + `check-app-nav-i18n: OK (${contributorCount} contributor(s), ${localeCount} locale(s), ` + + `${perApp.length} app(s) — ${apps} — every id labelled in every locale).` + ); +} + +/** + * Render ONE app's shortfall in ONE locale, with the source that declared each + * id. + * + * ⛔ The app is a parameter, never this file's former single constant: the + * bundles are keyed `apps..navigation.`, one namespace per app, so + * `apps.setup.navigation.nav_connect_agent` never answers for the same id under + * `apps.account.navigation` (#17759). A widened gate whose verdict still said + * `apps.setup.navigation` would send whoever it fails on to the wrong subtree + * — a confident diagnosis pointing somewhere innocent (#17891). + */ +function renderMissing(appName, locale, missing, declaredBy) { const lines = missing.map((id) => { const d = declaredBy.get(id); const literal = d?.label ? ` — author's literal ${JSON.stringify(d.label)}` : ''; @@ -159,7 +268,7 @@ function renderMissing(locale, missing, declaredBy) { return ` ${id}${from}${literal}`; }); return ( - `apps.${APP_NAME}.navigation — locale \`${locale}\` has no label for ` + + `apps.${appName}.navigation — locale \`${locale}\` has no label for ` + `${missing.length} runtime-merged nav id(s):\n${lines.join('\n')}` ); } @@ -167,19 +276,45 @@ function renderMissing(locale, missing, declaredBy) { // --------------------------------------------------------------------------- // The composition. EXPLICIT, never discovered: a contributor that drops out of // this list must do so in a diff someone reads (Route & surface ownership §2). -// Every entry must land at least one `setup` nav id. +// +// `apps` is each entry's DECLARED coverage, and the invariant is PER APP: every +// entry must land at least one nav id in EVERY app it declares. ⛔ Not one id +// across the union — see {@link contributionsForApp} for why that spelling is a +// softening rather than a generalisation. +// +// The two SHELL entries are what make the population reachable at all: an app +// whose shell nobody registers has no merged navigation for the contributions +// to merge into, so widening the filters without booting `@objectstack/account` +// would have reported "the `account` app is not registered at all" instead of +// judging a single label (#17891). // --------------------------------------------------------------------------- const CONTRIBUTORS = [ { source: '@objectstack/setup (SETUP_APP + SETUP_NAV_CONTRIBUTIONS)', + apps: ['setup'], async load() { const { createSetupAppPlugin } = await import('@objectstack/setup'); return { plugin: createSetupAppPlugin() }; }, }, + { + // The Account SHELL (#17891). `AccountAppPlugin.start` registers + // `ACCOUNT_APP` through the `manifest` service — the same seam every other + // entry here uses — so the fake `ctx` below already serves it and no + // credential fixture is needed. Its kernel `dependencies` name + // `com.objectstack.auth`; this gate calls `init`/`start` directly rather + // than running a kernel, so nothing resolves them and nothing needs to. + source: '@objectstack/account (ACCOUNT_APP)', + apps: ['account'], + async load() { + const { createAccountAppPlugin } = await import('@objectstack/account'); + return { plugin: createAccountAppPlugin() }; + }, + }, { source: '@objectstack/plugin-security', + apps: ['setup'], async load() { const { SecurityPlugin } = await import('@objectstack/plugin-security'); return { plugin: new SecurityPlugin({}) }; @@ -187,6 +322,7 @@ const CONTRIBUTORS = [ }, { source: '@objectstack/plugin-sharing', + apps: ['setup'], async load() { const { SharingServicePlugin } = await import('@objectstack/plugin-sharing'); return { plugin: new SharingServicePlugin({}) }; @@ -194,6 +330,7 @@ const CONTRIBUTORS = [ }, { source: '@objectstack/plugin-approvals', + apps: ['setup'], async load() { const { ApprovalsServicePlugin } = await import('@objectstack/plugin-approvals'); return { plugin: new ApprovalsServicePlugin({ disableService: true }) }; @@ -201,6 +338,7 @@ const CONTRIBUTORS = [ }, { source: '@objectstack/plugin-audit', + apps: ['setup'], async load() { const { AuditPlugin } = await import('@objectstack/plugin-audit'); return { plugin: new AuditPlugin() }; @@ -208,6 +346,7 @@ const CONTRIBUTORS = [ }, { source: '@objectstack/plugin-webhooks', + apps: ['setup'], async load() { const { WebhookOutboxPlugin } = await import('@objectstack/plugin-webhooks'); return { plugin: new WebhookOutboxPlugin({ autoEnqueue: false }) }; @@ -215,6 +354,7 @@ const CONTRIBUTORS = [ }, { source: '@objectstack/service-messaging', + apps: ['setup'], async load() { const { MessagingServicePlugin } = await import('@objectstack/service-messaging'); return { plugin: new MessagingServicePlugin({}) }; @@ -222,6 +362,7 @@ const CONTRIBUTORS = [ }, { source: '@objectstack/service-datasource', + apps: ['setup'], async load() { const { DatasourceAdminServicePlugin } = await import('@objectstack/service-datasource'); return { plugin: new DatasourceAdminServicePlugin({}) }; @@ -229,6 +370,12 @@ const CONTRIBUTORS = [ }, { source: '@objectstack/mcp (CONNECT_AGENT_UI_BUNDLE)', + // The one contributor that lands in BOTH apps: `CONNECT_AGENT_UI_BUNDLE` + // carries two `navigationContributions` entries pointing at one page — + // `setup`/`group_integrations` for admins and `account`/ + // `grp_account_developer` for the per-user half (#16746). The Account half + // is the contribution this gate was blind to. + apps: ['setup', 'account'], async load() { const { CONNECT_AGENT_UI_BUNDLE } = await import('@objectstack/mcp'); return { manifests: [CONNECT_AGENT_UI_BUNDLE] }; @@ -236,6 +383,7 @@ const CONTRIBUTORS = [ }, { source: '@objectstack/cloud-connection (cloud + marketplace UI bundles)', + apps: ['setup'], async load() { const mod = await import('@objectstack/cloud-connection'); return { @@ -341,6 +489,7 @@ function selfTest() { // literal the console falls back to — a verdict a reader cannot act on is the // same cost as no verdict. const rendered = renderMissing( + 'setup', 'zh-CN', ['nav_http_deliveries'], new Map([['nav_http_deliveries', { source: '@objectstack/plugin-webhooks', label: 'HTTP Deliveries' }]]), @@ -350,6 +499,88 @@ function selfTest() { expect('#5750 verdict names the fallback literal', rendered.includes('HTTP Deliveries'), rendered); expect('#5750 verdict names the locale', rendered.includes('zh-CN'), rendered); + // ── The POPULATION, and the one way widening it goes wrong (#17891) ── + // + // The gate judges more than one app now. The half that can be widened WRONG + // is the per-contributor invariant: flattening each contributor's ids into a + // union across apps reads as the obvious generalisation and quietly deletes + // the `setup` judgement that was already here. These cases are that + // spelling's negative control — the union is CONSTRUCTED below and required + // to be the thing that goes green, so the pin cannot pass by tautology. + const MIXED_LEDGER = [ + { source: '@objectstack/mcp', apps: ['setup', 'account'], idsByApp: { setup: [], account: ['nav_connect_agent'] } }, + { source: '@objectstack/setup (SETUP_APP + SETUP_NAV_CONTRIBUTIONS)', apps: ['setup'], idsByApp: { setup: ['group_overview'], account: [] } }, + ]; + expect( + '#17891 a contributor that stopped landing `setup` ids is named for setup, whatever it landed elsewhere', + contributorsWithNoNavIds(contributionsForApp(MIXED_LEDGER, 'setup')).join(',') === '@objectstack/mcp', + JSON.stringify(contributionsForApp(MIXED_LEDGER, 'setup')), + ); + expect( + '#17891 NEGATIVE CONTROL: the flat-union spelling this replaces goes GREEN on that same ledger', + contributorsWithNoNavIds(MIXED_LEDGER.map((c) => ({ source: c.source, ids: Object.values(c.idsByApp).flat() }))).length === 0, + 'if the union is not the green one, the case above proves nothing about the softening', + ); + expect( + '#17891 an app is judged only over the contributors that DECLARE it', + contributionsForApp(MIXED_LEDGER, 'account').map((c) => c.source).join(',') === '@objectstack/mcp', + JSON.stringify(contributionsForApp(MIXED_LEDGER, 'account')), + ); + expect( + '#17891 a declared app with no bucket at all reads as ZERO ids, never as not-applicable', + contributorsWithNoNavIds(contributionsForApp([{ source: 'x', apps: ['account'], idsByApp: {} }], 'account')).join(',') === 'x', + 'a missing per-app bucket must be a finding, not a skip', + ); + + // The verdict must name the APP as well as the id: one namespace per app, so + // `apps.setup.navigation.nav_connect_agent` does not answer for the same id + // under `apps.account.navigation` (#17759). A verdict naming only the id + // sends the reader to whichever subtree they guessed. + const renderedAccount = renderMissing( + 'account', + 'zh-CN', + ['nav_connect_agent'], + new Map([['nav_connect_agent', { source: '@objectstack/mcp', label: 'Connect an Agent' }]]), + ); + expect('#17891 the verdict names the app subtree it actually read', renderedAccount.includes('apps.account.navigation'), renderedAccount); + expect( + '#17891 NEGATIVE CONTROL: it does NOT name the other app — a widened gate still saying `setup` misreports', + !renderedAccount.includes('apps.setup.navigation'), + renderedAccount, + ); + + // The pass line. #17891's whole measurement was a DIFF of this line across a + // landing, and it came back byte-identical because one app's counts were all + // it carried. + const summary = summaryText(11, 4, [{ app: 'setup', ids: 54 }, { app: 'account', ids: 12 }]); + expect('#17891 the pass line names every app it judged, with that app\'s own count', summary.includes('setup: 54') && summary.includes('account: 12'), summary); + expect('#17891 the pass line still states the contributor and locale counts', summary.includes('11 contributor(s)') && summary.includes('4 locale(s)'), summary); + expect( + '#17891 NEGATIVE CONTROL: the pin can see an app LEAVING the line', + !summaryText(10, 4, [{ app: 'setup', ids: 54 }]).includes('account'), + 'a summary pin that cannot notice a vanished app is the blindness this card fixed, rebuilt', + ); + + // The population itself must stay reachable: every declared app needs a shell + // package, because the prerequisite probe and the CONTRIBUTORS roster are + // both keyed by it. An entry missing one refuses at a `join(undefined)` deep + // inside a probe rather than here. + expect( + '#17891 every app in the population declares the package its shell ships in', + APPS.every((app) => typeof app.name === 'string' && app.name && typeof app.shellPackage === 'string' && app.shellPackage.startsWith('@objectstack/')), + JSON.stringify(APPS), + ); + expect( + '#17891 every declared app is served by at least one CONTRIBUTORS entry', + APP_NAMES.every((name) => CONTRIBUTORS.some((c) => (c.apps ?? []).includes(name))), + JSON.stringify(CONTRIBUTORS.map((c) => ({ source: c.source, apps: c.apps }))), + ); + expect( + '#17891 every CONTRIBUTORS entry declares at least one app IN the population', + CONTRIBUTORS.every((c) => (c.apps ?? []).length > 0 && (c.apps ?? []).every((a) => APP_NAMES.includes(a))), + JSON.stringify(CONTRIBUTORS.map((c) => ({ source: c.source, apps: c.apps }))), + ); + // ── The refusal CLASS, and the advisory that must move with it (#14857) ── // // `checkBuildPrerequisite` is this gate's only refusal, and until its printer @@ -431,7 +662,8 @@ function selfTest() { } console.log( '✓ check:app-nav-i18n --self-test — the nav walk, the per-locale label verdict and the silent-contributor guard ' + - 'all go red on the shapes they exist to catch; ' + + `all go red on the shapes they exist to catch, over all ${APPS.length} app(s) in the population ` + + `(${APP_NAMES.join(', ')}) with the per-app invariant proven not to be a union (#17891); ` + `and the build-prerequisite refusal exits ${EXIT_PREREQUISITE_NOT_MET} — distinct from a finding's ${EXIT_FINDINGS} — ` + 'with an advisory that names the number it claims (#14857).', ); @@ -471,10 +703,18 @@ if (process.argv.includes('--self-test')) { * into the red while the code beside it was already correct.) */ function checkBuildPrerequisite() { - const probe = join(CLI_ROOT, 'node_modules', '@objectstack', 'setup', 'dist', 'index.mjs'); - if (existsSync(probe)) return; - console.error(buildPrerequisiteText(probe)); - process.exit(EXIT_PREREQUISITE_NOT_MET); + // ONE PROBE PER APP SHELL (#17891). A probe hard-coded to `@objectstack/setup` + // answers for one app in a population of two: with Account unbuilt it returns + // happily, and the missing build arrives eleven lines further down as + // `COULD NOT BOOT — @objectstack/account`, which is a finding about a plugin + // worn over a finding about the tree. It refuses on the FIRST missing one and + // names it, so the advisory keeps naming a file that is really absent. + for (const app of APPS) { + const probe = join(CLI_ROOT, 'node_modules', ...app.shellPackage.split('/'), 'dist', 'index.mjs'); + if (existsSync(probe)) continue; + console.error(buildPrerequisiteText(probe)); + process.exit(EXIT_PREREQUISITE_NOT_MET); + } } /** @@ -493,7 +733,8 @@ function checkBuildPrerequisite() { function buildPrerequisiteText(probe) { return ( `\ncheck-app-nav-i18n: PREREQUISITE NOT MET — the workspace packages are not built\n\n` + - ` This gate boots the real Setup composition, so it imports the BUILT output of\n` + + ` This gate boots the real platform-app composition (Setup and Account), so it\n` + + ` imports the BUILT output of\n` + ` every contributing package. This one is not there:\n\n` + ` ${probe}\n\n` + ` Fix: pnpm build (or: pnpm --filter '@objectstack/cli^...' build)\n\n` + @@ -548,9 +789,17 @@ const ctx = { getKernel: () => undefined, }; -/** `{ source, ids }` per contributor, plus id → { source, label } for the verdict. */ +/** + * `{ source, apps, idsByApp }` per contributor, plus app → id → { source, label } + * for the verdict. + * + * `declaredBy` is keyed by APP first because a nav id is unique within one app's + * tree and nothing indexes it across apps: `nav_connect_agent` is a real id in + * BOTH `setup` and `account` (one page, two doors — #16746), so a flat id-keyed + * map would have one entry silently overwrite the other. + */ const contributions = []; -const declaredBy = new Map(); +const declaredBy = new Map(APP_NAMES.map((name) => [name, new Map()])); for (const contributor of CONTRIBUTORS) { currentSink = []; @@ -575,31 +824,45 @@ for (const contributor of CONTRIBUTORS) { process.exit(1); } - const ids = []; + const idsByApp = Object.fromEntries(APP_NAMES.map((name) => [name, []])); for (const manifest of currentSink) { for (const contribution of manifest?.navigationContributions ?? []) { - if (contribution?.app !== APP_NAME) continue; + const target = contribution?.app; + if (!APP_NAMES.includes(target)) continue; for (const item of contribution.items ?? []) { if (!item?.id) continue; - ids.push(item.id); - declaredBy.set(item.id, { source: contributor.source, label: item.label }); + idsByApp[target].push(item.id); + declaredBy.get(target).set(item.id, { source: contributor.source, label: item.label }); } } // The app shell itself (group anchors) counts as this contributor's ids too. for (const app of manifest?.apps ?? []) { - if (app?.name !== APP_NAME) continue; + const name = app?.name; + if (!APP_NAMES.includes(name)) continue; for (const id of collectNavIds(app.navigation)) { - ids.push(id); - if (!declaredBy.has(id)) declaredBy.set(id, { source: contributor.source, label: undefined }); + idsByApp[name].push(id); + if (!declaredBy.get(name).has(id)) { + declaredBy.get(name).set(id, { source: contributor.source, label: undefined }); + } } } engine.registerApp(manifest); } - contributions.push({ source: contributor.source, ids }); + contributions.push({ source: contributor.source, apps: contributor.apps, idsByApp }); } -const mergedApp = engine.registry.getApp(APP_NAME); -const mergedIds = collectNavIds(mergedApp?.navigation); +/** + * app → merged nav ids, or `undefined` when the app was never registered. The + * two states must stay distinguishable: "registered, empty" and "not registered + * at all" have different remedies, and `collectNavIds(undefined)` answers `[]` + * for both. + */ +const mergedIdsByApp = new Map( + APPS.map((app) => { + const mergedApp = engine.registry.getApp(app.name); + return [app.name, mergedApp ? collectNavIds(mergedApp.navigation) : undefined]; + }), +); // The two verdicts are kept apart because their REMEDIES are opposites, and a // footer that prescribes one for the other is the #5862 defect (a confident @@ -609,24 +872,35 @@ const coverageErrors = []; // 1. The composition is complete — checked BEFORE the coverage verdict, because // an incomplete composition cannot give one. -for (const source of contributorsWithNoNavIds(contributions)) { - compositionErrors.push( - `${source} landed NO \`${APP_NAME}\` navigation id. Either it stopped contributing (then remove it ` + - `from CONTRIBUTORS in this script, in the same PR) or its registration silently no-ops — which would ` + - `make this gate greener, not redder, by giving it fewer ids to check.`, - ); -} -if (!mergedApp) { - compositionErrors.push( - `the \`${APP_NAME}\` app is not registered at all — the composition produced no app to judge.`, - ); +for (const app of APPS) { + for (const source of contributorsWithNoNavIds(contributionsForApp(contributions, app.name))) { + compositionErrors.push( + `${source} landed NO \`${app.name}\` navigation id. Either it stopped contributing to that app ` + + `(then drop \`${app.name}\` from its \`apps\` in CONTRIBUTORS in this script, in the same PR) or its ` + + `registration silently no-ops — which would make this gate greener, not redder, by giving it fewer ` + + `ids to check.`, + ); + } + if (mergedIdsByApp.get(app.name) === undefined) { + compositionErrors.push( + `the \`${app.name}\` app is not registered at all — the composition produced no app to judge. ` + + `Its shell ships in \`${app.shellPackage}\`, which must be a CONTRIBUTORS entry above.`, + ); + } } // 2. Every merged id carries a label in every locale the bundle declares. if (compositionErrors.length === 0) { - for (const [locale, data] of Object.entries(SetupAppTranslations)) { - const missing = missingLabels(mergedIds, data?.apps?.[APP_NAME]?.navigation); - if (missing.length) coverageErrors.push(renderMissing(locale, missing, declaredBy)); + // App-outer, locale-inner: one app's four verdicts stay contiguous, and a + // regression confined to one app renders exactly the lines it rendered before + // the population widened, in the same order. + for (const app of APPS) { + for (const [locale, data] of Object.entries(SetupAppTranslations)) { + const missing = missingLabels(mergedIdsByApp.get(app.name), data?.apps?.[app.name]?.navigation); + if (missing.length) { + coverageErrors.push(renderMissing(app.name, locale, missing, declaredBy.get(app.name))); + } + } } } @@ -649,14 +923,20 @@ if (errors.length) { console.error( ` These ids exist only AFTER the runtime merge, so neither \`pnpm check:i18n\` nor\n` + ` \`pnpm check:i18n-coverage\` can see them — that gap is what this gate closes (#5750).\n` + - ` Fix by adding the label to \`apps.${APP_NAME}.navigation\` in EVERY locale file under\n` + - ` packages/platform-objects/src/apps/translations/ (en, zh-CN, ja-JP, es-ES).`, + ` Fix by adding the label to the \`apps..navigation\` subtree NAMED IN EACH VERDICT\n` + + ` above, in EVERY locale file under\n` + + ` packages/platform-objects/src/apps/translations/ (en, zh-CN, ja-JP, es-ES).\n` + + ` ⛔ One namespace PER APP: \`apps.setup.navigation.\` never answers for the same id\n` + + ` under \`apps.account.navigation\`, so a twin entry needs its own key (#17759).`, ); } process.exit(1); } console.log( - `check-app-nav-i18n: OK (${CONTRIBUTORS.length} contributor(s), ${mergedIds.length} merged \`${APP_NAME}\` nav id(s), ` + - `${Object.keys(SetupAppTranslations).length} locale(s), every id labelled in every locale).`, + summaryText( + CONTRIBUTORS.length, + Object.keys(SetupAppTranslations).length, + APPS.map((app) => ({ app: app.name, ids: mergedIdsByApp.get(app.name).length })), + ), );