Skip to content

Store: define begin_effect / settle_effect(disposition) settlement seam (#216) - #248

Merged
George-RD merged 1 commit into
mainfrom
George-RD/dev-216
Aug 21, 2026
Merged

Store: define begin_effect / settle_effect(disposition) settlement seam (#216)#248
George-RD merged 1 commit into
mainfrom
George-RD/dev-216

Conversation

@George-RD

@George-RD George-RD commented Aug 21, 2026

Copy link
Copy Markdown
Owner

What

Defines the two-phase settlement seam SHAPE for non-rollbackable external
effects (spec #208 D-003), and pilots it on the Gmail draft pending-write fence.

  • Store::begin_effect(PendingWriteFence, AuditDescriptor) -> BeginEffect:
    claims the pending-write fence and appends its audit row in one
    Immediate transaction. Returns BeginEffect::Fenced(EffectFence) on a
    successful claim, or BeginEffect::AlreadyFenced (no write) on a lost
    concurrent claim — matching claim_pending_draft_write's contract.
  • Store::settle_effect(EffectFence, EffectDisposition, AuditDescriptor):
    routes on the typed EffectDisposition delivered by Effect Truth B+C: typed EffectDisposition and disposition-driven settlement (T2+T3) #238 — the same
    finalize / cancel / retain authority as settle_reservations:
    • ConfirmedSuccess → finalize (resolve the fence)
    • ConfirmedFailure / NotAttempted → cancel (resolve the fence)
    • DeliveryUnknown → retain + fence (leave the row pending; no duplicate send)
      Each transition is paired with its audit row in one Immediate transaction.
  • Pilot: pipeline/approval_draft.rs::create_approved_draft now drives its
    fence claim through begin_effect and its settlement through settle_effect.
    The connector call stays in the pipeline; the store owns only fence + audit.
  • Reuse, not duplication: extracted claim_pending_draft_write_conn /
    resolve_pending_draft_write_conn on-transaction helpers; the standalone
    claim_/resolve_pending_draft_write wrappers become #[cfg(test)] (their
    only remaining callers are regression tests).

Why

Per the ticket coordination note and DIRECTION.md: this ticket defines the seam
that Effect Truth #198 fills with disposition logic. #238's typed
EffectDisposition + settle_reservations() and #215's with_audited_effect
are merged, so settle_effect reuses the existing typed disposition and
routes to the existing settlement authority — no new disposition
classification
(that stays in #198). The seam also closes a Ledger gap: the
fence claim was previously written with no paired audit row; begin_effect now
audits it.

Verification

./scripts/check.sh is green for this change. New seam-shape tests
(store/effect_settlement_tests.rs) prove: DeliveryUnknown retains + fences
(no duplicate send), ConfirmedSuccess/ConfirmedFailure/NotAttempted
resolve the fence with a paired audit row, and a second begin_effect for the
same fingerprint claims no second fence and writes no audit row. Full kernel
suite (1146 tests) green.

Note: origin/main currently has an unrelated compile break in
openspine-authority tests (TaskGrant.user is now PrincipalId but the
#242 projection fixture still passes a String). It is outside this diff
(kernel only) — flagged to the coordinator separately.

Implementation notes (five-line summary)

  • Deviations: 3 — reuse EffectDisposition instead of a new Disposition
    enum (per dispatch, supersedes the stale CodeRabbit plan); begin_effect
    built on with_immediate_tx + append_audit_conn (not with_audited_effect,
    which always audits) so the lost-claim no-op writes nothing; new
    draft.pending_write_opened audit kind on the claim path.
  • Most likely to be revisited: whether EffectDisposition should move below
    store (store now references crate::api::...::EffectDisposition); deferred to Effect Truth Slice A-C: Characterise, type, and settle (Epic) #198.
  • Edge cases: scoped-admission path shares the piloted executor;
    NotAttempted unreachable at settlement in the pilot but handled.
  • Next session should read: spec Spec: deepened Store interface #208 D-003 and store/effect_settlement.rs.

Closes #216


Summary by cubic

Introduces a two-phase settlement seam for non-rollbackable external effects and pilots it on Gmail draft writes. Previously the pipeline claimed the draft fence without an audit and resolved fences directly; now begin_effect claims and audits atomically, and settle_effect(disposition) finalizes/cancels or retains with a paired audit, leaving delivery-unknown fences open to prevent duplicate sends.

  • New Store APIs: begin_effect(PendingWriteFence, AuditDescriptor) -> BeginEffect returns Fenced(EffectFence) or AlreadyFenced; settle_effect(EffectFence, EffectDisposition, AuditDescriptor) routes: ConfirmedSuccess → finalize; ConfirmedFailure/NotAttempted → cancel; DeliveryUnknown → retain and keep fence pending. Each write pairs with its audit in one Immediate transaction.
  • Pipeline: create_approved_draft now uses the seam; the connector call remains in the pipeline. Lost claims write draft.pending_write_fenced and skip the provider; inline outcome→disposition mapping is temporary until Effect Truth (Effect Truth Slice A-C: Characterise, type, and settle (Epic) #198).
  • Auditing: adds draft.pending_write_opened on successful claims; settlements use supplied kinds (draft.created, draft.creation_failed, draft.delivery_unknown) with references.
  • Refactor: extracted claim_pending_draft_write_conn / resolve_pending_draft_write_conn; old wrappers are #[cfg(test)]. New module store/effect_settlement.rs; re-exports BeginEffect, PendingWriteFence.
  • Tests: new seam-shape tests cover retain+fence, finalize, cancel, not-attempted, duplicate-claim behavior, and verify the audit chain.

Written for commit ee10a92. Summary will update on new commits.

Review in cubic

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@George-RD, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 37 minutes

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6406ec5b-9be5-4b6e-8e6e-fa99eade5db8

📥 Commits

Reviewing files that changed from the base of the PR and between 27ca4ab and ee10a92.

📒 Files selected for processing (5)
  • crates/openspine-kernel/src/pipeline/approval_draft.rs
  • crates/openspine-kernel/src/store/effect_settlement.rs
  • crates/openspine-kernel/src/store/effect_settlement_tests.rs
  • crates/openspine-kernel/src/store/mod.rs
  • crates/openspine-kernel/src/store/pending_draft.rs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@George-RD
George-RD merged commit eb63b7e into main Aug 21, 2026
3 checks passed
@George-RD
George-RD deleted the George-RD/dev-216 branch August 21, 2026 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Store: define begin_effect / settle_effect(disposition) settlement seam shape

1 participant