Skip to content

Commit 0e671d2

Browse files
feat(cli): derive each package's docs directory from the registered packages (#19492)
Fixes #18965 Clause-②: yes `os build` now derives **each package's docs directory from the packages the artifact registers**, not from a fixed depth under `src/` — the maintainer's ruling, decision batch #204 item 5, letter **B** (relayed at comment 5754492000). A (declare the convention, rename the reference fixture) and C (scan two levels) were rejected there and are not implemented here. ## The first reading the ruling named: yes, `packages[]` is assembled before the collector runs The ruling made this the first thing to check, and said to report rather than restructure if it came out the other way. It comes out the right way, so nothing was restructured. Measured in `packages/cli/src/commands/compile.ts`: | step | line | what it does | | --- | --- | --- | | `loadConfig` | 242 | loads `objectstack.config.ts`, already composed — the fixture's own `composeStacks([...], { manifest: 'preserve' })` is what produces `packages[]` | | `ObjectStackDefinitionSchema.safeParse` | 347 | `result.data.packages` is the parsed, assembled array from here on | | `collectAndLintDocs(absolutePath, result.data)` | **778** | the collector, handed that same array | ⇒ the resolution point the ruling assumes is available at the collector, 431 lines after `packages[]` exists. `os validate` (`validate.ts:560`), `os lint` (`lint.ts:954`) and `os dev` (`serve.ts:2655`) reach the same seam, so all four doors move together. ## What was wrong `sweepPackageDocsDirectories` asked one question per **direct child** of `src/`: does `src/CHILD/docs/` hold Markdown? #18962 added *ownership* to that loop (match the directory name against the artifact's packages) but not *resolution* — the one fixed level survived. So a project whose packages sit one level deeper, which is the shape this repo's own ADR-0130 D4 reference fixture `examples/app-multi-package` has, was invisible. **The card's repro, run through the real `os build` binary, before and after.** The "before" leg is an ablation of this branch back to the one-level walk, with `packages/cli` rebuilt and `ablation-dist-preflight` proving the mutation reached the `dist/` the bin actually loads: ```text BEFORE (ablated to the one-level walk; dist preflight: marker absent from all 532 built files) $ objectstack build # examples/app-multi-package + src/packages/orders/docs/crm_ord_guide.md -> Collecting package docs (ADR-0046)... 0 collected exit 0 packages[].docs -> [["com.example.multi.orders",[]],["com.example.multi.core",[]]] ... and no docs/uncollected-directory warning either: the sweep never looked there. AFTER (this branch; dist preflight: marker present in packages/cli/dist/utils/collect-docs.js) $ objectstack build -> Collecting package docs (ADR-0046)... 1 collected (1 from 1 package directory) exit 0 packages[].docs -> [["com.example.multi.orders",["crm_ord_guide"]],["com.example.multi.core",[]]] top-level docs -> [] # still the package body, never the top level (#18431 clause 1) marker in content -> true ``` Both legs restored the tree: `ablation-replace` reported `blob == HEAD` with `git diff HEAD` empty, and a whole-tree `git status --porcelain` read 0 lines afterwards. ## How the directory is found, and why no second depth is pinned A registered package carries **no source path**. `ArtifactPackageSchema` is a `strictObject` whose only key is `manifest`, and that body is `AssembledPackageBodySchema` — `ManifestSchema` plus the collection keys. Neither declares the directory the package was authored in. So the only thing that can locate a package on disk is its **name**, and the two spellings a docs directory is matched against are unchanged: the package's `id`, and the last dot-separated segment of that `id`. ⛔ Never `name` (a display string, free to be re-worded); ⛔ never `namespace` (ADR-0130 D1 exists so N packages may share one). The walk therefore carries no number at all. Two properties are the whole design: 1. **The recursion exists only to find a registered package**, and stops at the first directory that names one. A package's own subtree is that package's source, not more packages, so a `docs/` deeper inside a resolved package is not a second docs directory. 2. **A directory whose name names no package is never resolved**, at any depth — so the walk can only ever add directories a package claims. **The preserved fence, held by construction rather than by a branch guarding it.** With no `packages[]` there is nothing to search for, so there is no descent at all and a single-package stack is walked exactly one level, as it always was. The ablation measures this rather than asserting it: of the eight new cases, the ablation turned **seven red and left the single-package one green** — that case never depended on the recursion. All 73 pre-existing cases in the two files stayed green under the same ablation, including the pre-#18965 flat-layout pin (the ⭐ lit control) and the byte-exact single-package warning-text pin. ## The two decisions the dispatch asked me to make and justify **1 — the `src/docs/` only sentence at `collect-docs.ts` `uncollectedDocsMessage`: left byte-identical, deliberately.** Read with its own docblock in front of me, as asked. That string is emitted from exactly one branch — `refs.length === 0`, a stack that declares no `packages[]` — and for that stack letter B resolves no package directory at all, so `src/docs/` really is the only place its docs are read from. The sentence is not false for any stack that can reach it. A stack **with** packages gets the other pair of messages, which name the packages the directory was matched against and claim no fixed path. The decision is recorded in the docblock so the next reader does not re-litigate it. This is also what pin 3 requires: the single-package warning is byte-for-byte what it was, and a pre-existing test asserts the exact sentence. **2 — the two-level layout is built in the test's own fixture, ⛔ not added to `examples/app-multi-package`.** The reference fixture has no `docs/` directory and never had one (7 files at `32b5831c4e`; `git log --diff-filter=AD` over its docs paths returns nothing; lit control: `examples/` holds 256 files, 11 under a `/docs/` path, so the probe can see docs directories there). Two reasons, both recorded in the test block's docblock: - Several measurements quote that fixture's contents exactly — `artifact-packages.ts` sizes the per-package de-duplication residue on it, `build-json-advisory-parity.e2e.test.ts` reads its artifact — so giving it docs changes what all of them read, to buy what the unit cases already prove with per-file marker strings. - ⛔ And I did **not** pin that fixture's on-disk shape from the test either. Such a pin fails the day someone flattens the fixture to `src/PKG/`, which after this card is harmless in both directions — it would pin a property this fix deliberately stops being load-bearing, so it could only ever produce false red. The fixture's measured layout is recorded in the test docblock as the reading the synthetic layout reproduces. The repro above is the compensating evidence: it runs the real `os build` against the real fixture. ## One new refusal, and why it is a refusal Depth-free resolution makes a new ambiguity reachable: one package answering to **two** doc-bearing directories (`src/core/docs` and `src/packages/core/docs` in one tree). Both are reported and neither is collected — the same answer this collector already gives when one directory names two packages. ⛔ It is not merged and ⛔ not silently halved: `attachPackageDocs` keys its sets by package index through a `Map`, so collecting both would drop one without a word — this card's own defect, re-created one layer up. ## Pins | # | pin | where | | --- | --- | --- | | 1 | two-level `src/packages/PKG/docs/` collected and attributed to the right package, with pedigree | `collects the ADR-0130 D4 two-level layout…` | | 2 | ⭐ flat `src/PKG/docs/` collected exactly as today, beside a two-level one, each with its own marker | `⭐ lit control: the flat layout is collected exactly as before…` + the pre-existing #18431 flat pins | | 3 | ⛔ no `packages[]` ⇒ walked one level, nothing deeper reported; warning text byte-exact | `⛔ single-package regression…` + the pre-existing exact-sentence pin | | 4 | a directory matching no package, and one matching more than one, keep their distinct answers at the new depth | `a directory naming NO package…`, `an AMBIGUOUS directory name…` | | + | one package, two docs directories: refused, never merged, never dropped | `⛔ ONE package answering to TWO docs directories…` | | + | the walk stops at a package root | `⛔ stops at the package root…` | ## Verification | what | result | | --- | --- | | `pnpm --filter @objectstack/cli exec vitest run --project unit` | **222 files / 3141 tests, 0 failed** | | `pnpm --filter @objectstack/cli typecheck` | exit 0 (`tsc --noEmit` + `check:test-typecheck`; debt ledger unmoved: 3 files / 28 errors / 6 pinned signatures) | | `pnpm --filter '@objectstack/cli^...' build` | exit 0 | | reverse verification (ablation, unit) | 7 of 8 new cases red, the single-package one green by construction, 73 pre-existing green; restore proved `blob == HEAD`, `git diff HEAD` empty | | reverse verification (ablation, e2e through the bin) | dist preflight both ways; `0 collected` before, `1 collected` after; whole-tree porcelain 0 lines after | | `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, every derived family run | **61 derived, 61 run, 0 NOT-MEASURED, 0 UNRUN** — reconciled with `--ran`, each line carrying its own exit code | | `pnpm lint` (`eslint . --no-inline-config`, the whole repo — ⛔ not a narrowing) | exit 0 in 85s, at `4afa1e4669` | Four derived families first answered `exit 3` — `check:dual-build-cjs-loads`, `check:i18n`, `check:i18n-coverage`, `check:i18n-walk-parity` — every one of them **PREREQUISITE NOT MET**, i.e. NOT MEASURED, never a finding. All four were re-run to `exit 0` once the workspace was built, and the reconciliation above carries those codes. `packages/cli` `integration` tier is declared to CI: this diff touches no integration-layer file, no spawn entry point (`bin/`, `test/helpers/serve-process.ts`) and no driver/kernel startup path. ## Changeset `@objectstack/cli` **minor**, measured rather than assumed. `packages/cli` is a published package and `src/utils/collect-docs.ts` ships inside it, so `skip-changeset` is refused; and `Clause-②: yes` takes at least `minor`. ⛔ No `@objectstack/spec` changeset: nothing in `packages/spec` changed and nothing needed to — `packages[].manifest.docs` was already declared, which is what #18962 measured. **Why `Clause-②: yes`, stated here rather than inherited.** Clause ② is directional: widening the accept set triggers it, pulling code back to the declared contract does not. `os build` now accepts a source layout it previously read nothing from, so what an author may write and have collected grows. ⛔ Nothing narrows — every tree that built green still builds green, with the same `docs[]` and the same warnings. The same declaration, on the same collector, is the precedent: PR #18962 (card #18431) landed `Clause-②: yes` with a `@objectstack/cli` minor. ## Acceptance notes - **The module header's "Absence" bullet was already stale before this card**, and is corrected here because this change rewrites that exact sentence. It read "a `src/PKG/docs/` directory one level down is NEVER collected" — false since `df0c856e01` (#18962) made such a directory collectable when it names a package. ⛔ Not filed: it is the docblock of the function this PR changes, and leaving it while restating the contract beside it is not an option. - **Markdown under a `docs/` nested deeper INSIDE a resolved package** (`src/packages/orders/components/docs/x.md`) is still not collected and still not warned about. Unchanged in both directions — the one-level sweep never reached it either — and it is the convention working as designed rather than a defect: the package's docs directory is the one at its root. Pinned as `⛔ stops at the package root…` so the boundary is a decision on the record. Noted, not filed; ⛔ carrier: none — no queued PR or person touches this path, and no layout in this repo has that shape. - No governed surface is touched: the diff is `packages/cli/src` and one changeset. ⛔ No `docs/adr/**`, `.claude/**`, `skills/**`, `AGENTS.md`, `CLAUDE.md` or `docs/NORTH-STAR.md`. - ⛔ No labels were written by this branch. The dispatch permitted exactly one, `skip-changeset`, and only if my own measurement refused a changeset; it did not. --- _Generated by [Claude Code](https://claude.ai/code/session_01QCdUBjM47SxioST9z5Zwdf)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ef256e6 commit 0e671d2

