fix(snapshot): live Copy-mode backup; bound crash-child fixtures

[ScriptedAlchemy]

Two distinct changes

This PR still fixes #933, stacked on codex/tracedecay-total-redesign-plan-reopened (PR #707). Do not undraft or merge #707.

The production snapshot path and the crash fixtures are separate claims.

1. Production: Copy-mode live snapshot (not a test-only change)

SnapshotMode::Copy and backup_live_sqlite_database use the SQLite online backup API. They do not fs::copy a locked Windows main/WAL/SHM family, and they do not open a changing source with immutable=1.

Publication boundary

• Cancel/deadline is checked before any scratch is created.
• Staging is reserved with create_new (owner-only on Unix). A name collision is refused; the colliding file is not deleted.
• Only that reserved file is retired on failure.
• Unix: rename(2) atomically replaces an existing destination. Failure leaves the old file at its original path.
• Non-Unix: an existing destination is rejected (AlreadyExists). Displace/restore is not used; it cannot keep the old snapshot at its path until replace succeeds. Scratch and LCM callers already publish to a new path.

Acceptance in sqlite_read_snapshot_backup_tests.rs:

• Concurrent WAL writer → coherent committed prefix.
• WAL-resident rows included without checkpointing the source family.
• Cancel/deadline interrupt page-copy work and DELETE-journal Busy/Locked retries (Busy/Locked probed first).
• Failed/cancelled backup never publishes a new destination and never rewrites a prior destination.
• Exclusive staging refuses a collision without deleting it.
• Unix atomically replaces an existing destination; non-Unix refuses that replace and keeps the old file.
• Foreign snapshots leave source main/WAL/SHM untouched.
• Successful backup is not a freshness claim after the source changes.
• Windows: live WAL writer survives backup to a new path; raw fs::copy still fails with OS 32/33.

2. Fixtures: subprocess unclean-exit, not power-loss

Grafeo crash/reopen tests spawn a child that reaches the durable WAL phase and exits without a clean close. The parent bounds that process (45s), kills and reaps it on timeout, and fails if the durable-phase marker is missing. Only then is the abandoned container + sidecar copied.

This proves unclean process exit recovery. It does not prove machine power-loss durability.

The oversized LCM payload test drops the creator handle before size admission.

3. TRACEDECAY_SQLITE_UNSAFE_FAST

Removed the unused Windows CI export. Crash children still .env_remove it. Writer policy is checked directly (WAL / synchronous=NORMAL / wal_autocheckpoint=0).

Test plan

• cargo test -p tracedecay-runtime-core --lib sqlite_read_snapshot:: — 32 passed
• clippy -D warnings on runtime-core lib/tests
• Native Windows shards on this head (create-only dest, live backup, crash-child reap)

Checklist

• CHANGELOG.md updated (under [Unreleased] if no version bump)
• No secrets, credentials, or .env files included
• Breaking changes documented (if any)

  

[changeset-bot]

⚠️ No Changeset found

Latest commit: 1379690

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions]

Performance Comparison codex/tracedecay-total-redesign-plan-reopened → cursor/crash-snapshot-fixtures-933-fdd6

Total Elapsed Time: 4.03s → 4.07s (+1.0%)
CPU Baseline: 76.01µs → 78.58µs (+3.4%)
Benchmark ID: index-bench-timing

timing - Execution duration of functions.

