Skip to content

feat(cli): carry the computed conversions on every os validate --json and os build --json failure exit - #12285

Merged
os-trump merged 3 commits into
mainfrom
claude/issue-12125-conversions-on-failure-exits
Aug 25, 2026
Merged

feat(cli): carry the computed conversions on every os validate --json and os build --json failure exit#12285
os-trump merged 3 commits into
mainfrom
claude/issue-12125-conversions-on-failure-exits

Conversation

@os-trump

@os-trumpos-trump commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Fixes#12125

os validate --json and os build --json dropped the conversions field on
every failure exit — the same "computed, then dropped" shape as the warnings
family (#11643 / #11391 / #11772 / #12047), one field over.

conversionNotices is filled by the onConversionNotice sink handed to
normalizeStackInput at step 2, above the schema parse and above every
later gate in both commands. The notice was therefore already in hand at each
failure exit and was then discarded; conversions: reached the terminal
success payload alone.

An ADR-0087 D2 notice is the one advisory class carrying an expiry
retiresIn names the protocol major where the old shape stops loading. So a CI
job gating on either --json face could not see that its tree depends on a
conversion about to retire for as long as the tree also tripped any unrelated
gate.

What changed

conversions is now present on every emitJson exit of both commands: 6 in
validate.ts (5 failure + success) and 10 in compile.ts (9 failure +
success). Existing keys are unchanged; the success payloads are unchanged in
content.

The fix is a pure scope change — the sink array is declared above the try
so the catch-all exit can read it — and normalizeStackInput still runs at
exactly step 2. ⛔ Carrying, not computing: a run that throws in
loadConfig, above step 2, reports [] honestly, and a pin holds that line.

#12047 is not addressed here and #11772 is not addressed here — both are the
warnings side and already landed; this card is the conversions field only.

⛔ The open sub-question, NOT settled here

Whether warnings and conversions should be folded into one field is a
live question raised by the filer that the 2026-08-25 ruling did not address,
and this change was explicitly not given authority to settle it. The two fields
are deliberately mirrored, not merged, and neither was restructured to make a
future fold easier.

Implementing did not surface any forced fork. The two fields are written
side by side at each exit with no shared site that has to be written twice; the
warnings spread stays warningsSoFar() and conversions reads its single
sink directly. A conversionsSoFar() wrapper was deliberately NOT added:
warningsSoFar() exists to state the order of five producers once, and this
field has exactly one producer.

The REGRESSION GUARD — warnings and conversions stay separate fields pin in
each test file records the shape as-shipped so a fold happens deliberately
rather than as a side effect. It asserts the fold property and nothing else,
so it is green in both states — ⛔ not red-before evidence, nor an argument
that folding is wrong.

⚠️ It did not start that way. As first written it also asserted that the failure
payload carries the one notice, which made a pin named REGRESSION GUARD into
red-before evidence — and the ablation caught it (it was one of the 25 reds).
That duplicate claim is the doc errors / 3f (package docs) test's job on the
same fixture, so it was removed rather than the name being loosened. Reported
rather than reconciled.

Zone 3 — PM mechanism assumptions, measured

Both assumptions held, and the surface sweep found no third command:

  • onConversionNotice is passed at exactly two call sites in the repo outside
    packages/spec itself: validate.ts:148 and compile.ts:162. os build is
    a thin alias subclass (build.tsexport default class Build extends Compile), so compile.ts is that whole surface.
  • Seven other CLI commands call normalizeStackInputwithout the callback
    (info, doctor, lint, i18n/check, i18n/extract, lint/score,
    scaffold-validate), so they never compute notices at all and publish no
    conversions field. That is a "never computed" shape, not this card's
    "computed then dropped" one — recorded, not fixed here.
  • Per-exit: step 2 is above all 5 validate failure exits and all 9 compile
    failure exits, so every one of them carries the notice. The only honestly
    empty exit is a throw at load, above step 2 — pinned as [] in both
    files.

Measured difference from the sibling card: for warnings, os build's
two earliest exits (--strict-body and the protocol parse) are empty by
construction. Step 2 is above both, so conversions is populated there.
The two fields genuinely disagree on those exits, which is why this field
needed its own measurement rather than inheriting the sibling's table.

Anti-vacuity

⭐ A pin asserting 'conversions' in payload is green against a conversions: [] hard-coded at every exit. So every fixture drives a live conversion —
page-kind-jsx-to-html (ADR-0087 D2, protocol 11) on pages[0].kind: 'jsx'
and each exit asserts the array whole against the notice's identity
(conversionId, surface, from, to, path), which is the "and NO MORE"
half.

Both directions of the instrument are proven: a converts nothing fixture runs
the same exit with the canonical kind: 'html' and requires []. A payload
with the notice hard-coded in passes every other test here and fails that one.

Verification

All runs below are on the final commit 2c7f002708, and every heavy run went
through scripts/pm/os-verify-lock.sh (its own VERDICT line is the authority
quoted, never a bare $?).

Pins, fixed treeTest Files 2 passed (2) · Tests 29 passed (29),
VERDICT command-exit 0.

Ablation — revert only the two source files to the merge base, keep the
pins, restore under trap … EXIT INT TERM. Mutation confirmed on disk in both
directions before any result was read (conversions: conversionNotices 6→1 in
validate.ts and 10→1 in compile.ts; the hoisted 4-space sink 1→0 and the
in-try 6-space sink 0→1 in each). Result: Tests 23 failed | 6 passed (29).

The 6 green in both states are named as regression guards, ⛔ never as
red-before evidence: the two control tests (the success payload always carried
the field), the two extractor self-tests (synthetic instrument controls), and
the two fold guards.

Restore verified after the run — conversions: conversionNotices back to 6/10,
[#12125] markers present, and git diff HEAD empty.

Full-repo lintpnpm lint (eslint . --no-inline-config), not
narrowed
: VERDICT command-exit 0.

Typecheckpnpm --filter @objectstack/cli typecheck: exit 0.

Ratchetcheck:type-check-debt first refused (--re-measure cannot run: … @objectstack/service-knowledge has no built type entry point). ⛔
refuse ≠ pass, so it was NOT counted; the closure was built with lint.yml's
own command and it was re-run to a real measurement:
check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured in 191.5s, 1843 raw tsc error(s) total, none above its recorded number. surplus: none.

Gate family — re-derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack against the actual change set (5 paths); all 19
runnable families green, re-run on this head after the final commit.


Generated by Claude Code

…son` and `os build --json` failure exit (#12125)
`conversionNotices` is filled by the `onConversionNotice` sink handed to
`normalizeStackInput` at step 2 — above the schema parse and above every later
gate in both commands — and was then published on the terminal success payload
alone. All five `os validate` failure exits and all nine `os build` failure
exits dropped a list already in hand.
An ADR-0087 D2 notice is the one advisory class carrying an expiry, so a CI job
gating on either `--json` face could not see that its tree depends on a
conversion about to retire until every unrelated failure was fixed first.
The fix is a pure SCOPE change: the sink array is declared above the `try` so
the catch-all exit can read it. `normalizeStackInput` still runs at exactly
step 2, so a run that throws in `loadConfig` reports `[]` honestly — carrying,
never computing earlier.
`warnings` and `conversions` are deliberately NOT folded: whether they should
become one field is a live question the ruling did not address.
Part of #12125
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HbG3rGVLjZStHQxHDtzJdJ
@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 8cdd696592b0c3c7bb7c3caa776f91aa6eb8cb68.

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

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: os validate (command, 43 pages)
  • 2 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 8cdd696592b0c3c7bb7c3caa776f91aa6eb8cb68packageMentionDocs.

Which tree this was computed on

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

⚠️ 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 8cdd696592b0c3c7bb7c3caa776f91aa6eb8cb68 → 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 25, 2026
The card measured its fixture with `source: 'hi'`, which no longer reaches the
success exit at current `main`: the `jsx-no-root` authoring rule rejects it
("Expected a single root element"), so the control fixture stopped at the rule
gate instead of proving the conversion fires on a passing run.
`<div>hi</div>` is the same source the ADR-0087 conversion registry uses in its
own fixture for `page-kind-jsx-to-html`.
Part of #12125
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HbG3rGVLjZStHQxHDtzJdJ
…s name claims
The guard also asserted that the failure payload carries the one notice, which
is red before the fix — so a pin named REGRESSION GUARD was in fact red-before
evidence. Measured in the ablated tree: it failed for that assertion alone.
That claim is the `doc errors` / `3f (package docs)` test's job, on this very
fixture. The guard now asserts the FOLD property and nothing else.
Part of #12125
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HbG3rGVLjZStHQxHDtzJdJ
@os-trumpClaude

Copy link
Copy Markdown
CollaboratorAuthor

Contract-review verdict: PASS (#12125)

Reviewed by the contract-review chain at CONTRACT_REVIEW_TIER. Tier fuse, machine reading this sub-round: get_session.external_metadata.last_served_model = claude-fable-5.

Reviewed: the changeset and both source patches in full, the test-file design as described with its ablation record, and the ruling lineage on #12125.

Conformance to the family ruling (maintainer 2026-08-25 on #11772/#12047, option 1, extended to conversions by triage under the same-family rule):

  • Pure scope change, verified in the diff: the sink array hoists above the try; normalizeStackInput stays at step 2; every failure exit of both commands reads the one binding; a throw at load reports [] honestly. Carrying, not computing — and the per-exit measurement was re-derived rather than inherited (the ⭐ finding that conversions populates os build's two earliest exits where warnings is empty by construction is exactly why).
  • The fenced sub-question was not settled: the fields are mirrored, not merged, neither restructured toward a fold, and the fold guard is honestly labeled green-in-both-states — including the reported correction where the ablation caught the guard's first draft carrying a red-before assertion under a regression-guard name. That self-catch is the evidence discipline working.
  • No conversionsSoFar() wrapper, with the right reason (one producer, nothing to order). Anti-vacuity holds both directions: live conversion driven, array asserted whole against the notice identity, converts-nothing control.
  • Changeset declares the widening with the same consumer-facing honesty as the sibling (feat(cli): carry the computed advisory lists on every os validate --json failure exit #12130), including the []-means-stopped-early note and the structured retiresIn preservation that makes CI gating possible without a second run.

Routed to the maintainer, not blocking: the fold question (A/B/C in the dev report) goes to the decision box with the two implementation inputs (the recorded deliberate-separation comment; #12297's sequencing interest).

Landing completed by the chain in the same stroke per the maintainer's standing instruction (2026-08-25, verbatim: 「审核通过你应该直接负责合并吧,还要等原始的项目经理吗」): clearing needs:contract-review on this PR and #12125, flipping ready, arming the queue.


Generated by Claude Code

@os-trump
os-trump marked this pull request as ready for review August 25, 2026 17:37
@os-trump
os-trump added this pull request to the merge queueAug 25, 2026
Merged via the queue into main with commit 79cf692Aug 25, 2026
40 checks passed
@os-trump
os-trump deleted the claude/issue-12125-conversions-on-failure-exits branch August 25, 2026 18:03
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/xlteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os validate --json and os build --json drop the conversions field on every failure exit, the same way warnings was dropped

2 participants

@os-trump@claude