diff --git a/CHANGELOG.md b/CHANGELOG.md index b564e3bfd..7c83baa88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,102 @@ All notable changes to GBrain will be documented in this file. +## [0.42.16.0] - 2026-06-02 + +**`gbrain doctor` now tells you the brain is OOM-looping in one line, ranks every +problem by root cause, auto-drains stuck atom backlogs, and flags a thrashing DB +pool — so you never have to grep a worker log to learn why the brain is +unhealthy.** + +When a worker dies in a loop, the loud errors used to all point at the database +(connection dropped, lock-renewal-failed) while the real cause — the worker +running out of memory and getting drained by its own watchdog — scrolled by once +and got buried. Finding it took hours. The fixes that stop the loop shipped in +v0.42.5.0; this release makes the cause the first thing you see, and self-heals +the cases that don't need a human. + +Run `gbrain doctor`. If a worker is OOM-looping you now get one line: +`[FAIL] worker_oom_loop → Worker OOM-looping: cap=8192MB, 12 watchdog kills/24h → +raise --max-rss`. The top of the report is a new "Top issues (ranked by cause)" +block that puts root causes above downstream noise — the connection and queue +errors that are really just symptoms get tagged "(likely downstream of +worker_oom_loop)" instead of competing for your attention. + +### How to use it + +``` +gbrain doctor # human report, cause-ranked at the top +gbrain doctor --json | jq .top_issues # ranked issues for your agent to act on +gbrain config set autopilot.auto_drain.enabled false # opt out of auto-drain +``` + +### What's new + +- **`worker_oom_loop` doctor check** — the single authoritative OOM-loop signal. + It unions both worker modes: supervised workers (from the supervisor audit) and + bare `gbrain jobs work` workers (from the job table's watchdog-abort rows), so + it can't miss either. Names the memory cap (or the auto-sized default when the + breaker didn't stamp one) and the fix. Stays silent on brains that never OOM'd. +- **Cause-ranked doctor output** + a `top_issues` array in `gbrain doctor --json` + so an agent acts on the root cause without re-deriving the ranking. A downstream + link is asserted ONLY for known, real cause→effect edges (e.g. queue aborts are + caused by the OOM kill) — never guessed from two checks happening to fail at the + same time. +- **`pool_reap_health` check** — warns when a transaction-mode pooler is thrashing + (many socket reaps per hour) and fails when reconnects are actually failing + ("not auto-recovering"). The recovered-vs-stuck split no other signal expressed. +- **Autopilot auto-drains a stuck `extract_atoms` backlog** on a cadence when your + schema pack doesn't declare the phase — the silent backlog that used to grow for + weeks with zero signal. Default on, bounded by a per-day spend cap, submitted as + a protected job so no remote/MCP caller can trigger the Haiku spend. + +### Things to know after upgrade + +- The new checks are quiet on a healthy brain — they only surface during a real + incident. +- `autopilot.auto_drain` defaults on with a $2/day cap (~6 drains/day). It fires + only when the backlog exceeds 25 pages AND your pack doesn't declare + `extract_atoms` (i.e. the routine cycle isn't already handling it). Tune via + `autopilot.auto_drain.{enabled,threshold,window_seconds,max_usd_per_day}`. + +Evidence + the mechanical foundation this builds on: #1678 / #1735. + +### Itemized changes + +- `src/commands/doctor.ts` — new `computeWorkerOomLoopCheck` (unions supervisor + `rss_watchdog` crashes + `minion_jobs` watchdog-aborts; cap from the breaker + alert or `resolveDefaultMaxRssMb()` fallback) and `computePoolReapHealthCheck`; + cause-ranked "Top issues" header in `outputResults`; `top_issues` on + `DoctorReport` (additive, schema_version stays 2); the `supervisor` causeStr now + shows `rss=N (see worker_oom_loop)`; the `queue_health` watchdog message + cross-references `worker_oom_loop`. +- `src/core/doctor-cause-rank.ts` (NEW) — pure `rankIssues` + root/symptom tiers + + evidence-gated `downstream_of` (real edges only) + a drift guard exported for + the test. +- `src/core/audit/pool-recovery-audit.ts` (NEW) — reap/reconnect audit on the + shared audit-writer; error summaries redacted via `redactConnectionInfo`. +- `src/core/postgres-engine.ts` — `reconnect()` accepts the triggering error and + records `reap_detected` (CONNECTION_ENDED) vs `reconnect_other`, then + `reconnect_succeeded`/`reconnect_failed`, so only true pooler reaps are labeled. +- `src/core/retry.ts` + `src/core/retry-matcher.ts` — the retry reconnect callback + threads the error; new `isConnectionEndedError` classifier. +- `src/core/cycle/extract-atoms-drain.ts` — new `runExtractAtomsDrainForSource` + shared helper (one drain path for the CLI `--drain`, the Minion handler, and + autopilot); `src/commands/dream.ts` refactored to call it. +- `src/commands/jobs.ts` — `extract-atoms-drain` Minion handler; + `src/core/minions/protected-names.ts` adds it to `PROTECTED_JOB_NAMES`. +- `src/commands/autopilot.ts` — per-source auto-drain submission with a UTC-day + time-sloted idempotency key + daily cap; `src/core/config.ts` adds + `autopilot.auto_drain.*` config + the `autopilot.` key prefix. +- `src/core/minions/handlers/supervisor-audit.ts` — `readRecentSupervisorEvents` + reads current + previous ISO week so a 24h window can't lose a week-boundary + loop. +- `src/core/doctor-categories.ts` — registers `worker_oom_loop` + `pool_reap_health` + under ops. +- Tests: `test/doctor-cause-rank.test.ts`, `test/doctor-worker-oom-loop.test.ts`, + `test/doctor-pool-reap-health.test.ts`, `test/audit/pool-recovery-audit.test.ts`, + `test/extract-atoms-drain-handler.test.ts`, `test/autopilot-auto-drain-wiring.test.ts`, + plus extensions to `test/extract-atoms-drain.test.ts`. ## [0.42.15.0] - 2026-06-02 **Commands print real data when you run them from a subagent, a pipe, or cron, not just when you have a terminal.** A handful of gbrain commands quietly changed what they printed based on whether a terminal was attached. Run them from a coding agent, a `| cat` pipe, or a cron job and you'd get JSON when you wanted human text, or nothing useful at all. The read commands (`get`, `list`, `search`, `query`) were already fine; this fixes the ones that weren't. diff --git a/TODOS.md b/TODOS.md index 96976adb3..2aafcd0b0 100644 --- a/TODOS.md +++ b/TODOS.md @@ -1,5 +1,30 @@ # TODOS +## v0.42.12.0 #1685 brain-health-as-solved follow-ups (v0.42+) + +Deferred from the v0.42.12.0 wave (issue #1685, the posture umbrella over #1678/#1735). +The shipped checks (`worker_oom_loop`, `pool_reap_health`, cause-ranked `top_issues`, +per-source auto-drain) cover the diagnosis + self-heal demands; this is the one +explicitly-deferred demand. + +- [ ] **P3 — GAP E: secondary-error cause-ref tagging.** #1685 demand 3 asks that + downstream cascade errors (CONNECTION_ENDED, lock-renewal-failed, No database + connection) be tagged `secondary=true cause_ref=` so they can't + masquerade as the root cause in logs. v0.42.12.0 deferred this: the now-self- + identifying RSS watchdog exit (from #1735) plus the cause-ranked `doctor` header + (this wave, GAP C) already remove most of the symptom-masquerades-as-cause problem + at the doctor surface. The remaining gap is the raw worker LOG stream during a live + incident (not the doctor summary). Doing it right needs an incident-id correlator + threaded through the supervisor + DB-error paths — a bigger change than the doctor- + surface fixes this wave shipped. Pick up if live-log triage during an incident is + still painful after operators have the cause-ranked doctor. +- [ ] **P3 — `worker_oom_loop` remote/thin-client path.** The bare-worker half of the + OOM signal reads `minion_jobs` directly (Postgres-only, local). The HTTP MCP + thin-client doctor path (`doctorReportRemote`) doesn't surface it. Same brain-wide- + vs-source-scoping caveat noted inline at autopilot.ts (the `--source` remote scoping + is a separate TODO, mirroring orphan_ratio). Wire once the thin-client doctor grows + a supervisor/queue surface. + ## v0.42.15.0 isTTY-output follow-ups (v0.42+) Filed from the v0.42.15.0 wave (#1784, decouple primary output from diff --git a/VERSION b/VERSION index 310a587d0..ee551bb71 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.42.15.0 \ No newline at end of file +0.42.16.0 \ No newline at end of file diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index 7f267fd78..73e9bcffd 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -150,8 +150,12 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/core/minions/lock-renewal-tick.ts` — Pure extracted function from `MinionWorker.launchJob`'s setInterval body; the structural fix that closes the production unhandledRejection crash class. Exports `runLockRenewalTick(deps, state) → Promise` + `resolveLockRenewalKnobs(env, lockDuration) → LockRenewalKnobs`. Three env knobs, all parsed as positive integers with stderr-warn-once-per-process on bad input + default fallback: `GBRAIN_LOCK_RENEWAL_MAX_FAILURES` (default 3, audit-labeling only — abort triggering is time-based), `GBRAIN_LOCK_RENEWAL_CALL_TIMEOUT_MS` (default `lockDuration/3`, bounds the hung-renewLock vector via `Promise.race`), `GBRAIN_LOCK_RENEWAL_SAFETY_MARGIN_MS` (default `lockDuration/6`, ensures release before stall-detector reclaim). The tick checks `state.cancelled()` at three guard points (entry, after await resolves, after await throws) so a long renewLock that resolves after the job ended bails cleanly. Result type is a tagged union `{kind: 'ok' | 'cancelled' | 'lock_lost' | 'should_abort', reason?}`; all `abort.abort()` / `clearInterval` side effects live in the worker's closure scope, not the pure function. Audit calls wrapped in inner try/catch (defense-in-depth). `LockRenewalDeps` has an optional `reconnect?` callback; the renewal-tick's catch branch (after retryable-error classification + the time-based deadline check, before the inter-tick sleep) calls it ONCE, wrapped in its own try/catch. Recovers the mid-process-disconnect case (nulled `_sql`) where postgres.js auto-reconnect can't help because the engine's pool reference is gone. Deliberately NOT a background `withRetry` (a 12s background retry could refresh a lock AFTER the worker already decided `lock-renewal-failed` and another worker reclaimed it → two holders); the reconnect is bounded by the same `callTimeoutMs` race + abort signal as renewLock itself. Pinned by `test/worker-lock-renewal.test.ts` (18 hermetic state-machine cases — no fs, no PGLite, no real setInterval). - `src/core/minions/worker-exit-codes.ts` — single source of truth for reserved worker process exit codes, shared by the worker (sets them) and supervisor/CLI (classify them). Exports `WORKER_EXIT_RSS_WATCHDOG = 12`. The RSS watchdog drain must be self-identifying: a code-0 exit is indistinguishable from a healthy queue-drain, so the supervisor's `code===0 → clean_exit` classifier never counted it and a respawn loop stayed invisible. A distinct code makes the drain `likely_cause=rss_watchdog`. Code 12 is deliberately outside {0 clean, 1 runtime_error} and the 128+N signal range. - `src/core/minions/rss-default.ts` — cgroup-aware auto-sized default for the worker RSS watchdog cap. `resolveDefaultMaxRssMb(opts?)` / `describeDefaultMaxRss(opts?)` (provenance for the startup log) / `readCgroupMemLimitBytes(readFile?)`. Replaces a flat 2048MB default at every spawn site (`jobs work`, `jobs supervisor`, autopilot, `MinionSupervisor`). Formula `clamp(round(0.5 × basisMB), 4096, 16384)` where `basis = min(cgroupLimit, totalmem)`. LOAD-BEARING NUANCE: plain `os.totalmem()` reports HOST RAM, so in a 4GB cgroup on a 126GB host it would pick 16GB, the watchdog would never fire, and the kernel OOM-killer would SIGKILL at 4GB. The cap MUST sit below the real ceiling so the graceful drain (distinct exit code, loud log) beats the kernel's silent kill; the 4096 floor applies only when it stays below the basis. Reads cgroup v2 `/sys/fs/cgroup/memory.max` (literal `max` = unlimited) then v1 `/sys/fs/cgroup/memory/memory.limit_in_bytes`. Explicit `--max-rss` (including `0` to disable) always wins. Pinned by `test/rss-default.test.ts`. -- `src/core/cycle/extract-atoms-drain.ts` — pure single-hold bounded drain for the silent lens-phase backlog. `runExtractAtomsDrain(deps, opts)` over injected deps (`withLock`, `runBatch`, `countRemaining`, `now`, optional `onBatch`) loops bounded batches under ONE continuous lock hold, **rediscovering eligibility each batch** (idempotent NOT-EXISTS-on-`source_hash`, so content mutated by a concurrent process simply doesn't match — no cross-window stale cursor), until the backlog is empty OR the time window elapses. Returns `{phase, status, extracted, skipped, remaining, batches, stopped}`. Backs `gbrain dream --phase extract_atoms --drain`. Takes the SAME `cycleLockIdFor(sourceId)` the routine cycle takes (a concurrent autopilot tick genuinely defers with `cycle_already_running`); NO release/reacquire-between-windows primitive. Pinned by `test/extract-atoms-drain.test.ts`. +- `src/core/cycle/extract-atoms-drain.ts` — pure single-hold bounded drain for the silent lens-phase backlog. `runExtractAtomsDrain(deps, opts)` over injected deps (`withLock`, `runBatch`, `countRemaining`, `now`, optional `onBatch`) loops bounded batches under ONE continuous lock hold, **rediscovering eligibility each batch** (idempotent NOT-EXISTS-on-`source_hash`, so content mutated by a concurrent process simply doesn't match — no cross-window stale cursor), until the backlog is empty OR the time window elapses. Returns `{phase, status, extracted, skipped, remaining, batches, stopped}`. Backs `gbrain dream --phase extract_atoms --drain`. Takes the SAME `cycleLockIdFor(sourceId)` the routine cycle takes (a concurrent autopilot tick genuinely defers with `cycle_already_running`); NO release/reacquire-between-windows primitive. The shared wiring helper `runExtractAtomsDrainForSource(engine, {sourceId, windowSeconds, brainDir?, maxBatches?, onBatch?})` owns the lock+batch+count+defer wiring (dynamic imports of db-lock/cycle/extract-atoms keep the pure loop cheap to unit-test) and is the ONE drain path for three callers — `gbrain dream --drain` (which calls it), the `extract-atoms-drain` Minion handler, and autopilot auto-drain — so lock id / window / defer-on-busy can't drift. `sourceId: undefined` → legacy `gbrain-cycle` lock + `'default'` extraction; a real id → `gbrain-cycle:`. `LockUnavailableError` propagates to the caller (each reports the busy case its own way). Pinned by `test/extract-atoms-drain.test.ts`. - `scripts/check-worker-lock-renewal-shape.sh` — CI guard wired into `bun run verify`. Two invariants on `src/core/minions/worker.ts`: (1) the bug pattern `lockTimer = setInterval(async ...)` must NOT appear (narrowed via `lockTimer =` prefix so unrelated `setInterval(async)` calls — like the stall detector — don't false-fire), (2) `runLockRenewalTick` must remain referenced so the pure-function test seam survives refactors. Bug-pattern-specific by design — a future refactor to `setTimeout`-recursion or `AbortController`-based scheduling passes as long as the bug pattern stays absent. POSIX ERE + `[[:space:]]` for BSD-grep portability. Honors `GBRAIN_LOCK_RENEWAL_SHAPE_TARGET` env override for fixture-based meta-tests. Pinned by `test/scripts/check-worker-lock-renewal-shape.test.ts` (5 cases). +- `src/core/doctor-cause-rank.ts` — pure cause-ranking for `gbrain doctor`. `rankIssues(checks)` returns non-ok checks ordered fail-before-warn then root-before-symptom then name (deterministic). `ROOT_CAUSE_CHECKS` / `SYMPTOM_CHECKS` are ORDERING ONLY — tier membership asserts no causality. `downstream_of` is set ONLY from a small map of KNOWN grounded edges (`queue_health` / `supervisor` → `worker_oom_loop`, since they read the same `aborted: watchdog` / `rss_watchdog` source) AND only when the named root is itself failing — never a root×symptom cartesian (co-occurrence never implies causality). `fix` prefers `details.fix_hint` else the message. `CAUSE_GRAPH_NAMES` + `allKnownCheckNames()` back a drift guard asserting every graphed name is a real check. Consumed by `computeDoctorReport` (`top_issues` field, additive, schema_version stays 2) + the "Top issues (ranked by cause)" header in `outputResults`. Pinned by `test/doctor-cause-rank.test.ts`. +- `src/commands/doctor.ts` extension — `computeWorkerOomLoopCheck(engine)` is the single authoritative OOM-loop signal, unioning supervised `summarizeCrashes(readRecentSupervisorEvents(24)).by_cause.rss_watchdog` (cross-week read via `readRecentSupervisorEvents` so a Monday window can't lose Sunday) + bare-worker `minion_jobs error_text='aborted: watchdog'` count (Postgres-only; the same source `queue_health` subcheck 3 reads). Cap comes from the latest `rss_watchdog_loop` breaker alert's `max_rss_mb`, else `resolveDefaultMaxRssMb()` fallback. fail at breaker-tripped or oomKills≥5, warn at ≥1, null otherwise. `computePoolReapHealthCheck(engine)` is the Postgres-only `pool_reap_health` check reading `readRecentPoolRecoveries(1)` — fail when reconnect failures>0 (reconnect throwing is the actionable signal), warn at ≥10 reaps/hr (pooler thrash), null otherwise. Both registered in `buildChecks` after the `supervisor` block. The `supervisor` causeStr carries `rss=N (see worker_oom_loop)` and `queue_health`'s watchdog message cross-references `worker_oom_loop`. `DoctorReport.top_issues` + the cause-ranked render header. `worker_oom_loop` + `pool_reap_health` registered under ops in `doctor-categories.ts`. Pinned by `test/doctor-worker-oom-loop.test.ts`, `test/doctor-pool-reap-health.test.ts`. +- `src/core/audit/pool-recovery-audit.ts` — reap/reconnect audit on the shared `audit-writer` cathedral. Events: `reap_detected` (CONNECTION_ENDED), `reconnect_other` (network/auth/health-check), `reconnect_succeeded`, `reconnect_failed`. `readRecentPoolRecoveries(hours=1)` returns `{reaps, recoveries, failures, others, events}`. Error summaries route through `redactConnectionInfo` before truncation (DSN/host/IP safe). Emitted ONLY from `PostgresEngine.reconnect(ctx?)` (the rare reap-retry path, near-zero hot-path cost); `reconnect()` classifies the threaded error via `isConnectionEndedError` (in retry-matcher.ts) so only true pooler reaps are labeled `reap_detected`. The retry callback in retry.ts threads the triggering error as `(ctx?: {error?}) => Promise`. Pinned by `test/audit/pool-recovery-audit.test.ts`. +- `src/commands/autopilot.ts` extension — per-source `extract_atoms` auto-drain. Postgres-only block after the freshness fan-out: gated on `autopilot.auto_drain.enabled` (default true) AND `!packDeclaresPhase(engine,'extract_atoms')` (the silent-backlog condition) AND per-source `countExtractAtomsBacklog > threshold` (default 25) AND a daily cap `floor(max_usd_per_day / ~$0.30)`. Enumerates `loadAllSources`. Submits the PROTECTED `extract-atoms-drain` job (`{allowProtectedSubmit:true}`) with a UTC-day time-sloted idempotency key `autopilot-extract-atoms-drain::` (a static key would block the source after the first job completed). `src/core/minions/protected-names.ts` adds `extract-atoms-drain`; `src/commands/jobs.ts` registers the handler (thin wrapper over `runExtractAtomsDrainForSource`, `LockUnavailableError` → `{deferred:true}`); `src/core/config.ts` adds the `autopilot.auto_drain.*` config keys + the `autopilot.` key prefix. Pinned by `test/extract-atoms-drain-handler.test.ts`, `test/autopilot-auto-drain-wiring.test.ts`. - `src/commands/doctor.ts:checkBatchRetryHealth` — `batch_retry_health` check surfacing Supavisor circuit-breaker incidents. Wired into both `runDoctor` (local) and `doctorReportRemote` (thin-client). Reads last 24h. States: `ok` (zero exhausted in 24h OR <3 from a single site), `warn` (>=3 same-site OR >=5 cross-site), `fail` (>=20 sustained breaker). Surfaces bad `GBRAIN_BULK_*` env at doctor startup. Corrupt-JSONL tolerant. Paste-ready fix hints in every warn/fail message. Also reads `readRecentDbDisconnects(24)` and appends `Disconnect-call audit: N call(s) in 24h (most recent caller: ).` to ALL three message paths so connection-incident signal is greppable from one `gbrain doctor --json` call (module-import wrapped in try/catch so older brains without the audit file degrade silently). Pinned by `test/doctor-batch-retry.test.ts` (10 cases). - `src/core/audit/db-disconnect-audit.ts` — JSONL audit for every call to `db.disconnect()` and `PostgresEngine.disconnect()`. Built on `audit-writer.ts`. Schema: `{ts, engine_kind: 'postgres'|'pglite'|'unknown', connection_style: 'module'|'instance'|'unknown', caller_stack, command, pid}`. `caller_stack` captured via `new Error().stack` truncated to ~20 frames so operators identify the offending caller without inflating JSONL. Privacy: stack frames carry file paths but NO SQL content / row data / user strings. File: `~/.gbrain/audit/db-disconnect-YYYY-Www.jsonl` (honors `GBRAIN_AUDIT_DIR`). `readRecentDbDisconnects(hours=24)` walks current + previous ISO week and returns `{count, most_recent_caller, files_scanned}`. Wired into `src/core/db.ts:disconnect` and `src/core/postgres-engine.ts:disconnect`, logging BEFORE the early-return branches so even no-op disconnects on never-connected engines are recorded (that case may itself be a caller-side bug). Pinned by `test/db-disconnect-audit.test.ts` (6 cases: round-trip, stack truncation, sort order, empty-dir nulls, stable feature name, EROFS best-effort). - `src/core/facts/queue.ts:FactsQueue.drainPending` — method `drainPending({timeout?: number}): Promise<{drained, unfinished}>`. Semantically distinct from `shutdown()` (which calls `this.internalAbort.abort()` and would abort the very facts:absorb worker trying to log its post-completion event). Drain lets in-flight finish; only the wait is bounded. Default timeout `1000ms` so commands that don't enqueue facts pay one fast 0ms check before exit. `src/cli.ts` op-dispatch finally block awaits `getFactsQueue().drainPending({timeout: 1000})` BEFORE `engine.disconnect()`. Lazy-import keeps the facts-queue module off the hot path for ops that never touch it. Closes the trailing `'No database connection'` line after `gbrain capture` (post-page-write facts:absorb outlived the CLI process). Pinned by `test/facts-queue-drain-pending.test.ts` (4 cases: empty fast-path, in-flight settled without abort, unfinished count on timeout, default timeout = 1000ms). diff --git a/package.json b/package.json index 81760efd5..dd9057ee6 100644 --- a/package.json +++ b/package.json @@ -143,5 +143,5 @@ "bun": ">=1.3.10" }, "license": "MIT", - "version": "0.42.15.0" + "version": "0.42.16.0" } diff --git a/src/commands/autopilot.ts b/src/commands/autopilot.ts index dceed801e..d5831df74 100644 --- a/src/commands/autopilot.ts +++ b/src/commands/autopilot.ts @@ -685,6 +685,115 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) { logError('dispatch.freshness-gate', e); } + // ── #1685 GAP D: per-source extract_atoms auto-drain ─────────────── + // The silent-backlog incident: a pack that doesn't declare extract_atoms + // never runs the phase in the routine cycle, so the atom backlog grows + // invisibly. Auto-submit a bounded, PROTECTED drain per source when the + // backlog exceeds the threshold AND the active pack doesn't declare the + // phase. Default-ON, daily-spend-capped, time-sloted key so a new slot + // opens each UTC day (CODEX #1/#2/#3, DECISION 3C). Postgres-only — + // PGLite has no multi-process worker to run the job. + if (engine.kind === 'postgres') { + try { + const enabled = (await engine.getConfig('autopilot.auto_drain.enabled')) !== 'false'; + if (enabled) { + const { packDeclaresPhase } = await import('../core/cycle.ts'); + // packDeclaresPhase reads the active pack (brain-wide, not + // per-source). If the pack declares extract_atoms the routine + // cycle already drains it for every source — nothing to do. + const declares = await packDeclaresPhase(engine, 'extract_atoms'); + if (!declares) { + const parsePosInt = (v: string | null, d: number): number => { + if (v == null) return d; + const n = parseInt(v, 10); + return Number.isFinite(n) && n > 0 ? n : d; + }; + const parseNonNegFloat = (v: string | null, d: number): number => { + if (v == null) return d; + const n = parseFloat(v); + return Number.isFinite(n) && n >= 0 ? n : d; + }; + const threshold = parsePosInt(await engine.getConfig('autopilot.auto_drain.threshold'), 25); + const windowSeconds = parsePosInt(await engine.getConfig('autopilot.auto_drain.window_seconds'), 120); + const maxUsdPerDay = parseNonNegFloat(await engine.getConfig('autopilot.auto_drain.max_usd_per_day'), 2.0); + // Each drain run is BudgetTracker-capped at ~$0.30; bound the + // brain-wide daily count instead of a real-time spend ledger. + const PER_RUN_USD = 0.3; + const maxJobsToday = Math.max(0, Math.floor(maxUsdPerDay / PER_RUN_USD)); + const utcDay = new Date().toISOString().slice(0, 10); + + let submittedToday = 0; + try { + const rows = await engine.executeRaw<{ cnt: number }>( + `SELECT count(*)::int AS cnt FROM minion_jobs WHERE name = 'extract-atoms-drain' AND created_at >= $1::timestamptz`, + [`${utcDay}T00:00:00Z`], + ); + submittedToday = rows[0]?.cnt ?? 0; + } catch { + // count is best-effort; treat as 0 (cap still bounds submits this tick). + } + + if (submittedToday < maxJobsToday) { + const { loadAllSources } = await import('../core/sources-load.ts'); + const { countExtractAtomsBacklog } = await import('../core/cycle/extract-atoms.ts'); + const sources = await loadAllSources(engine); + for (const src of sources) { + if (submittedToday >= maxJobsToday) break; // brain-wide daily cap (fairness) + if (!src.local_path) continue; + const backlog = await countExtractAtomsBacklog(engine, src.id); + if (backlog === null || backlog <= threshold) continue; + // Time-sloted key (CODEX #2): a static key would block the + // source FOREVER once the first job completes. A new UTC-day + // slot reopens it each day. + const idemKey = `autopilot-extract-atoms-drain:${src.id}:${utcDay}`; + try { + // CODEX (impl review #4): DO NOT use maxWaiting here — it + // coalesces by (name, queue), NOT by source, so source B's + // submit would return source A's waiting row, B would never + // queue, and the cap counter would over-count. The per-source + // idempotency key is the correct dedup. Pre-check it so we + // submit + count only genuinely-new sources (queue.add returns + // the existing row on an idempotency hit with no created flag, + // which would otherwise over-count the daily cap). The + // single-instance autopilot lock + the unique idempotency + // index make this pre-check race-free. + const dupe = await engine.executeRaw<{ one: number }>( + `SELECT 1 AS one FROM minion_jobs WHERE idempotency_key = $1 LIMIT 1`, + [idemKey], + ); + if (dupe.length > 0) continue; // already queued/drained for this source today + const job = await queue.add( + 'extract-atoms-drain', + { sourceId: src.id, window: windowSeconds, repoPath: src.local_path }, + { + queue: 'default', + idempotency_key: idemKey, + max_attempts: 1, + timeout_ms: timeoutMs, + }, + { allowProtectedSubmit: true }, + ); + submittedToday++; + if (jsonMode) { + process.stderr.write(JSON.stringify({ + event: 'dispatched', job_id: job.id, mode: 'auto-drain', + source_id: src.id, backlog, + }) + '\n'); + } else { + console.log(`[dispatch] job #${job.id} extract-atoms-drain (auto-drain: ${src.id}; backlog=${backlog})`); + } + } catch (e) { + logError('dispatch.auto-drain', e); + } + } + } + } + } + } catch (e) { + logError('dispatch.auto-drain-gate', e); + } + } + // Cheap path: engine.getHealth() is a single SQL count query. const health = await engine.getHealth(); const score = health.brain_score; diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index f7b5d637c..f00d0fe21 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -21,6 +21,7 @@ import { loadCompletedMigrations } from '../core/preferences.ts'; import { compareVersions } from './migrations/index.ts'; import { createProgress, startHeartbeat, type ProgressReporter } from '../core/progress.ts'; import { categorizeCheck, type CheckCategory } from '../core/doctor-categories.ts'; +import { rankIssues, type RankedIssue } from '../core/doctor-cause-rank.ts'; import { getCliOptions, cliOptsToProgressOptions } from '../core/cli-options.ts'; import type { DbUrlSource } from '../core/config.ts'; import { gbrainPath } from '../core/config.ts'; @@ -127,6 +128,12 @@ export interface DoctorReport { meta: number; }; checks: Check[]; + /** + * v0.42.x (#1685 GAP C) — non-ok checks ranked by cause (root before symptom, + * fail before warn). Lets an agent act on the root cause without re-deriving + * the ranking. Additive + optional; schema_version stays at 2. + */ + top_issues?: RankedIssue[]; } function _penaltyScore(checks: Check[]): number { @@ -179,6 +186,7 @@ export function computeDoctorReport(checks: Check[]): DoctorReport { meta: _penaltyScore(meta), }, checks: tagged, + top_issues: rankIssues(tagged), }; } @@ -3568,6 +3576,176 @@ export async function checkCycleFreshness( * - `progress` reporter writes to stderr (heartbeats per check) * - `engine.executeRaw` / handler-leaf calls (the actual probe work) */ +/** + * issue #1685 (GAP A) — the single authoritative "worker is OOM-looping" signal. + * + * One `gbrain doctor` line replaces the hours of log archaeology the #1678 + * incident required: `cap=8192MB, N watchdog kills/24h → raise --max-rss`. + * + * UNIONS two sources so it's authoritative for BOTH worker modes (CODEX #5): + * - SUPERVISED workers: supervisor audit `worker_exited likely_cause=rss_watchdog`, + * read cross-week (CODEX #7) so a Mon read doesn't lose a Sun loop. + * - BARE `gbrain jobs work`: NO supervisor event is written; the only trace is + * `minion_jobs.error_text = 'aborted: watchdog'` (the same source queue_health + * subcheck 3 reads). Reading supervisor-only would miss bare workers entirely + * and the queue_health cross-reference would point at an unemitted check. + * + * Cap (CODEX #6): the breaker alert stamps `max_rss_mb`, but a fail from + * oomKills>=5 spread over 24h may have no breaker event → no stamped cap. Fall + * back to `resolveDefaultMaxRssMb()` so the message always renders a number. + * + * Returns null when the worker never OOM'd (don't warn installs that never hit + * it). Pure-ish: filesystem audit read + one minion_jobs count; no process.exit. + * Exported so `test/doctor-worker-oom-loop.test.ts` drives it directly. + */ +export async function computeWorkerOomLoopCheck( + engine: BrainEngine | null, +): Promise { + let supervisorKills = 0; + let capFromBreaker: number | null = null; + let breakerTripped = false; + try { + const { readRecentSupervisorEvents, summarizeCrashes } = await import( + '../core/minions/handlers/supervisor-audit.ts' + ); + const events = readRecentSupervisorEvents(24); + supervisorKills = summarizeCrashes(events).by_cause.rss_watchdog; + // Latest rss_watchdog_loop breaker alert carries the cap the supervisor + // spawned with (supervisor.ts:521); its presence also means the breaker + // tripped. Walk all events; last one wins for the cap. + for (const e of events) { + const row = e as Record; + if (e.event === 'health_warn' && row.reason === 'rss_watchdog_loop') { + breakerTripped = true; + const cap = Number(row.max_rss_mb); + if (Number.isFinite(cap) && cap > 0) capFromBreaker = cap; + } + } + } catch { + // supervisor-audit read is best-effort; fall through to minion_jobs. + } + + let bareWorkerKills = 0; + if (engine && engine.kind !== 'pglite') { + try { + const sql = db.getConnection(); + const rows: Array<{ cnt: number }> = await sql` + SELECT count(*)::int AS cnt + FROM minion_jobs + WHERE status IN ('dead', 'failed') + AND finished_at > now() - interval '24 hours' + AND error_text = 'aborted: watchdog' + `; + bareWorkerKills = rows[0]?.cnt ?? 0; + } catch { + // minion_jobs may not exist on a fresh brain; best-effort. + } + } + + // De-dup note (CODEX #5 accepted trade-off): a supervised watchdog kill aborts + // in-flight jobs, so it can show in BOTH counts. We accept slight over-count + // rather than miss bare workers — the signal is "is it OOM-looping," not an + // exact tally. `details` keeps the two sources separate for honesty. + const oomKills = supervisorKills + bareWorkerKills; + if (oomKills < 1 && !breakerTripped) return null; + + let capMb: number; + let capSource: 'breaker' | 'default'; + if (capFromBreaker !== null) { + capMb = capFromBreaker; + capSource = 'breaker'; + } else { + let def = 16384; + try { + const { resolveDefaultMaxRssMb } = await import('../core/minions/rss-default.ts'); + def = resolveDefaultMaxRssMb(); + } catch { + // keep the conservative ceiling fallback. + } + capMb = def; + capSource = 'default'; + } + + const fixHint = + 'raise --max-rss (gbrain jobs work --max-rss ; auto-sizes to min(0.5×RAM,16GB))'; + const capLabel = capSource === 'breaker' ? `cap=${capMb}MB` : `cap≈${capMb}MB (auto-sized default)`; + const status: Check['status'] = breakerTripped || oomKills >= 5 ? 'fail' : 'warn'; + return { + name: 'worker_oom_loop', + status, + message: + `Worker OOM-looping: ${capLabel}, ${oomKills} watchdog kill(s)/24h → ${fixHint}. ` + + `Peak RSS: see worker stderr.`, + details: { + oom_kills: oomKills, + supervisor_kills: supervisorKills, + bare_worker_kills: bareWorkerKills, + cap_mb: capMb, + cap_source: capSource, + breaker_tripped: breakerTripped, + fix_hint: fixHint, + }, + }; +} + +/** + * issue #1685 (GAP B) — DB pool reap health (Postgres-only). + * + * Answers the #1685 line "DB pool reaped N times/hr AND not auto-recovering" + * that no existing signal expresses. Reads the pool-recovery audit + * (`reconnect()` emits reap_detected / reconnect_succeeded / reconnect_failed): + * - fail: reaps>0 AND reconnect failures>0 → the pool is being reaped and + * rebuilds are throwing (genuinely not recovering). + * - warn: reaps>=10/hr, all recovered → pooler thrash (self-heal works but the + * cap is likely too low / concurrency too high). + * - else: null (quiet — a few reaps that all recovered is normal). + * + * Returns null on PGLite / no engine / audit-read failure. Exported so + * `test/doctor-pool-reap-health.test.ts` drives it directly. + */ +export async function computePoolReapHealthCheck( + engine: BrainEngine | null, +): Promise { + if (!engine || engine.kind === 'pglite') return null; + let r: { reaps: number; recoveries: number; failures: number }; + try { + const { readRecentPoolRecoveries } = await import('../core/audit/pool-recovery-audit.ts'); + r = readRecentPoolRecoveries(1); + } catch { + return null; + } + + // CODEX (impl review #3): the audit counts independent event kinds — it does + // NOT correlate a reconnect_failed to a preceding reap. So `reaps>0 AND + // failures>0` would falsely report "not auto-recovering" when a recovered reap + // and an unrelated reconnect failure merely co-occur in the same hour. Fail on + // the reconnect FAILURES themselves (reconnect throwing is the real, actionable + // problem regardless of reaps); report reaps as context, not as a causal claim. + if (r.failures > 0) { + const fix = 'check DB reachability / credentials (reconnect is throwing)'; + return { + name: 'pool_reap_health', + status: 'fail', + message: + `DB reconnect FAILED ${r.failures}× in last hour (${r.reaps} pooler reap(s) detected) ` + + `— reconnect is throwing; ${fix}.`, + details: { reaps: r.reaps, recoveries: r.recoveries, failures: r.failures, fix_hint: fix }, + }; + } + if (r.reaps >= 10) { + const fix = 'raise --max-rss or reduce worker concurrency (pooler thrash)'; + return { + name: 'pool_reap_health', + status: 'warn', + message: + `DB pool reaped ${r.reaps}× in last hour (self-heal recovered each) ` + + `— ${fix}.`, + details: { reaps: r.reaps, recoveries: r.recoveries, failures: r.failures, fix_hint: fix }, + }; + } + return null; +} + export async function buildChecks( engine: BrainEngine | null, args: string[], @@ -3847,7 +4025,7 @@ export async function buildChecks( // shape is the right contract. const summary = summarizeCrashes(events); const crashes24h = summary.total; - const causeStr = `runtime=${summary.by_cause.runtime_error} oom=${summary.by_cause.oom_or_external_kill} unknown=${summary.by_cause.unknown} legacy=${summary.by_cause.legacy}`; + const causeStr = `runtime=${summary.by_cause.runtime_error} oom=${summary.by_cause.oom_or_external_kill} rss=${summary.by_cause.rss_watchdog} unknown=${summary.by_cause.unknown} legacy=${summary.by_cause.legacy}${summary.by_cause.rss_watchdog > 0 ? ' (see worker_oom_loop)' : ''}`; const maxCrashesEvent = events.filter(e => e.event === 'max_crashes_exceeded').pop() ?? null; // Only surface a Check if the supervisor was ever observed (stops the @@ -3886,6 +4064,27 @@ export async function buildChecks( // Audit read / import failure is best-effort; skip silently. } + // 3b-quater. Worker OOM-loop (issue #1685 GAP A) — the single authoritative + // "is the worker OOM-looping" line, unioning supervised (supervisor audit) + // and bare-worker (minion_jobs watchdog-abort) kills. Returns null when the + // worker never OOM'd, so clean installs see nothing. + try { + const oomCheck = await computeWorkerOomLoopCheck(engine); + if (oomCheck) checks.push(oomCheck); + } catch { + // best-effort. + } + + // 3b-quinquies. DB pool reap health (issue #1685 GAP B) — Postgres pooler + // reap frequency + recovered-vs-stuck split. Quiet unless reaps thrash or + // reconnect is failing. + try { + const reapCheck = await computePoolReapHealthCheck(engine); + if (reapCheck) checks.push(reapCheck); + } catch { + // best-effort. + } + // 3b-tris. Stub-guard fire count (last 24h). The v0.34.5 stub guard in // fence-write.ts refuses to spawn unprefixed entity pages (e.g. bare // `alice.md` at brain root). Each fire is appended to @@ -6207,9 +6406,8 @@ export async function buildChecks( if (rssKillCount > 0) { problems.push( `${rssKillCount} job(s) dead-lettered for RSS-watchdog memory-limit kills in last 24h. ` + - `v0.22.14 changed the bare-worker --max-rss default from 0 (off) to 2048 MB. ` + `Fix: raise the limit (e.g. \`gbrain jobs work --max-rss 4096\`) or opt out (\`--max-rss 0\`). ` + - `See skills/migrations/v0.22.14.md.` + `→ see worker_oom_loop for the cap + fix (the authoritative OOM-loop signal).` ); } if (promptTooLongCount > 0) { @@ -6787,6 +6985,25 @@ function outputResults(checks: Check[], json: boolean): boolean { console.log('\nGBrain Health Check'); console.log('==================='); + + // #1685 GAP C — cause-ranked summary so the operator reads the root cause + // first instead of scrolling the full list. Caps at 5; clean brains skip it. + const topIssues = report.top_issues ?? []; + if (topIssues.length > 0) { + console.log(''); + console.log('Top issues (ranked by cause):'); + const shown = topIssues.slice(0, 5); + for (const issue of shown) { + const icon = issue.status === 'fail' ? 'FAIL' : 'WARN'; + const dn = issue.downstream_of ? ` (likely downstream of ${issue.downstream_of})` : ''; + console.log(` [${icon}] ${issue.name}${dn} → ${issue.fix}`); + } + if (topIssues.length > shown.length) { + console.log(` +${topIssues.length - shown.length} more — see full list below`); + } + console.log(''); + } + for (const c of report.checks) { const icon = c.status === 'ok' ? 'OK' : c.status === 'warn' ? 'WARN' : 'FAIL'; console.log(` [${icon}] ${c.name}: ${c.message}`); diff --git a/src/commands/dream.ts b/src/commands/dream.ts index 62ff3d826..e90a01963 100644 --- a/src/commands/dream.ts +++ b/src/commands/dream.ts @@ -27,7 +27,6 @@ import type { BrainEngine } from '../core/engine.ts'; import { runCycle, ALL_PHASES, - cycleLockIdFor, type CyclePhase, type CycleReport, } from '../core/cycle.ts'; @@ -452,15 +451,11 @@ async function runDrain( resolvedSourceId: string | undefined, brainDir: string | null, ): Promise { - const { withRefreshingLock, LockUnavailableError } = await import('../core/db-lock.ts'); - const { runPhaseExtractAtoms, countExtractAtomsBacklog } = await import('../core/cycle/extract-atoms.ts'); - const { runExtractAtomsDrain } = await import('../core/cycle/extract-atoms-drain.ts'); + const { LockUnavailableError } = await import('../core/db-lock.ts'); + const { countExtractAtomsBacklog } = await import('../core/cycle/extract-atoms.ts'); + const { runExtractAtomsDrainForSource } = await import('../core/cycle/extract-atoms-drain.ts'); const extractionSourceId = resolvedSourceId ?? 'default'; - // undefined → legacy 'gbrain-cycle' lock, exactly what the unscoped routine - // cycle holds; a real source → 'gbrain-cycle:'. Either way the drain and - // the routine cycle for THIS source genuinely contend (Codex #9). - const lockId = cycleLockIdFor(resolvedSourceId); // Dry-run: preview the backlog without holding the lock or extracting. if (opts.dryRun) { @@ -479,26 +474,16 @@ async function runDrain( let result; try { - result = await runExtractAtomsDrain( - { - withLock: (work) => withRefreshingLock(engine, lockId, work, { ttlMinutes: 5 }), - runBatch: async () => { - const r = await runPhaseExtractAtoms(engine, { - sourceId: extractionSourceId, - dryRun: false, - brainDir: brainDir ?? undefined, - }); - const d = (r.details ?? {}) as Record; - return { extracted: Number(d.atoms_extracted ?? 0), skipped: Number(d.duplicates_skipped ?? 0) }; - }, - countRemaining: () => countExtractAtomsBacklog(engine, extractionSourceId), - now: Date.now, - onBatch: opts.json ? undefined : ({ batch, extracted, remaining }) => { - process.stderr.write(`[drain] batch ${batch}: +${extracted} atom(s), ~${remaining ?? '?'} remaining\n`); - }, + // DECISION 5A: the lock/batch/count wiring lives in the shared helper so + // the CLI path, the Minion handler, and autopilot's auto-drain can't drift. + result = await runExtractAtomsDrainForSource(engine, { + sourceId: resolvedSourceId, + windowSeconds: opts.windowSeconds, + brainDir: brainDir ?? undefined, + onBatch: opts.json ? undefined : ({ batch, extracted, remaining }) => { + process.stderr.write(`[drain] batch ${batch}: +${extracted} atom(s), ~${remaining ?? '?'} remaining\n`); }, - { windowMs: opts.windowSeconds * 1000 }, - ); + }); } catch (e) { if (e instanceof LockUnavailableError) { if (opts.json) { diff --git a/src/commands/jobs.ts b/src/commands/jobs.ts index 8dd5599e1..fdedc7410 100644 --- a/src/commands/jobs.ts +++ b/src/commands/jobs.ts @@ -1623,6 +1623,36 @@ export async function registerBuiltinHandlers(worker: MinionWorker, engine: Brai worker.register('resolve_symbol_edges', makePhaseHandler('resolve_symbol_edges')); worker.register('recompute_emotional_weight', makePhaseHandler('recompute_emotional_weight')); + // v0.42.x (#1685 GAP D) — PROTECTED bounded extract_atoms backlog drain. + // Thin wrapper over the shared helper (DECISION 5A) so the CLI `--drain` + // path, this handler, and autopilot's auto-drain can't diverge on lock id / + // window / defer behavior. On LockUnavailableError (the routine cycle holds + // the per-source lock) the job completes `{ deferred: true }` and retries + // next tick instead of failing — cooperative interleave (CODEX accepted). + worker.register('extract-atoms-drain', async (job) => { + const { runExtractAtomsDrainForSource } = await import('../core/cycle/extract-atoms-drain.ts'); + const { LockUnavailableError } = await import('../core/db-lock.ts'); + const sourceId = typeof job.data.sourceId === 'string' ? job.data.sourceId : undefined; + const windowSeconds = + typeof job.data.window === 'number' && job.data.window > 0 ? job.data.window : 120; + const repoPath = + typeof job.data.repoPath === 'string' + ? job.data.repoPath + : ((await engine.getConfig('sync.repo_path')) ?? undefined); + try { + return await runExtractAtomsDrainForSource(engine, { + sourceId, + windowSeconds, + brainDir: repoPath, + }); + } catch (e) { + if (e instanceof LockUnavailableError) { + return { phase: 'extract_atoms', status: 'skipped', deferred: true, reason: 'cycle_already_running' }; + } + throw e; + } + }); + // v0.40 Federated Sync v2 — embed-backfill: per-source decoupled embed. // Cost-bounded via D6 ($10/job BudgetTracker) + D19 (source-level cooldown // + 24h rolling cap, gated at submit time). NOT in PROTECTED_JOB_NAMES — diff --git a/src/core/audit/pool-recovery-audit.ts b/src/core/audit/pool-recovery-audit.ts new file mode 100644 index 000000000..2cdc5e8bf --- /dev/null +++ b/src/core/audit/pool-recovery-audit.ts @@ -0,0 +1,134 @@ +/** + * issue #1685 (GAP B) — pool reconnect/reap recovery audit. + * + * The #1678 incident's DB-cascade noise looked like a connection bug. In + * reality a transaction-mode pooler reaps idle sockets between lock-renewal + * ticks; gbrain self-heals via `PostgresEngine.reconnect()`. The thing an + * operator actually needs to know — and that no existing signal expresses — is + * "the pool was reaped N times in the last hour AND is NOT auto-recovering." + * `batch_retry_health` surfaces connection retries but can't split + * recovered-from-stuck. This audit does. + * + * HONESTY (CODEX #8): `reconnect()` fires for ANY retryable connection error + * (network blip, auth race, pooler circuit), not just a pooler reap. Logging + * everything as a "reap" would mislabel. So the caller passes the classified + * error and we record the TRUE kind: + * - `reap_detected` the triggering error matched CONNECTION_ENDED + * (postgres.js's pooler-reap library code) + * - `reconnect_other` a reconnect for some other retryable cause (or no + * classified error, e.g. a health-check reconnect) + * - `reconnect_succeeded` the rebuild completed + * - `reconnect_failed` the rebuild threw (NOT auto-recovering) + * + * Built on the shared `audit-writer.ts` cathedral — same ISO-week rotation, + * same best-effort write semantics. File: + * `~/.gbrain/audit/pool-recovery-YYYY-Www.jsonl` (honors `GBRAIN_AUDIT_DIR`). + * + * Privacy: `error_summary` is the error message truncated to 200 chars. It can + * carry a DSN/host in a connection-failure message — routed through the shared + * `redactConnectionInfo` helper before truncation, same as lock-renewal-audit / + * batch-retry-audit (v0.41.26.1 posture). + */ + +import { createAuditWriter } from './audit-writer.ts'; +import { redactConnectionInfo } from './redact-connection-info.ts'; + +export type PoolRecoveryEventKind = + | 'reap_detected' + | 'reconnect_other' + | 'reconnect_succeeded' + | 'reconnect_failed'; + +export interface PoolRecoveryEvent { + ts: string; + kind: PoolRecoveryEventKind; + /** Redacted + truncated triggering-error message; absent on success events. */ + error_summary?: string; + pid: number; +} + +const FEATURE_NAME = 'pool-recovery'; + +const writer = createAuditWriter({ + featureName: FEATURE_NAME, + errorLabel: 'pool-recovery-audit', + errorTrailer: '; continuing', +}); + +/** Redact + truncate an error message for safe audit storage. */ +function summarizeError(err: unknown): string | undefined { + if (err === undefined || err === null) return undefined; + const raw = err instanceof Error ? err.message : String(err); + return redactConnectionInfo(raw).slice(0, 200); +} + +/** + * Log one pool-recovery event. Best-effort: stderr-warns on write failure but + * never throws. The caller's reconnect path continues regardless. + */ +export function logPoolRecovery(kind: PoolRecoveryEventKind, err?: unknown): void { + const summary = summarizeError(err); + writer.log({ + kind, + pid: process.pid, + ...(summary !== undefined ? { error_summary: summary } : {}), + }); +} + +export interface ReadPoolRecoveryResult { + events: PoolRecoveryEvent[]; + /** CONNECTION_ENDED-triggered reconnects (true pooler reaps) in window. */ + reaps: number; + /** Successful rebuilds in window. */ + recoveries: number; + /** Failed rebuilds in window (the "not auto-recovering" signal). */ + failures: number; + /** Non-reap reconnects (network/auth/health-check) in window. */ + others: number; + most_recent_ts: string | null; +} + +/** + * Read recent pool-recovery events. Default window is 1h (the "is it thrashing + * right now" question), not the audit-writer 7-day default. Consumed by the + * `pool_reap_health` doctor check. + */ +export function readRecentPoolRecoveries( + hours = 1, + now: Date = new Date(), +): ReadPoolRecoveryResult { + const days = hours / 24; + const cutoff = now.getTime() - hours * 3_600_000; + const events = writer + .readRecent(days, now) + .filter((e) => { + const t = Date.parse(e.ts); + return Number.isFinite(t) && t >= cutoff; + }) + .sort((a, b) => Date.parse(b.ts) - Date.parse(a.ts)); + + let reaps = 0; + let recoveries = 0; + let failures = 0; + let others = 0; + for (const e of events) { + if (e.kind === 'reap_detected') reaps++; + else if (e.kind === 'reconnect_succeeded') recoveries++; + else if (e.kind === 'reconnect_failed') failures++; + else if (e.kind === 'reconnect_other') others++; + } + + return { + events, + reaps, + recoveries, + failures, + others, + most_recent_ts: events[0]?.ts ?? null, + }; +} + +/** @internal — test seam to pin the file location / feature name. */ +export function _poolRecoveryAuditFeatureName(): string { + return FEATURE_NAME; +} diff --git a/src/core/config.ts b/src/core/config.ts index 0f9ab11ad..0aa815211 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -96,6 +96,22 @@ export interface GBrainConfig { */ max_usd?: number; }; + /** + * v0.42.x (#1685 GAP D) — extract_atoms backlog auto-drain. Default ON so a + * pack-gated silent backlog never piles up unseen; daily-spend-capped so the + * Haiku spend stays bounded. Read via the DB plane (`engine.getConfig`) at + * each autopilot tick. Disable with `gbrain config set autopilot.auto_drain.enabled false`. + */ + auto_drain?: { + /** Master switch. Default true. */ + enabled?: boolean; + /** Per-drain wallclock budget in seconds. Default 120. */ + window_seconds?: number; + /** Backlog must exceed this to trigger a drain. Default 25. */ + threshold?: number; + /** Daily spend cap (USD); bounds drains/day = floor(cap / ~$0.30). Default 2.0. */ + max_usd_per_day?: number; + }; }; eval?: { /** false disables capture entirely. Defaults to true. */ @@ -785,6 +801,7 @@ export const KNOWN_CONFIG_KEY_PREFIXES: readonly string[] = [ 'provider_base_urls.', // per-provider base URL overrides 'content_sanity.', // v0.41 content-sanity tunables 'mcp.', // mcp.publish_skills, mcp.skills_dir (PR1 skill catalog) + 'autopilot.', // autopilot.nightly_quality_probe.*, autopilot.auto_drain.* (#1685) 'self_upgrade.', // v0.42 self-upgrade (mode, quiet_hours, state) ]; diff --git a/src/core/cycle/extract-atoms-drain.ts b/src/core/cycle/extract-atoms-drain.ts index 0310313f6..98a4bfa69 100644 --- a/src/core/cycle/extract-atoms-drain.ts +++ b/src/core/cycle/extract-atoms-drain.ts @@ -17,9 +17,13 @@ * knows whether to run again. * * Pure over injected deps: no DB, no LLM, no lock primitive imported here, so - * the loop logic is unit-testable. `dream.ts` wires the real deps. + * the loop logic is unit-testable. The wiring helper `runExtractAtomsDrainForSource` + * (below) builds the real deps; it uses DYNAMIC imports so this module's static + * graph stays empty and the pure-loop unit tests don't drag in db-lock / cycle. */ +import type { BrainEngine } from '../engine.ts'; + export interface ExtractAtomsDrainDeps { /** * Run the loop body while holding the cycle lock. Implemented by the caller @@ -94,3 +98,74 @@ export async function runExtractAtomsDrain( return { phase: 'extract_atoms', status: 'ok', extracted, skipped, remaining, batches, stopped }; }); } + +// ─── Shared wiring helper (v0.42.x #1685 DECISION 5A) ────────────────────── +// +// ONE drain path, three callers: `gbrain dream --phase extract_atoms --drain` +// (dream.ts), the `extract-atoms-drain` Minion handler (jobs.ts), and the +// autopilot auto-drain submission (which routes through the handler). Before +// this helper the lock/batch/count wiring lived inline in dream.ts:482; a second +// copy in the handler would let lock id / window default / defer-on-lock-busy +// drift. Keeping the wiring here means those three callers can't diverge. +// +// Imports are dynamic so the pure `runExtractAtomsDrain` above stays cheap to +// import in unit tests (no db-lock / cycle / extract-atoms in the static graph). +// +// `LockUnavailableError` is NOT caught here — the pure loop's `withLock` +// (withRefreshingLock) throws it and it propagates to the caller, because each +// caller reports the busy-lock case differently (dream → exit 3; +// handler → `{ deferred: true }`). That matches the contract documented on +// `ExtractAtomsDrainDeps.withLock`. + +export interface DrainForSourceOpts { + /** + * The RESOLVED source id, or `undefined` for the legacy unscoped cycle. + * `undefined` → `cycleLockIdFor(undefined)` = the bare `gbrain-cycle` lock the + * unscoped routine cycle holds; a real id → `gbrain-cycle:`. Either way the + * drain and the routine cycle for THIS source genuinely contend (Codex #9). + * The extraction/backlog source is `sourceId ?? 'default'`. + */ + sourceId: string | undefined; + /** Wallclock budget in seconds. */ + windowSeconds: number; + /** Brain checkout dir, threaded to `runPhaseExtractAtoms` (optional — DB-only ok). */ + brainDir?: string; + /** Hard batch cap (belt-and-suspenders). */ + maxBatches?: number; + /** Optional per-batch progress sink (stderr line in dream; job progress in the handler). */ + onBatch?: ExtractAtomsDrainDeps['onBatch']; +} + +export async function runExtractAtomsDrainForSource( + engine: BrainEngine, + opts: DrainForSourceOpts, +): Promise { + const { withRefreshingLock } = await import('../db-lock.ts'); + const { runPhaseExtractAtoms, countExtractAtomsBacklog } = await import('./extract-atoms.ts'); + const { cycleLockIdFor } = await import('../cycle.ts'); + + const extractionSourceId = opts.sourceId ?? 'default'; + const lockId = cycleLockIdFor(opts.sourceId); + + return runExtractAtomsDrain( + { + withLock: (work) => withRefreshingLock(engine, lockId, work, { ttlMinutes: 5 }), + runBatch: async () => { + const r = await runPhaseExtractAtoms(engine, { + sourceId: extractionSourceId, + dryRun: false, + brainDir: opts.brainDir, + }); + const d = (r.details ?? {}) as Record; + return { + extracted: Number(d.atoms_extracted ?? 0), + skipped: Number(d.duplicates_skipped ?? 0), + }; + }, + countRemaining: () => countExtractAtomsBacklog(engine, extractionSourceId), + now: Date.now, + onBatch: opts.onBatch, + }, + { windowMs: opts.windowSeconds * 1000, maxBatches: opts.maxBatches }, + ); +} diff --git a/src/core/doctor-categories.ts b/src/core/doctor-categories.ts index f19b5eb64..48dca2fae 100644 --- a/src/core/doctor-categories.ts +++ b/src/core/doctor-categories.ts @@ -143,12 +143,14 @@ export const OPS_CHECK_NAMES: ReadonlySet = new Set([ 'rls', 'rls_event_trigger', 'search_mode', + 'pool_reap_health', 'self_upgrade_health', 'stale_locks', 'subagent_capability', 'subagent_health', 'supervisor', 'sync_consolidation', + 'worker_oom_loop', 'ze_embedding_health', ]); diff --git a/src/core/doctor-cause-rank.ts b/src/core/doctor-cause-rank.ts new file mode 100644 index 000000000..647c6ba8b --- /dev/null +++ b/src/core/doctor-cause-rank.ts @@ -0,0 +1,149 @@ +/** + * issue #1685 (GAP C) — cause-ranked doctor issues. + * + * The #1685 posture ask: `gbrain doctor` is the single health truth, and it + * surfaces CAUSE before symptoms. During the #1678 incident the loud lines were + * all downstream DB-cascade noise (CONNECTION_ENDED, lock-renewal-failed) while + * the one true cause (RSS-watchdog OOM kill) scrolled by once. This module ranks + * the non-ok checks so the operator reads root causes first. + * + * HONESTY CONTRACT (CODEX #9): two checks both failing does NOT prove one caused + * the other. So: + * - Tier membership (root vs symptom) is ORDERING ONLY. It sorts roots above + * symptoms; it asserts NO causality. + * - `downstream_of` — the one place we DO claim a causal link — is set ONLY + * from a small map of KNOWN, grounded edges, AND only when the named root is + * itself in the failing set. It is deliberately NOT a root×symptom cartesian. + * "Everything failing is downstream of every root" is the false-precision we + * refuse to ship. + * + * Pure: no I/O, no engine, no process.exit. Unit-tested directly by + * `test/doctor-cause-rank.test.ts`, including the drift guard that every name in + * the cause graph still exists in `doctor-categories.ts` (DECISION 4A). + */ + +import { + BRAIN_CHECK_NAMES, + SKILL_CHECK_NAMES, + OPS_CHECK_NAMES, + META_CHECK_NAMES, +} from './doctor-categories.ts'; + +/** Minimal structural shape of a doctor Check that ranking needs. */ +export interface RankableCheck { + name: string; + status: 'ok' | 'warn' | 'fail'; + message: string; + details?: Record; +} + +export interface RankedIssue { + name: string; + status: 'warn' | 'fail'; + /** + * Coarse sort bucket. `root` = a designated root-cause check; `symptom` = + * everything else (NOT a proof that it's a downstream effect — just "not on + * the root-cause list"). The precise causal claim lives in `downstream_of`. + */ + tier: 'root' | 'symptom'; + /** + * Set ONLY for a known causal edge whose root is also failing. Absent + * otherwise — we never invent causality from co-occurrence. + */ + downstream_of?: string; + /** One-line fix. Prefers `details.fix_hint`; falls back to the check message. */ + fix: string; +} + +/** + * Checks that, when failing, are usually the DISEASE. Sorted to the top so the + * operator reads the cause first. Membership is ORDERING ONLY (CODEX #9). + */ +export const ROOT_CAUSE_CHECKS: ReadonlySet = new Set([ + 'worker_oom_loop', + 'pool_reap_health', + 'connection', + 'sync_freshness', + 'schema_version', +]); + +/** + * Checks that are commonly DOWNSTREAM noise during an incident. Sorted below + * roots. Ordering only — not a causal claim. + */ +export const SYMPTOM_CHECKS: ReadonlySet = new Set([ + 'queue_health', + 'batch_retry_health', + 'supervisor', + 'stale_locks', +]); + +/** + * KNOWN causal edges (symptom → root). `downstream_of` is set ONLY from this + * map AND ONLY when the named root is itself failing. Each edge is a real, + * grounded link, not a taxonomy guess: + * - queue_health → worker_oom_loop: an RSS-watchdog OOM kill aborts in-flight + * jobs; queue_health reads the SAME `error_text='aborted: watchdog'` rows + * that worker_oom_loop counts for bare workers. + * - supervisor → worker_oom_loop: the watchdog drain is a supervisor + * `worker_exited likely_cause=rss_watchdog` — the exact event + * worker_oom_loop's supervised half counts. + */ +const DOWNSTREAM_EDGES: Readonly> = { + queue_health: 'worker_oom_loop', + supervisor: 'worker_oom_loop', +}; + +function tierOf(name: string): 'root' | 'symptom' { + return ROOT_CAUSE_CHECKS.has(name) ? 'root' : 'symptom'; +} + +/** + * Rank non-ok checks: fail before warn, root before symptom, then name + * (deterministic). Returns the full ranked list; the renderer caps to top-N. + */ +export function rankIssues(checks: RankableCheck[]): RankedIssue[] { + const failing = checks.filter((c) => c.status !== 'ok'); + const failingNames = new Set(failing.map((c) => c.name)); + + const issues: RankedIssue[] = failing.map((c) => { + const root = DOWNSTREAM_EDGES[c.name]; + const downstream_of = root && failingNames.has(root) ? root : undefined; + const hint = c.details?.fix_hint; + const fix = + typeof hint === 'string' && hint.trim().length > 0 ? hint : c.message; + return { + name: c.name, + status: c.status as 'warn' | 'fail', + tier: tierOf(c.name), + ...(downstream_of ? { downstream_of } : {}), + fix, + }; + }); + + const statusRank = (s: string): number => (s === 'fail' ? 0 : 1); + const tierRank = (t: string): number => (t === 'root' ? 0 : 1); + issues.sort( + (a, b) => + statusRank(a.status) - statusRank(b.status) || + tierRank(a.tier) - tierRank(b.tier) || + a.name.localeCompare(b.name), + ); + return issues; +} + +/** Every name referenced by the cause graph (tiers). Drift-guard target (4A). */ +export const CAUSE_GRAPH_NAMES: ReadonlySet = new Set([ + ...ROOT_CAUSE_CHECKS, + ...SYMPTOM_CHECKS, +]); + +/** Union of all category-known check names — the drift-guard comparison set. */ +export function allKnownCheckNames(): ReadonlySet { + return new Set([ + ...BRAIN_CHECK_NAMES, + ...SKILL_CHECK_NAMES, + ...OPS_CHECK_NAMES, + ...META_CHECK_NAMES, + ]); +} diff --git a/src/core/minions/handlers/supervisor-audit.ts b/src/core/minions/handlers/supervisor-audit.ts index 38ce47ef2..40f46c858 100644 --- a/src/core/minions/handlers/supervisor-audit.ts +++ b/src/core/minions/handlers/supervisor-audit.ts @@ -117,6 +117,33 @@ export function readSupervisorEvents(opts: { sinceMs?: number } = {}): Superviso return events; } +/** + * Cross-week supervisor read for windows that can straddle a Monday boundary + * (issue #1685, CODEX #7). The single-file `readSupervisorEvents` above can lose + * a Sunday-night OOM loop when read on Monday because it only opens the current + * ISO-week file. This variant delegates to the shared writer's `readRecent`, + * which walks current + previous week (the same prev-week walk db-disconnect and + * the other audits already use), then applies an hour-precision cutoff. + * + * Used by `worker_oom_loop` in doctor.ts so the OOM-loop verdict can't silently + * miss a week-boundary incident. The legacy `readSupervisorEvents` keeps its + * single-file semantics so the existing `supervisor` check's assertions don't + * shift (and PR #1688, concurrently editing that check, doesn't conflict). + */ +export function readRecentSupervisorEvents( + hours = 24, + now: Date = new Date(), +): SupervisorEmission[] { + const days = hours / 24; + const events = writer.readRecent(days, now); + const cutoff = now.getTime() - hours * 3_600_000; + return events.filter((e) => { + if (!e.event || !e.ts) return false; + const t = Date.parse(e.ts); + return !Number.isFinite(t) || t >= cutoff; + }); +} + /** * Denylist of clean-exit `likely_cause` values. Anything not in this set — * including future unrecognized values — counts as a crash. Matches the diff --git a/src/core/minions/lock-renewal-tick.ts b/src/core/minions/lock-renewal-tick.ts index f00226342..65e7d54b0 100644 --- a/src/core/minions/lock-renewal-tick.ts +++ b/src/core/minions/lock-renewal-tick.ts @@ -168,8 +168,15 @@ export interface LockRenewalDeps { * and could refresh a lock after the worker already gave it up (two holders). * Absent on engines without a pool (PGLite) and in the legacy tests; the * no-reconnect path behaves exactly as before. + * + * v0.42.12.0 (#1685 GAP B, CODEX impl review #2): accepts an optional ctx so + * the tick can thread the triggering renewLock error. PostgresEngine.reconnect + * classifies it — a CONNECTION_ENDED renewLock failure (the common pooler + * idle-reap) is then audited as `reap_detected`, not `reconnect_other`, so + * `pool_reap_health` actually fires for the #1678 incident pattern. The widened + * (optional-param) signature stays back-compatible with no-arg test mocks. */ - reconnect?: () => Promise; + reconnect?: (ctx?: { error?: unknown }) => Promise; } /** @@ -258,7 +265,9 @@ export async function runLockRenewalTick( const reconnect = deps.reconnect; try { await Promise.race([ - reconnect(), + // Thread the triggering renewLock error (CODEX impl review #2) so the + // engine can classify a CONNECTION_ENDED pooler reap as `reap_detected`. + reconnect({ error: err }), new Promise((_, reject) => { deps.setTimeout( () => reject(new Error(`reconnect timed out after ${state.knobs.callTimeoutMs}ms`)), diff --git a/src/core/minions/protected-names.ts b/src/core/minions/protected-names.ts index 52efa8f0d..8493e5487 100644 --- a/src/core/minions/protected-names.ts +++ b/src/core/minions/protected-names.ts @@ -56,6 +56,13 @@ export const PROTECTED_JOB_NAMES: ReadonlySet = new Set([ // handler must reject MCP submission). Costs user money (optimizer + // judge + rollouts) so PROTECTED is the right posture. 'skillopt', + // v0.42.x (#1685 GAP D, CODEX #1) — extract_atoms backlog drain. Each run + // calls Haiku to extract atoms (~$0.30/source/run), so it MUST NOT be + // submittable by an MCP/OAuth-scoped caller — same posture as the protected + // `extract-takes-from-pages`. Only trusted local callers (the autopilot + // auto-drain branch, an explicit `gbrain jobs submit extract-atoms-drain + // --allow-protected`) can insert it. + 'extract-atoms-drain', ]); /** Check a job name against the protected set. Normalizes whitespace first. */ diff --git a/src/core/minions/worker.ts b/src/core/minions/worker.ts index 24b2c4d01..8cb505c73 100644 --- a/src/core/minions/worker.ts +++ b/src/core/minions/worker.ts @@ -771,13 +771,15 @@ export class MinionWorker extends EventEmitter { // the engine owns a pool that a transaction-mode pooler can reap. Postgres // exposes reconnect(); PGLite (no pooler) doesn't, so the hook is absent // and the tick keeps its legacy no-reconnect behavior. - const engineReconnect = (this.engine as { reconnect?: () => Promise }).reconnect; + const engineReconnect = (this.engine as { reconnect?: (ctx?: { error?: unknown }) => Promise }).reconnect; const renewalDeps: LockRenewalDeps = { renewLock: (id, tok, dur) => this.queue.renewLock(id, tok, dur), audit: lockRenewalAudit, now: Date.now, setTimeout: (cb, ms) => globalThis.setTimeout(cb, ms), - ...(engineReconnect ? { reconnect: () => engineReconnect.call(this.engine) } : {}), + // Forward the tick's classified error (CODEX impl review #2) so a pooler + // reap during lock renewal is audited as reap_detected, not reconnect_other. + ...(engineReconnect ? { reconnect: (ctx?: { error?: unknown }) => engineReconnect.call(this.engine, ctx) } : {}), }; const lockTimer = setInterval(() => { diff --git a/src/core/postgres-engine.ts b/src/core/postgres-engine.ts index c6267ab73..9126909a6 100644 --- a/src/core/postgres-engine.ts +++ b/src/core/postgres-engine.ts @@ -1990,8 +1990,9 @@ export class PostgresEngine implements BrainEngine { // (postgres.js's own connection-replacement covers that case). // Fail-loud per retry.ts contract: a reconnect throw propagates // as the real cause, replacing the symptomatic - // "No database connection" error. - reconnect: () => this.reconnect(), + // "No database connection" error. ctx carries the triggering error so + // reconnect() can classify reap-vs-other for the pool-recovery audit. + reconnect: (ctx) => this.reconnect(ctx), }); } catch (err) { // Distinguish "retries exhausted" (a retryable error that ran out of @@ -4778,15 +4779,45 @@ export class PostgresEngine implements BrainEngine { /** * Reconnect the engine by tearing down the current pool and creating a fresh one. * No-ops if no saved config (module-singleton mode) or if already reconnecting. + * + * v0.42.x (#1685 GAP B): records a pool-recovery audit event so the + * `pool_reap_health` doctor check can answer "reaped N times AND not + * auto-recovering." `ctx.error` (threaded by retry.ts) is classified: a + * CONNECTION_ENDED match is a true pooler reap; anything else (or no error, + * e.g. the supervisor's health-check reconnect) is `reconnect_other`. All + * audit calls are best-effort and never block the reconnect (CODEX #8). */ - async reconnect(): Promise { + async reconnect(ctx?: { error?: unknown }): Promise { if (!this._savedConfig || this._reconnecting) return; this._reconnecting = true; + + let isReap = false; + if (ctx?.error !== undefined) { + try { + const { isConnectionEndedError } = await import('./retry-matcher.ts'); + isReap = isConnectionEndedError(ctx.error); + } catch { /* classification is best-effort */ } + } + try { + const { logPoolRecovery } = await import('./audit/pool-recovery-audit.ts'); + logPoolRecovery(isReap ? 'reap_detected' : 'reconnect_other', ctx?.error); + } catch { /* audit is best-effort */ } + try { // Tear down old pool (best-effort — it may already be dead) try { await this.disconnect(); } catch { /* swallow */ } // Create fresh pool await this.connect(this._savedConfig); + try { + const { logPoolRecovery } = await import('./audit/pool-recovery-audit.ts'); + logPoolRecovery('reconnect_succeeded'); + } catch { /* best-effort */ } + } catch (err) { + try { + const { logPoolRecovery } = await import('./audit/pool-recovery-audit.ts'); + logPoolRecovery('reconnect_failed', err); + } catch { /* best-effort */ } + throw err; } finally { this._reconnecting = false; } diff --git a/src/core/retry-matcher.ts b/src/core/retry-matcher.ts index c7fd0cec7..46afac4d4 100644 --- a/src/core/retry-matcher.ts +++ b/src/core/retry-matcher.ts @@ -115,6 +115,21 @@ export function isRetryableConnError(err: unknown): boolean { return CONN_PATTERNS.some(p => p.test(msg)); } +/** + * issue #1685 (CODEX #8): is this error specifically a POOLER REAP — postgres.js's + * library-level `CONNECTION_ENDED` code (the transaction-mode pooler dropping an + * idle socket between ticks)? Narrower than `isRetryableConnError`, which also + * matches 08xxx SQLSTATEs, network blips, and auth races. Used by + * `PostgresEngine.reconnect()` to label the pool-recovery audit honestly so a + * generic reconnect isn't mis-recorded as a reap. + */ +export function isConnectionEndedError(err: unknown): boolean { + const code = getCode(err); + if (code === 'CONNECTION_ENDED') return true; + const msg = getMessage(err); + return /CONNECTION_ENDED/i.test(msg); +} + /** * Convenience: is this error retryable for ANY reason (connection drop OR * statement timeout)? Backfill uses this — callers that need finer-grained diff --git a/src/core/retry.ts b/src/core/retry.ts index 4b7160bad..767cb4498 100644 --- a/src/core/retry.ts +++ b/src/core/retry.ts @@ -137,10 +137,15 @@ export interface WithRetryOpts { * for hours when DB credentials are bad. * * Engine-level callers (PostgresEngine.batchRetry) inject - * `() => this.reconnect()` which already handles both module and + * `(ctx) => this.reconnect(ctx)` which already handles both module and * instance pools, race-safe via `_reconnecting` guard. + * + * v0.42.x (#1685 CODEX #8): receives the triggering error so the engine can + * classify it (pooler reap vs network/auth) for the pool-recovery audit. The + * arg is optional — back-compat zero-arg callbacks (`() => this.reconnect()`) + * still satisfy the type. */ - reconnect?: () => Promise; + reconnect?: (ctx?: { error?: unknown }) => Promise; } /** @@ -272,7 +277,9 @@ export async function withRetry( // immediate-recovery half of that pair. if (opts.reconnect) { if (signal?.aborted) throw new RetryAbortError(); - await opts.reconnect(); + // Thread the triggering error so the engine can classify it (pooler + // reap vs other) for the pool-recovery audit (#1685 CODEX #8). + await opts.reconnect({ error: err }); } const delay = computeNextDelay(attempt, prevDelay, baseDelay, maxDelay, jitter); prevDelay = delay; diff --git a/test/audit/pool-recovery-audit.test.ts b/test/audit/pool-recovery-audit.test.ts new file mode 100644 index 000000000..c9a820c10 --- /dev/null +++ b/test/audit/pool-recovery-audit.test.ts @@ -0,0 +1,86 @@ +// #1685 GAP B — pool-recovery audit JSONL primitive. +// +// Hermetic: GBRAIN_AUDIT_DIR override via withEnv; fresh tempdir per test. + +import { describe, expect, test, beforeEach, afterEach } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { withEnv } from '../helpers/with-env.ts'; +import { + logPoolRecovery, + readRecentPoolRecoveries, + _poolRecoveryAuditFeatureName, +} from '../../src/core/audit/pool-recovery-audit.ts'; + +let tmpDir: string; +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pool-recovery-audit-')); +}); +afterEach(() => { + try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch { /* best-effort */ } +}); + +describe('logPoolRecovery + readRecentPoolRecoveries', () => { + test('round-trips a reap → recovered pair into the right counters', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + logPoolRecovery('reap_detected', Object.assign(new Error('write CONNECTION_ENDED'), { code: 'CONNECTION_ENDED' })); + logPoolRecovery('reconnect_succeeded'); + const r = readRecentPoolRecoveries(1); + expect(r.reaps).toBe(1); + expect(r.recoveries).toBe(1); + expect(r.failures).toBe(0); + expect(r.others).toBe(0); + expect(r.events).toHaveLength(2); + }); + }); + + test('counts a reap that failed to recover (the not-auto-recovering signal)', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + logPoolRecovery('reap_detected'); + logPoolRecovery('reconnect_failed', new Error('EHOSTUNREACH')); + const r = readRecentPoolRecoveries(1); + expect(r.reaps).toBe(1); + expect(r.failures).toBe(1); + expect(r.recoveries).toBe(0); + }); + }); + + test('reconnect_other is tracked separately from reaps', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + logPoolRecovery('reconnect_other', new Error('network blip')); + logPoolRecovery('reconnect_succeeded'); + const r = readRecentPoolRecoveries(1); + expect(r.reaps).toBe(0); + expect(r.others).toBe(1); + expect(r.recoveries).toBe(1); + }); + }); + + test('redacts connection info from the error summary', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + logPoolRecovery( + 'reconnect_failed', + new Error('could not connect to postgres://user:secret@db.example.com:5432/app (192.168.1.42)'), + ); + const r = readRecentPoolRecoveries(1); + const summary = r.events[0].error_summary ?? ''; + expect(summary).not.toContain('secret'); + expect(summary).not.toContain('192.168.1.42'); + expect(summary).toContain(' { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + const r = readRecentPoolRecoveries(1); + expect(r.reaps).toBe(0); + expect(r.events).toEqual([]); + expect(r.most_recent_ts).toBeNull(); + }); + }); + + test('stable feature name', () => { + expect(_poolRecoveryAuditFeatureName()).toBe('pool-recovery'); + }); +}); diff --git a/test/autopilot-auto-drain-wiring.test.ts b/test/autopilot-auto-drain-wiring.test.ts new file mode 100644 index 000000000..c8dd28c21 --- /dev/null +++ b/test/autopilot-auto-drain-wiring.test.ts @@ -0,0 +1,54 @@ +/** + * #1685 GAP D — autopilot auto-drain wiring regression guards. + * + * The submission is inline in the autopilot tick body, so these are + * source-shape assertions (the proven `autopilot-*-wiring.test.ts` pattern). + * The load-bearing one is CODEX #2: the idempotency key MUST carry a time slot, + * else queue.add returns the first completed job forever and the source never + * drains again. + */ +import { describe, test, expect } from 'bun:test'; +import { readFileSync } from 'fs'; +import { join } from 'path'; + +const SRC = readFileSync(join(import.meta.dir, '../src/commands/autopilot.ts'), 'utf8'); + +describe('autopilot auto-drain wiring', () => { + test('CODEX #2: idempotency key includes a UTC-day time slot (not static)', () => { + expect(SRC).toContain('autopilot-extract-atoms-drain:${src.id}:${utcDay}'); + // A static key would be the regression — guard against the bare form. + expect(SRC).not.toContain('`autopilot-extract-atoms-drain:${src.id}`'); + }); + + test('CODEX #1: submits with allowProtectedSubmit', () => { + expect(SRC).toMatch(/extract-atoms-drain[\s\S]{0,800}allowProtectedSubmit: true/); + }); + + test('CODEX #3: enumerates sources and counts backlog per source', () => { + expect(SRC).toContain('loadAllSources(engine)'); + expect(SRC).toContain('countExtractAtomsBacklog(engine, src.id)'); + }); + + test('gates on pack NOT declaring extract_atoms (the silent-backlog condition)', () => { + expect(SRC).toContain("packDeclaresPhase(engine, 'extract_atoms')"); + }); + + test('gates on the enabled flag and a daily spend cap (DECISION 3C)', () => { + expect(SRC).toContain('autopilot.auto_drain.enabled'); + expect(SRC).toContain('autopilot.auto_drain.max_usd_per_day'); + expect(SRC).toContain('maxJobsToday'); + }); + + test('is Postgres-gated (PGLite has no worker surface)', () => { + expect(SRC).toMatch(/engine\.kind === 'postgres'[\s\S]{0,400}auto_drain/); + }); + + test('CODEX impl #4: no maxWaiting (it coalesces by name+queue, not source)', () => { + // maxWaiting would return source A's waiting job for source B's submit, + // never queuing B and over-counting the cap. The per-source idempotency key + // is the dedup; a pre-check on it avoids counting idempotency-hit re-submits. + const drainBlock = SRC.slice(SRC.indexOf("'extract-atoms-drain'")); + expect(drainBlock.slice(0, 900)).not.toContain('maxWaiting'); + expect(SRC).toContain('WHERE idempotency_key = $1 LIMIT 1'); + }); +}); diff --git a/test/connection-resilience.test.ts b/test/connection-resilience.test.ts index 0e991e3ad..ee35ea62d 100644 --- a/test/connection-resilience.test.ts +++ b/test/connection-resilience.test.ts @@ -310,7 +310,10 @@ describe('Eng-review D3 — executeRaw has no per-call retry wrapper', () => { it('PostgresEngine.reconnect() still exists for supervisor-driven recovery', () => { const src = readFileSync(resolve('src/core/postgres-engine.ts'), 'utf-8'); - expect(src).toContain('async reconnect()'); + // v0.42.10.0 (#1685 GAP B): reconnect() gained an optional ctx param so it + // can classify the triggering error for the pool-recovery audit. Match the + // prefix so both `reconnect()` and `reconnect(ctx?)` satisfy the contract. + expect(src).toContain('async reconnect('); expect(src).toContain('await this.disconnect()'); }); diff --git a/test/doctor-cause-rank.test.ts b/test/doctor-cause-rank.test.ts new file mode 100644 index 000000000..18dc05dac --- /dev/null +++ b/test/doctor-cause-rank.test.ts @@ -0,0 +1,87 @@ +/** + * #1685 GAP C — cause-ranked doctor issues. Pure unit tests. + * + * Covers: fail-before-warn + root-before-symptom ordering, evidence-gated + * downstream_of (NEVER from co-occurrence alone — CODEX #9), fix-hint + * preference, and the DECISION 4A drift guard (every cause-graph name still + * exists in doctor-categories). + */ +import { describe, it, expect } from 'bun:test'; +import { + rankIssues, + ROOT_CAUSE_CHECKS, + SYMPTOM_CHECKS, + CAUSE_GRAPH_NAMES, + allKnownCheckNames, + type RankableCheck, +} from '../src/core/doctor-cause-rank.ts'; + +const ok = (name: string): RankableCheck => ({ name, status: 'ok', message: 'fine' }); +const warn = (name: string, msg = 'warned'): RankableCheck => ({ name, status: 'warn', message: msg }); +const fail = (name: string, msg = 'failed'): RankableCheck => ({ name, status: 'fail', message: msg }); + +describe('rankIssues', () => { + it('drops ok checks, returns [] when everything is healthy', () => { + expect(rankIssues([ok('connection'), ok('queue_health')])).toEqual([]); + }); + + it('orders fail before warn', () => { + const out = rankIssues([warn('stale_locks'), fail('schema_version')]); + expect(out.map((i) => i.name)).toEqual(['schema_version', 'stale_locks']); + }); + + it('orders root before symptom within the same status', () => { + // queue_health (symptom) + worker_oom_loop (root), both fail. + const out = rankIssues([fail('queue_health'), fail('worker_oom_loop')]); + expect(out.map((i) => i.name)).toEqual(['worker_oom_loop', 'queue_health']); + expect(out[0].tier).toBe('root'); + expect(out[1].tier).toBe('symptom'); + }); + + it('tags downstream_of ONLY when the named root is itself failing', () => { + const both = rankIssues([fail('worker_oom_loop'), fail('queue_health')]); + const q1 = both.find((i) => i.name === 'queue_health')!; + expect(q1.downstream_of).toBe('worker_oom_loop'); + }); + + it('does NOT tag downstream_of when the root is absent (no co-occurrence guess)', () => { + const out = rankIssues([fail('queue_health')]); // worker_oom_loop not failing + const q = out.find((i) => i.name === 'queue_health')!; + expect(q.downstream_of).toBeUndefined(); + }); + + it('does NOT tag downstream_of from a generic root×symptom cartesian', () => { + // schema_version is a root and stale_locks is a symptom, but there is NO + // declared causal edge between them — co-occurrence must not invent one. + const out = rankIssues([fail('schema_version'), fail('stale_locks')]); + const s = out.find((i) => i.name === 'stale_locks')!; + expect(s.downstream_of).toBeUndefined(); + }); + + it('uses details.fix_hint when present, else the message', () => { + const out = rankIssues([ + { name: 'worker_oom_loop', status: 'fail', message: 'long message', details: { fix_hint: 'raise --max-rss' } }, + warn('orphan_ratio', 'too many orphans'), + ]); + expect(out.find((i) => i.name === 'worker_oom_loop')!.fix).toBe('raise --max-rss'); + expect(out.find((i) => i.name === 'orphan_ratio')!.fix).toBe('too many orphans'); + }); + + it('is deterministic (name tiebreak) for same status+tier', () => { + const out = rankIssues([warn('zeta_unknown'), warn('alpha_unknown')]); + expect(out.map((i) => i.name)).toEqual(['alpha_unknown', 'zeta_unknown']); + }); +}); + +describe('DECISION 4A drift guard', () => { + it('every cause-graph name exists in doctor-categories known names', () => { + const known = allKnownCheckNames(); + const missing = [...CAUSE_GRAPH_NAMES].filter((n) => !known.has(n)); + expect(missing).toEqual([]); + }); + + it('root and symptom sets are disjoint', () => { + const overlap = [...ROOT_CAUSE_CHECKS].filter((n) => SYMPTOM_CHECKS.has(n)); + expect(overlap).toEqual([]); + }); +}); diff --git a/test/doctor-pool-reap-health.test.ts b/test/doctor-pool-reap-health.test.ts new file mode 100644 index 000000000..a57a60131 --- /dev/null +++ b/test/doctor-pool-reap-health.test.ts @@ -0,0 +1,77 @@ +// #1685 GAP B — pool_reap_health doctor check. +// +// computePoolReapHealthCheck only touches engine.kind + the pool-recovery audit +// (filesystem), so a minimal `{ kind: 'postgres' }` stub drives it hermetically. + +import { describe, expect, test, beforeEach, afterEach } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { withEnv } from './helpers/with-env.ts'; +import { logPoolRecovery } from '../src/core/audit/pool-recovery-audit.ts'; +import { computePoolReapHealthCheck } from '../src/commands/doctor.ts'; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const pg = { kind: 'postgres' } as any; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const pglite = { kind: 'pglite' } as any; + +let tmpDir: string; +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pool-reap-health-')); +}); +afterEach(() => { + try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch { /* best-effort */ } +}); + +describe('computePoolReapHealthCheck', () => { + test('null on PGLite (no pool) and on null engine', async () => { + expect(await computePoolReapHealthCheck(pglite)).toBeNull(); + expect(await computePoolReapHealthCheck(null)).toBeNull(); + }); + + test('fail when reconnect failed (reconnect is throwing)', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + logPoolRecovery('reap_detected'); + logPoolRecovery('reconnect_failed', new Error('EHOSTUNREACH')); + const c = await computePoolReapHealthCheck(pg); + expect(c?.status).toBe('fail'); + expect(c?.message).toContain('reconnect is throwing'); + expect(c?.name).toBe('pool_reap_health'); + }); + }); + + // CODEX impl review #3: the fail trigger is the reconnect FAILURES themselves + // (reconnect throwing is the real, actionable problem), NOT a fabricated + // reap→failure causal link. A reconnect_failed with zero reaps still fails. + test('fail on reconnect failure even with zero reaps (no false causality)', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + logPoolRecovery('reconnect_failed', new Error('password authentication failed')); + const c = await computePoolReapHealthCheck(pg); + expect(c?.status).toBe('fail'); + expect(c?.message).toContain('0 pooler reap(s) detected'); + expect(c?.message).not.toContain('not auto-recovering'); + }); + }); + + test('warn on pooler thrash (>=10 reaps all recovered)', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + for (let i = 0; i < 12; i++) { + logPoolRecovery('reap_detected'); + logPoolRecovery('reconnect_succeeded'); + } + const c = await computePoolReapHealthCheck(pg); + expect(c?.status).toBe('warn'); + expect(c?.message).toContain('12×'); + }); + }); + + test('null (quiet) when a few reaps all recovered', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + logPoolRecovery('reap_detected'); + logPoolRecovery('reconnect_succeeded'); + const c = await computePoolReapHealthCheck(pg); + expect(c).toBeNull(); + }); + }); +}); diff --git a/test/doctor-worker-oom-loop.test.ts b/test/doctor-worker-oom-loop.test.ts new file mode 100644 index 000000000..31754515d --- /dev/null +++ b/test/doctor-worker-oom-loop.test.ts @@ -0,0 +1,101 @@ +// #1685 GAP A — worker_oom_loop doctor check. +// +// Hermetic: writes synthetic supervisor audit JSONL into a GBRAIN_AUDIT_DIR +// tempdir and drives computeWorkerOomLoopCheck with engine=null (supervised +// half + cap logic + thresholds; the minion_jobs bare-worker branch is +// Postgres-only and mirrors the queue_health subcheck-3 query covered by E2E). +// Also pins the cross-week supervisor reader (CODEX #7). + +import { describe, expect, test, beforeEach, afterEach } from 'bun:test'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { withEnv } from './helpers/with-env.ts'; +import { computeWorkerOomLoopCheck } from '../src/commands/doctor.ts'; +import { + computeSupervisorAuditFilename, + readRecentSupervisorEvents, +} from '../src/core/minions/handlers/supervisor-audit.ts'; + +let tmpDir: string; +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'worker-oom-loop-')); +}); +afterEach(() => { + try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch { /* best-effort */ } +}); + +function writeSupervisorRows(rows: object[], fileDate = new Date()): void { + const file = path.join(tmpDir, computeSupervisorAuditFilename(fileDate)); + fs.writeFileSync(file, rows.map((r) => JSON.stringify(r)).join('\n') + '\n', 'utf8'); +} + +const nowIso = () => new Date().toISOString(); +const rssKill = () => ({ event: 'worker_exited', ts: nowIso(), likely_cause: 'rss_watchdog', code: 12 }); +const breaker = (cap: number) => ({ event: 'health_warn', ts: nowIso(), reason: 'rss_watchdog_loop', max_rss_mb: cap }); + +describe('computeWorkerOomLoopCheck', () => { + test('fail with breaker-stamped cap when the breaker tripped', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + writeSupervisorRows([rssKill(), rssKill(), rssKill(), rssKill(), rssKill(), rssKill(), breaker(2048)]); + const c = await computeWorkerOomLoopCheck(null); + expect(c?.status).toBe('fail'); + expect(c?.name).toBe('worker_oom_loop'); + expect(c?.message).toContain('cap=2048MB'); + expect(c?.message).toContain('raise --max-rss'); + expect(c?.details?.oom_kills).toBe(6); + expect(c?.details?.supervisor_kills).toBe(6); + expect(c?.details?.bare_worker_kills).toBe(0); + expect(c?.details?.cap_source).toBe('breaker'); + expect(c?.details?.breaker_tripped).toBe(true); + }); + }); + + test('warn with auto-sized cap fallback when no breaker event stamped a cap (CODEX #6)', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + writeSupervisorRows([rssKill(), rssKill()]); + const c = await computeWorkerOomLoopCheck(null); + expect(c?.status).toBe('warn'); + expect(c?.details?.cap_source).toBe('default'); + expect(c?.message).toContain('auto-sized default'); + expect(c?.details?.oom_kills).toBe(2); + }); + }); + + test('fail at >=5 kills even without a breaker event', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + writeSupervisorRows([rssKill(), rssKill(), rssKill(), rssKill(), rssKill()]); + const c = await computeWorkerOomLoopCheck(null); + expect(c?.status).toBe('fail'); + expect(c?.details?.breaker_tripped).toBe(false); + }); + }); + + test('null when the worker never OOM-looped', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + writeSupervisorRows([{ event: 'started', ts: nowIso() }]); + expect(await computeWorkerOomLoopCheck(null)).toBeNull(); + }); + }); + + test('null on an empty audit dir', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + expect(await computeWorkerOomLoopCheck(null)).toBeNull(); + }); + }); +}); + +describe('readRecentSupervisorEvents — cross-week (CODEX #7)', () => { + test('reads a within-24h event from the PREVIOUS ISO-week file', async () => { + await withEnv({ GBRAIN_AUDIT_DIR: tmpDir }, async () => { + // Event timestamped within the 24h window but written into last week's + // file (the Monday-reads-Sunday case). The single-file reader would miss + // it; the cross-week reader must find it. + const prevWeekFileDate = new Date(Date.now() - 7 * 86400000); + writeSupervisorRows([rssKill()], prevWeekFileDate); + const events = readRecentSupervisorEvents(24); + expect(events.length).toBe(1); + expect(events[0].event).toBe('worker_exited'); + }); + }); +}); diff --git a/test/dream-cli-flags.test.ts b/test/dream-cli-flags.test.ts index 3663996c2..1ffa759ce 100644 --- a/test/dream-cli-flags.test.ts +++ b/test/dream-cli-flags.test.ts @@ -119,9 +119,15 @@ describe('dream CLI flag wiring', () => { expect(dreamSrc).toContain('--drain currently supports only --phase extract_atoms'); }); - test('drain holds the same cycle lock id (contends with the routine cycle)', () => { - expect(dreamSrc).toContain('cycleLockIdFor(resolvedSourceId)'); - expect(dreamSrc).toContain('withRefreshingLock'); + test('drain routes through the shared helper with the resolved source (5A)', () => { + // v0.42.10.0 (#1685 GAP D / 5A): the lock+batch+count wiring moved into + // runExtractAtomsDrainForSource so the CLI, the Minion handler, and + // autopilot share ONE drain path. dream threads resolvedSourceId so the + // helper picks cycleLockIdFor(resolvedSourceId) — the same lock the routine + // cycle holds for that source. The lock-id contract is now pinned in + // test/extract-atoms-drain.test.ts ("shared wiring helper holds the cycle lock"). + expect(dreamSrc).toContain('runExtractAtomsDrainForSource'); + expect(dreamSrc).toContain('sourceId: resolvedSourceId'); }); test('drain reports remaining + exits non-zero when incomplete', () => { diff --git a/test/extract-atoms-drain-handler.test.ts b/test/extract-atoms-drain-handler.test.ts new file mode 100644 index 000000000..ac10a9dba --- /dev/null +++ b/test/extract-atoms-drain-handler.test.ts @@ -0,0 +1,52 @@ +/** + * #1685 GAP D — extract-atoms-drain Minion handler: registration + protected + * gate. Canonical PGLite block (CLAUDE.md R3+R4). + */ +import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { MinionQueue } from '../src/core/minions/queue.ts'; +import { MinionWorker } from '../src/core/minions/worker.ts'; +import { registerBuiltinHandlers } from '../src/commands/jobs.ts'; + +let engine: PGLiteEngine; +let queue: MinionQueue; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({ database_url: '' }); + await engine.initSchema(); + queue = new MinionQueue(engine); +}); + +afterAll(async () => { + await engine.disconnect(); +}); + +beforeEach(async () => { + await engine.executeRaw('DELETE FROM minion_jobs'); +}); + +describe('extract-atoms-drain handler', () => { + test('registerBuiltinHandlers registers the handler', async () => { + const worker = new MinionWorker(engine); + await registerBuiltinHandlers(worker, engine); + expect(worker.registeredNames).toContain('extract-atoms-drain'); + }); + + test('queue.add rejects an untrusted submission (PROTECTED, CODEX #1)', async () => { + await expect(queue.add('extract-atoms-drain', { sourceId: 'default' })).rejects.toThrow( + /protected job name/i, + ); + }); + + test('queue.add accepts a trusted submission (allowProtectedSubmit)', async () => { + const job = await queue.add( + 'extract-atoms-drain', + { sourceId: 'default', window: 120 }, + { queue: 'default' }, + { allowProtectedSubmit: true }, + ); + expect(job.id).toBeGreaterThan(0); + expect(job.name).toBe('extract-atoms-drain'); + }); +}); diff --git a/test/extract-atoms-drain.test.ts b/test/extract-atoms-drain.test.ts index cf9ab7e8a..cb7dae821 100644 --- a/test/extract-atoms-drain.test.ts +++ b/test/extract-atoms-drain.test.ts @@ -9,10 +9,13 @@ */ import { describe, it, expect } from 'bun:test'; +import { readFileSync } from 'fs'; +import { join } from 'path'; import { runExtractAtomsDrain, type ExtractAtomsDrainDeps, } from '../src/core/cycle/extract-atoms-drain.ts'; +import { isProtectedJobName, PROTECTED_JOB_NAMES } from '../src/core/minions/protected-names.ts'; function seq(values: Array): () => Promise { let i = 0; @@ -106,3 +109,28 @@ describe('runExtractAtomsDrain (issue #1678)', () => { expect(batches).toBe(4); }); }); + +// #1685 GAP D (CODEX #1) — the auto-drain Minion job burns Haiku, so it must be +// PROTECTED: no MCP/OAuth-scoped caller can submit it; only trusted local +// callers (autopilot, explicit CLI with --allow-protected) can. +describe('extract-atoms-drain protected-name membership', () => { + it('extract-atoms-drain is PROTECTED', () => { + expect(isProtectedJobName('extract-atoms-drain')).toBe(true); + expect(PROTECTED_JOB_NAMES.has('extract-atoms-drain')).toBe(true); + }); +}); + +// #1685 GAP D / 5A — the shared wiring helper is the single drain path. The +// "drain holds the same cycle lock id as the routine cycle" contract (moved out +// of dream.ts in the 5A refactor) lives here now. +describe('shared wiring helper holds the cycle lock (5A)', () => { + const src = readFileSync( + join(import.meta.dir, '../src/core/cycle/extract-atoms-drain.ts'), + 'utf8', + ); + it('runExtractAtomsDrainForSource uses cycleLockIdFor(opts.sourceId) + withRefreshingLock', () => { + expect(src).toContain('runExtractAtomsDrainForSource'); + expect(src).toContain('cycleLockIdFor(opts.sourceId)'); + expect(src).toContain('withRefreshingLock(engine, lockId'); + }); +}); diff --git a/test/worker-lock-renewal.test.ts b/test/worker-lock-renewal.test.ts index 63c49c12b..b13daa5ae 100644 --- a/test/worker-lock-renewal.test.ts +++ b/test/worker-lock-renewal.test.ts @@ -505,6 +505,26 @@ describe('runLockRenewalTick: reconnect-once dep (issue #1678)', () => { expect(state.consecutiveFailures).toBe(1); }); + // CODEX impl review #2 (#1685 GAP B): the tick must thread the triggering + // renewLock error to reconnect, so PostgresEngine.reconnect can classify a + // CONNECTION_ENDED pooler reap as reap_detected (not reconnect_other) for + // pool_reap_health. Pin the threading. + test('reconnect receives the triggering renewLock error', async () => { + const audit = freshAudit(); + const renewErr = new Error('write CONNECTION_ENDED'); + let received: unknown = 'NOT_CALLED'; + const deps: LockRenewalDeps = { + renewLock: async () => { throw renewErr; }, + audit: audit.sink, + now: () => 1000, + setTimeout: makeFakeTimer().setTimeout, + reconnect: async (ctx?: { error?: unknown }) => { received = ctx?.error; }, + }; + const result = await runLockRenewalTick(deps, makeState({ lastSuccessfulRenewalAt: 0 })); + expect(result).toEqual({ kind: 'ok' }); + expect(received).toBe(renewErr); + }); + test('a reconnect throw is swallowed — tick still returns ok (no unhandledRejection class)', async () => { const audit = freshAudit(); const deps: LockRenewalDeps = {