Skip to content

Commit b7810b7

Browse files
shkmshkm
committed
fix: resolve race condition and git staging detection in changes sidebar
- Fix race condition in useGitStatus hook where file change events could be missed. The event listener is now registered BEFORE starting the watcher to ensure no events are lost during setup. - Watch git directory (non-recursive) in addition to worktree directory to detect staging/unstaging changes. Previously, `git add` operations weren't detected because only file content changes were watched, not git index changes. - Update tests to wait for async operations to complete before asserting.
1 parent b63b0cb commit b7810b7

File tree

3 files changed

+79
-43
lines changed

3 files changed

+79
-43
lines changed

‎src-tauri/src/watcher.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,18 @@ pub fn watch_worktree(app: AppHandle, worktree_id: String, worktree_path: String
8585
return;
8686
}
8787

88+
// Also watch the git index file to detect staging/unstaging changes.
89+
// For regular repos, .git is a directory; for worktrees, .git is a file
90+
// pointing to the actual git directory (e.g., .git/worktrees/<name>).
91+
// The index file is in the git directory.
92+
if let Some(git_dir) = resolve_git_dir(path) {
93+
// Watch the git directory (non-recursive) to catch index changes
94+
if let Err(e) = watcher.watch(&git_dir, RecursiveMode::NonRecursive) {
95+
// Non-fatal: we can still watch file changes even if we can't watch the index
96+
eprintln!("[Watcher] Failed to watch git dir {:?}: {}", git_dir, e);
97+
}
98+
}
99+
88100
// Trailing-edge debounce: wait until no events for this duration
89101
let debounce_duration = Duration::from_millis(500);
90102
let mut pending_update = false;

‎src/hooks/useGitStatus.test.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -422,11 +422,16 @@ describe('useGitStatus', () => {
422422
useGitStatus({ id: 'worktree-1', path: '/not/a/git/repo' })
423423
);
424424

425+
// Wait for get_changed_files to be called (async setup must complete first)
425426
await waitFor(() => {
426-
expect(result.current.loading).toBe(false);
427+
expect(invokeHistory.some((h) => h.command === 'get_changed_files')).toBe(true);
428+
});
429+
430+
// Now wait for isGitRepo to be updated after error handling
431+
await waitFor(() => {
432+
expect(result.current.isGitRepo).toBe(false);
427433
});
428434

429-
expect(result.current.isGitRepo).toBe(false);
430435
expect(result.current.files).toEqual([]);
431436
consoleSpy.mockRestore();
432437
});
@@ -443,11 +448,16 @@ describe('useGitStatus', () => {
443448
useGitStatus({ id: 'worktree-1', path: '/not/a/git/repo' })
444449
);
445450

451+
// Wait for get_changed_files to be called (async setup must complete first)
446452
await waitFor(() => {
447-
expect(result.current.loading).toBe(false);
453+
expect(invokeHistory.some((h) => h.command === 'get_changed_files')).toBe(true);
454+
});
455+
456+
// Now wait for isGitRepo to be updated after error handling
457+
await waitFor(() => {
458+
expect(result.current.isGitRepo).toBe(false);
448459
});
449460

450-
expect(result.current.isGitRepo).toBe(false);
451461
consoleSpy.mockRestore();
452462
});
453463

@@ -463,7 +473,9 @@ describe('useGitStatus', () => {
463473
useGitStatus({ id: 'worktree-1', path: '/path/to/worktree' })
464474
);
465475

476+
// Wait for get_changed_files to be called and loading to complete
466477
await waitFor(() => {
478+
expect(invokeHistory.some((h) => h.command === 'get_changed_files')).toBe(true);
467479
expect(result.current.loading).toBe(false);
468480
});
469481

‎src/hooks/useGitStatus.ts‎

Lines changed: 51 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ export function useGitStatus(
9898
}, [worktree, projectPath]);
9999

