Extract hero_terminal_lib crate (PTY wire format, ANSI scrubbers, terminal envelope) #88

New issue

Open

opened 2026-05-01 05:00:02 +00:00 by despiegk · 0 comments

despiegk commented

2026-05-01 05:00:02 +00:00

Owner

Background

Issue #71 added a design proposal at docs/proposal-shared-terminal-lib.md for sharing terminal-related code between hero_proc and hero_router. Discussion clarified the boundary:

Session management is hero_router-specific (naming, TerminalSession, CRUD, attached() registry, inject_*, terminal.* RPC). Stays put. Not extracted.
The reusable surface is everything below the session layer — how a client talks to the hero_proc PTY backend and how it processes the screen output. That is duplicated today and worth sharing.

This issue tracks the actual extraction.

Crate location

New crate crates/hero_terminal_lib/ inside the hero_proc workspace (not hero_router as the original proposal suggested). Rationale: hero_proc owns the PTY contract — the wire format and output post-processing should live with the source of truth. hero_router consumes it as a downstream dependency.

This inverts the original proposal's direction; no follow-up CODEROOT plumbing is required because hero_router already depends on hero_proc (hero_proc_sdk is already a workspace dep — see hero_router/Cargo.toml:24).

Scope — three reusable modules

1. `ansi/` — output scrubbing (no behavior change)

Pure byte-level helpers for stripping the local-terminal CSI/DA query echoes from PTY output. Currently in hero_router/src/server/terminal.rs:

strip_local_terminal_queries(bytes: &mut Vec<u8>, rows: u16) -> Vec<Vec<u8>> (lines 222–311)
strip_da_plaintext_artifacts(bytes: &mut Vec<u8>) (lines 312–338)
pending_query_prefix_len(bytes: &[u8]) -> usize (lines 339–364)
mod strip_tests (lines 365–481) — moves verbatim

Zero IO, zero async, zero axum. Pure functions over &[u8]/&mut Vec<u8>.

2. `wire/` — hero_proc PTY WebSocket frame format

Define the protocol that hero_proc_server speaks on /api/jobs/{id}/pty and /api/services/{name}/pty as a typed enum, and ship encoders/decoders both ends share:

pub enum PtyFrame {
    Stdout(Vec<u8>),                  // server → client, binary
    Stdin(Vec<u8>),                   // client → server, binary
    Resize { cols: u16, rows: u16 },  // either direction, JSON text frame
}

impl PtyFrame {
    pub fn from_ws_text(s: &str) -> Result<Self, FrameError>;
    pub fn from_ws_binary(bytes: Vec<u8>, dir: Direction) -> Self;
    pub fn to_ws_text(&self) -> Option<String>;       // Some only for Resize
    pub fn to_ws_binary(&self) -> Option<Vec<u8>>;    // Some for Stdout/Stdin
}

Replaces:

