Add durable external-action settlement protocol - #704
Conversation
|
@coderabbitai review |
|
@codex please review |
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughEcho adds a domain-neutral external-action protocol with canonical requests, WAL-backed claims and settlements, strict validation, Merkle-indexed recovery, replay-safe settlement handling, comprehensive tests, and ADR/runtime documentation. ChangesExternal-action lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Edict
participant Echo
participant Adapter
participant WAL
Edict->>Echo: Construct canonical external-action request
Echo->>WAL: Commit REQUESTED record
Echo->>Adapter: Issue request-bound claim
Adapter->>Echo: Submit typed settlement candidate
Echo->>WAL: Commit SETTLED record
Echo-->>Edict: Resume from committed settlement bytes
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/warp-core/src/causal_wal.rs`:
- Around line 9626-9640: Centralize external-action transaction-kind to
record-kind mapping in a shared WalTransactionKind accessor, such as
external_action_record_kind() -> Option<WalRecordKind>. In
crates/warp-core/src/causal_wal.rs:9626-9640, update
validate_transaction_semantics to use the accessor while preserving the
exactly-one-frame validation. In
crates/warp-core/src/external_action.rs:1446-1470, replace
external_action_frame’s local expected match with the same accessor so both
validation paths share one lifecycle mapping.
In `@crates/warp-core/src/external_action.rs`:
- Around line 772-794: The Merkle mutation path currently traverses the same
256-level path twice, in root_digest_with_entry and refresh_merkle_path.
Collapse these operations into one traversal that computes each parent hash,
applies the corresponding node writes, and returns the resulting root; update
callers to use that combined result while preserving existing depth, sibling,
and empty-hash behavior.
- Around line 1432-1444: Change record_external_action_request,
claim_external_action, and admit_external_action_settlement to accept and mutate
a caller-owned RecoveredExternalActionIndexV1, advancing it only after each
successful commit instead of invoking full WAL replay. Retain
recover_external_action_index_from_store and its recover_from_frames_and_commits
path exclusively for cold recovery and initial index construction, avoiding
repeated payload decoding, allocations, and Merkle rebuilding on the kernel
request path.
In `@crates/warp-core/tests/external_action_protocol_tests.rs`:
- Around line 310-323: Replace the broad is_err() assertions in
external_action_protocol_tests.rs at lines 310-323, 338-351, 375-385, and
411-411 with exact Err(...) equality checks. At 310-323 assert the
request/authorization binding error; at 338-351 assert the
missing-lease-evidence variant; at 375-385 assert
ExternalActionProtocolErrorV1::MissingExternalEvidence; and at 411-411 assert
the frontier-mismatch variant, preserving each test’s existing setup.
- Around line 527-548: The current test only verifies invalid attempt ordinal
handling, not prevention of multiple claims for one request. Extend the relevant
external-action protocol test to record a request, perform an ordinal-0 claim
successfully, then submit a second ordinal-0 claim using a fresh lease digest;
assert the specific single-claim obstruction error and verify the store commit
count remains unchanged after the rejected claim.
- Around line 899-966: Update CommitFailingStore::flush_commit to determine the
current commit count without cloning the full commit vector. Use an existing
WalStorePort or InMemoryWalStore count/length accessor if available, preserving
the fail_on_commit_ordinal comparison and normal commit behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f7c29737-3f88-485d-b8e9-67f463ca1f9b
📒 Files selected for processing (13)
CHANGELOG.mdcrates/warp-core/src/causal_wal.rscrates/warp-core/src/evidence.rscrates/warp-core/src/external_action.rscrates/warp-core/src/lib.rscrates/warp-core/tests/external_action_protocol_tests.rsdocs/README.mddocs/adr/0026-durable-external-action-settlement.mddocs/adr/README.mddocs/architecture/application-contract-hosting.mddocs/topics/RuntimeAuthority.mddocs/topics/WAL.mddocs/topics/security/AuthorityBoundaries.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4ee2d9b6d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Merge gate: 40/40 checks pass at b36bb20; 0/12 review threads remain unresolved; the Code Lawyer pass fixed every admitted finding; focused and feature-complete local suites, clippy, formatting, determinism, and hook gates pass. Solo-maintainer substitution applies because no protected human-review requirement can be satisfied by a second collaborator. |
Records typed external-action requests before adapter execution and admitted settlements before deterministic resumption.
Closes #694.
Summary by CodeRabbit
New Features
Documentation
Tests