plan 10: missing and weak tests (per-step logging, trash path, fuzz oracle, w after error)

7 files changed, 183 insertions, 26 deletions

diff --git a/cmd/krino/history_test.go b/cmd/krino/history_test.go
index 6d8c1e4..7c0f810 100644
--- a/cmd/krino/history_test.go
+++ b/cmd/krino/history_test.go
@@ -506,6 +506,8 @@ func TestMinAgeRejectedOutsideSortAndExplain(t *testing.T) {
 		{"--min-age", "garbage", "undo", "-n"},
 		{"check", "--min-age", "1d"},
 		{"log", "--min-age", "1d"},
+		{"init", "--min-age", "1d"},
+		{"new", "--min-age", "1d", "x", "/tmp"},
 		{"-n", "--min-age="},
 	} {
 		if code, _, errOut := runCLI(t, args...); code != 2 || !strings.Contains(errOut, "--min-age") {
diff --git a/cmd/krino/review_test.go b/cmd/krino/review_test.go
index a1fbba7..b138451 100644
--- a/cmd/krino/review_test.go
+++ b/cmd/krino/review_test.go
@@ -3,6 +3,9 @@
 package main
 
 import (
+	"context"
+	"errors"
+	"fmt"
 	"io"
 	"strings"
 	"testing"
@@ -295,3 +298,31 @@ func TestNotReviewedOutcome(t *testing.T) {
 		t.Errorf("got %q", got)
 	}
 }
+
+// TestStopAfterApply: [w] stops krino after its directory whether or not the
+// apply succeeded (review cli F3) - a log that cannot be written must not
+// lead on to planning and prompting the next directory - and an interrupt
+// always stops it.
+func TestStopAfterApply(t *testing.T) {
+	logErr := errors.New("write krino.log: no space left on device")
+	cases := []struct {
+		action rune
+		err    error
+		want   bool
+	}{
+		{'a', nil, false},
+		{'c', nil, false},
+		{'w', nil, true},
+		{'a', logErr, false},
+		{'c', logErr, false},
+		{'w', logErr, true},
+		{'a', context.Canceled, true},
+		{'c', fmt.Errorf("apply: %w", context.Canceled), true},
+		{'a', context.DeadlineExceeded, true},
+	}
+	for _, c := range cases {
+		if got := stopAfterApply(c.action, c.err); got != c.want {
+			t.Errorf("stopAfterApply(%q, %v) = %v, want %v", c.action, c.err, got, c.want)
+		}
+	}
+}
diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go
index 4d5f509..637587d 100644
--- a/cmd/krino/sort.go
+++ b/cmd/krino/sort.go
@@ -134,7 +134,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 		// of polling forever.
 		l, err := lock.Acquire(ctx, e.Config.LockFile(d.Name), !g.yes)
 		if err != nil {
-			if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
+			if interrupted(err) {
 				// Interrupted while waiting for the lock: an interrupt, not
 				// a failure - the ctx.Err() check at the end of this
 				// function already turns this into exit 130, and nothing
@@ -258,21 +258,16 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 			}
 			res, aerr := e.Apply(ctx, toApply, approved, j, run)
 			if aerr != nil {
-				if errors.Is(aerr, context.Canceled) || errors.Is(aerr, context.DeadlineExceeded) {
-					// Interrupted mid-apply (fix round 2026-09-12/item 2):
-					// treated exactly like the cancelled lock wait above -
-					// not a failure ("context canceled" is a Go-ism, not
-					// something to show a user who just pressed Ctrl-C),
-					// and no further directory is even attempted. The
-					// ctx.Err() check at the end of this function already
-					// turns this into exit 130.
-					return true
+				// Interrupted mid-apply (fix round 2026-09-12/item 2):
+				// treated exactly like the cancelled lock wait above - not
+				// a failure ("context canceled" is a Go-ism, not something
+				// to show a user who just pressed Ctrl-C). The ctx.Err()
+				// check at the end of this function turns it into exit 130.
+				if !interrupted(aerr) {
+					fmt.Fprintf(stderr, "krino: %s: %v\n", d.Name, aerr)
+					exit = 1
 				}
-				fmt.Fprintf(stderr, "krino: %s: %v\n", d.Name, aerr)
-				exit = 1
-				// [w] stops krino whether or not its apply succeeded (review
-				// cli F3): no later directory is planned or asked about.
-				return action == 'w'
+				return stopAfterApply(action, aerr)
 			}
 			fmt.Fprintln(stdout, withNotReviewed(outcome(p, res.Applied, res.Failed, res.Declined), notReviewed))
 			// Ruling 1: only an actual step failure makes the run exit 1
@@ -280,7 +275,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 			if res.Failed > 0 {
 				exit = 1
 			}
-			return action == 'w'
+			return stopAfterApply(action, nil)
 		}()
 
 		if quit {
@@ -583,3 +578,16 @@ func padCell(s string, w int) string {
 	}
 	return s + strings.Repeat(" ", w-n)
 }
+
+// interrupted reports whether err is the context being cancelled (Ctrl-C,
+// SIGTERM, SIGHUP) rather than a failure.
+func interrupted(err error) bool {
+	return errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)
+}
+
+// stopAfterApply reports whether krino stops once a directory has been
+// applied: an interrupt stops it, and so does [w], whether or not its apply
+// succeeded (review cli F3) - no later directory is planned or asked about.
+func stopAfterApply(action rune, err error) bool {
+	return interrupted(err) || action == 'w'
+}
diff --git a/internal/engine/property_test.go b/internal/engine/property_test.go
index 135c20a..8a43d7b 100644
--- a/internal/engine/property_test.go
+++ b/internal/engine/property_test.go
@@ -135,7 +135,7 @@ func contentCounts(snap map[string]string) map[string]int {
 
 // checkApplyUndo builds c in a sandbox, applies every chain, checks nothing
 // was lost, undoes the run and checks the home directory - files and
-// directories - is as it was. It reports whether any file was applied.
+// directories - is as it was. It reports whether any step actually ran.
 func checkApplyUndo(t *testing.T, c propertyCase) bool {
 	h := sandbox(t)
 	old := time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC)
@@ -253,5 +253,5 @@ func checkApplyUndo(t *testing.T, c propertyCase) bool {
 			}
 		}
 	}
-	return true
+	return res.Applied > 0
 }
