fix(autosave): guard preferences-autosave dirty-clear when password/model pending (Opus SHOULD-FIX Q1)
Pre-release Opus review of v0.50.250 caught a UX regression in PR #1369: _autosavePreferencesSettings unconditionally cleared _settingsDirty=false and hid the unsaved-changes bar on every successful autosave. But password and model are still committed via the explicit 'Save Settings' button (password for security; model goes through /api/default-model). Race scenario: 1. User opens System pane, types a new password (sets _settingsDirty=true; bar appears on close) 2. User switches to Preferences, toggles any checkbox -> autosave fires -> _settingsDirty=false, bar permanently suppressed 3. User closes panel -> _closeSettingsPanel short-circuits because !_settingsDirty -> typed password silently discarded (loadSettingsPanel blanks pwField.value='' on next open) Same shape with model selector: pick a new default model, then toggle any preference -> autosave fires -> no warning on close -> model never persists. Fix: only clear _settingsDirty and hide settingsUnsavedBar when both the password field is empty AND the model selector matches its on-open snapshot. Pinned by an updated regression test asserting the conditional guard exists.
This commit is contained in:
1 parent
3a13be297e
commit
f8754ded70
2 files changed
+38
-8
No files matched your search
+16
-3
@@ -2748,9 +2748,22 @@ async function _autosavePreferencesSettings(payload){
|
||||
await api('/api/settings',{method:'POST',body:JSON.stringify(payload)});
|
||||
_settingsPreferencesAutosaveRetryPayload=null;
|
||||
_setPreferencesAutosaveStatus('saved');
|
||||
_settingsDirty=false;
|
||||
const bar=$('settingsUnsavedBar');
|
||||
if(bar) bar.style.display='none';
|
||||
// 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');
|
||||
|
||||
@@ -105,12 +105,29 @@ def test_password_still_uses_mark_dirty():
|
||||
|
||||
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."""
|
||||
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")
|
||||
assert "_settingsDirty=false" in block.replace(" ", ""), \
|
||||
"_autosavePreferencesSettings must set _settingsDirty=false on success"
|
||||
assert "settingsUnsavedBar" in block, \
|
||||
"_autosavePreferencesSettings must hide settingsUnsavedBar on success"
|
||||
# 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():
|
||||
|
||||
Reference in new issue
Block a user