From d327b7fb30bed3f25391962a79d8e7354956e4df 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: Sat, 23 May 2026 19:06:59 +0800 Subject: [PATCH] fix: preserve pending user questions across reconnects Desktop AskUserQuestion waits on the SDK permission bridge, and a transient renderer disconnect could previously let the short no-client cleanup kill the CLI. That turned a live user question into an AbortError and left history restore showing a stale interactive card. Track live SDK permission requests, replay them to reconnecting clients, and give sessions waiting on user input a longer cleanup grace. Render aborted terminal results as completed history instead of an unanswered prompt. Constraint: SDK permission requests are live in-memory control messages until answered or aborted. Rejected: Only de-dupe history cards in MessageList | would not keep the CLI alive or restore the active permission request. Rejected: Disable disconnect cleanup globally | would leak abandoned CLI processes for ordinary disconnected sessions. Confidence: high Scope-risk: moderate Reversibility: clean Directive: Do not shorten pending-permission cleanup without testing AskUserQuestion reconnect and abort paths. Tested: bun run check:desktop Tested: bun run check:server Tested: git diff --check Not-tested: Live Sub2API provider reconnect repro. --- .../components/chat/AskUserQuestion.test.tsx | 32 +++++++++ .../src/components/chat/AskUserQuestion.tsx | 15 +++-- desktop/src/i18n/locales/en.ts | 2 + desktop/src/i18n/locales/zh.ts | 2 + src/server/__tests__/conversations.test.ts | 56 ++++++++++++++++ .../__tests__/websocket-handler.test.ts | 66 +++++++++++++++++++ src/server/services/conversationService.ts | 43 ++++++++++++ src/server/ws/handler.ts | 36 ++++++++-- 8 files changed, 242 insertions(+), 10 deletions(-) diff --git a/desktop/src/components/chat/AskUserQuestion.test.tsx b/desktop/src/components/chat/AskUserQuestion.test.tsx index 4e5b030d..9428c364 100644 --- a/desktop/src/components/chat/AskUserQuestion.test.tsx +++ b/desktop/src/components/chat/AskUserQuestion.test.tsx @@ -363,4 +363,36 @@ describe('AskUserQuestion', () => { }, }) }) + + it('renders aborted permission results as terminal instead of asking again', () => { + useChatStore.setState((state) => ({ + sessions: { + ...state.sessions, + [ACTIVE_TAB]: { + ...state.sessions[ACTIVE_TAB]!, + pendingPermission: null, + chatState: 'idle', + }, + }, + })) + + render( + , + ) + + expect(screen.queryByPlaceholderText('Type your answer...')).toBeNull() + expect(screen.queryByRole('button', { name: /submit/i })).toBeNull() + expect(screen.getByText(/Tool permission request failed: AbortError/)).toBeTruthy() + }) }) diff --git a/desktop/src/components/chat/AskUserQuestion.tsx b/desktop/src/components/chat/AskUserQuestion.tsx index e7acbcc9..4ec8d6a5 100644 --- a/desktop/src/components/chat/AskUserQuestion.tsx +++ b/desktop/src/components/chat/AskUserQuestion.tsx @@ -89,21 +89,26 @@ export function AskUserQuestion({ sessionId, toolUseId, input, result }: Props) ? answers as Record : {} }, [result]) + const resultText = typeof result === 'string' && result.trim().length > 0 ? result.trim() : '' + const hasStructuredAnswers = Object.keys(resultAnswers).length > 0 + const hasTerminalResult = hasStructuredAnswers || resultText.length > 0 const pendingRequest = pendingPermission?.toolUseId === toolUseId ? pendingPermission : null const answeredText = useMemo(() => { - if (Object.keys(resultAnswers).length > 0) { + if (hasStructuredAnswers) { return questions .map((question) => resultAnswers[question.question]) .filter((answer): answer is string => typeof answer === 'string' && answer.trim().length > 0) .join(', ') } + if (resultText) return resultText return questions .map((question, index) => freeTexts[index]?.trim() || getSelectedAnswer(question, selections[index])) .filter(Boolean) .join('; ') - }, [freeTexts, questions, resultAnswers, selections]) - const submitted = Object.keys(resultAnswers).length > 0 || hasSubmitted + }, [freeTexts, hasStructuredAnswers, questions, resultAnswers, resultText, selections]) + const submitted = hasTerminalResult || hasSubmitted + const terminalWithoutAnswers = submitted && !hasStructuredAnswers && resultText.length > 0 const handleSelect = (qIndex: number, label: string) => { if (submitted) return @@ -221,7 +226,7 @@ export function AskUserQuestion({ sessionId, toolUseId, input, result }: Props) {submitted && ( - {t('question.answered')} + {t(terminalWithoutAnswers ? 'question.completed' : 'question.answered')} )} @@ -345,7 +350,7 @@ export function AskUserQuestion({ sessionId, toolUseId, input, result }: Props)
check_circle - {t('question.answeredPrefix')}{answeredText} + {t(terminalWithoutAnswers ? 'question.resultPrefix' : 'question.answeredPrefix')}{answeredText}
)} diff --git a/desktop/src/i18n/locales/en.ts b/desktop/src/i18n/locales/en.ts index f82b7729..ad297eff 100644 --- a/desktop/src/i18n/locales/en.ts +++ b/desktop/src/i18n/locales/en.ts @@ -1221,10 +1221,12 @@ export const en = { // ─── Ask User Question ────────────────────────────────────── 'question.needsInput': 'Claude needs your input', 'question.answered': 'Answered', + 'question.completed': 'Completed', 'question.customResponse': 'Or type a custom response:', 'question.typePlaceholder': 'Type your answer...', 'question.submit': 'Submit', 'question.answeredPrefix': 'Answered: ', + 'question.resultPrefix': 'Result: ', // ─── Thinking Block ────────────────────────────────────── 'thinking.label': 'Thinking', diff --git a/desktop/src/i18n/locales/zh.ts b/desktop/src/i18n/locales/zh.ts index b959f71e..54f1744f 100644 --- a/desktop/src/i18n/locales/zh.ts +++ b/desktop/src/i18n/locales/zh.ts @@ -1223,10 +1223,12 @@ export const zh: Record = { // ─── Ask User Question ────────────────────────────────────── 'question.needsInput': 'Claude 需要你的输入', 'question.answered': '已回答', + 'question.completed': '已结束', 'question.customResponse': '或输入自定义回复:', 'question.typePlaceholder': '输入你的回答...', 'question.submit': '提交', 'question.answeredPrefix': '已回答: ', + 'question.resultPrefix': '结果: ', // ─── Thinking Block ────────────────────────────────────── 'thinking.label': '思考中', diff --git a/src/server/__tests__/conversations.test.ts b/src/server/__tests__/conversations.test.ts index 780a818f..4965d061 100644 --- a/src/server/__tests__/conversations.test.ts +++ b/src/server/__tests__/conversations.test.ts @@ -351,6 +351,62 @@ describe('ConversationService', () => { }) }) + it('should expose live SDK permission requests for reconnecting clients', () => { + const svc = new ConversationService() + + ;(svc as any).sessions.set('session-pending-permission', { + proc: { pid: 1 }, + outputCallbacks: [], + workDir: process.cwd(), + permissionMode: 'default', + sdkToken: 'token', + sdkSocket: null, + pendingOutbound: [], + stderrLines: [], + sdkMessages: [], + initMessage: null, + pendingPermissionRequests: new Map(), + }) + + ;(svc as any).handleSdkPayload('session-pending-permission', JSON.stringify({ + type: 'control_request', + request_id: 'request-ask-1', + request: { + subtype: 'can_use_tool', + tool_name: 'AskUserQuestion', + tool_use_id: 'tool-ask-1', + input: { + questions: [ + { + header: 'Scope', + question: 'Which scope?', + options: [{ label: 'A', description: 'First' }, { label: 'B', description: 'Second' }], + }, + ], + }, + description: 'Answer questions?', + }, + })) + + expect(svc.getPendingPermissionRequests('session-pending-permission')).toEqual([ + { + requestId: 'request-ask-1', + toolName: 'AskUserQuestion', + toolUseId: 'tool-ask-1', + input: { + questions: [ + { + header: 'Scope', + question: 'Which scope?', + options: [{ label: 'A', description: 'First' }, { label: 'B', description: 'Second' }], + }, + ], + }, + description: 'Answer questions?', + }, + ]) + }) + it('should reconstruct usage and metadata from a persisted transcript', async () => { const previousConfigDir = process.env.CLAUDE_CONFIG_DIR const previousAnthropicApiKey = process.env.ANTHROPIC_API_KEY diff --git a/src/server/__tests__/websocket-handler.test.ts b/src/server/__tests__/websocket-handler.test.ts index d12f4074..4584ec2b 100644 --- a/src/server/__tests__/websocket-handler.test.ts +++ b/src/server/__tests__/websocket-handler.test.ts @@ -69,4 +69,70 @@ describe('WebSocket handler session isolation', () => { expect(clearCallbacks).toHaveBeenCalledWith(sessionId) expect(cancelComputerUse).toHaveBeenCalledWith(sessionId) }) + + it('replays pending permission requests when a client reconnects', () => { + const sessionId = `permission-reconnect-${crypto.randomUUID()}` + const ws = makeClientSocket(sessionId) + spyOn(conversationService, 'hasSession').mockReturnValue(true) + spyOn(conversationService, 'onOutput').mockImplementation(() => {}) + spyOn(conversationService, 'removeOutputCallback').mockImplementation(() => {}) + spyOn(conversationService, 'getPendingPermissionRequests').mockReturnValue([ + { + requestId: 'request-ask-1', + toolName: 'AskUserQuestion', + toolUseId: 'tool-ask-1', + input: { + questions: [ + { + header: 'Scope', + question: 'Which scope?', + options: [{ label: 'A', description: 'First' }, { label: 'B', description: 'Second' }], + }, + ], + }, + description: 'Answer questions?', + }, + ]) + + handleWebSocket.open(ws) + + expect(ws.sent.map((payload) => JSON.parse(payload))).toContainEqual({ + type: 'permission_request', + requestId: 'request-ask-1', + toolName: 'AskUserQuestion', + toolUseId: 'tool-ask-1', + input: { + questions: [ + { + header: 'Scope', + question: 'Which scope?', + options: [{ label: 'A', description: 'First' }, { label: 'B', description: 'Second' }], + }, + ], + }, + description: 'Answer questions?', + }) + }) + + it('keeps disconnected sessions alive longer while user input is pending', () => { + const sessionId = `permission-disconnect-${crypto.randomUUID()}` + const ws = makeClientSocket(sessionId) + const setTimeoutSpy = spyOn(globalThis, 'setTimeout').mockImplementation(() => 0 as any) + spyOn(conversationService, 'getPendingPermissionRequests').mockReturnValue([ + { + requestId: 'request-ask-1', + toolName: 'AskUserQuestion', + toolUseId: 'tool-ask-1', + input: { questions: [] }, + }, + ]) + + handleWebSocket.open(ws) + setTimeoutSpy.mockClear() + + handleWebSocket.close(ws, 1006, 'renderer reconnecting') + + expect(setTimeoutSpy).toHaveBeenCalled() + expect(setTimeoutSpy.mock.calls[0]?.[1]).toBeGreaterThan(30_000) + }) }) diff --git a/src/server/services/conversationService.ts b/src/server/services/conversationService.ts index c1bddd07..7ce4b501 100644 --- a/src/server/services/conversationService.ts +++ b/src/server/services/conversationService.ts @@ -67,12 +67,22 @@ type SessionProcess = { string, { toolName: string + toolUseId?: string + description?: string input: Record permissionSuggestions?: unknown[] } > } +export type PendingPermissionRequest = { + requestId: string + toolName: string + toolUseId?: string + input: Record + description?: string +} + type SessionStartOptions = { permissionMode?: string model?: string @@ -577,6 +587,19 @@ export class ConversationService { return session?.permissionMode || 'default' } + getPendingPermissionRequests(sessionId: string): PendingPermissionRequest[] { + const session = this.sessions.get(sessionId) + if (!session) return [] + + return Array.from(session.pendingPermissionRequests.entries()).map(([requestId, request]) => ({ + requestId, + toolName: request.toolName, + ...(request.toolUseId ? { toolUseId: request.toolUseId } : {}), + input: request.input, + ...(request.description ? { description: request.description } : {}), + })) + } + authorizeSdkConnection( sessionId: string, token: string | null | undefined, @@ -648,15 +671,35 @@ export class ConversationService { typeof msg.request.tool_name === 'string' ? msg.request.tool_name : 'Unknown', + toolUseId: + typeof msg.request.tool_use_id === 'string' && msg.request.tool_use_id.trim() + ? msg.request.tool_use_id + : undefined, input: msg.request.input && typeof msg.request.input === 'object' ? (msg.request.input as Record) : {}, + description: + typeof msg.request.description === 'string' && msg.request.description.trim() + ? msg.request.description + : undefined, permissionSuggestions: Array.isArray(msg.request.permission_suggestions) ? msg.request.permission_suggestions : undefined, }) } + if ( + (msg?.type === 'control_cancel_request' || msg?.type === 'control_response') && + typeof msg.request_id === 'string' + ) { + session.pendingPermissionRequests.delete(msg.request_id) + } + if ( + msg?.type === 'control_response' && + typeof msg.response?.request_id === 'string' + ) { + session.pendingPermissionRequests.delete(msg.response.request_id) + } for (const cb of session.outputCallbacks) { cb(msg) } diff --git a/src/server/ws/handler.ts b/src/server/ws/handler.ts index feadc37f..f77d36df 100644 --- a/src/server/ws/handler.ts +++ b/src/server/ws/handler.ts @@ -44,8 +44,10 @@ const sessionSlashCommands = new Map() /** * Timers for delayed session cleanup after client disconnect. - * If a client reconnects within 5 minutes, the timer is cancelled. + * If a client reconnects before the timer fires, the timer is cancelled. */ +const CLIENT_DISCONNECT_CLEANUP_MS = 30_000 +const PENDING_PERMISSION_DISCONNECT_CLEANUP_MS = 30 * 60_000 const sessionCleanupTimers = new Map>() /** @@ -152,6 +154,7 @@ export const handleWebSocket = { const msg: ServerMessage = { type: 'connected', sessionId } ws.send(JSON.stringify(msg)) + replayPendingPermissionRequests(ws, sessionId) }, message(ws: ServerWebSocket, rawMessage: string | Buffer) { @@ -238,16 +241,17 @@ export const handleWebSocket = { computerUseApprovalService.cancelSession(sessionId) - // Schedule delayed cleanup: if the client doesn't reconnect within 30 seconds, - // stop the CLI subprocess to avoid leaking resources. + // Schedule delayed cleanup. Sessions waiting on user input need a longer + // grace period so transient renderer disconnects do not abort the prompt. + const cleanupDelayMs = getDisconnectCleanupDelayMs(sessionId) const cleanupTimer = setTimeout(() => { sessionCleanupTimers.delete(sessionId) if (!hasActiveClients(sessionId)) { - console.log(`[WS] Session ${sessionId} not reconnected after 30s, stopping CLI subprocess`) + console.log(`[WS] Session ${sessionId} not reconnected after ${cleanupDelayMs}ms, stopping CLI subprocess`) conversationService.stopSession(sessionId) cleanupSessionRuntimeState(sessionId) } - }, 30_000) + }, cleanupDelayMs) sessionCleanupTimers.set(sessionId, cleanupTimer) }, @@ -1480,6 +1484,28 @@ function sendError(ws: ServerWebSocket, message: string, code: st sendMessage(ws, { type: 'error', message, code }) } +function getDisconnectCleanupDelayMs(sessionId: string): number { + return conversationService.getPendingPermissionRequests(sessionId).length > 0 + ? PENDING_PERMISSION_DISCONNECT_CLEANUP_MS + : CLIENT_DISCONNECT_CLEANUP_MS +} + +function replayPendingPermissionRequests( + ws: ServerWebSocket, + sessionId: string, +): void { + for (const request of conversationService.getPendingPermissionRequests(sessionId)) { + sendMessage(ws, { + type: 'permission_request', + requestId: request.requestId, + toolName: request.toolName, + ...(request.toolUseId ? { toolUseId: request.toolUseId } : {}), + input: request.input, + ...(request.description ? { description: request.description } : {}), + }) + } +} + function getDesktopSlashCommand(content: string): ReturnType { const parsed = parseSlashCommand(content.trim()) if (!parsed || parsed.isMcp) return null