From 1f62bcd919270f14779687694cabcfb2303be913 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Fri, 8 May 2026 09:58:24 +0800 Subject: [PATCH] Prevent IM chats from reusing deleted desktop sessions Deleting a desktop session removed the transcript but left IM adapter chat mappings and live WebSocket state able to point at the old session. The server now closes the active session socket and removes adapter mappings after deletion, while adapters refresh the shared session store before reads so a running process cannot reuse stale in-memory data. Constraint: Adapter session mappings are shared through adapter-sessions.json across long-running IM processes Rejected: Patch each adapter ensureSession path separately | shared store refresh fixes all current adapters and avoids drift Confidence: high Scope-risk: moderate Directive: Do not cache adapter session mappings without invalidating after server-side session deletion Tested: bun test common/__tests__/session-store.test.ts common/__tests__/ws-bridge.test.ts Tested: bun test src/server/__tests__/websocket-handler.test.ts src/server/__tests__/sessions.test.ts --timeout 30000 Tested: cd adapters && bunx tsc --noEmit Tested: bun run check:adapters Tested: bun run check:server Not-tested: bun run verify fails on pre-existing agent-utils coverage ratchet; changed-line coverage is 95.92% and affected lanes pass Related: https://github.com/NanmiCoder/cc-haha/issues/305 --- .../common/__tests__/session-store.test.ts | 35 ++++++++++++++++++ adapters/common/__tests__/ws-bridge.test.ts | 18 +++++++++ adapters/common/session-store.ts | 22 +++++++++++ adapters/common/ws-bridge.ts | 9 ++++- src/server/__tests__/sessions.test.ts | 37 +++++++++++++++++++ .../__tests__/websocket-handler.test.ts | 17 +++++++++ src/server/api/sessions.ts | 12 +++++- src/server/ws/handler.ts | 18 +++++++++ 8 files changed, 166 insertions(+), 2 deletions(-) diff --git a/adapters/common/__tests__/session-store.test.ts b/adapters/common/__tests__/session-store.test.ts index f27b91f8..a029084e 100644 --- a/adapters/common/__tests__/session-store.test.ts +++ b/adapters/common/__tests__/session-store.test.ts @@ -41,6 +41,41 @@ describe('SessionStore', () => { expect(store.get('chat-1')).toBeNull() }) + it('deletes every chat entry bound to a sessionId', () => { + store.set('chat-1', 'uuid-shared', '/project-a') + store.set('chat-2', 'uuid-other', '/project-b') + store.set('chat-3', 'uuid-shared', '/project-c') + + const removed = store.deleteBySessionId('uuid-shared') + + expect(removed.sort()).toEqual(['chat-1', 'chat-3']) + expect(store.get('chat-1')).toBeNull() + expect(store.get('chat-3')).toBeNull() + expect(store.get('chat-2')!.sessionId).toBe('uuid-other') + + const reloaded = new SessionStore(path.join(tmpDir, 'sessions.json')) + expect(reloaded.get('chat-1')).toBeNull() + expect(reloaded.get('chat-3')).toBeNull() + expect(reloaded.get('chat-2')!.sessionId).toBe('uuid-other') + }) + + it('refreshes from disk before reading so running adapters do not reuse deleted mappings', () => { + store.set('chat-1', 'uuid-stale', '/project') + const serverSideStore = new SessionStore(path.join(tmpDir, 'sessions.json')) + + expect(serverSideStore.deleteBySessionId('uuid-stale')).toEqual(['chat-1']) + + expect(store.get('chat-1')).toBeNull() + expect(store.listAll()).toEqual([]) + }) + + it('returns an empty list when deleting an unknown sessionId', () => { + store.set('chat-1', 'uuid-aaa', '/project') + + expect(store.deleteBySessionId('uuid-missing')).toEqual([]) + expect(store.get('chat-1')!.sessionId).toBe('uuid-aaa') + }) + it('persists to disk and reloads', () => { store.set('chat-1', 'uuid-aaa', '/path') diff --git a/adapters/common/__tests__/ws-bridge.test.ts b/adapters/common/__tests__/ws-bridge.test.ts index b691335d..6f4c4490 100644 --- a/adapters/common/__tests__/ws-bridge.test.ts +++ b/adapters/common/__tests__/ws-bridge.test.ts @@ -163,6 +163,24 @@ describe('WsBridge: handler serialization', () => { bridge.destroy() }) + it('forgets a chat when the server closes the session normally', async () => { + const bridge = new WsBridge(serverUrl, 'test') + bridge.onServerMessage('chat-deleted', () => {}) + bridge.connectSession('chat-deleted', 'sess-deleted') + await bridge.waitForOpen('chat-deleted') + + const serverWs = connections[0]! + serverWs.close(1000, 'session deleted') + + await new Promise((resolve) => setTimeout(resolve, 50)) + + expect(bridge.hasSession('chat-deleted')).toBe(false) + await new Promise((resolve) => setTimeout(resolve, 1_100)) + expect(connections).toHaveLength(1) + + bridge.destroy() + }) + it('resetSession clears the handler chain', async () => { const bridge = new WsBridge(serverUrl, 'test') bridge.onServerMessage('chat-reset', () => {}) diff --git a/adapters/common/session-store.ts b/adapters/common/session-store.ts index a54a6550..2ca4f16c 100644 --- a/adapters/common/session-store.ts +++ b/adapters/common/session-store.ts @@ -25,23 +25,45 @@ export class SessionStore { } get(chatId: string): SessionEntry | null { + this.refresh() return this.data[chatId] ?? null } set(chatId: string, sessionId: string, workDir: string): void { + this.refresh() this.data[chatId] = { sessionId, workDir, updatedAt: Date.now() } this.save() } delete(chatId: string): void { + this.refresh() delete this.data[chatId] this.save() } + deleteBySessionId(sessionId: string): string[] { + this.refresh() + const removed: string[] = [] + for (const [chatId, entry] of Object.entries(this.data)) { + if (entry.sessionId !== sessionId) continue + delete this.data[chatId] + removed.push(chatId) + } + if (removed.length > 0) { + this.save() + } + return removed + } + listAll(): Array<{ chatId: string } & SessionEntry> { + this.refresh() return Object.entries(this.data).map(([chatId, entry]) => ({ chatId, ...entry })) } + private refresh(): void { + this.data = this.load() + } + private load(): StoreData { try { return JSON.parse(fs.readFileSync(this.filePath, 'utf-8')) diff --git a/adapters/common/ws-bridge.ts b/adapters/common/ws-bridge.ts index 4f87e461..2ccc6ca5 100644 --- a/adapters/common/ws-bridge.ts +++ b/adapters/common/ws-bridge.ts @@ -201,7 +201,14 @@ export class WsBridge { ws.on('close', (code, reason) => { console.log(`[WsBridge] Disconnected: ${sessionId} (${code}: ${reason})`) - if (code === 1000) return + if (this.sessions.get(chatId) !== session) return + if (code === 1000) { + if (session.reconnectTimer) clearTimeout(session.reconnectTimer) + this.sessions.delete(chatId) + this.handlers.delete(chatId) + this.handlerChains.delete(chatId) + return + } this.scheduleReconnect(chatId, sessionId) }) diff --git a/src/server/__tests__/sessions.test.ts b/src/server/__tests__/sessions.test.ts index e82cad6e..a39eef73 100644 --- a/src/server/__tests__/sessions.test.ts +++ b/src/server/__tests__/sessions.test.ts @@ -1364,6 +1364,43 @@ describe('Sessions API', () => { expect(res2.status).toBe(404) }) + it('DELETE /api/sessions/:id should remove matching IM adapter session mappings', async () => { + const sessionId = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + const otherSessionId = 'ffffffff-1111-2222-3333-ffffffffffff' + await writeSessionFile('-tmp-api-test', sessionId, [makeSnapshotEntry()]) + await fs.writeFile( + path.join(tmpDir, 'adapter-sessions.json'), + JSON.stringify({ + 'wechat-chat': { + sessionId, + workDir: '/tmp/project-a', + updatedAt: 1, + }, + 'wechat-chat-2': { + sessionId, + workDir: '/tmp/project-b', + updatedAt: 2, + }, + 'other-chat': { + sessionId: otherSessionId, + workDir: '/tmp/project-c', + updatedAt: 3, + }, + }, null, 2), + 'utf-8', + ) + + const res = await fetch(`${baseUrl}/api/sessions/${sessionId}`, { method: 'DELETE' }) + expect(res.status).toBe(200) + + const persisted = JSON.parse( + await fs.readFile(path.join(tmpDir, 'adapter-sessions.json'), 'utf-8'), + ) + expect(persisted['wechat-chat']).toBeUndefined() + expect(persisted['wechat-chat-2']).toBeUndefined() + expect(persisted['other-chat'].sessionId).toBe(otherSessionId) + }) + it('DELETE /api/sessions/:id should roll back the deleted marker when file deletion fails', async () => { const sessionId = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' await writeSessionFile('-tmp-api-test', sessionId, [makeSnapshotEntry()]) diff --git a/src/server/__tests__/websocket-handler.test.ts b/src/server/__tests__/websocket-handler.test.ts index 33224b10..d12f4074 100644 --- a/src/server/__tests__/websocket-handler.test.ts +++ b/src/server/__tests__/websocket-handler.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, mock, spyOn } from 'bun:test' import type { ServerWebSocket } from 'bun' import { __resetWebSocketHandlerStateForTests, + closeSessionConnection, getActiveSessionIds, handleWebSocket, type WebSocketData, @@ -52,4 +53,20 @@ describe('WebSocket handler session isolation', () => { expect(clearCallbacks).not.toHaveBeenCalled() expect(cancelComputerUse).not.toHaveBeenCalled() }) + + it('closes and removes an active client socket when a session is deleted', () => { + const sessionId = `delete-${crypto.randomUUID()}` + const ws = makeClientSocket(sessionId) + const clearCallbacks = spyOn(conversationService, 'clearOutputCallbacks') + const cancelComputerUse = spyOn(computerUseApprovalService, 'cancelSession') + + handleWebSocket.open(ws) + + expect(closeSessionConnection(sessionId, 'session deleted')).toBe(true) + + expect(getActiveSessionIds()).not.toContain(sessionId) + expect(ws.close).toHaveBeenCalledWith(1000, 'session deleted') + expect(clearCallbacks).toHaveBeenCalledWith(sessionId) + expect(cancelComputerUse).toHaveBeenCalledWith(sessionId) + }) }) diff --git a/src/server/api/sessions.ts b/src/server/api/sessions.ts index 44f18248..f9077cea 100644 --- a/src/server/api/sessions.ts +++ b/src/server/api/sessions.ts @@ -17,7 +17,7 @@ import { sessionService } from '../services/sessionService.js' import { conversationService } from '../services/conversationService.js' import { ApiError, errorResponse } from '../middleware/errorHandler.js' -import { getSlashCommands } from '../ws/handler.js' +import { closeSessionConnection, getSlashCommands } from '../ws/handler.js' import { getCommandName } from '../../commands.js' import { getSkillDirCommands } from '../../skills/loadSkillsDir.js' import { WorkspaceService } from '../services/workspaceService.js' @@ -32,6 +32,7 @@ import { previewSessionRewind, type RewindTargetSelector, } from '../services/sessionRewindService.js' +import { SessionStore } from '../../../adapters/common/session-store.js' const workspaceService = new WorkspaceService( async (sessionId) => ( @@ -353,9 +354,18 @@ async function deleteSession(sessionId: string): Promise { conversationService.unmarkSessionDeleted(sessionId) throw error } + closeSessionConnection(sessionId, 'session deleted') + cleanupAdapterSessionMappings(sessionId) return Response.json({ ok: true }) } +function cleanupAdapterSessionMappings(sessionId: string): void { + const removedChatIds = new SessionStore().deleteBySessionId(sessionId) + if (removedChatIds.length > 0) { + console.log(`[Sessions API] Removed ${removedChatIds.length} adapter session mapping(s) for ${sessionId}`) + } +} + async function getSessionSlashCommands(sessionId: string): Promise { const cachedCommands = getSlashCommands(sessionId) if (cachedCommands.length > 0) { diff --git a/src/server/ws/handler.ts b/src/server/ws/handler.ts index be68d521..d5b2afca 100644 --- a/src/server/ws/handler.ts +++ b/src/server/ws/handler.ts @@ -1493,6 +1493,24 @@ export function sendToSession(sessionId: string, message: ServerMessage): boolea return true } +export function closeSessionConnection(sessionId: string, reason = 'session closed'): boolean { + const cleanupTimer = sessionCleanupTimers.get(sessionId) + if (cleanupTimer) { + clearTimeout(cleanupTimer) + sessionCleanupTimers.delete(sessionId) + } + computerUseApprovalService.cancelSession(sessionId) + conversationService.clearOutputCallbacks(sessionId) + cleanupSessionRuntimeState(sessionId) + + const ws = activeSessions.get(sessionId) + if (!ws) return false + + activeSessions.delete(sessionId) + ws.close(1000, reason) + return true +} + export function getActiveSessionIds(): string[] { return Array.from(activeSessions.keys()) }