From ed89bef4558000e751a8e6e3a8723d3380e7be6f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 06:06:19 +0000 Subject: [PATCH 1/7] feat(cli): read package docs from each package directory of an ADR-0130 layout Per-package docs attach to the owning package's body (packages[i].manifest.docs, ADR-0130 D4 option B) and are linted against that package's own namespace; a stack-level doc keeps stack.manifest.namespace. The #18170/#18428 warning stays for a directory neither convention reads, and now names why. Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- .../18431-per-package-docs-collector.md | 23 + packages/cli/src/commands/compile.ts | 32 +- packages/cli/src/commands/serve.ts | 17 +- .../utils/collect-docs.package-docs.test.ts | 379 +++++++++++++++++ packages/cli/src/utils/collect-docs.ts | 394 ++++++++++++++++-- 5 files changed, 810 insertions(+), 35 deletions(-) create mode 100644 .changeset/18431-per-package-docs-collector.md create mode 100644 packages/cli/src/utils/collect-docs.package-docs.test.ts diff --git a/.changeset/18431-per-package-docs-collector.md b/.changeset/18431-per-package-docs-collector.md new file mode 100644 index 00000000000..1d6cb189437 --- /dev/null +++ b/.changeset/18431-per-package-docs-collector.md @@ -0,0 +1,23 @@ +--- +"@objectstack/cli": minor +--- + +Clause-②: yes + +`os build` reads package docs from **each package directory** of an ADR-0130 layout — `src//docs/*.md` — and attaches them to the **owning package's body** (`packages[i].manifest.docs`), linted against **that package's own `namespace`** (#18431). + +A module can now ship its own docs. Before this, ADR-0046 collection was anchored at exactly one path, `/src/docs`, so an ADR-0130 project that moved its docs into their packages lost all of them — loudly since #18428, but lost. The maintainer's ruling (batch #147 item 4) decided the two contract questions that blocked the widening, and both are implemented literally: + +- **Where they attach**: to `packages[i]`, ⛔ never the artifact top level. The runtime already merges a package-owned collection back up for readers (`resolveArtifactCollections`, ADR-0130 D4), so a flattened copy would buy nothing and destroy the ownership D1 is about. +- **Whose namespace the lint uses**: the owning package's. A doc outside any package keeps `stack.manifest.namespace`. A multi-package artifact therefore has **one prefix rule per package** and ⛔ no single global prefix — and ⛔ no fallback between the two: a package doc that fails its own package's prefix is refused, never re-tried against the artifact's. + +What that costs, stated plainly: in a multi-package artifact whose packages declare namespaces different from the artifact manifest's, a doc owned by a package is now judged by the package's prefix. That shape could not ship docs at all before (the single global prefix refused it), which is why this lands as a widening; a doc that was named for the artifact's namespace while living inside a differently-namespaced package now asks to be renamed, and the refusal names the spelling. + +Also in this change: + +- **The #18428 warning stays**, and now says *why* a directory was not read. Unchanged, word for word, for a stack that declares no `packages[]` — where "read from `src/docs/` only" is still the whole truth. For a directory that names **no** package it lists the declared packages and the three spellings a directory is matched against (`id`, the last dot-segment of `id`, `name`); for one that names **more than one** it names the candidates and refuses to guess. ⛔ `namespace` is not a matching spelling: ADR-0130 D1 exists so that N packages can share one, so matching on it would be ambiguous exactly where it matters. +- **A cross-owner duplicate doc name is an error.** Doc uniqueness is logical — the metadata registry key carries no package coordinate — so once the prefix rule runs per package, two packages sharing a namespace can declare one name and silently overwrite each other at registration. Nothing else was looking across the sets. +- **`os dev` mirrors `os build`.** The config-load path collects the same per-package directories onto the same bodies, so dev serves what a built artifact serves. +- **The step line counts the whole collection**, package sets included, and says how many came from package directories — a build that read four package docs no longer announces `0 collected`. + +Single-package projects are untouched: with no `packages[]` there is nothing to attribute, the flat `src/docs/` keeps attaching exactly where it always did, and the emitted artifact is byte-identical. diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index 0f708f8fa9c..99ac29a47b4 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -21,7 +21,7 @@ import { buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint'; import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '@objectstack/lint'; import { resolveSduiManifest } from '../utils/sdui-manifest.js'; import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js'; -import { collectAndLintDocs, type DocIssue } from '../utils/collect-docs.js'; +import { attachPackageDocs, collectAndLintDocs, type DocIssue } from '../utils/collect-docs.js'; import { buildRuntimeBundle, cleanupOldRuntimeBundles } from '../utils/build-runtime.js'; import { printHeader, @@ -767,8 +767,24 @@ export default class Compile extends Command { // a run that collected four documents. Reporting the count is what // makes the two runs distinguishable; the ordering is what makes the // count available to report. + // + // [#18431] The count is the whole collection, per-package sets included. + // Those docs do not join `docs` — they go to the body of the package + // that owns them (ADR-0130 D4 option B) — so counting only the + // top-level array would re-create exactly the defect above one layer + // down: a build that read four package docs announcing `0 collected`. + // The parenthetical is added only when there ARE package docs, so a + // single-package build's line is unchanged. const docsResult = collectAndLintDocs(absolutePath, result.data as Record); - if (!flags.json) printStep(`Collecting package docs (ADR-0046)... ${docsResult.docs.length} collected`); + const packageDocCount = docsResult.packageDocs.reduce((n, set) => n + set.docs.length, 0); + if (!flags.json) { + printStep( + `Collecting package docs (ADR-0046)... ${docsResult.docs.length + packageDocCount} collected` + + (packageDocCount > 0 + ? ` (${packageDocCount} from ${docsResult.packageDocs.length} package director${docsResult.packageDocs.length === 1 ? 'y' : 'ies'})` + : ''), + ); + } const docErrors = docsResult.issues.filter((i) => i.severity === 'error'); // [#11727] Consumed by BOTH faces — the text block below and the `--json` // payload. Only the text block read it before, so the advisories were @@ -816,6 +832,18 @@ export default class Compile extends Command { if (docsResult.docs.length > 0) { finalBundle.docs = docsResult.docs; } + // [#18431] Docs read out of `src//docs/` attach to the body of the + // package that owns them — `packages[i].manifest`, ADR-0130 D4 option + // B — and ⛔ never to the top level, which is the maintainer's ruling + // (batch #147 item 4) and also what keeps ownership readable: the + // runtime merges a package-owned collection back up through + // `resolveArtifactCollections`, so a flattened copy would buy nothing + // and destroy the attribution. `attachPackageDocs` hands back the + // ARGUMENT when it adds nothing, so a stack with no per-package docs + // serializes from the very same references as before. + if (docsResult.packageDocs.length > 0) { + finalBundle.packages = attachPackageDocs(finalBundle.packages, docsResult.packageDocs); + } // 4b. Bundle handler functions into `/objectstack-runtime.{hash}.mjs` // and stamp the relative path into the JSON so the runtime can diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 803b955fb6a..382afd0e5f9 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -2639,10 +2639,19 @@ export default class Serve extends Command { // them and an artifact boot serves them. Mirror compile's collection so // docs render under /docs/ in dev exactly as from a built artifact. // Collection only (no lint-fail): docs are additive; never block boot. + // + // [#18431] The same mirroring, one level down: a `src//docs/` + // directory naming one of this config's `packages[]` entries is collected + // onto THAT package's body (ADR-0130 D4 option B), exactly where + // `os build` puts it. `AppPlugin` reads a package-owned collection back + // up through `resolveArtifactCollections`, so dev serves them as an + // artifact boot does. Leaving this half out would re-open the asymmetry + // the paragraph above exists to close — `os build` producing docs that + // `os dev` cannot show. if (!useArtifactFallback) { try { - const { collectDocsFromSrc } = await import('../utils/collect-docs.js'); - const collected = collectDocsFromSrc(absolutePath); + const { collectDocsFromSrc, attachPackageDocs } = await import('../utils/collect-docs.js'); + const collected = collectDocsFromSrc(absolutePath, (config as any)?.packages); if (collected.docs.length > 0) { const byName = new Map(); for (const d of (Array.isArray((config as any).docs) ? (config as any).docs : [])) { @@ -2651,6 +2660,10 @@ export default class Serve extends Command { for (const d of collected.docs) byName.set(d.name, d); config = { ...config, docs: Array.from(byName.values()) }; } + if (collected.packageDocs.length > 0) { + const packages = attachPackageDocs((config as any).packages, collected.packageDocs); + if (packages !== (config as any).packages) config = { ...config, packages }; + } } catch { /* docs are additive — never block boot on collection */ } diff --git a/packages/cli/src/utils/collect-docs.package-docs.test.ts b/packages/cli/src/utils/collect-docs.package-docs.test.ts new file mode 100644 index 00000000000..9736d687211 --- /dev/null +++ b/packages/cli/src/utils/collect-docs.package-docs.test.ts @@ -0,0 +1,379 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #18431 — per-package docs: `src//docs/` read into the OWNING package's + * body, and linted against that package's OWN namespace. + * + * The maintainer's ruling (batch #147 item 4) decided the two contract + * questions #18170 left open, and every case here is one of its clauses: + * + * 1. per-package docs attach to `packages[i]` — ⛔ not the artifact top level; + * 2. the doc lint uses the OWNING package's `namespace`; a doc outside any + * package keeps `stack.manifest.namespace`, and there is ⛔ no single + * global prefix; + * 3. the directory convention (`src//docs/`) is the one implemented + * first — the `defineStack({ docs })` spelling already reaches + * `packages[i].manifest.docs` through `composeStacks(…, { manifest: + * 'preserve' })`, which is measured in `package-body-docs-are-composed` + * below so the claim is not just asserted in a PR body; + * 4. the #18428 warning STAYS for docs in a place neither convention reads. + * + * ⚠️ Every assertion that a per-package pass "produced something" also asserts + * its PEDIGREE — the directory it came from, the package it was attributed to, + * and a marker string written into that one file — because a count alone is + * satisfied by an echo of a doc that was already somewhere else. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { composeStacks, defineStack } from '@objectstack/spec'; + +import { + attachPackageDocs, + collectAndLintDocs, + collectDocsFromSrc, + docsPackageRefs, + type DocItem, +} from './collect-docs.js'; + +let tmp: string; +let configPath: string; + +beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'os-pkg-docs-')); + configPath = path.join(tmp, 'objectstack.config.ts'); + fs.writeFileSync(configPath, '// stub'); + fs.mkdirSync(path.join(tmp, 'src'), { recursive: true }); +}); + +afterEach(() => { + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +/** Write one Markdown file into `src//docs/`, and hand back its marker. */ +const writePackageDoc = (dir: string, name: string, marker: string): string => { + const target = path.join(tmp, 'src', dir, 'docs'); + fs.mkdirSync(target, { recursive: true }); + fs.writeFileSync(path.join(target, `${name}.md`), `# ${name}\n\n${marker}\n`); + return marker; +}; + +const writeFlatDoc = (name: string, body: string) => { + const target = path.join(tmp, 'src', 'docs'); + fs.mkdirSync(target, { recursive: true }); + fs.writeFileSync(path.join(target, `${name}.md`), body); +}; + +/** An artifact `packages[]` entry, in the `{ manifest: }` wrapper D4 reserves. */ +const pkg = (body: Record) => ({ manifest: body }); + +const CORE = { + id: 'com.example.multi.core', + name: 'Multi-Package Core', + namespace: 'crm', + version: '1.0.0', + type: 'app', +}; +const ORDERS = { + id: 'com.example.multi.orders', + name: 'orders', + namespace: 'sales', + version: '1.0.0', + type: 'module', +}; + +/** The composed artifact these fixtures stand in for: two packages, `crm` on top. */ +const stack = (extra: Record = {}) => ({ + manifest: { ...CORE }, + packages: [pkg({ ...CORE }), pkg({ ...ORDERS })], + ...extra, +}); + +// ── Clause 3's measurement, held as a test rather than as a sentence ──────── +describe('the `defineStack({ docs })` spelling needs no collector work at all', () => { + it('package-body-docs-are-composed: `preserve` already puts a package\'s inline docs on its own body', () => { + const core = defineStack({ + manifest: { id: 'com.example.p.core', name: 'core', version: '1.0.0', type: 'app', namespace: 'crm' }, + docs: [{ name: 'crm_core_guide', label: 'Core', content: '# Core' }], + }); + const orders = defineStack({ + manifest: { id: 'com.example.p.orders', name: 'orders', version: '1.0.0', type: 'module', namespace: 'sales' }, + docs: [{ name: 'sales_orders_guide', label: 'Orders', content: '# Orders' }], + }); + const composed = composeStacks([core, orders], { manifest: 'preserve' }) as unknown as { + packages: Array<{ manifest: { id: string; docs?: DocItem[] } }>; + }; + + // This is the whole of the "which convention costs a second traversal" + // measurement: this one costs no traversal because composition already did + // the attribution — so the DIRECTORY convention is what was implemented. + expect(composed.packages.map((p) => [p.manifest.id, (p.manifest.docs ?? []).map((d) => d.name)])).toEqual([ + ['com.example.p.core', ['crm_core_guide']], + ['com.example.p.orders', ['sales_orders_guide']], + ]); + }); +}); + +// ── Clause 1: collection + attribution ───────────────────────────────────── +describe('collectDocsFromSrc reads src//docs/ for a resolvable package', () => { + it('attributes a directory named by the last segment of the package id, with its pedigree', () => { + const marker = writePackageDoc('orders', 'sales_playbook', 'MARKER-orders-playbook'); + const { docs, issues, packageDocs } = collectDocsFromSrc(configPath, stack().packages); + + expect(issues).toEqual([]); + expect(docs).toEqual([]); // ⛔ NOT the top level — the ruling's clause 1 + expect(packageDocs).toHaveLength(1); + const [set] = packageDocs; + // Pedigree, not just a count: where it was read, who owns it, what is in it. + expect(set.dir).toBe('src/orders/docs'); + expect(set.index).toBe(1); + expect(set.id).toBe('com.example.multi.orders'); + expect(set.namespace).toBe('sales'); + expect(set.docs.map((d) => d.name)).toEqual(['sales_playbook']); + expect(set.docs[0].content).toContain(marker); + expect(set.docs[0].label).toBe('sales_playbook'); // the first `#` heading + }); + + it('resolves a directory named by the package `name`, and one named by the full `id`', () => { + writePackageDoc('orders', 'sales_a', 'MARKER-a'); // ORDERS.name === 'orders' + writePackageDoc('com.example.multi.core', 'crm_b', 'MARKER-b'); // the full id + + const { packageDocs } = collectDocsFromSrc(configPath, stack().packages); + expect(packageDocs.map((s) => [s.dir, s.id, s.docs.map((d) => d.name)])).toEqual([ + ['src/com.example.multi.core/docs', 'com.example.multi.core', ['crm_b']], + ['src/orders/docs', 'com.example.multi.orders', ['sales_a']], + ]); + }); + + it('keeps reading the flat src/docs/ alongside the per-package ones', () => { + writeFlatDoc('crm_index', '# CRM'); + writePackageDoc('orders', 'sales_playbook', 'MARKER-both'); + + const { docs, packageDocs, issues } = collectDocsFromSrc(configPath, stack().packages); + expect(docs.map((d) => d.name)).toEqual(['crm_index']); + expect(packageDocs.flatMap((s) => s.docs.map((d) => d.name))).toEqual(['sales_playbook']); + expect(issues).toEqual([]); + }); + + it('applies the flatness rule inside a package directory, naming THAT directory', () => { + writePackageDoc('orders', 'sales_ok', 'MARKER-flat'); + fs.mkdirSync(path.join(tmp, 'src', 'orders', 'docs', 'deep')); + fs.writeFileSync(path.join(tmp, 'src', 'orders', 'docs', 'deep', 'sales_x.md'), '# x'); + + const { issues, packageDocs } = collectDocsFromSrc(configPath, stack().packages); + expect(issues.map((i) => [i.rule, i.path])).toEqual([['docs/flat-directory', 'src/orders/docs/deep']]); + expect(issues[0].message).toContain('under src/orders/docs/'); + // ...and the sibling file is still collected, so the error is not a bail-out. + expect(packageDocs[0].docs.map((d) => d.name)).toEqual(['sales_ok']); + }); +}); + +// ── Clause 4: the #18428 warning stays, and says WHY ─────────────────────── +describe('the #18170 warning stays for a directory neither convention reads', () => { + it('a stack with no packages[] gets the original sentence, unchanged', () => { + writePackageDoc('sales', 'crm_index', 'MARKER-none'); + const { docs, packageDocs, issues } = collectDocsFromSrc(configPath); + + expect(docs).toEqual([]); + expect(packageDocs).toEqual([]); + expect(issues).toHaveLength(1); + expect(issues[0].severity).toBe('warning'); + expect(issues[0].rule).toBe('docs/uncollected-directory'); + expect(issues[0].path).toBe('src/sales/docs'); + // The exact pre-#18431 text, so a rewrite of the single-package message + // fails here rather than in a customer's build log. + expect(issues[0].message).toBe( + 'src/sales/docs/ holds 1 Markdown file(s) that were NOT collected: package docs are read from src/docs/ only' + + " (ADR-0046 §3.2), so these are absent from the artifact's `docs[]` and from every book that includes them." + + ' Move them into src/docs/ (doc names carry the package namespace prefix, so packages do not collide there),' + + ' declare them inline as `defineStack({ docs })`, or delete them if they are not package docs.' + + ' Found: crm_index.md', + ); + }); + + it('a directory naming NO package is reported with the declared package list', () => { + writePackageDoc('billing', 'crm_index', 'MARKER-unmatched'); + const { packageDocs, issues } = collectDocsFromSrc(configPath, stack().packages); + + expect(packageDocs).toEqual([]); + expect(issues).toHaveLength(1); + expect(issues[0].severity).toBe('warning'); + expect(issues[0].rule).toBe('docs/uncollected-directory'); + expect(issues[0].message).toContain('"billing" names none of this artifact\'s packages'); + expect(issues[0].message).toContain('com.example.multi.core, com.example.multi.orders'); + expect(issues[0].message).toContain('crm_index.md'); + }); + + it('an AMBIGUOUS directory is reported and ⛔ never guessed', () => { + writePackageDoc('core', 'crm_index', 'MARKER-ambiguous'); + const twins = [pkg({ ...CORE }), pkg({ ...CORE, id: 'com.other.core', name: 'Other Core' })]; + + const { packageDocs, issues } = collectDocsFromSrc(configPath, twins); + expect(packageDocs).toEqual([]); + expect(issues).toHaveLength(1); + expect(issues[0].message).toContain('names 2 of this artifact\'s packages'); + expect(issues[0].message).toContain('com.example.multi.core, com.other.core'); + }); + + it('⛔ namespace is not a resolution spelling — shared namespaces are the ADR-0130 D1 case', () => { + // Both packages declare `crm`; a `src/crm/docs` directory must therefore + // resolve to NEITHER, not to the first one that happens to match. + writePackageDoc('crm', 'crm_index', 'MARKER-ns'); + const shared = [pkg({ ...CORE }), pkg({ ...ORDERS, namespace: 'crm' })]; + + const { packageDocs, issues } = collectDocsFromSrc(configPath, shared); + expect(packageDocs).toEqual([]); + expect(issues.map((i) => i.rule)).toEqual(['docs/uncollected-directory']); + expect(issues[0].message).toContain('"crm" names none of this artifact\'s packages'); + }); +}); + +// ── Clause 2: one prefix rule per package ────────────────────────────────── +describe('the doc lint reads the OWNING package namespace', () => { + it('accepts a package doc carrying its OWN package prefix, not the artifact manifest one', () => { + writePackageDoc('orders', 'sales_playbook', 'MARKER-ns-own'); + const { issues } = collectAndLintDocs(configPath, stack()); + + // `stack.manifest.namespace` is `crm`; the owning package's is `sales`. + // Before the ruling this was a build-failing `docs/namespace-prefix`. + expect(issues.filter((i) => i.severity === 'error')).toEqual([]); + }); + + it('REFUSES a package doc carrying the artifact prefix instead of its own — no fallback', () => { + writePackageDoc('orders', 'crm_playbook', 'MARKER-ns-wrong'); + const { issues } = collectAndLintDocs(configPath, stack()); + + const refusal = issues.filter((i) => i.rule === 'docs/namespace-prefix'); + expect(refusal).toHaveLength(1); + expect(refusal[0].severity).toBe('error'); + expect(refusal[0].path).toBe('packages[1].docs/crm_playbook'); + expect(refusal[0].message).toContain('rename to "sales_crm_playbook"'); + }); + + it('a stack-level doc outside any package keeps stack.manifest.namespace', () => { + writeFlatDoc('sales_orphan', '# Orphan'); // `sales` is a PACKAGE namespace, not the stack's + const { issues } = collectAndLintDocs(configPath, stack()); + + const refusal = issues.filter((i) => i.rule === 'docs/namespace-prefix'); + expect(refusal).toHaveLength(1); + expect(refusal[0].path).toBe('docs/sales_orphan'); // no `packages[i].` prefix + expect(refusal[0].message).toContain('rename to "crm_sales_orphan"'); + }); + + it('lints a package\'s INLINE body docs against that package too, and only once', () => { + const inlineDoc: DocItem = { name: 'crm_orders_inline', content: '# Inline' }; + const composedShape = { + manifest: { ...CORE }, + // What `composeStacks(…, { manifest: 'preserve' })` produces: the SAME + // item object on the body and in the flattened top level. + docs: [inlineDoc], + packages: [pkg({ ...CORE }), pkg({ ...ORDERS, docs: [inlineDoc] })], + }; + + const { issues } = collectAndLintDocs(configPath, composedShape); + const refusal = issues.filter((i) => i.rule === 'docs/namespace-prefix'); + expect(refusal).toHaveLength(1); // once — judged by `sales`, not twice by `sales` and `crm` + expect(refusal[0].path).toBe('packages[1].docs/crm_orders_inline'); + }); + + it('reports a doc name declared by two different owners — the one thing per-package lint cannot see', () => { + writePackageDoc('core', 'crm_shared', 'MARKER-dup-core'); + writePackageDoc('orders', 'crm_shared', 'MARKER-dup-orders'); + // Both packages share one namespace, which is exactly what ADR-0130 D1 buys + // — so the prefix rule does NOT keep these apart. + const shared = [pkg({ ...CORE }), pkg({ ...ORDERS, namespace: 'crm', name: 'orders' })]; + + const { issues } = collectAndLintDocs(configPath, { manifest: { ...CORE }, packages: shared }); + const dup = issues.filter((i) => i.rule === 'docs/duplicate-name'); + expect(dup).toHaveLength(1); + expect(dup[0].severity).toBe('error'); + expect(dup[0].message).toContain('package "com.example.multi.core" and package "com.example.multi.orders"'); + }); +}); + +// ── "Nothing existing moves" — the single-package shape, item for item ───── +describe('a stack with no packages[] is on the path it was always on', () => { + it('produces the same docs and the same issues whether or not `packages` is passed', () => { + writeFlatDoc('crm_index', '# CRM Overview\n\n[link](./crm_missing.md)'); + writePackageDoc('sales', 'crm_moved', 'MARKER-unchanged'); + const single = { manifest: { namespace: 'crm' } }; + + const withoutArg = collectDocsFromSrc(configPath); + const withEmpty = collectDocsFromSrc(configPath, undefined); + expect(withEmpty).toEqual(withoutArg); + + const linted = collectAndLintDocs(configPath, single); + expect(linted.docs.map((d) => d.name)).toEqual(['crm_index']); + expect(linted.packageDocs).toEqual([]); + // The broken link and the uncollected directory, exactly as before — no + // per-package path taken, no issue re-located under a `packages[i].`. + expect(linted.issues.map((i) => [i.rule, i.path])).toEqual([ + ['docs/uncollected-directory', 'src/sales/docs'], + ['docs/broken-link', 'docs/crm_index'], + ]); + }); +}); + +// ── docsPackageRefs + attachPackageDocs ──────────────────────────────────── +describe('docsPackageRefs', () => { + it('derives exactly three directory spellings per package, and no namespace', () => { + expect(docsPackageRefs(stack().packages)).toEqual([ + { + index: 0, + id: 'com.example.multi.core', + namespace: 'crm', + directoryNames: ['com.example.multi.core', 'core', 'Multi-Package Core'], + }, + { + index: 1, + id: 'com.example.multi.orders', + namespace: 'sales', + directoryNames: ['com.example.multi.orders', 'orders'], + }, + ]); + }); + + it('is empty for anything that is not an array', () => { + expect(docsPackageRefs(undefined)).toEqual([]); + expect(docsPackageRefs({})).toEqual([]); + }); +}); + +describe('attachPackageDocs', () => { + const set = (index: number, names: string[]) => ({ + index, + id: `p${index}`, + dir: `src/p${index}/docs`, + docs: names.map((name) => ({ name, content: `# ${name}` })), + }); + + it('writes onto packages[i].manifest.docs and leaves every other entry alone', () => { + const packages = [pkg({ ...CORE }), pkg({ ...ORDERS })]; + const out = attachPackageDocs(packages, [set(1, ['sales_playbook'])]) as Array<{ + manifest: Record; + }>; + + expect(out[0]).toBe(packages[0]); // untouched entry keeps its identity + expect((out[1].manifest.docs as DocItem[]).map((d) => d.name)).toEqual(['sales_playbook']); + expect(packages[1].manifest).not.toHaveProperty('docs'); // ⛔ no mutation in place + }); + + it('appends after the docs the body already carried, without duplicating them', () => { + const existing: DocItem = { name: 'sales_inline', content: '# Inline' }; + const packages = [pkg({ ...ORDERS, docs: [existing] })]; + const out = attachPackageDocs(packages, [ + { ...set(0, ['sales_playbook']), docs: [existing, { name: 'sales_playbook', content: '# sales_playbook' }] }, + ]) as Array<{ manifest: { docs: DocItem[] } }>; + + expect(out[0].manifest.docs.map((d) => d.name)).toEqual(['sales_inline', 'sales_playbook']); + }); + + it('hands back the ARGUMENT when it adds nothing — identity, not equality', () => { + const packages = [pkg({ ...CORE })]; + expect(attachPackageDocs(packages, [])).toBe(packages); + expect(attachPackageDocs(packages, [set(0, [])])).toBe(packages); + expect(attachPackageDocs(undefined, [set(0, ['x'])])).toBeUndefined(); + }); +}); diff --git a/packages/cli/src/utils/collect-docs.ts b/packages/cli/src/utils/collect-docs.ts index 3b2127035ba..6f5e6013277 100644 --- a/packages/cli/src/utils/collect-docs.ts +++ b/packages/cli/src/utils/collect-docs.ts @@ -34,6 +34,8 @@ import fs from 'fs'; import path from 'path'; +import { artifactPackages } from './artifact-packages.js'; + export interface DocTranslationItem { label?: string; description?: string; @@ -261,63 +263,194 @@ function markdownFilesIn(dir: string): string[] { * build (`compile.ts` exits 1 on any doc error), which would refuse a tree that * builds green today on a directory this collector can only GUESS was meant as * ADR-0046 docs — a `src//docs/` directory is not declared anywhere the - * build can read. Reading those files instead (the card's other option) widens - * the accepted set and needs a ruling this does not make: an ADR-0130 D4 - * artifact registers per package, so per-package docs would have to say which - * package body they belong to. ⛔ Neither is decided here; the loss is made - * audible, which is the card's stated minimum. + * build can read. + * + * ## What #18431 changed, and what it deliberately did NOT + * + * The maintainer's ruling (batch #147 item 4) answered the two contract + * questions this docblock used to record as open: per-package docs attach to + * the OWNING PACKAGE'S BODY (`packages[]`, ADR-0130 D4 option B), and the doc + * lint reads that package's OWN `namespace`. So a `src//docs/` directory + * whose `` names one of the artifact's `packages[]` entries is now + * COLLECTED, by {@link sweepPackageDocsDirectories} below. + * + * ⛔ The warning is not removed and not weakened — the ruling keeps it for docs + * in a place NEITHER convention reads, which is now a smaller but sharper set: + * a stack that declares no `packages[]` at all (every single-package app — + * where the message below is unchanged, because for that stack it is still + * exactly true), and a directory whose name matches no package or matches more + * than one (where the message NAMES the candidates, because "I read this + * convention and could not attribute the result" is a different fact from "I do + * not read this convention"). + */ +function uncollectedDocsMessage(rel: string, files: readonly string[]): string { + return `${rel}/ holds ${files.length} Markdown file(s) that were NOT collected: package docs are read from src/docs/ only (ADR-0046 §3.2), so these are absent from the artifact's \`docs[]\` and from every book that includes them. Move them into src/docs/ (doc names carry the package namespace prefix, so packages do not collide there), declare them inline as \`defineStack({ docs })\`, or delete them if they are not package docs. Found: ${files.join(', ')}`; +} + +/** + * One artifact `packages[]` entry, as the docs collector needs to see it: the + * position to attach collected docs to, the namespace its docs are linted + * against (the ruling's clause 2), and the `src/` directory names that name it. + * + * ⛔ The id is NOT computed here. `artifactPackages` owns that rule + * (`manifest.id`, falling back to `name`, then to the positional spelling) and + * its own header forbids a second copy: two readers computing "which package is + * this" slightly differently is how one seam comes to judge a different set of + * packages than another while both look right. + */ +export interface DocsPackageRef { + /** Position in the artifact's `packages[]`. */ + readonly index: number; + /** The id the runtime registers this package under, from `artifactPackages`. */ + readonly id: string; + /** The package's own `namespace` — the prefix rule for the docs it owns. */ + readonly namespace?: string; + /** The `src/` names this package answers to — see {@link docsPackageRefs}. */ + readonly directoryNames: readonly string[]; +} + +/** Docs read out of ONE package's own `src//docs/` directory (ADR-0130 D4). */ +export interface PackageDocSet { + /** Position in the artifact's `packages[]` — where {@link attachPackageDocs} writes. */ + readonly index: number; + readonly id: string; + readonly namespace?: string; + /** Where they were read from, relative to the config file. */ + readonly dir: string; + readonly docs: DocItem[]; +} + +/** + * The artifact's `packages[]`, reduced to what a `src//docs/` directory can + * be matched against. + * + * Three spellings, and no more: the package's full `id`, the LAST dot-separated + * segment of that id, and its `name`. The middle one is the load-bearing case — + * `examples/app-multi-package` declares `id: 'com.example.multi.core'` with + * `name: 'Multi-Package Core'`, so a `src/core/` directory can only be resolved + * through the id's tail. ⛔ `namespace` is deliberately NOT a spelling: ADR-0130 + * D1 exists so that N packages of one artifact can SHARE one namespace, so + * matching on it would be ambiguous exactly where multi-package layouts are + * most common. + * + * A directory that matches none, or more than one, is not attributed — it is + * reported, by {@link sweepPackageDocsDirectories}. + */ +export function docsPackageRefs(packages: unknown): DocsPackageRef[] { + if (!Array.isArray(packages)) return []; + return artifactPackages({ packages }).map(({ index, id, body }) => { + const directoryNames = new Set(); + if (typeof body.id === 'string' && body.id !== '') { + directoryNames.add(body.id); + const tail = body.id.split('.').pop(); + if (tail) directoryNames.add(tail); + } + if (typeof body.name === 'string' && body.name !== '') directoryNames.add(body.name); + return { + index, + id, + ...(typeof body.namespace === 'string' && body.namespace !== '' ? { namespace: body.namespace } : {}), + directoryNames: [...directoryNames], + }; + }); +} + +/** + * Walk `src/*/docs/` once and split it two ways: collected into the package + * that owns it, or reported as unread (#18170's warning, kept by the #18431 + * ruling's clause 4). + * + * ⭐ ONE traversal, and it is the traversal that was already here. The warning + * this function grew out of already read every `src//docs/` and already + * listed its Markdown files by name; collecting them costs the file reads and + * nothing else. That is the measurement the ruling asked for — see the PR that + * landed this — and it is why the directory convention is the one implemented + * first. */ -function uncollectedDocsDirectories(srcDir: string): DocIssue[] { +function sweepPackageDocsDirectories( + srcDir: string, + refs: readonly DocsPackageRef[], +): { packageDocs: PackageDocSet[]; issues: DocIssue[] } { let entries: fs.Dirent[]; try { entries = fs.readdirSync(srcDir, { withFileTypes: true }); } catch { - return []; + return { packageDocs: [], issues: [] }; } const issues: DocIssue[] = []; + const packageDocs: PackageDocSet[] = []; const packageDirs = entries.filter((e) => e.isDirectory() && e.name !== COLLECTED_DOCS_DIR); packageDirs.sort((a, b) => a.name.localeCompare(b.name)); for (const entry of packageDirs) { - const files = markdownFilesIn(path.join(srcDir, entry.name, COLLECTED_DOCS_DIR)); + const dir = path.join(srcDir, entry.name, COLLECTED_DOCS_DIR); + const files = markdownFilesIn(dir); if (files.length === 0) continue; const rel = `src/${entry.name}/docs`; + + const owners = refs.filter((ref) => ref.directoryNames.includes(entry.name)); + if (owners.length === 1) { + const compiled = compileDocsDirectory(dir, rel); + issues.push(...compiled.issues); + packageDocs.push({ + index: owners[0].index, + id: owners[0].id, + ...(owners[0].namespace !== undefined ? { namespace: owners[0].namespace } : {}), + dir: rel, + docs: compiled.docs, + }); + continue; + } + + if (refs.length === 0) { + // No `packages[]` at all — the single-package shape, where "read from + // src/docs/ only" is still the whole truth. ⛔ Message unchanged. + issues.push({ + severity: 'warning', + rule: 'docs/uncollected-directory', + message: uncollectedDocsMessage(rel, files), + path: rel, + }); + continue; + } + + const declared = refs.map((ref) => ref.id).join(', '); issues.push({ severity: 'warning', rule: 'docs/uncollected-directory', - message: `${rel}/ holds ${files.length} Markdown file(s) that were NOT collected: package docs are read from src/docs/ only (ADR-0046 §3.2), so these are absent from the artifact's \`docs[]\` and from every book that includes them. Move them into src/docs/ (doc names carry the package namespace prefix, so packages do not collide there), declare them inline as \`defineStack({ docs })\`, or delete them if they are not package docs. Found: ${files.join(', ')}`, + message: owners.length === 0 + ? `${rel}/ holds ${files.length} Markdown file(s) that were NOT collected: "${entry.name}" names none of this artifact's packages, so there is no package body to attach them to (ADR-0130 D4). A per-package docs directory is matched against a package's \`id\`, the last dot-separated segment of that \`id\`, or its \`name\` — rename the directory to one of those, declare the docs inline as \`defineStack({ docs })\` on the package that owns them, or move them into src/docs/. Declared packages: ${declared}. Found: ${files.join(', ')}` + : `${rel}/ holds ${files.length} Markdown file(s) that were NOT collected: "${entry.name}" names ${owners.length} of this artifact's packages (${owners.map((o) => o.id).join(', ')}), so which package body owns these docs is ambiguous and ⛔ this collector will not guess (ADR-0130 D4). Give those packages distinct \`id\`/\`name\` spellings, or declare the docs inline as \`defineStack({ docs })\` on the one that owns them. Found: ${files.join(', ')}`, path: rel, }); } - return issues; + return { packageDocs, issues }; } /** - * Read `src/docs/*.md` (flat) next to the given config file and compile - * each file into a `DocItem`. Structural problems (subdirectories, bad - * filename stems) are reported as error issues; offending files are - * skipped rather than partially collected. + * Read one flat docs directory and compile each `.md` file into a `DocItem`. + * Structural problems (subdirectories, bad filename stems) are reported as + * error issues; offending files are skipped rather than partially collected. * - * Markdown docs sitting one level down, in `src//docs/`, are never - * collected — they are REPORTED instead (see {@link uncollectedDocsDirectories}), - * whether or not `src/docs/` itself exists, because either way the build keeps - * none of them. + * `relBase` is how the directory is NAMED in every issue this raises, relative + * to the config file — `src/docs` for the stack's own flat directory, and + * `src//docs` for a package's. It is a parameter rather than a constant + * because #18431 gave this reader a second caller; for the flat directory the + * text it produces is byte-identical to what it produced before. */ -export function collectDocsFromSrc(configPath: string): { docs: DocItem[]; issues: DocIssue[] } { - const srcDir = path.join(path.dirname(configPath), 'src'); - const docsDir = path.join(srcDir, COLLECTED_DOCS_DIR); - const issues: DocIssue[] = uncollectedDocsDirectories(srcDir); +function compileDocsDirectory(docsDir: string, relBase: string): { docs: DocItem[]; issues: DocIssue[] } { + const issues: DocIssue[] = []; if (!fs.existsSync(docsDir)) return { docs: [], issues }; const baseByName = new Map(); const variants: Array<{ base: string; locale: string; item: DocTranslationItem; rel: string }> = []; for (const entry of fs.readdirSync(docsDir, { withFileTypes: true })) { - const rel = `src/docs/${entry.name}`; + const rel = `${relBase}/${entry.name}`; if (entry.isDirectory()) { issues.push({ severity: 'error', rule: 'docs/flat-directory', - message: `Subdirectory "${entry.name}" under src/docs/ is not allowed (ADR-0046 §3.2). Flatten all .md files directly into src/docs/.`, + message: `Subdirectory "${entry.name}" under ${relBase}/ is not allowed (ADR-0046 §3.2). Flatten all .md files directly into ${relBase}/.`, path: rel, }); continue; @@ -414,6 +547,33 @@ export function collectDocsFromSrc(configPath: string): { docs: DocItem[]; issue return { docs: [...baseByName.values()], issues }; } +/** + * Read `src/docs/*.md` (flat) next to the given config file, and — when the + * caller hands over the artifact's `packages[]` — every `src//docs/` whose + * directory name resolves to one of those packages (#18431, ADR-0130 D4). + * + * The two results stay SEPARATE and that separation is the ruling: the flat + * directory's docs are the stack's own and keep attaching where they always + * did (`docs` below, which `compile.ts` writes to the artifact's top level), + * while a package's docs go to `packageDocs` and from there into that package's + * own body — ⛔ never to the top level. + * + * ⚠️ Called with ONE argument, this function behaves exactly as it always has: + * `packageDocs` is empty, every `src//docs/` is reported by the #18170 + * warning, and `docs`/`issues` are byte-for-byte what they were. That is what + * makes "nothing existing moves" checkable rather than promised — a + * single-package stack declares no `packages[]`, so it takes the same branch. + */ +export function collectDocsFromSrc( + configPath: string, + packages?: unknown, +): { docs: DocItem[]; issues: DocIssue[]; packageDocs: PackageDocSet[] } { + const srcDir = path.join(path.dirname(configPath), 'src'); + const sweep = sweepPackageDocsDirectories(srcDir, docsPackageRefs(packages)); + const flat = compileDocsDirectory(path.join(srcDir, COLLECTED_DOCS_DIR), `src/${COLLECTED_DOCS_DIR}`); + return { docs: flat.docs, issues: [...sweep.issues, ...flat.issues], packageDocs: sweep.packageDocs }; +} + /** * Content + naming lint over the package's full doc set (collected files * plus any inline `defineStack({ docs })` items). @@ -686,19 +846,191 @@ export function lintMetadataEmbeds(docs: DocItem[], stack: Record + value !== null && typeof value === 'object' && !Array.isArray(value) + ? Object.fromEntries( + Object.keys(value as Record).sort().map((k) => [k, (value as Record)[k]]), + ) + : value) ?? 'undefined'; +} + +/** A membership test over {@link docIdentity}, reference-first. */ +function claimedDocs(items: readonly DocItem[]): { has: (doc: DocItem) => boolean } { + const refs = new WeakSet(); + const keys = new Set(); + for (const item of items) { + if (item !== null && typeof item === 'object') refs.add(item as object); + keys.add(docIdentity(item)); + } + return { + has: (doc) => (doc !== null && typeof doc === 'object' && refs.has(doc as object)) || keys.has(docIdentity(doc)), + }; +} + +/** The `docs` a `packages[]` entry already carries in its own assembled body. */ +function bodyDocsOf(packages: unknown, index: number): DocItem[] { + if (!Array.isArray(packages)) return []; + const body = (packages[index] as { manifest?: Record } | null | undefined)?.manifest; + const docs = body?.docs; + return Array.isArray(docs) ? (docs as DocItem[]) : []; +} + +/** + * The ONE rule the per-package split cannot enforce inside a single package: + * a doc name declared by two different owners. + * + * Doc uniqueness is logical — the metadata registry key carries no package + * coordinate — so two owners declaring one name means one silently overwrites + * the other at registration. `lintDocs` catches the collision WITHIN a set; the + * moment the lint runs per package (the ruling's clause 2) nothing else is + * looking across them, and ADR-0130 D1 makes the cross-set case reachable on + * purpose: N packages of one artifact may share one namespace, so their + * prefixes do not keep them apart. + */ +function lintDocNamesAcrossOwners( + sets: ReadonlyArray<{ label: string; docs: readonly DocItem[] }>, +): DocIssue[] { + const owners = new Map(); + for (const set of sets) { + const seen = new Set(); + for (const doc of set.docs) { + if (typeof doc?.name !== 'string' || seen.has(doc.name)) continue; // within-set dupes are `lintDocs`' job + seen.add(doc.name); + const labels = owners.get(doc.name) ?? []; + labels.push(set.label); + owners.set(doc.name, labels); + } + } + const issues: DocIssue[] = []; + for (const [name, labels] of owners) { + if (labels.length < 2) continue; + issues.push({ + severity: 'error', + rule: 'docs/duplicate-name', + message: `Doc name "${name}" is declared by ${labels.join(' and ')}. Doc names are one flat namespace across the whole artifact (the metadata registry key carries no package coordinate), so one of these silently overwrites the other at registration — rename one.`, + path: `docs/${name}`, + }); + } + return issues; +} + +/** Re-locate a per-package issue so its `path` says which package it came from. */ +function underPackage(issues: readonly DocIssue[], index: number): DocIssue[] { + return issues.map((issue) => ({ ...issue, path: `packages[${index}].${issue.path}` })); +} + +/** + * One-call entry for `os build` / `os validate` / `os lint`: collect + * `src/docs/*.md` and every resolvable `src//docs/`, merge the flat set + * with the stack's inline `docs`, and lint each set against the namespace of + * the package that owns it. + * + * Returns the stack-level doc array (inline items first — they were already + * schema-validated), which is what the artifact's TOP LEVEL carries, plus the + * per-package sets {@link attachPackageDocs} writes into `packages[]`, plus * every issue found. + * + * ## The lint partition (#18431, the ruling's clause 2) + * + * A doc is linted ONCE, against the namespace of whoever owns it: + * + * - a doc a `packages[]` body carries (composition folded a package's inline + * `defineStack({ docs })` there), or one read out of that package's + * `src//docs/`, is linted against THAT package's `namespace`; + * - everything else — the stack's own flat `src/docs/` and any top-level + * inline doc no package claims — keeps `stack.manifest.namespace`. + * + * ⛔ There is no single global prefix any more, and no fallback between the two + * rules: a package doc that fails its own package's prefix is refused, never + * re-tried against the artifact's. `lintDocNamesAcrossOwners` is what replaces + * the one thing the single global set used to give for free. + * + * ⚠️ A stack with no `packages[]` — every single-package app — takes exactly + * the old path: nothing is claimed, so the stack-level set IS the whole set and + * the issue list is unchanged, item for item. */ export function collectAndLintDocs( configPath: string, stack: Record, -): { docs: DocItem[]; issues: DocIssue[] } { +): { docs: DocItem[]; issues: DocIssue[]; packageDocs: PackageDocSet[] } { const inline = Array.isArray(stack.docs) ? (stack.docs as DocItem[]) : []; - const collected = collectDocsFromSrc(configPath); + const collected = collectDocsFromSrc(configPath, stack.packages); const namespace = (stack.manifest as { namespace?: string } | undefined)?.namespace; const docs = [...inline, ...collected.docs]; - const issues = [...collected.issues, ...lintDocs(docs, namespace), ...lintMetadataEmbeds(docs, stack)]; - return { docs, issues }; + + const collectedByIndex = new Map(collected.packageDocs.map((set) => [set.index, set])); + const owned = docsPackageRefs(stack.packages).map((ref) => { + const body = bodyDocsOf(stack.packages, ref.index); + const fromDisk = collectedByIndex.get(ref.index)?.docs ?? []; + return { ref, body, fromDisk, all: [...body, ...fromDisk] }; + }).filter((entry) => entry.all.length > 0); + + // The top level keeps every doc it carried — the artifact shape does not + // move. What the ownership split changes is only which namespace each doc is + // JUDGED against, so the stack-level lint drops the ones a package claims. + const claimed = claimedDocs(owned.flatMap((entry) => entry.body)); + const stackScoped = docs.filter((doc) => !claimed.has(doc)); + + const issues: DocIssue[] = [ + ...collected.issues, + ...lintDocs(stackScoped, namespace), + ...lintMetadataEmbeds(docs, stack), + ]; + for (const entry of owned) { + issues.push(...underPackage(lintDocs(entry.all, entry.ref.namespace), entry.ref.index)); + // Only the docs read off disk need an embed pass here: a doc already on the + // body is also in `docs` above, where `lintMetadataEmbeds` has judged it. + issues.push(...underPackage(lintMetadataEmbeds(entry.fromDisk, stack), entry.ref.index)); + } + issues.push(...lintDocNamesAcrossOwners([ + { label: 'the stack itself', docs: stackScoped }, + ...owned.map((entry) => ({ label: `package "${entry.ref.id}"`, docs: entry.all })), + ])); + + return { docs, issues, packageDocs: collected.packageDocs }; +} + +/** + * Write each collected {@link PackageDocSet} onto the body of the package that + * owns it — `packages[i].manifest.docs`, the structural position ADR-0130 D4 + * reserves for a package body (#18431, the ruling's clause 1). + * + * ⛔ Not the artifact top level. The runtime reads a package-owned collection + * back UP through `resolveArtifactCollections` + * (`packages/runtime/src/artifact-collections.ts`), so a doc written here is + * served exactly as a top-level one is — while `packages[]` keeps the OWNERSHIP + * that ADR-0130 D1 is about and a flattened copy would destroy. + * + * Returns the ARGUMENT ITSELF when nothing is added, so an artifact with no + * per-package docs is not merely equal to the one built before this landed — + * it is the same object, serialized from the same references. + */ +export function attachPackageDocs(packages: unknown, sets: readonly PackageDocSet[]): unknown { + if (!Array.isArray(packages) || sets.length === 0) return packages; + const byIndex = new Map(sets.map((set) => [set.index, set])); + let changed = false; + const out = packages.map((entry, index) => { + const set = byIndex.get(index); + if (!set || set.docs.length === 0) return entry; + const body = (entry as { manifest?: Record } | null | undefined)?.manifest; + if (body === null || typeof body !== 'object') return entry; + const existing = Array.isArray(body.docs) ? (body.docs as DocItem[]) : []; + const claimed = claimedDocs(existing); + const added = set.docs.filter((doc) => !claimed.has(doc)); + if (added.length === 0) return entry; + changed = true; + return { ...(entry as Record), manifest: { ...body, docs: [...existing, ...added] } }; + }); + return changed ? out : packages; } From e82d0efa2d5270c39a5856ae4e6abee12547b0f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 06:17:16 +0000 Subject: [PATCH 2/7] test(cli): pin the per-package docs attachment end to end The artifact on disk is the only place "packages[i].manifest.docs, not the top level" is observable, and the multi-package fixture's namespaces differ from the artifact manifest's on purpose: under a single global prefix rule the build exits 1 instead. Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- .../build-package-docs-attachment.e2e.test.ts | 189 ++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 packages/cli/test/build-package-docs-attachment.e2e.test.ts diff --git a/packages/cli/test/build-package-docs-attachment.e2e.test.ts b/packages/cli/test/build-package-docs-attachment.e2e.test.ts new file mode 100644 index 00000000000..16f24c3a91e --- /dev/null +++ b/packages/cli/test/build-package-docs-attachment.e2e.test.ts @@ -0,0 +1,189 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #18431 end to end — `os build` reads `src//docs/` and writes those docs + * onto the OWNING package's body, through the real command, the real lint and + * the real writer. + * + * ## What would be green without this file + * + * The unit pins next to the collector + * (`src/utils/collect-docs.package-docs.test.ts`) judge the collector. They + * cannot see the two things only the command decides: + * + * - **where the docs land in the emitted JSON.** The ruling puts them at + * `packages[i].manifest.docs` and ⛔ not at the top level, and the artifact + * on disk is the only place that distinction is observable. + * - **that the build still exits 0.** The per-package lint runs with a + * DIFFERENT namespace than the artifact manifest's; if the stack-level pass + * also judged those docs, this build would exit 1 on + * `docs/namespace-prefix` — and every `toEqual` about the artifact would be + * reading a file the previous run wrote. + * + * ⚠️ Pedigree, not counts. Each fixture doc carries a MARKER string written into + * exactly one file on disk, and the assertions read the marker out of the + * artifact — a count of 1 is satisfiable by an echo of the flat `src/docs/` + * doc, which is the failure shape this package measured twice this month. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync, mkdirSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +interface Run { code: number; stdout: string; stderr: string } + +function runCli(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...args], + { cwd, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) }, + (err, stdout, stderr) => { + resolvePromise({ + code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +/** + * Two packages that do NOT share a namespace, written the way + * `composeStacks(…, { manifest: 'preserve' })` writes one: collections + * flattened to the top level AND assembled onto each package body. + * + * The namespaces differ on purpose. `orders` is `ord` while the artifact + * manifest is `pkgdocs`, so a single global prefix rule — the pre-ruling + * behaviour — refuses `ord_playbook` and the build exits 1. + */ +const CONFIG_MULTI = ` +const account = { + name: 'pkgdocs_account', label: 'Account', sharingModel: 'private', + fields: { name: { type: 'text', label: 'Name' } }, +}; +const order = { + name: 'ord_order', label: 'Order', sharingModel: 'private', + fields: { name: { type: 'text', label: 'Number' } }, +}; + +const coreManifest = { id: 'com.example.pkgdocs.core', name: 'core', version: '1.0.0', type: 'app', namespace: 'pkgdocs' }; +const ordersManifest = { + id: 'com.example.pkgdocs.orders', name: 'orders', version: '1.0.0', type: 'module', namespace: 'ord', + dependencies: { 'com.example.pkgdocs.core': '^1.0.0' }, +}; + +export default { + manifest: coreManifest, + objects: [account, order], + packages: [ + { manifest: { ...coreManifest, objects: [account] } }, + { manifest: { ...ordersManifest, objects: [order] } }, + ], +}; +`; + +/** A single-package project with a flat `src/docs/` — the shape that must not move. */ +const CONFIG_FLAT = ` +export default { + manifest: { id: 'com.example.flat', name: 'flat', version: '1.0.0', type: 'app', namespace: 'flat' }, + objects: [ + { name: 'flat_thing', label: 'Thing', sharingModel: 'private', fields: { title: { type: 'text', label: 'Title' } } }, + ], +}; +`; + +const MARKER_PKG = 'MARKER-package-doc-18431'; +const MARKER_FLAT = 'MARKER-flat-doc-18431'; + +interface Artifact { + docs?: Array<{ name: string; content?: string }>; + packages?: Array<{ manifest: { id: string; namespace?: string; docs?: Array<{ name: string; content?: string }> } }>; +} + +const readArtifact = (dir: string): Artifact => + JSON.parse(readFileSync(join(dir, 'dist', 'objectstack.json'), 'utf8')) as Artifact; + +const dirs = { multi: '', flat: '' }; +let root = ''; + +beforeAll(() => { + root = mkdtempSync(join(tmpdir(), 'os-pkg-docs-e2e-')); + + dirs.multi = join(root, 'multi'); + mkdirSync(join(dirs.multi, 'src', 'orders', 'docs'), { recursive: true }); + mkdirSync(join(dirs.multi, 'src', 'docs'), { recursive: true }); + writeFileSync(join(dirs.multi, 'objectstack.config.ts'), CONFIG_MULTI); + // Owned by `com.example.pkgdocs.orders` — directory name === manifest `name`, + // and its own namespace `ord` is what the name must be prefixed with. + writeFileSync( + join(dirs.multi, 'src', 'orders', 'docs', 'ord_playbook.md'), + `# Orders Playbook\n\n${MARKER_PKG}\n`, + ); + // The stack's own flat doc, which keeps `stack.manifest.namespace`. + writeFileSync(join(dirs.multi, 'src', 'docs', 'pkgdocs_index.md'), `# Index\n\n${MARKER_FLAT}\n`); + + dirs.flat = join(root, 'flat'); + mkdirSync(join(dirs.flat, 'src', 'docs'), { recursive: true }); + writeFileSync(join(dirs.flat, 'objectstack.config.ts'), CONFIG_FLAT); + writeFileSync(join(dirs.flat, 'src', 'docs', 'flat_index.md'), `# Flat Index\n\n${MARKER_FLAT}\n`); +}); + +afterAll(() => { + if (root) rmSync(root, { recursive: true, force: true }); +}); + +describe('[#18431] per-package docs reach the owning package body', () => { + it('attaches to packages[i].manifest.docs and ⛔ not to the artifact top level', async () => { + const run = await runCli(['build'], dirs.multi); + // Asserted first: a non-zero exit makes every claim below vacuous, and a + // build that refused `ord_playbook` under the artifact prefix is exactly + // the pre-ruling behaviour this case exists to distinguish from. + expect(run.code, `stdout:\n${run.stdout}\nstderr:\n${run.stderr}`).toBe(0); + + const artifact = readArtifact(dirs.multi); + const orders = artifact.packages?.find((p) => p.manifest.id === 'com.example.pkgdocs.orders'); + expect(orders, 'the orders package entry').toBeDefined(); + expect(orders!.manifest.docs?.map((d) => d.name)).toEqual(['ord_playbook']); + // Pedigree: the bytes came out of that one file, not out of the flat doc. + expect(orders!.manifest.docs?.[0].content).toContain(MARKER_PKG); + + // The core package was never given docs, and the top level carries only the + // stack's own flat doc — the ruling's clause 1, read off the emitted JSON. + const core = artifact.packages?.find((p) => p.manifest.id === 'com.example.pkgdocs.core'); + expect(core!.manifest.docs).toBeUndefined(); + expect(artifact.docs?.map((d) => d.name)).toEqual(['pkgdocs_index']); + expect(JSON.stringify(artifact.docs)).not.toContain(MARKER_PKG); + }, 120_000); + + it('reports the whole collection on the step line, package directories named', async () => { + const run = await runCli(['build'], dirs.multi); + expect(run.code, `stdout:\n${run.stdout}\nstderr:\n${run.stderr}`).toBe(0); + // 1 flat + 1 package doc. A count of 1 here would mean the package pass ran + // and the author was told nothing about it. + expect(run.stdout).toMatch(/Collecting package docs \(ADR-0046\)\.\.\.\s*2 collected \(1 from 1 package directory\)/); + // ⛔ And the #18428 warning must NOT fire for a directory that WAS read. + expect(run.stdout).not.toMatch(/docs\/uncollected-directory/); + }, 120_000); + + it('a single-package project keeps its flat docs at the top level, with no packages[] invented', async () => { + const run = await runCli(['build'], dirs.flat); + expect(run.code, `stdout:\n${run.stdout}\nstderr:\n${run.stderr}`).toBe(0); + + const artifact = readArtifact(dirs.flat); + expect(artifact.docs?.map((d) => d.name)).toEqual(['flat_index']); + expect(artifact.docs?.[0].content).toContain(MARKER_FLAT); + expect(artifact).not.toHaveProperty('packages'); + expect(run.stdout).toMatch(/Collecting package docs \(ADR-0046\)\.\.\.\s*1 collected(?! \()/); + }, 120_000); +}); From 0ad1ee816009e98d76780b4d833a795cdfb53e9c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 06:41:56 +0000 Subject: [PATCH 3/7] fix(cli): the sweep docblock no longer closes itself `src/*/docs/` inside a block comment carries `*/`, which ends the comment: tsc read the prose after it as code. Spelled `src//docs/` instead. Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- packages/cli/src/utils/collect-docs.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/utils/collect-docs.ts b/packages/cli/src/utils/collect-docs.ts index 6f5e6013277..08ffed6b7be 100644 --- a/packages/cli/src/utils/collect-docs.ts +++ b/packages/cli/src/utils/collect-docs.ts @@ -356,9 +356,9 @@ export function docsPackageRefs(packages: unknown): DocsPackageRef[] { } /** - * Walk `src/*/docs/` once and split it two ways: collected into the package - * that owns it, or reported as unread (#18170's warning, kept by the #18431 - * ruling's clause 4). + * Walk every `src//docs/` once and split the result two ways: collected + * into the package that owns it, or reported as unread (#18170's warning, kept + * by the #18431 ruling's clause 4). * * ⭐ ONE traversal, and it is the traversal that was already here. The warning * this function grew out of already read every `src//docs/` and already From 8eb87800264f77fd642d380940456f2660ac9df2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 06:43:08 +0000 Subject: [PATCH 4/7] test(cli): classify attachPackageDocs in the build/validate parity roster Artifact assembly, not a gate: it places docs collectAndLintDocs already collected and linted, judges nothing, and os validate emits no artifact to place them in. NOT_A_GATE under its own reason rather than BUILD_ONLY_GATES, whose rows assert a gate that cannot run read-only. Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- .../cli/test/validate-build-gate-parity.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/cli/test/validate-build-gate-parity.test.ts b/packages/cli/test/validate-build-gate-parity.test.ts index 6b77b8565b5..8b9e6295ffa 100644 --- a/packages/cli/test/validate-build-gate-parity.test.ts +++ b/packages/cli/test/validate-build-gate-parity.test.ts @@ -177,6 +177,19 @@ const NOT_A_GATE: Readonly> = { // to a gap: both commands read it to COUNT the packages for the step line. 'Reads the artifact\'s `packages[]` for a count both commands print; the pass that judges them is a gate above': ['artifactPackages'], + // [#18431] Artifact ASSEMBLY, and deliberately not a `BUILD_ONLY_GATES` row. + // That ledger's entries are gates that cannot run read-only (they rewrite a + // committed snapshot, or emit a sibling module); filing this one there would + // assert it judges something, and it judges nothing. It takes the docs + // `collectAndLintDocs` already collected AND ALREADY LINTED — a name in + // SHARED_NON_REGISTRY_GATES above, run by all three doors — and places them + // on the body of the package that owns them (ADR-0130 D4). It refuses + // nothing, reports nothing, and returns its own argument when it has nothing + // to place. ⛔ Nor is there a parity gap behind it: `os validate` emits no + // artifact, so there is no `packages[]` for it to place them in, and every + // issue the placement could ever raise was raised by the gate above it. + 'Artifact assembly — puts already-collected, already-linted content into the bundle `os build` writes; judges nothing': + ['attachPackageDocs'], 'Presentation — renders, formats or serialises a verdict something else reached; judges nothing': [ 'printHeader', From 414b3ca946e90b654b0db664244311c62fc87a69 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 08:32:40 +0000 Subject: [PATCH 5/7] fix(cli): same-prefix doc links resolve artifact-wide, not per package Contract review finding C2. lintDocs resolves a same-prefix link against the set it is handed; partitioning the doc set per package made that set one package, so under ADR-0130 D1 - where packages share one namespace - an ordinary link from one package's doc to another's became docs/broken-link and an artifact that built green stopped building. The ownership split answers who judges a NAME; it must not answer whether a sibling's doc EXISTS. Links now resolve against every name the artifact carries, the same scope lintMetadataEmbeds already used for the same docs. Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- .../18431-per-package-docs-collector.md | 12 +- .../utils/collect-docs.package-docs.test.ts | 108 ++++++++++++++++++ packages/cli/src/utils/collect-docs.ts | 70 ++++++++++-- 3 files changed, 181 insertions(+), 9 deletions(-) diff --git a/.changeset/18431-per-package-docs-collector.md b/.changeset/18431-per-package-docs-collector.md index 1d6cb189437..470739998c2 100644 --- a/.changeset/18431-per-package-docs-collector.md +++ b/.changeset/18431-per-package-docs-collector.md @@ -11,7 +11,17 @@ A module can now ship its own docs. Before this, ADR-0046 collection was anchore - **Where they attach**: to `packages[i]`, ⛔ never the artifact top level. The runtime already merges a package-owned collection back up for readers (`resolveArtifactCollections`, ADR-0130 D4), so a flattened copy would buy nothing and destroy the ownership D1 is about. - **Whose namespace the lint uses**: the owning package's. A doc outside any package keeps `stack.manifest.namespace`. A multi-package artifact therefore has **one prefix rule per package** and ⛔ no single global prefix — and ⛔ no fallback between the two: a package doc that fails its own package's prefix is refused, never re-tried against the artifact's. -What that costs, stated plainly: in a multi-package artifact whose packages declare namespaces different from the artifact manifest's, a doc owned by a package is now judged by the package's prefix. That shape could not ship docs at all before (the single global prefix refused it), which is why this lands as a widening; a doc that was named for the artifact's namespace while living inside a differently-namespaced package now asks to be renamed, and the refusal names the spelling. +**What it costs, stated as the whole of it.** Exactly ONE class of input that `os build` accepted before is refused now, and it is the direct consequence of the ruled prefix rule: in a multi-package artifact (only `composeStacks(…, { manifest: 'preserve' })` produces one) whose packages declare namespaces DIFFERENT from the artifact manifest's, a doc owned by such a package used to be judged by the artifact's prefix and is now judged by its own package's. + +``` +FROM packages[i] with namespace "sales" inside an artifact whose manifest.namespace is "crm" + shipping a doc named crm_orders_guide -> accepted before, REFUSED now +TO rename it to sales_orders_guide (and the file to sales_orders_guide.md) +``` + +The refusal is `docs/namespace-prefix`, an error, and it names that exact spelling. Nothing else that built green stops building: an artifact whose packages share one namespace — the ADR-0130 D1 shape, and the one `examples/app-multi-package` documents — sees no change at all, because the per-package rule and the artifact rule are then the same rule. In the other direction the same change is a widening, and the larger half: that package could not ship a doc under its OWN prefix at all before. + +⚠️ Same-prefix LINKS and metadata-embed references are deliberately NOT partitioned with the naming rule — both resolve across the whole artifact. A doc's prefix says who judges its NAME; a link asks whether the target EXISTS, and ADR-0130 D1 exists so that N packages may share a namespace and cross-link inside it. Partitioning links too would have turned an ordinary cross-package link into `docs/broken-link` and stopped an artifact that built green from building; that was caught by this card's contract review and is pinned in the unit tier. Also in this change: diff --git a/packages/cli/src/utils/collect-docs.package-docs.test.ts b/packages/cli/src/utils/collect-docs.package-docs.test.ts index 9736d687211..65ea1aed19e 100644 --- a/packages/cli/src/utils/collect-docs.package-docs.test.ts +++ b/packages/cli/src/utils/collect-docs.package-docs.test.ts @@ -293,6 +293,114 @@ describe('the doc lint reads the OWNING package namespace', () => { }); }); +// ── C2 — the ownership split must not answer the EXISTENCE question ───────── +// +// Raised by this card's contract review and pinned HERE, in the unit tier, so +// it runs on the pull request that would reintroduce it. The e2e file next door +// carries the `.e2e.` filename tier, which is the NIGHTLY run — a pin that only +// ever runs after the merge is not what catches this. +// +// The defect it holds shut: `lintDocs` resolves a same-prefix link against the +// set it is handed. Partitioning the doc set per package (clause 2) silently +// made that set ONE PACKAGE, so in the ADR-0130 D1 shape — N packages sharing +// one namespace, the shape this card calls the common one — an ordinary link +// from package A's doc to package B's doc became `docs/broken-link`, an ERROR, +// and an artifact that built green stopped building. That is an unauthorised, +// undisclosed narrowing of the accept set. +// +// ⚠️ Each direction is asserted with a TRUE-POSITIVE twin built from the same +// fixture. A run of the "no broken link" side alone is indistinguishable from +// deleting the rule, which is the one outcome that must not read as a pass. +describe('same-prefix links resolve ARTIFACT-WIDE, not per package (C2)', () => { + /** Two packages sharing one namespace — ADR-0130 D1's whole point. */ + const SHARED_NS = [ + pkg({ ...CORE, namespace: 'crm', name: 'core' }), + pkg({ ...ORDERS, namespace: 'crm', name: 'orders' }), + ]; + const sharedStack = (extra: Record = {}) => ({ + manifest: { ...CORE, namespace: 'crm' }, + packages: SHARED_NS, + ...extra, + }); + const brokenLinks = (issues: ReadonlyArray<{ rule: string }>) => + issues.filter((i) => i.rule === 'docs/broken-link'); + + it('a link across two packages sharing one namespace resolves, in both directions', () => { + // `core` links to a doc `orders` owns, and `orders` links back. + writePackageDoc('core', 'crm_core_guide', 'See [orders](./crm_orders_guide.md).'); + writePackageDoc('orders', 'crm_orders_guide', 'Back to [core](./crm_core_guide.md).'); + + const { issues, packageDocs } = collectAndLintDocs(configPath, sharedStack()); + + // Pedigree first: both directories really were read, so "no broken link" + // is not the silence of a pass that collected nothing. + expect(packageDocs.map((s) => [s.dir, s.docs.map((d) => d.name)])).toEqual([ + ['src/core/docs', ['crm_core_guide']], + ['src/orders/docs', ['crm_orders_guide']], + ]); + expect(brokenLinks(issues)).toEqual([]); + expect(issues.filter((i) => i.severity === 'error')).toEqual([]); + }); + + it('...while a target NO package provides is still an error — the control that can fail', () => { + writePackageDoc('core', 'crm_core_guide', 'See [ghost](./crm_ghost.md).'); + writePackageDoc('orders', 'crm_orders_guide', '# Orders'); + + const broken = brokenLinks(collectAndLintDocs(configPath, sharedStack()).issues); + expect(broken).toHaveLength(1); + expect(broken[0]).toMatchObject({ severity: 'error', path: 'packages[0].docs/crm_core_guide' }); + }); + + it('a flat src/docs/ doc links to a package-carried doc under the stack prefix, and back', () => { + const inlineDoc: DocItem = { name: 'crm_inline', content: 'Up to [index](./crm_index.md).' }; + writeFlatDoc('crm_index', '# Index\n\nDown to [inline](./crm_inline.md).'); + + const { issues } = collectAndLintDocs(configPath, sharedStack({ + docs: [inlineDoc], + packages: [SHARED_NS[0], pkg({ ...ORDERS, namespace: 'crm', name: 'orders', docs: [inlineDoc] })], + })); + + expect(brokenLinks(issues)).toEqual([]); + }); + + it('...and the same pair with the target removed is an error on both ends', () => { + // Same fixture, one half deleted: the flat doc's target is gone and the + // package doc's target is gone, so BOTH ends report. + writeFlatDoc('crm_index', '# Index\n\nDown to [inline](./crm_inline.md).'); + writePackageDoc('orders', 'crm_orders_guide', 'Up to [missing](./crm_missing.md).'); + + const broken = brokenLinks(collectAndLintDocs(configPath, sharedStack()).issues); + expect(broken.map((i) => i.path).sort()).toEqual(['docs/crm_index', 'packages[1].docs/crm_orders_guide']); + }); + + it('a target under ANOTHER package\'s prefix is still skipped as a cross-package link', () => { + // Unchanged behaviour, asserted so the widening above is not read as + // "every link is now checked": a different prefix is resolved at publish + // time against dependency docs, not here. + writePackageDoc('orders', 'sales_playbook', 'See [foreign](./other_thing.md).'); + const differentNs = [pkg({ ...CORE }), pkg({ ...ORDERS })]; // crm + sales + + const { issues } = collectAndLintDocs(configPath, { manifest: { ...CORE }, packages: differentNs }); + expect(brokenLinks(issues)).toEqual([]); + }); + + it('metadata embeds and links are partitioned the SAME way — artifact-wide', () => { + // The inconsistency the review named: embeds already resolved artifact-wide + // while links did not. One fixture, both halves, one verdict. + const FENCE = '```'; + const embed = [`${FENCE}metadata`, 'type: flow\nname: crm_onboard', FENCE].join('\n'); + writePackageDoc('core', 'crm_core_guide', `Link: [o](./crm_orders_guide.md)\n\n${embed}`); + writePackageDoc('orders', 'crm_orders_guide', '# Orders'); + + const { issues } = collectAndLintDocs(configPath, sharedStack({ + // `crm_onboard` is owned by the artifact, not by `core` — an embed has + // always resolved against the whole stack, and now a link does too. + flows: [{ name: 'crm_onboard' }], + })); + expect(issues.filter((i) => i.severity === 'error')).toEqual([]); + }); +}); + // ── "Nothing existing moves" — the single-package shape, item for item ───── describe('a stack with no packages[] is on the path it was always on', () => { it('produces the same docs and the same issues whether or not `packages` is passed', () => { diff --git a/packages/cli/src/utils/collect-docs.ts b/packages/cli/src/utils/collect-docs.ts index 08ffed6b7be..0dadcf76ecc 100644 --- a/packages/cli/src/utils/collect-docs.ts +++ b/packages/cli/src/utils/collect-docs.ts @@ -577,8 +577,28 @@ export function collectDocsFromSrc( /** * Content + naming lint over the package's full doc set (collected files * plus any inline `defineStack({ docs })` items). + * + * `resolvableNames` is the set a same-prefix LINK resolves against, and it is + * deliberately separate from `docs` (#18431 contract review, finding C2). The + * naming rules below judge one OWNER — that is the per-package prefix rule the + * ruling asked for — but a link is not a judgment about an owner: it asks + * whether the target EXISTS, and ADR-0130 D1 exists so that N packages of one + * artifact may SHARE a namespace. Resolved against one package's own names, a + * link from package A to package B's doc under the namespace they share reads + * as broken and an artifact that built green stops building. So the caller + * hands in every name the artifact carries and links resolve artifact-wide — + * the same scope `lintMetadataEmbeds` already uses for the same docs, so the + * two halves of one lint are partitioned the same way rather than two ways. + * + * ⚠️ Omitted, it falls back to this set's own names: the pre-#18431 behaviour, + * and exactly right for the caller that omits it — a stack with no `packages[]`, + * where the one set IS the artifact. */ -export function lintDocs(docs: DocItem[], namespace: string | undefined): DocIssue[] { +export function lintDocs( + docs: DocItem[], + namespace: string | undefined, + resolvableNames?: ReadonlySet, +): DocIssue[] { const issues: DocIssue[] = []; if (docs.length === 0) return issues; @@ -672,10 +692,16 @@ export function lintDocs(docs: DocItem[], namespace: string | undefined): DocIss } } - // Same-package link resolution: `[text](./.md#anchor)` where the - // target carries OUR namespace prefix must resolve to a doc in this - // package. Targets with a different prefix are cross-package links, - // verified at publish time against dependency docs. + // Same-prefix link resolution: `[text](./NAME.md#anchor)` where the target + // carries OUR namespace prefix must resolve to a doc THIS ARTIFACT carries. + // Targets with a different prefix are cross-package links, verified at + // publish time against dependency docs. + // + // [#18431] Resolved against `resolvableNames` — every name in the artifact — + // rather than against `names`, which is this owner's set alone. See the + // docblock: the two are the same set for a single-package stack, and they + // differ exactly where ADR-0130 D1 lets packages share one namespace. + const resolvable = resolvableNames ?? names; for (const doc of docs) { const linkRe = /\]\((?:\.\/)?([a-zA-Z0-9_.-]+\.md)(#[^)]*)?\)/g; const scannable = stripCode(doc.content); @@ -684,7 +710,7 @@ export function lintDocs(docs: DocItem[], namespace: string | undefined): DocIss const target = m[1].slice(0, -3); if (m[1].includes('/')) continue; // path-shaped link; flatness rule already errs on real subdirs if (namespace && !target.startsWith(`${namespace}_`)) continue; - if (!names.has(target)) { + if (!resolvable.has(target)) { issues.push({ severity: 'error', rule: 'docs/broken-link', @@ -956,6 +982,21 @@ function underPackage(issues: readonly DocIssue[], index: number): DocIssue[] { * re-tried against the artifact's. `lintDocNamesAcrossOwners` is what replaces * the one thing the single global set used to give for free. * + * ## What the partition deliberately does NOT reach + * + * ⛔ The OWNERSHIP question and the EXISTENCE question are not the same + * question, and only the first one is partitioned. Same-prefix link resolution + * and metadata-embed reference liveness both resolve across the WHOLE artifact: + * a doc's namespace prefix says who judges its NAME, while a link asks whether + * the target is there, and ADR-0130 D1 exists precisely so that N packages of + * one artifact may share a namespace and cross-link inside it. + * + * Partitioning links too would have turned an ordinary cross-package link into + * `docs/broken-link` and stopped an artifact that built green from building — + * an unauthorised narrowing, caught by this card's contract review as C2 and + * pinned by `a link across two packages sharing one namespace` in + * `collect-docs.package-docs.test.ts`. + * * ⚠️ A stack with no `packages[]` — every single-package app — takes exactly * the old path: nothing is claimed, so the stack-level set IS the whole set and * the issue list is unchanged, item for item. @@ -982,13 +1023,26 @@ export function collectAndLintDocs( const claimed = claimedDocs(owned.flatMap((entry) => entry.body)); const stackScoped = docs.filter((doc) => !claimed.has(doc)); + // [#18431 contract review, C2] Every name this artifact carries, whoever owns + // it — what a same-prefix LINK resolves against. The ownership split decides + // which namespace a doc is JUDGED by; it must not decide whether a sibling + // package's doc EXISTS, because ADR-0130 D1 is the case where two packages + // share the prefix and a cross-package link is ordinary. For a stack with no + // `packages[]` this set is exactly `docs`, so the single-package path is + // unmoved. + const artifactNames: ReadonlySet = new Set( + [...docs, ...owned.flatMap((entry) => entry.all)] + .map((doc) => doc?.name) + .filter((name): name is string => typeof name === 'string'), + ); + const issues: DocIssue[] = [ ...collected.issues, - ...lintDocs(stackScoped, namespace), + ...lintDocs(stackScoped, namespace, artifactNames), ...lintMetadataEmbeds(docs, stack), ]; for (const entry of owned) { - issues.push(...underPackage(lintDocs(entry.all, entry.ref.namespace), entry.ref.index)); + issues.push(...underPackage(lintDocs(entry.all, entry.ref.namespace, artifactNames), entry.ref.index)); // Only the docs read off disk need an embed pass here: a doc already on the // body is also in `docs` above, where `lintMetadataEmbeds` has judged it. issues.push(...underPackage(lintMetadataEmbeds(entry.fromDisk, stack), entry.ref.index)); From c9a33a08af9f813bed2bcd8e2360542cd5eb33fc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 08:36:16 +0000 Subject: [PATCH 6/7] docs(cli): state the true reason for the cross-owner duplicate doc rule The rule preserves the refusal the single flattened set already produced; it is authoring hygiene. It is NOT "one silently overwrites the other at registration" - ADR-0048 s3.3/s3.4 retired that: packaged items are stored under a composite packageId:name key and two distinct packages coexist on one bare name by construction. Behaviour unchanged; only the justification and the message a reader acts on. Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- .../18431-per-package-docs-collector.md | 2 +- packages/cli/src/utils/collect-docs.ts | 27 +++++++++++++------ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/.changeset/18431-per-package-docs-collector.md b/.changeset/18431-per-package-docs-collector.md index 470739998c2..6e219a768d8 100644 --- a/.changeset/18431-per-package-docs-collector.md +++ b/.changeset/18431-per-package-docs-collector.md @@ -26,7 +26,7 @@ The refusal is `docs/namespace-prefix`, an error, and it names that exact spelli Also in this change: - **The #18428 warning stays**, and now says *why* a directory was not read. Unchanged, word for word, for a stack that declares no `packages[]` — where "read from `src/docs/` only" is still the whole truth. For a directory that names **no** package it lists the declared packages and the three spellings a directory is matched against (`id`, the last dot-segment of `id`, `name`); for one that names **more than one** it names the candidates and refuses to guess. ⛔ `namespace` is not a matching spelling: ADR-0130 D1 exists so that N packages can share one, so matching on it would be ambiguous exactly where it matters. -- **A cross-owner duplicate doc name is an error.** Doc uniqueness is logical — the metadata registry key carries no package coordinate — so once the prefix rule runs per package, two packages sharing a namespace can declare one name and silently overwrite each other at registration. Nothing else was looking across the sets. +- **A cross-owner duplicate doc name stays an error.** It PRESERVES a refusal rather than adding one: before the split every doc reached the lint in one flattened array, so two owners declaring one name already raised `docs/duplicate-name`. Splitting the set per package would have dropped that silently, and ADR-0130 D1 lets packages of one artifact share a namespace, so the prefix does not keep them apart. The rule is authoring hygiene — ⛔ not a claim that one registration overwrites the other, which ADR-0048 §3.3/§3.4 retired. - **`os dev` mirrors `os build`.** The config-load path collects the same per-package directories onto the same bodies, so dev serves what a built artifact serves. - **The step line counts the whole collection**, package sets included, and says how many came from package directories — a build that read four package docs no longer announces `0 collected`. diff --git a/packages/cli/src/utils/collect-docs.ts b/packages/cli/src/utils/collect-docs.ts index 0dadcf76ecc..8ab7a060693 100644 --- a/packages/cli/src/utils/collect-docs.ts +++ b/packages/cli/src/utils/collect-docs.ts @@ -916,13 +916,24 @@ function bodyDocsOf(packages: unknown, index: number): DocItem[] { * The ONE rule the per-package split cannot enforce inside a single package: * a doc name declared by two different owners. * - * Doc uniqueness is logical — the metadata registry key carries no package - * coordinate — so two owners declaring one name means one silently overwrites - * the other at registration. `lintDocs` catches the collision WITHIN a set; the - * moment the lint runs per package (the ruling's clause 2) nothing else is - * looking across them, and ADR-0130 D1 makes the cross-set case reachable on - * purpose: N packages of one artifact may share one namespace, so their - * prefixes do not keep them apart. + * This rule PRESERVES a refusal, it does not add one. Before the split every + * doc reached `lintDocs` in ONE flattened array — the composed top level is the + * concat of every package's docs — so two owners declaring one name were two + * entries in one set and `docs/duplicate-name` already fired. Splitting the set + * per package (the ruling's clause 2) would have dropped that refusal silently, + * and ADR-0130 D1 makes the case reachable on purpose: N packages of one + * artifact may share one namespace, so their prefixes do not keep them apart. + * + * ⚠️ The REASON is authoring hygiene, and ⛔ deliberately not "one silently + * overwrites the other at registration". That sentence is this module's older + * framing and ADR-0048 retired it: packaged items are stored under a composite + * `:` key and resolution is package-scoped, so two distinct + * packages coexist on one bare name by construction (§3.3, §3.4 — "the + * cross-package throw is retired"). What survives there is exactly what this + * is: an authoring-time hygiene lint. ⚠️ That `os build` refuses the shape at + * all is a standing disagreement with ADR-0048 §3.4 which PREDATES this card + * and is filed rather than changed here — ⛔ relaxing a refusal that shipped is + * not a rider on a widening. */ function lintDocNamesAcrossOwners( sets: ReadonlyArray<{ label: string; docs: readonly DocItem[] }>, @@ -944,7 +955,7 @@ function lintDocNamesAcrossOwners( issues.push({ severity: 'error', rule: 'docs/duplicate-name', - message: `Doc name "${name}" is declared by ${labels.join(' and ')}. Doc names are one flat namespace across the whole artifact (the metadata registry key carries no package coordinate), so one of these silently overwrites the other at registration — rename one.`, + message: `Doc name "${name}" is declared by ${labels.join(' and ')}. One artifact may not ship one doc name twice — rename one. Doc names are namespace-prefixed for authoring hygiene, and ADR-0130 D1 lets packages of one artifact SHARE a namespace, so the prefix does not keep these apart.`, path: `docs/${name}`, }); } From 1ba9dec2ed92f9156047ded0ab71db5fdbe137ff Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 08:43:07 +0000 Subject: [PATCH 7/7] test(cli): type the C2 helper as DocIssue so tsc sees the whole record Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- packages/cli/src/utils/collect-docs.package-docs.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/utils/collect-docs.package-docs.test.ts b/packages/cli/src/utils/collect-docs.package-docs.test.ts index 65ea1aed19e..84585874c23 100644 --- a/packages/cli/src/utils/collect-docs.package-docs.test.ts +++ b/packages/cli/src/utils/collect-docs.package-docs.test.ts @@ -36,6 +36,7 @@ import { collectDocsFromSrc, docsPackageRefs, type DocItem, + type DocIssue, } from './collect-docs.js'; let tmp: string; @@ -322,8 +323,7 @@ describe('same-prefix links resolve ARTIFACT-WIDE, not per package (C2)', () => packages: SHARED_NS, ...extra, }); - const brokenLinks = (issues: ReadonlyArray<{ rule: string }>) => - issues.filter((i) => i.rule === 'docs/broken-link'); + const brokenLinks = (issues: readonly DocIssue[]) => issues.filter((i) => i.rule === 'docs/broken-link'); it('a link across two packages sharing one namespace resolves, in both directions', () => { // `core` links to a doc `orders` owns, and `orders` links back.