Skip to content

feat(cli): carry the computed advisory lists on every os build --json failure exit - #12079

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-11772-build-json-failure-warnings
Aug 25, 2026
Merged

feat(cli): carry the computed advisory lists on every os build --json failure exit#12079
os-zhuang merged 2 commits into
mainfrom
claude/issue-11772-build-json-failure-warnings

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixes#11772

Maintainer ruling 2026-08-25 (5404971104, verbatim 「其他接受」): option 1 — every emitJson(...) failure exit in compile.ts carries the advisory lists the run has already computed, so warnings means the same thing on every exit and the text face's — re-run with --json for the full list pointer is true regardless of which later gate fails. Option 2 (payload shape depends on how far the run got) rejected; option 3 (weaken the pointer) dead.

Re-derived at origin/mainc804f0ca5 — the card's table was short by three

The dispatch flagged that the ruling's list of exits was not guaranteed exhaustive. Enumerated from source rather than from the card: compile.ts has 10 emitJson call sites — 9 failure exits and 1 success payload. The filing card's table listed six failure exits. The three it missed:

  • the protocol parse failure (step 3, reports under errors, not error);
  • the --no-runtime-bundle refusal (step 4b);
  • the bottom catch-all (catch (error)), whose payload is reached by any throw — including the artifact write.

The catch-all is the one worth calling out: the four lists were block-scoped consts inside the try, so they were not merely unpublished there, they were out of scope. That exit is reached with everything computed (e.g. an unwritable artifact path), which is exactly the scenario the truncation pointer promises to serve.

What each exit carries now

warnings is present on all 10 payloads. Contents are what the run had computed at that point — never more:

exit (step)existing keyswarnings beforeafter
strict-body: missing body (2b)issuesabsent[]
protocol parse failure (3)errorsabsent[]
author-time rules failed (3b)issuesruleAdvisoriesunchanged
capability provider preflight failed (3c)issuesabsentrule + capability
access matrix drift (3e)changesabsentrule + key + capability
docs validation failed (3f)issuesabsentall four
--no-runtime-bundle refusal (4b)errorabsentall four
runtime bundle failed (4b)errorabsentall four
thrown / caught (bottom)errorabsentwhat the run reached
successmanyall fourunchanged

Carrying, not computing. Each list stays computed at the step that owns it; the change only hoists the four bindings out of the try and adds one warningsSoFar() ordering site. No computation moved earlier, so no failure path pays for work it had not already done — and the two exits that run before any advisory step report [] because that is the truth about that run, not because the tree is clean. Per the dispatch's instruction, exits where a list is genuinely not computed yet are reported, not forced: that is the - cells above, and it is why 3c carries two lists rather than four.

