plan 10: a step that ran but could not be logged is named in the error

2 files changed, 66 insertions, 1 deletions

diff --git a/internal/engine/apply.go b/internal/engine/apply.go
index 09f3aab..cc87f30 100644
--- a/internal/engine/apply.go
+++ b/internal/engine/apply.go
@@ -113,7 +113,10 @@ func (e *Engine) applyFile(dirName string, c plan.Chain, approved bool, j *journ
 	// Each step is logged the moment it has run (review M9), not after the
 	// whole chain: a run killed mid-chain must leave what it did undoable.
 	results, err := apply.ChainLogged(c, func(i int, sr apply.StepResult) error {
-		return e.logStep(j, run, dirName, rel, i+1, c.Steps[i], sr)
+		if err := e.logStep(j, run, dirName, rel, i+1, c.Steps[i], sr); err != nil {
+			return unloggedStep(rel, c.Steps[i], sr, err)
+		}
+		return nil
 	})
 	if err != nil {
 		return FileResult{}, err
@@ -121,6 +124,25 @@ func (e *Engine) applyFile(dirName string, c plan.Chain, approved bool, j *journ
 	return FileResult{File: c.File, Steps: results}, nil
 }
 
+// unloggedStep is the error for a step whose log entry could not be written
+// (re-review N1). A step that ran is named with where its file is now:
+// undo cannot see it, so the user must be told where to look.
+func unloggedStep(rel string, step plan.Step, sr apply.StepResult, err error) error {
+	if sr.Status != "ok" {
+		return fmt.Errorf("%s: step %s (%s) could not be logged: %w", rel, actionName(step.Kind), sr.Status, err)
+	}
+	var where string
+	switch {
+	case step.Kind == plan.Copy:
+		where = "a copy is at " + xdg.Abbrev(sr.Dst)
+	case sr.Dst != "":
+		where = "the file is now at " + xdg.Abbrev(sr.Dst)
+	default:
+		where = "the file is deleted for good"
+	}
+	return fmt.Errorf("%s: %s ran but could not be logged, so undo cannot see it (%s): %w", rel, actionName(step.Kind), where, err)
+}
+
 // tallyFile updates result's Applied/Failed/Declined counters from one
 // file's step outcomes. The three are not mutually exclusive: a chain that
 // ran one step ok and then failed on the next counts toward both Applied
diff --git a/internal/engine/undo_identity_test.go b/internal/engine/undo_identity_test.go
index d4169be..23e0bf9 100644
--- a/internal/engine/undo_identity_test.go
+++ b/internal/engine/undo_identity_test.go
@@ -274,3 +274,46 @@ func TestUndoCanBeFinishedAfterAFailure(t *testing.T) {
 		t.Errorf("dl/a.pdf = %q, %v; want the original back", b, err)
 	}
 }
+
+// TestApplyReportsAStepThatCouldNotBeLogged: when the log stops accepting
+// writes mid-chain, the step that already ran is named in the error - file,
+// action and where the file is now - so the user can find what undo cannot
+// see (re-review N1).
+func TestApplyReportsAStepThatCouldNotBeLogged(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)
+	}
+	calls := 0
+	e.Now = func() time.Time {
+		calls++
+		if calls == 3 { // run-start and the rename are logged; the move is not
+			j.Close()
+		}
+		return time.Now()
+	}
+	_, err = e.Apply(context.Background(), dp, map[string]bool{"a.pdf": true}, j, "R")
+	if err == nil {
+		t.Fatal("apply succeeded with a closed log")
+	}
+	for _, want := range []string{"a.pdf", "move", "could not be logged", "Out/r-a.pdf"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("error %q does not mention %q", err, want)
+		}
+	}
+}