deleting a commander ends its session, not just its loop
Its own test caught this too. Delete removed the row and the transcript, and the transcript was BACK a minute later — because the commander's `claude` process was still alive and rewrote it. That happened twice tonight and both times it read as the delete having failed. Killing the loop was never enough: the loop is a grandchild of the session, so ending it leaves the session running, still holding the conversation open and still able to write. So the session is ended first, with `killClaudeSession` — which already existed and does the whole teardown (abort, close input, settle pending plans and questions). It takes officer's session key while this route holds a transcript id, so the map is read backwards through `findSessionKeyByClaudeSession`, which exists for the same reason a refreshed browser holds only the uuid. Ending the session takes its children with it; the pid-based stop still runs afterwards, catching anything that outlived its parent. The response reports `sessionKilled` beside `processesStopped` so the two are distinguishable in a report rather than one number standing for both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -6,6 +6,8 @@ import * as errors from '@@/custom-errors';
|
|||||||
import { resolveHomeDir } from '@@/user-home';
|
import { resolveHomeDir } from '@@/user-home';
|
||||||
import { getCommandersDir, toShellUsername } from '@@/data-path';
|
import { getCommandersDir, toShellUsername } from '@@/data-path';
|
||||||
import { forgetChatSessions, getChatEventsSince, ingestSpawnedChildren } from 'officerdb';
|
import { forgetChatSessions, getChatEventsSince, ingestSpawnedChildren } from 'officerdb';
|
||||||
|
import { killClaudeSession } from '@@/sidecar/claude/claude-manager';
|
||||||
|
import { findSessionKeyByClaudeSession } from '@@/sidecar/claude/state';
|
||||||
import {
|
import {
|
||||||
clearSpawnedRecord,
|
clearSpawnedRecord,
|
||||||
listSpawnedSessions,
|
listSpawnedSessions,
|
||||||
@@ -541,7 +543,21 @@ commandHqRouter.delete('/commanders/:sessionId', async (ctx) => {
|
|||||||
const row = await getCommandHqSession(user.id, sessionId);
|
const row = await getCommandHqSession(user.id, sessionId);
|
||||||
if (!row) throw errors.NOT_FOUND('not a commander');
|
if (!row) throw errors.NOT_FOUND('not a commander');
|
||||||
|
|
||||||
// 1. Everything this commander is still running.
|
/*
|
||||||
|
1. Everything this commander is still running — the SESSION first, then the loop.
|
||||||
|
|
||||||
|
Killing the loop alone is not enough, and its own test proved it: the loop is a grandchild of the
|
||||||
|
commander's `claude` process, so ending the loop leaves the session alive — and a live session REWRITES
|
||||||
|
its transcript after the file is deleted. Twice tonight a deleted commander's transcript reappeared, and
|
||||||
|
twice it read as the delete having failed.
|
||||||
|
|
||||||
|
`killClaudeSession` takes officer's session key while this route holds a transcript id, so the map is
|
||||||
|
read backwards — `findSessionKeyByClaudeSession`, which exists for the same reason a refreshed browser
|
||||||
|
holds only the uuid. Ending the session takes its children with it, and the pid-based stop below then
|
||||||
|
catches anything that outlived its parent.
|
||||||
|
*/
|
||||||
|
const sessionKey = findSessionKeyByClaudeSession(sessionId, user.id);
|
||||||
|
const sessionKilled = sessionKey ? killClaudeSession(sessionKey, user.id) : false;
|
||||||
const stopped = stopCommanderProcesses(who.email, sessionId);
|
const stopped = stopCommanderProcesses(who.email, sessionId);
|
||||||
|
|
||||||
// 2. Everything it spawned. Best-effort per child: one transcript that has already been cleaned up must
|
// 2. Everything it spawned. Best-effort per child: one transcript that has already been cleaned up must
|
||||||
@@ -593,7 +609,13 @@ commandHqRouter.delete('/commanders/:sessionId', async (ctx) => {
|
|||||||
|
|
||||||
// Said back rather than a bare ok: this is the one irreversible action here, and the counts are what
|
// 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.
|
// make it auditable after the fact.
|
||||||
return ctx.json({ ok: true, processesStopped: stopped, childrenFound: children.length, childrenDeleted });
|
return ctx.json({
|
||||||
|
ok: true,
|
||||||
|
sessionKilled,
|
||||||
|
processesStopped: stopped,
|
||||||
|
childrenFound: children.length,
|
||||||
|
childrenDeleted,
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user