From 2751b932dfb228fa6df0dee180c6b6dad1954db2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Fri, 24 Jul 2026 09:55:40 +0100 Subject: [PATCH] perf: cache storage analysis results (#1911) --- .../backendapp/storage_maintenance.go | 7 +- .../internal/system/storage/handler.go | 11 +- .../internal/system/storage/handler_test.go | 54 +++ .../internal/system/storage/operations.go | 31 +- .../system/storage/operations_test.go | 156 +++++++++ .../internal/system/storage/overview_cache.go | 119 +++++++ .../system/storage/overview_cache_test.go | 309 ++++++++++++++++++ .../backend/internal/system/storage/runner.go | 7 +- .../internal/system/storage_routes_test.go | 4 +- .../storage-maintenance-settings.test.tsx | 119 ++++++- .../storage/storage-maintenance-settings.tsx | 35 +- .../storage/storage-overview-card.test.tsx | 27 ++ .../system/storage/storage-overview-card.tsx | 14 +- .../storage/storage-policy-card.test.tsx | 101 +++++- .../system/storage/storage-policy-card.tsx | 15 +- .../system/mobile-storage-maintenance.spec.ts | 62 ++++ .../tests/system/storage-maintenance.spec.ts | 33 ++ .../system/use-storage-maintenance.test.tsx | 76 ++++- .../domains/system/use-storage-maintenance.ts | 21 +- apps/web/lib/api/domains/system-api.test.ts | 8 +- .../state/slices/system/system-slice.test.ts | 1 + apps/web/lib/types/system.ts | 1 + docs/plans/storage-overview-cache/plan.md | 98 ++++++ .../task-01-backend-cached-overview.md | 50 +++ .../task-02-frontend-settings-stability.md | 55 ++++ .../task-03-integrated-e2e.md | 46 +++ docs/public/operations.md | 5 + docs/specs/system-page/storage-maintenance.md | 33 +- 28 files changed, 1458 insertions(+), 40 deletions(-) create mode 100644 apps/backend/internal/system/storage/overview_cache.go create mode 100644 apps/backend/internal/system/storage/overview_cache_test.go create mode 100644 docs/plans/storage-overview-cache/plan.md create mode 100644 docs/plans/storage-overview-cache/task-01-backend-cached-overview.md create mode 100644 docs/plans/storage-overview-cache/task-02-frontend-settings-stability.md create mode 100644 docs/plans/storage-overview-cache/task-03-integrated-e2e.md diff --git a/apps/backend/internal/backendapp/storage_maintenance.go b/apps/backend/internal/backendapp/storage_maintenance.go index 20e2818367..47cf99becf 100644 --- a/apps/backend/internal/backendapp/storage_maintenance.go +++ b/apps/backend/internal/backendapp/storage_maintenance.go @@ -69,9 +69,10 @@ func provideStorageComposition( docker: dockerProvider, dockerClient: dockerClient, dockerHost: cfg.Docker.Host, homeDir: cfg.ResolvedHomeDir(), } + cachedOverview := storagepkg.NewOverviewCache(overview) providers := storageCleanupProviders(settings, workspaceFactory, goCache, dockerProvider) runner := storagepkg.NewRunner(storagepkg.RunnerConfig{ - Activity: coordinator, Store: store, Providers: providers, + Activity: coordinator, Store: store, Providers: providers, Overview: cachedOverview, }) scheduler := storagepkg.NewScheduler(settings, runner, storagepkg.SchedulerOptions{}) runtime := storagepkg.NewRuntime(storagepkg.RuntimeConfig{ @@ -84,10 +85,10 @@ func provideStorageComposition( } operations := storagepkg.NewOperations(storagepkg.OperationsConfig{ Settings: settings, Store: store, Jobs: tracker, Activity: coordinator, - Providers: providers, Overview: overview, GoCache: goCache, Quarantine: quarantine, + Providers: providers, Overview: cachedOverview, GoCache: goCache, Quarantine: quarantine, }) handler := storagepkg.NewHandler(storagepkg.HandlerConfig{ - Settings: settings, Runs: store, Quarantine: store, Overview: overview, + Settings: settings, Runs: store, Quarantine: store, Overview: cachedOverview, Mutations: operations, OnSettingsChanged: runtime.ApplySettings, }) return &storageComposition{ diff --git a/apps/backend/internal/system/storage/handler.go b/apps/backend/internal/system/storage/handler.go index 2db9751250..86ad91f0d1 100644 --- a/apps/backend/internal/system/storage/handler.go +++ b/apps/backend/internal/system/storage/handler.go @@ -55,11 +55,16 @@ type OverviewProvider interface { Capabilities(context.Context, StorageMaintenanceSettings) Capabilities } +type OverviewReader interface { + Get(context.Context) (OverviewSnapshot, error) + Capabilities(context.Context, StorageMaintenanceSettings) Capabilities +} + type HandlerConfig struct { Settings SettingsManager Runs RunLister Quarantine QuarantineLister - Overview OverviewProvider + Overview OverviewReader Mutations Mutations OnSettingsChanged func(StorageMaintenanceSettings) } @@ -199,7 +204,7 @@ func (h *Handler) getStorage(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return } - summary, err := h.config.Overview.Summary(c.Request.Context()) + snapshot, err := h.config.Overview.Get(c.Request.Context()) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return @@ -215,7 +220,7 @@ func (h *Handler) getStorage(c *gin.Context) { } c.JSON(http.StatusOK, gin.H{ "settings": settings, "capabilities": h.config.Overview.Capabilities(c.Request.Context(), settings), - "summary": summary, "last_run": lastRun, + "summary": snapshot.Summary, "analyzed_at": snapshot.AnalyzedAt, "last_run": lastRun, }) } diff --git a/apps/backend/internal/system/storage/handler_test.go b/apps/backend/internal/system/storage/handler_test.go index 41003b21f2..be9adf020c 100644 --- a/apps/backend/internal/system/storage/handler_test.go +++ b/apps/backend/internal/system/storage/handler_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/gin-gonic/gin" ) @@ -39,12 +40,65 @@ func TestPatchSettingsHidesInternalSaveFailure(t *testing.T) { } } +func TestGetStorageReturnsSnapshotAnalyzedAt(t *testing.T) { + gin.SetMode(gin.TestMode) + analyzedAt := time.Date(2026, time.July, 23, 12, 0, 0, 0, time.UTC) + router := gin.New() + RegisterRoutes(router.Group("/api/v1/system"), NewHandler(HandlerConfig{ + Settings: staticSettingsManager{}, Runs: staticRunLister{}, + Overview: staticCachedOverview{snapshot: OverviewSnapshot{ + Summary: Summary{Workspaces: map[string]any{"bytes": 42}}, AnalyzedAt: analyzedAt, + }}, + })) + response := httptest.NewRecorder() + router.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/api/v1/system/storage", nil)) + + if response.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", response.Code) + } + var body struct { + Summary Summary `json:"summary"` + AnalyzedAt time.Time `json:"analyzed_at"` + } + if err := json.Unmarshal(response.Body.Bytes(), &body); err != nil { + t.Fatalf("decode response: %v", err) + } + if !body.AnalyzedAt.Equal(analyzedAt) { + t.Fatalf("analyzed_at = %s, want %s", body.AnalyzedAt, analyzedAt) + } + if body.Summary.Workspaces.(map[string]any)["bytes"] != float64(42) { + t.Fatalf("summary = %#v", body.Summary) + } +} + type failingSettingsManager struct{ err error } func (f failingSettingsManager) GetSettings(context.Context) (StorageMaintenanceSettings, error) { return DefaultSettings(), nil } +type staticSettingsManager struct{} + +func (staticSettingsManager) GetSettings(context.Context) (StorageMaintenanceSettings, error) { + return DefaultSettings(), nil +} + +func (staticSettingsManager) SaveSettingsWithConfirmations(context.Context, StorageMaintenanceSettings, SaveConfirmations) (StorageMaintenanceSettings, error) { + return DefaultSettings(), nil +} + +type staticRunLister struct{} + +func (staticRunLister) ListRuns(context.Context, int) ([]MaintenanceRun, error) { return nil, nil } + +type staticCachedOverview struct{ snapshot OverviewSnapshot } + +func (o staticCachedOverview) Get(context.Context) (OverviewSnapshot, error) { return o.snapshot, nil } + +func (o staticCachedOverview) Capabilities(context.Context, StorageMaintenanceSettings) Capabilities { + return Capabilities{} +} + func (f failingSettingsManager) SaveSettingsWithConfirmations( context.Context, StorageMaintenanceSettings, diff --git a/apps/backend/internal/system/storage/operations.go b/apps/backend/internal/system/storage/operations.go index 2cb948021e..78dd82b00e 100644 --- a/apps/backend/internal/system/storage/operations.go +++ b/apps/backend/internal/system/storage/operations.go @@ -32,7 +32,7 @@ type OperationsConfig struct { Jobs *jobs.Tracker Activity *activity.Coordinator Providers []CleanupProvider - Overview OverviewProvider + Overview OverviewRefresher GoCache GoCacheAdopter Quarantine QuarantineController } @@ -60,6 +60,7 @@ func (o *Operations) AdoptGoCache( if err != nil { return StorageMaintenanceSettings{}, Capabilities{}, err } + o.invalidateOverview() return settings, o.config.Overview.Capabilities(ctx, settings), nil } @@ -75,11 +76,17 @@ func (o *Operations) Analyze(ctx context.Context) (string, error) { if _, err := o.config.Store.TransitionRun(jobCtx, id, RunStateRunning, nil, ""); err != nil { return nil, err } - summary, analyzeErr := o.config.Overview.Summary(jobCtx) - return o.finishAnalysis(jobCtx, id, summary, analyzeErr) + snapshot, analyzeErr := o.config.Overview.Refresh(jobCtx) + return o.finishAnalysis(jobCtx, id, snapshot.Summary, analyzeErr) }), nil } +type OverviewRefresher interface { + OverviewReader + OverviewInvalidator + Refresh(context.Context) (OverviewSnapshot, error) +} + func (o *Operations) RunNow(ctx context.Context, resources []string) (string, error) { settings, err := o.config.Settings.GetSettings(ctx) if err != nil { @@ -95,7 +102,7 @@ func (o *Operations) RunNow(ctx context.Context, resources []string) (string, er return o.startTracked(ctx, JobKindCleanup, func(jobCtx context.Context, id string) (map[string]any, error) { runner := NewRunner(RunnerConfig{ Activity: o.config.Activity, Store: o.config.Store, Providers: providers, - NewID: func() string { return id }, + NewID: func() string { return id }, Overview: o.config.Overview, }) run, runErr := runner.Run(jobCtx, RunTriggerManual, settings) return runResultMap(run), runErr @@ -106,7 +113,12 @@ func (o *Operations) RestoreQuarantine(ctx context.Context, id string) (Quaranti if o.config.Quarantine == nil { return QuarantineEntry{}, errors.New("quarantine provider is unavailable") } - return o.config.Quarantine.Restore(ctx, id) + entry, err := o.config.Quarantine.Restore(ctx, id) + if err != nil { + return QuarantineEntry{}, err + } + o.invalidateOverview() + return entry, nil } func (o *Operations) DeleteQuarantine(ctx context.Context, id, confirmation string) (string, error) { @@ -125,10 +137,19 @@ func (o *Operations) DeleteQuarantine(ctx context.Context, id, confirmation stri } return o.startTracked(ctx, JobKindQuarantineDelete, func(jobCtx context.Context, _ string) (map[string]any, error) { entry, err := o.config.Quarantine.PermanentDelete(jobCtx, id, confirmation) + if err == nil { + o.invalidateOverview() + } return map[string]any{"entry": entry}, err }), nil } +func (o *Operations) invalidateOverview() { + if o.config.Overview != nil { + o.config.Overview.Invalidate() + } +} + func (o *Operations) preflight(ctx context.Context) error { lease, busy, err := o.config.Activity.TryAcquireMaintenance(ctx, 0) if errors.Is(err, activity.ErrBusy) { diff --git a/apps/backend/internal/system/storage/operations_test.go b/apps/backend/internal/system/storage/operations_test.go index c5c0d47d7e..753d57ef12 100644 --- a/apps/backend/internal/system/storage/operations_test.go +++ b/apps/backend/internal/system/storage/operations_test.go @@ -57,6 +57,32 @@ func TestRunNowIgnoresQuietPeriodWithoutActiveTaskWork(t *testing.T) { } } +func TestRunNowInvalidatesOverviewAfterSuccess(t *testing.T) { + connection := newSQLite(t) + pool := db.NewPool(connection, connection) + rawSettings, err := systemsettings.NewStore(pool) + if err != nil { + t.Fatalf("new settings store: %v", err) + } + overview := &recordingRefreshOverview{} + tracker := jobs.NewTracker(nil, newOperationsTestLogger(t)) + operations := NewOperations(OperationsConfig{ + Settings: NewSettingsStore(rawSettings), Store: newStorageStore(t, pool), + Jobs: tracker, Activity: activity.NewCoordinator(activity.Options{}), + Providers: []CleanupProvider{&signallingCleanupProvider{called: make(chan struct{})}}, + Overview: overview, + }) + + jobID, err := operations.RunNow(context.Background(), nil) + if err != nil { + t.Fatalf("RunNow: %v", err) + } + waitForJobState(t, tracker, jobID, jobs.StateSucceeded) + if overview.invalidateCalls != 1 { + t.Fatalf("overview invalidations = %d, want 1", overview.invalidateCalls) + } +} + func TestRunNowRejectsCurrentTaskActivity(t *testing.T) { connection := newSQLite(t) pool := db.NewPool(connection, connection) @@ -167,6 +193,99 @@ func TestDeleteQuarantineRejectsBeforeRetentionWithoutStartingDelete(t *testing. } } +func TestAdoptGoCacheInvalidatesOverview(t *testing.T) { + connection := newSQLite(t) + pool := db.NewPool(connection, connection) + rawSettings, err := systemsettings.NewStore(pool) + if err != nil { + t.Fatalf("new settings store: %v", err) + } + overview := &recordingRefreshOverview{} + operations := NewOperations(OperationsConfig{ + Settings: NewSettingsStore(rawSettings), + Overview: overview, + GoCache: recordingGoCacheAdopter{}, + }) + + if _, _, err := operations.AdoptGoCache(context.Background(), t.TempDir(), "ADOPT"); err != nil { + t.Fatalf("AdoptGoCache: %v", err) + } + if overview.invalidateCalls != 1 { + t.Fatalf("overview invalidations = %d, want 1", overview.invalidateCalls) + } +} + +func TestRestoreQuarantineInvalidatesOverview(t *testing.T) { + overview := &recordingRefreshOverview{} + operations := NewOperations(OperationsConfig{ + Overview: overview, Quarantine: &recordingQuarantineController{}, + }) + + if _, err := operations.RestoreQuarantine(context.Background(), "entry"); err != nil { + t.Fatalf("RestoreQuarantine: %v", err) + } + if overview.invalidateCalls != 1 { + t.Fatalf("overview invalidations = %d, want 1", overview.invalidateCalls) + } +} + +func TestDeleteQuarantineInvalidatesOverviewAfterSuccess(t *testing.T) { + connection := newSQLite(t) + store := newStorageStore(t, db.NewPool(connection, connection)) + entry := testQuarantineEntry("expired-entry") + entry.DeleteAfter = time.Now().UTC().Add(-time.Hour) + if err := store.CreateQuarantineEntry(context.Background(), &entry); err != nil { + t.Fatal(err) + } + overview := &recordingRefreshOverview{} + tracker := jobs.NewTracker(nil, newOperationsTestLogger(t)) + operations := NewOperations(OperationsConfig{ + Store: store, Jobs: tracker, Overview: overview, + Quarantine: &recordingQuarantineController{}, + }) + + jobID, err := operations.DeleteQuarantine(context.Background(), entry.ID, "DELETE") + if err != nil { + t.Fatalf("DeleteQuarantine: %v", err) + } + waitForJobState(t, tracker, jobID, jobs.StateSucceeded) + if overview.invalidateCalls != 1 { + t.Fatalf("overview invalidations = %d, want 1", overview.invalidateCalls) + } +} + +func TestAnalyzeForcesOverviewRefresh(t *testing.T) { + connection := newSQLite(t) + pool := db.NewPool(connection, connection) + rawSettings, err := systemsettings.NewStore(pool) + if err != nil { + t.Fatalf("new settings store: %v", err) + } + overview := &recordingRefreshOverview{called: make(chan struct{})} + tracker := jobs.NewTracker(nil, newOperationsTestLogger(t)) + operations := NewOperations(OperationsConfig{ + Settings: NewSettingsStore(rawSettings), Store: newStorageStore(t, pool), + Jobs: tracker, Overview: overview, + }) + + jobID, err := operations.Analyze(context.Background()) + if err != nil { + t.Fatalf("Analyze: %v", err) + } + select { + case <-overview.called: + case <-time.After(time.Second): + t.Fatal("Analyze did not request an overview refresh") + } + if overview.refreshCalls != 1 { + t.Fatalf("Refresh calls = %d, want 1", overview.refreshCalls) + } + if overview.summaryCalls != 0 { + t.Fatalf("Summary calls = %d, want 0", overview.summaryCalls) + } + waitForJobState(t, tracker, jobID, jobs.StateSucceeded) +} + func waitForJobState(t *testing.T, tracker *jobs.Tracker, id string, state jobs.State) { t.Helper() deadline := time.Now().Add(time.Second) @@ -187,6 +306,43 @@ type recordingQuarantineController struct { deleteCalls int } +type recordingGoCacheAdopter struct{} + +func (recordingGoCacheAdopter) ValidateAdoption(context.Context, string, string) error { + return nil +} + +type recordingRefreshOverview struct { + called chan struct{} + refreshCalls int + summaryCalls int + invalidateCalls int +} + +func (o *recordingRefreshOverview) Summary(context.Context) (Summary, error) { + o.summaryCalls++ + close(o.called) + return Summary{}, nil +} + +func (o *recordingRefreshOverview) Refresh(context.Context) (OverviewSnapshot, error) { + o.refreshCalls++ + close(o.called) + return OverviewSnapshot{Summary: Summary{}}, nil +} + +func (o *recordingRefreshOverview) Get(context.Context) (OverviewSnapshot, error) { + return OverviewSnapshot{}, nil +} + +func (o *recordingRefreshOverview) Capabilities(context.Context, StorageMaintenanceSettings) Capabilities { + return Capabilities{} +} + +func (o *recordingRefreshOverview) Invalidate() { + o.invalidateCalls++ +} + type signallingCleanupProvider struct { called chan struct{} } diff --git a/apps/backend/internal/system/storage/overview_cache.go b/apps/backend/internal/system/storage/overview_cache.go new file mode 100644 index 0000000000..4d91fe0289 --- /dev/null +++ b/apps/backend/internal/system/storage/overview_cache.go @@ -0,0 +1,119 @@ +package storage + +import ( + "context" + "sync" + "time" +) + +const defaultOverviewCacheTTL = 15 * time.Minute + +// OverviewSnapshot is a successful storage overview and the time it was measured. +type OverviewSnapshot struct { + Summary Summary + AnalyzedAt time.Time +} + +type OverviewInvalidator interface { + Invalidate() +} + +// OverviewCache keeps one process-local successful overview snapshot. +type OverviewCache struct { + provider OverviewProvider + ttl time.Duration + now func() time.Time + + mu sync.Mutex + snapshot *OverviewSnapshot + flight *overviewFlight + generation uint64 +} + +type overviewFlight struct { + done chan struct{} + snapshot OverviewSnapshot + err error + generation uint64 +} + +func NewOverviewCache(provider OverviewProvider) *OverviewCache { + return &OverviewCache{provider: provider, ttl: defaultOverviewCacheTTL, now: time.Now} +} + +func (c *OverviewCache) Get(ctx context.Context) (OverviewSnapshot, error) { + c.mu.Lock() + if c.snapshot != nil && c.now().Sub(c.snapshot.AnalyzedAt) < c.ttl { + snapshot := *c.snapshot + c.mu.Unlock() + return snapshot, nil + } + if c.flight != nil { + flight := c.flight + c.mu.Unlock() + return waitForOverviewRefresh(ctx, flight) + } + flight := c.startRefreshLocked() + c.mu.Unlock() + go c.refresh(context.WithoutCancel(ctx), flight) + return waitForOverviewRefresh(ctx, flight) +} + +// Refresh bypasses the freshness window and replaces the shared snapshot on success. +func (c *OverviewCache) Refresh(ctx context.Context) (OverviewSnapshot, error) { + c.mu.Lock() + if c.flight != nil { + flight := c.flight + c.mu.Unlock() + return waitForOverviewRefresh(ctx, flight) + } + flight := c.startRefreshLocked() + c.mu.Unlock() + c.refresh(ctx, flight) + return flight.snapshot, flight.err +} + +func (c *OverviewCache) Capabilities(ctx context.Context, settings StorageMaintenanceSettings) Capabilities { + return c.provider.Capabilities(ctx, settings) +} + +func (c *OverviewCache) Invalidate() { + c.mu.Lock() + c.generation++ + c.snapshot = nil + c.flight = nil + c.mu.Unlock() +} + +func (c *OverviewCache) startRefreshLocked() *overviewFlight { + flight := &overviewFlight{done: make(chan struct{}), generation: c.generation} + c.flight = flight + return flight +} + +func (c *OverviewCache) refresh(ctx context.Context, flight *overviewFlight) { + summary, err := c.provider.Summary(ctx) + c.mu.Lock() + if err == nil { + snapshot := OverviewSnapshot{Summary: summary, AnalyzedAt: c.now().UTC()} + flight.snapshot = snapshot + if flight.generation == c.generation { + c.snapshot = &snapshot + } + } + flight.err = err + if c.flight == flight { + c.flight = nil + } + close(flight.done) + c.mu.Unlock() +} + +func waitForOverviewRefresh(ctx context.Context, flight *overviewFlight) (OverviewSnapshot, error) { + select { + case <-flight.done: + return flight.snapshot, flight.err + case <-ctx.Done(): + return OverviewSnapshot{}, ctx.Err() + } +} diff --git a/apps/backend/internal/system/storage/overview_cache_test.go b/apps/backend/internal/system/storage/overview_cache_test.go new file mode 100644 index 0000000000..134ac94f09 --- /dev/null +++ b/apps/backend/internal/system/storage/overview_cache_test.go @@ -0,0 +1,309 @@ +package storage + +import ( + "context" + "errors" + "sync" + "testing" + "testing/synctest" + "time" +) + +func TestOverviewCacheReusesSuccessfulSummaryWithinTTL(t *testing.T) { + now := time.Date(2026, time.July, 23, 12, 0, 0, 0, time.UTC) + provider := &countingOverview{} + cache := newOverviewCacheForTest(provider, func() time.Time { return now }) + + first, err := cache.Get(context.Background()) + if err != nil { + t.Fatalf("first Get: %v", err) + } + now = now.Add(14*time.Minute + 59*time.Second) + second, err := cache.Get(context.Background()) + if err != nil { + t.Fatalf("second Get: %v", err) + } + if provider.calls != 1 { + t.Fatalf("Summary calls = %d, want 1", provider.calls) + } + if !second.AnalyzedAt.Equal(first.AnalyzedAt) { + t.Fatalf("cached analyzed_at = %s, want %s", second.AnalyzedAt, first.AnalyzedAt) + } +} + +func TestOverviewCacheRefreshesAfterTTL(t *testing.T) { + now := time.Date(2026, time.July, 23, 12, 0, 0, 0, time.UTC) + provider := &countingOverview{} + cache := newOverviewCacheForTest(provider, func() time.Time { return now }) + if _, err := cache.Get(context.Background()); err != nil { + t.Fatalf("first Get: %v", err) + } + now = now.Add(15 * time.Minute) + if _, err := cache.Get(context.Background()); err != nil { + t.Fatalf("expired Get: %v", err) + } + if provider.calls != 2 { + t.Fatalf("Summary calls = %d, want 2", provider.calls) + } +} + +func TestOverviewCacheForceRefreshBypassesTTL(t *testing.T) { + now := time.Date(2026, time.July, 23, 12, 0, 0, 0, time.UTC) + provider := &countingOverview{} + cache := newOverviewCacheForTest(provider, func() time.Time { return now }) + if _, err := cache.Get(context.Background()); err != nil { + t.Fatalf("Get: %v", err) + } + now = now.Add(time.Minute) + forced, err := cache.Refresh(context.Background()) + if err != nil { + t.Fatalf("Refresh: %v", err) + } + if provider.calls != 2 { + t.Fatalf("Summary calls = %d, want 2", provider.calls) + } + if !forced.AnalyzedAt.Equal(now) { + t.Fatalf("forced analyzed_at = %s, want %s", forced.AnalyzedAt, now) + } +} + +func TestOverviewCacheKeepsLastSuccessfulSnapshotAfterFailedRefresh(t *testing.T) { + now := time.Date(2026, time.July, 23, 12, 0, 0, 0, time.UTC) + provider := &countingOverview{} + cache := newOverviewCacheForTest(provider, func() time.Time { return now }) + first, err := cache.Get(context.Background()) + if err != nil { + t.Fatalf("Get: %v", err) + } + provider.err = errors.New("provider unavailable") + now = now.Add(time.Minute) + if _, err := cache.Refresh(context.Background()); !errors.Is(err, provider.err) { + t.Fatalf("Refresh error = %v, want provider error", err) + } + cached, err := cache.Get(context.Background()) + if err != nil { + t.Fatalf("Get after failed refresh: %v", err) + } + if cached.Summary != first.Summary || !cached.AnalyzedAt.Equal(first.AnalyzedAt) { + t.Fatalf("cached snapshot = %#v, want %#v", cached, first) + } +} + +func TestOverviewCacheConcurrentRefreshWaitersReceiveFlightError(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + provider := &countingOverview{} + cache := NewOverviewCache(provider) + previous, err := cache.Get(context.Background()) + if err != nil { + t.Fatalf("initial Get: %v", err) + } + flightErr := errors.New("provider unavailable") + provider.err = flightErr + provider.started = make(chan struct{}) + provider.release = make(chan struct{}) + + results := make(chan error, 2) + go func() { _, err := cache.Refresh(context.Background()); results <- err }() + <-provider.started + go func() { _, err := cache.Refresh(context.Background()); results <- err }() + synctest.Wait() + close(provider.release) + + for range 2 { + if err := <-results; !errors.Is(err, flightErr) { + t.Fatalf("concurrent Refresh error = %v, want provider error", err) + } + } + cached, err := cache.Get(context.Background()) + if err != nil { + t.Fatalf("Get after failed refresh: %v", err) + } + if cached.Summary != previous.Summary || !cached.AnalyzedAt.Equal(previous.AnalyzedAt) { + t.Fatalf("cached snapshot = %#v, want %#v", cached, previous) + } + }) +} + +func TestOverviewCacheCoalescesConcurrentMisses(t *testing.T) { + provider := &countingOverview{started: make(chan struct{}), release: make(chan struct{})} + cache := NewOverviewCache(provider) + results := make(chan error, 2) + go func() { _, err := cache.Get(context.Background()); results <- err }() + <-provider.started + go func() { _, err := cache.Get(context.Background()); results <- err }() + close(provider.release) + for range 2 { + if err := <-results; err != nil { + t.Fatalf("Get: %v", err) + } + } + if provider.calls != 1 { + t.Fatalf("Summary calls = %d, want 1", provider.calls) + } +} + +func TestOverviewCacheInvalidationDuringRefreshDiscardsStaleSnapshot(t *testing.T) { + provider := &firstCallBlockingOverview{ + started: make(chan struct{}), + release: make(chan struct{}), + } + cache := NewOverviewCache(provider) + firstResult := make(chan overviewResult, 1) + go func() { + snapshot, err := cache.Get(context.Background()) + firstResult <- overviewResult{snapshot: snapshot, err: err} + }() + <-provider.started + + cache.Invalidate() + close(provider.release) + if result := <-firstResult; result.err != nil { + t.Fatalf("first Get: %v", result.err) + } + second, err := cache.Get(context.Background()) + if err != nil { + t.Fatalf("Get after invalidation: %v", err) + } + if second.Summary.Workspaces != 2 { + t.Fatalf("summary workspaces = %d, want refreshed value 2", second.Summary.Workspaces) + } +} + +func TestOverviewCacheGetInitiatorCancellationDoesNotCancelSharedFlight(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + provider := &cancellableOverview{started: make(chan struct{}), release: make(chan struct{})} + cache := NewOverviewCache(provider) + initiatorCtx, cancelInitiator := context.WithCancel(context.Background()) + defer cancelInitiator() + + initiatorResult := make(chan overviewResult, 1) + waiterResult := make(chan overviewResult, 1) + go func() { + snapshot, err := cache.Get(initiatorCtx) + initiatorResult <- overviewResult{snapshot: snapshot, err: err} + }() + <-provider.started + go func() { + snapshot, err := cache.Get(context.Background()) + waiterResult <- overviewResult{snapshot: snapshot, err: err} + }() + synctest.Wait() + + cancelInitiator() + if result := <-initiatorResult; !errors.Is(result.err, context.Canceled) { + t.Fatalf("initiator Get error = %v, want context canceled", result.err) + } + synctest.Wait() + select { + case result := <-waiterResult: + t.Fatalf("waiter Get completed before provider release: %+v", result) + default: + } + + close(provider.release) + result := <-waiterResult + if result.err != nil { + t.Fatalf("waiter Get: %v", result.err) + } + if result.snapshot.Summary.Workspaces != 1 { + t.Fatalf("waiter summary = %#v, want one provider result", result.snapshot.Summary) + } + if provider.calls != 1 { + t.Fatalf("Summary calls = %d, want 1", provider.calls) + } + cached, err := cache.Get(context.Background()) + if err != nil { + t.Fatalf("cached Get: %v", err) + } + if cached != result.snapshot { + t.Fatalf("cached snapshot = %#v, want %#v", cached, result.snapshot) + } + }) +} + +func newOverviewCacheForTest(provider OverviewProvider, now func() time.Time) *OverviewCache { + cache := NewOverviewCache(provider) + cache.now = now + return cache +} + +type countingOverview struct { + mu sync.Mutex + calls int + err error + started chan struct{} + release chan struct{} +} + +func (o *countingOverview) Summary(context.Context) (Summary, error) { + o.mu.Lock() + o.calls++ + call := o.calls + err := o.err + started, release := o.started, o.release + o.mu.Unlock() + if started != nil { + close(started) + <-release + } + if err != nil { + return Summary{}, err + } + return Summary{Workspaces: call}, nil +} + +func (o *countingOverview) Capabilities(context.Context, StorageMaintenanceSettings) Capabilities { + return Capabilities{} +} + +type overviewResult struct { + snapshot OverviewSnapshot + err error +} + +type cancellableOverview struct { + calls int + started chan struct{} + release chan struct{} +} + +type firstCallBlockingOverview struct { + mu sync.Mutex + calls int + started chan struct{} + release chan struct{} +} + +func (o *firstCallBlockingOverview) Summary(context.Context) (Summary, error) { + o.mu.Lock() + o.calls++ + call := o.calls + o.mu.Unlock() + if call == 1 { + close(o.started) + <-o.release + } + return Summary{Workspaces: call}, nil +} + +func (*firstCallBlockingOverview) Capabilities( + context.Context, + StorageMaintenanceSettings, +) Capabilities { + return Capabilities{} +} + +func (o *cancellableOverview) Summary(ctx context.Context) (Summary, error) { + o.calls++ + close(o.started) + select { + case <-o.release: + return Summary{Workspaces: o.calls}, nil + case <-ctx.Done(): + return Summary{}, ctx.Err() + } +} + +func (o *cancellableOverview) Capabilities(context.Context, StorageMaintenanceSettings) Capabilities { + return Capabilities{} +} diff --git a/apps/backend/internal/system/storage/runner.go b/apps/backend/internal/system/storage/runner.go index aefd5a6e25..009918faed 100644 --- a/apps/backend/internal/system/storage/runner.go +++ b/apps/backend/internal/system/storage/runner.go @@ -31,6 +31,7 @@ type RunnerConfig struct { Activity *activity.Coordinator Store RunStore Providers []CleanupProvider + Overview OverviewInvalidator NewID func() string Now func() time.Time } @@ -39,6 +40,7 @@ type Runner struct { activity *activity.Coordinator store RunStore providers []CleanupProvider + overview OverviewInvalidator newID func() string now func() time.Time } @@ -62,7 +64,7 @@ func NewRunner(config RunnerConfig) *Runner { } return &Runner{ activity: config.Activity, store: config.Store, providers: config.Providers, - newID: newID, now: now, + overview: config.Overview, newID: newID, now: now, } } @@ -174,6 +176,9 @@ func (r *Runner) finishRun( if err != nil { return MaintenanceRun{}, err } + if state == RunStateSucceeded && r.overview != nil { + r.overview.Invalidate() + } return run, runErr } diff --git a/apps/backend/internal/system/storage_routes_test.go b/apps/backend/internal/system/storage_routes_test.go index 48ee2ed66e..e3a64a31e4 100644 --- a/apps/backend/internal/system/storage_routes_test.go +++ b/apps/backend/internal/system/storage_routes_test.go @@ -194,8 +194,8 @@ func (f *fakeStorageMutations) DeleteQuarantine(context.Context, string, string) type emptyStorageOverview struct{} -func (emptyStorageOverview) Summary(context.Context) (storage.Summary, error) { - return storage.Summary{}, nil +func (emptyStorageOverview) Get(context.Context) (storage.OverviewSnapshot, error) { + return storage.OverviewSnapshot{Summary: storage.Summary{}}, nil } func (emptyStorageOverview) Capabilities(context.Context, storage.StorageMaintenanceSettings) storage.Capabilities { diff --git a/apps/web/components/settings/system/storage/storage-maintenance-settings.test.tsx b/apps/web/components/settings/system/storage/storage-maintenance-settings.test.tsx index 68f2841b8d..670d62d456 100644 --- a/apps/web/components/settings/system/storage/storage-maintenance-settings.test.tsx +++ b/apps/web/components/settings/system/storage/storage-maintenance-settings.test.tsx @@ -11,6 +11,7 @@ const mocks = vi.hoisted(() => ({ useSystemJob: vi.fn(), })); const IDLE_PERIOD_TEST_ID = "storage-idle-period"; +const SAVE_BUTTON_NAME = "Save changes"; vi.mock("@/hooks/domains/system/use-storage-maintenance", async (importOriginal) => ({ ...(await importOriginal()), @@ -60,6 +61,7 @@ const overview = { managed_container_bytes: 0, }, }, + analyzed_at: "2026-07-23T12:00:00Z", last_run: null, } satisfies StorageOverviewResponse; @@ -139,6 +141,60 @@ describe("StorageMaintenanceSettings", () => { expect(analyzeButton.disabled).toBe(true); }); + it.each(["analyze", "run", "restore", "delete"] as const)( + "keeps policy editing and shared Save available while %s is pending", + async (pendingAction) => { + const editableOverview = { ...overview, settings: { ...overview.settings, enabled: true } }; + const currentController = { ...controller(editableOverview), pendingAction }; + mocks.useStorageMaintenance.mockReturnValue(currentController); + render(, { wrapper: Providers }); + + const idlePeriod = screen.getByTestId(IDLE_PERIOD_TEST_ID) as HTMLInputElement; + expect(idlePeriod.disabled).toBe(false); + fireEvent.change(idlePeriod, { target: { value: "31" } }); + + const save = screen.getByRole("button", { name: SAVE_BUTTON_NAME }) as HTMLButtonElement; + expect(save.disabled).toBe(false); + fireEvent.click(save); + await waitFor(() => + expect(currentController.save).toHaveBeenCalledWith( + { ...editableOverview.settings, idle_for_minutes: 31 }, + undefined, + ), + ); + }, + ); + + it("blocks a pending adoption with its specific save reason", () => { + mocks.useStorageMaintenance.mockReturnValue({ + ...controller(overview), + pendingAction: "adopt", + }); + render(, { wrapper: Providers }); + + fireEvent.change(screen.getByTestId(IDLE_PERIOD_TEST_ID), { target: { value: "31" } }); + + expect( + (screen.getByRole("button", { name: SAVE_BUTTON_NAME }) as HTMLButtonElement).disabled, + ).toBe(true); + expect(screen.getByText("Wait for Go cache adoption to finish.")).toBeTruthy(); + }); + + it("keeps the contributor valid while its own save is pending", () => { + const editableOverview = { ...overview, settings: { ...overview.settings, enabled: true } }; + mocks.useStorageMaintenance.mockReturnValue({ + ...controller(editableOverview), + pendingAction: "save", + }); + render(, { wrapper: Providers }); + + fireEvent.change(screen.getByTestId(IDLE_PERIOD_TEST_ID), { target: { value: "31" } }); + + expect( + (screen.getByRole("button", { name: SAVE_BUTTON_NAME }) as HTMLButtonElement).disabled, + ).toBe(false); + }); + it("preserves a dirty policy draft when refreshed overview data arrives", () => { const { rerender } = render(, { wrapper: Providers }); const idlePeriod = screen.getByTestId(IDLE_PERIOD_TEST_ID) as HTMLInputElement; @@ -156,6 +212,65 @@ describe("StorageMaintenanceSettings", () => { }); }); +describe("StorageMaintenanceSettings pending policy", () => { + afterEach(cleanup); + + it("rebases an adopted Go cache path into a dirty policy draft before shared save", async () => { + const currentController = controller(overview); + mocks.useStorageMaintenance.mockReturnValue(currentController); + const { rerender } = render(, { wrapper: Providers }); + fireEvent.change(screen.getByTestId(IDLE_PERIOD_TEST_ID), { target: { value: "31" } }); + + const adoptedPath = "/mnt/shared/go-build"; + currentController.overview = { + ...overview, + settings: { + ...overview.settings, + go_cache: { ...overview.settings.go_cache, adopted_path: adoptedPath }, + }, + }; + rerender(); + + fireEvent.click(screen.getByRole("button", { name: SAVE_BUTTON_NAME })); + await waitFor(() => + expect(currentController.save).toHaveBeenCalledWith( + { + ...overview.settings, + idle_for_minutes: 31, + go_cache: { ...overview.settings.go_cache, adopted_path: adoptedPath }, + }, + undefined, + ), + ); + }); + + it.each(["analyze", "run", "restore", "delete"] as const)( + "disables storage action controls while %s is pending without disabling shared Save", + (pendingAction) => { + const currentController = { ...controller(overview), pendingAction }; + mocks.useStorageMaintenance.mockReturnValue(currentController); + render(, { wrapper: Providers }); + + expect((screen.getByTestId("storage-analyze") as HTMLButtonElement).disabled).toBe(true); + expect((screen.getByTestId("storage-run-now") as HTMLButtonElement).disabled).toBe(true); + fireEvent.change(screen.getByTestId(IDLE_PERIOD_TEST_ID), { target: { value: "31" } }); + expect( + (screen.getByRole("button", { name: SAVE_BUTTON_NAME }) as HTMLButtonElement).disabled, + ).toBe(false); + }, + ); + + it("blocks Go cache adoption while a save is pending", () => { + mocks.useStorageMaintenance.mockReturnValue({ + ...controller(overview), + pendingAction: "save", + }); + render(, { wrapper: Providers }); + + expect((screen.getByTestId("storage-go-cache-adopt") as HTMLButtonElement).disabled).toBe(true); + }); +}); + describe("StorageMaintenanceSettings coordinated save", () => { afterEach(cleanup); @@ -178,7 +293,7 @@ describe("StorageMaintenanceSettings coordinated save", () => { screen.getByTestId("storage-policy-section-schedule").getAttribute("data-settings-dirty"), ).toBe("true"); - fireEvent.click(screen.getByRole("button", { name: "Save changes" })); + fireEvent.click(screen.getByRole("button", { name: SAVE_BUTTON_NAME })); await waitFor(() => expect(currentController.save).toHaveBeenCalledWith( { ...overview.settings, idle_for_minutes: 31 }, @@ -202,7 +317,7 @@ describe("StorageMaintenanceSettings coordinated save", () => { expect(screen.getByTestId("storage-docker-dedicated").getAttribute("data-settings-dirty")).toBe( "true", ); - fireEvent.click(screen.getByRole("button", { name: "Save changes" })); + fireEvent.click(screen.getByRole("button", { name: SAVE_BUTTON_NAME })); await waitFor(() => expect(currentController.save).toHaveBeenCalledWith( diff --git a/apps/web/components/settings/system/storage/storage-maintenance-settings.tsx b/apps/web/components/settings/system/storage/storage-maintenance-settings.tsx index 73cd34c972..f80b5ec459 100644 --- a/apps/web/components/settings/system/storage/storage-maintenance-settings.tsx +++ b/apps/web/components/settings/system/storage/storage-maintenance-settings.tsx @@ -127,6 +127,16 @@ function serializeSettings(settings: Settings | null): string { return settings ? JSON.stringify(settings) : "loading"; } +function policyPendingAction(action: ReturnType["pendingAction"]) { + return action === "load" || action === "save" || action === "adopt"; +} + +function policyBlockedReason(action: ReturnType["pendingAction"]) { + if (action === "adopt") return "Wait for Go cache adoption to finish."; + if (action === "load") return "Wait for storage settings to finish loading."; + return undefined; +} + function useStoragePolicyDraft(controller: ReturnType) { const [draft, setDraft] = useState(null); const previousServerSettings = useRef(null); @@ -139,20 +149,23 @@ function useStoragePolicyDraft(controller: ReturnType { if (!draft || !savedSettings) return; const confirmation = @@ -173,12 +186,14 @@ function useStoragePolicyDraft(controller: ReturnType - + {controller.error && ( @@ -191,7 +206,7 @@ export function StorageMaintenanceSettings() {
void controller.runNow(["go_cache"])} /> {draft && savedSettings && controller.overview && ( @@ -199,7 +214,7 @@ export function StorageMaintenanceSettings() { settings={draft} savedSettings={savedSettings} capabilities={controller.overview.capabilities} - pending={pending} + pending={controlsPending} onChange={setDraft} onAdopt={controller.adopt} /> @@ -209,7 +224,7 @@ export function StorageMaintenanceSettings() { diff --git a/apps/web/components/settings/system/storage/storage-overview-card.test.tsx b/apps/web/components/settings/system/storage/storage-overview-card.test.tsx index 5d6b2b6c92..07d8076922 100644 --- a/apps/web/components/settings/system/storage/storage-overview-card.test.tsx +++ b/apps/web/components/settings/system/storage/storage-overview-card.test.tsx @@ -41,12 +41,39 @@ const degradedOverview = { managed_container_bytes: 0, }, }, + analyzed_at: "2026-07-23T12:00:00Z", last_run: null, } satisfies StorageOverviewResponse; afterEach(cleanup); describe("StorageOverviewCard", () => { + it("shows the relative age and absolute time of the returned snapshot", () => { + const analyzedAt = "2026-07-23T11:58:00Z"; + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-07-23T12:00:00Z")); + + render( + , + ); + + const timestamp = screen.getByText("Last analyzed 2m ago"); + expect(timestamp.tagName).toBe("TIME"); + expect(timestamp.getAttribute("dateTime")).toBe(analyzedAt); + expect(timestamp.getAttribute("title")).toBe(new Date(analyzedAt).toLocaleString()); + vi.useRealTimers(); + }); + + it("shows a loading spinner while the overview is unavailable", () => { + render(); + + expect(screen.getByRole("status", { name: "Loading" })).toBeTruthy(); + expect(screen.getByText("Loading storage data…")).toBeTruthy(); + }); + it("renders a degraded quarantine warning without inventing zero usage", () => { render(); diff --git a/apps/web/components/settings/system/storage/storage-overview-card.tsx b/apps/web/components/settings/system/storage/storage-overview-card.tsx index 077ecfd64c..1e083f5900 100644 --- a/apps/web/components/settings/system/storage/storage-overview-card.tsx +++ b/apps/web/components/settings/system/storage/storage-overview-card.tsx @@ -1,8 +1,10 @@ import { Accordion, AccordionContent, AccordionItem, AccordionTrigger } from "@kandev/ui/accordion"; import { Badge } from "@kandev/ui/badge"; import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@kandev/ui/card"; +import { Spinner } from "@kandev/ui/spinner"; import { IconChartPie, IconTrash } from "@tabler/icons-react"; import type { StorageOverviewResponse, StorageQuarantineSummary } from "@/lib/types/system"; +import { formatRelativeTime } from "@/lib/utils"; import { StorageActionButton } from "./storage-action-button"; import { formatGigabytes } from "./storage-units"; @@ -173,13 +175,15 @@ export function StorageOverviewCard({ overview, disabledReason, onRunGoCache }: if (!overview) { return ( - + + Loading storage data… ); } const { summary } = overview; + const analyzedAt = new Date(overview.analyzed_at).toLocaleString(); const cleanupDisabledReason = goCacheDisabledReason(overview, disabledReason); const resources = storageResources(overview); return ( @@ -193,6 +197,14 @@ export function StorageOverviewCard({ overview, disabledReason, onRunGoCache }: A read-only breakdown of current usage and reclaimable space. Run Analyze occasionally to refresh these estimates; it never deletes or moves anything. + diff --git a/apps/web/components/settings/system/storage/storage-policy-card.test.tsx b/apps/web/components/settings/system/storage/storage-policy-card.test.tsx index 0b161eadea..d83ac98d1a 100644 --- a/apps/web/components/settings/system/storage/storage-policy-card.test.tsx +++ b/apps/web/components/settings/system/storage/storage-policy-card.test.tsx @@ -37,6 +37,7 @@ const testIds = { dockerBuildCacheUnused: "storage-docker-build-cache-unused-hours", dockerImagesUnused: "storage-docker-unused-images-hours", }; +const ADOPTION_PATH_TEST_ID = "storage-go-cache-adopt-path"; afterEach(cleanup); @@ -44,12 +45,13 @@ function renderCard( pending = false, onChange = vi.fn(), currentSettings: StorageMaintenanceSettings = settings, + savedSettings: StorageMaintenanceSettings = settings, ) { render( { + it("hydrates the external cache path from persisted settings", () => { + const persistedSettings = { + ...settings, + go_cache: { ...settings.go_cache, adopted_path: "/var/cache/go-build" }, + }; + renderCard(false, vi.fn(), persistedSettings, persistedSettings); + + expect((screen.getByTestId(ADOPTION_PATH_TEST_ID) as HTMLInputElement).value).toBe( + "/var/cache/go-build", + ); + }); + + it("follows a persisted external cache path change while the input is clean", () => { + const firstSettings = { + ...settings, + go_cache: { ...settings.go_cache, adopted_path: "/var/cache/first" }, + }; + const secondSettings = { + ...settings, + go_cache: { ...settings.go_cache, adopted_path: "/var/cache/second" }, + }; + const { rerender } = render( + + + , + ); + + rerender( + + + , + ); + + expect((screen.getByTestId(ADOPTION_PATH_TEST_ID) as HTMLInputElement).value).toBe( + "/var/cache/second", + ); + }); + + it("preserves an external cache path being edited during a persisted update", () => { + const firstSettings = { + ...settings, + go_cache: { ...settings.go_cache, adopted_path: "/var/cache/first" }, + }; + const secondSettings = { + ...settings, + go_cache: { ...settings.go_cache, adopted_path: "/var/cache/second" }, + }; + const { rerender } = render( + + + , + ); + fireEvent.change(screen.getByTestId(ADOPTION_PATH_TEST_ID), { + target: { value: "/var/cache/draft" }, + }); + + rerender( + + + , + ); + + expect((screen.getByTestId(ADOPTION_PATH_TEST_ID) as HTMLInputElement).value).toBe( + "/var/cache/draft", + ); + }); +}); + describe("StoragePolicyCard", () => { it("edits every Docker cleanup threshold", () => { const onChange = renderCard(); diff --git a/apps/web/components/settings/system/storage/storage-policy-card.tsx b/apps/web/components/settings/system/storage/storage-policy-card.tsx index d545d69771..2ddb8ebb62 100644 --- a/apps/web/components/settings/system/storage/storage-policy-card.tsx +++ b/apps/web/components/settings/system/storage/storage-policy-card.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { Input } from "@kandev/ui/input"; import { Label } from "@kandev/ui/label"; import { Switch } from "@kandev/ui/switch"; @@ -513,7 +513,18 @@ export function StoragePolicyCard({ }: Props) { const [dockerDialogOpen, setDockerDialogOpen] = useState(false); const [adoptionDialogOpen, setAdoptionDialogOpen] = useState(false); - const [adoptionPath, setAdoptionPath] = useState(""); + const savedAdoptionPath = savedSettings.go_cache.adopted_path; + const [adoptionPath, setAdoptionPath] = useState(savedAdoptionPath); + const previousSavedAdoptionPath = useRef(savedAdoptionPath); + + useEffect(() => { + const previousPath = previousSavedAdoptionPath.current; + setAdoptionPath((currentPath) => + currentPath === previousPath ? savedAdoptionPath : currentPath, + ); + previousSavedAdoptionPath.current = savedAdoptionPath; + }, [savedAdoptionPath]); + return (
diff --git a/apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts b/apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts index 7cc3fa5355..925960c5a6 100644 --- a/apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts +++ b/apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts @@ -1,4 +1,5 @@ import fs from "node:fs"; +import type { Route } from "@playwright/test"; import { test, expect } from "../../fixtures/test-base"; import { seedManagedGoCache } from "../../helpers/storage-maintenance"; import { MobileKanbanPage } from "../../pages/mobile-kanban-page"; @@ -9,6 +10,8 @@ test.describe("Mobile storage maintenance", () => { backend, }) => { const cache = seedManagedGoCache(backend.tmpDir); + const externalGoCache = `${backend.tmpDir}/external-go-cache`; + fs.mkdirSync(externalGoCache, { recursive: true }); const mobile = new MobileKanbanPage(testPage); await mobile.goto(); await mobile.mobileMenuButton.click(); @@ -27,6 +30,7 @@ test.describe("Mobile storage maintenance", () => { ); await testPage.keyboard.press("Escape"); await testPage.getByTestId("storage-scheduling-enabled").click(); + await testPage.getByTestId("storage-go-cache-enabled").click(); await testPage.getByTestId("storage-idle-period").fill("12"); await expect(testPage.getByTestId("storage-policy-section-schedule")).toHaveAttribute( "data-settings-dirty", @@ -35,6 +39,8 @@ test.describe("Mobile storage maintenance", () => { await expect(testPage.getByTestId("settings-floating-save")).toBeVisible(); await testPage.getByRole("button", { name: "Save changes" }).click(); await expect(testPage.getByText("Storage policy saved")).toBeVisible(); + const analyzedTime = testPage.locator("time[datetime]").filter({ hasText: "Last analyzed" }); + await expect(analyzedTime).toHaveText(/^Last analyzed .+/); await testPage.getByTestId("storage-analyze").click(); await expect(testPage.getByTestId("storage-analyze")).toHaveAttribute( "data-job-state", @@ -56,6 +62,13 @@ test.describe("Mobile storage maintenance", () => { await testPage.getByTestId("storage-go-cache-clean").click(); expect((await explicitRequest).postDataJSON()).toEqual({ resources: ["go_cache"] }); await expect.poll(() => fs.existsSync(cache.artifact)).toBe(false); + await testPage.getByTestId("storage-go-cache-adopt-path").fill(externalGoCache); + await testPage.getByTestId("storage-go-cache-adopt").click(); + await testPage.getByTestId("storage-go-cache-adopt-confirm-confirmation").fill("ADOPT"); + await testPage.getByTestId("storage-go-cache-adopt-confirm").click(); + await expect(testPage.getByText("Go build cache adopted")).toBeVisible(); + await testPage.reload(); + await expect(testPage.getByTestId("storage-go-cache-adopt-path")).toHaveValue(externalGoCache); await testPage.getByRole("button", { name: "More information about Quarantine" }).click(); await expect(testPage.getByRole("tooltip")).toContainText("recoverable holding area"); await expect @@ -64,4 +77,53 @@ test.describe("Mobile storage maintenance", () => { ) .toBe(true); }); + + test("shows progress while storage data is loading", async ({ testPage }) => { + let overviewRequestStarted = false; + let releaseOverview: () => void = () => {}; + let markOverviewObserved: () => void = () => {}; + let markOverviewSettled: () => void = () => {}; + const overviewGate = new Promise((resolve) => { + releaseOverview = resolve; + }); + const overviewObserved = new Promise((resolve) => { + markOverviewObserved = resolve; + }); + const overviewSettled = new Promise((resolve) => { + markOverviewSettled = resolve; + }); + const overviewPattern = "**/api/v1/system/storage"; + const holdOverview = async (route: Route) => { + overviewRequestStarted = true; + markOverviewObserved(); + await overviewGate; + try { + await route.continue(); + } finally { + markOverviewSettled(); + } + }; + + await testPage.route(overviewPattern, holdOverview); + try { + await testPage.goto("/settings/system/storage"); + await overviewObserved; + + const spinner = testPage.getByTestId("storage-overview-spinner"); + await expect(spinner).toBeVisible(); + await expect(testPage.getByText("Loading storage data…")).toBeVisible(); + await expect(testPage.getByTestId("storage-overview-card")).toBeInViewport(); + expect( + await testPage.evaluate(() => document.documentElement.scrollWidth > window.innerWidth), + ).toBe(false); + + releaseOverview(); + await expect(spinner).toBeHidden(); + await expect(testPage.getByText("Storage analysis")).toBeVisible(); + } finally { + releaseOverview(); + if (overviewRequestStarted) await overviewSettled; + await testPage.unroute(overviewPattern, holdOverview); + } + }); }); diff --git a/apps/web/e2e/tests/system/storage-maintenance.spec.ts b/apps/web/e2e/tests/system/storage-maintenance.spec.ts index 12c9d0ab17..ce7f59bc19 100644 --- a/apps/web/e2e/tests/system/storage-maintenance.spec.ts +++ b/apps/web/e2e/tests/system/storage-maintenance.spec.ts @@ -64,6 +64,39 @@ test.describe("System storage maintenance", () => { await expect.poll(() => fs.existsSync(cache.artifact)).toBe(false); }); + test("reuses the cached snapshot until Analyze refreshes it", async ({ testPage }) => { + await testPage.goto("/settings/system/storage"); + const analyzedTime = testPage.locator("time[datetime]").filter({ hasText: "Last analyzed" }); + await expect(analyzedTime).toHaveText(/^Last analyzed .+/); + const initialAnalyzedAt = await analyzedTime.getAttribute("datetime"); + expect(initialAnalyzedAt).toBeTruthy(); + + await testPage.reload(); + await expect(analyzedTime).toHaveAttribute("datetime", initialAnalyzedAt!); + + const scheduling = testPage.getByTestId("storage-scheduling-enabled"); + const initialSchedulingState = await scheduling.getAttribute("data-state"); + try { + await scheduling.click(); + await testPage.getByRole("button", { name: "Save changes" }).click(); + await expect(testPage.getByText("Storage policy saved")).toBeVisible(); + await expect(analyzedTime).toHaveAttribute("datetime", initialAnalyzedAt!); + + await testPage.getByTestId("storage-analyze").click(); + await expect(testPage.getByTestId("storage-analyze")).toHaveAttribute( + "data-job-state", + "succeeded", + ); + await expect.poll(() => analyzedTime.getAttribute("datetime")).not.toBe(initialAnalyzedAt); + } finally { + if ((await scheduling.getAttribute("data-state")) !== initialSchedulingState) { + await scheduling.click(); + await testPage.getByRole("button", { name: "Save changes" }).click(); + await expect(testPage.getByText("Storage policy saved")).toBeVisible(); + } + } + }); + test("persists policy and analyzes, quarantines, and restores an orphan workspace", async ({ testPage, backend, diff --git a/apps/web/hooks/domains/system/use-storage-maintenance.test.tsx b/apps/web/hooks/domains/system/use-storage-maintenance.test.tsx index 3faf495765..6c14fc3ea8 100644 --- a/apps/web/hooks/domains/system/use-storage-maintenance.test.tsx +++ b/apps/web/hooks/domains/system/use-storage-maintenance.test.tsx @@ -80,16 +80,26 @@ const overview: StorageOverviewResponse = { managed_container_bytes: 60, }, }, + analyzed_at: "2026-07-23T12:00:00Z", last_run: null, }; +const cleanupJobId = "cleanup-job"; const cleanupJob = { - id: "cleanup-job", + id: cleanupJobId, kind: "storage-cleanup", state: "running", started_at: "2026-07-15T00:00:00Z", }; +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +} + function wrapper({ children }: { children: ReactNode }) { return {children}; } @@ -102,7 +112,7 @@ beforeEach(() => { mocks.fetchJob.mockResolvedValue(cleanupJob); mocks.save.mockResolvedValue({ settings }); // Keep cleanup jobs deterministic for controller action tests. - mocks.run.mockResolvedValue({ job_id: "cleanup-job" }); + mocks.run.mockResolvedValue({ job_id: cleanupJobId }); }); describe("useStorageMaintenance", () => { @@ -161,7 +171,7 @@ describe("useStorageMaintenance", () => { await act(async () => { await result.current.runNow(); }); - await waitFor(() => expect(result.current.cleanupJob?.id).toBe("cleanup-job")); + await waitFor(() => expect(result.current.cleanupJob?.id).toBe(cleanupJobId)); mocks.run.mockRejectedValueOnce(new Error("storage maintenance is busy")); await act(async () => { @@ -173,6 +183,66 @@ describe("useStorageMaintenance", () => { }); }); +describe("useStorageMaintenance pending action tracking", () => { + it("returns to a pending resource action after an overlapping save finishes", async () => { + const pendingRun = deferred<{ job_id: string }>(); + const pendingSave = deferred<{ settings: StorageMaintenanceSettings }>(); + mocks.run.mockReturnValueOnce(pendingRun.promise); + mocks.save.mockReturnValueOnce(pendingSave.promise); + const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); + await waitFor(() => expect(result.current.overview).toEqual(overview)); + + let runPromise!: Promise; + let savePromise!: Promise; + await act(async () => { + runPromise = result.current.runNow(); + savePromise = result.current.save(settings); + }); + await waitFor(() => expect(result.current.pendingAction).toBe("save")); + + await act(async () => { + pendingSave.resolve({ settings }); + await savePromise; + }); + expect(result.current.pendingAction).toBe("run"); + + await act(async () => { + pendingRun.resolve({ job_id: cleanupJobId }); + await runPromise; + }); + expect(result.current.pendingAction).toBeNull(); + }); + + it("keeps an overlapping save pending when the resource request finishes first", async () => { + const pendingRun = deferred<{ job_id: string }>(); + const pendingSave = deferred<{ settings: StorageMaintenanceSettings }>(); + mocks.run.mockReturnValueOnce(pendingRun.promise); + mocks.save.mockReturnValueOnce(pendingSave.promise); + const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); + await waitFor(() => expect(result.current.overview).toEqual(overview)); + + let runPromise!: Promise; + let savePromise!: Promise; + await act(async () => { + runPromise = result.current.runNow(); + savePromise = result.current.save(settings); + }); + await waitFor(() => expect(result.current.pendingAction).toBe("save")); + + await act(async () => { + pendingRun.resolve({ job_id: cleanupJobId }); + await runPromise; + }); + expect(result.current.pendingAction).toBe("save"); + + await act(async () => { + pendingSave.resolve({ settings }); + await savePromise; + }); + expect(result.current.pendingAction).toBeNull(); + }); +}); + describe("useStorageMaintenance terminal refresh", () => { it("surfaces and retries a failed refresh after a cleanup job finishes", async () => { mocks.fetchJob.mockResolvedValue({ diff --git a/apps/web/hooks/domains/system/use-storage-maintenance.ts b/apps/web/hooks/domains/system/use-storage-maintenance.ts index 7b8a2d16ca..2134202b2b 100644 --- a/apps/web/hooks/domains/system/use-storage-maintenance.ts +++ b/apps/web/hooks/domains/system/use-storage-maintenance.ts @@ -3,6 +3,7 @@ import { useCallback, useEffect, + useMemo, useRef, useState, type Dispatch, @@ -65,16 +66,28 @@ const MAX_TERMINAL_REFRESH_ATTEMPTS = 6; function useStorageActionRunner() { const { toast } = useToast(); - const [pendingAction, setPendingAction] = useState("load"); + const [loading, setLoading] = useState(true); + const [pendingActions, setPendingActions] = useState< + Array<{ id: number; action: Exclude }> + >([]); + const nextPendingActionId = useRef(0); const [error, setError] = useState(null); - const finishLoading = useCallback(() => setPendingAction(null), []); + const finishLoading = useCallback(() => setLoading(false), []); + const pendingAction = useMemo(() => { + if (loading) return "load"; + const policyAction = pendingActions.findLast( + ({ action }) => action === "save" || action === "adopt", + ); + return policyAction?.action ?? pendingActions[0]?.action ?? null; + }, [loading, pendingActions]); const perform = useCallback( async ( action: Exclude, work: () => Promise, rethrow = false, ) => { - setPendingAction(action); + const pendingActionId = nextPendingActionId.current++; + setPendingActions((current) => [...current, { id: pendingActionId, action }]); setError(null); try { await work(); @@ -84,7 +97,7 @@ function useStorageActionRunner() { toast({ title: "Storage action failed", description: message, variant: "error" }); if (rethrow) throw requestError; } finally { - setPendingAction(null); + setPendingActions((current) => current.filter(({ id }) => id !== pendingActionId)); } }, [toast], diff --git a/apps/web/lib/api/domains/system-api.test.ts b/apps/web/lib/api/domains/system-api.test.ts index d44726c6fe..de97636eeb 100644 --- a/apps/web/lib/api/domains/system-api.test.ts +++ b/apps/web/lib/api/domains/system-api.test.ts @@ -334,7 +334,13 @@ describe("storage maintenance", () => { it("loads overview and list resources without caching", async () => { fetchSpy .mockResolvedValueOnce( - jsonResponse({ settings, summary: {}, capabilities: {}, last_run: null }), + jsonResponse({ + settings, + summary: {}, + capabilities: {}, + analyzed_at: "2026-07-23T12:00:00Z", + last_run: null, + }), ) .mockResolvedValueOnce(jsonResponse({ runs: [{ id: "run-1" }] })) .mockResolvedValueOnce(jsonResponse({ entries: [{ id: "entry-1" }] })); diff --git a/apps/web/lib/state/slices/system/system-slice.test.ts b/apps/web/lib/state/slices/system/system-slice.test.ts index d5a9b4a267..c6cc613f67 100644 --- a/apps/web/lib/state/slices/system/system-slice.test.ts +++ b/apps/web/lib/state/slices/system/system-slice.test.ts @@ -131,6 +131,7 @@ describe("system storage slice", () => { managed_container_bytes: 0, }, }, + analyzed_at: "2026-07-23T12:00:00Z", last_run: null, } satisfies StorageOverviewResponse; store.getState().setSystemStorageOverview(overview); diff --git a/apps/web/lib/types/system.ts b/apps/web/lib/types/system.ts index 3487bc8208..2bc8d95918 100644 --- a/apps/web/lib/types/system.ts +++ b/apps/web/lib/types/system.ts @@ -291,6 +291,7 @@ export interface StorageOverviewResponse { settings: StorageMaintenanceSettings; capabilities: StorageCapabilities; summary: StorageSummary; + analyzed_at: string; last_run: StorageMaintenanceRun | null; } diff --git a/docs/plans/storage-overview-cache/plan.md b/docs/plans/storage-overview-cache/plan.md new file mode 100644 index 0000000000..cf5bfb0021 --- /dev/null +++ b/docs/plans/storage-overview-cache/plan.md @@ -0,0 +1,98 @@ +--- +spec: docs/specs/system-page/storage-maintenance.md +created: 2026-07-23 +status: implemented +--- + +# Implementation Plan: Storage Overview Cache and Settings Stability + +## Overview + +Put a concurrency-safe 15-minute cache in front of the existing typed overview providers, then +expose the snapshot timestamp through the existing Storage API. Manual Analyze uses the same cache +owner but forces a refresh. The frontend displays snapshot age, hydrates the external Go-cache path +from persisted settings, and separates policy-save blocking from unrelated analysis and cleanup +activity. + +## Backend + +### Cached overview contract + +- Add `apps/backend/internal/system/storage/overview_cache.go` with an `OverviewSnapshot` containing + `Summary` and `AnalyzedAt`, plus cached and forced-refresh methods around the existing provider. +- Use a 15-minute default TTL, cache only successful scans, and serialize concurrent cache misses so + page refreshes do not duplicate provider work. +- Keep the snapshot process-local. A backend restart starts with an empty cache. + +### API and composition + +- Update `apps/backend/internal/system/storage/handler.go` so `GET /storage` returns cached + `summary` and `analyzed_at`. +- Update `apps/backend/internal/system/storage/operations.go` so manual Analyze calls the forced + refresh path and records that summary in the existing run. +- Wrap `storageOverview` once in + `apps/backend/internal/backendapp/storage_maintenance.go`, then pass the same cache owner to the + handler and operations so forced analysis immediately updates subsequent GET responses. +- Update handler/composition fakes that implement the overview interface. + +## Frontend + +### Contracts and analysis card + +- Add `analyzed_at` to `StorageOverviewResponse` in `apps/web/lib/types/system.ts`. +- Update `apps/web/components/settings/system/storage/storage-overview-card.tsx` to render + “Last analyzed …” using the shared `formatRelativeTime` helper and an absolute timestamp tooltip. +- Preserve the existing loading spinner while the first post-startup snapshot is measured. + +### Policy and external Go cache + +- In `storage-policy-card.tsx`, initialize the external-cache input from + `savedSettings.go_cache.adopted_path` and synchronize later persisted changes without overwriting + an in-progress user edit. +- In `storage-maintenance-settings.tsx`, distinguish policy-mutating pending actions from analysis, + cleanup, restore, and delete jobs. Those jobs must not disable policy editing. +- Do not mark the contributor invalid because its own save is in progress. If adoption is the + conflicting mutation, show a specific adoption message rather than the generic current-action + text. + +## Tests + +- **Cache freshness and forcing:** `apps/backend/internal/system/storage/overview_cache_test.go` + verifies one source call within 15 minutes, refresh after expiry, forced refresh inside the TTL, + successful-snapshot timestamps, failed-refresh retention, and concurrent miss coalescing. +- **Handler/operations wiring:** focused tests in `handler_test.go` and `operations_test.go` verify + `analyzed_at` output and that manual Analyze uses the forced path. +- **External path hydration and save blocking:** existing Storage component tests verify the adopted + path survives rerender/reopen and analysis/cleanup do not disable policy controls or the shared + Save action. +- **Frontend contract:** update Storage fixtures and API tests for `analyzed_at`. + +## E2E Tests + +- Extend `apps/web/e2e/tests/system/storage-maintenance.spec.ts` to count overview measurements + across page refresh/save and verify manual Analyze produces a newer displayed timestamp. +- Extend `apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts` to verify the relative + analysis timestamp and persisted external-cache field remain usable without horizontal overflow. + +## Implementation Waves + +Wave 1: + +- [x] [Task 01: Backend cached overview](task-01-backend-cached-overview.md) + +Wave 2: + +- [x] [Task 02: Frontend settings stability](task-02-frontend-settings-stability.md) + +Wave 3: + +- [x] [Task 03: Integrated desktop and mobile coverage](task-03-integrated-e2e.md) + +## Risks + +- Summary values can intentionally lag newly saved thresholds or an adopted path until manual + Analyze or TTL expiry; `analyzed_at` makes that staleness visible. +- The cache must be shared by GET and manual Analyze. Separate wrappers would leave GET stale after + a forced scan. +- Pending-state changes must retain mutual exclusion for adoption and saving without coupling + policy controls to cleanup jobs that only consume a settings snapshot. diff --git a/docs/plans/storage-overview-cache/task-01-backend-cached-overview.md b/docs/plans/storage-overview-cache/task-01-backend-cached-overview.md new file mode 100644 index 0000000000..0bb6b61b68 --- /dev/null +++ b/docs/plans/storage-overview-cache/task-01-backend-cached-overview.md @@ -0,0 +1,50 @@ +--- +id: "01-backend-cached-overview" +title: "Backend cached overview" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/system-page/storage-maintenance.md" +--- + +# Task 01: Backend cached overview + +## Acceptance + +- Successful overview scans are reused for 15 minutes and concurrent cache misses invoke the typed + providers once. +- Manual Analyze bypasses the freshness window and replaces the shared successful snapshot. +- `GET /api/v1/system/storage` returns `analyzed_at` for the summary it serves. + +## Verification + +```bash +cd apps/backend && go test ./internal/system/storage ./internal/backendapp +``` + +## Files likely touched + +- `apps/backend/internal/system/storage/overview_cache.go` +- `apps/backend/internal/system/storage/overview_cache_test.go` +- `apps/backend/internal/system/storage/handler.go` +- `apps/backend/internal/system/storage/handler_test.go` +- `apps/backend/internal/system/storage/operations.go` +- `apps/backend/internal/system/storage/operations_test.go` +- `apps/backend/internal/backendapp/storage_maintenance.go` +- focused overview fakes in backend System route tests + +## Dependencies + +None. + +## Inputs + +- Spec: What, API surface, Persistence guarantees, and cache scenarios +- `apps/backend/internal/backendapp/storage_maintenance.go:storageOverview` +- Existing `storage.OverviewProvider`, handler GET, and manual Analyze call paths + +## Output contract + +Return a compact handoff capsule with acceptance status, changed entry points, focused test results, +risk tags, uncertainties, and set this task to `done`. diff --git a/docs/plans/storage-overview-cache/task-02-frontend-settings-stability.md b/docs/plans/storage-overview-cache/task-02-frontend-settings-stability.md new file mode 100644 index 0000000000..ce0a22ab61 --- /dev/null +++ b/docs/plans/storage-overview-cache/task-02-frontend-settings-stability.md @@ -0,0 +1,55 @@ +--- +id: "02-frontend-settings-stability" +title: "Frontend settings stability" +status: done +wave: 2 +depends_on: ["01-backend-cached-overview"] +plan: "plan.md" +spec: "../../specs/system-page/storage-maintenance.md" +--- + +# Task 02: Frontend settings stability + +## Acceptance + +- Storage analysis shows the relative age of the exact snapshot returned by the backend. +- The external Go-cache input hydrates from and follows the persisted adopted path without erasing + an in-progress edit. +- Analysis and cleanup do not disable policy editing; only conflicting settings mutations block + save, with a specific reason. + +## Verification + +```bash +cd apps && pnpm --filter @kandev/web test -- --run \ + components/settings/system/storage/storage-overview-card.test.tsx \ + components/settings/system/storage/storage-policy-card.test.tsx \ + components/settings/system/storage/storage-maintenance-settings.test.tsx +cd apps/web && pnpm run typecheck +``` + +## Files likely touched + +- `apps/web/lib/types/system.ts` +- Storage fixtures in `apps/web/lib/api/domains/system-api.test.ts` +- `apps/web/components/settings/system/storage/storage-overview-card.tsx` +- `apps/web/components/settings/system/storage/storage-overview-card.test.tsx` +- `apps/web/components/settings/system/storage/storage-policy-card.tsx` +- `apps/web/components/settings/system/storage/storage-policy-card.test.tsx` +- `apps/web/components/settings/system/storage/storage-maintenance-settings.tsx` +- `apps/web/components/settings/system/storage/storage-maintenance-settings.test.tsx` + +## Dependencies + +Task 01 defines the final `analyzed_at` API contract. + +## Inputs + +- Spec cache, relative timestamp, external Go-cache, and policy-editing scenarios +- Shared `formatRelativeTime` in `apps/web/lib/utils.ts` +- Existing coordinated-save contract in `settings-save-provider.tsx` + +## Output contract + +Return a compact handoff capsule with acceptance status, changed entry points, focused test results, +risk tags, uncertainties, and set this task to `done`. diff --git a/docs/plans/storage-overview-cache/task-03-integrated-e2e.md b/docs/plans/storage-overview-cache/task-03-integrated-e2e.md new file mode 100644 index 0000000000..bb508d36c0 --- /dev/null +++ b/docs/plans/storage-overview-cache/task-03-integrated-e2e.md @@ -0,0 +1,46 @@ +--- +id: "03-integrated-e2e" +title: "Integrated desktop and mobile coverage" +status: done +wave: 3 +depends_on: ["01-backend-cached-overview", "02-frontend-settings-stability"] +plan: "plan.md" +spec: "../../specs/system-page/storage-maintenance.md" +--- + +# Task 03: Integrated desktop and mobile coverage + +## Acceptance + +- A page refresh and policy save reuse the current overview snapshot while manual Analyze replaces + it and updates the displayed relative timestamp. +- The persisted external Go-cache path is visible after the Storage page reloads. +- Desktop and Pixel 5 flows remain usable without horizontal document overflow. + +## Verification + +```bash +cd apps/web && pnpm e2e:run --project chromium tests/system/storage-maintenance.spec.ts +cd apps/web && pnpm e2e:run --project mobile-chrome \ + tests/system/mobile-storage-maintenance.spec.ts +``` + +## Files likely touched + +- `apps/web/e2e/tests/system/storage-maintenance.spec.ts` +- `apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts` +- existing Storage E2E helpers only if needed for deterministic provider-call observation + +## Dependencies + +Tasks 01 and 02. + +## Inputs + +- Spec cache, external Go-cache, policy editing, and mobile scenarios +- Existing Storage desktop/mobile E2E flows + +## Output contract + +Return a compact handoff capsule with acceptance status, exact E2E commands/results, risk tags, +failure artifacts when applicable, uncertainties, and set this task to `done`. diff --git a/docs/public/operations.md b/docs/public/operations.md index d6bb41b45b..18de9e54bf 100644 --- a/docs/public/operations.md +++ b/docs/public/operations.md @@ -79,6 +79,11 @@ Open **Settings > System > Storage** to inspect Kandev-managed disk usage and co **Analyze** is read-only. **Run now** applies only the enabled cleanup rules and refuses to start while task resources are active or another maintenance run owns the cleanup gate. +Storage analysis results are cached in the running backend for 15 minutes, so page reloads and +policy saves reuse the displayed snapshot instead of scanning disk again. The page shows when that +snapshot was last analyzed. Select **Analyze** to force a fresh scan; restarting the backend also +clears the in-memory snapshot. + Scheduled cleanup is disabled by default and runs only after the configured resource-idle quiet period. Orphaned task workspaces move into Kandev's quarantine before permanent deletion; review the quarantine list to restore an entry or request deletion as a background job. Host-wide Docker diff --git a/docs/specs/system-page/storage-maintenance.md b/docs/specs/system-page/storage-maintenance.md index f5929f41c2..a6a560710d 100644 --- a/docs/specs/system-page/storage-maintenance.md +++ b/docs/specs/system-page/storage-maintenance.md @@ -1,6 +1,7 @@ --- status: shipped created: 2026-07-14 +updated: 2026-07-23 owner: cfl --- @@ -34,6 +35,14 @@ reclaim that space without maintaining cron or systemd configuration outside Kan bytes, the managed Go cache, the service user's default Go cache when it is a distinct path, Kandev-managed container count and writable-layer bytes, Docker image-layer bytes, Docker build cache, and unused Docker images. +- A successful storage analysis is reused for 15 minutes. Opening or refreshing the Storage page, + saving policy settings, and adopting an external Go cache consume that cached snapshot instead of + starting another filesystem or Docker scan. Manual **Analyze** always bypasses the cache and + replaces it with a fresh successful snapshot. +- The Storage analysis card shows when its snapshot was measured using a relative timestamp. + Policy editing remains available while read-only analysis or cleanup jobs run. A settings + mutation that can conflict with another settings mutation may block saving briefly, but the UI + names that operation instead of reporting an unspecified storage action. - Scheduled maintenance is install-wide, persists in Kandev's database, and is disabled by default. Enabling it does not require editing the VM, a systemd unit, or environment variables. @@ -128,6 +137,9 @@ Decision: [ADR-2026-07-19-workspace-symlink-entries](../../decisions/2026-07-19- - Kandev creates an ownership marker beside the managed cache. It never deletes the default user cache such as `/root/.cache/go-build` unless that exact path was explicitly adopted through the Storage page with a destructive confirmation. +- After adoption succeeds, the external Go-cache field shows the persisted + `go_cache.adopted_path` immediately and after page reload. Unrelated overview refreshes do not + erase a path the user is currently editing. - Analysis reports the managed cache's current bytes and read-only usage for the service user's distinct default Go cache (`$GOCACHE` when absolute, otherwise the platform user-cache path). Reporting the default cache does not adopt it or grant cleanup ownership. Cleanup rotates the @@ -278,7 +290,7 @@ All routes are under the existing authenticated System route group. ```text GET /api/v1/system/storage - -> { settings, capabilities, summary, last_run } + -> { settings, capabilities, summary, analyzed_at, last_run } PATCH /api/v1/system/storage/settings body: { @@ -317,6 +329,10 @@ DELETE /api/v1/system/storage/quarantine/:id availability, configured Docker host, and whether host-global Docker cleanup is allowed. API responses never expose secret environment values. +`analyzed_at` is the RFC 3339 timestamp of the successful analysis that produced `summary`. +`GET /storage` reuses that snapshot for 15 minutes. `POST /storage/analyze` bypasses the freshness +window and replaces the cached snapshot only when the forced analysis succeeds. + Storage operations use the existing `system.job.update` WebSocket event and polling fallback. Job kinds are `storage-analysis`, `storage-cleanup`, and `storage-quarantine-delete`. @@ -406,6 +422,9 @@ enabling host-global Docker cleanup require explicit UI confirmation and server- ## Persistence guarantees - Settings, cleanup intents, maintenance runs, and quarantine entries survive backend restarts. +- The 15-minute analysis snapshot is process-local and does not survive a backend restart. The first + Storage overview request after startup measures a new snapshot; later requests reuse it until it + expires or manual **Analyze** replaces it. - A scheduled loop starts only when `enabled=true`; startup does not immediately run destructive cleanup. The first scheduled run is eligible after one full configured interval. - Pending/retryable task cleanup resumes after startup independent of scheduled-maintenance @@ -422,6 +441,13 @@ enabling host-global Docker cleanup require explicit UI confirmation and server- daemon, **THEN** no destructive storage cleanup runs and the Storage page shows scheduling off. - **GIVEN** scheduling is disabled, **WHEN** the user selects **Analyze**, **THEN** the page shows reclaimable bytes without changing any filesystem or Docker resource. +- **GIVEN** a successful storage snapshot is less than 15 minutes old, **WHEN** the user refreshes + the page or saves policy settings, **THEN** the same summary and `analyzed_at` are returned without + invoking the storage providers again. +- **GIVEN** a cached storage snapshot of any age, **WHEN** the user selects **Analyze**, **THEN** all + analysis providers run and a successful result replaces the snapshot and `analyzed_at`. +- **GIVEN** a storage analysis or cleanup job is running, **WHEN** the user edits maintenance + policy, **THEN** the policy controls and shared Save action remain available. - **GIVEN** scheduling is enabled and a task is running a Go test, **WHEN** the maintenance interval arrives, **THEN** the run is recorded as `skipped_busy` and no provider changes resources. - **GIVEN** maintenance holds the idle gate, **WHEN** a new task launch arrives, **THEN** maintenance @@ -457,6 +483,8 @@ enabling host-global Docker cleanup require explicit UI confirmation and server- reports the reclaimed bytes. - **GIVEN** `/root/.cache/go-build` was not explicitly adopted, **WHEN** storage cleanup runs, **THEN** Kandev does not modify it. +- **GIVEN** an external Go cache was adopted successfully, **WHEN** the Storage page rerenders or is + reopened, **THEN** the external-cache input contains the persisted adopted path. - **GIVEN** `/root/.cache/go-build` is the service user's default Go cache and is not adopted, **WHEN** storage analysis runs, **THEN** its path and bytes are reported read-only while cleanup remains unavailable for that path. @@ -499,4 +527,5 @@ enabling host-global Docker cleanup require explicit UI confirmation and server- ## Implementation plan -See [the implementation plan](../../plans/storage-maintenance/plan.md). +- [Original Storage maintenance implementation](../../plans/storage-maintenance/plan.md) +- [Storage overview cache and settings follow-up](../../plans/storage-overview-cache/plan.md)