1 files changed, 45 insertions, 19 deletions

diff --git a/internal/apply/apply.go b/internal/apply/apply.go
index 713913e..5624b27 100644
--- a/internal/apply/apply.go
+++ b/internal/apply/apply.go
@@ -42,38 +42,59 @@ type StepResult struct {
 
 // Chain runs one file's steps in order and stops at the first failure,
 // marking the rest skipped. It never touches a file whose size or mtime no
-// longer matches what the plan recorded.
+// longer matches what the plan recorded. It is ChainLogged with no done.
 func Chain(c plan.Chain) []StepResult {
+	results, _ := ChainLogged(c, nil)
+	return results
+}
+
+// ChainLogged is Chain, calling done with each step's result as soon as
+// that step has run or been skipped, before the next one starts - so a
+// caller that logs from done never has a completed step missing from the
+// log when the process dies mid-chain (review M9). An error from done stops
+// the chain at once and is returned with the results so far.
+//
+// A move or rename that had to take a free name at apply time, because its
+// planned destination was taken since planning, stops the chain too: every
+// later step was planned against the name the file did not get, and must
+// not act on whatever is at that path (review M3).
+func ChainLogged(c plan.Chain, done func(i int, sr StepResult) error) ([]StepResult, error) {
 	results := make([]StepResult, len(c.Steps))
-	stopped := false
+	stopWhy := ""
 
 	for i, step := range c.Steps {
-		if stopped {
-			results[i] = StepResult{Step: step, Status: "skipped", Detail: "an earlier step in this chain failed"}
-			continue
-		}
-		if step.Skip != "" {
+		switch {
+		case stopWhy != "":
+			results[i] = StepResult{Step: step, Status: "skipped", Detail: stopWhy}
+		case step.Skip != "":
 			// Planning already decided this step will not run; it must not
 			// be attempted, so no pre-step check, no directory creation, no
 			// touching the file (spec: a step already marked Skip is
 			// reported, not attempted).
 			results[i] = StepResult{Step: step, Status: "skipped", Detail: step.Skip}
-			continue
+		default:
+			if err := checkUnchanged(step.Src, c.File); err != nil {
+				results[i] = StepResult{Step: step, Status: "failed", Detail: err.Error()}
+				stopWhy = "an earlier step in this chain failed"
+				break
+			}
+			res := runStep(step)
+			results[i] = res
+			switch {
+			case res.Status == "failed":
+				stopWhy = "an earlier step in this chain failed"
+			case (step.Kind == plan.Move || step.Kind == plan.Rename) && res.Dst != step.Dst:
+				stopWhy = fmt.Sprintf("an earlier step put the file at %s, not the planned %s", res.Dst, step.Dst)
+			}
 		}
-		if err := checkUnchanged(step.Src, c.File); err != nil {
-			results[i] = StepResult{Step: step, Status: "failed", Detail: err.Error()}
-			stopped = true
-			continue
-		}
-
-		res := runStep(step)
-		results[i] = res
-		if res.Status == "failed" {
-			stopped = true
+		if done != nil {
+			if err := done(i, results[i]); err != nil {
+				return results[:i+1], err
+			}
 		}
 	}
 
-	return results
+	return results, nil
 }
 
 // checkUnchanged is the guard that matters most: before every step, the
@@ -131,6 +152,11 @@ func runFileStep(step plan.Step) StepResult {
 	var displacedEntry string
 
 	if step.Displaces != "" {
+		// Re-checked at apply time (review M4): only a regular file may be
+		// trashed to make room, never a directory or link put there since.
+		if fi, err := os.Lstat(step.Displaces); err != nil || !fi.Mode().IsRegular() {
+			return StepResult{Step: step, Status: "failed", Detail: "the file to replace is gone or no longer a regular file"}
+		}
 		// overwrite policy: the file already at dst must be trashed before
 		// this step's own destination name is used, so no free-name search
 		// applies here — the whole point of displacing was to clear this