feat: queue resilience — wall-clock timeouts, backpressure, --no-worker, env concurrency (#379)

* feat: queue resilience — wall-clock timeouts, backpressure, --no-worker, env concurrency, shell guard

Prevents stall-induced queue blockage discovered in production (OpenClaw):

1. Wall-clock timeout sweep: dead-letters active jobs exceeding 2× timeout_ms
   (or 2 × lockDuration × max_stalled). Catches jobs stuck while holding DB
   connections where FOR UPDATE SKIP LOCKED stall detection skips them.

2. Submission backpressure (maxWaiting): caps waiting jobs per name at
   submission time. Prevents autopilot-cycle flood when the queue is blocked.

3. --no-worker flag for autopilot: skips spawning the built-in worker child.
   For environments where the worker lifecycle is managed externally (systemd,
   Docker, OpenClaw service-manager).

4. GBRAIN_WORKER_CONCURRENCY env var: fallback for --concurrency when the
   worker is spawned by autopilot (which can't pass CLI flags to the child).

5. Shell job env guard with clear logging: shell handler is always registered
   but throws UnrecoverableError with a clear message when
   GBRAIN_ALLOW_SHELL_JOBS=1 is not set, instead of silently not registering.

* feat: v0.19.1 Lane A — maxWaiting atomic guard, concurrency clamp, --max-waiting CLI

Addresses three production-hardening findings from the CEO + Eng + Codex
adversarial review of PR #379:

D2/H2: maxWaiting was TOCTOU-racy — two concurrent submitters could both
see waitingCount < max and both insert. Wrap the count+select+insert in
pg_advisory_xact_lock keyed on (name, queue). Serializes concurrent
decisions for the SAME key while leaving different keys fully parallel.
Lock auto-releases on txn commit/rollback — no cleanup path to leak.
Also fix the missing queue-scope bug: count and select now filter on
(name, queue) not name alone, so cross-queue same-name jobs don't
suppress each other.

D3/H3: resolveWorkerConcurrency silently accepted NaN / 0 / negative from
parseInt. `inFlight.size < NaN` is always false → worker claims nothing →
silent wedge from a single-typo env var. Clamp to ≥1 with a loud stderr
warning naming the bad value.

D5/H5: `gbrain jobs submit` never parsed `--max-waiting N` despite the
MinionJobInput field. Wire the flag with clamp [1, 100], mirror
`--max-stalled`. Extract `parseMaxWaitingFlag` for unit testing.

Q1: Silent coalesce was invisible by design. New
src/core/minions/backpressure-audit.ts mirrors shell-audit.ts's ISO-week
JSONL pattern: `~/.gbrain/audit/backpressure-YYYY-Www.jsonl`. Coalesce
events write one JSONL line with (queue, name, waiting_count, max_waiting,
returned_job_id, ts). Best-effort — disk-full never blocks submission.

A2: `gbrain jobs smoke --wedge-rescue` new opt-in regression case.
Forges a wedged-worker row state, invokes handleStalled + handleTimeouts
+ handleWallClockTimeouts in order, asserts only wall-clock evicts.
Mirrors the v0.14.3 `--sigkill-rescue` shape.

Tests: 23 new unit cases in test/minions.test.ts covering wall-clock
timeout (3 cases + non-interference with handleTimeouts), maxWaiting
(coalesce, clamp 0, floor, concurrent-submitter race via Promise.all,
cross-queue isolation, unset fallthrough), concurrency clamp (7 cases
incl. NaN/0/negative), parseMaxWaitingFlag (5 cases), backpressure
audit file write.

Part of v0.19.1 plan at ~/.claude/plans/ok-wintermute-wrote-this-polished-matsumoto.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat: v0.19.1 Lane B — doctor queue_health, autopilot peer probe, runbook

A5 / D4: New `queue_health` check in `gbrain doctor`. Postgres-only (PGLite
has no multi-process worker surface). Two subchecks, both cheap (single
SELECT each, status-index-covered):

- stalled-forever: any active job with started_at > 1h. Surfaces the
  worst offenders (top 5 by started_at ASC) with `gbrain jobs get/cancel`
  fix hints. The incident that motivated v0.19.1 ran 90+ min before the
  operator noticed.
- waiting-depth: per-name waiting count exceeds threshold. Default 10,
  overridable via GBRAIN_QUEUE_WAITING_THRESHOLD env (D9). Signals a
  submitter probably needs maxWaiting set.

Worker-heartbeat subcheck from the original plan dropped (D4/H4): no
minion_workers table exists, and lock_until-on-active-jobs is a lossy
proxy that can't distinguish idle-worker from dead-worker. Tracked as
follow-up B7.

A4: --no-worker peer-liveness probe in autopilot. When --no-worker is
set, every cycle runs a cheap SELECT checking for any active job whose
lock_until was refreshed in the last 2 minutes. After 3 consecutive
idle ticks, logs a loud WARNING naming the silent-wedge vector and
referencing B7 as the ground-truth follow-up. Re-arms on next live
signal so the warning doesn't spam every cycle.

A6: New docs/guides/queue-operations-runbook.md (one viewport, ~60
lines). "My queue looks wedged — what do I run?" in order of
escalation. What each doctor subcheck means. Self-check for the
--no-worker / no-worker-running footgun.

CLAUDE.md: key-files updates for handleWallClockTimeouts (v0.19.0 Layer
3 kill shot), maxWaiting advisory-lock rewrite (v0.19.1 D2), queue_health
doctor check (v0.19.1 D4), and backpressure-audit.ts.

Tests: all 143 minions + 13 doctor unit tests pass. No new test cases
required in Lane B; the doctor queue_health exercise is in the E2E
verification step (needs real PG to produce meaningful stalled-forever
rows). The --no-worker probe is exercised by the smoke case's wedge
setup in Lane A.

README: unchanged. Existing `gbrain jobs submit` examples don't show
--max-stalled, so no --max-waiting precedent to extend per A6 conditional.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: v0.19.1 Lane C — CHANGELOG entry, VERSION bump, remove SPEC.md

VERSION: 0.19.0 → 0.19.1 (patch; bug-fix-dominant, no schema change,
no new user-facing vocabulary).

CHANGELOG: new v0.19.1 entry at the top with the full release-summary
template per CLAUDE.md — bold two-line headline, lead paragraph, "numbers
that matter" before/after table measured against the real incident,
"what this means for OpenClaw users" closer, required "To take
advantage of v0.19.1" block naming the worker-restart requirement,
itemized changes by area, and "For contributors" section closing the
loop on the stale autopilot-idempotency narrative the CEO review was
based on.

Mechanism reframing per D1/H1: the 18-job pile-up was NOT caused by
missing idempotency (autopilot already passes
`idempotency_key: autopilot-cycle:${slot}` at autopilot.ts:241). The
18 jobs were 18 DIFFERENT slots stacking up behind the wedged one.
`maxWaiting` still caps the pile; the incident just wasn't about
idempotency. Adversarial review caught this before ship.

SPEC.md: deleted from repo root. It was Wintermute's planning artifact
for the original PR, not a shipped spec. Design docs belong under
docs/designs/ per repo convention; leaving one at repo root set a
precedent this repo doesn't want (A7/D11). CHANGELOG + the plan file
at ~/.claude/plans/ok-wintermute-wrote-this-polished-matsumoto.md are
the durable artifacts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: --wedge-rescue smoke state — both stall+timeout sweeps must skip

Smoke case was setting lock_until in the past, so handleStalled's
requeue path fired before handleWallClockTimeouts had a chance to
evict. Production scenario is "lock_until still live (worker
renewing) + timeout_at disqualified" — only wall-clock matches.

Single-connection smoke can't simulate a row lock held by another
txn, so we force the equivalent outcome:
- lock_until = now() + 30s → handleStalled skips (not a stall)
- timeout_at = NULL → handleTimeouts skips (needs NOT NULL)
- started_at = now() - 10s, timeout_ms=1000 → wall-clock matches
  (2 × timeout_ms = 2000ms threshold exceeded)

Verified: SMOKE PASS — Minions healthy + wedge rescue in 0.14s.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: CI failures — shell-handler tests + llms-full.txt drift

Two CI failure clusters, both pre-existing but surfaced by the v0.20.3
merge:

1) test/minions-shell.test.ts — 12 failing cases. The shell handler
   throws UnrecoverableError when GBRAIN_ALLOW_SHELL_JOBS !== '1' (the
   production RCE guard at shell.ts:210). The unit tests exercise
   handler mechanics, not the guard, but never set the env var — so
   every invocation exits through the guard path instead of the code
   being tested. Fix: set GBRAIN_ALLOW_SHELL_JOBS=1 in beforeAll,
   restore in afterAll. The env-guard IS still tested separately via
   the test/minions.test.ts case added in v0.20.3 Lane A which toggles
   the var itself.

