feat(app): wait_for_status + read_terminal on the live bridge (F-mcp-terminal-sync, #195)#227
Merged
Merged
Conversation
Answering an external request is not one thing: a read answers off owned state, a mutation needs `&mut` and the effect executor. That fork sat inline in `update`, which already carries a too-many-lines allow, and the wait rung adds a third shape (a reply that lands later, not in this update). Move it to `shell::serve`, unchanged, so there is one auditable place where an external caller meets `core::App`.
Orchestration could act and perceive, but not synchronise: `run_in_session`
writes and returns, so an agent had no way to know when the command finished
short of polling `snapshot` and racing the transition it was watching for.
`wait_for_status` blocks until a session's OSC-derived activity reaches one of
the asked-for statuses, and `read_terminal` returns one pane's visible text —
the deep read the light `snapshot` deliberately leaves out.
The wait is the first bridge request whose reply lands in a *later* `update`.
`ReplyPort` was already built for that (a take-once slot behind an `Arc`), so
the shell parks it in a waiter list and settles it from the status change
itself — nothing polled, no transition raced.
Three rules earn their own tests:
- a session already sitting on a target answers at once, or a caller asking
"tell me when it is idle" about an idle session would wait out its bound
having missed the transition it asked about;
- an exit settles every waiter whatever it asked for — a crash records
`Exited` without emitting a status change, and a dead session will never
reach the target anyway;
- a caller that gave up is swept off the list rather than held for a session
that may never move again.
Timing out is not a failure: the tool answers `{ status, timed_out: true }`
with the session's current status, so an agent can choose between waiting again
and giving up. The bound is the caller's `timeout_ms` (default 30 s), capped at
5 minutes so no parameterisation can park a caller indefinitely (Q7).
`read_terminal` keeps three outcomes distinct that a scoped `snapshot`
collapses into one absent key: unknown handle (invalid_params), live but
undrawn (`rendered: false`, retry), and text.
bastien-gallay
force-pushed
the
worktree-195-mcp-terminal-sync
branch
from
July 25, 2026 19:32
f5b11a0 to
3ecd0c7
Compare
Four defects the review found, all on the same seam — three of them ways a caller ends up parked on a session core has already given up on. `Exited` is terminal in core: `StatusChanged` refuses to overwrite it. So a wait placed *after* an unclean exit parked forever, because the exit that would have settled it had already fired. `serve_wait` now short-circuits on it, the same rule `settle_waiters` applies — extracted to one `settles` predicate rather than two copies that already disagreed. The settling status was taken from the message. A status still in flight when the exit landed is dropped by core, but was reported to the client — a crashed session answering "idle". Read it back from core instead, which also gives the clean-exit case (session gone from the registry) its `Exited` answer for free. Closing a tab drops its sessions with no PTY exit reaching `update`, so nothing settled those waiters, and nothing swept the ones whose caller had timed out on a quiet workspace. `serve` now sweeps first: it is the one path that runs on every external call, however still the sessions are. Finally, the status read-back after a timeout propagated its own failure. The likeliest cause of the first timeout is a wedged shell, which fails the read-back too — turning the answer the caller had earned into an error. It is best-effort now (`status: null`), on a 250ms bound so it barely extends the wait it follows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The wait rung, closing
act → wait → observe. Orchestration (#194) couldact and
snapshot(#212) could perceive, but nothing could synchronise:run_in_sessionwrites and returns, so an agent had no way to know when acommand finished short of polling
snapshot— and racing the very transitionit was watching for.
wait_for_status(session[, statuses][, timeout_ms])statuses(default idle-or-attention). Returns{ status, timed_out }.read_terminal(session[, lines])snapshotleaves out. Returns{ text, rendered }.How
The wait is the first bridge request whose reply lands in a later
update.That turned out to need no new machinery:
ReplyPortis already a take-onceslot behind an
Arc, built precisely so an answer need not be immediate. Theshell parks it in a waiter list and settles it from the status change itself —
nothing polled, no transition raced (the feature-torture report cut
wait_for_textfor exactly that failure mode).Three rules earn their own tests, each for a concrete failure:
asking "tell me when it's idle" about an idle session waits out its whole
bound, having missed the transition it asked about.
core:pty_exitedrecordsExitedand returns no effects — a crash emits nostatus change. And a dead session will never reach the target anyway.
session that may never move again (
ReplyPort::abandoned).Timing out is not a failure. On timeout the tool reads the session's
current status back and answers
{ status, timed_out: true }, so an agent canchoose between waiting again and giving up instead of getting a bare error. The
bound is the caller's
timeout_ms(default 30 s), capped at 5 minutes — noparameterisation can park a caller indefinitely (Q7).
read_terminalkeeps three outcomes distinct that a scopedsnapshotcollapses into a single absent key: unknown handle (
invalid_params), live butundrawn (
rendered: false— retry, don't give up on the handle), and text.Truncation reuses
core::snapshot::tail_linesrather than growing a secondrule that could drift from a snapshot's.
Two commits
refactor(app)— tidy-first, no behaviour change: theMessage::Bridgearm moves out of
update(which already carries a too-many-lines allow)into
shell::serve, so there is one auditable place where an externalcaller meets
core::App.feat(app)— the rung above.Tests
Written red first. 20 new, all green:
non-target status; ignores another session's change; rejects an unknown
handle without parking; settled by an unclean exit; abandoned waiter swept;
and four
read_terminalcases (text, line budget, undrawn, unknown handle).rejected (not silently dropped), settled status reported, rejection relayed
as
invalid_params, the timeout path (a two-reply test shell: the wait isreceived and never answered, then the status read-back), the cap, and
read_terminal's handle/line-budget/undrawn/rejection shaping.Full suite green (
cargo test --workspace);fmt·clippy·machete·deny·check-deps·check-arch·markdownlintall pass. ROADMAP entryticked.
Closes #195.