Skip to content

Commit 83e7ae9

Browse files
fix(cli): a multi-package artifact's flat src/docs rides the package that owns its manifest (#22403)
Fixes #22190 Clause-②: no ## What this does `os build` wrote a multi-package artifact's flat `src/docs/*.md` to the artifact's **top level**. Such an artifact keeps its metadata in `packages[]` only (ADR-0130 D4, 2026-09-22 addendum), so these docs were the only items left at the top level. On every boot, the metadata door's residual sweep (`packages/metadata/src/plugin.ts`) registered them under the artifact's `manifest.id` and warned that no package body declares them. The warning's remedy could not be followed in an ADR-0130 layout. The producer now conforms, as triage directed (`6054545749`). `placeCollectedDocs` (new, `packages/cli/src/utils/collect-docs.ts`) is the one call `compile.ts` uses to place collected docs: - each package's own `src/PKG/docs/` set goes onto that package's body, through `attachPackageDocs` as before; - the stack's own flat `src/docs/` set goes onto the body of `flatDocsOwner(stack)`, after that package's own directory docs. That owner is the **one** `packages[]` entry whose id equals the artifact's `manifest.id`. If no entry, or more than one, carries that id, or the manifest has no id, the flat set stays on the top level as before; - inline top-level `docs` never move. `packages/metadata` is not touched. The residual sweep stays as it is. ## Zone 2 hypotheses, measured | | verdict | evidence | |---|---|---| | H1: the flat set is top-level by #18431's ruling | **Not a fork.** The ruling's clause 1 (maintainer, `#18431` comment `5715694492`) reads: "The flat `src/docs/` of **a single-package stack** keeps attaching where it does today (top level = the one package)". It says nothing about a multi-package stack. The top-level placement for multi-package stacks came from the implementation (`df0c856e`, two days before the 2026-09-22 emitter flip). Later seat prose (the #19246 body, the metadata attribution test's comment) restated it as a ruling, but no maintainer record says so. Clause 2 (the namespace a stack-level doc is linted against) is unchanged here: flat docs are still judged by `stack.manifest.namespace`. | ruling text quoted; `git show df0c856` | | H2: the owner is decidable from the artifact | **Holds.** On the card-shaped fixture (`composeStacks([service, app], { manifest: 'preserve' })`, app source in `src/sales/`, service in `src/service/`), `manifest.id` is `app.example.acme`, and exactly one entry (index 1) carries it. When no entry or two entries carry the id, nothing moves, and both cases are pinned. The collector has no duplicate-id refusal of its own (`artifactPackages` does not check for one), so the two-entry case is pinned rather than assumed unreachable. | `flatDocsOwner` pins | | H3: single-package byte identity | **Holds.** Same fixture (inline doc, flat doc, an uncollected `src/sales/docs/`), built by base `117d34de` and by this branch: sha256 `710ab6c6acf915ac265c0fe0f05cca61e3418b85f84ace5f901e08000e000aef` both times. Build stdout is identical once timings are stripped, and the uncollected-directory warning appears in both. | one-off measurement | | H4: the WARN stops with no metadata change | **Holds, at boot.** See the boot reading below. | `os dev --fresh` before and after | | H5: other readers of top-level `docs[]` | Enumerated with `git grep`. The runtime reads collections generically (`resolveArtifactCollections` merges the top level with the bodies; `registerApp` and the metadata door register per body), and books resolve by doc `packageId`. Each reader answers the same owner and count. **Two answers changed, and both now match what a single-package artifact and a `src/PKG/docs/` doc already answer** (listed below). | API diff below | ## Boot-level reading (integration layer, run locally) Card-shaped fixture, `objectstack dev --fresh --seed-admin --no-watch`, authenticated reads. Base build is `117d34de`; the fix build is this branch. | reading | base | this branch | |---|---|---| | `[MetadataPlugin] … top-level metadata item(s) that none of its 2 package bodies declare` WARN | **1** | **0** | | artifact top-level keys | `manifest,packages,docs` | `manifest,packages` | | `GET /api/v1/meta/doc` docs, with owners | `acme_service_runbook`→service, `acme_faq`→app, `acme_guide`→app, `setup_overview` (4) | the same 4, same owners | | `GET /api/v1/meta/book/app.example.acme/tree` and the service package's book | identical | identical | | `GET /api/v1/meta/doc/acme_guide` `lock` / `editable` / `resettable` | `none` / `true` / `false` | `full` / `false` / `true` | | `GET /api/v1/packages`, app package `docs` | `[]` | `["acme_faq","acme_guide"]` | The last two rows changed. In both, the flat docs now read the way package docs already did: the service's `src/service/docs` doc reads `lock: full` on both builds, and a single-package artifact's flat doc reads `lock: full` (measured on the control fixture). Before this change, docs registered by the residual sweep were missing from the owning package's installed record, so they read back as unpackaged and freely editable even though their provenance said `package`. The changeset says so. ## Pins - `packages/cli/src/utils/collect-docs.flat-docs-owner.test.ts` (new, **unit** tier, so it runs on every PR): premise guard; `flatDocsOwner` for the unique, no-match, duplicate-id and no-id cases; placement (flat docs on the app body, carrying their markers); control: `src/service/docs` stays on the service body; the owner's own directory docs come first and are not dropped; inline top-level docs stay. **The door:** the real `MetadataPlugin` booted on the placed artifact in `artifact-only` mode logs **0** warnings and serves the same docs, owners and versions. The lit control is the same artifact with the flat set put back on the top level, built from the collection rather than from the placement: it draws exactly **1** warning from the same door. The test counts warnings and never reads their text. - `packages/cli/test/build-package-docs-attachment.e2e.test.ts` (nightly, integration): the command-level twin. It used to pin the old placement (the core body carried no docs and the top level carried `pkgdocs_index`). It now pins the new placement, plus a no-owner fixture (the manifest id names no entry) that keeps the top level. - `packages/cli/test/validate-build-gate-parity.test.ts`: the closed roster's artifact-assembly row now names `placeCollectedDocs`, the name `compile.ts` now calls. The row's reason is unchanged. **Ablation**, run from the committed state through `scripts/ablation-replace.mjs`. The mutation is `flatDocsOwner`'s `owners.length === 1` changed to `=== -22190`, so no owner is ever found. On-disk proof: anchor ×1→×0, blob `5db191c3f456`→`02e9d463bdfd`. The unit file reads **5 failed | 10 passed (15)**: every placement and door case fails. The service control, the lit control and all no-owner and single-package controls stay green. The e2e file reads **1 failed | 3 passed (4)**, with only the owned-placement case failing. Restore: blob equals HEAD `5db191c3f456`, and `git diff HEAD` is empty. The subject resolves from source through a relative import, so no rebuild was needed. ## Verification (union run after the last commit, HEAD `027bc7fa4`) - **Gates:** `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` (no paths, run at `027bc7fa4`) derived 65 families. The dispatch's list of 64 plus `pnpm check:cli-test-child-env`, which the derivation adds for the `packages/cli/test` edits. All 65 exit 0. `--ran` reconciliation: `65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN` (a derived zero: every row recorded its exit code). An earlier pass read `check:dual-build-cjs-loads` and `check:i18n-coverage` as `PREREQUISITE NOT MET` (exit 3: dist missing for packages outside the cli closure). After `turbo run build --filter=!@objectstack/docs`, both read OK in the union: `107 published require entry point(s) across 66 package(s) load` and `OK (13 config(s), 621 baselined untranslated string(s), none new)`. - **`pnpm lint`** (the full `eslint . --no-inline-config`, not narrowed): exit 0, no findings, at `027bc7fa4`. - **`pnpm --filter @objectstack/cli typecheck`:** exit 0. `tsc --noEmit` passes, and `check:test-typecheck: OK` holds 3 file(s) / 28 error(s) / 6 pinned signature(s) unchanged. - **cli `unit` layer** (`vitest run --project unit`): **271 files / 3983 tests passed**. Measured at `925e0742`; `git diff 925e074 027bc7f` is one docblock comment in `collect-docs.ts`. Re-run at `027bc7fa4` for the touched surface: unit (the three collect-docs files, `validate-build-gate-parity`, `non-array-packages-readers`) **5 files / 137 tests passed**; integration, nightly tier (`build-package-docs-attachment`, `build-docs-step-count`, `build-multi-package-artifact`) **3 files / 16 tests passed**. The integration layer was run locally only for these files, because the diff touches one of them. The rest of that layer is left to CI. ## Acceptance notes - **The dev config mirror (`serve.ts`) is not changed, because it is outside this card's file surface.** It still puts the flat set on the config's top level. Measured on this branch with `os serve objectstack.config.ts --dev` and no compiled artifact: `GET /api/v1/meta/doc` serves `acme_service_runbook` and `setup_overview` only. **The two flat docs are not served at all, and nothing warns.** `serve.ts` is byte-identical to base, so this behaviour predates this change. It is reported for the seat to file, not fixed here. `os dev` compiles first and serves from the artifact, so it is not affected. - `packages/metadata/src/plugin-artifact-packages-attribution.test.ts` (`domain:engine`, fenced): the `docsArtifact` docblock says that shape "is on every such artifact the compiler produces". That is now true only when no package owns the manifest. The test still passes because it builds its fixture by hand. Carrier: the next PR to touch that file. - `packages/cli/test/normalized-call-sites.test.ts`: the `serve.ts :: config.docs` row's `why` says the flat set joins the top level "as `collectAndLintDocs` returns it for the artifact's top level". For a multi-package build that is no longer where `os build` puts it. Carrier: the fix to the `serve.ts` mirror above. - `packages/cli/test/build-docs-step-count.e2e.test.ts`'s header quotes `finalBundle.docs = docsResult.docs`. The behaviour it describes (single-package fixtures) is unchanged, but the quoted line no longer exists. Carrier: none. - An artifact that is already built keeps its shape, and the warning, until it is rebuilt. --- _Generated by [Claude Code](https://claude.ai/code/session_01BmsuLyUeuG5CNpZFMH1jzS)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d8d0711 commit 83e7ae9

