aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--cmd/krino/history_test.go2
-rw-r--r--cmd/krino/review_test.go31
-rw-r--r--cmd/krino/sort.go40
-rw-r--r--internal/engine/property_test.go4
-rw-r--r--internal/engine/undo_identity_test.go60
-rw-r--r--internal/plan/fuzz_test.go69
-rw-r--r--internal/plan/testdata/fuzz/FuzzExpand/a8c52dd03776dd2c3
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")
generated by cgit v1.3 (git 2.53.0) at 2026-09-19 16:14:30 +0000