kayg/calternal
Watch
1
0
Fork
You've already forked calternal
 0
Code Issues 190 Pull requests 61 Projects Releases Packages Wiki Activity Actions 20

Recover interrupted renames and abandoned note save images #737

Closed
kayg wants to merge 1 commit from fix/storage-crash-recovery into dev
pull from: fix/storage-crash-recovery
merge into: kayg:dev
Conversation 4 Commits 1 Files changed 16 +658 -6
kayg commented 2026-10-28 17:49:36 +00:00
Owner
Copy link

A crash after a rename placed the file but before it updated the path index could make replay load the missing old path, fail the job, and leave its title unchanged. Recover placements by embedded identity before Home writers start, and finish an already-stored rename plan when its request is retried so the request journal receives a terminal result.

Recover abandoned save images only when their bytes are already durable in a canonical file or a journal version, including interrupted prefixes. The filesystem helper claims each image exclusively and holds a Linux read lease during proof and removal. Active outside writers, unknown bytes, broken leases, and unsupported leases retain the file with a warning. The small integer-only lease syscall wrapper follows the existing calternal-memory lint pattern. Directory inspection errors retain files without denying access to the Home.

Save the anchor test's edited source before its invariant snapshot. BlockChanged, exact source preservation, and the destination-body assertion remain intact.

Validation:

  • Deterministic SIGKILL before the rename index update reproduces the original failure and now recovers, retries, and undoes. Background replay is held so the original-key retry must finish the plan itself.
  • 200 real-binary rename kills passed path, heading, title, and same-key done checks.
  • 1,000 mixed real-binary kills passed terminal-job checks and all 1,000 acknowledged primary append checks. Final canonical bytes matched, zero orphan staging files remained, and all five SQLite checks passed. Home and state used separate private tmpfs filesystems; this tests process death.
  • 100 separate anchor test runs passed, zero failed.
  • Frozen Bun install, format check, workspace clippy with warnings denied, and workspace tests passed. Workspace output reports 1,162 passed, 0 failed, 20 ignored across 88 completed groups including doc tests. Targeted storage/crash suites: 16 passed, 1 ignored; rename crash tests: 3 passed; staging selection: 9 passed; split-save selection: 2 passed.

Related to #642.

A crash after a rename placed the file but before it updated the path index could make replay load the missing old path, fail the job, and leave its title unchanged. Recover placements by embedded identity before Home writers start, and finish an already-stored rename plan when its request is retried so the request journal receives a terminal result. Recover abandoned save images only when their bytes are already durable in a canonical file or a journal version, including interrupted prefixes. The filesystem helper claims each image exclusively and holds a Linux read lease during proof and removal. Active outside writers, unknown bytes, broken leases, and unsupported leases retain the file with a warning. The small integer-only lease syscall wrapper follows the existing calternal-memory lint pattern. Directory inspection errors retain files without denying access to the Home. Save the anchor test's edited source before its invariant snapshot. BlockChanged, exact source preservation, and the destination-body assertion remain intact. Validation: - Deterministic SIGKILL before the rename index update reproduces the original failure and now recovers, retries, and undoes. Background replay is held so the original-key retry must finish the plan itself. - 200 real-binary rename kills passed path, heading, title, and same-key done checks. - 1,000 mixed real-binary kills passed terminal-job checks and all 1,000 acknowledged primary append checks. Final canonical bytes matched, zero orphan staging files remained, and all five SQLite checks passed. Home and state used separate private tmpfs filesystems; this tests process death. - 100 separate anchor test runs passed, zero failed. - Frozen Bun install, format check, workspace clippy with warnings denied, and workspace tests passed. Workspace output reports 1,162 passed, 0 failed, 20 ignored across 88 completed groups including doc tests. Targeted storage/crash suites: 16 passed, 1 ignored; rename crash tests: 3 passed; staging selection: 9 passed; split-save selection: 2 passed. Related to #642.
fix: recover interrupted note renames and staged saves
Some checks failed
CI / server (pull_request) Successful in 57s
Details
CI / rust (pull_request) Successful in 20m26s
Details
CI / web (pull_request) Failing after 2h59m10s
Details
66f27ce3cd
kayg commented 2026-10-28 18:24:54 +00:00
Author
Owner
Copy link

Verdict: fix first. Reviewed head 66f27ce3cd953dbdcf2be43123f3e069136753c1; merge base 3d0c6e33d588335c745a660163bf8bf5cdc8412d. No product changes, commits, pushes, merges or deployments.

