diff --git a/docs/architecture/decisions/004-exit-code-on-control-chunk.md b/docs/architecture/decisions/004-exit-code-on-control-chunk.md index b06ed18..429b418 100644 --- a/docs/architecture/decisions/004-exit-code-on-control-chunk.md +++ b/docs/architecture/decisions/004-exit-code-on-control-chunk.md @@ -3,10 +3,10 @@ ## Status Accepted (ported from alknet ADR-055 2026-08-17; cross-references -renumbered to alktty's ADR range — ADR-052→001, ADR-053→002, +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 +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.) @@ -16,11 +16,11 @@ authoritative for any ADR not yet ported.) 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 +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 +`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 @@ -28,7 +28,7 @@ chunks. The exit code must ride on the chunk format itself. Two options: 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 + 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. @@ -45,7 +45,7 @@ the drainer writes it to the client before the writer closes. 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 +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 @@ -65,13 +65,13 @@ not data (process output). It rides on the control channel The `code` is an `i32` in this crate's exit-status convention: `ExitStatus::code()` on normal exit; on Unix, the negative signal -number on signal-terminated exit (e.g., `-9` for SIGKILL) — the same +number on signal-terminated exit (e.g., `-9` for SIGKILL), which is the same convention as Python's `subprocess.Popen.returncode`. It is **not** `ExitStatus::code()`'s behavior, which returns `None` for signal termination. Both local backends encode it: the pipe path via `ExitStatusExt::signal()` (`src/local/pipe.rs::exit_code_from`), the PTY path via a direct `libc::waitpid` reap decoded to `WEXITSTATUS` / -`-WTERMSIG` (`src/local/pty.rs::wait_exit_code`) — portable-pty 0.9.0's +`-WTERMSIG` (`src/local/pty.rs::wait_exit_code`); portable-pty 0.9.0's `ExitStatus` would flatten a signal death to `code = 1` with the signal only as a strsignal name string, so the waiter reaps by pid and skips the conversion (review #004). The chunk is the last control chunk @@ -94,7 +94,7 @@ session end. The exit chunk is the deterministic completion signal. 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 +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. @@ -116,14 +116,14 @@ 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., +`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 +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". @@ -144,30 +144,30 @@ the completion notification. 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 + 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 + reliable even when the backend can't determine the real code, so the client still knows the session ended. **Negative:** -- The "exit chunk is last" invariant is a one-way door — clients depend +- 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 + 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` sentinel collides with a real SIGHUP termination on the Unix pipe path: signal-terminated children are encoded `-N`, so - SIGHUP (signal 1) encodes as exactly `-1` — the same value the + SIGHUP (signal 1) encodes as exactly `-1`, the same value the adapter sends for a backend wait failure. A client can't distinguish a real SIGHUP termination from a backend wait failure from the code alone. The collision is narrow in practice (the usual @@ -175,7 +175,7 @@ the completion notification. `Signal { name: "HUP" }` control chunk, which the client knows it sent), but it is real for any other SIGHUP source. Note `std::process::ExitStatus::code()` cannot itself cause the - collision — it returns `None` for signal termination, so this + collision (it returns `None` for signal termination), so this conflation exists only through this crate's `-N` encoding. If distinguishing becomes load-bearing, a future control message type (`{"type":"exit_error","message":"..."}`) can carry the distinction @@ -214,16 +214,16 @@ additive. ## References -- [ADR-001](001-wire-format-and-two-carriage.md) — the wire format +- [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 +- [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 +- [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 + on cancel, since 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 diff --git a/docs/architecture/decisions/006-negotiation-framing-self-contained.md b/docs/architecture/decisions/006-negotiation-framing-self-contained.md index c612c98..d12625e 100644 --- a/docs/architecture/decisions/006-negotiation-framing-self-contained.md +++ b/docs/architecture/decisions/006-negotiation-framing-self-contained.md @@ -3,14 +3,14 @@ ## Status Accepted (ported from alknet ADR-057 2026-08-17; the dependency-edge -decision carries over unchanged — alktty depends on alkcall (for the +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, +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 +range; it is an alknet-core ADR. The alknet originals at `/workspace/@alkdev/alknet/docs/architecture/decisions/` remain authoritative.) @@ -18,7 +18,7 @@ authoritative.) 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 +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). @@ -39,7 +39,7 @@ pub async fn read_frame(&mut self) -> Result { 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 +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. @@ -53,13 +53,13 @@ comparison): 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, + 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 + 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. @@ -74,7 +74,7 @@ dependency edge is: ``` alktty └── alkcall (ProtocolHandler, Connection, AuthContext, Identity, - AccessControl, OwnershipProvider — alknet ADR-050) + AccessControl, OwnershipProvider; alknet ADR-050) ``` No alkcall-internal-wire-types dependency. The crate depends on alkcall @@ -99,10 +99,10 @@ 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 +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 +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). @@ -126,7 +126,7 @@ yet exist). 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 + `OperationSpec`/`Handler`/`OperationAdapter` types: actual type reuse, not framing glue. alktty does not need that exception.) **Negative:** @@ -164,7 +164,7 @@ wire-format change), but the dependency edge is one-way. 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 + different prefix width), alktty's is unaffected; they are independent implementations that happen to share a format today. ## References @@ -175,12 +175,12 @@ wire-format change), but the dependency edge is one-way. - [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 +- [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 +- `src/negotiation.rs`: the self-contained framing reader/writer this ADR commits (`NegotiationReader`, `NegotiationWriter`, `error_response_bytes`) -- alkcall's `FrameFramedReader`/`FrameFramedWriter` — the +- alkcall's `FrameFramedReader`/`FrameFramedWriter`: the `EventEnvelope`-bound methods that are NOT reused (the entangled length-prefix-read + `EventEnvelope`-deserialize that motivated this ADR) diff --git a/docs/architecture/decisions/007-tty-inside-channels.md b/docs/architecture/decisions/007-tty-inside-channels.md index af12209..b3113ce 100644 --- a/docs/architecture/decisions/007-tty-inside-channels.md +++ b/docs/architecture/decisions/007-tty-inside-channels.md @@ -1,16 +1,16 @@ -# ADR-007: TTY Inside Channels — Sub-Streams, Not Wire Format +# 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 +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 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 / +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.) @@ -28,14 +28,14 @@ modes differ only in *where the `BiStream` comes from* (a top-level 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 +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 +"run TTY's sub-demux on a channels-backed `BiStream`" (the same code +as direct mode, a 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 @@ -66,7 +66,7 @@ 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 +*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. @@ -80,21 +80,21 @@ 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 | +| **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 +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. +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: @@ -107,8 +107,8 @@ stream_type is unidirectional.** Control is now two halves: 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. +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 @@ -121,11 +121,11 @@ splits from 3 into 3+4). 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 + - `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= + - `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 @@ -135,7 +135,7 @@ splits from 3 into 3+4). 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 + 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`. @@ -147,16 +147,16 @@ splits from 3 into 3+4). 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. + 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 + (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 — + `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). @@ -166,19 +166,19 @@ splits from 3 into 3+4). 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 + (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) — +- **`TtyBackend` trait, `TtyHandle`, `TtyControl`** (ADR-002): unchanged. Backends don't know about channels or direct mode. -- **`DockerTtyBackend`, `LocalTtyBackend`** — unchanged. They implement +- **`DockerTtyBackend`, `LocalTtyBackend`**: unchanged. They implement `TtyBackend::allocate()` and return a `TtyHandle`. -- **`ControlMessage` enum** — unchanged. The JSON shape is the same in +- **`ControlMessage` enum**: unchanged. The JSON shape is the same in both modes. -- **The `alk/tty` ALPN string** — unchanged. Direct connections use it; +- **The `alk/tty` ALPN string**: unchanged. Direct connections use it; channels `channel/open` requests it. ### Crate dependency @@ -195,16 +195,16 @@ build; the feature-gated dependency is opt-in, same as `alknet-docker`'s ## Consequences **Positive:** -- The TTY crate's direct mode is unchanged — existing `alk/tty` +- 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 +- The channels path uses the channels layer's de-chunking, so there is + 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 `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 control channel and exit-chunk invariant carry forward cleanly: the `ControlMessage` enum and ordering semantics are mode-independent. @@ -220,17 +220,17 @@ build; the feature-gated dependency is opt-in, same as `alknet-docker`'s `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. + 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, +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. +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 — +**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 @@ -239,29 +239,29 @@ amended door-type discussion. ## References - **[ADR-008](008-channels-pure-channel-multiplexing.md)**: channels - pure channel multiplexing (reverses this ADR — TTY always uses its + 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 + (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 + 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 + 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**) + 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`) + 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 diff --git a/docs/architecture/tty-local.md b/docs/architecture/tty-local.md index 355ce5e..c544ede 100644 --- a/docs/architecture/tty-local.md +++ b/docs/architecture/tty-local.md @@ -6,10 +6,10 @@ status: draft (ported from alknet 2026-08-17; alknet-tty → alktty, last_updated: 2026-08-17 --- -# alktty — Local TTY Backend (`local` feature module) +# 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 +`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 @@ -24,12 +24,12 @@ it implements is in [tty-backend.md](tty-backend.md). (`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 +- **`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 + `None`, a 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 +- **`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 @@ -44,10 +44,10 @@ process group). 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, +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 +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. @@ -56,8 +56,8 @@ The PTY case is what makes a terminal a terminal: real resize (via (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. +(`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 @@ -65,7 +65,7 @@ The wrinkle that drove the Phase 0 POC: `portable_pty` is a **blocking `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 +records both as requirements, not open questions; the POC turned them into grounded requirements. ## Architecture @@ -89,15 +89,15 @@ pub enum StdinCmd { } ``` -1. **Reader thread** — blocking reads from `MasterPty::try_clone_reader()` +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 + (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 +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 @@ -105,10 +105,10 @@ pub enum StdinCmd { Unpin>` (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 wait → `oneshot::Sender` with the + 3. **Waiter thread**: blocking wait → `oneshot::Sender` with the exit code in ADR-004's encoding. On Unix the reap is `libc::waitpid(pid, ..)` on the direct child, decoded to - `WEXITSTATUS` (normal exit) or `-WTERMSIG` (signal-terminated exit — + `WEXITSTATUS` (normal exit) or `-WTERMSIG` (signal-terminated exit, the ADR-004 negative encoding): portable-pty 0.9.0's `ExitStatus` would flatten a signal-terminated child to `code = 1` with the signal only as a strsignal name string (review #004); reaping by pid avoids @@ -122,7 +122,7 @@ pub enum StdinCmd { REQ-TTY-01; ADR-004); its `Drop`-on-cancel kills the child (ADR-005). -`TtyHandle.stderr` is `None` (PTY backends merge stdout/stderr — kernel +`TtyHandle.stderr` is `None` (PTY backends merge stdout/stderr: a kernel PTY property, one output stream from the slave). `TtyHandle.control` is a `PtyControl` struct (the POC's concrete type; @@ -138,8 +138,8 @@ pub struct PtyControl { } ``` -`resize()` locks the master and calls `MasterPty::resize(PtySize)` — -non-blocking (it issues an `ioctl`). `signal()` — see REQ-TTY-02 below. +`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 @@ -151,10 +151,10 @@ 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)`), +`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)` +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: @@ -184,13 +184,13 @@ 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 +`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 +future's `Drop` (runs on cancel only; on resolve, the guard is disarmed) calls `ChildKiller::kill(SIGHUP)`: ```rust @@ -210,8 +210,8 @@ impl Drop for LocalExitFuture { ``` On cancel: the `Drop` kills the child (SIGHUP); the child exits; the -waiter thread reaps it (Unix: `libc::waitpid` — see "Waiter thread" -above) and exits (its `oneshot::send` fails silently — the receiver was +waiter thread reaps it (Unix: `libc::waitpid`; see "Waiter thread" +above) 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) after the kill; its SIGHUP death resolves to `-1` (the ADR-004 sentinel collision) on @@ -233,7 +233,7 @@ 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 +`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. @@ -241,24 +241,24 @@ ADR-005 for the contract and the trait-level rationale. `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 +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)` +(no PTY, so 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 +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. +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 @@ -282,13 +282,13 @@ local = ["dep:portable-pty", "dep:tokio-util", "tokio/process", "tokio/rt-multi- 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. +directly, with 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 +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 @@ -302,16 +302,16 @@ both the original alknet decision and the alktty consolidation. 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) +├── 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. +(no features) stays wasm-clean (see the crate root's `# WASM target` +doc comment). ## The Runner Pattern @@ -321,12 +321,12 @@ research identified. A coordinator sends a negotiation frame with 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. +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 +`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. @@ -354,18 +354,18 @@ building runner policy into alktty. 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 + 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` + 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 + `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 @@ -388,19 +388,19 @@ building runner policy into alktty. ## References -- [ADR-003](decisions/003-local-backend-placement.md) — the module +- [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 +- [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 +- [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) — +- [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`, +- `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 +- [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