Skip to content

fix(cli): carry the ADR-0112 error carriers on os lint --eval --json's generator-load exit - #16044

Queued
os-litant wants to merge 2 commits into
mainfrom
claude/issue-15549-lint-eval-generator-envelope
Queued

fix(cli): carry the ADR-0112 error carriers on os lint --eval --json's generator-load exit#16044
os-litant wants to merge 2 commits into
mainfrom
claude/issue-15549-lint-eval-generator-envelope

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes #15549

os lint --eval --json's generator-load failure emitted a bare {error}. The catch around the --generator load built its human message and then discarded the error object, so the one eval exit that does have a machine face was off-envelope: a consumer that reads code to branch got a real code from the same command's project-lint catch-all and undefined from eval mode.

The change

One line, plus its reasoning. The exit now spreads errorCodeFields(error) — the same helper run()'s project-lint catch-all spreads, rather than a second shape written by hand here:

await emitJson({ error: msg, ...errorCodeFields(error) }, 0, { compact: true });

Nothing is minted. errorCodeFields passes a producer's code through and returns {} otherwise — ADR-0112's ledger stays the authority on who may mint a code — so the repaired exit is polymorphic in exactly the way its sibling already is.

Driven, not reasoned about — the command's own bytes

Probed on bin/run-dev.js (the source entry) at the same commit, before and after. Four reachable generator-load failure classes:

generator before after
top-level throw carrying code + httpStatus {"error":"…: upstream refused the model"} {"error":"…","code":"FORBIDDEN","httpStatus":403}
top-level read of a missing file {"error":"…: ENOENT: no such file…"} {"error":"…","code":"ENOENT"}
module default-exports a non-function {"error":"…: module must default-export a function (prompt, id) => stack"} unchanged — plain Error, nothing to pass through
unresolvable path / syntax error {"error":"…: Build failed with 1 error…"} unchanged — esbuild's BuildFailure carries neither key

Exit code stays 1 in every row; the human path and the eval report exit are untouched.

⭐ The measurement that decides this is a repair and not a formality: code-carrying errors really do reach that catch. Probed against bundleRequire directly — a generator whose top-level evaluation throws propagates its error intact, so ENOENT and a full FORBIDDEN / 403 pair both arrive there, and both were being dropped. esbuild's own failure object has only errors/warnings, which is why two of the four rows correctly stay bare.

Tests

New: packages/cli/test/lint-eval-generator-load-envelope.e2e.test.ts — 7 cases driving the real command, no dist/ on the measured path.

Both halves are pinned, deliberately. A "repair" that minted a placeholder code for every load failure would satisfy the card's headline and hand consumers a vocabulary no ADR-0112 ledger declares, so the negatives (nothing is minted) fail that repair directly. the key set is exactly the carriers additionally pins the #14015 fence from this side: conversions is not added here, and a later widening goes red.

Ablation — the fix reverted, the test file kept, at e0938d3fdce. Mutation confirmed on disk (git hash-object moved off the HEAD blob; injected-text count 1, removed-text count 0), restore confirmed byte-identical (git diff HEAD empty, hash back to b8e0dbef945):

Tests  3 failed | 4 passed (7)
  × a coded failure at import surfaces BOTH carriers on the --json face
  × an errno thrown at import passes through as `code`
  × the key set is exactly the carriers — no `conversions`, no filler

Exactly the three carrier pins go red and the four negative/untouched controls stay green — the discriminating direction, not merely "it goes red". No rebuild is on that path: bin/run-dev.js loads ../src/… through tsx, so both commands/lint.ts and utils/format.ts are read from source by the child.

