redo after a partially-skipped undo silently destroys the only backup of a deleted file

[codeAnqiang-ma]

Summary

undo redo can permanently destroy the only surviving copy of a deleted file, and it does so silently — reporting re-applied N change(s) as if nothing went wrong. It triggers when a session was only partially undone, which the README documents as normal behaviour.

This breaks the invariant stated both in the README and in the code being violated:

Undo never deletes anything permanently, only parks it in the session store, so a session toggles between undone and applied as many times as you like. — README.md

Both directions preserve data (files are swapped with or parked in the session store, never deleted), so undo and redo can toggle a session indefinitely. — internal/restore/restore.go package comment

How the bad state is reached (every step is documented behaviour)

1. A session deletes two or more paths, so the journal holds unlink <path> <backup> entries. Those backups are the only copies of the deleted files.
2. The user recreates one of the deleted paths afterwards — e.g. echo ... > a, since shell redirection isn't captured by the shim, as the README itself notes.
3. undo apply skips that entry, exactly as documented: path exists again, use --force to overwrite. The other entry restores fine, so res.Done > 0 and the session is still marked undone (restore.go:386-392).
4. undo redo only checks s.Undone (cmd/undo/main.go:243-244). It has no way to know that one entry was never undone and its backup slot is still occupied.

Root cause

The OpUnlink redo branch moves the current file into the recorded backup path unconditionally (internal/restore/restore.go:189-197):

} else {                       // dir == Redo
    if !exists(field(0)) { done = false; break }
    if !act() { continue }
    err = moveAny(field(0), field(1))   // field(1) may still hold the only copy
}

moveAny (restore.go:47-76) starts with os.Rename, which atomically replaces an existing destination; its cross-filesystem fallback opens the destination with O_TRUNC, which also overwrites.

The undo direction guards every destination it writes (exists(field(0)) && !opts.Force → skip), and OpCreate's undo even checks !exists(slot(s, i)) before restoring. Only the OpUnlink / OpRename redo paths assume their backup slot must be empty. That asymmetry is the bug.

OpRename redo has the same issue: moveAny(new_, bak) at restore.go:262-265 will overwrite an occupied bak, which is the only backup of a file that mv clobbered.

Reproduction

Verified against 513536e with the real binary. The shim is Linux-only, so the session directory below is written by hand — but in the shim's exact on-disk format (tab-separated, per internal/journal/journal.go:17 and internal/journal/journal_test.go), equivalent to what rm keep.txt gone.txt leaves behind. The restore / session / journal packages are pure Go and platform-independent, so this script should run unchanged on Linux.

go build -o /tmp/undo-bin ./cmd/undo
cd /tmp && rm -rf vstore vwork
export UNDO_DATA_DIR=/tmp/vstore
SID=1700000000009999; SESS=vstore/sessions/$SID
mkdir -p $SESS/data vwork

# a session equivalent to `rm keep.txt gone.txt`
printf 'rm keep.txt gone.txt\n' > $SESS/cmd
printf '424242\n' > $SESS/pid
: > $SESS/done
printf 'ORIGINAL-KEEP\n' > $SESS/data/bak_keep
printf 'ORIGINAL-GONE\n' > $SESS/data/bak_gone
printf 'unlink\t%s\t%s\n' "$PWD/vwork/keep.txt" "$PWD/$SESS/data/bak_keep"  > $SESS/journal
printf 'unlink\t%s\t%s\n' "$PWD/vwork/gone.txt" "$PWD/$SESS/data/bak_gone" >> $SESS/journal

# the user recreates one of the deleted paths
printf 'RECREATED-KEEP\n' > vwork/keep.txt

/tmp/undo-bin apply $SID -y ; cat $SESS/data/bak_keep   # ORIGINAL-KEEP  <- still safe
/tmp/undo-bin redo  $SID -y ; cat $SESS/data/bak_keep   # RECREATED-KEEP <- destroyed
/tmp/undo-bin apply $SID -y ; cat vwork/keep.txt        # RECREATED-KEEP <- original gone

Observed output:

=== apply
skipped: deleted   /tmp/vwork/keep.txt: path exists again, use --force to overwrite
restored 1 change(s)
backup of keep.txt: ORIGINAL-KEEP

=== redo
re-applied 2 change(s)              <- no warning at all
backup of keep.txt: RECREATED-KEEP

=== apply
restored 2 change(s)
work/keep.txt: RECREATED-KEEP
grep -r ORIGINAL-KEEP vstore vwork  -> no matches; the original is gone from disk

Expected: redo never clobbers an occupied backup slot, so ORIGINAL-KEEP stays recoverable and the session can toggle indefinitely.

Actual: the only copy is overwritten, the CLI reports success, and the next undo hands back the wrong content. Nothing warns the user, which is what makes this worse than a crash — the tool exists to be a safety net, and here it quietly removes the net.

Suggested fix

Mirror the skip guards the undo direction already has. If the backup path still exists, that entry was never undone, so redo has nothing to re-apply:

case journal.OpUnlink:  // redo branch
    if exists(field(1)) {
        skip("backup still holds the deleted file (entry was never undone)")
        continue
    }

The same guard belongs before moveAny(new_, bak) in the OpRename redo branch. If you would rather have redo genuinely re-delete the file, park the current file in a fresh slot(s, i) instead of the fixed backup path — either approach preserves the data.

For what it's worth, the existing tests do cover "undo skips a recreated path" (TestUndoUnlinkConflictSkipsWithoutForce) and lossless mod swaps, but nothing exercises partial-undo followed by redo.

Environment

macOS 15 / arm64, Go 1.26.5, commit 513536e. Not run on Linux — the script above should be platform-independent, but I have not verified it there.

---

Found with AI assistance. I reproduced every step locally against the real binary, checked each cited line, and reviewed all conclusions before filing.