6.6 KiB
Journal publication and recovery review — 2026-09-06
Independent read-only re-review of the current crates/oa-core/src/source_files.rs. Both previously reported findings are addressed in the reviewed Windows implementation. No additional material defect was found within this bounded change. This review did not run Cargo, operate the native application, or modify production code. The integrated workspace run was still in progress; its result is not claimed here.
Findings before and after
| Finding and reproducible failure | Revised behavior | Evidence |
|---|---|---|
A destination check followed by fs::rename(stage, journal) was not a no-replacement operation. Another process could create an empty destination directory after the check; modern Windows Rust could replace it. A nonempty, complete existing journal was a different case and was not shown to be replaced. |
rename_journal_without_replacement uses MoveFileExW with flags zero. The OS operation itself refuses replacement, independently of the advisory precheck. The stage remains owned by its temporary-directory handle until publication succeeds. |
.local/save-empty-journal-probe.log reproduces the old behavior: existing empty destination, rename Ok(()), stage gone. The new Windows test journal_publication_does_not_replace_empty_target_created_after_precheck creates the competing directory inside the injected rename operation, then checks that the staged bytes and empty competing directory both remain. |
| Writer A stages before bytes X and after bytes Y, then waits to publish. Writer B completes the same Y and retires its journal. A publishes, detects that disk is no longer X, and previously entered rollback despite never writing. Since B's Y equals A's recorded after bytes, rollback could erase B's successful save. | New saves publish journal version 2 without a write marker. The first post-publication conflict retires only the journal. Before the first model write, the exact write-started marker is created and flushed. Explicit recovery of a valid v2 journal without this marker also retires only metadata, including when the initial retirement failed or the process stopped after publication. |
unwritten_journal_never_rolls_back_a_completed_writer_with_identical_after_bytes runs the competing completed save before publication and covers both successful and interrupted retirement. interrupted_publication_without_write_marker_never_rolls_back_external_matching_bytes independently exercises reopening without a marker. Both verify exact persisted candidate bytes remain. |
The first finding agrees with the current Rust 1.98.1 rename documentation: supported modern Windows filesystems can use Unix-like empty-directory replacement semantics. The replacement primitive now follows the explicit flags contract of Microsoft's MoveFileExW documentation. Paths used by production publication descend from the canonical project root; the reviewed raw UTF-16 conversion therefore retains the canonical Windows path prefix. It rejects interior NUL and keeps both terminated buffers alive through the FFI call.
Ownership and interruption checks
The Windows retry loop performs at most seven publication attempts, with six delays totaling 390 ms. Only OS errors 5, 32 and 33 are retried. It checks for any destination object before every attempt, preserves the final OS error and journal path, and does not repeat model writes. Exhaustion leaves publication unsuccessful; it does not claim ownership of an existing journal.
Recovery validates the journal version, entry, bounded before/after files, logical paths, marker bytes and directory contents before retiring an unwritten journal. A present marker must match the exact expected bytes; a link, nonregular file, oversized marker or malformed content cannot silently become permission to roll back. Version 1 journals keep their original rollback rules. With a valid v2 marker, the existing per-file before/after checks and interrupted rollback behavior remain in force. Atomic retirement precedes best-effort removal of fixed-name metadata files; cleanup does not recursively delete an untrusted directory.
Verification inspected
The core owner's .local/source6-core-recovery-tests.log records 39 core library tests and 8 external source-file tests passing. The passing library cases include both publication races, bounded transient retry/exhaustion, missing-marker retirement, malformed-marker rejection, v1 compatibility, partial-save rollback and resumable interrupted recovery. These are inspected results from the core owner's run, not a second run by this reviewer.
The core owner's .local/source-save-probe.log records 18 successful saves followed by two publication failures with Windows error 5. .local/source-save-fixed-30-probe.log records 30 successful saves and no remaining probe files. The inspected probe calls the production new_project, compile_bundle and save_project, compares saved bytes and bundle, and then removes its own generated entry. This is useful local reliability evidence, not a guarantee against arbitrary scanner lock durations or a multi-process stress test.
Limits retained
- The module promises recoverability, not atomic visibility of a whole multi-file bundle or cross-process compare-and-swap. An unrelated process can still modify paths between checks and replacement; concurrent recovery of an actively writing journal is not a supported locking protocol. The marker means writes may have begun, not proof that this writer produced every matching after byte.
- Windows directory synchronization remains a no-op, as documented in the module. No power-loss durability claim follows from the flushed marker.
- An interruption during marker creation can leave partial marker bytes. Recovery deliberately rejects that malformed journal without changing model bytes; the test verifies this refusal. The fix does not provide automatic repair of corrupted metadata.
- New code reads v1 and v2 journals. An older executable that only accepts v1 will reject a pending v2 journal; this change does not promise downgrade recovery.
- The strict no-replacement primitive is Windows-specific. The unchanged non-Windows publication branch still uses
fs::rename; this review does not claim that a racing empty destination is protected there. - The 390 ms delay budget bounds sleeps, not filesystem-call latency. Longer-lived scanner locks or permanent ACL errors remain reportable save failures. Native save/reopen validation of the final integrated executable and the full workspace result remain separate release evidence.