Skip to content

Commit ebed73b

Browse files
fix(cli)!: docs/nav-target judges each package body's navigationContributions on a multi-package artifact (#27394)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuVpEmthvkRbuHGjC6oSQZvd)_ Fixes #27343 Clause-②: no (narrowing) — on a multi-package `composeStacks(…, { manifest: 'preserve' })` artifact, a `type: 'doc'` item in a package body's `navigationContributions`, whose `doc` or `book` names nothing the artifact carries, built clean before and is now refused by `os build`, `os validate` and `os lint` (`docs/nav-target`). Metadata-protocol: narrowed Skills-docs: none — `skills/objectstack-ui/rules/pages.md` (the `type: 'doc'` nav section) and `content/docs/ui/apps.mdx` (the `docs/nav-target` paragraph) already teach the refusal with no single-package limit; this change makes contributions on multi-package artifacts meet that text. ## 维护者速读 - **改了什么**:`os build`/`os validate`/`os lint` 的文档导航检查(`docs/nav-target`)在多包产物上改为逐包读取各包自己的 `navigationContributions`(只改 `packages/cli/src/utils/collect-docs.ts` 的 `lintDocNavTargets` 一处读取)。 - **为什么改**:多包 `preserve` 产物的顶层 manifest 只是"最后一个包"(主包)的副本,其他包贡献到别的应用里的文档导航项从没被检查过,指向不存在文档的条目会静默通过构建、到运行时才坏。 - **风险与回滚**:一次收窄(BREAKING,`minor`):多包产物里"非主包贡献的导航项指向不存在文档"从静默通过变为构建错误;单包应用不变,主包的贡献只判一次不重复报。回滚即 revert 本 PR。 - **先看**:`packages/cli/src/utils/collect-docs.ts` 中 `lintDocNavTargets` 的 `contributions` 一行;`packages/cli/src/utils/collect-docs.nav-target.test.ts` 新用例;`.changeset/27343-doc-nav-target-package-contributions.md`。 - **两分钟验证**:`pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2 src/utils/collect-docs.nav-target.test.ts`,预期 10 个用例全部通过。 - **需要你做的**:无,席位落地。 ## What changes `lintDocNavTargets` read `navigationContributions` off the artifact's top-level manifest only. Measured on a real two-package `preserve` composition (`composeStacks([svc, crm], { manifest: 'preserve' })`): | Who contributes | Top-level `manifest.navigationContributions` | `packages[i].manifest.navigationContributions` | Issues before | |:--|:--|:--|:--| | first package (`svc`, non-owner) | absent | `packages[0]` only | 0 | | last package (`crm`, owner) | present (the owner's copy) | `packages[1]` | 1 | The contributions now come from each package body when the stack carries `packages[]`, otherwise from the top-level manifest. The walk is the same `artifactPackages(stack)` the function already uses for books and apps; no second walker. **Why not "top level when present, else bodies" (the card's wording).** Falsified by the table: when the owner contributes, the top level carries its copy, so that rule would still skip every other package's contributions. Reading the top level as well as the bodies would judge the owner's twice. The rule here is the one `collectNavGroupInputs` (`packages/cli/src/utils/nav-contribution-groups.ts`) already applies to the same key for the same reason. ## Tests One new case in `collect-docs.nav-target.test.ts`, on the real composer: - the top-level manifest carries no `navigationContributions` when only the non-owner contributes; - a resolving doc passes; - a dead doc in the non-owner's contribution is refused at `apps/setup/navigation/nav_svc_help`; - when both packages contribute a dead doc, exactly two issues come back, one per package (the owner's top-level copy is not judged a second time). ## Acceptance notes - **Narrowed population.** A hand-written artifact that carries `packages[]` and puts contributions on its top-level manifest that no body carries is no longer judged there. The runtime registers each `packages[]` entry's own manifest (ADR-0130 D4), so such a top-level copy is not what gets registered; `collectNavGroupInputs` makes the same call. - **Declaration lines** follow the dispatch: `Clause-②: no (narrowing)` and `Metadata-protocol: narrowed`, so the changeset is `minor` with a BREAKING banner and the ADR-0087 `not-required (no-migration-prescription)` disposition. The claim comment carries the bare `Clause-②: no`; the narrowing arm is from triage and the dispatch. - No in-tree example's `navigationContributions` carries a `type: 'doc'` item (the dogfood battery below is green). ## Verification Final head `6c5250cbd` (origin/main `28872e68d` merged in). The unit tier, typecheck and the first gate run are at `260fe77e0`, before that merge; the merge touches none of the three changed files. The dogfood battery, the gate re-run and the target test re-run are at `6c5250cbd`. <details><summary>Red, then green, then ablation</summary> - Before the source change, the new case: `Tests 1 failed | 9 passed (10)`, `AssertionError: expected [] to deeply equal [ [ 'docs/nav-target', …(1) ] ]`. - After it: `Tests 10 passed (10)`. - Ablation at `91e14b22` (fix committed first), through `scripts/ablation-replace.mjs`: anchor `const contributions = (bodies.length > 0` replaced with `const contributions = (false` (the old top-level-only read). Anchor x1 to x0, replacement x0 to x1, blob `f64208d1cd06` to `edd969ecd1bb`. Run: the new case fails with the same `expected [] …` assertion, `Tests 1 failed | 9 passed (10)`. Restore: blob equals HEAD (`f64208d1cd06`), `git diff HEAD` empty. The test imports `./collect-docs.js` from source, so no `dist/` rebuild is in the path. </details> <details><summary>Package checks</summary> - `pnpm --filter @objectstack/cli exec vitest run --project unit --maxWorkers=2` (under the verify lock, `VERDICT command-exit 0`): `Test Files 346 passed (346)`, `Tests 5033 passed (5033)`. The integration tier is left to CI. - `pnpm --filter @objectstack/cli build && pnpm --filter @objectstack/cli typecheck`: `VERDICT command-exit 0`; `check:test-typecheck: OK`. - eslint with the repo config over `packages/cli` (`eslint packages/cli --no-inline-config`): exit 0. </details> <details><summary>Derived gates</summary> - First run at `260fe77e0`: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 60 families on 3 changed paths (51 changed lines). All 60 ran, exit codes captured before any pipe; 57 exit 0. Three exit 3 on a prerequisite: `check-plugin-teardown-shape --self-test` (its pinned fixture commit was not in the shallow clone; exit 0, `48 cases pass` after fetching that commit), `check:i18n-coverage` (`@objectstack/connector-slack` had no build; exit 0 after building it) and `check:dual-build-cjs-loads` (needs a full workspace build). - Re-run at `6c5250cbd` after the dogfood closure build: the same 60 families (derivation unchanged), all 60 exit 0, `check:dual-build-cjs-loads` included. `check-adr-0087-registration --base origin/main` exit 0: the changeset reads `BREAKING+bang+clause-②-narrowing+metadata-protocol-narrowed`, `not-required (no-migration-prescription)`. `check:i18n-coverage`: `OK (14 config(s), 527 baselined untranslated string(s), none new)`. - `dispatch-gates --ran` at `6c5250cbd`: `60 derived famil(ies) accounted for — 60 run, 0 NOT-MEASURED (a DERIVED zero — all 60 recorded an exit code and none of them is 3)`. - Target test re-run at `6c5250cbd`: `Tests 10 passed (10)`. </details> <details><summary>Dogfood battery (owed by the narrowing arm)</summary> At `6c5250cbd`, under the verify lock, after building the `@objectstack/dogfood` dependency closure, `pnpm --filter @objectstack/dogfood test` with no shard and no path: `Test Files 348 passed | 3 skipped (351)`, `Tests 2806 passed | 20 skipped (2826)`, lock `VERDICT command-exit 0`. No failures. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01L6bhgXcN9HaZYSEUdJWEjw)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01L6bhgXcN9HaZYSEUdJWEjw --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9f76037 commit ebed73b

