deleting a conversation forgets what the platform stored about it
Found by a sanity check after two commanders were deleted through the UI: the Command HQ tables cascaded perfectly and four OTHER tables kept a row each, for ever. 24 orphaned rows in chat_session_models, 26 in chat_session_events, against 14 live transcripts. Nothing cascades because there is nothing to cascade FROM. All four are keyed on a transcript id, and a transcript is a file the harness writes — there is no row for a conversation, so the database cannot help and the delete path has to be explicit. It was not. Three of the four are inert: a model, a bell preference, a durable copy of turn output nothing will ask for again. `nudges` is not — it is a SCHEDULE, read by the tick in nudger.ts. A nudge belonging to a deleted conversation is a timer that keeps firing at something that no longer exists. That is what turned this from untidy into worth doing now. deleteClaudeSession returns the IDS it removed rather than a boolean, because a `/clear` chain is several transcripts presenting as one conversation and forgetting the head while orphaning its parts is the same bug at a smaller scale. THE TRAP THAT CHANGE INTRODUCED, caught before shipping: every call site said `if (deleteClaudeSession(...))` and an empty array is TRUTHY in JavaScript, so a delete that found nothing would have reported success. The typecheck was clean — `string[]` is a perfectly good truthy expression. All three sites test `.length` now. Fifth silently-wrong thing today that a typecheck could not see. Cleared the existing orphans with the same function the delete path now uses, after checking the one thing that would have made it wrong: whether any of them belonged to a member, whose transcripts live in a home this scan does not read. There is one account on this machine, so they were all the owner's. Two chat_session_events rows appeared again immediately after, and are not a leak: the durable log is written independently of the transcript — that is what makes turn output survive officer being down — so a session that has just begun legitimately has events before a transcript exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,42 @@
|
||||
import { inArray } from 'drizzle-orm';
|
||||
import { db } from './db';
|
||||
import { chatSessionEvents } from './chat-events/schema';
|
||||
import { chatNotificationPrefs } from './chat-notification-prefs/schema';
|
||||
import { chatSessionModels } from './chat-session-models/schema';
|
||||
import { nudges } from './nudges/schema';
|
||||
|
||||
/*
|
||||
Everything the platform stores ABOUT a conversation, forgotten together.
|
||||
|
||||
── Why this exists as one function ──
|
||||
|
||||
Four tables are keyed on a transcript id and none of them has a foreign key to anything, because the thing
|
||||
they describe is a FILE — a transcript on disk, written by the harness. There is no row for a chat session
|
||||
to reference, so the database cannot cascade and the delete path has to be explicit.
|
||||
|
||||
It was not. Deleting a conversation removed the transcript and left all four behind, for ever. Found on
|
||||
2026-08-22 by counting rows after two commanders were deleted through the UI: `chat_session_models` held 36
|
||||
rows against far fewer live conversations.
|
||||
|
||||
── Why it is more than untidy ──
|
||||
|
||||
Three of the four are inert — a model, a bell preference, a durable copy of turn output that nothing will
|
||||
ask for again. `nudges` is not: it is a SCHEDULE, and the tick in `nudger.ts` reads it. A nudge left behind
|
||||
by a deleted conversation is a timer that keeps firing at something that does not exist.
|
||||
|
||||
── Why it takes a list ──
|
||||
|
||||
A `/clear` chain is several transcripts presenting as one conversation, and deleting it deletes every part.
|
||||
Forgetting one id and leaving its siblings is the same bug at a smaller scale, so the caller passes the ids
|
||||
it actually removed.
|
||||
*/
|
||||
export async function forgetChatSessions(sessionIds: string[]): Promise<void> {
|
||||
if (!sessionIds.length) return;
|
||||
// Sequential rather than a transaction: these are independent tables with no invariant between them, and
|
||||
// a partial forget is strictly better than a failed delete that leaves the transcript gone and every row
|
||||
// in place. Each is idempotent — running it twice removes nothing the second time.
|
||||
await db.delete(nudges).where(inArray(nudges.sessionId, sessionIds));
|
||||
await db.delete(chatSessionModels).where(inArray(chatSessionModels.sessionId, sessionIds));
|
||||
await db.delete(chatNotificationPrefs).where(inArray(chatNotificationPrefs.sessionId, sessionIds));
|
||||
await db.delete(chatSessionEvents).where(inArray(chatSessionEvents.sessionId, sessionIds));
|
||||
}
|
||||
@@ -39,6 +39,7 @@ export * from './nudges';
|
||||
export * from './ai-provider-usage';
|
||||
export * from './anthropic-accounts';
|
||||
export * from './theme-favorites';
|
||||
export { forgetChatSessions } from './chat-session-cleanup';
|
||||
export * from './command-hq';
|
||||
export * from './dashboards';
|
||||
export * from './integrations';
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
getChatNotifyPreference,
|
||||
getChatNotifyPreferences,
|
||||
setChatNotifyPreference,
|
||||
forgetChatSessions,
|
||||
} from 'officerdb';
|
||||
import type { NotifyPreference } from 'officerdb/types';
|
||||
import { setNudge, getNudge, cancelNudge } from 'officerdb';
|
||||
@@ -376,8 +377,21 @@ chatRouter.delete('/sessions/:id', async (ctx) => {
|
||||
const id = ctx.req.param('id');
|
||||
const cwd = cwdOf(ctx, who.home);
|
||||
if (isOpenCodeSessionId(id) && !who.isOwner) return ctx.text('Not found', 404);
|
||||
const ok = isOpenCodeSessionId(id) ? await deleteOpenCodeSession(id) : deleteClaudeSession(who, cwd, id);
|
||||
if (!ok) return ctx.text('Not found', 404);
|
||||
if (isOpenCodeSessionId(id)) {
|
||||
if (!(await deleteOpenCodeSession(id))) return ctx.text('Not found', 404);
|
||||
return ctx.json({ ok: true });
|
||||
}
|
||||
|
||||
// `.length`, not truthiness: this returns the IDS it removed and an empty array is truthy in JavaScript,
|
||||
// so `if (removed)` would report a delete that found nothing as a success.
|
||||
const removed = deleteClaudeSession(who, cwd, id);
|
||||
if (!removed.length) return ctx.text('Not found', 404);
|
||||
|
||||
// The transcript is gone; forget what the platform stored ABOUT it. Four tables are keyed on a transcript
|
||||
// id with no foreign key — there is no row for a conversation to reference — so nothing cascades and this
|
||||
// has to be explicit. One of them, `nudges`, is a live schedule: left behind it keeps firing at a
|
||||
// conversation that no longer exists.
|
||||
await forgetChatSessions(removed);
|
||||
return ctx.json({ ok: true });
|
||||
});
|
||||
|
||||
|
||||
@@ -642,9 +642,12 @@ const findTranscript = (who: ChatIdentity, cwd: string, sessionId: string): stri
|
||||
* routinely (see `findTranscript`), and the wrong group would find no chain and silently delete one
|
||||
* part of several.
|
||||
*/
|
||||
export function deleteClaudeSession(who: ChatIdentity, cwd: string, sessionId: string): boolean {
|
||||
export function deleteClaudeSession(who: ChatIdentity, cwd: string, sessionId: string): string[] {
|
||||
const filePath = findTranscript(who, cwd, sessionId);
|
||||
if (!filePath) return false;
|
||||
// The IDS it removed, not a boolean — empty means nothing was there. The caller needs them to forget the
|
||||
// rows keyed on each one (`forgetChatSessions`), and a `/clear` chain is several transcripts presenting as
|
||||
// one conversation: returning true would have told a caller to forget the head and orphan its parts.
|
||||
if (!filePath) return [];
|
||||
|
||||
const ownCwd = firstCwd(who.osUser, filePath);
|
||||
const ids = ownCwd ? chainFileIds(who, ownCwd, sessionId) : [sessionId];
|
||||
@@ -653,7 +656,7 @@ export function deleteClaudeSession(who: ChatIdentity, cwd: string, sessionId: s
|
||||
// success — the conversation reappeared on the next refresh.
|
||||
const dir = dirname(filePath);
|
||||
for (const id of ids) removeAs(who.osUser, join(dir, `${id}.jsonl`));
|
||||
return true;
|
||||
return ids;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -464,19 +464,41 @@ function createMessageHandler(
|
||||
Fire-and-forget, swallowed failure, same posture as `record`: a database hiccup here must not be a
|
||||
reason a chat turn fails.
|
||||
*/
|
||||
// Whether the chosen name has actually reached the transcript. False until a rename RETURNS true — see
|
||||
// the retry note inside `fileAsCommander`.
|
||||
let commanderNamed = false;
|
||||
|
||||
const fileAsCommander = (): void => {
|
||||
// TEMPORARY instrumentation, 2026-08-22. The chosen name has failed to reach the transcript twice, and
|
||||
// reasoning about the path has been wrong twice. Remove once the cause is known.
|
||||
logger.warn('[commander-name] fileAsCommander', {
|
||||
commandHq,
|
||||
transcriptId,
|
||||
hasName: !!commanderName,
|
||||
name: commanderName ?? null,
|
||||
alreadyNamed: commanderNamed,
|
||||
});
|
||||
if (!commandHq || !transcriptId) return;
|
||||
const userId = sessionManager.getSession(sessionId)?.userId;
|
||||
if (!userId) return;
|
||||
const filedId = transcriptId;
|
||||
void createCommandHqSession(userId, filedId)
|
||||
.then(async () => {
|
||||
// The chosen name, applied once and only if one was chosen.
|
||||
//
|
||||
// `renameClaudeSession` writes a `summary` record, which is what sets `hasExplicitTitle` — and an
|
||||
// explicit title is never overwritten by the harness's own `ai-title` or by the first-message
|
||||
// fallback. So this wins permanently rather than racing the titling.
|
||||
if (!commanderName?.trim()) return;
|
||||
/*
|
||||
The chosen name, applied once — and RETRIED, because the first attempt usually fails.
|
||||
|
||||
`fileAsCommander` runs on `session:claude`, which arrives at the START of a turn: the transcript
|
||||
file may not be on disk yet, and `renameClaudeSession` returns false when it cannot find one. It
|
||||
returns a boolean that nothing was checking, so the name was silently dropped and the conversation
|
||||
kept the harness's own `ai-title` — which is exactly what the first live test showed.
|
||||
|
||||
So the attempt is recorded and retried at the end of the turn, by which point the file certainly
|
||||
exists. Idempotent: once it succeeds, `commanderNamed` stops it repeating.
|
||||
|
||||
It still writes a `summary` record, which sets `hasExplicitTitle` — and an explicit title is never
|
||||
overwritten by `ai-title` or by the first-message fallback. So it wins whenever it lands.
|
||||
*/
|
||||
if (!commanderName?.trim() || commanderNamed) return;
|
||||
// The same identity resolution every chat route uses, rather than a second copy of it: it is what
|
||||
// decides the home and the OS user, and a rename writes a file as that user.
|
||||
const user = await getUserById(userId).catch(() => null);
|
||||
@@ -487,7 +509,8 @@ function createMessageHandler(
|
||||
// working directory now, so that path is usually wrong. `findTranscript` treats the cwd as a HINT
|
||||
// and locates the transcript wherever it actually is — so this works either way, and passing a
|
||||
// path we know is probably wrong would be the kind of thing that reads as intent later.
|
||||
renameClaudeSession(who, who.home, filedId, commanderName.trim());
|
||||
commanderNamed = renameClaudeSession(who, who.home, filedId, commanderName.trim());
|
||||
logger.warn('[commander-name] rename attempt', { filedId, home: who.home, ok: commanderNamed });
|
||||
})
|
||||
.catch((err: unknown) => {
|
||||
logger.warn('Could not file this session as a commander', {
|
||||
@@ -554,6 +577,9 @@ function createMessageHandler(
|
||||
record();
|
||||
fileAsCommander();
|
||||
}
|
||||
// And retried on every result until the name lands. The first attempt runs before the transcript file
|
||||
// exists, so the end of the first turn is the earliest a rename can succeed.
|
||||
if (msg.type === 'result' && commandHq && !commanderNamed) fileAsCommander();
|
||||
|
||||
/*
|
||||
The watcher loop — see `docs/command-hq.md` → "The watcher loop" and `handoffContract` in
|
||||
|
||||
@@ -5,7 +5,7 @@ import { checkBudget } from '../sub-agents/budget';
|
||||
import * as errors from '@@/custom-errors';
|
||||
import { resolveHomeDir } from '@@/user-home';
|
||||
import { getCommandersDir, toShellUsername } from '@@/data-path';
|
||||
import { getChatEventsSince, ingestSpawnedChildren } from 'officerdb';
|
||||
import { forgetChatSessions, getChatEventsSince, ingestSpawnedChildren } from 'officerdb';
|
||||
import {
|
||||
clearSpawnedRecord,
|
||||
listSpawnedSessions,
|
||||
@@ -552,14 +552,23 @@ commandHqRouter.delete('/commanders/:sessionId', async (ctx) => {
|
||||
(c) => c.spawnedSessionId,
|
||||
);
|
||||
let childrenDeleted = 0;
|
||||
// Every transcript id actually removed — the commander's and its children's, chains included. Collected
|
||||
// rather than assumed, because a `/clear` chain is several ids behind one conversation.
|
||||
const forgotten: string[] = [];
|
||||
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, who.home, child)) childrenDeleted += 1;
|
||||
// transcript wherever it actually is, which is what makes this work for a child that ran in a
|
||||
// different directory from its parent.
|
||||
//
|
||||
// `.length`, not truthiness: this returns the ids it removed, and an empty array is truthy.
|
||||
const removedChild = deleteClaudeSession(who, who.home, child);
|
||||
if (removedChild.length) {
|
||||
childrenDeleted += 1;
|
||||
forgotten.push(...removedChild);
|
||||
}
|
||||
} catch {
|
||||
// Already gone, or never written. Counted as not-deleted rather than failing the request.
|
||||
}
|
||||
@@ -567,9 +576,21 @@ commandHqRouter.delete('/commanders/:sessionId', async (ctx) => {
|
||||
clearSpawnedRecord(who.email, sessionId);
|
||||
|
||||
// 3. The commander itself.
|
||||
deleteClaudeSession(who, who.home, sessionId);
|
||||
forgotten.push(...deleteClaudeSession(who, who.home, sessionId));
|
||||
await deleteCommandHqSession(user.id, sessionId);
|
||||
|
||||
/*
|
||||
Forget what the platform stored ABOUT every transcript that just went — the commander's and its
|
||||
children's, in one call.
|
||||
|
||||
The `command_hq_*` tables cascade from the row, because they reference it. These four do not: they are
|
||||
keyed on a transcript id, which is a FILE, so there is no row for them to reference and nothing to
|
||||
cascade from. Left behind they are mostly inert — a model, a bell preference, a durable copy of turn
|
||||
output — except `nudges`, which is a live schedule that would keep firing at a conversation that no
|
||||
longer exists.
|
||||
*/
|
||||
await forgetChatSessions(forgotten);
|
||||
|
||||
// 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 });
|
||||
|
||||
Reference in New Issue
Block a user