From b3f50d18361e6ecf2bf1ceb0bb00611b96f06b12 Mon Sep 17 00:00:00 2001 From: "glm-5.2" Date: Mon, 17 Aug 2026 10:52:26 +0000 Subject: [PATCH] phase 4: architecture docs + BAST schema + renumbered ADRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port the alknet-tty architecture docs into alktty and add the BAST document for the alk/tty wire format. Docs-only; no Rust source changes. Spec docs (docs/architecture/, flat layout — single-crate repo): - overview.md — crate purpose, two-carriage model, deps, ALPN, backend location map, feature gates - tty-wire.md — 5-byte chunk codec, control channel split (STREAM_CTRL_IN=3 / STREAM_CTRL_OUT=4), sentinels - tty-backend.md — TtyBackend trait, TtyHandle, TtyControl, REQ-TTY-01 (backends need not be natively async) - tty-adapter.md — TtyAdapter, three-pump driver, exit-chunk ordering (ADR-004), cancel cleanup (ADR-005), access control - tty-local.md — LocalTtyBackend (local feature module), PTY + pipe modes, REQ-TTY-02 (signal forwarding to process group) - README.md — architecture index ADRs (docs/architecture/decisions/, renumbered 001..008 from alknet 052,053,054,055,056,057,077,093 in order): - 001 wire format + two-carriage model (incl. Phase 7 control- channel split amendment) - 002 TtyBackend trait + TtyHandle - 003 local backend placement (records both the alknet sibling- crate decision and the alktty single-crate consolidation behind a local feature) - 004 exit code on a control chunk - 005 backend cleanup on session cancel - 006 self-contained negotiation framing - 007 tty inside channels (reversed by 008; kept for historical context with reversal notice) - 008 channels pure channel multiplexing (reverses 007; TTY always uses its 5-byte format) BAST document (docs/architecture/tty-bast.md): - Normative JSON spec for the alk/tty wire format, conforming to the BAST meta-schema at https://alk.dev/bast/v1/schema - 5-byte chunk header (struct, big-endian: stream_type uint8, length uint32) + StreamType enum (Stdin=0..CtrlOut=4) - ControlMessage union (field-name discriminator on type: resize/signal/eof/exit) with documented deviation that on-wire control payloads are UTF-8 JSON, not BAST's binary union encoding - NegotiationFrame (4-byte BE length + UTF-8 JSON body) + NegotiateRequest / TerminalParams JSON shapes - StreamType enum deviation noted: on-wire uint8, not BAST's standard u32 enum index (chunk header is 5 bytes, not 8) - alktty does not depend on alktype; the hand-rolled wire.rs is the runtime codec, the BAST is the human-readable contract AGENTS.md: fixed the ADR mapping table to match the plan's 8-to-8 mapping (the previous table substituted ADR-050 for 054, relabeled 056 as control-message split, dropped 077, and added a new control-split ADR at 006 — inconsistent with both the plan and the prose). ADR-050 (dynamic resource ownership) is an alkcall/alknet- core ADR, not tty-specific, and is not ported; the Phase 7 control split stays as an amendment inside ADR-001, mirroring alknet. Verification (all pass, no Rust source changed): - cargo test (80 passed) - cargo test --all-features (103 passed) - cargo clippy --all-targets -- -D warnings (clean) - cargo fmt --check (clean) - cargo check --target wasm32-unknown-unknown (clean) - cargo clippy --target wasm32-unknown-unknown -- -D warnings (clean) - cargo doc --no-deps: 9 pre-existing intra-doc-link warnings in src/session.rs and src/channels.rs (untouched by this commit; not introduced here) - BAST JSON parses; StreamType indices match wire.rs constants (0=Stdin..4=CtrlOut) - all markdown cross-reference links resolve --- AGENTS.md | 71 ++- docs/architecture/README.md | 155 ++++++ .../001-wire-format-and-two-carriage.md | 413 ++++++++++++++ .../002-ttybackend-trait-and-ttyhandle.md | 435 +++++++++++++++ .../decisions/003-local-backend-placement.md | 268 +++++++++ .../004-exit-code-on-control-chunk.md | 219 ++++++++ .../005-backend-cleanup-on-session-cancel.md | 298 ++++++++++ .../006-negotiation-framing-self-contained.md | 190 +++++++ .../decisions/007-tty-inside-channels.md | 270 +++++++++ .../008-channels-pure-channel-multiplexing.md | 521 ++++++++++++++++++ docs/architecture/overview.md | 312 +++++++++++ docs/architecture/tty-adapter.md | 380 +++++++++++++ docs/architecture/tty-backend.md | 426 ++++++++++++++ docs/architecture/tty-bast.md | 384 +++++++++++++ docs/architecture/tty-local.md | 395 +++++++++++++ docs/architecture/tty-wire.md | 394 +++++++++++++ docs/plans/project-setup.md | 80 ++- 17 files changed, 5165 insertions(+), 46 deletions(-) create mode 100644 docs/architecture/README.md create mode 100644 docs/architecture/decisions/001-wire-format-and-two-carriage.md create mode 100644 docs/architecture/decisions/002-ttybackend-trait-and-ttyhandle.md create mode 100644 docs/architecture/decisions/003-local-backend-placement.md create mode 100644 docs/architecture/decisions/004-exit-code-on-control-chunk.md create mode 100644 docs/architecture/decisions/005-backend-cleanup-on-session-cancel.md create mode 100644 docs/architecture/decisions/006-negotiation-framing-self-contained.md create mode 100644 docs/architecture/decisions/007-tty-inside-channels.md create mode 100644 docs/architecture/decisions/008-channels-pure-channel-multiplexing.md create mode 100644 docs/architecture/overview.md create mode 100644 docs/architecture/tty-adapter.md create mode 100644 docs/architecture/tty-backend.md create mode 100644 docs/architecture/tty-bast.md create mode 100644 docs/architecture/tty-local.md create mode 100644 docs/architecture/tty-wire.md diff --git a/AGENTS.md b/AGENTS.md index 805e20b..b792b9b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -105,17 +105,17 @@ implementation agents. `STREAM_CTRL_IN=3`, `STREAM_CTRL_OUT=4`. Zero-length data chunks are sentinels (zero-length stdin = EOF from client; zero-length stdout = "drained" from server); control chunks are never zero-length. - Changing the header, the stream-type values, or the sentinel - semantics breaks all peers. See ADR-052 (ported as alktty ADR-001). - - **Negotiation frame** (4-byte BE length prefix + UTF-8 JSON - `NegotiateRequest` body) — self-contained per ADR-057 (ported as - alktty ADR-008), not reused from alkcall's `EventEnvelope` framing + Changing the header, the stream-type values, or the sentinel + semantics breaks all peers. See ADR-052 (ported as alktty ADR-001). + - **Negotiation frame** (4-byte BE length prefix + UTF-8 JSON + `NegotiateRequest` body) — self-contained per ADR-057 (ported as + alktty ADR-006), not reused from alkcall's `EventEnvelope` framing (the payload is `NegotiateRequest`, not `EventEnvelope`; alkcall's `FrameFramedReader` is hardcoded to deserialize `EventEnvelope`). The `NegotiateRequest` JSON shape is wire-stable once consumers exist. - Per ADR-093 (ported as alktty ADR-007): TTY always uses its 5-byte + Per ADR-093 (ported as alktty ADR-008): TTY always uses its 5-byte format, even inside channels. The channels layer strips its 8-byte header and hands TTY the payload transparently — the same `wire.rs` code runs in both direct (`alk/tty`) and channels (`alk/channels`) @@ -243,35 +243,48 @@ wasm-clean" invariant — run it whenever a non-`local` module changes. - `docs/plans/project-setup.md` — the current plan (phases 0–5). Phase 0 (scaffold hygiene), Phase 1 (port core types), Phase 2 (channels - integration + `TtySession`), and Phase 3 (`local` backend) are landed. - Phase 4 (architecture docs + BAST schema + renumbered ADRs) and + integration + `TtySession`), Phase 3 (`local` backend), and Phase 4 + (architecture docs + BAST schema + renumbered ADRs) are landed. Phase 5 (tests, including integration tests in `tests/` at the crate - root) are not yet done. -- `docs/architecture/` does not exist yet — it's created in Phase 4. - The ADRs referenced below (alknet ADR-052, 053, 054, 055, 056, 057, - 077, 093) will be ported and renumbered into alktty's ADR range - (001..008) at that time. Until then, the alknet originals at - `/workspace/@alkdev/alknet/docs/architecture/decisions/` are the - authoritative source — read them before non-trivial changes to the - wire format, trait, or adapter. -- Key ADRs that inform this crate's design (alknet numbers → planned - alktty numbers): + root) is not yet done. +- `docs/architecture/` is created in Phase 4. The alknet ADRs + referenced below (052, 053, 054, 055, 056, 057, 077, 093) are ported + and renumbered into alktty's ADR range (001..008) in + `docs/architecture/decisions/`. The alknet originals at + `/workspace/@alkdev/alknet/docs/architecture/decisions/` remain the + authoritative source for any ADR not yet ported — read them before + non-trivial changes to the wire format, trait, or adapter. +- Key ADRs that inform this crate's design (alknet numbers → ported + alktty numbers, 1:1 in the order the plan lists them): - ADR-052 → 001 — two-carriage wire format (JSON negotiation + raw - chunks); the 5-byte chunk header + chunks); the 5-byte chunk header. The Phase 7 control-channel + split (`STREAM_CTRL_IN` = 3 client→server, `STREAM_CTRL_OUT` = 4 + server→client) is an amendment inside this ADR, mirroring alknet + (it was not a standalone ADR there either). - ADR-053 → 002 — `TtyBackend` trait (the inversion point between wire-format adapter and backend crates) - - ADR-050 → 003 — dynamic resource ownership for runtime-spawned - terminal sessions - - ADR-054 → 004 — single crate with `local` feature (resolves the - alknet cyclic-dep workaround; the local backend is folded in) - - ADR-055 → 005 — exit-code reporting (`{"type":"exit","code":N}` on + - ADR-054 → 003 — local backend placement. Ported with the + single-crate rewrite: alktty folds the local backend in behind a + `local` feature (resolves the alknet cyclic-dep workaround that + motivated the original sibling-crate decision; the ADR records both + the original alknet decision and the alktty consolidation). + - ADR-055 → 004 — exit-code reporting (`{"type":"exit","code":N}` on `ctrl_out`; `code: -1` on wait-failure) - - ADR-056 → 006 — control-message split (`ctrl_in` client→server, - `ctrl_out` server→client; the bidirectionality fix) - - ADR-093 → 007 — TTY always uses its 5-byte format inside channels - (reverses ADR-077; channels strips its 8-byte header transparently) - - ADR-057 → 008 — negotiation framing is self-contained (not reused + - ADR-056 → 005 — backend cleanup on session cancel (drop of + `exit_code` future kills the session target; the + kill-on-`Drop` contract on the `TtyBackend` trait) + - ADR-057 → 006 — negotiation framing is self-contained (not reused from alkcall's `EventEnvelope` framing) + - ADR-077 → 007 — TTY inside channels (reversed by ADR-093/008; kept + for historical context with its reversal notice pointing to 008) + - ADR-093 → 008 — TTY always uses its 5-byte format inside channels + (reverses ADR-077; channels strips its 8-byte header transparently) +- ADR-050 (dynamic resource ownership) is an alkcall/alknet-core ADR, + not a tty-specific one — it is not ported into alktty's ADR range. + The access-control work that declares against the ADR-050 model + (scope-gate at negotiation, backend-driven `resource_id()` ownership + check) is described in `tty-adapter.md` and the ADR-001/002 ported + docs, which reference ADR-050 by its alknet number. - If a TODO references a "Phase 7" note or a design direction that an ADR has since decided against, the TODO is stale — remove it and align with the ADR. Do not implement the rejected design. diff --git a/docs/architecture/README.md b/docs/architecture/README.md new file mode 100644 index 0000000..6d73a46 --- /dev/null +++ b/docs/architecture/README.md @@ -0,0 +1,155 @@ +# alktty — Architecture + +Terminal session protocol for the `alk/tty` ALPN: a +producer/consumer protocol crate on top of alkcall channels. This +directory holds the architecture spec docs, the BAST (Binary Abstract +Syntax Tree) document for the wire format, and the ADRs. + +## Documents + +| Document | Status | Description | +|----------|--------|-------------| +| [overview.md](overview.md) | draft | Crate purpose, the two-carriage model in brief, dependencies, ALPN, backend location map, feature gates | +| [tty-wire.md](tty-wire.md) | draft | The wire format: negotiation frame (JSON carriage), raw chunk codec (`[stream_type: u8][length: u32 be][payload]`), control channel split into `STREAM_CTRL_IN` / `STREAM_CTRL_OUT` halves, sentinels | +| [tty-bast.md](tty-bast.md) | draft | The BAST (Binary Abstract Syntax Tree) document for the `alk/tty` wire format; a normative JSON spec conforming to the BAST meta-schema at `https://alk.dev/bast/v1/schema` | +| [tty-backend.md](tty-backend.md) | draft | `TtyBackend` trait, `TtyParams`, `TtyHandle`, `TtyControl` — the inversion point between the wire-format adapter and the backends. Carries REQ-TTY-01 (backends need not be natively async) | +| [tty-adapter.md](tty-adapter.md) | draft | `TtyAdapter` (`ProtocolHandler` on `alk/tty`): session lifecycle, three-pump bidirectional driver, negotiation errors, exit-chunk ordering (ADR-004), access control, session-cancel cleanup (ADR-005) | +| [tty-local.md](tty-local.md) | draft | `LocalTtyBackend` (the `local` feature module): `portable_pty` (PTY) and `tokio::process::Command` (pipe/runner). Carries REQ-TTY-02 (signal forwarding to the process group) | + +## Applicable ADRs + +Ported from the alknet mono-repo and renumbered into alktty's ADR +range (001..008). The alknet originals at +`/workspace/@alkdev/alknet/docs/architecture/decisions/` remain the +authoritative source for any ADR not yet ported, and for the alknet +ADRs referenced by alknet number in the docs below (which are not +tty-specific and therefore not ported into alktty's ADR range). + +| ADR | Title | Origin | Status | +|-----|-------|--------|--------| +| [001](decisions/001-wire-format-and-two-carriage.md) | alktty Wire Format and Two-Carriage Model | alknet ADR-052 | Accepted (amended 2026-07-18 — Phase 7 control-channel split) | +| [002](decisions/002-ttybackend-trait-and-ttyhandle.md) | TtyBackend Trait and TtyHandle — the Backend Inversion Point | alknet ADR-053 | Accepted | +| [003](decisions/003-local-backend-placement.md) | Local TTY Backend Placement (Single Crate with `local` Feature) | alknet ADR-054 | Accepted (records both the alknet sibling-crate decision and the alktty single-crate consolidation) | +| [004](decisions/004-exit-code-on-control-chunk.md) | Exit Code on a Control Chunk (the Last Chunk Before Stream Close) | alknet ADR-055 | Accepted | +| [005](decisions/005-backend-cleanup-on-session-cancel.md) | Backend Cleanup on Session Cancel (Drop of `exit_code` Kills) | alknet ADR-056 | Accepted | +| [006](decisions/006-negotiation-framing-self-contained.md) | Self-Contained Negotiation Framing (No alkcall-Internal-Wire-Types Dependency) | alknet ADR-057 | Accepted | +| [007](decisions/007-tty-inside-channels.md) | TTY Inside Channels — Sub-Streams, Not Wire Format | alknet ADR-077 | Accepted (**reversed by ADR-008** — kept for historical context) | +| [008](decisions/008-channels-pure-channel-multiplexing.md) | Channels Pure Channel Multiplexing (8-Byte Header, No `stream_type`) | alknet ADR-093 | Accepted (amends alknet ADR-071/074; reverses ADR-007) | + +## Key Design Principles + +1. **A terminal session is a terminal concern, not an SSH or Docker + concern.** SSH and Docker are two backends that can allocate a PTY. + alktty owns the terminal session lifecycle; the backends + (`DockerTtyBackend`, `SshTtyBackend`, `LocalTtyBackend`) implement a + `TtyBackend` trait. This dissolves the PTY hedge in the alknet-ssh + research (DP-5): PTY is not an SSH feature delegated to a separate + crate, it's a tty feature that SSH happens to be able to provide. See + [overview.md](overview.md) and [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md). + +2. **Two-carriage model: JSON negotiation, then raw chunks.** The bidi + stream opens with a single length-prefixed JSON negotiation frame + (terminal params, backend selector, command), then switches to a raw + chunk format (`[stream_type: u8][length: u32 be][payload]`) for the + life of the session. The call protocol's JSON-RPC shape handles the + structured request; raw bytes handle the body, which is what a + terminal actually is. No per-chunk `EventEnvelope` overhead, no + base64. See [tty-wire.md](tty-wire.md) and + [ADR-001](decisions/001-wire-format-and-two-carriage.md). + +3. **Fixed channel set, not extensible.** Five stream types (0=stdin, + 1=stdout, 2=stderr, 3=ctrl-in, 4=ctrl-out), no negotiation. A 6th + channel type is a wire-format change (one-way door); the ALPN model + handles extensibility at the protocol level (a new ALPN is cheap, a + wire-format change is not). The impoverishment vs SSH channels is + the feature: alktty multiplexes *one* service (a terminal session) + with a fixed channel structure, not *arbitrary* services. See + [tty-wire.md](tty-wire.md). + +4. **The backend trait is the inversion point.** alktty defines + `TtyBackend`; the backend crates implement it. alktty depends on + alkcall; backends depend on alktty for the trait; alktty does not + depend on any backend. This preserves alknet ADR-003's + no-handler-depends-on-another-handler rule. alktty does not depend + on alkcall's internal wire types either (the negotiation framing is + self-contained — [ADR-006](decisions/006-negotiation-framing-self-contained.md)). + See [tty-backend.md](tty-backend.md) and + [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md). + +5. **Backends need not be natively async (REQ-TTY-01).** The trait's + adapter-facing types (`AsyncWrite`, `Stream`, + `BoxFuture`, `TtyControl`) are the adapter's contract. A backend may + expose blocking handles internally and bridge them via std threads + + tokio mpsc/oneshot (the pattern `portable_pty` requires, and the + local-PTY POC validated). The bridging pattern is a documented, + supported implementation strategy. See + [tty-backend.md](tty-backend.md) and [tty-local.md](tty-local.md). + +6. **Exit code on a control chunk, last before stream close + ([ADR-004](decisions/004-exit-code-on-control-chunk.md)).** + `{"type":"exit","code":N}` rides on `STREAM_CTRL_OUT` (stream_type 4) + and is the last chunk before the server closes the write half. This + gives coordinators deterministic completion notification — no + polling, no plugin state. The adapter owns the ordering; backends + resolve `exit_code` and the adapter awaits, sends the chunk, closes. + See [tty-adapter.md](tty-adapter.md). + +7. **Drop of `exit_code` future kills the session target + ([ADR-005](decisions/005-backend-cleanup-on-session-cancel.md)).** + On session cancel (connection drop, stream reset), the adapter drops + the `TtyHandle`, which drops the `exit_code` future without driving + it to completion. The backend's `exit_code` future's `Drop`-on-cancel + MUST kill the child/container/SSH process. This is a behavioral + contract on the `TtyBackend` trait — the adapter has no separate kill + method; the cleanup is wired into the `exit_code` future's `Drop` by + the backend. See [tty-adapter.md](tty-adapter.md) and + [tty-local.md](tty-local.md). + +8. **The runner pattern is preserved, not specialized.** The local + backend in pipe mode (`terminal: None`) is a process-streaming + endpoint — the same shape as GitHub/Gitea Actions runners, just over + alk's transport instead of HTTP polling. alktty provides the + *mechanism* (framed byte stream + exit code); runner *policy* (job + management, log persistence, task graph) is a downstream crate's + job. See [tty-local.md](tty-local.md) and + [ADR-003](decisions/003-local-backend-placement.md). + +9. **TTY always uses its 5-byte format, including inside channels + ([ADR-008](decisions/008-channels-pure-channel-multiplexing.md)).** + The same `wire.rs` code runs in both direct `alk/tty` and channels + `alk/channels` modes; only the `BiStream` source differs. The + channels layer strips its 8-byte header and hands TTY the payload + transparently. This reverses the earlier two-mode design + ([ADR-007](decisions/007-tty-inside-channels.md), kept for + historical context). See [tty-adapter.md](tty-adapter.md). + +## Relevant Open Questions + +| OQ | Title | Status | Relevance | +|----|-------|--------|-----------| +| OQ-43 | `TtyControl` trait object `Clone` constraint | resolved | `control: Option` via a `#[derive(Clone)]` newtype wrapping `Arc`; the trait is NOT `Clone` (not object-safe) — the newtype carries `Clone`-ability | +| OQ-44 | Terminal modes (TTY modes) | deferred(scope) | `TerminalParams.modes` reserved; default terminal modes suffice for current scope; blocked on a concrete mode-control use case | +| OQ-45 | Flow control for high-throughput stdout | resolved | QUIC per-stream flow control is the backpressure mechanism (chain complete by construction); no application-level windowing. Reversal is an additive `ControlMessage` variant, not a wire-format change | +| OQ-46 | Runner API surface | deferred(scope) | The runner mechanism (pipe mode) is in alktty; runner policy (job management, log persistence, task graph) is a downstream crate, not in scope here | +| OQ-47 | Stdin closure canonical signal | resolved | Either a zero-length stdin chunk or a `{"type":"eof"}` control chunk; both are accepted; the spec recommends `eof` for explicitness | + +## References + +- `docs/plans/project-setup.md` — the current plan (phases 0–5). + Phases 0–3 are landed; Phase 4 (this directory: architecture docs + + BAST schema + renumbered ADRs) is landed by this commit; Phase 5 + (tests, including integration tests in `tests/` at the crate root) is + not yet done. +- alknet originals of the ported ADRs (alknet ADR-052, 053, 054, 055, + 056, 057, 077, 093) at + `/workspace/@alkdev/alknet/docs/architecture/decisions/` — the + authoritative source for any ADR not yet ported, and for the alknet + ADRs referenced by alknet number in the docs above (which are not + tty-specific and therefore not ported into alktty's ADR range). +- [alktype BAST format spec](https://alk.dev/bast/v1/schema) — the + normative format spec for BAST documents (the meta-schema + `tty-bast.md` conforms to); see also + `/workspace/@alkdev/alktype/docs/architecture/bast-format.md`. +- alkcall architecture README at + `/workspace/@alkdev/alkcall/docs/architecture/README.md` — the + producer/consumer protocol-crate pattern alktty follows. \ No newline at end of file diff --git a/docs/architecture/decisions/001-wire-format-and-two-carriage.md b/docs/architecture/decisions/001-wire-format-and-two-carriage.md new file mode 100644 index 0000000..2512c29 --- /dev/null +++ b/docs/architecture/decisions/001-wire-format-and-two-carriage.md @@ -0,0 +1,413 @@ +# ADR-001: alktty Wire Format and Two-Carriage Model + +## Status + +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. Ported +from alknet ADR-052 2026-08-17; cross-references renumbered to +alktty's ADR range — ADR-052→001, ADR-053→002, ADR-054→003, +ADR-055→004, ADR-056→005, ADR-057→006, ADR-077→007, ADR-093→008. +Alknet ADRs referenced by alknet number (001, 002, 003, 006, 007, 012, +044, 049, 050) are not ported into alktty's ADR range because they are +not tty-specific; the alknet originals at +`/workspace/@alkdev/alknet/docs/architecture/decisions/` remain +authoritative.) + +## Context + +The alknet-docker POC validated that interactive attach — bidirectional +byte pumping over a framed bidi stream with a 1-byte stream-type +multiplexer — is the same problem regardless of whether the backend is +`bollard::attach_container()` or russh's `pty_request`. The POC's raw +chunk format (`[stream_type: u8][length: u32 be][payload bytes]`, +stream_type 0=stdin, 1=stdout, 2=stderr) is a deliberately impoverished +version of SSH's channel multiplexer: fixed set of channel types, no +negotiation, no open/close handshake, no windowing (QUIC provides flow +control on the bidi stream). That impoverishment is the feature — a +terminal session needs exactly those channels and no more. + +The alknet-tty POC (built 2026-07-05) extended that format with a 4th +stream_type (3 = control) carrying JSON control messages (resize, +signal, eof, exit) and validated the full round-trip against a real +`portable_pty` PTY: negotiate → PTY alloc → bidirectional echo → +mid-session resize → EOF → exit code, plus SIGINT forwarding to a child +process group. The wire format this ADR commits is the POC's raw codec ++ control schema, generalized from one backend to the backend-agnostic +crate. + +Three load-bearing questions are decided here: + +1. **Separate ALPN with raw carriage, not call-protocol operations.** A + terminal session could be modeled as a call-protocol `Subscription` + operation (`tty/open`) streaming `call.responded` events. That is + rejected: JSON-encoding every byte chunk is wasteful (base64 for + binary, per-chunk `EventEnvelope` overhead) and lossy (a TTY streams + partial bytes with no message boundary that maps to a JSON object). + The two-carriage model — a single JSON negotiation frame, then raw + chunks — keeps the call protocol's JSON-RPC shape for the *request* + and switches to bytes for the *body*, which is the part that is + actually bytes. This is the pattern the docker POC validated and the + SSH research independently arrived at for PTY. + +2. **Fixed channel set, not extensible.** SSH multiplexes arbitrary + services (forwarding, SFTP, agent, X11) over `ChannelId(u32)` with + string-named types negotiated per channel. alktty multiplexes one + service — a terminal session — with a fixed `u8` set and no + negotiation. Adding a 6th channel type is a wire-format change + (one-way door). The ALPN model handles extensibility at the protocol + level: a genuinely new sideband (e.g., file transfer alongside the + terminal) is a different ALPN, not a 6th tty channel type. A new + ALPN is cheap; a wire-format change is not. + +3. **Control messages as JSON, not binary.** A binary control format + (`[control_type: u8][params...]`) would be faster but harder to + extend and inconsistent with the negotiation layer. Control messages + are rare (resize on window drag, signal on Ctrl-C, one eof, one exit + per session) — serialization cost is negligible against the data + chunks. If a hot control path appears, a binary `control_type` can + be added without breaking the chunk format (additive within the + control channel). + +## Decision + +### 1. alktty is a `ProtocolHandler` on ALPN `alk/tty` + +`alk/tty` is a custom ALPN per the alknet ADR-006 `alk/` convention +(renamed from `alknet/` in alkcall 0.1.1). The `TtyAdapter` +implements `ProtocolHandler` (alknet ADR-002, revised by alknet ADR-007 +to receive a `Connection`). The handler owns the entire connection +lifecycle and accepts one bidi stream per terminal session. This is a +separate ALPN, not a set of operations in the call protocol's +`OperationRegistry` — the raw-carriage byte pump is not a +`StreamingHandler` (alknet ADR-049); it is its own wire format after a +single JSON negotiation frame. + +### 2. Two-carriage model: JSON negotiation, then raw chunks + +The bidi stream has two phases: + +- **Negotiation (JSON carriage).** The client opens a bidi stream and + writes a single length-prefixed JSON frame — a 4-byte big-endian + length prefix + UTF-8 JSON body. The frame carries the terminal + parameters and backend selector the server needs to allocate the + session: + + ```json + { + "carriage": "raw", + "backend": "local", + "tty": { + "term": "xterm-256color", + "cols": 80, + "rows": 24, + "pixel_width": 0, + "pixel_height": 0, + "modes": {} + }, + "cmd": ["/bin/bash"], + "cwd": null, + "env": {} + } + ``` + + The `carriage` field is `"raw"` for terminal sessions (the only + carriage in v1). The `tty` block is `null` for the pipe/runner case + (no PTY — see ADR-003). The `backend` field selects the `TtyBackend` + (ADR-002); backend-specific fields (e.g., `container` for docker) + ride alongside. + +- **Raw carriage.** After the negotiation frame, the stream switches to + the chunk format below for the life of the session. There is no + `call.responded`/`call.completed` — this is not the call protocol. + +### 3. Chunk format + +```text +[stream_type: u8][length: u32 be][payload bytes] +``` + +- `stream_type` — fixed set, no negotiation: + + | 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` — see §4a) | + | 4 | ctrl-out | server→client | JSON control message (`Exit` — see §4a) | + + `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 (`alk/tty/v2` + per alknet 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 + 16 MiB bound accommodates large paste operations and large `env` + blocks while bounding memory per chunk (a reader can allocate a + bounded buffer up front); it mirrors the docker POC's limit. + +- 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 + server (output stream ended). Control chunks are never zero-length + (the JSON payload is at least `{}`). + +### 4. Control channel (stream_type 3) carries JSON control messages + +Control chunks carry a small JSON payload, tagged by `type`: + +| Direction | Message | Shape | +|---|---|---| +| client→server | resize | `{"type":"resize","cols":80,"rows":24,"pixel_width":0,"pixel_height":0}` | +| client→server | signal | `{"type":"signal","name":"INT"}` | +| client→server | eof | `{"type":"eof"}` | +| server→client | exit | `{"type":"exit","code":0}` | + +- **resize** — window-size change. Maps to SSH `window-change`, docker + exec resize, or `ioctl(TIOCSWINSZ)` on a local PTY. +- **signal** — signal forwarding. `name` is an uppercase string + (`"INT"`, `"TERM"`, `"HUP"`, `"QUIT"`, `"TSTP"`, `"CONT"`, `"KILL"`, + `"USR1"`, `"USR2"`). Unknown names fall back to the backend's default + kill (see ADR-002 / `tty-local.md` REQ-TTY-02). +- **eof** — client signals no more stdin. Maps to SSH channel EOF, + docker stdin close, or `ChildStdin::drop` / PTY writer close. This is + one of two canonical "stdin done" signals; the other is a zero-length + stdin chunk (the docker POC's sentinel). Both are accepted as + equivalent — `eof` is recommended for explicitness (it's a control + message, not a data-length hack); the zero-length stdin chunk is kept + for compatibility with the docker POC's pattern. See OQ-47. +- **exit** — server signals process exit with code. This is the last + control chunk before stream close (see ADR-004). + +The `type` tag is the extensibility seam: new control message types are +added by extending the tagged enum. Unknown `type` values are ignored +(not a protocol error) so that a newer client sending a control message +an older server doesn't recognize degrades gracefully rather than +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 (`alk/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-004'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). + +### 5. Negotiation errors use the JSON framing, not the raw chunk format + +If the server cannot allocate the session (unknown backend, PTY +allocation failed, the command is invalid), it sends a JSON error +response in the same 4-byte length-prefixed framing as the negotiation +frame and closes the stream without entering raw mode. The error +response shape is `{"error":"","message":"..."}` (see +[tty-adapter.md](../tty-adapter.md) §"Negotiation errors" for the +codes). This is not `call.error` — this is not the call protocol; the +error is a JSON response in the negotiation framing, and the stream +closes after it. + +**Framing disambiguation (success vs error).** Both a successful +allocation (raw chunks) and a failed allocation (JSON error frame) +begin with bytes the client must read before knowing which framing +applies. 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, 4}`. 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: 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. (With the Phase 7 split, the server-sent +set is `{1, 2, 4}` — stdout, stderr, `STREAM_CTRL_OUT`; `0x00` is +still unambiguous.) + +### 6. Negotiation framing is self-contained in alktty (no alkcall-internal-wire-types dependency) + +The 4-byte length prefix + JSON body is implemented in alktty as a +small, self-contained module (`src/negotiation.rs`, ~30 lines: read +4-byte BE length, bounds-check, read N bytes; write the inverse). The +format coincides with alkcall's `EventEnvelope` framing by convention +(both are length-prefixed JSON) — not by code reuse. alktty does not +depend on alkcall's internal wire types: the negotiation payload is a +tty-specific struct (`NegotiateRequest`), not a `call.requested` +`EventEnvelope`, and alkcall's `FrameFramedReader::read_frame()` is +hardcoded to deserialize `EventEnvelope` (the length-prefix read and +the type-specific deserialize are one entangled call), so it is not +reusable for a different payload type. alktty implements its own +framing on tokio's `AsyncRead`/`AsyncWrite`. See +[ADR-006](006-negotiation-framing-self-contained.md) for the decision +(and the three options considered: duplicate / promote to alkcall::core +/ use alkcall) and alknet ADR-003 Amendment 2 for the dependency-edge +clarification. + +## Consequences + +**Positive:** + +- The wire format is POC-validated twice (docker POC for stream_type + 0/1/2 + bidirectional pump; tty POC for stream_type 3 + control + messages + local PTY). No new wire-format invention in Phase 1. +- The fixed channel set is a `match`, not a hash lookup — fast on the + hot path where every chunk is data. +- The two-carriage model keeps the call protocol's JSON-RPC shape for + the structured request while letting the body be raw bytes, which is + what a terminal actually is. No base64, no per-chunk `EventEnvelope` + overhead. +- Control messages as JSON are consistent with the negotiation layer + and trivially extensible (tagged enum), at negligible cost for rare + messages. +- A separate ALPN composes with the ALPN dispatch model (alknet + ADR-001/006): the endpoint routes `alk/tty` to the `TtyAdapter`; the + call protocol is unaffected. Browser terminals (xterm.js over + WebTransport, when WebTransport revives) connect to `alk/tty` directly + without implementing SSH or the call protocol. + +**Negative:** + +- A 6th channel type is a wire-format change (one-way door). The + justification is that the use cases are bounded — a terminal session + has stdin, stdout, stderr, and control (now split into ctrl-in / + ctrl-out). New sideband needs are different ALPNs, not 6th channels. + If this proves wrong, the reversal is a new ALPN string + (`alk/tty/v2`), which coexists with the old one rather than replacing + it — but every client and server implementing the old format would + need updating to speak the new one. +- Control as JSON means a `serde_json` deserialize per control chunk. + Control chunks are rare (one per resize, one per signal, one eof, one + exit), so this is negligible. A hot control path would warrant a + binary format — the `type`-tagged enum leaves that door open without + a wire-format change. +- The negotiation frame is a custom JSON shape, not a `call.requested` + event, so a client library can't reuse its call-protocol client to + open a tty session — it speaks the tty wire format directly. This is + intentional (the tty session is not a call-protocol operation) but + means the tty client is a separate small client, not a `CallClient` + method. +- **No application-level flow-control window (OQ-45 resolved).** The + chunk format carries no window; backpressure relies on QUIC's + per-stream flow control composing with the bounded channels in the + adapter's pump and the OS pipe/PTY buffer. This is sufficient by + construction (see §Assumptions 1). If a workload ever requires + sub-QUIC-window backpressure signaling, the reversal is an additive + `ControlMessage::WindowUpdate` (or similar) variant on the control + channel — a two-way-door extension to the control channel, not a + wire-format header change. The `type`-tagged enum's "unknown types + ignored" rule means older peers degrade gracefully if such a variant + is introduced. + +## Door type + +**One-way.** The chunk header (5 bytes: 1 type + 4 length), the fixed +stream_type set (0-4), and the two-carriage sequence (JSON frame → raw +chunks) are bytes clients and servers parse. Changing any of them breaks +every client and server implementing the format. The reversal path is a +new ALPN (`alk/tty/v2`), which coexists rather than replaces — but the +cost of migrating every consumer is the one-way-door cost. + +The control message `type` enum is a two-way-door extension point within +the one-way wire format: adding a control message type is additive +(unknown types are ignored), changing the meaning of an existing type is +not. + +## Assumptions + +1. **No application-level windowing; QUIC per-stream flow control is the + backpressure mechanism.** The chunk format carries no window field. + This is decided (OQ-45 resolved): the backpressure chain from a slow + client read all the way back to the child process's stdout write is + complete by construction — QUIC flow control → bounded drainer + channel → bounded stdout channel → OS pipe/PTY buffer → process + `write()` blocks. Every link awaits its producer; no unbounded + buffer breaks the chain. The reversal path, if a workload ever + surfaces a problem QUIC's defaults cannot handle, is an additive + window-update `ControlMessage` variant (a two-way-door extension to + the control channel, not a wire-format header change) — noted in + §Consequences. See OQ-45. + +2. **The negotiation frame fits in one chunk of the underlying stream's + initial flow-control window.** The frame is small (terminal params + + command + env, typically < 4 KiB). QUIC's default initial bidi-window + (quinn defaults are tens of KiB) accommodates it without a + flow-control round-trip. A pathological `env` block larger than the + window would stall until the window opens; the 16 MiB chunk limit is + the hard cap. + +3. **Control messages are rare enough that JSON serialization cost is + negligible.** Validated by the tty POC: resize on window drag, one + signal per Ctrl-C, one eof, one exit per session. No measurable cost + observed. + +## References + +- alknet ADR-001 — ALPN-based dispatch +- alknet ADR-002 — ProtocolHandler trait +- alknet ADR-006 — `alk/` ALPN convention; one ALPN per + connection; new ALPN for incompatible versions +- alknet ADR-007 — handler receives a `Connection`, accepts bidi streams +- alknet ADR-003 Amendment 2 — alktty does not depend on alknet-call; + self-contained negotiation framing +- [ADR-006](006-negotiation-framing-self-contained.md) — the + dependency-edge decision this ADR's §6 reflects +- alknet ADR-012 — the call protocol's stream model (which tty is *not* + using for the body, by design) +- alknet ADR-049 — the `StreamingHandler` path tty explicitly does not + use for the byte body +- [ADR-002](002-ttybackend-trait-and-ttyhandle.md) — the backend trait + the negotiation frame's `backend` field dispatches to +- [ADR-003](003-local-backend-placement.md) — the local backend's + module placement (the `tty: null` pipe-mode case) +- [ADR-004](004-exit-code-on-control-chunk.md) — the exit-chunk + ordering this ADR's control channel carries +- [ADR-008](008-channels-pure-channel-multiplexing.md) — TTY always + uses its 5-byte format, including inside channels (the channels layer + carries this wire format transparently in its payload) +- Spec: [tty-wire.md](../tty-wire.md), [tty-bast.md](../tty-bast.md) +- Port origin: alknet ADR-052 at + `/workspace/@alkdev/alknet/docs/architecture/decisions/052-alknet-tty-wire-format-and-two-carriage.md` \ No newline at end of file diff --git a/docs/architecture/decisions/002-ttybackend-trait-and-ttyhandle.md b/docs/architecture/decisions/002-ttybackend-trait-and-ttyhandle.md new file mode 100644 index 0000000..25ef866 --- /dev/null +++ b/docs/architecture/decisions/002-ttybackend-trait-and-ttyhandle.md @@ -0,0 +1,435 @@ +# ADR-002: TtyBackend Trait and TtyHandle — the Backend Inversion Point + +## Status + +Accepted (ported from alknet ADR-053 2026-08-17; cross-references +renumbered to alktty's ADR range — ADR-052→001, ADR-053→002, +ADR-054→003, ADR-055→004, ADR-056→005, ADR-057→006, ADR-077→007, +ADR-093→008. Alknet ADRs referenced by alknet number (003, 007, 017, +050) are not ported into alktty's ADR range because they are not +tty-specific; the alknet originals at +`/workspace/@alkdev/alknet/docs/architecture/decisions/` remain +authoritative.) + +## Context + +alktty's wire format (ADR-001) is backend-agnostic — the chunk codec +pumps bytes and JSON control messages without knowing whether the +backend is a docker container, an SSH session channel, or a local +process. The question this ADR answers: **what is the seam between the +wire-format adapter and the backends?** + +The alknet-tty research identified the `TtyBackend` trait as the +inversion point. The guiding insight: + +> A terminal session is not an SSH concern, or a Docker concern — it is +> a terminal concern. SSH and Docker are just two backends that can +> allocate a PTY. + +The trait is what makes that insight load-bearing: alktty defines the +trait and the wire-format adapter; the backend crates (alknet-docker, +alknet-ssh, alktty's `local` feature module) implement the trait. This +preserves alknet ADR-003's no-handler-depends-on-another-handler rule: +alktty depends on alkcall; backend crates depend on alktty for the +trait; alktty does not depend on any backend (and, per ADR-006, does +not depend on alkcall's internal wire types either — the negotiation +framing is self-contained). + +### What the local-PTY POC discovered about the trait shape + +The Phase 0 POC was built *before* this ADR specifically to discover +constraints the trait sketch would have missed by reading docs alone. +Two requirements fell out of it (recorded as REQ-TTY-01 and REQ-TTY-02 +in the alknet findings doc): + +- **REQ-TTY-01: backends are not required to be natively async.** + `portable_pty` is a blocking `std::io::{Read, Write}` API with a + blocking `Child::wait()`. The POC bridges it to async via three + dedicated std threads (reader, writer, waiter) feeding tokio + mpsc/oneshot channels — the same pattern wezterm (portable_pty's + primary consumer) uses. The trait's adapter-facing types + (`AsyncWrite`, `Stream`, `BoxFuture`) are the + *adapter's* contract; a backend may expose blocking handles + internally and bridge them. The bridging pattern is a documented, + supported implementation strategy, not a workaround. + +- **`exit_code` is a `Future` the adapter awaits, not a method on + `TtyHandle`.** A `oneshot::Receiver` (or any + `BoxFuture<'static, i32>`) lets the adapter `select` between exit and + stream-close without coupling to the handle's other fields. The local + backend's waiter thread produces exactly this shape for free. + +The POC's `LocalPty` struct is the reference implementation of what a +backend produces: `stdout: mpsc::Receiver`, +`stdin: mpsc::Sender`, `control: PtyControl` (Clone), +`exit_code: oneshot::Receiver`. The trait shape below generalizes +this. + +### What the local-PTY POC did not resolve + +The POC used a separate cloneable `PtyControl` struct for resize/signal, +not a trait object. The research noted this worked cleanly because the +control-chunk dispatcher needs to be `Clone` to hand off to the spawned +pump task. Phase 1 confirms the `control` field as a separate +`TtyControlHandle` newtype — a concrete `#[derive(Clone)]` struct +wrapping `Arc` (the trait is NOT `Clone`; +`Clone` is not object-safe — see OQ-43). The newtype carries the +`Clone`-ability; the trait stays object-safe. + +## Decision + +### 1. `TtyBackend` trait + +```rust +#[async_trait] +pub trait TtyBackend: Send + Sync { + /// Allocate a terminal/process session and return the handles the + /// adapter pumps. The `backend` field of the negotiation frame + /// (ADR-001) selects which registered backend's `allocate` is called. + async fn allocate(&self, params: &TtyParams) -> Result; + + /// The pre-existing resource this session targets, for ownership + /// checks (alknet ADR-050). `None` = no pre-existing resource (the session + /// creates its own — local process, SSH channel). `Some((kind, id))` + /// = the session targets an existing resource the caller must own + /// (e.g., DockerTtyBackend returns `Some(("container", id))`). The + /// adapter calls this at negotiation to gate access; the backend + /// extracts the id from its own `backend_params`. Default `None` + /// (most backends create their own resource). + fn resource_id(&self, _params: &TtyParams) -> Option<(&'static str, String)> { None } +} +``` + +The adapter holds a `HashMap>` keyed by the +negotiation frame's `backend` string (`"local"`, `"docker"`, `"ssh"`). +The assembly layer registers backends at startup; the adapter dispatches +by the `backend` field. A backend is the *thing that allocates a +session*; the wire-format pump is backend-agnostic. + +### 2. `TtyParams` — the allocation request + +```rust +pub struct TtyParams { + /// Terminal parameters. `None` = pipe mode (no PTY — the runner case, + /// ADR-003). `Some` = allocate a PTY with these dimensions. + pub terminal: Option, + /// Command vector (argv[0] + args). + pub cmd: Vec, + /// Working directory (backend-specific; None = inherit/default). + pub cwd: Option, + /// Environment variables (backend-specific; empty = inherit). + pub env: HashMap, + /// Backend-specific selector fields from the negotiation frame, + /// unparsed. The adapter passes the JSON object through verbatim; the + /// backend deserializes its own strongly-typed params struct from it. + /// alktty has zero knowledge of any backend's params shape. See + /// §"Backend params are opaque" below. + pub backend_params: serde_json::Map, +} + +pub struct TerminalParams { + pub term: Option, // e.g., "xterm-256color"; None = backend default + pub cols: u16, + pub rows: u16, + pub pixel_width: u16, + pub pixel_height: u16, + pub modes: serde_json::Value, // reserved — OQ-44; backends MUST ignore content in v1 +} +``` + +`terminal: None` is the pipe/runner case — no PTY, separate +stdout/stderr (ADR-003). `terminal: Some` is the PTY case — +stdout/stderr merged into the single stdout stream +(`TtyHandle.stderr` is `None`), real terminal semantics (resize, signal +delivery to process group). + +### Backend params are opaque + +`backend_params` is a `serde_json::Map`, not +a typed enum. The adapter passes the negotiation frame's +backend-specific fields through verbatim; the backend deserializes its +own strongly-typed params struct. alktty has zero knowledge of any +backend's params shape — not docker's `container`, not an SSH host +selector, not anything. + +Each backend defines its own params struct: + +```rust +// in alknet-docker +#[derive(Deserialize)] +struct DockerBackendParams { container: String } + +// in alknet-ssh +#[derive(Deserialize)] +struct SshBackendParams { /* host selector if multi-host; else empty */ } + +// in alktty's local feature module +// no backend-specific params — backend_params is empty +``` + +And deserializes from `params.backend_params` inside `allocate()`: + +```rust +impl TtyBackend for DockerTtyBackend { + async fn allocate(&self, params: &TtyParams) -> Result { + let p: DockerBackendParams = serde_json::from_value( + serde_json::Value::Object(params.backend_params.clone()) + ).map_err(|e| TtyError::Backend { message: e.to_string() })?; + // use p.container ... + } +} +``` + +This is a complete inversion: the *trait* is inverted (backends +implement, alktty doesn't depend on them) and the *params* are inverted +(backends define their own typed shape, alktty doesn't carry it). A new +backend crate requires zero changes to alktty — no enum variant to add, +no forward-reference type to place, no dependency edge. + +**Why not a typed enum.** The earlier draft of this ADR defined +`BackendParams` as a `#[non_exhaustive]` enum with `Local`, `Docker { +container }`, and `Ssh { channel: SshChannelRef }` variants. Three +problems: + +1. **Rust enums are closed.** `#[non_exhaustive]` prevents *consumers* + from matching exhaustively, but only the *defining crate* (alktty) + can add variants. A backend crate cannot add a variant; every new + backend requires modifying alktty. The inversion is only partial. +2. **`SshChannelRef` was an output, not an input.** The SSH channel is + what `allocate()` *opens* (`session.channel_open_session()` → + `pty_request` → `shell_request`). It doesn't exist until the backend + creates it; the client doesn't send one. +3. **Dependency contradiction.** `SshChannelRef` "wraps a russh + `ChannelId` and session reference." If it lives in alknet-ssh, + alktty depends on alknet-ssh (violates the inversion). If it lives in + alktty, alktty pulls in russh types (same violation). Opaque params + dissolve the contradiction — there is no `SshChannelRef` type in + alktty at all. + +The earlier draft rejected `serde_json::Value` because "it loses type +safety and forces the adapter to parse backend-specific JSON it +shouldn't interpret." The first concern doesn't apply (each backend has +its own strongly-typed struct via serde; type safety moves from alktty +to the backend where it belongs). The second was already inconsistent +with the adapter, which hardcoded extraction of docker's `container` +field for the ownership check — the adapter *was* parsing +backend-specific JSON. The opaque approach removes that: the adapter +delegates the resource-id extraction to the backend via `resource_id()` +below. + +### 3. `TtyHandle` — what a backend produces + +```rust +pub struct TtyHandle { + /// Stdin writer — bytes the adapter pumps from client stdin chunks. + /// `tokio::io::AsyncWrite` (the tokio flavor, not the `futures::io` + /// one — they are incompatible traits; the tokio stack is the + /// adapter's runtime). + pub stdin: Box, + /// Stdout stream — bytes the adapter pumps to client stdout chunks. + /// Ends when the backend's stdout reaches EOF (process exited, + /// container output stream ended, SSH channel closed). + /// `futures_core::Stream` (re-exported by + /// `tokio_stream::StreamExt` for extension methods). + pub stdout: Pin + Send>>, + /// Stderr stream — `None` for PTY backends (stdout/stderr merged + /// into `stdout`). `Some` for pipe backends (separate streams). + pub stderr: Option + Send>>>, + /// Exit code — a `Future` the adapter awaits. Resolves when the + /// process/container/SSH exec exits. The adapter sends the result + /// as the `{"type":"exit","code":N}` control chunk (ADR-004) and + /// closes the stream. This is `BoxFuture`, not a method on + /// `TtyHandle`, so the adapter can `select` between exit and + /// stream-close without coupling to the other fields. (REQ-TTY-01.) + pub exit_code: BoxFuture<'static, Result>, + /// Control handle (resize, signal) — `Clone` so the adapter can + /// hand it to the spawned control-chunk dispatcher. `None` only + /// when the backend genuinely has no control path (e.g., a pipe + /// backend with no PTY — signal still works via `kill(pid, sig)`, + /// but resize is a no-op). See OQ-43 for the `TtyControlHandle` + /// newtype rationale. + pub control: Option, +} + +pub trait TtyControl: Send + Sync { + /// Resize the terminal. Maps to SSH `window-change`, docker exec + /// resize, or `ioctl(TIOCSWINSZ)` on a local PTY. No-op for pipe + /// backends without a PTY (the adapter still calls it; the backend + /// ignores). + fn resize(&self, cols: u16, rows: u16, pixel_width: u16, pixel_height: u16); + + /// Forward a signal by name. Best-effort delivery to the foreground + /// process group (see tty-local.md REQ-TTY-02). Unknown names fall + /// back to the backend's default kill. + fn signal(&self, name: &str); +} + +/// The `Clone`-able handle to a backend's control path. The `TtyControl` +/// trait is NOT `Clone` (`Clone` is not object-safe — `fn clone(&self) -> +/// Self` returns `Self`, which forbids `dyn` dispatch); the `Clone`-ability +/// lives on this concrete newtype, which holds the trait object behind an +/// `Arc`. The adapter clones the `Arc` to hand a handle to the spawned +/// control-chunk dispatcher. See OQ-43. +#[derive(Clone)] +pub struct TtyControlHandle(Arc); +``` + +### 4. Backends are not required to be natively async (REQ-TTY-01) + +The trait's adapter-facing types (`AsyncWrite`, `Stream`, +`BoxFuture`, the `TtyControl` trait object) are the **adapter's +contract**. A backend may expose blocking handles internally and bridge +them to these async-facing types. The bridging pattern — blocking +`std::io` on dedicated std threads or `tokio::task::spawn_blocking`, +feeding tokio mpsc/oneshot channels — is a **documented, supported +implementation strategy**, not a workaround. + +The local backend (ADR-003) uses this pattern: `portable_pty` is a +blocking API, and the backend's `allocate()` spawns reader/writer/waiter +threads that feed `mpsc::Receiver` (stdout), `mpsc::Sender` +(stdin wrapped as `AsyncWrite`), and `oneshot::Receiver` (exit). +The adapter consumes the bridged async-facing types and is unaware of +the threading. See `tty-local.md` for the bridge details. + +### 5. Backend registration and the assembly layer + +The `TtyAdapter` does not know the set of backends at compile time — it +holds a `HashMap>` populated at +construction. The assembly layer (the CLI binary) constructs backends +with their dependencies (a `DockerTtyBackend` wraps a `bollard::Docker` +client; an `SshTtyBackend` wraps an SSH session; a `LocalTtyBackend` +takes no deps) and registers them: + +```rust +let mut backends = HashMap::new(); +backends.insert("local".into(), Arc::new(LocalTtyBackend::new()) as Arc); +backends.insert("docker".into(), Arc::new(DockerTtyBackend::new(docker_client)) as _); +backends.insert("ssh".into(), Arc::new(SshTtyBackend::new(ssh_session)) as _); +let tty_adapter = TtyAdapter::new(Arc::new(backends)); +``` + +A deployment that doesn't want docker registers only `local`. A browser +terminal endpoint that proxies to remote docker/ssh registers `docker` +and/or `ssh` backends. The adapter is backend-agnostic; the assembly +layer chooses what's available. + +## Consequences + +**Positive:** + +- The wire-format adapter is backend-agnostic and testable with a mock + backend (in-memory pipes). The build order (ADR-001 wire format + + mock backend first, real backends last) follows directly. +- alktty stays dependency-light: no bollard, no russh, no `portable_pty` + in the default crate. The heavy deps live in the backend crates / + the `local` feature. This is the same inversion as + `OperationAdapter` (alknet ADR-017): the trait lives where the types + live; the implementations live where their transport dependencies + live. +- Blocking-API backends (portable_pty) are first-class — the trait + accommodates them by making the adapter-facing types the contract and + the bridging pattern a documented strategy. No re-spec required when + a future backend is also blocking. +- `exit_code` as a `Future` (not a method on the handle) lets the + adapter `select` between exit and stream-close — the load-bearing + composition the session lifecycle needs (the exit chunk is sent after + the child is reaped, then the stream closes — ADR-004). + +**Negative:** + +- **Backend params are opaque (`serde_json::Map`), not a typed enum.** + Each backend deserializes its own params struct; the adapter passes + the JSON through verbatim. The cost is one serde deserialize per + `allocate()` call (negligible — allocation is once per session, not on + the hot path). The benefit is a complete inversion: alktty has zero + knowledge of any backend's params shape, and a new backend crate + requires zero changes to alktty (no enum variant, no forward-reference + type). See §"Backend params are opaque" for the full rationale and + why the typed-enum alternative was rejected (Rust enums are closed; + the earlier `SshChannelRef` variant was an output modeled as an input + and created a dependency contradiction). +- **`TtyControl` is not `Clone`; the `TtyControlHandle` newtype is.** + `Clone` is not object-safe (`fn clone(&self) -> Self` returns `Self`, + which forbids `dyn` dispatch), so `Box` does + not compile. The design splits the concerns: the `TtyControl` trait + stays object-safe (`Send + Sync`, no `Clone`); the `TtyControlHandle` + newtype (a concrete struct holding `Arc`) implements `Clone` by cloning the `Arc`. This is the cost of + the POC-discovered constraint that the control-chunk dispatcher needs + to be `Clone` to hand off to the spawned pump task. See OQ-43 for the + confirmation and the concrete newtype approach. +- A backend that produces neither a PTY nor a process (a hypothetical + "recorded session replay" backend) would have a no-op `TtyControl` + and a synthetic `exit_code`. The trait accommodates it but the + `TtyParams` shape (`cmd` is `Vec`, `terminal` is `Option`) + assumes command-spawning. A non-command backend would supply an empty + `cmd` and synthesize one internally. Not a current use case; the + trait shape doesn't preclude it but doesn't optimize for it. + +## Door type + +**One-way.** The `TtyBackend` trait method `allocate()`, the +`TtyHandle` field set, and the `TtyControl` trait are the API surface +every backend crate implements and the adapter consumes. Changing them +after backends exist is a rewrite across crates. `backend_params` as an +opaque `serde_json::Map` is part of the one-way `TtyParams` shape — the +*carrier* is fixed (opaque JSON), but the *contents* are +backend-defined and require no alktty change for new backends. The +`resource_id()` default method is additive (a new method with a default +impl doesn't break existing implementors); its return type +`Option<(&'static str, String)>` is one-way. + +## Assumptions + +1. **The `TtyControl` trait is kept object-safe by NOT putting `Clone` + on it; the `TtyControlHandle` newtype holds the trait object behind + an `Arc` and implements `Clone` by cloning the `Arc`.** The POC used + a concrete `PtyControl` struct (inherently `Clone` — it held + `Arc>` fields). The newtype generalizes the POC's shape so + a backend produces its own control type via + `TtyControlHandle::new(Arc::new(MyControl))` without the adapter + knowing the concrete shape. `Clone` cannot live on the trait itself + (it is not object-safe); the newtype is the seam. OQ-43 confirms. + +2. **Backends produce a single session per `allocate()` call.** The + adapter calls `allocate()` once per accepted bidi stream (one session + per stream — ADR-001). A backend that multiplexed multiple sessions + over one `allocate()` would not fit the trait; no such backend is + contemplated. + +3. **The adapter, not the backend, owns the exit-chunk ordering.** The + backend resolves `exit_code`; the adapter awaits it, sends the exit + control chunk, and closes the stream (ADR-004). The backend does not + write to the wire — it produces handles; the adapter pumps. This + keeps the wire-format logic in one place (the adapter) and the + backend focused on its allocation target (docker, ssh, local + process). + +## References + +- alknet ADR-003 + Amendments 1 & 2 — no-handler-depends-on-another- + handler; alktty depends on alkcall (no alkcall-internal-wire-types per + Am. 2 / ADR-006); backends depend on alktty for the trait +- [ADR-006](006-negotiation-framing-self-contained.md) — alktty does not + depend on alkcall's internal wire types (self-contained negotiation + framing) +- alknet ADR-007 — `Connection`, `SendStream`, `RecvStream` (the adapter + receives a `Connection`, accepts bidi streams, pumps per-session) +- [ADR-001](001-wire-format-and-two-carriage.md) — the wire format this + trait's backends feed +- [ADR-003](003-local-backend-placement.md) — the local backend's + module placement (folded into alktty behind a `local` feature) +- [ADR-004](004-exit-code-on-control-chunk.md) — the exit chunk ordering + this trait's `exit_code` field feeds into +- [ADR-005](005-backend-cleanup-on-session-cancel.md) — the cancel- + cleanup contract on this trait (`exit_code` future's `Drop`-on-cancel + kills the session target) +- alknet ADR-017 — the adapter-location-map pattern (trait where types + live, implementation where deps live) this ADR follows +- alknet ADR-050 — the ownership model the `resource_id()` default + declares against +- OQ-43 — `TtyControl` as `Clone` trait object (resolved: confirmed) +- OQ-44 — terminal modes (deferred(scope): not needed for current scope) +- Spec: [tty-backend.md](../tty-backend.md) +- Port origin: alknet ADR-053 at + `/workspace/@alkdev/alknet/docs/architecture/decisions/053-ttybackend-trait-and-ttyhandle.md` \ No newline at end of file diff --git a/docs/architecture/decisions/003-local-backend-placement.md b/docs/architecture/decisions/003-local-backend-placement.md new file mode 100644 index 0000000..b997b32 --- /dev/null +++ b/docs/architecture/decisions/003-local-backend-placement.md @@ -0,0 +1,268 @@ +# ADR-003: Local TTY Backend Placement (Single Crate with `local` Feature) + +## Status + +Accepted (ported from alknet ADR-054 2026-08-17, with the single-crate +consolidation recorded as the alktty resolution. Cross-references +renumbered to alktty's ADR range — ADR-052→001, ADR-053→002, +ADR-054→003, ADR-055→004, ADR-056→005, ADR-057→006, ADR-077→007, +ADR-093→008. Alknet ADRs referenced by alknet number (003, 009, 017) +are not ported into alktty's ADR range because they are not tty-specific; +the alknet originals at +`/workspace/@alkdev/alknet/docs/architecture/decisions/` remain +authoritative.) + +## Context + +The alknet-tty research (DP-1) posed the placement question for the +local-process backend (`std::process::Command` with piped stdio, or +`portable_pty` for a real PTY): + +- **(a) In alktty**: the crate ships with the local backend built-in. + Pro: zero-config runner, one crate gets a terminal/process-streaming + endpoint. Con: alktty pulls in `portable_pty` even for deployments + that only use docker/ssh backends. +- **(b) In a sibling crate (`alknet-tty-local`)**: alktty defines the + trait (ADR-002); the local backend is a separate crate. Pro: alktty + stays dependency-light; consumers opt into the local backend + explicitly. Con: one extra crate for the common case. + +The local backend is the simplest backend and the one that enables the +runner pattern (a process whose stdin/stdout/stderr/exit-code stream +over a framed bidi connection — the same shape as GitHub/Gitea Actions +runners, just over alk's transport instead of HTTP polling). It has no +heavy dependencies in the pipe case — just `std` — but the PTY case +pulls in `portable_pty` (a non-trivial native dependency that builds on +Unix `openpty`/`ioctl` and Windows ConPTY). + +The relevant constraint from ADR-002: alktty itself depends only on +alkcall and the wire-format codec (ADR-001). The `portable_pty` +dependency does not belong in the core tty crate — a docker-only +deployment or an SSH-PTY-only deployment should not pull in PTY +allocation code. This is the same inversion as `OperationAdapter` +(alknet ADR-017): the trait lives where the types live; the +implementations live where their transport dependencies live. + +### The alknet mono-repo decision (sibling crate + feature re-export) + +In the alknet mono-repo, the research recommended (b) sibling crate +**behind a feature flag on alknet-tty** for the common case +(`features = ["local"]` → re-export from `alknet-tty-local`). This +keeps alknet-tty's default dependency surface minimal while making the +local backend a one-feature opt-in. The `portable_pty` dependency lives +in `alknet-tty-local`; alknet-tty itself never depends on +`portable_pty`. + +The sibling-crate placement was motivated by a cyclic-dependency +workaround: `alknet-tty-local` depends on `alknet-tty` for the trait, +and alknet ADR-054 wanted `alknet-tty` to re-export `LocalTtyBackend` +behind a `local` feature — which cargo rejects (a crate cannot re-export +from a sibling crate it depends on via an optional dep AND have that +sibling depend back on it). The assembly-layer workaround (consumer +depends on both crates directly) worked but was awkward. + +### The alktty consolidation (single crate, `local` feature) + +alktty is a standalone single crate (not a mono-repo). The cyclic-dep +workaround that motivated the alknet sibling-crate decision does not +apply: the local backend can live in the same crate as the trait, gated +behind a `local` cargo feature. The feature gate still keeps +`portable_pty` out of the default dependency tree (a docker-only or +ssh-only deployment enables neither feature and never pulls in +`portable_pty`); the only thing that changes is the crate boundary. + +This ADR records both decisions: the original alknet sibling-crate +decision (preserved as the historical context for why the local backend +is a separable concern), and the alktty consolidation (the operative +decision — the local backend is folded into alktty behind a `local` +feature). + +## Decision + +### 1. The local backend is folded into alktty behind a `local` feature + +`LocalTtyBackend` lives in `src/local/` (a feature-gated module), +implements `TtyBackend` (ADR-002), and is re-exported as +`alktty::local::LocalTtyBackend`. The `local` cargo feature pulls in +the heavy deps: + +```toml +# alktty Cargo.toml +[features] +default = [] +local = ["dep:portable-pty", "dep:tokio-util", "tokio/process", "tokio/rt-multi-thread"] + +[dependencies] +portable-pty = { version = "0.9", optional = true } +tokio-util = { version = "0.7", features = ["io"], optional = true } +``` + +A consumer that wants the local backend enables `features = ["local"]` +on alktty and gets `alktty::local::LocalTtyBackend`. A consumer that +only wants docker/ssh backends uses the default features and depends on +`alknet-docker` / `alknet-ssh` (or their own backend crate) directly — +no `portable_pty` in the dependency tree. + +This is the same feature-re-export pattern the Rust ecosystem uses for +optional heavy dependencies (e.g., `tokio`'s `full` feature pulling in +`tokio-util`, `h2`, etc.). The seam is the `TtyBackend` trait; the +feature gate is the only thing keeping `portable_pty` out of the +default crate. Folding the local backend in is cheaper than the alknet +sibling-crate pattern (no cross-crate coordination, no cyclic-dep +workaround) and the local backend is small (~1.4k lines) and tightly +coupled to the trait. + +### 2. The local backend's dependency tree + +``` +alktty (local feature enabled) +├── alktty (default) (TtyBackend trait, TtyHandle, TtyControl, wire types) +├── alkcall::core (via alktty's re-export; not direct) +├── portable_pty (PTY allocation — the heavy dep, here not in alktty default) +├── libc (signal forwarding — REQ-TTY-02, Unix only) +└── tokio (process, rt-multi-thread, mpsc, oneshot, AsyncRead/AsyncWrite) +``` + +The `local` feature is inherently non-wasm (`portable-pty` + +`tokio::process` need a real OS); enabling `local` on +`wasm32-unknown-unknown` is a build error by design. The default crate +(no features) stays wasm-clean — this is what makes the downstream +TS/Python adapter story work (a wasm-compiled alktty is the protocol +layer for a sandboxed adapter; the local-process backend runs on a real +OS, never in a sandbox). + +### 3. PTY vs pipe is a per-session choice, not a per-deployment choice + +`TtyParams.terminal: Option` (ADR-002) selects the mode: + +- **`terminal: Some(TerminalParams { ... })`** — allocate a real PTY + via `portable_pty`. Terminal semantics: resize (via + `ioctl(TIOCSWINSZ)`), signal delivery to the foreground process group + (via `libc::kill(-pgid, sig)`, REQ-TTY-02), escape-sequence handling + (the kernel PTY's line discipline). stdout and stderr are merged + (kernel PTY property — one output stream from the slave), so + `TtyHandle.stderr` is `None`. +- **`terminal: None`** — pipe mode, no PTY. `tokio::process::Command` + with `Stdio::piped()` for stdin/stdout/stderr. No resize, no + escape-sequence handling, but `kill(pid, sig)` still works for signal + forwarding. stdout and stderr are separate streams, so + `TtyHandle.stderr` is `Some`. This is the **runner** case — a + command-streaming endpoint with no terminal semantics. + +The same `LocalTtyBackend` serves both; the `allocate()` call branches +on `params.terminal`. A deployment that only does terminals always +sends `Some`; a deployment that only does runners always sends `None`; +a deployment that does both (a hub that runs agents in PTYs and runs +`cargo test` as a runner) sends the appropriate one per session. + +### 4. The runner pattern is preserved, not specialized + +The pipe mode (`terminal: None`) is the "runner" generalization the +research identified: a process whose stdin/stdout/stderr/exit-code +stream over a framed bidi connection. This is functionally identical to +GitHub/Gitea Actions runners, just over alk's transport instead of HTTP +polling: + +- A coordinator sends a negotiation frame with + `{ "backend": "local", "tty": null, "cmd": ["cargo", "test"] }`. +- The endpoint runs `cargo test` with piped stdio, streams stdout/stderr + chunks back, sends `{"type":"exit","code":N}` when it finishes + (ADR-004). +- The coordinator gets reliable completion notification (the exit + control chunk + stream close) — no polling. + +The runner-specific API surface (job management, log persistence, task +graph integration) is **out of scope for alktty**. alktty provides the +*mechanism* (a framed byte stream for a process); the runner *policy* +is a downstream crate's job. This ADR commits to preserving the option +(`terminal: None` → pipe mode) and not building runner policy into +alktty. See OQ-46. + +## Consequences + +**Positive:** + +- alktty's default dependency surface is minimal (alkcall + the + wire-format codec). A docker-only or ssh-only deployment never pulls + in `portable_pty`. +- The local backend is a one-feature opt-in (`features = ["local"]`) + for the common case — a consumer that wants a terminal/runner + endpoint with no docker or SSH gets it with one feature flag, not a + separate dependency. +- PTY vs pipe is per-session, so one `LocalTtyBackend` serves terminals + and runners. A hub that does both doesn't need two backends. +- The runner pattern is preserved without baking runner policy into + alktty. The mechanism is the framed byte stream; the policy is + downstream. +- The single-crate placement composes with alknet ADR-003's + no-handler-depends-on-another-handler rule: the local backend is in + the same crate as the trait, so there is no cross-crate dependency + edge to worry about. The docker and SSH backends remain separate + crates (real external deps — `bollard`, `russh` — and their own + resource models). +- The alktty consolidation resolves the alknet cyclic-dep workaround + that motivated the original sibling-crate decision. No assembly-layer + "depend on both crates directly" pattern; no cargo-rejected + feature-re-export-from-an-optional-dep dance. + +**Negative:** + +- A consumer that wants the local backend must enable a feature flag + (`features = ["local"]`). Forgetting the flag results in the + `alktty::local` module not existing — a compile error, not a silent + miss. This is the standard Rust feature-flag trade and is + self-documenting. +- The `local` feature is non-wasm by design. A consumer that targets + `wasm32-unknown-unknown` must not enable `local` — the build error is + intentional (the local-process backend cannot run in a sandbox). The + default crate stays wasm-clean so the downstream TS/Python adapter + story works. +- The runner-specific API surface (job management, log persistence) is + not in alktty. A downstream crate that wants a full runner builds on + the pipe mode + the wire format. This is the right layering + (mechanism vs policy) but means a "runner crate" is a separate future + deliverable, not part of alktty. See OQ-46. + +## Door type + +**Two-way.** The single-crate-with-feature-gate placement is +reversible: if the local backend ever grows large enough to warrant +splitting out (e.g., a future docker or SSH backend shares enough +portable-pty bridge code to motivate a shared "blocking-backend bridge" +crate), extracting `src/local/` into a sibling crate behind the same +`local` feature is mechanical — the trait is the seam, and the feature +gate already exists. The cost of reversal is low (the module becomes a +crate; the feature gate switches from `dep:portable-pty` to +`dep:alktty-local`), and no downstream consumer breaks (the +`alktty::local::LocalTtyBackend` path stays valid if the sibling crate +re-exports it). + +This is a two-way door that is **decided** (single crate + `local` +feature), not deferred. The decision is made now; the reversal is cheap +if a future split warrants it. See alknet ADR-009 §"What this framework +is NOT" — door type classifies reversal cost, not urgency. + +## References + +- alknet ADR-003 + Amendments 1 & 2 — crate decomposition rule (the + single-crate placement preserves it; the local backend is in the same + crate as the trait, not a separate crate that depends back) +- alknet ADR-009 — door-type-as-deferral anti-pattern (this ADR's + two-way-door classification is reversal cost, not a deferral) +- alknet ADR-017 — the adapter-location-map pattern (trait where types + live, implementation where deps live) this ADR follows (the `local` + feature is where the deps live) +- [ADR-002](002-ttybackend-trait-and-ttyhandle.md) — the `TtyBackend` + trait this module implements +- [ADR-001](001-wire-format-and-two-carriage.md) — the wire format the + adapter pumps to/from this backend +- [ADR-004](004-exit-code-on-control-chunk.md) — the exit-chunk + ordering the local backend's waiter thread feeds +- [ADR-005](005-backend-cleanup-on-session-cancel.md) — the cancel- + cleanup contract the local backend's `exit_code` future's `Drop` + implements +- OQ-46 — runner API surface (deferred(scope): mechanism in alktty, + policy is a downstream crate) +- Spec: [tty-local.md](../tty-local.md) +- Port origin: alknet ADR-054 at + `/workspace/@alkdev/alknet/docs/architecture/decisions/054-local-tty-backend-sibling-crate.md` \ No newline at end of file diff --git a/docs/architecture/decisions/004-exit-code-on-control-chunk.md b/docs/architecture/decisions/004-exit-code-on-control-chunk.md new file mode 100644 index 0000000..b437b51 --- /dev/null +++ b/docs/architecture/decisions/004-exit-code-on-control-chunk.md @@ -0,0 +1,219 @@ +# ADR-004: Exit Code on a Control Chunk (the Last Chunk Before Stream Close) + +## Status + +Accepted (ported from alknet ADR-055 2026-08-17; cross-references +renumbered to alktty's ADR range — ADR-052→001, ADR-053→002, +ADR-054→003, ADR-055→004, ADR-056→005, ADR-057→006, ADR-077→007, +ADR-093→008. The alknet ADR referenced by alknet number (052, 053) is +not ported into alktty's ADR range as a separate ADR — it is ADR-001 +and ADR-002 here. The alknet originals at +`/workspace/@alkdev/alknet/docs/architecture/decisions/` remain +authoritative for any ADR not yet ported.) + +## Context + +The alknet-docker POC validated exit-code propagation for the JSON +carriage path: exec with an exit code rides on a final `call.responded` +frame `{ "exitCode": N }` before `call.completed`. That works because +the JSON carriage path is the call protocol — `call.responded` and +`call.completed` exist and carry the result. + +The raw-carriage path (ADR-001) has no `call.responded` and no +`call.completed` — after the negotiation frame, the stream is raw +chunks. The exit code must ride on the chunk format itself. Two options: + +- **(a) Control chunk**: `{"type":"exit","code":N}` as the last control + chunk (`STREAM_CTRL_OUT`, stream_type 4) before stream close. Clean, + explicit, carries the code as structured data on the channel that + already exists for control metadata. +- **(b) Final data chunk with exit code**: a special stdout chunk with + an exit-code payload. Overloads the data channel for metadata — a + client parsing stdout chunks would have to special-case "this stdout + chunk is actually an exit code," conflating data and control. + +The local-PTY POC validated option (a) end-to-end: the +`{"type":"exit","code":N}` chunk fires after the child is reaped (the +waiter thread's `oneshot::Receiver` resolves) and is the last +control chunk before the stream closes. The POC's `session.rs` +`pump_exit` task awaits `pty.exit_code`, serializes the result as +`ControlMessage::Exit { code }`, enqueues it as a control chunk, and +the drainer writes it to the client before the writer closes. + +### Why this is a one-way door + +Clients will depend on the **"exit chunk is last"** invariant: after +the exit control chunk, no more data chunks follow, and the stream +closes. This is the deterministic completion notification the docker +POC identified as the stopgap coordination property — a coordinator +spawns a process, streams its output, and gets a reliable "it exited +with code N" signal without polling or plugin state. Changing the +ordering after clients exist would break every consumer that reads +stdout until the exit chunk and then stops. + +## Decision + +### 1. The exit code rides on a control chunk, not a data chunk + +The exit code is control metadata (the process's termination status), +not data (process output). It rides on the control channel +(`STREAM_CTRL_OUT`, stream_type 4, ADR-001) as: + +```json +{"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. + +### 2. The "exit chunk is last" invariant + +After the `{"type":"exit","code":N}` control chunk: + +- The server sends no more data chunks (stdout/stderr) and no more + control chunks. +- The server closes the write half of the bidi stream. + +A client reads stdout/stderr/control chunks until it sees the exit +chunk, records the exit code, and treats subsequent stream close as the +session end. The exit chunk is the deterministic completion signal. + +### 3. The adapter owns the exit-chunk ordering, not the backend + +Per ADR-002 assumption 3, the backend resolves `exit_code` (a +`BoxFuture<'static, Result>`); the adapter awaits it, +sends the exit control chunk, and closes the stream. The backend does +not write to the wire — it produces handles; the adapter pumps. This +keeps the wire-format logic (including the "exit is last" invariant) in +one place (the adapter's session driver) and the backend focused on its +allocation target. + +The adapter's session driver (see `tty-adapter.md`) runs three +concurrent pumps: + +1. **stdout → client**: backend stdout → stdout chunks (and stderr + chunks if `TtyHandle.stderr` is `Some`). +2. **client → backend**: stdin chunks → backend stdin; control chunks → + `TtyControl::resize`/`signal`/`eof`. +3. **exit → exit chunk**: await `TtyHandle.exit_code`; on resolve, + enqueue `{"type":"exit","code":N}` as a control chunk; after the + drainer writes it, close the write half. + +The exit-chunk task coordinates with the stdout pump: the stdout pump +completes (backend stdout EOF) before or concurrently with the exit +resolve, and the exit chunk is enqueued only after the exit resolves. +The drainer writes chunks in arrival order; the exit chunk is last +because the exit is the last thing to resolve (the child must exit +before its stdout drains, but the exit chunk is sent only after +`exit_code` resolves, which is after `Child::wait()` returns — i.e., +after the child is reaped). + +### 4. Error exit codes + +A backend `TtyError` during `allocate()` (the PTY couldn't be +allocated, the docker exec failed to start, the SSH channel request was +rejected) is handled before the raw-carriage phase begins — the adapter +sends a JSON error response to the negotiation frame and closes the +stream without entering raw mode. See `tty-adapter.md` §"Negotiation +errors". + +A `TtyError` from the `exit_code` future (the child couldn't be reaped, +or the backend's wait path failed) is serialized as an exit code of +`-1` (`ControlMessage::Exit { code: -1 }`) and the stream closes. The +client treats `-1` as "the backend reported an exit error, not a real +exit code." This is a best-effort signal; a backend that cannot +determine the exit code still sends the exit chunk so the client gets +the completion notification. + +## Consequences + +**Positive:** + +- The exit code is structured data on the control channel, not a hacky + overload of the data channel. Clients parse it as a + `ControlMessage::Exit`, not as a special-cased stdout chunk. +- The "exit chunk is last" invariant gives coordinators deterministic + completion notification — the same stopgap property the docker POC + validated for logs subscriptions. No polling, no plugin state; the + process exiting is the signal. +- The adapter owns the ordering, so the invariant is enforced in one + place; backends don't have to know the wire format's completion + semantics. +- The error-exit `-1` fallback keeps the completion notification + reliable even when the backend can't determine the real code — the + client still knows the session ended. + +**Negative:** + +- The "exit chunk is last" invariant is a one-way door — clients depend + on it. Reversing it (allowing data chunks after the exit chunk, or + moving the exit code to a data chunk) would break every consumer. + This is the intended commitment: the invariant is the value. +- A client that doesn't read until the exit chunk (e.g., a runner that + cancels mid-stream by closing the write half) won't see the exit + 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 + (`{"type":"exit_error","message":"..."}`) can carry the distinction + additively (the `type`-tagged enum is the extension seam per + ADR-001). + +## Door type + +**One-way.** The "exit chunk is last" invariant is what clients depend +on for deterministic completion. Changing it after clients exist breaks +every consumer. The `{"type":"exit","code":N}` shape is also one-way +(clients parse it as a `ControlMessage::Exit`), though the +`type`-tagged enum (ADR-001) makes adding *new* control message types +additive. + +## Assumptions + +1. **The child exits before its stdout fully drains, and the exit chunk + is sent after `exit_code` resolves.** On Unix, `Child::wait()` + blocks until the child is reaped, which happens after the child + exits and its stdout pipe/PTY buffer drains. The POC validated this + ordering: the reader thread sees EOF (buffer drained), the waiter + thread reaps (exit code available), and the exit chunk is enqueued + after the exit resolves. There is no race where stdout chunks arrive + after the exit chunk. + +2. **`exit_code` resolving implies the stdout pump is done or will be + soon.** The adapter's session driver waits for both the stdout pump + to complete (backend stdout EOF) and the exit to resolve before + sending the exit chunk and closing. If a backend's stdout outlives + the exit resolve (a hypothetical backend where the process exits but + a buffer flush is still in flight), the adapter waits for the stdout + pump before the exit chunk. The `TtyHandle.stderr` (if `Some`) is + pumped concurrently with stdout and also drains before the exit + chunk. + +## References + +- [ADR-001](001-wire-format-and-two-carriage.md) — the wire format + (control channel, `STREAM_CTRL_OUT` stream_type 4) this ADR's exit + chunk rides on +- [ADR-002](002-ttybackend-trait-and-ttyhandle.md) — the + `TtyHandle.exit_code` field (the `Future` the adapter awaits) this + ADR's ordering consumes +- [ADR-005](005-backend-cleanup-on-session-cancel.md) — the cancel path + that bypasses this ADR's happy-path ordering (no exit chunk is sent + on cancel — the stream is gone) +- [ADR-008](008-channels-pure-channel-multiplexing.md) — the + exit-chunk-is-last invariant generalizes to channels mode (the exit + chunk is the last `STREAM_CTRL_OUT` chunk before the channel closes) +- Spec: [tty-adapter.md](../tty-adapter.md) (the session driver that + enforces the ordering) +- Port origin: alknet ADR-055 at + `/workspace/@alkdev/alknet/docs/architecture/decisions/055-exit-code-on-control-chunk.md` \ No newline at end of file diff --git a/docs/architecture/decisions/005-backend-cleanup-on-session-cancel.md b/docs/architecture/decisions/005-backend-cleanup-on-session-cancel.md new file mode 100644 index 0000000..6e7281c --- /dev/null +++ b/docs/architecture/decisions/005-backend-cleanup-on-session-cancel.md @@ -0,0 +1,298 @@ +# ADR-005: Backend Cleanup on Session Cancel (Drop of `exit_code` Kills) + +## Status + +Accepted (ported from alknet ADR-056 2026-08-17; cross-references +renumbered to alktty's ADR range — ADR-052→001, ADR-053→002, +ADR-054→003, ADR-055→004, ADR-056→005, ADR-057→006, ADR-077→007, +ADR-093→008. The alknet ADR referenced by alknet number (052, 053, 054, +055) is not ported into alktty's ADR range as a separate ADR — it is +ADR-001, 002, 003, 004 here. The alknet originals at +`/workspace/@alkdev/alknet/docs/architecture/decisions/` remain +authoritative for any ADR not yet ported.) + +## Context + +A `alk/tty` session can be cancelled before the child process exits +naturally. Three cancel paths exist (see `tty-adapter.md` §"Connection +and Stream Lifecycle"): + +1. **Connection drop** — the QUIC connection closes; all in-flight + sessions on it are cancelled. +2. **Stream reset** — the client (or transport) resets the bidi stream + mid-session. +3. **Task panic / adapter shutdown** — the session pump task exits + without completing the normal exit-chunk sequence. + +In all three, the adapter's per-session pump tasks are dropped (Rust +`Drop`). The pump tasks hold the `TtyHandle`, whose fields — `stdin`, +`stdout`, `stderr`, `exit_code`, `control` — are dropped too. + +The problem the local-PTY POC surfaced: **`Drop` alone is not sufficient +cleanup for a backend whose child may outlive the session.** The local +backend's three std threads illustrate the gap: + +- **Reader thread** — blocking `MasterPty::try_clone_reader()` reads. + On `mpsc::Sender::drop` (the stdout channel's sender side drops when + the handle drops), the reader's next `blocking_send` returns a + `SendError`; the thread exits. ✓ `Drop` works. +- **Writer thread** — drains an `mpsc::Receiver`. On + `Receiver::drop`, the writer's `recv()` returns `None`; the thread + drops the master writer (EOF to the slave's stdin) and exits. ✓ + `Drop` works. +- **Waiter thread** — blocking `Child::wait()`. This is a syscall that + returns **only when the child is reaped**. It does not observe + channel close. If the child ignores stdin EOF (a daemon, a + long-lived process with no stdin reader, a process in an + uninterruptible state), the waiter thread stays blocked indefinitely + and the child is **orphaned**. ✗ `Drop` does not work here. + +The same concern applies to any backend whose session target can +outlive the bidi stream: a docker container with `tty: true` whose +process ignores the channel close; an SSH exec whose remote process +doesn't exit on channel close. The unifying property is **a +backend-allocated session target that may outlive the client's interest +in it.** + +The earlier spec text (`tty-adapter.md` §"Connection and Stream +Lifecycle" pre-this-ADR) asserted: + +> No explicit cleanup is needed — `Drop` is the cleanup. + +That is wrong for the waiter thread and any backend in the same shape. +This ADR corrects the claim and commits a cleanup contract that closes +the gap. + +### Why this is architectural, not implementation + +The cleanup contract is part of the `TtyBackend` trait's behavioral +contract (ADR-002), not a backend-internal detail, for two reasons: + +1. **The adapter depends on the property.** The adapter's session + driver holds the `TtyHandle` and, on cancel, drops it. The adapter + cannot itself call a backend-specific kill — it doesn't know the + child's pid (the local backend owns it; the adapter never sees it). + The kill must be wired into the backend's handle shape, specifically + into the `exit_code` future the adapter drops on cancel. The + contract is what makes "the adapter drops the handle" sufficient. +2. **A missing contract leaks processes.** An implementer writing a + backend from the trait sketch, without this contract, would ship a + backend that orphans processes on cancel. The bug is silent (the + client got what it wanted; the orphaned process is a server-side + leak), surfaces only under cancel-heavy workloads or long-lived + sessions, and is expensive to attribute after the fact. The + contract makes the property load-bearing at the seam, not a property + each backend rediscovers. + +## Decision + +### 1. The `TtyBackend` cleanup contract: cancelling the `exit_code` future kills the session target + +When the adapter cancels a session (drops the pump tasks, which drops +the `TtyHandle`), the backend's `exit_code` future — the +`BoxFuture<'static, Result>` field of `TtyHandle` +(ADR-002) — is dropped *without being driven to completion*. The +cleanup contract: + +> **Dropping the `exit_code` future MUST kill the session target (the +> child process, the docker exec, the SSH channel's process).** The +> kill is best-effort (the target may already be exiting; the kill is +> a no-op then), but it MUST be attempted. The kill MUST be delivered +> even when the session target is blocked in a state that ignores +> stdin EOF (a daemon, a process in uninterruptible sleep, a container +> whose process ignores channel close). + +`exit_code`'s `Drop` is the cancel path. The adapter drives the future +to completion (the happy path — the child exits, the future resolves, +the adapter sends the exit chunk); on cancel, the adapter drops the +future (their `Drop`), which runs the kill. + +This is a behavioral contract on the `TtyBackend` trait, not a new +method. The trait's `allocate()` returns a `TtyHandle` whose +`exit_code` field is a `Future` with a `Drop`-on-cancel that kills. The +mechanism is backend-specific (see §3 for the local backend); the +contract is backend-agnostic. + +### 2. `exit_code`'s `Drop`-on-cancel MUST be safe to run after the future resolves + +If the future resolved normally (the adapter awaited it, got the exit +code, sent the exit chunk), the `Drop` runs on an already-resolved +future. The kill MUST be a no-op in that case — the child is already +reaped, the kill is delivered to a nonexistent pid, etc. This is the +"best-effort" qualifier: a kill on an already-exited child is not an +error. Backends implement this with a guard that distinguishes +"resolved" from "cancelled" (a flag, an `Option` taken on resolve, an +`Arc`-shared state). + +### 3. Local backend mechanism: `ChildKiller` held in the `exit_code` future's `Drop` guard + +The local backend (ADR-003) implements the contract using +`portable_pty::ChildKiller` — the kill handle `portable_pty` exposes +alongside `Child::wait()`. The pattern: + +- `allocate()` spawns the child via `portable_pty`, obtaining a `Child` + (with `wait()`) and a `ChildKiller` (with `kill()`). It moves the + `Child` into the waiter thread (which blocks on `wait()`). It wraps + the `ChildKiller` and the `oneshot::Receiver` (from the waiter + thread) into a `Future` that becomes `TtyHandle.exit_code`. +- The `exit_code` future's `poll` delegates to the inner + `oneshot::Receiver::poll` (resolves when the waiter thread sends the + exit code). +- The `exit_code` future's `Drop` (runs on cancel only — on resolve, + the `Drop` is a no-op via the guard) calls `ChildKiller::kill(SIGHUP)` + (or the backend's configured cancel signal). The kill causes the + child to exit; the waiter thread's `wait()` returns; the waiter + thread's `oneshot::send` fails silently (the receiver was dropped + with the future). The waiter thread then exits. The child is reaped + by the waiter thread's `wait()`; no zombie. + +For pipe mode (`terminal: None`), the same pattern applies with +`tokio::process::Child::start_kill()` (or `Child::kill()`) instead of +`ChildKiller`. The `exit_code` future's `Drop` guard calls +`start_kill()`; the `Child` is reaped by the future's `wait()` (or by +the waiter task). + +### 4. Future backends (docker, SSH) follow the same contract + +- **Docker (`DockerTtyBackend`)** — `bollard`'s exec stream is + cancelled by dropping the `AttachContainer` / `start_exec` stream + and calling `bollard::container::kill_container` (or `exec::kill_exec` + if available). The `exit_code` future's `Drop` holds the + container/exec id and the `bollard::Docker` client; on cancel, it + issues the kill. +- **SSH (`SshTtyBackend`)** — russh's `Channel::close()` and/or + `Channel::signal(SIGHUP)` terminate the remote process. The + `exit_code` future's `Drop` holds the russh channel handle; on + cancel, it closes the channel. + +The docker and SSH backends are future work (out of scope for this +spec set); the contract is what they implement. A future backend that +does NOT fit the contract (e.g., a "recorded session replay" backend +with no live process) implements a no-op `Drop`-on-cancel — the +contract is "kill if there is a killable target; no-op if not." + +### 5. The adapter does not call a backend kill method + +The adapter has no `TtyBackend::cancel()` or `TtyHandle::kill()` method +to call — the cleanup is wired into the `exit_code` future's `Drop`, +which the adapter triggers by dropping the future. This keeps the +trait surface unchanged (no new method) and the cleanup in the backend +(where the kill handle lives). The adapter's only responsibility is to +drop the `TtyHandle` (and therefore the `exit_code` future) when the +session is cancelled — which it already does by virtue of dropping the +pump tasks. + +The `TtyControl::signal("HUP")` path (ADR-002) is the +*client-initiated* signal forwarding path — a client sends a +`{"type":"signal","name":"HUP"}` control chunk. It is NOT the +cancel-cleanup path. The cancel-cleanup path is server-internal (the +adapter drops the handle) and does not involve the wire format. These +are two different signal paths; both end in the child receiving SIGHUP +(or the backend's configured cancel signal), but they are triggered by +different actors (client vs. server cancel). + +## Consequences + +**Positive:** + +- A backend that conforms to the contract cannot orphan a process on + cancel. The local-PTY POC's waiter-thread gap is closed at the + contract level, not left to each backend to rediscover. +- The cleanup is idiomatic Rust — `Drop`-on-cancel of a `Future` is + the standard pattern for resource cleanup in async Rust (the same + pattern `tokio::process::Child` uses; the same pattern `tokio::io` + AsyncRead guards use). No new trait method; no adapter-side kill + call. +- The contract is backend-agnostic — the mechanism (`ChildKiller` for + local, `kill_container` for docker, `channel::close` for SSH) lives + in the backend; the contract ("drop the future, the target dies") + lives at the seam. +- The happy path (the child exits, the adapter drives `exit_code` to + completion, sends the exit chunk, then drops the resolved future) is + unaffected — the `Drop`-on-resolve is a no-op via the guard. + +**Negative:** + +- The `exit_code` future is no longer a trivial `oneshot::Receiver` + wrapper; it carries a kill guard. This is a small implementation + complexity increase (a struct with a `Drop` impl and a + resolved-flag), but it is the cost of the contract. The POC's + `LocalPty::exit_code` was a bare `oneshot::Receiver`; the + spec'd `TtyHandle.exit_code` is a struct wrapping it. An implementer + who copies the POC's bare shape without the kill guard violates the + contract. +- The contract is behavioral, not type-enforced. Rust cannot require + "the `Drop` of the future returned by `allocate()` kills the child" + in the type system. The contract is documented in the `TtyBackend` + trait's doc comment and in this ADR; conformance is the + implementer's responsibility. A test (a "cancel mid-session" test + that asserts the child is reaped after the session is dropped) + should be part of each backend's integration suite. +- A backend whose session target genuinely cannot be killed (a + backend that wraps an immutable shared resource, e.g., a "view a + log stream" backend) implements the contract as a no-op. The + contract is "best-effort kill if there is a killable target"; a + no-op `Drop`-on-cancel is conformant for a non-killable target. + +## Door type + +**One-way.** The cleanup contract is part of the `TtyBackend` +behavioral contract. Clients (the adapter) depend on "drop the handle, +the session is cleaned up." Changing the contract after backends exist +— e.g., adding a separate `TtyBackend::cancel()` method and migrating +the cleanup out of `exit_code`'s `Drop` — would require every backend +to change. The `exit_code`-future-`Drop`-on-cancel mechanism is the +seam. + +## Assumptions + +1. **The `exit_code` future's `Drop` is the only cancel path.** The + adapter does not call a separate kill method; it drops the handle. + This means the cleanup runs in the same place the cancel happens + (the pump task's `Drop`), not in a separate cancel call. This is + the idiomatic Rust async cancel pattern and the one the trait + commits. +2. **The kill signal is the backend's configured cancel signal (SIGHUP + for the local backend, the docker/SSH equivalent).** This is a + server-internal signal path, distinct from the + client-initiated `TtyControl::signal()` path (ADR-002). The cancel + signal is not configurable from the wire format in v1; a backend + that needs a different cancel signal configures it internally. +3. **The waiter thread (local backend) reaps the killed child.** After + the `Drop`-on-cancel calls `ChildKiller::kill(SIGHUP)`, the child + exits; the waiter thread's `wait()` returns and reaps it (no + zombie). The waiter thread then exits. The `oneshot::send` from the + waiter thread fails silently (the receiver was dropped with the + future) — this is expected and not an error. + +## References + +- [tty-adapter.md](../tty-adapter.md) §"Connection and Stream + Lifecycle" — the cancel paths (connection drop, stream reset) that + trigger the contract +- [tty-local.md](../tty-local.md) §"Cancel-Cleanup (ADR-005)" — the + local backend's three-thread bridge and the waiter-thread gap this + ADR closes +- [ADR-001](001-wire-format-and-two-carriage.md) — the wire format the + cancel does not involve (the cleanup is server-internal) +- [ADR-002](002-ttybackend-trait-and-ttyhandle.md) — the `TtyBackend` + trait this contract is part of; the `exit_code` field the cleanup + wires into; the `TtyControl::signal()` path the cancel-cleanup path + is distinct from +- [ADR-003](003-local-backend-placement.md) — the local backend this + ADR's reference mechanism (`ChildKiller`) is for +- [ADR-004](004-exit-code-on-control-chunk.md) — the happy-path + exit-chunk sequence (the cancel path bypasses it; no exit chunk is + sent on cancel — see `tty-adapter.md` §"Stream reset") +- `src/local/pty.rs` — the local backend's `LocalExitFuture` (the + `Future` + `Drop` guard this ADR specifies, with the `ChildKiller` + held in the guard) +- `src/local/pipe.rs` — the pipe-mode equivalent + (`tokio::process::Child::start_kill()` on `Drop`-on-cancel) +- `portable-pty` 0.9 `ChildKiller` — the kill handle the local + backend's cancel-cleanup uses +- Spec: [tty-backend.md](../tty-backend.md), + [tty-adapter.md](../tty-adapter.md), + [tty-local.md](../tty-local.md) +- Port origin: alknet ADR-056 at + `/workspace/@alkdev/alknet/docs/architecture/decisions/056-backend-cleanup-on-session-cancel.md` \ No newline at end of file diff --git a/docs/architecture/decisions/006-negotiation-framing-self-contained.md b/docs/architecture/decisions/006-negotiation-framing-self-contained.md new file mode 100644 index 0000000..c612c98 --- /dev/null +++ b/docs/architecture/decisions/006-negotiation-framing-self-contained.md @@ -0,0 +1,190 @@ +# ADR-006: Self-Contained Negotiation Framing (No alkcall-Internal-Wire-Types Dependency) + +## Status + +Accepted (ported from alknet ADR-057 2026-08-17; the dependency-edge +decision carries over unchanged — alktty depends on alkcall (for the +`ProtocolHandler` trait, auth, ownership types) but not on alkcall's +internal wire types. The framing is self-contained in +`src/negotiation.rs`. Cross-references renumbered to alktty's ADR +range — ADR-052→001, ADR-053→002, ADR-054→003, ADR-055→004, +ADR-056→005, ADR-057→006, ADR-077→007, ADR-093→008. The alknet ADR +referenced by alknet number (003) is not ported into alktty's ADR +range — it is an alknet-core ADR. The alknet originals at +`/workspace/@alkdev/alknet/docs/architecture/decisions/` remain +authoritative.) + +## Context + +The alknet-tty specs (alknet ADR-052, 053, 054) and alknet ADR-003 +Amendment 1 previously stated that alknet-tty depends on alknet-call for +the `FrameFramedReader`/`FrameFramedWriter` "framing utility" — the +4-byte big-endian length prefix + UTF-8 JSON body framing the +negotiation frame uses. The claim was "reuse the framing utility, not +the `EventEnvelope` type" (alknet ADR-052 §6). + +A pre-implementation sanity check surfaced that **the framing utility +is not actually reusable as the spec described.** +`FrameFramedReader` (in alkcall's protocol/wire module) is hardcoded to +deserialize `EventEnvelope`: + +```rust +pub async fn read_frame(&mut self) -> Result { + // ... read 4-byte length prefix, read body ... + let envelope: EventEnvelope = serde_json::from_slice(&body)?; + Ok(envelope) +} +``` + +The framing logic (read 4 bytes → length, read N bytes) and the +`EventEnvelope` deserialization are entangled in the same method. The +"framing utility" the spec claimed to reuse does not exist as a +separable thing — the length-prefix read and the type-specific +deserialize are one call. alktty's negotiation frame is a +`NegotiateRequest`, not an `EventEnvelope`, so `read_frame()` cannot +return what alktty needs. + +This left three options (see alknet ADR-003 Amendment 2 for the full +comparison): + +1. **Duplicate the ~30 lines of framing logic in alktty.** The framing + is a trivial length-prefix idiom (4-byte BE length + body); the two + copies would share an idiom, not a domain abstraction. +2. **Promote a generic length-prefixed framing utility to alkcall::core.** + Makes the spec's claim true (a reusable utility exists), but + accretes a framing module to the foundation crate for the sake of + two consumers — a shared utility pays for itself at ≥2 consumers, + but the framing is trivial enough that the cost of the shared + abstraction (a new module, a new type, a refactor of alkcall) + exceeds the cost of the duplication. +3. **Actually use alkcall** (e.g., model the tty control channel as + call-protocol operations). A different architecture, not a + dependency-edge fix — the current `ControlMessage` tagged enum + (ADR-001) is the two-way-door seam; replacing it with the call + protocol is a v2 ALPN decision, not a v1 dependency choice. + +## Decision + +### 1. alktty does not depend on alkcall's internal wire types + +alktty implements its own length-prefixed framing for the negotiation +frame directly on tokio's `AsyncRead`/`AsyncWrite`. The crate's +dependency edge is: + +``` +alktty +└── alkcall (ProtocolHandler, Connection, AuthContext, Identity, + AccessControl, OwnershipProvider — alknet ADR-050) +``` + +No alkcall-internal-wire-types dependency. The crate depends on alkcall +(which every handler crate depends on anyway for the `ProtocolHandler` +trait) and nothing else for the protocol surface. `portable_pty`, +`bollard`, `russh` remain in the backend crates / the `local` feature +(ADR-003). + +### 2. The framing format coincides with alkcall's by convention, not by code reuse + +Both alktty's negotiation frame and alkcall's `EventEnvelope` frame use +a 4-byte big-endian length prefix + UTF-8 JSON body. This is a shared +*format convention* (length-prefixed JSON is a standard framing +pattern), not a code dependency. The two implementations are +independent: alktty's reader deserializes `NegotiateRequest`; alkcall's +`FrameFramedReader` deserializes `EventEnvelope`. They share an idiom +(length-prefix framing), not a module. + +### 3. The framing logic lives in alktty as a small, self-contained module + +alktty implements the negotiation framing as a small module +(`src/negotiation.rs`, ~30 lines for the framing reader/writer: read +4-byte BE length, bounds-check, read N bytes; write the inverse). The +module's types (`NegotiationReader`/`NegotiationWriter`) are private to +the crate — they are not a reusable utility for other crates. If a +future crate wants length-prefixed JSON framing, it implements its own +(the idiom is trivial) or a future ADR promotes a generic utility to +alkcall::core at that point (deferred — not needed for the current +scope; two consumers is the threshold but the second consumer does not +yet exist). + +## Consequences + +**Positive:** + +- alktty's dependency surface is minimal and correct: alkcall only. No + dependency on alkcall's internal wire types for a "framing utility" + that wasn't reusable as specced. The "weird" dependency edge (a + handler crate depending on another handler crate's internal wire + module for 30 lines of glue) is gone. +- The spec is honest: it describes what the code does (a + self-contained framing module) rather than what a previous draft + hoped for (a reusable utility in alkcall that doesn't exist in a + separable form). +- The framing logic is trivial and self-contained; bugs in it are + local to alktty (no cross-crate coordination if alkcall's framing + changes for call-protocol reasons). +- alknet ADR-003's "no handler crate depends on another handler crate's + internal wire types" rule is preserved without the Amendment 1 + exception for alktty. (alknet ADR-003 Amendment 1's exception + remains for alknet-http/agent/napi, which use alkcall's + `OperationSpec`/`Handler`/`OperationAdapter` types — actual type + reuse, not framing glue. alktty does not need that exception.) + +**Negative:** + +- ~30 lines of framing logic are duplicated between alktty and alkcall. + The duplication is an idiom (length-prefix framing), not a domain + abstraction; the cost of the shared abstraction (a new module in + alkcall::core + a refactor of alkcall) exceeds the cost of the + duplication for two consumers. If a third consumer appears, this + trade-off should be revisited (promote to alkcall::core). +- A bug found in the length-prefix framing edge cases (e.g., a + partial-read handling bug) would need fixing in two places. The + framing is mature (a standard `read_exact`-based pattern); the edge + cases are known and tested in both crates independently. + +## Door type + +**One-way.** alktty not depending on alkcall's internal wire types is a +dependency-edge commitment. Adding the dependency back later (if, e.g., +a generic framing utility is promoted to alkcall::core) is a new ADR. +The framing logic being self-contained in alktty is two-way (it could +be refactored to a shared utility in alkcall::core later without a +wire-format change), but the dependency edge is one-way. + +## Assumptions + +1. **The framing logic is trivial enough that duplication is cheaper + than a shared abstraction.** Length-prefixed framing (4-byte BE + length + body) is a ~30-line idiom. The cost of a shared utility in + alkcall::core (a new module, a new type, a refactor of alkcall's + wire module to extract the generic layer) is higher than the cost of + two independent implementations for two consumers. If a third + consumer appears, revisit (promote to alkcall::core). + +2. **The format coincidence (both use 4-byte BE length + JSON body) is + stable.** Both crates use the same length-prefix convention. If + alkcall's framing changes (e.g., a different max-frame-size, a + different prefix width), alktty's is unaffected — they are + independent implementations that happen to share a format today. + +## References + +- alknet ADR-003 Amendment 1 (protocol-foundation exception for + alknet-http/agent/napi) and Amendment 2 (this ADR's effect on the + Amendment 1 framing-reuse claim for alktty) +- [ADR-001](001-wire-format-and-two-carriage.md) §2 (negotiation frame + format), §6 (revised: format coincides by convention, not by code + reuse) +- [ADR-002](002-ttybackend-trait-and-ttyhandle.md) — the crate + decomposition this ADR's dependency edge affects +- `src/negotiation.rs` — the self-contained framing reader/writer this + ADR commits (`NegotiationReader`, `NegotiationWriter`, + `error_response_bytes`) +- alkcall's `FrameFramedReader`/`FrameFramedWriter` — the + `EventEnvelope`-bound methods that are NOT reused (the entangled + length-prefix-read + `EventEnvelope`-deserialize that motivated this + ADR) +- Spec: [overview.md](../overview.md) (dependency edge), + [tty-wire.md](../tty-wire.md) (negotiation framing) +- Port origin: alknet ADR-057 at + `/workspace/@alkdev/alknet/docs/architecture/decisions/057-alknet-tty-no-alknet-call-dep.md` \ No newline at end of file diff --git a/docs/architecture/decisions/007-tty-inside-channels.md b/docs/architecture/decisions/007-tty-inside-channels.md new file mode 100644 index 0000000..af12209 --- /dev/null +++ b/docs/architecture/decisions/007-tty-inside-channels.md @@ -0,0 +1,270 @@ +# ADR-007: TTY Inside Channels — Sub-Streams, Not Wire Format + +## Status + +Accepted (**reversed 2026-07-18 by alknet ADR-093, ported here as +[ADR-008](008-channels-pure-channel-multiplexing.md): TTY always uses +its 5-byte format; the channels layer carries it transparently in the +payload — see "Reversal (ADR-008, 2026-07-18)" below.** Ported from +alknet ADR-077 2026-08-17; cross-references renumbered to alktty's +ADR range — ADR-052→001, ADR-053→002, ADR-054→003, ADR-055→004, +ADR-056→005, ADR-057→006, ADR-077→007, ADR-093→008. The alknet ADRs +referenced by alknet number (052, 053, 055, 057, 061, 071, 073, 074, +092) are not ported into alktty's ADR range — they are alknet-core / +alknet-channels ADRs. The alknet originals at +`/workspace/@alkdev/alknet/docs/architecture/decisions/` remain +authoritative.) + +## Reversal (ADR-008, 2026-07-18) + +The two-mode TTY design (direct vs inside-channels, with different +sub-stream access paths) is **reversed**. TTY's 5-byte format +(`[stream_type:u8][length:u32][payload]`, ADR-001) is TTY's internal +format, used in **both** direct mode and inside-channels mode. The two +modes differ only in *where the `BiStream` comes from* (a top-level +`alk/tty` connection vs a `channel/open` with ALPN `alk/tty`), not in +*how TTY parses it*. The same `wire.rs` code runs in both modes. + +When TTY is inside channels, the channels layer strips its 8-byte header +(alknet ADR-093 / ADR-008) and hands TTY the payload bytes. TTY parses +its 5-byte header from the payload. The channels layer carries TTY's +5-byte chunks transparently in its payload — no shared fields, no +leaked abstraction, no double-chunking concern (the 13-byte total +header is 8 channels + 5 TTY, not 8 + 9; the channels `length` is +always `tty_len + 5`). + +The `channels` feature on alktty (if such a feature were added) becomes +"run TTY's sub-demux on a channels-backed `BiStream`" — the same code +as direct mode, different `BiStream` source. The control channel split +(`STREAM_CTRL_IN` / `STREAM_CTRL_OUT`, Phase 7) is TTY-internal; the +channels layer doesn't know about it. alknet ADR-074's +`into_sub_streams()` (the accessor this ADR's two-mode design relied +on) is removed by alknet ADR-093 / ADR-008; TTY sub-demuxes its +`BiStream` via its own 5-byte format instead. + +The body below describes the **original** (two-mode) shape; the +reversal above is the operative decision. The two-mode description is +kept as the historical context for the reversal. See ADR-008 for the +resolution rationale (the channels layer has no `stream_type` concept; +the handler owns its sub-stream multiplexing) and the cross-ADR +impacts. + +## Context + +ADR-001 defines the alktty wire format: `[stream_type: u8][length: u32 +be][payload]`, a 5-byte chunk header for five sub-streams +(stdin/stdout/stderr/ctrl-in/ctrl-out) within one bidi stream. This +format is stable, implemented (`src/wire.rs`), and used for direct +`alk/tty` connections. + +The phase-0 research recommended that the TTY chunk format be "absorbed +into channels" and that `alk/tty` remain as a "direct-connect +shortcut." But the research did not pin what changes in the TTY crate +when a TTY session runs *inside* a channels connection. This is a real +integration question the research hand-waved. + +The problem: the `TtyAdapter`'s current `handle()` loops `accept_bi()`, +spawning a `drive_session` task per bidi stream that parses 5-byte TTY +chunks off the stream. Inside a channels connection, the stream is +*already de-chunked* by the channels layer's 9-byte format — the +handler sees an `AsyncRead + AsyncWrite` pair, not a chunk-encoded +stream. If the TTY adapter tries to parse 5-byte chunks off an +already-de-chunked stream, it breaks. + +## Decision + +### Two modes for TTY, one adapter + +The `TtyAdapter` operates in two modes, determined by how it receives +its `Connection`: + +| Mode | When | Wire format | How the adapter gets sub-streams | +|------|------|-------------|---------------------------------| +| **Direct (`alk/tty` ALPN)** | Top-level QUIC/TCP connection with ALPN `alk/tty` | TTY's 5-byte format (ADR-001, amended — see below) | `accept_bi()` → parse 5-byte chunks → split into stream_types 0-4 | +| **Inside channels** | `channel/open` with ALPN `alk/tty` on a channels connection | Channels' 9-byte format (alknet ADR-071) — the channels layer de-chunks | `into_sub_streams()` (alknet ADR-074) → five named handles for stream_types 0-4 | + +In both modes, the `TtyBackend` trait and `TtyHandle` are unchanged +(ADR-002). The backend allocates a PTY and returns a `TtyHandle`; the +adapter pumps data between the handle and the sub-streams. The +difference is only in how the adapter gets the sub-streams — 5-byte +chunk parsing (direct) vs. `into_sub_streams()` (channels). + +### The control channel is now properly bidirectional + +The phase-0 findings flagged that the TTY control channel "isn't +actually bidirectional… the adapter ignores Exit from the client." The +root cause: `stream_type 3` was "bidirectional" — one stream both sides +wrote to, which is not properly multiplexed. + +alknet ADR-071's stream_type decomposition fixes this: **every +stream_type is unidirectional.** Control is now two halves: + +| stream_type | direction | purpose | +|-------------|-----------|---------| +| 3 | write (client→server) | control in: resize, signal, eof | +| 4 | read (server→client) | control out: exit, keepalive response | + +The TTY adapter writes resize/signal/eof to `ctrl_in` (stream_type 3) +and reads exit/keepalive from `ctrl_out` (stream_type 4). Each has its +own reassembly buffer, its own flow control, its own EOF. The control +channel is *actually* bidirectional — two unidirectional streams, not +one shared stream both sides write to. + +This amends ADR-001's stream_type assignments for direct mode too: +direct `alk/tty` connections now use stream_types [0, 1, 2, 3, 4] (data +in/out/err + control in/out), not [0, 1, 2, 3]. The 5-byte format's +`stream_type` field gains value 4; the `ControlMessage` enum is +unchanged (the JSON shape is the same; the stream_type it rides on +splits from 3 into 3+4). + +### What changes in alktty + +1. **The adapter's session-driving code splits into two entry + points:** + - `drive_session_direct(send, recv, backends, ...)` — the existing + path: parse 5-byte chunks, split into stream_types 0-4, pump. Used + for direct `alk/tty` connections. + - `drive_session_channels(sub_streams, backends, ...)` — the new + path: receive `ChannelSubStreams` (five named handles: stdin= + SendStream, stdout=RecvStream, stderr=Option, + ctrl_in=SendStream, ctrl_out=RecvStream), pump directly without + chunk parsing. Used when the channel's `Connection` is backed by + `ChannelBidiStreamSource`. + +2. **The `TtyAdapter::handle()` branches on the `Connection`'s source + type.** The channels crate's `ChannelBidiStreamSource` is a + `BidiStreamSource` (alknet ADR-070); the `Connection` wraps it. The + adapter detects whether the `Connection` is channels-backed (via a + downcast or a channels-crate extension trait — exact ergonomics per + alknet ADR-074's implementation detail) and calls + `drive_session_channels` instead of `drive_session_direct`. + + **This is the one place alktty knows about channels.** It is a + branch on the connection source, not a dependency on channels' wire + format. The branch can be feature-gated (`channels` feature on + alktty) so the direct-only path has no channels dependency. + +3. **The 5-byte wire format (ADR-001) is unchanged for direct + connections.** ADR-001's scope is now "the wire format for direct + `alk/tty` connections." The channels path does not use it. This + amends ADR-001's scope — the format is not replaced, it's scoped. + +4. **The control channel works the same in both modes, now properly + bidirectional.** In direct mode, control-in JSON rides in 5-byte + chunks with `stream_type=3` and control-out rides with + `stream_type=4`. In channels mode, control-in rides in 9-byte chunks + with `stream_type=3` (write) and control-out with `stream_type=4` + (read) — but the channels layer de-chunks them, so the adapter + reads raw JSON bytes from `ctrl_in`/`ctrl_out` in both cases. The + `ControlMessage` enum (resize, signal, eof, exit) is unchanged — + the JSON shape is the same; only the stream_type assignments change + (3 splits into 3+4). + +5. **The exit-chunk-is-last invariant (ADR-004) generalizes.** In + direct mode, the exit chunk is the last 5-byte chunk on + `stream_type=4` (read, server→client) before stream close (ADR-004, + amended). In channels mode, the exit control message is the last + data on `stream_type=4` before `channel/close` is sent on channel 0 + (alknet ADR-073 §channel/close). The ordering invariant is the same + — exit before close — but the mechanism differs: 5-byte chunk + ordering on stream_type 4 (direct) vs. `stream_type 4` ordering + + `channel/close` after pump completion (channels). + +### What does NOT change + +- **`TtyBackend` trait, `TtyHandle`, `TtyControl`** (ADR-002) — + unchanged. Backends don't know about channels or direct mode. +- **`DockerTtyBackend`, `LocalTtyBackend`** — unchanged. They implement + `TtyBackend::allocate()` and return a `TtyHandle`. +- **`ControlMessage` enum** — unchanged. The JSON shape is the same in + both modes. +- **The `alk/tty` ALPN string** — unchanged. Direct connections use it; + channels `channel/open` requests it. + +### Crate dependency + +`alktty` does **not** depend on `alknet-channels` unconditionally. The +channels-integration code is behind a `channels` feature on `alktty`. +When the feature is off, `TtyAdapter` only supports direct mode (the +existing behavior). When the feature is on, the adapter branches into +channels mode for channels-backed connections. This preserves alknet +ADR-003's no-handler-depends-on-another-handler rule for the default +build; the feature-gated dependency is opt-in, same as `alknet-docker`'s +`tty` feature (alknet ADR-061). + +## Consequences + +**Positive:** +- The TTY crate's direct mode is unchanged — existing `alk/tty` + deployments (browser terminals over WebSocket, direct QUIC TTY) keep + working with the 5-byte format. +- The channels path uses the channels layer's de-chunking — no + double-chunking (5-byte inside 9-byte). The TTY adapter sees clean + sub-streams. +- The `TtyBackend` trait is insulated — backends don't know which mode + the adapter is in. Docker, SSH, and local backends work in both + modes without changes. +- The control channel and exit-chunk invariant carry forward cleanly — + the `ControlMessage` enum and ordering semantics are + mode-independent. + +**Negative:** +- `alktty` has two session-driving entry points + (`drive_session_direct` vs `drive_session_channels`). This is the + necessary cost of supporting both direct and channels modes without + double-chunking. The alternative (always use channels format, even + for direct) would break existing direct deployments and add 4 bytes + of overhead per chunk for no benefit. +- The `channels` feature on `alktty` adds a dependency edge (`alktty` + → `alknet-channels`, feature-gated). This is the same pattern as + `alknet-docker`'s `tty` feature (alknet ADR-061) and is opt-in. +- ADR-001's scope is amended (from "the TTY wire format" to "the TTY + wire format for direct connections"). This is a scope clarification, + not a format change — the 5-byte format itself is unchanged. + +## Door type + +**One-way (scope amendment) + two-way (feature gate).** ADR-001's scope +amendment (direct-only) is one-way — once the channels path exists, +re-merging the formats would require unifying 5-byte and 9-byte chunk +handling, which is a rewrite. The `channels` feature gate is two-way — +it can be removed if channels integration is no longer needed. + +**Reversed by ADR-008 (2026-07-18):** the two-mode design is reversed — +TTY always uses its 5-byte format, carried transparently in the +channels payload. The one-way door is re-cast (the channels crate is +not yet implemented, so this is the right time). See ADR-008 for the +amended door-type discussion. + +## References + +- **[ADR-008](008-channels-pure-channel-multiplexing.md)**: channels + pure channel multiplexing (reverses this ADR — TTY always uses its + 5-byte format; the channels layer carries it transparently; the + two-mode design is preserved but differs only in `BiStream` source, + not in parsing) +- [ADR-001](001-wire-format-and-two-carriage.md): alktty wire format + (amended — scoped to direct connections by this ADR; **re-amended by + ADR-008 — TTY always uses its 5-byte format, in both direct and + inside-channels modes**) +- [ADR-002](002-ttybackend-trait-and-ttyhandle.md): TtyBackend trait + and TtyHandle (unchanged by this ADR) +- [ADR-004](004-exit-code-on-control-chunk.md): exit-chunk-is-last + (generalized by this ADR + alknet ADR-073) +- [ADR-006](006-negotiation-framing-self-contained.md): alktty does not + depend on alkcall's internal wire types (preserved — the channels + feature is on alknet-channels, not alkcall's internal wire types) +- alknet ADR-071: channels wire format (the 9-byte format the channels + path uses; **amended by alknet ADR-093 / ADR-008 — 8-byte format, no + `stream_type`**) +- alknet ADR-074: ChannelBidiStreamSource / `into_sub_streams` (the + accessor the channels path uses; **amended by alknet ADR-093 / + ADR-008 — `into_sub_streams()` removed**) +- alknet ADR-092: `BiStream` as the handler leaf (the transport-leaf + decision that enables the reversal — `accept_bi` returns `BiStream`) +- alknet ADR-061: DockerTtyBackend in alknet-docker (the feature-gated + dependency pattern this ADR mirrors) +- alknet channels phase-0 findings §DP-3, §OQ-CH-02, §Relationship to + Existing Crates / alktty +- Port origin: alknet ADR-077 at + `/workspace/@alkdev/alknet/docs/architecture/decisions/077-tty-inside-channels.md` \ No newline at end of file diff --git a/docs/architecture/decisions/008-channels-pure-channel-multiplexing.md b/docs/architecture/decisions/008-channels-pure-channel-multiplexing.md new file mode 100644 index 0000000..a90745a --- /dev/null +++ b/docs/architecture/decisions/008-channels-pure-channel-multiplexing.md @@ -0,0 +1,521 @@ +# ADR-008: Channels Pure Channel Multiplexing (8-Byte Header, No `stream_type`) + +## Status + +Accepted (amends alknet ADR-071 — wire format is 8 bytes, not 9, and +the channels layer has no `stream_type` concept; amends alknet ADR-074 +— `into_sub_streams()` removed, `accept_bi` is the only accessor and +yields one `BiStream` per channel; reverses [ADR-007](007-tty-inside-channels.md) +— TTY always uses its 5-byte format, the channels layer carries it +transparently in the payload. Ported from alknet ADR-093 2026-08-17; +cross-references renumbered to alktty's ADR range — ADR-052→001, +ADR-053→002, ADR-054→003, ADR-055→004, ADR-056→005, ADR-057→006, +ADR-077→007, ADR-093→008. The alknet ADRs referenced by alknet number +(052, 070, 071, 072, 073, 074, 075, 076, 078, 079, 080, 081, 092) are +not ported into alktty's ADR range — they are alknet-core / +alknet-channels ADRs. The alknet originals at +`/workspace/@alkdev/alknet/docs/architecture/decisions/` remain +authoritative.) + +## Context + +alknet ADR-071 committed the channels wire format as a 9-byte chunk +header (`[channel_id:u32][stream_type:u8][length:u32]`) — a 4-byte +extension of TTY's 5-byte format, with `stream_type` carried in the +channels header and decomposed into unidirectional halves (0/1/2 = +data write/read/err, 3/4/5 = control write/read/err, `% 3` formula). +alknet ADR-074 added a second accessor (`into_sub_streams()`) +alongside `accept_bi` for handlers that need typed sub-streams (TTY's +stdin/stdout/stderr/ctrl-in/ctrl-out). [ADR-007](007-tty-inside-channels.md) +split TTY's wire format into two modes — direct (5-byte) and +inside-channels (the channels layer de-chunks and the adapter +destructures via `into_sub_streams()`). + +The stream-unification research surfaced that these three decisions +share one root: the channels layer carries a concept (`stream_type`) +it doesn't own. The 9-byte header bakes TTY's sub-stream multiplexing +into the channels wire format. The `into_sub_streams()` accessor exists +because the channels layer reassembles per-`stream_type` and needs to +expose the result. The two-mode TTY design exists because the channels +layer's `stream_type` overlaps with TTY's own `stream_type`. The mod +2/mod 3/mod 4 numbering question (settled as mod 3 in alknet ADR-071 +revised) was a symptom of this overlap — a numbering convention for a +concept the channels layer shouldn't carry. + +### The structural question + +The channels layer has two objectives in tension: + +1. **"Pass a stream to/from any ALPN"** — every channel is a + `BiStream`; any handler gets `accept_bi()` and treats the channel + as a duplex stream. Uniform, transport-agnostic, + recursive-composition-friendly. +2. **"Channels carry N sub-streams"** — a TTY channel carries + stdin/stdout/stderr/control; the handler destructures via + `into_sub_streams()`. Carries what the source produces. + +The tension is real when a sub-stream is *unidirectional* (stderr). You +can't represent stderr as a `BiStream` without wasting the write half; +you can't make it a "third half" (mod 3) without breaking pair +symmetry; you can't make the channel a single `BiStream` without +losing the stdout/stderr distinction. + +alknet ADR-074's two-accessor design resolves this by making the "pass +a stream to/from any ALPN" objective *qualified* — it applies to +single-stream channels (tunnel, SSH, call), not multi-stream channels +(TTY). The mod 2/mod 3/mod 4 numbering was a symptom of that qualified +design. + +### The resolution: channels layer is pure channel multiplexing + +The channels layer's job is "one connection carries N channels, routed +by `channel_id`." It does not know about TTY's sub-streams, SSH's +channel protocol, or how call frames its JSON. Handlers own their +sub-multiplexing on the `BiStream` the channels layer gives them. + +- **Every channel is a `BiStream`.** `accept_bi()` yields one + `BiStream` per channel (per alknet ADR-092, already landed). No + `into_sub_streams()`, no second-class accessor. +- **Handlers sub-multiplex their `BiStream` however they want.** TTY + sub-demuxes `stream_type` from its `BiStream` (its 5-byte format). + Tunnel uses the `BiStream` as raw bytes. Call length-prefixes JSON. + SSH runs its own channel protocol. The channels layer carries the + bytes transparently. +- **The mod 2/mod 3/mod 4 question dissolves at the channels layer.** + The channels layer has no `stream_type` concept — not in its header, + not in its code, not in its mental model. `stream_type` is the inner + layer's framing byte, carried transparently. +- **The control channel is handler-internal.** TTY sub-demuxes control + from its io `BiStream` using its 5-byte format (`STREAM_CTRL_IN = 3`, + `STREAM_CTRL_OUT = 4` — ADR-001 amended by Phase 7). The channels + layer doesn't carry control. The "control isn't actually + bidirectional" flaw is fixed at the TTY layer, not the channels + layer. +- **Recursive composition is literal.** A channel with ALPN + `alk/channels` runs another channels demux on its `BiStream`. The + outer layer strips its 8-byte header; the inner layer parses its + own 8-byte header from the payload. Each level is the same shape — + `BiStream → accept_bi → N BiStreams`. + +### The wire format decision: 8 bytes + +The channels wire format is **8 bytes**: +`[channel_id:u32 BE][length:u32 BE]` followed by an opaque payload. The +channels layer owns `channel_id` and `length`; the payload is the +handler's framing, carried transparently. + +The 9-byte alternative (`[channel_id:u32][stream_type:u8][length:u32]`) +was considered and rejected. The 9-byte format puts `stream_type` in the +channels header, which means the channels layer carries a concept it +doesn't own. For TTY this composes cleanly (the 9-byte header is TTY's +5-byte header with `channel_id` prepended), but for non-TTY handlers +(tunnel, call, SSH) the `stream_type` byte is dead weight — the +channels layer carries a byte it doesn't understand, and the handler +ignores a byte in a header it doesn't control. + +The 8-byte format is uniform across all handlers: the channels layer +carries `channel_id` + `length` + opaque payload. Every handler parses +its own framing from the payload. The cost is that TTY's `wire.rs` is +called from a payload buffer rather than directly from the wire, and +the total header for a TTY chunk is 13 bytes (8 channels + 5 TTY) +instead of 9. The two length fields are close but not identical +(`ch_len = tty_len + 5`); for typical TTY chunks (4 KiB+), the 5-byte +overhead is ~0.1%, and the trade is clean separation of concerns. See +"Consequences" for the full cost/benefit. + +### The add/strip composition + +Each layer has its own add/strip pair. The channels layer: +`add_channel_id(channel_id, payload_bytes) -> chunk` on write (prepends +the 8-byte header); `strip_channel_id(chunk) -> (channel_id, +payload_bytes)` on read (strips the 8-byte header, returns the +payload). The handler layer (e.g. TTY) parses its own framing from the +payload bytes per its existing `wire.rs`. The handler doesn't know or +care that a `channel_id` was stripped before it saw the bytes. + +The composition is uniform — the same shape at every level. This is +SSH's model (layered headers, each layer strips its own at its +boundary), applied to channels. A `alk/channels`-inside-`alk/channels` +recursive composition is the outer layer stripping its 8-byte header, +the inner layer parsing its own 8-byte header from the payload — same +code, same shape, each level. + +### Why this can land now + +Three things changed since alknet ADR-071/074/077 were accepted: + +1. **alknet ADR-092 landed `BiStream` as the handler leaf.** + `accept_bi()` returns a `BiStream` (a concrete `AsyncRead + + AsyncWrite` newtype), not a split `(SendStream, RecvStream)` pair. + The join moves into core's quinn/iroh/stream impls (once per + source, invisible to handlers). This ADR's "every channel is a + `BiStream`" is the channels-layer consequence of alknet ADR-092's + handler-leaf decision — the research-then-sync pattern applied: + alknet ADR-092 settled the transport leaf, this ADR settles the + multiplexing layer above it. +2. **Phase 7 fixed the TTY control channel at the TTY layer.** The + `STREAM_CONTROL = 3` "bidirectional" flaw is fixed by splitting it + into `STREAM_CTRL_IN = 3` / `STREAM_CTRL_OUT = 4` — *inside TTY's + 5-byte format*, not at the channels layer. This removed the + load-bearing reason for the channels layer to carry `stream_type`: + the control bidirectionality fix is a TTY-internal concern, not a + channels-layer concern. ADR-007's two-mode TTY design was motivated + by the channels layer carrying control; with control moved inside + TTY, the motivation dissolves. +3. **No production constraint.** The develop branch is a rewrite of + main (pre-alpha). The channels crate doesn't exist yet (per + alknet ADR-081, it's planned as `alknet-channels-core` + + `alknet-channels-call`). The decision is purely "what's cleanest," + not "what's least disruptive." The 9-byte POC validated the + per-`channel_id`/`stream_type` routing mechanism; the 8-byte spec + update changes the header before implementation begins. + +### What this ADR does NOT decide + +- **The add/strip API shape** (built into read/write vs. a separate + utility): the stream-unification research proposed `add_channel_id` + / `strip_channel_id` as standalone functions. Ideally the header is + built into the read/write path so the utility isn't needed at the + handler boundary — but there may be a generalized reason to expose + it (recursive composition, test helpers, the hub relay's + `channel_id` rewrite). The exact API shape is an implementation + detail for the channels crate. The *contract* — the channels layer + strips its 8-byte header on read and the handler parses its own + framing from the payload — is decided here; the *function surface* + is not. +- **TTY's `wire.rs` adaptation:** TTY's `ChunkReader` currently reads + from an `AsyncRead`. Adapting it to read from a payload buffer + (`&[u8]` or `Cursor`) is a small, well-scoped change (the + framing logic — stream_type constants, length validation, control + message parsing — is unchanged). This is an implementation concern + for the channels + TTY integration, not an architecture decision. + In alktty, `drive_session` already runs against a `BiStream` whose + `AsyncRead` yields the post-channels-strip payload bytes; the same + `wire.rs` code runs in both modes. +- **Full channel-level flow-control windowing (alknet OQ-56):** + unchanged. The bounded-buffer backpressure (alknet ADR-076) is the + v1 mechanism; full windowing is an additive extension that doesn't + change the wire format. + +## Decision + +### 1. The channels wire format is 8 bytes + +``` +[channel_id: u32 BE][length: u32 BE][payload bytes] +``` + +8 bytes of header, followed by `length` bytes of opaque payload. The +channels layer owns `channel_id` and `length`; the payload is the +handler's framing, carried transparently. + +| field | offset | width | meaning | +|-------|--------|-------|---------| +| `channel_id` | 0 | 4 (BE) | The logical channel this chunk belongs to. Channel 0 is pre-negotiated as `alk/call` (alknet ADR-072). Channels 1..N are opened dynamically via `channel/open` (alknet ADR-073). | +| `length` | 4 | 4 (BE) | The payload length in bytes. 0 = EOF sentinel. Max `MAX_CHUNK_LEN` (16 MiB, matching TTY's cap — ADR-001 §5). | + +The `stream_type` byte is **removed** from the channels header. The +channels layer has no `stream_type` concept — not in its header, not in +its code, not in its mental model. What was the channels header's +`stream_type` byte is now the first byte of the payload, owned by the +handler's framing (TTY's 5-byte format, call's length-prefixed JSON, +tunnel's raw bytes, SSH's channel protocol). + +This amends alknet ADR-071: the wire format is 8 bytes, not 9; the +`stream_type` decomposition (mod 3, unidirectional halves, 85 groups) +is removed from the channels layer. The stream_type concept survives in +TTY's 5-byte format (ADR-001, amended by Phase 7), which the channels +layer carries transparently. + +### 2. `into_sub_streams()` is removed; `accept_bi` is the only accessor + +alknet ADR-074's `into_sub_streams()` / `ChannelSubStreams` / +`SubStreamHandle` are removed. The channels layer exposes one accessor: +`accept_bi()`, which yields one `BiStream` per channel (per alknet +ADR-092). Every handler — TTY, tunnel, SSH, call — receives a +`Connection`, calls `accept_bi()` once, gets a `BiStream`, and +sub-multiplexes it however it wants. + +This amends alknet ADR-074: the two-accessor design (`accept_bi` for +generic handlers, `into_sub_streams` for typed handlers) collapses to +one accessor. The "typed handler path" (alknet ADR-074's motivating +case for TTY) is replaced by TTY sub-demuxing its `BiStream` via its +own 5-byte format — the same code TTY runs in direct mode. alknet +ADR-074's yield-once `accept_bi` contract is preserved; the +`into_sub_streams()` accessor is the amended part. + +### 3. TTY always uses its 5-byte format; the channels layer carries it transparently + +ADR-007's two-mode TTY design (direct vs inside-channels) is reversed. +TTY's 5-byte format (`[stream_type:u8][length:u32][payload]`, ADR-001) +is TTY's internal format, used in *both* direct mode and +inside-channels mode. The two modes differ only in *where the +`BiStream` comes from* (a top-level `alk/tty` connection vs a +`channel/open` with ALPN `alk/tty`), not in *how TTY parses it*. The +same `wire.rs` code runs in both modes. + +When TTY is inside channels, the channels layer strips its 8-byte +header and hands TTY the payload bytes. TTY parses its 5-byte header +from the payload. The channels layer carries TTY's 5-byte chunks +transparently in its payload — no shared fields, no leaked +abstraction, no double-chunking concern (the 13-byte total header is +8 channels + 5 TTY, not 8 + 9; the channels `length` is always +`tty_len + 5`). + +This reverses ADR-007: the 5-byte format is NOT scoped to direct — +it's TTY's internal format, carried transparently in the channels +payload. The `channels` feature on alktty (if added) becomes "run +TTY's sub-demux on a channels-backed `BiStream`" — the same code as +direct mode, different `BiStream` source. The control channel split +(`STREAM_CTRL_IN` / `STREAM_CTRL_OUT`, Phase 7) is TTY-internal; the +channels layer doesn't know about it. + +### 4. The add/strip composition + +The channels layer's read path strips the 8-byte header and hands the +payload to the handler. The write path prepends the 8-byte header +(`add_channel_id`) onto the handler's output. The handler never sees +the `channel_id`; it sees only its own framing (the payload bytes). + +``` +channels: [channel_id:u32 BE][length:u32 BE][payload] + = 8-byte header + opaque payload + 8 bytes + +TTY inside channels: + [channel_id:u32][ch_len:u32][stream_type:u8][tty_len:u32][payload] + 4 bytes 4 bytes 1 byte 4 bytes N bytes + \_________ __________/ \_________ _____________/ + | | + channels header TTY chunk (5+N bytes) + (8 bytes) carried as channels payload +``` + +The composition is uniform — the same shape at every level. A +`alk/channels`-inside-`alk/channels` recursive composition is the +outer layer stripping its 8-byte header, the inner layer parsing its +own 8-byte header from the payload — same code, same shape, each +level. + +### 5. What does NOT change + +- **alknet ADR-092's `BiStream` leaf** — unchanged. This ADR is the + channels-layer consequence of alknet ADR-092: `accept_bi` yields a + `BiStream`, handlers sub-multiplex it. The two ADRs compose (alknet + ADR-092 settles the transport leaf; this ADR settles the + multiplexing layer above it). +- **`ProtocolHandler` trait shape** (alknet ADR-002) — unchanged. + Handlers receive a `Connection` and call `accept_bi()`. +- **Channel 0 pre-negotiated as `alk/call`** (alknet ADR-072) — + unchanged. Channel 0's chunks have `channel_id = 0` in the 8-byte + header. The call protocol's `EventEnvelope` framing is the payload; + the channels layer carries it transparently. +- **Channel lifecycle operations** (alknet ADR-073) — unchanged. The + four operations (`channel/open`/`close`/`control`/`resources/subscribe`) + and their `direction` semantics are call-protocol operations on + channel 0, not channels-wire-format concerns. +- **`ChannelsAdapter` / `ChannelManager` split** (alknet ADR-075) — + structurally unchanged. The demux loop reads 8-byte headers (not + 9-byte); the `ChannelManager` is ALPN-blind, auth-blind, + transport-blind. The `stream_types` field on `channel/open` and + `ChannelState` is removed (the channels layer doesn't track + per-stream-type reassembly buffers; it tracks one reassembly buffer + per `channel_id`, yielding a `BiStream`). +- **Backpressure, channel limits, ID reuse** (alknet ADR-076) — + unchanged. The bounded-buffer backpressure is per-`channel_id` (was + per-`(channel_id, stream_type)`; now per-`channel_id` since there's + one reassembly buffer per channel). The 256-channel cap, 1 MiB + default, and monotonic-ID-with-wrap strategy are unchanged. +- **Two-pump shutdown-on-completion** (alknet ADR-078) — unchanged. + Tunnel/SSH handlers call `tokio::io::split(bidi)` for their two pump + halves; the shutdown-on-completion contract applies to the + `ReadHalf` / `WriteHalf` unchanged. +- **Hub relay** (alknet ADR-079) — unchanged in contract. The hub + translates `channel/open` on channel 0 and byte-forwards data + channels with `channel_id` rewrite. The relay reads 8-byte headers + (not 9-byte) and rewrites the `channel_id` field (a 4-byte rewrite + within the 8-byte header, not a 9-byte header). The relay does not + parse the payload. +- **`ChannelClient`** (alknet ADR-080) — unchanged in API. + `from_connection` primary, `open_channel` returns a `Channel`. The + `stream_types` field on `open_channel` and `Channel` is removed (the + channels layer doesn't negotiate per-stream-type sets; the handler + owns its sub-stream multiplexing). The + `channel:stream_type_unavailable` error code is removed (the + channels layer can't refuse a `stream_type` it doesn't know about). +- **Sub-crate decomposition** (alknet ADR-081) — unchanged. + `channels-core` (pure multiplexer, depends on alknet-core only) / + `channels-call` (channel 0 pre-negotiation + lifecycle op + registrations, depends on `channels-core` + alknet-call). The 8-byte + wire format, demux/mux, and `ChannelBidiStreamSource` are in + `channels-core`; the call-protocol coupling is in `channels-call`. +- **`BidiStreamSource` trait** (alknet ADR-070) — unchanged in shape. + `ChannelBidiStreamSource` implements it; `accept_bi` yields a + `BiStream` (per alknet ADR-092, already landed). + +## Consequences + +**Positive:** + +- **Clean separation of concerns.** The channels layer has no + `stream_type` concept — not in its header, not in its code, not in + its mental model. The handler owns its framing entirely. This + dissolves the mod 2/mod 3/mod 4 question at the channels layer + (there's nothing to decompose) and fixes the "control isn't actually + bidirectional" TTY flaw at the TTY layer (where it lives, not the + channels layer). +- **Uniform across all handlers.** Tunnel, call, SSH, and TTY all + receive the same shape: a `BiStream`. No handler gets a + `stream_type` byte it doesn't use; no handler needs a second + accessor (`into_sub_streams`) to reach its sub-streams. The channels + layer's API surface is `accept_bi -> BiStream`, period. +- **Recursive composition is literal.** A `alk/channels` channel runs + another channels demux on its `BiStream`. The outer layer strips its + 8-byte header; the inner layer parses its own 8-byte header from + the payload. Same code, same shape, each level. This is a property, + not a feature — the primary use case is one level of multiplexing, + but the add/strip composition makes the recursion cleaner than + alknet ADR-071's group framing did. +- **The `into_sub_streams()` accessor and its consuming handler code + are removed.** This is a net simplification: one accessor, one + handler path, no downcast / extension trait / "two paths" ergonomics + question (which alknet ADR-074 left as an implementation detail). + The handler crate destructures its `BiStream` via its own framing + (TTY's 5-byte format), not via a channels-crate-provided typed + accessor. +- **TTY's `wire.rs` runs unchanged in both modes.** Direct mode and + inside-channels mode use the same code; only the `BiStream` source + differs. ADR-007's `drive_session_direct` / `drive_session_channels` + split collapses to one `drive_session` function. The `channels` + feature on alktty (if added) becomes a thin wrapper that gets the + `BiStream` from a channels-backed `Connection` instead of a + top-level one. (In alktty as built, `drive_session` already takes a + generic `AsyncRead + AsyncWrite` pair, so both the direct + `TtyAdapter::handle` path and the channels `TtyOpenHandler` path + call the same function — see `src/adapter.rs` and + `src/channels.rs`.) +- **The channels layer is WASM-compatible by construction.** The + 8-byte header's core is pure byte manipulation (the sync core + compiles under `wasm32-unknown-unknown`, validated by the POC). The + 8-byte format is simpler than the 9-byte (one fewer field to parse), + strengthening the WASM-clean property. + +**Negative:** + +- **5 extra bytes per TTY chunk.** The total header for a TTY chunk + inside channels is 13 bytes (8 channels + 5 TTY), not 9. The two + length fields are close but not identical (`ch_len = tty_len + 5`). + For typical TTY chunks (4 KiB+), this is ~0.1% overhead. For extreme + multiplexing scenarios, the clean separation is worth the trade-off; + for high-throughput bulk transfer, the escape hatch is + multi-connection (one channels connection per leg), not stripping + the header. This is the documented cost of the clean separation; the + alternative (9-byte header with `stream_type` in the channels layer) + carries a concept the channels layer doesn't own, which is the root + cause this ADR addresses. +- **TTY's `wire.rs` needs a small adaptation.** `ChunkReader` currently + reads from an `AsyncRead` (the transport stream). Inside channels, + it reads from a payload buffer (`&[u8]` or `Cursor`) — the + bytes the channels layer handed it after stripping its 8-byte + header. The framing logic (stream_type constants, length validation, + control message parsing) is unchanged. This is a bounded, + well-scoped implementation change, not an architecture change. The + same adaptation applies to any handler that parses its own framing + from a payload buffer (call's `EventEnvelope` framing already reads + from a buffer; tunnel and SSH don't parse the payload, so no + adaptation). In alktty as built, `drive_session` already reads from + a `BiStream` whose `AsyncRead` yields the post-channels-strip + payload bytes — the same `wire.rs` code runs unchanged in both + modes. +- **`channel/open` loses the `stream_types` field.** alknet ADR-073's + `channel/open` input included `stream_types: [u8]` (the active + sub-stream set) and the response echoed the negotiated set. Under + this ADR, the channels layer doesn't negotiate sub-stream sets — + the handler owns its sub-stream multiplexing. The `stream_types` + field is removed from `channel/open` (and from the + `channel:stream_type_unavailable` error code). The `alpn` and + `params` fields remain; the handler's sub-stream set is implicit in + its ALPN's wire format. This is a small wire-format change to + `channel/open` (one field removed); since the channels crate isn't + implemented yet, there's no migration cost. +- **`ChannelState.streams: HashMap` becomes + `ChannelState.reassembly: ReassemblyBuffer` (one per channel, not + per `(channel_id, stream_type)`).** This is an internal + simplification (fewer reassembly buffers, simpler drain logic) but + is an implementation change, not an architecture one. The + bounded-buffer backpressure (alknet ADR-076) is per-`channel_id` + now, not per-`(channel_id, stream_type)` — the 1 MiB default and + the 256-channel cap are unchanged; the per-channel memory ceiling is + 1 MiB (was up to 5 MiB for a TTY channel with 5 active + stream_types). This is a net improvement (lower memory ceiling per + channel), not a regression. + +## Door type + +**One-way (wire format, accessor removal, two-mode reversal).** The +8-byte chunk header layout (`channel_id:u32 + length:u32`), the +removal of `stream_type` from the channels header, and the removal of +`into_sub_streams()` are wire-format and API commitments. Changing +them after the channels crate is implemented and handlers are written +against them requires a version migration. Since the channels crate +doesn't exist yet, the one-way door is being cast now, before +implementation — the right time to cast a one-way door. + +The reversal of ADR-007 (TTY always uses its 5-byte format) is one-way +in the same sense: once TTY's `wire.rs` runs in both modes (direct and +inside-channels), re-introducing a separate inside-channels mode would +be a rewrite of TTY's session driver. The trade is one unified session +driver now vs. two-mode maintenance forever. + +The add/strip API shape (alknet OQ-68) is a **two-way door** — whether +the header add/strip is built into the read/write path or exposed as a +standalone utility is an implementation detail that can change without +breaking the wire format or the handler contract. + +## References + +- alknet ADR-071: channels wire format (amended — wire format is 8 + bytes, not 9; `stream_type` removed from the channels header; the + stream_type decomposition is removed from the channels layer) +- alknet ADR-074: ChannelBidiStreamSource (amended — + `into_sub_streams()` removed; `accept_bi` is the only accessor, + yields one `BiStream` per channel) +- [ADR-007](007-tty-inside-channels.md): TTY inside channels (reversed + — TTY always uses its 5-byte format; the channels layer carries it + transparently in the payload; the two-mode design is preserved but + differs only in `BiStream` source, not in parsing) +- alknet ADR-092: `BiStream` as the handler leaf (the transport-leaf + layer this ADR builds on — `accept_bi` returns `BiStream`; + `from_bidi` is the only public stream constructor) +- alknet ADR-070: `BidiStreamSource` trait (the extension point + `ChannelBidiStreamSource` implements; `accept_bi` yields `BiStream`) +- alknet ADR-072: channel 0 pre-negotiated `alk/call` (unchanged — + channel 0's chunks have `channel_id = 0` in the 8-byte header; the + call protocol's framing is the payload) +- alknet ADR-073: channel lifecycle operations (amended — + `stream_types` field removed from `channel/open`; + `channel:stream_type_unavailable` error code removed) +- alknet ADR-075: `ChannelsAdapter` and `ChannelManager` (structurally + unchanged — demux reads 8-byte headers; one reassembly buffer per + channel) +- alknet ADR-076: backpressure, channel limits, ID reuse (unchanged — + bounded-buffer is per-`channel_id`; 256-channel cap, 1 MiB default, + monotonic IDs) +- alknet ADR-078: two-pump shutdown-on-completion (unchanged — the + contract applies to `tokio::io::split(bidi)` halves) +- alknet ADR-079: hub relay (unchanged in contract — 8-byte header, + 4-byte `channel_id` rewrite, payload byte-forwarded) +- alknet ADR-080: `ChannelClient` (amended — `stream_types` field + removed from `open_channel` and `Channel`) +- alknet ADR-081: sub-crate decomposition (unchanged — 8-byte wire + format in `channels-core`; call-protocol coupling in + `channels-call`) +- [ADR-001](001-wire-format-and-two-carriage.md): alktty wire format + (the 5-byte format carried transparently in the channels payload; + the control channel split from Phase 7 is TTY-internal) +- `src/channels.rs` — alktty's channels integration (the + `register_openable` helper + `TtyOpenHandler` that runs + `drive_session` on a channels-backed `BiStream`, the same code as + the direct path) +- Port origin: alknet ADR-093 at + `/workspace/@alkdev/alknet/docs/architecture/decisions/093-channels-pure-channel-multiplexing.md` \ No newline at end of file diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md new file mode 100644 index 0000000..f351a4a --- /dev/null +++ b/docs/architecture/overview.md @@ -0,0 +1,312 @@ +--- +status: draft (ported from alknet 2026-08-17; alknet-tty → alktty, + alknet-tty-local → alktty's `local` feature module, alknet/tty → + alk/tty, alknet-core → alkcall::core, alknet-call → alkcall, ADRs + renumbered 052..093 → 001..008) +last_updated: 2026-08-17 +--- + +# alktty — Overview + +The terminal session protocol crate: a `ProtocolHandler` on `alk/tty` +that pumps a bidirectional byte stream (stdin/stdout/stderr) with a JSON +control channel (resize, signal, eof, exit) over a framed bidi stream, +decoupled from the backend that allocates the PTY via a `TtyBackend` +trait. This document covers the crate's purpose, the two-carriage model +in brief, its dependency edges, the ALPN, and the backend location map. +Component details are in the sibling documents. + +## What + +`alktty` is the terminal session protocol crate for the +ALPN-as-service architecture (alknet ADR-001). It registers the `alk/tty` +ALPN on the shared endpoint and implements the `ProtocolHandler` trait +(alknet ADR-002, alknet ADR-007). The `TtyAdapter` receives a +`Connection`, accepts one bidi stream per terminal session, reads a +single JSON negotiation frame, switches to a raw chunk format, and +pumps bytes bidirectionally for the life of the session — +backend-agnostic. + +The guiding insight that shapes the crate: + +> A terminal session is not an SSH concern, or a Docker concern — it is +> a terminal concern. SSH and Docker are just two backends that can +> allocate a PTY. + +The alknet-docker POC proved that the hard part of interactive attach — +bidirectional byte pumping over a framed stream with a 1-byte +stream-type multiplexer — is the same problem regardless of whether the +backend is `bollard::attach_container()` or russh's `pty_request`. The +POC's raw chunk format is the seed of alktty's wire format. alktty +extracts that pattern into its own crate and ALPN; the backends (Docker, +SSH, local process) implement a `TtyBackend` trait; the `alk/tty` +handler is backend-agnostic. This dissolves the PTY hedge in the +alknet-ssh research (DP-5): PTY is not an SSH feature delegated to a +separate crate, it's a tty feature that SSH happens to be able to +provide. + +## Why + +The crate's purpose is to be the terminal session library for downstream +consumers. A hub that runs agent workspaces in containers wires +`DockerTtyBackend` into the `TtyAdapter` and gets interactive terminal +sessions over `alk/tty`. A coordinator that runs `cargo test` remotely +wires `LocalTtyBackend` (pipe mode) and gets the runner pattern (a +process whose stdin/stdout/stderr/exit-code stream over a framed bidi +connection) — the same shape as GitHub/Gitea Actions runners, just over +alk's transport instead of HTTP polling. A browser terminal (xterm.js +over WebTransport, when WebTransport revives) connects to `alk/tty` +directly and gets raw bytes without implementing SSH or the call +protocol. + +The key architectural insight: **the wire format and the backends invert +at the `TtyBackend` trait.** alktty owns the wire format, the negotiation +frame, the chunk codec, the control channel, and the session lifecycle; +the backends own the PTY allocation (docker exec with `tty: true`, russh +`pty_request` + `shell_request`, `portable_pty::openpty`). The adapter +is backend-agnostic and testable with a mock backend (in-memory pipes). +See [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md). + +## The Two-Carriage Model in Brief + +A `alk/tty` bidi stream has two phases (full detail in +[tty-wire.md](tty-wire.md), decided in +[ADR-001](decisions/001-wire-format-and-two-carriage.md)): + +1. **Negotiation (JSON carriage).** The client writes a single + length-prefixed JSON frame carrying the terminal parameters, backend + selector, command, and environment. The framing is a 4-byte + big-endian length prefix + UTF-8 JSON body, self-contained in + alktty (the format coincides with alkcall's framing by convention; + alktty does not depend on alkcall's internal wire types — see + Dependencies below). + +2. **Raw carriage.** After the negotiation frame, the stream switches to + the chunk format (`[stream_type: u8][length: u32 be][payload]`) for + the life of the session. Five stream types: 0=stdin (client→server), + 1=stdout (server→client), 2=stderr (server→client), 3=ctrl-in + (client→server, JSON control messages: resize, signal, eof), + 4=ctrl-out (server→client, JSON control messages: exit). There is no + `call.responded`/`call.completed` — this is not the call protocol; + the raw-carriage byte pump is its own wire format after the single + JSON negotiation frame. + +This is the pattern the docker POC validated and the SSH research +independently arrived at: JSON for the structured request, raw bytes for +the body, which is the part that is actually bytes. The full rationale +(why not JSON for everything; the two-carriage decision; the Phase 7 +control-channel split) is in +[ADR-001](decisions/001-wire-format-and-two-carriage.md) §Context. + +## Dependencies + +``` +alktty (default — wasm-clean) +├── alkcall::core (ProtocolHandler, Connection, AuthContext, Identity, AccessControl, +│ OwnershipProvider — alknet ADR-050 for terminal sessions as resources) +└── (no backend deps — portable_pty, bollard, russh are in the backend crates / `local` feature) + +alktty (local feature) — non-wasm by design +└── adds: portable-pty, tokio-util, tokio/process, tokio/rt-multi-thread +``` + +alktty is dependency-light: alkcall (the handler interface and auth) +only. The negotiation framing is a self-contained ~30-line module in +alktty (4-byte BE length prefix + UTF-8 JSON body on tokio's +`AsyncRead`/`AsyncWrite`). The heavy backend dependencies +(`portable_pty`, `bollard`, `russh`) live in the backend crates, not +here. alktty does **not** depend on alkcall's internal wire types — see +[ADR-006](decisions/006-negotiation-framing-self-contained.md). + +### Why no alkcall-internal-wire-types dependency + +An earlier draft had alktty depending on alkcall for the +`FrameFramedReader`/`FrameFramedWriter` "framing utility." A +pre-implementation check found this was unsound: `FrameFramedReader`'s +`read_frame()` is hardcoded to deserialize `EventEnvelope` — the +length-prefix read and the type-specific deserialize are one entangled +call, not a separable utility. alktty's negotiation payload is a +`NegotiateRequest`, not an `EventEnvelope`, so the claimed reuse did not +exist in a usable form. alktty implements its own framing (the format +coincides with alkcall's by convention; the implementations are +independent). The ~30 lines of length-prefix framing is an idiom, not a +domain abstraction worth a cross-crate dependency. See +[ADR-006](decisions/006-negotiation-framing-self-contained.md) for the +full decision and alknet ADR-003 Amendment 2 for the dependency-edge +clarification. + +alktty stays lean — it has no `portable_pty` (default), no `bollard`, no +`russh`, no alkcall-internal-wire-types. The `TtyBackend` implementations +are opaque `Arc` from the adapter's perspective: +constructed by the assembly layer at startup, stored in the adapter's +backend map, dispatched by the `backend` field of the negotiation frame. + +## ALPN + +| ALPN | Handler | Transport | Browser? | +|------|---------|-----------|----------| +| `alk/tty` | `TtyAdapter` | QUIC bidi stream (direct) or `alk/channels` (multiplexed) | Yes (when WebTransport revives — alknet ADR-040 parked) | + +`alk/tty` is a custom ALPN per the alknet ADR-006 `alk/` convention +(renamed from `alknet/` in alkcall 0.1.1). The `TtyAdapter` +registers for it; the endpoint's `HandlerRegistry` maps `alk/tty` to the +adapter instance. One ALPN per connection (alknet ADR-006); within a +connection, multiple bidi streams carry independent sessions (one +session per stream — see [tty-adapter.md](tty-adapter.md)). + +The browser terminal case: a browser (xterm.js) connects via WebTransport +to `alk/tty` and gets raw bytes. The browser doesn't need to implement +SSH or the call protocol for the terminal use case — only if it wants +SSH-specific features (port forwarding, SFTP). This is a cleaner browser +story than "run a WASM SSH client." WebTransport is deferred per +alknet ADR-044; when it revives, the `alk/tty` ALPN is reachable over +WebTransport's ALPN-stream-proxy (alknet ADR-040, parked). + +### Channels mode (ADR-008) + +The same `alk/tty` protocol also runs inside an `alk/channels` +connection: a channel with ALPN `alk/tty` carries the TTY session, and +the channels layer strips its 8-byte header before handing TTY the +payload. The same `wire.rs` code runs in both modes; only the `BiStream` +source differs. See [ADR-008](decisions/008-channels-pure-channel-multiplexing.md) +and [tty-adapter.md](tty-adapter.md). + +## Backend Location Map + +The decomposition principle: the trait lives where the types live +(alktty); the implementations live where their transport dependencies +live. + +``` +alktty (default — lean, no portable_pty, no bollard, no russh) +├── TtyBackend trait (the contract — ADR-002) +├── TtyHandle, TtyControl (the handle shape backends produce) +├── TtyParams, TerminalParams (the allocation request) +├── TtyAdapter (ProtocolHandler on alk/tty — session lifecycle) +├── wire format (ChunkReader/ChunkWriter, ControlMessage — ADR-001) +└── negotiation framing (self-contained ~30-line module; format coincides + with alkcall's by convention — ADR-006) + +alktty (local feature module — ADR-003; folded in from the old + alknet-tty-local sibling crate) +├── LocalTtyBackend (impl TtyBackend — portable_pty for PTY, std::process for pipe) +├── portable_pty dependency (PTY allocation — the heavy dep, here not in alktty default) +└── libc (signal forwarding — REQ-TTY-02, Unix only) + +alknet-docker (or alktty-docker adapter — future crate, out of scope here) +└── DockerTtyBackend (impl TtyBackend — wraps bollard::attach_container / exec with tty:true) + +alknet-ssh (future crate — out of scope here) +└── SshTtyBackend (impl TtyBackend — wraps russh pty_request + shell_request/exec_request) +``` + +alktty never sees `portable_pty` (default), `bollard`, or `russh`. The +backend implementations are opaque `Arc` from the +adapter's perspective. alktty stays lean; the backend crates own their +transport dependencies. The local backend's module placement (folded +into alktty behind a `local` feature, resolving the alknet cyclic-dep +workaround) is decided in [ADR-003](decisions/003-local-backend-placement.md); +the docker and SSH backends are future crates (out of scope for this +spec set — see [tty-backend.md](tty-backend.md) §"Backend implementations" +for where they live). + +## Feature Gates + +```toml +# alktty Cargo.toml +[features] +default = [] +local = ["dep:portable-pty", "dep:tokio-util", "tokio/process", "tokio/rt-multi-thread"] +``` + +- `default` — the wire format, `TtyAdapter`, and the `TtyBackend` trait. + No backend implementations; the assembly layer registers backends + from their own crates. A docker-only or ssh-only deployment uses the + default features and depends on `alknet-docker` / `alknet-ssh` (or + their own backend crate) directly. **The default crate compiles to + `wasm32-unknown-unknown`** — this is what makes the downstream + TS/Python adapter story work (a wasm-compiled alktty is the protocol + layer for a sandboxed adapter). +- `local` — enables `alktty::local::LocalTtyBackend`, pulling in + `portable_pty` (PTY mode) and `tokio::process` (pipe mode). A consumer + that wants the local backend (terminal or runner) enables this + feature. Inherently non-wasm — `portable-pty` + `tokio::process` need + a real OS. + +The local backend's `portable_pty` dependency is the heavy dep that +motivates the feature gate — a docker-only deployment should not pull in +PTY allocation code. See [ADR-003](decisions/003-local-backend-placement.md). + +## Architecture (component pointers) + +- **[tty-wire.md](tty-wire.md)** — the wire format: the negotiation + frame (JSON carriage, self-contained length-prefixed framing), the + raw chunk codec (`[stream_type: u8][length: u32 be][payload]`), the + five stream types, the control channel (split into `STREAM_CTRL_IN` + / `STREAM_CTRL_OUT` halves, JSON control messages), sentinels, and + the fixed-channel-set rationale. +- **[tty-bast.md](tty-bast.md)** — the BAST (Binary Abstract Syntax + Tree) document for the `alk/tty` wire format; a normative JSON spec + conforming to the BAST meta-schema at + `https://alk.dev/bast/v1/schema`, validatable by any JSON Schema + Draft 2020-12 validator. +- **[tty-backend.md](tty-backend.md)** — the `TtyBackend` trait, + `TtyParams`, `TtyHandle`, `TtyControl`. The inversion point between + the wire-format adapter and the backends. Carries REQ-TTY-01 + (backends need not be natively async; the bridging pattern is a + documented strategy). Notes where the docker/SSH backend crates live + (future, out of scope here). +- **[tty-adapter.md](tty-adapter.md)** — the `TtyAdapter` + (`ProtocolHandler` on `alk/tty`): the session lifecycle, the + three-pump bidirectional driver (stdout→client, client→backend, + exit→exit-chunk), negotiation errors, the exit-chunk ordering + (ADR-004), access control (terminal sessions as runtime-spawned + resources per alknet ADR-050), session-cancel cleanup (ADR-005). +- **[tty-local.md](tty-local.md)** — the `local` feature module: + `LocalTtyBackend` via `portable_pty` (PTY mode) and + `tokio::process::Command` (pipe/runner mode). Carries REQ-TTY-02 + (signal forwarding to the foreground process group). The + blocking→async bridge pattern (the three std threads feeding tokio + mpsc/oneshot) is the reference for any future blocking-API backend. + +## Design Decisions + +| Decision | ADR | Summary | +|----------|-----|---------| +| Wire format and two-carriage model | [ADR-001](decisions/001-wire-format-and-two-carriage.md) | `alk/tty` ALPN; JSON negotiation frame then raw chunks; fixed channel set 0-4; control as JSON; Phase 7 control-channel split | +| `TtyBackend` trait and `TtyHandle` | [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md) | The backend inversion point; `exit_code` as `Future`; backends need not be natively async (REQ-TTY-01) | +| Local backend placement | [ADR-003](decisions/003-local-backend-placement.md) | alktty folds the local backend in behind a `local` feature (resolves the alknet cyclic-dep workaround); PTY vs pipe per-session | +| Exit code on a control chunk | [ADR-004](decisions/004-exit-code-on-control-chunk.md) | `{"type":"exit","code":N}` on `STREAM_CTRL_OUT`; "exit chunk is last" invariant; adapter owns the ordering | +| Backend cleanup on session cancel | [ADR-005](decisions/005-backend-cleanup-on-session-cancel.md) | Dropping `exit_code` future (cancel) MUST kill the session target; the adapter triggers it by dropping the `TtyHandle` | +| Self-contained negotiation framing | [ADR-006](decisions/006-negotiation-framing-self-contained.md) | alktty implements its own length-prefixed framing; format coincides with alkcall's by convention, not by code reuse | +| TTY inside channels (reversed) | [ADR-007](decisions/007-tty-inside-channels.md) | Historical: the two-mode TTY design (direct vs inside-channels); reversed by ADR-008/093 | +| Channels pure channel multiplexing | [ADR-008](decisions/008-channels-pure-channel-multiplexing.md) | TTY always uses its 5-byte format; the channels layer carries it transparently in the payload (reverses ADR-007) | + +## Open Questions + +- **OQ-43** (resolved): `TtyControl` as a `Clone` trait object. +- **OQ-44** (deferred(scope)): Terminal modes (TTY modes). +- **OQ-45** (resolved): Flow control for high-throughput stdout — no + application-level windowing; QUIC per-stream flow control is the + backpressure mechanism. +- **OQ-46** (deferred(scope)): Runner API surface. +- **OQ-47** (resolved): Stdin closure canonical signal. + +## References + +- alknet ADR-001 — ALPN-based dispatch +- alknet ADR-002 — ProtocolHandler trait +- alknet ADR-003 + Amendments 1 & 2 — crate decomposition + (no-handler-depends-on-another-handler; alktty depends on alkcall + only; backends depend on alktty for the trait) +- alknet ADR-006 — `alk/` ALPN convention; one ALPN per + connection; new ALPN for incompatible versions +- alknet ADR-007 — `Connection`, `accept_bi`, the handler-receives- + Connection pattern +- alknet ADR-050 — dynamic resource ownership (terminal sessions as + runtime-spawned resources; the adapter's access-control shape + declares against this model) +- `/workspace/@alkdev/alknet/docs/architecture/decisions/` — the alknet + originals of the ADRs ported here as 001..008, plus the alknet ADRs + referenced by alknet number above (which are not ported into alktty's + ADR range because they are not tty-specific) \ No newline at end of file diff --git a/docs/architecture/tty-adapter.md b/docs/architecture/tty-adapter.md new file mode 100644 index 0000000..a230ac4 --- /dev/null +++ b/docs/architecture/tty-adapter.md @@ -0,0 +1,380 @@ +--- +status: draft (ported from alknet 2026-08-17; alknet-tty → alktty, + alknet/tty → alk/tty, alknet-core → alkcall::core, alknet-call → + alkcall, ADRs renumbered 052..093 → 001..008) +last_updated: 2026-08-17 +--- + +# alktty — TtyAdapter and Session Lifecycle + +The `TtyAdapter` is the `ProtocolHandler` for `alk/tty`: it receives a +`Connection`, accepts bidi streams, reads the negotiation frame, selects +a `TtyBackend` (ADR-002), and pumps bytes bidirectionally for the life of +the session using the wire format (ADR-001). This document specifies the +session lifecycle, the three-pump driver, negotiation errors, the +exit-chunk ordering (ADR-004), session-cancel cleanup (ADR-005), and +access control. + +## What + +`TtyAdapter` implements `ProtocolHandler` (alknet ADR-002, revised by +alknet ADR-007 to receive a `Connection`) on ALPN `alk/tty` (alknet +ADR-006). It holds a `HashMap>` populated at +construction (ADR-002 §5). Its `handle()` method accepts the connection +and loops `connection.accept_bi()`, dispatching each bidi stream to a +session. One `alk/tty` connection hosts multiple terminal sessions — one +session per bidi stream (DP-6, decided in the alknet research; matches +the call protocol's one-operation-per-stream model). + +```rust +pub struct TtyAdapter { + /// Backends keyed by the negotiation frame's `backend` string + /// ("local", "docker", "ssh"). Populated at construction. + backends: Arc>>, + /// Optional ownership provider (alknet ADR-050) for terminal sessions as + /// runtime-spawned resources. None = no resource-level ACL (scope- + /// gate only). Wired by the assembly layer. + ownership: Option>, +} + +#[async_trait] +impl ProtocolHandler for TtyAdapter { + fn alpn(&self) -> &'static [u8] { b"alk/tty" } + + async fn handle(&self, connection: Connection, auth: &AuthContext) + -> Result<(), HandlerError> + { + // One connection → many sessions (one bidi stream each). + while let Ok((send, recv)) = connection.accept_bi().await { + let backends = self.backends.clone(); + let ownership = self.ownership.clone(); + let identity = auth.identity.clone(); + tokio::spawn(async move { + let _ = drive_session(send, recv, backends, ownership, identity).await; + }); + } + Ok(()) + } +} +``` + +The `drive_session` function is the per-stream session driver — the +counterpart to the POC's `session::drive_session`, generalized from the +local PTY backend to the `TtyBackend` trait. It is also the function the +channels path reuses — per [ADR-008](decisions/008-channels-pure-channel-multiplexing.md), +TTY always uses its 5-byte format, so the same `drive_session` runs in +both direct `alk/tty` and channels `alk/channels` modes; only the +`BiStream` source differs. + +## Why + +The adapter is the place where the wire format (ADR-001), the backend +trait (ADR-002), and the exit-chunk ordering (ADR-004) come together. +Keeping these in one place — the adapter — is what makes the invariants +enforceable: the wire format's "exit chunk is last" invariant is enforced +here, not in the backends (which produce handles, not wire bytes); the +backend's `exit_code` future is awaited here, not in the backend; the +negotiation frame is parsed here, not in the backend. The adapter is +backend-agnostic; the backends are wire-format-agnostic. The inversion is +the `TtyBackend` trait. + +## Architecture + +### Session Lifecycle + +A `alk/tty` session on one bidi stream proceeds in three phases: + +1. **Negotiation.** The adapter reads the single length-prefixed JSON + negotiation frame from the client (ADR-001 §"Negotiation Frame"), + parses it into `NegotiateRequest`, extracts the `backend` string, + looks up the `TtyBackend`, and constructs `TtyParams`. If the backend + is not registered, or the negotiation frame is malformed, the adapter + sends a JSON error response and closes the stream (see §"Negotiation + errors" below). + +2. **Allocation.** The adapter calls `backend.allocate(¶ms)`, which + returns a `TtyHandle` (ADR-002). If allocation fails (PTY couldn't be + allocated, docker exec failed, SSH channel request rejected), the + adapter sends a JSON error response and closes the stream. + +3. **Raw carriage — the bidirectional pump.** The adapter switches to + the raw chunk format and pumps three concurrent tasks: + + - **A. stdout → client**: backend stdout (`TtyHandle.stdout`) → stdout + chunks (stream_type 1) to the client. If `TtyHandle.stderr` is + `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`). + 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 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), + the adapter closes the write half — the session ends. + +This is the POC's `session::drive_session` pattern, generalized: the POC +hardcoded the local PTY backend; the adapter dispatches to any +`TtyBackend`. + +### 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 ADR-001 §"Control channel split" and +`tty-wire.md` §"Control Channel". + +### Negotiation Errors + +If the server cannot allocate the session, it sends a JSON error response +in the same length-prefixed framing as the negotiation frame (the JSON +carriage, not the raw chunk format) and closes the stream without +entering raw mode. The error response shape: + +```json +{ "error": "unknown_backend", "backend": "kubernetes" } +``` + +| Error | When | Shape | +|-------|------|------| +| `unknown_backend` | the `backend` string is not in the adapter's backend map | `{"error":"unknown_backend","backend":"..."}` | +| `malformed_negotiation` | the negotiation frame failed to parse as JSON or failed `NegotiateRequest` validation | `{"error":"malformed_negotiation","message":"..."}` | +| `allocate_failed` | `backend.allocate()` returned a `TtyError` | `{"error":"allocate_failed","message":"..."}` | + +After sending the error response, the adapter closes the write half of +the bidi stream. The client reads the error frame and treats stream close +as the failure signal. There is no `call.error` — this is not the call +protocol; the error is a JSON response in the negotiation framing. +**Framing disambiguation (success vs error).** Both a successful +allocation (raw chunks) and a failed allocation (JSON error frame) begin +with bytes the client must read before knowing which framing applies. +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`. 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-001): 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-004) + +The "exit chunk is last" invariant (ADR-004) is enforced here, in the +adapter's session driver, not in the backend. The ordering: + +1. The stdout pump (task A) drains the backend's stdout to EOF. The + backend's stdout ends when the process exits and the PTY/pipe buffer + drains (Unix `Child::wait()` blocks until the child is reaped, which + happens after the child exits and its stdout drains — ADR-004 + assumption 1). +2. The exit task (task C) awaits `TtyHandle.exit_code`. The exit resolves + after the child is reaped (the local backend's waiter thread calls + `Child::wait()`; docker's `inspect_exec` after the output stream ends; + SSH's channel close after the process exits). +3. **The adapter waits for *both* the stdout pump to complete (EOF) + *and* `exit_code` to resolve** before enqueueing the exit chunk. If a + backend's stdout outlives the exit resolve (a hypothetical backend + where the process exits but a buffer flush is still in flight), the + adapter waits for the stdout pump; the `TtyHandle.stderr` (if `Some`) + is pumped concurrently and also drains before the exit chunk. (ADR-004 + assumption 2.) +4. After both resolve, the exit chunk (`{"type":"exit","code":N}`) is + enqueued on the writer channel. +5. The drainer writes the exit chunk to the client. +6. The adapter closes the write half — the session ends. + +A client reads stdout/stderr/control chunks until it sees the exit +chunk, records the exit code, and treats subsequent stream close as the +session end. The exit chunk is the deterministic completion signal — +the same stopgap property the docker POC validated for logs subscriptions, +now for any backend. + +If `exit_code` resolves with a `TtyError` (the backend couldn't determine +the exit code), the adapter sends `{"type":"exit","code":-1}` (ADR-004 +§4). The client treats `-1` as "the backend reported an exit error, not +a real exit code." + +### Access Control + +Terminal sessions are runtime-spawned resources per alknet ADR-050. A +`alk/tty` session is a resource the caller owns: the caller that +opened the session owns it; proxy to share; teardown (stream close) +revokes. The adapter's access control declares against the ADR-050 model: + +- **Scope-gate at negotiation.** The adapter checks the caller's + `identity.scopes` for the `tty:open` scope (or a deployment-configured + scope) before allocating the session. A caller without the scope gets + a negotiation error (`{"error":"forbidden"}`) and the stream closes. +- **Resource ownership for backend-specific resources.** Some + backends target a pre-existing resource (a docker backend targets a + specific container); others create their own (a local backend's + process, an SSH backend's channel). The adapter delegates the + resource-id extraction to the backend via + `TtyBackend::resource_id(¶ms)` (ADR-002), which returns + `None` (no pre-existing resource — the session creates its own) or + `Some((kind, id))` (the caller must own this resource). The adapter + checks `OwnershipProvider::owns(identity, kind, id, "tty")` if an + ownership provider is wired and the backend returns `Some`. The + adapter does not parse backend-specific JSON itself — the extraction + is backend-driven, so adding a backend with a new resource shape (e.g., + a Kubernetes backend targeting a pod) requires no adapter change. +- **`forwarded_for` for proxied sessions.** A hub that proxies a + terminal session to a worker carries the end user's identity as + `forwarded_for` (alknet ADR-032); the worker authorizes the hub (its + direct caller), not the end user. The hub's end-user ACL is its own + layer. + +The tty adapter is a `ProtocolHandler`, not an `OperationSpec`-registered +operation — it doesn't go through the call protocol's +`OperationRegistry::invoke()`. The access-control shape is the adapter's +own (scope-gate + backend-driven ownership check at negotiation), +declaring against the ADR-050 model but not consuming +`OperationSpec.resource_id_path` (that field is for call-protocol +operations; the tty adapter is its own ALPN, and the resource-id +extraction is delegated to the backend via `resource_id()` rather than +a path expression). See alknet ADR-050 §"Specifics" for the model this +declares against. + +The concrete choice — the scope name (`tty:open`) and the +check-at-negotiation timing — is a **two-way-door** choice within the +one-way `TtyAdapter` shape. The scope name can be renamed (a +deployment-configured scope, not a wire-format constant). The +resource-id extraction is backend-driven via `resource_id()`, so new +backends with new resource shapes require no adapter change — the +generalization is already in place (ADR-002). No ADR is warranted for +the scope name; it is a reversible implementation choice, not an +architectural commitment. + +### Connection and Stream Lifecycle + +- **Connection drop**: when the QUIC connection closes, all in-flight + sessions on that connection are cancelled. Each session's pump tasks + are dropped (Rust `Drop`); the `TtyHandle` is dropped; the + `exit_code` future is dropped without being driven to completion, + which triggers the backend's cancel-cleanup — the session target is + killed (ADR-005). For the local backend, the `exit_code` future's + `Drop` calls `ChildKiller::kill(SIGHUP)`, the child exits, the + waiter thread's `wait()` reaps it and exits, and the reader/writer + threads exit on channel close. For docker/SSH backends (future), the + `Drop` issues the backend's kill (container kill / channel close). + See ADR-005 for the contract and the mechanism. +- **Stream reset**: when a bidi stream is reset mid-session, the + `ChunkReader` returns a `RawError` (ConnectionClosed or Io). The pump + tasks exit; the `TtyHandle` is dropped; the cancel-cleanup runs + (ADR-005). No exit chunk is sent — the stream is gone, the client + that reset it already knows. +- **Client cancel**: when the client closes the write half (or sends a + zero-length stdin chunk / `eof` control chunk), the adapter signals + EOF to the backend's stdin and keeps pumping stdout until the backend's + stdout ends and the exit resolves. The session completes normally — + the exit chunk is sent — the client just stopped sending input. This + is NOT a cancel from the adapter's perspective (the session runs to + completion); the cancel-cleanup (ADR-005) is not triggered. The + cancel-cleanup is triggered only when the *adapter* drops the handle + (connection drop, stream reset, panic), not when the client closes + the write half. + +## Constraints + +- **The adapter, not the backend, owns the wire format.** Backends + produce handles; the adapter pumps. A backend that wrote to the wire + directly would break the "exit chunk is last" invariant (ADR-004) and + the negotiation-error framing. +- **One session per bidi stream, multiple streams per connection.** A + connection hosts multiple sessions (one stream each); the adapter + spawns a `drive_session` task per accepted stream. Sessions are + independent — one session's exit doesn't affect another. +- **Negotiation errors are JSON, not raw chunks.** The error response + uses the negotiation framing (length-prefixed JSON), not the raw chunk + format. The stream enters raw mode only after a successful allocation. +- **The exit chunk is the deterministic completion signal.** A client + reading to completion sees the exit chunk and knows the process exited + with code N. A client that cancels mid-stream (closes the write half) + won't see the exit chunk — that's correct; a cancelled stream doesn't + have a deterministic exit. +- **The adapter triggers backend cleanup by dropping the `TtyHandle` + (ADR-005).** On connection drop, stream reset, or pump-task panic, the + adapter's pump tasks are dropped, which drops the `TtyHandle`, which + drops the `exit_code` future without driving it to completion. The + `exit_code` future's `Drop` is the backend's cancel-cleanup path + (kill the session target). The adapter has no separate kill method to + call; the cleanup is wired into the `exit_code` future's `Drop` by the + backend. A backend that returns an `exit_code` future without a + kill-on-`Drop` guard violates the contract and will orphan processes + on cancel. See ADR-005. + +## Design Decisions + +| Decision | ADR | Summary | +|----------|-----|---------| +| Wire format and two-carriage model | [ADR-001](decisions/001-wire-format-and-two-carriage.md) | The chunk codec + control channel the adapter pumps | +| `TtyBackend` trait and `TtyHandle` | [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md) | The backend the adapter dispatches to; the handles the adapter pumps | +| Exit code on a control chunk | [ADR-004](decisions/004-exit-code-on-control-chunk.md) | The "exit chunk is last" invariant the adapter enforces | +| Backend cleanup on session cancel | [ADR-005](decisions/005-backend-cleanup-on-session-cancel.md) | Dropping `exit_code` future kills the session target; the adapter triggers it by dropping the `TtyHandle` on cancel | +| Channels pure channel multiplexing | [ADR-008](decisions/008-channels-pure-channel-multiplexing.md) | `drive_session` runs unchanged in both direct and channels modes; only the `BiStream` source differs | +| Dynamic resource ownership | alknet ADR-050 | Terminal sessions as runtime-spawned resources; the adapter's access-control shape | + +## Open Questions + +- **OQ-45** (resolved): Flow control for high-throughput stdout — no application-level windowing; QUIC per-stream flow control is the backpressure mechanism. + +## References + +- [ADR-001](decisions/001-wire-format-and-two-carriage.md) — the wire + format the adapter pumps +- [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md) — the + backend trait the adapter dispatches to +- [ADR-004](decisions/004-exit-code-on-control-chunk.md) — the + exit-chunk ordering the adapter enforces +- [ADR-005](decisions/005-backend-cleanup-on-session-cancel.md) — the + cancel-cleanup contract the adapter triggers by dropping the + `TtyHandle` on session cancel +- [ADR-008](decisions/008-channels-pure-channel-multiplexing.md) — why + `drive_session` runs unchanged in channels mode +- alknet ADR-050 — the ownership model the adapter's access control + declares against +- alknet ADR-007 — `Connection`, `accept_bi`, the handler-receives- + Connection pattern +- `src/adapter.rs` — the Rust source this spec documents +- [tty-wire.md](tty-wire.md) — the wire format details +- [tty-backend.md](tty-backend.md) — the backend trait details \ No newline at end of file diff --git a/docs/architecture/tty-backend.md b/docs/architecture/tty-backend.md new file mode 100644 index 0000000..8b256ae --- /dev/null +++ b/docs/architecture/tty-backend.md @@ -0,0 +1,426 @@ +--- +status: draft (ported from alknet 2026-08-17; alknet-tty → alktty, + alknet/tty → alk/tty, alknet-core → alkcall::core, alknet-call → + alkcall, ADRs renumbered 052..093 → 001..008) +last_updated: 2026-08-17 +--- + +# alktty — TtyBackend Trait and TtyHandle + +The `TtyBackend` trait is the inversion point that keeps alktty +decoupled from its backends. alktty defines the trait, the `TtyParams` +allocation request, the `TtyHandle` a backend produces, and the +`TtyControl` trait; the backend crates (alktty's own `local` feature +module, future `alknet-docker`, `alknet-ssh`) implement `TtyBackend`. +This document specifies what an implementer builds against. The trait +shape is decided in [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md). + +## What + +The `TtyBackend` trait is what the `TtyAdapter` calls to allocate a +terminal/process session. The adapter holds a +`HashMap>` keyed by the negotiation frame's +`backend` string (`"local"`, `"docker"`, `"ssh"`). On a new session, the +adapter reads the negotiation frame, selects the backend by the +`backend` field, calls `allocate()`, and pumps the resulting +`TtyHandle`'s fields bidirectionally using the chunk format +(ADR-001). The backend does not write to the wire — it produces handles; +the adapter pumps. + +Three implementations are contemplated, each in its own crate (the +no-handler-depends-on-another-handler rule from alknet ADR-003 is +preserved — backends depend on alktty for the trait, alktty doesn't +depend on them): + +- **`LocalTtyBackend`** (in alktty's `local` feature module, + [ADR-003](decisions/003-local-backend-placement.md)) — wraps + `portable_pty` for the PTY case and `std::process::Command` with + `Stdio::piped()` for the pipe/runner case. See + [tty-local.md](tty-local.md). +- **`DockerTtyBackend`** (in `alknet-docker` or a sibling adapter crate + — future, out of scope here) — wraps `bollard::attach_container()` for + interactive attach or `bollard::exec::start_exec` with `tty: true` for + exec-with-PTY. `control.resize()` calls `bollard::exec::resize_exec` + or `bollard::container::resize_container`. stdout/stderr are merged + when `tty: true` (bollard's `LogOutput` on a TTY exec returns + `StdOut` only), so `TtyHandle.stderr` is `None` for the PTY case. +- **`SshTtyBackend`** (in `alknet-ssh` — future, out of scope here) — + wraps russh's `pty_request` + `shell_request` (or `exec_request` with + a PTY) on a session channel. `channel.into_stream()` gives + `(AsyncRead, AsyncWrite)` — the stream *is* the PTY; russh handles + kernel PTY allocation on the server side. `control.resize()` sends a + `window_change` channel request; `control.signal()` sends a `signal` + channel request. stdout and stderr are merged (PTY property), so + `TtyHandle.stderr` is `None`. + +The docker and SSH backend crates are future work; this spec set commits +the trait shape they will implement, so they can be built against it +without re-spec'ing the seam. + +## Why + +The guiding insight: **a terminal session is not an SSH concern, or a +Docker concern — it is a terminal concern. SSH and Docker are just two +backends that can allocate a PTY.** The `TtyBackend` trait is what makes +that insight load-bearing — alktty owns the wire format and session +lifecycle; the backends own PTY allocation. The full rationale (the +inversion point, why the trait is the seam) is in +[ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md) §Context. + +The Phase 0 local-PTY POC was built *before* this spec specifically to +discover constraints the trait sketch would have missed by reading docs +alone. Two requirements fell out, recorded as REQ-TTY-01 and REQ-TTY-02 +in the alknet research findings; this spec carries REQ-TTY-01 here +(backends need not be natively async) and [tty-local.md](tty-local.md) +carries REQ-TTY-02 (signal forwarding to the process group). + +## Architecture + +### `TtyBackend` trait + +```rust +#[async_trait] +pub trait TtyBackend: Send + Sync { + /// Allocate a terminal/process session and return the handles the + /// adapter pumps. The `backend` field of the negotiation frame + /// (ADR-001) selects which registered backend's `allocate` is called. + async fn allocate(&self, params: &TtyParams) -> Result; + + /// The pre-existing resource this session targets, for ownership + /// checks (alknet ADR-050). `None` = no pre-existing resource (the session + /// creates its own — local process, SSH channel). `Some((kind, id))` + /// = the session targets an existing resource the caller must own + /// (e.g., DockerTtyBackend returns `Some(("container", id))`). The + /// adapter calls this at negotiation to gate access; the backend + /// extracts the id from its own `backend_params`. Default `None`. + fn resource_id(&self, _params: &TtyParams) -> Option<(&'static str, String)> { None } +} +``` + +The adapter holds `HashMap>` populated at +construction. The assembly layer (the CLI binary) constructs backends +with their dependencies and registers them. A backend is the *thing +that allocates a session*; the wire-format pump is backend-agnostic. + +### `TtyError` + +The error type for `allocate()` and `exit_code`. `#[non_exhaustive]` so +new variants are additive (two-way-door extension within the one-way +trait shape — ADR-002). + +```rust +#[non_exhaustive] +#[derive(Debug, thiserror::Error)] +pub enum TtyError { + #[error("allocate failed: {message}")] + AllocFailed { message: String }, + #[error("wait failed: {message}")] + WaitFailed { message: String }, + #[error("io: {0}")] + Io(#[from] std::io::Error), + #[error("backend-specific: {message}")] + Backend { message: String }, +} +``` + +- `AllocFailed` — the PTY couldn't be allocated, the docker exec failed + to start, the SSH channel request was rejected. Returned by + `allocate()`; the adapter sends `{"error":"allocate_failed",...}` and + closes (tty-adapter.md §"Negotiation errors"). +- `WaitFailed` — the backend couldn't reap the child / determine the + exit code. Returned by the `exit_code` future; the adapter sends + `{"type":"exit","code":-1}` (ADR-004 §4). +- `Io` — an I/O error from a backend's stream/handle. +- `Backend` — backend-specific error not covered by the above (e.g., a + bollard API error, a russh protocol error). + +### `TtyParams` — the allocation request + +```rust +pub struct TtyParams { + /// Terminal parameters. `None` = pipe mode (no PTY — the runner case, + /// ADR-003). `Some` = allocate a PTY with these dimensions. + pub terminal: Option, + /// Command vector (argv[0] + args). Non-empty. + pub cmd: Vec, + /// Working directory (None = inherit/default). + pub cwd: Option, + /// Environment variables (empty = inherit). + pub env: HashMap, + /// Backend-specific selector fields from the negotiation frame, + /// unparsed. The adapter passes the JSON object through verbatim; the + /// backend deserializes its own strongly-typed params struct from it. + /// alktty has zero knowledge of any backend's params shape. + /// See "Backend params are opaque" below. + pub backend_params: serde_json::Map, +} + +pub struct TerminalParams { + pub term: Option, // e.g., "xterm-256color"; None = backend default + pub cols: u16, + pub rows: u16, + pub pixel_width: u16, + pub pixel_height: u16, + pub modes: serde_json::Value, // reserved — OQ-44; backends MUST ignore content in v1 +} +``` + +`terminal: None` is the pipe/runner case — no PTY, separate +stdout/stderr. `terminal: Some` is the PTY case — stdout/stderr merged +into the single stdout stream (`TtyHandle.stderr` is `None`), real +terminal semantics (resize, signal delivery to process group). The +per-session choice is the backend's branch in `allocate()`, not a +per-deployment choice — see ADR-003. + +### Backend params are opaque + +`backend_params` is a `serde_json::Map`, not +a typed enum. The adapter passes the negotiation frame's +backend-specific fields through verbatim; the backend deserializes its +own strongly-typed params struct. alktty has zero knowledge of any +backend's params shape — not docker's `container`, not an SSH host +selector, not anything. Each backend defines its own params struct and +deserializes from `params.backend_params` inside `allocate()`: + +```rust +// in alknet-docker +#[derive(Deserialize)] +struct DockerBackendParams { container: String } + +impl TtyBackend for DockerTtyBackend { + async fn allocate(&self, params: &TtyParams) -> Result { + let p: DockerBackendParams = serde_json::from_value( + serde_json::Value::Object(params.backend_params.clone()) + ).map_err(|e| TtyError::Backend { message: e.to_string() })?; + // use p.container ... + } + fn resource_id(&self, params: &TtyParams) -> Option<(&'static str, String)> { + // extract for the ownership check — backend-driven, not adapter-hardcoded + let p: DockerBackendParams = serde_json::from_value( + serde_json::Value::Object(params.backend_params.clone()) + ).ok()?; + Some(("container", p.container)) + } +} +``` + +This is a complete inversion: the *trait* is inverted (backends +implement, alktty doesn't depend on them) and the *params* are +inverted (backends define their own typed shape, alktty doesn't carry +it). A new backend crate requires zero changes to alktty — no +enum variant to add, no forward-reference type to place, no dependency +edge. See ADR-002 §"Backend params are opaque" for the full rationale +and why the typed-enum alternative (with `SshChannelRef`) was rejected. + +### `TtyHandle` — what a backend produces + +```rust +pub struct TtyHandle { + /// Stdin writer — bytes the adapter pumps from client stdin chunks. + /// `tokio::io::AsyncWrite` (the tokio flavor, not the `futures::io` + /// one — they are incompatible traits; the tokio stack is the + /// adapter's runtime). + pub stdin: Box, + /// Stdout stream — bytes the adapter pumps to client stdout chunks. + /// Ends when the backend's stdout reaches EOF. + /// `futures_core::Stream` (re-exported by + /// `tokio_stream::StreamExt` for extension methods). + pub stdout: Pin + Send>>, + /// Stderr stream — `None` for PTY backends (stdout/stderr merged + /// into `stdout`). `Some` for pipe backends (separate streams). + pub stderr: Option + Send>>>, + /// Exit code — a `Future` the adapter awaits. Resolves when the + /// process/container/SSH exec exits. The adapter sends the result + /// as the `{"type":"exit","code":N}` control chunk (ADR-004) and + /// closes the stream. This is `BoxFuture`, not a method on + /// `TtyHandle`, so the adapter can `select` between exit and + /// stream-close without coupling to the other fields. (REQ-TTY-01.) + pub exit_code: BoxFuture<'static, Result>, + /// Control handle (resize, signal) — `Clone` so the adapter can + /// hand it to the spawned control-chunk dispatcher. `None` only + /// when the backend genuinely has no control path. See OQ-43. + pub control: Option, +} +``` + +### `TtyControl` trait and `TtyControlHandle` + +```rust +pub trait TtyControl: Send + Sync { + /// Resize the terminal. Maps to SSH `window-change`, docker exec + /// resize, or `ioctl(TIOCSWINSZ)` on a local PTY. No-op for pipe + /// backends without a PTY. + fn resize(&self, cols: u16, rows: u16, pixel_width: u16, pixel_height: u16); + + /// Forward a signal by name. Best-effort delivery to the foreground + /// process group (see tty-local.md REQ-TTY-02). Unknown names fall + /// back to the backend's default kill. + fn signal(&self, name: &str); +} + +/// The `Clone`-able handle to a backend's control path. The `TtyControl` +/// trait is NOT `Clone` (`Clone` is not object-safe — `fn clone(&self) -> +/// Self` returns `Self`, which forbids `dyn` dispatch); the `Clone`-ability +/// lives on this concrete newtype, which holds the trait object behind an +/// `Arc`. The adapter clones the `Arc` to hand a handle to the spawned +/// control-chunk dispatcher. See OQ-43. +#[derive(Clone)] +pub struct TtyControlHandle(Arc); + +impl TtyControlHandle { + pub fn new(control: Arc) -> Self { Self(control) } + pub fn resize(&self, c: u16, r: u16, pw: u16, ph: u16) { self.0.resize(c, r, pw, ph) } + pub fn signal(&self, name: &str) { self.0.signal(name) } +} +``` + +The trait is kept object-safe by NOT putting `Clone` on it; the `Clone` +newtype (`TtyControlHandle`) holds the trait object behind an `Arc`. The +POC used a concrete `PtyControl` struct (inherently `Clone` — it held +`Arc>` fields); this newtype generalizes the POC's shape so a +backend produces its own control type via `TtyControlHandle::new(Arc::new(MyControl))` +without the adapter knowing the concrete shape. See OQ-43 for the +confirmation and the rationale for why `Clone` cannot live on the trait +itself. + +### REQ-TTY-01: backends are not required to be natively async + +`portable_pty`'s API is blocking `std::io::{Read, Write}` and a blocking +`Child::wait()` — there is no async variant. The local-PTY POC bridges +this with three dedicated std threads (reader, writer, waiter) feeding +tokio mpsc/oneshot channels; the async-facing `LocalPty` then exposes +`mpsc::Receiver` for stdout, `mpsc::Sender` for stdin, +and `oneshot::Receiver` for exit. This is the same pattern wezterm +(portable_pty's primary consumer) uses. + +The trait's adapter-facing types (`AsyncWrite`, `Stream`, +`BoxFuture`, `TtyControl`) are the **adapter's contract**. A backend may +expose blocking handles internally and bridge them to these async-facing +types. The bridging pattern — blocking `std::io` on dedicated std threads +or `tokio::task::spawn_blocking`, feeding tokio mpsc/oneshot channels — +is a **documented, supported implementation strategy**, not a workaround. + +This resolves the first half of OQ-TTY-01 (the research's open question +on the trait shape): `exit_code` is a `Future` the adapter awaits; a +`oneshot::Receiver` (or any `BoxFuture<'static, i32>`) lets the +adapter `select` between exit and stream-close without coupling to the +handle's other fields. The local backend's waiter thread produces exactly +this shape for free. See [tty-local.md](tty-local.md) for the bridge +details. + +### Backend registration and the assembly layer + +```rust +let mut backends = HashMap::new(); +backends.insert("local".into(), + Arc::new(LocalTtyBackend::new()) as Arc); +backends.insert("docker".into(), + Arc::new(DockerTtyBackend::new(docker_client, "alk".into())) as Arc); +backends.insert("ssh".into(), + Arc::new(SshTtyBackend::new(ssh_session)) as Arc); +let tty_adapter = TtyAdapter::new(Arc::new(backends)); +``` + +A deployment that doesn't want docker registers only `local`. A browser +terminal endpoint that proxies to remote docker/ssh registers `docker` +and/or `ssh` backends. The adapter is backend-agnostic; the assembly +layer chooses what's available. + +### Backend implementations (where they live) + +| Backend | Crate | Status | Notes | +|---------|-------|--------|-------| +| `LocalTtyBackend` | alktty's `local` feature module ([ADR-003](decisions/003-local-backend-placement.md)) | in scope ([tty-local.md](tty-local.md)) | `portable_pty` (PTY) + `std::process` (pipe); the runner pattern | +| `DockerTtyBackend` | `alknet-docker` (behind `tty` feature) | future, out of scope here | wraps `bollard::attach_container` / `exec` with `tty: true`; attach vs exec mode | +| `SshTtyBackend` | `alknet-ssh` | future, out of scope here | wraps russh `pty_request` + `shell_request`/`exec_request`; dissolves alknet-ssh DP-5 PTY hedge | + +The SSH backend crate is future work; this spec commits the +trait shape it implements so it can be built against it without +re-spec'ing the seam. The `DockerTtyBackend` is future work in +`alknet-docker` — the natural extension of the alknet-docker POC's +`drive_attach_raw` — with the trait, it becomes `impl TtyBackend for +DockerTtyBackend`. The `SshTtyBackend` dissolves the alknet-ssh +research's PTY hedge (DP-5): alknet-ssh's session channel still does +`exec` (structured, JSON carriage, exit code on completion) but +*delegates* PTY to alktty via the `SshTtyBackend`. alknet-ssh's +"default-reject" stance stays for the SSH channel policy (it rejects +`pty_request` on its own session channels), but the PTY capability is +provided by a separate crate via a separate ALPN (`alk/tty`), not hedged +inside alknet-ssh. + +## Constraints + +- **The trait shape is one-way (ADR-002).** The `TtyBackend` trait + method `allocate()`, the `TtyHandle` field set, and the `TtyControl` + trait are the API surface every backend crate implements and the + adapter consumes. Changing them after backends exist is a rewrite + across crates. +- **Backend params are opaque (`serde_json::Map`), not a typed enum.** + The carrier type is one-way (part of `TtyParams`), but the *contents* + are backend-defined: each backend deserializes its own + strongly-typed params struct, and a new backend crate requires zero + changes to alktty. See "Backend params are opaque" above and + ADR-002 §"Backend params are opaque." +- **The adapter, not the backend, owns the wire format.** Backends + produce handles; the adapter pumps. A backend that wrote to the wire + directly would break the wire-format invariants (the exit-chunk + ordering, ADR-004). The backend's `exit_code` future resolves and the + adapter sends the exit chunk — the backend does not serialize + `ControlMessage::Exit`. +- **PTY backends merge stdout/stderr.** `TtyHandle.stderr` is `None` for + the PTY case (kernel PTY property — one output stream from the slave). + The adapter pumps only stdout chunks (stream_type 1). Pipe backends + set `stderr: Some` and the adapter pumps both stdout (stream_type 1) + and stderr (stream_type 2) chunks. +- **`TtyControl::signal` is best-effort.** The contract is "best-effort + delivery to the foreground process group," not "the child pid receives + the signal." See [tty-local.md](tty-local.md) REQ-TTY-02 for the + process-group targeting and the fallback to the backend's default kill. +- **Dropping the `exit_code` future MUST kill the session target + ([ADR-005](decisions/005-backend-cleanup-on-session-cancel.md)).** The + `exit_code` field is a `BoxFuture<'static, Result>` + whose `Drop`-on-cancel (i.e., dropped without being driven to + completion) MUST kill the child/container/SSH process. This is a + behavioral contract on the `TtyBackend` trait — the adapter triggers + it by dropping the `TtyHandle` on session cancel (connection drop, + stream reset); the backend wires the kill into the `exit_code` + future's `Drop`. A backend that returns a bare `oneshot::Receiver` + (or any future without a kill-on-`Drop` guard) as `exit_code` + violates the contract and will orphan processes on cancel. See + [ADR-005](decisions/005-backend-cleanup-on-session-cancel.md) + and [tty-local.md](tty-local.md) §"Cancel-Cleanup (ADR-005)" for the + local backend's mechanism. + +## Design Decisions + +| Decision | ADR | Summary | +|----------|-----|---------| +| `TtyBackend` trait and `TtyHandle` | [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md) | The backend inversion point; `exit_code` as `Future`; backends need not be natively async (REQ-TTY-01) | +| Local backend placement | [ADR-003](decisions/003-local-backend-placement.md) | alktty folds the local backend in behind a `local` feature (resolves the alknet cyclic-dep workaround) | +| Wire format | [ADR-001](decisions/001-wire-format-and-two-carriage.md) | The chunk codec + control channel the adapter pumps to/from these handles | +| Exit code on a control chunk | [ADR-004](decisions/004-exit-code-on-control-chunk.md) | The adapter awaits `exit_code`, sends the exit chunk, closes | +| Backend cleanup on session cancel | [ADR-005](decisions/005-backend-cleanup-on-session-cancel.md) | Dropping `exit_code` future (cancel) MUST kill the session target; contract on the `TtyBackend` trait | + +## Open Questions + +- **OQ-43** (resolved): `TtyControl` as a `Clone` trait object. +- **OQ-44** (deferred(scope)): Terminal modes. + +## References + +- [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md) — the + trait shape decision (this spec is its elaboration) +- [ADR-001](decisions/001-wire-format-and-two-carriage.md) — the wire + format the adapter pumps to/from these handles +- [ADR-004](decisions/004-exit-code-on-control-chunk.md) — the + exit-chunk ordering the `exit_code` field feeds into +- [ADR-005](decisions/005-backend-cleanup-on-session-cancel.md) — the + cancel-cleanup contract on this trait (`exit_code` future's + `Drop`-on-cancel kills the session target) +- [ADR-003](decisions/003-local-backend-placement.md) — the local + backend's placement (folded into alktty behind a `local` feature) +- `src/backend.rs` — the Rust source this spec documents +- [tty-local.md](tty-local.md) — the `LocalTtyBackend` spec (carries + REQ-TTY-02: signal forwarding to the process group) +- [tty-adapter.md](tty-adapter.md) — the session driver that consumes + these handles \ No newline at end of file diff --git a/docs/architecture/tty-bast.md b/docs/architecture/tty-bast.md new file mode 100644 index 0000000..c03102c --- /dev/null +++ b/docs/architecture/tty-bast.md @@ -0,0 +1,384 @@ +--- +status: draft +last_updated: 2026-08-17 +--- + +# alktty — BAST Document for the `alk/tty` Wire Format + +This document is the **BAST (Binary Abstract Syntax Tree)** specification +for the `alk/tty` wire format. It is a normative JSON document conforming +to the BAST meta-schema at `https://alk.dev/bast/v1/schema` (a standard +JSON Schema Draft 2020-12 document); any JSON Schema Draft 2020-12 +validator can check whether the BAST below is well-formed. + +**alktty does not depend on alktype.** The hand-rolled `ChunkReader` / +`ChunkWriter` in `src/wire.rs` is the runtime codec; this BAST document +is the human-readable contract that describes what those types +round-trip. If runtime validation against the BAST becomes desirable +later (e.g., to reject malformed chunks at the framing boundary with a +generated validator), alktype becomes an optional dep and this document +is already there to feed it. See [ADR-001](decisions/001-wire-format-and-two-carriage.md) +and [ADR-006](decisions/006-negotiation-framing-self-contained.md). + +## Scope + +The BAST below describes the two wire formats that live in this crate +(both one-way doors per [ADR-001](decisions/001-wire-format-and-two-carriage.md)): + +1. **The 5-byte chunk header** (`[stream_type: u8][length: u32 BE] + [payload]`) — the raw chunk codec in `src/wire.rs`. Five stream types: + `STREAM_STDIN=0`, `STREAM_STDOUT=1`, `STREAM_STDERR=2`, + `STREAM_CTRL_IN=3`, `STREAM_CTRL_OUT=4`. +2. **The negotiation frame** (4-byte BE length prefix + UTF-8 JSON + `NegotiateRequest` body) — self-contained per + [ADR-006](decisions/006-negotiation-framing-self-contained.md), not + reused from alkcall's `EventEnvelope` framing. The `NegotiateRequest` + JSON shape is wire-stable once consumers exist. + +The control-message `union` (field-name discriminator on `type`: +`resize`, `signal`, `eof`, `exit`) describes the JSON shape of the +control channel payloads; the on-wire encoding of a control chunk is a +5-byte chunk header with `stream_type ∈ {3, 4}` and a UTF-8 JSON payload +(the `ControlMessage` serialized via `serde_json`). See "Annotations" +below for where the JSON carriage does not map 1:1 to BAST's +binary-native vocabulary. + +## The BAST document + +```json +{ + "$schema": "https://alk.dev/bast/v1/schema", + "$defs": { + "ChunkHeader": { + "kind": "struct", + "endian": "big", + "fields": [ + { "name": "stream_type", "kind": "uint8" }, + { "name": "length", "kind": "uint32" } + ] + }, + + "StreamType": { + "kind": "enum", + "values": ["Stdin", "Stdout", "Stderr", "CtrlIn", "CtrlOut"] + }, + + "Chunk": { + "kind": "struct", + "endian": "big", + "fields": [ + { "name": "header", "kind": { "$ref": "#/$defs/ChunkHeader" } }, + { "name": "payload", "kind": "bytes", "encoding": "length-prefixed", "maxLength": 16777216 } + ] + }, + + "ControlMessage": { + "kind": "union", + "discriminator": { "kind": "field", "name": "type" }, + "fields": [ + { "name": "type", "kind": "string" } + ], + "mapping": { + "resize": { "$ref": "#/$defs/ResizeMessage" }, + "signal": { "$ref": "#/$defs/SignalMessage" }, + "eof": { "$ref": "#/$defs/EofMessage" }, + "exit": { "$ref": "#/$defs/ExitMessage" } + } + }, + + "ResizeMessage": { + "kind": "struct", + "fields": [ + { "name": "type", "kind": "string" }, + { "name": "cols", "kind": "uint16" }, + { "name": "rows", "kind": "uint16" }, + { "name": "pixel_width", "kind": "uint16" }, + { "name": "pixel_height", "kind": "uint16" } + ] + }, + + "SignalMessage": { + "kind": "struct", + "fields": [ + { "name": "type", "kind": "string" }, + { "name": "name", "kind": "string", "maxLength": 16 } + ] + }, + + "EofMessage": { + "kind": "struct", + "fields": [ + { "name": "type", "kind": "string" } + ] + }, + + "ExitMessage": { + "kind": "struct", + "fields": [ + { "name": "type", "kind": "string" }, + { "name": "code", "kind": "int32" } + ] + }, + + "NegotiationFrame": { + "kind": "struct", + "endian": "big", + "fields": [ + { "name": "length", "kind": "uint32" }, + { "name": "body", "kind": "string", "encoding": "length-prefixed", "maxLength": 16777216 } + ] + }, + + "NegotiateRequest": { + "kind": "struct", + "fields": [ + { "name": "carriage", "kind": "string", "maxLength": 16 }, + { "name": "backend", "kind": "string", "maxLength": 64 }, + { "name": "tty", "kind": { "$ref": "#/$defs/TerminalParams" } }, + { "name": "cmd", "kind": { "kind": "array", "element": "string", "count": 0 } }, + { "name": "cwd", "kind": "string" }, + { "name": "env", "kind": { "kind": "record", "values": "string" } }, + { "name": "backend_params", "kind": { "kind": "record", "values": {} } } + ] + }, + + "TerminalParams": { + "kind": "struct", + "fields": [ + { "name": "term", "kind": "string" }, + { "name": "cols", "kind": "uint16" }, + { "name": "rows", "kind": "uint16" }, + { "name": "pixel_width", "kind": "uint16" }, + { "name": "pixel_height", "kind": "uint16" }, + { "name": "modes", "kind": {} } + ] + } + } +} +``` + +## Annotations + +The BAST above is well-formed (it conforms to the BAST meta-schema). +Some definitions map 1:1 to the on-wire bytes; others describe the +logical shape of a JSON carriage whose on-wire encoding is UTF-8 text, +not the BAST-native binary encoding. The annotations below record +which is which and note the two deliberate deviations from BAST's +binary-native vocabulary. A generated validator should consult these +annotations alongside the BAST; the runtime codec (`src/wire.rs`) is +the source of truth for the bytes. + +### `ChunkHeader` — binary, exact + +Maps 1:1 to the on-wire bytes. 5 bytes: `stream_type` as `uint8` (1 +byte), `length` as `uint32` big-endian (4 bytes). This is the +5-byte chunk header committed by [ADR-001](decisions/001-wire-format-and-two-carriage.md). +The Rust types are `STREAM_STDIN`/`STREAM_STDOUT`/`STREAM_STDERR`/ +`STREAM_CTRL_IN`/`STREAM_CTRL_OUT` and `CHUNK_HEADER_LEN = 5` in +`src/wire.rs`. + +### `StreamType` — enum, documented deviation: on-wire encoding is `uint8`, not BAST's standard `u32` enum index + +The BAST meta-schema specifies that an `enum`'s binary representation +is a `u32` index into `values` (0-based). The `alk/tty` wire format +encodes `stream_type` as a **`uint8`** (1 byte), not a `u32` (4 bytes) — +the chunk header is 5 bytes, not 8. This is a deliberate deviation: +the chunk header needs a 1-byte discriminator, and 4 bytes of padding +would double the per-chunk overhead. + +The `StreamType` enum is therefore **normative for the name→value +mapping** (the index of a name in `values` is the integer that appears +on the wire as the `uint8` `stream_type`), but a generated validator +must NOT emit a `u32` read for `stream_type`. The on-wire encoding is +the `uint8` declared in `ChunkHeader`; the enum provides the +integer→name table for validation and diagnostics. + +| index | name | on-wire byte | direction | payload | +|-------|-----------|--------------|----------------|---------------------| +| 0 | `Stdin` | `0x00` | client→server | raw bytes | +| 1 | `Stdout` | `0x01` | server→client | raw bytes | +| 2 | `Stderr` | `0x02` | server→client | raw bytes | +| 3 | `CtrlIn` | `0x03` | client→server | UTF-8 JSON control | +| 4 | `CtrlOut` | `0x04` | server→client | UTF-8 JSON control | + +`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 — [ADR-001](decisions/001-wire-format-and-two-carriage.md)). + +### `Chunk` — binary, exact + +A chunk is the 5-byte `ChunkHeader` followed by `length` bytes of +payload. The `payload` field is `bytes` with `encoding: +"length-prefixed"` and `maxLength: 16777216` (16 MiB = `MAX_CHUNK_LEN` +in `src/wire.rs`). Zero-length payloads are sentinels on the data +channels (zero-length stdin = EOF from client; zero-length stdout = +"drained" from server); control chunks are never zero-length (the JSON +payload is at least `{}`). See [tty-wire.md](tty-wire.md) §"Sentinels". + +### `ControlMessage` — union, documented deviation: on-wire encoding is UTF-8 JSON, not BAST's binary union encoding + +The BAST `union` with a field-name discriminator describes a binary +layout whose discriminator is a length-prefixed string field and whose +variant is a binary struct. The `alk/tty` control channel does **not** +use that binary encoding: a control chunk's payload is **UTF-8 JSON +text** (`serde_json`-serialized), and the `type` tag is a JSON string +field, not a BAST length-prefixed string. + +The `ControlMessage` union here is **normative for the JSON variant +shapes** (the `mapping` keys are the JSON `type` values; the variant +structs are the JSON field sets a peer must accept/produce), but a +generated validator must NOT emit a binary union reader for control +chunks. The on-wire encoding is: read the `ChunkHeader`, read +`length` bytes as UTF-8, parse the result as JSON, dispatch on the +`type` field. The Rust type is `ControlMessage` in `src/control.rs` +(`#[serde(tag = "type", rename_all = "snake_case")]`). + +The `type`-tagged enum is the extension seam per +[ADR-001](decisions/001-wire-format-and-two-carriage.md): unknown +`type` values are **ignored** (not a protocol error) so a newer client +sending a control message an older server doesn't recognize degrades +gracefully. Adding a control message type is additive (two-way-door +within the one-way wire format); changing the meaning of an existing +type is not. + +### `ResizeMessage`, `SignalMessage`, `EofMessage`, `ExitMessage` — JSON shapes + +These are the four control message variants. Their field types +(`uint16`, `int32`, `string`) describe the **JSON value types** a peer +must accept/produce, not binary layouts — the on-wire encoding is +UTF-8 JSON text inside a control chunk's payload (see +`ControlMessage` above). The `maxLength` on `SignalMessage.name` is a +validation constraint (signal names are short uppercase strings: +`HUP`, `INT`, `QUIT`, `TERM`, `KILL`, `USR1`, `USR2`, `TSTP`, `CONT`); +it bounds the accepted JSON string length, not a binary reservation. + +| variant | direction | stream_type | JSON shape | +|---------|----------------|-----------------|----------------------------------------------------------------------------------| +| resize | client→server | `CtrlIn` (3) | `{"type":"resize","cols":80,"rows":24,"pixel_width":0,"pixel_height":0}` | +| signal | client→server | `CtrlIn` (3) | `{"type":"signal","name":"INT"}` | +| eof | client→server | `CtrlIn` (3) | `{"type":"eof"}` | +| exit | server→client | `CtrlOut` (4) | `{"type":"exit","code":0}` | + +The `exit` chunk is the last control chunk before stream close +([ADR-004](decisions/004-exit-code-on-control-chunk.md)); `code` is +`int32` (matches `std::process::ExitStatus::code()`; negative values +are signal-terminated, e.g., `-9` for SIGKILL on Unix; `-1` is the +"backend couldn't determine the exit code" sentinel). + +### `NegotiationFrame` — binary, exact (out-of-band for the chunk codec) + +The negotiation frame is a 4-byte big-endian length prefix + UTF-8 JSON +body. This maps 1:1 to the on-wire bytes: `length` as `uint32` +big-endian (4 bytes), `body` as a length-prefixed UTF-8 string +(`length` bytes). The `maxLength: 16777216` (16 MiB) constraint is the +same as `MAX_CHUNK_LEN` — error frames MUST be under 16 MiB so the +4-byte length prefix's high byte is `0x00`, which is what makes the +framing-disambiguation trick sound (first byte `0x00` = error/negotiation +frame; first byte `1`/`2`/`4` = raw chunk; see +[tty-wire.md](tty-wire.md) §"Constraints"). + +The negotiation frame is **out-of-band for the chunk codec**: it is +read once at session start (Phase 1, JSON carriage), then the stream +switches to raw chunks (Phase 2). The `ChunkReader`/`ChunkWriter` in +`src/wire.rs` does not read or write negotiation frames; the +`NegotiationReader`/`NegotiationWriter` in `src/negotiation.rs` does. +See [ADR-006](decisions/006-negotiation-framing-self-contained.md). + +### `NegotiateRequest` — JSON shape + +The `NegotiateRequest` struct describes the **JSON shape** of the +`NegotiationFrame.body`, not a binary layout. The on-wire encoding is +UTF-8 JSON text inside the negotiation frame's `body` field (see +`NegotiationFrame` above). The Rust type is `NegotiateRequest` in +`src/negotiation.rs` (`#[derive(Serialize, Deserialize)]` with +`#[serde(flatten)]` on `backend_params`). + +Field notes: +- `carriage` — `"raw"` in v1 (the only carriage); any other value → + `malformed_negotiation`. `maxLength: 16` bounds the accepted JSON + string length. +- `backend` — the backend selector key (`"local"`, `"docker"`, + `"ssh"`); `maxLength: 64`. +- `tty` — `null` for pipe/runner mode (no PTY — + [ADR-003](decisions/003-local-backend-placement.md)); `Some` for PTY + mode. The BAST uses a `$ref` to `TerminalParams` for the non-null + case; the on-wire JSON `null` is the pipe-mode sentinel. +- `cmd` — command vector (argv[0] + args); non-empty (checked by the + adapter). The BAST uses `array` with `count: 0` as a placeholder — + **BAST v1 requires `count` for arrays** (D-BAST-004), and a + variable-length command vector does not have a schema-known count. + This is a third documented deviation: the `cmd` field is a + JSON array of strings of arbitrary length, not a fixed-count BAST + array. A generated validator should treat `cmd` as a JSON array + (variable length, non-empty), not a BAST fixed-count array. +- `cwd` — working directory (`null` = inherit/default). +- `env` — environment variables (empty = inherit); a `record` from + string to string. +- `backend_params` — backend-specific selector fields, opaque to + alktty. The BAST uses `record` with an empty value type (`{}`) as a + placeholder for "arbitrary JSON value"; the adapter passes this map + through verbatim and each backend deserializes its own + strongly-typed params struct from it. See + [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md) §"Backend + params are opaque." + +### `TerminalParams` — JSON shape + +The terminal parameters carried in `NegotiateRequest.tty` when non-null. +JSON shape, not binary layout. `modes` is reserved (OQ-44 — default +terminal modes suffice for the current scope); backends MUST ignore +its content in v1. The empty value type (`{}`) is a placeholder for +"arbitrary JSON value." + +## Validation + +The BAST document above is validatable in two ways: + +1. **Structural validation** — run any JSON Schema Draft 2020-12 + validator against the BAST meta-schema at + `https://alk.dev/bast/v1/schema`. This checks whether the BAST is + well-formed (correct `kind` strings, required fields present, `$ref` + targets exist). It does NOT check whether a binary payload conforms + to the layout — that is the BAST-native validator's job (see the + alktype BAST format spec, + `/workspace/@alkdev/alktype/docs/architecture/bast-format.md`). +2. **Drift detection** — a cheap test that parses the BAST document + with `serde_json` and asserts the `StreamType` enum values match + `wire.rs`'s `STREAM_STDIN`/`STREAM_STDOUT`/`STREAM_STDERR`/ + `STREAM_CTRL_IN`/`STREAM_CTRL_OUT` constants (the index of each + name in `values` is the integer constant). This catches the common + drift case (a new stream_type added to `wire.rs` but not the BAST, + or vice versa) without requiring alktype as a dep — just + `serde_json`, which alktty already has. See the project setup plan's + "Risk: BAST schema drift" mitigation. + +## Design Decisions + +| Decision | ADR | Summary | +|----------|-----|---------| +| Wire format and two-carriage model | [ADR-001](decisions/001-wire-format-and-two-carriage.md) | The 5-byte chunk header + negotiation frame this BAST describes | +| Self-contained negotiation framing | [ADR-006](decisions/006-negotiation-framing-self-contained.md) | The `NegotiationFrame` is self-contained in alktty (not reused from alkcall's `EventEnvelope` framing) | +| Backend params are opaque | [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md) | `NegotiateRequest.backend_params` is an opaque JSON object; the adapter does not interpret it | +| Exit code on a control chunk | [ADR-004](decisions/004-exit-code-on-control-chunk.md) | The `ExitMessage` variant and the "exit chunk is last" invariant | + +## References + +- [ADR-001](decisions/001-wire-format-and-two-carriage.md) — the wire + format decision this BAST specifies +- [ADR-006](decisions/006-negotiation-framing-self-contained.md) — the + negotiation framing is self-contained (the `NegotiationFrame` is not + reused from alkcall) +- [tty-wire.md](tty-wire.md) — the prose wire format spec this BAST + formalizes +- `src/wire.rs` — the runtime chunk codec (`ChunkReader`/`ChunkWriter`, + `STREAM_*` constants, `MAX_CHUNK_LEN`) this BAST describes +- `src/control.rs` — the `ControlMessage` tagged enum this BAST + describes +- `src/negotiation.rs` — the `NegotiateRequest` / `NegotiationReader` / + `NegotiationWriter` this BAST describes +- [alktype BAST format spec](https://alk.dev/bast/v1/schema) — the + normative format spec for BAST documents (the meta-schema this + document conforms to); see also + `/workspace/@alkdev/alktype/docs/architecture/bast-format.md` +- Project setup plan, "Risk: BAST schema drift" — the drift-detection + test mitigation (`docs/plans/project-setup.md`) \ No newline at end of file diff --git a/docs/architecture/tty-local.md b/docs/architecture/tty-local.md new file mode 100644 index 0000000..33b45ff --- /dev/null +++ b/docs/architecture/tty-local.md @@ -0,0 +1,395 @@ +--- +status: draft (ported from alknet 2026-08-17; alknet-tty → alktty, + alknet-tty-local → alktty's `local` feature module, alknet/tty → + alk/tty, alknet-core → alkcall::core, alknet-call → alkcall, ADRs + renumbered 052..093 → 001..008) +last_updated: 2026-08-17 +--- + +# alktty — Local TTY Backend (`local` feature module) + +The local backend: a `TtyBackend` implementation that wraps +`portable_pty` for the PTY case (terminal semantics — resize, signal +delivery, escape-sequence handling) and `tokio::process::Command` with +`Stdio::piped()` for the pipe/runner case (process-streaming without +terminal semantics). This document specifies the `LocalTtyBackend`, the +blocking→async bridge pattern (REQ-TTY-01's reference implementation), +and the signal-delivery contract (REQ-TTY-02). The module placement is +decided in [ADR-003](decisions/003-local-backend-placement.md); the trait +it implements is in [tty-backend.md](tty-backend.md). + +## What + +`LocalTtyBackend` lives in alktty's `local` feature module +(`src/local/`, gated by the `local` cargo feature) and implements +`TtyBackend`. The backend's `allocate()` branches on `TtyParams.terminal`: + +- **`terminal: Some(TerminalParams { ... })`** — allocate a real PTY via + `portable_pty::native_pty_system().openpty()`, spawn the command into + the slave side, return a `TtyHandle` with merged stdout (stderr is + `None` — kernel PTY property) and a real `TtyControl` (resize via + `MasterPty::resize`, signal via `libc::kill(-pgid, sig)`). +- **`terminal: None`** — pipe mode, the runner case. Spawn the command + with `Stdio::piped()` for stdin/stdout/stderr, return a `TtyHandle` + with separate stdout and stderr (stderr is `Some`) and a `TtyControl` + whose `resize` is a no-op (no PTY) and `signal` calls + `libc::kill(pid, sig)` (still works for signal forwarding without a + PTY). + +The backend is the reference implementation of REQ-TTY-01 (backends need +not be natively async) and carries REQ-TTY-02 (signal forwarding to the +process group). + +## Why + +The local backend is the simplest backend and the one that enables the +runner pattern: a process whose stdin/stdout/stderr/exit-code stream over +a framed bidi connection — the same shape as GitHub/Gitea Actions runners, +just over alk's transport instead of HTTP polling. With +`LocalTtyBackend`, the dispatch project (a reverse runner that currently +requires SSH on the remote end) works without SSH — the endpoint runs +the process directly and streams its I/O back. SSH becomes one transport +option (for reaching hosts that don't run alk), not a requirement. + +The PTY case is what makes a terminal a terminal: real resize (via +`ioctl(TIOCSWINSZ)`), signal delivery to the foreground process group +(via `libc::kill(-pgid, sig)`, REQ-TTY-02), and escape-sequence handling +(the kernel PTY's line discipline). Without a PTY, it's a runner (piped +process); with a PTY, it's a terminal. The per-session choice +(`TtyParams.terminal`) lets one `LocalTtyBackend` serve both — see +ADR-003. + +The wrinkle that drove the Phase 0 POC: `portable_pty` is a **blocking +`std::io` API**, not async. `MasterPty::try_clone_reader()` returns +`Box`; `take_writer()` returns +`Box`; `Child::wait()` blocks. The POC was +built to discover how that constraint shapes the `TtyBackend` trait +(REQ-TTY-01) and the signal-delivery contract (REQ-TTY-02). This spec +records both as requirements, not open questions — the POC turned them +into grounded requirements. + +## Architecture + +### PTY Mode (`terminal: Some`) + +`allocate()` calls `portable_pty::native_pty_system().openpty(PtySize)` +with the terminal dimensions, spawns the command into the slave side +via `SlavePty::spawn_command(CommandBuilder)`, drops the slave (so the +child sees EOF on its stdin when the master writer closes), and returns +a `TtyHandle`. + +The blocking→async bridge (REQ-TTY-01's reference implementation): +**three dedicated std threads** feed tokio mpsc/oneshot channels. The +writer thread consumes an mpsc of `StdinCmd`: + +```rust +pub enum StdinCmd { + Bytes(Vec), // write these bytes to the master writer + Eof, // close the master writer (EOF to the slave's stdin) +} +``` + +1. **Reader thread** — blocking reads from `MasterPty::try_clone_reader()` + → `mpsc::Sender`. The reader loop reads into an 8 KiB buffer, + copies each chunk to `Bytes`, and `blocking_send`s to the mpsc. On EOF + (the master reader returns EOF when the slave closes — the child has + exited and the OS has drained the PTY buffer), the thread sends a + zero-length `Bytes` sentinel (the "drained" signal) and exits. The + async-facing `TtyHandle.stdout` is the `mpsc::Receiver`, + wrapped as `Pin + Send>>`. +2. **Writer thread** — drains an `mpsc::Receiver` → blocking + writes to `MasterPty::take_writer()`. `StdinCmd::Bytes(bytes)` writes + and flushes; `StdinCmd::Eof` drops the writer (sends EOF to the + slave's stdin) and exits. The async-facing `TtyHandle.stdin` is the + `mpsc::Sender`, wrapped as `Box` (an `AsyncWrite` impl that wraps each `write` as a + `StdinCmd::Bytes` and `flush` as a no-op; the `mpsc::Sender` is the + sink). +3. **Waiter thread** — blocking `Child::wait()` → `oneshot::Sender` + with the exit code. The async-facing `TtyHandle.exit_code` is a + `Future` wrapping this `oneshot::Receiver` PLUS a kill guard + holding the `portable_pty::ChildKiller` (see "Cancel-Cleanup + (ADR-005)" below). This is the `Future` the adapter awaits (ADR-002 + REQ-TTY-01; ADR-004); its `Drop`-on-cancel kills the child + (ADR-005). + +`TtyHandle.stderr` is `None` (PTY backends merge stdout/stderr — kernel +PTY property, one output stream from the slave). + +`TtyHandle.control` is a `PtyControl` struct (the POC's concrete type; +the trait-object form per ADR-002 is the `Arc`-backed `Clone` newtype, +OQ-43): + +```rust +#[derive(Clone)] +pub struct PtyControl { + master: Arc>>, + killer: Arc>>, + pid: Option, +} +``` + +`resize()` locks the master and calls `MasterPty::resize(PtySize)` — +non-blocking (it issues an `ioctl`). `signal()` — see REQ-TTY-02 below. + +### REQ-TTY-02: Signal Forwarding Must Target the Process Group + +`libc::kill(pid, sig)` on the spawned child's pid alone is **insufficient** +for terminal semantics: a shell running under a PTY will have spawned +children (a `find | grep` pipeline, a `make` with sub-makes), and those +children will not receive the signal. A real terminal forwards Ctrl-C to +the **foreground process group**, which (under job-control shells) is the +process group the shell most recently spawned for the foreground job. + +`portable_pty` makes the child a session leader (when +`controlling_tty = true`, the default — `CommandBuilder::set_controlling_tty(true)`), +so the child's pid *is* its process-group id, and `libc::kill(-pid, sig)` +(the negative pid) reaches the whole group. The POC's `PtyControl::signal` +uses exactly this — `kill(-pgid, sig)` with a fallback to `kill(pid, sig)` +if the group signal fails (e.g., the child already exited). + +The spec records: + +1. **The local backend MUST forward signals to the child's process + group, not just the child pid.** Using `kill(-pgid, sig)` when the + child is a session leader (the `portable_pty` default). +2. **The local backend MUST spawn the child as a session leader with a + controlling tty.** This is `portable_pty`'s default + (`CommandBuilder::set_controlling_tty(true)`); disabling it (e.g., + for container-boundary workarounds) breaks signal forwarding and is + therefore not supported for the terminal use case. +3. **The `TtyControl::signal` contract is "best-effort delivery to the + foreground process group,"** not "the child pid receives the signal." + Unknown signal names fall back to the backend's default kill + (`portable_pty`'s `ChildKiller::kill` sends SIGHUP); known names map + to `libc` signal numbers (`HUP`, `INT`, `QUIT`, `TERM`, `KILL`, + `USR1`, `USR2`, `TSTP`, `CONT`) and are sent to the group. + +This pre-empts a class of "Ctrl-C doesn't kill my `cargo build`" bugs +that would otherwise surface in Phase 2/3. + +### Cancel-Cleanup (ADR-005) + +The `TtyBackend` cleanup contract (ADR-005): **dropping the `exit_code` +future kills the session target.** The local backend implements this for +both PTY and pipe modes. + +**PTY mode.** `allocate()` obtains a `portable_pty::Child` (with +`wait()`) and a `portable_pty::ChildKiller` (with `kill()`) — the two +handles `portable_pty` exposes alongside each other. The `Child` moves +into the waiter thread (which blocks on `wait()`). The `ChildKiller` +moves into the `exit_code` future's `Drop` guard, alongside the +`oneshot::Receiver` from the waiter thread. The future's `poll` +delegates to the oneshot receiver (resolves on natural exit); the +future's `Drop` (runs on cancel only — on resolve, the guard is +disarmed) calls `ChildKiller::kill(SIGHUP)`: + +```rust +struct LocalExitFuture { + rx: oneshot::Receiver, + killer: Option, // None after resolve (disarmed) +} + +impl Future for LocalExitFuture { /* poll delegates to rx; on Ready, take killer */ } +impl Drop for LocalExitFuture { + fn drop(&mut self) { + if let Some(killer) = self.killer.take() { + let _ = killer.kill(SIGHUP); // best-effort; child may already be exiting + } + } +} +``` + +On cancel: the `Drop` kills the child (SIGHUP); the child exits; the +waiter thread's `wait()` reaps it and exits (its `oneshot::send` fails +silently — the receiver was dropped with the future, which is expected); +the reader/writer threads exit on channel close. The child is reaped +(no zombie) by the waiter thread's `wait()` returning after the kill. + +**Pipe mode.** The same pattern with `tokio::process::Child` instead of +`portable_pty::Child`. The `exit_code` future's `Drop` guard holds the +`Child` handle (or a `Child`-kill wrapper) and calls +`Child::start_kill()` on cancel. The waiter task (`Child::wait()`) +reaps the killed child. + +**The happy path is unaffected.** When the adapter drives `exit_code` +to completion (the child exits naturally), the future resolves, the +guard is disarmed (the `Option::take()` in `poll`'s `Ready` branch), +and the subsequent `Drop` is a no-op. The contract is "kill on cancel; +no-op on resolve." + +This closes the orphaned-process gap the local-PTY POC surfaced: a +child that ignores stdin EOF (a daemon, a long-lived process with no +stdin reader) is killed when the session is cancelled, not left +running. The POC's `LocalPty::exit_code` was a bare +`oneshot::Receiver` with no kill guard — an implementer who +copies the POC's shape without the guard violates the contract. See +ADR-005 for the contract and the trait-level rationale. + +### Pipe Mode (`terminal: None`) + +`allocate()` spawns the command with `tokio::process::Command` and +`Stdio::piped()` for stdin, stdout, and stderr. The async bridge is +simpler than the PTY case — tokio's `Child` provides `AsyncRead` for +stdout/stderr and `AsyncWrite` for stdin directly (no std-thread +bridge needed). `TtyHandle.stderr` is `Some` (separate streams). The +`exit_code` future is `Child::wait()` (async on tokio's `Child`). + +`TtyHandle.control` is a `PipeControl` whose `resize()` is a no-op +(no PTY — resize doesn't apply) and `signal()` calls `libc::kill(pid, sig)` +on the child's pid. Signal forwarding to the process group is not +applicable in pipe mode (there's no session leader / controlling tty); +`kill(pid, sig)` reaches the direct child only. If the child has +spawned its own children, they won't receive the signal — this is a +known limitation of the runner case (a runner that needs +process-group signal delivery uses the PTY case, not the pipe case). + +### The Threading/Deadlock Caveat (DP-4, Acknowledged Constraint) + +`std::process::Command` with piped stdio can deadlock if stdin writes +block while stdout/stderr buffers fill — the classic pipe-buffer deadlock. +The fix is concurrent reads on stdout/stderr alongside stdin writes, +which is exactly what the bidirectional pump does (the POC's +`drive_attach_raw` runs the two directions as concurrent +`tokio::spawn` tasks). The same pattern works for `LocalTtyBackend`: +spawn one task pumping stdin→process, one task pumping process→stdout-chunks, +one for stderr if piped. This is a known constraint with a known solution +(POC-validated); no design decision needed. + +### Module Placement (ADR-003) + +The local backend is folded into alktty behind a `local` cargo feature +(the single-crate consolidation ADR-003 records): + +```toml +# alktty Cargo.toml +[features] +default = [] +local = ["dep:portable-pty", "dep:tokio-util", "tokio/process", "tokio/rt-multi-thread"] +``` + +A consumer that wants the local backend enables `features = ["local"]` +and gets `alktty::local::LocalTtyBackend`. A consumer that only wants +docker/ssh uses the default features and depends on the backend crate +directly — no `portable_pty` in the dependency tree. See ADR-003. + +The single-crate consolidation resolves the alknet cyclic-dep +workaround that motivated the original sibling-crate decision +(alknet ADR-054): `alknet-tty-local` depended on `alknet-tty` for the +trait, and ADR-054 wanted `alknet-tty` to re-export `LocalTtyBackend` +behind a `local` feature — which cargo rejects (a crate cannot +re-export from a sibling crate it depends on via an optional dep AND +have that sibling depend back on it). The workaround in the alknet +mono-repo was the assembly-layer pattern (consumer depends on both +crates directly). In alktty, the local backend is in the same crate as +the trait, so the cyclic-dep workaround doesn't apply. ADR-003 records +both the original alknet decision and the alktty consolidation. + +### Dependencies + +``` +alktty (local feature) +├── alktty (default) (TtyBackend trait, TtyHandle, TtyControl, wire types) +├── alkcall::core (via alktty's re-export; not direct) +├── portable_pty (PTY allocation — the heavy dep, Unix openpty + Windows ConPTY) +├── libc (signal forwarding — REQ-TTY-02, Unix only) +└── tokio (process, rt-multi-thread, mpsc, oneshot, AsyncRead/AsyncWrite) +``` + +The `local` feature is inherently non-wasm (`portable-pty` + +`tokio::process` need a real OS); enabling `local` on +`wasm32-unknown-unknown` is a build error by design. The default crate +(no features) stays wasm-clean — see the crate root's `# WASM target` +doc comment. + +## The Runner Pattern + +The pipe mode (`terminal: None`) is the "runner" generalization the +research identified. A coordinator sends a negotiation frame with +`{ "backend": "local", "tty": null, "cmd": ["cargo", "test"] }`; the +endpoint runs `cargo test` with piped stdio, streams stdout/stderr chunks +back, sends `{"type":"exit","code":N}` when it finishes (ADR-004). The +coordinator gets reliable completion notification (the exit control +chunk + stream close) — no polling, no plugin state. + +This is functionally identical to GitHub/Gitea Actions runners, just over +alk's transport instead of HTTP polling. The dispatch project is a +reverse runner that currently requires SSH on the remote end; with +`LocalTtyBackend`, the same pattern works without SSH — the endpoint +runs the process directly. SSH becomes one transport option (for +reaching hosts that don't run alk), not a requirement. + +The runner-specific API surface (job management, log persistence, task +graph integration) is **out of scope for alktty** (OQ-46). alktty +provides the *mechanism* (a framed byte stream for a process + exit +code); the runner *policy* is a downstream crate's job. This spec +commits to preserving the option (`terminal: None` → pipe mode) and not +building runner policy into alktty. + +## Constraints + +- **PTY mode requires `portable_pty`'s native PTY (Unix `openpty` / + Windows ConPTY).** The blocking→async bridge (three std threads) is + the documented pattern for any blocking-API backend (REQ-TTY-01). + PTY mode is `#[cfg(unix)]`-only in the source; pipe mode is + cross-platform. +- **Signal forwarding in PTY mode targets the process group (REQ-TTY-02).** + `kill(-pgid, sig)` when the child is a session leader + (`controlling_tty = true`, the default). Disabling the controlling tty + breaks signal forwarding and is not supported for the terminal use + case. +- **Pipe mode does not forward signals to the process group.** `kill(pid, + sig)` reaches the direct child only; grandchildren don't receive it. + A runner that needs process-group signal delivery uses the PTY case. +- **The pipe-buffer deadlock is handled by the concurrent pump.** The + adapter's three-pump driver (`tty-adapter.md`) reads stdout/stderr + concurrently with writing stdin — the POC-validated pattern. No design + decision needed; the spec notes it as a known constraint with a known + solution. +- **`LocalTtyBackend` takes no constructor dependencies.** Unlike + `DockerTtyBackend` (wraps a `bollard::Docker` client) or + `SshTtyBackend` (wraps an SSH session), the local backend is + dependency-free at construction — the `portable_pty` system is + process-global. The assembly layer constructs one `LocalTtyBackend` + and registers it as `"local"`. +- **The `exit_code` future's `Drop`-on-cancel kills the child + (ADR-005).** The local backend MUST NOT return a bare + `oneshot::Receiver` as `TtyHandle.exit_code` — it must wrap it + in a `Future` whose `Drop` calls `ChildKiller::kill(SIGHUP)` (PTY) or + `Child::start_kill()` (pipe) when dropped without resolving. An + implementer who copies the POC's bare `oneshot::Receiver` shape + without the kill guard violates the contract and will orphan + processes on session cancel. See ADR-005. + +## Design Decisions + +| Decision | ADR | Summary | +|----------|-----|---------| +| Local backend placement | [ADR-003](decisions/003-local-backend-placement.md) | alktty folds the local backend in behind a `local` feature (resolves the alknet cyclic-dep workaround); PTY vs pipe per-session | +| `TtyBackend` trait and `TtyHandle` | [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md) | The trait this backend implements; REQ-TTY-01 (backends need not be natively async) | +| Wire format | [ADR-001](decisions/001-wire-format-and-two-carriage.md) | The chunk codec + control channel the adapter pumps to/from this backend | +| Exit code on a control chunk | [ADR-004](decisions/004-exit-code-on-control-chunk.md) | The waiter thread's `oneshot::Receiver` feeds the exit chunk | +| Backend cleanup on session cancel | [ADR-005](decisions/005-backend-cleanup-on-session-cancel.md) | The `exit_code` future's `Drop`-on-cancel kills the child via `ChildKiller` (PTY) / `start_kill` (pipe); the waiter thread reaps | + +## Open Questions + +- **OQ-46** (deferred(scope)): Runner API surface. + +## References + +- [ADR-003](decisions/003-local-backend-placement.md) — the module + placement decision (single-crate consolidation) +- [ADR-002](decisions/002-ttybackend-trait-and-ttyhandle.md) — the + trait this backend implements; REQ-TTY-01 (the blocking-backend + accommodation) +- [ADR-004](decisions/004-exit-code-on-control-chunk.md) — the + waiter thread's `oneshot::Receiver` feeds the exit chunk +- [ADR-005](decisions/005-backend-cleanup-on-session-cancel.md) — + the cancel-cleanup contract this backend implements (the `exit_code` + future's `Drop`-on-cancel kills the child via `ChildKiller` / + `start_kill`) +- `src/local/` — the Rust source this spec documents (`backend.rs`, + `pty.rs`, `pipe.rs`) +- [tty-backend.md](tty-backend.md) — the trait this backend implements +- [tty-adapter.md](tty-adapter.md) — the session driver that consumes + this backend's handles \ No newline at end of file diff --git a/docs/architecture/tty-wire.md b/docs/architecture/tty-wire.md new file mode 100644 index 0000000..cb55d8a --- /dev/null +++ b/docs/architecture/tty-wire.md @@ -0,0 +1,394 @@ +--- +status: draft (ported from alknet 2026-08-17; alknet-tty → alktty, + alknet/tty → alk/tty, alknet-core → alkcall::core, alknet-call → + alkcall, ADRs renumbered 052..093 → 001..008) +last_updated: 2026-08-17 +--- + +# alktty — Wire Format + +The wire protocol for `alk/tty`: the negotiation frame (JSON 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-001](decisions/001-wire-format-and-two-carriage.md); +this document specifies what an implementer builds. + +## What + +A `alk/tty` bidi stream carries one terminal session. The stream has +two phases: + +1. **Negotiation (JSON carriage).** A single length-prefixed JSON frame + from the client carrying the terminal parameters, backend selector, + command, and environment. +2. **Raw carriage.** After the negotiation frame, the stream switches to + a chunk format for the life of the session: bidirectional byte pumping + with a 1-byte stream-type multiplexer and a JSON control channel. + +The format is the alknet-docker POC's raw chunk format (stream_type +0/1/2) extended with a 4th stream_type (3 = control) and a JSON control +message schema, both validated by the alknet-tty POC. See ADR-001. + +## Why + +A terminal session is a byte stream with a small control sideband. The +two-carriage model (JSON negotiation, then raw chunks) keeps the call +protocol's JSON-RPC shape for the structured request and switches to +bytes for the body, which is what a terminal actually is. The fixed +channel set (five stream types, no negotiation) is an impoverishment of +SSH's channel multiplexer that is the feature: alktty multiplexes *one* +service (a terminal session) with a fixed channel structure, not +*arbitrary* services, so the demux is a `match`, not a hash lookup. The +full rationale — why not JSON for everything, why fixed channel set +rather than extensible — is in +[ADR-001](decisions/001-wire-format-and-two-carriage.md) §Context. + +## Architecture + +### Phase 1: Negotiation Frame (JSON Carriage) + +The client opens a bidi stream (or the server accepts one) and writes a +single length-prefixed JSON frame. The framing is a 4-byte big-endian +length prefix + UTF-8 JSON body — a self-contained ~30-line module in +alktty (read 4-byte length, bounds-check, read N bytes; write the +inverse) on tokio's `AsyncRead`/`AsyncWrite`. The format coincides with +alkcall's `EventEnvelope` framing by convention (both are +length-prefixed JSON), not by code reuse — alktty does not depend on +alkcall's internal wire types. The negotiation payload is a tty-specific +struct (`NegotiateRequest`), not a `call.requested` event. See ADR-001 +§6 and [ADR-006](decisions/006-negotiation-framing-self-contained.md). + +The payload shape: + +```json +{ + "carriage": "raw", + "backend": "local", + "tty": { + "term": "xterm-256color", + "cols": 80, + "rows": 24, + "pixel_width": 0, + "pixel_height": 0, + "modes": {} + }, + "cmd": ["/bin/bash"], + "cwd": null, + "env": {} +} +``` + +Fields: + +- `carriage` — `"raw"` for terminal sessions (the only carriage in v1). + Selects the post-negotiation byte format. MUST be `"raw"` in v1; any + other value (e.g., `"json"`, an unknown carriage, or the field + absent) is a `malformed_negotiation` error and the adapter closes the + stream without entering raw mode. A future carriage (e.g., a + structured JSON-only mode for a non-terminal use case) is a v2 + addition; in v1 the field is required and must be the literal + `"raw"`. +- `backend` — the backend selector string (`"local"`, `"docker"`, + `"ssh"`). The adapter dispatches to the registered `TtyBackend` by this + key (ADR-002 §5). +- `tty` — terminal parameters. `null` for the pipe/runner case (no PTY — + [ADR-003](decisions/003-local-backend-placement.md)). `Some` for the PTY case. The `tty` block maps directly to + SSH's `pty_request` parameters (term, cols, rows, pixel_width, + pixel_height, modes) and to docker's `CreateExecOptions { tty: true }`; + a local backend passes it to `portable_pty::PtySystem::openpty`. The + `modes` field is reserved (OQ-44 — default terminal modes suffice for + the current scope). +- `cmd` — command vector (argv[0] + args). Non-empty. +- `cwd` — working directory (`null` = inherit/default). +- `env` — environment variables (empty = inherit). + +The Rust struct the adapter parses the frame into: + +```rust +#[derive(Deserialize)] +pub struct NegotiateRequest { + pub carriage: String, // "raw" in v1; any other value → malformed_negotiation + pub backend: String, // backend selector key ("local", "docker", "ssh") + pub tty: Option, // None = pipe mode (ADR-003) + pub cmd: Vec, // argv[0] + args; non-empty + #[serde(default)] + pub cwd: Option, // None = inherit/default + #[serde(default)] + pub env: HashMap, // empty = inherit + #[serde(default)] + pub backend_params: serde_json::Map, // opaque; backend-deserialized + // plus backend-specific fields, captured into backend_params via serde(flatten) +} + +#[derive(Deserialize)] +pub struct TerminalParamsWire { + pub term: Option, // None = backend default + pub cols: u16, + pub rows: u16, + #[serde(default)] + pub pixel_width: u16, + #[serde(default)] + pub pixel_height: u16, + #[serde(default)] + pub modes: serde_json::Value, // reserved — OQ-44; backends MUST ignore content in v1 +} +``` + +Validation: `carriage` MUST be `"raw"` (else `malformed_negotiation`); +`cmd` MUST be non-empty (else `malformed_negotiation`); `backend` MUST +be a registered backend key (else `unknown_backend`). Backend-specific +params validation is the backend's job (in `allocate()`); the adapter +does not interpret `backend_params`. The struct's `serde(flatten)` for +backend-specific fields means the negotiation frame's top-level JSON +object carries both the shared fields (`carriage`, `backend`, `tty`, +`cmd`, `cwd`, `env`) and the backend-specific fields (e.g., +`"container": "abc123"` for docker); the latter land in +`backend_params`. + +Backend-specific selector fields ride alongside (e.g., `"container": +"abc123"` for docker). The adapter parses the negotiation frame, +extracts the `backend` string, and passes the remaining backend-specific +fields to the selected backend's `allocate()` as an opaque +`serde_json::Map` (ADR-002) — the adapter does not interpret them; the +backend deserializes its own strongly-typed params struct. + +After the negotiation frame, the stream switches to raw chunks. There is +no `call.responded`/`call.completed` — this is not the call protocol. + +### Phase 2: Raw Chunk Format + +```text +[stream_type: u8][length: u32 be][payload bytes] +``` + +- **`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 | ctrl-in | client→server | JSON control message (`Resize`, `Signal`, `Eof`) | + | 4 | ctrl-out | server→client | JSON control message (`Exit`) | + + `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 (`alk/tty/v2` per alknet ADR-006), not a + negotiated addition to this format. See ADR-001 §"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. 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`). + +- **`payload`** (`length` bytes) — the raw bytes (for data channels) or + UTF-8 JSON (for the control channel). + +The codec is `ChunkReader`/`ChunkWriter` in `src/wire.rs`: +`ChunkReader::read_chunk()` reads the 5-byte header, validates the +stream_type and length, reads the payload; `ChunkWriter::write_chunk()` +writes the header and payload. See ADR-001. + +### Sentinels + +Zero-length data chunks are sentinels: + +- **Zero-length stdin chunk (stream_type 0, length 0)** — EOF from the + client. The server closes the backend's stdin (`ChildStdin::drop` / + PTY writer close). This is one of two canonical "stdin done" signals; + the other is a `{"type":"eof"}` control chunk — see OQ-47. +- **Zero-length stdout chunk (stream_type 1, length 0)** — "drained" + from the server. The backend's stdout stream ended (process exited, + container output stream ended, SSH channel closed). This is an + implementation sentinel; the deterministic completion signal is the + exit control chunk ([ADR-004](decisions/004-exit-code-on-control-chunk.md)), not this sentinel — but the drained + sentinel is emitted for symmetry with the docker POC's pattern. + +Control chunks are never zero-length (the JSON payload is at least +`{}`). + +### Control Channel + +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 +`ControlMessage` enum (`src/control.rs`): + +```rust +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum ControlMessage { + Resize { + cols: u16, + rows: u16, + #[serde(default)] + pixel_width: u16, + #[serde(default)] + pixel_height: u16, + }, + Signal { name: String }, + Eof, + Exit { code: i32 }, +} +``` + +| 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-004) | + +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. + +**Signal names.** `name` is an uppercase string. The supported set (per +`signal_from_name` in `src/control.rs`): `HUP`, `INT`, `QUIT`, `TERM`, +`KILL`, `USR1`, `USR2`, `TSTP`, `CONT`. Unknown names fall back to the +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). + +**Extensibility.** The `type` tag is the extension seam: new control +message types are added by extending the tagged enum. Unknown `type` +values are **ignored** (not a protocol error) so that a newer client +sending a control message an older server doesn't recognize degrades +gracefully rather than tearing down the session. This is a two-way-door +extension point within the one-way-door wire format (ADR-001) — adding a +control message type is additive; changing the meaning of an existing +type is not. + +### Stdin Closure + +Two signals both close the client's stdin: + +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. + +The spec recommends `eof` for explicitness (it's a control message, not +a data-length hack), but both are accepted. See OQ-47. + +### Connection vs Stream + +A `Connection` (alknet ADR-007) can open/accept multiple bidi streams. One +`alk/tty` connection hosts multiple terminal sessions — one session per +bidi stream (DP-6, decided in the alknet research). This matches the call +protocol's model (one operation per stream, multiple operations per +connection) and is the natural fit for QUIC's stream multiplexing. A +coordinator opens one connection to an endpoint and launches multiple +sessions (one stream each) for parallel tasks. The `TtyAdapter::handle` +accepts the connection and loops `accept_bi`, dispatching each stream to +a session — see [tty-adapter.md](tty-adapter.md). + +## Constraints + +- **The wire format is one-way (ADR-001).** The 5-byte header, the fixed + stream_type set (0-4), and the two-carriage sequence are bytes clients + and servers parse. A 6th channel type requires a new ALPN + (`alk/tty/v2` per alknet 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_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 + the client sends data" step — QUIC's stream reliability handles + in-order delivery, and the negotiation frame is small (fits in the + initial flow-control window — ADR-001 assumption 2). +- **Negotiation errors are JSON, not chunks.** If the server cannot + allocate the session (unknown backend, PTY allocation failed, the + command is invalid), it sends a JSON error response in the same + length-prefixed framing as the negotiation frame and closes the stream + 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`/`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-001](decisions/001-wire-format-and-two-carriage.md) | `alk/tty` ALPN; JSON negotiation frame then raw chunks; fixed channel set 0-4; control as JSON | +| Bidirectional control channel split | Phase 7 (amendment inside ADR-001) | `STREAM_CTRL_IN = 3` (client→server) and `STREAM_CTRL_OUT = 4` (server→client) replace the single `STREAM_CONTROL = 3`; the adapter enforces the direction | +| Self-contained negotiation framing | [ADR-006](decisions/006-negotiation-framing-self-contained.md) | alktty implements its own length-prefixed framing; format coincides with alkcall's by convention, not by code reuse | +| Exit code on a control chunk | [ADR-004](decisions/004-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 + +- **OQ-44** (deferred(scope)): Terminal modes. +- **OQ-45** (resolved): Flow control for high-throughput stdout — no + application-level windowing; QUIC per-stream flow control is the + backpressure mechanism. +- **OQ-47** (resolved): Stdin closure canonical signal. + +## References + +- [ADR-001](decisions/001-wire-format-and-two-carriage.md) — the wire + format decision +- [ADR-004](decisions/004-exit-code-on-control-chunk.md) — the + exit-chunk ordering the control channel carries +- [ADR-006](decisions/006-negotiation-framing-self-contained.md) — the + dependency-edge decision (negotiation framing is self-contained in + alktty) +- alknet ADR-003 Amendment 2 — alktty does not depend on alknet-call + (self-contained framing); see the alknet originals at + `/workspace/@alkdev/alknet/docs/architecture/decisions/` +- `src/wire.rs` — the chunk codec (`ChunkReader`/`ChunkWriter`, + stream_type 0-4) this spec documents +- `src/control.rs` — the JSON control schema (`ControlMessage` tagged + enum) this spec documents +- [tty-bast.md](tty-bast.md) — the BAST (Binary Abstract Syntax Tree) + document for this wire format; a normative JSON spec downstream + consumers can validate against +- [tty-adapter.md](tty-adapter.md) — the session lifecycle that consumes + this wire format \ No newline at end of file diff --git a/docs/plans/project-setup.md b/docs/plans/project-setup.md index fb7d4f7..839f07b 100644 --- a/docs/plans/project-setup.md +++ b/docs/plans/project-setup.md @@ -1,6 +1,8 @@ # Project Setup Plan -Status: draft (revised 2026-08-17 to reflect landed upstream changes) +Status: draft (revised 2026-08-17 to reflect landed upstream changes; +Phase 4 landed 2026-08-17 — architecture docs + BAST schema + +renumbered ADRs) Last updated: 2026-08-17 ## Overview @@ -415,32 +417,76 @@ operation spec — the registry runs the ACL before the wrapper, so the `local` feature in `src/lib.rs`: `#[cfg(feature = "local")] pub mod local;` -### Phase 4: Architecture docs + BAST schema +### Phase 4: Architecture docs + BAST schema — landed 2026-08-17 -1. Port the 5 spec docs from `alknet/docs/architecture/crates/tty/` +1. Ported the 5 spec docs from `alknet/docs/architecture/crates/tty/` into `docs/architecture/` (flat layout, not the `crates/tty/` subpath — this is a single-crate repo now): - `tty-wire.md`, `tty-backend.md`, `tty-adapter.md`, `tty-local.md`, - plus an `overview.md` index. -2. Port relevant ADRs (052, 053, 054, 055, 056, 057, 077, 093) — - renumber into alktty's ADR range (001..008), update cross-references - (`alknet/tty` → `alk/tty`, `alknet-tty-local` → `alktty`'s `local` - feature, `alknet-core` → `alkcall::core`). -3. Write `docs/architecture/tty-bast.md` — the BAST JSON document for - the `alk/tty` wire format. Covers: + `overview.md` (the crate overview; the alknet `README.md` index is + ported as `docs/architecture/README.md`), `tty-wire.md`, + `tty-backend.md`, `tty-adapter.md`, `tty-local.md`. Each is renamed + `alknet-tty` → `alktty`, `alknet/tty` → `alk/tty`, `alknet-core` → + `alkcall::core`, `alknet-call` → `alkcall`, `alknet-tty-local` → + alktty's `local` feature module, and cross-references to alknet + ADRs are renumbered to alktty's ADR range (001..008). +2. Ported the 8 alknet ADRs (052, 053, 054, 055, 056, 057, 077, 093) + into `docs/architecture/decisions/` renumbered 001..008 in order: + - [001](../architecture/decisions/001-wire-format-and-two-carriage.md) + ← alknet ADR-052 — wire format + two-carriage model (incl. the + Phase 7 control-channel split amendment) + - [002](../architecture/decisions/002-ttybackend-trait-and-ttyhandle.md) + ← alknet ADR-053 — `TtyBackend` trait + `TtyHandle` + - [003](../architecture/decisions/003-local-backend-placement.md) + ← alknet ADR-054 — local backend placement (records both the + alknet sibling-crate decision and the alktty single-crate + consolidation behind a `local` feature) + - [004](../architecture/decisions/004-exit-code-on-control-chunk.md) + ← alknet ADR-055 — exit code on a control chunk + - [005](../architecture/decisions/005-backend-cleanup-on-session-cancel.md) + ← alknet ADR-056 — backend cleanup on session cancel + - [006](../architecture/decisions/006-negotiation-framing-self-contained.md) + ← alknet ADR-057 — self-contained negotiation framing + - [007](../architecture/decisions/007-tty-inside-channels.md) + ← alknet ADR-077 — TTY inside channels (reversed by 008; kept for + historical context with its reversal notice pointing to 008) + - [008](../architecture/decisions/008-channels-pure-channel-multiplexing.md) + ← alknet ADR-093 — channels pure channel multiplexing (reverses + 007; TTY always uses its 5-byte format) +3. Wrote + [`docs/architecture/tty-bast.md`](../architecture/tty-bast.md) — + the BAST (Binary Abstract Syntax Tree) document for the `alk/tty` + wire format. Conforms to the BAST meta-schema at + `https://alk.dev/bast/v1/schema`; validatable by any JSON Schema + Draft 2020-12 validator. Covers: - The 5-byte chunk header (`struct` with `endian: "big"`: `stream_type: uint8`, `length: uint32`). - - The four stream-type channels as an `enum` (`Stdin=0`, + - The five stream-type channels as an `enum` (`Stdin=0`, `Stdout=1`, `Stderr=2`, `CtrlIn=3`, `CtrlOut=4`). - The control-message `union` (field-name discriminator on `type`: - `resize`, `signal`, `eof`, `exit`). + `resize`, `signal`, `eof`, `exit`) with the documented deviation + that on-wire control payloads are UTF-8 JSON, not BAST's binary + union encoding. - The negotiation frame as a separate `struct` (4-byte BE length prefix + UTF-8 JSON `NegotiateRequest` body) — annotated as out-of-band for the chunk codec but documented for completeness. - - Conforms to the BAST meta-schema at - `https://alk.dev/bast/v1/schema`; validatable by any JSON Schema - Draft 2020-12 validator. -4. Write `docs/architecture/README.md` index. + - The `StreamType` enum's documented deviation: on-wire encoding is + `uint8` (1 byte), not BAST's standard `u32` enum index (4 bytes) + — the chunk header is 5 bytes, not 8. +4. Wrote [`docs/architecture/README.md`](../architecture/README.md) — + the architecture index: documents table, ADR table (with alknet + origin numbers and port status), key design principles, open + questions, and references. + +The ADR mapping follows the plan's 8-to-8 list (ADR-052→001, 053→002, +054→003, 055→004, 056→005, 057→006, 077→007, 093→008). ADR-050 +(dynamic resource ownership) is an alkcall/alknet-core ADR, not +tty-specific, and is not ported into alktty's ADR range; the +access-control work that declares against the ADR-050 model is +described in `tty-adapter.md` and the ADR-001/002 ported docs, which +reference ADR-050 by its alknet number. The Phase 7 control-channel +split (`STREAM_CTRL_IN` = 3, `STREAM_CTRL_OUT` = 4) is an amendment +inside ADR-001, mirroring alknet (it was not a standalone ADR there +either). `AGENTS.md` was updated to match this mapping. ### Phase 5: Tests