+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| Function                                 | Calls                      | Avg                            | P95                            | Total                          | % Total                      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| tracedecay-index-bench                   | 1 → 1 (+0.0%)              | 4.03s → 4.07s (+1.0%)          | 4.03s → 4.07s (+1.0%)          | 4.03s → 4.07s (+1.0%)          | 100.00% → 100.00% (+0.0%)    |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| query.artifact.append_pages              | 13 → 13 (+0.0%)            | 156.65ms → 156.47ms (-0.1%)    | 209.06ms → 209.98ms (+0.4%)    | 2.04s → 2.03s (-0.5%)          | 50.56% → 50.03% (-1.0%)      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| query.artifact.batch.sqlite              | 13 → 13 (+0.0%)            | 111.39ms → 111.36ms (-0.0%)    | 148.24ms → 148.50ms (+0.2%)    | 1.45s → 1.45s (+0.0%)          | 35.95% → 35.60% (-1.0%)      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| code_index.workers.install               | 83 → 83 (+0.0%)            | 11.69ms → 11.72ms (+0.3%)      | 40.17ms → 40.60ms (+1.1%)      | 970.52ms → 972.82ms (+0.2%)    | 24.09% → 23.92% (-0.7%)      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| query.artifact.batch.postings            | 13 → 13 (+0.0%)            | 73.11ms → 72.79ms (-0.4%)      | 99.68ms → 98.17ms (-1.5%)      | 950.48ms → 946.30ms (-0.4%)    | 23.60% → 23.27% (-1.4%)      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| domain.canonical.sha256                  | 114526 → 114526 (+0.0%)    | 6.97µs → 7.09µs (+1.7%)        | 11.76µs → 12.05µs (+2.5%)      | 798.32ms → 812.14ms (+1.7%)    | 19.82% → 19.97% (+0.8%)      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| code_index.build.and_publish             | 2 → 2 (+0.0%)              | 387.11ms → 387.65ms (+0.1%)    | 401.34ms → 399.25ms (-0.5%)    | 774.22ms → 775.30ms (+0.1%)    | 19.22% → 19.07% (-0.8%)      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| code_index.chunk.index_file              | 276 → 276 (+0.0%)          | 1.74ms → 1.84ms (+5.7%)        | 2.52ms → 2.55ms (+1.2%)        | 480.34ms → 507.65ms (+5.7%)    | 11.93% → 12.48% (+4.6%)      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| query.artifact.finalization.advance_wake | 14 → 14 (+0.0%)            | 33.30ms → 35.56ms (+6.8%)      | 208.93ms → 216.79ms (+3.8%)    | 466.26ms → 497.83ms (+6.8%)    | 11.58% → 12.24% (+5.7%)      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| query.artifact.prepare_pages             | 13 → 13 (+0.0%)            | 36.82ms → 36.79ms (-0.1%)      | 53.38ms → 54.66ms (+2.4%)      | 478.64ms → 478.31ms (-0.1%)    | 11.88% → 11.76% (-1.0%)      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| query.artifact.batch.commit              | 13 → 13 (+0.0%)            | 35.68ms → 36.12ms (+1.2%)      | 45.71ms → 47.58ms (+4.1%)      | 463.83ms → 469.59ms (+1.2%)    | 11.52% → 11.55% (+0.3%)      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| code_index.extract.parser_artifact       | 276 → 276 (+0.0%)          | 1.62ms → 1.60ms (-1.2%)        | 2.42ms → 2.42ms (+0.0%)        | 447.89ms → 441.04ms (-1.5%)    | 11.12% → 10.85% (-2.4%)      |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| query.artifact.batch.parallel_prepare    | 13 → 13 (+0.0%)            | 29.21ms → 29.19ms (-0.1%)      | 42.24ms → 43.45ms (+2.9%)      | 379.74ms → 379.42ms (-0.1%)    | 9.43% → 9.33% (-1.1%)        |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| code_index.chunk.build                   | 276 → 276 (+0.0%)          | 1.07ms → 1.08ms (+0.9%)        | 1.53ms → 1.55ms (+1.3%)        | 295.91ms → 296.79ms (+0.3%)    | 7.35% → 7.30% (-0.7%)        |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+
| code_index.chunk.identify                | 276 → 276 (+0.0%)          | 1.06ms → 1.06ms (+0.0%)        | 1.51ms → 1.54ms (+2.0%)        | 291.91ms → 292.90ms (+0.3%)    | 7.25% → 7.20% (-0.7%)        |
+------------------------------------------+----------------------------+--------------------------------+--------------------------------+--------------------------------+------------------------------+

---

Generated with hotpath-rs

[ScriptedAlchemy]

Architecture review — HOLD for production snapshot and fixture-lifecycle gates

Reviewed 8518e088005fcada58f71fffeb9f8ba893b20f68; source review only, exact-head CI pending. Child reaches durable phase -> exits without clean close -> parent joins -> copy abandoned image is the correct crash fixture. Closing the oversized payload's creator before testing size admission is also correct.

P1: sqlite_read_snapshot.rs changes production Copy-mode semantics, not just tests. Keep this change visibly separate in the PR's claims and prove a snapshot of a concurrently written WAL database is coherent, cancellation/deadline interrupts Busy/Locked and page-copy work, partial scratch is retired, and the caller never receives an incompletely backed-up database. Preserve the Foreign-source no-mutation contract for the source main/WAL/SHM family; SQLITE_OPEN_READ_ONLY alone is not the entire acceptance proof. Exercise absent/cleaned sidecars and Windows live-writer behavior. No immutable=1 on a changing source to bypass locking.

P1 test isolation: the new refuse_sqlite_unsafe_fast performs process-global unsafe environment mutation in ordinary library tests. Prefer .env_remove on the controlled crash child and remove the audited-unused CI export, rather than mutating shared libtest environment to prove an inert flag is absent. Verify the real runtime journal/sync policy directly. A comment calling an unread variable a speed knob is misleading.

Bound and reap crash child processes on every failure path; a child that never publishes the durable-phase marker must fail the fixture, not hang the suite or leave locks behind. Keep subprocess-crash proof distinct from a claim about machine power-loss durability. Native Windows run and the affected snapshot/backup suites on the #707 merge tree remain required.

[ScriptedAlchemy]

