the engine owns a run: lock, log, run id, claims

3 files changed, 285 insertions, 73 deletions

diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go
index aeeb1d4..4c0a0d8 100644
--- a/cmd/krino/sort.go
+++ b/cmd/krino/sort.go
@@ -18,8 +18,6 @@ import (
 	"golang.org/x/term"
 
 	"krino/internal/engine"
-	"krino/internal/journal"
-	"krino/internal/lock"
 	"krino/internal/plan"
 	"krino/internal/scan"
 	"krino/internal/xdg"
@@ -50,8 +48,8 @@ const zeroOutcome = "0 applied · 0 failed · 0 declined"
 
 // cmdSort plans and, from Task 7, applies the included directories: flags
 // are checked before any config is read, a bad config stops the whole run
-// before scanning (spec §11), and one journal.Writer, run id and
-// plan.Claims cover every directory in the run. See docs/design.md
+// before scanning (spec §11), and one engine.Session - its log, run id and
+// claims - covers every directory in the run. See docs/design.md
 // §8.2-§8.4 and §11 for the flow this follows.
 func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 	if g.yes && g.dry {
@@ -96,39 +94,25 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 	stopSignals := installSignalHandler(cancel)
 	defer stopSignals()
 
-	// Ruling 2: journal.Open creates $XDG_STATE_HOME/krino/ and an empty
-	// krino.log as a side effect of merely being called, so a dry run must
-	// never call it at all - not open it and clean up afterwards. -n is
-	// known from the flags before the loop starts, so the gate is exactly
-	// that, nothing per-directory. One Writer and one run id cover every
-	// directory in the run (Task 5's undo depends on a single run id
-	// spanning all of them).
-	var j *journal.Writer
-	var run string
-	if !g.dry {
-		var err error
-		j, err = journal.Open(e.Config.LogFile())
-		if err != nil {
-			fmt.Fprintf(stderr, "krino: %v\n", err)
-			return 1
-		}
-		defer func() {
-			if cerr := j.Close(); cerr != nil {
-				fmt.Fprintf(stderr, "krino: %v\n", cerr)
-			}
-		}()
-		run = journal.NewRunID(e.Now())
+	// The run itself - the log, its run id and the claims every directory
+	// shares - belongs to the engine, so the GUI runs a directory exactly
+	// as this does (GUI design §1.3). Ruling 2: a dry session opens no log,
+	// since journal.Open creates the state directory and an empty krino.log
+	// merely by being called.
+	sess, err := e.NewSession(g.dry)
+	if err != nil {
+		fmt.Fprintf(stderr, "krino: %v\n", err)
+		return 1
 	}
+	defer func() {
+		if cerr := sess.Close(); cerr != nil {
+			fmt.Fprintf(stderr, "krino: %v\n", cerr)
+		}
+	}()
 
 	exit := 0
 	printed := false
 	jsonDirs := []plan.JSONDir{} // never nil: the document's "dirs" must marshal as [], not null
-	// A3: in a dry run one Claims is shared across every directory's Plan
-	// call below, so two directories that both plan a move to the same
-	// destination resolve the collision at planning time instead of each
-	// independently believing it owns that path. A real run applies each
-	// directory before planning the next and starts fresh claims after it.
-	claims := plan.NewClaims()
 
 	for _, d := range e.Dirs {
 		// Spec §3/§11: a second krino on the same directory waits for the
@@ -137,7 +121,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 		// is not unbounded in practice (fix round 2026-09-12/item 1): a
 		// signal cancels it and Acquire returns ctx.Err() promptly instead
 		// of polling forever.
-		l, err := lock.Acquire(ctx, e.Config.LockFile(d.Name), !g.yes)
+		l, err := sess.Lock(ctx, d, !g.yes)
 		if err != nil {
 			if interrupted(err) {
 				// Interrupted while waiting for the lock: an interrupt, not
@@ -151,7 +135,6 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 			continue
 		}
 
-		var applied []string // where this directory's applied steps put files
 		quit := func() bool {
 			defer func() {
 				if rerr := l.Release(); rerr != nil {
@@ -164,7 +147,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 				exit = 1
 				return false
 			}
-			dp, err := e.Plan(ctx, d, claims)
+			dp, err := sess.Plan(ctx, d)
 			if err != nil {
 				fmt.Fprintf(stderr, "krino: skipping %s: %v\n", d.Name, err)
 				exit = 1
@@ -262,30 +245,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 				toApply = &reviewed
 				notReviewed = len(actionable) - len(reviewedChains(actionable, approved))
 			}
-			res, aerr := e.Apply(ctx, toApply, approved, j, run)
-			if res != nil {
-				// Where files ended up: each copy, and the last place a
-				// move or rename put the file - not a path it passed
-				// through and left.
-				for _, fr := range res.Files {
-					final := ""
-					for _, sr := range fr.Steps {
-						switch {
-						case sr.Status != "ok":
-						case sr.Step.Kind == plan.DeletePermanent:
-							final = "" // gone: nothing is left anywhere
-						case sr.Dst == "":
-						case sr.Step.Kind == plan.Copy:
-							applied = append(applied, sr.Dst)
-						default:
-							final = sr.Dst
-						}
-					}
-					if final != "" {
-						applied = append(applied, final)
-					}
-				}
-			}
+			res, aerr := sess.Apply(ctx, toApply, approved)
 			if aerr != nil {
 				// Interrupted mid-apply (fix round 2026-09-12/item 2):
 				// treated exactly like the cancelled lock wait above - not
@@ -310,20 +270,6 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int {
 		if quit {
 			break
 		}
-		if !g.dry {
-			// This directory is applied (or skipped) now, so the disk is the
-			// truth for the next one: its claims - sources it moved away,
-			// destinations it planned but declined or failed - must not
-			// block a later directory (triage 28i). What it did put
-			// somewhere stays claimed, so a later (on-conflict overwrite)
-			// takes a free name rather than trash this run's own result
-			// (plan 11 review M1). A dry run applies nothing, so there every
-			// claim carries over.
-			claims = plan.NewClaims()
-			for _, p := range applied {
-				claims.Claim(p)
-			}
-		}
 	}
 
 	if g.json {
diff --git a/internal/engine/session.go b/internal/engine/session.go
new file mode 100644
index 0000000..8d52027
--- /dev/null
+++ b/internal/engine/session.go
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package engine
+
+import (
+	"context"
+	"fmt"
+
+	"krino/internal/journal"
+	"krino/internal/lock"
+	"krino/internal/plan"
+)
+
+// Session is one run of krino over one or more directories: the log it
+// writes, the run id every entry carries, and the claims its directories
+// share. Both the command line and the GUI go through it, so the order a
+// directory is locked, planned, applied and released in - and what a later
+// directory may still claim - is written once (GUI design §1.3).
+//
+// A dry session opens no log and takes no run id: journal.Open creates the
+// state directory and an empty krino.log merely by being called, and a dry
+// run must not (spec §11).
+type Session struct {
+	e      *Engine
+	j      *journal.Writer
+	run    string
+	claims *plan.Claims
+	dry    bool
+}
+
+// NewSession starts a run. A real one opens the log; the caller closes the
+// session when the run is over.
+func (e *Engine) NewSession(dry bool) (*Session, error) {
+	s := &Session{e: e, claims: plan.NewClaims(), dry: dry}
+	if dry {
+		return s, nil
+	}
+	j, err := journal.Open(e.Config.LogFile())
+	if err != nil {
+		return nil, err
+	}
+	s.j, s.run = j, journal.NewRunID(e.Now())
+	return s, nil
+}
+
+// Run is the run id every entry of this session carries; "" for a dry one.
+func (s *Session) Run() string { return s.run }
+
+// Journal is the log this session writes, nil for a dry one.
+func (s *Session) Journal() *journal.Writer { return s.j }
+
+// Lock takes d's lock (spec §3, §11). wait blocks until the holder is gone,
+// or until ctx is cancelled; without it a held lock returns lock.ErrHeld at
+// once, so a cron job never piles up behind a stuck run. The caller
+// releases it.
+func (s *Session) Lock(ctx context.Context, d *Dir, wait bool) (*lock.Lock, error) {
+	return lock.Acquire(ctx, s.e.Config.LockFile(d.Name), wait)
+}
+
+// LockDirs takes the locks of several directories, in the order given, and
+// releases every one it took if any of them cannot be had - so an undo
+// spanning directories never holds half of them (spec §10).
+func (s *Session) LockDirs(ctx context.Context, names []string, wait bool) ([]*lock.Lock, error) {
+	var held []*lock.Lock
+	for _, name := range names {
+		l, err := lock.Acquire(ctx, s.e.Config.LockFile(name), wait)
+		if err != nil {
+			for _, h := range held {
+				h.Release()
+			}
+			return nil, fmt.Errorf("%s: %w", name, err)
+		}
+		held = append(held, l)
+	}
+	return held, nil
+}
+
+// Plan builds d's plan with the run's claims.
+func (s *Session) Plan(ctx context.Context, d *Dir) (*DirPlan, error) {
+	return s.e.Plan(ctx, d, s.claims)
+}
+
+// Apply carries out the approved files of dp and logs the run's steps. A
+// real run applies each directory before the next is planned, so afterwards
+// the disk is the truth for the next one: only the paths this directory's
+// files ended up at stay claimed, which keeps a later (on-conflict
+// overwrite) from displacing this run's own result (spec §7.4). A dry
+// session keeps every claim, since it applies nothing.
+func (s *Session) Apply(ctx context.Context, dp *DirPlan, approved map[string]bool) (*ApplyResult, error) {
+	res, err := s.e.Apply(ctx, dp, approved, s.j, s.run)
+	if !s.dry {
+		s.claims = plan.NewClaims()
+		for _, p := range landedAt(res) {
+			s.claims.Claim(p)
+		}
+	}
+	return res, err
+}
+
+// landedAt is where res's files ended up: each copy, and the last place a
+// move or rename put a file - not a path it passed through and left, and
+// nothing at all for a file deleted for good.
+func landedAt(res *ApplyResult) []string {
+	if res == nil {
+		return nil
+	}
+	var out []string
+	for _, fr := range res.Files {
+		final := ""
+		for _, sr := range fr.Steps {
+			switch {
+			case sr.Status != "ok":
+			case sr.Step.Kind == plan.DeletePermanent:
+				final = ""
+			case sr.Dst == "":
+			case sr.Step.Kind == plan.Copy:
+				out = append(out, sr.Dst)
+			default:
+				final = sr.Dst
+			}
+		}
+		if final != "" {
+			out = append(out, final)
+		}
+	}
+	return out
+}
+
+// PlanUndo builds the reversal of runID (spec §10).
+func (s *Session) PlanUndo(runID string) (*UndoPlan, error) {
+	return s.e.PlanUndo(runID)
+}
+
+// ApplyUndo carries out up and logs it under this session's run id.
+func (s *Session) ApplyUndo(ctx context.Context, up *UndoPlan) (*ApplyResult, error) {
+	return s.e.ApplyUndo(ctx, up, s.j, s.run)
+}
+
+// Close closes the log. A dry session has nothing to close.
+func (s *Session) Close() error {
+	if s.j == nil {
+		return nil
+	}
+	err := s.j.Close()
+	s.j = nil
+	return err
+}
diff --git a/internal/engine/session_test.go b/internal/engine/session_test.go
new file mode 100644
index 0000000..ece69dd
--- /dev/null
+++ b/internal/engine/session_test.go
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package engine
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+// twoOverwritingDirs builds a and b, each holding x.pdf, both moving it to
+// ~/Out with (on-conflict overwrite).
+func twoOverwritingDirs(t *testing.T, h string) *Engine {
+	t.Helper()
+	old := time.Now().Add(-2 * time.Hour)
+	dirs := map[string]string{}
+	for _, n := range []string{"a", "b"} {
+		p := filepath.Join(h, n, "x.pdf")
+		if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(p, []byte("from "+n), 0o644); err != nil {
+			t.Fatal(err)
+		}
+		os.Chtimes(p, old, old)
+		dirs[n] = "(path \"~/" + n + "\")\n(on-conflict overwrite)\n(rule \"out\" (move \"~/Out\"))\n"
+	}
+	main := writeConfig(t, h, `(include "a" "b")`, dirs)
+	e, errs := Load(main)
+	if len(errs) > 0 {
+		t.Fatal(errs)
+	}
+	return e
+}
+
+// TestSessionKeepsOnlyAppliedDestinationsClaimed: one session covers a whole
+// run - its log, run id and claims - and after applying a directory only the
+// paths its files ended up at stay claimed, so a later directory's overwrite
+// takes a free name instead of trashing this run's own result (spec §7.4).
+func TestSessionKeepsOnlyAppliedDestinationsClaimed(t *testing.T) {
+	h := sandbox(t)
+	e := twoOverwritingDirs(t, h)
+	s, err := e.NewSession(false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer s.Close()
+	if s.Run() == "" {
+		t.Error("a real session has no run id")
+	}
+	for _, d := range e.Dirs {
+		dp, err := s.Plan(context.Background(), d)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if _, err := s.Apply(context.Background(), dp, map[string]bool{"x.pdf": true}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	for rel, want := range map[string]string{"Out/x.pdf": "from a", "Out/x_1.pdf": "from b"} {
+		if b, err := os.ReadFile(filepath.Join(h, rel)); err != nil || string(b) != want {
+			t.Errorf("%s: %q, %v; want %q", rel, b, err, want)
+		}
+	}
+	if entries, _ := os.ReadDir(filepath.Join(h, ".local", "share", "Trash", "files")); len(entries) != 0 {
+		t.Errorf("the Trash holds %d entries; nothing should have been displaced", len(entries))
+	}
+}
+
+// TestDrySessionWritesNoLog: a dry session opens no log, so a dry run never
+// creates the state directory's krino.log (spec §11).
+func TestDrySessionWritesNoLog(t *testing.T) {
+	h := sandbox(t)
+	e := twoOverwritingDirs(t, h)
+	s, err := e.NewSession(true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer s.Close()
+	if s.Run() != "" {
+		t.Errorf("a dry session took a run id: %q", s.Run())
+	}
+	if _, err := s.Plan(context.Background(), e.Dirs[0]); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := os.Stat(e.Config.LogFile()); !os.IsNotExist(err) {
+		t.Errorf("a dry session touched the log: %v", err)
+	}
+}
+
+// TestSessionLockIsTheDirectorysOwn: the session takes the same lock a
+// second krino waits for, and releasing it lets the next one in.
+func TestSessionLockIsTheDirectorysOwn(t *testing.T) {
+	h := sandbox(t)
+	e := twoOverwritingDirs(t, h)
+	s, err := e.NewSession(true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer s.Close()
+	l, err := s.Lock(context.Background(), e.Dirs[0], false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.Lock(context.Background(), e.Dirs[0], false); err == nil {
+		t.Error("the directory was locked twice")
+	}
+	if err := l.Release(); err != nil {
+		t.Fatal(err)
+	}
+	l2, err := s.Lock(context.Background(), e.Dirs[0], false)
+	if err != nil {
+		t.Errorf("the lock was not released: %v", err)
+	} else {
+		l2.Release()
+	}
+}