Skip to content

Extract shared restore-or-orphan swap policy (#29) - #73

Merged
jon-devlapaz merged 1 commit into
mainfrom
cursor/issue-29-shared-restore-or-orphan-b0ab
Sep 9, 2026
Merged

jon-devlapaz merged 1 commit into
mainfrom
cursor/issue-29-shared-restore-or-orphan-b0ab

Conversation

@jon-devlapaz

Copy link
Copy Markdown
Owner

Summary

Closes #29.

Three replace paths (publish_staged_tree, manifest::write_atomic, update::replace_binary) previously duplicated the same restore-or-orphan failure policy. This PR extracts that policy into paths::restore_or_orphan and paths::orphan_or_retain_after_restore_failure, reusing existing orphan_recovery_path / move_file_to_orphan.

What was unified vs what stayed separate

Unified (policy):

  • Try restore from backup
  • On rollback failure → rename backup to .tink-orphan-* beside destination root
  • On orphan rename failure → caller-supplied keep() fallback; error names recovery path

Separate (mechanics at the edges):

  • Tree (rollback_or_retain_backup / publish_staged_tree): fs::rename restore; staging TempDir::keep() fallback
  • Manifest (write_atomic): manifest backup restore; NamedTempFile::keep() fallback; no-backup path still uses remove_file
  • Binary (replace_binary): consuming persist() restore; orphan_or_retain_after_restore_failure after restore already failed

Staging, publish, and backup capture remain with each owner — no leaky file/tree abstraction.

Proof

cargo test restore_or_orphan -- --nocapture
cargo test rollback_failure_retains_recovery_backup_at_durable_orphan_path -- --nocapture
cargo test publish_staged_tree_restores_target_when_publish_fails -- --nocapture
cargo test write_atomic_restores_manifest_when_lock_publish_fails -- --nocapture
cargo test write_atomic_retains_orphan_on_double_failure -- --nocapture
cargo test replace_binary_retains_recovery_backup_when_rollback_fails -- --nocapture
cargo test replace_binary_rolls_back_when_published_probe_fails -- --nocapture
cargo test deposit_divergent_repair_retains_orphan_on_double_failure -- --nocapture
cargo test --workspace --all-targets --locked
cargo fmt --all -- --check
cargo clippy --workspace --all-targets --locked -- -D warnings

All passed locally (200 tests).

Docs

  • docs/RELIABILITY.md — notes shared helpers
  • docs/issue-29-verification-spike.md — matrix updated; shared helper → Done; removed stale "next PR = publish_staged_tree only" guidance

Close decision

Closes #29. Safety invariant, durable orphan naming (#72), and shared policy consolidation are complete. First-install-only staging (create paths with no prior live tree) remains intentionally out of scope — different shape, not #26-class.

Open in Web Open in Cursor 

Consolidate the tree, manifest, and binary double-failure paths behind
paths::restore_or_orphan and orphan_or_retain_after_restore_failure.
Call sites keep staging/publish mechanics and format errors at the edges.
Observable behavior and characterization tests are unchanged.

Co-authored-by: jon <jonathan10620.dev@gmail.com>
@jon-devlapaz
jon-devlapaz marked this pull request as ready for review September 9, 2026 18:14
@jon-devlapaz
jon-devlapaz merged commit b9e71ef into main Sep 9, 2026
6 checks passed
@jon-devlapaz
jon-devlapaz deleted the cursor/issue-29-shared-restore-or-orphan-b0ab branch September 9, 2026 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: durable atomic skill-tree swap (shared helper)

2 participants