From ba213d685dca7e7505071a33c7727bb800273317 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 10:01:14 +0000 Subject: [PATCH 01/14] refactor: project and workspace contexts keep shown data while reloading --- src/browser/contexts/ProjectContext.tsx | 16 +++-- .../contexts/WorkspaceContext.test.tsx | 63 +++++++++++++++++++ src/browser/contexts/WorkspaceContext.tsx | 22 ++++--- 3 files changed, 89 insertions(+), 12 deletions(-) diff --git a/src/browser/contexts/ProjectContext.tsx b/src/browser/contexts/ProjectContext.tsx index 594f3def422..96f7e1af3f1 100644 --- a/src/browser/contexts/ProjectContext.tsx +++ b/src/browser/contexts/ProjectContext.tsx @@ -19,6 +19,7 @@ import { getDraftScopeId } from "@/common/constants/storage"; import { deleteWorkspaceStorage } from "@/browser/utils/workspaceStorage"; import { getDraftStore } from "@/browser/stores/DraftStore"; import { getErrorMessage } from "@/common/utils/errors"; +import { isAbortError } from "@/browser/utils/isAbortError"; import type { ProjectWorkspaceCounts } from "@/common/utils/projectRemoval"; import { getProjectRouteId } from "@/common/utils/projectRouteId"; import { SCRATCH_PROJECT_CONFIG_KEY } from "@/common/constants/scratch"; @@ -136,6 +137,10 @@ function toNonEmptyTrimmed(value?: string | null): string | null { return trimmed && trimmed.length > 0 ? trimmed : null; } +function findSystemProjectPath(projects: Map): string | null { + return [...projects.entries()].find(([, cfg]) => cfg.projectKind === "system")?.[0] ?? null; +} + export function ProjectProvider(props: { children: ReactNode }) { const { api } = useAPI(); // Handlers use only the live client: offline they keep returning their result (for example @@ -149,9 +154,7 @@ export function ProjectProvider(props: { children: ReactNode }) { [allProjectsInternal] ); const systemProjectPath = useMemo( - () => - [...allProjectsInternal.entries()].find(([, cfg]) => cfg.projectKind === "system")?.[0] ?? - null, + () => findSystemProjectPath(allProjectsInternal), [allProjectsInternal] ); const [loading, setLoading] = useState(true); @@ -180,7 +183,7 @@ export function ProjectProvider(props: { children: ReactNode }) { const refresh = projectsRefreshRef.current; if (refresh.api !== api) return Promise.resolve(); if (!api) { - setLoaded(false); + // Keep `loaded` and the shown projects: the reconnect reloads them (stale-while-revalidate). setLoadError("API not connected"); return Promise.resolve(); } @@ -200,6 +203,8 @@ export function ProjectProvider(props: { children: ReactNode }) { setLoadError(null); } catch (error) { if (refresh !== projectsRefreshRef.current || !refresh.api) continue; + // A lost connection is no load error: the reconnect reloads. + if (isAbortError(error)) continue; // Keep successful data when a later refresh fails. console.error("Failed to load projects:", error); setLoadError(getErrorMessage(error)); @@ -598,7 +603,8 @@ export function ProjectProvider(props: { children: ReactNode }) { hasAnyProject, resolveNewChatProjectPath, getProjectConfig, - loading, + // Only the first load counts: a reload after a reconnect keeps the loaded projects shown. + loading: loading && !loaded, loaded, loadError, refreshProjects, diff --git a/src/browser/contexts/WorkspaceContext.test.tsx b/src/browser/contexts/WorkspaceContext.test.tsx index d46efe1a1fa..db8f22aa341 100644 --- a/src/browser/contexts/WorkspaceContext.test.tsx +++ b/src/browser/contexts/WorkspaceContext.test.tsx @@ -1003,6 +1003,69 @@ describe("WorkspaceContext", () => { }); }); + // A reconnect reloads projects and metadata through the new client. Until the reload answers, + // both contexts keep what they show and do not report "loading" again (stale-while-revalidate). + describe.each([...GAP_MODES])("reload after a reconnect (%s)", (mode) => { + test("keeps the loaded projects and metadata until the reload answers", async () => { + createMockAPI({ + workspace: { list: () => Promise.resolve([createWorkspaceMetadata({ id: "ws-old" })]) }, + projects: { list: () => Promise.resolve([["/old", { workspaces: [] }]]) }, + }); + const contexts = await setupWithConnection(mode); + await waitFor(() => expect(contexts.project().userProjects.has("/old")).toBe(true)); + expect(contexts.workspace().workspaceMetadata.has("ws-old")).toBe(true); + + // The next server answers only when the test lets it. + let answerProjects: (() => void) | null = null; + let answerSnapshot: (() => void) | null = null; + const projectsAnswered = new Promise((resolve) => (answerProjects = resolve)); + const snapshotAnswered = new Promise((resolve) => (answerSnapshot = resolve)); + currentClientMock = { + ...currentClientMock, + projects: { + ...currentClientMock.projects, + list: () => projectsAnswered.then(() => [["/new", { workspaces: [] }]]), + }, + workspace: { + ...currentClientMock.workspace, + onMetadata: () => + Promise.resolve( + (async function* () { + await snapshotAnswered; + yield { + type: "snapshot" as const, + workspaces: [createWorkspaceMetadata({ id: "ws-new" })], + }; + await new Promise(() => undefined); + })() + ), + }, + }; + + await contexts.harness.drop(); + contexts.harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + + expect(contexts.project().loading).toBe(false); + expect(contexts.project().loaded).toBe(true); + expect(contexts.project().userProjects.has("/old")).toBe(true); + expect(contexts.workspace().loading).toBe(false); + expect(contexts.workspace().loaded).toBe(true); + expect(contexts.workspace().workspaceMetadata.has("ws-old")).toBe(true); + + act(() => { + answerSnapshot?.(); + answerProjects?.(); + }); + await waitFor(() => expect(contexts.workspace().workspaceMetadata.has("ws-new")).toBe(true)); + await waitFor(() => expect(contexts.project().userProjects.has("/new")).toBe(true)); + expect(contexts.workspace().workspaceMetadata.has("ws-old")).toBe(false); + expect(contexts.project().userProjects.has("/old")).toBe(false); + }); + }); + describe("archiveWorkspace", () => { test("succeeds even when persisted layout is invalid JSON shape", async () => { const workspaceId = "ws-archive-invalid-layout"; diff --git a/src/browser/contexts/WorkspaceContext.tsx b/src/browser/contexts/WorkspaceContext.tsx index 38dabf5c7cc..2a787e1a15a 100644 --- a/src/browser/contexts/WorkspaceContext.tsx +++ b/src/browser/contexts/WorkspaceContext.tsx @@ -406,6 +406,15 @@ function buildActiveWorkspaceMetadataMap( return metadataMap; } +/** The workspace IDs whose last metadata update arrived after `arrival` (an arrival count). */ +function getIdsArrivedAfter(lastByWorkspaceId: Map, arrival: number): Set { + const ids = new Set(); + for (const [workspaceId, arrivedAt] of lastByWorkspaceId) { + if (arrivedAt > arrival) ids.add(workspaceId); + } + return ids; +} + export function WorkspaceProvider(props: WorkspaceProviderProps) { const { api } = useAPI(); // Handlers use only the live client: offline they keep returning their result (for example @@ -892,10 +901,10 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { // The metadata stream owns the initial load: its first event is a snapshot built after the // server attached the listener, so no change can fall between the load and the updates (#5189). useEffect(() => { - setLoading(true); + // `loading` starts true and stays false after the first load: a new client (reconnect) + // reloads behind the shown metadata, so neither `loading` nor `loaded` resets here. if (!api) { // api not available yet - effect will run again when api connects - setLoaded(false); setLoadError("API not connected"); return; } @@ -1545,7 +1554,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { const refreshWorkspaceMetadata = useCallback(async () => { if (!api) { - setLoaded(false); + // Keep `loaded` and the shown metadata: the reconnect reloads them. setLoadError("API not connected"); return; } @@ -1556,10 +1565,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { // A snapshot that arrived after the request, plus the updates after it, supersedes this list. if (arrivals.lastSnapshot > requestedAt) return; // Rows an update touched after the request keep their current state, including a deletion. - const touchedIds = new Set(); - for (const [workspaceId, arrivedAt] of arrivals.lastByWorkspaceId) { - if (arrivedAt > requestedAt) touchedIds.add(workspaceId); - } + const touchedIds = getIdsArrivedAfter(arrivals.lastByWorkspaceId, requestedAt); const listed = buildActiveWorkspaceMetadataMap( metadataList.filter((metadata) => !touchedIds.has(metadata.id)) ); @@ -1574,6 +1580,8 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { setLoaded(true); setLoadError(null); } catch (error) { + // A lost connection is no load error: the reconnect reloads. + if (isAbortError(error)) return; console.error("Failed to load workspace metadata:", error); // Keep the previous metadata map on failure so scoped preferences are not pruned. setLoadError(getErrorMessage(error)); From d9d713f7e5a42fd9a9c728b8bfeb7702e0e8b72a Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 10:14:31 +0000 Subject: [PATCH 02/14] refactor: agent and layout contexts keep shown data while reloading --- src/browser/contexts/AgentContext.test.tsx | 81 ++++++++++++++++++ src/browser/contexts/AgentContext.tsx | 44 ++++++---- .../contexts/UILayoutsContext.test.tsx | 83 +++++++++++++++++++ src/browser/contexts/UILayoutsContext.tsx | 24 ++++-- 4 files changed, 208 insertions(+), 24 deletions(-) create mode 100644 src/browser/contexts/UILayoutsContext.test.tsx diff --git a/src/browser/contexts/AgentContext.test.tsx b/src/browser/contexts/AgentContext.test.tsx index 403939e9c48..1f20aaeca82 100644 --- a/src/browser/contexts/AgentContext.test.tsx +++ b/src/browser/contexts/AgentContext.test.tsx @@ -12,6 +12,7 @@ import type { AgentDefinitionDescriptor } from "@/common/types/agentDefinition"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import { AgentProvider, useAgent, type AgentContextValue } from "./AgentContext"; import { APIProvider, type APIClient } from "./API"; +import { createConnectionHarness, GAP_MODES, type ConnectionHarness } from "./connection.testHarness"; import { ProjectProvider } from "./ProjectContext"; import { RouterProvider } from "./RouterContext"; import { WorkspaceProvider } from "./WorkspaceContext"; @@ -19,6 +20,7 @@ import { createTestApiClient, createTestConfig, createTestPreferencesConfig, + type TestApiOverrides, } from "@/browser/testUtils"; import type { AgentAiDefaults } from "@/common/types/agentAiDefaults"; import { getWorkspaceAiSelection } from "@/browser/utils/workspaceAiSettingsSync"; @@ -512,3 +514,82 @@ describe("AgentContext", () => { } }); }); + +// A reconnect brings a new client, so the agents load again. The picker keeps the agents it shows +// until that reload answers; only another workspace or project clears them. +describe.each([...GAP_MODES])("AgentContext across a reconnect (%s)", (mode) => { + let originalWindow: typeof globalThis.window; + let originalDocument: typeof globalThis.document; + let harness: ConnectionHarness; + /** The agents the next server lists, or a promise that answers later. */ + let serverAgents: AgentDefinitionDescriptor[] | Promise; + let latest: AgentContextValue | undefined; + + beforeEach(() => { + originalWindow = globalThis.window; + originalDocument = globalThis.document; + const dom = new GlobalWindow(); + globalThis.window = dom as unknown as Window & typeof globalThis; + globalThis.document = dom.document as unknown as Document; + latest = undefined; + serverAgents = [EXEC_AGENT]; + harness = createConnectionHarness(mode, (): TestApiOverrides => { + const answer = serverAgents; + return { agents: { list: () => Promise.resolve(answer) } }; + }); + }); + + afterEach(() => { + cleanup(); + globalThis.window = originalWindow; + globalThis.document = originalDocument; + }); + + function renderAgents(workspaceId: string) { + const tree = (id: string) => ( + + + (latest = value)} /> + + + ); + const view = render(tree(workspaceId)); + return { rerender: (id: string) => view.rerender(tree(id)) }; + } + + test("keeps the shown agents until the reload answers", async () => { + renderAgents("ws-a"); + await waitFor(() => expect(latest?.agents.map((agent) => agent.id)).toEqual(["exec"])); + + let answer: ((agents: AgentDefinitionDescriptor[]) => void) | null = null; + serverAgents = new Promise((resolve) => (answer = resolve)); + await harness.drop(); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + + expect(latest?.loaded).toBe(true); + expect(latest?.loadFailed).toBe(false); + expect(latest?.agents.map((agent) => agent.id)).toEqual(["exec"]); + + act(() => answer?.([EXEC_AGENT, PLAN_AGENT])); + await waitFor(() => expect(latest?.agents.map((agent) => agent.id)).toEqual(["exec", "plan"])); + }); + + test("a workspace switch while offline clears the shown agents", async () => { + const view = renderAgents("ws-a"); + await waitFor(() => expect(latest?.agents.map((agent) => agent.id)).toEqual(["exec"])); + + await harness.drop(); + view.rerender("ws-b"); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + expect(latest?.agents).toEqual([]); + + serverAgents = [PLAN_AGENT]; + harness.reconnect(); + await waitFor(() => expect(latest?.agents.map((agent) => agent.id)).toEqual(["plan"])); + }); +}); diff --git a/src/browser/contexts/AgentContext.tsx b/src/browser/contexts/AgentContext.tsx index 0ea751882ae..45493b8aa80 100644 --- a/src/browser/contexts/AgentContext.tsx +++ b/src/browser/contexts/AgentContext.tsx @@ -12,6 +12,7 @@ import { } from "react"; import { useAPI } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { useOptionalWorkspaceMetadata } from "@/browser/contexts/WorkspaceContext"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import { CUSTOM_EVENTS, createCustomEvent } from "@/common/constants/events"; @@ -65,6 +66,14 @@ function getScopeId(workspaceId: string | undefined, projectPath: string | undef return workspaceId ?? (projectPath ? getProjectScopeId(projectPath) : GLOBAL_SCOPE_ID); } +function isSameFetch( + current: { projectPath: string | undefined; workspaceId: string | undefined }, + projectPath: string | undefined, + workspaceId: string | undefined +): boolean { + return current.projectPath === projectPath && current.workspaceId === workspaceId; +} + function coerceAgentId(value: unknown): string { return normalizeAgentId(value, WORKSPACE_DEFAULTS.agentId); } @@ -144,7 +153,9 @@ function AgentProviderWithState(props: { if (!api || (!projectPath && !workspaceId)) { if (isMountedRef.current) { - setAgents([]); + // No setAgents([]): offline, keep the agents shown for this scope, the reconnect + // reloads them. A scope change (also to no scope) already cleared them in the load + // effect below. setLoaded(true); setLoadFailed(false); } @@ -153,24 +164,16 @@ function AgentProviderWithState(props: { try { const result = await api.agents.list({ projectPath, workspaceId }); - const current = fetchParamsRef.current; - if ( - current.projectPath === projectPath && - current.workspaceId === workspaceId && - isMountedRef.current - ) { + if (isSameFetch(fetchParamsRef.current, projectPath, workspaceId) && isMountedRef.current) { setAgents(result); setAgentBases(getScopeId(workspaceId, projectPath), result); setLoadFailed(false); setLoaded(true); } - } catch { - const current = fetchParamsRef.current; - if ( - current.projectPath === projectPath && - current.workspaceId === workspaceId && - isMountedRef.current - ) { + } catch (error) { + // A lost connection (or the reconnect that replaced the client) is no load failure. + if (isAbortError(error)) return; + if (isSameFetch(fetchParamsRef.current, projectPath, workspaceId) && isMountedRef.current) { setAgents([]); setAgentBases(getScopeId(workspaceId, projectPath), []); setLoadFailed(true); @@ -181,10 +184,17 @@ function AgentProviderWithState(props: { [api] ); + // The scope whose agents are shown. Only a new scope clears them: a new client (reconnect) + // reloads the same scope behind the shown agents. + const shownScopeRef = useRef(null); useEffect(() => { - setAgents([]); - setLoaded(false); - setLoadFailed(false); + const scope = JSON.stringify([props.projectPath, props.workspaceId]); + if (shownScopeRef.current !== scope) { + shownScopeRef.current = scope; + setAgents([]); + setLoaded(false); + setLoadFailed(false); + } void fetchAgents(props.projectPath, props.workspaceId); }, [fetchAgents, props.projectPath, props.workspaceId]); diff --git a/src/browser/contexts/UILayoutsContext.test.tsx b/src/browser/contexts/UILayoutsContext.test.tsx new file mode 100644 index 00000000000..5965a5b66cf --- /dev/null +++ b/src/browser/contexts/UILayoutsContext.test.tsx @@ -0,0 +1,83 @@ +// Bootstrap Happy DOM before react-dom evaluates. +import "../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, render, waitFor } from "@testing-library/react"; +import { installDom } from "../../../tests/ui/dom"; +import type { APIClient } from "@/browser/contexts/API"; +import { + createConnectionHarness, + GAP_MODES, + type ConnectionHarness, +} from "./connection.testHarness"; +import type { TestApiOverrides } from "@/browser/testUtils"; +import { normalizeLayoutPresetsConfig, type LayoutPresetsConfig } from "@/common/types/uiLayouts"; +import { UILayoutsProvider, useUILayouts } from "./UILayoutsContext"; + +/** A config with one slot (a keybind override), so it differs from the defaults (no slots). */ +function configWithSlot(slot: number): LayoutPresetsConfig { + const config = normalizeLayoutPresetsConfig({ + version: 2, + slots: [{ slot, keybindOverride: { key: "F1", ctrl: true } }], + }); + // Setup guard: the slot survives normalization, so the assertions below see it. + expect(config.slots.map((entry) => entry.slot)).toEqual([slot]); + return config; +} + +let harness: ConnectionHarness; +/** What the next server answers for uiLayouts.getAll: a config, or a promise of one. */ +let serverConfig: LayoutPresetsConfig | Promise; +let shownSlots: number[] | null = null; + +function SlotsProbe() { + const layouts = useUILayouts(); + shownSlots = layouts.loaded ? layouts.layoutPresets.slots.map((entry) => entry.slot) : null; + return null; +} + +// A reconnect (null-gap: a gap without a client) must keep the shown layouts until the new +// client's reload answers. +describe.each([...GAP_MODES])("UILayoutsProvider across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + shownSlots = null; + serverConfig = configWithSlot(1); + harness = createConnectionHarness(mode, (): TestApiOverrides => { + const answer = serverConfig; + return { uiLayouts: { getAll: () => Promise.resolve(answer) } }; + }); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps the loaded layouts while offline and until the reload answers", async () => { + render( + + + + + + ); + await waitFor(() => expect(shownSlots).toEqual([1])); + + let answer: ((config: LayoutPresetsConfig) => void) | null = null; + serverConfig = new Promise((resolve) => (answer = resolve)); + await harness.drop(); + expect(shownSlots).toEqual([1]); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + expect(shownSlots).toEqual([1]); + + act(() => answer?.(configWithSlot(2))); + await waitFor(() => expect(shownSlots).toEqual([2])); + }); +}); diff --git a/src/browser/contexts/UILayoutsContext.tsx b/src/browser/contexts/UILayoutsContext.tsx index 686b9d6c69d..13394f84cb2 100644 --- a/src/browser/contexts/UILayoutsContext.tsx +++ b/src/browser/contexts/UILayoutsContext.tsx @@ -8,6 +8,7 @@ import { type ReactNode, } from "react"; import { useAPI, useLiveAPI } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import assert from "@/common/utils/assert"; import { DEFAULT_LAYOUT_PRESETS_CONFIG, @@ -62,6 +63,16 @@ export function useUILayouts(): UILayoutsContextValue { return ctx; } +/** The typed name, else the slot's current name, else "Slot N". */ +function resolveSlotName( + name: string | null | undefined, + existingName: string | undefined, + slot: LayoutSlotNumber +): string { + const trimmedName = name?.trim(); + return trimmedName && trimmedName.length > 0 ? trimmedName : (existingName ?? `Slot ${slot}`); +} + export function UILayoutsProvider(props: { children: ReactNode }) { const { api } = useAPI(); const liveApi = useLiveAPI(); @@ -74,7 +85,8 @@ export function UILayoutsProvider(props: { children: ReactNode }) { const refresh = useCallback(async (): Promise => { if (!api) { - setLayoutPresets(DEFAULT_LAYOUT_PRESETS_CONFIG); + // Keep the shown presets offline (the defaults before the first load): the reconnect + // reloads them. setLoaded(true); setLoadFailed(false); return; @@ -85,7 +97,9 @@ export function UILayoutsProvider(props: { children: ReactNode }) { setLayoutPresets(getLayoutsConfigOrDefault(remote)); setLoaded(true); setLoadFailed(false); - } catch { + } catch (error) { + // A lost connection is no load failure: keep the shown presets for the reconnect. + if (isAbortError(error)) return; setLayoutPresets(DEFAULT_LAYOUT_PRESETS_CONFIG); setLoaded(true); setLoadFailed(true); @@ -162,11 +176,7 @@ export function UILayoutsProvider(props: { children: ReactNode }) { const base = await getConfigForWrite(); const existingPreset = getPresetForSlot(base, slot); - const trimmedName = name?.trim(); - const resolvedName = - trimmedName && trimmedName.length > 0 - ? trimmedName - : (existingPreset?.name ?? `Slot ${slot}`); + const resolvedName = resolveSlotName(name, existingPreset?.name, slot); const result = createPresetFromCurrentWorkspace( workspaceId, From ba065877d3b933fc5f2688a3efe9af80494087e3 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 10:17:21 +0000 Subject: [PATCH 03/14] refactor: data hooks keep shown data while reloading --- src/browser/hooks/useAnalytics.ts | 87 ++++++---- .../hooks/useAnalyticsReconnect.test.tsx | 80 +++++++++ src/browser/hooks/useCoderWorkspace.ts | 94 ++++++----- .../hooks/useCoderWorkspaceReconnect.test.tsx | 158 ++++++++++++++++++ src/browser/hooks/useWorkflowRunById.ts | 23 ++- .../useWorkflowRunByIdReconnect.test.tsx | 82 +++++++++ .../hooks/useWorkspaceHeartbeat.test.tsx | 85 ++++++++++ src/browser/hooks/useWorkspaceHeartbeat.ts | 44 +++-- 8 files changed, 566 insertions(+), 87 deletions(-) create mode 100644 src/browser/hooks/useAnalyticsReconnect.test.tsx create mode 100644 src/browser/hooks/useCoderWorkspaceReconnect.test.tsx create mode 100644 src/browser/hooks/useWorkflowRunByIdReconnect.test.tsx diff --git a/src/browser/hooks/useAnalytics.ts b/src/browser/hooks/useAnalytics.ts index 346d2850805..01f4096bb95 100644 --- a/src/browser/hooks/useAnalytics.ts +++ b/src/browser/hooks/useAnalytics.ts @@ -7,6 +7,7 @@ import { useAPI } from "@/browser/contexts/API"; import type { analytics } from "@/common/orpc/schemas/analytics"; import type { SavedQuery } from "@/common/types/savedQueries"; import { getErrorMessage } from "@/common/utils/errors"; +import { isAbortError } from "@/browser/utils/isAbortError"; export type Summary = z.infer; export type SpendOverTimeItem = z.infer[number]; @@ -24,6 +25,8 @@ export interface AsyncState { data: T | null; loading: boolean; error: string | null; + /** The query (JSON of its inputs) `data` answers; a reload of the same query keeps `data` shown. */ + dataKey?: string; } type SummaryInput = z.input; @@ -103,16 +106,19 @@ function getAnalyticsNamespace(api: APIClient): AnalyticsNamespace | null { function runAnalyticsEffect( api: APIClient | null, setState: React.Dispatch>>, + queryKey: readonly unknown[], fetcher: (analyticsApi: AnalyticsNamespace) => Promise ): (() => void) | undefined { - if (!api) { - setState((previousState) => ({ - data: previousState.data, - loading: true, - error: null, - })); - return; - } + const dataKey = JSON.stringify(queryKey); + // Stale-while-revalidate: a new client (reconnect) or no client keeps the shown data on + // screen. Only data of another query (or none) shows "loading". + setState((previousState) => ({ + data: previousState.data, + loading: !isShownFor(previousState, dataKey), + error: null, + dataKey: previousState.dataKey, + })); + if (!api) return; const analyticsApi = getAnalyticsNamespace(api); if (!analyticsApi) { @@ -120,28 +126,38 @@ function runAnalyticsEffect( return; } - let ignore = false; - setState((previousState) => ({ - data: previousState.data, - loading: true, - error: null, - })); + return startAnalyticsFetch(analyticsApi, setState, dataKey, fetcher); +} + +function isShownFor(state: AsyncState, dataKey: string): boolean { + return state.data != null && state.dataKey === dataKey; +} +/** Runs `fetcher` and stores its answer under `dataKey`; returns the cancel function. */ +function startAnalyticsFetch( + analyticsApi: AnalyticsNamespace, + setState: React.Dispatch>>, + dataKey: string, + fetcher: (analyticsApi: AnalyticsNamespace) => Promise +): () => void { + let ignore = false; void fetcher(analyticsApi) .then((data) => { if (ignore) { return; } - setState({ data, loading: false, error: null }); + setState({ data, loading: false, error: null, dataKey }); }) .catch((error: unknown) => { - if (ignore) { + // A lost connection is no query failure: the reconnect runs the query again. + if (ignore || isAbortError(error)) { return; } setState((previousState) => ({ data: previousState.data, loading: false, error: getErrorMessage(error), + dataKey: previousState.dataKey, })); }); @@ -167,7 +183,7 @@ export function useAnalyticsSummary( useEffect(() => { const fromDate = fromMs == null ? null : new Date(fromMs); const toDate = toMs == null ? null : new Date(toMs); - return runAnalyticsEffect(api, setState, (analyticsApi) => + return runAnalyticsEffect(api, setState, [projectPath, fromMs, toMs], (analyticsApi) => analyticsApi.getSummary({ projectPath: projectPath ?? null, from: fromDate, to: toDate }) ); }, [api, projectPath, fromMs, toMs]); @@ -200,14 +216,18 @@ export function useAnalyticsSpendOverTime(params: { useEffect(() => { const fromDate = fromMs == null ? null : new Date(fromMs); const toDate = toMs == null ? null : new Date(toMs); - return runAnalyticsEffect(api, setState, (analyticsApi) => - analyticsApi.getSpendOverTime({ - projectPath: params.projectPath ?? null, - granularity: params.granularity, - from: fromDate, - to: toDate, - timeZone: params.timeZone ?? null, - }) + return runAnalyticsEffect( + api, + setState, + [params.projectPath, params.granularity, params.timeZone, fromMs, toMs], + (analyticsApi) => + analyticsApi.getSpendOverTime({ + projectPath: params.projectPath ?? null, + granularity: params.granularity, + from: fromDate, + to: toDate, + timeZone: params.timeZone ?? null, + }) ); }, [api, params.projectPath, params.granularity, params.timeZone, fromMs, toMs]); @@ -230,7 +250,7 @@ export function useAnalyticsSpendByProject( useEffect(() => { const fromDate = fromMs == null ? null : new Date(fromMs); const toDate = toMs == null ? null : new Date(toMs); - return runAnalyticsEffect(api, setState, (analyticsApi) => + return runAnalyticsEffect(api, setState, [fromMs, toMs], (analyticsApi) => analyticsApi.getSpendByProject({ from: fromDate, to: toDate }) ); }, [api, fromMs, toMs]); @@ -255,7 +275,7 @@ export function useAnalyticsSpendByModel( useEffect(() => { const fromDate = fromMs == null ? null : new Date(fromMs); const toDate = toMs == null ? null : new Date(toMs); - return runAnalyticsEffect(api, setState, (analyticsApi) => + return runAnalyticsEffect(api, setState, [projectPath, fromMs, toMs], (analyticsApi) => analyticsApi.getSpendByModel({ projectPath: projectPath ?? null, from: fromDate, to: toDate }) ); }, [api, projectPath, fromMs, toMs]); @@ -280,7 +300,7 @@ export function useAnalyticsTokensByModel( useEffect(() => { const fromDate = fromMs == null ? null : new Date(fromMs); const toDate = toMs == null ? null : new Date(toMs); - return runAnalyticsEffect(api, setState, (analyticsApi) => + return runAnalyticsEffect(api, setState, [projectPath, fromMs, toMs], (analyticsApi) => analyticsApi.getTokensByModel({ projectPath: projectPath ?? null, from: fromDate, @@ -315,7 +335,7 @@ export function useAnalyticsTimingDistribution( useEffect(() => { const fromDate = fromMs == null ? null : new Date(fromMs); const toDate = toMs == null ? null : new Date(toMs); - return runAnalyticsEffect(api, setState, (analyticsApi) => + return runAnalyticsEffect(api, setState, [metric, projectPath, fromMs, toMs], (analyticsApi) => analyticsApi.getTimingDistribution({ metric, projectPath: projectPath ?? null, @@ -345,7 +365,7 @@ export function useAnalyticsProviderCacheHitRatio( useEffect(() => { const fromDate = fromMs == null ? null : new Date(fromMs); const toDate = toMs == null ? null : new Date(toMs); - return runAnalyticsEffect(api, setState, (analyticsApi) => + return runAnalyticsEffect(api, setState, [projectPath, fromMs, toMs], (analyticsApi) => analyticsApi.getCacheHitRatioByProvider({ projectPath: projectPath ?? null, from: fromDate, @@ -374,7 +394,7 @@ export function useAnalyticsAgentCostBreakdown( useEffect(() => { const fromDate = fromMs == null ? null : new Date(fromMs); const toDate = toMs == null ? null : new Date(toMs); - return runAnalyticsEffect(api, setState, (analyticsApi) => + return runAnalyticsEffect(api, setState, [projectPath, fromMs, toMs], (analyticsApi) => analyticsApi.getAgentCostBreakdown({ projectPath: projectPath ?? null, from: fromDate, @@ -403,7 +423,7 @@ export function useAnalyticsDelegationSummary( useEffect(() => { const fromDate = fromMs == null ? null : new Date(fromMs); const toDate = toMs == null ? null : new Date(toMs); - return runAnalyticsEffect(api, setState, (analyticsApi) => + return runAnalyticsEffect(api, setState, [projectPath, fromMs, toMs], (analyticsApi) => analyticsApi.getDelegationSummary({ projectPath: projectPath ?? null, from: fromDate, @@ -432,7 +452,8 @@ export function useAnalyticsRawQuery() { const executeQuery = async (sql: string) => { if (!api) { - setState({ data: null, loading: false, error: "API client not initialized" }); + // Offline, keep the shown result. + setState((prev) => ({ ...prev, loading: false, error: "API client not initialized" })); return; } diff --git a/src/browser/hooks/useAnalyticsReconnect.test.tsx b/src/browser/hooks/useAnalyticsReconnect.test.tsx new file mode 100644 index 00000000000..2cc4a2ed80d --- /dev/null +++ b/src/browser/hooks/useAnalyticsReconnect.test.tsx @@ -0,0 +1,80 @@ +// Bootstrap Happy DOM before react-dom evaluates. +import "../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, renderHook, waitFor } from "@testing-library/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 { useAnalyticsSummary, type Summary } from "./useAnalytics"; + +function summary(totalResponses: number): Summary { + return { + totalSpendUsd: 1, + todaySpendUsd: 1, + avgDailySpendUsd: 1, + cacheHitRatio: 0, + totalTokens: 1, + totalResponses, + }; +} + +let harness: ConnectionHarness; +/** What the next server's getSummary answers, or a promise of it. */ +let serverSummary: Summary | Promise; + +// The dashboard charts show a spinner instead of their data while `loading` is true. A reload +// through the new client after a reconnect must keep the shown data instead. +describe.each([...GAP_MODES])("useAnalyticsSummary across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + serverSummary = summary(1); + harness = createConnectionHarness(mode, (): TestApiOverrides => { + const answer = serverSummary; + return { analytics: { getSummary: () => Promise.resolve(answer) } }; + }); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps the shown summary until the reload answers; another project loads again", async () => { + const view = renderHook( + (props: { projectPath: string }) => useAnalyticsSummary(props.projectPath), + { + wrapper: harness.Wrapper, + initialProps: { projectPath: "/a" }, + } + ); + await waitFor(() => expect(view.result.current.data?.totalResponses).toBe(1)); + + let answer: ((value: Summary) => void) | null = null; + serverSummary = new Promise((resolve) => (answer = resolve)); + await harness.drop(); + expect(view.result.current.loading).toBe(false); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + expect(view.result.current.loading).toBe(false); + expect(view.result.current.error).toBeNull(); + expect(view.result.current.data?.totalResponses).toBe(1); + + act(() => answer?.(summary(2))); + await waitFor(() => expect(view.result.current.data?.totalResponses).toBe(2)); + + // The shown summary belongs to /a: under /b, the chart shows "loading" until /b answers. + view.rerender({ projectPath: "/b" }); + expect(view.result.current.loading).toBe(true); + }); +}); diff --git a/src/browser/hooks/useCoderWorkspace.ts b/src/browser/hooks/useCoderWorkspace.ts index 035a3a848e6..9314dba597c 100644 --- a/src/browser/hooks/useCoderWorkspace.ts +++ b/src/browser/hooks/useCoderWorkspace.ts @@ -7,6 +7,7 @@ */ import { useState, useEffect, useCallback, useRef } from "react"; import { useAPI } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import type { CoderInfo, CoderTemplate, @@ -91,6 +92,22 @@ interface UseCoderWorkspaceReturn { * State ownership: coderConfig is owned by parent (selectedRuntime.coder). * This hook derives `enabled` from coderConfig and manages only async data. */ +/** The first line of a list request's error, for the picker's error text. */ +function formatCoderListError(error: unknown): string { + const message = + error instanceof Error ? error.message.split("\n")[0].slice(0, 200).trim() : "Unknown error"; + return message || "Unknown error"; +} + +/** True while `config` still selects the template (and org) a presets request was made for. */ +function isTemplateSelected( + config: CoderWorkspaceConfig | null, + template: string, + org: string | undefined +): boolean { + return config?.template === template && config.templateOrg === org; +} + export function useCoderWorkspace({ coderConfig, onCoderConfigChange, @@ -122,6 +139,10 @@ export function useCoderWorkspace({ const [loadingTemplates, setLoadingTemplates] = useState(false); const [loadingPresets, setLoadingPresets] = useState(false); const [loadingWorkspaces, setLoadingWorkspaces] = useState(false); + // What the lists show: a reload of shown data keeps it on screen instead of a spinner. + const templatesShownRef = useRef(false); + const workspacesShownRef = useRef(false); + const shownPresetsKeyRef = useRef(null); const fetchCoderInfo = useCallback(async () => { // Advance sequence before the API guard so that an API context change @@ -141,8 +162,9 @@ export function useCoderWorkspace({ if (info.state !== "available" && coderConfigRef.current != null) { onCoderConfigChangeRef.current(null); } - } catch { - if (seq !== latestAuthSeqRef.current) { + } catch (error) { + // A lost connection says nothing about the CLI: keep the info and the user's Coder config. + if (seq !== latestAuthSeqRef.current || isAbortError(error)) { return; } @@ -180,15 +202,19 @@ export function useCoderWorkspace({ // Fetch templates when Coder is enabled useEffect(() => { - if (!api || !enabled || coderInfo?.state !== "available") { + if (!enabled || coderInfo?.state !== "available") { + templatesShownRef.current = false; setTemplates([]); setTemplatesError(null); setLoadingTemplates(false); return; } + // Offline, keep the shown templates: the reconnect reloads them. + if (!api) return; let mounted = true; - setLoadingTemplates(true); + // Only the first load shows "loading": a reload keeps the shown templates. + if (!templatesShownRef.current) setLoadingTemplates(true); setTemplatesError(null); api.coder @@ -196,6 +222,7 @@ export function useCoderWorkspace({ .then((result) => { if (!mounted) return; if (result.ok) { + templatesShownRef.current = true; setTemplates(result.templates); setTemplatesError(null); // Auto-select first template if none selected @@ -212,13 +239,9 @@ export function useCoderWorkspace({ } }) .catch((error) => { - if (!mounted) return; - const message = - error instanceof Error - ? error.message.split("\n")[0].slice(0, 200).trim() - : "Unknown error"; + if (!mounted || isAbortError(error)) return; setTemplates([]); - setTemplatesError(message || "Unknown error"); + setTemplatesError(formatCoderListError(error)); }) .finally(() => { if (mounted) { @@ -234,15 +257,19 @@ export function useCoderWorkspace({ // Fetch existing workspaces when Coder is enabled useEffect(() => { - if (!api || !enabled || coderInfo?.state !== "available") { + if (!enabled || coderInfo?.state !== "available") { + workspacesShownRef.current = false; setExistingWorkspaces([]); setWorkspacesError(null); setLoadingWorkspaces(false); return; } + // Offline, keep the shown workspaces: the reconnect reloads them. + if (!api) return; let mounted = true; - setLoadingWorkspaces(true); + // Only the first load shows "loading": a reload keeps the shown workspaces. + if (!workspacesShownRef.current) setLoadingWorkspaces(true); setWorkspacesError(null); api.coder @@ -250,6 +277,7 @@ export function useCoderWorkspace({ .then((result) => { if (!mounted) return; if (result.ok) { + workspacesShownRef.current = true; setExistingWorkspaces(result.workspaces); setWorkspacesError(null); } else { @@ -259,13 +287,9 @@ export function useCoderWorkspace({ } }) .catch((error) => { - if (!mounted) return; - const message = - error instanceof Error - ? error.message.split("\n")[0].slice(0, 200).trim() - : "Unknown error"; + if (!mounted || isAbortError(error)) return; setExistingWorkspaces([]); - setWorkspacesError(message || "Unknown error"); + setWorkspacesError(formatCoderListError(error)); }) .finally(() => { if (mounted) { @@ -280,20 +304,24 @@ export function useCoderWorkspace({ // Fetch presets when template changes (only for "new" mode) useEffect(() => { - if (!api || !enabled || !coderConfig?.template || coderConfig.existingWorkspace) { + if (!enabled || !coderConfig?.template || coderConfig.existingWorkspace) { + shownPresetsKeyRef.current = null; setPresets([]); setPresetsError(null); setLoadingPresets(false); return; } + // Offline, keep the shown presets: the reconnect reloads them. + if (!api) return; let mounted = true; - setLoadingPresets(true); - setPresetsError(null); - // Capture template/org at request time to detect stale responses const templateAtRequest = coderConfig.template; const orgAtRequest = coderConfig.templateOrg; + // Only another template shows "loading": a reload (reconnect) keeps the shown presets. + const presetsKey = JSON.stringify([templateAtRequest, orgAtRequest]); + if (shownPresetsKeyRef.current !== presetsKey) setLoadingPresets(true); + setPresetsError(null); api.coder .listPresets({ template: templateAtRequest, org: orgAtRequest }) @@ -303,14 +331,12 @@ export function useCoderWorkspace({ } // Stale response guard: if user changed template/org while request was in-flight, ignore this response - if ( - coderConfigRef.current?.template !== templateAtRequest || - coderConfigRef.current?.templateOrg !== orgAtRequest - ) { + if (!isTemplateSelected(coderConfigRef.current, templateAtRequest, orgAtRequest)) { return; } if (result.ok) { + shownPresetsKeyRef.current = presetsKey; setPresets(result.presets); setPresetsError(null); @@ -344,28 +370,20 @@ export function useCoderWorkspace({ } }) .catch((error) => { - if (!mounted) { + if (!mounted || isAbortError(error)) { return; } - if ( - coderConfigRef.current?.template !== templateAtRequest || - coderConfigRef.current?.templateOrg !== orgAtRequest - ) { + if (!isTemplateSelected(coderConfigRef.current, templateAtRequest, orgAtRequest)) { return; } - const message = - error instanceof Error - ? error.message.split("\n")[0].slice(0, 200).trim() - : "Unknown error"; setPresets([]); - setPresetsError(message || "Unknown error"); + setPresetsError(formatCoderListError(error)); }) .finally(() => { // Only clear loading for the active request (not stale ones) if ( mounted && - coderConfigRef.current?.template === templateAtRequest && - coderConfigRef.current?.templateOrg === orgAtRequest + isTemplateSelected(coderConfigRef.current, templateAtRequest, orgAtRequest) ) { setLoadingPresets(false); } diff --git a/src/browser/hooks/useCoderWorkspaceReconnect.test.tsx b/src/browser/hooks/useCoderWorkspaceReconnect.test.tsx new file mode 100644 index 00000000000..7bab82ca173 --- /dev/null +++ b/src/browser/hooks/useCoderWorkspaceReconnect.test.tsx @@ -0,0 +1,158 @@ +// Bootstrap Happy DOM before react-dom evaluates. +import "../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import { act, cleanup, renderHook, waitFor } from "@testing-library/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 { CoderPreset, CoderTemplate, CoderWorkspace } from "@/common/orpc/schemas/coder"; +import type { CoderWorkspaceConfig } from "@/common/types/runtime"; +import { useCoderWorkspace } from "./useCoderWorkspace"; + +const template = (name: string): CoderTemplate => ({ + name, + displayName: name, + organizationName: "org", +}); +const preset = (name: string): CoderPreset => ({ id: name, name, isDefault: false }); +const workspace = (name: string): CoderWorkspace => ({ + name, + templateName: "t1", + templateDisplayName: "t1", + status: "running", +}); + +interface ServerLists { + templates: CoderTemplate[]; + presets: CoderPreset[]; + workspaces: CoderWorkspace[]; +} + +let harness: ConnectionHarness; +/** What the next server lists, or a promise that answers later. */ +let serverLists: ServerLists | Promise; + +function serverOverrides(): TestApiOverrides { + const lists = Promise.resolve(serverLists); + return { + coder: { + getInfo: () => Promise.resolve({ state: "available" as const, version: "2.0.0" }), + listTemplates: () => lists.then((l) => ({ ok: true as const, templates: l.templates })), + listPresets: () => lists.then((l) => ({ ok: true as const, presets: l.presets })), + listWorkspaces: () => lists.then((l) => ({ ok: true as const, workspaces: l.workspaces })), + }, + }; +} + +function renderCoder(config: CoderWorkspaceConfig, onChange = mock(() => undefined)) { + const view = renderHook( + (props: { config: CoderWorkspaceConfig }) => + useCoderWorkspace({ + coderConfig: props.config, + onCoderConfigChange: onChange, + coderInfoRefreshPolicy: "mount-and-focus", + }), + { wrapper: harness.Wrapper, initialProps: { config } } + ); + return { view, onChange }; +} + +async function settle() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); +} + +const CONFIG: CoderWorkspaceConfig = { + existingWorkspace: false, + template: "t1", + templateOrg: "org", +}; + +// A reconnect reloads the Coder lists through the new client: the pickers keep what they show +// (no spinner) until the reload answers, and a lost connection never clears the Coder config. +describe.each([...GAP_MODES])("useCoderWorkspace across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + serverLists = { + templates: [template("t1")], + presets: [preset("p1"), preset("p2")], + workspaces: [workspace("w1")], + }; + harness = createConnectionHarness(mode, serverOverrides); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps the shown lists until the reload answers", async () => { + const { view } = renderCoder(CONFIG); + await waitFor(() => + expect(view.result.current.presets.map((p) => p.name)).toEqual(["p1", "p2"]) + ); + expect(view.result.current.templates.map((t) => t.name)).toEqual(["t1"]); + expect(view.result.current.existingWorkspaces.map((w) => w.name)).toEqual(["w1"]); + + let answer: ((lists: ServerLists) => void) | null = null; + serverLists = new Promise((resolve) => (answer = resolve)); + await harness.drop(); + harness.reconnect(); + await settle(); + + const shown = view.result.current; + expect(shown.loadingTemplates || shown.loadingPresets || shown.loadingWorkspaces).toBe(false); + expect(shown.templates.map((t) => t.name)).toEqual(["t1"]); + expect(shown.presets.map((p) => p.name)).toEqual(["p1", "p2"]); + expect(shown.existingWorkspaces.map((w) => w.name)).toEqual(["w1"]); + + act(() => + answer?.({ + templates: [template("t1"), template("t2")], + presets: [preset("p1"), preset("p2"), preset("p3")], + workspaces: [workspace("w2")], + }) + ); + await waitFor(() => + expect(view.result.current.existingWorkspaces.map((w) => w.name)).toEqual(["w2"]) + ); + expect(view.result.current.templates.map((t) => t.name)).toEqual(["t1", "t2"]); + expect(view.result.current.presets.map((p) => p.name)).toEqual(["p1", "p2", "p3"]); + }); + + test("another template shows loading presets", async () => { + const { view } = renderCoder(CONFIG); + await waitFor(() => expect(view.result.current.presets.length).toBe(2)); + + serverLists = new Promise(() => undefined); + await harness.drop(); + harness.reconnect(); + view.rerender({ config: { ...CONFIG, template: "t2" } }); + await settle(); + expect(view.result.current.loadingPresets).toBe(true); + }); + + test("a window focus while offline keeps the Coder info and config", async () => { + const { view, onChange } = renderCoder(CONFIG); + await waitFor(() => expect(view.result.current.coderInfo?.state).toBe("available")); + + await harness.drop(); + act(() => { + window.dispatchEvent(new window.Event("focus")); + }); + await settle(); + + expect(view.result.current.coderInfo?.state).toBe("available"); + expect(onChange.mock.calls.some((call: unknown[]) => call[0] === null)).toBe(false); + }); +}); diff --git a/src/browser/hooks/useWorkflowRunById.ts b/src/browser/hooks/useWorkflowRunById.ts index b2a581c2864..fdc09463775 100644 --- a/src/browser/hooks/useWorkflowRunById.ts +++ b/src/browser/hooks/useWorkflowRunById.ts @@ -1,6 +1,7 @@ import { useContext, useEffect, useState } from "react"; import { APIContext } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { isActiveWorkflowRunStatus, type WorkflowRunRecord } from "@/common/types/workflow"; const WORKFLOW_RUN_REFRESH_INTERVAL_MS = 2_000; @@ -42,6 +43,20 @@ export function getNewestWorkflowRunSnapshot( return compareWorkflowRunSnapshots(current, next) > 0 ? current : next; } +/** True when a poll that only follows active runs got a finished run. */ +function isPollingDoneAfter( + pollWhileActive: boolean | undefined, + pollAfterTerminal: boolean | undefined, + nextRun: WorkflowRunRecord | null +): boolean { + return ( + pollWhileActive === true && + pollAfterTerminal !== true && + nextRun != null && + !isActiveWorkflowRunStatus(nextRun.status) + ); +} + export interface UseWorkflowRunByIdResult { run: WorkflowRunRecord | null; loading: boolean; @@ -85,17 +100,15 @@ export function useWorkflowRunById(input: { setError(null); setLoading(false); if ( - input.pollWhileActive === true && - input.pollAfterTerminal !== true && - nextRun != null && - !isActiveWorkflowRunStatus(nextRun.status) && + isPollingDoneAfter(input.pollWhileActive, input.pollAfterTerminal, nextRun) && interval != null ) { window.clearInterval(interval); interval = null; } } catch (fetchError) { - if (ignore) { + // A lost connection is no load failure: keep the shown run, the reconnect polls again. + if (ignore || isAbortError(fetchError)) { return; } setError(fetchError instanceof Error ? fetchError.message : "Failed to load workflow run"); diff --git a/src/browser/hooks/useWorkflowRunByIdReconnect.test.tsx b/src/browser/hooks/useWorkflowRunByIdReconnect.test.tsx new file mode 100644 index 00000000000..c517157814f --- /dev/null +++ b/src/browser/hooks/useWorkflowRunByIdReconnect.test.tsx @@ -0,0 +1,82 @@ +// Bootstrap Happy DOM before react-dom evaluates. +import "../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, renderHook, waitFor } from "@testing-library/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 { WorkflowRunRecord } from "@/common/types/workflow"; +import { useWorkflowRunById } from "./useWorkflowRunById"; + +function makeRun(sequence: number): WorkflowRunRecord { + const at = `2026-06-26T00:00:0${sequence}.000Z`; + return { + id: "wfr_child", + workspaceId: "workspace-1", + workflow: { name: "loop", description: "Loop", scope: "global", executable: true }, + source: "export default function workflow() { return null; }", + sourceHash: "sha256:test", + args: {}, + status: "running", + createdAt: "2026-06-26T00:00:00.000Z", + updatedAt: at, + events: [{ sequence, type: "status", at, status: "running" }], + steps: [], + }; +} + +let harness: ConnectionHarness; +let serverRun: WorkflowRunRecord; + +// The nested-workflow views poll a running child run. While the connection is down, a poll +// fails with ConnectionLostError (kept-gap): that must not replace the shown run with an error. +describe.each([...GAP_MODES])("useWorkflowRunById across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + serverRun = makeRun(1); + harness = createConnectionHarness(mode, (): TestApiOverrides => { + const run = serverRun; + return { workflows: { getRun: () => Promise.resolve(run) } }; + }); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps the shown run without an error while offline", async () => { + const view = renderHook( + () => + useWorkflowRunById({ + workspaceId: "workspace-1", + runId: "wfr_child", + pollWhileActive: true, + }), + { wrapper: harness.Wrapper } + ); + await waitFor(() => expect(view.result.current.run?.events[0]?.sequence).toBe(1)); + + await harness.drop(); + // Let one poll interval pass while the connection is down. + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 2_100)); + }); + expect(view.result.current.error).toBeNull(); + expect(view.result.current.run?.events[0]?.sequence).toBe(1); + + serverRun = makeRun(2); + harness.reconnect(); + await waitFor(() => expect(view.result.current.run?.events[0]?.sequence).toBe(2)); + expect(view.result.current.error).toBeNull(); + }); +}); diff --git a/src/browser/hooks/useWorkspaceHeartbeat.test.tsx b/src/browser/hooks/useWorkspaceHeartbeat.test.tsx index 84a75234ac2..a34c2491abe 100644 --- a/src/browser/hooks/useWorkspaceHeartbeat.test.tsx +++ b/src/browser/hooks/useWorkspaceHeartbeat.test.tsx @@ -3,6 +3,11 @@ import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; import type React from "react"; import { APIProvider, type APIClient } from "@/browser/contexts/API"; +import { + createConnectionHarness, + GAP_MODES, + type ConnectionHarness, +} from "@/browser/contexts/connection.testHarness"; import { WorkspaceContext, type WorkspaceContext as WorkspaceContextValue, @@ -202,3 +207,83 @@ describe("useWorkspaceHeartbeat", () => { expect(updatedMetadata.heartbeat?.message).toBe(initialSettings.message); }); }); + +// A reconnect reloads the workspace's heartbeat settings through the new client. The dialog keeps +// the shown settings (no "loading" form) until the reload answers; another workspace resets it. +describe.each([...GAP_MODES])("useWorkspaceHeartbeat across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + let harness: ConnectionHarness; + /** The message the next server's heartbeat.get answers, or a promise of it. */ + let serverMessage: string | Promise; + + beforeEach(() => { + cleanupDom = installDom(); + serverMessage = "old"; + harness = createConnectionHarness(mode, (): TestApiOverrides => { + const message = Promise.resolve(serverMessage); + return { + workspace: { + heartbeat: { + get: () => + message.then((text) => ({ + enabled: true, + intervalMs: 120000, + contextMode: "normal" as const, + message: text, + })), + }, + }, + config: { getConfig: () => Promise.resolve(createTestConfig()) }, + }; + }); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + function Wrapper(props: { children: React.ReactNode }) { + const workspaceContext = { + workspaceMetadata: new Map(), + setWorkspaceMetadata: setWorkspaceMetadataMock, + } as unknown as WorkspaceContextValue; + return ( + + + {props.children} + + + ); + } + + test("keeps the shown settings until the reload answers", async () => { + const view = renderHook( + (props: { workspaceId: string }) => useWorkspaceHeartbeat({ workspaceId: props.workspaceId }), + { wrapper: Wrapper, initialProps: { workspaceId: TEST_WORKSPACE_ID } } + ); + await waitFor(() => expect(view.result.current.settings.message).toBe("old")); + + let answer: ((message: string) => void) | null = null; + serverMessage = new Promise((resolve) => (answer = resolve)); + await harness.drop(); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + expect(view.result.current.isLoading).toBe(false); + expect(view.result.current.settings.message).toBe("old"); + + act(() => answer?.("new")); + await waitFor(() => expect(view.result.current.settings.message).toBe("new")); + + // Another workspace shows "loading" (not the first workspace's settings) until it loads. + serverMessage = new Promise(() => undefined); + await harness.drop(); + harness.reconnect(); + view.rerender({ workspaceId: "workspace-2" }); + expect(view.result.current.isLoading).toBe(true); + expect(view.result.current.settings.message).not.toBe("new"); + }); +}); diff --git a/src/browser/hooks/useWorkspaceHeartbeat.ts b/src/browser/hooks/useWorkspaceHeartbeat.ts index d2b135d0b5b..6aaccb03a75 100644 --- a/src/browser/hooks/useWorkspaceHeartbeat.ts +++ b/src/browser/hooks/useWorkspaceHeartbeat.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { useAPI } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { useWorkspaceActions } from "@/browser/contexts/WorkspaceContext"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import { @@ -68,6 +69,20 @@ function normalizeHeartbeatSettings( }; } +/** The global heartbeat defaults the form shows as placeholders; undefined without a config. */ +function getGlobalHeartbeatDefaults( + config: { heartbeatDefaultPrompt?: string; heartbeatDefaultIntervalMs?: number } | null +): { + prompt: string | undefined; + intervalMs: number | undefined; +} { + if (config == null) return { prompt: undefined, intervalMs: undefined }; + return { + prompt: config.heartbeatDefaultPrompt?.trim() ?? undefined, + intervalMs: config.heartbeatDefaultIntervalMs ?? HEARTBEAT_DEFAULT_INTERVAL_MS, + }; +} + function getHeartbeatErrorMessage(error: unknown, fallbackMessage: string): string { if (error instanceof Error && error.message) { return error.message; @@ -97,14 +112,21 @@ export function useWorkspaceHeartbeat( const currentWorkspaceIdRef = useRef(workspaceId); currentWorkspaceIdRef.current = workspaceId; const latestSaveRequestIdRef = useRef(0); + // The workspace whose settings are shown (undefined before the first load effect). + const shownWorkspaceIdRef = useRef(undefined); useEffect(() => { - setSettings(getDefaultHeartbeatSettings()); - setIsLoading(true); - setIsSaving(false); - setError(null); - setGlobalDefaultPrompt(undefined); - setGlobalDefaultIntervalMs(undefined); + // Only another workspace resets the form to "loading": a new client (reconnect) reloads the + // same workspace behind the shown settings. + if (shownWorkspaceIdRef.current !== workspaceId) { + shownWorkspaceIdRef.current = workspaceId; + setSettings(getDefaultHeartbeatSettings()); + setIsLoading(true); + setIsSaving(false); + setError(null); + setGlobalDefaultPrompt(undefined); + setGlobalDefaultIntervalMs(undefined); + } if (!workspaceId) { setIsLoading(false); @@ -125,15 +147,15 @@ export function useWorkspaceHeartbeat( if (currentWorkspaceIdRef.current !== workspaceId) return; setSettings(normalizeHeartbeatSettings(heartbeat)); - setGlobalDefaultPrompt(config?.heartbeatDefaultPrompt?.trim() ?? undefined); - setGlobalDefaultIntervalMs( - config ? (config.heartbeatDefaultIntervalMs ?? HEARTBEAT_DEFAULT_INTERVAL_MS) : undefined - ); + const globalDefaults = getGlobalHeartbeatDefaults(config); + setGlobalDefaultPrompt(globalDefaults.prompt); + setGlobalDefaultIntervalMs(globalDefaults.intervalMs); setError(null); setIsLoading(false); }) .catch((loadError) => { - if (cancelled) return; + // A lost connection is no load failure: keep the shown settings for the reconnect. + if (cancelled || isAbortError(loadError)) return; if (currentWorkspaceIdRef.current !== workspaceId) return; setGlobalDefaultPrompt(undefined); From 8b5e504702436374a219d0ab505972fa2f5bcf28 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 10:22:00 +0000 Subject: [PATCH 04/14] refactor: composer suggestions and creation controls keep shown data while reloading --- .../ChatInput/useComposerSuggestions.ts | 84 ++++++---- .../useComposerSuggestionsReconnect.test.tsx | 147 ++++++++++++++++++ .../ChatInput/useCreationWorkspace.test.tsx | 58 +++++++ .../ChatInput/useCreationWorkspace.ts | 22 ++- 4 files changed, 273 insertions(+), 38 deletions(-) create mode 100644 src/browser/features/ChatInput/useComposerSuggestionsReconnect.test.tsx diff --git a/src/browser/features/ChatInput/useComposerSuggestions.ts b/src/browser/features/ChatInput/useComposerSuggestions.ts index 0a61612bea3..99682ae8ca6 100644 --- a/src/browser/features/ChatInput/useComposerSuggestions.ts +++ b/src/browser/features/ChatInput/useComposerSuggestions.ts @@ -1,6 +1,7 @@ import { useEffect, useId, useRef, useState } from "react"; import type { RefObject } from "react"; import { useAPI } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { useExperimentValue } from "@/browser/hooks/useExperiments"; import { usePersistedState } from "@/browser/hooks/usePersistedState"; import { EXPERIMENT_IDS } from "@/common/constants/experiments"; @@ -159,9 +160,13 @@ interface UseComposerSuggestionsOptions { projectPath: string | null; } +const NO_PLUGIN_COMMANDS: PluginSlashCommandDescriptor[] = []; + export function useComposerSuggestions(options: UseComposerSuggestionsOptions) { const { input, inputRef, projectPath, setInput, variant, workspaceId } = options; const { api } = useAPI(); + // The chat workspace the composer belongs to (none for the creation composer). + const chatWorkspaceId = variant === "workspace" ? workspaceId : undefined; const memoryEnabled = useExperimentValue(EXPERIMENT_IDS.MEMORY); const memoryConsolidationEnabled = useExperimentValue(EXPERIMENT_IDS.MEMORY_CONSOLIDATION); const rlmEnabled = useExperimentValue(EXPERIMENT_IDS.RLM); @@ -177,9 +182,7 @@ export function useComposerSuggestions(options: UseComposerSuggestionsOptions) { // listener keeps the descriptor reload in sync when the transfer lands // after mount and when a successful send clears the key. const [transferredDraftProjectDiscovery] = usePersistedState( - variant === "workspace" && workspaceId - ? getPendingDraftSkillDiscoveryKey(workspaceId) - : "__unused__", + chatWorkspaceId ? getPendingDraftSkillDiscoveryKey(chatWorkspaceId) : "__unused__", false, { listener: true } ); @@ -199,7 +202,16 @@ export function useComposerSuggestions(options: UseComposerSuggestionsOptions) { suggestions: SlashSuggestion[]; }>({ tokenKey: "", suggestions: [] }); const [mcpPrompts, setMcpPrompts] = useState([]); - const [pluginCommands, setPluginCommands] = useState([]); + // Tagged with their workspace: commands listed for another workspace never show here (they + // stay on screen across a reconnect, and a workspace switch while offline must not show them). + const [pluginCommandsState, setPluginCommandsState] = useState<{ + workspaceId: string | undefined; + commands: PluginSlashCommandDescriptor[]; + }>({ workspaceId: undefined, commands: NO_PLUGIN_COMMANDS }); + const pluginCommands = + pluginCommandsState.workspaceId === chatWorkspaceId + ? pluginCommandsState.commands + : NO_PLUGIN_COMMANDS; const [pluginMutationTick, setPluginMutationTick] = useState(0); const fileRequestId = useRef(0); const mcpRequestId = useRef(0); @@ -208,12 +220,11 @@ export function useComposerSuggestions(options: UseComposerSuggestionsOptions) { const mcpWorkspace = useRef(null); const mcpAbort = useRef(null); const listId = useId(); - const skillDiscovery: AgentSkillsDiscovery | null = - variant === "workspace" && workspaceId - ? { workspaceId, disableWorkspaceAgents: transferredDraftProjectDiscovery } - : variant === "creation" && projectPath - ? { projectPath } - : null; + const skillDiscovery: AgentSkillsDiscovery | null = chatWorkspaceId + ? { workspaceId: chatWorkspaceId, disableWorkspaceAgents: transferredDraftProjectDiscovery } + : variant === "creation" && projectPath + ? { projectPath } + : null; const agentSkills = useAgentSkills(skillDiscovery).skills; const activeToken = detectActiveComposerToken(input, cursor); @@ -248,43 +259,48 @@ export function useComposerSuggestions(options: UseComposerSuggestionsOptions) { useEffect(() => { let mounted = true; - if (!api || variant !== "workspace" || !workspaceId) { - setPluginCommands([]); - return; - } + // Offline, keep the shown commands: the reconnect lists them again. + if (!api || !chatWorkspaceId) return; api.workspace.plugins.slashCommands - .list({ workspaceId }) + .list({ workspaceId: chatWorkspaceId }) .then((commands) => { - if (mounted) setPluginCommands(commands); + if (mounted) setPluginCommandsState({ workspaceId: chatWorkspaceId, commands }); }) - .catch(() => { - if (mounted) setPluginCommands([]); + .catch((error: unknown) => { + if (mounted && !isAbortError(error)) { + setPluginCommandsState({ workspaceId: chatWorkspaceId, commands: NO_PLUGIN_COMMANDS }); + } }); return () => { mounted = false; }; - }, [api, variant, workspaceId, pluginMutationTick]); + }, [api, chatWorkspaceId, pluginMutationTick]); useEffect(() => { - if (!api || variant !== "workspace" || !workspaceId) { - mcpWorkspace.current = null; - mcpLoadedAt.current = 0; + const dropMcpRequest = () => { mcpRequestId.current++; mcpRequest.current = null; mcpAbort.current?.abort(); mcpAbort.current = null; + }; + if (!chatWorkspaceId) { + mcpWorkspace.current = null; + mcpLoadedAt.current = 0; + dropMcpRequest(); setMcpPrompts([]); return; } - if (mcpWorkspace.current !== workspaceId) { - mcpWorkspace.current = workspaceId; + if (mcpWorkspace.current !== chatWorkspaceId) { + mcpWorkspace.current = chatWorkspaceId; mcpLoadedAt.current = 0; - mcpRequestId.current++; - mcpRequest.current = null; - mcpAbort.current?.abort(); - mcpAbort.current = null; + dropMcpRequest(); setMcpPrompts([]); } + if (!api) { + // Offline: drop the lost client's request, keep the shown prompts for the reconnect. + dropMcpRequest(); + return; + } if (activeToken?.kind !== "slash" && activeToken?.kind !== "inline") return; if (Date.now() - mcpLoadedAt.current < 30_000 || mcpRequest.current) return; @@ -292,15 +308,17 @@ export function useComposerSuggestions(options: UseComposerSuggestionsOptions) { const controller = new AbortController(); mcpAbort.current = controller; const request = api.workspace.mcp.prompts - .list({ workspaceId }, { signal: controller.signal }) + .list({ workspaceId: chatWorkspaceId }, { signal: controller.signal }) .then((prompts) => { - if (mcpWorkspace.current === workspaceId && mcpRequestId.current === requestId) { + if (mcpWorkspace.current === chatWorkspaceId && mcpRequestId.current === requestId) { setMcpPrompts(prompts); mcpLoadedAt.current = Date.now(); } }) - .catch(() => { - if (mcpWorkspace.current === workspaceId && mcpRequestId.current === requestId) { + .catch((error: unknown) => { + // A lost connection is no answer: leave the prompts unloaded so the reconnect asks again. + if (isAbortError(error)) return; + if (mcpWorkspace.current === chatWorkspaceId && mcpRequestId.current === requestId) { mcpLoadedAt.current = Date.now(); } }) @@ -309,7 +327,7 @@ export function useComposerSuggestions(options: UseComposerSuggestionsOptions) { if (mcpAbort.current === controller) mcpAbort.current = null; }); mcpRequest.current = request; - }, [api, activeToken?.kind, activeTokenKey, variant, workspaceId]); + }, [api, activeToken?.kind, activeTokenKey, chatWorkspaceId]); useEffect(() => { const scope = variant === "workspace" ? workspaceId : projectPath; diff --git a/src/browser/features/ChatInput/useComposerSuggestionsReconnect.test.tsx b/src/browser/features/ChatInput/useComposerSuggestionsReconnect.test.tsx new file mode 100644 index 00000000000..70a3a9f3782 --- /dev/null +++ b/src/browser/features/ChatInput/useComposerSuggestionsReconnect.test.tsx @@ -0,0 +1,147 @@ +// Bootstrap Happy DOM before react-dom evaluates. +import "../../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, renderHook, waitFor } from "@testing-library/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 { PluginSlashCommandDescriptor } from "@/common/orpc/schemas/agentPlugins"; +import type { MCPPromptDescriptor } from "@/common/orpc/schemas/mcp"; +import { useComposerSuggestions } from "./useComposerSuggestions"; + +const GREET: PluginSlashCommandDescriptor = { + name: "greet", + description: "Say hello", + expansion: "Please greet the user warmly.", + pluginName: "my-plugin", + scope: "global", +}; + +const GREET_PROMPT: MCPPromptDescriptor = { + commandKey: "greet-prompt", + stableKey: "greet-prompt-1a2b", + serverName: "srv", + promptName: "greet", +}; + +let harness: ConnectionHarness; +/** The MCP prompts the next server lists, or a promise of them. */ +let serverPrompts: MCPPromptDescriptor[] | Promise; +/** The plugin commands the next server lists, or a promise of them. */ +let serverCommands: PluginSlashCommandDescriptor[] | Promise; + +function serverOverrides(): TestApiOverrides { + const commands = serverCommands; + const prompts = serverPrompts; + return { + workspace: { + plugins: { slashCommands: { list: () => Promise.resolve(commands) } }, + mcp: { prompts: { list: () => Promise.resolve(prompts) } }, + }, + }; +} + +function renderSuggestions(workspaceId: string) { + const inputRef = { current: null }; + return renderHook( + (props: { workspaceId: string }) => + useComposerSuggestions({ + input: "/gre", + setInput: () => undefined, + inputRef, + variant: "workspace", + workspaceId: props.workspaceId, + projectPath: null, + }), + { wrapper: harness.Wrapper, initialProps: { workspaceId } } + ); +} + +function hasGreet(suggestions: Array<{ id: string }>): boolean { + return suggestions.some((suggestion) => suggestion.id === "plugin-command:greet"); +} + +function hasGreetPrompt(suggestions: Array<{ id: string }>): boolean { + return suggestions.some((suggestion) => suggestion.id === "mcp-prompt:greet-prompt"); +} + +async function settle() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); +} + +// The slash menu lists plugin commands of the workspace. A reconnect keeps them listed until the +// new client answers; a workspace switch (also while offline) never shows the old workspace's. +describe.each([...GAP_MODES])("composer suggestions across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + serverCommands = [GREET]; + serverPrompts = [GREET_PROMPT]; + harness = createConnectionHarness(mode, serverOverrides); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps the listed commands while offline and until the reload answers", async () => { + const view = renderSuggestions("ws-a"); + await waitFor(() => expect(hasGreet(view.result.current.suggestions)).toBe(true)); + + let answer: ((commands: PluginSlashCommandDescriptor[]) => void) | null = null; + serverCommands = new Promise((resolve) => (answer = resolve)); + await harness.drop(); + await settle(); + expect(hasGreet(view.result.current.suggestions)).toBe(true); + harness.reconnect(); + await settle(); + expect(hasGreet(view.result.current.suggestions)).toBe(true); + + act(() => answer?.([])); + await waitFor(() => expect(hasGreet(view.result.current.suggestions)).toBe(false)); + }); + + test("a workspace switch while offline drops the old workspace's commands", async () => { + const view = renderSuggestions("ws-a"); + await waitFor(() => expect(hasGreet(view.result.current.suggestions)).toBe(true)); + + await harness.drop(); + view.rerender({ workspaceId: "ws-b" }); + await settle(); + expect(hasGreet(view.result.current.suggestions)).toBe(false); + }); + + test("keeps the listed MCP prompts while offline", async () => { + const view = renderSuggestions("ws-a"); + await waitFor(() => expect(hasGreetPrompt(view.result.current.suggestions)).toBe(true)); + + await harness.drop(); + await settle(); + expect(hasGreetPrompt(view.result.current.suggestions)).toBe(true); + }); + + test("an MCP prompt request lost with the connection is asked again after the reconnect", async () => { + // The first server never answers: the request is still in flight when the connection drops. + serverPrompts = new Promise(() => undefined); + harness = createConnectionHarness(mode, serverOverrides); + const view = renderSuggestions("ws-a"); + await waitFor(() => expect(hasGreet(view.result.current.suggestions)).toBe(true)); + + await harness.drop(); + await settle(); + serverPrompts = [GREET_PROMPT]; + harness.reconnect(); + await waitFor(() => expect(hasGreetPrompt(view.result.current.suggestions)).toBe(true)); + }); +}); diff --git a/src/browser/features/ChatInput/useCreationWorkspace.test.tsx b/src/browser/features/ChatInput/useCreationWorkspace.test.tsx index f87d4b9b6c0..0e113bf5e76 100644 --- a/src/browser/features/ChatInput/useCreationWorkspace.test.tsx +++ b/src/browser/features/ChatInput/useCreationWorkspace.test.tsx @@ -1,6 +1,7 @@ import { defaultCreationDraftScope, getDraftStore } from "@/browser/stores/DraftStore"; import { APIProvider, type APIClient } from "@/browser/contexts/API"; import { createTestApiClient } from "@/browser/testUtils"; +import { createConnectionHarness, GAP_MODES } from "@/browser/contexts/connection.testHarness"; import { getAppConfigStore } from "@/browser/stores/AppConfigStore"; import { getAutoRoutingPick, @@ -711,6 +712,63 @@ describe("useCreationWorkspace", () => { restoreDomGlobals(); }); + // A reconnect reloads branches and runtime availability through the new client. The creation + // controls keep the shown values (no "loading" again) until the reload answers. + describe.each([...GAP_MODES])("reload after a reconnect (%s)", (mode) => { + test("keeps the shown branches and runtimes until the reload answers", async () => { + setupWindow({ + listBranches: mock(() => Promise.resolve({ branches: ["main"], recommendedTrunk: "main" })), + }); + const harness = createConnectionHarness(mode, () => currentORPCClient ?? {}); + const resultRef: { current: ReturnType | null } = { + current: null, + }; + function Harness() { + resultRef.current = useCreationWorkspace({ + projectPath: TEST_PROJECT_PATH, + onWorkspaceCreated: () => undefined, + message: "", + }); + return null; + } + render( + + + + ); + await waitFor(() => expect(resultRef.current?.branchesLoaded).toBe(true)); + await waitFor(() => + expect(resultRef.current?.runtimeAvailabilityState.status).toBe("loaded") + ); + + let answerBranches: (() => void) | null = null; + const branchesAnswered = new Promise((resolve) => (answerBranches = resolve)); + const previous = currentORPCClient; + if (!previous) throw new Error("setupWindow() sets the client"); + currentORPCClient = { + ...previous, + projects: { + ...previous.projects, + listBranches: () => + branchesAnswered.then(() => ({ branches: ["main", "next"], recommendedTrunk: "main" })), + runtimeAvailability: () => new Promise(() => undefined), + }, + }; + await harness.drop(); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + + expect(resultRef.current?.branchesLoaded).toBe(true); + expect(resultRef.current?.branches).toEqual(["main"]); + expect(resultRef.current?.runtimeAvailabilityState.status).toBe("loaded"); + + act(() => answerBranches?.()); + await waitFor(() => expect(resultRef.current?.branches).toEqual(["main", "next"])); + }); + }); + test("loads branches when projectPath is provided", async () => { const listBranchesMock = mock( (): Promise => diff --git a/src/browser/features/ChatInput/useCreationWorkspace.ts b/src/browser/features/ChatInput/useCreationWorkspace.ts index 2159fbb3920..e9dfd3c332a 100644 --- a/src/browser/features/ChatInput/useCreationWorkspace.ts +++ b/src/browser/features/ChatInput/useCreationWorkspace.ts @@ -219,6 +219,11 @@ export type RuntimeAvailabilityState = | { status: "failed" } | { status: "loaded"; data: RuntimeAvailabilityMap }; +/** True for a real (non-blank) creation draft ID. */ +function isDraftId(draftId: string | null | undefined): draftId is string { + return typeof draftId === "string" && draftId.trim().length > 0; +} + function isWorkspaceDraftEmpty(workspaceId: string): boolean { const draft = getDraftStore().getView({ kind: "workspace", workspaceId }); return draft.text.trim().length === 0 && draft.attachmentCount === 0; @@ -313,7 +318,7 @@ export function useCreationWorkspace({ // single auto-naming/manual-name state). const workspaceNameScopeId = projectPath.trim().length > 0 - ? typeof draftId === "string" && draftId.trim().length > 0 + ? isDraftId(draftId) ? getDraftScopeId(projectPath, draftId) : getPendingScopeId(projectPath) : null; @@ -349,6 +354,10 @@ export function useCreationWorkspace({ } }, [kind, projectPath, api]); + // The project whose branches and runtimes are shown. Only a new project shows "loading" again: + // a new client (reconnect) reloads the same project behind the shown values. + const shownProjectPathRef = useRef(null); + // Load branches and runtime availability on mount with mounted guard useEffect(() => { if (kind === "scratch") { @@ -357,8 +366,11 @@ export function useCreationWorkspace({ } if (!projectPath.length || !api) return; let mounted = true; - setBranchesLoaded(false); - setRuntimeAvailabilityState({ status: "loading" }); + if (shownProjectPathRef.current !== projectPath) { + shownProjectPathRef.current = projectPath; + setBranchesLoaded(false); + setRuntimeAvailabilityState({ status: "loading" }); + } // Both requests run at once and settle independently. Send waits for the branch list (#6033), // so it must not also wait for the runtime probe (Docker, Podman, devcontainer CLI), which can // take several seconds after the branches are ready. @@ -616,7 +628,7 @@ export function useCreationWorkspace({ }) .catch(() => null); - const isDraftScope = typeof draftId === "string" && draftId.trim().length > 0; + const isDraftScope = isDraftId(draftId); const clearPendingDraft = () => { // Once the workspace exists, drop the draft even if the initial send fails // so we don't keep a hidden placeholder in the sidebar. @@ -708,7 +720,7 @@ export function useCreationWorkspace({ }, }); - if (typeof draftId === "string" && draftId.trim().length > 0 && promoteWorkspaceDraft) { + if (isDraftId(draftId) && promoteWorkspaceDraft) { // UI-only: show the created workspace in-place where the draft was rendered. promoteWorkspaceDraft(projectPath, draftId, metadata); } From b215ff6f80ea6126baac145b1d73140f76bffd81 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 10:25:52 +0000 Subject: [PATCH 05/14] refactor: heartbeat, server access, secrets and MCP sections keep shown data while reloading --- .../Sections/HeartbeatSection.test.tsx | 63 +++++++- .../Settings/Sections/HeartbeatSection.tsx | 36 ++++- .../Settings/Sections/MCPSettingsSection.tsx | 29 +--- .../Settings/Sections/SecretsSection.test.tsx | 149 ++++++++++++++++++ .../Settings/Sections/SecretsSection.tsx | 130 +++++++-------- .../Sections/ServerAccessSection.test.tsx | 68 ++++++++ .../Settings/Sections/ServerAccessSection.tsx | 80 ++++++---- 7 files changed, 427 insertions(+), 128 deletions(-) create mode 100644 src/browser/features/Settings/Sections/SecretsSection.test.tsx create mode 100644 src/browser/features/Settings/Sections/ServerAccessSection.test.tsx diff --git a/src/browser/features/Settings/Sections/HeartbeatSection.test.tsx b/src/browser/features/Settings/Sections/HeartbeatSection.test.tsx index fa79a0deab2..1a338f2d3c0 100644 --- a/src/browser/features/Settings/Sections/HeartbeatSection.test.tsx +++ b/src/browser/features/Settings/Sections/HeartbeatSection.test.tsx @@ -1,10 +1,11 @@ import "../../../../../tests/ui/dom"; -import { cleanup, fireEvent, render, waitFor } from "@testing-library/react"; +import { act, cleanup, fireEvent, render, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; import type { ReactNode } from "react"; import { APIProvider, type APIClient } from "@/browser/contexts/API"; +import { createConnectionHarness, GAP_MODES } from "@/browser/contexts/connection.testHarness"; import { ThemeProvider } from "@/browser/contexts/ThemeContext"; import { TooltipProvider } from "@/browser/components/Tooltip/Tooltip"; import { createTestApiClient, createTestConfig, type TestApiOverrides } from "@/browser/testUtils"; @@ -224,3 +225,63 @@ describe("HeartbeatSection", () => { }); }); }); + +// happy-dom does not deliver React's synthetic onChange for controlled fields, so call the +// field's React onChange prop directly (same workaround as InstructionsSection.test.tsx). +function typeIntoTextarea(textarea: HTMLTextAreaElement, value: string) { + const reactPropsKey = Object.keys(textarea).find((key) => key.startsWith("__reactProps")); + const reactProps = ( + reactPropsKey ? (textarea as unknown as Record)[reactPropsKey] : null + ) as { + onChange?: (event: { target: { value: string } }) => void; + } | null; + const onChange = reactProps?.onChange; + if (!onChange) throw new Error("Expected the textarea to expose its onChange handler"); + act(() => onChange({ target: { value } })); +} + +// A reconnect reloads the config through the new client. A prompt the user typed and did not +// save yet stays in the field (the reload must not overwrite it), and the field stays enabled. +describe.each([...GAP_MODES])("HeartbeatSection across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + }); + + afterEach(() => { + cleanup(); + mock.restore(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps a typed, unsaved prompt when the reload answers", async () => { + mockApi = createMockAPI({ heartbeatDefaultPrompt: "Saved prompt" }).api; + const harness = createConnectionHarness(mode, () => mockApi); + const view = render( + + + + + + + + ); + const promptField = (await waitFor(() => + view.getByLabelText("Default heartbeat prompt") + )) as HTMLTextAreaElement; + await waitFor(() => expect(promptField.value).toBe("Saved prompt")); + + typeIntoTextarea(promptField, "Typed, not saved"); + mockApi = createMockAPI({ heartbeatDefaultPrompt: "Prompt from the server" }).api; + await harness.drop(); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + + expect(promptField.value).toBe("Typed, not saved"); + expect(promptField.disabled).toBe(false); + }); +}); diff --git a/src/browser/features/Settings/Sections/HeartbeatSection.tsx b/src/browser/features/Settings/Sections/HeartbeatSection.tsx index 3ed49c58c6f..0fc54ad2e2f 100644 --- a/src/browser/features/Settings/Sections/HeartbeatSection.tsx +++ b/src/browser/features/Settings/Sections/HeartbeatSection.tsx @@ -21,6 +21,18 @@ const loadAgentHeartbeatsEnabled = async (api: SettingsApi) => const saveAgentHeartbeatsEnabled = (api: SettingsApi, enabled: boolean) => api.config.updateAgentHeartbeatsEnabled({ enabled }); +/** + * True when a config load may replace the field: no newer load or edit started since `nonce`, + * and the user has no unsaved value typed since the last load. + */ +function isUneditedLoad( + nonce: number, + nonceRef: { current: number }, + editedSinceLoadRef: { current: boolean } +): boolean { + return nonce === nonceRef.current && !editedSinceLoadRef.current; +} + export function HeartbeatSection() { const { api } = useAPI(); const [heartbeatDefaultPrompt, setHeartbeatDefaultPrompt] = useState(""); @@ -44,26 +56,36 @@ export function HeartbeatSection() { return; } - setHeartbeatDefaultPromptLoaded(false); + // A new client (reconnect) reloads behind the shown values: the fields stay enabled (no + // `Loaded` reset), and a value the user typed since the last load stays (no edit-mark reset). + // Until the reload confirms the shown values, a blur without an edit must not save them. setHeartbeatDefaultPromptLoadedOk(false); - setHeartbeatDefaultIntervalLoaded(false); setHeartbeatDefaultIntervalLoadedOk(false); - heartbeatDefaultPromptEditedSinceLoadRef.current = false; - heartbeatDefaultIntervalEditedSinceLoadRef.current = false; - const heartbeatDefaultPromptNonce = ++heartbeatDefaultPromptLoadNonceRef.current; const heartbeatDefaultIntervalNonce = ++heartbeatDefaultIntervalLoadNonceRef.current; void configPromise .then((cfg) => { - if (heartbeatDefaultPromptNonce === heartbeatDefaultPromptLoadNonceRef.current) { + if ( + isUneditedLoad( + heartbeatDefaultPromptNonce, + heartbeatDefaultPromptLoadNonceRef, + heartbeatDefaultPromptEditedSinceLoadRef + ) + ) { setHeartbeatDefaultPrompt(cfg.heartbeatDefaultPrompt ?? ""); setHeartbeatDefaultPromptLoaded(true); setHeartbeatDefaultPromptLoadedOk(true); heartbeatDefaultPromptEditedSinceLoadRef.current = false; } - if (heartbeatDefaultIntervalNonce === heartbeatDefaultIntervalLoadNonceRef.current) { + if ( + isUneditedLoad( + heartbeatDefaultIntervalNonce, + heartbeatDefaultIntervalLoadNonceRef, + heartbeatDefaultIntervalEditedSinceLoadRef + ) + ) { setDraftIntervalMinutes(formatIntervalMinutes(cfg.heartbeatDefaultIntervalMs)); setHeartbeatDefaultIntervalLoaded(true); setHeartbeatDefaultIntervalLoadedOk(true); diff --git a/src/browser/features/Settings/Sections/MCPSettingsSection.tsx b/src/browser/features/Settings/Sections/MCPSettingsSection.tsx index 7e757c59951..9b5d263d1ab 100644 --- a/src/browser/features/Settings/Sections/MCPSettingsSection.tsx +++ b/src/browser/features/Settings/Sections/MCPSettingsSection.tsx @@ -9,6 +9,7 @@ import { ClaudeDesignCard } from "./ClaudeDesignCard"; import { CLAUDE_DESIGN_SERVER_NAME } from "@/common/constants/claudeDesign"; import React, { useCallback, useEffect, useRef, useState } from "react"; import { useAPI, useLiveAPI } from "@/browser/contexts/API"; +import { useGlobalSecretKeys } from "./SecretsSection"; import { Trash2, Play, @@ -857,7 +858,8 @@ export const MCPSettingsSection: React.FC = () => { const { api } = useAPI(); const [servers, setServers] = useState>({}); const [loading, setLoading] = useState(false); - const [globalSecretKeys, setGlobalSecretKeys] = useState([]); + // Global secrets (used for {secret:"KEY"} header values). + const [globalSecretKeys] = useGlobalSecretKeys(api); const [error, setError] = useState(null); // Test state with caching (global MCP config) @@ -924,31 +926,6 @@ export const MCPSettingsSection: React.FC = () => { } }, [api]); - // Load global secrets (used for {secret:"KEY"} header values). - useEffect(() => { - if (!api) { - setGlobalSecretKeys([]); - return; - } - - let cancelled = false; - (async () => { - try { - const secrets = await api.secrets.get({}); - if (cancelled) return; - setGlobalSecretKeys(secrets.map((s) => s.key)); - } catch (err) { - if (cancelled) return; - console.error("Failed to load global secrets:", err); - setGlobalSecretKeys([]); - } - })(); - - return () => { - cancelled = true; - }; - }, [api]); - useEffect(() => { // A sibling can change reuse/server settings while the experiment stays enabled. const requests = refreshRequest.current; diff --git a/src/browser/features/Settings/Sections/SecretsSection.test.tsx b/src/browser/features/Settings/Sections/SecretsSection.test.tsx new file mode 100644 index 00000000000..b4ff412f9a3 --- /dev/null +++ b/src/browser/features/Settings/Sections/SecretsSection.test.tsx @@ -0,0 +1,149 @@ +// Bootstrap Happy DOM before react-dom evaluates. +import "../../../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, render, renderHook, waitFor } from "@testing-library/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 { ProjectProvider } from "@/browser/contexts/ProjectContext"; +import { RouterProvider } from "@/browser/contexts/RouterContext"; +import { SettingsProvider } from "@/browser/contexts/SettingsContext"; +import type { TestApiOverrides } from "@/browser/testUtils"; +import type { Secret } from "@/common/types/secrets"; +import { SecretsSection, useGlobalSecretKeys } from "./SecretsSection"; + +let harness: ConnectionHarness; +/** The global secrets the next server returns, or a promise of them. */ +let serverSecrets: Secret[] | Promise; + +function serverOverrides(): TestApiOverrides { + const secrets = serverSecrets; + return { + projects: { list: () => Promise.resolve([]) }, + secrets: { get: () => Promise.resolve(secrets) }, + }; +} + +// happy-dom does not deliver React's synthetic onChange for controlled fields, so call the +// field's React onChange prop directly (same workaround as InstructionsSection.test.tsx). +function typeInto(field: HTMLElement, value: string) { + const reactPropsKey = Object.keys(field).find((key) => key.startsWith("__reactProps")); + const reactProps = ( + reactPropsKey ? (field as unknown as Record)[reactPropsKey] : null + ) as { onChange?: (event: { target: { value: string } }) => void } | null; + const onChange = reactProps?.onChange; + if (!onChange) throw new Error("Expected the field to expose its onChange handler"); + act(() => onChange({ target: { value } })); +} + +async function settle() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); +} + +function renderSecrets() { + return render( + + + + + + + + + + ); +} + +// A reconnect reloads the secrets through the new client. The rows stay on screen (no "Loading +// secrets…" in their place), and rows the user edited are not replaced by the reload. +describe.each([...GAP_MODES])("SecretsSection across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + serverSecrets = [{ key: "OLD_KEY", value: "old" }]; + harness = createConnectionHarness(mode, serverOverrides); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps the shown rows until the reload answers", async () => { + const view = renderSecrets(); + await waitFor(() => expect(view.queryByDisplayValue("OLD_KEY")).not.toBeNull()); + + let answer: ((secrets: Secret[]) => void) | null = null; + serverSecrets = new Promise((resolve) => (answer = resolve)); + await harness.drop(); + await settle(); + expect(view.queryByDisplayValue("OLD_KEY")).not.toBeNull(); + harness.reconnect(); + await settle(); + expect(view.queryByDisplayValue("OLD_KEY")).not.toBeNull(); + expect(view.queryByText("Loading secrets…")).toBeNull(); + + act(() => answer?.([{ key: "NEW_KEY", value: "new" }])); + await waitFor(() => expect(view.queryByDisplayValue("NEW_KEY")).not.toBeNull()); + }); + + test("keeps a row the user edited when the reload answers", async () => { + const view = renderSecrets(); + const keyField = await waitFor(() => view.getByDisplayValue("OLD_KEY")); + typeInto(keyField, "EDITED_KEY"); + await waitFor(() => expect(view.queryByDisplayValue("EDITED_KEY")).not.toBeNull()); + + serverSecrets = [{ key: "SERVER_KEY", value: "server" }]; + await harness.drop(); + harness.reconnect(); + await settle(); + + expect(view.queryByDisplayValue("EDITED_KEY")).not.toBeNull(); + expect(view.queryByDisplayValue("SERVER_KEY")).toBeNull(); + }); +}); + +// The Secrets and MCP sections offer the global secret keys as {secret:"KEY"} references. +describe.each([...GAP_MODES])("useGlobalSecretKeys across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + serverSecrets = [{ key: "OLD_KEY", value: "old" }]; + harness = createConnectionHarness(mode, serverOverrides); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps the keys while offline and until the reload answers", async () => { + const view = renderHook(() => useGlobalSecretKeys(useAPI().api)[0], { + wrapper: harness.Wrapper, + }); + await waitFor(() => expect(view.result.current).toEqual(["OLD_KEY"])); + + let answer: ((secrets: Secret[]) => void) | null = null; + serverSecrets = new Promise((resolve) => (answer = resolve)); + await harness.drop(); + await settle(); + expect(view.result.current).toEqual(["OLD_KEY"]); + harness.reconnect(); + await settle(); + expect(view.result.current).toEqual(["OLD_KEY"]); + + act(() => answer?.([{ key: "NEW_KEY", value: "new" }])); + await waitFor(() => expect(view.result.current).toEqual(["NEW_KEY"])); + }); +}); diff --git a/src/browser/features/Settings/Sections/SecretsSection.tsx b/src/browser/features/Settings/Sections/SecretsSection.tsx index e56a9861f7b..6c76c45c4a5 100644 --- a/src/browser/features/Settings/Sections/SecretsSection.tsx +++ b/src/browser/features/Settings/Sections/SecretsSection.tsx @@ -2,7 +2,8 @@ import React, { useCallback, useEffect, useRef, useState } from "react"; import { Loader2, Trash2 } from "lucide-react"; import type { ProjectConfig } from "@/common/types/project"; import type { Secret } from "@/common/types/secrets"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, type APIClient } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { useProjectContext } from "@/browser/contexts/ProjectContext"; import { useSettings } from "@/browser/contexts/SettingsContext"; import { Button } from "@/browser/components/Button/Button"; @@ -119,6 +120,35 @@ function isProjectSecretsTarget( return project !== undefined && project.parentProjectPath == null; } +/** + * The keys of the global secrets, loaded through `api`. Offline (and while a reconnect reloads), + * the shown keys stay. Shared by the Secrets and MCP sections. + */ +export function useGlobalSecretKeys(api: APIClient | null) { + const [globalSecretKeys, setGlobalSecretKeys] = useState([]); + useEffect(() => { + if (!api) return; + + let cancelled = false; + (async () => { + try { + const secrets = await api.secrets.get({}); + if (cancelled) return; + setGlobalSecretKeys(secrets.map((s) => s.key)); + } catch (err) { + if (cancelled || isAbortError(err)) return; + console.error("Failed to load global secrets:", err); + setGlobalSecretKeys([]); + } + })(); + + return () => { + cancelled = true; + }; + }, [api]); + return [globalSecretKeys, setGlobalSecretKeys] as const; +} + export const SecretsSection: React.FC = () => { const { api } = useAPI(); const { userProjects } = useProjectContext(); @@ -142,7 +172,8 @@ export const SecretsSection: React.FC = () => { const [secrets, setSecrets] = useState([]); const [visibleSecrets, setVisibleSecrets] = useState>(() => new Set()); - const [globalSecretKeys, setGlobalSecretKeys] = useState([]); + // Global secret keys (used for {secret:"KEY"} project secret values). + const [globalSecretKeys, setGlobalSecretKeys] = useGlobalSecretKeys(api); const [injectedGlobalSecretKeys, setInjectedGlobalSecretKeys] = useState([]); // Track the last plaintext value per row index so toggling Source back to @@ -192,6 +223,12 @@ export const SecretsSection: React.FC = () => { const currentProjectPath = scope === "project" ? selectedProject : undefined; const isDirty = !secretsEqual(secrets, loadedSecrets); + // For loadSecrets: a reload must not replace rows the user edited. + const isDirtyRef = useRef(isDirty); + isDirtyRef.current = isDirty; + // The scope and project (JSON) whose loaded secrets are shown; null while none are. + const shownSecretsKeyRef = useRef(null); + const actionsDisabled = !isDirty || saving || loading; const sortedGlobalSecretKeys = globalSecretKeys .slice() @@ -207,28 +244,36 @@ export const SecretsSection: React.FC = () => { const isStaleRequest = () => loadSecretsRequestVersionRef.current !== requestVersion; - if (!api) { + // Only a new scope or project clears the list and shows "Loading": a new client (reconnect) + // reloads the same secrets behind the shown rows, and keeps rows the user edited. + const secretsKey = JSON.stringify([scope, currentProjectPath]); + if (shownSecretsKeyRef.current !== secretsKey) { + shownSecretsKeyRef.current = null; setLoadedSecrets([]); setSecrets([]); setInjectedGlobalSecretKeys([]); setVisibleSecrets(new Set()); - setError(null); - setLoading(false); - return; + setLoading(true); + // The cleared rows hold no edit (the next render agrees). + isDirtyRef.current = false; } + const keepEdits = isDirtyRef.current; + setError(null); - if (scope === "project" && !currentProjectPath) { - setLoadedSecrets([]); - setSecrets([]); - setInjectedGlobalSecretKeys([]); - setVisibleSecrets(new Set()); - setError(null); + if (!api) { + // Offline, keep the shown secrets: the reconnect reloads them. setLoading(false); return; } - setLoading(true); - setError(null); + const applyLoadedSecrets = (nextSecrets: Secret[]) => { + shownSecretsKeyRef.current = secretsKey; + setLoadedSecrets(nextSecrets); + if (keepEdits) return; + setSecrets(nextSecrets); + setVisibleSecrets(new Set()); + lastLiteralValuesRef.current = new Map(); + }; try { if (scope === "project") { @@ -247,8 +292,7 @@ export const SecretsSection: React.FC = () => { if (isStaleRequest()) { return; } - setLoadedSecrets(nextSecrets); - setSecrets(nextSecrets); + applyLoadedSecrets(nextSecrets); try { const injectedKeys = await api.secrets.getInjectedGlobals({ projectPath }); @@ -257,7 +301,7 @@ export const SecretsSection: React.FC = () => { } setInjectedGlobalSecretKeys(injectedKeys); } catch (err) { - if (isStaleRequest()) { + if (isStaleRequest() || isAbortError(err)) { return; } const message = err instanceof Error ? err.message : "Failed to load injected globals"; @@ -269,18 +313,12 @@ export const SecretsSection: React.FC = () => { if (isStaleRequest()) { return; } - setLoadedSecrets(nextSecrets); - setSecrets(nextSecrets); + applyLoadedSecrets(nextSecrets); setInjectedGlobalSecretKeys([]); } - - if (isStaleRequest()) { - return; - } - setVisibleSecrets(new Set()); - lastLiteralValuesRef.current = new Map(); } catch (err) { - if (isStaleRequest()) { + // A lost connection is no load failure: keep the shown secrets for the reconnect. + if (isStaleRequest() || isAbortError(err)) { return; } const message = err instanceof Error ? err.message : "Failed to load secrets"; @@ -301,31 +339,6 @@ export const SecretsSection: React.FC = () => { void loadSecrets(); }, [loadSecrets]); - // Load global secret keys (used for {secret:"KEY"} project secret values). - useEffect(() => { - if (!api) { - setGlobalSecretKeys([]); - return; - } - - let cancelled = false; - (async () => { - try { - const secrets = await api.secrets.get({}); - if (cancelled) return; - setGlobalSecretKeys(secrets.map((s) => s.key)); - } catch (err) { - if (cancelled) return; - console.error("Failed to load global secrets:", err); - setGlobalSecretKeys([]); - } - })(); - - return () => { - cancelled = true; - }; - }, [api]); - const addSecret = useCallback(() => { setSecrets((prev) => [...prev, { key: "", value: "" }]); }, []); @@ -505,7 +518,7 @@ export const SecretsSection: React.FC = () => { } finally { setSaving(false); } - }, [api, currentProjectPath, scope, secrets]); + }, [api, currentProjectPath, scope, secrets, setGlobalSecretKeys]); return (
@@ -778,19 +791,10 @@ export const SecretsSection: React.FC = () => {
- -
diff --git a/src/browser/features/Settings/Sections/ServerAccessSection.test.tsx b/src/browser/features/Settings/Sections/ServerAccessSection.test.tsx new file mode 100644 index 00000000000..4d0c6efdce3 --- /dev/null +++ b/src/browser/features/Settings/Sections/ServerAccessSection.test.tsx @@ -0,0 +1,68 @@ +// Bootstrap Happy DOM before react-dom evaluates. +import "../../../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, render, waitFor } from "@testing-library/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 { ServerAuthSession } from "@/common/orpc/types"; +import { ServerAccessSection } from "./ServerAccessSection"; + +function session(id: string, label: string): ServerAuthSession { + return { id, label, createdAtMs: 1, lastUsedAtMs: 1, isCurrent: false }; +} + +let harness: ConnectionHarness; +/** The sessions the next server lists, or a promise of them. */ +let serverSessions: ServerAuthSession[] | Promise; + +// The session list must stay on screen while offline and while the new client reloads it: +// "Loading sessions..." only replaces an empty list. +describe.each([...GAP_MODES])("ServerAccessSection across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + serverSessions = [session("s1", "Laptop")]; + harness = createConnectionHarness(mode, (): TestApiOverrides => { + const sessions = serverSessions; + return { serverAuth: { listSessions: () => Promise.resolve(sessions) } }; + }); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps the listed sessions until the reload answers", async () => { + const view = render( + + + + ); + await view.findByText("Laptop"); + + let answer: ((sessions: ServerAuthSession[]) => void) | null = null; + serverSessions = new Promise((resolve) => (answer = resolve)); + await harness.drop(); + expect(view.queryByText("Laptop")).not.toBeNull(); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + expect(view.queryByText("Laptop")).not.toBeNull(); + expect(view.queryByText("Loading sessions...")).toBeNull(); + + act(() => answer?.([session("s2", "Phone")])); + await view.findByText("Phone"); + await waitFor(() => expect(view.queryByText("Laptop")).toBeNull()); + }); +}); diff --git a/src/browser/features/Settings/Sections/ServerAccessSection.tsx b/src/browser/features/Settings/Sections/ServerAccessSection.tsx index 5fde6bc85cb..e518d0e3907 100644 --- a/src/browser/features/Settings/Sections/ServerAccessSection.tsx +++ b/src/browser/features/Settings/Sections/ServerAccessSection.tsx @@ -1,6 +1,7 @@ import { useCallback, useEffect, useState } from "react"; import { Button } from "@/browser/components/Button/Button"; import { useAPI } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import type { ServerAuthSession } from "@/common/orpc/types"; import { getErrorMessage } from "@/common/utils/errors"; @@ -33,6 +34,38 @@ function formatRelativeTime(timestampMs: number): string { return `${Math.floor(deltaMs / dayMs)}d ago`; } +function SessionRow(props: { + session: ServerAuthSession; + loading: boolean; + revoking: boolean; + onRevoke: () => void; +}) { + const session = props.session; + return ( +
+
+
+ {session.label} + {session.isCurrent ? " (Current)" : ""} +
+
+ Last active {formatRelativeTime(session.lastUsedAtMs)} · Created{" "} + {formatTimestamp(session.createdAtMs)} +
+
+ + +
+ ); +} + export function ServerAccessSection() { const { api, retry } = useAPI(); const [sessions, setSessions] = useState([]); @@ -43,7 +76,7 @@ export function ServerAccessSection() { const refreshSessions = useCallback(async () => { if (!api) { - setSessions([]); + // Offline, keep the shown sessions: the reconnect reloads them. setLoading(false); return; } @@ -55,6 +88,8 @@ export function ServerAccessSection() { const nextSessions = await api.serverAuth.listSessions(); setSessions(nextSessions); } catch (refreshError) { + // A lost connection is no load failure: keep the shown sessions for the reconnect. + if (isAbortError(refreshError)) return; const message = getErrorMessage(refreshError); setError(message); setSessions([]); @@ -155,41 +190,24 @@ export function ServerAccessSection() { {error ?
{error}
: null} - {loading ? ( + {/* A reload (Refresh, reconnect) keeps the shown sessions: "Loading" only replaces an empty list. */} + {loading && sessions.length === 0 ? (
Loading sessions...
) : sessions.length === 0 ? (
No active sessions found.
) : (
- {sessions.map((session) => { - const revokingThisSession = revokingSessionId === session.id; - - return ( -
-
-
- {session.label} - {session.isCurrent ? " (Current)" : ""} -
-
- Last active {formatRelativeTime(session.lastUsedAtMs)} · Created{" "} - {formatTimestamp(session.createdAtMs)} -
-
- - -
- ); - })} + {sessions.map((session) => ( + { + void handleRevokeSession(session); + }} + /> + ))}
)} From 5ec911819088fd2180b629eb26de5cf979fa80bb Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 10:33:01 +0000 Subject: [PATCH 06/14] refactor: settings sections, splash screens and update channels keep shown data while reloading --- src/browser/App.tsx | 2 +- .../Settings/Sections/BackupSection.tsx | 27 +++-- .../Sections/BackupSectionReconnect.test.tsx | 87 +++++++++++++++ .../Sections/ExperimentsSection.test.tsx | 33 ++++++ .../Settings/Sections/ExperimentsSection.tsx | 17 ++- .../Settings/Sections/GeneralSection.test.tsx | 38 +++++++ .../Settings/Sections/GeneralSection.tsx | 15 ++- .../Settings/Sections/TasksSection.tsx | 53 +++++---- .../Sections/TasksSection.ui.test.tsx | 104 +++++++++++++++++- .../SplashScreens/SplashScreenProvider.tsx | 26 +++-- .../SplashScreenProviderReconnect.test.tsx | 82 ++++++++++++++ 11 files changed, 430 insertions(+), 54 deletions(-) create mode 100644 src/browser/features/Settings/Sections/BackupSectionReconnect.test.tsx create mode 100644 src/browser/features/SplashScreens/SplashScreenProviderReconnect.test.tsx diff --git a/src/browser/App.tsx b/src/browser/App.tsx index 8e652e65192..7f28ded7f3c 100644 --- a/src/browser/App.tsx +++ b/src/browser/App.tsx @@ -298,7 +298,7 @@ function AppInner() { const [supportedUpdateChannels, setSupportedUpdateChannels] = useState([]); useEffect(() => { - setSupportedUpdateChannels([]); + // No reset here: a reconnect (new client) keeps the listed channels until the reload answers. if (!api) return; let active = true; diff --git a/src/browser/features/Settings/Sections/BackupSection.tsx b/src/browser/features/Settings/Sections/BackupSection.tsx index 1234b6fb40b..e3039f40ec7 100644 --- a/src/browser/features/Settings/Sections/BackupSection.tsx +++ b/src/browser/features/Settings/Sections/BackupSection.tsx @@ -172,6 +172,14 @@ function draftsEqual(left: BackupDraft, right: BackupDraft): boolean { ); } +/** The save error for the first required field the draft leaves blank, or null. */ +function getMissingFieldError(draft: BackupDraft): string | null { + if (draft.repoUrl.trim() === "") return "Repository URL is required."; + if (draft.branch.trim() === "") return "Branch is required."; + if (draft.path.trim() === "") return "Subdirectory is required."; + return null; +} + function getOperationErrorMessage(error: BackupOperationError): string { if (!error.files?.length) return error.message; return `${error.message}: ${error.files.join(", ")}`; @@ -312,6 +320,7 @@ export function BackupSection() { const refreshGenerationRef = useRef(0); const draftRef = useRef(draft); const savedDraftRef = useRef(savedDraft); + const settingsLoadedRef = useRef(false); const refreshRef = useRef<((options?: { markFresh?: boolean }) => Promise) | null>(null); draftRef.current = draft; savedDraftRef.current = savedDraft; @@ -341,7 +350,9 @@ export function BackupSection() { // Liveness belongs to this API subscription. An older effect can finish after its // replacement starts, so sharing this flag across generations creates stale writes. let streamLive = false; - setLoading(true); + // Only the first load hides the form: a new client (reconnect) reloads behind the shown + // settings. Destructive actions still wait until the reload is fresh. + if (!settingsLoadedRef.current) setLoading(true); setSettingsFresh(false); setSaveError(null); @@ -360,6 +371,7 @@ export function BackupSection() { setDraft(nextDraft); } savedDraftRef.current = nextDraft; + settingsLoadedRef.current = true; setSavedDraft(nextDraft); if (options?.markFresh !== false && streamLive) setSettingsFresh(true); setSaveError(null); @@ -445,16 +457,9 @@ export function BackupSection() { async function handleSave() { if (!api || activeAction !== null) return; - if (draft.repoUrl.trim() === "") { - setSaveError("Repository URL is required."); - return; - } - if (draft.branch.trim() === "") { - setSaveError("Branch is required."); - return; - } - if (draft.path.trim() === "") { - setSaveError("Subdirectory is required."); + const missingField = getMissingFieldError(draft); + if (missingField != null) { + setSaveError(missingField); return; } diff --git a/src/browser/features/Settings/Sections/BackupSectionReconnect.test.tsx b/src/browser/features/Settings/Sections/BackupSectionReconnect.test.tsx new file mode 100644 index 00000000000..5db623ab1e7 --- /dev/null +++ b/src/browser/features/Settings/Sections/BackupSectionReconnect.test.tsx @@ -0,0 +1,87 @@ +// Bootstrap Happy DOM before react-dom evaluates. +import "../../../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, render, waitFor } from "@testing-library/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 { ThemeProvider } from "@/browser/contexts/ThemeContext"; +import { TooltipProvider } from "@/browser/components/Tooltip/Tooltip"; +import type { TestApiOverrides } from "@/browser/testUtils"; +import { BackupSection } from "./BackupSection"; + +interface BackupSettings { + repoUrl: string; + branch: string; + path: string; +} + +let harness: ConnectionHarness; +/** The settings the next server returns, or a promise of them. */ +let serverSettings: BackupSettings | Promise; + +function serverOverrides(): TestApiOverrides { + const settings = serverSettings; + return { + backup: { getSettings: () => Promise.resolve(settings) }, + config: { + // A live config stream that sends nothing. + onConfigChanged: () => + Promise.resolve( + (async function* () { + await new Promise(() => undefined); + yield undefined; + })() + ), + }, + }; +} + +// A reconnect reloads the backup settings through the new client. The form stays on screen +// ("Loading backup settings..." replaces it only before the first load). +describe.each([...GAP_MODES])("BackupSection across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + serverSettings = { repoUrl: "git@example.com:old.git", branch: "main", path: "mux/" }; + harness = createConnectionHarness(mode, serverOverrides); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps the form until the reload answers", async () => { + const view = render( + + + + + + + + ); + await waitFor(() => expect(view.queryByDisplayValue("git@example.com:old.git")).not.toBeNull()); + + let answer: ((settings: BackupSettings) => void) | null = null; + serverSettings = new Promise((resolve) => (answer = resolve)); + await harness.drop(); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + expect(view.queryByText("Loading backup settings...")).toBeNull(); + expect(view.queryByDisplayValue("git@example.com:old.git")).not.toBeNull(); + + act(() => answer?.({ repoUrl: "git@example.com:new.git", branch: "main", path: "mux/" })); + await waitFor(() => expect(view.queryByDisplayValue("git@example.com:new.git")).not.toBeNull()); + }); +}); diff --git a/src/browser/features/Settings/Sections/ExperimentsSection.test.tsx b/src/browser/features/Settings/Sections/ExperimentsSection.test.tsx index 0da916653fb..d3eb8135c03 100644 --- a/src/browser/features/Settings/Sections/ExperimentsSection.test.tsx +++ b/src/browser/features/Settings/Sections/ExperimentsSection.test.tsx @@ -3,6 +3,7 @@ import { afterAll, afterEach, beforeEach, describe, expect, mock, test } from "b import { GlobalWindow } from "happy-dom"; import type { ReactNode } from "react"; import { APIProvider, type APIClient } from "@/browser/contexts/API"; +import { createConnectionHarness, GAP_MODES } from "@/browser/contexts/connection.testHarness"; import * as ActualExperimentsModule from "@/browser/hooks/useExperiments"; import * as ActualTelemetryModule from "@/browser/hooks/useTelemetry"; import { createTestApiClient, createTestConfig, type TestApiOverrides } from "@/browser/testUtils"; @@ -239,6 +240,38 @@ describe("PortableDesktopExperimentWarning", () => { expect(view.getByLabelText("Toggle RLM Mode")).toBeTruthy(); }); + // A reconnect checks the prerequisite again through the new client. The warning stays on + // screen while offline and until that check answers. + describe.each([...GAP_MODES])("across a reconnect (%s)", (mode) => { + test("keeps the missing-binary warning until the new check answers", async () => { + let nextStatus: Promise<{ available: false; reason: "binary_not_found" }> = Promise.resolve({ + available: false, + reason: "binary_not_found", + }); + const harness = createConnectionHarness(mode, (): TestApiOverrides => { + const status = nextStatus; + return { desktop: { getPrereqStatus: () => status } }; + }); + const view = render( + + + + ); + await waitFor(() => + expect(view.container.textContent).toContain("Portable Desktop is currently disabled") + ); + + nextStatus = new Promise(() => undefined); + await harness.drop(); + expect(view.container.textContent).toContain("Portable Desktop is currently disabled"); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + expect(view.container.textContent).toContain("Portable Desktop is currently disabled"); + }); + }); + test("loads prereq status on mount and shows the missing-binary warning when needed", async () => { const getPrereqStatus = mock(() => Promise.resolve({ available: false as const, reason: "binary_not_found" as const }) diff --git a/src/browser/features/Settings/Sections/ExperimentsSection.tsx b/src/browser/features/Settings/Sections/ExperimentsSection.tsx index 532017a45dc..c656d5b0200 100644 --- a/src/browser/features/Settings/Sections/ExperimentsSection.tsx +++ b/src/browser/features/Settings/Sections/ExperimentsSection.tsx @@ -23,6 +23,7 @@ import { import type { ApiServerStatus, DesktopPrereqStatus } from "@/common/orpc/types"; import { Input } from "@/browser/components/Input/Input"; import { useAPI } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { useTelemetry } from "@/browser/hooks/useTelemetry"; import { ArtifactsExperimentConfig } from "./ArtifactsExperimentConfig"; import { AutoModelRoutingExperimentConfig } from "./AutoModelRoutingExperimentConfig"; @@ -94,6 +95,10 @@ function ExperimentRow(props: ExperimentRowProps) { ); } +function isPortableDesktopBinaryMissing(status: DesktopPrereqStatus | null): boolean { + return !status?.available && status?.reason === "binary_not_found"; +} + export function PortableDesktopExperimentWarning() { const enabled = useExperimentValue(EXPERIMENT_IDS.PORTABLE_DESKTOP); const { api } = useAPI(); @@ -125,7 +130,8 @@ export function PortableDesktopExperimentWarning() { setPrereqStatus(nextStatus); } catch (e) { - if (requestIdRef.current !== requestId) { + // A lost connection is no check failure: the reconnect checks again. + if (requestIdRef.current !== requestId || isAbortError(e)) { return; } @@ -140,10 +146,13 @@ export function PortableDesktopExperimentWarning() { useEffect(() => { if (!enabled || !api) { requestIdRef.current += 1; - setPrereqStatus(null); setLoading(false); setRestarting(false); - setError(null); + // Offline (`!api`), keep the shown status: the reconnect checks again. + if (!enabled) { + setPrereqStatus(null); + setError(null); + } return; } @@ -173,7 +182,7 @@ export function PortableDesktopExperimentWarning() { } }, [api]); - const isBinaryMissing = !prereqStatus?.available && prereqStatus?.reason === "binary_not_found"; + const isBinaryMissing = isPortableDesktopBinaryMissing(prereqStatus); if (!enabled || !isBinaryMissing) { return null; diff --git a/src/browser/features/Settings/Sections/GeneralSection.test.tsx b/src/browser/features/Settings/Sections/GeneralSection.test.tsx index 16e02065c3e..87013e0ec47 100644 --- a/src/browser/features/Settings/Sections/GeneralSection.test.tsx +++ b/src/browser/features/Settings/Sections/GeneralSection.test.tsx @@ -7,6 +7,7 @@ import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; import { ThemeProvider } from "@/browser/contexts/ThemeContext"; import { APIProvider } from "@/browser/contexts/API"; +import { createConnectionHarness, GAP_MODES } from "@/browser/contexts/connection.testHarness"; import { createTestApiClient, createTestConfig, @@ -943,6 +944,43 @@ describe("GeneralSection", () => { await waitFor(() => expect(toggle.getAttribute("aria-checked")).toBe("false")); }); + // A reconnect reloads the config through the new client. The archive controls keep their + // loaded values and stay enabled while that reload is pending. + describe.each([...GAP_MODES])("across a reconnect (%s)", (mode) => { + test("keeps the archive controls enabled while the reload is pending", async () => { + const setup = createMockAPI({ worktreeArchiveBehavior: "delete" }); + mockApi = setup.api; + getAppConfigStore().setClient(createTestApiClient(mockApi)); + const harness = createConnectionHarness(mode, () => mockApi); + const view = render( + + + + + + ); + await waitFor(() => { + const trigger = getSelectTrigger(view, "Worktree archive behavior"); + expect(trigger.textContent).toContain("Delete checkout"); + expect(trigger.hasAttribute("disabled")).toBe(false); + }); + + mockApi = { + ...setup.api, + config: { ...setup.api.config, getConfig: () => new Promise(() => undefined) }, + }; + await harness.drop(); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + + const trigger = getSelectTrigger(view, "Worktree archive behavior"); + expect(trigger.textContent).toContain("Delete checkout"); + expect(trigger.hasAttribute("disabled")).toBe(false); + }); + }); + test("renders the worktree archive behavior copy and loads the saved value", async () => { const { view } = renderGeneralSection({ coderWorkspaceArchiveBehavior: "delete", diff --git a/src/browser/features/Settings/Sections/GeneralSection.tsx b/src/browser/features/Settings/Sections/GeneralSection.tsx index a3e04b0819d..555b141081b 100644 --- a/src/browser/features/Settings/Sections/GeneralSection.tsx +++ b/src/browser/features/Settings/Sections/GeneralSection.tsx @@ -18,6 +18,7 @@ import { useChatTranscriptFullWidth } from "@/browser/hooks/useChatTranscriptFul import { useTelemetry } from "@/browser/hooks/useTelemetry"; import { useTranscriptDensity } from "@/browser/hooks/useTranscriptDensity"; import { useAPI } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { useExperiment, useExperimentValue } from "@/browser/hooks/useExperiments"; import { EXPERIMENT_IDS } from "@/common/constants/experiments"; import assert from "@/common/utils/assert"; @@ -314,7 +315,8 @@ export function GeneralSection() { return; } - setArchiveSettingsLoaded(false); + // No setArchiveSettingsLoaded(false): a new client (reconnect) reloads behind the loaded + // values, and the nonces below keep a choice the user makes meanwhile. const archiveBehaviorNonce = ++archiveBehaviorLoadNonceRef.current; const llmDebugLogsNonce = ++llmDebugLogsLoadNonceRef.current; const keepScreenAwakeNonce = ++keepScreenAwakeLoadNonceRef.current; @@ -350,7 +352,10 @@ export function GeneralSection() { setKeepScreenAwake(keepScreenAwakeSavedRef.current); } }) - .catch(() => { + .catch((error: unknown) => { + // A lost connection is no read failure: the reconnect reads again. The defaults must + // not become editable (and saved) in the meantime. + if (isAbortError(error)) return; if (archiveBehaviorNonce === archiveBehaviorLoadNonceRef.current) { // Fall back to the safe defaults already in state so the controls can recover after a // config read failure and the next user change can persist a fresh value. @@ -359,6 +364,8 @@ export function GeneralSection() { }); }, [api]); + const archiveControlsDisabled = !api?.config?.updateCoderPrefs || !archiveSettingsLoaded; + const queueArchiveBehaviorUpdate = useCallback(() => { if (!api?.config?.updateCoderPrefs || !archiveSettingsLoaded) { return; @@ -1147,7 +1154,7 @@ export function GeneralSection() { onValueChange={(value) => handleArchiveBehaviorChange(value as CoderWorkspaceArchiveBehavior) } - disabled={!api?.config?.updateCoderPrefs || !archiveSettingsLoaded} + disabled={archiveControlsDisabled} > @@ -1175,7 +1182,7 @@ export function GeneralSection() { onValueChange={(value) => handleWorktreeArchiveBehaviorChange(value as WorktreeArchiveBehavior) } - disabled={!api?.config?.updateCoderPrefs || !archiveSettingsLoaded} + disabled={archiveControlsDisabled} > diff --git a/src/browser/features/Settings/Sections/TasksSection.tsx b/src/browser/features/Settings/Sections/TasksSection.tsx index be823fd0b0c..e70b4ebf335 100644 --- a/src/browser/features/Settings/Sections/TasksSection.tsx +++ b/src/browser/features/Settings/Sections/TasksSection.tsx @@ -46,6 +46,7 @@ import { } from "@/common/types/thinking"; import { normalizeModelInput } from "@/common/utils/ai/normalizeModelInput"; import { getErrorMessage } from "@/common/utils/errors"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { enforceThinkingPolicy } from "@/common/utils/thinking/policy"; import { normalizeAgentId } from "@/common/utils/agentIds"; import { WORKSPACE_DEFAULTS } from "@/constants/workspaceDefaults"; @@ -282,6 +283,11 @@ function areAgentAiDefaultsEqual(a: AgentAiDefaults, b: AgentAiDefaults): boolea return true; } +function clearSaveTimer(timerRef: { current: ReturnType | null }): void { + if (timerRef.current) clearTimeout(timerRef.current); + timerRef.current = null; +} + function coerceAgentId(value: unknown): string { return normalizeAgentId(value, WORKSPACE_DEFAULTS.agentId); } @@ -424,12 +430,13 @@ export function TasksSection() { // Resolve the workspace's active model so that when a sub-agent's model is // "Inherit", we show thinking levels for the workspace model (falling back to // the global default). This mirrors the workspace model resolution chain used when sending messages. - const selectedWorkspaceId = selectedWorkspace?.workspaceId ?? null; - const workspaceModel = useWorkspaceAiSelection(selectedWorkspaceId ?? "").model; + const selectedWorkspaceId = selectedWorkspace?.workspaceId ?? ""; + const workspaceModel = useWorkspaceAiSelection(selectedWorkspaceId).model; const inheritedEffectiveModel = selectedWorkspaceId ? workspaceModel : getDefaultModel(); const lastSyncedTaskSettingsRef = useRef(null); const lastSyncedAgentAiDefaultsRef = useRef(null); + const shownAgentsScopeRef = useRef(null); useEffect(() => { if (!api) return; @@ -441,6 +448,12 @@ export function TasksSection() { void api.config .getConfig() .then((cfg) => { + // A reload after a reconnect keeps values the user changed and not yet saved: the + // autosave sends them through the new client once `loaded` is back. + if (pendingSaveRef.current != null || savingRef.current) { + setLoaded(true); + return; + } const normalizedTaskSettings = normalizeTaskSettings(cfg.taskSettings); setTaskSettings(normalizedTaskSettings); const normalizedAgentDefaults = normalizeAgentAiDefaults(cfg.agentAiDefaults); @@ -453,6 +466,8 @@ export function TasksSection() { setLoaded(true); }) .catch((error: unknown) => { + // A lost connection is no load failure: the reconnect loads again. + if (isAbortError(error)) return; setSaveError(getErrorMessage(error)); setLoadFailed(true); setLoaded(true); @@ -473,8 +488,14 @@ export function TasksSection() { } let cancelled = false; - setAgentsLoaded(false); - setAgentsLoadFailed(false); + // Only a new scope shows "Loading agents…": a new client (reconnect) reloads the same + // scope behind the shown list. + const scope = JSON.stringify([projectPath, workspaceId]); + if (shownAgentsScopeRef.current !== scope) { + shownAgentsScopeRef.current = scope; + setAgentsLoaded(false); + setAgentsLoadFailed(false); + } void Promise.all([ api.agents.list({ projectPath, workspaceId }), @@ -487,8 +508,8 @@ export function TasksSection() { setAgentsLoadFailed(false); setAgentsLoaded(true); }) - .catch(() => { - if (cancelled) return; + .catch((error: unknown) => { + if (cancelled || isAbortError(error)) return; setAgents([]); setEnabledAgentIds(FALLBACK_AGENTS.map((agent) => agent.id)); setAgentsLoadFailed(true); @@ -519,20 +540,14 @@ export function TasksSection() { areAgentAiDefaultsEqual(lastAgentDefaults, agentAiDefaults) ) { pendingSaveRef.current = null; - if (saveTimerRef.current) { - clearTimeout(saveTimerRef.current); - saveTimerRef.current = null; - } + clearSaveTimer(saveTimerRef); return; } // Readers of the shared store see the edit before the debounced save lands. getAppConfigStore().updateOptimistically({ agentAiDefaults }); - if (saveTimerRef.current) { - clearTimeout(saveTimerRef.current); - saveTimerRef.current = null; - } + clearSaveTimer(saveTimerRef); saveTimerRef.current = setTimeout(() => { const flush = () => { @@ -600,10 +615,7 @@ export function TasksSection() { }, 400); return () => { - if (saveTimerRef.current) { - clearTimeout(saveTimerRef.current); - saveTimerRef.current = null; - } + clearSaveTimer(saveTimerRef); }; }, [api, agentAiDefaults, loaded, loadFailed, taskSettings]); @@ -614,10 +626,7 @@ export function TasksSection() { if (loadFailed) return; return () => { - if (saveTimerRef.current) { - clearTimeout(saveTimerRef.current); - saveTimerRef.current = null; - } + clearSaveTimer(saveTimerRef); if (savingRef.current) return; const payload = pendingSaveRef.current; diff --git a/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx b/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx index de34fd6ee00..23b77f76136 100644 --- a/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx +++ b/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx @@ -1,10 +1,12 @@ // Real Radix exports must initialize after the DOM, even when this suite runs first. import { installDom } from "../../../../../tests/ui/dom"; import type React from "react"; -import { cleanup, fireEvent, render, waitFor, within } from "@testing-library/react"; +import { act, cleanup, fireEvent, render, waitFor, within } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:test"; import * as tooltipModule from "@/browser/components/Tooltip/Tooltip"; import { APIProvider, type APIClient } from "@/browser/contexts/API"; +import { createConnectionHarness, GAP_MODES } from "@/browser/contexts/connection.testHarness"; +import { DEFAULT_TASK_SETTINGS } from "@/common/types/tasks"; import { createTestApiClient, createTestConfig, @@ -989,3 +991,103 @@ describe("TasksSection Auto routing defaults", () => { }); }); }); + +// A reconnect reloads the config and the agents through the new client. The agent list stays on +// screen ("Loading agents…" is for another project only), and a value the user changed but the +// debounced autosave has not sent yet is kept and saved, not replaced by the reload. +describe.each([...GAP_MODES])("TasksSection across a reconnect (%s)", (mode) => { + let restoreDom: (() => void) | null = null; + + beforeEach(() => { + restoreDom = installDom(); + experimentsEnabledByDefault = false; + experimentValues = {}; + selectedWorkspaceMock = { projectPath: "/proj", workspaceId: "ws-1" }; + }); + + afterEach(() => { + cleanup(); + apiMock = null; + selectedWorkspaceMock = null; + restoreDom?.(); + restoreDom = null; + }); + + function connect(maxParallelAgentTasks: number, agents: Promise) { + const saveConfig = mock(() => Promise.resolve(undefined)); + apiMock = { + config: { + getConfig: mock(() => + Promise.resolve( + createTestConfig({ + taskSettings: { ...DEFAULT_TASK_SETTINGS, maxParallelAgentTasks }, + }) + ) + ), + saveConfig, + }, + agents: { list: mock(() => agents) }, + }; + return saveConfig; + } + + function maxParallelInput(view: ReturnType): HTMLInputElement { + const row = view.getByText("Max Parallel Agent Tasks").closest(".justify-between"); + const input = row?.querySelector("input"); + if (!(input instanceof window.HTMLInputElement)) throw new Error("No max-parallel input"); + return input; + } + + test("keeps the agent list while the reload is pending", async () => { + connect(3, Promise.resolve(FALLBACK_AGENTS)); + const harness = createConnectionHarness(mode, () => apiMock ?? {}); + const view = render( + + + + ); + await waitFor(() => expect(view.queryByText("Loading agents…")).toBeNull()); + + connect(3, new Promise(() => undefined)); + await harness.drop(); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + expect(view.queryByText("Loading agents…")).toBeNull(); + }); + + test("keeps and saves a change the reload would overwrite", async () => { + connect(3, Promise.resolve(FALLBACK_AGENTS)); + const harness = createConnectionHarness(mode, () => apiMock ?? {}); + const view = render( + + + + ); + await waitFor(() => expect(maxParallelInput(view).value).toBe("3")); + + // The change, then the reconnect, before the debounced autosave fires. + const input = maxParallelInput(view); + const reactPropsKey = Object.keys(input).find((key) => key.startsWith("__reactProps")); + const onChange = ( + reactPropsKey ? (input as unknown as Record)[reactPropsKey] : null + ) as { onChange?: (event: { target: { value: string } }) => void } | null; + if (!onChange?.onChange) throw new Error("Expected the input to expose its onChange handler"); + const change = onChange.onChange; + act(() => change({ target: { value: "7" } })); + const saveConfig = connect(5, Promise.resolve(FALLBACK_AGENTS)); + await harness.drop(); + harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + + expect(maxParallelInput(view).value).toBe("7"); + await waitFor(() => expect(saveConfig).toHaveBeenCalled(), { timeout: 2_000 }); + const payload = saveConfig.mock.calls.at(-1)?.[0] as unknown as { + taskSettings: { maxParallelAgentTasks: number }; + }; + expect(payload.taskSettings.maxParallelAgentTasks).toBe(7); + }); +}); diff --git a/src/browser/features/SplashScreens/SplashScreenProvider.tsx b/src/browser/features/SplashScreens/SplashScreenProvider.tsx index 58911daa77a..db0ea2a0450 100644 --- a/src/browser/features/SplashScreens/SplashScreenProvider.tsx +++ b/src/browser/features/SplashScreens/SplashScreenProvider.tsx @@ -14,6 +14,7 @@ import { type SplashConfig, } from "./index"; import { useAPI } from "@/browser/contexts/API"; +import { isAbortError } from "@/browser/utils/isAbortError"; import { useSettings } from "@/browser/contexts/SettingsContext"; const SplashScreenActiveContext = createContext(false); @@ -41,6 +42,17 @@ export function useOnboardingPause(): OnboardingPauseContextValue { return ctx; } +/** The registry's undismissed splashes, sorted by priority (highest number first). */ +function getActiveSplashQueue(viewedIds: readonly string[]): SplashConfig[] { + return SPLASH_REGISTRY.filter((splash) => { + // Priority 0 = never show + if (splash.priority === 0) return false; + + // Check if this splash has been viewed + return !viewedIds.includes(splash.id); + }).sort((a, b) => b.priority - a.priority); // Higher number = higher priority = shown first +} + export function SplashScreenProvider({ children }: { children: ReactNode }) { const { api } = useAPI(); const { registerOnClose } = useSettings(); @@ -60,18 +72,10 @@ export function SplashScreenProvider({ children }: { children: ReactNode }) { void (async () => { try { const viewedIds = await api.splashScreens.getViewedSplashScreens(); - - // Filter registry to undismissed splashes, sorted by priority (highest number first) - const activeQueue = SPLASH_REGISTRY.filter((splash) => { - // Priority 0 = never show - if (splash.priority === 0) return false; - - // Check if this splash has been viewed - return !viewedIds.includes(splash.id); - }).sort((a, b) => b.priority - a.priority); // Higher number = higher priority = shown first - - setQueue(activeQueue); + setQueue(getActiveSplashQueue(viewedIds)); } catch (error) { + // A lost connection is no load failure: keep the shown splash for the reconnect. + if (isAbortError(error)) return; console.error("Failed to load viewed splash screens:", error); // On error, don't show any splash screens setQueue([]); diff --git a/src/browser/features/SplashScreens/SplashScreenProviderReconnect.test.tsx b/src/browser/features/SplashScreens/SplashScreenProviderReconnect.test.tsx new file mode 100644 index 00000000000..9db77284616 --- /dev/null +++ b/src/browser/features/SplashScreens/SplashScreenProviderReconnect.test.tsx @@ -0,0 +1,82 @@ +// Bootstrap Happy DOM before react-dom evaluates. +import "../../../../tests/ui/dom"; + +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import { act, cleanup, render } from "@testing-library/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 { RouterProvider } from "@/browser/contexts/RouterContext"; +import { SettingsProvider } from "@/browser/contexts/SettingsContext"; +import type { TestApiOverrides } from "@/browser/testUtils"; +import * as RealSplashRegistry from "@/browser/features/SplashScreens/index"; + +// A one-line splash instead of the onboarding wizard (and splashes on, as in the app). +restoreModulesAfterSuite([["@/browser/features/SplashScreens/index", { ...RealSplashRegistry }]]); +void mock.module("@/browser/features/SplashScreens/index", () => ({ + ...RealSplashRegistry, + DISABLE_SPLASH_SCREENS: false, + SPLASH_REGISTRY: [{ id: "test-splash", priority: 1, component: () =>

Test splash

}], +})); + +import { SplashScreenProvider } from "./SplashScreenProvider"; + +let harness: ConnectionHarness; +/** What the next server's getViewedSplashScreens answers, or a promise of it. */ +let serverViewed: string[] | Promise; + +async function settle() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); +} + +// A reconnect reloads the viewed splash list through the new client. If the connection drops +// again while that reload is in flight, the lost call is no load failure: the shown splash stays. +describe.each([...GAP_MODES])("SplashScreenProvider across a reconnect (%s)", (mode) => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + serverViewed = []; + harness = createConnectionHarness(mode, (): TestApiOverrides => { + const viewed = serverViewed; + return { splashScreens: { getViewedSplashScreens: () => Promise.resolve(viewed) } }; + }); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("keeps the shown splash when the reload is lost with the connection", async () => { + const view = render( + + + + + app + + + + + ); + await view.findByText("Test splash"); + + serverViewed = new Promise(() => undefined); + await harness.drop(); + harness.reconnect(); + await settle(); + await harness.drop(); + await settle(); + + expect(view.queryByText("Test splash")).not.toBeNull(); + }); +}); From a10323a47d24483ea426895d5ce2c34b0ec876f0 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 10:35:55 +0000 Subject: [PATCH 07/14] test: type the tasks reconnect save payload --- .../features/Settings/Sections/TasksSection.ui.test.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx b/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx index 23b77f76136..af2dcd7a9d0 100644 --- a/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx +++ b/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx @@ -1014,7 +1014,9 @@ describe.each([...GAP_MODES])("TasksSection across a reconnect (%s)", (mode) => }); function connect(maxParallelAgentTasks: number, agents: Promise) { - const saveConfig = mock(() => Promise.resolve(undefined)); + const saveConfig = mock((_payload: { taskSettings: { maxParallelAgentTasks: number } }) => + Promise.resolve(undefined) + ); apiMock = { config: { getConfig: mock(() => @@ -1085,9 +1087,6 @@ describe.each([...GAP_MODES])("TasksSection across a reconnect (%s)", (mode) => expect(maxParallelInput(view).value).toBe("7"); await waitFor(() => expect(saveConfig).toHaveBeenCalled(), { timeout: 2_000 }); - const payload = saveConfig.mock.calls.at(-1)?.[0] as unknown as { - taskSettings: { maxParallelAgentTasks: number }; - }; - expect(payload.taskSettings.maxParallelAgentTasks).toBe(7); + expect(saveConfig.mock.calls.at(-1)?.[0].taskSettings.maxParallelAgentTasks).toBe(7); }); }); From ec094e681f46ae4c2485f5f0422117296af2cbea Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 10:55:45 +0000 Subject: [PATCH 08/14] style: format the agent context reconnect test --- src/browser/contexts/AgentContext.test.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/browser/contexts/AgentContext.test.tsx b/src/browser/contexts/AgentContext.test.tsx index 1f20aaeca82..60b701ece3e 100644 --- a/src/browser/contexts/AgentContext.test.tsx +++ b/src/browser/contexts/AgentContext.test.tsx @@ -12,7 +12,11 @@ import type { AgentDefinitionDescriptor } from "@/common/types/agentDefinition"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import { AgentProvider, useAgent, type AgentContextValue } from "./AgentContext"; import { APIProvider, type APIClient } from "./API"; -import { createConnectionHarness, GAP_MODES, type ConnectionHarness } from "./connection.testHarness"; +import { + createConnectionHarness, + GAP_MODES, + type ConnectionHarness, +} from "./connection.testHarness"; import { ProjectProvider } from "./ProjectContext"; import { RouterProvider } from "./RouterContext"; import { WorkspaceProvider } from "./WorkspaceContext"; From e55b7afa6689b11fc02ce6425fdbf422bc24e1d9 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 12:06:14 +0000 Subject: [PATCH 09/14] fix: only a lost connection is ignored as no load failure; a server cancel is still reported A server cancel on a live socket is a plain AbortError, and isAbortError() matched it too. Swallowing it left the section stuck: no new client comes to load again. The reloads now ignore only isConnectionLostError(); each effect's own cleanup abort stays handled by its cancelled flag or request check. --- src/browser/contexts/AgentContext.tsx | 4 +-- src/browser/contexts/ProjectContext.tsx | 4 +-- src/browser/contexts/UILayoutsContext.tsx | 4 +-- src/browser/contexts/WorkspaceContext.tsx | 3 +- .../ChatInput/useComposerSuggestions.ts | 6 ++-- .../Settings/Sections/ExperimentsSection.tsx | 4 +-- .../Settings/Sections/GeneralSection.tsx | 4 +-- .../Settings/Sections/SecretsSection.tsx | 8 ++--- .../Sections/ServerAccessSection.test.tsx | 35 +++++++++++++++++++ .../Settings/Sections/ServerAccessSection.tsx | 4 +-- .../Settings/Sections/TasksSection.tsx | 6 ++-- .../SplashScreens/SplashScreenProvider.tsx | 4 +-- src/browser/hooks/useAnalytics.ts | 4 +-- src/browser/hooks/useCoderWorkspace.ts | 10 +++--- src/browser/hooks/useWorkflowRunById.ts | 4 +-- src/browser/hooks/useWorkspaceHeartbeat.ts | 4 +-- 16 files changed, 72 insertions(+), 36 deletions(-) diff --git a/src/browser/contexts/AgentContext.tsx b/src/browser/contexts/AgentContext.tsx index 45493b8aa80..6ca2c4fec98 100644 --- a/src/browser/contexts/AgentContext.tsx +++ b/src/browser/contexts/AgentContext.tsx @@ -12,7 +12,7 @@ import { } from "react"; import { useAPI } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { useOptionalWorkspaceMetadata } from "@/browser/contexts/WorkspaceContext"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import { CUSTOM_EVENTS, createCustomEvent } from "@/common/constants/events"; @@ -172,7 +172,7 @@ function AgentProviderWithState(props: { } } catch (error) { // A lost connection (or the reconnect that replaced the client) is no load failure. - if (isAbortError(error)) return; + if (isConnectionLostError(error)) return; if (isSameFetch(fetchParamsRef.current, projectPath, workspaceId) && isMountedRef.current) { setAgents([]); setAgentBases(getScopeId(workspaceId, projectPath), []); diff --git a/src/browser/contexts/ProjectContext.tsx b/src/browser/contexts/ProjectContext.tsx index 96f7e1af3f1..5dc9fea07d1 100644 --- a/src/browser/contexts/ProjectContext.tsx +++ b/src/browser/contexts/ProjectContext.tsx @@ -19,7 +19,7 @@ import { getDraftScopeId } from "@/common/constants/storage"; import { deleteWorkspaceStorage } from "@/browser/utils/workspaceStorage"; import { getDraftStore } from "@/browser/stores/DraftStore"; import { getErrorMessage } from "@/common/utils/errors"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import type { ProjectWorkspaceCounts } from "@/common/utils/projectRemoval"; import { getProjectRouteId } from "@/common/utils/projectRouteId"; import { SCRATCH_PROJECT_CONFIG_KEY } from "@/common/constants/scratch"; @@ -204,7 +204,7 @@ export function ProjectProvider(props: { children: ReactNode }) { } catch (error) { if (refresh !== projectsRefreshRef.current || !refresh.api) continue; // A lost connection is no load error: the reconnect reloads. - if (isAbortError(error)) continue; + if (isConnectionLostError(error)) continue; // Keep successful data when a later refresh fails. console.error("Failed to load projects:", error); setLoadError(getErrorMessage(error)); diff --git a/src/browser/contexts/UILayoutsContext.tsx b/src/browser/contexts/UILayoutsContext.tsx index 13394f84cb2..0321e6618df 100644 --- a/src/browser/contexts/UILayoutsContext.tsx +++ b/src/browser/contexts/UILayoutsContext.tsx @@ -8,7 +8,7 @@ import { type ReactNode, } from "react"; import { useAPI, useLiveAPI } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import assert from "@/common/utils/assert"; import { DEFAULT_LAYOUT_PRESETS_CONFIG, @@ -99,7 +99,7 @@ export function UILayoutsProvider(props: { children: ReactNode }) { setLoadFailed(false); } catch (error) { // A lost connection is no load failure: keep the shown presets for the reconnect. - if (isAbortError(error)) return; + if (isConnectionLostError(error)) return; setLayoutPresets(DEFAULT_LAYOUT_PRESETS_CONFIG); setLoaded(true); setLoadFailed(true); diff --git a/src/browser/contexts/WorkspaceContext.tsx b/src/browser/contexts/WorkspaceContext.tsx index 2a787e1a15a..11b8be03b81 100644 --- a/src/browser/contexts/WorkspaceContext.tsx +++ b/src/browser/contexts/WorkspaceContext.tsx @@ -41,6 +41,7 @@ import { import { isWorkspaceArchived } from "@/common/utils/archive"; import { appendPinnedTimestamp, reassignPinnedTimestamps } from "@/common/utils/pin"; import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { findAdjacentWorkspaceId } from "@/browser/utils/ui/workspaceDomNav"; import { useRouter } from "@/browser/contexts/RouterContext"; import { setWorkspaceAiMetadata } from "@/browser/utils/aiSelectionIntent"; @@ -1581,7 +1582,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { setLoadError(null); } catch (error) { // A lost connection is no load error: the reconnect reloads. - if (isAbortError(error)) return; + if (isConnectionLostError(error)) return; console.error("Failed to load workspace metadata:", error); // Keep the previous metadata map on failure so scoped preferences are not pruned. setLoadError(getErrorMessage(error)); diff --git a/src/browser/features/ChatInput/useComposerSuggestions.ts b/src/browser/features/ChatInput/useComposerSuggestions.ts index 99682ae8ca6..5283a7ee9aa 100644 --- a/src/browser/features/ChatInput/useComposerSuggestions.ts +++ b/src/browser/features/ChatInput/useComposerSuggestions.ts @@ -1,7 +1,7 @@ import { useEffect, useId, useRef, useState } from "react"; import type { RefObject } from "react"; import { useAPI } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { useExperimentValue } from "@/browser/hooks/useExperiments"; import { usePersistedState } from "@/browser/hooks/usePersistedState"; import { EXPERIMENT_IDS } from "@/common/constants/experiments"; @@ -267,7 +267,7 @@ export function useComposerSuggestions(options: UseComposerSuggestionsOptions) { if (mounted) setPluginCommandsState({ workspaceId: chatWorkspaceId, commands }); }) .catch((error: unknown) => { - if (mounted && !isAbortError(error)) { + if (mounted && !isConnectionLostError(error)) { setPluginCommandsState({ workspaceId: chatWorkspaceId, commands: NO_PLUGIN_COMMANDS }); } }); @@ -317,7 +317,7 @@ export function useComposerSuggestions(options: UseComposerSuggestionsOptions) { }) .catch((error: unknown) => { // A lost connection is no answer: leave the prompts unloaded so the reconnect asks again. - if (isAbortError(error)) return; + if (isConnectionLostError(error)) return; if (mcpWorkspace.current === chatWorkspaceId && mcpRequestId.current === requestId) { mcpLoadedAt.current = Date.now(); } diff --git a/src/browser/features/Settings/Sections/ExperimentsSection.tsx b/src/browser/features/Settings/Sections/ExperimentsSection.tsx index c656d5b0200..6f92697a683 100644 --- a/src/browser/features/Settings/Sections/ExperimentsSection.tsx +++ b/src/browser/features/Settings/Sections/ExperimentsSection.tsx @@ -23,7 +23,7 @@ import { import type { ApiServerStatus, DesktopPrereqStatus } from "@/common/orpc/types"; import { Input } from "@/browser/components/Input/Input"; import { useAPI } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { useTelemetry } from "@/browser/hooks/useTelemetry"; import { ArtifactsExperimentConfig } from "./ArtifactsExperimentConfig"; import { AutoModelRoutingExperimentConfig } from "./AutoModelRoutingExperimentConfig"; @@ -131,7 +131,7 @@ export function PortableDesktopExperimentWarning() { setPrereqStatus(nextStatus); } catch (e) { // A lost connection is no check failure: the reconnect checks again. - if (requestIdRef.current !== requestId || isAbortError(e)) { + if (requestIdRef.current !== requestId || isConnectionLostError(e)) { return; } diff --git a/src/browser/features/Settings/Sections/GeneralSection.tsx b/src/browser/features/Settings/Sections/GeneralSection.tsx index 555b141081b..10365242b57 100644 --- a/src/browser/features/Settings/Sections/GeneralSection.tsx +++ b/src/browser/features/Settings/Sections/GeneralSection.tsx @@ -18,7 +18,7 @@ import { useChatTranscriptFullWidth } from "@/browser/hooks/useChatTranscriptFul import { useTelemetry } from "@/browser/hooks/useTelemetry"; import { useTranscriptDensity } from "@/browser/hooks/useTranscriptDensity"; import { useAPI } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { useExperiment, useExperimentValue } from "@/browser/hooks/useExperiments"; import { EXPERIMENT_IDS } from "@/common/constants/experiments"; import assert from "@/common/utils/assert"; @@ -355,7 +355,7 @@ export function GeneralSection() { .catch((error: unknown) => { // A lost connection is no read failure: the reconnect reads again. The defaults must // not become editable (and saved) in the meantime. - if (isAbortError(error)) return; + if (isConnectionLostError(error)) return; if (archiveBehaviorNonce === archiveBehaviorLoadNonceRef.current) { // Fall back to the safe defaults already in state so the controls can recover after a // config read failure and the next user change can persist a fresh value. diff --git a/src/browser/features/Settings/Sections/SecretsSection.tsx b/src/browser/features/Settings/Sections/SecretsSection.tsx index 6c76c45c4a5..39835e4a590 100644 --- a/src/browser/features/Settings/Sections/SecretsSection.tsx +++ b/src/browser/features/Settings/Sections/SecretsSection.tsx @@ -3,7 +3,7 @@ import { Loader2, Trash2 } from "lucide-react"; import type { ProjectConfig } from "@/common/types/project"; import type { Secret } from "@/common/types/secrets"; import { useAPI, type APIClient } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { useProjectContext } from "@/browser/contexts/ProjectContext"; import { useSettings } from "@/browser/contexts/SettingsContext"; import { Button } from "@/browser/components/Button/Button"; @@ -136,7 +136,7 @@ export function useGlobalSecretKeys(api: APIClient | null) { if (cancelled) return; setGlobalSecretKeys(secrets.map((s) => s.key)); } catch (err) { - if (cancelled || isAbortError(err)) return; + if (cancelled || isConnectionLostError(err)) return; console.error("Failed to load global secrets:", err); setGlobalSecretKeys([]); } @@ -301,7 +301,7 @@ export const SecretsSection: React.FC = () => { } setInjectedGlobalSecretKeys(injectedKeys); } catch (err) { - if (isStaleRequest() || isAbortError(err)) { + if (isStaleRequest() || isConnectionLostError(err)) { return; } const message = err instanceof Error ? err.message : "Failed to load injected globals"; @@ -318,7 +318,7 @@ export const SecretsSection: React.FC = () => { } } catch (err) { // A lost connection is no load failure: keep the shown secrets for the reconnect. - if (isStaleRequest() || isAbortError(err)) { + if (isStaleRequest() || isConnectionLostError(err)) { return; } const message = err instanceof Error ? err.message : "Failed to load secrets"; diff --git a/src/browser/features/Settings/Sections/ServerAccessSection.test.tsx b/src/browser/features/Settings/Sections/ServerAccessSection.test.tsx index 4d0c6efdce3..c51e1741748 100644 --- a/src/browser/features/Settings/Sections/ServerAccessSection.test.tsx +++ b/src/browser/features/Settings/Sections/ServerAccessSection.test.tsx @@ -66,3 +66,38 @@ describe.each([...GAP_MODES])("ServerAccessSection across a reconnect (%s)", (mo await waitFor(() => expect(view.queryByText("Laptop")).toBeNull()); }); }); + +// A server cancel on a live socket is a plain AbortError. It is a real failure: no reconnect +// comes to reload, so swallowing it would leave the section stuck on stale or no sessions. +describe("ServerAccessSection on a server cancel", () => { + let cleanupDom: (() => void) | null = null; + + beforeEach(() => { + cleanupDom = installDom(); + }); + + afterEach(() => { + cleanup(); + cleanupDom?.(); + cleanupDom = null; + }); + + test("reports the cancel while connected", async () => { + harness = createConnectionHarness( + "kept-gap", + (): TestApiOverrides => ({ + serverAuth: { + listSessions: () => Promise.reject(new DOMException("Request cancelled", "AbortError")), + }, + }) + ); + const view = render( + + + + ); + + await view.findByText("Request cancelled"); + expect(view.queryByText("Loading sessions...")).toBeNull(); + }); +}); diff --git a/src/browser/features/Settings/Sections/ServerAccessSection.tsx b/src/browser/features/Settings/Sections/ServerAccessSection.tsx index e518d0e3907..d44f7944e3b 100644 --- a/src/browser/features/Settings/Sections/ServerAccessSection.tsx +++ b/src/browser/features/Settings/Sections/ServerAccessSection.tsx @@ -1,7 +1,7 @@ import { useCallback, useEffect, useState } from "react"; import { Button } from "@/browser/components/Button/Button"; import { useAPI } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import type { ServerAuthSession } from "@/common/orpc/types"; import { getErrorMessage } from "@/common/utils/errors"; @@ -89,7 +89,7 @@ export function ServerAccessSection() { setSessions(nextSessions); } catch (refreshError) { // A lost connection is no load failure: keep the shown sessions for the reconnect. - if (isAbortError(refreshError)) return; + if (isConnectionLostError(refreshError)) return; const message = getErrorMessage(refreshError); setError(message); setSessions([]); diff --git a/src/browser/features/Settings/Sections/TasksSection.tsx b/src/browser/features/Settings/Sections/TasksSection.tsx index e70b4ebf335..7fad7c84c87 100644 --- a/src/browser/features/Settings/Sections/TasksSection.tsx +++ b/src/browser/features/Settings/Sections/TasksSection.tsx @@ -46,7 +46,7 @@ import { } from "@/common/types/thinking"; import { normalizeModelInput } from "@/common/utils/ai/normalizeModelInput"; import { getErrorMessage } from "@/common/utils/errors"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { enforceThinkingPolicy } from "@/common/utils/thinking/policy"; import { normalizeAgentId } from "@/common/utils/agentIds"; import { WORKSPACE_DEFAULTS } from "@/constants/workspaceDefaults"; @@ -467,7 +467,7 @@ export function TasksSection() { }) .catch((error: unknown) => { // A lost connection is no load failure: the reconnect loads again. - if (isAbortError(error)) return; + if (isConnectionLostError(error)) return; setSaveError(getErrorMessage(error)); setLoadFailed(true); setLoaded(true); @@ -509,7 +509,7 @@ export function TasksSection() { setAgentsLoaded(true); }) .catch((error: unknown) => { - if (cancelled || isAbortError(error)) return; + if (cancelled || isConnectionLostError(error)) return; setAgents([]); setEnabledAgentIds(FALLBACK_AGENTS.map((agent) => agent.id)); setAgentsLoadFailed(true); diff --git a/src/browser/features/SplashScreens/SplashScreenProvider.tsx b/src/browser/features/SplashScreens/SplashScreenProvider.tsx index db0ea2a0450..b2b643d275d 100644 --- a/src/browser/features/SplashScreens/SplashScreenProvider.tsx +++ b/src/browser/features/SplashScreens/SplashScreenProvider.tsx @@ -14,7 +14,7 @@ import { type SplashConfig, } from "./index"; import { useAPI } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { useSettings } from "@/browser/contexts/SettingsContext"; const SplashScreenActiveContext = createContext(false); @@ -75,7 +75,7 @@ export function SplashScreenProvider({ children }: { children: ReactNode }) { setQueue(getActiveSplashQueue(viewedIds)); } catch (error) { // A lost connection is no load failure: keep the shown splash for the reconnect. - if (isAbortError(error)) return; + if (isConnectionLostError(error)) return; console.error("Failed to load viewed splash screens:", error); // On error, don't show any splash screens setQueue([]); diff --git a/src/browser/hooks/useAnalytics.ts b/src/browser/hooks/useAnalytics.ts index 01f4096bb95..3dc3dde3611 100644 --- a/src/browser/hooks/useAnalytics.ts +++ b/src/browser/hooks/useAnalytics.ts @@ -7,7 +7,7 @@ import { useAPI } from "@/browser/contexts/API"; import type { analytics } from "@/common/orpc/schemas/analytics"; import type { SavedQuery } from "@/common/types/savedQueries"; import { getErrorMessage } from "@/common/utils/errors"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; export type Summary = z.infer; export type SpendOverTimeItem = z.infer[number]; @@ -150,7 +150,7 @@ function startAnalyticsFetch( }) .catch((error: unknown) => { // A lost connection is no query failure: the reconnect runs the query again. - if (ignore || isAbortError(error)) { + if (ignore || isConnectionLostError(error)) { return; } setState((previousState) => ({ diff --git a/src/browser/hooks/useCoderWorkspace.ts b/src/browser/hooks/useCoderWorkspace.ts index 9314dba597c..96c800ae528 100644 --- a/src/browser/hooks/useCoderWorkspace.ts +++ b/src/browser/hooks/useCoderWorkspace.ts @@ -7,7 +7,7 @@ */ import { useState, useEffect, useCallback, useRef } from "react"; import { useAPI } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import type { CoderInfo, CoderTemplate, @@ -164,7 +164,7 @@ export function useCoderWorkspace({ } } catch (error) { // A lost connection says nothing about the CLI: keep the info and the user's Coder config. - if (seq !== latestAuthSeqRef.current || isAbortError(error)) { + if (seq !== latestAuthSeqRef.current || isConnectionLostError(error)) { return; } @@ -239,7 +239,7 @@ export function useCoderWorkspace({ } }) .catch((error) => { - if (!mounted || isAbortError(error)) return; + if (!mounted || isConnectionLostError(error)) return; setTemplates([]); setTemplatesError(formatCoderListError(error)); }) @@ -287,7 +287,7 @@ export function useCoderWorkspace({ } }) .catch((error) => { - if (!mounted || isAbortError(error)) return; + if (!mounted || isConnectionLostError(error)) return; setExistingWorkspaces([]); setWorkspacesError(formatCoderListError(error)); }) @@ -370,7 +370,7 @@ export function useCoderWorkspace({ } }) .catch((error) => { - if (!mounted || isAbortError(error)) { + if (!mounted || isConnectionLostError(error)) { return; } if (!isTemplateSelected(coderConfigRef.current, templateAtRequest, orgAtRequest)) { diff --git a/src/browser/hooks/useWorkflowRunById.ts b/src/browser/hooks/useWorkflowRunById.ts index fdc09463775..d4354f642fd 100644 --- a/src/browser/hooks/useWorkflowRunById.ts +++ b/src/browser/hooks/useWorkflowRunById.ts @@ -1,7 +1,7 @@ import { useContext, useEffect, useState } from "react"; import { APIContext } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { isActiveWorkflowRunStatus, type WorkflowRunRecord } from "@/common/types/workflow"; const WORKFLOW_RUN_REFRESH_INTERVAL_MS = 2_000; @@ -108,7 +108,7 @@ export function useWorkflowRunById(input: { } } catch (fetchError) { // A lost connection is no load failure: keep the shown run, the reconnect polls again. - if (ignore || isAbortError(fetchError)) { + if (ignore || isConnectionLostError(fetchError)) { return; } setError(fetchError instanceof Error ? fetchError.message : "Failed to load workflow run"); diff --git a/src/browser/hooks/useWorkspaceHeartbeat.ts b/src/browser/hooks/useWorkspaceHeartbeat.ts index 6aaccb03a75..118db448894 100644 --- a/src/browser/hooks/useWorkspaceHeartbeat.ts +++ b/src/browser/hooks/useWorkspaceHeartbeat.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { useAPI } from "@/browser/contexts/API"; -import { isAbortError } from "@/browser/utils/isAbortError"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { useWorkspaceActions } from "@/browser/contexts/WorkspaceContext"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; import { @@ -155,7 +155,7 @@ export function useWorkspaceHeartbeat( }) .catch((loadError) => { // A lost connection is no load failure: keep the shown settings for the reconnect. - if (cancelled || isAbortError(loadError)) return; + if (cancelled || isConnectionLostError(loadError)) return; if (currentWorkspaceIdRef.current !== workspaceId) return; setGlobalDefaultPrompt(undefined); From 9a3c6fd8091aa330ec094cd43b978eed5c6cd9c1 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 12:39:19 +0000 Subject: [PATCH 10/14] fix: kept settings controls do not look live while offline The settings sections keep their loaded values while Xum reconnects. Their server-backed controls stayed interactive offline and dropped the write without a word. Each one now reports the offline state or is disabled while there is no live client: - ServerAccess: Refresh and the revoke buttons are disabled. - Secrets: Save is disabled (Reset stays: it is local). - General: the archive selects, keep-awake, API debug logs, SSH host and default project directory are disabled. The transcript-width switch reports the failure through the live client. - Heartbeat: the fields and ConfigSwitchSetting are disabled. A save the lost connection cut off clears the edit mark, so the reload shows the saved value instead of an unsaved one that looks saved. - Tasks: a change made offline stays pending with an offline message, and the autosave sends it through the next client. A retry never takes the pending change while offline. - Experiments: the Portable Desktop warning's buttons are disabled. Backup already shows that its settings are unavailable while disconnected. --- .../Settings/Sections/ConfigSwitchSetting.tsx | 14 ++- .../Sections/ExperimentsSection.test.tsx | 28 ++++++ .../Settings/Sections/ExperimentsSection.tsx | 65 +++++++++----- .../Settings/Sections/GeneralSection.test.tsx | 31 +++++++ .../Settings/Sections/GeneralSection.tsx | 16 +++- .../Sections/HeartbeatSection.test.tsx | 38 ++++++++ .../Settings/Sections/HeartbeatSection.tsx | 87 +++++++++++++++---- .../Settings/Sections/SecretsSection.test.tsx | 17 ++++ .../Settings/Sections/SecretsSection.tsx | 10 ++- .../Sections/ServerAccessSection.test.tsx | 20 +++++ .../Settings/Sections/ServerAccessSection.tsx | 15 ++-- .../Settings/Sections/TasksSection.tsx | 34 ++++++-- .../Sections/TasksSection.ui.test.tsx | 35 ++++++++ 13 files changed, 351 insertions(+), 59 deletions(-) diff --git a/src/browser/features/Settings/Sections/ConfigSwitchSetting.tsx b/src/browser/features/Settings/Sections/ConfigSwitchSetting.tsx index e8c77b4c8ce..d326a230ed9 100644 --- a/src/browser/features/Settings/Sections/ConfigSwitchSetting.tsx +++ b/src/browser/features/Settings/Sections/ConfigSwitchSetting.tsx @@ -7,7 +7,7 @@ import { TooltipContent, TooltipTrigger, } from "@/browser/components/Tooltip/Tooltip"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; export type SettingsApi = NonNullable["api"]>; @@ -24,9 +24,19 @@ interface ConfigSwitchSettingProps { saveErrorMessage: string; } +/** + * The switch is locked until the saved value loads, and offline: a toggle could not reach the + * server, so it must not look applied. + */ +function isSwitchLocked(enabled: boolean | null, offline: boolean): boolean { + return enabled === null || offline; +} + /** A switch backed by one boolean in the backend config. */ export function ConfigSwitchSetting(props: ConfigSwitchSettingProps) { const { api } = useAPI(); + // Offline the switch is disabled: a toggle could not reach the server. + const offline = useLiveAPI() == null; const [enabled, setEnabled] = useState(null); const [error, setError] = useState(null); // Serialize writes so rapid toggles persist the last choice, and roll back only when the @@ -91,7 +101,7 @@ export function ConfigSwitchSetting(props: ConfigSwitchSettingProps) {
diff --git a/src/browser/features/Settings/Sections/ExperimentsSection.test.tsx b/src/browser/features/Settings/Sections/ExperimentsSection.test.tsx index d3eb8135c03..6f63f1cf051 100644 --- a/src/browser/features/Settings/Sections/ExperimentsSection.test.tsx +++ b/src/browser/features/Settings/Sections/ExperimentsSection.test.tsx @@ -270,6 +270,34 @@ describe("PortableDesktopExperimentWarning", () => { }); expect(view.container.textContent).toContain("Portable Desktop is currently disabled"); }); + + // Offline the kept warning's buttons could not reach the server: they must be disabled. + test("disables the warning's buttons while offline", async () => { + const harness = createConnectionHarness( + mode, + (): TestApiOverrides => ({ + desktop: { + getPrereqStatus: () => + Promise.resolve({ available: false as const, reason: "binary_not_found" as const }), + }, + }) + ); + const view = render( + + + + ); + const restart = (await view.findByRole("button", { + name: "Restart Xum", + })) as HTMLButtonElement; + const checkAgain = view.getByRole("button", { name: "Check again" }) as HTMLButtonElement; + await waitFor(() => expect(restart.disabled).toBe(false)); + + await harness.drop(); + expect(view.container.textContent).toContain("Portable Desktop is currently disabled"); + expect(restart.disabled).toBe(true); + expect(checkAgain.disabled).toBe(true); + }); }); test("loads prereq status on mount and shows the missing-binary warning when needed", async () => { diff --git a/src/browser/features/Settings/Sections/ExperimentsSection.tsx b/src/browser/features/Settings/Sections/ExperimentsSection.tsx index 6f92697a683..1b325528cda 100644 --- a/src/browser/features/Settings/Sections/ExperimentsSection.tsx +++ b/src/browser/features/Settings/Sections/ExperimentsSection.tsx @@ -22,7 +22,7 @@ import { } from "@/browser/components/SelectPrimitive/SelectPrimitive"; import type { ApiServerStatus, DesktopPrereqStatus } from "@/common/orpc/types"; import { Input } from "@/browser/components/Input/Input"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { useTelemetry } from "@/browser/hooks/useTelemetry"; import { ArtifactsExperimentConfig } from "./ArtifactsExperimentConfig"; @@ -99,6 +99,36 @@ function isPortableDesktopBinaryMissing(status: DesktopPrereqStatus | null): boo return !status?.available && status?.reason === "binary_not_found"; } +/** The warning's buttons. Offline they are disabled: they could not reach the server. */ +function PortableDesktopWarningActions(props: { + loading: boolean; + restarting: boolean; + onCheckAgain: () => void; + onRestart: () => void; +}) { + const offline = useLiveAPI() == null; + return ( +
+ + +
+ ); +} + export function PortableDesktopExperimentWarning() { const enabled = useExperimentValue(EXPERIMENT_IDS.PORTABLE_DESKTOP); const { api } = useAPI(); @@ -207,28 +237,17 @@ export function PortableDesktopExperimentWarning() { to enable this feature. If you installed it into a location that xum can already see, choose Check again. If you changed PATH after xum launched, restart xum to pick it up. -
- - -
+ {/* Offline the kept warning stays, but its buttons are disabled. */} + { + void loadPrereqStatus(); + }} + onRestart={() => { + void handleRestart(); + }} + /> {error &&
{error}
} diff --git a/src/browser/features/Settings/Sections/GeneralSection.test.tsx b/src/browser/features/Settings/Sections/GeneralSection.test.tsx index 87013e0ec47..f09ef98ace2 100644 --- a/src/browser/features/Settings/Sections/GeneralSection.test.tsx +++ b/src/browser/features/Settings/Sections/GeneralSection.test.tsx @@ -979,6 +979,37 @@ describe("GeneralSection", () => { expect(trigger.textContent).toContain("Delete checkout"); expect(trigger.hasAttribute("disabled")).toBe(false); }); + + // Offline the loaded values stay, but a control that writes to the server must not look + // usable: its write could not reach the server. + test("disables the server-backed controls while offline", async () => { + const setup = createMockAPI({ worktreeArchiveBehavior: "delete" }); + mockApi = setup.api; + getAppConfigStore().setClient(createTestApiClient(mockApi)); + const harness = createConnectionHarness(mode, () => mockApi); + const view = render( + + + + + + ); + const debugLogs = view.getByRole("switch", { name: "Toggle API Debug Logs" }); + await waitFor(() => { + const trigger = getSelectTrigger(view, "Worktree archive behavior"); + expect(trigger.hasAttribute("disabled")).toBe(false); + }); + expect(debugLogs.hasAttribute("disabled")).toBe(false); + + await harness.drop(); + const trigger = getSelectTrigger(view, "Worktree archive behavior"); + expect(trigger.textContent).toContain("Delete checkout"); + expect(trigger.hasAttribute("disabled")).toBe(true); + expect(debugLogs.hasAttribute("disabled")).toBe(true); + + harness.reconnect(); + await waitFor(() => expect(debugLogs.hasAttribute("disabled")).toBe(false)); + }); }); test("renders the worktree archive behavior copy and loads the saved value", async () => { diff --git a/src/browser/features/Settings/Sections/GeneralSection.tsx b/src/browser/features/Settings/Sections/GeneralSection.tsx index 10365242b57..a0b451825d1 100644 --- a/src/browser/features/Settings/Sections/GeneralSection.tsx +++ b/src/browser/features/Settings/Sections/GeneralSection.tsx @@ -17,7 +17,7 @@ import { import { useChatTranscriptFullWidth } from "@/browser/hooks/useChatTranscriptFullWidth"; import { useTelemetry } from "@/browser/hooks/useTelemetry"; import { useTranscriptDensity } from "@/browser/hooks/useTranscriptDensity"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { useExperiment, useExperimentValue } from "@/browser/hooks/useExperiments"; import { EXPERIMENT_IDS } from "@/common/constants/experiments"; @@ -177,6 +177,10 @@ let compactionStrategyWrites: Promise = Promise.resolve(); export function GeneralSection() { const { themePreference, setTheme } = useTheme(); const { api } = useAPI(); + // Offline, the loaded values stay on screen, but the controls that write to the server directly + // are disabled: their write could not reach it, and the control must not look saved. + const liveApi = useLiveAPI(); + const offline = liveApi == null; const telemetry = useTelemetry(); const [continuousCompaction, setContinuousCompaction] = useExperiment( EXPERIMENT_IDS.CONTINUOUS_COMPACTION @@ -364,7 +368,8 @@ export function GeneralSection() { }); }, [api]); - const archiveControlsDisabled = !api?.config?.updateCoderPrefs || !archiveSettingsLoaded; + const archiveControlsDisabled = + offline || !api?.config?.updateCoderPrefs || !archiveSettingsLoaded; const queueArchiveBehaviorUpdate = useCallback(() => { if (!api?.config?.updateCoderPrefs || !archiveSettingsLoaded) { @@ -440,7 +445,7 @@ export function GeneralSection() { ); const handleChatTranscriptFullWidthChange = (checked: boolean) => { - saveConfigOptimistically(api ?? null, { chatTranscriptFullWidth: checked }, (client) => { + saveConfigOptimistically(liveApi, { chatTranscriptFullWidth: checked }, (client) => { // Chained so a quick toggle back cannot land before the toggle it reverses. const update = chatTranscriptFullWidthUpdateChainRef.current.then(() => client.config.updateChatTranscriptFullWidth({ enabled: checked }) @@ -1131,6 +1136,7 @@ export function GeneralSection() { @@ -1255,6 +1261,7 @@ export function GeneralSection() { @@ -1273,6 +1280,7 @@ export function GeneralSection() { handleSshHostChange(e.target.value) } placeholder={window.location.hostname} + disabled={offline} className="border-border-medium bg-background-secondary h-9 w-40" /> @@ -1297,7 +1305,7 @@ export function GeneralSection() { } onBlur={handleCloneDirBlur} placeholder="~/.xum/projects" - disabled={!cloneDirLoaded} + disabled={!cloneDirLoaded || offline} className="border-border-medium bg-background-secondary h-9 w-80" /> diff --git a/src/browser/features/Settings/Sections/HeartbeatSection.test.tsx b/src/browser/features/Settings/Sections/HeartbeatSection.test.tsx index 1a338f2d3c0..2461cb8ad9b 100644 --- a/src/browser/features/Settings/Sections/HeartbeatSection.test.tsx +++ b/src/browser/features/Settings/Sections/HeartbeatSection.test.tsx @@ -284,4 +284,42 @@ describe.each([...GAP_MODES])("HeartbeatSection across a reconnect (%s)", (mode) expect(promptField.value).toBe("Typed, not saved"); expect(promptField.disabled).toBe(false); }); + + // A save that the drop cut off never reached the server. After the reconnect the field must + // show the saved value, not the unsaved one as if it were saved. Offline it is disabled. + test("shows the saved prompt after a save the drop cut off", async () => { + const first = createMockAPI({ heartbeatDefaultPrompt: "Saved prompt" }); + first.api.config.updateHeartbeatDefaultPrompt = mock(() => new Promise(() => undefined)); + mockApi = first.api; + const harness = createConnectionHarness(mode, () => mockApi); + const view = render( + + + + + + + + ); + const promptField = (await waitFor(() => + view.getByLabelText("Default heartbeat prompt") + )) as HTMLTextAreaElement; + await waitFor(() => expect(promptField.value).toBe("Saved prompt")); + + typeIntoTextarea(promptField, "Cut off"); + fireEvent.blur(promptField); + await waitFor(() => expect(first.api.config.updateHeartbeatDefaultPrompt).toHaveBeenCalled()); + + mockApi = createMockAPI({ heartbeatDefaultPrompt: "Saved prompt" }).api; + await harness.drop(); + expect(promptField.disabled).toBe(true); + // A real reconnect takes at least a socket round trip: the cut-off save rejects before it. + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + harness.reconnect(); + + await waitFor(() => expect(promptField.value).toBe("Saved prompt")); + expect(promptField.disabled).toBe(false); + }); }); diff --git a/src/browser/features/Settings/Sections/HeartbeatSection.tsx b/src/browser/features/Settings/Sections/HeartbeatSection.tsx index 0fc54ad2e2f..e918968358d 100644 --- a/src/browser/features/Settings/Sections/HeartbeatSection.tsx +++ b/src/browser/features/Settings/Sections/HeartbeatSection.tsx @@ -1,7 +1,8 @@ import React, { useCallback, useEffect, useRef, useState } from "react"; import { Input } from "@/browser/components/Input/Input"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; +import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { clampIntervalMinutes, formatIntervalMinutes, @@ -33,8 +34,35 @@ function isUneditedLoad( return nonce === nonceRef.current && !editedSinceLoadRef.current; } +/** + * Offline (the field was disabled while focused) a blur does not save: drop the edit mark, so the + * reload after the reconnect shows the saved value again instead of an unsaved one. + */ +function forgetOfflineEdit(offline: boolean, editedSinceLoadRef: { current: boolean }): void { + if (offline) editedSinceLoadRef.current = false; +} + +/** + * After a save that the lost connection cut off, drop the edit mark (unless the user typed again + * since `nonce`). The reload after the reconnect then shows the saved value again: an unsaved + * value kept on screen would look saved. + */ +function forgetLostEdit( + error: unknown, + nonce: number, + nonceRef: { current: number }, + editedSinceLoadRef: { current: boolean } +): void { + if (isConnectionLostError(error) && nonce === nonceRef.current) { + editedSinceLoadRef.current = false; + } +} + export function HeartbeatSection() { const { api } = useAPI(); + // Offline the fields are disabled: a save could not reach the server. + const liveApi = useLiveAPI(); + const offline = liveApi == null; const [heartbeatDefaultPrompt, setHeartbeatDefaultPrompt] = useState(""); const [heartbeatDefaultPromptLoaded, setHeartbeatDefaultPromptLoaded] = useState(false); const [heartbeatDefaultPromptLoadedOk, setHeartbeatDefaultPromptLoadedOk] = useState(false); @@ -108,7 +136,8 @@ export function HeartbeatSection() { }, [api]); const handleHeartbeatDefaultPromptBlur = useCallback(() => { - if (!heartbeatDefaultPromptLoaded || !api?.config?.updateHeartbeatDefaultPrompt) { + if (!heartbeatDefaultPromptLoaded || !liveApi?.config?.updateHeartbeatDefaultPrompt) { + forgetOfflineEdit(offline, heartbeatDefaultPromptEditedSinceLoadRef); return; } @@ -116,6 +145,7 @@ export function HeartbeatSection() { if (!heartbeatDefaultPromptLoadedOk && !heartbeatDefaultPromptEditedSinceLoadRef.current) { return; } + const nonce = heartbeatDefaultPromptLoadNonceRef.current; setHeartbeatDefaultPrompt(trimmedDefaultPrompt); @@ -124,7 +154,7 @@ export function HeartbeatSection() { // Best-effort only. }) .then(() => - api.config.updateHeartbeatDefaultPrompt({ + liveApi.config.updateHeartbeatDefaultPrompt({ defaultPrompt: trimmedDefaultPrompt || null, }) ) @@ -132,19 +162,40 @@ export function HeartbeatSection() { setHeartbeatDefaultPromptLoadedOk(true); heartbeatDefaultPromptEditedSinceLoadRef.current = false; }) - .catch(() => { + .catch((error: unknown) => { // Best-effort persistence. + forgetLostEdit( + error, + nonce, + heartbeatDefaultPromptLoadNonceRef, + heartbeatDefaultPromptEditedSinceLoadRef + ); }); - }, [api, heartbeatDefaultPrompt, heartbeatDefaultPromptLoaded, heartbeatDefaultPromptLoadedOk]); + }, [ + liveApi, + offline, + heartbeatDefaultPrompt, + heartbeatDefaultPromptLoaded, + heartbeatDefaultPromptLoadedOk, + ]); const handleHeartbeatDefaultIntervalBlur = useCallback(() => { - if (!heartbeatDefaultIntervalLoaded || !api?.config?.updateHeartbeatDefaultIntervalMs) { + if (!heartbeatDefaultIntervalLoaded || !liveApi?.config?.updateHeartbeatDefaultIntervalMs) { + forgetOfflineEdit(offline, heartbeatDefaultIntervalEditedSinceLoadRef); return; } if (!heartbeatDefaultIntervalLoadedOk && !heartbeatDefaultIntervalEditedSinceLoadRef.current) { return; } + const nonce = heartbeatDefaultIntervalLoadNonceRef.current; + const onSaveError = (error: unknown) => + forgetLostEdit( + error, + nonce, + heartbeatDefaultIntervalLoadNonceRef, + heartbeatDefaultIntervalEditedSinceLoadRef + ); const parsedMinutes = parseIntervalMinutes(draftIntervalMinutes); @@ -157,14 +208,13 @@ export function HeartbeatSection() { .catch(() => { /* Best-effort. */ }) - .then(() => api.config.updateHeartbeatDefaultIntervalMs({ intervalMs: null })) + .then(() => liveApi.config.updateHeartbeatDefaultIntervalMs({ intervalMs: null })) .then(() => { setHeartbeatDefaultIntervalLoadedOk(true); heartbeatDefaultIntervalEditedSinceLoadRef.current = false; }) - .catch(() => { - /* Best-effort. */ - }); + // Best-effort. + .catch(onSaveError); return; } @@ -179,7 +229,7 @@ export function HeartbeatSection() { // Best-effort only. }) .then(() => - api.config.updateHeartbeatDefaultIntervalMs({ + liveApi.config.updateHeartbeatDefaultIntervalMs({ intervalMs: intervalMinutesToMs(clampedMinutes), }) ) @@ -187,10 +237,15 @@ export function HeartbeatSection() { setHeartbeatDefaultIntervalLoadedOk(true); heartbeatDefaultIntervalEditedSinceLoadRef.current = false; }) - .catch(() => { - // Best-effort persistence. - }); - }, [api, draftIntervalMinutes, heartbeatDefaultIntervalLoaded, heartbeatDefaultIntervalLoadedOk]); + // Best-effort persistence. + .catch(onSaveError); + }, [ + liveApi, + offline, + draftIntervalMinutes, + heartbeatDefaultIntervalLoaded, + heartbeatDefaultIntervalLoadedOk, + ]); return (
@@ -237,6 +292,7 @@ export function HeartbeatSection() { setDraftIntervalMinutes(event.target.value); }} onBlur={handleHeartbeatDefaultIntervalBlur} + disabled={offline} className="border-border-medium bg-background-secondary h-9 w-24 text-right" aria-label="Default heartbeat threshold in minutes" /> @@ -263,6 +319,7 @@ export function HeartbeatSection() { setHeartbeatDefaultPrompt(event.target.value); }} onBlur={handleHeartbeatDefaultPromptBlur} + disabled={offline} className="border-border-medium bg-background-secondary text-foreground focus:border-accent focus:ring-accent mt-3 min-h-[120px] w-full resize-y rounded-md border p-3 text-sm leading-relaxed focus:ring-1 focus:outline-none disabled:cursor-not-allowed disabled:opacity-50" placeholder={HEARTBEAT_DEFAULT_MESSAGE_BODY} aria-label="Default heartbeat prompt" diff --git a/src/browser/features/Settings/Sections/SecretsSection.test.tsx b/src/browser/features/Settings/Sections/SecretsSection.test.tsx index b4ff412f9a3..6d66e3c027a 100644 --- a/src/browser/features/Settings/Sections/SecretsSection.test.tsx +++ b/src/browser/features/Settings/Sections/SecretsSection.test.tsx @@ -110,6 +110,23 @@ describe.each([...GAP_MODES])("SecretsSection across a reconnect (%s)", (mode) = expect(view.queryByDisplayValue("EDITED_KEY")).not.toBeNull(); expect(view.queryByDisplayValue("SERVER_KEY")).toBeNull(); }); + + // Offline the edited rows stay, but a Save could not reach the server: it must be disabled. + test("disables Save for an edited row while offline", async () => { + const view = renderSecrets(); + const keyField = await waitFor(() => view.getByDisplayValue("OLD_KEY")); + typeInto(keyField, "EDITED_KEY"); + const save = view.getByRole("button", { name: "Save" }) as HTMLButtonElement; + await waitFor(() => expect(save.disabled).toBe(false)); + + await harness.drop(); + await settle(); + expect(view.queryByDisplayValue("EDITED_KEY")).not.toBeNull(); + expect(save.disabled).toBe(true); + + harness.reconnect(); + await waitFor(() => expect(save.disabled).toBe(false)); + }); }); // The Secrets and MCP sections offer the global secret keys as {secret:"KEY"} references. diff --git a/src/browser/features/Settings/Sections/SecretsSection.tsx b/src/browser/features/Settings/Sections/SecretsSection.tsx index 39835e4a590..ebc7f2d6df5 100644 --- a/src/browser/features/Settings/Sections/SecretsSection.tsx +++ b/src/browser/features/Settings/Sections/SecretsSection.tsx @@ -2,7 +2,7 @@ import React, { useCallback, useEffect, useRef, useState } from "react"; import { Loader2, Trash2 } from "lucide-react"; import type { ProjectConfig } from "@/common/types/project"; import type { Secret } from "@/common/types/secrets"; -import { useAPI, type APIClient } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI, type APIClient } from "@/browser/contexts/API"; import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import { useProjectContext } from "@/browser/contexts/ProjectContext"; import { useSettings } from "@/browser/contexts/SettingsContext"; @@ -151,6 +151,8 @@ export function useGlobalSecretKeys(api: APIClient | null) { export const SecretsSection: React.FC = () => { const { api } = useAPI(); + // Offline, edited rows stay on screen, but Save is disabled: it could not reach the server. + const offline = useLiveAPI() == null; const { userProjects } = useProjectContext(); const { secretsProjectPath, setSecretsProjectPath } = useSettings(); const projectList = getTopLevelProjectEntries(userProjects).map(([projectPath]) => projectPath); @@ -794,7 +796,11 @@ export const SecretsSection: React.FC = () => { -
diff --git a/src/browser/features/Settings/Sections/ServerAccessSection.test.tsx b/src/browser/features/Settings/Sections/ServerAccessSection.test.tsx index c51e1741748..96268fd2626 100644 --- a/src/browser/features/Settings/Sections/ServerAccessSection.test.tsx +++ b/src/browser/features/Settings/Sections/ServerAccessSection.test.tsx @@ -65,6 +65,26 @@ describe.each([...GAP_MODES])("ServerAccessSection across a reconnect (%s)", (mo await view.findByText("Phone"); await waitFor(() => expect(view.queryByText("Laptop")).toBeNull()); }); + + // A revoke offline cannot reach the server: the kept rows must not offer it. + test("disables the session actions while offline", async () => { + const view = render( + + + + ); + const revoke = (await view.findByRole("button", { name: "Revoke" })) as HTMLButtonElement; + const refresh = view.getByRole("button", { name: "Refresh" }) as HTMLButtonElement; + expect(revoke.disabled).toBe(false); + + await harness.drop(); + expect(view.queryByText("Laptop")).not.toBeNull(); + expect(revoke.disabled).toBe(true); + expect(refresh.disabled).toBe(true); + + harness.reconnect(); + await waitFor(() => expect(revoke.disabled).toBe(false)); + }); }); // A server cancel on a live socket is a plain AbortError. It is a real failure: no reconnect diff --git a/src/browser/features/Settings/Sections/ServerAccessSection.tsx b/src/browser/features/Settings/Sections/ServerAccessSection.tsx index d44f7944e3b..6e148e1181e 100644 --- a/src/browser/features/Settings/Sections/ServerAccessSection.tsx +++ b/src/browser/features/Settings/Sections/ServerAccessSection.tsx @@ -1,6 +1,6 @@ import { useCallback, useEffect, useState } from "react"; import { Button } from "@/browser/components/Button/Button"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; import { isConnectionLostError } from "@/browser/contexts/connectionLink"; import type { ServerAuthSession } from "@/common/orpc/types"; import { getErrorMessage } from "@/common/utils/errors"; @@ -36,7 +36,7 @@ function formatRelativeTime(timestampMs: number): string { function SessionRow(props: { session: ServerAuthSession; - loading: boolean; + disabled: boolean; revoking: boolean; onRevoke: () => void; }) { @@ -58,7 +58,7 @@ function SessionRow(props: { variant={session.isCurrent ? "destructive" : "ghost"} size="sm" onClick={props.onRevoke} - disabled={props.loading || props.revoking} + disabled={props.disabled || props.revoking} > {props.revoking ? "Revoking..." : session.isCurrent ? "Log out" : "Revoke"} @@ -68,6 +68,9 @@ function SessionRow(props: { export function ServerAccessSection() { const { api, retry } = useAPI(); + // Offline, the kept sessions stay on screen, but every action is disabled: it could not reach + // the server, and a click must not look like it did something. + const offline = useLiveAPI() == null; const [sessions, setSessions] = useState([]); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); @@ -172,7 +175,7 @@ export function ServerAccessSection() { variant="secondary" size="sm" onClick={() => void refreshSessions()} - disabled={loading} + disabled={loading || offline} > Refresh @@ -182,7 +185,7 @@ export function ServerAccessSection() { onClick={() => { void handleRevokeOthers(); }} - disabled={loading || revokingOthers || !canRevokeOtherSessions} + disabled={loading || offline || revokingOthers || !canRevokeOtherSessions} > {revokingOthers ? "Revoking..." : "Revoke other sessions"} @@ -201,7 +204,7 @@ export function ServerAccessSection() { { void handleRevokeSession(session); diff --git a/src/browser/features/Settings/Sections/TasksSection.tsx b/src/browser/features/Settings/Sections/TasksSection.tsx index 7fad7c84c87..d8cc61a33ae 100644 --- a/src/browser/features/Settings/Sections/TasksSection.tsx +++ b/src/browser/features/Settings/Sections/TasksSection.tsx @@ -1,5 +1,5 @@ import React, { useEffect, useMemo, useRef, useState } from "react"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; import { useWorkspaceContext } from "@/browser/contexts/WorkspaceContext"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/browser/components/Tooltip/Tooltip"; import { Input } from "@/browser/components/Input/Input"; @@ -53,6 +53,9 @@ import { WORKSPACE_DEFAULTS } from "@/constants/workspaceDefaults"; import { FALLBACK_AGENTS, deriveTasksSectionAgentGroups } from "./TasksSection.agents"; const INHERIT = "__inherit__"; +/** Shown while an autosave waits for the connection. */ +const OFFLINE_SAVE_MESSAGE = + "Not connected to the Xum server. Your changes save after it reconnects."; // Agents whose requests run outside the send path (raw streamText: Dream in // memoryConsolidation, Name Workspace in workspaceTitleGenerator) and never @@ -390,6 +393,13 @@ function AiDefaultsControls(props: AiDefaultsControlsProps) { export function TasksSection() { const { api } = useAPI(); + // The autosave sends only through a live client. Offline it keeps the change pending and says + // so: the reload after the reconnect keeps a pending change, and the autosave sends it then. + const liveApi = useLiveAPI(); + const liveApiRef = useRef(liveApi); + useEffect(() => { + liveApiRef.current = liveApi; + }, [liveApi]); const { selectedWorkspace } = useWorkspaceContext(); const selectedWorkspaceRef = useRef(selectedWorkspace); @@ -522,7 +532,6 @@ export function TasksSection() { }, [api, selectedWorkspace?.projectPath, selectedWorkspace?.workspaceId]); useEffect(() => { - if (!api) return; if (!loaded) return; if (loadFailed) return; @@ -544,6 +553,14 @@ export function TasksSection() { return; } + if (!liveApi) { + // Offline: the change stays pending (the reload after the reconnect keeps it) and the + // autosave sends it once this effect runs with the new client. + clearSaveTimer(saveTimerRef); + setSaveError(OFFLINE_SAVE_MESSAGE); + return; + } + // Readers of the shared store see the edit before the debounced save lands. getAppConfigStore().updateOptimistically({ agentAiDefaults }); @@ -552,14 +569,17 @@ export function TasksSection() { saveTimerRef.current = setTimeout(() => { const flush = () => { if (savingRef.current) return; - if (!api) return; + // The live client now, not this effect's: a retry after a lost connection must not take + // the pending change while offline (it would fail again and lose it). + const client = liveApiRef.current; + if (!client) return; const payload = pendingSaveRef.current; if (!payload) return; pendingSaveRef.current = null; savingRef.current = true; - void api.config + void client.config .saveConfig(payload) .then(() => { const previousAgentDefaults = lastSyncedAgentAiDefaultsRef.current; @@ -585,8 +605,8 @@ export function TasksSection() { // while the user tweaks values. setAgentsLoadFailed(false); void Promise.all([ - api.agents.list({ projectPath, workspaceId }), - api.agents.list({ projectPath, workspaceId, includeDisabled: true }), + client.agents.list({ projectPath, workspaceId }), + client.agents.list({ projectPath, workspaceId, includeDisabled: true }), ]) .then(([enabled, all]) => { setAgents(all); @@ -617,7 +637,7 @@ export function TasksSection() { return () => { clearSaveTimer(saveTimerRef); }; - }, [api, agentAiDefaults, loaded, loadFailed, taskSettings]); + }, [liveApi, agentAiDefaults, loaded, loadFailed, taskSettings]); // Flush any pending debounced save on unmount so changes aren't lost. useEffect(() => { diff --git a/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx b/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx index af2dcd7a9d0..5c4eff0b09d 100644 --- a/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx +++ b/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx @@ -1089,4 +1089,39 @@ describe.each([...GAP_MODES])("TasksSection across a reconnect (%s)", (mode) => await waitFor(() => expect(saveConfig).toHaveBeenCalled(), { timeout: 2_000 }); expect(saveConfig.mock.calls.at(-1)?.[0].taskSettings.maxParallelAgentTasks).toBe(7); }); + + // An offline change cannot be saved yet. It must say so, stay pending, and be saved by the + // next client, not dropped while it looks saved. + test("reports a change made offline and saves it after the reconnect", async () => { + const firstSave = connect(3, Promise.resolve(FALLBACK_AGENTS)); + const harness = createConnectionHarness(mode, () => apiMock ?? {}); + const view = render( + + + + ); + await waitFor(() => expect(maxParallelInput(view).value).toBe("3")); + + await harness.drop(); + const input = maxParallelInput(view); + const reactPropsKey = Object.keys(input).find((key) => key.startsWith("__reactProps")); + const props = ( + reactPropsKey ? (input as unknown as Record)[reactPropsKey] : null + ) as { onChange?: (event: { target: { value: string } }) => void } | null; + if (!props?.onChange) throw new Error("Expected the input to expose its onChange handler"); + const change = props.onChange; + act(() => change({ target: { value: "7" } })); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 500)); + }); + expect(view.queryByText(/Not connected to the Xum server/)).not.toBeNull(); + + const saveConfig = connect(5, Promise.resolve(FALLBACK_AGENTS)); + harness.reconnect(); + await waitFor(() => expect(saveConfig).toHaveBeenCalled(), { timeout: 2_000 }); + expect(saveConfig.mock.calls.at(-1)?.[0].taskSettings.maxParallelAgentTasks).toBe(7); + expect(maxParallelInput(view).value).toBe("7"); + await waitFor(() => expect(view.queryByText(/Not connected to the Xum server/)).toBeNull()); + expect(firstSave).not.toHaveBeenCalled(); + }); }); From 71ee55a8e332ffab63450dc57ae06c18c9d15e1f Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 16:23:34 +0000 Subject: [PATCH 11/14] fix: reconnect reloads keep fresh edits, presets and the loader honest Four review findings on the kept-data Settings and context changes: - Secrets: a reload reads the form's dirtiness when it answers, not when it started, so an edit typed while the reload is in flight survives it. - Coder presets: another template clears the shown presets even offline, so the old template's presets are never selectable. - Tasks: every control is disabled while offline. An offline edit lived only in the section's state and was lost when the user left the section before the reconnect. - Workspaces: a new client shows the loader again while no snapshot has loaded, so a disconnect before the first snapshot does not leave the app with an empty metadata map. --- .../contexts/WorkspaceContext.test.tsx | 38 +++++++++++++++++++ src/browser/contexts/WorkspaceContext.tsx | 10 ++++- .../Settings/Sections/SecretsSection.test.tsx | 19 ++++++++++ .../Settings/Sections/SecretsSection.tsx | 9 ++--- .../Settings/Sections/TasksSection.tsx | 6 ++- .../Sections/TasksSection.ui.test.tsx | 36 +++++++----------- src/browser/hooks/useCoderWorkspace.ts | 15 +++++--- .../hooks/useCoderWorkspaceReconnect.test.tsx | 12 ++++++ 8 files changed, 109 insertions(+), 36 deletions(-) diff --git a/src/browser/contexts/WorkspaceContext.test.tsx b/src/browser/contexts/WorkspaceContext.test.tsx index db8f22aa341..95c6a4ed175 100644 --- a/src/browser/contexts/WorkspaceContext.test.tsx +++ b/src/browser/contexts/WorkspaceContext.test.tsx @@ -1066,6 +1066,44 @@ describe("WorkspaceContext", () => { }); }); + // Only a loaded snapshot makes later reloads non-blocking. Without one the app must keep its + // loader for the new client, not leave it with an empty metadata map. + describe.each([...GAP_MODES])("reconnect before a first snapshot (%s)", (mode) => { + test("shows loading again for the new client", async () => { + const { workspace: workspaceApi } = createMockAPI(); + workspaceApi.onMetadata.mockImplementation(() => + Promise.resolve( + // eslint-disable-next-line require-yield + (async function* () { + await Promise.resolve(); + })() as unknown as Awaited> + ) + ); + const contexts = await setupWithConnection(mode); + await waitFor(() => expect(contexts.workspace().loading).toBe(false)); + expect(contexts.workspace().loaded).toBe(false); + + // The next server never sends its snapshot. + currentClientMock = { + ...currentClientMock, + workspace: { + ...currentClientMock.workspace, + onMetadata: () => + Promise.resolve( + // eslint-disable-next-line require-yield + (async function* () { + await new Promise(() => undefined); + })() + ), + }, + }; + await contexts.harness.drop(); + contexts.harness.reconnect(); + await waitFor(() => expect(contexts.workspace().loading).toBe(true)); + expect(contexts.workspace().loaded).toBe(false); + }); + }); + describe("archiveWorkspace", () => { test("succeeds even when persisted layout is invalid JSON shape", async () => { const workspaceId = "ws-archive-invalid-layout"; diff --git a/src/browser/contexts/WorkspaceContext.tsx b/src/browser/contexts/WorkspaceContext.tsx index 11b8be03b81..4faca310460 100644 --- a/src/browser/contexts/WorkspaceContext.tsx +++ b/src/browser/contexts/WorkspaceContext.tsx @@ -483,6 +483,9 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { const routedDraftIdRef = useRef(null); const [loading, setLoading] = useState(true); const [loaded, setLoaded] = useState(false); + // For the metadata effect: a new client shows the loader again until a first snapshot loads. + const loadedRef = useRef(loaded); + loadedRef.current = loaded; const [loadError, setLoadError] = useState(null); const [workspaceDraftPromotionsByProject, setWorkspaceDraftPromotionsByProject] = @@ -902,13 +905,16 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { // The metadata stream owns the initial load: its first event is a snapshot built after the // server attached the listener, so no change can fall between the load and the updates (#5189). useEffect(() => { - // `loading` starts true and stays false after the first load: a new client (reconnect) - // reloads behind the shown metadata, so neither `loading` nor `loaded` resets here. + // After a first snapshot, a new client (reconnect) reloads behind the shown metadata, so + // neither `loading` nor `loaded` resets here. if (!api) { // api not available yet - effect will run again when api connects setLoadError("API not connected"); return; } + // No snapshot loaded yet (a failure or a lost connection ended the last load): the new client + // shows the loader again, or the app would leave it with an empty metadata map. + if (!loadedRef.current) setLoading(true); const controller = new AbortController(); const { signal } = controller; let snapshotApplied = false; diff --git a/src/browser/features/Settings/Sections/SecretsSection.test.tsx b/src/browser/features/Settings/Sections/SecretsSection.test.tsx index 6d66e3c027a..8ed04f8de70 100644 --- a/src/browser/features/Settings/Sections/SecretsSection.test.tsx +++ b/src/browser/features/Settings/Sections/SecretsSection.test.tsx @@ -111,6 +111,25 @@ describe.each([...GAP_MODES])("SecretsSection across a reconnect (%s)", (mode) = expect(view.queryByDisplayValue("SERVER_KEY")).toBeNull(); }); + // The reload starts on a clean form. An edit made while it is in flight must survive its answer. + test("keeps a row the user edited while the reload was pending", async () => { + const view = renderSecrets(); + await waitFor(() => expect(view.queryByDisplayValue("OLD_KEY")).not.toBeNull()); + + let answer: ((secrets: Secret[]) => void) | null = null; + serverSecrets = new Promise((resolve) => (answer = resolve)); + await harness.drop(); + harness.reconnect(); + await settle(); + typeInto(view.getByDisplayValue("OLD_KEY"), "EDITED_KEY"); + await waitFor(() => expect(view.queryByDisplayValue("EDITED_KEY")).not.toBeNull()); + + act(() => answer?.([{ key: "SERVER_KEY", value: "server" }])); + await settle(); + expect(view.queryByDisplayValue("EDITED_KEY")).not.toBeNull(); + expect(view.queryByDisplayValue("SERVER_KEY")).toBeNull(); + }); + // Offline the edited rows stay, but a Save could not reach the server: it must be disabled. test("disables Save for an edited row while offline", async () => { const view = renderSecrets(); diff --git a/src/browser/features/Settings/Sections/SecretsSection.tsx b/src/browser/features/Settings/Sections/SecretsSection.tsx index ebc7f2d6df5..48e9c56f006 100644 --- a/src/browser/features/Settings/Sections/SecretsSection.tsx +++ b/src/browser/features/Settings/Sections/SecretsSection.tsx @@ -249,17 +249,15 @@ export const SecretsSection: React.FC = () => { // Only a new scope or project clears the list and shows "Loading": a new client (reconnect) // reloads the same secrets behind the shown rows, and keeps rows the user edited. const secretsKey = JSON.stringify([scope, currentProjectPath]); - if (shownSecretsKeyRef.current !== secretsKey) { + const reloadsShownSecrets = shownSecretsKeyRef.current === secretsKey; + if (!reloadsShownSecrets) { shownSecretsKeyRef.current = null; setLoadedSecrets([]); setSecrets([]); setInjectedGlobalSecretKeys([]); setVisibleSecrets(new Set()); setLoading(true); - // The cleared rows hold no edit (the next render agrees). - isDirtyRef.current = false; } - const keepEdits = isDirtyRef.current; setError(null); if (!api) { @@ -271,7 +269,8 @@ export const SecretsSection: React.FC = () => { const applyLoadedSecrets = (nextSecrets: Secret[]) => { shownSecretsKeyRef.current = secretsKey; setLoadedSecrets(nextSecrets); - if (keepEdits) return; + // Read dirtiness at apply time: the user can edit while the reload is in flight. + if (reloadsShownSecrets && isDirtyRef.current) return; setSecrets(nextSecrets); setVisibleSecrets(new Set()); lastLiteralValuesRef.current = new Map(); diff --git a/src/browser/features/Settings/Sections/TasksSection.tsx b/src/browser/features/Settings/Sections/TasksSection.tsx index d8cc61a33ae..5dc04a5c42e 100644 --- a/src/browser/features/Settings/Sections/TasksSection.tsx +++ b/src/browser/features/Settings/Sections/TasksSection.tsx @@ -1130,7 +1130,9 @@ export function TasksSection() { }; return ( -
+ // Offline every control is disabled: an edit kept only in this section's state is lost when + // the user leaves the section before the reconnect. +

Task Settings

@@ -1265,6 +1267,6 @@ export function TasksSection() {
{unknownAgentIds.map(renderUnknownAgentDefaults)}
) : null} -
+
); } diff --git a/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx b/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx index 5c4eff0b09d..e69e3ea42d0 100644 --- a/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx +++ b/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx @@ -1090,10 +1090,10 @@ describe.each([...GAP_MODES])("TasksSection across a reconnect (%s)", (mode) => expect(saveConfig.mock.calls.at(-1)?.[0].taskSettings.maxParallelAgentTasks).toBe(7); }); - // An offline change cannot be saved yet. It must say so, stay pending, and be saved by the - // next client, not dropped while it looks saved. - test("reports a change made offline and saves it after the reconnect", async () => { - const firstSave = connect(3, Promise.resolve(FALLBACK_AGENTS)); + // Offline an edit would live only in this section's state, and leaving the section before the + // reconnect would lose it: the controls must be disabled until a live client is back. + test("disables the controls while offline", async () => { + connect(3, Promise.resolve(FALLBACK_AGENTS)); const harness = createConnectionHarness(mode, () => apiMock ?? {}); const view = render( @@ -1101,27 +1101,19 @@ describe.each([...GAP_MODES])("TasksSection across a reconnect (%s)", (mode) => ); await waitFor(() => expect(maxParallelInput(view).value).toBe("3")); + // happy-dom does not apply a disabled
to `:disabled`: read the fieldset (browsers + // disable every control inside it). + const isDisabled = (input: HTMLInputElement) => + input.disabled || input.closest("fieldset")?.disabled === true; + expect(isDisabled(maxParallelInput(view))).toBe(false); await harness.drop(); - const input = maxParallelInput(view); - const reactPropsKey = Object.keys(input).find((key) => key.startsWith("__reactProps")); - const props = ( - reactPropsKey ? (input as unknown as Record)[reactPropsKey] : null - ) as { onChange?: (event: { target: { value: string } }) => void } | null; - if (!props?.onChange) throw new Error("Expected the input to expose its onChange handler"); - const change = props.onChange; - act(() => change({ target: { value: "7" } })); - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 500)); - }); - expect(view.queryByText(/Not connected to the Xum server/)).not.toBeNull(); + await waitFor(() => expect(isDisabled(maxParallelInput(view))).toBe(true)); + // The kept value stays on screen. + expect(maxParallelInput(view).value).toBe("3"); - const saveConfig = connect(5, Promise.resolve(FALLBACK_AGENTS)); + connect(3, Promise.resolve(FALLBACK_AGENTS)); harness.reconnect(); - await waitFor(() => expect(saveConfig).toHaveBeenCalled(), { timeout: 2_000 }); - expect(saveConfig.mock.calls.at(-1)?.[0].taskSettings.maxParallelAgentTasks).toBe(7); - expect(maxParallelInput(view).value).toBe("7"); - await waitFor(() => expect(view.queryByText(/Not connected to the Xum server/)).toBeNull()); - expect(firstSave).not.toHaveBeenCalled(); + await waitFor(() => expect(isDisabled(maxParallelInput(view))).toBe(false)); }); }); diff --git a/src/browser/hooks/useCoderWorkspace.ts b/src/browser/hooks/useCoderWorkspace.ts index 96c800ae528..05d5e5b0970 100644 --- a/src/browser/hooks/useCoderWorkspace.ts +++ b/src/browser/hooks/useCoderWorkspace.ts @@ -311,16 +311,21 @@ export function useCoderWorkspace({ setLoadingPresets(false); return; } - // Offline, keep the shown presets: the reconnect reloads them. - if (!api) return; - - let mounted = true; // Capture template/org at request time to detect stale responses const templateAtRequest = coderConfig.template; const orgAtRequest = coderConfig.templateOrg; // Only another template shows "loading": a reload (reconnect) keeps the shown presets. const presetsKey = JSON.stringify([templateAtRequest, orgAtRequest]); - if (shownPresetsKeyRef.current !== presetsKey) setLoadingPresets(true); + if (shownPresetsKeyRef.current !== presetsKey) { + // The shown presets belong to another template: never offer them, offline either. + shownPresetsKeyRef.current = null; + setPresets([]); + setLoadingPresets(api != null); + } + // Offline, keep the shown presets of this template: the reconnect reloads them. + if (!api) return; + + let mounted = true; setPresetsError(null); api.coder diff --git a/src/browser/hooks/useCoderWorkspaceReconnect.test.tsx b/src/browser/hooks/useCoderWorkspaceReconnect.test.tsx index 7bab82ca173..c0820046063 100644 --- a/src/browser/hooks/useCoderWorkspaceReconnect.test.tsx +++ b/src/browser/hooks/useCoderWorkspaceReconnect.test.tsx @@ -142,6 +142,18 @@ describe.each([...GAP_MODES])("useCoderWorkspace across a reconnect (%s)", (mode expect(view.result.current.loadingPresets).toBe(true); }); + // Offline no reload can replace them: the old template's presets must not stay selectable. + test("another template while offline clears the shown presets", async () => { + const { view } = renderCoder(CONFIG); + await waitFor(() => expect(view.result.current.presets.length).toBe(2)); + + await harness.drop(); + view.rerender({ config: { ...CONFIG, template: "t2" } }); + await settle(); + expect(view.result.current.presets).toEqual([]); + expect(view.result.current.loadingPresets).toBe(false); + }); + test("a window focus while offline keeps the Coder info and config", async () => { const { view, onChange } = renderCoder(CONFIG); await waitFor(() => expect(view.result.current.coderInfo?.state).toBe("available")); From 714dd37416505ee0a17db443509ef27d13cb5ed1 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 17:01:09 +0000 Subject: [PATCH 12/14] fix: a reconnect reload holds the send gates and the Tasks controls until it answers Four review findings on the kept-data reloads: - Tasks: the controls stay disabled until a load answers. The autosave does not track an edit made before the reload answers, so the answer would have replaced it. - Coder presets: every online load sets `loadingPresets`, a reconnect reload too. ChatInput blocks a Coder send on it, so a send cannot use a preset the server removed while offline. The shown presets stay. - Creation branches: every load clears `branchesLoaded`, the send barrier for a trunk branch (#6033). The shown branches stay. - MCP prompts: a new client lists the prompts again, even inside the 30 s cache window, because they can change while offline. --- .../features/ChatInput/useComposerSuggestions.ts | 7 +++++++ .../useComposerSuggestionsReconnect.test.tsx | 12 ++++++++++++ .../features/ChatInput/useCreationWorkspace.test.tsx | 8 +++++--- .../features/ChatInput/useCreationWorkspace.ts | 8 +++++--- .../features/Settings/Sections/TasksSection.tsx | 10 ++++++++-- .../Settings/Sections/TasksSection.ui.test.tsx | 11 +++++++++++ src/browser/hooks/useCoderWorkspace.ts | 10 +++++++--- .../hooks/useCoderWorkspaceReconnect.test.tsx | 5 ++++- 8 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/browser/features/ChatInput/useComposerSuggestions.ts b/src/browser/features/ChatInput/useComposerSuggestions.ts index 5283a7ee9aa..cf25a894808 100644 --- a/src/browser/features/ChatInput/useComposerSuggestions.ts +++ b/src/browser/features/ChatInput/useComposerSuggestions.ts @@ -217,6 +217,8 @@ export function useComposerSuggestions(options: UseComposerSuggestionsOptions) { const mcpRequestId = useRef(0); const mcpRequest = useRef | null>(null); const mcpLoadedAt = useRef(0); + // The client the prompts were listed with: a new one (reconnect) lists them again. + const mcpClient = useRef(null); const mcpWorkspace = useRef(null); const mcpAbort = useRef(null); const listId = useId(); @@ -296,6 +298,11 @@ export function useComposerSuggestions(options: UseComposerSuggestionsOptions) { dropMcpRequest(); setMcpPrompts([]); } + if (mcpClient.current !== api) { + // Prompts can change while offline: the shown ones stay, but the next client lists again. + mcpClient.current = api; + mcpLoadedAt.current = 0; + } if (!api) { // Offline: drop the lost client's request, keep the shown prompts for the reconnect. dropMcpRequest(); diff --git a/src/browser/features/ChatInput/useComposerSuggestionsReconnect.test.tsx b/src/browser/features/ChatInput/useComposerSuggestionsReconnect.test.tsx index 70a3a9f3782..d0d5ceb277a 100644 --- a/src/browser/features/ChatInput/useComposerSuggestionsReconnect.test.tsx +++ b/src/browser/features/ChatInput/useComposerSuggestionsReconnect.test.tsx @@ -131,6 +131,18 @@ describe.each([...GAP_MODES])("composer suggestions across a reconnect (%s)", (m expect(hasGreetPrompt(view.result.current.suggestions)).toBe(true); }); + // Prompts listed just before the drop are inside the 30 s cache window. The new client must + // list them again: they can change while offline. + test("MCP prompts listed before a drop are listed again after the reconnect", async () => { + const view = renderSuggestions("ws-a"); + await waitFor(() => expect(hasGreetPrompt(view.result.current.suggestions)).toBe(true)); + + serverPrompts = []; + await harness.drop(); + harness.reconnect(); + await waitFor(() => expect(hasGreetPrompt(view.result.current.suggestions)).toBe(false)); + }); + test("an MCP prompt request lost with the connection is asked again after the reconnect", async () => { // The first server never answers: the request is still in flight when the connection drops. serverPrompts = new Promise(() => undefined); diff --git a/src/browser/features/ChatInput/useCreationWorkspace.test.tsx b/src/browser/features/ChatInput/useCreationWorkspace.test.tsx index 0e113bf5e76..392a209fb14 100644 --- a/src/browser/features/ChatInput/useCreationWorkspace.test.tsx +++ b/src/browser/features/ChatInput/useCreationWorkspace.test.tsx @@ -713,9 +713,10 @@ describe("useCreationWorkspace", () => { }); // A reconnect reloads branches and runtime availability through the new client. The creation - // controls keep the shown values (no "loading" again) until the reload answers. + // controls keep the shown values until the reload answers. `branchesLoaded` is the send barrier + // (#6033), so it is false until the new branch list answers. describe.each([...GAP_MODES])("reload after a reconnect (%s)", (mode) => { - test("keeps the shown branches and runtimes until the reload answers", async () => { + test("keeps the shown branches and runtimes, and blocks the send, until the reload answers", async () => { setupWindow({ listBranches: mock(() => Promise.resolve({ branches: ["main"], recommendedTrunk: "main" })), }); @@ -760,12 +761,13 @@ describe("useCreationWorkspace", () => { await new Promise((resolve) => setTimeout(resolve, 30)); }); - expect(resultRef.current?.branchesLoaded).toBe(true); + expect(resultRef.current?.branchesLoaded).toBe(false); expect(resultRef.current?.branches).toEqual(["main"]); expect(resultRef.current?.runtimeAvailabilityState.status).toBe("loaded"); act(() => answerBranches?.()); await waitFor(() => expect(resultRef.current?.branches).toEqual(["main", "next"])); + expect(resultRef.current?.branchesLoaded).toBe(true); }); }); diff --git a/src/browser/features/ChatInput/useCreationWorkspace.ts b/src/browser/features/ChatInput/useCreationWorkspace.ts index e9dfd3c332a..eed13a70207 100644 --- a/src/browser/features/ChatInput/useCreationWorkspace.ts +++ b/src/browser/features/ChatInput/useCreationWorkspace.ts @@ -354,8 +354,8 @@ export function useCreationWorkspace({ } }, [kind, projectPath, api]); - // The project whose branches and runtimes are shown. Only a new project shows "loading" again: - // a new client (reconnect) reloads the same project behind the shown values. + // The project whose runtimes are shown. Only a new project shows the runtimes as "loading" + // again: a new client (reconnect) reloads the same project behind the shown values. const shownProjectPathRef = useRef(null); // Load branches and runtime availability on mount with mounted guard @@ -366,9 +366,11 @@ export function useCreationWorkspace({ } if (!projectPath.length || !api) return; let mounted = true; + // Every load clears `branchesLoaded`, a reload (reconnect) too: it is the send barrier for a + // trunk branch (#6033), and the branches can change while offline. + setBranchesLoaded(false); if (shownProjectPathRef.current !== projectPath) { shownProjectPathRef.current = projectPath; - setBranchesLoaded(false); setRuntimeAvailabilityState({ status: "loading" }); } // Both requests run at once and settle independently. Send waits for the branch list (#6033), diff --git a/src/browser/features/Settings/Sections/TasksSection.tsx b/src/browser/features/Settings/Sections/TasksSection.tsx index 5dc04a5c42e..9bb03215618 100644 --- a/src/browser/features/Settings/Sections/TasksSection.tsx +++ b/src/browser/features/Settings/Sections/TasksSection.tsx @@ -286,6 +286,11 @@ function areAgentAiDefaultsEqual(a: AgentAiDefaults, b: AgentAiDefaults): boolea return true; } +/** Offline, or before a load answered: an edit then would be lost or replaced. */ +function areControlsLocked(offline: boolean, loaded: boolean): boolean { + return offline || !loaded; +} + function clearSaveTimer(timerRef: { current: ReturnType | null }): void { if (timerRef.current) clearTimeout(timerRef.current); timerRef.current = null; @@ -1131,8 +1136,9 @@ export function TasksSection() { return ( // Offline every control is disabled: an edit kept only in this section's state is lost when - // the user leaves the section before the reconnect. -
+ // the user leaves the section before the reconnect. Until a load answers, too: the autosave + // does not track an edit made then, so the answer would replace it. +

Task Settings

diff --git a/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx b/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx index e69e3ea42d0..fedfd0902ba 100644 --- a/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx +++ b/src/browser/features/Settings/Sections/TasksSection.ui.test.tsx @@ -1112,8 +1112,19 @@ describe.each([...GAP_MODES])("TasksSection across a reconnect (%s)", (mode) => // The kept value stays on screen. expect(maxParallelInput(view).value).toBe("3"); + // Until the reload answers the controls stay disabled: an edit then would be replaced by it. + let answer: (() => void) | null = null; + const answered = new Promise((resolve) => (answer = resolve)); connect(3, Promise.resolve(FALLBACK_AGENTS)); + const config = apiMock?.config.getConfig; + if (!apiMock || !config) throw new Error("connect() sets the client"); + apiMock.config.getConfig = mock(() => answered.then(() => config() as unknown)); harness.reconnect(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + expect(isDisabled(maxParallelInput(view))).toBe(true); + act(() => answer?.()); await waitFor(() => expect(isDisabled(maxParallelInput(view))).toBe(false)); }); }); diff --git a/src/browser/hooks/useCoderWorkspace.ts b/src/browser/hooks/useCoderWorkspace.ts index 05d5e5b0970..f9f28ef550a 100644 --- a/src/browser/hooks/useCoderWorkspace.ts +++ b/src/browser/hooks/useCoderWorkspace.ts @@ -314,18 +314,22 @@ export function useCoderWorkspace({ // Capture template/org at request time to detect stale responses const templateAtRequest = coderConfig.template; const orgAtRequest = coderConfig.templateOrg; - // Only another template shows "loading": a reload (reconnect) keeps the shown presets. const presetsKey = JSON.stringify([templateAtRequest, orgAtRequest]); if (shownPresetsKeyRef.current !== presetsKey) { // The shown presets belong to another template: never offer them, offline either. shownPresetsKeyRef.current = null; setPresets([]); - setLoadingPresets(api != null); } // Offline, keep the shown presets of this template: the reconnect reloads them. - if (!api) return; + if (!api) { + setLoadingPresets(false); + return; + } let mounted = true; + // Every load sets "loading", a reload (reconnect) too: ChatInput blocks a Coder send on it, + // so no send can use a preset the server removed while offline. + setLoadingPresets(true); setPresetsError(null); api.coder diff --git a/src/browser/hooks/useCoderWorkspaceReconnect.test.tsx b/src/browser/hooks/useCoderWorkspaceReconnect.test.tsx index c0820046063..233a92ec935 100644 --- a/src/browser/hooks/useCoderWorkspaceReconnect.test.tsx +++ b/src/browser/hooks/useCoderWorkspaceReconnect.test.tsx @@ -111,7 +111,9 @@ describe.each([...GAP_MODES])("useCoderWorkspace across a reconnect (%s)", (mode await settle(); const shown = view.result.current; - expect(shown.loadingTemplates || shown.loadingPresets || shown.loadingWorkspaces).toBe(false); + expect(shown.loadingTemplates || shown.loadingWorkspaces).toBe(false); + // ChatInput blocks a Coder send on loadingPresets: the reload must hold it. + expect(shown.loadingPresets).toBe(true); expect(shown.templates.map((t) => t.name)).toEqual(["t1"]); expect(shown.presets.map((p) => p.name)).toEqual(["p1", "p2"]); expect(shown.existingWorkspaces.map((w) => w.name)).toEqual(["w1"]); @@ -128,6 +130,7 @@ describe.each([...GAP_MODES])("useCoderWorkspace across a reconnect (%s)", (mode ); expect(view.result.current.templates.map((t) => t.name)).toEqual(["t1", "t2"]); expect(view.result.current.presets.map((p) => p.name)).toEqual(["p1", "p2", "p3"]); + expect(view.result.current.loadingPresets).toBe(false); }); test("another template shows loading presets", async () => { From cce8c997642e485c19d3009065dfd105de72b3c7 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 13 Oct 2026 19:57:21 +0000 Subject: [PATCH 13/14] fix: reconnect reloads replace a removed preset and hold coupled saves Review findings on the kept-data reloads: - Coder presets: a reload that no longer lists the selected preset selects a listed one, so a preset removed while offline is not sent. - General: the two archive selects are disabled until each config read answers. They are written together, so a change before the read would send a stale value for the other one. - Heartbeat: Save waits for the new client's read (`isReloading`): the kept form would overwrite changes made on the server while offline. - Backup: Save waits for a pending settings read, for the same reason. A test also pins that a drop before the first metadata snapshot keeps the startup loader: the ConnectionLostError is not a load failure. --- .../WorkspaceHeartbeatModal.test.tsx | 1 + .../WorkspaceHeartbeatModal.tsx | 18 +++++++- .../contexts/WorkspaceContext.test.tsx | 32 +++++++++++++- .../Settings/Sections/BackupSection.tsx | 10 ++++- .../Sections/BackupSectionReconnect.test.tsx | 43 +++++++++++++++++++ .../Settings/Sections/GeneralSection.test.tsx | 7 +-- .../Settings/Sections/GeneralSection.tsx | 5 ++- src/browser/hooks/useCoderWorkspace.ts | 6 ++- .../hooks/useCoderWorkspaceReconnect.test.tsx | 24 +++++++++++ .../hooks/useWorkspaceHeartbeat.test.tsx | 3 ++ src/browser/hooks/useWorkspaceHeartbeat.ts | 9 ++++ 11 files changed, 147 insertions(+), 11 deletions(-) diff --git a/src/browser/components/WorkspaceHeartbeatModal/WorkspaceHeartbeatModal.test.tsx b/src/browser/components/WorkspaceHeartbeatModal/WorkspaceHeartbeatModal.test.tsx index 359b1e09092..ef30bc5922f 100644 --- a/src/browser/components/WorkspaceHeartbeatModal/WorkspaceHeartbeatModal.test.tsx +++ b/src/browser/components/WorkspaceHeartbeatModal/WorkspaceHeartbeatModal.test.tsx @@ -119,6 +119,7 @@ describe("WorkspaceHeartbeatModal", () => { ? createHeartbeatSettings() : (settingsByWorkspaceId.get(workspaceId) ?? createHeartbeatSettings()), isLoading: hookIsLoading, + isReloading: false, isSaving: hookIsSaving, error: hookError, globalDefaultPrompt: undefined, diff --git a/src/browser/components/WorkspaceHeartbeatModal/WorkspaceHeartbeatModal.tsx b/src/browser/components/WorkspaceHeartbeatModal/WorkspaceHeartbeatModal.tsx index 526acb51849..1b4d0b01a64 100644 --- a/src/browser/components/WorkspaceHeartbeatModal/WorkspaceHeartbeatModal.tsx +++ b/src/browser/components/WorkspaceHeartbeatModal/WorkspaceHeartbeatModal.tsx @@ -131,10 +131,21 @@ function getDraftMessageForSave(value: string): string { return normalizeDraftMessage(value) ?? ""; } +/** A save waits for loads (a reconnect's read too: it would overwrite offline changes). */ +function isSaveBlocked(state: { + isLoading: boolean; + isReloading: boolean; + isSaving: boolean; + hasValidationError: boolean; +}): boolean { + return state.isLoading || state.isReloading || state.isSaving || state.hasValidationError; +} + export function WorkspaceHeartbeatModal(props: WorkspaceHeartbeatModalProps) { const { settings, isLoading, + isReloading, isSaving, error, save, @@ -239,7 +250,12 @@ export function WorkspaceHeartbeatModal(props: WorkspaceHeartbeatModalProps) { const errorMessages = [validationError, error].filter( (message): message is string => message != null ); - const hasBlockingError = isLoading || isSaving || validationError != null; + const hasBlockingError = isSaveBlocked({ + isLoading, + isReloading, + isSaving, + hasValidationError: validationError != null, + }); const handleIntervalBlur = () => { const parsedMinutes = parseIntervalMinutes(draftIntervalMinutes); diff --git a/src/browser/contexts/WorkspaceContext.test.tsx b/src/browser/contexts/WorkspaceContext.test.tsx index 95c6a4ed175..64f3f96e927 100644 --- a/src/browser/contexts/WorkspaceContext.test.tsx +++ b/src/browser/contexts/WorkspaceContext.test.tsx @@ -1102,6 +1102,32 @@ describe("WorkspaceContext", () => { await waitFor(() => expect(contexts.workspace().loading).toBe(true)); expect(contexts.workspace().loaded).toBe(false); }); + + // A stream lost with the connection rejects with ConnectionLostError: no load failure, so + // the startup loader stays for the whole outage, not only once the new client arrives. + test("a drop before the first snapshot keeps loading during the outage", async () => { + const { workspace: workspaceApi } = createMockAPI(); + workspaceApi.onMetadata.mockImplementation(() => + Promise.resolve( + // eslint-disable-next-line require-yield + (async function* () { + await new Promise(() => undefined); + })() as unknown as Awaited> + ) + ); + const contexts = await setupWithConnection(mode, { awaitLoad: false }); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + expect(contexts.workspace().loading).toBe(true); + + await contexts.harness.drop(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 50)); + }); + expect(contexts.workspace().loading).toBe(true); + expect(contexts.workspace().loaded).toBe(false); + }); }); describe("archiveWorkspace", () => { @@ -2498,7 +2524,7 @@ function createDraftServiceClient(service: DraftService): TestApiOverrides currentClientMock); const workspaceRef = { current: null as WorkspaceContext | null }; const projectRef = { current: null as ReturnType | null }; @@ -2519,7 +2545,9 @@ async function setupWithConnection(mode: GapMode) { ); getWorkspaceStoreRaw().setClient(harness.client); - await waitFor(() => expect(workspaceRef.current?.loading).toBe(false)); + if (options?.awaitLoad !== false) { + await waitFor(() => expect(workspaceRef.current?.loading).toBe(false)); + } return { harness, workspace: () => workspaceRef.current!, project: () => projectRef.current! }; } diff --git a/src/browser/features/Settings/Sections/BackupSection.tsx b/src/browser/features/Settings/Sections/BackupSection.tsx index e3039f40ec7..645d51123ea 100644 --- a/src/browser/features/Settings/Sections/BackupSection.tsx +++ b/src/browser/features/Settings/Sections/BackupSection.tsx @@ -335,6 +335,10 @@ export function BackupSection() { const configured = settingsFresh && savedDraft.repoUrl.trim() !== ""; const saving = activeAction === "save"; const busy = activeAction !== null; + // A settings read is pending (a reconnect keeps the form on screen while it reloads). Save + // waits for it: the draft's untouched fields would overwrite changes made on the server. + const [reading, setReading] = useState(false); + const saveBlocked = busy || reading; useEffect(() => { if (!api) { @@ -359,6 +363,7 @@ export function BackupSection() { const refresh = async (options?: { markFresh?: boolean }) => { const version = (refreshGenerationRef.current += 1); setSettingsFresh(false); + setReading(true); try { const settings = await api.backup.getSettings(); @@ -399,6 +404,7 @@ export function BackupSection() { } finally { if (!signal.aborted && version === refreshGenerationRef.current) { setLoading(false); + setReading(false); } } }; @@ -456,7 +462,7 @@ export function BackupSection() { }, [api]); async function handleSave() { - if (!api || activeAction !== null) return; + if (!api || saveBlocked) return; const missingField = getMissingFieldError(draft); if (missingField != null) { setSaveError(missingField); @@ -898,7 +904,7 @@ export function BackupSection() {