- portable-pty 0.9.0's ExitStatus flattens a signal-terminated child to
code 1 with the signal only as a strsignal name string, so a SIGTERM
death reported {"code":1} — indistinguishable from a real exit 1 and
a violation of ADR-004 decision 1's negative-code convention (review
#004, consumer input from alkssh)
- on Unix the waiter now skips the portable-pty conversion entirely:
libc::waitpid(pid) on the direct child, decoded WIFEXITED/WEXITSTATUS
and WIFSIGNALED/-WTERMSIG (numeric, locale-independent); Child::wait()
remains as the waitpid-failure fallback and the non-Unix primary
(std::process::Child's Drop neither kills nor reaps, so the two paths
never double-reap)
- signal features now round-trip: signal("TERM") -> -15, ADR-056 cancel
kill (SIGHUP) -> -1 (the documented ADR-004 sentinel collision)
- tests: SIGINT -> -2, SIGTERM -> -15, SIGHUP fallback -> -1 pinned
- docs: ADR-004 decision 1, tty-local.md, ADR-005, tty-adapter.md,
review #004 closed as resolved with disposition
Verification: cargo test (113), cargo test --all-features (138 + the
tests/pty.rs, tests/pipe.rs, tests/negotiation.rs integrations),
clippy --all-targets --all-features -D warnings, fmt, doc,
fuzz/shared corpus replay (9)
7.9 KiB
status, last_updated
| status | last_updated |
|---|---|
| resolved | 2026-11-01 |
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<i32>) converts the reaped status with
status.exit_code() as i32 — and discards the signal identity:
- portable-pty 0.9.0's
ExitStatusisstruct ExitStatus { code: u32, signal: Option<String> }; a signal-terminated child is constructed viawith_signal(name), which hardcodescode = 1and keeps the signal only as a humanized name string (signal() -> Option<&str>). Verified against the published 0.9.0 source (lib.rs, theExitStatusimpl). - So a
SIGTERM-terminated child yieldsexit_code = 1, the oneshot carries1, 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−15encoding 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-cleanupChildKiller::kill()(SIGHUP) both end in signal-terminated reaps that the waiter reports as plain1. - 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)
- 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).
- 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).
- 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
-1sentinel 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; alksshdocs/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
-1sentinel lineage ADR-004 inherits)
Resolution (2026-11-01)
The finding was verified against the published portable-pty 0.9.0
source and confirmed in full: the waiter's status.exit_code() as i32
discarded signal identity; the From<std::process::ExitStatus> impl
keeps a signal death only as a strsignal name string with code = 1
(unwrap_or(1)), while the pipe path conformed.
Resolved in-crate, without a fork and without an upstream ask, via
option (1) of the framing above — but through a mechanism the framing
did not list: on Unix the waiter now skips portable-pty's status
conversion entirely and reaps the direct child with
libc::waitpid(pid, ..) (src/local/pty.rs::wait_exit_code), decoding
WIFEXITED/WEXITSTATUS (normal exit) and WIFSIGNALED/WTERMSIG
(signal death → -N). This is fully numeric, locale-independent, and
exhaustive (no name→signum table, no strsignal parsing). It relies on
portable-pty's unix backend returning a std::process::Child whose
Drop neither kills nor reaps — so the two paths never double-reap
(Child::wait() remains as the waitpid-failure fallback and the
non-Unix primary).
Behavioral notes recorded alongside:
- Signal deaths from the crate's own features now encode honestly:
signal("TERM")→-15; the ADR-056 cancel kill (SIGHUP) →-1, which is the documented ADR-004 sentinel collision (now common on the cancel path, not just the wait-failure path). - Tests strengthened: PTY-path
SIGINT→-2,SIGTERM→-15, and the SIGHUP fallback →-1collision is pinned. alkssh needs no change; its mapping table consumes the negative code verbatim.
Docs updated: ADR-004 decision 1 (both backends' encoding reference),
tty-local.md (waiter thread + cancel-cleanup), ADR-005 and
tty-adapter.md (mechanism wording).