Skip to content

fix(tui): keep Next cancel and quit working under an event flood - #450

Open
BunsDev wants to merge 7 commits into
test/411-approval-edgesfrom
test/408-event-flood
Open

BunsDev wants to merge 7 commits into
test/411-approval-edgesfrom
test/408-event-flood

Conversation

@BunsDev

@BunsDev BunsDev commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Summary

Sends a real flood of text deltas through the real client and query loop, and fixes the two problems it found in Next. Classic's behaviour is unchanged.

Problem Before After
Quitting during a running turn Nothing from that turn was saved. In a new conversation that meant no session file at all, so the prompt and the partial answer were both gone. The turn is stopped, then the prompt and what it delivered so far are saved. If it has not stopped within 2 seconds, only the prompt from that turn is saved.
Deltas still queued when a turn is cancelled Each one restarted the turn on screen, so further Ctrl+C presses cancelled again and never reached the exit sequence. They no longer touch the screen. Ctrl+C reaches the exit sequence straight away.

The second fix also makes a cancelled flood much cheaper (release build, Next, one host, 120x40, 1,000,000 deltas, 2 runs each):

main b3c952f This branch
Settled after Escape 16.5-17.4 s 7.2 s
RSS after settling about 1.7 GB 330-363 MB
Typed key painted during the burst 29-33 ms 30-31 ms
Escape acknowledged 30-36 ms 39-44 ms

Two runs are too few to read anything into the Escape difference, and these rates are far beyond any real provider.

Context

Behaviour to check

  • Quitting can now take up to 2 seconds longer. The terminal is handed back first, then the process waits up to 2 s for the turn to stop. A cancelled flood stops at once. A running sleep did not react to cancellation: exit took about 2.3 s against 0.3 s on main, and only the prompt was saved. The worker copies the turn's messages back only when it finishes, and it was stopped after the 2 s wait, so anything that turn had delivered would have been lost too. The tool did not finish after quit, on either build.
  • Ctrl+C and Ctrl+D do nothing while an approval panel is open. Both were tried: the process stays open and the tool does not run. Answer the panel first. This PR doesn't change that, but it means the exit path isn't reached with a panel open through those keys.

Docs

docs/keybindings.md said one Ctrl+D exits and that it signals EOF when the prompt has content. It takes two presses within 2 seconds, and it does nothing while the prompt has content. docs/commands.md said unsaved state is flushed on exit, which wasn't true of a running turn. It now says what each renderer does, including what is saved when a Next turn does not stop within 2 seconds.

Validation

  • Failing first: deltas_still_queued_when_a_turn_is_cancelled_cannot_restart_it, and the two "keeps the prompt" and "keeps the partial answer" checks, failed before their fixes.
  • Control: with the delta guard disabled on a debug build, exactly the three "Ctrl+C twice more" checks fail and the other 16 pass.
  • scripts/tui-tests/run.sh event_flood: 19 passed, 0 failed, at the review head. At 8f41e75: 25 passed, 0 failed, including the bounded quit; with the wait raised to 20 s, that check fails (the exit took about 18 s by the wall clock).
  • cargo fmt --all --check, cargo clippy --workspace --all-targets --locked -- -D warnings.
  • cargo test --workspace --locked: 2916 passed, 0 failed, at the review head. At 9e6c8a6, which merges test(tui-tests): run approval edges through real engine-local tools #449's new head and so main (Rust 1.99.1): 2915 passed, 0 failed, 52 ignored, with fmt, clippy, engine_contract, slash_command_registry, keybindings_golden and classic_baselines passing. 10b3b48 changes only the terminal case.
  • Full tmux suite: 145 passed, 0 failed. The flood case also passed on its own under bash and zsh.
  • Test scripts' unit tests pass, and git diff --check is clean.

Landing review (2026-11-06)

A second review read 9ab7a49 and found no blockers.

Finding Resolution
Minor: docs/commands.md said a running Next turn's output so far is saved on quit, which is false when the turn has not stopped within 2 s (its messages are copied back only when the worker finishes); the body gave the wrong cause for the sleep probe Docs fixed in 9e6c8a6; body corrected above
Minor: the 2 s value was untested: the unit test passes its own 20 ms wait and the terminal case allowed 30 s Fixed in 10b3b48: the case quits while a Bash sleep ignores the stop and requires the exit within 8 s and the prompt saved. Raising the wait to 20 s makes it fail
Minor, gap rather than regression: the guard drops only backlog text, so a backlog tool start can still mark the turn as streaming during a cancel Follow-up

3c6cf12 merges #449's new head.

A re-review read 10b3b48. The first two findings are resolved, and the gap stays a follow-up. It added one nit: the case counted wait passes, each with a tmux call, as tenths of a second, and called an upper bound "waits about 2 s". Fixed in 1651d86: the exit is timed by the wall clock, and the check says it bounds the exit within 8 s from above; the partial-answer check after a burst is what fails if the wait goes away. 8f41e75 merges #449's new head. A check of 1651d86 found the timer started after the pause that follows the last key, so an exit just over 8 s could pass; fixed in da2721d, which starts it at the last key. At da2721d the case passes 25 of 25, and the exit while a tool ignores the stop reads about 2 s.

Not tested: Windows, Linux, or a real provider. On a fast release build a 300,000-delta burst can settle before the later Ctrl+C presses, so that part of the case only has teeth on a debug build, which is what tui-tests.yml runs. The unit test covers the same logic on any build.

Quitting during a running turn saved nothing from that turn, so a new
conversation lost its prompt and partial answer. Next now stops the
worker, waits up to 2 seconds and saves what it holds.

Deltas still queued after a cancel restarted the turn on screen, so
further Ctrl+C presses cancelled again instead of reaching the exit
sequence. Next now ignores them until the runtime settles, which also
makes a cancelled flood cheaper to drain.

Adds a tmux case that sends a real burst through the client and query
loop, and corrects the docs on how Ctrl+D exits and what is saved.
Classic is unchanged.

Refs #408
@vercel

vercel Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs Ready Ready Preview Nov 6, 2026 11:28am UTC

Request Review

The running turn's messages reach the saved session only after its worker
finishes. If it has not stopped within the 2 second wait, it is aborted
first, and only the prompt from that turn is saved.

Refs #408
Nothing checked the 2 second wait itself: the unit test passes its own short
wait and the terminal case allowed 30 seconds. The case now quits while a
Bash sleep ignores the stop and requires the exit within 8 seconds, with the
prompt saved. Raising the wait to 20 seconds makes it fail.

Refs #408
FLOOD_QUIT_TENTHS counted passes of a loop that also runs tmux, so it
read a slow exit as faster than it was. The bounded quit check now says
what it bounds: an exit within 8 s, from above only.
The timer started after the pause that follows the last key, so an exit
just over 8 s measured under it.

This branch was successfully deployed

1 active deployment
Preview — da2721d2 Deployed Nov 6, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant