From 11c0d1667fac66f4e351fc82cfac3f0c0a8b0d82 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Thu, 4 Jun 2026 11:13:45 -0700 Subject: [PATCH] =?UTF-8?q?Release=20v0.51.254=20=E2=80=94=20Release=20HV?= =?UTF-8?q?=20(stage-r2)=20(#3593)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Release v0.51.254 — Release HV (stage-r2) Phase-2 medium wave 1 — 4 PRs (UI/mobile/cancel fixes + an un-held model dedup). ### Fixed | Issue/PR | Author | Fix | |----------|--------|-----| | #3528 | @franksong2702 | Render partial tool calls after cancel — interrupted turns keep their `_partial_tool_calls` rows in the transcript + fallback tool-cards. (Codex confirmed it stays render-only, not forwarded to the provider API.) | | #3550 | @lurebat | Android offline recovery soft-reattaches the live stream instead of hard-reloading the page on a transient background/disconnect. | | #3479 | @mvanhorn | iOS Safari no longer snaps the conversation to the top when a handoff/compression card is inserted mid-stream or on `refreshSession()`. | | #3478 | @JayC-L | **Un-held:** named custom providers (`@custom:name:model`) dedup against bare model IDs without regressing Ollama multi-colon tags (`qwen2.5:7b-instruct-q4`). Only `@custom:` IDs strip the two-segment prefix. | ### Hold-sweep note #3478/#3489 was held yesterday for an Ollama multi-colon-tag regression risk (a blanket `lastIndexOf` would lose the model). The author pushed a scoped fix (only `@custom:` IDs use `lastIndexOf`); I verified `_normId` in node against the regression cases — Ollama bare tags are preserved. Un-held + shipped. ### Gate - Full pytest suite: **7588 passed, 0 failed** - ESLint: CLEAN · ruff: CLEAN · browser-smoke: CLEAN - Codex (regression): **SAFE TO SHIP** — #3552 partial-tool-calls verified render-only (no `_API_SAFE_MSG_KEYS` leak / no 400-on-strict-provider, the v0.50.251 #1375 trap); #3551 no EventSource double-subscribe; #3541 no regression vs the #3525 scroll-follow shipped in v0.51.253; #3489 no over-dedup. Co-authored-by: franksong2702 Co-authored-by: lurebat Co-authored-by: mvanhorn Co-authored-by: JayC-L --- CHANGELOG.md | 8 + static/sessions.js | 6 +- static/style.css | 9 +- static/ui.js | 123 ++++++++++- .../test_issue3479_ios_stream_scroll_jump.py | 90 ++++++++ tests/test_issue401.py | 198 ++++++++++-------- tests/test_offline_soft_recovery.py | 102 +++++++++ 7 files changed, 437 insertions(+), 99 deletions(-) create mode 100644 tests/test_issue3479_ios_stream_scroll_jump.py create mode 100644 tests/test_offline_soft_recovery.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0be09a7a..a6173ff8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ ## [Unreleased] +## [v0.51.254] — 2026-06-04 — Release HV (stage-r2 — mobile/cancel/scroll fixes + custom-provider model dedup) + +### Fixed +- Client rendering no longer drops assistant-only partial-tool-call rows (`_partial_tool_calls`) from the visible transcript or fallback tool-card reconstruction after cancellation. This keeps interrupted turns visible and ensures interrupted tool calls still render as settled tool cards in history. (#3528, @franksong2702) +- On Android, briefly backgrounding the WebUI no longer hard-reloads the whole page — offline recovery now soft-reattaches the live stream instead of a full reload, so you don't lose composer/scroll state on a transient disconnect. (#3550, @lurebat) +- On iOS Safari, inserting a handoff/compression card mid-stream (or reconnecting via `refreshSession()`) no longer snaps the conversation to the top. (#3479, @mvanhorn) +- Named custom providers (`@custom:name:model`) now deduplicate against bare model IDs correctly, without regressing Ollama multi-colon tags (e.g. `qwen2.5:7b-instruct-q4`) — only `@custom:`-prefixed IDs strip the two-segment prefix; everything else keeps the single-prefix strip. (#3478, @JayC-L) + ## [v0.51.253] — 2026-06-04 — Release HU (stage-r1 — low-risk batch: scroll/badge/count/test fixes + compat docs) ### Fixed diff --git a/static/sessions.js b/static/sessions.js index 819aa958..43da0efe 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1512,8 +1512,12 @@ function _syncToolCallsForLoadedMessages(messages, sessionToolCalls){ const hasMessageToolMetadata=msgs.some(m=>{ if(!m) return false; const hasTc=Array.isArray(m.tool_calls)&&m.tool_calls.length>0; + // `_partial_tool_calls` are emitted by interrupted/partial turns and must also + // anchor rendering to the owning assistant message, so we can reconstruct + // settled tool cards from the message history when available. + const hasPartialTc=Array.isArray(m._partial_tool_calls)&&m._partial_tool_calls.length>0; const hasTu=Array.isArray(m.content)&&m.content.some(p=>p&&p.type==='tool_use'); - return hasTc||hasTu; + return hasTc||hasPartialTc||hasTu; }); if(!hasMessageToolMetadata&&Array.isArray(sessionToolCalls)&&sessionToolCalls.length){ S.toolCalls=sessionToolCalls.map(tc=>({...tc,done:true})); diff --git a/static/style.css b/static/style.css index 31f76ac6..3e4ea817 100644 --- a/static/style.css +++ b/static/style.css @@ -1404,9 +1404,10 @@ .approval-btn.deny:hover{background:rgba(239,83,80,.12);border-color:var(--error);} .approval-btn.loading{opacity:.7;cursor:wait;} /* ── Clarify card ── */ - .clarify-card{position:absolute;left:0;right:0;bottom:-24px;max-width:var(--msg-max);margin:0 auto;padding:0 20px;box-sizing:border-box;width:100%;overflow:hidden;pointer-events:none;max-height:min(calc(100vh - 280px),420px);} + .clarify-card{position:absolute;left:0;right:0;bottom:-24px;max-width:var(--msg-max);margin:0 auto;padding:0 20px;box-sizing:border-box;width:100%;overflow:hidden;pointer-events:none;max-height:clamp(180px,min(68vh,calc(100vh - 220px)),420px);} .clarify-card.visible{pointer-events:auto;} - .clarify-card .clarify-inner{max-height:min(calc(100vh - 280px),420px);overflow-y:auto;transform:translateY(100%);opacity:0;transition:transform .4s cubic-bezier(.32,.72,.16,1),opacity .25s ease,max-height .2s ease;} + .clarify-card .clarify-inner{max-height:clamp(180px,min(68vh,calc(100vh - 220px)),420px);overflow-y:auto;overscroll-behavior:contain;-webkit-overflow-scrolling:touch;box-sizing:border-box;transform:translateY(100%);opacity:0;transition:transform .4s cubic-bezier(.32,.72,.16,1),opacity .25s ease,max-height .2s ease;} + @supports (height:100dvh){.clarify-card,.clarify-card .clarify-inner{max-height:clamp(180px,min(68dvh,calc(100dvh - 220px)),420px);}} .clarify-card.visible .clarify-inner{transform:translateY(0);opacity:1;} .clarify-inner{background:var(--surface);backdrop-filter:blur(8px);border:1px solid var(--accent-bg-strong);border-radius:12px;padding:12px 14px 36px;box-shadow:0 1px 0 rgba(255,255,255,.02) inset;} .clarify-header{display:flex;align-items:center;gap:8px;margin-bottom:10px;font-size:12px;font-weight:700;color:var(--blue);letter-spacing:.01em;} @@ -2430,7 +2431,9 @@ .approval-btn{padding:8px 12px;font-size:12px;min-height:44px;} .approval-kbd{display:none;} /* Clarify card */ - .clarify-card{padding-left:10px;padding-right:10px;} + .clarify-card{padding-left:10px;padding-right:10px;max-height:clamp(180px,min(62vh,calc(100vh - 180px)),360px);} + .clarify-card .clarify-inner{max-height:clamp(180px,min(62vh,calc(100vh - 180px)),360px);} + @supports (height:100dvh){.clarify-card,.clarify-card .clarify-inner{max-height:clamp(180px,min(62dvh,calc(100dvh - 180px)),360px);}} .clarify-inner{padding:12px 12px 13px;} .clarify-response{flex-direction:column;align-items:stretch;} .clarify-input,.clarify-submit{width:100%;min-height:44px;} diff --git a/static/ui.js b/static/ui.js index 3ef3140c..ac64191c 100644 --- a/static/ui.js +++ b/static/ui.js @@ -86,13 +86,45 @@ async function checkOfflineRecoveryNow(){ _setOfflineChecking(true); const ok=await _probeOfflineRecovery(); _setOfflineChecking(false); - if(ok){_stopOfflineProbeTimer();window.location.reload();return true;} + if(ok){_stopOfflineProbeTimer();await _recoverFromOfflineSoftly();return true;} showOfflineBanner('network'); return false; })(); try{return await _offlineProbePromise;} finally{_offlineProbePromise=null;} } +// Recover from a transient "Connection lost" without a full page reload. +// +// The offline banner fires whenever a fetch/SSE errors — which Android does +// aggressively every time the PWA is backgrounded, even for a second. The old +// behaviour here was `window.location.reload()`: a hard cold boot that re-runs +// the whole app and re-pulls /api/sessions + /api/session, producing the +// multi-second "reload to see the conversation I was just in" flash on every +// resume. The reload was also intermittent (only when a request actually +// errored that time), matching the reported "sometimes it reloads, sometimes +// it doesn't". +// +// The server keeps the agent running and buffers stream events while no +// subscriber is attached (#2307), so a hard reload is never required to +// recover — we just need to reattach. This does the soft path: hide the +// banner, restart the gateway SSE (bfcache/background kills the connection), +// and re-fetch the active session so any messages that landed while we were +// away appear. A full reload is the fallback only if the soft path throws. +async function _recoverFromOfflineSoftly(){ + try{ + _hideOfflineBanner(); + if(typeof startGatewaySSE==='function') startGatewaySSE(); + if(S.session && typeof refreshSession==='function'){ + await refreshSession(); + } + return true; + }catch(_){ + // Soft reattach failed (server mid-restart, session gone, etc.) — fall + // back to the original hard reload so the user is never stuck offline. + window.location.reload(); + return false; + } +} function _isAbortError(e){return !!(e&&(e.name==='AbortError'||e.code===20));} function _patchOfflineFetch(){ if(_offlineFetchPatched||typeof window.fetch!=='function')return; @@ -1340,11 +1372,19 @@ function _addLiveModelsToSelect(provider, models, sel){ sel.appendChild(providerGroup); } const existingIds=new Set([...sel.options].map(o=>o.value)); - // Normalized dedup: strip one @provider: prefix and namespace so + // Normalized dedup: strip @provider: prefix and namespace so // 'minimax/minimax-m2.7' matches '@nous:minimax/minimax-m2.7' (#907). + // Named custom IDs (@custom:name:model) have a known two-segment prefix + // and must strip both to dedup against bare model IDs (#3478). const _normId=id=>{ let s=String(id||''); - if(s.startsWith('@')&&s.includes(':')) s=s.substring(s.indexOf(':')+1); + if(s.startsWith('@')&&s.includes(':')){ + if(s.startsWith('@custom:')){ + s=s.substring(s.lastIndexOf(':')+1)||s; + }else{ + s=s.substring(s.indexOf(':')+1); + } + } s=s.split('/').pop(); return s.replace(/-/g,'.').toLowerCase(); }; @@ -5032,7 +5072,7 @@ async function refreshSession() { if(pendingMsg) S.messages.push(pendingMsg); S.activeStreamId=data.session.active_stream_id||null; - syncTopbar(); renderMessages(); + syncTopbar(); _renderMessagesWithScrollSnapshot(); showToast('Conversation refreshed'); } catch(e) { setStatus('Refresh failed: ' + e.message); } } @@ -6000,6 +6040,7 @@ function _compressionCardsNode(state){ } function appendLiveCompressionCard(state){ if(!S.session||!S.activeStreamId||!state) return false; + const scrollSnapshot=_captureMessageScrollSnapshot(); let turn=$('liveAssistantTurn'); if(!turn){ turn=_createAssistantTurn(); @@ -6035,6 +6076,7 @@ function appendLiveCompressionCard(state){ const existing=inner.querySelector('[data-live-compression-card="1"]'); if(existing) existing.replaceWith(node); else inner.appendChild(node); + _restoreMessageScrollSnapshotSameFrame(scrollSnapshot); if(typeof scrollIfPinned==='function') scrollIfPinned(); return true; } @@ -6301,7 +6343,7 @@ function _handoffStateForCurrentSession(){ } function clearHandoffUi(){ window._handoffUi=null; - renderMessages(); + _renderMessagesWithScrollSnapshot(); } function setHandoffUi(state){ if(!state){ @@ -6309,7 +6351,7 @@ function setHandoffUi(state){ return; } window._handoffUi={...state}; - renderMessages(); + _renderMessagesWithScrollSnapshot(); } function _handoffCardsHtml(state){ if(!state) return ''; @@ -6542,7 +6584,13 @@ function _cliToolCardHasDiffSnippet(resultSnippet, patchSnippet){ function _captureMessageScrollSnapshot(){ const el=$('messages'); if(!el) return null; - return {top:el.scrollTop}; + const bottom=Math.max(0,el.scrollHeight-el.scrollTop-el.clientHeight); + return { + top:el.scrollTop, + bottom, + pinned:_shouldFollowMessagesOnDomReplace(), + userUnpinned:_messageUserUnpinned, + }; } function _restoreMessageScrollSnapshot(snapshot){ const el=$('messages'); @@ -6554,6 +6602,33 @@ function _restoreMessageScrollSnapshot(snapshot){ _lastScrollTop=el.scrollTop; requestAnimationFrame(()=>{ setTimeout(()=>{_programmaticScroll=false;},0); }); } +function _restoreMessageScrollSnapshotSameFrame(snapshot){ + const el=$('messages'); + if(!el||!snapshot) return; + const maxTop=Math.max(0,el.scrollHeight-el.clientHeight); + const bottom=Number(snapshot.bottom); + const target=(snapshot.pinned===true&&Number.isFinite(bottom)) + ? maxTop-Math.max(0,bottom) + : Number(snapshot.top)||0; + _programmaticScroll=true; + el.scrollTop=Math.max(0,Math.min(target,maxTop)); + _lastScrollTop=el.scrollTop; + if(snapshot.pinned===true){ + _messageUserUnpinned=false; + _scrollPinned=true; + _nearBottomCount=2; + }else if(snapshot.userUnpinned===true){ + _messageUserUnpinned=true; + _scrollPinned=false; + _nearBottomCount=0; + } + requestAnimationFrame(()=>{ setTimeout(()=>{_programmaticScroll=false;},0); }); +} +function _renderMessagesWithScrollSnapshot(options){ + const scrollSnapshot=_captureMessageScrollSnapshot(); + renderMessages({...(options||{}),preserveScroll:true}); + _restoreMessageScrollSnapshotSameFrame(scrollSnapshot); +} function _scrollAfterMessageRender(preserveScroll, scrollSnapshot){ // Terminal stream renders can happen after S.activeStreamId is cleared. // In that case, preserveScroll asks the normal pin-state helper to decide: @@ -6632,7 +6707,8 @@ function renderMessages(options){ if(m.role==='assistant'){ const hasTc=Array.isArray(m.tool_calls)&&m.tool_calls.length>0; const hasTu=Array.isArray(m.content)&&m.content.some(p=>p&&p.type==='tool_use'); - if(hasTc||hasTu||_messageHasReasoningPayload(m)) return true; + const hasPartialTc=Array.isArray(m._partial_tool_calls)&&m._partial_tool_calls.length>0; + if(hasTc||hasTu||hasPartialTc||_messageHasReasoningPayload(m)) return true; if(_assistantMessageHasVisibleContent(m)) return true; } return m._statusCard||msgContent(m)||m.attachments?.length; @@ -6663,7 +6739,8 @@ function renderMessages(options){ if(_isRecoveryControlMessage(m)){ri++;continue;} const hasTc=Array.isArray(m.tool_calls)&&m.tool_calls.length>0; const hasTu=Array.isArray(m.content)&&m.content.some(p=>p&&p.type==='tool_use'); - if(msgContent(m)||m._statusCard||m.attachments?.length||(m.role==='assistant'&&(hasTc||hasTu||_messageHasReasoningPayload(m)||_assistantMessageHasVisibleContent(m)))) rebuilt.push({m,rawIdx:ri}); + const hasPartialTc=Array.isArray(m._partial_tool_calls)&&m._partial_tool_calls.length>0; + if(msgContent(m)||m._statusCard||m.attachments?.length||(m.role==='assistant'&&(hasTc||hasTu||hasPartialTc||_messageHasReasoningPayload(m)||_assistantMessageHasVisibleContent(m)))) rebuilt.push({m,rawIdx:ri}); ri++; } _visWithIdxCache=rebuilt; @@ -7042,7 +7119,8 @@ function renderMessages(options){ if(m.role==='assistant'){ const hasTopLevelToolCalls=Array.isArray(m.tool_calls)&&m.tool_calls.length>0; const hasContentToolUse=Array.isArray(m.content)&&m.content.some(p=>p&&typeof p==='object'&&p.type==='tool_use'); - if(hasTopLevelToolCalls||hasContentToolUse) fallbackToolSources.push({m,rawIdx}); + const hasPartialToolCalls=Array.isArray(m._partial_tool_calls)&&m._partial_tool_calls.length>0; + if(hasTopLevelToolCalls||hasContentToolUse||hasPartialToolCalls) fallbackToolSources.push({m,rawIdx}); } }); const derived=[]; @@ -7093,6 +7171,31 @@ function renderMessages(options){ }); }); } + // WebUI-internal partial tool calls captured on cancel/stop + // (private shape: name/args/done/preview/snippet, no OpenAI envelope). + if(Array.isArray(m._partial_tool_calls)){ + m._partial_tool_calls.forEach(tc=>{ + if(!tc||typeof tc!=='object') return; + const name=tc.name||'tool'; + const args=tc.args||{}; + const tid=tc.id||tc.call_id||tc.tool_call_id||tc.tid||''; + const patchSnippet=_cliPatchSnippetFromArgs(name,args); + const resultSnippet=_cliToolResultSnippet(tc.snippet||tc.result||tc.output||tc.preview||''); + const argsSnap={}; + if(args && typeof args==='object'){ + Object.keys(args).slice(0,4).forEach(k=>{ const v=String(args[k]); argsSnap[k]=v.slice(0,120)+(v.length>120?'...':''); }); + } + derived.push({ + name, + snippet:_cliToolCardSnippet(resultSnippet,patchSnippet), + is_diff:_cliToolCardHasDiffSnippet(resultSnippet,patchSnippet), + tid, + assistant_msg_idx:rawIdx, + args:argsSnap, + done:true, + }); + }); + } }); if(derived.length) S.toolCalls=derived; } diff --git a/tests/test_issue3479_ios_stream_scroll_jump.py b/tests/test_issue3479_ios_stream_scroll_jump.py new file mode 100644 index 00000000..fe85e2b9 --- /dev/null +++ b/tests/test_issue3479_ios_stream_scroll_jump.py @@ -0,0 +1,90 @@ +"""Regression coverage for #3479: iOS Safari must not see a top-scroll frame. + +The live token path writes into an existing streaming-markdown DOM node. The +jump came from discrete transcript rebuilds and card replacement paths, so these +tests pin those call sites rather than the per-token renderer. +""" + +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +UI_JS = (ROOT / "static" / "ui.js").read_text(encoding="utf-8") +STYLE_CSS = (ROOT / "static" / "style.css").read_text(encoding="utf-8") + + +def _function_body(src: str, name: str) -> str: + marker = f"function {name}" + start = src.find(marker) + assert start >= 0, f"{name} not found" + brace = src.find("{", start) + assert brace >= 0, f"{name} body not found" + depth = 0 + for pos in range(brace, len(src)): + ch = src[pos] + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return src[start : pos + 1] + raise AssertionError(f"{name} body did not terminate") + + +def _compact(text: str) -> str: + return "".join(text.split()) + + +def test_refresh_session_uses_same_frame_scroll_snapshot_restore(): + body = _function_body(UI_JS, "refreshSession") + + assert "syncTopbar(); _renderMessagesWithScrollSnapshot();" in body + assert "syncTopbar(); renderMessages();" not in body + + +def test_handoff_rebuilds_use_same_frame_scroll_snapshot_restore(): + clear_body = _function_body(UI_JS, "clearHandoffUi") + set_body = _function_body(UI_JS, "setHandoffUi") + + assert "_renderMessagesWithScrollSnapshot();" in clear_body + assert "_renderMessagesWithScrollSnapshot();" in set_body + assert "renderMessages();" not in clear_body + assert "renderMessages();" not in set_body + + +def test_live_compression_card_replacement_restores_snapshot_before_follow_settle(): + body = _function_body(UI_JS, "appendLiveCompressionCard") + + capture_idx = body.index("const scrollSnapshot=_captureMessageScrollSnapshot();") + replace_idx = body.index("if(existing) existing.replaceWith(node);") + restore_idx = body.index("_restoreMessageScrollSnapshotSameFrame(scrollSnapshot);") + settle_idx = body.index("if(typeof scrollIfPinned==='function') scrollIfPinned();") + + assert capture_idx < replace_idx < restore_idx < settle_idx + + +def test_same_frame_snapshot_preserves_bottom_distance_and_unpinned_state(): + capture = _function_body(UI_JS, "_captureMessageScrollSnapshot") + restore = _function_body(UI_JS, "_restoreMessageScrollSnapshotSameFrame") + wrapper = _function_body(UI_JS, "_renderMessagesWithScrollSnapshot") + + assert "bottom" in capture + assert "pinned:_shouldFollowMessagesOnDomReplace()" in _compact(capture) + assert "userUnpinned:_messageUserUnpinned" in _compact(capture) + assert "maxTop-Math.max(0,bottom)" in restore + assert "_messageUserUnpinned=true" in restore + assert "_scrollPinned=false" in restore + assert "renderMessages({...(options||{}),preserveScroll:true});" in wrapper + assert "_restoreMessageScrollSnapshotSameFrame(scrollSnapshot);" in wrapper + + +def test_clarify_card_is_height_clamped_and_scrollable_on_mobile_viewports(): + compact = _compact(STYLE_CSS) + + assert ".clarify-card" in STYLE_CSS + assert "max-height:clamp(180px,min(68vh,calc(100vh-220px)),420px)" in compact + assert "@supports(height:100dvh)" in compact + assert "max-height:clamp(180px,min(62dvh,calc(100dvh-180px)),360px)" in compact + assert "overflow-y:auto" in compact + assert "-webkit-overflow-scrolling:touch" in compact + assert "overscroll-behavior:contain" in compact diff --git a/tests/test_issue401.py b/tests/test_issue401.py index ab8b3c26..d6347757 100644 --- a/tests/test_issue401.py +++ b/tests/test_issue401.py @@ -16,6 +16,41 @@ import textwrap REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve() SESSIONS_JS = (REPO_ROOT / "static" / "sessions.js").read_text(encoding="utf-8") UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8") +_SYNC_TOOL_CALLS_FN_NAME = ( + "function _syncToolCallsForLoadedMessages(messages, sessionToolCalls){" +) +_SYNC_TOOL_CALLS_FN_END_MARKER = "async function _ensureMessagesLoaded" +_SYNC_TOOL_CALLS_FN_START = SESSIONS_JS.find(_SYNC_TOOL_CALLS_FN_NAME) +_SYNC_TOOL_CALLS_FN_END = SESSIONS_JS.find( + _SYNC_TOOL_CALLS_FN_END_MARKER, + _SYNC_TOOL_CALLS_FN_START, +) +assert _SYNC_TOOL_CALLS_FN_START >= 0 +assert _SYNC_TOOL_CALLS_FN_END > _SYNC_TOOL_CALLS_FN_START +_SYNC_TOOL_CALLS_FN = SESSIONS_JS[_SYNC_TOOL_CALLS_FN_START:_SYNC_TOOL_CALLS_FN_END] + + +def _run_sync_tool_calls(messages: list, session_tool_calls: list) -> list: + assert _SYNC_TOOL_CALLS_FN_NAME in SESSIONS_JS + assert _SYNC_TOOL_CALLS_FN.strip() + script = textwrap.dedent( + f""" + const S = {{}}; + S.toolCalls = []; + {_SYNC_TOOL_CALLS_FN} + const messages = {json.dumps(messages)}; + const sessionToolCalls = {json.dumps(session_tool_calls)}; + _syncToolCallsForLoadedMessages(messages, sessionToolCalls); + process.stdout.write(JSON.stringify(S.toolCalls)); + """ + ) + proc = subprocess.run( + ["node", "-e", script], + check=True, + capture_output=True, + text=True, + ) + return json.loads(proc.stdout) def test_loadsession_preserves_tool_rows(): @@ -38,103 +73,96 @@ def test_loadsession_uses_session_toolcalls_only_as_fallback(): def test_rendermessages_treats_openai_toolcall_assistants_as_visible(): """OpenAI assistant rows with empty content but tool_calls must stay anchorable.""" assert "const hasTc=Array.isArray(m.tool_calls)&&m.tool_calls.length>0;" in UI_JS - assert "if(hasTc||hasTu||_messageHasReasoningPayload(m)) return true;" in UI_JS + assert "if(hasTc||hasTu||hasPartialTc||_messageHasReasoningPayload(m)) return true;" in UI_JS -def _run_js(script_body: str) -> dict: - script = textwrap.dedent(f""" - function loadSessionShape(messages, sessionToolCalls) {{ - const filtered = (messages || []).filter(m => m && m.role); - const hasMessageToolMetadata = filtered.some(m => {{ - if (!m || m.role !== 'assistant') return false; - const hasTc = Array.isArray(m.tool_calls) && m.tool_calls.length > 0; - const hasTu = Array.isArray(m.content) && m.content.some(p => p && p.type === 'tool_use'); - return hasTc || hasTu; - }}); - const toolCalls = (!hasMessageToolMetadata && sessionToolCalls && sessionToolCalls.length) - ? sessionToolCalls.map(tc => ({{ ...tc, done: true }})) - : []; - return {{ filtered, hasMessageToolMetadata, toolCalls }}; - }} +def test_rendermessages_treats_partial_toolcall_assistants_as_visible(): + """Assistant rows carrying `_partial_tool_calls` must stay anchorable.""" + assert "const hasPartialTc=Array.isArray(m._partial_tool_calls)&&m._partial_tool_calls.length>0;" in UI_JS + assert "if(hasTc||hasTu||hasPartialTc||_messageHasReasoningPayload(m)) return true;" in UI_JS - {script_body} - """) - proc = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) - return json.loads(proc.stdout) + +def test_rendermessages_rebuilds_tool_cards_from_partial_tool_calls(): + """Fallback reconstruction should include private `_partial_tool_calls` rows.""" + assert "const hasPartialToolCalls=Array.isArray(m._partial_tool_calls)&&m._partial_tool_calls.length>0;" in UI_JS + assert "if(hasTopLevelToolCalls||hasContentToolUse||hasPartialToolCalls) fallbackToolSources.push({m,rawIdx});" in UI_JS + assert "if(Array.isArray(m._partial_tool_calls)){" in UI_JS + assert "tc.snippet||tc.result||tc.output||tc.preview" in UI_JS + assert "done:true" in UI_JS def test_reload_keeps_empty_assistant_toolcall_anchor(): """OpenAI-style assistant {content:'', tool_calls:[...]} must survive reload.""" - result = _run_js(""" - const messages = [ - { role: 'user', content: 'list files' }, - { - role: 'assistant', - content: '', - tool_calls: [{ id: 'call-1', function: { name: 'terminal', arguments: '{}' } }] - }, - { role: 'tool', tool_call_id: 'call-1', content: '{"output":"ok"}' }, - { role: 'assistant', content: 'Done.' } - ]; - const loaded = loadSessionShape(messages, [{ name: 'terminal', assistant_msg_idx: 1 }]); - process.stdout.write(JSON.stringify({ - filtered_len: loaded.filtered.length, - has_metadata: loaded.hasMessageToolMetadata, - fallback_len: loaded.toolCalls.length, - assistant_tool_idx: loaded.filtered.findIndex(m => m.role === 'assistant' && m.tool_calls), - tool_idx: loaded.filtered.findIndex(m => m.role === 'tool') - })); - """) - assert result["filtered_len"] == 4 - assert result["has_metadata"] is True - assert result["fallback_len"] == 0 - assert result["assistant_tool_idx"] == 1 - assert result["tool_idx"] == 2 + messages = [ + {"role": "user", "content": "list files"}, + { + "role": "assistant", + "content": "", + "tool_calls": [{"id": "call-1", "function": {"name": "terminal", "arguments": "{}"}}], + }, + {"role": "tool", "tool_call_id": "call-1", "content": '{"output":"ok"}'}, + {"role": "assistant", "content": "Done."}, + ] + filtered = [m for m in messages if m and m.get("role")] + tool_calls = _run_sync_tool_calls( + messages, + [{"name": "terminal", "assistant_msg_idx": 1}], + ) + assert len(filtered) == 4 + assert tool_calls == [] + assert filtered[1]["role"] == "assistant" and filtered[1].get("tool_calls") + assert filtered[2]["role"] == "tool" + + +def test_reload_keeps_empty_assistant_partial_toolcall_anchor(): + """Partial tool-call rows with empty content must survive reload.""" + messages = [ + {"role": "user", "content": "open log"}, + { + "role": "assistant", + "content": "", + "_partial_tool_calls": [{"name": "read_file", "args": {"path": "README.md"}}], + }, + {"role": "assistant", "content": "Done."}, + ] + filtered = [m for m in messages if m and m.get("role")] + tool_calls = _run_sync_tool_calls( + messages, + [{"name": "write_file", "assistant_msg_idx": 1}], + ) + assert len(filtered) == 3 + assert tool_calls == [] + assert filtered[1]["role"] == "assistant" + assert isinstance(filtered[1].get("_partial_tool_calls"), list) + assert filtered[1]["_partial_tool_calls"][0]["name"] == "read_file" def test_reload_uses_session_summary_when_messages_have_no_tool_metadata(): """Older sessions should still render from session.tool_calls on reload.""" - result = _run_js(""" - const messages = [ - { role: 'user', content: 'build site' }, - { role: 'assistant', content: 'Starting.' }, - { role: 'tool', content: '{"bytes_written": 4955}' }, - { role: 'assistant', content: '' } - ]; - const sessionToolCalls = [ - { name: 'write_file', assistant_msg_idx: 1, snippet: 'bytes_written', tid: '' } - ]; - const loaded = loadSessionShape(messages, sessionToolCalls); - process.stdout.write(JSON.stringify({ - has_metadata: loaded.hasMessageToolMetadata, - fallback_len: loaded.toolCalls.length, - done_flag: loaded.toolCalls[0] && loaded.toolCalls[0].done === true - })); - """) - assert result["has_metadata"] is False - assert result["fallback_len"] == 1 - assert result["done_flag"] is True + messages = [ + {"role": "user", "content": "build site"}, + {"role": "assistant", "content": "Starting."}, + {"role": "tool", "content": '{"bytes_written": 4955}'}, + {"role": "assistant", "content": ""}, + ] + tool_calls = _run_sync_tool_calls( + messages, + [{"name": "write_file", "assistant_msg_idx": 1, "snippet": "bytes_written", "tid": ""}], + ) + assert len(tool_calls) == 1 + assert tool_calls[0]["done"] is True def test_rebased_legacy_toolcalls_stay_distinct_in_paged_window(): """Backend-rebased legacy tool calls must not be rebased a second time.""" - result = _run_js(""" - const messages = [ - { role: 'assistant', content: 'first legacy page row' }, - { role: 'assistant', content: 'second legacy page row' } - ]; - const sessionToolCalls = [ - { name: 'first_tool', assistant_msg_idx: 0, snippet: 'first' }, - { name: 'second_tool', assistant_msg_idx: 1, snippet: 'second' } - ]; - const loaded = loadSessionShape(messages, sessionToolCalls); - const renderedRawIdxs = messages.map((_, idx) => idx); - process.stdout.write(JSON.stringify({ - indices: loaded.toolCalls.map(tc => tc.assistant_msg_idx), - distinct_indices: new Set(loaded.toolCalls.map(tc => tc.assistant_msg_idx)).size, - anchors_visible: loaded.toolCalls.every(tc => renderedRawIdxs.includes(tc.assistant_msg_idx)) - })); - """) - assert result["indices"] == [0, 1] - assert result["distinct_indices"] == 2 - assert result["anchors_visible"] is True + messages = [ + {"role": "assistant", "content": "first legacy page row"}, + {"role": "assistant", "content": "second legacy page row"}, + ] + session_tool_calls = [ + {"name": "first_tool", "assistant_msg_idx": 0, "snippet": "first"}, + {"name": "second_tool", "assistant_msg_idx": 1, "snippet": "second"}, + ] + tool_calls = _run_sync_tool_calls(messages, session_tool_calls) + assert [tc["assistant_msg_idx"] for tc in tool_calls] == [0, 1] + assert len({tc["assistant_msg_idx"] for tc in tool_calls}) == 2 diff --git a/tests/test_offline_soft_recovery.py b/tests/test_offline_soft_recovery.py new file mode 100644 index 00000000..5142892d --- /dev/null +++ b/tests/test_offline_soft_recovery.py @@ -0,0 +1,102 @@ +"""Offline recovery must reattach softly, not hard-reload the whole page. + +Symptom (Android PWA): backgrounding the app for even a second often showed a +"Connection lost" banner, and on returning the whole page did a multi-second +cold reload. Confirmed from the live server journal: the client's offline +recovery probe (`GET /health?offline_probe=...`) fired repeatedly from the +phone, and each successful probe ran `window.location.reload()`. + +Root cause: `checkOfflineRecoveryNow()` in static/ui.js called +`window.location.reload()` on the first healthy probe. The offline banner is +raised on any fetch/SSE error — which mobile backgrounding triggers constantly +— so a transient background turned into a full app cold boot (re-run boot, +re-pull /api/sessions + /api/session). Intermittent because it only fired when +a request actually errored that cycle. + +Fix: recover softly via `_recoverFromOfflineSoftly()` — hide the banner, +restart the gateway SSE, and re-fetch the active session through the existing +`refreshSession()` reattach path. The server keeps the agent running and +buffers stream events while no subscriber is attached (#2307), so a hard reload +is never required. A full `window.location.reload()` remains only as the catch +fallback if the soft reattach throws. + +State layer: this only changes the *client* recovery transition (banner → +reattach). It does not touch server stream buffering, session persistence, or +the compression/replay paths. +""" + +from pathlib import Path + +ROOT = Path(__file__).parent.parent + + +def _ui_js() -> str: + return (ROOT / "static" / "ui.js").read_text(encoding="utf-8") + + +def _fn_body(src: str, marker: str) -> str: + idx = src.find(marker) + assert idx != -1, f"{marker!r} not found in ui.js" + brace = src.find("{", idx) + depth = 1 + i = brace + 1 + while i < len(src) and depth: + if src[i] == "{": + depth += 1 + elif src[i] == "}": + depth -= 1 + i += 1 + assert depth == 0, f"{marker!r} body did not close" + return src[brace + 1 : i - 1] + + +class TestOfflineSoftRecovery: + def test_recovery_does_not_hard_reload_on_success(self): + """The success branch must call the soft recover, not location.reload().""" + src = _ui_js() + body = _fn_body(src, "async function checkOfflineRecoveryNow(") + assert "_recoverFromOfflineSoftly(" in body, ( + "checkOfflineRecoveryNow must recover via _recoverFromOfflineSoftly() " + "on a healthy probe instead of hard-reloading the page" + ) + assert "window.location.reload()" not in body, ( + "checkOfflineRecoveryNow must not call window.location.reload() on " + "recovery — that is the multi-second cold-boot regression" + ) + + def test_soft_recover_helper_exists(self): + """The soft recovery helper must exist.""" + assert "async function _recoverFromOfflineSoftly(" in _ui_js(), ( + "_recoverFromOfflineSoftly() helper must exist" + ) + + def test_soft_recover_hides_banner_and_reattaches(self): + """Soft recovery must hide the banner, restart SSE, and refresh the session.""" + body = _fn_body(_ui_js(), "async function _recoverFromOfflineSoftly(") + assert "_hideOfflineBanner()" in body, ( + "_recoverFromOfflineSoftly must hide the offline banner" + ) + assert "startGatewaySSE" in body, ( + "_recoverFromOfflineSoftly must restart the gateway SSE " + "(background/bfcache kills the connection)" + ) + assert "refreshSession" in body, ( + "_recoverFromOfflineSoftly must reattach the active session via " + "refreshSession() so messages that arrived while away appear" + ) + + def test_soft_recover_falls_back_to_hard_reload(self): + """A failed soft reattach must fall back to a full reload (never stuck).""" + body = _fn_body(_ui_js(), "async function _recoverFromOfflineSoftly(") + assert "window.location.reload()" in body, ( + "_recoverFromOfflineSoftly must keep window.location.reload() as the " + "catch fallback so a failed soft reattach never leaves the user stuck" + ) + + def test_refresh_session_guarded_with_typeof(self): + """Reattach calls must be typeof-guarded for safe degradation.""" + body = _fn_body(_ui_js(), "async function _recoverFromOfflineSoftly(") + assert "typeof refreshSession==='function'" in body or \ + "typeof refreshSession === 'function'" in body, ( + "refreshSession() call must be typeof-guarded" + )