v0.35.5.1 fix(doctor): stop counting clean supervisor exits as crashes (#1108)

* feat(supervisor-audit): shared isCrashExit + summarizeCrashes classifier

Adds the read-side foundation for reading `likely_cause` off `worker_exited`
audit events. Denylist semantics — only `clean_exit` and `graceful_shutdown`
are non-crashes. Future unrecognized causes surface by default.

`isCrashExit(event)` classifies a single audit event with legacy
`code !== 0` fallback for pre-v0.34 entries lacking `likely_cause`.

`summarizeCrashes(events)` aggregates a 24h window into a `CrashSummary`
with per-cause counts (runtime_error, oom_or_external_kill, unknown,
legacy) and a `clean_exits` total.

Both helpers live next to `readSupervisorEvents` so the producer (the
JSONL writer) and the consumers (doctor + jobs CLI) share one regression
point. Test matrix pins all 9 isCrashExit branches plus 5 summarizeCrashes
aggregation cases including the future-cause denylist regression guard.

* fix(doctor,jobs): wire supervisor check to summarizeCrashes

`gbrain doctor` and `gbrain jobs supervisor status` both counted every
`worker_exited` audit event as a crash, regardless of `likely_cause`.
After v0.34.3.0 added RSS-watchdog drains (code=0), the count inflated
to 120+/day on a healthy brain — the alarm pattern users reported.

Both surfaces now go through `summarizeCrashes(events)` (single
regression point, can't drift). The warn threshold drops from `>3`
to `>=1` now that the counter is calibrated; the per-cause breakdown
(runtime=N oom=M unknown=K legacy=L) gives operators triage context
in the message without grep'ing the JSONL audit.

`gbrain jobs supervisor status --json` adds `crashes_by_cause` and
`clean_exits_24h` fields so monitoring dashboards bind to the named
buckets.

4 source-grep wiring assertions in doctor.test.ts pin both call sites
against drift.

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

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* docs: document v0.35.5.0 supervisor-audit crash classifier

Add CLAUDE.md entry for src/core/minions/handlers/supervisor-audit.ts
covering the new isCrashExit/summarizeCrashes/CrashSummary/CLEAN_EXIT_CAUSES
exports. Extend doctor.ts and jobs.ts entries with the v0.35.5.0
wire-up: shared helper, denylist semantics, >=1 warn threshold, per-cause
breakdown in messages, crashes_by_cause + clean_exits_24h in JSON.
Regenerate llms-full.txt to match.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-05-17 14:27:31 -07:00
committed by GitHub
co-authored by Claude Opus 4.7
parent 4446e9f9d2
commit 0620094121
11 changed files with 423 additions and 34 deletions
+45
View File
@@ -2,6 +2,51 @@
All notable changes to GBrain will be documented in this file.
## [0.35.5.1] - 2026-05-16
**`gbrain doctor` stops counting clean supervisor exits as crashes — the "120x/24h" alarm finally reflects real crashes, with per-cause breakdown for operator triage.**
`doctor.ts:1013` and `gbrain jobs supervisor status` at `jobs.ts:805` both counted every `worker_exited` audit event as a crash, regardless of cause. After v0.34.3.0's RSS-watchdog work added more code=0 worker drains, the count inflated to 120+/day on a healthy brain — the exact alarm pattern users started seeing ("Supervisor crashes: 120x/24h, was 62x — nearly doubled"). The classifier upstream in `child-worker-supervisor.ts:309-321` was already stamping a five-value `likely_cause` field on every exit (clean_exit, graceful_shutdown, runtime_error, oom_or_external_kill, unknown); neither read site looked at it. v0.35.5.1 ships a shared `summarizeCrashes(events)` helper colocated with `readSupervisorEvents`, denylist semantics so future failure modes surface by default, threshold dropped from `>3` to `>=1`, and per-cause breakdown in the messages so an operator triages in one glance.
**Relationship to v0.35.4.0:** that release shipped a binary `classifyWorkerExit({code})` helper (clean if `code === 0`, crash otherwise). It correctly stopped counting RSS-watchdog drains as crashes, but treated SIGTERM-driven graceful shutdowns as crashes (because `null !== 0`) and lost the cause distinction operators need to triage. v0.35.5.1 layers on top: reads `likely_cause` so `graceful_shutdown` is correctly clean, and bucketizes real crashes into `runtime_error` (code bugs), `oom_or_external_kill` (memory pressure), `unknown` (other), `legacy` (pre-v0.34 or future unrecognized). The `classifyWorkerExit` helper from v0.35.4.0 remains in use by the supervisor's internal restart policy where the binary check is the right shape; the doctor + jobs surfaces use the richer classifier.
### What you can now do
**Trust the doctor's supervisor count.** `gbrain doctor` reports actual crashes, not RSS-watchdog drains or SIGTERM stops. The "120x/24h" alarm class is dead.
**See per-cause breakdown at a glance.** The warn message reads `Worker crashed Nx in last 24h (runtime=A oom=B unknown=C legacy=D)` — distinguishes memory pressure (oom) from code bugs (runtime) without grep'ing the JSONL audit.
**Hook `crashes_by_cause` into monitoring.** `gbrain jobs supervisor status --json` now exposes `crashes_by_cause: {runtime_error, oom_or_external_kill, unknown, legacy}` and `clean_exits_24h` fields. Dashboards bind to named buckets.
**Future failure modes surface by default.** Denylist semantics: only `clean_exit` and `graceful_shutdown` are non-crashes; everything else (including new `likely_cause` values added upstream in future releases) counts as a crash. The bug we just fixed cannot silently recur on the next schema addition.
### Itemized changes
- `src/core/minions/handlers/supervisor-audit.ts` exports new `isCrashExit(event)`, `summarizeCrashes(events)`, `CrashSummary` type, and `CLEAN_EXIT_CAUSES` constant. Single regression point — both CLI surfaces import from here so they cannot drift.
- `src/commands/doctor.ts:1011-1043` replaces the ad-hoc `events.filter(e => e.event === 'worker_exited').length` with `summarizeCrashes(events)`. Drops the warn threshold from `>3` to `>=1` (any real crash is signal now that the counter is calibrated). Widens the ok message with `clean_exits_24h=N` and the warn message with `runtime=N oom=M unknown=K legacy=L`.
- `src/commands/jobs.ts:803-826` same wiring. JSON output adds `crashes_by_cause` and `clean_exits_24h` fields. Human output gains a per-cause line under `Crashes (24h)` plus a `Clean exits (24h)` line.
- `test/supervisor-audit.test.ts` (new) — 14 unit cases: 9-case `isCrashExit` branch matrix (every `likely_cause` value, denylist regression guard for unrecognized future causes, legacy fallback paths for pre-v0.34 entries, non-exit-event defensive case), 5-case `summarizeCrashes` aggregator (mixed-stream bucket assertions, empty input, non-exit-only, unrecognized-cause routing to legacy, null-code edge case).
- `test/doctor.test.ts` — 4 source-grep wiring assertions guarding both surfaces against drift: doctor + jobs.ts use `summarizeCrashes`, the ad-hoc filter pattern is gone, threshold is `>=1`, per-cause breakdown substrings (`runtime=`, `oom=`, `unknown=`, `legacy=`, `clean_exits_24h=`, `crashes_by_cause`) appear in both files.
- Why denylist semantics (codex outside-voice catch during `/plan-eng-review`): the set of clean-exit causes is small and explicit (the worker exited because we asked it to); the set of crash causes is open-ended. Allowlist would silently underreport future failure modes; denylist surfaces them by default. The bug being fixed today is itself an allowlist-of-event-names — the denylist shape forecloses the recurrence.
- Plan + review trail: `/plan-eng-review` cleared the plan with 3 substantive findings resolved (shared helper extraction, denylist over allowlist, threshold rebaseline pulled into scope). Codex outside-voice surfaced the duplicate bug at `jobs.ts:805` (would have shipped doctor-only otherwise and let the two surfaces drift). Coverage audit reports 100% on the 22 logical branches the diff introduces. Adversarial review surfaced 13 follow-up observations, all either pre-existing surfaces or accepted plan trade-offs — none in-scope for this PR.
## To take advantage of v0.35.5.1
`gbrain upgrade` is all you need. No schema migration, no config change, no manual action.
1. Run `gbrain upgrade`.
2. Verify the count:
```bash
gbrain doctor 2>&1 | grep -i supervisor
gbrain jobs supervisor status --json | jq '{crashes_24h, clean_exits_24h, crashes_by_cause}'
```
Both commands MUST report identical `crashes_24h` (cross-surface parity is the regression guard). If the new count is still high, the watchdog OOMs / SIGKILLs are real — investigate via `~/.gbrain/audit/supervisor-*.jsonl` or check the per-cause breakdown in the warn message.
3. Cross-check with raw JSONL if you want ground truth:
```bash
jq -r 'select(.event=="worker_exited") | .likely_cause' ~/.gbrain/audit/supervisor-*.jsonl | sort | uniq -c
```
4. If anything looks wrong, file an issue: https://github.com/garrytan/gbrain/issues with the doctor output and the relevant lines from the audit JSONL.
## [0.35.5.0] - 2026-05-16
**Upgrade goes through on stuck Supabase brains. Orphan counts get honest. `gbrain think` over MCP actually answers. Worktrees stop being misclassified. node_modules stops leaking.**
+3 -2
View File
File diff suppressed because one or more lines are too long
+1 -1
View File
@@ -1 +1 @@
0.35.5.0
0.35.5.1
+3 -2
View File
File diff suppressed because one or more lines are too long
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "gbrain",
"version": "0.35.5.0",
"version": "0.35.5.1",
"description": "Postgres-native personal knowledge brain with hybrid RAG search",
"type": "module",
"main": "src/core/index.ts",
+21 -11
View File
@@ -8,7 +8,6 @@ import { loadCompletedMigrations } from '../core/preferences.ts';
import { compareVersions } from './migrations/index.ts';
import { createProgress, startHeartbeat, type ProgressReporter } from '../core/progress.ts';
import { getCliOptions, cliOptsToProgressOptions } from '../core/cli-options.ts';
import { classifyWorkerExit } from '../core/minions/exit-classification.ts';
import type { DbUrlSource } from '../core/config.ts';
import { join } from 'path';
import { existsSync, readFileSync, readdirSync } from 'fs';
@@ -994,7 +993,7 @@ export async function runDoctor(engine: BrainEngine | null, args: string[], dbSo
// Does NOT run the supervisor itself — this is a read-only health check.
try {
const { DEFAULT_PID_FILE } = await import('../core/minions/supervisor.ts');
const { readSupervisorEvents } = await import('../core/minions/handlers/supervisor-audit.ts');
const { readSupervisorEvents, summarizeCrashes } = await import('../core/minions/handlers/supervisor-audit.ts');
let supervisorPid: number | null = null;
let running = false;
@@ -1011,12 +1010,20 @@ export async function runDoctor(engine: BrainEngine | null, args: string[], dbSo
const events = readSupervisorEvents({ sinceMs: 24 * 60 * 60 * 1000 });
const lastStart = events.filter(e => e.event === 'started').pop()?.ts ?? null;
// Only count non-zero exits as crashes; clean restarts (code 0) are normal
// worker lifecycle — the worker finishes its queue and exits cleanly.
const allExits = events.filter(e => e.event === 'worker_exited');
const realCrashes = allExits.filter(e => classifyWorkerExit(e as { code?: number | null }) === 'crash');
const cleanRestarts = allExits.length - realCrashes.length;
const crashes24h = realCrashes.length;
// Shared classifier — same code path runs in `gbrain jobs supervisor
// status` (src/commands/jobs.ts). Counts only events whose `likely_cause`
// is NOT in the clean denylist (clean_exit, graceful_shutdown). Pre-v0.34
// entries lacking `likely_cause` fall back to `code !== 0`. Supersedes
// v0.35.4.0's binary `classifyWorkerExit({code})` on this surface: the
// `likely_cause` read correctly classifies SIGTERM (code=null,
// likely_cause='graceful_shutdown') as clean, and produces per-cause
// buckets so operators triage memory pressure (oom) vs code bugs
// (runtime) without grep'ing JSONL. `classifyWorkerExit` is still
// used by the supervisor's internal restart policy where the binary
// shape is the right contract.
const summary = summarizeCrashes(events);
const crashes24h = summary.total;
const causeStr = `runtime=${summary.by_cause.runtime_error} oom=${summary.by_cause.oom_or_external_kill} unknown=${summary.by_cause.unknown} legacy=${summary.by_cause.legacy}`;
const maxCrashesEvent = events.filter(e => e.event === 'max_crashes_exceeded').pop() ?? null;
// Only surface a Check if the supervisor was ever observed (stops the
@@ -1034,17 +1041,20 @@ export async function runDoctor(engine: BrainEngine | null, args: string[], dbSo
status: 'warn',
message: `Supervisor not running (last_start=${lastStart ?? 'unknown'}). Restart with: gbrain jobs supervisor start --detach`,
});
} else if (crashes24h > 3) {
} else if (crashes24h >= 1) {
// Threshold dropped from `>3` (pre-fix, inflated by clean exits being
// miscounted) to `>=1` (any real crash is signal). Per-cause breakdown
// gives operators triage context without grep'ing the JSONL.
checks.push({
name: 'supervisor',
status: 'warn',
message: `Supervisor running but worker crashed ${crashes24h}x in last 24h (${cleanRestarts} clean restarts excluded). Check ~/.gbrain/audit/supervisor-*.jsonl for causes.`,
message: `Worker crashed ${crashes24h}x in last 24h (${causeStr}). Check ~/.gbrain/audit/supervisor-*.jsonl for context.`,
});
} else {
checks.push({
name: 'supervisor',
status: 'ok',
message: `running=true pid=${supervisorPid} last_start=${lastStart ?? 'unknown'} crashes=${crashes24h}${cleanRestarts > 0 ? ` clean_restarts=${cleanRestarts}` : ''}`,
message: `running=true pid=${supervisorPid} last_start=${lastStart ?? 'unknown'} crashes_24h=${crashes24h} clean_exits_24h=${summary.clean_exits}`,
});
}
}
+11 -9
View File
@@ -6,7 +6,6 @@
import type { BrainEngine } from '../core/engine.ts';
import { MinionQueue } from '../core/minions/queue.ts';
import { MinionWorker } from '../core/minions/worker.ts';
import { classifyWorkerExit } from '../core/minions/exit-classification.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';
@@ -786,7 +785,7 @@ HANDLER TYPES (built in)
// ----- status subcommand -----
if (isStatusCmd) {
const { existsSync, readFileSync } = await import('fs');
const { readSupervisorEvents } = await import('../core/minions/handlers/supervisor-audit.ts');
const { readSupervisorEvents, summarizeCrashes } = await import('../core/minions/handlers/supervisor-audit.ts');
let supervisorPid: number | null = null;
let running = false;
@@ -803,10 +802,11 @@ HANDLER TYPES (built in)
const events = readSupervisorEvents({ sinceMs: 24 * 60 * 60 * 1000 });
const lastStart = events.filter(e => e.event === 'started').pop()?.ts ?? null;
const allExits = events.filter(e => e.event === 'worker_exited');
const realCrashes = allExits.filter(e => classifyWorkerExit(e as { code?: number | null }) === 'crash');
const cleanRestarts = allExits.length - realCrashes.length;
const crashes24h = realCrashes.length;
// Shared classifier — same code path runs in `gbrain doctor` so the
// two surfaces cannot drift on what counts as a crash. Supersedes
// v0.35.4.0's binary `classifyWorkerExit({code})` on this surface;
// see doctor.ts for the layering rationale.
const summary = summarizeCrashes(events);
const maxCrashesEvent = events.filter(e => e.event === 'max_crashes_exceeded').pop() ?? null;
const status = {
@@ -814,8 +814,9 @@ HANDLER TYPES (built in)
supervisor_pid: supervisorPid,
pid_file: pidFile,
last_start: lastStart,
crashes_24h: crashes24h,
clean_restarts_24h: cleanRestarts,
crashes_24h: summary.total,
clean_exits_24h: summary.clean_exits,
crashes_by_cause: summary.by_cause,
max_crashes_exceeded: !!maxCrashesEvent,
};
@@ -826,7 +827,8 @@ HANDLER TYPES (built in)
if (supervisorPid) console.log(` PID: ${supervisorPid}`);
console.log(` PID file: ${pidFile}`);
if (lastStart) console.log(` Last start: ${lastStart}`);
console.log(` Crashes (24h): ${crashes24h}${cleanRestarts > 0 ? ` (${cleanRestarts} clean restarts excluded)` : ''}`);
console.log(` Crashes (24h): ${summary.total} (runtime=${summary.by_cause.runtime_error} oom=${summary.by_cause.oom_or_external_kill} unknown=${summary.by_cause.unknown} legacy=${summary.by_cause.legacy})`);
console.log(` Clean exits (24h): ${summary.clean_exits}`);
if (maxCrashesEvent) console.log(` ⚠ Max crashes exceeded at ${maxCrashesEvent.ts}`);
}
process.exit(running ? 0 : 1);
@@ -105,3 +105,90 @@ export function readSupervisorEvents(opts: { sinceMs?: number } = {}): Superviso
}
return events;
}
/**
* Denylist of clean-exit `likely_cause` values. Anything not in this set —
* including future unrecognized values — counts as a crash. Matches the
* domain asymmetry: clean exits are explicit (the worker exited because we
* asked it to); crashes are an open catch-all. If a future maintainer adds a
* new `likely_cause` upstream in `child-worker-supervisor.ts` (e.g.
* `lock_lost`, `panic`), the doctor surfaces it by default instead of
* silently underreporting — denylist semantics close the bug class this
* helper was added to fix.
*/
const CLEAN_EXIT_CAUSES = new Set(['clean_exit', 'graceful_shutdown']);
/**
* Per-cause crash bucket shape returned by `summarizeCrashes()`. Bucket names
* mirror the upstream `likely_cause` values: `runtime_error` (code=1),
* `oom_or_external_kill` (SIGKILL), `unknown` (other signals/codes). The
* `legacy` bucket catches pre-v0.34 entries lacking `likely_cause` that fall
* through to the `code !== 0` fallback.
*/
export interface CrashSummary {
total: number;
by_cause: {
runtime_error: number;
oom_or_external_kill: number;
unknown: number;
legacy: number;
};
clean_exits: number;
}
/**
* Classify a single audit event. Returns true when the event represents a
* worker crash (not a clean shutdown, watchdog drain, or non-exit lifecycle
* event). Pre-v0.34 audit lines lacking `likely_cause` fall back to
* `code !== 0`.
*/
export function isCrashExit(event: SupervisorEmission): boolean {
if (event.event !== 'worker_exited') return false;
const cause = event.likely_cause as string | undefined;
if (cause === undefined) {
// Legacy fallback for pre-v0.34 entries lacking `likely_cause`. Treat
// any non-zero exit code as a crash; missing/null `code` also counts
// (truly malformed line — fail-loud, the user can investigate the audit
// file directly).
const code = event.code as number | null | undefined;
return code !== 0;
}
return !CLEAN_EXIT_CAUSES.has(cause);
}
/**
* Summarize crash counts across a window of supervisor audit events. Both
* `gbrain doctor` and `gbrain jobs supervisor status` consume this — single
* regression point, single test target.
*
* Bucketing rule: `worker_exited` events classified as crashes by
* `isCrashExit()` are dispatched to `by_cause` based on `likely_cause`. The
* `legacy` bucket catches BOTH (a) pre-v0.34 entries lacking `likely_cause`
* that fell through to the `code !== 0` fallback, AND (b) future
* unrecognized `likely_cause` values not in the explicit allowlist
* (`runtime_error` / `oom_or_external_kill` / `unknown`). Operators
* watching `legacy=N` rise know the upstream classifier added a value the
* doctor doesn't yet name — that's the intended signal for "extend my
* bucket vocabulary."
*/
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 },
clean_exits: 0,
};
for (const e of events) {
if (e.event !== 'worker_exited') continue;
if (!isCrashExit(e)) {
summary.clean_exits++;
continue;
}
summary.total++;
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 === 'unknown') summary.by_cause.unknown++;
else summary.by_cause.legacy++; // pre-v0.34 fallback OR future unrecognized cause
}
return summary;
}
+59
View File
@@ -579,6 +579,65 @@ describe('v0.32.4 — sync_freshness check', () => {
});
});
// Supervisor crash classifier wiring. Pre-fix, doctor.ts:1013 counted every
// `worker_exited` event as a crash regardless of `likely_cause`, inflating
// `crashes_24h` to 120+/day from RSS-watchdog drains and SIGTERM stops.
// These tests pin the read-side wiring so doctor and `gbrain jobs supervisor
// status` (jobs.ts:805) cannot drift: both go through `summarizeCrashes`.
describe('supervisor crash classifier wiring (v0.35.x)', () => {
test('doctor.ts uses summarizeCrashes — no ad-hoc worker_exited filter', async () => {
const source = await Bun.file(new URL('../src/commands/doctor.ts', import.meta.url)).text();
// Wired to the shared helper.
expect(source).toContain('summarizeCrashes');
// The pre-fix ad-hoc filter pattern must NOT survive. The exact buggy
// expression was `events.filter(e => e.event === 'worker_exited').length`.
// Match the structural fingerprint, not whitespace.
expect(source).not.toMatch(
/events\.filter\([^)]*e\.event\s*===\s*'worker_exited'[^)]*\)\.length/,
);
});
test('doctor.ts warn threshold dropped from >3 to >=1', async () => {
const source = await Bun.file(new URL('../src/commands/doctor.ts', import.meta.url)).text();
// The pre-fix `crashes24h > 3` threshold made sense only because the
// counter was over-counting clean exits. Under accurate counts, any real
// crash is signal — threshold lands at `>=1`.
expect(source).toMatch(/crashes24h\s*>=\s*1/);
// The old `> 3` predicate must not survive on the supervisor check.
expect(source).not.toMatch(/crashes24h\s*>\s*3/);
});
test('doctor.ts ok + warn messages include per-cause breakdown and clean_exits_24h', async () => {
const source = await Bun.file(new URL('../src/commands/doctor.ts', import.meta.url)).text();
// Per-cause breakdown surfaces qualitative signal (oom vs runtime vs unknown
// vs legacy) so operators can triage without grep'ing JSONL.
expect(source).toContain('runtime=');
expect(source).toContain('oom=');
expect(source).toContain('unknown=');
expect(source).toContain('legacy=');
// Clean-exit count surfaces alongside crash count for transparency.
expect(source).toContain('clean_exits_24h=');
});
test('jobs.ts supervisor status uses summarizeCrashes — same wiring as doctor', async () => {
const source = await Bun.file(new URL('../src/commands/jobs.ts', import.meta.url)).text();
// Both surfaces MUST go through the shared helper. Without this, the two
// CLI commands report drifting crash counts (the bug class codex caught
// during the eng review outside-voice pass).
expect(source).toContain('summarizeCrashes');
expect(source).not.toMatch(
/events\.filter\([^)]*e\.event\s*===\s*'worker_exited'[^)]*\)\.length/,
);
// JSON output exposes the per-cause breakdown so dashboards/monitors can
// distinguish memory pressure from code bugs without re-classifying.
expect(source).toContain('crashes_by_cause');
expect(source).toContain('clean_exits_24h');
});
});
// v0.34.5 stub-guard observability tests (from v0.35.4.0). Doctor surfaces
// the 24h fire count for the resolver-stub-guard. WARN at >10 hits is the
// signal that prefix-expansion in resolveEntitySlug is missing a case.
describe('stub_guard_24h check (v0.34.5)', () => {
test('doctor source defines the stub_guard_24h check', async () => {
const source = await Bun.file(new URL('../src/commands/doctor.ts', import.meta.url)).text();
+15 -8
View File
@@ -34,22 +34,29 @@ describe('consumer wire-up — helper used by all 3 sites (no inline filters lef
// accidentally inlines the rule again gets caught at test time, not at
// production-divergence time.
// v0.35.5.0: doctor.ts and jobs.ts moved from `classifyWorkerExit` (binary
// code-based) to `summarizeCrashes` (per-cause via `likely_cause`). The
// wire-up contract is "must use a shared helper, not an inline filter" —
// the specific helper differs by site. The supervisor's internal restart
// policy still uses `classifyWorkerExit` (binary is the right shape there).
const SITES = [
{ label: 'doctor.ts', path: 'src/commands/doctor.ts' },
{ label: 'jobs.ts', path: 'src/commands/jobs.ts' },
{ label: 'child-worker-supervisor.ts', path: 'src/core/minions/child-worker-supervisor.ts' },
{ label: 'doctor.ts', path: 'src/commands/doctor.ts', helper: 'summarizeCrashes' },
{ label: 'jobs.ts', path: 'src/commands/jobs.ts', helper: 'summarizeCrashes' },
{ label: 'child-worker-supervisor.ts', path: 'src/core/minions/child-worker-supervisor.ts', helper: 'classifyWorkerExit' },
];
for (const site of SITES) {
it(`${site.label} imports classifyWorkerExit`, () => {
it(`${site.label} uses a shared classifier helper (${site.helper})`, () => {
const source = readFileSync(join(import.meta.dir, '..', site.path), 'utf8');
// Either named import OR import-from of the helper file — both count.
expect(source).toMatch(/(import\s+\{[^}]*classifyWorkerExit[^}]*\}|from\s+['"][^'"]*exit-classification)/);
// Helper is either imported by name (top-level) or via dynamic import.
const helperRe = new RegExp(`\\b${site.helper}\\b`);
expect(source).toMatch(helperRe);
});
it(`${site.label} calls classifyWorkerExit at least once`, () => {
it(`${site.label} calls ${site.helper} at least once`, () => {
const source = readFileSync(join(import.meta.dir, '..', site.path), 'utf8');
expect(source).toMatch(/classifyWorkerExit\s*\(/);
const callRe = new RegExp(`\\b${site.helper}\\s*\\(`);
expect(source).toMatch(callRe);
});
}
+177
View File
@@ -0,0 +1,177 @@
/**
* Unit tests for the shared crash classifier used by `gbrain doctor` and
* `gbrain jobs supervisor status`. Both surfaces import `isCrashExit` +
* `summarizeCrashes` from `src/core/minions/handlers/supervisor-audit.ts`;
* pinning them here keeps the two CLI surfaces from drifting.
*
* Why this file exists: pre-fix the doctor counted every `worker_exited`
* event as a crash, regardless of `likely_cause`. Clean SIGTERM shutdowns
* and RSS-watchdog drains (code=0) inflated `crashes_24h` to 120+/day on
* Garry's brain. The classifier upstream in child-worker-supervisor.ts
* already stamped `likely_cause` correctly; the read sites just ignored it.
* These tests pin every branch of the new shared classifier so the bug
* cannot silently recur.
*/
import { describe, test, expect } from 'bun:test';
import {
isCrashExit,
summarizeCrashes,
type CrashSummary,
} from '../src/core/minions/handlers/supervisor-audit.ts';
import type { SupervisorEmission } from '../src/core/minions/supervisor.ts';
// Helper: build a SupervisorEmission of the given event with arbitrary extra
// fields. `ts` is required by the type but irrelevant to the classifier; we
// stamp a constant so failures show predictable fixtures.
function evt(
event: SupervisorEmission['event'],
extra: Record<string, unknown> = {},
): SupervisorEmission {
return { event, ts: '2026-05-16T00:00:00Z', ...extra };
}
describe('isCrashExit — branch matrix', () => {
// Case 1: explicit clean exit (worker returned code=0 voluntarily,
// e.g. RSS watchdog drain). The most common cause of the original
// bug — every drain was being counted as a crash.
test('clean_exit is not a crash', () => {
expect(isCrashExit(evt('worker_exited', { likely_cause: 'clean_exit', code: 0 }))).toBe(false);
});
// Case 2: SIGTERM-driven shutdown (operator stop, OS-initiated graceful
// termination). Also not a crash.
test('graceful_shutdown is not a crash', () => {
expect(isCrashExit(evt('worker_exited', { likely_cause: 'graceful_shutdown', signal: 'SIGTERM' }))).toBe(false);
});
// Case 3: code=1 from the worker process — a real runtime error.
test('runtime_error is a crash', () => {
expect(isCrashExit(evt('worker_exited', { likely_cause: 'runtime_error', code: 1 }))).toBe(true);
});
// Case 4: SIGKILL (kernel OOM kill, external `kill -9`). Real crash.
test('oom_or_external_kill is a crash', () => {
expect(isCrashExit(evt('worker_exited', { likely_cause: 'oom_or_external_kill', signal: 'SIGKILL' }))).toBe(true);
});
// Case 5: catch-all bucket from the upstream classifier (unusual code or
// signal combination). Real crash.
test('unknown is a crash', () => {
expect(isCrashExit(evt('worker_exited', { likely_cause: 'unknown', code: 137 }))).toBe(true);
});
// Case 6: denylist regression guard. If a future maintainer adds a NEW
// `likely_cause` value upstream (e.g. `lock_lost`, `panic`,
// `db_connection_lost`), the doctor MUST surface it by default. Allowlist
// semantics would have silently misclassified this as clean — the exact
// bug class this fix exists to close.
test('unrecognized future likely_cause is a crash (denylist regression guard)', () => {
expect(isCrashExit(evt('worker_exited', { likely_cause: 'future_value_not_known' }))).toBe(true);
});
// Case 7: legacy fallback path — pre-v0.34 audit lines lacking
// `likely_cause`. Use `code` to classify: code=0 is clean.
test('legacy (no likely_cause) with code=0 is not a crash', () => {
expect(isCrashExit(evt('worker_exited', { code: 0 }))).toBe(false);
});
// Case 8: legacy fallback path — pre-v0.34 entry with code=1 (real crash).
test('legacy (no likely_cause) with code=1 is a crash', () => {
expect(isCrashExit(evt('worker_exited', { code: 1 }))).toBe(true);
});
// Case 9: defensive — non-exit lifecycle events MUST never be counted as
// crashes regardless of their other fields. Catches a future caller that
// forgets the upstream `event === 'worker_exited'` filter.
test('non-exit event is never a crash', () => {
expect(isCrashExit(evt('started'))).toBe(false);
expect(isCrashExit(evt('worker_spawned', { code: 1 }))).toBe(false);
expect(isCrashExit(evt('max_crashes_exceeded', { likely_cause: 'runtime_error' }))).toBe(false);
});
});
describe('summarizeCrashes — aggregation', () => {
// Feed a representative mixed stream and assert every bucket. The mix
// exercises every classifier branch so the message-format consumers
// (doctor.ts and jobs.ts) get a stable shape.
test('aggregates a mixed stream into per-cause buckets and clean_exits', () => {
const events: SupervisorEmission[] = [
evt('worker_exited', { likely_cause: 'runtime_error', code: 1 }),
evt('worker_exited', { likely_cause: 'runtime_error', code: 1 }),
evt('worker_exited', { likely_cause: 'oom_or_external_kill', signal: 'SIGKILL' }),
evt('worker_exited', { likely_cause: 'unknown', code: 137 }),
evt('worker_exited', { code: 1 }), // legacy (no likely_cause)
evt('worker_exited', { likely_cause: 'clean_exit', code: 0 }),
evt('worker_exited', { likely_cause: 'clean_exit', code: 0 }),
evt('worker_exited', { likely_cause: 'clean_exit', code: 0 }),
evt('worker_exited', { likely_cause: 'graceful_shutdown', signal: 'SIGTERM' }),
// Non-exit events MUST be ignored (no double-counting against either bucket).
evt('started'),
evt('worker_spawned'),
evt('health_warn'),
];
const summary: CrashSummary = summarizeCrashes(events);
expect(summary.total).toBe(5);
expect(summary.by_cause.runtime_error).toBe(2);
expect(summary.by_cause.oom_or_external_kill).toBe(1);
expect(summary.by_cause.unknown).toBe(1);
expect(summary.by_cause.legacy).toBe(1);
expect(summary.clean_exits).toBe(4);
// total + clean_exits should equal the count of worker_exited events,
// proving non-exit lifecycle events were excluded from both buckets.
const exitCount = events.filter((e) => e.event === 'worker_exited').length;
expect(summary.total + summary.clean_exits).toBe(exitCount);
});
test('empty input returns zero summary', () => {
const summary = summarizeCrashes([]);
expect(summary).toEqual({
total: 0,
by_cause: { runtime_error: 0, oom_or_external_kill: 0, unknown: 0, legacy: 0 },
clean_exits: 0,
});
});
test('only non-exit events returns zero summary', () => {
const summary = summarizeCrashes([evt('started'), evt('worker_spawned'), evt('stopped')]);
expect(summary.total).toBe(0);
expect(summary.clean_exits).toBe(0);
});
// Denylist regression guard at the AGGREGATOR level. isCrashExit Case 6
// proves an unrecognized future `likely_cause` is counted as a crash; this
// pins which BUCKET it lands in. The `else` branch in summarizeCrashes
// routes any crash-classified event whose cause doesn't match the three
// explicit buckets into `legacy`. Operators watching `legacy=N` rise know
// the upstream classifier added a value the doctor doesn't yet name —
// that's the intended signal.
test('unrecognized future likely_cause routes to legacy bucket', () => {
const summary = summarizeCrashes([
evt('worker_exited', { likely_cause: 'lock_lost' }),
evt('worker_exited', { likely_cause: 'panic' }),
]);
expect(summary.total).toBe(2);
expect(summary.by_cause.legacy).toBe(2);
expect(summary.by_cause.runtime_error).toBe(0);
expect(summary.by_cause.oom_or_external_kill).toBe(0);
expect(summary.by_cause.unknown).toBe(0);
});
// Truly malformed legacy line — `likely_cause` missing AND `code` null
// (or undefined). The classifier comment explicitly says "fail-loud, the
// user can investigate the audit file directly", which means count it.
// null !== 0 is true so isCrashExit returns true; summarizeCrashes then
// lands it in legacy. Pinning this prevents a regression where a future
// refactor adds `code != null` and silently drops malformed entries.
test('legacy entry with null code counts as crash in legacy bucket', () => {
const summary = summarizeCrashes([
evt('worker_exited', { code: null }),
]);
expect(summary.total).toBe(1);
expect(summary.by_cause.legacy).toBe(1);
expect(summary.clean_exits).toBe(0);
});
});