fix(core): keep chained control-mode command lists from answering the wrong waiter - #1129
Conversation
tmux answers a chained command list with one %begin/%end block per command it runs, and stops the list at its first error. The reader handed out one waiter per block, so the second and third blocks of spawn's `new-window ; set-window-option ; set-window-option` could reach whichever command was sent next. Each waiter now carries how many commands its line holds, and the reader keeps it until that many blocks, or an error, are in. Measured on tmux 3.2 and 3.7c: nothing on the wire groups a list's blocks (each has its own command number), so the count has to come from the sender. Closes #1120 Claude-Session: https://claude.ai/code/session_01N1NAYr4d15oadDybC7855A
…-block waiter contract
|
Greptile SummaryThis PR fixes control-mode response routing for tmux command lists by retaining each waiter until all expected response blocks arrive or an error terminates the list.
Confidence Score: 5/5The PR appears safe to merge, with the command-list response-routing fix consistently applied and covered by focused regression tests. No actionable failure remains: production lists derive their expected block count from the same command slice used to construct the wire command, errors terminate the active waiter correctly, and subsequent responses remain aligned.
|
| Filename | Overview |
|---|---|
| src/agent/control_mode/mod.rs | Introduces block-counted waiters and aggregates all response blocks belonging to a command list. |
| src/agent/tmux.rs | Sends window creation and birth-option commands through the new counted command-list path. |
| src/agent/control_mode/tests.rs | Adds real-tmux regressions for complete lists and lists terminated by an error. |
| docs/ARCHITECTURE.md | Documents tmux command-list response framing and waiter ownership. |
Reviews (1): Last reviewed commit: "chore: no-mistakes document - docs(core)..." | Re-trigger Greptile



Intent
Resolve thurbox issue #1120: control-mode responses can be matched to the wrong waiter. A chained command list such as spawn's 'new-window ; set-window-option remain-on-exit ; set-window-option window-size' (from #1107) answers with one %begin/%end block per command, but deliver_response popped one waiter per %end, so the later (empty) blocks could reach whichever command was sent next. Done means: a test that fails before the fix and passes after it; no response block from a chained command list can reach the wrong waiter, pinned by a test that drives a multi-command list on real tmux; the repo gate passes; the PR says 'Closes #1120'. Hard constraints from the task brief: test before fix; no scope beyond the issue; do not widen a public API beyond what the issue asks for; do not reopen #1107's retention race — remain-on-exit must stay in the same command list as new-window; do not change the program-pane e2e timeouts from #1113 in this PR. Decisions made: measured raw control mode on tmux 3.2 (built from the release tarball) and 3.7c — each command in a list gets its own block and its own command number (tmux's item->number is a global counter), so nothing on the wire groups a list's blocks, and an error drops the rest of the list (a failing first command gives 1 block, a failing middle command gives 2). Therefore the sender must say how many commands its line holds: ControlMode gains send_command_list(&[&str]) (pub(super), not public) which joins with ' ; ' and records the count on the waiter; send_command is the one-command case; the reader keeps a partially answered waiter until that many blocks or the first %error arrive, concatenating their lines. TmuxBackend::ctrl_command delegates to a new private ctrl_command_list with the same reconnect-and-retry; spawn builds new-window plus birth_option_commands (renamed from birth_options_suffix, now returning one command per option) and sends them as one list, so the count and the join come from the same slice. A list with an error fails as a whole (send_command's existing contract); a window created before a failing set-window-option is not cleaned up, same as main. Tests are in-crate in src/agent/control_mode/tests.rs because ControlMode is pub(super); they start a throwaway tmux server on a unique socket without mutating process env and skip when tmux is absent. One test holds a list's middle block open with run-shell 'sleep 0.5' so the next command queues behind it; the other cuts a list short with an invalid option. Both failed before the fix on 3.2 and 3.7c (next command got 'third'; the list's error was lost) and pass after. The PR description follows the no-mistakes five headings and ends with the line '— LeTuR's agent'.
What Changed
ControlModegains aWaiter { tx, blocks }type and apub(super) send_command_list(&[&str])that joins commands with" ; "and records how many%begin/%endblocks the resulting answer spans;send_commandbecomes the one-block case delegating to the samesend_command_on(.., blocks).deliver_responsenow accumulates an in-progressAnsweracross a waiter's blocks — concatenating each block's lines — and only pops the next queue entry once that many blocks have arrived or the first%errordoes, instead of handing each%end/%errorto a fresh waiter.TmuxBackend::ctrl_commanddelegates to a newctrl_command_list(same reconnect-and-retry on broken pipe/timeout) andspawn_windowsendsnew-windowplus the renamedbirth_option_commands(wasbirth_options_suffix) as one list viasend_command_list, so the block count and the;-join come from the same command slice.src/agent/control_mode/tests.rsthat start a throwaway tmux server: one holds a list's middle block open withrun-shell 'sleep 0.5'to prove the next queued command doesn't get answered by it, the other cuts a list short with an invalidset-window-optionand checks the error is reported and the following command still gets its own answer.docs/ARCHITECTURE.mddocuments the command-list waiter contract.Closes #1120
— LeTuR's agent
Risk Assessment
✅ Low: The fix is narrowly scoped to the reported issue, correctly tracks per-waiter block counts derived from the same slice used to join the command line, keeps remain-on-exit in the same list as new-window as required, adds real-tmux regression tests that reproduce the original bug and pass after the fix, and compiles cleanly with no other call sites depending on the changed internal types.
Testing
Baseline
cargo nextest run --allhad already passed. I additionally ran the two new real-tmux tests in src/agent/control_mode/tests.rs (a_command_list_is_answered_once_all_its_blocks_are_in and a_command_list_cut_short_by_an_error_fails_and_keeps_later_answers_in_place) against a live tmux 3.7c server — both pass at the target commit, and the run-shell-held-block scenario end-to-end demonstrates a chained command list (new-window-style) no longer leaks a block to a subsequently sent command. To satisfy the fail-before/pass-after requirement myself rather than just trusting the PR description, I temporarily checked out the base commit, wrote a scratch test exercising the identical scenario through the pre-fix API, and confirmed it fails with precisely the reported symptom (the next command receives the list's leftover "third" block instead of its own "second" answer); I then discarded that scratch file and restored the worktree exactly to the target commit. Reviewed the diff against the intent's hard constraints: send_command_list is pub(super) (not a widened public API), new-window and its birth_option_commands (renamed from birth_options_suffix) are still sent as a single command list preserving #1107's retention-race fix, and no program-pane e2e timeout files from #1113 were touched. No issues found.Evidence: Real-tmux regression test failing before the fix (bug #1120 reproduced)
Evidence: Same scenario passing after the fix, via the shipped tests, on real tmux
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
cargo nextest run --allcargo nextest run --lib -p thurbox control_mode::tests::a_command_list (both new real-tmux tests, on tmux 3.7c) — PASS at target commit bb76114cargo nextest run --lib -p thurbox control_mode:: — all 93 tests in the module PASS at target commitManual regression repro: checked out base commit 90ba74b, added a scratch test driving the same real-tmux scenario through the pre-fix send_command API, confirmed it fails with the exact #1120 symptom (next command receives a leftover block from the prior list), then discarded the scratch test and restored the worktree to bb76114 (git status clean, HEAD verified)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.