fix: release WAL writer lease before closing its descriptor

[flyingrobots]

Summary

A filesystem WAL owner could drop while a duplicate descriptor kept its OS lock alive, causing immediate takeover to fail with WriterEpochLeaseUnavailable. The store now owns a private guard that explicitly attempts unlock before close. A live owner still excludes contenders, and closing a retired descriptor cannot release its successor's independently acquired lease.

Closes #736. Narrow CI prerequisite for #735. Broader process-bound authority and explicit fallible release remain in #718.

Scope and provenance

• Runtime code: internal filesystem WAL lease lifecycle, regression, WAL topic, changelog.
• Ports the isolated behavior from recovered commit 54b7abc3edc95ed8609eb488f6c20c3d21cbd475. No historical falsification/WIP branch is merged.
• The regression atomically claims a scratch directory, never deletes an existing candidate, and additionally proves successor exclusion.
• No WAL format, scheduler, package, compiler, provider, or dependency pin changes.
• Targets current main after integrating the merged Admit generic Edict pure-program contracts #724/Evaluate compiler-produced pure Edict programs #726/Evaluate checked unsigned subtraction in pure Edict programs #731/Evaluate bounded byte lengths from compiler-produced programs #733 prerequisites in 09d8c8334c398511b70533f9b79dfd505c9f9210. feat: explicitly admit the ordered Edict contract publication #735 depends on this focused repair. The changelog-only conflict retained both sets of entries.

Validation

All executable checks ran in Docker using copied sources, without host repository/Git mounts:

• RED: retaining a real duplicated descriptor caused the successor acquisition to fail after owner drop, matching the hosted failure's refusal.
• GREEN: the new regression passed in ordinary and det_fixed configurations, including live-owner exclusion and old-descriptor close after successor acquisition.
• All 125 WAL hardening tests passed in each configuration. The one intentionally ignored subprocess entry point is exercised by its parent witness.
• Strict Clippy passed for the library and WAL hardening target, and the det_fixed library. Rust formatting and git diff --check passed.

The regression models descriptor retention deterministically; it is not a process/fork authority proof. Drop remains best effort and cannot return an OS unlock failure. These limits are documented in docs/topics/WAL.md with the Rust file-lock contract and #718.

Current Code Lawyer validation

At 09d8c8334c398511b70533f9b79dfd505c9f9210, copied-source Docker validation passed the lease regression plus all 125 WAL hardening tests in ordinary and det_fixed configurations. The ignored subprocess entry is invoked by its parent witness, not omitted acceptance work. Strict Clippy and formatting passed. Historical RED remains the retained-descriptor failure in the original isolated experiment; current GREEN is a new run against merged main.

Independent review and hosted CI are running for this exact head. No approval is inferred from a rate-limited bot status. The worker is stopped; retained aggregate build/cache is ~5.63 GiB, data ~1.37 GiB, logs ~6.02 MiB, with >50 GiB free on both host and Docker VM. No new worker/image/cache was created.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: flyingrobots/echo/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: cd22116f-aae6-4622-955e-a6a276d69653

📥 Commits

Reviewing files that changed from the base of the PR and between 09b98b7 and 09d8c83.

📒 Files selected for processing (3)

• CHANGELOG.md
• crates/warp-core/src/causal_wal.rs
• docs/topics/WAL.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The filesystem WAL store now holds its writer lease in a guard that attempts an explicit unlock when dropped. A regression test covers lease acquisition with a retained duplicate descriptor. The WAL documentation and changelog describe the behavior and its limitations.

Changes

Filesystem WAL writer lease

Layer / File(s)	Summary
Lease guard and regression coverage crates/warp-core/src/causal_wal.rs, docs/topics/WAL.md, CHANGELOG.md	FilesystemWalStore now retains a WriterEpochLock, which attempts to unlock the file on drop. A test checks that a successor can acquire the lease while a duplicate descriptor remains open and that the successor continues to exclude other writers. The documentation and changelog describe the drop behavior and its limitations.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 09d8c

No merge-blocking issue remains in the supplied evidence. Drop-time unlock remains best-effort, as documented.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 09d8c

