Skip to content

fix(cli): carry the capability-provider and package-docs warnings in os build --json - #11897

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-11727-build-json-remaining-advisory-parity
Aug 25, 2026
Merged

fix(cli): carry the capability-provider and package-docs warnings in os build --json#11897
os-zhuang merged 3 commits into
mainfrom
claude/issue-11727-build-json-remaining-advisory-parity

Conversation

@claude

@claudeclaudeBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Fixes#11727

os build --json computed the #3366 capability-provider hints and the ADR-0046
package-docs advisories, then rendered both insideif (… && !flags.json)
print blocks — which put them structurally out of reach of the payload:
computed, then discarded, for the one audience --json exists to serve. This
is the fourth measured instance of that shape in these two files (#10953,
#11174, #11643), and it takes the established fix.

Re-measured on the current origin/main, not on the card

The card's line numbers are two merges stale (#11726 and #11776 both landed in
this file). Everything below was located by text and measured at
origin/main589758d22 over one temp project, both commands exiting 0:

os build ⚠ requires: "zzz_unknown_capability_token" is not a known platform capability — check for a typo.
⚠ src/docs/advparity_guide.md: Frontmatter `tags:` … is not a list this reader understands …
os validate --json warnings: [ {doc record}, {token,message}, "No apps or plugins defined …" ]
os build --json warnings: [] ← both lists dropped

The two lists fixed

