Bound the force-plan bot's decision journal with a retention horizon

Darling/Darling.Tests/DarlingRetentionTests.cs

@@ -141,6 +141,58 @@ anchoring a slice on an ineligible row would delete nothing and terminate the dr
         Assert.Equal("status IN ('succeeded', 'failed')", DarlingRetention.TerminalCommandStatuses);
     }
 
+    [Fact]
+    public void PlanForceLedgerRetention_IsAYear_AndIsTheLongestHorizonInTheStore()
+    {
+        /* collect.plan_force_actions (the force-plan bot's decision journal) is not a collector and not a
+           hypertable, so it purges through the same batched DELETE builder as config_alert_log, on its own
+           action_time column. A year: it is the audit trail of a bot WRITING to production servers, so it
+           has to outlive the metrics that motivated each decision by enough that "why did this plan change"
+           is still answerable releases later. */
+        Assert.Equal(365, DarlingRetention.PlanForceLedgerRetentionDays);
+
+        /* The LONGEST horizon in the store, deliberately — strictly greater than every sibling, including
+           the alert log's already-generous 90 days. This is the assertion that catches a copy-paste of a
+           shorter sibling's constant into the purge call. */
+        Assert.True(
+            DarlingRetention.PlanForceLedgerRetentionDays > DarlingRetention.AlertHistoryRetentionDays,
+            "the plan-force ledger must outlive alert history");
+        Assert.True(
+            DarlingRetention.PlanForceLedgerRetentionDays > DarlingRetention.CollectionLogRetentionDays,
+            "the plan-force ledger must outlive the collection log");
+        Assert.True(
+            DarlingRetention.PlanForceLedgerRetentionDays > DarlingRetention.CommandHistoryRetentionDays,
+            "the plan-force ledger must outlive command history");
+
+        /* SCHEMA-QUALIFIED to match the V107 DDL and PgPlanForceActionStore, which both name
+           collect.plan_force_actions explicitly. No extra predicate: unlike config.config_command every row
+           here is eligible once it is past the horizon — the journal is append-only, so there is no live-row
+           state a purge could strand. */
+        Assert.Equal(
+            "DELETE FROM collect.plan_force_actions WHERE action_time < $1"
+            + " AND action_time >= (SELECT min(action_time) FROM collect.plan_force_actions WHERE action_time < $1)"
+            + " AND action_time < (SELECT min(action_time) FROM collect.plan_force_actions WHERE action_time < $1) + INTERVAL '1 days'",
+            DarlingRetention.TimeSlicedDeleteSql("collect.plan_force_actions", "action_time"));
+    }
+
+    [Fact]
+    public void ThePlanForceLedger_IsNotACollectorTable_SoTheCatalogLoopCannotReachIt()
+    {
+        /* Why the purge needs its own explicit block rather than an entry in the shared catalog: the journal
+           is written by the service's post-analysis bot pass, not by a collector, so nothing in
+           CollectorCatalog.All names it and the catalog-driven loop skips it entirely. If it ever DOES gain a
+           catalog entry, this test fails and the explicit block becomes a double-purge to delete. */
+        Assert.DoesNotContain(
+            CollectorCatalog.All,
+            d => d.TargetTable.Contains("plan_force_actions", StringComparison.OrdinalIgnoreCase));
+
+        /* Positive control for that negative: the same predicate DOES find a real collector table, so the
+           assertion above is a fact about the catalog rather than a mis-spelled probe that can never match. */
+        Assert.Contains(
+            CollectorCatalog.All,
+            d => d.TargetTable.Equals("file_io_stats", StringComparison.OrdinalIgnoreCase));
+    }
+
     [Fact]
     public void TimeSlicedDelete_WithoutAnExtraPredicate_IsUnchanged()
     {
@@ -409,6 +461,32 @@ the two NOT NULL value columns are required (the rest of the V3 columns default)
                 await insert.ExecuteNonQueryAsync(ct);
             }
 
