diff --git a/src/databases/officer_db/src/chat-session-cleanup.ts b/src/databases/officer_db/src/chat-session-cleanup.ts index a181c2aa..09b8d999 100644 --- a/src/databases/officer_db/src/chat-session-cleanup.ts +++ b/src/databases/officer_db/src/chat-session-cleanup.ts @@ -1,4 +1,4 @@ -import { inArray, sql } from 'drizzle-orm'; +import { eq, inArray, sql } from 'drizzle-orm'; import { db } from './db'; import { chatSessionEvents } from './chat-events/schema'; import { chatNotificationPrefs } from './chat-notification-prefs/schema'; @@ -90,3 +90,24 @@ export async function forgetChatSessions({ transcriptIds, sessionKeys = [] }: Fo await db.delete(chatSessionEvents).where(inArray(chatSessionEvents.sessionId, sessionKeys)); } } + +/** + * Officer's session key for a transcript, read from the durable event log. + * + * The in-memory maps (`sessionKeyForTranscript`, `findSessionKeyByClaudeSession`) only answer for a session + * younger than the process that is asking. After a restart both return undefined, and the delete path then + * skips the kill entirely — so the transcript is removed while the harness is still writing it, and the + * harness writes it straight back. A deleted commander reappears as an untitled stub. + * + * Every `result` event carries both ids in its payload, which survives any restart. Same trick as + * `forgetChatSessions`, and for the same reason: the rows are the only place the two id spaces are written + * down together. + */ +export async function sessionKeyForTranscriptId(transcriptId: string): Promise { + const [row] = await db + .select({ sessionId: chatSessionEvents.sessionId }) + .from(chatSessionEvents) + .where(eq(sql`${chatSessionEvents.event}->>'claudeSessionId'`, transcriptId)) + .limit(1); + return row?.sessionId; +} diff --git a/src/databases/officer_db/src/index.ts b/src/databases/officer_db/src/index.ts index 34e9a5e3..53275efb 100644 --- a/src/databases/officer_db/src/index.ts +++ b/src/databases/officer_db/src/index.ts @@ -39,7 +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 { forgetChatSessions, sessionKeyForTranscriptId } from './chat-session-cleanup'; export * from './command-hq'; export * from './dashboards'; export * from './integrations'; diff --git a/src/servers/api/chat/claude-sessions.ts b/src/servers/api/chat/claude-sessions.ts index a959e41e..2288c7e1 100644 --- a/src/servers/api/chat/claude-sessions.ts +++ b/src/servers/api/chat/claude-sessions.ts @@ -999,9 +999,23 @@ function scanGroup(who: ChatIdentity, cwd: string): TranscriptSummary[] { * It is applied by the CALLER, because that requires a database read and this function is synchronous over a * directory scan. */ +/** + * A transcript with no human or assistant turn in it is not a conversation. + * + * The harness writes metadata records — `last-prompt`, `ai-title`, `atis-latch` — into a transcript file + * independently of the conversation, and it will happily create the file to hold them. So a session that was + * deleted, or one that never sent anything, leaves a file containing only those three lines, which listed as + * "(untitled) · 0 msgs" and accumulated: four of them from one evening of testing. + * + * Filtered at the list rather than deleted on disk. The files are the harness's, it is entitled to write + * them, and a cleanup that raced it would be the same class of bug as the one that produced them. + */ +const hasConversation = (s: ClaudeSessionSummary): boolean => s.messageCount > 0; + export function listClaudeSessions(who: ChatIdentity, cwd: string): ClaudeSessionSummary[] { return mergeChains(scanGroup(who, cwd)) .map(publish) + .filter(hasConversation) .sort((a, b) => b.updatedAt.localeCompare(a.updatedAt)); } diff --git a/src/servers/api/command-hq/router.ts b/src/servers/api/command-hq/router.ts index 0f5622b4..58641d31 100644 --- a/src/servers/api/command-hq/router.ts +++ b/src/servers/api/command-hq/router.ts @@ -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 { forgetChatSessions, getChatEventsSince, ingestSpawnedChildren } from 'officerdb'; +import { forgetChatSessions, getChatEventsSince, ingestSpawnedChildren, sessionKeyForTranscriptId } from 'officerdb'; import * as sidecar from '@@/sidecar-registry'; import { findSessionKeyByClaudeSession } from '@@/sidecar/claude/state'; import { sessionKeyForTranscript } from '../chat/websocket'; @@ -559,7 +559,18 @@ commandHqRouter.delete('/commanders/:sessionId', async (ctx) => { */ // Same mapping problem as the briefing: the sidecar's map is not visible from here, so the platform's own // is asked first and the sidecar's is kept as a fallback for a session that predates this process. - const sessionKey = sessionKeyForTranscript(sessionId) ?? findSessionKeyByClaudeSession(sessionId, user.id); + /* + Third fallback, and the only one that survives a restart. + + Both maps above are in-memory and answer only for a session younger than this process. After a + restart they both return undefined, no kill is sent, and the transcript is deleted while the harness + is still writing — so it writes it back and the deleted commander returns as an untitled stub. Seen + twice tonight, after the fix that was meant to prevent exactly this. + */ + const sessionKey = + sessionKeyForTranscript(sessionId) ?? + findSessionKeyByClaudeSession(sessionId, user.id) ?? + (await sessionKeyForTranscriptId(sessionId).catch(() => undefined)); /* Across the socket, and AWAITED — both halves matter.