The repair improves takeover without adding a public operation or broadening filesystem access. A conditional integrity risk remains: an inherited writer can retain local authority after explicit unlock allows a successor to acquire the lease. This requires trusted-host process behavior; production reachability is not established.

Retained concerns

• Medium · security · inferred: If a forked process continues using an inherited FilesystemWalStore, dropping either inherited guard can unlock the shared open-file description while the other store retains its local writer_lock and active_epoch. A successor can then acquire its own lease, while stale append or commit operations still pass the inherited store's local checks. Process-bound authority was already incomplete, but this PR removes the retained descriptor's exclusion of independently acquired successors. The regression covers descriptor cleanup, not this inherited-writer transition; production reachability is unknown.

Security review details

Security Blast Radius

• inferred — The directly evidenced exposure is shared WAL storage under a store root and the processes using its writer authority. Exploiting the inherited-writer transition requires access to an inherited trusted-host store, not merely submitting application intents. No supplied relationship or deployment evidence establishes wider tenant or service exposure.

Security Findings and Attack Paths

• inferred — The conditional path is inherited store authority, followed by explicit unlock in another process, successor acquisition, and stale mutation through the inherited store's still-populated local state. This can undermine WAL single-writer integrity. It is a source-supported transition concern, not a demonstrated production exploit.

Trust Boundaries and Controls

• observed — Fresh contenders still must acquire the existing writer-epoch.lock through try_lock, with contention returning WriterEpochLeaseUnavailable. Append and commit operations retain epoch matching, and commits retain their capability checks. The change adds no new path input or public caller boundary.

Resilience and Maintainability Implications

• observed — The strongest counterevidence is the regression's exclusion checks before owner drop, after successor acquisition, and after retired-descriptor closure. These cover normal takeover and descriptor cleanup, but the retained object is only a File, not an inherited store capable of issuing stale writes.

Hardening Proposals

• proposed — Bind mutation and release authority to the originating process, or enforce that inherited stores cannot be used before acquiring fresh authority. Validate fork, drop, takeover, and stale-mutation interleavings separately from descriptor-only cleanup.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR addresses #736. The WAL lease guard attempts unlock before closing its descriptor. The regression covers live-owner exclusion, successor acquisition while the duplicate remains open, and contin…
Out of Scope Changes check	✅ Passed	The reported changes are limited to the filesystem WAL lease guard and regression in causal_wal.rs, WAL topic documentation, and the changelog. These changes support #736. No unrelated changes are rep…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: releasing the WAL writer lease before closing its descriptor.

Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[flyingrobots]

Code Lawyer activity summary

Item	Evidence	Outcome
Integrate current main	09d8c833; both parents audited, changelog conflict preserves both entries; lease source unchanged from 00fd1e6c	Committed and pushed
Descriptor lifecycle regression	Historical RED fails successor acquisition while duplicate remains; current Docker GREEN passes in ordinary and det_fixed configurations	Passed
Surrounding WAL safety	125 hardening tests per configuration, subprocess entry explicitly exercised by parent	Passed
Static checks	Strict Clippy, formatting, diff whitespace, signed merge commit	Passed
Independent review / hosted CI	Exact current head	In progress

No new source defect found in the primary audit. Fallible explicit release and broader process-bound authority remain #718; this PR does not claim those acceptance criteria. No Jim pins or historical WIP branches changed. Resource worker stopped after passing guarded run; measurements recorded in PR body.

[flyingrobots]

Independent Code Lawyer review and evidence reconciliation

The full independent review and its correction addendum follow. Approval is scoped to this patch and its witnesses, not proof of absence of all defects or completion of Jim's persistent-rope integration. The addendum supersedes the original report's incorrect log coordinates and downstream links.

Primary reviewer clarification of two names accidentally introduced by the addendum: this code invokes std::fs::File::unlock, not fs2::FileExt::unlock (causal_wal.rs:31,8467); the owning type is FilesystemWalStore, not CausalWalStore (causal_wal.rs:5576). The original review correctly identifies the standard-library API. No fs2 dependency or API change is part of this patch. The regression spans lines 8508–8533, and the attempt-bound declaration is line 8494. Observed Markdown wrapping is not an additional mandatory policy.

Independent Adversarial Code Lawyer Review: PR #737

Repository: flyingrobots/echo
PR: #737 (fix/writer-lease-release)
Target Branch: main at 09b98b74d48dce709111528dd3db3ecb7f7c29db
Exact Review Head: 09d8c8334c398511b70533f9b79dfd505c9f9210
Merge Parents:

• Parent 1 (Feature): 00fd1e6c400b52297cb7953158a54d7e4dae0fdc
• Parent 2 (Main): 09b98b74d48dce709111528dd3db3ecb7f7c29db
  Binding Role: Independent Adversarial Reviewer authorized under fallback protocol during automated reviewer rate limiting (GraphQL evidence).

---

1. Executive Summary & Review Scope

PR #737 addresses Issue #736 by replacing the raw File stored in FilesystemWalStore.writer_lock with a private wrapper struct WriterEpochLock. This wrapper implements Drop to perform an explicit, best-effort self.0.unlock() before the underlying descriptor is closed.

Under Unix file-locking semantics (flock), an advisory lock belongs to the underlying open file description. If a descriptor is duplicated (e.g. across fork() prior to exec()), closing the parent descriptor alone does not release the lock. Explicit unlocking releases the lock across all retained duplicates pointing to that open file description.

Inspected Scope & Exclusions

• Inspected:
  1. Internal filesystem WAL writer lease lifecycle: acquisition, mutation guards, release on close_epoch, release on store drop, and early-error unwind paths in crates/warp-core/src/causal_wal.rs.
  2. The deterministic duplicated-descriptor regression test owner_drop_releases_retained_descriptor_without_releasing_successor and bounded atomic scratch directory claiming scratch_root.
  3. Merge commit 09d8c8334c398511b70533f9b79dfd505c9f9210 against both parents 00fd1e6c400b52297cb7953158a54d7e4dae0fdc and 09b98b74d48dce709111528dd3db3ecb7f7c29db.
  4. Documentation changes in docs/topics/WAL.md and CHANGELOG.md.
  5. Execution logs and resource receipts in <audit evidence>/ and <historical evidence>/.
