diff --git a/src/servers/api/command-hq/router.ts b/src/servers/api/command-hq/router.ts index 513ab6bd..88de742b 100644 --- a/src/servers/api/command-hq/router.ts +++ b/src/servers/api/command-hq/router.ts @@ -6,6 +6,13 @@ import * as errors from '@@/custom-errors'; import { resolveHomeDir } from '@@/user-home'; import { getCommandersDir, toShellUsername } from '@@/data-path'; import { getChatEventsSince } from 'officerdb'; +import { + clearSpawnedRecord, + listSpawnedSessions, + loopPidPath, + spawnedRecordPath, + stopCommanderProcesses, +} from './spawned'; import type { ServerMessage } from '../chat/types'; import { listClaudeSessions, @@ -469,11 +476,25 @@ commandHqRouter.patch('/commanders/:sessionId', async (ctx) => { * disk, so hide it" pattern `purgeCommandHqSessions` exists for — it is the one place that makes the two * agree by deleting the thing itself, on purpose, because the owner asked to. * - * A watcher still armed at the time of deletion is left as-is rather than disarmed first: the transcript - * is gone either way, `armCommandHqWatcher`/`recordCommandHqWatcherFired` can no longer find a row to - * update once this returns, and the running background process (if any) dies on its own the same way it - * would if the harness process were killed out from under any other conversation — not a new failure - * mode this route has to account for. + * ── It cascades, and it has to ── + * + * Deleting a commander means being finished with it and with everything it produced. So, in this order: + * + * 1. STOP the processes. The loop the commander started is a descendant of the commander's own `claude`, + * and any agent the loop spawned is a descendant of the loop, so one `killTree` from the session's + * process takes all of it. The comment here used to claim the background process "dies on its own" — + * it does not. Nothing in this route touched a process, so a deleted commander left its loop running: + * still polling, still able to fire, still able to spawn an agent, with no chip to stop it from and no + * row left to disarm. + * 2. DELETE the children's transcripts, read from the spawned record. They cannot be found any other way + * — see `spawned.ts` for the three approaches that do not work. + * 3. DELETE the commander's own transcript and row, as before. + * + * Stopping first is deliberate: a live agent writing to a transcript being deleted is a race, and the loop + * could spawn a new one between steps 2 and 3. + * + * A watcher still armed is not disarmed first, and does not need to be — the row goes away entirely, and + * `armCommandHqWatcher`/`recordCommandHqWatcherFired` can no longer find one to update. */ commandHqRouter.delete('/commanders/:sessionId', async (ctx) => { const user = ctx.get('user'); @@ -483,9 +504,34 @@ commandHqRouter.delete('/commanders/:sessionId', async (ctx) => { const row = await getCommandHqSession(user.id, sessionId); if (!row) throw errors.NOT_FOUND('not a commander'); + // 1. Everything this commander is still running. + const stopped = stopCommanderProcesses(who.email, sessionId); + + // 2. Everything it spawned. Best-effort per child: one transcript that has already been cleaned up must + // not stop the rest from going, and a child is often gone before its parent. + const children = listSpawnedSessions(who.email, sessionId); + let childrenDeleted = 0; + for (const child of children) { + try { + // A spawned agent runs in the platform checkout, not the commanders directory, so its transcript + // lives in that project's group — the same lookup `/chat/sessions/:id` falls back to. + // The cwd is a HINT here, not an authority — `deleteClaudeSession` falls back to locating the + // transcript wherever it actually is, which is what makes this work for a child that ran in the + // platform checkout rather than the commanders directory. + if (deleteClaudeSession(who, getCommandersDir(who.email), child)) childrenDeleted += 1; + } catch { + // Already gone, or never written. Counted as not-deleted rather than failing the request. + } + } + clearSpawnedRecord(who.email, sessionId); + + // 3. The commander itself. deleteClaudeSession(who, getCommandersDir(who.email), sessionId); await deleteCommandHqSession(user.id, sessionId); - return ctx.json({ ok: true }); + + // Said back rather than a bare ok: this is the one irreversible action here, and the counts are what + // make it auditable after the fact. + return ctx.json({ ok: true, processesStopped: stopped, childrenFound: children.length, childrenDeleted }); }); /** @@ -518,6 +564,29 @@ commandHqRouter.post('/commanders/:sessionId/tags', async (ctx) => { return ctx.json({ ok: true }); }); +/** + * GET /command-hq/commanders/:sessionId/record-paths + * + * Where this commander's loop must write its pid and the session ids it spawns. + * + * The composer needs these to build the background-job prompt, and cannot derive them: they are rooted in + * `DATA_PATH`, which is a server fact. Returning paths rather than accepting an upload is deliberate — the + * loop writes them itself with a shell redirect, which needs no token and works from inside a background + * job, where an authenticated POST would need a credential the loop has no business holding. + * + * Not gated on the commander existing as a ROW: a brand-new commander asks for these before its first turn + * has filed it, and refusing then would mean the very first loop is the one that cannot record itself. + */ +commandHqRouter.get('/commanders/:sessionId/record-paths', async (ctx) => { + const user = ctx.get('user'); + const who = await commandHqIdentity(user); + const sessionId = ctx.req.param('sessionId'); + return ctx.json({ + loopPidPath: loopPidPath(who.email, sessionId), + spawnedPath: spawnedRecordPath(who.email, sessionId), + }); +}); + // DELETE /command-hq/commanders/:sessionId/tags/:tagId commandHqRouter.delete('/commanders/:sessionId/tags/:tagId', async (ctx: Context) => { const user = ctx.get('user'); diff --git a/src/servers/api/command-hq/spawned.ts b/src/servers/api/command-hq/spawned.ts new file mode 100644 index 00000000..50f0be97 --- /dev/null +++ b/src/servers/api/command-hq/spawned.ts @@ -0,0 +1,145 @@ +import { existsSync, mkdirSync, readFileSync, rmSync, appendFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { getCommanderStateDir } from '../../data-path'; +import { descendantPids } from '../tasks/process-tree'; + +/* + What a commander spawned, and why the platform cannot work it out for itself. + + A commander's action is a headless agent — `claude -p`, started by the shell loop the commander itself + started. The platform never sees that spawn: it happens inside a background job, two processes below a + conversation, and the agent talks to Anthropic rather than to us. + + Three ways to recover the link were tried and none works: + + - the PROCESS holds nothing. No open transcript descriptor (it appends and closes), and nothing + identifying in its environment. Verified 2026-08-22. + - the TRANSCRIPT is indistinguishable. `queue-operation`, `ai-title`, `last-prompt`, `atis-latch` look + like markers the platform injects and are the harness's own, appearing in every session equally. A + discriminator built on them rejected a real child and reported zero when one was running. + - CONTENT matching finds the wrong sessions. An ordinary chat that happens to quote a procedure matches + as well as the agent following it. + + So the loop records it, and the platform reads it. One newline-delimited file per commander: + + //commander-state//spawned + + The loop can only do that if it knows which commander it belongs to, which it does not by default — the + id is injected into the background-job prompt by `background-job-prompt.ts`. That is the whole chain, and + every link is required. +*/ + +const FILE = 'spawned'; +const PID_FILE = 'loop-pid'; + +/** A session id as the harness writes them. Anything else is a corrupt line and is dropped, not trusted. */ +const SESSION_ID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** Where the loop should append. Handed to the agent in its prompt; nothing guesses it. */ +export function spawnedRecordPath(email: string, commanderSessionId: string): string { + return join(getCommanderStateDir(email, commanderSessionId), FILE); +} + +/** + * Every agent this commander recorded, oldest first, de-duplicated. + * + * Tolerant on purpose. The writer is a shell loop appending under whatever conditions it is in, so a + * truncated write, a blank line or a stray log line must not break a delete — they are dropped and the rest + * is used. A record that fails closed here would make a commander undeletable because of a bad line. + */ +export function listSpawnedSessions(email: string, commanderSessionId: string): string[] { + const path = spawnedRecordPath(email, commanderSessionId); + if (!existsSync(path)) return []; + try { + const seen = new Set(); + for (const raw of readFileSync(path, 'utf-8').split('\n')) { + const id = raw.trim(); + if (SESSION_ID.test(id)) seen.add(id.toLowerCase()); + } + return [...seen]; + } catch { + return []; + } +} + +/** + * Record one, idempotently. For the platform's own use — the loop appends directly, by design. + * + * Kept because the platform spawning an agent on a commander's behalf is the obvious next shape, and when + * that arrives it must write the same file rather than a second record. + */ +export function recordSpawnedSession(email: string, commanderSessionId: string, spawnedId: string): void { + if (!SESSION_ID.test(spawnedId)) return; + if (listSpawnedSessions(email, commanderSessionId).includes(spawnedId.toLowerCase())) return; + const dir = getCommanderStateDir(email, commanderSessionId); + mkdirSync(dir, { recursive: true }); + appendFileSync(join(dir, FILE), `${spawnedId.toLowerCase()}\n`, { mode: 0o600 }); +} + +/** Drop the whole record. Called when the commander is deleted, after its children have been dealt with. */ +export function clearSpawnedRecord(email: string, commanderSessionId: string): void { + try { + rmSync(getCommanderStateDir(email, commanderSessionId), { recursive: true, force: true }); + } catch { + // A record we cannot remove is untidy, not incorrect: the commander is gone and nothing reads this + // without a commander to read it for. + } +} + +/* + ── Stopping what a commander is running ── + + The platform has no session-to-process map: `claude-manager` spawns through the SDK and keeps no pid, and + a `claude` process cannot be attributed to a particular commander from outside — every commander's session + runs with the same cwd, so the process table cannot tell them apart. + + So the loop records its own pid, the same way it records what it spawns, and for the same reason: it is the + only party that knows. From that one number the whole tree follows — the loop's children include any agent + it spawned, and `descendantPids` was written for exactly this ("Bun's proc.kill() signals only the direct + child, so a running grandchild is left orphaned and keeps going"). + + A commander with no recorded pid stops nothing and SAYS so. That is the honest answer for a commander armed + before this existed, and pretending otherwise would report a clean delete over a loop still polling. +*/ + +/** Where the loop should write its own pid. Handed to it in its prompt, like the spawned record. */ +export function loopPidPath(email: string, commanderSessionId: string): string { + return join(getCommanderStateDir(email, commanderSessionId), PID_FILE); +} + +/** The loop's pid, if it recorded one and the process is still alive. */ +export function liveLoopPid(email: string, commanderSessionId: string): number | null { + const path = loopPidPath(email, commanderSessionId); + if (!existsSync(path)) return null; + const pid = Number(readFileSync(path, 'utf-8').trim()); + if (!Number.isInteger(pid) || pid <= 1) return null; + try { + // Signal 0 tests for existence without touching it. A recorded pid whose process has gone is not an + // error — loops end — and reusing a dead pid's number would kill something unrelated. + process.kill(pid, 0); + return pid; + } catch { + return null; + } +} + +/** + * Stop the loop and everything under it. Returns how many processes were signalled. + * + * Children first, then the loop, so the loop cannot spawn a replacement between the two. + */ +export function stopCommanderProcesses(email: string, commanderSessionId: string): number { + const pid = liveLoopPid(email, commanderSessionId); + if (!pid) return 0; + const tree = [...descendantPids(pid), pid]; + let signalled = 0; + for (const target of tree) { + try { + process.kill(target, 'SIGTERM'); + signalled += 1; + } catch { + // Gone between the walk and the signal. Not a failure — that is the outcome we wanted. + } + } + return signalled; +} diff --git a/src/servers/data-path.ts b/src/servers/data-path.ts index a504e432..bb9da9ef 100644 --- a/src/servers/data-path.ts +++ b/src/servers/data-path.ts @@ -219,6 +219,21 @@ export const getEmailAttachmentCacheDir = (ownerEmail: string) => */ export const getCommandersDir = (email: string) => join(DATA_PATH, email, 'commanders'); +/** + * Where a commander records what it spawned — one directory per commander, named by its session id. + * + * Beside the transcripts rather than inside them: a transcript is the harness's file and the platform does + * not write into it. `spawned` is a plain newline-delimited list of session ids, appended by the loop the + * commander started, because nothing else CAN know them — a spawned agent's id is not recoverable from its + * process (no open descriptor, nothing in its environment) and its transcript is indistinguishable from any + * other session's. + * + * That makes this record load-bearing rather than convenient. Without it a commander cannot be deleted + * completely, its children cannot be followed, and its cost cannot be totalled. + */ +export const getCommanderStateDir = (email: string, sessionId: string) => + join(DATA_PATH, email, 'commander-state', sessionId); + /** * Which mail account's store to use, found on DISK rather than by asking the database. * diff --git a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts index f4737538..e95b1617 100644 --- a/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts +++ b/src/workspaces/officerdev/src/apps/Chat/EmbeddableChat/useEmbeddableChat.ts @@ -622,6 +622,13 @@ export function useEmbeddableChat(params: UseEmbeddableChatParams, onMessageComp would lose the brand-new composer, which has no session yet. */ isCommander: params.context === 'command-hq' || params.isCommander === true, + /* + Claude's transcript id, which is what a commander is keyed on everywhere — the Command HQ row, the + record of what it spawned, the delete cascade. `sessionId` above is OFFICER's handle for the same + conversation and is not interchangeable; using it here would key a commander's record on an id nothing + else knows, which is the bug that made a working commander render as an empty pane. + */ + claudeSessionId: chat.claudeSessionId, messages, historicalCount, streamingText, diff --git a/src/workspaces/officerdev/src/apps/Chat/background-job-prompt.ts b/src/workspaces/officerdev/src/apps/Chat/background-job-prompt.ts index 681d3b0c..7dbb5fd0 100644 --- a/src/workspaces/officerdev/src/apps/Chat/background-job-prompt.ts +++ b/src/workspaces/officerdev/src/apps/Chat/background-job-prompt.ts @@ -32,6 +32,14 @@ export type FireBehavior = 'stop' | 'handoff' | 'keep-going'; export type BackgroundJobSpec = { /** `command` runs verbatim; `prompt` is an intent the agent works out. A watcher is always `prompt`. */ mode: 'command' | 'prompt'; + /** + * Where a COMMANDER's loop must record itself. Absent for an ordinary chat's background job. + * + * Two paths, both handed over rather than guessed, because the loop cannot derive either: it does not know + * which commander it belongs to, and nothing outside it knows its pid or what it spawns. Without this + * record a commander cannot be stopped or fully deleted — see `api/command-hq/spawned.ts`. + */ + record?: { loopPidPath: string; spawnedPath: string }; /** The literal command, or the intent. */ body: string; /** Prose: what to wait for and what to do when it happens. Empty means no watcher. */ @@ -141,9 +149,40 @@ export function buildBackgroundJobPrompt(spec: BackgroundJobSpec): string { ); } + /* + A commander's loop must make itself findable. Nothing else can: the platform has no session-to-process + map, a spawned agent's id is not recoverable from its process or its transcript, and the loop is the only + party that knows either. Without these two lines a commander cannot be stopped or deleted completely — + its loop outlives it, still polling and still able to spawn. + + Numbered after whatever came before, like the rules above, and only present when a record path was + supplied — an ordinary chat's background job has no commander to belong to. + */ + // The number the NEXT rule would have taken, so nothing lands on a fraction and the output rule below + // simply moves down one when this is present. + const nextRuleNumber = watching ? 8 : spec.mode === 'command' ? 5 : 4; + if (spec.record) { + rules.push( + ``, + `${nextRuleNumber}. RECORD YOURSELF, first thing, before the wait begins:`, + ``, + ` mkdir -p "$(dirname '${spec.record.loopPidPath}')"`, + ` echo $$ > '${spec.record.loopPidPath}'`, + ``, + ` And every time you spawn an agent, append its session id — the \`session_id\` on the`, + ` \`{"type":"system","subtype":"init"}\` event, which you are already parsing — one per line:`, + ``, + ` echo "" >> '${spec.record.spawnedPath}'`, + ``, + ` This is not bookkeeping. It is the only record of what you are and what you started: the`, + ` platform cannot find your process, and a spawned agent's id is unrecoverable once it exits.`, + ` Without it, deleting this commander leaves you running.`, + ); + } + // Unconditional — every shape of job gets this, not only watchers. `watching`/`spec.mode` only decide // which number it lands on, matching whichever rules already came before it in this particular prompt. - const outputRuleNumber = watching ? 8 : spec.mode === 'command' ? 5 : 4; + const outputRuleNumber = nextRuleNumber + (spec.record ? 1 : 0); rules.push( ``, `${outputRuleNumber}. Do not throw away the command's own output — no \`>/dev/null\`, no \`2>&1\` into a`, diff --git a/src/workspaces/officerdev/src/apps/Chat/components/BackgroundJobDialog.tsx b/src/workspaces/officerdev/src/apps/Chat/components/BackgroundJobDialog.tsx index 0db2dbe7..b1ae5718 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/BackgroundJobDialog.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/BackgroundJobDialog.tsx @@ -35,6 +35,13 @@ type BackgroundJobDialogProps = { cwd: string; /** `armWatcher` is true only for a `handoff` watcher — the one case the caller needs to tell the server about. */ onSubmit: (intent: string, preamble: string, armWatcher: boolean) => void; + /** + * Where a commander's loop must record its pid and what it spawns. Absent for any other chat. + * + * Passed in rather than derived here: the paths are rooted in the platform's data directory and keyed on + * the commander's session id, neither of which a dialog knows. + */ + recordPaths?: { loopPidPath: string; spawnedPath: string }; /** * Is this composer running inside a Command HQ commander? * @@ -55,6 +62,7 @@ export const BackgroundJobDialog = ({ cwd, onSubmit, isCommander = false, + recordPaths, }: BackgroundJobDialogProps) => { const [mode, setMode] = useState<'command' | 'prompt'>('command'); const [body, setBody] = useState(''); @@ -102,6 +110,10 @@ export const BackgroundJobDialog = ({ onFire, duration: timed ? minutes : null, cwd, + // Only a commander gets the record instruction: it is what makes stopping and deleting one possible, + // and an ordinary chat's background job has no commander to belong to. `record` is undefined until the + // session has an id, which is the same moment it becomes a commander — see NewCommander. + ...(recordPaths ? { record: recordPaths } : {}), }; // The intent is what the transcript shows; the preamble rides underneath it. onSubmit( diff --git a/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx b/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx index 913214e1..fdffa53b 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/InputArea.tsx @@ -6,6 +6,7 @@ import type { QueuedPrompt, UseEmbeddableChatType } from '../EmbeddableChat/useE import { ModelSelector } from './ModelSelector'; import { BackgroundJobDialog } from './BackgroundJobDialog'; import { ClaudeLoginDialog } from './ClaudeLoginDialog'; +import { useCommanderRecordPaths } from '../useCommanderRecordPaths'; import { armBackgroundJob } from '../../../background-job-origin'; import { useWorkspace } from '../../../components/Workspace'; import { AttachmentList } from './AttachmentList'; @@ -19,6 +20,9 @@ type InputAreaProps = { export const InputArea = ({ manager }: InputAreaProps) => { const [loginOpen, setLoginOpen] = useState(false); + // Only for a commander, and only once it has an id. Everything a commander's loop needs in order to be + // stoppable and deletable hangs off these two paths — see api/command-hq/spawned.ts. + const recordPaths = useCommanderRecordPaths(manager.isCommander ? manager.claudeSessionId : null); const { input, setInput, @@ -209,6 +213,7 @@ export const InputArea = ({ manager }: InputAreaProps) => { server. Only the radio button was missing, so the feature looked unbuilt. */ isCommander={manager.isCommander} + recordPaths={recordPaths} onSubmit={(intent, preamble, armWatcher) => { // Claimed by the next job to start, which is how the preview tells this one apart from the // background jobs an agent starts on its own. diff --git a/src/workspaces/officerdev/src/apps/Chat/useCommanderRecordPaths.ts b/src/workspaces/officerdev/src/apps/Chat/useCommanderRecordPaths.ts new file mode 100644 index 00000000..4840b8ca --- /dev/null +++ b/src/workspaces/officerdev/src/apps/Chat/useCommanderRecordPaths.ts @@ -0,0 +1,27 @@ +import { useQuery } from '@tanstack/react-query'; +import { useClient } from 'hooks/useClient'; + +type RecordPaths = { loopPidPath: string; spawnedPath: string }; + +/** + * Where this commander's loop must record its pid and what it spawns. + * + * Fetched rather than derived: both are rooted in `DATA_PATH`, a server fact. Cached indefinitely because + * they are a pure function of the session id — nothing can change them but a different commander. + * + * Null for anything that is not a commander, and for a commander that has no id yet. The composer passes + * whatever this returns straight into the background-job prompt, so absent means the loop is simply not + * asked to record — correct for an ordinary chat, and the reason a commander must have its id before its + * first background job. + */ +export function useCommanderRecordPaths(sessionId: string | null): RecordPaths | undefined { + const client = useClient(); + const { data } = useQuery({ + queryKey: ['command-hq', 'record-paths', sessionId], + queryFn: () => client.get(`/command-hq/commanders/${sessionId}/record-paths`), + enabled: !!sessionId, + staleTime: Infinity, + retry: false, + }); + return data; +}