From 754d66e27d110824d4df28a4ba30a24a0a1bc1bb Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 09:54:31 +0000 Subject: [PATCH 01/11] refactor: plan, transcript and process dialogs keep shown content while reloading A new API client after a reconnect re-runs each dialog's read. The dialog now keeps what it shows until the new read lands, ignores reads cut off by the lost connection, and resets only for another workspace, task or process (the viewers are keyed by it). Signed-off-by: Thomas Kosiewski --- .../BackgroundBashOutputDialog.tsx | 33 ++- .../features/RightSidebar/PlanFileDialog.tsx | 125 +++++---- .../RightSidebar/dialogsReconnect.test.tsx | 241 ++++++++++++++++++ .../Tools/SubagentTranscriptDialog.tsx | 30 ++- 4 files changed, 353 insertions(+), 76 deletions(-) create mode 100644 src/browser/features/RightSidebar/dialogsReconnect.test.tsx diff --git a/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx b/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx index 739df965732..a3426635330 100644 --- a/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx +++ b/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx @@ -4,11 +4,13 @@ import { Dialog, DialogContent, DialogHeader, DialogTitle } from "../Dialog/Dial import { DetailContent } from "@/browser/features/Tools/Shared/ToolPrimitives"; import { useAPI } from "@/browser/contexts/API"; import { useBackgroundProcesses } from "@/browser/stores/BackgroundBashStore"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { appendLiveBashOutputChunk, type LiveBashOutputInternal, } from "@/browser/utils/messages/liveBashOutputBuffer"; import { BASH_TRUNCATE_MAX_TOTAL_BYTES } from "@/common/constants/toolLimits"; +import { getErrorMessage } from "@/common/utils/errors"; const BACKGROUND_BASH_INITIAL_TAIL_BYTES = 64_000; const BACKGROUND_BASH_POLL_INTERVAL_MS = 500; @@ -47,7 +49,13 @@ export const BackgroundBashOutputDialog: React.FC - + {/* Keyed by the process, so another process starts from an empty viewer, while a new API + client (a reconnect) only re-runs the viewer's read and keeps the output it shows. */} + ); @@ -71,14 +79,11 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str const [isLoading, setIsLoading] = useState(true); useEffect(() => { - setOutput(undefined); - setStatus("running"); - setTruncatedStart(false); + // No reset of the shown output here: the first read below replaces it. Before the first + // read lands, the viewer still shows "Loading…" (its initial state). setError(null); - setIsLoading(true); if (!api) { - setIsLoading(false); setError("API unavailable"); return; } @@ -109,8 +114,11 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str } setStatus(result.data.status); - if (result.data.truncatedStart) { - setTruncatedStart(true); + // The first read is a fresh tail (the only read that can be truncated at its start), so + // it replaces any output kept from an earlier run (a reconnect); later reads append. + if (offset === undefined) { + setTruncatedStart(result.data.truncatedStart); + setOutput(undefined); } offset = result.data.nextOffset; @@ -133,12 +141,13 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str } }; - // A rejected read (e.g. a lost server connection) stops polling with the error shown instead - // of escaping as an unhandled rejection; reopening the dialog starts a fresh read. + // A rejected read stops polling with the error shown instead of escaping as an unhandled + // rejection; reopening the dialog starts a fresh read. A lost connection (an abort) shows no + // error and keeps the output: the next client polls again. run().catch((err: unknown) => { - if (cancelled) return; + if (cancelled || isAbortError(err)) return; setIsLoading(false); - setError(err instanceof Error ? err.message : String(err)); + setError(getErrorMessage(err)); }); return () => { diff --git a/src/browser/features/RightSidebar/PlanFileDialog.tsx b/src/browser/features/RightSidebar/PlanFileDialog.tsx index 9139e112dff..d9db936287c 100644 --- a/src/browser/features/RightSidebar/PlanFileDialog.tsx +++ b/src/browser/features/RightSidebar/PlanFileDialog.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from "react"; +import React, { useEffect, useRef, useState } from "react"; import assert from "@/common/utils/assert"; import { useAPI } from "@/browser/contexts/API"; import { MarkdownCore } from "@/browser/features/Messages/MarkdownCore"; @@ -9,6 +9,7 @@ import { DialogHeader, DialogTitle, } from "@/browser/components/Dialog/Dialog"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { getErrorMessage } from "@/common/utils/errors"; interface PlanFileDialogProps { @@ -17,31 +18,36 @@ interface PlanFileDialogProps { workspaceId: string; } -export const PlanFileDialog: React.FC = (props) => { +export const PlanFileDialog: React.FC = (props) => ( + + + {/* PostCompactionSection renders this dialog while closed: the body, and so its plan + read, mounts only while the dialog is open. Keyed by workspace, so another workspace + (or reopening the dialog) starts from an empty body, while a new API client (a + reconnect) re-reads the plan in the background and keeps it shown. */} + + + +); + +function usePlanFile(workspaceId: string) { const { api } = useAPI(); - const [isLoading, setIsLoading] = useState(false); + const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); const [content, setContent] = useState(null); const [path, setPath] = useState(null); + // True once a plan is on screen: offline (no client during a reconnect) then keeps it. + const hasPlanRef = useRef(false); useEffect(() => { - // PostCompactionSection renders this dialog while closed. - // Delay IPC plan-file reads until the user explicitly opens the preview. - if (!props.open) { - return; - } - - assert(props.workspaceId.trim().length > 0, "workspaceId is required to load plan preview"); - - setIsLoading(true); - setError(null); - setContent(null); - setPath(null); + assert(workspaceId.trim().length > 0, "workspaceId is required to load plan preview"); if (!api) { - setIsLoading(false); - setError("API unavailable"); + if (!hasPlanRef.current) { + setIsLoading(false); + setError("API unavailable"); + } return; } @@ -49,7 +55,7 @@ export const PlanFileDialog: React.FC = (props) => { const run = async () => { try { - const result = await api.workspace.getPlanContent({ workspaceId: props.workspaceId }); + const result = await api.workspace.getPlanContent({ workspaceId }); if (cancelled) { return; @@ -63,11 +69,15 @@ export const PlanFileDialog: React.FC = (props) => { assert(result.data.path.length > 0, "Plan path should be non-empty"); + setError(null); setContent(result.data.content); setPath(result.data.path); setIsLoading(false); + hasPlanRef.current = true; } catch (error: unknown) { - if (cancelled) { + // A lost connection (or this effect's own cleanup) is not a plan error: keep what is + // shown for the next client. + if (cancelled || isAbortError(error)) { return; } @@ -81,42 +91,49 @@ export const PlanFileDialog: React.FC = (props) => { return () => { cancelled = true; }; - }, [api, props.open, props.workspaceId]); + }, [api, workspaceId]); + + return { isLoading, error, content, path }; +} + +function PlanFileBody(props: { workspaceId: string }) { + const { isLoading, error, content, path } = usePlanFile(props.workspaceId); return ( - - - - - Plan file - {path && ( - - {path} - - )} - - - -
- {error ? ( -
- {error} -
- ) : isLoading ? ( -
Loading plan…
- ) : content !== null ? ( - content.length > 0 ? ( - - - - ) : ( -
Plan file is empty
- ) - ) : ( -
No plan loaded
+ <> + + + Plan file + {path && ( + + {path} + )} -
- - + + + +
+ {error ? ( +
+ {error} +
+ ) : isLoading ? ( +
Loading plan…
+ ) : content !== null ? ( + content.length > 0 ? ( + + {/* Keyed by the text: a reload replaces the plan in place, and Streamdown's + elements re-render only when their source position changes, so an edit that + keeps the positions would otherwise stay stale on screen. */} + + + ) : ( +
Plan file is empty
+ ) + ) : ( +
No plan loaded
+ )} +
+ ); -}; +} diff --git a/src/browser/features/RightSidebar/dialogsReconnect.test.tsx b/src/browser/features/RightSidebar/dialogsReconnect.test.tsx new file mode 100644 index 00000000000..e168584d59a --- /dev/null +++ b/src/browser/features/RightSidebar/dialogsReconnect.test.tsx @@ -0,0 +1,241 @@ +// Bootstrap Happy DOM before react-dom evaluates (see MemoryTab.test.tsx). +import "../../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, render, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { installDom } from "../../../../tests/ui/dom"; +import { BackgroundBashOutputDialog } from "@/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog"; +import type { APIClient } from "@/browser/contexts/API"; +import { + createConnectionHarness, + GAP_MODES, + type ConnectionHarness, +} from "@/browser/contexts/connection.testHarness"; +import { SubagentTranscriptDialog } from "@/browser/features/Tools/SubagentTranscriptDialog"; +import type { TestApiOverrides } from "@/browser/testUtils"; +import { PlanFileDialog } from "./PlanFileDialog"; + +/* + * A dialog that shows loaded content keeps it across a dropped connection: the new client + * reloads it in the background and replaces it only when the read lands. Opening another + * workspace, task or process still clears it. + */ + +interface Deferred { + promise: Promise; + resolve: (value: T) => void; +} + +function deferred(): Deferred { + let resolve!: (value: T) => void; + const promise = new Promise((r) => (resolve = r)); + return { promise, resolve }; +} + +/** What each server procedure answers; a test swaps them before a reconnect. */ +let answerPlan: () => Promise; +let answerTranscript: () => Promise; +let answerOutput: () => Promise; + +function planResult(content: string) { + return { success: true as const, data: { content, path: "/plans/plan.md" } }; +} + +function transcriptResult(thinkingLevel: "low" | "high") { + return { messages: [], thinkingLevel }; +} + +function outputResult(output: string) { + return { + success: true as const, + data: { status: "exited" as const, output, nextOffset: output.length, truncatedStart: false }, + }; +} + +function serverOverrides(): TestApiOverrides { + return { + workspace: { + getPlanContent: () => answerPlan(), + getSubagentTranscript: () => answerTranscript(), + backgroundBashes: { getOutput: () => answerOutput() }, + }, + } as unknown as TestApiOverrides; +} + +let harness: ConnectionHarness; + +function Wrapper(props: { children: ReactNode }) { + const Connection = harness.Wrapper; + return {props.children}; +} + +async function flush() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); +} + +/** Drop the connection and wait until the dialog settled on it. */ +async function drop() { + await harness.drop(); + await flush(); +} + +async function reconnect() { + harness.reconnect(); + await flush(); +} + +function noop() { + return undefined; +} + +describe.each([...GAP_MODES])("dialogs across a dropped connection (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + harness = createConnectionHarness(mode, serverOverrides); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("the plan dialog keeps the shown plan until the new client's read lands", async () => { + answerPlan = () => Promise.resolve(planResult("Plan A")); + const view = render(, { + wrapper: Wrapper, + }); + await view.findByText("Plan A"); + + await drop(); + expect(view.queryByText("Plan A")).not.toBeNull(); + expect(view.queryByText("API unavailable")).toBeNull(); + + // The new client's read is still in flight, and is then cut off by a second drop. + const pending = deferred(); + answerPlan = () => pending.promise; + await reconnect(); + expect(view.queryByText("Plan A")).not.toBeNull(); + expect(view.queryByText("Loading plan…")).toBeNull(); + await drop(); + expect(view.queryByText("Plan A")).not.toBeNull(); + expect(view.queryByTestId("plan-file-dialog-error")).toBeNull(); + + const next = deferred(); + answerPlan = () => next.promise; + await reconnect(); + expect(view.queryByText("Plan A")).not.toBeNull(); + next.resolve(planResult("Plan B")); + await view.findByText("Plan B"); + expect(view.queryByText("Plan A")).toBeNull(); + }); + + test("the transcript dialog keeps the shown transcript until the new client's read lands", async () => { + answerTranscript = () => Promise.resolve(transcriptResult("high")); + const view = render(, { + wrapper: Wrapper, + }); + await view.findByText("thinking: high"); + expect(view.queryByText("Transcript is empty")).not.toBeNull(); + + // Offline may show an error box, but above the transcript, which stays. + await drop(); + expect(view.queryByText("thinking: high")).not.toBeNull(); + expect(view.queryByText("Transcript is empty")).not.toBeNull(); + + const pending = deferred(); + answerTranscript = () => pending.promise; + await reconnect(); + expect(view.queryByText("thinking: high")).not.toBeNull(); + expect(view.queryByText("Transcript is empty")).not.toBeNull(); + await drop(); + expect(view.queryByText("Transcript is empty")).not.toBeNull(); + expect(view.queryByText(/WebSocket|connection/i)).toBeNull(); + + const next = deferred(); + answerTranscript = () => next.promise; + await reconnect(); + next.resolve(transcriptResult("low")); + await view.findByText("thinking: low"); + }); + + test("the background process dialog keeps the shown output until the new client's read lands", async () => { + answerOutput = () => Promise.resolve(outputResult("line A")); + const view = render( + , + { wrapper: Wrapper } + ); + await view.findByText("line A"); + + await drop(); + expect(view.queryByText("line A")).not.toBeNull(); + + const pending = deferred(); + answerOutput = () => pending.promise; + await reconnect(); + expect(view.queryByText("line A")).not.toBeNull(); + expect(view.queryByText("Loading…")).toBeNull(); + await drop(); + expect(view.queryByText("line A")).not.toBeNull(); + expect(view.queryByText(/WebSocket|connection/i)).toBeNull(); + + // The new read is a fresh tail: it replaces the kept output instead of appending to it. + const next = deferred(); + answerOutput = () => next.promise; + await reconnect(); + next.resolve(outputResult("line B")); + await view.findByText("line B"); + expect(view.queryByText(/line A/)).toBeNull(); + }); +}); + +describe("dialogs opened on other data", () => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + harness = createConnectionHarness("kept-gap", serverOverrides); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("another workspace clears the shown plan while its own plan loads", async () => { + answerPlan = () => Promise.resolve(planResult("Plan A")); + const view = render(, { + wrapper: Wrapper, + }); + await view.findByText("Plan A"); + + const pending = deferred(); + answerPlan = () => pending.promise; + view.rerender(); + await waitFor(() => expect(view.queryByText("Loading plan…")).not.toBeNull()); + expect(view.queryByText("Plan A")).toBeNull(); + }); + + test("another process clears the shown output while its own output loads", async () => { + answerOutput = () => Promise.resolve(outputResult("line A")); + const view = render( + , + { wrapper: Wrapper } + ); + await view.findByText("line A"); + + const pending = deferred(); + answerOutput = () => pending.promise; + view.rerender( + + ); + await waitFor(() => expect(view.queryByText("Loading…")).not.toBeNull()); + expect(view.queryByText("line A")).toBeNull(); + }); +}); diff --git a/src/browser/features/Tools/SubagentTranscriptDialog.tsx b/src/browser/features/Tools/SubagentTranscriptDialog.tsx index d10b2fc5145..469a87e8ca2 100644 --- a/src/browser/features/Tools/SubagentTranscriptDialog.tsx +++ b/src/browser/features/Tools/SubagentTranscriptDialog.tsx @@ -14,6 +14,7 @@ import { ErrorBox, LoadingDots } from "./Shared/ToolPrimitives"; import { MessageRenderer } from "@/browser/features/Messages/MessageRenderer"; import { ModelDisplay } from "@/browser/features/Messages/ModelDisplay"; import { BashCollapsedSummaryModeProvider } from "./BashCollapsedSummaryModeContext"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { getErrorMessage } from "@/common/utils/errors"; interface SubagentTranscriptDialogProps { @@ -54,7 +55,10 @@ export const SubagentTranscriptDialog: React.FC = + {/* Keyed by the transcript, so another task starts from an empty viewer, while a new API + client (a reconnect) only re-runs the viewer's read and keeps the transcript shown. */} void; - setThinkingLevel?: (thinkingLevel: ThinkingLevel | undefined) => void; + setModel: (model: string | undefined) => void; + setThinkingLevel: (thinkingLevel: ThinkingLevel | undefined) => void; }> = (props) => { const { api } = useAPI(); @@ -81,10 +85,18 @@ const SubagentTranscriptViewer: React.FC<{ const setModel = props.setModel; const setThinkingLevel = props.setThinkingLevel; - const [isLoading, setIsLoading] = useState(false); + // Loading until the first read lands: a fresh viewer (another task, or the dialog reopened) + // starts empty, and a new API client keeps what the viewer shows. + const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); const [messages, setMessages] = useState(null); + // The header's model and thinking level belong to the transcript this viewer shows. + useEffect(() => { + setModel(undefined); + setThinkingLevel(undefined); + }, [setModel, setThinkingLevel]); + useEffect(() => { // TaskToolCall renders this dialog component for each completed task even while closed. // Avoid expensive disk/IPC transcript loads until the dialog is opened. @@ -92,11 +104,7 @@ const SubagentTranscriptViewer: React.FC<{ return; } - setIsLoading(true); setError(null); - setMessages(null); - setModel?.(undefined); - setThinkingLevel?.(undefined); if (!api) { setIsLoading(false); @@ -116,11 +124,13 @@ const SubagentTranscriptViewer: React.FC<{ if (cancelled) return; setMessages(transcript.messages); - setModel?.(transcript.model); - setThinkingLevel?.(transcript.thinkingLevel); + setModel(transcript.model); + setThinkingLevel(transcript.thinkingLevel); setIsLoading(false); } catch (err: unknown) { - if (cancelled) return; + // A lost connection (or this effect's own cleanup) is not a transcript error: keep what + // is shown for the next client. + if (cancelled || isAbortError(err)) return; setIsLoading(false); setError(getErrorMessage(err)); } From f9e70e4d50309a18e2c6a7b09b5a511b4bffc840 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 09:54:32 +0000 Subject: [PATCH 02/11] refactor: DevTools and Workflows feeds keep shown runs while reloading A new API client re-subscribes; the shown runs (and the Workflows empty state with its script list) stay until the new snapshot replaces them. Only another workspace clears them. Signed-off-by: Thomas Kosiewski --- .../DevToolsTab/useDevToolsSubscription.ts | 43 ++-- .../RightSidebar/Workflows/WorkflowsTab.tsx | 45 +++- .../RightSidebar/Workflows/useWorkflowRuns.ts | 55 +++-- .../RightSidebar/feedsReconnect.test.tsx | 230 ++++++++++++++++++ 4 files changed, 325 insertions(+), 48 deletions(-) create mode 100644 src/browser/features/RightSidebar/feedsReconnect.test.tsx diff --git a/src/browser/features/RightSidebar/DevToolsTab/useDevToolsSubscription.ts b/src/browser/features/RightSidebar/DevToolsTab/useDevToolsSubscription.ts index df832d18f7a..5db1bdd85fe 100644 --- a/src/browser/features/RightSidebar/DevToolsTab/useDevToolsSubscription.ts +++ b/src/browser/features/RightSidebar/DevToolsTab/useDevToolsSubscription.ts @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { useAPI } from "@/browser/contexts/API"; import { isAbortError } from "@/browser/utils/isAbortError"; import type { DevToolsEvent, DevToolsRunSummary, DevToolsStep } from "@/common/types/devtools"; @@ -9,16 +9,21 @@ export function useDevToolsSubscription(workspaceId: string) { const [runs, setRuns] = useState([]); const [stepsByRun, setStepsByRun] = useState>(new Map()); const [error, setError] = useState(null); + // The workspace whose runs are shown. Only another workspace clears them: a re-run for the same + // one (no client during a reconnect, then a new client) keeps them until the new snapshot. + const shownWorkspaceIdRef = useRef(null); useEffect(() => { - if (!api) { + if (shownWorkspaceIdRef.current !== workspaceId) { + shownWorkspaceIdRef.current = workspaceId; setRuns([]); setStepsByRun(new Map()); + } + + if (!api) { return; } - setRuns([]); - setStepsByRun(new Map()); setError(null); const controller = new AbortController(); @@ -43,20 +48,9 @@ export function useDevToolsSubscription(workspaceId: string) { setRuns(event.runs); setStepsByRun(new Map()); break; - case "run-created": { - setRuns((previousRuns) => { - // De-duplicate if the run is already in state (e.g., from initial snapshot). - const existingRunIndex = previousRuns.findIndex((run) => run.id === event.run.id); - if (existingRunIndex >= 0) { - const updatedRuns = [...previousRuns]; - updatedRuns[existingRunIndex] = event.run; - return updatedRuns; - } - - return [event.run, ...previousRuns]; - }); + case "run-created": + setRuns((previousRuns) => upsertRun(previousRuns, event.run)); break; - } case "run-updated": setRuns((previousRuns) => previousRuns.map((run) => (run.id === event.run.id ? event.run : run)) @@ -106,6 +100,21 @@ export function useDevToolsSubscription(workspaceId: string) { return { runs, stepsByRun, error }; } +function upsertRun( + previousRuns: DevToolsRunSummary[], + incomingRun: DevToolsRunSummary +): DevToolsRunSummary[] { + // De-duplicate if the run is already in state (e.g., from initial snapshot). + const existingRunIndex = previousRuns.findIndex((run) => run.id === incomingRun.id); + if (existingRunIndex >= 0) { + const updatedRuns = [...previousRuns]; + updatedRuns[existingRunIndex] = incomingRun; + return updatedRuns; + } + + return [incomingRun, ...previousRuns]; +} + function upsertStep( previousStepsByRun: Map, incomingStep: DevToolsStep diff --git a/src/browser/features/RightSidebar/Workflows/WorkflowsTab.tsx b/src/browser/features/RightSidebar/Workflows/WorkflowsTab.tsx index 075bd355a7b..a7ce9dd9c1a 100644 --- a/src/browser/features/RightSidebar/Workflows/WorkflowsTab.tsx +++ b/src/browser/features/RightSidebar/Workflows/WorkflowsTab.tsx @@ -12,31 +12,34 @@ import { projectWorkflowRun, selectPrimaryWorkflowRun } from "./projectWorkflowR import { useWorkflowRuns } from "./useWorkflowRuns"; /** - * Right-sidebar Workflows tab: a live, scannable view of the workspace's - * durable workflow runs. Focuses the most recent active run by default; the - * run-history list lets the user pin any prior run into focus. + * The empty state's script launcher for one workspace: the runnable scripts, the script being + * started, and the last start error. */ -export const WorkflowsTab: React.FC<{ workspaceId: string }> = (props) => { +function useWorkflowLauncher(workspaceId: string, onStarted: () => void) { const { api } = useAPI(); - const { runs, loading, error } = useWorkflowRuns(props.workspaceId); - const [overrideRunId, setOverrideRunId] = React.useState(null); const [scripts, setScripts] = React.useState([]); const [busyScriptPath, setBusyScriptPath] = React.useState(null); const [runError, setRunError] = React.useState(null); + // The workspace the launcher state belongs to. + const launcherWorkspaceIdRef = React.useRef(null); // Discover runnable scripts for the empty-state list (one per workspace). React.useEffect(() => { // Reset launcher state first so a workspace switch never shows or launches a stale script // from the previous workspace — including if the new fetch is still pending or fails. - setScripts([]); - setBusyScriptPath(null); - setRunError(null); + // A re-run for the same workspace (a new API client after a reconnect) keeps the list. + if (launcherWorkspaceIdRef.current !== workspaceId) { + launcherWorkspaceIdRef.current = workspaceId; + setScripts([]); + setBusyScriptPath(null); + setRunError(null); + } if (api == null) { return; } let ignore = false; api.workflows - .listScripts({ workspaceId: props.workspaceId }) + .listScripts({ workspaceId }) .then((result) => { if (!ignore) { setScripts(result); @@ -48,7 +51,7 @@ export const WorkflowsTab: React.FC<{ workspaceId: string }> = (props) => { return () => { ignore = true; }; - }, [api, props.workspaceId]); + }, [api, workspaceId]); const runScript = async (script: AvailableWorkflow, args: Record) => { if (api == null || busyScriptPath != null) { @@ -58,14 +61,14 @@ export const WorkflowsTab: React.FC<{ workspaceId: string }> = (props) => { setRunError(null); try { await api.workflows.start({ - workspaceId: props.workspaceId, + workspaceId, scriptPath: script.scriptPath, args, // Background the launch so the call returns immediately; the run then streams // into the tab via the subscription and becomes the focused run. runInBackground: true, }); - setOverrideRunId(null); + onStarted(); } catch (startError) { setRunError(startError instanceof Error ? startError.message : "Failed to start workflow"); } finally { @@ -73,6 +76,22 @@ export const WorkflowsTab: React.FC<{ workspaceId: string }> = (props) => { } }; + return { scripts, busyScriptPath, runError, runScript }; +} + +/** + * Right-sidebar Workflows tab: a live, scannable view of the workspace's + * durable workflow runs. Focuses the most recent active run by default; the + * run-history list lets the user pin any prior run into focus. + */ +export const WorkflowsTab: React.FC<{ workspaceId: string }> = (props) => { + const { runs, loading, error } = useWorkflowRuns(props.workspaceId); + const [overrideRunId, setOverrideRunId] = React.useState(null); + const { scripts, busyScriptPath, runError, runScript } = useWorkflowLauncher( + props.workspaceId, + () => setOverrideRunId(null) + ); + if (error != null) { return
Failed to load workflows: {error}
; } diff --git a/src/browser/features/RightSidebar/Workflows/useWorkflowRuns.ts b/src/browser/features/RightSidebar/Workflows/useWorkflowRuns.ts index 93db8ec6b34..88771d6d573 100644 --- a/src/browser/features/RightSidebar/Workflows/useWorkflowRuns.ts +++ b/src/browser/features/RightSidebar/Workflows/useWorkflowRuns.ts @@ -1,4 +1,4 @@ -import { useContext, useEffect, useState } from "react"; +import { useContext, useEffect, useRef, useState } from "react"; import { APIContext, useAPI, type APIClient } from "@/browser/contexts/API"; import { isAbortError } from "@/browser/utils/isAbortError"; @@ -23,6 +23,29 @@ function recency(run: WorkflowRunRecord): number { return Number.isFinite(value) ? value : 0; } +/** + * The state update for one feed event: a snapshot replaces the shown runs, a delta upserts one + * run. Called as the event arrives, so an unknown event throws into the feed's error handling. + */ +function workflowRunsUpdate( + event: WorkflowRunStreamEvent +): (previous: Map) => Map { + switch (event.type) { + case "snapshot": { + const snapshot = new Map(event.runs.map((run) => [run.id, run])); + return () => snapshot; + } + case "run-changed": + return (previous) => { + const next = new Map(previous); + next.set(event.run.id, event.run); + return next; + }; + default: + return assertNever(event); + } +} + /** * Live workflow-run feed for one workspace. Subscribes to `workflows.subscribe` * (snapshot + per-write deltas) and keeps a local id→record map in sync, so the @@ -34,16 +57,23 @@ export function useWorkflowRuns(workspaceId: string): UseWorkflowRunsResult { const [runsById, setRunsById] = useState>(new Map()); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); + // The workspace whose runs are shown. Only another workspace clears them: a re-run for the same + // one (no client during a reconnect, then a new client) keeps them, and the empty state, until + // the new snapshot replaces them. + const shownWorkspaceIdRef = useRef(null); useEffect(() => { - if (!api) { + if (shownWorkspaceIdRef.current !== workspaceId) { + shownWorkspaceIdRef.current = workspaceId; setRunsById(new Map()); + setLoading(true); + } + + if (!api) { setLoading(false); return; } - setRunsById(new Map()); - setLoading(true); setError(null); const controller = new AbortController(); @@ -62,20 +92,9 @@ export function useWorkflowRuns(workspaceId: string): UseWorkflowRunsResult { if (signal.aborted) { break; } - switch (event.type) { - case "snapshot": - setRunsById(new Map(event.runs.map((run) => [run.id, run]))); - setLoading(false); - break; - case "run-changed": - setRunsById((previous) => { - const next = new Map(previous); - next.set(event.run.id, event.run); - return next; - }); - break; - default: - assertNever(event); + setRunsById(workflowRunsUpdate(event)); + if (event.type === "snapshot") { + setLoading(false); } } }; diff --git a/src/browser/features/RightSidebar/feedsReconnect.test.tsx b/src/browser/features/RightSidebar/feedsReconnect.test.tsx new file mode 100644 index 00000000000..bf8f56e92df --- /dev/null +++ b/src/browser/features/RightSidebar/feedsReconnect.test.tsx @@ -0,0 +1,230 @@ +// Bootstrap Happy DOM before react-dom evaluates (see MemoryTab.test.tsx). +import "../../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, render, renderHook, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { installDom } from "../../../../tests/ui/dom"; +import type { APIClient } from "@/browser/contexts/API"; +import { + createConnectionHarness, + GAP_MODES, + type ConnectionHarness, +} from "@/browser/contexts/connection.testHarness"; +import type { TestApiOverrides } from "@/browser/testUtils"; +import type { DevToolsRunSummary } from "@/common/types/devtools"; +import type { AvailableWorkflow, WorkflowRunRecord } from "@/common/types/workflow"; +import { useDevToolsSubscription } from "./DevToolsTab/useDevToolsSubscription"; +import { useWorkflowRuns } from "./Workflows/useWorkflowRuns"; +import { WorkflowsTab } from "./Workflows/WorkflowsTab"; + +/* + * Live feeds (a snapshot, then deltas) keep what they show across a dropped connection: the new + * client's snapshot replaces it when it arrives. Another workspace still clears it. + */ + +interface Deferred { + promise: Promise; + resolve: (value: T) => void; +} + +function deferred(): Deferred { + let resolve!: (value: T) => void; + const promise = new Promise((r) => (resolve = r)); + return { promise, resolve }; +} + +/** The ids each feed's next snapshot carries; a test swaps them (or holds them) before a connect. */ +let devToolsSnapshot: () => Promise; +let workflowSnapshot: () => Promise; +let scriptNames: () => Promise; + +/** A stream that sends one snapshot, then stays open until the connection drops. */ +function snapshotStream(snapshot: Promise) { + return Promise.resolve( + (async function* () { + yield await snapshot; + await new Promise(() => undefined); + })() + ); +} + +function devToolsRun(id: string): DevToolsRunSummary { + return { id } as unknown as DevToolsRunSummary; +} + +function workflowRun(id: string): WorkflowRunRecord { + return { id, updatedAt: "2026-01-01T00:00:00.000Z" } as unknown as WorkflowRunRecord; +} + +function script(name: string): AvailableWorkflow { + const workflow: AvailableWorkflow = { + descriptor: { name, description: `${name} script`, scope: "project", executable: true }, + scriptPath: `skill://${name}/workflow.js`, + args: [], + }; + return workflow; +} + +function serverOverrides(): TestApiOverrides { + return { + devtools: { + subscribe: () => + snapshotStream( + devToolsSnapshot().then((ids) => ({ type: "snapshot", runs: ids.map(devToolsRun) })) + ), + }, + workflows: { + subscribe: () => + snapshotStream( + workflowSnapshot().then((ids) => ({ type: "snapshot", runs: ids.map(workflowRun) })) + ), + listScripts: () => scriptNames().then((names) => names.map(script)), + }, + } as unknown as TestApiOverrides; +} + +let harness: ConnectionHarness; + +function Wrapper(props: { children: ReactNode }) { + const Connection = harness.Wrapper; + return {props.children}; +} + +async function flush() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); +} + +async function drop() { + await harness.drop(); + await flush(); +} + +async function reconnect() { + harness.reconnect(); + await flush(); +} + +function ids(runs: Array<{ id: string }>): string[] { + return runs.map((run) => run.id); +} + +describe.each([...GAP_MODES])("live feeds across a dropped connection (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + harness = createConnectionHarness(mode, serverOverrides); + devToolsSnapshot = () => Promise.resolve(["run-a"]); + workflowSnapshot = () => Promise.resolve(["run-a"]); + scriptNames = () => Promise.resolve(["deploy"]); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("the DevTools feed keeps the shown runs until the new snapshot", async () => { + const view = renderHook(() => useDevToolsSubscription("ws-1"), { wrapper: Wrapper }); + await waitFor(() => expect(ids(view.result.current.runs)).toEqual(["run-a"])); + + await drop(); + expect(ids(view.result.current.runs)).toEqual(["run-a"]); + + const next = deferred(); + devToolsSnapshot = () => next.promise; + await reconnect(); + expect(ids(view.result.current.runs)).toEqual(["run-a"]); + + next.resolve(["run-b"]); + await waitFor(() => expect(ids(view.result.current.runs)).toEqual(["run-b"])); + }); + + test("the workflow feed keeps the shown runs until the new snapshot", async () => { + const view = renderHook(() => useWorkflowRuns("ws-1"), { wrapper: Wrapper }); + await waitFor(() => expect(ids(view.result.current.runs)).toEqual(["run-a"])); + + await drop(); + expect(ids(view.result.current.runs)).toEqual(["run-a"]); + + const next = deferred(); + workflowSnapshot = () => next.promise; + await reconnect(); + expect(ids(view.result.current.runs)).toEqual(["run-a"]); + expect(view.result.current.loading).toBe(false); + + next.resolve(["run-b"]); + await waitFor(() => expect(ids(view.result.current.runs)).toEqual(["run-b"])); + }); + + test("the Workflows tab keeps its empty state and script list while it reloads", async () => { + workflowSnapshot = () => Promise.resolve([]); + const view = render(, { wrapper: Wrapper }); + await view.findByText("deploy"); + + await drop(); + expect(view.queryByText("deploy")).not.toBeNull(); + + const nextRuns = deferred(); + const nextScripts = deferred(); + workflowSnapshot = () => nextRuns.promise; + scriptNames = () => nextScripts.promise; + await reconnect(); + expect(view.queryByText("deploy")).not.toBeNull(); + expect(view.queryByText("Loading workflow runs…")).toBeNull(); + + nextRuns.resolve([]); + nextScripts.resolve(["release"]); + await view.findByText("release"); + expect(view.queryByText("deploy")).toBeNull(); + }); +}); + +describe("live feeds of another workspace", () => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + harness = createConnectionHarness("kept-gap", serverOverrides); + devToolsSnapshot = () => Promise.resolve(["run-a"]); + workflowSnapshot = () => Promise.resolve(["run-a"]); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("another workspace clears the shown runs while its own snapshot loads", async () => { + const devTools = renderHook( + (props: { workspaceId: string }) => useDevToolsSubscription(props.workspaceId), + { + wrapper: Wrapper, + initialProps: { workspaceId: "ws-1" }, + } + ); + const workflows = renderHook( + (props: { workspaceId: string }) => useWorkflowRuns(props.workspaceId), + { + wrapper: Wrapper, + initialProps: { workspaceId: "ws-1" }, + } + ); + await waitFor(() => expect(ids(devTools.result.current.runs)).toEqual(["run-a"])); + await waitFor(() => expect(ids(workflows.result.current.runs)).toEqual(["run-a"])); + + devToolsSnapshot = () => new Promise(() => undefined); + workflowSnapshot = () => new Promise(() => undefined); + devTools.rerender({ workspaceId: "ws-2" }); + workflows.rerender({ workspaceId: "ws-2" }); + await flush(); + expect(ids(devTools.result.current.runs)).toEqual([]); + expect(ids(workflows.result.current.runs)).toEqual([]); + expect(workflows.result.current.loading).toBe(true); + }); +}); From ac24a4759b5d2aba25c6843733cd0561223c1148 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 09:57:49 +0000 Subject: [PATCH 03/11] refactor: goal board, goal defaults, MCP overview and Instructions keep shown data while reloading A new API client re-runs each load. The panes keep what they show until the new read lands: no loading state over loaded data, no reset when a read is cut off by the lost connection. The goal defaults stay editable during a reload and an edit made meanwhile wins over the reload's older answer; another workspace still clears its override. Signed-off-by: Thomas Kosiewski --- .../AdditionalSystemContextScratchpad.tsx | 24 +- .../InstructionsTab/InstructionsTab.tsx | 4 +- .../ProjectMCPOverview/ProjectMCPOverview.tsx | 12 +- .../RightSidebar/GoalDefaultsSection.tsx | 92 +++++-- .../RightSidebar/loadersReconnect.test.tsx | 257 ++++++++++++++++++ .../features/RightSidebar/useGoalBoard.ts | 42 ++- 6 files changed, 373 insertions(+), 58 deletions(-) create mode 100644 src/browser/features/RightSidebar/loadersReconnect.test.tsx diff --git a/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx b/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx index 000d8daa0ed..b4f482a1fa7 100644 --- a/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx +++ b/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx @@ -14,6 +14,7 @@ import { useAdditionalSystemContextSnapshot, } from "@/browser/utils/additionalSystemContextStore"; import { focusInstructionsTab } from "@/browser/utils/instructionsTabFocus"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { cn } from "@/common/lib/utils"; import { getErrorMessage } from "@/common/utils/errors"; @@ -56,6 +57,10 @@ export function useChatInstructions(workspaceId: string): ScratchpadState { const dirtyRef = useRef(false); const mountedRef = useRef(true); const savingTimerRef = useRef | null>(null); + // The workspace whose instructions the editor shows (loaded, or failed to load). A re-run for it + // (a new API client after a reconnect) keeps the editor enabled and the user's edits: it does + // not flash "Loading…" or take focus from the textarea. + const loadedWorkspaceIdRef = useRef(null); const clearSavingTimer = () => { if (savingTimerRef.current != null) { @@ -73,20 +78,24 @@ export function useChatInstructions(workspaceId: string): ScratchpadState { }, []); useEffect(() => { - dirtyRef.current = false; - setLoading(true); - setSavingVisible(false); - clearSavingTimer(); + if (loadedWorkspaceIdRef.current !== workspaceId) { + loadedWorkspaceIdRef.current = null; + dirtyRef.current = false; + setLoading(true); + setSavingVisible(false); + clearSavingTimer(); + } setError(null); if (!api) return; const loadVersion = getAdditionalSystemContextVersion(workspaceId); + // Set by the cleanup, which also runs on unmount. let cancelled = false; api.workspace .getAdditionalSystemContext({ workspaceId }) .then((result) => { - if (cancelled || !mountedRef.current) return; + if (cancelled) return; if (!dirtyRef.current && getAdditionalSystemContextVersion(workspaceId) === loadVersion) { updateAdditionalSystemContextSnapshot(workspaceId, { content: result.content, @@ -94,11 +103,14 @@ export function useChatInstructions(workspaceId: string): ScratchpadState { }); } setLoading(false); + loadedWorkspaceIdRef.current = workspaceId; }) .catch((err) => { - if (cancelled || !mountedRef.current) return; + // A lost connection is not a load error: keep the editor as it is for the next client. + if (cancelled || isAbortError(err)) return; setError(getErrorMessage(err)); setLoading(false); + loadedWorkspaceIdRef.current = workspaceId; }); return () => { diff --git a/src/browser/components/InstructionsTab/InstructionsTab.tsx b/src/browser/components/InstructionsTab/InstructionsTab.tsx index 7aff8a0fdd3..438972f063e 100644 --- a/src/browser/components/InstructionsTab/InstructionsTab.tsx +++ b/src/browser/components/InstructionsTab/InstructionsTab.tsx @@ -79,7 +79,9 @@ function InstructionsTabImpl(props: InstructionsTabProps) {
{error && } - {!error && !loading && data?.files.length === 0 && } + {/* Not gated on `loading`: a reload (refresh, or a new API client after a reconnect) + keeps the empty state on screen, as it keeps a file list. */} + {!error && data?.files.length === 0 && } {data && data.files.length > 0 && } {loading && !data && }
diff --git a/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx b/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx index d0caa54cf3f..0c84af92754 100644 --- a/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx +++ b/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx @@ -6,6 +6,7 @@ import { useSettings } from "@/browser/contexts/SettingsContext"; import { Button } from "@/browser/components/Button/Button"; import { getMCPServersKey } from "@/common/constants/storage"; import { readPersistedState, updatePersistedState } from "@/browser/hooks/usePersistedState"; +import { isAbortError } from "@/browser/utils/isAbortError"; interface ProjectMCPOverviewProps { projectPath: string; @@ -37,16 +38,15 @@ export const ProjectMCPOverview: React.FC = (props) => // Cache for next load updatePersistedState(getMCPServersKey(projectPath), newServers); setError(null); + setLoading(false); }) .catch((err) => { - if (cancelled) return; + // A lost connection is not a load error: keep the shown servers (and the spinner) for + // the next client, which loads them again. + if (cancelled || isAbortError(err)) return; setServers({}); setError(err instanceof Error ? err.message : "Failed to load MCP servers"); - }) - .finally(() => { - if (!cancelled) { - setLoading(false); - } + setLoading(false); }); return () => { diff --git a/src/browser/features/RightSidebar/GoalDefaultsSection.tsx b/src/browser/features/RightSidebar/GoalDefaultsSection.tsx index 3dd176884aa..ec488dccf2b 100644 --- a/src/browser/features/RightSidebar/GoalDefaultsSection.tsx +++ b/src/browser/features/RightSidebar/GoalDefaultsSection.tsx @@ -4,6 +4,7 @@ import { useEffect, useRef, useState } from "react"; import { Input } from "@/browser/components/Input/Input"; import { useAPI } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { GoalDefaultsControls } from "@/browser/features/Settings/Sections/GoalsSection"; import { cn } from "@/common/lib/utils"; import type { WorkspaceGoalDefaultsOverride } from "@/browser/utils/goals/resolveGoalSetIntent"; @@ -67,11 +68,12 @@ function parseTurnCapValue(value: string): number | null { return Number.isSafeInteger(parsed) && parsed > 0 ? parsed : null; } -export function GoalDefaultsSection(props: GoalDefaultsSectionProps) { +/** + * The workspace's stored override, read once per workspace. The panel edits it optimistically + * through `setEditedOverride`. + */ +function useWorkspaceGoalOverride(workspaceId: string) { const { api } = useAPI(); - const [globalDefaults, setGlobalDefaults] = useState(() => ({ - ...DEFAULT_GOAL_DEFAULTS, - })); const [override, setOverride] = useState(null); // Distinguishes "loaded; no override stored" (override === null, isLoading // === false) from "still loading" (override === null, isLoading === true). @@ -80,6 +82,61 @@ export function GoalDefaultsSection(props: GoalDefaultsSectionProps) { // which `set` then writes — clobbering any saved override on disk. // Preserve saved workspace defaults while loading. const [isLoading, setIsLoading] = useState(true); + // The workspace whose override is loaded. A re-read for it (a new API client after a reconnect) + // keeps the controls shown and enabled; only another workspace resets them. + const loadedWorkspaceIdRef = useRef(null); + // Counts the user's edits, so an edit made while a re-read is in flight wins over its result. + const editCountRef = useRef(0); + + // Workspace override is loaded once per workspaceId. `null` (the + // canonical "no override" state) is what we expect for fresh workspaces. + // Reset `isLoading` whenever the workspaceId changes so a workspace + // switch doesn't let the new workspace's edits race the old read. + useEffect(() => { + if (loadedWorkspaceIdRef.current !== workspaceId) { + loadedWorkspaceIdRef.current = null; + setOverride(null); + setIsLoading(true); + } + if (!api) return; + let cancelled = false; + const editCountAtStart = editCountRef.current; + void api.workspace.goalDefaults + .get({ workspaceId }) + .then((value) => { + if (cancelled) return; + if (editCountRef.current === editCountAtStart) { + setOverride(value ?? null); + } + setIsLoading(false); + loadedWorkspaceIdRef.current = workspaceId; + }) + .catch((error: unknown) => { + // A lost connection keeps what is shown for the next client. Another failure enables + // the controls on what is shown (the empty override on a first read), as before. + if (cancelled || isAbortError(error)) return; + setIsLoading(false); + loadedWorkspaceIdRef.current = workspaceId; + }); + return () => { + cancelled = true; + }; + }, [api, workspaceId]); + + const setEditedOverride = (next: WorkspaceGoalDefaultsOverride) => { + editCountRef.current += 1; + setOverride(next); + }; + + return { override, isLoading, setEditedOverride }; +} + +export function GoalDefaultsSection(props: GoalDefaultsSectionProps) { + const { api } = useAPI(); + const [globalDefaults, setGlobalDefaults] = useState(() => ({ + ...DEFAULT_GOAL_DEFAULTS, + })); + const { override, isLoading, setEditedOverride } = useWorkspaceGoalOverride(props.workspaceId); // Pull the global defaults so we can render inherited values inside the // workspace-override panel. We only re-read on mount/api change; updates @@ -105,31 +162,6 @@ export function GoalDefaultsSection(props: GoalDefaultsSectionProps) { }; }, [api]); - // Workspace override is loaded once per workspaceId. `null` (the - // canonical "no override" state) is what we expect for fresh workspaces. - // Reset `isLoading` whenever the workspaceId changes so a workspace - // switch doesn't let the new workspace's edits race the old read. - useEffect(() => { - if (!api) return; - let cancelled = false; - setIsLoading(true); - void api.workspace.goalDefaults - .get({ workspaceId: props.workspaceId }) - .then((value) => { - if (cancelled) return; - setOverride(value ?? null); - setIsLoading(false); - }) - .catch(() => { - if (cancelled) return; - setOverride(null); - setIsLoading(false); - }); - return () => { - cancelled = true; - }; - }, [api, props.workspaceId]); - const onPersistProp = props.onPersist; // Serialize workspace-default saves. The backend setter // reads → mutates → writes the full config record, so two saves in @@ -144,7 +176,7 @@ export function GoalDefaultsSection(props: GoalDefaultsSectionProps) { // Otherwise a stale all-null shape could overwrite saved overrides. // The UI also disables the toggles via `isLoading` below. if (isLoading) return; - setOverride(next); + setEditedOverride(next); if (!api) return; saveChainRef.current = saveChainRef.current .catch(() => { diff --git a/src/browser/features/RightSidebar/loadersReconnect.test.tsx b/src/browser/features/RightSidebar/loadersReconnect.test.tsx new file mode 100644 index 00000000000..fb0577fe7f1 --- /dev/null +++ b/src/browser/features/RightSidebar/loadersReconnect.test.tsx @@ -0,0 +1,257 @@ +// Bootstrap Happy DOM before react-dom evaluates (see MemoryTab.test.tsx). +import "../../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, fireEvent, render, renderHook, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { installDom } from "../../../../tests/ui/dom"; +import { InstructionsTab } from "@/browser/components/InstructionsTab/InstructionsTab"; +import { ProjectMCPOverview } from "@/browser/components/ProjectMCPOverview/ProjectMCPOverview"; +import type { APIClient } from "@/browser/contexts/API"; +import { + createConnectionHarness, + GAP_MODES, + type ConnectionHarness, +} from "@/browser/contexts/connection.testHarness"; +import { RouterProvider } from "@/browser/contexts/RouterContext"; +import { SettingsProvider } from "@/browser/contexts/SettingsContext"; +import { TooltipProvider } from "@/browser/components/Tooltip/Tooltip"; +import type { TestApiOverrides } from "@/browser/testUtils"; +import { GoalDefaultsSection } from "./GoalDefaultsSection"; +import { useGoalBoard } from "./useGoalBoard"; + +/* + * Panes that load once per workspace or project keep what they show across a dropped + * connection: the new client reloads in the background, a read cut off by the drop changes + * nothing, and only the read that lands replaces the shown data. + */ + +interface Deferred { + promise: Promise; + resolve: (value: T) => void; +} + +function deferred(): Deferred { + let resolve!: (value: T) => void; + const promise = new Promise((r) => (resolve = r)); + return { promise, resolve }; +} + +/** What each server procedure answers; a test swaps them (or holds them) before a connect. */ +let answerBoard: () => Promise; +let answerServers: () => Promise; +let answerInstructions: () => Promise; +let answerChatInstructions: () => Promise; +let answerOverride: () => Promise; +let savedOverrides: unknown[] = []; + +function board(goalId: string) { + return { entries: [{ id: goalId }] }; +} + +function servers(name: string) { + return { [name]: { transport: "stdio", command: "run", disabled: false } }; +} + +function turnCapOverride(turnCap: number) { + return { defaultBudgetCents: null, defaultTurnCap: turnCap, alwaysRequireExplicitBudget: null }; +} + +function serverOverrides(): TestApiOverrides { + return { + workspace: { + getGoalBoard: () => answerBoard(), + getInstructions: () => answerInstructions(), + getAdditionalSystemContext: () => answerChatInstructions(), + goalDefaults: { + get: () => answerOverride(), + set: (input: unknown) => { + savedOverrides.push(input); + return Promise.resolve(); + }, + }, + }, + mcp: { list: () => answerServers() }, + config: { getConfig: () => Promise.resolve({}) }, + } as unknown as TestApiOverrides; +} + +let harness: ConnectionHarness; + +function Wrapper(props: { children: ReactNode }) { + const Connection = harness.Wrapper; + return ( + + + + {props.children} + + + + ); +} + +async function flush() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); +} + +async function drop() { + await harness.drop(); + await flush(); +} + +async function reconnect() { + harness.reconnect(); + await flush(); +} + +/** Never answers: the read stays in flight until a drop cuts it off. */ +function pending(): Promise { + return new Promise(() => undefined); +} + +function goalIds(result: { current: { board: { entries: Array<{ id: string }> } } }): string[] { + return result.current.board.entries.map((entry) => entry.id); +} + +describe.each([...GAP_MODES])("loaded panes across a dropped connection (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + window.localStorage.clear(); + harness = createConnectionHarness(mode, serverOverrides); + savedOverrides = []; + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("the goal board keeps its goals through a reload the drop cut off", async () => { + answerBoard = () => Promise.resolve(board("goal-a")); + const view = renderHook(() => useGoalBoard("ws-1"), { wrapper: Wrapper }); + await waitFor(() => expect(goalIds(view.result)).toEqual(["goal-a"])); + + await drop(); + expect(goalIds(view.result)).toEqual(["goal-a"]); + + // The new client's read is still in flight when a second drop cuts it off. + answerBoard = pending; + await reconnect(); + await drop(); + expect(goalIds(view.result)).toEqual(["goal-a"]); + + answerBoard = () => Promise.resolve(board("goal-b")); + await reconnect(); + await waitFor(() => expect(goalIds(view.result)).toEqual(["goal-b"])); + }); + + test("the MCP overview keeps its servers through a reload the drop cut off", async () => { + answerServers = () => Promise.resolve(servers("alpha")); + const view = render(, { + wrapper: Wrapper, + }); + await view.findByText("alpha"); + + await drop(); + expect(view.queryByText("alpha")).not.toBeNull(); + + answerServers = pending; + await reconnect(); + await drop(); + expect(view.queryByText("alpha")).not.toBeNull(); + expect(view.queryByText("No MCP servers enabled for this project.")).toBeNull(); + + answerServers = () => Promise.resolve(servers("beta")); + await reconnect(); + await view.findByText("beta"); + }); + + test("the Instructions tab keeps its empty state and an enabled editor while it reloads", async () => { + answerInstructions = () => Promise.resolve({ files: [], totalTokens: 0 }); + answerChatInstructions = () => Promise.resolve({ content: "be brief", enabled: true }); + const view = render(, { wrapper: Wrapper }); + const editor = (await view.findByLabelText("Chat Instructions editor")) as HTMLTextAreaElement; + await waitFor(() => expect(editor.value).toBe("be brief")); + await view.findByText("No instruction files loaded for this workspace."); + expect(editor.disabled).toBe(false); + + await drop(); + const nextInstructions = deferred(); + const nextChatInstructions = deferred(); + answerInstructions = () => nextInstructions.promise; + answerChatInstructions = () => nextChatInstructions.promise; + await reconnect(); + expect(view.queryByText("No instruction files loaded for this workspace.")).not.toBeNull(); + expect(editor.disabled).toBe(false); + expect(view.queryByText("Loading…")).toBeNull(); + + nextInstructions.resolve({ files: [], totalTokens: 0 }); + nextChatInstructions.resolve({ content: "be precise", enabled: true }); + await waitFor(() => expect(editor.value).toBe("be precise")); + }); + + test("the goal defaults keep the override editable while they reload, and an edit wins", async () => { + answerOverride = () => Promise.resolve(turnCapOverride(5)); + const view = render(, { + wrapper: Wrapper, + }); + const turnCap = () => + view.getByLabelText("Workspace default goal turn cap") as HTMLInputElement; + await waitFor(() => expect(turnCap().value).toBe("5")); + + await drop(); + const next = deferred(); + answerOverride = () => next.promise; + await reconnect(); + expect(turnCap().value).toBe("5"); + expect(turnCap().disabled).toBe(false); + expect(view.queryByText("Loading…")).toBeNull(); + + // The user edits while the reload is in flight; its older answer must not undo the edit. + fireEvent.change(turnCap(), { target: { value: "9" } }); + fireEvent.blur(turnCap()); + await waitFor(() => expect(savedOverrides).toHaveLength(1)); + next.resolve(turnCapOverride(7)); + await flush(); + expect(turnCap().value).toBe("9"); + }); +}); + +describe("loaded panes of another workspace", () => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + window.localStorage.clear(); + harness = createConnectionHarness("kept-gap", serverOverrides); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("another workspace clears the shown override while its own loads", async () => { + answerOverride = () => Promise.resolve(turnCapOverride(5)); + const view = render(, { + wrapper: Wrapper, + }); + await waitFor(() => + expect( + (view.getByLabelText("Workspace default goal turn cap") as HTMLInputElement).value + ).toBe("5") + ); + + answerOverride = pending; + view.rerender(); + await waitFor(() => expect(view.queryByText("Loading…")).not.toBeNull()); + expect(view.queryByLabelText("Workspace default goal turn cap")).toBeNull(); + }); +}); diff --git a/src/browser/features/RightSidebar/useGoalBoard.ts b/src/browser/features/RightSidebar/useGoalBoard.ts index 8819e639b6c..09e9aac27be 100644 --- a/src/browser/features/RightSidebar/useGoalBoard.ts +++ b/src/browser/features/RightSidebar/useGoalBoard.ts @@ -1,6 +1,7 @@ import { useCallback, useContext, useEffect, useState } from "react"; -import { APIContext } from "@/browser/contexts/API"; +import { APIContext, type APIClient } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import type { GoalBoardSnapshot } from "@/common/types/goal"; /** @@ -20,6 +21,23 @@ import type { GoalBoardSnapshot } from "@/common/types/goal"; * tighter coupling could push board snapshots through the activity * channel; today's reads are cheap enough to drive on-demand. */ +const EMPTY_BOARD: GoalBoardSnapshot = { entries: [] }; + +/** + * Reads a workspace's board. A failed read is an empty board, as before, but a read cut off by a + * lost connection resolves to null: the caller keeps the board it shows for the next client. + */ +async function readGoalBoard( + api: APIClient, + workspaceId: string +): Promise { + try { + return (await api.workspace.getGoalBoard({ workspaceId })) ?? EMPTY_BOARD; + } catch (error) { + return isAbortError(error) ? null : EMPTY_BOARD; + } +} + export interface UseGoalBoardResult { board: GoalBoardSnapshot; isLoading: boolean; @@ -46,30 +64,24 @@ export function useGoalBoard( // `useGoalDefaults`. const context = useContext(APIContext); const api = context?.api ?? null; - const [board, setBoard] = useState({ entries: [] }); + const [board, setBoard] = useState(EMPTY_BOARD); const [isLoading, setIsLoading] = useState(true); const [refreshKey, setRefreshKey] = useState(0); useEffect(() => { if (!api || !workspaceId) { - setBoard({ entries: [] }); + // No client (a reconnect) keeps the shown board; no workspace has no board. + setBoard((current) => (workspaceId ? current : EMPTY_BOARD)); setIsLoading(false); return; } let cancelled = false; setIsLoading(true); - void api.workspace - .getGoalBoard({ workspaceId }) - .then((snapshot) => { - if (cancelled) return; - setBoard(snapshot ?? { entries: [] }); - setIsLoading(false); - }) - .catch(() => { - if (cancelled) return; - setBoard({ entries: [] }); - setIsLoading(false); - }); + void readGoalBoard(api, workspaceId).then((snapshot) => { + if (cancelled || snapshot == null) return; + setBoard(snapshot); + setIsLoading(false); + }); return () => { cancelled = true; }; From 5e2e8efd87704e87da5b63395eaf7b7665b76e2c Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 10:02:15 +0000 Subject: [PATCH 04/11] test: type the goal board fixture in the loaders reconnect test Signed-off-by: Thomas Kosiewski --- .../features/RightSidebar/loadersReconnect.test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/browser/features/RightSidebar/loadersReconnect.test.tsx b/src/browser/features/RightSidebar/loadersReconnect.test.tsx index fb0577fe7f1..07bfc558c08 100644 --- a/src/browser/features/RightSidebar/loadersReconnect.test.tsx +++ b/src/browser/features/RightSidebar/loadersReconnect.test.tsx @@ -18,7 +18,7 @@ import { SettingsProvider } from "@/browser/contexts/SettingsContext"; import { TooltipProvider } from "@/browser/components/Tooltip/Tooltip"; import type { TestApiOverrides } from "@/browser/testUtils"; import { GoalDefaultsSection } from "./GoalDefaultsSection"; -import { useGoalBoard } from "./useGoalBoard"; +import { useGoalBoard, type UseGoalBoardResult } from "./useGoalBoard"; /* * Panes that load once per workspace or project keep what they show across a dropped @@ -46,7 +46,7 @@ let answerOverride: () => Promise; let savedOverrides: unknown[] = []; function board(goalId: string) { - return { entries: [{ id: goalId }] }; + return { entries: [{ section: "upcoming", goal: { goalId } }] }; } function servers(name: string) { @@ -112,8 +112,8 @@ function pending(): Promise { return new Promise(() => undefined); } -function goalIds(result: { current: { board: { entries: Array<{ id: string }> } } }): string[] { - return result.current.board.entries.map((entry) => entry.id); +function goalIds(result: { current: UseGoalBoardResult }): string[] { + return result.current.board.entries.map((entry) => entry.goal.goalId); } describe.each([...GAP_MODES])("loaded panes across a dropped connection (%s)", (mode) => { From bebb5e71929705c1d4364f905f87bbad9285e25b Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 10:02:16 +0000 Subject: [PATCH 05/11] refactor: browser tab and sidebar tab gates keep shown state while reconnecting Without a client, the browser tab keeps its discovered sessions and page tabs, and the sidebar keeps the last known debug-logs and desktop values (so it does not drop those tabs). A listing or read cut off by the lost connection shows no error and changes nothing. The two sidebar reads move into named hooks. Signed-off-by: Thomas Kosiewski --- .../BrowserTab/BrowserTab.reconnect.test.tsx | 146 +++++++++++++ .../RightSidebar/BrowserTab/BrowserTab.tsx | 33 +-- .../RightSidebar.reconnect.test.tsx | 115 ++++++++++ .../features/RightSidebar/RightSidebar.tsx | 199 ++++++++++-------- 4 files changed, 392 insertions(+), 101 deletions(-) create mode 100644 src/browser/features/RightSidebar/BrowserTab/BrowserTab.reconnect.test.tsx create mode 100644 src/browser/features/RightSidebar/RightSidebar.reconnect.test.tsx diff --git a/src/browser/features/RightSidebar/BrowserTab/BrowserTab.reconnect.test.tsx b/src/browser/features/RightSidebar/BrowserTab/BrowserTab.reconnect.test.tsx new file mode 100644 index 00000000000..cdca0d2f7e8 --- /dev/null +++ b/src/browser/features/RightSidebar/BrowserTab/BrowserTab.reconnect.test.tsx @@ -0,0 +1,146 @@ +// Bootstrap Happy DOM before react-dom evaluates (see MemoryTab.test.tsx). +import "../../../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import { act, cleanup, render } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { installDom } from "../../../../../tests/ui/dom"; +import { restoreModulesAfterSuite } from "../../../../../tests/ui/moduleMocks"; +import type { APIClient } from "@/browser/contexts/API"; +import { + createConnectionHarness, + GAP_MODES, + type ConnectionHarness, +} from "@/browser/contexts/connection.testHarness"; +import * as RealBrowserBridgeConnectionModule from "@/browser/features/RightSidebar/BrowserTab/useBrowserBridgeConnection"; +import type { TestApiOverrides } from "@/browser/testUtils"; +import type { BrowserPageTab } from "./browserBridgeTypes"; + +// The live bridge opens its own socket; this suite is about the API client, so stub the bridge +// (as BrowserTab.test.tsx does) and restore the real module after the suite. +restoreModulesAfterSuite([ + [ + "@/browser/features/RightSidebar/BrowserTab/useBrowserBridgeConnection", + { ...RealBrowserBridgeConnectionModule }, + ], +]); +void mock.module("@/browser/features/RightSidebar/BrowserTab/useBrowserBridgeConnection", () => ({ + useBrowserBridgeConnection: () => ({ + session: null, + connect: () => undefined, + disconnect: () => undefined, + sendInput: () => undefined, + setPendingUrl: () => undefined, + }), +})); + +import { BrowserTab } from "./BrowserTab"; + +/* + * A dropped connection must not empty the browser tab: the discovered session and its page-tab + * strip stay, a listing cut off by the drop shows no error, and the next client's listing + * replaces them. + */ + +/** What the server's listings answer; a test swaps them (or holds them) before a connect. */ +let answerSessions: () => Promise; +let answerTabs: () => Promise; + +function pageTab(tabId: string, title: string, active: boolean): BrowserPageTab { + return { tabId, label: null, title, url: `https://${tabId}.example.com/`, active, type: "page" }; +} + +function serverOverrides(): TestApiOverrides { + return { + browser: { + listSessions: () => answerSessions(), + listTabs: () => answerTabs(), + }, + } as unknown as TestApiOverrides; +} + +let harness: ConnectionHarness; + +function Wrapper(props: { children: ReactNode }) { + const Connection = harness.Wrapper; + return {props.children}; +} + +async function flush() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); +} + +async function drop() { + await harness.drop(); + await flush(); +} + +async function reconnect() { + harness.reconnect(); + await flush(); +} + +/** Never answers: the listing stays in flight until a drop cuts it off. */ +function pending(): Promise { + return new Promise(() => undefined); +} + +describe.each([...GAP_MODES])("BrowserTab across a dropped connection (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + window.localStorage.clear(); + harness = createConnectionHarness(mode, serverOverrides); + answerSessions = () => + Promise.resolve({ + sessions: [{ sessionName: "alpha", status: "attachable" }], + otherSessions: [], + }); + answerTabs = () => + Promise.resolve({ + tabs: [pageTab("t1", "First tab", true), pageTab("t2", "Second tab", false)], + error: undefined, + }); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps the page-tab strip through listings the drop cut off", async () => { + const view = render(, { + wrapper: Wrapper, + }); + await view.findByTestId("browser-page-tab-t2"); + + await drop(); + expect(view.queryByTestId("browser-page-tab-t2")).not.toBeNull(); + + // The new client's listings are still in flight when a second drop cuts them off. + answerSessions = pending; + answerTabs = pending; + await reconnect(); + await drop(); + expect(view.queryByTestId("browser-page-tab-t2")).not.toBeNull(); + expect(view.queryByText(/Not connected to the Xum server/)).toBeNull(); + + answerSessions = () => + Promise.resolve({ + sessions: [{ sessionName: "alpha", status: "attachable" }], + otherSessions: [], + }); + answerTabs = () => + Promise.resolve({ + tabs: [pageTab("t1", "First tab", true), pageTab("t3", "Third tab", false)], + error: undefined, + }); + await reconnect(); + await view.findByTestId("browser-page-tab-t3"); + expect(view.queryByTestId("browser-page-tab-t2")).toBeNull(); + }); +}); diff --git a/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx b/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx index b5778154f02..e37dce63ccf 100644 --- a/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx +++ b/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx @@ -2,6 +2,7 @@ import { useEffect, useRef, useState, type KeyboardEvent as ReactKeyboardEvent } import { Check, ChevronDown, Globe2, Loader2, Play, TriangleAlert } from "lucide-react"; import { useAPI } from "@/browser/contexts/API"; import { stopKeyboardPropagation } from "@/browser/utils/events"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { usePersistedState } from "@/browser/hooks/usePersistedState"; import { getBrowserSelectedSessionKey } from "@/common/constants/storage"; import { cn } from "@/common/lib/utils"; @@ -121,6 +122,10 @@ function matchesBrowserPageTabRef(tab: BrowserPageTab, tabRef: string | null): b return tabRef != null && tab.tabId === tabRef; } +function errorMessageOr(error: unknown, fallback: string): string { + return error instanceof Error ? error.message : fallback; +} + function areBrowserPageTabsEqual(first: BrowserPageTab[], second: BrowserPageTab[]): boolean { return JSON.stringify(first) === JSON.stringify(second); } @@ -186,9 +191,8 @@ export function BrowserTab(props: BrowserTabProps) { useEffect(() => { if (api == null) { + // Keep the discovered sessions through a reconnect: the new client re-lists them. setDiscoveryError("Browser API client is unavailable."); - setDiscoveredSessions([]); - setOtherDiscoveredSessions([]); return; } @@ -216,15 +220,14 @@ export function BrowserTab(props: BrowserTabProps) { chooseSelectedSession(currentSessionName, result.sessions) ); } catch (error) { - if (cancelled) { + // A lost connection is not a discovery failure; the next client re-lists the sessions. + if (cancelled || isAbortError(error)) { return; } // Preserve the last known discovery result so transient refresh failures do not // tear down an otherwise healthy browser bridge. - setDiscoveryError( - error instanceof Error ? error.message : "Failed to discover browser sessions." - ); + setDiscoveryError(errorMessageOr(error, "Failed to discover browser sessions.")); } finally { discoveryRefreshInFlightRef.current = false; } @@ -243,11 +246,13 @@ export function BrowserTab(props: BrowserTabProps) { }, [api, props.workspaceId, setSelectedCurrentSessionName]); useEffect(() => { - if ( - api == null || - selectedSessionName == null || - selectedDiscoveredSessionStatus !== "attachable" - ) { + // Without a client (a reconnect), keep the tab strip of the selected session as it is: the + // new client re-lists its tabs, and only another session resets them below. + if (api == null) { + tabRefreshInFlightSessionNameRef.current = null; + return; + } + if (selectedSessionName == null || selectedDiscoveredSessionStatus !== "attachable") { tabRefreshInFlightSessionNameRef.current = null; pageTabsSessionNameRef.current = null; setPageTabs((currentTabs) => (currentTabs.length === 0 ? currentTabs : [])); @@ -298,14 +303,12 @@ export function BrowserTab(props: BrowserTabProps) { ); setPageTabsError(null); } catch (error) { - if (cancelled) { + if (cancelled || isAbortError(error)) { return; } setPageTabsError( - error instanceof Error - ? error.message - : `Failed to list browser tabs for session "${selectedSessionName}".` + errorMessageOr(error, `Failed to list browser tabs for session "${selectedSessionName}".`) ); } finally { if (tabRefreshInFlightSessionNameRef.current === selectedSessionName) { diff --git a/src/browser/features/RightSidebar/RightSidebar.reconnect.test.tsx b/src/browser/features/RightSidebar/RightSidebar.reconnect.test.tsx new file mode 100644 index 00000000000..0763c412285 --- /dev/null +++ b/src/browser/features/RightSidebar/RightSidebar.reconnect.test.tsx @@ -0,0 +1,115 @@ +// Bootstrap Happy DOM before react-dom evaluates (see MemoryTab.test.tsx). +import "../../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, renderHook } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { installDom } from "../../../../tests/ui/dom"; +import { useAPI, type APIClient } from "@/browser/contexts/API"; +import { + createConnectionHarness, + GAP_MODES, + type ConnectionHarness, +} from "@/browser/contexts/connection.testHarness"; +import type { TestApiOverrides } from "@/browser/testUtils"; +import { useDesktopAvailable, useLlmDebugLogsEnabled } from "./RightSidebar"; + +/* + * The sidebar drops its Debug and Desktop tabs when the setting (or capability) reads false. + * A dropped connection is not such an answer: the last known value stays, so the tabs stay, + * until the next client reads it again. + */ + +/** What the server answers; a test swaps it (or holds it) before a connect. */ +let answerConfig: () => Promise; +let answerCapability: () => Promise; + +function serverOverrides(): TestApiOverrides { + return { + config: { getConfig: () => answerConfig() }, + desktop: { getCapability: () => answerCapability() }, + } as unknown as TestApiOverrides; +} + +let harness: ConnectionHarness; + +function Wrapper(props: { children: ReactNode }) { + const Connection = harness.Wrapper; + return {props.children}; +} + +async function flush() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); +} + +async function drop() { + await harness.drop(); + await flush(); +} + +async function reconnect() { + harness.reconnect(); + await flush(); +} + +/** Never answers: the read stays in flight until a drop cuts it off. */ +function pending(): Promise { + return new Promise(() => undefined); +} + +describe.each([...GAP_MODES])("sidebar tab gates across a dropped connection (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + harness = createConnectionHarness(mode, serverOverrides); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("the debug-logs setting keeps its value through a read the drop cut off", async () => { + answerConfig = () => Promise.resolve({ llmDebugLogs: true }); + const view = renderHook(() => useLlmDebugLogsEnabled(useAPI().api), { wrapper: Wrapper }); + await flush(); + expect(view.result.current).toBe(true); + + await drop(); + expect(view.result.current).toBe(true); + + answerConfig = pending; + await reconnect(); + await drop(); + expect(view.result.current).toBe(true); + + answerConfig = () => Promise.resolve({ llmDebugLogs: false }); + await reconnect(); + expect(view.result.current).toBe(false); + }); + + test("the desktop capability keeps its value through a read the drop cut off", async () => { + answerCapability = () => Promise.resolve({ available: true }); + const view = renderHook(() => useDesktopAvailable(useAPI().api, true, "ws-1"), { + wrapper: Wrapper, + }); + await flush(); + expect(view.result.current).toBe(true); + + await drop(); + expect(view.result.current).toBe(true); + + answerCapability = pending; + await reconnect(); + await drop(); + expect(view.result.current).toBe(true); + + answerCapability = () => Promise.resolve({ available: false }); + await reconnect(); + expect(view.result.current).toBe(false); + }); +}); diff --git a/src/browser/features/RightSidebar/RightSidebar.tsx b/src/browser/features/RightSidebar/RightSidebar.tsx index 26731c423ea..3f2fb441bb6 100644 --- a/src/browser/features/RightSidebar/RightSidebar.tsx +++ b/src/browser/features/RightSidebar/RightSidebar.tsx @@ -22,7 +22,7 @@ import { usePersistedState, } from "@/browser/hooks/usePersistedState"; import { useProvidersConfig } from "@/browser/hooks/useProvidersConfig"; -import { useAPI, liveClientOf } from "@/browser/contexts/API"; +import { useAPI, liveClientOf, type APIClient } from "@/browser/contexts/API"; import { useWorkspaceMetadata } from "@/browser/contexts/WorkspaceContext"; import { useSendMessageOptions } from "@/browser/hooks/useSendMessageOptions"; import { @@ -39,6 +39,7 @@ import type { GoalCreateIntent } from "@/browser/features/RightSidebar/GoalTab"; import { usePopoverError } from "@/browser/hooks/usePopoverError"; import { PopoverError } from "@/browser/components/PopoverError/PopoverError"; import { hasWorkspaceRepository } from "@/browser/utils/workspaceCapabilities"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { getErrorMessage } from "@/common/utils/errors"; import { showFeedbackToast } from "@/browser/utils/feedbackToast"; import { openSideChat } from "@/browser/utils/chatCommands"; @@ -847,6 +848,115 @@ const RegistryTabPanel: React.FC<{ ); }; +/** + * Whether LLM debug logs are on: null until the config is read, so the layout keeps its Debug + * tab until the setting is known. A toggle (the LLM_DEBUG_LOGS_CHANGED event) wins over a config + * read still in flight. + */ +export function useLlmDebugLogsEnabled(api: APIClient | null): boolean | null { + const [llmDebugLogsEnabled, setLlmDebugLogsEnabled] = React.useState(null); + const debugLogsLocalOverrideRef = React.useRef(false); + + React.useEffect(() => { + // Without a client (a reconnect), keep the last known value: the layout keeps its tabs. + if (!api) { + return; + } + + // Reset local override so a fresh config fetch can set the initial value. + debugLogsLocalOverrideRef.current = false; + let cancelled = false; + + void api.config + .getConfig() + .then((cfg) => { + if (cancelled) { + return; + } + + // If the user toggled debug logs while this config fetch was in flight, + // the event listener has already set the authoritative value. + if (debugLogsLocalOverrideRef.current) { + return; + } + + setLlmDebugLogsEnabled(cfg.llmDebugLogs === true); + }) + .catch((error: unknown) => { + // A lost connection says nothing about the setting: keep the Debug tab for the next client. + if (cancelled || isAbortError(error)) { + return; + } + + setLlmDebugLogsEnabled(false); + }); + + return () => { + cancelled = true; + }; + }, [api]); + + React.useEffect(() => { + const handleLlmDebugLogsChanged = (event: Event) => { + const detail = (event as CustomEvent<{ enabled: boolean }>).detail; + debugLogsLocalOverrideRef.current = true; + setLlmDebugLogsEnabled(detail?.enabled === true); + }; + + window.addEventListener(CUSTOM_EVENTS.LLM_DEBUG_LOGS_CHANGED, handleLlmDebugLogsChanged); + return () => + window.removeEventListener(CUSTOM_EVENTS.LLM_DEBUG_LOGS_CHANGED, handleLlmDebugLogsChanged); + }, []); + + return llmDebugLogsEnabled; +} + +/** + * Whether the workspace can show a Desktop tab: null while unknown, so the layout keeps the tab. + */ +export function useDesktopAvailable( + api: APIClient | null, + desktopExperimentEnabled: boolean, + workspaceId: string +): boolean | null { + const [desktopAvailable, setDesktopAvailable] = React.useState(null); + + React.useEffect(() => { + if (!desktopExperimentEnabled) { + setDesktopAvailable(false); + return; + } + + // A degraded (slow) connection still has a usable api; only a missing api means offline. + // Offline keeps the last known value, so the layout keeps its Desktop tab. + if (!api) { + return; + } + + let cancelled = false; + + void api.desktop + .getCapability({ workspaceId }) + .then((capability) => { + if (!cancelled) { + setDesktopAvailable(capability.available); + } + }) + .catch((error: unknown) => { + // A lost connection is not a missing capability: keep the Desktop tab for the next client. + if (!cancelled && !isAbortError(error)) { + setDesktopAvailable(false); + } + }); + + return () => { + cancelled = true; + }; + }, [api, desktopExperimentEnabled, workspaceId]); + + return desktopAvailable; +} + const RightSidebarComponent: React.FC = ({ workspaceId, workspacePath, @@ -942,9 +1052,8 @@ const RightSidebarComponent: React.FC = ({ // Opening the tab by shortcut leaves focus where it was (usually the chat input), so the // panel claims focus once it renders, like terminals do with autoFocusTerminalSession. const [autoFocusArtifacts, setAutoFocusArtifacts] = React.useState(false); - const [llmDebugLogsEnabled, setLlmDebugLogsEnabled] = React.useState(null); - const [desktopAvailable, setDesktopAvailable] = React.useState(null); - const debugLogsLocalOverrideRef = React.useRef(false); + const llmDebugLogsEnabled = useLlmDebugLogsEnabled(api); + const desktopAvailable = useDesktopAvailable(api, desktopExperimentEnabled, workspaceId); const setDisplayedGoal = async (intent: { // RightSidebar buttons only ever request user-facing transitions; @@ -1045,56 +1154,6 @@ const RightSidebarComponent: React.FC = ({ }); }; - React.useEffect(() => { - if (!api) { - setLlmDebugLogsEnabled(null); - return; - } - - // Reset local override so a fresh config fetch can set the initial value. - debugLogsLocalOverrideRef.current = false; - let cancelled = false; - - void api.config - .getConfig() - .then((cfg) => { - if (cancelled) { - return; - } - - // If the user toggled debug logs while this config fetch was in flight, - // the event listener has already set the authoritative value. - if (debugLogsLocalOverrideRef.current) { - return; - } - - setLlmDebugLogsEnabled(cfg.llmDebugLogs === true); - }) - .catch(() => { - if (cancelled) { - return; - } - - setLlmDebugLogsEnabled(false); - }); - - return () => { - cancelled = true; - }; - }, [api]); - - React.useEffect(() => { - const handleLlmDebugLogsChanged = (event: Event) => { - const detail = (event as CustomEvent<{ enabled: boolean }>).detail; - debugLogsLocalOverrideRef.current = true; - setLlmDebugLogsEnabled(detail?.enabled === true); - }; - - window.addEventListener(CUSTOM_EVENTS.LLM_DEBUG_LOGS_CHANGED, handleLlmDebugLogsChanged); - return () => - window.removeEventListener(CUSTOM_EVENTS.LLM_DEBUG_LOGS_CHANGED, handleLlmDebugLogsChanged); - }, []); - // A tool explicitly requested for new layouts (RIGHT_SIDEBAR_TAB_KEY). Without one, a // workspace starts with just the New tab, which guides the user to the tools. // Not gated on canReviewDiffs: that is false until workspace metadata loads, and a "review" @@ -1224,38 +1283,6 @@ const RightSidebarComponent: React.FC = ({ setLayoutRaw, }); - React.useEffect(() => { - if (!desktopExperimentEnabled) { - setDesktopAvailable(false); - return; - } - - // A degraded (slow) connection still has a usable api; only a missing api means offline. - if (!api) { - setDesktopAvailable(null); - return; - } - - let cancelled = false; - - void api.desktop - .getCapability({ workspaceId }) - .then((capability) => { - if (!cancelled) { - setDesktopAvailable(capability.available); - } - }) - .catch(() => { - if (!cancelled) { - setDesktopAvailable(false); - } - }); - - return () => { - cancelled = true; - }; - }, [api, desktopExperimentEnabled, workspaceId]); - React.useEffect(() => { if (desktopAvailable == null) { return; From c31b1d1a32d36734e738c149c530ea175499857b Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 11:42:55 +0000 Subject: [PATCH 06/11] fix: edits made while reconnecting reach the next client; the MCP overview keeps servers per project The chat-instructions editor and the goal defaults stay editable while Xum reconnects. An edit that no client received (made without a client, or its save cut off by the lost connection) looked saved, and the reload skipped it as a local edit, so the backend never got it. Both now keep that edit and send it to the next client, and the next read does not show over it. ProjectPage keeps the MCP overview mounted across project navigation. The overview now keeps the shown servers next to their project, so a cut-off load for project B no longer leaves project A's servers on screen. --- .../AdditionalSystemContextScratchpad.tsx | 128 ++++++++++++----- .../ProjectMCPOverview/ProjectMCPOverview.tsx | 32 ++++- .../RightSidebar/GoalDefaultsSection.tsx | 134 +++++++++++------- .../RightSidebar/loadersReconnect.test.tsx | 86 +++++++++++ 4 files changed, 291 insertions(+), 89 deletions(-) diff --git a/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx b/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx index b4f482a1fa7..504ac4b7458 100644 --- a/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx +++ b/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx @@ -2,12 +2,13 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { ArrowUpRight, BookOpen } from "lucide-react"; import { ChatInputDecoration } from "@/browser/components/ChatPane/ChatInputDecoration"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, type APIClient } from "@/browser/contexts/API"; import { type AdditionalSystemContextSnapshot, getAdditionalSystemContextFocusGeneration, getAdditionalSystemContextVersion, queueAdditionalSystemContextSave, + readAdditionalSystemContextSnapshot, requestAdditionalSystemContextFocus, subscribeAdditionalSystemContextFocus, updateAdditionalSystemContextSnapshot, @@ -48,19 +49,11 @@ interface ScratchpadState { setEnabled: (enabled: boolean) => void; } -export function useChatInstructions(workspaceId: string): ScratchpadState { - const { api } = useAPI(); - const snapshot = useAdditionalSystemContextSnapshot(workspaceId); - const [loading, setLoading] = useState(true); +/** Shows "Saving…" once a save takes longer than the indicator delay. */ +function useSavingIndicator() { const [savingVisible, setSavingVisible] = useState(false); - const [error, setError] = useState(null); - const dirtyRef = useRef(false); const mountedRef = useRef(true); const savingTimerRef = useRef | null>(null); - // The workspace whose instructions the editor shows (loaded, or failed to load). A re-run for it - // (a new API client after a reconnect) keeps the editor enabled and the user's edits: it does - // not flash "Loading…" or take focus from the textarea. - const loadedWorkspaceIdRef = useRef(null); const clearSavingTimer = () => { if (savingTimerRef.current != null) { @@ -77,13 +70,97 @@ export function useChatInstructions(workspaceId: string): ScratchpadState { }; }, []); + const startSaving = useCallback(() => { + // Only show the "Saving…" message if the save takes more than a beat — + // sub-second writes shouldn't flicker the UI on every keystroke. + clearSavingTimer(); + savingTimerRef.current = setTimeout(() => { + if (mountedRef.current) setSavingVisible(true); + savingTimerRef.current = null; + }, SAVING_INDICATOR_DELAY_MS); + }, []); + + const hideSaving = useCallback(() => { + clearSavingTimer(); + setSavingVisible(false); + }, []); + + return { savingVisible, startSaving, hideSaving, mountedRef }; +} + +/** + * Sends the editor's saves through the store's save queue. + * + * The editor stays editable while Xum reconnects. An edit that no client received (made without a + * client, or its save cut off by the lost connection) is kept and sent to the next client. + * Otherwise it would look saved, and the reload would skip it as a local edit, but the backend + * would never get it. + */ +function useChatInstructionsSave(api: APIClient | null, setError: (error: string | null) => void) { + const { savingVisible, startSaving, hideSaving, mountedRef } = useSavingIndicator(); + // The workspace whose latest edit no client has received yet. + const unsentWorkspaceIdRef = useRef(null); + + const send = useCallback( + (client: APIClient, workspaceId: string, next: AdditionalSystemContextSnapshot) => { + setError(null); + startSaving(); + queueAdditionalSystemContextSave(client, workspaceId, next, { + onError: (err) => { + if (!mountedRef.current) return; + // A lost connection is not a save error: the next client gets the edit. + if (isAbortError(err)) { + unsentWorkspaceIdRef.current = workspaceId; + return; + } + setError(getErrorMessage(err)); + }, + onIdle: () => { + if (!mountedRef.current) return; + hideSaving(); + }, + }); + }, + [setError, startSaving, hideSaving, mountedRef] + ); + + // A new client (after a reconnect) gets the edit the last one did not, as the store holds it now. + useEffect(() => { + const unsentWorkspaceId = unsentWorkspaceIdRef.current; + if (api == null || unsentWorkspaceId == null) return; + unsentWorkspaceIdRef.current = null; + send(api, unsentWorkspaceId, readAdditionalSystemContextSnapshot(unsentWorkspaceId)); + }, [api, send]); + + const save = (workspaceId: string, next: AdditionalSystemContextSnapshot) => { + if (api == null) { + unsentWorkspaceIdRef.current = workspaceId; + return; + } + send(api, workspaceId, next); + }; + + return { savingVisible, save, hideSaving }; +} + +export function useChatInstructions(workspaceId: string): ScratchpadState { + const { api } = useAPI(); + const snapshot = useAdditionalSystemContextSnapshot(workspaceId); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); + const { savingVisible, save, hideSaving } = useChatInstructionsSave(api, setError); + const dirtyRef = useRef(false); + // The workspace whose instructions the editor shows (loaded, or failed to load). A re-run for it + // (a new API client after a reconnect) keeps the editor enabled and the user's edits: it does + // not flash "Loading…" or take focus from the textarea. + const loadedWorkspaceIdRef = useRef(null); + useEffect(() => { if (loadedWorkspaceIdRef.current !== workspaceId) { loadedWorkspaceIdRef.current = null; dirtyRef.current = false; setLoading(true); - setSavingVisible(false); - clearSavingTimer(); + hideSaving(); } setError(null); @@ -116,34 +193,15 @@ export function useChatInstructions(workspaceId: string): ScratchpadState { return () => { cancelled = true; }; - }, [api, workspaceId]); + }, [api, workspaceId, hideSaving]); const queue = useCallback( (next: AdditionalSystemContextSnapshot) => { dirtyRef.current = true; updateAdditionalSystemContextSnapshot(workspaceId, next); - if (!api) return; - setError(null); - // Only show the "Saving…" message if the save takes more than a beat — - // sub-second writes shouldn't flicker the UI on every keystroke. - clearSavingTimer(); - savingTimerRef.current = setTimeout(() => { - if (mountedRef.current) setSavingVisible(true); - savingTimerRef.current = null; - }, SAVING_INDICATOR_DELAY_MS); - queueAdditionalSystemContextSave(api, workspaceId, next, { - onError: (err) => { - if (!mountedRef.current) return; - setError(getErrorMessage(err)); - }, - onIdle: () => { - if (!mountedRef.current) return; - clearSavingTimer(); - setSavingVisible(false); - }, - }); + save(workspaceId, next); }, - [api, workspaceId] + [save, workspaceId] ); const setContent = useCallback( diff --git a/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx b/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx index 0c84af92754..54ebca4b9dc 100644 --- a/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx +++ b/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx @@ -12,6 +12,28 @@ interface ProjectMCPOverviewProps { projectPath: string; } +interface ShownServers { + projectPath: string; + servers: Record; +} + +/** Reads the cached list from localStorage (no flash before the first load). */ +function cachedServers(projectPath: string): ShownServers { + return { + projectPath, + servers: readPersistedState>(getMCPServersKey(projectPath), {}), + }; +} + +/** + * The servers to show for `projectPath`. ProjectPage keeps this card mounted across project + * navigation, and a load cut off by a lost connection keeps the shown list: never show another + * project's list under this one, show this project's cached list until its own load lands. + */ +function serversFor(shown: ShownServers, projectPath: string): Record { + return shown.projectPath === projectPath ? shown.servers : cachedServers(projectPath).servers; +} + export const ProjectMCPOverview: React.FC = (props) => { const projectPath = props.projectPath; const { api } = useAPI(); @@ -19,10 +41,8 @@ export const ProjectMCPOverview: React.FC = (props) => const [loading, setLoading] = React.useState(false); const [error, setError] = React.useState(null); - // Initialize from localStorage cache to avoid flash - const [servers, setServers] = React.useState>(() => - readPersistedState>(getMCPServersKey(projectPath), {}) - ); + const [shown, setShown] = React.useState(() => cachedServers(projectPath)); + const servers = serversFor(shown, projectPath); React.useEffect(() => { if (!api || settings.isOpen) return; @@ -34,7 +54,7 @@ export const ProjectMCPOverview: React.FC = (props) => .then((result) => { if (cancelled) return; const newServers = result ?? {}; - setServers(newServers); + setShown({ projectPath, servers: newServers }); // Cache for next load updatePersistedState(getMCPServersKey(projectPath), newServers); setError(null); @@ -44,7 +64,7 @@ export const ProjectMCPOverview: React.FC = (props) => // A lost connection is not a load error: keep the shown servers (and the spinner) for // the next client, which loads them again. if (cancelled || isAbortError(err)) return; - setServers({}); + setShown({ projectPath, servers: {} }); setError(err instanceof Error ? err.message : "Failed to load MCP servers"); setLoading(false); }); diff --git a/src/browser/features/RightSidebar/GoalDefaultsSection.tsx b/src/browser/features/RightSidebar/GoalDefaultsSection.tsx index ec488dccf2b..45d4fbe1002 100644 --- a/src/browser/features/RightSidebar/GoalDefaultsSection.tsx +++ b/src/browser/features/RightSidebar/GoalDefaultsSection.tsx @@ -3,7 +3,7 @@ import React from "react"; import { useEffect, useRef, useState } from "react"; import { Input } from "@/browser/components/Input/Input"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, type APIClient } from "@/browser/contexts/API"; import { isAbortError } from "@/browser/utils/isAbortError"; import { GoalDefaultsControls } from "@/browser/features/Settings/Sections/GoalsSection"; import { cn } from "@/common/lib/utils"; @@ -68,11 +68,80 @@ function parseTurnCapValue(value: string): number | null { return Number.isSafeInteger(parsed) && parsed > 0 ? parsed : null; } +interface GoalOverrideWrite { + workspaceId: string; + override: WorkspaceGoalDefaultsOverride; + onSaved: (() => void) | undefined; +} + +/** + * Serializes workspace-default writes. The backend setter reads → mutates → writes the full + * config record, so two writes in flight could land out of order — a slower first request could + * complete after a later edit and overwrite it on disk while the UI continues to show the latest + * optimistic state, silently reverting the user's edit on next refetch. Each write waits for the + * previous one. + * + * The controls stay editable while Xum reconnects. A write that no client received (made without + * a client, or cut off by the lost connection) is kept until `sendUnsent` hands it to the next + * client. Otherwise the edit would look saved but never reach the backend. + */ +class GoalOverrideWriter { + private chain: Promise = Promise.resolve(); + private latest: WorkspaceGoalDefaultsOverride | null = null; + private unsent: GoalOverrideWrite | null = null; + private readonly writesByWorkspace = new Map(); + + /** + * How many writes this workspace has had. A read that began before a write must not show its + * (older) answer over the written value. + */ + writeCount(workspaceId: string): number { + return this.writesByWorkspace.get(workspaceId) ?? 0; + } + + write(api: APIClient | null, edit: GoalOverrideWrite): void { + this.writesByWorkspace.set(edit.workspaceId, this.writeCount(edit.workspaceId) + 1); + this.latest = edit.override; + if (api == null) { + this.unsent = edit; + return; + } + this.unsent = null; + this.chain = this.chain.then(() => + api.workspace.goalDefaults + .set({ + workspaceId: edit.workspaceId, + defaultBudgetCents: edit.override.defaultBudgetCents, + defaultTurnCap: edit.override.defaultTurnCap, + alwaysRequireExplicitBudget: edit.override.alwaysRequireExplicitBudget, + }) + .then(() => { + // Notify parent so prefilled create-form inputs can re-read the effective defaults. + edit.onSaved?.(); + }) + .catch((error: unknown) => { + // Other errors are swallowed (the optimistic edit already updated local state), as + // in the global panel. A newer edit replaces a lost one. + if (isAbortError(error) && this.latest === edit.override) { + this.unsent = edit; + } + }) + ); + } + + /** Sends the write the last client did not get to `api`. */ + sendUnsent(api: APIClient): void { + if (this.unsent != null) { + this.write(api, this.unsent); + } + } +} + /** * The workspace's stored override, read once per workspace. The panel edits it optimistically - * through `setEditedOverride`. + * through `edit`, which also writes it. */ -function useWorkspaceGoalOverride(workspaceId: string) { +function useWorkspaceGoalOverride(workspaceId: string, onPersist: (() => void) | undefined) { const { api } = useAPI(); const [override, setOverride] = useState(null); // Distinguishes "loaded; no override stored" (override === null, isLoading @@ -85,8 +154,8 @@ function useWorkspaceGoalOverride(workspaceId: string) { // The workspace whose override is loaded. A re-read for it (a new API client after a reconnect) // keeps the controls shown and enabled; only another workspace resets them. const loadedWorkspaceIdRef = useRef(null); - // Counts the user's edits, so an edit made while a re-read is in flight wins over its result. - const editCountRef = useRef(0); + // Also counts the user's edits, so an edit made while a re-read is in flight wins over its result. + const [writer] = useState(() => new GoalOverrideWriter()); // Workspace override is loaded once per workspaceId. `null` (the // canonical "no override" state) is what we expect for fresh workspaces. @@ -100,12 +169,15 @@ function useWorkspaceGoalOverride(workspaceId: string) { } if (!api) return; let cancelled = false; - const editCountAtStart = editCountRef.current; + const writesAtStart = writer.writeCount(workspaceId); + // An edit the last client did not get goes to this one first. It counts as a write after the + // read began, so the read's answer (maybe still the older stored value) does not show over it. + writer.sendUnsent(api); void api.workspace.goalDefaults .get({ workspaceId }) .then((value) => { if (cancelled) return; - if (editCountRef.current === editCountAtStart) { + if (writer.writeCount(workspaceId) === writesAtStart) { setOverride(value ?? null); } setIsLoading(false); @@ -121,14 +193,14 @@ function useWorkspaceGoalOverride(workspaceId: string) { return () => { cancelled = true; }; - }, [api, workspaceId]); + }, [api, workspaceId, writer]); - const setEditedOverride = (next: WorkspaceGoalDefaultsOverride) => { - editCountRef.current += 1; + const edit = (next: WorkspaceGoalDefaultsOverride) => { setOverride(next); + writer.write(api, { workspaceId, override: next, onSaved: onPersist }); }; - return { override, isLoading, setEditedOverride }; + return { override, isLoading, edit }; } export function GoalDefaultsSection(props: GoalDefaultsSectionProps) { @@ -136,7 +208,8 @@ export function GoalDefaultsSection(props: GoalDefaultsSectionProps) { const [globalDefaults, setGlobalDefaults] = useState(() => ({ ...DEFAULT_GOAL_DEFAULTS, })); - const { override, isLoading, setEditedOverride } = useWorkspaceGoalOverride(props.workspaceId); + const onPersistProp = props.onPersist; + const { override, isLoading, edit } = useWorkspaceGoalOverride(props.workspaceId, onPersistProp); // Pull the global defaults so we can render inherited values inside the // workspace-override panel. We only re-read on mount/api change; updates @@ -162,47 +235,12 @@ export function GoalDefaultsSection(props: GoalDefaultsSectionProps) { }; }, [api]); - const onPersistProp = props.onPersist; - // Serialize workspace-default saves. The backend setter - // reads → mutates → writes the full config record, so two saves in - // flight could land out of order — a slower first request could - // complete after a later edit and overwrite it on disk while the UI - // continues to show the latest optimistic state, silently reverting - // the user's edit on next refetch. Chain saves via a promise ref so - // each request awaits the previous before issuing its own write. - const saveChainRef = useRef>(Promise.resolve()); const persistOverride = (next: WorkspaceGoalDefaultsOverride) => { // Ignore edits while the initial read is in flight. // Otherwise a stale all-null shape could overwrite saved overrides. // The UI also disables the toggles via `isLoading` below. if (isLoading) return; - setEditedOverride(next); - if (!api) return; - saveChainRef.current = saveChainRef.current - .catch(() => { - // Previous save failed; we already swallowed its error in its - // own `.catch`, but the chain still has to continue so the - // next save isn't permanently rejected. - }) - .then(() => - api.workspace.goalDefaults - .set({ - workspaceId: props.workspaceId, - defaultBudgetCents: next.defaultBudgetCents, - defaultTurnCap: next.defaultTurnCap, - alwaysRequireExplicitBudget: next.alwaysRequireExplicitBudget, - }) - .then(() => { - // Notify parent so prefilled create-form inputs can re-read - // the effective defaults. Errors are swallowed (the - // optimistic `setOverride` above already updated local - // state). - onPersistProp?.(); - }) - .catch(() => { - /* swallow — same rationale as global panel */ - }) - ); + edit(next); }; // Synthesize a "current override" object (all-null is what the diff --git a/src/browser/features/RightSidebar/loadersReconnect.test.tsx b/src/browser/features/RightSidebar/loadersReconnect.test.tsx index 07bfc558c08..f60a382aa5a 100644 --- a/src/browser/features/RightSidebar/loadersReconnect.test.tsx +++ b/src/browser/features/RightSidebar/loadersReconnect.test.tsx @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { act, cleanup, fireEvent, render, renderHook, waitFor } from "@testing-library/react"; import type { ReactNode } from "react"; import { installDom } from "../../../../tests/ui/dom"; +import { useChatInstructions } from "@/browser/components/InstructionsTab/AdditionalSystemContextScratchpad"; import { InstructionsTab } from "@/browser/components/InstructionsTab/InstructionsTab"; import { ProjectMCPOverview } from "@/browser/components/ProjectMCPOverview/ProjectMCPOverview"; import type { APIClient } from "@/browser/contexts/API"; @@ -44,6 +45,7 @@ let answerInstructions: () => Promise; let answerChatInstructions: () => Promise; let answerOverride: () => Promise; let savedOverrides: unknown[] = []; +let savedChatInstructions: Array<{ content: string; enabled: boolean }> = []; function board(goalId: string) { return { entries: [{ section: "upcoming", goal: { goalId } }] }; @@ -63,6 +65,10 @@ function serverOverrides(): TestApiOverrides { getGoalBoard: () => answerBoard(), getInstructions: () => answerInstructions(), getAdditionalSystemContext: () => answerChatInstructions(), + setAdditionalSystemContext: (input: { content: string; enabled: boolean }) => { + savedChatInstructions.push({ content: input.content, enabled: input.enabled }); + return Promise.resolve({ content: input.content, enabled: input.enabled }); + }, goalDefaults: { get: () => answerOverride(), set: (input: unknown) => { @@ -124,6 +130,7 @@ describe.each([...GAP_MODES])("loaded panes across a dropped connection (%s)", ( window.localStorage.clear(); harness = createConnectionHarness(mode, serverOverrides); savedOverrides = []; + savedChatInstructions = []; }); afterEach(() => { @@ -223,6 +230,85 @@ describe.each([...GAP_MODES])("loaded panes across a dropped connection (%s)", ( }); }); +describe.each([...GAP_MODES])("edits and navigation while reconnecting (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + window.localStorage.clear(); + harness = createConnectionHarness(mode, serverOverrides); + savedOverrides = []; + savedChatInstructions = []; + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("a chat-instructions edit made while reconnecting reaches the next client", async () => { + // The instructions store is per workspace and process-wide: a fresh workspace per mode. + const workspaceId = `ws-offline-edit-${mode}`; + answerChatInstructions = () => Promise.resolve({ content: "be brief", enabled: true }); + const view = renderHook(() => useChatInstructions(workspaceId), { wrapper: Wrapper }); + await waitFor(() => expect(view.result.current.content).toBe("be brief")); + + await drop(); + act(() => view.result.current.setContent("be kind")); + await flush(); + expect(view.result.current.error).toBeNull(); + + // The next server still stores the old text: the edit must reach it and stay on screen. + await reconnect(); + await waitFor(() => + expect(savedChatInstructions).toEqual([{ content: "be kind", enabled: true }]) + ); + expect(view.result.current.content).toBe("be kind"); + expect(view.result.current.error).toBeNull(); + }); + + test("a goal-defaults edit made while reconnecting reaches the next client", async () => { + answerOverride = () => Promise.resolve(turnCapOverride(5)); + const view = render(, { + wrapper: Wrapper, + }); + const turnCap = () => + view.getByLabelText("Workspace default goal turn cap") as HTMLInputElement; + await waitFor(() => expect(turnCap().value).toBe("5")); + + await drop(); + fireEvent.change(turnCap(), { target: { value: "9" } }); + fireEvent.blur(turnCap()); + await flush(); + + // The next server still stores 5: the edit must reach it, and its read must not undo it. + await reconnect(); + await waitFor(() => expect(savedOverrides).toHaveLength(1)); + expect(savedOverrides[0]).toMatchObject({ workspaceId: "ws-1", defaultTurnCap: 9 }); + expect(turnCap().value).toBe("9"); + }); + + test("the MCP overview never shows one project's servers under another", async () => { + answerServers = () => Promise.resolve(servers("alpha")); + const view = render(, { wrapper: Wrapper }); + await view.findByText("alpha"); + + // Project B's first list is still loading when the connection drops and cuts it off. + answerServers = pending; + view.rerender(); + await flush(); + expect(view.queryByText("alpha")).toBeNull(); + await drop(); + expect(view.queryByText("alpha")).toBeNull(); + + answerServers = () => Promise.resolve(servers("beta")); + await reconnect(); + await view.findByText("beta"); + expect(view.queryByText("alpha")).toBeNull(); + }); +}); + describe("loaded panes of another workspace", () => { let cleanupDom: (() => void) | null = null; From 2ac4afcd61b9101d6dddb47a1d46aa09fd92de40 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 11:42:56 +0000 Subject: [PATCH 07/11] fix: panes opened while reconnecting show their loading state once a client reads The Workflows tab, the plan dialog and the transcript dialog could open while there was no client. They then stopped loading (the plan dialog also kept its offline error), so after the reconnect they showed the empty state or 'No transcript loaded' during the first read. Each now shows its loading state until its first load lands, and keeps already shown data on ordinary reconnects. --- .../features/RightSidebar/PlanFileDialog.tsx | 77 +++++++++++-------- .../RightSidebar/Workflows/useWorkflowRuns.ts | 3 +- .../RightSidebar/dialogsReconnect.test.tsx | 51 ++++++++++++ .../RightSidebar/feedsReconnect.test.tsx | 34 ++++++++ .../Tools/SubagentTranscriptDialog.tsx | 11 ++- 5 files changed, 140 insertions(+), 36 deletions(-) diff --git a/src/browser/features/RightSidebar/PlanFileDialog.tsx b/src/browser/features/RightSidebar/PlanFileDialog.tsx index d9db936287c..5e27369b1c7 100644 --- a/src/browser/features/RightSidebar/PlanFileDialog.tsx +++ b/src/browser/features/RightSidebar/PlanFileDialog.tsx @@ -1,6 +1,6 @@ import React, { useEffect, useRef, useState } from "react"; import assert from "@/common/utils/assert"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, type APIClient } from "@/browser/contexts/API"; import { MarkdownCore } from "@/browser/features/Messages/MarkdownCore"; import { PlanMarkdownContainer } from "@/browser/features/Messages/MarkdownRenderer"; import { @@ -30,6 +30,27 @@ export const PlanFileDialog: React.FC = (props) => ( ); +type PlanRead = + | { status: "ok"; content: string; path: string } + | { status: "error"; error: string } + /** Cut off by a lost connection (an abort): not a plan error. */ + | { status: "lost" }; + +async function readPlan(api: APIClient, workspaceId: string): Promise { + try { + const result = await api.workspace.getPlanContent({ workspaceId }); + if (!result.success) { + return { status: "error", error: result.error }; + } + assert(result.data.path.length > 0, "Plan path should be non-empty"); + return { status: "ok", content: result.data.content, path: result.data.path }; + } catch (error: unknown) { + return isAbortError(error) + ? { status: "lost" } + : { status: "error", error: getErrorMessage(error) }; + } +} + function usePlanFile(workspaceId: string) { const { api } = useAPI(); @@ -51,42 +72,30 @@ function usePlanFile(workspaceId: string) { return; } - let cancelled = false; - - const run = async () => { - try { - const result = await api.workspace.getPlanContent({ workspaceId }); - - if (cancelled) { - return; - } - - if (!result.success) { - setError(result.error); - setIsLoading(false); - return; - } + // A read with a client replaces the offline error. Without a plan on screen, it shows + // "Loading plan…" (the dialog may have opened while there was no client). + setError(null); + if (!hasPlanRef.current) { + setIsLoading(true); + } - assert(result.data.path.length > 0, "Plan path should be non-empty"); + let cancelled = false; - setError(null); - setContent(result.data.content); - setPath(result.data.path); - setIsLoading(false); - hasPlanRef.current = true; - } catch (error: unknown) { - // A lost connection (or this effect's own cleanup) is not a plan error: keep what is - // shown for the next client. - if (cancelled || isAbortError(error)) { - return; - } - - setError(getErrorMessage(error)); - setIsLoading(false); + void readPlan(api, workspaceId).then((read) => { + // A lost connection (or this effect's own cleanup) keeps what is shown for the next client. + if (cancelled || read.status === "lost") { + return; } - }; - - void run(); + setIsLoading(false); + if (read.status === "error") { + setError(read.error); + return; + } + setError(null); + setContent(read.content); + setPath(read.path); + hasPlanRef.current = true; + }); return () => { cancelled = true; diff --git a/src/browser/features/RightSidebar/Workflows/useWorkflowRuns.ts b/src/browser/features/RightSidebar/Workflows/useWorkflowRuns.ts index 88771d6d573..44964c25a1d 100644 --- a/src/browser/features/RightSidebar/Workflows/useWorkflowRuns.ts +++ b/src/browser/features/RightSidebar/Workflows/useWorkflowRuns.ts @@ -69,8 +69,9 @@ export function useWorkflowRuns(workspaceId: string): UseWorkflowRunsResult { setLoading(true); } + // Without a client nothing is loaded yet, so `loading` stays as it is: still true before the + // first snapshot of this workspace (the tab must not show its empty state, then flip to runs). if (!api) { - setLoading(false); return; } diff --git a/src/browser/features/RightSidebar/dialogsReconnect.test.tsx b/src/browser/features/RightSidebar/dialogsReconnect.test.tsx index e168584d59a..94da9846383 100644 --- a/src/browser/features/RightSidebar/dialogsReconnect.test.tsx +++ b/src/browser/features/RightSidebar/dialogsReconnect.test.tsx @@ -194,6 +194,57 @@ describe.each([...GAP_MODES])("dialogs across a dropped connection (%s)", (mode) }); }); +describe.each([...GAP_MODES])("dialogs opened while reconnecting (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(async () => { + cleanupDom = installDom(); + harness = createConnectionHarness(mode, serverOverrides); + // The dialog opens during the gap: nothing is shown yet. + await harness.drop(); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("the plan dialog shows its loading state once a client reads the plan", async () => { + answerPlan = () => Promise.resolve(planResult("unused")); + const view = render(, { + wrapper: Wrapper, + }); + await flush(); + + const next = deferred(); + answerPlan = () => next.promise; + await reconnect(); + expect(view.queryByText("Loading plan…")).not.toBeNull(); + expect(view.queryByTestId("plan-file-dialog-error")).toBeNull(); + + next.resolve(planResult("Plan A")); + await view.findByText("Plan A"); + }); + + test("the transcript dialog shows its loading state once a client reads the transcript", async () => { + answerTranscript = () => Promise.resolve(transcriptResult("low")); + const view = render(, { + wrapper: Wrapper, + }); + await flush(); + + const next = deferred(); + answerTranscript = () => next.promise; + await reconnect(); + expect(view.queryByText(/Loading transcript/)).not.toBeNull(); + expect(view.queryByText("No transcript loaded")).toBeNull(); + + next.resolve(transcriptResult("high")); + await view.findByText("thinking: high"); + }); +}); + describe("dialogs opened on other data", () => { let cleanupDom: (() => void) | null = null; diff --git a/src/browser/features/RightSidebar/feedsReconnect.test.tsx b/src/browser/features/RightSidebar/feedsReconnect.test.tsx index bf8f56e92df..c94b40910b8 100644 --- a/src/browser/features/RightSidebar/feedsReconnect.test.tsx +++ b/src/browser/features/RightSidebar/feedsReconnect.test.tsx @@ -184,6 +184,40 @@ describe.each([...GAP_MODES])("live feeds across a dropped connection (%s)", (mo }); }); +describe.each([...GAP_MODES])("the Workflows tab opened while reconnecting (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(async () => { + cleanupDom = installDom(); + harness = createConnectionHarness(mode, serverOverrides); + scriptNames = () => Promise.resolve(["deploy"]); + // The tab opens during the gap: no snapshot has landed yet. + await harness.drop(); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("shows its loading state, not the empty state, until the first snapshot lands", async () => { + workflowSnapshot = () => Promise.resolve([]); + const view = render(, { wrapper: Wrapper }); + await flush(); + + const next = deferred(); + workflowSnapshot = () => next.promise; + await reconnect(); + expect(view.queryByText("Loading workflow runs…")).not.toBeNull(); + expect(view.queryByText("deploy")).toBeNull(); + + next.resolve([]); + await view.findByText("deploy"); + expect(view.queryByText("Loading workflow runs…")).toBeNull(); + }); +}); + describe("live feeds of another workspace", () => { let cleanupDom: (() => void) | null = null; diff --git a/src/browser/features/Tools/SubagentTranscriptDialog.tsx b/src/browser/features/Tools/SubagentTranscriptDialog.tsx index 469a87e8ca2..fbaef44d71f 100644 --- a/src/browser/features/Tools/SubagentTranscriptDialog.tsx +++ b/src/browser/features/Tools/SubagentTranscriptDialog.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useMemo, useState } from "react"; +import React, { useEffect, useMemo, useRef, useState } from "react"; import type { DisplayedMessage, MuxMessage } from "@/common/types/message"; import type { ThinkingLevel } from "@/common/types/thinking"; import type { ChatMuxMessage } from "@/common/orpc/types"; @@ -90,6 +90,8 @@ const SubagentTranscriptViewer: React.FC<{ const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); const [messages, setMessages] = useState(null); + // True once a transcript is on screen: a re-read (a new API client) then keeps it shown. + const hasTranscriptRef = useRef(false); // The header's model and thinking level belong to the transcript this viewer shows. useEffect(() => { @@ -112,6 +114,12 @@ const SubagentTranscriptViewer: React.FC<{ return; } + // Without a transcript on screen, show the loading state (the dialog may have opened while + // there was no client). + if (!hasTranscriptRef.current) { + setIsLoading(true); + } + let cancelled = false; const run = async () => { @@ -127,6 +135,7 @@ const SubagentTranscriptViewer: React.FC<{ setModel(transcript.model); setThinkingLevel(transcript.thinkingLevel); setIsLoading(false); + hasTranscriptRef.current = true; } catch (err: unknown) { // A lost connection (or this effect's own cleanup) is not a transcript error: keep what // is shown for the next client. From e43c652b0d5872a09fa048c2df2dc89c0438bd2c Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 12:10:31 +0000 Subject: [PATCH 08/11] fix: only a lost connection keeps a pane quiet; a server cancel shows its error This layer ignored abort-named failures as "the connection was lost". isAbortError() also matches a server cancel on a live socket (a plain AbortError). No new client comes to re-run the effect then, so the pane stopped for good: the MCP overview kept its spinner over the old list, and a process dialog stopped polling without an error. Every check this layer added now uses isConnectionLostError(). The effects' own cleanup aborts stay handled by their cancelled flags. --- .../BackgroundBashOutputDialog.tsx | 10 +- .../AdditionalSystemContextScratchpad.tsx | 6 +- .../ProjectMCPOverview/ProjectMCPOverview.tsx | 4 +- .../RightSidebar/BrowserTab/BrowserTab.tsx | 6 +- .../RightSidebar/GoalDefaultsSection.tsx | 6 +- .../features/RightSidebar/PlanFileDialog.tsx | 6 +- .../features/RightSidebar/RightSidebar.tsx | 6 +- .../RightSidebar/offlinePanes.test.tsx | 101 ++++++++++++++++++ .../features/RightSidebar/useGoalBoard.ts | 4 +- .../Tools/SubagentTranscriptDialog.tsx | 4 +- 10 files changed, 128 insertions(+), 25 deletions(-) create mode 100644 src/browser/features/RightSidebar/offlinePanes.test.tsx diff --git a/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx b/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx index a3426635330..01b4f683670 100644 --- a/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx +++ b/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx @@ -4,7 +4,7 @@ import { Dialog, DialogContent, DialogHeader, DialogTitle } from "../Dialog/Dial import { DetailContent } from "@/browser/features/Tools/Shared/ToolPrimitives"; import { useAPI } from "@/browser/contexts/API"; import { useBackgroundProcesses } from "@/browser/stores/BackgroundBashStore"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { appendLiveBashOutputChunk, type LiveBashOutputInternal, @@ -142,10 +142,12 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str }; // A rejected read stops polling with the error shown instead of escaping as an unhandled - // rejection; reopening the dialog starts a fresh read. A lost connection (an abort) shows no - // error and keeps the output: the next client polls again. + // rejection; reopening the dialog starts a fresh read. Only a lost connection + // (ConnectionLostError) shows no error and keeps the output: the next client polls again. A + // server cancel on a live socket is a plain AbortError, and no new client would ever re-run + // this poll, so it shows its error. run().catch((err: unknown) => { - if (cancelled || isAbortError(err)) return; + if (cancelled || isConnectionLostError(err)) return; setIsLoading(false); setError(getErrorMessage(err)); }); diff --git a/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx b/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx index 504ac4b7458..eed3bc24628 100644 --- a/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx +++ b/src/browser/components/InstructionsTab/AdditionalSystemContextScratchpad.tsx @@ -15,7 +15,7 @@ import { useAdditionalSystemContextSnapshot, } from "@/browser/utils/additionalSystemContextStore"; import { focusInstructionsTab } from "@/browser/utils/instructionsTabFocus"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { cn } from "@/common/lib/utils"; import { getErrorMessage } from "@/common/utils/errors"; @@ -109,7 +109,7 @@ function useChatInstructionsSave(api: APIClient | null, setError: (error: string onError: (err) => { if (!mountedRef.current) return; // A lost connection is not a save error: the next client gets the edit. - if (isAbortError(err)) { + if (isConnectionLostError(err)) { unsentWorkspaceIdRef.current = workspaceId; return; } @@ -184,7 +184,7 @@ export function useChatInstructions(workspaceId: string): ScratchpadState { }) .catch((err) => { // A lost connection is not a load error: keep the editor as it is for the next client. - if (cancelled || isAbortError(err)) return; + if (cancelled || isConnectionLostError(err)) return; setError(getErrorMessage(err)); setLoading(false); loadedWorkspaceIdRef.current = workspaceId; diff --git a/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx b/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx index 54ebca4b9dc..4d7d15ded9d 100644 --- a/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx +++ b/src/browser/components/ProjectMCPOverview/ProjectMCPOverview.tsx @@ -6,7 +6,7 @@ import { useSettings } from "@/browser/contexts/SettingsContext"; import { Button } from "@/browser/components/Button/Button"; import { getMCPServersKey } from "@/common/constants/storage"; import { readPersistedState, updatePersistedState } from "@/browser/hooks/usePersistedState"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; interface ProjectMCPOverviewProps { projectPath: string; @@ -63,7 +63,7 @@ export const ProjectMCPOverview: React.FC = (props) => .catch((err) => { // A lost connection is not a load error: keep the shown servers (and the spinner) for // the next client, which loads them again. - if (cancelled || isAbortError(err)) return; + if (cancelled || isConnectionLostError(err)) return; setShown({ projectPath, servers: {} }); setError(err instanceof Error ? err.message : "Failed to load MCP servers"); setLoading(false); diff --git a/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx b/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx index e37dce63ccf..eca8fff8769 100644 --- a/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx +++ b/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx @@ -2,7 +2,7 @@ import { useEffect, useRef, useState, type KeyboardEvent as ReactKeyboardEvent } import { Check, ChevronDown, Globe2, Loader2, Play, TriangleAlert } from "lucide-react"; import { useAPI } from "@/browser/contexts/API"; import { stopKeyboardPropagation } from "@/browser/utils/events"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { usePersistedState } from "@/browser/hooks/usePersistedState"; import { getBrowserSelectedSessionKey } from "@/common/constants/storage"; import { cn } from "@/common/lib/utils"; @@ -221,7 +221,7 @@ export function BrowserTab(props: BrowserTabProps) { ); } catch (error) { // A lost connection is not a discovery failure; the next client re-lists the sessions. - if (cancelled || isAbortError(error)) { + if (cancelled || isConnectionLostError(error)) { return; } @@ -303,7 +303,7 @@ export function BrowserTab(props: BrowserTabProps) { ); setPageTabsError(null); } catch (error) { - if (cancelled || isAbortError(error)) { + if (cancelled || isConnectionLostError(error)) { return; } diff --git a/src/browser/features/RightSidebar/GoalDefaultsSection.tsx b/src/browser/features/RightSidebar/GoalDefaultsSection.tsx index 45d4fbe1002..344cf3b42bf 100644 --- a/src/browser/features/RightSidebar/GoalDefaultsSection.tsx +++ b/src/browser/features/RightSidebar/GoalDefaultsSection.tsx @@ -4,7 +4,7 @@ import { useEffect, useRef, useState } from "react"; import { Input } from "@/browser/components/Input/Input"; import { useAPI, type APIClient } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { GoalDefaultsControls } from "@/browser/features/Settings/Sections/GoalsSection"; import { cn } from "@/common/lib/utils"; import type { WorkspaceGoalDefaultsOverride } from "@/browser/utils/goals/resolveGoalSetIntent"; @@ -122,7 +122,7 @@ class GoalOverrideWriter { .catch((error: unknown) => { // Other errors are swallowed (the optimistic edit already updated local state), as // in the global panel. A newer edit replaces a lost one. - if (isAbortError(error) && this.latest === edit.override) { + if (isConnectionLostError(error) && this.latest === edit.override) { this.unsent = edit; } }) @@ -186,7 +186,7 @@ function useWorkspaceGoalOverride(workspaceId: string, onPersist: (() => void) | .catch((error: unknown) => { // A lost connection keeps what is shown for the next client. Another failure enables // the controls on what is shown (the empty override on a first read), as before. - if (cancelled || isAbortError(error)) return; + if (cancelled || isConnectionLostError(error)) return; setIsLoading(false); loadedWorkspaceIdRef.current = workspaceId; }); diff --git a/src/browser/features/RightSidebar/PlanFileDialog.tsx b/src/browser/features/RightSidebar/PlanFileDialog.tsx index 5e27369b1c7..1a5ea885fc9 100644 --- a/src/browser/features/RightSidebar/PlanFileDialog.tsx +++ b/src/browser/features/RightSidebar/PlanFileDialog.tsx @@ -9,7 +9,7 @@ import { DialogHeader, DialogTitle, } from "@/browser/components/Dialog/Dialog"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { getErrorMessage } from "@/common/utils/errors"; interface PlanFileDialogProps { @@ -33,7 +33,7 @@ export const PlanFileDialog: React.FC = (props) => ( type PlanRead = | { status: "ok"; content: string; path: string } | { status: "error"; error: string } - /** Cut off by a lost connection (an abort): not a plan error. */ + /** Cut off by a lost connection (ConnectionLostError): not a plan error. */ | { status: "lost" }; async function readPlan(api: APIClient, workspaceId: string): Promise { @@ -45,7 +45,7 @@ async function readPlan(api: APIClient, workspaceId: string): Promise assert(result.data.path.length > 0, "Plan path should be non-empty"); return { status: "ok", content: result.data.content, path: result.data.path }; } catch (error: unknown) { - return isAbortError(error) + return isConnectionLostError(error) ? { status: "lost" } : { status: "error", error: getErrorMessage(error) }; } diff --git a/src/browser/features/RightSidebar/RightSidebar.tsx b/src/browser/features/RightSidebar/RightSidebar.tsx index 3f2fb441bb6..b0fe2e174ad 100644 --- a/src/browser/features/RightSidebar/RightSidebar.tsx +++ b/src/browser/features/RightSidebar/RightSidebar.tsx @@ -39,7 +39,7 @@ import type { GoalCreateIntent } from "@/browser/features/RightSidebar/GoalTab"; import { usePopoverError } from "@/browser/hooks/usePopoverError"; import { PopoverError } from "@/browser/components/PopoverError/PopoverError"; import { hasWorkspaceRepository } from "@/browser/utils/workspaceCapabilities"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { getErrorMessage } from "@/common/utils/errors"; import { showFeedbackToast } from "@/browser/utils/feedbackToast"; import { openSideChat } from "@/browser/utils/chatCommands"; @@ -884,7 +884,7 @@ export function useLlmDebugLogsEnabled(api: APIClient | null): boolean | null { }) .catch((error: unknown) => { // A lost connection says nothing about the setting: keep the Debug tab for the next client. - if (cancelled || isAbortError(error)) { + if (cancelled || isConnectionLostError(error)) { return; } @@ -944,7 +944,7 @@ export function useDesktopAvailable( }) .catch((error: unknown) => { // A lost connection is not a missing capability: keep the Desktop tab for the next client. - if (!cancelled && !isAbortError(error)) { + if (!cancelled && !isConnectionLostError(error)) { setDesktopAvailable(false); } }); diff --git a/src/browser/features/RightSidebar/offlinePanes.test.tsx b/src/browser/features/RightSidebar/offlinePanes.test.tsx new file mode 100644 index 00000000000..9a21753d110 --- /dev/null +++ b/src/browser/features/RightSidebar/offlinePanes.test.tsx @@ -0,0 +1,101 @@ +// Bootstrap Happy DOM before react-dom evaluates (see MemoryTab.test.tsx). +import "../../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, render } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { installDom } from "../../../../tests/ui/dom"; +import { BackgroundBashOutputDialog } from "@/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog"; +import { ProjectMCPOverview } from "@/browser/components/ProjectMCPOverview/ProjectMCPOverview"; +import type { APIClient } from "@/browser/contexts/API"; +import { + createConnectionHarness, + GAP_MODES, + type ConnectionHarness, +} from "@/browser/contexts/connection.testHarness"; +import { RouterProvider } from "@/browser/contexts/RouterContext"; +import { SettingsProvider } from "@/browser/contexts/SettingsContext"; +import type { TestApiOverrides } from "@/browser/testUtils"; + +/* + * Panes keep what they show while Xum reconnects. That must not hide failures: + * - a server cancel on a live socket (a plain AbortError) is an error like any other, not a lost + * connection, since no new client will come to retry it. + */ + +let answerServers: () => Promise; +let answerOutput: () => Promise; + +/** A server cancel on a live socket: an abort that is not a lost connection. */ +function serverCancel(): Promise { + return Promise.reject(new DOMException("cancelled by the server", "AbortError")); +} + +function serverOverrides(): TestApiOverrides { + return { + mcp: { list: () => answerServers() }, + workspace: { backgroundBashes: { getOutput: () => answerOutput() } }, + } as unknown as TestApiOverrides; +} + +let harness: ConnectionHarness; + +function Wrapper(props: { children: ReactNode }) { + const Connection = harness.Wrapper; + return ( + + + {props.children} + + + ); +} + +async function flush() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); +} + +function noop() { + return undefined; +} + +describe.each([...GAP_MODES])("panes kept across a dropped connection (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + window.localStorage.clear(); + harness = createConnectionHarness(mode, serverOverrides); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("a server cancel on a live socket shows the MCP load error and ends its spinner", async () => { + answerServers = serverCancel; + const view = render(, { + wrapper: Wrapper, + }); + await flush(); + + expect(view.queryByText("cancelled by the server")).not.toBeNull(); + expect(view.container.querySelector(".animate-spin")).toBeNull(); + }); + + test("a server cancel on a live socket shows the process output error instead of loading", async () => { + answerOutput = serverCancel; + const view = render( + , + { wrapper: Wrapper } + ); + await flush(); + + expect(view.queryByText("cancelled by the server")).not.toBeNull(); + expect(view.queryByText("Loading…")).toBeNull(); + }); +}); diff --git a/src/browser/features/RightSidebar/useGoalBoard.ts b/src/browser/features/RightSidebar/useGoalBoard.ts index 09e9aac27be..625a2e5cb75 100644 --- a/src/browser/features/RightSidebar/useGoalBoard.ts +++ b/src/browser/features/RightSidebar/useGoalBoard.ts @@ -1,7 +1,7 @@ import { useCallback, useContext, useEffect, useState } from "react"; import { APIContext, type APIClient } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import type { GoalBoardSnapshot } from "@/common/types/goal"; /** @@ -34,7 +34,7 @@ async function readGoalBoard( try { return (await api.workspace.getGoalBoard({ workspaceId })) ?? EMPTY_BOARD; } catch (error) { - return isAbortError(error) ? null : EMPTY_BOARD; + return isConnectionLostError(error) ? null : EMPTY_BOARD; } } diff --git a/src/browser/features/Tools/SubagentTranscriptDialog.tsx b/src/browser/features/Tools/SubagentTranscriptDialog.tsx index fbaef44d71f..885206e5df8 100644 --- a/src/browser/features/Tools/SubagentTranscriptDialog.tsx +++ b/src/browser/features/Tools/SubagentTranscriptDialog.tsx @@ -14,7 +14,7 @@ import { ErrorBox, LoadingDots } from "./Shared/ToolPrimitives"; import { MessageRenderer } from "@/browser/features/Messages/MessageRenderer"; import { ModelDisplay } from "@/browser/features/Messages/ModelDisplay"; import { BashCollapsedSummaryModeProvider } from "./BashCollapsedSummaryModeContext"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { getErrorMessage } from "@/common/utils/errors"; interface SubagentTranscriptDialogProps { @@ -139,7 +139,7 @@ const SubagentTranscriptViewer: React.FC<{ } catch (err: unknown) { // A lost connection (or this effect's own cleanup) is not a transcript error: keep what // is shown for the next client. - if (cancelled || isAbortError(err)) return; + if (cancelled || isConnectionLostError(err)) return; setIsLoading(false); setError(getErrorMessage(err)); } From a1257a156e527e7bcfd7d8118c5c2837c95c9959 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 12:10:54 +0000 Subject: [PATCH 09/11] fix: kept panes do not look live while offline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Panes keep what they show while Xum reconnects, but controls that need the server did nothing: - Workflows: Run and Start are disabled without a live client. - Goal board: promote, remove, archive, revive, reorder, queue and edit report "Not connected" without a live client; an edit stays open. - Browser tab: picking another session while offline clears the previous session's page tabs. - Background process: a viewer opened offline (or whose first read was cut off) ends its loading state and shows the error; it shows "Loading…" again once a client reads. - DevTools: an expanded run whose detail read was cut off by the lost connection reads it again on the next client instead of keeping the connection error. --- .../BackgroundBashOutputDialog.tsx | 30 ++- .../BrowserTab/BrowserTab.reconnect.test.tsx | 32 ++- .../RightSidebar/BrowserTab/BrowserTab.tsx | 18 +- .../DevToolsTab/DevToolsRunCard.tsx | 91 ++++++--- .../RightSidebar/GoalBoardSections.tsx | 54 ++++-- .../Workflows/WorkflowEmptyState.tsx | 21 +- .../RightSidebar/Workflows/WorkflowsTab.tsx | 14 +- .../RightSidebar/offlinePanes.test.tsx | 183 +++++++++++++++++- 8 files changed, 366 insertions(+), 77 deletions(-) diff --git a/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx b/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx index 01b4f683670..8c44293a3c4 100644 --- a/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx +++ b/src/browser/components/BackgroundBashOutputDialog/BackgroundBashOutputDialog.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from "react"; +import React, { useEffect, useRef, useState } from "react"; import { CopyButton } from "../CopyButton/CopyButton"; import { Dialog, DialogContent, DialogHeader, DialogTitle } from "../Dialog/Dialog"; import { DetailContent } from "@/browser/features/Tools/Shared/ToolPrimitives"; @@ -60,6 +60,14 @@ export const BackgroundBashOutputDialog: React.FC ); +/** The output pane: "Loading…" until the first read, then the output or a placeholder. */ +function outputPaneText(state: { isLoading: boolean; text: string; hasError: boolean }): string { + if (state.isLoading) return "Loading…"; + if (state.text.length > 0) return state.text; + // The error shows above the pane; an empty pane under it reads better than "No output yet". + return state.hasError ? "" : "No output yet"; +} + const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: string }> = ( props ) => { @@ -77,16 +85,24 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str const [truncatedStart, setTruncatedStart] = useState(false); const [error, setError] = useState(null); const [isLoading, setIsLoading] = useState(true); + // True once a read landed: the viewer then shows output (or "No output yet"), not "Loading…". + const hasReadRef = useRef(false); useEffect(() => { - // No reset of the shown output here: the first read below replaces it. Before the first - // read lands, the viewer still shows "Loading…" (its initial state). + // No reset of the shown output here: the first read below replaces it. setError(null); if (!api) { + // Nothing can load without a client: a fresh viewer shows only the error, not a "Loading…" + // that never ends. A viewer that already read keeps its output. + setIsLoading(false); setError("API unavailable"); return; } + // Until the first read lands, the viewer shows "Loading…" (also after an offline start). + if (!hasReadRef.current) { + setIsLoading(true); + } let cancelled = false; @@ -119,6 +135,7 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str if (offset === undefined) { setTruncatedStart(result.data.truncatedStart); setOutput(undefined); + hasReadRef.current = true; } offset = result.data.nextOffset; @@ -147,7 +164,10 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str // server cancel on a live socket is a plain AbortError, and no new client would ever re-run // this poll, so it shows its error. run().catch((err: unknown) => { - if (cancelled || isConnectionLostError(err)) return; + if (cancelled) return; + // A viewer that already read keeps its output for the next client. A fresh one (its first + // read cut off) shows the error, not a "Loading…" with nothing behind it. + if (isConnectionLostError(err) && hasReadRef.current) return; setIsLoading(false); setError(getErrorMessage(err)); }); @@ -195,7 +215,7 @@ const BackgroundBashOutputViewer: React.FC<{ workspaceId: string; processId: str min-h keeps a visible pane for "No output yet" while still letting very short windows shrink it instead of clipping it at the dialog edge. */} - {isLoading ? "Loading…" : text.length > 0 ? text : error ? "" : "No output yet"} + {outputPaneText({ isLoading, text, hasError: error != null })} ); diff --git a/src/browser/features/RightSidebar/BrowserTab/BrowserTab.reconnect.test.tsx b/src/browser/features/RightSidebar/BrowserTab/BrowserTab.reconnect.test.tsx index cdca0d2f7e8..e4db35a45b3 100644 --- a/src/browser/features/RightSidebar/BrowserTab/BrowserTab.reconnect.test.tsx +++ b/src/browser/features/RightSidebar/BrowserTab/BrowserTab.reconnect.test.tsx @@ -2,7 +2,7 @@ import "../../../../../tests/ui/dom"; import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; -import { act, cleanup, render } from "@testing-library/react"; +import { act, cleanup, fireEvent, render } from "@testing-library/react"; import type { ReactNode } from "react"; import { installDom } from "../../../../../tests/ui/dom"; import { restoreModulesAfterSuite } from "../../../../../tests/ui/moduleMocks"; @@ -143,4 +143,34 @@ describe.each([...GAP_MODES])("BrowserTab across a dropped connection (%s)", (mo await view.findByTestId("browser-page-tab-t3"); expect(view.queryByTestId("browser-page-tab-t2")).toBeNull(); }); + + test("picking another session while reconnecting clears the previous session's page tabs", async () => { + answerSessions = () => + Promise.resolve({ + sessions: [ + { sessionName: "alpha", status: "attachable" }, + { sessionName: "beta", status: "attachable" }, + ], + otherSessions: [], + }); + const view = render(, { + wrapper: Wrapper, + }); + await view.findByTestId("browser-page-tab-t2"); + + await drop(); + fireEvent.click(view.getByText("alpha")); + fireEvent.click(view.getByTestId("browser-session-beta")); + await flush(); + expect(view.queryByTestId("browser-page-tab-t2")).toBeNull(); + + answerTabs = () => + Promise.resolve({ + tabs: [pageTab("b1", "Beta one", true), pageTab("b2", "Beta two", false)], + error: undefined, + }); + await reconnect(); + await view.findByTestId("browser-page-tab-b2"); + expect(view.queryByTestId("browser-page-tab-t2")).toBeNull(); + }); }); diff --git a/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx b/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx index eca8fff8769..03c1bf833bb 100644 --- a/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx +++ b/src/browser/features/RightSidebar/BrowserTab/BrowserTab.tsx @@ -246,8 +246,17 @@ export function BrowserTab(props: BrowserTabProps) { }, [api, props.workspaceId, setSelectedCurrentSessionName]); useEffect(() => { + // Another session never shows the previous session's tabs, even without a client: picking a + // kept discovered session while Xum reconnects clears the strip at once. + if (pageTabsSessionNameRef.current !== selectedSessionName) { + pageTabsSessionNameRef.current = selectedSessionName; + setPageTabs((currentTabs) => (currentTabs.length === 0 ? currentTabs : [])); + setPageTabsError(null); + pendingTabIdRef.current = null; + setPendingTabId(null); + } // Without a client (a reconnect), keep the tab strip of the selected session as it is: the - // new client re-lists its tabs, and only another session resets them below. + // new client re-lists its tabs. if (api == null) { tabRefreshInFlightSessionNameRef.current = null; return; @@ -263,13 +272,6 @@ export function BrowserTab(props: BrowserTabProps) { } let cancelled = false; - if (pageTabsSessionNameRef.current !== selectedSessionName) { - pageTabsSessionNameRef.current = selectedSessionName; - setPageTabs((currentTabs) => (currentTabs.length === 0 ? currentTabs : [])); - setPageTabsError(null); - pendingTabIdRef.current = null; - setPendingTabId(null); - } const refreshTabs = async (): Promise => { if (tabRefreshInFlightSessionNameRef.current === selectedSessionName) { diff --git a/src/browser/features/RightSidebar/DevToolsTab/DevToolsRunCard.tsx b/src/browser/features/RightSidebar/DevToolsTab/DevToolsRunCard.tsx index 9d9397534de..2e2b67b14bd 100644 --- a/src/browser/features/RightSidebar/DevToolsTab/DevToolsRunCard.tsx +++ b/src/browser/features/RightSidebar/DevToolsTab/DevToolsRunCard.tsx @@ -1,6 +1,7 @@ -import { useState } from "react"; +import { useEffect, useState } from "react"; import { AlertCircle, ChevronRight, Loader2 } from "lucide-react"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, type APIClient } from "@/browser/contexts/API"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { cn } from "@/common/lib/utils"; import type { DevToolsRunSummary, DevToolsStep } from "@/common/types/devtools"; import { formatDuration } from "@/common/utils/formatDuration"; @@ -12,44 +13,72 @@ interface DevToolsRunCardProps { liveSteps?: DevToolsStep[]; } -export function DevToolsRunCard(props: DevToolsRunCardProps) { - const { api } = useAPI(); +type RunDetailRead = + | { status: "ok"; steps: DevToolsStep[] } + | { status: "error"; error: string } + /** Cut off by a lost connection (ConnectionLostError): not an error, the next client retries. */ + | { status: "lost" }; - const [expanded, setExpanded] = useState(false); +async function readRunDetail( + api: APIClient, + workspaceId: string, + runId: string +): Promise { + try { + const result = await api.devtools.getRunDetail({ workspaceId, runId }); + return { status: "ok", steps: result?.steps ?? [] }; + } catch (error: unknown) { + if (isConnectionLostError(error)) return { status: "lost" }; + return { + status: "error", + error: error instanceof Error ? error.message : "Failed to load run detail", + }; + } +} + +/** + * The run's steps, read while the card is expanded and has none yet. A read cut off by a lost + * connection keeps the spinner, and the next client (a new `api`: the runs list stays mounted + * across a reconnect) reads it again instead of the card keeping a connection error. + */ +function useRunDetail(expanded: boolean, workspaceId: string, runId: string) { + const { api } = useAPI(); const [steps, setSteps] = useState(null); const [loading, setLoading] = useState(false); const [error, setError] = useState(null); - const displaySteps = mergeDisplaySteps(steps, props.liveSteps); - - const handleToggle = () => { - const nextExpanded = !expanded; - setExpanded(nextExpanded); - - if (!nextExpanded || steps !== null || !api) { + const needsDetail = expanded && steps === null; + useEffect(() => { + if (!needsDetail || !api) { return; } - + let cancelled = false; setLoading(true); setError(null); - api.devtools - .getRunDetail({ - workspaceId: props.workspaceId, - runId: props.run.id, - }) - .then((result) => { - if (!result) { - setSteps([]); - return; - } - setSteps(result.steps); - }) - .catch((detailError: unknown) => { - setError(detailError instanceof Error ? detailError.message : "Failed to load run detail"); - }) - .finally(() => { - setLoading(false); - }); + void readRunDetail(api, workspaceId, runId).then((read) => { + if (cancelled || read.status === "lost") return; + setLoading(false); + if (read.status === "ok") { + setSteps(read.steps); + } else { + setError(read.error); + } + }); + return () => { + cancelled = true; + }; + }, [api, needsDetail, workspaceId, runId]); + + return { steps, loading, error }; +} + +export function DevToolsRunCard(props: DevToolsRunCardProps) { + const [expanded, setExpanded] = useState(false); + const { steps, loading, error } = useRunDetail(expanded, props.workspaceId, props.run.id); + const displaySteps = mergeDisplaySteps(steps, props.liveSteps); + + const handleToggle = () => { + setExpanded(!expanded); }; return ( diff --git a/src/browser/features/RightSidebar/GoalBoardSections.tsx b/src/browser/features/RightSidebar/GoalBoardSections.tsx index db9dfcff262..620a6d6c19a 100644 --- a/src/browser/features/RightSidebar/GoalBoardSections.tsx +++ b/src/browser/features/RightSidebar/GoalBoardSections.tsx @@ -27,7 +27,8 @@ import { } from "lucide-react"; import { useRef, useState } from "react"; -import { useAPI } from "@/browser/contexts/API"; +import { useLiveAPI, type APIClient } from "@/browser/contexts/API"; +import { ConnectionLostError } from "@/browser/contexts/connectionLink"; import { useGoalDefaults } from "@/browser/utils/goals/useGoalDefaults"; import { loadGoalDefaults, resolveGoalSetIntent } from "@/browser/utils/goals/resolveGoalSetIntent"; import { cn } from "@/common/lib/utils"; @@ -57,6 +58,18 @@ import { * so clicking the row content doesn't fire a drag; `PointerSensor`'s * `distance: 6` activation keeps normal clicks intact. */ +/** + * The live client for a board change. While Xum reconnects the board stays on screen, so a click + * must not silently do nothing (or close an editor as if it saved): without a live client this + * throws, and each handler shows the error like any failed change. + */ +function liveClientFor(api: APIClient | null): APIClient { + if (api == null) { + throw new ConnectionLostError(); + } + return api; +} + interface GoalBoardSectionsProps { workspaceId: string; board: GoalBoardSnapshot; @@ -178,7 +191,7 @@ interface UpcomingSectionProps { } function UpcomingSection(props: UpcomingSectionProps) { - const { api } = useAPI(); + const api = useLiveAPI(); // Surfaces any backend rejection so users see why a click had no // effect. Cleared on next mutation. Backend errors arrive with the // server-side `error.message` intact via `ORPCError("BAD_REQUEST")` @@ -199,14 +212,14 @@ function UpcomingSection(props: UpcomingSectionProps) { const handleDragEnd = async (event: DragEndEvent) => { const { active, over } = event; - if (!api || !over || active.id === over.id) return; + if (!over || active.id === over.id) return; const ids = props.entries.map((e) => e.goal.goalId); const fromIndex = ids.indexOf(String(active.id)); const toIndex = ids.indexOf(String(over.id)); if (fromIndex === -1 || toIndex === -1) return; const next = arrayMove(ids, fromIndex, toIndex); try { - await api.workspace.reorderUpcomingGoals({ + await liveClientFor(api).workspace.reorderUpcomingGoals({ workspaceId: props.workspaceId, upcomingIds: next, }); @@ -218,9 +231,11 @@ function UpcomingSection(props: UpcomingSectionProps) { }; const promote = async (goalId: string) => { - if (!api) return; try { - await api.workspace.promoteUpcomingGoal({ workspaceId: props.workspaceId, goalId }); + await liveClientFor(api).workspace.promoteUpcomingGoal({ + workspaceId: props.workspaceId, + goalId, + }); clearError(); props.onMutated(); } catch (caught) { @@ -229,9 +244,8 @@ function UpcomingSection(props: UpcomingSectionProps) { }; const archive = async (goalId: string) => { - if (!api) return; try { - await api.workspace.archiveGoal({ workspaceId: props.workspaceId, goalId }); + await liveClientFor(api).workspace.archiveGoal({ workspaceId: props.workspaceId, goalId }); clearError(); props.onMutated(); } catch (caught) { @@ -240,9 +254,8 @@ function UpcomingSection(props: UpcomingSectionProps) { }; const update = async (goalId: string, patch: UpcomingGoalPatch) => { - if (!api) return; try { - await api.workspace.updateUpcomingGoal({ + await liveClientFor(api).workspace.updateUpcomingGoal({ workspaceId: props.workspaceId, goalId, objective: patch.objective, @@ -521,15 +534,14 @@ interface CompletedSectionProps { } function CompletedSection(props: CompletedSectionProps) { - const { api } = useAPI(); + const api = useLiveAPI(); const [error, setError] = useState(null); if (props.entries.length === 0) return null; const archive = async (goalId: string) => { - if (!api) return; try { - await api.workspace.archiveGoal({ workspaceId: props.workspaceId, goalId }); + await liveClientFor(api).workspace.archiveGoal({ workspaceId: props.workspaceId, goalId }); setError(null); props.onMutated(); } catch (caught) { @@ -575,14 +587,16 @@ interface ArchivedSectionProps { } function ArchivedSection(props: ArchivedSectionProps) { - const { api } = useAPI(); + const api = useLiveAPI(); const [error, setError] = useState(null); if (props.entries.length === 0) return null; const revive = async (goalId: string) => { - if (!api) return; try { - await api.workspace.reviveArchivedGoal({ workspaceId: props.workspaceId, goalId }); + await liveClientFor(api).workspace.reviveArchivedGoal({ + workspaceId: props.workspaceId, + goalId, + }); setError(null); props.onMutated(); } catch (caught) { @@ -624,7 +638,7 @@ interface UpcomingAdderProps { } function UpcomingAdder(props: UpcomingAdderProps) { - const { api } = useAPI(); + const api = useLiveAPI(); const { defaults } = useGoalDefaults(props.workspaceId); const [isOpen, setIsOpen] = useState(false); const [isSubmitting, setIsSubmitting] = useState(false); @@ -641,7 +655,6 @@ function UpcomingAdder(props: UpcomingAdderProps) { }; const submit = async () => { - if (!api) return; const objective = (objectiveRef.current?.value ?? "").trim(); if (objective.length === 0) { setError("Goal objective is required."); @@ -678,18 +691,19 @@ function UpcomingAdder(props: UpcomingAdderProps) { setIsSubmitting(true); setError(null); try { + const client = liveClientFor(api); // Load effective defaults at submit time so a quick submit before // the hook fetch resolves still persists the current workspace/global // limits. Fall back to the hook snapshot if the network read fails. let effective = defaults; try { - effective = await loadGoalDefaults(api, props.workspaceId); + effective = await loadGoalDefaults(client, props.workspaceId); } catch { // Keep `effective` at the hook value (which is itself a // best-effort snapshot or the canonical default). } const resolved = resolveGoalSetIntent({ objective, budgetCents, turnCap }, effective); - await api.workspace.addUpcomingGoal({ + await client.workspace.addUpcomingGoal({ workspaceId: props.workspaceId, objective: resolved.objective, budgetCents: resolved.budgetCents, diff --git a/src/browser/features/RightSidebar/Workflows/WorkflowEmptyState.tsx b/src/browser/features/RightSidebar/Workflows/WorkflowEmptyState.tsx index 52bc6b34921..484a23a3a97 100644 --- a/src/browser/features/RightSidebar/Workflows/WorkflowEmptyState.tsx +++ b/src/browser/features/RightSidebar/Workflows/WorkflowEmptyState.tsx @@ -140,6 +140,20 @@ interface WorkflowEmptyStateProps { // Keyed by scriptPath (unique) rather than descriptor.name, which can collide across skill // workflows that omit meta.name (both normalize to "workflow"). busyScriptPath: string | null; + /** No live connection: nothing can start, so Run and Start are disabled. */ + offline?: boolean; +} + +/** Run is disabled for a blocked script, while another script starts, and while offline. */ +function isRunDisabled( + script: AvailableWorkflow, + busyScriptPath: string | null, + offline: boolean +): boolean { + if (offline || !script.descriptor.executable) { + return true; + } + return busyScriptPath != null && busyScriptPath !== script.scriptPath; } /** @@ -149,6 +163,7 @@ interface WorkflowEmptyStateProps { */ export const WorkflowEmptyState: React.FC = (props) => { const [configuring, setConfiguring] = React.useState(null); + const offline = props.offline === true; return (
@@ -225,9 +240,7 @@ export const WorkflowEmptyState: React.FC = (props) =>
- )} + {runs.length > 0 && }
@@ -83,3 +53,39 @@ export function DevToolsTab(props: DevToolsTabProps) {
); } + +/** Clears the debug logs. It needs a live connection: while Xum reconnects it is disabled. */ +function ClearLogsButton(props: { workspaceId: string }) { + const api = useLiveAPI(); + const [clearing, setClearing] = useState(false); + + const handleClear = () => { + if (!api || clearing) return; + + setClearing(true); + void api.devtools + .clear({ workspaceId: props.workspaceId }) + .catch((clearError: unknown) => { + console.warn("Failed to clear debug logs:", clearError); + }) + .finally(() => { + setClearing(false); + }); + }; + + return ( + + ); +} diff --git a/src/browser/features/RightSidebar/GoalDefaultsSection.tsx b/src/browser/features/RightSidebar/GoalDefaultsSection.tsx index 344cf3b42bf..28c857b5ede 100644 --- a/src/browser/features/RightSidebar/GoalDefaultsSection.tsx +++ b/src/browser/features/RightSidebar/GoalDefaultsSection.tsx @@ -3,10 +3,11 @@ import React from "react"; import { useEffect, useRef, useState } from "react"; import { Input } from "@/browser/components/Input/Input"; -import { useAPI, type APIClient } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI, type APIClient } from "@/browser/contexts/API"; import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { GoalDefaultsControls } from "@/browser/features/Settings/Sections/GoalsSection"; import { cn } from "@/common/lib/utils"; +import { getErrorMessage } from "@/common/utils/errors"; import type { WorkspaceGoalDefaultsOverride } from "@/browser/utils/goals/resolveGoalSetIntent"; import { mergeGoalDefaults } from "@/browser/utils/goals/resolveGoalSetIntent"; import { DEFAULT_GOAL_DEFAULTS, normalizeGoalDefaults, type GoalDefaults } from "@/constants/goals"; @@ -72,6 +73,7 @@ interface GoalOverrideWrite { workspaceId: string; override: WorkspaceGoalDefaultsOverride; onSaved: (() => void) | undefined; + onFailed: (error: unknown) => void; } /** @@ -80,33 +82,21 @@ interface GoalOverrideWrite { * complete after a later edit and overwrite it on disk while the UI continues to show the latest * optimistic state, silently reverting the user's edit on next refetch. Each write waits for the * previous one. - * - * The controls stay editable while Xum reconnects. A write that no client received (made without - * a client, or cut off by the lost connection) is kept until `sendUnsent` hands it to the next - * client. Otherwise the edit would look saved but never reach the backend. */ class GoalOverrideWriter { private chain: Promise = Promise.resolve(); - private latest: WorkspaceGoalDefaultsOverride | null = null; - private unsent: GoalOverrideWrite | null = null; private readonly writesByWorkspace = new Map(); /** - * How many writes this workspace has had. A read that began before a write must not show its - * (older) answer over the written value. + * How many writes this workspace has had. A read that began before a write (a reload by a new + * client after a reconnect) must not show its older answer over the written value. */ writeCount(workspaceId: string): number { return this.writesByWorkspace.get(workspaceId) ?? 0; } - write(api: APIClient | null, edit: GoalOverrideWrite): void { + write(api: APIClient, edit: GoalOverrideWrite): void { this.writesByWorkspace.set(edit.workspaceId, this.writeCount(edit.workspaceId) + 1); - this.latest = edit.override; - if (api == null) { - this.unsent = edit; - return; - } - this.unsent = null; this.chain = this.chain.then(() => api.workspace.goalDefaults .set({ @@ -115,34 +105,37 @@ class GoalOverrideWriter { defaultTurnCap: edit.override.defaultTurnCap, alwaysRequireExplicitBudget: edit.override.alwaysRequireExplicitBudget, }) - .then(() => { - // Notify parent so prefilled create-form inputs can re-read the effective defaults. - edit.onSaved?.(); - }) - .catch((error: unknown) => { - // Other errors are swallowed (the optimistic edit already updated local state), as - // in the global panel. A newer edit replaces a lost one. - if (isConnectionLostError(error) && this.latest === edit.override) { - this.unsent = edit; - } - }) + // Notify parent so prefilled create-form inputs can re-read the effective defaults. + .then(() => edit.onSaved?.(), edit.onFailed) ); } +} - /** Sends the write the last client did not get to `api`. */ - sendUnsent(api: APIClient): void { - if (this.unsent != null) { - this.write(api, this.unsent); - } +type OverrideRead = + | { status: "ok"; override: WorkspaceGoalDefaultsOverride | null } + | { status: "failed" } + /** Cut off by a lost connection (ConnectionLostError): the next client reads it again. */ + | { status: "lost" }; + +async function readOverride(api: APIClient, workspaceId: string): Promise { + try { + return { + status: "ok", + override: (await api.workspace.goalDefaults.get({ workspaceId })) ?? null, + }; + } catch (error: unknown) { + return isConnectionLostError(error) ? { status: "lost" } : { status: "failed" }; } } /** * The workspace's stored override, read once per workspace. The panel edits it optimistically - * through `edit`, which also writes it. + * through `edit`, which also writes it. Writes need a live connection: while Xum reconnects the + * override stays on screen but `readOnly` is true, and nothing waits for a later client. */ function useWorkspaceGoalOverride(workspaceId: string, onPersist: (() => void) | undefined) { const { api } = useAPI(); + const liveApi = useLiveAPI(); const [override, setOverride] = useState(null); // Distinguishes "loaded; no override stored" (override === null, isLoading // === false) from "still loading" (override === null, isLoading === true). @@ -151,10 +144,11 @@ function useWorkspaceGoalOverride(workspaceId: string, onPersist: (() => void) | // which `set` then writes — clobbering any saved override on disk. // Preserve saved workspace defaults while loading. const [isLoading, setIsLoading] = useState(true); + // A write that failed (a lost connection included): the shown value may not be stored. + const [saveError, setSaveError] = useState(null); // The workspace whose override is loaded. A re-read for it (a new API client after a reconnect) - // keeps the controls shown and enabled; only another workspace resets them. + // keeps the controls shown; only another workspace resets them. const loadedWorkspaceIdRef = useRef(null); - // Also counts the user's edits, so an edit made while a re-read is in flight wins over its result. const [writer] = useState(() => new GoalOverrideWriter()); // Workspace override is loaded once per workspaceId. `null` (the @@ -165,42 +159,40 @@ function useWorkspaceGoalOverride(workspaceId: string, onPersist: (() => void) | if (loadedWorkspaceIdRef.current !== workspaceId) { loadedWorkspaceIdRef.current = null; setOverride(null); + setSaveError(null); setIsLoading(true); } if (!api) return; let cancelled = false; const writesAtStart = writer.writeCount(workspaceId); - // An edit the last client did not get goes to this one first. It counts as a write after the - // read began, so the read's answer (maybe still the older stored value) does not show over it. - writer.sendUnsent(api); - void api.workspace.goalDefaults - .get({ workspaceId }) - .then((value) => { - if (cancelled) return; - if (writer.writeCount(workspaceId) === writesAtStart) { - setOverride(value ?? null); - } - setIsLoading(false); - loadedWorkspaceIdRef.current = workspaceId; - }) - .catch((error: unknown) => { - // A lost connection keeps what is shown for the next client. Another failure enables - // the controls on what is shown (the empty override on a first read), as before. - if (cancelled || isConnectionLostError(error)) return; - setIsLoading(false); - loadedWorkspaceIdRef.current = workspaceId; - }); + void readOverride(api, workspaceId).then((read) => { + // A lost connection keeps what is shown for the next client. Another failure enables the + // controls on what is shown (the empty override on a first read), as before. + if (cancelled || read.status === "lost") return; + if (read.status === "ok" && writer.writeCount(workspaceId) === writesAtStart) { + setOverride(read.override); + } + setIsLoading(false); + loadedWorkspaceIdRef.current = workspaceId; + }); return () => { cancelled = true; }; }, [api, workspaceId, writer]); const edit = (next: WorkspaceGoalDefaultsOverride) => { + if (liveApi == null) return; setOverride(next); - writer.write(api, { workspaceId, override: next, onSaved: onPersist }); + setSaveError(null); + writer.write(liveApi, { + workspaceId, + override: next, + onSaved: onPersist, + onFailed: (error) => setSaveError(getErrorMessage(error)), + }); }; - return { override, isLoading, edit }; + return { override, isLoading, readOnly: liveApi == null, saveError, edit }; } export function GoalDefaultsSection(props: GoalDefaultsSectionProps) { @@ -209,7 +201,10 @@ export function GoalDefaultsSection(props: GoalDefaultsSectionProps) { ...DEFAULT_GOAL_DEFAULTS, })); const onPersistProp = props.onPersist; - const { override, isLoading, edit } = useWorkspaceGoalOverride(props.workspaceId, onPersistProp); + const { override, isLoading, readOnly, saveError, edit } = useWorkspaceGoalOverride( + props.workspaceId, + onPersistProp + ); // Pull the global defaults so we can render inherited values inside the // workspace-override panel. We only re-read on mount/api change; updates @@ -267,7 +262,14 @@ export function GoalDefaultsSection(props: GoalDefaultsSectionProps) { globalDefaults={globalDefaults} onChange={persistOverride} isLoading={isLoading} + // Off while loading, and while there is no live connection to save to. + disabled={isLoading || readOnly} /> + {saveError != null && ( +

+ {saveError} +

+ )}
@@ -340,6 +342,7 @@ interface WorkspaceOverridePanelProps { globalDefaults: GoalDefaults; onChange: (next: WorkspaceGoalDefaultsOverride) => void; isLoading: boolean; + disabled: boolean; } function WorkspaceOverridePanel(props: WorkspaceOverridePanelProps) { @@ -358,14 +361,14 @@ function WorkspaceOverridePanel(props: WorkspaceOverridePanelProps) { override={props.override.defaultBudgetCents} inheritValue={props.globalDefaults.defaultBudgetCents} onChange={(value) => props.onChange({ ...props.override, defaultBudgetCents: value })} - disabled={props.isLoading} + disabled={props.disabled} /> props.onChange({ ...props.override, defaultTurnCap: value })} - disabled={props.isLoading} + disabled={props.disabled} /> props.onChange({ ...props.override, alwaysRequireExplicitBudget: value }) } - disabled={props.isLoading} + disabled={props.disabled} /> ); @@ -435,6 +438,7 @@ function BudgetOverrideRow(props: BudgetOverrideRowProps) { onChange={(event) => setDraft(event.target.value)} onBlur={(event) => commit(event.currentTarget.value)} className="border-border-medium bg-background-secondary h-8 w-20 text-right text-sm" + disabled={props.disabled} /> } @@ -513,6 +517,7 @@ function TurnCapOverrideRow(props: TurnCapOverrideRowProps) { onChange={(event) => setDraft(event.target.value)} onBlur={(event) => commit(event.currentTarget.value)} className="border-border-medium bg-background-secondary h-8 w-20 text-right text-sm" + disabled={props.disabled} /> } /> @@ -549,6 +554,7 @@ function ExplicitBudgetOverrideRow(props: ExplicitBudgetOverrideRowProps) { checked={props.override ?? false} onChange={(event) => props.onChange(event.target.checked)} className="accent-accent h-4 w-4" + disabled={props.disabled} /> {(props.override ?? false) ? "ON" : "OFF"} diff --git a/src/browser/features/RightSidebar/Workflows/WorkflowRunHeader.tsx b/src/browser/features/RightSidebar/Workflows/WorkflowRunHeader.tsx index ed770e40c54..7d63684dcd6 100644 --- a/src/browser/features/RightSidebar/Workflows/WorkflowRunHeader.tsx +++ b/src/browser/features/RightSidebar/Workflows/WorkflowRunHeader.tsx @@ -1,7 +1,7 @@ import React from "react"; import { Check, ChevronRight, Clock, Coins, Play, RotateCcw, Square, Workflow } from "lucide-react"; -import { useAPI } from "@/browser/contexts/API"; +import { useLiveAPI } from "@/browser/contexts/API"; import type { WorkflowRunRecord } from "@/common/types/workflow"; import { canRetryWorkflowFromCheckpoint } from "@/common/utils/workflowRetryEligibility"; @@ -48,8 +48,11 @@ interface WorkflowRunHeaderProps { } export const WorkflowRunHeader: React.FC = (props) => { - const { api } = useAPI(); + // Run actions need a live connection. While Xum reconnects the run stays on screen, but its + // actions are disabled (`blocked`), so a click never silently does nothing. + const api = useLiveAPI(); const [busy, setBusy] = React.useState(false); + const blocked = busy || api == null; const [actionError, setActionError] = React.useState(null); const run = props.run; @@ -62,7 +65,7 @@ export const WorkflowRunHeader: React.FC = (props) => { const canRerun = rerunScriptPath != null; const runAction = async (action: () => Promise) => { - if (api == null || busy) { + if (blocked) { return; } setBusy(true); @@ -222,16 +225,16 @@ export const WorkflowRunHeader: React.FC = (props) => {
{isLive ? ( - ) : run.status === "interrupted" ? ( <> - {canRerun && ( - )} @@ -239,12 +242,12 @@ export const WorkflowRunHeader: React.FC = (props) => { ) : ( <> {canRetry && ( - )} {canRerun && ( - )} diff --git a/src/browser/features/RightSidebar/loadersReconnect.test.tsx b/src/browser/features/RightSidebar/loadersReconnect.test.tsx index 160ada96aa8..c0eca4a12c7 100644 --- a/src/browser/features/RightSidebar/loadersReconnect.test.tsx +++ b/src/browser/features/RightSidebar/loadersReconnect.test.tsx @@ -251,25 +251,25 @@ describe.each([...GAP_MODES])("edits and navigation while reconnecting (%s)", (m cleanupDom = null; }); - test("a chat-instructions edit made while reconnecting reaches the next client", async () => { - // The instructions store is per workspace and process-wide: a fresh workspace per mode. + test("the chat-instructions editor is read-only while offline, and editable again after the reconnect", async () => { const workspaceId = `ws-offline-edit-${mode}`; + answerInstructions = () => Promise.resolve({ files: [], totalTokens: 0 }); answerChatInstructions = () => Promise.resolve({ content: "be brief", enabled: true }); - const view = renderHook(() => useChatInstructions(workspaceId), { wrapper: Wrapper }); - await waitFor(() => expect(view.result.current.content).toBe("be brief")); + const view = render(, { wrapper: Wrapper }); + const editor = (await view.findByLabelText("Chat Instructions editor")) as HTMLTextAreaElement; + const toggle = view.getByLabelText("Enable Chat Instructions") as HTMLInputElement; + await waitFor(() => expect(editor.value).toBe("be brief")); + expect(editor.disabled).toBe(false); await drop(); - act(() => view.result.current.setContent("be kind")); - await flush(); - expect(view.result.current.error).toBeNull(); + expect(editor.value).toBe("be brief"); + expect(editor.disabled).toBe(true); + expect(toggle.disabled).toBe(true); - // The next server still stores the old text: the edit must reach it and stay on screen. await reconnect(); - await waitFor(() => - expect(savedChatInstructions).toEqual([{ content: "be kind", enabled: true }]) - ); - expect(view.result.current.content).toBe("be kind"); - expect(view.result.current.error).toBeNull(); + await waitFor(() => expect(editor.disabled).toBe(false)); + expect(toggle.disabled).toBe(false); + expect(savedChatInstructions).toEqual([]); }); test("after a failed save, a reconnect shows what the server stored", async () => { @@ -287,25 +287,27 @@ describe.each([...GAP_MODES])("edits and navigation while reconnecting (%s)", (m await waitFor(() => expect(view.result.current.content).toBe("be brief")); }); - test("a goal-defaults edit made while reconnecting reaches the next client", async () => { + test("the goal defaults are read-only while offline, and editable again after the reconnect", async () => { answerOverride = () => Promise.resolve(turnCapOverride(5)); const view = render(, { wrapper: Wrapper, }); const turnCap = () => view.getByLabelText("Workspace default goal turn cap") as HTMLInputElement; + const stopOverriding = () => + view.getByRole("button", { name: "Stop overriding Default turn cap" }) as HTMLButtonElement; await waitFor(() => expect(turnCap().value).toBe("5")); + expect(turnCap().disabled).toBe(false); await drop(); - fireEvent.change(turnCap(), { target: { value: "9" } }); - fireEvent.blur(turnCap()); - await flush(); + expect(turnCap().value).toBe("5"); + expect(turnCap().disabled).toBe(true); + expect(stopOverriding().disabled).toBe(true); - // The next server still stores 5: the edit must reach it, and its read must not undo it. await reconnect(); - await waitFor(() => expect(savedOverrides).toHaveLength(1)); - expect(savedOverrides[0]).toMatchObject({ workspaceId: "ws-1", defaultTurnCap: 9 }); - expect(turnCap().value).toBe("9"); + await waitFor(() => expect(turnCap().disabled).toBe(false)); + expect(stopOverriding().disabled).toBe(false); + expect(savedOverrides).toEqual([]); }); test("the MCP overview never shows one project's servers under another", async () => { diff --git a/src/browser/features/RightSidebar/offlinePanes.test.tsx b/src/browser/features/RightSidebar/offlinePanes.test.tsx index 094e8a0acc0..32cd9be9ed3 100644 --- a/src/browser/features/RightSidebar/offlinePanes.test.tsx +++ b/src/browser/features/RightSidebar/offlinePanes.test.tsx @@ -18,8 +18,11 @@ import { SettingsProvider } from "@/browser/contexts/SettingsContext"; import type { TestApiOverrides } from "@/browser/testUtils"; import type { DevToolsRunSummary } from "@/common/types/devtools"; import type { GoalBoardSnapshot } from "@/common/types/goal"; -import type { AvailableWorkflow } from "@/common/types/workflow"; +import type { AvailableWorkflow, WorkflowRunRecord } from "@/common/types/workflow"; import { DevToolsRunCard } from "./DevToolsTab/DevToolsRunCard"; +import { DevToolsTab } from "./DevToolsTab/DevToolsTab"; +import { projectWorkflowRun } from "./Workflows/projectWorkflowRun"; +import { WorkflowRunHeader } from "./Workflows/WorkflowRunHeader"; import { GoalBoardSections } from "./GoalBoardSections"; import { WorkflowsTab } from "./Workflows/WorkflowsTab"; @@ -82,7 +85,17 @@ function serverOverrides(): TestApiOverrides { listScripts: () => Promise.resolve([workflowScript()]), start: mutation("start"), }, - devtools: { getRunDetail: () => answerRunDetail() }, + devtools: { + getRunDetail: () => answerRunDetail(), + subscribe: () => + Promise.resolve( + (async function* () { + yield { type: "snapshot", runs: [devToolsRun()] }; + await new Promise(() => undefined); + })() + ), + clear: mutation("clear"), + }, config: { getConfig: () => Promise.resolve({}) }, } as unknown as TestApiOverrides; } @@ -130,6 +143,22 @@ function devToolsRun(): DevToolsRunSummary { } as unknown as DevToolsRunSummary; } +function workflowRun(status: WorkflowRunRecord["status"]): WorkflowRunRecord { + return { + id: "wfr_test", + workspaceId: "ws-1", + workflow: { name: "deploy", description: "deploy", scope: "project", executable: true }, + source: "export default function workflow() { return null; }", + sourceHash: "sha256:test", + args: {}, + status, + createdAt: "2026-05-29T00:00:00.000Z", + updatedAt: "2026-05-29T00:00:01.000Z", + events: [], + steps: [], + }; +} + let harness: ConnectionHarness; function Wrapper(props: { children: ReactNode }) { @@ -275,4 +304,66 @@ describe.each([...GAP_MODES])("panes kept across a dropped connection (%s)", (mo await view.findByText("No steps recorded"); expect(view.queryByText(/Not connected/)).toBeNull(); }); + + test("the run header's actions are disabled while there is no live client", async () => { + const live = workflowRun("running"); + const interrupted = workflowRun("interrupted"); + const view = render( + <> + + + , + { wrapper: Wrapper } + ); + const interrupt = () => view.getByRole("button", { name: /Interrupt/ }) as HTMLButtonElement; + const resume = () => view.getByRole("button", { name: /Resume/ }) as HTMLButtonElement; + expect(interrupt().disabled).toBe(false); + expect(resume().disabled).toBe(false); + + await drop(); + expect(interrupt().disabled).toBe(true); + expect(resume().disabled).toBe(true); + + await reconnect(); + expect(interrupt().disabled).toBe(false); + expect(resume().disabled).toBe(false); + }); + + test("DevTools Clear is disabled while there is no live client", async () => { + const view = render(, { wrapper: Wrapper }); + const clear = () => view.getByRole("button", { name: "Clear debug logs" }) as HTMLButtonElement; + await view.findByText("hello"); + expect(clear().disabled).toBe(false); + + await drop(); + expect(view.queryByText("hello")).not.toBeNull(); + expect(clear().disabled).toBe(true); + + await reconnect(); + expect(clear().disabled).toBe(false); + fireEvent.click(clear()); + await waitFor(() => expect(mutations).toEqual(["clear"])); + }); + + test("a DevTools run expanded while reconnecting waits for its detail instead of showing none", async () => { + answerRunDetail = () => Promise.resolve(null); + const view = render(, { + wrapper: Wrapper, + }); + await drop(); + // In the kept gap the kept client's read fails at once with a lost connection. + answerRunDetail = pending; + fireEvent.click(view.getByRole("button", { name: /hello/ })); + await flush(); + expect(view.queryByText("No steps recorded")).toBeNull(); + expect(view.container.querySelector(".animate-spin")).not.toBeNull(); + + answerRunDetail = () => Promise.resolve(null); + await reconnect(); + await view.findByText("No steps recorded"); + }); });