fix(db): bound submitter_outcome_log and alert_dedup_claims by retention policy (#10106)

Both tables are written per event with no delete path anywhere in src/, the
same class #9473 already bounded: submitter_outcome_log's only reader is
windowed, and alert_dedup_claims is a pure hourly-expiring idempotency claim.
Adds the RETENTION_POLICY entries, their RETENTION_PK_COLUMN/composite-PK
mapping, and the paired leading-column index migration.

Co-authored-by: bitfathers94 <237535319+bitfathers94@users.noreply.github.com>

Author: bitfathers94

migrations/0211_retention_column_indexes_round_three.sql

@@ -0,0 +1,5 @@
+-- #10058: submitter_outcome_log and alert_dedup_claims gained a RETENTION_POLICY entry in the same change,
+-- following 0193/0196's precedent of pairing every new policy table with a leading-column index on its
+-- retention timestamp so the batched delete's inner SELECT is an index range scan rather than a full scan.
+CREATE INDEX IF NOT EXISTS idx_submitter_outcome_log_recorded_at ON submitter_outcome_log(recorded_at);
+CREATE INDEX IF NOT EXISTS idx_alert_dedup_claims_created_at ON alert_dedup_claims(created_at);

src/db/retention.ts

@@ -118,6 +118,17 @@ export const RETENTION_POLICY: readonly RetentionRule[] = [
   // plus contributor content (the largest, most sensitive artifact in the replay family), the re-query mode
   // is a debugging tool for RECENT decisions, and the public promptDigest commitment outlives the text.
   { table: "decision_replay_prompts", column: "created_at", days: 30 },
+  // #10058: two more members of the same re-derivable/per-event class #9473 bounded, found by the same audit
+  // sweep for tables written per event with NO delete path anywhere in src/:
+  //   - submitter_outcome_log is per (project, submitter, pull_number, outcome), appended on every PR
+  //     terminal (src/review/submitter-reputation.ts), and its only reader is already windowed
+  //     (`recorded_at >= datetime('now', ?)`) -- same "windowed reader, aged rows are pure dead weight" shape
+  //     as contributor_gate_history above, so it gets the same 90-day window.
+  //   - alert_dedup_claims is a pure hourly-expiring idempotency claim (src/review/alerts.ts hashes an hour
+  //     bucket into its dedup key), never read again once its hour passes -- same short-lived-idempotency-log
+  //     shape as webhook_events / orb_webhook_events above, so it gets the same 14-day window.
+  { table: "submitter_outcome_log", column: "recorded_at", days: 90 },
+  { table: "alert_dedup_claims", column: "created_at", days: 14 },
 ];
 
 // #9083: a real, single-column, indexable primary key for the ordered-range delete below, keyed by table
@@ -168,6 +179,8 @@ export const RETENTION_PK_COLUMN: Readonly<Record<string, string>> = {
   decision_replay_inputs: "record_id",
   // Same key shape as decision_replay_inputs above, for the same reason.
   decision_replay_prompts: "record_id",
+  // #10058: alert_dedup_claims has a genuine single-column `id TEXT PRIMARY KEY` (migrations/0181).
+  alert_dedup_claims: "id",
 };
 
 /**
@@ -191,6 +204,9 @@ export const RETENTION_COMPOSITE_PK_TABLES: ReadonlySet<string> = new Set([
   "ai_review_cache",
   "ai_slop_cache",
   "linked_issue_satisfaction_cache",
+  // #10058: PRIMARY KEY (project, submitter, pull_number, outcome) -- no genuine single-column id, so this
+  // table pays the rowid/ctid cost noted above rather than getting a surrogate key added just for pruning.
+  "submitter_outcome_log",
 ]);
 
 /**

test/unit/retention.test.ts

@@ -1,7 +1,7 @@
 import { describe, expect, it } from "vitest";
 import { createApp } from "../../src/api/routes";
 import { getDb } from "../../src/db/client";
-import { dedupeSignalSnapshots, pruneExpiredRecords, RETENTION_COMPOSITE_PK_TABLES, RETENTION_PK_COLUMN, RETENTION_POLICY, retentionCutoffIsoForTable } from "../../src/db/retention";
+import { dedupeSignalSnapshots, pruneExpiredRecords, RETENTION_COMPOSITE_PK_TABLES, RETENTION_PK_COLUMN, RETENTION_POLICY, retentionCutoffIsoForTable, retentionDaysForTable } from "../../src/db/retention";
 import { computeFleetAnalytics } from "../../src/orb/analytics";
 import { listFleetInstances } from "../../src/orb/fleet-admin";
 import { getOrbGlobalStats } from "../../src/orb/outcomes";
@@ -417,6 +417,69 @@ describe("pruneExpiredRecords", () => {
     const remaining = await env.DB.prepare("SELECT count(*) AS n FROM ai_review_cache").first<{ n: number }>();
     expect(remaining?.n).toBe(0);
   });
+
+  // #10058: submitter_outcome_log and alert_dedup_claims both store CURRENT_TIMESTAMP-format timestamps
+  // (`'YYYY-MM-DD HH:MM:SS'`, no `T`, no zone) because both writers omit the column and let the DB default
+  // supply it -- unlike every other policy table's ISO-8601 `column`. pruneExpiredRecords binds an ISO cutoff
+  // and compares as text; seeding in the exact writer-produced format (not `daysAgo`'s ISO string) is what
+  // proves the comparison still resolves the right rows on both sides of the cutoff.
+  it("prunes submitter_outcome_log older than its 90-day window and keeps recent rows (#10058)", async () => {
+    const env = createTestEnv();
+    await env.DB.prepare(
+      `INSERT INTO submitter_outcome_log (project, submitter, pull_number, outcome, recorded_at)
+       VALUES
+         ('acme/widgets', 'alice', 1, 'merged', '2026-01-01 00:00:00'),
+         ('acme/widgets', 'alice', 2, 'merged', '2026-06-12 00:00:00')`,
+    ).run();
+
+    const rule = RETENTION_POLICY.find((r) => r.table === "submitter_outcome_log");
+    expect(rule).toEqual({ table: "submitter_outcome_log", column: "recorded_at", days: 90 });
+
+    const results = await pruneExpiredRecords(env, { nowMs: NOW, policy: [rule!] });
+    expect(results[0]?.deleted).toBe(1);
+    const rows = await env.DB.prepare("SELECT pull_number FROM submitter_outcome_log").all<{ pull_number: number }>();
+    expect(rows.results.map((row) => row.pull_number)).toEqual([2]);
+  });
+
+  it("prunes alert_dedup_claims older than its 14-day window and keeps recent rows (#10058)", async () => {
+    const env = createTestEnv();
+    await env.DB.prepare(
+      `INSERT INTO alert_dedup_claims (id, project, target_id, notification_key, status, created_at)
+       VALUES
+         ('adc-old', 'acme/widgets', '__healthcheck__', 'hash-old', 'sent', '2026-01-01 00:00:00'),
+         ('adc-recent', 'acme/widgets', '__healthcheck__', 'hash-recent', 'sent', '2026-06-12 00:00:00')`,
+    ).run();
+
+    const rule = RETENTION_POLICY.find((r) => r.table === "alert_dedup_claims");
+    expect(rule).toEqual({ table: "alert_dedup_claims", column: "created_at", days: 14 });
+
+    const results = await pruneExpiredRecords(env, { nowMs: NOW, policy: [rule!] });
+    expect(results[0]?.deleted).toBe(1);
+    const rows = await env.DB.prepare("SELECT id FROM alert_dedup_claims").all<{ id: string }>();
+    expect(rows.results.map((row) => row.id)).toEqual(["adc-recent"]);
+  });
+
+  // pkColumnFor's two arms (src/db/retention.ts:211): alert_dedup_claims has a genuine `id` mapping in
+  // RETENTION_PK_COLUMN, while submitter_outcome_log's composite PK means it falls back to the `?? "rowid"`
+  // arm. A small batchSize also proves the batched-delete loop's `changes < batchSize` exit (line 420) fires
+  // on a real partial-then-final pair of iterations, not only on an empty first pass.
+  it("submitter_outcome_log's composite PK falls back to rowid ordering across multiple batches (#10058)", async () => {
+    const env = createTestEnv();
+    await env.DB.prepare(
+      `INSERT INTO submitter_outcome_log (project, submitter, pull_number, outcome, recorded_at)
+       VALUES
+         ('acme/widgets', 'alice', 1, 'merged', '2026-01-01 00:00:00'),
+         ('acme/widgets', 'alice', 2, 'closed', '2026-01-02 00:00:00'),
+         ('acme/widgets', 'alice', 3, 'merged', '2026-01-03 00:00:00'),
+         ('acme/widgets', 'bob', 1, 'merged', '2026-06-12 00:00:00')`,
+    ).run();
+
+    const rule = RETENTION_POLICY.find((r) => r.table === "submitter_outcome_log");
+    const results = await pruneExpiredRecords(env, { nowMs: NOW, policy: [rule!], batchSize: 2 });
+    expect(results[0]?.deleted).toBe(3);
+    const remaining = await env.DB.prepare("SELECT count(*) AS n FROM submitter_outcome_log").first<{ n: number }>();
+    expect(remaining?.n).toBe(1);
+  });
 });
 
 describe("dedupeSignalSnapshots", () => {
@@ -976,6 +1039,13 @@ describe("retentionCutoffIsoForTable (#9474)", () => {
     // Within a second of a locally computed 180-day cutoff -- pins the default-arg arm without clock flake.
     expect(Math.abs(Date.parse(cutoff!) - (Date.now() - 180 * 86_400_000))).toBeLessThan(1000);
   });
+
+  // #10058 regression: pins the two windows so a future edit that drops or shrinks either entry fails loudly
+  // rather than silently restoring unbounded growth on submitter_outcome_log or alert_dedup_claims.
+  it("submitter_outcome_log is 90 days and alert_dedup_claims is 14 days (#10058)", () => {
+    expect(retentionDaysForTable("submitter_outcome_log")).toBe(90);
+    expect(retentionDaysForTable("alert_dedup_claims")).toBe(14);
+  });
 });
 
 // #9783: orb_signals is a PUBLIC data source (computeFleetAnalytics' /fairness headline, #9775's weekly fleet