Skip to content

Commit eb3336e

Browse files
committed
fix(cli): one space after the generator-load separator, on both faces
`os lint --eval --generator ""` refused with `Failed to load generator "": is not a valid JS file` — two spaces. `bundle-require` composes its own message as `${filepath} is not a valid JS file`, so an empty filepath contributes nothing and the fragment's leading space lands against the space in our own `": "` separator. Neither side is wrong alone, and the seam was unreachable before the truthiness-guard repair: that guard skipped the whole load block. The composed message now drops leading spaces from the detail, so the separator carries exactly one. This reads the seam and never `flags.generator`, so the empty string still answers through the door an unresolvable path already answers through, and every detail that does not open with a space is byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
1 parent 923caed commit eb3336e

3 files changed

Lines changed: 268 additions & 1 deletion

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
`os lint --eval --generator ""` no longer prints a double space in its refusal.
6+
7+
`bundle-require` composes its own refusal as `<filepath> is not a valid JS file`, so an
8+
empty filepath contributes no characters and that fragment arrives with a leading space —
9+
which landed against the space in our own `": "` separator:
10+
11+
```
12+
Failed to load generator "": is not a valid JS file # before, both faces
13+
Failed to load generator "": is not a valid JS file # after
14+
```
15+
16+
The composed message now drops leading spaces from the detail, so the separator carries
17+
exactly one. The empty string still answers through the same door an unresolvable path
18+
answers through — same `catch`, same exit code 1, same one-key `{error}` document on the
19+
`--json` face — and every refusal whose detail does not open with a space is byte-identical,
20+
the unresolvable-path case included.

