From 49437e20abc2128aa5d4143ca307720361d6be03 Mon Sep 17 00:00:00 2001 From: happy5318 <34227488+happy5318@users.noreply.github.com> Date: Sun, 9 Aug 2026 06:10:04 +0800 Subject: [PATCH] fix: phantom 'Compressing context' barrier when compressed SSE is lost/delayed (#6572) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: clear stale compression barrier on turn-done when compressed SSE is lost When a turn completes (done SSE event) but the "compressed" SSE event was lost or delayed, the compression UI state remained in phase="running". The done handler unconditionally rebound the stale state sessionId to the continuation session, surfacing a phantom "Compressing context" barrier on a session that never actually compressed. Fix: when the done event session_id differs from the active session AND the compression UI is still in "running" phase, clear it via clearCompressionUi() instead of rebinding. Non-running states (e.g. "done") are still rebound as before. Added regression tests verifying: - stale running state is cleared (not rebound) on session change - non-running states are still rebound - the old unconditional rebind is gone * fix: also clear phantom compression on same-session done (A->A case) Reviewer feedback on PR #6572: the previous fix only cleared stale running compression when the session ID changed (A->B). But the backend also supports compression without session rotation (A->A), where the compressed SSE being lost leaves the same phantom barrier. Fix: in the done handler, clear any owned automatic compression state that is still in "running" phase, regardless of whether the session ID changed. The condition is now phase==='running' alone, not session_id change AND running. Added behavioral state-transition tests for all four cases: - running A->B (rotation, compressed lost): cleared - running A->A (no rotation, compressed lost): cleared - non-running A->B (rotation, compressed seen): rebound - non-running A->A (no rotation, compressed seen): rebound * test: real SSE regression for compression phantom barrier Rewrite test_compression_phantom_barrier.py to use real browser + SSE instead of handwritten Python simulation. The new tests: - Boot real WebUI server with isolated state - Use deterministic mock gateway (Runs API mode) - Verify done handler clears stale running compression state - Cover both A→B (rotation) and A→A (no rotation) cases Key insight: /v1/capabilities must return run_approval_response=true to enable the Runs API path in gateway_chat.py. Fixes reviewer feedback: simulation never called attachLiveStream, clearCompressionUi, or registered done listeners. * test: replace Playwright test with hermetic source-grep suite for compression phantom barrier The previous Playwright-based test failed CI in no-egress sandboxes due to external resource errors (net::ERR_INTERNET_DISCONNECTED) and was structurally unsound: the mock window.S shadowed the lexical S, so the compressing handler short-circuited before the barrier could be established, producing a false- positive assertion. New suite uses static source analysis (same convention as test_issue840, test_issue3040, test_bg_task_complete_focus_drop): - No browser, no network, no playwright dependency - Proves the done handler clears phase:'running' without gating on session ID change (A->A case coverage) - Asserts gating conditions: automatic && owner && done session exists - Negative controls: non-owner / manual compression are ignored - Source-level regression proof: reverting the production hunk breaks tests 13 tests. All pass in <3s on bare Python. * test: exercise compression cleanup through real SSE handlers Replace source-grep assertions with a hermetic Playwright harness that loads the shipping classic scripts and dispatches MockEventSource events through attachLiveStream. Cover A-to-A/A-to-B cleanup, non-running rebind, ownership controls, canonical cleanup side effects, and a production-branch mutation proof. --------- Co-authored-by: happy5318 Co-authored-by: nesquena-hermes --- static/messages.js | 14 +- tests/test_compression_phantom_barrier.py | 298 ++++++++++++++++++++++ 2 files changed, 311 insertions(+), 1 deletion(-) create mode 100644 tests/test_compression_phantom_barrier.py diff --git a/static/messages.js b/static/messages.js index aec6e11c..8142a714 100644 --- a/static/messages.js +++ b/static/messages.js @@ -5940,7 +5940,19 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ window._compressionUi.sessionId===activeSid&& d.session&&d.session.session_id ){ - window._compressionUi={...window._compressionUi, sessionId:d.session.session_id}; + if(window._compressionUi.phase==='running'){ + // Turn completed (done event) but the compression UI is still in + // 'running' phase - the 'compressed' SSE event was lost or delayed. + // Clear the stale running state instead of leaving it active, + // which would surface a phantom "Compressing context" barrier. + // This covers both A->B (session rotation) and A->A (no rotation) + // since in both cases a running phase at done-time means the + // compressed event never arrived. + if(typeof clearCompressionUi==='function') clearCompressionUi(); + else window._compressionUi=null; + } else { + window._compressionUi={...window._compressionUi, sessionId:d.session.session_id}; + } } // Find the last assistant message once for both reasoning persistence and timestamp lastAsst=[...S.messages].reverse().find(m=>m.role==='assistant'); diff --git a/tests/test_compression_phantom_barrier.py b/tests/test_compression_phantom_barrier.py new file mode 100644 index 00000000..c1cf9038 --- /dev/null +++ b/tests/test_compression_phantom_barrier.py @@ -0,0 +1,298 @@ +"""Behavioral regression tests for the phantom compression barrier. + +The harness loads the shipping classic scripts in a real browser page, replaces +only the SSE transport, and dispatches events through attachLiveStream's real +listeners. All HTTP requests are fulfilled in-process; no WebUI server or +network access is used. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest +from playwright.sync_api import sync_playwright + +ROOT = Path(__file__).resolve().parents[1] +STATIC = ROOT / "static" +INDEX_HTML = (STATIC / "index.html").read_text(encoding="utf-8") +UI_JS = (STATIC / "ui.js").read_text(encoding="utf-8") +MESSAGES_JS = (STATIC / "messages.js").read_text(encoding="utf-8") +SUPPORT_SCRIPTS = [ + (STATIC / name).read_text(encoding="utf-8") + for name in ("i18n.js", "icons.js", "assistant_turn_anchors.js") +] + +def _matching_brace(source: str, opening: int) -> int: + """Return the closing brace paired with ``opening``.""" + depth = 0 + for index in range(opening, len(source)): + if source[index] == "{": + depth += 1 + elif source[index] == "}": + depth -= 1 + if depth == 0: + return index + raise AssertionError("unclosed JavaScript block in mutation target") + + +def _revert_running_cleanup(source: str) -> str: + """Revert the reviewed phase branch inside the production done listener.""" + done_start = source.index("source.addEventListener('done',e=>{") + done_end = source.index("source.addEventListener('stream_end'", done_start) + done_handler = source[done_start:done_end] + phase_match = re.search( + r"if\s*\(\s*window\._compressionUi\.phase\s*===\s*['\"]running['\"]\s*\)\s*\{", + done_handler, + ) + assert phase_match, "running cleanup branch missing from production done listener" + + branch_start = done_start + phase_match.start() + if_open = source.index("{", done_start + phase_match.start(), done_start + phase_match.end()) + if_close = _matching_brace(source, if_open) + else_match = re.match(r"\s*else\s*\{", source[if_close + 1 :]) + assert else_match, "running cleanup branch no longer has its rebind control" + else_open = if_close + 1 + else_match.end() - 1 + else_close = _matching_brace(source, else_open) + + old_branch = source[branch_start : else_close + 1] + assert "clearCompressionUi()" in old_branch + assert "sessionId:d.session.session_id" in old_branch.replace(" ", "") + reverted = "window._compressionUi={...window._compressionUi, sessionId:d.session.session_id};" + return source[:branch_start] + reverted + source[else_close + 1 :] + + +# Revert only the fix under review to its pre-fix unconditional rebind. The +# ownership/automatic guard remains intact, so controls must behave identically. +MUTATED_MESSAGES_JS = _revert_running_cleanup(MESSAGES_JS) + +# The real page DOM is used, but its deferred script/resource tags are removed: +# scripts are injected explicitly below and every request is fulfilled locally. +HARNESS_HTML = re.sub(r"]*>.*?", "", INDEX_HTML, flags=re.I | re.S) +HARNESS_HTML = re.sub(r"]*>", "", HARNESS_HTML, flags=re.I) + +_MOCK_EVENT_SOURCE = r""" +class MockEventSource { + static CONNECTING = 0; + static OPEN = 1; + static CLOSED = 2; + static instances = []; + constructor(url) { + this.url = String(url); + this.readyState = MockEventSource.CONNECTING; + this.listeners = new Map(); + MockEventSource.instances.push(this); + } + addEventListener(name, listener) { + const listeners = this.listeners.get(name) || []; + listeners.push(listener); + this.listeners.set(name, listeners); + } + removeEventListener(name, listener) { + this.listeners.set(name, (this.listeners.get(name) || []).filter(fn => fn !== listener)); + } + emit(name, payload) { + const event = {type: name, data: JSON.stringify(payload)}; + for (const listener of this.listeners.get(name) || []) listener(event); + } + close() { this.readyState = MockEventSource.CLOSED; } +} +window.EventSource = MockEventSource; +window.MockEventSource = MockEventSource; +""" + +# These are unrelated terminal-event surfaces. Compression state, timer, lock, +# placeholder, rendering, attachLiveStream, and both SSE listeners remain the +# production implementations. +_STABILIZE_UNRELATED_UI = r""" +appendLiveCompressionCard = () => false; +resetTurnWorkspaceMutations = () => {}; +_resetStreamScrollFollow = () => {}; +ensureLiveWorklogShell = () => {}; +_shouldFollowMessagesOnDomReplace = () => false; +_isMessagePaneNearBottom = () => false; +_markSessionViewed = () => {}; +setBusy = () => {}; +setComposerStatus = () => {}; +setStatus = () => {}; +syncTopbar = () => {}; +renderMessages = () => {}; +renderSessionList = () => {}; +loadDir = () => {}; +playNotificationSound = () => {}; +sendBrowserNotification = () => {}; +api = async () => ({}); +""" + +_SCENARIO = r""" +async ({kind, doneSid}) => { + const activeSid = 'session-a'; + const streamId = 'stream-under-test'; + S.session = {session_id: activeSid, messages: []}; + S.messages = []; + S.toolCalls = []; + S.activeStreamId = streamId; + S.busy = true; + window._compressionUi = null; + window._compressionLockSid = null; + + const input = document.getElementById('msg'); + const cards = document.getElementById('liveCompressionCards'); + input.placeholder = 'Original placeholder'; + cards.innerHTML = ''; + cards.style.display = ''; + + attachLiveStream(activeSid, streamId, []); + const source = MockEventSource.instances.at(-1); + if (!source) throw new Error('attachLiveStream did not construct EventSource'); + if (!source.listeners.has('compressing') || !source.listeners.has('done')) { + throw new Error('production compressing/done listeners were not registered'); + } + + if (kind === 'running') { + source.emit('compressing', {session_id: activeSid}); + } else if (kind === 'non-running') { + setCompressionUi({sessionId: activeSid, phase: 'done', automatic: true}); + } else if (kind === 'manual') { + setCompressionUi({sessionId: activeSid, phase: 'running', automatic: false}); + } else if (kind === 'unowned') { + setCompressionUi({phase: 'running', automatic: true}); + } else if (kind === 'stale-owner') { + setCompressionUi({sessionId: 'session-stale', phase: 'running', automatic: true}); + } else { + throw new Error(`unknown scenario: ${kind}`); + } + + const before = { + state: window._compressionUi ? {...window._compressionUi} : null, + timerRunning: _compressionElapsedTimer !== null, + lock: _compressionSessionLock(), + placeholder: input.placeholder, + }; + + // A real clearCompressionUi call must remove this stale rendered surface. + cards.innerHTML = 'stale'; + cards.style.display = ''; + + source.emit('done', { + session: { + session_id: doneSid, + messages: [{role: 'assistant', content: 'complete'}], + tool_calls: [], + }, + stream_id: streamId, + }); + + const after = { + state: window._compressionUi ? {...window._compressionUi} : null, + timerRunning: _compressionElapsedTimer !== null, + lock: _compressionSessionLock(), + placeholder: input.placeholder, + cardsHtml: cards.innerHTML, + cardsDisplay: cards.style.display, + currentSid: S.session && S.session.session_id, + }; + return {before, after}; +} +""" + + +@pytest.fixture(scope="module") +def browser(): + with sync_playwright() as playwright: + instance = playwright.chromium.launch( + headless=True, + args=["--no-sandbox", "--disable-dev-shm-usage"], + ) + yield instance + instance.close() + + +def _run(browser, kind: str, done_sid: str, *, mutated: bool = False) -> dict: + page = browser.new_page() + page.route( + "**/*", + lambda route: route.fulfill( + status=200, + content_type="text/html" if route.request.url == "http://harness.test/" else "text/plain", + body=HARNESS_HTML if route.request.url == "http://harness.test/" else "", + ), + ) + page.add_init_script(_MOCK_EVENT_SOURCE) + try: + page.goto("http://harness.test/", wait_until="domcontentloaded") + for script in SUPPORT_SCRIPTS: + page.add_script_tag(content=script) + page.add_script_tag(content=UI_JS) + page.add_script_tag(content=MUTATED_MESSAGES_JS if mutated else MESSAGES_JS) + page.evaluate(_STABILIZE_UNRELATED_UI) + return page.evaluate(_SCENARIO, {"kind": kind, "doneSid": done_sid}) + finally: + page.close() + + +def _assert_running_prerequisite(result: dict) -> None: + before = result["before"] + assert before["state"]["sessionId"] == "session-a" + assert before["state"]["automatic"] is True + assert before["state"]["phase"] == "running" + assert before["timerRunning"] is True + assert before["lock"] == "session-a" + assert before["placeholder"] == "Type a message — it will queue and send after compression" + + +def _assert_canonical_clear(result: dict, done_sid: str) -> None: + after = result["after"] + assert after == { + "state": None, + "timerRunning": False, + "lock": None, + "placeholder": "Original placeholder", + "cardsHtml": "", + "cardsDisplay": "none", + "currentSid": done_sid, + } + + +@pytest.mark.parametrize("done_sid", ["session-a", "session-b"], ids=["A-to-A", "A-to-B"]) +def test_owned_running_state_is_cleared_through_real_sse_path(browser, done_sid): + result = _run(browser, "running", done_sid) + _assert_running_prerequisite(result) + _assert_canonical_clear(result, done_sid) + + +@pytest.mark.parametrize("done_sid", ["session-a", "session-b"], ids=["A-to-A", "A-to-B"]) +def test_non_running_state_is_rebound_through_real_done_listener(browser, done_sid): + result = _run(browser, "non-running", done_sid) + assert result["before"]["state"]["phase"] == "done" + assert result["after"]["state"]["phase"] == "done" + assert result["after"]["state"]["sessionId"] == done_sid + + +@pytest.mark.parametrize("kind", ["manual", "unowned", "stale-owner"]) +def test_unowned_compression_controls_are_untouched(browser, kind): + result = _run(browser, kind, "session-b") + assert result["after"]["state"] == result["before"]["state"] + + +def test_reverting_cleanup_branch_breaks_owned_running_only(browser): + for done_sid in ("session-a", "session-b"): + result = _run(browser, "running", done_sid, mutated=True) + _assert_running_prerequisite(result) + with pytest.raises(AssertionError): + _assert_canonical_clear(result, done_sid) + assert result["after"]["state"]["phase"] == "running" + assert result["after"]["state"]["sessionId"] == done_sid + assert result["after"]["timerRunning"] is True + assert result["after"]["lock"] == "session-a" + assert result["after"]["cardsHtml"] != "" + + for done_sid in ("session-a", "session-b"): + result = _run(browser, "non-running", done_sid, mutated=True) + assert result["after"]["state"]["phase"] == "done" + assert result["after"]["state"]["sessionId"] == done_sid + + for kind in ("manual", "unowned", "stale-owner"): + result = _run(browser, kind, "session-b", mutated=True) + assert result["after"]["state"] == result["before"]["state"]