Skip to content

Commit d03fdbf

Browse files
committed
fix(cli): refuse an empty --generator in os lint --eval instead of running offline
`runEval` guarded the generator load with a truthiness test, so `--generator ""` fell through it: the module was never loaded, no warning was printed, and the `Failed to load generator` message that exists for exactly this failure was never reached. Driven before the change on both entries, from a lint-clean project, with a generator that writes a marker at top-level evaluation: os lint --eval --generator "" exit 0 · Mode: offline · 5/5 passed · marker ABSENT os lint --eval exit 0 · Mode: offline · 5/5 passed · marker ABSENT Normalise the elapsed-time token and those two stdouts were byte-identical — one sha256 across `bin/run-dev.js` and `bin/run.js` alike — stderr was 0 bytes in all four runs, and the `--json` face differed only in `duration`. So an empty generator was indistinguishable from not passing the flag, on every channel the command has, while the report said `Mode: offline` to an operator who had asked for a live run. The guard now tests `!== undefined`, the same test the `--generator` precondition guard one frame up uses, so one flag has one rule for "the operator typed it". #15550 settled that rule for the non-eval side; this applies it to the eval side rather than reopening it. No new refusal shape is introduced: an empty string is a path that names no module, so it answers through the load path an unresolvable path already answered through — exit 1, the reason on `error`, one JSON document under `--json`, and no minted ADR-0112 code. The pins hold both directions: the refusal on both faces, the distinguishability the card is actually about, the two sides answering `--generator ""` alike, and the negatives — a real generator still loads and runs live, offline `--eval` and a plain lint are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
1 parent 4a1a3b0 commit d03fdbf

3 files changed

Lines changed: 342 additions & 1 deletion

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
**BREAKING** `os lint --eval --generator ""` now refuses instead of quietly running the offline eval, matching the rule the same flag already follows without `--eval`.
6+
7+
Eval mode guarded the generator load with a truthiness test, so an empty string fell straight through it: the module was never loaded, no warning was printed, and the `Failed to load generator` message that exists for exactly this failure was never reached. What came out was the ordinary offline report — `Mode: offline`, `5/5 passed · mean 99/100`, exit 0 — to someone who had asked for a live run and read that score as their generator's.
8+
9+
It was not merely ineffective. Driven against the same command with the flag absent entirely, and with the elapsed-time token normalised, the two runs produced byte-identical stdout, empty stderr and the same exit code on every face the command has, `--json` included. There was no channel on which the difference was visible. The usual way to type it is `--generator "$GEN"` in a script where `GEN` is unset.
10+
11+
The guard now tests whether the flag was provided rather than whether its value is truthy — the same test `os lint --generator` outside `--eval` has used since it started refusing — so one flag has one rule for "the operator typed it". No new failure shape is introduced: an empty string is a path that names no module, so it answers through the load path an unresolvable path already answered through, with the reason on `error`, exit 1, and on `--json` a single JSON document. No error code is invented for it.
12+
13+
A scripted invocation that passed an empty `--generator` to `os lint --eval` now exits 1 with the reason, where it previously exited 0 having silently scored the bundled corpus instead. Every other invocation is untouched: `--eval --generator <module>` still loads the module and scores live output, `--eval` alone still scores the bundled corpus offline, and a plain project lint is unchanged.
14+
15+
<!-- adr-0087: not-required (no-migration-prescription) The change narrows what one CLI flag value is accepted at invocation time. No metadata surface, stored row or spec declaration is touched, so `objectstack migrate meta` has nothing to carry and the ledger has nothing to record. -->

