diff --git a/apps/backend/internal/backendapp/storage_maintenance.go b/apps/backend/internal/backendapp/storage_maintenance.go index d2733978c6..4e216e7521 100644 --- a/apps/backend/internal/backendapp/storage_maintenance.go +++ b/apps/backend/internal/backendapp/storage_maintenance.go @@ -70,7 +70,11 @@ func provideStorageComposition( homeDir: cfg.ResolvedHomeDir(), } cachedOverview := storagepkg.NewOverviewCache(overview) - providers := storageCleanupProviders(settings, workspaceFactory, goCache, dockerProvider) + quarantine := &workspaceQuarantineController{ + settings: settings, store: store, factory: workspaceFactory, homeDir: cfg.ResolvedHomeDir(), + activity: coordinator, + } + providers := storageCleanupProviders(settings, workspaceFactory, goCache, dockerProvider, quarantine) runner := storagepkg.NewRunner(storagepkg.RunnerConfig{ Activity: coordinator, Store: store, Providers: providers, Overview: cachedOverview, }) @@ -79,10 +83,6 @@ func provideStorageComposition( Scheduler: scheduler, Settings: settings, Worker: taskSvc, Reconciler: &workspaceReconciler{settings: settings, factory: workspaceFactory}, }) - quarantine := &workspaceQuarantineController{ - settings: settings, store: store, factory: workspaceFactory, homeDir: cfg.ResolvedHomeDir(), - activity: coordinator, - } operations := storagepkg.NewOperations(storagepkg.OperationsConfig{ Settings: settings, Store: store, Jobs: tracker, Activity: coordinator, Providers: providers, Overview: cachedOverview, GoCache: goCache, Quarantine: quarantine, @@ -191,6 +191,21 @@ type namedCleanupProvider struct { cleanup func(context.Context) (map[string]any, error) } +type quarantinePurger interface { + Purge(context.Context, storagepkg.QuarantinePurgeScope, string) (storagepkg.QuarantinePurgeResult, error) +} + +type quarantineCleanupProvider struct { + purger quarantinePurger +} + +func (p quarantineCleanupProvider) Name() string { return "quarantine" } + +func (p quarantineCleanupProvider) Cleanup(ctx context.Context) (map[string]any, error) { + result, err := p.purger.Purge(ctx, storagepkg.QuarantinePurgeScopeEligible, storagepkg.QuarantineConfirmationEligible) + return toMap(result), err +} + type goCacheCleanupProvider struct { provider *gocache.Provider } @@ -215,8 +230,10 @@ func storageCleanupProviders( workspaceFactory workspaceFactory, goCache *gocache.Provider, docker *dockerstore.Provider, + quarantine quarantinePurger, ) []storagepkg.CleanupProvider { return []storagepkg.CleanupProvider{ + quarantineCleanupProvider{purger: quarantine}, workspaceCleanupAdapter(settings, workspaceFactory), goCacheCleanupProvider{provider: goCache}, dockerContainerCleanupAdapter(settings, docker), @@ -312,11 +329,67 @@ type workspaceQuarantineController struct { type quarantineEntryStore interface { GetQuarantineEntry(context.Context, string) (storagepkg.QuarantineEntry, error) + ListQuarantineEntries(context.Context, bool) ([]storagepkg.QuarantineEntry, error) TransitionQuarantineEntry( context.Context, string, storagepkg.QuarantineState, string, ) (storagepkg.QuarantineEntry, error) } +func (c *workspaceQuarantineController) Purge( + ctx context.Context, + scope storagepkg.QuarantinePurgeScope, + confirmation string, +) (storagepkg.QuarantinePurgeResult, error) { + result := storagepkg.QuarantinePurgeResult{Scope: scope} + force := false + switch scope { + case storagepkg.QuarantinePurgeScopeEligible: + if confirmation != storagepkg.QuarantineConfirmationEligible { + return result, fmt.Errorf("%w: eligible quarantine purge requires %s confirmation", storagepkg.ErrValidation, storagepkg.QuarantineConfirmationEligible) + } + case storagepkg.QuarantinePurgeScopeAll: + if confirmation != storagepkg.QuarantineConfirmationForce { + return result, storagepkg.ErrForceDeleteConfirmation + } + force = true + default: + return result, fmt.Errorf("%w: unknown quarantine purge scope %q", storagepkg.ErrValidation, scope) + } + entries, err := c.store.ListQuarantineEntries(ctx, false) + if err != nil { + return result, err + } + now := time.Now().UTC() + var purgeErrs []error + for _, entry := range entries { + if err := ctx.Err(); err != nil { + return result, err + } + result.Considered++ + if !force && now.Before(entry.DeleteAfter) { + result.Protected++ + result.ProtectedBytes += entry.SizeBytes + continue + } + var deleted storagepkg.QuarantineEntry + if force { + deleted, err = c.PermanentDeleteForce(ctx, entry.ID, confirmation) + } else { + deleted, err = c.PermanentDelete(ctx, entry.ID, storagepkg.QuarantineConfirmationDelete) + } + if err != nil { + result.Failed++ + result.FailedBytes += entry.SizeBytes + result.Failures = append(result.Failures, storagepkg.QuarantinePurgeFailure{ID: entry.ID, Error: err.Error()}) + purgeErrs = append(purgeErrs, fmt.Errorf("%s: %w", entry.ID, err)) + continue + } + result.Deleted++ + result.DeletedBytes += deleted.SizeBytes + } + return result, errors.Join(purgeErrs...) +} + const ( goCacheOwnershipManaged = "managed" goCacheOwnershipAdopted = "adopted" @@ -362,12 +435,32 @@ func (c *workspaceQuarantineController) PermanentDelete( ctx context.Context, id string, confirmation string, +) (storagepkg.QuarantineEntry, error) { + return c.permanentDelete(ctx, id, confirmation, false) +} + +func (c *workspaceQuarantineController) PermanentDeleteForce( + ctx context.Context, + id string, + confirmation string, +) (storagepkg.QuarantineEntry, error) { + return c.permanentDelete(ctx, id, confirmation, true) +} + +func (c *workspaceQuarantineController) permanentDelete( + ctx context.Context, + id string, + confirmation string, + force bool, ) (storagepkg.QuarantineEntry, error) { entry, err := c.store.GetQuarantineEntry(ctx, id) if err != nil { return storagepkg.QuarantineEntry{}, err } if entry.ResourceType == storagepkg.ResourceTypeGoCache { + if force { + return c.deleteGoCacheForce(ctx, entry, confirmation) + } return c.deleteGoCache(ctx, entry, confirmation) } if entry.ResourceType != storagepkg.ResourceTypeTaskWorkspace { @@ -377,6 +470,9 @@ func (c *workspaceQuarantineController) PermanentDelete( if err != nil { return storagepkg.QuarantineEntry{}, err } + if force { + return c.factory(settings).PermanentDeleteForce(ctx, id, confirmation) + } return c.factory(settings).PermanentDelete(ctx, id, confirmation) } @@ -482,13 +578,34 @@ func (c *workspaceQuarantineController) deleteGoCache( entry storagepkg.QuarantineEntry, confirmation string, ) (storagepkg.QuarantineEntry, error) { - if confirmation != "DELETE" { + return c.deleteGoCacheWithRetention(ctx, entry, confirmation, false) +} + +func (c *workspaceQuarantineController) deleteGoCacheForce( + ctx context.Context, + entry storagepkg.QuarantineEntry, + confirmation string, +) (storagepkg.QuarantineEntry, error) { + return c.deleteGoCacheWithRetention(ctx, entry, confirmation, true) +} + +func (c *workspaceQuarantineController) deleteGoCacheWithRetention( + ctx context.Context, + entry storagepkg.QuarantineEntry, + confirmation string, + force bool, +) (storagepkg.QuarantineEntry, error) { + if force { + if confirmation != storagepkg.QuarantineConfirmationForce { + return storagepkg.QuarantineEntry{}, storagepkg.ErrForceDeleteConfirmation + } + } else if confirmation != storagepkg.QuarantineConfirmationDelete { return storagepkg.QuarantineEntry{}, fmt.Errorf("%w: quarantine deletion requires DELETE confirmation", storagepkg.ErrValidation) } if err := c.validateGoCacheEntry(ctx, entry); err != nil { return storagepkg.QuarantineEntry{}, err } - if time.Now().UTC().Before(entry.DeleteAfter) { + if !force && time.Now().UTC().Before(entry.DeleteAfter) { return storagepkg.QuarantineEntry{}, fmt.Errorf("%w: quarantine retention deadline has not elapsed", storagepkg.ErrConflict) } if err := c.rejectAmbiguousMissingGoCachePayload(entry); err != nil { @@ -498,7 +615,7 @@ func (c *workspaceQuarantineController) deleteGoCache( return storagepkg.QuarantineEntry{}, fmt.Errorf("delete quarantined Go cache: %w", err) } deleted, err := c.store.TransitionQuarantineEntry( - ctx, entry.ID, storagepkg.QuarantineStateDeleted, "", + context.WithoutCancel(ctx), entry.ID, storagepkg.QuarantineStateDeleted, "", ) if err != nil { return storagepkg.QuarantineEntry{}, fmt.Errorf("persist Go-cache deletion: %w", err) diff --git a/apps/backend/internal/backendapp/storage_maintenance_test.go b/apps/backend/internal/backendapp/storage_maintenance_test.go index 32bcc1b9bb..ae7c02e3ce 100644 --- a/apps/backend/internal/backendapp/storage_maintenance_test.go +++ b/apps/backend/internal/backendapp/storage_maintenance_test.go @@ -393,6 +393,118 @@ func TestQuarantineControllerRetriesFailedGoCacheDelete(t *testing.T) { } } +func TestQuarantineControllerForceDeletesProtectedGoCache(t *testing.T) { + home := t.TempDir() + original := filepath.Join(home, "cache", "go-build") + quarantined := filepath.Join(home, "trash", "go-cache", "entry-cache") + if err := os.MkdirAll(quarantined, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(quarantined, "artifact"), []byte("cache"), 0o600); err != nil { + t.Fatal(err) + } + settings, store := newStorageMaintenanceStores(t) + entry := createGoCacheQuarantineEntry(t, store, original, quarantined, time.Now().UTC().Add(time.Hour)) + controller := &workspaceQuarantineController{settings: settings, store: store, homeDir: home} + + if _, err := controller.PermanentDeleteForce(context.Background(), entry.ID, "DELETE"); !errors.Is(err, storagepkg.ErrForceDeleteConfirmation) { + t.Fatalf("wrong force confirmation error = %v, want %v", err, storagepkg.ErrForceDeleteConfirmation) + } + deleted, err := controller.PermanentDeleteForce(context.Background(), entry.ID, "DELETE ALL NOW") + if err != nil { + t.Fatalf("PermanentDeleteForce: %v", err) + } + if deleted.State != storagepkg.QuarantineStateDeleted { + t.Fatalf("state = %q, want deleted", deleted.State) + } + if _, err := os.Stat(quarantined); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("quarantine path still exists: %v", err) + } +} + +func TestQuarantineControllerPurgeEligibleReportsProtectedAndDeleted(t *testing.T) { + home := t.TempDir() + settings, store := newStorageMaintenanceStores(t) + protected := storagepkg.QuarantineEntry{ + ID: "protected-workspace", ResourceType: storagepkg.ResourceTypeTaskWorkspace, + OriginalPath: filepath.Join(home, "tasks", "protected"), + QuarantinePath: filepath.Join(home, "trash", "tasks", "protected-workspace"), + SizeBytes: 17, State: storagepkg.QuarantineStateQuarantined, + QuarantinedAt: time.Now().UTC(), DeleteAfter: time.Now().UTC().Add(time.Hour), + } + if err := os.MkdirAll(protected.QuarantinePath, 0o700); err != nil { + t.Fatal(err) + } + if err := store.CreateQuarantineEntry(context.Background(), &protected); err != nil { + t.Fatal(err) + } + eligible := createGoCacheQuarantineEntryWithID( + t, store, home, "eligible-cache", time.Now().UTC().Add(-time.Hour), + ) + controller := &workspaceQuarantineController{settings: settings, store: store, homeDir: home} + + result, err := controller.Purge(context.Background(), storagepkg.QuarantinePurgeScopeEligible, "DELETE ELIGIBLE") + if err != nil { + t.Fatalf("Purge: %v", err) + } + if result.Considered != 2 || result.Deleted != 1 || result.Protected != 1 || result.Failed != 0 { + t.Fatalf("purge result = %#v, want considered=2 deleted=1 protected=1 failed=0", result) + } + if result.DeletedBytes != eligible.SizeBytes || result.ProtectedBytes != protected.SizeBytes { + t.Fatalf("purge bytes = deleted:%d protected:%d, want deleted:%d protected:%d", result.DeletedBytes, result.ProtectedBytes, eligible.SizeBytes, protected.SizeBytes) + } + if _, err := os.Stat(eligible.QuarantinePath); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("eligible quarantine path still exists: %v", err) + } + if _, err := os.Stat(protected.QuarantinePath); err != nil { + t.Fatalf("protected quarantine path changed: %v", err) + } +} + +func TestQuarantineCleanupProviderPurgesEligibleEntries(t *testing.T) { + purger := &recordingQuarantinePurger{} + provider := quarantineCleanupProvider{purger: purger} + + result, err := provider.Cleanup(context.Background()) + if err != nil { + t.Fatalf("Cleanup: %v", err) + } + if purger.scope != storagepkg.QuarantinePurgeScopeEligible || purger.confirmation != "DELETE ELIGIBLE" { + t.Fatalf("purge request = scope:%q confirmation:%q", purger.scope, purger.confirmation) + } + if result["deleted"] != float64(2) || result["protected"] != float64(1) { + t.Fatalf("provider result = %#v, want deleted=2 protected=1", result) + } +} + +func TestStorageCleanupProvidersIncludesQuarantineProvider(t *testing.T) { + providers := storageCleanupProviders(nil, nil, nil, nil, &recordingQuarantinePurger{}) + if len(providers) != 6 { + t.Fatalf("provider count = %d, want 6", len(providers)) + } + if providers[0].Name() != "quarantine" { + t.Fatalf("first provider = %q, want quarantine", providers[0].Name()) + } + if providers[1].Name() != "workspaces" || providers[2].Name() != "go_cache" { + t.Fatalf("provider order = %q, %q, want workspaces, go_cache", providers[1].Name(), providers[2].Name()) + } +} + +type recordingQuarantinePurger struct { + scope storagepkg.QuarantinePurgeScope + confirmation string +} + +func (p *recordingQuarantinePurger) Purge( + _ context.Context, + scope storagepkg.QuarantinePurgeScope, + confirmation string, +) (storagepkg.QuarantinePurgeResult, error) { + p.scope = scope + p.confirmation = confirmation + return storagepkg.QuarantinePurgeResult{Deleted: 2, Protected: 1}, nil +} + func TestQuarantineControllerDoesNotTreatPopulatedReplacementAsRestoredPayload(t *testing.T) { states := []storagepkg.QuarantineState{ storagepkg.QuarantineStateQuarantined, @@ -539,6 +651,12 @@ func (s *failingTransitionQuarantineStore) GetQuarantineEntry( return s.entry, nil } +func (s *failingTransitionQuarantineStore) ListQuarantineEntries( + context.Context, bool, +) ([]storagepkg.QuarantineEntry, error) { + return []storagepkg.QuarantineEntry{s.entry}, nil +} + func (s *failingTransitionQuarantineStore) TransitionQuarantineEntry( _ context.Context, _ string, @@ -712,6 +830,40 @@ func createGoCacheQuarantineEntry( return entry } +func createGoCacheQuarantineEntryWithID( + t *testing.T, + store *storagepkg.Store, + home string, + id string, + deleteAfter time.Time, +) storagepkg.QuarantineEntry { + t.Helper() + original := filepath.Join(home, "cache", "go-build") + quarantined := filepath.Join(home, "trash", "go-cache", id) + if err := os.MkdirAll(quarantined, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(quarantined, "artifact"), []byte("cache"), 0o600); err != nil { + t.Fatal(err) + } + ownership := "managed" + metadata, err := json.Marshal(map[string]string{"ownership": ownership}) + if err != nil { + t.Fatal(err) + } + entry := storagepkg.QuarantineEntry{ + ID: id, ResourceType: storagepkg.ResourceTypeGoCache, + OriginalPath: original, QuarantinePath: quarantined, + State: storagepkg.QuarantineStateQuarantined, + QuarantinedAt: time.Now().UTC().Add(-2 * time.Hour), DeleteAfter: deleteAfter, + Metadata: metadata, + } + if err := store.CreateQuarantineEntry(context.Background(), &entry); err != nil { + t.Fatal(err) + } + return entry +} + func markGoCacheQuarantineFailed(t *testing.T, store *storagepkg.Store, id string) { t.Helper() if _, err := store.TransitionQuarantineEntry( diff --git a/apps/backend/internal/system/jobs/jobs.go b/apps/backend/internal/system/jobs/jobs.go index b99ac2525f..631680ee50 100644 --- a/apps/backend/internal/system/jobs/jobs.go +++ b/apps/backend/internal/system/jobs/jobs.go @@ -29,10 +29,13 @@ const ( // Job is a tracked unit of long-running work. type Job struct { - ID string `json:"id"` - Kind string `json:"kind"` - State State `json:"state"` - Message string `json:"message,omitempty"` + ID string `json:"id"` + Kind string `json:"kind"` + State State `json:"state"` + Message string `json:"message,omitempty"` + // Result may contain partial progress even when State is failed. A job + // closure can return both a result and an error so best-effort operations + // can report committed work alongside the failure. Result map[string]interface{} `json:"result,omitempty"` StartedAt time.Time `json:"started_at"` EndedAt time.Time `json:"ended_at,omitempty"` @@ -93,13 +96,13 @@ func (t *Tracker) run(ctx context.Context, job *Job, fn func(ctx context.Context t.transition(ctx, job.ID, func(j *Job) { j.EndedAt = time.Now().UTC() + j.Result = result if err != nil { j.State = StateFailed j.Message = err.Error() return } j.State = StateSucceeded - j.Result = result }) } diff --git a/apps/backend/internal/system/storage/handler.go b/apps/backend/internal/system/storage/handler.go index 5fd828dc30..0a1297b525 100644 --- a/apps/backend/internal/system/storage/handler.go +++ b/apps/backend/internal/system/storage/handler.go @@ -28,6 +28,7 @@ type Mutations interface { RunNow(context.Context, []string, bool) (string, error) RestoreQuarantine(context.Context, string) (QuarantineEntry, error) DeleteQuarantine(context.Context, string, string) (string, error) + PurgeQuarantine(context.Context, QuarantinePurgeScope, string) (string, error) } type Capabilities struct { @@ -86,6 +87,7 @@ func RegisterRoutes(group *gin.RouterGroup, handler *Handler) { group.GET("/storage/runs", handler.listRuns) group.GET("/storage/quarantine", handler.listQuarantine) group.POST("/storage/quarantine/:id/restore", handler.restoreQuarantine) + group.DELETE("/storage/quarantine", handler.deleteQuarantineBulk) group.DELETE("/storage/quarantine/:id", handler.deleteQuarantine) } @@ -172,7 +174,7 @@ type confirmationRequest struct { func (h *Handler) deleteQuarantine(c *gin.Context) { var request confirmationRequest - if err := c.ShouldBindJSON(&request); err != nil || request.Confirm != "DELETE" { + if err := c.ShouldBindJSON(&request); err != nil || request.Confirm != QuarantineConfirmationDelete { c.JSON(http.StatusBadRequest, gin.H{"error": "quarantine deletion requires DELETE confirmation"}) return } @@ -180,6 +182,27 @@ func (h *Handler) deleteQuarantine(c *gin.Context) { writeAcceptedJob(c, id, err) } +type bulkQuarantineRequest struct { + Scope QuarantinePurgeScope `json:"scope" binding:"required"` + Confirm string `json:"confirm" binding:"required"` +} + +func (h *Handler) deleteQuarantineBulk(c *gin.Context) { + var request bulkQuarantineRequest + if err := c.ShouldBindJSON(&request); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + if (request.Scope == QuarantinePurgeScopeEligible && request.Confirm != QuarantineConfirmationEligible) || + (request.Scope == QuarantinePurgeScopeAll && request.Confirm != QuarantineConfirmationForce) || + (request.Scope != QuarantinePurgeScopeEligible && request.Scope != QuarantinePurgeScopeAll) { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid quarantine purge scope or confirmation"}) + return + } + id, err := h.config.Mutations.PurgeQuarantine(c.Request.Context(), request.Scope, request.Confirm) + writeAcceptedJob(c, id, err) +} + func writeAcceptedJob(c *gin.Context, id string, err error) { if err != nil { writeMutationError(c, err) diff --git a/apps/backend/internal/system/storage/handler_test.go b/apps/backend/internal/system/storage/handler_test.go index be9adf020c..556bdbef5b 100644 --- a/apps/backend/internal/system/storage/handler_test.go +++ b/apps/backend/internal/system/storage/handler_test.go @@ -71,6 +71,54 @@ func TestGetStorageReturnsSnapshotAnalyzedAt(t *testing.T) { } } +func TestDeleteQuarantineBulkValidatesConfirmation(t *testing.T) { + gin.SetMode(gin.TestMode) + mutations := &recordingMutations{} + router := gin.New() + RegisterRoutes(router.Group("/api/v1/system"), NewHandler(HandlerConfig{Mutations: mutations})) + + for _, test := range []struct { + body string + want int + }{ + {`{"scope":"eligible","confirm":"DELETE ALL NOW"}`, http.StatusBadRequest}, + {`{"scope":"all","confirm":"DELETE ELIGIBLE"}`, http.StatusBadRequest}, + {`{"scope":"eligible","confirm":"DELETE ELIGIBLE"}`, http.StatusAccepted}, + {`{"scope":"all","confirm":"DELETE ALL NOW"}`, http.StatusAccepted}, + } { + request := httptest.NewRequest(http.MethodDelete, "/api/v1/system/storage/quarantine", strings.NewReader(test.body)) + request.Header.Set("Content-Type", "application/json") + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + if response.Code != test.want { + t.Fatalf("body %s status = %d, want %d", test.body, response.Code, test.want) + } + } + if mutations.purgeCalls != 2 { + t.Fatalf("purge calls = %d, want 2", mutations.purgeCalls) + } +} + +type recordingMutations struct{ purgeCalls int } + +func (m *recordingMutations) AdoptGoCache(context.Context, string, string) (StorageMaintenanceSettings, Capabilities, error) { + return DefaultSettings(), Capabilities{}, nil +} +func (m *recordingMutations) Analyze(context.Context) (string, error) { return "job", nil } +func (m *recordingMutations) RunNow(context.Context, []string, bool) (string, error) { + return "job", nil +} +func (m *recordingMutations) RestoreQuarantine(context.Context, string) (QuarantineEntry, error) { + return QuarantineEntry{}, nil +} +func (m *recordingMutations) DeleteQuarantine(context.Context, string, string) (string, error) { + return "job", nil +} +func (m *recordingMutations) PurgeQuarantine(context.Context, QuarantinePurgeScope, string) (string, error) { + m.purgeCalls++ + return "job", nil +} + type failingSettingsManager struct{ err error } func (f failingSettingsManager) GetSettings(context.Context) (StorageMaintenanceSettings, error) { diff --git a/apps/backend/internal/system/storage/operations.go b/apps/backend/internal/system/storage/operations.go index 2af35dc17c..83b79e9d3a 100644 --- a/apps/backend/internal/system/storage/operations.go +++ b/apps/backend/internal/system/storage/operations.go @@ -24,6 +24,7 @@ type GoCacheAdopter interface { type QuarantineController interface { Restore(context.Context, string) (QuarantineEntry, error) PermanentDelete(context.Context, string, string) (QuarantineEntry, error) + Purge(context.Context, QuarantinePurgeScope, string) (QuarantinePurgeResult, error) } type OperationsConfig struct { @@ -122,7 +123,7 @@ func (o *Operations) RestoreQuarantine(ctx context.Context, id string) (Quaranti } func (o *Operations) DeleteQuarantine(ctx context.Context, id, confirmation string) (string, error) { - if confirmation != "DELETE" { + if confirmation != QuarantineConfirmationDelete { return "", validationError("quarantine deletion requires DELETE confirmation") } if o.config.Quarantine == nil { @@ -144,6 +145,35 @@ func (o *Operations) DeleteQuarantine(ctx context.Context, id, confirmation stri }), nil } +func (o *Operations) PurgeQuarantine( + ctx context.Context, + scope QuarantinePurgeScope, + confirmation string, +) (string, error) { + switch scope { + case QuarantinePurgeScopeEligible: + if confirmation != QuarantineConfirmationEligible { + return "", validationError("eligible quarantine purge requires %s confirmation", QuarantineConfirmationEligible) + } + case QuarantinePurgeScopeAll: + if confirmation != QuarantineConfirmationForce { + return "", validationError("forced quarantine purge requires %s confirmation", QuarantineConfirmationForce) + } + default: + return "", validationError("unknown quarantine purge scope %q", scope) + } + if o.config.Quarantine == nil { + return "", errors.New("quarantine provider is unavailable") + } + return o.startTracked(ctx, JobKindQuarantineDelete, func(jobCtx context.Context, _ string) (map[string]any, error) { + result, err := o.config.Quarantine.Purge(jobCtx, scope, confirmation) + if result.Deleted > 0 { + o.invalidateOverview() + } + return valueMap(result), err + }), nil +} + func (o *Operations) invalidateOverview() { if o.config.Overview != nil { o.config.Overview.Invalidate() diff --git a/apps/backend/internal/system/storage/operations_test.go b/apps/backend/internal/system/storage/operations_test.go index 09e5e032a1..92075f4eed 100644 --- a/apps/backend/internal/system/storage/operations_test.go +++ b/apps/backend/internal/system/storage/operations_test.go @@ -319,6 +319,54 @@ func TestDeleteQuarantineInvalidatesOverviewAfterSuccess(t *testing.T) { } } +func TestPurgeQuarantineValidatesScopeAndConfirmation(t *testing.T) { + controller := &recordingQuarantineController{} + operations := NewOperations(OperationsConfig{Quarantine: controller}) + for _, test := range []struct { + scope QuarantinePurgeScope + confirmation string + }{ + {QuarantinePurgeScopeEligible, "DELETE ALL NOW"}, + {QuarantinePurgeScopeAll, "DELETE ELIGIBLE"}, + {"unknown", "DELETE ELIGIBLE"}, + } { + if jobID, err := operations.PurgeQuarantine(context.Background(), test.scope, test.confirmation); !errors.Is(err, ErrValidation) || jobID != "" { + t.Fatalf("PurgeQuarantine(%q, %q) = (%q, %v), want validation error and no job", test.scope, test.confirmation, jobID, err) + } + } + if controller.purgeCalls != 0 { + t.Fatalf("purge calls = %d, want 0", controller.purgeCalls) + } +} + +func TestPurgeQuarantineTracksPartialResultAndInvalidatesOverview(t *testing.T) { + overview := &recordingRefreshOverview{} + controller := &recordingQuarantineController{ + purgeResult: QuarantinePurgeResult{ + Scope: QuarantinePurgeScopeEligible, Considered: 2, Deleted: 1, DeletedBytes: 42, + Protected: 1, ProtectedBytes: 8, Failed: 0, FailedBytes: 0, + }, + purgeErr: errors.New("one entry failed"), + } + tracker := jobs.NewTracker(nil, newOperationsTestLogger(t)) + operations := NewOperations(OperationsConfig{ + Jobs: tracker, Overview: overview, Quarantine: controller, + }) + + jobID, err := operations.PurgeQuarantine(context.Background(), QuarantinePurgeScopeEligible, "DELETE ELIGIBLE") + if err != nil { + t.Fatalf("PurgeQuarantine: %v", err) + } + waitForJobState(t, tracker, jobID, jobs.StateFailed) + job := tracker.Get(jobID) + if job.Result["deleted"] != float64(1) || job.Result["deleted_bytes"] != float64(42) { + t.Fatalf("job result = %#v, want partial purge result", job.Result) + } + if overview.invalidateCalls != 1 { + t.Fatalf("overview invalidations = %d, want 1", overview.invalidateCalls) + } +} + func TestAnalyzeForcesOverviewRefresh(t *testing.T) { connection := newSQLite(t) pool := db.NewPool(connection, connection) @@ -369,6 +417,9 @@ func waitForJobState(t *testing.T, tracker *jobs.Tracker, id string, state jobs. type recordingQuarantineController struct { deleteCalls int + purgeCalls int + purgeResult QuarantinePurgeResult + purgeErr error } type recordingGoCacheAdopter struct{} @@ -449,6 +500,15 @@ func (c *recordingQuarantineController) PermanentDelete( return QuarantineEntry{}, nil } +func (c *recordingQuarantineController) Purge( + context.Context, + QuarantinePurgeScope, + string, +) (QuarantinePurgeResult, error) { + c.purgeCalls++ + return c.purgeResult, c.purgeErr +} + func newOperationsTestLogger(t *testing.T) *logger.Logger { t.Helper() log, err := logger.NewFromZap(zap.NewNop()) diff --git a/apps/backend/internal/system/storage/quarantine_purge.go b/apps/backend/internal/system/storage/quarantine_purge.go new file mode 100644 index 0000000000..bf43a9f9e3 --- /dev/null +++ b/apps/backend/internal/system/storage/quarantine_purge.go @@ -0,0 +1,29 @@ +package storage + +type QuarantinePurgeScope string + +const ( + QuarantinePurgeScopeEligible QuarantinePurgeScope = "eligible" + QuarantinePurgeScopeAll QuarantinePurgeScope = "all" + + QuarantineConfirmationDelete = "DELETE" + QuarantineConfirmationEligible = "DELETE ELIGIBLE" + QuarantineConfirmationForce = "DELETE ALL NOW" +) + +type QuarantinePurgeFailure struct { + ID string `json:"id"` + Error string `json:"error"` +} + +type QuarantinePurgeResult struct { + Scope QuarantinePurgeScope `json:"scope"` + Considered int `json:"considered"` + Deleted int `json:"deleted"` + DeletedBytes int64 `json:"deleted_bytes"` + Protected int `json:"protected"` + ProtectedBytes int64 `json:"protected_bytes"` + Failed int `json:"failed"` + FailedBytes int64 `json:"failed_bytes"` + Failures []QuarantinePurgeFailure `json:"failures,omitempty"` +} diff --git a/apps/backend/internal/system/storage/types.go b/apps/backend/internal/system/storage/types.go index ea654ad84e..33febfd982 100644 --- a/apps/backend/internal/system/storage/types.go +++ b/apps/backend/internal/system/storage/types.go @@ -11,6 +11,7 @@ var ErrValidation = errors.New("storage maintenance settings validation") var ( ErrAdoptionRequired = errors.New("go cache adoption requires the dedicated endpoint") ErrDedicatedDockerConfirmation = errors.New("dedicated Docker acknowledgement requires confirmation") + ErrForceDeleteConfirmation = errors.New("quarantine force deletion requires DELETE ALL NOW confirmation") ErrInvalidPersistedSettings = errors.New("invalid persisted storage maintenance settings") ) diff --git a/apps/backend/internal/system/storage/workspaces/provider.go b/apps/backend/internal/system/storage/workspaces/provider.go index 99a5d7362b..89f44e254e 100644 --- a/apps/backend/internal/system/storage/workspaces/provider.go +++ b/apps/backend/internal/system/storage/workspaces/provider.go @@ -340,7 +340,28 @@ func (p *Provider) PermanentDelete( id string, confirmation string, ) (storage.QuarantineEntry, error) { - if confirmation != "DELETE" { + return p.permanentDelete(ctx, id, confirmation, false) +} + +func (p *Provider) PermanentDeleteForce( + ctx context.Context, + id string, + confirmation string, +) (storage.QuarantineEntry, error) { + return p.permanentDelete(ctx, id, confirmation, true) +} + +func (p *Provider) permanentDelete( + ctx context.Context, + id string, + confirmation string, + force bool, +) (storage.QuarantineEntry, error) { + if force { + if confirmation != storage.QuarantineConfirmationForce { + return storage.QuarantineEntry{}, storage.ErrForceDeleteConfirmation + } + } else if confirmation != storage.QuarantineConfirmationDelete { return storage.QuarantineEntry{}, ErrDeleteConfirmation } if p.config.Store == nil { @@ -353,13 +374,13 @@ func (p *Provider) PermanentDelete( if err := p.validateEntryPaths(entry); err != nil { return entry, err } - if p.config.Now().Before(entry.DeleteAfter) { + if !force && p.config.Now().Before(entry.DeleteAfter) { return entry, fmt.Errorf("%w: quarantine retention deadline has not elapsed", storage.ErrConflict) } if _, err := directorySizeNoFollow(entry.QuarantinePath); err != nil && !errors.Is(err, os.ErrNotExist) { return entry, fmt.Errorf("validate quarantined workspace: %w", err) } - if p.config.Pruner != nil && !p.config.Now().Before(entry.DeleteAfter) { + if p.config.Pruner != nil { if err := p.config.Pruner.PruneQuarantinedWorkspace(ctx, entry); err != nil { _, _ = p.config.Store.TransitionQuarantineEntry(ctx, id, storage.QuarantineStateFailed, err.Error()) return entry, fmt.Errorf("prune stale Git worktree registration: %w", err) @@ -369,7 +390,7 @@ func (p *Provider) PermanentDelete( _, _ = p.config.Store.TransitionQuarantineEntry(ctx, id, storage.QuarantineStateFailed, err.Error()) return entry, fmt.Errorf("delete quarantined workspace: %w", err) } - deleted, err := p.config.Store.TransitionQuarantineEntry(ctx, id, storage.QuarantineStateDeleted, "") + deleted, err := p.config.Store.TransitionQuarantineEntry(context.WithoutCancel(ctx), id, storage.QuarantineStateDeleted, "") if err != nil { return entry, fmt.Errorf("persist workspace deletion: %w", err) } diff --git a/apps/backend/internal/system/storage/workspaces/provider_test.go b/apps/backend/internal/system/storage/workspaces/provider_test.go index 655ba02edd..3225736d2b 100644 --- a/apps/backend/internal/system/storage/workspaces/provider_test.go +++ b/apps/backend/internal/system/storage/workspaces/provider_test.go @@ -574,6 +574,36 @@ func TestPermanentDeleteBeforeRetentionReturnsConflictAndKeepsQuarantine(t *test } } +func TestPermanentDeleteForceBypassesRetentionWithDedicatedConfirmation(t *testing.T) { + provider, root, store := newProviderFixture(t, Inventory{Complete: true}, nil) + pruner := &recordingPruner{} + provider.config.Pruner = pruner + candidate := createOwnedCandidate(t, root, "force-delete-task_abc", OwnershipMarker{ + TaskID: "force-delete-task", TaskDirName: "force-delete-task_abc", LayoutVersion: LayoutVersionSemantic, + }) + old := time.Date(2026, time.July, 1, 0, 0, 0, 0, time.UTC) + if err := os.Chtimes(candidate, old, old); err != nil { + t.Fatalf("Chtimes: %v", err) + } + if _, err := provider.Cleanup(context.Background()); err != nil { + t.Fatalf("Cleanup: %v", err) + } + entry := store.entries["entry-1"] + + if _, err := provider.PermanentDeleteForce(context.Background(), entry.ID, "DELETE"); !errors.Is(err, storage.ErrForceDeleteConfirmation) { + t.Fatalf("wrong force confirmation error = %v, want %v", err, storage.ErrForceDeleteConfirmation) + } + if _, err := provider.PermanentDeleteForce(context.Background(), entry.ID, "DELETE ALL NOW"); err != nil { + t.Fatalf("PermanentDeleteForce: %v", err) + } + if pruner.calls != 1 { + t.Fatalf("pruner calls = %d, want 1", pruner.calls) + } + if _, err := os.Stat(entry.QuarantinePath); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("quarantine path still exists: %v", err) + } +} + func TestReconcileRecreatesMissingRecordFromQuarantineManifest(t *testing.T) { provider, root, store := newProviderFixture(t, Inventory{Complete: true}, nil) candidate := createOwnedCandidate(t, root, "reconcile-task_abc", OwnershipMarker{TaskID: "reconcile-task", TaskDirName: "reconcile-task_abc", LayoutVersion: LayoutVersionSemantic}) diff --git a/apps/backend/internal/system/storage_routes_test.go b/apps/backend/internal/system/storage_routes_test.go index 39027f43ea..946b7a02f5 100644 --- a/apps/backend/internal/system/storage_routes_test.go +++ b/apps/backend/internal/system/storage_routes_test.go @@ -217,6 +217,10 @@ func (f *fakeStorageMutations) DeleteQuarantine(context.Context, string, string) return "delete-job", f.deleteErr } +func (f *fakeStorageMutations) PurgeQuarantine(context.Context, storage.QuarantinePurgeScope, string) (string, error) { + return "purge-job", nil +} + type emptyStorageOverview struct{} func (emptyStorageOverview) Get(context.Context) (storage.OverviewSnapshot, error) { diff --git a/apps/web/components/settings/system/storage/storage-confirmation-dialogs.tsx b/apps/web/components/settings/system/storage/storage-confirmation-dialogs.tsx index 6cccdbd0d1..64e6688375 100644 --- a/apps/web/components/settings/system/storage/storage-confirmation-dialogs.tsx +++ b/apps/web/components/settings/system/storage/storage-confirmation-dialogs.tsx @@ -12,14 +12,14 @@ import { AlertDialogTitle, } from "@kandev/ui/alert-dialog"; import { Input } from "@kandev/ui/input"; -import type { StorageQuarantineEntry } from "@/lib/types/system"; +import type { StorageQuarantineEntry, StorageQuarantinePurgeScope } from "@/lib/types/system"; type ConfirmationDialogProps = { open: boolean; onOpenChange: (open: boolean) => void; title: string; description: string; - phrase: "DEDICATED" | "ADOPT" | "DELETE"; + phrase: "DEDICATED" | "ADOPT" | "DELETE" | "DELETE ELIGIBLE" | "DELETE ALL NOW"; actionLabel: string; actionTestId: string; destructive?: boolean; @@ -113,3 +113,35 @@ export function PermanentDeleteDialog({ /> ); } + +export function QuarantinePurgeDialog({ + scope, + eligibleCount, + protectedCount, + ...props +}: Pick & { + scope: StorageQuarantinePurgeScope; + eligibleCount: number; + protectedCount: number; +}) { + const eligible = scope === "eligible"; + return ( + + ); +} 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 41c897db21..594d6df214 100644 --- a/apps/web/components/settings/system/storage/storage-maintenance-settings.tsx +++ b/apps/web/components/settings/system/storage/storage-maintenance-settings.tsx @@ -277,9 +277,16 @@ export function StorageMaintenanceSettings() { ); 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 d11faafb73..777a0aafa1 100644 --- a/apps/web/components/settings/system/storage/storage-quarantine-card.tsx +++ b/apps/web/components/settings/system/storage/storage-quarantine-card.tsx @@ -1,102 +1,232 @@ "use client"; -import { useState } from "react"; +import { useEffect, useState } from "react"; import { Badge } from "@kandev/ui/badge"; import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@kandev/ui/card"; import { IconRestore, IconTrash } from "@tabler/icons-react"; -import type { StorageQuarantineEntry } from "@/lib/types/system"; +import type { StorageQuarantineEntry, StorageQuarantinePurgeScope } from "@/lib/types/system"; import { JobProgressIndicator } from "../job-progress-indicator"; -import { PermanentDeleteDialog } from "./storage-confirmation-dialogs"; +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 { + formatQuarantineDeadline, + isQuarantineEligible, + quarantineCounts, + quarantineDeleteAfter, +} from "./storage-quarantine"; + +const MAX_TIMER_DELAY_MS = 2_147_483_647; type Props = { entries: StorageQuarantineEntry[]; deleteJobId?: string; + deleteJobActive?: boolean; disabledReason?: string; + schedulingEnabled: boolean; + checkIntervalHours: number; onRestore: (id: string) => Promise; onDelete: (id: string) => Promise; + onClearEligible: () => Promise; + onForceClearAll: () => Promise; }; +function QuarantineEntryRow({ + entry, + now, + disabledReason, + onRestore, + onDelete, +}: { + entry: StorageQuarantineEntry; + now: Date; + disabledReason?: string; + onRestore: (id: string) => Promise; + onDelete: (entry: StorageQuarantineEntry) => void; +}) { + const eligible = isQuarantineEligible(entry, now); + return ( +
+
+
+
+ {entry.resource_type.replace("_", " ")} + + {formatGigabytes(entry.size_bytes)} + +
+

{entry.original_path}

+

+ Trash: {entry.quarantine_path} +

+ {entry.last_error && ( +

{entry.last_error}

+ )} +

+ {eligible ? ( + Eligible now + ) : ( + <> + Protected until{" "} + + + )} +

+
+
+ void onRestore(entry.id)} + data-testid={`storage-quarantine-${entry.id}-restore`} + > + Restore + + onDelete(entry)} + data-testid={`storage-quarantine-${entry.id}-delete`} + > + Delete + +
+
+
+ ); +} + +function QuarantineHeader({ + entries, + counts, + deleteJobId, + bulkDisabledReason, + schedulingEnabled, + checkIntervalHours, + onPurge, +}: { + entries: StorageQuarantineEntry[]; + counts: ReturnType; + deleteJobId?: string; + bulkDisabledReason?: string; + schedulingEnabled: boolean; + checkIntervalHours: number; + onPurge: (scope: StorageQuarantinePurgeScope) => void; +}) { + return ( + +
+ + Quarantine + + Quarantine is Kandev's recoverable holding area. Orphan task workspaces and rotated Go + caches are moved here before deletion. You can restore an item during its retention + period; after the deadline, a later maintenance run may permanently delete it. + + + +
+
+ onPurge("eligible")} + data-testid="storage-quarantine-clear-eligible" + > + Clear eligible + + onPurge("all")} + data-testid="storage-quarantine-force-clear" + > + Force clear all + +
+ + Cleanup moves recoverable data here first instead of deleting it immediately. Restore an + item if you still need it, or delete it permanently when you are certain. {counts.eligible}{" "} + eligible now · {counts.protected} protected + +

+ {schedulingEnabled + ? `Eligible entries are removed by the first successful maintenance run after their deadline (every ${checkIntervalHours} hours when the idle gate allows it).` + : "Automatic quarantine cleanup is off. Run maintenance manually or use a quarantine action when you are ready."} +

+ + ); +} + export function StorageQuarantineCard({ entries, deleteJobId, + deleteJobActive, disabledReason, + schedulingEnabled, + checkIntervalHours, onRestore, onDelete, + onClearEligible, + onForceClearAll, }: Props) { const [deleteEntry, setDeleteEntry] = useState(null); + const [purgeScope, setPurgeScope] = useState(null); + const [now, setNow] = useState(() => new Date()); + useEffect(() => { + const nextDeadline = entries + .map(quarantineDeleteAfter) + .map((deadline) => deadline.getTime()) + .filter((deadline) => deadline > now.getTime()) + .sort((left, right) => left - right)[0]; + if (nextDeadline === undefined) return; + const delay = Math.min(MAX_TIMER_DELAY_MS, Math.max(0, nextDeadline - now.getTime() + 1)); + const timer = setTimeout(() => setNow(new Date()), delay); + return () => clearTimeout(timer); + }, [entries, now]); + const counts = quarantineCounts(entries, now); + const bulkDisabledReason = + disabledReason ?? (deleteJobActive ? "A quarantine cleanup is still running." : undefined); return ( - -
- - Quarantine - - Quarantine is Kandev's recoverable holding area. Orphan task workspaces and rotated Go - caches are moved here before deletion. You can restore an item during its retention - period; after the deadline, a later maintenance run may permanently delete it. - - - -
- - Cleanup moves recoverable data here first instead of deleting it immediately. Restore an - item if you still need it, or delete it permanently when you are certain. - - + {entries.length === 0 && (

No restorable quarantined resources.

)} {entries.map((entry) => ( -
-
-
-
- {entry.resource_type.replace("_", " ")} - - {formatGigabytes(entry.size_bytes)} - -
-

{entry.original_path}

-

- Trash: {entry.quarantine_path} -

- {entry.last_error && ( -

{entry.last_error}

- )} -
-
- void onRestore(entry.id)} - data-testid={`storage-quarantine-${entry.id}-restore`} - > - Restore - - setDeleteEntry(entry)} - data-testid={`storage-quarantine-${entry.id}-delete`} - > - Delete - -
-
-
+ entry={entry} + now={now} + disabledReason={bulkDisabledReason} + onRestore={onRestore} + onDelete={setDeleteEntry} + /> ))} + {purgeScope && ( + { + if (!open) setPurgeScope(null); + }} + onConfirm={() => { + if (purgeScope === "eligible") void onClearEligible(); + else void onForceClearAll(); + setPurgeScope(null); + }} + /> + )} ); } diff --git a/apps/web/components/settings/system/storage/storage-quarantine.test.ts b/apps/web/components/settings/system/storage/storage-quarantine.test.ts new file mode 100644 index 0000000000..8d9df99f21 --- /dev/null +++ b/apps/web/components/settings/system/storage/storage-quarantine.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; +import type { StorageQuarantineEntry } from "@/lib/types/system"; +import { isQuarantineEligible, quarantineCounts } from "./storage-quarantine"; + +const entry = (id: string, deleteAfter: string, bytes: number): StorageQuarantineEntry => ({ + id, + resource_type: "task_workspace", + original_path: `/tmp/${id}`, + quarantine_path: `/tmp/trash/${id}`, + size_bytes: bytes, + state: "quarantined", + quarantined_at: "2026-07-29T00:00:00Z", + delete_after: deleteAfter, + last_error: "", + metadata: {}, +}); + +describe("storage quarantine eligibility", () => { + const now = new Date("2026-07-29T12:00:00Z"); + + it("treats the retention deadline as inclusive", () => { + expect(isQuarantineEligible(entry("at", "2026-07-29T12:00:00Z", 1), now)).toBe(true); + expect(isQuarantineEligible(entry("later", "2026-07-29T12:00:01Z", 1), now)).toBe(false); + }); + + it("aggregates eligible and protected counts and bytes", () => { + expect( + quarantineCounts( + [entry("old", "2026-07-28T12:00:00Z", 10), entry("new", "2026-07-30T12:00:00Z", 20)], + now, + ), + ).toEqual({ eligible: 1, eligibleBytes: 10, protected: 1, protectedBytes: 20 }); + }); +}); diff --git a/apps/web/components/settings/system/storage/storage-quarantine.ts b/apps/web/components/settings/system/storage/storage-quarantine.ts new file mode 100644 index 0000000000..1664584609 --- /dev/null +++ b/apps/web/components/settings/system/storage/storage-quarantine.ts @@ -0,0 +1,32 @@ +import type { StorageQuarantineEntry } from "@/lib/types/system"; + +export function quarantineDeleteAfter(entry: StorageQuarantineEntry): Date { + return new Date(entry.delete_after); +} + +export function isQuarantineEligible(entry: StorageQuarantineEntry, now = new Date()): boolean { + return quarantineDeleteAfter(entry).getTime() <= now.getTime(); +} + +export function quarantineCounts(entries: StorageQuarantineEntry[], now = new Date()) { + return entries.reduce( + (counts, entry) => { + if (isQuarantineEligible(entry, now)) { + counts.eligible += 1; + counts.eligibleBytes += entry.size_bytes; + } else { + counts.protected += 1; + counts.protectedBytes += entry.size_bytes; + } + return counts; + }, + { eligible: 0, eligibleBytes: 0, protected: 0, protectedBytes: 0 }, + ); +} + +export function formatQuarantineDeadline(entry: StorageQuarantineEntry): string { + return quarantineDeleteAfter(entry).toLocaleString(undefined, { + dateStyle: "medium", + timeStyle: "short", + }); +} 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 03adcc6f4d..cf57fe4e90 100644 --- a/apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts +++ b/apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts @@ -1,4 +1,5 @@ import fs from "node:fs"; +import path from "node:path"; import type { Route } from "@playwright/test"; import { test, expect } from "../../fixtures/test-base"; import { seedManagedGoCache } from "../../helpers/storage-maintenance"; @@ -173,4 +174,59 @@ test.describe("Mobile storage maintenance", () => { await testPage.unroute(overviewPattern, holdOverview); } }); + + test("keeps both quarantine cleanup actions reachable on a phone", async ({ + testPage, + prCapture, + }) => { + const entry = { + id: "mobile-protected", + resource_type: "task_workspace", + original_path: "/tmp/mobile-protected", + quarantine_path: "/tmp/trash/mobile-protected", + size_bytes: 1024, + state: "quarantined", + quarantined_at: "2026-07-29T00:00:00Z", + delete_after: new Date(Date.now() + 86_400_000).toISOString(), + last_error: "", + metadata: {}, + }; + await testPage.route("**/api/v1/system/storage/quarantine", async (route) => { + if (route.request().method() === "GET") { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ entries: [entry] }), + }); + return; + } + expect(route.request().method()).toBe("DELETE"); + expect(route.request().postDataJSON()).toEqual({ scope: "all", confirm: "DELETE ALL NOW" }); + await route.fulfill({ + status: 202, + contentType: "application/json", + body: JSON.stringify({ job_id: "mobile-force-purge" }), + }); + }); + await testPage.goto("/settings/system/storage"); + await expect(testPage.getByTestId("storage-quarantine-force-clear")).toBeVisible(); + await testPage.getByTestId("storage-quarantine-card").scrollIntoViewIfNeeded(); + await prCapture.screenshot("quarantine-actions", { + caption: "Mobile quarantine keeps both cleanup actions reachable", + }); + if (process.env.CAPTURE_PR_ASSETS) { + await testPage.getByTestId("storage-quarantine-card").screenshot({ + path: path.resolve(process.cwd(), ".pr-assets/mobile-quarantine-card.png"), + }); + } + await testPage.getByTestId("storage-quarantine-force-clear").tap(); + await testPage + .getByTestId("storage-quarantine-force-clear-confirm-confirmation") + .fill("DELETE ALL NOW"); + await testPage.getByTestId("storage-quarantine-force-clear-confirm").tap(); + await expect(testPage.getByText("Forced quarantine cleanup started")).toBeVisible(); + expect( + await testPage.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth), + ).toBe(true); + }); }); diff --git a/apps/web/e2e/tests/system/storage-maintenance.spec.ts b/apps/web/e2e/tests/system/storage-maintenance.spec.ts index c934c5fdd0..c7fc0ac9e4 100644 --- a/apps/web/e2e/tests/system/storage-maintenance.spec.ts +++ b/apps/web/e2e/tests/system/storage-maintenance.spec.ts @@ -252,4 +252,75 @@ test.describe("System storage maintenance", () => { { timeout: 20_000 }, ); }); + + test("shows quarantine deadlines and clears only eligible entries", async ({ + testPage, + prCapture, + }) => { + const entries = [ + { + id: "eligible-entry", + resource_type: "task_workspace", + original_path: "/tmp/eligible", + quarantine_path: "/tmp/trash/eligible", + size_bytes: 1024, + state: "quarantined", + quarantined_at: "2026-07-20T00:00:00Z", + delete_after: new Date(Date.now() - 60_000).toISOString(), + last_error: "", + metadata: {}, + }, + { + id: "protected-entry", + resource_type: "task_workspace", + original_path: "/tmp/protected", + quarantine_path: "/tmp/trash/protected", + size_bytes: 2048, + state: "quarantined", + quarantined_at: "2026-07-29T00:00:00Z", + delete_after: new Date(Date.now() + 86_400_000).toISOString(), + last_error: "", + metadata: {}, + }, + ]; + await testPage.route("**/api/v1/system/storage/quarantine", async (route) => { + if (route.request().method() === "GET") { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ entries }), + }); + return; + } + expect(route.request().method()).toBe("DELETE"); + expect(route.request().postDataJSON()).toEqual({ + scope: "eligible", + confirm: "DELETE ELIGIBLE", + }); + await route.fulfill({ + status: 202, + contentType: "application/json", + body: JSON.stringify({ job_id: "eligible-purge" }), + }); + }); + await testPage.goto("/settings/system/storage"); + await expect( + testPage.getByTestId("storage-quarantine-eligible-entry").getByText("Eligible now"), + ).toBeVisible(); + await expect(testPage.getByText(/Protected until/)).toBeVisible(); + await expect(testPage.getByTestId("storage-quarantine-protected-entry-delete")).toBeDisabled(); + await testPage.getByTestId("storage-quarantine-card").scrollIntoViewIfNeeded(); + await prCapture.screenshot("quarantine-deadlines", { + caption: "Desktop quarantine shows eligibility and protected retention deadlines", + }); + await testPage.getByTestId("storage-quarantine-clear-eligible").click(); + await testPage + .getByTestId("storage-quarantine-clear-eligible-confirm-confirmation") + .fill("DELETE ELIGIBLE"); + await testPage.getByTestId("storage-quarantine-clear-eligible-confirm").click(); + await expect(testPage.getByText("Eligible quarantine cleanup started")).toBeVisible(); + await prCapture.screenshot("quarantine-clear-eligible", { + caption: "Desktop typed confirmation starts eligible-only quarantine cleanup", + }); + }); }); 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 40bfc4f9e0..df51071738 100644 --- a/apps/web/hooks/domains/system/use-storage-maintenance.test.tsx +++ b/apps/web/hooks/domains/system/use-storage-maintenance.test.tsx @@ -9,6 +9,7 @@ const mocks = vi.hoisted(() => ({ adopt: vi.fn(), analyze: vi.fn(), deleteEntry: vi.fn(), + purge: vi.fn(), fetchJob: vi.fn(), fetchOverview: vi.fn(), fetchQuarantine: vi.fn(), @@ -27,6 +28,7 @@ vi.mock("@/lib/api/domains/system-api", () => ({ adoptStorageGoCache: mocks.adopt, analyzeStorage: mocks.analyze, deleteStorageQuarantine: mocks.deleteEntry, + purgeStorageQuarantine: mocks.purge, fetchSystemJob: mocks.fetchJob, fetchStorageOverview: mocks.fetchOverview, fetchStorageQuarantine: mocks.fetchQuarantine, @@ -116,6 +118,7 @@ beforeEach(() => { mocks.save.mockResolvedValue({ settings }); // Keep cleanup jobs deterministic for controller action tests. mocks.run.mockResolvedValue({ job_id: cleanupJobId }); + mocks.purge.mockResolvedValue({ job_id: "purge-job" }); }); describe("useStorageMaintenance", () => { @@ -140,6 +143,19 @@ describe("useStorageMaintenance", () => { }); }); + it("starts eligible and forced quarantine bulk jobs", async () => { + const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); + await waitFor(() => expect(result.current.overview).toEqual(overview)); + + await act(async () => { + await result.current.clearEligible(); + await result.current.forceClearAll(); + }); + + expect(mocks.purge).toHaveBeenNthCalledWith(1, "eligible"); + expect(mocks.purge).toHaveBeenNthCalledWith(2, "all"); + }); + it("rejects failed saves so the settings coordinator can keep the draft dirty", async () => { mocks.save.mockRejectedValueOnce(new Error("save unavailable")); const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); diff --git a/apps/web/hooks/domains/system/use-storage-maintenance.ts b/apps/web/hooks/domains/system/use-storage-maintenance.ts index 33b0c0efa4..16116e66cf 100644 --- a/apps/web/hooks/domains/system/use-storage-maintenance.ts +++ b/apps/web/hooks/domains/system/use-storage-maintenance.ts @@ -19,6 +19,7 @@ import { fetchStorageOverview, fetchStorageQuarantine, fetchStorageRuns, + purgeStorageQuarantine, restoreStorageQuarantine, runStorageMaintenance, saveStorageSettings, @@ -27,6 +28,7 @@ import type { StorageBusyResource, StorageBusyResponse, StorageMaintenanceSettings, + StorageQuarantinePurgeScope, SystemJob, } from "@/lib/types/system"; import { useSystemJob } from "./use-system-jobs"; @@ -39,6 +41,7 @@ export type StoragePendingAction = | "adopt" | "restore" | "delete" + | "purge" | null; export interface StorageBusyState { @@ -189,6 +192,31 @@ function useStorageDeleteAction( ); } +function useStorageBulkDeleteAction( + perform: ReturnType["perform"], + toast: ReturnType["toast"], + clearBusy: () => void, + setDeleteJobId: Dispatch>, +) { + return useCallback( + async (scope: StorageQuarantinePurgeScope) => { + clearBusy(); + return perform("purge", async () => { + const accepted = await purgeStorageQuarantine(scope); + setDeleteJobId(accepted.job_id); + toast({ + title: + scope === "eligible" + ? "Eligible quarantine cleanup started" + : "Forced quarantine cleanup started", + variant: "success", + }); + }); + }, + [clearBusy, perform, setDeleteJobId, toast], + ); +} + function useStorageActions(reload: Reload) { const { toast } = useToast(); const { pendingAction, error, setError, finishLoading, perform } = useStorageActionRunner(); @@ -265,6 +293,7 @@ function useStorageActions(reload: Reload) { [clearBusy, perform, reload, toast], ); const permanentlyDelete = useStorageDeleteAction(perform, toast, clearBusy, setDeleteJobId); + const purge = useStorageBulkDeleteAction(perform, toast, clearBusy, setDeleteJobId); return { pendingAction, @@ -279,6 +308,8 @@ function useStorageActions(reload: Reload) { adopt, restore, permanentlyDelete, + clearEligible: useCallback(() => purge("eligible"), [purge]), + forceClearAll: useCallback(() => purge("all"), [purge]), analysisJob, cleanupJob, deleteJob, diff --git a/apps/web/lib/api/domains/system-api.test.ts b/apps/web/lib/api/domains/system-api.test.ts index 8300ff1aca..928b871733 100644 --- a/apps/web/lib/api/domains/system-api.test.ts +++ b/apps/web/lib/api/domains/system-api.test.ts @@ -33,6 +33,7 @@ import { fetchStorageOverview, fetchStorageQuarantine, fetchStorageRuns, + purgeStorageQuarantine, restoreStorageQuarantine, runStorageMaintenance, saveStorageSettings, @@ -380,6 +381,24 @@ describe("storage maintenance", () => { expect(response.job_id).toBe("delete-job"); }); + it("uses typed confirmations for bulk quarantine purge", async () => { + fetchSpy.mockResolvedValueOnce(jsonResponse({ job_id: "eligible-job" })); + await purgeStorageQuarantine("eligible"); + expect(lastCall().url).toBe(`${BASE}/storage/quarantine`); + expect(method()).toBe("DELETE"); + expect(JSON.parse(String(lastCall().init?.body))).toEqual({ + scope: "eligible", + confirm: "DELETE ELIGIBLE", + }); + + fetchSpy.mockResolvedValueOnce(jsonResponse({ job_id: "all-job" })); + await purgeStorageQuarantine("all"); + expect(JSON.parse(String(lastCall().init?.body))).toEqual({ + scope: "all", + confirm: "DELETE ALL NOW", + }); + }); + it("starts analysis, selected cleanup, and restore operations", async () => { fetchSpy .mockResolvedValueOnce(jsonResponse({ job_id: "analysis" })) diff --git a/apps/web/lib/api/domains/system-api.ts b/apps/web/lib/api/domains/system-api.ts index a80ca6b01e..75552221de 100644 --- a/apps/web/lib/api/domains/system-api.ts +++ b/apps/web/lib/api/domains/system-api.ts @@ -16,6 +16,7 @@ import type { StorageMaintenanceSettings, StorageOverviewResponse, StorageQuarantineEntry, + StorageQuarantinePurgeScope, StorageSettingsResponse, } from "@/lib/types/system"; @@ -311,3 +312,20 @@ export function deleteStorageQuarantine( }, ); } + +export function purgeStorageQuarantine( + scope: StorageQuarantinePurgeScope, + options?: ApiRequestOptions, +): Promise { + return fetchJson(`${SYSTEM_BASE}/storage/quarantine`, { + ...options, + init: { + ...(options?.init ?? {}), + method: "DELETE", + body: JSON.stringify({ + scope, + confirm: scope === "eligible" ? "DELETE ELIGIBLE" : "DELETE ALL NOW", + }), + }, + }); +} diff --git a/apps/web/lib/types/system.ts b/apps/web/lib/types/system.ts index 57ab16dc15..b8296db9c0 100644 --- a/apps/web/lib/types/system.ts +++ b/apps/web/lib/types/system.ts @@ -298,6 +298,25 @@ export interface StorageQuarantineEntry { metadata: Record; } +export type StorageQuarantinePurgeScope = "eligible" | "all"; + +export interface StorageQuarantinePurgeFailure { + id: string; + error: string; +} + +export interface StorageQuarantinePurgeResult { + scope: StorageQuarantinePurgeScope; + considered: number; + deleted: number; + deleted_bytes: number; + protected: number; + protected_bytes: number; + failed: number; + failed_bytes: number; + failures?: StorageQuarantinePurgeFailure[]; +} + export interface StorageOverviewResponse { settings: StorageMaintenanceSettings; capabilities: StorageCapabilities; diff --git a/docs/decisions/2026-07-29-quarantine-retention-override.md b/docs/decisions/2026-07-29-quarantine-retention-override.md new file mode 100644 index 0000000000..e5cc47358b --- /dev/null +++ b/docs/decisions/2026-07-29-quarantine-retention-override.md @@ -0,0 +1,58 @@ +# ADR-2026-07-29-quarantine-retention-override: Make Quarantine Retention Overridable but Visible + +**Status:** accepted +**Date:** 2026-07-29 +**Area:** backend, frontend + +## Context + +Storage maintenance atomically moves owned task workspaces and Go caches into quarantine, but the +initial implementation enforced each entry's retention deadline without showing that deadline in +the Storage page. It also exposed an enabled delete action before the deadline and described +automatic deletion that no maintenance provider performed. Operators need predictable automatic +reclamation and a deliberate way to recover disk space immediately when they accept losing the +restore window. + +## Decision + +The persisted `storage_quarantine_entries.delete_after` timestamp remains the normal permanent +deletion boundary. Individual deletion, **Clear eligible**, and scheduled or full manual storage +maintenance delete only entries at or after that timestamp. + +The Storage page shows the deletion-eligibility timestamp and whether automatic cleanup is enabled. +Scheduled maintenance and full manual **Run now** include a typed quarantine provider that +permanently deletes eligible entries. When scheduling is disabled, no separate quarantine sweeper +runs; eligible entries remain until a full manual run or an explicit quarantine action. + +Install operators also receive a separate **Force clear all** action. It requires the stronger +server-validated confirmation phrase `DELETE ALL NOW` and may permanently delete active quarantine +entries before their retention deadlines. This override bypasses only the retention clock. Resource +ownership, path containment, symlink, state-transition, and Git worktree-pruning safety checks +remain mandatory. + +Bulk deletion is best-effort across entries. Its durable System job reports deleted, protected, and +failed counts and bytes; any entry that fails remains visible and retryable. + +## Consequences + +- Operators can see the earliest safe deletion time and understand whether reclamation is automatic + or requires a manual action. +- Normal maintenance restores the intended bounded quarantine lifecycle instead of retaining + expired entries indefinitely. +- A deliberate install-wide override can reclaim all quarantine space immediately, at the cost of + losing protected restore windows. +- Disabling scheduled maintenance also disables automatic quarantine deletion, so the UI and + public documentation must make that dependency explicit. +- The force action needs stronger confirmation and regression coverage proving it cannot bypass + ownership or path-safety validation. + +## Alternatives Considered + +- **Never allow retention bypass.** Rejected because an operator facing disk exhaustion needs an + explicit way to trade recovery for immediate reclamation. +- **Make every delete action bypass retention after confirmation.** Rejected because it makes the + normal row action undermine the configured safety policy. +- **Run a dedicated quarantine sweeper while scheduled maintenance is disabled.** Rejected because + disabling destructive scheduling should stop unattended permanent deletion. +- **Keep quarantine deletion manual-only.** Rejected because expired data would continue to + accumulate despite an enabled storage-maintenance schedule. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index e2b4690798..1b1c5f1bbb 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -86,3 +86,4 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-07-28-visible-wip-overflow-queues | [Separate Visible Queueing From WIP Admission](2026-07-28-visible-wip-overflow-queues.md) | proposed | backend, frontend, protocol, workflow | 2026-07-28 | | 2026-07-28-coarse-running-busy-signal | [Restore Coarse Running Prompt Admission](2026-07-28-coarse-running-busy-signal.md) | accepted | backend, frontend, protocol | 2026-07-28 | | 2026-07-29-agent-stall-user-controlled-recovery | [Keep Agent Stall Recovery User Controlled](2026-07-29-agent-stall-user-controlled-recovery.md) | accepted | backend, frontend, protocol | 2026-07-29 | +| 2026-07-29-quarantine-retention-override | [Make Quarantine Retention Overridable but Visible](2026-07-29-quarantine-retention-override.md) | accepted | backend, frontend | 2026-07-29 | diff --git a/docs/plans/quarantine-lifecycle/plan.md b/docs/plans/quarantine-lifecycle/plan.md new file mode 100644 index 0000000000..70661d5b95 --- /dev/null +++ b/docs/plans/quarantine-lifecycle/plan.md @@ -0,0 +1,230 @@ +--- +spec: docs/specs/system-page/storage-maintenance.md +created: 2026-07-29 +status: complete +--- + +# Implementation Plan: Quarantine Lifecycle + +## Overview + +Complete the quarantine lifecycle by adding a typed best-effort purge operation, running eligible +purges during full maintenance, and exposing separate retention-respecting and retention-bypassing +bulk actions. Then wire the API through the existing System job flow, make eligibility and +automatic-cleanup timing visible on desktop and mobile, update operator documentation, and prove the +flows with focused backend, frontend, and Playwright coverage. + +No schema migration is required: `storage_quarantine_entries.delete_after`, state, size, and error +fields already contain the durable data needed for eligibility and result accounting. + +--- + +## Backend + +### Typed purge result and deletion modes + +- Add `apps/backend/internal/system/storage/quarantine_purge.go` with + `QuarantinePurgeScope` (`eligible`, `all`), `QuarantinePurgeFailure`, and + `QuarantinePurgeResult`. +- Extend the backend quarantine controller contract with a bulk purge method that returns considered, + deleted, protected, and failed counts/bytes plus per-entry failures. +- Iterate all active `quarantined|failed` entries deterministically. Eligible-only purge skips future + `delete_after` entries; force purge attempts every active entry. +- Continue after entry failures and return the complete result together with an aggregate error so + successful deletions remain committed while the System job or maintenance run is visibly failed. + +### Resource deletion safety + +- Refactor `apps/backend/internal/system/storage/workspaces/provider.go` so normal and forced + permanent deletion share path, state, pruning, and filesystem safety validation. The force path + skips only the retention comparison. +- Refactor Go-cache deletion in + `apps/backend/internal/backendapp/storage_maintenance.go` through the same retention-mode boundary. + Preserve ownership metadata validation, managed-trash containment, replacement-path ambiguity + checks, and state transitions for both modes. +- Require `DELETE ALL NOW` at the external force boundary; never use the generic maintenance + `force` flag as a retention override. + +### Maintenance provider composition + +- Create the `workspaceQuarantineController` before assembling cleanup providers in + `apps/backend/internal/backendapp/storage_maintenance.go`. +- Add a typed `quarantine` cleanup provider to `storageCleanupProviders`. Its ordinary `Cleanup` + purges only eligible entries and returns `QuarantinePurgeResult` in the maintenance-run result. +- Include the provider in scheduled and full manual runs. Existing explicit selections such as + `resources: ["go_cache"]` continue to select only the named provider and do not purge unrelated + quarantine entries. +- Keep the provider inside the existing maintenance activity lease so task admission can preempt a + long purge. Provider errors remain isolated by `Runner.runProviders`. + +### Bulk HTTP operation + +- Extend `apps/backend/internal/system/storage/operations.go` with + `PurgeQuarantine(ctx, scope, confirmation)`, validate the scope/phrase pair before starting work, + and run it as `storage-quarantine-delete`. +- Register `DELETE /api/v1/system/storage/quarantine` in + `apps/backend/internal/system/storage/handler.go` with body + `{scope, confirm}`. Preserve `DELETE /quarantine/:id` and its retention `409`. +- Return `400` for unknown scopes or mismatched confirmation phrases. A started bulk purge returns + `202 {job_id}` and publishes its structured result through the existing System job tracker. +- Invalidate the cached storage overview after any successful deletion, including a partially + successful job. + +--- + +## Frontend + +### Types, API client, and domain controller + +- Add the bulk scope/result types to `apps/web/lib/types/system.ts`. +- Add `purgeStorageQuarantine(scope)` to + `apps/web/lib/api/domains/system-api.ts`, mapping `eligible` to `DELETE ELIGIBLE` and `all` to + `DELETE ALL NOW`. +- Extend `apps/web/hooks/domains/system/use-storage-maintenance.ts` with one shared bulk-delete job + path, terminal refresh, success/error feedback, and actions for `clearEligible` and + `forceClearAll`. +- Keep individual deletion wired to `DELETE`; the UI will only enable that action for entries whose + deadline has elapsed. + +### Quarantine card + +- Update `apps/web/components/settings/system/storage/storage-quarantine-card.tsx` to derive and + display `eligible now` versus `protected until ` from each persisted + `delete_after` value using semantic `