Skip to content

Commit 923caed

Browse files
claude[bot]claude
andauthored
fix(cli): os lint --eval --json stops leaking esbuild's diagnostics to stderr, and the pin now covers every door (#16855)
* fix(cli): `os lint --eval --json` no longer leaks esbuild's diagnostics to stderr The `--generator` load calls `bundleRequire`, and esbuild's own logger writes to stderr from inside that call — before anything throws, so the `catch` that builds the one-key `{error}` document never gets a chance to suppress it. Pass `esbuildOptions: { logLevel: 'silent' }` to that one call site, and only when `--json` is set. esbuild still THROWS its `BuildFailure`, so the refusal is unchanged; the human face passes no new option at all and is byte-identical. Extends `lint-eval-generator-load-envelope.e2e.test.ts` to the two doors its `expect(run.stderr).toBe('')` did not drive: the unresolvable path, and a generator that loads fine but makes esbuild warn. Each carries its own negative control on the same run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * chore(changeset): declare the `os lint --eval --json` stderr silence Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 429ec1e commit 923caed

3 files changed

Lines changed: 179 additions & 0 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
`os lint --eval --json` no longer leaks esbuild's own diagnostics to stderr while loading a `--generator` module.
6+
7+
A `--json` invocation is a machine face, and its stdout document was already well-formed — but the `--generator` load runs through `bundleRequire`, and esbuild's logger writes straight to stderr from inside that call, before anything throws. The `catch` that builds the one-key `{error}` document therefore never got a chance to suppress it, and a caller who asked for JSON got an internal bundler's diagnostic on the human channel alongside it.
8+
9+
Measured on `bin/run-dev.js` with `NO_COLOR=1`, two runs that both leaked:
10+
11+
- an unresolvable `--generator` path: exit 1, a well-formed `{error}` on stdout, and `✘ [ERROR] Could not resolve "<path>"` on stderr;
12+
- a generator that bundles and loads *successfully* but makes esbuild warn: exit 0, the full live eval report on stdout, and 340 bytes of `▲ [WARNING] …` on stderr. Nothing throws on this path at all, so no error handling was ever involved.
13+
14+
The load now passes `esbuildOptions: { logLevel: 'silent' }`, scoped to that one call site and applied only when `--json` is set.
15+
16+
- **The refusal is unchanged.** `logLevel` governs whether esbuild *prints*; it still throws its `BuildFailure` with `errors` populated, and that text already forms the tail of the `{error}` string on stdout. Both stdout documents above are byte-identical before and after.
17+
- **The human face is untouched**, by construction rather than by restating a default: without `--json` no `esbuildOptions` is passed at all. `os lint --eval --generator <bad>` still prints esbuild's line on stderr exactly as before.
18+
- **What is suppressed beyond the leak itself:** under `--json`, an esbuild *warning* on a generator that loads fine now reaches nothing. A warning is not thrown, so no handler carries it onto stdout. This is inside the defect rather than beyond it — the machine face is not a place for human-channel output — but a `--json` consumer that was reading stderr for bundler warnings will no longer see them.
19+
- The other `bundleRequire` callers in the CLI (`os serve` / `os dev`, config loading, scaffold validation) are not affected and keep their diagnostics.

