Skip to content

Commit 9c3fda5

Browse files
os-litantclaude
andauthored
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 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 * chore(changeset): grade the lint eval envelope change minor, not patch 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9c270bb commit 9c3fda5

3 files changed

Lines changed: 273 additions & 1 deletion

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
`os lint --eval --json` now carries the ADR-0112 error carriers on its generator-load failure, instead of a bare `{error}`.
6+
7+
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 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 case a consumer is most likely to be caught by, because the face is present and looks answerable.
8+
9+
The exit now spreads `errorCodeFields(error)`, the same helper the project-lint catch-all spreads, so both failure faces of `os lint` are built from one source rather than two hand-written shapes.
10+
11+
Nothing is minted. `errorCodeFields` passes a producer's code through and returns nothing otherwise — ADR-0112's ledger stays the authority on who may mint a code — so the exit is polymorphic in exactly the way its sibling already is. Measured on the command's own output, across the reachable load-failure classes:
12+
13+
- a generator whose top-level evaluation throws a coded failure (an SDK refusal as the module builds its client at import) now answers `{"error": …, "code": "FORBIDDEN", "httpStatus": 403}`; both keys were being dropped;
14+
- a file the generator reads at import that is missing now answers `code: "ENOENT"`, the errno vocabulary already documented for this command, and no invented HTTP status;
15+
- an unresolvable path or a syntax error — esbuild's own build failure, which carries neither key — still answers a bare `{error}`, as does the hand-thrown "module must default-export a function".
16+
17+
The human (non-`--json`) path, the eval report exit, and offline eval are unchanged.

