From 1fc4080800b1beb8c33bbf2b4354c254d8d8fbd4 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Tue, 14 Jul 2026 21:12:01 +0200 Subject: [PATCH] fix(outbox): never resurrect a synchronous row on reclaim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stale-claim reclaimer added earlier in this branch un-claimed every timed-out row, including SYNCHRONOUS ones. `reply_to.is_some()` is the schema's "never retry" signal (0009_outbox.sql) and the dispatcher honours it: those rows are an HTTP request someone is blocked on, and re-running one means re-running its side effects — a charge, an email, an external POST — for a caller that is long gone. So a dispatcher dying mid-sync-dispatch would have its row resurrected 10 minutes later and executed a second time, for nobody. Before the reclaimer those rows were merely stranded; the reclaimer turned inert into harmful. The reclaim now gates on `reply_to IS NULL`. Stale sync rows are DELETED instead: they can never be dispatched again and nobody is waiting on them, so leaving them would just accumulate garbage. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/manager-core/src/outbox_repo.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/crates/manager-core/src/outbox_repo.rs b/crates/manager-core/src/outbox_repo.rs index 8ebd81e..a7652fc 100644 --- a/crates/manager-core/src/outbox_repo.rs +++ b/crates/manager-core/src/outbox_repo.rs @@ -133,6 +133,11 @@ pub trait OutboxRepo: Send + Sync { /// Returns how many rows were reclaimed. `attempt_count` is deliberately NOT /// bumped: the handler never ran, so a reclaim must not consume the row's /// retry budget (same reasoning as the transient queue `release`). + /// + /// SYNCHRONOUS rows (`reply_to IS NOT NULL`) are NOT reclaimed — the schema + /// makes `reply_to.is_some()` the "never retry" signal, and their caller is + /// long gone. They are deleted instead: nobody is waiting, they can never be + /// dispatched, and leaving them would leak a row per crashed sync dispatch. async fn reclaim_stale_claims(&self, timeout_secs: u32) -> Result; /// Remove a row after a terminal outcome (success or dead-letter). @@ -231,14 +236,34 @@ impl OutboxRepo for PostgresOutboxRepo { } async fn reclaim_stale_claims(&self, timeout_secs: u32) -> Result { + // `reply_to IS NULL` is load-bearing: a row with a reply_to is a SYNCHRONOUS + // HTTP dispatch, and the schema (0009_outbox.sql) makes reply_to.is_some() + // the "never retry" signal. Returning one to the queue would re-execute a + // handler whose caller left minutes ago and whose inbox oneshot no longer + // exists — running its side effects a second time for nobody. let res = sqlx::query( "UPDATE outbox SET claimed_at = NULL, claimed_by = NULL \ WHERE claimed_at IS NOT NULL \ + AND reply_to IS NULL \ AND claimed_at < NOW() - ($1 || ' seconds')::INTERVAL", ) .bind(i64::from(timeout_secs)) .execute(&self.pool) .await?; + + // A stale SYNC row can never be dispatched again (never-retry) and nobody + // is waiting on it, so it is pure garbage — drop it rather than leak a row + // per crashed sync dispatch forever. + sqlx::query( + "DELETE FROM outbox \ + WHERE claimed_at IS NOT NULL \ + AND reply_to IS NOT NULL \ + AND claimed_at < NOW() - ($1 || ' seconds')::INTERVAL", + ) + .bind(i64::from(timeout_secs)) + .execute(&self.pool) + .await?; + Ok(res.rows_affected()) }