fix(chat): classify interrupted response causes

(cherry picked from commit 5c1e802cd6ee8565da74c7ffe57e6407fe21bf02)
This commit is contained in:
ai-ag2026
2026-05-22 11:08:08 +02:00
parent efe3d7c296
commit 2f1ca959f1
10 changed files with 200 additions and 23 deletions

View File

@@ -5,7 +5,7 @@
### Fixed
- Clarify `Response interrupted` recovery markers so they report that the live response stream stopped instead of asserting that the WebUI process restarted. The same stale-recovery path also covers browser/SSE disconnects and lost worker bookkeeping, so the marker now matches systemd evidence instead of implying a restart that did not happen.
- Clarify `Response interrupted` recovery markers so they report that the live response stream stopped instead of asserting that the WebUI process restarted. The recovery path now records distinct interruption causes for real process restarts, stream/run split-brain, and lost worker bookkeeping; browser-side SSE transport failures show a separate `Connection interrupted` message, and client-side `BrokenPipeError` disconnects no longer get logged as server 500s.
## [v0.51.131] — 2026-05-24 — Release DC (stage-batch13 — 6-PR notes-drawer + context-parity + PWA-swipe + locale polish)

View File

@@ -787,11 +787,87 @@ _INTERRUPTED_NEUTRAL_WORDING = (
'Partial output may have been lost.'
)
_INTERRUPTION_CAUSE_DETAILS = {
'process_restart': (
'Evidence: the WebUI process started after this turn began, so this '
'looks like a real process crash or restart.'
),
'stream_run_split_brain': (
'Evidence: the browser response stream was gone but the worker registry '
'still listed the run. This is a stream/run bookkeeping split-brain.'
),
'lost_worker_bookkeeping': (
'Evidence: the stream was gone and worker bookkeeping no longer had an '
'active run for it. This usually means the worker state was lost or '
'cleaned up without a terminal event.'
),
'unknown': (
'Evidence: the stream stopped, but the WebUI could not classify the '
'interruption more precisely.'
),
}
def _classify_interruption_cause(
*, stream_id: str | None = None, pending_started_at=None,
) -> str:
"""Classify the stale live-response state without overstating certainty."""
try:
started = float(pending_started_at) if pending_started_at else None
except (TypeError, ValueError):
started = None
if started is not None:
try:
if float(getattr(_cfg, 'SERVER_START_TIME', 0.0) or 0.0) > started:
return 'process_restart'
except (TypeError, ValueError):
pass
if stream_id:
try:
with _cfg.ACTIVE_RUNS_LOCK:
if str(stream_id) in _cfg.ACTIVE_RUNS:
return 'stream_run_split_brain'
except Exception:
pass
return 'lost_worker_bookkeeping'
return 'unknown'
def _interrupted_content_for(
*, recovered_output: bool, pending_retry: bool, interruption_cause: str,
) -> str:
if recovered_output:
outcome = (
'The partial output above was recovered from the run journal, '
'but the interrupted agent process could not continue.'
)
elif pending_retry:
outcome = (
'Recovering the partial output from the run journal — '
'reload this session to retry.'
)
else:
outcome = 'The user message above was preserved, but no agent output was recovered.'
cause_detail = _INTERRUPTION_CAUSE_DETAILS.get(
interruption_cause,
_INTERRUPTION_CAUSE_DETAILS['unknown'],
)
return (
'**Response interrupted.**\n\n'
'The live response stream stopped before this turn finished. '
f'{cause_detail} {outcome}'
)
def _interrupted_recovery_marker(
*,
recovered_output: bool = False,
pending_retry: bool = False,
stream_id: str | None = None,
pending_started_at=None,
) -> dict:
"""Build the standard interrupted-turn marker.
@@ -809,18 +885,22 @@ def _interrupted_recovery_marker(
set so the caller cannot accidentally re-arm retry on a successful
repair.
"""
if recovered_output:
content = _INTERRUPTED_RECOVERED_WORDING
elif pending_retry:
content = _INTERRUPTED_PENDING_RETRY_WORDING
else:
content = _INTERRUPTED_NO_OUTPUT_WORDING
interruption_cause = _classify_interruption_cause(
stream_id=stream_id,
pending_started_at=pending_started_at,
)
content = _interrupted_content_for(
recovered_output=recovered_output,
pending_retry=pending_retry,
interruption_cause=interruption_cause,
)
marker = {
'role': 'assistant',
'content': content,
'timestamp': int(time.time()),
'_error': True,
'type': 'interrupted',
'interruption_cause': interruption_cause,
}
if pending_retry and not recovered_output:
marker['_pending_journal_recovery'] = True
@@ -1218,15 +1298,26 @@ def _journal_retry_lock_for_sid(sid: str) -> threading.Lock:
def _build_recovery_marker_with_retry_hook(
*, recovered_output: bool, stream_id: str | None,
*, recovered_output: bool, stream_id: str | None, pending_started_at=None,
) -> dict:
"""Build an interrupted-turn marker, arming the lazy-retry hook when
visible output was not recovered yet but a stream id is available."""
if recovered_output:
return _interrupted_recovery_marker(recovered_output=True)
return _interrupted_recovery_marker(
recovered_output=True,
stream_id=stream_id,
pending_started_at=pending_started_at,
)
if not stream_id:
return _interrupted_recovery_marker(recovered_output=False)
marker = _interrupted_recovery_marker(pending_retry=True)
return _interrupted_recovery_marker(
recovered_output=False,
pending_started_at=pending_started_at,
)
marker = _interrupted_recovery_marker(
pending_retry=True,
stream_id=stream_id,
pending_started_at=pending_started_at,
)
marker['_journal_retry_stream_id'] = str(stream_id)
marker['_journal_retry_attempts'] = 0
marker['_journal_retry_first_seen_ts'] = int(time.time())
@@ -1511,13 +1602,16 @@ def _apply_core_sync_or_error_marker(
stream_id_for_recheck or session.active_stream_id,
)
_stream_id = stream_id_for_recheck or session.active_stream_id
_pending_started_at = session.pending_started_at
session.active_stream_id = None
session.pending_user_message = None
session.pending_attachments = []
session.pending_started_at = None
session.messages.append(
_build_recovery_marker_with_retry_hook(
recovered_output=recovered_output, stream_id=_stream_id,
recovered_output=recovered_output,
stream_id=_stream_id,
pending_started_at=_pending_started_at,
)
)
session.save(touch_updated_at=touch_updated_at)
@@ -1562,13 +1656,18 @@ def _apply_core_sync_or_error_marker(
_stream_id,
dedupe_existing=True,
)
_pending_started_at = session.pending_started_at
session.active_stream_id = None
session.pending_user_message = None
session.pending_attachments = []
session.pending_started_at = None
if recovered_output:
session.messages.append(
_interrupted_recovery_marker(recovered_output=True)
_interrupted_recovery_marker(
recovered_output=True,
stream_id=_stream_id,
pending_started_at=_pending_started_at,
)
)
# NOTE: when the core transcript was synced in but the run journal
# is not yet visible, intentionally do NOT append a lazy-retry
@@ -1604,13 +1703,16 @@ def _apply_core_sync_or_error_marker(
stream_id_for_recheck or session.active_stream_id,
)
_stream_id = stream_id_for_recheck or session.active_stream_id
_pending_started_at = session.pending_started_at
session.active_stream_id = None
session.pending_user_message = None
session.pending_attachments = []
session.pending_started_at = None
session.messages.append(
_build_recovery_marker_with_retry_hook(
recovered_output=recovered_output, stream_id=_stream_id,
recovered_output=recovered_output,
stream_id=_stream_id,
pending_started_at=_pending_started_at,
)
)
session.save(touch_updated_at=touch_updated_at)

