From b936fe1932a8b4acfbb7cf9f45e03ce28815544a Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Tue, 20 Oct 2026 14:39:19 +0100 Subject: [PATCH] chore(config): drop the unread retention.execution_history_days default retention.execution_history_days (90) sat in DefaultSettings with no reader. Execution retention is the constant DefaultExecutionTTLDays (30), which the cleanup handler passes to CleanupOldExecutions and the plan health lookback reuses. Remove the dead entry and its test row, add a test that keeps it out, and document the real horizon in the config README. No migration: DefaultSettings is an in-memory Go slice that nothing writes to storage, and no table in the schema holds key/value settings, so there is no row on existing installs to delete. Closes #88 Co-Authored-By: Claude Opus 5.5 --- internal/config/README.md | 11 ++++++++++- internal/config/defaults.go | 12 ++++-------- internal/config/defaults_test.go | 13 ++++++++++++- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/internal/config/README.md b/internal/config/README.md index 7959145c3..295998fe4 100644 --- a/internal/config/README.md +++ b/internal/config/README.md @@ -212,9 +212,18 @@ purchaseDefaults := config.GetDefaultsByCategory("purchase_defaults") ### Data Retention - `retention.purchase_history_days`: `1095` (int) - 3 years -- `retention.execution_history_days`: `90` (int) - `retention.recommendation_cache_hours`: `24` (int) +Execution history retention is not a setting. It is the compile-time +constant `DefaultExecutionTTLDays` (`constants.go`, 30 days), which +`internal/server/handler.go` passes to `CleanupOldExecutions` on each +cleanup run. That sweep deletes completed executions by `scheduled_date` +and canceled ones by `updated_at` once they are older than the horizon, +plus rows whose own `expires_at` lapsed that long ago (except statuses +the health score counts). It never deletes failed executions. The plan health score's lookback window +(`planHealthLookbackDays` in `internal/api/plan_health.go`) is the same +constant, so changing one changes both. + ### API Settings - `api.rate_limit_requests_per_minute`: `100` (int) diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 656796fbd..e0849f06b 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -285,14 +285,10 @@ var DefaultSettings = []ConfigSetting{ Description: "Days to retain purchase history (3 years default)", UpdatedAt: time.Time{}, }, - { - Key: "retention.execution_history_days", - Value: 90, - Type: "int", - Category: "retention", - Description: "Days to retain execution history records", - UpdatedAt: time.Time{}, - }, + // Execution history retention is deliberately not a setting: it is + // DefaultExecutionTTLDays (constants.go), passed to CleanupOldExecutions + // by internal/server/handler.go and shared with the plan health + // lookback window (internal/api/plan_health.go). See issue #88. { Key: "retention.recommendation_cache_hours", Value: 24, diff --git a/internal/config/defaults_test.go b/internal/config/defaults_test.go index bcb5da2fe..044186b03 100644 --- a/internal/config/defaults_test.go +++ b/internal/config/defaults_test.go @@ -218,7 +218,6 @@ func TestDefaultSettings_Retention(t *testing.T) { expectedValue int }{ {"retention.purchase_history_days", 1095}, - {"retention.execution_history_days", 90}, {"retention.recommendation_cache_hours", 24}, } @@ -233,6 +232,18 @@ func TestDefaultSettings_Retention(t *testing.T) { } } +// Execution retention is not a setting. It is the compile-time constant +// DefaultExecutionTTLDays, which the cleanup handler passes to +// CleanupOldExecutions and which the plan health lookback window reuses. +// A "retention.execution_history_days" default (90, against a live value +// of 30) once sat here with no reader, and it was cited as the retention +// policy in review (issue #88). Keep it from coming back as an unwired key. +func TestDefaultSettings_NoExecutionRetentionKey(t *testing.T) { + assert.Nil(t, GetDefaultSetting("retention.execution_history_days"), + "execution retention is DefaultExecutionTTLDays; a settings key for it would have no reader") + assert.Equal(t, 30, DefaultExecutionTTLDays) +} + func TestDefaultSettings_API(t *testing.T) { tests := []struct { key string