diff --git a/CHANGELOG.md b/CHANGELOG.md index ac16492fd5..793dfdb73e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## [Unreleased] +## [v0.50.250] — 2026-04-30 + +### Fixed +- **Cross-tab thinking-card cleanup no longer touches the wrong session's DOM** — switching browser tabs while a stream is running could leave `finalizeThinkingCard()` operating on a stale `liveAssistantTurn` node — the thinking card belonged to the stream that started it, not the session currently displayed in the active tab. The guard early-returns when the live turn's `dataset.sessionId` does not match `S.session.session_id`. Per-site stamps were also added: every place that creates `liveAssistantTurn` (3 sites in `static/ui.js`) now writes the current session id onto `dataset.sessionId` so the guard has the data it needs to compare. Without the stamps the guard would always early-return (because `undefined !== ""` is always true), breaking the streaming UI completely — caught during pre-release review of #1366. Plus a regression test that fails any future `liveAssistantTurn` creation site that forgets the stamp. (`static/ui.js`, `tests/test_pr1366_finalize_thinking_card_guard.py`) @JKJameson — PR #1366 +- **Clarify SSE health timer is now an actual stale-detector, not an unconditional 60s force-reconnect** — the timer at `static/messages.js:1715` shipped in v0.50.249 / PR #1355 closed and re-opened the EventSource every 60s regardless of activity, with a comment that wrongly claimed it was a "no event in 60s" detector. Effects on healthy connections: one TCP/SSE setup+teardown per minute per active session, plus a `clarify._lock` round-trip and fresh `initial` snapshot push from the server. Now tracks `lastEventAt` on `initial`/`clarify` event arrivals; only reconnects when the gap exceeds 60s. On a session with steady clarify traffic the timer never reconnects; on a long-idle session it still reconnects roughly every 60-120s (the residual idle reconnect could be eliminated with a server-side `ping` event or a longer threshold — tracked as a follow-up). Originally pulled out of the v0.50.249 batch as out-of-scope; brought back per the rule that small correctness-improving fixes ship even when flagged out-of-scope. (`static/messages.js`) — PR #1367 (Opus pre-release review of v0.50.249, SHOULD-FIX #2) +- **Preferences panel autosaves all fields (Phase 2 of #1003)** — extends the autosave pattern from the Appearance panel to the Preferences panel so 13 preference fields (send_key, language, show_token_usage, simplified_tool_calling, show_cli_sessions, sync_to_insights, check_for_updates, sound_enabled, notifications_enabled, sidebar_density, auto_title_refresh_every, busy_input_mode, bot_name) save automatically without requiring a manual "Save Settings" click. 350ms debounce on field changes (additional 500ms wrapper on the bot_name text input). Inline status feedback (saving / saved / failed + retry). Password field still requires explicit save (security — never autosave passwords). Model selector still requires explicit save (different code path). Reuses the i18n keys (`settings_autosave_saving`/`saved`/`failed`/`retry`) already present in all 8 locales from Phase 1. (`static/index.html`, `static/panels.js`) @fecolinhares — PR #1369 + ## [v0.50.249] — 2026-04-30 ### Added diff --git a/static/index.html b/static/index.html index 8b34284ae4..e93dcef54f 100644 --- a/static/index.html +++ b/static/index.html @@ -793,6 +793,7 @@

What can I help with?