diff --git a/internal/engine/undo_identity_test.go b/internal/engine/undo_identity_test.go
index 7de4eb9..f9d6307 100644
--- a/internal/engine/undo_identity_test.go
+++ b/internal/engine/undo_identity_test.go
@@ -432,3 +432,63 @@ func TestUndoDoesNotOfferOnlyADirectoryRemoval(t *testing.T) {
 		t.Errorf("after a complete undo, still offered: %+v", again.Files)
 	}
 }
+
+// TestApplyLogsEachStepAsItCompletes: the first step's log entry is written
+// before the second step runs - observed from the clock the log asks for
+// each entry's time - so a run killed mid-chain leaves what it did undoable
+// (review M9).
+func TestApplyLogsEachStepAsItCompletes(t *testing.T) {
+	h := sandbox(t)
+	p := filepath.Join(h, "dl", "a.pdf")
+	os.MkdirAll(filepath.Dir(p), 0o755)
+	os.WriteFile(p, []byte("one"), 0o644)
+	old := time.Now().Add(-2 * time.Hour)
+	os.Chtimes(p, old, old)
+	main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": "(path \"~/dl\")\n(rule \"r\" (rename \"r-{name}\") (move \"Out\"))\n"})
+	e, errs := Load(main)
+	if len(errs) > 0 {
+		t.Fatal(errs)
+	}
+	dp, err := e.Plan(context.Background(), e.Dirs[0], plan.NewClaims())
+	if err != nil {
+		t.Fatal(err)
+	}
+	j, err := journal.Open(filepath.Join(h, "state", "krino.log"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer j.Close()
+	calls := 0
+	e.Now = func() time.Time {
+		calls++
+		if calls == 2 { // the rename's own entry is about to be written
+			if _, err := os.Lstat(filepath.Join(h, "dl", "Out", "r-a.pdf")); err == nil {
+				t.Error("the move had already run when the rename was logged: steps are logged after the whole chain")
+			}
+		}
+		return time.Now()
+	}
+	if _, err := e.Apply(context.Background(), dp, map[string]bool{"a.pdf": true}, j, "R"); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// TestUndoRefusesATrashEntryRecordedForAnotherPath: a trash entry with the
+// size and mtime the run logged, whose trashinfo now names another original
+// path, belongs to another file and is refused (review M2).
+func TestUndoRefusesATrashEntryRecordedForAnotherPath(t *testing.T) {
+	e, run, h, _ := appliedRun(t, map[string]map[string]string{"dl": {"a.pdf": "one"}},
+		map[string]string{"dl": "(path \"~/dl\")\n(rule \"r\" (delete))\n"})
+	info := filepath.Join(trash.Dir(), "info", "a.pdf.trashinfo")
+	body := "[Trash Info]\nPath=" + filepath.Join(h, "elsewhere", "a.pdf") + "\nDeletionDate=2026-09-14T10:00:00\n"
+	if err := os.WriteFile(info, []byte(body), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	up, err := e.PlanUndo(run)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if f := undoFileNamed(t, up, "dl", "a.pdf"); !strings.Contains(f.Refused, "belongs to another file") {
+		t.Errorf("Refused = %q; want the trash entry named as another file's", f.Refused)
+	}
+}
diff --git a/internal/plan/fuzz_test.go b/internal/plan/fuzz_test.go
index 6deb225..b17d3c4 100644
--- a/internal/plan/fuzz_test.go
+++ b/internal/plan/fuzz_test.go
@@ -3,6 +3,7 @@
 package plan
 
 import (
+	"path"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -10,6 +11,7 @@ import (
 
 	"krino/internal/config"
 	"krino/internal/scan"
+	"krino/internal/xdg"
 )
 
 // FuzzExpand: expanding any template against any file name never panics,
@@ -17,7 +19,8 @@ import (
 // capture groups there are - MaxIndex and Expand read placeholders alike.
 // Used as a destination - relative, in the home directory, or bare - a
 // template never plans a move outside the directory its text names before
-// the first placeholder (review M1).
+// the first placeholder (review M1), checked against textDir, not the
+// planner's own staticDir.
 func FuzzExpand(f *testing.F) {
 	for _, s := range []string{"{name}", "{stem}{ext}", "{mtime:%Y/%m}", "{1}_{2}", "{{literal}}", "{", "}", "{now:%", "{0}", "{9}", "{99999999999999999999}"} {
 		f.Add(s, "a.b.pdf")
@@ -37,15 +40,65 @@ func FuzzExpand(f *testing.F) {
 		if n, err := MaxIndex(tmpl); err == nil && n > 2 && errA == nil {
 			t.Fatalf("Expand(%q) = %q, though it uses {%d} and only 2 groups exist", tmpl, a, n)
 		}
+		if name == "" || name == "." || name == ".." || strings.ContainsAny(name, "/\x00") {
+			return // not a name a directory entry can have
+		}
 		for _, dest := range []string{tmpl, "Out/" + tmpl, "~/docs/" + tmpl} {
-			in := []Input{{
-				File:  scan.File{Path: "/r/x.pdf", Rel: "x.pdf", Name: "x.pdf", ModTime: facts.ModTime},
-				Rules: []RuleMatch{{Name: "a", Captures: facts.Captures, Actions: []config.Action{{Kind: config.Move, Arg: dest}}}},
-			}}
-			s := Build("/r", in, facts.Now, NoDisk{}, NewClaims())[0].Steps[0]
-			if strings.ContainsRune(dest, '{') && s.Skip == "" && !within(filepath.Dir(s.Dst), staticDir(dest, "/r")) {
-				t.Fatalf("destination %q planned %q, outside %q", dest, s.Dst, staticDir(dest, "/r"))
+			if !strings.ContainsRune(dest, '{') {
+				continue
+			}
+			got := plannedDir(dest, name, facts.Captures, facts)
+			if got == "" {
+				continue
+			}
+			if base := textDir(dest, "/r", xdg.Home()); !underDir(got, base) {
+				t.Fatalf("destination %q planned %q for %q, outside %q, the directory its text names", dest, got, name, base)
 			}
 		}
 	})
 }
+
+// plannedDir builds a move of a file named name to dest with captures and
+// returns the directory the step would put it in, or "" when the step is
+// skipped.
+func plannedDir(dest, name string, captures []string, facts Facts) string {
+	in := []Input{{
+		File:  scan.File{Path: "/r/" + name, Rel: name, Name: name, ModTime: facts.ModTime},
+		Rules: []RuleMatch{{Name: "a", Captures: captures, Actions: []config.Action{{Kind: config.Move, Arg: dest}}}},
+	}}
+	s := Build("/r", in, facts.Now, NoDisk{}, NewClaims())[0].Steps[0]
+	if s.Skip != "" {
+		return ""
+	}
+	return filepath.Dir(s.Dst)
+}
+
+// textDir is the directory a destination's text names before its first
+// '{' (spec 15.1), worked out here by hand rather than by staticDir: the
+// text up to the last '/' before the brace, "~" as the home directory, a
+// relative path under root.
+func textDir(dest, root, home string) string {
+	prefix := dest[:strings.IndexByte(dest, '{')]
+	switch cut := strings.LastIndexByte(prefix, '/'); {
+	case cut < 0:
+		prefix = ""
+	case cut == 0:
+		return "/"
+	default:
+		prefix = prefix[:cut]
+	}
+	switch {
+	case prefix == "~":
+		return home
+	case strings.HasPrefix(prefix, "~/"):
+		return path.Join(home, prefix[2:])
+	case strings.HasPrefix(prefix, "/"):
+		return path.Clean(prefix)
+	}
+	return path.Join(root, prefix)
+}
+
+// underDir reports whether path is dir or lies under it, by whole segments.
+func underDir(path, dir string) bool {
+	return dir == "/" || path == dir || strings.HasPrefix(path, dir+"/")
+}
diff --git a/internal/plan/testdata/fuzz/FuzzExpand/a8c52dd03776dd2c b/internal/plan/testdata/fuzz/FuzzExpand/a8c52dd03776dd2c
new file mode 100644
index 0000000..0201563
--- /dev/null
+++ b/internal/plan/testdata/fuzz/FuzzExpand/a8c52dd03776dd2c
@@ -0,0 +1,3 @@
+go test fuzz v1
+string("{{/{2}")
+string("0")