diff --git a/CHANGELOG.md b/CHANGELOG.md index 01f33142..793dfdb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### 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 diff --git a/tests/test_1003_preferences_autosave.py b/tests/test_1003_preferences_autosave.py new file mode 100644 index 00000000..39a16c5c --- /dev/null +++ b/tests/test_1003_preferences_autosave.py @@ -0,0 +1,161 @@ +"""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 — otherwise the bar shows stale state.""" + block = _function_block(PANELS_JS, "_autosavePreferencesSettings") + assert "_settingsDirty=false" in block.replace(" ", ""), \ + "_autosavePreferencesSettings must set _settingsDirty=false on success" + assert "settingsUnsavedBar" in block, \ + "_autosavePreferencesSettings must hide settingsUnsavedBar on success" + + +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