diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index 619c3eafd9..1bddce8d28 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -831,6 +831,7 @@ func startGatewayAndServe( }) storageComposition, err := provideStorageComposition( cfg, dbPool, systemSvc.Jobs, lifecycleMgr, services.WorktreeMgr, services.Task, + func(message string, err error) { log.Error(message, zap.Error(err)) }, ) if err != nil { log.Error("Failed to initialize storage maintenance", zap.Error(err)) diff --git a/apps/backend/internal/backendapp/storage_maintenance.go b/apps/backend/internal/backendapp/storage_maintenance.go index 4e216e7521..c4aeed5f16 100644 --- a/apps/backend/internal/backendapp/storage_maintenance.go +++ b/apps/backend/internal/backendapp/storage_maintenance.go @@ -36,6 +36,7 @@ func provideStorageComposition( lifecycleMgr *lifecycle.Manager, worktreeMgr *worktree.Manager, taskSvc *taskservice.Service, + logError func(string, error), ) (*storageComposition, error) { rawSettings, err := systemsettings.NewStore(pool) if err != nil { @@ -89,7 +90,7 @@ func provideStorageComposition( }) handler := storagepkg.NewHandler(storagepkg.HandlerConfig{ Settings: settings, Runs: store, Quarantine: store, Overview: cachedOverview, - Mutations: operations, OnSettingsChanged: runtime.ApplySettings, + Mutations: operations, OnSettingsChanged: runtime.ApplySettings, LogError: logError, }) return &storageComposition{ handler: handler, runtime: runtime, workspaceRestorer: quarantine, @@ -167,11 +168,18 @@ func (o *storageOverview) Capabilities( ctx context.Context, settings storagepkg.StorageMaintenanceSettings, ) storagepkg.Capabilities { - dockerAvailable := o.dockerClient.Ping(ctx) == nil + return o.SettingsCapabilities(ctx, settings) +} + +func (o *storageOverview) SettingsCapabilities( + ctx context.Context, + settings storagepkg.StorageMaintenanceSettings, +) storagepkg.Capabilities { goPath := settings.GoCache.AdoptedPath if goPath == "" { goPath = filepath.Join(o.homeDir, "cache", "go-build") } + dockerAvailable := o.dockerClient != nil && o.dockerClient.Ping(ctx) == nil return storagepkg.Capabilities{ ManagedGoCachePath: goPath, GoCacheAdoptionAvailable: true, DockerAvailable: dockerAvailable, DockerHost: o.dockerHost, diff --git a/apps/backend/internal/gateway/websocket/task_notifications_test.go b/apps/backend/internal/gateway/websocket/task_notifications_test.go index 9654dee183..1c36bc0e5f 100644 --- a/apps/backend/internal/gateway/websocket/task_notifications_test.go +++ b/apps/backend/internal/gateway/websocket/task_notifications_test.go @@ -107,7 +107,7 @@ func TestTaskEventBroadcaster_NoDuplicateSubscriptions(t *testing.T) { // // Update this number when adding or removing event subscriptions in // RegisterTaskNotifications — it is intentionally exact. - const wantSubscriptions = 61 + const wantSubscriptions = 62 if got := len(b.subscriptions); got != wantSubscriptions { t.Errorf("RegisterTaskNotifications created %d subscriptions, want %d — "+ "did an event get subscribed twice?", got, wantSubscriptions) diff --git a/apps/backend/internal/integration/mcp_stop_task_test.go b/apps/backend/internal/integration/mcp_stop_task_test.go index 307889650e..4ec298730e 100644 --- a/apps/backend/internal/integration/mcp_stop_task_test.go +++ b/apps/backend/internal/integration/mcp_stop_task_test.go @@ -221,6 +221,23 @@ func TestMCPStopTask_DirectParentStopsLongRunningChild(t *testing.T) { require.Equal(t, v1.TaskStateInProgress, taskBefore.State) require.True(t, manager.IsAgentRunningForSession(context.Background(), launch.SessionID)) promptsBefore := manager.promptCalls.Load() + // Agent stream delivery is asynchronous with respect to the RUNNING state + // notification. Wait for the simulated tool-call message before taking the + // baseline so a legitimate in-flight stream event is not mistaken for output + // created after the coordinator stop. + require.Eventually(t, func() bool { + messages, listErr := ts.TaskRepo.ListMessages(context.Background(), launch.SessionID) + if listErr != nil { + return false + } + for _, message := range messages { + if message.Type == models.MessageTypeToolCall && + message.Metadata["tool_call_id"] == "long-running-work" { + return true + } + } + return false + }, 2*time.Second, 10*time.Millisecond) messagesBefore, err := ts.TaskRepo.ListMessages(context.Background(), launch.SessionID) require.NoError(t, err) diff --git a/apps/backend/internal/orchestrator/event_handlers_agent.go b/apps/backend/internal/orchestrator/event_handlers_agent.go index 4d218f28b7..f6ff30d559 100644 --- a/apps/backend/internal/orchestrator/event_handlers_agent.go +++ b/apps/backend/internal/orchestrator/event_handlers_agent.go @@ -232,6 +232,7 @@ func (s *Service) handleAgentBootReady(ctx context.Context, data watcher.AgentEv zap.String("session_id", data.SessionID)) return } + if s.isCancelInFlight(data.SessionID) { s.logger.Debug("ignoring agent.boot_ready while cancel is in progress", zap.String("task_id", data.TaskID), @@ -278,7 +279,6 @@ func (s *Service) handleAgentBootReady(ctx context.Context, data watcher.AgentEv } else { s.setSessionWaitingForInput(ctx, data.TaskID, data.SessionID, session) } - // Drain any orphaned queued message. handleAgentReady drains on turn-end, // but a session that crashed mid-turn (or never started its first turn) // won't fire agent.ready — leaving e.g. workflow auto-start prompts stuck diff --git a/apps/backend/internal/orchestrator/event_handlers_clarification.go b/apps/backend/internal/orchestrator/event_handlers_clarification.go index ec59b92a0d..62b111450a 100644 --- a/apps/backend/internal/orchestrator/event_handlers_clarification.go +++ b/apps/backend/internal/orchestrator/event_handlers_clarification.go @@ -457,6 +457,9 @@ func (s *Service) PauseForClarificationInput(ctx context.Context, sessionID stri // (agent crashed mid-turn). In that case, skip the cancel signal but still reconcile the // session's state so clarification recovery can proceed with a fresh prompt. func (s *Service) cancelAgentSilent(ctx context.Context, taskID, sessionID string) error { + endCancel := s.beginCancelInFlight(sessionID) + defer endCancel() + if s.agentManager == nil { s.logger.Debug("skipping silent clarification cancel because agent manager is not configured", zap.String("task_id", taskID), diff --git a/apps/backend/internal/orchestrator/event_handlers_pending_move_test.go b/apps/backend/internal/orchestrator/event_handlers_pending_move_test.go index 92aaae2719..72c155690d 100644 --- a/apps/backend/internal/orchestrator/event_handlers_pending_move_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_pending_move_test.go @@ -898,6 +898,8 @@ func TestHandleAgentBootReady_DoesNotDrainWhileCancelInFlight(t *testing.T) { ); err != nil { t.Fatalf("queue prompt: %v", err) } + endCancel := svc.beginCancelInFlight("s1") + defer endCancel() lock, release := svc.acquireCancelInFlightGuard("s1") defer release() lock.Lock() diff --git a/apps/backend/internal/orchestrator/event_handlers_streaming.go b/apps/backend/internal/orchestrator/event_handlers_streaming.go index 4bde67c582..845fd9231a 100644 --- a/apps/backend/internal/orchestrator/event_handlers_streaming.go +++ b/apps/backend/internal/orchestrator/event_handlers_streaming.go @@ -22,7 +22,17 @@ func (s *Service) handleAgentStreamEvent(ctx context.Context, payload *lifecycle if payload == nil || payload.Data == nil { return } - + if payload.SessionID != "" { + // Serialize stream side effects with cancellation/interrupt decisions. + // Checking the terminal-execution marker alone is insufficient: a stream + // handler can pass that check, then block while persisting a message while + // a coordinator stop marks the execution terminal. Holding the shared + // per-session guard makes the check and its side effects one decision. + lock, release := s.acquireCancelInFlightGuard(payload.SessionID) + defer release() + lock.Lock() + defer lock.Unlock() + } taskID := payload.TaskID sessionID := payload.SessionID eventType := payload.Data.Type diff --git a/apps/backend/internal/orchestrator/event_handlers_streaming_test.go b/apps/backend/internal/orchestrator/event_handlers_streaming_test.go index c5b4824a1b..64a39ed008 100644 --- a/apps/backend/internal/orchestrator/event_handlers_streaming_test.go +++ b/apps/backend/internal/orchestrator/event_handlers_streaming_test.go @@ -1068,6 +1068,75 @@ func TestToolCallFromCompletedExecutionDoesNotCreateMessage(t *testing.T) { "stale completed-execution tool calls must be dropped before message side effects") } +type blockingToolCallMessageCreator struct { + mockMessageCreator + entered chan struct{} + release chan struct{} +} + +func (m *blockingToolCallMessageCreator) CreateToolCallMessage( + context.Context, + string, string, string, string, string, string, string, *streams.NormalizedPayload, +) error { + close(m.entered) + <-m.release + m.toolCallWrites++ + return nil +} + +func TestAgentStreamEventWaitsForCancellationGuard(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "t1", "s1", "") + + creator := &blockingToolCallMessageCreator{ + entered: make(chan struct{}), + release: make(chan struct{}), + } + svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo()) + svc.messageCreator = creator + + lock, releaseGuard := svc.acquireCancelInFlightGuard("s1") + lock.Lock() + + eventDone := make(chan struct{}) + go func() { + svc.handleAgentStreamEvent(ctx, &lifecycle.AgentStreamEventPayload{ + TaskID: "t1", + SessionID: "s1", + ExecutionID: "exec-1", + Data: &lifecycle.AgentStreamEventData{ + Type: agentEventToolCall, + ToolCallID: "tool-1", + ToolStatus: "running", + }, + }) + close(eventDone) + }() + + select { + case <-creator.entered: + t.Fatal("stream side effect bypassed the cancellation guard") + case <-time.After(100 * time.Millisecond): + } + + lock.Unlock() + releaseGuard() + + select { + case <-creator.entered: + case <-time.After(time.Second): + t.Fatal("stream handler did not proceed after cancellation guard release") + } + close(creator.release) + select { + case <-eventDone: + case <-time.After(time.Second): + t.Fatal("stream handler did not finish") + } + require.Equal(t, 1, creator.toolCallWrites) +} + func TestToolCallStreamFromCompletedExecutionDoesNotSaveAgentText(t *testing.T) { ctx := context.Background() repo := setupTestRepo(t) diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index bc949daa9b..f0367c6046 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -546,10 +546,12 @@ type Service struct { // / handleAgentBootReady (TryLock, skip if a cancel/interrupt owns it), // or QueueAndInterruptForPeerMessage (blocking Lock — must wait rather // than work around a busy lock with an unguarded insert; see its doc - // comment). All of these must go through the same per-session guard — - // a second, independent lock for any of them would defeat the mutual - // exclusion the others rely on to avoid racing each other's - // take-and-dispatch decision for the same session. + // comment), or an agent stream handler persisting output (blocking Lock so + // cancellation cannot commit while a stream side effect is in flight). + // All of these must go through the same per-session guard — a second, + // independent lock for any of them would defeat the mutual exclusion the + // others rely on to avoid racing each other's take-and-dispatch decision + // for the same session. // // Entries are reference-counted (acquireCancelInFlightGuard / // releaseCancelInFlightGuard) and pruned once nobody holds a reference, @@ -558,6 +560,16 @@ type Service struct { // long-lived backend's lifetime. cancelInFlight map[string]*cancelInFlightGuard + // cancelOperations tracks cancellation intent separately from the shared + // cancelInFlight mutex. Stream and lifecycle handlers use that mutex to + // serialize their side effects, but they are not themselves cancellations; + // treating every mutex holder as a cancellation would make agent.boot_ready + // discard a legitimate boot signal while a stream frame is being persisted. + // Values are reference counts so duplicate cancellation requests waiting on + // the shared guard keep cancellation priority until the last request exits. + cancelOperationsMu sync.Mutex + cancelOperations map[string]int + // transientRetries tracks in-progress transient-provider-error (529 // Overloaded) retry loops. key: sessionID, value: *transientRetryEntry. // A backoff timer per session re-drives the failed prompt; cancelled on diff --git a/apps/backend/internal/orchestrator/task_operations.go b/apps/backend/internal/orchestrator/task_operations.go index c23a9245bd..00506f63cd 100644 --- a/apps/backend/internal/orchestrator/task_operations.go +++ b/apps/backend/internal/orchestrator/task_operations.go @@ -2443,6 +2443,9 @@ func coordinatorStopSessionID(session *models.TaskSession) string { } func (s *Service) stopTaskSessionForCoordinator(ctx context.Context, taskID, sessionID string) (bool, error) { + endCancel := s.beginCancelInFlight(sessionID) + defer endCancel() + lock, release := s.acquireCancelInFlightGuard(sessionID) lock.Lock() @@ -3916,20 +3919,50 @@ func (s *Service) acquireCancelInFlightGuard(sessionID string) (*sync.Mutex, fun return &guard.mu, release } -// isCancelInFlight reports whether sessionID's cancelInFlight guard is -// currently held by someone else, without itself claiming it — a -// TryLock-then-immediately-Unlock peek rather than a real acquisition. The -// guard entry this creates to perform the peek is released (and pruned if -// nothing else references it) before this returns, so a passive readiness -// probe never leaves permanent state behind. +// beginCancelInFlight records that a cancellation operation for sessionID is +// active (or waiting to claim the shared per-session guard). This is separate +// from cancelInFlight itself: stream handlers also hold that mutex while +// persisting output, but must not make boot-ready handling mistake their work +// for a cancellation. The returned release function is idempotent so callers +// can safely use it with both explicit and deferred cleanup. +func (s *Service) beginCancelInFlight(sessionID string) func() { + if sessionID == "" { + return func() {} + } + + s.cancelOperationsMu.Lock() + if s.cancelOperations == nil { + s.cancelOperations = make(map[string]int) + } + s.cancelOperations[sessionID]++ + s.cancelOperationsMu.Unlock() + + var once sync.Once + return func() { + once.Do(func() { + s.cancelOperationsMu.Lock() + count := s.cancelOperations[sessionID] + if count <= 1 { + delete(s.cancelOperations, sessionID) + } else { + s.cancelOperations[sessionID] = count - 1 + } + s.cancelOperationsMu.Unlock() + }) + } +} + +// isCancelInFlight reports whether an actual cancellation operation for +// sessionID is active or waiting on the shared guard. It deliberately does +// not inspect the guard mutex itself because stream/lifecycle handlers also +// use that mutex for non-cancelling side effects. func (s *Service) isCancelInFlight(sessionID string) bool { - lock, release := s.acquireCancelInFlightGuard(sessionID) - defer release() - if lock.TryLock() { - lock.Unlock() + if sessionID == "" { return false } - return true + s.cancelOperationsMu.Lock() + defer s.cancelOperationsMu.Unlock() + return s.cancelOperations[sessionID] > 0 } // CancelAgent interrupts the current agent turn without terminating the process, @@ -3946,6 +3979,8 @@ func (s *Service) CancelAgent(ctx context.Context, sessionID string) error { } s.logger.Debug("cancelling agent turn", zap.String("session_id", sessionID)) + endCancel := s.beginCancelInFlight(sessionID) + defer endCancel() // Deduplicate concurrent retries. The UI's cancel button has no in-flight // disable, so impatient users click it multiple times while the agent is diff --git a/apps/backend/internal/orchestrator/task_operations_test.go b/apps/backend/internal/orchestrator/task_operations_test.go index 3d1964eada..5400fac5ee 100644 --- a/apps/backend/internal/orchestrator/task_operations_test.go +++ b/apps/backend/internal/orchestrator/task_operations_test.go @@ -2256,6 +2256,29 @@ func TestAcquireCancelInFlightGuard_PrunesEntryWhenNoLongerReferenced(t *testing } } +func TestCancelIntentDoesNotFollowSharedGuard(t *testing.T) { + svc := &Service{} + lock, release := svc.acquireCancelInFlightGuard("s1") + lock.Lock() + defer func() { + lock.Unlock() + release() + }() + + if svc.isCancelInFlight("s1") { + t.Fatal("a non-cancelling shared guard holder must not look like a cancellation") + } + + endCancel := svc.beginCancelInFlight("s1") + if !svc.isCancelInFlight("s1") { + t.Fatal("active cancellation intent must be visible to readiness probes") + } + endCancel() + if svc.isCancelInFlight("s1") { + t.Fatal("released cancellation intent must no longer be visible") + } +} + // --- StartCreatedSession --- func TestStartCreatedSession_WrongTask(t *testing.T) { diff --git a/apps/backend/internal/system/storage/handler.go b/apps/backend/internal/system/storage/handler.go index 0a1297b525..f4e1c7c90c 100644 --- a/apps/backend/internal/system/storage/handler.go +++ b/apps/backend/internal/system/storage/handler.go @@ -54,11 +54,13 @@ type QuarantineSummary struct { type OverviewProvider interface { Summary(context.Context) (Summary, error) Capabilities(context.Context, StorageMaintenanceSettings) Capabilities + SettingsCapabilities(context.Context, StorageMaintenanceSettings) Capabilities } type OverviewReader interface { Get(context.Context) (OverviewSnapshot, error) Capabilities(context.Context, StorageMaintenanceSettings) Capabilities + SettingsCapabilities(context.Context, StorageMaintenanceSettings) Capabilities } type HandlerConfig struct { @@ -68,6 +70,7 @@ type HandlerConfig struct { Overview OverviewReader Mutations Mutations OnSettingsChanged func(StorageMaintenanceSettings) + LogError func(string, error) } type Handler struct { @@ -78,8 +81,15 @@ func NewHandler(config HandlerConfig) *Handler { return &Handler{config: config} } +func (h *Handler) logError(message string, err error) { + if h.config.LogError != nil { + h.config.LogError(message, err) + } +} + func RegisterRoutes(group *gin.RouterGroup, handler *Handler) { group.GET("/storage", handler.getStorage) + group.GET("/storage/settings", handler.getStorageSettings) group.PATCH("/storage/settings", handler.patchSettings) group.POST("/storage/go-cache/adopt", handler.adoptGoCache) group.POST("/storage/analyze", handler.analyze) @@ -228,7 +238,8 @@ func writeMutationError(c *gin.Context, err error) { func (h *Handler) getStorage(c *gin.Context) { settings, err := h.config.Settings.GetSettings(c.Request.Context()) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + h.logError("failed to load storage settings", err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to load storage settings"}) return } snapshot, err := h.config.Overview.Get(c.Request.Context()) @@ -251,6 +262,19 @@ func (h *Handler) getStorage(c *gin.Context) { }) } +func (h *Handler) getStorageSettings(c *gin.Context) { + settings, err := h.config.Settings.GetSettings(c.Request.Context()) + if err != nil { + h.logError("failed to load storage settings", err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to load storage settings"}) + return + } + c.JSON(http.StatusOK, gin.H{ + "settings": settings, + "capabilities": h.config.Overview.SettingsCapabilities(c.Request.Context(), settings), + }) +} + type patchSettingsRequest struct { Settings StorageMaintenanceSettings `json:"settings" binding:"required"` Confirmations struct { diff --git a/apps/backend/internal/system/storage/handler_test.go b/apps/backend/internal/system/storage/handler_test.go index 556bdbef5b..6540702d06 100644 --- a/apps/backend/internal/system/storage/handler_test.go +++ b/apps/backend/internal/system/storage/handler_test.go @@ -71,6 +71,81 @@ func TestGetStorageReturnsSnapshotAnalyzedAt(t *testing.T) { } } +func TestGetStorageSettingsReturnsPolicyWithoutOverviewScan(t *testing.T) { + gin.SetMode(gin.TestMode) + settings := DefaultSettings() + capabilities := Capabilities{ + ManagedGoCachePath: "/data/cache/go-build", + GoCacheAdoptionAvailable: true, + DockerAvailable: true, + DockerHost: "unix:///var/run/docker.sock", + HostGlobalDockerCleanup: true, + } + overview := &recordingOverviewReader{capabilities: capabilities} + router := gin.New() + RegisterRoutes(router.Group("/api/v1/system"), NewHandler(HandlerConfig{ + Settings: staticSettingsManager{settings: settings}, + Overview: overview, + })) + + response := httptest.NewRecorder() + router.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/api/v1/system/storage/settings", nil)) + + if response.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", response.Code) + } + var body struct { + Settings StorageMaintenanceSettings `json:"settings"` + Capabilities Capabilities `json:"capabilities"` + } + if err := json.Unmarshal(response.Body.Bytes(), &body); err != nil { + t.Fatalf("decode response: %v", err) + } + if body.Settings != settings { + t.Fatalf("settings = %#v, want %#v", body.Settings, settings) + } + if body.Capabilities != capabilities { + t.Fatalf("capabilities = %#v, want %#v", body.Capabilities, capabilities) + } + if overview.settingsCapabilitiesCalls != 1 { + t.Fatalf("settings capabilities calls = %d, want 1", overview.settingsCapabilitiesCalls) + } + if overview.capabilitiesCalls != 0 { + t.Fatalf("full capabilities calls = %d, want 0", overview.capabilitiesCalls) + } + if overview.getCalls != 0 { + t.Fatalf("overview scans = %d, want 0", overview.getCalls) + } +} + +func TestGetStorageSettingsHidesInternalLoadFailure(t *testing.T) { + gin.SetMode(gin.TestMode) + internalErr := errors.New("database credentials leaked") + var loggedMessage string + var loggedErr error + router := gin.New() + RegisterRoutes(router.Group("/api/v1/system"), NewHandler(HandlerConfig{ + Settings: failingSettingsManager{getErr: internalErr}, + LogError: func(message string, err error) { + loggedMessage, loggedErr = message, err + }, + })) + + response := httptest.NewRecorder() + router.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/api/v1/system/storage/settings", nil)) + + if response.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500", response.Code) + } + if strings.Contains(response.Body.String(), "credentials") || + !strings.Contains(response.Body.String(), "failed to load storage settings") { + t.Fatalf("response did not use a client-safe message: %s", response.Body.String()) + } + if loggedMessage != "failed to load storage settings" || !errors.Is(loggedErr, internalErr) { + t.Fatalf("logged error = (%q, %v), want original error", loggedMessage, loggedErr) + } +} + func TestDeleteQuarantineBulkValidatesConfirmation(t *testing.T) { gin.SetMode(gin.TestMode) mutations := &recordingMutations{} @@ -119,16 +194,25 @@ func (m *recordingMutations) PurgeQuarantine(context.Context, QuarantinePurgeSco return "job", nil } -type failingSettingsManager struct{ err error } +type failingSettingsManager struct { + err error + getErr error +} func (f failingSettingsManager) GetSettings(context.Context) (StorageMaintenanceSettings, error) { + if f.getErr != nil { + return DefaultSettings(), f.getErr + } return DefaultSettings(), nil } -type staticSettingsManager struct{} +type staticSettingsManager struct{ settings StorageMaintenanceSettings } -func (staticSettingsManager) GetSettings(context.Context) (StorageMaintenanceSettings, error) { - return DefaultSettings(), nil +func (s staticSettingsManager) GetSettings(context.Context) (StorageMaintenanceSettings, error) { + if s.settings == (StorageMaintenanceSettings{}) { + return DefaultSettings(), nil + } + return s.settings, nil } func (staticSettingsManager) SaveSettingsWithConfirmations(context.Context, StorageMaintenanceSettings, SaveConfirmations) (StorageMaintenanceSettings, error) { @@ -147,6 +231,38 @@ func (o staticCachedOverview) Capabilities(context.Context, StorageMaintenanceSe return Capabilities{} } +func (o staticCachedOverview) SettingsCapabilities( + context.Context, + StorageMaintenanceSettings, +) Capabilities { + return Capabilities{} +} + +type recordingOverviewReader struct { + capabilities Capabilities + getCalls int + capabilitiesCalls int + settingsCapabilitiesCalls int +} + +func (o *recordingOverviewReader) Get(context.Context) (OverviewSnapshot, error) { + o.getCalls++ + return OverviewSnapshot{}, nil +} + +func (o *recordingOverviewReader) Capabilities(context.Context, StorageMaintenanceSettings) Capabilities { + o.capabilitiesCalls++ + return o.capabilities +} + +func (o *recordingOverviewReader) SettingsCapabilities( + context.Context, + StorageMaintenanceSettings, +) Capabilities { + o.settingsCapabilitiesCalls++ + return o.capabilities +} + func (f failingSettingsManager) SaveSettingsWithConfirmations( context.Context, StorageMaintenanceSettings, diff --git a/apps/backend/internal/system/storage/operations_test.go b/apps/backend/internal/system/storage/operations_test.go index 92075f4eed..de3eb3c7da 100644 --- a/apps/backend/internal/system/storage/operations_test.go +++ b/apps/backend/internal/system/storage/operations_test.go @@ -455,6 +455,13 @@ func (o *recordingRefreshOverview) Capabilities(context.Context, StorageMaintena return Capabilities{} } +func (o *recordingRefreshOverview) SettingsCapabilities( + context.Context, + StorageMaintenanceSettings, +) Capabilities { + return Capabilities{} +} + func (o *recordingRefreshOverview) Invalidate() { o.invalidateCalls++ } diff --git a/apps/backend/internal/system/storage/overview_cache.go b/apps/backend/internal/system/storage/overview_cache.go index 4d91fe0289..6efd058283 100644 --- a/apps/backend/internal/system/storage/overview_cache.go +++ b/apps/backend/internal/system/storage/overview_cache.go @@ -77,6 +77,13 @@ func (c *OverviewCache) Capabilities(ctx context.Context, settings StorageMainte return c.provider.Capabilities(ctx, settings) } +func (c *OverviewCache) SettingsCapabilities( + ctx context.Context, + settings StorageMaintenanceSettings, +) Capabilities { + return c.provider.SettingsCapabilities(ctx, settings) +} + func (c *OverviewCache) Invalidate() { c.mu.Lock() c.generation++ diff --git a/apps/backend/internal/system/storage/overview_cache_test.go b/apps/backend/internal/system/storage/overview_cache_test.go index 134ac94f09..9b5ed5f371 100644 --- a/apps/backend/internal/system/storage/overview_cache_test.go +++ b/apps/backend/internal/system/storage/overview_cache_test.go @@ -256,6 +256,13 @@ func (o *countingOverview) Capabilities(context.Context, StorageMaintenanceSetti return Capabilities{} } +func (o *countingOverview) SettingsCapabilities( + context.Context, + StorageMaintenanceSettings, +) Capabilities { + return Capabilities{} +} + type overviewResult struct { snapshot OverviewSnapshot err error @@ -293,6 +300,13 @@ func (*firstCallBlockingOverview) Capabilities( return Capabilities{} } +func (*firstCallBlockingOverview) SettingsCapabilities( + context.Context, + StorageMaintenanceSettings, +) Capabilities { + return Capabilities{} +} + func (o *cancellableOverview) Summary(ctx context.Context) (Summary, error) { o.calls++ close(o.started) @@ -307,3 +321,10 @@ func (o *cancellableOverview) Summary(ctx context.Context) (Summary, error) { func (o *cancellableOverview) Capabilities(context.Context, StorageMaintenanceSettings) Capabilities { return Capabilities{} } + +func (o *cancellableOverview) SettingsCapabilities( + context.Context, + StorageMaintenanceSettings, +) Capabilities { + return Capabilities{} +} diff --git a/apps/backend/internal/system/storage_routes_test.go b/apps/backend/internal/system/storage_routes_test.go index 946b7a02f5..686802e0ed 100644 --- a/apps/backend/internal/system/storage_routes_test.go +++ b/apps/backend/internal/system/storage_routes_test.go @@ -31,6 +31,18 @@ func TestRegisterRoutesExposesStorageSummary(t *testing.T) { } } +func TestRegisterRoutesExposesStorageSettings(t *testing.T) { + router := newStorageRoutesTestRouter(t) + + request := httptest.NewRequest(http.MethodGet, "/api/v1/system/storage/settings", nil) + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + + if response.Code != http.StatusOK { + t.Fatalf("GET /api/v1/system/storage/settings status = %d, want 200", response.Code) + } +} + func TestStorageSettingsRejectInvalidPatch(t *testing.T) { router := newStorageRoutesTestRouter(t) settings := storage.DefaultSettings() @@ -230,3 +242,10 @@ func (emptyStorageOverview) Get(context.Context) (storage.OverviewSnapshot, erro func (emptyStorageOverview) Capabilities(context.Context, storage.StorageMaintenanceSettings) storage.Capabilities { return storage.Capabilities{} } + +func (emptyStorageOverview) SettingsCapabilities( + context.Context, + storage.StorageMaintenanceSettings, +) storage.Capabilities { + return 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 f9aadda767..dc9a008a0a 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 @@ -12,6 +12,7 @@ const mocks = vi.hoisted(() => ({ })); const IDLE_PERIOD_TEST_ID = "storage-idle-period"; const SAVE_BUTTON_NAME = "Save changes"; +const ANALYZE_TEST_ID = "storage-analyze"; vi.mock("@/hooks/domains/system/use-storage-maintenance", async (importOriginal) => ({ ...(await importOriginal()), @@ -117,7 +118,7 @@ describe("StorageMaintenanceSettings", () => { render(, { wrapper: Providers }); - const analyzeButton = screen.getByTestId("storage-analyze"); + const analyzeButton = screen.getByTestId(ANALYZE_TEST_ID); expect(analyzeButton.textContent?.trim()).toBe("Analysis complete"); expect(analyzeButton.getAttribute("data-job-state")).toBe("succeeded"); expect(screen.queryByTestId("storage-analysis-job")).toBeNull(); @@ -137,7 +138,7 @@ describe("StorageMaintenanceSettings", () => { render(, { wrapper: Providers }); - const analyzeButton = screen.getByTestId("storage-analyze") as HTMLButtonElement; + const analyzeButton = screen.getByTestId(ANALYZE_TEST_ID) as HTMLButtonElement; expect(analyzeButton.textContent?.trim()).toBe("Analyzing..."); expect(analyzeButton.disabled).toBe(true); }); @@ -283,7 +284,7 @@ describe("StorageMaintenanceSettings pending policy", () => { mocks.useStorageMaintenance.mockReturnValue(currentController); render(, { wrapper: Providers }); - expect((screen.getByTestId("storage-analyze") as HTMLButtonElement).disabled).toBe(true); + expect((screen.getByTestId(ANALYZE_TEST_ID) 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( @@ -292,6 +293,19 @@ describe("StorageMaintenanceSettings pending policy", () => { }, ); + it("keeps section actions enabled while policy is loading", () => { + mocks.useStorageMaintenance.mockReturnValue({ + ...controller(overview), + loading: { policy: true, overview: false, runs: false, quarantine: false }, + sectionErrors: { policy: null, overview: null, runs: null, quarantine: null }, + }); + + render(, { wrapper: Providers }); + + expect((screen.getByTestId(ANALYZE_TEST_ID) as HTMLButtonElement).disabled).toBe(false); + expect((screen.getByTestId("storage-run-now") as HTMLButtonElement).disabled).toBe(false); + }); + it("blocks Go cache adoption while a save is pending", () => { mocks.useStorageMaintenance.mockReturnValue({ ...controller(overview), 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 594d6df214..15437c3ee1 100644 --- a/apps/web/components/settings/system/storage/storage-maintenance-settings.tsx +++ b/apps/web/components/settings/system/storage/storage-maintenance-settings.tsx @@ -2,8 +2,10 @@ import { useEffect, useRef, useState, type ReactNode } from "react"; import { Alert, AlertDescription, AlertTitle } from "@kandev/ui/alert"; +import { Card, CardContent } from "@kandev/ui/card"; import { Spinner } from "@kandev/ui/spinner"; import { IconAlertTriangle, IconCheck, IconPlayerPlay, IconRefresh } from "@tabler/icons-react"; +import { useTranslation } from "react-i18next"; import { useStorageMaintenance, type StorageBusyState, @@ -187,19 +189,33 @@ function serializeSettings(settings: Settings | null): string { } function policyPendingAction(action: ReturnType["pendingAction"]) { - return action === "load" || action === "save" || action === "adopt"; + return 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."; +function policyBlockedReason( + t: (key: string) => string, + action: ReturnType["pendingAction"], + loading: boolean, +) { + if (action === "adopt") return t("settings:storageAdoptionPending"); + if (loading) return t("settings:storagePolicyLoadingBlock"); + return undefined; +} + +function storageActionDisabledReason( + t: (key: string) => string, + action: ReturnType["pendingAction"], +) { + if (action) return t("settings:storageActionPending"); return undefined; } function useStoragePolicyDraft(controller: ReturnType) { + const { t } = useTranslation(); const [draft, setDraft] = useState(null); const previousServerSettings = useRef(null); - const savedSettings = controller.overview?.settings ?? null; + const savedSettings = controller.policy?.settings ?? controller.overview?.settings ?? null; + const policyLoading = controller.loading?.policy ?? !savedSettings; useEffect(() => { if (!savedSettings) return; @@ -216,7 +232,7 @@ function useStoragePolicyDraft(controller: ReturnType + + {loading && } + {loading ? t("settings:loading") : t("settings:storageSectionUnavailable")} + {error && {error}} + + + ); +} + +function StoragePrimarySections({ + controller, + disabledReason, + draft, + setDraft, + savedSettings, +}: { + controller: ReturnType; + disabledReason?: string; + draft: Settings | null; + setDraft: (settings: Settings | null) => void; + savedSettings: Settings | null; +}) { const controlsPending = policyPendingAction(controller.pendingAction); - const actionDisabledReason = controller.pendingAction - ? "Wait for the current storage action to finish." - : undefined; + const policyLoading = controller.loading?.policy ?? !savedSettings; + const capabilities = controller.policy?.capabilities ?? controller.overview?.capabilities; + return ( +
+ void controller.runNow(["go_cache"])} + /> + + {draft && savedSettings && capabilities && ( + + )} +
+ ); +} + +function StorageQuarantineSection({ + controller, + disabledReason, + savedSettings, +}: { + controller: ReturnType; + disabledReason?: string; + savedSettings: Settings | null; +}) { + const deleteJobActive = + controller.deleteJob?.state === "queued" || controller.deleteJob?.state === "running"; + return ( + + ); +} + +function StoragePageSections({ + controller, + disabledReason, +}: { + controller: ReturnType; + disabledReason?: string; +}) { + const { draft, setDraft, savedSettings } = useStoragePolicyDraft(controller); + return ( + <> + + + + + ); +} + +export function StorageMaintenanceSettings() { + const { t } = useTranslation(); + const controller = useStorageMaintenance(); + const actionDisabledReason = storageActionDisabledReason(t, controller.pendingAction); return (
@@ -256,38 +384,7 @@ export function StorageMaintenanceSettings() { -
- void controller.runNow(["go_cache"])} - /> - {draft && savedSettings && controller.overview && ( - - )} -
- - +
); } 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 07d8076922..3aa6ddbaad 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 @@ -1,4 +1,5 @@ import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { TooltipProvider } from "@kandev/ui/tooltip"; import { afterEach, describe, expect, it, vi } from "vitest"; import type { StorageOverviewResponse } from "@/lib/types/system"; import { StorageOverviewCard } from "./storage-overview-card"; @@ -132,5 +133,48 @@ describe("StorageOverviewCard", () => { expect( screen.getByTestId("storage-resource-docker-image-layers-trigger").textContent, ).toContain("14 GB"); + expect(screen.getByTestId("storage-analysis-total").textContent).toContain( + "Total counted: 47 GB", + ); + expect(screen.getByTestId("storage-analysis-total-partial")).toBeTruthy(); + }); +}); + +describe("StorageOverviewCard refresh and policy state", () => { + it("uses the current saved policy for Go cache cleanup eligibility", () => { + const overview = { + ...degradedOverview, + settings: { + ...degradedOverview.settings, + go_cache: { ...degradedOverview.settings.go_cache, max_bytes: 20 * 1024 ** 3 }, + }, + summary: { + ...degradedOverview.summary, + go_cache: { + ...degradedOverview.summary.go_cache, + size_bytes: 15 * 1024 ** 3, + }, + }, + } satisfies StorageOverviewResponse; + const savedSettings = { + ...overview.settings, + go_cache: { ...overview.settings.go_cache, max_bytes: 10 * 1024 ** 3 }, + }; + + render( + , + { wrapper: TooltipProvider }, + ); + + fireEvent.click(screen.getByTestId("storage-resource-go-cache-trigger")); + expect((screen.getByTestId("storage-go-cache-clean") as HTMLButtonElement).disabled).toBe( + false, + ); + }); + + it("shows refresh progress while a cached snapshot is loading", () => { + render(); + + expect(screen.getByTestId("storage-overview-spinner")).toBeTruthy(); }); }); 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 1e083f5900..5104a2becc 100644 --- a/apps/web/components/settings/system/storage/storage-overview-card.tsx +++ b/apps/web/components/settings/system/storage/storage-overview-card.tsx @@ -3,13 +3,22 @@ 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 { useTranslation } from "react-i18next"; +import type { + StorageMaintenanceSettings, + StorageOverviewResponse, + StorageQuarantineSummary, +} from "@/lib/types/system"; import { formatRelativeTime } from "@/lib/utils"; import { StorageActionButton } from "./storage-action-button"; import { formatGigabytes } from "./storage-units"; +import { storageAnalysisTotal } from "./storage-totals"; interface Props { overview: StorageOverviewResponse | null; + settings?: StorageMaintenanceSettings; + loading?: boolean; + error?: string | null; disabledReason?: string; onRunGoCache: () => void; } @@ -22,12 +31,19 @@ interface StorageResource { warning?: string; } -function goCacheDisabledReason(overview: StorageOverviewResponse, pendingReason?: string) { +function goCacheDisabledReason( + overview: StorageOverviewResponse, + pendingReason?: string, + settings?: StorageMaintenanceSettings, +) { if (pendingReason) return pendingReason; if (overview.summary.go_cache.owned !== true) { return "Only a Kandev-owned Go build cache can be cleaned."; } - if ((overview.summary.go_cache.size_bytes ?? 0) <= overview.settings.go_cache.max_bytes) { + if ( + (overview.summary.go_cache.size_bytes ?? 0) <= + (settings ?? overview.settings).go_cache.max_bytes + ) { return "The Go build cache is below its configured size limit."; } return undefined; @@ -171,26 +187,40 @@ function ResourceRow({ resource, goCacheCleanupDisabledReason, onRunGoCache }: R ); } -export function StorageOverviewCard({ overview, disabledReason, onRunGoCache }: Props) { +export function StorageOverviewCard({ + overview, + settings, + loading, + error, + disabledReason, + onRunGoCache, +}: Props) { + const { t } = useTranslation(); + const isLoading = loading ?? overview === null; if (!overview) { return ( - - Loading storage data… + {isLoading && } + + {isLoading ? t("settings:storageLoadingData") : t("settings:storageSectionUnavailable")} + + {error && {error}} ); } const { summary } = overview; const analyzedAt = new Date(overview.analyzed_at).toLocaleString(); - const cleanupDisabledReason = goCacheDisabledReason(overview, disabledReason); + const cleanupDisabledReason = goCacheDisabledReason(overview, disabledReason, settings); const resources = storageResources(overview); + const total = storageAnalysisTotal(summary); return ( Storage analysis + {isLoading && } {!summary.docker.available && Docker unavailable} @@ -205,6 +235,24 @@ export function StorageOverviewCard({ overview, disabledReason, onRunGoCache }: > Last analyzed {formatRelativeTime(overview.analyzed_at)} +
+ + {t("settings:storageTotalCounted", { size: formatGigabytes(total.bytes) })} + + {total.partial && ( + + {t("settings:storageTotalPartial")} + + )} +
+ {error && ( +

+ {t("settings:storageSectionUnavailable")}: {error} +

+ )} diff --git a/apps/web/components/settings/system/storage/storage-quarantine-card.test.tsx b/apps/web/components/settings/system/storage/storage-quarantine-card.test.tsx new file mode 100644 index 0000000000..21264aae82 --- /dev/null +++ b/apps/web/components/settings/system/storage/storage-quarantine-card.test.tsx @@ -0,0 +1,91 @@ +import type { ReactNode } from "react"; +import { cleanup, render, screen } from "@testing-library/react"; +import { TooltipProvider } from "@kandev/ui/tooltip"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { StorageQuarantineEntry } from "@/lib/types/system"; +import { StorageQuarantineCard } from "./storage-quarantine-card"; + +vi.mock("@/hooks/domains/system/use-system-jobs", () => ({ + useSystemJob: () => undefined, + useSystemJobs: () => [], +})); + +afterEach(cleanup); + +function Providers({ children }: { children: ReactNode }) { + return {children}; +} + +function renderCard( + props: { entries?: StorageQuarantineEntry[]; loading?: boolean; error?: string | null } = {}, +) { + return render( + , + { wrapper: Providers }, + ); +} + +describe("StorageQuarantineCard", () => { + it("renders an independent loading state", () => { + renderCard({ loading: true }); + + expect(screen.getByTestId("storage-quarantine-spinner")).toBeTruthy(); + expect(screen.getByText("Loading quarantine…")).toBeTruthy(); + expect(screen.queryByText("No restorable quarantined resources.")).toBeNull(); + expect(screen.queryByTestId("storage-quarantine-total")).toBeNull(); + }); + + it("renders an isolated error state", () => { + renderCard({ error: "quarantine unavailable" }); + + expect(screen.getByTestId("storage-quarantine-error").textContent).toContain( + "quarantine unavailable", + ); + expect(screen.queryByTestId("storage-quarantine-spinner")).toBeNull(); + expect(screen.queryByTestId("storage-quarantine-total")).toBeNull(); + }); + + it("shows the total of all listed entries", () => { + const entries = [ + { + id: "cache-1", + resource_type: "go_cache", + original_path: "/var/cache/go-build", + quarantine_path: "/var/trash/cache-1", + size_bytes: 2 * 1024 ** 3, + state: "quarantined", + quarantined_at: "2026-07-23T12:00:00Z", + delete_after: "2026-08-23T12:00:00Z", + last_error: "", + metadata: {}, + }, + { + id: "workspace-1", + resource_type: "task_workspace", + original_path: "/var/tasks/workspace-1", + quarantine_path: "/var/trash/workspace-1", + size_bytes: 1024 ** 3, + state: "quarantined", + quarantined_at: "2026-07-23T12:00:00Z", + delete_after: "2026-08-23T12:00:00Z", + last_error: "", + metadata: {}, + }, + ] satisfies StorageQuarantineEntry[]; + + renderCard({ entries }); + + expect(screen.getByTestId("storage-quarantine-total").textContent).toBe( + "Total quarantined: 3 GB", + ); + }); +}); diff --git a/apps/web/components/settings/system/storage/storage-quarantine-card.tsx b/apps/web/components/settings/system/storage/storage-quarantine-card.tsx index 777a0aafa1..b6de4969bd 100644 --- a/apps/web/components/settings/system/storage/storage-quarantine-card.tsx +++ b/apps/web/components/settings/system/storage/storage-quarantine-card.tsx @@ -3,13 +3,16 @@ import { useEffect, useState } from "react"; import { Badge } from "@kandev/ui/badge"; import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@kandev/ui/card"; +import { Spinner } from "@kandev/ui/spinner"; import { IconRestore, IconTrash } from "@tabler/icons-react"; +import { useTranslation } from "react-i18next"; import type { StorageQuarantineEntry, StorageQuarantinePurgeScope } from "@/lib/types/system"; import { JobProgressIndicator } from "../job-progress-indicator"; import { PermanentDeleteDialog, QuarantinePurgeDialog } from "./storage-confirmation-dialogs"; import { StorageActionButton } from "./storage-action-button"; import { StorageSettingHelp } from "./storage-setting-help"; import { formatGigabytes } from "./storage-units"; +import { quarantineTotalBytes } from "./storage-totals"; import { formatQuarantineDeadline, isQuarantineEligible, @@ -21,6 +24,8 @@ const MAX_TIMER_DELAY_MS = 2_147_483_647; type Props = { entries: StorageQuarantineEntry[]; + loading?: boolean; + error?: string | null; deleteJobId?: string; deleteJobActive?: boolean; disabledReason?: string; @@ -107,7 +112,9 @@ function QuarantineHeader({ bulkDisabledReason, schedulingEnabled, checkIntervalHours, + showTotal, onPurge, + totalBytes, }: { entries: StorageQuarantineEntry[]; counts: ReturnType; @@ -115,8 +122,11 @@ function QuarantineHeader({ bulkDisabledReason?: string; schedulingEnabled: boolean; checkIntervalHours: number; + showTotal: boolean; onPurge: (scope: StorageQuarantinePurgeScope) => void; + totalBytes: number; }) { + const { t } = useTranslation(); return (
@@ -165,6 +175,11 @@ function QuarantineHeader({ item if you still need it, or delete it permanently when you are certain. {counts.eligible}{" "} eligible now · {counts.protected} protected + {showTotal && ( +

+ {t("settings:storageQuarantineTotal", { size: formatGigabytes(totalBytes) })} +

+ )}

{schedulingEnabled ? `Eligible entries are removed by the first successful maintenance run after their deadline (every ${checkIntervalHours} hours when the idle gate allows it).` @@ -174,8 +189,58 @@ function QuarantineHeader({ ); } +function QuarantineContent({ + entries, + loading, + error, + now, + disabledReason, + onRestore, + onDelete, +}: { + entries: StorageQuarantineEntry[]; + loading: boolean; + error?: string | null; + now: Date; + disabledReason?: string; + onRestore: (id: string) => Promise; + onDelete: (entry: StorageQuarantineEntry) => void; +}) { + const { t } = useTranslation(); + if (loading) { + return ( +

+ + {t("settings:storageQuarantineLoading")} +
+ ); + } + if (error) { + return ( +

+ {t("settings:storageSectionUnavailable")}: {error} +

+ ); + } + if (entries.length === 0) { + return

{t("settings:storageQuarantineEmpty")}

; + } + return entries.map((entry) => ( + + )); +} + export function StorageQuarantineCard({ entries, + loading = false, + error, deleteJobId, deleteJobActive, disabledReason, @@ -201,6 +266,7 @@ export function StorageQuarantineCard({ return () => clearTimeout(timer); }, [entries, now]); const counts = quarantineCounts(entries, now); + const totalBytes = quarantineTotalBytes(entries); const bulkDisabledReason = disabledReason ?? (deleteJobActive ? "A quarantine cleanup is still running." : undefined); return ( @@ -212,22 +278,20 @@ export function StorageQuarantineCard({ bulkDisabledReason={bulkDisabledReason} schedulingEnabled={schedulingEnabled} checkIntervalHours={checkIntervalHours} + showTotal={!loading && !error} onPurge={setPurgeScope} + totalBytes={totalBytes} /> - {entries.length === 0 && ( -

No restorable quarantined resources.

- )} - {entries.map((entry) => ( - - ))} + { + it("renders an independent loading state", () => { + render(); + + expect(screen.getByTestId("storage-run-history-spinner")).toBeTruthy(); + expect(screen.getByText("Loading maintenance history…")).toBeTruthy(); + expect(screen.queryByText("No storage maintenance runs yet.")).toBeNull(); + }); + + it("renders an isolated error state", () => { + render(); + + expect(screen.getByTestId("storage-run-history-error").textContent).toContain( + "history unavailable", + ); + expect(screen.queryByTestId("storage-run-history-spinner")).toBeNull(); + }); +}); diff --git a/apps/web/components/settings/system/storage/storage-run-history.tsx b/apps/web/components/settings/system/storage/storage-run-history.tsx index 74c306a804..0165646c71 100644 --- a/apps/web/components/settings/system/storage/storage-run-history.tsx +++ b/apps/web/components/settings/system/storage/storage-run-history.tsx @@ -1,6 +1,8 @@ import { Accordion, AccordionContent, AccordionItem, AccordionTrigger } from "@kandev/ui/accordion"; import { Badge } from "@kandev/ui/badge"; import { Card, CardContent, CardHeader, CardTitle } from "@kandev/ui/card"; +import { Spinner } from "@kandev/ui/spinner"; +import { useTranslation } from "react-i18next"; import type { StorageMaintenanceRun } from "@/lib/types/system"; function dateLabel(value: string): string { @@ -8,39 +10,75 @@ function dateLabel(value: string): string { return Number.isNaN(parsed.getTime()) ? value : parsed.toLocaleString(); } -export function StorageRunHistory({ runs }: { runs: StorageMaintenanceRun[] }) { +export function StorageRunHistory({ + runs, + loading = false, + error, +}: { + runs: StorageMaintenanceRun[]; + loading?: boolean; + error?: string | null; +}) { + const content = ; return ( Maintenance history - - {runs.length === 0 ? ( -

No storage maintenance runs yet.

- ) : ( - - {runs.map((run) => ( - - - - - {run.state} - - {run.trigger} - {dateLabel(run.started_at)} - - - - {run.message &&

{run.message}

} - 
-                    {JSON.stringify(run.result, null, 2)}
-
- - - ))} - - )} - + {content} ); } + +function StorageRunHistoryContent({ + runs, + loading, + error, +}: { + runs: StorageMaintenanceRun[]; + loading: boolean; + error?: string | null; +}) { + const { t } = useTranslation(); + if (loading) { + return ( +
+ + {t("settings:storageHistoryLoading")} +
+ ); + } + if (error) { + return ( +

+ {t("settings:storageSectionUnavailable")}: {error} +

+ ); + } + if (runs.length === 0) { + return

{t("settings:storageHistoryEmpty")}

; + } + return ( + + {runs.map((run) => ( + + + + + {run.state} + + {run.trigger} + {dateLabel(run.started_at)} + + + + {run.message &&

{run.message}

} + 
+              {JSON.stringify(run.result, null, 2)}
+
+ + + ))} + + ); +} diff --git a/apps/web/components/settings/system/storage/storage-totals.test.ts b/apps/web/components/settings/system/storage/storage-totals.test.ts new file mode 100644 index 0000000000..9e9a1f86ed --- /dev/null +++ b/apps/web/components/settings/system/storage/storage-totals.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "vitest"; +import type { StorageQuarantineEntry, StorageSummary } from "@/lib/types/system"; +import { quarantineTotalBytes, storageAnalysisTotal } from "./storage-totals"; + +const completeSummary: StorageSummary = { + workspaces: { + total_bytes: 10, + active_bytes: 8, + candidate_bytes: 9, + }, + go_cache: { + path: "/var/cache/kandev/go-build", + size_bytes: 3, + owned: true, + unmanaged_path: "/root/.cache/go-build", + unmanaged_size_bytes: 4, + }, + quarantine: { count: 1, size_bytes: 2 }, + docker: { + available: true, + managed_container_count: 1, + managed_container_bytes: 5, + image_layer_bytes: 6, + build_cache_bytes: 7, + unused_image_bytes: 11, + }, +}; + +describe("storageAnalysisTotal", () => { + it("sums non-overlapping top-level measurements", () => { + expect(storageAnalysisTotal(completeSummary)).toEqual({ bytes: 37, partial: false }); + }); + + it("excludes workspace subsets and unused Docker images", () => { + const withoutSubsets = storageAnalysisTotal(completeSummary); + const summary = { + ...completeSummary, + workspaces: { ...completeSummary.workspaces, active_bytes: 0, candidate_bytes: 0 }, + docker: { ...completeSummary.docker, unused_image_bytes: 0 }, + }; + + expect(storageAnalysisTotal(summary)).toEqual(withoutSubsets); + }); + + it("marks unavailable top-level measurements as partial", () => { + const summary: StorageSummary = { + ...completeSummary, + workspaces: { available: false, warning: "workspace inventory unavailable" }, + quarantine: { available: false, warning: "quarantine unavailable" }, + go_cache: { ...completeSummary.go_cache, size_bytes: undefined }, + docker: { ...completeSummary.docker, available: false }, + }; + + expect(storageAnalysisTotal(summary)).toEqual({ bytes: 4, partial: true }); + }); + + it("does not require a user cache when no distinct path is reported", () => { + const summary: StorageSummary = { + ...completeSummary, + go_cache: { + ...completeSummary.go_cache, + unmanaged_path: undefined, + unmanaged_size_bytes: undefined, + }, + }; + + expect(storageAnalysisTotal(summary)).toEqual({ bytes: 33, partial: false }); + }); +}); + +describe("quarantineTotalBytes", () => { + it("sums every listed entry, including zero-byte entries", () => { + const entries = [ + { size_bytes: 5 }, + { size_bytes: 0 }, + { size_bytes: 8 }, + ] as StorageQuarantineEntry[]; + + expect(quarantineTotalBytes(entries)).toBe(13); + expect(quarantineTotalBytes([])).toBe(0); + }); +}); diff --git a/apps/web/components/settings/system/storage/storage-totals.ts b/apps/web/components/settings/system/storage/storage-totals.ts new file mode 100644 index 0000000000..d589f80cd5 --- /dev/null +++ b/apps/web/components/settings/system/storage/storage-totals.ts @@ -0,0 +1,49 @@ +import type { StorageQuarantineEntry, StorageSummary } from "@/lib/types/system"; + +export interface StorageAnalysisTotal { + bytes: number; + partial: boolean; +} + +function isMeasuredBytes(value: number | undefined): value is number { + return value !== undefined && Number.isFinite(value) && value >= 0; +} + +export function storageAnalysisTotal(summary: StorageSummary): StorageAnalysisTotal { + const total: StorageAnalysisTotal = { bytes: 0, partial: false }; + const addMeasurement = (value: number | undefined, available = true) => { + if (!available || !isMeasuredBytes(value)) { + total.partial = true; + return; + } + total.bytes += value; + }; + + addMeasurement(summary.workspaces.total_bytes, summary.workspaces.available !== false); + addMeasurement( + summary.quarantine.available === false ? undefined : summary.quarantine.size_bytes, + ); + addMeasurement(summary.go_cache.size_bytes, summary.go_cache.available !== false); + if (summary.go_cache.unmanaged_path) { + addMeasurement(summary.go_cache.unmanaged_size_bytes); + } + + if (summary.docker.available) { + addMeasurement(summary.docker.managed_container_bytes); + addMeasurement(summary.docker.image_layer_bytes); + addMeasurement(summary.docker.build_cache_bytes); + } else { + addMeasurement(undefined, false); + addMeasurement(undefined, false); + addMeasurement(undefined, false); + } + + return total; +} + +export function quarantineTotalBytes(entries: StorageQuarantineEntry[]): number { + return entries.reduce( + (total, entry) => total + (isMeasuredBytes(entry.size_bytes) ? entry.size_bytes : 0), + 0, + ); +} 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 cf57fe4e90..b4bf02c410 100644 --- a/apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts +++ b/apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts @@ -126,7 +126,7 @@ test.describe("Mobile storage maintenance", () => { .toBe(true); }); - test("shows progress while storage data is loading", async ({ testPage }) => { + test("shows progress while storage data is loading", async ({ testPage, prCapture }) => { let overviewRequestStarted = false; let releaseOverview: () => void = () => {}; let markOverviewObserved: () => void = () => {}; @@ -161,6 +161,15 @@ test.describe("Mobile storage maintenance", () => { await expect(spinner).toBeVisible(); await expect(testPage.getByText("Loading storage data…")).toBeVisible(); await expect(testPage.getByTestId("storage-overview-card")).toBeInViewport(); + await expect(testPage.getByTestId("storage-policy-card")).toBeVisible(); + await expect(testPage.getByTestId("storage-run-history")).toBeVisible(); + await expect(testPage.getByTestId("storage-quarantine-card")).toBeVisible(); + await expect(testPage.getByTestId("storage-analysis-total")).toHaveCount(0); + await expect(testPage.getByTestId("toast-message")).toHaveCount(0); + await prCapture.screenshot("progressive-loading", { + caption: "Mobile storage keeps policy, history, and quarantine visible during analysis", + fullPage: true, + }); expect( await testPage.evaluate(() => document.documentElement.scrollWidth > window.innerWidth), ).toBe(false); @@ -168,6 +177,8 @@ test.describe("Mobile storage maintenance", () => { releaseOverview(); await expect(spinner).toBeHidden(); await expect(testPage.getByText("Storage analysis")).toBeVisible(); + await expect(testPage.getByTestId("storage-analysis-total")).toBeVisible(); + await expect(testPage.getByTestId("storage-quarantine-total")).toBeVisible(); } finally { releaseOverview(); if (overviewRequestStarted) await overviewSettled; diff --git a/apps/web/e2e/tests/system/storage-maintenance.spec.ts b/apps/web/e2e/tests/system/storage-maintenance.spec.ts index c7fc0ac9e4..79d5fc78af 100644 --- a/apps/web/e2e/tests/system/storage-maintenance.spec.ts +++ b/apps/web/e2e/tests/system/storage-maintenance.spec.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import path from "node:path"; +import type { Route } from "@playwright/test"; import { test, expect } from "../../fixtures/test-base"; import { seedManagedGoCache } from "../../helpers/storage-maintenance"; @@ -97,6 +98,68 @@ test.describe("System storage maintenance", () => { } }); + test("shows fast storage sections while the overview scan is pending", async ({ + testPage, + prCapture, + }) => { + 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) => { + const request = route.request(); + const pathname = new URL(request.url()).pathname; + if (request.method() !== "GET" || pathname !== "/api/v1/system/storage") { + await route.continue(); + return; + } + 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; + + await expect(testPage.getByTestId("storage-policy-card")).toBeVisible(); + await expect(testPage.getByTestId("storage-run-history")).toBeVisible(); + await expect(testPage.getByTestId("storage-quarantine-card")).toBeVisible(); + await expect(testPage.getByTestId("storage-overview-spinner")).toBeVisible(); + await expect(testPage.getByTestId("storage-analysis-total")).toHaveCount(0); + await expect(testPage.getByTestId("toast-message")).toHaveCount(0); + await prCapture.screenshot("progressive-loading", { + caption: "Desktop storage shows policy, history, and quarantine while analysis scans", + fullPage: true, + }); + + releaseOverview(); + await expect(testPage.getByTestId("storage-analysis-total")).toBeVisible(); + await expect(testPage.getByTestId("storage-quarantine-total")).toBeVisible(); + await expect(testPage.getByTestId("storage-overview-spinner")).toHaveCount(0); + } finally { + releaseOverview(); + if (overviewRequestStarted) await overviewSettled; + await testPage.unroute(overviewPattern, holdOverview); + } + }); + 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 df51071738..66c8d15e9a 100644 --- a/apps/web/hooks/domains/system/use-storage-maintenance.test.tsx +++ b/apps/web/hooks/domains/system/use-storage-maintenance.test.tsx @@ -3,7 +3,11 @@ import { act, renderHook, waitFor } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { StateProvider } from "@/components/state-provider"; import { ApiError } from "@/lib/api/client"; -import type { StorageMaintenanceSettings, StorageOverviewResponse } from "@/lib/types/system"; +import type { + StorageMaintenanceSettings, + StorageOverviewResponse, + StoragePolicyResponse, +} from "@/lib/types/system"; const mocks = vi.hoisted(() => ({ adopt: vi.fn(), @@ -12,6 +16,7 @@ const mocks = vi.hoisted(() => ({ purge: vi.fn(), fetchJob: vi.fn(), fetchOverview: vi.fn(), + fetchPolicy: vi.fn(), fetchQuarantine: vi.fn(), fetchRuns: vi.fn(), restore: vi.fn(), @@ -31,6 +36,7 @@ vi.mock("@/lib/api/domains/system-api", () => ({ purgeStorageQuarantine: mocks.purge, fetchSystemJob: mocks.fetchJob, fetchStorageOverview: mocks.fetchOverview, + fetchStoragePolicy: mocks.fetchPolicy, fetchStorageQuarantine: mocks.fetchQuarantine, fetchStorageRuns: mocks.fetchRuns, restoreStorageQuarantine: mocks.restore, @@ -99,10 +105,12 @@ const TEST_COMMAND_BUSY_LABEL = "A test command is running"; function deferred() { let resolve!: (value: T) => void; - const promise = new Promise((resolvePromise) => { + let reject!: (reason?: unknown) => void; + const promise = new Promise((resolvePromise, rejectPromise) => { resolve = resolvePromise; + reject = rejectPromise; }); - return { promise, resolve }; + return { promise, resolve, reject }; } function wrapper({ children }: { children: ReactNode }) { @@ -112,6 +120,10 @@ function wrapper({ children }: { children: ReactNode }) { beforeEach(() => { vi.clearAllMocks(); mocks.fetchOverview.mockResolvedValue(overview); + mocks.fetchPolicy.mockResolvedValue({ + settings: overview.settings, + capabilities: overview.capabilities, + }); mocks.fetchRuns.mockResolvedValue([]); mocks.fetchQuarantine.mockResolvedValue([]); mocks.fetchJob.mockResolvedValue(cleanupJob); @@ -122,6 +134,29 @@ beforeEach(() => { }); describe("useStorageMaintenance", () => { + it("publishes fast sections before a cold overview scan finishes", async () => { + const overviewRequest = deferred(); + mocks.fetchOverview.mockReturnValueOnce(overviewRequest.promise); + + const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); + + await waitFor(() => expect(result.current.policy?.settings).toEqual(settings)); + expect(result.current.loading).toMatchObject({ + policy: false, + runs: false, + quarantine: false, + overview: true, + }); + expect(result.current.overview).toBeNull(); + + await act(async () => { + overviewRequest.resolve(overview); + await overviewRequest.promise; + }); + await waitFor(() => expect(result.current.overview).toEqual(overview)); + expect(result.current.loading.overview).toBe(false); + }); + it("loads overview, run history, and quarantine through the domain controller", async () => { const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); await waitFor(() => expect(result.current.overview).toEqual(overview)); @@ -143,6 +178,20 @@ describe("useStorageMaintenance", () => { }); }); + it("refreshes policy after save without starting another overview scan", async () => { + const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); + await waitFor(() => expect(result.current.overview).toEqual(overview)); + mocks.fetchOverview.mockClear(); + mocks.fetchPolicy.mockClear(); + + await act(async () => { + await result.current.save(settings); + }); + + expect(mocks.fetchPolicy).toHaveBeenCalledTimes(1); + expect(mocks.fetchOverview).not.toHaveBeenCalled(); + }); + it("starts eligible and forced quarantine bulk jobs", async () => { const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); await waitFor(() => expect(result.current.overview).toEqual(overview)); @@ -463,4 +512,59 @@ describe("useStorageMaintenance reload ordering", () => { expect(result.current.overview).toEqual(newerOverview); }); + + it("does not surface a stale reload failure after a newer result commits", async () => { + const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); + await waitFor(() => expect(result.current.overview).toEqual(overview)); + const olderResponse = deferred(); + const newerOverview = { + ...overview, + settings: { ...overview.settings, idle_for_minutes: 24 }, + }; + mocks.fetchOverview + .mockReturnValueOnce(olderResponse.promise) + .mockResolvedValueOnce(newerOverview); + + let olderReload!: Promise; + await act(async () => { + olderReload = result.current.reload(["overview"]); + await result.current.reload(["overview"]); + }); + await waitFor(() => expect(result.current.overview).toEqual(newerOverview)); + + await act(async () => { + olderResponse.reject(new Error("stale overview unavailable")); + await olderReload; + }); + + expect(result.current.overview).toEqual(newerOverview); + expect(result.current.sectionErrors.overview).toBeNull(); + }); + + it("does not let a stale policy response overwrite go-cache adoption", async () => { + const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); + await waitFor(() => expect(result.current.overview).toEqual(overview)); + const policyResponse = deferred(); + const adoptedSettings = { + ...settings, + go_cache: { ...settings.go_cache, adopted_path: "/custom/go-build" }, + }; + const adoptedResponse = { settings: adoptedSettings, capabilities: overview.capabilities }; + mocks.fetchPolicy.mockReturnValueOnce(policyResponse.promise); + mocks.adopt.mockResolvedValueOnce(adoptedResponse); + + let staleReload!: Promise; + await act(async () => { + staleReload = result.current.reload(["policy"]); + await result.current.adopt("/custom/go-build"); + }); + expect(result.current.policy?.settings).toEqual(adoptedSettings); + + await act(async () => { + policyResponse.resolve({ settings, capabilities: overview.capabilities }); + await staleReload; + }); + + expect(result.current.policy?.settings).toEqual(adoptedSettings); + }); }); diff --git a/apps/web/hooks/domains/system/use-storage-maintenance.ts b/apps/web/hooks/domains/system/use-storage-maintenance.ts index 16116e66cf..f4d3000efe 100644 --- a/apps/web/hooks/domains/system/use-storage-maintenance.ts +++ b/apps/web/hooks/domains/system/use-storage-maintenance.ts @@ -17,6 +17,7 @@ import { analyzeStorage, deleteStorageQuarantine, fetchStorageOverview, + fetchStoragePolicy, fetchStorageQuarantine, fetchStorageRuns, purgeStorageQuarantine, @@ -28,13 +29,13 @@ import type { StorageBusyResource, StorageBusyResponse, StorageMaintenanceSettings, + StoragePolicyResponse, StorageQuarantinePurgeScope, SystemJob, } from "@/lib/types/system"; import { useSystemJob } from "./use-system-jobs"; export type StoragePendingAction = - | "load" | "save" | "analyze" | "run" @@ -89,7 +90,10 @@ export function settingsWithDockerAcknowledgement( }; } -type Reload = () => Promise; +export type StorageSection = "policy" | "overview" | "runs" | "quarantine"; +export type StorageSectionLoading = Record; +export type StorageSectionErrors = Record; +type Reload = (sections?: StorageSection[]) => Promise; type SetStorageError = Dispatch>; const TERMINAL_REFRESH_RETRY_MS = 1000; const TERMINAL_REFRESH_MAX_RETRY_MS = 8000; @@ -97,23 +101,20 @@ const MAX_TERMINAL_REFRESH_ATTEMPTS = 6; function useStorageActionRunner() { const { toast } = useToast(); - const [loading, setLoading] = useState(true); const [pendingActions, setPendingActions] = useState< - Array<{ id: number; action: Exclude }> + Array<{ id: number; action: Exclude }> >([]); const nextPendingActionId = useRef(0); const [error, setError] = useState(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]); + }, [pendingActions]); const perform = useCallback( async ( - action: Exclude, + action: Exclude, work: () => Promise, rethrow = false, ) => { @@ -133,7 +134,7 @@ function useStorageActionRunner() { }, [toast], ); - return { pendingAction, error, setError, finishLoading, perform }; + return { pendingAction, error, setError, perform }; } function useStoragePolicyActions( @@ -141,6 +142,7 @@ function useStoragePolicyActions( reload: Reload, toast: ReturnType["toast"], clearBusy: () => void, + setPolicy: (policy: StoragePolicyResponse) => void, ) { const save = useCallback( async (settings: StorageMaintenanceSettings, confirmation?: "DEDICATED") => { @@ -149,7 +151,7 @@ function useStoragePolicyActions( "save", async () => { await saveStorageSettings(settings, confirmation); - await reload(); + await reload(["policy"]); toast({ title: "Storage policy saved", variant: "success" }); }, true, @@ -162,12 +164,12 @@ function useStoragePolicyActions( async (path: string) => { clearBusy(); return perform("adopt", async () => { - await adoptStorageGoCache(path); - await reload(); + const response = await adoptStorageGoCache(path); + setPolicy(response); toast({ title: "Go build cache adopted", variant: "success" }); }); }, - [clearBusy, perform, reload, toast], + [clearBusy, perform, setPolicy, toast], ); return { save, adopt }; @@ -217,9 +219,9 @@ function useStorageBulkDeleteAction( ); } -function useStorageActions(reload: Reload) { +function useStorageActions(reload: Reload, setPolicy: (policy: StoragePolicyResponse) => void) { const { toast } = useToast(); - const { pendingAction, error, setError, finishLoading, perform } = useStorageActionRunner(); + const { pendingAction, error, setError, perform } = useStorageActionRunner(); const [analysisJobId, setAnalysisJobId] = useState(null); const [cleanupJobId, setCleanupJobId] = useState(null); const [deleteJobId, setDeleteJobId] = useState(null); @@ -228,7 +230,7 @@ function useStorageActions(reload: Reload) { const cleanupJob = useSystemJob(cleanupJobId); const deleteJob = useSystemJob(deleteJobId); const clearBusy = useCallback(() => setBusy(null), []); - const { save, adopt } = useStoragePolicyActions(perform, reload, toast, clearBusy); + const { save, adopt } = useStoragePolicyActions(perform, reload, toast, clearBusy, setPolicy); const analyze = useCallback(async () => { clearBusy(); @@ -286,7 +288,7 @@ function useStorageActions(reload: Reload) { clearBusy(); return perform("restore", async () => { await restoreStorageQuarantine(id); - await reload(); + await reload(["quarantine"]); toast({ title: "Quarantined resource restored", variant: "success" }); }); }, @@ -298,7 +300,6 @@ function useStorageActions(reload: Reload) { return { pendingAction, error, - finishLoading, setError, save, analyze, @@ -365,35 +366,94 @@ function useReloadCompletedJobs( export function useStorageMaintenance() { const storage = useAppStore((state) => state.system.storage); + const setPolicy = useAppStore((state) => state.setSystemStoragePolicy); const setOverview = useAppStore((state) => state.setSystemStorageOverview); const setRuns = useAppStore((state) => state.setSystemStorageRuns); const setQuarantine = useAppStore((state) => state.setSystemStorageQuarantine); - const reloadGeneration = useRef(0); - const reload = useCallback(async () => { - const generation = ++reloadGeneration.current; - const [overview, runs, quarantine] = await Promise.all([ - fetchStorageOverview(), - fetchStorageRuns(20), - fetchStorageQuarantine(), - ]); - if (generation !== reloadGeneration.current) return; - setOverview(overview); - setRuns(runs); - setQuarantine(quarantine); - }, [setOverview, setQuarantine, setRuns]); - const { finishLoading, setError, ...actions } = useStorageActions(reload); + const [loading, setLoading] = useState({ + policy: true, + overview: true, + runs: true, + quarantine: true, + }); + const [sectionErrors, setSectionErrors] = useState({ + policy: null, + overview: null, + runs: null, + quarantine: null, + }); + const sectionGenerations = useRef>({ + policy: 0, + overview: 0, + runs: 0, + quarantine: 0, + }); + const loadSection = useCallback( + async (section: StorageSection, request: () => Promise, commit: (value: T) => void) => { + const generation = ++sectionGenerations.current[section]; + setLoading((current) => ({ ...current, [section]: true })); + setSectionErrors((current) => ({ ...current, [section]: null })); + try { + const value = await request(); + if (generation === sectionGenerations.current[section]) commit(value); + } catch (requestError) { + if (generation === sectionGenerations.current[section]) { + setSectionErrors((current) => ({ + ...current, + [section]: messageFromError(requestError), + })); + throw requestError; + } + } finally { + if (generation === sectionGenerations.current[section]) { + setLoading((current) => ({ ...current, [section]: false })); + } + } + }, + [], + ); + const reload = useCallback( + async (sections: StorageSection[] = ["policy", "overview", "runs", "quarantine"]) => { + const jobs = sections.map((section) => { + switch (section) { + case "policy": + return loadSection("policy", fetchStoragePolicy, setPolicy); + case "overview": + return loadSection("overview", fetchStorageOverview, setOverview); + case "runs": + return loadSection("runs", () => fetchStorageRuns(20), setRuns); + case "quarantine": + return loadSection("quarantine", fetchStorageQuarantine, setQuarantine); + } + }); + const results = await Promise.allSettled(jobs); + const failure = results.find( + (result): result is PromiseRejectedResult => result.status === "rejected", + ); + if (failure) throw failure.reason; + }, + [loadSection, setOverview, setPolicy, setQuarantine, setRuns], + ); + const commitAdoptedPolicy = useCallback( + (policy: StoragePolicyResponse) => { + sectionGenerations.current.policy += 1; + setPolicy(policy); + setSectionErrors((current) => ({ ...current, policy: null })); + setLoading((current) => ({ ...current, policy: false })); + }, + [setPolicy], + ); + const actions = useStorageActions(reload, commitAdoptedPolicy); useEffect(() => { - void reload() - .catch((requestError) => setError(messageFromError(requestError))) - .finally(finishLoading); - }, [finishLoading, reload, setError]); + void reload().catch(() => undefined); + }, [reload]); useReloadCompletedJobs( reload, - setError, + actions.setError, actions.analysisJob, actions.cleanupJob, actions.deleteJob, ); - return { ...storage, ...actions, reload }; + return { ...storage, ...actions, loading, sectionErrors, reload }; } diff --git a/apps/web/lib/api/domains/system-api.test.ts b/apps/web/lib/api/domains/system-api.test.ts index 928b871733..cff2a69e00 100644 --- a/apps/web/lib/api/domains/system-api.test.ts +++ b/apps/web/lib/api/domains/system-api.test.ts @@ -31,6 +31,7 @@ import { analyzeStorage, deleteStorageQuarantine, fetchStorageOverview, + fetchStoragePolicy, fetchStorageQuarantine, fetchStorageRuns, purgeStorageQuarantine, @@ -312,31 +313,31 @@ describe("restart", () => { }); }); -describe("storage maintenance", () => { - const settings = { - enabled: false, - check_interval_hours: 24, - idle_for_minutes: 10, - orphan_grace_hours: 168, - quarantine_retention_hours: 168, - workspaces: { enabled: true }, - kandev_containers: { enabled: true }, - go_cache: { enabled: false, max_bytes: 16106127360, adopted_path: "" }, - docker: { - dedicated_daemon_acknowledged: false, - build_cache_enabled: false, - build_cache_keep_bytes: 10737418240, - build_cache_unused_hours: 168, - unused_images_enabled: false, - unused_images_hours: 168, - }, - }; +const storageSettings = { + enabled: false, + check_interval_hours: 24, + idle_for_minutes: 10, + orphan_grace_hours: 168, + quarantine_retention_hours: 168, + workspaces: { enabled: true }, + kandev_containers: { enabled: true }, + go_cache: { enabled: false, max_bytes: 16106127360, adopted_path: "" }, + docker: { + dedicated_daemon_acknowledged: false, + build_cache_enabled: false, + build_cache_keep_bytes: 10737418240, + build_cache_unused_hours: 168, + unused_images_enabled: false, + unused_images_hours: 168, + }, +}; +describe("storage maintenance", () => { it("loads overview and list resources without caching", async () => { fetchSpy .mockResolvedValueOnce( jsonResponse({ - settings, + settings: storageSettings, summary: {}, capabilities: {}, analyzed_at: "2026-07-23T12:00:00Z", @@ -355,19 +356,19 @@ describe("storage maintenance", () => { }); it("saves dedicated Docker acknowledgement with its confirmation", async () => { - fetchSpy.mockResolvedValueOnce(jsonResponse({ settings })); - await saveStorageSettings(settings, "DEDICATED"); + fetchSpy.mockResolvedValueOnce(jsonResponse({ settings: storageSettings })); + await saveStorageSettings(storageSettings, "DEDICATED"); expect(lastCall().url).toBe(`${BASE}/storage/settings`); expect(method()).toBe("PATCH"); expect(JSON.parse(String(lastCall().init?.body))).toEqual({ - settings, + settings: storageSettings, confirmations: { dedicated_docker: "DEDICATED" }, }); }); it("uses fixed confirmations for Go-cache adoption and permanent deletion", async () => { fetchSpy - .mockResolvedValueOnce(jsonResponse({ settings, capabilities: {} })) + .mockResolvedValueOnce(jsonResponse({ settings: storageSettings, capabilities: {} })) .mockResolvedValueOnce(jsonResponse({ job_id: "delete-job" })); await adoptStorageGoCache("/root/.cache/go-build"); expect(JSON.parse(String(lastCall().init?.body))).toEqual({ @@ -416,3 +417,18 @@ describe("storage maintenance", () => { expect((await restoreStorageQuarantine("entry-1")).id).toBe("restored"); }); }); + +describe("storage policy", () => { + it("loads storage policy without requesting the scan-backed overview", async () => { + fetchSpy.mockResolvedValueOnce( + jsonResponse({ settings: storageSettings, capabilities: { docker_available: true } }), + ); + + const response = await fetchStoragePolicy(); + + expect(lastCall().url).toBe(`${BASE}/storage/settings`); + expect(lastCall().init?.cache).toBe("no-store"); + expect(response.settings).toEqual(storageSettings); + expect(response.capabilities.docker_available).toBe(true); + }); +}); diff --git a/apps/web/lib/api/domains/system-api.ts b/apps/web/lib/api/domains/system-api.ts index 75552221de..c0ff12e2ab 100644 --- a/apps/web/lib/api/domains/system-api.ts +++ b/apps/web/lib/api/domains/system-api.ts @@ -15,6 +15,7 @@ import type { StorageMaintenanceRun, StorageMaintenanceSettings, StorageOverviewResponse, + StoragePolicyResponse, StorageQuarantineEntry, StorageQuarantinePurgeScope, StorageSettingsResponse, @@ -205,6 +206,13 @@ export function fetchStorageOverview( }); } +export function fetchStoragePolicy(options?: ApiRequestOptions): Promise { + return fetchJson(`${SYSTEM_BASE}/storage/settings`, { + ...options, + cache: "no-store", + }); +} + export function saveStorageSettings( settings: StorageMaintenanceSettings, dedicatedDockerConfirmation?: "DEDICATED", 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 c6cc613f67..6872742e88 100644 --- a/apps/web/lib/state/slices/system/system-slice.test.ts +++ b/apps/web/lib/state/slices/system/system-slice.test.ts @@ -134,10 +134,17 @@ describe("system storage slice", () => { analyzed_at: "2026-07-23T12:00:00Z", last_run: null, } satisfies StorageOverviewResponse; + const policy = { settings: overview.settings, capabilities: overview.capabilities }; + store.getState().setSystemStoragePolicy(policy); store.getState().setSystemStorageOverview(overview); store.getState().setSystemStorageRuns([]); store.getState().setSystemStorageQuarantine([]); - expect(store.getState().system.storage).toEqual({ overview, runs: [], quarantine: [] }); + expect(store.getState().system.storage).toEqual({ + policy, + overview, + runs: [], + quarantine: [], + }); }); }); diff --git a/apps/web/lib/state/slices/system/system-slice.ts b/apps/web/lib/state/slices/system/system-slice.ts index 0474be1c43..af799e82a2 100644 --- a/apps/web/lib/state/slices/system/system-slice.ts +++ b/apps/web/lib/state/slices/system/system-slice.ts @@ -11,7 +11,7 @@ export const defaultSystemState: SystemSliceState = { updates: null, jobs: {}, metrics: null, - storage: { overview: null, runs: [], quarantine: [] }, + storage: { policy: null, overview: null, runs: [], quarantine: [] }, }, }; @@ -67,6 +67,10 @@ export const createSystemSlice: StateCreator< set((draft) => { draft.system.metrics = snapshot; }), + setSystemStoragePolicy: (policy) => + set((draft) => { + draft.system.storage.policy = policy; + }), setSystemStorageOverview: (overview) => set((draft) => { draft.system.storage.overview = overview; diff --git a/apps/web/lib/state/slices/system/types.ts b/apps/web/lib/state/slices/system/types.ts index a9cc2291d7..f85489d3cf 100644 --- a/apps/web/lib/state/slices/system/types.ts +++ b/apps/web/lib/state/slices/system/types.ts @@ -9,6 +9,7 @@ import type { SystemMetricsSnapshot, StorageMaintenanceRun, StorageOverviewResponse, + StoragePolicyResponse, StorageQuarantineEntry, } from "@/lib/types/system"; @@ -36,6 +37,7 @@ export type SystemSliceState = { jobs: SystemJobsMap; metrics: SystemMetricsSnapshot | null; storage: { + policy: StoragePolicyResponse | null; overview: StorageOverviewResponse | null; runs: StorageMaintenanceRun[]; quarantine: StorageQuarantineEntry[]; @@ -54,6 +56,7 @@ export type SystemSliceActions = { upsertSystemJob: (job: SystemJob) => void; clearSystemJob: (jobId: string) => void; setSystemMetricsSnapshot: (snapshot: SystemMetricsSnapshot) => void; + setSystemStoragePolicy: (policy: StoragePolicyResponse) => void; setSystemStorageOverview: (overview: StorageOverviewResponse) => void; setSystemStorageRuns: (runs: StorageMaintenanceRun[]) => void; setSystemStorageQuarantine: (entries: StorageQuarantineEntry[]) => void; diff --git a/apps/web/lib/types/system.ts b/apps/web/lib/types/system.ts index b8296db9c0..881dc5b2ef 100644 --- a/apps/web/lib/types/system.ts +++ b/apps/web/lib/types/system.ts @@ -325,6 +325,11 @@ export interface StorageOverviewResponse { last_run: StorageMaintenanceRun | null; } +export interface StoragePolicyResponse { + settings: StorageMaintenanceSettings; + capabilities: StorageCapabilities; +} + export interface StorageSettingsResponse { settings: StorageMaintenanceSettings; } diff --git a/apps/web/src/locales/en/settings.json b/apps/web/src/locales/en/settings.json index 583f00343a..1323aeb49c 100644 --- a/apps/web/src/locales/en/settings.json +++ b/apps/web/src/locales/en/settings.json @@ -331,6 +331,18 @@ "supportedEditors": "Supported Editors", "supportsPlaceholders": "Supports placeholders: {{placeholders}}", "systemDefault": "System default", + "storageLoadingData": "Loading storage data…", + "storageAdoptionPending": "Wait for Go cache adoption to finish.", + "storagePolicyLoadingBlock": "Wait for storage settings to finish loading.", + "storageActionPending": "Wait for the current storage action to finish.", + "storageHistoryLoading": "Loading maintenance history…", + "storageHistoryEmpty": "No storage maintenance runs yet.", + "storageQuarantineLoading": "Loading quarantine…", + "storageQuarantineEmpty": "No restorable quarantined resources.", + "storageQuarantineTotal": "Total quarantined: {{size}}", + "storageSectionUnavailable": "This storage section could not be loaded", + "storageTotalCounted": "Total counted: {{size}}", + "storageTotalPartial": "Partial measurement", "systemLoad1Min": "System load (1 min)", "taskActions": "Task Actions", "taskOverview": "Task overview", diff --git a/apps/web/src/locales/pseudo/settings.json b/apps/web/src/locales/pseudo/settings.json index 06a43bdd6f..016d1fe07a 100644 --- a/apps/web/src/locales/pseudo/settings.json +++ b/apps/web/src/locales/pseudo/settings.json @@ -331,6 +331,18 @@ "supportedEditors": "Śũƥƥōŕţēď Ēďĩţōŕś", "supportsPlaceholders": "Śũƥƥōŕţś ƥĺàćēĥōĺďēŕś: {{placeholders}}", "systemDefault": "Śŷśţēḿ ďēƒàũĺţ", + "storageLoadingData": "Ĺōàďĩńĝ śţōŕàĝē ďàţà…", + "storageAdoptionPending": "Ŵàĩţ ƒōŕ Ĝō ćàćĥē àďōƥţĩōń ţō ƒĩńĩśĥ.", + "storagePolicyLoadingBlock": "Ŵàĩţ ƒōŕ śţōŕàĝē śēţţĩńĝś ţō ƒĩńĩśĥ ĺōàďĩńĝ.", + "storageActionPending": "Ŵàĩţ ƒōŕ ţĥē ćũŕŕēńţ śţōŕàĝē àćţĩōń ţō ƒĩńĩśĥ.", + "storageHistoryLoading": "Ĺōàďĩńĝ ḿàĩńţēńàńćē ĥĩśţōŕŷ…", + "storageHistoryEmpty": "Ńō śţōŕàĝē ḿàĩńţēńàńćē ŕũńś ŷēţ.", + "storageQuarantineLoading": "Ĺōàďĩńĝ qũàŕàńţĩńē…", + "storageQuarantineEmpty": "Ńō ŕēśţōŕàƀĺē qũàŕàńţĩńēď ŕēśōũŕćēś.", + "storageQuarantineTotal": "Ţōţàĺ qũàŕàńţĩńēď: {{size}}", + "storageSectionUnavailable": "Ţĥĩś śţōŕàĝē śēćţĩōń ćōũĺď ńōţ ƀē ĺōàďēď", + "storageTotalCounted": "Ţōţàĺ ćōũńţēď: {{size}}", + "storageTotalPartial": "Ƥàŕţĩàĺ ḿēàśũŕēḿēńţ", "systemLoad1Min": "Śŷśţēḿ ĺōàď (1 ḿĩń)", "taskActions": "Ţàśķ Àćţĩōńś", "taskOverview": "Ţàśķ ōvēŕvĩēŵ", diff --git a/docs/plans/storage-progressive-loading/plan.md b/docs/plans/storage-progressive-loading/plan.md new file mode 100644 index 0000000000..f1c25a5953 --- /dev/null +++ b/docs/plans/storage-progressive-loading/plan.md @@ -0,0 +1,147 @@ +--- +spec: docs/specs/system-page/storage-maintenance.md +created: 2026-08-02 +status: complete +--- + +# Implementation Plan: Progressive Storage Loading and Totals + +## Overview + +Decouple the lightweight Storage policy read from the scan-backed overview, then let the frontend +publish policy, history, quarantine, and analysis results independently. Add derived totals for the +analysis snapshot and current quarantine list, with explicit partial semantics when a top-level +measurement is unavailable. Preserve the existing `GET /storage` response for compatibility. + +## Backend + +### Lightweight policy read + +- In `apps/backend/internal/system/storage/handler.go`, register + `GET /api/v1/system/storage/settings` beside the existing `PATCH` route. +- Implement `getSettings` by calling `SettingsManager.GetSettings` and + `OverviewReader.SettingsCapabilities`; return `{ settings, capabilities }` without calling + `OverviewReader.Get`, `RunLister.ListRuns`, or any analysis provider. +- Keep `GET /api/v1/system/storage` unchanged so existing clients still receive + `{ settings, capabilities, summary, analyzed_at, last_run }`. +- Cover the route directly in `apps/backend/internal/system/storage/handler_test.go` with a + recording overview reader that fails the test if `Get` is invoked, and through the composed + System router in `apps/backend/internal/system/storage_routes_test.go`. + +## Frontend + +### API and state contracts + +- Add `StoragePolicyResponse` to `apps/web/lib/types/system.ts` and + `fetchStoragePolicy()` to `apps/web/lib/api/domains/system-api.ts` for the new lightweight GET. +- Add `policy: StoragePolicyResponse | null` and `setSystemStoragePolicy` to the System Zustand + slice in `apps/web/lib/state/slices/system/types.ts` and `system-slice.ts`. +- Extend the existing API and slice tests for the route, response shape, and policy update. + +### Independent loading and refresh + +- Refactor `apps/web/hooks/domains/system/use-storage-maintenance.ts` so policy, overview, runs, + and quarantine requests start together but commit independently. Track per-section loading and + error state plus per-section request generations so a slow older response cannot overwrite newer + data from the same section. +- Remove first-load completion from the global `pendingAction`; policy-save coordination depends + on `policyLoading`, while analysis, history, and quarantine can remain independently pending. +- Make save refresh only the lightweight policy contract, and apply the existing adoption response + directly to policy state. Restore refreshes quarantine without waiting for an overview scan. + Terminal analysis/cleanup/delete jobs refresh the affected sections concurrently so history and + quarantine can settle even when the invalidated overview needs a new scan. +- Update `storage-maintenance-settings.tsx` to derive drafts and scheduling copy from `policy`, and + pass section loading/error state to the analysis, history, and quarantine cards. +- Give `storage-overview-card.tsx`, `storage-run-history.tsx`, and + `storage-quarantine-card.tsx` distinct progress, empty, and error states; a failure in one card + must not suppress sibling cards. + +### Totals + +- Add `apps/web/components/settings/system/storage/storage-totals.ts` with pure helpers that: + - sum workspace total, quarantine, managed and distinct user Go caches, managed-container + writable layers, Docker image layers, and Docker build cache; + - exclude active/candidate workspace and unused-image subset bytes to avoid double-counting; + - return a `partial` flag when a top-level measurement is unavailable or missing; and + - sum `size_bytes` across every listed quarantine entry. +- Render a localized **Total counted** value and partial indicator in + `storage-overview-card.tsx`, and a localized total in `storage-quarantine-card.tsx`. +- Add the new user-facing strings to `apps/web/src/locales/en/settings.json`; do not localize + identifiers, test IDs, confirmation tokens, or values used in logic. + +### Mobile design contract + +- Desktop outcome: the existing full-width card stack renders lightweight cards before analysis, + and totals appear in the relevant card headers without changing actions or navigation. +- Mobile entry point and exemplar: keep the Settings sheet → System → Storage path and existing + one-column card composition proven by + `apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts`. +- Mobile hierarchy: policy, history, and quarantine remain ordinary inline cards in the page's + single document scroll; Storage analysis owns only its card-level progress state. No drawer, + overlay, fixed control, or second scroll owner is introduced. +- Touch behavior and state are shared with desktop. Existing action targets stay at least 44px; + the totals are read-only and add no touch affordance. The mobile E2E scenario delays analysis, + proves the other cards and totals render, and asserts no horizontal document overflow. + +## Tests + +- **Lightweight policy route:** `apps/backend/internal/system/storage/handler_test.go` verifies the + response and proves no overview scan or run-history read occurs. +- **Composed route:** `apps/backend/internal/system/storage_routes_test.go` verifies + `GET /api/v1/system/storage/settings` is registered and returns `200`. +- **API and store:** `apps/web/lib/api/domains/system-api.test.ts` and + `apps/web/lib/state/slices/system/system-slice.test.ts` verify the new policy contract. +- **Progressive controller:** `apps/web/hooks/domains/system/use-storage-maintenance.test.tsx` + uses deferred promises to prove policy, runs, and quarantine publish before overview; covers + isolated failures, same-section stale-response protection, and mutation-specific refreshes. +- **Totals:** `apps/web/components/settings/system/storage/storage-totals.test.ts` verifies exact + sums, subset exclusion, partial analysis, zero/empty data, and quarantine aggregation. +- **Rendered states:** `storage-maintenance-settings.test.tsx`, + `storage-overview-card.test.tsx`, `storage-run-history.test.tsx`, and + `storage-quarantine-card.test.tsx` verify policy composition, loading/error/empty distinctions, + and localized totals. + +## E2E Tests + +- **Desktop progressive load:** extend + `apps/web/e2e/tests/system/storage-maintenance.spec.ts` to delay `GET /storage`, then verify the + policy controls, maintenance history, and quarantine list are visible before analysis finishes; + after release, verify the analysis total and quarantine total. +- **Mobile parity:** extend + `apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts` with the same delayed-analysis + outcome on `mobile-chrome`, including totals and absence of horizontal overflow. + +## Verification Results + +Completed. Each task records its exact commands and outcomes in its `## Results` section. + +## Implementation Waves And Parallel Candidates + +Wave 1: + +- [x] [Task 01: Lightweight policy endpoint](task-01-policy-endpoint.md) — done + +Wave 2: + +- [x] [Task 02: Progressive section loading](task-02-progressive-loading.md) — done + +Wave 3: + +- [x] [Task 03: Storage totals](task-03-storage-totals.md) — done + +Wave 4: + +- [x] [Task 04: Desktop and mobile E2E](task-04-storage-e2e.md) — done + +The tasks run sequentially in the primary conversation. The waves do not authorize subagents. + +## Risks + +- Docker unused-image bytes are a reclaimable subset of image-layer usage; adding both would + overstate the total. The pure total helper and tests make that exclusion explicit. +- Cleanup and restore invalidate the overview cache, so an automatic overview refresh may scan + again. Section-specific refreshes must let history and quarantine update without awaiting it. +- Empty arrays currently mean both “not loaded” and “loaded with no items”; explicit section state + is required to prevent misleading empty-state copy during requests. +- Capability reads ping Docker. They are expected to be much cheaper than `DiskUsage` and + filesystem traversal, but remain isolated from the overview scan and failure-scoped to policy. diff --git a/docs/plans/storage-progressive-loading/task-01-policy-endpoint.md b/docs/plans/storage-progressive-loading/task-01-policy-endpoint.md new file mode 100644 index 0000000000..422096e871 --- /dev/null +++ b/docs/plans/storage-progressive-loading/task-01-policy-endpoint.md @@ -0,0 +1,67 @@ +--- +id: "01-policy-endpoint" +title: "Lightweight policy endpoint" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/system-page/storage-maintenance.md" +--- + +# Task 01: Lightweight policy endpoint + +## Acceptance + +- `GET /api/v1/system/storage/settings` returns persisted settings and current capabilities. +- The lightweight GET does not request an overview snapshot, scan providers, or read run history. +- The existing scan-backed `GET /api/v1/system/storage` contract remains unchanged. + +## Verification + +From `apps/backend`: + +```bash +rtk go test ./internal/system/storage ./internal/system +``` + +## Files likely touched + +- `apps/backend/internal/system/storage/handler.go` +- `apps/backend/internal/system/storage/handler_test.go` +- `apps/backend/internal/system/storage_routes_test.go` + +## Dependencies + +None. + +## Parallelism + +Sequential. This contract is required by Task 02. + +## Inputs + +- Spec: What, API surface, failure modes, and first-load scenarios +- Plan: Backend → Lightweight policy read +- Existing `SettingsManager`, `OverviewReader.Capabilities`, and Storage route tests + +## Risks + +- Do not call `OverviewReader.Get` from the new handler. +- Keep internal settings read errors from exposing sensitive details beyond the existing contract. + +## Output contract + +Report the route and response shape, files changed, exact test result, blockers/risks, and update +this task plus `plan.md` status in the same conversation. + +## Results + +- RED: `rtk go test -run TestGetStorageSettingsReturnsPolicyWithoutOverviewScan ./internal/system/storage` + failed as expected with HTTP 404 before the route existed. +- GREEN/final: `rtk go test ./internal/system/storage ./internal/system` — 107 tests passed. +- Added `GET /api/v1/system/storage/settings`; it returns settings and settings-derived capabilities + without calling the scan-backed overview reader or probing Docker. Existing `GET /storage` behavior + remains unchanged. +- Settings-load failures now use a client-safe response while preserving the original error in the + server-side logging callback. +- No external side effects or cleanup required. diff --git a/docs/plans/storage-progressive-loading/task-02-progressive-loading.md b/docs/plans/storage-progressive-loading/task-02-progressive-loading.md new file mode 100644 index 0000000000..8585b975cb --- /dev/null +++ b/docs/plans/storage-progressive-loading/task-02-progressive-loading.md @@ -0,0 +1,96 @@ +--- +id: "02-progressive-loading" +title: "Progressive section loading" +status: done +wave: 2 +depends_on: ["01-policy-endpoint"] +plan: "plan.md" +spec: "../../specs/system-page/storage-maintenance.md" +--- + +# Task 02: Progressive section loading + +## Acceptance + +- Policy, history, and quarantine publish and render without waiting for the overview request. +- Each section distinguishes loading, empty, ready, and failed states without hiding successful + sibling sections or accepting stale same-section responses. +- Save, adoption, restore, and terminal-job refreshes request only the data they need and never make + lightweight section updates wait for an invalidated overview scan. + +## Verification + +From `apps/`, install dependencies once if absent: + +```bash +rtk pnpm install --frozen-lockfile +rtk pnpm --filter @kandev/web test -- --run lib/api/domains/system-api.test.ts lib/state/slices/system/system-slice.test.ts hooks/domains/system/use-storage-maintenance.test.tsx components/settings/system/storage/storage-maintenance-settings.test.tsx components/settings/system/storage/storage-overview-card.test.tsx components/settings/system/storage/storage-run-history.test.tsx components/settings/system/storage/storage-quarantine-card.test.tsx +``` + +From `apps/`: + +```bash +rtk pnpm --filter @kandev/web run typecheck +``` + +The focused suite and typecheck run from the `apps/` workspace so pnpm resolves the monorepo +workspace consistently. + +## Files likely touched + +- `apps/web/lib/types/system.ts` +- `apps/web/lib/api/domains/system-api.ts` +- `apps/web/lib/api/domains/system-api.test.ts` +- `apps/web/lib/state/slices/system/types.ts` +- `apps/web/lib/state/slices/system/system-slice.ts` +- `apps/web/lib/state/slices/system/system-slice.test.ts` +- `apps/web/hooks/domains/system/use-storage-maintenance.ts` +- `apps/web/hooks/domains/system/use-storage-maintenance.test.tsx` +- `apps/web/components/settings/system/storage/storage-maintenance-settings.tsx` +- `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-run-history.tsx` +- `apps/web/components/settings/system/storage/storage-run-history.test.tsx` +- `apps/web/components/settings/system/storage/storage-quarantine-card.tsx` +- `apps/web/components/settings/system/storage/storage-quarantine-card.test.tsx` + +## Dependencies + +Task 01. + +## Parallelism + +Sequential. It establishes the section state consumed by Tasks 03 and 04. + +## Inputs + +- Spec: independent loading, isolated failures, and persistence scenarios +- Plan: Frontend → API and state contracts; Independent loading and refresh +- Existing controller generation guard and terminal refresh retry tests + +## Risks + +- Preserve settings drafts when policy responses arrive; never overwrite an unsaved user edit. +- Avoid turning a section failure into an unhandled promise rejection or a permanent loading state. +- Reconcile these likely files with the actual diff before marking the task done. + +## Output contract + +Report section state behavior, mutation refresh behavior, files changed, exact test results, +blockers/risks, and update this task plus `plan.md` status in the same conversation. + +## Results + +- Added the lightweight `GET /api/v1/system/storage/settings` client contract and persisted policy + state separately from the scan-backed overview. +- Split storage loading into policy, overview, history, and quarantine sections with per-section + loading/error state and generation guards so stale responses cannot overwrite newer data. +- Policy save/adoption and quarantine restore now refresh only their affected sections; terminal job + refreshes still reload all sections for a consistent post-run snapshot. +- Stale section failures no longer escape newer successful reloads, and Go-cache adoption invalidates + in-flight policy reads before committing its response. +- Rendered independent loading/error cards for policy, history, and quarantine while overview scanning + remains in progress. +- RED: API/store-focused tests initially failed because the new policy client and setter were absent. +- GREEN: `rtk pnpm --filter @kandev/web test -- --run lib/api/domains/system-api.test.ts lib/state/slices/system/system-slice.test.ts hooks/domains/system/use-storage-maintenance.test.tsx components/settings/system/storage/storage-maintenance-settings.test.tsx components/settings/system/storage/storage-overview-card.test.tsx components/settings/system/storage/storage-run-history.test.tsx components/settings/system/storage/storage-quarantine-card.test.tsx` — 7 files, 85 tests passed. +- GREEN: `rtk pnpm --filter @kandev/web run typecheck` from `apps` passed. diff --git a/docs/plans/storage-progressive-loading/task-03-storage-totals.md b/docs/plans/storage-progressive-loading/task-03-storage-totals.md new file mode 100644 index 0000000000..5ee9ef1966 --- /dev/null +++ b/docs/plans/storage-progressive-loading/task-03-storage-totals.md @@ -0,0 +1,82 @@ +--- +id: "03-storage-totals" +title: "Storage totals" +status: done +wave: 3 +depends_on: ["02-progressive-loading"] +plan: "plan.md" +spec: "../../specs/system-page/storage-maintenance.md" +--- + +# Task 03: Storage totals + +## Acceptance + +- Storage analysis shows the sum of available non-overlapping top-level measurements and visibly + identifies a partial total when any required category is unavailable or missing. +- Analysis excludes active/candidate workspace bytes and unused-image bytes because those values + are subsets of already counted categories. +- Quarantine shows the sum of every currently listed entry's `size_bytes`, including zero/empty + state, without depending on the overview snapshot. + +## Verification + +From `apps/`, install dependencies once if absent: + +```bash +rtk pnpm install --frozen-lockfile +rtk pnpm --filter @kandev/web test -- components/settings/system/storage/storage-totals.test.ts components/settings/system/storage/storage-overview-card.test.tsx components/settings/system/storage/storage-quarantine-card.test.tsx +rtk pnpm --filter @kandev/web i18n:check +rtk pnpm --filter @kandev/web i18n:ratchet +``` + +## Files likely touched + +- `apps/web/components/settings/system/storage/storage-totals.ts` +- `apps/web/components/settings/system/storage/storage-totals.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-quarantine-card.tsx` +- `apps/web/components/settings/system/storage/storage-quarantine-card.test.tsx` +- `apps/web/src/locales/en/settings.json` + +## Dependencies + +Task 02. + +## Parallelism + +Sequential. It uses Task 02's loaded-section contract and supplies selectors for Task 04. + +## Inputs + +- Spec: total-counted and quarantine-total requirements and scenarios +- Plan: Frontend → Totals +- Existing `formatGigabytes` presentation helper + +## Risks + +- Keep the total a derived UI value; do not persist it or change byte-based API fields. +- Do not translate confirmation tokens, resource IDs, or values used for comparisons. + +## Output contract + +Report aggregation semantics, localized presentation, files changed, exact test results, +blockers/risks, and update this task plus `plan.md` status in the same conversation. + +## Results + +- Added pure `storageAnalysisTotal` and `quarantineTotalBytes` helpers. The analysis total adds + workspace total bytes, quarantine bytes, managed and distinct user Go caches, managed-container + writable layers, Docker image layers, and Docker build cache; active/candidate workspace bytes + and unused-image bytes remain excluded subsets. +- Missing, invalid, or unavailable top-level measurements set `partial: true` while preserving all + valid measurements in the displayed total. Quarantine totals sum every listed entry independently + of the overview snapshot. +- Added localized total labels and a partial-measurement badge to the analysis card, plus a + localized quarantine total. +- RED: `storage-totals.test.ts` initially failed because the new helper module did not exist. +- GREEN: `rtk pnpm --filter @kandev/web test -- --run components/settings/system/storage/storage-totals.test.ts components/settings/system/storage/storage-overview-card.test.tsx components/settings/system/storage/storage-quarantine-card.test.tsx` — 3 files, 13 tests passed. +- GREEN: `rtk pnpm --filter @kandev/web i18n:check` passed. +- GREEN: `rtk pnpm --filter @kandev/web i18n:ratchet` passed. +- GREEN: `rtk pnpm run typecheck` from `apps/web` passed. diff --git a/docs/plans/storage-progressive-loading/task-04-storage-e2e.md b/docs/plans/storage-progressive-loading/task-04-storage-e2e.md new file mode 100644 index 0000000000..47bfee3fe4 --- /dev/null +++ b/docs/plans/storage-progressive-loading/task-04-storage-e2e.md @@ -0,0 +1,83 @@ +--- +id: "04-storage-e2e" +title: "Storage desktop and mobile E2E" +status: done +wave: 4 +depends_on: ["03-storage-totals"] +plan: "plan.md" +spec: "../../specs/system-page/storage-maintenance.md" +--- + +# Task 04: Storage Desktop and Mobile E2E + +## Acceptance + +- Desktop E2E delays the scan-backed overview and proves policy, history, and quarantine render + first, then verifies analysis and quarantine totals after the delayed response settles. +- Mobile E2E proves the same user value through Settings sheet → System → Storage on the existing + one-column composition. +- The mobile state retains touch-reachable controls and has no horizontal document overflow. + +## Verification + +From `apps/`, install dependencies once if absent: + +```bash +rtk pnpm install --frozen-lockfile +``` + +From `apps/web`: + +```bash +rtk pnpm e2e:run tests/system/storage-maintenance.spec.ts +rtk pnpm e2e:run --project mobile-chrome tests/system/mobile-storage-maintenance.spec.ts +``` + +Confirm each command discovers the intended test count. The managed runner rebuilds the Go backend +and production Vite assets before executing and tears down its isolated runtime afterward. + +## Files likely touched + +- `apps/web/e2e/tests/system/storage-maintenance.spec.ts` +- `apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts` +- Existing Storage component files only if stable `data-testid` selectors are missing + +## Dependencies + +Task 03. + +## Parallelism + +Sequential; this is integrated proof of Tasks 01–03. + +## Inputs + +- Spec: first-load, totals, failure isolation, and mobile scenarios +- Plan: E2E Tests and Mobile design contract +- Existing delayed-overview mobile test and Storage settings navigation flow + +## Risks + +- Route patterns must distinguish `GET /storage` from `GET /storage/settings`. +- E2E assertions must observe fast sections before releasing the overview gate, not merely after all + requests have settled. + +## Output contract + +Report desktop/mobile flows, discovered test counts, exact command outcomes, artifact or teardown +evidence, blockers/risks, and update this task plus `plan.md` status in the same conversation. + +## Results + +- Added a desktop delayed-overview scenario that distinguishes `GET /api/v1/system/storage` from + the lightweight settings route and verifies policy, history, and quarantine cards before the + scan-backed overview is released. It then verifies both derived totals. +- Extended the mobile delayed-overview scenario with the same fast-section and totals assertions, + while retaining the no-horizontal-overflow check. +- GREEN: `rtk pnpm e2e:run tests/system/storage-maintenance.spec.ts` — 6 tests discovered, 6 passed + in 15.9s. +- GREEN: `rtk pnpm e2e:run --project mobile-chrome tests/system/mobile-storage-maintenance.spec.ts` + — 4 tests discovered, 4 passed in 10.3s. +- Retained PR capture artifacts: `storage-maintenance--progressive-loading.png` and + `mobile-storage-maintenance--progressive-loading.png`; the managed runners completed teardown + after each capture. diff --git a/docs/specs/system-page/storage-maintenance.md b/docs/specs/system-page/storage-maintenance.md index b568e9bfda..da4f82f588 100644 --- a/docs/specs/system-page/storage-maintenance.md +++ b/docs/specs/system-page/storage-maintenance.md @@ -1,7 +1,7 @@ --- status: building created: 2026-07-14 -updated: 2026-07-29 +updated: 2026-08-02 owner: cfl --- @@ -23,6 +23,13 @@ reclaim that space without maintaining cron or systemd configuration outside Kan - The page presents storage analysis and maintenance policy as separate full-width sections. Analysis and cleanup state replaces the label and icon inside the action button that started it instead of appearing as detached page status. +- On first load, maintenance policy, maintenance history, quarantine, and storage analysis load as + independent sections. A cold filesystem or Docker scan keeps only Storage analysis in its loading + state; policy controls, persisted run history, and the database-backed quarantine list render as + soon as their own requests finish. +- Each independently loaded section surfaces its own loading and failure state. A failed or slow + analysis never replaces already available policy, history, or quarantine content with a page-wide + loading or error state. - User-facing storage totals and editable size limits are shown in GB. The frontend converts those values to and from the byte-based API without changing the persisted data model. - Maintenance settings use separate cards grouped by scope: schedule, workspaces and containers, @@ -36,6 +43,12 @@ 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. +- Storage analysis shows a total counted size derived from the available non-overlapping top-level + measurements: total task workspaces, quarantine, managed and distinct user Go caches, + Kandev-managed container writable layers, Docker image layers, and Docker build cache. Active and + candidate workspace bytes and unused-image bytes remain visible subset measurements and are not + added again. If any top-level measurement is unavailable, the total is visibly identified as + partial rather than presented as complete host disk usage. - 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 @@ -84,6 +97,9 @@ reclaim that space without maintaining cron or systemd configuration outside Kan the actual automatic deletion occurs on the first successful scheduled maintenance run after that time. When scheduling is disabled, it says that automatic deletion is off and names full manual **Run now** or an explicit quarantine action as the available cleanup paths. +- The Quarantine section shows the sum of `size_bytes` for every currently listed restorable entry. + Its total is derived from the independently loaded quarantine list and does not wait for or depend + on the Storage analysis snapshot. - Scheduled and full manual maintenance runs permanently delete eligible quarantine entries. Resource-specific manual runs do not delete unrelated quarantine entries. - **Clear eligible** permanently deletes every active quarantine entry whose retention deadline has @@ -317,6 +333,9 @@ All routes are under the existing authenticated System route group. GET /api/v1/system/storage -> { settings, capabilities, summary, analyzed_at, last_run } +GET /api/v1/system/storage/settings + -> { settings, capabilities } + PATCH /api/v1/system/storage/settings body: { settings: complete StorageMaintenanceSettings object, @@ -366,6 +385,11 @@ DELETE /api/v1/system/storage/quarantine availability, configured Docker host, and whether host-global Docker cleanup is allowed. API responses never expose secret environment values. +`GET /storage/settings` is the lightweight policy-read contract. It reads persisted settings and +capabilities without requesting an overview snapshot or invoking filesystem, Go-cache, quarantine, +or Docker analysis providers. `GET /storage` remains backward compatible and continues to return +the complete scan-backed overview contract. + `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. @@ -492,6 +516,9 @@ the configured restore window. ambiguous-state, or Git worktree-pruning failure keeps the affected entry and reports the error. - Docker list/usage failure marks Docker analysis unavailable. Docker prune failure records the daemon error and does not affect other providers. +- A policy, history, quarantine, or analysis read failure is isolated to that section. Other + successful section responses remain visible and usable, and retrying or completing one section + does not discard newer data already returned by another. - Loss of the dedicated-daemon acknowledgment between analysis and cleanup cancels host-global Docker operations. - Failure to persist a run or cleanup intent prevents its destructive operation from starting. @@ -521,6 +548,11 @@ the configured restore window. 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** the first Storage analysis after backend startup is still scanning, **WHEN** the + lightweight policy, history, and quarantine requests complete, **THEN** those three sections are + visible and usable while only Storage analysis continues to show progress. +- **GIVEN** one Storage section request fails, **WHEN** another section request succeeds, **THEN** + the successful section renders its current data and the failed section shows its own error state. - **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. @@ -546,6 +578,11 @@ the configured restore window. - **GIVEN** task roots include active, recent orphan, and grace-eligible orphan directories, **WHEN** storage analysis runs, **THEN** total workspace bytes include every classified task root while active and reclaimable bytes remain separate subsets. +- **GIVEN** all analysis providers return measurements, **WHEN** Storage analysis renders its total, + **THEN** it sums the non-overlapping top-level measurements once and does not add active, + candidate, or unused-image subset bytes again. +- **GIVEN** one top-level analysis measurement is unavailable, **WHEN** Storage analysis renders its + total, **THEN** it sums the available measurements and identifies the result as partial. - **GIVEN** archived or deleted tasks retain ready environment or active worktree rows for recovery, **WHEN** storage analysis or cleanup classifies their old directories, **THEN** those historical rows do not protect the directories from normal orphan grace and quarantine rules unless a live @@ -562,6 +599,9 @@ the configured restore window. - **GIVEN** protected and eligible quarantine entries, **WHEN** the Storage page renders, **THEN** each row shows its exact `delete_after` timestamp and protected-or-eligible status, and the page states whether automatic scheduled cleanup is enabled. +- **GIVEN** multiple restorable quarantine entries, **WHEN** the independently loaded Quarantine + section renders, **THEN** its total equals the sum of every listed entry's `size_bytes` without + waiting for Storage analysis. - **GIVEN** protected and eligible quarantine entries, **WHEN** the user confirms **Clear eligible** with `DELETE ELIGIBLE`, **THEN** every eligible entry is permanently deleted, protected entries remain, and the completed job reports both groups. @@ -645,3 +685,4 @@ the configured restore window. - [Original Storage maintenance implementation](../../plans/storage-maintenance/plan.md) - [Storage overview cache and settings follow-up](../../plans/storage-overview-cache/plan.md) - [Quarantine lifecycle follow-up](../../plans/quarantine-lifecycle/plan.md) +- [Progressive Storage loading and totals](../../plans/storage-progressive-loading/plan.md)