From ac25cd48603ddbcee8b339d6fb646a253319fd58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 25 Aug 2026 16:36:05 +0000 Subject: [PATCH] do not wake a commander that is working, and never more than three times MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit two guards the first cut was missing, both about the same thing: a dead pid is not a dead watch. a watcher EXITS when it fires — that is the mechanism, since a background process reaches the model only by ending. so the healthy path is poll, find, exit, hand back, act, re-arm, and for the whole middle of that the pid file names a process that is gone. ticking into that window would have interrupted a commander mid-work to tell it its watch had died. so: if a turn is running, the exit is already being handled and the re-arm is the last thing that turn will do. and a bound, for the case the first guard cannot see. a commander that genuinely cannot re-arm would otherwise be woken every two minutes forever, each wake a real agent turn — the same unbounded retry prompts/deploy-commander.md records the first deploy watcher dying of. three wakes in six hours, then leave it alone; a live loop resets the count. an unreadable attempts file counts as tried, because failing towards fewer wakes is the cheap direction. the near miss that prompted this is recorded honestly in the file: a pid read as dead two seconds after being written and the process was in fact alive and mid-claim. the lesson is that a pid file is a poor instant answer to "is this watching", which is enough to justify both guards without claiming more than was seen. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- src/servers/api/command-hq/watcher-revive.ts | 109 ++++++++++++++++++- 1 file changed, 103 insertions(+), 6 deletions(-) diff --git a/src/servers/api/command-hq/watcher-revive.ts b/src/servers/api/command-hq/watcher-revive.ts index dcde04b0..39c1220e 100644 --- a/src/servers/api/command-hq/watcher-revive.ts +++ b/src/servers/api/command-hq/watcher-revive.ts @@ -1,8 +1,10 @@ import { listAllCommanders, getUserById } from 'officerdb'; import { getCommanderStateDir } from '@@/data-path'; -import { existsSync } from 'node:fs'; +import { existsSync, readFileSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; -import { startClaudeTurn, COMMANDER_DEFAULT_MODEL } from '../chat/websocket'; +import { startClaudeTurn, COMMANDER_DEFAULT_MODEL, collectRunningTasks } from '../chat/websocket'; +import { getChatEventsSince } from 'officerdb'; +import type { ServerMessage } from '../chat/types'; import { clearLoopPid, liveLoopPids } from './spawned'; import { getNotifyServerUrl, notifyProducerHeaders } from '../notify/router'; @@ -57,11 +59,93 @@ const TICK_MS = 120_000; const FIRST_TICK_MS = 120_000; const PID_FILE = 'loop-pid'; +const ATTEMPTS_FILE = 'revive-attempts'; -/** A watch that was running and is not. See the note above for why this is the pid file's question. */ -function watchDied(email: string, dirName: string): boolean { +/** + * How many times this will wake one commander, and over what window. + * + * **This bound is the backstop, not the correctness.** The guard that matters is `isGenerating` below; + * this one exists for the case that guard cannot see — a commander that genuinely cannot re-arm, waking, + * failing, and being woken again every two minutes, each wake costing a real agent turn, all night. + * + * That is the exact failure `prompts/deploy-commander.md` records for the first deploy watcher: a careful + * loop with an unbounded retry inside it. A commander that cannot re-arm itself is not a thing to keep + * asking; it is a thing to tell a person about once. + */ +const MAX_ATTEMPTS = 3; +const ATTEMPT_WINDOW_MS = 6 * 60 * 60 * 1000; + +/** Wake timestamps inside the window. Anything older is dropped rather than kept — this is a rate, not a history. */ +function recentAttempts(email: string, dirName: string): number[] { + const path = join(getCommanderStateDir(email, dirName), ATTEMPTS_FILE); + if (!existsSync(path)) return []; + const cutoff = Date.now() - ATTEMPT_WINDOW_MS; + try { + return readFileSync(path, 'utf-8') + .split('\n') + .map((line) => Number(line.trim())) + .filter((n) => Number.isFinite(n) && n > cutoff); + } catch { + // Unreadable counts as "we have tried" rather than "we have not". Failing towards fewer wakes is the + // safe direction: the cost of one missed revival is a watch that stays off and is reported; the cost + // of miscounting the other way is a turn every two minutes. + return new Array(MAX_ATTEMPTS).fill(Date.now()); + } +} + +function recordAttempt(email: string, dirName: string): void { + const path = join(getCommanderStateDir(email, dirName), ATTEMPTS_FILE); + try { + writeFileSync(path, [...recentAttempts(email, dirName), Date.now()].join('\n')); + } catch (err) { + console.error( + `[watcher-revive] could not record an attempt for ${dirName}:`, + err instanceof Error ? err.message : err, + ); + } +} + +/** A commander that re-armed successfully starts from zero, so a crash months later is not refused. */ +function clearAttempts(email: string, dirName: string): void { + try { + writeFileSync(join(getCommanderStateDir(email, dirName), ATTEMPTS_FILE), ''); + } catch { + // Not worth failing over — the window expires on its own. + } +} + +/** + * A watch that was running and is not. + * + * **A dead pid is NOT the same as a dead watch, and this is the trap.** A watcher is built to EXIT when it + * fires — that is the whole mechanism, because a background process reaches the model only by ending. So + * the normal, healthy path goes: poll, find something, exit, hand back to the agent, agent acts, agent + * re-arms. For the whole of that middle stretch the pid file names a process that is gone. + * + * The exit-on-fire model is `docs/command-hq.md` → "The watcher loop", settled 2026-08-18, and it is the + * reason this guard exists rather than any incident. There WAS a near miss on 2026-08-25 — commander 40 + * was woken, rebuilt its watch, and two seconds after it recorded a pid that pid read as dead — but the + * process turned out to be alive and mid-claim, so the honest account is that a pid file is a poor instant + * answer to "is this watching", not that a fire was observed being misread. Either way the window is real: + * between a watcher exiting and its agent re-arming, the file names a process that is gone. + * + * Hence `isGenerating`: if a turn is running, the exit is being handled and the re-arm is the last thing + * that turn will do. Waking then is not just wasteful, it is wrong — it would arrive mid-work carrying an + * instruction the agent has no reason to obey. + */ +async function watchDied(email: string, dirName: string, sessionId: string): Promise { if (!existsSync(join(getCommanderStateDir(email, dirName), PID_FILE))) return false; - return liveLoopPids(email, dirName).length === 0; + if (liveLoopPids(email, dirName).length > 0) return false; + + try { + const events = await getChatEventsSince(sessionId, 0); + if (collectRunningTasks(events.map((e) => e.event as ServerMessage)).length > 0) return false; + } catch { + // Cannot tell whether it is working, so assume it is. A missed revival is reported next tick; a wake + // delivered into a live turn is not recoverable by waiting. + return false; + } + return true; } /** @@ -187,9 +271,22 @@ export function startWatcherRevive(): void { for (const commander of await listAllCommanders()) { const user = await getUserById(commander.userId); if (!user) continue; - if (!watchDied(user.email, commander.dirName)) continue; + if (!(await watchDied(user.email, commander.dirName, commander.sessionId))) { + // Healthy, stopped, or mid-turn. A live loop means the last wake worked, so the count resets. + if (liveLoopPids(user.email, commander.dirName).length > 0) clearAttempts(user.email, commander.dirName); + continue; + } + + const attempts = recentAttempts(user.email, commander.dirName); + if (attempts.length >= MAX_ATTEMPTS) { + console.warn( + `[watcher-revive] commander ${commander.dirName}: ${attempts.length} failed wakes — leaving it alone`, + ); + continue; + } console.warn(`[watcher-revive] commander ${commander.dirName}: watch is dead — waking it`); + recordAttempt(user.email, commander.dirName); await tellTheOwner(commander.userId, commander.dirName); await wake(commander.userId, commander.sessionId, commander.dirName, user.email); }