fix: preserve terminal handoff across idle sidebar refresh

(cherry picked from commit 85f1c2d34b62b0ade85f51ed2cf8c1cf56f5ab9e)
This commit is contained in:
Frank Song committed 2026-09-23 11:57:31 +08:00
1 parent 6f14958bdc
commit 5dcda6f537
13 files changed
+348 -4

No files matched your search

@@ -0,0 +1,58 @@
# Sidebar idle / terminal handoff evidence
## Failure and scope
The independent `/api/sessions` response can report idle before the chat SSE's
terminal frame reaches the page. Three sidebar paths were retiring chat state:
idle reconciliation, INFLIGHT purging, and optimistic-row retirement. The idle
path clears the Worklog and active stream ID; the subsequent `done` is rejected
as stale, so no settled Anchor scene is persisted.
A real-HTTP barrier reproduces this on both published PR head
`75c8ff8d2a991f22389e3e4263ee03fb8d8f67a0` and frozen upstream
`b09fded7b8c98b549a49aaaece3721015337d307`. Applying only the `sessions.js`
product fix to the frozen upstream also passes the identical oracle: this is a
shared lifecycle defect, not evidence that artifact replay introduced it. The
historical hosted run lacks this instrumentation, so its exact interleaving
cannot be proved retrospectively; its empty-surface/no-persistence signature
matches the controlled reproduction.
## Reproduction
```sh
LIFECYCLE_SCENARIO=normal LIFECYCLE_TEST_BITE=settle-worklog-frame-proof \
.venv/bin/python tests/browser_conversation_lifecycle.py
./scripts/test.sh tests/test_sidebar_terminal_handoff.py -q
```
The existing frame proof now uses a test-only server wrapper to hold the actual
`done` HTTP write. The real worker finishes; the real browser receives an idle
sidebar response through `renderSessionList()` while its native EventSource
remains open. The test verifies pane, INFLIGHT and registry ownership, then
releases `done` and retains all existing painted-frame, final-answer, semantic
activity and hard-reload assertions. The fixture always releases/cleans up and
has a bounded wait. Normal production server startup is unchanged.
No renderer, terminal payload or persistence response is fabricated. Runtime
events come from the existing deterministic localhost Gateway fixture, not a
live model/provider. All state and files are disposable. These are browser
viewport checks, not physical-device acceptance.
## Before / after
Each screenshot is taken after the idle sidebar response, before the terminal
HTTP frame is released. The exact published source fails at both widths. The
patched source preserves three live activity rows and then passes completion
and hard reload. `results.json` records the predicate results without machine
paths or credentials.
| Viewport | Before | After |
| --- | --- | --- |
| 1440 x 900 | [Empty surface](before-1440.png) | [Owned handoff retained](after-1440.png) |
| 390 x 844 | [Empty surface](before-390.png) | [Owned handoff retained](after-390.png) |
The fix only defers these sidebar cleanups for the current pane's exact OPEN
chat transport. Missing, CONNECTING, CLOSED and mismatched sources still take
the existing stale recovery path. It does not add a separate recovery timer or
claim to redesign transport liveness; prolonged network failure remains the
responsibility of the existing stream error/recovery machinery.
Binary file not shown.

After

Width:  |  Height:  |  Size: 59 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 30 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 56 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 28 KiB

