fix: make upstream sync crash recoverable

[3metaJun]

Problem

scripts/sync-upstream.mjs --apply previously wrote and removed managed files directly. A filesystem error or process crash could leave a partially refreshed checkout with an old manifest.

Change

• add a durable per-target lock with stale-owner and takeover-guard recovery
• stage writes beside their targets and persist an immutable transaction journal
• commit target files with same-volume renames and install the manifest last
• roll back ordinary failures, retain diagnostics when rollback also fails, and recover interrupted transactions on the next --apply
• make dry runs and check-upstream hold the same lock and refuse pending recovery state
• validate journal paths, fingerprints, modes, sidecars, and target overlap
• document the crash-recoverable semantics and add fault-injected subprocess coverage

This provides mutual exclusion, rollback before returning an ordinary error, and recovery after hard exits. It does not claim instantaneous multi-file visibility for readers that ignore the lock.

Verification

• npm test on Windows: 68 passed, 1 existing Unix-only skip
• WSL Node 18 targeted sync tests: 20 passed
• npm run check-package
• git diff --check

---

Agent: GPT-6 via Codex

[3metaJun]

Adversarial review (multi-agent, post-merge)

Ran three independent adversarial reviews against cb771be — (1) concurrency/crash-recovery correctness, (2) security/input validation, (3) cross-platform + test quality — including live experiments: 8-way concurrent lock races on fresh/stale/stale+guard locks, ~40-payload path-validator fuzzing, NTFS junction escapes, a crafted-journal recovery tampering attempt, and crash simulations at each commit point. npm test re-run on Windows (Node 24): 68 pass + 1 pre-existing Unix-only skip — matches the PR claim exactly; targeted sync tests also pass 20/20 under WSL.

Bottom line: the transaction core holds up — mutual exclusion is airtight, readers never see a torn tree (the independent transactions.length refusal covers every window), rollback restores original content and is idempotent at every traced crash point, journal publication is atomic, manifest-last is enforced, and no path-escape/injection holes were found. The weaknesses are concentrated in staleness detection and off-model states that wedge the tool with circular advice — two of which contradict the README's recovery claim.

Findings

1. [P1] Staleness is PID-liveness only; PID reuse wedges lock takeover and transaction recovery.
processIsAlive (scripts/sync-upstream-transaction.mjs:455) treats any existing PID as alive, and EPERM as alive too (so other users'/SYSTEM processes count). A crashed sync's PID gets recycled on Windows (aggressively) → takeover is refused at :598, recovery refused at :677. Reproduced: planted a lock naming a live unrelated process → --apply, dry-run, and check-upstream all fail, including when an interrupted transaction is pending — so "if the process is interrupted, the next --apply recovers the unfinished transaction" (README) is false in exactly the crash scenario this PR targets. Corner case: if the new sync process itself receives the recycled PID, takeover is impossible by construction. No flag bypasses this (--force doesn't); only undocumented manual deletion of the lock/journal escapes.

2. [P1] Crash in the commit window + external target drift = permanent wedge with circular advice.
A crash between the last rename and COMMITTED leaves a journal that must roll back; if any target changed since (which the README itself admits readers may do), rollback fails. Then dry-run/check-upstream advise "rerun sync-upstream with --apply" (:601, :652) — which fails identically, forever. Reproduced. Same wedge on the committed path: crash after COMMITTED + user edits a freshly synced file → validateCommittedTargets (:449) fails on every subsequent apply. Both are untested and have no documented manual escape (rm -rf .mstack-sync-upstream/transactions/<id>).

3. [P2] Directory fsync is a silent no-op on Windows — durability is process-crash-grade only.
fsyncDirectory (:32) swallows EPERM on win32 (verified: openSync(dir,'r')+fsync throws EPERM here), so NTFS gives no ordering between the two renames in commitJournal. A power loss can persist stage→target while losing target→backup; rollback then hits "backup is missing" (:388) and retains state permanently. Process-kill crashes (the only kind the tests exercise) are unaffected. The README's unconditional recovery claim needs this caveat.

4. [P2] Lock-initializer scanner misclassifies takeover-guard temp files.
removeDeadLockInitializers (:507) accepts any <lock>.*.tmp then hard-throws on names not matching lock.<pid>.<uuid>.tmp — but stale-lock takeover creates <lock>.takeover.<pid>.<uuid>.tmp. Reproduced in an 8-way race: an innocent concurrent dry-run during a legitimate takeover dies with "Invalid upstream sync lock initializer". Relatedly, any stray file in the transaction dir bricks every command (including dry-run) until manual deletion.

5. [P2] The lock provides no mutual exclusion across the Windows↔WSL boundary the PR claims to support.
A WSL sync's lock names a Linux PID that usually has no Windows namesake → Windows wrongfully takes over (two concurrent writers); a dead WSL lock can be blocked by an unrelated Windows PID → spurious lockout. Cross-OS recovery is separately impossible: readJournal compares realpathSync(targetRoot) against the journal's OS-specific path (C:\… vs /mnt/c/…, :241) → always "targets a different checkout". Per-OS it works (verified on /mnt/c drvfs), but the platform is supported enough to hit this.

6. [P2] Read-only commands now serialize on the exclusive lock.
Pre-PR neither driver locked; now two concurrent dry-runs or check-upstream runs conflict (second exits 1). Parallel CI jobs checking one checkout will flake. Undocumented (README only mentions --apply exclusion) and untested — only apply-vs-apply is covered.

7. [P3] Hardlink-based lock creation fails raw on filesystems without hardlinks. FAT32/exFAT/many SMB shares → every command, including check-upstream, dies with a bare libuv error.

8. [P3] Recovery trusts the on-disk journal as authorization for deletes. Demonstrated end-to-end: a crafted journal (before: absent, forgeable after fingerprint) makes recovery delete named files and clean state. Theoretical in this threat model (the journal lives inside the target checkout — same trust boundary), but readJournal doesn't re-apply buildJournal's reserved-name rule (:712), so a tampered journal may target sync-state paths.

9. [P3] Minor: COMMITTING marker is written (:834) but never read by any recovery decision; pathsOverlap misses Win32 aliases (foo. vs foo — fails closed as a spurious failure, never silent corruption); valueAfter blindly consumes the next argv element; no transient retry for EPERM/EBUSY (AV/indexer) during commit; no test asserts file modes survive commit/rollback.

Suggested follow-ups

1. Make staleness identity-based, not PID-only: record hostname + boot-id + start time in the lock/journal, treat PID-alive-with-different-identity as stale, and add an explicit escape hatch (e.g. --recover-stale) or documented manual cleanup.
2. Make the :601/:652 errors actionable ("delete .mstack-sync-upstream/transactions/<id> to discard, or run …") so readers aren't stuck in a rerun loop.
3. Widen the initializer regex to include takeover temps (or ignore unrecognized .tmps instead of throwing).
4. Document win32 durability limits and the read-only-command serialization; consider a shared mode for dry-run/check-upstream.
5. Tests for the wedges above (COMMITTED+drift, live-owner refusal, takeover race, concurrent readers) and fsync-failure injection — the current preload can't inject fsync/chmod failures, so that failure family is unexercised.

None of these lose data — every observed failure mode fails stop with state retained — but findings 1–2 make the "next --apply recovers" promise unreliable on the exact platforms this was written for.