packages/cli/src/commands/lint.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -960,6 +960,61 @@ export default class Lint extends Command {
960960
const { mod } = await bundleRequire({
961961
filepath: flags.generator,
962962
external: BUNDLE_REQUIRE_EXTERNALS,
963+
// [#16358] Under `--json` this call site had TWO channels, and only
964+
// one of them was ours. esbuild's own logger writes straight to
965+
// stderr from inside the bundle, BEFORE anything throws, so the
966+
// `catch` below — which does produce a correct one-key `{error}`
967+
// document on stdout — never gets the chance to suppress it.
968+
// Measured on this entry at 7f96e1417e, both doors:
969+
//
970+
// os lint --eval --json --generator /tmp/os16358/nope.mjs
971+
// exit 1 · stdout 143 B (well-formed `{error}`) · stderr 55 B
972+
// `✘ [ERROR] Could not resolve "/tmp/os16358/nope.mjs"`
973+
// os lint --eval --json --generator ./warns-but-loads.mjs
974+
// exit 0 · stdout 3158 B (the eval report) · stderr 340 B
975+
// `▲ [WARNING] The "typeof" operator will never evaluate to …`
976+
//
977+
// ⇒ the leak is NOT confined to the failure branch. `--json` is a
978+
// machine face; anything on stderr is a human-channel emission the
979+
// caller did not ask for, and both of those are an internal
980+
// bundler's diagnostic rather than an ObjectStack refusal.
981+
//
982+
// ⛔ NOTHING is lost from the refusal. esbuild still THROWS its
983+
// `BuildFailure` with `errors` populated — `logLevel` governs only
984+
// whether esbuild PRINTS — and that message is already the tail of
985+
// the `{error}` string the `catch` builds:
986+
// `… Build failed with 1 error:\nerror: Could not resolve "…"`.
987+
// Silencing the logger must not silence the refusal, and it does
988+
// not; `test/lint-eval-generator-load-envelope.e2e.test.ts` pins
989+
// both halves on the same run.
990+
//
991+
// ⚠️ WHAT THIS DOES SUPPRESS, stated rather than shipped quietly:
992+
// under `--json`, an esbuild WARNING on a generator that loads fine
993+
// (the second run above) reached stderr before and now reaches
994+
// nothing — a warning is not thrown, so no `catch` carries it onto
995+
// stdout. That is inside the defect, not beyond it: the property
996+
// the sibling pin's comment states is about the `--json` face as a
997+
// whole, not about its error branch.
998+
//
999+
// ⛔ The human face is NOT touched, and is not touched BY
1000+
// CONSTRUCTION rather than by restating a default: when `--json` is
1001+
// absent this passes no `esbuildOptions` at all, so bundle-require's
1002+
// own esbuild defaults apply exactly as before. A `logLevel:
1003+
// 'warning'` written out here would be me copying a default I would
1004+
// then own.
1005+
//
1006+
// ⛔ Scope is this ONE call site. The other `bundleRequire` callers
1007+
// in this package (`utils/config.ts`, `utils/scaffold-validate.ts`,
1008+
// `commands/serve.ts`) keep their diagnostics; a global esbuild
1009+
// silence would trade one under-read for a larger one.
1010+
//
1011+
// Why `logLevel` and not the two alternatives the card left open:
1012+
// esbuild's JS API exposes no logger hook to install (its whole
1013+
// logging surface is `logLevel` plus the `errors`/`warnings` arrays
1014+
// on the result), and capturing `process.stderr.write` around an
1015+
// await is a process-global monkey-patch that would swallow
1016+
// concurrent writes that are not esbuild's.
1017+
...(flags.json ? { esbuildOptions: { logLevel: 'silent' as const } } : {}),
9631018
});
9641019
const fn = (mod as any).default ?? (mod as any).generate;
9651020
if (typeof fn !== 'function') {

packages/cli/test/lint-eval-generator-load-envelope.e2e.test.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,44 @@
4242
* #14015 with its own review gate. `the key set is exactly the carriers` pins
4343
* that fence from this side, so a well-meaning widening goes red here.
4444
*
45+
* ## [#16358] The second property this file pins: the human channel stays EMPTY
46+
*
47+
* `a coded failure at import surfaces BOTH carriers` below ends with
48+
* `expect(run.stderr).toBe('')` under the comment *"A --json run leaks nothing
49+
* to the human channel"*. That comment states a property of the `--json` face
50+
* AS A WHOLE, and it was honest about the one door it drove — a module that
51+
* EXISTS AND THROWS AT IMPORT, where esbuild bundles cleanly and prints
52+
* nothing. The other doors into the same `catch` were uncovered, and they
53+
* leaked: esbuild's own logger writes straight to stderr from inside
54+
* `bundleRequire`, BEFORE anything throws, so the `catch` that builds the
55+
* one-key `{error}` document never gets a chance to suppress it.
56+
*
57+
* Re-driven at `7f96e1417e` before the repair, `bin/run-dev.js`, `NO_COLOR=1`:
58+
*
59+
* os lint --eval --json --generator /tmp/os16358/nope.mjs
60+
* exit 1 · stdout 143 B (well-formed `{error}`) · stderr 55 B
61+
* `✘ [ERROR] Could not resolve "/tmp/os16358/nope.mjs"`
62+
* os lint --eval --json --generator /tmp/os16358/warn.mjs (LOADS FINE)
63+
* exit 0 · stdout 3158 B (the live eval report) · stderr 340 B
64+
* `▲ [WARNING] The "typeof" operator will never evaluate to "null"`
65+
*
66+
* ⇒ same command, same face, three answers — with a green pin asserting the
67+
* one that held. The repair passes `esbuildOptions: { logLevel: 'silent' }`
68+
* to that ONE `bundleRequire` call and ONLY when `--json` is set; the two
69+
* cases below drive the two uncovered doors, and each carries its own
70+
* negative control so a fix that silenced the REFUSAL along with the logger
71+
* goes red here rather than reading green:
72+
*
73+
* - unresolvable path — stderr empty AND the exit is still 1 with the
74+
* well-formed one-key `{error}` naming the unresolved path;
75+
* - warning-only — stderr empty on the `--json` face AND the SAME fixture
76+
* still shows the warning on the HUMAN face. That second leg is what
77+
* keeps the first from going vacuous: if a future esbuild stopped
78+
* emitting `impossible-typeof`, an `stderr === ''` assertion alone would
79+
* stay green while measuring nothing, and the human-face leg reddens
80+
* instead of hiding it. It also pins the scope of the silence — ⛔ the
81+
* repair must not reach the face that asked for human output.
82+
*
4583
* ## Why no `dist/` sits on the measured path
4684
*
4785
* These run the CLI through `bin/run-dev.js`, the SOURCE entry — same CLI, run
@@ -139,6 +177,21 @@ export default function () { return {}; }
139177
const NOT_A_FUNCTION = `export default { nope: true };
140178
`;
141179

180+
/**
181+
* [#16358] Bundles and LOADS successfully, and makes esbuild emit a warning
182+
* while doing it (`impossible-typeof`). Nothing throws here, so no `catch`
183+
* ever sees this diagnostic and nothing carries it onto stdout — before the
184+
* repair it reached stderr on both faces, including the machine one.
185+
*
186+
* The generated stack is deliberately trivial: this fixture measures the
187+
* CHANNEL, not the rubric. `mode: 'live'` in the payload is what proves the
188+
* module was actually loaded and called.
189+
*/
190+
const WARNS_BUT_LOADS = `const probe = 1;
191+
if (typeof probe === 'null') { throw new Error('unreachable'); }
192+
export default function () { return { objects: [] }; }
193+
`;
194+
142195
beforeAll(() => {
143196
dir = mkdtempSync(join(tmpdir(), 'os-lint-eval-envelope-'));
144197
});
@@ -211,6 +264,58 @@ describe('os lint --eval --json — nothing is minted', () => {
211264
}, 120_000);
212265
});
213266

