From c2b7055a64591d371c08cf8a2a3647207e8bcd80 Mon Sep 17 00:00:00 2001 From: "glm-5.2" Date: Sat, 18 Jul 2026 17:02:59 +0000 Subject: [PATCH] refactor(tty): split control channel into STREAM_CTRL_IN/STREAM_CTRL_OUT halves (Phase 7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The single STREAM_CONTROL = 3 was documented as bidirectional but the adapter had to ignore Exit from the client because the two directions were indistinguishable on the same stream_type — half-duplex in disguise. Phase 7 splits it into two halves so the bidirectionality is literal on the wire. Changes: - wire.rs: STREAM_CTRL_IN = 3 (client→server), STREAM_CTRL_OUT = 4 (server→client); InvalidStreamType bound > 3 → > 4; Chunk::control → Chunk::ctrl_in/ctrl_out; ChunkWriter::write_control_json → write_ctrl_in_json/write_ctrl_out_json; tests split accordingly. - control.rs: ControlMessage doc updated with the stream_type column; JSON shape unchanged. - adapter.rs: pump_client_to_backend dispatches on STREAM_CTRL_IN (Resize/Signal/Eof; Exit on ctrl_in is a protocol violation, ignored); send_exit_chunk emits on STREAM_CTRL_OUT; STREAM_CTRL_OUT from the client is a protocol violation, ignored. 3 new tests for the direction enforcement; existing tests updated to the new stream_types. - negotiation.rs: framing-disambiguation doc updated (server-sent stream_type set is {1, 2, 4}). - alknet-tty-local/tests: common/mod.rs, pty.rs, pipe.rs updated to the new constants. Specs: - ADR-052 amended (§4a 'Control channel split (Phase 7 amendment)'). - tty-wire.md + tty-adapter.md updated (last_updated 2026-07-18). Verification: - cargo test -p alknet-tty: 65 passed (was 61; +4 new tests). - cargo test -p alknet-tty-local: 19 passed. - cargo test --workspace --all-features: 1017 passed, 0 failed. - cargo clippy --workspace --all-features: clean. - cargo fmt --all: clean. --- crates/alknet-tty-local/tests/common/mod.rs | 15 +- crates/alknet-tty-local/tests/pipe.rs | 2 +- crates/alknet-tty-local/tests/pty.rs | 4 +- crates/alknet-tty/src/adapter.rs | 170 +++++++++++++++--- crates/alknet-tty/src/control.rs | 61 ++++--- crates/alknet-tty/src/negotiation.rs | 6 +- crates/alknet-tty/src/wire.rs | 111 +++++++++--- docs/architecture/crates/tty/tty-adapter.md | 69 +++++-- docs/architecture/crates/tty/tty-wire.md | 113 ++++++++---- ...alknet-tty-wire-format-and-two-carriage.md | 64 ++++++- .../alknet-crate-extraction/findings.md | 98 +++++++++- 11 files changed, 571 insertions(+), 142 deletions(-) diff --git a/crates/alknet-tty-local/tests/common/mod.rs b/crates/alknet-tty-local/tests/common/mod.rs index 37a5414..f8970c4 100644 --- a/crates/alknet-tty-local/tests/common/mod.rs +++ b/crates/alknet-tty-local/tests/common/mod.rs @@ -21,7 +21,9 @@ use std::sync::Arc; use alknet_core::auth::Identity; use alknet_tty::adapter::drive_session; use alknet_tty::backend::TtyBackend; -use alknet_tty::wire::{ChunkReader, STREAM_CONTROL, STREAM_STDERR, STREAM_STDOUT}; +use alknet_tty::wire::{ + ChunkReader, STREAM_CTRL_IN, STREAM_CTRL_OUT, STREAM_STDERR, STREAM_STDOUT, +}; use bytes::Bytes; use tokio::io::duplex; use tokio::io::{AsyncReadExt, AsyncWriteExt}; @@ -70,10 +72,11 @@ impl ClientSide { self.write.flush().await.unwrap(); } - /// Write a control chunk (stream_type 3) carrying a serialized - /// `ControlMessage` JSON payload. + /// Write a client→server control chunk (`STREAM_CTRL_IN`, stream_type + /// 3) carrying a serialized `ControlMessage` JSON payload (`Resize`, + /// `Signal`, or `Eof`). pub async fn write_control(&mut self, json: &[u8]) { - self.write_chunk(STREAM_CONTROL, json).await; + self.write_chunk(STREAM_CTRL_IN, json).await; } /// Read one raw chunk from the server. Returns the `stream_type` @@ -145,7 +148,7 @@ impl ClientSide { stderr.extend_from_slice(&bytes); } } - STREAM_CONTROL => { + STREAM_CTRL_OUT => { let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); if v["type"] == "exit" { return Some((stdout, stderr, v["code"].as_i64().unwrap() as i32)); @@ -177,7 +180,7 @@ impl ClientSide { stderr.extend_from_slice(&bytes); } } - STREAM_CONTROL => { + STREAM_CTRL_OUT => { let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); if v["type"] == "exit" { return Some((stdout, stderr, v["code"].as_i64().unwrap() as i32)); diff --git a/crates/alknet-tty-local/tests/pipe.rs b/crates/alknet-tty-local/tests/pipe.rs index 88c950f..11d1ca7 100644 --- a/crates/alknet-tty-local/tests/pipe.rs +++ b/crates/alknet-tty-local/tests/pipe.rs @@ -199,7 +199,7 @@ async fn pipe_echo_emits_stdout_chunk_then_sentinel() { if st == STREAM_STDOUT && !bytes.is_empty() { saw_nonempty_stdout = true; } - if st == alknet_tty::wire::STREAM_CONTROL { + if st == alknet_tty::wire::STREAM_CTRL_OUT { let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); if v["type"] == "exit" { break; diff --git a/crates/alknet-tty-local/tests/pty.rs b/crates/alknet-tty-local/tests/pty.rs index f0b0ec9..ed59074 100644 --- a/crates/alknet-tty-local/tests/pty.rs +++ b/crates/alknet-tty-local/tests/pty.rs @@ -17,7 +17,7 @@ mod common; use std::sync::Arc; use std::time::Duration; -use alknet_tty::wire::{STREAM_CONTROL, STREAM_STDIN}; +use alknet_tty::wire::{STREAM_CTRL_OUT, STREAM_STDIN}; use alknet_tty_local::LocalTtyBackend; use common::{negotiate_pty_json, spawn_session}; @@ -245,7 +245,7 @@ async fn pty_exit_chunk_is_last() { if saw_exit { panic!("chunk arrived after exit: stream_type={st}, bytes={bytes:?} (ADR-055)"); } - if st == STREAM_CONTROL { + if st == STREAM_CTRL_OUT { let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); if v["type"] == "exit" { assert_eq!(v["code"], 0); diff --git a/crates/alknet-tty/src/adapter.rs b/crates/alknet-tty/src/adapter.rs index efa7d50..9ce4882 100644 --- a/crates/alknet-tty/src/adapter.rs +++ b/crates/alknet-tty/src/adapter.rs @@ -22,19 +22,34 @@ //! (stream_type 2) when `TtyHandle.stderr` is `Some`. On backend stdout //! EOF, emit a zero-length stdout sentinel. //! - **B. client → backend**: stdin chunks (stream_type 0) → -//! `TtyHandle.stdin`; control chunks (stream_type 3) → -//! `ControlMessage` dispatch (`Resize`, `Signal`, `Eof`; `Exit` is -//! server→client only and ignored). Zero-length stdin chunk or -//! read-half close → EOF to backend stdin. +//! `TtyHandle.stdin`; client→server control chunks (stream_type 3, +//! `STREAM_CTRL_IN`) → `ControlMessage` dispatch (`Resize`, `Signal`, +//! `Eof`). `STREAM_CTRL_OUT` (stream_type 4) from the client is a +//! protocol violation (it's the server→client half) and is ignored; +//! `Exit` arriving on `STREAM_CTRL_IN` is likewise a protocol +//! violation and ignored. Zero-length stdin chunk or read-half close +//! → EOF to backend stdin. //! - **C. exit → exit chunk**: await `TtyHandle.exit_code`; on resolve, -//! enqueue `{"type":"exit","code":N}` as a control chunk (stream_type -//! 3). On `TtyError` → `{"type":"exit","code":-1}`. +//! enqueue `{"type":"exit","code":N}` as a server→client control +//! chunk (stream_type 4, `STREAM_CTRL_OUT`). On `TtyError` → +//! `{"type":"exit","code":-1}`. //! //! The adapter enforces the **exit-chunk-is-last** invariant (ADR-055): //! it waits for BOTH the stdout/stderr pumps to complete AND `exit_code` //! to resolve before enqueueing the exit chunk. A drainer task writes //! chunks to the client in arrival order; the exit chunk is last. //! +//! # Bidirectional control channel (Phase 7) +//! +//! The control channel is split into two halves so it is genuinely +//! bidirectional on the wire: `STREAM_CTRL_IN = 3` carries client→server +//! control (`Resize`, `Signal`, `Eof`); `STREAM_CTRL_OUT = 4` carries +//! server→client control (`Exit`). The previous single `STREAM_CONTROL = +//! 3` was documented as "bidirectional" but the adapter ignored `Exit` +//! from the client because the two directions were indistinguishable on +//! the same stream_type. The split makes the bidirectionality explicit +//! — see `docs/research/alknet-crate-extraction/findings.md` Phase 7. +//! //! # Cancel cleanup (ADR-056) //! //! On connection drop or stream reset, the pump tasks are dropped, which @@ -63,7 +78,7 @@ use crate::control::ControlMessage; use crate::negotiation::{ error_response_bytes, NegotiateRequest, NegotiationError, NegotiationReader, NegotiationWriter, }; -use crate::wire::{Chunk, ChunkReader, ChunkWriter, RawError, STREAM_CONTROL, STREAM_STDIN}; +use crate::wire::{Chunk, ChunkReader, ChunkWriter, RawError, STREAM_CTRL_IN, STREAM_STDIN}; /// The scope required to open a `alknet/tty` session (ADR-050). A two-way-door /// choice (reversible: a deployment-configured scope, not a wire-format @@ -378,7 +393,7 @@ async fn send_exit_chunk(writer_tx: &mpsc::Sender, code: i32) { let exit_msg = ControlMessage::Exit { code }; match exit_msg.to_json() { Ok(json) => { - let chunk = Chunk::control(json); + let chunk = Chunk::ctrl_out(json); if writer_tx.send(chunk).await.is_err() { debug!("tty: writer channel closed before exit chunk"); } @@ -423,9 +438,25 @@ async fn pump_stderr( debug!("tty: stderr pump done"); } -/// Pump client chunks → backend: stdin chunks → `TtyHandle.stdin`, control -/// chunks → `ControlMessage` dispatch. On client read-half close or a -/// zero-length stdin chunk, signal EOF to the backend's stdin. +/// Pump client chunks → backend: stdin chunks → `TtyHandle.stdin`, +/// client→server control chunks (`STREAM_CTRL_IN`, stream_type 3) → +/// `ControlMessage` dispatch. On client read-half close or a zero-length +/// stdin chunk, signal EOF to the backend's stdin. +/// +/// # Direction enforcement (Phase 7) +/// +/// The control channel is split into two halves. This pump reads from +/// the client, so it dispatches only `STREAM_CTRL_IN` (client→server): +/// +/// - `Resize` / `Signal` / `Eof` → forward to the backend's control +/// handle (`TtyControlHandle::resize` / `signal` / `stdin.shutdown`). +/// - `Exit` arriving on `STREAM_CTRL_IN` is a protocol violation +/// (`Exit` is server→client only, belongs on `STREAM_CTRL_OUT`); the +/// adapter ignores it. (The previous single `STREAM_CONTROL = 3` +/// couldn't distinguish the two directions, so `Exit` from the client +/// was always ignored — the split makes the rejection explicit.) +/// - `STREAM_CTRL_OUT` (stream_type 4) from the client is a protocol +/// violation (it's the server→client half); the adapter ignores it. async fn pump_client_to_backend( client_read: R, mut stdin: Box, @@ -446,7 +477,7 @@ async fn pump_client_to_backend( break; } } - STREAM_CONTROL => match ControlMessage::from_slice(&chunk.bytes) { + STREAM_CTRL_IN => match ControlMessage::from_slice(&chunk.bytes) { Ok(ControlMessage::Resize { cols, rows, @@ -467,12 +498,21 @@ async fn pump_client_to_backend( debug!("tty: client stdin EOF (eof control)"); } Ok(ControlMessage::Exit { .. }) => { - debug!("tty: ignoring Exit control from client (server→client only)"); + debug!( + "tty: ignoring Exit control on STREAM_CTRL_IN \ + (server→client only; belongs on STREAM_CTRL_OUT)" + ); } Err(e) => { debug!("tty: ignoring unknown control type: {e}"); } }, + crate::wire::STREAM_CTRL_OUT => { + debug!( + "tty: ignoring STREAM_CTRL_OUT (stream_type 4) from client \ + (server→client half; client should not write on it)" + ); + } other => { debug!("tty: ignoring stream_type {other} from client"); } @@ -868,7 +908,7 @@ mod tests { assert!(bytes.is_empty()); let (st, bytes) = client.read_chunk().await; - assert_eq!(st, crate::wire::STREAM_CONTROL); + assert_eq!(st, crate::wire::STREAM_CTRL_OUT); let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); assert_eq!(v["type"], "exit"); assert_eq!(v["code"], 0); @@ -899,7 +939,7 @@ mod tests { loop { let (st, bytes) = client.read_chunk().await; - if st == crate::wire::STREAM_CONTROL { + if st == crate::wire::STREAM_CTRL_OUT { let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); if v["type"] == "exit" { assert_eq!(v["code"], 7); @@ -959,13 +999,13 @@ mod tests { client .write_chunk( - crate::wire::STREAM_CONTROL, + crate::wire::STREAM_CTRL_IN, br#"{"type":"resize","cols":100,"rows":50}"#, ) .await; client .write_chunk( - crate::wire::STREAM_CONTROL, + crate::wire::STREAM_CTRL_IN, br#"{"type":"signal","name":"INT"}"#, ) .await; @@ -1003,7 +1043,7 @@ mod tests { client.write_negotiation(TEST_NEG).await; client - .write_chunk(crate::wire::STREAM_CONTROL, br#"{"type":"unknown"}"#) + .write_chunk(crate::wire::STREAM_CTRL_IN, br#"{"type":"unknown"}"#) .await; let stdout_tx = backend.take_stdout_tx().await.expect("stdout tx"); @@ -1025,6 +1065,9 @@ mod tests { #[tokio::test] async fn exit_control_from_client_ignored() { + // `Exit` is server→client only (belongs on `STREAM_CTRL_OUT`). + // Sending it on `STREAM_CTRL_IN` (client→server) is a protocol + // violation; the adapter ignores it and keeps pumping stdout. let (backend, _control, _cancel) = TestBackend::builder().build(); let backends = make_backends(backend.clone()); let (mut client, server) = make_client_and_server(); @@ -1037,7 +1080,7 @@ mod tests { client.write_negotiation(TEST_NEG).await; client - .write_chunk(crate::wire::STREAM_CONTROL, br#"{"type":"exit","code":99}"#) + .write_chunk(crate::wire::STREAM_CTRL_IN, br#"{"type":"exit","code":99}"#) .await; let stdout_tx = backend.take_stdout_tx().await.expect("stdout tx"); @@ -1057,6 +1100,87 @@ mod tests { let _ = session.await; } + #[tokio::test] + async fn ctrl_out_from_client_ignored() { + // `STREAM_CTRL_OUT` (stream_type 4) is the server→client half. + // The client writing on it is a protocol violation; the adapter + // ignores the chunk and keeps pumping stdout (Phase 7). + let (backend, _control, _cancel) = TestBackend::builder().build(); + let backends = make_backends(backend.clone()); + let (mut client, server) = make_client_and_server(); + + let identity = identity_with_scope(TTY_OPEN_SCOPE); + let session = tokio::spawn(async move { + drive_session_server(server, backends, None, identity).await; + }); + + client.write_negotiation(TEST_NEG).await; + + // Bogus: a client writing on the server→client control half. + client + .write_chunk( + crate::wire::STREAM_CTRL_OUT, + br#"{"type":"exit","code":99}"#, + ) + .await; + + let stdout_tx = backend.take_stdout_tx().await.expect("stdout tx"); + let _ = backend.take_stderr_tx().await; + stdout_tx + .send(Bytes::from_static(b"after-bogus-ctrl-out")) + .await + .unwrap(); + drop(stdout_tx); + let exit_tx = backend.take_exit_tx().await.expect("exit tx"); + exit_tx.send(Ok(0)).unwrap(); + + let (st, bytes) = client.read_chunk().await; + assert_eq!(st, STREAM_STDOUT); + assert_eq!(bytes.as_ref(), b"after-bogus-ctrl-out"); + + let _ = session.await; + } + + #[tokio::test] + async fn exit_chunk_arrives_on_ctrl_out_not_ctrl_in() { + // Verifies the adapter emits `Exit` on `STREAM_CTRL_OUT` (4), not + // `STREAM_CTRL_IN` (3) — the Phase 7 bidirectionality fix. A client + // distinguishing the two halves can route exit vs. control + // without parsing the JSON tag first. + let (backend, _control, _cancel) = TestBackend::builder().build(); + let backends = make_backends(backend.clone()); + let (mut client, server) = make_client_and_server(); + + let identity = identity_with_scope(TTY_OPEN_SCOPE); + let session = tokio::spawn(async move { + drive_session_server(server, backends, None, identity).await; + }); + + client.write_negotiation(TEST_NEG).await; + + let stdout_tx = backend.take_stdout_tx().await.expect("stdout tx"); + let _ = backend.take_stderr_tx().await; + drop(stdout_tx); + let exit_tx = backend.take_exit_tx().await.expect("exit tx"); + exit_tx.send(Ok(42)).unwrap(); + + let (st, bytes) = client.read_chunk().await; + assert_eq!(st, STREAM_STDOUT); + assert!(bytes.is_empty()); + + let (st, bytes) = client.read_chunk().await; + assert_eq!( + st, + crate::wire::STREAM_CTRL_OUT, + "exit chunk must arrive on STREAM_CTRL_OUT (4), not STREAM_CTRL_IN (3)" + ); + let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(v["type"], "exit"); + assert_eq!(v["code"], 42); + + let _ = session.await; + } + #[tokio::test] async fn unknown_backend_error() { let (backend, _control, _cancel) = TestBackend::builder().build(); @@ -1184,7 +1308,7 @@ mod tests { loop { let (st, bytes) = client.read_chunk().await; - if st == crate::wire::STREAM_CONTROL { + if st == crate::wire::STREAM_CTRL_OUT { let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); assert_eq!(v["type"], "exit"); assert_eq!(v["code"], -1); @@ -1294,7 +1418,7 @@ mod tests { loop { let (st, bytes) = client.read_chunk().await; - if st == crate::wire::STREAM_CONTROL { + if st == crate::wire::STREAM_CTRL_OUT { let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); assert_eq!(v["type"], "exit"); assert_eq!(v["code"], 0); @@ -1346,7 +1470,7 @@ mod tests { assert_eq!(bytes.as_ref(), b"err"); saw_stderr = true; } - crate::wire::STREAM_CONTROL => { + crate::wire::STREAM_CTRL_OUT => { let v: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); assert_eq!(v["type"], "exit"); saw_exit = true; @@ -1379,7 +1503,7 @@ mod tests { client.write_chunk(STREAM_STDIN, b"first").await; client - .write_chunk(crate::wire::STREAM_CONTROL, br#"{"type":"eof"}"#) + .write_chunk(crate::wire::STREAM_CTRL_IN, br#"{"type":"eof"}"#) .await; let mut received = Vec::new(); diff --git a/crates/alknet-tty/src/control.rs b/crates/alknet-tty/src/control.rs index 78c3215..036fe7f 100644 --- a/crates/alknet-tty/src/control.rs +++ b/crates/alknet-tty/src/control.rs @@ -1,4 +1,14 @@ -//! Control messages carried in `stream_type 3` chunks (ADR-052). +//! Control messages carried in `stream_type 3` (`ctrl_in`) and +//! `stream_type 4` (`ctrl_out`) chunks (ADR-052, amended Phase 7). +//! +//! The control channel is split into two halves so it is genuinely +//! bidirectional on the wire: `STREAM_CTRL_IN = 3` carries client→server +//! control (`Resize`, `Signal`, `Eof`); `STREAM_CTRL_OUT = 4` carries +//! server→client control (`Exit`). The previous single +//! `STREAM_CONTROL = 3` was documented as "bidirectional" but the adapter +//! ignored `Exit` from the client because it had no way to distinguish +//! the two directions on the same stream_type — see +//! `docs/research/alknet-crate-extraction/findings.md` Phase 7. //! //! Control chunks carry a JSON payload tagged by `type`. The schema is the //! POC's `ControlMessage` (`/workspace/alknet-tty-poc/src/control.rs`): @@ -23,20 +33,30 @@ use serde::{Deserialize, Serialize}; -/// A control message riding on `stream_type 3`. +/// A control message riding on `STREAM_CTRL_IN` (stream_type 3, +/// client→server) or `STREAM_CTRL_OUT` (stream_type 4, server→client). /// /// Direction and mapping (per `tty-wire.md` §"Control Channel"): /// -/// | direction | variant | maps to | -/// |----------------|----------|----------------------------------------------------| -/// | client→server | `Resize` | SSH `window-change`, docker exec resize, `ioctl` | -/// | client→server | `Signal` | SSH `signal`, docker exec signal, `kill(-pgid, n)` | -/// | client→server | `Eof` | SSH channel EOF, docker stdin close, `ChildStdin` | -/// | server→client | `Exit` | the completion signal (ADR-055) | +/// | direction | stream_type | variant | maps to | +/// |----------------|-----------------|----------|----------------------------------------------------| +/// | client→server | `STREAM_CTRL_IN` (3) | `Resize` | SSH `window-change`, docker exec resize, `ioctl` | +/// | client→server | `STREAM_CTRL_IN` (3) | `Signal` | SSH `signal`, docker exec signal, `kill(-pgid, n)` | +/// | client→server | `STREAM_CTRL_IN` (3) | `Eof` | SSH channel EOF, docker stdin close, `ChildStdin` | +/// | server→client | `STREAM_CTRL_OUT` (4) | `Exit` | the completion signal (ADR-055) | +/// +/// The direction is enforced by the adapter, not by this enum: a `Resize` +/// arriving on `STREAM_CTRL_OUT` is a protocol violation (the adapter +/// ignores it), and an `Exit` arriving on `STREAM_CTRL_IN` is likewise a +/// protocol violation (the adapter ignores it). The split is what makes +/// the control channel genuinely bidirectional — the previous single +/// `STREAM_CONTROL = 3` was documented as "bidirectional" but the +/// adapter had to ignore `Exit` from the client because the two +/// directions were indistinguishable on the same stream_type. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "snake_case")] pub enum ControlMessage { - /// Terminal window resize (client→server). + /// Terminal window resize (client→server, `STREAM_CTRL_IN`). /// /// `pixel_width`/`pixel_height` default to 0 (most terminals don't /// report pixel dimensions; SSH's `pty_request` carries them for @@ -49,23 +69,24 @@ pub enum ControlMessage { #[serde(default)] pixel_height: u16, }, - /// Forward a signal to the child process group (client→server). + /// Forward a signal to the child process group (client→server, + /// `STREAM_CTRL_IN`). /// /// `name` is an uppercase string from the supported set (see /// [`signal_from_name`]). Unknown names fall back to the backend's /// default kill in the adapter (tty-local.md REQ-TTY-02). Signal { name: String }, - /// Client stdin is done (client→server). The server closes the - /// backend's stdin (`ChildStdin::drop` / PTY writer close) but keeps - /// pumping stdout + the exit chunk. See `tty-wire.md` §"Stdin - /// Closure". + /// Client stdin is done (client→server, `STREAM_CTRL_IN`). The + /// server closes the backend's stdin (`ChildStdin::drop` / PTY writer + /// close) but keeps pumping stdout + the exit chunk. See + /// `tty-wire.md` §"Stdin Closure". Eof, - /// Process exit code (server→client). 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). + /// 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). Exit { code: i32 }, } diff --git a/crates/alknet-tty/src/negotiation.rs b/crates/alknet-tty/src/negotiation.rs index 9627722..86d942e 100644 --- a/crates/alknet-tty/src/negotiation.rs +++ b/crates/alknet-tty/src/negotiation.rs @@ -21,8 +21,10 @@ //! - An error frame's 4-byte big-endian length prefix starts with `0x00` //! because error frames MUST be under 16 MiB ([`MAX_CHUNK_LEN`]) so the //! high byte is zero (a wire-format invariant, not an assumption). -//! - A raw chunk's first byte is a `stream_type` in `{1, 2, 3}` — -//! `0` (stdin from server) is invalid, so `0x00` is unambiguous. +//! - A raw chunk's first byte is a `stream_type`. The server never sends +//! `0` (stdin — client→server only) or `3` (`STREAM_CTRL_IN` — +//! client→server only), so the server-sent set is `{1, 2, 4}` +//! (stdout, stderr, `STREAM_CTRL_OUT`); `0x00` is unambiguous. //! //! See ADR-052 §5 and `tty-wire.md` §"Constraints". diff --git a/crates/alknet-tty/src/wire.rs b/crates/alknet-tty/src/wire.rs index 5aadd53..2ed402f 100644 --- a/crates/alknet-tty/src/wire.rs +++ b/crates/alknet-tty/src/wire.rs @@ -7,10 +7,20 @@ //! ``` //! //! `stream_type`: -//! - 0 = stdin (client→server, raw bytes) -//! - 1 = stdout (server→client, raw bytes) -//! - 2 = stderr (server→client, raw bytes) -//! - 3 = control (bidirectional, JSON control message — see [`crate::control`]) +//! - 0 = stdin (client→server, raw bytes) +//! - 1 = stdout (server→client, raw bytes) +//! - 2 = stderr (server→client, raw bytes) +//! - 3 = ctrl_in (client→server, JSON control message — see [`crate::control`]) +//! - 4 = ctrl_out (server→client, JSON control message — see [`crate::control`]) +//! +//! The control channel is split into two halves so it is genuinely +//! bidirectional on the wire: `STREAM_CTRL_IN = 3` carries client→server +//! control (resize, signal, eof); `STREAM_CTRL_OUT = 4` carries +//! server→client control (exit). The previous single `STREAM_CONTROL = 3` +//! was documented as "bidirectional" but the adapter ignored `Exit` from +//! the client because it had no way to distinguish the two directions on +//! the same stream_type — see `docs/research/alknet-crate-extraction/ +//! findings.md` Phase 7. //! //! Zero-length data chunks are sentinels: a zero-length stdin chunk is EOF //! from the client; a zero-length stdout chunk is "drained" from the @@ -29,8 +39,11 @@ pub const STREAM_STDIN: u8 = 0; pub const STREAM_STDOUT: u8 = 1; /// stderr channel (server→client, raw bytes). pub const STREAM_STDERR: u8 = 2; -/// control channel (bidirectional, JSON control message). -pub const STREAM_CONTROL: u8 = 3; +/// Control channel, client→server half (JSON control message — +/// `Resize`, `Signal`, `Eof`). +pub const STREAM_CTRL_IN: u8 = 3; +/// Control channel, server→client half (JSON control message — `Exit`). +pub const STREAM_CTRL_OUT: u8 = 4; /// Chunk header length in bytes: 1 byte `stream_type` + 4 bytes `length`. pub const CHUNK_HEADER_LEN: usize = 5; @@ -54,7 +67,7 @@ pub enum RawError { /// The peer closed the stream cleanly (unexpected EOF on header or payload). #[error("connection closed")] ConnectionClosed, - /// The chunk header's `stream_type` byte was > 3. + /// The chunk header's `stream_type` byte was > 4. #[error("invalid chunk header: stream type {0}")] InvalidStreamType(u8), /// The chunk payload length exceeded `MAX_CHUNK_LEN`. @@ -66,11 +79,12 @@ pub enum RawError { /// payload bytes. /// /// Construct with [`Chunk::stdin`], [`Chunk::stdout`], [`Chunk::stderr`], -/// or [`Chunk::control`] for the four fixed channels. +/// [`Chunk::ctrl_in`], or [`Chunk::ctrl_out`] for the five fixed +/// channels. #[derive(Debug, Clone)] pub struct Chunk { /// The channel: one of [`STREAM_STDIN`], [`STREAM_STDOUT`], - /// [`STREAM_STDERR`], [`STREAM_CONTROL`]. + /// [`STREAM_STDERR`], [`STREAM_CTRL_IN`], [`STREAM_CTRL_OUT`]. pub stream_type: u8, /// The payload bytes (raw for data channels, UTF-8 JSON for control). pub bytes: bytes::Bytes, @@ -101,10 +115,19 @@ impl Chunk { } } - /// A control chunk (stream_type 3). - pub fn control(bytes: bytes::Bytes) -> Self { + /// A client→server control chunk (stream_type 3) — `Resize`, `Signal`, + /// or `Eof`. + pub fn ctrl_in(bytes: bytes::Bytes) -> Self { Self { - stream_type: STREAM_CONTROL, + stream_type: STREAM_CTRL_IN, + bytes, + } + } + + /// A server→client control chunk (stream_type 4) — `Exit`. + pub fn ctrl_out(bytes: bytes::Bytes) -> Self { + Self { + stream_type: STREAM_CTRL_OUT, bytes, } } @@ -113,7 +136,7 @@ impl Chunk { /// Reads raw chunks from an [`AsyncRead`] transport. /// /// [`ChunkReader::read_chunk`] reads the 5-byte header, validates the -/// `stream_type` (≤ 3, else [`RawError::InvalidStreamType`]) and the +/// `stream_type` (≤ 4, else [`RawError::InvalidStreamType`]) and the /// payload length (≤ [`MAX_CHUNK_LEN`], else [`RawError::ChunkTooLarge`]), /// then reads the payload. On a clean `UnexpectedEof` reading either the /// header or the payload, it returns [`RawError::ConnectionClosed`] — the @@ -148,7 +171,7 @@ impl ChunkReader { } let stream_type = self.header[0]; - if stream_type > 3 { + if stream_type > 4 { return Err(RawError::InvalidStreamType(stream_type)); } @@ -183,9 +206,10 @@ impl ChunkReader { /// Writes raw chunks to an [`AsyncWrite`] transport. /// /// [`ChunkWriter::write_chunk`] writes the 5-byte header then the payload -/// (if non-empty), then flushes. [`ChunkWriter::write_stdin`] and -/// [`ChunkWriter::write_control_json`] are convenience helpers for the -/// two most common write paths. +/// (if non-empty), then flushes. [`ChunkWriter::write_stdin`], +/// [`ChunkWriter::write_ctrl_in_json`], and +/// [`ChunkWriter::write_ctrl_out_json`] are convenience helpers for the +/// most common write paths. pub struct ChunkWriter { writer: W, } @@ -229,10 +253,24 @@ impl ChunkWriter { Ok(()) } - /// Write a control chunk (stream_type 3) carrying a JSON payload. - pub async fn write_control_json(&mut self, json: &[u8]) -> Result<(), RawError> { + /// Write a client→server control chunk (stream_type 3) carrying a JSON + /// payload (`Resize`, `Signal`, or `Eof`). + pub async fn write_ctrl_in_json(&mut self, json: &[u8]) -> Result<(), RawError> { let mut header = [0u8; CHUNK_HEADER_LEN]; - header[0] = STREAM_CONTROL; + header[0] = STREAM_CTRL_IN; + let len = json.len() as u32; + header[1..].copy_from_slice(&len.to_be_bytes()); + self.writer.write_all(&header).await?; + self.writer.write_all(json).await?; + self.writer.flush().await?; + Ok(()) + } + + /// Write a server→client control chunk (stream_type 4) carrying a JSON + /// payload (`Exit`). + pub async fn write_ctrl_out_json(&mut self, json: &[u8]) -> Result<(), RawError> { + let mut header = [0u8; CHUNK_HEADER_LEN]; + header[0] = STREAM_CTRL_OUT; let len = json.len() as u32; header[1..].copy_from_slice(&len.to_be_bytes()); self.writer.write_all(&header).await?; @@ -281,8 +319,13 @@ mod tests { } #[tokio::test] - async fn round_trip_control() { - round_trip(STREAM_CONTROL, br#"{"type":"eof"}"#).await; + async fn round_trip_ctrl_in() { + round_trip(STREAM_CTRL_IN, br#"{"type":"eof"}"#).await; + } + + #[tokio::test] + async fn round_trip_ctrl_out() { + round_trip(STREAM_CTRL_OUT, br#"{"type":"exit","code":0}"#).await; } #[tokio::test] @@ -303,27 +346,41 @@ mod tests { } #[tokio::test] - async fn round_trip_write_control_json_helper() { + async fn round_trip_write_ctrl_in_json_helper() { let (mut a, mut b) = duplex(8 * 1024); let mut writer = ChunkWriter::new(&mut a); let mut reader = ChunkReader::new(&mut b); let json = br#"{"type":"resize","cols":80,"rows":24}"#; - writer.write_control_json(json).await.unwrap(); + writer.write_ctrl_in_json(json).await.unwrap(); let read = reader.read_chunk().await.unwrap(); - assert_eq!(read.stream_type, STREAM_CONTROL); + assert_eq!(read.stream_type, STREAM_CTRL_IN); + assert_eq!(read.bytes.as_ref(), json); + } + + #[tokio::test] + async fn round_trip_write_ctrl_out_json_helper() { + let (mut a, mut b) = duplex(8 * 1024); + let mut writer = ChunkWriter::new(&mut a); + let mut reader = ChunkReader::new(&mut b); + + let json = br#"{"type":"exit","code":0}"#; + writer.write_ctrl_out_json(json).await.unwrap(); + let read = reader.read_chunk().await.unwrap(); + assert_eq!(read.stream_type, STREAM_CTRL_OUT); assert_eq!(read.bytes.as_ref(), json); } #[tokio::test] async fn invalid_stream_type() { let (mut a, mut b) = duplex(8 * 1024); - a.write_all(&[4u8, 0, 0, 0, 0]).await.unwrap(); + // 5 is one past the highest valid stream_type (4 = STREAM_CTRL_OUT). + a.write_all(&[5u8, 0, 0, 0, 0]).await.unwrap(); a.flush().await.unwrap(); let mut reader = ChunkReader::new(&mut b); let err = reader.read_chunk().await.unwrap_err(); - assert!(matches!(err, RawError::InvalidStreamType(4))); + assert!(matches!(err, RawError::InvalidStreamType(5))); } #[tokio::test] diff --git a/docs/architecture/crates/tty/tty-adapter.md b/docs/architecture/crates/tty/tty-adapter.md index c67a404..9258457 100644 --- a/docs/architecture/crates/tty/tty-adapter.md +++ b/docs/architecture/crates/tty/tty-adapter.md @@ -1,6 +1,6 @@ --- status: draft -last_updated: 2026-07-07 +last_updated: 2026-07-18 --- # alknet-tty — TtyAdapter and Session Lifecycle @@ -100,14 +100,18 @@ A `alknet/tty` session on one bidi stream proceeds in three phases: `Some`, a concurrent stderr pump emits stderr chunks (stream_type 2). On backend stdout EOF, emit a zero-length stdout sentinel. - **B. client → backend**: client chunks → backend. stdin chunks - (stream_type 0) → `TtyHandle.stdin` (via `AsyncWrite`). Control - chunks (stream_type 3) → `ControlMessage` dispatch: `Resize` → - `TtyControl::resize`, `Signal` → `TtyControl::signal`, `Eof` → - close stdin. `Exit` from the client is ignored (server→client only). + (stream_type 0) → `TtyHandle.stdin` (via `AsyncWrite`). + Client→server control chunks (`STREAM_CTRL_IN`, stream_type 3) → + `ControlMessage` dispatch: `Resize` → `TtyControl::resize`, `Signal` + → `TtyControl::signal`, `Eof` → close stdin. `Exit` on + `STREAM_CTRL_IN` is a protocol violation (it's server→client only) + and is ignored. `STREAM_CTRL_OUT` (stream_type 4) from the client is + a protocol violation (it's the server→client half) and is ignored. On client read-half close or a zero-length stdin chunk, signal EOF to the backend's stdin. - **C. exit → exit chunk**: await `TtyHandle.exit_code`; on resolve, - enqueue `{"type":"exit","code":N}` as a control chunk (stream_type 3). + enqueue `{"type":"exit","code":N}` as a server→client control + chunk (`STREAM_CTRL_OUT`, stream_type 4). A drainer task writes chunks to the client in arrival order. After the exit chunk is written (task C resolves and the exit chunk drains), @@ -118,6 +122,36 @@ hardcoded the local PTY backend; the adapter dispatches to any `TtyBackend`. See `/workspace/alknet-tty-poc/src/session.rs` for the reference implementation of the three-pump driver. +### Bidirectional Control Channel (Phase 7) + +The control channel is split into two halves so it is genuinely +bidirectional on the wire: + +- **`STREAM_CTRL_IN = 3`** — client→server control (`Resize`, `Signal`, + `Eof`). +- **`STREAM_CTRL_OUT = 4`** — server→client control (`Exit`). + +The adapter enforces the direction: + +- An `Exit` arriving on `STREAM_CTRL_IN` is a protocol violation + (server→client message on the client→server half) — the adapter + ignores it (the previous single `STREAM_CONTROL = 3` could not + distinguish the two directions, so `Exit` from the client was always + ignored; the split makes the rejection explicit). +- A `Resize`/`Signal`/`Eof` arriving on `STREAM_CTRL_OUT` is a protocol + violation (client→server message on the server→client half) — the + adapter ignores it (the server never dispatches control messages it + receives on the server→client half). +- `STREAM_CTRL_OUT` (stream_type 4) chunks written by the client are a + protocol violation (the client should not write on the server→client + half) — the adapter ignores them. + +The exit chunk (`Exit`) is emitted on `STREAM_CTRL_OUT` (stream_type +4), not on the previous `STREAM_CONTROL = 3`. A client distinguishing +the two halves can route exit vs. control without parsing the JSON +`type` tag first. See `docs/research/alknet-crate-extraction/findings.md` +Phase 7 and `tty-wire.md` §"Control Channel". + ### Negotiation Errors If the server cannot allocate the session, it sends a JSON error response @@ -146,17 +180,18 @@ The disambiguation is by the first byte: a JSON error frame's 4-byte big-endian length prefix always starts with `0x00` (error frames MUST be under 16 MiB — `MAX_CHUNK_LEN` — so the high byte is zero; this is a wire-format invariant, not an assumption), while a raw chunk's first -byte is a `stream_type` in `{0, 1, 2, 3}`. A stream_type of `0` (stdin -from server) is invalid — the server never sends stdin chunks — so the -client distinguishes: read the first byte; if it is `0x00`, interpret -the next 4 bytes as a big-endian length prefix and read that many bytes -as a JSON error frame; otherwise interpret it as a `stream_type` byte -and continue reading the raw chunk header. This is a one-way-door -wire-format invariant (ADR-052): error frames use the negotiation -framing (length prefix) and MUST be under 16 MiB; success uses the raw -chunk framing (stream_type byte first); the `0x00`-as-length-prefix vs -`0x00`-as-invalid-stream_type disambiguation is what makes the two -distinguishable on the wire. +byte is a `stream_type`. The server never sends `0` (stdin — +client→server only) or `3` (`STREAM_CTRL_IN` — client→server only), so +the server-sent set is `{1, 2, 4}` (stdout, stderr, `STREAM_CTRL_OUT`); +`0x00` is unambiguous. The client distinguishes: read the first byte; if +it is `0x00`, interpret the next 4 bytes as a big-endian length prefix +and read that many bytes as a JSON error frame; otherwise interpret it +as a `stream_type` byte and continue reading the raw chunk header. This +is a one-way-door wire-format invariant (ADR-052): error frames use the +negotiation framing (length prefix) and MUST be under 16 MiB; success +uses the raw chunk framing (stream_type byte first); the +`0x00`-as-length-prefix vs `0x00`-as-invalid-stream_type disambiguation +is what makes the two distinguishable on the wire. ### Exit-Chunk Ordering (ADR-055) diff --git a/docs/architecture/crates/tty/tty-wire.md b/docs/architecture/crates/tty/tty-wire.md index da307cb..efa4caf 100644 --- a/docs/architecture/crates/tty/tty-wire.md +++ b/docs/architecture/crates/tty/tty-wire.md @@ -1,13 +1,14 @@ --- status: draft -last_updated: 2026-07-07 +last_updated: 2026-07-18 --- # alknet-tty — Wire Format The wire protocol for `alknet/tty`: the negotiation frame (JSON -carriage), the raw chunk codec, the control channel, and the sentinels. -The two-carriage model is decided in +carriage), the raw chunk codec, the control channel (split into +`STREAM_CTRL_IN` / `STREAM_CTRL_OUT` halves — Phase 7), and the +sentinels. The two-carriage model is decided in [ADR-052](../../decisions/052-alknet-tty-wire-format-and-two-carriage.md); this document specifies what an implementer builds. @@ -164,19 +165,34 @@ no `call.responded`/`call.completed` — this is not the call protocol. - **`stream_type`** (1 byte) — the channel: - | stream_type | channel | direction | payload | - |---|---|---|---| - | 0 | data-in (stdin) | client→server | raw bytes | - | 1 | data-out (stdout) | server→client | raw bytes | - | 2 | data-err (stderr) | server→client | raw bytes | - | 3 | control | bidirectional | JSON control message | + | stream_type | channel | direction | payload | + |-------------|-------------|----------------|---------------------| + | 0 | data-in (stdin) | client→server | raw bytes | + | 1 | data-out (stdout) | server→client | raw bytes | + | 2 | data-err (stderr) | server→client | raw bytes | + | 3 | ctrl-in | client→server | JSON control message (`Resize`, `Signal`, `Eof`) | + | 4 | ctrl-out | server→client | JSON control message (`Exit`) | - `stream_type > 3` is a protocol error (`InvalidStreamType`). There is - no extension escape hatch in the byte — a 5th channel is a wire-format + `stream_type > 4` is a protocol error (`InvalidStreamType`). There is + no extension escape hatch in the byte — a 6th channel is a wire-format change requiring a new ALPN (`alknet/tty/v2` per ADR-006), not a negotiated addition to this format. See ADR-052 §"Fixed channel set, not extensible." + **Bidirectional control channel (Phase 7).** The control channel is + split into two halves so it is genuinely bidirectional on the wire: + `STREAM_CTRL_IN = 3` carries client→server control (`Resize`, + `Signal`, `Eof`); `STREAM_CTRL_OUT = 4` carries server→client control + (`Exit`). The previous single `STREAM_CONTROL = 3` was documented as + "bidirectional" but the adapter ignored `Exit` from the client + because the two directions were indistinguishable on the same + stream_type — see `docs/research/alknet-crate-extraction/findings.md` + Phase 7. The split makes the bidirectionality explicit: each + direction has its own stream_type, and the adapter enforces the + direction (an `Exit` arriving on `STREAM_CTRL_IN` is a protocol + violation and is ignored; a `Resize` arriving on `STREAM_CTRL_OUT` is + likewise a protocol violation and is ignored). + - **`length`** (4 bytes, big-endian) — payload length in bytes. Max 16 MiB (`MAX_CHUNK_LEN = 16 * 1024 * 1024`). A chunk larger than 16 MiB is a protocol error (`ChunkTooLarge`). @@ -208,9 +224,14 @@ Zero-length data chunks are sentinels: Control chunks are never zero-length (the JSON payload is at least `{}`). -### Control Channel (stream_type 3) +### Control Channel -Control chunks carry a JSON payload tagged by `type`. The schema is the +The control channel is split into two halves (Phase 7): + +- **`STREAM_CTRL_IN` (stream_type 3)** — client→server control. +- **`STREAM_CTRL_OUT` (stream_type 4)** — server→client control. + +Each half carries JSON payloads tagged by `type`. The schema is the POC's `ControlMessage` (`/workspace/alknet-tty-poc/src/control.rs`): ```rust @@ -231,12 +252,22 @@ pub enum ControlMessage { } ``` -| Direction | Message | Shape | Maps to | -|---|---|---|---| -| client→server | resize | `{"type":"resize","cols":80,"rows":24,"pixel_width":0,"pixel_height":0}` | SSH `window-change`, docker exec resize, `ioctl(TIOCSWINSZ)` | -| client→server | signal | `{"type":"signal","name":"INT"}` | SSH `signal`, docker exec signal, `kill(-pgid, sig)` (REQ-TTY-02) | -| client→server | eof | `{"type":"eof"}` | SSH channel EOF, docker stdin close, `ChildStdin::drop` | -| server→client | exit | `{"type":"exit","code":0}` | the terminal/completion signal (ADR-055) | +| stream_type | direction | Message | Shape | Maps to | +|-------------|----------------|---------|-------|---------| +| 3 (ctrl_in) | client→server | resize | `{"type":"resize","cols":80,"rows":24,"pixel_width":0,"pixel_height":0}` | SSH `window-change`, docker exec resize, `ioctl(TIOCSWINSZ)` | +| 3 (ctrl_in) | client→server | signal | `{"type":"signal","name":"INT"}` | SSH `signal`, docker exec signal, `kill(-pgid, sig)` (REQ-TTY-02) | +| 3 (ctrl_in) | client→server | eof | `{"type":"eof"}` | SSH channel EOF, docker stdin close, `ChildStdin::drop` | +| 4 (ctrl_out) | server→client | exit | `{"type":"exit","code":0}` | the terminal/completion signal (ADR-055) | + +The adapter enforces the direction: an `Exit` arriving on +`STREAM_CTRL_IN` is a protocol violation (the adapter ignores it); a +`Resize`/`Signal`/`Eof` arriving on `STREAM_CTRL_OUT` is likewise a +protocol violation (the adapter ignores it). The split makes the +control channel genuinely bidirectional on the wire — the previous +single `STREAM_CONTROL = 3` was documented as "bidirectional" but the +adapter had to ignore `Exit` from the client because the two directions +were indistinguishable on the same stream_type. See +`docs/research/alknet-crate-extraction/findings.md` Phase 7. **Signal names.** `name` is an uppercase string. The supported set (per the POC's `signal_from_name`): `HUP`, `INT`, `QUIT`, `TERM`, `KILL`, @@ -261,12 +292,12 @@ type is not. Two signals both close the client's stdin: -1. **`{"type":"eof"}` control chunk** (stream_type 3) — explicit, - recommended. Tells the server to close the backend's stdin - (`ChildStdin::drop` / PTY writer close). The client may still want to - receive remaining stdout + the exit code, so the server does not tear - down the session on eof — it just closes stdin and keeps pumping - output. +1. **`{"type":"eof"}` control chunk** (stream_type 3, `STREAM_CTRL_IN`) + — explicit, recommended. Tells the server to close the backend's + stdin (`ChildStdin::drop` / PTY writer close). The client may still + want to receive remaining stdout + the exit code, so the server does + not tear down the session on eof — it just closes stdin and keeps + pumping output. 2. **Zero-length stdin chunk** (stream_type 0, length 0) — the docker POC's sentinel. Accepted for compatibility with that pattern. @@ -288,17 +319,26 @@ a session — see [tty-adapter.md](tty-adapter.md). ## Constraints - **The wire format is one-way (ADR-052).** The 5-byte header, the fixed - stream_type set (0-3), and the two-carriage sequence are bytes clients - and servers parse. A 5th channel type requires a new ALPN + stream_type set (0-4), and the two-carriage sequence are bytes clients + and servers parse. A 6th channel type requires a new ALPN (`alknet/tty/v2` per ADR-006), not a negotiated addition. +- **The control channel is split into two halves (Phase 7).** + `STREAM_CTRL_IN = 3` is client→server (`Resize`, `Signal`, `Eof`); + `STREAM_CTRL_OUT = 4` is server→client (`Exit`). The adapter enforces + the direction: an `Exit` on `STREAM_CTRL_IN` is ignored; a `Resize` on + `STREAM_CTRL_OUT` is ignored. The split is what makes the control + channel genuinely bidirectional on the wire — the previous single + `STREAM_CONTROL = 3` was documented as "bidirectional" but the adapter + had to ignore `Exit` from the client because the two directions were + indistinguishable on the same stream_type. - **No windowing.** The chunk format has no flow-control window; QUIC's per-stream flow control is the backpressure mechanism (OQ-45 resolved: the backpressure chain is complete by construction — QUIC flow control → bounded drainer channel → bounded stdout channel → OS pipe/PTY buffer → process `write()` blocks; no unbounded buffer breaks the chain). The reversal path, if ever needed, is an additive - `ControlMessage` variant on stream_type 3, not a wire-format header - change. + `ControlMessage` variant on `STREAM_CTRL_IN`/`STREAM_CTRL_OUT`, not a + wire-format header change. - **No negotiation round-trip.** The client writes the negotiation frame and starts sending chunks; the server reads the frame and starts pumping. There is no "the server acknowledges the negotiation before @@ -312,18 +352,21 @@ a session — see [tty-adapter.md](tty-adapter.md). without entering raw mode. The error response MUST be under 16 MiB (`MAX_CHUNK_LEN`) so the 4-byte big-endian length prefix's high byte is `0x00` — this is what makes the framing-disambiguation trick - (first byte `0x00` = error frame, first byte `1`/`2`/`3` = raw chunk) - sound; it is a wire-format invariant, not an empirical observation. - See [tty-adapter.md](tty-adapter.md) §"Negotiation errors". + (first byte `0x00` = error frame, first byte `1`/`2`/`4` = raw chunk; + the server never sends `0` (stdin, client→server) or `3` + (`STREAM_CTRL_IN`, client→server), so `0x00` is unambiguous) sound; it + is a wire-format invariant, not an empirical observation. See + [tty-adapter.md](tty-adapter.md) §"Negotiation errors". ## Design Decisions | Decision | ADR | Summary | |----------|-----|---------| -| Wire format and two-carriage model | [ADR-052](../../decisions/052-alknet-tty-wire-format-and-two-carriage.md) | `alknet/tty` ALPN; JSON negotiation frame then raw chunks; fixed channel set 0-3; control as JSON | +| Wire format and two-carriage model | [ADR-052](../../decisions/052-alknet-tty-wire-format-and-two-carriage.md) | `alknet/tty` ALPN; JSON negotiation frame then raw chunks; fixed channel set 0-4; control as JSON | +| Bidirectional control channel split | Phase 7 (this doc, amended) | `STREAM_CTRL_IN = 3` (client→server) and `STREAM_CTRL_OUT = 4` (server→client) replace the single `STREAM_CONTROL = 3`; the adapter enforces the direction | | No alknet-call dependency (self-contained framing) | [ADR-057](../../decisions/057-alknet-tty-no-alknet-call-dep.md) | alknet-tty implements its own length-prefixed framing; format coincides with alknet-call's by convention, not by code reuse | -| Exit code on a control chunk | [ADR-055](../../decisions/055-exit-code-on-control-chunk.md) | `{"type":"exit","code":N}` on stream_type 3; "exit chunk is last" invariant | -| Stdin closure canonical signal | OQ-47 | Either `eof` control chunk or zero-length stdin chunk; `eof` recommended | +| Exit code on a control chunk | [ADR-055](../../decisions/055-exit-code-on-control-chunk.md) | `{"type":"exit","code":N}` on `STREAM_CTRL_OUT` (stream_type 4); "exit chunk is last" invariant | +| Stdin closure canonical signal | OQ-47 | Either `eof` control chunk (`STREAM_CTRL_IN`) or zero-length stdin chunk; `eof` recommended | ## Open Questions diff --git a/docs/architecture/decisions/052-alknet-tty-wire-format-and-two-carriage.md b/docs/architecture/decisions/052-alknet-tty-wire-format-and-two-carriage.md index 0dc9ed7..cbbe7df 100644 --- a/docs/architecture/decisions/052-alknet-tty-wire-format-and-two-carriage.md +++ b/docs/architecture/decisions/052-alknet-tty-wire-format-and-two-carriage.md @@ -2,7 +2,9 @@ ## Status -Accepted +Accepted (amended 2026-07-18 — Phase 7: the control channel is split +into `STREAM_CTRL_IN = 3` (client→server) and `STREAM_CTRL_OUT = 4` +(server→client) halves; see §"Control channel split" below) ## Context @@ -123,12 +125,15 @@ The bidi stream has two phases: | 0 | data-in (stdin) | client→server | raw bytes | | 1 | data-out (stdout) | server→client | raw bytes | | 2 | data-err (stderr) | server→client | raw bytes | - | 3 | control | bidirectional | JSON control message | + | 3 | ctrl-in | client→server | JSON control message (`Resize`, `Signal`, `Eof` — see §4a) | + | 4 | ctrl-out | server→client | JSON control message (`Exit` — see §4a) | - `stream_type > 3` is a protocol error (`InvalidStreamType`). There is - no extension escape hatch in the byte — a 5th channel is a wire-format - change requiring a new ALPN (`alknet/tty/v2` per ADR-006), not a - negotiated addition to this format. + `stream_type > 4` is a protocol error (`InvalidStreamType`). (Phase 7 + amendment, §4a: the original `3 = control (bidirectional)` is split + into `3 = ctrl-in` and `4 = ctrl-out`; the original bound was + `> 3`.) There is no extension escape hatch in the byte — a 6th + channel is a wire-format change requiring a new ALPN (`alknet/tty/v2` + per ADR-006), not a negotiated addition to this format. - `length` — payload length in bytes, u32 big-endian, max 16 MiB. A chunk larger than 16 MiB is a protocol error (`ChunkTooLarge`). The @@ -176,6 +181,53 @@ tearing down the session. This is a two-way-door extension point within the one-way-door wire format — adding a control message type is additive; changing the chunk header is not. +### 4a. Control channel split (Phase 7 amendment, 2026-07-18) + +The single `stream_type 3 = control (bidirectional)` in §3 and §4 above +is **split into two halves** so the control channel is genuinely +bidirectional on the wire: + +| stream_type | channel | direction | payload | +|-------------|-------------|----------------|------------------------------------------------------| +| 3 | `STREAM_CTRL_IN` | client→server | JSON control message (`Resize`, `Signal`, `Eof`) | +| 4 | `STREAM_CTRL_OUT` | server→client | JSON control message (`Exit`) | + +The `stream_type > 3` protocol-error bound becomes `stream_type > 4`. +The chunk header is otherwise unchanged (5 bytes: 1 type + 4 length). + +**Why the split.** The original §4 documented stream_type 3 as +"bidirectional" and listed the four control messages with their +directions. But the adapter had no way to distinguish the two +directions on the same stream_type — `Exit` from the client was always +ignored (the adapter's `pump_client_to_backend` matched `Exit` and +logged "ignoring Exit control from client (server→client only)"). The +spec said "bidirectional"; the code was half-duplex. The split makes +the bidirectionality literal: each direction has its own stream_type, +the adapter enforces the direction (an `Exit` arriving on +`STREAM_CTRL_IN` is a protocol violation; a `Resize` arriving on +`STREAM_CTRL_OUT` is a protocol violation), and a client can route +exit vs. control without parsing the JSON `type` tag first. + +**Door type.** One-way, same as the original §3 / §4. The stream_type +set is bytes clients and servers parse. A client written against the +old single-`STREAM_CONTROL` shape will misread `STREAM_CTRL_OUT = 4` as +`InvalidStreamType (> 3)` and tear down the session — the split is a +wire-format change, not an additive extension. The reversal path is +the same as the original: a new ALPN (`alknet/tty/v2`), which coexists +rather than replaces. The trade is one new stream_type byte now vs. +the half-duplex-in-disguise flaw forever. + +**What changes in the spec.** §3's stream_type table gains a 5th row +(`4 = ctrl_out, server→client`); the bound becomes `> 4`. §4's +direction table is unchanged in content (the four messages keep their +directions and shapes) but the direction is now encoded in the +stream_type, not just in the adapter's behavior. ADR-055's "exit chunk +is last" invariant is unchanged — the exit chunk still rides the +control channel, just on `STREAM_CTRL_OUT` (stream_type 4) instead of +the old single `STREAM_CONTROL` (stream_type 3). See +`docs/research/alknet-crate-extraction/findings.md` Phase 7 for the +full migration notes. + ### 5. Negotiation errors use the JSON framing, not the raw chunk format If the server cannot allocate the session (unknown backend, PTY diff --git a/docs/research/alknet-crate-extraction/findings.md b/docs/research/alknet-crate-extraction/findings.md index fa1f96e..a504913 100644 --- a/docs/research/alknet-crate-extraction/findings.md +++ b/docs/research/alknet-crate-extraction/findings.md @@ -480,8 +480,22 @@ only type returned by `accept_bi()`, `from_stream` is removed, ### Phase 7: TTY control-channel bidirectionality fix -**What:** Fix the "control isn't actually bidirectional" flaw in -`alknet-tty`. The current `STREAM_CONTROL = 3` is documented as +**Status:** **Done** (this commit, 2026-07-18). All "Done when" +criteria met: `STREAM_CONTROL` is replaced with `STREAM_CTRL_IN = 3` / +`STREAM_CTRL_OUT = 4`; `InvalidStreamType` bound is `> 4`; the adapter +dispatches `Resize`/`Signal`/`Eof` on `STREAM_CTRL_IN` and emits `Exit` +on `STREAM_CTRL_OUT`; protocol violations in both directions (`Exit` on +ctrl_in, anything on ctrl_out from the client) are ignored; +`cargo test -p alknet-tty` passes (65 tests, was 61 — 4 new tests for +the split); `cargo test -p alknet-tty-local` passes (19 tests); the +full workspace `cargo test --workspace --all-features` is fully green +(1017 tests, 0 failures). ADR-052 is amended (§4a "Control channel +split"); `tty-wire.md` and `tty-adapter.md` are updated; the +`negotiation.rs` framing-disambiguation doc is updated (server-sent +stream_type set is now `{1, 2, 4}`). + +**What was planned:** Fix the "control isn't actually bidirectional" flaw +in `alknet-tty`. The current `STREAM_CONTROL = 3` is documented as "bidirectional" but the adapter ignores `Exit` from the client (`adapter.rs:462-463`). The fix: @@ -496,6 +510,84 @@ only type returned by `accept_bi()`, `from_stream` is removed, This is a TTY-layer fix — the channels layer has no `stream_type` concept and is unaffected. +**What was done (`crates/alknet-tty`, +~110/-~60 lines net +~50):** + +- **`wire.rs`**: split `pub const STREAM_CONTROL: u8 = 3` into + `STREAM_CTRL_IN: u8 = 3` (client→server) and `STREAM_CTRL_OUT: u8 = + 4` (server→client); updated the module doc to describe the split; + updated `RawError::InvalidStreamType` doc comment (`> 4`); updated + `ChunkReader::read_chunk` bound check (`> 3` → `> 4`); replaced + `Chunk::control` with `Chunk::ctrl_in` + `Chunk::ctrl_out` + constructors; replaced `ChunkWriter::write_control_json` with + `write_ctrl_in_json` + `write_ctrl_out_json` helpers; split the + `round_trip_control` test into `round_trip_ctrl_in` + + `round_trip_ctrl_out`; split `round_trip_write_control_json_helper` + into `round_trip_write_ctrl_in_json_helper` + + `round_trip_write_ctrl_out_json_helper`; updated the + `invalid_stream_type` test (the boundary byte is now 5, not 4). +- **`control.rs`**: updated the module doc and the `ControlMessage` + enum doc (direction table now includes the `stream_type` column — + `STREAM_CTRL_IN` for `Resize`/`Signal`/`Eof`, `STREAM_CTRL_OUT` for + `Exit`; notes the adapter enforces the direction; explains the + half-duplex-in-disguise flaw the split fixes). The JSON shape is + unchanged — the `ControlMessage` enum, the `to_json`/`from_slice` + methods, and the `signal_from_name` helper are byte-for-byte + unchanged. The 9 unit tests in `control.rs` are unchanged. +- **`adapter.rs`**: updated the module doc (session lifecycle + + "Bidirectional control channel (Phase 7)" section); updated the + `STREAM_CONTROL` import to `STREAM_CTRL_IN`; `send_exit_chunk` now + emits `Chunk::ctrl_out(json)` (was `Chunk::control(json)`); the + `pump_client_to_backend` doc explains the direction enforcement; + the `STREAM_CTRL_IN` match arm dispatches `Resize`/`Signal`/`Eof` + and ignores `Exit` (with a debug log explaining the protocol + violation); a new `STREAM_CTRL_OUT` match arm logs and ignores the + protocol violation (the client writing on the server→client half); + updated all 10 test references to use the correct half — + `STREAM_CTRL_OUT` for reading the exit chunk from the server (4 + sites), `STREAM_CTRL_IN` for writing control from the client (6 + sites). Added 3 new tests: `ctrl_out_from_client_ignored`, + `exit_chunk_arrives_on_ctrl_out_not_ctrl_in`, and updated + `exit_control_from_client_ignored` (which now documents the protocol + violation explicitly). +- **`negotiation.rs`**: updated the framing-disambiguation doc + comment (the server-sent stream_type set is now `{1, 2, 4}` — the + server never sends `0` (stdin, client→server) or `3` + (`STREAM_CTRL_IN`, client→server); `0x00` is unambiguous as the + error-frame length-prefix high byte). +- **`crates/alknet-tty-local/tests/`**: updated `common/mod.rs` + (replaced `STREAM_CONTROL` import with `STREAM_CTRL_IN` + + `STREAM_CTRL_OUT`; `write_control` writes on `STREAM_CTRL_IN`; the + `read_until_exit` + `read_until_exit_timeout` match arms for the + exit chunk use `STREAM_CTRL_OUT`); updated `pty.rs` and `pipe.rs` + to use `STREAM_CTRL_OUT` for the exit-chunk reads. No production + code changes in `alknet-tty-local` (the backend doesn't write to + the wire — it produces handles; the adapter pumps). + +**Spec updates:** + +- **ADR-052** (`docs/architecture/decisions/052-...md`): Status changed + to "Accepted (amended 2026-07-18 — Phase 7: the control channel is + split into `STREAM_CTRL_IN = 3` (client→server) and + `STREAM_CTRL_OUT = 4` (server→client) halves; see §"Control channel + split" below)". §3's stream_type table gains a 5th row (`4 = ctrl_out, + server→client`); the bound becomes `> 4`. New §4a "Control channel + split (Phase 7 amendment)" documents the why, the door-type, and + the spec deltas. +- **`docs/architecture/crates/tty/tty-wire.md`**: updated `last_updated` + to 2026-07-18; the Phase 2 Raw Chunk Format stream_type table gains + the `ctrl_in`/`ctrl_out` rows; the "Bidirectional control channel + (Phase 7)" paragraph explains the split; the Control Channel + section now uses the two halves; the Constraints section reflects + the new bound (`> 4`) and the new framing-disambiguation + (`{1, 2, 4}` server-sent); the Design Decisions table gains a + "Bidirectional control channel split" row. +- **`docs/architecture/crates/tty/tty-adapter.md`**: updated + `last_updated` to 2026-07-18; the Session Lifecycle bullet B + references `STREAM_CTRL_IN`; bullet C references `STREAM_CTRL_OUT`; + new "Bidirectional Control Channel (Phase 7)" section documents the + direction enforcement; the Framing Disambiguation paragraph + references the new server-sent set `{1, 2, 4}`. + **Compilable state:** `cargo test -p alknet-tty` passes. The control channel is properly bidirectional. @@ -606,7 +698,7 @@ Phase 9 is fully closed. | 4 (core prune) | core is lightweight; `endpoint.rs` gone; `ConnectionCredentials` in core | | 5 (call prune) | call is pure protocol; `connect` + TLS helpers + `CallCredentials` + `from_call` dead path gone; Category B tests already moved; 2 `CallCredentials` tests moved to core | | 6 (stream unification) | **Done** (`b60a584`). `BiStream` is the handler leaf; `accept_bi` returns `BiStream`; `from_stream` removed; `from_bidi` is the only public constructor; `SendStream`/`RecvStream` are thin internal newtypes. Subsumed Phase 9's `QuicStream`/`QuicStreamDuplex` removal. | -| 7 (TTY control fix) | TTY control channel is properly bidirectional (`STREAM_CTRL_IN = 3`, `STREAM_CTRL_OUT = 4`); `InvalidStreamType` bound updated. **Unchanged by Phase 6** — Phase 7's work is in `wire.rs` and `control.rs`, neither of which Phase 6 touched. | +| 7 (TTY control fix) | **Done** (this commit). TTY control channel is properly bidirectional (`STREAM_CTRL_IN = 3`, `STREAM_CTRL_OUT = 4`); `InvalidStreamType` bound is `> 4`; the adapter dispatches on `STREAM_CTRL_IN` and emits on `STREAM_CTRL_OUT`; protocol violations in both directions are ignored. ADR-052 amended (§4a); `tty-wire.md` + `tty-adapter.md` + `negotiation.rs` framing-disambiguation doc updated. 4 new tests in `adapter.rs`; 1017 tests pass workspace-wide. | | 8 (channels spec) | Channels spec updated to 8-byte wire format; no `stream_type` concept; `into_sub_streams` removed; ADRs 071/074/077 amended. **Unchanged by Phase 6** — docs-only, no code. | | 9 (http fix) | **Done — subsumed by Phase 6** (`b60a584`) + test-helper fix (``). `QuicStream` wrapper removed from `alknet-http`; `BiStream` used directly; `QuicStreamDuplex` test helper removed. Pre-existing `to_mcp` test-helper bug (`full_registry_with_ops` used `HandlerKind::Once` for `Subscription` ops, rejected by the registry's kind validation since ADR-049) fixed — `cargo test --workspace --all-features` is fully green (1008 tests, 0 failures). |