diff --git a/backend/cmd/server/main.go b/backend/cmd/server/main.go index 2a3c99d..0ffa167 100644 --- a/backend/cmd/server/main.go +++ b/backend/cmd/server/main.go @@ -139,32 +139,22 @@ func main() { }() go func() { + // Prune shortly after startup as well as on the tick. The ticker's first + // fire is 24h away, so an instance that restarts daily — a container on a + // nightly compose pull, say — never reached it and the cleanup never ran + // at all. The short delay keeps the prune off the startup critical path. + startup := time.NewTimer(2 * time.Minute) + defer startup.Stop() + ticker := time.NewTicker(24 * time.Hour) defer ticker.Stop() for { select { + case <-startup.C: + db.PruneHistory() case <-ticker.C: - retentionStr, err := db.GetSetting("max_log_retention_days") - if err != nil { - slog.Error("Failed to get log retention setting", "error", err) - continue - } - retentionDays := 90 - if retentionStr != "" { - if _, err := fmt.Sscanf(retentionStr, "%d", &retentionDays); err != nil { - slog.Error("Failed to parse log retention days", "error", err) - continue - } - } - if retentionDays < 7 { - retentionDays = 7 - } - if err := db.DeleteOldActionLogs(retentionDays); err != nil { - slog.Error("Failed to delete old action logs", "error", err) - } else { - slog.Info("Cleaned up old action logs", "retention_days", retentionDays) - } + db.PruneHistory() case <-ctx.Done(): return } diff --git a/backend/internal/database/migrations.go b/backend/internal/database/migrations.go index f87db75..3dca8d4 100644 --- a/backend/internal/database/migrations.go +++ b/backend/internal/database/migrations.go @@ -279,6 +279,21 @@ CREATE INDEX IF NOT EXISTS idx_action_log_created_at ON action_log(created_at); ALTER TABLE action_log ADD COLUMN request_id TEXT; CREATE INDEX IF NOT EXISTS idx_action_log_request_id ON action_log(request_id); +`, + }, + { + Version: 11, + Name: "history_retention_settings", + SQL: ` +-- Retention for the two history tables that previously grew without bound. +-- Defaults match max_log_retention_days so all three behave the same; the +-- floor is enforced in code (database.RetentionDays), not here, so an operator +-- editing the row directly cannot bypass it either. +INSERT OR IGNORE INTO settings (key, value) VALUES ('max_update_history_retention_days', '90'); +INSERT OR IGNORE INTO settings (key, value) VALUES ('max_backup_history_retention_days', '90'); + +-- update_history was only ever queried by completed_at through a full scan. +CREATE INDEX IF NOT EXISTS idx_update_history_completed_at ON update_history(completed_at); `, }, } diff --git a/backend/internal/database/retention.go b/backend/internal/database/retention.go new file mode 100644 index 0000000..2ccb757 --- /dev/null +++ b/backend/internal/database/retention.go @@ -0,0 +1,135 @@ +package database + +import ( + "log/slog" + "strconv" + "strings" +) + +// Retention bounds, shared by every history table so one operator-facing +// concept ("how long is history kept") behaves identically everywhere. +const ( + // DefaultRetentionDays applies when the setting is unset or unparseable. + DefaultRetentionDays = 90 + // MinRetentionDays is the floor. A prune is destructive and irreversible, + // so an absurdly low setting must not wipe the history an operator is in + // the middle of reading. Matches the action_log floor. + MinRetentionDays = 7 +) + +// Settings keys holding per-table retention. action_log keeps its original key +// so existing deployments' configured value is not silently reset. +const ( + SettingLogRetentionDays = "max_log_retention_days" + SettingUpdateHistoryRetentionDays = "max_update_history_retention_days" + SettingBackupHistoryRetentionDays = "max_backup_history_retention_days" +) + +// RetentionDays reads a retention setting, applying the default when it is +// unset or unparseable and clamping to the floor. +// +// It never returns an error: a retention lookup failing must not stop the +// cleanup pass from pruning the other tables. +func RetentionDays(value string) int { + days := DefaultRetentionDays + trimmed := strings.TrimSpace(value) + if trimmed != "" { + parsed, err := strconv.Atoi(trimmed) + if err != nil { + slog.Warn("Unparseable retention setting, using the default", + "value", value, "default_days", DefaultRetentionDays) + return DefaultRetentionDays + } + days = parsed + } + if days < MinRetentionDays { + return MinRetentionDays + } + return days +} + +// RetentionDays resolves the retention for a settings key. +func (d *DB) RetentionDays(key string) int { + value, err := d.GetSetting(key) + if err != nil { + // A missing row is the normal case on a fresh install before the + // migration's seed, not a fault worth failing the pass over. + return DefaultRetentionDays + } + return RetentionDays(value) +} + +// DeleteOldUpdateHistory removes update_history rows completed longer ago than +// retentionDays, returning how many went. +// +// The rows are matched on completed_at, the same column the manual "clear +// history older than" endpoint uses, so a run still in flight (completed_at +// unset) is never pruned out from under itself. +func (d *DB) DeleteOldUpdateHistory(retentionDays int) (int, error) { + query := `DELETE FROM update_history + WHERE completed_at IS NOT NULL + AND completed_at < datetime('now', '-' || ? || ' days')` + result, err := d.db.Exec(query, retentionDays) + if err != nil { + return 0, err + } + affected, _ := result.RowsAffected() + return int(affected), nil +} + +// DeleteOldBackupRuns removes backup_runs started longer ago than +// retentionDays. backup_run_items rows go with their parent via +// ON DELETE CASCADE, which is only effective because foreign_keys enforcement +// is set pool-wide on the DSN (agent-os-94t); the cascade is covered by a test +// rather than assumed. +func (d *DB) DeleteOldBackupRuns(retentionDays int) (int, error) { + query := `DELETE FROM backup_runs WHERE started_at < datetime('now', '-' || ? || ' days')` + result, err := d.db.Exec(query, retentionDays) + if err != nil { + return 0, err + } + affected, _ := result.RowsAffected() + return int(affected), nil +} + +// RetentionResult reports what one cleanup pass removed. +type RetentionResult struct { + UpdateHistory int + BackupRuns int +} + +// PruneHistory runs every retention policy once. Each table is pruned +// independently: one failing must not skip the others, since the whole point is +// bounding growth on a long-lived instance. +func (d *DB) PruneHistory() RetentionResult { + var result RetentionResult + + logDays := d.RetentionDays(SettingLogRetentionDays) + if err := d.DeleteOldActionLogs(logDays); err != nil { + slog.Error("Failed to delete old action logs", "error", err, "retention_days", logDays) + } + + updateDays := d.RetentionDays(SettingUpdateHistoryRetentionDays) + if n, err := d.DeleteOldUpdateHistory(updateDays); err != nil { + slog.Error("Failed to delete old update history", "error", err, "retention_days", updateDays) + } else { + result.UpdateHistory = n + } + + backupDays := d.RetentionDays(SettingBackupHistoryRetentionDays) + if n, err := d.DeleteOldBackupRuns(backupDays); err != nil { + slog.Error("Failed to delete old backup runs", "error", err, "retention_days", backupDays) + } else { + result.BackupRuns = n + } + + slog.Info("History retention pass complete", + "log_retention_days", logDays, + "update_history_retention_days", updateDays, + "backup_history_retention_days", backupDays, + "update_history_deleted", result.UpdateHistory, + "backup_runs_deleted", result.BackupRuns, + ) + + return result +} diff --git a/backend/internal/database/retention_test.go b/backend/internal/database/retention_test.go new file mode 100644 index 0000000..756d1fc --- /dev/null +++ b/backend/internal/database/retention_test.go @@ -0,0 +1,223 @@ +package database + +import ( + "fmt" + "testing" + "time" +) + +// update_history, backup_runs and backup_run_items grew without bound: only +// sessions and action_log were ever pruned. The growth is slow, silent and only +// shows up on exactly the deployments that matter — long-lived ones with +// scheduled updates or backups switched on (agent-os-0jp). + +// seedUpdateHistory inserts a row completed daysAgo days in the past. +func seedUpdateHistory(t *testing.T, d *DB, id string, daysAgo int) { + t.Helper() + when := time.Now().AddDate(0, 0, -daysAgo).UTC().Format(time.RFC3339) + _, err := d.db.Exec(`INSERT INTO update_history + (id, container_id, container_name, image, status, trigger, started_at, completed_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + id, "c-"+id, "container-"+id, "img:latest", "success", "auto", when, when) + if err != nil { + t.Fatalf("seed update_history %s: %v", id, err) + } +} + +// seedBackupRun inserts a run started daysAgo days in the past, with one item. +func seedBackupRun(t *testing.T, d *DB, id string, daysAgo int) { + t.Helper() + when := time.Now().AddDate(0, 0, -daysAgo).UTC().Format(time.RFC3339) + _, err := d.db.Exec(`INSERT INTO backup_runs (id, kind, trigger, status, started_at) + VALUES (?, ?, ?, ?, ?)`, id, "backup", "scheduled", "success", when) + if err != nil { + t.Fatalf("seed backup_runs %s: %v", id, err) + } + _, err = d.db.Exec(`INSERT INTO backup_run_items (id, run_id, stack_id, status) + VALUES (?, ?, ?, ?)`, "item-"+id, id, "stacks~demo:default", "success") + if err != nil { + t.Fatalf("seed backup_run_items for %s: %v", id, err) + } +} + +func countRows(t *testing.T, d *DB, table string) int { + t.Helper() + var n int + if err := d.db.QueryRow("SELECT COUNT(*) FROM " + table).Scan(&n); err != nil { + t.Fatalf("count %s: %v", table, err) + } + return n +} + +func newRetentionTestDB(t *testing.T) *DB { + t.Helper() + db, err := NewWithMigrations(":memory:") + if err != nil { + t.Fatalf("NewWithMigrations: %v", err) + } + t.Cleanup(func() { db.Close() }) + return db +} + +func TestDeleteOldUpdateHistory_RespectsBoundary(t *testing.T) { + db := newRetentionTestDB(t) + seedUpdateHistory(t, db, "ancient", 400) + seedUpdateHistory(t, db, "old", 91) + seedUpdateHistory(t, db, "fresh", 3) + + deleted, err := db.DeleteOldUpdateHistory(90) + if err != nil { + t.Fatalf("DeleteOldUpdateHistory: %v", err) + } + if deleted != 2 { + t.Errorf("deleted %d rows, want the 2 older than 90 days", deleted) + } + if got := countRows(t, db, "update_history"); got != 1 { + t.Errorf("update_history has %d rows, want only the fresh one", got) + } +} + +func TestDeleteOldBackupRuns_RespectsBoundary(t *testing.T) { + db := newRetentionTestDB(t) + seedBackupRun(t, db, "ancient", 400) + seedBackupRun(t, db, "old", 91) + seedBackupRun(t, db, "fresh", 3) + + deleted, err := db.DeleteOldBackupRuns(90) + if err != nil { + t.Fatalf("DeleteOldBackupRuns: %v", err) + } + if deleted != 2 { + t.Errorf("deleted %d runs, want the 2 older than 90 days", deleted) + } + if got := countRows(t, db, "backup_runs"); got != 1 { + t.Errorf("backup_runs has %d rows, want only the fresh one", got) + } +} + +// TestDeleteOldBackupRuns_CascadesToItems verifies rather than assumes the +// cascade: backup_run_items declares ON DELETE CASCADE, but that only takes +// effect when foreign_keys enforcement is genuinely on for the connection +// (see agent-os-94t). Orphaned items would defeat the whole point of the prune. +func TestDeleteOldBackupRuns_CascadesToItems(t *testing.T) { + db := newRetentionTestDB(t) + seedBackupRun(t, db, "old", 120) + seedBackupRun(t, db, "fresh", 1) + + if got := countRows(t, db, "backup_run_items"); got != 2 { + t.Fatalf("test precondition: expected 2 items, got %d", got) + } + + if _, err := db.DeleteOldBackupRuns(90); err != nil { + t.Fatalf("DeleteOldBackupRuns: %v", err) + } + + if got := countRows(t, db, "backup_run_items"); got != 1 { + t.Errorf("backup_run_items has %d rows, want 1 — the old run's item was orphaned", got) + } + + var orphans int + err := db.db.QueryRow(`SELECT COUNT(*) FROM backup_run_items i + WHERE NOT EXISTS (SELECT 1 FROM backup_runs r WHERE r.id = i.run_id)`).Scan(&orphans) + if err != nil { + t.Fatalf("orphan check: %v", err) + } + if orphans != 0 { + t.Errorf("%d backup_run_items rows have no parent run", orphans) + } +} + +// TestRetentionDays_FloorIsClamped mirrors the action_log behaviour: an absurdly +// low setting must not let a prune wipe the history the operator is looking at. +func TestRetentionDays_FloorIsClamped(t *testing.T) { + cases := []struct { + setting string + want int + }{ + {"", DefaultRetentionDays}, + {"0", MinRetentionDays}, + {"1", MinRetentionDays}, + {"-30", MinRetentionDays}, + {"not-a-number", DefaultRetentionDays}, + {"30", 30}, + {fmt.Sprint(MinRetentionDays), MinRetentionDays}, + } + + db := newRetentionTestDB(t) + for _, tc := range cases { + t.Run("setting="+tc.setting, func(t *testing.T) { + if err := db.SetSetting("test_retention_key", tc.setting); err != nil { + t.Fatalf("SetSetting: %v", err) + } + if got := db.RetentionDays("test_retention_key"); got != tc.want { + t.Errorf("RetentionDays(%q) = %d, want %d", tc.setting, got, tc.want) + } + }) + } +} + +// TestRetentionDays_UnsetKeyUsesDefault covers a fresh install where the +// migration's INSERT OR IGNORE has not seeded the key. +func TestRetentionDays_UnsetKeyUsesDefault(t *testing.T) { + db := newRetentionTestDB(t) + if got := db.RetentionDays("key_that_does_not_exist"); got != DefaultRetentionDays { + t.Errorf("RetentionDays for a missing key = %d, want %d", got, DefaultRetentionDays) + } +} + +// TestPruneHistory_PrunesEveryTable covers the pass the daily ticker runs: one +// table failing must not skip the others, and all three settings are honoured. +func TestPruneHistory_PrunesEveryTable(t *testing.T) { + db := newRetentionTestDB(t) + + seedUpdateHistory(t, db, "old", 200) + seedUpdateHistory(t, db, "fresh", 1) + seedBackupRun(t, db, "old", 200) + seedBackupRun(t, db, "fresh", 1) + if _, err := db.db.Exec( + `INSERT INTO action_log (id, user_id, action, detail, created_at) + VALUES ('old', 'u', 'login', '{}', datetime('now', '-200 days')), + ('fresh', 'u', 'login', '{}', datetime('now'))`); err != nil { + t.Fatalf("seed action_log: %v", err) + } + + result := db.PruneHistory() + + if result.UpdateHistory != 1 || result.BackupRuns != 1 { + t.Errorf("PruneHistory reported %+v, want 1 of each", result) + } + for table, want := range map[string]int{ + "update_history": 1, + "backup_runs": 1, + "backup_run_items": 1, + "action_log": 1, + } { + if got := countRows(t, db, table); got != want { + t.Errorf("%s has %d rows after the pass, want %d", table, got, want) + } + } +} + +// TestPruneHistory_HonoursConfiguredRetention proves the pass reads the +// settings rather than a hardcoded 90 days. +func TestPruneHistory_HonoursConfiguredRetention(t *testing.T) { + db := newRetentionTestDB(t) + seedUpdateHistory(t, db, "twenty-days", 20) + seedBackupRun(t, db, "twenty-days", 20) + + // Default is 90 days, so nothing should go yet. + if r := db.PruneHistory(); r.UpdateHistory != 0 || r.BackupRuns != 0 { + t.Fatalf("default retention deleted %+v, want nothing", r) + } + + if err := db.SetSetting(SettingUpdateHistoryRetentionDays, "10"); err != nil { + t.Fatalf("SetSetting: %v", err) + } + if err := db.SetSetting(SettingBackupHistoryRetentionDays, "10"); err != nil { + t.Fatalf("SetSetting: %v", err) + } + + if r := db.PruneHistory(); r.UpdateHistory != 1 || r.BackupRuns != 1 { + t.Errorf("with retention 10 days the pass deleted %+v, want 1 of each", r) + } +} diff --git a/backend/internal/handlers/settings.go b/backend/internal/handlers/settings.go index bbb0790..fc49033 100644 --- a/backend/internal/handlers/settings.go +++ b/backend/internal/handlers/settings.go @@ -315,57 +315,82 @@ func parseEnvFile(path string) ([]map[string]string, error) { } func (h *SettingsHandler) GetLogRetention(c *gin.Context) { - retentionStr, err := h.db.GetSetting("max_log_retention_days") - if err != nil { - c.JSON(http.StatusInternalServerError, models.NewAppError( - http.StatusInternalServerError, - "INTERNAL_ERROR", - "Failed to get log retention setting", - )) - return - } - - retentionDays := 90 - if retentionStr != "" { - if _, err := fmt.Sscanf(retentionStr, "%d", &retentionDays); err != nil { - slog.Error("Failed to parse log retention days", "error", err) - retentionDays = 90 - } - } - + // All three histories are read through the same clamped accessor, so the + // UI shows the value that will actually be applied rather than the raw row. c.JSON(http.StatusOK, gin.H{ - "retentionDays": retentionDays, + "retentionDays": h.db.RetentionDays(database.SettingLogRetentionDays), + "updateHistoryRetentionDays": h.db.RetentionDays(database.SettingUpdateHistoryRetentionDays), + "backupHistoryRetentionDays": h.db.RetentionDays(database.SettingBackupHistoryRetentionDays), + "minRetentionDays": database.MinRetentionDays, }) } func (h *SettingsHandler) UpdateLogRetention(c *gin.Context) { + // All three fields are optional so a client can update one without having + // to know the others; at least one must be present. var req struct { - RetentionDays int `json:"retentionDays" binding:"required,min=7"` + RetentionDays *int `json:"retentionDays"` + UpdateHistoryRetentionDays *int `json:"updateHistoryRetentionDays"` + BackupHistoryRetentionDays *int `json:"backupHistoryRetentionDays"` } if err := c.ShouldBindJSON(&req); err != nil { c.JSON(http.StatusBadRequest, models.NewAppError( http.StatusBadRequest, "VALIDATION_ERROR", - "Retention days must be at least 7", + "Invalid request body", )) return } - if err := h.db.SetSetting("max_log_retention_days", fmt.Sprintf("%d", req.RetentionDays)); err != nil { - slog.Error("Failed to update log retention setting", "error", err) - c.JSON(http.StatusInternalServerError, models.NewAppError( - http.StatusInternalServerError, - "INTERNAL_ERROR", - "Failed to update log retention setting", + updates := []struct { + value *int + key string + label string + }{ + {req.RetentionDays, database.SettingLogRetentionDays, "log_retention"}, + {req.UpdateHistoryRetentionDays, database.SettingUpdateHistoryRetentionDays, "update_history_retention"}, + {req.BackupHistoryRetentionDays, database.SettingBackupHistoryRetentionDays, "backup_history_retention"}, + } + + applied := gin.H{} + for _, u := range updates { + if u.value == nil { + continue + } + if *u.value < database.MinRetentionDays { + c.JSON(http.StatusBadRequest, models.NewAppError( + http.StatusBadRequest, + "VALIDATION_ERROR", + fmt.Sprintf("Retention days must be at least %d", database.MinRetentionDays), + )) + return + } + if err := h.db.SetSetting(u.key, strconv.Itoa(*u.value)); err != nil { + slog.Error("Failed to update retention setting", "setting", u.label, "error", err) + c.JSON(http.StatusInternalServerError, models.NewAppError( + http.StatusInternalServerError, + "INTERNAL_ERROR", + "Failed to update retention setting", + )) + return + } + applied[u.label] = *u.value + } + + if len(applied) == 0 { + c.JSON(http.StatusBadRequest, models.NewAppError( + http.StatusBadRequest, + "VALIDATION_ERROR", + "At least one retention value is required", )) return } - slog.Info("Log retention updated", "retention_days", req.RetentionDays) + slog.Info("Retention settings updated", "applied", applied) logActionFromContext(h.actionLog, c, nil, services.ActionUpdateSettings, gin.H{ - "setting": "log_retention", - "retention_days": req.RetentionDays, + "setting": "retention", + "applied": applied, }) c.Status(http.StatusNoContent) } diff --git a/backend/internal/handlers/settings_retention_test.go b/backend/internal/handlers/settings_retention_test.go new file mode 100644 index 0000000..fbe15e8 --- /dev/null +++ b/backend/internal/handlers/settings_retention_test.go @@ -0,0 +1,117 @@ +package handlers + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/thinkbig1979/capstan/backend/internal/database" +) + +// The retention endpoint used to govern action_log alone. update_history and +// backup_runs grew without bound (agent-os-0jp), so it now carries all three. + +func newRetentionRouter(t *testing.T) (*database.DB, http.Handler) { + t.Helper() + db, err := database.NewWithMigrations(t.TempDir()) + require.NoError(t, err) + t.Cleanup(func() { db.Close() }) + + handler := NewSettingsHandler(db, "", "test-secret", false, nil, nil) + return db, setupSettingsRouter(handler) +} + +func putRetention(t *testing.T, router http.Handler, body string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodPut, "/settings/log-retention", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + return w +} + +func TestGetLogRetention_ReturnsEveryHistoryTable(t *testing.T) { + db, router := newRetentionRouter(t) + require.NoError(t, db.SetSetting(database.SettingUpdateHistoryRetentionDays, "45")) + require.NoError(t, db.SetSetting(database.SettingBackupHistoryRetentionDays, "30")) + + req := httptest.NewRequest(http.MethodGet, "/settings/log-retention", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + require.Equal(t, http.StatusOK, w.Code) + var body map[string]int + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) + + assert.Equal(t, 90, body["retentionDays"], "audit log default") + assert.Equal(t, 45, body["updateHistoryRetentionDays"]) + assert.Equal(t, 30, body["backupHistoryRetentionDays"]) + assert.Equal(t, database.MinRetentionDays, body["minRetentionDays"]) +} + +// TestGetLogRetention_ClampsStoredValue: a row edited by hand below the floor +// must still report the value that will actually be applied. +func TestGetLogRetention_ClampsStoredValue(t *testing.T) { + db, router := newRetentionRouter(t) + require.NoError(t, db.SetSetting(database.SettingUpdateHistoryRetentionDays, "1")) + + req := httptest.NewRequest(http.MethodGet, "/settings/log-retention", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + var body map[string]int + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) + assert.Equal(t, database.MinRetentionDays, body["updateHistoryRetentionDays"]) +} + +func TestUpdateLogRetention_AcceptsPartialUpdate(t *testing.T) { + db, router := newRetentionRouter(t) + require.NoError(t, db.SetSetting(database.SettingLogRetentionDays, "90")) + + w := putRetention(t, router, `{"updateHistoryRetentionDays": 60}`) + assert.Equal(t, http.StatusNoContent, w.Code) + + value, err := db.GetSetting(database.SettingUpdateHistoryRetentionDays) + require.NoError(t, err) + assert.Equal(t, "60", value) + + // The untouched setting is left alone rather than reset to a default. + value, err = db.GetSetting(database.SettingLogRetentionDays) + require.NoError(t, err) + assert.Equal(t, "90", value) +} + +func TestUpdateLogRetention_RejectsBelowFloorOnEveryField(t *testing.T) { + for _, field := range []string{ + "retentionDays", "updateHistoryRetentionDays", "backupHistoryRetentionDays", + } { + t.Run(field, func(t *testing.T) { + db, router := newRetentionRouter(t) + + w := putRetention(t, router, `{"`+field+`": 1}`) + assert.Equal(t, http.StatusBadRequest, w.Code) + + // Nothing was written, so a rejected request cannot half-apply. + for _, key := range []string{ + database.SettingUpdateHistoryRetentionDays, + database.SettingBackupHistoryRetentionDays, + } { + value, err := db.GetSetting(key) + require.NoError(t, err) + assert.Equal(t, "90", value, "%s should be untouched", key) + } + }) + } +} + +func TestUpdateLogRetention_RejectsEmptyBody(t *testing.T) { + _, router := newRetentionRouter(t) + + w := putRetention(t, router, `{}`) + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "At least one retention value is required") +} diff --git a/frontend/src/components/settings/AuditLogContent.tsx b/frontend/src/components/settings/AuditLogContent.tsx index 6cc91c9..a811126 100644 --- a/frontend/src/components/settings/AuditLogContent.tsx +++ b/frontend/src/components/settings/AuditLogContent.tsx @@ -12,6 +12,7 @@ import { LoadingSpinner } from '@/components/LoadingSkeleton' import { HelpHint } from '@/components/ui/help-hint' import { ChevronLeft, ChevronRight, ChevronDown, ScrollText, X } from 'lucide-react' import { queryKeys } from '@/lib/query-keys' +import { HistoryRetentionSection } from './HistoryRetentionSection' const ALL_ACTIONS = '__all__' @@ -65,7 +66,7 @@ function AuditedEventsNote() { ) } -export function AuditLogContent() { +function AuditLogTable() { const [page, setPage] = useState(1) const [expanded, setExpanded] = useState>(new Set()) const pageSize = 50 @@ -309,3 +310,14 @@ export function AuditLogContent() { ) } + +/** The audit log itself, plus the retention controls that govern how long it + * and the other history tables are kept. */ +export function AuditLogContent() { + return ( +
+ + +
+ ) +} diff --git a/frontend/src/components/settings/HistoryRetentionSection.tsx b/frontend/src/components/settings/HistoryRetentionSection.tsx new file mode 100644 index 0000000..418f921 --- /dev/null +++ b/frontend/src/components/settings/HistoryRetentionSection.tsx @@ -0,0 +1,113 @@ +import { useState } from 'react' +import { toast } from 'sonner' +import { Button } from '@/components/ui/button' +import { Input } from '@/components/ui/input' +import { Label } from '@/components/ui/label' +import { LoadingSpinner } from '@/components/LoadingSkeleton' +import { HelpHint } from '@/components/ui/help-hint' +import { useRetentionSettings, useUpdateRetentionSettings } from '@/hooks/useResources' + +type FieldKey = 'retentionDays' | 'updateHistoryRetentionDays' | 'backupHistoryRetentionDays' + +const FIELDS: { key: FieldKey; label: string; id: string; hint: string }[] = [ + { + key: 'retentionDays', + label: 'Audit log', + id: 'retention-audit-log', + hint: 'Who did what, and when.', + }, + { + key: 'updateHistoryRetentionDays', + label: 'Update history', + id: 'retention-update-history', + hint: 'One row per container image update.', + }, + { + key: 'backupHistoryRetentionDays', + label: 'Backup history', + id: 'retention-backup-history', + hint: 'One row per backup run, plus one per stack in it.', + }, +] + +/** History retention for the three tables that are pruned on a daily pass. + * Before this existed the retention endpoint had no caller in the UI at all, + * so the only way to change it was to edit the settings row by hand. */ +export function HistoryRetentionSection() { + const { data, isLoading } = useRetentionSettings() + const updateRetention = useUpdateRetentionSettings() + const [draft, setDraft] = useState>>({}) + + if (isLoading) { + return
+ } + + const min = data?.minRetentionDays ?? 7 + const valueOf = (key: FieldKey) => draft[key] ?? data?.[key] ?? 90 + const dirty = FIELDS.some(({ key }) => draft[key] !== undefined && draft[key] !== data?.[key]) + const belowFloor = FIELDS.some(({ key }) => valueOf(key) < min) + + const handleSave = () => { + const payload: Partial> = {} + for (const { key } of FIELDS) { + if (draft[key] !== undefined && draft[key] !== data?.[key]) payload[key] = draft[key] + } + updateRetention.mutate(payload, { + onSuccess: () => { + toast.success('Retention updated') + setDraft({}) + }, + onError: () => toast.error('Failed to update retention'), + }) + } + + return ( +
{ e.preventDefault(); handleSave() }} + className="space-y-4 pt-4 border-t" + > +
+

History retention

+ +

+ How many days of history to keep. A cleanup pass runs shortly after startup and + once a day after that, deleting anything older. +

+

+ Removing a backup run also removes its per-stack rows. This prunes Capstan's + own records only — it never touches your restic snapshots. +

+

Minimum {min} days. Deleted history cannot be recovered.

+
+
+ +
+ {FIELDS.map(({ key, label, id, hint }) => ( +
+ + + setDraft((d) => ({ ...d, [key]: parseInt(e.target.value, 10) || 0 })) + } + /> +

{hint}

+
+ ))} +
+ + {belowFloor && ( +

+ Retention must be at least {min} days. +

+ )} + + +
+ ) +} diff --git a/frontend/src/components/settings/__tests__/HistoryRetentionSection.test.tsx b/frontend/src/components/settings/__tests__/HistoryRetentionSection.test.tsx new file mode 100644 index 0000000..a5cc3fb --- /dev/null +++ b/frontend/src/components/settings/__tests__/HistoryRetentionSection.test.tsx @@ -0,0 +1,103 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { render, screen, fireEvent, waitFor } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import type { ReactNode } from 'react' +import { HistoryRetentionSection } from '../HistoryRetentionSection' + +// update_history and backup_runs previously grew without bound, and the +// retention endpoint had no caller in the UI at all (agent-os-0jp). + +const mockGetRetention = vi.fn() +const mockUpdateRetention = vi.fn() + +vi.mock('@/lib/api', () => ({ + settingsApi: { + getRetention: (...args: unknown[]) => mockGetRetention(...args), + updateRetention: (...args: unknown[]) => mockUpdateRetention(...args), + }, +})) + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), warning: vi.fn() }, +})) + +function createWrapper() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }) + return ({ children }: { children: ReactNode }) => ( + {children} + ) +} + +const SETTINGS = { + retentionDays: 90, + updateHistoryRetentionDays: 45, + backupHistoryRetentionDays: 30, + minRetentionDays: 7, +} + +function renderSection() { + return render(, { wrapper: createWrapper() }) +} + +describe('HistoryRetentionSection', () => { + beforeEach(() => { + vi.clearAllMocks() + mockGetRetention.mockResolvedValue(SETTINGS) + mockUpdateRetention.mockResolvedValue(undefined) + }) + + it('shows the configured retention for all three history tables', async () => { + renderSection() + + expect(await screen.findByLabelText('Audit log')).toHaveValue(90) + expect(screen.getByLabelText('Update history')).toHaveValue(45) + expect(screen.getByLabelText('Backup history')).toHaveValue(30) + }) + + it('sends only the fields that changed', async () => { + renderSection() + + const updateHistory = await screen.findByLabelText('Update history') + fireEvent.change(updateHistory, { target: { value: '60' } }) + fireEvent.click(screen.getByRole('button', { name: /save retention/i })) + + await waitFor(() => expect(mockUpdateRetention).toHaveBeenCalledTimes(1)) + expect(mockUpdateRetention).toHaveBeenCalledWith({ updateHistoryRetentionDays: 60 }) + }) + + it('disables saving until something changes', async () => { + renderSection() + + const save = await screen.findByRole('button', { name: /save retention/i }) + expect(save).toBeDisabled() + + fireEvent.change(screen.getByLabelText('Audit log'), { target: { value: '120' } }) + expect(save).toBeEnabled() + }) + + // A prune is irreversible, so the floor is enforced server-side too. The UI + // blocks the request rather than letting the operator get a 400 back. + it('refuses to submit a value below the server floor', async () => { + renderSection() + + const auditLog = await screen.findByLabelText('Audit log') + fireEvent.change(auditLog, { target: { value: '2' } }) + + expect(screen.getByRole('button', { name: /save retention/i })).toBeDisabled() + expect(screen.getByText(/at least 7 days/i)).toBeInTheDocument() + expect(mockUpdateRetention).not.toHaveBeenCalled() + }) + + it('surfaces a failed save', async () => { + const { toast } = await import('sonner') + mockUpdateRetention.mockRejectedValue(new Error('boom')) + renderSection() + + fireEvent.change(await screen.findByLabelText('Audit log'), { target: { value: '120' } }) + fireEvent.click(screen.getByRole('button', { name: /save retention/i })) + + await waitFor(() => expect(toast.error).toHaveBeenCalledWith('Failed to update retention')) + }) +}) diff --git a/frontend/src/hooks/useResources.ts b/frontend/src/hooks/useResources.ts index 8448414..1e2a72a 100644 --- a/frontend/src/hooks/useResources.ts +++ b/frontend/src/hooks/useResources.ts @@ -541,6 +541,28 @@ export function useUpdateGlobalEnv() { }) } +export function useRetentionSettings() { + return useQuery({ + queryKey: queryKeys.settings.retention(), + queryFn: () => settingsApi.getRetention(), + retry: 1, + }) +} + +export function useUpdateRetentionSettings() { + const queryClient = useQueryClient() + return useMutation({ + mutationFn: (data: { + retentionDays?: number + updateHistoryRetentionDays?: number + backupHistoryRetentionDays?: number + }) => settingsApi.updateRetention(data), + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: queryKeys.settings.retention() }) + }, + }) +} + export function useUpdateGitSettings() { const queryClient = useQueryClient() return useMutation({ diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index fb9312d..8df43d1 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -18,6 +18,7 @@ import type { UpdateHistoryEntry, AutoUpdatePolicy, UpdateSettings, + RetentionSettings, UpdateHistoryFilters, GitStatus, GitCommit, @@ -181,6 +182,15 @@ export const settingsApi = { return response.data }, + getRetention: async () => { + const response = await apiClient.get('/settings/log-retention') + return response.data + }, + + updateRetention: async (data: Partial>) => { + await apiClient.put('/settings/log-retention', data) + }, + getAuditLog: async ( page = 1, pageSize = 50, diff --git a/frontend/src/lib/query-keys.ts b/frontend/src/lib/query-keys.ts index 4fb4396..8ddcd91 100644 --- a/frontend/src/lib/query-keys.ts +++ b/frontend/src/lib/query-keys.ts @@ -82,6 +82,7 @@ export const queryKeys = { updates: () => ['settings', 'updates'] as const, git: () => ['settings', 'git'] as const, globalEnv: () => ['settings', 'global-env'] as const, + retention: () => ['settings', 'retention'] as const, }, /** Git state for a stack. `all` prefix-matches log and diff. */ diff --git a/frontend/src/types/index.ts b/frontend/src/types/index.ts index ccc29ce..ee77bc1 100644 --- a/frontend/src/types/index.ts +++ b/frontend/src/types/index.ts @@ -296,6 +296,15 @@ export interface AutoUpdatePolicy { updatedAt: string } +/** How long each history table is kept, in days. `minRetentionDays` is the + * server-enforced floor — a prune is irreversible, so the API rejects less. */ +export interface RetentionSettings { + retentionDays: number + updateHistoryRetentionDays: number + backupHistoryRetentionDays: number + minRetentionDays: number +} + export interface UpdateSettings { scanIntervalMinutes: number lastScanAt: string | null