From 60125ee626c4fb96db13fca4a4acc9d4c2eaece5 Mon Sep 17 00:00:00 2001 From: Time Attakc <89218912+time-attack@users.noreply.github.com> Date: Tue, 21 Jul 2026 12:29:35 -0700 Subject: [PATCH] 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..enabled` / `cycle..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 --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 * 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 * 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 --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 --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 --------- Co-authored-by: masashiono0611 Co-authored-by: Claude Sonnet 5 Co-authored-by: Garry Tan --- src/commands/dream.ts | 65 ++++++++++ src/core/cycle.ts | 36 +++++- src/core/cycle/conversation-facts-backfill.ts | 36 ++++-- src/core/cycle/enrich-thin.ts | 32 +++-- src/core/cycle/patterns.ts | 14 ++- src/core/cycle/synthesize.ts | 17 ++- src/core/skillopt/cycle-phase.ts | 26 ++-- test/core/cycle.serial.test.ts | 65 ++++++++++ test/dream-cli-flags.test.ts | 46 +++++++ test/dream.test.ts | 113 ++++++++++++++++++ test/e2e/dream-patterns-pglite.test.ts | 39 ++++++ 11 files changed, 456 insertions(+), 33 deletions(-) diff --git a/src/commands/dream.ts b/src/commands/dream.ts index e90a01963..d2bc62bb5 100644 --- a/src/commands/dream.ts +++ b/src/commands/dream.ts @@ -76,6 +76,18 @@ interface DreamArgs { drain: boolean; /** Drain wallclock budget in seconds. Default 300 (5 min). */ windowSeconds: number; + /** + * issue #2860 — `--once`. One-shot bypass of the named `--phase`'s own + * `dream..enabled` / `cycle..enabled` config gate, for this + * invocation only. Never reads or writes config — unlike the old + * "toggle enabled true, run, toggle back to false" workaround, a crash + * mid-run can't leave any global state stuck. Requires an explicit + * `--phase `; bare `--once` is a usage error (there'd be no single + * phase to target). Applies only to phases with a config `.enabled` gate + * (patterns, synthesize, conversation_facts_backfill, enrich_thin, + * skillopt) — a no-op for phases that always run when named directly. + */ + once: boolean; } const ISO_DATE_RE = /^\d{4}-\d{2}-\d{2}$/; @@ -105,6 +117,14 @@ function collectFlagValues(args: string[], flag: string): string[] | null { function parseArgs(args: string[]): DreamArgs { const phaseIdx = args.indexOf('--phase'); + // issue #2860 (Codex P3): captured BEFORE --input/--drain get a chance to + // implicitly default `phase` below, so --once's validation can require + // the user actually TYPED --phase, not merely that some phase ended up + // resolved. Without this, `--input --once` and `--drain --once` + // slip past the "explicit --phase required" contract (the derived + // `phase` value is already non-null by the time that check runs) and + // --once becomes silently ineffective for both. + const phaseWasExplicit = phaseIdx !== -1; const rawPhase = phaseIdx !== -1 ? args[phaseIdx + 1] : null; let phase = rawPhase && (ALL_PHASES as string[]).includes(rawPhase) ? (rawPhase as CyclePhase) @@ -214,6 +234,35 @@ function parseArgs(args: string[]): DreamArgs { } } + // issue #2860: --once requires an EXPLICIT single --phase target (typed + // by the user, not merely implied by --input/--drain — see + // `phaseWasExplicit` above). Bare `--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. An implicit phase + // (from --input or --drain) is rejected too: --drain returns before + // onceForPhase is ever read, and --input already bypasses the + // synthesize gate on its own, so --once would silently do nothing in + // either case — reject loudly instead of pretending it worked (Codex + // review finding). + // + // 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'); + const wantsHelp = args.includes('--help') || args.includes('-h'); + if (once && !phaseWasExplicit && !wantsHelp) { + console.error( + '--once requires an explicit --phase (bypasses that one ' + + 'phase\'s dream..enabled / cycle..enabled gate for ' + + 'this run only; never touches config). A phase implied by --input ' + + 'or --drain does not count — --once would silently do nothing for ' + + 'those. Usage: gbrain dream --phase --once', + ); + process.exit(2); + } + return { json: args.includes('--json'), dryRun: args.includes('--dry-run'), @@ -229,6 +278,7 @@ function parseArgs(args: string[]): DreamArgs { source, drain, windowSeconds, + once, }; } @@ -310,6 +360,17 @@ Options: "--dry-run" does NOT mean "zero LLM calls." --json Emit the CycleReport as JSON (agent-readable) --phase Run a single phase: ${ALL_PHASES.join(' | ')} + --once With --phase : run that phase once even if its + own dream..enabled / cycle..enabled + config gate is false. Never reads or writes config — + unlike toggling the flag on/off around the run, a + crash mid-invocation can't leave it stuck. Applies to + patterns, synthesize, conversation_facts_backfill, + enrich_thin, skillopt; no-op on phases with no such + gate. Requires an EXPLICIT --phase — a phase + implied by --input or --drain does not count (bare + --once, or --once with --input/--drain and no + explicit --phase, is a usage error). --pull git pull the brain repo before syncing (default: no pull) --dir Brain directory (default: configured brain). On a postgres/remote brain with no local checkout, the @@ -353,6 +414,7 @@ Examples: gbrain dream gbrain dream --dry-run --json gbrain dream --phase lint + gbrain dream --phase patterns --once # run once, ignore dream.patterns.enabled=false gbrain dream --phase synthesize --input ~/transcripts/2026-04-25.txt gbrain dream --phase synthesize --from 2026-04-01 --to 2026-04-25 0 2 * * * gbrain dream --json # nightly via cron @@ -594,6 +656,9 @@ export async function runDream(engine: BrainEngine | null, args: string[]): Prom synthFrom: opts.from ?? undefined, synthTo: opts.to ?? undefined, synthBypassDreamGuard: opts.bypassDreamGuard, + // issue #2860: opts.phase is guaranteed non-null here when opts.once is + // set (parseArgs enforces --once requires --phase). + onceForPhase: opts.once ? opts.phase! : undefined, }); if (opts.json) { diff --git a/src/core/cycle.ts b/src/core/cycle.ts index 100eab2a3..a10b1ca80 100644 --- a/src/core/cycle.ts +++ b/src/core/cycle.ts @@ -479,6 +479,27 @@ export interface CycleOpts { * Validated via `assertValidSourceId` in `cycleLockIdFor` (defense-in-depth). */ sourceId?: string; + /** + * issue #2860 — one-shot per-invocation bypass of a phase's own + * `dream..enabled` / `cycle..enabled` config gate. Wired + * from `gbrain dream --phase --once`. + * + * Deliberately typed as the SINGLE named `CyclePhase`, not a boolean — + * each gated phase's dispatch block below only honors the override when + * `onceForPhase` matches ITS OWN phase name, so the bypass can never leak + * to a different phase even if a caller passes a wider `phases` array + * than the CLI does (the CLI always restricts to `phases: [phase]`). + * + * Never reads or writes config — the phase still evaluates its config + * gate every call; this only overrides the boolean OUTCOME for that one + * call. Applies to: patterns, synthesize, conversation_facts_backfill, + * enrich_thin, skillopt (the phases that gate on a `.enabled` config + * key read inside the phase's own module). Does NOT apply to + * extract_atoms / synthesize_concepts — those are pack-gated via + * `packDeclaresPhase`, a different mechanism with its own existing + * one-shot escape hatch (`--drain` for extract_atoms). + */ + onceForPhase?: CyclePhase; /** * Absolute wall-clock deadline (epoch ms) of the enclosing minion job, * from `MinionJobContext.deadlineAtMs` (the claim-time `timeout_at` @@ -1695,6 +1716,7 @@ export async function runCycle( // #1586: scope synthesized writes to the cycle's resolved source // (explicit --source wins, else derived from the checkout dir). sourceId: cycleSourceId, + once: opts.onceForPhase === 'synthesize', })); result.duration_ms = duration_ms; phaseResults.push(result); @@ -1898,6 +1920,7 @@ export async function runCycle( brainDir, dryRun, yieldDuringPhase: opts.yieldDuringPhase, + once: opts.onceForPhase === 'patterns', deadlineAtMs: opts.deadlineAtMs ?? null, })); result.duration_ms = duration_ms; @@ -2114,7 +2137,11 @@ export async function runCycle( progress.start('cycle.conversation_facts_backfill'); const { runPhaseConversationFactsBackfill } = await import('./cycle/conversation-facts-backfill.ts'); const { result, duration_ms } = await timePhase(() => - runPhaseConversationFactsBackfill(engine, { dryRun, signal: opts.signal }), + runPhaseConversationFactsBackfill(engine, { + dryRun, + signal: opts.signal, + once: opts.onceForPhase === 'conversation_facts_backfill', + }), ); result.duration_ms = duration_ms; phaseResults.push(result); @@ -2142,7 +2169,11 @@ export async function runCycle( progress.start('cycle.enrich_thin'); const { runPhaseEnrichThin } = await import('./cycle/enrich-thin.ts'); const { result, duration_ms } = await timePhase(() => - runPhaseEnrichThin(engine, { dryRun, signal: opts.signal }), + runPhaseEnrichThin(engine, { + dryRun, + signal: opts.signal, + once: opts.onceForPhase === 'enrich_thin', + }), ); result.duration_ms = duration_ms; phaseResults.push(result); @@ -2173,6 +2204,7 @@ export async function runCycle( runPhaseSkillopt({ engine, dryRun, + once: opts.onceForPhase === 'skillopt', ...(opts.signal ? { signal: opts.signal } : {}), }), ); diff --git a/src/core/cycle/conversation-facts-backfill.ts b/src/core/cycle/conversation-facts-backfill.ts index a6b3db7bc..68464a850 100644 --- a/src/core/cycle/conversation-facts-backfill.ts +++ b/src/core/cycle/conversation-facts-backfill.ts @@ -57,6 +57,14 @@ import { export interface ConversationFactsBackfillPhaseOpts { dryRun?: boolean; signal?: AbortSignal; + /** + * issue #2860 — `gbrain dream --phase conversation_facts_backfill --once`. + * Bypasses the `cycle.conversation_facts_backfill.enabled` gate for THIS + * call only; never reads or writes config. Per-source + brain-wide cost/ + * walltime caps still apply — the override lifts the on/off switch, not + * the spend guards. + */ + once?: boolean; } /** Phase return shape (matches PhaseResult contract from cycle.ts). */ @@ -155,17 +163,23 @@ export async function runPhaseConversationFactsBackfill( const cfg = await loadCfg(engine); if (!cfg.enabled) { - return { - phase: 'conversation_facts_backfill', - status: 'skipped', - duration_ms: 0, - summary: 'cycle.conversation_facts_backfill.enabled=false (default OFF)', - details: { - reason: 'disabled', - enable_hint: - 'gbrain config set cycle.conversation_facts_backfill.enabled true', - }, - }; + if (!opts.once) { + return { + phase: 'conversation_facts_backfill', + status: 'skipped', + duration_ms: 0, + summary: 'cycle.conversation_facts_backfill.enabled=false (default OFF)', + details: { + reason: 'disabled', + enable_hint: + 'gbrain config set cycle.conversation_facts_backfill.enabled true', + }, + }; + } + process.stderr.write( + '[dream] --once: cycle.conversation_facts_backfill.enabled is false but ' + + '--phase conversation_facts_backfill --once forces this run (config untouched)\n', + ); } const startedAt = Date.now(); diff --git a/src/core/cycle/enrich-thin.ts b/src/core/cycle/enrich-thin.ts index 02a89966f..85c3f9f6e 100644 --- a/src/core/cycle/enrich-thin.ts +++ b/src/core/cycle/enrich-thin.ts @@ -45,6 +45,12 @@ import { export interface EnrichThinPhaseOpts { dryRun?: boolean; signal?: AbortSignal; + /** + * issue #2860 — `gbrain dream --phase enrich_thin --once`. Bypasses the + * `cycle.enrich_thin.enabled` gate for THIS call only; never reads or + * writes config. Per-source + brain-wide cost/walltime caps still apply. + */ + once?: boolean; } export interface EnrichThinPhaseResult { @@ -139,16 +145,22 @@ export async function runPhaseEnrichThin( const cfg = await loadCfg(engine); if (!cfg.enabled) { - return { - phase: 'enrich_thin', - status: 'skipped', - duration_ms: 0, - summary: 'cycle.enrich_thin.enabled=false (default OFF)', - details: { - reason: 'disabled', - enable_hint: 'gbrain config set cycle.enrich_thin.enabled true', - }, - }; + if (!opts.once) { + return { + phase: 'enrich_thin', + status: 'skipped', + duration_ms: 0, + summary: 'cycle.enrich_thin.enabled=false (default OFF)', + details: { + reason: 'disabled', + enable_hint: 'gbrain config set cycle.enrich_thin.enabled true', + }, + }; + } + process.stderr.write( + '[dream] --once: cycle.enrich_thin.enabled is false but ' + + '--phase enrich_thin --once forces this run (config untouched)\n', + ); } const startedAt = Date.now(); diff --git a/src/core/cycle/patterns.ts b/src/core/cycle/patterns.ts index 788381a63..13b202e12 100644 --- a/src/core/cycle/patterns.ts +++ b/src/core/cycle/patterns.ts @@ -37,6 +37,12 @@ export interface PatternsPhaseOpts { brainDir: string; dryRun: boolean; yieldDuringPhase?: () => Promise; + /** + * issue #2860 — `gbrain dream --phase patterns --once`. Bypasses the + * `dream.patterns.enabled` gate for THIS call only; never reads or + * writes config. + */ + once?: boolean; /** * Absolute deadline (epoch ms) of the enclosing minion job, or null for * direct callers (`gbrain dream`). When set, the subagent's job timeout @@ -99,7 +105,13 @@ export async function runPhasePatterns( const config = await loadPatternsConfig(engine); if (!config.enabled) { - return skipped('disabled', 'dream.patterns.enabled is false'); + if (!opts.once) { + return skipped('disabled', 'dream.patterns.enabled is false'); + } + process.stderr.write( + '[dream] --once: dream.patterns.enabled is false but ' + + '--phase patterns --once forces this run (config untouched)\n', + ); } // Gather reflections within lookback window. diff --git a/src/core/cycle/synthesize.ts b/src/core/cycle/synthesize.ts index 59aad6630..eea299413 100644 --- a/src/core/cycle/synthesize.ts +++ b/src/core/cycle/synthesize.ts @@ -252,6 +252,13 @@ export interface SynthesizePhaseOpts { * correct (source_id, slug) row. Unset → legacy 'default'. */ sourceId?: string; + /** + * issue #2860 — `gbrain dream --phase synthesize --once`. Bypasses the + * `dream.synthesize.enabled` gate for THIS call only (does NOT bypass + * the `session_corpus_dir` not-configured check — there's nothing to + * run without a corpus). Never reads or writes config. + */ + once?: boolean; } export async function runPhaseSynthesize( @@ -285,8 +292,14 @@ export async function runPhaseSynthesize( 'dream.synthesize.session_corpus_dir is unset'); } if (!opts.inputFile && !config.enabled) { - return skipped('not_configured', - 'dream.synthesize.enabled is explicitly false'); + if (!opts.once) { + return skipped('not_configured', + 'dream.synthesize.enabled is explicitly false'); + } + process.stderr.write( + '[dream] --once: dream.synthesize.enabled is false but ' + + '--phase synthesize --once forces this run (config untouched)\n', + ); } // Cooldown check (skipped for explicit --input / --date / --from / --to runs). diff --git a/src/core/skillopt/cycle-phase.ts b/src/core/skillopt/cycle-phase.ts index d10c79706..b3fcb6bae 100644 --- a/src/core/skillopt/cycle-phase.ts +++ b/src/core/skillopt/cycle-phase.ts @@ -29,6 +29,12 @@ export interface SkilloptPhaseOpts { engine: BrainEngine; dryRun?: boolean; signal?: AbortSignal; + /** + * issue #2860 — `gbrain dream --phase skillopt --once`. Bypasses the + * `cycle.skillopt.enabled` feature flag for THIS call only; never reads + * or writes config. Per-skill + brain-wide cost caps still apply. + */ + once?: boolean; } export interface SkilloptPhaseResult { @@ -63,13 +69,19 @@ export async function runPhaseSkillopt(opts: SkilloptPhaseOpts): Promise { expect(syncCalls.at(-1)?.sourceId).toBe(''); }); }); + +// ─── issue #2860: --once one-shot phase-enabled bypass (onceForPhase) ─ +// +// CycleOpts.onceForPhase is deliberately typed as a single CyclePhase (not +// a boolean) so the override can never leak to a phase other than the one +// it names — even if a caller passes a wider `phases` array than the CLI +// does (dream.ts always restricts to `phases: [phase]` when --once is +// set). This exercises that boundary directly against runCycle, using the +// real (unmocked) patterns.ts module — cheap because with zero reflections +// seeded it never reaches an LLM call regardless of the enabled gate. +describe('runCycle — onceForPhase bypasses only the named phase (issue #2860)', () => { + beforeEach(async () => { + await truncateCycleLocks(sharedEngine); + await sharedEngine.setConfig('dream.patterns.enabled', 'false'); + }); + + afterEach(async () => { + // Restore default so later describe blocks in this file (which run + // patterns as part of the full ALL_PHASES cycle) aren't affected. + await sharedEngine.setConfig('dream.patterns.enabled', 'true'); + }); + + test('onceForPhase matching the requested phase bypasses its disabled gate', async () => { + const report = await runCycle(sharedEngine, { + brainDir: '/tmp/brain-2860-a', + phases: ['patterns'], + onceForPhase: 'patterns', + }); + const patternsResult = report.phases.find(p => p.phase === 'patterns'); + // Bypassed 'disabled' → falls through to the next gate (no reflections + // seeded). If the override didn't work, this would read 'disabled'. + expect(patternsResult?.status).toBe('skipped'); + expect((patternsResult?.details as { reason?: string })?.reason).toBe('insufficient_evidence'); + }); + + test('onceForPhase naming a DIFFERENT phase does not leak the bypass', async () => { + const report = await runCycle(sharedEngine, { + brainDir: '/tmp/brain-2860-b', + phases: ['patterns'], + onceForPhase: 'synthesize', // mismatched — must NOT bypass patterns' gate + }); + const patternsResult = report.phases.find(p => p.phase === 'patterns'); + expect(patternsResult?.status).toBe('skipped'); + expect((patternsResult?.details as { reason?: string })?.reason).toBe('disabled'); + }); + + test('no onceForPhase set → unchanged behavior (still gated)', async () => { + const report = await runCycle(sharedEngine, { + brainDir: '/tmp/brain-2860-c', + phases: ['patterns'], + }); + const patternsResult = report.phases.find(p => p.phase === 'patterns'); + expect(patternsResult?.status).toBe('skipped'); + expect((patternsResult?.details as { reason?: string })?.reason).toBe('disabled'); + }); + + test('config is never written by the override', async () => { + await runCycle(sharedEngine, { + brainDir: '/tmp/brain-2860-d', + phases: ['patterns'], + onceForPhase: 'patterns', + }); + expect(await sharedEngine.getConfig('dream.patterns.enabled')).toBe('false'); + }); +}); diff --git a/test/dream-cli-flags.test.ts b/test/dream-cli-flags.test.ts index 1ffa759ce..3e4a7510c 100644 --- a/test/dream-cli-flags.test.ts +++ b/test/dream-cli-flags.test.ts @@ -135,4 +135,50 @@ describe('dream CLI flag wiring', () => { 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 '); + // --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'); + }); + }); }); diff --git a/test/dream.test.ts b/test/dream.test.ts index 0d4510218..f4380eb14 100644 --- a/test/dream.test.ts +++ b/test/dream.test.ts @@ -151,6 +151,119 @@ describe('runDream — --phase restricts the cycle', () => { }); }); +// ─── --once (issue #2860) ─────────────────────────────────────────── + +describe('runDream — --once (issue #2860)', () => { + let repo: string; + let engine: InstanceType; + + 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 an explicit --phase /); + exitSpy.mockRestore(); + errSpy.mockRestore(); + }); + + // Codex P3 finding: --input implicitly sets `phase = 'synthesize'` and + // --drain implicitly sets `phase = 'extract_atoms'` — an EARLIER version + // of the --once validation checked the derived `phase` value, which was + // already non-null by the time it ran, so both of these silently slipped + // past the "explicit --phase required" contract (and --once became a + // no-op: --drain returns before onceForPhase is ever read; --input + // already bypasses the synthesize gate on its own). The fix validates + // against `phaseWasExplicit` (whether the user actually typed --phase) + // instead of the derived value. + test('--input --once (no explicit --phase) exits 2 — an implied phase does not count', async () => { + const exitSpy = spyOn(process, 'exit').mockImplementation(() => { throw new Error('EXIT'); }); + const errSpy = spyOn(console, 'error').mockImplementation(() => {}); + try { + await runDream(engine, ['--dir', repo, '--input', '/tmp/gbrain-2860-nonexistent.txt', '--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 an explicit --phase /); + exitSpy.mockRestore(); + errSpy.mockRestore(); + }); + + test('--drain --once (no explicit --phase) exits 2 — an implied phase does not count', async () => { + const exitSpy = spyOn(process, 'exit').mockImplementation(() => { throw new Error('EXIT'); }); + const errSpy = spyOn(console, 'error').mockImplementation(() => {}); + try { + await runDream(engine, ['--dir', repo, '--drain', '--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 an explicit --phase /); + 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', () => { diff --git a/test/e2e/dream-patterns-pglite.test.ts b/test/e2e/dream-patterns-pglite.test.ts index 8d55f4a5e..08719166b 100644 --- a/test/e2e/dream-patterns-pglite.test.ts +++ b/test/e2e/dream-patterns-pglite.test.ts @@ -99,6 +99,45 @@ describe('E2E patterns — disabled', () => { await rig.cleanup(); } }, 30_000); + + // issue #2860 — `gbrain dream --phase patterns --once` must bypass the + // enabled gate WITHOUT touching config (the whole point: the old + // toggle-on/run/toggle-off workaround left `enabled` stuck true forever + // if the process died between steps, running patterns hourly and + // burning ~$400 in LLM spend before it was caught). + test('once:true bypasses the disabled gate for this call only', async () => { + const rig = await setupRig(); + try { + await rig.engine.setConfig('dream.patterns.enabled', 'false'); + + // Without --once: still skipped as 'disabled' (unchanged behavior). + const gated = await runPhasePatterns(rig.engine, { + brainDir: rig.brainDir, + dryRun: false, + }); + expect(gated.status).toBe('skipped'); + expect((gated.details as { reason?: string }).reason).toBe('disabled'); + + // With --once: the disabled gate no longer short-circuits — the call + // proceeds past it to the NEXT gate (insufficient_evidence, since no + // reflections were seeded). If --once didn't work, this would still + // report 'disabled'. + const forced = await runPhasePatterns(rig.engine, { + brainDir: rig.brainDir, + dryRun: false, + once: true, + }); + expect(forced.status).toBe('skipped'); + expect((forced.details as { reason?: string }).reason).toBe('insufficient_evidence'); + + // Config was NEVER written — the override is call-scoped only, unlike + // the toggle-on/toggle-off workaround the issue exists to replace. + const stillDisabled = await rig.engine.getConfig('dream.patterns.enabled'); + expect(stillDisabled).toBe('false'); + } finally { + await rig.cleanup(); + } + }, 30_000); }); describe('E2E patterns — insufficient_evidence', () => {