Order now lives at one site that the success payload reads too (os validate --json's order minus its trailing structuralWarnings), so member order cannot drift between exits — the same "one list cannot drift from itself" idiom #11643 and #11727 used one list over.

Pin evidence — per exit, and failing without the behaviour

packages/cli/test/build-json-failure-warnings.e2e.test.ts (11 assertions). Each failure exit is driven by a fixture carrying a known advisory of each class, and the payload is asserted to carry exactly the classes the run had reached — no fewer and no more. The "no more" half is what distinguishes option 1 from a change that hoisted the computations earlier:

3c capability preflight { rule: 1, doc: 0, key: 0, cap: 1 } <- key/doc not computed yet
3e access-matrix drift { rule: 1, doc: 0, key: 1, cap: 1 } <- doc not computed yet
3f package docs { rule: 1, doc: 1, key: 1, cap: 1 } <- the card's headline
4b --no-runtime-bundle { rule: 1, doc: 1, key: 1, cap: 1 }
catch-all (EISDIR) { rule: 1, doc: 1, key: 1, cap: 1 }
3b author-time rules { rule: 1, doc: 0, key: 0, cap: 0 } <- regression guard

The 3b row is a REGRESSION GUARD, not red-before evidence, and is named as such in the file: that exit already published warnings: ruleAdvisories, and at 3b warningsSoFar() is exactly [...ruleAdvisories]. Its cap: 0 corrected a wrong first draft of my own — the fixture declares the unknown capability token, but the #3366 preflight is one step later, so the hint does not exist yet.

Ablation — revert only packages/cli/src/commands/compile.ts to c804f0ca5, keep the pins, run. Mutation and restore each confirmed on disk with anchored greps in both directions before the run (warnings: warningsSoFar() 10 → 0 → 10; the one-line four-list spread 0 → 1 → 0), and the script carries a trap … EXIT INT TERM restore:

MUTATION_CONFIRMED_ON_DISK Tests 9 failed | 2 passed (11)
RESTORE_CONFIRMED_ON_DISK Tests 11 passed (11)

The 9 red are the 7 behavioural failure-exit pins plus both structural pins. The 2 green in both states are the ones named in advance: the 3b regression guard, and the instrument's own positive control.

No dist/ on the measured path, and it is proven, not asserted. These pins drive packages/cli/bin/run-dev.js, whose header states it is "the SOURCE entry point — same CLI, run from src/ through tsx, used by this repo's gates and e2e suites so they do not depend on packages/cli/dist having been built". packages/cli/dist was absent for the whole ablation, so the child could not have resolved through it, and the mutation went red with no rebuild. (The contrast case is live in this same package: serve-node-env-production-default.e2e.test.ts deliberately drives the built bin/run.js, and it fails with command serve not found on a tree where packages/cli has not been built — see below.)

The instrument produces a positive before its negative is trusted. The exhaustiveness pin extracts every emitJson payload literal by brace matching; it is first run over synthetic input containing one payload withwarnings and one without, and asserted to find both and flag exactly the one missing it. A "no payload lacks warnings" pass from an extractor that finds nothing would otherwise read the same.

Clause ② — assessed against what was built: yes

The --json machine contract widens on the failure exits. A consumer that branches on warnings being absent from a failure payload sees a different shape after this change. needs:contract-review is hung on this PR and on the card; ⛔ this seat does not clear it.

The changeset names which exits gain the field and what a consumer keying off its absence must do instead — read success === false for "is this a failure payload", and ⛔ never read warnings: [] on a failure payload as "this tree raises no advisories" (it means the run stopped before those advisories were computed).

Checks — each gate's own verdict line

Gate family re-derived from the actual change set with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (the script derives its own change set; 21 gates over 4 paths).

All quoted from each gate's own verdict line, at 8c7bf1249 (the final commit; git rev-parse --short HEAD), with exit codes captured before any pipe.

Full-repo lint — the standing ask, run whole rather than narrowed:

LINT_EXIT=0 eslint . --no-inline-config

Package checks

TYPECHECK_EXIT=0 tsc --noEmit (@objectstack/cli)
CLI_TEST_EXIT=0 @objectstack/cli — Test Files 178 passed (178) · Tests 2025 passed (2025)
✓ check:cli-test-child-env: 32 spawner source(s) among 90 under packages/cli/test/**; no new bulk
process.env copy reaches a spawned child (0 baselined, ⛔ SHRINK-ONLY; 2 deliberate site(s) pinned)
✓ check-nul-bytes: OK (scanned 6667 text file(s); no raw ASCII control bytes)
✓ check:test-source-alias OK — 72 packages with tests scanned; 61 registered as still resolving a
workspace dep through `dist/`
✓ check:type-source-resolution OK — 93 tsc program(s) across 77 packages scanned
✓ check:published-files — 69 publishable package(s) of 78 workspace member(s) declare a `files` whitelist
✓ check:slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new
✓ check:cross-package-test-inputs — OK: 16 package(s) read outside themselves, all declared
✓ check-ci-filter-parity — OK: all 96 declared cross-package glob(s) (81 unique) are covered
✓ check:plugin-teardown-shape: 63 Plugin implementation(s) across 4648 source(s); baseline fully burned down
✓ check:changeset-gate-self-tests — 212 + 116 assertions over real temp git repos
✓ check-adr-0087-registration: this PR adds no declared-breaking changeset (1 non-breaking changeset seen)
✓ check-changeset-no-major: This diff introduces no `major` bump
✓ check-empty-changeset: No empty-frontmatter changeset introduced by this diff
✓ check:objectui-changeset — objectui-range --self-test: all checks passed
✓ release-rehearsal-clone --self-test passed
✓ check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys)
✓ check-i18n-coverage: OK (12 config(s), 657 baselined untranslated string(s), none new)
✓ check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured in 708.0s,
1898 raw tsc error(s) total, none above its recorded number

Two gates initially refused rather than failed, and refuse ≠ pass — so neither is reported from that state. Both were missing-build preconditions that CI satisfies and this worktree did not, and both were driven to a real measurement before being quoted above:

  • check:i18n-coverage first printed COULD NOT MEASURE — 1 of 12 config(s) failed to lint (@objectstack/connector-mcp had no build output), with its own note that the result "says NOTHING about whether any declared label went untranslated". Building that one package turned it into the OK (12 config(s) … none new) line above.
  • check:type-check-debt first threw --re-measure cannot run: 1 workspace dependenc(ies) … have no built type entry point on disk -- @objectstack/service-knowledge. Building it produced the re-measured OK line above.

⛔ The TEST_DEBT / type-check-debt ledger was not touched (#11788 owns it this round). The gate notes @objectstack/plugin-approvals could be lowered by 1; that is left for its owner rather than absorbed here.

The one test failure I saw, and why it is not this change.serve-node-env-production-default.e2e.test.ts failed with command serve not found while packages/cli/dist was absent. That file's own header states it spawns the shippedbin/run.js and that "bin/run.js plus a genuinely built dist/ is the only shape" it works in. Building @objectstack/cli made it pass 3/3 — so it was this worktree's missing build, measured, not assumed, and independent of compile.ts.

Out-of-scope finding

⛔ Left for the reviewer, per the dispatch: this PR stays draft, auto-merge is not enabled, and needs:contract-review is not cleared by this seat — the chain clears it and records its verdict on the card.


Generated by Claude Code


Generated by Claude Code

…n` failure exit (#11772)
`warnings` lived on the terminal success payload only (plus `ruleAdvisories`
alone on the author-time-rules failure), while the text face prints its
advisory blocks — the #11529 author-time advisories at 3b and the #3786
undeclared authoring-key findings at 3d — several gates earlier, each ending
in `— re-run with --json for the full list`. A build that then failed at a
later gate emitted that gate's failure payload, and none of those carried the
list: the remedy the notice named returned a payload without the withheld
entries in it (the #11643 / #11391 "the remedy named is unreachable" shape).
Maintainer ruling 2026-08-25, option 1 of three: every `emitJson` failure exit
carries the lists the run has ALREADY COMPUTED, so `warnings` means the same
thing on every exit. Option 2 (shape depends on how far the run got) and
option 3 (weaken the pointer) were both rejected.
Nine failure exits, three more than the filing card's table listed — it missed
the protocol-parse exit, the `--no-runtime-bundle` refusal and the bottom
catch-all. Every one now reads a single `warningsSoFar()` site, which the
success payload reads too, so the member order (`os validate --json`'s, minus
its trailing `structuralWarnings`) cannot drift between exits.
Carrying, not computing: each list stays computed at the step that owns it, so
an exit upstream of a step reports that list empty rather than paying for a
computation it had not already done.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
@github-actions

github-actionsBot commented Aug 25, 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 0e0bf8049dfcef2ac29c43cfa15f9963c3b921a0.

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 0e0bf8049dfcef2ac29c43cfa15f9963c3b921a0packageMentionDocs.

Which tree this was computed on

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

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

…type (#11772)
`Test Core (2/6)` failed on `packages/lint/src/authoring-rule-wiring.test.ts:291`
-- "os build imports no unratcheted symbol from @objectstack/lint":
AssertionError: compile.ts imports AuthoringFinding from @objectstack/lint
directly. Register the rule in AUTHORING_RULES, or add the symbol to
LINT_IMPORT_RATCHET with a reason.
expected [ 'AuthoringFinding' ] to deeply equal []
WHY THIS CARD TRIPPED IT. Hoisting the four advisory lists out of the `try` so
every `emitJson` failure exit can read `warningsSoFar()` turned one inferred
binding into a declared one. Before the hoist the list arrived destructured --
`const { errors: ruleErrors, advisories: ruleAdvisories } = splitBySeverity(...)`
-- and its type was inferred, so no name was imported. A `let` declared ahead of
the assignment needs a written type, and the first spelling reached for the
underlying finding type, adding `AuthoringFinding` to compile.ts's import list.
The #4409 import scan reads that list and had never been told about the symbol.
`import type` does not escape the scan, and that is deliberate: `lintImportsIn()`
matches `/import\s+(?:type\s+)?\{([^}]*)\}\s*from\s*['"]@objectstack\/lint['"]/g`
and then strips a per-name `type ` prefix, so both the statement-level and the
inline modifier are seen. The guard is asking which SYMBOLS this command file
names, not which of them survive to runtime.
THE FIX. Bind the annotation to the function that produces the value:
let ruleAdvisories: ReturnType<typeof splitBySeverity>['advisories'] = [];
`splitBySeverity` is already an import this file makes and already carries a
LINT_IMPORT_RATCHET entry ("Pure partition of a finding list into gating vs
advisory. Carries no rule identity at all."), so no symbol is added and no
exemption is widened. It resolves to exactly the same type the old annotation
spelled -- `AuthoringFinding[]`, the function is sync so no `Awaited` is
involved -- and `tsc --noEmit` is clean.
WHY THIS IS NOT A GATE BYPASS. Nothing was added to LINT_IMPORT_RATCHET or to
AUTHORING_RULES, no assertion in authoring-rule-wiring.test.ts was touched, and
no test was skipped or relaxed. The ratchet's exemption set is byte-identical
before and after; the import list it scans is what shrank. The annotation is
strictly tighter than the one it replaces: it is now pinned to
`splitBySeverity`'s declared shape, so if that function's `advisories` member
ever changes type this binding follows it instead of silently disagreeing --
the same "one list cannot drift from itself" idiom this card applied to the
member ORDER of `warningsSoFar()`, now applied to the list's TYPE.
Behaviour is unchanged: type-only edit, no emitted JS differs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

CI repair: Test Core (2/6) — the hoisted advisory list is now typed off splitBySeverity

8c7bf1246da672d43. One file, type-only, no behaviour change.

The red

packages/lint/src/authoring-rule-wiring.test.ts:291"os build imports no unratcheted symbol from @objectstack/lint":

AssertionError: compile.ts imports AuthoringFinding from @objectstack/lint directly.
Register the rule in AUTHORING_RULES, or add the symbol to LINT_IMPORT_RATCHET with a reason.
expected [ 'AuthoringFinding' ] to deeply equal []

Why this card tripped it

Hoisting the four advisory lists out of the try so every emitJson failure exit can read warningsSoFar() turned one inferred binding into a declared one. Before the hoist the list arrived destructured — const { errors: ruleErrors, advisories: ruleAdvisories } = splitBySeverity(findings) — so its type was inferred and no name was imported. A let declared ahead of its assignment needs a written type, and the first spelling reached for the underlying finding type, putting AuthoringFinding on compile.ts's import list. The #4409 import scan reads that list and had never been told about the symbol.

import type does not escape the scan, and that is deliberate — lintImportsIn() is

/import\s+(?:type\s+)?\{([^}]*)\}\s*from\s*['"]@objectstack\/lint['"]/g

and then strips a per-name type prefix, so the statement-level and inline modifiers are both seen. The guard asks which symbols this command file names, not which survive to runtime.

The fix

letruleAdvisories: ReturnType<typeofsplitBySeverity>['advisories']=[];

splitBySeverity is already imported here and already carries a LINT_IMPORT_RATCHET entry ("Pure partition of a finding list into gating vs advisory. Carries no rule identity at all."), so no symbol is added and no exemption is widened. It resolves to exactly what the old annotation spelled — AuthoringFinding[]; the function is sync, so no Awaited is involved.

It is also the tighter statement, and the same idiom this PR already applies one level over: the member order of warningsSoFar() lives at one site so it cannot drift from itself; the list's type is now pinned to the function that fills it, so a change to splitBySeverity's advisories member propagates here instead of silently disagreeing.

Not a gate bypass

Nothing added to LINT_IMPORT_RATCHET or AUTHORING_RULES; no assertion in authoring-rule-wiring.test.ts touched; nothing skipped or relaxed. The exemption set is byte-identical before and after — what shrank is the import list being scanned.

Readings

Red reproduced locally on 8c7bf124 first (REPRO_EXIT=1), then re-run on the fix. All exit codes captured before any pipe.

checkbeforeafter
pnpm --filter @objectstack/lint testTest Files 1 failed (1) · Tests 1 failed | 25 passed (26)Test Files 81 passed (81) · Tests 2294 passed (2294)
pnpm --filter @objectstack/cli testTest Files 178 passed (178) · Tests 2025 passed (2025)
pnpm --filter @objectstack/cli typecheck (tsc --noEmit)exit 0, no diagnostics
eslint --no-inline-config on compile.ts"errorCount":0,"warningCount":0

The two structural pins in build-json-failure-warnings.e2e.test.ts (const warningsSoFar = () => [ and the spread-order regex) are untouched by this edit and passed: vitest list --filesOnly collects exactly 178 files including test/build-json-failure-warnings.e2e.test.ts, and all 178 passed.

Measured, untouched: packages/cli/src/utils/scaffold-validate.ts also imports type AuthoringFinding from @objectstack/lint and is green — the guard scans only packages/cli/src/commands/{validate,compile,lint}.ts, so that file is outside its reach. Not this card's business; flagging it only so the asymmetry isn't read as an oversight here.

One environment difference worth stating rather than smoothing over: CI's shard reported 2288 passed | 5 skipped, the local run 2294 passed with no skips — the 5 skipped upstream ran and passed locally.

Generated by Claude Code


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 25, 2026 11:50
@os-zhuang
os-zhuang merged commit d114d5e into mainAug 25, 2026
35 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-11772-build-json-failure-warnings branch August 25, 2026 11:50
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os build --json's failure payloads carry no warnings, so the truncation notice's --json pointer is a dead end when a later gate fails

3 participants

@os-zhuang@os-trump@claude