packages/cli/src/commands/lint.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,37 @@ export default class Lint extends Command {
891891
private async runEval(flags: any, timer: ReturnType<typeof createTimer>): Promise<void> {
892892
let generate: ((prompt: string, id: string) => unknown | Promise<unknown>) | undefined;
893893

894-
if (flags.generator) {
894+
// [#16161] `!== undefined`, not truthiness — the SAME test the
895+
// `--generator` precondition guard in `run()` above uses, so one flag has
896+
// one rule for "the operator typed it".
897+
//
898+
// Driven on this entry before this change, from the probe project below,
899+
// with a generator that writes a marker file at TOP-LEVEL evaluation:
900+
//
901+
// os lint --eval --generator "" exit 0 · Mode: offline · 5/5 passed · marker ABSENT
902+
// os lint --eval exit 0 · Mode: offline · 5/5 passed · marker ABSENT
903+
//
904+
// Normalise the elapsed-time token and those two stdouts were BYTE-IDENTICAL
905+
// (one sha256 across both entries, `bin/run-dev.js` and `bin/run.js`);
906+
// stderr was 0 bytes in all four runs and the `--json` face differed only in
907+
// `duration`. So the empty string was not merely ineffective — it was
908+
// indistinguishable from not passing the flag, on every channel this command
909+
// has, while the report said `Mode: offline` to an operator who had asked for
910+
// a live run. The classic way to type it is `--generator "$GEN"` with `GEN`
911+
// unset in a script.
912+
//
913+
// ⛔ The opposite rule — empty means "not passed" — is not open here. #15550
914+
// settled it for the non-eval side one guard up, and the two sides read one
915+
// flag; splitting them would put two spellings of `--generator` under two
916+
// rules. Reversing it is a decision, not a patch.
917+
//
918+
// ⛔ No new refusal shape is invented for the empty case. Once the load is
919+
// attempted, an unresolvable path answers the way an unresolvable path
920+
// already answers here — the `catch` below, exit 1, `Failed to load
921+
// generator ""` on both faces. That is the same envelope
922+
// `--generator ./does-not-exist.mjs --eval` has answered with all along; an
923+
// empty string is a path that names no module, not a separate error class.
924+
if (flags.generator !== undefined) {
895925
try {
896926
const { mod } = await bundleRequire({
897927
filepath: flags.generator,
Lines changed: 296 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,296 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `os lint --eval --generator ""` ran the offline eval and said nothing.
5+
*
6+
* ## The measured before-shape
7+
*
8+
* `runEval` guarded the generator load with a TRUTHINESS test, so an empty
9+
* string fell through it: no load, no else branch, no warning, and the
10+
* `Failed to load generator` message that exists for exactly this kind of
11+
* failure was never reached. Driven on both entries before the fix, from the
12+
* lint-clean probe project below, with a generator that writes a marker file at
13+
* TOP-LEVEL evaluation so "was it loaded?" is answered by the filesystem
14+
* instead of by reading control flow:
15+
*
16+
* os lint --eval --generator "" exit 0 · Mode: offline · 5/5 passed · marker ABSENT
17+
* os lint --eval exit 0 · Mode: offline · 5/5 passed · marker ABSENT
18+
*
19+
* ⇒ and the two were not merely alike. Normalise the elapsed-time token and
20+
* their stdouts were BYTE-IDENTICAL — one sha256 across `bin/run-dev.js` and
21+
* `bin/run.js` alike — stderr was 0 bytes in all four runs, and the `--json`
22+
* face differed only in `duration`. Passing an empty generator was
23+
* INDISTINGUISHABLE from not passing the flag, on every channel this command
24+
* has, while the report said `Mode: offline` to an operator who had asked for a
25+
* live run and read `5/5 passed` as their generator's score.
26+
*
27+
* ## What is pinned, and why the negatives are not decoration
28+
*
29+
* The fix NARROWS the accept set — `--generator ""` goes from accepted to
30+
* refused — so the pins hold both directions:
31+
*
32+
* - positive — the refusal happens, on both faces, in the envelope this
33+
* command already answers with (#16044): the human message on `error`,
34+
* exit 1, `--json` stdout still one JSON document.
35+
* - ⭐ distinguishability — the empty-generator run and the no-flag run must
36+
* DIFFER once the elapsed token is normalised. That is the property this
37+
* card is actually about, and it is the assertion that was byte-for-byte
38+
* false before the fix.
39+
* - ⭐ symmetry — the eval and non-eval sides must answer `--generator ""`
40+
* the same way. #15550 / PR #16115 gave the non-eval side a `!== undefined`
41+
* test; this card is that ruling reaching the eval side. One flag, one rule
42+
* for "the operator typed it". The two refusal MESSAGES differ, and should
43+
* — they refuse different things — so the pin is on the disposition, not
44+
* the prose: exit code, refusal on stdout, generator never loaded, and the
45+
* `--json` key set. It fails loudly if either side drifts again.
46+
* - ⛔ negative — a real `--eval --generator <module>` still LOADS (marker
47+
* present, `Mode: live`), offline `--eval` and a plain project lint are
48+
* untouched, and an unresolvable path refuses exactly as it always has. A
49+
* "fix" that refused too broadly fails these directly.
50+
*
51+
* ⛔ `nothing is minted` pins the ADR-0112 restraint: `errorCodeFields` passes a
52+
* producer's code through and returns `{}` otherwise, and bundle-require's
53+
* rejection of an empty path carries neither key — so the payload's key set is
54+
* exactly `error`. A later edit that invents a code for it goes red here.
55+
*
56+
* ⛔ No separate refusal shape is invented for the empty case, and these pins
57+
* are deliberately not written to require one: once the load is attempted, an
58+
* empty string is a path that names no module and answers the way an
59+
* unresolvable path already answered. The sub-message belongs to bundle-require
60+
* and is NOT pinned; what is pinned is that the command names the flag and the
61+
* value it was given.
62+
*
63+
* ## Why no `dist/` sits on the measured path
64+
*
65+
* These run the CLI through `bin/run-dev.js`, the SOURCE entry — same CLI, run
66+
* from `src/` through tsx — so `commands/lint.ts` is loaded from source by the
67+
* child and this change is measured without a rebuild. (The shipped
68+
* `bin/run.js` was driven by hand and agreed on every reading above.)
69+
*/
70+
71+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
72+
import { execFile } from 'node:child_process';
73+
import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
74+
import { tmpdir } from 'node:os';
75+
import { join, resolve } from 'node:path';
76+
import { fileURLToPath } from 'node:url';
77+
import { childEnv } from './helpers/serve-process.js';
78+
79+
const HERE = resolve(fileURLToPath(import.meta.url), '..');
80+
const CLI = resolve(HERE, '../bin/run-dev.js');
81+
const TSX = resolve(HERE, '../../../node_modules/.bin/tsx');
82+
83+
interface Run {
84+
code: number;
85+
stdout: string;
86+
stderr: string;
87+
}
88+
89+
let dir: string;
90+
91+
/** A project this command lints CLEAN, so any non-zero exit is the refusal. */
92+
const CONFIG = `export default {
93+
name: 'empty_generator_probe',
94+
objects: [
95+
{
96+
name: 'probe_item',
97+
label: 'Probe Item',
98+
sharingModel: 'private',
99+
fields: { name: { type: 'text', label: 'Name' } },
100+
},
101+
],
102+
};
103+
`;
104+
105+
/**
106+
* The marker path the generator writes at import. Absent ⇒ the module was
107+
* never evaluated, which is the fact "was the generator loaded?" needs.
108+
*/
109+
const MARKER = 'GENERATOR_WAS_LOADED.marker';
110+
111+
const GENERATOR = `import { writeFileSync } from 'node:fs';
112+
writeFileSync(new URL('./${MARKER}', import.meta.url), 'loaded\\n');
113+
export default function generate() {
114+
return { name: 'from_generator', objects: [] };
115+
}
116+
`;
117+
118+
function markerPresent(): boolean {
119+
return existsSync(join(dir, MARKER));
120+
}
121+
122+
function clearMarker(): void {
123+
rmSync(join(dir, MARKER), { force: true });
124+
}
125+
126+
function runLint(args: string[]): Promise<Run> {
127+
return new Promise((resolvePromise) => {
128+
execFile(
129+
TSX,
130+
[CLI, 'lint', ...args],
131+
{ cwd: dir, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) },
132+
(err, stdout, stderr) => {
133+
resolvePromise({
134+
code: err
135+
? typeof (err as { code?: unknown }).code === 'number'
136+
? (err as unknown as { code: number }).code
137+
: 1
138+
: 0,
139+
stdout: String(stdout),
140+
stderr: String(stderr),
141+
});
142+
},
143+
);
144+
});
145+
}
146+
147+
/** stdout as ONE JSON document, or a failure that quotes what was there instead. */
148+
function payloadOf(run: Run, label: string): Record<string, unknown> {
149+
try {
150+
return JSON.parse(run.stdout) as Record<string, unknown>;
151+
} catch {
152+
throw new Error(
153+
`${label}: stdout was not one JSON document (exit ${run.code}, ${run.stdout.length} stdout bytes)\n` +
154+
`stdout: ${JSON.stringify(run.stdout)}\nstderr: ${JSON.stringify(run.stderr)}`,
155+
);
156+
}
157+
}
158+
159+
/**
160+
* The elapsed-time token is the only part of an eval report that varies run to
161+
* run; normalising it is what turned "the two runs look the same" into
162+
* "the two runs ARE the same" when this was measured.
163+
*/
164+
function normaliseElapsed(stdout: string): string {
165+
return stdout.replace(/\(\d+(\.\d+)?m?s\)/g, '(ELAPSED)');
166+
}
167+
168+
beforeAll(() => {
169+
dir = mkdtempSync(join(tmpdir(), 'os-lint-eval-empty-generator-'));
170+
writeFileSync(join(dir, 'objectstack.config.mjs'), CONFIG, 'utf8');
171+
writeFileSync(join(dir, 'gen-marker.mjs'), GENERATOR, 'utf8');
172+
});
173+
174+
afterAll(() => {
175+
rmSync(dir, { recursive: true, force: true });
176+
});
177+
178+
describe('os lint --eval --generator "" is refused, not silently ignored', () => {
179+
it('refuses on the human face, naming the flag and the empty value it was given', async () => {
180+
clearMarker();
181+
const run = await runLint(['--eval', '--generator', '']);
182+
183+
expect(run.code).toBe(1);
184+
expect(run.stdout).toContain('Failed to load generator ""');
185+
// ⛔ The sharpest pin: the offline eval did NOT run in place of the refusal.
186+
// Before the fix this line read `Mode: offline` with `5/5 passed` under it.
187+
expect(run.stdout).not.toContain('Mode: offline');
188+
expect(run.stdout).not.toContain('passed');
189+
expect(markerPresent()).toBe(false);
190+
}, 120_000);
191+
192+
it('the --json face stays a machine face — one JSON document, nothing on stderr', async () => {
193+
clearMarker();
194+
const run = await runLint(['--json', '--eval', '--generator', '']);
195+
const payload = payloadOf(run, 'json refusal');
196+
197+
expect(run.code).toBe(1);
198+
expect(String(payload.error)).toContain('Failed to load generator ""');
199+
expect(run.stderr).toBe('');
200+
expect(markerPresent()).toBe(false);
201+
// Before the fix this was the ordinary offline eval report: ok/mode/results.
202+
expect(payload.mode).toBeUndefined();
203+
expect(payload.results).toBeUndefined();
204+
}, 120_000);
205+
206+
it('nothing is minted — the payload key set is exactly `error`', async () => {
207+
// ADR-0112: `errorCodeFields` passes a producer's code through and returns
208+
// `{}` otherwise; bundle-require's empty-path rejection carries neither key.
209+
const run = await runLint(['--json', '--eval', '--generator', '']);
210+
const payload = payloadOf(run, 'key set');
211+
212+
expect(Object.keys(payload)).toEqual(['error']);
213+
}, 120_000);
214+
});
215+
216+
describe('os lint --eval — the empty generator is DISTINGUISHABLE from not passing the flag', () => {
217+
it('differs from the no-flag run once the elapsed token is normalised', async () => {
218+
// ⭐ The card's grading pivot, as an assertion. Measured before the fix:
219+
// these two stdouts were byte-identical after this exact normalisation, and
220+
// both exited 0 with 0 bytes on stderr — so an operator had no channel on
221+
// which to see that their generator was never loaded.
222+
clearMarker();
223+
const withEmpty = await runLint(['--eval', '--generator', '']);
224+
const withoutFlag = await runLint(['--eval']);
225+
226+
expect(normaliseElapsed(withEmpty.stdout)).not.toBe(normaliseElapsed(withoutFlag.stdout));
227+
expect(withEmpty.code).not.toBe(withoutFlag.code);
228+
expect(withoutFlag.code).toBe(0);
229+
expect(withoutFlag.stdout).toContain('Mode: offline');
230+
}, 120_000);
231+
});
232+
233+
describe('os lint --generator "" — the eval and non-eval sides answer the same way', () => {
234+
it('both refuse: same exit code, refusal on stdout, generator never loaded', async () => {
235+
// ⭐ #15550 / PR #16115 settled `!== undefined` for the non-eval side; this
236+
// card is that ruling reaching the eval side. The two refusals say
237+
// different things — they refuse different things — so what is pinned is
238+
// the disposition, which is what the asymmetry was about.
239+
clearMarker();
240+
const evalSide = await runLint(['--eval', '--generator', '']);
241+
const evalSideMarker = markerPresent();
242+
clearMarker();
243+
const nonEvalSide = await runLint(['--generator', '']);
244+
const nonEvalSideMarker = markerPresent();
245+
246+
expect(evalSide.code).toBe(nonEvalSide.code);
247+
expect(evalSide.code).toBe(1);
248+
expect(evalSide.stdout).not.toBe('');
249+
expect(nonEvalSide.stdout).not.toBe('');
250+
expect(evalSideMarker).toBe(false);
251+
expect(nonEvalSideMarker).toBe(false);
252+
}, 120_000);
253+
254+
it('both answer the same --json envelope: exit 1 and a lone `error` key', async () => {
255+
const evalSide = await runLint(['--json', '--eval', '--generator', '']);
256+
const nonEvalSide = await runLint(['--json', '--generator', '']);
257+
258+
expect(Object.keys(payloadOf(evalSide, 'json eval side'))).toEqual(['error']);
259+
expect(Object.keys(payloadOf(nonEvalSide, 'json non-eval side'))).toEqual(['error']);
260+
expect(evalSide.code).toBe(1);
261+
expect(nonEvalSide.code).toBe(1);
262+
}, 120_000);
263+
});
264+
265+
describe('os lint --eval — what the refusal must NOT move', () => {
266+
it('`--eval` with the flag absent still runs the offline eval', async () => {
267+
const run = await runLint(['--eval']);
268+
269+
expect(run.code).toBe(0);
270+
expect(run.stdout).toContain('Mode: offline');
271+
}, 120_000);
272+
273+
it('`--eval --generator <module>` still loads the generator and runs live', async () => {
274+
clearMarker();
275+
const run = await runLint(['--eval', '--generator', './gen-marker.mjs']);
276+
277+
expect(markerPresent()).toBe(true);
278+
expect(run.stdout).toContain('Mode: live');
279+
}, 120_000);
280+
281+
it('an unresolvable generator path refuses exactly as it always has', async () => {
282+
clearMarker();
283+
const run = await runLint(['--eval', '--generator', './does-not-exist.mjs']);
284+
285+
expect(run.code).toBe(1);
286+
expect(run.stdout).toContain('Failed to load generator "./does-not-exist.mjs"');
287+
expect(markerPresent()).toBe(false);
288+
}, 120_000);
289+
290+
it('a plain project lint is untouched', async () => {
291+
const run = await runLint([]);
292+
293+
expect(run.code).toBe(0);
294+
expect(run.stdout).toContain('All checks passed');
295+
}, 120_000);
296+
});

0 commit comments

Comments
 (0)