Retry retryable ack failures instead of abandoning the work item (redux)

CHANGELOG.md

@@ -49,6 +49,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Relative SQLite queue delays saturate instead of overflowing their timestamp
   representation. (#74)
 
+- **Activity acknowledgement retries** - Retry transient provider errors when
+  acknowledging activity success or application errors, up to five attempts with
+  lock renewal retained. This avoids unnecessary activity re-execution.
+  Cancellation acknowledgements remain single-attempt, and retries do not extend
+  the shutdown grace period. A permanent error after a transient acknowledgement
+  error is reported as an uncertain outcome rather than a confirmed infrastructure
+  failure; neither commit nor redelivery is guaranteed. Uncertain outcomes are
+  counted separately in `duroxide_activity_ack_uncertain_total`. (#72)
+
+### Provider notes for 0.2.0
+
+- **Duplicate activity acknowledgements** - Completion acknowledgements can be
+  retried after the transaction committed but its response was lost. Providers
+  must atomically remove the locked worker item and enqueue its completion.
+  After that commit, repeating `ack_work_item(token, Some(completion))` with the
+  same token must return a non-retryable error without enqueueing another
+  completion. See the [provider contract](docs/provider-implementation-guide.md#error-classification). (#72)
+- After upgrading the Duroxide dependency, register
+  [`duroxide::provider_validations::test_ack_work_item_duplicate_rejected`](src/provider_validation/atomicity.rs)
+  in your provider's validation suite. It acknowledges the same token twice and
+  verifies that the second call fails non-retryably, exactly one completion is
+  queued, and the worker item is removed. SQLite already runs this validation;
+  external providers must add their own wrapper. (#72)
+
 ## [0.1.32] - 2026-10-11
 
 **Release:** <https://crates.io/crates/duroxide/0.1.32>

docs/duroxide-telemetry-spec.md

@@ -209,6 +209,22 @@ sum by (activity_name, tag) (rate(duroxide_activity_admission_deferrals_total[5m
 
 ---
 
+#### `duroxide_activity_ack_uncertain_total` (Counter)
+
+**Description:** Activity completion acknowledgements ending in a permanent error
+after a retryable error. Incremented once per uncertain outcome, not per retry.
+
+**Labels:** _(none)_
+
+**Use Case:** Track uncertain acknowledgements separately from confirmed
+infrastructure failures. An earlier attempt may have committed or the lock may
+have been lost; redelivery is not guaranteed. Successful retries and confirmed
+acknowledgement failures do not increment this counter.
+
+Read the in-process count through `MetricsProvider::activity_ack_uncertain()`.
+
+---
+
 ### 1.3 Provider (Storage) Metrics
 
 #### `duroxide_provider_operation_duration_seconds` (Histogram)

docs/metrics-specification.md

@@ -32,6 +32,7 @@ This document provides a complete reference of all metrics emitted by duroxide v
 | `duroxide_activity_duration_seconds` | Histogram | Activity | `activity_name`, `outcome`, `tag` | ✅ `test_activity_duration_tracking` | ✅ Code Review | Buckets: 0.01-300s |
 | `duroxide_activity_infrastructure_errors_total` | Counter | Activity | `activity_name` | ✅ `metrics_capture_activity_and_orchestration_outcomes` | ✅ Code Review | Infra-specific errors |
 | `duroxide_activity_configuration_errors_total` | Counter | Activity | `activity_name` | ✅ `test_error_classification_metrics` | ✅ Code Review | Config-specific errors |
+| `duroxide_activity_ack_uncertain_total` | Counter | Activity | _(none)_ | ✅ `test_metrics_activity_ack_uncertain` | ✅ Emitted | Counts uncertain acknowledgement outcomes, not confirmed failures |
 | **Poison Message** |
 | `duroxide_orchestration_poison_total` | Counter | Poison | _(none)_ | ✅ `test_poison_message_metrics` | ✅ Code Review | Orchestrations exceeding max_attempts |
 | `duroxide_activity_poison_total` | Counter | Poison | _(none)_ | ✅ `test_poison_message_metrics` | ✅ Code Review | Activities exceeding max_attempts |
@@ -485,6 +486,22 @@ sum by (activity_name, tag) (rate(duroxide_activity_admission_deferrals_total[5m
 
 ---
 
+### 2.6 `duroxide_activity_ack_uncertain_total` (Counter)
+
+**Description:** Activity completion acknowledgements ending in a permanent error
+after a retryable error. Incremented once per uncertain outcome, not per retry.
+
+**Labels:** _(none)_
+
+**Purpose:** Track uncertain acknowledgements separately from confirmed activity
+infrastructure failures. An earlier attempt may have committed or the lock may
+have been lost; redelivery is not guaranteed. This counter does not increment
+`duroxide_activity_executions_total` or `duroxide_activity_infrastructure_errors_total`.
+Read the in-process count through `MetricsProvider::activity_ack_uncertain()`;
+the public fields of `MetricsSnapshot` are unchanged.
+
+---
+
 ## 3. Sub-Orchestration Metrics
 
 > **Session Metrics (Future):** Session-specific metrics (session claims, migrations,

docs/observability-guide.md

@@ -198,6 +198,7 @@ All metrics follow Prometheus naming conventions with `_total` for counters and
 | `duroxide_activity_admission_deferrals_total` | Counter | Activity | activity_name, tag | Track admission deferrals whose abandon returned success |
 | `duroxide_activity_duration_seconds` | Histogram | Activity | activity_name, outcome | Activity latency p95/p99 |
 | `duroxide_activity_errors_total` | Counter | Activity | activity_name, error_type, retryable | Detailed error tracking |
+| `duroxide_activity_ack_uncertain_total` | Counter | Activity | _(none)_ | Track uncertain acknowledgement outcomes separately from failures |
 | **Sub-Orchestration** |
 | `duroxide_suborchestration_calls_total` | Counter | Sub-Orch | parent_orchestration, child_orchestration, outcome | Track sub-orch calls |
 | `duroxide_suborchestration_duration_seconds` | Histogram | Sub-Orch | parent_orchestration, child_orchestration, outcome | Sub-orch latency |
@@ -299,6 +300,11 @@ before admission count as infrastructure errors, not deferrals.
 - **Labels**: `activity_name`, `error_type` (app_error|infrastructure_error|config_error), `retryable` (true|false)
 - **Use**: Distinguish transient vs permanent errors
 
+**`duroxide_activity_ack_uncertain_total`** (Counter)
+- **Description**: Activity completion acknowledgements ending in a permanent error after a retryable error
+- **Labels**: _(none)_
+- **Use**: Track uncertainty separately from confirmed infrastructure failures. Counted once per uncertain outcome, not per retry. An earlier attempt may have committed or the lock may have been lost; redelivery is not guaranteed.
+
 #### Sub-Orchestration Metrics
 
 **`duroxide_suborchestration_calls_total`** (Counter)

docs/provider-implementation-guide.md

@@ -904,7 +904,7 @@ Provider methods return `Result<..., ProviderError>`. The runtime uses error cla
 // Retryable: Transient failures, runtime will retry with backoff
 ProviderError::retryable("operation", "Database busy")
 
-// Permanent: Unrecoverable, runtime will fail the orchestration
+// Permanent: Retrying the operation will not recover it
 ProviderError::permanent("operation", "Duplicate event detected")
 ```
 
@@ -914,12 +914,47 @@ ProviderError::permanent("operation", "Duplicate event detected")
 - Network failure
 - Temporary resource exhaustion
 
-**Permanent errors** (runtime will fail):
+**Permanent errors** (do not retry):
 - Duplicate events
 - Missing lock token (already processed or cancelled)
 - Data corruption
 - Invalid input
 
+For acknowledgements of activity success or application errors,
+the runtime retries retryable errors in place up to five total attempts before
+abandoning the work item. A permanent error on the first attempt goes directly
+to abandonment. The retry waits are 100, 200, 400, and 800 milliseconds: 1.5 seconds of cumulative backoff,
+excluding time spent in provider calls. Normal activity completion retains lock
+renewal while acknowledgement is pending. Activities still have at-least-once
+execution semantics; exhausted retries or lost locks can lead to re-execution.
+
+Cancellation acknowledgements (`ack_work_item(token, None)`) are attempted only
+once. If that attempt fails, the worker falls back to abandonment rather than
+retrying the acknowledgement.
+
+A retryable error does not necessarily mean the transaction rolled back: a
+connection failure can leave the commit outcome unknown. If an acknowledgement
+already committed, a repeated call must reject the missing lock token with a
+permanent error without enqueueing another completion. Preserve the atomic
+delete-and-enqueue operation and lock validation described below.
+
+A permanent error after a retryable acknowledgement error is reported as an
+**uncertain outcome**, not as a confirmed activity infrastructure failure. The
+runtime cannot distinguish an earlier commit from lock loss or another permanent
+error. It increments `duroxide_activity_ack_uncertain_total` once for this outcome,
+without attempting abandonment or counting an activity infrastructure error.
+Redelivery is not guaranteed: the work item may already be gone. If uncommitted
+work remains in the queue, it can become eligible for redelivery after its lock
+expires.
+
+Acknowledgement retries do not extend the shutdown grace period. The default
+`shutdown(None)` grace is 1 second, shorter than the full 1.5-second backoff budget,
+so shutdown may abort a retrying acknowledgement, including during a backoff wait.
+Shutdown remains a bounded drain: retries continue within the grace period and
+are aborted when it expires. Choose a longer grace period to allow more time for
+retries and provider calls. Uncommitted work is recovered through lock expiry;
+an acknowledgement that committed before cancellation remains committed.
+
 ### Atomicity Requirements
 
 Several operations must be atomic (all-or-nothing):

docs/provider-testing-guide.md

@@ -422,6 +422,7 @@ use duroxide::providers::Provider;
 use duroxide::provider_validations::{
     ProviderFactory,
     // Atomicity tests
+    test_ack_work_item_duplicate_rejected,
     test_atomicity_failure_rollback,
     test_multi_operation_atomic_ack,
     // Locking tests
@@ -511,11 +512,12 @@ async fn test_my_provider_worker_queue_fifo_ordering() {
 
 The validation test suite includes the following categories:
 
-1. **Atomicity Tests (4 tests)**
+1. **Atomicity Tests (5 tests)**
    - `test_atomicity_failure_rollback` - All-or-nothing commit semantics, rollback on failure
    - `test_multi_operation_atomic_ack` - Complex ack succeeds atomically
    - `test_lock_released_only_on_successful_ack` - Lock only released on success
    - `test_concurrent_ack_prevention` - Only one ack succeeds with same token
+   - `test_ack_work_item_duplicate_rejected` - Repeating `ack_work_item(Some(...))` with the same token fails non-retryably and leaves exactly one completion queued, without relying on replay deduplication
 
 2. **Error Handling Tests (5 tests)**
    - `test_invalid_lock_token_on_ack` - Invalid lock token rejection

src/provider_validation/atomicity.rs

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-use crate::provider_validation::{Event, EventKind, ExecutionMetadata, start_item};
+use crate::provider_validation::{Event, EventKind, ExecutionMetadata, create_instance, start_item};
 use crate::provider_validations::ProviderFactory;
 use crate::providers::{TagFilter, WorkItem};
 use std::sync::Arc;
@@ -463,3 +463,68 @@ pub async fn test_concurrent_ack_prevention<F: ProviderFactory>(factory: &F) {
     assert_eq!(failures.len(), 1, "Exactly one ack should fail");
     tracing::info!("✓ Test passed: concurrent ack prevention verified");
 }
+
+/// Test 2.5: Duplicate Worker Ack
+/// Goal: Reject a repeated completion ack without enqueueing a duplicate completion.
+pub async fn test_ack_work_item_duplicate_rejected<F: ProviderFactory>(factory: &F) {
+    let provider = factory.create_provider().await;
+    let instance = "instance-duplicate-ack";
+    create_instance(provider.as_ref(), instance).await.unwrap();
+
+    provider
+        .enqueue_for_worker(WorkItem::ActivityExecute {
+            instance: instance.to_string(),
+            execution_id: 1,
+            id: 2,
+            name: "TestActivity".to_string(),
+            input: "{}".to_string(),
+            session_id: None,
+            tag: None,
+        })
+        .await
+        .unwrap();
+    let (_, token, _) = provider
+        .fetch_work_item(Duration::from_secs(30), Duration::ZERO, None, &TagFilter::default())
+        .await
+        .unwrap()
+        .unwrap();
+    let completion = WorkItem::ActivityCompleted {
+        instance: instance.to_string(),
+        execution_id: 1,
+        id: 2,
+        result: "done".to_string(),
+    };
+    provider.ack_work_item(&token, Some(completion.clone())).await.unwrap();
+    let error = provider
+        .ack_work_item(&token, Some(completion.clone()))
+        .await
+        .expect_err("A second ack with the same token must fail");
+    assert!(!error.is_retryable(), "A repeated ack must fail permanently: {error}");
+
+    let (item, token, _) = provider
+        .fetch_orchestration_item(Duration::from_secs(30), Duration::ZERO, None)
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(item.messages, vec![completion], "Exactly one completion must be queued");
+    provider
+        .ack_orchestration_item(&token, 1, vec![], vec![], vec![], ExecutionMetadata::default(), vec![])
+        .await
+        .unwrap();
+    assert!(
+        provider
+            .fetch_orchestration_item(Duration::from_secs(30), Duration::ZERO, None)
+            .await
+            .unwrap()
+            .is_none(),
+        "No duplicate completion may remain outside the first batch"
+    );
+    assert!(
+        provider
+            .fetch_work_item(Duration::from_secs(30), Duration::ZERO, None, &TagFilter::default())
+            .await
+            .unwrap()
+            .is_none(),
+        "The acknowledged worker item must be removed"
+    );
+}

src/provider_validations.rs

@@ -62,6 +62,7 @@ pub use crate::provider_validation::race_replay;
 /// - `test_multi_operation_atomic_ack` - Verify complex ack succeeds atomically
 /// - `test_lock_released_only_on_successful_ack` - Verify lock only released on success
 /// - `test_concurrent_ack_prevention` - Verify only one ack succeeds with same token
+/// - `test_ack_work_item_duplicate_rejected` - Verify a repeated worker ack cannot enqueue another completion
 ///
 /// **Error Handling Tests:**
 /// - `test_invalid_lock_token_on_ack` - Verify invalid lock tokens are rejected
@@ -141,8 +142,8 @@ pub use crate::provider_validation::instance_creation::{
 
 #[cfg(feature = "provider-test")]
 pub use crate::provider_validation::atomicity::{
-    test_atomicity_failure_rollback, test_concurrent_ack_prevention, test_lock_released_only_on_successful_ack,
-    test_multi_operation_atomic_ack,
+    test_ack_work_item_duplicate_rejected, test_atomicity_failure_rollback, test_concurrent_ack_prevention,
+    test_lock_released_only_on_successful_ack, test_multi_operation_atomic_ack,
 };
 
 #[cfg(feature = "provider-test")]

src/providers/mod.rs

@@ -1811,6 +1811,9 @@ pub trait Provider: Any + Send + Sync {
     /// - The activity was cancelled (the backing entry was removed/invalidated by orchestration)
     /// - The lock was stolen by another worker (lock expired and work-item was re-fetched/completed)
     /// - The lock expired (worker took too long without renewing)
+    /// - An earlier acknowledgement committed, but its response was lost
+    ///
+    /// Repeating an acknowledgement with the same token MUST NOT enqueue another completion.
     ///
     /// The worker dispatcher uses this error to detect cancellation races and log appropriately.
     ///