packages/cli/src/commands/lint.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1022,7 +1022,34 @@ export default class Lint extends Command {
10221022
}
10231023
generate = fn;
10241024
} catch (error: any) {
1025-
const msg = `Failed to load generator "${flags.generator}": ${error?.message || error}`;
1025+
// [#16359] Our separator `": "` already carries the ONE space between
1026+
// the quoted value and the reason; the detail must not bring a second.
1027+
// `bundle-require` composes its own refusal as
1028+
// `${filepath} is not a valid JS file`, so an EMPTY filepath
1029+
// contributes no characters and that fragment arrives with a LEADING
1030+
// space, which lands against ours. Re-driven at 923caede80 through
1031+
// `od -c`, both faces, `bin/run-dev.js`, `NO_COLOR=1`:
1032+
//
1033+
// os lint --eval --generator "" -> `generator "": is not a valid JS file`
1034+
// os lint --eval --json --generator "" -> the same two spaces inside `{error}`
1035+
// os lint --eval [--json] --generator <unresolvable path> -> ONE space
1036+
//
1037+
// ⛔ This is NOT a branch on the empty value. #16161 ruled that the
1038+
// empty string must answer through the door an unresolvable path
1039+
// already answers through, and a bespoke message for it would be the
1040+
// second refusal shape that card exists to avoid. The normalisation
1041+
// below reads the SEAM and never `flags.generator`, and applies to
1042+
// every detail alike — so both inputs still reach this one `catch`,
1043+
// this one composition, this one envelope and this one exit code, and
1044+
// every detail that does not open with a space is byte-identical.
1045+
//
1046+
// Leading SPACES only, deliberately not `trimStart()`: a detail that
1047+
// opens with a newline is a different shape (our space then a line
1048+
// break), not a doubled separator, and stays exactly as it prints
1049+
// today. `test/lint-eval-generator-refusal-separator.test.ts` pins the
1050+
// bytes on both faces, with the unresolvable path as the control.
1051+
const detail = `${error?.message || error}`.replace(/^ +/, '');
1052+
const msg = `Failed to load generator "${flags.generator}": ${detail}`;
10261053
// [#15549] The ADR-0112 carriers, spread from the SAME helper the
10271054
// project-lint catch-all in `run()` uses — not a second shape invented
10281055
// here. Before this, the `catch` built `msg` and DISCARDED `error`, so
Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#16359] `os lint --eval --generator ""` printed TWO spaces after our own
5+
* separator, on both faces.
6+
*
7+
* ## The measured before-shape
8+
*
9+
* Re-driven at `923caede80` (this card's branch point) through `od -c`, not by
10+
* eye — a double space is exactly the kind of detail that survives a copy
11+
* badly, and the filing seat did not independently re-drive it. `bin/run-dev.js`,
12+
* `NO_COLOR=1`, both faces:
13+
*
14+
* os lint --eval --generator "" exit 1 · stdout 59 B · stderr 0 B
15+
* ` ✗ Failed to load generator "": is not a valid JS file`
16+
* os lint --eval --json --generator "" exit 1 · stdout 67 B · stderr 0 B
17+
* `{"error":"Failed to load generator \"\": is not a valid JS file"}`
18+
* os lint --eval --generator <unresolvable> ONE space
19+
* os lint --eval --json --generator <unresolvable> ONE space
20+
*
21+
* ⇒ the defect is on BOTH faces, and the non-empty path was already correct.
22+
*
23+
* ## Why the seam exists
24+
*
25+
* `bundle-require` composes its own refusal as `${filepath} is not a valid JS
26+
* file`. An EMPTY filepath contributes no characters, so that fragment arrives
27+
* with a LEADING space and lands against the space in our own `": "`
28+
* separator. Neither side is wrong alone, and the seam was UNREACHABLE before
29+
* #16341 (card #16161) — the truthiness guard skipped the whole load block, so
30+
* no message was printed at all. A defect newly made REACHABLE by a correct
31+
* fix, ⛔ not a regression that fix introduced.
32+
*
33+
* ## What is pinned, and the property that outranks the spacing
34+
*
35+
* #16161 ruled that the empty string must answer through the door an
36+
* unresolvable path already answers through; a bespoke message for the empty
37+
* value would be the second refusal shape that card exists to avoid. So the
38+
* repair normalises the SEAM (leading spaces on the detail) and never branches
39+
* on `flags.generator`, and this file pins BOTH halves:
40+
*
41+
* - the empty value's message bytes, EXACTLY — a `toContain` of a fragment a
42+
* double space would still satisfy is not a pin;
43+
* - ⛔ the same-door property — the empty string and an unresolvable path
44+
* reach the same exit code, the same one-key `{error}` envelope and the
45+
* same `Failed to load generator "…": …` shape. A repair that special-cased
46+
* the empty value could satisfy every spacing assertion here and would fail
47+
* `the same door`.
48+
*
49+
* The unresolvable path is also the NEGATIVE CONTROL for the repair itself:
50+
* that is the direction a seam trim most easily breaks, so its message is
51+
* asserted to still carry exactly one space and its own detail intact.
52+
*
53+
* ## Why this file is queue tier and not `*.e2e.test.ts`
54+
*
55+
* `*.e2e.test.ts` is the NIGHTLY tier (`scripts/nightly-tiers.mjs`): a pin
56+
* there never guards a pull request or a merge-queue entry — deliberate and
57+
* documented, ⛔ not a defect, but not protection for this property either.
58+
* The nightly cut is by FILENAME and the `unit`/`integration` cut is by
59+
* BEHAVIOUR (`packages/cli/vitest-tiers.ts`), so this file — which spawns the
60+
* CLI and carries no `.e2e` name — is collected on every PR and every queue
61+
* entry, in the `integration` project. A `unit`-tier pin would have to test a
62+
* pure helper extracted out of `runEval`, which trades the ACTUAL emitted
63+
* bytes on the actual command for a new exported surface; these assertions are
64+
* about bytes an operator sees, so they are driven through the CLI.
65+
*
66+
* ⛔ The existing `test/lint-eval-generator-load-envelope.e2e.test.ts` pins —
67+
* including its four `stderr).toBe('')` assertions — are neither weakened,
68+
* rewritten nor moved by this card; this is a new file beside them.
69+
*
70+
* ## Why no `dist/` sits on the measured path
71+
*
72+
* These run the CLI through `bin/run-dev.js`, the SOURCE entry — same CLI, run
73+
* from `src/` through tsx — so `commands/lint.ts` and `utils/format.ts` are
74+
* both loaded from source by the child and this change is measured without a
75+
* rebuild.
76+
*/
77+
78+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
79+
import { execFile } from 'node:child_process';
80+
import { mkdtempSync, rmSync } from 'node:fs';
81+
import { tmpdir } from 'node:os';
82+
import { join, resolve } from 'node:path';
83+
import { fileURLToPath } from 'node:url';
84+
import { childEnv } from './helpers/serve-process.js';
85+
86+
const HERE = resolve(fileURLToPath(import.meta.url), '..');
87+
const CLI = resolve(HERE, '../bin/run-dev.js');
88+
const TSX = resolve(HERE, '../../../node_modules/.bin/tsx');
89+
90+
interface Run {
91+
code: number;
92+
stdout: string;
93+
stderr: string;
94+
}
95+
96+
let dir: string;
97+
98+
function runLint(args: string[]): Promise<Run> {
99+
return new Promise((resolvePromise) => {
100+
execFile(
101+
TSX,
102+
[CLI, 'lint', '--eval', ...args],
103+
{ cwd: dir, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) },
104+
(err, stdout, stderr) => {
105+
resolvePromise({
106+
code: err
107+
? typeof (err as { code?: unknown }).code === 'number'
108+
? (err as unknown as { code: number }).code
109+
: 1
110+
: 0,
111+
stdout: String(stdout),
112+
stderr: String(stderr),
113+
});
114+
},
115+
);
116+
});
117+
}
118+
119+
/** The human face prints through `printError`, which writes the line to STDOUT. */
120+
function humanLine(run: Run, label: string): string {
121+
const line = run.stdout.split('\n').find((l) => l.includes('Failed to load generator'));
122+
if (line === undefined) {
123+
throw new Error(
124+
`${label}: no refusal line on stdout (exit ${run.code})\n` +
125+
`stdout: ${JSON.stringify(run.stdout)}\nstderr: ${JSON.stringify(run.stderr)}`,
126+
);
127+
}
128+
return line;
129+
}
130+
131+
function payloadOf(run: Run, label: string): { error?: string } {
132+
try {
133+
return JSON.parse(run.stdout) as { error?: string };
134+
} catch {
135+
throw new Error(
136+
`${label}: stdout was not one JSON document (exit ${run.code}, ${run.stdout.length} bytes)\n` +
137+
`stdout: ${JSON.stringify(run.stdout)}\nstderr: ${JSON.stringify(run.stderr)}`,
138+
);
139+
}
140+
}
141+
142+
beforeAll(() => {
143+
dir = mkdtempSync(join(tmpdir(), 'os-lint-eval-separator-'));
144+
});
145+
146+
afterAll(() => {
147+
rmSync(dir, { recursive: true, force: true });
148+
});
149+
150+
describe('os lint --eval --generator "" — one space after our separator, on both faces', () => {
151+
it('the machine face carries the message BYTE-EXACTLY', async () => {
152+
const run = await runLint(['--json', '--generator', '']);
153+
const payload = payloadOf(run, 'empty generator — machine face');
154+
155+
expect(run.code).toBe(1);
156+
// The whole composed message, not a fragment: two spaces fail this line.
157+
expect(payload.error).toBe('Failed to load generator "": is not a valid JS file');
158+
}, 120_000);
159+
160+
it('the human face carries the same message, byte for byte', async () => {
161+
const run = await runLint(['--generator', '']);
162+
163+
expect(run.code).toBe(1);
164+
// `printError` -> `errorLine` -> ` ✗ ${msg}`, uncoloured under NO_COLOR.
165+
expect(humanLine(run, 'empty generator — human face')).toBe(
166+
' ✗ Failed to load generator "": is not a valid JS file',
167+
);
168+
}, 120_000);
169+
});
170+
171+
describe('os lint --eval — the non-empty detail is untouched [negative control]', () => {
172+
it('an unresolvable path still answers with exactly one space, on the machine face', async () => {
173+
const missing = join(dir, 'does-not-exist.mjs');
174+
const run = await runLint(['--json', '--generator', missing]);
175+
const payload = payloadOf(run, 'unresolvable path — machine face');
176+
177+
expect(run.code).toBe(1);
178+
// The seam, exactly: one space, and esbuild's own first line intact behind
179+
// it. This is the direction a seam trim most easily breaks.
180+
expect(payload.error).toContain(`"${missing}": Build failed with 1 error:`);
181+
// ⛔ and the detail is not otherwise re-flowed.
182+
expect(payload.error).toContain(`error: Could not resolve "${missing}"`);
183+
}, 120_000);
184+
185+
it('an unresolvable path still answers with exactly one space, on the human face', async () => {
186+
const missing = join(dir, 'does-not-exist.mjs');
187+
const run = await runLint(['--generator', missing]);
188+
189+
expect(run.code).toBe(1);
190+
expect(humanLine(run, 'unresolvable path — human face')).toBe(
191+
` ✗ Failed to load generator "${missing}": Build failed with 1 error:`,
192+
);
193+
}, 120_000);
194+
});
195+
196+
describe('os lint --eval — the empty string and an unresolvable path answer through the SAME door [#16161]', () => {
197+
it('same exit code, same envelope, same message shape', async () => {
198+
const empty = await runLint(['--json', '--generator', '']);
199+
const missing = await runLint(['--json', '--generator', join(dir, 'does-not-exist.mjs')]);
200+
201+
const emptyPayload = payloadOf(empty, 'same door — empty');
202+
const missingPayload = payloadOf(missing, 'same door — unresolvable');
203+
204+
// ⛔ The property #16161 ruled on, and the one a "special-case the empty
205+
// value" repair would break while still satisfying every spacing
206+
// assertion above. The empty string is a path that names no module, not a
207+
// separate error class.
208+
expect(empty.code).toBe(missing.code);
209+
expect(empty.code).toBe(1);
210+
expect(Object.keys(emptyPayload)).toEqual(Object.keys(missingPayload));
211+
expect(Object.keys(emptyPayload)).toEqual(['error']);
212+
213+
for (const [label, message] of [
214+
['empty', emptyPayload.error],
215+
['unresolvable', missingPayload.error],
216+
] as const) {
217+
expect(message, label).toMatch(/^Failed to load generator "[^]*?": \S/);
218+
}
219+
}, 120_000);
220+
});

0 commit comments

Comments
 (0)