domains / app-frameworks / can1357/oh-my-pi
Safe rollback ordering
When a function performs a multi-step mutation of shared, possibly concurrently-accessed state (e.g., registering or rolling back a worktree on disk), handle failures so the system is never left in an ambiguous or unsafe state, and never mistake "error" for "expected absence."
When a function performs a multi-step mutation of shared, possibly concurrently-accessed state (e.g., registering or rolling back a worktree on disk), handle failures so the system is never left in an ambiguous or unsafe state, and never mistake “error” for “expected absence.”
-
Classify errors precisely before acting on them. Don’t swallow every failure into the same “not present” case just because a missing file is one legitimate cause. For example, reading an optional setup-marker file should treat “file not found” as “no mark,” but any other I/O error (e.g., the marker is unreadable or is actually a directory) must propagate as a real failure instead of being silently treated as “setup finished,” which could let another process unsafely reuse the resource.
-
When rolling back a partially-completed operation, order the cleanup steps so that at every intermediate point other processes see either a fully valid state or an unambiguously invalid/locked one — never something that looks available or complete. For instance, when undoing a worktree registration, remove the back-reference (
gitdir) before anything else so no process can list or take over the entry; only remove the checkout/registration once you’re sure nothing else can race with it. -
If a rollback step itself fails, stop there and leave the resource in its last known-safe state (e.g., still locked and marked with its live owner) rather than continuing and producing a half-cleaned, ambiguous result. Surface the original error so the caller/operator can recover (e.g., via manual
git worktree remove), instead of silently discarding it. -
Validate preconditions robustly against the actual resolved state, not just the literal input — e.g., canonicalize paths (including symlinked ancestors) before comparing a destination against a source, so error conditions like “destination is inside source” are reliably detected rather than missed due to superficial path differences.
Example pattern for staged writes with rollback:
let written: Result<()> = (|| {
fs::write(admin.join("locked"), "initializing\n")?;
if let Some(owner) = setup_owner {
fs::write(admin.join(SETUP_MARK), format!("{owner}\n"))?;
}
fs::write(path.join(".git"), gitdir_line)?;
fs::write(admin.join("gitdir"), back_reference)?;
Ok(())
})();
if let Err(err) = written {
// Remove the back-reference first so nothing else can see/take over
// a half-written, locked entry; only then clean up the rest.
let _ = fs::remove_dir_all(&admin);
return Err(err);
}
Applying this discipline prevents silent data corruption, race conditions between concurrent processes, and unrecoverable partial states after a failure.</description>