Repository navigation
feat(recover): request a graceful exit for an explicitly selected process - #1222
Open
antoinetoussaint-byte wants to merge 2 commits into
Open
antoinetoussaint-byte wants to merge 2 commits into
antoinetoussaint-byte wants to merge 2 commits into
Conversation
…cess `recover process` could only request SIGKILL, which discards whatever the target had not written. For a store that owns authoritative state — a Redis holding revocation records and nonces — that can reopen revoked tokens, so there was no supported way to stop one and keep its data. --graceful requests exactly one SIGTERM through the existing base.SignalProcess, letting the process run its own shutdown. It is mutually exclusive with --force and takes the same canonical full-identity receipt. It never escalates: an exit that stays unconfirmed within the five-second budget is reported unconfirmed, and a target that ignores SIGTERM is left alive and untouched rather than killed as a fallback. --force keeps its explicit SIGKILL semantics unchanged. No Core API, protobuf, group, registry or orphan-recovery effect is involved. The result is independently versioned at schema_version 2, adding the requested mode and signal so a reader never infers which irreversibility contract applied; those fields describe the request, not proof of exit or persistence. The --preview receipt stays schema_version 1: its shape did not change, and bumping it would invalidate receipts already saved for review. Core's owner accepted that split explicitly, recorded verbatim in docs/design/process-result-v2-exception.md. Tests use real processes: a SIGTERM handler writes a durable marker before exiting (after a readiness handshake, since signalling before the handler is installed proves nothing), a SIGTERM-ignoring target is still alive at its original birth when the budget expires with exactly one signal sent, and the mode-exclusion, stale-receipt and unknown-selection cases refuse before any effect. Updates docs/commands.md and regenerates docs/cli-reference.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`race` failed on this branch with `readlink /proc/<pid>/exe: no such file or directory` from the graceful test. It is not a flake and not a test-only problem. On Linux a process that has exited but has not yet been waited for keeps its /proc stat and loses its executable link. Core deliberately refuses to call that absence — a thread-group leader can exit while its threads live on — and returns the readlink error instead. The settle loop treated any such error as a failed exit, so `recover process --graceful` reported failure for a target that had in fact shut down cleanly. The race detector made the window wide enough to hit; it is reachable whenever the target's parent is still around to reap it. The retry is deliberately narrow. Core maps a missing /proc/<pid>/stat to ErrProcessNotFound, so the only ENOENT it can return is this window; every other inspection failure — permission, unparseable stat, identity changed mid-read — carries no ENOENT and stays fatal. The retry applies only while waiting out an exit already requested: before the signal an unresolvable reading is still a refusal to act, and nothing is sent. A budget that expires without a verdict still reports the exit unconfirmed, now naming what it kept seeing. Covered by a deterministic unit test over a fake inspector returning core's own *fs.PathError: settles through the window, refuses before the signal, reports unconfirmed on expiry, and still fails a changed birth. Verified red with only the settle-loop branch reverted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
recover processcould only request SIGKILL, which discards whatever the target had not written. For a store that owns authoritative state — a Redis holding revocation records and nonces — that can reopen revoked tokens, so there was no supported way to stop one and keep its data. Classification: feat at the CLI-owned explicit-recovery boundary. Related: #932.Reuses Core's existing
base.SignalProcess, which already supplies current-birth + PGID-bound delivery; no Core API, protobuf, helper or new carrier is added, and no group, registry or orphan-recovery effect is involved.The two modes
--gracefulrequests exactly one SIGTERM and lets the process run its own shutdown. It is mutually exclusive with--force, requires the same canonical full-identity--expectreceipt, and never escalates: an exit that stays unconfirmed within the existing five-second budget is reported unconfirmed, and a target that ignores SIGTERM is left alive and untouched rather than killed as a fallback.--forcekeeps its explicit, irreversible SIGKILL semantics unchanged.Which mode to use is a data question, not a speed one, and the help and docs say so:
--forcediscards unwritten state, and process exit alone is not durability evidence — an RDB/AOF save and readback is.Result versioning
The result is independently versioned at
schema_version2, adding the requestedmodeandsignalso a reader never has to infer which irreversibility contract applied. Those fields describe the request, not proof of exit or persistence. The--previewreceipt staysschema_version1: its shape did not change, and bumping it would invalidate receipts already saved for review. Core's owner accepted that split explicitly; the acceptance is recorded verbatim indocs/design/process-result-v2-exception.md, andTestProcessExitReceiptNativeShapeasserts both versions so a silent re-merge fails.Verification
[SIGTERM]sent and the result reporting unconfirmed.--gracefulwithout a receipt, combined with--forceor--preview, with an empty or missing--expect, and with a receipt whose PID does not match — all before any effect.mode=force/signal=killed.go build ./...,go vet ./...,go test ./cmdandgolangci-lint run ./cmd/...all clean on current main.docs/cli-reference.mdis regenerated from the command tree (-update-reference), not hand-edited — which also picked upadd solutionanddoctor solutionfrom main.Independently exercised before this PR
Root ran the reviewed binary against three live Local Redis instances: fresh previews, then
--graceful, all three reaching observed absence with valid final RDB readbacks. That is the behaviour this change exists for, confirmed on real data rather than inferred from the tests.Limits
Delivery alone is not exit, and observed absence is not proof this command caused it. A same-birth exec after admission can still receive the signal. Cancellation cannot revoke in-flight delivery. The polling budget bounds observation only, not Core's synchronous inspection. The command does not detect which binary produced a receipt, so the same reviewed implementation on the same host/boot is still required, and any change requires a fresh preview.
🤖 Generated with Claude Code