267+
describe('os lint --eval --json — the human channel stays empty on EVERY door [#16358]', () => {
268+
it('the unresolvable path leaks nothing to stderr — and still refuses on stdout', async () => {
269+
// The door the sibling pin above does NOT drive. esbuild never reaches
270+
// module evaluation: it throws a `BuildFailure`, and its logger had
271+
// already written `✘ [ERROR] Could not resolve "…"` to stderr from inside
272+
// `bundleRequire` — 55 bytes measured at 7f96e1417e for this path length
273+
// (the emission is `✘ [ERROR] Could not resolve "<path>"`, so the byte
274+
// count tracks the path; the card's 54 was a 20-character path).
275+
const run = await runJson(join(dir, 'does-not-exist.mjs'));
276+
277+
// The property the card exists for, in the same shape the existing pin
278+
// uses one describe up.
279+
expect(run.stderr).toBe('');
280+
281+
// ⛔ NEGATIVE CONTROL, on the SAME run: silencing esbuild's logger must
282+
// not also silence the refusal. A repair that swallowed the throw would
283+
// satisfy the line above and fail every line below.
284+
const payload = payloadOf(run, 'unresolvable path — stderr pin');
285+
expect(run.code).toBe(1);
286+
expect(Object.keys(payload)).toEqual(['error']);
287+
expect(payload.error).toContain('Failed to load generator');
288+
expect(payload.error).toContain('Could not resolve');
289+
}, 120_000);
290+
291+
it('a generator that only WARNS leaks nothing either — and the human face still shows it', async () => {
292+
const file = generator('warns-but-loads', WARNS_BUT_LOADS);
293+
294+
// The third door: nothing throws at all, so the `catch` is never entered
295+
// and there is no error path to blame. 340 bytes of esbuild warning
296+
// reached stderr here before the repair, on a run that exits 0.
297+
const machine = await runJson(file);
298+
expect(machine.stderr).toBe('');
299+
300+
const payload = payloadOf(machine, 'warning generator — machine face') as unknown as {
301+
mode?: string;
302+
error?: unknown;
303+
};
304+
// The module really was loaded and called — otherwise `stderr === ''`
305+
// above would be measuring a run that never bundled anything.
306+
expect(payload.mode).toBe('live');
307+
expect(payload.error).toBeUndefined();
308+
309+
// ⛔ SCOPE CONTROL: the silence is the machine face's, not the command's.
310+
// The same fixture on the HUMAN face must still show esbuild's warning —
311+
// which also keeps the assertion above from going vacuous if a future
312+
// esbuild stops emitting this diagnostic.
313+
const human = await runLint(['--generator', file]);
314+
expect(human.stderr).toContain('[WARNING]');
315+
expect(human.stderr).toContain('typeof');
316+
}, 120_000);
317+
});
318+
214319
describe('os lint --eval — the untouched controls', () => {
215320
it('the human path is unchanged: still exit 1, still no JSON document', async () => {
216321
const run = await runLint(['--generator', join(dir, 'does-not-exist.mjs')]);

0 commit comments

Comments
 (0)