fix: per-handler claim lease in ledger, nats max_deliver, pin handler names

crates/application/src/worker.rs

@@ -3,7 +3,7 @@ use std::sync::Arc;
 use domain::{
     errors::DomainError,
     events::EventEnvelope,
-    ports::{EventConsumer, EventHandler, ProcessedEventLedger},
+    ports::{Begin, EventConsumer, EventHandler, ProcessedEventLedger},
 };
 use futures::StreamExt;
 use tokio::sync::Semaphore;
@@ -98,38 +98,57 @@ async fn dispatch(
     for handler in handlers.iter() {
         let name = handler.name();
         if let Some(id) = &id {
-            match ledger.has_processed(id, name).await {
-                Ok(true) => {
+            match ledger.begin(id, name).await {
+                Ok(Begin::Run) => {}
+                Ok(Begin::AlreadyDone) => {
                     tracing::debug!(handler = name, "already processed, skipping");
                     continue;
                 }
-                Ok(false) => {}
+                Ok(Begin::InProgressElsewhere) => {
+                    tracing::debug!(
+                        handler = name,
+                        "running in another delivery, retrying later"
+                    );
+                    any_transient = true;
+                    continue;
+                }
                 Err(e) => {
                     // Running anyway could repeat a side effect; retry the delivery instead.
-                    tracing::warn!(handler = name, "ledger lookup failed, will retry: {e}");
+                    tracing::warn!(handler = name, "ledger claim failed, will retry: {e}");
                     any_transient = true;
                     continue;
                 }
             }
         }
-        match handler.handle(&envelope.event).await {
+        let result = handler.handle(&envelope.event).await;
+        match &result {
             Ok(()) => {
                 if let Some(id) = &id
-                    && let Err(e) = ledger.record_processed(id, name).await
+                    && let Err(e) = ledger.complete(id, name).await
                 {
                     // The work is done; failing the delivery would only repeat it.
-                    tracing::warn!(handler = name, "recording processed event failed: {e}");
+                    // The claim lapses after its lease and the handler may run once more.
+                    tracing::warn!(handler = name, "completing ledger claim failed: {e}");
                 }
             }
-            Err(e) if e.is_transient() => {
-                tracing::warn!(handler = name, "transient handler error, will retry: {e}");
-                any_transient = true;
-            }
             Err(e) => {
-                tracing::warn!(
-                    handler = name,
-                    "permanent handler error (not retrying): {e}"
-                );
+                // Permanent failures are released too, not recorded: if a sibling
+                // handler's transient failure redelivers this event, a handler that
+                // failed permanently runs (and fails) again.
+                if let Some(id) = &id
+                    && let Err(le) = ledger.release(id, name).await
+                {
+                    tracing::warn!(handler = name, "releasing ledger claim failed: {le}");
+                }
+                if e.is_transient() {
+                    tracing::warn!(handler = name, "transient handler error, will retry: {e}");
+                    any_transient = true;
+                } else {
+                    tracing::warn!(
+                        handler = name,
+                        "permanent handler error (not retrying): {e}"
+                    );
+                }
             }
         }
     }

crates/application/tests/jobs/processed_events_cleanup_test.rs

@@ -10,11 +10,13 @@ async fn removes_only_rows_older_than_the_retention_window() {
     store.seed_processed_event(
         &stale,
         "h",
+        true,
         now - chrono::Duration::days(PROCESSED_EVENTS_RETENTION_DAYS + 1),
     );
     store.seed_processed_event(
         &fresh,
         "h",
+        true,
         now - chrono::Duration::days(PROCESSED_EVENTS_RETENTION_DAYS - 1),
     );
 
@@ -23,9 +25,9 @@ async fn removes_only_rows_older_than_the_retention_window() {
         .await
         .unwrap();
 
-    use domain::ports::ProcessedEventLedger;
-    assert!(!store.has_processed(&stale, "h").await.unwrap());
-    assert!(store.has_processed(&fresh, "h").await.unwrap());
+    use domain::ports::{Begin, ProcessedEventLedger};
+    assert_eq!(store.begin(&stale, "h").await.unwrap(), Begin::Run);
+    assert_eq!(store.begin(&fresh, "h").await.unwrap(), Begin::AlreadyDone);
 }
 
 #[test]

crates/application/tests/worker/worker_test.rs

@@ -477,3 +477,83 @@ async fn duplicate_handler_names_are_rejected() {
     let err = result.err().expect("duplicate names must be rejected");
     assert!(err.to_string().contains("same"), "{err}");
 }
+
+#[tokio::test]
+async fn a_handler_already_running_elsewhere_is_not_run_concurrently() {
+    use domain::ports::{Begin, ProcessedEventLedger};
+    let (handler, calls) = CountingHandler::new("slow", 0);
+    let store = InMemoryStore::new();
+    let id = domain::events::EventId::new();
+    // Another delivery holds a live claim on this (event, handler).
+    assert_eq!(store.begin(&id, "slow").await.unwrap(), Begin::Run);
+    let nacks = Arc::new(Mutex::new(0));
+    let consumer = RedeliveringConsumer {
+        id: Some(id),
+        event: movie_discovered(),
+        times: 1,
+        nacks: Arc::clone(&nacks),
+    };
+
+    WorkerService::new(Arc::new(consumer), vec![handler], store)
+        .unwrap()
+        .run(tokio::sync::watch::channel(false).1)
+        .await;
+
+    assert_eq!(
+        *calls.lock().unwrap(),
+        0,
+        "must not run beside the claim holder"
+    );
+    assert_eq!(*nacks.lock().unwrap(), 1, "delivery is retried later");
+}
+
+#[tokio::test]
+async fn a_stale_in_progress_claim_is_taken_over() {
+    use domain::ports::HANDLER_LEASE_SECONDS;
+    let (handler, calls) = CountingHandler::new("crashed", 0);
+    let store = InMemoryStore::new();
+    let id = domain::events::EventId::new();
+    store.seed_processed_event(
+        &id,
+        "crashed",
+        false,
+        chrono::Utc::now().naive_utc() - chrono::Duration::seconds(HANDLER_LEASE_SECONDS + 1),
+    );
+    let nacks = Arc::new(Mutex::new(0));
+    let consumer = RedeliveringConsumer {
+        id: Some(id),
+        event: movie_discovered(),
+        times: 1,
+        nacks: Arc::clone(&nacks),
+    };
+
+    WorkerService::new(Arc::new(consumer), vec![handler], store)
+        .unwrap()
+        .run(tokio::sync::watch::channel(false).1)
+        .await;
+
+    assert_eq!(*calls.lock().unwrap(), 1);
+    assert_eq!(*nacks.lock().unwrap(), 0);
+}
+
+#[tokio::test]
+async fn a_failed_handler_releases_its_claim_so_redelivery_retries() {
+    let (handler, calls) = CountingHandler::new("flaky", 1);
+    let store = InMemoryStore::new();
+    let nacks = Arc::new(Mutex::new(0));
+    let consumer = RedeliveringConsumer {
+        id: Some(domain::events::EventId::new()),
+        event: movie_discovered(),
+        times: 3,
+        nacks: Arc::clone(&nacks),
+    };
+
+    WorkerService::new(Arc::new(consumer), vec![handler], store.clone())
+        .unwrap()
+        .run(tokio::sync::watch::channel(false).1)
+        .await;
+
+    // Delivery 1 fails and releases; 2 succeeds; 3 is skipped as done.
+    assert_eq!(*calls.lock().unwrap(), 2);
+    assert_eq!(*nacks.lock().unwrap(), 1);
+}