From 80a070313a9ddc0e8437c860fd81a998ed5dae75 Mon Sep 17 00:00:00 2001 From: Felipe Date: Mon, 4 May 2026 13:56:00 -0300 Subject: [PATCH] fix(prune): add --errored mode for blanket error-state cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `genie prune --zombies` only archives reconciler-tagged dead-pane rows (audit reason 'dead_pane_zombie' or 'stale_spawn_dead_pane'). That's the safe, conservative sweep — but operators occasionally want a blanket cleanup of any inert error-state row regardless of reason. Add `genie prune --errored`: - Archives any state=error + auto_resume=false agent older than the TTL. - 1h default TTL (vs 24h for --zombies) — opt-in flag means the operator has already decided the rows are noise. - Audit reason: 'errored_ttl_exhausted' (distinct from 'dead_pane_zombie_ttl_exhausted' so forensics can tell which sweep archived a row). - Mutually exclusive with --zombies; pick one mode per invocation. - Set auto_resume=true on a row to keep it visible past the TTL. New helpers: - archiveAllExhaustedErrored(ttlHours) — same as archiveExhaustedZombies but without the audit-reason EXISTS filter. - listAllExhaustedErrored(ttlHours) — dry-run companion. `prune.ts` reorganized around a `PruneMode` discriminator with mode-specific default TTL + label tables. --zombies semantics unchanged. Wish: cli-noise-and-hygiene-cleanup G3 (deliverables #2-5). Pairs with the prior G3#1 fix (broaden zombie reason filter). Smoke (live DB): - Seeded a 2h-old error+exhausted row with NO audit reason. - listExhaustedZombies(1) skipped it (correct: no audit tag). - listAllExhaustedErrored(1) matched it (correct: blanket). - archiveAllExhaustedErrored(1) flipped state→'archived' and emitted audit reason='errored_ttl_exhausted'. - genie ls errored count dropped 6→3 (3 newer-than-1h survived TTL). --- src/lib/__tests__/zombie-spawns.test.ts | 16 ++++ src/lib/agent-registry.ts | 76 ++++++++++++++++++ src/term-commands/prune.ts | 100 +++++++++++++++++------- 3 files changed, 162 insertions(+), 30 deletions(-) diff --git a/src/lib/__tests__/zombie-spawns.test.ts b/src/lib/__tests__/zombie-spawns.test.ts index 9f25ab496..10d69e38b 100644 --- a/src/lib/__tests__/zombie-spawns.test.ts +++ b/src/lib/__tests__/zombie-spawns.test.ts @@ -121,6 +121,22 @@ describe('reconcileStaleSpawns dead-pane pass', () => { const matches = source.split(archiveFilter).length - 1; expect(matches).toBeGreaterThanOrEqual(2); }); + + test('archiveAllExhaustedErrored: blanket sweep with distinct audit reason', () => { + // `genie prune --errored` must NOT carry the audit-reason filter that + // --zombies uses, otherwise it would behave identically to --zombies. + // It must also tag a distinct audit reason so forensics can tell which + // sweep archived a row. + const source = readFileSync(join(__dirname, '..', 'agent-registry.ts'), 'utf-8'); + + // Function exists with the documented 1h default. + expect(source).toContain('export async function archiveAllExhaustedErrored'); + expect(source).toContain('EXHAUSTED_ERRORED_TTL_HOURS = 1'); + // Distinct audit reason from the zombie sweep. + expect(source).toContain("reason: 'errored_ttl_exhausted'"); + // List companion exists for --dry-run. + expect(source).toContain('export async function listAllExhaustedErrored'); + }); }); // --------------------------------------------------------------------------- diff --git a/src/lib/agent-registry.ts b/src/lib/agent-registry.ts index 35842cc30..d215c9cb2 100644 --- a/src/lib/agent-registry.ts +++ b/src/lib/agent-registry.ts @@ -668,6 +668,82 @@ export async function archiveExhaustedZombies(ttlHours = DEAD_PANE_ZOMBIE_TTL_HO } } +/** + * Default TTL (hours) for `genie prune --errored` mode. Shorter than + * `DEAD_PANE_ZOMBIE_TTL_HOURS` because `--errored` is opt-in: an operator + * who runs it has already decided the rows are noise. The 1h floor still + * gives a brief grace window to inspect freshly-failed agents. + */ +const EXHAUSTED_ERRORED_TTL_HOURS = 1; + +/** + * Archive any exhausted error-state agent older than the TTL, regardless + * of audit reason. Distinct from `archiveExhaustedZombies` which only + * touches reconciler-tagged dead-pane rows. Use this when you want a + * blanket sweep of inert error rows; use `auto_resume=true` to keep a + * row visible past the TTL. + * + * Audit reason: `errored_ttl_exhausted` (distinct from + * `dead_pane_zombie_ttl_exhausted` so forensics can tell which sweep + * archived the row). + * + * @param ttlHours - Minimum age in hours before archival (default: 1) + * @returns IDs of agents that were archived + */ +export async function archiveAllExhaustedErrored(ttlHours = EXHAUSTED_ERRORED_TTL_HOURS): Promise { + try { + const sql = await getConnection(); + const rows = await sql<{ id: string }[]>` + UPDATE agents a + SET state = 'archived', last_state_change = now() + WHERE a.state = 'error' + AND a.auto_resume = false + AND a.last_state_change < now() - make_interval(hours => ${ttlHours}) + RETURNING id + `; + const auditPromises: Promise[] = []; + for (const row of rows) { + console.error(`[prune] Archived exhausted errored ${row.id} (TTL ${ttlHours}h)`); + auditPromises.push( + recordAuditEvent('worker', row.id, 'state_changed', 'cli', { + state: 'archived', + reason: 'errored_ttl_exhausted', + ttl_hours: ttlHours, + }).catch(() => {}), + ); + } + await Promise.allSettled(auditPromises); + return rows.map((r: { id: string }) => r.id); + } catch { + return []; + } +} + +/** + * Dry-run companion to `archiveAllExhaustedErrored` — lists rows that + * would be archived without mutating state. + */ +export async function listAllExhaustedErrored( + ttlHours = EXHAUSTED_ERRORED_TTL_HOURS, +): Promise> { + try { + const sql = await getConnection(); + const rows = await sql<{ id: string; last_state_change: string }[]>` + SELECT a.id, a.last_state_change FROM agents a + WHERE a.state = 'error' + AND a.auto_resume = false + AND a.last_state_change < now() - make_interval(hours => ${ttlHours}) + ORDER BY a.last_state_change ASC + `; + return rows.map((r: { id: string; last_state_change: string }) => ({ + id: r.id, + lastStateChange: r.last_state_change, + })); + } catch { + return []; + } +} + /** * Count dead-pane zombies eligible for TTL archive without mutating them. * Used by `genie prune --zombies --dry-run`. diff --git a/src/term-commands/prune.ts b/src/term-commands/prune.ts index 3a420441f..8aaacbe71 100644 --- a/src/term-commands/prune.ts +++ b/src/term-commands/prune.ts @@ -1,28 +1,49 @@ /** * genie prune — bulk cleanup of stale or exhausted registry entries. * - * Subcommands/flags: - * genie prune --zombies Archive exhausted dead-pane zombies - * genie prune --zombies --dry-run List zombies that would be archived - * genie prune --ttl-hours Override the default 24h TTL + * Targets (mutually exclusive — pick one per invocation): + * --zombies Reconciler-tagged dead-pane zombies (24h default TTL). + * Conservative: only matches rows with audit reason + * 'dead_pane_zombie' or 'stale_spawn_dead_pane'. + * --errored Any exhausted error-state row regardless of reason + * (1h default TTL). Opt-in sweep — set auto_resume=true + * on rows you want to keep visible past the TTL. * - * A "dead-pane zombie" is an agent whose reconciler audit trail tagged - * it `reason=dead_pane_zombie` (tmux pane vanished mid-run) AND whose - * auto-resume budget was subsequently exhausted (auto_resume=false). - * Once that happens the row is inert: it never resumes and clutters - * `genie ls`. See issue #1293. + * Flags: + * --dry-run List targets without mutating + * --ttl-hours Override the mode default + * + * See issue #1293 for the original zombie cleanup story. */ import type { Command } from 'commander'; -import { archiveExhaustedZombies, listExhaustedZombies } from '../lib/agent-registry.js'; +import { + archiveAllExhaustedErrored, + archiveExhaustedZombies, + listAllExhaustedErrored, + listExhaustedZombies, +} from '../lib/agent-registry.js'; import { isAvailable, shutdown } from '../lib/db.js'; interface PruneOptions { zombies?: boolean; + errored?: boolean; dryRun?: boolean; ttlHours?: number; } +type PruneMode = 'zombies' | 'errored'; + +const DEFAULT_TTL_HOURS: Record = { + zombies: 24, + errored: 1, +}; + +const TARGET_LABEL: Record = { + zombies: 'zombie agent', + errored: 'errored agent', +}; + function parsePositiveInt(value: string, name: string): number { const parsed = Number.parseInt(value, 10); if (!Number.isFinite(parsed) || parsed <= 0) { @@ -31,40 +52,55 @@ function parsePositiveInt(value: string, name: string): number { return parsed; } -async function runDryRun(ttlHours: number): Promise { - const zombies = await listExhaustedZombies(ttlHours); - if (zombies.length === 0) { - console.log(`No exhausted zombies older than ${ttlHours}h.`); +async function listTargets(mode: PruneMode, ttlHours: number): Promise> { + return mode === 'zombies' ? listExhaustedZombies(ttlHours) : listAllExhaustedErrored(ttlHours); +} + +async function archiveTargets(mode: PruneMode, ttlHours: number): Promise { + return mode === 'zombies' ? archiveExhaustedZombies(ttlHours) : archiveAllExhaustedErrored(ttlHours); +} + +async function runDryRun(mode: PruneMode, ttlHours: number): Promise { + const rows = await listTargets(mode, ttlHours); + if (rows.length === 0) { + console.log(`No exhausted ${TARGET_LABEL[mode]}s older than ${ttlHours}h.`); return; } - const plural = zombies.length === 1 ? '' : 's'; - console.log(`Would archive ${zombies.length} zombie agent${plural} older than ${ttlHours}h:`); - for (const z of zombies) { - console.log(` ${z.id} (last state change: ${z.lastStateChange})`); + const plural = rows.length === 1 ? '' : 's'; + console.log(`Would archive ${rows.length} ${TARGET_LABEL[mode]}${plural} older than ${ttlHours}h:`); + for (const r of rows) { + console.log(` ${r.id} (last state change: ${r.lastStateChange})`); } } -async function runArchive(ttlHours: number): Promise { - const ids = await archiveExhaustedZombies(ttlHours); +async function runArchive(mode: PruneMode, ttlHours: number): Promise { + const ids = await archiveTargets(mode, ttlHours); if (ids.length === 0) { - console.log(`No exhausted zombies older than ${ttlHours}h. Nothing to archive.`); + console.log(`No exhausted ${TARGET_LABEL[mode]}s older than ${ttlHours}h. Nothing to archive.`); return; } const plural = ids.length === 1 ? '' : 's'; - console.log(`Archived ${ids.length} zombie agent${plural} older than ${ttlHours}h:`); + console.log(`Archived ${ids.length} ${TARGET_LABEL[mode]}${plural} older than ${ttlHours}h:`); for (const id of ids) { console.log(` ${id}`); } } -async function pruneCommand(options: PruneOptions): Promise { - if (!options.zombies) { - console.error('Error: no prune target specified. Use `--zombies`.'); - console.error('See `genie prune --help` for available targets.'); +function resolveMode(options: PruneOptions): PruneMode { + if (options.zombies && options.errored) { + console.error('Error: --zombies and --errored are mutually exclusive.'); process.exit(2); } + if (options.zombies) return 'zombies'; + if (options.errored) return 'errored'; + console.error('Error: no prune target specified. Use `--zombies` or `--errored`.'); + console.error('See `genie prune --help` for available targets.'); + process.exit(2); +} - const ttlHours = options.ttlHours ?? 24; +async function pruneCommand(options: PruneOptions): Promise { + const mode = resolveMode(options); + const ttlHours = options.ttlHours ?? DEFAULT_TTL_HOURS[mode]; if (!(await isAvailable())) { console.error('Database is not running. Start it with: genie db status'); @@ -72,7 +108,7 @@ async function pruneCommand(options: PruneOptions): Promise { } try { - await (options.dryRun ? runDryRun(ttlHours) : runArchive(ttlHours)); + await (options.dryRun ? runDryRun(mode, ttlHours) : runArchive(mode, ttlHours)); } catch (err) { const message = err instanceof Error ? err.message : String(err); console.error(`Prune failed: ${message}`); @@ -86,9 +122,13 @@ export function registerPruneCommands(program: Command): void { program .command('prune') .description('Bulk cleanup of stale or exhausted registry entries') - .option('--zombies', 'Archive dead-pane zombies whose auto-resume retries are exhausted') + .option('--zombies', 'Archive reconciler-tagged dead-pane zombies (24h default TTL)') + .option( + '--errored', + 'Archive any exhausted error-state agent regardless of reason (1h default TTL; set auto_resume=true to keep a row visible)', + ) .option('--dry-run', 'List targets that would be affected without mutating') - .option('--ttl-hours ', 'Minimum age in hours before a zombie is eligible for archive (default: 24)', (v) => + .option('--ttl-hours ', 'Override the mode default TTL in hours (24 for --zombies, 1 for --errored)', (v) => parsePositiveInt(v, '--ttl-hours'), ) .action(pruneCommand);