1 files changed, 144 insertions, 0 deletions
diff --git a/cmd/krino/review_test.go b/cmd/krino/review_test.go
new file mode 100644
index 0000000..193a191
--- /dev/null
+++ b/cmd/krino/review_test.go
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import (
+	"strings"
+	"testing"
+
+	"krino/internal/plan"
+	"krino/internal/scan"
+)
+
+func chains(rels ...string) []plan.Chain {
+	out := make([]plan.Chain, len(rels))
+	for i, r := range rels {
+		out[i] = plan.Chain{File: scan.File{Rel: r}, Steps: []plan.Step{{Kind: plan.Move, Dst: "/w/" + r}}}
+	}
+	return out
+}
+
+func TestChoosePerFile(t *testing.T) {
+	// c enters per-file mode, then y n y for three files.
+	approved, action, err := reviewChains(strings.NewReader("cyny"), new(strings.Builder), chains("a", "b", "c"), "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if action != 'c' {
+		t.Errorf("action = %q", action)
+	}
+	if !approved["a"] || approved["b"] || !approved["c"] {
+		t.Errorf("approved = %v; want a and c only", approved)
+	}
+}
+
+func TestApplyAllAndSkip(t *testing.T) {
+	approved, action, _ := reviewChains(strings.NewReader("a"), new(strings.Builder), chains("a", "b"), "")
+	if action != 'a' || len(approved) != 2 {
+		t.Errorf("[a] = %q %v; want every file approved", action, approved)
+	}
+	approved, action, _ = reviewChains(strings.NewReader("s"), new(strings.Builder), chains("a", "b"), "")
+	if action != 's' || len(approved) != 0 {
+		t.Errorf("[s] = %q %v; want nothing approved", action, approved)
+	}
+}
+
+func TestPerFileDoneStopsAsking(t *testing.T) {
+	// c, y for the first, then d: apply what was chosen so far.
+	approved, _, _ := reviewChains(strings.NewReader("cyd"), new(strings.Builder), chains("a", "b", "c"), "")
+	if !approved["a"] || approved["b"] || approved["c"] {
+		t.Errorf("approved = %v; want only a", approved)
+	}
+}
+
+// TestPerFileQuitAppliesNothing: spec §8.3's [q] on the per-file prompt is
+// "quit, apply nothing" - stronger than [d], which keeps what was already
+// chosen. It folds into the same top-level 'q' the caller already handles
+// for the directory-level menu (spec §8.2's [q]), and discards even a file
+// already marked yes.
+func TestPerFileQuitAppliesNothing(t *testing.T) {
+	approved, action, err := reviewChains(strings.NewReader("cyq"), new(strings.Builder), chains("a", "b"), "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if action != 'q' {
+		t.Errorf("action = %q, want 'q'", action)
+	}
+	if len(approved) != 0 {
+		t.Errorf("approved = %v; want nothing, even though a was marked yes first", approved)
+	}
+}
+
+// TestPerFileYesToAllRemaining: spec §8.3's [a] mid-review approves the
+// current file and every remaining one without asking again.
+func TestPerFileYesToAllRemaining(t *testing.T) {
+	approved, action, err := reviewChains(strings.NewReader("ca"), new(strings.Builder), chains("a", "b", "c"), "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if action != 'c' || len(approved) != 3 {
+		t.Errorf("approved = %v action = %q; want all three approved", approved, action)
+	}
+}
+
+// TestInvalidKeyReprompts: an unrecognised key at either the top-level menu
+// or the per-file prompt does not abort the review - it is reported and the
+// same prompt is read again.
+func TestInvalidKeyReprompts(t *testing.T) {
+	out := new(strings.Builder)
+	approved, action, err := reviewChains(strings.NewReader("zs"), out, chains("a"), "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if action != 's' || len(approved) != 0 {
+		t.Errorf("approved = %v action = %q; want [s] after the bad key", approved, action)
+	}
+	if !strings.Contains(out.String(), "z") {
+		t.Errorf("no mention of the rejected key:\n%s", out)
+	}
+}
+
+// TestPerFileDestinationIsRootRelative is review finding 1 (fix round
+// 2026-09-12): reviewPerFile must render a destination the same way the
+// directory-level table does (render.go's destText) - root-relative for a
+// destination inside root, ~-abbreviated for one outside it - not always
+// abbreviated because root was never passed through to actionCell at all.
+// Both halves are pinned: getting only the "inside" half right would still
+// let an outside-root destination silently regress to some other form.
+func TestPerFileDestinationIsRootRelative(t *testing.T) {
+	t.Setenv("HOME", "/home/x")
+	root := "/home/x/dl"
+	cs := []plan.Chain{
+		{File: scan.File{Rel: "inside.txt"}, Steps: []plan.Step{{Kind: plan.Move, Dst: root + "/Work/Acme/inside.txt"}}},
+		{File: scan.File{Rel: "outside.txt"}, Steps: []plan.Step{{Kind: plan.Move, Dst: "/home/x/backup/outside.txt"}}},
+	}
+	out := new(strings.Builder)
+	if _, _, err := reviewChains(strings.NewReader("cyy"), out, cs, root); err != nil {
+		t.Fatal(err)
+	}
+	text := out.String()
+	if !strings.Contains(text, "move    → Work/Acme/") {
+		t.Errorf("destination inside root should be root-relative, not ~-abbreviated:\n%s", text)
+	}
+	if !strings.Contains(text, "move    → ~/backup/") {
+		t.Errorf("destination outside root should be ~-abbreviated:\n%s", text)
+	}
+}
+
+// TestNoColourEscapeFromColourFalse pins Ruling 7's safety guarantee at the
+// unit that actually decides it: with colour off, colourDeletePermanently
+// must not alter the text at all, and with colour on it must add an escape
+// around exactly the one marker spec §8.2 singles out for emphasis.
+func TestColourDeletePermanently(t *testing.T) {
+	plain := "  4  setup-1.2.deb  DELETE permanently  old-pkgs  age 94d\n"
+	if got := colourDeletePermanently(plain, false); got != plain {
+		t.Errorf("colour=false must leave the text untouched:\n%q", got)
+	}
+	got := colourDeletePermanently(plain, true)
+	if !strings.Contains(got, "\x1b[") {
+		t.Errorf("colour=true should add an escape sequence:\n%q", got)
+	}
+	if !strings.Contains(got, "DELETE permanently") {
+		t.Errorf("colour=true should not remove the marker text itself:\n%q", got)
+	}
+}