Repository navigation
fix: recover native first-fsync lock crashes before TTL expiry - #105
Merged
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
Reviewer's GuideThe PR adds narrowly scoped Linux recovery for first-fsync lock crashes, backed by an extensive isolated syscall-injection harness that compares exact durable state and restart receipts, plus CI, documentation, fixtures, and 50-writer concurrency validation. Sequence diagram for first-fsync lock crash recoverysequenceDiagram
participant Mutation
participant Lock as Lock manager
participant Gate as Publication gate
participant Kernel as Linux kernel lease
participant Owner as Original process
participant Recovery as Durable recovery
Mutation->>Lock: acquire_lock_attempt()
Lock->>Gate: write complete lock payload
Lock->>Gate: fsync()
Kernel-->>Lock: SIGKILL during first fsync
Mutation--xOwner: process dies
Recovery->>Kernel: hold permanent lease
Recovery->>Owner: confirmed_dead_owner()
Recovery->>Gate: read_lock_owner()
alt identity matches and gate is empty
Recovery->>Recovery: write recovery intent receipt
Recovery->>Gate: reclaim_stale_gate()
Recovery-->>Mutation: restart succeeds before TTL expiry
else live, unknown, replaced, or occupied gate
Recovery-->>Mutation: LockConflict
end
Flow diagram for syscall-injection crash corpus validationflowchart TD
A[Create acknowledged synthetic seed] --> B[Run update or comment reference]
B --> C[Observe task-local rename and fsync ordinals]
C --> D[Inject SIGKILL at each declared boundary]
D --> E[Restart the same operation family]
E --> F{State and receipt match reference?}
F -->|yes| G[Verify exact item/history bytes]
F -->|no| H[Retain bounded synthetic failure evidence]
G --> I{Native and published parity?}
I -->|yes| J[Pass corpus boundary]
I -->|no| H
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…into fix/crash-restart-recovery
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…into fix/crash-restart-recovery
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
unbraind
commented
Oct 23, 2026
unbraind
left a comment
unbraind
left a comment
Owner
Author
There was a problem hiding this comment.
Orchestrator review (built-in Claude review agent, verified by the orchestrator; bots are not waited for since 2026-10-22):
VERDICT: MERGE
FINDINGS:
- none
SUMMARY: The PR recovers fresh native lock gates before TTL expiry after confirming owner death, preserving lease, identity, emptiness and audit safeguards. Static review found meaningful crash-replay, refusal and concurrency tests with unchanged coverage gates; tests were not rerun locally.
STOP.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Native update/comment killed at the first lock fsync preserves acknowledged data but previously restarted with lock-conflict exit 4. This change admits recovery of a fresh empty publication gate while holding its permanent kernel lease and verifying that the original process is dead on the same Linux host/namespace. Gate identity and emptiness checks remain mandatory; live, unknown, occupied and replacement gates retain their refusal behavior. The durable recovery-intent receipt precedes lock removal. Item/history serialization is unchanged.
This branch includes the PR 104 crash-harness stack and retains the original requirement to merge after PR 104. The corpus measures real SIGKILL injection at observed task-local rename/fsync entries, compares complete reference item/history bytes and raw restart receipts, and retains minimal synthetic reproductions. Each runtime uses its own exact restart reference; the shared cross-runtime acknowledgement projection is update id/status or comment id/comments.
Current main
300eaa9a9d06f0e2cfc6afcfa8b66492a960a77bis merged. Conflict resolution retains both crash/benchmark recipes and ignore rules, the main non-git process helper, and the stricter 50-writer workload of three comments per writer. The expected marker helper covers all 150 unique comments; the test also verifies every marker exactly once and the complete 151-record history chain. The documented helper keeps the existing 100-line Clippy function limit intact. A stale harness comment now describes fixed reference invocations after main introduced shared native journal recovery.Behavioral controls at the integrated source:
mutation::acquire_lock_attempt, rebuild, and runnode --test --test-name-pattern="retained (update|comment)" scripts/crash-harness.test.mts: two failures, both restart exit 4. Restore/rebuild: both pass.expected_markers, leaving the three-comment workload intact: the concurrency regression compiles and fails with exit 101, actual 150 markers versus expected 50. Restore the full marker set: the Rust acceptance command passes.Pinned-PM linked acceptance passes all three commands (44 cases, zero failures or skips on Linux):
node --test scripts/crash-harness.test.mtsenv CARGO_BUILD_JOBS=2 PM_RUST_REQUIRE_PUBLISHED_CLI=1 cargo +1.90.0 test --locked --test concurrency_contract --test parity_1021_contractenv CARGO_BUILD_JOBS=2 cargo +1.90.0 test --locked --lib gateStrict erasable TypeScript, ESLint, Rust formatting, all-target/all-feature Clippy, private rustdoc, changelog regeneration/check inside the merge, and isolated required check-only tracker health pass. Existing tracker policy disables runtime result persistence; append-only item comments retain the linked receipts and negative controls.
Full CI evidence: run 40018857828 passed all nine jobs for
c03cd0dce87e99e4f5c2c952b4311b8908742451. All three unchangedjust release-checkaggregates passed. Ordinary/instrumented Rust counts are Linux 601/601, macOS 589/589 and Windows 583/583, with zero failures or ignored tests. Lines, functions, regions and branches are exactly 100 percent on every platform at unchanged scope and thresholds. The Linux crash harness passes 22/22 with zero skips; macOS passes 13 with nine declared Linux-only skips, and Windows passes 12 with ten declared platform skips. TypeScript checking, ESLint, private rustdoc, Clippy, audit, changelog and required check-only tracker health pass. All three separate Rust and latency jobs pass, and CodeQL succeeds. Full release evidence comes from CI; no incomplete local cold build is counted as a full gate.Retained historical evidence and boundaries:
Process-kill evidence does not establish power-loss durability, unmeasured syscall coverage, cross-runtime journal recovery, or a TypeScript coverage percentage. The existing comment lock-diagnostic mismatch and older tracker chain advisories remain separately tracked; check-only health does not certify those older history chains.
GraphQL reports zero review threads. CodeRabbit skipped automatic review and provided no review body or outside-diff findings; unavailable or skipped reviewers are missing review evidence. No thread replies/resolutions, PR merge, release, dependency-pin change or threshold reduction was performed.
PM: recovery work and revert receipts, crash harness. The recovery item remains in progress with its claim released for orchestrator verification.