3 files changed, 77 insertions, 0 deletions

diff --git a/gui/internal/model/history.go b/gui/internal/model/history.go
index 7c166bb..87ab6e7 100644
--- a/gui/internal/model/history.go
+++ b/gui/internal/model/history.go
@@ -114,6 +114,10 @@ type UndoTab struct {
 	sess  *engine.Session
 	up    *engine.UndoPlan
 	locks []*lock.Lock
+
+	// applying is set while the reversal is in flight; Close refuses then,
+	// for the reason PlanTab.applying gives.
+	applying bool
 }
 
 // PlanUndo builds the reversal of runID and locks every directory it
@@ -208,6 +212,8 @@ func (t *UndoTab) SelectedCount() int {
 // said no to (spec §9). Refused files ride along unchanged, as they do on
 // the command line. The locks are released afterwards: the plan is history.
 func (t *UndoTab) Apply(ctx context.Context) (*engine.ApplyResult, error) {
+	t.applying = true
+	defer func() { t.applying = false }()
 	toApply := &engine.UndoPlan{Run: t.up.Run, Cleanup: t.up.Cleanup}
 	for i, f := range t.up.Files {
 		if f.Refused == "" && !t.Rows[i].Selected {
@@ -266,6 +272,9 @@ func undoOutcome(row UndoRow, fr engine.FileResult) string {
 // does. One lock's failure never stops the rest from being released, or the
 // session from being closed. Closing twice is not an error.
 func (t *UndoTab) Close() error {
+	if t.applying {
+		return ErrApplying
+	}
 	var first error
 	for _, l := range t.locks {
 		if err := l.Release(); err != nil && first == nil {
diff --git a/gui/internal/model/plan.go b/gui/internal/model/plan.go
index ccbf17b..c3e9654 100644
--- a/gui/internal/model/plan.go
+++ b/gui/internal/model/plan.go
@@ -7,6 +7,7 @@ package model
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sort"
 	"time"
@@ -50,8 +51,24 @@ type PlanTab struct {
 	sess *engine.Session
 	dp   *engine.DirPlan
 	lock *lock.Lock
+
+	// applying is set while Apply is in flight. Close must refuse then:
+	// releasing the lock and closing the log under a running apply moves
+	// files the log never records, so undo cannot see them, and unlocks a
+	// directory krino is still working in. The window can reach Close from
+	// several places while an apply runs - saving rules or settings, adding
+	// a directory, closing the window - so the refusal lives here rather
+	// than in whichever of them remembers.
+	applying bool
+
+	// testBeforeApply, when set, is called just before the engine's Apply
+	// begins, so a test can act while the apply is in flight.
+	testBeforeApply func()
 }
 
+// ErrApplying is returned by Close while an apply is still running.
+var ErrApplying = errors.New("krino is still applying this plan")
+
 // Counts is the plan's summary line.
 type Counts struct {
 	Scanned, Acting, Excluded, Skipped, Unmatched, Warned int
@@ -95,6 +112,9 @@ func (t *PlanTab) Run() string { return t.sess.Run() }
 // Close releases the directory's lock and ends the run, which Apply also
 // does once the plan is history. Closing twice is not an error.
 func (t *PlanTab) Close() error {
+	if t.applying {
+		return ErrApplying
+	}
 	if t.lock == nil {
 		return nil
 	}
@@ -303,7 +323,12 @@ func (t *PlanTab) Apply(ctx context.Context) (*engine.ApplyResult, error) {
 			approved[r.Rel] = true
 		}
 	}
+	t.applying = true
+	if t.testBeforeApply != nil {
+		t.testBeforeApply()
+	}
 	res, err := t.sess.Apply(ctx, t.dp, approved)
+	t.applying = false
 	t.Applied = true
 	// An applied plan is history: the run is over and the directory free
 	// again, without closing the window (GUI design §3).
diff --git a/gui/internal/model/plan_test.go b/gui/internal/model/plan_test.go
index a36cf85..1bae2c5 100644
--- a/gui/internal/model/plan_test.go
+++ b/gui/internal/model/plan_test.go
@@ -462,3 +462,46 @@ func TestKeepThisCopyRefusesANonDuplicate(t *testing.T) {
 		t.Error("a row that does not exist was accepted")
 	}
 }
+
+// TestCloseDuringApplyIsRefused: Close releases the directory lock and
+// closes the log. Called while an apply is running - which the window
+// allows: saving rules, saving settings, adding a directory and closing the
+// window all reach it - the engine goes on moving files with the log shut
+// under it, so a file is moved that no krino undo can see, and the
+// directory is unlocked while krino is still working in it.
+func TestCloseDuringApplyIsRefused(t *testing.T) {
+	conf := "(path \"~/dl\")\n(rule \"pdfs\" (when (type pdf)) (move \"Docs\"))\n"
+	e, _ := sandboxDir(t, conf, map[string]string{"a.pdf": "one", "b.pdf": "two"})
+	tab, err := Plan(context.Background(), e, e.Dirs[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	for i := range tab.Rows {
+		tab.Rows[i].Selected = true
+	}
+
+	started := make(chan struct{})
+	done := make(chan struct{})
+	tab.testBeforeApply = func() {
+		close(started)
+		// Hold the apply open while Close is attempted.
+		<-done
+	}
+	var applyErr error
+	go func() {
+		_, applyErr = tab.Apply(context.Background())
+	}()
+	<-started
+
+	if err := tab.Close(); err == nil {
+		t.Error("Close during an apply was allowed: the lock and the log go out from under it")
+	}
+	close(done)
+	// Let the apply finish before the sandbox is torn down.
+	for i := 0; i < 200 && !tab.Applied; i++ {
+		time.Sleep(10 * time.Millisecond)
+	}
+	if applyErr != nil {
+		t.Errorf("the apply itself failed: %v", applyErr)
+	}
+}