* 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 <happy5318@users.noreply.github.com> Co-authored-by: nesquena-hermes <nesquena+hermes@gmail.com>
299 lines
11 KiB
Python
299 lines
11 KiB
Python
"""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"<script\b[^>]*>.*?</script>", "", INDEX_HTML, flags=re.I | re.S)
|
|
HARNESS_HTML = re.sub(r"<link\b[^>]*>", "", 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 = '<span data-stale-compression>stale</span>';
|
|
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"]
|