krino: acting — trash, journal, apply, lock, review, undo

1 files changed, 317 insertions, 0 deletions

diff --git a/cmd/krino/history_test.go b/cmd/krino/history_test.go
new file mode 100644
index 0000000..2667b63
--- /dev/null
+++ b/cmd/krino/history_test.go
@@ -0,0 +1,317 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"krino/internal/engine"
+)
+
+func TestLogListsRunsAndUndoReverses(t *testing.T) {
+	h := matchingFixture(t)
+	if code, _, errOut := runCLI(t, "-y"); code != 0 {
+		t.Fatalf("apply: %d %s", code, errOut)
+	}
+	filed := filepath.Join(h, "dl", "Work", "Acme", "inv1.txt")
+	if _, err := os.Stat(filed); err != nil {
+		t.Fatalf("nothing was filed: %v", err)
+	}
+
+	code, out, errOut := runCLI(t, "log")
+	if code != 0 {
+		t.Fatalf("log: %d %s", code, errOut)
+	}
+	if !strings.Contains(out, "moved") || !strings.Contains(out, "dl") {
+		t.Errorf("log output:\n%s", out)
+	}
+
+	if code, _, errOut = runCLI(t, "undo", "-y"); code != 0 {
+		t.Fatalf("undo: %d %s", code, errOut)
+	}
+	if _, err := os.Stat(filepath.Join(h, "dl", "inv1.txt")); err != nil {
+		t.Errorf("undo did not put the file back: %v", err)
+	}
+	if _, err := os.Stat(filed); !os.IsNotExist(err) {
+		t.Error("the filed copy survived the undo")
+	}
+
+	if _, out, _ = runCLI(t, "log"); !strings.Contains(out, "undone") {
+		t.Errorf("log does not mark the run undone:\n%s", out)
+	}
+	if code, _, errOut = runCLI(t, "undo", "-y"); code == 0 {
+		t.Errorf("undoing an undo run succeeded: %q", errOut)
+	}
+}
+
+func TestUndoDryRunChangesNothing(t *testing.T) {
+	h := matchingFixture(t)
+	runCLI(t, "-y")
+	filed := filepath.Join(h, "dl", "Work", "Acme", "inv1.txt")
+	if code, out, _ := runCLI(t, "undo", "-n"); code != 0 || !strings.Contains(out, "undo-move") {
+		t.Errorf("undo -n: %d\n%s", code, out)
+	}
+	if _, err := os.Stat(filed); err != nil {
+		t.Error("undo -n moved a file")
+	}
+}
+
+// TestUndoFailsImmediatelyWithHeldLock is fix round 2026-09-12, item 1: an
+// undo moves files just as an apply does, so it needs the same per-directory
+// guard sort's TestSecondRunFailsImmediatelyWithYes already pins for the
+// forward path (spec §11: "a second krino on the same directory ... fails
+// immediately with -y"). The lock file is held under the config NAME "dl",
+// not any filesystem path - UndoFile.Dir is the journal's `dir` column,
+// which is the directory's name from krino.conf, not its root.
+//
+// Ruling (fix round 2026-09-12, follow-up): the lock is acquired before the
+// plan is even shown, matching cmdSort's own window (acquired before
+// Plan/review, held across both) rather than only around ApplyUndo - so
+// this also asserts the refusal is noticed before any plan output reaches
+// stdout. A version of this test that only checked the exit code and
+// stderr would pass equally whether the lock were taken early or late, and
+// so would not be pinning the thing this ruling is actually about.
+func TestUndoFailsImmediatelyWithHeldLock(t *testing.T) {
+	h := matchingFixture(t)
+	if code, _, errOut := runCLI(t, "-y"); code != 0 {
+		t.Fatalf("apply: %d %s", code, errOut)
+	}
+	filed := filepath.Join(h, "dl", "Work", "Acme", "inv1.txt")
+	if _, err := os.Stat(filed); err != nil {
+		t.Fatalf("nothing was filed: %v", err)
+	}
+
+	held := filepath.Join(h, ".local", "state", "krino", "dl.lock")
+	if err := os.MkdirAll(filepath.Dir(held), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(held, []byte(fmt.Sprintf("pid %d\n", os.Getpid())), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	code, out, errOut := runCLI(t, "undo", "-y")
+	if code != 1 || !strings.Contains(errOut, "another krino") {
+		t.Errorf("undo -y against a held lock: %d %q", code, errOut)
+	}
+	if strings.Contains(out, "to reverse") || strings.Contains(out, "undo-move") {
+		t.Errorf("undo printed the plan before noticing the held lock:\n%s", out)
+	}
+	if _, err := os.Stat(filed); err != nil {
+		t.Errorf("undo reversed the file despite the held lock: %v", err)
+	}
+}
+
+// undoFiles builds minimal engine.UndoFile fixtures for reviewUndoFiles/
+// reviewUndoPerFile, named by rel path only (Dir left blank - the tests
+// below never render it).
+func undoFiles(rels ...string) []engine.UndoFile {
+	out := make([]engine.UndoFile, len(rels))
+	for i, r := range rels {
+		out[i] = engine.UndoFile{File: r, Steps: []engine.UndoStep{{Action: "undo-move", Src: "/t/" + r, Dst: "/s/" + r}}}
+	}
+	return out
+}
+
+// TestFinalizeUndoPlanMarksUnapprovedAsDeclined is the wiring point for fix
+// round 2026-09-12, item 2: a refused file rides through untouched (its own
+// Refused reason is what ApplyUndo checks first), an approved file rides
+// through untouched too, and anything else - explicitly declined, or never
+// reached because [d]/[q] cut a per-file review short - comes out with
+// Declined set rather than being dropped from the plan.
+func TestFinalizeUndoPlanMarksUnapprovedAsDeclined(t *testing.T) {
+	up := &engine.UndoPlan{Run: "r1", Files: []engine.UndoFile{
+		{File: "a"},                  // index 0: approved
+		{File: "b"},                  // index 1: not approved -> declined
+		{File: "c", Refused: "gone"}, // index 2: refused, never declined
+	}}
+	out := finalizeUndoPlan(up, map[int]bool{0: true})
+	if len(out.Files) != 3 {
+		t.Fatalf("files = %+v, want all three carried through", out.Files)
+	}
+	if out.Files[0].Declined || out.Files[0].Refused != "" {
+		t.Errorf("approved file changed: %+v", out.Files[0])
+	}
+	if !out.Files[1].Declined || out.Files[1].Refused != "" {
+		t.Errorf("unapproved file not marked declined: %+v", out.Files[1])
+	}
+	if out.Files[2].Declined {
+		t.Errorf("a refused file must not also be marked declined: %+v", out.Files[2])
+	}
+	if out.Files[2].Refused != "gone" {
+		t.Errorf("refused file's reason changed: %+v", out.Files[2])
+	}
+}
+
+// TestReviewUndoApplyAll: [a] approves every reversible file by index.
+func TestReviewUndoApplyAll(t *testing.T) {
+	approved, action, err := reviewUndoFiles(strings.NewReader("a"), new(strings.Builder), undoFiles("a", "b"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if action != 'a' || len(approved) != 2 || !approved[0] || !approved[1] {
+		t.Errorf("approved = %v action = %q; want both approved", approved, action)
+	}
+}
+
+// TestReviewUndoSkipAndQuit: [s] and [q] both approve nothing.
+func TestReviewUndoSkipAndQuit(t *testing.T) {
+	approved, action, _ := reviewUndoFiles(strings.NewReader("s"), new(strings.Builder), undoFiles("a", "b"))
+	if action != 's' || len(approved) != 0 {
+		t.Errorf("[s] = %q %v; want nothing approved", action, approved)
+	}
+	approved, action, _ = reviewUndoFiles(strings.NewReader("q"), new(strings.Builder), undoFiles("a", "b"))
+	if action != 'q' || len(approved) != 0 {
+		t.Errorf("[q] = %q %v; want nothing approved", action, approved)
+	}
+}
+
+// TestReviewUndoChoosePerFile: [c] then per-file y/n, keyed by index.
+func TestReviewUndoChoosePerFile(t *testing.T) {
+	approved, action, err := reviewUndoFiles(strings.NewReader("cyn"), new(strings.Builder), undoFiles("a", "b"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if action != 'c' || !approved[0] || approved[1] {
+		t.Errorf("approved = %v action = %q; want only index 0", approved, action)
+	}
+}
+
+// TestReviewUndoRefusedFileNotPrompted is spec §10: a file PlanUndo already
+// refused is shown (with its reason - covered end to end by
+// TestUndoRefusesChangedDestination-style flows through cmdUndo) but never
+// asked about, so a [c] session reading one key per file must not stall
+// waiting for a key that reviewUndoPerFile never asks for. Three files, one
+// key each for the two reversible ones ("y", "n"), none for the refused
+// middle one: if it were prompted, the second key ("n") would answer for it
+// instead of the third file, and this test would see index 2 approved
+// instead of unset.
+func TestReviewUndoRefusedFileNotPrompted(t *testing.T) {
+	files := undoFiles("a", "b", "c")
+	files[1].Refused = "b changed since the run"
+	out := new(strings.Builder)
+	approved, action, err := reviewUndoFiles(strings.NewReader("cyn"), out, files)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if action != 'c' || !approved[0] || approved[1] || approved[2] {
+		t.Errorf("approved = %v action = %q; want only index 0 (1 is refused, 2 never reached)", approved, action)
+	}
+	if !strings.Contains(out.String(), "b changed since the run") {
+		t.Errorf("refused file's reason not shown:\n%s", out)
+	}
+}
+
+// TestPrintUndoPlan is fix wave item 3 (Important), rebuilding fix round
+// 2026-09-12's own golden test: that version hand-built its UndoSteps,
+// including a Dst on the undo-copy step the real code never sets (Dst is
+// deliberately left "" - trash.Put only chooses the entry name at execution
+// time), so it was structurally incapable of catching the bug it was meant
+// to guard against - an undo-copy row rendering as a bare
+// "undo-copy      → " with nothing said about what it would do to the
+// user's backup copy, the single most destructive step an undo plan takes.
+// The same lesson as Task 9's review: a hand-assembled fixture hides a test
+// that cannot detect a broken copy-undo. This version runs a REAL forward
+// apply (copy then move, so mkdir, copy and move all appear in one file's
+// own chain) and a REAL PlanUndo, editing one file's result afterward so
+// the plan also carries a genuinely refused row, then renders that.
+func TestPrintUndoPlan(t *testing.T) {
+	h := home(t)
+	dl := filepath.Join(h, "dl")
+	if err := os.MkdirAll(filepath.Join(dl, "Work"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
+	for _, n := range []string{"inv1.pdf", "notes.pdf"} {
+		p := filepath.Join(dl, n)
+		if err := os.WriteFile(p, []byte("content of "+n), 0o644); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Chtimes(p, old, old); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if code, _, errOut := runCLI(t, "init"); code != 0 {
+		t.Fatal(errOut)
+	}
+	if code, _, errOut := runCLI(t, "new", "dl", dl); code != 0 {
+		t.Fatal(errOut)
+	}
+	// "keep" mixes all three action kinds a single file's own chain can
+	// carry: copy needs a fresh ~/backup (one mkdir), move lands in the
+	// pre-created Work (no mkdir of its own).
+	rules := "(path \"~/dl\")\n(min-age 0s)\n(rule \"keep\" (when (type pdf)) (copy \"~/backup\") (move \"Work\"))\n"
+	if err := os.WriteFile(filepath.Join(h, ".config/krino/dirs/dl.conf"), []byte(rules), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if code, _, errOut := runCLI(t, "-y"); code != 0 {
+		t.Fatalf("apply: %d %s", code, errOut)
+	}
+
+	// notes.pdf's moved copy is edited after the run, so PlanUndo genuinely
+	// refuses its reversal - the row this exercises must still say
+	// something true, never render blank.
+	moved := filepath.Join(dl, "Work", "notes.pdf")
+	if err := os.WriteFile(moved, []byte("edited since the run"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	e, errs := engine.Load(filepath.Join(h, ".config", "krino", "krino.conf"))
+	if len(errs) > 0 {
+		t.Fatal(errs)
+	}
+	runs, err := e.Runs(1)
+	if err != nil || len(runs) != 1 {
+		t.Fatalf("runs = %+v, err = %v", runs, err)
+	}
+	up, err := e.PlanUndo(runs[0].ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var buf bytes.Buffer
+	printUndoPlan(&buf, up)
+	out := buf.String()
+
+	for _, want := range []string{
+		"2 files · 1 to reverse · 1 refused\n",
+		"undo-move      → ~/dl/inv1.pdf\n",
+		"undo-copy      ~/backup/inv1.pdf → trash\n",
+		"undo-mkdir     ~/backup\n",
+		// Minor 4 / fix wave item 5: the refusal reason must be abbreviated
+		// against $HOME exactly like every step cell above it, not printed
+		// as a raw absolute path.
+		"refused: ~/dl/Work/notes.pdf changed since the run\n",
+	} {
+		if !strings.Contains(out, want) {
+			t.Errorf("output lacks %q:\n%s", want, out)
+		}
+	}
+	if strings.Contains(out, "undo-copy      → ") {
+		t.Errorf("undo-copy rendered a blank destination:\n%s", out)
+	}
+	if strings.Contains(out, h) {
+		t.Errorf("output leaked a raw absolute path instead of abbreviating against $HOME:\n%s", out)
+	}
+}
+
+// TestReviewUndoInvalidKeyReprompts mirrors review_test.go's
+// TestInvalidKeyReprompts for the undo-specific menu.
+func TestReviewUndoInvalidKeyReprompts(t *testing.T) {
+	out := new(strings.Builder)
+	approved, action, err := reviewUndoFiles(strings.NewReader("zs"), out, undoFiles("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)
+	}
+}