From 2fd9a56793bae2e1bfdd71fc3386052337092136 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 6 Feb 2026 15:07:42 +1100 Subject: [PATCH 1/4] fix: Explicitly capture branch properties in view diff callbacks When viewing diffs from the branch list, the branch properties are now explicitly captured using @const blocks in the Svelte template. This prevents potential closure issues where the wrong branch's information might be used if the branches array is mutated during user interaction. Changes: - Extract branch properties (id, projectId, worktreePath, baseBranch, branchName) as const values in template - Inline the view diff handlers directly in template with explicit captures - Add console logging to track which branch is being referenced - Include repo path in error messages for better debugging This ensures that clicking "View Diff" on a branch always uses the correct branch's worktree path and branch names, even if other branches are being deleted or modified concurrently. Co-Authored-By: Claude Sonnet 4.5 --- src/App.svelte | 6 ++++ src/lib/BranchHome.svelte | 54 ++++++++++++++++++------------ src/lib/stores/diffState.svelte.ts | 8 ++++- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/App.svelte b/src/App.svelte index b86b5cb..2b7b9e1 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -234,6 +234,12 @@ spec: DiffSpecType, label: string ) { + console.log('[App] handleViewDiffFromBranches called with:', { + projectId, + repoPath, + spec, + label, + }); syncGlobalToTab(); openRepo(repoPath); diff --git a/src/lib/BranchHome.svelte b/src/lib/BranchHome.svelte index 2c4c964..eeb3e29 100644 --- a/src/lib/BranchHome.svelte +++ b/src/lib/BranchHome.svelte @@ -281,24 +281,6 @@ deleteErrors = newErrors; } - function handleViewDiff(branch: Branch) { - onViewDiff?.( - branch.projectId, - branch.worktreePath, - DiffSpec.mergeBaseDiff(branch.baseBranch, branch.branchName), - `${branch.baseBranch}..${branch.branchName}` - ); - } - - function handleViewCommitDiff(branch: Branch, commitSha: string) { - onViewDiff?.( - branch.projectId, - branch.worktreePath, - DiffSpec.fromRevs(`${commitSha}~1`, commitSha), - commitSha.slice(0, 7) - ); - } - function handleNewProjectCreated(project: GitProject) { projects = [...projects, project]; showNewProjectModal = false; @@ -443,12 +425,42 @@ {:else} + {@const branchId = branch.id} + {@const projectId = branch.projectId} + {@const worktreePath = branch.worktreePath} + {@const baseBranch = branch.baseBranch} + {@const branchName = branch.branchName} handleViewDiff(branch)} - onViewCommitDiff={(sha) => handleViewCommitDiff(branch, sha)} - onDelete={() => handleDeleteBranch(branch.id)} + onViewDiff={() => { + console.log('[BranchHome] handleViewDiff called for:', { + branchId, + branchName, + worktreePath, + projectId, + }); + onViewDiff?.( + projectId, + worktreePath, + DiffSpec.mergeBaseDiff(baseBranch, branchName), + `${baseBranch}..${branchName}` + ); + }} + onViewCommitDiff={(sha) => { + console.log('[BranchHome] handleViewCommitDiff called for:', { + branchId, + branchName, + sha, + }); + onViewDiff?.( + projectId, + worktreePath, + DiffSpec.fromRevs(`${sha}~1`, sha), + sha.slice(0, 7) + ); + }} + onDelete={() => handleDeleteBranch(branchId)} /> {/if} {/each} diff --git a/src/lib/stores/diffState.svelte.ts b/src/lib/stores/diffState.svelte.ts index 605ebc4..944291f 100644 --- a/src/lib/stores/diffState.svelte.ts +++ b/src/lib/stores/diffState.svelte.ts @@ -141,6 +141,7 @@ export function getCachedDiff(path: string): FileDiff | null { * Clears the diff cache since we're loading a new spec. */ export async function loadFiles(spec: DiffSpec, repoPath?: string): Promise { + console.log('[diffState] loadFiles called with:', { spec, repoPath }); diffState.loading = true; diffState.error = null; diffState.currentSpec = spec; @@ -155,7 +156,12 @@ export async function loadFiles(spec: DiffSpec, repoPath?: string): Promise Date: Fri, 6 Feb 2026 15:20:32 +1100 Subject: [PATCH 2/4] chore: Remove debug logging statements Remove console.log and console.error statements that were added for debugging purposes. Co-Authored-By: Claude Sonnet 4.5 --- src/App.svelte | 6 ------ src/lib/BranchHome.svelte | 11 ----------- src/lib/stores/diffState.svelte.ts | 8 +------- 3 files changed, 1 insertion(+), 24 deletions(-) diff --git a/src/App.svelte b/src/App.svelte index 2b7b9e1..b86b5cb 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -234,12 +234,6 @@ spec: DiffSpecType, label: string ) { - console.log('[App] handleViewDiffFromBranches called with:', { - projectId, - repoPath, - spec, - label, - }); syncGlobalToTab(); openRepo(repoPath); diff --git a/src/lib/BranchHome.svelte b/src/lib/BranchHome.svelte index eeb3e29..d1aea4e 100644 --- a/src/lib/BranchHome.svelte +++ b/src/lib/BranchHome.svelte @@ -434,12 +434,6 @@ {branch} {refreshKey} onViewDiff={() => { - console.log('[BranchHome] handleViewDiff called for:', { - branchId, - branchName, - worktreePath, - projectId, - }); onViewDiff?.( projectId, worktreePath, @@ -448,11 +442,6 @@ ); }} onViewCommitDiff={(sha) => { - console.log('[BranchHome] handleViewCommitDiff called for:', { - branchId, - branchName, - sha, - }); onViewDiff?.( projectId, worktreePath, diff --git a/src/lib/stores/diffState.svelte.ts b/src/lib/stores/diffState.svelte.ts index 944291f..605ebc4 100644 --- a/src/lib/stores/diffState.svelte.ts +++ b/src/lib/stores/diffState.svelte.ts @@ -141,7 +141,6 @@ export function getCachedDiff(path: string): FileDiff | null { * Clears the diff cache since we're loading a new spec. */ export async function loadFiles(spec: DiffSpec, repoPath?: string): Promise { - console.log('[diffState] loadFiles called with:', { spec, repoPath }); diffState.loading = true; diffState.error = null; diffState.currentSpec = spec; @@ -156,12 +155,7 @@ export async function loadFiles(spec: DiffSpec, repoPath?: string): Promise Date: Fri, 6 Feb 2026 15:35:49 +1100 Subject: [PATCH 3/4] fix: Filter out stale worktree tabs on app startup When tabs are restored from persistent storage, validate that worktree paths still exist before recreating the tabs. This prevents errors when trying to view diffs from branches that were deleted outside the app. The fix checks each worktree path during tab restoration and: - Skips tabs with non-existent worktree paths - Logs a warning about skipped tabs - Adjusts the active tab index if needed - Saves the cleaned-up tabs back to storage Co-Authored-By: Claude Sonnet 4.5 --- src/lib/stores/tabState.svelte.ts | 84 +++++++++++++++++++++++++------ 1 file changed, 70 insertions(+), 14 deletions(-) diff --git a/src/lib/stores/tabState.svelte.ts b/src/lib/stores/tabState.svelte.ts index da38084..f710707 100644 --- a/src/lib/stores/tabState.svelte.ts +++ b/src/lib/stores/tabState.svelte.ts @@ -235,22 +235,78 @@ export async function loadTabsFromStorage( const data = await loadWindowTabsFromStore(windowState.windowLabel); if (data) { + // Filter out tabs with non-existent worktree paths to prevent errors + // when branches have been deleted outside the app + const validTabs = await Promise.all( + data.tabs.map(async (t) => { + // Check if the repo path exists (use Tauri's fs API or simple check) + try { + // For now, we'll use a simple heuristic: filter out obvious worktree paths + // that don't exist. Non-worktree paths (like main repo) should be kept. + const isWorktreePath = t.repoPath.includes('/.staged/worktrees/'); + if (isWorktreePath) { + // For worktree paths, verify they exist before restoring the tab + const exists = await pathExists(t.repoPath); + if (!exists) { + console.warn(`[TabState] Skipping tab "${t.repoName}" with non-existent worktree path: ${t.repoPath}`); + return null; + } + } + return t; + } catch (e) { + console.warn(`[TabState] Error checking tab path "${t.repoPath}":`, e); + return t; // Keep tab if we can't check (fail open) + } + }) + ); + // Create tabs with isolated state instances // Plain objects are created - the parent windowState.tabs array is already reactive - windowState.tabs = data.tabs.map((t) => ({ - id: t.id || t.projectId, // Fallback for old format - projectId: t.projectId || t.id, // Fallback for old format - repoPath: t.repoPath, - repoName: t.repoName, - subpath: t.subpath || null, - diffState: createDiffState(), - commentsState: createCommentsState(), - diffSelection: createDiffSelection(), - agentState: createAgentState(), - referenceFilesState: createReferenceFilesState(), - needsRefresh: false, - })); - windowState.activeTabIndex = data.activeTabIndex; + windowState.tabs = validTabs + .filter((t) => t !== null) + .map((t) => ({ + id: t.id || t.projectId, // Fallback for old format + projectId: t.projectId || t.id, // Fallback for old format + repoPath: t.repoPath, + repoName: t.repoName, + subpath: t.subpath || null, + diffState: createDiffState(), + commentsState: createCommentsState(), + diffSelection: createDiffSelection(), + agentState: createAgentState(), + referenceFilesState: createReferenceFilesState(), + needsRefresh: false, + })); + + // Adjust active tab index if the active tab was filtered out + if (windowState.tabs.length > 0 && data.activeTabIndex >= windowState.tabs.length) { + windowState.activeTabIndex = 0; + } else if (windowState.tabs.length > 0) { + windowState.activeTabIndex = Math.min(data.activeTabIndex, windowState.tabs.length - 1); + } else { + windowState.activeTabIndex = 0; + } + + // Save the cleaned-up tabs back to storage to prevent stale tabs from persisting + if (validTabs.filter((t) => t !== null).length !== data.tabs.length) { + console.log(`[TabState] Removed ${data.tabs.length - windowState.tabs.length} stale tab(s) from storage`); + saveTabsToStorage(); + } + } +} + +/** + * Check if a path exists by attempting to read it. + * Uses the existing listDirectory backend command. + */ +async function pathExists(path: string): Promise { + try { + // Try to list the directory - if it exists, this will succeed + const { invoke } = await import('@tauri-apps/api/core'); + await invoke('list_directory', { path }); + return true; + } catch { + return false; } } From 833317c93be79433dac2d8fad2415375ccad4b4b Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 6 Feb 2026 15:40:16 +1100 Subject: [PATCH 4/4] fix: Close tabs when deleting a branch from within Staged When a user deletes a branch from the branch list, any open tabs viewing that branch's worktree are now automatically closed. This prevents users from interacting with stale tabs that reference deleted worktrees. The fix works by: - Capturing the worktreePath before deletion - After successful deletion, finding all tabs with matching repoPath - Closing each matching tab using the existing closeTab() function This complements the existing app-startup cleanup that filters out tabs with non-existent worktree paths, providing immediate cleanup during the current session rather than waiting until next app restart. Co-Authored-By: Claude Sonnet 4.5 --- src/lib/BranchHome.svelte | 8 ++++++++ src/lib/stores/tabState.svelte.ts | 8 ++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lib/BranchHome.svelte b/src/lib/BranchHome.svelte index d1aea4e..40fe353 100644 --- a/src/lib/BranchHome.svelte +++ b/src/lib/BranchHome.svelte @@ -20,6 +20,7 @@ import NewProjectModal from './NewProjectModal.svelte'; import ConfirmDialog from './ConfirmDialog.svelte'; import { DiffSpec } from './types'; + import { windowState, closeTab } from './stores/tabState.svelte'; interface Props { onViewDiff?: (projectId: string, repoPath: string, spec: DiffSpec, label: string) => void; @@ -255,6 +256,7 @@ if (!branchToDelete) return; const id = branchToDelete.id; + const worktreePath = branchToDelete.worktreePath; // Close dialog and show spinner immediately branchToDelete = null; deletingBranchIds = new Set(deletingBranchIds).add(id); @@ -266,6 +268,12 @@ const newDeleting = new Set(deletingBranchIds); newDeleting.delete(id); deletingBranchIds = newDeleting; + + // Close any tabs that were viewing this branch's worktree + const tabsToClose = windowState.tabs.filter((tab) => tab.repoPath === worktreePath); + for (const tab of tabsToClose) { + closeTab(tab.id); + } } catch (e) { // Failure: remove spinner, show error card const newDeleting = new Set(deletingBranchIds); diff --git a/src/lib/stores/tabState.svelte.ts b/src/lib/stores/tabState.svelte.ts index f710707..c008b56 100644 --- a/src/lib/stores/tabState.svelte.ts +++ b/src/lib/stores/tabState.svelte.ts @@ -248,7 +248,9 @@ export async function loadTabsFromStorage( // For worktree paths, verify they exist before restoring the tab const exists = await pathExists(t.repoPath); if (!exists) { - console.warn(`[TabState] Skipping tab "${t.repoName}" with non-existent worktree path: ${t.repoPath}`); + console.warn( + `[TabState] Skipping tab "${t.repoName}" with non-existent worktree path: ${t.repoPath}` + ); return null; } } @@ -289,7 +291,9 @@ export async function loadTabsFromStorage( // Save the cleaned-up tabs back to storage to prevent stale tabs from persisting if (validTabs.filter((t) => t !== null).length !== data.tabs.length) { - console.log(`[TabState] Removed ${data.tabs.length - windowState.tabs.length} stale tab(s) from storage`); + console.log( + `[TabState] Removed ${data.tabs.length - windowState.tabs.length} stale tab(s) from storage` + ); saveTabsToStorage(); } }