diff --git a/docs/reviews/004-pty-exit-signal-fidelity-alkssh-input.md b/docs/reviews/004-pty-exit-signal-fidelity-alkssh-input.md new file mode 100644 index 0000000..a0a126c --- /dev/null +++ b/docs/reviews/004-pty-exit-signal-fidelity-alkssh-input.md @@ -0,0 +1,127 @@ +--- +status: open +last_updated: 2026-10-31 +--- + +# Code Review #004 — PTY-path exit-signal fidelity (consumer input from alkssh) + +## Purpose + +Consumer-input review filed from alkssh 0.1.0 (published 2026-10-31) +under the family's upstream posture (file asks, never work around). +The ask was booked alkssh-side at OQ-10 with the note "recorded … not +tracked here — it constrains alktty"; this review is that filing. + +The finding: the `LocalTtyBackend`'s PTY path cannot express ADR-004 +decision 1's own exit-status convention for signal-terminated +children. The pipe path conforms; the PTY path reports a signal death +as a plain nonzero code. + +## The convention (ADR-004 decision 1) + +The exit chunk carries `{"type":"exit","code":N}` with `code` an +`i32` in this crate's exit-status convention — normal exit → +`ExitStatus::code()`; on Unix, **signal-terminated exit → the negative +signal number** (e.g. `-9`), Python's `Popen.returncode` parity. This +is a one-way door: consumers read `code < 0` as "died by signal N" +(also the `-1` sentinel's collision note — ADR-004 decision 1, the +2026-10-31 doc commit `6e33320`). + +## The gap + +The waiter thread (`src/local/pty.rs`, the "Waiter thread" spawn, +` Child::wait()` → `oneshot`) converts the reaped status with +`status.exit_code() as i32` — and discards the signal identity: + +- portable-pty 0.9.0's `ExitStatus` is + `struct ExitStatus { code: u32, signal: Option }`; a + signal-terminated child is constructed via `with_signal(name)`, + which hardcodes `code = 1` and keeps the signal **only as a + humanized name string** (`signal() -> Option<&str>`). Verified + against the published 0.9.0 source (`lib.rs`, the `ExitStatus` impl). +- So a `SIGTERM`-terminated child yields `exit_code = 1`, the oneshot + carries `1`, and the adapter emits `{"type":"exit","code":1}`. The + consumer cannot distinguish "exited with status 1" from "terminated + by signal 15". ADR-004's own `−15` encoding is unrecoverable on this + path. + +Two aggravations worth recording in this crate's own terms: + +- The crate's **own** signal features produce signal deaths the PTY + path then flattens: REQ-TTY-02's `signal()` (`libc::kill(-pgid, + sig)`) and ADR-056's cancel-cleanup `ChildKiller::kill()` (SIGHUP) + both end in signal-terminated reaps that the waiter reports as plain + `1`. +- The pipe path already conforms (ADR-004 decision 1's + `ExitStatusExt::signal()` reference; `src/local/pipe.rs:: + exit_code_from`) — the two backends disagree on the same convention. + +## Consumer consequence (alkssh, the vanilla-SSH front door) + +SSH reports a signal death as `exit-signal(name)` + `core_dumped`, +distinct from `exit-status(n)`. alkssh's derivation contract (its +`mapping.md` §"Exit chunk → SSH exit reporting"): `code ≥ 0` → +`exit-status(N)`; negative `−N` on Unix → `exit-signal(name)` via a +server-side signal-number→name table; `−1` → `exit-status(255)` +(never a signal claim — the sentinel collides with SIGHUP; +`core_dumped` never asserted). The contract was verified live on the +pipe path (`−15` → `exit-signal("TERM")`, alkssh POC-3 §D.2). + +On the PTY path, that same session reports `exit-status(1)` — while +vanilla `sshd` for the identical child reports +`exit-signal(15)`. Fail-closed, honest, and never a wrong claim — but +a fidelity gap versus both the family convention and vanilla sshd +parity, in the crate's first published consumer's primary path +(`ssh host` interactive / `ssh host "cmd"` every day). + +alkssh holds no alkssh-side workaround and adapts automatically with +**no contract change** the day the convention holds on the PTY path: +its mapping table consumes ADR-004's negative code verbatim. Nothing +in alkssh needs to change for this to be worth fixing; the +justification is tty/pty-grounded independently (see below). + +## Why this is also an in-crate issue, not only alkssh's + +The negative-code convention is this crate's documented, one-way +wire shape (ADR-004), and the crate advertises `Popen.returncode` +parity in the ADR itself. The PTY is the backend where signal +identity is the most semantically meaningful (shells, editors, and +every long-running session dies by signal at least sometimes), and +the backend's own signal-forwarding capabilities (REQ-TTY-02, the +resize/signal control handle) make the flattened report internally +inconsistent: the feature exists to deliver a signal, the exit report +cannot say a signal happened. + +## What a resolution decides (framing, not prescription) + +1. Whether the PTY backend conforms to ADR-004's negative-code + convention (the pipe path's precedent) — in which case the waiter + needs the signal identity, which portable-pty 0.9.0 does expose + (as a name string, requiring a name → Unix signum mapping at the + alktty side, or an upstream ask for a numeric form). +2. Or whether the PTY path's flattening is ruled the documented + posture — which would need its own ADR, since it narrows ADR-004 + decision 1 per-backend (a change to a wire-visible convention, + one-way from alkssh's consumer). +3. Whatever widens what the exit chunk carries (e.g. a separate + signal field instead of the negative encoding) is + alkssh-irrelevant-by-design — alkssh OQ-10 resolved that no + widening was warranted from the consumer seat — and would ripple + `ControlMessage::Exit`'s parsed shape regardless. Not asked for. + +## References + +- ADR-004 (the exit-code-on-control-chunk convention; decision 1's + negative encoding and the `-1` sentinel note) +- `src/local/pty.rs` (the waiter thread; `exit_code() as i32`), + `src/local/pipe.rs::exit_code_from` (the conforming pipe path) +- portable-pty 0.9.0 `src/lib.rs` — `ExitStatus { code, signal }`, + `with_signal` (code hardcoded 1, signal as name string) +- alkssh OQ-10 + (`/workspace/@alkdev/alkssh/docs/architecture/open-questions.md`, + closed as contract + upstream constraint) — the origin of this + filing; alkssh `docs/architecture/mapping.md` §"Exit chunk → SSH + exit reporting" (the derivation contract this gap constrains) +- alkssh POC-3 §D.2 (pipe-path fidelity proven live; PTY-path limit + verified against portable-pty source) +- alknet ADR-055 (the `-1` sentinel lineage ADR-004 inherits) \ No newline at end of file