‎packages/cli/src/commands/lint.ts‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,31 @@ export default class Lint extends Command {
807807
generate = fn;
808808
} catch (error: any) {
809809
const msg = `Failed to load generator "${flags.generator}": ${error?.message || error}`;
810-
if (flags.json) await emitJson({ error: msg }, 0, { compact: true });
810+
// [#15549] The ADR-0112 carriers, spread from the SAME helper the
811+
// project-lint catch-all in `run()` uses — not a second shape invented
812+
// here. Before this, the `catch` built `msg` and DISCARDED `error`, so
813+
// a machine consumer that reads `code` to branch got a real code from
814+
// project-lint mode and `undefined` from eval mode, on one command.
815+
//
816+
// ⛔ Nothing is MINTED. `errorCodeFields` passes a producer's code
817+
// through and returns `{}` otherwise — ADR-0112's ledger is the
818+
// authority on who may mint one — so this exit stays polymorphic in
819+
// exactly the way its sibling is: the hand-thrown "must default-export
820+
// a function" above carries neither key and still gets neither.
821+
//
822+
// The keys are REACHABLE here, which is what makes this a repair and
823+
// not a formality. Measured against `bundleRequire` on this entry: a
824+
// generator whose TOP-LEVEL EVALUATION throws propagates that error
825+
// intact, so `code: "ENOENT"` (a file the module read at import) and a
826+
// full `code` + `httpStatus` pair (an SDK refusal at import) both
827+
// arrive here — and both were being dropped. esbuild's own
828+
// `BuildFailure` — the unresolvable-path and syntax-error cases —
829+
// carries neither key, and still correctly emits a bare `{error}`.
830+
//
831+
// ⛔ `conversions` is NOT added alongside them: that key on the
832+
// `--eval` exits is a different card, fenced by #14015 with its own
833+
// review gate. This changes the carriers only.
834+
if (flags.json) await emitJson({ error: msg, ...errorCodeFields(error) }, 0, { compact: true });
811835
else printError(msg);
812836
process.exit(1);
813837
}
Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `os lint --eval --json`'s generator-load failure emitted a bare `{error}`.
5+
*
6+
* ## The measured before-shape
7+
*
8+
* The `catch` around the `--generator` load built its message and then
9+
* DISCARDED the error object, so the one eval exit that does have a machine
10+
* face was off-envelope. Driven on this entry before the fix, with a generator
11+
* whose top-level evaluation throws a fully coded failure:
12+
*
13+
* os lint --eval --json --generator ./toplevel-coded.mjs
14+
* exit 1 · {"error":"Failed to load generator \"…\": upstream refused the model"}
15+
*
16+
* ⇒ `code` and `httpStatus` were both present on the thrown error and neither
17+
* reached the payload. A consumer that reads `code` to branch got a real code
18+
* from the project-lint catch-all and `undefined` from eval mode, on the same
19+
* command — the case a consumer is most likely to be caught by, because the
20+
* face is present and looks answerable.
21+
*
22+
* ## What is pinned, and what is deliberately NOT
23+
*
24+
* The fix spreads `errorCodeFields(error)` — the SAME helper `run()`'s
25+
* project-lint catch-all spreads. That helper PASSES A CODE THROUGH and mints
26+
* nothing, so the repaired exit is polymorphic in exactly the way its sibling
27+
* is, and this file pins BOTH halves:
28+
*
29+
* - positive — an error that CARRIES the keys now surfaces them;
30+
* - ⛔ negative — an error that carries neither still gets neither. A "fix"
31+
* that minted a placeholder code for every load failure would satisfy the
32+
* card's headline and hand consumers a vocabulary no ADR-0112 ledger
33+
* declares. `nothing is minted` fails that fix directly.
34+
*
35+
* The negatives are not decoration: TWO of the four reachable load-failure
36+
* classes carry nothing. esbuild's `BuildFailure` (unresolvable path, syntax
37+
* error) has only `errors`/`warnings`, and the hand-thrown "module must
38+
* default-export a function" is a plain `Error`. Both must stay bare.
39+
*
40+
* ⛔ `conversions` is NOT asserted as present and must not be added by a later
41+
* edit here: that key on the `--eval` exits is a different card, fenced by
42+
* #14015 with its own review gate. `the key set is exactly the carriers` pins
43+
* that fence from this side, so a well-meaning widening goes red here.
44+
*
45+
* ## Why no `dist/` sits on the measured path
46+
*
47+
* These run the CLI through `bin/run-dev.js`, the SOURCE entry — same CLI, run
48+
* from `src/` through tsx — so `commands/lint.ts` and `utils/format.ts` are
49+
* both loaded from source by the child and this change is measured without a
50+
* rebuild.
51+
*/
52+
53+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
54+
import { execFile } from 'node:child_process';
55+
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
56+
import { tmpdir } from 'node:os';
57+
import { join, resolve } from 'node:path';
58+
import { fileURLToPath } from 'node:url';
59+
import { childEnv } from './helpers/serve-process.js';
60+
61+
const HERE = resolve(fileURLToPath(import.meta.url), '..');
62+
const CLI = resolve(HERE, '../bin/run-dev.js');
63+
const TSX = resolve(HERE, '../../../node_modules/.bin/tsx');
64+
65+
interface Run {
66+
code: number;
67+
stdout: string;
68+
stderr: string;
69+
}
70+
71+
let dir: string;
72+
73+
function generator(name: string, source: string): string {
74+
const file = join(dir, `${name}.mjs`);
75+
writeFileSync(file, source, 'utf8');
76+
return file;
77+
}
78+
79+
function runLint(args: string[]): Promise<Run> {
80+
return new Promise((resolvePromise) => {
81+
execFile(
82+
TSX,
83+
[CLI, 'lint', '--eval', ...args],
84+
{ cwd: dir, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) },
85+
(err, stdout, stderr) => {
86+
resolvePromise({
87+
code: err
88+
? typeof (err as { code?: unknown }).code === 'number'
89+
? (err as unknown as { code: number }).code
90+
: 1
91+
: 0,
92+
stdout: String(stdout),
93+
stderr: String(stderr),
94+
});
95+
},
96+
);
97+
});
98+
}
99+
100+
const runJson = (generatorPath: string) => runLint(['--json', '--generator', generatorPath]);
101+
102+
interface ErrorPayload {
103+
error?: string;
104+
code?: unknown;
105+
httpStatus?: unknown;
106+
}
107+
108+
/** stdout as ONE JSON document, or a failure that quotes what was there instead. */
109+
function payloadOf(run: Run, label: string): ErrorPayload {
110+
try {
111+
return JSON.parse(run.stdout) as ErrorPayload;
112+
} catch {
113+
throw new Error(
114+
`${label}: stdout was not one JSON document (exit ${run.code}, ${run.stdout.length} stdout bytes)\n` +
115+
`stdout: ${JSON.stringify(run.stdout)}\nstderr: ${JSON.stringify(run.stderr)}`,
116+
);
117+
}
118+
}
119+
120+
/**
121+
* A generator whose TOP-LEVEL evaluation throws a fully coded failure — the
122+
* shape an SDK refusal takes when a live generator module builds its client at
123+
* import time. The error propagates out of `bundleRequire` with both carriers
124+
* intact; before the fix both were dropped.
125+
*/
126+
const CODED_THROW = `const e = new Error('upstream refused the model');
127+
e.code = 'FORBIDDEN';
128+
e.httpStatus = 403;
129+
throw e;
130+
`;
131+
132+
/** The errno vocabulary `format.ts` names for `os lint` — a file read at import. */
133+
const ERRNO_THROW = `import { readFileSync } from 'node:fs';
134+
readFileSync('/definitely/not/here.json');
135+
export default function () { return {}; }
136+
`;
137+
138+
/** Loads fine, exports the wrong thing — the hand-thrown plain `Error`. */
139+
const NOT_A_FUNCTION = `export default { nope: true };
140+
`;
141+
142+
beforeAll(() => {
143+
dir = mkdtempSync(join(tmpdir(), 'os-lint-eval-envelope-'));
144+
});
145+
146+
afterAll(() => {
147+
rmSync(dir, { recursive: true, force: true });
148+
});
149+
150+
describe('os lint --eval --json — the generator-load failure carries the ADR-0112 envelope', () => {
151+
it('a coded failure at import surfaces BOTH carriers on the --json face', async () => {
152+
const run = await runJson(generator('toplevel-coded', CODED_THROW));
153+
const payload = payloadOf(run, 'coded throw');
154+
155+
expect(run.code).toBe(1);
156+
expect(payload.error).toContain('Failed to load generator');
157+
expect(payload.error).toContain('upstream refused the model');
158+
159+
// The two keys the card exists for. Bare `{error}` before the fix.
160+
expect(payload.code).toBe('FORBIDDEN');
161+
expect(payload.httpStatus).toBe(403);
162+
163+
// A --json run leaks nothing to the human channel.
164+
expect(run.stderr).toBe('');
165+
}, 120_000);
166+
167+
it('an errno thrown at import passes through as `code`', async () => {
168+
const run = await runJson(generator('toplevel-errno', ERRNO_THROW));
169+
const payload = payloadOf(run, 'errno throw');
170+
171+
expect(run.code).toBe(1);
172+
expect(payload.code).toBe('ENOENT');
173+
// Pass-through, not minting: the errno carries no HTTP status and none is
174+
// invented for it.
175+
expect(payload.httpStatus).toBeUndefined();
176+
}, 120_000);
177+
178+
it('the key set is exactly the carriers — no `conversions`, no filler', async () => {
179+
// Pins the #14015 fence from this side, and pins that the spread adds the
180+
// two carriers and nothing else.
181+
const run = await runJson(generator('key-set', CODED_THROW));
182+
const payload = payloadOf(run, 'key set');
183+
184+
expect(Object.keys(payload).sort()).toEqual(['code', 'error', 'httpStatus']);
185+
}, 120_000);
186+
});
187+
188+
describe('os lint --eval --json — nothing is minted', () => {
189+
it('the hand-thrown "must default-export a function" stays exactly `{error}`', async () => {
190+
// The load SUCCEEDS and the export is wrong, so the error is our own plain
191+
// `Error`. It carries no code, so it gets none — the same answer the
192+
// project-lint catch-all gives for its own hand-thrown refusals.
193+
const run = await runJson(generator('not-a-function', NOT_A_FUNCTION));
194+
const payload = payloadOf(run, 'not a function');
195+
196+
expect(run.code).toBe(1);
197+
expect(payload.error).toContain('module must default-export a function');
198+
expect(Object.keys(payload)).toEqual(['error']);
199+
}, 120_000);
200+
201+
it("esbuild's BuildFailure carries neither key, so the payload stays bare", async () => {
202+
// An unresolvable path never reaches module evaluation: esbuild throws a
203+
// `BuildFailure` whose own keys are `errors`/`warnings` only. Asserted so a
204+
// later edit cannot "improve" this into a minted code.
205+
const run = await runJson(join(dir, 'does-not-exist.mjs'));
206+
const payload = payloadOf(run, 'unresolvable path');
207+
208+
expect(run.code).toBe(1);
209+
expect(payload.error).toContain('Failed to load generator');
210+
expect(Object.keys(payload)).toEqual(['error']);
211+
}, 120_000);
212+
});
213+
214+
describe('os lint --eval — the untouched controls', () => {
215+
it('the human path is unchanged: still exit 1, still no JSON document', async () => {
216+
const run = await runLint(['--generator', join(dir, 'does-not-exist.mjs')]);
217+
218+
expect(run.code).toBe(1);
219+
expect(run.stdout).toContain('Failed to load generator');
220+
expect(() => JSON.parse(run.stdout)).toThrow();
221+
}, 120_000);
222+
223+
it('offline eval is untouched: exit 0 and a report, not an error envelope', async () => {
224+
const run = await runLint(['--json']);
225+
const payload = payloadOf(run, 'offline baseline') as unknown as { ok?: boolean };
226+
227+
expect(run.code).toBe(0);
228+
expect(payload.ok).toBe(true);
229+
expect((payload as ErrorPayload).error).toBeUndefined();
230+
}, 120_000);
231+
});

0 commit comments

Comments
 (0)