Add WAL and topology durability evidence - #603
Conversation
|
Warning Review limit reached
More reviews will be available in 29 minutes and 57 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThe PR switches documentation entry points to ChangesTopology intent recovery
Sequence Diagram(s)WAL topology recovery sequenceDiagram
participant WalTransactionBuilder
participant build_topology_intent_transaction
participant WalCommittedTransaction
participant recover_topology_index
participant RecoveredTopologyIndex
WalTransactionBuilder->>build_topology_intent_transaction: topology intent records
build_topology_intent_transaction->>WalCommittedTransaction: commit topology transaction
WalCommittedTransaction->>recover_topology_index: committed WAL history
recover_topology_index->>RecoveredTopologyIndex: recovered topology index
WSC envelope flow sequenceDiagram
participant WscTopologyRecords
participant topology_records_to_wsc_envelope
participant WscStoreEnvelope
participant WscStorePort
participant topology_records_from_wsc_store
participant topology_records_from_wsc_envelope
WscTopologyRecords->>topology_records_to_wsc_envelope: topology intent records
topology_records_to_wsc_envelope->>WscStoreEnvelope: deterministic envelope
WscStorePort->>topology_records_from_wsc_store: committed envelopes
topology_records_from_wsc_store->>topology_records_from_wsc_envelope: validated envelope bytes
topology_records_from_wsc_envelope->>WscTopologyRecords: recovered topology records
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
💡 Codex Review
Line 4 in c1ec6b9
The VitePress config still routes both the Home nav item and the Docs Map sidebar item to /, and VitePress file routing generates that route from docs/index.md; after this rename there is no docs/index.md, so the published docs homepage will no longer generate /index.html and those entry points will 404. Keep the map at docs/index.md or add an explicit rewrite/redirect before removing it.
ℹ️ 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".
| | `not_accepted` | The intent never reached WAL-backed accepted submission posture. | | ||
| | `accepted_pending` | The intent was accepted and can be recovered, but no decided receipt was recovered. | | ||
| | `decided_applied` | A recovered receipt says the work applied under named law. | | ||
| | `decided_rejected` | A recovered receipt says the work was rejected, conflicted, or obstructed. | |
There was a problem hiding this comment.
List obstructed as a separate recovery posture
For submissions whose tick decision is obstructed, the recovery code exposes RecoveredSubmissionPosture::Obstructed rather than DecidedRejected (apply_tick_receipt_record maps WalTickDecision::Obstructed to that separate posture, and the CLI labels it Obstructed). Collapsing obstructed outcomes into decided_rejected here will mislead editor/product code using this table as the recovery gate, causing it to treat obstruction as a rejection instead of preserving the distinct obstruction posture.
Useful? React with 👍 / 👎.
| For Jim/jedit, "dirty" should not mean "at risk of being lost." It should mean | ||
| "not currently materialized to the host file projection." If an edit intent has | ||
| received WAL-backed accepted-submission evidence, the edit belongs to | ||
| recoverable causal history even before the host file is written. If that edit is |
There was a problem hiding this comment.
Keep dirty warnings until edit material is recoverable
In the crash/quit-before-file-projection case, the current ACK transaction only stores SubmissionAcceptanceRecord fields such as the submission id and canonical envelope digest, not the canonical edit bytes themselves (SubmissionAcceptanceRecord::to_payload_bytes confirms this), and the caveat below says retained material availability is still future work. Treating a WAL-backed ACK alone as meaning the edit content is recoverable can make jedit suppress a dirty-buffer warning even though recovery can only prove an accepted-pending digest, not restore the user's text.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@AGENTS.md`:
- Around line 61-62: The AGENTS.md documentation map section still lists both
docs/README.md and docs/index.md as active maps, which conflicts with the
rename. Update the entries so docs/README.md is the sole primary VitePress
documentation map, and either remove the docs/index.md entry or relabel it
clearly as legacy/superseded to avoid ambiguity for agents.
In `@crates/warp-core/src/causal_wal.rs`:
- Around line 3951-3961: The BraidEvent recovery path in causal_wal.rs currently
indexes records without checking that the inner BraidCreated data is
self-consistent. In TopologyIntentRecord::BraidEvent, validate that the embedded
braid identity matches the outer TopologyBraidEventRecord.braid_id and reject
contradictory status_after values before calling insert_unique. Add the
consistency check in the BraidEvent handling branch and return a
WalRecoveryIndexError from this path when the record is impossible, so invalid
braid-log evidence never reaches braid_event_maps.
- Around line 3537-3540: Canonicalize writer_heads before they are serialized or
used in stable comparisons, since the current caller-provided order in the
writer_heads handling affects duplicate detection, recovered roots, and WSC
topology sort keys. Update the code around the writer_heads serialization path
and insert_unique flow to sort the heads by (worldline_id, head_id) first, then
serialize the normalized order consistently in the related
duplicate/root/topology code paths.
- Around line 3923-3950: Recovery currently deduplicates StrandFork and
StrandDrop only by strand_id, so mismatched child_worldline_id values for the
same strand can slip through. Update the WAL recovery logic in the
TopologyIntentRecord::StrandFork and TopologyIntentRecord::StrandDrop handling
to compare the forked child_worldline_id against any existing drop for that
strand and vice versa, and fail recovery with a new or existing
WalRecoveryIndexError when they disagree. Use the existing insert_unique flow in
causal_wal.rs as the location to add the consistency check so fork/drop records
for the same strand cannot recover with conflicting child worldlines.
- Around line 3557-3560: The `writer_count` read in `causal_wal.rs` is used
directly in `Vec::with_capacity`, which can over-allocate on malformed input;
before allocating in this decode path, bound `writer_count` against the
remaining buffer size using the expected encoded writer-head length. Update the
logic around `cursor.read_u64()`, `writer_count`, and `writer_heads` so the size
check happens first and the function returns `WalDecodeError::UnexpectedEof` or
a similar decode error before any large allocation is attempted.
In `@crates/warp-core/src/wsc/store.rs`:
- Around line 1240-1302: The topology intent encoding currently only enforces
idempotency collisions for SuffixImportRecord, while StrandForkRecord,
StrandDropRecord, BraidEvent, and BraidShell also carry idempotency_key_digest
and can still duplicate on retries. Update the topology intent handling in the
store logic around the existing BTreeMap inserts and insert_wsc_unique calls to
add per-record-type idempotency maps for the other intent variants, and route
each Some(idempotency_key_digest) through insert_wsc_unique so conflicting
retries are rejected consistently.
- Around line 1146-1203: The decoder in topology_records_from_wsc_envelope
currently accepts any attachment scan that matches the topology schema, but it
must also enforce the canonical topology graph shape produced by
topology_records_to_wsc_envelope. Add validation for the fixed topology root,
node type, edge type/id, and topology_node_id(role, payload) layout before
decoding attachments, and reject any extra or misplaced topology attachments
that are not part of that canonical tree. Use the existing symbols in WscFile,
warp_view, and topology_records_from_wsc_envelope to locate the decode path and
keep the kernel’s ordering/stability rules intact.
In `@crates/warp-core/tests/wsc_store_tests.rs`:
- Around line 631-647: The current test for conflicting duplicate strand forks
is correct, but the obstruction enum name is semantically ambiguous. Update the
surrounding test/documentation near
topology_records_reject_conflicting_duplicate_strand_fork and the
topology_records_to_wsc_envelope/canonical_topology_records path to explicitly
state that DuplicateEnvelopeMismatch is the canonical signal for internal
payload integrity conflicts, not just store-level collisions. If appropriate,
add or rename a more specific variant such as TopologyRecordConflict, or at
minimum document the mapping where duplicate_mismatch is produced from
conflicting TopologyIntentRecord::StrandFork records.
🪄 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: bb80b1d3-ca2f-4030-ae3c-afc4ae306479
📒 Files selected for processing (16)
AGENTS.mdCHANGELOG.mdGUIDE.mdREADME.mdcrates/warp-core/src/causal_wal.rscrates/warp-core/src/wsc/mod.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/wsc_store_tests.rsdocs/README.mddocs/design/braids-and-strands-hardening/goalpost-06-topology-intents-and-wal-recovery.mddocs/design/braids-and-strands-roadmap.mddocs/procedures/DOCS-LIFECYCLE.mddocs/topics/WAL.mdxtask/src/main.rs
|
Docs map follow-up: I checked the current checkout and there is no live |
Summary
docs/topics/WAL.mdexplaining Echo's WAL as the durable causal-history commit boundarydocs/index.mdtodocs/README.mdcargo xtask test-slice durability-releaseas the joined WAL/WSC release witnessCloses #579
Closes #580
Closes #581
Closes #604
Closes #605
Closes #606
Closes #607
Closes #608
Validation
pnpm exec markdownlint-cli2 docs/topics/WAL.md docs/index.mdcargo checkcargo test -p warp-core --features native_rule_bootstrap,trusted_runtime,host_test --test trusted_runtime_host_loop_tests runtime_wal_ackpnpm exec markdownlint-cli2 AGENTS.md README.md GUIDE.md docs/README.md docs/procedures/DOCS-LIFECYCLE.mdgit diff --checkcargo test -p xtask root_relative_link_resolves_against_docs_rootcargo test -p xtaskcargo fmt --allcargo test -p warp-core --lockedcargo test -p xtask durability_stale_claimsscripts/check-wal-wsc-doctrine.shscripts/tests/check_wal_wsc_doctrine_test.shcargo test -p xtask test_slice_durability_releasecargo xtask test-slice durability-releasepnpm exec markdownlint-cli2 docs/README.md docs/design/braids-and-strands-hardening/goalpost-06-topology-intents-and-wal-recovery.md docs/workflows.md docs/design/wal-wsc-release-closure-audit.mdcargo fmt --all -- --checkNotes
The WAL topic distinguishes proven ACK/recovery-index contracts from stricter filesystem durability and WSC portability claims. The follow-up commits now add topology intent durability and the joined release witness. The closure audit leaves broader release-bar work open where it is outside this PR's evidence boundary.