Clause ② — declared per limb, from the delivered diff

  • Mechanical floor — YES. The delivered diff puts two new keys, code and httpStatus, on a published --json payload, and the table above measures them arriving. That is the "new key on a published payload" shape exactly. (Nothing under packages/spec/src/** is touched; the floor is tripped by the new key, not by that path.)
  • Conformance limb — YES, under the grade-YES-when-unclear doctrine. On the reading where the "verdict" is the payload the face answers, the input class of code-carrying load failures moves between two already-published shapes on a shipped face. On the narrower accept/reject reading it is NO — every input keeps its exit code, its human message and its accept/reject outcome, and the change is purely additive. The call is not clear, so it is graded YES.

⛔ The needs:contract-review label is not applied here — that is the seat's to place on the card and the PR together.

Verification

Gate union derived at e0938d3fdce with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, asserted against its own Reconciliation — 56 famil(ies) line: 56 commands emitted, 56 families reconciled. All exit codes captured before any pipe.

  • 56/56 green. Four (check:i18n, check:i18n-coverage, check:i18n-walk-parity, check:dual-build-cjs-loads) first answered PREREQUISITE NOT MET on an unbuilt dist/; the CLI and spec were built and all four then returned real verdict lines.
  • Artifact rosters block, run separately (37 families, outside that total by the tool's design): 34 green, 3 NOT MEASUREDcheck-partof-closing-keyword.mjs and check-single-claim-paths.mjs exit 2 NOT WIRED (they need PR context that exists only in CI), and check:react-declaration-parity states "MANIFEST is not set … This gate did NOT run", which needs a browser-built objectui manifest this container has no way to produce. None is a pass and none is caused by this diff.
  • pnpm --filter @objectstack/cli typecheck green, and the new test file is confirmed inside the type-checked set (1 hit under tsconfig.test.json --listFiles), not merely adjacent to it.
  • vitest run over the affected files — the new e2e, the sibling lint-eval-json-unscorable-stack.e2e, and format.error-code-fields33/33 pass.
  • Lint: a declared narrowing, not a skipped run. Repo-wide pnpm lint is CI's. Locally eslint ran over the two changed source files: population read from eslint's own resolution (neither file is ignored), file count read from --format json (2 files, 0 errors, 0 warnings), and the config itself declares it "never enables type-aware linting … for ANY file", so this diff cannot move the verdict of any file it does not touch.

Scope

This card only. Handed back rather than folded in, each still a distinct shape: #14974 (a path in the same mode with no payload at all), #15547 (resolveConfigPath printing human text to stdout), the conversions sibling fenced by #14015, and #15550 (--generator's "Requires --eval." claim). This diff touches neither packages/rest/src/rest-server.ts nor packages/cli/src/commands/serve.ts, so it collides with no hard serial; it touches no file #15550 would.

Opened as a draft, not flipped ready, auto-merge not armed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N


Generated by Claude Code

…`'s generator-load exit

Eval mode's `--generator` load failure was the one exit on that mode with a
machine face, and it was off-envelope. The `catch` built its human message and
then discarded the error object, so `code` and `httpStatus` could never reach
the payload: a consumer that reads `code` to branch got a real code from the
same command's project-lint catch-all and `undefined` from eval mode.

The exit now spreads `errorCodeFields(error)` — the same helper that catch-all
spreads — so both failure faces of `os lint` are built from one source instead
of two hand-written shapes. Nothing is minted: the helper passes a producer's
code through and returns nothing otherwise, so the exit stays polymorphic in
exactly the way its sibling already is.

Measured on the command's own bytes, not on a JSON literal in a test: a
generator whose top-level evaluation throws now surfaces `FORBIDDEN`/`403` and
`ENOENT` respectively, where both were dropped before; esbuild's own build
failure and the hand-thrown "must default-export a function" carry neither key
and still emit a bare `{error}`.

`conversions` is deliberately not added alongside the carriers — that key on
the `--eval` exits is a separate card with its own review fence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
@github-actions github-actions Bot added the size/m label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

20 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 53cbad9f75572600ef43bb2a18071633fd6c0f68.

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

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • 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 — 22 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 53cbad9f75572600ef43bb2a18071633fd6c0f68packageMentionDocs.

Which tree this was computed on

This run read content/docs from 8851ffd8a4449628ab171b543e47ba24891c199b — the merge of head 98179cae022584b0405e0c7ce9bec43d501076ab into base 53cbad9f75572600ef43bb2a18071633fd6c0f68, 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 8851ffd8a4449628ab171b543e47ba24891c199b && git checkout 8851ffd8a4449628ab171b543e47ba24891c199b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 53cbad9f75572600ef43bb2a18071633fd6c0f68 98179cae022584b0405e0c7ce9bec43d501076ab && git checkout -B drift-repro 53cbad9f75572600ef43bb2a18071633fd6c0f68 && git merge --no-ff 98179cae022584b0405e0c7ce9bec43d501076ab

node scripts/docs-audit/affected-docs.mjs --json 53cbad9f75572600ef43bb2a18071633fd6c0f68

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026

@os-litant os-litant left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contract review (clause ②) — #16044 at head e0938d3fdce

Submitted as a COMMENT review, not an approval: GitHub refuses APPROVE on a PR opened under the same account, and this seat holds that account. The verdict line below is the verdict; the review event type is not.

Implemented-by: claude/issue-15549-lint-eval-generator-envelope (mode:subagent, no session of its own)
Reviewed-by:    context-isolated contract-review subagent of the dispatching session; fed the card, the seat's clause-② ruling and the PR only

VERDICT: CHANGES REQUESTED — one blocking item, one word wide (changeset bump level). Every other leg measured clears.

  • Mechanical floor: YES. code and httpStatus are new keys on a published payload — the pre-fix literal emitJson({ error: msg }, 0, { compact: true }) (blob a0bb8e2046c, identical on main at e1d4f9e3f99) could carry no key but error, and after the diff both keys are measured arriving on the real command's stdout (table below). Conditional presence and pass-through do not unmake a new key: the key set the face can answer with grew from {error} to {error, code?, httpStatus?}. The seat's mechanical-floor YES survives this reading.
  • Non-mechanizable conformance limb: YES, under the grade-YES-when-unclear doctrine. On the reading where the verdict is the shape the face answers, the input class "generator modules whose top-level evaluation throws a coded error" is re-selected between two already-published shapes on a shipped face — bare {error} (published on this exit) and {error, code, httpStatus} (published on the sibling project-lint catch-all and on every --format json failure envelope since #13510). On the accept/reject reading it is NO: every input keeps exit 1, its human message and its outcome. The two readings disagree, so the call is not clear and it is graded YES.

The dev's refinement, tested at source rather than accepted

The claim: the project-lint catch-all does not emit a full envelope unconditionally either, so key-for-key symmetry would have required minting.

  • packages/cli/src/utils/format.ts errorCodeFields (lines 246–254, unchanged by this PR): starts from {}, returns {} for an exit signal, sets code only for a non-empty string, sets httpStatus only for an integer. It returns nothing for a plain Error.
  • packages/cli/src/commands/lint.ts lines 760–762 (unchanged): the project-lint catch-all emits { error, ...errorCodeFields(error), conversions }. So the card's "the full ADR-0112 envelope" was over-stated; the sibling is conditional too.
  • Therefore the repair adding no keys on two of the four load-failure classes is correct, not an under-repair: a bare Error from the hand-thrown "must default-export a function" and esbuild's BuildFailure carry neither key and the sibling would answer them the same way. Forcing symmetry would have minted a code the ADR-0112 ledger does not own. The refinement is right; the PR does not under-repair.

① Derived judgments — every input class, driven on the real command (bin/run-dev.js under tsx, unpiped, exit captured first)

input class BEFORE (base lint.ts) AFTER (head) exit judgment
top-level throw with code: 'FORBIDDEN', httpStatus: 403 {error} {error, code: "FORBIDDEN", httpStatus: 403} 1 → 1 correct — the card's headline case
top-level readFileSync of a missing file {error} {error, code: "ENOENT"} 1 → 1 correct — errno passes through, no status invented
module default-exports a non-function {error} {error} 1 → 1 correct — plain Error, nothing to pass through
unresolvable path (esbuild BuildFailure) {error} {error} 1 → 1 correct — BuildFailure carries neither key
syntax error (esbuild BuildFailure) {error} {error} 1 → 1 correct
code: 'EEXIT' + httpStatus: 500 (exit-signal shape) {error} {error} 1 → 1 correct — isExitSignal exclusion holds on this exit
oclif: { exit: 2 } + a string code {error} {error} 1 → 1 correct — same exclusion, other spelling
numeric code: 403 + httpStatus: 403 {error} {error, httpStatus: 403} 1 → 1 correct — per-key independence; numeric code rejected, status kept
httpStatus: 502, no code {error} {error, httpStatus: 502} 1 → 1 correct
code: '' + httpStatus: 418 {error} {error, httpStatus: 418} 1 → 1 correct — empty string is not a code
code: 'WEIRD' + httpStatus: 4.5 {error} {error, code: "WEIRD"} 1 → 1 correct — fractional status dropped
throw 'a bare string' {error} {error} 1 → 1 correct — no crash on a non-object
human path (--generator, no --json) text on stdout, no JSON unchanged 1 → 1 untouched
offline eval (--eval --json, no generator) report, ok: true unchanged 0 → 0 untouched

What each control would look like if the thing under test were broken: a minting fix would put a code on the four bare rows; a coupled emit-both-or-neither would blank the three status-only rows; a missing exit-signal exclusion would print code: "EEXIT" on row 6; a change to the exit code would move any row off 1; the human-path and offline rows would gain a JSON document or an error key. None of those happened.

"No rebuild needed" — verified, not relied on. packages/cli/dist/commands/lint.js hashed 913eecfeb60 before, during and after the BEFORE phase (dist was never rebuilt), while the source-only restore of lint.ts to blob a0bb8e2046c flipped every carrier row back to bare and the restore to b8e0dbef945 flipped them forward again. The only thing that moved between the two readings was src/commands/lint.ts, so src/ is what the child loads (oclif dev-mode tsPath over rootDir: src / outDir: dist).

Ablation reproduced (fix reverted on disk, test file kept): Tests 3 failed | 4 passed (7) — the three red are exactly the carrier pins ("a coded failure at import surfaces BOTH carriers on the --json face", "an errno thrown at import passes through as code", "the key set is exactly the carriers — no conversions, no filler") and the four negative/untouched controls stay green — the discriminating direction the PR claims. Mutation proved on disk before measuring: lint.ts at blob a0bb8e2046c, injected-text count 0, removed-text count 1, dist/commands/lint.js still 913eecfeb60. Restore confirmed byte-identical: git hash-object back to b8e0dbef945, git status --porcelain empty.

② Semver grade — BLOCKING

The changeset declares "@objectstack/cli": patch. The lane's own precedents grade exactly this class — an additive member on a published machine-readable surface — as minor, and one of them rejected the "bug-framed, so patch" reading in so many words:

  • .changeset/cli-json-error-envelope-adr-0112-code.md (#13510, the PR that first put code/httpStatus on CLI --format json failure envelopes): "@objectstack/cli": minor.
  • Commit 8aaa118d1f5 (#13741, conversions on os lint --json): "@objectstack/cli": minor"matching the two nearest precedents on this lane rather than the bug/feature framing … Triage graded this card a Bug … restoring a parity contract can still widen a wire surface, and the grade follows the surface."
  • The PR's own clause-② declaration rests on "two new keys on a published --json payload"; the in-seat review rules (contract-review.md item ②) require the change level and the changeset declaration to agree. They do not: the diff is graded a new key for the tier and a patch for the version.

Reproduction: sed -n 2p .changeset/lint-eval-generator-load-envelope.md"@objectstack/cli": patch; the precedent's changeset was consumed at release, so read it from history: git show 098a08ffafa:.changeset/cli-json-error-envelope-adr-0112-code.md | sed -n 2p"@objectstack/cli": minor, and git show 8aaa118d1f5 --stat names the #13741 one, graded minor in its own commit message. Fix: patchminor in that one line. Nothing else in the changeset body is wrong — every class it names was measured above. No gate catches this (check-changeset-no-major refuses only major).

③ Boundary flags the dev raised — each answered

  • conversions deliberately not added — correct. #14015's disposition names adding conversions to the --eval exits as branch 2, "⛔ out of scope … needs its own card and an at-tier contract review", and the test the key set is exactly the carriers pins that fence from this side. Adding it here would have widened a second key set under a card that fences it.
  • No isExitSignal re-throw added — acceptable as delivered. Read: the try contains only bundleRequire(...) and one throw new Error(...); nothing in it calls this.exit(). Measured: even if an exit-signal-shaped error did arrive (rows 6–7), errorCodeFields returns {}, so no EEXIT can reach the payload. The asymmetry with the sibling catch-all is real and pre-dates this PR; leaving it out keeps the diff to the card. Not blocking.
  • Three NOT FILED hand-backs in the report — the card-framing refinement is confirmed above; the exit-signal asymmetry is confirmed above; the no-token container fact is a container fact.

Verification

  • Gate union: derived in my own worktree with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands <the three PR paths> → 56 commands, asserted against the tool's own Reconciliation — 56 famil(ies) line. Result: 56/56 green — 29 node scripts/… families and 27 pnpm check:* families, each exit code captured before any pipe (runner writes <exit>\t<verdict> per row; 2 and 3 are named, never graded). check:i18n, check:i18n-coverage, check:i18n-walk-parity and check:dual-build-cjs-loads ran against a built dist/ and returned real verdict lines (exit 0; zero PREREQUISITE lines in their logs).
  • Artifact rosters block (37 families, outside that total by design, run separately): 34 green, 3 NOT MEASURED (named below with their own verdict lines); none of the 3 is caused by this diff. In that block the pnpm-spelled check:partof-closing-keyword and check:single-claim-paths resolve to --self-test only (#16030) — their green grades the checkers, not this PR; the PR-judging invocations are the bare node forms, reported below by name.
  • pnpm --filter @objectstack/cli typecheck exit 0, and the new test file is inside the type-checked set (1 hit under tsconfig.test.json --listFiles).
  • vitest at head over the new e2e, lint-eval-json-unscorable-stack.e2e and format.error-code-fields: 20/20 across the two e2e files (7 new + 13 sibling) and 13/13 in src/utils/format.error-code-fields.test.ts — the 33 the PR reports, reproduced; src/commands/meta/delete-json-error-code.test.ts 6/6 alongside.
  • ESLint over the two changed source files: 2 files, 0 errors, 0 warnings, neither ignored (--print-config resolves for both). Repo-wide pnpm lint is the CI job's: Lint & Repo Gates completed success on this head at 22:02Z.
  • Docs: no hand-written page states this payload's shape (Failed to load generator / --generator / --eval: 0 hits across content/docs and the CLI README; the drift bot's 24 rows re-derived locally and grepped). Nothing to update.
  • Base branch: main moved from 2e357650306 to e1d4f9e3f99 (three commits: #16034, #15980, #15968) during this review; none touches lint.ts or format.ts, and git merge-tree of the head onto it is clean. On this head every one of the 37 CI check runs has completed and every non-skipped one is success, the six required contexts included — no base-branch red is riding this PR.
  • Hard serials: the PR's three files touch neither packages/rest/src/rest-server.ts nor packages/cli/src/commands/serve.ts; nothing #15550 would touch is touched.

NOT MEASURED — by name, each with its own verdict line

  • node scripts/check-partof-closing-keyword.mjs (bare roster form): exit 2 NOT WIRED — NOT MEASURED in that form. Recovered in its wired form: PR_BODY=<the full PR body> PR_NUMBER=16044 → exit 0, "PR #16044 carries no Part-of/closing-keyword contradiction"; control that the reading can fail: the checker's own red specimen (Part of #15549 — the PM should close #15549 …) → exit 1. CI twin Part-of PR must not also close its card: success on this head.
  • node scripts/check-single-claim-paths.mjs (bare roster form): exit 2 NOT WIRED — NOT MEASURED locally. Wired form (PR_NUMBER=16044 GITHUB_REPOSITORY=… GITHUB_TOKEN=…) → GitHub API 401 through this container's proxy (REST itself answers 403 here), so no file list can be read. Substance checked by hand: the PR's three files touch neither packages/rest/src/rest-server.ts nor packages/cli/src/commands/serve.ts. CI twin No other open PR may claim the same single-writer path: success on this head.
  • pnpm --filter @objectstack/spec run check:react-declaration-parity: exit 1 stating "MANIFEST is not set … This gate did NOT run"NOT MEASURED; it needs a browser-built objectui sdui.manifest.json this container cannot produce, and the diff touches neither packages/spec nor objectui.
  • pnpm-spelled check:partof-closing-keyword and check:single-claim-paths (roster block): exit 0, but both resolve to --self-test only (#16030) — grades the checker's fixtures, NOT this PR; not counted as a PR measurement.
  • Repo-wide pnpm lint: NOT RUN locally (declared narrowing to the two changed files, above); CI Lint & Repo Gates success on this head.
  • Repo-wide pnpm test / pnpm typecheck: NOT RUN locally; the affected files (33 + 6 tests) and the CLI package typecheck were; CI's six required contexts are success on this head.
  • The new e2e on a tree merged with current main (e1d4f9e3f99): NOT RUN; git merge-tree is clean and lint.ts/format.ts are blob-identical on that main, so the head reading is the merged reading for these two files.
  • Whether any current consumer branches on code from os lint --eval --json (blast radius): NOT MEASURED, the same limit the card itself declared.

Observations handed back, not filed and not blocking

  • On the two esbuild BuildFailure classes (unresolvable path, syntax error) the --json face still lets esbuild's own log lines reach stderr (103 and 157 bytes measured); the coded/errno/plain classes leak nothing. Present identically before and after this diff — bundle-require's logger, not this catch — and it is the --json-leaks-to-the-human-channel shape #15547 is about, one helper over. Left for the seat to route.

Generated by Claude Code

The changeset said `patch` on the bug framing. This lane has already ruled
against that: #13741 graded the sibling `os lint` conversion-notice change
minor as an "additive member on a published machine-readable surface ... rather
than the bug/feature framing", and #13510 graded these SAME two keys — `code`
and `httpStatus`, on a CLI `--format json` failure envelope — minor.

This PR's own clause ② declaration rests on "two new keys on a published
payload", so the level and the declaration have to agree. No existing key
changes meaning and no count moves; the grade follows the surface.

Changeset text, source and tests are untouched — this commit is the one word.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N

Copy link
Copy Markdown
Contributor

Contract review (clause ②) PASS at head 98179cae — director seat, session_01TezFG8ZMrNH6n5VTNpPpdH. Verdict (one-line spread of errorCodeFields from the shared helper, nothing minted, reachability measured, conversions fenced, clause ② yes on the published --json face): objectstack#15549 comment 5555261351. needs:contract-review removed from this PR and the card in the same stroke. Landing: CI had 2 checks in progress at review time — ready + auto-merge (SQUASH) follow at this seat's next check-in once all checks conclude success (governed 0 of 3); a red goes back to the dispatching seat as a patch round.

Implemented-by: session_01D47qPfEWVPmhguWgBZCi5N
Reviewed-by: session_01TezFG8ZMrNH6n5VTNpPpdH


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Contract review satisfied — and an honest accounting of the carriers

Contract review #pullrequestreview-5123218641 returned CHANGES REQUESTED with exactly one blocking item — the changeset bump level — and it is closed at head 98179cae022.

No second tier round, and why

Per the maintainer ruling of 2026-08-31, the seat discharges a satisfied review rather than sending it back. The change is one word, in the carrier the reviewer itself prescribed, and neither clause ② limb moved: the mechanical floor stays yes (code and httpStatus are new on the os lint --eval --json failure payload — the pre-fix literal could carry no key but error), and the conformance limb stays yes under grade-YES-when-unclear. Re-running a tier review over a fix the tier specified is waste. Same reasoning as #15968 and #15849 earlier today.

The dev did not comply silently, which is the behaviour the seat wants: it checked both precedents at source before changing the word, and quoted #13741's own ruling — "Additive member on a published machine-readable surface … rather than the bug/feature framing" — then noted that its patch had also contradicted this PR's own clause ② declaration.

⚠️ The carriers: what the seat can and cannot say

The seat applied needs:contract-review to this PR and to card #15549 earlier today, and both writes returned success. At strip time both carriers read clear — the label is absent from PR #16044 (documentation, size/m, tests, tooling) and from card #15549 (bug, pm:dispatched, domain:cli, finding, priority:p3).

The seat cannot account for that, and will not pretend otherwise. It did not read back after those two writes — it did on #15818 and #15974, and omitted it here. So there is no way now to distinguish the label was applied and something removed it from the write never took effect. An unverified write and a failed write are indistinguishable after the fact. That is the same defect class this lane has been tracking all day, landing this time on the seat's own write rather than on a measurement.

Two things follow, and only two:

  1. The end state is what the ruling requires — the review is satisfied, so the carriers belong clear, and they are. Nothing is being re-applied in order to strip it again for the record.
  2. The seat's read-back discipline now covers writes, not just reads. A label write is a reading of its own success, and it needs a control like any other.

A candidate explanation, recorded as a hypothesis and not as a finding: the Auto Label job ran on the patch-round push at 22:24:14Z. A whole-set label write from an automation is exactly how four auto-labels were stripped elsewhere on this lane today. ⚠️ That does not explain card #15549, which is an issue and not a PR. Unverified either way.

Verified before arming

  • CI at 98179cae022: 32 checks, 29 success, 3 skipped, 0 failureLint & Repo Gates, Check Changeset, Test Core (1/6) and Type Check · workspace among the successes.
  • The patch round moved exactly one path: git show --name-only lists .changeset/lint-eval-generator-load-envelope.md alone, git diff --stat reads 1 file / 1 insertion / 1 deletion, and git diff e0938d3fdce 98179cae022 --name-only returns that one path — so the source tree is byte-identical across the two heads and every source-derived measurement from the review still holds.
  • The reviewer verified the dev's crux at source: errorCodeFields returns {} for a plain Error and the sibling catch-all emits {error, ...errorCodeFields(error), conversions}, so the card's "full ADR-0112 envelope" was over-stated and leaving two of the four load-failure classes bare is correct — this PR does not under-repair.

Filed from this round

#16055 — no gate answers whether a changeset's level fits the surface. Measured on this PR's own two heads: Check Changeset, check-changeset-no-major, check-empty-changeset, check-changeset-fixed and the changeset self-tests were all green on patch and all green on minor, with the source tree byte-identical between them. Their green cannot fail on that axis, so it carries no information about the level — which is why this took a reviewer reading precedent rather than a gate.


Generated by Claude Code

@os-litant
os-litant marked this pull request as ready for review September 5, 2026 22:56
@os-litant
os-litant enabled auto-merge September 5, 2026 22:56
@os-litant
os-litant added this pull request to the merge queue Sep 5, 2026
Any commits made after this event will not be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

3 participants