System-level re-review at f24ab494b — two concrete fixes required

The production snapshot change is not merely a fixture repair. I read the online-backup implementation and the new backup tests. Removing the unused shard-wide unsafe environment setting and separating the crash child from the live parent are improvements. This review does not claim to have run the Rust suites.

P1 — cleanup must not delete a pre-existing destination

sqlite_read_snapshot.rs::backup_live_sqlite_database_with unconditionally removes destination whenever run_online_backup errors. That includes the very first cancellation checkpoint or failure to open the source, before this attempt has created or published a destination. The public backup_live_sqlite_database path has no new-destination precondition or ownership receipt, so an unreadable/missing source can delete an existing valid backup. publish_complete_backup likewise unlinks the destination before renaming and deletes it again on rename failure, so failed replacement need not preserve the prior snapshot.

Delete only scratch material created by this attempt. Either explicitly enforce create-only destination semantics without touching an existing path, or retain the old destination until an authorized atomic replacement succeeds. Use attempt-owned staging (the fixed .backup-partial name also cannot distinguish two public helper calls), and reject source/destination aliases. Test an existing destination with source-open failure, cancellation before first step, failure after copying and publication failure; its bytes and identity must survive each failed attempt. A scratch-copy caller whose destination is always new can keep that narrower contract, but it must not make the public helper's cleanup destructive.

P2 — the new Busy/Locked test does not actually establish reader contention

sqlite_read_snapshot_backup_tests.rs::live_backup_deadline_interrupts_busy_locked_retries uses WAL mode plus BEGIN EXCLUSIVE on another connection, then expects read-side backup to time out. SQLite's transaction contract makes EXCLUSIVE equivalent to IMMEDIATE in WAL mode; this does not exclude snapshot readers. An isolated mechanism check here with SQLite 3.46.1 copied the fixture successfully in 0.143 ms while the writer transaction remained open (backup status DONE, both pages copied). This is not execution of the pinned rusqlite test, but it confirms the fixture premise is wrong, in agreement with SQLite's Transaction and Online Backup API documentation.

Use an actual rollback-journal exclusive-reader conflict, or a controlled backup-step boundary, and prove a Busy/Locked observation occurs before testing deadline/cancellation. Otherwise a fast runner fails expect_err, while a loaded runner can accidentally pass by using up 50 ms on unrelated work. Keep the WAL-concurrent-writer test separately; there success is the desired behavior.

Preserve the system contract

The copied image must be transactionally coherent and its later attach token must still validate the source generation; a successful backup alone does not authorize a freshness claim after the source changes. Keep checkpoint/deadline checks around the cooperative page loop and never use immutable=1 on the live source. Crash-child evidence proves process-crash recovery, not power-loss durability. The returned exact-head CI/hotpath-coverage are pending and SDK/profile runs queued.

[ScriptedAlchemy]

Follow-up on the fix pushed during review — a3e902b7a

The final inventory showed this newer head, so I re-read the backup implementation and the new tests before carrying the earlier findings forward.

Resolved in source: source-open and cooperative cancellation failures now retire the attempt's staging path rather than deleting the pre-existing destination. The new tests assert preserved destination bytes and file identity. The Busy/Locked fixture now uses DELETE journal mode, and explicitly observes a Busy/Locked step before testing the deadline. Withdraw the earlier WAL-EXCLUSIVE fixture objection. The WAL concurrent-writer test correctly remains a success path. I did not execute these repository tests.

Remaining Windows publication gap: replace_existing_destination under cfg(not(unix)) first renames the existing destination to displaced, then renames staging to destination. If the second move fails, restoration is attempted with let _ = fs::rename(&displaced, destination). A failed restoration is silently discarded; the function returns the staging-rename error without reporting that the old snapshot is no longer at its original path. A process exit between those two moves likewise leaves the old file displaced with no recovery record. Thus the implementation still does not establish the documented promise that the old destination stays in place until replacement succeeds. The current forced-publish-failure test exits before any displacement and cannot exercise this branch.

Use a supported atomic replacement authority, or reject replacing an existing destination where that contract cannot be provided. If non-atomic displacement is an intentional requirement, give it explicit recoverable ownership and report restoration failure rather than discarding it. Add native Windows failure injection after displacement and on restoration itself; prove recovery of the exact old file, not merely that some bytes remain at an unreported scratch name. This is distinct from the now-fixed early-error deletion.

Also make scratch ownership real at creation: PID/counter naming alone is not exclusive creation, and Connection::open_with_flags(...CREATE) can open a stale same-name file. Reserve the staging file exclusively before treating it as this attempt's deletable material, or refuse collisions. Do not delete a colliding file on an early checkpoint/source-open error.

The original findings have therefore been narrowed, not left stale: early failure cleanup and the lock-test premise are corrected; safe replacement and scratch ownership remain the publication boundary to finish.