From 7250cdbf61df543aaf5e0d234f4f28635d53c3f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Tue, 28 Jul 2026 09:23:42 +0100 Subject: [PATCH] feat: explain busy storage cleanup and allow override (#1993) --- .../skills/spec-driven-development/SKILL.md | 6 + AGENTS.md | 3 + .../agent/runtime/activity/coordinator.go | 74 +++++++- .../runtime/activity/coordinator_test.go | 60 ++++++ .../backendapp/storage_maintenance.go | 5 +- .../backendapp/storage_maintenance_test.go | 3 + .../internal/system/storage/handler.go | 10 +- .../internal/system/storage/operations.go | 22 ++- .../system/storage/operations_test.go | 73 +++++++- .../backend/internal/system/storage/runner.go | 36 +++- .../internal/system/storage_routes_test.go | 29 ++- .../storage-maintenance-settings.test.tsx | 32 ++++ .../storage/storage-maintenance-settings.tsx | 69 ++++++- .../system/mobile-storage-maintenance.spec.ts | 47 +++++ .../tests/system/storage-maintenance.spec.ts | 29 ++- .../system/use-storage-maintenance.test.tsx | 81 ++++++++ .../domains/system/use-storage-maintenance.ts | 177 +++++++++++++----- apps/web/lib/api/domains/system-api.test.ts | 10 +- apps/web/lib/api/domains/system-api.ts | 6 +- apps/web/lib/types/system.ts | 11 ++ docs/plans/storage-busy-override/plan.md | 124 ++++++++++++ .../task-01-backend-activity-api.md | 58 ++++++ .../task-02-frontend-busy-override.md | 60 ++++++ .../task-03-e2e-busy-override.md | 53 ++++++ docs/public/operations.md | 4 +- docs/specs/system-page/storage-maintenance.md | 36 +++- 26 files changed, 1031 insertions(+), 87 deletions(-) create mode 100644 docs/plans/storage-busy-override/plan.md create mode 100644 docs/plans/storage-busy-override/task-01-backend-activity-api.md create mode 100644 docs/plans/storage-busy-override/task-02-frontend-busy-override.md create mode 100644 docs/plans/storage-busy-override/task-03-e2e-busy-override.md diff --git a/.agents/skills/spec-driven-development/SKILL.md b/.agents/skills/spec-driven-development/SKILL.md index 02de49f8a3..0f52fd6496 100644 --- a/.agents/skills/spec-driven-development/SKILL.md +++ b/.agents/skills/spec-driven-development/SKILL.md @@ -19,6 +19,12 @@ Intent -> Spec -> Plan + task files -> User model switch -> Sequential TDD imple Do not replace the spec, plan, or task files with chat-only notes. Do not skip from intent to code unless the user explicitly opts out of this workflow. +When a user explicitly asks to proceed with feature planning, complete the whole +strong-model design package — spec, plan, and task files — before pausing for +review or a model switch. Pause after a standalone spec only when the user asks +to review that spec before planning, or when a material open question blocks a +safe plan. + ## Phase 0: Track The Work Keep a visible task list and mark each phase as it completes: diff --git a/AGENTS.md b/AGENTS.md index 39f83d9972..7b30b2f916 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -104,6 +104,9 @@ The read-only `pr-poller` is the sole repository-defined exception: use it only after the user explicitly asks to wait for or monitor PR updates. Use the user's strong model for specs, plans, task files, and high-risk design. +When the user asks to proceed with feature planning, produce the spec, plan, +and task files as one design package; pause after a spec only when the user +explicitly requests spec review or a material open question prevents planning. At the completed-plan checkpoint, ask the user to manually switch the main session to a lower-cost implementation model. Detailed feature, fix, validation, and delegation routing lives in the relevant skills, especially diff --git a/apps/backend/internal/agent/runtime/activity/coordinator.go b/apps/backend/internal/agent/runtime/activity/coordinator.go index 4cc4267066..6ed9be9abf 100644 --- a/apps/backend/internal/agent/runtime/activity/coordinator.go +++ b/apps/backend/internal/agent/runtime/activity/coordinator.go @@ -28,6 +28,14 @@ const ( KindMaintenanceRunning Kind = "maintenance_running" ) +// BusyResource is a privacy-safe description of host activity that can block +// destructive storage maintenance. It intentionally contains a category, not +// task/session identity or command content. +type BusyResource struct { + Kind Kind `json:"kind"` + Label string `json:"label"` +} + type Options struct { Now func() time.Time } @@ -138,13 +146,30 @@ func (c *Coordinator) AcquireTask(ctx context.Context, kind Kind) (*TaskLease, e func (c *Coordinator) TryAcquireMaintenance( ctx context.Context, quietPeriod time.Duration, +) (*MaintenanceLease, []Kind, error) { + return c.tryAcquireMaintenance(ctx, quietPeriod, false) +} + +// TryAcquireMaintenanceForce admits manual maintenance alongside task work +// that is already active. It still rejects another maintenance run, and the +// returned lease remains preemptible by task work admitted afterwards. +func (c *Coordinator) TryAcquireMaintenanceForce( + ctx context.Context, +) (*MaintenanceLease, []Kind, error) { + return c.tryAcquireMaintenance(ctx, 0, true) +} + +func (c *Coordinator) tryAcquireMaintenance( + ctx context.Context, + quietPeriod time.Duration, + ignoreActive bool, ) (*MaintenanceLease, []Kind, error) { c.mu.Lock() defer c.mu.Unlock() if c.maintenance != nil { return nil, []Kind{KindMaintenanceRunning}, ErrBusy } - if busy := c.busyKindsLocked(); len(busy) > 0 { + if busy := c.busyKindsLocked(); !ignoreActive && len(busy) > 0 { return nil, busy, ErrBusy } if quietPeriod > 0 && c.now().UTC().Sub(c.lastActivity) < quietPeriod { @@ -162,6 +187,14 @@ func (c *Coordinator) BusyKinds() []Kind { return c.busyKindsLocked() } +// BusyResources returns the current activity categories with labels suitable +// for a user-facing maintenance warning. +func (c *Coordinator) BusyResources() []BusyResource { + c.mu.Lock() + defer c.mu.Unlock() + return BusyResourcesForKinds(c.busyKindsLocked()) +} + func (c *Coordinator) releaseTask(kind Kind) { c.mu.Lock() defer c.mu.Unlock() @@ -198,3 +231,42 @@ func (c *Coordinator) busyKindsLocked() []Kind { sort.Slice(busy, func(i, j int) bool { return busy[i] < busy[j] }) return busy } + +// BusyResourcesForKinds converts stable activity kinds into user-facing +// descriptions without exposing task/session identity. +func BusyResourcesForKinds(kinds []Kind) []BusyResource { + resources := make([]BusyResource, 0, len(kinds)) + for _, kind := range kinds { + resources = append(resources, BusyResource{Kind: kind, Label: busyLabel(kind)}) + } + return resources +} + +func busyLabel(kind Kind) string { + switch kind { + case KindExecutionStarting: + return "An agent execution is starting" + case KindExecutionPreparing: + return "An agent execution is preparing" + case KindExecutionRunning: + return "An agent execution is running" + case KindExecutionStopping: + return "An agent execution is stopping" + case KindShellCommand: + return "A shell command is running" + case KindTestCommand: + return "A test command is running" + case KindSetupScript: + return "A setup script is running" + case KindCleanupScript: + return "A cleanup script is running" + case KindDockerImageBuild: + return "A Docker image build is running" + case KindMaintenanceRunning: + return "Another storage maintenance run is running" + case KindQuietPeriod: + return "Kandev is in its configured idle period" + default: + return "Kandev host activity is running" + } +} diff --git a/apps/backend/internal/agent/runtime/activity/coordinator_test.go b/apps/backend/internal/agent/runtime/activity/coordinator_test.go index 2064513fd7..9066e6c127 100644 --- a/apps/backend/internal/agent/runtime/activity/coordinator_test.go +++ b/apps/backend/internal/agent/runtime/activity/coordinator_test.go @@ -96,3 +96,63 @@ func TestMaintenanceAlreadyRunningReportsMaintenanceKind(t *testing.T) { t.Fatalf("busy kinds = %v, want [%s]", busy, want) } } + +func TestForcedMaintenanceIgnoresCurrentTaskButRemainsPreemptible(t *testing.T) { + coordinator := NewCoordinator(Options{}) + activeTask, err := coordinator.AcquireTask(context.Background(), KindTestCommand) + if err != nil { + t.Fatalf("AcquireTask: %v", err) + } + defer activeTask.Release() + + maintenance, busy, err := coordinator.TryAcquireMaintenanceForce(context.Background()) + if err != nil { + t.Fatalf("TryAcquireMaintenanceForce: %v", err) + } + if len(busy) != 0 { + t.Fatalf("forced maintenance busy kinds = %v, want empty", busy) + } + + admitted := make(chan *TaskLease, 1) + go func() { + lease, acquireErr := coordinator.AcquireTask(context.Background(), KindShellCommand) + if acquireErr == nil { + admitted <- lease + } + }() + + select { + case <-maintenance.Context().Done(): + case <-time.After(time.Second): + t.Fatal("new task admission did not preempt forced maintenance") + } + select { + case <-admitted: + t.Fatal("new task admitted before forced maintenance drained") + default: + } + maintenance.Release() + select { + case lease := <-admitted: + lease.Release() + case <-time.After(time.Second): + t.Fatal("new task was not admitted after forced maintenance drained") + } +} + +func TestBusyResourcesExposeStableLabels(t *testing.T) { + coordinator := NewCoordinator(Options{}) + lease, err := coordinator.AcquireTask(context.Background(), KindDockerImageBuild) + if err != nil { + t.Fatalf("AcquireTask: %v", err) + } + defer lease.Release() + + resources := coordinator.BusyResources() + if len(resources) != 1 { + t.Fatalf("BusyResources = %v, want one resource", resources) + } + if resources[0].Kind != KindDockerImageBuild || resources[0].Label == "" { + t.Fatalf("BusyResources = %#v, want docker build resource with label", resources) + } +} diff --git a/apps/backend/internal/backendapp/storage_maintenance.go b/apps/backend/internal/backendapp/storage_maintenance.go index 47cf99becf..d2733978c6 100644 --- a/apps/backend/internal/backendapp/storage_maintenance.go +++ b/apps/backend/internal/backendapp/storage_maintenance.go @@ -469,7 +469,10 @@ func (c *workspaceQuarantineController) acquireGoCacheMaintenance( } lease, busy, err := c.activity.TryAcquireMaintenance(ctx, 0) if errors.Is(err, activity.ErrBusy) { - return nil, &storagepkg.BusyError{Resources: busy} + return nil, &storagepkg.BusyError{ + Resources: activity.BusyResourcesForKinds(busy), + ForceAvailable: false, + } } return lease, err } diff --git a/apps/backend/internal/backendapp/storage_maintenance_test.go b/apps/backend/internal/backendapp/storage_maintenance_test.go index 8a7740ed15..32bcc1b9bb 100644 --- a/apps/backend/internal/backendapp/storage_maintenance_test.go +++ b/apps/backend/internal/backendapp/storage_maintenance_test.go @@ -297,6 +297,9 @@ func TestQuarantineControllerRetainsGoCacheWhileTaskActivityIsRunning(t *testing if !errors.As(err, &busy) { t.Fatalf("Restore error = %v, want BusyError", err) } + if busy.ForceAvailable || len(busy.Resources) != 1 || busy.Resources[0].Label == "" { + t.Fatalf("busy response = %#v, want labeled non-overridable resource", busy) + } if _, err := os.Stat(artifact); err != nil { t.Fatalf("quarantined cache changed while task active: %v", err) } diff --git a/apps/backend/internal/system/storage/handler.go b/apps/backend/internal/system/storage/handler.go index 86ad91f0d1..5fd828dc30 100644 --- a/apps/backend/internal/system/storage/handler.go +++ b/apps/backend/internal/system/storage/handler.go @@ -25,7 +25,7 @@ type QuarantineLister interface { type Mutations interface { AdoptGoCache(context.Context, string, string) (StorageMaintenanceSettings, Capabilities, error) Analyze(context.Context) (string, error) - RunNow(context.Context, []string) (string, error) + RunNow(context.Context, []string, bool) (string, error) RestoreQuarantine(context.Context, string) (QuarantineEntry, error) DeleteQuarantine(context.Context, string, string) (string, error) } @@ -129,6 +129,7 @@ func (h *Handler) adoptGoCache(c *gin.Context) { type runRequest struct { Resources []string `json:"resources"` + Force bool `json:"force"` } func (h *Handler) analyze(c *gin.Context) { @@ -144,10 +145,13 @@ func (h *Handler) runNow(c *gin.Context) { return } } - id, err := h.config.Mutations.RunNow(c.Request.Context(), request.Resources) + id, err := h.config.Mutations.RunNow(c.Request.Context(), request.Resources, request.Force) var busy *BusyError if errors.As(err, &busy) { - c.JSON(http.StatusConflict, gin.H{"error": busy.Error(), "busy_resources": busy.Resources}) + c.JSON(http.StatusConflict, gin.H{ + "error": busy.Error(), "busy_resources": busy.Resources, + "force_available": busy.ForceAvailable, + }) return } writeAcceptedJob(c, id, err) diff --git a/apps/backend/internal/system/storage/operations.go b/apps/backend/internal/system/storage/operations.go index 78dd82b00e..2af35dc17c 100644 --- a/apps/backend/internal/system/storage/operations.go +++ b/apps/backend/internal/system/storage/operations.go @@ -87,7 +87,7 @@ type OverviewRefresher interface { Refresh(context.Context) (OverviewSnapshot, error) } -func (o *Operations) RunNow(ctx context.Context, resources []string) (string, error) { +func (o *Operations) RunNow(ctx context.Context, resources []string, force bool) (string, error) { settings, err := o.config.Settings.GetSettings(ctx) if err != nil { return "", err @@ -96,13 +96,13 @@ func (o *Operations) RunNow(ctx context.Context, resources []string) (string, er if err != nil { return "", err } - if err := o.preflight(ctx); err != nil { + if err := o.preflight(ctx, force); err != nil { return "", err } return o.startTracked(ctx, JobKindCleanup, func(jobCtx context.Context, id string) (map[string]any, error) { runner := NewRunner(RunnerConfig{ Activity: o.config.Activity, Store: o.config.Store, Providers: providers, - NewID: func() string { return id }, Overview: o.config.Overview, + Force: force, NewID: func() string { return id }, Overview: o.config.Overview, }) run, runErr := runner.Run(jobCtx, RunTriggerManual, settings) return runResultMap(run), runErr @@ -150,10 +150,20 @@ func (o *Operations) invalidateOverview() { } } -func (o *Operations) preflight(ctx context.Context) error { - lease, busy, err := o.config.Activity.TryAcquireMaintenance(ctx, 0) +func (o *Operations) preflight(ctx context.Context, force bool) error { + var lease *activity.MaintenanceLease + var busy []activity.Kind + var err error + if force { + lease, busy, err = o.config.Activity.TryAcquireMaintenanceForce(ctx) + } else { + lease, busy, err = o.config.Activity.TryAcquireMaintenance(ctx, 0) + } if errors.Is(err, activity.ErrBusy) { - return &BusyError{Resources: busy} + return &BusyError{ + Resources: activity.BusyResourcesForKinds(busy), + ForceAvailable: forceAvailable(busy), + } } if err != nil { return err diff --git a/apps/backend/internal/system/storage/operations_test.go b/apps/backend/internal/system/storage/operations_test.go index 753d57ef12..09e5e032a1 100644 --- a/apps/backend/internal/system/storage/operations_test.go +++ b/apps/backend/internal/system/storage/operations_test.go @@ -29,7 +29,7 @@ func TestRunNowIgnoresQuietPeriodWithoutActiveTaskWork(t *testing.T) { Jobs: tracker, Activity: coordinator, Providers: []CleanupProvider{provider}, }) - jobID, err := operations.RunNow(context.Background(), nil) + jobID, err := operations.RunNow(context.Background(), nil, false) if err != nil { t.Fatalf("RunNow: %v", err) } @@ -73,7 +73,7 @@ func TestRunNowInvalidatesOverviewAfterSuccess(t *testing.T) { Overview: overview, }) - jobID, err := operations.RunNow(context.Background(), nil) + jobID, err := operations.RunNow(context.Background(), nil, false) if err != nil { t.Fatalf("RunNow: %v", err) } @@ -101,7 +101,7 @@ func TestRunNowRejectsCurrentTaskActivity(t *testing.T) { Jobs: jobs.NewTracker(nil, newOperationsTestLogger(t)), Activity: coordinator, }) - jobID, err := operations.RunNow(context.Background(), nil) + jobID, err := operations.RunNow(context.Background(), nil, false) var busyErr *BusyError if !errors.As(err, &busyErr) { t.Fatalf("RunNow error = %v, want BusyError", err) @@ -109,6 +109,71 @@ func TestRunNowRejectsCurrentTaskActivity(t *testing.T) { if jobID != "" { t.Fatalf("busy RunNow job ID = %q, want empty", jobID) } + if !busyErr.ForceAvailable || len(busyErr.Resources) != 1 || busyErr.Resources[0].Label == "" { + t.Fatalf("busy response = %#v, want labeled force-available resource", busyErr) + } +} + +func TestRunNowForceRunsBesideCurrentTaskActivity(t *testing.T) { + connection := newSQLite(t) + pool := db.NewPool(connection, connection) + rawSettings, err := systemsettings.NewStore(pool) + if err != nil { + t.Fatalf("new settings store: %v", err) + } + coordinator := activity.NewCoordinator(activity.Options{}) + taskLease, err := coordinator.AcquireTask(context.Background(), activity.KindTestCommand) + if err != nil { + t.Fatalf("AcquireTask: %v", err) + } + defer taskLease.Release() + provider := &signallingCleanupProvider{called: make(chan struct{})} + tracker := jobs.NewTracker(nil, newOperationsTestLogger(t)) + operations := NewOperations(OperationsConfig{ + Settings: NewSettingsStore(rawSettings), Store: newStorageStore(t, pool), Jobs: tracker, + Activity: coordinator, Providers: []CleanupProvider{provider}, + }) + + jobID, err := operations.RunNow(context.Background(), nil, true) + if err != nil { + t.Fatalf("forced RunNow: %v", err) + } + select { + case <-provider.called: + case <-time.After(time.Second): + t.Fatal("forced cleanup did not run beside active task") + } + waitForJobState(t, tracker, jobID, jobs.StateSucceeded) +} + +func TestRunNowForceBlockedByExistingMaintenanceIsNotForceAvailable(t *testing.T) { + connection := newSQLite(t) + pool := db.NewPool(connection, connection) + rawSettings, err := systemsettings.NewStore(pool) + if err != nil { + t.Fatalf("new settings store: %v", err) + } + coordinator := activity.NewCoordinator(activity.Options{}) + held, _, err := coordinator.TryAcquireMaintenance(context.Background(), 0) + if err != nil { + t.Fatalf("TryAcquireMaintenance: %v", err) + } + defer held.Release() + operations := NewOperations(OperationsConfig{ + Settings: NewSettingsStore(rawSettings), Store: newStorageStore(t, pool), Activity: coordinator, + }) + + jobID, err := operations.RunNow(context.Background(), nil, true) + var busyErr *BusyError + if !errors.As(err, &busyErr) { + t.Fatalf("forced RunNow error = %v, want BusyError", err) + } + if jobID != "" { + t.Fatalf("busy forced RunNow job ID = %q, want empty", jobID) + } + if busyErr.ForceAvailable || len(busyErr.Resources) != 1 || busyErr.Resources[0].Kind != activity.KindMaintenanceRunning { + t.Fatalf("busy response = %#v, want non-overridable maintenance resource", busyErr) + } } func TestRunNowMarksPreemptedTrackedJobFailed(t *testing.T) { @@ -129,7 +194,7 @@ func TestRunNowMarksPreemptedTrackedJobFailed(t *testing.T) { Jobs: tracker, Activity: coordinator, Providers: []CleanupProvider{provider}, }) - jobID, err := operations.RunNow(context.Background(), nil) + jobID, err := operations.RunNow(context.Background(), nil, false) if err != nil { t.Fatalf("RunNow: %v", err) } diff --git a/apps/backend/internal/system/storage/runner.go b/apps/backend/internal/system/storage/runner.go index 009918faed..e5a1987580 100644 --- a/apps/backend/internal/system/storage/runner.go +++ b/apps/backend/internal/system/storage/runner.go @@ -32,6 +32,7 @@ type RunnerConfig struct { Store RunStore Providers []CleanupProvider Overview OverviewInvalidator + Force bool NewID func() string Now func() time.Time } @@ -41,6 +42,7 @@ type Runner struct { store RunStore providers []CleanupProvider overview OverviewInvalidator + force bool newID func() string now func() time.Time } @@ -48,10 +50,11 @@ type Runner struct { const terminalTransitionTimeout = 5 * time.Second type BusyError struct { - Resources []activity.Kind + Resources []activity.BusyResource `json:"busy_resources"` + ForceAvailable bool `json:"force_available"` } -func (e *BusyError) Error() string { return "storage maintenance is busy" } +func (e *BusyError) Error() string { return "storage cleanup is blocked by active Kandev work" } func NewRunner(config RunnerConfig) *Runner { newID := config.NewID @@ -64,7 +67,7 @@ func NewRunner(config RunnerConfig) *Runner { } return &Runner{ activity: config.Activity, store: config.Store, providers: config.Providers, - overview: config.Overview, newID: newID, now: now, + overview: config.Overview, force: config.Force, newID: newID, now: now, } } @@ -78,7 +81,7 @@ func (r *Runner) Run( return MaintenanceRun{}, err } quietPeriod := quietPeriodForTrigger(trigger, settings) - lease, busy, err := r.activity.TryAcquireMaintenance(ctx, quietPeriod) + lease, busy, err := r.acquireMaintenance(ctx, quietPeriod, trigger) if errors.Is(err, activity.ErrBusy) { result := marshalRunResult(map[string]any{"busy_resources": busy}) run, transitionErr := r.transitionRun(ctx, run.ID, RunStateSkippedBusy, result, "host resources are busy") @@ -86,7 +89,10 @@ func (r *Runner) Run( return MaintenanceRun{}, transitionErr } if trigger == RunTriggerManual { - return run, &BusyError{Resources: busy} + return run, &BusyError{ + Resources: activity.BusyResourcesForKinds(busy), + ForceAvailable: forceAvailable(busy), + } } return run, nil } @@ -109,6 +115,26 @@ func (r *Runner) Run( return r.finishRun(ctx, run.ID, lease.Context(), result, runErr) } +func (r *Runner) acquireMaintenance( + ctx context.Context, + quietPeriod time.Duration, + trigger RunTrigger, +) (*activity.MaintenanceLease, []activity.Kind, error) { + if r.force && trigger == RunTriggerManual { + return r.activity.TryAcquireMaintenanceForce(ctx) + } + return r.activity.TryAcquireMaintenance(ctx, quietPeriod) +} + +func forceAvailable(kinds []activity.Kind) bool { + for _, kind := range kinds { + if kind == activity.KindMaintenanceRunning { + return false + } + } + return len(kinds) > 0 +} + func quietPeriodForTrigger(trigger RunTrigger, settings StorageMaintenanceSettings) time.Duration { if trigger == RunTriggerManual { return 0 diff --git a/apps/backend/internal/system/storage_routes_test.go b/apps/backend/internal/system/storage_routes_test.go index e3a64a31e4..39027f43ea 100644 --- a/apps/backend/internal/system/storage_routes_test.go +++ b/apps/backend/internal/system/storage_routes_test.go @@ -10,6 +10,7 @@ import ( "github.com/gin-gonic/gin" "github.com/jmoiron/sqlx" + "github.com/kandev/kandev/internal/agent/runtime/activity" "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/db" systemsettings "github.com/kandev/kandev/internal/system/settings" @@ -95,6 +96,25 @@ func TestStorageAsyncRoutesAndBusyResponse(t *testing.T) { if response.Code != http.StatusConflict { t.Fatalf("busy run status = %d, want 409", response.Code) } + var busyBody struct { + BusyResources []activity.BusyResource `json:"busy_resources"` + ForceAvailable bool `json:"force_available"` + } + if err := json.Unmarshal(response.Body.Bytes(), &busyBody); err != nil { + t.Fatalf("decode busy response: %v", err) + } + if len(busyBody.BusyResources) != 1 || busyBody.BusyResources[0].Label == "" || !busyBody.ForceAvailable { + t.Fatalf("busy response = %#v, want labeled force-available blocker", busyBody) + } + + mutations.busy = false + request = httptest.NewRequest(http.MethodPost, "/api/v1/system/storage/run", bytes.NewBufferString(`{"force":true}`)) + request.Header.Set("Content-Type", "application/json") + response = httptest.NewRecorder() + router.ServeHTTP(response, request) + if response.Code != http.StatusAccepted || !mutations.force { + t.Fatalf("forced run status = %d force=%v, want 202/true", response.Code, mutations.force) + } } func TestStorageRestoreConflictReturnsConflict(t *testing.T) { @@ -167,6 +187,7 @@ func newStorageRoutesTestRouterWithMutations(t *testing.T, mutations storage.Mut type fakeStorageMutations struct { busy bool + force bool restoreErr error deleteErr error } @@ -177,9 +198,13 @@ func (f *fakeStorageMutations) AdoptGoCache(_ context.Context, _ string, _ strin func (f *fakeStorageMutations) Analyze(context.Context) (string, error) { return "analysis-job", nil } -func (f *fakeStorageMutations) RunNow(context.Context, []string) (string, error) { +func (f *fakeStorageMutations) RunNow(_ context.Context, _ []string, force bool) (string, error) { + f.force = force if f.busy { - return "", &storage.BusyError{} + return "", &storage.BusyError{ + Resources: []activity.BusyResource{{Kind: activity.KindTestCommand, Label: "A test command is running"}}, + ForceAvailable: true, + } } return "cleanup-job", nil } 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 670d62d456..f9aadda767 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 @@ -93,6 +93,7 @@ function Providers({ children }: { children: ReactNode }) { ); } +// eslint-disable-next-line max-lines-per-function describe("StorageMaintenanceSettings", () => { afterEach(cleanup); @@ -212,6 +213,37 @@ describe("StorageMaintenanceSettings", () => { }); }); +describe("StorageMaintenanceSettings busy feedback", () => { + afterEach(cleanup); + + beforeEach(() => { + mocks.useSystemJob.mockReturnValue(undefined); + }); + + it("explains busy activity and exposes the direct Run anyway action", () => { + const runAnyway = vi.fn(); + mocks.useStorageMaintenance.mockReturnValue({ + ...controller(overview), + busy: { + resources: [ + { kind: "execution_running", label: "An agent execution is running" }, + { kind: "test_command", label: "A test command is running" }, + ], + forceAvailable: true, + }, + runAnyway, + }); + + render(, { wrapper: Providers }); + + expect(screen.getByText("An agent execution is running")).toBeTruthy(); + expect(screen.getByText("A test command is running")).toBeTruthy(); + expect(screen.getByText(/may disrupt this active work/i)).toBeTruthy(); + fireEvent.click(screen.getByRole("button", { name: "Run anyway" })); + expect(runAnyway).toHaveBeenCalledTimes(1); + }); +}); + describe("StorageMaintenanceSettings pending policy", () => { afterEach(cleanup); 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 f80b5ec459..41c897db21 100644 --- a/apps/web/components/settings/system/storage/storage-maintenance-settings.tsx +++ b/apps/web/components/settings/system/storage/storage-maintenance-settings.tsx @@ -4,7 +4,10 @@ import { useEffect, useRef, useState, type ReactNode } from "react"; import { Alert, AlertDescription, AlertTitle } from "@kandev/ui/alert"; import { Spinner } from "@kandev/ui/spinner"; import { IconAlertTriangle, IconCheck, IconPlayerPlay, IconRefresh } from "@tabler/icons-react"; -import { useStorageMaintenance } from "@/hooks/domains/system/use-storage-maintenance"; +import { + useStorageMaintenance, + type StorageBusyState, +} from "@/hooks/domains/system/use-storage-maintenance"; import type { StorageMaintenanceSettings as Settings, SystemJob } from "@/lib/types/system"; import { useSettingsSaveContributor } from "../../settings-save-provider"; import { StorageActionButton } from "./storage-action-button"; @@ -123,6 +126,62 @@ function StorageActions({ ); } +function StorageActionFeedback({ + controller, +}: { + controller: ReturnType; +}) { + if (controller.busy) { + return ( + void controller.runAnyway()} /> + ); + } + if (!controller.error) return null; + return ( + + + Storage action failed + {controller.error} + + ); +} + +function StorageBusyFeedback({ + busy, + onRunAnyway, +}: { + busy: StorageBusyState; + onRunAnyway: () => void; +}) { + return ( + + + Storage cleanup found active Kandev work + +

Cleanup may disrupt the following work:

+
    + {busy.resources.map((resource) => ( +
  • {resource.label}
    • + ))} +
+ {busy.forceAvailable && ( + <> +

Running cleanup anyway may disrupt this active work.

+ + Run anyway + + + )} + + + ); +} + function serializeSettings(settings: Settings | null): string { return settings ? JSON.stringify(settings) : "loading"; } @@ -195,13 +254,7 @@ export function StorageMaintenanceSettings() {
- {controller.error && ( - - - Storage action failed - {controller.error} - - )} +
{ + test("explains busy activity and allows Run anyway without horizontal overflow", async ({ + testPage, + prCapture, + }) => { + let runAttempts = 0; + await testPage.route("**/api/v1/system/storage/run", async (route) => { + runAttempts += 1; + if (runAttempts === 1) { + await route.fulfill({ + status: 409, + contentType: "application/json", + body: JSON.stringify({ + error: "storage cleanup is blocked by active Kandev work", + busy_resources: [{ kind: "test_command", label: "A test command is running" }], + force_available: true, + }), + }); + return; + } + await route.fulfill({ + status: 202, + contentType: "application/json", + body: JSON.stringify({ job_id: "mobile-force-cleanup" }), + }); + }); + + await testPage.goto("/settings/system/storage"); + await testPage.getByTestId("storage-run-now").tap(); + await expect(testPage.getByTestId("storage-busy")).toContainText("A test command is running"); + await expect(testPage.getByTestId("storage-run-anyway")).toBeVisible(); + expect( + await testPage.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth), + ).toBe(true); + await prCapture.screenshot("busy-feedback", { + caption: "Mobile storage cleanup keeps the warning and Run anyway action in one column", + fullPage: true, + }); + const forceRequest = testPage.waitForRequest( + (request) => + request.method() === "POST" && + new URL(request.url()).pathname === "/api/v1/system/storage/run" && + request.postDataJSON()?.force === true, + ); + await testPage.getByTestId("storage-run-anyway").tap(); + expect((await forceRequest).postDataJSON()).toEqual({ force: true }); + }); + test("opens Storage from mobile navigation and analyzes without horizontal overflow", async ({ testPage, backend, diff --git a/apps/web/e2e/tests/system/storage-maintenance.spec.ts b/apps/web/e2e/tests/system/storage-maintenance.spec.ts index ce7f59bc19..c934c5fdd0 100644 --- a/apps/web/e2e/tests/system/storage-maintenance.spec.ts +++ b/apps/web/e2e/tests/system/storage-maintenance.spec.ts @@ -174,6 +174,7 @@ test.describe("System storage maintenance", () => { testPage, apiClient, seedData, + prCapture, }) => { const task = await apiClient.createTaskWithAgent( seedData.workspaceId, @@ -224,7 +225,31 @@ test.describe("System storage maintenance", () => { status: 409, body: { busy_resources: expect.any(Array) }, }); - await expect(testPage.getByTestId("storage-error")).toContainText(/busy|active/i); - await expect(testPage.getByTestId("storage-run-now")).not.toHaveAttribute("data-job-state"); + expect(responseBody.force_available).toBe(true); + expect(responseBody.busy_resources[0]).toMatchObject({ + kind: expect.any(String), + label: expect.any(String), + }); + await expect(testPage.getByTestId("storage-busy")).toContainText( + responseBody.busy_resources[0].label, + ); + await expect(testPage.getByTestId("storage-busy")).toContainText(/may disrupt/i); + await prCapture.screenshot("busy-feedback", { + caption: "Desktop storage cleanup explains the active work and offers Run anyway", + fullPage: true, + }); + + const forceRequest = testPage.waitForRequest( + (request) => + request.method() === "POST" && + new URL(request.url()).pathname === "/api/v1/system/storage/run", + ); + await testPage.getByTestId("storage-run-anyway").click(); + expect((await forceRequest).postDataJSON()).toEqual({ force: true }); + await expect(testPage.getByTestId("storage-run-now")).toHaveAttribute( + "data-job-state", + "succeeded", + { timeout: 20_000 }, + ); }); }); 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 6c14fc3ea8..40bfc4f9e0 100644 --- a/apps/web/hooks/domains/system/use-storage-maintenance.test.tsx +++ b/apps/web/hooks/domains/system/use-storage-maintenance.test.tsx @@ -2,6 +2,7 @@ import type { ReactNode } from "react"; 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"; const mocks = vi.hoisted(() => ({ @@ -91,6 +92,8 @@ const cleanupJob = { state: "running", started_at: "2026-07-15T00:00:00Z", }; +const STORAGE_BUSY_ERROR_MESSAGE = "storage cleanup is blocked by active Kandev work"; +const TEST_COMMAND_BUSY_LABEL = "A test command is running"; function deferred() { let resolve!: (value: T) => void; @@ -183,6 +186,84 @@ describe("useStorageMaintenance", () => { }); }); +describe("useStorageMaintenance busy feedback", () => { + it("retains labeled busy feedback and reruns the same resources with force", async () => { + mocks.run.mockRejectedValueOnce( + new ApiError(STORAGE_BUSY_ERROR_MESSAGE, 409, { + busy_resources: [{ kind: "test_command", label: TEST_COMMAND_BUSY_LABEL }], + force_available: true, + }), + ); + const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); + await waitFor(() => expect(result.current.overview).toEqual(overview)); + + await act(async () => { + await result.current.runNow(["go_cache"]); + }); + expect(result.current.busy).toEqual({ + resources: [{ kind: "test_command", label: TEST_COMMAND_BUSY_LABEL }], + forceAvailable: true, + resourceSelection: ["go_cache"], + }); + + await act(async () => { + await result.current.runAnyway(); + }); + expect(mocks.run).toHaveBeenNthCalledWith(2, ["go_cache"], true); + }); + + it("restores busy feedback when the forced retry is rejected", async () => { + const initialBusyError = new ApiError(STORAGE_BUSY_ERROR_MESSAGE, 409, { + busy_resources: [{ kind: "test_command", label: TEST_COMMAND_BUSY_LABEL }], + force_available: true, + }); + const forcedBusyError = new ApiError(STORAGE_BUSY_ERROR_MESSAGE, 409, { + busy_resources: [{ kind: "maintenance_running", label: "Storage maintenance is running" }], + force_available: false, + }); + mocks.run.mockRejectedValueOnce(initialBusyError).mockRejectedValueOnce(forcedBusyError); + const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); + await waitFor(() => expect(result.current.overview).toEqual(overview)); + + await act(async () => { + await result.current.runNow(["go_cache"]); + }); + await act(async () => { + await result.current.runAnyway(); + }); + + expect(result.current.busy).toEqual({ + resources: [{ kind: "maintenance_running", label: "Storage maintenance is running" }], + forceAvailable: false, + resourceSelection: ["go_cache"], + }); + expect(mocks.toast).not.toHaveBeenCalledWith( + expect.objectContaining({ title: "Storage action failed" }), + ); + }); + + it("clears stale busy feedback when another storage action starts", async () => { + mocks.run.mockRejectedValueOnce( + new ApiError(STORAGE_BUSY_ERROR_MESSAGE, 409, { + busy_resources: [{ kind: "test_command", label: TEST_COMMAND_BUSY_LABEL }], + force_available: true, + }), + ); + const { result } = renderHook(() => useStorageMaintenance(), { wrapper }); + await waitFor(() => expect(result.current.overview).toEqual(overview)); + + await act(async () => { + await result.current.runNow(); + }); + expect(result.current.busy).not.toBeNull(); + + await act(async () => { + await result.current.save(settings); + }); + expect(result.current.busy).toBeNull(); + }); +}); + describe("useStorageMaintenance pending action tracking", () => { it("returns to a pending resource action after an overlapping save finishes", async () => { const pendingRun = deferred<{ job_id: string }>(); diff --git a/apps/web/hooks/domains/system/use-storage-maintenance.ts b/apps/web/hooks/domains/system/use-storage-maintenance.ts index 2134202b2b..33b0c0efa4 100644 --- a/apps/web/hooks/domains/system/use-storage-maintenance.ts +++ b/apps/web/hooks/domains/system/use-storage-maintenance.ts @@ -11,6 +11,7 @@ import { } from "react"; import { useAppStore } from "@/components/state-provider"; import { useToast } from "@/components/toast-provider"; +import { ApiError } from "@/lib/api/client"; import { adoptStorageGoCache, analyzeStorage, @@ -22,7 +23,12 @@ import { runStorageMaintenance, saveStorageSettings, } from "@/lib/api/domains/system-api"; -import type { StorageMaintenanceSettings, SystemJob } from "@/lib/types/system"; +import type { + StorageBusyResource, + StorageBusyResponse, + StorageMaintenanceSettings, + SystemJob, +} from "@/lib/types/system"; import { useSystemJob } from "./use-system-jobs"; export type StoragePendingAction = @@ -35,6 +41,12 @@ export type StoragePendingAction = | "delete" | null; +export interface StorageBusyState { + resources: StorageBusyResource[]; + forceAvailable: boolean; + resourceSelection?: string[]; +} + function messageFromError(error: unknown): string { return error instanceof Error ? error.message : String(error); } @@ -43,6 +55,22 @@ function isTerminal(state?: string): boolean { return state === "succeeded" || state === "failed"; } +function busyStateFromError(error: unknown, resourceSelection?: string[]): StorageBusyState | null { + if (!(error instanceof ApiError) || error.status !== 409) return null; + const body = error.body as Partial | null; + if (!body || !Array.isArray(body.busy_resources)) return null; + const resources = body.busy_resources.filter( + (resource): resource is StorageBusyResource => + Boolean(resource) && typeof resource.kind === "string" && typeof resource.label === "string", + ); + if (resources.length === 0) return null; + return { + resources, + forceAvailable: body.force_available === true, + resourceSelection, + }; +} + export function settingsWithDockerAcknowledgement( settings: StorageMaintenanceSettings, acknowledged: boolean, @@ -105,19 +133,16 @@ function useStorageActionRunner() { return { pendingAction, error, setError, finishLoading, perform }; } -function useStorageActions(reload: Reload) { - const { toast } = useToast(); - const { pendingAction, error, setError, finishLoading, perform } = useStorageActionRunner(); - const [analysisJobId, setAnalysisJobId] = useState(null); - const [cleanupJobId, setCleanupJobId] = useState(null); - const [deleteJobId, setDeleteJobId] = useState(null); - const analysisJob = useSystemJob(analysisJobId); - const cleanupJob = useSystemJob(cleanupJobId); - const deleteJob = useSystemJob(deleteJobId); - +function useStoragePolicyActions( + perform: ReturnType["perform"], + reload: Reload, + toast: ReturnType["toast"], + clearBusy: () => void, +) { const save = useCallback( - async (settings: StorageMaintenanceSettings, confirmation?: "DEDICATED") => - perform( + async (settings: StorageMaintenanceSettings, confirmation?: "DEDICATED") => { + clearBusy(); + return perform( "save", async () => { await saveStorageSettings(settings, confirmation); @@ -125,61 +150,121 @@ function useStorageActions(reload: Reload) { toast({ title: "Storage policy saved", variant: "success" }); }, true, - ), - [perform, reload, toast], + ); + }, + [clearBusy, perform, reload, toast], ); const adopt = useCallback( - async (path: string) => - perform("adopt", async () => { + async (path: string) => { + clearBusy(); + return perform("adopt", async () => { await adoptStorageGoCache(path); await reload(); toast({ title: "Go build cache adopted", variant: "success" }); - }), - [perform, reload, toast], + }); + }, + [clearBusy, perform, reload, toast], ); - const analyze = useCallback( - async () => - perform("analyze", async () => { - const accepted = await analyzeStorage(); - setAnalysisJobId(accepted.job_id); - toast({ title: "Storage analysis started", variant: "success" }); - }), - [perform, toast], + return { save, adopt }; +} + +function useStorageDeleteAction( + perform: ReturnType["perform"], + toast: ReturnType["toast"], + clearBusy: () => void, + setDeleteJobId: Dispatch>, +) { + return useCallback( + async (id: string) => { + clearBusy(); + return perform("delete", async () => { + const accepted = await deleteStorageQuarantine(id); + setDeleteJobId(accepted.job_id); + toast({ title: "Permanent deletion started", variant: "success" }); + }); + }, + [clearBusy, perform, setDeleteJobId, toast], ); +} + +function useStorageActions(reload: Reload) { + const { toast } = useToast(); + const { pendingAction, error, setError, finishLoading, perform } = useStorageActionRunner(); + const [analysisJobId, setAnalysisJobId] = useState(null); + const [cleanupJobId, setCleanupJobId] = useState(null); + const [deleteJobId, setDeleteJobId] = useState(null); + const [busy, setBusy] = useState(null); + const analysisJob = useSystemJob(analysisJobId); + const cleanupJob = useSystemJob(cleanupJobId); + const deleteJob = useSystemJob(deleteJobId); + const clearBusy = useCallback(() => setBusy(null), []); + const { save, adopt } = useStoragePolicyActions(perform, reload, toast, clearBusy); + + const analyze = useCallback(async () => { + clearBusy(); + return perform("analyze", async () => { + const accepted = await analyzeStorage(); + setAnalysisJobId(accepted.job_id); + toast({ title: "Storage analysis started", variant: "success" }); + }); + }, [clearBusy, perform, toast]); const runNow = useCallback( async (resources?: string[]) => { setCleanupJobId(null); + clearBusy(); return perform("run", async () => { - const accepted = await runStorageMaintenance(resources); - setCleanupJobId(accepted.job_id); - toast({ title: "Storage maintenance started", variant: "success" }); + try { + const accepted = await runStorageMaintenance(resources); + setCleanupJobId(accepted.job_id); + toast({ title: "Storage maintenance started", variant: "success" }); + } catch (error) { + const nextBusy = busyStateFromError(error, resources); + if (nextBusy) { + setBusy(nextBusy); + return; + } + throw error; + } }); }, - [perform, toast], + [clearBusy, perform, toast], ); + const runAnyway = useCallback(async () => { + if (!busy?.forceAvailable) return; + const resources = busy.resourceSelection; + clearBusy(); + setCleanupJobId(null); + return perform("run", async () => { + try { + const accepted = await runStorageMaintenance(resources, true); + setCleanupJobId(accepted.job_id); + toast({ title: "Storage maintenance started", variant: "success" }); + } catch (error) { + const nextBusy = busyStateFromError(error, resources); + if (nextBusy) { + setBusy(nextBusy); + return; + } + throw error; + } + }); + }, [busy, clearBusy, perform, toast]); const restore = useCallback( - async (id: string) => - perform("restore", async () => { + async (id: string) => { + clearBusy(); + return perform("restore", async () => { await restoreStorageQuarantine(id); await reload(); toast({ title: "Quarantined resource restored", variant: "success" }); - }), - [perform, reload, toast], - ); - - const permanentlyDelete = useCallback( - async (id: string) => - perform("delete", async () => { - const accepted = await deleteStorageQuarantine(id); - setDeleteJobId(accepted.job_id); - toast({ title: "Permanent deletion started", variant: "success" }); - }), - [perform, toast], + }); + }, + [clearBusy, perform, reload, toast], ); + const permanentlyDelete = useStorageDeleteAction(perform, toast, clearBusy, setDeleteJobId); return { pendingAction, @@ -189,6 +274,8 @@ function useStorageActions(reload: Reload) { save, analyze, runNow, + runAnyway, + busy, adopt, restore, permanentlyDelete, diff --git a/apps/web/lib/api/domains/system-api.test.ts b/apps/web/lib/api/domains/system-api.test.ts index de97636eeb..8300ff1aca 100644 --- a/apps/web/lib/api/domains/system-api.test.ts +++ b/apps/web/lib/api/domains/system-api.test.ts @@ -383,11 +383,17 @@ describe("storage maintenance", () => { it("starts analysis, selected cleanup, and restore operations", async () => { fetchSpy .mockResolvedValueOnce(jsonResponse({ job_id: "analysis" })) - .mockResolvedValueOnce(jsonResponse({ job_id: "cleanup" })) - .mockResolvedValueOnce(jsonResponse({ entry: { id: "restored" } })); + .mockResolvedValueOnce(jsonResponse({ job_id: "cleanup" })); expect((await analyzeStorage()).job_id).toBe("analysis"); expect((await runStorageMaintenance(["workspaces"])).job_id).toBe("cleanup"); expect(JSON.parse(String(lastCall().init?.body))).toEqual({ resources: ["workspaces"] }); + fetchSpy.mockResolvedValueOnce(jsonResponse({ job_id: "forced-cleanup" })); + expect((await runStorageMaintenance(["workspaces"], true)).job_id).toBe("forced-cleanup"); + expect(JSON.parse(String(lastCall().init?.body))).toEqual({ + resources: ["workspaces"], + force: true, + }); + fetchSpy.mockResolvedValueOnce(jsonResponse({ entry: { id: "restored" } })); expect((await restoreStorageQuarantine("entry-1")).id).toBe("restored"); }); }); diff --git a/apps/web/lib/api/domains/system-api.ts b/apps/web/lib/api/domains/system-api.ts index f5ca75d7e1..a80ca6b01e 100644 --- a/apps/web/lib/api/domains/system-api.ts +++ b/apps/web/lib/api/domains/system-api.ts @@ -247,6 +247,7 @@ export function analyzeStorage(options?: ApiRequestOptions): Promise { return fetchJson(`${SYSTEM_BASE}/storage/run`, { @@ -254,7 +255,10 @@ export function runStorageMaintenance( init: { ...(options?.init ?? {}), method: "POST", - body: JSON.stringify(resources?.length ? { resources } : {}), + body: JSON.stringify({ + ...(resources?.length ? { resources } : {}), + ...(force ? { force: true } : {}), + }), }, }); } diff --git a/apps/web/lib/types/system.ts b/apps/web/lib/types/system.ts index 2bc8d95918..57ab16dc15 100644 --- a/apps/web/lib/types/system.ts +++ b/apps/web/lib/types/system.ts @@ -259,6 +259,17 @@ export type StorageRunState = | "cancelled" | "skipped_busy"; +export interface StorageBusyResource { + kind: string; + label: string; +} + +export interface StorageBusyResponse { + error: string; + busy_resources: StorageBusyResource[]; + force_available: boolean; +} + export interface StorageMaintenanceRun { id: string; trigger: "scheduled" | "manual" | "analysis"; diff --git a/docs/plans/storage-busy-override/plan.md b/docs/plans/storage-busy-override/plan.md new file mode 100644 index 0000000000..18851cc692 --- /dev/null +++ b/docs/plans/storage-busy-override/plan.md @@ -0,0 +1,124 @@ +--- +spec: docs/specs/system-page/storage-maintenance.md +created: 2026-07-27 +status: implemented +--- + +# Implementation Plan: Storage Busy Feedback and Override + +## Overview + +Make a blocked manual cleanup explain the Kandev activity categories that hold the gate, then let +the operator explicitly run the same cleanup alongside current task activity. Preserve the +install-wide mutual exclusion that prevents overlapping maintenance runs, and keep future task work +able to cancel maintenance as it does today. The work proceeds from backend activity/API semantics, +through the shared frontend controller and Storage page, to desktop and Pixel 5 coverage. + +## Backend + +### Activity-gate and manual-run contract + +- Extend `apps/backend/internal/agent/runtime/activity/coordinator.go` with a typed public busy + resource shape that retains the stable `Kind` and supplies the user-facing label. Preserve the + existing kind values and sorting, but do not expose task prompts, paths, IDs, or names. +- Add an explicit manual-maintenance acquisition mode that may ignore already-active task work but + still rejects `maintenance_running`. It must install the normal maintenance lease so a task that + begins after the force run can cancel it through `AcquireTask`. +- Thread an explicit `force` flag through `apps/backend/internal/system/storage/operations.go` and + `runner.go`. Normal manual and scheduled paths retain their current admission behavior; only a + manual force request skips the current-activity check. +- Extend the `Mutations` interface and `runRequest` in + `apps/backend/internal/system/storage/handler.go`. A busy response returns typed + `busy_resources` and `force_available`; accepted forced runs retain the existing `202 { job_id }` + response. + +### Backend tests + +- Update `apps/backend/internal/agent/runtime/activity/coordinator_test.go` to prove force admission + coexists with active task work, refuses an existing maintenance lease, and remains preemptible by + subsequently admitted task activity. +- Extend `apps/backend/internal/system/storage/operations_test.go`, `runner_test.go`, + `handler_test.go`, and `storage_routes_test.go` for the typed 409 response, force=false rejection, + force=true job start, and maintenance-running refusal. + +## Frontend + +### API, types, and domain controller + +- Add the typed busy response/resource contracts to `apps/web/lib/types/system.ts` and let + `runStorageMaintenance` in `apps/web/lib/api/domains/system-api.ts` send an optional `force` body + field without changing callers that omit it. +- In `apps/web/hooks/domains/system/use-storage-maintenance.ts`, inspect `ApiError.body` for the + Storage 409 shape. Keep normal failures as ordinary action errors, but expose a structured busy + state and a `runAnyway` action that repeats the original resource selection with `force: true`. + +### Storage page and mobile contract + +- Update `apps/web/components/settings/system/storage/storage-maintenance-settings.tsx` to replace + the generic busy alert with a plain-language list of the reported activity labels, a disruption + warning, and a distinct **Run anyway** button when `force_available` is true. An existing + maintenance run remains informational with no override control. +- Desktop keeps the busy feedback directly below the existing Analyze/Run-now action bar. On phones, + the existing one-column Storage actions layout remains the nearest shipped exemplar: the warning + and full-width, 44px **Run anyway** action stack inline with the page, need no drawer or dialog, + preserve the document as the single scroll owner, and retain zero horizontal overflow. + +## Tests + +- **Activity and operation semantics:** force cleanup runs beside existing task activity, cannot run + beside maintenance, and later task work still cancels it. **Files:** + `apps/backend/internal/agent/runtime/activity/coordinator_test.go`, + `apps/backend/internal/system/storage/operations_test.go`, and `runner_test.go`. + **How:** deterministic lease/channel tests plus existing tracked-job tests. +- **HTTP contract:** the Storage route returns labeled blockers and availability on 409, accepts + `force: true`, and does not change the ordinary 202 job response. **Files:** + `apps/backend/internal/system/storage/handler_test.go` and `storage_routes_test.go`. +- **Controller and rendered feedback:** the hook preserves resource selection after a structured + 409, sends force only from **Run anyway**, and the settings page renders warning/list/button only + when supplied by the backend. **Files:** + `apps/web/hooks/domains/system/use-storage-maintenance.test.tsx`, + `apps/web/components/settings/system/storage/storage-maintenance-settings.test.tsx`, and + `apps/web/lib/api/domains/system-api.test.ts`. + +## E2E Tests + +- **Scenario:** a running task blocks manual cleanup. **File:** + `apps/web/e2e/tests/system/storage-maintenance.spec.ts`. **Verify:** the page identifies the + active work, exposes **Run anyway**, sends `{ force: true }`, and receives a normal cleanup job. +- **Scenario:** the same busy feedback is usable on Pixel 5. **File:** + `apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts`. **Verify:** the warning and + override remain visible/tappable, start the cleanup, and leave document horizontal overflow at + zero. + +## Implementation Waves + +Wave 1: + +- [x] [Task 01: Backend activity and API contract](task-01-backend-activity-api.md) + +Wave 2: + +- [x] [Task 02: Storage busy feedback and override UI](task-02-frontend-busy-override.md) + +Wave 3: + +- [x] [Task 03: Desktop and mobile override coverage](task-03-e2e-busy-override.md) + +## Risks + +- A forced cleanup can disrupt the active work it names; the inline warning must remain explicit + and the feature must never silently retry with force. +- Bypassing an active task must not become a bypass for an existing maintenance run; the latter + remains an unavailable override. +- The activity coordinator currently knows categories rather than user/task identity. The API must + stay at that privacy-safe category granularity unless a later, separately designed feature adds + activity identity. + +## Validation Results + +- Backend activity/storage/system race tests: passed (107 tests). +- Backend backendapp/storage/system integration tests: passed (312 tests). +- Web Storage unit tests: passed (57 tests). +- Web typecheck: passed. +- Managed desktop/mobile Storage E2E: passed (7 tests). +- Commit hooks and GitHub PR checks: passed. diff --git a/docs/plans/storage-busy-override/task-01-backend-activity-api.md b/docs/plans/storage-busy-override/task-01-backend-activity-api.md new file mode 100644 index 0000000000..a32eabe584 --- /dev/null +++ b/docs/plans/storage-busy-override/task-01-backend-activity-api.md @@ -0,0 +1,58 @@ +--- +id: "01-backend-activity-api" +title: "Backend activity and API contract" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/system-page/storage-maintenance.md" +--- + +# Task 01: Backend Activity and API Contract + +## Acceptance + +- A blocked manual Storage run returns stable busy kinds with user-facing labels and declares + whether a force request is available. +- `force: true` starts a cleanup beside current task activity, but never beside another maintenance + run; a later task acquisition still cancels the forced maintenance lease. +- Existing scheduled and non-forced manual admission behavior, run persistence, and 202 job shape + remain unchanged. + +## Verification + +```bash +cd apps/backend && go test ./internal/agent/runtime/activity ./internal/system/storage ./internal/system +``` + +## Files likely touched + +- `apps/backend/internal/agent/runtime/activity/coordinator.go` +- `apps/backend/internal/agent/runtime/activity/coordinator_test.go` +- `apps/backend/internal/system/storage/runner.go` +- `apps/backend/internal/system/storage/runner_test.go` +- `apps/backend/internal/system/storage/operations.go` +- `apps/backend/internal/system/storage/operations_test.go` +- `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. + +## Inputs + +- Spec: Manual Run now, API surface, and busy-override scenarios +- Plan: Backend and Tests +- Existing activity lease behavior in `internal/agent/runtime/activity/coordinator.go` + +## Output contract + +Report acceptance status, changed request/response shapes, focused test results, risks, and update +this task plus `plan.md` to `done` in the same conversation. + +## Validation Results + +- `go test ./internal/agent/runtime/activity ./internal/system/storage ./internal/system -count=1 -race`: passed. +- `go test ./internal/backendapp ./internal/system/storage ./internal/system -count=1`: passed. diff --git a/docs/plans/storage-busy-override/task-02-frontend-busy-override.md b/docs/plans/storage-busy-override/task-02-frontend-busy-override.md new file mode 100644 index 0000000000..b9ae4b3239 --- /dev/null +++ b/docs/plans/storage-busy-override/task-02-frontend-busy-override.md @@ -0,0 +1,60 @@ +--- +id: "02-frontend-busy-override" +title: "Storage busy feedback and override UI" +status: done +wave: 2 +depends_on: ["01-backend-activity-api"] +plan: "plan.md" +spec: "../../specs/system-page/storage-maintenance.md" +--- + +# Task 02: Storage Busy Feedback and Override UI + +## Acceptance + +- The Storage controller recognizes the typed 409 response, retains the original cleanup resource + selection, and sends `force: true` only after the user selects **Run anyway**. +- The settings page names every reported active category, warns about disruption, and renders the + direct override only when the backend says it is available. +- The existing phone one-column action composition presents the warning and an accessible, full-width + override button without a dialog, hover-only behavior, or horizontal overflow. + +## Verification + +```bash +cd apps && pnpm --filter @kandev/web test -- --run \ + hooks/domains/system/use-storage-maintenance.test.tsx \ + components/settings/system/storage/storage-maintenance-settings.test.tsx \ + lib/api/domains/system-api.test.ts +cd apps/web && pnpm run typecheck +``` + +## 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/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-maintenance-settings.test.tsx` + +## Dependencies + +Task 01 defines the typed 409 response and the `force` request field. + +## Inputs + +- Spec: API surface and busy-override scenarios +- Plan: Frontend and Storage page/mobile contract +- Existing `ApiError.body` handling in `apps/web/lib/api/client.ts` + +## Output contract + +Report acceptance status, UI/API contract changes, focused test results, mobile evidence or blocker, +risks, and update this task plus `plan.md` to `done` in the same conversation. + +## Validation Results + +- Focused web tests for the hook, Storage settings, and system API: passed (57 tests). +- `cd apps/web && pnpm run typecheck`: passed. diff --git a/docs/plans/storage-busy-override/task-03-e2e-busy-override.md b/docs/plans/storage-busy-override/task-03-e2e-busy-override.md new file mode 100644 index 0000000000..f502a75832 --- /dev/null +++ b/docs/plans/storage-busy-override/task-03-e2e-busy-override.md @@ -0,0 +1,53 @@ +--- +id: "03-e2e-busy-override" +title: "Desktop and mobile override coverage" +status: done +wave: 3 +depends_on: ["01-backend-activity-api", "02-frontend-busy-override"] +plan: "plan.md" +spec: "../../specs/system-page/storage-maintenance.md" +--- + +# Task 03: Desktop and Mobile Override Coverage + +## Acceptance + +- Desktop E2E proves a running task yields labeled busy feedback, then **Run anyway** starts the + normal cleanup job with `force: true`. +- Pixel 5 E2E proves the same action is visible and tappable in the Storage page’s one-column flow, + completes the user outcome, and leaves document horizontal overflow at zero. +- The maintenance-running case remains unbypassable when practical to cover at the public route; + otherwise the backend contract test is named as its coverage. + +## Verification + +```bash +cd apps/web && pnpm e2e:run --project chromium tests/system/storage-maintenance.spec.ts +cd apps/web && pnpm e2e:run --project mobile-chrome tests/system/mobile-storage-maintenance.spec.ts +``` + +## Files likely touched + +- `apps/web/e2e/tests/system/storage-maintenance.spec.ts` +- `apps/web/e2e/tests/system/mobile-storage-maintenance.spec.ts` +- `apps/web/e2e/helpers/storage-maintenance.ts` only if a deterministic activity fixture is needed + +## Dependencies + +Tasks 01 and 02. + +## Inputs + +- Spec: busy-override scenarios +- Plan: E2E Tests and mobile contract +- Existing active-task busy scenario in `tests/system/storage-maintenance.spec.ts` + +## Output contract + +Report acceptance status, exact E2E command results, screenshots/traces on failure, risks, and +update this task plus `plan.md` to `done` in the same conversation. + +## Validation Results + +- Desktop Storage E2E: passed (4 tests in the focused suite). +- Pixel 5 Storage E2E: passed (3 tests in the focused suite). diff --git a/docs/public/operations.md b/docs/public/operations.md index 88bcda608d..2f5f079938 100644 --- a/docs/public/operations.md +++ b/docs/public/operations.md @@ -77,7 +77,9 @@ The supported SQLite layout for the System database and restore pages is the der Open **Settings > System > Storage** to inspect Kandev-managed disk usage and configure cleanup. **Analyze** is read-only. **Run now** applies only the enabled cleanup rules and refuses to start -while task resources are active or another maintenance run owns the cleanup gate. +while another maintenance run owns the cleanup gate. If task resources are active, the page names +the active work and offers **Run anyway** after an explicit disruption warning. Use that override +only when the active task work can tolerate cleanup running alongside it. Storage analysis results are cached in the running backend for 15 minutes, so page reloads and policy saves reuse the displayed snapshot instead of scanning disk again. The page shows when that diff --git a/docs/specs/system-page/storage-maintenance.md b/docs/specs/system-page/storage-maintenance.md index a6a560710d..8f20368106 100644 --- a/docs/specs/system-page/storage-maintenance.md +++ b/docs/specs/system-page/storage-maintenance.md @@ -1,7 +1,7 @@ --- -status: shipped +status: building created: 2026-07-14 -updated: 2026-07-23 +updated: 2026-07-27 owner: cfl --- @@ -54,8 +54,13 @@ reclaim that space without maintaining cron or systemd configuration outside Kan context, waits for the active provider operation to stop, and then proceeds. Maintenance never races a newly admitted task. - Manual **Run now** uses the same mutual-exclusion/current-activity gate, but does not wait out - the configured quiet period. It returns a visible busy result when task activity is current or - another maintenance run holds the gate, and has no force-while-busy mode. + the configured quiet period. When current Kandev activity blocks it, the page names each activity + type it found (for example, a running agent session or test command), warns that cleanup can + disrupt that work, and offers a distinct **Run anyway** action directly in the busy state. +- **Run anyway** starts the requested manual cleanup alongside the activity that originally blocked + it. It skips only the current-activity admission check: it neither stops the activity nor allows + two storage-maintenance runs to overlap. New task work may still preempt the cleanup through the + normal maintenance cancellation path. - The Go-cache analysis row exposes a resource-specific **Clean Go cache** action only when the cache is Kandev-owned and above its configured maximum. That action submits an explicit `go_cache` selection through the same manual-run gate. @@ -307,9 +312,13 @@ POST /api/v1/system/storage/analyze -> 202 { job_id } POST /api/v1/system/storage/run - body: { resources?: string[] } + body: { resources?: string[], force?: boolean } -> 202 { job_id } - -> 409 { error, busy_resources[] } when the idle gate is unavailable + -> 409 { + error: string, + busy_resources: [{ kind: string, label: string }], + force_available: boolean + } when current activity or another maintenance run holds the gate GET /api/v1/system/storage/runs?limit=20 -> { runs: StorageMaintenanceRun[] } @@ -336,6 +345,13 @@ window and replaces the cached snapshot only when the forced analysis succeeds. Storage operations use the existing `system.job.update` WebSocket event and polling fallback. Job kinds are `storage-analysis`, `storage-cleanup`, and `storage-quarantine-delete`. +`busy_resources` uses stable machine-readable `kind` values and plain-language `label` values. +The response exposes activity categories, not task names, prompts, paths, or other session content. +`force_available` is true only when `force: true` can bypass the reported current task activity; +it is false when another storage-maintenance run already holds the install-wide maintenance lease. +When `force: true` is accepted, the response remains the ordinary `202 { job_id }` cleanup-job +contract. + The task unarchive response may additionally include: ```json @@ -452,6 +468,14 @@ enabling host-global Docker cleanup require explicit UI confirmation and server- arrives, **THEN** the run is recorded as `skipped_busy` and no provider changes resources. - **GIVEN** maintenance holds the idle gate, **WHEN** a new task launch arrives, **THEN** maintenance is cancelled and the launch proceeds only after the active provider stops. +- **GIVEN** a running agent session or command blocks manual cleanup, **WHEN** the user selects + **Run now**, **THEN** the page names the activity categories found, warns that cleanup can disrupt + them, and shows **Run anyway** without opening a confirmation dialog. +- **GIVEN** a manual cleanup is blocked only by current task activity, **WHEN** the user selects + **Run anyway**, **THEN** Kandev starts the requested cleanup with `force: true` while the existing + activity continues and records the normal cleanup-job result. +- **GIVEN** a manual cleanup is blocked by another storage-maintenance run, **WHEN** the user views + the busy feedback, **THEN** it identifies maintenance as the blocker and does not offer a bypass. - **GIVEN** an unreferenced task directory older than the orphan grace period contains `node_modules`, **WHEN** workspace cleanup runs with a successful authoritative inventory, **THEN** the whole task root moves to quarantine and its measured bytes appear in the run result.