diff --git a/docs/architecture/decisions/004-exit-code-on-control-chunk.md b/docs/architecture/decisions/004-exit-code-on-control-chunk.md index b437b51..8b0df96 100644 --- a/docs/architecture/decisions/004-exit-code-on-control-chunk.md +++ b/docs/architecture/decisions/004-exit-code-on-control-chunk.md @@ -63,10 +63,14 @@ not data (process output). It rides on the control channel {"type":"exit","code":0} ``` -The `code` is an `i32` (matches `std::process::ExitStatus::code()` and -Unix wait-status convention; negative values are signal-terminated, -e.g., `-9` for SIGKILL, matching `ExitStatus::code()`'s behavior on -Unix). The chunk is the last control chunk before stream close. +The `code` is an `i32` in this crate's exit-status convention: +`ExitStatus::code()` on normal exit; on Unix, the negative signal +number on signal-terminated exit (e.g., `-9` for SIGKILL) — the same +convention as Python's `subprocess.Popen.returncode`. It is **not** +`ExitStatus::code()`'s behavior, which returns `None` for signal +termination (the encoding comes from `ExitStatusExt::signal()`, see +`src/local/pipe.rs::exit_code_from`). The chunk is the last control +chunk before stream close. ### 2. The "exit chunk is last" invariant @@ -156,15 +160,19 @@ the completion notification. code. That's correct — a cancelled stream doesn't have a deterministic exit; the client that cancels already knows it cancelled. The exit chunk is for the client that reads to completion. -- The `-1` error-exit code conflates "the backend couldn't determine - the exit" with "the process exited with code -1" (which doesn't - happen on Unix — `ExitStatus::code()` returns `None` for signal - termination, not -1; the POC's waiter thread sends -1 only on - `wait()` failure, not on signal termination — signal termination - sends the negative signal number, e.g., -9 for SIGKILL). A client - that needs to distinguish "real exit -1" from "backend error" can't - from the code alone. This is a documented edge case; if it becomes - load-bearing, a future control message type +- The `-1` sentinel collides with a real SIGHUP termination on the + Unix pipe path: signal-terminated children are encoded `-N`, so + SIGHUP (signal 1) encodes as exactly `-1` — the same value the + adapter sends for a backend wait failure. A client can't + distinguish a real SIGHUP termination from a backend wait failure + from the code alone. The collision is narrow in practice (the usual + way a pipe-mode child receives SIGHUP is a client-sent + `Signal { name: "HUP" }` control chunk, which the client knows it + sent), but it is real for any other SIGHUP source. Note + `std::process::ExitStatus::code()` cannot itself cause the + collision — it returns `None` for signal termination, so this + conflation exists only through this crate's `-N` encoding. If + distinguishing becomes load-bearing, a future control message type (`{"type":"exit_error","message":"..."}`) can carry the distinction additively (the `type`-tagged enum is the extension seam per ADR-001). diff --git a/docs/architecture/tty-wire.md b/docs/architecture/tty-wire.md index e7ef4a3..0be8b77 100644 --- a/docs/architecture/tty-wire.md +++ b/docs/architecture/tty-wire.md @@ -270,9 +270,13 @@ were indistinguishable on the same stream_type. backend's default kill (see [tty-local.md](tty-local.md) REQ-TTY-02 — `portable_pty`'s `ChildKiller::kill` sends SIGHUP). -**Exit code.** `code` is `i32` (matches `std::process::ExitStatus::code()`; -negative values are signal-terminated, e.g., -9 for SIGKILL on Unix). The -exit chunk is the last control chunk before stream close (ADR-004). +**Exit code.** `code` is `i32`: `ExitStatus::code()` on normal exit; on +Unix, the negative signal number on signal-terminated exit — the crate's +own convention, not `ExitStatus::code()`'s, which returns `None` for +signal termination. `-1` is the adapter's "could not determine" sentinel +and (via the `-N` encoding) the SIGHUP value — the SIGHUP/sentinel +collision is a documented edge case (ADR-004). The exit chunk is the +last control chunk before stream close (ADR-004). **Extensibility.** The `type` tag is the extension seam: new control message types are added by extending the tagged enum. Unknown `type` diff --git a/src/control.rs b/src/control.rs index a236575..b62211c 100644 --- a/src/control.rs +++ b/src/control.rs @@ -85,10 +85,13 @@ pub enum ControlMessage { Eof, /// Process exit code (server→client, `STREAM_CTRL_OUT`). The exit /// chunk is the last control chunk before stream close (ADR-055). - /// `code` is `i32` matching `std::process::ExitStatus::code()`; - /// negative values are signal-terminated (e.g., `-9` for SIGKILL on - /// Unix). `-1` is the adapter's best-effort "backend could not - /// determine the exit code" sentinel (ADR-055 §4). + /// `code` is `i32`: `ExitStatus::code()` on normal exit; on Unix, + /// the negative signal number on signal-terminated exit (e.g., + /// `-9` for SIGKILL — the crate's convention, not + /// `ExitStatus::code()`'s, which returns `None` there). `-1` is + /// the adapter's best-effort "backend could not determine the + /// exit code" sentinel (ADR-055 §4) — colliding with the SIGHUP + /// encoding on the Unix pipe path. Exit { code: i32 }, } diff --git a/src/local/pipe.rs b/src/local/pipe.rs index 3c75292..8b982bf 100644 --- a/src/local/pipe.rs +++ b/src/local/pipe.rs @@ -283,8 +283,10 @@ impl Drop for PipeExitFuture { /// Map an `ExitStatus` to the adapter's `i32` exit code: `code()` on normal /// exit; on Unix, the negative signal number on signal-terminated exit -/// (matches `std` convention and ADR-055 §4). Falls back to `-1` (the -/// adapter's "could not determine" sentinel — ADR-055 §4). +/// (ADR-055 §4's convention — `ExitStatus::code()` returns `None` for +/// signal termination, so this crate's encoding is via +/// `ExitStatusExt::signal()`). Falls back to `-1` (the adapter's +/// "could not determine" sentinel — ADR-055 §4). fn exit_code_from(status: ExitStatus) -> i32 { if let Some(code) = status.code() { return code; diff --git a/src/session.rs b/src/session.rs index 36cadbb..8babe04 100644 --- a/src/session.rs +++ b/src/session.rs @@ -509,10 +509,13 @@ impl TtySession { /// Await the `Exit` control chunk and return the process exit /// code. The exit chunk is the last control chunk before stream - /// close (ADR-055). `code` is `i32` matching - /// `std::process::ExitStatus::code()`; negative values are - /// signal-terminated, `-1` is the adapter's best-effort "backend - /// could not determine the exit code" sentinel. + /// close (ADR-055). `code` is `i32`: `ExitStatus::code()` on + /// normal exit; on Unix, the negative signal number on + /// signal-terminated exit (e.g., `-15` for SIGTERM — the crate's + /// convention, not `ExitStatus::code()`'s, which returns `None` + /// there). `-1` is the adapter's best-effort "backend could not + /// determine the exit code" sentinel — colliding with the SIGHUP + /// encoding on the Unix pipe path (ADR-055 §4). /// /// Returns [`TtySessionError::NoExitChunk`] if the session ends /// (server closed the stream) before an exit chunk is observed.