From 2f071a1ba3a587e8816b55b296f48918b26fe352 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 22 Feb 2026 00:58:00 -0800 Subject: [PATCH] Fix deleting worktree crash and improve UX (#798) * Changes from fix/deleting-worktree * fix: Improve worktree deletion safety and branch cleanup logic * fix: Improve error handling and async operations across auto-mode and worktree services * Update apps/server/src/routes/auto-mode/routes/analyze-project.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .../auto-mode/routes/analyze-project.ts | 9 +- .../src/routes/worktree/routes/delete.ts | 72 +++++++- apps/server/src/services/auto-mode/facade.ts | 27 ++- .../src/services/auto-mode/global-service.ts | 2 +- apps/ui/src/components/views/board-view.tsx | 163 ++++++++++-------- .../dialogs/delete-worktree-dialog.tsx | 12 +- .../worktree-panel/hooks/use-worktrees.ts | 18 +- apps/ui/src/hooks/use-cursor-status-init.ts | 15 +- apps/ui/src/hooks/use-provider-auth-init.ts | 27 +-- apps/ui/src/hooks/use-settings-migration.ts | 10 +- apps/ui/src/hooks/use-settings-sync.ts | 17 +- apps/ui/src/lib/settings-utils.ts | 27 +++ apps/ui/src/store/app-store.ts | 7 + apps/ui/src/store/types/state-types.ts | 2 + libs/types/src/feature.ts | 2 +- 15 files changed, 300 insertions(+), 110 deletions(-) create mode 100644 apps/ui/src/lib/settings-utils.ts diff --git a/apps/server/src/routes/auto-mode/routes/analyze-project.ts b/apps/server/src/routes/auto-mode/routes/analyze-project.ts index 9ba22c50..cae70c36 100644 --- a/apps/server/src/routes/auto-mode/routes/analyze-project.ts +++ b/apps/server/src/routes/auto-mode/routes/analyze-project.ts @@ -19,10 +19,11 @@ export function createAnalyzeProjectHandler(autoModeService: AutoModeServiceComp return; } - // Start analysis in background - autoModeService.analyzeProject(projectPath).catch((error) => { - logger.error(`[AutoMode] Project analysis error:`, error); - }); + // Kick off analysis in the background; attach a rejection handler so + // unhandled-promise warnings don't surface and errors are at least logged. + // Synchronous throws (e.g. "not implemented") still propagate here. + const analysisPromise = autoModeService.analyzeProject(projectPath); + analysisPromise.catch((err) => logError(err, 'Background analyzeProject failed')); res.json({ success: true, message: 'Project analysis started' }); } catch (error) { diff --git a/apps/server/src/routes/worktree/routes/delete.ts b/apps/server/src/routes/worktree/routes/delete.ts index 06703ff1..fcb42f59 100644 --- a/apps/server/src/routes/worktree/routes/delete.ts +++ b/apps/server/src/routes/worktree/routes/delete.ts @@ -5,6 +5,7 @@ import type { Request, Response } from 'express'; import { exec } from 'child_process'; import { promisify } from 'util'; +import fs from 'fs/promises'; import { isGitRepo } from '@automaker/git-utils'; import { getErrorMessage, logError, isValidBranchName } from '../common.js'; import { execGitCommand } from '../../../lib/git.js'; @@ -46,20 +47,79 @@ export function createDeleteHandler() { }); branchName = stdout.trim(); } catch { - // Could not get branch name + // Could not get branch name - worktree directory may already be gone + logger.debug('Could not determine branch for worktree, directory may be missing'); } // Remove the worktree (using array arguments to prevent injection) + let removeSucceeded = false; try { await execGitCommand(['worktree', 'remove', worktreePath, '--force'], projectPath); - } catch { - // Try with prune if remove fails - await execGitCommand(['worktree', 'prune'], projectPath); + removeSucceeded = true; + } catch (removeError) { + // `git worktree remove` can fail if the directory is already missing + // or in a bad state. Try pruning stale worktree entries as a fallback. + logger.debug('git worktree remove failed, trying prune', { + error: getErrorMessage(removeError), + }); + try { + await execGitCommand(['worktree', 'prune'], projectPath); + + // Verify the specific worktree is no longer registered after prune. + // `git worktree prune` exits 0 even if worktreePath was never registered, + // so we must explicitly check the worktree list to avoid false positives. + const { stdout: listOut } = await execAsync('git worktree list --porcelain', { + cwd: projectPath, + }); + // Parse porcelain output and check for an exact path match. + // Using substring .includes() can produce false positives when one + // worktree path is a prefix of another (e.g. /foo vs /foobar). + const stillRegistered = listOut + .split('\n') + .filter((line) => line.startsWith('worktree ')) + .map((line) => line.slice('worktree '.length).trim()) + .some((registeredPath) => registeredPath === worktreePath); + if (stillRegistered) { + // Prune didn't clean up our entry - treat as failure + throw removeError; + } + removeSucceeded = true; + } catch (pruneError) { + // If pruneError is the original removeError re-thrown, propagate it + if (pruneError === removeError) { + throw removeError; + } + logger.warn('git worktree prune also failed', { + error: getErrorMessage(pruneError), + }); + // If both remove and prune fail, still try to return success + // if the worktree directory no longer exists (it may have been + // manually deleted already). + let dirExists = false; + try { + await fs.access(worktreePath); + dirExists = true; + } catch { + // Directory doesn't exist + } + if (dirExists) { + // Directory still exists - this is a real failure + throw removeError; + } + // Directory is gone, treat as success + removeSucceeded = true; + } } - // Optionally delete the branch + // Optionally delete the branch (only if worktree was successfully removed) let branchDeleted = false; - if (deleteBranch && branchName && branchName !== 'main' && branchName !== 'master') { + if ( + removeSucceeded && + deleteBranch && + branchName && + branchName !== 'main' && + branchName !== 'master' + ) { // Validate branch name to prevent command injection if (!isValidBranchName(branchName)) { logger.warn(`Invalid branch name detected, skipping deletion: ${branchName}`); diff --git a/apps/server/src/services/auto-mode/facade.ts b/apps/server/src/services/auto-mode/facade.ts index c00dfdb8..150be157 100644 --- a/apps/server/src/services/auto-mode/facade.ts +++ b/apps/server/src/services/auto-mode/facade.ts @@ -910,7 +910,7 @@ export class AutoModeServiceFacade { if (feature) { title = feature.title; description = feature.description; - branchName = feature.branchName; + branchName = feature.branchName ?? undefined; } } catch { // Silently ignore @@ -1140,10 +1140,31 @@ export class AutoModeServiceFacade { // =========================================================================== /** - * Save execution state for recovery + * Save execution state for recovery. + * + * Uses the active auto-loop config for each worktree so that the persisted + * state reflects the real branch and maxConcurrency values rather than the + * hard-coded fallbacks (null / DEFAULT_MAX_CONCURRENCY). */ private async saveExecutionState(): Promise { - return this.saveExecutionStateForProject(null, DEFAULT_MAX_CONCURRENCY); + const projectWorktrees = this.autoLoopCoordinator + .getActiveWorktrees() + .filter((w) => w.projectPath === this.projectPath); + + if (projectWorktrees.length === 0) { + // No active auto loops — save with defaults as a best-effort fallback. + return this.saveExecutionStateForProject(null, DEFAULT_MAX_CONCURRENCY); + } + + // Save state for every active worktree using its real config values. + for (const { branchName } of projectWorktrees) { + const config = this.autoLoopCoordinator.getAutoLoopConfigForProject( + this.projectPath, + branchName + ); + const maxConcurrency = config?.maxConcurrency ?? DEFAULT_MAX_CONCURRENCY; + await this.saveExecutionStateForProject(branchName, maxConcurrency); + } } /** diff --git a/apps/server/src/services/auto-mode/global-service.ts b/apps/server/src/services/auto-mode/global-service.ts index 459562eb..478f48b3 100644 --- a/apps/server/src/services/auto-mode/global-service.ts +++ b/apps/server/src/services/auto-mode/global-service.ts @@ -159,7 +159,7 @@ export class GlobalAutoModeService { if (feature) { title = feature.title; description = feature.description; - branchName = feature.branchName; + branchName = feature.branchName ?? undefined; } } catch { // Silently ignore diff --git a/apps/ui/src/components/views/board-view.tsx b/apps/ui/src/components/views/board-view.tsx index e57e0201..5d3a33cc 100644 --- a/apps/ui/src/components/views/board-view.tsx +++ b/apps/ui/src/components/views/board-view.tsx @@ -99,6 +99,7 @@ import { useQueryClient } from '@tanstack/react-query'; import { queryKeys } from '@/lib/query-keys'; import { useAutoModeQueryInvalidation } from '@/hooks/use-query-invalidation'; import { useUpdateGlobalSettings } from '@/hooks/mutations/use-settings-mutations'; +import { forceSyncSettingsToServer } from '@/hooks/use-settings-sync'; // Stable empty array to avoid infinite loop in selector const EMPTY_WORKTREES: ReturnType['getWorktrees']> = []; @@ -114,6 +115,7 @@ export function BoardView() { pendingPlanApproval, setPendingPlanApproval, updateFeature, + batchUpdateFeatures, getCurrentWorktree, setCurrentWorktree, getWorktrees, @@ -132,6 +134,7 @@ export function BoardView() { pendingPlanApproval: state.pendingPlanApproval, setPendingPlanApproval: state.setPendingPlanApproval, updateFeature: state.updateFeature, + batchUpdateFeatures: state.batchUpdateFeatures, getCurrentWorktree: state.getCurrentWorktree, setCurrentWorktree: state.setCurrentWorktree, getWorktrees: state.getWorktrees, @@ -411,25 +414,34 @@ export function BoardView() { currentProject, }); + // Shared helper: batch-reset branch assignment and persist for each affected feature. + // Used when worktrees are deleted or branches are removed during merge. + const batchResetBranchFeatures = useCallback( + (branchName: string) => { + const affectedIds = hookFeatures.filter((f) => f.branchName === branchName).map((f) => f.id); + if (affectedIds.length === 0) return; + const updates: Partial = { branchName: null }; + batchUpdateFeatures(affectedIds, updates); + for (const id of affectedIds) { + persistFeatureUpdate(id, updates).catch((err: unknown) => { + console.error( + `[batchResetBranchFeatures] Failed to persist update for feature ${id}:`, + err + ); + }); + } + }, + [hookFeatures, batchUpdateFeatures, persistFeatureUpdate] + ); + // Memoize the removed worktrees handler to prevent infinite loops const handleRemovedWorktrees = useCallback( (removedWorktrees: Array<{ path: string; branch: string }>) => { - // Reset features that were assigned to the removed worktrees (by branch) - hookFeatures.forEach((feature) => { - const matchesRemovedWorktree = removedWorktrees.some((removed) => { - // Match by branch name since worktreePath is no longer stored - return feature.branchName === removed.branch; - }); - - if (matchesRemovedWorktree) { - // Reset the feature's branch assignment - update both local state and persist - const updates = { branchName: null as unknown as string | undefined }; - updateFeature(feature.id, updates); - persistFeatureUpdate(feature.id, updates); - } - }); + for (const { branch } of removedWorktrees) { + batchResetBranchFeatures(branch); + } }, - [hookFeatures, updateFeature, persistFeatureUpdate] + [batchResetBranchFeatures] ); // Get current worktree info (path) for filtering features @@ -437,28 +449,6 @@ export function BoardView() { const currentWorktreeInfo = currentProject ? getCurrentWorktree(currentProject.path) : null; const currentWorktreePath = currentWorktreeInfo?.path ?? null; - // Track the previous worktree path to detect worktree switches - const prevWorktreePathRef = useRef(undefined); - - // When the active worktree changes, invalidate feature queries to ensure - // feature cards (especially their todo lists / planSpec tasks) render fresh data. - // Without this, cards that unmount when filtered out and remount when the user - // switches back may show stale or missing todo list data until the next polling cycle. - useEffect(() => { - // Skip the initial mount (prevWorktreePathRef starts as undefined) - if (prevWorktreePathRef.current === undefined) { - prevWorktreePathRef.current = currentWorktreePath; - return; - } - // Only invalidate when the worktree actually changed - if (prevWorktreePathRef.current !== currentWorktreePath && currentProject?.path) { - queryClient.invalidateQueries({ - queryKey: queryKeys.features.all(currentProject.path), - }); - } - prevWorktreePathRef.current = currentWorktreePath; - }, [currentWorktreePath, currentProject?.path, queryClient]); - // Select worktrees for the current project directly from the store. // Using a project-scoped selector prevents re-renders when OTHER projects' // worktrees change (the old selector subscribed to the entire worktreesByProject @@ -1603,17 +1593,7 @@ export function BoardView() { onStashPopConflict={handleStashPopConflict} onStashApplyConflict={handleStashApplyConflict} onBranchDeletedDuringMerge={(branchName) => { - // Reset features that were assigned to the deleted branch (same logic as onDeleted in DeleteWorktreeDialog) - hookFeatures.forEach((feature) => { - if (feature.branchName === branchName) { - // Reset the feature's branch assignment - update both local state and persist - const updates = { - branchName: null as unknown as string | undefined, - }; - updateFeature(feature.id, updates); - persistFeatureUpdate(feature.id, updates); - } - }); + batchResetBranchFeatures(branchName); setWorktreeRefreshKey((k) => k + 1); }} onRemovedWorktrees={handleRemovedWorktrees} @@ -1990,31 +1970,76 @@ export function BoardView() { } defaultDeleteBranch={getDefaultDeleteBranch(currentProject.path)} onDeleted={(deletedWorktree, _deletedBranch) => { - // If the deleted worktree was currently selected, immediately reset to main - // to prevent the UI from trying to render a non-existent worktree view - if ( - currentWorktreePath !== null && - pathsEqual(currentWorktreePath, deletedWorktree.path) - ) { - const mainBranch = worktrees.find((w) => w.isMain)?.branch || 'main'; - setCurrentWorktree(currentProject.path, null, mainBranch); - } + // 1. Reset current worktree to main FIRST. This must happen + // BEFORE removing from the list to ensure downstream hooks + // (useAutoMode, useBoardFeatures) see a valid worktree and + // never try to render the deleted worktree. + const mainBranch = worktrees.find((w) => w.isMain)?.branch || 'main'; + setCurrentWorktree(currentProject.path, null, mainBranch); - // Reset features that were assigned to the deleted worktree (by branch) - hookFeatures.forEach((feature) => { - // Match by branch name since worktreePath is no longer stored - if (feature.branchName === deletedWorktree.branch) { - // Reset the feature's branch assignment - update both local state and persist - const updates = { - branchName: null as unknown as string | undefined, + // 2. Immediately remove the deleted worktree from the store's + // worktree list so the UI never renders a stale tab/dropdown + // item that can be clicked and cause a crash. + const remainingWorktrees = worktrees.filter( + (w) => !pathsEqual(w.path, deletedWorktree.path) + ); + setWorktrees(currentProject.path, remainingWorktrees); + + // 3. Cancel any in-flight worktree queries, then optimistically + // update the React Query cache so the worktree disappears + // from the dropdown immediately. Cancelling first prevents a + // pending refetch from overwriting our optimistic update with + // stale server data. + const worktreeQueryKey = queryKeys.worktrees.all(currentProject.path); + void queryClient.cancelQueries({ queryKey: worktreeQueryKey }); + queryClient.setQueryData( + worktreeQueryKey, + ( + old: + | { + worktrees: WorktreeInfo[]; + removedWorktrees: Array<{ path: string; branch: string }>; + } + | undefined + ) => { + if (!old) return old; + return { + ...old, + worktrees: old.worktrees.filter( + (w: WorktreeInfo) => !pathsEqual(w.path, deletedWorktree.path) + ), }; - updateFeature(feature.id, updates); - persistFeatureUpdate(feature.id, updates); + } + ); + + // 4. Batch-reset features assigned to the deleted worktree in one + // store mutation to avoid N individual updateFeature calls that + // cascade into React error #185. + batchResetBranchFeatures(deletedWorktree.branch); + + // 5. Do NOT trigger setWorktreeRefreshKey here. The optimistic + // cache update (step 3) already removed the worktree from + // both the Zustand store and React Query cache. Incrementing + // the refresh key would cause invalidateQueries → server + // refetch, and if the server's .worktrees/ directory scan + // finds remnants of the deleted worktree, it would re-add + // it to the dropdown. The 30-second polling interval in + // WorktreePanel will eventually reconcile with the server. + setSelectedWorktreeForAction(null); + + // 6. Force-sync settings immediately so the reset worktree + // selection is persisted before any potential page reload. + // Without this, the debounced sync (1s) may not complete + // in time and the stale worktree path survives in + // server settings, causing the deleted worktree to + // reappear on next load. + forceSyncSettingsToServer().then((ok) => { + if (!ok) { + logger.warn( + 'forceSyncSettingsToServer failed after worktree deletion; stale path may reappear on reload' + ); } }); - - setWorktreeRefreshKey((k) => k + 1); - setSelectedWorktreeForAction(null); }} /> diff --git a/apps/ui/src/components/views/board-view/dialogs/delete-worktree-dialog.tsx b/apps/ui/src/components/views/board-view/dialogs/delete-worktree-dialog.tsx index e366b03e..050e2dba 100644 --- a/apps/ui/src/components/views/board-view/dialogs/delete-worktree-dialog.tsx +++ b/apps/ui/src/components/views/board-view/dialogs/delete-worktree-dialog.tsx @@ -72,9 +72,19 @@ export function DeleteWorktreeDialog({ ? `Branch "${worktree.branch}" was also deleted` : `Branch "${worktree.branch}" was kept`, }); - onDeleted(worktree, deleteBranch); + // Close the dialog first, then notify the parent. + // This ensures the dialog unmounts before the parent + // triggers potentially heavy state updates (feature branch + // resets, worktree refresh), reducing concurrent re-renders + // that can cascade into React error #185. onOpenChange(false); setDeleteBranch(false); + try { + onDeleted(worktree, deleteBranch); + } catch (error) { + // Prevent errors in onDeleted from propagating to the error boundary + console.error('onDeleted callback failed:', error); + } } else { toast.error('Failed to delete worktree', { description: result.error, diff --git a/apps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktrees.ts b/apps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktrees.ts index 829c3ce6..1a1c7cee 100644 --- a/apps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktrees.ts +++ b/apps/ui/src/components/views/board-view/worktree-panel/hooks/use-worktrees.ts @@ -111,13 +111,17 @@ export function useWorktrees({ setCurrentWorktree(projectPath, worktree.isMain ? null : worktree.path, worktree.branch); - // Invalidate feature queries when switching worktrees to ensure fresh data. - // Without this, feature cards that remount after the worktree switch may have stale - // or missing planSpec/task data, causing todo lists to appear empty until the next - // polling cycle or user interaction triggers a re-render. - queryClient.invalidateQueries({ - queryKey: queryKeys.features.all(projectPath), - }); + // Defer feature query invalidation so the store update and client-side + // re-filtering happen in the current render cycle first. The features + // list is the same regardless of worktree (filtering is client-side), + // so the board updates instantly. The deferred invalidation ensures + // feature card details (planSpec, todo lists) are refreshed in the + // background without blocking the worktree switch. + setTimeout(() => { + queryClient.invalidateQueries({ + queryKey: queryKeys.features.all(projectPath), + }); + }, 0); }, [projectPath, setCurrentWorktree, queryClient, currentWorktreePath] ); diff --git a/apps/ui/src/hooks/use-cursor-status-init.ts b/apps/ui/src/hooks/use-cursor-status-init.ts index 79e277c7..a1100f22 100644 --- a/apps/ui/src/hooks/use-cursor-status-init.ts +++ b/apps/ui/src/hooks/use-cursor-status-init.ts @@ -8,12 +8,21 @@ import { getHttpApiClient } from '@/lib/http-api-client'; * before the user opens feature dialogs. */ export function useCursorStatusInit() { - const { setCursorCliStatus, cursorCliStatus } = useSetupStore(); + // Use individual selectors instead of bare useSetupStore() to prevent + // re-rendering on every setup store mutation during initialization. + const setCursorCliStatus = useSetupStore((s) => s.setCursorCliStatus); const initialized = useRef(false); useEffect(() => { // Only initialize once per session - if (initialized.current || cursorCliStatus !== null) { + if (initialized.current) { + return; + } + // Check current status at call time rather than via dependency to avoid + // re-renders when other setup store fields change during initialization. + const currentStatus = useSetupStore.getState().cursorCliStatus; + if (currentStatus !== null) { + initialized.current = true; return; } initialized.current = true; @@ -42,5 +51,5 @@ export function useCursorStatusInit() { }; initCursorStatus(); - }, [setCursorCliStatus, cursorCliStatus]); + }, [setCursorCliStatus]); } diff --git a/apps/ui/src/hooks/use-provider-auth-init.ts b/apps/ui/src/hooks/use-provider-auth-init.ts index 6d2470cf..04b8a743 100644 --- a/apps/ui/src/hooks/use-provider-auth-init.ts +++ b/apps/ui/src/hooks/use-provider-auth-init.ts @@ -17,17 +17,16 @@ const logger = createLogger('ProviderAuthInit'); * without needing to visit the settings page first. */ export function useProviderAuthInit() { - const { - setClaudeAuthStatus, - setCodexAuthStatus, - setZaiAuthStatus, - setGeminiCliStatus, - setGeminiAuthStatus, - claudeAuthStatus, - codexAuthStatus, - zaiAuthStatus, - geminiAuthStatus, - } = useSetupStore(); + // IMPORTANT: Use individual selectors instead of bare useSetupStore() to prevent + // re-rendering on every setup store mutation. The bare call subscribes to the ENTIRE + // store, which during initialization causes cascading re-renders as multiple status + // setters fire in rapid succession. With enough rapid mutations, React hits the + // maximum update depth limit (error #185). + const setClaudeAuthStatus = useSetupStore((s) => s.setClaudeAuthStatus); + const setCodexAuthStatus = useSetupStore((s) => s.setCodexAuthStatus); + const setZaiAuthStatus = useSetupStore((s) => s.setZaiAuthStatus); + const setGeminiCliStatus = useSetupStore((s) => s.setGeminiCliStatus); + const setGeminiAuthStatus = useSetupStore((s) => s.setGeminiAuthStatus); const initialized = useRef(false); const refreshStatuses = useCallback(async () => { @@ -219,5 +218,9 @@ export function useProviderAuthInit() { // Always call refreshStatuses() to background re-validate on app restart, // even when statuses are pre-populated from persisted storage (cache case). void refreshStatuses(); - }, [refreshStatuses, claudeAuthStatus, codexAuthStatus, zaiAuthStatus, geminiAuthStatus]); + // Only depend on the callback ref. The status values were previously included + // but they are outputs of refreshStatuses(), not inputs — including them caused + // cascading re-renders during initialization that triggered React error #185 + // (maximum update depth exceeded) on first run. + }, [refreshStatuses]); } diff --git a/apps/ui/src/hooks/use-settings-migration.ts b/apps/ui/src/hooks/use-settings-migration.ts index a69afbe1..b878c9e7 100644 --- a/apps/ui/src/hooks/use-settings-migration.ts +++ b/apps/ui/src/hooks/use-settings-migration.ts @@ -26,6 +26,7 @@ import { useEffect, useState, useRef } from 'react'; import { createLogger } from '@automaker/utils/logger'; import { getHttpApiClient, waitForApiKeyInit } from '@/lib/http-api-client'; import { getItem, setItem } from '@/lib/storage'; +import { sanitizeWorktreeByProject } from '@/lib/settings-utils'; import { useAppStore, THEME_STORAGE_KEY } from '@/store/app-store'; import { useSetupStore } from '@/store/setup-store'; import { @@ -794,7 +795,14 @@ export function hydrateStoreFromSettings(settings: GlobalSettings): void { projectHistory: settings.projectHistory ?? [], projectHistoryIndex: settings.projectHistoryIndex ?? -1, lastSelectedSessionByProject: settings.lastSelectedSessionByProject ?? {}, - currentWorktreeByProject: settings.currentWorktreeByProject ?? {}, + // Sanitize currentWorktreeByProject: only restore entries where path is null + // (main branch). Non-null paths point to worktree directories that may have + // been deleted while the app was closed. Restoring a stale path causes the + // board to render an invalid worktree selection, triggering a crash loop + // (error boundary reloads → restores same bad path → crash again). + // The use-worktrees validation effect will re-discover valid worktrees + // from the server once they load. + currentWorktreeByProject: sanitizeWorktreeByProject(settings.currentWorktreeByProject), // UI State worktreePanelCollapsed: settings.worktreePanelCollapsed ?? false, lastProjectDir: settings.lastProjectDir ?? '', diff --git a/apps/ui/src/hooks/use-settings-sync.ts b/apps/ui/src/hooks/use-settings-sync.ts index 9f2e3b4b..302b694d 100644 --- a/apps/ui/src/hooks/use-settings-sync.ts +++ b/apps/ui/src/hooks/use-settings-sync.ts @@ -19,6 +19,7 @@ import { useAppStore, type ThemeMode, THEME_STORAGE_KEY } from '@/store/app-stor import { useSetupStore } from '@/store/setup-store'; import { useAuthStore } from '@/store/auth-store'; import { waitForMigrationComplete, resetMigrationState } from './use-settings-migration'; +import { sanitizeWorktreeByProject } from '@/lib/settings-utils'; import { DEFAULT_OPENCODE_MODEL, DEFAULT_GEMINI_MODEL, @@ -584,6 +585,15 @@ export async function forceSyncSettingsToServer(): Promise { updates[field] = setupState[field as keyof typeof setupState]; } + // Update localStorage cache immediately so a page reload before the + // server response arrives still sees the latest state (e.g. after + // deleting a worktree, the stale worktree path won't survive in cache). + try { + setItem('automaker-settings-cache', JSON.stringify(updates)); + } catch (storageError) { + logger.warn('Failed to update localStorage cache during force sync:', storageError); + } + const result = await api.settings.updateGlobal(updates); return result.success; } catch (error) { @@ -796,8 +806,11 @@ export async function refreshSettingsFromServer(): Promise { projectHistory: serverSettings.projectHistory, projectHistoryIndex: serverSettings.projectHistoryIndex, lastSelectedSessionByProject: serverSettings.lastSelectedSessionByProject, - currentWorktreeByProject: - serverSettings.currentWorktreeByProject ?? currentAppState.currentWorktreeByProject, + // Sanitize: only restore entries with path === null (main branch). + // Non-null paths may reference deleted worktrees, causing crash loops. + currentWorktreeByProject: sanitizeWorktreeByProject( + serverSettings.currentWorktreeByProject ?? currentAppState.currentWorktreeByProject + ), // UI State (previously in localStorage) worktreePanelCollapsed: serverSettings.worktreePanelCollapsed ?? false, lastProjectDir: serverSettings.lastProjectDir ?? '', diff --git a/apps/ui/src/lib/settings-utils.ts b/apps/ui/src/lib/settings-utils.ts new file mode 100644 index 00000000..96fab28c --- /dev/null +++ b/apps/ui/src/lib/settings-utils.ts @@ -0,0 +1,27 @@ +/** + * Shared settings utility functions + */ + +/** + * Drop currentWorktreeByProject entries with non-null paths. + * Non-null paths reference worktree directories that may have been deleted, + * and restoring them causes crash loops (board renders invalid worktree + * -> error boundary reloads -> restores same stale path). + */ +export function sanitizeWorktreeByProject( + raw: Record | undefined +): Record { + if (!raw) return {}; + const sanitized: Record = {}; + for (const [projectPath, worktree] of Object.entries(raw)) { + if ( + typeof worktree === 'object' && + worktree !== null && + 'path' in worktree && + worktree.path === null + ) { + sanitized[projectPath] = worktree; + } + } + return sanitized; +} diff --git a/apps/ui/src/store/app-store.ts b/apps/ui/src/store/app-store.ts index 96e60422..ced82af4 100644 --- a/apps/ui/src/store/app-store.ts +++ b/apps/ui/src/store/app-store.ts @@ -878,6 +878,13 @@ export const useAppStore = create()((set, get) => ({ set((state) => ({ features: state.features.map((f) => (f.id === id ? { ...f, ...updates } : f)), })), + batchUpdateFeatures: (ids, updates) => { + if (ids.length === 0) return; + const idSet = new Set(ids); + set((state) => ({ + features: state.features.map((f) => (idSet.has(f.id) ? { ...f, ...updates } : f)), + })); + }, addFeature: (feature) => { const id = feature.id ?? `feature-${Date.now()}-${Math.random().toString(36).slice(2)}`; const newFeature = { ...feature, id } as Feature; diff --git a/apps/ui/src/store/types/state-types.ts b/apps/ui/src/store/types/state-types.ts index df8d600d..b729db92 100644 --- a/apps/ui/src/store/types/state-types.ts +++ b/apps/ui/src/store/types/state-types.ts @@ -440,6 +440,8 @@ export interface AppActions { // Feature actions setFeatures: (features: Feature[]) => void; updateFeature: (id: string, updates: Partial) => void; + /** Apply the same updates to multiple features in a single store mutation. */ + batchUpdateFeatures: (ids: string[], updates: Partial) => void; addFeature: (feature: Omit & Partial>) => Feature; removeFeature: (id: string) => void; moveFeature: (id: string, newStatus: Feature['status']) => void; diff --git a/libs/types/src/feature.ts b/libs/types/src/feature.ts index a053345b..2b983cfe 100644 --- a/libs/types/src/feature.ts +++ b/libs/types/src/feature.ts @@ -91,7 +91,7 @@ export interface Feature { imagePaths?: Array; textFilePaths?: FeatureTextFilePath[]; // Branch info - worktree path is derived at runtime from branchName - branchName?: string; // Name of the feature branch (undefined = use current worktree) + branchName?: string | null; // Name of the feature branch (undefined/null = use current worktree) skipTests?: boolean; excludedPipelineSteps?: string[]; // Array of pipeline step IDs to skip for this feature thinkingLevel?: ThinkingLevel;