P1: cleanup can discard the only newer external version when the canonical changes during recovery. In crates/fsio/src/staging.rs:129, durable_file_starts_with reads the canonical into memory, then fsyncs it and returns true. The caller at crates/notes/src/history/staging.rs:45 accepts that result without journal proof. recover_staging checks only its staging inode before unlinking at staging.rs:90. Another program can change the canonical between comparison and deletion, invalidating the evidence for disposal.

Independent deterministic repro against the ordinary binary:

  1. Save initial version B and stop the server. Write a unique newer external version A to the canonical and an adjacent .calternal-tmp-f737-Canonical race.md. A has never been sent to the server or journal.
  2. Restart with an fsync shim that resolves the fd pathname and pauses only canonical fsync using a FIFO handshake. This occurs after recovery reads A. The shim changes no syscall results.
  3. From the harness process, restore older B to the canonical, release fsync, and await notes.get.
  4. Canonical is B; staging is deleted; no Home file contains A. The only newer external version was discarded.

Exact observation:

Stage exists=False; remaining Home files containing complete A=[]

Repro sources and evidence: /home/agent/wt/verify-737/.verification/{fsync-pause.c,race.py,verify.py,canonical-race.log}. Build the shim with gcc -shared -fPIC -O2 -o .verification/fsync-pause.so .verification/fsync-pause.c; run the repro on a fresh fixture with /usr/local/bin/build-slot.sh timeout 120 python3 .verification/race.py. The fixture is separate from the durability loop. This is a real interprocess write race, synchronized without fixed sleeps. Suggested fix: require independent durable history evidence or durably preserve unknown external bytes before removal; a point-in-time canonical read/fsync cannot prove that those bytes remain recoverable.

Independent checks that passed:

  • SIGKILL after rename file placement before index update, after the move step before progress save, and after the title/backlink edit batch before progress save. Original request-key retries returned done, with complete step counts, the new path and heading, updated backlink, no old path, and the acknowledged Unicode body preserved.
  • Reflected staging image removed; newer external staging image retained; external canonical change made before restart preserved both versions byte for byte.
  • Anchors still assert BlockChanged, exact source-text equality and destination-body preservation. All six filtered tests passed. fsio: 40 passed; notes staging: 2 passed; split-save crash: 2 passed; binary storage/crash suites: 16 passed, 1 existing stress test ignored.
  • Fresh durability loop: 1,000/1,000 abrupt stops and restarts passed, with 250 each of saves, renames, Trash and restore. All 1,000 acknowledged Unicode markers survived exactly once after every restart. Original-key retries reached terminal success, auxiliary heading/path/body stayed correct, final canonical bytes equaled notes.get, and no staging leftovers remained after each restart and explicit save barriers.

The original loop stopped at round 143 because its staging scan could overlap a live automatic save. All acknowledged text checks passed before that stop. That failed gate remains recorded; it is a harness limitation, not a second product finding. The fresh run checked after explicit save barriers on both notes. Randomized kills do not prove which syscall was interrupted; the deterministic rename and save boundary tests provide separate coverage. These are process-death tests on local storage, not block-device power-loss tests.

Gate output, verbatim:

independent-rename exit=0
fsio exit=0
staging exit=0
anchors exit=0
storage-crash exit=0
binary-storage exit=0
independent-staging exit=0
canonical-race exit=0
durability-1000 exit=1
durability-1000-settled exit=0

canonical-race exit=0 means the P1 defect was successfully reproduced; it is not a safety pass. durability-1000 exit=1 is the original scan-overlap harness run; durability-1000-settled exit=0 is the fresh completed 1,000-round run.

The ordinary binary build passed under build-slot. An initial build command used a nonexistent package feature and was corrected before compilation. Every local build/test and the durability harness ran through one build slot at a time, with cargo -j3. The temporary independent Rust test registration has been removed; git diff --exit-code passes. Only uncommitted review artifacts remain, at /home/agent/wt/verify-737/findings.md and .verification/. No issue was closed.

