feat(recover): request a graceful exit for an explicitly selected process

cmd/recover_process.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"strconv"
 	"syscall"
@@ -22,6 +23,8 @@ const (
 	processExitReceiptLimit   = 16 << 10
 	processObservationUnknown = "unknown"
 	processObservationAbsent  = "absent"
+	processExitModeGraceful   = "graceful"
+	processExitModeForce      = "force"
 )
 
 // Version 1 is a fixed local native-identity exception, not a registry format.
@@ -31,9 +34,16 @@ type processExitReceipt struct {
 	Process       base.ProcessIdentity `json:"process"`
 }
 
+// processExitResult is version 2: version 1 named no mode, because SIGKILL was
+// the only one. The --preview receipt stays version 1 — its shape did not
+// change, and bumping it would invalidate receipts already saved for review.
 type processExitResult struct {
-	SchemaVersion     int    `json:"schema_version"`
-	PID               int    `json:"pid"`
+	SchemaVersion int `json:"schema_version"`
+	PID           int `json:"pid"`
+	// Mode and Signal name what was REQUESTED, so a reader of the result never
+	// has to infer which of the two irreversibility contracts applied.
+	Mode              string `json:"mode"`
+	Signal            string `json:"signal"`
 	SignalAttempted   bool   `json:"signal_attempted"`
 	SignalAccepted    bool   `json:"signal_accepted"`
 	Observation       string `json:"observation"`
@@ -45,14 +55,19 @@ func init() {
 }
 
 func newRecoverProcessCommand(inspect func(int) (base.ProcessIdentity, error), signal func(context.Context, base.ProcessIdentity, syscall.Signal) error) *cobra.Command {
-	var preview, force bool
+	var preview, graceful, force bool
 	var path string
 	command := &cobra.Command{
 		Use: "process PID", Short: "Preview or explicitly force one independently selected process to exit",
 		Long: `Select one process by PID, independently of workspace or group ownership.
-Save --preview verbatim, review its full identity, then use --expect FILE --force.
-Execution compares the full current identity and requests SIGKILL through Core
-for that process birth only. SIGKILL is irreversible. No graceful signal,
+Save --preview verbatim, review its full identity, then use --expect FILE with
+exactly one of --graceful or --force. Execution compares the full current
+identity and requests ONE signal through Core for that process birth only:
+--graceful requests SIGTERM, --force requests SIGKILL. SIGKILL is irreversible
+and discards whatever the process had not yet written; --graceful lets the
+process run its own shutdown, which is what a store holding authoritative state
+needs. Neither escalates: a --graceful request is never followed by SIGKILL, and
+an unconfirmed exit stays unconfirmed rather than falling back to force. No
 escalation, group operation, registry removal or orphan recovery is performed.
 Executable equality is checked at admission only: a same-birth exec can still
 receive the signal. Children may react to lost connections or parent death;
@@ -62,15 +77,22 @@ Cancellation cannot revoke in-flight delivery. The five-second polling budget
 does not interrupt synchronous inspection or output. An error may follow an
 effect: inspect the disposition before any retry. Use the same reviewed binary
 on the same host/boot; changing implementation requires a fresh preview.`,
-		Example: "  codefly recover process 12345 --preview > process-exit.json\n  codefly recover process 12345 --expect process-exit.json --force",
+		Example: "  codefly recover process 12345 --preview > process-exit.json\n  codefly recover process 12345 --expect process-exit.json --graceful\n  codefly recover process 12345 --expect process-exit.json --force",
 		Args:    cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			pid, parseErr := strconv.Atoi(args[0])
 			if parseErr != nil || pid <= 1 || pid == os.Getpid() || strconv.Itoa(pid) != args[0] {
 				return errors.New("PID must be a canonical decimal integer greater than 1, excluding this CLI")
 			}
-			if preview == (path != "") || (cmd.Flags().Changed("expect") && path == "") || force != !preview {
-				return errors.New("select --preview alone, or --expect FILE with --force")
+			// Exactly one mode, and the receipt belongs to the two that signal.
+			modes := 0
+			for _, selected := range []bool{preview, graceful, force} {
+				if selected {
+					modes++
+				}
+			}
+			if modes != 1 || preview == (path != "") || (cmd.Flags().Changed("expect") && path == "") {
+				return errors.New("select --preview alone, or --expect FILE with exactly one of --graceful or --force")
 			}
 			ctx, done := common.NewContext()
 			defer done()
@@ -103,12 +125,17 @@ on the same host/boot; changing implementation requires a fresh preview.`,
 			if expected.Process.PID != pid {
 				return errors.New("receipt PID does not match the explicitly selected PID")
 			}
-			result, effectErr := forceProcessExit(ctx, expected.Process, inspect, signal)
+			requested, mode := syscall.SIGKILL, processExitModeForce
+			if graceful {
+				requested, mode = syscall.SIGTERM, processExitModeGraceful
+			}
+			result, effectErr := requestProcessExit(ctx, expected.Process, requested, mode, inspect, signal)
 			return errors.Join(effectErr, json.NewEncoder(cmd.OutOrStdout()).Encode(result))
 		},
 	}
 	command.Flags().BoolVar(&preview, "preview", false, "Print the current process identity without signaling")
 	command.Flags().StringVar(&path, "expect", "", "Use the complete verbatim version-1 preview receipt")
+	command.Flags().BoolVar(&graceful, "graceful", false, "Request one SIGTERM for the selected process birth, letting it shut down itself; never escalates to SIGKILL")
 	command.Flags().BoolVar(&force, "force", false, "Explicitly request irreversible SIGKILL for the selected process birth")
 	return command
 }
@@ -158,12 +185,31 @@ func readProcessExitReceipt(ctx context.Context, path string) (processExitReceip
 	return receipt, ctx.Err()
 }
 
+// errExitInProgress marks the one inspection failure that means "this process
+// is on its way out", as opposed to "this inspection went wrong".
+//
+// Core reads /proc/<pid>/stat and then /proc/<pid>/exe. A missing stat it
+// reports as ErrProcessNotFound; a missing exe link with the stat still
+// readable it deliberately refuses to call absence, because a thread-group
+// leader can exit while its threads live on. That same reading is what a
+// process looks like between its own exit and its parent's wait, so the only
+// ENOENT core can hand back is a process in the middle of leaving. Every other
+// inspection failure — a permission error, an unparseable stat, an identity
+// that changed mid-read — carries no ENOENT and stays fatal here.
+//
+// Before the signal, even this is a refusal to act: an unclear reading is no
+// basis for sending anything. While polling for an exit that has already been
+// requested it means "not yet", so the loop keeps waiting for a verdict. If
+// the budget expires without one, the exit is reported unconfirmed — the
+// honest answer, and the behaviour that already existed.
+var errExitInProgress = errors.New("process is exiting but its identity is not yet resolvable")
+
 func sameProcessBirth(first, second base.ProcessIdentity) bool {
 	return first.PID == second.PID && first.BootID == second.BootID && first.StartID == second.StartID
 }
 
-func forceProcessExit(ctx context.Context, expected base.ProcessIdentity, inspect func(int) (base.ProcessIdentity, error), signal func(context.Context, base.ProcessIdentity, syscall.Signal) error) (processExitResult, error) {
-	result := processExitResult{SchemaVersion: 1, PID: expected.PID, Observation: processObservationUnknown}
+func requestProcessExit(ctx context.Context, expected base.ProcessIdentity, requested syscall.Signal, mode string, inspect func(int) (base.ProcessIdentity, error), signal func(context.Context, base.ProcessIdentity, syscall.Signal) error) (processExitResult, error) {
+	result := processExitResult{SchemaVersion: 2, PID: expected.PID, Mode: mode, Signal: requested.String(), Observation: processObservationUnknown}
 	observe := func() (base.ProcessIdentity, error) {
 		result.Observation, result.ExpectedBirthGone = processObservationUnknown, false
 		if err := ctx.Err(); err != nil {
@@ -174,6 +220,8 @@ func forceProcessExit(ctx context.Context, expected base.ProcessIdentity, inspec
 		case errors.Is(err, base.ErrProcessNotFound):
 			result.Observation, result.ExpectedBirthGone = processObservationAbsent, true
 			err = nil
+		case errors.Is(err, fs.ErrNotExist):
+			err = errors.Join(errExitInProgress, err)
 		case err != nil:
 		case current.PID != expected.PID || current.PGID <= 0 || current.BootID == "" || current.StartID == 0 || current.Executable == "":
 			err = errors.New("core returned an incomplete or mismatched process observation")
@@ -196,23 +244,31 @@ func forceProcessExit(ctx context.Context, expected base.ProcessIdentity, inspec
 		return result, err
 	}
 	result.SignalAttempted = true
-	deliveryErr := signal(ctx, expected, syscall.SIGKILL)
+	// Exactly one request, whichever mode: nothing here sends a second signal.
+	deliveryErr := signal(ctx, expected, requested)
 	result.SignalAccepted = deliveryErr == nil
 	// This bounds polling, not synchronous owner inspection or in-flight delivery.
 	deadline := time.NewTimer(5 * time.Second)
 	defer deadline.Stop()
 	ticker := time.NewTicker(100 * time.Millisecond)
 	defer ticker.Stop()
+	var unresolved error
 	for {
 		_, err := observe()
-		if err != nil || result.Observation == processObservationAbsent {
+		switch {
+		case result.Observation == processObservationAbsent:
+			return result, errors.Join(deliveryErr, err)
+		case errors.Is(err, errExitInProgress):
+			// Keep the reason so an expired budget can say what it kept seeing.
+			unresolved = err
+		case err != nil:
 			return result, errors.Join(deliveryErr, err)
 		}
 		select {
 		case <-ctx.Done():
-			return result, errors.Join(deliveryErr, ctx.Err())
+			return result, errors.Join(deliveryErr, unresolved, ctx.Err())
 		case <-deadline.C:
-			return result, errors.Join(deliveryErr, errors.New("process exit unconfirmed within polling budget"))
+			return result, errors.Join(deliveryErr, unresolved, errors.New("process exit unconfirmed within polling budget"))
 		case <-ticker.C:
 		}
 	}

cmd/recover_process_integration_test.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"os"
 	"os/exec"
+	"os/signal"
 	"path/filepath"
 	"strconv"
 	"syscall"
@@ -101,7 +102,7 @@ func TestProcessExitRealTargetAndSameGroupSibling(t *testing.T) {
 	// delivery race, while this tests the CLI admission boundary with a live PID.
 	stale := siblingIdentity
 	stale.StartID++
-	result, err = forceProcessExit(t.Context(), stale, base.InspectProcess, base.SignalProcess)
+	result, err = requestProcessExit(t.Context(), stale, syscall.SIGKILL, "force", base.InspectProcess, base.SignalProcess)
 	require.Error(t, err)
 	require.False(t, result.SignalAttempted)
 	require.Equal(t, "changed_birth", result.Observation)
@@ -142,7 +143,7 @@ func TestProcessExitCanTriggerIndirectChildReaction(t *testing.T) {
 	parent.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
 	identity, done := startProcessExitChild(t, parent)
 	require.Eventually(t, func() bool { _, err := os.Stat(ready); return err == nil }, 5*time.Second, 10*time.Millisecond)
-	result, err := forceProcessExit(t.Context(), identity, base.InspectProcess, base.SignalProcess)
+	result, err := requestProcessExit(t.Context(), identity, syscall.SIGKILL, "force", base.InspectProcess, base.SignalProcess)
 	assertProcessExitRealReadback(t, result, err)
 	select {
 	case err := <-done:
@@ -156,3 +157,93 @@ func TestProcessExitCanTriggerIndirectChildReaction(t *testing.T) {
 	// This is intentional contrary evidence: one-PID delivery does not prove
 	// zero child effects. It qualifies no historical agent cleanup implementation.
 }
+
+// TestProcessExitGracefulHelper is the real SIGTERM target. It installs a
+// handler that writes a durable marker and only then exits, which is the whole
+// point of --graceful: a store holding authoritative state must get to finish
+// writing. "ignore" instead refuses to die, so the timeout can be observed.
+func TestProcessExitGracefulHelper(t *testing.T) {
+	switch os.Getenv("CLI934_PROCESS_EXIT_GRACEFUL") {
+	case "handler":
+		signals := make(chan os.Signal, 1)
+		signal.Notify(signals, syscall.SIGTERM)
+		// Readiness AFTER the handler is installed: a SIGTERM delivered before
+		// that point takes the default disposition and the target dies without
+		// running its shutdown, which would make this test prove nothing.
+		require.NoError(t, os.WriteFile(os.Getenv("CLI934_PROCESS_EXIT_READY"), []byte("ready"), 0o600))
+		<-signals
+		require.NoError(t, os.WriteFile(os.Getenv("CLI934_PROCESS_EXIT_MARKER"), []byte("shutdown written before exit\n"), 0o600))
+		os.Exit(0)
+	case "ignore":
+		signal.Ignore(syscall.SIGTERM)
+		require.NoError(t, os.WriteFile(os.Getenv("CLI934_PROCESS_EXIT_READY"), []byte("ready"), 0o600))
+		for {
+			time.Sleep(time.Second)
+		}
+	}
+}
+
+// TestProcessExitGracefulLetsTheTargetWriteBeforeExit is the positive: one
+// SIGTERM, the target's own shutdown runs to completion, and the result names
+// the mode and signal that were requested.
+func TestProcessExitGracefulLetsTheTargetWriteBeforeExit(t *testing.T) {
+	dir := t.TempDir()
+	marker, ready := filepath.Join(dir, "marker"), filepath.Join(dir, "ready")
+	target := exec.Command(os.Args[0], "-test.run=^TestProcessExitGracefulHelper$")
+	target.Env = append(os.Environ(), "CLI934_PROCESS_EXIT_GRACEFUL=handler",
+		"CLI934_PROCESS_EXIT_MARKER="+marker, "CLI934_PROCESS_EXIT_READY="+ready)
+	target.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	identity, done := startProcessExitChild(t, target)
+	require.Eventually(t, func() bool { _, err := os.Stat(ready); return err == nil }, 10*time.Second, 10*time.Millisecond,
+		"the target must have installed its SIGTERM handler first")
+
+	result, err := requestProcessExit(t.Context(), identity, syscall.SIGTERM, "graceful", base.InspectProcess, base.SignalProcess)
+	require.NoError(t, err)
+	require.True(t, result.SignalAttempted)
+	require.True(t, result.SignalAccepted)
+	require.Equal(t, "absent", result.Observation)
+	require.True(t, result.ExpectedBirthGone)
+	require.Equal(t, "graceful", result.Mode)
+	require.Equal(t, syscall.SIGTERM.String(), result.Signal)
+
+	select {
+	case <-done:
+	case <-time.After(5 * time.Second):
+		t.Fatal("graceful target never exited")
+	}
+	body, readErr := os.ReadFile(marker)
+	require.NoError(t, readErr, "the target's own shutdown must have run")
+	require.Contains(t, string(body), "shutdown written before exit")
+}
+
+// TestProcessExitGracefulNeverEscalates is the protection the disposition is
+// really about: a target that ignores SIGTERM stays ALIVE when the polling
+// budget expires. The command reports the exit unconfirmed and sends nothing
+// else — no SIGKILL fallback — so a store that did not finish writing is never
+// taken down by this command.
+func TestProcessExitGracefulNeverEscalates(t *testing.T) {
+	ready := filepath.Join(t.TempDir(), "ready")
+	target := exec.Command(os.Args[0], "-test.run=^TestProcessExitGracefulHelper$")
+	target.Env = append(os.Environ(), "CLI934_PROCESS_EXIT_GRACEFUL=ignore", "CLI934_PROCESS_EXIT_READY="+ready)
+	target.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	identity, _ := startProcessExitChild(t, target)
+	require.Eventually(t, func() bool { _, err := os.Stat(ready); return err == nil }, 10*time.Second, 10*time.Millisecond,
+		"the target must be ignoring SIGTERM before it is sent")
+
+	var sent []syscall.Signal
+	result, err := requestProcessExit(t.Context(), identity, syscall.SIGTERM, "graceful", base.InspectProcess,
+		func(ctx context.Context, id base.ProcessIdentity, sig syscall.Signal) error {
+			sent = append(sent, sig)
+			return base.SignalProcess(ctx, id, sig)
+		})
+	require.ErrorContains(t, err, "unconfirmed")
+	require.Equal(t, []syscall.Signal{syscall.SIGTERM}, sent, "exactly one signal, and never SIGKILL")
+	require.Equal(t, "same_birth", result.Observation)
+	require.False(t, result.ExpectedBirthGone)
+	require.Equal(t, "graceful", result.Mode)
+
+	// And it is still running, at its original birth.
+	alive, inspectErr := base.InspectProcess(identity.PID)
+	require.NoError(t, inspectErr)
+	require.Equal(t, identity, alive, "an unconfirmed graceful exit leaves the target untouched")
+}