mirror of
https://github.com/NanmiCoder/claude-code-haha.git
synced 2026-10-10 20:03:13 +08:00
fix: isolate desktop session streaming state
Desktop chat sessions can stream and reconnect concurrently, so shared in-process buffers must not be keyed only by process lifetime. This change scopes streaming delta buffers, task-tool refresh bookkeeping, and CLI task mutations to the session that produced the event. It also ignores stale client socket closes after a newer socket has replaced the active connection for the same session. Constraint: Desktop users can keep multiple sessions and tabs active at the same time. Rejected: Serialize all desktop streaming through one active session | would hide the race instead of preserving multi-session behavior. Confidence: high Scope-risk: moderate Directive: Do not reintroduce process-global chat/task pending state without session keys and multi-session regression tests. Tested: cd desktop && bun run test -- src/stores/chatStore.test.ts src/stores/cliTaskStore.test.ts Tested: bun test src/server/__tests__/websocket-handler.test.ts Tested: cd desktop && bun run lint Tested: bun run check:desktop Tested: bun run check:server Tested: bun run check:coverage | changed-lines 100%, fails only existing agent-utils baseline Tested: bun run verify | 8 passed, 1 failed on existing agent-utils coverage baseline, 2 skipped Not-tested: Real desktop dual-window manual smoke. Related: https://github.com/NanmiCoder/cc-haha/issues/302 Related: https://github.com/NanmiCoder/cc-haha/issues/303
This commit is contained in:
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
@@ -106,4 +106,78 @@ describe('cliTaskStore', () => {
|
||||
|
||||
expect(useCLITaskStore.getState().resetting).toBe(false)
|
||||
})
|
||||
|
||||
it('refreshes tasks for the currently tracked session by default', async () => {
|
||||
vi.mocked(cliTasksApi.getTasksForList).mockResolvedValue({
|
||||
tasks: [makeTask('session-1', 'in_progress')],
|
||||
})
|
||||
|
||||
useCLITaskStore.setState({
|
||||
sessionId: 'session-1',
|
||||
tasks: [],
|
||||
expanded: false,
|
||||
completedAndDismissed: false,
|
||||
dismissedCompletionKey: null,
|
||||
})
|
||||
|
||||
await useCLITaskStore.getState().refreshTasks()
|
||||
|
||||
expect(cliTasksApi.getTasksForList).toHaveBeenCalledWith('session-1')
|
||||
expect(useCLITaskStore.getState().tasks).toMatchObject([
|
||||
{ taskListId: 'session-1', status: 'in_progress' },
|
||||
])
|
||||
})
|
||||
|
||||
it('marks completed tasks dismissed for the currently tracked session by default', () => {
|
||||
useCLITaskStore.setState({
|
||||
sessionId: 'session-1',
|
||||
tasks: [makeTask('session-1', 'completed')],
|
||||
expanded: true,
|
||||
completedAndDismissed: false,
|
||||
dismissedCompletionKey: null,
|
||||
})
|
||||
|
||||
useCLITaskStore.getState().markCompletedAndDismissed()
|
||||
|
||||
expect(useCLITaskStore.getState()).toMatchObject({
|
||||
completedAndDismissed: true,
|
||||
dismissedCompletionKey: 'session-1::1::Keep current session isolated::completed::::',
|
||||
expanded: false,
|
||||
})
|
||||
})
|
||||
|
||||
it('ignores TodoWrite updates for a session that is not currently tracked', () => {
|
||||
useCLITaskStore.setState({
|
||||
sessionId: 'session-1',
|
||||
tasks: [makeTask('session-1', 'in_progress')],
|
||||
expanded: true,
|
||||
completedAndDismissed: false,
|
||||
dismissedCompletionKey: null,
|
||||
})
|
||||
|
||||
useCLITaskStore.getState().setTasksFromTodos([
|
||||
{ content: 'Session 2 task', status: 'completed' },
|
||||
], 'session-2')
|
||||
|
||||
expect(useCLITaskStore.getState().tasks).toMatchObject([
|
||||
{ taskListId: 'session-1', subject: 'Keep current session isolated' },
|
||||
])
|
||||
})
|
||||
|
||||
it('does not reset completed tasks for a different session', async () => {
|
||||
useCLITaskStore.setState({
|
||||
sessionId: 'session-1',
|
||||
tasks: [makeTask('session-1', 'completed')],
|
||||
expanded: true,
|
||||
completedAndDismissed: false,
|
||||
dismissedCompletionKey: null,
|
||||
})
|
||||
|
||||
await useCLITaskStore.getState().resetCompletedTasks('session-2')
|
||||
|
||||
expect(vi.mocked(cliTasksApi.resetTaskList)).not.toHaveBeenCalled()
|
||||
expect(useCLITaskStore.getState().tasks).toMatchObject([
|
||||
{ taskListId: 'session-1', status: 'completed' },
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -25,16 +25,16 @@ type CLITaskStore = {
|
||||
|
||||
/** Fetch tasks for a given session (uses sessionId as taskListId) */
|
||||
fetchSessionTasks: (sessionId: string) => Promise<void>
|
||||
/** Refresh tasks for the currently tracked session */
|
||||
refreshTasks: () => Promise<void>
|
||||
/** Refresh tasks for the currently tracked session, or a specific session if provided */
|
||||
refreshTasks: (sessionId?: string) => Promise<void>
|
||||
/** Update tasks from TodoWrite V1 tool input (in-memory, no disk read needed) */
|
||||
setTasksFromTodos: (todos: TodoItem[]) => void
|
||||
setTasksFromTodos: (todos: TodoItem[], sessionId?: string) => void
|
||||
/** Mark that completed tasks were already dismissed (conversation continued) */
|
||||
markCompletedAndDismissed: () => void
|
||||
markCompletedAndDismissed: (sessionId?: string) => void
|
||||
/** Clear a completed task list locally and remotely so the next cycle starts clean */
|
||||
resetCompletedTasks: () => Promise<void>
|
||||
resetCompletedTasks: (sessionId?: string) => Promise<void>
|
||||
/** Clear task tracking state */
|
||||
clearTasks: () => void
|
||||
clearTasks: (sessionId?: string) => void
|
||||
/** Toggle expanded state */
|
||||
toggleExpanded: () => void
|
||||
}
|
||||
@@ -116,8 +116,8 @@ export const useCLITaskStore = create<CLITaskStore>((set, get) => ({
|
||||
}
|
||||
},
|
||||
|
||||
refreshTasks: async () => {
|
||||
const { sessionId } = get()
|
||||
refreshTasks: async (targetSessionId) => {
|
||||
const sessionId = targetSessionId ?? get().sessionId
|
||||
if (!sessionId) return
|
||||
try {
|
||||
const { tasks } = await cliTasksApi.getTasksForList(sessionId)
|
||||
@@ -132,15 +132,19 @@ export const useCLITaskStore = create<CLITaskStore>((set, get) => ({
|
||||
}
|
||||
},
|
||||
|
||||
setTasksFromTodos: (todos) => {
|
||||
const tasks = mapTodosToTasks(todos, get().sessionId)
|
||||
setTasksFromTodos: (todos, targetSessionId) => {
|
||||
const sessionId = targetSessionId ?? get().sessionId
|
||||
if (!sessionId || get().sessionId !== sessionId) return
|
||||
const tasks = mapTodosToTasks(todos, sessionId)
|
||||
set((state) => ({
|
||||
tasks,
|
||||
...resolveDismissState(tasks, state.dismissedCompletionKey),
|
||||
}))
|
||||
},
|
||||
|
||||
markCompletedAndDismissed: () => {
|
||||
markCompletedAndDismissed: (targetSessionId) => {
|
||||
const sessionId = targetSessionId ?? get().sessionId
|
||||
if (!sessionId || get().sessionId !== sessionId) return
|
||||
const completionKey = buildCompletedTaskKey(get().tasks)
|
||||
if (!completionKey) return
|
||||
|
||||
@@ -151,10 +155,12 @@ export const useCLITaskStore = create<CLITaskStore>((set, get) => ({
|
||||
})
|
||||
},
|
||||
|
||||
resetCompletedTasks: async () => {
|
||||
const { sessionId, tasks } = get()
|
||||
resetCompletedTasks: async (targetSessionId) => {
|
||||
const sessionId = targetSessionId ?? get().sessionId
|
||||
if (!sessionId || get().sessionId !== sessionId) return
|
||||
const { tasks } = get()
|
||||
const completionKey = buildCompletedTaskKey(tasks)
|
||||
if (!sessionId || !completionKey) return
|
||||
if (!completionKey) return
|
||||
|
||||
set({
|
||||
tasks: [],
|
||||
@@ -173,7 +179,8 @@ export const useCLITaskStore = create<CLITaskStore>((set, get) => ({
|
||||
}
|
||||
},
|
||||
|
||||
clearTasks: () => {
|
||||
clearTasks: (targetSessionId) => {
|
||||
if (targetSessionId && get().sessionId !== targetSessionId) return
|
||||
set({
|
||||
sessionId: null,
|
||||
tasks: [],
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
import { afterEach, describe, expect, it, mock, spyOn } from 'bun:test'
|
||||
import type { ServerWebSocket } from 'bun'
|
||||
import {
|
||||
__resetWebSocketHandlerStateForTests,
|
||||
getActiveSessionIds,
|
||||
handleWebSocket,
|
||||
type WebSocketData,
|
||||
} from '../ws/handler.js'
|
||||
import { conversationService } from '../services/conversationService.js'
|
||||
import { computerUseApprovalService } from '../services/computerUseApprovalService.js'
|
||||
|
||||
function makeClientSocket(sessionId: string) {
|
||||
const sent: string[] = []
|
||||
return {
|
||||
data: {
|
||||
sessionId,
|
||||
connectedAt: Date.now(),
|
||||
channel: 'client',
|
||||
sdkToken: null,
|
||||
serverPort: 0,
|
||||
serverHost: '127.0.0.1',
|
||||
},
|
||||
send: mock((payload: string) => {
|
||||
sent.push(payload)
|
||||
}),
|
||||
close: mock(() => {}),
|
||||
sent,
|
||||
} as unknown as ServerWebSocket<WebSocketData> & { sent: string[] }
|
||||
}
|
||||
|
||||
describe('WebSocket handler session isolation', () => {
|
||||
afterEach(() => {
|
||||
__resetWebSocketHandlerStateForTests()
|
||||
mock.restore()
|
||||
})
|
||||
|
||||
it('ignores stale disconnects from an older socket for the same session', () => {
|
||||
const sessionId = `duplicate-${crypto.randomUUID()}`
|
||||
const first = makeClientSocket(sessionId)
|
||||
const second = makeClientSocket(sessionId)
|
||||
const clearCallbacks = spyOn(conversationService, 'clearOutputCallbacks')
|
||||
const cancelComputerUse = spyOn(computerUseApprovalService, 'cancelSession')
|
||||
|
||||
handleWebSocket.open(first)
|
||||
handleWebSocket.open(second)
|
||||
clearCallbacks.mockClear()
|
||||
cancelComputerUse.mockClear()
|
||||
|
||||
handleWebSocket.close(first, 1000, 'stale tab closed')
|
||||
|
||||
expect(getActiveSessionIds()).toContain(sessionId)
|
||||
expect(clearCallbacks).not.toHaveBeenCalled()
|
||||
expect(cancelComputerUse).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -193,6 +193,10 @@ export const handleWebSocket = {
|
||||
}
|
||||
|
||||
console.log(`[WS] Client disconnected from session: ${sessionId} (${code}: ${reason})`)
|
||||
if (activeSessions.get(sessionId) !== ws) {
|
||||
console.log(`[WS] Ignoring stale client disconnect for session: ${sessionId}`)
|
||||
return
|
||||
}
|
||||
computerUseApprovalService.cancelSession(sessionId)
|
||||
activeSessions.delete(sessionId)
|
||||
conversationService.clearOutputCallbacks(sessionId)
|
||||
@@ -1498,3 +1502,11 @@ export function sendToSession(sessionId: string, message: ServerMessage): boolea
|
||||
export function getActiveSessionIds(): string[] {
|
||||
return Array.from(activeSessions.keys())
|
||||
}
|
||||
|
||||
export function __resetWebSocketHandlerStateForTests(): void {
|
||||
for (const timer of sessionCleanupTimers.values()) clearTimeout(timer)
|
||||
for (const timer of prewarmIdleTimers.values()) clearTimeout(timer)
|
||||
activeSessions.clear()
|
||||
sessionCleanupTimers.clear()
|
||||
prewarmIdleTimers.clear()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user