Repository navigation
Commit 68a57ad
fix(cli): os lint and os i18n check expect a contributed navigation item's label under its target app (#25171)
Fixes #24993
Clause-②: no
Metadata-protocol: none
## What changes
`collectExpectedEntries` (`packages/cli/src/utils/i18n-extract.ts`) is
the one walk behind `os lint`'s `i18n/missing-*` family, `os i18n check`
and `os i18n extract`. Its navigation arm read `apps[].navigation` only.
An item a package contributes into an app through
`manifest.navigationContributions` (ADR-0029 D7) never enters that
array: the runtime folds it in on every read
(`SchemaRegistry.applyNavContributions`), and `translateApp` resolves
its label from `apps.TARGET.navigation.ID.label`.
`translation-target-unknown` accepts that key since #18203. So the key
was accepted but never expected, and deleting its translation left both
doors green.
- **New arm.** After the apps' own trees, every
`navigationContributions[]` entry is walked by the same
`walkNavigation`, keyed under its target `app`. The seed is the item's
own `label`. A contributed group's children follow the rule an own
group's children follow. An entry with no string `app` is skipped.
- **Carriers.** `navigationContributionsOf` reads the two carriers the
target rule reads (`contributedNavItemsByApp` in
`packages/lint/src/validate-translation-references.ts`): the stack's own
`manifest.navigationContributions`, and
`packages[].manifest.navigationContributions` (the ADR-0130 D4 entry
body). Both are read unconditionally, as the rule reads them. On a
`preserve` composition whose last input is the contributor, both
carriers hold the same items, and the walker's existing `dedupeByPath`
keeps one entry per key (first emission wins, so an id an app also
declares keeps the app's own entry).
- **Unchanged.** An app's own items: same walk, same order, emitted
first. `i18n-coverage.ts` is not touched: its surface phrase already
reads "navigation items".
- Changeset `.changeset/24993-contributed-nav-i18n-coverage.md`:
`@objectstack/cli` `patch`.
## The mechanism hypotheses, measured (BASE `6b44efa7b`)
- **H1: confirmed in shape; reuse not taken.** The arm adds each
`navigationContributions[].items[]` id under its target app's
`navigation` keys. The target rule's resolution is
`contributedNavItemsByApp`, which is module-private to
`@objectstack/lint` (its index exports `validateTranslationReferences`,
not the reader). Importing it would need a new public export from
`@objectstack/lint`, outside this card's file surface. The other in-repo
readers each need a new import line in `i18n-extract.ts`'s import block,
which the dispatch reserved for PR #24947: `collectNavGroupInputs` (cli)
reads either/or carriers, not both; `stackPackageBodies`
(`@objectstack/core`) covers only the package carrier. So the carrier
read is written here, in about ten lines that mirror the rule's two
carriers. It is held to the rule by a parity pin, described below.
- **H2: confirmed.** The seed label is the contributed item's own
`label` (`sourceValue` / `inline` = `Cases`). The key sits under the app
it is contributed TO, read from every package body of the composed
artifact (the union stack both doors hand the walker since #22238).
- **H3: confirmed.** A contributed group (`grp_knowledge`) and its child
(`nav_article`) are both expected, by the same `walkNavigation` the own
tree uses. Deleting the child's key is reported.
- **H4: refuted as a coverage move.** `examples/app-multi-package`
contributes `nav_orders` into `multi_crm`, but it declares no `i18n`
block and ships no bundle. Only the default locale is active, and its
inline labels satisfy it. Its `check:i18n-coverage` count stays at its
baselined `0`. No example declares a translated locale and also
contributes navigation, so there was nothing to translate and no
baseline moved.
## Both doors, measured on the card's probe
The fixture has the card's shape: a service module contributes
`nav_case` into the app's `grp_service` group, and adds a group of its
own (`grp_knowledge` with child `nav_article`). It is composed
module-first with `manifest: 'preserve'`, so the contributions live only
in the module's package body. The fixture declares `en` and `zh-CN`, and
every other key is translated. Each row deletes one zh-CN navigation
key. Run with the source CLI (`bin/run-dev.js`) from a scratch
directory, at `d4e0e405a`. The ablated runs used the same tree with the
arm's loop emptied, as in the ablation below.
| zh-CN key deleted | `os lint --strict --json`, fixed | ablated (arm
removed) | `os i18n check --json --strict`, fixed | ablated |
|---|---|---|---|---|
| none | exit 0, `failing: 0` | exit 0 | exit 0, 15 keys expected | exit
0, 12 keys expected |
| `nav_case` (contributed) | **exit 1**, `i18n/missing-navigation`
`translations.zh-CN.apps.cnv_desk.navigation.nav_case.label` | exit 0,
no finding | **exit 1**, `error zh-CN
apps.cnv_desk.navigation.nav_case.label` | exit 0, no finding |
| `nav_article` (child of a contributed group) | **exit 1**, same rule,
`…nav_article.label` | exit 0, no finding | **exit 1**,
`…nav_article.label` | exit 0, no finding |
| `nav_accounts` (control: the app's own) | exit 1,
`…nav_accounts.label` | exit 1, `…nav_accounts.label` | exit 1 | exit 1
|
The ablated column reproduces the card's finding: the contributed
deletions exit 0, and the own-item control exits 1.
## Tests
New `packages/cli/test/i18n-contributed-nav-coverage.test.ts` (unit
tier: no spawn, no kernel). It has 13 cases over the real `defineStack`
+ `composeStacks(…, { manifest: 'preserve' })` +
`authoringRuleUnionStack` path that both doors take:
- **The walker.** It expects each contributed item under the target app,
after the app's own items, seeded with its own label. It reads a single
stack's own manifest, including for an app the stack only contributes
into. When the top-level manifest is the contributor's copy, each key is
expected once. An own id reused by a contribution keeps the app's entry.
- **The gate.** With every label translated, there is no navigation gap.
A deleted contributed item is reported, and so is a deleted child of a
contributed group. The own-item control is reported as before. When both
carriers hold the contribution, the deleted key is reported once.
- **Parity with `translation-target-unknown`.** A zh-CN bundle carrying
exactly the navigation keys the walker expects draws no
`translation-target-unknown` finding, on the composed stack and on a
contribute-only single stack. Each case first asserts that the bundle
really carries the contributed ids, so the pin cannot pass empty. The
control: an id nothing declares or contributes is still refused.
**Ablation.** The fix was committed first. `node
scripts/ablation-replace.mjs` replaced the arm's `for (const
contribution of navigationContributionsOf(config)) {` with a loop over
`[]`. The tool reported the anchor going 1 → 0 and the blob
`c23f3ab341d6` → `055daea8a714`. The test file and the subject resolve
from `src/` (relative import; `run-dev.js` runs `src/` through tsx), so
no `dist/` leg applies.
- Ablated: **8 failed | 5 passed (13)**. All 8 contributed-specific
cases went red. The 5 that stayed green are the controls: the fixture
shape, the own-id dedupe, no gap when every label is translated, the
own-item report, and the ghost-id refusal.
- Restored: blob == HEAD (`c23f3ab341d6`), and `git diff HEAD` is empty
(the tool's own verdict).
- Fixed: **13 passed (13)**.
## Local verification
Final head `4f263166d`: this branch plus a merge of `main` at
`06900c9c2`, which brought one objectui pin bump and touches no file of
this diff. Every lock run below printed its own VERDICT line, which is
quoted.
- **Build.** `pnpm --workspace-concurrency=2 --filter
'@objectstack/cli^...' build` → `VERDICT command-exit 0`.
- **Typecheck.** `pnpm --filter @objectstack/cli typecheck` (`tsc
--noEmit` plus `check:test-typecheck`) → `VERDICT command-exit 0`,
test-layer debt held at 3 files / 28 errors / 6 pinned signatures. `tsc
-p tsconfig.test.json --listFiles` lists the new test file once, so it
is type-checked.
- **Unit tier.** `pnpm --filter @objectstack/cli exec vitest run
--project unit --maxWorkers=2` at `0c16d8246` (before the merge) →
**Test Files 323 passed (323) · Tests 4769 passed (4769)**, `VERDICT
command-exit 0`. The integration tier is left to CI: this diff touches
no spawn entry and no integration-tier file.
- **Re-check after the merge, at `4f263166d`.** The new file, plus the
three cli tests that read `.objectui-sha`: **4 files, 44 tests passed**.
- **Gates.** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 68 commands. All 68 were run at
`4f263166d`, with each exit code captured before any pipe. `--ran`: `68
derived famil(ies) accounted for — 66 run, 2 NOT-MEASURED`, 0 unrun.
Readings that bear on this change:
- `check:i18n-coverage`: `OK (14 config(s), 621 baselined untranslated
string(s), none new)`. The first attempt refused with exit 3,
`PREREQUISITE NOT MET` (the showcase example's `connector-slack` was
unbuilt). The re-run came after the gate's own printed turbo build.
- `check:i18n`: `OK (10 package(s) — all bundles in sync`.
- `check:i18n-walk-parity`: `11 declared group(s), 9 walked, 2
exempted`.
- `check:test-source-alias`, `check:cross-package-test-inputs`,
`check:nul-bytes`, `check:issue-citations` and `check:doc-authoring`:
green.
- **NOT MEASURED: `check:type-check-debt`.** Reason: twice `VERDICT
queue-timeout (exit 99)`. The shared verify lock was held more than 19
minutes by another session's `pnpm --filter @objectstack/dogfood test`
(pid 28810, alive). CI's required jobs run it.
- **NOT MEASURED: `check:dual-build-cjs-loads`.** Reason: `PREREQUISITE
NOT MET`, because six packages outside this diff's closure have no
`dist/` (`studio`, `client-react`, `embedder-openai`,
`knowledge-ragflow`, `organizations`, `service-cluster-redis`). The
build that would supply them got the same queue-timeout. CI's required
jobs run it.
- **Dogfood.** Not run: the title carries no `!` and `Clause-②` has no
`(narrowing)` arm.
## Acceptance notes
- **A module cannot ship a contributed item's translation without
restating the target app's label.** In `TranslationDataSchema`, the
`apps.APP` entry requires `label`
(`packages/spec/src/system/translation.zod.ts`). Measured: a module pack
carrying only `apps.cnv_desk.navigation.nav_case.label` is refused by
`defineStack` with `translations.0.zh-CN.apps.cnv_desk.label: Required
property 'label' is missing.` That refusal is loud, not silent. The way
around it is to restate the label of an app the module does not own, and
that restated label merges with the owner's translation. Whether the
module's copy then wins at runtime is not measured. This is a boundary
note, not filed. The fixture keeps every navigation label in the app
package's pack, the layout HotCRM's packs were measured in (#18203).
Carrier: none.
- **The walker and the target rule still walk an item differently.** The
walker reads `id ?? name` and descends `items` as well as `children`.
The rule reads `id` and descends `children` only, plus
`areas[].navigation`, which `translateApp` does not translate. For own
items this predates this PR and is unchanged. On spec-valid input (an
`id` and `children`) the two agree, and the parity pin holds them there.
Noted, not filed. Carrier: none.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01AFrKwndmrCLtF5V9hNtrw3)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent ab488d9 commit 68a57ad
3 files changed
Lines changed: 324 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
93 | 93 | | |
94 | 94 | | |
95 | 95 | | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
96 | 99 | | |
97 | 100 | | |
98 | 101 | | |
| |||
1451 | 1454 | | |
1452 | 1455 | | |
1453 | 1456 | | |
| 1457 | + | |
| 1458 | + | |
| 1459 | + | |
| 1460 | + | |
| 1461 | + | |
| 1462 | + | |
| 1463 | + | |
| 1464 | + | |
| 1465 | + | |
| 1466 | + | |
| 1467 | + | |
| 1468 | + | |
| 1469 | + | |
| 1470 | + | |
| 1471 | + | |
| 1472 | + | |
1454 | 1473 | | |
1455 | 1474 | | |
1456 | 1475 | | |
| |||
2146 | 2165 | | |
2147 | 2166 | | |
2148 | 2167 | | |
| 2168 | + | |
| 2169 | + | |
| 2170 | + | |
| 2171 | + | |
| 2172 | + | |
| 2173 | + | |
| 2174 | + | |
| 2175 | + | |
| 2176 | + | |
| 2177 | + | |
| 2178 | + | |
| 2179 | + | |
| 2180 | + | |
| 2181 | + | |
| 2182 | + | |
| 2183 | + | |
| 2184 | + | |
| 2185 | + | |
| 2186 | + | |
| 2187 | + | |
| 2188 | + | |
| 2189 | + | |
| 2190 | + | |
| 2191 | + | |
| 2192 | + | |
| 2193 | + | |
| 2194 | + | |
| 2195 | + | |
| 2196 | + | |
| 2197 | + | |
| 2198 | + | |
| 2199 | + | |
| 2200 | + | |
| 2201 | + | |
| 2202 | + | |
| 2203 | + | |
| 2204 | + | |
| 2205 | + | |
| 2206 | + | |
2149 | 2207 | | |
2150 | 2208 | | |
2151 | 2209 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
0 commit comments