v0.42.22.0 fix(minions): supervisor progress watchdog + worker DB self-defense — alive-but-wedged worker self-heals (#1801) (#1824)

* fix(minions): supervisor progress watchdog + worker DB self-defense under supervision (#1801)

Alive-but-wedged worker (dead DB pool, process still up) now self-heals in
minutes instead of a silent 15h halt.

- supervisor: progress watchdog restarts a child that makes no forward progress
  on claimable work (name+queue-scoped, active_healthy/due-delayed aware,
  startup-grace + loop-budget bounded); runtime handler-name derivation.
- child-worker-supervisor: killChild gates on liveness not .killed (also fixes
  the existing shutdown SIGKILL no-op); restartCurrentChild kills the captured
  child ref; intentional restart doesn't count toward max_crashes.
- worker: DB-liveness probe runs under supervision (db_dead self-exit), stall
  detection stays supervised-off.
- doctor: standalone per-queue wedged_queue check + state->status fix in the
  remote queue_health check.
- jobs/queue: queue-scoped getStats wedge fields + jobs stats WEDGED line.

* fix(minions): wedge_restart_loop one-shot + supervised-probe comment + jobs-stats threshold (review)

Pre-landing adversarial review findings:
- wedge_restart_loop warn now fires once per exhausted window via a re-arming
  flag, not every health tick (was flooding the audit log for the full window).
- Correct the stale GBRAIN_SUPERVISED comment: the DB probe runs under
  supervision now; only stall detection is skipped.
- jobs stats WEDGED line reads GBRAIN_WEDGED_QUEUE_WARN_MINUTES so it agrees
  with the doctor wedged_queue threshold.

* chore: bump version and changelog (v0.42.22.0)

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

* docs: queue-ops runbook + KEY_FILES for the #1801 wedge watchdog (v0.42.22.0)

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

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-06-03 14:14:13 -07:00
committed by GitHub
co-authored by Claude Opus 4.8
parent f3ade6c0c3
commit f4959348c2
18 changed files with 1407 additions and 59 deletions
+3 -3
View File
@@ -210,9 +210,9 @@ per-release `**vX.Y.Z:**` narration — CI enforces this
- `src/core/zombie-reap.ts` — idempotent `installSigchldHandler()` so JS-spawned children get reaped via Bun's internal `waitpid()`. Bun (like Node) only auto-reaps when a SIGCHLD listener is registered; without it, every child the worker spawns (shell jobs, embed batches, sub-agents) becomes a zombie on exit and holds connection slots. Called once at module load from `src/cli.ts` (Windows platform guard — SIGCHLD doesn't exist on Windows). Cross-file leak guard via `_uninstallSigchldHandlerForTests()`. Layer 1 of the three-layer zombie defense; Layer 2 is tini-as-PID-1 wrapping the worker subtree (via `src/core/minions/spawn-helpers.ts`); Layer 3 is the container's own tini for hard Bun crashes.
- `src/core/minions/` — Minions job queue: BullMQ-inspired, Postgres-native (queue, worker, backoff, types, protected-names, quiet-hours, stagger, handlers/shell).
- `src/core/minions/queue.ts` — MinionQueue class (submit, claim, complete, fail, stall detection, parent-child, depth/child-cap, per-job timeouts, cascade-kill, attachments, idempotency keys, child_done inbox, removeOnComplete/Fail). `add()` takes a 4th `trusted` arg (separate from `opts` to prevent spread leakage); protected names in `PROTECTED_JOB_NAMES` require `{allowProtectedSubmit: true}` and the check runs trim-normalized (whitespace-bypass safe). `add()` plumbs `max_stalled` through with a `[1, 100]` clamp; omitted values let the schema DEFAULT (5) kick in. `handleWallClockTimeouts(lockDurationMs)` is Layer 3 kill shot for jobs where `FOR UPDATE SKIP LOCKED` stall detection and the timeout sweep both fail to evict (wedged worker holding a row lock via a pending transaction). The `maxWaiting` coalesce path uses `pg_advisory_xact_lock` keyed on `(name, queue)` to serialize concurrent submits for the same key, and filters on `queue` in addition to `name` so cross-queue same-name jobs don't suppress each other.
- `src/core/minions/worker.ts` — MinionWorker class (handler registry, lock renewal, graceful shutdown, timeout safety net). Aborted jobs call `failJob` with reason (`timeout`/`cancel`/`lock-lost`/`shutdown`); `shutdownAbort` (instance field) fires on SIGTERM/SIGINT and propagates to `ctx.shutdownSignal` (shell handler listens; non-shell handlers don't). Per-job timeout fires `abort.abort(new Error('timeout'))` then a 30s grace-then-evict safety net force-evicts the job from `inFlight` and marks it dead if the handler ignores the abort signal (the moved-out generic abort listener now fires for ANY abort reason). The `launchJob` lock-renewal block is a thin sync wrapper around the pure `runLockRenewalTick` from `src/core/minions/lock-renewal-tick.ts` (NEVER `setInterval(async () => await renewLock(...))` — that shape was the unhandledRejection crash class during PgBouncer rotation). Gaps closed: (1) `cancelled` flag captured in the timer closure stops in-flight IIFEs writing misleading audit events after the job ended; (2) re-entrancy guard `tickInFlight` + per-call `Promise.race` timeout (`GBRAIN_LOCK_RENEWAL_CALL_TIMEOUT_MS`, default `lockDuration/3`); (3) time-based abort (`Date.now() - lastSuccessfulRenewalAt >= lockDuration - GBRAIN_LOCK_RENEWAL_SAFETY_MARGIN_MS`) so we release the lock BEFORE another worker can reclaim; (4) explicit `.catch()` on the stored `executeJob(...).finally(...)` promise closes the second unhandledRejection vector; (5) exported `INFRASTRUCTURE_ABORT_REASONS = new Set(['lock-renewal-failed', 'lock-lost'])` so executeJob's catch skips `failJob` for these (PgBouncer blips don't dead-letter healthy jobs; the stall detector reclaims). CI guard `scripts/check-worker-lock-renewal-shape.sh` (in `bun run verify`) asserts the bug pattern stays absent AND `launchJob` keeps calling `runLockRenewalTick`. Engine-ownership invariant: `start()` does NOT call `engine.disconnect()` on shutdown — the CLI handler in `src/commands/jobs.ts case 'work'` owns engine lifecycle via try/finally with loud error logging. RSS watchdog uses non-file-backed pages on Linux: exported `parseRssFromProcStatus(status)` (pure parser; field-presence regex so `RssAnon: 0 + RssShmem: 512` parses correctly) and `getAccurateRss(readStatus?)` (reads `/proc/self/status` for `RssAnon + RssShmem`, falls back to `process.memoryUsage().rss` on macOS / restricted containers / kernel <4.5); the default `getRss` in `WorkerOpts` is `getAccurateRss`. `checkMemoryLimit` tracks peak RSS, fires an 80%-of-cap soft-warn (once per crossing, carrying peak + in-flight job kinds), and on exceed sets `_rssWatchdogTriggered=true` (exposed via `get rssWatchdogTriggered()`) so `jobs work`'s finally can `process.exit(WORKER_EXIT_RSS_WATCHDOG)` after disconnect (drain self-identifying instead of an opaque code-0 exit). The poll loop wraps `claim` in try/catch: on a retryable conn error it reconnects ONCE and continues to the next tick rather than blind-retrying (a retry after `UPDATE...RETURNING` committed but the socket died would double-claim). `LockRenewalDeps` is wired with `reconnect` when the engine supports it. Pinned by `test/worker-lock-renewal.test.ts` (18), `test/audit/lock-renewal-audit.test.ts` (11), `test/scripts/check-worker-lock-renewal-shape.test.ts` (5), `test/worker-shutdown-disconnect.test.ts` (asserts `disconnectSpy).not.toHaveBeenCalled()`), `test/worker-rss.test.ts` (11).
- `src/core/minions/supervisor.ts` — MinionSupervisor process manager. Spawns `gbrain jobs work` as a child, restarts on crash with exponential backoff, periodic health check. `consecutiveHealthFailures` counter; on 3 consecutive failures emits `health_warn` with `reason: 'db_connection_degraded'` and calls `engine.reconnect()` to swap in a fresh pool, then resets. Worker exit classifier emits `likely_cause` on `worker_exited` events: `oom_or_external_kill` (SIGKILL), `graceful_shutdown` (SIGTERM), `runtime_error` (code 1), `clean_exit` (code 0), `unknown`. Consumes `detectTini()` + `buildSpawnInvocation()` from `src/core/minions/spawn-helpers.ts` to wrap the worker subtree in tini-as-PID-1 when tini is on `PATH` (handles native-addon zombie reaping the in-process SIGCHLD reaper can't reach); exposes `isTiniDetected` read-only accessor. The spawn-and-respawn loop is the shared `ChildWorkerSupervisor` core: MinionSupervisor composes it via `runSuperviseLoop()``new ChildWorkerSupervisor({...})` and maps `ChildSupervisorEvent` back through `emit()` SupervisorEvent (JSONL audit consumers see byte-compatible output). PID lock, signal handlers, health check, and `process.exit` on max-crashes stay in MinionSupervisor. `code=0` leaves `crashCount` untouched (so a worker alternating real crashes + watchdog drains still trips `max_crashes`); `cleanRestartBudget` (default 10 restarts per 60s) caps the macOS/non-Linux-fallback tight-loop via `health_warn { reason: 'clean_restart_budget_exceeded' }` + backoff. `shutdown()` drains via `childSupervisor.killChild('SIGTERM')` + `awaitChildExit(35_000)`. Pinned by `test/supervisor.test.ts` (16 cases) and `test/supervisor-tini.test.ts`.
- `src/core/minions/child-worker-supervisor.ts` — shared spawn-and-respawn core reused by both `MinionSupervisor` (standalone `gbrain jobs supervisor` daemon) and `src/commands/autopilot.ts` (autopilot daemon), so the two consumers can't drift into parallel-loop bugs. Pure class: NO PID file, NO signal handlers, NO `process.exit`, NO health check. Lifecycle events fire via injected `onEvent: (ChildSupervisorEvent) => void`. Exit classifier: `code === 0` leaves `crashCount` UNCHANGED (preserves flap detection across mixed exit sequences); `code != 0` follows `runDuration > stableRunResetMs ? 1 : ++crashCount`. Clean-restart budget: sliding window of code=0 exits; when count exceeds `cleanRestartBudget` (default 10) inside `cleanRestartWindowMs` (default 60s), emits `health_warn { reason: 'clean_restart_budget_exceeded' }` and applies `cleanRestartBudgetBackoffMs` (default 1s). The exit classifier special-cases `WORKER_EXIT_RSS_WATCHDOG``likely_cause='rss_watchdog'`, and that exit does NOT bump `crashCount` (routes to its own breaker); a dedicated `_watchdogExitTimestamps` sliding window trips a loud `rss_watchdog_loop` health_warn naming the cap when N watchdog exits land inside the window, INDEPENDENT of the stable-run reset (which would otherwise hide a >5-min-run watchdog drain loop). New opts `watchdogLoopBudget` (3), `watchdogLoopWindowMs` (600000), `watchdogBackoffMs` (30000); `ChildSupervisorEvent` extended. Public read-only accessors `childAlive`, `inBackoff`, `crashCount`; `killChild(signal)` + `awaitChildExit(timeoutMs)` for shutdown (the latter short-circuits when `child.exitCode !== null || child.signalCode !== null` so fast-SIGTERM responders don't cause a 35s shutdown hang). Test hooks `_backoffFloorMs`, `_now`. `supervisor-audit.ts` adds `rss_watchdog` as a non-clean cause + its own `CrashSummary.by_cause` bucket (denylist preserved so future causes route to `legacy`). Pinned by `test/child-worker-supervisor.test.ts` (7 cases).
- `src/core/minions/worker.ts` — MinionWorker class (handler registry, lock renewal, graceful shutdown, timeout safety net). Aborted jobs call `failJob` with reason (`timeout`/`cancel`/`lock-lost`/`shutdown`); `shutdownAbort` (instance field) fires on SIGTERM/SIGINT and propagates to `ctx.shutdownSignal` (shell handler listens; non-shell handlers don't). Per-job timeout fires `abort.abort(new Error('timeout'))` then a 30s grace-then-evict safety net force-evicts the job from `inFlight` and marks it dead if the handler ignores the abort signal (the moved-out generic abort listener now fires for ANY abort reason). The `launchJob` lock-renewal block is a thin sync wrapper around the pure `runLockRenewalTick` from `src/core/minions/lock-renewal-tick.ts` (NEVER `setInterval(async () => await renewLock(...))` — that shape was the unhandledRejection crash class during PgBouncer rotation). Gaps closed: (1) `cancelled` flag captured in the timer closure stops in-flight IIFEs writing misleading audit events after the job ended; (2) re-entrancy guard `tickInFlight` + per-call `Promise.race` timeout (`GBRAIN_LOCK_RENEWAL_CALL_TIMEOUT_MS`, default `lockDuration/3`); (3) time-based abort (`Date.now() - lastSuccessfulRenewalAt >= lockDuration - GBRAIN_LOCK_RENEWAL_SAFETY_MARGIN_MS`) so we release the lock BEFORE another worker can reclaim; (4) explicit `.catch()` on the stored `executeJob(...).finally(...)` promise closes the second unhandledRejection vector; (5) exported `INFRASTRUCTURE_ABORT_REASONS = new Set(['lock-renewal-failed', 'lock-lost'])` so executeJob's catch skips `failJob` for these (PgBouncer blips don't dead-letter healthy jobs; the stall detector reclaims). CI guard `scripts/check-worker-lock-renewal-shape.sh` (in `bun run verify`) asserts the bug pattern stays absent AND `launchJob` keeps calling `runLockRenewalTick`. Engine-ownership invariant: `start()` does NOT call `engine.disconnect()` on shutdown — the CLI handler in `src/commands/jobs.ts case 'work'` owns engine lifecycle via try/finally with loud error logging. RSS watchdog uses non-file-backed pages on Linux: exported `parseRssFromProcStatus(status)` (pure parser; field-presence regex so `RssAnon: 0 + RssShmem: 512` parses correctly) and `getAccurateRss(readStatus?)` (reads `/proc/self/status` for `RssAnon + RssShmem`, falls back to `process.memoryUsage().rss` on macOS / restricted containers / kernel <4.5); the default `getRss` in `WorkerOpts` is `getAccurateRss`. `checkMemoryLimit` tracks peak RSS, fires an 80%-of-cap soft-warn (once per crossing, carrying peak + in-flight job kinds), and on exceed sets `_rssWatchdogTriggered=true` (exposed via `get rssWatchdogTriggered()`) so `jobs work`'s finally can `process.exit(WORKER_EXIT_RSS_WATCHDOG)` after disconnect (drain self-identifying instead of an opaque code-0 exit). The poll loop wraps `claim` in try/catch: on a retryable conn error it reconnects ONCE and continues to the next tick rather than blind-retrying (a retry after `UPDATE...RETURNING` committed but the socket died would double-claim). `LockRenewalDeps` is wired with `reconnect` when the engine supports it. The self-health DB-liveness probe now runs EVEN under a supervisor (`GBRAIN_SUPERVISED=1`): the outer guard is `if (this.opts.healthCheckInterval > 0)` and only the STALL-detection block is wrapped in `if (!isSupervisedChild)` — so a supervised worker whose own pool dies self-exits `unhealthy(db_dead)` after `dbFailExitAfter` probes (the supervisor watches a different connection and can't see this worker's dead pool), while the supervisor's progress watchdog owns forward-progress (#1801). Pinned by `test/worker-lock-renewal.test.ts` (18), `test/audit/lock-renewal-audit.test.ts` (11), `test/scripts/check-worker-lock-renewal-shape.test.ts` (5), `test/worker-shutdown-disconnect.test.ts` (asserts `disconnectSpy).not.toHaveBeenCalled()`), `test/worker-rss.test.ts` (11), `test/worker-supervised-db-probe.test.ts` (3).
- `src/core/minions/supervisor.ts` — MinionSupervisor process manager. Spawns `gbrain jobs work` as a child, restarts on crash with exponential backoff, periodic health check. `consecutiveHealthFailures` counter; on 3 consecutive failures emits `health_warn` with `reason: 'db_connection_degraded'` and calls `engine.reconnect()` to swap in a fresh pool, then resets. Worker exit classifier emits `likely_cause` on `worker_exited` events: `oom_or_external_kill` (SIGKILL), `graceful_shutdown` (SIGTERM), `runtime_error` (code 1), `clean_exit` (code 0), `unknown`. Consumes `detectTini()` + `buildSpawnInvocation()` from `src/core/minions/spawn-helpers.ts` to wrap the worker subtree in tini-as-PID-1 when tini is on `PATH` (handles native-addon zombie reaping the in-process SIGCHLD reaper can't reach); exposes `isTiniDetected` read-only accessor. The spawn-and-respawn loop is the shared `ChildWorkerSupervisor` core: MinionSupervisor composes it via `runSuperviseLoop()``new ChildWorkerSupervisor({...})` and maps `ChildSupervisorEvent` back through `emit()` SupervisorEvent (JSONL audit consumers see byte-compatible output). PID lock, signal handlers, health check, and `process.exit` on max-crashes stay in MinionSupervisor. `code=0` leaves `crashCount` untouched (so a worker alternating real crashes + watchdog drains still trips `max_crashes`); `cleanRestartBudget` (default 10 restarts per 60s) caps the macOS/non-Linux-fallback tight-loop via `health_warn { reason: 'clean_restart_budget_exceeded' }` + backoff. `shutdown()` drains via `childSupervisor.killChild('SIGTERM')` + `awaitChildExit(35_000)`. Progress watchdog (#1801): `healthCheck()` restarts an alive-but-wedged child via `childSupervisor.restartCurrentChild(35_000)` when a queue has claimable work, 0 live-lock active jobs, and stale completions across `wedgeRestartChecks` (default 3) consecutive checks past `wedgeRestartMinutes` (default 15, 0 disables) + a `startupGraceMs` window; bounded by `wedgeRestartLoopBudget` (default 3 / `wedgeRestartLoopWindowMs`) which switches to a one-shot `wedge_restart_loop` alert. The wedge query is the exported `queryWedgeSignals(engine, queue, handlerNames)` — name+queue-scoped, `active_healthy` = live-lock only (an expired-lock active row does NOT mask the wedge), due-delayed counted. Claimable names are derived at start via a throwaway `registerBuiltinHandlers` worker (its new `quiet` opt). Flags `--wedge-restart-minutes` / `--wedge-restart-checks` + env `GBRAIN_WEDGE_RESTART_MINUTES` / `GBRAIN_WEDGE_RESTART_CHECKS`. Pinned by `test/supervisor.test.ts` (16 cases), `test/supervisor-tini.test.ts`, and `test/supervisor-wedge.test.ts`.
- `src/core/minions/child-worker-supervisor.ts` — shared spawn-and-respawn core reused by both `MinionSupervisor` (standalone `gbrain jobs supervisor` daemon) and `src/commands/autopilot.ts` (autopilot daemon), so the two consumers can't drift into parallel-loop bugs. Pure class: NO PID file, NO signal handlers, NO `process.exit`, NO health check. Lifecycle events fire via injected `onEvent: (ChildSupervisorEvent) => void`. Exit classifier: `code === 0` leaves `crashCount` UNCHANGED (preserves flap detection across mixed exit sequences); `code != 0` follows `runDuration > stableRunResetMs ? 1 : ++crashCount`. Clean-restart budget: sliding window of code=0 exits; when count exceeds `cleanRestartBudget` (default 10) inside `cleanRestartWindowMs` (default 60s), emits `health_warn { reason: 'clean_restart_budget_exceeded' }` and applies `cleanRestartBudgetBackoffMs` (default 1s). The exit classifier special-cases `WORKER_EXIT_RSS_WATCHDOG``likely_cause='rss_watchdog'`, and that exit does NOT bump `crashCount` (routes to its own breaker); a dedicated `_watchdogExitTimestamps` sliding window trips a loud `rss_watchdog_loop` health_warn naming the cap when N watchdog exits land inside the window, INDEPENDENT of the stable-run reset (which would otherwise hide a >5-min-run watchdog drain loop). New opts `watchdogLoopBudget` (3), `watchdogLoopWindowMs` (600000), `watchdogBackoffMs` (30000); `ChildSupervisorEvent` extended. Public read-only accessors `childAlive`, `inBackoff`, `crashCount`; `killChild(signal)` gates on liveness (`exitCode === null && signalCode === null`), NOT `.killed``.killed` flips true once a signal is *sent*, so the old `!this._child.killed` guard made a follow-up SIGKILL after an ignored SIGTERM a silent no-op (#1801; the bug also lived in the `shutdown()` drain). `restartCurrentChild(graceMs)` (#1801 wedge self-heal) captures the CURRENT child ref, SIGTERM→grace→SIGKILLs THAT ref (never the respawn — closes the timer-kills-fresh-worker race), and flags `_intentionalRestart` so the exit is `likelyCause='wedge_restart'`, leaves `crashCount` UNTOUCHED (never trips `max_crashes`; like `rss_watchdog`), and respawns immediately (`backoff ms:0 reason='wedge_restart'`). `awaitChildExit(timeoutMs)` short-circuits when `child.exitCode !== null || child.signalCode !== null` so fast-SIGTERM responders don't cause a 35s shutdown hang. Test hooks `_backoffFloorMs`, `_now`. `supervisor-audit.ts` adds `rss_watchdog` as a non-clean cause + its own `CrashSummary.by_cause` bucket, and `wedge_restart` to `CLEAN_EXIT_CAUSES` (a self-heal, not a crash; denylist preserved so future causes route to `legacy`). Pinned by `test/child-worker-supervisor.test.ts` (12 cases).
- `src/core/minions/spawn-helpers.ts` — pure `detectTini()` + `buildSpawnInvocation()` consumed by both `supervisor.ts` and `autopilot.ts` (resolves the DRY violation between the two spawn sites and makes tini wrapping testable without `mock.module()`, rule R2 of `scripts/check-test-isolation.sh`). `detectTini()` calls `execFileSync('which', ['tini'])` with explicit `env: process.env` so Bun sees runtime PATH mutations. `buildSpawnInvocation(tiniPath, cmd, args)` returns `{cmd, args}` with tini prepended when present, or the bare invocation otherwise. Pinned by `test/spawn-helpers.test.ts` (5) and `test/supervisor-tini.test.ts` (4).
- `src/core/minions/types.ts``MinionJobInput` + `MinionJobStatus` + handler context types. `MinionJobInput.max_stalled` is optional; omitted values let the schema DEFAULT (5) kick in, provided values are clamped to `[1, 100]`.
- `src/core/minions/protected-names.ts` — side-effect-free constant module exporting `PROTECTED_JOB_NAMES` + `isProtectedJobName()`. Kept pure so queue core can import without loading handler modules.