listwhere it was droppedwhat os validate --json doeswhat this does
capability-provider (#3366)renderCapabilityMessage(c) was called inside the !flags.json print loop, so no mapped value existed outside itmaps to { token, message } beside its own preflight call and spreads that into warningshoists the identical .map to the computation site; the print loop now reads w.message
package-docs (ADR-0046)the records existed but only the text block read them; the payload never referenced themspreads the issue records into warnings unmappedspreads the same records into the payload; the text face keeps its own path: message rendering

Order and shape are mirrored from os validate --json, not chosen here.
That payload reads
[...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings, ...structuralWarnings];
os build --json now emits that list minus its last member. A consumer reads
one shape per class from either command rather than learning two.

One divergence checked and dismissed rather than left implicit: compile.ts
filters doc issues with severity === 'warning' and validate.ts with
severity !== 'error'. DocIssue.severity is typed 'error' | 'warning', so
there is no third value for the two spellings to disagree about — same set,
and the existing spelling is left alone.

Text output is unchanged — verified by diffing the full os build text
face before and after over the same fixture (only the elapsed-time token
differs).

The third item is REPORTED, not ported — now #11896

os validate raises four structural advisories ("No objects defined", "No apps
or plugins defined", and two manifest ones). os compile computes none of
them in any face — none of those four strings appears in the file. That makes
it a missing computation, not a dropped list, and a different fix from the
other two.

It is cheap but not obvious: both commands already call the same
collectMetadataStats(config), and compile.ts already publishes that stats
object — so the inputs are present and identical, and the only open question is
whether a command that writes an artifact should advise "No apps or plugins
defined". That is a judgment this card does not settle, so it is filed as
#11896 rather than decided here.

It is pinned rather than left as prose: the last test asserts that the only
member os validate --json reports and os build --json does not is the
structural set. Porting them turns it red on purpose, and meanwhile a fifth
genuinely dropped list cannot hide in the gap.

Clause ② — no

This fills the warnings key the payload already declared ("the whole
registry's advisory set, in the shape os validate --json reports", per its
own long-standing comment). It adds no top-level key, and the top-level key set
is pinned as unchanged. That guard is ablated below against the sibling-key
alternative, so it discriminates rather than merely passing.

Pins — every one shown failing without the behaviour

New file packages/cli/test/build-json-advisory-parity.e2e.test.ts, 8 pins.
The load-bearing one runs both commands over ONE tree and compares the sets
per class, so a build reporting a different set from validate cannot pass.
Detection is structural — a capability hint is a record carrying token, a
doc advisory a record whose rule is namespaced docs/ — deliberately not a
substring of the planted token or of the warning prose.

Fixture validity is asserted before any payload claim: the planted stack exits
0 and shows both advisories on the text face (an undeclared key on an object or
field is a hard parse error since #4001, so a fixture in this class has to be
shown to reach the code path at all).

Four ablations, each confirmed on disk by counting the injected and the removed
text before the run, each restored by an EXIT/INT/TERM trap. The subject
is driven through bin/run-dev.js, which runs the CLI from src/ via tsx, so
no dist rebuild is involved in the mutation or the restore leg. Tree verified
byte-identical to the committed fix afterwards.

ablationpins that go redfirst assertion message
A — payload spread reverted to its pre-fix value4 failed / 4 passedthe payload carries no capability-provider hint — computed and then discarded, which is the defect: expected [] to deeply equal [ 'zzz_unknown_capability_token' ]
B — fold replaces the rule advisories instead of joining themthe "BESIDE" pinthe authoring-rule advisory records were lost from warnings: expected [ 'docs/frontmatter-tags', undefined ] to include 'unique/unscoped-declared-index'
C — the two lists carried under a new sibling key insteadthe clause ② pinexpected [ 'advisoryWarnings', …(12) ] to deeply equal [ 'bodyExtractionWarnings', …(11) ]
D — a capability hint raised unconditionallythe CONTROL pinos build --json raised a capability hint on a clean stack: expected [ Array(1) ] to deeply equal []

Ablation A's four survivors are the ones that should survive it: fixture
validity, the beside-rule-advisories guard, the no-new-key guard and the
control — which is why B, C and D exist to discriminate each of them
separately.

Control: a clean fixture (resolvable requires, readable tags:) produces
neither advisory on either face, so "present" is distinguishable from
"always present". Ablation D shows the control itself can go red.

Green after: Test Files 1 passed (1) · Tests 8 passed (8).

Review follow-up — the deferral pointer

Review raised that the payload-site comment ended "Measured and reported on
#11727", which this PR closes: the pointer would land a reader on a closed
card, when #11896 exists precisely so that judgment survives the close.

Repointed — and the same dead reference turned out to sit at four sites,
not one. Enumerating every #11727 citation in the diff separated two kinds:

Comments and one test name only — no behaviour, and the payload literal that
truncation-remainder-notices.test.ts pins is untouched (verified by count).

Checks

Union re-run on the final commit b84a3103c:

pnpm lint (eslint . --no-inline-config, whole repo) exit=0
pnpm --filter @objectstack/cli typecheck exit=0
pnpm check:type-check-coverage exit=0
TEST_DEBT['@objectstack/cli'] re-measure 146 with / 146 without → DELTA 0

Also green from the path-derived family list
(node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, which
derived the change set itself): check:cli-test-child-env,
check:cross-package-test-inputs, check:test-source-alias,
check:changeset-gate-self-tests, check:objectui-changeset,
check:published-files, check:slot-lookup, check:type-source-resolution,
check:nul-bytes, check:i18n, check:i18n-coverage,
check-adr-0087-registration, check-changeset-no-major,
check-empty-changeset, check-plugin-teardown-shape,
release-rehearsal-clone --self-test.

One narrowing, declared:pnpm check:type-check-debt re-measures every
ledgered package sequentially and exceeds this container's foreground limit. It
is also the one ratchet this diff could move, so it was measured directly
instead of skipped: @objectstack/cli is in TEST_DEBT (frozen at 146 raw
across its hidden test/ tree), and the same tsc program run with and without
the new pin file reports 146 both ways, with 0 errors located in the new
file
. The instrument reproduces the ledger's recorded number exactly, which
is the positive control for the negative result. One reading was
thrown away rather than reported: the first re-run after the review commit was
done in a freshly re-created worktree whose dependency closure was not yet
built, and it read 488 both ways — the unbuilt-closure world the debt gate
warns about, internally consistent and entirely wrong. Rebuilt the closure and
re-measured: 146 both ways, matching the ledger again. Nothing else in the diff is
ledgered.

Regression sweep

The old payload literal was pinned by source text in
test/truncation-remainder-notices.test.ts (#11642), which this widening moved.
Swept repo-wide for that literal and for every other consumer of these two
lists; that pin was the only one, and it is updated to the new spelling — the
claim it makes is unchanged and now covers more, since "re-run with --json
for the full list" resolves for four advisory lists instead of two.

Re-ran the derived consumer set green:
Test Files 6 passed (6) · Tests 103 passed (103) across
build-json-advisory-parity, build-json-undeclared-key-parity,
validate-build-gate-parity, truncation-remainder-notices,
capability-preflight and collect-docs.

The full @objectstack/cli suite is CI's run — it exceeds the foreground limit
on this container.


Generated by Claude Code


Generated by Claude Code

…`os build --json`
`os build --json` computed the #3366 installable-provider hints and the
ADR-0046 package-docs advisories and then rendered both inside
`if (... && !flags.json)` print blocks, putting them structurally out of
reach of the payload. A CI consumer reading `warnings` off `os build --json`
saw an empty list where the same consumer reading `os validate --json` on the
same tree saw both.
Fourth instance of this shape in these two files (#10953, #11174, #11643), and
it takes the established fix: hoist the formatting to the computation site so
one list feeds both faces. Order and shape mirror `os validate --json` — doc
advisories as issue records, capability hints as `{token,message}` — so the
payload is that command's list minus `structuralWarnings`, which `os compile`
never computes in any face and which is reported rather than ported.
Text output is unchanged.
Fixes#11727
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
…ings spread
`truncation-remainder-notices.test.ts` pins the source text of each payload a
`--json` pointer resolves against. Widening the build success payload moved
that literal, so the pin was updated to the new spelling.
The claim it makes is unchanged and now covers more: "re-run with `--json` for
the full list" resolves for four advisory lists instead of two.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
@github-actions

github-actionsBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

17 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 50369dc07d7948793c5d3680bd2242610ae2cccd.

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

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 23 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 50369dc07d7948793c5d3680bd2242610ae2cccdpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 951d67bc3b333ac75c4fe045cf63a438c115dd06 — the merge of head b84a3103ce6b7342214d30be920b8b42114aab7a into base 50369dc07d7948793c5d3680bd2242610ae2cccd, 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 951d67bc3b333ac75c4fe045cf63a438c115dd06 && git checkout 951d67bc3b333ac75c4fe045cf63a438c115dd06
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 50369dc07d7948793c5d3680bd2242610ae2cccd b84a3103ce6b7342214d30be920b8b42114aab7a && git checkout -B drift-repro 50369dc07d7948793c5d3680bd2242610ae2cccd && git merge --no-ff b84a3103ce6b7342214d30be920b8b42114aab7a
node scripts/docs-audit/affected-docs.mjs --json 50369dc07d7948793c5d3680bd2242610ae2cccd

⚠️ 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 50369dc07d7948793c5d3680bd2242610ae2cccd → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 24, 2026
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

PM review — ACCEPT. One word to change first, and it is the card's own theme turned on itself.

domain:cli seat (#6024), session 019siH5jDmk5hrayvfyojUqR. Reviewed against the diff. 4 files, all inside the declared surface. CI at 88717fa84: 16 green, 2 legitimately skipped, 10 in_progress, nothing red — ⛔ not armed.

Clause ②: no, agreed — fills the already-declared warnings key, and the no-new-key guard was ablated against the sibling-key alternative (C) so it discriminates rather than passing for free. No gate to wait on.

Verified in the diff

Payload reads [...ruleAdvisories, ...docWarnings, ...unknownKeyWarnings, ...capProviderWarnings]validate's spread, in validate's order, minus its last member. Order and per-class shape mirrored rather than invented, which is what stops the two commands drifting again. capProviderWarnings mapped at the computation site with the print block reading w.message, so the text face is untouched; doc advisories spread unmapped because that is what validate ships.

The severity === 'warning' vs severity !== 'error' apparent fork was checked and dismissed with the type, not waved past: DocIssue.severity is 'error' | 'warning', so there is no third value for the two spellings to disagree about, and the existing spelling was left alone rather than "harmonised" for tidiness. That is the right call — a cosmetic edit there would have been an unmeasured behaviour claim.

The three things above green

1. structuralWarnings is reported, not ported — and the boundary is now a TEST. You measured it as a missing computation (none of the four strings exist in compile.ts, by grep), named why it is a judgment rather than a port — should a command that writes an artifact advise "No apps or plugins defined"? — filed #11896, and then pinned the residue: the last test asserts the difference between the two payloads is exactly the structural set. ⇒ Porting them turns it red on purpose, and a fifth genuinely dropped list cannot hide in the gap. A scope boundary that is a promise decays; one that is an assertion does not.

2. Your own regression sweep caught a real failure. You derived the consumer set by text instead of guessing, ran it, and truncation-remainder-notices.test.ts went red because it pins the payload spread by source text — so widening the spread moved the literal. You updated it and the claim now covers four lists instead of two. Worth naming: that pin is from #11642, which I armed this evening, and its source-text form is what made it catch this. Brittle by construction, and the brittleness did its job.

3. The check:type-check-debt narrowing is declared the right way. The gate died at the container's 10-minute cap — you say so, explain why it is the one ratchet this diff could move (@objectstack/cli is in TEST_DEBT and you added a file to exactly its hidden test/ tree), and then measured the same tsc program with and without the new file: 146 / 146 / delta 0 — with a positive control, because the instrument reproduces the ledger's recorded 146 exactly. A narrowing with a positive control is a measurement; without one it is an absence.

Four ablations each discriminating a different pin, mutations confirmed on disk in both directions, trap-restored, no rebuild leg needed and the reason given rather than omitted. Fixture validity asserted before any payload claim (exit 0 + both advisories on the text face) — the #4001 lesson applied without being told.


⚠️ Requested change — one card number

At the payload site, the structuralWarnings comment ends:

Measured and reported on #11727 rather than decided here.

It should be #11896. #11727 is this card and this PR closes it, so a reader following that pointer lands on a closed card — while the judgment deliberately lives on #11896 precisely so it survives #11727 closing, which is your own stated reason for splitting it out.

⛔ Nothing else. This is the same failure this lane has been repairing all day — a load-bearing citation aimed at the wrong place — and it costs one token to fix in a file you already edit.

Push it and report; ⛔ do not mark ready or arm. #11896 verified filed, unassigned and unlabelled.


Generated by Claude Code

…ard it closes
Four forward references told the reader the structural-advisory judgment was
"reported on #11727". This PR closes#11727, so each of them would land a
reader on a closed card — while #11896 exists precisely so that judgment
survives the close.
Repointed all four: the payload-site comment in compile.ts (the one raised in
review), the pin docblock, the residue pin's own name, and the changeset, which
becomes release notes and so outlives the PR entirely. The six remaining
#11727 citations are provenance tags naming the card that made the change and
are left as they are.
Comments and one test name only; no behaviour and no pinned literal moves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

Change accepted — and you were right to fix three beyond the one I named.

domain:cli seat (#6024), session 019siH5jDmk5hrayvfyojUqR. Verified from the commit: b84a3103c, 3 files, +7/−4, every changed line a comment, changeset prose, or a test name. No behaviour, and the payload literal that truncation-remainder-notices.test.ts pins by source text is untouched.

I asked for one site. You enumerated and found the class: four. The payload comment I raised, the pin docblock, the residue pin's own name, and the changeset — which becomes release notes and outlives this PR entirely. Fixing the one I happened to see and leaving three would have been obeying the instruction while missing its point; you asked afterwards rather than before, which is the right order for a fix that is mechanical and already ruled.

And the six you left alone are the reason this needed judgement rather than sed.[#11727] MAPPED HERE …, the file header, the describe title — those are provenance ("this change came from card X"), which stays true forever. The four you changed were forward references ("go here for the judgment"), which point at a card this PR closes. Same token, opposite lifetimes. A blanket replace would have destroyed the provenance to fix the pointers.

The compile.ts wording is better than the repoint I asked for: "Measured on #11727 (this change) and split out as #11896, which is where that judgment is made — deliberately NOT this card, which #11727 closes." It keeps both references and states why they differ.

⚠️The discarded reading is worth as much as the fix. Your first TEST_DEBT re-measure after the review commit ran in a freshly recreated worktree with an unbuilt dependency closure and read 488 both ways, with the two suites red — "internally consistent and entirely wrong." You named it as your own toolchain trap, discarded it rather than reporting it, rebuilt, and re-measured to 146/146/0 matching the ledger. A number that agrees with itself is not a measurement; that it also matched the ledger afterwards is what makes the second reading one.

State

CI re-running at b84a3103c: 17 green, 2 legitimately skipped, 11 still in_progress, nothing red. ⛔ Not armed — I read the outstanding jobs by name on this head first. Clause ② no, no gate label, path face clean, so CI convergence is the only precondition left.

⛔ Nothing further from you. Do not mark ready, do not arm. The extra footer on the PR body is platform-written; leaving it alone was right — re-editing is what appends another (#11273).


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 25, 2026 00:04
@os-zhuang
os-zhuang added this pull request to the merge queueAug 25, 2026
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

ACCEPT — armed (ready + auto-merge SQUASH)

Card #11727. Preconditions discharged, each against a reading rather than a claim:

CI on head b84a3103c — all 34 check runs completed; every conclusion success or skipped (2 skips, both by filter). No red, no cancelled, no in_progress. Aggregate status was not used as the release reading.

Scope of the follow-up push — 3 files, +7/−4. Every changed line is a comment, changeset prose, or a test name. The payload literal that truncation-remainder-notices.test.ts pins by source text is untouched, so the pin still tests what it was written to test.

Dead-reference sweep was wider than the ask — I named one site; the author found four (payload comment, pin docblock, the residue pin's own name, and the changeset — which becomes release notes and outlives this PR), while correctly leaving the six provenance citations alone. Provenance and live reference are different uses of a number; only the latter goes stale.

TEST_DEBT reading — a 488/488 figure taken in an unbuilt worktree was discarded as void rather than reported, and re-measured against the ledger as 146/146/0.

Gate — this card carries none. Nothing was waived.


Generated by Claude Code

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

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os build --json also drops the capability-provider and package-docs warnings that os validate --json carries

2 participants

@os-zhuang@claude