From edd55a9b0fdf7d30f766417fb455163ba644de94 Mon Sep 17 00:00:00 2001 From: 7418 <7418@openclaw.ai> Date: Tue, 24 Feb 2026 21:39:09 +0800 Subject: [PATCH] fix: improve session resume resilience, permission timeout, and stream idle handling 1. Resume pre-check and degradation (claude-client.ts): - Check working_directory exists on disk before attempting resume; if missing, skip resume, clear sdk_session_id, notify frontend - On resume peek failure, clear sdk_session_id to prevent repeated retries on the same broken session - Change iterator wrap condition from sdkSessionId to shouldResume so skipped resumes don't enter the peek path 2. Per-request independent permission timeout (permission-registry.ts): - Replace lazy cleanup (cleanupExpired on next register) with a dedicated setTimeout per permission request - Auto-deny after 5 minutes with clearTimeout on resolve/abort - Extract denyAndRemove helper for abort and timeout paths 3. Permission wait visibility (StreamingMessage.tsx): - Always show StreamingStatusBar during streaming (remove !pendingPermission guard) - Display "Waiting for authorization: " with elapsed timer when a permission request is pending, instead of hiding the bar 4. Stream idle timeout (ChatView.tsx): - Add 330s idle timeout (slightly above 5-min permission timeout to avoid races) using setInterval that checks lastEventTime - Every SSE callback calls markActive() to reset the timer - On idle timeout, abort the stream and show a readable error - Clean up interval in both catch and finally blocks Co-Authored-By: Claude Opus 4.6 --- src/components/chat/ChatView.tsx | 127 ++++++++++++++++------- src/components/chat/StreamingMessage.tsx | 8 +- src/lib/claude-client.ts | 41 +++++++- src/lib/permission-registry.ts | 45 ++++---- 4 files changed, 152 insertions(+), 69 deletions(-) diff --git a/src/components/chat/ChatView.tsx b/src/components/chat/ChatView.tsx index 195b0f1..68d32fe 100644 --- a/src/components/chat/ChatView.tsx +++ b/src/components/chat/ChatView.tsx @@ -216,6 +216,20 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal let accumulated = ''; + // Stream idle timeout: abort if no SSE events arrive for this duration. + // Set slightly above the 5-minute permission timeout (300s) to avoid races. + const STREAM_IDLE_TIMEOUT_MS = 330_000; + let lastEventTime = Date.now(); + let isIdleTimeout = false; + const idleCheckTimer = setInterval(() => { + if (Date.now() - lastEventTime >= STREAM_IDLE_TIMEOUT_MS) { + clearInterval(idleCheckTimer); + isIdleTimeout = true; + controller.abort(); + } + }, 10_000); + const markActive = () => { lastEventTime = Date.now(); }; + try { const response = await fetch('/api/chat', { method: 'POST', @@ -240,11 +254,13 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal const result = await consumeSSEStream(reader, { onText: (acc) => { + markActive(); accumulated = acc; accumulatedRef.current = acc; setStreamingContent(acc); }, onToolUse: (tool) => { + markActive(); setStreamingToolOutput(''); setToolUses((prev) => { if (prev.some((t) => t.id === tool.id)) return prev; @@ -254,6 +270,7 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal }); }, onToolResult: (res) => { + markActive(); setStreamingToolOutput(''); setToolResults((prev) => { const next = [...prev, res]; @@ -265,15 +282,18 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal window.dispatchEvent(new Event('refresh-file-tree')); }, onToolOutput: (data) => { + markActive(); setStreamingToolOutput((prev) => { const next = prev + (prev ? '\n' : '') + data; return next.length > 5000 ? next.slice(-5000) : next; }); }, onToolProgress: (toolName, elapsed) => { + markActive(); setStatusText(`Running ${toolName}... (${elapsed}s)`); }, onStatus: (text) => { + markActive(); if (text?.startsWith('Connected (')) { setStatusText(text); setTimeout(() => setStatusText(undefined), 2000); @@ -281,21 +301,28 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal setStatusText(text); } }, - onResult: () => { /* token usage captured by consumeSSEStream */ }, + onResult: () => { + markActive(); + /* token usage captured by consumeSSEStream */ + }, onPermissionRequest: (permData) => { + markActive(); setPendingPermission(permData); setPermissionResolved(null); setPendingApprovalSessionId(sessionId); }, onToolTimeout: (toolName, elapsedSeconds) => { + markActive(); toolTimeoutRef.current = { toolName, elapsedSeconds }; }, onModeChanged: (sdkMode) => { + markActive(); // Map SDK permissionMode to UI mode const uiMode = sdkMode === 'plan' ? 'plan' : 'code'; handleModeChange(uiMode); }, onError: (acc) => { + markActive(); accumulated = acc; accumulatedRef.current = acc; setStreamingContent(acc); @@ -341,56 +368,75 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal setMessages((prev) => [...prev, assistantMessage]); } } catch (error) { + clearInterval(idleCheckTimer); + if (error instanceof DOMException && error.name === 'AbortError') { - const timeoutInfo = toolTimeoutRef.current; - if (timeoutInfo) { - // Tool execution timed out — save partial content and auto-retry + // Stream idle timeout — no SSE events for too long + if (isIdleTimeout) { + const idleSecs = Math.round(STREAM_IDLE_TIMEOUT_MS / 1000); + const errContent = accumulated.trim() + ? accumulated.trim() + `\n\n**Error:** Stream idle timeout — no response for ${idleSecs}s. The connection may have dropped.` + : `**Error:** Stream idle timeout — no response for ${idleSecs}s. The connection may have dropped.`; + const errMessage: Message = { + id: 'temp-error-' + Date.now(), + session_id: sessionId, + role: 'assistant', + content: errContent, + created_at: new Date().toISOString(), + token_usage: null, + }; + setMessages((prev) => [...prev, errMessage]); + } else { + const timeoutInfo = toolTimeoutRef.current; + if (timeoutInfo) { + // Tool execution timed out — save partial content and auto-retry + if (accumulated.trim()) { + const partialMessage: Message = { + id: 'temp-assistant-' + Date.now(), + session_id: sessionId, + role: 'assistant', + content: accumulated.trim() + `\n\n*(tool ${timeoutInfo.toolName} timed out after ${timeoutInfo.elapsedSeconds}s)*`, + created_at: new Date().toISOString(), + token_usage: null, + }; + setMessages((prev) => [...prev, partialMessage]); + } + // Clean up before auto-retry + toolTimeoutRef.current = null; + setIsStreaming(false); + setStreamingSessionId(''); + setStreamingContent(''); + accumulatedRef.current = ''; + toolUsesRef.current = []; + toolResultsRef.current = []; + setToolUses([]); + setToolResults([]); + setStreamingToolOutput(''); + setStatusText(undefined); + setPendingPermission(null); + setPermissionResolved(null); + setPendingApprovalSessionId(''); + abortControllerRef.current = null; + // Auto-retry: send a follow-up message telling the model to adjust strategy + setTimeout(() => { + sendMessageRef.current?.( + `The previous tool "${timeoutInfo.toolName}" timed out after ${timeoutInfo.elapsedSeconds} seconds. Please try a different approach to accomplish the task. Avoid repeating the same operation that got stuck.` + ); + }, 500); + return; // Skip the normal finally cleanup since we did it above + } + // User manually stopped generation — add partial content if (accumulated.trim()) { const partialMessage: Message = { id: 'temp-assistant-' + Date.now(), session_id: sessionId, role: 'assistant', - content: accumulated.trim() + `\n\n*(tool ${timeoutInfo.toolName} timed out after ${timeoutInfo.elapsedSeconds}s)*`, + content: accumulated.trim() + '\n\n*(generation stopped)*', created_at: new Date().toISOString(), token_usage: null, }; setMessages((prev) => [...prev, partialMessage]); } - // Clean up before auto-retry - toolTimeoutRef.current = null; - setIsStreaming(false); - setStreamingSessionId(''); - setStreamingContent(''); - accumulatedRef.current = ''; - toolUsesRef.current = []; - toolResultsRef.current = []; - setToolUses([]); - setToolResults([]); - setStreamingToolOutput(''); - setStatusText(undefined); - setPendingPermission(null); - setPermissionResolved(null); - setPendingApprovalSessionId(''); - abortControllerRef.current = null; - // Auto-retry: send a follow-up message telling the model to adjust strategy - setTimeout(() => { - sendMessageRef.current?.( - `The previous tool "${timeoutInfo.toolName}" timed out after ${timeoutInfo.elapsedSeconds} seconds. Please try a different approach to accomplish the task. Avoid repeating the same operation that got stuck.` - ); - }, 500); - return; // Skip the normal finally cleanup since we did it above - } - // User manually stopped generation — add partial content - if (accumulated.trim()) { - const partialMessage: Message = { - id: 'temp-assistant-' + Date.now(), - session_id: sessionId, - role: 'assistant', - content: accumulated.trim() + '\n\n*(generation stopped)*', - created_at: new Date().toISOString(), - token_usage: null, - }; - setMessages((prev) => [...prev, partialMessage]); } } else { const errMsg = error instanceof Error ? error.message : 'Unknown error'; @@ -405,6 +451,7 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal setMessages((prev) => [...prev, errorMessage]); } } finally { + clearInterval(idleCheckTimer); toolTimeoutRef.current = null; setIsStreaming(false); setStreamingSessionId(''); diff --git a/src/components/chat/StreamingMessage.tsx b/src/components/chat/StreamingMessage.tsx index 60bf06a..f03af23 100644 --- a/src/components/chat/StreamingMessage.tsx +++ b/src/components/chat/StreamingMessage.tsx @@ -460,9 +460,11 @@ export function StreamingMessage({ )} - {/* Status bar during streaming */} - {isStreaming && !pendingPermission && } diff --git a/src/lib/claude-client.ts b/src/lib/claude-client.ts index db77423..a68cff9 100644 --- a/src/lib/claude-client.ts +++ b/src/lib/claude-client.ts @@ -385,8 +385,27 @@ export function streamClaude(options: ClaudeStreamOptions): ReadableStream; } catch (resumeError) { - console.warn('[claude-client] Resume failed, retrying without resume:', resumeError instanceof Error ? resumeError.message : resumeError); + const errMsg = resumeError instanceof Error ? resumeError.message : String(resumeError); + console.warn('[claude-client] Resume failed, retrying without resume:', errMsg); + // Clear stale sdk_session_id so future messages don't retry this broken resume + if (sessionId) { + try { updateSdkSessionId(sessionId, ''); } catch { /* best effort */ } + } + // Notify frontend about the fallback + controller.enqueue(formatSSE({ + type: 'status', + data: JSON.stringify({ + notification: true, + title: 'Session fallback', + message: 'Previous session could not be resumed. Starting fresh conversation.', + }), + })); // Remove resume and try again as a fresh conversation delete queryOptions.resume; conversation = query({ diff --git a/src/lib/permission-registry.ts b/src/lib/permission-registry.ts index a6ae320..96d3d30 100644 --- a/src/lib/permission-registry.ts +++ b/src/lib/permission-registry.ts @@ -4,7 +4,8 @@ interface PendingPermission { resolve: (result: PermissionResult) => void; createdAt: number; abortSignal?: AbortSignal; - toolInput: Record; // Original tool input for updatedInput in allow response + toolInput: Record; + timer: ReturnType; } const TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes @@ -22,50 +23,49 @@ function getMap(): Map { } /** - * Lazily clean up expired entries (older than TIMEOUT_MS). + * Helper to deny and remove a pending permission entry. */ -function cleanupExpired() { +function denyAndRemove(id: string, message: string) { const map = getMap(); - const now = Date.now(); - for (const [id, entry] of map) { - if (now - entry.createdAt > TIMEOUT_MS) { - entry.resolve({ behavior: 'deny', message: 'Permission request timed out' }); - map.delete(id); - } - } + const entry = map.get(id); + if (!entry) return; + clearTimeout(entry.timer); + entry.resolve({ behavior: 'deny', message }); + map.delete(id); } /** * Register a pending permission request. - * Returns a Promise that resolves when the user responds. + * Returns a Promise that resolves when the user responds or after TIMEOUT_MS. */ export function registerPendingPermission( id: string, toolInput: Record, abortSignal?: AbortSignal, ): Promise { - // Lazily clean up expired entries on each registration - cleanupExpired(); - const map = getMap(); return new Promise((resolve) => { + // Per-request independent timer: auto-deny after TIMEOUT_MS + const timer = setTimeout(() => { + if (map.has(id)) { + console.warn(`[permission-registry] Permission request ${id} timed out after ${TIMEOUT_MS / 1000}s`); + resolve({ behavior: 'deny', message: 'Permission request timed out' }); + map.delete(id); + } + }, TIMEOUT_MS); + map.set(id, { resolve, createdAt: Date.now(), abortSignal, toolInput, + timer, }); // Auto-deny if the abort signal fires (client disconnect / stop button) if (abortSignal) { - const onAbort = () => { - if (map.has(id)) { - resolve({ behavior: 'deny', message: 'Request aborted' }); - map.delete(id); - } - }; - abortSignal.addEventListener('abort', onAbort, { once: true }); + abortSignal.addEventListener('abort', () => denyAndRemove(id, 'Request aborted'), { once: true }); } }); } @@ -82,7 +82,8 @@ export function resolvePendingPermission( const entry = map.get(id); if (!entry) return false; - // SDK requires updatedInput when allowing — inject the original tool input + clearTimeout(entry.timer); + if (result.behavior === 'allow' && !result.updatedInput) { result = { ...result, updatedInput: entry.toolInput }; }