mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-29 19:01:39 +00:00
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>
This commit is contained in:
co-authored by
Claude Sonnet 5
masashiono0611
Garry Tan
parent
e320ad71b3
commit
60125ee626
@@ -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.<phase>.enabled` / `cycle.<phase>.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 <name>`; 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 <f> --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 <name> (bypasses that one ' +
|
||||
'phase\'s dream.<phase>.enabled / cycle.<phase>.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 <name> --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 <name> Run a single phase: ${ALL_PHASES.join(' | ')}
|
||||
--once With --phase <name>: run that phase once even if its
|
||||
own dream.<phase>.enabled / cycle.<phase>.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 <name> — 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 <path> 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) {
|
||||
|
||||
+34
-2
@@ -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.<phase>.enabled` / `cycle.<phase>.enabled` config gate. Wired
|
||||
* from `gbrain dream --phase <name> --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 } : {}),
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -37,6 +37,12 @@ export interface PatternsPhaseOpts {
|
||||
brainDir: string;
|
||||
dryRun: boolean;
|
||||
yieldDuringPhase?: () => Promise<void>;
|
||||
/**
|
||||
* 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.
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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<Skillop
|
||||
enabled = v === 'true';
|
||||
} catch { /* default OFF */ }
|
||||
if (!enabled) {
|
||||
return {
|
||||
phase: 'skillopt',
|
||||
status: 'skipped',
|
||||
duration_ms: Date.now() - start,
|
||||
summary: 'feature flag off (gbrain config set cycle.skillopt.enabled true to enable)',
|
||||
details: { reason: 'feature_flag_off' },
|
||||
};
|
||||
if (!opts.once) {
|
||||
return {
|
||||
phase: 'skillopt',
|
||||
status: 'skipped',
|
||||
duration_ms: Date.now() - start,
|
||||
summary: 'feature flag off (gbrain config set cycle.skillopt.enabled true to enable)',
|
||||
details: { reason: 'feature_flag_off' },
|
||||
};
|
||||
}
|
||||
process.stderr.write(
|
||||
'[dream] --once: cycle.skillopt.enabled is false but ' +
|
||||
'--phase skillopt --once forces this run (config untouched)\n',
|
||||
);
|
||||
}
|
||||
|
||||
// Per-skill + brain-wide cost caps.
|
||||
|
||||
@@ -560,3 +560,68 @@ describe('runCycle — sourceId resolution (regression #475)', () => {
|
||||
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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 <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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -151,6 +151,119 @@ 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 an explicit --phase <name>/);
|
||||
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 <file> --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 <name>/);
|
||||
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 <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', () => {
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user