+
diff --git a/static/messages.js b/static/messages.js index 2209952f14..e9e9b868a9 100644 --- a/static/messages.js +++ b/static/messages.js @@ -1709,8 +1709,22 @@ function startClarifyPolling(sid) { _startClarifyFallbackPoll(sid); }; - // Health timer: if no event received in 60s, reconnect. + // Stale-detector: track last event timestamp; only reconnect if no event + // (initial or clarify) has arrived in 60s. The server sends a keepalive + // comment line every 30s but EventSource silently consumes those; we only + // bump lastEventAt on actual application events. With no real events for + // 60s on a long-lived clarify connection the server is effectively silent + // and a reconnect is the safe move. + // + // Without the lastEventAt gate the original PR force-reconnected every 60s + // regardless of activity, which churned one TCP/SSE setup per minute per + // active session. (Opus pre-release review of v0.50.249.) + let _lastClarifyEventAt = Date.now(); + const _markClarifyEvent = () => { _lastClarifyEventAt = Date.now(); }; + _clarifyEventSource.addEventListener('initial', _markClarifyEvent); + _clarifyEventSource.addEventListener('clarify', _markClarifyEvent); _clarifyHealthTimer = setInterval(function() { + if (Date.now() - _lastClarifyEventAt < 60000) return; if (_clarifyEventSource) { try { _clarifyEventSource.close(); } catch(_){} _clarifyEventSource = null; diff --git a/static/panels.js b/static/panels.js index 67865b3c73..2a1bea7032 100644 --- a/static/panels.js +++ b/static/panels.js @@ -2492,6 +2492,8 @@ let _settingsSection = 'conversation'; let _currentSettingsSection = 'conversation'; let _settingsAppearanceAutosaveTimer = null; let _settingsAppearanceAutosaveRetryPayload = null; +let _settingsPreferencesAutosaveTimer = null; +let _settingsPreferencesAutosaveRetryPayload = null; function switchSettingsSection(name){ const section=(name==='appearance'||name==='preferences'||name==='providers'||name==='system')?name:'conversation'; @@ -2675,6 +2677,105 @@ function _retryAppearanceAutosave(){ _autosaveAppearanceSettings(payload); } +// ── Phase 2: Preferences autosave (Issue #1003) ─────────────────────── + +function _preferencesPayloadFromUi(){ + const payload={}; + const sendKeySel=$('settingsSendKey'); + if(sendKeySel) payload.send_key=sendKeySel.value; + const langSel=$('settingsLanguage'); + if(langSel) payload.language=langSel.value; + const showUsageCb=$('settingsShowTokenUsage'); + if(showUsageCb) payload.show_token_usage=showUsageCb.checked; + const simplifiedToolCb=$('settingsSimplifiedToolCalling'); + if(simplifiedToolCb) payload.simplified_tool_calling=simplifiedToolCb.checked; + const showCliCb=$('settingsShowCliSessions'); + if(showCliCb) payload.show_cli_sessions=showCliCb.checked; + const syncCb=$('settingsSyncInsights'); + if(syncCb) payload.sync_to_insights=syncCb.checked; + const updateCb=$('settingsCheckUpdates'); + if(updateCb) payload.check_for_updates=updateCb.checked; + const soundCb=$('settingsSoundEnabled'); + if(soundCb) payload.sound_enabled=soundCb.checked; + const notifCb=$('settingsNotificationsEnabled'); + if(notifCb) payload.notifications_enabled=notifCb.checked; + const sidebarDensitySel=$('settingsSidebarDensity'); + if(sidebarDensitySel) payload.sidebar_density=sidebarDensitySel.value; + const autoTitleRefreshSel=$('settingsAutoTitleRefresh'); + if(autoTitleRefreshSel) payload.auto_title_refresh_every=parseInt(autoTitleRefreshSel.value,10); + const busyInputModeSel=$('settingsBusyInputMode'); + if(busyInputModeSel) payload.busy_input_mode=busyInputModeSel.value; + const botNameField=$('settingsBotName'); + if(botNameField) payload.bot_name=botNameField.value; + return payload; +} + +function _setPreferencesAutosaveStatus(state){ + const el=$('settingsPreferencesAutosaveStatus'); + if(!el) return; + el.className='settings-autosave-status'; + if(!state){ + el.textContent=''; + return; + } + el.classList.add('is-'+state); + if(state==='saving'){ + el.textContent=t('settings_autosave_saving'); + }else if(state==='saved'){ + el.textContent=t('settings_autosave_saved'); + }else if(state==='failed'){ + el.innerHTML=`${esc(t('settings_autosave_failed'))} `; + } +} + +function _rememberPreferencesSaved(payload){ + if(!payload) return; + if(payload.send_key!==undefined) localStorage.setItem('hermes-pref-send_key',payload.send_key); + if(payload.language!==undefined) localStorage.setItem('hermes-pref-language',payload.language); +} + +function _schedulePreferencesAutosave(){ + const payload=_preferencesPayloadFromUi(); + _rememberPreferencesSaved(payload); + _settingsPreferencesAutosaveRetryPayload=payload; + _setPreferencesAutosaveStatus('saving'); + if(_settingsPreferencesAutosaveTimer) clearTimeout(_settingsPreferencesAutosaveTimer); + _settingsPreferencesAutosaveTimer=setTimeout(()=>_autosavePreferencesSettings(payload),350); +} + +async function _autosavePreferencesSettings(payload){ + try{ + await api('/api/settings',{method:'POST',body:JSON.stringify(payload)}); + _settingsPreferencesAutosaveRetryPayload=null; + _setPreferencesAutosaveStatus('saved'); + // Only clear the global dirty flag and hide the unsaved-changes bar when + // there is no pending edit on a manually-saved field. Password and model + // are still committed via the explicit "Save Settings" button (password + // for security; model goes through /api/default-model). Without this + // guard, autosaving a checkbox right after a user typed in the password + // field would silently dismiss the password edit. (Opus pre-release + // review of v0.50.250, SHOULD-FIX Q1.) + const pwField=$('settingsPassword'); + const pwDirty=!!(pwField&&pwField.value); + const modelSel=$('settingsModel'); + const modelDirty=!!(modelSel&&((modelSel.value||'')!==(_settingsHermesDefaultModelOnOpen||''))); + if(!pwDirty&&!modelDirty){ + _settingsDirty=false; + const bar=$('settingsUnsavedBar'); + if(bar) bar.style.display='none'; + } + }catch(e){ + console.warn('[settings] preferences autosave failed', e); + _setPreferencesAutosaveStatus('failed'); + } +} + +function _retryPreferencesAutosave(){ + const payload=_settingsPreferencesAutosaveRetryPayload||_preferencesPayloadFromUi(); + _setPreferencesAutosaveStatus('saving'); + _autosavePreferencesSettings(payload); +} + async function loadSettingsPanel(){ try{ const settings=await api('/api/settings'); @@ -2761,7 +2862,7 @@ async function loadSettingsPanel(){ } // Send key preference const sendKeySel=$('settingsSendKey'); - if(sendKeySel){sendKeySel.value=settings.send_key||'enter';sendKeySel.addEventListener('change',_markSettingsDirty,{once:false});} + if(sendKeySel){sendKeySel.value=settings.send_key||'enter';sendKeySel.addEventListener('change',_schedulePreferencesAutosave,{once:false});} // Language preference — populate from LOCALES bundle const langSel=$('settingsLanguage'); if(langSel){ @@ -2774,20 +2875,20 @@ async function loadSettingsPanel(){ } } langSel.value=resolvedLanguage; - langSel.addEventListener('change',_markSettingsDirty,{once:false}); + langSel.addEventListener('change',_schedulePreferencesAutosave,{once:false}); } const showUsageCb=$('settingsShowTokenUsage'); - if(showUsageCb){showUsageCb.checked=!!settings.show_token_usage;showUsageCb.addEventListener('change',_markSettingsDirty,{once:false});} + if(showUsageCb){showUsageCb.checked=!!settings.show_token_usage;showUsageCb.addEventListener('change',_schedulePreferencesAutosave,{once:false});} const simplifiedToolCb=$('settingsSimplifiedToolCalling'); - if(simplifiedToolCb){simplifiedToolCb.checked=settings.simplified_tool_calling!==false;simplifiedToolCb.addEventListener('change',_markSettingsDirty,{once:false});} + if(simplifiedToolCb){simplifiedToolCb.checked=settings.simplified_tool_calling!==false;simplifiedToolCb.addEventListener('change',_schedulePreferencesAutosave,{once:false});} const showCliCb=$('settingsShowCliSessions'); - if(showCliCb){showCliCb.checked=!!settings.show_cli_sessions;showCliCb.addEventListener('change',_markSettingsDirty,{once:false});} + if(showCliCb){showCliCb.checked=!!settings.show_cli_sessions;showCliCb.addEventListener('change',_schedulePreferencesAutosave,{once:false});} const syncCb=$('settingsSyncInsights'); - if(syncCb){syncCb.checked=!!settings.sync_to_insights;syncCb.addEventListener('change',_markSettingsDirty,{once:false});} + if(syncCb){syncCb.checked=!!settings.sync_to_insights;syncCb.addEventListener('change',_schedulePreferencesAutosave,{once:false});} const updateCb=$('settingsCheckUpdates'); - if(updateCb){updateCb.checked=settings.check_for_updates!==false;updateCb.addEventListener('change',_markSettingsDirty,{once:false});} + if(updateCb){updateCb.checked=settings.check_for_updates!==false;updateCb.addEventListener('change',_schedulePreferencesAutosave,{once:false});} const soundCb=$('settingsSoundEnabled'); - if(soundCb){soundCb.checked=!!settings.sound_enabled;soundCb.addEventListener('change',_markSettingsDirty,{once:false});} + if(soundCb){soundCb.checked=!!settings.sound_enabled;soundCb.addEventListener('change',_schedulePreferencesAutosave,{once:false});} // TTS settings (localStorage-only, no server round-trip needed) const ttsEnabledCb=$('settingsTtsEnabled'); if(ttsEnabledCb){ttsEnabledCb.checked=localStorage.getItem('hermes-tts-enabled')==='true';ttsEnabledCb.onchange=function(){localStorage.setItem('hermes-tts-enabled',this.checked?'true':'false');_applyTtsEnabled(this.checked);};} @@ -2829,29 +2930,36 @@ async function loadSettingsPanel(){ ttsPitchSlider.oninput=function(){if(ttsPitchValue)ttsPitchValue.textContent=parseFloat(this.value).toFixed(1);localStorage.setItem('hermes-tts-pitch',this.value);}; } const notifCb=$('settingsNotificationsEnabled'); - if(notifCb){notifCb.checked=!!settings.notifications_enabled;notifCb.addEventListener('change',_markSettingsDirty,{once:false});} + if(notifCb){notifCb.checked=!!settings.notifications_enabled;notifCb.addEventListener('change',_schedulePreferencesAutosave,{once:false});} // show_thinking has no settings panel checkbox — controlled via /reasoning show|hide const sidebarDensitySel=$('settingsSidebarDensity'); if(sidebarDensitySel){ sidebarDensitySel.value=settings.sidebar_density==='detailed'?'detailed':'compact'; - sidebarDensitySel.addEventListener('change',_markSettingsDirty,{once:false}); + sidebarDensitySel.addEventListener('change',_schedulePreferencesAutosave,{once:false}); } const autoTitleRefreshSel=$('settingsAutoTitleRefresh'); if(autoTitleRefreshSel){ const val=String(settings.auto_title_refresh_every||'0'); autoTitleRefreshSel.value=['0','5','10','20'].includes(val)?val:'0'; - autoTitleRefreshSel.addEventListener('change',_markSettingsDirty,{once:false}); + autoTitleRefreshSel.addEventListener('change',_schedulePreferencesAutosave,{once:false}); } // Busy input mode const busyInputModeSel=$('settingsBusyInputMode'); if(busyInputModeSel){ const val=String(settings.busy_input_mode||'queue'); busyInputModeSel.value=['queue','interrupt','steer'].includes(val)?val:'queue'; - busyInputModeSel.addEventListener('change',_markSettingsDirty,{once:false}); + busyInputModeSel.addEventListener('change',_schedulePreferencesAutosave,{once:false}); } - // Bot name + // Bot name — debounced autosave (text input) const botNameField=$('settingsBotName'); - if(botNameField){botNameField.value=settings.bot_name||'Hermes';botNameField.addEventListener('input',_markSettingsDirty,{once:false});} + if(botNameField){ + botNameField.value=settings.bot_name||'Hermes'; + let botNameTimer=null; + botNameField.addEventListener('input',()=>{ + if(botNameTimer) clearTimeout(botNameTimer); + botNameTimer=setTimeout(_schedulePreferencesAutosave,500); + },{once:false}); + } // Password field: always blank (we don't send hash back) const pwField=$('settingsPassword'); if(pwField){pwField.value='';pwField.addEventListener('input',_markSettingsDirty,{once:false});} diff --git a/static/ui.js b/static/ui.js index 823a1c31e4..06d4493352 100644 --- a/static/ui.js +++ b/static/ui.js @@ -3172,6 +3172,11 @@ function renderMessages(){ seg.dataset.rawText=String(content).trim(); if(m._live){ currentAssistantTurn.id='liveAssistantTurn'; + // Stamp the session id on the live turn so finalizeThinkingCard() + // and other late callbacks can verify they're operating on the + // right session's DOM (the user may have switched tabs/sessions + // while this stream is still streaming). See #1366. + if(S.session) currentAssistantTurn.dataset.sessionId=S.session.session_id; seg.setAttribute('data-live-assistant','1'); } if(_ERR_MSG_RE.test(String(content||'').trim())) seg.dataset.error='1'; @@ -3543,6 +3548,7 @@ function appendLiveToolCard(tc){ if(!turn){ turn=_createAssistantTurn(); turn.id='liveAssistantTurn'; + if(S.session) turn.dataset.sessionId=S.session.session_id; // see #1366 $('msgInner').appendChild(turn); } const inner=_assistantTurnBlocks(turn); @@ -4275,6 +4281,12 @@ function _thinkingMarkup(text=''){ : `
`; } function finalizeThinkingCard(){ + // Guard: only finalize thinking card if we're looking at the session that started it. + // Without this check, switching tabs while a stream is running causes finalizeThinkingCard + // to remove/modify the thinking card DOM of the wrong session — the card belongs to the + // stream that started it, not the session currently displayed. + const _guardTurn = $('liveAssistantTurn'); + if(_guardTurn && S.session && _guardTurn.dataset.sessionId !== S.session.session_id) return; if(!isSimplifiedToolCalling()){ const row=$('thinkingRow'); if(!row) return; @@ -4324,6 +4336,7 @@ function appendThinking(text=''){ if(!turn){ turn=_createAssistantTurn(); turn.id='liveAssistantTurn'; + if(S.session) turn.dataset.sessionId=S.session.session_id; // see #1366 $('msgInner').appendChild(turn); } const blocks=_assistantTurnBlocks(turn); diff --git a/tests/test_1003_preferences_autosave.py b/tests/test_1003_preferences_autosave.py new file mode 100644 index 0000000000..5283a5c027 --- /dev/null +++ b/tests/test_1003_preferences_autosave.py @@ -0,0 +1,178 @@ +"""Regression checks for Issue #1003 Phase 2: Preferences settings autosave (PR #1369). + +Mirrors the structure of test_1003_appearance_autosave.py to verify the +preferences-panel autosave pattern is wired correctly: + +- All 13 preference fields use _schedulePreferencesAutosave (not _markSettingsDirty) +- Password field MUST still call _markSettingsDirty (security: never autosave) +- _preferencesPayloadFromUi covers all 13 fields +- _setPreferencesAutosaveStatus uses the shared i18n keys +- Status div exists in static/index.html +- _autosavePreferencesSettings clears the dirty flag and hides the unsaved bar +""" +import re +from pathlib import Path + +PANELS_JS = (Path(__file__).parent.parent / "static" / "panels.js").read_text(encoding="utf-8") +INDEX_HTML = (Path(__file__).parent.parent / "static" / "index.html").read_text(encoding="utf-8") +I18N_JS = (Path(__file__).parent.parent / "static" / "i18n.js").read_text(encoding="utf-8") + + +def _function_block(src: str, name: str) -> str: + marker = re.search(rf"(^|\n)(?:async\s+)?function\s+{re.escape(name)}\(", src) + assert marker is not None, f"{name}() not found" + start = marker.start() + next_marker = re.search(r"\n(?:function\s+\w+\(|async\s+function\s+\w+\()", src[start + 1:]) + end = start + 1 + next_marker.start() if next_marker else len(src) + return src[start:end] + + +def _load_settings_panel_block() -> str: + return _function_block(PANELS_JS, "loadSettingsPanel") + + +# ── Field-by-field autosave wiring ─────────────────────────────────────── + +PREFERENCE_FIELDS_AUTOSAVE = [ + # (DOM id, field name in _preferencesPayloadFromUi) + ("settingsSendKey", "send_key"), + ("settingsLanguage", "language"), + ("settingsShowTokenUsage", "show_token_usage"), + ("settingsSimplifiedToolCalling", "simplified_tool_calling"), + ("settingsShowCliSessions", "show_cli_sessions"), + ("settingsSyncInsights", "sync_to_insights"), + ("settingsCheckUpdates", "check_for_updates"), + ("settingsSoundEnabled", "sound_enabled"), + ("settingsNotificationsEnabled", "notifications_enabled"), + ("settingsSidebarDensity", "sidebar_density"), + ("settingsAutoTitleRefresh", "auto_title_refresh_every"), + ("settingsBusyInputMode", "busy_input_mode"), + ("settingsBotName", "bot_name"), +] + + +def test_all_13_preference_fields_have_autosave_payload_entries(): + """_preferencesPayloadFromUi must include all 13 preference fields.""" + block = _function_block(PANELS_JS, "_preferencesPayloadFromUi") + for dom_id, field in PREFERENCE_FIELDS_AUTOSAVE: + assert f"$('{dom_id}')" in block, \ + f"_preferencesPayloadFromUi missing reference to {dom_id}" + assert f"payload.{field}=" in block, \ + f"_preferencesPayloadFromUi missing payload assignment for {field}" + + +def test_preference_fields_use_schedule_autosave_not_mark_dirty(): + """All 12 listener attachments (excluding bot_name's debounce wrapper) must + use _schedulePreferencesAutosave. bot_name uses a wrapper but still + eventually calls _schedulePreferencesAutosave.""" + panel = _load_settings_panel_block() + # Each field should have at least one addEventListener call wired to the autosave + # path. We check that for each non-password/non-model field, the dirty marker + # has been replaced. + for dom_id, _field in PREFERENCE_FIELDS_AUTOSAVE: + if dom_id == "settingsBotName": + # Bot name uses a 500ms wrapper that calls _schedulePreferencesAutosave + # via setTimeout. The wrapper itself is in the loadSettingsPanel block. + assert "_schedulePreferencesAutosave" in panel, \ + "_schedulePreferencesAutosave must be referenced for bot_name flow" + continue + # For other fields: search the field's block for the addEventListener call + # and verify it points to _schedulePreferencesAutosave. + # We use a context window around the dom_id to find the listener. + idx = panel.find(f"$('{dom_id}')") + assert idx != -1, f"{dom_id} not loaded in loadSettingsPanel" + # Window of next ~600 chars covers the .addEventListener call + window = panel[idx:idx + 600] + assert "addEventListener" in window, f"{dom_id} has no addEventListener" + assert "_schedulePreferencesAutosave" in window, \ + f"{dom_id} listener should call _schedulePreferencesAutosave (Phase 2 #1003)" + assert "_markSettingsDirty" not in window, \ + f"{dom_id} should not call _markSettingsDirty (Phase 2 autosaves it)" + + +def test_password_still_uses_mark_dirty(): + """SECURITY INVARIANT: password field must NEVER autosave; it must still + call _markSettingsDirty so user explicitly clicks Save Settings.""" + panel = _load_settings_panel_block() + idx = panel.find("$('settingsPassword')") + assert idx != -1, "settingsPassword field not loaded" + window = panel[idx:idx + 400] + assert "_markSettingsDirty" in window, \ + "Password field MUST call _markSettingsDirty (security: never autosave passwords)" + assert "_schedulePreferencesAutosave" not in window, \ + "Password field MUST NOT call _schedulePreferencesAutosave (security)" + + +def test_autosave_clears_dirty_flag_and_hides_unsaved_bar(): + """_autosavePreferencesSettings must clear the dirty flag and hide the + unsaved-changes bar on success — but ONLY when password and model are + not pending. Q1 from Opus pre-release review of v0.50.250.""" + block = _function_block(PANELS_JS, "_autosavePreferencesSettings") + # Must check pwField/modelSel state before clearing dirty + hiding bar + assert "settingsPassword" in block, ( + "_autosavePreferencesSettings must check the password field before " + "clearing _settingsDirty (Opus SHOULD-FIX Q1: autosave was clobbering " + "pending password edits)" + ) + assert "settingsModel" in block, ( + "_autosavePreferencesSettings must check the model selector before " + "clearing _settingsDirty (autosave was clobbering pending model changes)" + ) + assert "_settingsHermesDefaultModelOnOpen" in block, ( + "_autosavePreferencesSettings must compare the model selector value " + "against the on-open snapshot to detect a pending change" + ) + # The clear-and-hide block must be conditional, not unconditional + compact = block.replace(" ", "").replace("\n", "") + assert "if(!pwDirty&&!modelDirty)" in compact or "if(pwDirty||modelDirty)" in compact, ( + "_autosavePreferencesSettings must guard the dirty-clear and bar-hide " + "with a condition that defers when a manual field has pending edits" + ) + + +def test_status_div_exists_in_index_html(): + """The status div must be present in index.html for status feedback.""" + assert 'id="settingsPreferencesAutosaveStatus"' in INDEX_HTML + + +def test_set_status_uses_shared_i18n_keys(): + """_setPreferencesAutosaveStatus must use the shared i18n keys from Phase 1.""" + block = _function_block(PANELS_JS, "_setPreferencesAutosaveStatus") + for key in [ + "settings_autosave_saving", + "settings_autosave_saved", + "settings_autosave_failed", + "settings_autosave_retry", + ]: + assert key in block, f"_setPreferencesAutosaveStatus must use '{key}'" + + +def test_retry_function_exists_and_falls_back_gracefully(): + """_retryPreferencesAutosave must exist and use the saved retry payload (or + rebuild from UI if unavailable).""" + block = _function_block(PANELS_JS, "_retryPreferencesAutosave") + assert "_settingsPreferencesAutosaveRetryPayload" in block, \ + "Retry must reference the stored payload" + assert "_preferencesPayloadFromUi" in block, \ + "Retry must fall back to rebuilding from UI when no stored payload" + assert "_autosavePreferencesSettings" in block, \ + "Retry must invoke _autosavePreferencesSettings" + + +def test_debounce_cancels_pending_timer_on_rapid_input(): + """_schedulePreferencesAutosave must clear any in-flight timer before + setting a new one — otherwise rapid changes queue up multiple POSTs.""" + block = _function_block(PANELS_JS, "_schedulePreferencesAutosave") + assert "clearTimeout(_settingsPreferencesAutosaveTimer)" in block, \ + "_schedulePreferencesAutosave must clearTimeout the prior timer" + assert "350" in block, \ + "_schedulePreferencesAutosave must use 350ms debounce (matching Phase 1)" + + +def test_phase1_appearance_autosave_still_passes(): + """Sanity: Phase 2 must not break Phase 1's pattern. The Appearance autosave + functions and i18n keys must still exist.""" + assert "function _appearancePayloadFromUi" in PANELS_JS + assert "function _autosaveAppearanceSettings" in PANELS_JS + assert "function _scheduleAppearanceAutosave" in PANELS_JS + assert 'id="settingsAppearanceAutosaveStatus"' in INDEX_HTML diff --git a/tests/test_pr1366_finalize_thinking_card_guard.py b/tests/test_pr1366_finalize_thinking_card_guard.py new file mode 100644 index 0000000000..8ce6fba624 --- /dev/null +++ b/tests/test_pr1366_finalize_thinking_card_guard.py @@ -0,0 +1,87 @@ +"""Regression tests for the v0.50.250 finalizeThinkingCard cross-tab guard. + +PR #1366 added an early-return guard to finalizeThinkingCard(): + const _guardTurn = $('liveAssistantTurn'); + if(_guardTurn && S.session && _guardTurn.dataset.sessionId !== S.session.session_id) return; + +The guard's correctness depends on `liveAssistantTurn.dataset.sessionId` +being set whenever the turn is created. If it's never set, the +comparison is `undefined !== ""` which is always true, and +finalizeThinkingCard() always early-returns — breaking the streaming +UI completely (every assistant turn's thinking card stays open +forever). + +These tests pin both invariants: + 1. The guard exists in finalizeThinkingCard() + 2. Every site that creates `liveAssistantTurn` also stamps the + dataset.sessionId attribute +""" + +import pathlib +import re + +REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve() +UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8") + + +def test_finalize_thinking_card_guard_exists(): + """finalizeThinkingCard() must early-return when displayed session != streaming session.""" + start = UI_JS.find("function finalizeThinkingCard()") + assert start != -1, "finalizeThinkingCard() must exist" + end = UI_JS.find("\nfunction ", start + 1) + body = UI_JS[start:end if end != -1 else len(UI_JS)] + # The guard must read dataset.sessionId from the live turn AND compare + # against S.session.session_id. The exact form must early-return. + assert "dataset.sessionId" in body, ( + "finalizeThinkingCard() must read dataset.sessionId from the live turn " + "to detect cross-tab/cross-session DOM mismatch." + ) + assert "S.session.session_id" in body, ( + "finalizeThinkingCard() guard must compare against S.session.session_id." + ) + + +def test_live_turn_creation_sites_stamp_session_id(): + """Every site that sets `turn.id='liveAssistantTurn'` must also set + `turn.dataset.sessionId`. If any site forgets the stamp, the guard in + finalizeThinkingCard() always early-returns at that branch (because + `undefined !== ""` is always true), breaking the streaming UI. + """ + # Find every block that sets the id. Track each occurrence and verify + # a dataset.sessionId stamp appears within ~5 lines after it. + sites = [] + for m in re.finditer(r"\.id=['\"]liveAssistantTurn['\"]", UI_JS): + # Find what variable name was used (e.g. `turn.id=`, `currentAssistantTurn.id=`) + line_start = UI_JS.rfind("\n", 0, m.start()) + 1 + line = UI_JS[line_start:m.end() + 1] + # Get the variable name + var_m = re.search(r"(\w+)\.id=['\"]liveAssistantTurn['\"]", line) + var_name = var_m.group(1) if var_m else "?" + # Look at the next ~500 chars for a dataset.sessionId stamp on the same var + # (500 chars accommodates an explanatory comment block before the stamp). + window = UI_JS[m.end():m.end() + 500] + stamped = bool(re.search(rf"{re.escape(var_name)}\.dataset\.sessionId\s*=", window)) + sites.append((var_name, m.start(), stamped)) + + assert sites, "Expected at least one site setting `.id='liveAssistantTurn'`" + unstamped = [(v, p) for v, p, s in sites if not s] + assert not unstamped, ( + f"Found {len(unstamped)} site(s) where `.id='liveAssistantTurn'` " + f"is set but `.dataset.sessionId` is NOT stamped within the next " + f"500 chars: {unstamped}. Without the stamp, the guard in " + f"finalizeThinkingCard() always early-returns at this branch (because " + f"undefined !== '' is always true), breaking the streaming UI. " + f"Add `if(S.session) .dataset.sessionId=S.session.session_id;` " + f"after the id assignment." + ) + + +def test_at_least_three_live_turn_sites(): + """Sanity check: there are at least 3 sites that create the live turn. + If a future refactor reduces this, the test_live_turn_creation_sites_stamp_session_id + test still catches missing stamps, but this catches accidental site removal.""" + matches = re.findall(r"\.id=['\"]liveAssistantTurn['\"]", UI_JS) + assert len(matches) >= 3, ( + f"Expected at least 3 sites assigning liveAssistantTurn id, found {len(matches)}. " + "If sites were intentionally consolidated, this assertion can be relaxed." + )