6 files changed

Lines changed: 556 additions & 27 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
`os build`: a multi-package artifact's flat `src/docs/` is now carried by the package that owns the artifact's manifest, so its boot no longer warns about them
6+
7+
Clause-②: no
8+
9+
A multi-package artifact (one that carries `packages[]`, ADR-0130 D4) keeps its metadata in its package bodies only. `os build` still wrote the stack's own flat `src/docs/*.md` to the artifact's top level. At every boot, the metadata service registered them under the artifact's `manifest.id` and logged `carries N top-level metadata item(s) that none of its N package bodies declare`. The warning told the author to rebuild the artifact, which did not help: in an ADR-0130 layout the app package's source directory (for example `src/sales/`) is not named after the package, so no docs directory the build reads belongs to it.
10+
11+
- **What changes.** When exactly one `packages[]` entry has the artifact's `manifest.id` as its id, `os build` writes the flat docs onto that entry's body, after any docs from that package's own `src/<pkg>/docs/`. The artifact's top level no longer carries them. `composeStacks(…, { manifest: 'preserve' })` always produces this case, because the artifact's `manifest` is one of its inputs' manifests.
12+
- **What stays the same.** The docs are served under the same package id, the doc count is the same, and books resolve the same tree. The doc lint still checks their names against `manifest.namespace`, and the `Collecting package docs` step line prints the same count.
13+
- **What a running instance now reports differently.** These docs are now part of the owning package's installed record (`GET /api/v1/packages`) and are protected like that package's other docs (`lock: 'full'`, resettable), the same as the flat docs of a single-package artifact. Before, they were registered outside any package record and read back as freely editable.
14+
- **When nothing moves.** If no entry has the manifest id, if several do, or if the artifact declares no `manifest.id`, the flat docs stay on the top level as before. A single-package artifact (no `packages[]`) is byte-identical to before.
15+
- **To pick it up.** Rebuild with `os build`. An artifact that was already built keeps its shape, and the warning, until it is rebuilt.

