Provably pre-effect ConnectorUnavailable rejection permanently retains the scoped standing-rule reservation

[George-RD]

What to build

ConnectorUnavailable rejections are classified as ConfirmedFailure but they are pre-effect: no attempt was made and the service was not reached. They should be NotAttempted to permit retry. Document the connector boundary contract and fix the classification.

Acceptance criteria

1. Classify ConnectorUnavailable as NotAttempted, not ConfirmedFailure. 2. Add regression test: a ConnectorUnavailable leaves the pending row pending and permits retry on the next attempt. 3. Document the provider contract boundary.

Blocked by

None (T1 characterization; sub-issue of epic #198)

Original report

Found by an adversarial correctness/security review of #130 (ship-recurring-gmail-draft-proof) before merge. Labelled FOLLOW-UP-ISSUE: the direction is conservative (over-consumes the owner's budget rather than over-granting), so it is a delegation-accounting defect rather than an unsafe one.

Defect

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

Err(DispatchError::ConnectorUnavailable(err)) => return Err(err),

This leaves the typed EffectOutcome channel and surfaces as a bare anyhow::Error. crates/openspine-kernel/src/api/scoped_admission.rs:537 maps every executor Err to DispatchError::Resource(err), and crates/openspine-kernel/src/api/actions.rs:1200-1201 classifies Resource as retain_reservation = true.

Why it is wrong

admit_connector_write's own contract says the opposite (connector_breaker.rs:169-172): "An admission rejection is provably pre-effect — no connector future has been polled — so a caller that separates the steps can report it as a refusal rather than as a failed write." The effect ordering here is known. Yet the scoped lane retains a reserved quota row and a reserved rate row against the owner's reviewed rule for an effect that provably never happened, and refreshes the lapse clock via note_standing_rule_use (actions.rs:1211-1215). The change's own contract table at scoped_admission.rs:453-459 promises RefusedPreEffect -> Connector -> cancel, and actions.rs:1170-1180 records that no reconciler exists, so a retained row can never be released.

Reachable whenever the breaker transitions to Open/HalfOpen between the read-only thread fetch and the write admission. crate::spend::guard_connector_for at approval_draft.rs:188 takes the same route.

Corroborating: the post-write Err(DispatchError::ConnectorUnavailable(err)) arm at approval_draft.rs:306-309 is dead — call_with_admitted_connector_write can only return DeliveryUnknown or the output of map_write_error.

Suggested fix

Return EffectOutcome::RefusedPreEffect (after the connector_unavailable audit map_admission_error already writes) instead of Err(err) at approval_draft.rs:202, so the scoped lane cancels rather than retains. Remove the dead arm at :306-309. Add a regression test asserting an unavailable connector cancels the reservation and restores full budget.

[coderabbitai]

Coding Plan

Summary

• Change the admission-time ConnectorUnavailable arm in create_approved_draft to return Ok(EffectOutcome::RefusedPreEffect), so the scoped lane cancels the reservation and restores budget instead of retaining it.
• Rely on the existing connector_unavailable audit write in map_admission_error. Add no new audit.
• Remove the dead post-write ConnectorUnavailable arm, which cannot execute because call_with_admitted_connector_write never returns that variant.
• Add a regression test in scoped_admission_outcome_tests.rs that trips the Gmail breaker before write admission and asserts zero reserved and committed usage, full budget restoration, no pending-write fence, no provider write, and exactly one connector_unavailable audit event.

Design Choices

Design Choice 1: Which dispatch lane the regression test targets

Options Considered:

1. Test only the scoped lane, because it holds the reservation that the ticket says is retained.
2. Also test the non-scoped post_approval.rs lanes for the changed error-propagation behavior.

Chosen Option: 1

Rationale: The defect is a reservation-accounting defect. Only the scoped lane holds reservations. The regression test targets the scoped lane. The non-scoped lane has no reservation to assert against. The non-scoped lane only changes from a propagated error to a swallowed refusal. This matches the existing behavior of the other RefusedPreEffect arms in the executor.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Correct the pre-effect ConnectorUnavailable outcome in the draft executor

This phase changes create_approved_draft so a provably pre-effect admission rejection reports a refusal through the typed EffectOutcome channel instead of a bare error. This lets the scoped lane cancel the reservation and restore budget. This phase also removes the confirmed dead post-write arm.

Task 1: Return `RefusedPreEffect` for the admission-time `ConnectorUnavailable` rejection

Change the admission-time match arm so an unavailable connector is reported as a pre-effect refusal.

• Update crates/openspine-kernel/src/pipeline/approval_draft.rs in create_approved_draft.
• Change the admission-time match arm on the admit_connector_write result. The current arm returns Err(err) for Err(DispatchError::ConnectorUnavailable(err)).
• Return Ok(EffectOutcome::RefusedPreEffect) from this arm instead.
• Do not add a new audit write. map_admission_error already wrote the connector_unavailable audit event before it returned ConnectorUnavailable.
• Confirm this arm runs before the durable pending_draft_write row insert, so no fence is recorded for the refusal.
• Rationale: the scoped lane maps RefusedPreEffect to DispatchError::Connector, which sets retain_reservation = false and cancels the reservation, matching the documented contract table in scoped_admission.rs.

Task 2: Remove the dead post-write `ConnectorUnavailable` arm

Remove the unreachable arm that cannot execute given the possible return values of the write call.

• Update crates/openspine-kernel/src/pipeline/approval_draft.rs in the match draft_result block.
• Remove the Err(DispatchError::ConnectorUnavailable(err)) => { ... Err(err) } arm.
• Verified basis: call_with_admitted_connector_write in connector_breaker.rs can only return Ok(_), Err(DispatchError::DeliveryUnknown(_)), or the output of map_write_error, and map_write_error returns only DeliveryUnknown or Connector. It never returns ConnectorUnavailable.
• Keep the exhaustiveness of the match valid. The Err(err) catch-all arm continues to cover the remaining DispatchError variants. Confirm the compiler still accepts the match after removal.
• Leave the existing Err(DispatchError::DeliveryUnknown(_)) => unreachable!("handled above") arm unchanged, because the early DeliveryUnknown return above the match still requires it for exhaustiveness.

🤖 Prompt for AI agents

Implement changes to `create_approved_draft` in
`crates/openspine-kernel/src/pipeline/approval_draft.rs` so a pre-effect
admission rejection reports through the typed `EffectOutcome` channel and the
dead post-write arm is removed.

1. Correct the pre-effect ConnectorUnavailable outcome:
- Locate the admission-time match arm on the `admit_connector_write` result. The
current arm returns `Err(err)` for
`Err(DispatchError::ConnectorUnavailable(err))`.
- Change this arm to return `Ok(EffectOutcome::RefusedPreEffect)` instead of
`Err(err)`.
- Do not add a new audit write. `map_admission_error` already writes the
`connector_unavailable` audit event before returning `ConnectorUnavailable`.
- Confirm this arm executes before the durable `pending_draft_write` row insert,
so no fence is recorded for the refusal.
- This change ensures the scoped lane maps `RefusedPreEffect` to
`DispatchError::Connector`, which sets `retain_reservation = false` and cancels
the reservation.

2. Remove the dead post-write ConnectorUnavailable arm:
- In the `match draft_result` block, remove the
`Err(DispatchError::ConnectorUnavailable(err)) => { ... Err(err) }` arm. This
arm is unreachable because `call_with_admitted_connector_write` can only return
`Ok(_)`, `Err(DispatchError::DeliveryUnknown(_))`, or the output of
`map_write_error` (which returns only `DeliveryUnknown` or `Connector`), never
`ConnectorUnavailable`.
- Keep the `match` exhaustive after removal. The `Err(err)` catch-all arm must
continue to cover the remaining `DispatchError` variants. Verify the code
compiles after removal.
- Leave the `Err(DispatchError::DeliveryUnknown(_)) => unreachable!("handled
above")` arm unchanged, since the early `DeliveryUnknown` return above the match
still requires it for exhaustiveness.

Phase 2: Add a regression test for reservation cancellation on connector-unavailable

This phase adds a test that proves a pre-effect ConnectorUnavailable rejection cancels the scoped standing-rule reservation and restores full budget, and does not leave a pending-write fence.

Add the connector-unavailable reservation-cancellation regression test

Add a test that trips the breaker before write admission and asserts full reservation cancellation.

• Add the test to crates/openspine-kernel/src/api/scoped_admission_outcome_tests.rs.
• Use the harness from scoped_admission_support.rs: draft_env, mint_draft_grant, resolved_context, scoped_manifest, dispatch, usage_count, drafts_written, and audit_count.
• Model the test on scoped_no_executor_cancels_reservation_and_restores_budget and pre_effect_refusal_cancels_reservation_without_writing.
• Setup: activate a scoped standing rule, then take a before budget snapshot with state.store.standing_rule_remaining(rule_id, now).
• Trip the Gmail breaker to Open before dispatch. Call state.connectors.record_connector_outcome("gmail", false) the default failure threshold number of times (3).
• Drive the dispatch through mediate_and_dispatch_action for email.create_draft, the same path the existing outcome tests use.
• Assertions:
  • usage_count(&state, rule_id, "reserved") == 0 and usage_count(&state, rule_id, "committed") == 0.
  • standing_rule_remaining(rule_id, now) equals the before snapshot, proving full quota and rate restoration.
  • state.store.count_pending_draft_writes() == 0, proving no fence was recorded.
  • drafts_written(...) == 0, proving no provider write occurred.
  • state.store.count_audit_events_of_kind("connector_unavailable") equals 1, proving the admission audit fired exactly once.
  • state.store.count_audit_events_of_kind("action.dispatch_failed") equals 0, confirming no double audit.
• Note on timing: trip the breaker after the read-only thread fetch admission path or ensure the setup reaches the write-admission point. The breaker must be Open at the admit_connector_write call inside create_approved_draft. If the read-side guard_connector_for and thread fetch use a different gate, verify the mock still lets the flow reach the write admission. Adjust the mock so the read step succeeds and the write admission rejects with ConnectorUnavailable.

🤖 Prompt for AI agents

Add a regression test to
`crates/openspine-kernel/src/api/scoped_admission_outcome_tests.rs` that proves
a pre-effect `ConnectorUnavailable` rejection cancels the scoped standing-rule
reservation and restores full budget.

- Use the test harness from `scoped_admission_support.rs`: `draft_env`,
`mint_draft_grant`, `resolved_context`, `scoped_manifest`, `dispatch`,
`usage_count`, `drafts_written`, and `audit_count`.
- Model the test structure on
`scoped_no_executor_cancels_reservation_and_restores_budget` and
`pre_effect_refusal_cancels_reservation_without_writing`.
- Setup steps:
  - Activate a scoped standing rule.
- Take a `before` budget snapshot using
`state.store.standing_rule_remaining(rule_id, now)`.
- Trip the Gmail breaker to Open before dispatch by calling
`state.connectors.record_connector_outcome("gmail", false)` the default failure
threshold number of times (3).
- Drive the dispatch through `mediate_and_dispatch_action` for
`email.create_draft`, matching the path used by the existing outcome tests.
- Add these assertions:
  - `usage_count(&state, rule_id, "reserved") == 0`.
  - `usage_count(&state, rule_id, "committed") == 0`.
- `standing_rule_remaining(rule_id, now)` equals the `before` snapshot, proving
full quota and rate restoration.
- `state.store.count_pending_draft_writes() == 0`, proving no fence was
recorded.
  - `drafts_written(...) == 0`, proving no provider write occurred.
- `state.store.count_audit_events_of_kind("connector_unavailable")` equals 1,
proving the admission audit fired exactly once.
- `state.store.count_audit_events_of_kind("action.dispatch_failed")` equals 0,
confirming no double audit.
- Handle timing carefully: the breaker must be Open at the
`admit_connector_write` call inside `create_approved_draft`. If the read-side
`guard_connector_for` and thread fetch use a different gate, verify the mock
still lets the flow reach the write admission. Adjust the mock so the read step
succeeds and the write admission rejects with `ConnectorUnavailable`.

Research

The openspine-kernel crate runs Gmail draft creation through create_approved_draft in crates/openspine-kernel/src/pipeline/approval_draft.rs. This executor returns a typed EffectOutcome (Executed, RefusedPreEffect, DeliveryUnknown, FailedAfterAttempt).

Two lanes call the executor:

• The scoped lane (crates/openspine-kernel/src/api/scoped_admission.rs::dispatch_scoped_effect) maps EffectOutcome to DispatchError and drives standing-rule reservation cancel/retain decisions in crates/openspine-kernel/src/api/actions.rs::mediate_and_dispatch_action.
• The non-scoped lane (crates/openspine-kernel/src/pipeline/post_approval.rs) holds no reservation and discards or only special-cases the outcome.

admit_connector_write and map_admission_error in crates/openspine-kernel/src/api/connector_breaker.rs produce DispatchError::ConnectorUnavailable and already write the connector_unavailable audit event.

Reservation-outcome tests live in crates/openspine-kernel/src/api/scoped_admission_outcome_tests.rs and use the harness in scoped_admission_support.rs.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement changes to `create_approved_draft` in
`crates/openspine-kernel/src/pipeline/approval_draft.rs` so a pre-effect
admission rejection reports through the typed `EffectOutcome` channel and the
dead post-write arm is removed.

1. Correct the pre-effect ConnectorUnavailable outcome:
- Locate the admission-time match arm on the `admit_connector_write` result. The
current arm returns `Err(err)` for
`Err(DispatchError::ConnectorUnavailable(err))`.
- Change this arm to return `Ok(EffectOutcome::RefusedPreEffect)` instead of
`Err(err)`.
- Do not add a new audit write. `map_admission_error` already writes the
`connector_unavailable` audit event before returning `ConnectorUnavailable`.
- Confirm this arm executes before the durable `pending_draft_write` row insert,
so no fence is recorded for the refusal.
- This change ensures the scoped lane maps `RefusedPreEffect` to
`DispatchError::Connector`, which sets `retain_reservation = false` and cancels
the reservation.

2. Remove the dead post-write ConnectorUnavailable arm:
- In the `match draft_result` block, remove the
`Err(DispatchError::ConnectorUnavailable(err)) => { ... Err(err) }` arm. This
arm is unreachable because `call_with_admitted_connector_write` can only return
`Ok(_)`, `Err(DispatchError::DeliveryUnknown(_))`, or the output of
`map_write_error` (which returns only `DeliveryUnknown` or `Connector`), never
`ConnectorUnavailable`.
- Keep the `match` exhaustive after removal. The `Err(err)` catch-all arm must
continue to cover the remaining `DispatchError` variants. Verify the code
compiles after removal.
- Leave the `Err(DispatchError::DeliveryUnknown(_)) => unreachable!("handled
above")` arm unchanged, since the early `DeliveryUnknown` return above the match
still requires it for exhaustiveness.
===============================================================================

Task: 2

Add a regression test to
`crates/openspine-kernel/src/api/scoped_admission_outcome_tests.rs` that proves
a pre-effect `ConnectorUnavailable` rejection cancels the scoped standing-rule
reservation and restores full budget.

- Use the test harness from `scoped_admission_support.rs`: `draft_env`,
`mint_draft_grant`, `resolved_context`, `scoped_manifest`, `dispatch`,
`usage_count`, `drafts_written`, and `audit_count`.
- Model the test structure on
`scoped_no_executor_cancels_reservation_and_restores_budget` and
`pre_effect_refusal_cancels_reservation_without_writing`.
- Setup steps:
  - Activate a scoped standing rule.
- Take a `before` budget snapshot using
`state.store.standing_rule_remaining(rule_id, now)`.
- Trip the Gmail breaker to Open before dispatch by calling
`state.connectors.record_connector_outcome("gmail", false)` the default failure
threshold number of times (3).
- Drive the dispatch through `mediate_and_dispatch_action` for
`email.create_draft`, matching the path used by the existing outcome tests.
- Add these assertions:
  - `usage_count(&state, rule_id, "reserved") == 0`.
  - `usage_count(&state, rule_id, "committed") == 0`.
- `standing_rule_remaining(rule_id, now)` equals the `before` snapshot, proving
full quota and rate restoration.
- `state.store.count_pending_draft_writes() == 0`, proving no fence was
recorded.
  - `drafts_written(...) == 0`, proving no provider write occurred.
- `state.store.count_audit_events_of_kind("connector_unavailable")` equals 1,
proving the admission audit fired exactly once.
- `state.store.count_audit_events_of_kind("action.dispatch_failed")` equals 0,
confirming no double audit.
- Handle timing carefully: the breaker must be Open at the
`admit_connector_write` call inside `create_approved_draft`. If the read-side
`guard_connector_for` and thread fetch use a different gate, verify the mock
still lets the flow reach the write admission. Adjust the mock so the read step
succeeds and the write admission rejects with `ConnectorUnavailable`.

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!

[George-RD]

Shape note from the 2026-08-20 architecture review (fold ticket #183, wayfinder map #182): this retention bug is the NotAttempted quadrant of disposition-driven settlement — a provably pre-effect refusal must cancel its reservation, and that rule should live in one settlement module keyed on typed disposition, not in caller-side error interpretation (see the fuller note on #173). Naming the reservation — today a bare (String, u32, String) tuple in four modules — is the enabling step: a typed reservation can carry its own settle-on-disposition semantics, making "refused pre-effect but retained" unrepresentable.

[George-RD]

Restructured into the cohesive roadmap (map #182); original preserved below.