diff --git a/src/browser/App.tsx b/src/browser/App.tsx index dfe7808069e..8e652e65192 100644 --- a/src/browser/App.tsx +++ b/src/browser/App.tsx @@ -88,7 +88,7 @@ import type { UpdateChannel } from "@/common/types/project"; import { useTelemetry } from "./hooks/useTelemetry"; import { getRuntimeTypeForTelemetry } from "@/common/telemetry"; import { useStartWorkspaceCreation } from "./hooks/useStartWorkspaceCreation"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; import { requestActiveTurnThinkingLevel } from "@/browser/utils/activeTurnThinking"; import { getAutoRouting, getWorkspaceAiSelection } from "@/browser/utils/workspaceAiSettingsSync"; import { AuthTokenModal } from "@/browser/components/AuthTokenModal/AuthTokenModal"; @@ -288,6 +288,13 @@ function AppInner() { const { layoutPresets, applySlotToWorkspace, saveCurrentWorkspaceToSlot } = useUILayouts(); const { getMinOverride: getMinThinkingOverride } = useMinThinkingLevels(); const { api, status, error, authenticate, retry } = useAPI(); + // Handlers and command sources see null offline, even while useAPI() keeps the last client. + const liveApi = useLiveAPI(); + // Read after a confirmation dialog: the connection can drop while it is open. + const liveApiRef = useRef(liveApi); + useEffect(() => { + liveApiRef.current = liveApi; + }, [liveApi]); const [supportedUpdateChannels, setSupportedUpdateChannels] = useState([]); useEffect(() => { @@ -701,7 +708,8 @@ function AppInner() { const togglePremiumServiceTier = useCallback( async (targetServiceTier: PremiumServiceTier) => { const scopeId = selectedWorkspace?.workspaceId ?? creationScopeId; - if (!api || !scopeId || providersConfig == null || fastModeToggleInFlightRef.current) return; + if (!liveApi || !scopeId || providersConfig == null || fastModeToggleInFlightRef.current) + return; // Creation composers use the same project-scoped model preference as their selector, // so the global shortcut remains available before the first workspace exists. @@ -747,11 +755,11 @@ function AppInner() { if (provider === "openai") { const current = getWorkspaceAiSelection(scopeId).serviceTier ?? providersConfig.openai?.serviceTier; - setWorkspaceServiceTier(api, scopeId, toggleServiceTier(current, targetServiceTier)); + setWorkspaceServiceTier(liveApi, scopeId, toggleServiceTier(current, targetServiceTier)); return; } const patch = await applyFastModeToggle( - api.providers, + liveApi.providers, provider, providersConfig[provider], targetServiceTier @@ -768,7 +776,7 @@ function AppInner() { } }, [ - api, + liveApi, creationScopeId, getModelForWorkspace, getRouteForModel, @@ -813,14 +821,14 @@ function AppInner() { projectPaths.length >= 2, "createMultiProjectWorkspace requires at least two projects" ); - if (!api) { + if (!liveApi) { throw new Error("Not connected to server"); } const [primaryProjectPath] = projectPaths; assert(primaryProjectPath, "createMultiProjectWorkspace requires a primary project"); - const branchResult = await api.projects.listBranches({ projectPath: primaryProjectPath }); + const branchResult = await liveApi.projects.listBranches({ projectPath: primaryProjectPath }); const candidateBranches = branchResult.branches.filter( (branch): branch is string => typeof branch === "string" ); @@ -841,7 +849,7 @@ function AppInner() { // Multi-project container names must stay unique and path-separator-free. const projects = createProjectRefs(projectPaths); - const metadata = await api.workspace.createMultiProject({ + const metadata = await liveApi.workspace.createMultiProject({ projects, branchName: `multi-${Date.now().toString(36)}`, trunkBranch, @@ -866,7 +874,7 @@ function AppInner() { setSelectedWorkspace(toWorkspaceSelection(metadata)); setMultiProjectWorkspaceModalOpen(false); }, - [api, setSelectedWorkspace, setWorkspaceMetadata, telemetry, workspaceStore] + [liveApi, setSelectedWorkspace, setWorkspaceMetadata, telemetry, workspaceStore] ); const archiveMergedWorkspacesInProjectFromPalette = useCallback( @@ -874,7 +882,7 @@ function AppInner() { const trimmedProjectPath = projectPath.trim(); if (!trimmedProjectPath) return; - if (!api) { + if (!liveApi) { if (typeof window !== "undefined") { window.alert("Cannot archive merged workspaces: API not connected"); } @@ -882,7 +890,7 @@ function AppInner() { } try { - const result = await api.workspace.archiveMergedInProject({ + const result = await liveApi.workspace.archiveMergedInProject({ projectPath: trimmedProjectPath, }); @@ -919,15 +927,15 @@ function AppInner() { } } }, - [api] + [liveApi] ); const getBranchesForProject = useCallback( async (projectPath: string): Promise => { - if (!api) { + if (!liveApi) { return { branches: [], recommendedTrunk: null }; } - const branchResult = await api.projects.listBranches({ projectPath }); + const branchResult = await liveApi.projects.listBranches({ projectPath }); const sanitizedBranches = branchResult.branches.filter( (branch): branch is string => typeof branch === "string" ); @@ -942,7 +950,7 @@ function AppInner() { recommendedTrunk: recommended, }; }, - [api] + [liveApi] ); const selectWorkspaceFromPalette = useCallback( @@ -970,7 +978,7 @@ function AppInner() { ); const removeSubagentFromPalette = async (workspaceId: string, title: string) => { const notice = await confirmAndRemoveSubagent({ - api, + getApi: () => liveApiRef.current, confirm: confirmDialog, removeSubagent, workspaceId, @@ -1114,7 +1122,7 @@ function AppInner() { } }, onClearTimingStats: (workspaceId: string) => workspaceStore.clearTimingStats(workspaceId), - api, + api: liveApi, confirmDialog, }; diff --git a/src/browser/components/ChatPane/ChatPane.tsx b/src/browser/components/ChatPane/ChatPane.tsx index 2719e3eea65..d3cb2763a49 100644 --- a/src/browser/components/ChatPane/ChatPane.tsx +++ b/src/browser/components/ChatPane/ChatPane.tsx @@ -87,7 +87,7 @@ import { useContextSwitchWarning } from "@/browser/hooks/useContextSwitchWarning import { useProvidersConfig } from "@/browser/hooks/useProvidersConfig"; import { useSendMessageOptions } from "@/browser/hooks/useSendMessageOptions"; import type { TerminalSessionCreateOptions } from "@/browser/utils/terminal"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; import { useChatTranscriptFullWidth } from "@/browser/hooks/useChatTranscriptFullWidth"; import { CHAT_DOCK_GUTTER_CLASS } from "@/constants/layout"; import { isTranscriptMutationAllowed } from "@/browser/utils/transcriptBarrier"; @@ -355,6 +355,7 @@ export const ChatPaneContent: React.FC = (props) => { const chatTranscriptFullWidth = useChatTranscriptFullWidth(); const [transcriptDensity] = useTranscriptDensity(); const { api } = useAPI(); + const liveApi = useLiveAPI(); const { workspaceMetadata } = useWorkspaceContext(); const storeRaw = useWorkspaceStoreRaw(); const aggregator = useWorkspaceAggregator(workspaceId); @@ -1055,10 +1056,10 @@ export const ChatPaneContent: React.FC = (props) => { const handleQueuedDispatchModeChange = async (queueDispatchMode: QueueDispatchMode) => { clearQueuedActionError(); - if (!api) { + if (!liveApi) { throw new Error("Workspace API is unavailable."); } - const result = await api.workspace.setQueuedMessageDispatchMode({ + const result = await liveApi.workspace.setQueuedMessageDispatchMode({ workspaceId, queueDispatchMode, }); diff --git a/src/browser/components/DelegatedCreationInterruptedBanner/DelegatedCreationInterruptedBanner.tsx b/src/browser/components/DelegatedCreationInterruptedBanner/DelegatedCreationInterruptedBanner.tsx index 646f5aa09ab..6cb328453bc 100644 --- a/src/browser/components/DelegatedCreationInterruptedBanner/DelegatedCreationInterruptedBanner.tsx +++ b/src/browser/components/DelegatedCreationInterruptedBanner/DelegatedCreationInterruptedBanner.tsx @@ -1,7 +1,7 @@ import React, { useRef, useState } from "react"; import { Check, Loader2, TriangleAlert, Trash2 } from "lucide-react"; import { ChatDockSurface } from "@/browser/components/ChatPane/chatDockColumn"; -import { useAPI } from "@/browser/contexts/API"; +import { useLiveAPI } from "@/browser/contexts/API"; import { useConfirmDialog, type ConfirmDialogOptions, @@ -41,7 +41,7 @@ export interface DelegatedCreationInterruptedNoticeProps { export const DelegatedCreationInterruptedNotice: React.FC< DelegatedCreationInterruptedNoticeProps > = (props) => { - const { api } = useAPI(); + const liveApi = useLiveAPI(); const [pendingAction, setPendingAction] = useState(null); const [actionError, setActionError] = useState(null); // Synchronous guard: a second click can land before the disabled state renders. @@ -79,8 +79,8 @@ export const DelegatedCreationInterruptedNotice: React.FC< const keep = () => run("keep", async () => { - if (!api) return "Not connected to the backend"; - const result = await api.workspace.keepInterruptedDelegatedWorkspace({ + if (!liveApi) return "Not connected to the backend"; + const result = await liveApi.workspace.keepInterruptedDelegatedWorkspace({ workspaceId: props.workspaceId, }); return result.success ? null : result.error; diff --git a/src/browser/components/ProjectSidebar/ProjectSidebar.tsx b/src/browser/components/ProjectSidebar/ProjectSidebar.tsx index 10e2fb6505d..fac8a743759 100644 --- a/src/browser/components/ProjectSidebar/ProjectSidebar.tsx +++ b/src/browser/components/ProjectSidebar/ProjectSidebar.tsx @@ -42,7 +42,7 @@ import { isEditableElement, KEYBINDS, } from "@/browser/utils/ui/keybinds"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; import { CUSTOM_EVENTS, getStorageChangeEvent, @@ -662,18 +662,18 @@ function SidebarTitleEditKeybinds(props: { sortedWorkspacesByProject: Map; }) { const { requestEdit, wrapGenerateTitle } = useTitleEdit(); - const { api } = useAPI(); + const liveApi = useLiveAPI(); const regenerateTitleForWorkspace = useCallback( (workspaceId: string) => { wrapGenerateTitle(workspaceId, () => { - if (!api) { + if (!liveApi) { return Promise.resolve({ success: false, error: "Not connected to server" }); } - return api.workspace.regenerateTitle({ workspaceId }); + return liveApi.workspace.regenerateTitle({ workspaceId }); }); }, - [wrapGenerateTitle, api] + [wrapGenerateTitle, liveApi] ); useEffect(() => { diff --git a/src/browser/contexts/ProjectContext.tsx b/src/browser/contexts/ProjectContext.tsx index 3b0c23ccfd9..594f3def422 100644 --- a/src/browser/contexts/ProjectContext.tsx +++ b/src/browser/contexts/ProjectContext.tsx @@ -8,7 +8,7 @@ import { useState, type ReactNode, } from "react"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; import type { ProjectConfig } from "@/common/types/project"; import type { BranchListResult } from "@/common/orpc/types"; import type { z } from "zod"; @@ -138,6 +138,9 @@ function toNonEmptyTrimmed(value?: string | null): string | null { export function ProjectProvider(props: { children: ReactNode }) { const { api } = useAPI(); + // Handlers use only the live client: offline they keep returning their result (for example + // { success: false }), even while useAPI() keeps the last client during a reconnect. + const liveApi = useLiveAPI(); const [allProjectsInternal, setAllProjectsInternal] = useState>( new Map() ); @@ -285,7 +288,7 @@ export function ProjectProvider(props: { children: ReactNode }) { const removeProject = useCallback( async (path: string, options?: { force?: boolean }): Promise => { - if (!api) { + if (!liveApi) { return { success: false, error: { type: "unknown", message: "API not connected" }, @@ -295,7 +298,7 @@ export function ProjectProvider(props: { children: ReactNode }) { const draftIds = (getDraftStore().getCreationDraftsByProject()[path] ?? []).map( (draft) => draft.draftId ); - const result = await api.projects.remove({ + const result = await liveApi.projects.remove({ projectPath: path, force: options?.force, }); @@ -340,7 +343,7 @@ export function ProjectProvider(props: { children: ReactNode }) { }; } }, - [api, refreshProjects] + [liveApi, refreshProjects] ); const resolveProjectPath = useCallback( @@ -424,10 +427,10 @@ export function ProjectProvider(props: { children: ReactNode }) { const getBranchesForProject = useCallback( async (projectPath: string): Promise => { - if (!api) { + if (!liveApi) { return { branches: [], recommendedTrunk: "" }; } - const branchResult = await api.projects.listBranches({ projectPath }); + const branchResult = await liveApi.projects.listBranches({ projectPath }); const branches = branchResult.branches; const sanitizedBranches = Array.isArray(branches) ? branches.filter((branch): branch is string => typeof branch === "string") @@ -444,7 +447,7 @@ export function ProjectProvider(props: { children: ReactNode }) { recommendedTrunk: recommended, }; }, - [api] + [liveApi] ); const openWorkspaceModal = useCallback( @@ -508,63 +511,63 @@ export function ProjectProvider(props: { children: ReactNode }) { const getSecrets = useCallback( async (projectPath: string): Promise => { - if (!api) return []; - return await api.secrets.get({ projectPath }); + if (!liveApi) return []; + return await liveApi.secrets.get({ projectPath }); }, - [api] + [liveApi] ); const updateSecrets = useCallback( async (projectPath: string, secrets: Secret[]) => { - if (!api) return; - const result = await api.secrets.update({ projectPath, secrets }); + if (!liveApi) return; + const result = await liveApi.secrets.update({ projectPath, secrets }); if (!result.success) { console.error("Failed to update secrets:", result.error); } }, - [api] + [liveApi] ); const updateDisplayName = useCallback( async (projectPath: string, displayName: string | null): Promise> => { - if (!api) return { success: false, error: "API not connected" }; + if (!liveApi) return { success: false, error: "API not connected" }; try { - await api.projects.setDisplayName({ projectPath, displayName }); + await liveApi.projects.setDisplayName({ projectPath, displayName }); await refreshProjects(); return { success: true, data: undefined }; } catch (error) { return { success: false, error: getErrorMessage(error) }; } }, - [api, refreshProjects] + [liveApi, refreshProjects] ); const updateColor = useCallback( async (projectPath: string, color: string | null): Promise> => { - if (!api) return { success: false, error: "API not connected" }; + if (!liveApi) return { success: false, error: "API not connected" }; try { - await api.projects.setColor({ projectPath, color }); + await liveApi.projects.setColor({ projectPath, color }); await refreshProjects(); return { success: true, data: undefined }; } catch (error) { return { success: false, error: getErrorMessage(error) }; } }, - [api, refreshProjects] + [liveApi, refreshProjects] ); const updateCustomInstructions = useCallback( async (projectPath: string, customInstructions: string | null): Promise> => { - if (!api) return { success: false, error: "API not connected" }; + if (!liveApi) return { success: false, error: "API not connected" }; try { - await api.projects.setCustomInstructions({ projectPath, customInstructions }); + await liveApi.projects.setCustomInstructions({ projectPath, customInstructions }); await refreshProjects(); return { success: true, data: undefined }; } catch (error) { return { success: false, error: getErrorMessage(error) }; } }, - [api, refreshProjects] + [liveApi, refreshProjects] ); const assignWorkspaceToSubProject = useCallback( @@ -573,8 +576,8 @@ export function ProjectProvider(props: { children: ReactNode }) { workspaceId: string, subProjectPath: string | null ): Promise> => { - if (!api) return { success: false, error: "API not connected" }; - const result = await api.projects.subProjects.assignWorkspace({ + if (!liveApi) return { success: false, error: "API not connected" }; + const result = await liveApi.projects.subProjects.assignWorkspace({ projectPath, workspaceId, subProjectPath, @@ -584,7 +587,7 @@ export function ProjectProvider(props: { children: ReactNode }) { } return result; }, - [api, refreshProjects] + [liveApi, refreshProjects] ); const value = useMemo( @@ -601,13 +604,13 @@ export function ProjectProvider(props: { children: ReactNode }) { refreshProjects, addProject, removeProject, - // Inline (not useCallback-wrapped): `api` is already a dependency of + // Inline (not useCallback-wrapped): `liveApi` is already a dependency of // this provider value memo, mirroring the other inline actions below. getRemovalBlockers: async (path: string): Promise => { - if (!api) { + if (!liveApi) { throw new Error("API not connected"); } - return api.projects.getRemovalBlockers({ projectPath: path }); + return liveApi.projects.getRemovalBlockers({ projectPath: path }); }, projectCreateInitialPath, isProjectCreateModalOpen, @@ -629,14 +632,14 @@ export function ProjectProvider(props: { children: ReactNode }) { updateColor, updateCustomInstructions, // Defined inline (not useCallback): the repo bans new manual useCallback - // memoization, and inlining keeps exhaustive-deps satisfied via `api`. + // memoization, and inlining keeps exhaustive-deps satisfied via `liveApi`. updateCodeWorkspaceSyncPath: async ( projectPath: string, codeWorkspaceSyncPath: string | null ): Promise> => { - if (!api) return { success: false, error: "API not connected" }; + if (!liveApi) return { success: false, error: "API not connected" }; try { - await api.projects.setCodeWorkspaceSyncPath({ projectPath, codeWorkspaceSyncPath }); + await liveApi.projects.setCodeWorkspaceSyncPath({ projectPath, codeWorkspaceSyncPath }); await refreshProjects(); return { success: true, data: undefined }; } catch (error) { @@ -669,7 +672,7 @@ export function ProjectProvider(props: { children: ReactNode }) { updateDisplayName, updateColor, updateCustomInstructions, - api, + liveApi, assignWorkspaceToSubProject, ] ); diff --git a/src/browser/contexts/UILayoutsContext.tsx b/src/browser/contexts/UILayoutsContext.tsx index 4b160b10831..686b9d6c69d 100644 --- a/src/browser/contexts/UILayoutsContext.tsx +++ b/src/browser/contexts/UILayoutsContext.tsx @@ -7,7 +7,7 @@ import { useState, type ReactNode, } from "react"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; import assert from "@/common/utils/assert"; import { DEFAULT_LAYOUT_PRESETS_CONFIG, @@ -64,6 +64,7 @@ export function useUILayouts(): UILayoutsContextValue { export function UILayoutsProvider(props: { children: ReactNode }) { const { api } = useAPI(); + const liveApi = useLiveAPI(); const [layoutPresets, setLayoutPresets] = useState( DEFAULT_LAYOUT_PRESETS_CONFIG @@ -119,14 +120,14 @@ export function UILayoutsProvider(props: { children: ReactNode }) { async (next: LayoutPresetsConfig): Promise => { const normalized = normalizeLayoutPresetsConfig(next); - if (!api) { + if (!liveApi) { throw new Error("ORPC client not initialized"); } - await api.uiLayouts.saveAll({ layoutPresets: normalized }); + await liveApi.uiLayouts.saveAll({ layoutPresets: normalized }); setLayoutPresets(normalized); }, - [api] + [liveApi] ); useEffect(() => { @@ -140,9 +141,11 @@ export function UILayoutsProvider(props: { children: ReactNode }) { return; } - await applyLayoutPresetToWorkspace(api ?? null, workspaceId, preset); + // Offline, the preset is not applied: a terminal list cut off by the lost connection would + // look empty and drop the layout's terminal tabs. + await applyLayoutPresetToWorkspace(liveApi, workspaceId, preset); }, - [api, layoutPresets] + [liveApi, layoutPresets] ); const saveCurrentWorkspaceToSlot = useCallback( diff --git a/src/browser/contexts/WorkspaceContext.test.tsx b/src/browser/contexts/WorkspaceContext.test.tsx index c2ec0bce85b..d46efe1a1fa 100644 --- a/src/browser/contexts/WorkspaceContext.test.tsx +++ b/src/browser/contexts/WorkspaceContext.test.tsx @@ -35,6 +35,7 @@ import { resetWorkspaceStorageGcForTests } from "@/browser/utils/workspaceStorag import { resetCreationDraftStorageGcForTests } from "@/browser/utils/creationDraftStorageGc"; import { APIProvider, type APIClient } from "@/browser/contexts/API"; +import { createConnectionHarness, GAP_MODES, type GapMode } from "./connection.testHarness"; import * as path from "path"; // eslint-disable-next-line local/no-cross-boundary-imports -- test-only: drafts run against the real backend service import { Config } from "@/node/config"; @@ -963,6 +964,45 @@ describe("WorkspaceContext", () => { expect(result).toEqual(failure); }); + // useAPI() keeps the last client while reconnecting (kept-gap). The handlers must still see + // "offline": they return their result and do not call the lost connection. + describe.each([...GAP_MODES])("handlers while the connection is down (%s)", (mode) => { + test("return their offline result without calling the server", async () => { + const { workspace: workspaceApi } = createMockAPI(); + const contexts = await setupWithConnection(mode); + await contexts.harness.drop(); + + const notConnected = { success: false as const, error: "API not connected" }; + await act(async () => { + expect(await contexts.workspace().archiveWorkspace("ws-1")).toEqual(notConnected); + expect(await contexts.workspace().unarchiveWorkspace("ws-1")).toEqual(notConnected); + expect(await contexts.workspace().updateWorkspaceTitle("ws-1", "next")).toEqual( + notConnected + ); + expect(await contexts.workspace().setWorkspacePinned("ws-1", true)).toEqual(notConnected); + expect(await contexts.workspace().removeWorkspace("ws-1")).toEqual(notConnected); + expect(await contexts.workspace().getWorkspaceInfo("ws-1")).toBeNull(); + expect( + await contexts.project().assignWorkspaceToSubProject("/alpha", "ws-1", null) + ).toEqual(notConnected); + expect(await contexts.project().updateDisplayName("/alpha", "A")).toEqual(notConnected); + expect(await contexts.project().getSecrets("/alpha")).toEqual([]); + const created: unknown = await contexts + .workspace() + .createWorkspace("/alpha", "branch", "main") + .catch((error: unknown) => error); + expect(created).toEqual(new Error("API not connected")); + }); + expect(workspaceApi.archive).not.toHaveBeenCalled(); + expect(workspaceApi.unarchive).not.toHaveBeenCalled(); + expect(workspaceApi.updateTitle).not.toHaveBeenCalled(); + expect(workspaceApi.setPinned).not.toHaveBeenCalled(); + expect(workspaceApi.remove).not.toHaveBeenCalled(); + expect(workspaceApi.getInfo).not.toHaveBeenCalled(); + expect(workspaceApi.create).not.toHaveBeenCalled(); + }); + }); + describe("archiveWorkspace", () => { test("succeeds even when persisted layout is invalid JSON shape", async () => { const workspaceId = "ws-archive-invalid-layout"; @@ -2356,6 +2396,32 @@ function createDraftServiceClient(service: DraftService): TestApiOverrides currentClientMock); + const workspaceRef = { current: null as WorkspaceContext | null }; + const projectRef = { current: null as ReturnType | null }; + function ContextCapture() { + workspaceRef.current = useWorkspaceContext(); + projectRef.current = useProjectContext(); + return null; + } + render( + + + + + + + + + + ); + getWorkspaceStoreRaw().setClient(harness.client); + await waitFor(() => expect(workspaceRef.current?.loading).toBe(false)); + return { harness, workspace: () => workspaceRef.current!, project: () => projectRef.current! }; +} + async function setup() { const contexts = await setupWithProjectContext(); return contexts.workspace; diff --git a/src/browser/contexts/WorkspaceContext.tsx b/src/browser/contexts/WorkspaceContext.tsx index 401561f2c06..38dabf5c7cc 100644 --- a/src/browser/contexts/WorkspaceContext.tsx +++ b/src/browser/contexts/WorkspaceContext.tsx @@ -408,6 +408,9 @@ function buildActiveWorkspaceMetadataMap( export function WorkspaceProvider(props: WorkspaceProviderProps) { const { api } = useAPI(); + // Handlers use only the live client: offline they keep returning their result (for example + // { success: false }), even while useAPI() keeps the last client during a reconnect. + const liveApi = useLiveAPI(); // Get project refresh function from ProjectContext const { @@ -465,7 +468,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { [workspaceStore] ); // The discard waits for a live server: useAPI().api can keep the last client while offline. - useDiscardSideChatOnLeave(useLiveAPI(), currentWorkspaceId, workspaceMetadata); + useDiscardSideChatOnLeave(liveApi, currentWorkspaceId, workspaceMetadata); // The routed creation draft id, for the startup creation-draft storage GC (never collected). const routedDraftIdRef = useRef(null); const [loading, setLoading] = useState(true); @@ -1111,12 +1114,12 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { trunkBranch: string, runtimeConfig?: RuntimeConfig ) => { - if (!api) throw new Error("API not connected"); + if (!liveApi) throw new Error("API not connected"); console.assert( typeof trunkBranch === "string" && trunkBranch.trim().length > 0, "Expected trunk branch to be provided when creating a workspace" ); - const result = await api.workspace.create({ + const result = await liveApi.workspace.create({ projectPath, branchName, trunkBranch, @@ -1146,7 +1149,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { throw new Error(result.error); } }, - [api, refreshProjects, setWorkspaceMetadata] + [liveApi, refreshProjects, setWorkspaceMetadata] ); // Shared by workspace removal and user-confirmed sub-agent removal (#5106): both end with the @@ -1156,7 +1159,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { workspaceId: string, remove: (client: APIClient) => Promise ): Promise => { - if (!api) return { success: false, error: "API not connected" }; + if (!liveApi) return { success: false, error: "API not connected" }; // Capture state before the async operation. // We check currentWorkspaceId (from URL) rather than selectedWorkspace @@ -1169,7 +1172,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { const parentSelection = getParentWorkspaceSelection(metadata, workspaceMetadata); try { - const result = await remove(api); + const result = await remove(liveApi); if (result.success) { // Clean up workspace-specific localStorage keys deleteWorkspaceStorage(workspaceId); @@ -1218,7 +1221,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { selectedWorkspace, setSelectedWorkspace, workspaceMetadata, - api, + liveApi, setWorkspaceMetadata, ] ); @@ -1262,9 +1265,9 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { workspaceId: string, newTitle: string ): Promise<{ success: boolean; error?: string }> => { - if (!api) return { success: false, error: "API not connected" }; + if (!liveApi) return { success: false, error: "API not connected" }; try { - const result = await api.workspace.updateTitle({ workspaceId, title: newTitle }); + const result = await liveApi.workspace.updateTitle({ workspaceId, title: newTitle }); if (result.success) { // Workspace metadata subscription handles the title update automatically. // No need to refetch all metadata - this avoids expensive post-compaction @@ -1280,7 +1283,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { return { success: false, error: errorMessage }; } }, - [api] + [liveApi] ); /** @@ -1291,7 +1294,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { */ const setWorkspacePinned = useCallback( async (workspaceId: string, pinned: boolean): Promise<{ success: boolean; error?: string }> => { - if (!api) return { success: false, error: "API not connected" }; + if (!liveApi) return { success: false, error: "API not connected" }; let previousPinnedAtById: Map | null = null; setWorkspaceMetadata((prev) => { @@ -1336,7 +1339,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { }; try { - const result = await api.workspace.setPinned({ workspaceId, pinned }); + const result = await liveApi.workspace.setPinned({ workspaceId, pinned }); if (result.success) { return { success: true }; } @@ -1350,7 +1353,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { return { success: false, error: errorMessage }; } }, - [api, setWorkspaceMetadata] + [liveApi, setWorkspaceMetadata] ); /** @@ -1362,7 +1365,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { */ const reorderPinnedWorkspaces = useCallback( async (workspaceIds: string[]): Promise<{ success: boolean; error?: string }> => { - if (!api) return { success: false, error: "API not connected" }; + if (!liveApi) return { success: false, error: "API not connected" }; let previousPinnedAtById: Map | undefined; let optimisticPinnedAtById: Map | undefined; @@ -1412,7 +1415,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { }; try { - const result = await api.workspace.reorderPinned({ workspaceIds }); + const result = await liveApi.workspace.reorderPinned({ workspaceIds }); if (result.success) { return { success: true }; } @@ -1426,7 +1429,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { return { success: false, error: errorMessage }; } }, - [api, setWorkspaceMetadata] + [liveApi, setWorkspaceMetadata] ); const [archivingWorkspaceIds, setArchivingWorkspaceIds] = useState>( @@ -1438,14 +1441,14 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { async ( workspaceId: string ): Promise<{ success: boolean; error?: string; data?: ArchivePreflightResult }> => { - if (!api) return { success: false, error: "API not connected" }; + if (!liveApi) return { success: false, error: "API not connected" }; try { const result = await trackArchivingRequest( archivingRequestCounts.current, setArchivingWorkspaceIds, workspaceId, - () => api.workspace.preflightArchive({ workspaceId }) + () => liveApi.workspace.preflightArchive({ workspaceId }) ); if (result.success) { return { success: true, data: result.data }; @@ -1455,7 +1458,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { return { success: false, error: getErrorMessage(error) }; } }, - [api] + [liveApi] ); const archiveWorkspace = useCallback( @@ -1463,7 +1466,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { workspaceId: string, options?: { acknowledgedUntrackedPaths?: string[] } ): Promise<{ success: boolean; error?: string; data?: ArchiveWorkspaceResult }> => { - if (!api) return { success: false, error: "API not connected" }; + if (!liveApi) return { success: false, error: "API not connected" }; try { const result = await trackArchivingRequest( @@ -1471,7 +1474,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { setArchivingWorkspaceIds, workspaceId, () => - api.workspace.archive({ + liveApi.workspace.archive({ workspaceId, acknowledgedUntrackedPaths: options?.acknowledgedUntrackedPaths, }) @@ -1516,14 +1519,14 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { return { success: false, error: errorMessage }; } }, - [api] + [liveApi] ); const unarchiveWorkspace = useCallback( async (workspaceId: string): Promise<{ success: boolean; error?: string }> => { - if (!api) return { success: false, error: "API not connected" }; + if (!liveApi) return { success: false, error: "API not connected" }; try { - const result = await api.workspace.unarchive({ workspaceId }); + const result = await liveApi.workspace.unarchive({ workspaceId }); if (result.success) { // Workspace metadata subscription handles the state update automatically. return { success: true }; @@ -1537,7 +1540,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { return { success: false, error: errorMessage }; } }, - [api] + [liveApi] ); const refreshWorkspaceMetadata = useCallback(async () => { @@ -1579,15 +1582,15 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { const getWorkspaceInfo = useCallback( async (workspaceId: string) => { - if (!api) return null; - const metadata = await api.workspace.getInfo({ workspaceId }); + if (!liveApi) return null; + const metadata = await liveApi.workspace.getInfo({ workspaceId }); if (metadata) { ensureCreatedAt(metadata); setWorkspaceAiMetadata(metadata.id, metadata); } return metadata; }, - [api] + [liveApi] ); const promoteWorkspaceDraft = useCallback( diff --git a/src/browser/contexts/offlineResultGates.test.ts b/src/browser/contexts/offlineResultGates.test.ts new file mode 100644 index 00000000000..97b1fb6ae28 --- /dev/null +++ b/src/browser/contexts/offlineResultGates.test.ts @@ -0,0 +1,163 @@ +import { describe, expect, test } from "bun:test"; +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import ts from "typescript"; + +/** + * A handler that answers "offline" with a result (`{ success: false }`) or an error must check + * the live client (`useLiveAPI()`), not `useAPI().api`. While Xum reconnects, useAPI() can keep + * the last client: a gate on it lets the call through, and the call fails with + * ConnectionLostError instead of the handler's own offline result. + * + * The check follows the gated name to its declaration, whatever it is called: `{ api } = + * useAPI()`, `{ api: client } = useAPI()`, `useAPI().api`, or `state.api` with + * `state = useAPI()`. + */ +/** The names an offline gate checks: `!x`, `x == null`, and each operand of `a || b`. */ +function missingCheckNames(expression: ts.Expression): ts.Identifier[] { + if ( + ts.isBinaryExpression(expression) && + expression.operatorToken.kind === ts.SyntaxKind.BarBarToken + ) { + return [...missingCheckNames(expression.left), ...missingCheckNames(expression.right)]; + } + if (ts.isParenthesizedExpression(expression)) return missingCheckNames(expression.expression); + const name = missingCheckName(expression); + return name == null ? [] : [name]; +} + +function missingCheckName(expression: ts.Expression): ts.Identifier | null { + if ( + ts.isPrefixUnaryExpression(expression) && + expression.operator === ts.SyntaxKind.ExclamationToken + ) { + return ts.isIdentifier(expression.operand) ? expression.operand : null; + } + const isNullCheck = + ts.isBinaryExpression(expression) && + (expression.operatorToken.kind === ts.SyntaxKind.EqualsEqualsToken || + expression.operatorToken.kind === ts.SyntaxKind.EqualsEqualsEqualsToken) && + (expression.right.kind === ts.SyntaxKind.NullKeyword || + (ts.isIdentifier(expression.right) && expression.right.text === "undefined")); + return isNullCheck && ts.isIdentifier(expression.left) ? expression.left : null; +} + +const isUseAPICall = (node: ts.Expression | undefined): boolean => + node != null && + ts.isCallExpression(node) && + ts.isIdentifier(node.expression) && + node.expression.text === "useAPI"; + +/** The declaration of `name` that is visible at `from`, searching the enclosing blocks. */ +function findDeclaration(name: string, from: ts.Node): ts.VariableDeclaration | null { + for (let scope: ts.Node | undefined = from.parent; scope != null; scope = scope.parent) { + if (!ts.isBlock(scope) && !ts.isSourceFile(scope)) continue; + for (const statement of scope.statements) { + if (!ts.isVariableStatement(statement)) continue; + for (const declaration of statement.declarationList.declarations) { + if (declaredNames(declaration).has(name)) return declaration; + } + } + } + return null; +} + +/** Local name -> the property it reads (`{ api: client }` gives client -> api). */ +function declaredNames(declaration: ts.VariableDeclaration): Map { + const names = new Map(); + if (ts.isIdentifier(declaration.name)) names.set(declaration.name.text, null); + if (ts.isObjectBindingPattern(declaration.name)) { + for (const element of declaration.name.elements) { + if (!ts.isIdentifier(element.name)) continue; + const property = element.propertyName ?? element.name; + names.set(element.name.text, ts.isIdentifier(property) ? property.text : null); + } + } + return names; +} + +/** Whether `name` (declared at `declaration`) is the client from useAPI(). */ +function isUseAPIClient(name: string, declaration: ts.VariableDeclaration): boolean { + const init = declaration.initializer; + if (ts.isObjectBindingPattern(declaration.name)) { + return isUseAPICall(init) && declaredNames(declaration).get(name) === "api"; + } + // `useAPI().api`, or `state.api` where `state = useAPI()`. + if (init == null || !ts.isPropertyAccessExpression(init) || init.name.text !== "api") { + return false; + } + if (isUseAPICall(init.expression)) return true; + if (!ts.isIdentifier(init.expression)) return false; + const holder = findDeclaration(init.expression.text, declaration); + return holder != null && ts.isIdentifier(holder.name) && isUseAPICall(holder.initializer); +} + +function isUseAPIClientCheck(expression: ts.Expression): boolean { + return missingCheckNames(expression).some((name) => { + const declaration = findDeclaration(name.text, expression); + return declaration != null && isUseAPIClient(name.text, declaration); + }); +} + +function isOfflineResult(statement: ts.Statement | undefined, source: ts.SourceFile): boolean { + if (statement == null) return false; + if (ts.isThrowStatement(statement)) return true; + return ( + ts.isReturnStatement(statement) && + statement.expression != null && + /success:\s*false/.test(statement.expression.getText(source)) + ); +} + +function findOldGates(file: string, text: string): string[] { + const found: string[] = []; + const source = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX); + const visit = (node: ts.Node) => { + if (ts.isIfStatement(node) && isUseAPIClientCheck(node.expression)) { + const then = node.thenStatement; + if (isOfflineResult(ts.isBlock(then) ? then.statements[0] : then, source)) { + const line = source.getLineAndCharacterOfPosition(node.getStart(source)).line + 1; + found.push(`${file}:${line}`); + } + } + ts.forEachChild(node, visit); + }; + visit(source); + return found; +} + +function findOldGatesInRenderer(): string[] { + const files = execFileSync("git", ["ls-files", "src/browser"], { encoding: "utf8" }) + .split("\n") + .filter((file) => /\.tsx?$/.test(file) && !/\.(test|stories)\./.test(file)); + return files.flatMap((file) => { + const text = readFileSync(file, "utf8"); + return text.includes("useAPI") ? findOldGates(file, text) : []; + }); +} + +describe("offline result gates", () => { + test("check the live client, not useAPI().api", () => { + expect(findOldGatesInRenderer()).toEqual([]); + }); + + // The scanner itself: it follows each name to where the client comes from. + test.each([ + ["const { api } = useAPI();", "api", true], + ["const { api: client } = useAPI();", "client", true], + ["const client = useAPI().api;", "client", true], + ["const state = useAPI(); const client = state.api;", "client", true], + ["const api = useLiveAPI();", "api", false], + ["const { api } = props;", "api", false], + ])("%s, then a gate on %s: old gate %p", (declaration, name, old) => { + const text = [ + "function useX() {", + ` ${declaration}`, + " const run = () => {", + ` if (!ready || !${name}) return { success: false, error: "offline" };`, + " };", + "}", + ].join("\n"); + expect(findOldGates("x.tsx", text)).toEqual(old ? ["x.tsx:4"] : []); + }); +}); diff --git a/src/browser/features/ChatInput/useComposerAttachments.ts b/src/browser/features/ChatInput/useComposerAttachments.ts index f396b7d1093..b5892497fea 100644 --- a/src/browser/features/ChatInput/useComposerAttachments.ts +++ b/src/browser/features/ChatInput/useComposerAttachments.ts @@ -1,6 +1,6 @@ import { useState } from "react"; import type React from "react"; -import { useAPI } from "@/browser/contexts/API"; +import { useLiveAPI } from "@/browser/contexts/API"; import { extractAttachmentsFromClipboard, extractAttachmentsFromDrop, @@ -44,7 +44,7 @@ export function estimateBase64DataUrlBytes(dataUrl: string): number | null { return Math.floor((base64.length * 3) / 4) - padding; } export function useComposerAttachments(options: UseComposerAttachmentsOptions) { - const { api } = useAPI(); + const liveApi = useLiveAPI(); const { setAttachments, editingMessage, pushToast, variant, workspaceId } = options; const [processingAttachmentCount, setProcessingAttachmentCount] = useState(0); @@ -70,10 +70,10 @@ export function useComposerAttachments(options: UseComposerAttachmentsOptions) { stageAttachment: variant === "workspace" ? async (file, dataBase64) => { - if (!api) throw new Error("Not connected to server"); + if (!liveApi) throw new Error("Not connected to server"); if (workspaceId == null) throw new Error("Files can be staged after opening a workspace."); - const result = await api.workspace.stageAttachment({ + const result = await liveApi.workspace.stageAttachment({ workspaceId, filename: file.name, mediaType: file.type || null, diff --git a/src/browser/features/ChatInput/useCreationWorkspace.ts b/src/browser/features/ChatInput/useCreationWorkspace.ts index 45faf2a4efb..f61f00a4f56 100644 --- a/src/browser/features/ChatInput/useCreationWorkspace.ts +++ b/src/browser/features/ChatInput/useCreationWorkspace.ts @@ -33,7 +33,7 @@ import type { SendMessageError } from "@/common/types/errors"; import { useOptionalWorkspaceContext } from "@/browser/contexts/WorkspaceContext"; import { useRouter } from "@/browser/contexts/RouterContext"; import type { Toast } from "@/browser/features/ChatInput/ChatInputToast"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; import { useProjectContext } from "@/browser/contexts/ProjectContext"; import { ConfirmationModal } from "@/browser/components/ConfirmationModal/ConfirmationModal"; import { useProvidersConfig } from "@/browser/hooks/useProvidersConfig"; @@ -286,6 +286,7 @@ export function useCreationWorkspace({ // Keep router state fresh synchronously so auto-navigation checks don't lag behind route changes. latestRouteRef.current = { currentWorkspaceId, currentProjectId, pendingDraftId }; const { api } = useAPI(); + const liveApi = useLiveAPI(); const { getProjectConfig, refreshProjects, loading: projectsLoading } = useProjectContext(); const { config: providersConfig } = useProvidersConfig(); const [branches, setBranches] = useState([]); @@ -392,7 +393,7 @@ export function useCreationWorkspace({ // File-only sends are valid; the attached-files notice or provider file // parts carry the content. const hasSendableAttachments = pendingFilesToStage.length > 0 || (fileParts?.length ?? 0) > 0; - if ((!messageText.trim() && !hasSendableAttachments) || isSending || !api) { + if ((!messageText.trim() && !hasSendableAttachments) || isSending || !liveApi) { return { success: false }; } @@ -549,8 +550,8 @@ export function useCreationWorkspace({ const createResult = kind === "scratch" - ? await api.workspace.createScratch({ title: createTitle }) - : await api.workspace.create({ + ? await liveApi.workspace.createScratch({ title: createTitle }) + : await liveApi.workspace.create({ projectPath, branchName: identity.name, trunkBranch: settings.trunkBranch, @@ -578,7 +579,7 @@ export function useCreationWorkspace({ // is portable across devices even before the first stream starts. Initial /goal commands do // not send a normal user message, so they await this write before setting the goal; that lets // the backend kickoff continuation use the same model/agent selected in creation. - const initialAiSettingsPersisted = api.workspace + const initialAiSettingsPersisted = liveApi.workspace .updateAgentAISettings({ workspaceId: metadata.id, agentId: creationAgentId, @@ -723,7 +724,7 @@ export function useCreationWorkspace({ // first send so the attached-files notice can reference real staged paths. const stagingOutcome = pendingFilesToStage.length > 0 - ? await stagePendingFiles(api, metadata.id, pendingFilesToStage) + ? await stagePendingFiles(liveApi, metadata.id, pendingFilesToStage) : { staged: [], failures: [] }; const stagingFailed = stagingOutcome.failures.length > 0; @@ -755,7 +756,7 @@ export function useCreationWorkspace({ if (initialSlashCommand) { await initialAiSettingsPersisted; const commandEnv: SlashCommandEnv = { - api, + api: liveApi, workspaceId: metadata.id, variant: "workspace", projectPath: metadata.projectPath, @@ -864,7 +865,7 @@ export function useCreationWorkspace({ // A transport-level rejection (e.g. oRPC disconnect) must flow through // the same failure branch as success:false: the outer catch would skip // the staged-draft transfer and the creation draft is already cleared. - const sendResult = await api.workspace + const sendResult = await liveApi.workspace .sendMessage({ workspaceId: metadata.id, message: appendStagedAttachmentNotice(messageText, stagingOutcome.staged), @@ -931,7 +932,7 @@ export function useCreationWorkspace({ }, [ kind, - api, + liveApi, isSending, projectPath, projectScopeId, @@ -969,8 +970,8 @@ export function useCreationWorkspace({ cancelLabel: "Don't create", onConfirm: async () => { try { - if (!api) throw new Error("API not available"); - await api.projects.setTrust({ + if (!liveApi) throw new Error("API not available"); + await liveApi.projects.setTrust({ projectPath: trustPrompt.projectPath, trusted: true, }); diff --git a/src/browser/features/RightSidebar/RightSidebar.tsx b/src/browser/features/RightSidebar/RightSidebar.tsx index 053aa25b8aa..26731c423ea 100644 --- a/src/browser/features/RightSidebar/RightSidebar.tsx +++ b/src/browser/features/RightSidebar/RightSidebar.tsx @@ -22,7 +22,7 @@ import { usePersistedState, } from "@/browser/hooks/usePersistedState"; import { useProvidersConfig } from "@/browser/hooks/useProvidersConfig"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, liveClientOf } from "@/browser/contexts/API"; import { useWorkspaceMetadata } from "@/browser/contexts/WorkspaceContext"; import { useSendMessageOptions } from "@/browser/hooks/useSendMessageOptions"; import { @@ -917,6 +917,7 @@ const RightSidebarComponent: React.FC = ({ // API for reading config and managing terminal sessions. const apiState = useAPI(); const api = apiState.api; + const liveApi = liveClientOf(apiState); const desktopExperimentEnabled = useExperimentValue(EXPERIMENT_IDS.PORTABLE_DESKTOP); const artifactsExperimentEnabled = useExperimentValue(EXPERIMENT_IDS.ARTIFACTS); const browserExperimentEnabled = useExperimentValue(EXPERIMENT_IDS.AGENT_BROWSER); @@ -957,14 +958,14 @@ const RightSidebarComponent: React.FC = ({ // to mutate the existing goal record instead of archiving+recreating. editInPlace?: boolean; }) => { - if (!api) { + if (!liveApi) { throw new Error("Backend is not connected."); } // Target the goal this sidebar displayed: a goal replaced since then refuses the edit // (#5461) instead of receiving it. // Before the sidebar state loads, the displayed goal is unknown (not "no goal"): read it. const intendedGoalId = sidebarState == null ? undefined : intendedGoalIdOf(goal); - const result = await setGoalForIntendedGoal(api, workspaceId, intent, intendedGoalId); + const result = await setGoalForIntendedGoal(liveApi, workspaceId, intent, intendedGoalId); if (!result.success) { throw new Error(getGoalSetErrorMessage(result.error)); } @@ -1004,21 +1005,21 @@ const RightSidebarComponent: React.FC = ({ }; const handleGoalClear = async () => { - if (!api) { + if (!liveApi) { throw new Error("Backend is not connected."); } - await api.workspace.clearGoal({ workspaceId }); + await liveApi.workspace.clearGoal({ workspaceId }); }; const handleGoalCreate = async (intent: GoalCreateIntent) => { - if (!api) { + if (!liveApi) { throw new Error("Backend is not connected."); } // Apply shared defaults (turn cap + `alwaysRequireExplicitBudget`) // so the GoalTab form, slash command, and command palette produce // identical goals for identical inputs. Pass workspaceId so any // per-workspace override wins over the global. - const defaults = await loadGoalDefaults(api, workspaceId); + const defaults = await loadGoalDefaults(liveApi, workspaceId); const resolved = resolveGoalSetIntent(intent, defaults); if (hasGoalBudgetLimit(resolved.budgetCents)) { // Fetch provider config at submit time so quick submits before the @@ -1027,7 +1028,7 @@ const RightSidebarComponent: React.FC = ({ // surfaces in lockstep. let freshProvidersConfig: unknown = providersConfig; try { - freshProvidersConfig = await api.providers.getConfig(); + freshProvidersConfig = await liveApi.providers.getConfig(); } catch { // Fall back to the hook value (which may still be null). Better // to surface the pricing-gate error than to leak the network diff --git a/src/browser/features/Settings/Sections/ModelsSection.tsx b/src/browser/features/Settings/Sections/ModelsSection.tsx index 48a8967f4c1..86cbc93f3d9 100644 --- a/src/browser/features/Settings/Sections/ModelsSection.tsx +++ b/src/browser/features/Settings/Sections/ModelsSection.tsx @@ -11,7 +11,7 @@ import { SelectTrigger, SelectValue, } from "@/browser/components/SelectPrimitive/SelectPrimitive"; -import { useAPI } from "@/browser/contexts/API"; +import { useAPI, useLiveAPI } from "@/browser/contexts/API"; import { useSettings } from "@/browser/contexts/SettingsContext"; import { useModelsFromSettings } from "@/browser/hooks/useModelsFromSettings"; import { useRouting } from "@/browser/hooks/useRouting"; @@ -127,6 +127,7 @@ export function shouldAllowRouteOverrideInSettings(modelId: string): boolean { export function ModelsSection() { const { api } = useAPI(); + const liveApi = useLiveAPI(); const { open: openSettings, close: closeSettings } = useSettings(); const { config, loading, updateModelsOptimistically } = useProvidersConfig(); const [lastProvider, setLastProvider] = usePersistedState(LAST_CUSTOM_MODEL_PROVIDER_KEY, ""); @@ -226,14 +227,14 @@ export function ModelsSection() { return false; } - if (!api) return false; + if (!liveApi) return false; setError(null); // Optimistic update - returns new models array for API call const updatedModels = updateModelsOptimistically(provider, (models) => [...models, modelId]); // Save in background - void api.providers.setModels({ provider, models: updatedModels }); + void liveApi.providers.setModels({ provider, models: updatedModels }); return true; }; @@ -514,7 +515,7 @@ export function ModelsSection() { const handleRemoveModel = useCallback( (provider: string, modelId: string) => { - if (!config || !api) return; + if (!config || !liveApi) return; // Optimistic update - returns new models array for API call const updatedModels = updateModelsOptimistically(provider, (models) => @@ -522,9 +523,9 @@ export function ModelsSection() { ); // Save in background - void api.providers.setModels({ provider, models: updatedModels }); + void liveApi.providers.setModels({ provider, models: updatedModels }); }, - [api, config, updateModelsOptimistically] + [liveApi, config, updateModelsOptimistically] ); const handleStartEdit = useCallback( @@ -573,7 +574,7 @@ export function ModelsSection() { }, []); const handleSaveEdit = useCallback(() => { - if (!config || !editing || !api) return; + if (!config || !editing || !liveApi) return; const trimmedModelId = editing.newModelId.trim(); if (!trimmedModelId) { @@ -634,8 +635,8 @@ export function ModelsSection() { setEditing(null); // Save in background - void api.providers.setModels({ provider: editing.provider, models: updatedModels }); - }, [api, editing, config, modelExists, updateModelsOptimistically]); + void liveApi.providers.setModels({ provider: editing.provider, models: updatedModels }); + }, [liveApi, editing, config, modelExists, updateModelsOptimistically]); // Show loading state while config is being fetched if (loading || !config) { diff --git a/src/browser/hooks/useExperiments.ts b/src/browser/hooks/useExperiments.ts index 60978cf0433..4ff3c012116 100644 --- a/src/browser/hooks/useExperiments.ts +++ b/src/browser/hooks/useExperiments.ts @@ -1,6 +1,6 @@ import { useSyncExternalStore } from "react"; import type { ExperimentId } from "@/common/constants/experiments"; -import { useAPI } from "@/browser/contexts/API"; +import { useLiveAPI } from "@/browser/contexts/API"; import { getAppConfigStore } from "@/browser/stores/AppConfigStore"; /** The backend value of one experiment from the AppConfigStore snapshot (off until it loads). */ @@ -17,12 +17,12 @@ export function useExperiment( experimentId: ExperimentId ): [boolean, (enabled: boolean) => Promise] { const enabled = useExperimentValue(experimentId); - const { api } = useAPI(); + const liveApi = useLiveAPI(); // No optimistic state: the toggle shows the snapshot refetched after the backend write. Do not // rely on onConfigChanged alone: a dropped subscription never delivers it. const setEnabled = async (value: boolean) => { - if (!api) throw new Error("Not connected to the backend"); - await api.experiments.set({ experimentId, enabled: value }); + if (!liveApi) throw new Error("Not connected to the backend"); + await liveApi.experiments.set({ experimentId, enabled: value }); await getAppConfigStore().refresh(); }; return [enabled, setEnabled]; diff --git a/src/browser/hooks/useOpenInEditor.ts b/src/browser/hooks/useOpenInEditor.ts index e3fbb7147d9..e94e20ada3a 100644 --- a/src/browser/hooks/useOpenInEditor.ts +++ b/src/browser/hooks/useOpenInEditor.ts @@ -1,5 +1,5 @@ import { useCallback } from "react"; -import { useAPI } from "@/browser/contexts/API"; +import { useLiveAPI } from "@/browser/contexts/API"; import { useSettings } from "@/browser/contexts/SettingsContext"; import type { RuntimeConfig } from "@/common/types/runtime"; import { openInEditor } from "@/browser/utils/openInEditor"; @@ -13,7 +13,8 @@ export type { OpenInEditorResult } from "@/browser/utils/openInEditor"; * `src/browser/utils/openInEditor.ts`. */ export function useOpenInEditor() { - const { api } = useAPI(); + // The live client only: offline, openInEditor() returns its "API not available" result. + const api = useLiveAPI(); const { open: openSettings } = useSettings(); return useCallback( diff --git a/src/browser/utils/subagentRemoval.test.ts b/src/browser/utils/subagentRemoval.test.ts index a7bd9e14511..64783a8e053 100644 --- a/src/browser/utils/subagentRemoval.test.ts +++ b/src/browser/utils/subagentRemoval.test.ts @@ -19,18 +19,32 @@ function setup(options: { const api = createTestApiClient({ tasks: { previewRemoval: () => Promise.resolve(options.preview) }, }); + const live = { api: api as typeof api | null }; const run = () => confirmAndRemoveSubagent({ - api, + getApi: () => live.api, confirm, removeSubagent, workspaceId: "child", title: "Child", }); - return { api, confirm, removeSubagent, run }; + return { api, live, confirm, removeSubagent, run }; } describe("confirmAndRemoveSubagent (#5106)", () => { + test("a connection lost while the dialog is open removes nothing", async () => { + const offline = setup({ + preview: { success: true, data: { summary: null, paths: [] } }, + confirmed: true, + }); + offline.confirm.mockImplementation(() => { + offline.live.api = null; + return Promise.resolve(true); + }); + expect(await offline.run()).toEqual({ kind: "error", message: "API not connected" }); + expect(offline.removeSubagent).not.toHaveBeenCalled(); + }); + test("lists the unpreserved work and removes only after the user confirms", async () => { const preview = { summary: "Removing this sub-agent would delete files.", diff --git a/src/browser/utils/subagentRemoval.ts b/src/browser/utils/subagentRemoval.ts index e854d7fa83a..1ac158c5c08 100644 --- a/src/browser/utils/subagentRemoval.ts +++ b/src/browser/utils/subagentRemoval.ts @@ -14,7 +14,8 @@ export type SubagentRemovalNotice = { kind: "error" | "warning"; message: string * returns a warning, not an error (#5190). */ export async function confirmAndRemoveSubagent(params: { - api: APIClient | null; + /** The live client now (null while offline): read again after the user confirms. */ + getApi: () => APIClient | null; confirm: (options: ConfirmDialogOptions) => Promise; removeSubagent: ( workspaceId: string, @@ -23,11 +24,13 @@ export async function confirmAndRemoveSubagent(params: { workspaceId: string; title: string; }): Promise { - if (params.api == null) return { kind: "error", message: "API not connected" }; + const notConnected = { kind: "error", message: "API not connected" } as const; + const api = params.getApi(); + if (api == null) return notConnected; // Palette actions run fire-and-forget, so a transport failure must come back as an error too. let preview: Awaited>; try { - preview = await params.api.tasks.previewRemoval({ taskId: params.workspaceId }); + preview = await api.tasks.previewRemoval({ taskId: params.workspaceId }); } catch (error) { return { kind: "error", message: getErrorMessage(error) }; } @@ -41,8 +44,13 @@ export async function confirmAndRemoveSubagent(params: { confirmVariant: "destructive", }); if (!confirmed) return null; + // The connection can drop while the dialog is open. + if (params.getApi() == null) return notConnected; // The backend refuses if the work changed since this preview (#5106). - const result = await params.removeSubagent(params.workspaceId, preview.data); + return removalNotice(await params.removeSubagent(params.workspaceId, preview.data)); +} + +function removalNotice(result: WorkspaceRemoveResult): SubagentRemovalNotice { if (!result.success) { return { kind: "error", message: result.error ?? "Failed to remove the sub-agent" }; }