Verdict: **fix first**. Reviewed head `66f27ce3cd953dbdcf2be43123f3e069136753c1`; merge base `3d0c6e33d588335c745a660163bf8bf5cdc8412d`. No product changes, commits, pushes, merges or deployments. **P1: cleanup can discard the only newer external version when the canonical changes during recovery.** In `crates/fsio/src/staging.rs:129`, `durable_file_starts_with` reads the canonical into memory, then fsyncs it and returns true. The caller at `crates/notes/src/history/staging.rs:45` accepts that result without journal proof. `recover_staging` checks only its staging inode before unlinking at `staging.rs:90`. Another program can change the canonical between comparison and deletion, invalidating the evidence for disposal. Independent deterministic repro against the ordinary binary: 1. Save initial version B and stop the server. Write a unique newer external version A to the canonical and an adjacent `.calternal-tmp-f737-Canonical race.md`. A has never been sent to the server or journal. 2. Restart with an fsync shim that resolves the fd pathname and pauses only canonical fsync using a FIFO handshake. This occurs after recovery reads A. The shim changes no syscall results. 3. From the harness process, restore older B to the canonical, release fsync, and await `notes.get`. 4. Canonical is B; staging is deleted; no Home file contains A. The only newer external version was discarded. Exact observation: ``` Stage exists=False; remaining Home files containing complete A=[] ``` Repro sources and evidence: `/home/agent/wt/verify-737/.verification/{fsync-pause.c,race.py,verify.py,canonical-race.log}`. Build the shim with `gcc -shared -fPIC -O2 -o .verification/fsync-pause.so .verification/fsync-pause.c`; run the repro on a fresh fixture with `/usr/local/bin/build-slot.sh timeout 120 python3 .verification/race.py`. The fixture is separate from the durability loop. This is a real interprocess write race, synchronized without fixed sleeps. Suggested fix: require independent durable history evidence or durably preserve unknown external bytes before removal; a point-in-time canonical read/fsync cannot prove that those bytes remain recoverable. Independent checks that passed: - SIGKILL after rename file placement before index update, after the move step before progress save, and after the title/backlink edit batch before progress save. Original request-key retries returned `done`, with complete step counts, the new path and heading, updated backlink, no old path, and the acknowledged Unicode body preserved. - Reflected staging image removed; newer external staging image retained; external canonical change made before restart preserved both versions byte for byte. - Anchors still assert `BlockChanged`, exact source-text equality and destination-body preservation. All six filtered tests passed. fsio: 40 passed; notes staging: 2 passed; split-save crash: 2 passed; binary storage/crash suites: 16 passed, 1 existing stress test ignored. - Fresh durability loop: **1,000/1,000 abrupt stops and restarts passed**, with 250 each of saves, renames, Trash and restore. All 1,000 acknowledged Unicode markers survived exactly once after every restart. Original-key retries reached terminal success, auxiliary heading/path/body stayed correct, final canonical bytes equaled `notes.get`, and no staging leftovers remained after each restart and explicit save barriers. The original loop stopped at round 143 because its staging scan could overlap a live automatic save. All acknowledged text checks passed before that stop. That failed gate remains recorded; it is a harness limitation, not a second product finding. The fresh run checked after explicit save barriers on both notes. Randomized kills do not prove which syscall was interrupted; the deterministic rename and save boundary tests provide separate coverage. These are process-death tests on local storage, not block-device power-loss tests. Gate output, verbatim: ```text independent-rename exit=0 fsio exit=0 staging exit=0 anchors exit=0 storage-crash exit=0 binary-storage exit=0 independent-staging exit=0 canonical-race exit=0 durability-1000 exit=1 durability-1000-settled exit=0 ``` `canonical-race exit=0` means the P1 defect was successfully reproduced; it is not a safety pass. `durability-1000 exit=1` is the original scan-overlap harness run; `durability-1000-settled exit=0` is the fresh completed 1,000-round run. The ordinary binary build passed under build-slot. An initial build command used a nonexistent package feature and was corrected before compilation. Every local build/test and the durability harness ran through one build slot at a time, with cargo `-j3`. The temporary independent Rust test registration has been removed; `git diff --exit-code` passes. Only uncommitted review artifacts remain, at `/home/agent/wt/verify-737/findings.md` and `.verification/`. No issue was closed.
kayg commented 2026-10-28 18:29:50 +00:00
Author
Owner
Copy link

Finding confirmed: the new permanent test staging_canonical_race_keeps_equal_unjournaled_bytes fails on 66f27ce3 because recovery deletes stage A when canonical A matches; restoring canonical B immediately after cleanup then loses the only copy of unjournaled A. Exact failure: std::fs::read(&stage).unwrap() returned NotFound; 0 passed, 1 failed.

