Skip to content

Commit 9ebb0cb

Browse files
committed
fix(cli): refuse os lint --generator without --eval instead of ignoring it
The flag's description ends "Requires --eval." and nothing checked it. `flags.generator` is read at exactly three sites, all inside `runEval`, which `run()` reaches only when `flags.eval` is set — so outside eval mode the flag reached no code at all. Driven before the change, from a lint-clean project, with a generator that writes a marker at top-level evaluation: os lint --generator ./gen-marker.mjs exit 0 · All checks passed · marker ABSENT os lint --generator ./does-not-exist.mjs exit 0 · All checks passed os lint --json --generator ./does-not-exist.mjs exit 0 · {"passed":true,…} Accepted, never loaded, and not named once on either face. The operator got a successful-looking run whose generator was never called. The repair refuses rather than deleting the sentence: nothing outside eval mode reads the flag, so dropping the claim would document a no-op instead of removing one. The refusal keeps the envelope this command already answers with — the human message on `error`, exit 1, both faces — and mints no ADR-0112 code, because it has no producer error to pass one through. Not oclif's `dependsOn: ['eval']`, measured rather than assumed: it refuses in the parser with exit 2, a stack trace on stderr, and empty stdout under `--json`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
1 parent 2648774 commit 9ebb0cb

2 files changed

Lines changed: 272 additions & 0 deletions

File tree

