bridge_tmux_control_to_websocket bounded the post-%exit drain wait by
total elapsed time (5s). A forwarder that was still actively sending
frames but hadn't finished the whole backlog within that window got
force-cancelled anyway, silently dropping whatever was still queued —
the forward_done event fires on cancellation too, so the drop looked
like a clean completion. Under CI/host contention (or a genuinely slow
client) this drops the tail of a burst-then-exit program's output.
Bound the wait by IDLE time instead: only give up once the forwarder
goes _FORWARD_DRAIN_IDLE_TIMEOUT_S with no frame actually sent, since
the backlog is finite (the reader already queued the EOF sentinel) and
a forwarder that keeps moving bytes must be allowed to finish however
long that takes. Log the queued backlog size when a stall does trigger
the fallback cancel, so a future regression is diagnosable.
Also poll for tmux to finish reflowing a soft-wrapped line before
asserting on the seed capture, instead of a single fixed-delay
capture that can race the reflow under load, and give the registry-io
marker-poll tests more realistic headroom for contended hosts (each
poll round-trips a real tmux subprocess spawn).
Co-authored-by: Isaac <no-reply@databricks.com>
Signed-off-by: harry-yao_data <harry.yao@databricks.com>
Related issue
Summary
tests/terminals/test_control_bridge.py::test_control_bridge_burst_then_exit_delivers_full_tailfailed 10 times across 10 distinct commits with 4 confirmed flakes (failed, then passed on
re-run of the same commit):
This is a real product bug, not test timing. The test already waited deterministically on
reader_done/forward_doneevents, and bytes were still short — which is the tell.bridge_tmux_control_to_websocketbounded the post-%exitdrain with a total-elapsed-timetimeout.
asyncio.shieldprotectedforward_taskfrom thatwait_for's own timeout, but oncethe 5s elapsed the code fell through to an unconditional cancel loop that cancelled
forward_taskfor real — even while it was actively sending frames, just slower than 5s.And because the done-callback fires on any completion including cancellation,
forward_donewas set identically whether the forwarder finished cleanly or was killed mid-drain. The
"drain complete" signal was genuinely a lie, for the test and for any real caller.
Impact on users: a TUI that bursts output just before exiting loses the tail of that output in
the web terminal, under host contention or with a slow client.
Fix: bound the drain by idle time instead of total elapsed time.
_forward_terminal_to_wsgains an
on_frame_senthook; the bridge tracks last-send activity and only gives up once theforwarder has gone a full window with zero frames sent. The backlog is finite (the reader
queues the EOF sentinel before exiting), so a forwarder still moving bytes must be allowed to
finish. A genuinely wedged forwarder still falls through to the same cancel path, but now logs
the queue depth and idle duration so a future stall is diagnosable rather than a bare timeout.
Two test-side races in the same area, both found by stress-running the directory:
test_seed_rejoins_soft_wrapped_linesandtest_seed_alternate_screen_does_not_leak_primary_historyeach did one fixedsleep(0.5)then a single capture. tmux renders a pane program's writes into its gridasynchronously, so under load the capture can race the reflow. Replaced with a
_capture_seed_untilpoll helper that returns the last capture either way, so a genuinerejoin bug is reported with the actual pane content instead of a bare timeout.
tests/terminals/test_registry_io.py's_MARKER_BUDGET_Sis a hang guard whose duration isincidental — each poll round-trips a real
send-keys/capture-panesubprocess spawn plusbash running the command. It now uses
tests.budgets.budget(5.0), so it follows CI's scaleknob (20s under
ci.yml'sOMNIGENT_TEST_TIMEOUT_SCALE=4, still 5s locally so a genuinehang fails fast) rather than a larger hardcoded constant.
Test Plan
and instrumenting the cancel site produced
DEBUG-CANCEL task=tmux-control-forward qsize=264— the forwarder was force-cancelled with264 chunks still queued, and the test failed with the same "dropped the tail" assertion as
CI. In CI's unmodified 5s runs the residual was only 2-4 KB because contention pushed the
total drain just past 5s; same mechanism, smaller remainder.
than the test's 5ms/frame send delay) → passes, confirming the new logic tolerates a
slow-but-progressing drain the old code would have killed.
pytest tests/terminals -n 8 --dist=loadfile -q -p no:randomlygreen. The worker's own15-iteration run before the seed fix had 1 failure, in the alt-screen seed test now fixed here.
budget(5.0)resolves to20.0withOMNIGENT_TEST_TIMEOUT_SCALE=4and5.0unset, and that
repl-sdk(the group owningtests/terminals) is in theci.ymlmatrix whoseshared
Run pyteststep sets that variable — so the scaling actually applies here.pre-commit run --files <4 changed files>: all hooks pass exceptpyrefly, whose errors areall in unrelated
sdks/python-client/omnigent/replmodules resolving against a differentworktree's editable SDK path (a known shared-venv artifact); none reference the changed files.
Demo
Type of change
Test coverage
Coverage notes
test_control_bridge_burst_then_exit_delivers_full_tailis the regression test for the productfix and is unchanged — its assertion (full 2 MB payload delivered) was already correct; it was
the bridge that dropped bytes. Manual verification covers the mechanism itself: the
shrunk-timeout instrumentation above is what demonstrates cancellation-with-backlog, and it is
not committed because reproducing it requires deliberately mis-sizing a production constant.
Changelog
Terminal output from a program that prints a burst just before exiting is no longer truncated in the web terminal
This pull request and its description were written by Isaac.