The fix uses independent durable journal evidence for removal. A stable canonical snapshot alone is insufficient for the requested just-after case: another program can replace the pathname immediately after the last check or lease release. Canonical files therefore no longer authorize deletion at all. Unknown bytes stay in the original staging path and are explicitly reported as a staging conflict copy; they are never imported under a duplicate note ID. This also removes the racy canonical helper rather than leaving an unsafe public API. Journal snapshots, staging leases, exclusive claims, byte rechecks and directory fsync remain.

Finding confirmed: the new permanent test staging_canonical_race_keeps_equal_unjournaled_bytes fails on 66f27ce3 because recovery deletes stage A when canonical A matches; restoring canonical B immediately after cleanup then loses the only copy of unjournaled A. Exact failure: std::fs::read(&stage).unwrap() returned NotFound; 0 passed, 1 failed. The fix uses independent durable journal evidence for removal. A stable canonical snapshot alone is insufficient for the requested just-after case: another program can replace the pathname immediately after the last check or lease release. Canonical files therefore no longer authorize deletion at all. Unknown bytes stay in the original staging path and are explicitly reported as a staging conflict copy; they are never imported under a duplicate note ID. This also removes the racy canonical helper rather than leaving an unsafe public API. Journal snapshots, staging leases, exclusive claims, byte rechecks and directory fsync remain.
kayg commented 2026-10-28 18:37:48 +00:00
Author
Owner
Copy link

Finished storage-fix-2. Head e46ce1c306, branch fix/storage-staging-cleanup-race, base 66f27ce3cd. Replacement PR: #740. PR #737's branch is unchanged; the job's forge rule required a new branch and PR.

Finding: canonical A was read, another program restored B during canonical fsync, then cleanup deleted the only unjournaled A. The permanent FIFO test reaches that exact handshake and detects the loss against the original ordinary binary.

Fix: only independent durable journal evidence can authorize deletion. A mutable canonical match, even one checked again, cannot prove safety immediately after the check. The canonical-only helper and shortcut are removed. Unproven staging bytes remain byte for byte at their recovery path and are explicitly reported as a staging conflict copy. Existing staging leases, exclusive claims and byte checks remain. There is no added per-keystroke or vault-wide hot-path work.

Tests: the original repro now passes against this ordinary binary. Rewrite and pathname replacement cases pass before, at the former comparison boundary, and immediately after recovery; the removed canonical check is handled by racing at completion when no fsync arrives. Equal unjournaled bytes also survive repeated recovery. All final fmt, Clippy -D warnings, workspace, staging and canonical-race gates passed. The settled 1,000-stop loop passed all 1,000 rounds, 250 of each operation, with every acknowledged Unicode marker present exactly once after every restart and no staging leftovers after save barriers.

The first workspace and canonical-race invocations failed before recovery because the new fixture omitted Idempotency-Key. That fixture error was corrected and both gates were rerun successfully. The original-head expected-loss gate confirms the old defect, not a safety pass. Existing ignored tests remain ignored. These checks cover process death on local storage, not block-device power loss.

Gate output, verbatim:

fmt exit=0
clippy exit=0
workspace-tests exit=101
staging-race exit=0
canonical-race exit=101
build-binary exit=0
durability-1000-settled exit=0
reviewer-repro-original-head expected-loss-observed exit=0
canonical-race-corrected exit=0
workspace-tests-corrected exit=0
fmt-final exit=0
clippy-final exit=0

The corrected binary repro output, verbatim:

    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.21s
     Running tests/staging_canonical_race.rs (target/debug/deps/staging_canonical_race-da30e4ab2e4e5aa1)

running 1 test
canonical-only disposal check absent; race at completion
canonical race during, rewrite: PASS
canonical-only disposal check absent; race at completion
canonical race during, replace: PASS
canonical race before, rewrite: PASS
canonical race before, replace: PASS
canonical race after, rewrite: PASS
canonical race after, replace: PASS
test staging_canonical_race_before_during_and_after_recovery ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.53s

The settled loop result, verbatim:

Check 5 PASS: 1000/1000 abrupt stops and restarts completed: 250 saves, 250 renames, 250 Trash operations, 250 restores. All 1000 acknowledged text markers survived exactly once after every restart; final canonical bytes equal notes.get; no staging leftovers after restart and explicit save barriers in any round. Randomized SIGKILL timing does not prove a particular syscall was interrupted; deterministic boundary checks supply that coverage.

