docs(review): consumer input from alkssh — PTY-path exit-signal fidelity vs the ADR-004 negative-code convention
This commit is contained in:
1 parent
6e33320e69
commit
d7a296d7bd
1 file changed
+127
@@ -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<i32>`) 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<String> }`; 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)
|
||||
Reference in new issue
Block a user