fix(cli): carry the ADR-0112 error carriers on os lint --eval --json's generator-load exit - #16044
fix(cli): carry the ADR-0112 error carriers on os lint --eval --json's generator-load exit#16044os-litant wants to merge 2 commits into
os lint --eval --json's generator-load exit#16044Conversation
…`'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
📓 Docs Drift CheckThis PR changes 1 package(s): 20 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # 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
|
os-litant
left a comment
There was a problem hiding this comment.
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.
codeandhttpStatusare new keys on a published payload — the pre-fix literalemitJson({ error: msg }, 0, { compact: true })(bloba0bb8e2046c, identical onmainate1d4f9e3f99) could carry no key buterror, 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 jsonfailure 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.tserrorCodeFields(lines 246–254, unchanged by this PR): starts from{}, returns{}for an exit signal, setscodeonly for a non-empty string, setshttpStatusonly for an integer. It returns nothing for a plainError.packages/cli/src/commands/lint.tslines 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
Errorfrom the hand-thrown "must default-export a function" and esbuild'sBuildFailurecarry 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 putcode/httpStatuson CLI--format jsonfailure envelopes):"@objectstack/cli": minor.- Commit
8aaa118d1f5(#13741,conversionsonos 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
--jsonpayload"; the in-seat review rules (contract-review.mditem ②) 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: patch → minor 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
conversionsdeliberately not added — correct. #14015's disposition names addingconversionsto the--evalexits as branch 2, "⛔ out of scope … needs its own card and an at-tier contract review", and the testthe key set is exactly the carrierspins that fence from this side. Adding it here would have widened a second key set under a card that fences it.- No
isExitSignalre-throw added — acceptable as delivered. Read: thetrycontains onlybundleRequire(...)and onethrow new Error(...); nothing in it callsthis.exit(). Measured: even if an exit-signal-shaped error did arrive (rows 6–7),errorCodeFieldsreturns{}, so noEEXITcan 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 ownReconciliation — 56 famil(ies)line. Result: 56/56 green — 29node scripts/…families and 27pnpm 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-parityandcheck:dual-build-cjs-loadsran against a builtdist/and returned real verdict lines (exit 0; zeroPREREQUISITElines 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-keywordandcheck:single-claim-pathsresolve to--self-testonly (#16030) — their green grades the checkers, not this PR; the PR-judging invocations are the barenodeforms, reported below by name. pnpm --filter @objectstack/cli typecheckexit 0, and the new test file is inside the type-checked set (1 hit undertsconfig.test.json --listFiles).- vitest at head over the new e2e,
lint-eval-json-unscorable-stack.e2eandformat.error-code-fields: 20/20 across the two e2e files (7 new + 13 sibling) and 13/13 insrc/utils/format.error-code-fields.test.ts— the 33 the PR reports, reproduced;src/commands/meta/delete-json-error-code.test.ts6/6 alongside. - ESLint over the two changed source files: 2 files, 0 errors, 0 warnings, neither ignored (
--print-configresolves for both). Repo-widepnpm lintis the CI job's:Lint & Repo Gatescompletedsuccesson this head at 22:02Z. - Docs: no hand-written page states this payload's shape (
Failed to load generator/--generator/--eval: 0 hits acrosscontent/docsand the CLI README; the drift bot's 24 rows re-derived locally and grepped). Nothing to update. - Base branch:
mainmoved from2e357650306toe1d4f9e3f99(three commits: #16034, #15980, #15968) during this review; none toucheslint.tsorformat.ts, andgit merge-treeof the head onto it is clean. On this head every one of the 37 CI check runs has completed and every non-skipped one issuccess, 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.tsnorpackages/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 twinPart-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 neitherpackages/rest/src/rest-server.tsnorpackages/cli/src/commands/serve.ts. CI twinNo 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 objectuisdui.manifest.jsonthis container cannot produce, and the diff touches neitherpackages/specnor objectui.- pnpm-spelled
check:partof-closing-keywordandcheck:single-claim-paths(roster block): exit 0, but both resolve to--self-testonly (#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); CILint & Repo Gatessuccess 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-treeis clean andlint.ts/format.tsare blob-identical on thatmain, so the head reading is the merged reading for these two files. - Whether any current consumer branches on
codefromos 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
BuildFailureclasses (unresolvable path, syntax error) the--jsonface 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
|
Contract review (clause ②) PASS at head Implemented-by: Generated by Claude Code |
Contract review satisfied — and an honest accounting of the carriersContract review #pullrequestreview-5123218641 returned CHANGES REQUESTED with exactly one blocking item — the changeset bump level — and it is closed at head No second tier round, and whyPer 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 ( 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
|
Fixes #15549
os lint --eval --json's generator-load failure emitted a bare{error}. Thecatcharound the--generatorload 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 readscodeto branch got a real code from the same command's project-lint catch-all andundefinedfrom eval mode.The change
One line, plus its reasoning. The exit now spreads
errorCodeFields(error)— the same helperrun()'s project-lint catch-all spreads, rather than a second shape written by hand here:Nothing is minted.
errorCodeFieldspasses 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:code+httpStatus{"error":"…: upstream refused the model"}{"error":"…","code":"FORBIDDEN","httpStatus":403}{"error":"…: ENOENT: no such file…"}{"error":"…","code":"ENOENT"}{"error":"…: module must default-export a function (prompt, id) => stack"}Error, nothing to pass through{"error":"…: Build failed with 1 error…"}BuildFailurecarries neither keyExit 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 againstbundleRequiredirectly — a generator whose top-level evaluation throws propagates its error intact, soENOENTand a fullFORBIDDEN/403pair both arrive there, and both were being dropped. esbuild's own failure object has onlyerrors/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, nodist/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 carriersadditionally pins the #14015 fence from this side:conversionsis 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-objectmoved off the HEAD blob; injected-text count 1, removed-text count 0), restore confirmed byte-identical (git diff HEADempty, hash back tob8e0dbef945):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.jsloads../src/…through tsx, so bothcommands/lint.tsandutils/format.tsare read from source by the child.Clause ② — declared per limb, from the delivered diff
codeandhttpStatus, on a published--jsonpayload, and the table above measures them arriving. That is the "new key on a published payload" shape exactly. (Nothing underpackages/spec/src/**is touched; the floor is tripped by the new key, not by that path.)⛔ The
needs:contract-reviewlabel is not applied here — that is the seat's to place on the card and the PR together.Verification
Gate union derived at
e0938d3fdcewithnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, asserted against its ownReconciliation — 56 famil(ies)line: 56 commands emitted, 56 families reconciled. All exit codes captured before any pipe.check:i18n,check:i18n-coverage,check:i18n-walk-parity,check:dual-build-cjs-loads) first answered PREREQUISITE NOT MET on an unbuiltdist/; the CLI and spec were built and all four then returned real verdict lines.check-partof-closing-keyword.mjsandcheck-single-claim-paths.mjsexit 2 NOT WIRED (they need PR context that exists only in CI), andcheck:react-declaration-paritystates "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 typecheckgreen, and the new test file is confirmed inside the type-checked set (1 hit undertsconfig.test.json --listFiles), not merely adjacent to it.vitest runover the affected files — the new e2e, the siblinglint-eval-json-unscorable-stack.e2e, andformat.error-code-fields— 33/33 pass.pnpm lintis 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 (
resolveConfigPathprinting human text to stdout), theconversionssibling fenced by #14015, and #15550 (--generator's "Requires --eval." claim). This diff touches neitherpackages/rest/src/rest-server.tsnorpackages/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