diff --git a/src/servers/api/bots/brief.ts b/src/servers/api/bots/brief.ts index 8df0fa29..a11b2873 100644 --- a/src/servers/api/bots/brief.ts +++ b/src/servers/api/bots/brief.ts @@ -95,8 +95,31 @@ For a "has it changed" watch, the same shape with the comparison against \`$STAT fetch, extract the smallest stable piece, compare, and \`exit 0\` printing the new value when it differs. Write the new value to \`$STATE\` **before** exiting, or the next arming fires on the same change again. -4. **When you are handed back**, do what the owner asked, then **re-arm the same script the same way, - before you end the turn**. Act, then re-arm, in the one turn. Never build a second loop on top of this. +4. **Record what you armed, in \`.bot.json\` in this directory.** This is not optional and + it is not bookkeeping — it is the only thing that can stop your watch. Nothing else knows your pid, and + a bot that is deleted while its record is missing leaves a process polling a website for ever. + +\`\`\`json +{ + "sessionId": "6305e570-f26a-4989-a0ae-2135c1076505", + "watches": "newweb.fofosfluffybastards.com, every 60s, notify the owner when down", + "script": "/abs/path/to/newweb-uptime.sh", + "state": "/abs/path/to/newweb-uptime.state", + "pids": [1316235], + "armedAt": "2026-08-26T07:33:00Z" +} +\`\`\` + + **Rewrite it every time you arm**, with the new pid. \`pids\` is an array because a re-arm can briefly + overlap; list every process you started that is still meant to be running. Your session id is the file + name you were told to use — if you do not know it, look at which transcript you are in. + +5. **When you are handed back**, do what the owner asked, then **re-arm the same script the same way, + before you end the turn**, and rewrite the record. Act, re-arm, record — in the one turn. Never build a + second loop on top of this. + +6. **If you are woken and told your watch is gone**, read your own \`.bot.json\`, arm the script it names + again, and rewrite it. That file is what survives a reboot; the conversation may be days behind. Do not go hunting for how any of this works — it is above, and looking costs the owner a minute of tool calls before anything is armed. diff --git a/src/servers/api/bots/teardown.ts b/src/servers/api/bots/teardown.ts new file mode 100644 index 00000000..99b51329 --- /dev/null +++ b/src/servers/api/bots/teardown.ts @@ -0,0 +1,133 @@ +import { existsSync, readFileSync, readdirSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { getBotsDir } from '@@/data-path'; +import { descendantPids } from '../tasks/process-tree'; + +/** + * Stopping a bot, and knowing which ones have stopped themselves. + * + * ── Why a record exists at all ── + * + * A bot's watch is a detached shell process. Nothing else on the machine knows it belongs to that bot: not + * the transcript, not the session list, not the process table, which sees an anonymous `bash` in a + * temporary directory. So the bot writes `.bot.json` beside its script, and that file is the + * only thing that can connect the two. + * + * The cost of not having it was paid before it existed: a bot was deleted on 2026-08-26 and its watcher + * was still polling a website eleven minutes later, with nothing in the product able to find or stop it. + * + * ── Why the id is in the FILENAME ── + * + * Every bot shares one directory, so the record has to be addressable without reading all of them. Deleting + * a session knows only its id; `join(dir, id + '.bot.json')` is then a lookup rather than a scan, and a + * record whose contents are damaged still gets found and cleaned up. + */ + +export type BotRecord = { + sessionId?: string; + watches?: string; + script?: string; + state?: string; + pids?: number[]; + armedAt?: string; +}; + +const recordPath = (email: string, sessionId: string) => join(getBotsDir(email), `${sessionId}.bot.json`); + +/** Refuse anything that is not a bare id — this reaches here from a URL. */ +const safeId = (sessionId: string) => /^[A-Za-z0-9._-]+$/.test(sessionId) && !sessionId.includes('..'); + +export function readBotRecord(email: string, sessionId: string): BotRecord | null { + if (!safeId(sessionId)) return null; + const path = recordPath(email, sessionId); + if (!existsSync(path)) return null; + try { + return JSON.parse(readFileSync(path, 'utf-8')) as BotRecord; + } catch { + // Damaged rather than absent. The caller still deletes the file — a record it cannot read is a record + // that can never stop anything, and leaving it would only make the next reader believe there is one. + return {}; + } +} + +const alive = (pid: number): boolean => { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +}; + +/** Every recorded pid still running, which is how "is this bot actually watching" is answered. */ +export function liveBotPids(email: string, sessionId: string): number[] { + const record = readBotRecord(email, sessionId); + return (record?.pids ?? []).filter((pid) => Number.isInteger(pid) && pid > 1 && alive(pid)); +} + +/** + * Stop a bot's watch and forget it. + * + * Children first, then the recorded process, so the loop cannot spawn a replacement between the two — + * the same ordering `stopCommanderProcesses` uses and for the same reason. + * + * Best effort throughout: a bot whose record is missing, damaged, or names processes that are already + * gone still has its files removed. Returns how many processes were signalled, which is worth logging — + * "deleted a bot and stopped nothing" is a different event from "deleted a bot that was not armed". + */ +export function tearDownBot(email: string, sessionId: string): number { + const record = readBotRecord(email, sessionId); + if (!record) return 0; + + const pids = (record.pids ?? []).filter((pid) => Number.isInteger(pid) && pid > 1); + const tree = [...new Set(pids.flatMap((pid) => [...descendantPids(pid), pid]))]; + + let signalled = 0; + for (const pid of tree) { + try { + process.kill(pid, 'SIGTERM'); + signalled += 1; + } catch { + // Gone between the walk and the signal. That is the outcome we wanted. + } + } + + // The script and its state go with it. They are the bot's, they are named in its own record, and leaving + // them means the next bot with the same idea finds a stale baseline and does not fire. + for (const path of [record.script, record.state]) { + if (!path) continue; + // Only inside the bots directory. A record is written by an agent, and a path out of that tree is + // either a mistake or something worse — either way it is not this function's business to delete. + if (!path.startsWith(getBotsDir(email))) continue; + try { + rmSync(path, { force: true }); + } catch { + // Not worth failing a delete over. + } + } + + try { + rmSync(recordPath(email, sessionId), { force: true }); + } catch { + // Same. + } + + return signalled; +} + +/** Every bot that has a record, for the sweep that revives them. */ +export function listBotRecords(email: string): { sessionId: string; record: BotRecord }[] { + let names: string[]; + try { + names = readdirSync(getBotsDir(email)); + } catch { + return []; // no directory yet is the ordinary state of an account with no bots + } + + return names + .filter((name) => name.endsWith('.bot.json')) + .map((name) => { + const sessionId = name.slice(0, -'.bot.json'.length); + return { sessionId, record: readBotRecord(email, sessionId) ?? {} }; + }); +} diff --git a/src/servers/api/chat/chat.ts b/src/servers/api/chat/chat.ts index b5559053..f24c7154 100644 --- a/src/servers/api/chat/chat.ts +++ b/src/servers/api/chat/chat.ts @@ -1,4 +1,5 @@ import { getPluginChatsDir, getBotsDir } from '@@/data-path'; +import { tearDownBot } from '../bots/teardown'; import type { Context } from 'hono'; import { emitToSession, sessionKeyForTranscript } from './websocket'; import { pauseBackgroundTask, restartBackgroundTask } from './task-restart'; @@ -470,6 +471,26 @@ chatRouter.delete('/sessions/:id', async (ctx) => { conversation — no live session holds it, so nothing is keyed on a key to remove. */ await forgetChatSessions({ transcriptIds: removed, sessionKeys: liveKey ? [liveKey] : [] }); + + /* + And stop anything a BOT armed, which nothing above would have touched. + + A bot's watch is a detached shell process. Killing the harness ends the agent; the loop it started is + not its child and carries on. That gap was paid for on 2026-08-26: a bot was deleted and its watcher + was still polling a website eleven minutes later, with nothing in the product able to find it. + + Every transcript removed, not just the id asked for — `deleteClaudeSession` returns a chain, and a + resumed conversation's earlier transcripts can each carry their own record. + + Unconditional rather than gated on "was this a bot": the record only exists in the bots directory and + is keyed by session id, so for anything else this reads a path that is not there and returns 0. A + branch on the cwd would be a second place to get the question wrong. + */ + for (const transcriptId of removed) { + const stopped = tearDownBot(who.email, transcriptId); + if (stopped > 0) console.log(`[bots] ${transcriptId} deleted — stopped ${stopped} process(es)`); + } + return ctx.json({ ok: true }); }); diff --git a/src/servers/api/command-hq/watcher-revive.ts b/src/servers/api/command-hq/watcher-revive.ts index acca3da0..691bd8c1 100644 --- a/src/servers/api/command-hq/watcher-revive.ts +++ b/src/servers/api/command-hq/watcher-revive.ts @@ -1,10 +1,11 @@ import { listAllCommanders, getUserById } from 'officerdb'; -import { getCommanderStateDir } from '@@/data-path'; +import { getCommanderStateDir, getBotsDir } from '@@/data-path'; import { existsSync, readFileSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { startClaudeTurn, COMMANDER_DEFAULT_MODEL } from '../chat/websocket'; import * as sidecar from '@@/sidecar-registry'; import { clearLoopPid, liveLoopPids } from './spawned'; +import { listBotRecords, liveBotPids } from '../bots/teardown'; import { getNotifyServerUrl, notifyProducerHeaders } from '../notify/router'; // Waking a commander whose watch has died. @@ -60,6 +61,18 @@ const FIRST_TICK_MS = 120_000; const PID_FILE = 'loop-pid'; const ATTEMPTS_FILE = 'revive-attempts'; +/** + * Where a subject's wake attempts are counted. + * + * A commander keeps them in its own `state/`. A bot has no directory of its own, so its key is + * `bot-` and the file goes in the bots directory under that name — same bound, same window, + * one implementation. + */ +const attemptsPath = (email: string, key: string) => + key.startsWith('bot-') + ? join(getBotsDir(email), `${key}.${ATTEMPTS_FILE}`) + : join(getCommanderStateDir(email, key), ATTEMPTS_FILE); + /** * How many times this will wake one commander, and over what window. * @@ -76,7 +89,7 @@ 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); + const path = attemptsPath(email, dirName); if (!existsSync(path)) return []; const cutoff = Date.now() - ATTEMPT_WINDOW_MS; try { @@ -93,7 +106,7 @@ function recentAttempts(email: string, dirName: string): number[] { } function recordAttempt(email: string, dirName: string): void { - const path = join(getCommanderStateDir(email, dirName), ATTEMPTS_FILE); + const path = attemptsPath(email, dirName); try { writeFileSync(path, [...recentAttempts(email, dirName), Date.now()].join('\n')); } catch (err) { @@ -267,6 +280,82 @@ async function tellTheOwner(userId: number, dirName: string): Promise { } } +/** + * The same question, asked of bots. + * + * A bot's watch dies exactly as a commander's does — a reboot, a killed process tree — and with the same + * silence: the card still lists it and nothing is polling anything. The difference is only where the + * record lives. A commander writes a pid file into its own `state/`; a bot writes `.bot.json` + * beside its script, because bots share one directory and the id has to be in the filename. + * + * A record with no live pid is a watch that has stopped. There is no `armed` flag to consult and there + * should not be: the record is written when it arms, so its existence IS the intent, and the pids in it + * are the fact. + */ +async function reviveBots(userId: number, email: string): Promise { + for (const { sessionId, record } of listBotRecords(email)) { + if (!record.pids?.length) continue; // never armed, or already torn down + if (liveBotPids(email, sessionId).length > 0) continue; + + const attempts = recentAttempts(email, `bot-${sessionId}`); + if (attempts.length >= MAX_ATTEMPTS) continue; + + console.warn(`[watcher-revive] bot ${sessionId}: watch is dead — waking it`); + recordAttempt(email, `bot-${sessionId}`); + await tellTheOwner(userId, `bot ${sessionId.slice(0, 8)}`); + await wakeBot(userId, sessionId, email); + } +} + +/** + * What a bot is told. Shorter than a commander's, because a bot has one job and its record names it. + */ +const WAKE_BOT_PROMPT = [ + 'Your watch is not running — the recorded process is gone, usually a reboot or something that killed', + 'the tree it was started under.', + '', + 'Read your own `.bot.json` in this directory, arm the script it names again exactly as', + 'before, and rewrite the record with the new pid. Do not start a second watch beside it, and do not', + 'change what it watches.', + '', + 'If the record or the script is missing, say what is gone and stop. Do not reconstruct a watch from', + 'memory of the conversation — a watch that is subtly different from the one the owner agreed to is', + 'worse than none, because it looks like it is working.', +].join('\n'); + +async function wakeBot(userId: number, sessionId: string, email: string): Promise { + await new Promise((resolve) => { + let settled = false; + const done = () => { + if (!settled) { + settled = true; + resolve(); + } + }; + const timer = setTimeout(done, 60_000); + + void startClaudeTurn({ + email, + username: email, + userId, + ws: null, + sessionId, + model: COMMANDER_DEFAULT_MODEL, + msg: { prompt: WAKE_BOT_PROMPT, resumeSessionId: sessionId }, + effectivePrompt: WAKE_BOT_PROMPT, + onOutcome: (outcome) => { + clearTimeout(timer); + if (!outcome.ok) console.error(`[watcher-revive] bot ${sessionId}: ${outcome.error}`); + done(); + }, + }).catch((err) => { + clearTimeout(timer); + console.error(`[watcher-revive] bot ${sessionId} threw:`, err instanceof Error ? err.message : err); + done(); + }); + }); +} + let timer: ReturnType | null = null; /** @@ -305,6 +394,17 @@ export function startWatcherRevive(): void { await tellTheOwner(commander.userId, commander.dirName); await wake(commander.userId, commander.sessionId, commander.dirName, user.email); } + + // Bots, same tick. Their records live per account, so this needs the set of owners rather than the + // commander list — an account with no commanders can still have bots. + const owners = new Map(); + for (const c of await listAllCommanders()) { + const u = await getUserById(c.userId); + if (u) owners.set(c.userId, u.email); + } + const me = await getUserById(1); + if (me) owners.set(1, me.email); + for (const [userId, email] of owners) await reviveBots(userId, email); } catch (err) { console.error('[watcher-revive] tick failed:', err instanceof Error ? err.message : err); } finally {