packages/cli/src/commands/lint.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,60 @@ export default class Lint extends Command {
514514
const configPath = args.config;
515515
const timer = createTimer();
516516

517+
// ── `--generator` means nothing without `--eval` — refuse, don't ignore ──
518+
//
519+
// [#15550] The flag's own description ends "Requires --eval." and nothing
520+
// checked it. Driven on this entry before this change, from a lint-clean
521+
// project, with a generator that writes a marker file at TOP-LEVEL
522+
// evaluation so "was it loaded?" is answered by the filesystem rather than
523+
// by reading the control flow:
524+
//
525+
// os lint --generator ./gen-marker.mjs exit 0 · All checks passed · marker ABSENT
526+
// os lint --generator ./does-not-exist.mjs exit 0 · All checks passed
527+
// os lint --json --generator ./does-not-exist.mjs exit 0 · {"passed":true,…}
528+
//
529+
// ⇒ accepted by the parser, never loaded, and not named once on either
530+
// face — a path that does not exist passes too. `flags.generator` is read
531+
// at exactly three sites, all inside `runEval`, which `run()` reaches only
532+
// when `flags.eval` is set, so outside eval mode the flag reaches no code
533+
// at all.
534+
//
535+
// That is Prime Directive #10's declared-≠-enforced shape landing on the
536+
// person least able to diagnose it: a successful-looking run whose
537+
// generator was never called, saying nothing. The direction is #12 — refuse
538+
// the off-contract invocation loudly at the boundary. The alternative
539+
// repair, deleting "Requires --eval." from the description, was rejected
540+
// for the reason that sentence exists: nothing outside eval mode reads this
541+
// flag, so dropping the claim documents a no-op flag instead of removing
542+
// one, and blesses the silent acceptance rather than ending it.
543+
//
544+
// ⛔ NOT oclif's `dependsOn: ['eval']`, and that is measured rather than
545+
// assumed. It does refuse — but it refuses in the PARSER, before the
546+
// command runs, so the refusal is oclif's and not this command's: exit 2, a
547+
// stack trace on stderr, and under `--json` an EMPTY STDOUT (measured, both
548+
// faces). A machine consumer of `os lint --json` would get no JSON document
549+
// at all — the off-envelope class #15549/#16044 had just repaired one exit
550+
// over on this very command. Refusing here keeps the envelope this command
551+
// already answers with: the human message on `error`, exit 1, both faces.
552+
//
553+
// ⛔ Nor the raw-argv guard `os migrate meta` uses for its stored-only
554+
// flags. That one exists because oclif reads a `default: false` boolean and
555+
// an `env`-backed string as "provided"; `--generator` has neither a default
556+
// nor an `env`, so `!== undefined` already means the operator typed it.
557+
//
558+
// ⛔ Nothing is minted: no `code` is attached. This refusal has no producer
559+
// error to pass one through, and ADR-0112's ledger is the authority on who
560+
// may mint one — the same restraint the generator-load exit below keeps.
561+
if (flags.generator !== undefined && !flags.eval) {
562+
const message =
563+
'--generator only applies to `os lint --eval` (the metadata-generation eval). '
564+
+ 'Without --eval this command lints the current project and never loads the generator. '
565+
+ 'Re-run as `os lint --eval --generator <module>`.';
566+
if (flags.json) await emitJson({ error: message }, 0, { compact: true });
567+
else printError(message);
568+
process.exit(1);
569+
}
570+
517571
// ── Eval mode — score generated metadata against the convention rubric ──
518572
// Short-circuits the project lint: this evaluates a generation corpus, not
519573
// the current config.
Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `os lint --generator` claimed a precondition nothing checked.
5+
*
6+
* ## The measured before-shape
7+
*
8+
* The flag's description ends "Requires --eval." and `run()` enforced nothing.
9+
* Driven on this entry before the fix, from a lint-clean project, with a
10+
* generator that writes a marker file at TOP-LEVEL evaluation so "was it
11+
* loaded?" is answered by the filesystem instead of by reading control flow:
12+
*
13+
* os lint --generator ./gen-marker.mjs exit 0 · All checks passed · marker ABSENT
14+
* os lint --generator ./does-not-exist.mjs exit 0 · All checks passed
15+
* os lint --json --generator ./does-not-exist.mjs exit 0 · {"passed":true,…}
16+
*
17+
* ⇒ accepted by the parser, never loaded, and not named once on either face —
18+
* a path that does not exist passed too. The operator got a successful-looking
19+
* run whose generator was never called, with nothing said. Silent, not loud.
20+
*
21+
* ## What is pinned, and why the negatives are not decoration
22+
*
23+
* The fix REFUSES rather than amending the description, so the accept set
24+
* narrows and the pins have to hold both directions:
25+
*
26+
* - positive — the refusal happens, on both faces, with the same envelope
27+
* this command already answers with (#16044): the human message on
28+
* `error`, exit 1, `--json` stdout still one JSON document.
29+
* - ⛔ negative — `--eval --generator` still LOADS the generator (marker
30+
* present, live mode). A "fix" that refused too broadly, or that refused
31+
* after loading the module, fails these directly. So does one that moved
32+
* plain `os lint` or offline `--eval`.
33+
*
34+
* ⛔ `nothing is minted` pins the ADR-0112 restraint from this side: the
35+
* refusal has no producer error to pass a `code` through, so the payload's key
36+
* set is exactly `error`. A later edit that invents a code for it goes red here
37+
* rather than handing consumers a vocabulary no ledger declares.
38+
*
39+
* ⛔ The refusal is deliberately NOT oclif's `dependsOn: ['eval']`. Measured on
40+
* this entry: `dependsOn` refuses in the parser with exit 2, a stack trace on
41+
* stderr, and EMPTY STDOUT under `--json`. `the --json face stays a machine
42+
* face` and the exit-code assertions fail that implementation.
43+
*
44+
* ## Why no `dist/` sits on the measured path
45+
*
46+
* These run the CLI through `bin/run-dev.js`, the SOURCE entry — same CLI, run
47+
* from `src/` through tsx — so `commands/lint.ts` is loaded from source by the
48+
* child and this change is measured without a rebuild.
49+
*/
50+
51+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
52+
import { execFile } from 'node:child_process';
53+
import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
54+
import { tmpdir } from 'node:os';
55+
import { join, resolve } from 'node:path';
56+
import { fileURLToPath } from 'node:url';
57+
import { childEnv } from './helpers/serve-process.js';
58+
59+
const HERE = resolve(fileURLToPath(import.meta.url), '..');
60+
const CLI = resolve(HERE, '../bin/run-dev.js');
61+
const TSX = resolve(HERE, '../../../node_modules/.bin/tsx');
62+
63+
interface Run {
64+
code: number;
65+
stdout: string;
66+
stderr: string;
67+
}
68+
69+
let dir: string;
70+
71+
/** A project this command lints CLEAN, so any non-zero exit is the refusal. */
72+
const CONFIG = `export default {
73+
name: 'refusal_probe',
74+
objects: [
75+
{
76+
name: 'probe_item',
77+
label: 'Probe Item',
78+
sharingModel: 'private',
79+
fields: { name: { type: 'text', label: 'Name' } },
80+
},
81+
],
82+
};
83+
`;
84+
85+
/**
86+
* The marker path the generator writes at import. Absent ⇒ the module was
87+
* never evaluated, which is the fact "was the generator loaded?" needs.
88+
*/
89+
const MARKER = 'GENERATOR_WAS_LOADED.marker';
90+
91+
const GENERATOR = `import { writeFileSync } from 'node:fs';
92+
writeFileSync(new URL('./${MARKER}', import.meta.url), 'loaded\\n');
93+
export default function generate() {
94+
return { name: 'from_generator', objects: [] };
95+
}
96+
`;
97+
98+
function markerPresent(): boolean {
99+
return existsSync(join(dir, MARKER));
100+
}
101+
102+
function clearMarker(): void {
103+
rmSync(join(dir, MARKER), { force: true });
104+
}
105+
106+
function runLint(args: string[]): Promise<Run> {
107+
return new Promise((resolvePromise) => {
108+
execFile(
109+
TSX,
110+
[CLI, 'lint', ...args],
111+
{ cwd: dir, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) },
112+
(err, stdout, stderr) => {
113+
resolvePromise({
114+
code: err
115+
? typeof (err as { code?: unknown }).code === 'number'
116+
? (err as unknown as { code: number }).code
117+
: 1
118+
: 0,
119+
stdout: String(stdout),
120+
stderr: String(stderr),
121+
});
122+
},
123+
);
124+
});
125+
}
126+
127+
/** stdout as ONE JSON document, or a failure that quotes what was there instead. */
128+
function payloadOf(run: Run, label: string): Record<string, unknown> {
129+
try {
130+
return JSON.parse(run.stdout) as Record<string, unknown>;
131+
} catch {
132+
throw new Error(
133+
`${label}: stdout was not one JSON document (exit ${run.code}, ${run.stdout.length} stdout bytes)\n` +
134+
`stdout: ${JSON.stringify(run.stdout)}\nstderr: ${JSON.stringify(run.stderr)}`,
135+
);
136+
}
137+
}
138+
139+
beforeAll(() => {
140+
dir = mkdtempSync(join(tmpdir(), 'os-lint-generator-requires-eval-'));
141+
writeFileSync(join(dir, 'objectstack.config.mjs'), CONFIG, 'utf8');
142+
writeFileSync(join(dir, 'gen-marker.mjs'), GENERATOR, 'utf8');
143+
});
144+
145+
afterAll(() => {
146+
rmSync(dir, { recursive: true, force: true });
147+
});
148+
149+
describe('os lint --generator without --eval is refused', () => {
150+
it('refuses on the human face, naming the flag it requires', async () => {
151+
clearMarker();
152+
const run = await runLint(['--generator', './gen-marker.mjs']);
153+
154+
expect(run.code).toBe(1);
155+
expect(run.stdout).toContain('--generator');
156+
expect(run.stdout).toContain('--eval');
157+
// ⛔ The sharpest pin: the refusal happens INSTEAD of the run, not after
158+
// loading the module. Before the fix this marker was absent for the
159+
// opposite reason — nothing read the flag at all — so it is asserted
160+
// together with the exit code, which was 0 then.
161+
expect(markerPresent()).toBe(false);
162+
}, 120_000);
163+
164+
it('the --json face stays a machine face — one JSON document, nothing on stderr', async () => {
165+
clearMarker();
166+
const run = await runLint(['--json', '--generator', './gen-marker.mjs']);
167+
const payload = payloadOf(run, 'json refusal');
168+
169+
expect(run.code).toBe(1);
170+
expect(String(payload.error)).toContain('--eval');
171+
expect(run.stderr).toBe('');
172+
expect(markerPresent()).toBe(false);
173+
}, 120_000);
174+
175+
it('nothing is minted — the payload key set is exactly `error`', async () => {
176+
// ADR-0112: this refusal has no producer error to pass a code through, and
177+
// the ledger is the authority on who may mint one.
178+
const run = await runLint(['--json', '--generator', './gen-marker.mjs']);
179+
const payload = payloadOf(run, 'key set');
180+
181+
expect(Object.keys(payload)).toEqual(['error']);
182+
}, 120_000);
183+
184+
it('is judged on the flag being TYPED, not on the path resolving', async () => {
185+
// Before the fix this exited 0 with "All checks passed" — a generator path
186+
// that does not exist was accepted as readily as one that does.
187+
const run = await runLint(['--generator', './does-not-exist.mjs']);
188+
189+
expect(run.code).toBe(1);
190+
expect(run.stdout).toContain('--eval');
191+
// The refusal is this command's, not esbuild's: the module is never reached.
192+
expect(run.stdout).not.toContain('Failed to load generator');
193+
}, 120_000);
194+
});
195+
196+
describe('os lint — what the refusal must NOT move', () => {
197+
it('`--eval --generator` still loads the generator and runs live', async () => {
198+
clearMarker();
199+
const run = await runLint(['--eval', '--generator', './gen-marker.mjs']);
200+
201+
expect(markerPresent()).toBe(true);
202+
expect(run.stdout).toContain('Mode: live');
203+
}, 120_000);
204+
205+
it('offline `--eval` with no generator is untouched', async () => {
206+
const run = await runLint(['--eval']);
207+
208+
expect(run.code).toBe(0);
209+
expect(run.stdout).toContain('Mode: offline');
210+
}, 120_000);
211+
212+
it('a plain project lint is untouched', async () => {
213+
const run = await runLint([]);
214+
215+
expect(run.code).toBe(0);
216+
expect(run.stdout).toContain('All checks passed');
217+
}, 120_000);
218+
});

0 commit comments

Comments
 (0)