Admit contract-defined inverse intents from causal history#659
Conversation
|
Warning Review limit reached
Next review available in: 50 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 Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughAdds typed causal receipt references, versioned retained WAL evidence, replayable provenance validation, installed-contract inverse admission, and restart recovery checks. Updates WSC storage, CLI posture output, integration tests, and runtime documentation to use the new causal identity model. ChangesRuntime durability and inverse admission
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
…verse-admission # Conflicts: # CHANGELOG.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bf2def0e5
ℹ️ 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".
…verse-admission # Conflicts: # crates/warp-core/src/trusted_runtime_host.rs # docs/BEARING.md # docs/README.md # docs/design/0018-contract-hosted-file-history-substrate/design.md # docs/design/product-facing-intent-outcome-api.md # docs/design/witnessed-submission-persistence.md
Code Lawyer self-audit
The correction will remove the stale upstream copy, retain the branch copy that covers recovered causal receipt fields, rerun the exact RED witness, and land as its own commit. Cc: @codex for a second opinion. |
Code Lawyer Activity Summary
RED / GREEN evidence
All three originating review threads are resolved. The worktree is clean and the verified head is @codex review please |
Code Lawyer follow-up
GREEN evidence:
@codex review please |
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/warp-core/src/causal_wal.rs (1)
7345-7405: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate cross-record identity before committing atomic WAL transactions.
The builders accept mutually inconsistent evidence without checks:
- Acceptance and retained envelope can name different submissions or envelope digests.
- Receipt and correlation can carry different
receipt_refvalues.- Replay state-delta bytes can carry a receipt digest unrelated to the tick receipt.
Decode and validate these contracts before
commit; preferably accept typed retained records rather than unchecked bytes.As per coding guidelines, one transaction must form a canonical append-only evidence claim.
🤖 Prompt for 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. In `@crates/warp-core/src/causal_wal.rs` around lines 7345 - 7405, Validate cross-record identity in build_submission_acceptance_with_material_transaction, build_tick_transaction, and build_replayable_tick_transaction before calling commit. Ensure acceptance records match retained envelope submission and envelope digests, receipt and correlation share the same receipt_ref, and replayable state-delta material carries the tick receipt’s expected digest. Prefer a typed retained state-delta record over unchecked bytes, and return the established validation/build error on mismatches so only canonical evidence claims are committed.Sources: Coding guidelines, Path instructions
crates/warp-core/src/receipt.rs (1)
37-94: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate all blocker invariants in the retained-parts constructor.
This public constructor accepts duplicate, unsorted, forward, or disposition-incompatible blockers. Because the receipt digest excludes blockers, malformed attribution can still pass the provenance store’s transaction/digest checks. Enforce every documented
blocked_byinvariant here and return a typed error.As per coding guidelines, retained receipt evidence must be canonical before entering append-only history.
🤖 Prompt for 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. In `@crates/warp-core/src/receipt.rs` around lines 37 - 94, The public TickReceipt::try_from_retained_parts constructor currently validates only parallel lengths; validate every documented blocked_by invariant there, including canonical ordering/uniqueness, rejecting forward references and disposition-incompatible blockers. Add appropriate typed TickReceiptPartsError variants carrying the relevant context, and return them before computing the digest or constructing the receipt; keep TickReceipt::new’s existing behavior unchanged.Sources: Coding guidelines, Path instructions
crates/warp-core/src/wsc/store.rs (1)
4551-4586: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winRedundant
by_receiptmap duplicatesby_correlation.
by_receiptandby_correlationare both keyed byrecord.receipt_refand receive identical inserted values, soby_receiptis a pure duplicate that costs an extraBTreeMapplus an extrarecord.clone()(cloning aVec<CausalTickReceiptRef>) per record for no behavioral benefit. Drop it and reuseby_correlationfor the conflict check.♻️ Proposed fix
fn canonical_receipt_correlations( records: &[WalReceiptCorrelationRecord], ) -> Result<Vec<WalReceiptCorrelationRecord>, WscStoreObstruction> { let mut by_correlation = BTreeMap::new(); let mut by_submission = BTreeMap::new(); let mut by_ticket = BTreeMap::new(); - let mut by_receipt = BTreeMap::new(); for record in records { if let Some(existing) = by_submission.get(&record.receipt_ref.submission_id) { if existing != record { return Err(WscStoreObstruction::duplicate_mismatch( WscStoreEnvelopeId::from_hash(record.receipt_ref.submission_id), )); } } if let Some(existing) = by_ticket.get(&record.receipt_ref.ticket_digest) { if existing != record { return Err(WscStoreObstruction::duplicate_mismatch( WscStoreEnvelopeId::from_hash(record.receipt_ref.ticket_digest), )); } } - if let Some(existing) = by_receipt.get(&record.receipt_ref) { + if let Some(existing) = by_correlation.get(&record.receipt_ref) { if existing != record { return Err(WscStoreObstruction::duplicate_mismatch( WscStoreEnvelopeId::from_hash(record.receipt_ref.identity_digest()), )); } } by_correlation.insert(record.receipt_ref, record.clone()); by_submission.insert(record.receipt_ref.submission_id, record.clone()); - by_ticket.insert(record.receipt_ref.ticket_digest, record.clone()); - by_receipt.insert(record.receipt_ref, record.clone()); + by_ticket.insert(record.receipt_ref.ticket_digest, record.clone()); } Ok(by_correlation.into_values().collect()) }As per path instructions, "warp-core is the deterministic kernel... Performance matters — flag unnecessary allocations in hot paths."
🤖 Prompt for 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. In `@crates/warp-core/src/wsc/store.rs` around lines 4551 - 4586, Remove the redundant by_receipt BTreeMap and its insert from canonical_receipt_correlations. Reuse by_correlation for the receipt_ref conflict check, preserving the existing mismatch error behavior while eliminating the extra map and record clone.Source: Path instructions
🤖 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 `@backlog/bad-code/RE-034-causal-tick-receipt-reference.md`:
- Around line 6-7: Update the active backlog item to mark the inverse-admission
witness complete, reflecting that
inverse_intent_resolves_one_admitted_transition_after_restart has been added.
Change the pending status and check off the corresponding backlog entry while
preserving the rest of the item.
In `@crates/warp-core/src/causal_wal.rs`:
- Around line 2230-2297: The receipt-correlation encoder and decoder must use
one canonical representation for an empty causal-parent set. Update
to_payload_bytes and from_payload_bytes so empty parents are either always
encoded/read with a count or explicit zero-count payloads are rejected as
non-canonical; preserve deterministic sorting and validation for non-empty
parent lists.
- Around line 2701-2710: Make recovery record application append-only: update
apply_tick_receipt_record, apply_correlation_record, and the related handlers
around the other insert calls to return Result and reject conflicting duplicate
receipt, ticket, decision, or causal-parent records. Allow only exact duplicates
as no-ops, and remove any overwrite or remove-and-replace behavior, including
parent ancestry changes in apply_correlation_record. Propagate these errors
through all callers.
In `@crates/warp-core/src/coordinator.rs`:
- Around line 1340-1346: Update receipt_correlations storage and admission paths
around receipt_correlation_for_receipt_ref to maintain deterministic BTreeMap
indexes keyed by CausalTickReceiptRef and current-basis coordinate. Populate
both indexes whenever a ReceiptCorrelationRecord is admitted, then replace the
values().find scan and reuse the indexes for inverse-admission basis lookups
while preserving exact-match behavior.
- Around line 2029-2039: In the recovered submission flow around the envelope
lookup, derive the canonical causal-parent reference set from
envelope.causal_parents() and compare it with the independently persisted
correlation parents before restoration at the later correlation step. Return the
appropriate replay-mismatch error on any difference, while preserving the
existing ingress ID validation and successful restoration path.
In `@crates/warp-core/src/engine_impl.rs`:
- Around line 432-436: Update the cfg_attr on the contract_inverse_handlers
field to allow dead_code only when not(all(feature = "native_rule_bootstrap",
feature = "trusted_runtime")). Keep the existing suppression for
contract_query_observer_packages unchanged.
In `@crates/warp-core/src/provenance_codec.rs`:
- Around line 178-234: Update validate_local_commit to validate entry.parents
ordering before computing the commit hash: require each adjacent pair of parent
commit_hash values to be strictly increasing using a windows(2) check, and
return the existing inconsistency error for non-canonical ordering. Keep the
existing hash validation unchanged for canonically ordered parents.
In `@crates/warp-core/src/provenance_store.rs`:
- Around line 605-609: Update ProvenanceEntry validation around
validate_recorded_event_entry to reject any tick_receipt when the recorded event
is not a local event, preserving receipt acceptance only for local commits.
Ensure ProvenanceEntry::with_tick_receipt cannot create an entry that bypasses
this local-commit-only constraint, either through validation or a typed builder.
In `@crates/warp-core/tests/external_consumer_contract_fixture_tests.rs`:
- Around line 351-368: Update temp_runtime_wal_dir so an AlreadyExists result
from create_dir is treated as a path collision: do not remove the existing
directory, and continue to the next TEMP_COUNTER suffix. Preserve the retry
limit and existing panic behavior for other filesystem errors and exhausted
attempts.
In `@crates/warp-core/tests/provenance_retention_codec_tests.rs`:
- Around line 18-21: Replace the hard-coded EXPECTED_COMMIT_HASH_OFFSET and
generic is_err() assertion in the provenance retention codec test with
fixture-driven lookup of the encoded commit hash, then assert the exact
commitment-validation error for the mutated data. Update the related assertions
around the failure case while preserving deterministic behavior and avoiding
duplicated codec-layout offsets.
In `@crates/warp-core/tests/trusted_runtime_host_loop_tests.rs`:
- Around line 645-648: Replace the inequality checks in both
recovery-certificate assertions with exact comparisons against the canonical
root calculated from all recovered evidence, reusing the same full-root
calculator or production verifier at
crates/warp-core/tests/trusted_runtime_host_loop_tests.rs:645-648 and 1763-1766.
Ensure both sites validate the expanded recovery certificate deterministically
rather than merely differing from the obsolete submissions/receipts-only root.
---
Outside diff comments:
In `@crates/warp-core/src/causal_wal.rs`:
- Around line 7345-7405: Validate cross-record identity in
build_submission_acceptance_with_material_transaction, build_tick_transaction,
and build_replayable_tick_transaction before calling commit. Ensure acceptance
records match retained envelope submission and envelope digests, receipt and
correlation share the same receipt_ref, and replayable state-delta material
carries the tick receipt’s expected digest. Prefer a typed retained state-delta
record over unchecked bytes, and return the established validation/build error
on mismatches so only canonical evidence claims are committed.
In `@crates/warp-core/src/receipt.rs`:
- Around line 37-94: The public TickReceipt::try_from_retained_parts constructor
currently validates only parallel lengths; validate every documented blocked_by
invariant there, including canonical ordering/uniqueness, rejecting forward
references and disposition-incompatible blockers. Add appropriate typed
TickReceiptPartsError variants carrying the relevant context, and return them
before computing the digest or constructing the receipt; keep TickReceipt::new’s
existing behavior unchanged.
In `@crates/warp-core/src/wsc/store.rs`:
- Around line 4551-4586: Remove the redundant by_receipt BTreeMap and its insert
from canonical_receipt_correlations. Reuse by_correlation for the receipt_ref
conflict check, preserving the existing mismatch error behavior while
eliminating the extra map and record clone.
🪄 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
Run ID: a7a43d99-ae00-4e4d-b90f-41390a2bcad8
📒 Files selected for processing (37)
.ban-nondeterminism-allowlistCHANGELOG.mdbacklog/bad-code/RE-034-causal-tick-receipt-reference.mdcrates/warp-cli/src/wal.rscrates/warp-cli/tests/cli_integration.rscrates/warp-cli/tests/support/runtime_wal_fixture.rscrates/warp-core/src/causal_receipt.rscrates/warp-core/src/causal_wal.rscrates/warp-core/src/contract_host.rscrates/warp-core/src/contract_inverse.rscrates/warp-core/src/contract_obstruction.rscrates/warp-core/src/contract_registry.rscrates/warp-core/src/coordinator.rscrates/warp-core/src/engine_impl.rscrates/warp-core/src/head_inbox.rscrates/warp-core/src/lib.rscrates/warp-core/src/playback.rscrates/warp-core/src/provenance_codec.rscrates/warp-core/src/provenance_store.rscrates/warp-core/src/receipt.rscrates/warp-core/src/trusted_runtime_host.rscrates/warp-core/src/wsc/store.rscrates/warp-core/tests/causal_wal_hardening_tests.rscrates/warp-core/tests/causal_wal_tests.rscrates/warp-core/tests/external_consumer_contract_fixture_tests.rscrates/warp-core/tests/ingress_retention_codec_tests.rscrates/warp-core/tests/installed_contract_intent_pipeline_tests.rscrates/warp-core/tests/installed_contract_registry_tests.rscrates/warp-core/tests/provenance_retention_codec_tests.rscrates/warp-core/tests/receipt_causal_parent_recovery_tests.rscrates/warp-core/tests/retained_evidence_ref_tests.rscrates/warp-core/tests/trusted_runtime_host_loop_tests.rscrates/warp-core/tests/wsc_store_tests.rsdocs/README.mddocs/topics/ContractInverseAdmission.mddocs/topics/README.mddocs/topics/WAL.md
Code Lawyer self-audit follow-up
The correction will reject an explicit zero count with the existing Cc: @codex for a second opinion. |
Code Lawyer follow-up closure
RED evidence:
GREEN evidence:
The pushed PR head is @codex review please |
…nverse-admission # Conflicts: # CHANGELOG.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6e45077a8
ℹ️ 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".
…t-inverse-admission # Conflicts: # crates/warp-core/tests/provenance_retention_codec_tests.rs
…-inverse-admission
Code Lawyer Activity SummaryFinal reviewed head:
Verification
CodeRabbit’s incremental review was rate limited after its initial full audit. Requesting an alternate Codex review of exact head |
|
@codex review please |
Code Lawyer CI follow-up
Final reviewed head is now |
|
@codex review please |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
Closes #472.
Why
Undo and redo must be forward causal history, not process-local snapshots or an LRU correlation cache. An application contract owns inverse semantics; Echo owns evidence resolution, admission, durability, ordering, and receipts.
Validation
cargo test -p warp-core --features native_rule_bootstrap,trusted_runtime,host_testcargo test -p warp-core --features native_rule_bootstrap --test installed_contract_registry_testscargo test -p warp-cli --test cli_integrationcargo check --workspace --all-targetscargo clippyfor warp-core and warp-cli targetscargo fmt --all -- --checkgit diff --checkSummary by CodeRabbit
New Features
Bug Fixes
Documentation