From c6b768162260df12c8c724db19fcd4fc0f4fe172 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 16 Oct 2025 13:46:46 -0600 Subject: [PATCH] feat(chat): enhance background status handling and UI updates for terminal states --- src/api/providers/openai-native.ts | 104 ++++++++++++++++---- src/core/task/Task.ts | 39 +++++++- webview-ui/src/components/chat/ChatView.tsx | 56 +++++++---- 3 files changed, 159 insertions(+), 40 deletions(-) diff --git a/src/api/providers/openai-native.ts b/src/api/providers/openai-native.ts index 9f3983d98a..bd0be481c5 100644 --- a/src/api/providers/openai-native.ts +++ b/src/api/providers/openai-native.ts @@ -25,6 +25,22 @@ import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata } from ". export type OpenAiNativeModel = ReturnType +// GPT-5 specific types + +// Constants for model identification +const GPT5_MODEL_PREFIX = "gpt-5" + +// Marker for terminal background-mode failures so we don't attempt resume/poll fallbacks +function createTerminalBackgroundError(message: string): Error { + const err = new Error(message) + ;(err as any).isTerminalBackgroundError = true + err.name = "TerminalBackgroundError" + return err +} +function isTerminalBackgroundError(err: any): boolean { + return !!(err && (err as any).isTerminalBackgroundError) +} + export class OpenAiNativeHandler extends BaseProvider implements SingleCompletionHandler { protected options: ApiHandlerOptions private client: OpenAI @@ -372,6 +388,10 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio } } } catch (iterErr) { + // If terminal failure, propagate and do not attempt resume/poll + if (isTerminalBackgroundError(iterErr)) { + throw iterErr + } // Stream dropped mid-flight; attempt resume for background requests if (canAttemptResume()) { for await (const chunk of this.attemptResumeOrPoll( @@ -386,11 +406,18 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio throw iterErr } } catch (sdkErr: any) { + // Propagate terminal background failures without fallback + if (isTerminalBackgroundError(sdkErr)) { + throw sdkErr + } // For errors, fallback to manual SSE via fetch try { yield* this.makeResponsesApiRequest(requestBody, model, metadata, systemPrompt, messages) } catch (fallbackErr) { // If SSE fallback fails mid-stream and we can resume, try that + if (isTerminalBackgroundError(fallbackErr)) { + throw fallbackErr + } if (canAttemptResume()) { for await (const chunk of this.attemptResumeOrPoll( this.lastResponseId!, @@ -937,9 +964,20 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio else if (parsed.type === "response.error" || parsed.type === "error") { // Error event from the API if (parsed.error || parsed.message) { - throw new Error( - `Responses API error: ${parsed.error?.message || parsed.message || "Unknown error"}`, - ) + const errMsg = `Responses API error: ${parsed.error?.message || parsed.message || "Unknown error"}` + // For background mode, treat as terminal to avoid futile resume attempts + if (this.currentRequestIsBackground) { + // Surface a failed status for UI lifecycle before terminating + yield { + type: "status", + mode: "background", + status: "failed", + ...(parsed.response?.id ? { responseId: parsed.response.id } : {}), + } + throw createTerminalBackgroundError(errMsg) + } + // Non-background: propagate as a standard error + throw new Error(errMsg) } } // Handle incomplete event @@ -975,7 +1013,7 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio } // Response failed if (parsed.error || parsed.message) { - throw new Error( + throw createTerminalBackgroundError( `Response failed: ${parsed.error?.message || parsed.message || "Unknown failure"}`, ) } @@ -1110,6 +1148,10 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio // This can happen in certain edge cases and shouldn't break the flow } catch (error) { if (error instanceof Error) { + // Preserve terminal background errors so callers can avoid resume attempts + if ((error as any).isTerminalBackgroundError) { + throw error + } throw new Error(`Error processing response stream: ${error.message}`) } throw new Error("Unexpected error processing response stream") @@ -1147,25 +1189,25 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio }, }) - if (!res.ok || !res.body) { + if (!res.ok) { throw new Error(`Resume request failed (${res.status})`) } + if (!res.body) { + throw new Error("Resume request failed (no body)") + } this.resumeCutoffSequence = lastSeq - let emittedInProgress = false + // Handshake accepted: immediately switch UI from reconnecting -> in_progress + yield { + type: "status", + mode: "background", + status: "in_progress", + responseId, + } + try { for await (const chunk of this.handleStreamResponse(res.body, model)) { - // After the handshake and first accepted chunk, emit in_progress once - if (!emittedInProgress) { - emittedInProgress = true - yield { - type: "status", - mode: "background", - status: "in_progress", - responseId, - } - } // Avoid double-emitting in_progress if the inner handler surfaces it if (chunk.type === "status" && (chunk as any).status === "in_progress") { continue @@ -1180,9 +1222,13 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio this.resumeCutoffSequence = undefined throw e } - } catch { - // Wait with backoff before next attempt + } catch (err) { + // If terminal error, don't keep retrying resume; fall back to polling immediately const delay = resumeBaseDelayMs * Math.pow(2, attempt) + if (isTerminalBackgroundError(err)) { + break + } + // Otherwise retry with backoff if (delay > 0) { await new Promise((r) => setTimeout(r, delay)) } @@ -1296,10 +1342,21 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio } if (status === "failed" || status === "canceled") { - throw new Error(`Response ${status}: ${respId || responseId}`) + const detail: string | undefined = resp?.error?.message ?? raw?.error?.message + const msg = detail ? `Response ${status}: ${detail}` : `Response ${status}: ${respId || responseId}` + throw createTerminalBackgroundError(msg) } - } catch { - // ignore transient poll errors + } catch (err) { + // If we've already emitted a terminal status, propagate to consumer to stop polling. + if (lastEmittedStatus === "failed" || lastEmittedStatus === "canceled") { + throw err + } + // Otherwise ignore transient poll errors + } + + // Stop polling immediately on terminal background statuses + if (lastEmittedStatus === "failed" || lastEmittedStatus === "canceled") { + throw new Error(`Background polling terminated with status=${lastEmittedStatus} for ${responseId}`) } await new Promise((r) => setTimeout(r, pollIntervalMs)) @@ -1350,6 +1407,11 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio if (mappedStatus === "completed" || mappedStatus === "failed" || mappedStatus === "canceled") { this.currentRequestIsBackground = undefined } + // Throw terminal error to integrate with standard failure path (surfaced in UI) + if (mappedStatus === "failed" || mappedStatus === "canceled") { + const msg = (event as any)?.error?.message || (event as any)?.message || `Response ${mappedStatus}` + throw createTerminalBackgroundError(msg) + } // Do not return; allow further handling (e.g., usage on done/completed) } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 6b8fbeaedb..b2a0da548e 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2437,6 +2437,7 @@ export class Task extends EventEmitter implements TaskLike { if (!chunk) { // Sometimes chunk is undefined, no idea that can cause // it, but this workaround seems to fix it. + item = await iterator.next() continue } @@ -2625,7 +2626,14 @@ export class Task extends EventEmitter implements TaskLike { if (chunk.responseId) { ;(apiReqMsg as any).metadata.responseId = chunk.responseId } + // Temporary debug to confirm UI metadata updates + console.log( + `[BackgroundMode] status update -> ${chunk.status} (resp=${chunk.responseId ?? "n/a"})`, + ) await this.updateClineMessage(apiReqMsg) + // Force state refresh to ensure UI recomputes derived labels/memos + const provider = this.providerRef.deref() + await provider?.postStateToWebview() } } catch {} break @@ -2704,6 +2712,10 @@ export class Task extends EventEmitter implements TaskLike { "\n\n[Response interrupted by a tool use result. Only one tool may be used at a time and should be placed at the end of the message.]" break } + // Prefetch the next item after processing the current chunk. + // This ensures terminal status chunks (e.g., failed/canceled/completed) + // are not skipped when the provider throws on the following next(). + item = await iterator.next() } // Finalize any remaining streaming tool calls that weren't explicitly ended @@ -3186,8 +3198,31 @@ export class Task extends EventEmitter implements TaskLike { continue } else { // If there's no assistant_responses, that means we got no text - // or tool_use content blocks from API which we should assume is - // an error. + // or tool_use content blocks from API which we should assume is an error. + // Prefer any streaming failure details captured on the last api_req_started message. + let errorText = + "Unexpected API Response: The language model did not provide any assistant messages. This may indicate an issue with the API or the model's output." + try { + const lastApiReqStartedIdx = findLastIndex( + this.clineMessages, + (m) => m.type === "say" && m.say === "api_req_started", + ) + if (lastApiReqStartedIdx !== -1) { + const info = JSON.parse( + this.clineMessages[lastApiReqStartedIdx].text || "{}", + ) as ClineApiReqInfo + if ( + typeof info?.streamingFailedMessage === "string" && + info.streamingFailedMessage.trim().length > 0 + ) { + errorText = info.streamingFailedMessage + } + } + } catch { + // ignore parse issues and keep default message + } + + await this.say("error", errorText) // IMPORTANT: For native tool protocol, we already added the user message to // apiConversationHistory at line 1876. Since the assistant failed to respond, diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 6ee163fe41..fd61af6078 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -505,27 +505,49 @@ const ChatViewComponent: React.ForwardRefRenderFunction message.say === "api_req_started", + ) + // Extract background terminal state and cancel reason/cost if present + let bgDone = false + let cancelReason: string | null | undefined = undefined + let cost: any = undefined + + if (lastApiReqStarted && lastApiReqStarted.say === "api_req_started") { + const meta: any = (lastApiReqStarted as any).metadata + const bgStatus = meta?.background === true ? meta?.backgroundStatus : undefined + bgDone = bgStatus === "completed" || bgStatus === "failed" || bgStatus === "canceled" + + try { + if (lastApiReqStarted.text !== null && lastApiReqStarted.text !== undefined) { + const info = JSON.parse(lastApiReqStarted.text) + cost = info?.cost + cancelReason = info?.cancelReason + } + } catch { + // ignore malformed json + } + } + + // If background reached a terminal state or the provider recorded a cancel reason, + // treat UI as not streaming regardless of partial flags or missing cost. + if (bgDone || cancelReason != null) { + return false + } + + // Partial assistant content means streaming unless overridden by the terminal checks above. + const isLastMessagePartial = modifiedMessages.at(-1)?.partial === true if (isLastMessagePartial) { return true - } else { - const lastApiReqStarted = findLast( - modifiedMessages, - (message: ClineMessage) => message.say === "api_req_started", - ) + } - if ( - lastApiReqStarted && - lastApiReqStarted.text !== null && - lastApiReqStarted.text !== undefined && - lastApiReqStarted.say === "api_req_started" - ) { - const cost = JSON.parse(lastApiReqStarted.text).cost - - if (cost === undefined) { - return true // API request has not finished yet. - } + // Otherwise, if the API request hasn't finished (no cost yet), consider it streaming. + if (lastApiReqStarted && lastApiReqStarted.say === "api_req_started") { + if (cost === undefined) { + return true } }