3 files changed, 140 insertions, 26 deletions

diff --git a/internal/apply/apply.go b/internal/apply/apply.go
index c0055a2..b36035b 100644
--- a/internal/apply/apply.go
+++ b/internal/apply/apply.go
@@ -45,7 +45,7 @@ type StepResult struct {
 // marking the rest skipped. It never touches a file whose size or mtime no
 // longer matches what the plan recorded. It is ChainLogged with no done.
 func Chain(c plan.Chain) []StepResult {
-	results, _ := ChainLogged(context.Background(), c, nil)
+	results, _ := ChainLogged(context.Background(), c, nil, nil)
 	return results
 }
 
@@ -63,7 +63,7 @@ func Chain(c plan.Chain) []StepResult {
 // Once ctx is cancelled (an interrupt), the step already under way finishes
 // and every later step is skipped as "interrupted": an interrupt stops after
 // the current step, not after the file's whole chain (spec §11).
-func ChainLogged(ctx context.Context, c plan.Chain, done func(i int, sr StepResult) error) ([]StepResult, error) {
+func ChainLogged(ctx context.Context, c plan.Chain, done func(i int, sr StepResult) error, displaced func(i int, step plan.Step, entry string) error) ([]StepResult, error) {
 	results := make([]StepResult, len(c.Steps))
 	stopWhy := ""
 
@@ -86,7 +86,12 @@ func ChainLogged(ctx context.Context, c plan.Chain, done func(i int, sr StepResu
 				stopWhy = "an earlier step in this chain failed"
 				break
 			}
-			res := runStep(step)
+			res := runStep(step, func(entry string) error {
+				if displaced == nil {
+					return nil
+				}
+				return displaced(i, step, entry)
+			})
 			results[i] = res
 			switch {
 			case res.Status == "failed":
@@ -133,11 +138,13 @@ func checkUnchanged(src string, f scan.File) error {
 }
 
 // runStep dispatches one already-checked, non-skipped step to the code that
-// actually carries it out.
-func runStep(step plan.Step) StepResult {
+// actually carries it out. reportDisplace is called the moment a displaced
+// file has reached the Trash, before the step that needed its name begins;
+// a step whose displace cannot be reported does not go on to use the name.
+func runStep(step plan.Step, reportDisplace func(entry string) error) StepResult {
 	switch step.Kind {
 	case plan.Copy, plan.Move, plan.Rename:
-		return runFileStep(step)
+		return runFileStep(step, reportDisplace)
 	case plan.Trash:
 		return runTrashStep(step)
 	case plan.DeletePermanent:
@@ -155,7 +162,13 @@ func runStep(step plan.Step) StepResult {
 // overwriting a file the plan never accounted for. Missing destination
 // directories are created and recorded in Made, outermost first, whether or
 // not the step that needed them goes on to succeed.
-func runFileStep(step plan.Step) StepResult {
+//
+// The displace is reported through reportDisplace as soon as trash.Put
+// returns, not when this step finishes: the user's file is in the Trash
+// from that moment, durably, and for a copy or a cross-device move the rest
+// of the step is the whole data transfer. A process killed in that window
+// used to leave the file in the Trash with nothing in the log to say so.
+func runFileStep(step plan.Step, reportDisplace func(entry string) error) StepResult {
 	dst := step.Dst
 	var displacedEntry string
 
@@ -178,6 +191,15 @@ func runFileStep(step plan.Step) StepResult {
 			return StepResult{Step: step, Status: "failed", Detail: "displacing the existing file: " + err.Error()}
 		}
 		displacedEntry = entry
+		if reportDisplace != nil {
+			if err := reportDisplace(entry); err != nil {
+				// The file is already in the Trash and cannot be recorded.
+				// Using the name now would compound an unlogged destructive
+				// act with a second one.
+				return StepResult{Step: step, Status: "failed", DisplacedEntry: entry,
+					Detail: "the file it replaces went to the Trash but could not be logged, so undo cannot see it: " + err.Error()}
+			}
+		}
 	} else if _, err := os.Lstat(dst); err == nil {
 		free, err := nextFreeName(dst)
 		if err != nil {
diff --git a/internal/apply/swap_test.go b/internal/apply/swap_test.go
index d16d3f9..297db72 100644
--- a/internal/apply/swap_test.go
+++ b/internal/apply/swap_test.go
@@ -4,6 +4,7 @@ package apply
 
 import (
 	"context"
+	"errors"
 	"os"
 	"path/filepath"
 	"strings"
@@ -156,7 +157,7 @@ func TestChainLoggedReportsEachStepBeforeTheNext(t *testing.T) {
 			}
 		}
 		return nil
-	})
+	}, nil)
 	if err != nil || len(calls) != 2 || res[1].Status != "ok" {
 		t.Fatalf("calls %v, err %v, results %+v", calls, err, res)
 	}
@@ -206,7 +207,7 @@ func TestChainLoggedStopsBetweenStepsWhenInterrupted(t *testing.T) {
 			cancel()
 		}
 		return nil
-	})
+	}, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -217,3 +218,88 @@ func TestChainLoggedStopsBetweenStepsWhenInterrupted(t *testing.T) {
 		t.Errorf("the finished step was undone or never ran: %v", err)
 	}
 }
+
+// TestDisplaceIsReportedBeforeTheStepThatNeededIt: trashing the file in the
+// way is a destructive act of its own, and it is durable the moment
+// trash.Put returns. Reporting it only when the whole step finishes leaves
+// a window - the entire data transfer of a copy or a cross-device move -
+// in which the user's file is in the Trash with nothing recording that it
+// went there. Killed in that window, krino log said "nothing applied".
+func TestDisplaceIsReportedBeforeTheStepThatNeededIt(t *testing.T) {
+	root := t.TempDir()
+	// The Trash must be on the same filesystem as the file being trashed.
+	t.Setenv("XDG_DATA_HOME", filepath.Join(root, "share"))
+	src := filepath.Join(root, "a.pdf")
+	dst := filepath.Join(root, "W", "a.pdf")
+	if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(dst, []byte("the file already there"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	c := planned(t, root, "a.pdf", "incoming",
+		plan.Step{Kind: plan.Copy, Src: src, Dst: dst, Displaces: dst},
+	)
+
+	var order []string
+	res, err := ChainLogged(context.Background(), c,
+		func(i int, sr StepResult) error {
+			order = append(order, "step")
+			return nil
+		},
+		func(i int, step plan.Step, entry string) error {
+			order = append(order, "displace")
+			if entry == "" {
+				t.Error("the displace was reported with no trash entry")
+			}
+			// The file is in the Trash already; the copy has not begun.
+			if b, err := os.ReadFile(dst); err == nil && string(b) == "incoming" {
+				t.Error("the displace was reported only after the copy had run")
+			}
+			return nil
+		})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(order) != 2 || order[0] != "displace" || order[1] != "step" {
+		t.Errorf("order = %v; want the displace reported first", order)
+	}
+	if res[0].Status != "ok" {
+		t.Errorf("step = %+v", res[0])
+	}
+}
+
+// TestDisplaceThatCannotBeReportedFailsTheStep: if the displace cannot be
+// written to the log, the step must not go on to use the name - the user's
+// file is already in the Trash and nothing would record it.
+func TestDisplaceThatCannotBeReportedFailsTheStep(t *testing.T) {
+	root := t.TempDir()
+	t.Setenv("XDG_DATA_HOME", filepath.Join(root, "share"))
+	src := filepath.Join(root, "a.pdf")
+	dst := filepath.Join(root, "W", "a.pdf")
+	if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(dst, []byte("the file already there"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	c := planned(t, root, "a.pdf", "incoming",
+		plan.Step{Kind: plan.Copy, Src: src, Dst: dst, Displaces: dst},
+	)
+	res, err := ChainLogged(context.Background(), c, nil,
+		func(i int, step plan.Step, entry string) error {
+			return errors.New("log is closed")
+		})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if res[0].Status != "failed" || !strings.Contains(res[0].Detail, "could not be logged") {
+		t.Errorf("step = %+v; want a failure naming the unlogged displace", res[0])
+	}
+	if res[0].DisplacedEntry == "" {
+		t.Error("the failure does not carry the trash entry, so nothing can say where the file went")
+	}
+	if b, rerr := os.ReadFile(dst); rerr == nil && string(b) == "incoming" {
+		t.Error("the copy ran even though the displace could not be logged")
+	}
+}
diff --git a/internal/engine/apply.go b/internal/engine/apply.go
index 1e59aa1..e2d4db8 100644
--- a/internal/engine/apply.go
+++ b/internal/engine/apply.go
@@ -118,6 +118,11 @@ func (e *Engine) applyFile(ctx context.Context, dirName string, c plan.Chain, ap
 			return unloggedStep(rel, c.Steps[i], sr, err)
 		}
 		return nil
+	}, func(i int, step plan.Step, entry string) error {
+		// The displaced file is in the Trash from this moment, whatever
+		// becomes of the step that needed its name, so its line is written
+		// here rather than with the rest of the step.
+		return e.logDisplace(j, run, dirName, rel, i+1, step, entry)
 	})
 	if err != nil {
 		return FileResult{}, err
@@ -125,6 +130,24 @@ func (e *Engine) applyFile(ctx context.Context, dirName string, c plan.Chain, ap
 	return FileResult{File: c.File, Steps: results}, nil
 }
 
+// logDisplace writes the line for a file trashed to make room, as soon as
+// it has reached the Trash. It is its own action with its own line (spec
+// §9), independent of whether the step that needed the name then succeeds.
+func (e *Engine) logDisplace(j *journal.Writer, run, dirName, rel string, stepNum int, step plan.Step, entry string) error {
+	dst := filepath.Join(trash.Dir(), "files", entry)
+	size, mtime := statSizeModTime(dst)
+	return j.Append(journal.Entry{
+		Time: e.Now(), Run: run, Dir: dirName, File: rel, Step: stepNum,
+		Action: "displace", Status: "ok", Rule: step.Rule,
+		// Detail carries the trash entry name explicitly: Dst's shape
+		// (trash.Dir()/files/<entry>) is internal/apply's and this file's
+		// own convention, not a contract undo may quietly depend on.
+		// PlanUndo/ApplyUndo read the name from here, never by taking
+		// Dst's basename.
+		Src: step.Displaces, Dst: dst, Size: size, ModTime: mtime, Detail: entry,
+	})
+}
+
 // unloggedStep is the error for a step whose log entry could not be
 // written. A step that ran is named with where its file is now: undo
 // cannot see it, so the user must be told where to look.
@@ -233,23 +256,6 @@ func actionName(k plan.Kind) string {
 // step's planned destination (informational only — PlanUndo never reverses
 // a non-"ok" entry) and Size/ModTime stay zero.
 func (e *Engine) logStep(j *journal.Writer, run, dirName, rel string, stepNum int, step plan.Step, sr apply.StepResult) error {
-	if sr.DisplacedEntry != "" {
-		dst := filepath.Join(trash.Dir(), "files", sr.DisplacedEntry)
-		size, mtime := statSizeModTime(dst)
-		if err := j.Append(journal.Entry{
-			Time: e.Now(), Run: run, Dir: dirName, File: rel, Step: stepNum,
-			Action: "displace", Status: "ok", Rule: step.Rule,
-			// Detail carries the trash entry name explicitly: Dst's shape
-			// (trash.Dir()/files/<entry>) is internal/apply's and this
-			// file's own convention, not a contract undo may quietly
-			// depend on. PlanUndo/ApplyUndo read the name from here,
-			// never by taking Dst's basename.
-			Src: step.Displaces, Dst: dst, Size: size, ModTime: mtime, Detail: sr.DisplacedEntry,
-		}); err != nil {
-			return err
-		}
-	}
-
 	for _, dir := range sr.Made {
 		size, mtime := statSizeModTime(dir)
 		if err := j.Append(journal.Entry{