3 files changed

Lines changed: 49 additions & 2 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/cli': minor
3+
---
4+
5+
fix(cli)!: `docs/nav-target` judges the navigation each package body contributes on a multi-package artifact
6+
7+
Clause-②: no (narrowing) — on a multi-package `composeStacks(…, { manifest: 'preserve' })` artifact, a `type: 'doc'` item in a package body's `navigationContributions`, whose `doc` or `book` names nothing the artifact carries, built clean before and is now refused by `os build`, `os validate` and `os lint` (`docs/nav-target`).
8+
Metadata-protocol: narrowed
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type of the package's public surface changes: the build-time docs lint now reaches the navigation contributions a package body carries, judging them by the rule it already applies to a single-package stack's contributions. A refused item is fixed in the contribution; there is nothing for a migration to rewrite. -->
11+
12+
**BREAKING** accept-set narrowing in the build-time docs lint (`os build`, `os validate`, `os lint`).
13+
14+
- **Before.** The `docs/nav-target` check read `navigationContributions` off the artifact's top-level manifest only. On a multi-package `preserve` artifact that manifest is the owner package's copy, so a contribution from any other package was never judged: a dead `doc` or `book` target built clean and degraded at runtime.
15+
- **Now.** When the stack carries `packages[]`, the check reads each package body's `navigationContributions` (the owner's included, judged once); otherwise the top-level manifest's. A contributed `doc` item resolves against every doc the artifact carries, and a `book` item against the declared books and package ids, as a single-package contribution always did.
16+
- **Unchanged.** A stack with no `packages[]` is judged exactly as before.
17+
18+
**The remedy.** For a refused item, name a doc in `src/docs/*.md` or a declared book (a package id is its own implicit book), or remove the item from the contribution. The refusal message names the nearest match.

