Files
gbrain/test/dream-cli-flags.test.ts
T
60125ee626 feat(dream): --once for one-shot phase runs without toggling config gates (takeover of #2983) (#3031)
* feat(dream): --once for one-shot phase runs without toggling config gates

Fixes the "toggle enabled true, run, toggle back to false" workaround
that gbrain doctor's extract_atoms_backlog message implicitly
recommends and that #2860's reporter had to script around: with an
external orchestrator running `gbrain dream --phase patterns` on a
cadence outside the autopilot, the only way to run patterns once was
`config set dream.patterns.enabled true` -> run -> `config set ...
false`. A crash between steps left the flag stuck true, and the
autopilot (which polls the same flag) re-enqueued patterns every
cycle -- 119 LLM jobs / ~$400 over 24h before it was caught.

Root cause: `--phase X` only controls which phase FUNCTION cycle.ts
calls; it does not bypass that phase's own `dream.<phase>.enabled` /
`cycle.<phase>.enabled` config read. Each gated phase (patterns,
synthesize, conversation_facts_backfill, enrich_thin, skillopt) reads
its enabled flag internally and skips regardless of how the phase was
selected -- confirmed by reading each phase module, not assumed.
extract_atoms/synthesize_concepts are a DIFFERENT mechanism entirely
(pack-declaration via packDeclaresPhase, not a config .enabled read)
and already have a working one-shot escape hatch: `--drain`. The
existing doctor message for extract_atoms already says `--phase
extract_atoms --drain --window 120`, so no doctor text needed
updating there -- verified by reading src/commands/doctor.ts directly
rather than assuming the paraphrase in the issue was literal.

Design: `gbrain dream --phase <name> --once`. Requires an explicit
--phase (bare --once is a usage error, exit 2) so it can never
force-enable every disabled phase at once in a full/default cycle --
that would recreate the same unbounded-spend risk the flag exists to
prevent. Threaded through CycleOpts as `onceForPhase?: CyclePhase`
(the literal phase name, not a boolean) so the bypass can never leak
to a phase other than the one named, even if a future programmatic
caller passes a wider `phases` array than the CLI does. Never reads
or writes config -- the phase still evaluates its .enabled gate every
call; --once only overrides the boolean OUTCOME for that one
invocation, mirroring the existing --unsafe-bypass-dream-guard /
--input precedents (stderr warning at the bypass point, no new
config-touching code path).

Rejected alternatives (documented per task instructions):
- Making explicit --phase X always bypass .enabled: breaking change
  for existing crons that rely on the disabled flag as a cheap no-op;
  an upgrade would silently start running LLM/write phases.
- A new subcommand: adds a whole dispatch/help/arg surface that
  internally routes through the same override anyway.
- Extending --once to also bypass packDeclaresPhase for
  extract_atoms/synthesize_concepts: conflates two different gating
  mechanisms (config toggle vs. pack membership) under one flag;
  extract_atoms already has --drain, which is purpose-built for its
  batched/windowed execution model.

Design was cross-validated by an independent second-model review
(external design consultation) before implementation; its
recommendation to also update the extract_atoms doctor message to
`--once` was NOT adopted because that phase has no .enabled gate to
bypass -- doing so would be a documented no-op, contradicted by
reading src/commands/doctor.ts:3264 directly.

Tests: 9 new (structural CLI-flag wiring in dream-cli-flags.test.ts;
a real PGLite E2E test in dream-patterns-pglite.test.ts proving the
bypass fires AND that dream.patterns.enabled is never written; 4
runCycle-level tests in cycle.serial.test.ts proving onceForPhase
does not leak across phases). Verified 6 of 9 fail against the
pre-fix source (via git stash of source-only changes) to confirm
they're meaningful regressions, not tautologies.

Closes #2860

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* 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>

* fix(dream): --once must require an EXPLICIT --phase, not a derived one

Codex review finding (P3): the --once validation checked the derived
`phase` value, but `phase` gets defaulted implicitly by --input
(implies --phase synthesize) and --drain (implies --phase
extract_atoms) BEFORE that check ran. So `gbrain dream --input <f>
--once` and `gbrain dream --drain --once` both slipped past the
"explicit --phase required" contract silently -- and --once became a
true no-op in both cases: --drain returns from runDream before
onceForPhase is ever read (the drain path doesn't call runCycle at
all), and --input already bypasses the synthesize enabled-gate on its
own via the existing opts.inputFile check, so onceForPhase would
never even be consulted.

Fix: capture `phaseWasExplicit = phaseIdx !== -1` at the very top of
parseArgs, before the --input/--drain defaulting blocks run, and
validate --once against that instead of the derived `phase`. Updated
the usage-error message and --help text to say "an explicit --phase"
so a user hitting this understands why `--input ... --once` doesn't
count.

Tests: 2 new pins in test/dream.test.ts exercising runDream directly
(--input <file> --once exits 2; --drain --once exits 2), plus a
structural test in dream-cli-flags.test.ts pinning that
phaseWasExplicit is captured before both implicit-defaulting blocks.
Updated the two existing structural/behavioral tests whose literal
guard-condition / error-message assertions changed shape.

Verified: dream-cli-flags.test.ts 27/27, dream.test.ts 31/31,
typecheck clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: masashiono0611 <masashi.ono.0611@gmail.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Garry Tan <garrytan@gmail.com>
2026-07-21 12:29:35 -07:00

185 lines
7.6 KiB
TypeScript

/**
* Structural tests for `gbrain dream` argv parsing (v0.21).
*
* Verifies the help text + parser source contains the new flags
* (--input, --date, --from, --to) and that conflict detection is wired.
* The actual parseArgs is internal; we exercise it via the source file
* structure to avoid spinning up a process per test.
*/
import { describe, test, expect } from 'bun:test';
import { readFileSync } from 'fs';
const dreamSrc = readFileSync(new URL('../src/commands/dream.ts', import.meta.url), 'utf-8');
describe('dream CLI flag wiring', () => {
test('declares --input flag with file argument', () => {
expect(dreamSrc).toContain("'--input'");
expect(dreamSrc).toContain('inputFile');
});
test('declares --date / --from / --to flags', () => {
expect(dreamSrc).toContain("'--date'");
expect(dreamSrc).toContain("'--from'");
expect(dreamSrc).toContain("'--to'");
});
test('validates ISO date format', () => {
expect(dreamSrc).toMatch(/ISO_DATE_RE/);
expect(dreamSrc).toContain('YYYY-MM-DD');
});
test('--input + --date conflict detection', () => {
expect(dreamSrc).toContain('--input cannot be combined with --date');
});
test('--input implies --phase synthesize', () => {
expect(dreamSrc).toContain("phase = 'synthesize'");
});
test('--from > --to range validation', () => {
expect(dreamSrc).toContain('empty range');
});
test('forwards synth fields to runCycle', () => {
expect(dreamSrc).toContain('synthInputFile');
expect(dreamSrc).toContain('synthDate');
expect(dreamSrc).toContain('synthFrom');
expect(dreamSrc).toContain('synthTo');
});
test('totals line includes synth + patterns counters', () => {
expect(dreamSrc).toContain('synth_transcripts');
expect(dreamSrc).toContain('synth_pages');
expect(dreamSrc).toContain('patterns=');
});
test('help text documents dry-run synthesis semantics (Codex finding #8)', () => {
expect(dreamSrc).toContain('skips the Sonnet');
expect(dreamSrc.toLowerCase()).toContain('zero llm calls');
});
// v0.41.13: --source / --source-id flag wiring (supersedes PR #1559).
// Structural-only tests; behavioral tests live in test/dream.test.ts.
describe('--source / --source-id wiring (v0.41.13)', () => {
test('declares --source flag in argv parsing', () => {
expect(dreamSrc).toContain("'--source'");
});
test('declares --source-id alias in argv parsing', () => {
expect(dreamSrc).toContain("'--source-id'");
});
test('forwards resolved sourceId to runCycle', () => {
// The runCycle call must pass sourceId; gate name "sourceId"
// not "source" because CycleOpts.sourceId is the contract.
expect(dreamSrc).toMatch(/sourceId:\s*resolvedSourceId/);
});
test('imports resolveSourceId from canonical source-resolver helper', () => {
expect(dreamSrc).toContain("from '../core/source-resolver.ts'");
expect(dreamSrc).toContain('resolveSourceId');
});
test('declares isResolverUserError predicate for typed-error catch (T3 from eng review)', () => {
expect(dreamSrc).toContain('function isResolverUserError');
});
test('documents --source in --help output', () => {
expect(dreamSrc).toContain('--source <id>');
expect(dreamSrc).toContain('--source-id <id>');
});
test('preserves --help short-circuit ordering comment (IRON RULE)', () => {
// The comment lives in runDream BEFORE the engine-null gate.
// Future refactors that reorder these blocks will trip this guard.
expect(dreamSrc).toContain('IRON RULE: --help short-circuits BEFORE');
});
test('declares engine-null guard for --source', () => {
expect(dreamSrc).toContain('requires a connected brain');
});
test('declares archived-source guard', () => {
expect(dreamSrc).toMatch(/source.*is archived/);
expect(dreamSrc).toContain('gbrain sources restore');
});
});
// issue #1678 — --drain bounded backlog drain wiring (structural).
describe('--drain wiring', () => {
test('declares --drain and --window flags', () => {
expect(dreamSrc).toContain("'--drain'");
expect(dreamSrc).toContain("'--window'");
expect(dreamSrc).toContain('windowSeconds');
});
test('--drain defaults to extract_atoms and rejects other phases', () => {
expect(dreamSrc).toContain("phase = 'extract_atoms'");
expect(dreamSrc).toContain('--drain currently supports only --phase extract_atoms');
});
test('drain routes through the shared helper with the resolved source (5A)', () => {
// v0.42.10.0 (#1685 GAP D / 5A): the lock+batch+count wiring moved into
// runExtractAtomsDrainForSource so the CLI, the Minion handler, and
// autopilot share ONE drain path. dream threads resolvedSourceId so the
// helper picks cycleLockIdFor(resolvedSourceId) — the same lock the routine
// cycle holds for that source. The lock-id contract is now pinned in
// test/extract-atoms-drain.test.ts ("shared wiring helper holds the cycle lock").
expect(dreamSrc).toContain('runExtractAtomsDrainForSource');
expect(dreamSrc).toContain('sourceId: resolvedSourceId');
});
test('drain reports remaining + exits non-zero when incomplete', () => {
expect(dreamSrc).toContain('EXIT_DRAIN_INCOMPLETE');
expect(dreamSrc).toContain('cycle_already_running');
});
});
// issue #2860 — --once one-shot phase-enabled-gate bypass (structural).
// Behavioral coverage: test/e2e/dream-patterns-pglite.test.ts (bypass +
// config-untouched) and test/core/cycle.serial.test.ts (non-leak across
// phases via CycleOpts.onceForPhase).
describe('--once wiring (issue #2860)', () => {
test('declares --once flag', () => {
expect(dreamSrc).toContain("'--once'");
});
test('rejects bare --once with no --phase (exit 2)', () => {
expect(dreamSrc).toContain('--once requires an explicit --phase <name>');
// --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 && !phaseWasExplicit && !wantsHelp)');
});
// Codex P3 finding: the derived `phase` value gets populated by
// --input/--drain BEFORE this validation used to run, so those two
// silently slipped past an `!phase`-based check. The fix validates
// against `phaseWasExplicit` (captured at `phaseIdx !== -1`, before
// any implicit defaulting) instead. Behavioral pins live in
// test/dream.test.ts.
test('validates against phaseWasExplicit, captured before --input/--drain defaulting', () => {
expect(dreamSrc).toContain('const phaseWasExplicit = phaseIdx !== -1;');
// Must be declared before the --input-implies-synthesize and
// --drain-implies-extract_atoms defaulting blocks so it captures
// presence prior to any implicit phase assignment.
const explicitIdx = dreamSrc.indexOf('const phaseWasExplicit = phaseIdx !== -1;');
const inputImpliesIdx = dreamSrc.indexOf("phase = 'synthesize'");
const drainImpliesIdx = dreamSrc.indexOf("phase = 'extract_atoms'");
expect(explicitIdx).toBeGreaterThan(-1);
expect(explicitIdx).toBeLessThan(inputImpliesIdx);
expect(explicitIdx).toBeLessThan(drainImpliesIdx);
});
test('threads onceForPhase to runCycle, gated on opts.once', () => {
expect(dreamSrc).toMatch(/onceForPhase:\s*opts\.once\s*\?\s*opts\.phase!\s*:\s*undefined/);
});
test('documents --once in --help output', () => {
expect(dreamSrc).toContain('--once');
expect(dreamSrc).toContain('Never reads or writes config');
});
});
});