From 30cadb1345517bd0ce45151d474b72922687f1a9 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sat, 25 Apr 2026 10:50:54 -0700 Subject: [PATCH] fix(dashboard): show planning thinking output and silently recover from stream errors Two related planning-mode issues: 1. The streaming "thinking" panel only showed for models that emit explicit thinking_delta events (e.g. Anthropic Extended Thinking). For every other model the user saw a spinner with no streaming content because text_delta was being accumulated server-side without being broadcast over SSE. Now onText also forwards deltas through the same stream channel so any model surfaces its in-flight output. 2. Returning to the planning screen after the browser tab was backgrounded long enough for the SSE socket to time out would land the user in a permanent error view ("Session failed while contacting the AI") even though the server session was still alive. The onError handler now first re-fetches the AI session row; if the server still reports the session as generating or awaiting_input it silently reconnects without surfacing the transient error. Only genuine server-side failures still surface. Tests for the obsolete manual-retry recovery path were rewritten as auto-recovery assertions. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../app/components/PlanningModeModal.tsx | 72 ++++++++++++------- .../__tests__/PlanningModeModal.test.tsx | 33 +++------ packages/dashboard/src/planning.ts | 9 ++- 3 files changed, 65 insertions(+), 49 deletions(-) diff --git a/packages/dashboard/app/components/PlanningModeModal.tsx b/packages/dashboard/app/components/PlanningModeModal.tsx index 38331c9e4..39f5ec61f 100644 --- a/packages/dashboard/app/components/PlanningModeModal.tsx +++ b/packages/dashboard/app/components/PlanningModeModal.tsx @@ -246,32 +246,54 @@ export function PlanningModeModal({ isOpen, onClose, onTaskCreated, onTasksCreat }, onError: (message) => { const errorMessage = message || "Session failed while contacting the AI."; - setIsReconnecting(false); - setIsRetrying(false); - setError(null); - setView((prev) => { - if (prev.type === "question" || prev.type === "summary" || prev.type === "error") { - return { type: "error", session: prev.session, errorMessage }; - } - return { - type: "error", - session: { sessionId, currentQuestion: null, summary: null }, - errorMessage, - }; - }); - setStreamingOutput(""); - currentSessionIdRef.current = sessionId; - broadcastUpdate({ - sessionId, - status: "error", - needsInput: false, - owningTabId: sessionTabId, - type: "planning", - title: initialPlan.trim() || "Planning session", - projectId: projectId ?? null, - }); - broadcastCompleted({ sessionId, status: "error" }); + // A single transient stream error (e.g. tab was backgrounded long + // enough for the SSE to time out) should not bounce the user to a + // permanent error view. Refetch the session state — if the server + // still has it in a recoverable state, silently reconnect; only + // surface the error if the server actually persisted one. + setIsReconnecting(true); + (async () => { + try { + const session = await fetchAiSession(sessionId); + if ( + session && + (session.status === "generating" || session.status === "awaiting_input") + ) { + connectToPlanningStream(sessionId); + return; + } + } catch { + // fall through to error view below + } + + setIsReconnecting(false); + setIsRetrying(false); + setError(null); + setView((prev) => { + if (prev.type === "question" || prev.type === "summary" || prev.type === "error") { + return { type: "error", session: prev.session, errorMessage }; + } + return { + type: "error", + session: { sessionId, currentQuestion: null, summary: null }, + errorMessage, + }; + }); + setStreamingOutput(""); + currentSessionIdRef.current = sessionId; + + broadcastUpdate({ + sessionId, + status: "error", + needsInput: false, + owningTabId: sessionTabId, + type: "planning", + title: initialPlan.trim() || "Planning session", + projectId: projectId ?? null, + }); + broadcastCompleted({ sessionId, status: "error" }); + })(); }, onComplete: () => { setIsReconnecting(false); diff --git a/packages/dashboard/app/components/__tests__/PlanningModeModal.test.tsx b/packages/dashboard/app/components/__tests__/PlanningModeModal.test.tsx index ecbc59733..665471e20 100644 --- a/packages/dashboard/app/components/__tests__/PlanningModeModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/PlanningModeModal.test.tsx @@ -640,7 +640,7 @@ describe("PlanningModeModal", () => { expect(mockConnectPlanningStream).toHaveBeenCalledTimes(2); }); - it("recovers retry from connection-loss when server session is still generating", async () => { + it("auto-recovers from a stream error when server session is still generating", async () => { let streamAttempt = 0; mockConnectPlanningStream.mockImplementation((_sessionId: string, _projectId: string | undefined, handlers: any) => { streamAttempt += 1; @@ -654,7 +654,6 @@ describe("PlanningModeModal", () => { }; }); - mockRetryPlanningSession.mockRejectedValueOnce(new Error("Planning session session-123 is not in an error state")); mockFetchAiSession.mockResolvedValueOnce({ id: "session-123", type: "planning", @@ -688,22 +687,16 @@ describe("PlanningModeModal", () => { }); fireEvent.click(screen.getByText("Start Planning")); + // No manual retry button — onError silently re-fetches the session, + // sees status="generating", and reconnects without surfacing the error. await waitFor(() => { - expect(screen.getByText("Connection lost")).toBeDefined(); - }); - - fireEvent.click(screen.getByRole("button", { name: "Retry" })); - - await waitFor(() => { - expect(mockRetryPlanningSession).toHaveBeenCalledWith("session-123", undefined, expect.any(String)); expect(mockFetchAiSession).toHaveBeenCalledWith("session-123"); + expect(mockConnectPlanningStream).toHaveBeenCalledTimes(2); }); - expect(await screen.findByText("AI is thinking...")).toBeDefined(); - expect(screen.getByText("Still thinking...")).toBeDefined(); - expect(mockConnectPlanningStream).toHaveBeenCalledTimes(2); + expect(screen.queryByText("Connection lost")).toBeNull(); }); - it("recovers retry from connection-loss when server session is awaiting input", async () => { + it("auto-recovers from a stream error when server session is awaiting input", async () => { let streamAttempt = 0; mockConnectPlanningStream.mockImplementation((_sessionId: string, _projectId: string | undefined, handlers: any) => { streamAttempt += 1; @@ -717,7 +710,6 @@ describe("PlanningModeModal", () => { }; }); - mockRetryPlanningSession.mockRejectedValueOnce(new Error("Planning session session-123 is not in an error state")); mockFetchAiSession.mockResolvedValueOnce({ id: "session-123", type: "planning", @@ -751,18 +743,13 @@ describe("PlanningModeModal", () => { }); fireEvent.click(screen.getByText("Start Planning")); + // Silent recovery: onError re-fetches the session, sees status= + // "awaiting_input", and reconnects without surfacing the error. await waitFor(() => { - expect(screen.getByText("Connection lost")).toBeDefined(); - }); - - fireEvent.click(screen.getByRole("button", { name: "Retry" })); - - await waitFor(() => { - expect(mockRetryPlanningSession).toHaveBeenCalledWith("session-123", undefined, expect.any(String)); expect(mockFetchAiSession).toHaveBeenCalledWith("session-123"); + expect(mockConnectPlanningStream).toHaveBeenCalledTimes(2); }); - expect(await screen.findByText("What is the scope?")).toBeDefined(); - expect(mockConnectPlanningStream).toHaveBeenCalledTimes(2); + expect(screen.queryByText("Connection lost")).toBeNull(); }); }); diff --git a/packages/dashboard/src/planning.ts b/packages/dashboard/src/planning.ts index 410ffe92d..af987582f 100644 --- a/packages/dashboard/src/planning.ts +++ b/packages/dashboard/src/planning.ts @@ -922,8 +922,15 @@ async function createPlanningAgent( }); }, onText: (delta: string) => { - // Capture AI response text - will be parsed at end of turn + // Capture AI response text — will be parsed at end of turn. Also + // surface it through the same stream so non-thinking models (which + // never emit thinking_delta) still show streaming output in the UI. session.thinkingOutput += delta; + persistThinking(session.id, session.thinkingOutput); + planningStreamManager.broadcast(session.id, { + type: "thinking", + data: delta, + }); }, }); }