3 files changed

Lines changed: 327 additions & 36 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
**Clause-②: yes** — `os build` accepts a source layout it previously read nothing from, so what an author may write and have collected widens. ⛔ Nothing narrows: every tree that built green still builds green, with the same `docs[]` and the same warnings.
6+
7+
`os build` now derives **each package's docs directory from the packages the artifact registers**, not from a fixed depth under `src/` (maintainer ruling, decision batch #204 item 5, letter B).
8+
9+
Before this, the sweep asked one question per direct child of `src/`: does `src/CHILD/docs/` hold Markdown? So a project whose packages sit one level deeper — the shape this repo's own ADR-0130 D4 reference fixture `examples/app-multi-package` has, `src/packages/PKG/` — was invisible to it. A doc at `src/packages/orders/docs/ord_guide.md` was dropped **silently**: no `docs[]` entry, exit 0, and not even the `docs/uncollected-directory` warning, because the sweep never looked there. That is the #18170 defect verbatim, one level down, and after #18431 it was out of reach of both the diagnostic and the collection.
10+
11+
Both layouts are now one case rather than two:
12+
13+
```
14+
src/orders/docs/sales_guide.md -> packages[].manifest.docs (unchanged)
15+
src/packages/orders/docs/sales_guide.md -> packages[].manifest.docs (new)
16+
```
17+
18+
**How the directory is found.** A registered package carries no source path — `ArtifactPackageSchema` is a `strictObject` whose only key is the assembled body — so the only thing that can locate one on disk is its NAME, and the two spellings a docs directory is matched against are unchanged: the package's `id`, and the last dot-separated segment of that `id`. ⛔ Never `name` (a display string, free to be re-worded) and ⛔ never `namespace` (ADR-0130 D1 exists so N packages may share one).
19+
20+
**No second depth was pinned.** The walk descends only in SEARCH of a registered package and stops at the first directory that names one — so a package's own subtree stays its source, and a `docs/` inside it is not a second docs directory. With no `packages[]` there is nothing to search for, so there is no descent at all: a single-package stack is walked exactly one level, its `docs[]` and its warning text byte-for-byte what they were. That is the fence the ruling preserved from batch #147 item 4, held by construction rather than by a branch guarding it.
21+
22+
**One new refusal.** Depth-free resolution makes one package able to answer to two doc-bearing directories (`src/core/docs` and `src/packages/core/docs` in one tree). Both are reported and neither is collected — the same answer this collector already gives when one directory names two packages. ⛔ It is not merged and ⛔ not silently halved: docs attach by package index, so collecting both would drop one without a word.
23+
24+
A directory that matches **no** package and one that matches **more than one** keep their existing, distinct diagnostics, now at whatever depth they are found.

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

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,154 @@ describe('the #18170 warning stays for a directory neither convention reads', ()
258258
});
259259
});
260260