• Explicit Exclusions & Limits:
  • This review operates under static inspection of existing logs; it does not execute host builds, Docker runs, or benchmarks.
  • This repair does not provide a fallible explicit-release error reporting API or prove process/fork authority boundaries, cross-session OS crashes, or physical power loss. Those contracts are tracked under Issue #718.
  • No public API signatures, WAL frame formats, compiler/provider pins, or application WASM ABI acceptance criteria (#296, #302) are modified or approved herein.

---

2. Line-by-Line Code Path Trace

Path 1: Fresh Writer Epoch Acquisition

• Production Entry: FilesystemWalStore::acquire_fresh_writer_epoch.
  • Active Guard Check: crates/warp-core/src/causal_wal.rs:5735-5737. If self.writer_lock.is_some(), refuses with WalStoreError::WriterEpochAlreadyActive.
  • Lock File Acquisition: crates/warp-core/src/causal_wal.rs:5738 calls acquire_writer_epoch_lock(&self.root).
  • Lock Acquisition Core: crates/warp-core/src/causal_wal.rs:8473-8485. Opens writer-epoch.lock via OpenOptions::new().create(true).read(true).write(true).truncate(false). Invokes lock.try_lock(). On TryLockError::WouldBlock, returns Err(WalStoreError::WriterEpochLeaseUnavailable). On OS error, converts to WalStoreError::Io. On success, wraps descriptor in WriterEpochLock(lock).
  • Ledger Reload & Crash Recovery: crates/warp-core/src/causal_wal.rs:5739-5756. If self.reload_writer_epoch_ledger()? fails, or if closing a recovered active epoch fails during self.persist_writer_epoch_ledger() (line 5752), the function exits early via ?. The local variable writer_lock drops, executing WriterEpochLock::drop (let _ = self.0.unlock();), releasing the OS lock before closing the descriptor.
  • Validation & Ledger Persistence: crates/warp-core/src/causal_wal.rs:5816-5827. If validate_writer_epoch_request or persist_writer_epoch_ledger fails, writer_lock drops and unlocks.
  • Installation: crates/warp-core/src/causal_wal.rs:5828. Only after durable ledger persistence succeeds is the lock installed into the store: self.writer_lock = Some(writer_lock);.
• Pre-PR Parity Check: In 09b98b74 (crates/warp-core/src/causal_wal.rs:5738, 5828), the identical sequence was executed, but writer_lock held a raw File. Early drop closed the descriptor but did not issue an explicit unlock.

Path 2: Interface Epoch Acquisition

• Production Entry: FilesystemWalStore::acquire_writer_epoch (trait WalStorePort).
  • Conditional Acquisition: crates/warp-core/src/causal_wal.rs:5971-5975. If self.writer_lock.is_some(), reuses existing lock (writer_lock = None). Otherwise, acquires Some(acquire_writer_epoch_lock(&self.root)?).
  • Rollback Safety: crates/warp-core/src/causal_wal.rs:5976-5989. If reload, validation, or ledger persistence fails, any newly acquired writer_lock drops and unlocks. Existing held locks remain untouched.
  • Installation: crates/warp-core/src/causal_wal.rs:5990-5992. self.writer_lock = Some(writer_lock); only executes if a new lock was acquired.

Path 3: Orderly Epoch Closure

• Production Entry: FilesystemWalStore::close_epoch.
  • Precondition Check: crates/warp-core/src/causal_wal.rs:6124 verifies self.ensure_writer_lock()?.
  • Persistence Guard: crates/warp-core/src/causal_wal.rs:6137-6140. If self.persist_writer_epoch_ledger() fails, restores previous ledger state and returns early. The writer lock is NOT dropped or released.
  • Explicit Release: crates/warp-core/src/causal_wal.rs:6141 sets self.writer_lock = None;. Dropping the previous Some(WriterEpochLock) triggers WriterEpochLock::drop, executing let _ = self.0.unlock(); before closing the file descriptor.

Path 4: Store Drop & Field Teardown Order

• Production Entry: Implicit compiler-generated Drop for FilesystemWalStore.
  • Field Order: crates/warp-core/src/causal_wal.rs:5577-5586.
    1. root: PathBuf
    2. segment_id: WalSegmentId
    3. active_epoch: Option<WriterEpoch>
    4. closed_epochs: Vec<WriterEpoch>
    5. epoch_closures: BTreeMap<WriterEpochId, WriterEpochClosure>
    6. writer_lock: Option<WriterEpochLock>
    7. manifests: Vec<WalManifest>
    8. sync_evidence: Vec<FilesystemSyncEvidence>
    9. fault_plan: FilesystemWalFaultPlan
  • Evaluation: writer_lock drops cleanly at step 6. No subsequent fields rely on maintaining the OS lease during their deallocation.

Path 5: Lease-Gated Mutation Operations

• Parallel verification ensures all store mutations enforce active lease possession via ensure_writer_lock(&self):
  • flush_commit_with_capabilities: crates/warp-core/src/causal_wal.rs:5872
  • rotate_segment: crates/warp-core/src/causal_wal.rs:5934
  • append_frame: crates/warp-core/src/causal_wal.rs:6001
  • publish_manifest: crates/warp-core/src/causal_wal.rs:6095
  • close_epoch: crates/warp-core/src/causal_wal.rs:6124
• Parallel Store Implementation: InMemoryWalStore does not interact with the operating system filesystem or file leases; its lease validity is checked purely in memory via validate_writer_epoch_request.

---

3. Merge Audit & Integration Invariants

PR head 09d8c8334c398511b70533f9b79dfd505c9f9210 merges Parent 1 (00fd1e6c400b52297cb7953158a54d7e4dae0fdc) and Parent 2 (09b98b74d48dce709111528dd3db3ecb7f7c29db).

09d8c833 (HEAD: Merge main prerequisites into WAL lease release)
|\
| * 09b98b74 (main: PR #733 edict-byte-length)
| | ... includes #724, #726, #731
* | 00fd1e6c (feature: fix WAL writer lease release before closing descriptor)
|/
8c725d69 (common base: feature/edict-pure-evaluation)

Invariant Checks Against Both Parents:

1. Source Byte-Identity:
   • Diffs between 00fd1e6c and 09d8c833 for crates/warp-core/src/causal_wal.rs and docs/topics/WAL.md yield zero differences. The lease release implementation and documentation are byte-identical to Parent 1.
2. Main Prerequisites Integrity:
   • 09b98b74 merged PRs #724 (pure provider bounds/reproduction), #726 (pure evaluation), #731 (unsigned subtraction), and #733 (bounded byte length).
   • Comparing 09b98b74 to 09d8c833 across the entire repository reveals changes in exactly three files: CHANGELOG.md, crates/warp-core/src/causal_wal.rs, and docs/topics/WAL.md. None of the provider or pure evaluation modules were altered or regressed by this merge.
3. Conflict Resolution Audit (CHANGELOG.md):
   • The textual conflict in CHANGELOG.md was resolved cleanly:
     • Lines 10–13 retain the WAL writer lease release fix entry.
     • Lines 15–38 retain all five native pure provider verification fixes from Admit generic Edict pure-program contracts #724 / Evaluate compiler-produced pure Edict programs #726.
     • Lines 42–50 retain the evaluator additions from Evaluate checked unsigned subtraction in pure Edict programs #731 and Evaluate bounded byte lengths from compiler-produced programs #733.
   • No changelog entries were dropped, duplicated, or corrupted.
4. Historical Isolation:
   • Git tree inspection confirms that historical WIP branch feat/falsification was NOT merged. Commit 00fd1e6c is a clean, isolated commit on top of 8c725d69, cherry-picking the minimal logic from historical commit 54b7abc3 without importing unverified test artifacts.

---

4. Verification of Claims, Constants, and Evidence

Raw Evidence Artifacts

• Current Head Completed Run: echo-737-lawyer/validation-green.log (339 lines, 19,603 bytes).
• Current Head Launch Manifest: echo-737-lawyer/validation-green.launch.json.
• Guarded Resource Runner: echo-737-lawyer/guarded-worker.py.
• Historical RED Run: echo-writer-lease-validation/red.log (105 lines, 4,359 bytes).

Constant & Limit Validation

1. Directory Allocation Limit (MAX_DIRECTORY_ATTEMPTS = 1_024):
   • Code Reference: crates/warp-core/src/causal_wal.rs:8494.
   • Behavior: Loops 0..1_024 attempting fs::create_dir(&root). Existing candidates (ErrorKind::AlreadyExists) are skipped without deletion. If all 1,024 slots are occupied, it fails closed with panic!("no unclaimed scratch directory"). Only the allocated directory is removed at test completion (line 8532).
2. Resource Boundaries & Guard Contract (validation-green.launch.json):
   • Host free disk floor: 50 GiB (53,687,091,200 bytes). Measured PRE: 763,836,071,936 bytes; POST: 763,978,960,896 bytes. (Passed).
   • VM free disk floor: 50 GiB (53,687,091,200 bytes). Measured PRE: 725,922,054,144 bytes; POST: 726,066,241,536 bytes. (Passed).
   • Build cache limit: 20 GiB (21,474,836,480 bytes). Measured PRE: 5,955,093,634 bytes; POST: 6,047,467,705 bytes (~5.63 GiB). (Passed).
   • Non-cache data limit: 4 GiB (4,294,967,296 bytes). Measured PRE: 1,467,868,290 bytes; POST: 1,468,905,657 bytes (~1.37 GiB). (Passed).
   • Aggregate log limit: 128 MiB (134,217,728 bytes). Measured PRE: 6,289,186 bytes; POST: 6,308,789 bytes (~6.02 MiB). (Passed).
   • Workload timeout: 1500 seconds. Single log file bound: 8 MiB.
   • Container specification: Worker echo-read-runtime (echo-read-runtime:red), 4.0 CPUs, 6 GiB RAM, 512 PIDs, no host volume/bind mounts.

Test Execution & Oracle Counts (validation-green.log)

1. Unit Test (Ordinary):
   • Line 17: 1 passed; 0 failed; 0 ignored; 689 filtered out. Regression test owner_drop_releases_retained_descriptor_without_releasing_successor passed.
2. Hardening Integration Suite (Ordinary):
   • Line 48: test emit_filesystem_writer_epoch_process_step ... ignored.
   • Lines 74–82 & 87–92: Two separate single-test execution blocks for emit_filesystem_writer_epoch_process_step. These are subprocess phases invoked by parent test filesystem_writer_epoch_chain_crosses_independent_processes and must not be double-counted.
   • Line 163: test result: ok. 125 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.15s.
3. Unit Test (det_fixed):
   • Line 175: 1 passed; 0 failed; 0 ignored; 689 filtered out. Passed.
4. Hardening Integration Suite (det_fixed):
   • Line 206: emit_filesystem_writer_epoch_process_step ignored.
   • Lines 232–240 & 245–250: Subprocess phases invoked by parent test.
   • Line 321: test result: ok. 125 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.15s.
5. Static Analysis & Formatting:
   • Lines 323–332: Strict Clippy (-D warnings -D missing_docs) on library and hardening test target passed with zero warnings.
   • Lines 333–338: Strict Clippy on det_fixed library target passed with zero warnings.
   • Line 339: cargo fmt --all -- --check completed with clean exit code 0.

Historical RED Evidence

• In echo-writer-lease-validation/red.log:83-91, with WriterEpochLock retaining File but lacking the Drop unlock implementation, the regression panicked at line 8512: successor after owner drops: WriterEpochLeaseUnavailable.
• Compiler analysis at line 8461 warned field 0 is never read, demonstrating that the inner descriptor was never referenced for an explicit unlock.
• This historical RED failure proves the existence of the descriptor retention defect and confirms the validity of the regression oracle.

---

5. State Machine, Errors, and Concurrency Transitions

1. Unwind & Panic Safety:
   • In WriterEpochLock::drop, let _ = self.0.unlock(); deliberately swallows Result<(), std::io::Error>. Drop::drop cannot return an error. Attempting to panic inside drop during stack unwinding would abort the process. This best-effort release is documented in docs/topics/WAL.md:224-227.
2. Partial Acquisition Unwind:
   • If acquire_fresh_writer_epoch fails at line 5739 (reload_writer_epoch_ledger), line 5752 (persisting closed epoch), line 5816 (request validation), or line 5824 (persisting active epoch), the local writer_lock is dropped, triggering unlock and closing the descriptor. The store is never left with an active lock in an inconsistent epoch state.
3. Persist Failure During Closure:
   • In close_epoch (crates/warp-core/src/causal_wal.rs:6137-6141), self.writer_lock = None; occurs strictly AFTER self.persist_writer_epoch_ledger() succeeds. If persisting fails, the previous epoch state is reinstalled, the error is returned, and self.writer_lock remains held.
4. Old Duplicate Descriptor Teardown:
   • In crates/warp-core/src/causal_wal.rs:8525-8529, dropping retained (the old duplicate descriptor) does not release the successor's lock. The successor's lock was obtained via an independent open call, producing a distinct open file description. Closing the old duplicate does not affect the successor's flock.

---

6. Repository Standards Compliance

1. Documentation Standards (docs/DOCUMENTATION_STANDARDS.md):
   • The concept of filesystem WAL authority is canonically owned by docs/topics/WAL.md. The additions at lines 218–227 accurately reflect the best-effort unlock fallback, cite the Rust standard library file-lock contract, and link to Issue #718 for deferred process-bound authority.
   • Prose adheres to 80-column wrapping and single-claim paragraph structure.
2. Changelog Standards (CHANGELOG.md):
   • Added under ### Fixed at lines 10–13, accurately summarizing the observable fix without exaggerating guarantees.
3. Clippy & Codebase Conventions:
   • Test module uses #[allow(clippy::expect_used, clippy::panic)] on mod writer_lease_tests, adhering to the codebase convention established in PR feat(warp-core): ADR-0008 Phases 0–3 runtime primitives #300.
   • Production code in causal_wal.rs introduces no expect, unwrap, or unhandled lints, compiling cleanly under -D warnings -D missing_docs.
4. Git Rules (AGENTS.md):
   • History contains standard non-amended commits (00fd1e6c and 09d8c833). No rebase, force push, or push to main occurred.

---

7. Findings & Explicit Evidence Limits

Demonstrated Code Defects: None (No P0–P5 Findings)

No active defects, resource leaks, regression landmines, or lint violations were identified in the changed files.

Explicit Coverage Gaps & Evidence Limits

1. Best-Effort Drop Limitation:
   • Detail: If File::unlock() encounters an operating system error (e.g. EIO on a degraded network filesystem), WriterEpochLock::drop silently discards the error. The caller cannot inspect or handle this failure.
   • Status: Disclosed and accepted as documented scope. Full explicit fallible release is tracked in Issue #718.
2. Determinism Witness Modeling vs. Physical Fork/Exec:
   • Detail: The regression uses try_clone() within a single process to model the duplicate open-file-description lifetime. It does not execute an actual multi-process fork()/exec() sequence or test hardware power loss.
   • Status: Disclosed and accepted as documented scope. Multi-process authority tests remain in #718.
3. Static Review Audit:
   • Detail: In accordance with the adversarial review protocol, this review inspected existing execution logs and receipts. No new test runs, builds, or container executions were performed by this reviewer.

---

8. Verification Checklist

• Every code path traced:
  • Fresh acquisition: crates/warp-core/src/causal_wal.rs:5731-5830
  • Port interface acquisition: crates/warp-core/src/causal_wal.rs:5967-5994
  • Orderly closure: crates/warp-core/src/causal_wal.rs:6123-6143
  • Drop / unwind: crates/warp-core/src/causal_wal.rs:8463-8469
  • Mutation guards: crates/warp-core/src/causal_wal.rs:5872, 5934, 6001, 6095, 6124
  • In-memory parallel path: crates/warp-core/src/causal_wal.rs:2014-2045
• Every merge audited:
  • Parent 1 SHA: 00fd1e6c400b52297cb7953158a54d7e4dae0fdc
  • Parent 2 SHA: 09b98b74d48dce709111528dd3db3ecb7f7c29db
  • Head SHA: 09d8c8334c398511b70533f9b79dfd505c9f9210
  • Verified byte-identity of lease implementation between 00fd1e6c and 09d8c833.
  • Verified no regression or loss of merged main prerequisites (Admit generic Edict pure-program contracts #724, Evaluate compiler-produced pure Edict programs #726, Evaluate checked unsigned subtraction in pure Edict programs #731, Evaluate bounded byte lengths from compiler-produced programs #733).
  • Verified clean conflict resolution in CHANGELOG.md.
• Every constant and claim checked against evidence:
  • MAX_DIRECTORY_ATTEMPTS = 1_024: crates/warp-core/src/causal_wal.rs:8494.
  • Timeout: 1500s; Log cap: 8 MiB; Monitor interval: 2s: echo-737-lawyer/guarded-worker.py.
  • Limits: 20 GiB build, 4 GiB data, 128 MiB logs, 50 GiB disk floor: echo-737-lawyer/validation-green.launch.json.
• Every numeric figure checked:
  • Unit regression test count: 1 passed in ordinary and det_fixed.
  • Hardening test count: 125 passed, 1 ignored in ordinary and det_fixed (echo-737-lawyer/validation-green.log:163, 321).
  • Subprocess invocations: 2 extra lines accounted for via parent witness (echo-737-lawyer/validation-green.log:74-92, 232-250).
  • Storage usage: POST 6,047,467,705 build bytes (~5.63 GiB), 1,468,905,657 data bytes (~1.37 GiB), 6,308,789 log bytes (~6.02 MiB).
• Checks executed vs inspected:
  • Static source/git inspection: EXECUTED.
  • Review evidence inspection: EXECUTED.
  • Docker execution & compiler checks: INSPECTED via validated logs (echo-737-lawyer/validation-green.log); NOT rerun.
  • Physical power loss / fork authority: UNAVAILABLE by design; deferred to #718.

---

Verdict

APPROVE

Code Lawyer Review Addendum: PR #737 (09d8c8334c398511b70533f9b79dfd505c9f9210)

This addendum records report corrections without asserting new runtime findings:

1. Application Acceptance Coordinates: Downstream acceptance tracking resides in JedIT persistent-rope integration (jedit#296 and jedit#302), not Echo WASM ABI acceptance.
2. Log Boundaries & Format Evidence: validation-green.log terminates at line 338 (nl -ba). Because cargo fmt --all -- --check is silent on success, no line 339 command exists. Passing status is inferred indirectly via runner completion EXIT 0 over the launch manifest chain (validation-green.launch.json), not via explicit log output.
3. RED Witness Disclosures: red.log comprises 117 lines. Lines 85–95 record a compiler dead_code warning for unused field 0, which does not prove lack of unlock. The genuine refusal witness is the panic at lines 105–108 (successor after owner drops: WriterEpochLeaseUnavailable) and summary failure at line 115.
4. Resource Accounting Source: POST metrics (6,047,467,705 build; 1,468,905,657 data; 6,308,789 logs bytes) stem from the runner’s post-execution receipt in the prompt, distinct from PRE-run metrics in validation-green.launch.json (5,955,093,634 build; 1,467,868,290 data; 6,289,186 logs). The 2-second fail-closed monitor (guarded-worker.py) was inspected statically; live dynamic enforcement was not directly witnessed.
5. Assurance & Regression Limits: Byte-identity with parent 00fd1e6c provides merge compatibility evidence, not proof of absence of all regressions. WriterEpochLock::drop executes best-effort unlock via fs2::FileExt::unlock; OS-level unlock failures cannot be prevented. Process/fork lifecycle authority remains deferred to Make OS lease guards prove process-bound release across inherited descriptors #718.
6. Conventions: Extraneous attribution to PR feat(warp-core): ADR-0008 Phases 0–3 runtime primitives #300 is retracted; single-line wrapping reflects local observed conventions rather than mandatory cross-repository policy.

---

Mandatory Verification Checklist (Preserved & Corrected)

• Runtime Paths Traced:
  • acquire_writer_epoch: causal_wal.rs:5967-5993 acquires lock via acquire_writer_epoch_lock into WriterEpochLock.
  • acquire_fresh_writer_epoch: causal_wal.rs:5738 binds temporary WriterEpochLock, best-effort unlocked on drop before descriptor close (causal_wal.rs:8463-8469).
  • close_epoch: causal_wal.rs:6141 clears self.writer_lock = None, invoking WriterEpochLock::drop.
  • CausalWalStore: writer_lock: Option<WriterEpochLock> held at causal_wal.rs:5582.
• Merge Audited: 09d8c833 unifies 00fd1e6c and 09b98b74. Resolved conflict in CHANGELOG.md:10-14 correctly retains lease release alongside merged provider PRs (Admit generic Edict pure-program contracts #724, Evaluate compiler-produced pure Edict programs #726, Evaluate checked unsigned subtraction in pure Edict programs #731, Evaluate bounded byte lengths from compiler-produced programs #733). WAL implementation is byte-identical to 00fd1e6c.
• Constants & Claims Verified:
  • Scratch directory claim bound (1024) verified at causal_wal.rs:8493.
  • Duplicated-descriptor regression witness verified at causal_wal.rs:8507-8547.
  • Passing test suites: 1 regression + 125 hardening tests in standard and det_fixed modes (validation-green.log:1-338).
• Scope & Limitations: Static inspection only; physical power loss, crash-fork authority, and explicit fallible unlock reporting deferred to Make OS lease guards prove process-bound release across inherited descriptors #718.

---

APPROVE

Final activity summary and merge gate

• Integration commit 09d8c833 is signed, pushed, and clean. Both merge parents and the changelog conflict were inspected; no source change was needed beyond the existing lease fix.
• Current-head Docker evidence passed: one retained-descriptor regression plus 125 WAL hardening tests in each ordinary and det_fixed configuration, strict Clippy, formatting. The intentionally ignored subprocess entry is exercised by its parent witness. The primary runner returned EXIT 0; its receipt is transcribed separately in validation-completion.json.
• All 40 hosted checks passed. Complete GraphQL retrieval found no review threads and no changes-requested reviews. Repository rules require signed commits and resolved threads, with zero formal approval count; the authorized independent scoped APPROVE supplies the Code Lawyer review gate.
• No actionable code findings remain. OS unlock failures, explicit fallible release, and fork/process authority remain disclosed under Make OS lease guards prove process-bound release across inherited descriptors #718. Existing Jim producer pins remain unchanged.
• Merge is already authorized by the user. No draft PR or protected-branch bypass is involved.