From 2f7d8405e1040b3444ac434b1c2c13a66c097ccf Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Tue, 28 Apr 2026 12:03:52 -0700 Subject: [PATCH 1/7] fix(desktop): make file tree resilient to slow directory loads - Time out listDirectory after 5s and retry up to 3x with linear backoff - Abort in-flight requests on unmount and workspace/root change - Show a loading spinner in FilesTab while the workspace query resolves - Enable abortOnUnmount globally on the workspace tRPC client --- .../host-service/useFileTree/useFileTree.ts | 317 ++++++++++++++---- .../components/FilesTab/FilesTab.tsx | 21 +- .../workspace-client/src/workspace-trpc.ts | 4 +- 3 files changed, 281 insertions(+), 61 deletions(-) diff --git a/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts b/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts index 1d07dd299d8..f4ba56cd82a 100644 --- a/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts +++ b/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts @@ -40,6 +40,134 @@ interface FileTreeState { loadingDirectories: Set; } +interface FileTreeContext { + rootPath: string; + workspaceId: string; +} + +interface LoadDirectoryOptions { + force?: boolean; + ignoreInFlight?: boolean; + attempt?: number; +} + +const LIST_DIRECTORY_TIMEOUT_MS = 5_000; +const LIST_DIRECTORY_MAX_ATTEMPTS = 3; +const LIST_DIRECTORY_RETRY_DELAY_MS = 300; + +class ListDirectoryTimeoutError extends Error { + constructor(timeoutMs: number) { + super(`listDirectory timed out after ${timeoutMs}ms`); + this.name = "ListDirectoryTimeoutError"; + } +} + +async function withAbortableTimeout( + fetcher: (signal: AbortSignal) => Promise, + timeoutMs: number, + abortController: AbortController, +): Promise { + let timeout: ReturnType | null = null; + let timedOut = false; + timeout = setTimeout(() => { + timedOut = true; + abortController.abort(); + }, timeoutMs); + + try { + return await fetcher(abortController.signal); + } catch (error) { + if (timedOut) { + throw new ListDirectoryTimeoutError(timeoutMs); + } + throw error; + } finally { + if (timeout) { + clearTimeout(timeout); + } + } +} + +function applyDirectoryEntries( + current: FileTreeState, + absolutePath: string, + entries: FsEntry[], +): FileTreeState { + const nextEntries = new Map(current.entriesByPath); + const nextChildren = new Map(current.childPathsByDirectory); + const nextLoaded = new Set(current.loadedDirectories); + const nextInvalidated = new Set(current.invalidatedDirectories); + const nextLoading = new Set(current.loadingDirectories); + nextLoading.delete(absolutePath); + nextLoaded.add(absolutePath); + nextInvalidated.delete(absolutePath); + + for (const entry of entries) { + nextEntries.set(entry.absolutePath, entry); + } + + nextChildren.set( + absolutePath, + entries.map((entry) => entry.absolutePath), + ); + + return { + ...current, + childPathsByDirectory: nextChildren, + entriesByPath: nextEntries, + invalidatedDirectories: nextInvalidated, + loadedDirectories: nextLoaded, + loadingDirectories: nextLoading, + }; +} + +function isSameFileTreeContext( + left: FileTreeContext, + right: FileTreeContext, +): boolean { + return ( + left.rootPath === right.rootPath && left.workspaceId === right.workspaceId + ); +} + +function markDirectoryLoading( + current: FileTreeState, + absolutePath: string, +): FileTreeState { + const nextLoading = new Set(current.loadingDirectories); + nextLoading.add(absolutePath); + return { + ...current, + loadingDirectories: nextLoading, + }; +} + +function clearDirectoryLoading( + current: FileTreeState, + absolutePath: string, +): FileTreeState { + const nextLoading = new Set(current.loadingDirectories); + nextLoading.delete(absolutePath); + return { + ...current, + loadingDirectories: nextLoading, + }; +} + +function shouldRetryListDirectory( + error: unknown, + nextAttempt: number, +): boolean { + return ( + error instanceof ListDirectoryTimeoutError && + nextAttempt <= LIST_DIRECTORY_MAX_ATTEMPTS + ); +} + +function getListDirectoryRetryDelayMs(attempt: number): number { + return LIST_DIRECTORY_RETRY_DELAY_MS * attempt; +} + function createInitialState(): FileTreeState { return { childPathsByDirectory: new Map(), @@ -171,10 +299,39 @@ export function useFileTree({ rootPath, }: UseFileTreeParams): UseFileTreeResult { const utils = workspaceTrpc.useUtils(); + const activeContextRef = useRef({ rootPath, workspaceId }); + activeContextRef.current = { rootPath, workspaceId }; + const isMountedRef = useRef(false); + const retryTimersRef = useRef(new Set>()); + const activeLoadAbortControllersRef = useRef(new Set()); const [state, setState] = useState(() => createInitialState()); const stateRef = useRef(state); stateRef.current = state; + const clearRetryTimers = useCallback(() => { + for (const timer of retryTimersRef.current) { + clearTimeout(timer); + } + retryTimersRef.current.clear(); + }, []); + + const abortActiveLoads = useCallback(() => { + for (const abortController of activeLoadAbortControllersRef.current) { + abortController.abort(); + } + activeLoadAbortControllersRef.current.clear(); + }, []); + + useEffect(() => { + isMountedRef.current = true; + + return () => { + isMountedRef.current = false; + clearRetryTimers(); + abortActiveLoads(); + }; + }, [abortActiveLoads, clearRetryTimers]); + const updateState = useCallback( (updater: (current: FileTreeState) => FileTreeState) => { setState((current) => { @@ -186,14 +343,38 @@ export function useFileTree({ [], ); + const loadDirectoryRef = useRef< + | ((absolutePath: string, options?: LoadDirectoryOptions) => Promise) + | null + >(null); + + const isRequestCurrent = useCallback((requestContext: FileTreeContext) => { + return ( + isMountedRef.current && + isSameFileTreeContext(activeContextRef.current, requestContext) + ); + }, []); + const loadDirectory = useCallback( - async (absolutePath: string, force = false): Promise => { + async ( + absolutePath: string, + options: LoadDirectoryOptions = {}, + ): Promise => { + const { force = false, ignoreInFlight = false, attempt = 1 } = options; if (!workspaceId || !absolutePath) { return; } + const requestContext = { rootPath, workspaceId }; + if (!isRequestCurrent(requestContext)) { + return; + } + const currentState = stateRef.current; - if (currentState.loadingDirectories.has(absolutePath)) { + if ( + !ignoreInFlight && + currentState.loadingDirectories.has(absolutePath) + ) { return; } @@ -205,50 +386,62 @@ export function useFileTree({ return; } - updateState((current) => { - const nextLoading = new Set(current.loadingDirectories); - nextLoading.add(absolutePath); - return { - ...current, - loadingDirectories: nextLoading, - }; - }); + const input = { workspaceId, absolutePath }; + const cachedResult = utils.filesystem.listDirectory.getData(input); + if (cachedResult) { + updateState((current) => + applyDirectoryEntries(current, absolutePath, cachedResult.entries), + ); + if (!force) { + return; + } + } - try { - const result = await utils.filesystem.listDirectory.fetch({ - workspaceId, - absolutePath, - }); + updateState((current) => markDirectoryLoading(current, absolutePath)); - updateState((current) => { - const nextEntries = new Map(current.entriesByPath); - const nextChildren = new Map(current.childPathsByDirectory); - const nextLoaded = new Set(current.loadedDirectories); - const nextInvalidated = new Set(current.invalidatedDirectories); - const nextLoading = new Set(current.loadingDirectories); - nextLoading.delete(absolutePath); - nextLoaded.add(absolutePath); - nextInvalidated.delete(absolutePath); - - for (const entry of result.entries) { - nextEntries.set(entry.absolutePath, entry); - } + const abortController = new AbortController(); + activeLoadAbortControllersRef.current.add(abortController); - nextChildren.set( - absolutePath, - result.entries.map((entry) => entry.absolutePath), - ); + try { + const result = await withAbortableTimeout( + (signal) => + utils.filesystem.listDirectory.fetch(input, { + trpc: { signal }, + }), + LIST_DIRECTORY_TIMEOUT_MS, + abortController, + ); - return { - ...current, - childPathsByDirectory: nextChildren, - entriesByPath: nextEntries, - invalidatedDirectories: nextInvalidated, - loadedDirectories: nextLoaded, - loadingDirectories: nextLoading, - }; + if (!isRequestCurrent(requestContext)) { + return; + } + + updateState((current) => { + return applyDirectoryEntries(current, absolutePath, result.entries); }); } catch (error) { + if (!isRequestCurrent(requestContext)) { + return; + } + + const nextAttempt = attempt + 1; + if (shouldRetryListDirectory(error, nextAttempt)) { + const retryTimer = setTimeout(() => { + retryTimersRef.current.delete(retryTimer); + if (!isRequestCurrent(requestContext)) { + return; + } + + void loadDirectoryRef.current?.(absolutePath, { + attempt: nextAttempt, + force: true, + ignoreInFlight: true, + }); + }, getListDirectoryRetryDelayMs(attempt)); + retryTimersRef.current.add(retryTimer); + return; + } + console.error( "[workspace-client/useFileTree] Failed to load directory:", { @@ -258,21 +451,25 @@ export function useFileTree({ ); updateState((current) => { - const nextLoading = new Set(current.loadingDirectories); - nextLoading.delete(absolutePath); - return { - ...current, - loadingDirectories: nextLoading, - }; + return clearDirectoryLoading(current, absolutePath); }); + } finally { + activeLoadAbortControllersRef.current.delete(abortController); } }, - [updateState, utils.filesystem.listDirectory, workspaceId], + [ + isRequestCurrent, + rootPath, + updateState, + utils.filesystem.listDirectory, + workspaceId, + ], ); + loadDirectoryRef.current = loadDirectory; const refreshPath = useCallback( async (absolutePath: string): Promise => { - await loadDirectory(absolutePath, true); + await loadDirectory(absolutePath, { force: true }); }, [loadDirectory], ); @@ -288,10 +485,10 @@ export function useFileTree({ (left, right) => left.split(/[/\\]/).length - right.split(/[/\\]/).length, ); - await loadDirectory(rootPath, true); + await loadDirectory(rootPath, { force: true }); for (const absolutePath of expandedDirectories) { if (absolutePath !== rootPath) { - await loadDirectory(absolutePath, true); + await loadDirectory(absolutePath, { force: true }); } } }, [loadDirectory, rootPath]); @@ -346,13 +543,21 @@ export function useFileTree({ }, [updateState]); useEffect(() => { + clearRetryTimers(); + abortActiveLoads(); updateState(() => createInitialState()); if (!rootPath) { return; } - void loadDirectory(rootPath, true); - }, [loadDirectory, rootPath, updateState]); + void loadDirectory(rootPath, { force: true }); + }, [ + abortActiveLoads, + clearRetryTimers, + loadDirectory, + rootPath, + updateState, + ]); useWorkspaceEvent( "fs:events", @@ -404,16 +609,16 @@ export function useFileTree({ }); if (stateRef.current.loadedDirectories.has(oldParentPath)) { - void loadDirectory(oldParentPath, true); + void loadDirectory(oldParentPath, { force: true }); } if (stateRef.current.loadedDirectories.has(newParentPath)) { - void loadDirectory(newParentPath, true); + void loadDirectory(newParentPath, { force: true }); } if ( event.isDirectory && stateRef.current.expandedDirectories.has(event.absolutePath) ) { - void loadDirectory(event.absolutePath, true); + void loadDirectory(event.absolutePath, { force: true }); } return; } @@ -438,7 +643,7 @@ export function useFileTree({ }); if (stateRef.current.loadedDirectories.has(parentPath)) { - void loadDirectory(parentPath, true); + void loadDirectory(parentPath, { force: true }); } }, Boolean(workspaceId && rootPath), diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx index b90f8e315b0..68efdfeaca6 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx @@ -5,7 +5,13 @@ import { toast } from "@superset/ui/sonner"; import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip"; import { workspaceTrpc } from "@superset/workspace-client"; import type { inferRouterOutputs } from "@trpc/server"; -import { FilePlus, FolderPlus, FoldVertical, RefreshCw } from "lucide-react"; +import { + FilePlus, + FolderPlus, + FoldVertical, + Loader2, + RefreshCw, +} from "lucide-react"; import { Fragment, useCallback, useEffect, useRef, useState } from "react"; import { type FileTreeNode, @@ -462,10 +468,17 @@ export function FilesTab({ [workspaceId, deletePath], ); - if (!workspaceQuery.data?.worktreePath) { + if (!rootPath) { return ( -
- Workspace worktree not available +
+ {workspaceQuery.isLoading || workspaceQuery.isFetching ? ( + <> + + Loading files... + + ) : ( + "Workspace worktree not available" + )}
); } diff --git a/packages/workspace-client/src/workspace-trpc.ts b/packages/workspace-client/src/workspace-trpc.ts index d601513a5c2..6d38436c15f 100644 --- a/packages/workspace-client/src/workspace-trpc.ts +++ b/packages/workspace-client/src/workspace-trpc.ts @@ -1,4 +1,6 @@ import type { AppRouter } from "@superset/host-service/trpc"; import { createTRPCReact } from "@trpc/react-query"; -export const workspaceTrpc = createTRPCReact(); +export const workspaceTrpc = createTRPCReact({ + abortOnUnmount: true, +}); From c44a81ad00ab238bcc983a4492cabdc5c4d0cbec Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Tue, 28 Apr 2026 12:18:47 -0700 Subject: [PATCH 2/7] refactor(desktop): simplify useFileTree cancellation, drop FilesTab workspace name - Use AbortController membership in the active set as the "still relevant" check, replacing isMountedRef + activeContextRef + isRequestCurrent - Inline single-use helpers (markDirectoryLoading, clearDirectoryLoading, shouldRetryListDirectory, getListDirectoryRetryDelayMs) - Drop the workspaceName prop from FilesTab/WorkspaceSidebar/WorkspaceContent; the files header is just "Explorer" --- .../host-service/useFileTree/useFileTree.ts | 121 ++++-------------- .../WorkspaceSidebar/WorkspaceSidebar.tsx | 3 - .../components/FilesTab/FilesTab.tsx | 4 +- .../v2-workspace/$workspaceId/page.tsx | 4 - 4 files changed, 27 insertions(+), 105 deletions(-) diff --git a/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts b/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts index f4ba56cd82a..4716db7f02b 100644 --- a/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts +++ b/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts @@ -40,11 +40,6 @@ interface FileTreeState { loadingDirectories: Set; } -interface FileTreeContext { - rootPath: string; - workspaceId: string; -} - interface LoadDirectoryOptions { force?: boolean; ignoreInFlight?: boolean; @@ -67,9 +62,8 @@ async function withAbortableTimeout( timeoutMs: number, abortController: AbortController, ): Promise { - let timeout: ReturnType | null = null; let timedOut = false; - timeout = setTimeout(() => { + const timeout = setTimeout(() => { timedOut = true; abortController.abort(); }, timeoutMs); @@ -82,9 +76,7 @@ async function withAbortableTimeout( } throw error; } finally { - if (timeout) { - clearTimeout(timeout); - } + clearTimeout(timeout); } } @@ -121,53 +113,6 @@ function applyDirectoryEntries( }; } -function isSameFileTreeContext( - left: FileTreeContext, - right: FileTreeContext, -): boolean { - return ( - left.rootPath === right.rootPath && left.workspaceId === right.workspaceId - ); -} - -function markDirectoryLoading( - current: FileTreeState, - absolutePath: string, -): FileTreeState { - const nextLoading = new Set(current.loadingDirectories); - nextLoading.add(absolutePath); - return { - ...current, - loadingDirectories: nextLoading, - }; -} - -function clearDirectoryLoading( - current: FileTreeState, - absolutePath: string, -): FileTreeState { - const nextLoading = new Set(current.loadingDirectories); - nextLoading.delete(absolutePath); - return { - ...current, - loadingDirectories: nextLoading, - }; -} - -function shouldRetryListDirectory( - error: unknown, - nextAttempt: number, -): boolean { - return ( - error instanceof ListDirectoryTimeoutError && - nextAttempt <= LIST_DIRECTORY_MAX_ATTEMPTS - ); -} - -function getListDirectoryRetryDelayMs(attempt: number): number { - return LIST_DIRECTORY_RETRY_DELAY_MS * attempt; -} - function createInitialState(): FileTreeState { return { childPathsByDirectory: new Map(), @@ -299,9 +244,6 @@ export function useFileTree({ rootPath, }: UseFileTreeParams): UseFileTreeResult { const utils = workspaceTrpc.useUtils(); - const activeContextRef = useRef({ rootPath, workspaceId }); - activeContextRef.current = { rootPath, workspaceId }; - const isMountedRef = useRef(false); const retryTimersRef = useRef(new Set>()); const activeLoadAbortControllersRef = useRef(new Set()); const [state, setState] = useState(() => createInitialState()); @@ -323,10 +265,7 @@ export function useFileTree({ }, []); useEffect(() => { - isMountedRef.current = true; - return () => { - isMountedRef.current = false; clearRetryTimers(); abortActiveLoads(); }; @@ -348,13 +287,6 @@ export function useFileTree({ | null >(null); - const isRequestCurrent = useCallback((requestContext: FileTreeContext) => { - return ( - isMountedRef.current && - isSameFileTreeContext(activeContextRef.current, requestContext) - ); - }, []); - const loadDirectory = useCallback( async ( absolutePath: string, @@ -365,11 +297,6 @@ export function useFileTree({ return; } - const requestContext = { rootPath, workspaceId }; - if (!isRequestCurrent(requestContext)) { - return; - } - const currentState = stateRef.current; if ( !ignoreInFlight && @@ -397,10 +324,19 @@ export function useFileTree({ } } - updateState((current) => markDirectoryLoading(current, absolutePath)); + updateState((current) => ({ + ...current, + loadingDirectories: new Set(current.loadingDirectories).add( + absolutePath, + ), + })); const abortController = new AbortController(); activeLoadAbortControllersRef.current.add(abortController); + // External cancellation (unmount or workspace/root change) clears the + // active set, so membership doubles as a "still relevant" check. + const isStillCurrent = () => + activeLoadAbortControllersRef.current.has(abortController); try { const result = await withAbortableTimeout( @@ -412,32 +348,31 @@ export function useFileTree({ abortController, ); - if (!isRequestCurrent(requestContext)) { + if (!isStillCurrent()) { return; } - updateState((current) => { - return applyDirectoryEntries(current, absolutePath, result.entries); - }); + updateState((current) => + applyDirectoryEntries(current, absolutePath, result.entries), + ); } catch (error) { - if (!isRequestCurrent(requestContext)) { + if (!isStillCurrent()) { return; } const nextAttempt = attempt + 1; - if (shouldRetryListDirectory(error, nextAttempt)) { + if ( + error instanceof ListDirectoryTimeoutError && + nextAttempt <= LIST_DIRECTORY_MAX_ATTEMPTS + ) { const retryTimer = setTimeout(() => { retryTimersRef.current.delete(retryTimer); - if (!isRequestCurrent(requestContext)) { - return; - } - void loadDirectoryRef.current?.(absolutePath, { attempt: nextAttempt, force: true, ignoreInFlight: true, }); - }, getListDirectoryRetryDelayMs(attempt)); + }, LIST_DIRECTORY_RETRY_DELAY_MS * attempt); retryTimersRef.current.add(retryTimer); return; } @@ -451,19 +386,15 @@ export function useFileTree({ ); updateState((current) => { - return clearDirectoryLoading(current, absolutePath); + const nextLoading = new Set(current.loadingDirectories); + nextLoading.delete(absolutePath); + return { ...current, loadingDirectories: nextLoading }; }); } finally { activeLoadAbortControllersRef.current.delete(abortController); } }, - [ - isRequestCurrent, - rootPath, - updateState, - utils.filesystem.listDirectory, - workspaceId, - ], + [updateState, utils.filesystem.listDirectory, workspaceId], ); loadDirectoryRef.current = loadDirectory; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx index 5f9f027668d..1f73ec81700 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx @@ -29,7 +29,6 @@ interface WorkspaceSidebarProps { selectedFilePath?: string; pendingReveal?: PendingReveal | null; workspaceId: string; - workspaceName?: string; } function IconButton({ @@ -66,7 +65,6 @@ export function WorkspaceSidebar({ selectedFilePath, pendingReveal, workspaceId, - workspaceName, }: WorkspaceSidebarProps) { const collections = useCollections(); const { preferences, setRightSidebarTab } = useV2UserPreferences(); @@ -119,7 +117,6 @@ export function WorkspaceSidebar({ selectedFilePath={selectedFilePath} pendingReveal={pendingReveal} workspaceId={workspaceId} - workspaceName={workspaceName} gitStatus={gitStatus.data} /> ), diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx index 68efdfeaca6..02ad7e10d4f 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx @@ -45,7 +45,6 @@ interface FilesTabProps { isDirectory: boolean; } | null; workspaceId: string; - workspaceName?: string; gitStatus: GitStatusData | undefined; } @@ -215,7 +214,6 @@ export function FilesTab({ selectedFilePath, pendingReveal, workspaceId, - workspaceName, gitStatus, }: FilesTabProps) { const [_isRefreshing, setIsRefreshing] = useState(false); @@ -516,7 +514,7 @@ export function FilesTab({ zIndex: 20, }} > - {workspaceName ?? "Explorer"} + Explorer
diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx index cbe20ee23dc..f107e6aa122 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx @@ -105,7 +105,6 @@ function V2WorkspacePage() { Date: Tue, 28 Apr 2026 14:40:00 -0700 Subject: [PATCH 3/7] fix(desktop): only show files-tab spinner on initial load, not refetches --- .../WorkspaceSidebar/components/FilesTab/FilesTab.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx index 02ad7e10d4f..5cb23450489 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx @@ -469,7 +469,7 @@ export function FilesTab({ if (!rootPath) { return (
- {workspaceQuery.isLoading || workspaceQuery.isFetching ? ( + {workspaceQuery.isLoading ? ( <> Loading files... From 4cd618cc7bd100b99541e863937dbc1e4e17e778 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Tue, 28 Apr 2026 15:07:28 -0700 Subject: [PATCH 4/7] feat(host-service): plumb AbortSignal through listDirectory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the renderer aborts a listDirectory query (timeout or workspace switch), the host-service was running fs.readdir + per-symlink fs.stat to completion and discarding the result. Node's fs API doesn't honor AbortSignal, but we can short-circuit between operations — useful in symlink-heavy directories (node_modules) where the per-entry stat loop dominates. --- bun.lock | 2 +- .../host-service/src/trpc/router/filesystem/filesystem.ts | 4 ++-- packages/workspace-fs/src/core/service.ts | 7 ++++--- packages/workspace-fs/src/fs.ts | 8 ++++++++ packages/workspace-fs/src/host/service.ts | 3 ++- 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/bun.lock b/bun.lock index a0de8ed3144..8a15c65e4eb 100644 --- a/bun.lock +++ b/bun.lock @@ -110,7 +110,7 @@ }, "apps/desktop": { "name": "@superset/desktop", - "version": "1.6.2", + "version": "1.7.2", "dependencies": { "@ai-sdk/anthropic": "^3.0.43", "@ai-sdk/openai": "3.0.36", diff --git a/packages/host-service/src/trpc/router/filesystem/filesystem.ts b/packages/host-service/src/trpc/router/filesystem/filesystem.ts index 96b41812804..0b336245282 100644 --- a/packages/host-service/src/trpc/router/filesystem/filesystem.ts +++ b/packages/host-service/src/trpc/router/filesystem/filesystem.ts @@ -58,10 +58,10 @@ export const filesystemRouter = router({ absolutePath: z.string(), }), ) - .query(async ({ ctx, input }) => { + .query(async ({ ctx, input, signal }) => { const { workspaceId, ...serviceInput } = input; const service = getFilesystemService(ctx, workspaceId); - return await service.listDirectory(serviceInput); + return await service.listDirectory(serviceInput, { signal }); }), readFile: protectedProcedure diff --git a/packages/workspace-fs/src/core/service.ts b/packages/workspace-fs/src/core/service.ts index 787f9948d22..5f75713c735 100644 --- a/packages/workspace-fs/src/core/service.ts +++ b/packages/workspace-fs/src/core/service.ts @@ -9,9 +9,10 @@ import type { } from "../types"; export interface FsService { - listDirectory(input: { - absolutePath: string; - }): Promise<{ entries: FsEntry[] }>; + listDirectory( + input: { absolutePath: string }, + options?: { signal?: AbortSignal }, + ): Promise<{ entries: FsEntry[] }>; readFile(input: { absolutePath: string; diff --git a/packages/workspace-fs/src/fs.ts b/packages/workspace-fs/src/fs.ts index a72073b2c67..c479d309b91 100644 --- a/packages/workspace-fs/src/fs.ts +++ b/packages/workspace-fs/src/fs.ts @@ -371,15 +371,23 @@ async function writeAtomically({ export async function listDirectory({ rootPath, absolutePath, + signal, }: { rootPath: string; absolutePath: string; + signal?: AbortSignal; }): Promise { const targetPath = ensureWithinRoot({ rootPath, absolutePath }); + signal?.throwIfAborted(); + // Node's fs.readdir/fs.stat ignore AbortSignal, so we can only interrupt + // between operations — useful for symlink-heavy dirs where the per-entry + // stat loop dominates the cost. const entries = await fs.readdir(targetPath, { withFileTypes: true }); + signal?.throwIfAborted(); const mapped = await Promise.all( entries.map(async (entry) => { + signal?.throwIfAborted(); let kind = direntToKind(entry); // Resolve symlinks to determine target type (e.g. symlinked dirs in node_modules) if (kind === "symlink") { diff --git a/packages/workspace-fs/src/host/service.ts b/packages/workspace-fs/src/host/service.ts index 973be99bc72..b96ee33feca 100644 --- a/packages/workspace-fs/src/host/service.ts +++ b/packages/workspace-fs/src/host/service.ts @@ -136,10 +136,11 @@ export function createFsHostService( const { rootPath } = options; return { - async listDirectory(input) { + async listDirectory(input, options) { const entries = await listDirectory({ rootPath, absolutePath: input.absolutePath, + signal: options?.signal, }); return { entries }; }, From 12fa12a7d68bf5fbf287b20dac2ee94c05293eeb Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Tue, 28 Apr 2026 15:23:56 -0700 Subject: [PATCH 5/7] refactor(host-service): batch listDirectory stat calls for cancellable abort Process per-entry symlink stats in batches of 16 with a signal check between batches. With Promise.all-over-everything, all stats kick off in the same microtask and an in-flight abort can't interrupt any of them. Batching bounds the zombie work to one batch (~16 stats) per abort. --- packages/workspace-fs/src/fs.ts | 57 ++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/packages/workspace-fs/src/fs.ts b/packages/workspace-fs/src/fs.ts index c479d309b91..4f71767e5ae 100644 --- a/packages/workspace-fs/src/fs.ts +++ b/packages/workspace-fs/src/fs.ts @@ -368,6 +368,11 @@ async function writeAtomically({ } } +// Symlink-resolution batch size. Node's fs.readdir and fs.stat ignore +// AbortSignal, so we can only check it between operations — batching the +// per-entry stat calls bounds how much zombie work continues after an abort. +const LIST_DIRECTORY_STAT_BATCH_SIZE = 16; + export async function listDirectory({ rootPath, absolutePath, @@ -379,33 +384,35 @@ export async function listDirectory({ }): Promise { const targetPath = ensureWithinRoot({ rootPath, absolutePath }); signal?.throwIfAborted(); - // Node's fs.readdir/fs.stat ignore AbortSignal, so we can only interrupt - // between operations — useful for symlink-heavy dirs where the per-entry - // stat loop dominates the cost. const entries = await fs.readdir(targetPath, { withFileTypes: true }); - signal?.throwIfAborted(); - const mapped = await Promise.all( - entries.map(async (entry) => { - signal?.throwIfAborted(); - let kind = direntToKind(entry); - // Resolve symlinks to determine target type (e.g. symlinked dirs in node_modules) - if (kind === "symlink") { - try { - const stats = await fs.stat(path.join(targetPath, entry.name)); - if (stats.isDirectory()) kind = "directory"; - else if (stats.isFile()) kind = "file"; - } catch { - // Dangling symlink or permission error — keep as "symlink" - } - } - return { - absolutePath: path.join(targetPath, entry.name), - name: entry.name, - kind, - }; - }), - ); + const mapped: FsEntry[] = []; + for (let i = 0; i < entries.length; i += LIST_DIRECTORY_STAT_BATCH_SIZE) { + signal?.throwIfAborted(); + const batch = await Promise.all( + entries + .slice(i, i + LIST_DIRECTORY_STAT_BATCH_SIZE) + .map(async (entry) => { + let kind = direntToKind(entry); + // Resolve symlinks to determine target type (e.g. symlinked dirs in node_modules) + if (kind === "symlink") { + try { + const stats = await fs.stat(path.join(targetPath, entry.name)); + if (stats.isDirectory()) kind = "directory"; + else if (stats.isFile()) kind = "file"; + } catch { + // Dangling symlink or permission error — keep as "symlink" + } + } + return { + absolutePath: path.join(targetPath, entry.name), + name: entry.name, + kind, + }; + }), + ); + mapped.push(...batch); + } return mapped.sort((left, right) => { const leftIsDir = left.kind === "directory"; From f76ad1a39f567d9cc93a2a4ac925839627937aaf Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 29 Apr 2026 12:37:02 -0700 Subject: [PATCH 6/7] feat(host-service): tRPC query timeout middleware + retry on TIMEOUT Hung host-service IPC (slow git, slow filesystem ops) was leaving the renderer spinning indefinitely. Replace the bespoke per-hook retry/timeout in useFileTree with a single server-side middleware that bounds every query procedure. Server (queryProcedure builder + middleware): - t.middleware races next() against a per-procedure timeout, rejecting with a TRPCError({ code: "TIMEOUT" }). Default 5s, override via `.meta({ timeoutMs })` on procedures that legitimately take longer (search, listCommits, getDiff, getStatus, getBranchSyncStatus, getPullRequestThreads, readFile). - Switch all query procedures in filesystem and git routers to queryProcedure; mutations remain on protectedProcedure. Client (workspace-client QueryClient): - defaultOptions.queries.retry retries TIMEOUT errors up to 2 times with linear backoff (300ms, 600ms). Other errors keep the previous single retry. useFileTree: - Drop withAbortableTimeout, ListDirectoryTimeoutError, retry-timer set, retry recursion, loadDirectoryRef, activeLoadAbortControllersRef. - Just await utils.filesystem.listDirectory.fetch(input). React Query's retry policy handles TIMEOUT; abortOnUnmount cancels in-flight on unmount/workspace switch. ~120 lines removed. Side benefit: git.getStatus / listBranches / listCommits etc. all gain hung-IPC protection automatically. The Changes tab no longer spins forever on a slow `git status`. --- .../host-service/useFileTree/useFileTree.ts | 150 ++---------------- packages/host-service/src/trpc/index.ts | 118 ++++++++++---- .../src/trpc/router/filesystem/filesystem.ts | 15 +- .../host-service/src/trpc/router/git/git.ts | 26 +-- .../WorkspaceClientProvider.tsx | 21 ++- 5 files changed, 137 insertions(+), 193 deletions(-) diff --git a/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts b/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts index 4716db7f02b..9032188bf20 100644 --- a/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts +++ b/apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts @@ -42,42 +42,6 @@ interface FileTreeState { interface LoadDirectoryOptions { force?: boolean; - ignoreInFlight?: boolean; - attempt?: number; -} - -const LIST_DIRECTORY_TIMEOUT_MS = 5_000; -const LIST_DIRECTORY_MAX_ATTEMPTS = 3; -const LIST_DIRECTORY_RETRY_DELAY_MS = 300; - -class ListDirectoryTimeoutError extends Error { - constructor(timeoutMs: number) { - super(`listDirectory timed out after ${timeoutMs}ms`); - this.name = "ListDirectoryTimeoutError"; - } -} - -async function withAbortableTimeout( - fetcher: (signal: AbortSignal) => Promise, - timeoutMs: number, - abortController: AbortController, -): Promise { - let timedOut = false; - const timeout = setTimeout(() => { - timedOut = true; - abortController.abort(); - }, timeoutMs); - - try { - return await fetcher(abortController.signal); - } catch (error) { - if (timedOut) { - throw new ListDirectoryTimeoutError(timeoutMs); - } - throw error; - } finally { - clearTimeout(timeout); - } } function applyDirectoryEntries( @@ -244,33 +208,10 @@ export function useFileTree({ rootPath, }: UseFileTreeParams): UseFileTreeResult { const utils = workspaceTrpc.useUtils(); - const retryTimersRef = useRef(new Set>()); - const activeLoadAbortControllersRef = useRef(new Set()); const [state, setState] = useState(() => createInitialState()); const stateRef = useRef(state); stateRef.current = state; - const clearRetryTimers = useCallback(() => { - for (const timer of retryTimersRef.current) { - clearTimeout(timer); - } - retryTimersRef.current.clear(); - }, []); - - const abortActiveLoads = useCallback(() => { - for (const abortController of activeLoadAbortControllersRef.current) { - abortController.abort(); - } - activeLoadAbortControllersRef.current.clear(); - }, []); - - useEffect(() => { - return () => { - clearRetryTimers(); - abortActiveLoads(); - }; - }, [abortActiveLoads, clearRetryTimers]); - const updateState = useCallback( (updater: (current: FileTreeState) => FileTreeState) => { setState((current) => { @@ -282,29 +223,16 @@ export function useFileTree({ [], ); - const loadDirectoryRef = useRef< - | ((absolutePath: string, options?: LoadDirectoryOptions) => Promise) - | null - >(null); - const loadDirectory = useCallback( async ( absolutePath: string, options: LoadDirectoryOptions = {}, ): Promise => { - const { force = false, ignoreInFlight = false, attempt = 1 } = options; - if (!workspaceId || !absolutePath) { - return; - } + const { force = false } = options; + if (!workspaceId || !absolutePath) return; const currentState = stateRef.current; - if ( - !ignoreInFlight && - currentState.loadingDirectories.has(absolutePath) - ) { - return; - } - + if (currentState.loadingDirectories.has(absolutePath)) return; if ( !force && currentState.loadedDirectories.has(absolutePath) && @@ -319,9 +247,7 @@ export function useFileTree({ updateState((current) => applyDirectoryEntries(current, absolutePath, cachedResult.entries), ); - if (!force) { - return; - } + if (!force) return; } updateState((current) => ({ @@ -331,72 +257,27 @@ export function useFileTree({ ), })); - const abortController = new AbortController(); - activeLoadAbortControllersRef.current.add(abortController); - // External cancellation (unmount or workspace/root change) clears the - // active set, so membership doubles as a "still relevant" check. - const isStillCurrent = () => - activeLoadAbortControllersRef.current.has(abortController); - try { - const result = await withAbortableTimeout( - (signal) => - utils.filesystem.listDirectory.fetch(input, { - trpc: { signal }, - }), - LIST_DIRECTORY_TIMEOUT_MS, - abortController, - ); - - if (!isStillCurrent()) { - return; - } - + // Server-side timeout + React Query's TIMEOUT-aware retry handle + // hung host-service IPC; we just await the fetch and apply results. + const result = await utils.filesystem.listDirectory.fetch(input); updateState((current) => applyDirectoryEntries(current, absolutePath, result.entries), ); } catch (error) { - if (!isStillCurrent()) { - return; - } - - const nextAttempt = attempt + 1; - if ( - error instanceof ListDirectoryTimeoutError && - nextAttempt <= LIST_DIRECTORY_MAX_ATTEMPTS - ) { - const retryTimer = setTimeout(() => { - retryTimersRef.current.delete(retryTimer); - void loadDirectoryRef.current?.(absolutePath, { - attempt: nextAttempt, - force: true, - ignoreInFlight: true, - }); - }, LIST_DIRECTORY_RETRY_DELAY_MS * attempt); - retryTimersRef.current.add(retryTimer); - return; - } - console.error( "[workspace-client/useFileTree] Failed to load directory:", - { - absolutePath, - error, - }, + { absolutePath, error }, ); - updateState((current) => { const nextLoading = new Set(current.loadingDirectories); nextLoading.delete(absolutePath); return { ...current, loadingDirectories: nextLoading }; }); - } finally { - activeLoadAbortControllersRef.current.delete(abortController); } }, [updateState, utils.filesystem.listDirectory, workspaceId], ); - loadDirectoryRef.current = loadDirectory; const refreshPath = useCallback( async (absolutePath: string): Promise => { @@ -474,21 +355,10 @@ export function useFileTree({ }, [updateState]); useEffect(() => { - clearRetryTimers(); - abortActiveLoads(); updateState(() => createInitialState()); - if (!rootPath) { - return; - } - + if (!rootPath) return; void loadDirectory(rootPath, { force: true }); - }, [ - abortActiveLoads, - clearRetryTimers, - loadDirectory, - rootPath, - updateState, - ]); + }, [loadDirectory, rootPath, updateState]); useWorkspaceEvent( "fs:events", diff --git a/packages/host-service/src/trpc/index.ts b/packages/host-service/src/trpc/index.ts index a74f14b22d7..df64976a6b0 100644 --- a/packages/host-service/src/trpc/index.ts +++ b/packages/host-service/src/trpc/index.ts @@ -8,41 +8,54 @@ import { type TeardownFailureCause, } from "./error-types"; -const t = initTRPC.context().create({ - transformer: superjson, - errorFormatter({ shape, error }) { - // tRPC wraps non-Error `cause` values via getCauseFromUnknown() into a - // synthetic UnknownCauseError that carries the original fields as own - // properties. Superjson then serializes it as an Error (message/stack - // only) and drops our fields. Re-build a plain object so the wire - // format keeps `kind`, `exitCode`, `outputTail`, etc. - const teardownFailure: TeardownFailureCause | undefined = - isTeardownFailureCause(error.cause) - ? { - kind: "TEARDOWN_FAILED", - exitCode: error.cause.exitCode, - signal: error.cause.signal, - timedOut: error.cause.timedOut, - outputTail: error.cause.outputTail, - } - : undefined; - const projectNotSetup: ProjectNotSetupCause | undefined = - isProjectNotSetupCause(error.cause) - ? { - kind: "PROJECT_NOT_SETUP", - projectId: error.cause.projectId, - } - : undefined; - return { - ...shape, - data: { - ...shape.data, - teardownFailure, - projectNotSetup, - }, - }; - }, -}); +interface RouterMeta { + /** + * Per-procedure timeout in milliseconds, applied to query procedures + * via `queryProcedure`. Defaults to 5_000 when omitted. Set higher for + * procedures that legitimately take longer (e.g. searching large + * histories or shelling out to long-running commands). + */ + timeoutMs?: number; +} + +const t = initTRPC + .context() + .meta() + .create({ + transformer: superjson, + errorFormatter({ shape, error }) { + // tRPC wraps non-Error `cause` values via getCauseFromUnknown() into a + // synthetic UnknownCauseError that carries the original fields as own + // properties. Superjson then serializes it as an Error (message/stack + // only) and drops our fields. Re-build a plain object so the wire + // format keeps `kind`, `exitCode`, `outputTail`, etc. + const teardownFailure: TeardownFailureCause | undefined = + isTeardownFailureCause(error.cause) + ? { + kind: "TEARDOWN_FAILED", + exitCode: error.cause.exitCode, + signal: error.cause.signal, + timedOut: error.cause.timedOut, + outputTail: error.cause.outputTail, + } + : undefined; + const projectNotSetup: ProjectNotSetupCause | undefined = + isProjectNotSetupCause(error.cause) + ? { + kind: "PROJECT_NOT_SETUP", + projectId: error.cause.projectId, + } + : undefined; + return { + ...shape, + data: { + ...shape.data, + teardownFailure, + projectNotSetup, + }, + }; + }, + }); export const router = t.router; export const publicProcedure = t.procedure; @@ -57,6 +70,41 @@ export const protectedProcedure = t.procedure.use(async ({ ctx, next }) => { return next({ ctx }); }); +const DEFAULT_QUERY_TIMEOUT_MS = 5_000; + +const timeoutMiddleware = t.middleware(async ({ next, type, path, meta }) => { + if (type !== "query") return next(); + const timeoutMs = meta?.timeoutMs ?? DEFAULT_QUERY_TIMEOUT_MS; + + let timer: ReturnType | undefined; + const timeoutPromise = new Promise((_, reject) => { + timer = setTimeout(() => { + reject( + new TRPCError({ + code: "TIMEOUT", + message: `${path} timed out after ${timeoutMs}ms`, + }), + ); + }, timeoutMs); + }); + + try { + return await Promise.race([next(), timeoutPromise]); + } finally { + if (timer) clearTimeout(timer); + } +}); + +/** + * Query procedures with a server-side timeout. Hung filesystem/git work + * rejects after `meta.timeoutMs` (default 5s) so the renderer doesn't + * spin forever. React Query is configured to retry on `TIMEOUT` errors. + * + * Use this for `.query` procedures only — mutations (writes, deletes, + * etc.) have variable latency and shouldn't share a blanket budget. + */ +export const queryProcedure = protectedProcedure.use(timeoutMiddleware); + export type { ProjectNotSetupCause, TeardownFailureCause, diff --git a/packages/host-service/src/trpc/router/filesystem/filesystem.ts b/packages/host-service/src/trpc/router/filesystem/filesystem.ts index 0b336245282..ab1df1ad405 100644 --- a/packages/host-service/src/trpc/router/filesystem/filesystem.ts +++ b/packages/host-service/src/trpc/router/filesystem/filesystem.ts @@ -3,7 +3,7 @@ import { isAbsolute, join, normalize, resolve } from "node:path"; import { TRPCError } from "@trpc/server"; import { z } from "zod"; import type { HostServiceContext } from "../../../types"; -import { protectedProcedure, router } from "../../index"; +import { protectedProcedure, queryProcedure, router } from "../../index"; function getFilesystemService(ctx: HostServiceContext, workspaceId: string) { try { @@ -51,7 +51,7 @@ const writeFileContentSchema = z.union([ ]); export const filesystemRouter = router({ - listDirectory: protectedProcedure + listDirectory: queryProcedure .input( z.object({ workspaceId: z.string(), @@ -64,7 +64,8 @@ export const filesystemRouter = router({ return await service.listDirectory(serviceInput, { signal }); }), - readFile: protectedProcedure + readFile: queryProcedure + .meta({ timeoutMs: 30_000 }) .input( z.object({ workspaceId: z.string(), @@ -89,7 +90,7 @@ export const filesystemRouter = router({ return result; }), - getMetadata: protectedProcedure + getMetadata: queryProcedure .input( z.object({ workspaceId: z.string(), @@ -248,7 +249,8 @@ export const filesystemRouter = router({ return await service.copyPath(serviceInput); }), - searchFiles: protectedProcedure + searchFiles: queryProcedure + .meta({ timeoutMs: 30_000 }) .input( z .object({ @@ -285,7 +287,8 @@ export const filesystemRouter = router({ }); }), - searchContent: protectedProcedure + searchContent: queryProcedure + .meta({ timeoutMs: 60_000 }) .input( z.object({ workspaceId: z.string(), diff --git a/packages/host-service/src/trpc/router/git/git.ts b/packages/host-service/src/trpc/router/git/git.ts index 40402ef3d1d..325b61d183b 100644 --- a/packages/host-service/src/trpc/router/git/git.ts +++ b/packages/host-service/src/trpc/router/git/git.ts @@ -3,7 +3,7 @@ import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; import { z } from "zod"; import { projects, pullRequests, workspaces } from "../../../db/schema"; -import { protectedProcedure, router } from "../../index"; +import { protectedProcedure, queryProcedure, router } from "../../index"; import type { ChangedFile, CheckConclusionState, @@ -34,7 +34,7 @@ import { import { resolveWorktreePath } from "./utils/resolve-worktree"; export const gitRouter = router({ - listBranches: protectedProcedure + listBranches: queryProcedure .input(z.object({ workspaceId: z.string() })) .query(async ({ ctx, input }) => { const worktreePath = resolveWorktreePath(ctx, input.workspaceId); @@ -64,7 +64,8 @@ export const gitRouter = router({ return { branches }; }), - getStatus: protectedProcedure + getStatus: queryProcedure + .meta({ timeoutMs: 15_000 }) .input( z.object({ workspaceId: z.string(), @@ -216,7 +217,8 @@ export const gitRouter = router({ }; }), - listCommits: protectedProcedure + listCommits: queryProcedure + .meta({ timeoutMs: 30_000 }) .input( z.object({ workspaceId: z.string(), @@ -253,7 +255,8 @@ export const gitRouter = router({ return { commits }; }), - getCommitFiles: protectedProcedure + getCommitFiles: queryProcedure + .meta({ timeoutMs: 15_000 }) .input( z.object({ workspaceId: z.string(), @@ -271,7 +274,7 @@ export const gitRouter = router({ return { files }; }), - getBaseBranch: protectedProcedure + getBaseBranch: queryProcedure .input(z.object({ workspaceId: z.string() })) .query(async ({ ctx, input }) => { const worktreePath = resolveWorktreePath(ctx, input.workspaceId); @@ -358,7 +361,8 @@ export const gitRouter = router({ return { name: input.newName }; }), - getDiff: protectedProcedure + getDiff: queryProcedure + .meta({ timeoutMs: 30_000 }) .input( z.object({ workspaceId: z.string(), @@ -436,7 +440,8 @@ export const gitRouter = router({ }; }), - getBranchSyncStatus: protectedProcedure + getBranchSyncStatus: queryProcedure + .meta({ timeoutMs: 30_000 }) .input(z.object({ workspaceId: z.string() })) .query(async ({ ctx, input }) => { const worktreePath = resolveWorktreePath(ctx, input.workspaceId); @@ -503,7 +508,7 @@ export const gitRouter = router({ }; }), - getPullRequest: protectedProcedure + getPullRequest: queryProcedure .input(z.object({ workspaceId: z.string() })) .query(({ ctx, input }) => { const workspace = ctx.db.query.workspaces @@ -562,7 +567,8 @@ export const gitRouter = router({ }; }), - getPullRequestThreads: protectedProcedure + getPullRequestThreads: queryProcedure + .meta({ timeoutMs: 30_000 }) .input(z.object({ workspaceId: z.string() })) .query(async ({ ctx, input }) => { const workspace = ctx.db.query.workspaces diff --git a/packages/workspace-client/src/providers/WorkspaceClientProvider/WorkspaceClientProvider.tsx b/packages/workspace-client/src/providers/WorkspaceClientProvider/WorkspaceClientProvider.tsx index 1ad9dfb12c4..3e359b6a871 100644 --- a/packages/workspace-client/src/providers/WorkspaceClientProvider/WorkspaceClientProvider.tsx +++ b/packages/workspace-client/src/providers/WorkspaceClientProvider/WorkspaceClientProvider.tsx @@ -1,11 +1,17 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { httpBatchStreamLink } from "@trpc/client"; +import { httpBatchStreamLink, TRPCClientError } from "@trpc/client"; import { createContext, type ReactNode, useContext } from "react"; import superjson from "superjson"; import { workspaceTrpc } from "../../workspace-trpc"; const STALE_TIME_MS = 5_000; const GC_TIME_MS = 30 * 60 * 1_000; +const MAX_TIMEOUT_RETRIES = 2; +const TIMEOUT_RETRY_BASE_DELAY_MS = 300; + +function isTimeoutError(error: unknown): boolean { + return error instanceof TRPCClientError && error.data?.code === "TIMEOUT"; +} export interface WorkspaceClientContextValue { hostUrl: string; @@ -49,7 +55,18 @@ function getWorkspaceClients( defaultOptions: { queries: { refetchOnWindowFocus: false, - retry: 1, + // Retry server-side TIMEOUT errors a couple of times — these come + // from `queryProcedure`'s middleware when a host-service query + // (filesystem, git) takes longer than its budget. Other errors + // fall back to a single retry as before. + retry: (failureCount, error) => { + if (isTimeoutError(error)) return failureCount < MAX_TIMEOUT_RETRIES; + return failureCount < 1; + }, + retryDelay: (attempt, error) => + isTimeoutError(error) + ? TIMEOUT_RETRY_BASE_DELAY_MS * (attempt + 1) + : Math.min(1000 * 2 ** attempt, 30_000), staleTime: STALE_TIME_MS, gcTime: GC_TIME_MS, }, From eff085f876c04af8e9644a59c809eeb436248cc1 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 29 Apr 2026 12:51:46 -0700 Subject: [PATCH 7/7] docs(host-service): add QUERY_TIMEOUTS.md reference Documents the queryProcedure / timeoutMiddleware pattern: where it lives, how to set per-procedure budgets via .meta({ timeoutMs }), the current budget table, and what timeouts do (and don't) interrupt. --- packages/host-service/QUERY_TIMEOUTS.md | 77 +++++++++++++++++++++++++ packages/host-service/src/trpc/index.ts | 7 ++- 2 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 packages/host-service/QUERY_TIMEOUTS.md diff --git a/packages/host-service/QUERY_TIMEOUTS.md b/packages/host-service/QUERY_TIMEOUTS.md new file mode 100644 index 00000000000..0bf1c1f81e9 --- /dev/null +++ b/packages/host-service/QUERY_TIMEOUTS.md @@ -0,0 +1,77 @@ +# Query Timeouts + +Every host-service query procedure has a server-side timeout. If a query +takes longer than its budget, the procedure rejects with +`TRPCError({ code: "TIMEOUT" })`. The renderer's `QueryClient` retries +those errors a couple of times with linear backoff, so transient slow +ops self-recover without leaving the UI spinning forever. + +## Where it lives + +- **Server middleware**: `src/trpc/index.ts` — `timeoutMiddleware` races + `next()` against a per-procedure timer. +- **Builder**: `queryProcedure = protectedProcedure.use(timeoutMiddleware)` + in the same file. Use this for every `.query` procedure. +- **Per-procedure overrides**: `.meta({ timeoutMs })` on the procedure + builder. Without `meta`, the default is 5000 ms. +- **Client retry**: `WorkspaceClientProvider` in `packages/workspace-client` + retries `TIMEOUT` errors up to 2 times with `300 ms × attempt` backoff. + Other errors keep the previous single retry. + +Mutations stay on `protectedProcedure` because their latency is highly +variable (file writes, network calls) and a blanket budget would do more +harm than good. + +## Adding a new query + +```ts +// Defaults to 5s — fine for most local fs/git work. +myFastQuery: queryProcedure + .input(...) + .query(async ({ ctx, input }) => { ... }), + +// Override when the work legitimately takes longer. +mySlowQuery: queryProcedure + .meta({ timeoutMs: 30_000 }) + .input(...) + .query(async ({ ctx, input }) => { ... }), +``` + +Pick the smallest budget that fits the slowest legitimate run on real +hardware. Too generous and the UX of a hung host-service degrades; too +tight and healthy queries time out under load. + +## Current budgets + +| Procedure | Budget | Reason | +|---|---|---| +| `filesystem.listDirectory`, `filesystem.getMetadata` | 5s | Fast in practice | +| `filesystem.readFile` | 30s | Large files (e.g. lockfiles, generated bundles) | +| `filesystem.searchFiles` | 30s | ripgrep on large repos | +| `filesystem.searchContent` | 60s | content search worst case | +| `git.listBranches`, `git.getBaseBranch`, `git.getPullRequest` | 5s | Cheap reads | +| `git.getStatus`, `git.getCommitFiles` | 15s | Slow on big working trees | +| `git.listCommits`, `git.getDiff`, `git.getBranchSyncStatus`, `git.getPullRequestThreads` | 30s | Long history, big diffs, GitHub API | + +## What the timeout does *not* do + +The middleware only races a timer against the procedure's `next()` +result. It does **not** kill the underlying work — `fs.readdir`, `git` +child processes, etc. continue server-side until they finish naturally. +For ops that *can* be cancelled, the procedure should plumb the +`AbortSignal` through. `filesystem.listDirectory` does this: +the renderer's tRPC client provides `signal` automatically (and +`abortOnUnmount: true` aborts on unmount), the procedure forwards it +to the `FsService`, and `workspace-fs/fs.ts::listDirectory` checks +`signal?.throwIfAborted()` between `fs.readdir` and each batch of stat +calls. Node's `fs.readdir`/`fs.stat` themselves ignore `AbortSignal`, +so the readdir syscall is uncancellable; we can only short-circuit +between operations. + +## What a timeout looks like to the client + +`TRPCClientError` with `error.data.code === "TIMEOUT"`. The +`WorkspaceClientProvider` retry predicate keys on this. Bespoke per-hook +retry logic should not be necessary — if it is, the procedure's budget +is probably wrong, or the underlying work isn't really a single query +and should be split. diff --git a/packages/host-service/src/trpc/index.ts b/packages/host-service/src/trpc/index.ts index df64976a6b0..dd09c811239 100644 --- a/packages/host-service/src/trpc/index.ts +++ b/packages/host-service/src/trpc/index.ts @@ -100,8 +100,11 @@ const timeoutMiddleware = t.middleware(async ({ next, type, path, meta }) => { * rejects after `meta.timeoutMs` (default 5s) so the renderer doesn't * spin forever. React Query is configured to retry on `TIMEOUT` errors. * - * Use this for `.query` procedures only — mutations (writes, deletes, - * etc.) have variable latency and shouldn't share a blanket budget. + * Use this for `.query` procedures only — mutations have variable + * latency and shouldn't share a blanket budget. + * + * See `packages/host-service/QUERY_TIMEOUTS.md` for the policy and + * current per-procedure budgets. */ export const queryProcedure = protectedProcedure.use(timeoutMiddleware);