@@ -0,0 +1,66 @@
[
{
"mode": "before",
"viewport": [
1440,
900
],
"exit": 1,
"observation": {
"serverIdle": true,
"paneRetained": false,
"inflightRetained": false,
"sourceRetained": true,
"registryRetained": true,
"liveRows": 0
}
},
{
"mode": "before",
"viewport": [
390,
844
],
"exit": 1,
"observation": {
"serverIdle": true,
"paneRetained": false,
"inflightRetained": false,
"sourceRetained": true,
"registryRetained": true,
"liveRows": 0
}
},
{
"mode": "after",
"viewport": [
1440,
900
],
"exit": 0,
"observation": {
"serverIdle": true,
"paneRetained": true,
"inflightRetained": true,
"sourceRetained": true,
"registryRetained": true,
"liveRows": 3
}
},
{
"mode": "after",
"viewport": [
390,
844
],
"exit": 0,
"observation": {
"serverIdle": true,
"paneRetained": true,
"inflightRetained": true,
"sourceRetained": true,
"registryRetained": true,
"liveRows": 3
}
}
]
@@ -470,6 +470,11 @@ Expected behavior:
- A session row's running indicator should reflect a real active run or a
clearly restorable state, not stale persisted metadata alone.
- An idle sidebar response does not prove that the browser has consumed the
terminal chat frame. While the current pane still owns its exact OPEN chat
transport, list reconciliation and cache purging must preserve that handoff.
Missing, connecting, closed, or mismatched transports keep the normal stale
state recovery path; a cached stream ID or busy flag is insufficient.
- Background completion, cancellation, or failure should be represented without
stealing the visible pane from the user.
- Session switching should not erase pending live context, in-flight snapshots,
+16 -1
View File
@@ -852,6 +852,10 @@ function _reconcileActiveSessionIdleStateFromList(serverRows) {
const serverRow=serverRows.find(s=>s&&s.session_id===sid);
if (!serverRow) return false;
if (!_isServerIdleSessionRow(serverRow)) return false;
// Sidebar idle metadata can beat the terminal frame on the independent chat
// SSE. Let its exact OPEN transport finish the Anchor handoff; orphaned or
// disconnected streams still use the existing idle recovery below.
if (_hasOwnedOpenLiveStream(sid)) return false;
let changed=false;
if (S.busy) { S.busy=false; changed=true; }
if (S.activeStreamId) { S.activeStreamId=null; changed=true; }
@@ -940,6 +944,8 @@ function _purgeStaleInflightEntries() {
if (typeof _sendInProgress !== 'undefined' && _sendInProgress && sid === _sendInProgressSid) {
continue;
}
// The sidebar render must not purge what the idle reconciler preserved.
if (_hasOwnedOpenLiveStream(sid)) continue;
if (!sessionsById.has(sid)) {
const knownSource = sourceById ? sourceById.get(sid) : null;
if (currentSidebarSource && (!knownSource || knownSource !== currentSidebarSource)) {
@@ -961,6 +967,14 @@ function _purgeStaleInflightEntries() {
}
}
function _hasOwnedOpenLiveStream(sid) {
if (typeof S === 'undefined' || !S || !S.session || S.session.session_id !== sid || !S.activeStreamId) return false;
const live = typeof LIVE_STREAMS === 'object' && LIVE_STREAMS ? LIVE_STREAMS[sid] : null;
// readyState 1 is EventSource.OPEN. A cached stream ID or busy flag alone
// is not ownership and must never disable recovery for a stuck indicator.
return Boolean(live && live.streamId === S.activeStreamId && live.source && live.source.readyState === 1);
}
function _rememberSessionListSource(s, sid = null, allowScopeFallback = true) {
const resolvedSid = sid || (s && s.session_id);
if (!resolvedSid) return;
@@ -5278,7 +5292,8 @@ function _shouldKeepLocalOnlyOptimisticSessionRow(local){
function _dropStaleOptimisticSessionRow(sid){
if(!sid) return;
if(typeof _rememberSessionListSource==='function') _rememberSessionListSource(null, sid, false);
if(INFLIGHT&&INFLIGHT[sid]){
// Retiring sidebar optimism must not retire the independent chat owner.
if(INFLIGHT&&INFLIGHT[sid]&&!_hasOwnedOpenLiveStream(sid)){
delete INFLIGHT[sid];
if(typeof clearInflightState==='function') clearInflightState(sid);
}
+63 -1
View File
@@ -267,8 +267,17 @@ def _start_webui_server(repo_root: Path, env: dict, artifact_dir: Path):
suffix = "" if attempts == 1 else f"-attempt-{attempt + 1}"
log_path = artifact_dir / f"server{suffix}.log"
log = log_path.open("w", encoding="utf-8")
command = [sys.executable, str(repo_root / "server.py")]
barrier_dir = env.get("HERMES_TEST_TERMINAL_BARRIER_DIR")
if barrier_dir:
command = [
sys.executable,
str(repo_root / "tests" / "sidebar_terminal_barrier_server.py"),
str(repo_root),
barrier_dir,
]
proc = subprocess.Popen(
[sys.executable, str(repo_root / "server.py")],
command,
cwd=repo_root,
env=run_env,
stdout=log,
@@ -286,6 +295,49 @@ def _start_webui_server(repo_root: Path, env: dict, artifact_dir: Path):
raise RuntimeError(f"WebUI server did not become healthy{detail}")
def _exercise_sidebar_terminal_handoff(page, barrier_dir: Path, artifact_dir: Path):
"""Order the real idle-list response before the real terminal SSE write."""
deadline = time.monotonic() + 10
while not (barrier_dir / "ready").exists():
if time.monotonic() >= deadline:
raise AssertionError("terminal HTTP barrier was not reached")
page.wait_for_timeout(10)
observation = page.evaluate(
"""async () => {
const sid = S.session.session_id;
const stream = S.activeStreamId;
const inflight = INFLIGHT[sid];
const source = LIVE_STREAMS[sid] && LIVE_STREAMS[sid].source;
const registries = window._liveAnchorRegistries;
const registry = registries && registries.get(stream);
await renderSessionList();
await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve)));
const live = document.querySelector('#liveAssistantTurn');
return {
serverIdle: _allSessions.some(row => row.session_id === sid && _isServerIdleSessionRow(row)),
paneRetained: S.busy === true && S.activeStreamId === stream,
inflightRetained: !!inflight && INFLIGHT[sid] === inflight,
sourceRetained: !!source && source.readyState === EventSource.OPEN &&
LIVE_STREAMS[sid] && LIVE_STREAMS[sid].source === source,
registryRetained: !!registry && registries.get(stream) === registry,
liveRows: live ? live.querySelectorAll('[data-anchor-scene-row="1"]').length : 0,
};
}"""
)
(artifact_dir / "sidebar-before-terminal.json").write_text(
json.dumps(observation, indent=2), encoding="utf-8"
)
page.screenshot(path=str(artifact_dir / "sidebar-before-terminal.png"), full_page=True)
for invariant in ("serverIdle", "paneRetained", "inflightRetained", "sourceRetained", "registryRetained"):
assert observation[invariant] is True, {
"message": "idle sidebar response revoked the live terminal handoff",
"invariant": invariant,
"observation": observation,
}
assert observation["liveRows"] > 0, observation
(barrier_dir / "release").touch()
class DeterministicGateway:
"""A localhost-only Gateway Runs server with test-controlled phase gates."""
@@ -782,6 +834,12 @@ def main() -> int:
"NO_PROXY": "127.0.0.1,localhost",
"no_proxy": "127.0.0.1,localhost",
})
env.pop("HERMES_TEST_TERMINAL_BARRIER_DIR", None)
terminal_barrier_dir = None
if TEST_BITE == "settle-worklog-frame-proof" and scenario == "normal":
terminal_barrier_dir = state_dir / "terminal-barrier"
terminal_barrier_dir.mkdir()
env["HERMES_TEST_TERMINAL_BARRIER_DIR"] = str(terminal_barrier_dir)
proc = None
log = None
log_path = None
@@ -941,6 +999,8 @@ def main() -> int:
timeout=10000,
)
gateway.release_terminal.set()
if terminal_barrier_dir is not None:
_exercise_sidebar_terminal_handoff(page, terminal_barrier_dir, artifact_dir)
page.wait_for_function(
"""text => typeof S !== 'undefined' && S.busy === false && !S.activeStreamId &&
((document.querySelector('#msgInner') || {}).innerText || '').includes(text)""",
@@ -1176,6 +1236,8 @@ def main() -> int:
exit_code = 1
return 1
finally:
if terminal_barrier_dir is not None:
(terminal_barrier_dir / "release").touch()
gateway.close()
if browser is not None:
browser.close()
+40
View File
@@ -0,0 +1,40 @@
"""Test-only server for the public settle-frame gate's cross-channel race.
Hold the actual HTTP terminal frame after the worker has finished. The browser
keeps its native, open EventSource while /api/sessions independently reports idle.
No browser state, renderer, persistence result, or terminal payload is mocked.
"""
from pathlib import Path
import sys
import time
def main():
if len(sys.argv) != 3:
raise SystemExit("expected repository root and isolated barrier directory")
repo_root, barrier_dir = (Path(value) for value in sys.argv[1:])
sys.path.insert(0, str(repo_root))
import server
from api import routes
ready = barrier_dir / "ready"
release = barrier_dir / "release"
original = routes._sse_with_id
def terminal_barrier(handler, event, data, event_id=None):
if event == "done" and handler.path.startswith("/api/chat/stream?"):
ready.touch()
deadline = time.monotonic() + 15
while not release.exists():
if time.monotonic() >= deadline:
raise TimeoutError("test did not release the terminal HTTP frame")
time.sleep(0.005)
return original(handler, event, data, event_id)
routes._sse_with_id = terminal_barrier
server.main()
if __name__ == "__main__":
main()
@@ -183,6 +183,7 @@ def test_optimistic_merge_preserves_server_running_state_when_local_cache_is_sta
optimistic_body = _function_body(SESSIONS_SRC, "function _isOptimisticFirstTurnSessionRow(")
keep_body = _function_body(SESSIONS_SRC, "function _shouldKeepLocalOnlyOptimisticSessionRow(")
drop_body = _function_body(SESSIONS_SRC, "function _dropStaleOptimisticSessionRow(")
owner_body = _function_body(SESSIONS_SRC, "function _hasOwnedOpenLiveStream(")
merge_body = _function_body(SESSIONS_SRC, "function _mergeOptimisticFirstTurnSessions(")
script = f"""
@@ -206,6 +207,7 @@ function _isServerIdleSessionRow(s) {{{helper_body}}}
function _isSessionLocallyStreaming(s) {{{local_body}}}
function _isOptimisticFirstTurnSessionRow(s) {{{optimistic_body}}}
function _shouldKeepLocalOnlyOptimisticSessionRow(local) {{{keep_body}}}
function _hasOwnedOpenLiveStream(sid) {{{owner_body}}}
function _dropStaleOptimisticSessionRow(sid) {{{drop_body}}}
function _mergeOptimisticFirstTurnSessions(fetchedSessions) {{{merge_body}}}
const merged = _mergeOptimisticFirstTurnSessions([{{
@@ -536,7 +536,7 @@ def test_source_filtered_cache_preserves_hidden_bucket_runtime_state():
remember_source_fn = _extract_function(src, "_rememberSessionListSource")
remember_streaming_fn = _extract_function(src, "_rememberRenderedStreamingState")
remember_snapshot_fn = _extract_function(src, "_rememberRenderedSessionSnapshot")
purge_fn = _extract_function(src, "_purgeStaleInflightEntries")
purge_fn = _extract_function(src, "_hasOwnedOpenLiveStream") + "\n" + _extract_function(src, "_purgeStaleInflightEntries")
mark_fn = _extract_function(src, "_markPollingCompletionUnreadTransitions")
script = f"""
global._allSessions = [{{
@@ -654,7 +654,7 @@ def test_session_list_response_omits_bucket_counts_when_missing(monkeypatch):
@pytest.mark.skipif(NODE is None, reason="node not on PATH")
def test_scope_mismatch_error_path_respects_sidebar_source():
src = SESSIONS_JS.read_text(encoding="utf-8")
purge_fn = _extract_function(src, "_purgeStaleInflightEntries")
purge_fn = _extract_function(src, "_hasOwnedOpenLiveStream") + "\n" + _extract_function(src, "_purgeStaleInflightEntries")
clear_fn = _extract_function(src, "_clearSessionSourceTabCounts")
requested_source_fn = _extract_function(src, "_requestedSessionSidebarSource")
exclude_hidden_fn = _extract_function(src, "_sessionListExcludeHiddenEnabled")
+96
View File
@@ -0,0 +1,96 @@
"""Sidebar metadata must not retire a still-open, exact chat transport.
The live SSE and /api/sessions responses are independently scheduled. An idle
sidebar row can arrive before the terminal frame carrying the Anchor scene.
"""
import json
from pathlib import Path
import shutil
import subprocess
import pytest
from tests.test_issue2454_active_session_spinner import _function_body
ROOT = Path(__file__).resolve().parents[1]
def _run_case(operation, transport, *, absent=False, close_after=False):
source = (ROOT / "static/sessions.js").read_text(encoding="utf-8")
names = ["_isServerIdleSessionRow", "_reconcileActiveSessionIdleStateFromList", "_purgeStaleInflightEntries", "_dropStaleOptimisticSessionRow"]
if "function _hasOwnedOpenLiveStream(" in source:
names.append("_hasOwnedOpenLiveStream")
parameters = {
"_isServerIdleSessionRow": "s",
"_reconcileActiveSessionIdleStateFromList": "serverRows",
"_hasOwnedOpenLiveStream": "sid",
"_dropStaleOptimisticSessionRow": "sid",
}
functions = "\n".join(
f"function {name}({parameters.get(name, '')}) {{"
+ _function_body(source, f"function {name}(") + "}"
for name in names
)
script = """
const S={session:{session_id:'current',active_stream_id:'turn-1'},busy:true,activeStreamId:'turn-1'};
const INFLIGHT={current:{streamId:'turn-1',messages:[{role:'assistant',content:'work so far'}]}};
const original=INFLIGHT.current;
const LIVE_STREAMS={current:TRANSPORT};
const _sessionStreamingById=new Map();
const calls=[];
const idle={session_id:'current',is_streaming:false,active_stream_id:null,pending_user_message:null};
const _allSessions=ABSENT?[]:[idle];
function clearInflightState(){calls.push('clear');}
function _forgetObservedStreamingSession(){}
function hideApprovalCard(){}
function hideLiveRunStatus(){}
function clearLiveToolCards(){calls.push('clear-dom');}
function updateSendBtn(){}
function _scheduleActiveSessionIdleReload(){calls.push('reload');}
FUNCTIONS
function run(){if(OPERATION==='idle')_reconcileActiveSessionIdleStateFromList([idle]);else if(OPERATION==='optimistic')_dropStaleOptimisticSessionRow('current');else _purgeStaleInflightEntries();}
run();
const beforeClose={busy:S.busy,stream:S.activeStreamId,retained:INFLIGHT.current===original,calls:[...calls]};
if(CLOSE_AFTER){LIVE_STREAMS.current.source.readyState=2;run();}
console.log(JSON.stringify({beforeClose,afterClose:{retained:!!INFLIGHT.current,busy:S.busy}}));
"""
script = script.replace("TRANSPORT", json.dumps(transport)).replace("ABSENT", json.dumps(absent))
script = script.replace("FUNCTIONS", functions).replace("OPERATION", json.dumps(operation))
script = script.replace("CLOSE_AFTER", json.dumps(close_after))
node = shutil.which("node")
if not node:
pytest.skip("node required")
result = subprocess.run([node, "-e", script], text=True, capture_output=True, timeout=20)
assert result.returncode == 0, result.stderr
return json.loads(result.stdout)
@pytest.mark.parametrize("operation", ["idle", "purge", "optimistic"])
def test_current_open_transport_keeps_terminal_handoff_and_recovers_after_close(operation):
result = _run_case(operation, {"streamId": "turn-1", "source": {"readyState": 1}}, close_after=True)
assert result["beforeClose"] == {"busy": True, "stream": "turn-1", "retained": True, "calls": []}
assert result["afterClose"]["retained"] is False
if operation == "idle":
assert result["afterClose"]["busy"] is False
@pytest.mark.parametrize("operation", ["idle", "purge", "optimistic"])
@pytest.mark.parametrize("transport", [
None,
{"streamId": "turn-1", "source": None},
{"streamId": "turn-1", "source": {"readyState": 0}},
{"streamId": "turn-1", "source": {"readyState": 2}},
{"streamId": "older-turn", "source": {"readyState": 1}},
])
def test_missing_closed_connecting_or_wrong_transport_does_not_block_idle_recovery(operation, transport):
result = _run_case(operation, transport)
assert result["beforeClose"]["retained"] is False
if operation == "idle":
assert result["beforeClose"]["busy"] is False
assert "reload" in result["beforeClose"]["calls"]
def test_current_open_transport_survives_a_filtered_sidebar_list():
result = _run_case("purge", {"streamId": "turn-1", "source": {"readyState": 1}}, absent=True)
assert result["beforeClose"]["retained"] is True