diff --git a/crates/openspine-kernel/src/api/actions.rs b/crates/openspine-kernel/src/api/actions.rs index 5c5fcc95..66c1cbcc 100644 --- a/crates/openspine-kernel/src/api/actions.rs +++ b/crates/openspine-kernel/src/api/actions.rs @@ -1404,6 +1404,7 @@ async fn mediate_and_dispatch_action_with_attribution_and_token( fired_pending, now, ); + let executor_recorded = dispatched.executor_recorded; match dispatched.result { Ok(result) => Ok((GateDecision::Allow, None, Some(result), standing_budget)), Err(err) => { @@ -1416,6 +1417,17 @@ async fn mediate_and_dispatch_action_with_attribution_and_token( if matches!(err, DispatchError::ConnectorUnavailable(_)) { return Err(err); } + // #244: when the dispatch executor already owns this effect's + // record (it self-appended its typed audit event and, for a + // failure, self-batched it into the owner digest), the mediation + // handler must not re-append `action.dispatch_failed` or re-call + // `batch_failure`. Doing so double-counts one scoped effect and + // files a pre-effect `NotAttempted` refusal under the wrong, + // Connector-class vocabulary. Settlement already ran above off the + // typed disposition, so the reservation is resolved regardless. + if executor_recorded { + return Err(err); + } let digest_class = match &err { DispatchError::Resource(_) | DispatchError::DeliveryUnknown(_) => { FailureClass::Resource @@ -1477,6 +1489,14 @@ async fn mediate_and_dispatch_action_with_attribution_and_token( pub(crate) struct DispatchedEffect { pub(crate) disposition: EffectDisposition, pub(crate) result: Result, + /// The dispatch path's executor already appended its own audit event and, + /// for a failure, already self-batched it into the owner digest. When set, + /// the mediation error handler MUST NOT re-append `action.dispatch_failed` + /// or re-call `batch_failure` — doing so double-counts one effect. The + /// scoped executor owns the record for every disposition it returns; the + /// generic path and the un-recorded scoped arms (registry miss, executor + /// error) leave this false so the handler records them once. + pub(crate) executor_recorded: bool, } #[derive(Debug)] diff --git a/crates/openspine-kernel/src/api/connector_breaker.rs b/crates/openspine-kernel/src/api/connector_breaker.rs index 84c03009..84766a68 100644 --- a/crates/openspine-kernel/src/api/connector_breaker.rs +++ b/crates/openspine-kernel/src/api/connector_breaker.rs @@ -264,6 +264,10 @@ pub(crate) async fn dispatch_allowed_action( DispatchedEffect { disposition: generic_effect_disposition(&result), result, + // The generic path's handlers do not self-record their failures, so the + // mediation error handler owns the `action.dispatch_failed` audit and + // the `batch_failure` call for every error class here. + executor_recorded: false, } } diff --git a/crates/openspine-kernel/src/api/effect_executors.rs b/crates/openspine-kernel/src/api/effect_executors.rs index 7102dd71..05e70b6d 100644 --- a/crates/openspine-kernel/src/api/effect_executors.rs +++ b/crates/openspine-kernel/src/api/effect_executors.rs @@ -66,6 +66,40 @@ pub(crate) enum EffectDisposition { ConfirmedFailure, } +/// An executor failure raised AFTER the executor already committed its typed +/// effect record — the [`EffectDisposition`] was settled and its audit event +/// appended — and only a secondary, fail-closed durable write (the owner-digest +/// batch surfacing) then failed. It carries the already-settled `disposition` +/// so the dispatcher preserves record-once provenance (#244): the error still +/// surfaces (the owner-digest write is fail-closed and redrivable, never +/// silently swallowed), but the mediation error handler must NOT re-append +/// `action.dispatch_failed` or re-`batch_failure` on top of the record the +/// executor already owns. An executor that bailed BEFORE recording anything +/// returns a plain error instead, which the dispatcher treats as un-recorded. +#[derive(Debug)] +pub(crate) struct RecordedEffectError { + /// The disposition the executor settled before the secondary write failed. + pub(crate) disposition: EffectDisposition, + /// The underlying fail-closed error (e.g. the owner-digest store write). + pub(crate) source: anyhow::Error, +} + +impl std::fmt::Display for RecordedEffectError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "effect already recorded as {:?}, but a follow-up fail-closed durable write failed: {}", + self.disposition, self.source + ) + } +} + +impl std::error::Error for RecordedEffectError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(self.source.as_ref()) + } +} + /// Kernel-owned effect executors keyed by their catalog `executor_id`. pub(crate) struct EffectExecutorRegistry { map: HashMap<&'static str, EffectExecutor>, diff --git a/crates/openspine-kernel/src/api/scoped_admission.rs b/crates/openspine-kernel/src/api/scoped_admission.rs index f7868930..33bd4a52 100644 --- a/crates/openspine-kernel/src/api/scoped_admission.rs +++ b/crates/openspine-kernel/src/api/scoped_admission.rs @@ -44,7 +44,7 @@ use ulid::Ulid; use super::actions::{DispatchError, DispatchedEffect}; use super::connector_breaker::call_with_connector; -use super::effect_executors::EffectDisposition; +use super::effect_executors::{EffectDisposition, RecordedEffectError}; use crate::pipeline::AppState; /// The kernel-resolved context for one action plus the digest-bound request @@ -526,36 +526,69 @@ pub(crate) async fn dispatch_scoped_effect( return DispatchedEffect { disposition: EffectDisposition::NotAttempted, result: Err(DispatchError::NoExecutor(admission.request.action.clone())), + // Registry miss: no executor ran, so nothing recorded this effect. + executor_recorded: false, }; }; - match executor(state, grant, &admission.request, owner_surface).await { + dispatched_from_executor(executor(state, grant, &admission.request, owner_surface).await) +} + +/// Map one executor outcome to its [`DispatchedEffect`]. Pure so the record-once +/// provenance (#244) is unit-testable without store fault injection. Every +/// `Ok(..)` disposition is executor-owned: the executor already appended its +/// typed audit event (and, for a failure, self-batched the owner digest), so +/// `executor_recorded` is set and the mediation handler must not re-record. An +/// `Err` splits by provenance: a [`RecordedEffectError`] means the executor had +/// already committed its record and only a follow-up fail-closed durable write +/// failed — surface it (redrivable) but preserve `executor_recorded: true` and +/// the settled disposition; any other error means the executor bailed BEFORE +/// recording anything, so it is un-recorded (`executor_recorded: false`) and, +/// with unknown effect ordering, fails closed as `DeliveryUnknown` (retain). +fn dispatched_from_executor(outcome: anyhow::Result) -> DispatchedEffect { + match outcome { Ok(EffectDisposition::ConfirmedSuccess) => DispatchedEffect { disposition: EffectDisposition::ConfirmedSuccess, result: Ok(json!({"created": true})), + executor_recorded: true, }, Ok(EffectDisposition::DeliveryUnknown) => DispatchedEffect { disposition: EffectDisposition::DeliveryUnknown, result: Err(DispatchError::DeliveryUnknown(anyhow!( "gmail draft write outcome is unknown; the reconciliation fence stays open" ))), + executor_recorded: true, }, Ok(EffectDisposition::NotAttempted) => DispatchedEffect { disposition: EffectDisposition::NotAttempted, result: Err(DispatchError::Connector(anyhow!( "scope-matched draft creation was refused before any provider write" ))), + executor_recorded: true, }, Ok(EffectDisposition::ConfirmedFailure) => DispatchedEffect { disposition: EffectDisposition::ConfirmedFailure, result: Err(DispatchError::Connector(anyhow!( "scope-matched draft creation failed after an attempted write" ))), + executor_recorded: true, }, - // An executor error's effect ordering is unknown; fail closed by - // retaining (DeliveryUnknown) while auditing it truthfully as Resource. - Err(err) => DispatchedEffect { - disposition: EffectDisposition::DeliveryUnknown, - result: Err(DispatchError::Resource(err)), + Err(err) => match err.downcast::() { + // The executor already committed its record; only a follow-up + // fail-closed durable write failed. Surface it for redrive, but the + // record is owned — the mediation handler must not duplicate it. + Ok(recorded) => DispatchedEffect { + disposition: recorded.disposition, + result: Err(DispatchError::Resource(recorded.source)), + executor_recorded: true, + }, + // The executor bailed before writing any audit record; its effect + // ordering is unknown, so fail closed by retaining (DeliveryUnknown) + // while the mediation handler records it truthfully as Resource. + Err(err) => DispatchedEffect { + disposition: EffectDisposition::DeliveryUnknown, + result: Err(DispatchError::Resource(err)), + executor_recorded: false, + }, }, } } @@ -570,6 +603,9 @@ mod effect_truth_tests; #[path = "scoped_admission_outcome_tests.rs"] mod outcome_tests; #[cfg(test)] +#[path = "scoped_admission_parity_tests.rs"] +mod parity_tests; +#[cfg(test)] #[path = "scoped_admission_recheck_tests.rs"] mod recheck_tests; #[cfg(test)] diff --git a/crates/openspine-kernel/src/api/scoped_admission_outcome_tests.rs b/crates/openspine-kernel/src/api/scoped_admission_outcome_tests.rs index 99e6fbf2..69970f74 100644 --- a/crates/openspine-kernel/src/api/scoped_admission_outcome_tests.rs +++ b/crates/openspine-kernel/src/api/scoped_admission_outcome_tests.rs @@ -188,6 +188,25 @@ async fn failure_after_attempt_cancels_reservation() { 0, "a confirmed failure resolves the fence" ); + // #244: the scoped executor owns the record for its `ConfirmedFailure`, so + // the failure is recorded exactly once — one `draft.creation_failed` audit + // row and one owner-digest row — and the mediation handler no longer + // re-files it under the generic `action.dispatch_failed` vocabulary. + assert_eq!( + audit_count(&env.state, "draft.creation_failed"), + 1, + "the executor self-audits the failure exactly once" + ); + assert_eq!( + audit_count(&env.state, "action.dispatch_failed"), + 0, + "the mediation handler must not re-audit an executor-owned failure" + ); + assert_eq!( + digest_count(&env.state), + 1, + "a failed scoped draft surfaces exactly one owner-digest row" + ); } /// Acceptance: a pre-effect refusal inside the executor cancels the @@ -282,6 +301,25 @@ async fn pre_effect_refusal_cancels_reservation_without_writing() { 0, "a refusal records no pending-write fence" ); + // #244: a target-mutation refusal is a pre-effect `NotAttempted`. The + // executor audits it under its own vocabulary exactly once; the mediation + // handler must not re-file it as a generic `action.dispatch_failed`, and it + // must not surface as a Connector-class owner-digest failure at all. + assert_eq!( + audit_count(&state, "draft.target_mutated_since_approval"), + 1, + "the executor audits the pre-effect refusal exactly once" + ); + assert_eq!( + audit_count(&state, "action.dispatch_failed"), + 0, + "a pre-effect refusal is not re-filed as a generic dispatch failure" + ); + assert_eq!( + digest_count(&state), + 0, + "a pre-effect refusal is not batched as a Connector-class digest failure" + ); } /// Task 3 boundary: a construction failure leaves the decision at diff --git a/crates/openspine-kernel/src/api/scoped_admission_parity_tests.rs b/crates/openspine-kernel/src/api/scoped_admission_parity_tests.rs new file mode 100644 index 00000000..9f28baed --- /dev/null +++ b/crates/openspine-kernel/src/api/scoped_admission_parity_tests.rs @@ -0,0 +1,281 @@ +//! #244 parity: a failed `email.create_draft` must produce the SAME durable +//! record shape whether it flows through the scope-matched standing-rule path +//! (`mediate_and_dispatch_action` -> `dispatch_scoped_effect`) or the +//! interactive-approval path (which calls the shared executor directly and +//! discards the outcome, `pipeline/post_approval.rs:80`). Before #244 the +//! scoped path double-recorded: the executor self-audited/self-batched, and the +//! mediation error handler then re-appended `action.dispatch_failed` and +//! re-called `batch_failure`. Both paths must now record exactly once. +use jiff::Timestamp; +use openspine_schemas::action::{ActionId, ActionRequest}; +use openspine_schemas::digest::digest_of; +use openspine_schemas::event::{TargetRef, TargetRefKind}; +use serde_json::json; +use ulid::Ulid; + +use super::dispatched_from_executor; +use super::scoped_admission_support::*; +use crate::api::actions::{mediate_and_dispatch_action, DispatchError, FailureSurface}; +use crate::api::effect_executors::{EffectDisposition, RecordedEffectError}; + +/// One failure's durable footprint: the two audit vocabularies that could +/// record it and the owner-digest row count. +#[derive(Debug, PartialEq, Eq)] +struct RecordShape { + creation_failed: i64, + dispatch_failed: i64, + digest_rows: usize, +} + +/// Drive a scoped `ConfirmedFailure` through the production mediation path +/// against an HTTP 400 draft response and read back its record footprint. +async fn scoped_confirmed_failure_shape() -> RecordShape { + let env = draft_env(&["thread-1"]).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 + .store + .activate_standing_rule( + &scoped_manifest("rule-parity", &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(), "a 400 draft write is a confirmed failure"); + + RecordShape { + creation_failed: audit_count(&env.state, "draft.creation_failed"), + dispatch_failed: audit_count(&env.state, "action.dispatch_failed"), + digest_rows: digest_count(&env.state), + } +} + +/// Drive the same `ConfirmedFailure` through the interactive-approval path, +/// which invokes the shared executor directly and discards its outcome exactly +/// as `handle_create_approved_draft` does, then read back its footprint. +async fn interactive_confirmed_failure_shape() -> RecordShape { + let env = draft_env(&["thread-1"]).await; + mount_drafts( + &env.api_server, + 400, + json!({"error": {"code": 400, "message": "invalid draft"}}), + ) + .await; + let grant = mint_draft_grant(&env.state, "thread-1"); + // The executor recomputes the target digest from the thread's newest + // non-owner recipient (`alice@example.com`, per `draft_env`); binding the + // request to that exact recipient reaches the provider write instead of a + // pre-effect target-mutation refusal. + let payload_ref = env + .state + .artifacts + .put(serde_json::to_vec(&draft_payload()).unwrap().as_slice()) + .unwrap(); + let target_digest = digest_of(&json!({ + "thread_id": "thread-1", + "connector": "gmail_primary", + "account_role": "owner_mailbox", + "recipients": ["alice@example.com"], + })); + let request = ActionRequest { + id: Ulid::new(), + task_grant_id: grant.id, + action: ActionId::new("email.create_draft"), + target_ref: Some(TargetRef { + kind: TargetRefKind::EmailThread, + id: Some("thread-1".to_string()), + }), + payload_ref: Some(payload_ref), + target_digest: Some(target_digest), + selection_token_id: None, + params: std::collections::BTreeMap::new(), + skill_attribution: None, + requested_at: Timestamp::now(), + schema_version: 1, + }; + + let outcome = crate::pipeline::gmail_create_draft_executor( + &env.state, + &grant, + &request, + &crate::test_support::owner_surface_for(&env.state, CHAT_ID), + ) + .await + .unwrap(); + assert_eq!( + outcome, + EffectDisposition::ConfirmedFailure, + "a 400 draft write is a confirmed failure on the interactive path too" + ); + + RecordShape { + creation_failed: audit_count(&env.state, "draft.creation_failed"), + dispatch_failed: audit_count(&env.state, "action.dispatch_failed"), + digest_rows: digest_count(&env.state), + } +} + +/// Acceptance (#244): one failed scoped draft and one failed interactive draft +/// produce the identical single-record footprint — one `draft.creation_failed` +/// audit row, zero `action.dispatch_failed` rows, and one owner-digest row. +#[tokio::test] +async fn scoped_and_interactive_confirmed_failure_record_shape_is_identical() { + let scoped = scoped_confirmed_failure_shape().await; + let interactive = interactive_confirmed_failure_shape().await; + + let expected = RecordShape { + creation_failed: 1, + dispatch_failed: 0, + digest_rows: 1, + }; + assert_eq!( + scoped, expected, + "the scoped path records the failure exactly once" + ); + assert_eq!( + interactive, expected, + "the interactive path records the failure exactly once" + ); + assert_eq!( + scoped, interactive, + "both admission paths must produce the same record shape" + ); +} + +/// #244 provenance mapping (dispatcher half): a [`RecordedEffectError`] — the +/// executor already committed its typed record and only a follow-up fail-closed +/// durable write failed — must map to `executor_recorded: true` carrying the +/// settled disposition, so the mediation handler skips the duplicate audit/batch +/// while the failure stays surfaced for redrive. +#[test] +fn recorded_effect_error_maps_to_executor_owned_surfaced_failure() { + let dispatched = dispatched_from_executor(Err(RecordedEffectError { + disposition: EffectDisposition::ConfirmedFailure, + source: anyhow::anyhow!("owner-digest store write failed after settle"), + } + .into())); + assert_eq!(dispatched.disposition, EffectDisposition::ConfirmedFailure); + assert!( + dispatched.executor_recorded, + "the executor already owns the record; mediation must not duplicate it" + ); + assert!( + matches!(dispatched.result, Err(DispatchError::Resource(_))), + "the fail-closed digest error stays surfaced for redrive" + ); +} + +/// A plain executor error (bailed BEFORE recording anything) must stay +/// un-recorded so the mediation handler records the failure exactly once, and +/// fail closed as `DeliveryUnknown` because the effect ordering is unknown. +#[test] +fn plain_executor_error_stays_unrecorded_and_fails_closed() { + let dispatched = dispatched_from_executor(Err(anyhow::anyhow!( + "artifact read failed before any audit" + ))); + assert_eq!(dispatched.disposition, EffectDisposition::DeliveryUnknown); + assert!( + !dispatched.executor_recorded, + "nothing was recorded; the mediation handler must record it once" + ); +} + +/// #244 fail-closed provenance (executor half): when the executor's fail-closed +/// owner-digest write fails AFTER it already settled the effect and appended +/// `draft.creation_failed`, `create_approved_draft` surfaces a +/// [`RecordedEffectError`] — not a bare `Err` (which would make the dispatcher +/// re-record) and not a swallowed success (which would drop the digest with no +/// redrive). The primary failure audit exists exactly once and no digest row is +/// falsely claimed. Uses the artifact store's existing `set_fault_put_for_test` +/// seam to fail the digest write's encrypt round-trip. +#[tokio::test] +async fn digest_write_failure_after_settle_surfaces_recorded_effect_error() { + let env = draft_env(&["thread-1"]).await; + mount_drafts( + &env.api_server, + 400, + json!({"error": {"code": 400, "message": "invalid draft"}}), + ) + .await; + let grant = mint_draft_grant(&env.state, "thread-1"); + // Recipient matches `draft_env`'s newest non-owner participant so the + // executor's re-derivation reaches the provider write (a 400 confirmed + // failure) instead of a pre-effect refusal. + let payload_ref = env + .state + .artifacts + .put(serde_json::to_vec(&draft_payload()).unwrap().as_slice()) + .unwrap(); + let target_digest = digest_of(&json!({ + "thread_id": "thread-1", + "connector": "gmail_primary", + "account_role": "owner_mailbox", + "recipients": ["alice@example.com"], + })); + let request = ActionRequest { + id: Ulid::new(), + task_grant_id: grant.id, + action: ActionId::new("email.create_draft"), + target_ref: Some(TargetRef { + kind: TargetRefKind::EmailThread, + id: Some("thread-1".to_string()), + }), + payload_ref: Some(payload_ref), + target_digest: Some(target_digest), + selection_token_id: None, + params: std::collections::BTreeMap::new(), + skill_attribution: None, + requested_at: Timestamp::now(), + schema_version: 1, + }; + + // Fail the owner-digest write's encrypt round-trip (a SYSTEM_SCOPE `put`). + // The payload blob above is already stored, and the confirmed-failure path + // performs no other SYSTEM_SCOPE `put` before `batch_failure`. + env.state.artifacts.set_fault_put_for_test(true); + + let outcome = crate::pipeline::gmail_create_draft_executor( + &env.state, + &grant, + &request, + &crate::test_support::owner_surface_for(&env.state, CHAT_ID), + ) + .await; + + let err = outcome.expect_err("a fail-closed digest write must surface an error"); + let recorded = err + .downcast_ref::() + .expect("the error must carry record-once provenance, not a bare store error"); + assert_eq!( + recorded.disposition, + EffectDisposition::ConfirmedFailure, + "the settled disposition travels with the error" + ); + assert_eq!( + audit_count(&env.state, "draft.creation_failed"), + 1, + "the primary failure audit was committed before the digest write failed" + ); + assert_eq!( + digest_count(&env.state), + 0, + "the fail-closed digest write left no row" + ); +} diff --git a/crates/openspine-kernel/src/api/scoped_admission_support.rs b/crates/openspine-kernel/src/api/scoped_admission_support.rs index 4c9de09e..8ea26180 100644 --- a/crates/openspine-kernel/src/api/scoped_admission_support.rs +++ b/crates/openspine-kernel/src/api/scoped_admission_support.rs @@ -337,6 +337,10 @@ pub(super) fn audit_count(state: &AppState, event: &str) -> i64 { .unwrap() } +pub(super) fn digest_count(state: &AppState) -> usize { + state.store.owner_digest_items().unwrap().len() +} + pub(super) async fn dispatch(state: &AppState, grant: &TaskGrant) -> (GateDecision, Option) { let result = mediate_and_dispatch_action( state, diff --git a/crates/openspine-kernel/src/disclosure/disclosure_messaging_tests.rs b/crates/openspine-kernel/src/disclosure/disclosure_messaging_tests.rs index cf2a81a1..b91a2df9 100644 --- a/crates/openspine-kernel/src/disclosure/disclosure_messaging_tests.rs +++ b/crates/openspine-kernel/src/disclosure/disclosure_messaging_tests.rs @@ -104,6 +104,7 @@ fn private_section() -> BriefcaseSection { visibility: VisibilityClass::WorkerScratch, depth: 0, disclosure_class: Some(DisclosureClass::Private), + origin: None, payload: json!("condition X"), } } diff --git a/crates/openspine-kernel/src/pipeline/approval_draft.rs b/crates/openspine-kernel/src/pipeline/approval_draft.rs index 6561916c..04a55eef 100644 --- a/crates/openspine-kernel/src/pipeline/approval_draft.rs +++ b/crates/openspine-kernel/src/pipeline/approval_draft.rs @@ -1,6 +1,6 @@ use crate::api::actions::DispatchError; use crate::api::connector_breaker::call_with_connector; -use crate::api::effect_executors::EffectDisposition; +use crate::api::effect_executors::{EffectDisposition, RecordedEffectError}; use crate::artifact_store::ArtifactStoreError; use openspine_schemas::action::ActionRequest; use openspine_schemas::artifact::ArtifactRef; @@ -13,6 +13,36 @@ use super::{notify_owner_best_effort, AppState}; use crate::store::{AuditDescriptor, BeginEffect, PendingWriteFence}; use openspine_schemas::owner_surface::OwnerSurfaceRef; +/// Record the owner-digest surfacing for a failure the executor has ALREADY +/// committed (its typed audit/settle succeeded), then return the settled +/// `disposition`. The owner-digest write is deliberately fail-closed +/// (`failure_surfacing::batch_failure`): on error it is NOT swallowed — a +/// dropped digest would leave no row and no redrive — but it must not be +/// propagated as a bare `Err`, because `dispatch_scoped_effect` would then map +/// it to `executor_recorded: false` and the mediation handler would re-append +/// `action.dispatch_failed` and re-batch, double-counting one failure (#244). +/// Instead it is surfaced as a [`RecordedEffectError`] carrying the settled +/// disposition, so the failure stays visible and redrivable while the mediation +/// handler skips the duplicate audit/batch. The invariant this preserves: +/// `create_approved_draft` returns a bare `Err` only from PRE-record failures; +/// any error after a committed failure audit is a `RecordedEffectError`. +fn batch_then_record( + state: &AppState, + disposition: EffectDisposition, + class: crate::failure_surfacing::FailureClass, + summary: &str, + detail: &str, +) -> anyhow::Result { + match crate::failure_surfacing::batch_failure(state, class, summary, detail) { + Ok(()) => Ok(disposition), + Err(err) => Err(RecordedEffectError { + disposition, + source: err.into(), + } + .into()), + } +} + /// Actually create the Gmail draft after `gate()` confirms a matching, /// unexpired approval. Re-derives the recipient from a live Gmail fetch and /// re-checks it against the proposal-bound digest before calling @@ -78,13 +108,13 @@ pub(crate) async fn create_approved_draft( &[], &[], )?; - crate::failure_surfacing::batch_failure( + return batch_then_record( state, + EffectDisposition::NotAttempted, crate::failure_surfacing::FailureClass::Connector, "gmail connector unavailable during approval", "gmail connector unavailable during approval", - )?; - return Ok(EffectDisposition::NotAttempted); + ); }; crate::spend::guard_connector_for(state, grant).await?; @@ -111,13 +141,13 @@ pub(crate) async fn create_approved_draft( &[], &[], )?; - crate::failure_surfacing::batch_failure( + return batch_then_record( state, + EffectDisposition::NotAttempted, crate::failure_surfacing::FailureClass::Connector, "gmail thread fetch failed during approval", &format!("{err:?}"), - )?; - return Ok(EffectDisposition::NotAttempted); + ); } }; let Some(target) = crate::gmail::newest_non_owner_recipient(&thread, gmail.mailbox_address()) @@ -220,13 +250,13 @@ pub(crate) async fn create_approved_draft( &[], std::slice::from_ref(payload_ref), )?; - crate::failure_surfacing::batch_failure( + return batch_then_record( state, + EffectDisposition::NotAttempted, crate::failure_surfacing::FailureClass::Connector, "gmail create draft admission rejected during approval", &format!("{err:?}"), - )?; - return Ok(EffectDisposition::NotAttempted); + ); } }; // Candidate Gmail-write extension: persist durable pending evidence before @@ -336,13 +366,13 @@ pub(crate) async fn create_approved_draft( state .store .settle_effect(fence, EffectDisposition::ConfirmedFailure, audit)?; - crate::failure_surfacing::batch_failure( + batch_then_record( state, + EffectDisposition::ConfirmedFailure, crate::failure_surfacing::FailureClass::Connector, "gmail create draft failed during approval", &format!("{err:?}"), - )?; - Ok(EffectDisposition::ConfirmedFailure) + ) } } }