0.10.0 follow-up: user-input deadline under the heartbeat, terminal routes off the runtime workers, sleep inhibitor as a tokio child

[Hmbown]

Summary

The review bots that woke when #6370 was marked ready posted eight threads on the merged head. Each one was verified against main 056eb55 before touching anything; these are the six that hold, in the order they matter for 0.10.0.

• User-input timeout never fired (Devin, red). await_user_input rebuilt timeout(wait, recv()) on every pass through its select!, and the Engine silently freezes mid-run: user messages are persisted but never answered; no error, no log line, no crash entry #6184 heartbeat wins that select every 60 s, dropping the pending timeout each time. With the default 300 s an unanswered prompt parked the turn forever. The wait now takes one absolute Instant deadline and selects on timeout_at; user_input_timeout_seconds = 0 still waits indefinitely. Regression test user_input_deadline_is_not_reset_by_the_wait_heartbeat (50 ms test heartbeat vs a 200 ms timeout) fails on the previous code.
• Sleep inhibitor on a runtime worker (Devin + Claude Code Review). SleepGuard spawned caffeinate/systemd-inhibit with std::process and its Drop ran kill + wait inline from Engine::run_turn, which the blocking-call convention (Blocking calls in async paths: audit thread::sleep and std::fs in tool paths, pick one convention #6149) rules out and the budget script does not count. It is now a tokio::process child spawned with kill_on_drop: Drop drops the child, which sends the release signal synchronously, and the runtime reaps the process. hold requires a runtime context, which its only caller already is.
• Terminal routes drove the PTY inline (Devin). The four /v1/terminal/{name}/* handlers locked the session mutex and wrote/resized/killed on a Tokio worker; a 64 KiB input write to a shell that stopped reading blocks until the kernel buffer drains. Every session touch now goes through one with_session helper on the blocking pool, matching the tool side.
• OpenHarmony advertised terminals it answers 501 for (Devin). runtime/info's terminal_* flags used cfg!(unix) while the routes are gated on all(unix, not(ohos)); the flags now use the routes' gate and the capability test asserts it.
• A cursor past the head was echoed back (Devin). read_since returned a future cursor as next_cursor, so a client continuing from it skipped every byte produced before the stream reached that position. The start now clamps to total. This is a deliberate contract change from feat(runtime-api): terminal byte stream (#34), stream resume + idempotent submit (#76), pet agent-count pin (#12) #6361's "honest about a cursor ahead" wording: a future cursor still reads nothing and is not a gap, but it hands back the head.
• Docs. The input encoding default is base64, as the handler and its round-trip test already say; the 501 platforms name OpenHarmony beside Windows; the clamp above is stated.

Not changed: the changelog note (CONTRIBUTING reserves changelog edits for the merge-time receipts pass; #6370 was that pass and is already on main). No changelog hunks in this PR for the same reason.

No-Issue: post-merge review-bot findings on #6370, verified against main; no tracker issue exists for them (the timeout regression touches #6184's heartbeat but does not close #6184).

Testing

Head 50b4dec (the first head, c6b52ff, failed every non-test lib build on field child is never read once the old Drop was gone; the explicit Drop commit fixes that and cargo check -p codewhale-tui --lib --locked is clean locally: exit 0, zero warnings).

CI on 50b4dec, all green: Lint ✓, Test (ubuntu-latest) ✓, Test (macos-latest) ✓, Test (windows-latest) ✓, cargo check (aarch64-unknown-linux-ohos) ✓, Mobile runtime smoke ✓, Safety gate ✓, npm wrapper smoke ✓, Version drift ✓, Integrations ✓, VS Code ✓, link ✓, CodeQL (rust/python/js/actions) ✓, GitGuardian ✓.

Local, tui lib test binary built from this tree (cargo rustc -p codewhale-tui --lib --profile test --all-features --locked, sealed HOME, proxy unset):

• core::engine::approval::tests::user_input_deadline_is_not_reset_by_the_wait_heartbeat — test result: ok. 1 passed; 0 failed (0.22 s). Without the fix (the two await_user_input hunks reverted, test kept, rebuilt): test result: FAILED. 0 passed; 1 failed (3.23 s) — panicked … a bounded user-input wait must end at its own deadline: Elapsed(()), i.e. the 200 ms timeout never fired and the 3 s outer guard tripped. File restored; tree clean.
• core::engine::approval::tests:: — 8 passed (includes the Engine silently freezes mid-run: user messages are persisted but never answered; no error, no log line, no crash entry #6184 parked-approval announcement test).
• sleep_guard:: — 2 passed (on the binary carrying the explicit Drop).
• tools::terminal_session::tests:: — 14 passed (bounded replay incl. the new past-the-head clamp, session-level read, resize-reaches-kernel, …).
• runtime_api::tests::terminal_* — 2 passed (terminal_routes_serve_output_input_resize_and_kill_over_http against a real PTY through the blocking-pool helper; unknown session 404).
• runtime_api::tests::runtime_info_advertises_terminal_capabilities — 1 passed.
• runtime_api::tests::thread_event* — 1 passed.

Tip re-verification of the Linux MCP startup on this tree's debug TUI (sha 5d00b589e1ec): raw-pty SIGPIPE probe with a required MCP server that exits immediately, 12 launches — 0 SIGPIPE deaths, 12/12 reached the launch card. Cucumber --all-features (55 tests) launch_card_pty:: group: test result: ok. 10 passed; 0 failed; 6 ignored (113.8 s).

• cargo fmt --all -- --check
• cargo clippy --workspace --all-targets --all-features --locked (warning-free under the CI allow list) — CI Lint ✓ on 50b4dec
• cargo test --workspace --all-features --locked — CI Test ubuntu ✓ / macos ✓ / windows ✓ on 50b4dec

Checklist

• This PR adds a new layer/module/abstraction — it names or deletes the layer it replaces (with_session replaces lock_session in the same module; nothing else is new)
• Updated docs or comments as needed
• Added or updated tests where relevant
• Verified TUI behavior manually if UI changes — no UI surface changes; PTY launch group and SIGPIPE probe above
• Harvested/co-authored credit uses a GitHub numeric noreply address (n/a)

🤖 Generated with Claude Code

https://claude.ai/code/session_0134iUMxmGuXiG1LzPgfZVnv

[Copilot]

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

[claude]

Claude finished @Hmbown's task in 1m 29s —— View job

---

Review of PR #6371 (merged)

• Gather context (diff, CLAUDE.md/AGENTS.md rules)
• Review approval.rs deadline fix
• Review sleep_guard.rs tokio child change
• Review runtime_api/terminal.rs blocking-pool change
• Review tools/terminal_session.rs clamp fix
• Review docs update
• Post findings

Note: this repo checkout is a shallow, single-commit snapshot of the already-merged head, so I reviewed the final state of the six changed files (crates/tui/src/core/engine/approval.rs, crates/tui/src/sleep_guard.rs, crates/tui/src/runtime_api.rs, crates/tui/src/runtime_api/terminal.rs, crates/tui/src/tools/terminal_session.rs, docs/RUNTIME_API.md) against the description rather than a git diff, and traced each described fix through the code.

Findings: none. All six fixes check out against the code as-is:

1. User-input deadline (approval.rs:280,304-310) — deadline is computed once as an absolute tokio::time::Instant outside the select! loop and reused via timeout_at every iteration, so the heartbeat (WAIT_HEARTBEAT) winning select! repeatedly no longer resets it. The regression test user_input_deadline_is_not_reset_by_the_wait_heartbeat exercises exactly this interaction (50 ms heartbeat vs 200 ms timeout) and is a real assertion on ToolError::Timeout, not a tautology.
2. Sleep inhibitor (sleep_guard.rs) — start_inhibitor now spawns via tokio::process::Command with kill_on_drop(true); Drop just drops the Child, which sends the signal without an inline wait. No blocking call runs on a runtime worker. kill_on_drop's underlying kill is a non-blocking signal send, so this is safe from Engine::run_turn.
3. Terminal routes (runtime_api/terminal.rs:207-225) — with_session centralizes the lock + PTY operation inside spawn_blocking, and all four handlers (terminal_output, terminal_input, terminal_resize, terminal_kill) route through it. No inline mutex lock or write happens on the async handler itself anymore.
4. runtime/info capability flags (runtime_api.rs:600-603) — now cfg!(all(unix, not(target_env = "ohos"))), matching the routes' own #[cfg(all(unix, not(target_env = "ohos")))] gate exactly (previously cfg!(unix), which is true on OpenHarmony even though those routes 501). runtime_info_advertises_terminal_capabilities in runtime_api/tests.rs asserts against the same expression rather than a literal, so it can't drift from the routes again.
5. Cursor clamp (tools/terminal_session.rs:177-192) — start = cursor.max(dropped).min(self.total) clamps a future cursor to total; walked the arithmetic for cursor > total: skip = total - dropped (all retained bytes), take = 0, next_cursor = total. Matches the stated contract change and the beyond/ahead test cases at lines ~1406/1439.
6. Docs (docs/RUNTIME_API.md) — the input default (base64), the OpenHarmony 501 note, and the cursor-clamp behavior are all stated and consistent with the code.

One pre-existing (not introduced by this PR) note worth flagging separately if anyone revisits sleep_guard.rs: on Linux, systemd-inhibit ... sleep infinity is tracked as a tokio::process::Child for the systemd-inhibit process only; killing it releases the inhibitor lock (tied to the process), but the grandchild sleep infinity is not itself part of the tracked Child and could be left as an orphan depending on how systemd-inhibit forwards signals. This predates the PR (same invocation existed under std::process before) and isn't something this diff needs to fix, just calling it out for visibility.

No provider/model-prefix routing code, no new tests needed beyond what's already added (the regression test for the timeout bug is the one that matters and is documented as failing without the fix), and no reuse/duplication issues — with_session deletes the old lock_session per-handler duplication as the PR checklist states.

[devin-ai-integration]

Devin Review found 1 potential issue.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-21T01:28:38.508504Z	50b4dec	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.