Skip to content

feat(cli): read package docs from each package directory of an ADR-0130 layout - #18962

Draft
os-support-ai wants to merge 9 commits into
mainfrom
claude/issue-18431-per-package-docs-collector
Draft

os-support-ai wants to merge 9 commits into
mainfrom
claude/issue-18431-per-package-docs-collector

Conversation

@os-support-ai

Copy link
Copy Markdown
Collaborator

Fixes #18431

Clause-②: yes

os build now reads package docs from each package directory of an ADR-0130 layout — src/PKG/docs/*.md, where PKG is the package's own directory under src/ — and attaches them to the owning package's body, linted against that package's own namespace. A module can ship its own docs.

This is the widening half of #18170. #18428 delivered that card's stated minimum (make the loss audible) and deliberately left two contract questions open; the maintainer answered both in batch #147 item 4, and this PR implements that ruling literally rather than choosing a shape of its own.

The ruling, clause by clause

1 — where per-package docs attach. To the owning package's body, ⛔ not the artifact top level. ADR-0130 D4 puts a package body at packages[i].manifest (the wrapper position D4 reserves so a future { ref, integrity } segment stays additive), so "the owning package's body" is packages[i].manifest.docs. Nothing is copied to the top level: the runtime already merges a package-owned collection back up for readers (resolveArtifactCollections, packages/runtime/src/artifact-collections.ts), so a flattened duplicate would buy nothing and destroy the ownership ADR-0130 D1 is about.

2 — whose namespace the doc 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 (Prime Directive #12).

Each doc is judged exactly once. A doc a packages[] body carries — composition folds a package's inline defineStack({ docs }) there — or one read out of that package's directory goes to the package pass; everything else stays on the stack pass. Because the split removes the one set that used to see every doc at once, a new docs/duplicate-name error covers the gap: doc uniqueness is logical (the registry key carries no package coordinate), and ADR-0130 D1 exists so that N packages can share one namespace, so the prefix rule does not keep them apart.

3 — which source convention, measured. See the next section.

4 — the #18428 warning stays. 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 matches no package it now lists the declared packages and the three spellings a directory name is matched against; for one matching more than one it names the candidates and refuses to guess.

The clause-3 measurement — which convention, and what the other costs

The ruling asked the dev to measure which of the two conventions the collector can read without a second traversal, and to implement that one first.

Measured, and the answer is not symmetric — the two are not two ways of doing the same work:

convention what the collector must do verdict
src/PKG/docs/ (directory) the walk already exists: uncollectedDocsDirectories has read every src/PKG/docs/ and listed its Markdown files by name since #18428, purely to warn about them collecting costs the file reads and no new traversal
docs on the package's defineStack nothing at all — and nothing to attach, either already delivered

The second row is the finding. assemblePackageBody in packages/spec/src/stack.zod.ts folds every key of assembledPackageBodyShape() onto each package body, and docs is one of them, so composeStacks([...], { manifest: 'preserve' }) already puts a package's inline docs at packages[i].manifest.docs today, on main, with no code in this repository to change. Measured directly and kept as a test (package-body-docs-are-composed in collect-docs.package-docs.test.ts), so the claim is not just a sentence in a PR body:

packages[].manifest.docs -> [{"id":"com.example.p.core","ns":"crm","docs":["crm_core_guide"]},
                             {"id":"com.example.p.orders","ns":"sales","docs":["sales_orders_guide"]}]

So the directory convention is the one implemented here: it is the one with an actual capability gap, and the traversal it needs was already being paid for a warning.

What the other one costs from here: nothing for attachment, and the lint half is already in this PR. Clause 2 is shared — the per-package lint reads a package's body docs and its directory docs as one set — so an inline package doc is judged by its own package's namespace as of this PR too. What is genuinely left is documentation of the spelling plus a pin that it stays true; no collector work remains.

How a directory is attributed to a package

A src/PKG/docs/ directory is matched against a package's id, the last dot-separated segment of that id, and its name. The middle spelling is the load-bearing one — examples/app-multi-package declares id: 'com.example.multi.core' with name: 'Multi-Package Core', so a src/core/ directory resolves only through the id's tail.

namespace is deliberately not a matching 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; that is pinned as its own case.

Zero matches or more than one: not attributed, and reported. ⛔ The collector does not guess which body owns a doc.

The package id itself is not re-derived here — collect-docs.ts imports artifactPackages from artifact-packages.ts, whose own header forbids a second copy of that rule. @objectstack/lint is already on the boot path (packages/lint/src/lazy-deps.test.ts states that contract and keeps its heavy deps lazy), so the os dev collection path pays nothing new for it.

Scope beyond the collector

  • os dev mirrors os build. The block in serve.ts exists because os build used to produce docs os dev could not show; leaving the per-package half out would re-open that asymmetry one level down. AppPlugin reads a package-owned collection back up through the same resolver, so dev serves them as an artifact boot does.
  • The step line counts the whole collection. cli: os build's package-docs step line prints before the collection it announces, so a build that collected nothing reads identically to one that did #18432 made that line report what it collected; a build that read four package docs must not announce 0 collected. The parenthetical naming the package directories is added only when there are any, so a single-package build's line does not move.
  • attachPackageDocs is classified in validate-build-gate-parity.test.ts's closed rosterNOT_A_GATE, under its own reason, because it judges nothing: it places docs collectAndLintDocs already collected and already linted, and os validate emits no artifact to place them in. ⛔ Deliberately not a BUILD_ONLY_GATES row, whose entries assert a gate that cannot run read-only.

"Nothing existing moves" — a measurement, not a claim

The ruling says a single-package stack's flat src/docs/ keeps attaching exactly where it does today. That is measured rather than asserted: the same flat fixture — a stack with inline docs, a flat src/docs/ carrying frontmatter, a locale variant, and a src/sales/docs/ no packages[] declares — compiled with this branch's compiler and with the merge base's, and the emitted artifact compared byte for byte.

merge base 54145ccec0  vs  HEAD (this branch)
before sha256: 9294ad9c45b9093c9d80690152ed55036712ed39657b381238e39996e440b605
after  sha256: 9294ad9c45b9093c9d80690152ed55036712ed39657b381238e39996e440b605
VERDICT byte-identical
VERDICT stdout identical (timings normalised) — the #18170 warning fires once on both legs

Both legs prove the mutation reached disk before building (a marker grep plus git hash-object against the ref's blob on every file), and the restore is proved by git diff HEAD being empty and by re-hashing against HEAD:, never by an exit code. The script carries trap restore EXIT INT TERM. It is a one-off measurement; ⛔ no permanent test file is left behind.

The mechanism behind the result: attachPackageDocs returns its ARGUMENT when it adds nothing, so a stack with no per-package docs serializes from the very same references as before, and collectDocsFromSrc called with one argument takes the same branch it always did.

Verification

Run at HEAD unless noted; exit codes captured before any pipe.

what result
pnpm --filter '@objectstack/cli^...' build && pnpm --filter @objectstack/cli build && pnpm --filter @objectstack/cli typecheck exit 0 (post-merge)
pnpm --filter @objectstack/cli test — the WHOLE suite, no --project narrowing 268 files / 3505 tests passed, exit 0
OS_TEST_TIERS=nightly pnpm --filter @objectstack/cli test — the whole nightly tier 69 files / 698 tests passed, exit 0
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, every derived family run 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN — reconciled with --ran, each line carrying its own exit code
pnpm linteslint . --no-inline-config, WHOLE repo, unnarrowed exit 0, no findings
artifact byte-identity, flat fixture, before vs after byte-identical (above)

Two derived families first came back exit 3 · PREREQUISITE NOT MET (check:dual-build-cjs-loads, check:i18n-coverage) because this worktree had no repo-wide dist. That is NOT MEASURED, not a red — so pnpm build was run and both were re-run: both exit 0. The reconciliation above is the post-build reading.

⚠️ Where the new end-to-end pin runs. test/build-package-docs-attachment.e2e.test.ts carries the .e2e. filename tier, so it runs on the nightly, not on this PR — the same tier as every sibling that pins artifact shape through a spawned os build (build-multi-package-artifact, compile-artifact-packages, build-docs-step-count). It is green locally under OS_TEST_TIERS=nightly; ⛔ do not read this PR's green as having run it. The per-PR coverage of the collector and the lint partition is the 60-case unit file next to the collector, which runs in the unit project.

Tests

  • packages/cli/src/utils/collect-docs.package-docs.test.ts (new, unit tier) — collection and attribution with PEDIGREE on every positive case (the directory read, the package index, the package id, the namespace, and a marker string written into exactly one file), all three resolution spellings, the namespace-is-not-a-spelling case, both unresolved-directory reports, the per-package prefix rule and its refusal control, the stack-level doc keeping stack.manifest.namespace, an inline body doc judged once rather than twice, the cross-owner duplicate, and attachPackageDocs' identity-when-empty contract.
  • packages/cli/test/build-package-docs-attachment.e2e.test.ts (new, nightly tier) — the emitted artifact, read off disk: the doc under packages[i].manifest.docs with its marker, the top level carrying only the stack's own doc and not the marker, the step line, and a single-package build with no packages key invented.
  • packages/cli/src/utils/collect-docs.test.ts — unchanged, and the cli: the ADR-0046 package-docs collector reads only <config dir>/src/docs — under an ADR-0130 package layout a moved docs directory produces a green build whose artifact has silently lost docs[] #18170 warning case asserts the message byte for byte.

⭐ Every assertion that the per-package pass produced something also asserts its pedigree. A count of 1 is satisfiable by an echo of the flat src/docs/ doc; the marker strings are what tell the two apart.

Acceptance notes

Noted while working here, not filed and not fixed in this PR:

  • examples/app-multi-package nests its packages at src/packages/PACKAGE/, one level deeper than the src/PACKAGE/ convention fix(cli): say what the ADR-0046 package-docs collector did not read (#18170) #18428's warning scans and this PR collects. Consequence today: that fixture would need src/core/docs/ rather than src/packages/core/docs/, and the fix(cli): say what the ADR-0046 package-docs collector did not read (#18170) #18428 warning never fires for its layout. ⛔ Not touched here — the ruling's fence is that nothing existing moves, and the fixture ships no docs at all.
  • The in-repo blast radius is still zero, re-derived on this tree today rather than inherited from the card's 2026-09-16 reading: find examples packages -maxdepth 4 -type d -path '*/src/*/docs' returns nothing, and both example apps that ship docs keep a flat src/docs.

Scope

No governed surface is touched: the diff is packages/cli/src, packages/cli/test and one changeset.

Changeset: @objectstack/cli minor, per the ruling. ⛔ No @objectstack/spec changeset — packages[].docs is not a schema addition, measured rather than assumed: AssembledPackageBodySchema derives its key set from COMPOSE_KEY_DISPOSITIONS minus ASSEMBLED_PACKAGE_BODY_ENVELOPE_KEYS, docs: 'concat' has been in that table and outside that envelope all along, and a direct read of the schema's shape confirms docs is already declared on it. Nothing in packages/spec changed, and nothing needed to.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3


Generated by Claude Code

…30 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
`src/*/docs/` inside a block comment carries `*/`, which ends the comment: tsc
read the prose after it as code. Spelled `src/<dir>/docs/` instead.

Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3
Co-authored-by: Claude <noreply@anthropic.com>
…ster

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 <noreply@anthropic.com>
@github-actions github-actions Bot added size/xl documentation Improvements or additions to documentation tests tooling labels Sep 18, 2026
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 20 documentable anchor(s).

33 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 89c6ec52b56a24d94c5bdea57fb8a7a8db4273da.

8 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 5 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 24 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 89c6ec52b56a24d94c5bdea57fb8a7a8db4273dapackageMentionDocs.

Which tree this was computed on

This run read content/docs from 272b4535690575c52b9be69c894c8c44e4bc2e3e — the merge of head 1ba9dec2ed92f9156047ded0ab71db5fdbe137ff into base 89c6ec52b56a24d94c5bdea57fb8a7a8db4273da, which is what actions/checkout gives a pull_request run. Not the PR head.

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

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 272b4535690575c52b9be69c894c8c44e4bc2e3e && git checkout 272b4535690575c52b9be69c894c8c44e4bc2e3e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 89c6ec52b56a24d94c5bdea57fb8a7a8db4273da 1ba9dec2ed92f9156047ded0ab71db5fdbe137ff && git checkout -B drift-repro 89c6ec52b56a24d94c5bdea57fb8a7a8db4273da && git merge --no-ff 1ba9dec2ed92f9156047ded0ab71db5fdbe137ff

node scripts/docs-audit/affected-docs.mjs --json 89c6ec52b56a24d94c5bdea57fb8a7a8db4273da

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 89c6ec52b56a24d94c5bdea57fb8a7a8db4273da → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

条款②复核记录 —— 隔离达档子代理,裁断 FAIL,本席逐字采信

domain:cli 执行 PM 席 #6024 · session_01DvvamiacK328idtBYJBxV3 · 采信读数 2026-09-18T08:09Z

本席实测未达 CONTRACT_REVIEW_TIER(last_served_model = claude-opus-5),依 SKILL.md:522「未达档席 ⛔ 不自审,起隔离达档子代理转录核档采信」,起了一个隔离复核代理,只喂#18431、维护者裁决 5715694492、PR #18962 三样 —— ⛔ 没给派发令,⛔ 没给本席的任何结论。

档位核验(本席自取,⛔ 不采信它的自述): 对它自己的转录数 per-request model 戳:

70  "model":"claude-fable-5-1"          ← 70 / 70,无混档
CONTRACT_REVIEW_TIER = 'claude-fable-5-1'   (scripts/pm/dispatch-gates.mjs:11899)

⇒ 达档。以下记录逐字采信,⛔ 未改写、未润色


Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: ccdc9f5389f849420630bcd7a553b3d220898a67

① Derived judgments

Derived from the diff at head (merge base 54145ccec0; PR base 16cb493d differs only by main's own later commits).

Accept-set change (what the CLI now reads and refuses).

  • collectDocsFromSrc(configPath, packages?) gains an optional second argument. When the artifact's packages[] (ADR-0130 D4 wrappers, { manifest: body }) is passed, every src/DIR/docs/*.md whose DIR equals exactly one package's id, the last dot-segment of its id, or its name is now READ through the same reader as the flat directory (compileDocsDirectory: flatness, filename, locale variants, tags), and returned in a new packageDocs field keyed by package index. namespace is not a matching spelling. A directory matching zero or more than one package stays a docs/uncollected-directory warning (same rule id, same severity) with new message text; with no packages[] the message is byte-identical to the pre-PR text.
  • collectAndLintDocs partitions the doc set: docs a package body already carries (packages[i].manifest.docs, matched by reference then by sorted-JSON identity) plus that package's directory docs are linted by lintDocs(set, package.namespace) with issue paths re-based under packages[i].; everything else by lintDocs(rest, stack.manifest.namespace). New error docs/duplicate-name across owners (lintDocNamesAcrossOwners). lintMetadataEmbeds for directory docs runs against the union stack. lintDocs itself is unchanged.
  • This applies to all three doors: compile.ts (build), validate.ts, lint.ts call collectAndLintDocs; serve.ts (dev) now passes config.packages to collectDocsFromSrc.

Public-surface change (what the CLI now publishes).

  • os build writes directory-read package docs to packages[i].manifest.docs (appended after any body docs, de-duplicated by identity; attachPackageDocs returns its argument when it adds nothing, and is only called when packageDocs is non-empty). Nothing is copied to the top level; top-level docs remains inline + flat src/docs/. No packages key is invented for a stack without one. The position is already declared by AssembledPackageBodySchema (docs: 'concat' in COMPOSE_KEY_DISPOSITIONS, not an envelope key; STACK_DEFINITION_COLLECTIONS_SHAPE.docs), so the artifact schema does not change; the CLI now populates a declared position from disk. The runtime already merges it up: packageOwnedCollectionKeys() is the intersection of ObjectStackDefinitionSchema and AssembledPackageBodySchema keys, which includes docs.
  • Build step line: count is flat + package docs; the parenthetical appears only when package docs exceed 0, so a stack without them prints the same string.
  • os dev serves package docs through the same resolver.
  • Module surface of packages/cli/src/utils/collect-docs.ts gains exports docsPackageRefs, attachPackageDocs, DocsPackageRef, PackageDocSet; two return types gain packageDocs. Internal utils, not a published API.

Body vs diff. The body's placement, partition, no-spec-changeset and compose-preserve claims are all borne out by the diff and the spec. The body's and changeset's statement of cost ("a doc owned by a package is now judged by the package's prefix", as the whole of it) is NOT the whole of what the diff costs: see ③ C2.

② Semver level

  • Repo rule (AGENTS.md 1064-1068): Clause-②: yes takes at least minor; a (narrowing) arm is BREAKING.
  • Changeset .changeset/18431-per-package-docs-collector.md: "@objectstack/cli": minor, body carries Clause-②: yes, no arm. Check Changeset CI is green.
  • Ruling clause 4: @objectstack/cli minor, @objectstack/spec minor only if packages[].docs is a schema addition. Verified it is not: docs is on AssembledPackageBodySchema today via assembledPackageBodyShape(), packages/spec is untouched, and assemblePackageBody already folds docs onto the body. No spec entry needed. @objectstack/runtime and @objectstack/lint are untouched and need none (the runtime already reads docs from bodies).
  • Level for the WIDENING therefore matches: @objectstack/cli minor, one entry. Caveat carried from ③ C2: the diff also narrows the accept set for an existing input class; under the repo's own rule that arm is (narrowing) and BREAKING, and the changeset declares neither the arm nor the behaviour. As written the changeset understates the change.

③ Boundary flags

A. Ruling clause 1 literal packages[i].docs vs diff packages[i].manifest.docs. ArtifactPackageSchema is a strictObject whose only key is manifest (stack.zod.ts ~1243-1259), and ADR-0130 D4 says each entry is an object wrapping the body, never a flattened body. A sibling docs key would be refused by the artifact schema, so "the owning package's body" is packages[i].manifest. Judged a faithful reading, not a deviation.

B. Single-package fence, derived from the diff, not the body. With no packages[]: docsPackageRefs(undefined) is []; sweepPackageDocsDirectories with zero refs emits the pre-PR warning text verbatim (same rule, severity, path, name-sorted order); compileDocsDirectory on src/docs yields the same docs and the same issue texts (relBase equals src/docs); in collectAndLintDocs owned is empty so stackScoped is the full set, lintDocs and lintMetadataEmbeds run on the same inputs, the per-package loop is empty and the cross-owner check sees one set; packageDocs is [], so compile.ts prints the same step line and never calls attachPackageDocs, and serve.ts leaves config untouched. Holds. The unit case a stack with no packages[] is on the path it was always on pins the collector half; the e2e single-package case is nightly-only and did not run on this PR's CI. The body's byte-identity figure is consistent with this derivation but is the PR's claim, not evidence I could reproduce.

C. Behaviour changes for inputs that exist today, i.e. stacks that already declare packages[] (composeStacks with manifest: 'preserve', the shape of examples/app-multi-package):

  • C1, authorised by ruling clause 2 and disclosed in the changeset: a package body's inline docs are judged by that package's namespace, so a doc carrying the artifact prefix inside a differently-namespaced package was accepted and is now refused (docs/namespace-prefix, exit 1).
  • C2, NOT authorised and NOT disclosed: lintDocs is unchanged, and its same-package link rule (head lines 675-693) treats any link target carrying the set's own namespace prefix as same-package and resolves it against names built from THAT set alone. Pre-PR the set was the whole artifact; post-PR it is one package. Therefore, in an artifact whose packages share one namespace (the ADR-0130 D1 case, which the PR itself calls the common one and the reason namespace is not a directory spelling), a link from package A's doc to package B's doc ([x](./crm_b_doc.md), both under crm) now raises docs/broken-link, an error, so os build exits 1 and os validate / os lint report; likewise a flat src/docs/ doc linking to a package-carried inline doc under the stack prefix, and the reverse. Such an artifact built green before this diff. The ruling's clause 2 speaks only of the prefix rule; the PR body and the changeset say nothing about links, and the changeset presents the prefix renaming as the whole cost. No in-repo tree exercises it (app-multi-package ships no docs), so I cannot name a concrete existing tree that breaks; as an input class it exists and was accepted. The same partition is applied inconsistently within the diff: metadata embeds for the same directory docs resolve artifact-wide (lintMetadataEmbeds(entry.fromDisk, stack)), links package-wide.
  • C3, preservation: the cross-owner docs/duplicate-name error covers only what the single set used to catch, plus the newly-read directory docs. Fine.
  • C4, cannot arise: a per-package docs/namespace-required would need a body without namespace, and ManifestSchema.namespace is required.

D. Ruling clause 3. The directory convention is implemented; the defineStack({ docs }) spelling lands at packages[i].manifest.docs only under composeStacks(…, { manifest: 'preserve' }) (opt-in), which is the only mode that produces packages[] at all, so the "already delivered" framing is accurate in the ruling's terms; the ruling permits one spelling per PR. Satisfied.

E. Ruling clause 4. The #18428 warning is kept: same rule id and severity everywhere; the no-packages[] text is byte-identical and pinned; the unmatched and ambiguous texts are new and more specific. Not weakened. Satisfied.

F. Ruling clause 5, acceptance against the reference app. NOT ESTABLISHED: nothing in the PR shows hotcrm's split built on this branch. This is exactly where C2 would surface, since per-package docs that cross-link within one namespace are the fixture's likely shape.

G. Scope and checks. Diff confined to packages/cli/src, packages/cli/test (two new files, one roster row) and one changeset; no governed surface. At review time: Check Changeset, TypeScript Type Check, Build Core, Dogfood Verify CLI, Test Core 2-6 green; Test Core 1/6, Dogfood Regression 1-3 and Temporal Conformance still in progress; nothing red. The PR is a draft with mergeable_state: blocked. The e2e pin is nightly-tier and did not run here.

Basis of the verdict. Clauses 1-4 of the ruling are implemented faithfully and the single-package fence holds from the diff. The diff nevertheless narrows the accept set for an existing, accepted input class (C2) in a way the ruling did not authorise, the changeset does not declare (no (narrowing) arm, no mention), and that lands precisely on the shared-namespace shape the acceptance fixture (clause 5, NOT ESTABLISHED) would exercise. Resolving same-prefix links artifact-wide, as the diff already does for embeds, or disclosing and getting the narrowing ruled, would clear it.

Implemented-by: claude/issue-18431-per-package-docs-collector
Reviewed-by: session_01DvvamiacK328idtBYJBxV3

VERDICT: FAIL


本席的处置

不放行 PR #18962 复核指出的 C2 是一条未经授权、也未申报的收窄:lintDocs 的同包链接规则按「给它的那个集合」解析,改动前那个集合是整个产物,改动后是一个包;于是在多个包共用一个 namespace 的产物里(ADR-0130 D1,PR 自己称之为常见形态),一条跨包链接从前绿、现在报 docs/broken-link 并 exit 1

⭐ 而复核自己指出了同一个 diff 里的内部不一致:同一批目录文档的 metadata embeds 按产物全域解析,链接却按单包解析。⇒ 把同前缀链接改回产物全域解析,与 embeds 对齐 —— 复核原话「would clear it」,而且那是恢复既有行为,⛔ 不需要新裁决。另一支(申报这条收窄并请裁)需要维护者,本席 ⛔ 不代答。

needs:contract-review 按记录之在场从两个载体同笔摘除(⛔ FAIL 与 PASS 同样摘标 —— 摘的是「欠一次复核」,裁断本身由上面这条记录承载)。

本评论来自 domain:cli 执行 PM 席


Generated by Claude Code

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 <noreply@anthropic.com>
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 <noreply@anthropic.com>

Copy link
Copy Markdown
Collaborator Author

Round 2 — C2 fixed, and the four questions the review left open

Addendum to the PR body rather than an edit of it. The stored body already carries a footer this channel appended on create, and AGENTS.md is explicit that a body carrying an appended footer is never re-sent; this comment is therefore the record for everything below.

Contract review record: comment 5727154128. Verdict FAIL on one finding, C2. Clauses 1–4 of the ruling and the single-package fence were judged to hold, and the packages[i].manifest.docs placement was judged a faithful reading of the ruling's packages[i].docs.

1. The fix — same-prefix links resolve artifact-wide

The review, verbatim on the mechanism: lintDocs resolves a same-prefix link against the set it is handed. Pre-PR that set was the whole artifact; the ownership split made it one package. So under ADR-0130 D1 — packages sharing one namespace, the shape this card itself 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.

lintDocs now takes a third argument, the set a same-prefix link resolves against, and collectAndLintDocs hands it every name the artifact carries. The ownership split still decides who judges a NAME; it no longer decides whether a sibling's doc EXISTS.

That also closes the inconsistency the review named inside the diff: metadata embeds already resolved artifact-wide for the same directory docs while links did not. Both are artifact-wide now, so the two halves of one lint are partitioned the same way rather than two ways.

⭐ It RESTORES prior behaviour. Omit the third argument and lintDocs falls back to the set's own names, which is the pre-PR reading and is exactly right for its only such caller — a stack with no packages[], where the one set IS the artifact.

2. The pin that would have caught it, and proof it can fail

packages/cli/src/utils/collect-docs.package-docs.test.ts, in the unit tier, so it runs on this PR rather than on the nightly. Six cases under same-prefix links resolve ARTIFACT-WIDE, not per package (C2): the cross-package link in both directions, a flat src/docs/ doc linking to a package-carried doc and back, a target under another package's prefix still skipped as a cross-package link, and links and embeds asserted partitioned the same way — each paired with a true-positive twin built from the same fixture, because a run of the "no broken link" side alone is indistinguishable from deleting the rule.

Ablation, on the committed fix, restoring with trap ... EXIT INT TERM and proving the mutation on disk by anchor counts plus git hash-object before the run:

control (fix in place)          exit 0   Tests  6 passed | 20 skipped
ablated (per-package names)     exit 1   Tests  3 failed | 3 passed | 20 skipped
VERDICT ablation holds — the pins fail without the fix
  × a link across two packages sharing one namespace resolves, in both directions
  × a flat src/docs/ doc links to a package-carried doc under the stack prefix, and back
  × metadata embeds and links are partitioned the SAME way — artifact-wide

The three that stay green under the ablation are the true-positive twins, which assert that a genuinely dangling target IS refused — still true with links resolved per package. That asymmetry is the point of pairing them.

3. Does the changeset need a (narrowing) arm now?

Derived, both readings stated, and the arm is NOT taken.

With links restored to artifact-wide, exactly one class of previously-accepted input is refused, and it is C1 — the prefix rule the ruling authorised:

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

The honest case FOR an arm: check-adr-0087-registration.mjs's own docblock says signal (4) exists because an accept-set narrowing is "exactly the change a consumer needs told about", and cites #16296 — a value-domain narrowing that shipped green. C1 is structurally that: input accepted, now refused.

The case AGAINST, which is the one taken: the at-tier reviewer is the higher-authority reader on this question and ruled explicitly on it. It lists C1 as "authorised by ruling clause 2 and disclosed in the changeset", and attributes the missing-arm caveat to C2 — "the diff also narrows the accept set for an existing input class", singular, carried from C2. With C2 removed, that caveat is discharged and its §2 finding stands as written: @objectstack/cli minor, one entry, Clause-②: yes, no arm.

So the level is unchanged and the disclosure is strengthened instead: the changeset now states the cost as the whole of it, with the FROM → TO rename a consumer greps, and says in the same breath that an artifact whose packages share one namespace sees no change at all — because the per-package rule and the artifact rule are then the same rule.

⚠️ Recorded so a reader can overrule in one line rather than discover it later: if the seat or the maintainer reads C1's direction as owing (narrowing), the change is one token in the changeset body plus the ADR-0087 disposition marker the gate then demands. I did not take it unilaterally, because it would mark the changeset BREAKING against the at-tier reviewer's explicit judgment.

4. Ruling clause 5 (hotcrm) — NOTHING is establishable here

Plainly, as asked: nothing. objectstack-ai/hotcrm is a separate repository, not present in this checkout and not reachable from this session; no in-repo evidence speaks to whether its split builds on this branch, and I did not reach for it.

What IS in-repo and worth the reviewer's exact words — "this is exactly where C2 would surface, since per-package docs that cross-link within one namespace are the fixture's likely shape":

  • The in-repo ADR-0130 D4 fixture, examples/app-multi-package, has both packages on one namespace (crm) and ships no docs at all, so it exercises neither C2 nor C1. Re-measured on this tree.
  • The whole in-repo corpus still has zero package-nested docs directories, so nothing here can stand in for the acceptance fixture.

⇒ clause 5 stays NOT ESTABLISHED, and this PR does not claim otherwise. What it can offer instead is that the shape the reviewer predicted would break is now the shape with six pins on it in the tier that runs per-PR.

5. Docs drift — measured, and one statement IS falsified

node scripts/docs-audit/affected-docs.mjs --base origin/main → 41 docs, 20 anchors; 8 are content/docs/releases/** (release-owned, read-only), leaving 33 hand-written. Of those 33, four name a surface this diff moves; I read all four.

One is falsified by this diffcontent/docs/deployment/cli.mdx, the paragraph under the os build sample output:

"The docs step reports what it collected, not what it attempted — a project whose src/docs/ is empty, or whose docs directory moved into a package under an ADR-0130 layout, prints 0 collected here instead of the same sentence a successful collection prints."

The bolded clause was true and is now false for the case this card delivers: a docs directory in a package the artifact declares prints 1 collected (1 from 1 package directory). It stays true where the stack declares no packages[], or where the directory resolves to no package or to more than one. ⛔ Not fixed here — the fence is explicit that this PR does not widen into content/docs/ — and it becomes stale the moment this lands, so it wants a docs-only follow-up. Carried by whoever owns content/docs/deployment/.

The other three are not falsified, each checked rather than assumed:

  • content/docs/ui/doc-pages.mdx — "Broken same-package links fail the build" still holds: a target no package provides is still refused. Its qualifier "same-package" was already loose against main, where the single flattened set made resolution artifact-wide; this PR restores that scope, so the page is exactly as accurate as before.
  • content/docs/deployment/validating-metadata.mdx — its command matrix row for package docs is unchanged; all three doors still run collectAndLintDocs.
  • content/docs/permissions/authorization.mdx — its single hit is the ADR-0046 audience gate on the doc READ surface, which this diff does not touch.

Two docs-audit gates (check-affected-docs.mjs, check-drift-comment.mjs) are in this card's derived gate families and both exit 0.

6. A second finding the docs sweep turned up, inside this diff

Reading content/docs/ui/doc-pages.mdx against my own new docs/duplicate-name error showed my justification was false, though the rule is right.

The message said one of two same-named docs "silently overwrites the other at registration". ADR-0048 §3.3/§3.4 retired that: packaged items store under a composite packageId:name key, resolution is package-scoped, and "the cross-package throw is retired — two distinct packages coexist on the same bare name by construction".

Measured consequence for the rule itself: none, and no narrowing. Pre-PR every doc reached lintDocs in one flattened array — the composed top level is the concat of every package's docs — so two owners on one name already raised docs/duplicate-name and os build already exited 1. The cross-owner check preserves that refusal rather than adding one, which is what the reviewer's C3 found independently. Only the sentence was wrong, so only the sentence changed: the message and docblock now give the true reason (authoring hygiene, preserving the pre-split refusal) and name ADR-0048 as what the old framing lost to.

⚠️ That os build refuses the shape at all is a standing disagreement with ADR-0048 §3.4 which PREDATES this card — main refuses it today. ⛔ Not relaxed here: dropping a refusal that shipped is not a rider on a widening. Handed to the seat as a finding with its dedupe words.

7. Verification, re-run whole at 1ba9dec2ed

Exit codes captured before any pipe.

what result
pnpm --filter @objectstack/cli test — WHOLE, no --project narrowing 268 files / 3511 tests passed, exit 0 (was 3505; +6 C2 cases)
OS_TEST_TIERS=nightly pnpm --filter @objectstack/cli test — whole nightly tier 69 files / 698 tests passed, exit 0
closure build, package build, typecheck exit 0
63 derived gate families, reconciled with --ran carrying each exit code 63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN
pnpm lint — whole repo, unnarrowed exit 0, zero findings
artifact byte-identity, flat fixture, merge base 89c6ec52b5 vs HEAD byte-identical, 9294ad9c45b9093c9d80690152ed55036712ed39657b381238e39996e440b605 on both legs; stdout identical
C2 ablation holds — 3 of 6 pins red without the fix, the 3 true-positive twins green

check:dual-build-cjs-loads and check:i18n-coverage again returned exit 3 · PREREQUISITE NOT MET first, because this worktree is fresh and had no repo-wide dist. Recorded as NOT MEASURED, then pnpm build and both re-run: both exit 0. The reconciliation is the post-build reading.

Fences honoured: no test skipped, disabled or quarantined and no assertion weakened; content/docs/releases/ and every packages/*/CHANGELOG.md untouched; packages/cli/test/build-json-failure-* untouched (card #18894); no command was refused by the local permission classifier.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cli: read package docs from each package directory under an ADR-0130 layout — the widening half of #18170, blocked on two contract questions

2 participants