diff --git a/CHANGELOG.md b/CHANGELOG.md index 3638b95ab..7ea3a1a93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,158 @@ All notable changes to GBrain will be documented in this file. +## [0.42.5.0] - 2026-06-01 + +**If your background worker has been dying every few minutes and the logs keep +blaming the database, this release explains why and stops it. The real cause was +almost never the database. It was a memory cap set way too low, killing +legitimate work and leaving behind a trail of connection errors that looked like +the problem but were only the symptom.** + +Here's what was happening. The worker has a safety valve that drains it when +memory gets too high, meant to catch a runaway leak. The default cap was 2GB. But +a brain doing embeddings legitimately needs around 10GB of working memory, so the +valve fired on every heavy cycle, drained the worker mid-job, the pooler then +reaped the half-open database socket, and every call after that threw "No +database connection." One operator's worker exited 400+ times in 24 hours. The +single line that would have explained it scrolled by once per cycle, buried under +hundreds of database errors. It took hours to find. + +Three things were wrong, and all three are fixed: + +1. **The memory-cap drain looked exactly like a clean shutdown**, so nothing + counted it or alerted on it. Now it exits with its own distinct code, shows up + in `gbrain doctor` and supervisor logs as `rss_watchdog`, and after a few + loops in a window the supervisor prints a loud "worker OOM-looping: raise + --max-rss" line and backs off instead of hot-looping. +2. **The 2GB default was a footgun.** It now auto-sizes from your machine's RAM + (half of it, clamped to 4-16GB), and it reads your container/cgroup limit so a + small container doesn't get a cap set above its real ceiling. Pass `--max-rss` + to override. You'll see the resolved number on worker startup. +3. **The database errors that followed the drain had no recovery path** in the + job-lock code, so one reaped socket cascaded into a dead worker. Those hot + paths now reconnect and recover, and `CONNECTION_ENDED` (the pooler's + socket-reap error) is finally recognized as retryable everywhere. +4. **The dream cycle could kill its own database connection.** While tracing the + same bug class, we found the `lint` phase created a second, competing + database connection to read four config values and then closed it — which + tore down the shared connection the rest of the cycle was using. On a + Postgres brain with a configured connection string, `gbrain dream` could die + mid-cycle with the same misleading "no database connection" error right after + linting. The lint phase now reuses the cycle's existing connection. + +Separately, this release fixes a long-standing invisible backlog: on a brain +whose schema pack doesn't run the `extract_atoms` lens phase, that phase silently +never ran in the nightly cycle and pages piled up forever with zero signal. +`gbrain doctor` now counts that backlog and tells you the exact command to drain +it, and there's a new first-class drain mode for grinding it down on demand. + +### How to take advantage + +Most of this is automatic on upgrade. To use the new pieces: + +- **Diagnose a watchdog loop fast:** `gbrain doctor` and `gbrain jobs supervisor + status` now break crashes out by cause. An `rss_watchdog` count means "raise + the cap," not "debug the database." +- **Set the memory cap explicitly if you want:** `gbrain jobs work --max-rss + 16384` (megabytes; `--max-rss 0` disables the watchdog). The worker prints the + resolved cap and where it came from on startup. +- **Drain an atom backlog on demand:** `gbrain dream --phase extract_atoms + --drain --window 120 --json`. It holds the cycle lock once, processes batches + until the backlog empties or the window elapses, reports `{extracted, + remaining}`, and exits non-zero while work remains so a cron loop knows to run + again. `--dry-run` previews the count without doing work. +- **See the backlog:** `gbrain doctor --json` includes an `extract_atoms_backlog` + check that warns (with the drain command) when eligible pages pile up under a + pack that doesn't run the phase. + +### To take advantage of v0.42.5.0 + +`gbrain upgrade` applies everything. No schema migration in this release. If +`gbrain doctor` still flags a watchdog loop after upgrade: + +1. Check the cause breakdown: `gbrain doctor --json` (look for + `rss_watchdog` under the supervisor check). +2. Raise the cap to fit your embed working set: + ```bash + gbrain jobs work --max-rss 16384 # or pass via your supervisor/launchd unit + ``` +3. If `extract_atoms_backlog` warns, drain it: + ```bash + gbrain dream --phase extract_atoms --drain --window 120 + ``` +4. If anything still looks wrong, file an issue with `gbrain doctor` output: + https://github.com/garrytan/gbrain/issues + +### Itemized changes + +- **Self-identifying watchdog exit.** New `WORKER_EXIT_RSS_WATCHDOG` exit code + (`src/core/minions/worker-exit-codes.ts`). The worker sets a flag on a memory + drain; the CLI (`gbrain jobs work`) exits with the distinct code so the + supervisor classifies it as `likely_cause=rss_watchdog` instead of a silent + clean exit. `src/core/minions/child-worker-supervisor.ts` tracks watchdog + exits in their own sliding window (independent of `crashCount`, so the >5-min + stable-run reset can't defeat the breaker) and emits a loud `rss_watchdog_loop` + `health_warn` plus a backoff once the window budget is exceeded. + `supervisor-audit.ts` gains an `rss_watchdog` crash bucket. +- **Pre-kill soft warning + diagnostics.** The watchdog logs peak RSS and the + in-flight job kind, and warns once at 80% of the cap before the drain so you get + a heads-up rather than a silent death. +- **Cgroup-aware auto-sized default.** `src/core/minions/rss-default.ts`: + `resolveDefaultMaxRssMb()` = `clamp(0.5 × min(cgroupLimit, totalRAM), 4096, + 16384)` MB. Replaces the flat `2048` default in `gbrain jobs work`, `gbrain jobs + supervisor`, the autopilot-managed worker, and `MinionSupervisor`. Reads cgroup + v2 `memory.max` and v1 `memory.limit_in_bytes` so the cap stays below the real + process ceiling (graceful drain beats the kernel OOM-killer). +- **Cycle lint phase reuses the shared engine (issue #1678, same disconnect + family).** `resolveLintContentSanity` in `src/commands/lint.ts` created a + module-style engine (`createEngine` without `poolSize` wraps the `db.ts` + singleton) for the content-sanity DB-plane config lift, then `disconnect()`ed + it — cascading to `db.disconnect()` and nulling the singleton the cycle's lint + phase shares. Every later phase then threw `connect() has not been called` + (most visibly `conversation_facts_backfill`'s `getConfig`). `LintOpts` gains an + optional `engine`; `runPhaseLint` (cycle) and the `lint` / `lint-fix` Minion + handlers pass their live engine so lint reuses it with zero connection churn. + Standalone `gbrain lint` (CLI_ONLY, no shared engine) keeps the create-own + path. Pinned by `test/lint-shared-engine.test.ts` (engine reused, never + disconnected) and the now-passing `test/e2e/cycle.test.ts` + + `test/e2e/dream.test.ts`. The E2E phase-count assertion was also made + non-brittle (asserts `ALL_PHASES.length` instead of a hardcoded number that + had drifted stale). +- **Lock-path self-heal.** `src/core/retry-matcher.ts` now classifies + `CONNECTION_ENDED` (postgres.js's socket-reap code) as retryable via both code + and message. `PostgresEngine`'s `sql` getter no longer falls through to the + never-connected module singleton when an instance pool's connection went away; + it throws a clear, retryable error so the retry path rebuilds the pool. + `promoteDelayed` reconnects and retries on a reaped socket; `claim` recovers on + the next poll tick instead of crashing the worker (a blind retry could + double-claim a job); the lock-renewal tick rebuilds the pool once, bounded by + its own timeout, rather than racing a background retry against the renewal + deadline. +- **Visible lens-phase backlog.** New `extract_atoms_backlog` check in `gbrain + doctor` counts eligible-but-unextracted pages and warns (with the `--drain` + command) when the active pack doesn't run the phase. The nightly cycle's + pack-gated skip now carries a `pack_gated: true` marker so it's greppable. +- **First-class bounded drain.** `gbrain dream --phase extract_atoms --drain + [--window ]` holds the cycle lock once (the same lock id the routine + cycle uses, so they genuinely take turns), rediscovers eligibility each batch + (no stale-content extraction), reports `{extracted, skipped, remaining}`, and + exits non-zero while the backlog remains. + +### For contributors + +- New CI-guarded audit site `minion-lock` in `BATCH_AUDIT_SITES` + (`src/core/retry.ts`). +- New modules: `worker-exit-codes.ts`, `rss-default.ts`, `cycle/extract-atoms-drain.ts`. +- `packDeclaresPhase` and `countExtractAtomsBacklog` are now exported for the + doctor check. +- ~70 new test cases across `test/rss-default.test.ts`, + `test/worker-watchdog-trigger.test.ts`, `test/child-worker-supervisor.test.ts`, + `test/supervisor-audit.test.ts`, `test/retry-matcher.test.ts`, + `test/worker-lock-renewal.test.ts`, `test/postgres-engine-getter-selfheal.test.ts`, + `test/queue-lock-retry.test.ts`, `test/doctor-extract-atoms-backlog.test.ts`, + `test/extract-atoms-drain.test.ts`, and the supervisor/dream flag suites. + ## [0.42.4.0] - 2026-06-01 **`gbrain think` stops writing blank pages, and a bad `--model` now fails loud instead of going quiet.** @@ -617,6 +769,7 @@ Every originally-deferred follow-up is included: - **Issue #1481 closed** — supersedes the original proposal with the decisions captured in plan `~/.claude/plans/system-instruction-you-are-working-drifting-falcon.md`. + ## [0.41.38.0] - 2026-05-30 **Two fixes for Supabase brains with a code source. `gbrain code-callers` and diff --git a/CLAUDE.md b/CLAUDE.md index 0ce8f1529..0979d1952 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -175,7 +175,10 @@ strict behavior when unset. - `src/core/audit/batch-retry-audit.ts` (v0.41.19.0, extended v0.41.26.1) — JSONL audit primitive for batch-retry events. Built on `audit-writer.ts` cathedral. Schema: `{ts, site, batch_size, attempt, outcome: 'success' | 'exhausted', delay_ms, error_message_summary, error_code?}`. Privacy posture: NEVER logs slugs / page IDs / content (mirrors `shell-audit.ts` from v0.20+). `logBatchRetry` fires per successful retry recovery; `logBatchExhausted` fires when retries exhaust and rows are lost. `readRecentBatchRetryEvents(hours=24)` returns `{events, corrupted_lines, files_scanned, files_unreadable}` — corruption + permission errors surface to doctor, not silently swallowed (codex H-9). `pruneOldBatchRetryAuditFiles(daysToKeep=30)` deletes old files (codex H-8 — implements the v0.41 plan's "30-day pruning convention" for real). Called from `gbrain dream --phase purge`. File: `~/.gbrain/audit/batch-retry-YYYY-Www.jsonl` (honors `GBRAIN_AUDIT_DIR`). **v0.41.26.1 privacy backfill:** `summarizeError` now routes error messages through the shared `redactConnectionInfo` helper from `src/core/audit/redact-connection-info.ts` BEFORE truncation, so DSNs / hostnames / credentials / IPv4 octets can't leak from a Postgres connection-failure error into operator-shared JSONL dumps. Same risk class as lock-renewal-audit; closed in the same wave. Pinned by `test/audit/batch-retry-audit.test.ts` (12 cases) + new `test/audit/batch-retry-redaction.test.ts` (3 privacy regressions). - `src/core/audit/lock-renewal-audit.ts` (v0.41.26.1) — JSONL audit primitive for per-job lock-renewal faults. Sibling of `batch-retry-audit.ts`; built on `audit-writer.ts`. Four outcomes: `failure` (single renewLock throw, counter incremented), `success_after_failure` (recovery; emits the recovery count), `gave_up` (time-based deadline exceeded; abort fired), `executeJob_rejected` (the SECOND unhandledRejection vector from D7 — the stored `executeJob(...).finally(...)` promise itself rejected, e.g. failJob threw during the same DB outage). Schema: `{ts, job_id, job_name, attempt?, outcome, error_message_summary?, error_code?}`. Privacy: NEVER logs `lock_token` or `job.data`. Error summaries route through `redactConnectionInfo` BEFORE truncation. Defense-in-depth: every audit call inside the lock-renewal tick's catch block is wrapped in its own inner try/catch so a misbehaving audit-writer can't re-introduce the unhandledRejection bug class via a new surface. `readRecentLockRenewalEvents(hours=24)` walks current + previous ISO week with corrupted-line tolerance. `pruneOldLockRenewalAuditFiles(daysToKeep=30)` is ready for future dream-cycle purge wiring (filed as TODO-LR-3). File: `~/.gbrain/audit/lock-renewal-YYYY-Www.jsonl`. Operator UX is `tail -F` until the doctor check lands (TODO-LR-2). Pinned by `test/audit/lock-renewal-audit.test.ts` (11 cases). - `src/core/audit/redact-connection-info.ts` (v0.41.26.1) — Shared pure helper. `redactConnectionInfo(text: string): string` strips Postgres connection info before any audit JSONL write: `postgres://`/`postgresql://` URLs, `host=foo`, `user=foo`, `password=foo`, `pwd=foo`, IPv4 octets. Each match becomes ``. Negative-lookbehind/lookahead `[\w.@-]` on the IPv4 pattern defeats version-string false positives like `v3.1.4.0` or `tree-sitter@0.26.3.1` while still matching real IPs in PG errors (`(192.168.1.42)`). Order-sensitive pattern set: URL forms first so substrings inside URLs don't get double-redacted. Idempotent (running twice produces same output), pure (no I/O), hot-path-safe (regex compiled at module load). Wired into BOTH `lock-renewal-audit.ts` (new) AND `batch-retry-audit.ts` (privacy backfill — same risk class). Risk model documented in module header: Postgres errors during connection failures embed DSNs / credentials / hostnames into error messages; operators routinely paste audit dumps into GitHub issues / Slack to debug; the audit channel must be safe to share by construction. Known limitations: bare-quoted hostnames (`at "db.example.com"`) and bare-quoted usernames (`for user "postgres.foo"`) are NOT caught — the IP in those PG error shapes is the highest-value leak and IS caught. Quoted-string patterns filed as TODO-LR-5. Pinned by `test/audit/redact-connection-info.test.ts` (15 cases covering all 5 patterns + real Supabase fixture + ENOTFOUND fixture + version-string false-positive defense). -- `src/core/minions/lock-renewal-tick.ts` (v0.41.26.1) — Pure extracted function from `MinionWorker.launchJob`'s setInterval body. The structural fix that closes the v0.41.22.1 production unhandledRejection crash class. Exports `runLockRenewalTick(deps, state) → Promise` plus `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 uses time-based instead), `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 we 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?}` that the worker switches on; all `abort.abort()` / `clearInterval` side effects live in the worker's closure scope, not the pure function. Audit calls wrapped in inner try/catch (codex C4 defense-in-depth) so the audit module can never re-introduce the bug class through a new surface. Pinned by `test/worker-lock-renewal.test.ts` (18 hermetic state-machine cases — no fs, no PGLite, no real setInterval). +- `src/core/minions/lock-renewal-tick.ts` (v0.41.26.1) — Pure extracted function from `MinionWorker.launchJob`'s setInterval body. The structural fix that closes the v0.41.22.1 production unhandledRejection crash class. Exports `runLockRenewalTick(deps, state) → Promise` plus `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 uses time-based instead), `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 we 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?}` that the worker switches on; all `abort.abort()` / `clearInterval` side effects live in the worker's closure scope, not the pure function. Audit calls wrapped in inner try/catch (codex C4 defense-in-depth) so the audit module can never re-introduce the bug class through a new surface. Pinned by `test/worker-lock-renewal.test.ts` (18 hermetic state-machine cases — no fs, no PGLite, no real setInterval). **v0.42.5.0 (#1678):** `LockRenewalDeps` gains an optional `reconnect?` callback; the renewal-tick's catch branch (after the retryable-error classification + the time-based deadline check, before the inter-tick sleep) calls it ONCE, wrapped in its own try/catch so a reconnect throw can't re-introduce the unhandledRejection class. Recovers the mid-process-disconnect case (nulled `_sql`) where postgres.js's auto-reconnect can't help because the engine's pool reference is gone. Deliberately NOT a background `withRetry` (Codex #2: 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. +- `src/core/minions/worker-exit-codes.ts` (v0.42.5.0, #1678, NEW) — 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`. Before this, the RSS watchdog drained via `gracefulShutdown('watchdog')` which set `running=false` and exited code 0 — indistinguishable from a healthy queue-drain, so the supervisor's `code===0 → clean_exit` classifier never counted it and a ~400×/24h respawn loop stayed invisible (its only symptom was the downstream DB-connection cascade from the worker being shot mid-cycle). A distinct code makes the drain self-identifying as `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` (v0.42.5.0, #1678, NEW) — cgroup-aware auto-sized default for the worker RSS watchdog cap. `resolveDefaultMaxRssMb(opts?)` / `describeDefaultMaxRss(opts?)` (provenance for the startup log) / `readCgroupMemLimitBytes(readFile?)`. Replaces the flat `?? 2048` MB default at every spawn site (`jobs work`, `jobs supervisor`, autopilot, `MinionSupervisor`) — 2048 was absurdly low for any brain doing embeddings (working set ~10GB), so the watchdog drained legit work on every heavy cycle. Formula `clamp(round(0.5 × basisMB), 4096, 16384)` where `basis = min(cgroupLimit, totalmem)`. THE LOAD-BEARING NUANCE (Codex #5): 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 — straight back to the opaque death this fixes. 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 is applied 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` (v0.42.5.0, #1678, NEW) — 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 — Codex #8: 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 (Codex #9: a concurrent autopilot tick genuinely defers with `cycle_already_running`); NO release/reacquire-between-windows primitive (Codex #10). Pinned by `test/extract-atoms-drain.test.ts`. - `scripts/check-worker-lock-renewal-shape.sh` (v0.41.26.1, D4) — CI guard wired into `bun run verify`. Two invariants on `src/core/minions/worker.ts`: (1) the v0.41.22.1 bug pattern `lockTimer = setInterval(async ...)` must NOT appear (narrowed via `lockTimer =` prefix so unrelated `setInterval(async)` calls elsewhere in the file — like the stall detector at line ~269, codex C13 territory — don't false-fire), (2) `runLockRenewalTick` must remain referenced so the pure-function test seam survives refactors. Bug-pattern-specific by design (codex C12) — a future refactor to `setTimeout`-recursion or `AbortController`-based scheduling passes as long as the bug pattern stays absent. Uses 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 including the C13 false-positive defense for the stall-detector pattern elsewhere in the file). - `src/commands/doctor.ts:checkBatchRetryHealth` (v0.41.19.0, extended v0.41.27.0) — `batch_retry_health` check surfacing Supavisor circuit-breaker incidents. Wired into both `runDoctor` (local) and `doctorReportRemote` (thin-client). Reads last **24h** (not 7d — codex H-9: avoid permanent noise from one historical blip). 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 (codex M-10) instead of first-retry. Corrupt-JSONL tolerant. Paste-ready fix hints in every warn/fail message. Pinned by `test/doctor-batch-retry.test.ts` (10 cases). **v0.41.27.0 (#1570, codex finding 11):** extended (NOT a new check) to read `readRecentDbDisconnects(24)` and append `Disconnect-call audit: N call(s) in 24h (most recent caller: ).` to ALL three message paths (ok / warn / fail). The single-place surface keeps connection-incident signal greppable from one `gbrain doctor --json` call so operators correlating the v0.41.27 retry-reconnect symptom fix with the offending caller don't have to grep two audit files. Module-import wrapped in try/catch so older brains without the v0.41.27 audit file degrade silently. - `src/core/audit/db-disconnect-audit.ts` (v0.41.27.0, #1570, NEW) — JSONL audit for every call to `db.disconnect()` and `PostgresEngine.disconnect()`. Built on `audit-writer.ts` cathedral (no parallel subsystem; codex finding 11). 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 forever. Privacy: stack frames carry file paths but NO SQL content / row data / user strings — matches the v0.20 shell-audit posture. 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}`. **The instrument half of the "instrument first, fix later" pivot** — v0.41.27.0 ships the symptom fix (retry reconnect callback + facts:absorb drain) AND this diagnostic surface; v0.41.28+ patches the specific ownership boundary once production data tells us which code path is calling `disconnect()` mid-process. Wired into `src/core/db.ts:disconnect` and `src/core/postgres-engine.ts:disconnect` (logs 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). @@ -221,9 +224,9 @@ strict behavior when unset. - `src/core/zombie-reap.ts` (v0.28.1) — idempotent `installSigchldHandler()` so JS-spawned children get reaped via Bun's internal `waitpid()`. Bun (like Node) only auto-reaps when a SIGCHLD listener is registered; without it, every child the worker spawns (shell jobs, embed batches, sub-agents) becomes a zombie on exit and holds connection slots. Called once at module load from `src/cli.ts` (with Windows platform guard — SIGCHLD doesn't exist on Windows). Cross-file leak guard via `_uninstallSigchldHandlerForTests()` for tests. Layer 1 of the three-layer zombie defense; Layer 2 is tini-as-PID-1 wrapping the worker subtree (via `src/core/minions/spawn-helpers.ts`); Layer 3 is the container's own tini for hard Bun crashes. - `src/core/minions/` — Minions job queue: BullMQ-inspired, Postgres-native (queue, worker, backoff, types, protected-names, quiet-hours, stagger, handlers/shell). - `src/core/minions/queue.ts` — MinionQueue class (submit, claim, complete, fail, stall detection, parent-child, depth/child-cap, per-job timeouts, cascade-kill, attachments, idempotency keys, child_done inbox, removeOnComplete/Fail). `add()` takes a 4th `trusted` arg (separate from `opts` to prevent spread leakage); protected names in `PROTECTED_JOB_NAMES` require `{allowProtectedSubmit: true}` and the check runs trim-normalized (whitespace-bypass safe). v0.14.1 #219: `add()` plumbs `max_stalled` through with a `[1, 100]` clamp; omitted values let the schema DEFAULT (5) kick in. v0.19.0: `handleWallClockTimeouts(lockDurationMs)` is Layer 3 kill shot for jobs where `FOR UPDATE SKIP LOCKED` stall detection and the timeout sweep both fail to evict (wedged worker holding a row lock via a pending transaction). v0.19.1: `maxWaiting` coalesce path now uses `pg_advisory_xact_lock` keyed on `(name, queue)` to serialize concurrent submits for the same key, and filters on `queue` in addition to `name` so cross-queue same-name jobs don't suppress each other. -- `src/core/minions/worker.ts` — MinionWorker class (handler registry, lock renewal, graceful shutdown, timeout safety net). v0.14.0 abort-path fix: aborted jobs now call `failJob` with reason (`timeout`/`cancel`/`lock-lost`/`shutdown`) instead of returning silently. `shutdownAbort` (instance field) fires on process SIGTERM/SIGINT and propagates to `ctx.shutdownSignal` — shell handler listens to it; non-shell handlers don't. v0.22.1 (#403): per-job timeout fires `abort.abort(new Error('timeout'))` then a 30-second grace-then-evict safety net force-evicts the job from `inFlight` and marks it dead in DB if the handler ignores the abort signal — frees the slot even when a handler wedges (the 98-waiting-0-active prod incident driver). **v0.41.26.1 (supersedes #1567):** the lock-renewal cathedral wave. The `launchJob` setInterval block — which previously was `setInterval(async () => await renewLock(...))` — is the v0.41.22.1 production crash class (~39 worker exits/day from `unhandledRejection at renewLock` during PgBouncer connection rotation). Replaced with a thin sync wrapper around the new pure `runLockRenewalTick` function from `src/core/minions/lock-renewal-tick.ts`. Five additional gaps closed in the same wave: (1) `cancelled` flag captured in the timer closure stops in-flight IIFEs from writing misleading audit events after the job ended; (2) re-entrancy guard (`tickInFlight`) plus per-call `Promise.race` timeout (`GBRAIN_LOCK_RENEWAL_CALL_TIMEOUT_MS`, default `lockDuration/3`) so a hung renewLock can't wedge the worker indefinitely; (3) time-based abort (`Date.now() - lastSuccessfulRenewalAt >= lockDuration - GBRAIN_LOCK_RENEWAL_SAFETY_MARGIN_MS`) replaces count-based, so we release the lock BEFORE another worker can reclaim; (4) explicit `.catch()` on the stored `executeJob(...).finally(...)` promise closes the SECOND unhandledRejection vector (if failJob throws during the same DB outage); (5) `INFRASTRUCTURE_ABORT_REASONS = new Set(['lock-renewal-failed', 'lock-lost'])` is exported; executeJob's catch skips `failJob` for these reasons so PgBouncer blips no longer dead-letter healthy jobs (the stall detector reclaims cleanly with the lock already expired). The 30s grace-evict safety net moved out of the `job.timeout_ms`-only branch into a generic abort listener that fires for ANY abort reason. CI guard `scripts/check-worker-lock-renewal-shape.sh` (wired into `bun run verify`) asserts the bug pattern `lockTimer = setInterval(async ...)` stays absent AND `launchJob` continues to call `runLockRenewalTick` so the test seam survives refactors. Behavior pinned by `test/worker-lock-renewal.test.ts` (18 hermetic state-machine cases over the pure function), `test/audit/lock-renewal-audit.test.ts` (11 cases), `test/scripts/check-worker-lock-renewal-shape.test.ts` (5 fixture-driven meta-tests). **v0.28.1 engine-ownership invariant:** `start()` no longer calls `engine.disconnect()` on shutdown — that was a leaky abstraction (the worker disconnected an engine it didn't own). The CLI handler in `src/commands/jobs.ts case 'work'` now owns engine lifecycle via try/finally with loud error logging on disconnect failure. Pinned by `test/worker-shutdown-disconnect.test.ts` asserting the inverse (`disconnectSpy).not.toHaveBeenCalled()`). **v0.34.3.0:** RSS watchdog metric switched to non-file-backed pages on Linux. New exports `parseRssFromProcStatus(status)` (pure parser, exported for unit tests) and `getAccurateRss(readStatus?)` (reads `/proc/self/status` for `RssAnon + RssShmem`, falls back to `process.memoryUsage().rss` on macOS / restricted containers / kernel <4.5). The default `getRss` injected into `WorkerOpts` is now `getAccurateRss` instead of `process.memoryUsage().rss`. Closes the prod incident where VmRSS inflated to 7GB on a 96K-page brain (file-backed git packfile mmaps) while heap stayed at ~100MB; the watchdog was firing every autopilot cycle. M1 parser fix uses field-presence regex checks so `RssAnon: 0 + RssShmem: 512` (shmem-only worker case) parses correctly instead of falling through to VmRSS. Pinned by `test/worker-rss.test.ts` (11 cases). +- `src/core/minions/worker.ts` — MinionWorker class (handler registry, lock renewal, graceful shutdown, timeout safety net). v0.14.0 abort-path fix: aborted jobs now call `failJob` with reason (`timeout`/`cancel`/`lock-lost`/`shutdown`) instead of returning silently. `shutdownAbort` (instance field) fires on process SIGTERM/SIGINT and propagates to `ctx.shutdownSignal` — shell handler listens to it; non-shell handlers don't. v0.22.1 (#403): per-job timeout fires `abort.abort(new Error('timeout'))` then a 30-second grace-then-evict safety net force-evicts the job from `inFlight` and marks it dead in DB if the handler ignores the abort signal — frees the slot even when a handler wedges (the 98-waiting-0-active prod incident driver). **v0.41.26.1 (supersedes #1567):** the lock-renewal cathedral wave. The `launchJob` setInterval block — which previously was `setInterval(async () => await renewLock(...))` — is the v0.41.22.1 production crash class (~39 worker exits/day from `unhandledRejection at renewLock` during PgBouncer connection rotation). Replaced with a thin sync wrapper around the new pure `runLockRenewalTick` function from `src/core/minions/lock-renewal-tick.ts`. Five additional gaps closed in the same wave: (1) `cancelled` flag captured in the timer closure stops in-flight IIFEs from writing misleading audit events after the job ended; (2) re-entrancy guard (`tickInFlight`) plus per-call `Promise.race` timeout (`GBRAIN_LOCK_RENEWAL_CALL_TIMEOUT_MS`, default `lockDuration/3`) so a hung renewLock can't wedge the worker indefinitely; (3) time-based abort (`Date.now() - lastSuccessfulRenewalAt >= lockDuration - GBRAIN_LOCK_RENEWAL_SAFETY_MARGIN_MS`) replaces count-based, so we release the lock BEFORE another worker can reclaim; (4) explicit `.catch()` on the stored `executeJob(...).finally(...)` promise closes the SECOND unhandledRejection vector (if failJob throws during the same DB outage); (5) `INFRASTRUCTURE_ABORT_REASONS = new Set(['lock-renewal-failed', 'lock-lost'])` is exported; executeJob's catch skips `failJob` for these reasons so PgBouncer blips no longer dead-letter healthy jobs (the stall detector reclaims cleanly with the lock already expired). The 30s grace-evict safety net moved out of the `job.timeout_ms`-only branch into a generic abort listener that fires for ANY abort reason. CI guard `scripts/check-worker-lock-renewal-shape.sh` (wired into `bun run verify`) asserts the bug pattern `lockTimer = setInterval(async ...)` stays absent AND `launchJob` continues to call `runLockRenewalTick` so the test seam survives refactors. Behavior pinned by `test/worker-lock-renewal.test.ts` (18 hermetic state-machine cases over the pure function), `test/audit/lock-renewal-audit.test.ts` (11 cases), `test/scripts/check-worker-lock-renewal-shape.test.ts` (5 fixture-driven meta-tests). **v0.28.1 engine-ownership invariant:** `start()` no longer calls `engine.disconnect()` on shutdown — that was a leaky abstraction (the worker disconnected an engine it didn't own). The CLI handler in `src/commands/jobs.ts case 'work'` now owns engine lifecycle via try/finally with loud error logging on disconnect failure. Pinned by `test/worker-shutdown-disconnect.test.ts` asserting the inverse (`disconnectSpy).not.toHaveBeenCalled()`). **v0.34.3.0:** RSS watchdog metric switched to non-file-backed pages on Linux. New exports `parseRssFromProcStatus(status)` (pure parser, exported for unit tests) and `getAccurateRss(readStatus?)` (reads `/proc/self/status` for `RssAnon + RssShmem`, falls back to `process.memoryUsage().rss` on macOS / restricted containers / kernel <4.5). The default `getRss` injected into `WorkerOpts` is now `getAccurateRss` instead of `process.memoryUsage().rss`. Closes the prod incident where VmRSS inflated to 7GB on a 96K-page brain (file-backed git packfile mmaps) while heap stayed at ~100MB; the watchdog was firing every autopilot cycle. M1 parser fix uses field-presence regex checks so `RssAnon: 0 + RssShmem: 512` (shmem-only worker case) parses correctly instead of falling through to VmRSS. Pinned by `test/worker-rss.test.ts` (11 cases). **v0.42.5.0 (#1678):** `checkMemoryLimit` now tracks peak RSS, fires an 80%-of-cap soft-warn (once per crossing, carrying peak + in-flight job kinds), and on exceed sets `_rssWatchdogTriggered=true` (exposed via `get rssWatchdogTriggered()`) so `jobs work`'s finally can `process.exit(WORKER_EXIT_RSS_WATCHDOG)` after disconnect — making the drain self-identifying instead of an opaque code-0 exit. The poll loop wraps `claim` in try/catch: on a retryable conn error it reconnects ONCE and continues to the next tick rather than blind-retrying (Codex #1: a retry after the `UPDATE...RETURNING` committed but the socket died would double-claim). `LockRenewalDeps` is wired with `reconnect` when the engine supports it. - `src/core/minions/supervisor.ts` — MinionSupervisor process manager. Spawns `gbrain jobs work` as a child, restarts on crash with exponential backoff, periodic health check. v0.22.1 (#406): `consecutiveHealthFailures` counter; on 3 consecutive failures emits `health_warn` with `reason: 'db_connection_degraded'` and calls `engine.reconnect()` to swap in a fresh pool, then resets the counter. Worker exit classifier emits `likely_cause` field on `worker_exited` events: `oom_or_external_kill` (SIGKILL), `graceful_shutdown` (SIGTERM), `runtime_error` (code 1), `clean_exit` (code 0), `unknown`. **v0.28.1:** consumes `detectTini()` + `buildSpawnInvocation()` from `src/core/minions/spawn-helpers.ts` to wrap the worker subtree in tini-as-PID-1 when tini is on `PATH` (handles native-addon zombie reaping that the in-process SIGCHLD reaper can't reach). Exposes `isTiniDetected` read-only accessor for tests. **v0.34.3.0:** spawn-and-respawn loop extracted into the shared `ChildWorkerSupervisor` core (see entry below). MinionSupervisor now composes the inner class via `runSuperviseLoop()` → `new ChildWorkerSupervisor({...})` and maps `ChildSupervisorEvent` shapes back through the existing `emit()` SupervisorEvent channel — JSONL audit consumers see byte-compatible output across the rename. PID lock, signal handlers, health check, and `process.exit` on max-crashes stay in MinionSupervisor (standalone-daemon concerns). The pre-shipped reset-to-0-on-code=0 hunk that originally fixed the prod crash-counter incident is gone; the same fix lives in the shared core under the D1 amendment (code=0 leaves `crashCount` untouched, so a worker alternating real crashes + watchdog drains still trips `max_crashes`). D2 `cleanRestartBudget` (default 10 restarts per 60s) caps the macOS/non-Linux-fallback tight-loop by emitting `health_warn { reason: 'clean_restart_budget_exceeded' }` plus backoff after the threshold trips. `shutdown()` drains via `childSupervisor.killChild('SIGTERM')` + `awaitChildExit(35_000)` instead of reaching into `this.child` directly. Pinned by `test/supervisor.test.ts` (16 cases; existing tests that previously relied on clean-exit-as-crash semantics now use exit-1 workers since clean exits no longer count) and `test/supervisor-tini.test.ts`. -- `src/core/minions/child-worker-supervisor.ts` (v0.34.3.0) — shared spawn-and-respawn core extracted from `MinionSupervisor` so it can be reused by both `MinionSupervisor` (standalone `gbrain jobs supervisor` daemon) and `src/commands/autopilot.ts` (autopilot daemon). Pre-v0.34.3.0 the two consumers maintained parallel spawn loops that drifted into the same bug class — Codex caught it during plan-eng-review on PR #1003. Pure class: NO PID file, NO signal handlers, NO `process.exit`, NO health check. Lifecycle events fire via injected `onEvent: (ChildSupervisorEvent) => void` callback so each composer routes to its own log/audit channel. **D1 exit classifier:** `code === 0` leaves `crashCount` UNCHANGED (preserves flap detection across mixed exit sequences — a worker that alternates `exit 1 / exit 0 / exit 1 / exit 0` correctly trips `max_crashes` after 10 real crashes regardless of intervening clean exits). `code != 0` follows the existing `runDuration > stableRunResetMs ? 1 : ++crashCount` rule. **D2 clean-restart budget:** sliding window tracks code=0 exits; when count exceeds `cleanRestartBudget` (default 10) inside `cleanRestartWindowMs` (default 60s), emits `health_warn { reason: 'clean_restart_budget_exceeded' }` and applies `cleanRestartBudgetBackoffMs` (default 1s) before the next spawn. Caps the worst-case tight-loop on macOS / restricted containers / kernel <4.5 where the worker's RSS watchdog falls back to VmRSS. Public read-only accessors `childAlive`, `inBackoff`, `crashCount` for composer health checks; `killChild(signal)` + `awaitChildExit(timeoutMs)` for shutdown paths. `awaitChildExit` short-circuits when `child.exitCode !== null || child.signalCode !== null` (regression caught in pre-landing /review: pre-fix, fast-SIGTERM responders caused a 35-second shutdown hang because the late `once('exit', ...)` listener never fired). Test hooks: `_backoffFloorMs` skips the real backoff curve, `_now` injects a fake clock. Pinned by `test/child-worker-supervisor.test.ts` (7 cases: D1 classifier with code=0 not counted, interleaved exits still trip max_crashes, stable-run + clean-exit interaction across faked 6-minute run, D2 budget triggers backoff + health_warn, budget config is per-instance, awaitChildExit short-circuit, event-shape regression). Plan that produced the design lives at `~/.claude/plans/this-is-a-real-sleepy-sketch.md`. +- `src/core/minions/child-worker-supervisor.ts` (v0.34.3.0) — shared spawn-and-respawn core extracted from `MinionSupervisor` so it can be reused by both `MinionSupervisor` (standalone `gbrain jobs supervisor` daemon) and `src/commands/autopilot.ts` (autopilot daemon). Pre-v0.34.3.0 the two consumers maintained parallel spawn loops that drifted into the same bug class — Codex caught it during plan-eng-review on PR #1003. Pure class: NO PID file, NO signal handlers, NO `process.exit`, NO health check. Lifecycle events fire via injected `onEvent: (ChildSupervisorEvent) => void` callback so each composer routes to its own log/audit channel. **D1 exit classifier:** `code === 0` leaves `crashCount` UNCHANGED (preserves flap detection across mixed exit sequences — a worker that alternates `exit 1 / exit 0 / exit 1 / exit 0` correctly trips `max_crashes` after 10 real crashes regardless of intervening clean exits). `code != 0` follows the existing `runDuration > stableRunResetMs ? 1 : ++crashCount` rule. **D2 clean-restart budget:** sliding window tracks code=0 exits; when count exceeds `cleanRestartBudget` (default 10) inside `cleanRestartWindowMs` (default 60s), emits `health_warn { reason: 'clean_restart_budget_exceeded' }` and applies `cleanRestartBudgetBackoffMs` (default 1s) before the next spawn. Caps the worst-case tight-loop on macOS / restricted containers / kernel <4.5 where the worker's RSS watchdog falls back to VmRSS. Public read-only accessors `childAlive`, `inBackoff`, `crashCount` for composer health checks; `killChild(signal)` + `awaitChildExit(timeoutMs)` for shutdown paths. `awaitChildExit` short-circuits when `child.exitCode !== null || child.signalCode !== null` (regression caught in pre-landing /review: pre-fix, fast-SIGTERM responders caused a 35-second shutdown hang because the late `once('exit', ...)` listener never fired). Test hooks: `_backoffFloorMs` skips the real backoff curve, `_now` injects a fake clock. Pinned by `test/child-worker-supervisor.test.ts` (7 cases: D1 classifier with code=0 not counted, interleaved exits still trip max_crashes, stable-run + clean-exit interaction across faked 6-minute run, D2 budget triggers backoff + health_warn, budget config is per-instance, awaitChildExit short-circuit, event-shape regression). Plan that produced the design lives at `~/.claude/plans/this-is-a-real-sleepy-sketch.md`. **v0.42.5.0 (#1678):** the exit classifier special-cases `WORKER_EXIT_RSS_WATCHDOG` — `likely_cause='rss_watchdog'`, and that exit does NOT bump `crashCount` (so it routes to its own breaker, not the generic crash path). A dedicated `_watchdogExitTimestamps` sliding window (mirroring `_cleanRestartTimestamps`) trips a loud `rss_watchdog_loop` health_warn naming the cap when N watchdog exits land inside the window — INDEPENDENT of the stable-run reset that hid the original 400×/day loop (a >5-min run that watchdog-drains would otherwise never trip `max_crashes`). New opts `watchdogLoopBudget` (3), `watchdogLoopWindowMs` (600000), `watchdogBackoffMs` (30000); `ChildSupervisorEvent` extended. `supervisor-audit.ts` adds `rss_watchdog` as a non-clean cause + its own `CrashSummary.by_cause` bucket (denylist preserved so future causes still route to `legacy`). - `src/core/minions/spawn-helpers.ts` (v0.28.1) — pure `detectTini()` + `buildSpawnInvocation()` helpers consumed by both `supervisor.ts` and `autopilot.ts`. Resolves the DRY violation between the two spawn sites and makes the tini wrapping testable without `mock.module()` (rule R2 of `scripts/check-test-isolation.sh`). `detectTini()` calls `execFileSync('which', ['tini'])` with explicit `env: process.env` so Bun sees runtime PATH mutations (the env-snapshot bug fix). `buildSpawnInvocation(tiniPath, cmd, args)` returns `{cmd, args}` with tini prepended when present, or the bare invocation otherwise. Pinned by `test/spawn-helpers.test.ts` (5 cases) and `test/supervisor-tini.test.ts` (4 cases). - `src/core/minions/types.ts` — `MinionJobInput` + `MinionJobStatus` + handler context types. `MinionJobInput.max_stalled` (new in v0.14.1) is optional; omitted values let the schema DEFAULT (5) kick in, provided values are clamped to `[1, 100]`. - `src/core/minions/protected-names.ts` — side-effect-free constant module exporting `PROTECTED_JOB_NAMES` + `isProtectedJobName()`. Kept pure so queue core can import without loading handler modules. @@ -308,7 +311,7 @@ strict behavior when unset. - `src/core/transcripts.ts` (v0.29) — `listRecentTranscripts(engine, opts)` library reused by both the `gbrain transcripts recent` CLI and the `get_recent_transcripts` MCP op. Reads `dream.synthesize.session_corpus_dir` + `dream.synthesize.meeting_transcripts_dir` config keys (same as `discoverTranscripts`); walks for `.txt` files within `days`; applies `isDreamOutput` guard from `transcript-discovery.ts` (skips dream-generated files); returns `{path, date, mtime, length, summary}[]` sorted newest-first. Summary mode (default true) returns first non-empty line + ~250 trailing chars. Full mode caps at 100KB/file. Missing/non-existent corpus dirs return `[]`, not error. **Trust gate lives in the op handler, not here**: the op throws `permission_denied` for `ctx.remote === true`; this library is a trusted library function used by both the gated op and the local CLI. - `src/core/operations-descriptions.ts` (v0.29) — Constants module for tool descriptions. Pinned via `test/operations-descriptions.test.ts`. Houses `GET_RECENT_SALIENCE_DESCRIPTION`, `FIND_ANOMALIES_DESCRIPTION`, `GET_RECENT_TRANSCRIPTS_DESCRIPTION` plus the redirect-edited `LIST_PAGES_DESCRIPTION`, `QUERY_DESCRIPTION`, `SEARCH_DESCRIPTION`. Stable surface for the Tier-2 LLM routing eval — extracting them keeps the test from binding to whatever was in `operations.ts` at test-run time. - `src/core/cycle/transcript-discovery.ts` (v0.23) — Pure filesystem walk for synthesize. `discoverTranscripts(opts)` filters `.txt` files by date range, min_chars, and word-boundary regex `excludePatterns` (Q-3: `medical` matches "medical advice" but NOT "comedical"; power users may pass full regex). `readSingleTranscript(path)` is the `gbrain dream --input ` ad-hoc path. **v0.23.2 self-consumption guard:** `DREAM_OUTPUT_MARKER_RE` (anchored at frontmatter open `---\n`, optional BOM + CRLF tolerance, scans first 2000 chars for `dream_generated: true` with case-insensitive value and word boundary on `true`) drives `isDreamOutput(content, bypass=false)`. Both `discoverTranscripts` and `readSingleTranscript` skip matching files and emit a `[dream] skipped : dream_generated marker` stderr log (no more silent skips). `bypassGuard?: boolean` on `DiscoverOpts` and `readSingleTranscript`'s opts disables the guard for the explicit `--unsafe-bypass-dream-guard` escape hatch only — never auto-applied for `--input`. Replaces v0.23.1's `DREAM_OUTPUT_SLUGS` content-prefix list. -- `src/commands/dream.ts` — v0.17 `gbrain dream` CLI; ~80-line thin alias over `runCycle`. brainDir resolution requires explicit `--dir` OR `sync.repo_path` config. Flags: `--dry-run`, `--json`, `--phase `, `--pull`, `--dir `. **v0.23 added** `--input ` (ad-hoc transcript, implies `--phase synthesize`), `--date YYYY-MM-DD`, `--from --to ` (backfill range). Conflict detection: `--input` + `--date` exits 2. ISO date validation. `--dry-run` runs Haiku significance verdict but skips Sonnet synthesis (codex finding #8 — NOT zero LLM calls). Exit code 1 on status=failed. **v0.23.2 added** `--unsafe-bypass-dream-guard` (long-form intentional, plumbed through `runCycle.synthBypassDreamGuard` → `SynthesizePhaseOpts.bypassDreamGuard` → `discoverTranscripts({bypassGuard})` and `readSingleTranscript({bypassGuard})`). Loud stderr warning fires at synthesize-phase entry when set. Never auto-applied for `--input` so any caller can't silently re-trigger the loop bug. **v0.41.38.0:** `resolveBrainDir` now returns `string | null` (order: `--dir` → the resolved `--source`'s `local_path` → global `sync.repo_path` → null) instead of `process.exit(1)` on "No brain directory found". A checkout-less postgres/Supabase brain runs the DB-only phases (incl. `resolve_symbol_edges`, the call-graph builder) and skips the 6 filesystem phases with `details.reason: 'no_brain_dir'`; `runDream` owns the only hard error (no checkout AND no engine). When `--source` resolves but that source has no on-disk checkout, returns null (DB-only) rather than borrowing a different source's global `sync.repo_path` (would mix scopes — codex P1 review). Pinned by `test/dream-postgres.serial.test.ts`. +- `src/commands/dream.ts` — v0.17 `gbrain dream` CLI; ~80-line thin alias over `runCycle`. brainDir resolution requires explicit `--dir` OR `sync.repo_path` config. Flags: `--dry-run`, `--json`, `--phase `, `--pull`, `--dir `. **v0.23 added** `--input ` (ad-hoc transcript, implies `--phase synthesize`), `--date YYYY-MM-DD`, `--from --to ` (backfill range). Conflict detection: `--input` + `--date` exits 2. ISO date validation. `--dry-run` runs Haiku significance verdict but skips Sonnet synthesis (codex finding #8 — NOT zero LLM calls). Exit code 1 on status=failed. **v0.23.2 added** `--unsafe-bypass-dream-guard` (long-form intentional, plumbed through `runCycle.synthBypassDreamGuard` → `SynthesizePhaseOpts.bypassDreamGuard` → `discoverTranscripts({bypassGuard})` and `readSingleTranscript({bypassGuard})`). Loud stderr warning fires at synthesize-phase entry when set. Never auto-applied for `--input` so any caller can't silently re-trigger the loop bug. **v0.41.38.0:** `resolveBrainDir` now returns `string | null` (order: `--dir` → the resolved `--source`'s `local_path` → global `sync.repo_path` → null) instead of `process.exit(1)` on "No brain directory found". A checkout-less postgres/Supabase brain runs the DB-only phases (incl. `resolve_symbol_edges`, the call-graph builder) and skips the 6 filesystem phases with `details.reason: 'no_brain_dir'`; `runDream` owns the only hard error (no checkout AND no engine). When `--source` resolves but that source has no on-disk checkout, returns null (DB-only) rather than borrowing a different source's global `sync.repo_path` (would mix scopes — codex P1 review). Pinned by `test/dream-postgres.serial.test.ts`. **v0.42.5.0 (#1678):** new `--drain [--window ]` for `--phase extract_atoms` — `runDrain()` bypasses the pack-gate and runs the single-hold bounded drain from `src/core/cycle/extract-atoms-drain.ts` under the same `cycleLockIdFor(sourceId)` the routine cycle uses (so a concurrent autopilot tick defers with `cycle_already_running`), reporting `{extracted, skipped, remaining}`. Exits `EXIT_DRAIN_INCOMPLETE=3` while `remaining > 0` so a cron/agent loop knows to continue. A null backlog count (the count query FAILED) is also treated as incomplete (exit 3), never as a drained success (Codex pre-landing #4). `LockUnavailableError` → `cycle_already_running` skip (also exit 3). The `extract_atoms_backlog` doctor check (`computeExtractAtomsBacklogCheck`) surfaces the silent pack-gated backlog with the exact `--drain` command; pack-gated cycle skips carry a greppable `pack_gated:true` marker. - `src/commands/friction.ts` + `src/core/friction.ts` (v0.23) — `gbrain friction {log,render,list,summary}` reporter. Append-only JSONL under `$GBRAIN_HOME/friction/.jsonl`. Schema is a flat extension of `StructuredAgentError` (D20). Render groups by severity → phase, defaults to `--redact` for md output (strips `$HOME`/`$CWD` to placeholders so reports paste safely in PRs). Run-id resolves from `--run-id` > `$GBRAIN_FRICTION_RUN_ID` > `standalone.jsonl`. Skills the claw-test exercises gain a `_friction-protocol.md` callout so agents know when to log friction. - `src/commands/claw-test.ts` + `src/core/claw-test/` (v0.23) — `gbrain claw-test [--scenario ] [--live --agent openclaw]`. End-to-end "fresh user" friction harness. Two modes: scripted (CI gate, agent-free) and live (real openclaw subprocess, $1–2 in tokens). Sets `GBRAIN_HOME=` for hermeticity and captures gbrain's `--progress-json` events from each child's stderr to verify expected phases ran (`import.files`, `extract.links_fs`, `doctor.db_checks`). Phases for scripted mode: setup → install_brain (`gbrain init --pglite`) → import (`--no-embed`) → query → extract → verify (`gbrain doctor --json`, asserts `status: 'ok'`) → render. Live mode hands `BRIEF.md` from `test/fixtures/claw-test-scenarios//` to the agent runner. v1 ships with the OpenClaw runner only (`src/core/claw-test/runners/openclaw.ts`, invokes `openclaw agent --local --agent --message `); hermes runner deferred to v1.1. Transcript capture (`transcript-capture.ts`) uses `fs.createWriteStream` with `'drain'`-event backpressure — D17 fix for the 256KB-burst child-stall scenario. v0.18 upgrade scenario seeded via `seed-pglite.ts` SQL replay. - `skills/_friction-protocol.md` (v0.23) — shared cross-cutting convention skill (like `_brain-filing-rules.md`). Tells agents when to call `gbrain friction log` and how to choose a severity. Routes to friction CLI from any skill the claw-test exercises. diff --git a/TODOS.md b/TODOS.md index 4e8283445..3fe7ac7ee 100644 --- a/TODOS.md +++ b/TODOS.md @@ -1,5 +1,58 @@ # TODOS +## v0.42.5.0 watchdog / pooler-reap / lens-backlog follow-ups (v0.42+) + +Deferred from the v0.42.5.0 wave (issue #1678). The shipped fixes are complete +and tested; these are documented tradeoffs and stronger-but-bigger versions. + +- [ ] **P2 — `claim` idempotent recovery.** v0.42.5.0 deliberately does NOT + inline-retry `claim` (a retry after the `UPDATE...RETURNING` committed but the + socket died could double-claim a job); instead the worker poll loop reconnects + and re-claims on the next tick. Codex independently flagged the residual: if + claim's UPDATE commits but the connection dies before `RETURNING` reaches the + worker, that job is `active` in the DB but absent from `inFlight` (orphaned). It + is NOT lost — the stall detector reclaims it once `lock_until` expires (~one + lock-duration + stall-interval, ~60s) and requeues it (stalled_counter 0 → first + stall requeues, not dead-letters). The stronger fix: after a reconnect, look up + an active job already holding this worker's `lock_token` before claiming a new + one, so the orphan is recovered immediately instead of after a stall cycle. + Needs the claim path to thread the lock_token through recovery. +- [ ] **P3 — `dream --drain` PGLite lock-path parity.** The drain takes the DB + refreshing lock (`cycleLockIdFor`), which is the correct lock the routine cycle + uses on Postgres. On PGLite the routine cycle uses the global FILE lock instead, + so the drain's DB lock doesn't contend with it. This is currently moot because + PGLite's exclusive single-process file lock means a separate `gbrain dream + --drain` process can't even open the brain while autopilot's `gbrain dream` + holds it (one fails at connect). If PGLite ever gains multi-handle access, + the drain must also acquire the cycle file lock. Codex-flagged; low risk today. +- [ ] **P2 — `synthesize_concepts_backlog` doctor check.** The `extract_atoms` + backlog check shipped; `synthesize_concepts` did not, because that phase is a + stub with no real eligibility predicate (a NOT-EXISTS analog to atom + `source_hash`). Add the check once the phase has a concrete "what's left" + definition, else it's a fake signal. +- [ ] **P3 — `renewLock` AbortSignal-bounded retry.** The renewal tick recovers + via a bounded reconnect-once + postgres.js auto-reconnect + multi-tick grace, + NOT a `withRetry` around `renewLock` (which would race the tick's own timeout + and could refresh a lock after another worker reclaimed it). If production shows + the multi-tick grace is insufficient under sustained pooler churn, add an + abort-aligned bounded retry under `callTimeoutMs`. +- [ ] **P3 — Waiter-flag cooperative lock.** The `--drain` mode uses a single + bounded lock hold (autopilot defers for the window) rather than a + release/reacquire-between-windows protocol with a `wants_lock` signal column. + Tighter interleaving (autopilot preempts a long drain mid-window) would need + that protocol + a migration; deferred as not worth the surface for the bounded + window the drain already provides. +- [ ] **P3 — `cycle.force_phases` config.** No config to force a pack-gated phase + (e.g. `extract_atoms`) to run inside the routine 5-min cycle. The `--drain` + escape hatch + doctor warning cover the operator need; a config override would + let the routine cycle run an expensive lens phase every tick (the reason it's + pack-gated). Add only if a real workflow needs it. +- [ ] **P3 — Full per-job-kind RSS peak tracking.** The watchdog logs peak RSS + + the in-flight job kind on the drain line and the 80% soft-warn, but doesn't + persist per-job-kind peaks to an audit file or surface "embed-backfill peaked at + 9.8GB, cap 8GB" in doctor. Add persisted tracking + a doctor check if operators + want trend visibility rather than the point-in-time log line. + ## v0.42.2.0 gbrain connect follow-ups (v0.42+) - [ ] **T6 (P3): `gbrain connect --env-token` form.** Ship the env-var-indirection diff --git a/VERSION b/VERSION index 85654d5e1..de33bf3b0 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.42.4.0 \ No newline at end of file +0.42.5.0 \ No newline at end of file diff --git a/llms-full.txt b/llms-full.txt index cb090f936..548b807de 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -317,7 +317,10 @@ strict behavior when unset. - `src/core/audit/batch-retry-audit.ts` (v0.41.19.0, extended v0.41.26.1) — JSONL audit primitive for batch-retry events. Built on `audit-writer.ts` cathedral. Schema: `{ts, site, batch_size, attempt, outcome: 'success' | 'exhausted', delay_ms, error_message_summary, error_code?}`. Privacy posture: NEVER logs slugs / page IDs / content (mirrors `shell-audit.ts` from v0.20+). `logBatchRetry` fires per successful retry recovery; `logBatchExhausted` fires when retries exhaust and rows are lost. `readRecentBatchRetryEvents(hours=24)` returns `{events, corrupted_lines, files_scanned, files_unreadable}` — corruption + permission errors surface to doctor, not silently swallowed (codex H-9). `pruneOldBatchRetryAuditFiles(daysToKeep=30)` deletes old files (codex H-8 — implements the v0.41 plan's "30-day pruning convention" for real). Called from `gbrain dream --phase purge`. File: `~/.gbrain/audit/batch-retry-YYYY-Www.jsonl` (honors `GBRAIN_AUDIT_DIR`). **v0.41.26.1 privacy backfill:** `summarizeError` now routes error messages through the shared `redactConnectionInfo` helper from `src/core/audit/redact-connection-info.ts` BEFORE truncation, so DSNs / hostnames / credentials / IPv4 octets can't leak from a Postgres connection-failure error into operator-shared JSONL dumps. Same risk class as lock-renewal-audit; closed in the same wave. Pinned by `test/audit/batch-retry-audit.test.ts` (12 cases) + new `test/audit/batch-retry-redaction.test.ts` (3 privacy regressions). - `src/core/audit/lock-renewal-audit.ts` (v0.41.26.1) — JSONL audit primitive for per-job lock-renewal faults. Sibling of `batch-retry-audit.ts`; built on `audit-writer.ts`. Four outcomes: `failure` (single renewLock throw, counter incremented), `success_after_failure` (recovery; emits the recovery count), `gave_up` (time-based deadline exceeded; abort fired), `executeJob_rejected` (the SECOND unhandledRejection vector from D7 — the stored `executeJob(...).finally(...)` promise itself rejected, e.g. failJob threw during the same DB outage). Schema: `{ts, job_id, job_name, attempt?, outcome, error_message_summary?, error_code?}`. Privacy: NEVER logs `lock_token` or `job.data`. Error summaries route through `redactConnectionInfo` BEFORE truncation. Defense-in-depth: every audit call inside the lock-renewal tick's catch block is wrapped in its own inner try/catch so a misbehaving audit-writer can't re-introduce the unhandledRejection bug class via a new surface. `readRecentLockRenewalEvents(hours=24)` walks current + previous ISO week with corrupted-line tolerance. `pruneOldLockRenewalAuditFiles(daysToKeep=30)` is ready for future dream-cycle purge wiring (filed as TODO-LR-3). File: `~/.gbrain/audit/lock-renewal-YYYY-Www.jsonl`. Operator UX is `tail -F` until the doctor check lands (TODO-LR-2). Pinned by `test/audit/lock-renewal-audit.test.ts` (11 cases). - `src/core/audit/redact-connection-info.ts` (v0.41.26.1) — Shared pure helper. `redactConnectionInfo(text: string): string` strips Postgres connection info before any audit JSONL write: `postgres://`/`postgresql://` URLs, `host=foo`, `user=foo`, `password=foo`, `pwd=foo`, IPv4 octets. Each match becomes ``. Negative-lookbehind/lookahead `[\w.@-]` on the IPv4 pattern defeats version-string false positives like `v3.1.4.0` or `tree-sitter@0.26.3.1` while still matching real IPs in PG errors (`(192.168.1.42)`). Order-sensitive pattern set: URL forms first so substrings inside URLs don't get double-redacted. Idempotent (running twice produces same output), pure (no I/O), hot-path-safe (regex compiled at module load). Wired into BOTH `lock-renewal-audit.ts` (new) AND `batch-retry-audit.ts` (privacy backfill — same risk class). Risk model documented in module header: Postgres errors during connection failures embed DSNs / credentials / hostnames into error messages; operators routinely paste audit dumps into GitHub issues / Slack to debug; the audit channel must be safe to share by construction. Known limitations: bare-quoted hostnames (`at "db.example.com"`) and bare-quoted usernames (`for user "postgres.foo"`) are NOT caught — the IP in those PG error shapes is the highest-value leak and IS caught. Quoted-string patterns filed as TODO-LR-5. Pinned by `test/audit/redact-connection-info.test.ts` (15 cases covering all 5 patterns + real Supabase fixture + ENOTFOUND fixture + version-string false-positive defense). -- `src/core/minions/lock-renewal-tick.ts` (v0.41.26.1) — Pure extracted function from `MinionWorker.launchJob`'s setInterval body. The structural fix that closes the v0.41.22.1 production unhandledRejection crash class. Exports `runLockRenewalTick(deps, state) → Promise` plus `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 uses time-based instead), `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 we 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?}` that the worker switches on; all `abort.abort()` / `clearInterval` side effects live in the worker's closure scope, not the pure function. Audit calls wrapped in inner try/catch (codex C4 defense-in-depth) so the audit module can never re-introduce the bug class through a new surface. Pinned by `test/worker-lock-renewal.test.ts` (18 hermetic state-machine cases — no fs, no PGLite, no real setInterval). +- `src/core/minions/lock-renewal-tick.ts` (v0.41.26.1) — Pure extracted function from `MinionWorker.launchJob`'s setInterval body. The structural fix that closes the v0.41.22.1 production unhandledRejection crash class. Exports `runLockRenewalTick(deps, state) → Promise` plus `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 uses time-based instead), `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 we 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?}` that the worker switches on; all `abort.abort()` / `clearInterval` side effects live in the worker's closure scope, not the pure function. Audit calls wrapped in inner try/catch (codex C4 defense-in-depth) so the audit module can never re-introduce the bug class through a new surface. Pinned by `test/worker-lock-renewal.test.ts` (18 hermetic state-machine cases — no fs, no PGLite, no real setInterval). **v0.42.5.0 (#1678):** `LockRenewalDeps` gains an optional `reconnect?` callback; the renewal-tick's catch branch (after the retryable-error classification + the time-based deadline check, before the inter-tick sleep) calls it ONCE, wrapped in its own try/catch so a reconnect throw can't re-introduce the unhandledRejection class. Recovers the mid-process-disconnect case (nulled `_sql`) where postgres.js's auto-reconnect can't help because the engine's pool reference is gone. Deliberately NOT a background `withRetry` (Codex #2: 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. +- `src/core/minions/worker-exit-codes.ts` (v0.42.5.0, #1678, NEW) — 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`. Before this, the RSS watchdog drained via `gracefulShutdown('watchdog')` which set `running=false` and exited code 0 — indistinguishable from a healthy queue-drain, so the supervisor's `code===0 → clean_exit` classifier never counted it and a ~400×/24h respawn loop stayed invisible (its only symptom was the downstream DB-connection cascade from the worker being shot mid-cycle). A distinct code makes the drain self-identifying as `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` (v0.42.5.0, #1678, NEW) — cgroup-aware auto-sized default for the worker RSS watchdog cap. `resolveDefaultMaxRssMb(opts?)` / `describeDefaultMaxRss(opts?)` (provenance for the startup log) / `readCgroupMemLimitBytes(readFile?)`. Replaces the flat `?? 2048` MB default at every spawn site (`jobs work`, `jobs supervisor`, autopilot, `MinionSupervisor`) — 2048 was absurdly low for any brain doing embeddings (working set ~10GB), so the watchdog drained legit work on every heavy cycle. Formula `clamp(round(0.5 × basisMB), 4096, 16384)` where `basis = min(cgroupLimit, totalmem)`. THE LOAD-BEARING NUANCE (Codex #5): 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 — straight back to the opaque death this fixes. 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 is applied 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` (v0.42.5.0, #1678, NEW) — 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 — Codex #8: 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 (Codex #9: a concurrent autopilot tick genuinely defers with `cycle_already_running`); NO release/reacquire-between-windows primitive (Codex #10). Pinned by `test/extract-atoms-drain.test.ts`. - `scripts/check-worker-lock-renewal-shape.sh` (v0.41.26.1, D4) — CI guard wired into `bun run verify`. Two invariants on `src/core/minions/worker.ts`: (1) the v0.41.22.1 bug pattern `lockTimer = setInterval(async ...)` must NOT appear (narrowed via `lockTimer =` prefix so unrelated `setInterval(async)` calls elsewhere in the file — like the stall detector at line ~269, codex C13 territory — don't false-fire), (2) `runLockRenewalTick` must remain referenced so the pure-function test seam survives refactors. Bug-pattern-specific by design (codex C12) — a future refactor to `setTimeout`-recursion or `AbortController`-based scheduling passes as long as the bug pattern stays absent. Uses 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 including the C13 false-positive defense for the stall-detector pattern elsewhere in the file). - `src/commands/doctor.ts:checkBatchRetryHealth` (v0.41.19.0, extended v0.41.27.0) — `batch_retry_health` check surfacing Supavisor circuit-breaker incidents. Wired into both `runDoctor` (local) and `doctorReportRemote` (thin-client). Reads last **24h** (not 7d — codex H-9: avoid permanent noise from one historical blip). 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 (codex M-10) instead of first-retry. Corrupt-JSONL tolerant. Paste-ready fix hints in every warn/fail message. Pinned by `test/doctor-batch-retry.test.ts` (10 cases). **v0.41.27.0 (#1570, codex finding 11):** extended (NOT a new check) to read `readRecentDbDisconnects(24)` and append `Disconnect-call audit: N call(s) in 24h (most recent caller: ).` to ALL three message paths (ok / warn / fail). The single-place surface keeps connection-incident signal greppable from one `gbrain doctor --json` call so operators correlating the v0.41.27 retry-reconnect symptom fix with the offending caller don't have to grep two audit files. Module-import wrapped in try/catch so older brains without the v0.41.27 audit file degrade silently. - `src/core/audit/db-disconnect-audit.ts` (v0.41.27.0, #1570, NEW) — JSONL audit for every call to `db.disconnect()` and `PostgresEngine.disconnect()`. Built on `audit-writer.ts` cathedral (no parallel subsystem; codex finding 11). 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 forever. Privacy: stack frames carry file paths but NO SQL content / row data / user strings — matches the v0.20 shell-audit posture. 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}`. **The instrument half of the "instrument first, fix later" pivot** — v0.41.27.0 ships the symptom fix (retry reconnect callback + facts:absorb drain) AND this diagnostic surface; v0.41.28+ patches the specific ownership boundary once production data tells us which code path is calling `disconnect()` mid-process. Wired into `src/core/db.ts:disconnect` and `src/core/postgres-engine.ts:disconnect` (logs 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). @@ -363,9 +366,9 @@ strict behavior when unset. - `src/core/zombie-reap.ts` (v0.28.1) — idempotent `installSigchldHandler()` so JS-spawned children get reaped via Bun's internal `waitpid()`. Bun (like Node) only auto-reaps when a SIGCHLD listener is registered; without it, every child the worker spawns (shell jobs, embed batches, sub-agents) becomes a zombie on exit and holds connection slots. Called once at module load from `src/cli.ts` (with Windows platform guard — SIGCHLD doesn't exist on Windows). Cross-file leak guard via `_uninstallSigchldHandlerForTests()` for tests. Layer 1 of the three-layer zombie defense; Layer 2 is tini-as-PID-1 wrapping the worker subtree (via `src/core/minions/spawn-helpers.ts`); Layer 3 is the container's own tini for hard Bun crashes. - `src/core/minions/` — Minions job queue: BullMQ-inspired, Postgres-native (queue, worker, backoff, types, protected-names, quiet-hours, stagger, handlers/shell). - `src/core/minions/queue.ts` — MinionQueue class (submit, claim, complete, fail, stall detection, parent-child, depth/child-cap, per-job timeouts, cascade-kill, attachments, idempotency keys, child_done inbox, removeOnComplete/Fail). `add()` takes a 4th `trusted` arg (separate from `opts` to prevent spread leakage); protected names in `PROTECTED_JOB_NAMES` require `{allowProtectedSubmit: true}` and the check runs trim-normalized (whitespace-bypass safe). v0.14.1 #219: `add()` plumbs `max_stalled` through with a `[1, 100]` clamp; omitted values let the schema DEFAULT (5) kick in. v0.19.0: `handleWallClockTimeouts(lockDurationMs)` is Layer 3 kill shot for jobs where `FOR UPDATE SKIP LOCKED` stall detection and the timeout sweep both fail to evict (wedged worker holding a row lock via a pending transaction). v0.19.1: `maxWaiting` coalesce path now uses `pg_advisory_xact_lock` keyed on `(name, queue)` to serialize concurrent submits for the same key, and filters on `queue` in addition to `name` so cross-queue same-name jobs don't suppress each other. -- `src/core/minions/worker.ts` — MinionWorker class (handler registry, lock renewal, graceful shutdown, timeout safety net). v0.14.0 abort-path fix: aborted jobs now call `failJob` with reason (`timeout`/`cancel`/`lock-lost`/`shutdown`) instead of returning silently. `shutdownAbort` (instance field) fires on process SIGTERM/SIGINT and propagates to `ctx.shutdownSignal` — shell handler listens to it; non-shell handlers don't. v0.22.1 (#403): per-job timeout fires `abort.abort(new Error('timeout'))` then a 30-second grace-then-evict safety net force-evicts the job from `inFlight` and marks it dead in DB if the handler ignores the abort signal — frees the slot even when a handler wedges (the 98-waiting-0-active prod incident driver). **v0.41.26.1 (supersedes #1567):** the lock-renewal cathedral wave. The `launchJob` setInterval block — which previously was `setInterval(async () => await renewLock(...))` — is the v0.41.22.1 production crash class (~39 worker exits/day from `unhandledRejection at renewLock` during PgBouncer connection rotation). Replaced with a thin sync wrapper around the new pure `runLockRenewalTick` function from `src/core/minions/lock-renewal-tick.ts`. Five additional gaps closed in the same wave: (1) `cancelled` flag captured in the timer closure stops in-flight IIFEs from writing misleading audit events after the job ended; (2) re-entrancy guard (`tickInFlight`) plus per-call `Promise.race` timeout (`GBRAIN_LOCK_RENEWAL_CALL_TIMEOUT_MS`, default `lockDuration/3`) so a hung renewLock can't wedge the worker indefinitely; (3) time-based abort (`Date.now() - lastSuccessfulRenewalAt >= lockDuration - GBRAIN_LOCK_RENEWAL_SAFETY_MARGIN_MS`) replaces count-based, so we release the lock BEFORE another worker can reclaim; (4) explicit `.catch()` on the stored `executeJob(...).finally(...)` promise closes the SECOND unhandledRejection vector (if failJob throws during the same DB outage); (5) `INFRASTRUCTURE_ABORT_REASONS = new Set(['lock-renewal-failed', 'lock-lost'])` is exported; executeJob's catch skips `failJob` for these reasons so PgBouncer blips no longer dead-letter healthy jobs (the stall detector reclaims cleanly with the lock already expired). The 30s grace-evict safety net moved out of the `job.timeout_ms`-only branch into a generic abort listener that fires for ANY abort reason. CI guard `scripts/check-worker-lock-renewal-shape.sh` (wired into `bun run verify`) asserts the bug pattern `lockTimer = setInterval(async ...)` stays absent AND `launchJob` continues to call `runLockRenewalTick` so the test seam survives refactors. Behavior pinned by `test/worker-lock-renewal.test.ts` (18 hermetic state-machine cases over the pure function), `test/audit/lock-renewal-audit.test.ts` (11 cases), `test/scripts/check-worker-lock-renewal-shape.test.ts` (5 fixture-driven meta-tests). **v0.28.1 engine-ownership invariant:** `start()` no longer calls `engine.disconnect()` on shutdown — that was a leaky abstraction (the worker disconnected an engine it didn't own). The CLI handler in `src/commands/jobs.ts case 'work'` now owns engine lifecycle via try/finally with loud error logging on disconnect failure. Pinned by `test/worker-shutdown-disconnect.test.ts` asserting the inverse (`disconnectSpy).not.toHaveBeenCalled()`). **v0.34.3.0:** RSS watchdog metric switched to non-file-backed pages on Linux. New exports `parseRssFromProcStatus(status)` (pure parser, exported for unit tests) and `getAccurateRss(readStatus?)` (reads `/proc/self/status` for `RssAnon + RssShmem`, falls back to `process.memoryUsage().rss` on macOS / restricted containers / kernel <4.5). The default `getRss` injected into `WorkerOpts` is now `getAccurateRss` instead of `process.memoryUsage().rss`. Closes the prod incident where VmRSS inflated to 7GB on a 96K-page brain (file-backed git packfile mmaps) while heap stayed at ~100MB; the watchdog was firing every autopilot cycle. M1 parser fix uses field-presence regex checks so `RssAnon: 0 + RssShmem: 512` (shmem-only worker case) parses correctly instead of falling through to VmRSS. Pinned by `test/worker-rss.test.ts` (11 cases). +- `src/core/minions/worker.ts` — MinionWorker class (handler registry, lock renewal, graceful shutdown, timeout safety net). v0.14.0 abort-path fix: aborted jobs now call `failJob` with reason (`timeout`/`cancel`/`lock-lost`/`shutdown`) instead of returning silently. `shutdownAbort` (instance field) fires on process SIGTERM/SIGINT and propagates to `ctx.shutdownSignal` — shell handler listens to it; non-shell handlers don't. v0.22.1 (#403): per-job timeout fires `abort.abort(new Error('timeout'))` then a 30-second grace-then-evict safety net force-evicts the job from `inFlight` and marks it dead in DB if the handler ignores the abort signal — frees the slot even when a handler wedges (the 98-waiting-0-active prod incident driver). **v0.41.26.1 (supersedes #1567):** the lock-renewal cathedral wave. The `launchJob` setInterval block — which previously was `setInterval(async () => await renewLock(...))` — is the v0.41.22.1 production crash class (~39 worker exits/day from `unhandledRejection at renewLock` during PgBouncer connection rotation). Replaced with a thin sync wrapper around the new pure `runLockRenewalTick` function from `src/core/minions/lock-renewal-tick.ts`. Five additional gaps closed in the same wave: (1) `cancelled` flag captured in the timer closure stops in-flight IIFEs from writing misleading audit events after the job ended; (2) re-entrancy guard (`tickInFlight`) plus per-call `Promise.race` timeout (`GBRAIN_LOCK_RENEWAL_CALL_TIMEOUT_MS`, default `lockDuration/3`) so a hung renewLock can't wedge the worker indefinitely; (3) time-based abort (`Date.now() - lastSuccessfulRenewalAt >= lockDuration - GBRAIN_LOCK_RENEWAL_SAFETY_MARGIN_MS`) replaces count-based, so we release the lock BEFORE another worker can reclaim; (4) explicit `.catch()` on the stored `executeJob(...).finally(...)` promise closes the SECOND unhandledRejection vector (if failJob throws during the same DB outage); (5) `INFRASTRUCTURE_ABORT_REASONS = new Set(['lock-renewal-failed', 'lock-lost'])` is exported; executeJob's catch skips `failJob` for these reasons so PgBouncer blips no longer dead-letter healthy jobs (the stall detector reclaims cleanly with the lock already expired). The 30s grace-evict safety net moved out of the `job.timeout_ms`-only branch into a generic abort listener that fires for ANY abort reason. CI guard `scripts/check-worker-lock-renewal-shape.sh` (wired into `bun run verify`) asserts the bug pattern `lockTimer = setInterval(async ...)` stays absent AND `launchJob` continues to call `runLockRenewalTick` so the test seam survives refactors. Behavior pinned by `test/worker-lock-renewal.test.ts` (18 hermetic state-machine cases over the pure function), `test/audit/lock-renewal-audit.test.ts` (11 cases), `test/scripts/check-worker-lock-renewal-shape.test.ts` (5 fixture-driven meta-tests). **v0.28.1 engine-ownership invariant:** `start()` no longer calls `engine.disconnect()` on shutdown — that was a leaky abstraction (the worker disconnected an engine it didn't own). The CLI handler in `src/commands/jobs.ts case 'work'` now owns engine lifecycle via try/finally with loud error logging on disconnect failure. Pinned by `test/worker-shutdown-disconnect.test.ts` asserting the inverse (`disconnectSpy).not.toHaveBeenCalled()`). **v0.34.3.0:** RSS watchdog metric switched to non-file-backed pages on Linux. New exports `parseRssFromProcStatus(status)` (pure parser, exported for unit tests) and `getAccurateRss(readStatus?)` (reads `/proc/self/status` for `RssAnon + RssShmem`, falls back to `process.memoryUsage().rss` on macOS / restricted containers / kernel <4.5). The default `getRss` injected into `WorkerOpts` is now `getAccurateRss` instead of `process.memoryUsage().rss`. Closes the prod incident where VmRSS inflated to 7GB on a 96K-page brain (file-backed git packfile mmaps) while heap stayed at ~100MB; the watchdog was firing every autopilot cycle. M1 parser fix uses field-presence regex checks so `RssAnon: 0 + RssShmem: 512` (shmem-only worker case) parses correctly instead of falling through to VmRSS. Pinned by `test/worker-rss.test.ts` (11 cases). **v0.42.5.0 (#1678):** `checkMemoryLimit` now tracks peak RSS, fires an 80%-of-cap soft-warn (once per crossing, carrying peak + in-flight job kinds), and on exceed sets `_rssWatchdogTriggered=true` (exposed via `get rssWatchdogTriggered()`) so `jobs work`'s finally can `process.exit(WORKER_EXIT_RSS_WATCHDOG)` after disconnect — making the drain self-identifying instead of an opaque code-0 exit. The poll loop wraps `claim` in try/catch: on a retryable conn error it reconnects ONCE and continues to the next tick rather than blind-retrying (Codex #1: a retry after the `UPDATE...RETURNING` committed but the socket died would double-claim). `LockRenewalDeps` is wired with `reconnect` when the engine supports it. - `src/core/minions/supervisor.ts` — MinionSupervisor process manager. Spawns `gbrain jobs work` as a child, restarts on crash with exponential backoff, periodic health check. v0.22.1 (#406): `consecutiveHealthFailures` counter; on 3 consecutive failures emits `health_warn` with `reason: 'db_connection_degraded'` and calls `engine.reconnect()` to swap in a fresh pool, then resets the counter. Worker exit classifier emits `likely_cause` field on `worker_exited` events: `oom_or_external_kill` (SIGKILL), `graceful_shutdown` (SIGTERM), `runtime_error` (code 1), `clean_exit` (code 0), `unknown`. **v0.28.1:** consumes `detectTini()` + `buildSpawnInvocation()` from `src/core/minions/spawn-helpers.ts` to wrap the worker subtree in tini-as-PID-1 when tini is on `PATH` (handles native-addon zombie reaping that the in-process SIGCHLD reaper can't reach). Exposes `isTiniDetected` read-only accessor for tests. **v0.34.3.0:** spawn-and-respawn loop extracted into the shared `ChildWorkerSupervisor` core (see entry below). MinionSupervisor now composes the inner class via `runSuperviseLoop()` → `new ChildWorkerSupervisor({...})` and maps `ChildSupervisorEvent` shapes back through the existing `emit()` SupervisorEvent channel — JSONL audit consumers see byte-compatible output across the rename. PID lock, signal handlers, health check, and `process.exit` on max-crashes stay in MinionSupervisor (standalone-daemon concerns). The pre-shipped reset-to-0-on-code=0 hunk that originally fixed the prod crash-counter incident is gone; the same fix lives in the shared core under the D1 amendment (code=0 leaves `crashCount` untouched, so a worker alternating real crashes + watchdog drains still trips `max_crashes`). D2 `cleanRestartBudget` (default 10 restarts per 60s) caps the macOS/non-Linux-fallback tight-loop by emitting `health_warn { reason: 'clean_restart_budget_exceeded' }` plus backoff after the threshold trips. `shutdown()` drains via `childSupervisor.killChild('SIGTERM')` + `awaitChildExit(35_000)` instead of reaching into `this.child` directly. Pinned by `test/supervisor.test.ts` (16 cases; existing tests that previously relied on clean-exit-as-crash semantics now use exit-1 workers since clean exits no longer count) and `test/supervisor-tini.test.ts`. -- `src/core/minions/child-worker-supervisor.ts` (v0.34.3.0) — shared spawn-and-respawn core extracted from `MinionSupervisor` so it can be reused by both `MinionSupervisor` (standalone `gbrain jobs supervisor` daemon) and `src/commands/autopilot.ts` (autopilot daemon). Pre-v0.34.3.0 the two consumers maintained parallel spawn loops that drifted into the same bug class — Codex caught it during plan-eng-review on PR #1003. Pure class: NO PID file, NO signal handlers, NO `process.exit`, NO health check. Lifecycle events fire via injected `onEvent: (ChildSupervisorEvent) => void` callback so each composer routes to its own log/audit channel. **D1 exit classifier:** `code === 0` leaves `crashCount` UNCHANGED (preserves flap detection across mixed exit sequences — a worker that alternates `exit 1 / exit 0 / exit 1 / exit 0` correctly trips `max_crashes` after 10 real crashes regardless of intervening clean exits). `code != 0` follows the existing `runDuration > stableRunResetMs ? 1 : ++crashCount` rule. **D2 clean-restart budget:** sliding window tracks code=0 exits; when count exceeds `cleanRestartBudget` (default 10) inside `cleanRestartWindowMs` (default 60s), emits `health_warn { reason: 'clean_restart_budget_exceeded' }` and applies `cleanRestartBudgetBackoffMs` (default 1s) before the next spawn. Caps the worst-case tight-loop on macOS / restricted containers / kernel <4.5 where the worker's RSS watchdog falls back to VmRSS. Public read-only accessors `childAlive`, `inBackoff`, `crashCount` for composer health checks; `killChild(signal)` + `awaitChildExit(timeoutMs)` for shutdown paths. `awaitChildExit` short-circuits when `child.exitCode !== null || child.signalCode !== null` (regression caught in pre-landing /review: pre-fix, fast-SIGTERM responders caused a 35-second shutdown hang because the late `once('exit', ...)` listener never fired). Test hooks: `_backoffFloorMs` skips the real backoff curve, `_now` injects a fake clock. Pinned by `test/child-worker-supervisor.test.ts` (7 cases: D1 classifier with code=0 not counted, interleaved exits still trip max_crashes, stable-run + clean-exit interaction across faked 6-minute run, D2 budget triggers backoff + health_warn, budget config is per-instance, awaitChildExit short-circuit, event-shape regression). Plan that produced the design lives at `~/.claude/plans/this-is-a-real-sleepy-sketch.md`. +- `src/core/minions/child-worker-supervisor.ts` (v0.34.3.0) — shared spawn-and-respawn core extracted from `MinionSupervisor` so it can be reused by both `MinionSupervisor` (standalone `gbrain jobs supervisor` daemon) and `src/commands/autopilot.ts` (autopilot daemon). Pre-v0.34.3.0 the two consumers maintained parallel spawn loops that drifted into the same bug class — Codex caught it during plan-eng-review on PR #1003. Pure class: NO PID file, NO signal handlers, NO `process.exit`, NO health check. Lifecycle events fire via injected `onEvent: (ChildSupervisorEvent) => void` callback so each composer routes to its own log/audit channel. **D1 exit classifier:** `code === 0` leaves `crashCount` UNCHANGED (preserves flap detection across mixed exit sequences — a worker that alternates `exit 1 / exit 0 / exit 1 / exit 0` correctly trips `max_crashes` after 10 real crashes regardless of intervening clean exits). `code != 0` follows the existing `runDuration > stableRunResetMs ? 1 : ++crashCount` rule. **D2 clean-restart budget:** sliding window tracks code=0 exits; when count exceeds `cleanRestartBudget` (default 10) inside `cleanRestartWindowMs` (default 60s), emits `health_warn { reason: 'clean_restart_budget_exceeded' }` and applies `cleanRestartBudgetBackoffMs` (default 1s) before the next spawn. Caps the worst-case tight-loop on macOS / restricted containers / kernel <4.5 where the worker's RSS watchdog falls back to VmRSS. Public read-only accessors `childAlive`, `inBackoff`, `crashCount` for composer health checks; `killChild(signal)` + `awaitChildExit(timeoutMs)` for shutdown paths. `awaitChildExit` short-circuits when `child.exitCode !== null || child.signalCode !== null` (regression caught in pre-landing /review: pre-fix, fast-SIGTERM responders caused a 35-second shutdown hang because the late `once('exit', ...)` listener never fired). Test hooks: `_backoffFloorMs` skips the real backoff curve, `_now` injects a fake clock. Pinned by `test/child-worker-supervisor.test.ts` (7 cases: D1 classifier with code=0 not counted, interleaved exits still trip max_crashes, stable-run + clean-exit interaction across faked 6-minute run, D2 budget triggers backoff + health_warn, budget config is per-instance, awaitChildExit short-circuit, event-shape regression). Plan that produced the design lives at `~/.claude/plans/this-is-a-real-sleepy-sketch.md`. **v0.42.5.0 (#1678):** the exit classifier special-cases `WORKER_EXIT_RSS_WATCHDOG` — `likely_cause='rss_watchdog'`, and that exit does NOT bump `crashCount` (so it routes to its own breaker, not the generic crash path). A dedicated `_watchdogExitTimestamps` sliding window (mirroring `_cleanRestartTimestamps`) trips a loud `rss_watchdog_loop` health_warn naming the cap when N watchdog exits land inside the window — INDEPENDENT of the stable-run reset that hid the original 400×/day loop (a >5-min run that watchdog-drains would otherwise never trip `max_crashes`). New opts `watchdogLoopBudget` (3), `watchdogLoopWindowMs` (600000), `watchdogBackoffMs` (30000); `ChildSupervisorEvent` extended. `supervisor-audit.ts` adds `rss_watchdog` as a non-clean cause + its own `CrashSummary.by_cause` bucket (denylist preserved so future causes still route to `legacy`). - `src/core/minions/spawn-helpers.ts` (v0.28.1) — pure `detectTini()` + `buildSpawnInvocation()` helpers consumed by both `supervisor.ts` and `autopilot.ts`. Resolves the DRY violation between the two spawn sites and makes the tini wrapping testable without `mock.module()` (rule R2 of `scripts/check-test-isolation.sh`). `detectTini()` calls `execFileSync('which', ['tini'])` with explicit `env: process.env` so Bun sees runtime PATH mutations (the env-snapshot bug fix). `buildSpawnInvocation(tiniPath, cmd, args)` returns `{cmd, args}` with tini prepended when present, or the bare invocation otherwise. Pinned by `test/spawn-helpers.test.ts` (5 cases) and `test/supervisor-tini.test.ts` (4 cases). - `src/core/minions/types.ts` — `MinionJobInput` + `MinionJobStatus` + handler context types. `MinionJobInput.max_stalled` (new in v0.14.1) is optional; omitted values let the schema DEFAULT (5) kick in, provided values are clamped to `[1, 100]`. - `src/core/minions/protected-names.ts` — side-effect-free constant module exporting `PROTECTED_JOB_NAMES` + `isProtectedJobName()`. Kept pure so queue core can import without loading handler modules. @@ -450,7 +453,7 @@ strict behavior when unset. - `src/core/transcripts.ts` (v0.29) — `listRecentTranscripts(engine, opts)` library reused by both the `gbrain transcripts recent` CLI and the `get_recent_transcripts` MCP op. Reads `dream.synthesize.session_corpus_dir` + `dream.synthesize.meeting_transcripts_dir` config keys (same as `discoverTranscripts`); walks for `.txt` files within `days`; applies `isDreamOutput` guard from `transcript-discovery.ts` (skips dream-generated files); returns `{path, date, mtime, length, summary}[]` sorted newest-first. Summary mode (default true) returns first non-empty line + ~250 trailing chars. Full mode caps at 100KB/file. Missing/non-existent corpus dirs return `[]`, not error. **Trust gate lives in the op handler, not here**: the op throws `permission_denied` for `ctx.remote === true`; this library is a trusted library function used by both the gated op and the local CLI. - `src/core/operations-descriptions.ts` (v0.29) — Constants module for tool descriptions. Pinned via `test/operations-descriptions.test.ts`. Houses `GET_RECENT_SALIENCE_DESCRIPTION`, `FIND_ANOMALIES_DESCRIPTION`, `GET_RECENT_TRANSCRIPTS_DESCRIPTION` plus the redirect-edited `LIST_PAGES_DESCRIPTION`, `QUERY_DESCRIPTION`, `SEARCH_DESCRIPTION`. Stable surface for the Tier-2 LLM routing eval — extracting them keeps the test from binding to whatever was in `operations.ts` at test-run time. - `src/core/cycle/transcript-discovery.ts` (v0.23) — Pure filesystem walk for synthesize. `discoverTranscripts(opts)` filters `.txt` files by date range, min_chars, and word-boundary regex `excludePatterns` (Q-3: `medical` matches "medical advice" but NOT "comedical"; power users may pass full regex). `readSingleTranscript(path)` is the `gbrain dream --input ` ad-hoc path. **v0.23.2 self-consumption guard:** `DREAM_OUTPUT_MARKER_RE` (anchored at frontmatter open `---\n`, optional BOM + CRLF tolerance, scans first 2000 chars for `dream_generated: true` with case-insensitive value and word boundary on `true`) drives `isDreamOutput(content, bypass=false)`. Both `discoverTranscripts` and `readSingleTranscript` skip matching files and emit a `[dream] skipped : dream_generated marker` stderr log (no more silent skips). `bypassGuard?: boolean` on `DiscoverOpts` and `readSingleTranscript`'s opts disables the guard for the explicit `--unsafe-bypass-dream-guard` escape hatch only — never auto-applied for `--input`. Replaces v0.23.1's `DREAM_OUTPUT_SLUGS` content-prefix list. -- `src/commands/dream.ts` — v0.17 `gbrain dream` CLI; ~80-line thin alias over `runCycle`. brainDir resolution requires explicit `--dir` OR `sync.repo_path` config. Flags: `--dry-run`, `--json`, `--phase `, `--pull`, `--dir `. **v0.23 added** `--input ` (ad-hoc transcript, implies `--phase synthesize`), `--date YYYY-MM-DD`, `--from --to ` (backfill range). Conflict detection: `--input` + `--date` exits 2. ISO date validation. `--dry-run` runs Haiku significance verdict but skips Sonnet synthesis (codex finding #8 — NOT zero LLM calls). Exit code 1 on status=failed. **v0.23.2 added** `--unsafe-bypass-dream-guard` (long-form intentional, plumbed through `runCycle.synthBypassDreamGuard` → `SynthesizePhaseOpts.bypassDreamGuard` → `discoverTranscripts({bypassGuard})` and `readSingleTranscript({bypassGuard})`). Loud stderr warning fires at synthesize-phase entry when set. Never auto-applied for `--input` so any caller can't silently re-trigger the loop bug. **v0.41.38.0:** `resolveBrainDir` now returns `string | null` (order: `--dir` → the resolved `--source`'s `local_path` → global `sync.repo_path` → null) instead of `process.exit(1)` on "No brain directory found". A checkout-less postgres/Supabase brain runs the DB-only phases (incl. `resolve_symbol_edges`, the call-graph builder) and skips the 6 filesystem phases with `details.reason: 'no_brain_dir'`; `runDream` owns the only hard error (no checkout AND no engine). When `--source` resolves but that source has no on-disk checkout, returns null (DB-only) rather than borrowing a different source's global `sync.repo_path` (would mix scopes — codex P1 review). Pinned by `test/dream-postgres.serial.test.ts`. +- `src/commands/dream.ts` — v0.17 `gbrain dream` CLI; ~80-line thin alias over `runCycle`. brainDir resolution requires explicit `--dir` OR `sync.repo_path` config. Flags: `--dry-run`, `--json`, `--phase `, `--pull`, `--dir `. **v0.23 added** `--input ` (ad-hoc transcript, implies `--phase synthesize`), `--date YYYY-MM-DD`, `--from --to ` (backfill range). Conflict detection: `--input` + `--date` exits 2. ISO date validation. `--dry-run` runs Haiku significance verdict but skips Sonnet synthesis (codex finding #8 — NOT zero LLM calls). Exit code 1 on status=failed. **v0.23.2 added** `--unsafe-bypass-dream-guard` (long-form intentional, plumbed through `runCycle.synthBypassDreamGuard` → `SynthesizePhaseOpts.bypassDreamGuard` → `discoverTranscripts({bypassGuard})` and `readSingleTranscript({bypassGuard})`). Loud stderr warning fires at synthesize-phase entry when set. Never auto-applied for `--input` so any caller can't silently re-trigger the loop bug. **v0.41.38.0:** `resolveBrainDir` now returns `string | null` (order: `--dir` → the resolved `--source`'s `local_path` → global `sync.repo_path` → null) instead of `process.exit(1)` on "No brain directory found". A checkout-less postgres/Supabase brain runs the DB-only phases (incl. `resolve_symbol_edges`, the call-graph builder) and skips the 6 filesystem phases with `details.reason: 'no_brain_dir'`; `runDream` owns the only hard error (no checkout AND no engine). When `--source` resolves but that source has no on-disk checkout, returns null (DB-only) rather than borrowing a different source's global `sync.repo_path` (would mix scopes — codex P1 review). Pinned by `test/dream-postgres.serial.test.ts`. **v0.42.5.0 (#1678):** new `--drain [--window ]` for `--phase extract_atoms` — `runDrain()` bypasses the pack-gate and runs the single-hold bounded drain from `src/core/cycle/extract-atoms-drain.ts` under the same `cycleLockIdFor(sourceId)` the routine cycle uses (so a concurrent autopilot tick defers with `cycle_already_running`), reporting `{extracted, skipped, remaining}`. Exits `EXIT_DRAIN_INCOMPLETE=3` while `remaining > 0` so a cron/agent loop knows to continue. A null backlog count (the count query FAILED) is also treated as incomplete (exit 3), never as a drained success (Codex pre-landing #4). `LockUnavailableError` → `cycle_already_running` skip (also exit 3). The `extract_atoms_backlog` doctor check (`computeExtractAtomsBacklogCheck`) surfaces the silent pack-gated backlog with the exact `--drain` command; pack-gated cycle skips carry a greppable `pack_gated:true` marker. - `src/commands/friction.ts` + `src/core/friction.ts` (v0.23) — `gbrain friction {log,render,list,summary}` reporter. Append-only JSONL under `$GBRAIN_HOME/friction/.jsonl`. Schema is a flat extension of `StructuredAgentError` (D20). Render groups by severity → phase, defaults to `--redact` for md output (strips `$HOME`/`$CWD` to placeholders so reports paste safely in PRs). Run-id resolves from `--run-id` > `$GBRAIN_FRICTION_RUN_ID` > `standalone.jsonl`. Skills the claw-test exercises gain a `_friction-protocol.md` callout so agents know when to log friction. - `src/commands/claw-test.ts` + `src/core/claw-test/` (v0.23) — `gbrain claw-test [--scenario ] [--live --agent openclaw]`. End-to-end "fresh user" friction harness. Two modes: scripted (CI gate, agent-free) and live (real openclaw subprocess, $1–2 in tokens). Sets `GBRAIN_HOME=` for hermeticity and captures gbrain's `--progress-json` events from each child's stderr to verify expected phases ran (`import.files`, `extract.links_fs`, `doctor.db_checks`). Phases for scripted mode: setup → install_brain (`gbrain init --pglite`) → import (`--no-embed`) → query → extract → verify (`gbrain doctor --json`, asserts `status: 'ok'`) → render. Live mode hands `BRIEF.md` from `test/fixtures/claw-test-scenarios//` to the agent runner. v1 ships with the OpenClaw runner only (`src/core/claw-test/runners/openclaw.ts`, invokes `openclaw agent --local --agent --message `); hermes runner deferred to v1.1. Transcript capture (`transcript-capture.ts`) uses `fs.createWriteStream` with `'drain'`-event backpressure — D17 fix for the 256KB-burst child-stall scenario. v0.18 upgrade scenario seeded via `seed-pglite.ts` SQL replay. - `skills/_friction-protocol.md` (v0.23) — shared cross-cutting convention skill (like `_brain-filing-rules.md`). Tells agents when to call `gbrain friction log` and how to choose a severity. Routes to friction CLI from any skill the claw-test exercises. diff --git a/package.json b/package.json index 9cecdf3c0..20d527907 100644 --- a/package.json +++ b/package.json @@ -142,5 +142,5 @@ "bun": ">=1.3.10" }, "license": "MIT", - "version": "0.42.4.0" + "version": "0.42.5.0" } diff --git a/src/commands/autopilot.ts b/src/commands/autopilot.ts index feb98124d..4d1a470e2 100644 --- a/src/commands/autopilot.ts +++ b/src/commands/autopilot.ts @@ -191,12 +191,16 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) { if (spawnManagedWorker) { const cliPath = resolveGbrainCliPath(); - // Inject the RSS watchdog default (2048 MB) for the autopilot-supervised - // worker. Bare `gbrain jobs work` has no default; the supervisor and - // autopilot are the production paths that opt in. + // Cgroup-aware auto-sized RSS watchdog cap (issue #1678). The old flat + // 2048MB killed legit embed work (~10GB) on every cycle → silent + // ~400×/24h respawn loop. resolveDefaultMaxRssMb clamps 0.5×min(cgroup, + // RAM) to [4096,16384]. Bare `gbrain jobs work` resolves the same default; + // we pass it explicitly so the spawn log + child agree. + const { resolveDefaultMaxRssMb } = await import('../core/minions/rss-default.ts'); + const autopilotMaxRssMb = resolveDefaultMaxRssMb(); childSupervisor = new ChildWorkerSupervisor({ cliPath, - args: ['jobs', 'work', '--max-rss', '2048'], + args: ['jobs', 'work', '--max-rss', String(autopilotMaxRssMb)], // process.env clone; autopilot doesn't gate shell jobs the way the // standalone supervisor does (autopilot is the operator-trust path). env: { ...process.env }, @@ -212,7 +216,7 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) { // existing logs see the same lines. if (event.kind === 'worker_spawned') { console.log( - `[autopilot] Minions worker spawned (pid: ${event.pid}, watchdog: 2048MB${event.tini ? ', tini: active' : ''})`, + `[autopilot] Minions worker spawned (pid: ${event.pid}, watchdog: ${autopilotMaxRssMb}MB${event.tini ? ', tini: active' : ''})`, ); } else if (event.kind === 'worker_spawn_failed') { console.error( diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index f51216464..ed665a7dd 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -2526,6 +2526,76 @@ export async function computeConversationFactsBacklogCheck( } } +/** + * issue #1678 — extract_atoms_backlog doctor check. + * + * Closes the "silent backlog" gap: extract_atoms is pack-gated, so on a brain + * whose active pack doesn't declare the phase it NEVER runs in the routine + * cycle and pages accumulate forever with zero signal (the cycle reports a + * clean `skipped`). This check counts the eligible-but-unextracted pages and, + * when the pack doesn't run the phase AND the backlog is real, WARNs with the + * exact `--drain` command. + * + * PAGE-BACKLOG-ONLY (Codex #11): extract_atoms also discovers transcript files + * at runtime; this counts DB pages only — labeled in details. No + * synthesize_concepts sibling this wave (Codex #12: that phase is a stub with + * no real eligibility predicate; a check would be a fake signal). + */ +export async function computeExtractAtomsBacklogCheck( + engine: BrainEngine, +): Promise { + const name = 'extract_atoms_backlog'; + const approx = 'page backlog only; transcript corpus not counted'; + try { + const { countExtractAtomsBacklog } = await import('../core/cycle/extract-atoms.ts'); + const backlog = await countExtractAtomsBacklog(engine); // brain-wide + if (backlog === null) { + return { name, status: 'warn', message: 'backlog query failed (could not count eligible pages)' }; + } + + const { packDeclaresPhase } = await import('../core/cycle.ts'); + let declared = false; + try { declared = await packDeclaresPhase(engine, 'extract_atoms'); } catch { declared = false; } + + if (backlog === 0) { + return { + name, status: 'ok', + message: 'no pages awaiting atom extraction', + details: { backlog, pack_declares_phase: declared, known_approximation: approx }, + }; + } + + // The incident: pack does NOT run the phase but a real backlog exists → + // it will grow forever without a signal. WARN with the drain command. + if (!declared && backlog > 10) { + const fix = 'gbrain dream --phase extract_atoms --drain --window 120 (or declare extract_atoms in your active schema pack)'; + return { + name, status: 'warn', + message: `${backlog} pages eligible for atom extraction but the active pack does not run extract_atoms — backlog growing. Fix: ${fix}`, + details: { backlog, pack_declares_phase: false, fix_hint: fix, known_approximation: approx }, + }; + } + + if (declared) { + // Pack runs it; the routine cycle drains in bounded batches. Informational. + return { + name, status: 'ok', + message: `${backlog} page(s) pending; active pack runs extract_atoms each cycle`, + details: { backlog, pack_declares_phase: true, known_approximation: approx }, + }; + } + + // Not declared but below the warn threshold. + return { + name, status: 'ok', + message: `${backlog} page(s) eligible (below warn threshold; pack does not run extract_atoms)`, + details: { backlog, pack_declares_phase: false, known_approximation: approx }, + }; + } catch (err) { + return { name, status: 'warn', message: `extract_atoms_backlog check failed: ${(err as Error).message}` }; + } +} + /** * v0.42 — extract_health doctor check. * @@ -3607,6 +3677,18 @@ export async function buildChecks( } } + // 3d.2b issue #1678 — extract_atoms_backlog. Surfaces the silent + // pack-gated-phase backlog: when the active pack doesn't run extract_atoms + // but eligible pages pile up, WARN with the `--drain` command. OK when the + // pack runs the phase (routine cycle drains it) or there's no backlog. + if (engine) { + try { + checks.push(await computeExtractAtomsBacklogCheck(engine)); + } catch { + // Best-effort; backlog query failure shouldn't stop doctor. + } + } + // 3d.3 v0.41.13.0 — conversation_format_coverage. Scans up to 200 // most-recent conversation-type pages, runs parseConversation in // dry mode, reports per-pattern hit counts + unmatched count. Warn diff --git a/src/commands/dream.ts b/src/commands/dream.ts index 28b4b9463..62ff3d826 100644 --- a/src/commands/dream.ts +++ b/src/commands/dream.ts @@ -27,6 +27,7 @@ import type { BrainEngine } from '../core/engine.ts'; import { runCycle, ALL_PHASES, + cycleLockIdFor, type CyclePhase, type CycleReport, } from '../core/cycle.ts'; @@ -66,9 +67,22 @@ interface DreamArgs { * until a follow-up CLI cleanup picks one. Supersedes PR #1559. */ source: string | null; + /** + * issue #1678: bounded single-hold backlog drain. `--drain` (currently only + * for `--phase extract_atoms`) holds the cycle lock once and loops bounded + * batches, rediscovering eligibility each batch, until the backlog empties or + * `--window` seconds elapse. Reports {extracted, skipped, remaining}; exits + * non-zero when remaining > 0 so a cron/agent loop knows to run again. + */ + drain: boolean; + /** Drain wallclock budget in seconds. Default 300 (5 min). */ + windowSeconds: number; } const ISO_DATE_RE = /^\d{4}-\d{2}-\d{2}$/; +const DEFAULT_DRAIN_WINDOW_SECONDS = 300; +/** Exit code for "drain ran but the backlog isn't empty — run again". */ +const EXIT_DRAIN_INCOMPLETE = 3; /** * Collect every occurrence of `-- ` in argv. Used to @@ -179,6 +193,28 @@ function parseArgs(args: string[]): DreamArgs { } const source = uniqSource[0] ?? uniqSourceId[0] ?? null; + // issue #1678: --drain [--window ]. Only extract_atoms is drainable + // this wave (it has a real eligibility predicate; synthesize_concepts does + // not — Codex #12). --drain with no --phase defaults to extract_atoms. + const drain = args.includes('--drain'); + const windowIdx = args.indexOf('--window'); + let windowSeconds = DEFAULT_DRAIN_WINDOW_SECONDS; + if (windowIdx !== -1) { + const raw = args[windowIdx + 1]; + if (raw === undefined || !/^\d+$/.test(raw.trim()) || parseInt(raw, 10) <= 0) { + console.error(`--window must be a positive integer (seconds); got "${raw}"`); + process.exit(2); + } + windowSeconds = parseInt(raw, 10); + } + if (drain) { + if (!phase) phase = 'extract_atoms'; + else if (phase !== 'extract_atoms') { + console.error(`--drain currently supports only --phase extract_atoms (got "${phase}")`); + process.exit(2); + } + } + return { json: args.includes('--json'), dryRun: args.includes('--dry-run'), @@ -192,6 +228,8 @@ function parseArgs(args: string[]): DreamArgs { to, bypassDreamGuard: args.includes('--unsafe-bypass-dream-guard'), source, + drain, + windowSeconds, }; } @@ -294,6 +332,16 @@ Options: --from YYYY-MM-DD Backfill range start (use with --to). --to YYYY-MM-DD Backfill range end. + --drain Bounded backlog drain for --phase extract_atoms + (the default phase when --drain is set). Holds the + cycle lock once, processes batches until the backlog + empties or --window elapses, reports {extracted, + remaining}, and exits 3 when the backlog isn't empty + so a cron/agent loop knows to run again. Use this to + grind down an extract_atoms backlog on a brain whose + pack doesn't run the phase in the routine cycle. + --window Drain wallclock budget. Default 300 (5 min). + --unsafe-bypass-dream-guard Disable the self-consumption guard. Use only when you know the input file is NOT dream-cycle output but the @@ -392,6 +440,86 @@ function isResolverUserError(e: unknown): boolean { || m.startsWith('Invalid GBRAIN_SOURCE value'); } +/** + * issue #1678 — bounded single-hold extract_atoms drain (see DreamArgs.drain). + * Holds the cycle lock once (same id the routine cycle uses for this source), + * loops bounded batches rediscovering eligibility, reports remaining, exits + * EXIT_DRAIN_INCOMPLETE when the backlog isn't empty so a loop knows to retry. + */ +async function runDrain( + engine: BrainEngine, + opts: DreamArgs, + 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 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) { + const remaining = await countExtractAtomsBacklog(engine, extractionSourceId); + if (opts.json) { + console.log(JSON.stringify({ phase: 'extract_atoms', status: 'ok', dry_run: true, extracted: 0, skipped: 0, remaining, batches: 0, stopped: 'window' }, null, 2)); + } else { + console.log(`[drain] dry-run: ${remaining ?? '?'} page(s) eligible for atom extraction (no work done)`); + } + // null = the backlog count query FAILED — treat as incomplete, never as + // "drained" (Codex: `remaining ?? 0` would exit 0 on a failed count and + // make automation believe the backlog cleared when it was never verified). + if (remaining === null || remaining > 0) process.exit(EXIT_DRAIN_INCOMPLETE); + return; + } + + 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`); + }, + }, + { windowMs: opts.windowSeconds * 1000 }, + ); + } catch (e) { + if (e instanceof LockUnavailableError) { + if (opts.json) { + console.log(JSON.stringify({ phase: 'extract_atoms', status: 'skipped', reason: 'cycle_already_running' }, null, 2)); + } else { + console.log('[drain] skipped: another cycle holds the lock (cycle_already_running) — run again shortly'); + } + process.exit(EXIT_DRAIN_INCOMPLETE); + } + throw e; + } + + if (opts.json) { + console.log(JSON.stringify(result, null, 2)); + } else { + console.log(`[drain] extracted ${result.extracted} atom(s) across ${result.batches} batch(es); ${result.remaining ?? '?'} remaining (stopped: ${result.stopped})`); + } + // null remaining = the final count query failed; do not report success. + if (result.remaining === null || result.remaining > 0) process.exit(EXIT_DRAIN_INCOMPLETE); +} + export async function runDream(engine: BrainEngine | null, args: string[]): Promise { const opts = parseArgs(args); @@ -459,6 +587,15 @@ export async function runDream(engine: BrainEngine | null, args: string[]): Prom ); process.exit(1); } + // ─── issue #1678: bounded single-hold extract_atoms drain ────────── + if (opts.drain) { + if (engine === null) { + console.error('gbrain dream --drain requires a connected brain (no engine available)'); + process.exit(1); + } + return runDrain(engine, opts, resolvedSourceId, brainDir); + } + const phases: CyclePhase[] | undefined = opts.phase ? [opts.phase] : undefined; const report = await runCycle(engine, { diff --git a/src/commands/jobs.ts b/src/commands/jobs.ts index 6df8e7e34..55a87c541 100644 --- a/src/commands/jobs.ts +++ b/src/commands/jobs.ts @@ -6,6 +6,7 @@ import type { BrainEngine } from '../core/engine.ts'; import { MinionQueue } from '../core/minions/queue.ts'; import { MinionWorker } from '../core/minions/worker.ts'; +import { WORKER_EXIT_RSS_WATCHDOG } from '../core/minions/worker-exit-codes.ts'; import type { MinionJob, MinionJobStatus } from '../core/minions/types.ts'; import { loadConfig, isThinClient } from '../core/config.ts'; import { callRemoteTool, unpackToolResult } from '../core/mcp-client.ts'; @@ -778,11 +779,14 @@ HANDLER TYPES (built in) const queueName = parseFlag(args, '--queue') ?? 'default'; const concurrency = resolveWorkerConcurrency(args); - // --max-rss defaults to 2048 for bare workers (matching supervisor default). - // This catches memory-leak stalls that previously went undetected without - // a supervisor. Operators can opt out with `--max-rss 0`. + // --max-rss: explicit value wins (including 0 to disable the watchdog). + // Absent → cgroup-aware auto-size (issue #1678): the flat 2048MB default + // killed legit embed work (~10GB) on every cycle and produced a silent + // ~400×/24h respawn loop. See src/core/minions/rss-default.ts. const maxRssExplicit = parseMaxRssFlag(args); - const maxRssMb = maxRssExplicit ?? 2048; + const { resolveDefaultMaxRssMb, describeDefaultMaxRss } = + await import('../core/minions/rss-default.ts'); + const maxRssMb = maxRssExplicit ?? resolveDefaultMaxRssMb(); // --health-interval: self-health-check period in ms. 0 disables. Default: 60_000 (60s). // Provides DB liveness probes + stall detection for bare workers. @@ -836,7 +840,15 @@ HANDLER TYPES (built in) }); const isSupervisedChild = process.env.GBRAIN_SUPERVISED === '1'; - const watchdogNote = maxRssMb > 0 ? `, watchdog: ${maxRssMb}MB` : ''; + let watchdogNote = ''; + if (maxRssMb > 0) { + if (maxRssExplicit !== undefined) { + watchdogNote = `, watchdog: ${maxRssMb}MB (explicit)`; + } else { + const d = describeDefaultMaxRss(); + watchdogNote = `, watchdog: ${maxRssMb}MB (auto-sized from ${Math.round(d.basisMb / 1024)}GB ${d.source} RAM)`; + } + } const healthNote = !isSupervisedChild && healthCheckInterval > 0 ? `, health-check: ${Math.round(healthCheckInterval / 1000)}s` : ''; @@ -856,6 +868,18 @@ HANDLER TYPES (built in) // tests in earlier waves of this branch. try { await engine.disconnect(); } catch (e) { console.error('[gbrain jobs work] engine disconnect failed during shutdown:', e); } + + // If the RSS watchdog (not a normal SIGTERM) drained the worker, exit + // with the distinct WORKER_EXIT_RSS_WATCHDOG code so the supervisor + // classifies the drain as `rss_watchdog` (cause-keyed backoff + loud + // alert) instead of a silent `clean_exit`. The worker exposes the + // intent; the CLI owns process.exit (same ownership boundary as the + // engine-disconnect above). Explicit process.exit also guarantees the + // code even if a lingering handle would otherwise keep the process + // alive past natural exit (issue #1678, Codex #7). + if (worker.rssWatchdogTriggered) { + process.exit(WORKER_EXIT_RSS_WATCHDOG); + } } break; } @@ -1021,9 +1045,13 @@ HANDLER TYPES (built in) const allowShellJobs = hasFlag(args, '--allow-shell-jobs') || !!process.env.GBRAIN_ALLOW_SHELL_JOBS; const detach = hasFlag(args, '--detach'); - // Supervisor defaults --max-rss 2048 (MB) — main production path uses - // the supervisor, so the watchdog is on by default here. - const maxRssMb = parseMaxRssFlag(args) ?? 2048; + // Supervisor's --max-rss: explicit wins; absent → cgroup-aware auto-size + // (issue #1678). The supervisor is the main production path, so the + // watchdog is on by default — but at a realistic, RAM-relative cap + // instead of the old flat 2048MB footgun. + const { resolveDefaultMaxRssMb: resolveSupMaxRss } = + await import('../core/minions/rss-default.ts'); + const maxRssMb = parseMaxRssFlag(args) ?? resolveSupMaxRss(); const cliPath = parseFlag(args, '--cli-path') ?? resolveGbrainCliPath(); @@ -1207,7 +1235,9 @@ export async function registerBuiltinHandlers(worker: MinionWorker, engine: Brai worker.register('lint', async (job) => { const { runLintCore } = await import('./lint.ts'); const target = typeof job.data.dir === 'string' ? job.data.dir : '.'; - const result = await runLintCore({ target, fix: !!job.data.fix, dryRun: !!job.data.dryRun }); + // issue #1678: reuse the worker's live engine for lint's content-sanity + // DB lift so it doesn't create + disconnect a competing engine. + const result = await runLintCore({ target, fix: !!job.data.fix, dryRun: !!job.data.dryRun, engine }); return result; }); @@ -1258,7 +1288,8 @@ export async function registerBuiltinHandlers(worker: MinionWorker, engine: Brai worker.register('lint-fix', async (job) => { const { runLintCore } = await import('./lint.ts'); const target = typeof job.data.dir === 'string' ? job.data.dir : '.'; - return await runLintCore({ target, fix: true, dryRun: false }); + // issue #1678: reuse the worker's live engine (see 'lint' handler). + return await runLintCore({ target, fix: true, dryRun: false, engine }); }); worker.register('integrity-auto', async () => { diff --git a/src/commands/lint.ts b/src/commands/lint.ts index 521103c39..40ea4c9f4 100644 --- a/src/commands/lint.ts +++ b/src/commands/lint.ts @@ -26,6 +26,7 @@ import { } from '../core/content-sanity.ts'; import { loadOperatorLiterals } from '../core/content-sanity-literals.ts'; import { loadConfig, loadConfigWithEngine, gbrainPath } from '../core/config.ts'; +import type { BrainEngine } from '../core/engine.ts'; export interface LintIssue { file: string; @@ -295,32 +296,53 @@ export function fixContent(content: string): string { * Also loads the operator literals file (`~/.gbrain/junk-substrings.txt`) * once per lint invocation so multi-file lint runs amortize the read. */ -async function resolveLintContentSanity(): Promise { +async function resolveLintContentSanity( + sharedEngine?: BrainEngine, +): Promise { const base = loadConfig(); let cs = base?.content_sanity; - // DB-plane lift: only attempt when the file/env config suggests an - // engine is configured. Avoids spinning up a fresh PGLite just to - // read 4 config keys in a CI lint run that has no brain at all. - const hasEngineConfig = !!(base?.database_url || base?.database_path); - if (hasEngineConfig) { + // DB-plane lift. issue #1678: when the caller already holds a live engine + // (the cycle's lint phase, the Minion lint handler), REUSE it — do NOT + // create + disconnect our own. A self-created engine here is module-style + // (createEngine without poolSize wraps the db.ts singleton), so its + // disconnect() cascades to db.disconnect() and NULLS the shared singleton + // mid-cycle — which broke every subsequent cycle phase with a misleading + // "connect() has not been called". Reusing the live engine reads the same + // 4 config keys with zero connection churn. + if (sharedEngine) { try { - const { createEngine } = await import('../core/engine-factory.ts'); - const engine = await createEngine({ - engine: base!.engine, - database_url: base!.database_url, - database_path: base!.database_path, - }); - try { - await engine.connect({}); - const lifted = await loadConfigWithEngine(engine, base); - cs = lifted?.content_sanity ?? cs; - } finally { - await engine.disconnect().catch(() => { /* best-effort cleanup */ }); - } + const lifted = await loadConfigWithEngine(sharedEngine, base); + cs = lifted?.content_sanity ?? cs; } catch { - // Engine unreachable or failed mid-probe — fall through to - // file/env values. Lint should never block on engine state. + // best-effort; fall through to file/env values. + } + } else { + // Standalone path (CLI `gbrain lint`, which is CLI_ONLY and shares no + // engine): only attempt when the file/env config suggests an engine is + // configured. Avoids spinning up a fresh PGLite just to read 4 config + // keys in a CI lint run that has no brain at all. Safe to create + + // disconnect here because nothing else shares this process's singleton. + const hasEngineConfig = !!(base?.database_url || base?.database_path); + if (hasEngineConfig) { + try { + const { createEngine } = await import('../core/engine-factory.ts'); + const engine = await createEngine({ + engine: base!.engine, + database_url: base!.database_url, + database_path: base!.database_path, + }); + try { + await engine.connect({}); + const lifted = await loadConfigWithEngine(engine, base); + cs = lifted?.content_sanity ?? cs; + } finally { + await engine.disconnect().catch(() => { /* best-effort cleanup */ }); + } + } catch { + // Engine unreachable or failed mid-probe — fall through to + // file/env values. Lint should never block on engine state. + } } } @@ -361,6 +383,12 @@ export interface LintOpts { * `runLintCore` resolves via the file/env/DB chain. Tests inject * this directly to bypass the FS + engine layers. */ contentSanity?: LintContentOpts['contentSanity']; + /** issue #1678: a live, already-connected engine to REUSE for the + * content-sanity DB-plane config lift. Callers with a shared engine (the + * cycle lint phase, Minion lint handlers) MUST pass it so lint doesn't + * create + disconnect a competing module-style engine that nulls the + * shared db singleton mid-cycle. */ + engine?: BrainEngine; } export interface LintResult { @@ -392,7 +420,7 @@ export async function runLintCore(opts: LintOpts): Promise { // Resolve content-sanity config once for this lint run (D1: lift DB // config when reachable). Caller can pre-pass via opts.contentSanity // (tests, Minion handler) to bypass the engine probe entirely. - const contentSanity = opts.contentSanity ?? await resolveLintContentSanity(); + const contentSanity = opts.contentSanity ?? await resolveLintContentSanity(opts.engine); const lintOpts: LintContentOpts = { contentSanity }; let totalIssues = 0; diff --git a/src/core/cycle.ts b/src/core/cycle.ts index 6e8edb014..6b78ef424 100644 --- a/src/core/cycle.ts +++ b/src/core/cycle.ts @@ -700,10 +700,15 @@ function checkAborted(signal?: AbortSignal): void { // keyword is the minimal seam that lets behavioral tests drive the // wrapper's result-mapping (counter → status enum + summary) without // going through runCycle's full setup cost. -export async function runPhaseLint(brainDir: string, dryRun: boolean): Promise { +export async function runPhaseLint(brainDir: string, dryRun: boolean, engine?: BrainEngine | null): Promise { try { const { runLintCore } = await import('../commands/lint.ts'); - const result = await runLintCore({ target: brainDir, fix: true, dryRun }); + // issue #1678: pass the cycle's live engine so lint's content-sanity + // DB-plane lift REUSES it instead of creating + disconnecting a + // competing module-style engine that nulls the shared db singleton + // mid-cycle (which broke every phase after lint with a misleading + // "connect() has not been called"). + const result = await runLintCore({ target: brainDir, fix: true, dryRun, engine: engine ?? undefined }); const issues = result.total_issues ?? 0; const fixed = result.total_fixed ?? 0; const remaining = Math.max(0, issues - fixed); @@ -821,7 +826,7 @@ async function resolveSourceForDir( // Better to skip a pack-gated phase than to run it for a brain that // can't resolve its active pack. Skipped phases land in the cycle report // with `not_in_active_pack` so doctor can surface to the user. -async function packDeclaresPhase( +export async function packDeclaresPhase( engine: BrainEngine, phase: CyclePhase, ): Promise { @@ -1481,7 +1486,7 @@ export async function runCycle( phaseResults.push(skipNoBrainDir('lint')); } else { progress.start('cycle.lint'); - const { result, duration_ms } = await timePhase(() => runPhaseLint(brainDir, dryRun)); + const { result, duration_ms } = await timePhase(() => runPhaseLint(brainDir, dryRun, engine)); result.duration_ms = duration_ms; phaseResults.push(result); progress.finish(); @@ -1655,12 +1660,17 @@ export async function runCycle( details: { reason: 'no_database' }, }); } else if (!(await packDeclaresPhase(engine, 'extract_atoms'))) { + // issue #1678: the routine cycle skip stays cheap (no per-tick backlog + // count), but the detail is greppable — `pack_gated: true` lets the + // `extract_atoms_backlog` doctor check / log scrapers tell a + // deliberately-off phase apart from a phase that ran with no work. The + // backlog signal itself lives in doctor (one count, on demand). phaseResults.push({ phase: 'extract_atoms', status: 'skipped', duration_ms: 0, - summary: 'extract_atoms: active pack does not declare this phase', - details: { reason: 'not_in_active_pack' }, + summary: 'extract_atoms: active pack does not declare this phase (run `gbrain dream --phase extract_atoms --drain` to drain a backlog)', + details: { reason: 'not_in_active_pack', pack_gated: true }, }); } else { progress.start('cycle.extract_atoms'); @@ -1765,12 +1775,16 @@ export async function runCycle( details: { reason: 'no_database' }, }); } else if (!(await packDeclaresPhase(engine, 'synthesize_concepts'))) { + // issue #1678: same greppable marker as extract_atoms. (No doctor + // backlog check for synthesize_concepts this wave — Codex #12: that + // phase has no real eligibility predicate yet, so a check would be a + // fake signal. Filed as a follow-up.) phaseResults.push({ phase: 'synthesize_concepts', status: 'skipped', duration_ms: 0, summary: 'synthesize_concepts: active pack does not declare this phase', - details: { reason: 'not_in_active_pack' }, + details: { reason: 'not_in_active_pack', pack_gated: true }, }); } else { progress.start('cycle.synthesize_concepts'); diff --git a/src/core/cycle/extract-atoms-drain.ts b/src/core/cycle/extract-atoms-drain.ts new file mode 100644 index 000000000..0310313f6 --- /dev/null +++ b/src/core/cycle/extract-atoms-drain.ts @@ -0,0 +1,96 @@ +/** + * issue #1678 — bounded single-hold drain for extract_atoms. + * + * The operator/agent escape hatch for a backlog the routine cycle won't touch + * (pack-gated off) or can't keep up with. Design per Codex #8/#9/#10: + * + * - SINGLE continuous lock hold (no release/reacquire between batches). The + * caller wraps the loop in `withRefreshingLock(cycleLockIdFor(sourceId))` — + * the SAME lock id the routine cycle uses for that source — so the two + * genuinely contend (no source-vs-legacy lock mismatch) and there's no + * release-gap where autopilot/sync could mutate pages mid-drain (which would + * let the drain extract atoms from stale content). + * - REDISCOVER eligibility each batch (the injected `runBatch` re-runs the + * NOT-EXISTS-on-source_hash discovery), so stale content simply doesn't + * match — no cross-window cursor of page lists. + * - BOUNDED by a wallclock window; reports `remaining` so a cron/agent loop + * 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. + */ + +export interface ExtractAtomsDrainDeps { + /** + * Run the loop body while holding the cycle lock. Implemented by the caller + * via `withRefreshingLock`. MUST throw when the lock is held by another + * process (e.g. `LockUnavailableError`) — the drain lets that propagate so + * the caller can report `cycle_already_running` and exit, matching the + * routine cycle's skip contract. + */ + withLock: (work: () => Promise) => Promise; + /** Process one bounded batch (rediscovers eligibility). Returns counts. */ + runBatch: () => Promise<{ extracted: number; skipped: number }>; + /** Count remaining eligible-but-unextracted pages, or null on query error. */ + countRemaining: () => Promise; + /** Injectable clock. Production: Date.now. */ + now: () => number; + /** Optional progress sink (one line per batch). */ + onBatch?: (info: { batch: number; extracted: number; remaining: number | null }) => void; +} + +export interface ExtractAtomsDrainOpts { + /** Wallclock budget in ms. The loop stops after this elapses. */ + windowMs: number; + /** Hard cap on batches (belt-and-suspenders against a 0-progress loop). Default 1000. */ + maxBatches?: number; +} + +export interface ExtractAtomsDrainResult { + phase: 'extract_atoms'; + status: 'ok'; + extracted: number; + skipped: number; + /** Eligible pages still pending after the window. null if the count errored. */ + remaining: number | null; + /** Batches actually processed. */ + batches: number; + /** Why the loop stopped: drained | window | no_progress | max_batches. */ + stopped: 'drained' | 'window' | 'no_progress' | 'max_batches'; +} + +export async function runExtractAtomsDrain( + deps: ExtractAtomsDrainDeps, + opts: ExtractAtomsDrainOpts, +): Promise { + const maxBatches = opts.maxBatches ?? 1000; + return deps.withLock(async () => { + const deadline = deps.now() + opts.windowMs; + let extracted = 0; + let skipped = 0; + let batches = 0; + let stopped: ExtractAtomsDrainResult['stopped'] = 'window'; + + while (deps.now() < deadline) { + if (batches >= maxBatches) { stopped = 'max_batches'; break; } + + const before = await deps.countRemaining(); + if (before === 0) { stopped = 'drained'; break; } + + const r = await deps.runBatch(); + extracted += r.extracted; + skipped += r.skipped; + batches++; + deps.onBatch?.({ batch: batches, extracted: r.extracted, remaining: before }); + + // Stop if a batch made zero forward progress — extraction is failing or + // everything left is ineligible (e.g. all skipped). Prevents a hot loop + // that spends budget without draining. + if (r.extracted === 0 && r.skipped === 0) { stopped = 'no_progress'; break; } + } + + const remaining = await deps.countRemaining(); + if (remaining === 0) stopped = 'drained'; + return { phase: 'extract_atoms', status: 'ok', extracted, skipped, remaining, batches, stopped }; + }); +} diff --git a/src/core/cycle/extract-atoms.ts b/src/core/cycle/extract-atoms.ts index 2e4c7e3d6..f1e94bb06 100644 --- a/src/core/cycle/extract-atoms.ts +++ b/src/core/cycle/extract-atoms.ts @@ -219,6 +219,71 @@ export async function discoverExtractablePages( } } +/** + * issue #1678 (C4) — count DB pages eligible for atom extraction that have NO + * atom row yet. Single source of truth for the backlog number: the doctor + * `extract_atoms_backlog` check calls this so its definition can't drift from + * what the phase actually processes. Uses the SAME eligibility predicate as + * `discoverExtractablePages` (minus the LIMIT and affectedSlugs filter) so it + * rides migration v104's `pages_atom_source_hash_idx` partial index and stays + * O(log n) on 100K+ brains. + * + * PAGE-BACKLOG-ONLY (Codex #11): extract_atoms also discovers transcript files + * at runtime; this count covers DB pages only. Callers label that caveat. + * + * Fail-soft: returns null on error so the doctor check can report a warn + * (query failed) rather than a misleading 0. + */ +export async function countExtractAtomsBacklog( + engine: BrainEngine, + sourceId?: string, +): Promise { + try { + // Two modes: scoped (the phase's per-source `remaining`) vs brain-wide + // (doctor — matches the conversation-facts check's cross-source posture). + // The atom must live in the SAME source as the page either way, so the + // brain-wide form keys the NOT EXISTS on `atom.source_id = p.source_id`. + const scoped = sourceId !== undefined; + const sql = scoped + ? `SELECT COUNT(*) AS cnt FROM pages p + WHERE p.source_id = $1 + AND p.type = ANY($2::text[]) + AND p.deleted_at IS NULL + AND p.content_hash IS NOT NULL + AND COALESCE(p.frontmatter->>'imported_from', '') <> 'markdown-greenfield' + AND COALESCE(p.frontmatter->>'dream_generated', '') <> 'true' + AND length(COALESCE(p.compiled_truth, '')) >= $3 + AND NOT EXISTS ( + SELECT 1 FROM pages atom + WHERE atom.type = 'atom' AND atom.source_id = $1 + AND atom.frontmatter->>'source_hash' = substring(p.content_hash from 1 for 16) + AND atom.deleted_at IS NULL + )` + : `SELECT COUNT(*) AS cnt FROM pages p + WHERE p.type = ANY($1::text[]) + AND p.deleted_at IS NULL + AND p.content_hash IS NOT NULL + AND COALESCE(p.frontmatter->>'imported_from', '') <> 'markdown-greenfield' + AND COALESCE(p.frontmatter->>'dream_generated', '') <> 'true' + AND length(COALESCE(p.compiled_truth, '')) >= $2 + AND NOT EXISTS ( + SELECT 1 FROM pages atom + WHERE atom.type = 'atom' AND atom.source_id = p.source_id + AND atom.frontmatter->>'source_hash' = substring(p.content_hash from 1 for 16) + AND atom.deleted_at IS NULL + )`; + const params = scoped + ? [sourceId, EXTRACTABLE_PAGE_TYPES as unknown as string[], MIN_PAGE_CHARS_FOR_EXTRACTION] + : [EXTRACTABLE_PAGE_TYPES as unknown as string[], MIN_PAGE_CHARS_FOR_EXTRACTION]; + const rows = await engine.executeRaw<{ cnt: string | number }>(sql, params); + return Number(rows[0]?.cnt ?? 0); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + console.error(`[extract_atoms] backlog count failed: ${msg}`); + return null; + } +} + /** * Batch source-hash idempotency check. Returns the set of contentHash16 * values that already have an atom row for this source. One SQL diff --git a/src/core/doctor-categories.ts b/src/core/doctor-categories.ts index a801120e4..680794e21 100644 --- a/src/core/doctor-categories.ts +++ b/src/core/doctor-categories.ts @@ -73,6 +73,7 @@ export const BRAIN_CHECK_NAMES: ReadonlySet = new Set([ 'embedding_width_consistency', 'embeddings', 'eval_drift', + 'extract_atoms_backlog', 'extract_health', 'facts_embedding_width_consistency', 'facts_extraction_health', diff --git a/src/core/minions/child-worker-supervisor.ts b/src/core/minions/child-worker-supervisor.ts index 163e9392b..1c0009b86 100644 --- a/src/core/minions/child-worker-supervisor.ts +++ b/src/core/minions/child-worker-supervisor.ts @@ -40,6 +40,7 @@ import { spawn, type ChildProcess } from 'child_process'; import { buildSpawnInvocation, detectTini } from './spawn-helpers.ts'; import { classifyWorkerExit } from './exit-classification.ts'; import { calculateBackoffMs } from './supervisor.ts'; +import { WORKER_EXIT_RSS_WATCHDOG } from './worker-exit-codes.ts'; export type ChildSupervisorEvent = | { kind: 'worker_spawned'; pid: number; tini: boolean } @@ -56,11 +57,11 @@ export type ChildSupervisorEvent = kind: 'backoff'; ms: number; crashCount: number; - reason: 'clean_exit' | 'crash' | 'budget_exceeded'; + reason: 'clean_exit' | 'crash' | 'budget_exceeded' | 'rss_watchdog'; } | { kind: 'health_warn'; - reason: 'clean_restart_budget_exceeded'; + reason: 'clean_restart_budget_exceeded' | 'rss_watchdog_loop'; count: number; windowMs: number; }; @@ -91,6 +92,26 @@ export interface ChildWorkerSupervisorOpts { /** Backoff applied when budget is exceeded. Default 1 second. */ cleanRestartBudgetBackoffMs?: number; + /** + * v0.42.5.0 (issue #1678) — RSS-watchdog loop breaker, cause-keyed and + * INDEPENDENT of crashCount/max_crashes. A watchdog drain + * (WORKER_EXIT_RSS_WATCHDOG) means the worker hit its memory cap, not that + * the code is defective — so it does NOT count toward max_crashes (that + * would stop ALL job processing, worse than slow-looping). Instead we + * always apply `watchdogBackoffMs` (so an instant-OOM-on-startup can't + * hot-loop) and, when more than `watchdogLoopBudget` watchdog exits land + * inside `watchdogLoopWindowMs`, emit a loud `rss_watchdog_loop` health_warn + * so the operator sees "raise --max-rss" instead of chasing a phantom + * connection/lock failure. NOTE: the stable-run reset that defeats + * max_crashes for >5-min runs is exactly why a SEPARATE window is required + * here — routing watchdog exits through the crash path would never trip. + */ + watchdogLoopBudget?: number; + /** Sliding window for the watchdog-loop breaker. Default 10 minutes. */ + watchdogLoopWindowMs?: number; + /** Backoff applied after every watchdog drain. Default 30 seconds. */ + watchdogBackoffMs?: number; + /** * Test-only override: minimum backoff in ms between child respawns. * Tests pass `1` to make crash-loops finish in < 1s. Not exposed via CLI. @@ -114,6 +135,9 @@ const DEFAULTS = { cleanRestartBudget: 10, cleanRestartWindowMs: 60_000, cleanRestartBudgetBackoffMs: 1_000, + watchdogLoopBudget: 3, + watchdogLoopWindowMs: 10 * 60 * 1000, + watchdogBackoffMs: 30_000, } as const; export class ChildWorkerSupervisor { @@ -122,6 +146,9 @@ export class ChildWorkerSupervisor { private _crashCount = 0; private _lastExitCode: number | null = null; private _cleanRestartTimestamps: number[] = []; + /** Sliding window of RSS-watchdog exit timestamps (issue #1678). Separate + * from crashCount so the >5-min stable-run reset can't defeat the breaker. */ + private _watchdogExitTimestamps: number[] = []; private _child: ChildProcess | null = null; private _inBackoff = false; private _lastStartTime = 0; @@ -291,7 +318,20 @@ export class ChildWorkerSupervisor { // through the shared `classifyWorkerExit` helper so doctor.ts and // jobs.ts (audit-log consumers) read the same rule. this._lastExitCode = code; - if (classifyWorkerExit({ code }) === 'clean_exit') { + if (code === WORKER_EXIT_RSS_WATCHDOG) { + // issue #1678: RSS-watchdog drain. NOT a code defect — leave + // crashCount untouched so it never trips max_crashes (which would + // stop ALL job processing). Tracked in its own window so the + // breaker survives the >5-min stable-run reset that defeats the + // generic crash path. + const nowMs = this.now(); + this._watchdogExitTimestamps.push(nowMs); + const windowMs = this.opts.watchdogLoopWindowMs ?? DEFAULTS.watchdogLoopWindowMs; + const cutoff = nowMs - windowMs; + this._watchdogExitTimestamps = this._watchdogExitTimestamps.filter( + (t) => t > cutoff, + ); + } else if (classifyWorkerExit({ code }) === 'clean_exit') { const nowMs = this.now(); this._cleanRestartTimestamps.push(nowMs); const windowMs = this.opts.cleanRestartWindowMs ?? DEFAULTS.cleanRestartWindowMs; @@ -315,6 +355,8 @@ export class ChildWorkerSupervisor { likelyCause = 'oom_or_external_kill'; } else if (signal === 'SIGTERM') { likelyCause = 'graceful_shutdown'; + } else if (code === WORKER_EXIT_RSS_WATCHDOG) { + likelyCause = 'rss_watchdog'; } else if (code === 1) { likelyCause = 'runtime_error'; } else if (code === 0) { @@ -381,6 +423,42 @@ export class ChildWorkerSupervisor { return; } + // issue #1678: RSS-watchdog drain. Always back off (so an instant-OOM on + // startup can't hot-loop) and, when more than `watchdogLoopBudget` drains + // land inside the window, emit the loud `rss_watchdog_loop` alert. crashCount + // is untouched (the worker is fine; the cap is too low), so this branch + // never trips max_crashes — the workload keeps running, just paced + loud. + if (this._lastExitCode === WORKER_EXIT_RSS_WATCHDOG) { + const count = this._watchdogExitTimestamps.length; + const budget = this.opts.watchdogLoopBudget ?? DEFAULTS.watchdogLoopBudget; + const windowMs = this.opts.watchdogLoopWindowMs ?? DEFAULTS.watchdogLoopWindowMs; + if (count > budget) { + this.opts.onEvent({ + kind: 'health_warn', + reason: 'rss_watchdog_loop', + count, + windowMs, + }); + } + const watchdogBackoff = + this.opts._backoffFloorMs !== undefined + ? this.opts._backoffFloorMs + : this.opts.watchdogBackoffMs ?? DEFAULTS.watchdogBackoffMs; + this.opts.onEvent({ + kind: 'backoff', + ms: Math.round(watchdogBackoff), + crashCount: this._crashCount, + reason: 'rss_watchdog', + }); + this._inBackoff = true; + try { + await this.sleep(watchdogBackoff); + } finally { + this._inBackoff = false; + } + return; + } + // code != 0: exponential backoff scaled by crashCount-1 (retry-attempt // index). On first crash: crashCount=1, exponent=0 -> 1s. After stable- // run reset: crashCount=1 again -> 1s fresh cycle. diff --git a/src/core/minions/handlers/supervisor-audit.ts b/src/core/minions/handlers/supervisor-audit.ts index f72a91fb6..38ce47ef2 100644 --- a/src/core/minions/handlers/supervisor-audit.ts +++ b/src/core/minions/handlers/supervisor-audit.ts @@ -141,6 +141,11 @@ export interface CrashSummary { by_cause: { runtime_error: number; oom_or_external_kill: number; + /** v0.42.5.0: worker drained itself because RSS crossed the watchdog cap + * (issue #1678). A real problem (the cap is too low for the workload, or a + * leak) but distinct from an OOM-killer SIGKILL — surfaced as its own + * bucket so operators see "raise --max-rss" signal, not generic crashes. */ + rss_watchdog: number; unknown: number; legacy: number; }; @@ -185,7 +190,7 @@ export function isCrashExit(event: SupervisorEmission): boolean { export function summarizeCrashes(events: SupervisorEmission[]): CrashSummary { const summary: CrashSummary = { total: 0, - by_cause: { runtime_error: 0, oom_or_external_kill: 0, unknown: 0, legacy: 0 }, + by_cause: { runtime_error: 0, oom_or_external_kill: 0, rss_watchdog: 0, unknown: 0, legacy: 0 }, clean_exits: 0, }; for (const e of events) { @@ -198,6 +203,7 @@ export function summarizeCrashes(events: SupervisorEmission[]): CrashSummary { const cause = e.likely_cause as string | undefined; if (cause === 'runtime_error') summary.by_cause.runtime_error++; else if (cause === 'oom_or_external_kill') summary.by_cause.oom_or_external_kill++; + else if (cause === 'rss_watchdog') summary.by_cause.rss_watchdog++; else if (cause === 'unknown') summary.by_cause.unknown++; else summary.by_cause.legacy++; // pre-v0.34 fallback OR future unrecognized cause } diff --git a/src/core/minions/lock-renewal-tick.ts b/src/core/minions/lock-renewal-tick.ts index 1124a3987..f00226342 100644 --- a/src/core/minions/lock-renewal-tick.ts +++ b/src/core/minions/lock-renewal-tick.ts @@ -159,6 +159,17 @@ export interface LockRenewalDeps { * harmless — at worst we have a dangling reject that no one awaits). */ setTimeout: (cb: () => void, ms: number) => unknown; + /** + * issue #1678 (Codex #2): OPTIONAL pool rebuild. When a renewLock throw + * looks like a reaped / nulled connection, the tick calls this ONCE + * (bounded by callTimeoutMs) before returning `ok`, so the NEXT tick's + * renewLock hits a live pool. This is deliberately NOT a `withRetry` around + * renewLock — a background retry would outlive this tick's own timeout race + * 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. + */ + reconnect?: () => Promise; } /** @@ -235,6 +246,30 @@ export async function runLockRenewalTick( } catch { /* audit best-effort */ } return { kind: 'should_abort', reason: 'lock-renewal-failed' }; } + + // issue #1678 (Codex #2): not yet at the deadline, so we'll retry on the + // next tick. If the engine can rebuild its pool, do it ONCE now (bounded + // by callTimeoutMs) so the next renewLock sees a live connection instead + // of throwing the same reaped-socket error until the deadline. Best-effort: + // a reconnect throw/timeout is swallowed (next tick retries) and must NEVER + // escape this catch — that would re-introduce the unhandledRejection class + // this module was built to close. + if (deps.reconnect) { + const reconnect = deps.reconnect; + try { + await Promise.race([ + reconnect(), + new Promise((_, reject) => { + deps.setTimeout( + () => reject(new Error(`reconnect timed out after ${state.knobs.callTimeoutMs}ms`)), + state.knobs.callTimeoutMs, + ); + }), + ]); + } catch { /* reconnect best-effort; next tick retries against a fresh attempt */ } + if (state.cancelled()) return { kind: 'cancelled' }; + } + return { kind: 'ok' }; // counter incremented; not yet at deadline } diff --git a/src/core/minions/queue.ts b/src/core/minions/queue.ts index 02a271ced..5a19e7511 100644 --- a/src/core/minions/queue.ts +++ b/src/core/minions/queue.ts @@ -16,6 +16,14 @@ import type { import { rowToMinionJob, rowToInboxMessage, rowToAttachment } from './types.ts'; import { validateAttachment } from './attachments.ts'; import { isProtectedJobName } from './protected-names.ts'; +import { + withRetry, BULK_RETRY_OPTS, resolveBulkRetryOpts, computeNextDelay, + isRetryableConnError, +} from '../retry.ts'; +import { + logBatchRetry as auditLogBatchRetry, + logBatchExhausted as auditLogBatchExhausted, +} from '../audit/batch-retry-audit.ts'; /** Options for opting into protected-job-name submission. Passed as a separate * 4th arg to `MinionQueue.add()` (NOT folded into `opts`) so user-spread @@ -1032,14 +1040,51 @@ export class MinionQueue { return rows.length > 0; } + /** + * issue #1678 — self-healing retry for the Minion hot-path lock SQL. + * ONLY promoteDelayed routes through this: it's idempotent (re-running the + * same UPDATE on already-promoted rows is a no-op), so a retry after a + * reaped pooler socket can't cause double-work. `claim` and `renewLock` + * deliberately do NOT use this — see their call sites for why (Codex #1/#2): + * blind-retrying claim can double-claim a job, and retrying renewLock races + * the renewal-tick's own timeout. The reconnect callback rebuilds the + * instance pool between attempts when the engine supports it (Postgres); + * PGLite has no pooler reaping so reconnect is absent and the retry is a + * cheap pass-through. + */ + private async lockRetry(fn: () => Promise): Promise { + const reconnect = (this.engine as { reconnect?: () => Promise }).reconnect; + const opts = resolveBulkRetryOpts(); + let prevDelay = 0; + try { + return await withRetry(fn, { + maxRetries: opts.maxRetries, + delayMs: opts.delayMs, + delayMaxMs: opts.delayMaxMs, + jitter: BULK_RETRY_OPTS.jitter, + auditSite: 'minion-lock', + onRetry: (attempt, err) => { + const delay = computeNextDelay(attempt - 1, prevDelay, opts.delayMs, opts.delayMaxMs, BULK_RETRY_OPTS.jitter); + prevDelay = delay; + auditLogBatchRetry('minion-lock', 1, attempt, delay, err); + }, + reconnect: reconnect ? () => reconnect.call(this.engine) : undefined, + }); + } catch (err) { + if (err instanceof Error && err.name === 'RetryAbortError') throw err; + if (isRetryableConnError(err)) auditLogBatchExhausted('minion-lock', 1, opts.maxRetries + 1, err); + throw err; + } + } + /** Promote delayed jobs whose delay_until has passed. Returns promoted jobs. */ async promoteDelayed(): Promise { - const rows = await this.engine.executeRaw>( + const rows = await this.lockRetry(() => this.engine.executeRaw>( `UPDATE minion_jobs SET status = 'waiting', delay_until = NULL, lock_token = NULL, lock_until = NULL, updated_at = now() WHERE status = 'delayed' AND delay_until <= now() RETURNING *` - ); + )); return rows.map(rowToMinionJob); } diff --git a/src/core/minions/rss-default.ts b/src/core/minions/rss-default.ts new file mode 100644 index 000000000..fbe85b3f6 --- /dev/null +++ b/src/core/minions/rss-default.ts @@ -0,0 +1,138 @@ +/** + * Auto-sized default for the worker RSS watchdog cap (issue #1678). + * + * The pre-v0.42.5.0 default was a flat 2048MB — absurdly low for any brain + * doing embeddings (working set ~10GB), so the watchdog drained legit work on + * every heavy cycle and produced a silent ~400×/24h respawn loop. The watchdog + * is LEAK detection, not a container-OOM metric, so the default should be + * generous: comfortably above a real embed working set, capped so a 126GB box + * doesn't set a uselessly-high bar. + * + * THE LOAD-BEARING NUANCE (Codex #5): plain `os.totalmem()` reports HOST RAM. + * In a container / cgroup / launchd-memory-limited service it can report 64GB + * while the process's real ceiling is 4GB. If we auto-sized to 0.5×64GB=16GB + * the watchdog would NEVER fire and the kernel OOM-killer would SIGKILL the + * worker at 4GB — straight back to the opaque death this whole fix exists to + * prevent. So the basis is `min(cgroupLimit, totalmem)`: the watchdog cap MUST + * sit below the real memory ceiling so the graceful drain (distinct exit code, + * loud log) beats the kernel's silent kill. + * + * Formula: `clamp(round(0.5 × basisMB), 4096, 16384)`. + * 8GB box → 4096 (floor) + * 16GB box → 8192 + * 32GB box → 16384 (ceil) + * 126GB box→ 16384 (ceil) + * 4GB cgroup on a 126GB host → 2048 (0.5×4096) — below the 4GB ceiling so + * the drain beats the OOM-killer. (Below the 4096 floor, so floored... see + * note: the floor is intentionally NOT applied when it would exceed the + * basis — a cap above the real ceiling is the bug. See `clampToBasis`.) + * + * Explicit `--max-rss` always overrides this (including `--max-rss 0` to + * disable the watchdog entirely). + */ + +import { totalmem } from 'os'; +import { readFileSync } from 'fs'; + +const MB = 1024 * 1024; +export const RSS_DEFAULT_FLOOR_MB = 4096; +export const RSS_DEFAULT_CEIL_MB = 16384; +export const RSS_DEFAULT_FRACTION = 0.5; + +export interface ResolvedMaxRss { + /** The resolved cap in MB. */ + mb: number; + /** Where the memory basis came from. */ + source: 'cgroup-limited' | 'host'; + /** The basis (min of cgroup limit and host RAM) in MB, for the startup log. */ + basisMb: number; +} + +/** + * Read the cgroup memory limit in bytes, or null when no enforced limit is + * visible (macOS, bare metal, cgroup "max"/unlimited sentinel, or unreadable). + * + * cgroup v2: `/sys/fs/cgroup/memory.max` — literal `max` means unlimited. + * cgroup v1: `/sys/fs/cgroup/memory/memory.limit_in_bytes` — unlimited is a + * huge sentinel (~PAGE_COUNTER_MAX); `Math.min(limit, totalmem)` downstream + * naturally collapses that to totalmem, so we don't special-case it here. + * + * `readFile` is injectable for hermetic tests. + */ +export function readCgroupMemLimitBytes( + readFile: (path: string) => string = (p) => readFileSync(p, 'utf8'), +): number | null { + // cgroup v2 + try { + const raw = readFile('/sys/fs/cgroup/memory.max').trim(); + if (raw && raw !== 'max') { + const n = Number(raw); + if (Number.isFinite(n) && n > 0) return n; + } else if (raw === 'max') { + return null; + } + } catch { + /* not cgroup v2 / unreadable */ + } + // cgroup v1 + try { + const raw = readFile('/sys/fs/cgroup/memory/memory.limit_in_bytes').trim(); + const n = Number(raw); + if (Number.isFinite(n) && n > 0) return n; + } catch { + /* not cgroup v1 / unreadable */ + } + return null; +} + +export interface ResolveMaxRssOpts { + /** Override host RAM (bytes). Tests inject; production uses os.totalmem(). */ + totalMemBytes?: number; + /** + * Override the cgroup limit (bytes), or `null` for "no cgroup limit". When + * omitted, the real cgroup files are probed. Tests inject to stay hermetic. + */ + cgroupLimitBytes?: number | null; +} + +/** + * Resolve the auto-sized watchdog cap with provenance. Use this when you want + * to log where the number came from; `resolveDefaultMaxRssMb` is the thin + * number-only wrapper. + */ +export function describeDefaultMaxRss(opts: ResolveMaxRssOpts = {}): ResolvedMaxRss { + const totalMemBytes = opts.totalMemBytes ?? totalmem(); + const cgroup = + opts.cgroupLimitBytes !== undefined ? opts.cgroupLimitBytes : readCgroupMemLimitBytes(); + + // Basis = the smaller of host RAM and any enforced cgroup limit. A cgroup + // "unlimited" sentinel (huge number) loses the min to totalmem, so it reads + // as 'host'. + const cgroupLimited = cgroup !== null && cgroup < totalMemBytes; + const basisBytes = cgroupLimited ? (cgroup as number) : totalMemBytes; + const basisMb = Math.round(basisBytes / MB); + + // 0.5×basis is always strictly below the real ceiling — that's the base. + let mb = Math.min(Math.round(basisMb * RSS_DEFAULT_FRACTION), RSS_DEFAULT_CEIL_MB); + // Apply the floor ONLY when it stays below the basis. On a tiny cgroup (e.g. + // 4GB) the 4096 floor would equal/exceed the real ceiling and place the cap + // AT or above the limit — defeating "drain before OOM-kill". When the floor + // can't fit under the ceiling, the 0.5×basis value stands (safely below). + if (RSS_DEFAULT_FLOOR_MB < basisMb) { + mb = Math.max(mb, RSS_DEFAULT_FLOOR_MB); + } + // Final invariant: the cap must be strictly below the real memory ceiling so + // the graceful drain always beats the kernel OOM-killer. + if (mb >= basisMb) mb = Math.max(1, Math.floor(basisMb * RSS_DEFAULT_FRACTION)); + + return { mb, source: cgroupLimited ? 'cgroup-limited' : 'host', basisMb }; +} + +/** + * The auto-sized default watchdog cap in MB. Replaces the flat `?? 2048` at + * every production spawn site (jobs work, jobs supervisor, autopilot). Explicit + * `--max-rss` always wins over this. + */ +export function resolveDefaultMaxRssMb(opts: ResolveMaxRssOpts = {}): number { + return describeDefaultMaxRss(opts).mb; +} diff --git a/src/core/minions/supervisor.ts b/src/core/minions/supervisor.ts index 5a553dd72..325aa55b5 100644 --- a/src/core/minions/supervisor.ts +++ b/src/core/minions/supervisor.ts @@ -27,6 +27,7 @@ */ import { detectTini } from './spawn-helpers.ts'; +import { resolveDefaultMaxRssMb } from './rss-default.ts'; import { ChildWorkerSupervisor, type ChildSupervisorEvent, @@ -79,7 +80,9 @@ export interface SupervisorOpts { /** JSON mode: emit JSONL events on stderr, reserve stdout for data payloads. Default: false. */ json: boolean; /** RSS threshold (MB) passed to the spawned worker as `--max-rss N`. - * Default: 2048. Set to 0 to spawn the worker without a watchdog. */ + * When omitted, the constructor auto-sizes cgroup-aware via + * resolveDefaultMaxRssMb() (issue #1678) instead of a flat default. + * Set to 0 to spawn the worker without a watchdog. */ maxRssMb: number; /** Optional event sink (Lane C audit writer). Called for every lifecycle event. */ onEvent?: (event: SupervisorEmission) => void; @@ -159,6 +162,15 @@ export class MinionSupervisor { this.engine = engine; this.opts = { ...DEFAULTS, ...opts }; + // issue #1678 (Codex #4): when the caller didn't pin an explicit cap, + // auto-size cgroup-aware instead of the flat DEFAULTS.maxRssMb footgun. + // The CLI (jobs.ts supervisor) already resolves this and passes a concrete + // number; this covers direct-API / programmatic construction so the + // standalone supervisor never silently runs on the old 2048 default. + if (opts.maxRssMb === undefined) { + this.opts.maxRssMb = resolveDefaultMaxRssMb(); + } + // Detect tini for zombie reaping. Resolved once at construction so we // don't shell out on every respawn. Belt-and-suspenders with the // SIGCHLD handler in cli.ts — tini catches children spawned by native @@ -502,6 +514,11 @@ export class MinionSupervisor { count: event.count, window_ms: event.windowMs, queue: this.opts.queue, + // issue #1678 (A3): the supervisor knows the --max-rss it spawned + // with; name it in the OOM-loop alert so the operator's fix + // ("raise --max-rss") is one glance away. Peak RSS stays in the + // worker's own stderr line (the supervisor never sees it). + ...(event.reason === 'rss_watchdog_loop' ? { max_rss_mb: this.opts.maxRssMb } : {}), }); return; } diff --git a/src/core/minions/worker-exit-codes.ts b/src/core/minions/worker-exit-codes.ts new file mode 100644 index 000000000..b33e67367 --- /dev/null +++ b/src/core/minions/worker-exit-codes.ts @@ -0,0 +1,24 @@ +/** + * Reserved worker process exit codes — single source of truth shared by the + * worker (which sets them) and the supervisor / CLI (which classify them). + * + * Why a dedicated code for the RSS watchdog drain: before v0.42.5.0 the + * watchdog called `gracefulShutdown('watchdog')` which set `running=false` and + * let the process exit via natural cleanup → **exit code 0**. The supervisor + * classifies code 0 as `clean_exit` and does NOT increment `crashCount`, so a + * watchdog drain was indistinguishable from a healthy queue-drain. On a box + * where the embed working set legitimately needs ~10GB but the cap is 2048MB, + * that produced a silent ~400×/24h respawn loop whose only visible symptom was + * the downstream DB-connection cascade from the worker being drained mid-cycle + * (issue #1678). A distinct, reserved exit code makes the watchdog drain + * self-identifying: `worker_exited likely_cause=rss_watchdog` instead of + * `clean_exit`, and lets the supervisor apply a cause-keyed backoff + loud + * operator alert. + * + * Range choice: small single-digit-teens integer, deliberately outside + * {0 clean, 1 runtime_error} and the 128+N signal-derived range. 12 has no + * other meaning in this codebase. + */ + +/** Worker drained itself because RSS crossed the watchdog cap. */ +export const WORKER_EXIT_RSS_WATCHDOG = 12; diff --git a/src/core/minions/worker.ts b/src/core/minions/worker.ts index f611e1e28..24b2c4d01 100644 --- a/src/core/minions/worker.ts +++ b/src/core/minions/worker.ts @@ -28,6 +28,7 @@ import { type LockRenewalState, } from './lock-renewal-tick.ts'; import { lockRenewalAudit } from '../audit/lock-renewal-audit.ts'; +import { isRetryableConnError } from '../retry-matcher.ts'; /** * Abort reasons that signal infrastructure failure (PgBouncer outage, @@ -165,6 +166,23 @@ export class MinionWorker extends EventEmitter { private jobsCompleted = 0; /** Idempotency latch for gracefulShutdown — per-job and periodic check sites can race. */ private gracefulShutdownFired = false; + /** + * Set true when the RSS watchdog (not a normal SIGTERM) initiated the + * drain. The CLI handler (src/commands/jobs.ts case 'work') reads this + * AFTER start() resolves and exits the process with + * WORKER_EXIT_RSS_WATCHDOG so the supervisor can classify the drain as + * `rss_watchdog` instead of a clean exit. The worker deliberately does + * NOT set process.exitCode itself — that would leak a non-zero code into + * embedding hosts (tests, other process owners) that call start()/stop() + * in-process. Ownership of process exit stays with the CLI, same as the + * engine-disconnect boundary. + */ + private _rssWatchdogTriggered = false; + /** Peak observed RSS (MB) this process lifetime — surfaced in the watchdog + * drain line and the 80% soft-warn so operators can size --max-rss. */ + private _peakRssMb = 0; + /** Latch so the 80%-of-cap soft-warn fires once per crossing, not every check. */ + private _softWarnFired = false; private opts: Required; @@ -218,6 +236,16 @@ export class MinionWorker extends EventEmitter { return Array.from(this.handlers.keys()); } + /** + * True when the RSS watchdog drained this worker (vs a normal SIGTERM + * shutdown). The CLI handler reads this after `start()` resolves to set + * the distinct WORKER_EXIT_RSS_WATCHDOG process exit code. See the field + * comment on `_rssWatchdogTriggered` for the ownership rationale. + */ + get rssWatchdogTriggered(): boolean { + return this._rssWatchdogTriggered; + } + /** Emit 'unhealthy' with a no-listener fallback. The default contract is * fail-stop: pre-EventEmitter-refactor behavior was process.exit(1) inside * the timer; the refactor moved that responsibility to the CLI subscriber. @@ -466,12 +494,35 @@ export class MinionWorker extends EventEmitter { // Claim jobs up to concurrency limit if (this.inFlight.size < this.opts.concurrency) { const lockToken = `${this.workerId}:${Date.now()}`; - const job = await this.queue.claim( - lockToken, - this.opts.lockDuration, - this.opts.queue, - this.registeredNames, - ); + let job: MinionJob | null; + try { + job = await this.queue.claim( + lockToken, + this.opts.lockDuration, + this.opts.queue, + this.registeredNames, + ); + } catch (e) { + // issue #1678 (Codex #1): a reaped pooler socket / nulled instance + // pool throws a retryable conn error here. Blind-retrying claim is + // UNSAFE — if the UPDATE...RETURNING committed but the connection + // died before the row reached us, a retry would claim a SECOND + // job (invisible active job, no renewal, later stall). So instead: + // reconnect once and let the NEXT poll tick re-claim against a live + // pool. Non-retryable errors propagate (real bug → PM restart). + if (!isRetryableConnError(e)) throw e; + const msg = e instanceof Error ? e.message : String(e); + console.error(`[worker] claim hit a connection error; reconnecting, retry on next tick: ${msg}`); + const reconnect = (this.engine as { reconnect?: () => Promise }).reconnect; + if (reconnect) { + try { await reconnect.call(this.engine); } + catch (re) { + console.error(`[worker] reconnect after claim error failed: ${re instanceof Error ? re.message : String(re)}`); + } + } + await new Promise(resolve => setTimeout(resolve, this.opts.pollInterval)); + continue; + } if (job) { // Quiet-hours gate: evaluated at claim time, not dispatch. @@ -604,12 +655,37 @@ export class MinionWorker extends EventEmitter { return; } const rssMb = Math.round(rss / (1024 * 1024)); - if (rssMb < this.opts.maxRssMb) return; + if (rssMb > this._peakRssMb) this._peakRssMb = rssMb; + // Names of the jobs in flight when memory crested — the diagnostic an + // operator needs to know WHICH job kind is the memory hog. + const inFlightKinds = Array.from(this.inFlight.values()).map(f => f.job.name); + + // 80%-of-cap soft warn: fires once per crossing (re-arms once RSS drops + // back under the line) so operators get a heads-up BEFORE the kill rather + // than a silent death. Cheap: one extra comparison per check. + const softLine = Math.floor(this.opts.maxRssMb * 0.8); + if (rssMb < this.opts.maxRssMb) { + if (rssMb >= softLine && !this._softWarnFired) { + this._softWarnFired = true; + const ts = new Date().toISOString().slice(11, 19); + console.warn( + `[watchdog ${ts}] approaching cap: rss=${rssMb}MB (${Math.round((rssMb / this.opts.maxRssMb) * 100)}% of ${this.opts.maxRssMb}MB) ` + + `peak=${this._peakRssMb}MB in_flight=${inFlightKinds.join(',') || 'none'} — next overshoot will drain. ` + + `Raise --max-rss if this job kind legitimately needs more.`, + ); + } else if (rssMb < softLine) { + this._softWarnFired = false; + } + return; + } + + this._rssWatchdogTriggered = true; const ts = new Date().toISOString().slice(11, 19); console.warn( - `[watchdog ${ts}] rss=${rssMb}MB threshold=${this.opts.maxRssMb}MB ` + - `jobs_completed=${this.jobsCompleted} source=${source} — draining`, + `[watchdog ${ts}] rss=${rssMb}MB threshold=${this.opts.maxRssMb}MB peak=${this._peakRssMb}MB ` + + `jobs_completed=${this.jobsCompleted} in_flight=${inFlightKinds.join(',') || 'none'} source=${source} — draining ` + + `(raise --max-rss if this is legitimate working set, not a leak)`, ); this.gracefulShutdown('watchdog'); } @@ -691,11 +767,17 @@ export class MinionWorker extends EventEmitter { consecutiveFailures: 0, cancelled: () => cancelled, }; + // issue #1678 (Codex #2): hand the tick a bounded reconnect-once hook when + // 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 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) } : {}), }; const lockTimer = setInterval(() => { diff --git a/src/core/postgres-engine.ts b/src/core/postgres-engine.ts index 61d081a19..8dc83ad78 100644 --- a/src/core/postgres-engine.ts +++ b/src/core/postgres-engine.ts @@ -121,6 +121,22 @@ export class PostgresEngine implements BrainEngine { // Instance connection (for workers) or fall back to module global (backward compat) get sql(): ReturnType { if (this._sql) return this._sql; + // issue #1678: an instance-pool engine whose _sql went null (a mid-process + // disconnect/reconnect, or a reaped socket) must NOT fall through to the + // module singleton — that singleton was never connected on a worker, so + // db.getConnection() throws the misleading "connect() has not been called". + // Throw a tailored RETRYABLE error instead (isRetryableConnError matches + // problem === 'No database connection'), so a caller wrapped in + // withRetry+reconnect rebuilds this instance's pool and recovers. The + // module / never-connected path (style 'module' or null) keeps the legacy + // getConnection() behavior. + if (this._connectionStyle === 'instance') { + throw new GBrainError( + 'No database connection', + 'instance connection pool was torn down (socket reaped or mid-process disconnect)', + 'Transient — the operation reconnects and retries. If it persists, check pooler/Supavisor health.', + ); + } return db.getConnection(); } @@ -793,7 +809,7 @@ export class PostgresEngine implements BrainEngine { } async transaction(fn: (engine: BrainEngine) => Promise): Promise { - const conn = this._sql || db.getConnection(); + const conn = this.sql; return conn.begin(async (tx) => { // Create a scoped engine with tx as its connection, no shared state mutation const txEngine = Object.create(this) as PostgresEngine; @@ -804,7 +820,7 @@ export class PostgresEngine implements BrainEngine { } async withReservedConnection(fn: (conn: ReservedConnection) => Promise): Promise { - const pool = this._sql || db.getConnection(); + const pool = this.sql; const reserved = await pool.reserve(); try { const conn: ReservedConnection = { @@ -4113,7 +4129,7 @@ export class PostgresEngine implements BrainEngine { oldRow: number, newRow: Omit, ): Promise<{ oldRow: number; newRow: number }> { - const conn = this._sql || db.getConnection(); + const conn = this.sql; return await conn.begin(async (tx) => { const [existing] = await tx` SELECT resolved_at FROM takes WHERE page_id = ${pageId} AND row_num = ${oldRow} diff --git a/src/core/retry-matcher.ts b/src/core/retry-matcher.ts index 071fa07fe..c7fd0cec7 100644 --- a/src/core/retry-matcher.ts +++ b/src/core/retry-matcher.ts @@ -25,6 +25,12 @@ const CONN_PATTERNS = [ // postgres.js's auto-recovery between queries). Matches the literal // message shape from PR #1416's reported batch-loss incident. /No database connection/i, + // v0.42.5.0 (issue #1678): postgres.js throws errors carrying + // `code: 'CONNECTION_ENDED'` (a LIBRARY code, not an 08xxx SQLSTATE) when a + // transaction-mode pooler reaps an idle socket between queries. Without an + // explicit match it was only accidentally caught by /connection.*closed/i. + // Match the message form too for wrappers that fold the code into the text. + /CONNECTION_ENDED/i, ]; interface PgError { @@ -93,6 +99,9 @@ export function isRetryableConnError(err: unknown): boolean { // 08001 sqlclient_unable_to_establish_sqlconnection // 08004 sqlserver_rejected_establishment_of_sqlconnection if (code && /^08/.test(code)) return true; + // v0.42.5.0 (issue #1678): postgres.js's library-level connection-ended + // code. Not an 08xxx SQLSTATE, so the /^08/ test above misses it. + if (code === 'CONNECTION_ENDED') return true; // v0.41.2.1: typed-shape match for gbrain's own GBrainError // (problem === 'No database connection'). Avoids brittle string match // when the error wrapper is gbrain-internal. diff --git a/src/core/retry.ts b/src/core/retry.ts index 3fa4ac89b..5bc93cde5 100644 --- a/src/core/retry.ts +++ b/src/core/retry.ts @@ -93,6 +93,10 @@ export const BATCH_AUDIT_SITES = [ 'reindex.multimodal', // backfill-base.ts outer connection-retry layer. 'backfill.outer', + // queue.ts Minion hot-path lock recovery (issue #1678): promoteDelayed + // self-heal on a reaped pooler socket. claim/renewLock deliberately do NOT + // route here (Codex #1/#2) — the poll loop and renewal-tick recover those. + 'minion-lock', ] as const; export type BatchAuditSite = (typeof BATCH_AUDIT_SITES)[number]; diff --git a/test/audit/batch-retry-audit.test.ts b/test/audit/batch-retry-audit.test.ts index 9fcae9c5b..a9216bf67 100644 --- a/test/audit/batch-retry-audit.test.ts +++ b/test/audit/batch-retry-audit.test.ts @@ -180,14 +180,15 @@ describe('pruneOldBatchRetryAuditFiles — codex H-8 actual pruning', () => { }); test('no-op when audit dir does not exist (ENOENT)', async () => { - // Point GBRAIN_AUDIT_DIR at a guaranteed-missing subdir of the per-test - // tmpDir. Without this override the function reads the real ~/.gbrain/audit, - // so the assertion flakes on any dev machine that already has a real - // batch-retry-*.jsonl on disk (returns kept:1, not kept:0). Hermetic now, - // matching this file's header contract. - await withEnv({ GBRAIN_AUDIT_DIR: path.join(tmpDir, 'does-not-exist') }, async () => { + // Isolate GBRAIN_AUDIT_DIR at a guaranteed-nonexistent path (CLAUDE.md R1). + // Pre-fix this called pruneOld with no override, so on a real dev machine it + // walked ~/.gbrain/audit — which may already hold this-week's batch-retry + // files from other (un-isolated) suites, making `kept` non-zero and the test + // flaky. Pointing at a missing subdir makes the ENOENT path deterministic. + const missing = path.join(tmpDir, 'does-not-exist'); + await withEnv({ GBRAIN_AUDIT_DIR: missing }, async () => { const result = pruneOldBatchRetryAuditFiles(30, new Date()); - // The function never throws on a missing dir; it returns the empty result. + // The function never throws on a missing dir; returns the empty result. expect(result).toEqual({ removed: 0, kept: 0 }); }); }); diff --git a/test/autopilot-supervisor-wiring.test.ts b/test/autopilot-supervisor-wiring.test.ts index 0073255bb..ac3ec2f77 100644 --- a/test/autopilot-supervisor-wiring.test.ts +++ b/test/autopilot-supervisor-wiring.test.ts @@ -13,7 +13,7 @@ * static-shape regressions read the source file and pin the load-bearing * constants: * - * - `--max-rss 2048` is passed to the worker (incident-driving default) + * - the worker is spawned with an auto-sized `--max-rss` (issue #1678) * - `maxCrashes: 5` matches the prior `crashCount >= 5` give-up rule * - The autopilot composes ChildWorkerSupervisor (not the legacy * inline `child.on('exit')` loop) @@ -50,12 +50,15 @@ describe('autopilot.ts ↔ ChildWorkerSupervisor wiring', () => { expect(AUTOPILOT_SRC).not.toContain('STABLE_RUN_RESET_MS'); }); - it("constructs ChildWorkerSupervisor with --max-rss 2048", () => { - // The worker spawn args must include both flag tokens in argv order. - // This is the incident-driving default; changing it without a deliberate - // decision would regress the workaround for VmRSS inflation. - expect(AUTOPILOT_SRC).toContain("'--max-rss', '2048'"); + it("spawns the worker with an auto-sized --max-rss (issue #1678)", () => { + // Post-v0.41.39.0 the flat 2048 default is gone: autopilot resolves + // resolveDefaultMaxRssMb() (cgroup-aware) and passes it as the cap. The + // argv must still carry the --max-rss flag token + the resolved value. + expect(AUTOPILOT_SRC).toContain("resolveDefaultMaxRssMb"); + expect(AUTOPILOT_SRC).toContain("'--max-rss', String(autopilotMaxRssMb)"); expect(AUTOPILOT_SRC).toContain("'jobs', 'work'"); + // The footgun literal must NOT come back. + expect(AUTOPILOT_SRC).not.toContain("'--max-rss', '2048'"); }); it("constructs ChildWorkerSupervisor with maxCrashes: 5", () => { diff --git a/test/child-worker-supervisor.test.ts b/test/child-worker-supervisor.test.ts index 6f91408fc..603d81c35 100644 --- a/test/child-worker-supervisor.test.ts +++ b/test/child-worker-supervisor.test.ts @@ -55,6 +55,9 @@ async function runUntilTerminal( cleanRestartWindowMs: number; cleanRestartBudgetBackoffMs: number; stableRunResetMs: number; + watchdogLoopBudget: number; + watchdogLoopWindowMs: number; + watchdogBackoffMs: number; _now: () => number; stopAfterEvents: number; // safety net so a buggy test can't hang }>, @@ -73,6 +76,9 @@ async function runUntilTerminal( cleanRestartWindowMs: overrides.cleanRestartWindowMs, cleanRestartBudgetBackoffMs: overrides.cleanRestartBudgetBackoffMs, stableRunResetMs: overrides.stableRunResetMs, + watchdogLoopBudget: overrides.watchdogLoopBudget, + watchdogLoopWindowMs: overrides.watchdogLoopWindowMs, + watchdogBackoffMs: overrides.watchdogBackoffMs, _now: overrides._now, isStopping: () => stopping, onMaxCrashesExceeded: (count, max) => { @@ -407,4 +413,61 @@ esac } }); }); + + // issue #1678: RSS-watchdog exits (code 12) are cause-keyed and must NOT + // route through the generic crash path — the >5-min stable-run reset would + // defeat max_crashes and the 400×/24h loop would never stop being silent. + describe('rss_watchdog breaker (issue #1678)', () => { + it('code=12 is labeled rss_watchdog and never increments crashCount', async () => { + const h = makeHarness('wd-nocrash', 'exit 12'); + try { + const { events, maxCrashesFired } = await runUntilTerminal(h, { + maxCrashes: 3, + _backoffFloorMs: 1, + stopAfterEvents: 18, // ~6 spawn/exit/backoff triples + }); + const exited = events.filter( + (e): e is Extract => + e.kind === 'worker_exited', + ); + // Looped well past maxCrashes WITHOUT tripping it — the whole point. + expect(maxCrashesFired).toBeNull(); + expect(exited.length).toBeGreaterThan(3); + for (const e of exited) { + expect(e.code).toBe(12); + expect(e.likelyCause).toBe('rss_watchdog'); + expect(e.crashCount).toBe(0); // never counted as a crash + } + } finally { + h.cleanup(); + } + }); + + it('emits rss_watchdog_loop health_warn once the window budget is exceeded', async () => { + const h = makeHarness('wd-loop', 'exit 12'); + try { + const { events } = await runUntilTerminal(h, { + maxCrashes: 99, + _backoffFloorMs: 1, + watchdogLoopBudget: 2, + watchdogLoopWindowMs: 600_000, + stopAfterEvents: 24, + }); + const warns = events.filter( + (e): e is Extract => + e.kind === 'health_warn' && e.reason === 'rss_watchdog_loop', + ); + // Budget=2 → the 3rd+ watchdog exit in-window fires the loud alert. + expect(warns.length).toBeGreaterThan(0); + expect(warns[0].count).toBeGreaterThan(2); + // And every backoff after a watchdog exit is reason=rss_watchdog. + const wdBackoffs = events.filter( + (e) => e.kind === 'backoff' && e.reason === 'rss_watchdog', + ); + expect(wdBackoffs.length).toBeGreaterThan(0); + } finally { + h.cleanup(); + } + }); + }); }); diff --git a/test/core/retry.test.ts b/test/core/retry.test.ts index 9b8363a11..9714775fe 100644 --- a/test/core/retry.test.ts +++ b/test/core/retry.test.ts @@ -348,6 +348,7 @@ describe('BATCH_AUDIT_SITES typed enum + isBatchAuditSite guard (D10c codex)', ( 'sync.import_file', 'reindex.markdown', 'reindex.multimodal', 'backfill.outer', + 'minion-lock', ]); expect(new Set([...BATCH_AUDIT_SITES])).toEqual(expected); }); diff --git a/test/doctor-extract-atoms-backlog.test.ts b/test/doctor-extract-atoms-backlog.test.ts new file mode 100644 index 000000000..83f502283 --- /dev/null +++ b/test/doctor-extract-atoms-backlog.test.ts @@ -0,0 +1,98 @@ +/** + * issue #1678 — extract_atoms backlog count + doctor check. + * + * Pins: + * - countExtractAtomsBacklog counts eligible-but-unextracted pages (scoped + + * brain-wide) and excludes pages that already have an atom (NOT EXISTS). + * - computeExtractAtomsBacklogCheck WARNs with a `--drain` hint when the pack + * doesn't run the phase and the backlog is real; OK at 0. + * + * Real in-memory PGLite (canonical block, R3+R4). GBRAIN_HOME is pointed at an + * empty tmpdir for the doctor-check cases so packDeclaresPhase resolves the + * bundled base pack (which does NOT declare extract_atoms) deterministically, + * independent of the developer's real ~/.gbrain config. + */ + +import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; +import { mkdtempSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { resetPgliteState } from './helpers/reset-pglite.ts'; +import { withEnv } from './helpers/with-env.ts'; +import { countExtractAtomsBacklog } from '../src/core/cycle/extract-atoms.ts'; +import { computeExtractAtomsBacklogCheck } from '../src/commands/doctor.ts'; + +let engine: PGLiteEngine; +const EMPTY_HOME = mkdtempSync(join(tmpdir(), 'gbrain-xa-backlog-home-')); + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); +}); + +afterAll(async () => { + await engine.disconnect(); +}); + +beforeEach(async () => { + await resetPgliteState(engine); +}); + +const BODY = 'x'.repeat(600); // >= MIN_PAGE_CHARS_FOR_EXTRACTION (500) + +async function seedArticle(slug: string) { + return engine.putPage(slug, { type: 'article', title: slug, compiled_truth: BODY }); +} + +describe('countExtractAtomsBacklog (issue #1678)', () => { + it('counts eligible pages with no atom (scoped + brain-wide)', async () => { + await seedArticle('article-a'); + await seedArticle('article-b'); + await seedArticle('article-c'); + expect(await countExtractAtomsBacklog(engine)).toBe(3); + expect(await countExtractAtomsBacklog(engine, 'default')).toBe(3); + }); + + it('excludes a page that already has a matching atom (NOT EXISTS)', async () => { + const p = await seedArticle('article-x'); + const h16 = (p.content_hash ?? '').slice(0, 16); + expect(h16.length).toBe(16); + await engine.putPage('atoms/a1', { + type: 'atom', + title: 'a1', + compiled_truth: 'an extracted nugget', + frontmatter: { source_hash: h16 }, + }); + expect(await countExtractAtomsBacklog(engine)).toBe(0); + }); + + it('ignores short pages and dream-generated pages', async () => { + await engine.putPage('article-short', { type: 'article', title: 's', compiled_truth: 'too short' }); + await engine.putPage('article-dream', { + type: 'article', title: 'd', compiled_truth: BODY, + frontmatter: { dream_generated: 'true' }, + }); + expect(await countExtractAtomsBacklog(engine)).toBe(0); + }); +}); + +describe('computeExtractAtomsBacklogCheck (issue #1678)', () => { + it('OK with no backlog', async () => { + const check = await withEnv({ GBRAIN_HOME: EMPTY_HOME }, () => + computeExtractAtomsBacklogCheck(engine)); + expect(check.status).toBe('ok'); + expect((check.details as { backlog: number }).backlog).toBe(0); + }); + + it('WARNs with a --drain hint when the pack does not run the phase and backlog > 10', async () => { + for (let i = 0; i < 11; i++) await seedArticle(`article-${i}`); + const check = await withEnv({ GBRAIN_HOME: EMPTY_HOME }, () => + computeExtractAtomsBacklogCheck(engine)); + expect(check.status).toBe('warn'); + expect(check.message).toContain('--drain'); + expect((check.details as { pack_declares_phase: boolean }).pack_declares_phase).toBe(false); + expect((check.details as { known_approximation: string }).known_approximation).toContain('page backlog only'); + }); +}); diff --git a/test/dream-cli-flags.test.ts b/test/dream-cli-flags.test.ts index 26b3ac3cf..3663996c2 100644 --- a/test/dream-cli-flags.test.ts +++ b/test/dream-cli-flags.test.ts @@ -105,4 +105,28 @@ describe('dream CLI flag wiring', () => { expect(dreamSrc).toContain('gbrain sources restore'); }); }); + + // issue #1678 — --drain bounded backlog drain wiring (structural). + describe('--drain wiring', () => { + test('declares --drain and --window flags', () => { + expect(dreamSrc).toContain("'--drain'"); + expect(dreamSrc).toContain("'--window'"); + expect(dreamSrc).toContain('windowSeconds'); + }); + + test('--drain defaults to extract_atoms and rejects other phases', () => { + expect(dreamSrc).toContain("phase = 'extract_atoms'"); + 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 reports remaining + exits non-zero when incomplete', () => { + expect(dreamSrc).toContain('EXIT_DRAIN_INCOMPLETE'); + expect(dreamSrc).toContain('cycle_already_running'); + }); + }); }); diff --git a/test/e2e/cycle.test.ts b/test/e2e/cycle.test.ts index e49319f99..8ae3dbcb9 100644 --- a/test/e2e/cycle.test.ts +++ b/test/e2e/cycle.test.ts @@ -29,7 +29,7 @@ mock.module('../../src/core/embedding.ts', () => ({ currentEmbeddingSignature: () => 'test:model:1536', })); -const { runCycle } = await import('../../src/core/cycle.ts'); +const { runCycle, ALL_PHASES } = await import('../../src/core/cycle.ts'); const skip = !hasDatabase(); const describeE2E = skip ? describe.skip : describe; @@ -99,17 +99,13 @@ describeE2E('E2E: runCycle against real Postgres', () => { }); expect(report.schema_version).toBe('1'); - // Cycle ran all 16 phases (or skipped the ones that don't support dry-run). - // Phase history: - // v0.23 = 8 phases (lint → backlinks → sync → synthesize → extract → patterns → embed → orphans) - // v0.26.5 = 9 (added `purge` after orphans) - // v0.29 = 10 (added `recompute_emotional_weight` between patterns and embed) - // v0.31 = 11 (added `consolidate` between recompute_emotional_weight and embed) - // v0.32.2 = 12 (added `extract_facts` between extract and patterns) - // v0.33.3 = 13 (added `resolve_symbol_edges` between extract_facts and patterns) - // v0.36.1.0 = 16 (added propose_takes + grade_takes + calibration_profile — hindsight calibration wave) - // v0.39.0.0 = 17 (added `schema-suggest` between orphans and purge — T12 schema cathedral) - expect(report.phases.length).toBe(19); // v0.41: +extract_atoms, +synthesize_concepts + // Every phase in ALL_PHASES pushes exactly one result (pack-gated phases + // like extract_atoms / synthesize_concepts push a 'skipped' result), so + // the full dry-run cycle's phase count always equals ALL_PHASES.length. + // Assert against the live constant rather than a hardcoded number so this + // doesn't go stale every time a phase is added (it drifted to 19 while the + // real count was 20 — the conversation_facts_backfill phase was missed). + expect(report.phases.length).toBe(ALL_PHASES.length); // Nothing got written. const afterPages = await conn.unsafe(`SELECT count(*)::int AS n FROM pages`); diff --git a/test/extract-atoms-drain.test.ts b/test/extract-atoms-drain.test.ts new file mode 100644 index 000000000..cf9ab7e8a --- /dev/null +++ b/test/extract-atoms-drain.test.ts @@ -0,0 +1,108 @@ +/** + * issue #1678 — bounded single-hold extract_atoms drain loop. + * + * Pure-over-injected-deps, so no DB / LLM / lock primitive. Pins: + * - drains to empty (rediscovers each batch via countRemaining), stops 'drained' + * - the wallclock window bounds the loop, stops 'window' with remaining > 0 + * - a zero-progress batch stops the loop (no hot loop burning budget) + * - a busy lock (withLock throws) propagates so the caller reports skipped + */ + +import { describe, it, expect } from 'bun:test'; +import { + runExtractAtomsDrain, + type ExtractAtomsDrainDeps, +} from '../src/core/cycle/extract-atoms-drain.ts'; + +function seq(values: Array): () => Promise { + let i = 0; + return async () => values[Math.min(i++, values.length - 1)]; +} + +const passThroughLock: ExtractAtomsDrainDeps['withLock'] = (work) => work(); + +describe('runExtractAtomsDrain (issue #1678)', () => { + it('drains to empty and reports stopped=drained', async () => { + let batches = 0; + const result = await runExtractAtomsDrain( + { + withLock: passThroughLock, + countRemaining: seq([3, 2, 1, 0, 0]), + runBatch: async () => { batches++; return { extracted: 1, skipped: 0 }; }, + now: () => 0, + }, + { windowMs: 1_000_000 }, + ); + expect(result.stopped).toBe('drained'); + expect(result.remaining).toBe(0); + expect(result.batches).toBe(3); + expect(result.extracted).toBe(3); + expect(batches).toBe(3); + }); + + it('stops at the wallclock window with remaining > 0', async () => { + // SYNC stepping clock: now() #1 sets deadline (0+100=100); the while-check + // then sees 50, 50 (two batches), then 999999 → past deadline → stop. + const times = [0, 50, 50, 999_999]; + let ti = 0; + const now = () => times[Math.min(ti++, times.length - 1)]; + const result = await runExtractAtomsDrain( + { + withLock: passThroughLock, + countRemaining: async () => 5, // never drains + runBatch: async () => ({ extracted: 1, skipped: 0 }), + now, + }, + { windowMs: 100 }, + ); + expect(result.stopped).toBe('window'); + expect(result.remaining).toBe(5); + expect(result.batches).toBe(2); + }); + + it('stops on a zero-progress batch (no hot loop)', async () => { + let batches = 0; + const result = await runExtractAtomsDrain( + { + withLock: passThroughLock, + countRemaining: async () => 5, + runBatch: async () => { batches++; return { extracted: 0, skipped: 0 }; }, + now: () => 0, + }, + { windowMs: 1_000_000 }, + ); + expect(result.stopped).toBe('no_progress'); + expect(batches).toBe(1); + expect(result.remaining).toBe(5); + }); + + it('propagates a busy-lock error (caller reports cycle_already_running)', async () => { + class FakeBusy extends Error {} + await expect( + runExtractAtomsDrain( + { + withLock: () => { throw new FakeBusy('held'); }, + countRemaining: async () => 5, + runBatch: async () => ({ extracted: 1, skipped: 0 }), + now: () => 0, + }, + { windowMs: 1000 }, + ), + ).rejects.toThrow('held'); + }); + + it('respects maxBatches as a belt-and-suspenders cap', async () => { + let batches = 0; + const result = await runExtractAtomsDrain( + { + withLock: passThroughLock, + countRemaining: async () => 999, // never drains + runBatch: async () => { batches++; return { extracted: 1, skipped: 0 }; }, + now: () => 0, // window never elapses + }, + { windowMs: 1_000_000, maxBatches: 4 }, + ); + expect(result.stopped).toBe('max_batches'); + expect(batches).toBe(4); + }); +}); diff --git a/test/fixtures/supervisor-runner.ts b/test/fixtures/supervisor-runner.ts index 8387f6bbf..ebdef1862 100644 --- a/test/fixtures/supervisor-runner.ts +++ b/test/fixtures/supervisor-runner.ts @@ -40,6 +40,11 @@ const backoffFloor = parseInt(process.env.SUP_BACKOFF_FLOOR_MS ?? '1', 10); const healthInterval = parseInt(process.env.SUP_HEALTH_INTERVAL_MS ?? '999999', 10); const allowShellJobs = process.env.SUP_ALLOW_SHELL_JOBS === '1'; const queueName = process.env.SUP_QUEUE ?? 'default'; +// SUP_MAX_RSS: when set, pin an explicit watchdog cap (tests the passthrough +// path). When unset, MinionSupervisor auto-sizes cgroup-aware (issue #1678). +const maxRssExplicit = process.env.SUP_MAX_RSS !== undefined + ? parseInt(process.env.SUP_MAX_RSS, 10) + : undefined; if (process.env.SUP_AUDIT_DIR) { process.env.GBRAIN_AUDIT_DIR = process.env.SUP_AUDIT_DIR; @@ -57,6 +62,7 @@ const supervisor = new MinionSupervisor(mockEngine as BrainEngine, { allowShellJobs, json: true, _backoffFloorMs: backoffFloor, + ...(maxRssExplicit !== undefined ? { maxRssMb: maxRssExplicit } : {}), onEvent: (emission) => writeSupervisorEvent(emission, supervisorPid), }); diff --git a/test/lint-shared-engine.test.ts b/test/lint-shared-engine.test.ts new file mode 100644 index 000000000..a50ddbc96 --- /dev/null +++ b/test/lint-shared-engine.test.ts @@ -0,0 +1,51 @@ +/** + * issue #1678 — lint must REUSE a caller-provided engine for the + * content-sanity DB-plane config lift, never create + disconnect its own. + * + * The bug: resolveLintContentSanity created a module-style engine + * (createEngine without poolSize wraps the db.ts singleton) and disconnect()ed + * it, which cascaded to db.disconnect() and NULLED the shared singleton the + * cycle's lint phase depends on — breaking every subsequent cycle phase with a + * misleading "connect() has not been called". When the caller passes a live + * engine, lint must use it directly with zero connection churn. + * + * Hermetic: a fake BrainEngine that records disconnect() calls + serves + * getConfig. No real DB. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { mkdtempSync, writeFileSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { runLintCore } from '../src/commands/lint.ts'; +import type { BrainEngine } from '../src/core/engine.ts'; + +let dir: string; +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'lint-shared-engine-')); + writeFileSync(join(dir, 'a.md'), '---\ntype: note\ntitle: A\n---\n\nSome content.\n'); +}); +afterEach(() => { + try { rmSync(dir, { recursive: true, force: true }); } catch { /* noop */ } +}); + +describe('runLintCore engine reuse (issue #1678)', () => { + it('reuses a provided engine for the content-sanity lift and NEVER disconnects it', async () => { + const state = { disconnects: 0, connects: 0, getConfigCalls: 0 }; + const engine = { + kind: 'postgres' as const, + getConfig: async () => { state.getConfigCalls++; return null; }, + connect: async () => { state.connects++; }, + disconnect: async () => { state.disconnects++; }, + } as unknown as BrainEngine; + + await runLintCore({ target: dir, fix: false, dryRun: true, engine }); + + // The load-bearing assertion: the shared engine was used (getConfig hit) + // but NEVER disconnected and NEVER re-connected — no connection churn that + // could null a shared singleton mid-cycle. + expect(state.getConfigCalls).toBeGreaterThan(0); + expect(state.disconnects).toBe(0); + expect(state.connects).toBe(0); + }); +}); diff --git a/test/postgres-engine-getter-selfheal.test.ts b/test/postgres-engine-getter-selfheal.test.ts new file mode 100644 index 000000000..c1b33696e --- /dev/null +++ b/test/postgres-engine-getter-selfheal.test.ts @@ -0,0 +1,43 @@ +/** + * issue #1678 — the `PostgresEngine.sql` getter must not fall through to the + * never-connected module singleton when an INSTANCE pool's _sql went null + * (mid-process disconnect, or a reaped pooler socket). Pre-fix it threw the + * misleading "connect() has not been called"; post-fix it throws a tailored + * RETRYABLE error so withRetry+reconnect rebuilds the pool and recovers. + * + * Pure: pokes the private fields and reads the synchronous getter; no real DB. + */ + +import { describe, it, expect } from 'bun:test'; +import { PostgresEngine } from '../src/core/postgres-engine.ts'; +import { isRetryableConnError } from '../src/core/retry-matcher.ts'; + +describe('PostgresEngine.sql getter self-heal (issue #1678)', () => { + it('instance-pool + null _sql throws a RETRYABLE error naming the reaped pool', () => { + const e = new PostgresEngine(); + (e as unknown as { _connectionStyle: string })._connectionStyle = 'instance'; + (e as unknown as { _sql: unknown })._sql = null; + + let thrown: unknown; + try { + // accessing the getter triggers the throw + void e.sql; + } catch (err) { + thrown = err; + } + expect(thrown).toBeDefined(); + // Must be classified retryable so the lock/batch retry paths reconnect. + expect(isRetryableConnError(thrown)).toBe(true); + // Must NOT be the misleading legacy message. + const msg = (thrown as Error).message; + expect(msg).toContain('instance connection pool'); + expect(msg).not.toContain('connect() has not been called'); + }); + + it('a live instance _sql is returned directly (no throw)', () => { + const e = new PostgresEngine(); + const fakeSql = { tag: 'live-pool' }; + (e as unknown as { _sql: unknown })._sql = fakeSql; + expect(e.sql as unknown).toBe(fakeSql); + }); +}); diff --git a/test/queue-lock-retry.test.ts b/test/queue-lock-retry.test.ts new file mode 100644 index 000000000..f881589ad --- /dev/null +++ b/test/queue-lock-retry.test.ts @@ -0,0 +1,70 @@ +/** + * issue #1678 — Minion hot-path lock recovery contract. + * + * - promoteDelayed (idempotent) self-heals: a reaped-socket CONNECTION_ENDED + * triggers a reconnect + retry against a fresh pool. + * - claim does NOT retry inline (Codex #1): blind-retrying a claim whose + * UPDATE...RETURNING may have committed could double-claim a job. The error + * propagates; the worker poll loop reconnects + re-claims on the next tick. + * + * Hermetic: a fake BrainEngine whose executeRaw is scripted; no real DB. + */ + +import { describe, it, expect } from 'bun:test'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { MinionQueue } from '../src/core/minions/queue.ts'; +import { withEnv } from './helpers/with-env.ts'; + +function connEndedError(): Error & { code: string } { + const e = new Error('write CONNECTION_ENDED localhost:6543') as Error & { code: string }; + e.code = 'CONNECTION_ENDED'; + return e; +} + +const AUDIT_DIR = join(tmpdir(), `gbrain-queue-lock-retry-${process.pid}-${Date.now()}`); +// Fast retry + isolated audit dir so the test doesn't sleep ~1s or pollute ~/.gbrain. +const FAST_ENV = { + GBRAIN_BULK_RETRY_BASE_MS: '1', + GBRAIN_BULK_RETRY_MAX_MS: '2', + GBRAIN_AUDIT_DIR: AUDIT_DIR, +}; + +describe('MinionQueue lock-path recovery (issue #1678)', () => { + it('promoteDelayed reconnects + retries on a reaped-socket error', async () => { + await withEnv(FAST_ENV, async () => { + let calls = 0; + let reconnects = 0; + const engine = { + kind: 'postgres', + executeRaw: async () => { + calls++; + if (calls === 1) throw connEndedError(); + return []; + }, + reconnect: async () => { reconnects++; }, + } as unknown as ConstructorParameters[0]; + + const q = new MinionQueue(engine); + const out = await q.promoteDelayed(); + expect(out).toEqual([]); + expect(calls).toBe(2); // first attempt threw, retry succeeded + expect(reconnects).toBe(1); // reconnect fired between attempts + }); + }); + + it('claim does NOT retry inline on a reaped-socket error (Codex #1 double-claim guard)', async () => { + await withEnv(FAST_ENV, async () => { + let calls = 0; + const engine = { + kind: 'postgres', + executeRaw: async () => { calls++; throw connEndedError(); }, + reconnect: async () => {}, + } as unknown as ConstructorParameters[0]; + + const q = new MinionQueue(engine); + await expect(q.claim('tok', 1000, 'default', ['sync'])).rejects.toThrow('CONNECTION_ENDED'); + expect(calls).toBe(1); // exactly one attempt — no inline retry + }); + }); +}); diff --git a/test/retry-matcher.test.ts b/test/retry-matcher.test.ts index e4b4728b8..5b8c33a31 100644 --- a/test/retry-matcher.test.ts +++ b/test/retry-matcher.test.ts @@ -75,6 +75,24 @@ describe('isRetryableConnError', () => { test('does not match arbitrary errors', () => { expect(isRetryableConnError(new Error('something else'))).toBe(false); }); + + // issue #1678: postgres.js's transaction-mode pooler reaps idle sockets and + // throws errors carrying `code: 'CONNECTION_ENDED'` (a library code, not an + // 08xxx SQLSTATE). Must be retryable via BOTH the code and the message form. + test('matches CONNECTION_ENDED via code', () => { + expect(isRetryableConnError(pgError('CONNECTION_ENDED', 'write CONNECTION_ENDED'))).toBe(true); + }); + + test('matches CONNECTION_ENDED via message even without the code', () => { + expect(isRetryableConnError(new Error('write CONNECTION_ENDED localhost:6543'))).toBe(true); + }); + + // The getter self-heal throws a GBrainError whose `problem` field is + // 'No database connection' — the existing typed-shape match must keep firing. + test('matches the instance-pool-reaped GBrainError shape (problem field)', () => { + const err = { problem: 'No database connection', message: 'instance pool torn down' }; + expect(isRetryableConnError(err)).toBe(true); + }); }); describe('isRetryableError', () => { diff --git a/test/rss-default.test.ts b/test/rss-default.test.ts new file mode 100644 index 000000000..ced4f14b7 --- /dev/null +++ b/test/rss-default.test.ts @@ -0,0 +1,98 @@ +/** + * issue #1678 — cgroup-aware auto-sized RSS watchdog default. + * + * The load-bearing case (Codex #5): a tiny cgroup limit on a huge host must + * win, so the watchdog cap sits BELOW the real ceiling and the graceful drain + * beats the kernel OOM-killer. Plain os.totalmem() would pick a 16GB cap on a + * 4GB-limited container and re-break into a silent SIGKILL. + */ + +import { describe, it, expect } from 'bun:test'; +import { + resolveDefaultMaxRssMb, + describeDefaultMaxRss, + readCgroupMemLimitBytes, + RSS_DEFAULT_FLOOR_MB, + RSS_DEFAULT_CEIL_MB, +} from '../src/core/minions/rss-default.ts'; + +const GB = 1024 * 1024 * 1024; + +describe('resolveDefaultMaxRssMb — clamp', () => { + it('8GB host (no cgroup) → floor 4096', () => { + expect(resolveDefaultMaxRssMb({ totalMemBytes: 8 * GB, cgroupLimitBytes: null })).toBe(4096); + }); + + it('16GB host → 8192 (0.5x, inside the band)', () => { + expect(resolveDefaultMaxRssMb({ totalMemBytes: 16 * GB, cgroupLimitBytes: null })).toBe(8192); + }); + + it('32GB host → ceil 16384', () => { + expect(resolveDefaultMaxRssMb({ totalMemBytes: 32 * GB, cgroupLimitBytes: null })).toBe(16384); + }); + + it('126GB host → ceil 16384 (the incident box)', () => { + expect(resolveDefaultMaxRssMb({ totalMemBytes: 126 * GB, cgroupLimitBytes: null })).toBe(16384); + }); + + it('result always within [floor, ceil] for huge hosts', () => { + const mb = resolveDefaultMaxRssMb({ totalMemBytes: 1024 * GB, cgroupLimitBytes: null }); + expect(mb).toBeGreaterThanOrEqual(RSS_DEFAULT_FLOOR_MB); + expect(mb).toBeLessThanOrEqual(RSS_DEFAULT_CEIL_MB); + }); +}); + +describe('resolveDefaultMaxRssMb — cgroup limit wins (Codex #5)', () => { + it('4GB cgroup on a 126GB host → cap stays BELOW the 4GB ceiling', () => { + const d = describeDefaultMaxRss({ totalMemBytes: 126 * GB, cgroupLimitBytes: 4 * GB }); + expect(d.source).toBe('cgroup-limited'); + expect(d.basisMb).toBe(4096); + // 0.5x4096 = 2048, below the 4096 floor but the floor must NOT push the cap + // up to/above the real 4GB ceiling — that would defeat drain-before-OOM. + expect(d.mb).toBeLessThan(4096); + expect(d.mb).toBe(2048); + }); + + it('8GB cgroup on a big host → 4096 (0.5x), source cgroup-limited', () => { + const d = describeDefaultMaxRss({ totalMemBytes: 64 * GB, cgroupLimitBytes: 8 * GB }); + expect(d.mb).toBe(4096); + expect(d.source).toBe('cgroup-limited'); + }); + + it('cgroup limit >= host RAM reads as host (unlimited sentinel collapses via min)', () => { + const d = describeDefaultMaxRss({ totalMemBytes: 16 * GB, cgroupLimitBytes: 9_223_372_036_854_771_712 }); + expect(d.source).toBe('host'); + expect(d.mb).toBe(8192); + }); +}); + +describe('readCgroupMemLimitBytes', () => { + it('cgroup v2 "max" → null (no enforced limit)', () => { + const read = (p: string) => { + if (p === '/sys/fs/cgroup/memory.max') return 'max\n'; + throw new Error('ENOENT'); + }; + expect(readCgroupMemLimitBytes(read)).toBeNull(); + }); + + it('cgroup v2 numeric → that value', () => { + const read = (p: string) => { + if (p === '/sys/fs/cgroup/memory.max') return String(4 * GB) + '\n'; + throw new Error('ENOENT'); + }; + expect(readCgroupMemLimitBytes(read)).toBe(4 * GB); + }); + + it('falls back to cgroup v1 when v2 unreadable', () => { + const read = (p: string) => { + if (p === '/sys/fs/cgroup/memory/memory.limit_in_bytes') return String(2 * GB); + throw new Error('ENOENT'); + }; + expect(readCgroupMemLimitBytes(read)).toBe(2 * GB); + }); + + it('neither file present → null', () => { + const read = () => { throw new Error('ENOENT'); }; + expect(readCgroupMemLimitBytes(read)).toBeNull(); + }); +}); diff --git a/test/supervisor-audit.test.ts b/test/supervisor-audit.test.ts index 7f128c4b4..b32d21186 100644 --- a/test/supervisor-audit.test.ts +++ b/test/supervisor-audit.test.ts @@ -130,11 +130,35 @@ describe('summarizeCrashes — aggregation', () => { const summary = summarizeCrashes([]); expect(summary).toEqual({ total: 0, - by_cause: { runtime_error: 0, oom_or_external_kill: 0, unknown: 0, legacy: 0 }, + by_cause: { runtime_error: 0, oom_or_external_kill: 0, rss_watchdog: 0, unknown: 0, legacy: 0 }, clean_exits: 0, }); }); + // issue #1678: rss_watchdog is a crash-classified cause (NOT in + // CLEAN_EXIT_CAUSES) with its OWN bucket — operators watching + // by_cause.rss_watchdog rise know the cap is too low for the workload, a + // distinct signal from a generic runtime_error or OOM-killer SIGKILL. + test('rss_watchdog routes to its own bucket, not legacy', () => { + const summary = summarizeCrashes([ + evt('worker_exited', { likely_cause: 'rss_watchdog' }), + evt('worker_exited', { likely_cause: 'rss_watchdog' }), + evt('worker_exited', { likely_cause: 'runtime_error' }), + ]); + expect(summary.total).toBe(3); + expect(summary.by_cause.rss_watchdog).toBe(2); + expect(summary.by_cause.runtime_error).toBe(1); + expect(summary.by_cause.legacy).toBe(0); + expect(summary.clean_exits).toBe(0); + }); + + // isCrashExit treats rss_watchdog as a crash (it's a real problem), NOT a + // clean exit — pins that the worker draining itself on a too-low cap shows + // up in operator health surfaces instead of looking like a clean drain. + test('isCrashExit classifies rss_watchdog as a crash', () => { + expect(isCrashExit(evt('worker_exited', { likely_cause: 'rss_watchdog' }))).toBe(true); + }); + test('only non-exit events returns zero summary', () => { const summary = summarizeCrashes([evt('started'), evt('worker_spawned'), evt('stopped')]); expect(summary.total).toBe(0); diff --git a/test/supervisor.test.ts b/test/supervisor.test.ts index badbb1d22..80f95d577 100644 --- a/test/supervisor.test.ts +++ b/test/supervisor.test.ts @@ -414,21 +414,20 @@ describe('MinionSupervisor', () => { }, 15_000); }); - describe('integration: --max-rss spawn args (v0.21)', () => { - it('passes --max-rss 2048 to spawned worker by default', async () => { + describe('integration: --max-rss spawn args (v0.21, auto-sized v0.41.39.0)', () => { + it('passes an explicit --max-rss through to the spawned worker', async () => { const outFile = join(tmpdir(), `gbrain-sup-maxrss-${process.pid}-${Date.now()}.txt`); try { unlinkSync(outFile); } catch { /* may not exist */ } - // Worker logs its argv to OUT_FILE so the test can assert --max-rss 2048 - // landed there. spawnOnce in supervisor.ts builds: - // ['jobs', 'work', '--concurrency', '1', '--queue', 'default', '--max-rss', '2048'] - // exit 1 required post-D1/D2: code=0 workers respawn forever. - const h = makeHarness('maxrss-default', `printf '%s\\n' "$*" > "$OUT_FILE" ; exit 1`); + // SUP_MAX_RSS pins an explicit cap; the supervisor must pass it through + // verbatim. exit 1 required post-D1/D2: code=0 workers respawn forever. + const h = makeHarness('maxrss-explicit', `printf '%s\\n' "$*" > "$OUT_FILE" ; exit 1`); try { const sup = spawnSupervisor(h, { OUT_FILE: outFile, SUP_MAX_CRASHES: '1', + SUP_MAX_RSS: '2048', }); await sup.exited; @@ -441,6 +440,37 @@ describe('MinionSupervisor', () => { h.cleanup(); } }, 15_000); + + // issue #1678: with no explicit cap the supervisor auto-sizes cgroup-aware + // instead of the old flat 2048 footgun. Same machine → the in-test + // resolveDefaultMaxRssMb() equals what the spawned supervisor computes. + it('auto-sizes --max-rss when no explicit cap is given', async () => { + const outFile = join(tmpdir(), `gbrain-sup-maxrss-auto-${process.pid}-${Date.now()}.txt`); + try { unlinkSync(outFile); } catch { /* may not exist */ } + + const { resolveDefaultMaxRssMb } = await import('../src/core/minions/rss-default.ts'); + const expected = resolveDefaultMaxRssMb(); + + const h = makeHarness('maxrss-auto', `printf '%s\\n' "$*" > "$OUT_FILE" ; exit 1`); + try { + const sup = spawnSupervisor(h, { + OUT_FILE: outFile, + SUP_MAX_CRASHES: '1', + }); + await sup.exited; + + expect(existsSync(outFile)).toBe(true); + const argv = readFileSync(outFile, 'utf8').trim(); + expect(argv).toContain(`--max-rss ${expected}`); + // Auto-sized value is clamped into the sane range, never the old 2048 + // unless the box genuinely resolves there. + expect(expected).toBeGreaterThanOrEqual(4096); + expect(expected).toBeLessThanOrEqual(16384); + } finally { + try { unlinkSync(outFile); } catch { /* noop */ } + h.cleanup(); + } + }, 15_000); }); describe('integration: audit file rotation + helper', () => { diff --git a/test/worker-lock-renewal.test.ts b/test/worker-lock-renewal.test.ts index 751ccea89..63c49c12b 100644 --- a/test/worker-lock-renewal.test.ts +++ b/test/worker-lock-renewal.test.ts @@ -483,3 +483,69 @@ describe('resolveLockRenewalKnobs', () => { expect(resolveLockRenewalKnobs({}, 30_000).maxFailuresForAudit).toBe(3); }); }); + +// issue #1678 (Codex #2): the bounded reconnect-once hook. NOT a withRetry on +// renewLock (that races this tick's own timeout); a single pool rebuild before +// the next tick so the next renewLock hits a live connection. +describe('runLockRenewalTick: reconnect-once dep (issue #1678)', () => { + test('reconnect fires after a renewLock throw within deadline; result still ok', async () => { + const audit = freshAudit(); + let reconnectCalls = 0; + const deps: LockRenewalDeps = { + renewLock: async () => { throw new Error('write CONNECTION_ENDED'); }, + audit: audit.sink, + now: () => 1000, // well within deadline + setTimeout: makeFakeTimer().setTimeout, + reconnect: async () => { reconnectCalls++; }, + }; + const state = makeState({ lastSuccessfulRenewalAt: 0 }); + const result = await runLockRenewalTick(deps, state); + expect(result).toEqual({ kind: 'ok' }); + expect(reconnectCalls).toBe(1); + expect(state.consecutiveFailures).toBe(1); + }); + + test('a reconnect throw is swallowed — tick still returns ok (no unhandledRejection class)', async () => { + const audit = freshAudit(); + const deps: LockRenewalDeps = { + renewLock: async () => { throw new Error('Connection terminated unexpectedly'); }, + audit: audit.sink, + now: () => 1000, + setTimeout: makeFakeTimer().setTimeout, + reconnect: async () => { throw new Error('reconnect failed: EHOSTUNREACH'); }, + }; + const state = makeState({ lastSuccessfulRenewalAt: 0 }); + const result = await runLockRenewalTick(deps, state); + expect(result).toEqual({ kind: 'ok' }); + }); + + test('reconnect is NOT called on a successful renewal', async () => { + let reconnectCalls = 0; + const deps: LockRenewalDeps = { + renewLock: async () => true, + audit: freshAudit().sink, + now: () => 1000, + setTimeout: makeFakeTimer().setTimeout, + reconnect: async () => { reconnectCalls++; }, + }; + const result = await runLockRenewalTick(deps, makeState({ lastSuccessfulRenewalAt: 500 })); + expect(result).toEqual({ kind: 'ok' }); + expect(reconnectCalls).toBe(0); + }); + + test('reconnect is NOT called when the tick aborts at the deadline', async () => { + let reconnectCalls = 0; + const deps: LockRenewalDeps = { + renewLock: async () => { throw new Error('write CONNECTION_ENDED'); }, + audit: freshAudit().sink, + // sinceLastSuccess = 30000 - 0 = 30000 >= deadline (30000-5000=25000) → abort + now: () => 30_000, + setTimeout: makeFakeTimer().setTimeout, + reconnect: async () => { reconnectCalls++; }, + }; + const state = makeState({ lastSuccessfulRenewalAt: 0 }); + const result = await runLockRenewalTick(deps, state); + expect(result).toEqual({ kind: 'should_abort', reason: 'lock-renewal-failed' }); + expect(reconnectCalls).toBe(0); // pointless to reconnect when we're giving up the lock + }); +}); diff --git a/test/worker-watchdog-trigger.test.ts b/test/worker-watchdog-trigger.test.ts new file mode 100644 index 000000000..4d35a52c2 --- /dev/null +++ b/test/worker-watchdog-trigger.test.ts @@ -0,0 +1,80 @@ +/** + * issue #1678 — worker-side RSS watchdog behavior. + * + * Pins that: + * 1. Crossing the cap sets `rssWatchdogTriggered` (the flag the CLI reads to + * exit with WORKER_EXIT_RSS_WATCHDOG) and drains the worker. + * 2. The 80%-of-cap soft warn fires BEFORE the kill, once per crossing, + * carrying the peak + in-flight job kinds — and does NOT drain. + * + * Uses a real in-memory PGLite engine (canonical block per CLAUDE.md R3+R4) + * + a stubbed `getRss` so the test is deterministic and hermetic. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { MinionWorker } from '../src/core/minions/worker.ts'; + +const MB = 1024 * 1024; + +let engine: PGLiteEngine; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); +}); + +afterAll(async () => { + await engine.disconnect(); +}); + +describe('worker RSS watchdog (issue #1678)', () => { + it('crossing the cap sets rssWatchdogTriggered and drains', async () => { + const worker = new MinionWorker(engine, { + queue: 'default', + concurrency: 1, + maxRssMb: 100, + getRss: () => 500 * MB, // 5x the cap + rssCheckInterval: 25, + healthCheckInterval: 0, // no self-health timer in this test + pollInterval: 25, + }); + worker.register('noop', async () => {}); + + // start() resolves on its own: the periodic check trips the watchdog, + // gracefulShutdown sets running=false, the loop exits. + await worker.start(); + expect(worker.rssWatchdogTriggered).toBe(true); + }); + + it('80% soft-warn fires before the kill and does not drain', async () => { + const warns: string[] = []; + const origWarn = console.warn; + console.warn = (...a: unknown[]) => { warns.push(a.join(' ')); }; + + const worker = new MinionWorker(engine, { + queue: 'default', + concurrency: 1, + maxRssMb: 100, + getRss: () => 85 * MB, // 85% — above soft line, below cap + rssCheckInterval: 25, + healthCheckInterval: 0, + pollInterval: 25, + }); + worker.register('noop', async () => {}); + + const runPromise = worker.start(); + // Let a couple of periodic checks fire. + await new Promise((r) => setTimeout(r, 120)); + worker.stop(); + await runPromise; + console.warn = origWarn; + + const softWarn = warns.find((w) => w.includes('approaching cap')); + expect(softWarn).toBeDefined(); + expect(softWarn).toContain('85%'); + // Soft warn must NOT have drained the worker. + expect(worker.rssWatchdogTriggered).toBe(false); + }); +});