mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-18 13:23:33 +08:00
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.
This commit is contained in:
parent
d116067d9c
commit
d327b7fb30
@ -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(
|
||||
<AskUserQuestion
|
||||
toolUseId="tool-1"
|
||||
input={{
|
||||
questions: [
|
||||
{
|
||||
question: 'Which scope?',
|
||||
options: [{ label: 'Single page' }, { label: 'Tabs' }],
|
||||
},
|
||||
],
|
||||
}}
|
||||
result="Tool permission request failed: AbortError"
|
||||
/>,
|
||||
)
|
||||
|
||||
expect(screen.queryByPlaceholderText('Type your answer...')).toBeNull()
|
||||
expect(screen.queryByRole('button', { name: /submit/i })).toBeNull()
|
||||
expect(screen.getByText(/Tool permission request failed: AbortError/)).toBeTruthy()
|
||||
})
|
||||
})
|
||||
|
||||
@ -89,21 +89,26 @@ export function AskUserQuestion({ sessionId, toolUseId, input, result }: Props)
|
||||
? answers as Record<string, string>
|
||||
: {}
|
||||
}, [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)
|
||||
</span>
|
||||
{submitted && (
|
||||
<span className="ml-2 inline-flex items-center px-2 py-0.5 rounded-full text-[10px] font-bold uppercase tracking-wider bg-[var(--color-surface-container-high)] text-[var(--color-text-tertiary)]">
|
||||
{t('question.answered')}
|
||||
{t(terminalWithoutAnswers ? 'question.completed' : 'question.answered')}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
@ -345,7 +350,7 @@ export function AskUserQuestion({ sessionId, toolUseId, input, result }: Props)
|
||||
<div className="flex items-center gap-2 text-xs text-[var(--color-text-secondary)]">
|
||||
<span className="material-symbols-outlined text-[14px] text-[var(--color-success)]">check_circle</span>
|
||||
<span>
|
||||
{t('question.answeredPrefix')}<strong>{answeredText}</strong>
|
||||
{t(terminalWithoutAnswers ? 'question.resultPrefix' : 'question.answeredPrefix')}<strong>{answeredText}</strong>
|
||||
</span>
|
||||
</div>
|
||||
)}
|
||||
|
||||
@ -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',
|
||||
|
||||
@ -1223,10 +1223,12 @@ export const zh: Record<TranslationKey, string> = {
|
||||
// ─── Ask User Question ──────────────────────────────────────
|
||||
'question.needsInput': 'Claude 需要你的输入',
|
||||
'question.answered': '已回答',
|
||||
'question.completed': '已结束',
|
||||
'question.customResponse': '或输入自定义回复:',
|
||||
'question.typePlaceholder': '输入你的回答...',
|
||||
'question.submit': '提交',
|
||||
'question.answeredPrefix': '已回答: ',
|
||||
'question.resultPrefix': '结果: ',
|
||||
|
||||
// ─── Thinking Block ──────────────────────────────────────
|
||||
'thinking.label': '思考中',
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@ -67,12 +67,22 @@ type SessionProcess = {
|
||||
string,
|
||||
{
|
||||
toolName: string
|
||||
toolUseId?: string
|
||||
description?: string
|
||||
input: Record<string, unknown>
|
||||
permissionSuggestions?: unknown[]
|
||||
}
|
||||
>
|
||||
}
|
||||
|
||||
export type PendingPermissionRequest = {
|
||||
requestId: string
|
||||
toolName: string
|
||||
toolUseId?: string
|
||||
input: Record<string, unknown>
|
||||
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<string, unknown>)
|
||||
: {},
|
||||
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)
|
||||
}
|
||||
|
||||
@ -44,8 +44,10 @@ const sessionSlashCommands = new Map<string, SessionSlashCommand[]>()
|
||||
|
||||
/**
|
||||
* 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<string, ReturnType<typeof setTimeout>>()
|
||||
|
||||
/**
|
||||
@ -152,6 +154,7 @@ export const handleWebSocket = {
|
||||
|
||||
const msg: ServerMessage = { type: 'connected', sessionId }
|
||||
ws.send(JSON.stringify(msg))
|
||||
replayPendingPermissionRequests(ws, sessionId)
|
||||
},
|
||||
|
||||
message(ws: ServerWebSocket<WebSocketData>, 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<WebSocketData>, 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<WebSocketData>,
|
||||
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<typeof parseSlashCommand> {
|
||||
const parsed = parseSlashCommand(content.trim())
|
||||
if (!parsed || parsed.isMcp) return null
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user