mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
fix(dream): --help short-circuits before --once usage validation
Codex review finding (P2): `gbrain dream --help --once` (no --phase)
called process.exit(2) from the new --once usage-error check inside
parseArgs before runDream's documented IRON RULE ("--help
short-circuits BEFORE any engine-bearing work") ever got a chance to
run -- parseArgs computes ALL its validations unconditionally before
runDream checks opts.help. Repo precedent for this ordering already
exists as a pinned regression test (test/dream.test.ts's "--help
--source whatever prints help and exits 0").
Fix: compute wantsHelp once in parseArgs and exempt the --once
validation when it's set, mirroring that precedent. Added the same
class of pinned tests here: bare `--once` still exits 2 with the
usage hint, `--help --once` prints help and exits 0, and a real
--phase patterns --once run against a PGLite engine proves the
bypass actually fires (falls through to insufficient_evidence
instead of disabled) without writing dream.patterns.enabled. Also
fixed the structural test in dream-cli-flags.test.ts that asserted
the exact pre-fix guard-condition source text.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
4b2b8115e4
commit
ca6673c327
@@ -230,8 +230,14 @@ function parseArgs(args: string[]): DreamArgs {
|
||||
// `--once` (full/default cycle) has no single phase to bypass the gate
|
||||
// for, and force-enabling EVERY currently-disabled phase at once would
|
||||
// be exactly the kind of surprise-spend risk the flag exists to prevent.
|
||||
//
|
||||
// Codex review finding: `--help` must short-circuit BEFORE this exits(2),
|
||||
// matching the "IRON RULE" pinned by test/dream.test.ts's
|
||||
// "--help --source whatever prints help and exits 0" case — `gbrain
|
||||
// dream --help --once` (no --phase) must show help, not a usage error.
|
||||
const once = args.includes('--once');
|
||||
if (once && !phase) {
|
||||
const wantsHelp = args.includes('--help') || args.includes('-h');
|
||||
if (once && !phase && !wantsHelp) {
|
||||
console.error(
|
||||
'--once requires --phase <name> (bypasses that one phase\'s ' +
|
||||
'dream.<phase>.enabled / cycle.<phase>.enabled gate for this run only; ' +
|
||||
|
||||
@@ -147,7 +147,10 @@ describe('dream CLI flag wiring', () => {
|
||||
|
||||
test('rejects bare --once with no --phase (exit 2)', () => {
|
||||
expect(dreamSrc).toContain('--once requires --phase <name>');
|
||||
expect(dreamSrc).toContain('if (once && !phase)');
|
||||
// --help must short-circuit this validation (Codex review finding) —
|
||||
// see the "--help --once" test in test/dream.test.ts for the
|
||||
// behavioral pin of this exact ordering.
|
||||
expect(dreamSrc).toContain('if (once && !phase && !wantsHelp)');
|
||||
});
|
||||
|
||||
test('threads onceForPhase to runCycle, gated on opts.once', () => {
|
||||
|
||||
@@ -151,6 +151,80 @@ describe('runDream — --phase <name> restricts the cycle', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─── --once (issue #2860) ───────────────────────────────────────────
|
||||
|
||||
describe('runDream — --once (issue #2860)', () => {
|
||||
let repo: string;
|
||||
let engine: InstanceType<typeof PGLiteEngine>;
|
||||
|
||||
beforeEach(async () => {
|
||||
repo = makeGitRepo();
|
||||
engine = await makePGLite();
|
||||
}, 60_000);
|
||||
|
||||
afterEach(async () => {
|
||||
if (engine) await engine.disconnect();
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
}, 60_000);
|
||||
|
||||
test('bare --once (no --phase) exits 2 with a usage hint', async () => {
|
||||
const exitSpy = spyOn(process, 'exit').mockImplementation(() => { throw new Error('EXIT'); });
|
||||
const errSpy = spyOn(console, 'error').mockImplementation(() => {});
|
||||
try {
|
||||
await runDream(engine, ['--dir', repo, '--once']);
|
||||
throw new Error('expected runDream to exit');
|
||||
} catch (e: any) {
|
||||
expect(e.message).toBe('EXIT');
|
||||
}
|
||||
expect(exitSpy).toHaveBeenCalledWith(2);
|
||||
expect(errSpy.mock.calls.flat().join(' ')).toMatch(/--once requires --phase <name>/);
|
||||
exitSpy.mockRestore();
|
||||
errSpy.mockRestore();
|
||||
});
|
||||
|
||||
// Codex review finding: --help must short-circuit BEFORE the bare---once
|
||||
// usage error, matching the same IRON RULE pinned above for --source
|
||||
// ("--help --source whatever prints help and exits 0").
|
||||
test('--help --once (no --phase) prints help and exits 0, not the usage error', async () => {
|
||||
const exitSpy = spyOn(process, 'exit').mockImplementation(() => { throw new Error('EXIT'); });
|
||||
const logSpy = spyOn(console, 'log').mockImplementation(() => {});
|
||||
try {
|
||||
const result = await runDream(engine, ['--help', '--once']);
|
||||
expect(result).toBeUndefined();
|
||||
} catch (e: any) {
|
||||
throw new Error('--help with --once should NOT exit; got: ' + e.message);
|
||||
}
|
||||
expect(exitSpy).not.toHaveBeenCalled();
|
||||
expect(logSpy.mock.calls.flat().join(' ')).toMatch(/Usage: gbrain dream/);
|
||||
exitSpy.mockRestore();
|
||||
logSpy.mockRestore();
|
||||
});
|
||||
|
||||
test('--phase patterns --once bypasses dream.patterns.enabled=false without writing config', async () => {
|
||||
await engine.setConfig('dream.patterns.enabled', 'false');
|
||||
const report = await runDream(engine, ['--dir', repo, '--phase', 'patterns', '--once', '--json']);
|
||||
expect(report).toBeTruthy();
|
||||
if (report) {
|
||||
expect(report.phases.length).toBe(1);
|
||||
// Bypassed 'disabled' → falls through to the next gate. No
|
||||
// reflections seeded, so it reports insufficient_evidence, not
|
||||
// 'disabled' — proving --once actually forced the run.
|
||||
expect(report.phases[0].phase).toBe('patterns');
|
||||
expect((report.phases[0].details as { reason?: string }).reason).toBe('insufficient_evidence');
|
||||
}
|
||||
expect(await engine.getConfig('dream.patterns.enabled')).toBe('false');
|
||||
});
|
||||
|
||||
test('--phase patterns (no --once) with dream.patterns.enabled=false still skips as disabled', async () => {
|
||||
await engine.setConfig('dream.patterns.enabled', 'false');
|
||||
const report = await runDream(engine, ['--dir', repo, '--phase', 'patterns', '--json']);
|
||||
expect(report).toBeTruthy();
|
||||
if (report) {
|
||||
expect((report.phases[0].details as { reason?: string }).reason).toBe('disabled');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Output format ─────────────────────────────────────────────────
|
||||
|
||||
describe('runDream — output format', () => {
|
||||
|
||||
Reference in New Issue
Block a user