diff --git a/.changeset/lint-generator-requires-eval.md b/.changeset/lint-generator-requires-eval.md new file mode 100644 index 0000000000..f83be20b1a --- /dev/null +++ b/.changeset/lint-generator-requires-eval.md @@ -0,0 +1,13 @@ +--- +"@objectstack/cli": minor +--- + +**BREAKING** `os lint --generator` now refuses to run without `--eval`, instead of accepting the flag and ignoring it. + +The flag's own description has always ended "Requires --eval.", and nothing checked it. `--generator` is read only by eval mode, so outside `--eval` the flag reached no code at all: `os lint --generator ./gen.mjs` linted the current project, exited 0 with "All checks passed", never loaded the module, and named the flag nowhere on either the human face or `--json`. A path that did not exist was accepted just as readily. Someone who meant to score a live generator got a successful-looking run whose generator was never called, with nothing said. + +The refusal is this command's own, not the argument parser's, so it keeps the shape the command's other failures already have: the reason on `error`, exit 1, and on `--json` a single JSON document with stdout still reserved for the machine. No error code is invented for it. + +A scripted invocation that passed `--generator` outside eval mode now exits 1 with the reason, where it previously exited 0 having silently skipped the generator. Eval mode itself is untouched: `--eval --generator` still loads the module and scores live output, and `--eval` alone still scores the bundled corpus offline. + + diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index b8e0dbef94..d6c14bb15f 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -514,6 +514,75 @@ export default class Lint extends Command { const configPath = args.config; const timer = createTimer(); + // ── `--generator` means nothing without `--eval` — refuse, don't ignore ── + // + // [#15550] The flag's own description ends "Requires --eval." and nothing + // checked it. Driven on this entry before this change, from a lint-clean + // project, with a generator that writes a marker file at TOP-LEVEL + // evaluation so "was it loaded?" is answered by the filesystem rather than + // by reading the control flow: + // + // 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 by the parser, never loaded, and not named once on either + // face — a path that does not exist passes too. `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 reaches no code + // at all. + // + // That is Prime Directive #10's declared-≠-enforced shape landing on the + // person least able to diagnose it: a successful-looking run whose + // generator was never called, saying nothing. The direction is #12 — refuse + // the off-contract invocation loudly at the boundary. The alternative + // repair, deleting "Requires --eval." from the description, was rejected + // for the reason that sentence exists: nothing outside eval mode reads this + // flag, so dropping the claim documents a no-op flag instead of removing + // one, and blesses the silent acceptance rather than ending it. + // + // ⛔ NOT oclif's `dependsOn: ['eval']` — and the reason is BLAST RADIUS, + // not an inability to answer inside this command's envelope. + // + // Bare `dependsOn` refuses in the PARSER, before the command runs, so its + // refusal is oclif's: exit 2, and under `--json` an EMPTY STDOUT. (The + // stack trace that accompanies it on `bin/run-dev.js` is a DEV-ENTRY + // artefact of `settings.debug`; the shipped `bin/run.js` prints oclif's + // pretty message with no stack. Don't generalise the dev entry's output.) + // + // ⚠️ That much CAN be brought inside the envelope: a `catch()` override on + // the parse was measured answering exit 1 with `{error}` on the `--json` + // face and an empty stderr. So "the framework spelling cannot be + // enveloped" is FALSE, and ⛔ nobody should re-derive this choice from it. + // + // The real objection is scope. That override re-shapes EVERY parse error on + // this command, not the one precondition this card is about: every unknown + // flag and every bad value would move from exit 2 / stderr to exit 1 / + // stdout, and would carry oclif's own prose plus its `--help` hint inside + // the JSON `error` string — a wide, uncommissioned change to the very + // `--json` envelope #15549/#16044 had just repaired one exit over. A guard + // here moves ONE invocation class and leaves every other parse error + // exactly as it was, while keeping the envelope this command already + // answers with: the human message on `error`, exit 1, both faces. + // + // ⛔ Nor the raw-argv guard `os migrate meta` uses for its stored-only + // flags. That one exists because oclif reads a `default: false` boolean and + // an `env`-backed string as "provided"; `--generator` has neither a default + // nor an `env`, so `!== undefined` already means the operator typed it. + // + // ⛔ Nothing is minted: no `code` is attached. This refusal has no producer + // error to pass one through, and ADR-0112's ledger is the authority on who + // may mint one — the same restraint the generator-load exit below keeps. + if (flags.generator !== undefined && !flags.eval) { + const message = + '--generator only applies to `os lint --eval` (the metadata-generation eval). ' + + 'Without --eval this command lints the current project and never loads the generator. ' + + 'Re-run as `os lint --eval --generator `.'; + if (flags.json) await emitJson({ error: message }, 0, { compact: true }); + else printError(message); + process.exit(1); + } + // ── Eval mode — score generated metadata against the convention rubric ── // Short-circuits the project lint: this evaluates a generation corpus, not // the current config. diff --git a/packages/cli/test/lint-generator-requires-eval.e2e.test.ts b/packages/cli/test/lint-generator-requires-eval.e2e.test.ts new file mode 100644 index 0000000000..61de6e667b --- /dev/null +++ b/packages/cli/test/lint-generator-requires-eval.e2e.test.ts @@ -0,0 +1,222 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os lint --generator` claimed a precondition nothing checked. + * + * ## The measured before-shape + * + * The flag's description ends "Requires --eval." and `run()` enforced nothing. + * Driven on this entry before the fix, from a lint-clean project, with a + * generator that writes a marker file at TOP-LEVEL evaluation so "was it + * loaded?" is answered by the filesystem instead of by reading control flow: + * + * 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 by the parser, never loaded, and not named once on either face — + * a path that does not exist passed too. The operator got a successful-looking + * run whose generator was never called, with nothing said. Silent, not loud. + * + * ## What is pinned, and why the negatives are not decoration + * + * The fix REFUSES rather than amending the description, so the accept set + * narrows and the pins have to hold both directions: + * + * - positive — the refusal happens, on both faces, with the same envelope + * this command already answers with (#16044): the human message on + * `error`, exit 1, `--json` stdout still one JSON document. + * - ⛔ negative — `--eval --generator` still LOADS the generator (marker + * present, live mode). A "fix" that refused too broadly, or that refused + * after loading the module, fails these directly. So does one that moved + * plain `os lint` or offline `--eval`. + * + * ⛔ `nothing is minted` pins the ADR-0112 restraint from this side: the + * refusal has no producer error to pass a `code` through, so the payload's key + * set is exactly `error`. A later edit that invents a code for it goes red here + * rather than handing consumers a vocabulary no ledger declares. + * + * ⛔ The refusal is deliberately NOT oclif's `dependsOn: ['eval']` — and these + * pins are NOT the argument for that. Bare `dependsOn` does fail them (exit 2, + * empty stdout under `--json`, measured), but a `catch()` override brings it + * inside the envelope and PASSES them. So a green here is not a verdict on the + * choice, and ⛔ must not be read as one. The reason the guard lives in the + * command is BLAST RADIUS — that override re-shapes every parse error on this + * command rather than this one precondition — and it is recorded where the + * decision is, at the guard in `src/commands/lint.ts`. + * + * ## Why no `dist/` sits on the measured path + * + * These run the CLI through `bin/run-dev.js`, the SOURCE entry — same CLI, run + * from `src/` through tsx — so `commands/lint.ts` is loaded from source by the + * child and this change is measured without a rebuild. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +let dir: string; + +/** A project this command lints CLEAN, so any non-zero exit is the refusal. */ +const CONFIG = `export default { + name: 'refusal_probe', + objects: [ + { + name: 'probe_item', + label: 'Probe Item', + sharingModel: 'private', + fields: { name: { type: 'text', label: 'Name' } }, + }, + ], +}; +`; + +/** + * The marker path the generator writes at import. Absent ⇒ the module was + * never evaluated, which is the fact "was the generator loaded?" needs. + */ +const MARKER = 'GENERATOR_WAS_LOADED.marker'; + +const GENERATOR = `import { writeFileSync } from 'node:fs'; +writeFileSync(new URL('./${MARKER}', import.meta.url), 'loaded\\n'); +export default function generate() { + return { name: 'from_generator', objects: [] }; +} +`; + +function markerPresent(): boolean { + return existsSync(join(dir, MARKER)); +} + +function clearMarker(): void { + rmSync(join(dir, MARKER), { force: true }); +} + +function runLint(args: string[]): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, 'lint', ...args], + { cwd: dir, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) }, + (err, stdout, stderr) => { + resolvePromise({ + code: err + ? typeof (err as { code?: unknown }).code === 'number' + ? (err as unknown as { code: number }).code + : 1 + : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +/** stdout as ONE JSON document, or a failure that quotes what was there instead. */ +function payloadOf(run: Run, label: string): Record { + try { + return JSON.parse(run.stdout) as Record; + } catch { + throw new Error( + `${label}: stdout was not one JSON document (exit ${run.code}, ${run.stdout.length} stdout bytes)\n` + + `stdout: ${JSON.stringify(run.stdout)}\nstderr: ${JSON.stringify(run.stderr)}`, + ); + } +} + +beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'os-lint-generator-requires-eval-')); + writeFileSync(join(dir, 'objectstack.config.mjs'), CONFIG, 'utf8'); + writeFileSync(join(dir, 'gen-marker.mjs'), GENERATOR, 'utf8'); +}); + +afterAll(() => { + rmSync(dir, { recursive: true, force: true }); +}); + +describe('os lint --generator without --eval is refused', () => { + it('refuses on the human face, naming the flag it requires', async () => { + clearMarker(); + const run = await runLint(['--generator', './gen-marker.mjs']); + + expect(run.code).toBe(1); + expect(run.stdout).toContain('--generator'); + expect(run.stdout).toContain('--eval'); + // ⛔ The sharpest pin: the refusal happens INSTEAD of the run, not after + // loading the module. Before the fix this marker was absent for the + // opposite reason — nothing read the flag at all — so it is asserted + // together with the exit code, which was 0 then. + expect(markerPresent()).toBe(false); + }, 120_000); + + it('the --json face stays a machine face — one JSON document, nothing on stderr', async () => { + clearMarker(); + const run = await runLint(['--json', '--generator', './gen-marker.mjs']); + const payload = payloadOf(run, 'json refusal'); + + expect(run.code).toBe(1); + expect(String(payload.error)).toContain('--eval'); + expect(run.stderr).toBe(''); + expect(markerPresent()).toBe(false); + }, 120_000); + + it('nothing is minted — the payload key set is exactly `error`', async () => { + // ADR-0112: this refusal has no producer error to pass a code through, and + // the ledger is the authority on who may mint one. + const run = await runLint(['--json', '--generator', './gen-marker.mjs']); + const payload = payloadOf(run, 'key set'); + + expect(Object.keys(payload)).toEqual(['error']); + }, 120_000); + + it('is judged on the flag being TYPED, not on the path resolving', async () => { + // Before the fix this exited 0 with "All checks passed" — a generator path + // that does not exist was accepted as readily as one that does. + const run = await runLint(['--generator', './does-not-exist.mjs']); + + expect(run.code).toBe(1); + expect(run.stdout).toContain('--eval'); + // The refusal is this command's, not esbuild's: the module is never reached. + expect(run.stdout).not.toContain('Failed to load generator'); + }, 120_000); +}); + +describe('os lint — what the refusal must NOT move', () => { + it('`--eval --generator` still loads the generator and runs live', async () => { + clearMarker(); + const run = await runLint(['--eval', '--generator', './gen-marker.mjs']); + + expect(markerPresent()).toBe(true); + expect(run.stdout).toContain('Mode: live'); + }, 120_000); + + it('offline `--eval` with no generator is untouched', async () => { + const run = await runLint(['--eval']); + + expect(run.code).toBe(0); + expect(run.stdout).toContain('Mode: offline'); + }, 120_000); + + it('a plain project lint is untouched', async () => { + const run = await runLint([]); + + expect(run.code).toBe(0); + expect(run.stdout).toContain('All checks passed'); + }, 120_000); +});