From a701247ca61ba865c588bc532fb766d411edf775 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Mon, 2 Nov 2026 08:27:32 +0000 Subject: [PATCH] chore: bump to 0.8.2, changelog for the PTY exit-signal fidelity fix - version 0.8.1 -> 0.8.2 (patch: behavior-only bug fix in the local feature's PTY waiter from db0b1fc; ADR-004 decision 1's negative-code convention now holds on the PTY path; no API, wire-format, or dependency change) - CHANGELOG entry for 0.8.2 (review #004 resolution: the waitpid reap mechanism, the signal round-trip notes, the tests, the docs) - finish the em-dash sweep in prose 58da42a missed: doc comments in tests/ (negotiation, pipe, pty, common) and fuzz/shared (control_json, negotiation_frame, session_opseq); prose rewrites only, no code, JSON literals, or wire shapes touched - README checked: no version pins, nothing to update Verification: cargo test; cargo test --all-features; cargo clippy --all-targets -D warnings (default and --all-features); cargo fmt --check; cargo doc --no-deps; fuzz/shared corpus replay (9 seeds pass); cargo check + clippy for wasm32-unknown-unknown -D warnings; cargo publish --dry-run --allow-dirty (43 files, 194.4 KiB); detached campaigns over the four targets (chunk_frame 632s, negotiation_frame 602s, control_json 610s, session_opseq 608s) all exiting 0 with 0 crash/oom/timeout findings. --- CHANGELOG.md | 51 ++++++++++++++++++++++++++++ Cargo.lock | 2 +- Cargo.toml | 2 +- fuzz/Cargo.lock | 2 +- fuzz/shared/src/control_json.rs | 2 +- fuzz/shared/src/negotiation_frame.rs | 8 ++--- fuzz/shared/src/session_opseq.rs | 18 +++++----- tests/common/mod.rs | 6 ++-- tests/negotiation.rs | 2 +- tests/pipe.rs | 8 ++--- tests/pty.rs | 6 ++-- 11 files changed, 79 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e202f51..f096086 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,56 @@ All notable changes to this crate are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this crate adheres to [Semantic Versioning](https://semver.org/). +## [0.8.2] - 2026-11-02 + +The PTY-path exit-signal fidelity fix (review #004, consumer input from +alkssh 0.1.0): a behavior-only bug fix in the `local` feature's PTY +waiter. The pipe path already conformed; the wire format, the +`TtyBackend` trait, the adapter, the session, and alktty's public API +surface are unchanged. Patch release per the bug-fix rule: the fix +brings the PTY path into conformance with the convention ADR-004 +decision 1 already documents (consumers that read the exit chunk, +notably alkssh, already consume negative codes verbatim, with no +contract change needed). + +The finding: portable-pty 0.9.0's `ExitStatus` flattens a +signal-terminated child to `code = 1` with the signal surviving only as +a strsignal name string, so the waiter's `status.exit_code() as i32` +reported a SIGTERM death as `{"type":"exit","code":1}`, +indistinguishable from a real `exit 1`, and ADR-004's own `-N` +encoding (`-15` for SIGTERM) was unrecoverable on the PTY path. The +crate's own signal features made this internally inconsistent: REQ-TTY-02's +`signal("TERM")` (the `libc::kill(-pgid, sig)` forward) and the ADR-056 +cancel cleanup's `ChildKiller::kill()` (SIGHUP) both ended in +signal-terminated reaps the waiter reported as plain `1`. + +### Fixed + +- **PTY-path exit-status encoding now honors ADR-004 decision 1: on + Unix a signal-terminated child reports the negative signal number + (e.g. `-15` for SIGTERM), Python's `Popen.returncode` parity.** The + waiter (`wait_exit_code` in `src/local/pty.rs`) skips portable-pty's + status conversion entirely on Unix and reaps the direct child via + `libc::waitpid(pid, ..)`, decoding `WIFEXITED`/`WEXITSTATUS` (normal + exit) and `WIFSIGNALED`/`WTERMSIG` (signal death → `-N`). Fully + numeric, locale-independent, and exhaustive: no name→signum table, no + `strsignal` parsing, no upstream ask, no fork. This relies on + portable-pty's unix backend returning a `std::process::Child` whose + `Drop` neither kills nor reaps, so the two reap paths never + double-reap (`Child::wait()` remains as the waitpid-failure fallback + and the non-Unix primary). The pipe path's encodings are unchanged. +- **Signal features now round-trip through the exit report.** + `signal("TERM")` resolves as `-15` on the exit chunk; the ADR-056 + cancel cleanup (SIGHUP) resolves as `-1`, which is the documented + ADR-004 sentinel collision (that collision is now common on the + cancel path, not just the wait-failure path; the `-1` → + "could not determine / exit-status(255)" consumer reading stands). +- **Tests strengthened:** the PTY-path pinning now covers `SIGINT` → + `-2`, `SIGTERM` → `-15`, and the SIGHUP fallback → `-1` collision. +- **Docs aligned:** ADR-004 decision 1 now references both backends' + encodings, and `tty-local.md`, ADR-005, and `tty-adapter.md` carry + the mechanism wording (review #004 closed as resolved). + ## [0.8.1] - 2026-10-22 The alkcall 0.10.1 adoption: a dependency bump only; no source change @@ -498,6 +548,7 @@ alknet mono-repo. the client→backend pump is aborted at session end instead of lingering until the client disconnects. +[0.8.2]: https://git.alk.dev/alkdev/alktty/releases/tag/v0.8.2 [0.8.1]: https://git.alk.dev/alkdev/alktty/releases/tag/v0.8.1 [0.8.0]: https://git.alk.dev/alkdev/alktty/releases/tag/v0.8.0 [0.7.0]: https://git.alk.dev/alkdev/alktty/releases/tag/v0.7.0 diff --git a/Cargo.lock b/Cargo.lock index 5ec3aa7..7745771 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -48,7 +48,7 @@ dependencies = [ [[package]] name = "alktty" -version = "0.8.1" +version = "0.8.2" dependencies = [ "alkcall", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index d85e7f1..6abb617 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "alktty" -version = "0.8.1" +version = "0.8.2" edition = "2021" rust-version = "1.88" license = "MIT OR Apache-2.0" diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index 399dab0..2c9c345 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -48,7 +48,7 @@ dependencies = [ [[package]] name = "alktty" -version = "0.8.1" +version = "0.8.2" dependencies = [ "alkcall", "async-trait", diff --git a/fuzz/shared/src/control_json.rs b/fuzz/shared/src/control_json.rs index e286098..d66b26d 100644 --- a/fuzz/shared/src/control_json.rs +++ b/fuzz/shared/src/control_json.rs @@ -10,7 +10,7 @@ pub fn fuzz_control_json(data: &[u8]) { Err(e) => { // Empty input yields the EOF-class serde error; non-empty // garbage may be either EOF-class (truncated JSON) or a - // syntax/class error — both are clean rejections. + // syntax/class error; both are clean rejections. if e.is_eof() { // fine on any input shape } else { diff --git a/fuzz/shared/src/negotiation_frame.rs b/fuzz/shared/src/negotiation_frame.rs index 43ea8a8..77cf920 100644 --- a/fuzz/shared/src/negotiation_frame.rs +++ b/fuzz/shared/src/negotiation_frame.rs @@ -67,7 +67,7 @@ pub fn fuzz_negotiation_frame(data: &[u8]) { // The framing layer is length-prefix-only: an extra byte // counted in the prefix still reads fine (the reader returns // raw bytes; the *adapter's* serde parse of the body is what - // would reject trailing junk — from_slice on a longer buffer + // would reject trailing junk (from_slice on a longer buffer // including the extra byte is the actual rejector). Assert the // reader consumes exactly the claimed body: the extra byte is // part of `frame`, not trailing garbage the reader ignores. @@ -89,7 +89,7 @@ fn claimed_u32(bytes: [u8; 4]) -> u32 { /// or negotiation frame under `MAX_CHUNK_LEN` starts with `0x00`, which is /// never a server-sent `stream_type`, so a `ChunkReader` peek /// distinguishes framing. A frame of exactly `MAX_CHUNK_LEN` has prefix -/// `[0x01, …]` — colliding with `STREAM_STDOUT`; the read paths admit it +/// `[0x01, …]`, colliding with `STREAM_STDOUT`; the read paths admit it /// (`>` rejects, `==` passes) and only tiny error frames are ever sent, so /// the boundary is pinned here rather than "fixed" (the framing shape is /// wire-stable). @@ -122,7 +122,7 @@ pub fn assert_error_frame_peek_disambiguation(frame: &[u8]) { /// Rebuild an error-object frame through `error_response_bytes` and assert /// the disambiguation invariant + fidelity. Only runs when the fuzz input -/// parses as a JSON object with an `"error"` string — the fuzzer's path +/// parses as a JSON object with an `"error"` string: the fuzzer's path /// into `error_response_bytes`. fn maybe_check_error_response(frame: &[u8]) { let Ok(value) = serde_json::from_slice::(frame) else { @@ -278,7 +278,7 @@ fn assert_valid_shape(data: &[u8], frame: &Bytes) { let len = u32::from_be_bytes([data[0], data[1], data[2], data[3]]); assert!( len <= MAX_CHUNK_LEN, - "accepted frame must be bounded (a zero-length frame is admitted by this reader — the adapter's serde parse rejects it — unlike alkcall's envelope framing)" + "accepted frame must be bounded: a zero-length frame is admitted by this reader, and the adapter's serde parse rejects it, unlike alkcall's envelope framing" ); assert_eq!(frame.len(), len as usize); assert_eq!( diff --git a/fuzz/shared/src/session_opseq.rs b/fuzz/shared/src/session_opseq.rs index 3a3c515..a0da80b 100644 --- a/fuzz/shared/src/session_opseq.rs +++ b/fuzz/shared/src/session_opseq.rs @@ -1,5 +1,5 @@ //! Invariants for the producer session pump (`pump_session`, driven via -//! `drive_session_pre_negotiated`) under adversarial op sequences — the +//! `drive_session_pre_negotiated`) under adversarial op sequences: the //! alktty analog of alkcall's `manager_routing` target (the stateful //! interleaving bug class, alkcall §7.8). The negotiation framing is out //! of scope here (target `negotiation_frame` covers it); the request is a @@ -7,10 +7,10 @@ //! //! Encoded invariants: //! -//! 1. **No panic** — anywhere: in the driver, in the pump tasks (asserted +//! 1. **No panic** anywhere: in the driver, in the pump tasks (asserted //! via the session task's `JoinError`), or in the harness model. //! 2. **Kill-on-Drop (ADR-005):** `kill_fired == !exit_resolved` after -//! teardown — the `exit_code` future is either driven to completion +//! teardown: the `exit_code` future is either driven to completion //! (no kill) or dropped unresolved (kill attempted). With no //! exit-resolution op issued the session is cancelled unresolved and //! the kill MUST have fired; a resolved future MUST NOT fire the kill @@ -31,19 +31,19 @@ //! (exact prefix, never a stray byte outside it); chunks issued after //! the stdin shutdown (zero-length stdin chunk, `Eof` control, or //! write-half close) are never delivered. Byte-exactness on the -//! completed path is deliberately NOT asserted — the adapter aborts +//! completed path is deliberately NOT asserted: the adapter aborts //! the input pump after the exit chunk is enqueued, so in-flight //! duplex bytes may be unprocessed (a timing window, not a bug); //! unit tests own the exact-delivery cases. //! 6. **Control dispatch:** resize/signal counters never exceed the //! issued count; each issued message is dispatched promptly while no //! exit resolution has raced the pump (the dispatch poll is skipped -//! once an exit op is issued — the pump may be aborted any time +//! once an exit op is issued; the pump may be aborted any time //! after). //! 7. **Take-once allocation:** the pre-negotiated driver allocates the //! backend exactly once per session. //! 8. **Termination:** teardown (abort + join) completes within the -//! harness bound — a wedged pump is a finding, not a hang. +//! harness bound: a wedged pump is a finding, not a hang. //! //! Harness bounds keep exec/s high: 128 ops, 4 KiB per chunk, 32 KiB //! total produced and total client-written bytes (the 64 KiB duplex per @@ -386,7 +386,7 @@ impl Harness { Ok(Err(_)) => { // The server half of the duplex is dropped only when the // session task returns, and the session returns only after - // the drainer flushed the exit chunk — so a failed write + // the drainer flushed the exit chunk, so a failed write // means the session completed while this write was in // flight. Drain and verify; a close without the exit chunk // is a premature close (a bug). @@ -1079,7 +1079,7 @@ mod corpus_replay { } /// Semantic fixture (alkcall §7.7 pattern): cancel mid-session with an - /// unresolved exit — the kill guard MUST fire (ADR-005). + /// unresolved exit; the kill guard MUST fire (ADR-005). #[test] fn cancel_with_unresolved_exit_fires_the_kill_guard() { fuzz_session_opseq(&SessionSequence { @@ -1095,7 +1095,7 @@ mod corpus_replay { } /// Semantic fixture: stdin data, then the EOF sentinel, then an `Eof` - /// control, then a further stdin chunk (the trap) — the post-EOF chunk + /// control, then a further stdin chunk (the trap); the post-EOF chunk /// must never reach the backend. #[test] fn stdin_after_eof_never_reaches_the_backend() { diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 7fc0da7..3c35c0a 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -109,7 +109,7 @@ impl ClientSide { /// Read a length-prefixed JSON error frame (Phase 1 framing, used /// for negotiation errors). The first byte of the 4-byte length - /// prefix MUST be `0x00` (the framing-disambiguation invariant — + /// prefix MUST be `0x00` (the framing-disambiguation invariant, /// ADR-052 §5); this method asserts that. Returns the parsed JSON. pub async fn read_error_frame(&mut self) -> serde_json::Value { let mut first = [0u8; 1]; @@ -305,8 +305,8 @@ pub fn nanos_seed() -> u64 { /// Poll until `path` exists (or `timeout` elapses), sleeping 10 ms /// between probes. This is the readiness signal for tests that need /// the child to have reached a state (e.g. `exec`'d) before acting on -/// it — replaces fixed pre-action sleeps, which fail spuriously on a -/// loaded machine (review #001 N4). Returns `false` on timeout. +/// it. This replaces fixed pre-action sleeps, which fail spuriously +/// on a loaded machine (review #001 N4). Returns `false` on timeout. pub async fn wait_for_file(path: &std::path::Path, timeout: Duration) -> bool { let deadline = tokio::time::Instant::now() + timeout; loop { diff --git a/tests/negotiation.rs b/tests/negotiation.rs index 95c02f2..b6147d8 100644 --- a/tests/negotiation.rs +++ b/tests/negotiation.rs @@ -1,6 +1,6 @@ #![cfg(feature = "local")] -//! Negotiation error tests — scenarios 14–18 of +//! Negotiation error tests: scenarios 14–18 of //! `tasks/tty/integration-test.md`. //! //! 14. unknown_backend: error response, first byte `0x00` diff --git a/tests/pipe.rs b/tests/pipe.rs index d24cd8e..425a4b2 100644 --- a/tests/pipe.rs +++ b/tests/pipe.rs @@ -1,6 +1,6 @@ #![cfg(feature = "local")] -//! End-to-end integration tests for pipe mode (`terminal: None`) — +//! End-to-end integration tests for pipe mode (`terminal: None`): //! `LocalTtyBackend` + `TtyAdapter::drive_session` over a //! `tokio::io::duplex` transport stand-in, running real commands. //! @@ -96,7 +96,7 @@ async fn pipe_signal_sigterm_kills_child() { .await; // Readiness signal (N4): the child exec'd `sleep` once the marker - // file exists — no fixed sleep. + // file exists; no fixed sleep. assert!( common::wait_for_file(&marker, Duration::from_secs(5)).await, "child never became ready" @@ -171,8 +171,8 @@ async fn pipe_cancel_cleanup_kills_child_no_orphan() { /// 13. Resize no-op: negotiate `cmd:["cat"]`, send a `resize` control /// chunk, assert no error (PipeControl::resize is a no-op). Close the /// write half to signal end-of-input (the adapter's input pump drops -/// the `ChildStdin` on `ConnectionClosed`, which closes the pipe — -/// tokio's `ChildStdin::poll_shutdown` is a no-op on Unix). Await exit. +/// the `ChildStdin` on `ConnectionClosed`, which closes the pipe +/// (tokio's `ChildStdin::poll_shutdown` is a no-op on Unix). Await exit. #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn pipe_resize_noop() { let backend = Arc::new(LocalTtyBackend::new()); diff --git a/tests/pty.rs b/tests/pty.rs index f3de37a..abb92cf 100644 --- a/tests/pty.rs +++ b/tests/pty.rs @@ -1,6 +1,6 @@ #![cfg(feature = "local")] -//! End-to-end integration tests for PTY mode (`terminal: Some`) — +//! End-to-end integration tests for PTY mode (`terminal: Some`): //! `LocalTtyBackend` + `TtyAdapter::drive_session` over a //! `tokio::io::duplex` transport stand-in, running real commands. //! @@ -128,7 +128,7 @@ async fn pty_signal_sigint_kills_child() { .await; // Readiness signal (N4): the child exec'd `sleep` once the marker - // file exists — no fixed sleep. + // file exists; no fixed sleep. assert!( common::wait_for_file(&marker, Duration::from_secs(5)).await, "child never became ready" @@ -154,7 +154,7 @@ async fn pty_signal_sigint_kills_child() { /// ; sleep 60'` (sleep is a child of bash, bash stays in the /// foreground waiting), wait for the marker, send `signal:"INT"`, /// assert the `sleep` child also receives the signal (the process -/// group is targeted — REQ-TTY-02). +/// group is targeted; REQ-TTY-02). #[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn pty_signal_reaches_process_group_child() {