feat(runtime-api): terminal byte stream (#34), stream resume + idempotent submit (#76), pet agent-count pin (#12)

[Hmbown]

Summary

Two Core unblocks from the tracker (codewhale-app#61), on one branch.

#34 — the Engine's terminal byte stream

The app-side ticket asks Core for "authenticated byte input/output, resize, exit and bounded replay" over the stateful terminal path, and says to link real Core work before claiming those paths. Two commits:

1. The owner gained the primitives (crates/tui/src/tools/terminal_session.rs). The pty master is now retained (it was dropped after the reader/writer clones were taken), so resize_session can reach the kernel's window; OutputBuffer::read_since reads from an absolute cursor without consuming and sets gap when the 512 KiB ring has moved past it (report, not repair); session_exit_status and kill_session make exit observable. take_output now reuses that same cursor arithmetic instead of its own copy.
2. The routes that consume them (crates/tui/src/runtime_api/terminal.rs), which is also what makes them live code — cargo clippy --all-targets failed the lib build on the primitives alone, correctly, and this PR is the consumer.

• GET /v1/terminal/{name}/output — wire-shaped like the jobs stream on purpose (cursor/max_bytes/format → offset/next_cursor/total/dropped). Reads never consume, so several clients can hold independent cursors.
• POST /v1/terminal/{name}/input — text or base64, bounded per frame. Input stays attributable by route: this is the client's writer, terminal_send is the agent's.
• POST /v1/terminal/{name}/resize and POST /v1/terminal/{name}/kill.
• Routes attach to shells the Engine already owns and never create one (404 otherwise) — an HTTP request must not conjure a shell the Engine does not know about.
• RuntimeCapabilities gains terminal_stream, terminal_input, terminal_resize, terminal_kill, set from cfg!(unix), so a client gates its pane on the flag rather than discovering the gap from a failed request. Windows answers 501 for all four (the owner is Unix-only), which keeps "this build cannot do terminals" distinguishable from "that session is gone".
• docs/RUNTIME_API.md documents the family in the GPUI section with its four stated limitations: no wait_ms long poll, no scrollback recovery, live sessions only (a restarted Engine reports no session rather than pretending to reattach), no runtime-sdk wrapper yet.

#12 — the pet's agent count

The ticket's premise ("no producer populates Activity.parallel") turned out to be stale: Core already emits it end to end (engine → event_loop.rs:1946 → metadata()'s allowlist → pet-native.js:1796 agent:${id} spans → the · ×N caption), and the JS half is already covered by pet/tests/pet-engine.test.mjs. So there is no producer to write — a second one would be a second authority for a count that already flows. What was missing is any test of the Rust half, which is the half that fails silently. This pins it, and the pin is proven to work: deleting Event::AgentSpawned from the allowlist fails it with agent spawns are observed. The app-side rendering half stays in codewhale-app.

#76 — resumable streaming and idempotent submission

Two commits, both Core-side.

Resume from Last-Event-ID. The durable half already existed (per-thread seq, a JSONL event log, since_seq replay with a bounded tail), but nothing on the wire could use it: every SSE frame was written without an id:, so a browser EventSource had nothing to resume from. Journal frames now carry their durable seq as the SSE id — journal frames only, since the stream.progress frames are transport progress and an id there would invite a client to resume from a point it never received. stream_thread_events reads Last-Event-ID as the cursor when no explicit since_seq was asked for; an explicit query cursor wins, so a deliberate replay-from-zero is never silently overridden by a stale header. An opaque id (proxy, older client) starts from the durable head instead of failing to open the stream. RuntimeCapabilities gains event_stream_resume so a client can gate its reconnect controls on it.

A replay is an acknowledgement, not an admission. POST /v1/threads/{id}/turns answered 201 whether it admitted a new turn or handed back the original for a repeated operation_key — so a client retrying after a dropped response could not tell whether it had created a second turn. The admission path now reports the disposition (start_turn_with_reporting_replay; the two replay returns say true, the tail says false), and the route answers 200 { ..., idempotent_replay: true } for a replay, keeping 201 for a fresh admission. The flag is omitted on admission, so existing responses stay byte-identical, and the 94 existing start_turn callers are untouched.

The existing turn_endpoint_operation_key_returns_original_and_conflicts_on_mismatch test asserted the old 201; it now pins the stronger contract, including 409 on a changed request with the same key, exactly one SendMessage, and exactly one turn.

Not in this PR: app-server minting an operation_key for its own submissions (client half, app lane), and the Windows ConPTY port for the terminal family.

Additional verification for these two commits:

• ... tests.rs thread_event_frames_carry → 1 test run: 1 passed (first frame's id: equals its payload seq; reconnect with only Last-Event-ID lands on the next durable event; ?since_seq=0 with a stale header still replays from zero) · last_event_id_accepts → 1 passed · ... tests.rs event → 170 passed
• ... tests.rs turn_endpoint_operation_key → 1 test run: 1 passed, plus the paths the signature change touches: turn_operation 5 passed · start_turn 5 passed · agent_mail 6 passed · thread_goal 5 passed · steer 33 passed
• clippy --all-targets -D warnings exit 0 · fmt exit 0 · both ratchets PASS

Unreding main (same branch, no extra PR)

main was red on two jobs, and neither is visible in a PR rollup — check-runtime-contract-budget is advisory on pull requests and fatal on push, and Test (macos-latest) is not one of the three required checks.

Lint — an unrecorded identity change. execute_tools now enters every catalog outside Plan (tool_catalog.rs:350, code-mode Phase 1 e23ce514c), moving the Act and Operate full tool names and identity digests; and the agent tool advertises its cwd parameter, growing the shared active surface by 256 schema bytes / 64 estimated tokens everywhere that tool appears (including Plan full, which is why it grew without gaining a tool). The checker refuses identity drift and --update cannot bypass it, so this is the explicit maintainer edit the file's header asks for: identities re-pinned, 14 ceilings raised to the measured values, and both facts recorded in the file's _comment history. check-runtime-contract-budget.py → PASS: all 55 metrics are exactly at budget.

Test (macos-latest) — a real race, proven fixed. threads_running_lists_active_turns_and_clears_on_settle forced a synthetic settle into the durable store, but the engine still owns that record and can persist afterwards; the listing is read from the store, so a later engine write puts the turn back in flight and the single read fails. The assertion now polls until the settle wins (with a ci_scaled(5s) deadline, so a genuinely stuck turn still fails).

Proven rather than argued: with the record flipped back to InProgress after the test's write and settled 300ms later, the old single read fails with the CI shape verbatim (left: [... "status": "in_progress"], right: Array []) while the polling version passes the identical injection (0.549s — it waited for the settle). The injection was reverted; the commit holds only the fix.

Testing

• scripts/dev-test.sh crates/tui/src/runtime_api/tests.rs terminal_routes_serve → Summary [0.208s] 1 test run: 1 passed — the full seam over HTTP against a real PTY: input through the route executes in the Engine's own session, output comes back through the route at an absolute cursor, a repeat read at the same cursor returns identical bytes, resize is confirmed by the shell's own stty size reporting 40 100 (a handler that merely stored the numbers would fail this), and kill is observed as running: false.
• ... terminal_output_for_an_unknown → 1 test run: 1 passed; ... terminal_capabilities → 1 test run: 1 passed.
• ... crates/tui/src/tools/terminal_session.rs terminal → 360 tests run: 360 passed, 12543 skipped.
• ... crates/protocol/src/runtime/mod.rs runtime:: → 10 tests run: 10 passed.
• cargo clippy -p codewhale-tui -p codewhale-protocol --all-targets --locked -- -D warnings -A clippy::uninlined_format_args -A clippy::too_many_arguments -A clippy::unnecessary_map_or exit 0 · cargo fmt --all -- --check exit 0
• check-blocking-calls-budget.py → 601 sites across 177 files, within budget · check-dead-code-budget.py → PASS: 185 attributes, exactly at budget.

Not verified: Windows. The owner is #[cfg(unix)] end to end and every test here is cfg(all(test, unix)); ConPTY qualification is its own slice, as the ticket says.

Checklist

• No new layer replacing an existing one: this is the existing terminal owner plus routes over it
• Docs updated (docs/RUNTIME_API.md, and the limitations are stated in the module)
• Tests added where they protect behaviour that fails silently
• TUI manual check: n/a (no TUI surface changed)

No-Issue: Core half of tracker codewhale-app#61; #34 and #12 live in the app repo, so there is no issue in this repo to close.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[codewhale-agent]

Codewhale review

Complete review coverage: 1/1 passes, 7 file patches, sha256:c6e5419426afb13b4d85b0c7d3407136036a0868693778347ba2cd19a36b95f8.

Pass 1: The PR adds Engine-owned terminal byte-stream routes (output/input/resize/kill) over the existing PTY owner plus new primitives in terminal_session.rs (retained master, absolute non-consuming reads, resize, exit status, kill), advertises four new capabilities, documents them, and pins the pet agent-event projection. The unix paths look coherent: take_output is refactored onto the same cursor arithmetic and appears behaviour-preserving, reads are genuinely non-consuming, and input is bounded. The defects are (a) a non-unix build break, (b) a documented input default that the implementation contradicts, (c) a running field whose documented meaning is stronger than what the code computes, and (d) a capability test that is not unix-gated.

Findings

• [ERROR] Non-unix build of runtime_api::terminal cannot resolve the unix-only terminal_session::READ_LIMIT (crates/tui/src/runtime_api/terminal.rs:152)
  crates/tui/src/runtime_api/terminal.rs is compiled on every target — mod terminal; in crates/tui/src/runtime_api.rs is not cfg-gated — but READ_LIMIT is declared #[cfg(unix)] pub(crate) const READ_LIMIT: usize = 64 * 1024; (crates/tui/src/tools/terminal_session.rs:42). bounded_max_bytes (terminal.rs:150) is itself not unix-gated and references that path twice (terminal.rs:152 and :155), so on a not(unix) target the two path references cannot be resolved (name-not-found at those lines): cargo check --target x86_64-pc-windows-msvc fails before any of the 501 stubs in mod platform is reached. That directly contradicts the PR's Windows story ('the Windows routes answer 501') and the cfg!(unix) capability flags, which only make sense if the crate builds on Windows. The five helpers touched by this compile unit (chunk_encoding, encode_bytes, decode_bytes, bounded_max_bytes, bounded_dimension) are used only by the unix handlers and by #[cfg(all(test, unix))] tests, so they are dead code on non-unix even after the path resolves. The reported cargo clippy --all-targets -D warnings run was executed on a unix host, so the entire #[cfg(not(unix))] module and this reference were never compiled and the failure could not have been observed. Fix by making the limit available on all targets (a #[cfg(not(unix))] READ_LIMIT, or a cfg-free shared constant) or by gating the unix-only helpers so the non-unix module is self-contained.
• [WARNING] running is documented as 'exited and drained' but is computed from try_wait alone, so a client can drop the shell's last bytes (crates/tui/src/runtime_api/terminal.rs:77)
  The response field doc says /// False once the shell has exited and no bytes remain past next_cursor., and docs/RUNTIME_API.md tells clients to learn the exit from output (running / exit_code). The implementation is let running = exit.is_none(); (terminal.rs:210), derived purely from child.try_wait(). Reaping the child is independent of the reader thread that appends into the ring, so there is a real window in which a poll returns running: false while next_cursor < total (the shell's final line has not been appended yet). A client that stops polling on running: false — exactly what the field doc says is safe — silently truncates the stream; this is the scenario the feature exists to serve. Note that the sibling jobs route expresses the same state as done = 'a terminal status and nothing left past the cursor' (docs/RUNTIME_API.md:1266), and the inline comment at terminal.rs:219-220 also contradicts the field doc ('running alone must not imply there is nothing behind us'), so the two comments in this new file disagree. Minimal fix: either reword the field to say the shell has exited and instruct clients to drain until next_cursor == total, or derive the flag from both signals (finished only when the child has exited and chunk.next_cursor == chunk.total).
• [WARNING] Terminal input's documented default encoding is text, but the route decodes base64 by default (docs/RUNTIME_API.md:1233)
  docs/RUNTIME_API.md:1233-1234 states for POST /v1/terminal/{name}/input: { "data", "encoding"? }, UTF-8 text by default or base64 for exact bytes — the same default the pre-existing jobs stdin route documents (data is UTF-8 text by default or base64, docs/RUNTIME_API.md:1267-1269). The code does the opposite: terminal_input calls decode_bytes(&request.data, request.encoding.as_deref().unwrap_or("base64")) (terminal.rs:240), and the in-code doc comment for the field (terminal.rs:86) says base64 is the default. A client that implements the documented contract and posts {"data":"ls\n"} with no encoding gets 400 data is not valid base64 instead of typing the command; only an explicit encoding: "text" works. Nothing in /v1/runtime/info lets a client detect the real default, so this is a wire-contract divergence, not a cosmetic doc nit. Decide the intended default and make code and docs agree (the jobs stdin precedent argues for text); a regression test posting the documented body shape would pin it.
• [WARNING] runtime_info_advertises_terminal_capabilities asserts the four flags are true on every platform, but the server sets them from cfg!(unix) (crates/tui/src/runtime_api/tests.rs:13760)
  The new test (crates/tui/src/runtime_api/tests.rs:13738) hard-asserts assert_eq!(info["capabilities"][capability], true, ...) for terminal_stream/terminal_input/terminal_resize/terminal_kill, while default_runtime_capabilities() sets all four from cfg!(unix). On a Windows build the server truthfully answers false (and the routes answer 501), so the assertion fails there — i.e. once the non-unix build compiles (see the READ_LIMIT issue), the Windows test run is red for a reason that has nothing to do with the code under test. The sibling test added in the same PR, terminal_routes_serve_a_live_engine_session_over_http, is correctly marked #[cfg(unix)]. Fix by asserting cfg!(unix) instead of the literal true (which keeps the test meaningful on both platforms), or by gating the test with #[cfg(unix)].

Suggestions

• crates/tui/src/runtime_api/terminal.rs:152 — bounded_max_bytes must not be the only unconditional consumer of the #[cfg(unix)] terminal_session::READ_LIMIT. Either declare the limit for all targets (e.g. a #[cfg(not(unix))] pub(crate) const READ_LIMIT: usize = 64 * 1024; next to the unix one in crates/tui/src/tools/terminal_session.rs:42, or hoist it to a cfg-free location), or mark the unix-only request-validation helpers (chunk_encoding, encode_bytes, decode_bytes, bounded_max_bytes, bounded_dimension) #[cfg(unix)] so the not(unix) module needs none of them. Verify with a Windows-target cargo check/clippy --all-targets -D warnings, which the reported Linux run never exercises.
• docs/RUNTIME_API.md:1233 — Make the documented default match the implementation. If base64 really is the intended default for POST /v1/terminal/{name}/input, say so here (and note that it differs from the jobs stdin route, which defaults to text); if text is intended, change the unwrap_or("base64") in terminal_input (crates/tui/src/runtime_api/terminal.rs:240) to unwrap_or("text") and keep the docs and the in-code field comment consistent.
• crates/tui/src/runtime_api/tests.rs:13760 — Replace the literal true in info["capabilities"][capability], true, with cfg!(unix) so the test asserts the flag matches the build rather than hard-coding the unix answer; alternatively mark the whole test #[cfg(unix)] the way its sibling terminal_routes_serve_a_live_engine_session_over_http already is. Run rustfmt after the edit.

Assessment

Pass 1: Static review only: no build, test or runtime check was executed here, and the reported clippy/fmt runs were on a unix host. On the unix paths the design holds up — the take_output refactor onto OutputBuffer::read_since(cursor, usize::MAX) is behaviour-preserving (same clamped start, reads to the end, still sets read_cursor = total), the route-level reads are non-consuming and independent per cursor, input is bounded before it reaches the PTY, and resize/kill are genuinely delegated to the kernel/child. The serious gap is that the change was only ever compiled for unix: #[cfg(not(unix))] mod platform, and the unconditional terminal_session::READ_LIMIT reference outside it, mean the Windows build described in the PR text is unverified at best and does not compile as written. I could not inspect the projection (metadata) in crates/tui/src/tui/pet_watch/mod.rs, so the exact-JSON expectations of the new pet test, including its worker_status/id keys, remain unverified from the supplied excerpts. One further open question: the new route calls child.try_wait(), which reaps the child; whether the existing cancel/completion path in terminal_session.rs (command marker, CANCEL_CONFIRM_TIMEOUT, sentinel retries) ever calls child.wait() on the same session could not be checked from the excerpts — if it does, an HTTP output poll could make that later wait fail with ECHILD.

---

Advisory review by Codewhale (codewhale review --pr 6361 --post, head ba2d23f8e97fb76da74da3b495bfadf912b59230). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.

[codewhale-agent]

[ERROR] Non-unix build of runtime_api::terminal cannot resolve the unix-only terminal_session::READ_LIMIT

crates/tui/src/runtime_api/terminal.rs is compiled on every target — mod terminal; in crates/tui/src/runtime_api.rs is not cfg-gated — but READ_LIMIT is declared #[cfg(unix)] pub(crate) const READ_LIMIT: usize = 64 * 1024; (crates/tui/src/tools/terminal_session.rs:42). bounded_max_bytes (terminal.rs:150) is itself not unix-gated and references that path twice (terminal.rs:152 and :155), so on a not(unix) target the two path references cannot be resolved (name-not-found at those lines): cargo check --target x86_64-pc-windows-msvc fails before any of the 501 stubs in mod platform is reached. That directly contradicts the PR's Windows story ('the Windows routes answer 501') and the cfg!(unix) capability flags, which only make sense if the crate builds on Windows. The five helpers touched by this compile unit (chunk_encoding, encode_bytes, decode_bytes, bounded_max_bytes, bounded_dimension) are used only by the unix handlers and by #[cfg(all(test, unix))] tests, so they are dead code on non-unix even after the path resolves. The reported cargo clippy --all-targets -D warnings run was executed on a unix host, so the entire #[cfg(not(unix))] module and this reference were never compiled and the failure could not have been observed. Fix by making the limit available on all targets (a #[cfg(not(unix))] READ_LIMIT, or a cfg-free shared constant) or by gating the unix-only helpers so the non-unix module is self-contained.

[codewhale-agent]

[WARNING] running is documented as 'exited and drained' but is computed from try_wait alone, so a client can drop the shell's last bytes

The response field doc says /// False once the shell has exited and no bytes remain past next_cursor., and docs/RUNTIME_API.md tells clients to learn the exit from output (running / exit_code). The implementation is let running = exit.is_none(); (terminal.rs:210), derived purely from child.try_wait(). Reaping the child is independent of the reader thread that appends into the ring, so there is a real window in which a poll returns running: false while next_cursor < total (the shell's final line has not been appended yet). A client that stops polling on running: false — exactly what the field doc says is safe — silently truncates the stream; this is the scenario the feature exists to serve. Note that the sibling jobs route expresses the same state as done = 'a terminal status and nothing left past the cursor' (docs/RUNTIME_API.md:1266), and the inline comment at terminal.rs:219-220 also contradicts the field doc ('running alone must not imply there is nothing behind us'), so the two comments in this new file disagree. Minimal fix: either reword the field to say the shell has exited and instruct clients to drain until next_cursor == total, or derive the flag from both signals (finished only when the child has exited and chunk.next_cursor == chunk.total).

[codewhale-agent]

[WARNING] Terminal input's documented default encoding is text, but the route decodes base64 by default

docs/RUNTIME_API.md:1233-1234 states for POST /v1/terminal/{name}/input: { "data", "encoding"? }, UTF-8 text by default or base64 for exact bytes — the same default the pre-existing jobs stdin route documents (data is UTF-8 text by default or base64, docs/RUNTIME_API.md:1267-1269). The code does the opposite: terminal_input calls decode_bytes(&request.data, request.encoding.as_deref().unwrap_or("base64")) (terminal.rs:240), and the in-code doc comment for the field (terminal.rs:86) says base64 is the default. A client that implements the documented contract and posts {"data":"ls\n"} with no encoding gets 400 data is not valid base64 instead of typing the command; only an explicit encoding: "text" works. Nothing in /v1/runtime/info lets a client detect the real default, so this is a wire-contract divergence, not a cosmetic doc nit. Decide the intended default and make code and docs agree (the jobs stdin precedent argues for text); a regression test posting the documented body shape would pin it.

[codewhale-agent]

[WARNING] runtime_info_advertises_terminal_capabilities asserts the four flags are true on every platform, but the server sets them from cfg!(unix)

The new test (crates/tui/src/runtime_api/tests.rs:13738) hard-asserts assert_eq!(info["capabilities"][capability], true, ...) for terminal_stream/terminal_input/terminal_resize/terminal_kill, while default_runtime_capabilities() sets all four from cfg!(unix). On a Windows build the server truthfully answers false (and the routes answer 501), so the assertion fails there — i.e. once the non-unix build compiles (see the READ_LIMIT issue), the Windows test run is red for a reason that has nothing to do with the code under test. The sibling test added in the same PR, terminal_routes_serve_a_live_engine_session_over_http, is correctly marked #[cfg(unix)]. Fix by asserting cfg!(unix) instead of the literal true (which keeps the test meaningful on both platforms), or by gating the test with #[cfg(unix)].

[codewhale-agent]

bounded_max_bytes must not be the only unconditional consumer of the #[cfg(unix)] terminal_session::READ_LIMIT. Either declare the limit for all targets (e.g. a #[cfg(not(unix))] pub(crate) const READ_LIMIT: usize = 64 * 1024; next to the unix one in crates/tui/src/tools/terminal_session.rs:42, or hoist it to a cfg-free location), or mark the unix-only request-validation helpers (chunk_encoding, encode_bytes, decode_bytes, bounded_max_bytes, bounded_dimension) #[cfg(unix)] so the not(unix) module needs none of them. Verify with a Windows-target cargo check/clippy --all-targets -D warnings, which the reported Linux run never exercises.

[codewhale-agent]

Make the documented default match the implementation. If base64 really is the intended default for POST /v1/terminal/{name}/input, say so here (and note that it differs from the jobs stdin route, which defaults to text); if text is intended, change the unwrap_or("base64") in terminal_input (crates/tui/src/runtime_api/terminal.rs:240) to unwrap_or("text") and keep the docs and the in-code field comment consistent.

[codewhale-agent]

Replace the literal true in info["capabilities"][capability], true, with cfg!(unix) so the test asserts the flag matches the build rather than hard-coding the unix answer; alternatively mark the whole test #[cfg(unix)] the way its sibling terminal_routes_serve_a_live_engine_session_over_http already is. Run rustfmt after the edit.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[codewhale-agent]

Codewhale review

Complete review coverage: 1/1 passes, 12 file patches, sha256:fcc05990dfe456e6cc25f892d61a988f41ed2990350ee784d8423a72d6ee47bd.

Pass 1: Reviewed the 12-file diff: the new /v1/terminal byte-stream family (owner primitives + routes + capabilities), the SSE Last-Event-ID resume and idempotent-submit reporting in runtime_api.rs/runtime_threads.rs, and the new sleep guard. No error-level defect was found in the code paths I could inspect. I found one documentation contract that would break a client that follows it, one field-level contract mismatch in the new terminal response, and one non-unix lint issue. Three invariants sit outside the supplied diff (live-phase dedup base, the cfg(not(unix)) handler bodies, portable_pty error semantics) and I could not verify them.

Findings

• [WARNING] RUNTIME_API.md documents the wrong default encoding for POST /v1/terminal/{name}/input (docs/RUNTIME_API.md:1233)
  The new public doc line says the input route takes "UTF-8 text by default or base64 for exact bytes", but the handler decodes with request.encoding.as_deref().unwrap_or("base64") (crates/tui/src/runtime_api/terminal.rs, terminal_input), and TerminalInputRequest::encoding's own doc comment says "base64 (default, exact bytes)". A client that follows the published contract and POSTs {"data":"ls -la\n"} with no encoding field never reaches the shell: decode_bytes runs the base64 decoder and the request fails with 400 "data is not valid base64"; if the text happens to be a valid base64 string it is silently typed as the decoded bytes instead of the text, which is worse than an error. Either the doc line or the code default must change (the sibling /v1/threads/{id}/jobs/{job_id}/stdin route documents and uses text-by-default, so a maintainer may prefer aligning the code instead of the doc).
• [INFO] running is documented as "no bytes remain" but is derived from the child's exit alone (crates/tui/src/runtime_api/terminal.rs:77)
  TerminalOutputResponse::running is documented as "False once the shell has exited and no bytes remain past next_cursor", but the handler sets running = exit.is_none() from session_exit_status (a try_wait on the child), and the inline comment at the construction site explicitly says the opposite: "running alone must not imply there is nothing behind us". The two statements in the same file contradict each other, and the field doc is the one the client reads. Because the reader thread appends asynchronously, the child can be observed as exited while its final output has not been appended yet — exactly the race the jobs stream closes with done. A client that implements its "stream finished" condition as running == false (as the doc invites) can stop polling and lose the shell's trailing bytes; the field must instead be read as "the child has exited" and the client must keep polling until next_cursor == total (and offset == cursor, i.e. no gap). Fixing the field doc (or adding a real drain flag computed against chunk.next_cursor == chunk.total) is the smallest change.
• [INFO] sleep_guard's std::process imports are unused on non-unix builds (crates/tui/src/sleep_guard.rs:30)
  use std::process::{Child, Command, Stdio}; is unconditional, but all three names are used only under #[cfg(unix)] (SleepGuard::child, spawn, and the macOS/Linux start_inhibitor), while the struct itself, hold() and the module are compiled on Windows too (the module has a #[cfg(not(unix))] { Self {} } branch). On a cfg(not(unix)) build every one of the three imports is unused, i.e. an unused_imports warning that becomes a build failure under the -D warnings invocation this PR reports running. The same crate already gates exactly these sorts of imports (crates/tui/src/tools/terminal_session.rs puts #[cfg(unix)] on std::sync/std::time imports), so this looks like an omission rather than a policy. Fix: put #[cfg(unix)] on that use line; the Windows build then needs no imports at all.

Suggestions

• docs/RUNTIME_API.md:1233 — State the default the implementation actually uses (unwrap_or("base64")), so a client that omits encoding is not told to send raw UTF-8 that the route then rejects with 400 "data is not valid base64". If text-by-default is what was intended (to match the jobs stdin route), change the default in terminal_input instead and keep this line.

  - `POST /v1/terminal/{name}/input` — `{ "data", "encoding"? }`, `base64` by
    default for exact bytes, or `text` for UTF-8 → `{ "name", "written" }`
  

Assessment

Pass 1: No build, test or runtime check was executed for this review; everything below is source inspection of the supplied diff and the supplementary excerpts.

Strengths I could verify by reading: OutputBuffer::read_since is arithmetically equivalent to the cursor logic take_output used before (so the refactor is behaviour-preserving for the agent path) and cannot overflow next_cursor for any cursor/max_bytes combination, including a cursor ahead of the stream; both replay returns in start_turn_with_source set the replay flag and only the fresh-admission tail clears it, so the 200/201 split matches the flag; last_event_id only accepts a decimal and treats an opaque id as "no cursor", and query.since_seq.or_else(header) gives the query cursor precedence as documented.

Open questions I could not settle from the material given (none of these is asserted as a defect):

1. The call site that hands the broadcast receiver to replay_live_thread_events is not in the diff, so I cannot confirm that the live-phase dedup base now follows the header-derived cursor. If that function dedups against query.since_seq rather than against the last seq it actually emitted during replay, a Last-Event-ID reconnect could re-deliver events that were already emitted in the replay; the added test only inspects the first frame, so it would not catch that. The neighbouring event_handoff_replays_and_dedupes_interaction_prompts_without_a_gap test covers the handoff window for the query-cursor path.
2. The #[cfg(not(unix))] platform module (and the pub(super) use platform::… re-export) is not compiled by any Linux clippy/cargo check run, so nothing in this PR's verification touches those four bodies — ApiError::not_implemented's existence and the module's signatures are unverified here. The same is true of the three #[cfg(unix)]-only imports in crates/tui/src/runtime_api/terminal.rs (base64::Engine as _, crate::tools::terminal_session), which I deliberately did not report because a use super::* glob in the platform module makes the lint outcome ambiguous.
3. Whether POST /v1/terminal/{name}/kill is idempotent depends on portable_pty's unix Child::kill. std::process::Child::kill returns InvalidInput once the child has been reaped (try_wait caches the status), and the output route calls try_wait on every poll — so a kill issued after a client has observed running: false may surface as a 500 rather than a no-op. If portable_pty propagates that error, the route should treat an already-exited child as success (killed: false or 200); the wrapper source is outside the diff, so I did not raise it as a finding.
4. The derive line for StartTurnResponse is outside the supplied diff. If it includes Deserialize, the new idempotent_replay field needs #[serde(default)] as well as skip_serializing_if, since admission responses omit it; the tests in this crate parse the body as serde_json::Value, so they would not reveal that.

Note also that the 64 KiB decoded input bound (TERMINAL_INPUT_MAX_BYTES) needs a router body limit well above ~88 KiB for base64 frames; the value behind DefaultBodyLimit in build_router is not in the diff, so the advertised bound may or may not be reachable.

---

Advisory review by Codewhale (codewhale review --pr 6361 --post, head 919d60232b5498abcb23fe7ab213ee984c102127). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.

[codewhale-agent]

[WARNING] RUNTIME_API.md documents the wrong default encoding for POST /v1/terminal/{name}/input

The new public doc line says the input route takes "UTF-8 text by default or base64 for exact bytes", but the handler decodes with request.encoding.as_deref().unwrap_or("base64") (crates/tui/src/runtime_api/terminal.rs, terminal_input), and TerminalInputRequest::encoding's own doc comment says "base64 (default, exact bytes)". A client that follows the published contract and POSTs {"data":"ls -la\n"} with no encoding field never reaches the shell: decode_bytes runs the base64 decoder and the request fails with 400 "data is not valid base64"; if the text happens to be a valid base64 string it is silently typed as the decoded bytes instead of the text, which is worse than an error. Either the doc line or the code default must change (the sibling /v1/threads/{id}/jobs/{job_id}/stdin route documents and uses text-by-default, so a maintainer may prefer aligning the code instead of the doc).

[codewhale-agent]

[INFO] running is documented as "no bytes remain" but is derived from the child's exit alone

TerminalOutputResponse::running is documented as "False once the shell has exited and no bytes remain past next_cursor", but the handler sets running = exit.is_none() from session_exit_status (a try_wait on the child), and the inline comment at the construction site explicitly says the opposite: "running alone must not imply there is nothing behind us". The two statements in the same file contradict each other, and the field doc is the one the client reads. Because the reader thread appends asynchronously, the child can be observed as exited while its final output has not been appended yet — exactly the race the jobs stream closes with done. A client that implements its "stream finished" condition as running == false (as the doc invites) can stop polling and lose the shell's trailing bytes; the field must instead be read as "the child has exited" and the client must keep polling until next_cursor == total (and offset == cursor, i.e. no gap). Fixing the field doc (or adding a real drain flag computed against chunk.next_cursor == chunk.total) is the smallest change.

[codewhale-agent]

[INFO] sleep_guard's std::process imports are unused on non-unix builds

use std::process::{Child, Command, Stdio}; is unconditional, but all three names are used only under #[cfg(unix)] (SleepGuard::child, spawn, and the macOS/Linux start_inhibitor), while the struct itself, hold() and the module are compiled on Windows too (the module has a #[cfg(not(unix))] { Self {} } branch). On a cfg(not(unix)) build every one of the three imports is unused, i.e. an unused_imports warning that becomes a build failure under the -D warnings invocation this PR reports running. The same crate already gates exactly these sorts of imports (crates/tui/src/tools/terminal_session.rs puts #[cfg(unix)] on std::sync/std::time imports), so this looks like an omission rather than a policy. Fix: put #[cfg(unix)] on that use line; the Windows build then needs no imports at all.

[codewhale-agent]

State the default the implementation actually uses (unwrap_or("base64")), so a client that omits encoding is not told to send raw UTF-8 that the route then rejects with 400 "data is not valid base64". If text-by-default is what was intended (to match the jobs stdin route), change the default in terminal_input instead and keep this line.

Suggested change

	- `POST /v1/terminal/{name}/input` — `{ "data", "encoding"? }`, UTF-8 text by
	default or `base64` for exact bytes → `{ "name", "written" }`
	- `POST /v1/terminal/{name}/input` — `{ "data", "encoding"? }`, `base64` by
	default for exact bytes, or `text` for UTF-8 → `{ "name", "written" }`

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 0s —— View job

---

I'll analyze this and get back to you.

[Hmbown]

This branch is being carried into the 0.10.0 readiness PR #6370 as a merge of work/0.9.14-core-unblocks at 041e628 (all ten commits, authorship intact), rather than re-implemented.

The three conflicts against current main were all generated or measured files and resolved to main's newer versions: docs/public-surface-facts.json and web/lib/facts.generated.ts (the 0.10.0 capture facts) and scripts/runtime-contract-budget.json (main's 2026-09-19 Linux lock-in already carries the agent cwd / execute_tools ceilings this branch measured; every metric value is identical, only the _comment history differed).

On the merged tree (rustc 1.98.1, Linux): terminal_routes_serve_a_live_engine_session_over_http, terminal_output_for_an_unknown_session…, runtime_info_advertises_terminal_capabilities, thread_event_frames_carry_the_id…, last_event_id_accepts_only_decimal_cursors, turn_endpoint_operation_key_returns_original…, threads_running_lists_active_turns_and_clears_on_settle, a_parked_approval_announces…, agent_events_forward… each 1 passed; sleep_guard 2 passed; terminal_session 15 passed; runtime_api::tests:: 249 passed; turn_loop 67 passed. If #6370 lands by squash instead of merge, this PR will need closing by hand with that credit.

---

Generated by Claude Code