Skip to content

fix(tui): decouple WS read/write with dedicated writer task - #54612

Closed
yingliang-zhang wants to merge 1 commit into
NousResearch:mainfrom
yingliang-zhang:fix/tui-ws-read-write-decouple
Closed

yingliang-zhang wants to merge 1 commit into
NousResearch:mainfrom
yingliang-zhang:fix/tui-ws-read-write-decouple

Conversation

@yingliang-zhang

Copy link
Copy Markdown

Problem

The TUI gateway's handle_ws loop was a single serial coroutine:

receive_text → dispatch → write_async → (back to receive_text)

When streaming events from agent worker threads flooded the event loop with send_text coroutines, the write path stalled, blocking the read loop. Inbound RPCs — prompt.submit, session.resume, session.interrupt — sat unread in the WS receive buffer until the desktop client's 30s RPC timeout fired, surfacing as:

  • request timed out: prompt.submit (prompt bounced back to composer)
  • request timed out: session.resume (session switch fails)
  • Esc/Stop not working during heavy tool output

Root cause

WSTransport.write() from worker threads scheduled a send_text coroutine on the event loop and then blocked the caller with fut.result(timeout=10s). Under heavy tool output (kubectl, tar, long terminal commands) the loop couldn't drain the scheduled coroutines fast enough, creating a positive-feedback stall:

  1. Each timed-out write left a frame "in flight" (coroutine scheduled but not yet executed)
  2. More frames piled up → loop fell further behind
  3. The read loop never got CPU time to process new RPC requests
  4. Client times out after 30s, prompt is returned to user

Why this is hard for users to work around

The "heaviness" of a session is not determined by session count or duration — a single kubectl describe or tar -tvf can produce thousands of output lines that flood the WS write path. Users cannot predict which command will trigger the stall.

Fix

Replace the synchronous schedule-and-block write path with a thread-safe queue + dedicated writer task.

Changes (tui_gateway/ws.py)

Before After
write() schedules coroutine on loop + blocks with fut.result(timeout=10s) write() does queue.put_nowait() — returns instantly, never touches event loop
Multiple _safe_send coroutines compete for send_text on same socket Single _writer_loop task drains queue → one send_text at a time
Read loop blocks on await write_async(resp) if WS send is slow Read loop never blocks — write_async just enqueues
_WS_WRITE_TIMEOUT_S = 10.0 timeout + "frame left in flight" warnings No timeout needed — writes never block
Writer thread blocked for up to 10s per frame Writer threads return instantly

Design

                    ┌───────────────────┐
  Agent threads ───▶│  queue.Queue      │──▶ _writer_loop ──▶ ws.send_text
  (streaming events) │  (maxsize=4096)  │   (single task,    (serialized,
  Pool workers ─────▶│                   │    drains queue)    no concurrency)
  (RPC responses)    └───────────────────┘
                             ▲
  handle_ws read loop ───────┘ (write_async, non-blocking)
  (never blocks on write)
  • WSTransport.write(): queue.put_nowait(), thread-safe, returns True unless transport closed. On queue overflow (client completely stuck), drops streaming frames and logs.
  • WSTransport.write_async(): same semantics for event-loop callers. On overflow, returns False so handle_ws treats the RPC response as undeliverable and disconnects (4096 pending frames = dead client).
  • _writer_loop(): single async task, drains queue via run_in_executor(None, q.get), calls send_text one frame at a time. Only coroutine that touches the socket.
  • handle_ws: starts writer after transport creation, checks _closed at top of each read iteration (writer failure → prompt teardown), drains writer with 2s timeout on disconnect.

What this does NOT change

  • Transport protocol interface unchanged (write(obj) -> bool, close() -> None)
  • StdioTransport unaffected (separate implementation)
  • server.py dispatch logic unchanged
  • No new config keys, env vars, or dependencies
  • No prompt caching impact (TUI gateway only)

Tests

Updated tests/test_tui_gateway_ws.py:

Test What it verifies
test_ws_disconnect_reaps_flagged_session_and_closes_worker Existing: disconnect teardown still works
test_ws_disconnect_preserves_and_repoints_reconnectable_session Existing: session detach on disconnect
test_ws_write_does_not_block_on_slow_event_loop New: writes return instantly even when loop is stalled; frames delivered when loop recovers
test_ws_write_returns_false_when_closed New: write() returns False after close() — the "peer is gone" signal
test_ws_writer_serializes_sends New: 4 threads × 5 frames = 20 frames, all delivered without corruption or interleaving

All 5 tests pass. Full tests/tui_gateway/ suite: 248 passed, 1 pre-existing failure unrelated to this change (test_goal_command — stale turn-budget literal).

Related issues

@yingliang-zhang
yingliang-zhang force-pushed the fix/tui-ws-read-write-decouple branch from 80b7262 to 860628c Compare June 29, 2026 04:21
@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jun 29, 2026
The TUI gateway's handle_ws loop was a single serial coroutine:
receive_text → dispatch → write_async.  When streaming events from
agent worker threads flooded the event loop with send_text coroutines,
the write path stalled, blocking the read loop.  Inbound RPCs —
prompt.submit, session.resume, session.interrupt — sat unread in the
WS receive buffer until the desktop client's 30s RPC timeout fired,
surfacing as 'request timed out: prompt.submit' and bounced prompts.

Root cause: WSTransport.write() from worker threads scheduled a
send_text coroutine on the event loop and then blocked the caller with
fut.result(timeout=10s).  Under heavy tool output (kubectl, tar, long
terminal commands) the loop couldn't drain the scheduled coroutines
fast enough, creating a positive-feedback stall: each timed-out write
left a frame 'in flight', the loop fell further behind, and the read
loop never got CPU time to process new RPC requests.

Fix: replace the synchronous schedule-and-block write path with a
thread-safe queue + dedicated writer task.

- WSTransport now owns a queue.Queue (maxsize=4096) and a single
  _writer_loop async task that drains it one frame at a time.
- write() from any thread: non-blocking put_nowait, returns instantly.
  Never blocks the caller, never touches the event loop.
- write_async() from the event loop: same non-blocking semantics.
- Only the writer task calls ws.send_text, so there are never
  concurrent sends on the same socket (a fragile Starlette path).
- handle_ws checks transport._closed at the top of each read iteration
  so a dead writer (send_text failure) tears down promptly.
- On disconnect, the writer is given a 2s drain window before cancel.

The read loop is now fully decoupled from write backpressure: a slow
client can stall the writer, but RPC reads and dispatch never block.

Addresses: NousResearch#53420, NousResearch#53773, NousResearch#48445, NousResearch#51665
@teknium1

Copy link
Copy Markdown
Collaborator

Closing in favor of #55545, which merged the GIL-pressure WS-disconnect fix to main (commits ebb81f10c + 9d10dcd49, contributor authorship preserved).

This was part of a 4-PR cluster all targeting the same root cause (#48445/#50005). The dedicated-writer-task approach here is a substantial read/write-loop rewrite (+233/-117); we landed the lighter-footprint combination instead: loopback ws-ping tuning + token-frame coalescing (@peetwan, #51841) plus routing frontend-polled RPCs off the read loop into the thread pool (@Zyxxx-xxxyZ, #54287). Together those resolve the read-loop starvation (the prompt.submit/session.resume timeouts you described) without rewriting the serial loop.

Appreciate the clear root-cause writeup on the read/write coupling — it matched our diagnosis exactly. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants