Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions crates/openspine-kernel/src/api/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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
Expand Down Expand Up @@ -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<Value, DispatchError>,
/// 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)]
Expand Down
4 changes: 4 additions & 0 deletions crates/openspine-kernel/src/api/connector_breaker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
34 changes: 34 additions & 0 deletions crates/openspine-kernel/src/api/effect_executors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
50 changes: 43 additions & 7 deletions crates/openspine-kernel/src/api/scoped_admission.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<EffectDisposition>) -> 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::<RecordedEffectError>() {
// 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,
},
},
}
}
Expand All @@ -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)]
Expand Down
38 changes: 38 additions & 0 deletions crates/openspine-kernel/src/api/scoped_admission_outcome_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading