SIGTERM and SIGHUP run the SIGINT drain

CHANGELOG.md

@@ -7,6 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- **A `kill` or a closed terminal now runs the same cleanup Ctrl-C does.** Only
+  SIGINT had a handler, so `kill <dl>` — a supervisor timing a run out, a CI job
+  being cancelled, a shutdown sweep — and closing the terminal window (SIGHUP)
+  both ended `dl` where it stood, leaving the staged plaintext `GH_TOKEN` file on
+  disk and the `devpod up` child orphaned: the exact pair the SIGINT handler
+  exists to prevent, and in SIGHUP's case unwatched, since the window any
+  complaint would have appeared in is the one that just went away. All three
+  signals now run the one async-signal-safe drain — kill the child's process
+  group, unlink the registered temp files, `_exit` — and the code they exit with
+  is **128 + the signal number**: 130 for Ctrl-C, 143 for a `kill`, 129 for a
+  closed terminal. What still does not run on any of them is the `--rm`
+  removal, which a signal handler may not do (#304).
+
+  **`nohup dl …` still outlives its terminal.** A SIGTERM or SIGHUP that was
+  already set to be ignored when `dl` started stays ignored and ends nothing,
+  which is how `nohup` works and what keeps it working. Ctrl-C is deliberately
+  not switchable the same way, and is unchanged from previous releases: a shell
+  script backgrounding a job hands its child an ignored SIGINT whether anyone
+  wanted one or not, so honouring it would silently stop the cleanup for every
+  `dl` launched from a script or a CI step.
+
 ## [0.12.0] - 2026-08-23
 
 ### Removed

README.md

@@ -351,7 +351,29 @@ that session, and either fires the removal.
 removal (a signal handler may not allocate or lock, and this one `_exit`s):
 
 - Ctrl-C during the clone or the container build, before any pty exists.
-- Closing the terminal window — SIGHUP, which `dl` does not handle at all.
+- `kill <dl>` from another shell, and a supervisor or CI runner cancelling the job.
+- Closing the terminal window.
+
+What all three *do* run is the cleanup the removal is not: the staged plaintext
+`GH_TOKEN` file is unlinked and the `devpod up` child is killed, so none of these three
+leaves a credential on disk or a build running behind you. The one exception is a run
+whose SIGTERM was disarmed before it started: the drain fells the build with a
+`killpg(…, SIGTERM)`, so disarming that signal disarms its own reach into the child too.
+(Ctrl-\ — SIGQUIT — is not one of them and still does: it means "die now and dump core",
+and tidying up first is not what it asks for.) The workspace is what stays — still there
+under its name, and `dl <ws> rm` is how it goes.
+
+They are told apart by the exit code, which is **128 + the signal number**: 130 for
+Ctrl-C, 143 for a `kill`, 129 for a closed terminal.
+
+Two of the three can be switched off in the ordinary way, and one cannot. If a SIGTERM
+or a SIGHUP was **already set to be ignored** when `dl` started — which is what
+`nohup dl …` does to SIGHUP — that stays ignored and ends nothing, so `nohup` still
+outlives the terminal it was started from. **Ctrl-C is not switchable like that**, and
+that is deliberate rather than an omission: a shell script backgrounding a job (`dl … &`)
+hands its child an ignored SIGINT whether or not anyone wanted one, so honouring it there
+would quietly stop the cleanup for every `dl` run from a script or a CI step. Ctrl-C
+behaves exactly as it always has.
 
 One-line check for your own setup: start `dl <ws> --rm` and press Ctrl-C once. A
 fresh prompt *inside* the container means Ctrl-C is being forwarded and the removal will

docs/rust-rewrite-plan.md

@@ -187,7 +187,7 @@ Nothing diverges silently.
 | 24 | dl.py:4888's `Failed to create workspace: {e}` wrapper string has no producer: the `OSError`/`RuntimeError` class it caught is typed at the source (devpod missing/blocked/timed out render as their own one-liners; a NUL command is row 19's refusal), each with its Python exit code. | One failure, one line, no generic wrapper (row 4's aim). |
 | 25 | A path spec that normalises to `/` (`dl /`, `//`, `/.`, `/..`) is refused with `'/' does not name a workspace: its path has no final component to name one after.` (exit 1) instead of handing devpod an empty `--id`. Python also exited 1, but only after `devpod up` ran with a blank id and the follow-up `devpod ssh ""` failed. | Refusing an unnameable spec up front beats creating a blank-id workspace; exit code matches, wording differs. |
 | 26 | `dl --version <extra-arg>` / `aid --version <extra-arg>` exit 2 (clap rejects an argument beside `--version`) where Python ignored the extra and printed the version (exit 0). | Companion to rows 2/14/15 — the generated parser owns its usage errors. |
-| 27 | On SIGINT dl prints no timing summary and no partial output — the handler is `_exit(130)` after an async-signal-safe cleanup (unlink the staged token / temp files, `killpg` the `devpod up` group). Python emitted a timing summary from a `finally`. | A signal handler may not allocate, lock, or format; correctness of the credential cleanup wins over the summary (row 5: timing is not a parity dimension). |
+| 27 | On SIGINT, SIGTERM or SIGHUP dl prints no timing summary and no partial output — the handler is `_exit(128 + signo)` (130 / 143 / 129) after an async-signal-safe cleanup (unlink the staged token / temp files, `killpg` the `devpod up` group). Python handled only `KeyboardInterrupt`: it exited 130 and emitted a timing summary from a `finally`, while a SIGTERM or SIGHUP killed it outright with no cleanup at all — as it did dl until #304. A `SIG_IGN` disposition inherited at startup is honoured for SIGTERM and SIGHUP, so `nohup dl …` still outlives its terminal, and overridden for SIGINT. | A signal handler may not allocate, lock, or format; correctness of the credential cleanup wins over the summary (row 5: timing is not a parity dimension). The two added signals abandon exactly what a Ctrl-C abandoned — the same staged plaintext token, the same orphaned build — so they get the same drain, and 128+signo is what Python's `sys.exit(130)` already spelled out long-hand for one of the three. Note this is the opposite convention to `Session::exit_status`, which keeps Python's negative `returncode` for a *child* killed by a signal rather than inventing 128+n; each follows its own Python precedent and neither generalises. The SIGINT exception to the inherited-ignore rule is the pre-existing path kept rather than uniformity lost: a non-interactive shell backgrounding a job ignores SIGINT and SIGQUIT for its child under POSIX job control, which is no statement about interruption, and honouring it would stop the drain for every `dl … &` in a script. Pinned by the `INHERITED_IGNORE` table in `dl/tests/interrupt.rs`, one row per signal, and by the `drained_signals` guard beside the handled set in `dl/src/lib.rs` — which fails if a signal is added there, naming that table. The two lists cannot be machine-checked against each other (an integration test is a separate crate and the set is private), so the guard buys "cannot grow in silence" rather than "cannot disagree". |
 | 28 | The `metadata.json` reader takes strict JSON where Python's `json.loads` took its extensions: `NaN`/`Infinity` literals, numbers beyond f64 range (`1e400`, which Python reads as `inf`), and lone-surrogate `\uD800` escapes all read as corruption, so a file carrying one is quarantined to `metadata.json.corrupt` (bytes intact, single slot) and the run starts with an empty cache where Python loaded the records around the value. | serde_json cannot represent a lone surrogate in a Rust `String` and f64 cannot hold `1e400`, so accepting them would mean values the store could not round-trip; no build of dl (Python or Rust) ever writes any of them — only a hand-edited or third-party-written file can carry one — and the quarantine keeps the bytes recoverable. Pinned by `pythons_json_extensions_read_as_corruption_here` in `domain/metadata.rs`. |
 | 29 | A clone the host refused as not-found gets a second line naming the same repository under an owner the completion cache knows: `Did you mean 'kinisi-robotics/kinisi_ros'? …`. Python printed git's stderr and stopped. The line is added only for a host's own not-found wording (GitHub, GitLab, Bitbucket), only from a clone step, and only when the cache holds the same repo name under another owner *and* not under the owner typed; at most three candidates are named and any remainder is counted. The git text above it is unchanged in every case. | A mistyped owner and a revoked permission produce the same six lines from git, so git's words cannot tell them apart — and devlaunch already holds the answer in the list its own shell completion offers. Additive: nothing is removed or reworded, and a machine with no cache entry sees exactly what it saw before. The suggestion names a *spec*, not a `dl …` command line, because `aid` renders through the same function and its own invocation carries an agent prompt. Pinned by the `wrong_owner_hint` tests in `dl/tests/launch.rs` and `dl/src/render.rs`. |
 

rust/aid/src/interactive.rs

@@ -22,7 +22,7 @@
 //! stderr after the prompt is submitted, so the build's progress is seen — just
 //! not interleaved with the typing. The child is deliberately left in aid's
 //! process group: a terminal Ctrl-C mid-editing reaches both processes, and the
-//! child's own interrupt handler (the shared `dl::install_interrupt_handler`
+//! child's own interrupt handler (the shared `dl::install_signal_handlers`
 //! disposition) kills its `devpod up` group and unlinks its staged token file,
 //! so abandoning the editor tears the whole boot down with no new machinery.
 //!

rust/aid/src/main.rs

@@ -33,7 +33,7 @@ use std::io::Write as _;
 use rewrite::UsageError;
 
 fn main() {
-    dl::install_interrupt_handler();
+    dl::install_signal_handlers();
     // `args_os` and a lossy decode, not `args`: `std::env::args()` panics on an
     // argument that is not valid UTF-8, which would end `aid $'\xff'` with an exit
     // 101 and a traceback. Python decoded argv lossily and carried on

rust/aid/tests/interrupt.rs

@@ -6,7 +6,7 @@
 //! `up` child. The audit that followed the port found aid's `main` had kept a bare
 //! `_exit` handler that cleaned up neither, where `dl`'s handler did — the two
 //! process entry points had drifted. Both now install one shared disposition
-//! (`dl::install_interrupt_handler`); this is aid's half of the proof, the twin of
+//! (`dl::install_signal_handlers`); this is aid's half of the proof, the twin of
 //! `dl/tests/interrupt.rs`.
 //!
 //! Linux-only (as the whole port is, #254): it reads liveness through `kill -0` and

rust/devlaunch-runner/src/interrupt.rs

@@ -1,8 +1,10 @@
 //! Async-signal-safe cleanup for a `dl` that is killed mid-flight.
 //!
-//! `dl`'s SIGINT disposition is `_exit(130)` (see the `dl` binary's `main`): a
-//! signal handler may do almost nothing — not allocate, not lock a mutex, not
-//! call a libc function outside the async-signal-safe list — so it cannot run
+//! `dl`'s disposition for SIGINT, SIGTERM and SIGHUP alike is `_exit(128 +
+//! signo)` — 130, 143, 129 — after the cleanup below (see `dl`'s
+//! `install_signal_handlers`, which both binaries call from `main`). A signal
+//! handler may do almost nothing — not allocate, not lock a mutex, not call a
+//! libc function outside the async-signal-safe list — so it cannot run
 //! destructors. Python got its cleanup for free, because a `KeyboardInterrupt`
 //! *unwinds*: the `with` blocks that staged the GitHub token, wrote the metadata
 //! temp and unpacked the tools bundle all ran their `finally`. `_exit` runs
@@ -168,10 +170,19 @@ unsafe fn drain() {
     // gone, closing the window in which an orphan could still read it.
     let pgid = FOREGROUND_PGID.load(Ordering::SeqCst);
     if pgid > 0 {
-        // SAFETY: `killpg` is async-signal-safe. A stale pgid (the child was
-        // reaped between load and here) at worst signals a recycled group; the
-        // window is closed by `clear_foreground_child` running before the reap
-        // is observable and by SIGINT delivery being to this process.
+        // SAFETY: `killpg` is async-signal-safe. The one hazard is a stale pgid:
+        // the child was reaped, and `clear_foreground_child` — which runs just
+        // after the wait returns, not before it — has not stored the zero yet. In
+        // that window the group is empty, so `killpg` fails ESRCH and nothing
+        // happens; for it to name a *live* group instead, the kernel would have
+        // had to recycle that pid as a new group leader within those few
+        // instructions, which needs the whole pid space to wrap first.
+        //
+        // Deliberately not argued from which signal delivered this: any of the
+        // three handled signals can arrive here, and a group-wide or cgroup-wide
+        // SIGTERM reaches the child as well as `dl`. The bound above holds
+        // whatever woke the handler, which is why it is stated in terms of the
+        // reap rather than the sender.
         unsafe {
             libc::killpg(pgid, libc::SIGTERM);
         }