Skip to content

feat(bap): classify external outcomes for reconciliation - #228

Closed
seonghobae wants to merge 2 commits into
feat/bap-idempotency-receipt-contractfrom
feat/bap-external-outcome-reconciliation
Closed

feat(bap): classify external outcomes for reconciliation#228
seonghobae wants to merge 2 commits into
feat/bap-idempotency-receipt-contractfrom
feat/bap-external-outcome-reconciliation

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Partial implementation of #200, stacked on exact current PR #211 head 6ce74587dcfeede71ad6ceea94fe5c72dc0f244a.

Buyer/recovery gap

The BAP lifecycle already has explicit reconciliation/dead-letter states and exact idempotency receipts, but crash recovery still lacks a typed boundary that distinguishes a command known to have produced no external side effect, a command known to have produced an external side effect, an unknown outcome, and an explicitly reconciliation-required outcome. Without that distinction, a future durable runtime could accidentally retry a side effect or treat uncertainty as success.

Test-first state

Exact test-only head 09dfd50a02eb26896c7ac649a8a00a7a35d6bc4a adds focused regressions requiring BapExternalOutcome, BapRecoveryDirective, BapExternalOutcomeError, and BapTaskLifecycle::classify_external_outcome(...). Confirmed-no-side-effect must permit retry without lifecycle mutation; confirmed-side-effect must require post-condition verification without lifecycle mutation; unknown and explicit-reconciliation outcomes must require the existing explicit reconciliation transition while retaining their distinct cause; classification outside Running must fail closed.

The production API is intentionally absent on this head so CI should establish the missing boundary as RED before implementation.

Truth boundary

This slice will remain storage-neutral and authority-neutral. It will not attest whether a side effect occurred, persist recovery evidence, execute retries, resolve reconciliation, authenticate callers, or grant browser/network/model/secret authority. A trusted runtime must derive the external-outcome fact from reviewed evidence, durably preserve it, and separately execute the lifecycle transition or post-condition verification required by the typed directive.

No merge, approval, workflow, secret, ruleset, release, tag, or publish mutation is included.

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

Copy link
Copy Markdown
Contributor Author

Closing this Draft as a proven duplicate of the earlier canonical BAP crash-recovery owner #214 rather than implementing a second competing outcome API.

Fresh comparison on the common exact base 6ce74587dcfeede71ad6ceea94fe5c72dc0f244a shows #214 already owns the same production boundary with a stronger contract: BapExternalSideEffectOutcome distinguishes confirmed-no-side-effect, confirmed-side-effect, unknown, and reconciliation-required outcomes; BapRecoveryAction maps them to fail-closed handling; and BapCommandRecovery preserves the distinct classification while binding it to the exact immutable command receipt plus canonical recovery-evidence digest. Its permits_redispatch() also validates the exact retained receipt/lifecycle and refuses terminal or reconciliation-held redispatch.

By contrast this PR changes only tests/external_outcome_reconciliation.rs and its exact head 384d54dc7b040e8ecac6c01dc47ef52cce7a1280 is intentionally RED because the parallel BapExternalOutcome / BapRecoveryDirective / classify_external_outcome API is absent. Implementing that test contract here would duplicate the already-open #214 authority surface and weaken stack convergence. #214 exact head 8a3463bdb224f1c1226811f88c3d6f3a15d96f8f remains on the same live #211 base and currently has successful exact-head Rust contracts and Production coverage.

No unique production behavior from #228 is being discarded: the distinct unknown-vs-explicit-reconciliation cause is already retained by BapCommandRecovery::external_outcome(), while both correctly require reconciliation handling. Any further recovery-classification defect should therefore be repaired on #214, not by reviving this sibling API.

@seonghobae seonghobae closed this Aug 26, 2026
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.

1 participant