release: v0.50.248
Bundles: - #1349 fix(ui): show context indicator percentage without explicit context_length - #1350 feat(approval): SSE long-connection for real-time approval notifications Pre-release fixes applied: - Inline subscribe + snapshot under a single _lock acquisition in _handle_approval_sse_stream() to close the snapshot/subscribe race flagged in pre-release review. A submit_pending() arriving between the snapshot read and subscribe call would have been lost (appended to _pending after our snapshot AND notified to subscribers before we joined). Now atomic. - Added tests/test_pr1350_sse_atomic_subscribe.py (4 source-level invariants covering the atomic-lock-block guarantee). Co-authored-by: jasonjcwu <jasonjcwu@users.noreply.github.com>
This commit is contained in:
@@ -2,6 +2,14 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [v0.50.248] — 2026-04-30
|
||||
|
||||
### Added
|
||||
- **Real-time approval notifications via SSE long-connection** — replaces the 1.5s HTTP polling loop with a Server-Sent Events endpoint at `/api/approval/stream?session_id=` that pushes approval events to the browser the instant they fire. Cuts approval latency from up to 1.5s down to near-instant and eliminates the "always polling" network noise users observed. Backend uses a thread-safe subscriber registry (`_approval_sse_subscribers` dict, bounded `queue.Queue(maxsize=16)` per subscriber, silent drop on full to prevent leaks from slow tabs). 30s keepalive comments prevent proxy/CDN timeouts; `_CLIENT_DISCONNECT_ERRORS` + `finally` block guarantee subscriber cleanup on any exit path. **Subscribe and snapshot are taken atomically under a single `_lock` acquisition** so a `submit_pending()` arriving in the gap can't be lost. Frontend uses `EventSource` with automatic 3s HTTP polling fallback on `onerror`. 42 new tests cover wiring, lifecycle, multi-subscriber, cross-session isolation, queue overflow, and concurrent subscribe/notify stress. (`api/routes.py`, `static/messages.js`, `tests/test_approval_sse.py`) @fxd-jason — PR #1350
|
||||
|
||||
### Fixed
|
||||
- **Context indicator percentage shows even without explicit `context_length`** — frontend companion to the v0.50.246 backend fix. The context ring used to display `·` (no data) whenever `context_length` was 0 or missing — fresh agents, interrupted streams, or models without compressor state. Now defaults to **128K** when `usage.context_length` is falsy and labels the indicator with `(est. 128K)` so users can tell apparent vs. measured. Falls back to `input_tokens` for `last_prompt_tokens` so the ring lights up immediately on the first user message. (`static/ui.js`) @fxd-jason — PR #1349
|
||||
|
||||
## [v0.50.247] — 2026-04-30
|
||||
|
||||
### Added
|
||||
|
||||
@@ -2773,10 +2773,17 @@ def _handle_approval_sse_stream(handler, parsed):
|
||||
if not sid:
|
||||
return bad(handler, "session_id is required")
|
||||
|
||||
# Send initial snapshot (any approval already queued before SSE connected).
|
||||
# Subscribe AND snapshot atomically under a single _lock acquisition so a
|
||||
# submit_pending() that fires between the two cannot be lost. If we
|
||||
# snapshot first then subscribe (the naive ordering), an approval that
|
||||
# arrives in the gap is appended to _pending (after our snapshot) AND
|
||||
# notified to subscribers (before we joined) — leaving the client unaware
|
||||
# until the next event arrives.
|
||||
q = queue.Queue(maxsize=16)
|
||||
initial_pending = None
|
||||
initial_count = 0
|
||||
with _lock:
|
||||
_approval_sse_subscribers.setdefault(sid, []).append(q)
|
||||
q_list = _pending.get(sid)
|
||||
if isinstance(q_list, list):
|
||||
initial_pending = dict(q_list[0]) if q_list else None
|
||||
@@ -2797,7 +2804,6 @@ def _handle_approval_sse_stream(handler, parsed):
|
||||
# Push initial state immediately so the client doesn't miss anything.
|
||||
_sse(handler, 'initial', {"pending": initial_pending, "pending_count": initial_count})
|
||||
|
||||
q = _approval_sse_subscribe(sid)
|
||||
try:
|
||||
while True:
|
||||
try:
|
||||
|
||||
108
tests/test_pr1350_sse_atomic_subscribe.py
Normal file
108
tests/test_pr1350_sse_atomic_subscribe.py
Normal file
@@ -0,0 +1,108 @@
|
||||
"""Test that the SSE subscribe + snapshot are taken atomically under _lock.
|
||||
|
||||
Regression test for the snapshot/subscribe race condition: if subscribe
|
||||
happens AFTER the snapshot, a submit_pending() that fires in the gap is
|
||||
both appended to _pending (after our snapshot) AND notified to subscribers
|
||||
(before we joined) — the client never learns about it until the next event.
|
||||
|
||||
The fix in v0.50.248 takes the lock once, registers the subscriber queue,
|
||||
THEN reads the snapshot — all under the same lock acquisition.
|
||||
|
||||
This test verifies the source-level invariant rather than the runtime
|
||||
behavior: the subscriber-registration line MUST appear inside the same
|
||||
`with _lock:` block as the snapshot read, and BEFORE the snapshot read.
|
||||
"""
|
||||
|
||||
import pathlib
|
||||
import sys
|
||||
|
||||
REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve()
|
||||
sys.path.insert(0, str(REPO_ROOT))
|
||||
|
||||
ROUTES_SRC = (REPO_ROOT / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _extract_lock_block(body: str) -> str:
|
||||
"""Extract the body of the first `with _lock:` block from the handler.
|
||||
|
||||
Lines are part of the block as long as they are blank or start with the
|
||||
block's indent (>= 8 spaces, since the handler body itself is at 4 spaces
|
||||
and the lock block indents one level deeper).
|
||||
"""
|
||||
lines = body.split("\n")
|
||||
out: list[str] = []
|
||||
in_block = False
|
||||
block_indent = None
|
||||
for line in lines:
|
||||
if not in_block:
|
||||
if line.strip() == "with _lock:":
|
||||
in_block = True
|
||||
continue
|
||||
# Determine block indent from the first non-empty line we see inside.
|
||||
if block_indent is None:
|
||||
stripped = line.lstrip(" ")
|
||||
if stripped == "":
|
||||
continue # blank lines don't set indent
|
||||
block_indent = len(line) - len(stripped)
|
||||
out.append(line)
|
||||
continue
|
||||
# Continuation: blank lines OK, otherwise must be at >= block_indent.
|
||||
if line.strip() == "":
|
||||
out.append(line)
|
||||
continue
|
||||
line_indent = len(line) - len(line.lstrip(" "))
|
||||
if line_indent >= block_indent:
|
||||
out.append(line)
|
||||
else:
|
||||
break
|
||||
return "\n".join(out)
|
||||
|
||||
|
||||
def _handler_body() -> str:
|
||||
start = ROUTES_SRC.find("def _handle_approval_sse_stream(")
|
||||
assert start != -1, "_handle_approval_sse_stream must exist"
|
||||
end = ROUTES_SRC.find("\ndef ", start + 1)
|
||||
return ROUTES_SRC[start:end if end != -1 else len(ROUTES_SRC)]
|
||||
|
||||
|
||||
def test_snapshot_taken_under_lock():
|
||||
"""The initial _pending snapshot must be guarded by `with _lock:`."""
|
||||
lock_body = _extract_lock_block(_handler_body())
|
||||
assert lock_body, "_handle_approval_sse_stream must contain a `with _lock:` block"
|
||||
assert "_pending.get(sid)" in lock_body, \
|
||||
"Initial snapshot of _pending must be read inside the `with _lock:` block"
|
||||
|
||||
|
||||
def test_subscriber_registered_inside_lock():
|
||||
"""The subscriber queue must be registered inside the same `with _lock:` block."""
|
||||
lock_body = _extract_lock_block(_handler_body())
|
||||
assert lock_body, "Handler must contain a `with _lock:` block"
|
||||
assert "_approval_sse_subscribers" in lock_body and "append(q)" in lock_body, \
|
||||
("Subscriber registration (`_approval_sse_subscribers.setdefault(sid, []).append(q)`) "
|
||||
"must happen inside the same `with _lock:` block as the snapshot. "
|
||||
"Otherwise a submit_pending() between snapshot-and-subscribe is lost.")
|
||||
|
||||
|
||||
def test_subscribe_before_snapshot_in_lock():
|
||||
"""Inside the lock, the subscriber must be registered BEFORE reading the snapshot."""
|
||||
lock_body = _extract_lock_block(_handler_body())
|
||||
assert lock_body, "Handler must contain a `with _lock:` block"
|
||||
|
||||
sub_idx = lock_body.find("_approval_sse_subscribers")
|
||||
snap_idx = lock_body.find("_pending.get(sid)")
|
||||
|
||||
assert sub_idx != -1, "Subscriber registration must be inside the lock"
|
||||
assert snap_idx != -1, "Snapshot read must be inside the lock"
|
||||
assert sub_idx < snap_idx, (
|
||||
"Subscriber registration must come BEFORE the snapshot read inside the lock. "
|
||||
"Otherwise an approval arriving between subscribe and snapshot is silently dropped."
|
||||
)
|
||||
|
||||
|
||||
def test_no_double_subscribe_outside_lock():
|
||||
"""The handler must not also call `_approval_sse_subscribe()` (legacy code path)."""
|
||||
body = _handler_body()
|
||||
assert "= _approval_sse_subscribe(sid)" not in body, (
|
||||
"_handle_approval_sse_stream must not call _approval_sse_subscribe() — "
|
||||
"the atomic version inlines subscribe inside the snapshot lock block."
|
||||
)
|
||||
Reference in New Issue
Block a user