* feat(terminal): broadcast output to every viewer instead of one shared queue TerminalSession.output was a single queue.Queue read destructively by the SSE handler. Two tabs/windows viewing the SAME session each open their own EventSource, so two _handle_terminal_output handlers competed on that one queue: every PTY chunk was delivered to exactly one of them. Each tab saw a disjoint half of the byte stream, and only one ever received terminal_closed. Output now fans out, mirroring StreamChannel/SessionChannel: each SSE consumer subscribe()s its own queue (seeded with a bounded backlog so a first/late attach still replays the recent scrollback, preserving the old buffer-until-first- consumer behaviour), and put_output broadcasts to all subscribers. A slow viewer's queue drops its own oldest chunk (drop-oldest, isolated per subscriber) so one lagging tab can't starve another. The handler unsubscribes in a finally so the subscriber list can't grow. Stacked on the terminal fd-leak fix (same file). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: cover terminal broadcast lifecycle * fix: serialize terminal subscriber fanout * test: cover terminal unsubscribe publication race * restore timing-flaky pytest.skip on test_terminal_survives_short_lived_request_thread (keep API-updated body); changelog #5836 --------- Co-authored-by: ai-ag2026 <261867348+ai-ag2026@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: nesquena-hermes <agent@nesquena-hermes>
90 lines
4.1 KiB
Python
90 lines
4.1 KiB
Python
"""Tests for #1623: SSE app heartbeat must fire well under the kernel keepalive timeout.
|
|
|
|
Bug shape: server.py's per-connection TCP keepalive (added v0.50.289 / #1581)
|
|
declares a peer dead at KEEPIDLE=10s + KEEPINTVL=5s * KEEPCNT=3 = 25s. The
|
|
SSE handlers in api/routes.py used a 30s app-level heartbeat. When the LLM
|
|
is thinking and the queue is idle, the kernel could tear down the socket
|
|
before the app sent its first heartbeat byte — flaky-network drops at ~10s
|
|
that the user perceived as "the stream died around 10 seconds in."
|
|
|
|
Fix: lower the heartbeat to 5s at every SSE handler and pin the inequality
|
|
with a regression test so future tuning of either timer can't re-introduce
|
|
the misalignment.
|
|
"""
|
|
|
|
from pathlib import Path
|
|
|
|
|
|
REPO = Path(__file__).parent.parent
|
|
|
|
|
|
def test_sse_heartbeat_constant_below_kernel_keepalive_window():
|
|
"""The named constant exists and is at most half the kernel keepalive
|
|
timeout (10 + 5*3 = 25s). 5s gives the kernel ~5x headroom."""
|
|
src = (REPO / "api" / "routes.py").read_text(encoding="utf-8")
|
|
|
|
# The constant must be defined.
|
|
assert "_SSE_HEARTBEAT_INTERVAL_SECONDS" in src, (
|
|
"Named SSE heartbeat constant must exist (#1623)"
|
|
)
|
|
|
|
# Pull the literal value.
|
|
import re
|
|
m = re.search(r"_SSE_HEARTBEAT_INTERVAL_SECONDS\s*=\s*(\d+)", src)
|
|
assert m, "Could not parse _SSE_HEARTBEAT_INTERVAL_SECONDS literal"
|
|
heartbeat = int(m.group(1))
|
|
|
|
# Reproduce the kernel-keepalive window from server.py setsockopt block.
|
|
server_src = (REPO / "server.py").read_text(encoding="utf-8")
|
|
assert "TCP_KEEPIDLE" in server_src, "TCP_KEEPIDLE must be set on accepted connections"
|
|
keepidle = int(re.search(r"TCP_KEEPIDLE.*?(\d+)\)", server_src, re.S).group(1))
|
|
keepintvl = int(re.search(r"TCP_KEEPINTVL.*?(\d+)\)", server_src, re.S).group(1))
|
|
keepcnt = int(re.search(r"TCP_KEEPCNT.*?(\d+)\)", server_src, re.S).group(1))
|
|
kernel_window = keepidle + keepintvl * keepcnt
|
|
|
|
# The acceptance criterion from the bug: app heartbeat <= kernel window / 2.
|
|
assert heartbeat * 2 <= kernel_window, (
|
|
f"App SSE heartbeat ({heartbeat}s) must be at most half of the kernel "
|
|
f"keepalive window ({kernel_window}s = {keepidle} + {keepintvl}*{keepcnt}). "
|
|
f"Otherwise flaky-network probes can tear down the socket before the "
|
|
f"app sends a heartbeat byte. (#1623)"
|
|
)
|
|
|
|
|
|
def test_no_sse_handler_uses_30s_or_higher_timeout():
|
|
"""No SSE/long-poll handler in routes.py should still be using the old
|
|
30s/25s timeout. Every queue.get(timeout=...) call inside an SSE handler
|
|
must reference the named constant, not a hard-coded number."""
|
|
src = (REPO / "api" / "routes.py").read_text(encoding="utf-8")
|
|
|
|
import re
|
|
# Catch q.get(timeout=30), subscriber.get(timeout=30), term.output.get(timeout=25), etc.
|
|
bad = re.findall(r"\.get\(timeout=3[05]\)", src)
|
|
assert not bad, (
|
|
f"Found {len(bad)} SSE handler call(s) still using a 25/30s timeout: {bad}. "
|
|
"All should use _SSE_HEARTBEAT_INTERVAL_SECONDS (#1623)."
|
|
)
|
|
|
|
|
|
def test_each_named_sse_handler_uses_constant():
|
|
"""Each known SSE handler queue-poll site must reference the constant."""
|
|
src = (REPO / "api" / "routes.py").read_text(encoding="utf-8")
|
|
|
|
expected_callers = [
|
|
"subscriber.get(timeout=_SSE_HEARTBEAT_INTERVAL_SECONDS)", # main agent SSE
|
|
"output.get(timeout=_SSE_HEARTBEAT_INTERVAL_SECONDS)", # terminal SSE (per-consumer queue)
|
|
]
|
|
for caller in expected_callers:
|
|
assert caller in src, (
|
|
f"Expected SSE handler to call {caller!r} (#1623). "
|
|
"If this assertion fails, the SSE heartbeat misalignment may have regressed."
|
|
)
|
|
|
|
# Also: at least 3 sites should be using the constant overall (main agent,
|
|
# terminal, plus the gateway watcher and approval/clarify pollers).
|
|
n_uses = src.count("get(timeout=_SSE_HEARTBEAT_INTERVAL_SECONDS)")
|
|
assert n_uses >= 4, (
|
|
f"Expected at least 4 SSE/long-poll sites using the named constant; found {n_uses}. "
|
|
"Every long-lived idle queue poll must align below the kernel keepalive window."
|
|
)
|