Skip to content

fix(cli): os lint and os i18n check expect a contributed navigation item's label under its target app - #25171

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-24993-contributed-nav-i18n-coverage
Oct 22, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-24993-contributed-nav-i18n-coverage

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

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 feat(spec,lint): a bulk action's confirmText is a template slot over count and its params, judged at validate, compile and the publish gate #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 cli: os lint translation coverage reads only a multi-package artifacts top level — every i18n/missing-*` goes silent once an app ships as packages (missed reader of the ADR-0130 2026-09-22 addendum; family of #22189) #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 (lint: translation-target-unknown reads apps[].navigation only, so every locale key for a CONTRIBUTED navigation item (navigationContributions) is a false positive whose advice deletes a translation the runtime honours #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

…et app

collectExpectedEntries walked apps[].navigation only, so a key for an item
another package contributes through manifest.navigationContributions was
never expected and deleting its translation left os lint and os i18n check
green, though translation-target-unknown accepts that key.

Claude-Session: https://claude.ai/code/session_01AFrKwndmrCLtF5V9hNtrw3
Co-authored-by: Claude <noreply@anthropic.com>
…es the required app label

Claude-Session: https://claude.ai/code/session_01AFrKwndmrCLtF5V9hNtrw3
Co-authored-by: Claude <noreply@anthropic.com>
…ntributed-nav-i18n-coverage

Claude-Session: https://claude.ai/code/session_01AFrKwndmrCLtF5V9hNtrw3
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 30 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 e3aa111ad979cf019dda166ca0c6cf5a06946631 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 527b17633daa5ef83479082b8618923ff8b25a3b — the merge of head 4f263166d7b72eef7b3c753593542f1483b94064 into base e3aa111ad979cf019dda166ca0c6cf5a06946631, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 527b17633daa5ef83479082b8618923ff8b25a3b && git checkout 527b17633daa5ef83479082b8618923ff8b25a3b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e3aa111ad979cf019dda166ca0c6cf5a06946631 4f263166d7b72eef7b3c753593542f1483b94064 && git checkout -B drift-repro e3aa111ad979cf019dda166ca0c6cf5a06946631 && git merge --no-ff 4f263166d7b72eef7b3c753593542f1483b94064

node scripts/docs-audit/affected-docs.mjs --json e3aa111ad979cf019dda166ca0c6cf5a06946631

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Oct 22, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT (landing on green)

domain:cli seat 2 (#22648) · os-bill · session session_01AFrKwndmrCLtF5V9hNtrw3 · 2026-10-22T04:52Z · head 4f263166d7b72eef7b3c753593542f1483b94064 · claim 6331745267 → report 6333000353.

Check Reading
Shape Draft, base main. First line Fixes #24993; Clause-②: no, Metadata-protocol: none. One changeset, @objectstack/cli: patch. 3 files, +324 / −0. check-governed-merges.mjs --pr 25171: not governed, 324 lines ≤ 3000. No contract face, so this ACCEPT is the review of record at the seat's tier.
The card's finding, read against the diff collectExpectedEntries now walks every navigationContributions entry after the apps' own trees, through the same walkNavigation, keyed under the TARGET app and seeded with the item's own label. The entries come from the two carriers translation-target-unknown reads (the stack's manifest and packages[].manifest); an app's own entry wins an id collision (dedupeByPath). The import block and pushBulkActionDefs (PR #24947's region) are untouched.
Evidence New pin (13 cases over real defineStack + composeStacks); the ablation (the contributions loop over []) reds 8, the 5 controls stay green. Both doors on the card's probe shape: deleting a contributed item's zh-CN key (and a contributed group's child) now exits 1 in os lint --strict --json and os i18n check --strict; the own-item control still exits 1. CLI unit tier 323 files / 4769 tests. 66/68 derived families exit 0; check:type-check-debt and check:dual-build-cjs-loads NOT MEASURED locally (verify-lock queue timeout behind another session's dogfood run; unbuilt packages outside the closure) and owed by CI's required jobs.
Changeset prose Read sentence by sentence against the diff: true, including "an item the app also declares keeps the app's entry".
Accepted deviations The rule's reader (contributedNavItemsByApp) is module-private to @objectstack/lint, so the walk carries a ~10-line local reader of the same two carriers, held to the rule by a parity pin that runs validateTranslationReferences over every expected key. Noted for the next touch of @objectstack/lint's translation rule: export that reader and drop the copy, so one reading serves both.
Out of scope Noted, not filed: a module's translation pack cannot carry a contributed item's key without restating the target app's label (TranslationDataSchema requires it; the refusal is loud); the walker and the rule read a nav item's children slightly differently on spec-invalid input (agreement on valid input is pinned).
Landing CI was converging at this reading (17 success, 7 skipped, 14 in_progress, 0 failure). Not governed ⇒ pr_ready + automerge_enable through the relay once every check on this head is success or a by-design skip.

Generated by Claude Code

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 tests tooling

Projects

None yet

2 participants