+            /* collect.plan_force_actions (the force-plan bot's decision journal) purges on its own 365-day
+               horizon. The two ages are chosen to DISCRIMINATE that horizon rather than merely exercise it: a
+               400-day row is past it and goes, and a 100-day row SURVIVES even though it is past every other
+               horizon in the store (90-day alert history, 60-day collection_log, 30-day base). So this fails
+               if the purge is wired to a shorter sibling's constant by copy-paste, and it fails if the table
+               is not purged at all. Only the NOT NULL columns without defaults are supplied; action_id is
+               GENERATED ALWAYS AS IDENTITY, so it is never written. */
+            foreach (var (ageDays, decision) in new[] { (400, "would_force"), (100, "blocked") })
+            {
+                using var insert = new NpgsqlCommand(
+                    "INSERT INTO collect.plan_force_actions"
+                    + " (action_time, server_id, server_name, database_name, query_id, plan_id, action, mode, decision, outcome)"
+                    + " VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)", connection);
+                insert.Parameters.AddWithValue(utcNow.AddDays(-ageDays));
+                insert.Parameters.AddWithValue(TestServerId);
+                insert.Parameters.AddWithValue("retention-e2e");
+                insert.Parameters.AddWithValue("retention_e2e_db");
+                insert.Parameters.AddWithValue(1L);
+                insert.Parameters.AddWithValue(2L);
+                insert.Parameters.AddWithValue("force");
+                insert.Parameters.AddWithValue("dry_run");
+                insert.Parameters.AddWithValue(decision);
+                insert.Parameters.AddWithValue("journaled");
+                await insert.ExecuteNonQueryAsync(ct);
+            }
+
             /* At least our three expired rows go (40-day wait_stats, 70-day collection_log, 100-day
                config_alert_log); a shared dev store may shed more. The extension-free DELETE path on purpose
                (timescaleAvailable: false) — it must keep working even on a store whose tables ARE hypertables,
@@ -477,6 +555,20 @@ the two NOT NULL value columns are required (the rest of the V3 columns default)
                 Assert.DoesNotContain(survivors, s => s.Status == "failed");
             }
 
+            using (var read = new NpgsqlCommand(
+                "SELECT action_time FROM collect.plan_force_actions WHERE server_id = $1", connection))
+            {
+                read.Parameters.AddWithValue(TestServerId);
+                using var reader = await read.ExecuteReaderAsync(ct);
+                Assert.True(await reader.ReadAsync(ct),
+                    $"the 100-day plan_force_actions row (inside the 365-day horizon) did not survive the purge; {purgeLog.Joined}");
+                var survivor = reader.GetDateTime(0);
+                Assert.True(survivor < utcNow.AddDays(-99) && survivor > utcNow.AddDays(-101),
+                    $"the surviving journal row should be the 100-day one, got {survivor:O}; {purgeLog.Joined}");
+                Assert.False(await reader.ReadAsync(ct),
+                    $"the 400-day plan_force_actions row survived past the 365-day horizon; {purgeLog.Joined}");
+            }
+
             /* The purge writes ONE auditable run-record under the fleet sentinel server_id — SUCCESS here
                (every table purged cleanly on this store). Never attributed to a real monitored server. */
             using (var read = new NpgsqlCommand(
@@ -644,6 +736,7 @@ private static async Task DeleteTestRowsAsync(NpgsqlConnection connection, Cance
             $"DELETE FROM collection_log WHERE server_id = {TestServerId}; " +
             $"DELETE FROM config_alert_log WHERE server_id = {TestServerId}; " +
             $"DELETE FROM config.config_command WHERE target_server_id = {TestServerId}; " +
+            $"DELETE FROM collect.plan_force_actions WHERE server_id = {TestServerId}; " +
             $"DELETE FROM collection_log WHERE server_id = {DarlingObservability.FleetServerId} AND collector_name = 'data_retention';",
             connection);
         await cleanup.ExecuteNonQueryAsync(ct);

Darling/PerformanceMonitor.Darling.Service/DarlingRetention.cs

@@ -28,8 +28,9 @@ namespace PerformanceMonitor.Darling.Service;
 /// purge via hypertable <c>drop_chunks</c> instead, which detaches whole expired chunks in O(1)
 /// instead of scanning rows. collection_log — a hypertable since V23, though converted directly by the
 /// V23 migration rather than the catalog loop — purges the SAME way (drop_chunks with a DELETE fallback for
-/// a plain-PostgreSQL store). config_alert_log and config.config_command stay DELETE-based either way
-/// (never converted — plain config-side registry tables), as do the analysis tables
+/// a plain-PostgreSQL store). config_alert_log, config.config_command and collect.plan_force_actions stay
+/// DELETE-based either way (never converted — the first two are plain config-side registry tables, and the
+/// journal keeps an identity PRIMARY KEY its own rows reference), as do the analysis tables
 /// (PgFindingStore.CleanupOldFindingsAsync owns those). Retention horizons are the shared
 /// per-collector <see cref="CollectorScheduleDefaults"/> (identity-pinned to Lite's
 /// ScheduleManager table), so both SKUs keep the same data horizons out of the box. NOTE: Lite
@@ -97,6 +98,26 @@ public static class DarlingRetention
     /// </summary>
     internal const int CommandHistoryRetentionDays = DataRetentionBaseDays;
 
+    /// <summary>
+    /// collect.plan_force_actions (the auto force-plan bot's decision journal) keeps its rows this long. Like
+    /// config_alert_log it is neither a collector (no <see cref="CollectorScheduleDefaults"/> horizon) nor a
+    /// hypertable, and it is APPEND-ONLY with no other purge path — so this horizon is the only thing bounding
+    /// it. The bot's own cooldowns bound the arrival RATE, which is not a size bound: a bounded rate over
+    /// unbounded time is unbounded.
+    /// <para>A full year — deliberately the longest horizon in the store, because this is the audit trail of a
+    /// bot WRITING to production servers and it has to outlive the metrics that motivated each decision
+    /// (<see cref="DataRetentionBaseDays"/>) by enough that "why did this plan change" is still answerable
+    /// releases later. That is 4x the alert log's already-generous <see cref="AlertHistoryRetentionDays"/> and
+    /// 12x the metric window, and it is nearly free: volume is capped by the bot's per-query cooldown and
+    /// per-server daily force budget, so the ceiling is a few decisions per server per day rather than a
+    /// sample per collection cycle. It also clears, by a wide margin, the longest window the bot itself reads
+    /// back when judging eligibility (a week, for the two-taken-back-forces cooldown), so retention can never
+    /// make the bot forget a decision it is still bound by. Generous, but BOUNDED — a monitoring store is
+    /// sized for rolling windows, and "forever" is not a horizon. No operator setting governs this, so the
+    /// constant is the single source of truth.</para>
+    /// </summary>
+    internal const int PlanForceLedgerRetentionDays = 365;
+
     /// <summary>
     /// The terminal-status filter for the command purge — the two states
     /// <c>ViewerDataService.IsTerminal</c> recognizes, which are also the only two
@@ -116,8 +137,9 @@ keeps a large first purge from ever hitting a timeout at all — a single unboun
     /// <summary>
     /// Purges every collector table past its shared <see cref="CollectorScheduleDefaults"/>
     /// RetentionDays, plus collection_log past <see cref="CollectionLogRetentionDays"/>,
-    /// config_alert_log past <see cref="AlertHistoryRetentionDays"/>, and terminal
-    /// config.config_command rows past <see cref="CommandHistoryRetentionDays"/>.
+    /// config_alert_log past <see cref="AlertHistoryRetentionDays"/>, terminal
+    /// config.config_command rows past <see cref="CommandHistoryRetentionDays"/>, and
+    /// collect.plan_force_actions past <see cref="PlanForceLedgerRetentionDays"/>.
     /// When <paramref name="timescaleAvailable"/> (the worker's startup detection), the
     /// collector tables purge via <c>drop_chunks</c> (<see cref="DropChunksSqlFor"/>) with a
     /// per-table DELETE fallback so a table that failed hypertable conversion still honors its
@@ -579,6 +601,39 @@ every night into a warning nobody reads. Keyed on created_at (NOT NULL, so no ro
                 tablesFailed++;
             }
 
+            /* collect.plan_force_actions (the force-plan bot's decision journal) purges on its own action_time
+               column at PlanForceLedgerRetentionDays — the longest horizon in the store. NOT in
+               CollectorCatalog.All (it is written by the service's post-analysis bot pass, not a collector), so
+               the loop above skips it, and it is append-only with no other purge path, which makes this the
+               only thing bounding it.
+               A batched DELETE, never a hypertable: action_id is a PRIMARY KEY that related_action_id points
+               back to (a self-review row references the force row it re-judges), and TimescaleSupport already
+               excludes PK-bearing tables for exactly that reason — conversion would reject the key or force it
+               onto the partition column, breaking the self-reference the append-only design is built on.
+               The work bound is the one-day SLICE, not an index seek: idx_plan_force_actions_time leads with
+               server_id and this DELETE has no server_id predicate, so that index cannot be seeked here. Same
+               shape as config_alert_log's purge against idx_config_alert_log_time(server_id, metric_name,
+               alert_time), and adequate for the same reason — the bot's per-query cooldown and per-server
+               daily budget cap arrivals at a few rows per server per day, so there is never much to scan.
+               SCHEMA-QUALIFIED to match the V107 DDL and PgPlanForceActionStore, which both name
+               collect.plan_force_actions explicitly. Unlike config.config_command a bare name would also
+               resolve here (search_path = collect, config, public), but naming the schema keeps the purge and
+               the writer readable against each other.
+               Failure-isolated like every sibling: a failed statement is warned + counted, the sweep goes on. */
+            var forceLedgerDeleted = await PurgeOneAsync(
+                postgres, "collect.plan_force_actions",
+                TimeSlicedDeleteSql("collect.plan_force_actions", "action_time"),
+                utcNow.AddDays(-PlanForceLedgerRetentionDays), logger, cancellationToken);
+            if (forceLedgerDeleted is not null)
+            {
+                tablesPurged++;
+                totalRowsDeleted += forceLedgerDeleted.Value;
+            }
+            else
+            {
+                tablesFailed++;
+            }
+
             var summary = new PurgeSummary(tablesPurged, totalRowsDeleted, totalChunksDropped);
             logger?.LogInformation(
                 "Retention purge: {Tables} table(s) purged, {Rows} row(s) deleted, {Chunks} chunk(s) dropped, {Failed} failed, {ElapsedMs}ms",

Darling/PerformanceMonitor.Darling.Storage/PgMigrations.cs

@@ -2297,9 +2297,14 @@ CREATE INDEX IF NOT EXISTS idx_pg_buffer_usage_time
     /// generator-parity pin applies): it is written by the service's post-analysis bot pass, the
     /// store_metrics/collector_cost pattern. APPEND-ONLY by design — reviews and outcomes are their own rows
     /// pointing back via <c>related_action_id</c>, never UPDATEs, so the trail cannot be rewritten by the
-    /// thing it audits. Deliberately NOT enrolled in retention: an audit of writes to production servers is
-    /// the one series that should outlive the metrics that motivated it, and its volume is bounded by the
-    /// bot's own cooldowns (at most one decision per query per cooldown window).</para>
+    /// thing it audits. Enrolled in retention at the LONGEST horizon in the store
+    /// (<c>DarlingRetention.PlanForceLedgerRetentionDays</c>, a year), because an audit of writes to
+    /// production servers is the one series that should outlive the metrics that motivated it. The bot's own
+    /// cooldowns (at most one decision per query per cooldown window) bound the arrival RATE, which is not a
+    /// size bound — a bounded rate over unbounded time is unbounded — so the horizon is what keeps it
+    /// finite. A batched DELETE on <c>action_time</c>, not <c>drop_chunks</c>: the identity PRIMARY KEY that
+    /// <c>related_action_id</c> points back to is exactly what
+    /// <c>TimescaleSupport</c> excludes PK-bearing tables for.</para>
     ///
     /// <para><c>reasons</c> is a comma-joined text of the named gate/blocker reasons (the same strings the
     /// MCP <c>structured_remediation</c> blockers carry) rather than <c>text[]</c>, so a future Lite twin

Darling/README.md

@@ -862,6 +862,7 @@ A purge runs on the first sweep after startup and then daily, driven by the same
 | 30 days | Most collector tables (wait/query/procedure/Query Store stats, CPU, memory, file I/O, tempdb, perfmon, deadlocks, blocking, sessions, config snapshots), plus `collection_log` and `analysis_findings` |
 | 90 days | `database_size_stats`, `index_object_stats`, `pvs_stats` |
 | 365 days | `server_properties` |
+| 365 days | `collect.plan_force_actions`, the auto force-plan bot's decision journal — a **fixed** horizon (`DarlingRetention.PlanForceLedgerRetentionDays`) rather than a per-collector one, and deliberately the longest in the store: it audits a bot *writing* to production servers rather than measuring them, so it has to outlive the metrics that motivated each decision. Append-only, never a hypertable, so it purges by bounded DELETE |
 
 On plain PostgreSQL the purge is DELETE-based. With TimescaleDB it switches to `drop_chunks` — a metadata-only detach of whole expired chunks (rows inside a partially-expired chunk survive until the whole chunk ages out; up to ~1 day of grace at the 1-day chunk width), with a per-table DELETE fallback for any table that is not a hypertable. Failure-isolated per table: one stuck purge is logged and retried the next day without stopping the sweep.
 

docs/how-collection-works.md

@@ -143,7 +143,7 @@ Three independent mechanisms:
 
 - **Daily service purge** — horizons come from `CollectorScheduleDefaults` (7 days for snapshot-ish collectors, 30 for most, 90 for size/index/PVS, 365 for `server_properties` and `job_history`). With TimescaleDB this is `drop_chunks`, which is metadata-only; without it, a time-sliced `DELETE` that is safe against compressed chunks. Failure-isolated per table, with an auditable run-record under `server_id = 0`.
 - **TimescaleDB retention policies** for the rollup tiers — raw `query_stats` at 4 days, hourly aggregates at 90, daily kept indefinitely. Every policy is created *paused* and arms itself only once it can prove each downstream consumer has already captured the range it would drop. The governing rule, stated in the code: never drop what your consumer has not captured yet.
-- **Bounded deletes** for the non-hypertable tables (alert history at 90 days, terminal commands at 30 — a pending command is never purged at any age).
+- **Bounded deletes** for the non-hypertable tables (alert history at 90 days, terminal commands at 30 — a pending command is never purged at any age, and the force-plan bot's decision journal at 365, the longest horizon in the store because it audits writes to production servers rather than measuring them).
 
 Darling deliberately does not archive before deleting; compression is the archive.