2) llms-full.txt — stale against CLAUDE.md. Key-files entries for
   queue.ts, doctor.ts, and the new backpressure-audit.ts updated in
   v0.20.3 Lane B triggered the build-llms drift guard. Regenerated
   via `bun run build:llms`; no behavior change, just the inlined-docs
   bundle catching up to source.

Full test run: 2367 pass, 0 fail across 137 files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: root <root@localhost>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-04-24 01:09:28 -07:00
committed by GitHub
co-authored by root Claude Opus 4.7
parent e3f704229b
commit d838d4792b
16 changed files with 940 additions and 20 deletions
+12
View File
@@ -12,8 +12,18 @@ import * as os from 'node:os';
let engine: PGLiteEngine;
let queue: MinionQueue;
// The shell handler at src/core/minions/handlers/shell.ts:210 throws
// UnrecoverableError when GBRAIN_ALLOW_SHELL_JOBS !== '1'. That's the
// production-worker RCE guard. Unit tests here exercise the handler
// mechanics, not the guard, so we enable it for the whole file and
// restore on teardown. The separate "rejects when env not set" case
// (in the minion-shell submission E2E / the queue-resilience wave)
// toggles the var itself.
let prevAllowShellJobs: string | undefined;
beforeAll(async () => {
prevAllowShellJobs = process.env.GBRAIN_ALLOW_SHELL_JOBS;
process.env.GBRAIN_ALLOW_SHELL_JOBS = '1';
engine = new PGLiteEngine();
await engine.connect({ database_url: '' });
await engine.initSchema();
@@ -22,6 +32,8 @@ beforeAll(async () => {
afterAll(async () => {
await engine.disconnect();
if (prevAllowShellJobs === undefined) delete process.env.GBRAIN_ALLOW_SHELL_JOBS;
else process.env.GBRAIN_ALLOW_SHELL_JOBS = prevAllowShellJobs;
});
beforeEach(async () => {
+243
View File
@@ -1653,3 +1653,246 @@ describe('MinionQueue: Attachments', () => {
expect(list[0].filename).toBe('b.txt');
});
});
// --- v0.19.1 — queue-resilience (wall-clock sweep, maxWaiting race, concurrency clamp) ---
describe('MinionQueue: v0.19.1 handleWallClockTimeouts (Layer 3 kill shot)', () => {
test('evicts active job past 2× timeout_ms — sets dead + wall-clock error_text', async () => {
const job = await queue.add('noop', {}, { timeout_ms: 100 });
await engine.executeRaw(
`UPDATE minion_jobs
SET status='active',
lock_token='wc-test',
lock_until=now() - interval '1 second',
started_at=now() - interval '1 second',
timeout_at=now() - interval '0.9 second',
attempts_started = attempts_started + 1
WHERE id=$1`,
[job.id],
);
const killed = await queue.handleWallClockTimeouts(30_000);
expect(killed.length).toBe(1);
expect(killed[0].id).toBe(job.id);
const after = await queue.getJob(job.id);
expect(after?.status).toBe('dead');
expect(after?.error_text).toBe('wall-clock timeout exceeded');
});
test('timeout_ms NULL fallback uses 2 × lockDuration × max_stalled threshold', async () => {
const job = await queue.add('noop', {}, { max_stalled: 3 });
// Force timeout_ms / timeout_at NULL on-disk (columns might or might not be set by add).
await engine.executeRaw(
`UPDATE minion_jobs
SET status='active',
timeout_ms=NULL,
timeout_at=NULL,
lock_token='wc-null',
lock_until=now() - interval '1 second',
started_at=now() - interval '61 seconds',
attempts_started = attempts_started + 1
WHERE id=$1`,
[job.id],
);
// 2 × lockDurationMs × max_stalled = 2 × 10_000 × 3 = 60_000 ms. started_at is 61s ago.
const killed = await queue.handleWallClockTimeouts(10_000);
expect(killed.length).toBe(1);
expect(killed[0].id).toBe(job.id);
const after = await queue.getJob(job.id);
expect(after?.status).toBe('dead');
});
test('respects threshold — active job within window is NOT killed', async () => {
const job = await queue.add('noop', {}, { timeout_ms: 100_000 });
await engine.executeRaw(
`UPDATE minion_jobs
SET status='active',
lock_token='wc-inside',
lock_until=now() + interval '30 seconds',
started_at=now() - interval '10 seconds',
timeout_at=now() + interval '90 seconds',
attempts_started = attempts_started + 1
WHERE id=$1`,
[job.id],
);
const killed = await queue.handleWallClockTimeouts(30_000);
expect(killed.length).toBe(0);
const after = await queue.getJob(job.id);
expect(after?.status).toBe('active');
});
});
describe('MinionQueue: v0.19.1 maxWaiting — cap correctness + race (D2/H2)', () => {
test('coalesces 3rd submission when cap is 2 — returns existing most-recent waiting row', async () => {
const a = await queue.add('poll', {}, { maxWaiting: 2 });
const b = await queue.add('poll', {}, { maxWaiting: 2 });
const c = await queue.add('poll', {}, { maxWaiting: 2 });
expect(a.id).not.toBe(b.id);
expect(c.id).toBe(b.id); // coalesced to the most-recent waiting row
const rows = await engine.executeRaw<{ count: string }>(
`SELECT count(*)::text AS count FROM minion_jobs WHERE name='poll' AND status='waiting'`,
);
expect(parseInt(rows[0].count, 10)).toBe(2);
});
test('clamps maxWaiting: 0 → 1 (strictest cap)', async () => {
const a = await queue.add('squeeze', {}, { maxWaiting: 0 });
const b = await queue.add('squeeze', {}, { maxWaiting: 0 });
expect(b.id).toBe(a.id); // 0 clamped to 1, 2nd coalesces into 1st
});
test('floors maxWaiting: 1.7 → 1', async () => {
const a = await queue.add('floor', {}, { maxWaiting: 1.7 });
const b = await queue.add('floor', {}, { maxWaiting: 1.7 });
expect(b.id).toBe(a.id);
});
test('concurrent submitters respect the cap under Promise.all race (H2)', async () => {
// Serialized by pg_advisory_xact_lock keyed on (name, queue). Without it,
// two concurrent submits both see count<max and both insert — the TOCTOU
// bug codex caught in D2/H2.
const results = await Promise.all([
queue.add('race', {}, { maxWaiting: 2 }),
queue.add('race', {}, { maxWaiting: 2 }),
queue.add('race', {}, { maxWaiting: 2 }),
]);
expect(results.length).toBe(3);
const rows = await engine.executeRaw<{ count: string }>(
`SELECT count(*)::text AS count FROM minion_jobs WHERE name='race' AND status='waiting'`,
);
expect(parseInt(rows[0].count, 10)).toBe(2); // cap held under concurrency
});
test('cross-queue isolation — same name in queue A does NOT suppress queue B (H2 secondary)', async () => {
const a = await queue.add('isolate', {}, { maxWaiting: 1, queue: 'default' });
// cap hit on queue=default with maxWaiting=1; 2nd would coalesce into `a`
const a2 = await queue.add('isolate', {}, { maxWaiting: 1, queue: 'default' });
expect(a2.id).toBe(a.id);
// Different queue — MUST insert a fresh row, NOT coalesce into queue=default
const b = await queue.add('isolate', {}, { maxWaiting: 1, queue: 'shell' });
expect(b.id).not.toBe(a.id);
expect(b.queue).toBe('shell');
});
test('unset maxWaiting — normal submit path, no coalesce, no cap', async () => {
const a = await queue.add('uncapped', {});
const b = await queue.add('uncapped', {});
const c = await queue.add('uncapped', {});
expect(new Set([a.id, b.id, c.id]).size).toBe(3);
});
});
describe('resolveWorkerConcurrency (v0.19.1 H3): clamp + validation', () => {
// jobs.ts handler — tested via direct import. Warning goes to stderr;
// tests verify return value only, not the warning line.
let resolveWorkerConcurrency: (args: string[], env?: NodeJS.ProcessEnv) => number;
let parseMaxWaitingFlag: (args: string[]) => number | undefined;
beforeAll(async () => {
const mod = await import('../src/commands/jobs.ts');
resolveWorkerConcurrency = mod.resolveWorkerConcurrency;
parseMaxWaitingFlag = mod.parseMaxWaitingFlag;
});
test('flag=4 env-unset → 4', () => {
expect(resolveWorkerConcurrency(['--concurrency', '4'], {} as NodeJS.ProcessEnv)).toBe(4);
});
test('flag-unset env=8 → 8', () => {
expect(resolveWorkerConcurrency([], { GBRAIN_WORKER_CONCURRENCY: '8' } as NodeJS.ProcessEnv)).toBe(8);
});
test('flag=2 env=8 → 2 (flag wins)', () => {
expect(resolveWorkerConcurrency(['--concurrency', '2'], { GBRAIN_WORKER_CONCURRENCY: '8' } as NodeJS.ProcessEnv)).toBe(2);
});
test('both unset → 1', () => {
expect(resolveWorkerConcurrency([], {} as NodeJS.ProcessEnv)).toBe(1);
});
test('garbage env "foo" → clamped to 1 (H3)', () => {
expect(resolveWorkerConcurrency([], { GBRAIN_WORKER_CONCURRENCY: 'foo' } as NodeJS.ProcessEnv)).toBe(1);
});
test('env=0 → clamped to 1 (H3 — prevents silent wedge)', () => {
expect(resolveWorkerConcurrency([], { GBRAIN_WORKER_CONCURRENCY: '0' } as NodeJS.ProcessEnv)).toBe(1);
});
test('env=-5 → clamped to 1 (H3)', () => {
expect(resolveWorkerConcurrency([], { GBRAIN_WORKER_CONCURRENCY: '-5' } as NodeJS.ProcessEnv)).toBe(1);
});
});
describe('parseMaxWaitingFlag (v0.19.1 H5): CLI flag wiring', () => {
let parseMaxWaitingFlag: (args: string[]) => number | undefined;
beforeAll(async () => {
parseMaxWaitingFlag = (await import('../src/commands/jobs.ts')).parseMaxWaitingFlag;
});
test('absent → undefined (no cap, default submit path)', () => {
expect(parseMaxWaitingFlag(['foo', '--params', '{}'])).toBeUndefined();
});
test('--max-waiting 2 → 2 (happy path)', () => {
expect(parseMaxWaitingFlag(['foo', '--max-waiting', '2'])).toBe(2);
});
test('--max-waiting 200 → clamped to 100', () => {
expect(parseMaxWaitingFlag(['foo', '--max-waiting', '200'])).toBe(100);
});
test('--max-waiting 0 → throws', () => {
expect(() => parseMaxWaitingFlag(['foo', '--max-waiting', '0'])).toThrow('positive integer');
});
test('--max-waiting abc → throws', () => {
expect(() => parseMaxWaitingFlag(['foo', '--max-waiting', 'abc'])).toThrow('positive integer');
});
});
describe('backpressure-audit (v0.19.1 Q1): JSONL on coalesce', () => {
test('logBackpressureCoalesce writes one JSONL line per coalesce', async () => {
const { logBackpressureCoalesce, resolveAuditDir, computeAuditFilename } =
await import('../src/core/minions/backpressure-audit.ts');
const fs = await import('node:fs');
const path = await import('node:path');
const os = await import('node:os');
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gbrain-audit-'));
const prev = process.env.GBRAIN_AUDIT_DIR;
process.env.GBRAIN_AUDIT_DIR = tmp;
try {
expect(resolveAuditDir()).toBe(tmp);
logBackpressureCoalesce({
queue: 'default',
name: 'poll',
waiting_count: 2,
max_waiting: 2,
returned_job_id: 42,
});
const file = path.join(tmp, computeAuditFilename());
const text = fs.readFileSync(file, 'utf8');
const line = JSON.parse(text.trim());
expect(line.decision).toBe('coalesced');
expect(line.name).toBe('poll');
expect(line.returned_job_id).toBe(42);
expect(typeof line.ts).toBe('string');
} finally {
if (prev === undefined) delete process.env.GBRAIN_AUDIT_DIR;
else process.env.GBRAIN_AUDIT_DIR = prev;
fs.rmSync(tmp, { recursive: true, force: true });
}
});
});
describe('MinionQueue: v0.19.1 wall-clock + handleTimeouts non-interference (T1)', () => {
test('wall-clock sweep does NOT evict a job that handleTimeouts would handle', async () => {
// Retry-able timeout: timeout_at < now() AND lock_until > now() — handleTimeouts
// is the correct killer here. wall-clock's 2× threshold has not fired yet.
const job = await queue.add('noop', {}, { timeout_ms: 100_000 });
await engine.executeRaw(
`UPDATE minion_jobs
SET status='active',
lock_token='t1',
lock_until=now() + interval '30 seconds',
started_at=now() - interval '2 seconds',
timeout_at=now() - interval '0.5 seconds',
attempts_started = attempts_started + 1
WHERE id=$1`,
[job.id],
);
// At this point: started_at is 2s ago, 2×timeout_ms = 200s. Wall-clock should NOT fire.
const killed = await queue.handleWallClockTimeouts(30_000);
expect(killed.length).toBe(0);
const after = await queue.getJob(job.id);
expect(after?.status).toBe('active');
});
});