Fix Gmail effect truth and settlement (#173, #174)

crates/openspine-kernel/src/api/connector_breaker.rs

@@ -298,20 +298,26 @@ fn map_admission_error(
 
 /// Map a resolved (non-timeout) write outcome to a dispatch error.
 ///
-/// Candidate Gmail-write extension: a `gmail.create_draft` whose effect is not
-/// confirmed by the returned value must retain durable pending evidence unless
-/// the provider returns a *confirmed* response. A transport/no-response class
-/// may mean the write landed before the response was lost, so it surfaces as
-/// [`DispatchError::DeliveryUnknown`] rather than a confirmed failure. Only a
-/// response the provider explicitly reports as failed (e.g. a definite `api`
-/// error) resolves the pending row.
+/// A `gmail.create_draft` whose effect is not confirmed by the returned value
+/// must retain durable pending evidence unless the provider response proves
+/// non-occurrence. Transport/no-response, malformed success responses, HTTP
+/// 429, and HTTP 5xx may all mean the write landed before the response was
+/// lost or substituted, so they surface as [`DispatchError::DeliveryUnknown`].
+/// A definite client rejection (HTTP 4xx other than 429) is a confirmed
+/// failure and resolves the pending row.
 fn map_write_error(err: anyhow::Error) -> DispatchError {
     if let Some(gmail) = err.downcast_ref::<crate::gmail::GmailError>() {
-        if matches!(
-            gmail.class,
-            crate::gmail::GmailFailureClass::Transport
-                | crate::gmail::GmailFailureClass::MalformedResponse
-        ) {
+        let ambiguous_api_status = matches!(
+            (gmail.class, gmail.status),
+            (crate::gmail::GmailFailureClass::Api, Some(429 | 500..=599))
+        );
+        if ambiguous_api_status
+            || matches!(
+                gmail.class,
+                crate::gmail::GmailFailureClass::Transport
+                    | crate::gmail::GmailFailureClass::MalformedResponse
+            )
+        {
             return DispatchError::DeliveryUnknown(anyhow!(
                 "gmail write outcome is unconfirmed (delivery-unknown): {gmail}"
             ));
@@ -343,11 +349,48 @@ mod tests {
     }
 
     #[test]
-    fn confirmed_gmail_api_write_failure_is_connector_error() {
+    fn gmail_5xx_write_is_delivery_unknown() {
+        // #173: a 5xx from drafts.create does not prove the write failed — the
+        // draft may have landed before the intermediary errored — so it is
+        // delivery-unknown, not a confirmed failure.
+        for status in [500u16, 502, 503, 504] {
+            let err = map_write_error(anyhow::Error::new(crate::gmail::GmailError {
+                status: Some(status),
+                class: crate::gmail::GmailFailureClass::Api,
+            }));
+            assert!(
+                matches!(err, DispatchError::DeliveryUnknown(_)),
+                "status {status} must be delivery-unknown, got {err:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn gmail_429_write_is_delivery_unknown() {
+        // #173: a 429 (rate limited) likewise does not prove non-occurrence.
         let err = map_write_error(anyhow::Error::new(crate::gmail::GmailError {
-            status: Some(500),
+            status: Some(429),
             class: crate::gmail::GmailFailureClass::Api,
         }));
-        assert!(matches!(err, DispatchError::Connector(_)));
+        assert!(
+            matches!(err, DispatchError::DeliveryUnknown(_)),
+            "got {err:?}"
+        );
+    }
+
+    #[test]
+    fn confirmed_gmail_4xx_write_failure_is_connector_error() {
+        // A definite client rejection (4xx other than 429) proves the write
+        // did not take hold and resolves the pending row.
+        for status in [400u16, 403, 422] {
+            let err = map_write_error(anyhow::Error::new(crate::gmail::GmailError {
+                status: Some(status),
+                class: crate::gmail::GmailFailureClass::Api,
+            }));
+            assert!(
+                matches!(err, DispatchError::Connector(_)),
+                "status {status} must be a confirmed connector failure, got {err:?}"
+            );
+        }
     }
 }

crates/openspine-kernel/src/api/scoped_admission.rs

@@ -542,6 +542,9 @@ pub(crate) async fn dispatch_scoped_effect(
 #[path = "scoped_admission_drift_tests.rs"]
 mod drift_tests;
 #[cfg(test)]
+#[path = "scoped_admission_effect_truth_tests.rs"]
+mod effect_truth_tests;
+#[cfg(test)]
 #[path = "scoped_admission_outcome_tests.rs"]
 mod outcome_tests;
 #[cfg(test)]

crates/openspine-kernel/src/api/scoped_admission_effect_truth_tests.rs

@@ -0,0 +1,60 @@
+//! Effect Truth regressions for scope-matched standing-rule settlement.
+
+use jiff::Timestamp;
+use openspine_schemas::action::ActionId;
+use serde_json::json;
+
+use super::scoped_admission_support::*;
+use crate::api::actions::{mediate_and_dispatch_action, FailureSurface};
+
+/// #173 on the delegated path: a provider 5xx is delivery-unknown, so the
+/// scoped reservation is RETAINED (not cancelled) and the fence stays open.
+/// A 5xx was previously collapsed to a confirmed failure, cancelling the
+/// reservation and releasing reviewed budget for a write that may have landed.
+#[tokio::test]
+async fn scoped_provider_5xx_retains_reservation_and_leaves_fence_open() {
+    let env = draft_env(&["thread-1"]).await;
+    mount_drafts(&env.api_server, 502, json!({"error": {"code": 502}})).await;
+    let grant = mint_draft_grant(&env.state, "thread-1");
+    let context = resolved_context(&env.state, &grant).await;
+    env.state
+        .store
+        .activate_standing_rule(
+            &scoped_manifest("rule-5xx", &context),
+            None,
+            Timestamp::now(),
+        )
+        .unwrap();
+
+    let result = mediate_and_dispatch_action(
+        &env.state,
+        &grant,
+        ActionId::new("email.create_draft"),
+        &crate::test_support::telegram_surface(CHAT_ID),
+        Some(&draft_payload()),
+        FailureSurface::DirectResponse,
+        None,
+    )
+    .await;
+
+    assert!(
+        result.is_err(),
+        "an unconfirmed 5xx write is never reported as a success"
+    );
+    assert_eq!(usage_count(&env.state, "rule-5xx", "reserved"), 1);
+    assert_eq!(usage_count(&env.state, "rule-5xx", "committed"), 0);
+    assert_eq!(
+        env.state
+            .store
+            .standing_rule_remaining("rule-5xx", Timestamp::now())
+            .unwrap(),
+        (4, 2),
+        "a retained reservation keeps consuming quota and rate"
+    );
+    assert_eq!(
+        env.state.store.count_pending_draft_writes().unwrap(),
+        1,
+        "the reconciliation fence stays open on a 5xx"
+    );
+    assert!(audit_count(&env.state, "draft.delivery_unknown") >= 1);
+}

crates/openspine-kernel/src/api/scoped_admission_outcome_tests.rs

@@ -152,7 +152,12 @@ async fn pending_delivery_unknown_fences_scoped_retry_before_reservation() {
 #[tokio::test]
 async fn failure_after_attempt_cancels_reservation() {
     let env = draft_env(&["thread-1"]).await;
-    mount_drafts(&env.api_server, 500, json!({"error": "boom"})).await;
+    mount_drafts(
+        &env.api_server,
+        400,
+        json!({"error": {"code": 400, "message": "invalid draft"}}),
+    )
+    .await;
     let grant = mint_draft_grant(&env.state, "thread-1");
     let context = resolved_context(&env.state, &grant).await;
     env.state

crates/openspine-kernel/src/pipeline/approval_draft.rs

@@ -16,11 +16,15 @@ use openspine_schemas::owner_surface::OwnerSurfaceRef;
 /// unexpired approval. Re-derives the recipient from a live Gmail fetch and
 /// re-checks it against the proposal-bound digest before calling
 /// `create_draft`, because a new thread message can change the recipient.
-/// Candidate Gmail-write extension: a write timeout/no-response leaves
-/// durable pending evidence for manual/operator reconciliation; no automatic
-/// resend is safe without Gmail idempotency.
-/// Returns a truthful outcome distinguishing pre-effect refusal, confirmed
-/// execution, delivery uncertainty, and failure after an attempted write.
+/// Provider-boundary contract: connector admission is taken before the pending
+/// fence and before polling any provider future. A rejected admission is
+/// [`EffectOutcome::RefusedPreEffect`]. Once the write future is polled, a
+/// confirmed Gmail success is [`EffectOutcome::Executed`], a definite client
+/// rejection is [`EffectOutcome::FailedAfterAttempt`], and an outcome that does
+/// not prove non-occurrence (timeout, transport/malformed response, HTTP 429,
+/// or HTTP 5xx) is [`EffectOutcome::DeliveryUnknown`] with durable pending
+/// evidence left open for reconciliation. No automatic resend is safe without
+/// Gmail idempotency.
 pub(crate) async fn create_approved_draft(
     state: &AppState,
     grant: &TaskGrant,
@@ -92,7 +96,9 @@ pub(crate) async fn create_approved_draft(
     .await
     {
         Ok(thread) => thread,
-        Err(DispatchError::ConnectorUnavailable(err)) => return Err(err),
+        Err(DispatchError::ConnectorUnavailable(_)) => {
+            return Ok(EffectOutcome::RefusedPreEffect);
+        }
         Err(err) => {
             state.store.append_audit(
                 "draft.creation_failed",
@@ -199,7 +205,9 @@ pub(crate) async fn create_approved_draft(
         grant,
     ) {
         Ok(permit) => permit,
-        Err(DispatchError::ConnectorUnavailable(err)) => return Err(err),
+        Err(DispatchError::ConnectorUnavailable(_)) => {
+            return Ok(EffectOutcome::RefusedPreEffect);
+        }
         Err(err) => {
             state.store.append_audit(
                 "draft.creation_failed",
@@ -303,10 +311,6 @@ pub(crate) async fn create_approved_draft(
             notify_owner_best_effort(state, owner_surface, "Draft created in Gmail.").await;
             Ok(EffectOutcome::Executed)
         }
-        Err(DispatchError::ConnectorUnavailable(err)) => {
-            state.store.resolve_pending_draft_write(pending_id)?;
-            Err(err)
-        }
         Err(DispatchError::DeliveryUnknown(_)) => unreachable!("handled above"),
         Err(err) => {
             state.store.resolve_pending_draft_write(pending_id)?;

crates/openspine-kernel/src/pipeline/tests/approval_draft_admission_tests.rs

@@ -151,3 +151,75 @@ async fn definite_write_failure_is_failed_after_attempt_and_resolves_the_fence()
         "the fence was recorded before the attempted write"
     );
 }
+
+#[tokio::test]
+async fn provider_5xx_write_is_delivery_unknown_and_leaves_the_fence_open() {
+    // #173: a 502 from drafts.create is NOT a confirmed failure — the draft
+    // may have been created before the intermediary answered — so the outcome
+    // is `DeliveryUnknown`, the pending-write fence stays `pending` (never
+    // resolved), and no retry is auto-sent. Previously a 5xx was collapsed to
+    // `FailedAfterAttempt`, resolving the fence and permitting a duplicate
+    // provider write on the delegated retry path.
+    let token_server = MockServer::start().await;
+    let api_server = MockServer::start().await;
+    Mock::given(method("GET"))
+        .and(path("/gmail/v1/users/me/threads/thread-1"))
+        .respond_with(
+            ResponseTemplate::new(200).set_body_json(thread_with_sender("alice@example.com")),
+        )
+        .mount(&api_server)
+        .await;
+    Mock::given(method("POST"))
+        .and(path("/gmail/v1/users/me/drafts"))
+        .respond_with(ResponseTemplate::new(502).set_body_json(json!({
+            "error": {"code": 502, "message": "bad gateway"}
+        })))
+        .mount(&api_server)
+        .await;
+    let gmail = gmail_with_token_mock(&token_server, &api_server).await;
+    let state = test_state_with_gmail(gmail);
+    let grant = approval_fixture_grant();
+    let request = approval_fixture_request(
+        &state,
+        grant.id,
+        "Re: invoice",
+        "sounds good",
+        "alice@example.com",
+    );
+
+    let outcome = crate::pipeline::approval::create_approved_draft(
+        &state,
+        &grant,
+        &request,
+        &crate::test_support::owner_surface(&state),
+    )
+    .await
+    .unwrap();
+
+    assert_eq!(outcome, EffectOutcome::DeliveryUnknown);
+    assert_eq!(
+        state
+            .store
+            .count_audit_events_of_kind("draft.delivery_unknown")
+            .unwrap(),
+        1
+    );
+    assert_eq!(
+        state
+            .store
+            .count_audit_events_of_kind("draft.creation_failed")
+            .unwrap(),
+        0,
+        "a 5xx is never a confirmed creation failure"
+    );
+    assert_eq!(
+        state.store.count_pending_draft_writes().unwrap(),
+        1,
+        "the reconciliation fence stays open on an unconfirmed write"
+    );
+    assert_eq!(
+        total_pending_draft_write_rows(&state),
+        1,
+        "the fence was recorded before the attempted write and left pending"
+    );
+}

crates/openspine-kernel/src/pipeline/tests/approval_draft_reconcile_tests.rs

@@ -331,13 +331,13 @@ pub(crate) fn total_pending_draft_write_rows(state: &AppState) -> i64 {
 }
 
 #[tokio::test]
-async fn unavailable_gmail_connector_refuses_before_any_fence_row() {
-    // An Open breaker blocks the executor at its FIRST connector call — the
-    // live thread fetch — so it exits before the write is ever admitted and no
-    // pending-write fence row is recorded. Deliberately scoped to that: it
-    // cannot pin write-admission ordering, because a breaker open enough to
-    // reject the write also rejects the preceding fetch. The write-admission
-    // ordering is pinned by the rate-limit test below.
+async fn unavailable_gmail_connector_is_a_pre_effect_refusal_without_a_fence_row() {
+    // #174: an Open breaker blocks the executor at its FIRST connector call —
+    // the live thread fetch — before polling any provider future. The executor
+    // must preserve that known ordering as `RefusedPreEffect`; a bare error is
+    // collapsed to Resource by the scoped caller and permanently retains the
+    // reservation. This test cannot pin the later write-admission arm because
+    // an Open breaker necessarily rejects the preceding fetch first.
     let token_server = MockServer::start().await;
     let api_server = MockServer::start().await;
     // No mocks mounted: the Open breaker must block before any Gmail call.
@@ -356,23 +356,29 @@ async fn unavailable_gmail_connector_refuses_before_any_fence_row() {
         state.connectors.record_connector_outcome("gmail", false);
     }
 
-    let result = crate::pipeline::approval::create_approved_draft(
+    let outcome = crate::pipeline::approval::create_approved_draft(
         &state,
         &grant,
         &request,
         &crate::test_support::owner_surface(&state),
     )
-    .await;
+    .await
+    .expect("pre-effect connector rejection is a typed refusal, not an executor error");
 
-    assert!(
-        result.is_err(),
-        "an unavailable gmail connector propagates as an error, not an outcome: {result:?}"
-    );
+    assert_eq!(outcome, EffectOutcome::RefusedPreEffect);
     assert_eq!(
         total_pending_draft_write_rows(&state),
         0,
         "refusing at the thread fetch must record no pending-write fence"
     );
+    assert_eq!(
+        state
+            .store
+            .count_audit_events_of_kind("connector_unavailable")
+            .unwrap(),
+        1,
+        "admission records exactly one connector_unavailable audit"
+    );
     assert_eq!(
         state
             .store