Evidence remains uncommitted in /home/agent/wt/storage-fix-1/.verification/. No evidence artifacts were committed. No issue was closed. No merge or deployment.

Finished storage-fix-2. Head e46ce1c30652c051332123fbbb9db49abfc9c73b, branch fix/storage-staging-cleanup-race, base 66f27ce3cd953dbdcf2be43123f3e069136753c1. Replacement PR: https://git.kayg.org/kayg/calternal/pulls/740. PR #737's branch is unchanged; the job's forge rule required a new branch and PR. Finding: canonical A was read, another program restored B during canonical fsync, then cleanup deleted the only unjournaled A. The permanent FIFO test reaches that exact handshake and detects the loss against the original ordinary binary. Fix: only independent durable journal evidence can authorize deletion. A mutable canonical match, even one checked again, cannot prove safety immediately after the check. The canonical-only helper and shortcut are removed. Unproven staging bytes remain byte for byte at their recovery path and are explicitly reported as a staging conflict copy. Existing staging leases, exclusive claims and byte checks remain. There is no added per-keystroke or vault-wide hot-path work. Tests: the original repro now passes against this ordinary binary. Rewrite and pathname replacement cases pass before, at the former comparison boundary, and immediately after recovery; the removed canonical check is handled by racing at completion when no fsync arrives. Equal unjournaled bytes also survive repeated recovery. All final fmt, Clippy -D warnings, workspace, staging and canonical-race gates passed. The settled 1,000-stop loop passed all 1,000 rounds, 250 of each operation, with every acknowledged Unicode marker present exactly once after every restart and no staging leftovers after save barriers. The first workspace and canonical-race invocations failed before recovery because the new fixture omitted Idempotency-Key. That fixture error was corrected and both gates were rerun successfully. The original-head expected-loss gate confirms the old defect, not a safety pass. Existing ignored tests remain ignored. These checks cover process death on local storage, not block-device power loss. Gate output, verbatim: ```text fmt exit=0 clippy exit=0 workspace-tests exit=101 staging-race exit=0 canonical-race exit=101 build-binary exit=0 durability-1000-settled exit=0 reviewer-repro-original-head expected-loss-observed exit=0 canonical-race-corrected exit=0 workspace-tests-corrected exit=0 fmt-final exit=0 clippy-final exit=0 ``` The corrected binary repro output, verbatim: ```text Finished `test` profile [unoptimized + debuginfo] target(s) in 0.21s Running tests/staging_canonical_race.rs (target/debug/deps/staging_canonical_race-da30e4ab2e4e5aa1) running 1 test canonical-only disposal check absent; race at completion canonical race during, rewrite: PASS canonical-only disposal check absent; race at completion canonical race during, replace: PASS canonical race before, rewrite: PASS canonical race before, replace: PASS canonical race after, rewrite: PASS canonical race after, replace: PASS test staging_canonical_race_before_during_and_after_recovery ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.53s ``` The settled loop result, verbatim: ```text Check 5 PASS: 1000/1000 abrupt stops and restarts completed: 250 saves, 250 renames, 250 Trash operations, 250 restores. All 1000 acknowledged text markers survived exactly once after every restart; final canonical bytes equal notes.get; no staging leftovers after restart and explicit save barriers in any round. Randomized SIGKILL timing does not prove a particular syscall was interrupted; deterministic boundary checks supply that coverage. ``` Evidence remains uncommitted in /home/agent/wt/storage-fix-1/.verification/. No evidence artifacts were committed. No issue was closed. No merge or deployment.
kayg commented 2026-11-04 13:04:31 +00:00
Author
Owner
Copy link

Closing: superseded by #740, then #742, #744, #749 and now #750 (named in the PR body or comments). Branch kept.

Closing: superseded by #740, then #742, #744, #749 and now #750 (named in the PR body or comments). Branch kept.
kayg closed this pull request 2026-11-04 13:04:31 +00:00
Some checks failed
CI / server (pull_request) Successful in 57s
Details
CI / rust (pull_request) Successful in 20m26s
Details
CI / web (pull_request) Failing after 2h59m10s
Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
backend
Rust server

  
design
Mockups, UI and TASTE

  
docs
ADRs, CONTEXT, TASTE

  
idea
Parked idea, never worked until promoted

  
infra
Deploy, CI, VMs

  
perf
Performance and budgets

  
reliability
Data integrity, corruption traps

  
web
Web client

No labels
backend
design
docs
idea
infra
perf
reliability
web
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal!737
No description provided.