fix(daemon): add per-CWD git info cache for cc statusline #208
Conversation
Previously, the daemon stored only a single activeWorkingDir and gitInfo, causing race conditions when parallel requests came from different CWDs. Now each working directory has its own cache entry with separate git info, preventing incorrect branch/dirty status from being returned. - Add GitCacheEntry struct with Info, LastAccessed, and LastFetched fields - Replace single cache with map[string]*GitCacheEntry keyed by normalized path - Background fetch iterates over all recently accessed directories - Stale entries evicted after inactivity timeout Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @AnnatarHe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical race condition in the daemon's Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses a race condition by introducing a per-CWD cache for git information, which is a solid improvement. The implementation is clean and the new tests provide good coverage for the changes. I have a couple of suggestions. First, to improve performance when multiple repositories are active, the git info fetching could be parallelized. Second, several of the new tests use hardcoded Unix-style path separators, which will cause them to fail on Windows. Making them platform-independent will improve test suite robustness. Overall, great work on fixing the race condition and improving the caching logic.
| for _, dir := range dirsToFetch { | ||
| info := GetGitInfo(dir) | ||
|
|
||
| s.mu.Lock() | ||
| if entry, exists := s.gitCache[dir]; exists { | ||
| entry.Info = info | ||
| entry.LastFetched = time.Now() | ||
| } | ||
| s.mu.Unlock() | ||
|
|
||
| slog.Debug("Git info updated", | ||
| slog.String("workingDir", dir), | ||
| slog.String("branch", info.Branch), | ||
| slog.Bool("dirty", info.Dirty)) | ||
| } |
There was a problem hiding this comment.
The current implementation fetches git info for each directory sequentially. If there are multiple active repositories and GetGitInfo is slow (as it involves disk I/O), this loop could take longer than the CCInfoFetchInterval, causing fetches to pile up. To improve performance, you could fetch the git info for each directory in parallel using goroutines and a sync.WaitGroup.
var wg sync.WaitGroup
for _, dir := range dirsToFetch {
wg.Add(1)
go func(dir string) {
defer wg.Done()
info := GetGitInfo(dir)
s.mu.Lock()
if entry, exists := s.gitCache[dir]; exists {
entry.Info = info
entry.LastFetched = time.Now()
}
s.mu.Unlock()
slog.Debug("Git info updated",
slog.String("workingDir", dir),
slog.String("branch", info.Branch),
slog.Bool("dirty", info.Dirty))
}(dir)
}
wg.Wait()| defer service.mu.RUnlock() | ||
| // Both should map to the same normalized path | ||
| assert.Len(s.T(), service.gitCache, 1) | ||
| assert.Contains(s.T(), service.gitCache, "/project/a") |
There was a problem hiding this comment.
The test uses a hardcoded path /project/a which assumes a Unix-like path separator. The implementation correctly uses filepath.Clean which will produce platform-specific paths (e.g., \project\a on Windows). This will cause the test to fail on Windows. To make the test platform-independent, you should use filepath.Clean in the assertion as well.
This issue is present in multiple new tests (TestGetCachedGitInfo_CreatesNewEntry, TestGetCachedGitInfo_MultipleCWDs, TestGetCachedGitInfo_UpdatesLastAccessed, TestGetCachedGitInfo_ReturnsCachedValue, TestCleanupStaleGitCache_RemovesOldEntries, TestConcurrentGetCachedGitInfo_DifferentDirs, TestStopTimer_ClearsGitCache). Please apply similar fixes there. You'll also need to import the path/filepath package in this test file.
| assert.Contains(s.T(), service.gitCache, "/project/a") | |
| assert.Contains(s.T(), service.gitCache, filepath.Clean("/project/a")) |
References
- For platform-independent paths, use
filepath.Jointo combine segments andos.UserHomeDir()to get the home directory, rather than hardcoding path separators or environment variables like$HOME.
Summary
cc statuslinewhen parallel requests come from different CWDsactiveWorkingDirandgitInfowas stored, causing requests to overwrite each otherChanges
GitCacheEntrystruct withInfo,LastAccessed, andLastFetchedfieldsmap[string]*GitCacheEntrykeyed by normalized pathTest plan
🤖 Generated with Claude Code