100100
// Initial load and start watcher
101+
// IMPORTANT: We register the event listener BEFORE starting the watcher to avoid
102+
// a race condition where events could be emitted before the listener is ready.
101103
useEffect(() => {
102104
if (!worktree) {
103105
setFiles([]);
@@ -107,55 +109,65 @@ export function useGitStatus(
107109

108110
// Reset isGitRepo when target changes
109111
setIsGitRepo(true);
110-
refresh();
111112

112-
// Start watching if not already watching this worktree (only for uncommitted mode)
113-
if (mode === 'uncommitted' && watchingRef.current !== worktree.id) {
114-
// Stop previous watcher if any
115-
if (watchingRef.current) {
116-
invoke('stop_watching', { worktreeId: watchingRef.current }).catch(() => {});
117-
}
118-
watchingRef.current = worktree.id;
119-
invoke('start_watching', {
120-
worktreeId: worktree.id,
121-
worktreePath: worktree.path,
122-
}).catch((err) => console.error('Failed to start watching:', err));
123-
} else if (mode === 'branch' && watchingRef.current) {
124-
// Stop watching when in branch mode
125-
invoke('stop_watching', { worktreeId: watchingRef.current }).catch(() => {});
126-
watchingRef.current = null;
127-
}
113+
let cancelled = false;
114+
let unlistenFn: UnlistenFn | null = null;
128115

129-
// Cleanup on unmount
130-
return () => {
131-
if (watchingRef.current) {
132-
invoke('stop_watching', { worktreeId: watchingRef.current }).catch(() => {});
116+
const setup = async () => {
117+
// In uncommitted mode, register listener FIRST, then start watcher
118+
if (mode === 'uncommitted') {
119+
// Register the event listener before starting the watcher
120+
unlistenFn = await listen<FilesChanged>('files-changed', (event) => {
121+
// Only update if this is for our worktree and effect hasn't been cancelled
122+
if (!cancelled && event.payload.worktree_path === worktree.path) {
123+
setFiles(event.payload.files);
124+
}
125+
});
126+
127+
// If cancelled during listener setup, clean up immediately
128+
if (cancelled) {
129+
unlistenFn();
130+
return;
131+
}
132+
133+
// Now start the watcher (listener is already ready to receive events)
134+
if (watchingRef.current !== worktree.id) {
135+
// Stop previous watcher if any
136+
if (watchingRef.current) {
137+
await invoke('stop_watching', { worktreeId: watchingRef.current }).catch(() => {});
138+
}
139+
watchingRef.current = worktree.id;
140+
await invoke('start_watching', {
141+
worktreeId: worktree.id,
142+
worktreePath: worktree.path,
143+
}).catch((err) => console.error('Failed to start watching:', err));
144+
}
145+
} else if (mode === 'branch' && watchingRef.current) {
146+
// Stop watching when in branch mode
147+
await invoke('stop_watching', { worktreeId: watchingRef.current }).catch(() => {});
133148
watchingRef.current = null;
134149
}
135-
};
136-
}, [worktree, refresh, mode]);
137-
138-
// Listen for file change events (only in uncommitted mode)
139-
useEffect(() => {
140-
if (!worktree || mode !== 'uncommitted') return;
141150

142-
let unlisten: UnlistenFn | null = null;
143-
144-
listen<FilesChanged>('files-changed', (event) => {
145-
// Only update if this is for our worktree
146-
if (event.payload.worktree_path === worktree.path) {
147-
setFiles(event.payload.files);
151+
// Fetch initial data after listener is set up
152+
if (!cancelled) {
153+
refresh();
148154
}
149-
}).then((fn) => {
150-
unlisten = fn;
151-
});
155+
};
156+
157+
setup();
152158

159+
// Cleanup on unmount or deps change
153160
return () => {
154-
if (unlisten) {
155-
unlisten();
161+
cancelled = true;
162+
if (unlistenFn) {
163+
unlistenFn();
164+
}
165+
if (watchingRef.current) {
166+
invoke('stop_watching', { worktreeId: watchingRef.current }).catch(() => {});
167+
watchingRef.current = null;
156168
}
157169
};
158-
}, [worktree, mode]);
170+
}, [worktree, refresh, mode]);
159171

160172
return {
161173
files,

0 commit comments

Comments
 (0)