View File

@@ -1126,7 +1126,7 @@ def _run_journal_status_payload(summary: dict, *, active: bool = False) -> dict:
terminal = bool(summary.get("terminal"))
terminal_state = summary.get("terminal_state")
if not active and not terminal:
terminal_state = "stale-from-restart"
terminal_state = "lost-worker-bookkeeping"
return {
"session_id": summary.get("session_id"),
"run_id": summary.get("run_id"),

View File

@@ -278,7 +278,7 @@ def stale_interrupted_event(session_id: str, run_id: str, *, after_seq: int | No
"type": "apperror",
"created_at": time.time(),
"terminal": True,
"terminal_state": "stale-from-restart",
"terminal_state": "lost-worker-bookkeeping",
"payload": payload,
"synthetic": True,
}

View File

@@ -87,9 +87,16 @@ If after running steps 1-4 the import still fails *and* `pip install -e .` succe
## "Response interrupted." marker keeps saying "no agent output was recovered"
**Symptom.** After a live response stream stops before a turn completes (manual restart, OOM, crash, browser/SSE disconnect, lost worker bookkeeping, …), the affected chat shows an `**Response interrupted.**` marker with the wording *"The user message above was preserved, but no agent output was recovered."*, even though the run-journal for that turn is present on disk and contains the partial tokens the agent had already streamed.
**Symptom.** After a live response stream stops before a turn completes (manual restart, OOM, crash, browser/SSE disconnect, lost worker bookkeeping, …), the affected chat shows an `**Response interrupted.**` marker. If the run-journal for that turn is already visible on disk, the marker says the partial output was recovered; if not, it preserves the user turn and says no agent output was recovered yet.
**Why.** Sidecar repair re-checks the run-journal after it detects a stale stream and uses the result as a one-shot signal. On WSL2 (9p / DrvFs) and on some network-backed setups, the run-journal `.jsonl` is written by the stopped worker but the WebUI process reads it through a page-cache state that has not yet seen those writes — recovery returns "empty" and the marker is baked permanently. The fix introduces a *lazy* retry path: when sidecar repair cannot read visible output but knows the stream id, it stores a `_pending_journal_recovery` flag on the marker and re-attempts recovery from `get_session()` until the journal becomes readable (or the retry budget is exhausted).
**Why.** Sidecar repair re-checks the run-journal after it detects a stale stream and uses the result as a one-shot signal. On WSL2 (9p / DrvFs) and on some network-backed setups, the run-journal `.jsonl` is written by the stopped worker but the WebUI process reads it through a page-cache state that has not yet seen those writes — recovery returns "empty" and the marker would otherwise be baked permanently. The fix introduces a *lazy* retry path: when sidecar repair cannot read visible output but knows the stream id, it stores a `_pending_journal_recovery` flag on the marker and re-attempts recovery from `get_session()` until the journal becomes readable (or the retry budget is exhausted).
**Interruption classes.** The WebUI now keeps the user-facing cases separate instead of implying every stale stream was a restart:
- **Browser/SSE connection interrupted** — the live browser `EventSource` transport dropped. The UI reports `Connection interrupted` and tries status/replay/session restore before showing the final browser-side notice.
- **Lost worker bookkeeping** — the stream id is gone and the worker registry no longer has an active run. Recovery markers carry `interruption_cause: "lost_worker_bookkeeping"` and `/api/chat/stream/status` reports `terminal_state: "lost-worker-bookkeeping"` for non-terminal journals that are no longer active.
- **Stream/run split-brain** — the stream is gone but `ACTIVE_RUNS` still lists the worker. Recovery markers carry `interruption_cause: "stream_run_split_brain"` so the transcript says this is a bookkeeping split-brain rather than a restart.
- **Process crash/restart** — `SERVER_START_TIME` is newer than `pending_started_at`, meaning the WebUI process started after the turn began. Recovery markers carry `interruption_cause: "process_restart"` and explicitly say the process-start evidence points to a crash or restart.
**Diagnostic.**

View File

@@ -258,6 +258,11 @@ class Handler(BaseHTTPRequestHandler):
result = handle_get(self, parsed)
if result is False:
return j(self, {'error': 'not found'}, status=404)
except (BrokenPipeError, ConnectionResetError, ConnectionAbortedError):
# The browser/client closed the socket while we were writing the
# response. This is expected for probes, tab closes, and SSE
# reconnect races; do not convert it into a misleading server 500.
return
except Exception as e:
print(f'[webui] ERROR {self.command} {self.path}\n' + traceback.format_exc(), flush=True)
return j(self, {'error': 'Internal server error'}, status=500)
@@ -284,6 +289,11 @@ class Handler(BaseHTTPRequestHandler):
result = route_func(self, parsed)
if result is False:
return j(self, {'error': 'not found'}, status=404)
except (BrokenPipeError, ConnectionResetError, ConnectionAbortedError):
# The browser/client closed the socket while we were writing the
# response. This is expected for probes, tab closes, and SSE
# reconnect races; do not convert it into a misleading server 500.
return
except Exception as e:
print(f'[webui] ERROR {self.command} {self.path}\n' + traceback.format_exc(), flush=True)
return j(self, {'error': 'Internal server error'}, status=500)

View File

@@ -2245,12 +2245,12 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
if(S.session&&S.session.session_id===activeSid){
S.activeStreamId=null;
clearLiveToolCards();if(!assistantText)removeThinking();
S.messages.push({role:'assistant',content:'**Error:** Connection lost'});renderMessages({preserveScroll:true});
S.messages.push({role:'assistant',content:'**Connection interrupted:** The browser lost the live SSE connection before the response finished. If the worker completed, reopening this session should restore the settled transcript.'});renderMessages({preserveScroll:true});
_markSessionViewed(activeSid, S.messages.length);
}else{
if(typeof trackBackgroundError==='function'){
const _errTitle=(typeof _allSessions!=='undefined'&&_allSessions.find(s=>s.session_id===activeSid)||{}).title||null;
trackBackgroundError(activeSid,_errTitle,'Connection lost');
trackBackgroundError(activeSid,_errTitle,'Connection interrupted');
}
}
_setActivePaneIdleIfOwner();

View File

@@ -116,7 +116,7 @@ def test_stale_interrupted_event_reports_non_terminal_journal(tmp_path, monkeypa
assert event["event"] == "apperror"
assert event["seq"] == 2
assert event["terminal_state"] == "stale-from-restart"
assert event["terminal_state"] == "lost-worker-bookkeeping"
assert event["payload"]["type"] == "interrupted"
assert "last journaled event" in event["payload"]["hint"]
assert "process restarted" not in event["payload"]["message"]

View File

@@ -40,7 +40,7 @@ def test_replay_emits_event_ids_and_stale_restart_diagnostic():
def test_session_payload_exposes_runtime_journal_for_stale_streams():
assert "original_stream_id = getattr(s, \"active_stream_id\", None)" in ROUTES_SRC
assert '"runtime_journal"' in ROUTES_SRC
assert 'terminal_state = "stale-from-restart"' in ROUTES_SRC
assert 'terminal_state = "lost-worker-bookkeeping"' in ROUTES_SRC
def test_status_payload_marks_non_terminal_dead_journal_as_stale():
@@ -60,7 +60,7 @@ def test_status_payload_marks_non_terminal_dead_journal_as_stale():
)
assert payload["terminal"] is False
assert payload["terminal_state"] == "stale-from-restart"
assert payload["terminal_state"] == "lost-worker-bookkeeping"
assert payload["last_event_id"] == "run_1:3"

View File

@@ -53,11 +53,13 @@ def _isolate_stream_state():
config.CANCEL_FLAGS.clear()
config.AGENT_INSTANCES.clear()
config.STREAM_PARTIAL_TEXT.clear()
config.ACTIVE_RUNS.clear()
yield
config.STREAMS.clear()
config.CANCEL_FLAGS.clear()
config.AGENT_INSTANCES.clear()
config.STREAM_PARTIAL_TEXT.clear()
config.ACTIVE_RUNS.clear()
@pytest.fixture(autouse=True)
@@ -272,6 +274,62 @@ def test_interrupted_recovery_markers_do_not_claim_restart_as_fact():
assert "before this turn finished" in text
def test_interrupted_marker_distinguishes_real_process_restart(monkeypatch):
monkeypatch.setattr(config, "SERVER_START_TIME", 2000.0)
marker = models._interrupted_recovery_marker(
recovered_output=False,
stream_id="stream_crash",
pending_started_at=1000.0,
)
assert marker["interruption_cause"] == "process_restart"
assert "WebUI process started after this turn began" in marker["content"]
assert "process restarted" not in marker["content"]
def test_interrupted_marker_distinguishes_stream_run_split_brain(monkeypatch):
monkeypatch.setattr(config, "SERVER_START_TIME", 1000.0)
config.ACTIVE_RUNS["stream_split"] = {"session_id": "sid", "phase": "running"}
marker = models._interrupted_recovery_marker(
recovered_output=False,
stream_id="stream_split",
pending_started_at=2000.0,
)
assert marker["interruption_cause"] == "stream_run_split_brain"
assert "stream was gone but the worker registry still listed the run" in marker["content"]
def test_interrupted_marker_distinguishes_lost_worker_bookkeeping(monkeypatch):
monkeypatch.setattr(config, "SERVER_START_TIME", 1000.0)
marker = models._interrupted_recovery_marker(
recovered_output=False,
stream_id="stream_lost",
pending_started_at=2000.0,
)
assert marker["interruption_cause"] == "lost_worker_bookkeeping"
assert "worker bookkeeping no longer had an active run" in marker["content"]
def test_messages_js_names_browser_sse_disconnect_separately():
repo = models.Path(__file__).parent.parent
js = (repo / "static" / "messages.js").read_text(encoding="utf-8")
assert "Connection interrupted" in js
assert "browser lost the live SSE connection" in js
assert "Connection lost" not in js
def test_server_treats_broken_pipe_as_client_disconnect_not_500():
server_py = (models.Path(__file__).parent.parent / "server.py").read_text(encoding="utf-8")
assert "except (BrokenPipeError, ConnectionResetError, ConnectionAbortedError):" in server_py
assert "do not convert it into a misleading server 500" in server_py
def test_lost_response_recovered_on_second_read(hermes_home):
sid = "9f14583f0e4e4444aaaa111122223333"
stream_id = "7c8b4108d52b4aba9af362d3a54f47ac"