Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 66 additions & 20 deletions daemon/cc_info_timer.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package daemon
import (
"context"
"log/slog"
"path/filepath"
"sync"
"time"

Expand All @@ -21,6 +22,12 @@ type CCInfoCache struct {
FetchedAt time.Time
}

// GitCacheEntry holds cached git info for a single working directory
type GitCacheEntry struct {
Info GitInfo
LastAccessed time.Time
LastFetched time.Time
}

// CCInfoTimerService manages lazy-fetching of CC info data
type CCInfoTimerService struct {
Expand All @@ -37,9 +44,8 @@ type CCInfoTimerService struct {
stopChan chan struct{}
wg sync.WaitGroup

// Git info (single active working directory, fetched by timer)
activeWorkingDir string
gitInfo GitInfo
// Git info cache (per working directory)
gitCache map[string]*GitCacheEntry
}

// NewCCInfoTimerService creates a new CC info timer service
Expand All @@ -48,6 +54,7 @@ func NewCCInfoTimerService(config *model.ShellTimeConfig) *CCInfoTimerService {
config: config,
cache: make(map[CCInfoTimeRange]CCInfoCache),
activeRanges: make(map[CCInfoTimeRange]bool),
gitCache: make(map[string]*GitCacheEntry),
stopChan: make(chan struct{}),
}
}
Expand Down Expand Up @@ -122,11 +129,10 @@ func (s *CCInfoTimerService) stopTimer() {
s.ticker.Stop()
s.timerRunning = false

// Clear active ranges and git info when stopping
// Clear active ranges and git cache when stopping
s.mu.Lock()
s.activeRanges = make(map[CCInfoTimeRange]bool)
s.activeWorkingDir = ""
s.gitInfo = GitInfo{}
s.gitCache = make(map[string]*GitCacheEntry)
s.mu.Unlock()

slog.Info("CC info timer stopped due to inactivity")
Expand Down Expand Up @@ -272,39 +278,79 @@ func (s *CCInfoTimerService) fetchCCInfo(ctx context.Context, timeRange CCInfoTi
}, nil
}

// GetCachedGitInfo marks the working directory as active and returns cached git info.
// GetCachedGitInfo returns cached git info for the given working directory.
// It marks the directory as active for background refresh.
// Git info is fetched by the background timer, so first call may return empty.
func (s *CCInfoTimerService) GetCachedGitInfo(workingDir string) GitInfo {
if workingDir == "" {
return GitInfo{}
}

// Normalize path for consistent cache keys
normalizedDir := filepath.Clean(workingDir)

s.mu.Lock()
s.activeWorkingDir = workingDir
info := s.gitInfo
entry, exists := s.gitCache[normalizedDir]
if !exists {
// Create new entry for this directory
entry = &GitCacheEntry{
LastAccessed: time.Now(),
}
s.gitCache[normalizedDir] = entry
} else {
// Update last accessed time
entry.LastAccessed = time.Now()
}
info := entry.Info
s.mu.Unlock()

return info
}

// fetchGitInfo fetches git info for the active working directory
// fetchGitInfo fetches git info for all recently accessed working directories
func (s *CCInfoTimerService) fetchGitInfo() {
// Collect directories that need refresh (under read lock)
s.mu.RLock()
workingDir := s.activeWorkingDir
dirsToFetch := make([]string, 0, len(s.gitCache))
for dir, entry := range s.gitCache {
// Only fetch for recently accessed entries
if time.Since(entry.LastAccessed) <= CCInfoInactivityTimeout {
dirsToFetch = append(dirsToFetch, dir)
}
}
s.mu.RUnlock()

if workingDir == "" {
return
// Fetch git info for each directory (outside lock to avoid blocking)
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))
}
Comment on lines +324 to 338
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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()


info := GetGitInfo(workingDir)
// Cleanup stale entries
s.cleanupStaleGitCache()
}

// cleanupStaleGitCache removes entries not accessed within the inactivity timeout
func (s *CCInfoTimerService) cleanupStaleGitCache() {
s.mu.Lock()
s.gitInfo = info
s.mu.Unlock()
defer s.mu.Unlock()

slog.Debug("Git info updated",
slog.String("workingDir", workingDir),
slog.String("branch", info.Branch),
slog.Bool("dirty", info.Dirty))
for dir, entry := range s.gitCache {
if time.Since(entry.LastAccessed) > CCInfoInactivityTimeout {
delete(s.gitCache, dir)
slog.Debug("Git cache entry evicted",
slog.String("workingDir", dir))
}
}
}
171 changes: 171 additions & 0 deletions daemon/cc_info_timer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,177 @@ func (s *CCInfoTimerTestSuite) TestConcurrentNotifyActivity() {
wg.Wait()
}

// Git Cache Tests

func (s *CCInfoTimerTestSuite) TestGetCachedGitInfo_EmptyWorkingDir() {
config := &model.ShellTimeConfig{}
service := NewCCInfoTimerService(config)

info := service.GetCachedGitInfo("")

assert.Equal(s.T(), GitInfo{}, info)
}

func (s *CCInfoTimerTestSuite) TestGetCachedGitInfo_CreatesNewEntry() {
config := &model.ShellTimeConfig{}
service := NewCCInfoTimerService(config)

_ = service.GetCachedGitInfo("/project/a")

service.mu.RLock()
defer service.mu.RUnlock()
assert.Contains(s.T(), service.gitCache, "/project/a")
assert.NotNil(s.T(), service.gitCache["/project/a"])
}

