mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
v0.42.52.0 fix(reliability): autopilot dead-job storm + supervisor wedge + sync/status/minion reliability (#2194 #2227 #1994 #1737 #1738 #1950 #1984) (#2287)
* test(supervisor): pin LOCK_HELD fence-exit is never counted as a crash (#2227) A duplicate supervisor loses the queue-scoped DB singleton lock (#1849) and exits LOCK_HELD before spawning a worker or emitting 'started'. summarizeCrashes counts only worker_exited, so the fence path is structurally uncountable. Pin it so a future refactor that logs worker_exited on the fence path fails here instead of silently re-introducing the crash-budget breaker-trip loop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(autopilot): per-source cycle binds FS phases to source.local_path, not global repo (#2194 #2227) A per-source autopilot-cycle inherited the global sync.repo_path as brainDir while stamping DB freshness for source_id — mixed scope. FS phases (sync/lint/extract) ran against the wrong tree, so the failure-cooldown and freshness gates would attribute work to the wrong source. Resolve the source's local_path in the handler (reuse the archive-recheck SELECT) and bind brainDir to it; a pure-DB source gets null (FS phases skip) instead of falling through to the global checkout. Legacy no-source dispatch keeps the global repoPath. Prerequisite for the cooldown/split commits (codex outside-voice #8). Resolves TODOS:634. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(supervisor): detect a live supervisor via the DB lock under split $HOME (#2227) jobs supervisor status + doctor read the HOME-derived pidfile, so a supervisor started under a different $HOME (keeper=/root vs ops=/data) read as 'not running' while healthy — the false signal that drives an operator to spawn a duplicate. Both surfaces now fall back to the queue-scoped DB singleton lock (#1849), the HOME-independent authority, when the pidfile shows nothing. New isLockHolderLive keys on lock freshness (ttl + heartbeat steal-grace), never process.kill, so PID reuse can't false-positive (pid-liveness-alone-pid-reuse). Status surfaces the holder host/pid + recorded concurrency/max-rss from the latest started event. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(supervisor): degraded retry instead of permanent give-up on crash storm (#1994 #2227) max_crashes_exceeded gave up forever, so a transient DB-pooler blip that tripped the soft budget wedged the queue until a human restart (#2227's breaker-trips tail). Crossing the soft budget now enters degraded mode: keep respawning with capped exponential backoff (60s cap — a paced retry, not a hot loop) and emit a loud crash_budget_degraded health_warn. The existing stable-run reset clears the count once a respawn survives >5min, so a recovered DB self-heals. Permanent give-up fires only at a much-higher hard ceiling (maxCrashes × 10), tunable/disablable via GBRAIN_SUPERVISOR_HARD_STOP_CRASHES (0 = never). Resolves TODOS:92. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(autopilot): clamp fan-out to worker concurrency + doctor warning (#2194) Fan-out resolved to 4 (Postgres) regardless of worker --concurrency, so surplus cycles queued behind the worker and raced the stalled-sweeper. Two fixes for the same mismatch: - resolveEffectiveFanoutMax clamps to max(1, concurrency-1) (reserve a slot), gated on a LIVE DB-lock holder so a stale started-audit row can't shrink throughput (codex #9/D5); no live holder → unknown → unclamped base. Escape hatch autopilot.fanout_clamp_to_concurrency. - doctor's autopilot_fanout_concurrency check warns when fan-out exceeds effective slots — the misconfig was silent before. Advisory (started-event concurrency), wired into both doctor surfaces. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(autopilot): per-source failure cooldown — break the dead-job storm (#2194) Only SUCCESS gated dispatch, so a source whose cycle kept failing/timing-out re-fanned-out every 5-min tick forever (200+ dead jobs/24h). Now a failed source backs off with bounded exponential cooldown (10→120min). Read at DISPATCH from minion_jobs dead/failed rows (timeouts/RSS-kills dead-letter via SQL and never run handler code, so a write-only hook would miss them) AND re-checked at CLAIM time in the handler (codex #5: already-queued/retrying jobs). A success clears it (codex #7); null-source rows excluded (codex #6); engine-parity via executeRaw. Disable with autopilot.failure_cooldown_min=0. Fail-open if config/history reads error. Surfaced via fanout_cooldown_skipped + the fanout summary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(autopilot): split the cycle — per-source phases + one global-maintenance job (#2194 #2227) N per-source cycles each ran the brain-wide global phases (embed-all/orphans/ purge/…) concurrently, thrashing the same rows and taking the worker 4→10GB in <60s → RSS-kill → orphaned stalls. Split them: per-source jobs now run only source-scoped (+ mixed) phases and stamp last_source_cycle_at; a new autopilot-global-maintenance job runs the global phases ONCE per window (idempotency_key + maxWaiting:1 = structural single-flight) and stamps autopilot.last_global_at. This is the codex-endorsed design that replaced the rejected skip-and-stamp-fresh approach (codex #1/#2): no freshness poisoning, no starvation — global work always runs as its own job, never marked done when it wasn't. PHASE_SCOPE is now a runtime partition (GLOBAL ∪ NON_GLOBAL == ALL). last_full_cycle_at still written for doctor/legacy (no longer a global gate). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(doctor): guard nullable engine in supervisor DB-lock fallback (#2227) Follow-up to the supervisor-visibility commit: doctor's engine binding is BrainEngine | null, so the inspectLock fallback must guard on a non-null engine (tsc TS2345). No behavior change — a null engine simply skips the DB-lock probe and falls back to the pidfile reading, as before. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(doctor): categorize autopilot_fanout_concurrency check as ops (#2194) Follow-up to the fan-out/concurrency commit: the doctor-categories drift guard requires every check name in doctor.ts to belong to exactly one category set. Add the new autopilot_fanout_concurrency check to OPS_CHECK_NAMES (infrastructure liveness, alongside wedged_queue/supervisor). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: update KEY_FILES for the autopilot cycle split + supervisor degraded-retry (#2194 #2227) Post-ship document-release: refresh the KEY_FILES current-state entries that drifted — cycle.ts (GLOBAL/NON_GLOBAL phase split + last_source_cycle_at / autopilot.last_global_at), jobs.ts (per-source local_path brainDir, claim-time cooldown, autopilot-global-maintenance handler), supervisor.ts + child-worker (degraded retry instead of permanent give-up; hard ceiling), db-lock.ts (isLockHolderLive), handler-timeouts (new handler). Regenerated llms bundle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(minions): handleTimeouts counts the timed-out run as a spent attempt (#1737) The per-job timeout_at dead-letter (handleTimeouts) set status='dead' without incrementing attempts_made, unlike the wall-clock and stall dead-letter siblings. It is the FIRST killer to fire for the long-lane handlers (subagent / embed-backfill / autopilot-cycle) because timeout_ms is stamped at submit, so a timed-out long job reported `attempts: 0/N (started: N)`. Mirror the siblings with attempts_made + 1 (terminal, no retry). Safe against double-count: the worker sweep runs handleStalled -> handleTimeouts -> handleWallClockTimeouts sequentially and awaited, each guarded on status='active', so the first to dead-letter excludes the row from the rest. Regression assertions added (test/minions.test.ts + e2e/minions-resilience.test.ts) so the increment can't be silently dropped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agent): recognize trailing switches in `agent run`, keep prompts freeform (#1738) parseRunFlags() broke flag parsing at the first positional token, so any flag after the prompt (`gbrain agent run "do X" --detach`) was swallowed into the prompt string and silently ignored. Now the no-value switches --detach/--follow/ --no-follow are hoisted when they trail the prompt, while everything else stays verbatim: an unknown --word is treated as prompt text (no "unknown flag" throw), a --switch mid-prompt is preserved, and `--` suppresses hoisting entirely for a literal escape. Value-flags now reject a missing or flag-shaped value (and --max-turns/--timeout-ms a non-number) instead of capturing undefined/NaN. Contract change: a prompt that starts with or trails an unguarded --word no longer errors; a literal trailing --detach needs `--`. Help text updated; tests revised + extended (test/agent-cli.test.ts). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sync): honest live-sync status + progress-aware stall-abort (#1950) Finishes the #2255 honest-freshness story for two gaps it left. (a) `gbrain sources status` printed "idle" while a sync proc held the per-source lock (the reported bug). New shared liveSyncStatus() helper in db-lock.ts reads the SAME live-lock signal `gbrain doctor` uses; runStatus now shows "running" (BACKFILL column + a sync_running field in --json) and suppresses the misleading "never synced" warning while a sync is live. One helper, so the surfaces can't drift (doctor/status retrofit tracked as a follow-up). (b) A sync wedged-but-alive kept refreshing its lock heartbeat (it fires on its own timer) and hadn't hit the wall-clock deadline, so only a manual pkill freed it. New in-band stall watchdog keys off FORWARD IMPORT PROGRESS (progress.tick), not the heartbeat: if no file completes for GBRAIN_SYNC_STALL_ABORT_SECONDS (default 900s), it aborts via a controller composed into opts.signal, so the drain returns partial() (last_commit unchanged, next run resumes from the checkpoint) and withRefreshingLock releases the lock. Limits, documented in code: a single file slower than the window trips it; a fully starved event loop won't fire the timer (the wall-clock hard deadline is that backstop). Tests: liveSyncStatus (live/expired/none/per-source) in db-lock-inspect; the resolveStallAbortSeconds env matrix in sync-hard-deadline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(status): version field + per-section --deadline-ms budget (#1984) `gbrain status` had no version in its JSON envelope and could hang on a slow connection with no way to get a partial answer. Two additions: - version: the StatusReport JSON now carries the local gbrain CLI version so a poller can pin behavior to a build. Thin-client also surfaces remote_version (the brain server's version), and the get_status_snapshot MCP op reports its version for that parity. - --deadline-ms=N / --fast: a shared wall-clock budget. Each section is raced against the REMAINING budget via Promise.race (NOT process-watchdog, which SIGKILLs and can't return partial output), so one slow/hung section can't strand the snapshot — it's marked stale and the rest still return. The envelope gains partial:true + stale_sections[]; exit code stays 0 (a snapshot was produced). Invalid --deadline-ms → exit 2. Tests: parseDeadlineFlag + withSectionDeadline (hermetic), the usage-error exit, version presence in the PGLite envelope, and the op's version key. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sync): report stall_timeout distinctly + document in-flight limit (#1950) Pre-landing review (codex + adversarial): the stall watchdog aborted opts.signal but the per-iteration abort checks returned partial('timeout'), collapsing a wedge-reap into a user --timeout/SIGINT so JSON consumers couldn't tell them apart. Add a 'stall_timeout' reason (set via a stallAborted flag) on the three import-loop abort sites; deletes/renames-phase and checkpoint sites stay 'timeout'. Sharpen the watchdog comment: the abort is observed BETWEEN files, so a hang inside a single importFile is not interrupted until it returns (TODO: thread a cancellation signal through importFile). * fix(agent): `--` escape suppresses trailing-switch hoisting anywhere (#1738) Pre-landing review: the leading-flag loop breaks at the first positional, so the `escaped` flag only fired for a leading `--`. A `--` placed after a positional left trailing-switch hoisting active, so `agent run note -- body --detach` silently detached and dropped the `--` as junk. Suppress hoisting whenever a literal `--` appears in the prompt. Regression test added. * fix(status): deadline-ms usage-error + scoped stale_sections + cancel losing remote call (#1984) Pre-landing review (codex): (1) bare `--deadline-ms` with no value silently fell through to no-budget/--fast instead of a usage error; (2) thin-client timeout reported both sync+cycle stale even under `--section sync`, naming a section the caller excluded (local path was already correct); (3) the section race abandoned the remote promise locally but didn't cancel the in-flight MCP call — pass the budget as timeoutMs so the losing side actually cancels. Regression test added. * v0.42.52.0 fix(reliability): autopilot dead-job storm + supervisor wedge + sync/status/minion reliability (#2194 #2227 #1994 #1737 #1738 #1950 #1984) Bundles the already-reviewed autopilot/supervisor stabilization (#2194 #2227 #1994: cycle split, per-source failure cooldown, fan-out clamp, degraded supervisor retry, DB-lock live-supervisor detection) with four operational fixes: minion timeout attempt-accounting (#1737), agent-run trailing-flag parsing (#1738), honest live-sync sources status + progress-aware stall watchdog (#1950), and status version + --deadline-ms partial result (#1984). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: document GBRAIN_SYNC_STALL_ABORT_SECONDS env knob (#1950) Post-ship doc sync (/document-release): add the sync stall watchdog env var to the CLAUDE.md sync-tuning table (Five → Six knobs) + regenerate the llms bundle. * test: quarantine #2249 fanout tests as *.serial (R1 env-isolation) (#2194) The cherry-picked autopilot-fanout-clamp + doctor-autopilot-fanout-concurrency tests mutate process.env.GBRAIN_AUDIT_DIR in beforeEach/afterEach, which the check:test-isolation R1 lint flags (parallel shards load multiple files per process). Rename to *.serial.test.ts (sanctioned quarantine — they run under --max-concurrency=1) instead of restructuring the reviewed test bodies. No logic change; both files stay green (9 tests). Fixes the failing verify CI check. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
9bf96db807
commit
bb2e88c42a
+55
-3
@@ -43,7 +43,7 @@ describe('parseRunFlags', () => {
|
||||
expect(rest).toEqual(['hello', 'world']);
|
||||
});
|
||||
|
||||
test('flags before prompt are parsed, unknown token ends flag parsing', () => {
|
||||
test('leading flags parsed; first positional begins the prompt', () => {
|
||||
const { flags, rest } = agentTesting.parseRunFlags([
|
||||
'--model', 'claude-opus-4-7', '--max-turns', '30', 'summarize', 'everything',
|
||||
]);
|
||||
@@ -69,8 +69,60 @@ describe('parseRunFlags', () => {
|
||||
expect(rest).toEqual(['--not-a-flag']);
|
||||
});
|
||||
|
||||
test('unknown flag throws', () => {
|
||||
expect(() => agentTesting.parseRunFlags(['--what', 'x'])).toThrow(/unknown flag/);
|
||||
test('#1738: unknown --flag is prompt text, not an error', () => {
|
||||
const { rest } = agentTesting.parseRunFlags(['--what', 'x']);
|
||||
expect(rest).toEqual(['--what', 'x']);
|
||||
});
|
||||
|
||||
test('#1738: trailing --detach is hoisted out of the prompt', () => {
|
||||
const { flags, rest } = agentTesting.parseRunFlags(['do', 'the', 'thing', '--detach']);
|
||||
expect(flags.detach).toBe(true);
|
||||
expect(flags.follow).toBe(false);
|
||||
expect(rest).toEqual(['do', 'the', 'thing']);
|
||||
});
|
||||
|
||||
test('#1738: leading flags + trailing switch both apply', () => {
|
||||
const { flags, rest } = agentTesting.parseRunFlags(['--model', 'm', 'summarize', '--detach', '--no-follow']);
|
||||
expect(flags.model).toBe('m');
|
||||
expect(flags.detach).toBe(true);
|
||||
expect(flags.follow).toBe(false);
|
||||
expect(rest).toEqual(['summarize']);
|
||||
});
|
||||
|
||||
test('#1738: a --switch mid-prompt (not trailing) stays verbatim', () => {
|
||||
const { flags, rest } = agentTesting.parseRunFlags(['summarize', '--detach', 'the', 'doc']);
|
||||
expect(flags.detach).toBe(false);
|
||||
expect(rest).toEqual(['summarize', '--detach', 'the', 'doc']);
|
||||
});
|
||||
|
||||
test('#1738: a freeform prompt starting with --word is preserved', () => {
|
||||
const { rest } = agentTesting.parseRunFlags(['--note:', 'do', 'the', 'thing']);
|
||||
expect(rest).toEqual(['--note:', 'do', 'the', 'thing']);
|
||||
});
|
||||
|
||||
test('#1738: -- suppresses trailing-switch hoisting', () => {
|
||||
const { flags, rest } = agentTesting.parseRunFlags(['--', 'do', 'x', '--detach']);
|
||||
expect(flags.detach).toBe(false);
|
||||
expect(rest).toEqual(['do', 'x', '--detach']);
|
||||
});
|
||||
|
||||
test('#1738: -- AFTER a positional also suppresses hoisting (no silent detach flip)', () => {
|
||||
// The leading-flag loop breaks at the first positional, so the `escaped`
|
||||
// flag never fires for a `--` placed later. A literal `--` ANYWHERE must
|
||||
// still mean "hoist nothing" — otherwise `agent run note -- body --detach`
|
||||
// silently detaches and drops the `--` as junk.
|
||||
const { flags, rest } = agentTesting.parseRunFlags(['note', '--', 'body', '--detach']);
|
||||
expect(flags.detach).toBe(false);
|
||||
expect(rest).toEqual(['note', '--', 'body', '--detach']);
|
||||
});
|
||||
|
||||
test('#1738: value-flag missing its value throws a usage error', () => {
|
||||
expect(() => agentTesting.parseRunFlags(['--model'])).toThrow(/requires a value/);
|
||||
expect(() => agentTesting.parseRunFlags(['--model', '--detach', 'x'])).toThrow(/requires a value/);
|
||||
});
|
||||
|
||||
test('#1738: numeric value-flag rejects a non-number', () => {
|
||||
expect(() => agentTesting.parseRunFlags(['--max-turns', 'abc', 'x'])).toThrow(/expects a number/);
|
||||
});
|
||||
|
||||
test('--subagent-def + --timeout-ms parsed', () => {
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
/**
|
||||
* issue #2227/#2194 (TODOS:634, codex #8) — a per-source `autopilot-cycle`
|
||||
* binds its filesystem phases to the SOURCE's own checkout (`local_path`),
|
||||
* never the global brain's `sync.repo_path`.
|
||||
*
|
||||
* Pre-fix the handler fed `repoPath` (the global checkout) into runCycle even
|
||||
* when `source_id` was set, so FS phases (sync/lint/extract) ran against the
|
||||
* wrong tree while DB freshness was stamped for `source_id` — mixed scope.
|
||||
* That made the failure-cooldown and freshness gates attribute work to the
|
||||
* wrong source, the prerequisite codex flagged before the storm-breaker could
|
||||
* be trusted.
|
||||
*
|
||||
* Drives the REAL handler captured from registerBuiltinHandlers (not a
|
||||
* source-grep) so a reintroduced repoPath fallthrough fails here. PGLite
|
||||
* in-memory. The report's `brain_dir` mirrors the cycle's effective brainDir
|
||||
* (cycle.ts:2324), so it's the observable proxy for "which checkout did FS
|
||||
* phases bind to".
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
|
||||
import { mkdtempSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { registerBuiltinHandlers } from '../src/commands/jobs.ts';
|
||||
import { resetPgliteState } from './helpers/reset-pglite.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.disconnect();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await resetPgliteState(engine);
|
||||
});
|
||||
|
||||
async function captureHandlers(): Promise<Map<string, (job: any) => Promise<any>>> {
|
||||
const handlers = new Map<string, (job: any) => Promise<any>>();
|
||||
const fakeWorker = { register(name: string, fn: (job: any) => Promise<any>) { handlers.set(name, fn); } };
|
||||
await registerBuiltinHandlers(fakeWorker as never, engine);
|
||||
return handlers;
|
||||
}
|
||||
|
||||
describe('autopilot-cycle handler — per-source checkout binding (#2227/#2194)', () => {
|
||||
test('source_id with local_path → brainDir is the SOURCE checkout, not the global repo', async () => {
|
||||
const sourceDir = mkdtempSync(join(tmpdir(), 'gbrain-src-'));
|
||||
// A DIFFERENT global checkout must NOT win for a per-source job.
|
||||
await engine.setConfig('sync.repo_path', '/some/global/brain/checkout');
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config, archived, created_at)
|
||||
VALUES ('repo-a', 'Repo A', $1, '{}'::jsonb, false, now())`,
|
||||
[sourceDir],
|
||||
);
|
||||
|
||||
const handlers = await captureHandlers();
|
||||
const handler = handlers.get('autopilot-cycle')!;
|
||||
// DB-only phase keeps the test cheap; brain_dir is stamped from opts.brainDir
|
||||
// regardless of which phases run, so it still proves the binding.
|
||||
const result = await handler({
|
||||
data: { source_id: 'repo-a', phases: ['resolve_symbol_edges'] },
|
||||
signal: undefined,
|
||||
});
|
||||
|
||||
expect(result.report.brain_dir).toBe(sourceDir);
|
||||
expect(result.report.brain_dir).not.toBe('/some/global/brain/checkout');
|
||||
});
|
||||
|
||||
test('source_id with NULL local_path → brainDir is null (FS phases skip), never the global repo', async () => {
|
||||
// The mixed-scope bug: a pure-DB source must NOT fall through to repoPath.
|
||||
await engine.setConfig('sync.repo_path', '/some/global/brain/checkout');
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, local_path, config, archived, created_at)
|
||||
VALUES ('db-only', 'DB Only', NULL, '{}'::jsonb, false, now())`,
|
||||
[],
|
||||
);
|
||||
|
||||
const handlers = await captureHandlers();
|
||||
const handler = handlers.get('autopilot-cycle')!;
|
||||
const result = await handler({
|
||||
data: { source_id: 'db-only', phases: ['resolve_symbol_edges'] },
|
||||
signal: undefined,
|
||||
});
|
||||
|
||||
expect(result.report.brain_dir).toBeNull();
|
||||
expect(result.report.brain_dir).not.toBe('/some/global/brain/checkout');
|
||||
});
|
||||
|
||||
test('legacy (no source_id) keeps the global repoPath — back-compat', async () => {
|
||||
const globalDir = mkdtempSync(join(tmpdir(), 'gbrain-global-'));
|
||||
await engine.setConfig('sync.repo_path', globalDir);
|
||||
|
||||
const handlers = await captureHandlers();
|
||||
const handler = handlers.get('autopilot-cycle')!;
|
||||
const result = await handler({
|
||||
data: { phases: ['resolve_symbol_edges'] },
|
||||
signal: undefined,
|
||||
});
|
||||
|
||||
expect(result.report.brain_dir).toBe(globalDir);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,151 @@
|
||||
/**
|
||||
* #2194 fix #2 — failure cooldown (the storm-breaker).
|
||||
*
|
||||
* A source whose autopilot-cycle keeps failing re-dispatched every 5-min tick
|
||||
* (only SUCCESS gated dispatch), so the same handful of sources failed and
|
||||
* re-fanned-out forever — 200+ dead jobs/24h. The cooldown backs a failed
|
||||
* source off with bounded exponential delay, read at dispatch from minion_jobs
|
||||
* (dead/failed rows) AND re-checked at claim time (codex #5). A success clears
|
||||
* it (codex #7). These tests pin the pure math, the engine query, the
|
||||
* null-source guard (codex #6), and the dispatch/claim-time gates.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { resetPgliteState } from './helpers/reset-pglite.ts';
|
||||
import {
|
||||
cooldownMinForCount,
|
||||
isInFailureCooldown,
|
||||
readRecentSourceFailures,
|
||||
isSourceInCooldown,
|
||||
selectSourcesForDispatch,
|
||||
resolveFailureCooldownOpts,
|
||||
type SourceFailure,
|
||||
type CooldownOpts,
|
||||
} from '../src/commands/autopilot-fanout.ts';
|
||||
import type { SourceRow } from '../src/core/engine.ts';
|
||||
|
||||
const OPTS: CooldownOpts = { baseMin: 10, capMin: 120 };
|
||||
|
||||
describe('cooldownMinForCount — bounded exponential', () => {
|
||||
test('grows 10, 20, 40, 80 then caps at 120', () => {
|
||||
expect(cooldownMinForCount(1, OPTS)).toBe(10);
|
||||
expect(cooldownMinForCount(2, OPTS)).toBe(20);
|
||||
expect(cooldownMinForCount(3, OPTS)).toBe(40);
|
||||
expect(cooldownMinForCount(4, OPTS)).toBe(80);
|
||||
expect(cooldownMinForCount(5, OPTS)).toBe(120); // 160 capped to 120
|
||||
expect(cooldownMinForCount(99, OPTS)).toBe(120);
|
||||
});
|
||||
test('zero/negative count or disabled base → 0', () => {
|
||||
expect(cooldownMinForCount(0, OPTS)).toBe(0);
|
||||
expect(cooldownMinForCount(3, { baseMin: 0, capMin: 120 })).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isInFailureCooldown — pure decision', () => {
|
||||
const now = Date.UTC(2026, 5, 16, 12, 0, 0);
|
||||
const ago = (min: number) => new Date(now - min * 60_000);
|
||||
|
||||
test('no failure record → not in cooldown', () => {
|
||||
expect(isInFailureCooldown(undefined, null, now, OPTS)).toBe(false);
|
||||
});
|
||||
test('disabled (baseMin 0) → never in cooldown', () => {
|
||||
expect(isInFailureCooldown({ count: 5, lastFailedAt: ago(1) }, null, now, { baseMin: 0, capMin: 120 })).toBe(false);
|
||||
});
|
||||
test('failed 5min ago, count 1 (cooldown 10) → in cooldown', () => {
|
||||
expect(isInFailureCooldown({ count: 1, lastFailedAt: ago(5) }, null, now, OPTS)).toBe(true);
|
||||
});
|
||||
test('failed 15min ago, count 1 (cooldown 10) → recovered by time', () => {
|
||||
expect(isInFailureCooldown({ count: 1, lastFailedAt: ago(15) }, null, now, OPTS)).toBe(false);
|
||||
});
|
||||
test('success at/after the latest failure → cleared (codex #7)', () => {
|
||||
const failure: SourceFailure = { count: 3, lastFailedAt: ago(5) };
|
||||
expect(isInFailureCooldown(failure, ago(4), now, OPTS)).toBe(false); // success 4min ago > fail 5min ago
|
||||
});
|
||||
test('success BEFORE the latest failure, still in window → suppressed', () => {
|
||||
const failure: SourceFailure = { count: 3, lastFailedAt: ago(5) };
|
||||
expect(isInFailureCooldown(failure, ago(30), now, OPTS)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('selectSourcesForDispatch — cooldown bucket', () => {
|
||||
const src = (id: string): SourceRow => ({ id, name: id, config: {} } as SourceRow);
|
||||
test('a stale source in cooldown is held in skippedCooldown, not dispatched', () => {
|
||||
const sources = [src('a'), src('b')];
|
||||
const failures = new Map<string, SourceFailure>([
|
||||
['a', { count: 1, lastFailedAt: new Date(Date.now() - 60_000) }], // 1min ago, cooldown 10min
|
||||
]);
|
||||
const r = selectSourcesForDispatch(sources, 4, Date.now(), 60, failures, OPTS);
|
||||
expect(r.dispatch.map(s => s.id)).toEqual(['b']);
|
||||
expect(r.skippedCooldown.map(s => s.id)).toEqual(['a']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('readRecentSourceFailures + isSourceInCooldown (PGLite)', () => {
|
||||
let engine: PGLiteEngine;
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
}, 30000);
|
||||
afterAll(async () => { await engine.disconnect(); });
|
||||
beforeEach(async () => { await resetPgliteState(engine); });
|
||||
|
||||
async function addJob(status: string, sourceId: string | null, finishedMinAgo: number): Promise<void> {
|
||||
const finished = new Date(Date.now() - finishedMinAgo * 60_000).toISOString();
|
||||
const data = sourceId === null ? {} : { source_id: sourceId };
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO minion_jobs (name, status, data, finished_at) VALUES ('autopilot-cycle', $1, $2, $3)`,
|
||||
[status, data, finished],
|
||||
);
|
||||
}
|
||||
|
||||
test('groups dead/failed jobs by source with count + max(finished_at)', async () => {
|
||||
await addJob('dead', 'repo-a', 5);
|
||||
await addJob('failed', 'repo-a', 2);
|
||||
await addJob('dead', 'repo-b', 10);
|
||||
await addJob('completed', 'repo-a', 1); // not counted
|
||||
const map = await readRecentSourceFailures(engine, { sinceMin: 120 });
|
||||
expect(map.get('repo-a')?.count).toBe(2);
|
||||
expect(map.get('repo-b')?.count).toBe(1);
|
||||
expect(map.has('repo-a')).toBe(true);
|
||||
// last failed is the most recent of the two (2 min ago).
|
||||
const lastA = map.get('repo-a')!.lastFailedAt.getTime();
|
||||
expect(Date.now() - lastA).toBeLessThan(4 * 60_000);
|
||||
});
|
||||
|
||||
test('null source_id rows are excluded (codex #6)', async () => {
|
||||
await addJob('dead', null, 3);
|
||||
await addJob('dead', 'repo-c', 3);
|
||||
const map = await readRecentSourceFailures(engine, { sinceMin: 120 });
|
||||
expect(map.has('repo-c')).toBe(true);
|
||||
expect([...map.keys()].some(k => !k)).toBe(false);
|
||||
expect(map.size).toBe(1);
|
||||
});
|
||||
|
||||
test('failures older than the window are not counted', async () => {
|
||||
await addJob('dead', 'repo-old', 500); // way outside a 120min window
|
||||
const map = await readRecentSourceFailures(engine, { sinceMin: 120 });
|
||||
expect(map.has('repo-old')).toBe(false);
|
||||
});
|
||||
|
||||
test('isSourceInCooldown: recent failure → true; cleared after a success stamp', async () => {
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO sources (id, name, config, created_at) VALUES ('repo-cd', 'r', '{}'::jsonb, now())`, [],
|
||||
);
|
||||
await addJob('dead', 'repo-cd', 1); // 1 min ago, count 1 → 10min cooldown
|
||||
expect(await isSourceInCooldown(engine, 'repo-cd')).toBe(true);
|
||||
|
||||
// Operator repairs + a successful cycle stamps last_source_cycle_at NOW.
|
||||
await engine.updateSourceConfig('repo-cd', { last_source_cycle_at: new Date().toISOString() });
|
||||
expect(await isSourceInCooldown(engine, 'repo-cd')).toBe(false);
|
||||
});
|
||||
|
||||
test('isSourceInCooldown returns false when cooldown disabled (failure_cooldown_min=0)', async () => {
|
||||
await engine.setConfig('autopilot.failure_cooldown_min', '0');
|
||||
await addJob('dead', 'repo-dis', 1);
|
||||
expect(await isSourceInCooldown(engine, 'repo-dis')).toBe(false);
|
||||
const opts = await resolveFailureCooldownOpts(engine);
|
||||
expect(opts.baseMin).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,110 @@
|
||||
/**
|
||||
* #2194 fix #1 (codex #9 / D5): resolveEffectiveFanoutMax clamps the per-tick
|
||||
* fan-out to the worker's effective concurrency (max(1, concurrency-1),
|
||||
* reserving ≥1 slot) — but ONLY when a LIVE supervisor holds the queue lock.
|
||||
* A stale `started` audit row must not shrink throughput for a supervisor that
|
||||
* isn't running that config, so with no live holder the clamp is skipped and
|
||||
* the unclamped base is used.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test';
|
||||
import { mkdtempSync, writeFileSync, rmSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { tryAcquireDbLock } from '../src/core/db-lock.ts';
|
||||
import { supervisorLockId, SUPERVISOR_LOCK_TTL_MIN } from '../src/core/minions/supervisor.ts';
|
||||
import { computeSupervisorAuditFilename } from '../src/core/minions/handlers/supervisor-audit.ts';
|
||||
import { resolveEffectiveFanoutMax } from '../src/commands/autopilot-fanout.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
let auditDir: string;
|
||||
const prevAuditDir = process.env.GBRAIN_AUDIT_DIR;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
}, 30000);
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.disconnect();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
auditDir = mkdtempSync(join(tmpdir(), 'gbrain-clamp-'));
|
||||
process.env.GBRAIN_AUDIT_DIR = auditDir;
|
||||
await engine.executeRaw(`DELETE FROM gbrain_cycle_locks WHERE id LIKE 'gbrain-supervisor:%'`);
|
||||
// base fan-out: override to 8 so the clamp's effect is visible on PGLite
|
||||
// (whose natural default is 1). The clamp logic is engine-agnostic.
|
||||
await engine.setConfig('autopilot.fanout_max_per_tick', '8');
|
||||
await engine.setConfig('autopilot.fanout_clamp_to_concurrency', 'true');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (prevAuditDir === undefined) delete process.env.GBRAIN_AUDIT_DIR;
|
||||
else process.env.GBRAIN_AUDIT_DIR = prevAuditDir;
|
||||
try { rmSync(auditDir, { recursive: true, force: true }); } catch { /* noop */ }
|
||||
});
|
||||
|
||||
function writeStarted(concurrency: number): void {
|
||||
const file = join(auditDir, computeSupervisorAuditFilename());
|
||||
writeFileSync(file, JSON.stringify({
|
||||
event: 'started', ts: new Date().toISOString(), supervisor_pid: 4242,
|
||||
queue: 'default', concurrency,
|
||||
}) + '\n', 'utf8');
|
||||
}
|
||||
|
||||
describe('resolveEffectiveFanoutMax — clamp gated on live supervisor (#2194/codex #9)', () => {
|
||||
test('NO live holder → no clamp (stale audit row cannot shrink throughput)', async () => {
|
||||
writeStarted(3); // audit says concurrency 3, but no live lock holder
|
||||
const n = await resolveEffectiveFanoutMax(engine, 'default');
|
||||
expect(n).toBe(8); // unclamped base
|
||||
});
|
||||
|
||||
test('live holder + concurrency 3 → clamp to max(1, 3-1) = 2', async () => {
|
||||
writeStarted(3);
|
||||
const holder = await tryAcquireDbLock(engine, supervisorLockId('default'), SUPERVISOR_LOCK_TTL_MIN);
|
||||
expect(holder).not.toBeNull();
|
||||
try {
|
||||
const n = await resolveEffectiveFanoutMax(engine, 'default');
|
||||
expect(n).toBe(2);
|
||||
} finally {
|
||||
await holder!.release();
|
||||
}
|
||||
});
|
||||
|
||||
test('live holder but clamp disabled → unclamped base', async () => {
|
||||
await engine.setConfig('autopilot.fanout_clamp_to_concurrency', 'false');
|
||||
writeStarted(3);
|
||||
const holder = await tryAcquireDbLock(engine, supervisorLockId('default'), SUPERVISOR_LOCK_TTL_MIN);
|
||||
try {
|
||||
const n = await resolveEffectiveFanoutMax(engine, 'default');
|
||||
expect(n).toBe(8);
|
||||
} finally {
|
||||
await holder!.release();
|
||||
}
|
||||
});
|
||||
|
||||
test('live holder + concurrency 1 → floor at 1 (never below 1)', async () => {
|
||||
writeStarted(1);
|
||||
const holder = await tryAcquireDbLock(engine, supervisorLockId('default'), SUPERVISOR_LOCK_TTL_MIN);
|
||||
try {
|
||||
const n = await resolveEffectiveFanoutMax(engine, 'default');
|
||||
expect(n).toBe(1);
|
||||
} finally {
|
||||
await holder!.release();
|
||||
}
|
||||
});
|
||||
|
||||
test('live holder but no started event (concurrency unknown) → no clamp', async () => {
|
||||
// lock row exists but audit has no concurrency → fall back to base.
|
||||
const holder = await tryAcquireDbLock(engine, supervisorLockId('default'), SUPERVISOR_LOCK_TTL_MIN);
|
||||
try {
|
||||
const n = await resolveEffectiveFanoutMax(engine, 'default');
|
||||
expect(n).toBe(8);
|
||||
} finally {
|
||||
await holder!.release();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -28,8 +28,11 @@ describe('autopilot.ts ↔ dispatchPerSource wiring', () => {
|
||||
);
|
||||
});
|
||||
|
||||
test('imports resolveFanoutMax (so PGLite gets fanoutMax=1 per codex P1-3)', () => {
|
||||
expect(AUTOPILOT_SRC).toMatch(/resolveFanoutMax/);
|
||||
test('imports resolveEffectiveFanoutMax (clamps to worker concurrency; PGLite base still 1)', () => {
|
||||
// #2194 fix #1: autopilot now resolves the CLAMPED fan-out (gated on a live
|
||||
// supervisor) instead of the raw resolveFanoutMax. The clamp wraps
|
||||
// resolveFanoutMax, so PGLite's base-1 still holds (codex P1-3).
|
||||
expect(AUTOPILOT_SRC).toMatch(/resolveEffectiveFanoutMax/);
|
||||
});
|
||||
|
||||
test('calls dispatchPerSource within the shouldFullCycle branch', () => {
|
||||
|
||||
@@ -0,0 +1,146 @@
|
||||
/**
|
||||
* #2194 fix #3 / #2227 bug #3 — the cycle split.
|
||||
*
|
||||
* Per-source autopilot cycles run ONLY source-scoped (+ mixed) phases; the
|
||||
* brain-wide `global` phases run ONCE in a separate autopilot-global-maintenance
|
||||
* job. This replaces the rejected skip-and-stamp-fresh design (codex #1/#2): the
|
||||
* split makes single-flight structural (one global job, not N concurrent embeds)
|
||||
* and never marks a source "fresh" for global work it didn't do. These tests pin
|
||||
* the phase partition, the dispatch gate, the per-source phase set, and the
|
||||
* global handler stamping autopilot.last_global_at.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { resetPgliteState } from './helpers/reset-pglite.ts';
|
||||
import { registerBuiltinHandlers } from '../src/commands/jobs.ts';
|
||||
import {
|
||||
ALL_PHASES,
|
||||
GLOBAL_PHASES,
|
||||
NON_GLOBAL_PHASES,
|
||||
PHASE_SCOPE,
|
||||
LAST_GLOBAL_AT_KEY,
|
||||
} from '../src/core/cycle.ts';
|
||||
import {
|
||||
dispatchGlobalMaintenance,
|
||||
isGlobalMaintenanceStale,
|
||||
dispatchPerSource,
|
||||
} from '../src/commands/autopilot-fanout.ts';
|
||||
import type { BrainEngine } from '../src/core/engine.ts';
|
||||
|
||||
describe('cycle phase partition (#2194 fix #3)', () => {
|
||||
test('GLOBAL ∪ NON_GLOBAL == ALL_PHASES, no overlap', () => {
|
||||
const union = new Set([...GLOBAL_PHASES, ...NON_GLOBAL_PHASES]);
|
||||
expect(union.size).toBe(ALL_PHASES.length);
|
||||
for (const p of ALL_PHASES) expect(union.has(p)).toBe(true);
|
||||
// No phase in both.
|
||||
const overlap = GLOBAL_PHASES.filter((p) => NON_GLOBAL_PHASES.includes(p));
|
||||
expect(overlap).toEqual([]);
|
||||
});
|
||||
|
||||
test('every GLOBAL phase is PHASE_SCOPE==="global"; embed is global, lint is not', () => {
|
||||
for (const p of GLOBAL_PHASES) expect(PHASE_SCOPE[p]).toBe('global');
|
||||
expect(GLOBAL_PHASES).toContain('embed');
|
||||
expect(GLOBAL_PHASES).toContain('orphans');
|
||||
expect(GLOBAL_PHASES).toContain('purge');
|
||||
expect(NON_GLOBAL_PHASES).toContain('lint');
|
||||
expect(NON_GLOBAL_PHASES).toContain('sync');
|
||||
expect(NON_GLOBAL_PHASES).not.toContain('embed');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isGlobalMaintenanceStale', () => {
|
||||
const now = Date.UTC(2026, 5, 16, 12, 0, 0);
|
||||
test('null/unparseable → stale (must run)', () => {
|
||||
expect(isGlobalMaintenanceStale(null, now)).toBe(true);
|
||||
expect(isGlobalMaintenanceStale('not-a-date', now)).toBe(true);
|
||||
});
|
||||
test('older than floor → stale; within floor → fresh', () => {
|
||||
expect(isGlobalMaintenanceStale(new Date(now - 61 * 60_000).toISOString(), now, 60)).toBe(true);
|
||||
expect(isGlobalMaintenanceStale(new Date(now - 10 * 60_000).toISOString(), now, 60)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('dispatchGlobalMaintenance — single-flight gate', () => {
|
||||
function stubs(lastGlobalAt: string | null) {
|
||||
const added: Array<{ name: string; data: any; opts: any }> = [];
|
||||
const engine = {
|
||||
kind: 'postgres' as const,
|
||||
getConfig: async (k: string) => (k === LAST_GLOBAL_AT_KEY ? lastGlobalAt : null),
|
||||
} as unknown as BrainEngine;
|
||||
const queue = {
|
||||
add: async (name: string, data: unknown, opts: Record<string, unknown>) => {
|
||||
added.push({ name, data, opts }); return { id: 1 };
|
||||
},
|
||||
} as any;
|
||||
return { engine, queue, added };
|
||||
}
|
||||
|
||||
test('stale (never run) → dispatches one global job with single-flight opts', async () => {
|
||||
const { engine, queue, added } = stubs(null);
|
||||
const r = await dispatchGlobalMaintenance(engine, queue, { repoPath: '/tmp', slot: 's1', timeoutMs: 1, jsonMode: true, emit: () => {} });
|
||||
expect(r.dispatched).toBe(true);
|
||||
expect(added.length).toBe(1);
|
||||
expect(added[0].name).toBe('autopilot-global-maintenance');
|
||||
expect(added[0].opts.idempotency_key).toBe('autopilot-global:s1');
|
||||
expect(added[0].opts.maxWaiting).toBe(1); // structural single-flight
|
||||
expect(added[0].data.phases).toEqual(GLOBAL_PHASES);
|
||||
});
|
||||
|
||||
test('fresh → does NOT dispatch', async () => {
|
||||
const { engine, queue, added } = stubs(new Date().toISOString());
|
||||
const r = await dispatchGlobalMaintenance(engine, queue, { repoPath: '/tmp', slot: 's1', timeoutMs: 1, jsonMode: true, emit: () => {} });
|
||||
expect(r.dispatched).toBe(false);
|
||||
expect(added.length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('dispatchPerSource — per-source jobs carry NON_GLOBAL phases (no embed)', () => {
|
||||
test('each per-source job sets phases = NON_GLOBAL_PHASES', async () => {
|
||||
const sources = [{ id: 'repo-a', name: 'a', config: {} }, { id: 'repo-b', name: 'b', config: {} }];
|
||||
const added: any[] = [];
|
||||
const engine = {
|
||||
kind: 'postgres' as const,
|
||||
listAllSources: async () => sources,
|
||||
getConfig: async () => null,
|
||||
executeRaw: async () => [],
|
||||
} as unknown as BrainEngine;
|
||||
const queue = { add: async (name: string, data: unknown, opts: unknown) => { added.push({ name, data, opts }); return { id: added.length }; } } as any;
|
||||
await dispatchPerSource(engine, queue, { repoPath: '/tmp', slot: 's', timeoutMs: 1, fanoutMax: 4, jsonMode: true, emit: () => {}, log: () => {} });
|
||||
expect(added.length).toBe(2);
|
||||
for (const j of added) {
|
||||
expect(j.data.phases).toEqual(NON_GLOBAL_PHASES);
|
||||
expect(j.data.phases).not.toContain('embed');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('autopilot-global-maintenance handler stamps last_global_at (PGLite)', () => {
|
||||
let engine: PGLiteEngine;
|
||||
beforeAll(async () => { engine = new PGLiteEngine(); await engine.connect({}); await engine.initSchema(); }, 30000);
|
||||
afterAll(async () => { await engine.disconnect(); });
|
||||
beforeEach(async () => { await resetPgliteState(engine); });
|
||||
|
||||
async function captureHandlers() {
|
||||
const handlers = new Map<string, (job: any) => Promise<any>>();
|
||||
const fakeWorker = { register(name: string, fn: (job: any) => Promise<any>) { handlers.set(name, fn); } };
|
||||
await registerBuiltinHandlers(fakeWorker as never, engine);
|
||||
return handlers;
|
||||
}
|
||||
|
||||
test('runs global phases (no source_id) and stamps autopilot.last_global_at on success', async () => {
|
||||
expect(await engine.getConfig(LAST_GLOBAL_AT_KEY)).toBeNull();
|
||||
const handlers = await captureHandlers();
|
||||
const handler = handlers.get('autopilot-global-maintenance');
|
||||
expect(handler).toBeTruthy();
|
||||
|
||||
const result = await handler!({ data: { phases: ['orphans', 'embed'] }, signal: undefined });
|
||||
// The cycle ran the requested global phases (DB-only on an empty brain).
|
||||
expect(result.report.phases.some((p: any) => p.phase === 'orphans')).toBe(true);
|
||||
expect(['ok', 'clean', 'partial']).toContain(result.report.status);
|
||||
// Freshness stamped so the dispatch gate backs off.
|
||||
const stamped = await engine.getConfig(LAST_GLOBAL_AT_KEY);
|
||||
expect(stamped).not.toBeNull();
|
||||
expect(Number.isFinite(new Date(stamped!).getTime())).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -50,6 +50,7 @@ async function runUntilTerminal(
|
||||
h: Harness,
|
||||
overrides: Partial<{
|
||||
maxCrashes: number;
|
||||
hardStopMaxCrashes: number;
|
||||
_backoffFloorMs: number;
|
||||
cleanRestartBudget: number;
|
||||
cleanRestartWindowMs: number;
|
||||
@@ -71,6 +72,7 @@ async function runUntilTerminal(
|
||||
cliPath: h.workerScript,
|
||||
args: [],
|
||||
maxCrashes: overrides.maxCrashes ?? 3,
|
||||
hardStopMaxCrashes: overrides.hardStopMaxCrashes,
|
||||
_backoffFloorMs: overrides._backoffFloorMs ?? 5,
|
||||
cleanRestartBudget: overrides.cleanRestartBudget,
|
||||
cleanRestartWindowMs: overrides.cleanRestartWindowMs,
|
||||
@@ -159,12 +161,15 @@ if [ $((NEXT % 2)) -eq 1 ]; then exit 1; else exit 0; fi
|
||||
try {
|
||||
const res = await runUntilTerminal(h, {
|
||||
maxCrashes: 3,
|
||||
// issue #1994: the soft budget no longer gives up; pin the hard
|
||||
// ceiling to 3 so this counting test still fires give-up at 3.
|
||||
hardStopMaxCrashes: 3,
|
||||
_backoffFloorMs: 5,
|
||||
stopAfterEvents: 200,
|
||||
});
|
||||
|
||||
expect(res.maxCrashesFired).not.toBeNull();
|
||||
// 3 code!=0 exits → max_crashes=3
|
||||
// 3 code!=0 exits → hard ceiling=3
|
||||
expect(res.maxCrashesFired!.count).toBe(3);
|
||||
|
||||
const exits = res.events.filter((e) => e.kind === 'worker_exited');
|
||||
@@ -235,6 +240,7 @@ esac
|
||||
|
||||
const res = await runUntilTerminal(h, {
|
||||
maxCrashes: 3,
|
||||
hardStopMaxCrashes: 3, // issue #1994: pin give-up to 3 for this counting test
|
||||
_backoffFloorMs: 5,
|
||||
_now: fakeNow,
|
||||
stopAfterEvents: 200,
|
||||
@@ -471,6 +477,66 @@ esac
|
||||
});
|
||||
});
|
||||
|
||||
// issue #1994 (#2227 tail): crossing the SOFT crash budget no longer
|
||||
// permanently gives up. The supervisor enters degraded mode (capped backoff
|
||||
// + loud warn) so a transient outage self-heals; permanent give-up fires only
|
||||
// at the much-higher hard ceiling.
|
||||
describe('degraded-mode crash backoff (issue #1994)', () => {
|
||||
it('crossing the soft budget does NOT give up; it warns and keeps retrying to the hard ceiling', async () => {
|
||||
const h = makeHarness('degraded-softbudget', 'exit 1');
|
||||
try {
|
||||
const { events, maxCrashesFired } = await runUntilTerminal(h, {
|
||||
maxCrashes: 3, // soft budget
|
||||
hardStopMaxCrashes: 6, // hard ceiling
|
||||
_backoffFloorMs: 1,
|
||||
stopAfterEvents: 200,
|
||||
});
|
||||
// Permanent give-up fired at the HARD ceiling (6), not the soft budget (3).
|
||||
expect(maxCrashesFired).not.toBeNull();
|
||||
expect(maxCrashesFired!.count).toBe(6);
|
||||
expect(maxCrashesFired!.max).toBe(6);
|
||||
|
||||
// The soft-budget crossing announced degraded mode (at least once).
|
||||
const degraded = events.filter(
|
||||
(e): e is Extract<ChildSupervisorEvent, { kind: 'health_warn' }> =>
|
||||
e.kind === 'health_warn' && e.reason === 'crash_budget_degraded',
|
||||
);
|
||||
expect(degraded.length).toBeGreaterThanOrEqual(1);
|
||||
expect(degraded[0].max).toBe(3);
|
||||
expect(degraded[0].count).toBeGreaterThanOrEqual(3);
|
||||
|
||||
// It kept respawning past the soft budget (more than 3 crash exits).
|
||||
const crashes = events.filter(
|
||||
(e): e is Extract<ChildSupervisorEvent, { kind: 'worker_exited' }> =>
|
||||
e.kind === 'worker_exited' && e.code === 1,
|
||||
);
|
||||
expect(crashes.length).toBe(6);
|
||||
} finally {
|
||||
h.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it('hardStopMaxCrashes=0 disables permanent give-up (retry-forever-with-backoff)', async () => {
|
||||
const h = makeHarness('degraded-noforever', 'exit 1');
|
||||
try {
|
||||
const { events, maxCrashesFired } = await runUntilTerminal(h, {
|
||||
maxCrashes: 3,
|
||||
hardStopMaxCrashes: 0, // never permanently stop
|
||||
_backoffFloorMs: 1,
|
||||
stopAfterEvents: 40, // the safety net stops the test, not a give-up
|
||||
});
|
||||
// Never gave up despite many crashes past the soft budget.
|
||||
expect(maxCrashesFired).toBeNull();
|
||||
const crashes = events.filter(
|
||||
(e) => e.kind === 'worker_exited' && (e as any).code === 1,
|
||||
);
|
||||
expect(crashes.length).toBeGreaterThan(3);
|
||||
} finally {
|
||||
h.cleanup();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('issue #1801 — restartCurrentChild + killChild liveness fix', () => {
|
||||
const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms));
|
||||
// ESRCH = no such process (dead). EPERM = process exists but we can't
|
||||
|
||||
@@ -21,6 +21,8 @@ import {
|
||||
inspectLock,
|
||||
listStaleLocks,
|
||||
deleteLockRow,
|
||||
liveSyncStatus,
|
||||
syncLockId,
|
||||
} from '../src/core/db-lock.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
@@ -183,3 +185,39 @@ describe('deleteLockRow', () => {
|
||||
await handle!.release();
|
||||
});
|
||||
});
|
||||
|
||||
describe('liveSyncStatus (#1950)', () => {
|
||||
test('returns null when the source holds no lock (idle)', async () => {
|
||||
const live = await liveSyncStatus(engine, 'never-synced');
|
||||
expect(live).toBeNull();
|
||||
});
|
||||
|
||||
test('returns the holder when a live (non-expired) sync lock is held', async () => {
|
||||
const handle = await tryAcquireDbLock(engine, syncLockId('busy-source'));
|
||||
expect(handle).not.toBeNull();
|
||||
const live = await liveSyncStatus(engine, 'busy-source');
|
||||
expect(live).not.toBeNull();
|
||||
expect(live!.holder_pid).toBe(process.pid);
|
||||
expect(typeof live!.holder_host).toBe('string');
|
||||
expect(live!.holder_host.length).toBeGreaterThan(0);
|
||||
await handle!.release();
|
||||
});
|
||||
|
||||
test('returns null for a stale (TTL-expired) lock — structurally available, not running', async () => {
|
||||
await (engine as any).db.query(
|
||||
`INSERT INTO gbrain_cycle_locks (id, holder_pid, holder_host, acquired_at, ttl_expires_at)
|
||||
VALUES ($1, $2, $3, NOW() - INTERVAL '2 hours', NOW() - INTERVAL '1 hour')`,
|
||||
[syncLockId('wedged-dead'), 31337, 'old-host'],
|
||||
);
|
||||
const live = await liveSyncStatus(engine, 'wedged-dead');
|
||||
expect(live).toBeNull();
|
||||
});
|
||||
|
||||
test('is scoped per source — one source live does not mark another running', async () => {
|
||||
const handle = await tryAcquireDbLock(engine, syncLockId('source-a'));
|
||||
expect(handle).not.toBeNull();
|
||||
expect(await liveSyncStatus(engine, 'source-a')).not.toBeNull();
|
||||
expect(await liveSyncStatus(engine, 'source-b')).toBeNull();
|
||||
await handle!.release();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
/**
|
||||
* #2194 fix #5: `gbrain doctor` warns when autopilot's per-tick fan-out exceeds
|
||||
* the worker's effective concurrency. Fanning out more cycles than there are
|
||||
* worker slots guarantees waiters that race the stalled-sweeper — a silent
|
||||
* misconfig the operator never saw before this check.
|
||||
*
|
||||
* Drives computeAutopilotFanoutConcurrencyCheck directly with a fake engine so
|
||||
* the fan-out (config) and concurrency (audit) inputs are controllable without
|
||||
* spawning a supervisor. The audit read is stubbed via GBRAIN_AUDIT_DIR + a
|
||||
* hand-written started event.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import { mkdtempSync, writeFileSync, rmSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import { computeAutopilotFanoutConcurrencyCheck } from '../src/commands/doctor.ts';
|
||||
import { computeSupervisorAuditFilename } from '../src/core/minions/handlers/supervisor-audit.ts';
|
||||
|
||||
// Minimal fake engine: postgres-kind + a config map for fanout override.
|
||||
function fakeEngine(config: Record<string, string> = {}) {
|
||||
return {
|
||||
kind: 'postgres' as const,
|
||||
getConfig: async (k: string) => config[k] ?? null,
|
||||
} as any;
|
||||
}
|
||||
|
||||
let auditDir: string;
|
||||
const prevAuditDir = process.env.GBRAIN_AUDIT_DIR;
|
||||
|
||||
beforeEach(() => {
|
||||
auditDir = mkdtempSync(join(tmpdir(), 'gbrain-fanout-doctor-'));
|
||||
process.env.GBRAIN_AUDIT_DIR = auditDir;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (prevAuditDir === undefined) delete process.env.GBRAIN_AUDIT_DIR;
|
||||
else process.env.GBRAIN_AUDIT_DIR = prevAuditDir;
|
||||
try { rmSync(auditDir, { recursive: true, force: true }); } catch { /* noop */ }
|
||||
});
|
||||
|
||||
/** Write a `started` audit event with the given concurrency for queue 'default'. */
|
||||
function writeStarted(concurrency: number): void {
|
||||
const file = join(auditDir, computeSupervisorAuditFilename());
|
||||
const line = JSON.stringify({
|
||||
event: 'started',
|
||||
ts: new Date().toISOString(),
|
||||
supervisor_pid: 4242,
|
||||
queue: 'default',
|
||||
concurrency,
|
||||
});
|
||||
writeFileSync(file, line + '\n', 'utf8');
|
||||
}
|
||||
|
||||
describe('computeAutopilotFanoutConcurrencyCheck (#2194 fix #5)', () => {
|
||||
test('warns when fan-out (4) exceeds effective slots (concurrency 2 → 1)', async () => {
|
||||
writeStarted(2);
|
||||
const check = await computeAutopilotFanoutConcurrencyCheck(fakeEngine());
|
||||
expect(check.status).toBe('warn');
|
||||
expect(check.message).toContain('exceeds worker concurrency');
|
||||
expect(check.details).toMatchObject({ fanout_max: 4, concurrency: 2, effective_slots: 1 });
|
||||
});
|
||||
|
||||
test('ok when fan-out fits (override 1, concurrency 4)', async () => {
|
||||
writeStarted(4);
|
||||
const check = await computeAutopilotFanoutConcurrencyCheck(
|
||||
fakeEngine({ 'autopilot.fanout_max_per_tick': '1' }),
|
||||
);
|
||||
expect(check.status).toBe('ok');
|
||||
});
|
||||
|
||||
test('ok/skip when no supervisor has ever started (no noise on unsupervised brains)', async () => {
|
||||
// No started event written.
|
||||
const check = await computeAutopilotFanoutConcurrencyCheck(fakeEngine());
|
||||
expect(check.status).toBe('ok');
|
||||
expect(check.message).toContain('No supervisor observed');
|
||||
});
|
||||
|
||||
test('PGLite short-circuits (single-writer, fan-out is 1)', async () => {
|
||||
const check = await computeAutopilotFanoutConcurrencyCheck({ kind: 'pglite', getConfig: async () => null } as any);
|
||||
expect(check.status).toBe('ok');
|
||||
expect(check.message).toContain('PGLite');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,62 @@
|
||||
/**
|
||||
* #2194 fix #2 — engine-parity for the failure-cooldown query.
|
||||
*
|
||||
* readRecentSourceFailures runs ONE SQL through engine.executeRaw (the
|
||||
* engine-agnostic path), so PGLite and Postgres must return identical
|
||||
* groupings. This pins that: it seeds the SAME dead/failed autopilot-cycle rows
|
||||
* into both engines and asserts the per-source counts + the null-source
|
||||
* exclusion (codex #6) match. PGLite always runs; Postgres runs only when
|
||||
* DATABASE_URL is set (mirrors engine-parity.test.ts's gating).
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||||
import { PGLiteEngine } from '../../src/core/pglite-engine.ts';
|
||||
import type { BrainEngine } from '../../src/core/engine.ts';
|
||||
import { hasDatabase, setupDB, teardownDB, getEngine } from './helpers.ts';
|
||||
import { readRecentSourceFailures } from '../../src/commands/autopilot-fanout.ts';
|
||||
|
||||
const SKIP_PG = !hasDatabase();
|
||||
|
||||
async function seed(engine: BrainEngine): Promise<void> {
|
||||
const rows: Array<[string, Record<string, unknown>, number]> = [
|
||||
['dead', { source_id: 'repo-a' }, 5],
|
||||
['failed', { source_id: 'repo-a' }, 2],
|
||||
['dead', { source_id: 'repo-b' }, 10],
|
||||
['completed', { source_id: 'repo-a' }, 1], // excluded (not a failure)
|
||||
['dead', {}, 3], // excluded (null source_id, codex #6)
|
||||
];
|
||||
for (const [status, data, minAgo] of rows) {
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO minion_jobs (name, status, data, finished_at) VALUES ('autopilot-cycle', $1, $2, $3)`,
|
||||
[status, data, new Date(Date.now() - minAgo * 60_000).toISOString()],
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
function summarize(map: Map<string, { count: number }>): Record<string, number> {
|
||||
const out: Record<string, number> = {};
|
||||
for (const [k, v] of map) out[k] = v.count;
|
||||
return out;
|
||||
}
|
||||
|
||||
describe('failure-cooldown query — PGLite', () => {
|
||||
let engine: PGLiteEngine;
|
||||
beforeAll(async () => { engine = new PGLiteEngine(); await engine.connect({}); await engine.initSchema(); await seed(engine); }, 30000);
|
||||
afterAll(async () => { await engine.disconnect(); });
|
||||
|
||||
test('groups failures by source, excludes completed + null-source', async () => {
|
||||
const map = await readRecentSourceFailures(engine, { sinceMin: 120 });
|
||||
expect(summarize(map)).toEqual({ 'repo-a': 2, 'repo-b': 1 });
|
||||
});
|
||||
});
|
||||
|
||||
(SKIP_PG ? describe.skip : describe)('failure-cooldown query — Postgres parity', () => {
|
||||
let engine: BrainEngine;
|
||||
beforeAll(async () => { await setupDB(); engine = await getEngine(); await seed(engine); }, 60000);
|
||||
afterAll(async () => { await teardownDB(); });
|
||||
|
||||
test('Postgres returns the SAME groupings as PGLite', async () => {
|
||||
const map = await readRecentSourceFailures(engine, { sinceMin: 120 });
|
||||
expect(summarize(map)).toEqual({ 'repo-a': 2, 'repo-b': 1 });
|
||||
});
|
||||
});
|
||||
@@ -138,6 +138,10 @@ describeE2E('E2E: Minions resilience (OpenClaw real-world patterns)', () => {
|
||||
|
||||
const final = await queue.getJob(job.id);
|
||||
expect(final?.error_text).toMatch(/timeout exceeded/i);
|
||||
// #1737 regression: the runaway run is counted as a spent attempt by
|
||||
// handleTimeouts (it's the first killer to fire), so accounting reads
|
||||
// honestly instead of `attempts: 0/1 (started: 1)`.
|
||||
expect(final?.attempts_made).toBe(1);
|
||||
} finally {
|
||||
await a.disconnect();
|
||||
await b.disconnect();
|
||||
|
||||
@@ -82,6 +82,11 @@ describe('gbrain status E2E (PGLite)', () => {
|
||||
expect(parsed).toHaveProperty('workers');
|
||||
expect(parsed).toHaveProperty('queue');
|
||||
expect(parsed).toHaveProperty('autopilot');
|
||||
// #1984: version field present (local CLI version) for poller pinning;
|
||||
// a normal (un-budgeted) run is never partial.
|
||||
expect(typeof parsed.version).toBe('string');
|
||||
expect(parsed.version.length).toBeGreaterThan(0);
|
||||
expect(parsed.partial).toBeUndefined();
|
||||
});
|
||||
|
||||
test('dual cycle rows: last_full + last_targeted surface independently', async () => {
|
||||
|
||||
@@ -64,6 +64,8 @@ describe('get_status_snapshot handler shape', () => {
|
||||
expect(result.schema_version).toBe(1);
|
||||
expect(result).toHaveProperty('sync');
|
||||
expect(result).toHaveProperty('cycle');
|
||||
// #1984: the op now also reports the brain server's version (thin-client parity).
|
||||
expect(typeof result.version).toBe('string');
|
||||
expect(result).not.toHaveProperty('locks');
|
||||
expect(result).not.toHaveProperty('workers');
|
||||
expect(result).not.toHaveProperty('queue');
|
||||
|
||||
@@ -1300,6 +1300,12 @@ describe('MinionQueue: handleTimeouts', () => {
|
||||
const dead = await queue.getJob(job.id);
|
||||
expect(dead!.status).toBe('dead');
|
||||
expect(dead!.error_text).toBe('timeout exceeded');
|
||||
// #1737 regression: the timed-out run counts as a spent attempt, mirroring
|
||||
// the wall-clock + stall dead-letter paths. Without this the job reads
|
||||
// `attempts: 0/N (started: N)`. Asserted on both the RETURNING row and the
|
||||
// persisted row so a future refactor can't silently drop the increment.
|
||||
expect(timedOut[0].attempts_made).toBe(1);
|
||||
expect(dead!.attempts_made).toBe(1);
|
||||
});
|
||||
|
||||
test('handleTimeouts ignores stalled jobs (lock_until > now guard)', async () => {
|
||||
|
||||
@@ -14,7 +14,65 @@
|
||||
*/
|
||||
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { parseSectionFlag, runStatus } from '../src/commands/status.ts';
|
||||
import {
|
||||
parseSectionFlag,
|
||||
parseDeadlineFlag,
|
||||
withSectionDeadline,
|
||||
runStatus,
|
||||
FAST_DEADLINE_MS,
|
||||
} from '../src/commands/status.ts';
|
||||
|
||||
describe('parseDeadlineFlag (#1984)', () => {
|
||||
test('no flag → undefined (no budget)', () => {
|
||||
expect(parseDeadlineFlag([])).toBeUndefined();
|
||||
expect(parseDeadlineFlag(['--json'])).toBeUndefined();
|
||||
});
|
||||
|
||||
test('--fast applies the preset budget', () => {
|
||||
expect(parseDeadlineFlag(['--fast'])).toBe(FAST_DEADLINE_MS);
|
||||
});
|
||||
|
||||
test('--deadline-ms in both forms', () => {
|
||||
expect(parseDeadlineFlag(['--deadline-ms', '500'])).toBe(500);
|
||||
expect(parseDeadlineFlag(['--deadline-ms=750'])).toBe(750);
|
||||
});
|
||||
|
||||
test('explicit --deadline-ms wins over --fast', () => {
|
||||
expect(parseDeadlineFlag(['--fast', '--deadline-ms=100'])).toBe(100);
|
||||
});
|
||||
|
||||
test('non-positive / non-numeric → usage_error', () => {
|
||||
expect(parseDeadlineFlag(['--deadline-ms', '0'])).toBe('usage_error');
|
||||
expect(parseDeadlineFlag(['--deadline-ms', '-5'])).toBe('usage_error');
|
||||
expect(parseDeadlineFlag(['--deadline-ms', 'soon'])).toBe('usage_error');
|
||||
});
|
||||
|
||||
test('bare --deadline-ms with no value → usage_error (not a silent no-budget fallthrough)', () => {
|
||||
expect(parseDeadlineFlag(['--deadline-ms'])).toBe('usage_error');
|
||||
expect(parseDeadlineFlag(['--fast', '--deadline-ms'])).toBe('usage_error');
|
||||
});
|
||||
});
|
||||
|
||||
describe('withSectionDeadline (#1984)', () => {
|
||||
test('resolves the value when it beats the budget', async () => {
|
||||
let timedOut = false;
|
||||
const v = await withSectionDeadline(Promise.resolve(42), 1000, () => { timedOut = true; });
|
||||
expect(v).toBe(42);
|
||||
expect(timedOut).toBe(false);
|
||||
});
|
||||
|
||||
test('returns undefined + fires onTimeout when the budget elapses', async () => {
|
||||
let timedOut = false;
|
||||
const v = await withSectionDeadline(new Promise<number>(() => {}), 10, () => { timedOut = true; });
|
||||
expect(v).toBeUndefined();
|
||||
expect(timedOut).toBe(true);
|
||||
});
|
||||
|
||||
test('no budget (undefined/<=0) awaits the promise as-is', async () => {
|
||||
expect(await withSectionDeadline(Promise.resolve('x'), undefined, () => {})).toBe('x');
|
||||
expect(await withSectionDeadline(Promise.resolve('y'), 0, () => {})).toBe('y');
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseSectionFlag', () => {
|
||||
test('no --section flag → undefined (all sections)', () => {
|
||||
@@ -73,4 +131,14 @@ describe('runStatus exit codes', () => {
|
||||
expect(r.exitCode).toBe(1);
|
||||
expect(captured).toMatch(/snapshot failed|no engine connected/);
|
||||
});
|
||||
|
||||
test('#1984: invalid --deadline-ms → exit 2 (usage error)', async () => {
|
||||
let captured = '';
|
||||
const r = await runStatus(null, ['--deadline-ms', '0'], {
|
||||
stdout: () => {},
|
||||
stderr: (s: string) => { captured += s; },
|
||||
});
|
||||
expect(r.exitCode).toBe(2);
|
||||
expect(captured).toContain('--deadline-ms');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -199,3 +199,45 @@ describe('summarizeCrashes — aggregation', () => {
|
||||
expect(summary.clean_exits).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
// issue #2227 bug #1/#2: a duplicate supervisor (different $HOME / --pid-file)
|
||||
// passes the pidfile guard but loses the queue-scoped DB singleton lock
|
||||
// (#1849) and `process.exit(ExitCodes.LOCK_HELD)`s. That fence-exit happens in
|
||||
// supervisor.ts:start() BEFORE the worker is ever spawned and BEFORE the
|
||||
// `started` event is emitted (the DB-lock acquire at :528 precedes the
|
||||
// `started` emit at :555), so the loser contributes NO `worker_exited` events
|
||||
// to the audit trail. The crash ledger (summarizeCrashes) moves ONLY on
|
||||
// `worker_exited`, so a fence-collision can never burn the crash budget the
|
||||
// guard exists to protect. The field report claimed 20 "unknown" crashes were
|
||||
// fence exits; this pins that the fence path is structurally uncountable, so a
|
||||
// future refactor that (wrongly) logs a `worker_exited` on the LOCK_HELD path
|
||||
// would fail here instead of silently re-introducing the breaker-trip loop.
|
||||
describe('summarizeCrashes — LOCK_HELD fence-exit is never a crash (issue #2227)', () => {
|
||||
test('a losing duplicate supervisor (no worker_exited) contributes zero crashes', () => {
|
||||
// The loser exits at the DB-lock fence before emitting `started`, so its
|
||||
// audit contribution is empty. Even paired with the winner's healthy
|
||||
// stream, total crashes stay 0.
|
||||
const winnerHealthy: SupervisorEmission[] = [
|
||||
evt('started', { supervisor_pid: 4242, queue: 'default', concurrency: 3 }),
|
||||
evt('worker_spawned', { worker_pid: 4243 }),
|
||||
// ... no worker_exited yet (winner still running)
|
||||
];
|
||||
expect(summarizeCrashes(winnerHealthy).total).toBe(0);
|
||||
});
|
||||
|
||||
test('started + stopped with no worker_exited (clean fence path) is zero crashes', () => {
|
||||
// Defensive: even if a future change made the loser emit lifecycle events
|
||||
// (started/shutting_down/stopped) around the LOCK_HELD exit, none of those
|
||||
// are `worker_exited`, so the ledger must stay at 0. The only way this test
|
||||
// fails is if someone logs a `worker_exited` on the fence path — which is
|
||||
// exactly the regression #2227 fix #2 guards against.
|
||||
const fenceStream: SupervisorEmission[] = [
|
||||
evt('started', { supervisor_pid: 5252 }),
|
||||
evt('shutting_down', { reason: 'LOCK_HELD' }),
|
||||
evt('stopped', { exit_code: 2 }),
|
||||
];
|
||||
const summary = summarizeCrashes(fenceStream);
|
||||
expect(summary.total).toBe(0);
|
||||
expect(summary.clean_exits).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,9 +14,26 @@ import { existsSync, unlinkSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { tryAcquireDbLock } from '../src/core/db-lock.ts';
|
||||
import { MinionSupervisor, ExitCodes, supervisorLockId, classifySupervisorSingleton } from '../src/core/minions/supervisor.ts';
|
||||
import type { DbLockHandle } from '../src/core/db-lock.ts';
|
||||
import { tryAcquireDbLock, inspectLock, isLockHolderLive } from '../src/core/db-lock.ts';
|
||||
import { MinionSupervisor, ExitCodes, supervisorLockId, classifySupervisorSingleton, SUPERVISOR_LOCK_TTL_MIN } from '../src/core/minions/supervisor.ts';
|
||||
import type { DbLockHandle, LockSnapshot } from '../src/core/db-lock.ts';
|
||||
|
||||
// Build a LockSnapshot fixture for the isLockHolderLive matrix. Only ttl_expired
|
||||
// and ms_since_last_refresh are consulted; the rest are filled for shape.
|
||||
function snap(over: Partial<LockSnapshot>): LockSnapshot {
|
||||
return {
|
||||
id: 'gbrain-supervisor:default',
|
||||
holder_pid: 4242,
|
||||
holder_host: 'box',
|
||||
acquired_at: new Date(),
|
||||
ttl_expires_at: new Date(),
|
||||
age_ms: 1000,
|
||||
ttl_expired: false,
|
||||
last_refreshed_at: new Date(),
|
||||
ms_since_last_refresh: 0,
|
||||
...over,
|
||||
};
|
||||
}
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
|
||||
@@ -147,6 +164,48 @@ describe('#1849 LOCK_HELD path does not strand the pidfile', () => {
|
||||
|
||||
});
|
||||
|
||||
describe('#2227 isLockHolderLive — PID-reuse-safe supervisor liveness', () => {
|
||||
test('fresh TTL → live (the normal running case)', () => {
|
||||
expect(isLockHolderLive(snap({ ttl_expired: false }), SUPERVISOR_LOCK_TTL_MIN)).toBe(true);
|
||||
});
|
||||
|
||||
test('expired TTL but refreshed within the steal grace → live (starved-but-alive #1794)', () => {
|
||||
// ttl lapsed but the holder heartbeat is recent → it is alive, just starved.
|
||||
expect(isLockHolderLive(snap({ ttl_expired: true, ms_since_last_refresh: 5_000 }), SUPERVISOR_LOCK_TTL_MIN)).toBe(true);
|
||||
});
|
||||
|
||||
test('expired TTL and stale heartbeat → dead (a gone supervisor stops refreshing)', () => {
|
||||
expect(isLockHolderLive(snap({ ttl_expired: true, ms_since_last_refresh: 36_000_000 }), SUPERVISOR_LOCK_TTL_MIN)).toBe(false);
|
||||
});
|
||||
|
||||
test('expired TTL and no heartbeat column → dead', () => {
|
||||
expect(isLockHolderLive(snap({ ttl_expired: true, ms_since_last_refresh: null }), SUPERVISOR_LOCK_TTL_MIN)).toBe(false);
|
||||
});
|
||||
|
||||
test('liveness NEVER consults process.kill (PID reuse cannot false-positive)', () => {
|
||||
// A row whose holder_pid happens to be a live, unrelated process (PID reuse)
|
||||
// but whose lock is stale must read as NOT live — proving freshness, not the
|
||||
// PID probe, is the signal. holder_pid=1 (init, always alive) + expired/stale.
|
||||
expect(isLockHolderLive(snap({ holder_pid: 1, ttl_expired: true, ms_since_last_refresh: 36_000_000 }), SUPERVISOR_LOCK_TTL_MIN)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#2227 status detects a live supervisor via the DB lock (split-$HOME)', () => {
|
||||
test('a live queue lock with no local pidfile reads as running via inspectLock', async () => {
|
||||
// Simulate the keeper holding the queue lock under a different $HOME: there
|
||||
// is a live lock row but the local pidfile path is empty.
|
||||
const holder = await tryAcquireDbLock(engine, supervisorLockId('default'), SUPERVISOR_LOCK_TTL_MIN);
|
||||
expect(holder).not.toBeNull();
|
||||
const live = await inspectLock(engine, supervisorLockId('default'));
|
||||
expect(live).not.toBeNull();
|
||||
expect(isLockHolderLive(live!, SUPERVISOR_LOCK_TTL_MIN)).toBe(true);
|
||||
await holder!.release();
|
||||
// After release the row is gone → not running.
|
||||
const gone = await inspectLock(engine, supervisorLockId('default'));
|
||||
expect(gone).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('#1849 refresh-failure fails safe (F1A)', () => {
|
||||
test('exits LOCK_LOST after the failure threshold; tolerates a single blip', async () => {
|
||||
const sup = new MinionSupervisor(engine, { cliPath: '/bin/sh', healthInterval: 0, json: true });
|
||||
|
||||
+38
-1
@@ -4,7 +4,7 @@ import { spawn } from 'child_process';
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import { readSupervisorEvents, computeSupervisorAuditFilename } from '../src/core/minions/handlers/supervisor-audit.ts';
|
||||
import { calculateBackoffMs } from '../src/core/minions/supervisor.ts';
|
||||
import { calculateBackoffMs, resolveHardStopMaxCrashes } from '../src/core/minions/supervisor.ts';
|
||||
|
||||
const TEST_PID_FILE = '/tmp/gbrain-supervisor-test.pid';
|
||||
|
||||
@@ -58,6 +58,16 @@ function spawnSupervisor(h: IntegrationHarness, overrides: Record<string, string
|
||||
SUP_HEALTH_INTERVAL_MS: '999999', // effectively off
|
||||
...overrides,
|
||||
};
|
||||
// issue #1994: the soft crash budget now DEGRADES (retry-with-backoff) rather
|
||||
// than permanently giving up; permanent give-up fires at a much-higher hard
|
||||
// ceiling (maxCrashes × 10). These integration tests assert the give-up
|
||||
// LIFECYCLE (audit events, exit code, pidfile cleanup), so pin the hard
|
||||
// ceiling to the soft budget by default — the degraded path is unit-tested in
|
||||
// child-worker-supervisor.test.ts. Tests that want true degraded behavior
|
||||
// pass GBRAIN_SUPERVISOR_HARD_STOP_CRASHES explicitly.
|
||||
if (env.GBRAIN_SUPERVISOR_HARD_STOP_CRASHES === undefined) {
|
||||
env.GBRAIN_SUPERVISOR_HARD_STOP_CRASHES = env.SUP_MAX_CRASHES;
|
||||
}
|
||||
|
||||
const child = spawn('bun', [join(import.meta.dir, 'fixtures/supervisor-runner.ts')], {
|
||||
env,
|
||||
@@ -104,6 +114,31 @@ async function waitFor(pred: () => boolean, timeoutMs: number, tickMs = 20): Pro
|
||||
}
|
||||
|
||||
describe('MinionSupervisor', () => {
|
||||
describe('resolveHardStopMaxCrashes (issue #1994)', () => {
|
||||
const KEY = 'GBRAIN_SUPERVISOR_HARD_STOP_CRASHES';
|
||||
afterEach(() => { delete process.env[KEY]; });
|
||||
|
||||
it('defaults to maxCrashes × 10 when no override', () => {
|
||||
delete process.env[KEY];
|
||||
expect(resolveHardStopMaxCrashes(10)).toBe(100);
|
||||
expect(resolveHardStopMaxCrashes(3)).toBe(30);
|
||||
});
|
||||
|
||||
it('honors a valid non-negative integer override', () => {
|
||||
process.env[KEY] = '0'; // 0 = disable permanent give-up
|
||||
expect(resolveHardStopMaxCrashes(10)).toBe(0);
|
||||
process.env[KEY] = '5';
|
||||
expect(resolveHardStopMaxCrashes(10)).toBe(5);
|
||||
});
|
||||
|
||||
it('ignores a negative or non-integer override (falls back to default)', () => {
|
||||
process.env[KEY] = '-1';
|
||||
expect(resolveHardStopMaxCrashes(10)).toBe(100);
|
||||
process.env[KEY] = 'abc';
|
||||
expect(resolveHardStopMaxCrashes(10)).toBe(100);
|
||||
});
|
||||
});
|
||||
|
||||
describe('calculateBackoffMs', () => {
|
||||
it('returns ~1s for first crash', () => {
|
||||
const backoff = calculateBackoffMs(0);
|
||||
@@ -197,6 +232,8 @@ describe('MinionSupervisor', () => {
|
||||
// hit max-crashes, then exit via shutdown() with code 1.
|
||||
const h = makeHarness('max-crashes', 'exit 1');
|
||||
try {
|
||||
// hard ceiling defaults to SUP_MAX_CRASHES in the harness (see
|
||||
// spawnSupervisor) so this give-up lifecycle still fires at 3 (#1994).
|
||||
const sup = spawnSupervisor(h, { SUP_MAX_CRASHES: '3' });
|
||||
const { code } = await sup.exited;
|
||||
|
||||
|
||||
@@ -6,11 +6,33 @@ import { describe, test, expect } from 'bun:test';
|
||||
import {
|
||||
resolveSyncHardDeadline,
|
||||
composeAbortSignals,
|
||||
resolveStallAbortSeconds,
|
||||
DEFAULT_SYNC_STALL_ABORT_SEC,
|
||||
HARD_DEADLINE_GRACE_SEC,
|
||||
} from '../src/commands/sync.ts';
|
||||
|
||||
const GRACE_MS = HARD_DEADLINE_GRACE_SEC * 1000;
|
||||
|
||||
describe('resolveStallAbortSeconds (#1950)', () => {
|
||||
test('defaults to 900s when the env var is unset or empty', () => {
|
||||
expect(resolveStallAbortSeconds({})).toBe(DEFAULT_SYNC_STALL_ABORT_SEC);
|
||||
expect(resolveStallAbortSeconds({ GBRAIN_SYNC_STALL_ABORT_SECONDS: '' })).toBe(900);
|
||||
});
|
||||
|
||||
test('honors a positive override', () => {
|
||||
expect(resolveStallAbortSeconds({ GBRAIN_SYNC_STALL_ABORT_SECONDS: '120' })).toBe(120);
|
||||
});
|
||||
|
||||
test('<=0 disables the watchdog (returned verbatim)', () => {
|
||||
expect(resolveStallAbortSeconds({ GBRAIN_SYNC_STALL_ABORT_SECONDS: '0' })).toBe(0);
|
||||
expect(resolveStallAbortSeconds({ GBRAIN_SYNC_STALL_ABORT_SECONDS: '-1' })).toBe(-1);
|
||||
});
|
||||
|
||||
test('falls back to the default on a non-numeric value', () => {
|
||||
expect(resolveStallAbortSeconds({ GBRAIN_SYNC_STALL_ABORT_SECONDS: 'nope' })).toBe(900);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveSyncHardDeadline', () => {
|
||||
test('--no-hard-deadline disables everything (even with --timeout)', () => {
|
||||
const r = resolveSyncHardDeadline(['--source', 'x', '--timeout', '60', '--no-hard-deadline'], { isTty: false });
|
||||
|
||||
Reference in New Issue
Block a user