tests: apply error exits 1 and w stops krino through the real command; undo projection occupied half

4 files changed, 130 insertions, 1 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0b82a8d..0b3ac7b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,9 @@
   could change its answer; `explain` shows `?`. A document read only in part
   answers the keywords found in what was read, and leaves the others
   unknown.
+- Tests: a genuine apply error exits 1, and `w` in review stops krino even
+  when applying fails, both through the real command; the undo projection's
+  "about to be occupied" half.
 - Build: `make build` and `make install` refuse a `VERSION` that is not a
   plain version string, as `cross` and `release` did; `make cross` names
   its directories without the tag's `v`; the dependency gate covers test
diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go
index 163d388..5a175a4 100644
--- a/cmd/krino/sort.go
+++ b/cmd/krino/sort.go
@@ -37,6 +37,11 @@ import (
 // what go test does with stdin.
 var stdin = os.Stdin
 
+// stdinIsTerminal is the check that review can prompt at all, a seam so a
+// test can drive review through a pipe (tui.ReadKey reads a pipe byte by
+// byte).
+var stdinIsTerminal = func() bool { return term.IsTerminal(int(stdin.Fd())) }
+
 // zeroOutcome is the per-directory outcome line for a directory that had
 // nothing applied to it - either because nothing was actionable, or
 // because the user chose [s] or [q] - so the same wording is not retyped
@@ -73,7 +78,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 
 	// Spec §8.4: with neither -y nor -n, krino asks; asking a non-terminal
 	// stdin would just hang (or read garbage), so it refuses instead.
-	if !g.yes && !g.dry && !term.IsTerminal(int(stdin.Fd())) {
+	if !g.yes && !g.dry && !stdinIsTerminal() {
 		return usageError(stderr, "refusing to prompt: stdin is not a terminal (use -y or -n)")
 	}
 
diff --git a/cmd/krino/sort_test.go b/cmd/krino/sort_test.go
index 2a72e6a..22e00b4 100644
--- a/cmd/krino/sort_test.go
+++ b/cmd/krino/sort_test.go
@@ -265,3 +265,71 @@ func TestLaterDirectoryIsNotBlockedByAnEarlierOnesClaims(t *testing.T) {
 		t.Errorf("cb's x.txt did not move into the place ca's left free: %q, %v\n%s", b, err, out)
 	}
 }
+
+// devFullFixture builds two directories, d1 and d2, each with two files a
+// rule moves, and a log at /dev/full, where every write fails: applying
+// anything is a genuine apply error, not a step failure.
+func devFullFixture(t *testing.T) {
+	t.Helper()
+	if _, err := os.Stat("/dev/full"); err != nil {
+		t.Skip("no /dev/full on this system")
+	}
+	h := home(t)
+	old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
+	if code, _, errOut := runCLI(t, "init"); code != 0 {
+		t.Fatal(errOut)
+	}
+	for _, n := range []string{"d1", "d2"} {
+		for _, name := range []string{"a.pdf", "b.pdf"} {
+			p := filepath.Join(h, n, name)
+			os.MkdirAll(filepath.Dir(p), 0o755)
+			os.WriteFile(p, []byte(n+name), 0o644)
+			os.Chtimes(p, old, old)
+		}
+		if code, _, errOut := runCLI(t, "new", n, filepath.Join(h, n)); code != 0 {
+			t.Fatal(errOut)
+		}
+		os.WriteFile(filepath.Join(h, ".config", "krino", "dirs", n+".conf"), []byte("(path \"~/"+n+"\")\n(rule \"r\" (move \"Out\"))\n"), 0o644)
+	}
+	f, err := os.OpenFile(filepath.Join(h, ".config", "krino", "krino.conf"), os.O_APPEND|os.O_WRONLY, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	f.WriteString("(log \"/dev/full\")\n")
+	f.Close()
+}
+
+// TestApplyErrorExitsOne: an error applying a directory - here the log
+// cannot be written - makes krino exit 1 (triage 34m: removing that exit
+// code left every test passing).
+func TestApplyErrorExitsOne(t *testing.T) {
+	devFullFixture(t)
+	code, _, errOut := runCLI(t, "-y", "d1")
+	if code != 1 || !strings.Contains(errOut, "no space left") {
+		t.Errorf("exit %d, stderr %q; want 1 and the write error", code, errOut)
+	}
+}
+
+// TestWriteStopsKrinoWhenApplyFails: [w] in review stops krino after its
+// directory even when applying it fails - the next directory is not planned
+// or asked about (review cli F3), driven through the real command with a
+// pipe standing in for the terminal (triage 28m).
+func TestWriteStopsKrinoWhenApplyFails(t *testing.T) {
+	devFullFixture(t)
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	w.WriteString("cyw") // choose per file, yes to a.pdf, write before b.pdf
+	w.Close()
+	oldStdin, oldTerm := stdin, stdinIsTerminal
+	stdin, stdinIsTerminal = r, func() bool { return true }
+	t.Cleanup(func() { stdin, stdinIsTerminal = oldStdin, oldTerm })
+	code, out, errOut := runCLI(t, "--no-pager")
+	if code != 1 {
+		t.Errorf("exit %d, want 1\n%s\n%s", code, out, errOut)
+	}
+	if strings.Contains(out, "krino: d2") {
+		t.Errorf("d2 was planned after [w]:\n%s", out)
+	}
+}
diff --git a/internal/engine/undo_identity_test.go b/internal/engine/undo_identity_test.go
index f42c5da..01f57d8 100644
--- a/internal/engine/undo_identity_test.go
+++ b/internal/engine/undo_identity_test.go
@@ -572,3 +572,56 @@ func TestUndoRunWithADamagedLineIsStillAnUndo(t *testing.T) {
 		t.Errorf("PlanUndo(undo run with a damaged line) = %v; want refused as an undo", err)
 	}
 }
+
+// TestUndoOverwriteThenMove: a move that replaced an existing file and was
+// then moved on is undone whole - the moved file goes back, and the file it
+// replaced is restored to the path the later reversal vacates (triage 34m).
+func TestUndoOverwriteThenMove(t *testing.T) {
+	e, run, h, logPath := appliedRun(t, map[string]map[string]string{"dl": {"a.pdf": "one", "Out/a.pdf": "old"}},
+		map[string]string{"dl": "(path \"~/dl\")\n(ignore \"Out/\")\n(recursive yes)\n(rule \"r\" (on-conflict overwrite) (move \"Out\") (move \"Out2\"))\n"})
+	if b, _ := os.ReadFile(filepath.Join(h, "dl", "Out2", "a.pdf")); string(b) != "one" {
+		t.Fatalf("setup: the chain did not run as planned")
+	}
+	up, err := e.PlanUndo(run)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, f := range up.Files {
+		if f.Refused != "" {
+			t.Fatalf("%s refused: %s (%+v)", f.File, f.Refused, f.Steps)
+		}
+	}
+	j, err := journal.Open(logPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer j.Close()
+	if _, err := e.ApplyUndo(context.Background(), up, j, journal.NewRunID(time.Now())); err != nil {
+		t.Fatal(err)
+	}
+	for rel, want := range map[string]string{"a.pdf": "one", "Out/a.pdf": "old"} {
+		if b, err := os.ReadFile(filepath.Join(h, "dl", rel)); err != nil || string(b) != want {
+			t.Errorf("%s after undo: %q, %v; want %q", rel, b, err, want)
+		}
+	}
+}
+
+// TestProjectionSeesAPathAnEarlierStepWillFill: a path not on disk yet that
+// a queued reversal will put a file at is occupied for the steps after it,
+// and free again once a later one moves that file on (triage 34m: the
+// "occupied" half of the projection had no test that could fail).
+func TestProjectionSeesAPathAnEarlierStepWillFill(t *testing.T) {
+	x := filepath.Join(t.TempDir(), "x.pdf")
+	p := newUndoProjection()
+	if p.occupiedNow(x) {
+		t.Fatal("an empty path reads as occupied")
+	}
+	p.record(UndoStep{Action: "undo-move", Src: x + ".elsewhere", Dst: x})
+	if !p.occupiedNow(x) {
+		t.Error("a path a queued undo-move fills reads as free")
+	}
+	p.record(UndoStep{Action: "undo-rename", Src: x, Dst: x + ".back"})
+	if p.occupiedNow(x) {
+		t.Error("a path a later reversal vacates still reads as occupied")
+	}
+}