The bespoke {\"resize\":{\"cols\":N,\"rows\":M}} JSON parsing in hero_proc_server/src/web.rs::handle_pty_ws (line 358).
The mirror parsing/emission of the same shape in hero_router/src/server/terminal.rs::pty_handler.

After this lands, both ends call PtyFrame::from_ws_* / to_ws_* instead of inlining JSON parsing. Wire bytes are unchanged — existing browsers connecting to either end keep working.

3. `event/` — typed `TerminalEvent` envelope (browser ↔ hero_router)

The hero_router-side envelope used between the browser xterm.js code and the pty_handler proxy. Lives in hero_router/static/js/terminal.js today, parsed inline on the Rust side:

pub enum TerminalEvent {
    UserInput { data: Vec<u8> },             // base64 on the wire
    TerminalReply { name: String, data: Vec<u8> },
    Resize { cols: u16, rows: u16 },
}

Rust gains a typed enum + serde glue. The matching JS encoder/decoder in terminal.js is left alone in this issue — only the Rust side adopts the type.

Useful when a second client (MCP shell tool, alternate UI) needs to speak the same envelope. If only hero_router ever uses it, this module is a small win at low cost; we get to keep the Stdin/Stdout layer (PtyFrame) clean.

Out of scope (explicitly)

Anything in the session-management layer: TerminalSession, ShellType, encode_job_name/decode_job_name, validate_name, shell_suffix, attached(), inject_reply, inject_resize, create_session/list_sessions/get_session/delete_session, the terminal.* RPC handlers. All hero_router-specific. Stay there.
Rewriting the browser xterm.js code in either repo.
Changing the PTY ownership model (PtyHandle, portable_pty supervisor — stay in hero_proc_server).
Any change to JobSummary, JobFilter, or backend RPC shape.

Implementation plan

Step 1 — Create the crate

New crates/hero_terminal_lib/ in the hero_proc workspace; add to root Cargo.toml [workspace] members.
Dependencies (workspace-pinned where possible): serde, serde_json, base64, thiserror. No axum, no tokio, no tokio-tungstenite. Pure data + algorithms.
Empty lib.rs re-exporting the three modules.

Step 2 — Move `ansi/` (no behavior change)

Copy strip_local_terminal_queries, strip_da_plaintext_artifacts, pending_query_prefix_len, and mod strip_tests into hero_terminal_lib::ansi.
Remove the originals from hero_router/src/server/terminal.rs; add use hero_terminal_lib::ansi::{strip_local_terminal_queries, strip_da_plaintext_artifacts, pending_query_prefix_len};.
cargo test -p hero_terminal_lib runs the moved strip_tests. cargo build -p hero_router succeeds with the new dep wired in.

Step 3 — Define `wire::PtyFrame` and adopt on both ends

New hero_terminal_lib::wire::{PtyFrame, FrameError, Direction} with the encode/decode methods listed above.
hero_proc_server/src/web.rs::handle_pty_ws (line 318): replace the inline Message::Binary / Message::Text arms with PtyFrame::from_ws_binary / from_ws_text. The match-on-PtyFrame arms produce the same master.write / master.resize calls as today.
hero_router/src/server/terminal.rs::pty_handler: same swap, on the side that proxies bytes to/from hero_proc.
Unit tests in hero_terminal_lib: round-trip every variant; reject malformed JSON; tolerate unknown JSON keys (forward-compat).

Step 4 — Define `event::TerminalEvent`

New hero_terminal_lib::event::{TerminalEvent, EventError} with serde derives matching the existing JSON shape produced/consumed in terminal.js.
hero_router/src/server/terminal.rs::pty_handler: where it currently parses/builds the envelope inline, switch to serde_json::from_str::<TerminalEvent> / serde_json::to_string.
Unit tests: parse fixtures captured from the current JS sender; assert structural equality after round-trip.

Step 5 — Verification

cargo test --workspace green in both repos.
Manual smoke test in hero_router: open the Terminal UI, start a session, type, resize, detach, reattach. No regression vs. pre-extraction behavior.
Manual smoke test in hero_proc: open the Terminal tab on the admin dashboard (post-#71 with the TTY filter), attach to a TTY job, type, resize. No regression.

Step 6 — Update the proposal doc

docs/proposal-shared-terminal-lib.md gets a "Status: implemented in #" header and is shortened to a one-paragraph pointer to the crate's README.

Acceptance criteria

crates/hero_terminal_lib/ exists in the hero_proc workspace with the three modules ansi, wire, event.
Crate has zero axum, zero tokio, zero tokio-tungstenite dependencies.
hero_router consumes the crate via a normal git dep on hero_proc (added to hero_router/Cargo.toml [workspace.dependencies]).
hero_router/src/server/terminal.rs no longer contains the strip_* / pending_query_prefix_len definitions or the strip_tests module — those use sites import from hero_terminal_lib::ansi.
hero_proc_server::web::handle_pty_ws parses inbound frames via PtyFrame::from_ws_* (no inline serde_json::from_str for the resize shape).
hero_router::server::terminal::pty_handler uses both PtyFrame (for the hero_proc side) and TerminalEvent (for the browser side).
cargo test --workspace passes in both repos. Pre-existing failures from #71 (harness::tests::test_harness_starts_and_stops, ready::tests::declare_creates_marker, the 5 doc-test compile errors) are unchanged.
Wire bytes on the WebSocket are byte-identical to pre-extraction. No client-side change required.
docs/proposal-shared-terminal-lib.md updated to reflect the implemented state.

Notes

The crate is named hero_terminal_lib (matching the _lib suffix already used by hero_proc_lib).
We deliberately do NOT bundle the resize JSON shape change with a protocol upgrade. If someone later wants typed JSON for resize (e.g. a {\"v\":1,\"kind\":\"resize\",...} envelope), that's a follow-up — and PtyFrame makes it a single point to evolve.
This work depends on, but is not blocked by, #71. The TTY-filter UI change shipped in #71 is independent.

## Background Issue #71 added a design proposal at `docs/proposal-shared-terminal-lib.md` for sharing terminal-related code between `hero_proc` and `hero_router`. Discussion clarified the boundary: - **Session management is hero_router-specific** (naming, `TerminalSession`, CRUD, `attached()` registry, `inject_*`, `terminal.*` RPC). Stays put. Not extracted. - **The reusable surface is everything *below* the session layer** — how a client talks to the hero_proc PTY backend and how it processes the screen output. That is duplicated today and worth sharing. This issue tracks the actual extraction. ## Crate location New crate `crates/hero_terminal_lib/` inside the **hero_proc** workspace (not hero_router as the original proposal suggested). Rationale: hero_proc owns the PTY contract — the wire format and output post-processing should live with the source of truth. `hero_router` consumes it as a downstream dependency. This inverts the original proposal's direction; no follow-up CODEROOT plumbing is required because hero_router already depends on hero_proc (`hero_proc_sdk` is already a workspace dep — see `hero_router/Cargo.toml:24`). ## Scope — three reusable modules ### 1. `ansi/` — output scrubbing (no behavior change) Pure byte-level helpers for stripping the local-terminal CSI/DA query echoes from PTY output. Currently in `hero_router/src/server/terminal.rs`: - `strip_local_terminal_queries(bytes: &mut Vec<u8>, rows: u16) -> Vec<Vec<u8>>` (lines 222–311) - `strip_da_plaintext_artifacts(bytes: &mut Vec<u8>)` (lines 312–338) - `pending_query_prefix_len(bytes: &[u8]) -> usize` (lines 339–364) - `mod strip_tests` (lines 365–481) — moves verbatim Zero IO, zero async, zero axum. Pure functions over `&[u8]`/`&mut Vec<u8>`. ### 2. `wire/` — hero_proc PTY WebSocket frame format Define the protocol that **hero_proc_server speaks on `/api/jobs/{id}/pty` and `/api/services/{name}/pty`** as a typed enum, and ship encoders/decoders both ends share: ```rust pub enum PtyFrame { Stdout(Vec<u8>), // server → client, binary Stdin(Vec<u8>), // client → server, binary Resize { cols: u16, rows: u16 }, // either direction, JSON text frame } impl PtyFrame { pub fn from_ws_text(s: &str) -> Result<Self, FrameError>; pub fn from_ws_binary(bytes: Vec<u8>, dir: Direction) -> Self; pub fn to_ws_text(&self) -> Option<String>; // Some only for Resize pub fn to_ws_binary(&self) -> Option<Vec<u8>>; // Some for Stdout/Stdin } ``` Replaces: - The bespoke `{\"resize\":{\"cols\":N,\"rows\":M}}` JSON parsing in `hero_proc_server/src/web.rs::handle_pty_ws` (line 358). - The mirror parsing/emission of the same shape in `hero_router/src/server/terminal.rs::pty_handler`. After this lands, both ends call `PtyFrame::from_ws_*` / `to_ws_*` instead of inlining JSON parsing. Wire bytes are unchanged — existing browsers connecting to either end keep working. ### 3. `event/` — typed `TerminalEvent` envelope (browser ↔ hero_router) The hero_router-side envelope used between the browser xterm.js code and the `pty_handler` proxy. Lives in `hero_router/static/js/terminal.js` today, parsed inline on the Rust side: ```rust pub enum TerminalEvent { UserInput { data: Vec<u8> }, // base64 on the wire TerminalReply { name: String, data: Vec<u8> }, Resize { cols: u16, rows: u16 }, } ``` Rust gains a typed enum + serde glue. The matching JS encoder/decoder in `terminal.js` is left alone in this issue — only the Rust side adopts the type. Useful when a second client (MCP shell tool, alternate UI) needs to speak the same envelope. If only hero_router ever uses it, this module is a small win at low cost; we get to keep the `Stdin/Stdout` layer (`PtyFrame`) clean. ## Out of scope (explicitly) - Anything in the session-management layer: `TerminalSession`, `ShellType`, `encode_job_name`/`decode_job_name`, `validate_name`, `shell_suffix`, `attached()`, `inject_reply`, `inject_resize`, `create_session`/`list_sessions`/`get_session`/`delete_session`, the `terminal.*` RPC handlers. All hero_router-specific. Stay there. - Rewriting the browser xterm.js code in either repo. - Changing the PTY ownership model (`PtyHandle`, `portable_pty` supervisor — stay in `hero_proc_server`). - Any change to `JobSummary`, `JobFilter`, or backend RPC shape. ## Implementation plan ### Step 1 — Create the crate - New `crates/hero_terminal_lib/` in the hero_proc workspace; add to root `Cargo.toml` `[workspace] members`. - Dependencies (workspace-pinned where possible): `serde`, `serde_json`, `base64`, `thiserror`. **No** `axum`, **no** `tokio`, **no** `tokio-tungstenite`. Pure data + algorithms. - Empty `lib.rs` re-exporting the three modules. ### Step 2 — Move `ansi/` (no behavior change) - Copy `strip_local_terminal_queries`, `strip_da_plaintext_artifacts`, `pending_query_prefix_len`, and `mod strip_tests` into `hero_terminal_lib::ansi`. - Remove the originals from `hero_router/src/server/terminal.rs`; add `use hero_terminal_lib::ansi::{strip_local_terminal_queries, strip_da_plaintext_artifacts, pending_query_prefix_len};`. - `cargo test -p hero_terminal_lib` runs the moved `strip_tests`. `cargo build -p hero_router` succeeds with the new dep wired in. ### Step 3 — Define `wire::PtyFrame` and adopt on both ends - New `hero_terminal_lib::wire::{PtyFrame, FrameError, Direction}` with the encode/decode methods listed above. - `hero_proc_server/src/web.rs::handle_pty_ws` (line 318): replace the inline `Message::Binary` / `Message::Text` arms with `PtyFrame::from_ws_binary` / `from_ws_text`. The match-on-`PtyFrame` arms produce the same `master.write` / `master.resize` calls as today. - `hero_router/src/server/terminal.rs::pty_handler`: same swap, on the side that proxies bytes to/from hero_proc. - Unit tests in `hero_terminal_lib`: round-trip every variant; reject malformed JSON; tolerate unknown JSON keys (forward-compat). ### Step 4 — Define `event::TerminalEvent` - New `hero_terminal_lib::event::{TerminalEvent, EventError}` with `serde` derives matching the existing JSON shape produced/consumed in `terminal.js`. - `hero_router/src/server/terminal.rs::pty_handler`: where it currently parses/builds the envelope inline, switch to `serde_json::from_str::<TerminalEvent>` / `serde_json::to_string`. - Unit tests: parse fixtures captured from the current JS sender; assert structural equality after round-trip. ### Step 5 — Verification - `cargo test --workspace` green in both repos. - Manual smoke test in `hero_router`: open the Terminal UI, start a session, type, resize, detach, reattach. No regression vs. pre-extraction behavior. - Manual smoke test in `hero_proc`: open the Terminal tab on the admin dashboard (post-#71 with the TTY filter), attach to a TTY job, type, resize. No regression. ### Step 6 — Update the proposal doc - `docs/proposal-shared-terminal-lib.md` gets a "**Status:** implemented in #<this-issue>" header and is shortened to a one-paragraph pointer to the crate's README. ## Acceptance criteria - [ ] `crates/hero_terminal_lib/` exists in the hero_proc workspace with the three modules `ansi`, `wire`, `event`. - [ ] Crate has zero `axum`, zero `tokio`, zero `tokio-tungstenite` dependencies. - [ ] `hero_router` consumes the crate via a normal git dep on hero_proc (added to `hero_router/Cargo.toml [workspace.dependencies]`). - [ ] `hero_router/src/server/terminal.rs` no longer contains the `strip_*` / `pending_query_prefix_len` definitions or the `strip_tests` module — those use sites import from `hero_terminal_lib::ansi`. - [ ] `hero_proc_server::web::handle_pty_ws` parses inbound frames via `PtyFrame::from_ws_*` (no inline `serde_json::from_str` for the resize shape). - [ ] `hero_router::server::terminal::pty_handler` uses both `PtyFrame` (for the hero_proc side) and `TerminalEvent` (for the browser side). - [ ] `cargo test --workspace` passes in both repos. Pre-existing failures from #71 (`harness::tests::test_harness_starts_and_stops`, `ready::tests::declare_creates_marker`, the 5 doc-test compile errors) are unchanged. - [ ] Wire bytes on the WebSocket are byte-identical to pre-extraction. No client-side change required. - [ ] `docs/proposal-shared-terminal-lib.md` updated to reflect the implemented state. ## Notes - The crate is named `hero_terminal_lib` (matching the `_lib` suffix already used by `hero_proc_lib`). - We deliberately do NOT bundle the resize JSON shape change with a protocol upgrade. If someone later wants typed JSON for resize (e.g. a `{\"v\":1,\"kind\":\"resize\",...}` envelope), that's a follow-up — and `PtyFrame` makes it a single point to evolve. - This work depends on, but is not blocked by, #71. The TTY-filter UI change shipped in #71 is independent.