261+
// ── #18965: the docs directory resolves PER REGISTERED PACKAGE, at no depth ─
262+
/**
263+
* The maintainer's ruling (batch #204 item 5, letter B): `os build` derives
264+
* each package's docs directory from the packages the artifact REGISTERS, not
265+
* from a fixed depth under `src/`. A (declare `src/<pkg>/docs/` the convention
266+
* and rename the reference fixture) and C (scan two levels) were rejected.
267+
*
268+
* ## Why the two-level layout is built HERE and not in the reference fixture
269+
*
270+
* The ruling names "one test per layout … and `examples/app-multi-package`'s".
271+
* Measured at `32b5831c4e`, that fixture holds exactly 7 files and no `docs/`
272+
* directory anywhere, and `git log --diff-filter=AD` over
273+
* `examples/app-multi-package/**\/docs/**` returns nothing — it never had one.
274+
* ⭐ Lit control on the same probe: `examples/` holds 256 files, 11 of them
275+
* under a `/docs/` path, so the probe can see docs directories there.
276+
*
277+
* So "one test per layout" needed a choice, and the layout — not the fixture
278+
* file — is what this collector resolves against:
279+
*
280+
* - ⛔ NOT by adding a docs directory to `examples/app-multi-package`. It is
281+
* the ADR-0130 D4 reference fixture and several measurements quote its
282+
* contents exactly (`artifact-packages.ts` sizes the per-package
283+
* de-duplication residue on it; `build-json-advisory-parity.e2e.test.ts`
284+
* reads its artifact), so giving it docs changes what all of them read to
285+
* buy what the cases below already prove.
286+
* - ⛔ NOR by pinning that fixture's on-disk SHAPE from here. Such a pin
287+
* fails the day someone flattens the fixture to `src/<pkg>/` — a change
288+
* that, after this card, is harmless in both directions. Pinning a
289+
* property this fix deliberately stops being load-bearing would be a pin
290+
* that only ever produces false red.
291+
*
292+
* ⇒ the layout is reproduced here, where a marker string proves the pedigree
293+
* of every collected doc, and the reading above is the record that it is the
294+
* reference fixture's layout being reproduced.
295+
*/
296+
describe('the docs directory is derived from the packages the artifact registers', () => {
297+
it('collects the ADR-0130 D4 two-level layout and attributes it to the right package', () => {
298+
const marker = writePackageDoc(path.join('packages', 'orders'), 'sales_playbook', 'MARKER-two-level');
299+
const { docs, issues, packageDocs } = collectDocsFromSrc(configPath, stack().packages);
300+
301+
expect(issues).toEqual([]);
302+
expect(docs).toEqual([]); // ⛔ still not the top level — the #18431 ruling's clause 1
303+
expect(packageDocs).toHaveLength(1);
304+
expect(packageDocs[0].index).toBe(1);
305+
expect(packageDocs[0].id).toBe('com.example.multi.orders');
306+
expect(packageDocs[0].namespace).toBe('sales');
307+
expect(packageDocs[0].dir).toBe('src/packages/orders/docs');
308+
expect(packageDocs[0].docs.map((d) => d.name)).toEqual(['sales_playbook']);
309+
expect(packageDocs[0].docs[0].content).toContain(marker);
310+
});
311+
312+
it('⭐ lit control: the flat layout is collected exactly as before, beside a two-level one', () => {
313+
const flat = writePackageDoc('core', 'crm_core_guide', 'MARKER-flat-control');
314+
const nested = writePackageDoc(path.join('packages', 'orders'), 'sales_playbook', 'MARKER-nested');
315+
const { packageDocs, issues } = collectDocsFromSrc(configPath, stack().packages);
316+
317+
expect(issues).toEqual([]);
318+
expect(packageDocs.map((set) => [set.id, set.dir, set.docs.map((d) => d.name)])).toEqual([
319+
['com.example.multi.core', 'src/core/docs', ['crm_core_guide']],
320+
['com.example.multi.orders', 'src/packages/orders/docs', ['sales_playbook']],
321+
]);
322+
// ⚠️ Pedigree, not count: without these the pair above is satisfied by a
323+
// collector that read one directory twice.
324+
expect(packageDocs[0].docs[0].content).toContain(flat);
325+
expect(packageDocs[1].docs[0].content).toContain(nested);
326+
});
327+
328+
it('lints each two-level package against its OWN namespace, end to end', () => {
329+
writePackageDoc(path.join('packages', 'core'), 'crm_core_guide', 'MARKER-core');
330+
writePackageDoc(path.join('packages', 'orders'), 'sales_playbook', 'MARKER-orders');
331+
const { issues, packageDocs } = collectAndLintDocs(configPath, stack());
332+
333+
expect(issues).toEqual([]);
334+
expect(packageDocs.map((set) => [set.index, set.dir])).toEqual([
335+
[0, 'src/packages/core/docs'],
336+
[1, 'src/packages/orders/docs'],
337+
]);
338+
});
339+
340+
it('⛔ stops at the package root — a `docs/` deeper inside a resolved package is not a second one', () => {
341+
writePackageDoc(path.join('packages', 'orders'), 'sales_playbook', 'MARKER-root');
342+
const inside = path.join(tmp, 'src', 'packages', 'orders', 'components', 'docs');
343+
fs.mkdirSync(inside, { recursive: true });
344+
fs.writeFileSync(path.join(inside, 'sales_widget.md'), '# widget');
345+
346+
const { packageDocs, issues } = collectDocsFromSrc(configPath, stack().packages);
347+
expect(packageDocs).toHaveLength(1);
348+
expect(packageDocs[0].dir).toBe('src/packages/orders/docs');
349+
expect(packageDocs[0].docs.map((d) => d.name)).toEqual(['sales_playbook']);
350+
// A package's subtree is that package's SOURCE, not more packages — the
351+
// walk stops at a package root, so this is unchanged from before #18965
352+
// (the one-level sweep never reached it either).
353+
expect(issues).toEqual([]);
354+
});
355+
356+
it('a directory naming NO package keeps its own answer at the new depth', () => {
357+
writePackageDoc(path.join('packages', 'billing'), 'crm_index', 'MARKER-unmatched-deep');
358+
const { packageDocs, issues } = collectDocsFromSrc(configPath, stack().packages);
359+
360+
expect(packageDocs).toEqual([]);
361+
expect(issues).toHaveLength(1);
362+
expect(issues[0].rule).toBe('docs/uncollected-directory');
363+
expect(issues[0].path).toBe('src/packages/billing/docs');
364+
expect(issues[0].message).toContain('"billing" names none of this artifact\'s packages');
365+
});
366+
367+
it('an AMBIGUOUS directory name keeps its own answer at the new depth', () => {
368+
writePackageDoc(path.join('packages', 'core'), 'crm_index', 'MARKER-ambiguous-deep');
369+
const twins = [pkg({ ...CORE }), pkg({ ...CORE, id: 'com.other.core', name: 'Other Core' })];
370+
371+
const { packageDocs, issues } = collectDocsFromSrc(configPath, twins);
372+
expect(packageDocs).toEqual([]);
373+
expect(issues).toHaveLength(1);
374+
expect(issues[0].path).toBe('src/packages/core/docs');
375+
expect(issues[0].message).toContain('names 2 of this artifact\'s packages');
376+
});
377+
378+
it('⛔ ONE package answering to TWO docs directories is refused — never merged, never silently dropped', () => {
379+
writePackageDoc('orders', 'sales_flat', 'MARKER-loc-flat');
380+
writePackageDoc(path.join('packages', 'orders'), 'sales_nested', 'MARKER-loc-nested');
381+
382+
const { packageDocs, issues } = collectDocsFromSrc(configPath, stack().packages);
383+
// ⛔ Neither is collected: `attachPackageDocs` keys its sets by package
384+
// index through a Map, so collecting both would drop one without a word.
385+
expect(packageDocs).toEqual([]);
386+
expect(issues.map((i) => [i.rule, i.path])).toEqual([
387+
['docs/uncollected-directory', 'src/orders/docs'],
388+
['docs/uncollected-directory', 'src/packages/orders/docs'],
389+
]);
390+
// Each side names the OTHER, so the author can see both halves from either.
391+
expect(issues[0].message).toContain('answers to 2 docs directories');
392+
expect(issues[0].message).toContain('src/packages/orders/docs/');
393+
expect(issues[1].message).toContain('src/orders/docs/');
394+
});
395+
396+
it('⛔ single-package regression: a stack with no packages[] is walked ONE level and reports nothing deeper', () => {
397+
writePackageDoc(path.join('packages', 'orders'), 'sales_playbook', 'MARKER-invisible');
398+
const { docs, packageDocs, issues } = collectDocsFromSrc(configPath);
399+
400+
expect(docs).toEqual([]);
401+
expect(packageDocs).toEqual([]);
402+
// With no registered package there is nothing to search FOR, so there is
403+
// no recursion at all: `src/packages/docs` does not exist, so the sweep
404+
// finds nothing and says nothing — byte-for-byte the pre-#18965 answer.
405+
expect(issues).toEqual([]);
406+
});
407+
});
408+
261409
// ── Clause 2: one prefix rule per package ──────────────────────────────────
262410
describe('the doc lint reads the OWNING package namespace', () => {
263411
it('accepts a package doc carrying its OWN package prefix, not the artifact manifest one', () => {

0 commit comments

Comments
 (0)