fix(mcp): sweep terminal approval rows past a retention window

Review finding on PR open-legal-products#247: terminal ledger rows (executed / failed /
denied / expired) were retained forever, and each one carries the full
tool-argument jsonb the model proposed — in a legal product that can be
privileged matter data (case ids, document excerpts, party names)
sitting in a table with no cleanup path.

WHY THIS MATTERS
Data-minimization is part of the security posture: every copy of
sensitive data is another thing a breach, a misconfigured export, or an
over-broad query can leak. The ledger's arguments payload exists for
exactly one purpose — showing the user what they are approving and
executing precisely that — and that purpose is over within minutes.
Terminal rows keep only short-term forensic value ("what ran today?"),
so they get a bounded lifetime (24h) instead of an unbounded one.

WHAT IS AN OPPORTUNISTIC SWEEP
Instead of standing up cron/job infrastructure for a low-traffic
table, cleanup piggybacks on an operation that already touches the
table: every new pending-call INSERT first deletes terminal rows older
than the retention window. The repo already uses insert-time cleanup
for the OAuth state store (saveCodeVerifier deletes the stale state
row before inserting the new one); this follows the same shape. The
table only grows while the approval flow is being used — which is
precisely when the sweep runs.

HOW THE FIX WORKS
    await db.from("user_mcp_pending_tool_calls")
        .delete()
        .in("status", ["executed", "failed", "denied", "expired"])
        .lt("created_at", cutoff);   // now - 24h

- Only TERMINAL statuses are eligible: pending/approved/executing rows
  are the approval flow's live state and are never deleted, whatever
  their age (they are bounded anyway — expires_at retires them to a
  terminal state within minutes).
- Best-effort: a sweep failure is logged and swallowed, because the
  user is waiting on the approval prompt the insert serves; the next
  insert retries the sweep.
- The sweep is deliberately global (not per-user): the service-role
  backend is the only writer, and an active user's insert also clears
  other users' aged-out rows, so retention holds even for users who
  never return.

Test: an insert with aged executed/expired rows, a recent denied row,
and an old-but-live pending row on the table deletes exactly the aged
terminal rows and nothing else.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: amal66

backend/migrations/20260802_01_mcp_pending_tool_calls.sql

@@ -15,6 +15,12 @@
 -- states are written only after the MCP call actually finishes, so the
 -- ledger records what happened, not what was about to happen.
 --
+-- Retention: terminal rows (executed / failed / denied / expired) keep the
+-- full tool-argument payload, which can contain sensitive matter data, so
+-- they are not kept forever. The backend opportunistically deletes terminal
+-- rows older than a retention window whenever a new pending call is
+-- inserted (see sweepExpiredTerminalMcpToolCalls in lib/mcp/approvals.ts).
+--
 -- RLS is enabled with no browser policies, matching the other MCP tables:
 -- only the service-role backend reads or writes rows, and it always scopes
 -- queries by user_id.

backend/src/lib/mcp/__tests__/approvals.test.ts

@@ -5,6 +5,7 @@ import {
     decideMcpPendingToolCall,
     markMcpToolCallExecuted,
     markMcpToolCallFailed,
+    MCP_PENDING_CALL_RETENTION_MS,
     waitForMcpApprovalDecision,
 } from "../approvals";
 import type { ConnectorRow, Db, ToolCacheRow } from "../types";
