From 3766ce1f3fc5e81cceeb9eb3d1fafc56148f4319 Mon Sep 17 00:00:00 2001
From: Sam Rolfe <samuelrolfe@gmail.com>
Date: Tue, 13 Oct 2026 10:45:16 +1100
Subject: [PATCH] retention: write the audit row before the actions_log cap

RetainWithAudit appends the 'retain' audit row inside the same transaction and
before the cap, so the cap counts it. After a run, actions_log holds at most
EMAIL_AGENT_ACTIONS_LOG_MAX_ROWS rows, including the audit row. Adds a test.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
---
 cmd/email-agent/main.go          | 12 +++++-------
 internal/store/retention.go      | 16 ++++++++++++++++
 internal/store/retention_test.go | 29 +++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/cmd/email-agent/main.go b/cmd/email-agent/main.go
index 18e2efa..6ac4695 100644
--- a/cmd/email-agent/main.go
+++ b/cmd/email-agent/main.go
@@ -344,16 +344,14 @@ func runRetain(cfg *config.Config, logger *slog.Logger) error {
 		BodyMaxAge:        time.Duration(cfg.RetentionDays) * 24 * time.Hour,
 		ActionsLogMaxRows: cfg.ActionsLogMaxRows,
 	}
-	res, err := st.Retain(ctx, policy)
+	// The audit row is written inside the same transaction, before the cap, so
+	// the cap counts it. This is what keeps actions_log bounded across runs.
+	detail := fmt.Sprintf("bodies_cleared=%%d retention_days=%d max_rows=%d",
+		cfg.RetentionDays, cfg.ActionsLogMaxRows)
+	res, err := st.RetainWithAudit(ctx, policy, "scheduler", detail)
 	if err != nil {
 		return err
 	}
-	if err := st.LogAction(ctx, "retain", "scheduler",
-		fmt.Sprintf("bodies_cleared=%d actions_deleted=%d retention_days=%d max_rows=%d",
-			res.BodiesCleared, res.ActionsDeleted, cfg.RetentionDays, cfg.ActionsLogMaxRows)); err != nil {
-		// Retention already committed; a failed audit row must not fail the job.
-		logger.Error("log retention action", "error", err)
-	}
 
 	cutoff := "disabled"
 	if !res.BodyCutoff.IsZero() {
diff --git a/internal/store/retention.go b/internal/store/retention.go
index 84da690..706a983 100644
--- a/internal/store/retention.go
+++ b/internal/store/retention.go
@@ -49,6 +49,14 @@ type RetentionResult struct {
 // pass never leaves the database in a mixed state. It is safe to run on a
 // schedule: every statement is idempotent.
 func (s *Store) Retain(ctx context.Context, p RetentionPolicy) (RetentionResult, error) {
+	return s.RetainWithAudit(ctx, p, "", "")
+}
+
+// RetainWithAudit is Retain plus one actions_log row (FR-13), appended inside
+// the same transaction before the cap is applied so the audit row is itself
+// counted by the cap. An empty detail writes no audit row. This keeps the
+// action log bounded even though every run records that it ran.
+func (s *Store) RetainWithAudit(ctx context.Context, p RetentionPolicy, actor, detail string) (RetentionResult, error) {
 	var res RetentionResult
 
 	tx, err := s.db.BeginTx(ctx, nil)
@@ -74,6 +82,14 @@ WHERE body_text IS NOT NULL AND body_text <> '' AND received_at < ?`,
 		}
 	}
 
+	if detail != "" {
+		if _, err := tx.ExecContext(ctx, `
+INSERT INTO actions_log (message_id, action, actor, detail)
+VALUES (NULL, 'retain', ?, ?)`, nullString(actor), detail); err != nil {
+			return res, fmt.Errorf("log retention action: %w", err)
+		}
+	}
+
 	if p.ActionsLogMaxRows > 0 {
 		// Keep the newest N rows and delete the rest. id is monotonic, so it
 		// is the stable ordering for an append-only table.
diff --git a/internal/store/retention_test.go b/internal/store/retention_test.go
index 8e5d198..50a08dd 100644
--- a/internal/store/retention_test.go
+++ b/internal/store/retention_test.go
@@ -121,6 +121,35 @@ func TestRetainCapsActionsLog(t *testing.T) {
 	}
 }
 
+func TestRetainWithAuditCountsItsOwnRow(t *testing.T) {
+	s, ctx := testStore(t)
+
+	for i := 0; i < 5; i++ {
+		if err := s.LogAction(ctx, "fetch", "scheduler", "pass"); err != nil {
+			t.Fatalf("LogAction() %d error = %v", i, err)
+		}
+	}
+
+	res, err := s.RetainWithAudit(ctx, RetentionPolicy{ActionsLogMaxRows: 3}, "scheduler", "bodies_cleared=0")
+	if err != nil {
+		t.Fatalf("RetainWithAudit() error = %v", err)
+	}
+	// 5 existing + 1 audit = 6; keep 3, so 3 are deleted.
+	if res.ActionsDeleted != 3 {
+		t.Errorf("ActionsDeleted = %d, want 3", res.ActionsDeleted)
+	}
+	entries, err := s.RecentActions(ctx, 100)
+	if err != nil {
+		t.Fatalf("RecentActions() error = %v", err)
+	}
+	if len(entries) != 3 {
+		t.Fatalf("actions_log rows = %d, want 3 (cap counts the audit row)", len(entries))
+	}
+	if entries[0].Action != "retain" {
+		t.Errorf("newest row action = %q, want retain", entries[0].Action)
+	}
+}
+
 func TestRetainDisabledPolicyIsNoOp(t *testing.T) {
 	s, ctx := testStore(t)