From ed8125ec4b07d2c3e83bc775ffcc57a03228cfc9 Mon Sep 17 00:00:00 2001 From: Edwin <18327273+thinkbig1979@users.noreply.github.com> Date: Fri, 31 Jul 2026 21:31:53 +0200 Subject: [PATCH] feat(retention): bound update_history and backup_runs, surface retention in the UI Three tables grew without bound. Only sessions (hourly) and action_log (daily) were ever pruned; cached_updates is replaced wholesale on each scan so it is bounded by container count. update_history, backup_runs and backup_run_items were not pruned at all -- one row per image update, one per backup run, and one per stack per run, kept forever. The growth is slow, silent, and only shows up on exactly the deployments that matter: long-lived ones with automation switched on. Nothing surfaced it -- no size metric, no warning, no bound. The consequences arrive in the order that makes them hardest to diagnose: history views get slow, then the SQLite file gets awkward, then -- now that capstan.db is itself in the backup set (agent-os-36o) -- every backup run drags an ever-growing database with it. internal/database/retention.go holds one retention concept for all four history tables: DeleteOldUpdateHistory and DeleteOldBackupRuns alongside the existing DeleteOldActionLogs, a RetentionDays accessor that defaults to 90 and clamps to a floor of 7, and PruneHistory running every policy in one pass. Each table is pruned independently, since one failing must not skip the others. The floor lives in code rather than the schema so editing the settings row by hand cannot bypass it either -- a prune is irreversible. main.go's daily ticker now calls PruneHistory, replacing twenty lines of inline parsing, and a startup pass runs at T+2min. The ticker's first fire is 24 hours away, so an instance that restarts daily -- a container on a nightly compose pull -- never reached it and the cleanup never ran at all. Migration 11 seeds the two new settings keys and indexes update_history.completed_at, previously reachable only by full scan. Two corrections to the issue as filed: DeleteUpdateHistoryOlderThan is described as having no callers at all. It has one -- handlers/updates.go:731, a manual "clear history older than" endpoint. Nothing invoked it automatically, which is what this fixes; the manual endpoint is untouched. The new controls were to go "next to the existing log retention control". There is no existing control: grep for log-retention across frontend/src returns nothing, so the endpoint had no caller outside backend tests and the only way to change retention was to edit the settings row by hand. This adds a History retention section to Settings -> Audit Log covering all three, which is the first time any of it is reachable from the UI. The endpoint now takes all three fields as optional pointers so a client can update one without knowing the others; retentionDays alone still behaves exactly as before and the pre-existing handler tests pass unchanged. Verified with the manual check the issue asks for -- backdate rows, restart, confirm the startup pass removes them and logs how many: SEEDED (update_history, backup_runs, backup_run_items, action_log): (2,2,2,1) level=INFO msg="History retention pass complete" update_history_deleted=1 backup_runs_deleted=1 AFTER: (1, 1, 1, 0) SURVIVORS: [u-new, b-new, i-new] backup_run_items going 2 -> 1 with no explicit delete is the cascade working; it is covered by a test rather than assumed, because ON DELETE CASCADE only fires when foreign_keys enforcement is genuinely on for the connection (agent-os-94t). The frontend floor test was confirmed to fail with the guard removed. agent-os-0jp --- backend/cmd/server/main.go | 30 +-- backend/internal/database/migrations.go | 15 ++ backend/internal/database/retention.go | 135 +++++++++++ backend/internal/database/retention_test.go | 223 ++++++++++++++++++ backend/internal/handlers/settings.go | 85 ++++--- .../handlers/settings_retention_test.go | 117 +++++++++ .../components/settings/AuditLogContent.tsx | 14 +- .../settings/HistoryRetentionSection.tsx | 113 +++++++++ .../HistoryRetentionSection.test.tsx | 103 ++++++++ frontend/src/hooks/useResources.ts | 22 ++ frontend/src/lib/api.ts | 10 + frontend/src/lib/query-keys.ts | 1 + frontend/src/types/index.ts | 9 + 13 files changed, 826 insertions(+), 51 deletions(-) create mode 100644 backend/internal/database/retention.go create mode 100644 backend/internal/database/retention_test.go create mode 100644 backend/internal/handlers/settings_retention_test.go create mode 100644 frontend/src/components/settings/HistoryRetentionSection.tsx create mode 100644 frontend/src/components/settings/__tests__/HistoryRetentionSection.test.tsx 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