plan 10: an interrupt stops between steps; nohup keeps ignoring hangups

6 files changed, 64 insertions, 11 deletions

diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go
index 901b19e..52875ed 100644
--- a/cmd/krino/sort.go
+++ b/cmd/krino/sort.go
@@ -359,7 +359,13 @@ func installSignalHandler(cancel context.CancelFunc) func() {
 	}
 
 	sig := make(chan os.Signal, 1)
-	signal.Notify(sig, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP)
+	signal.Notify(sig, os.Interrupt, syscall.SIGTERM)
+	// A hangup stops krino like SIGTERM - unless it was started ignoring
+	// hangups (nohup), which must go on meaning "keep running" (re-review
+	// pa F3): asking for SIGHUP would re-enable it.
+	if !signal.Ignored(syscall.SIGHUP) {
+		signal.Notify(sig, syscall.SIGHUP)
+	}
 	done := make(chan struct{})
 	go func() {
 		signals := 0
diff --git a/docs/design.md b/docs/design.md
index 99e5b55..07728fc 100644
--- a/docs/design.md
+++ b/docs/design.md
@@ -701,8 +701,11 @@ Exit status: 0 success, including nothing to do and everything declined;
 1 one or more steps failed; 2 usage or config error; 130 interrupted.
 
 A config error anywhere stops the whole run before any scanning: krino never
-acts on a config it only partly understood. Ctrl-C during apply finishes the
-current step, logs it, and stops. A second krino on the same directory waits
+acts on a config it only partly understood. Ctrl-C (SIGTERM, or SIGHUP unless
+started under `nohup`) during apply finishes the current step, logs it, skips
+the rest of that file's chain, and stops. A second interrupt exits at once:
+the step in flight is not logged, and a copy in progress can leave a
+temporary `.krino-*` file and an empty directory behind. A second krino on the same directory waits
 for the lock, or fails immediately with `-y` (so a cron job never piles up).
 
 ## 12. Architecture
diff --git a/internal/apply/apply.go b/internal/apply/apply.go
index 684641e..a93aad4 100644
--- a/internal/apply/apply.go
+++ b/internal/apply/apply.go
@@ -7,6 +7,7 @@
 package apply
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"os"
@@ -44,7 +45,7 @@ type StepResult struct {
 // marking the rest skipped. It never touches a file whose size or mtime no
 // longer matches what the plan recorded. It is ChainLogged with no done.
 func Chain(c plan.Chain) []StepResult {
-	results, _ := ChainLogged(c, nil)
+	results, _ := ChainLogged(context.Background(), c, nil)
 	return results
 }
 
@@ -58,11 +59,18 @@ func Chain(c plan.Chain) []StepResult {
 // planned destination was taken since planning, stops the chain too: every
 // later step was planned against the name the file did not get, and must
 // not act on whatever is at that path (review M3).
-func ChainLogged(c plan.Chain, done func(i int, sr StepResult) error) ([]StepResult, error) {
+//
+// Once ctx is cancelled (an interrupt), the step already under way finishes
+// and every later step is skipped as "interrupted": an interrupt stops after
+// the current step, not after the file's whole chain (spec §11).
+func ChainLogged(ctx context.Context, c plan.Chain, done func(i int, sr StepResult) error) ([]StepResult, error) {
 	results := make([]StepResult, len(c.Steps))
 	stopWhy := ""
 
 	for i, step := range c.Steps {
+		if stopWhy == "" && ctx.Err() != nil {
+			stopWhy = "interrupted"
+		}
 		switch {
 		case stopWhy != "":
 			results[i] = StepResult{Step: step, Status: "skipped", Detail: stopWhy}
diff --git a/internal/apply/swap_test.go b/internal/apply/swap_test.go
index c185686..ccac945 100644
--- a/internal/apply/swap_test.go
+++ b/internal/apply/swap_test.go
@@ -3,6 +3,7 @@
 package apply
 
 import (
+	"context"
 	"os"
 	"path/filepath"
 	"strings"
@@ -144,7 +145,7 @@ func TestChainLoggedReportsEachStepBeforeTheNext(t *testing.T) {
 		plan.Step{Kind: plan.Rename, Src: moved, Dst: renamed},
 	)
 	var calls []int
-	res, err := ChainLogged(c, func(i int, sr StepResult) error {
+	res, err := ChainLogged(context.Background(), c, func(i int, sr StepResult) error {
 		calls = append(calls, i)
 		if i == 0 {
 			if _, err := os.Lstat(moved); err != nil {
@@ -185,3 +186,34 @@ func TestChainRefusesToDisplaceANonRegularTarget(t *testing.T) {
 		t.Errorf("the directory was displaced: %v", err)
 	}
 }
+
+// TestChainLoggedStopsBetweenStepsWhenInterrupted: once the context is
+// cancelled, the chain finishes the step it is on and skips the rest, so an
+// interrupt stops after the current step, not after the file's whole chain
+// (re-review pa F7).
+func TestChainLoggedStopsBetweenStepsWhenInterrupted(t *testing.T) {
+	root := t.TempDir()
+	p := filepath.Join(root, "a.pdf")
+	moved := filepath.Join(root, "W", "a.pdf")
+	c := planned(t, root, "a.pdf", "body",
+		plan.Step{Kind: plan.Move, Src: p, Dst: moved},
+		plan.Step{Kind: plan.Rename, Src: moved, Dst: filepath.Join(root, "W", "b.pdf")},
+	)
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	res, err := ChainLogged(ctx, c, func(i int, sr StepResult) error {
+		if i == 0 {
+			cancel()
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if res[0].Status != "ok" || res[1].Status != "skipped" || res[1].Detail != "interrupted" {
+		t.Errorf("steps = %s, %s %q; want ok, then skipped as interrupted", res[0].Status, res[1].Status, res[1].Detail)
+	}
+	if _, err := os.Lstat(moved); err != nil {
+		t.Errorf("the finished step was undone or never ran: %v", err)
+	}
+}
diff --git a/internal/engine/apply.go b/internal/engine/apply.go
index 75248fb..f9cb9ef 100644
--- a/internal/engine/apply.go
+++ b/internal/engine/apply.go
@@ -80,7 +80,7 @@ func (e *Engine) Apply(ctx context.Context, dp *DirPlan, approved map[string]boo
 		if err := ctx.Err(); err != nil {
 			return result, err
 		}
-		fr, err := e.applyFile(dp.Dir.Name, c, approved[c.File.Rel], j, run)
+		fr, err := e.applyFile(ctx, dp.Dir.Name, c, approved[c.File.Rel], j, run)
 		if err != nil {
 			return result, fmt.Errorf("engine: apply: %w", err)
 		}
@@ -95,7 +95,7 @@ func (e *Engine) Apply(ctx context.Context, dp *DirPlan, approved map[string]boo
 }
 
 // applyFile carries out (or declines) one file's chain and logs it.
-func (e *Engine) applyFile(dirName string, c plan.Chain, approved bool, j *journal.Writer, run string) (FileResult, error) {
+func (e *Engine) applyFile(ctx context.Context, dirName string, c plan.Chain, approved bool, j *journal.Writer, run string) (FileResult, error) {
 	rel := c.File.Rel
 
 	if !approved {
@@ -112,7 +112,7 @@ 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 {
+	results, err := apply.ChainLogged(ctx, c, func(i int, sr apply.StepResult) error {
 		if err := e.logStep(j, run, dirName, rel, i+1, c.Steps[i], sr); err != nil {
 			return unloggedStep(rel, c.Steps[i], sr, err)
 		}
diff --git a/man/krino.1 b/man/krino.1
index 246184b..e9d41da 100644
--- a/man/krino.1
+++ b/man/krino.1
@@ -431,7 +431,8 @@ found after a directory name is a usage error rather than a guess.
 .It 130
 Interrupted
 .Pq Ic Ctrl-C
-or terminated; the current step, if any, is finished and logged first.
+or terminated; the current step, if any, is finished and logged first, and
+the rest of that file's steps are skipped.
 .El
 .Sh KNOWN LIMITATIONS
 .Ic krino explain
@@ -454,7 +455,10 @@ Wrapping counts characters, not terminal columns: a name in a wide script
 (CJK) or with many combining marks can run past the terminal's width.
 .Pp
 A second interrupt exits at once: every step completed so far is logged
-and can be undone, but the step in flight when it arrived is not logged.
+and can be undone, but the step in flight when it arrived is not logged,
+and a copy in progress can leave a temporary
+.Pa .krino-*
+file and an empty directory behind.
 .Pp
 Undo identifies a file by its directory and its path within it.
 A file one directory's rules move into another included directory, which