Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/lint-generator-requires-eval.md
Original file line number Diff line number Diff line change
@@ -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.

<!-- adr-0087: not-required (no-migration-prescription) The change narrows what a CLI flag combination accepts 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. -->
69 changes: 69 additions & 0 deletions packages/cli/src/commands/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <module>`.';
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.
Expand Down
222 changes: 222 additions & 0 deletions packages/cli/test/lint-generator-requires-eval.e2e.test.ts
Original file line number Diff line number Diff line change
@@ -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<Run> {
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<string, unknown> {
try {
return JSON.parse(run.stdout) as Record<string, unknown>;
} 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);
});
Loading