‎packages/cli/src/utils/collect-docs.nav-target.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,25 @@ describe('docs/nav-target — a doc nav item must open something this package ha
104104
expect(issues.map((i) => [i.rule, i.path])).toEqual([['docs/nav-target', 'apps/crm_app/navigation/nav_page']]);
105105
});
106106

107+
it('judges the doc entries every package body contributes on a multi-package `preserve` artifact, each once', () => {
108+
// The real composer: the top-level manifest is the last package's (the owner's) copy, so a
109+
// non-owner package's contributions sit only in its body.
110+
const contribution = (id: string, doc: string) => [{ app: 'setup', items: [{ id, type: 'doc', doc }] }];
111+
const composed = (svc: unknown, crm?: unknown) => JSON.parse(JSON.stringify(composeStacks([
112+
defineStack({ manifest: { id: 'com.example.svc', name: 'svc', namespace: 'svc', version: '1.0.0', type: 'app', navigationContributions: svc } } as never, { strict: false }),
113+
defineStack({ manifest: { id: 'com.example.crm', name: 'crm', namespace: 'crm', version: '1.0.0', type: 'app', ...(crm ? { navigationContributions: crm } : {}) } } as never, { strict: false }),
114+
], { manifest: 'preserve' }))) as Record<string, unknown>;
115+
const nonOwner = composed(contribution('nav_svc_help', 'ghost_doc'));
116+
expect((nonOwner.manifest as Record<string, unknown>).navigationContributions).toBeUndefined();
117+
expect(lintDocNavTargets(composed(contribution('nav_svc_help', 'crm_intro')), DOCS)).toEqual([]);
118+
expect(lintDocNavTargets(nonOwner, DOCS).map((i) => [i.rule, i.path]))
119+
.toEqual([['docs/nav-target', 'apps/setup/navigation/nav_svc_help']]);
120+
// The owner contributes too: its top-level copy is not judged a second time.
121+
const both = composed(contribution('nav_svc_help', 'ghost_doc'), contribution('nav_crm_help', 'ghost_doc'));
122+
expect(lintDocNavTargets(both, DOCS).map((i) => i.path))
123+
.toEqual(['apps/setup/navigation/nav_svc_help', 'apps/setup/navigation/nav_crm_help']);
124+
});
125+
107126
it('resolves a book declared on a package body, and that package id as its implicit book', () => {
108127
expect(lintDocNavTargets({
109128
packages: [{ manifest: { id: 'com.example.help', books: [{ name: 'help_centre', groups: [] }] } }],

‎packages/cli/src/utils/collect-docs.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,7 +1110,9 @@ export function lintMetadataEmbeds(
11101110
*
11111111
* Walked: every app's `navigation` and `areas[].navigation` (children
11121112
* included) — the top level's apps, or each package body's when the top level
1113-
* carries none — and the items of `manifest.navigationContributions`.
1113+
* carries none — and the items of `navigationContributions`: each package
1114+
* body's when the stack carries `packages[]`, otherwise the top-level
1115+
* manifest's.
11141116
*
11151117
* ## What resolves
11161118
*
@@ -1194,7 +1196,15 @@ export function lintDocNavTargets(stack: Record<string, unknown>, docNames: Read
11941196
// A package may also CONTRIBUTE a doc entry into an app it does not own
11951197
// (ADR-0029 D7). The entry opens this package's docs, so it is judged here,
11961198
// against this package's set, under the name of the app it lands in.
1197-
const contributions = (stack.manifest as { navigationContributions?: unknown } | undefined)?.navigationContributions;
1199+
// On a multi-package `preserve` artifact each package's contributions sit in
1200+
// its body, and the top-level manifest is only the owner's copy: each body's
1201+
// when the stack carries `packages[]` (so the owner's are judged once),
1202+
// otherwise the top-level manifest's — `collectNavGroupInputs`' rule, over
1203+
// the same `artifactPackages` walk as the books and apps above.
1204+
const bodies = artifactPackages(stack);
1205+
const contributions = (bodies.length > 0
1206+
? bodies.flatMap(({ body }) => (Array.isArray(body.navigationContributions) ? body.navigationContributions : []))
1207+
: (stack.manifest as { navigationContributions?: unknown } | undefined)?.navigationContributions);
11981208
if (Array.isArray(contributions)) {
11991209
for (const c of contributions as AnyRec[]) {
12001210
check(c?.items, typeof c?.app === 'string' ? c.app : '(unnamed)');

0 commit comments

Comments
 (0)