From 0096a27c5b0576eea998d49f736bdad8f462c4cb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 12 Feb 2026 20:31:25 +0000 Subject: [PATCH] fix(bugseti): add background TTL sweeper and configurable workspace limits The workspace map previously only cleaned up during Capture() calls, meaning stale entries would accumulate indefinitely if no new captures occurred. This adds: - Background sweeper goroutine (Start/Stop lifecycle) that runs every 5 minutes to evict expired workspaces - Configurable MaxWorkspaces and WorkspaceTTLMinutes in Config (defaults: 100 entries, 24h TTL) replacing hardcoded constants - cleanup() now returns eviction count for observability logging - Nil-config fallback to safe defaults Fixes #54 Co-Authored-By: Claude Opus 4.6 --- cmd/bugseti/workspace.go | 85 +++++++++++++++++++++++++++++---- cmd/bugseti/workspace_test.go | 90 ++++++++++++++++++++++++++++++----- internal/bugseti/config.go | 32 +++++++++++++ 3 files changed, 185 insertions(+), 22 deletions(-) diff --git a/cmd/bugseti/workspace.go b/cmd/bugseti/workspace.go index 79712d92..933514f9 100644 --- a/cmd/bugseti/workspace.go +++ b/cmd/bugseti/workspace.go @@ -17,10 +17,12 @@ import ( ) const ( - // maxWorkspaces is the upper bound on cached workspace entries. - maxWorkspaces = 100 - // workspaceTTL is how long a workspace stays in memory before eviction. - workspaceTTL = 24 * time.Hour + // defaultMaxWorkspaces is the fallback upper bound when config is unavailable. + defaultMaxWorkspaces = 100 + // defaultWorkspaceTTL is the fallback TTL when config is unavailable. + defaultWorkspaceTTL = 24 * time.Hour + // sweepInterval is how often the background sweeper runs. + sweepInterval = 5 * time.Minute ) // WorkspaceService manages DataNode-backed workspaces for issues. @@ -28,8 +30,10 @@ const ( // snapshotted, packaged as a TIM container, or shipped as a crash report. type WorkspaceService struct { config *bugseti.ConfigService - workspaces map[string]*Workspace // issue ID → workspace + workspaces map[string]*Workspace // issue ID -> workspace mu sync.RWMutex + done chan struct{} // signals the background sweeper to stop + stopped chan struct{} // closed when the sweeper goroutine exits } // Workspace tracks a DataNode-backed workspace for an issue. @@ -55,10 +59,13 @@ type CrashReport struct { } // NewWorkspaceService creates a new WorkspaceService. +// Call Start() to begin the background TTL sweeper. func NewWorkspaceService(config *bugseti.ConfigService) *WorkspaceService { return &WorkspaceService{ config: config, workspaces: make(map[string]*Workspace), + done: make(chan struct{}), + stopped: make(chan struct{}), } } @@ -67,6 +74,56 @@ func (w *WorkspaceService) ServiceName() string { return "WorkspaceService" } +// Start launches the background sweeper goroutine that periodically +// evicts expired workspaces. This prevents unbounded map growth even +// when no new Capture calls arrive. +func (w *WorkspaceService) Start() { + go func() { + defer close(w.stopped) + ticker := time.NewTicker(sweepInterval) + defer ticker.Stop() + + for { + select { + case <-ticker.C: + w.mu.Lock() + evicted := w.cleanup() + w.mu.Unlock() + if evicted > 0 { + log.Printf("Workspace sweeper: evicted %d stale entries, %d remaining", evicted, w.ActiveWorkspaces()) + } + case <-w.done: + return + } + } + }() + log.Printf("Workspace sweeper started (interval=%s, ttl=%s, max=%d)", + sweepInterval, w.ttl(), w.maxCap()) +} + +// Stop signals the background sweeper to exit and waits for it to finish. +func (w *WorkspaceService) Stop() { + close(w.done) + <-w.stopped + log.Printf("Workspace sweeper stopped") +} + +// ttl returns the configured workspace TTL, falling back to the default. +func (w *WorkspaceService) ttl() time.Duration { + if w.config != nil { + return w.config.GetWorkspaceTTL() + } + return defaultWorkspaceTTL +} + +// maxCap returns the configured max workspace count, falling back to the default. +func (w *WorkspaceService) maxCap() int { + if w.config != nil { + return w.config.GetMaxWorkspaces() + } + return defaultMaxWorkspaces +} + // Capture loads a filesystem workspace into a DataNode Medium. // Call this after git clone to create the in-memory snapshot. func (w *WorkspaceService) Capture(issue *bugseti.Issue, diskPath string) error { @@ -251,18 +308,23 @@ func (w *WorkspaceService) SaveCrashReport(report *CrashReport) (string, error) // cleanup evicts expired workspaces and enforces the max size cap. // Must be called with w.mu held for writing. -func (w *WorkspaceService) cleanup() { +// Returns the number of evicted entries. +func (w *WorkspaceService) cleanup() int { now := time.Now() + ttl := w.ttl() + cap := w.maxCap() + evicted := 0 // First pass: evict entries older than TTL. for id, ws := range w.workspaces { - if now.Sub(ws.CreatedAt) > workspaceTTL { + if now.Sub(ws.CreatedAt) > ttl { delete(w.workspaces, id) + evicted++ } } // Second pass: if still over cap, evict oldest entries. - if len(w.workspaces) > maxWorkspaces { + if len(w.workspaces) > cap { type entry struct { id string createdAt time.Time @@ -274,11 +336,14 @@ func (w *WorkspaceService) cleanup() { sort.Slice(entries, func(i, j int) bool { return entries[i].createdAt.Before(entries[j].createdAt) }) - evict := len(w.workspaces) - maxWorkspaces - for i := 0; i < evict; i++ { + toEvict := len(w.workspaces) - cap + for i := 0; i < toEvict; i++ { delete(w.workspaces, entries[i].id) + evicted++ } } + + return evicted } // Release removes a workspace from memory. diff --git a/cmd/bugseti/workspace_test.go b/cmd/bugseti/workspace_test.go index 546e8d39..2ff22554 100644 --- a/cmd/bugseti/workspace_test.go +++ b/cmd/bugseti/workspace_test.go @@ -33,9 +33,11 @@ func TestCleanup_TTL(t *testing.T) { func TestCleanup_MaxSize(t *testing.T) { svc := NewWorkspaceService(bugseti.NewConfigService()) + maxCap := svc.maxCap() + // Fill beyond the cap with fresh entries. svc.mu.Lock() - for i := 0; i < maxWorkspaces+20; i++ { + for i := 0; i < maxCap+20; i++ { svc.workspaces[fmt.Sprintf("ws-%d", i)] = &Workspace{ CreatedAt: time.Now().Add(-time.Duration(i) * time.Minute), } @@ -43,30 +45,28 @@ func TestCleanup_MaxSize(t *testing.T) { svc.cleanup() svc.mu.Unlock() - if got := svc.ActiveWorkspaces(); got != maxWorkspaces { - t.Errorf("expected %d workspaces after cap cleanup, got %d", maxWorkspaces, got) + if got := svc.ActiveWorkspaces(); got != maxCap { + t.Errorf("expected %d workspaces after cap cleanup, got %d", maxCap, got) } } func TestCleanup_EvictsOldestWhenOverCap(t *testing.T) { svc := NewWorkspaceService(bugseti.NewConfigService()) - // Create maxWorkspaces+1 entries; the newest should survive. + maxCap := svc.maxCap() + + // Create maxCap+1 entries; the newest should survive. svc.mu.Lock() - for i := 0; i <= maxWorkspaces; i++ { + for i := 0; i <= maxCap; i++ { svc.workspaces[fmt.Sprintf("ws-%d", i)] = &Workspace{ - CreatedAt: time.Now().Add(-time.Duration(maxWorkspaces-i) * time.Minute), + CreatedAt: time.Now().Add(-time.Duration(maxCap-i) * time.Minute), } } svc.cleanup() svc.mu.Unlock() - // The newest entry (ws-) should still exist. - newest := fmt.Sprintf("ws-%d", maxWorkspaces) - if m := svc.GetMedium(newest); m != nil { - // GetMedium returns nil for entries with nil Medium, which is expected here. - // We just want to verify the key still exists. - } + // The newest entry (ws-) should still exist. + newest := fmt.Sprintf("ws-%d", maxCap) svc.mu.RLock() _, exists := svc.workspaces[newest] @@ -83,3 +83,69 @@ func TestCleanup_EvictsOldestWhenOverCap(t *testing.T) { t.Error("expected oldest workspace to be evicted") } } + +func TestCleanup_ReturnsEvictedCount(t *testing.T) { + svc := NewWorkspaceService(bugseti.NewConfigService()) + + svc.mu.Lock() + for i := 0; i < 3; i++ { + svc.workspaces[fmt.Sprintf("old-%d", i)] = &Workspace{ + CreatedAt: time.Now().Add(-25 * time.Hour), + } + } + svc.workspaces["fresh"] = &Workspace{ + CreatedAt: time.Now(), + } + evicted := svc.cleanup() + svc.mu.Unlock() + + if evicted != 3 { + t.Errorf("expected 3 evicted entries, got %d", evicted) + } +} + +func TestStartStop(t *testing.T) { + svc := NewWorkspaceService(bugseti.NewConfigService()) + svc.Start() + + // Add a stale entry while the sweeper is running. + svc.mu.Lock() + svc.workspaces["stale"] = &Workspace{ + CreatedAt: time.Now().Add(-25 * time.Hour), + } + svc.mu.Unlock() + + // Stop should return without hanging. + svc.Stop() +} + +func TestConfigurableTTL(t *testing.T) { + cfg := bugseti.NewConfigService() + svc := NewWorkspaceService(cfg) + + // Default TTL should be 24h (1440 minutes). + if got := svc.ttl(); got != 24*time.Hour { + t.Errorf("expected default TTL of 24h, got %s", got) + } + + // Default max cap should be 100. + if got := svc.maxCap(); got != 100 { + t.Errorf("expected default max cap of 100, got %d", got) + } +} + +func TestNilConfigFallback(t *testing.T) { + svc := &WorkspaceService{ + config: nil, + workspaces: make(map[string]*Workspace), + done: make(chan struct{}), + stopped: make(chan struct{}), + } + + if got := svc.ttl(); got != defaultWorkspaceTTL { + t.Errorf("expected fallback TTL %s, got %s", defaultWorkspaceTTL, got) + } + if got := svc.maxCap(); got != defaultMaxWorkspaces { + t.Errorf("expected fallback max cap %d, got %d", defaultMaxWorkspaces, got) + } +} diff --git a/internal/bugseti/config.go b/internal/bugseti/config.go index 88ad9678..7f949b1d 100644 --- a/internal/bugseti/config.go +++ b/internal/bugseti/config.go @@ -52,6 +52,10 @@ type Config struct { MaxConcurrentIssues int `json:"maxConcurrentIssues"` AutoSeedContext bool `json:"autoSeedContext"` + // Workspace cache + MaxWorkspaces int `json:"maxWorkspaces"` // Upper bound on cached workspace entries (0 = default 100) + WorkspaceTTLMinutes int `json:"workspaceTtlMinutes"` // TTL for workspace entries in minutes (0 = default 1440 = 24h) + // Updates UpdateChannel string `json:"updateChannel"` // stable, beta, nightly AutoUpdate bool `json:"autoUpdate"` // Automatically install updates @@ -99,6 +103,8 @@ func NewConfigService() *ConfigService { AutoSeedContext: true, DataDir: bugsetiDir, MarketplaceMCPRoot: "", + MaxWorkspaces: 100, + WorkspaceTTLMinutes: 1440, // 24 hours UpdateChannel: "stable", AutoUpdate: false, UpdateCheckInterval: 6, // Check every 6 hours @@ -169,6 +175,12 @@ func (c *ConfigService) mergeDefaults(config *Config) { if config.DataDir == "" { config.DataDir = c.config.DataDir } + if config.MaxWorkspaces == 0 { + config.MaxWorkspaces = 100 + } + if config.WorkspaceTTLMinutes == 0 { + config.WorkspaceTTLMinutes = 1440 + } if config.UpdateChannel == "" { config.UpdateChannel = "stable" } @@ -406,6 +418,26 @@ func (c *ConfigService) SetAutoSeedEnabled(enabled bool) error { return c.saveUnsafe() } +// GetMaxWorkspaces returns the maximum number of cached workspaces. +func (c *ConfigService) GetMaxWorkspaces() int { + c.mu.RLock() + defer c.mu.RUnlock() + if c.config.MaxWorkspaces <= 0 { + return 100 + } + return c.config.MaxWorkspaces +} + +// GetWorkspaceTTL returns the workspace TTL as a time.Duration. +func (c *ConfigService) GetWorkspaceTTL() time.Duration { + c.mu.RLock() + defer c.mu.RUnlock() + if c.config.WorkspaceTTLMinutes <= 0 { + return 24 * time.Hour + } + return time.Duration(c.config.WorkspaceTTLMinutes) * time.Minute +} + // UpdateSettings holds update-related configuration. type UpdateSettings struct { Channel string `json:"channel"`