plan 10: resumed undo re-checks what an earlier undo put back; occupied directory removals are not offered forever

1 files changed, 25 insertions, 2 deletions

diff --git a/internal/engine/apply.go b/internal/engine/apply.go
index ed2fbe5..75248fb 100644
--- a/internal/engine/apply.go
+++ b/internal/engine/apply.go
@@ -409,11 +409,32 @@ func (e *Engine) PlanUndo(runID string) (*UndoPlan, error) {
 		if len(uf.Steps) == 0 && uf.Refused == "" {
 			continue
 		}
+		if uf.Refused == "" && onlyOccupiedDirectoryRemovals(uf.Steps) {
+			// Nothing of the file itself is left to reverse, only directories
+			// the run made that something else still occupies: offering them
+			// would repeat on every undo (re-review undo F3). An empty one is
+			// still offered, and removed.
+			continue
+		}
 		up.Files = append(up.Files, uf)
 	}
 	return up, nil
 }
 
+// onlyOccupiedDirectoryRemovals reports whether every step is an undo-mkdir
+// of a directory that is not empty now, so none of them could run.
+func onlyOccupiedDirectoryRemovals(steps []UndoStep) bool {
+	for _, s := range steps {
+		if s.Action != "undo-mkdir" {
+			return false
+		}
+		if entries, err := os.ReadDir(s.Src); err == nil && len(entries) == 0 {
+			return false
+		}
+	}
+	return true
+}
+
 // isUndoRun reports whether every one of entries' file-scoped actions is
 // already an "undo-" action, i.e. entries belongs to a run ApplyUndo itself
 // produced. Runs cannot themselves be undone (spec §10). This does not read
@@ -523,9 +544,11 @@ func planUndoFile(dir, file string, ents []journal.Entry, reversed map[journal.R
 		step := reverseStep(en)
 		if k := (journal.ReversedKey{Dir: dir, File: file, Action: step.Action, Src: step.Src}); reversed[k] > 0 {
 			// An earlier undo of this run already reversed this step: the
-			// disk already shows it, and it is not offered again.
+			// disk already shows it, and it is not offered again. It is not
+			// recorded in the projection either (re-review undo F1): what it
+			// put back is on disk now and is checked there, so a file changed
+			// since is refused rather than vouched for by the old reversal.
 			reversed[k]--
-			proj.record(step)
 			continue
 		}
 		if (en.Action == "move" || en.Action == "rename") && proj.occupied[en.Dst] {