@@ -23,7 +24,7 @@ function createFakeDb(initial: Row[] = []) {
 
     function from(_table: string) {
         const state = {
-            op: "select" as "select" | "insert" | "update",
+            op: "select" as "select" | "insert" | "update" | "delete",
             values: {} as Row,
             filters: [] as Array<(row: Row) => boolean>,
             single: false,
@@ -39,6 +40,10 @@ function createFakeDb(initial: Row[] = []) {
                 state.values = values;
                 return api;
             },
+            delete() {
+                state.op = "delete";
+                return api;
+            },
             select(_columns?: string) {
                 return api;
             },
@@ -51,6 +56,14 @@ function createFakeDb(initial: Row[] = []) {
                 state.filters.push((row) => String(row[key]) > String(value));
                 return api;
             },
+            lt(key: string, value: unknown) {
+                state.filters.push((row) => String(row[key]) < String(value));
+                return api;
+            },
+            in(key: string, values: unknown[]) {
+                state.filters.push((row) => values.includes(row[key]));
+                return api;
+            },
             single() {
                 state.single = true;
                 return api;
@@ -77,6 +90,10 @@ function createFakeDb(initial: Row[] = []) {
                         for (const row of matched)
                             Object.assign(row, state.values);
                     }
+                    if (state.op === "delete") {
+                        for (const row of matched)
+                            rows.splice(rows.indexOf(row), 1);
+                    }
                     data = state.single
                         ? matched[0]
                             ? { ...matched[0] }
@@ -123,6 +140,50 @@ describe("pending MCP tool call lifecycle", () => {
         expect(rows).toHaveLength(1);
     });
 
+    it("inserting a new pending call sweeps terminal rows past the retention window", async () => {
+        // Terminal rows carry the full tool-argument payload (sensitive
+        // matter data), so each new pending-call insert opportunistically
+        // deletes terminal rows older than the retention window. Live rows
+        // and recent terminal rows are untouched.
+        const beyondRetention = new Date(
+            Date.now() - MCP_PENDING_CALL_RETENTION_MS - 60_000,
+        ).toISOString();
+        const { db, rows } = createFakeDb([
+            {
+                id: "old-executed",
+                status: "executed",
+                created_at: beyondRetention,
+            },
+            {
+                id: "old-expired",
+                status: "expired",
+                created_at: beyondRetention,
+            },
+            {
+                id: "recent-denied",
+                status: "denied",
+                created_at: new Date().toISOString(),
+            },
+            {
+                // Old but non-terminal: the sweep must never delete the
+                // approval flow's live state, whatever its age.
+                id: "old-pending",
+                status: "pending",
+                created_at: beyondRetention,
+                expires_at: new Date(Date.now() + 60_000).toISOString(),
+            },
+        ]);
+
+        await seedPending(db);
+
+        const ids = rows.map((row) => row.id);
+        expect(ids).not.toContain("old-executed");
+        expect(ids).not.toContain("old-expired");
+        expect(ids).toContain("recent-denied");
+        expect(ids).toContain("old-pending");
+        expect(rows).toHaveLength(3);
+    });
+
     it("approves only for the owning user; a stranger's decision changes nothing", async () => {
         const { db, rows } = createFakeDb();
         const pending = await seedPending(db);

backend/src/lib/mcp/approvals.ts

@@ -12,8 +12,41 @@ export const MCP_APPROVAL_TTL_MS = 2 * 60 * 1000;
 export const MCP_APPROVAL_WAIT_MS = 90 * 1000;
 const POLL_INTERVAL_MS = 1_500;
 
+// Terminal ledger rows keep the full tool-argument payload — which in a
+// legal product can contain sensitive matter data — so they must not
+// accumulate forever. They stay long enough to debug a session and to
+// answer "what ran today?", then get swept.
+export const MCP_PENDING_CALL_RETENTION_MS = 24 * 60 * 60 * 1000;
+const TERMINAL_STATUSES = ["executed", "failed", "denied", "expired"];
+
 const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
 
+/**
+ * Opportunistic retention sweep, piggybacked on every new pending-call
+ * insert (the same insert-time cleanup pattern the OAuth state store uses):
+ * no cron/job infrastructure, and the table only sees traffic when the
+ * approval flow is in use anyway. Only TERMINAL rows past the retention
+ * window are deleted — live pending/approved/executing rows are the
+ * approval flow's working state and are never touched. Best-effort by
+ * design: a failed sweep must not block the approval the user is waiting
+ * on; the next insert retries it.
+ */
+async function sweepExpiredTerminalMcpToolCalls(db: Db): Promise<void> {
+    const cutoff = new Date(
+        Date.now() - MCP_PENDING_CALL_RETENTION_MS,
+    ).toISOString();
+    const { error } = await db
+        .from("user_mcp_pending_tool_calls")
+        .delete()
+        .in("status", TERMINAL_STATUSES)
+        .lt("created_at", cutoff);
+    if (error) {
+        console.error("[mcp-approvals] retention sweep failed", {
+            error: error.message,
+        });
+    }
+}
+
 /**
  * Record the EXACT call the model proposed. What the user later approves is
  * this row — execution reads the stored arguments back from it, never the
@@ -26,6 +59,7 @@ export async function createPendingMcpToolCall(
     args: Record<string, unknown>,
     db: Db = createServerSupabase(),
 ): Promise<PendingToolCallRow> {
+    await sweepExpiredTerminalMcpToolCalls(db);
     const { data, error } = await db
         .from("user_mcp_pending_tool_calls")
         .insert({