diff --git a/cmd/recover_process.go b/cmd/recover_process.go index 0af980a2..a017fcbe 100644 --- a/cmd/recover_process.go +++ b/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//stat and then /proc//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: } } diff --git a/cmd/recover_process_integration_test.go b/cmd/recover_process_integration_test.go index c83d5fd7..0e64c5ce 100644 --- a/cmd/recover_process_integration_test.go +++ b/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") +} diff --git a/cmd/recover_process_test.go b/cmd/recover_process_test.go index c6e31c2c..9b333dc7 100644 --- a/cmd/recover_process_test.go +++ b/cmd/recover_process_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "io/fs" "os" "path/filepath" "reflect" @@ -44,9 +45,10 @@ func TestProcessExitReceiptNativeShape(t *testing.T) { } _, raw := processExitTestReceipt(t, processExitTestIdentity()) require.Equal(t, "{\"schema_version\":1,\"process\":{\"PID\":1234567,\"PGID\":1234567,\"BootID\":\"boot\",\"StartID\":42,\"Executable\":\"fixture\"}}\n", string(raw)) - result, err := json.Marshal(processExitResult{SchemaVersion: 1, PID: 1234567, Observation: "unknown"}) + // The result is v2 (mode/signal added); the receipt above stays v1. + result, err := json.Marshal(processExitResult{SchemaVersion: 2, PID: 1234567, Mode: "force", Signal: "killed", Observation: "unknown"}) require.NoError(t, err) - require.JSONEq(t, `{"schema_version":1,"pid":1234567,"signal_attempted":false,"signal_accepted":false,"observation":"unknown","expected_birth_gone":false}`, string(result)) + require.JSONEq(t, `{"schema_version":2,"pid":1234567,"mode":"force","signal":"killed","signal_attempted":false,"signal_accepted":false,"observation":"unknown","expected_birth_gone":false}`, string(result)) } func TestProcessExitReceiptRejectsAlteredEvidence(t *testing.T) { @@ -108,6 +110,12 @@ func TestProcessExitCommandRefusesBeforeEffects(t *testing.T) { {pid, "--expect", path}, {pid, "--force"}, {pid, "--preview", "--force"}, {pid, "--preview", "--expect", path}, {pid, "--expect=", "--force"}, {pid, "--expect", path + "-missing", "--force"}, {"1234568", "--expect", path, "--force"}, {pid, "another", "--preview"}, + // --graceful is a mode, not a modifier: it needs its receipt, it cannot + // be combined with --force or --preview, and a stale receipt or a + // missing one refuses before anything is signaled. + {pid, "--graceful"}, {pid, "--graceful", "--force"}, {pid, "--graceful", "--force", "--expect", path}, + {pid, "--preview", "--graceful"}, {pid, "--graceful", "--expect="}, + {pid, "--graceful", "--expect", path + "-missing"}, {"1234568", "--expect", path, "--graceful"}, } for _, args := range cases { t.Run(strings.Join(args, " "), func(t *testing.T) { @@ -166,7 +174,7 @@ func TestProcessExitOwnerOutcomes(t *testing.T) { ctx, cancel := context.WithCancel(t.Context()) defer cancel() reads, calls := 0, 0 - result, err := forceProcessExit(ctx, expected, func(pid int) (base.ProcessIdentity, error) { + result, err := requestProcessExit(ctx, expected, syscall.SIGKILL, "force", func(pid int) (base.ProcessIdentity, error) { require.Equal(t, expected.PID, pid) require.Less(t, reads, len(tc.reads)) r := tc.reads[reads] @@ -186,7 +194,8 @@ func TestProcessExitOwnerOutcomes(t *testing.T) { if tc.delivery != nil { require.ErrorIs(t, err, tc.delivery) } - tc.want.SchemaVersion, tc.want.PID = 1, expected.PID + tc.want.SchemaVersion, tc.want.PID = 2, expected.PID + tc.want.Mode, tc.want.Signal = "force", syscall.SIGKILL.String() require.Equal(t, tc.want, result) require.Equal(t, len(tc.reads), reads) if result.SignalAttempted { @@ -202,14 +211,14 @@ func TestProcessExitCancellationAndPollingBudget(t *testing.T) { expected := processExitTestIdentity() ctx, cancel := context.WithCancel(t.Context()) cancel() - result, err := forceProcessExit(ctx, expected, func(int) (base.ProcessIdentity, error) { t.Fatal("canceled inspection"); return expected, nil }, func(context.Context, base.ProcessIdentity, syscall.Signal) error { + result, err := requestProcessExit(ctx, expected, syscall.SIGKILL, "force", func(int) (base.ProcessIdentity, error) { t.Fatal("canceled inspection"); return expected, nil }, func(context.Context, base.ProcessIdentity, syscall.Signal) error { t.Fatal("canceled signal") return nil }) require.ErrorIs(t, err, context.Canceled) require.False(t, result.SignalAttempted) reads := 0 - result, err = forceProcessExit(t.Context(), expected, func(int) (base.ProcessIdentity, error) { reads++; return expected, nil }, func(context.Context, base.ProcessIdentity, syscall.Signal) error { return nil }) + result, err = requestProcessExit(t.Context(), expected, syscall.SIGKILL, "force", func(int) (base.ProcessIdentity, error) { reads++; return expected, nil }, func(context.Context, base.ProcessIdentity, syscall.Signal) error { return nil }) require.ErrorContains(t, err, "polling budget") require.Greater(t, reads, 1) require.Equal(t, "same_birth", result.Observation) @@ -217,6 +226,71 @@ func TestProcessExitCancellationAndPollingBudget(t *testing.T) { require.True(t, result.SignalAccepted) } +// TestProcessExitSettlesThroughAnIncompleteInspection is the zombie window: on +// Linux a process that has exited but has not yet been waited for keeps its +// /proc stat and loses its executable link, and core reports that as an +// inspection it could not complete rather than as absence — deliberately, since +// the same reading appears when a thread-group leader exits with threads still +// running. +// +// Polling must treat that as "not yet" and keep waiting. Reporting it as a +// failure made `recover process --graceful` fail against a target that had in +// fact shut down cleanly, which is how it failed under the race detector in CI. +func TestProcessExitSettlesThroughAnIncompleteInspection(t *testing.T) { + expected := processExitTestIdentity() + // Exactly what core hands back in that window: os.Readlink's own + // *fs.PathError over ENOENT. A missing /proc//stat is mapped to + // ErrProcessNotFound by core, so this is the only ENOENT it can produce. + zombie := &fs.PathError{Op: "readlink", Path: "/proc/3874/exe", Err: syscall.ENOENT} + reads := 0 + result, err := requestProcessExit(t.Context(), expected, syscall.SIGTERM, "graceful", func(int) (base.ProcessIdentity, error) { + reads++ + switch { + case reads == 1: + return expected, nil // the pre-signal observation + case reads < 4: + return base.ProcessIdentity{}, zombie + } + return base.ProcessIdentity{}, base.ErrProcessNotFound + }, func(context.Context, base.ProcessIdentity, syscall.Signal) error { return nil }) + require.NoError(t, err, "an inspection that reached no verdict is not a failed exit") + require.Equal(t, processObservationAbsent, result.Observation) + require.True(t, result.ExpectedBirthGone) + require.Equal(t, "graceful", result.Mode) + require.Equal(t, syscall.SIGTERM.String(), result.Signal) + + // A target whose inspection never resolves still reports the exit + // unconfirmed when the budget expires, and says what it kept seeing rather + // than only that it ran out of time. + stuck := 0 + _, err = requestProcessExit(t.Context(), expected, syscall.SIGTERM, "graceful", func(int) (base.ProcessIdentity, error) { + if stuck++; stuck == 1 { + return expected, nil + } + return base.ProcessIdentity{}, zombie + }, func(context.Context, base.ProcessIdentity, syscall.Signal) error { return nil }) + require.ErrorContains(t, err, "polling budget") + require.ErrorContains(t, err, "no such file or directory", "an expired budget must say what it kept seeing") + + // And an inspection that cannot complete BEFORE the signal is still a + // refusal to act: the retry is only for an exit already requested. + var sent int + _, err = requestProcessExit(t.Context(), expected, syscall.SIGTERM, "graceful", func(int) (base.ProcessIdentity, error) { + return base.ProcessIdentity{}, zombie + }, func(context.Context, base.ProcessIdentity, syscall.Signal) error { sent++; return nil }) + require.ErrorIs(t, err, errExitInProgress) + require.Zero(t, sent, "nothing may be signalled on a reading that reached no verdict") + + // An inspection that reaches a verdict of a DIFFERENT birth is still fatal: + // this must not become a blanket retry. + replaced := expected + replaced.StartID++ + _, err = requestProcessExit(t.Context(), expected, syscall.SIGTERM, "graceful", func(int) (base.ProcessIdentity, error) { + return replaced, nil + }, func(context.Context, base.ProcessIdentity, syscall.Signal) error { return nil }) + require.ErrorContains(t, err, "different birth") +} + type processExitFailWriter struct{ err error } func (writer processExitFailWriter) Write([]byte) (int, error) { return 0, writer.err } diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 966ccb69..6908c87a 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -4352,9 +4352,14 @@ Preview or explicitly force one independently selected process to exit ``` 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; @@ -4375,6 +4380,7 @@ Flags: ``` --expect string Use the complete verbatim version-1 preview receipt --force Explicitly request irreversible SIGKILL for the selected process birth + --graceful Request one SIGTERM for the selected process birth, letting it shut down itself; never escalates to SIGKILL --preview Print the current process identity without signaling ``` diff --git a/docs/commands.md b/docs/commands.md index 554ae783..aaec098c 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2386,134 +2386,6 @@ contract and handler before building. See [Runnables](runnable.md). |------|-------------| | `--agent` | Agent type (required). Examples: `go-grpc`, `python-grpc`, `nextjs`, `krakend` | -**`add solution`:** - -```sh -codefly add solution wiki --package-id obin/wiki \ - --consumes connect:documents/documents/grpc --consumes connect:robin/chat/connect -``` - -Scaffolds a solution — a module a host installs and runs as a product surface — -in the one shape a solution has. There is no shape flag: a second shape beside -it is what produced nine solutions that do not look alike. - -It composes verbs that already exist (`add module`, then `add service` for each -of the two services) and writes the documents Core already specifies, by -marshalling Core's own types and reading each file back through Core's own -reader. It adds no executor protocol and no new agent. - -What it creates: - -| | | -|------|-------------| -| `services/backend` | the entry service: presents the solution's identity, serves the API the page calls, and is the workload a host installs | -| `services/web` | the shell that serves the page where no host shell mounts it — not published, because a host that federates the remote needs no shell | -| `solution.codefly.yaml` | what the solution exposes, and the route name it addresses each consumed module by | -| `module.contract.codefly.yaml` | the principal its credentials are issued to, with `owner_aware: true` | -| `module.package.codefly.yaml` | the published identity; without it the render declares no presence and nobody can install the solution | -| `services/backend/solution.frontend.codefly.yaml` | the page's title, order and `./Page` remote | - -`services/fe-remote` — the `./Page` remote itself — is an npm package, not a -Codefly service, and is yours to author. - -**The rule the backend lives under.** A module call the authority holds to a -*person* — one its artifact interface admits only in person, with no -intervening actor — is made by the **page**, with that person's own Work -Context. It is never relayed through the backend: a backend reaches a module on -a context minted for the solution's own audience, so relaying such a call puts -the solution between the person and the module and admission refuses it. Reads -and work that are the solution's own go through the backend as usual. This is -why the entry service declares the authority interface and the authority's hop -and **no module edge at all** — every consumed module is reached through that -hop, which lets through only a call the authority checked. - -| Flag | Description | -|------|-------------| -| `--package-id` | The `/` identity the solution is published under. Required — nothing can derive who publishes it | -| `--agent` | Service agent for the backend (default `go`). The backend's language is this flag, not a different shape | -| `--consumes` | A module this solution calls, as `[=]://`. Repeatable. Route names only — authority is declared once, in the module contract | -| `--executor` | A `codefly:solution` executor to name in the solution manifest. Omitted by default: Core specifies no executor protocol and none is published | -| `--title` / `--order` | The page's title and position in the host's shell | -| `--authority-interface` | The authority interface the backend requires | -| `--minimum-codefly-version` | The Codefly range the package declares. Required when scaffolding with a build from source, which reports no release version | - -The **composition** is not scaffolded: which workspace composes the solution, -which modules it pins and at what commits, and the configuration groups those -modules need, are the composition's to say. The command prints each of them, -with the failure each one causes when it is skipped. - -### `codefly doctor solution [module]` - -Judge a solution against the shape `add solution` generates, without changing -anything. With no argument every module in the workspace is judged; a module -that ships no solution manifest is reported as "not a solution" rather than as -broken. - -```bash -codefly doctor solution -codefly doctor solution wiki --json -``` - -It exists because the shape was previously checkable only against somebody's -survey of the solutions that happened to exist — which is how one fleet reached -nine solutions in four different states, two of them unable to receive a person -capability at all with nothing failing to say so. A shape nothing can check is a -convention, and a convention drifts. - -Every rule is one that silently broke something. The two worth knowing: - -| | | -|---|---| -| `owner_aware_missing` | The authority issues a person a capability at a solution **only** when its module contract declares `owner_aware: true`, and refuses by name otherwise. Without it the solution receives no owner-selected scope and can act for nobody — and nothing fails at build time to tell you. | -| `consumed_module_without_binding` | A module named in `api.consumes` that the contract declares no binding for. A **warning**: correct when the module is reached with the viewer's own credential (wiki reaches robin, tracing and annotations that way), a bug when the solution calls it on a person's behalf — and the two look identical until a call is refused. | -| `module_edge_declared` | A solution reaches every module through the **host's door** — a saas host's gateway and frontend, or the authority's hop beside the authority interface. A `service-dependencies` entry naming anything else renders into a mesh allow rule straight to that module's service, past the authority's check. | - -Exit codes: `0` every judged solution conforms (warnings allowed), `1` at least -one check failed. `--json` prints a versioned report per module -(`{schema_version, module, dir, is_solution, status, checks[]}`), so a -consumer repository can gate on it instead of hand-rolling the equivalent. - -**Permissions.** The contract is read through Core's own loader, which refuses -a role granting a scope no binding backs and no ceiling covers — *"role -`installer` grants `documents:read`, which is neither within the module's own -scope ceilings nor a scope one of its bindings spells out"*. That is the -missing-permission class, caught before install rather than at the first -refused call. What a composition's values resolve each binding to is not -judged: that needs the composition, not the directory. - -`page_shell_has_no_app` is worth knowing about: conforming the fleet to one -shape added the `web` shell's *manifest* to solutions whose page had never been -written, because writing one would have been inventing product in a conforming -change. Those solutions are correctly shaped and `codefly run service web` has -nothing to start on them. A declared service with no source is otherwise -invisible — every manifest reads fine, and the first sign is a run that does -nothing. - -A warning is not a failure: it names something worth seeing that does not make -the solution wrong — publishing the page's shell, a headless solution with no -page declaration, a solution with no shell at all. - -| Flag | Description | -|------|-------------| -| `--json` | Print a versioned report per module to stdout | -| `--dir` | Judge this module directory on its own, without resolving a workspace | -| `--require-solution` | With `--dir`, fail when the directory ships no solution manifest | - -`--dir` exists because a consumer repository gating a tree of solutions has -module directories, not necessarily a workspace at each one — `excel-extract` -is a real solution with no `workspace.codefly.yaml`, and requiring one made it -the single solution that could not be judged at all. - -`--require-solution` closes the gap `--dir` leaves: a directory with no -solution manifest reads as "not a solution" and passes, which is right for a -library sitting in the tree and wrong for a solution whose manifest is -missing. A gate that walks `solutions/*/` means "every one of these is a -solution", and says so with the flag. - -```bash -for s in solutions/*/; do codefly doctor solution --dir "$s" --require-solution || exit 1; done -``` - ### `codefly override service /` Run one service of a composed module from somewhere else on this machine, @@ -2559,7 +2431,6 @@ codefly update workspace --agent-override codefly.dev/go-grpc=0.1.47-dev.abc123d codefly update service api # one service to its latest compatible release codefly update service api --agent-version 0.1.47-dev.abc123def456 # pin one service exactly codefly update deps # Go dependencies under the current directory -codefly update deps --dependency golang.org/x/net@v0.62.0 # only this existing external requirement, including indirect pins ``` Without flags, `update workspace` and `update service` move to the agent's @@ -4567,24 +4438,48 @@ process group, descendants, containers or registrations. ```bash codefly recover process 12345 --preview > process-exit.json # Review the complete identity and independently establish the target. +codefly recover process 12345 --expect process-exit.json --graceful +# …or, when losing unwritten state is acceptable: codefly recover process 12345 --expect process-exit.json --force ``` Execution requires the same PID and full current identity as the preview, including process group and executable at admission. PID1 and this CLI's PID -refuse. The only signal is explicit, irreversible SIGKILL through Core's -`SignalProcess`: there is no graceful default, escalation or automatic retry. +refuse. **Exactly one of `--graceful` or `--force` is required with +`--expect`**, and they are mutually exclusive: `--graceful` requests one +`SIGTERM`, `--force` requests one irreversible `SIGKILL`, both through Core's +`SignalProcess`. There is no default mode, no escalation and no automatic +retry — a `--graceful` request is **never** followed by `SIGKILL`, and an exit +that stays unconfirmed is reported as unconfirmed rather than falling back to +force. + +Which mode to use is a data question, not a speed one. `--graceful` lets the +process run its own shutdown, so a store that owns authoritative state — a +Redis holding revocation records and nonces, say — gets to finish writing +before it exits. `--force` discards whatever the process had not yet written; +for such a store that can reopen revoked tokens, and process exit alone is not +durability evidence (an RDB/AOF save and readback is). A target that ignores +`SIGTERM` stays alive when the polling budget expires, untouched. Core binds delivery to process birth and group, not an immutable executable; a same-birth exec after admission can still receive the signal. One CLI owner call may involve Core's internal kernel-token retries. -The result reports `signal_attempted`, `signal_accepted`, `observation` and -`expected_birth_gone`. Success requires observed absence and no delivery, +The result is `schema_version` 2 and reports the requested `mode` and `signal` +alongside `signal_attempted`, `signal_accepted`, `observation` and +`expected_birth_gone`, so a reader never has to infer which irreversibility +contract applied. The `--preview` receipt is unchanged at `schema_version` 1. Success requires observed absence and no delivery, inspection, cancellation or output error. Already-absent is successful without a signal attempt. A replacement is reported as `changed_birth` with a nonzero exit and is never selected; its presence proves neither PID availability nor that this command caused the original exit. Same-birth exec/group changes do not prove exit. Ordinary owner inspection errors remain `unknown`, not absence. +One inspection failure is treated as "not yet" rather than as a failure, and +only while waiting out an exit already requested: a process that has exited but +has not yet been waited for keeps its process entry and loses its executable +link, which the owner reports as neither presence nor absence. Polling keeps +waiting for a verdict; if none arrives the exit is reported unconfirmed, naming +what it kept seeing. Before the signal the same reading refuses to act, and +every other inspection failure stays an error in both places. Delivery errors remain errors even if subsequent observation finds absence. The five-second polling budget does not interrupt synchronous inspection or output. Cancellation prevents a new owner call once observed; it cannot revoke @@ -4598,9 +4493,13 @@ Orphan recovery and registry removal remain separate explicit operations. **Fixed local receipt exception:** Core's owner explicitly approved this separate native-identity exception under its architecture contract policy. -Version1 embeds `base.ProcessIdentity` unchanged in a `schema_version`/`process` -envelope and uses a fixed local result shape. Both ends must use the same -reviewed implementation on the same host/boot; any change requires fresh preview. +The fixed version-1 preview receipt embeds `base.ProcessIdentity` unchanged in +a `schema_version`/`process` envelope. The independently versioned result is +fixed at version 2, adding requested `mode` and `signal` under the +[explicit Core owner acceptance](design/process-result-v2-exception.md). These +fields describe the request, not proof of exit or persistence. Both ends must +use the same reviewed implementation on the same host/boot; any implementation +change requires fresh preview even when the receipt schema is unchanged. The command does not detect the binary that produced a receipt. It is untrusted comparison evidence, not a credential, ownership grant, registry or remote API. The complete default JSON encoding plus its single newline must be saved diff --git a/docs/design/process-result-v2-exception.md b/docs/design/process-result-v2-exception.md new file mode 100644 index 00000000..d8f17939 --- /dev/null +++ b/docs/design/process-result-v2-exception.md @@ -0,0 +1,13 @@ +# Core owner acceptance: bounded local process result v2 + +Accepted for exact reviewed CLI cmd/recover_process.go SHA256 52f7b993d3a3e52eea832f22070d351ca74373071875b601f9967621596b33d3 and docs/commands.md SHA256 060dc17f96e0619591d3964afba2c5414404c1c9283bc8c29a3a8b2fe16afb7d, following full reading of /tmp/cli934-graceful-recovery-review.md and the current source/guide. + +This is the explicit additional exception required by the prior fixed-local-result contract: permit fixed schema_version2 processExitResult containing schema_version, pid, mode, signal, signal_attempted, signal_accepted, observation, expected_birth_gone. Mode and signal describe the REQUEST, not proof of delivery/exit/persistence. The native ProcessIdentity preview receipt remains canonical fixed schema_version1 unchanged; no registry/Docker/remote protocol exception follows. Future result or receipt evolution still requires explicit owner review or the protobuf default. Keep maintained exact-shape assertions for both independent versions. + +The same reviewed implementation/host/boot and fresh-preview requirement remains. Unchanged receipt schema does not authorize reuse of a preview made with another binary. CLI owns explicit graceful/force policy, Core SignalProcess owns identity-bound delivery. One SIGTERM owner call for graceful, no escalation/group/reap/registry operation, preserved unknown/error outcomes. Accepted result v2 is intentionally distinguishable from force-only result v1; no compatibility fallback is required. + +Small documentation precision: replace the current exception paragraph's ambiguous “Version1 ... and uses a fixed local result shape” with explicit receipt-v1/result-v2 wording and link this acceptance. This records the already accepted narrow exception; no mechanism/API change is required. The command guide now correctly documents graceful/force and resultv2 above that paragraph. + +Source/contract acceptance only. Independent Darwin tests are attributed to the reviewer report, not rerun here. This receipt does not prove Redis final-save durability or authorize target selection/signaling. No production source edits or process/engine effects by Core owner. + +Acceptance provenance: Core owner receipt `core-cli934-graceful-result-v2-exception.md`, SHA256 `0e4a44e1c9deef2f1d509484f81aad5f58b9c03da10d43bff888960d78ea8088`, recorded 2026-10-28. This copy retains the owner acceptance; hashes above identify the accepted inputs before the requested command-guide clarification.