‎packages/cli/src/commands/compile.ts‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
2424
import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '@objectstack/lint';
2525
import { resolveJsxGateManifest, printJsxGateNotices } from '../utils/sdui-manifest.js';
2626
import { preflightDeclaredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
27-
import { attachPackageDocs, collectAndLintDocs, type DocIssue } from '../utils/collect-docs.js';
27+
import { collectAndLintDocs, placeCollectedDocs, type DocIssue } from '../utils/collect-docs.js';
2828
import { buildRuntimeBundle, cleanupOldRuntimeBundles } from '../utils/build-runtime.js';
2929
import {
3030
printHeader,
@@ -981,9 +981,6 @@ export default class Compile extends Command {
981981
}
982982

983983
const finalBundle: Record<string, unknown> = { ...(result.data as Record<string, unknown>) };
984-
if (docsResult.docs.length > 0) {
985-
finalBundle.docs = docsResult.docs;
986-
}
987984
// [#18431] Docs read out of `src/<pkg>/docs/` attach to the body of the
988985
// package that owns them — `packages[i].manifest`, ADR-0130 D4 option
989986
// B — and ⛔ never to the top level, which is the maintainer's ruling
@@ -993,11 +990,22 @@ export default class Compile extends Command {
993990
// `registerMetadataCollections` over `METADATA_ARRAY_KEYS`, which
994991
// carries `docs` — so a doc on a body is served under that package
995992
// and a flattened copy would buy nothing while destroying the
996-
// attribution. `attachPackageDocs` hands back the
997-
// ARGUMENT when it adds nothing, so a stack with no per-package docs
998-
// serializes from the very same references as before.
999-
if (docsResult.packageDocs.length > 0) {
1000-
finalBundle.packages = attachPackageDocs(finalBundle.packages, docsResult.packageDocs);
993+
// attribution.
994+
// [#22190] The stack's own flat `src/docs/` follows the same rule one
995+
// step up: on a multi-package artifact it rides the body of the
996+
// package that owns the artifact's manifest, because that artifact's
997+
// top level is no package's body and the metadata door warns about
998+
// every item it finds there. With no `packages[]` the top level IS
999+
// the one package, so it stays there. `placeCollectedDocs` makes both
1000+
// choices; it hands back the very `docs` array and `packages` value
1001+
// when it moves nothing, so a single-package stack serializes from
1002+
// the same references as before.
1003+
const placed = placeCollectedDocs(finalBundle, docsResult);
1004+
if (placed.docs.length > 0) {
1005+
finalBundle.docs = placed.docs;
1006+
}
1007+
if (placed.packages !== finalBundle.packages) {
1008+
finalBundle.packages = placed.packages;
10011009
}
10021010

10031011
// 4b. Bundle handler functions into `<artifactDir>/objectstack-runtime.{hash}.mjs`

0 commit comments

Comments
 (0)