1 files changed, 70 insertions, 28 deletions
diff --git a/internal/engine/apply.go b/internal/engine/apply.go
index 83ea4fe..94d65e4 100644
--- a/internal/engine/apply.go
+++ b/internal/engine/apply.go
@@ -343,21 +343,26 @@ func (e *Engine) PlanUndo(runID string) (*UndoPlan, error) {
 		return nil, fmt.Errorf("engine: plan undo: run %s is itself an undo and cannot be undone", runID)
 	}
 
-	var order []string
-	byFile := map[string][]journal.Entry{}
+	// Entries are grouped by directory and file together (review M7): one run
+	// spans every directory, and two directories can each hold a file of the
+	// same name.
+	type fileKey struct{ dir, file string }
+	var order []fileKey
+	byFile := map[fileKey][]journal.Entry{}
 	for _, en := range entries {
 		if en.File == "" { // run-start / run-end
 			continue
 		}
-		if _, seen := byFile[en.File]; !seen {
-			order = append(order, en.File)
+		k := fileKey{en.Dir, en.File}
+		if _, seen := byFile[k]; !seen {
+			order = append(order, k)
 		}
-		byFile[en.File] = append(byFile[en.File], en)
+		byFile[k] = append(byFile[k], en)
 	}
 
 	up := &UndoPlan{Run: runID}
-	for _, file := range order {
-		uf := planUndoFile(file, byFile[file])
+	for _, k := range order {
+		uf := planUndoFile(k.dir, k.file, byFile[k])
 		// Critical finding, Task 8's review: a file every one of whose
 		// entries has Status != "ok" (declined by the ORIGINAL run's own
 		// review, or skipped, or failed before anything happened) yields
@@ -459,8 +464,8 @@ func isFileAffecting(action string) bool {
 // two steps of one file that touch the same path, and every un-contended
 // check keeps behaving exactly as before, since the projection only ever
 // overrides a real occupant that this same chain is itself about to clear.
-func planUndoFile(file string, ents []journal.Entry) UndoFile {
-	uf := UndoFile{File: file, Dir: dirOf(ents)}
+func planUndoFile(dir, file string, ents []journal.Entry) UndoFile {
+	uf := UndoFile{File: file, Dir: dir}
 	proj := newUndoProjection()
 	for i := len(ents) - 1; i >= 0; i-- {
 		en := ents[i]
@@ -560,17 +565,6 @@ func needsOccupancyCheck(action string) bool {
 	return false
 }
 
-// dirOf returns the first non-empty Dir among ents, which should all agree
-// since one file is always processed within one configured directory.
-func dirOf(ents []journal.Entry) string {
-	for _, en := range ents {
-		if en.Dir != "" {
-			return en.Dir
-		}
-	}
-	return ""
-}
-
 // reverseStep computes the UndoStep for one logged "ok" entry, per spec
 // §10's reversal table. It only stats the filesystem to decide Refused (the
 // "changed since" and "trash entry is gone" checks); it never mutates
@@ -597,9 +591,7 @@ func reverseStep(en journal.Entry) UndoStep {
 		us.Refused = refuseIfChanged(en)
 	case "trash", "displace":
 		us.Src, us.Dst = en.Dst, en.Src
-		if _, err := os.Stat(en.Dst); err != nil {
-			us.Refused = "the trash entry is gone"
-		}
+		us.Refused = refuseIfTrashChanged(en)
 	case "mkdir":
 		us.Src = en.Dst
 	}
@@ -635,6 +627,38 @@ func refuseIfChanged(en journal.Entry) string {
 	return ""
 }
 
+// refuseIfTrashChanged is the trash reversal's identity check (review M2):
+// the entry must still be the file this run put there - the size and mtime
+// the run logged for it - and its trashinfo must still record the path it
+// came from. Emptying the Trash and trashing another file of the same name
+// would otherwise have undo restore that file instead.
+func refuseIfTrashChanged(en journal.Entry) string {
+	fi, err := os.Lstat(en.Dst)
+	if err != nil {
+		return "the trash entry is gone"
+	}
+	if fi.Size() != en.Size || !fi.ModTime().Equal(en.ModTime) {
+		return fmt.Sprintf("the trash entry %s is not the file this run put there", en.Detail)
+	}
+	if p, err := trash.InfoPath(en.Detail); err != nil || p != en.Src {
+		return fmt.Sprintf("the trash entry %s now belongs to another file", en.Detail)
+	}
+	return ""
+}
+
+// recheck repeats, at execution time, the identity check PlanUndo made
+// (review undo F8): an undo plan is shown and approved first, and a file
+// changed in that window must not be moved back or trashed.
+func recheck(step UndoStep) string {
+	switch step.Action {
+	case "undo-move", "undo-rename", "undo-copy":
+		return refuseIfChanged(step.Original)
+	case "undo-trash", "undo-displace":
+		return refuseIfTrashChanged(step.Original)
+	}
+	return ""
+}
+
 // refuseIfSrcExists is the second half of every "reversal puts a file back
 // at a fixed path" refusal condition: reversing would silently clobber
 // whatever is there now. Fix wave item 1: it is no longer reverseStep's own
@@ -773,7 +797,10 @@ func (e *Engine) ApplyUndo(ctx context.Context, up *UndoPlan, j *journal.Writer,
 		tallyFile(result, fr.Steps, func(i int) bool { return isFileAffecting(f.Steps[i].Action) })
 		for i, us := range f.Steps {
 			if us.Action == "undo-mkdir" && fr.Steps[i].Status == "failed" {
-				retries = append(retries, dirRetry{dir: us.Src, dirName: f.Dir, file: f.File, step: i + 1})
+				retries = append(retries, dirRetry{dir: us.Src, dirName: f.Dir, file: f.File, step: i + 1, log: true})
+			}
+			for _, made := range fr.Steps[i].Made {
+				retries = append(retries, dirRetry{dir: made})
 			}
 		}
 	}
@@ -802,6 +829,10 @@ type dirRetry struct {
 	dirName string
 	file    string
 	step    int
+	// log is false for a directory this undo run itself created on the way
+	// (runUndoStep's Made): the original run's log already says it was
+	// removed, so removing it again needs no entry of its own.
+	log bool
 }
 
 // retryDirRemovals is ApplyUndo's run-wide second pass (Task 1, plan 5): once
@@ -833,6 +864,9 @@ func (e *Engine) retryDirRemovals(j *journal.Writer, run string, candidates []di
 			// original refusal already recorded this, and it stands.
 			continue
 		}
+		if !c.log {
+			continue
+		}
 		if err := j.Append(journal.Entry{
 			Time: e.Now(), Run: run, Dir: c.dirName, File: c.file, Step: c.step,
 			Action: "undo-mkdir", Status: "ok", Src: c.dir,
@@ -924,16 +958,24 @@ func (e *Engine) undoFile(f UndoFile, j *journal.Writer, run string) (FileResult
 // Step field does not apply here (there is no plan.Kind for an undo) and is
 // left zero.
 func runUndoStep(step UndoStep) apply.StepResult {
+	if why := recheck(step); why != "" {
+		return apply.StepResult{Status: "failed", Detail: why}
+	}
 	switch step.Action {
 	case "undo-move", "undo-rename":
-		if err := os.MkdirAll(filepath.Dir(step.Dst), 0o755); err != nil {
-			return apply.StepResult{Status: "failed", Detail: err.Error()}
+		// The directories created here are returned in Made: ApplyUndo removes
+		// them again once empty, since an earlier file's undo-mkdir may already
+		// have removed the directory this file passes back through (review
+		// undo F6).
+		made, err := apply.MkdirAllTracked(filepath.Dir(step.Dst))
+		if err != nil {
+			return apply.StepResult{Status: "failed", Detail: err.Error(), Made: made}
 		}
 		if err := renameOrCopy(step.Src, step.Dst); err != nil {
-			return apply.StepResult{Status: "failed", Detail: err.Error()}
+			return apply.StepResult{Status: "failed", Detail: err.Error(), Made: made}
 		}
 		size, mtime := statSizeModTime(step.Dst)
-		return apply.StepResult{Status: "ok", Dst: step.Dst, Size: size, ModTime: mtime}
+		return apply.StepResult{Status: "ok", Dst: step.Dst, Size: size, ModTime: mtime, Made: made}
 
 	case "undo-copy":
 		entry, err := trash.Put(step.Src)