func (s *CCInfoTimerTestSuite) TestGetCachedGitInfo_MultipleCWDs() {
config := &model.ShellTimeConfig{}
service := NewCCInfoTimerService(config)

// Request git info for multiple directories
_ = service.GetCachedGitInfo("/project/a")
_ = service.GetCachedGitInfo("/project/b")
_ = service.GetCachedGitInfo("/project/c")

service.mu.RLock()
defer service.mu.RUnlock()
assert.Len(s.T(), service.gitCache, 3)
assert.Contains(s.T(), service.gitCache, "/project/a")
assert.Contains(s.T(), service.gitCache, "/project/b")
assert.Contains(s.T(), service.gitCache, "/project/c")
}

func (s *CCInfoTimerTestSuite) TestGetCachedGitInfo_PathNormalization() {
config := &model.ShellTimeConfig{}
service := NewCCInfoTimerService(config)

// Request with trailing slash and without - should be same entry
_ = service.GetCachedGitInfo("/project/a/")
_ = service.GetCachedGitInfo("/project/a")

service.mu.RLock()
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")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
assert.Contains(s.T(), service.gitCache, "/project/a")
assert.Contains(s.T(), service.gitCache, filepath.Clean("/project/a"))
References
  1. For platform-independent paths, use filepath.Join to combine segments and os.UserHomeDir() to get the home directory, rather than hardcoding path separators or environment variables like $HOME.

}

func (s *CCInfoTimerTestSuite) TestGetCachedGitInfo_UpdatesLastAccessed() {
config := &model.ShellTimeConfig{}
service := NewCCInfoTimerService(config)

// First access
_ = service.GetCachedGitInfo("/project/a")
service.mu.RLock()
firstAccess := service.gitCache["/project/a"].LastAccessed
service.mu.RUnlock()

time.Sleep(10 * time.Millisecond)

// Second access
_ = service.GetCachedGitInfo("/project/a")
service.mu.RLock()
secondAccess := service.gitCache["/project/a"].LastAccessed
service.mu.RUnlock()

assert.True(s.T(), secondAccess.After(firstAccess))
}

func (s *CCInfoTimerTestSuite) TestGetCachedGitInfo_ReturnsCachedValue() {
config := &model.ShellTimeConfig{}
service := NewCCInfoTimerService(config)

// Manually set cached git info
expectedInfo := GitInfo{Branch: "main", Dirty: true, IsRepo: true}
service.mu.Lock()
service.gitCache["/project/a"] = &GitCacheEntry{
Info: expectedInfo,
LastAccessed: time.Now(),
LastFetched: time.Now(),
}
service.mu.Unlock()

info := service.GetCachedGitInfo("/project/a")

assert.Equal(s.T(), expectedInfo, info)
}

func (s *CCInfoTimerTestSuite) TestCleanupStaleGitCache_RemovesOldEntries() {
config := &model.ShellTimeConfig{}
service := NewCCInfoTimerService(config)

// Add entries with different ages
service.mu.Lock()
service.gitCache["/project/old"] = &GitCacheEntry{
LastAccessed: time.Now().Add(-CCInfoInactivityTimeout - time.Minute),
}
service.gitCache["/project/new"] = &GitCacheEntry{
LastAccessed: time.Now(),
}
service.mu.Unlock()

service.cleanupStaleGitCache()

service.mu.RLock()
defer service.mu.RUnlock()
assert.NotContains(s.T(), service.gitCache, "/project/old")
assert.Contains(s.T(), service.gitCache, "/project/new")
}

func (s *CCInfoTimerTestSuite) TestConcurrentGetCachedGitInfo_DifferentDirs() {
config := &model.ShellTimeConfig{}
service := NewCCInfoTimerService(config)

var wg sync.WaitGroup
numGoroutines := 100

for i := 0; i < numGoroutines; i++ {
wg.Add(1)
go func(idx int) {
defer wg.Done()
// Access different directories
dir := "/project/" + string(rune('a'+idx%10))
service.GetCachedGitInfo(dir)
}(i)
}

// Should complete without race conditions
wg.Wait()

// Verify cache has entries
service.mu.RLock()
defer service.mu.RUnlock()
assert.GreaterOrEqual(s.T(), len(service.gitCache), 1)
}

func (s *CCInfoTimerTestSuite) TestStopTimer_ClearsGitCache() {
config := &model.ShellTimeConfig{
Token: "test-token",
APIEndpoint: s.server.URL,
}
service := NewCCInfoTimerService(config)

// Add git cache entries
service.mu.Lock()
service.gitCache["/project/a"] = &GitCacheEntry{LastAccessed: time.Now()}
service.gitCache["/project/b"] = &GitCacheEntry{LastAccessed: time.Now()}
service.mu.Unlock()

// Start timer
service.NotifyActivity()
time.Sleep(20 * time.Millisecond)

// Stop timer
service.timerMu.Lock()
service.stopTimer()
service.timerMu.Unlock()

// Git cache should be cleared
service.mu.RLock()
defer service.mu.RUnlock()
assert.Empty(s.T(), service.gitCache)
}

func TestCCInfoTimerTestSuite(t *testing.T) {
suite.Run(t, new(CCInfoTimerTestSuite))
}
Loading