From d4059e99a8af7b04cc3a5c54c192d353a96eebf1 Mon Sep 17 00:00:00 2001 From: "praisonai-triage-agent[bot]" <272766704+praisonai-triage-agent[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 13:49:40 +0000 Subject: [PATCH 1/3] fix: check settings write result before applying in desktop UI (fixes #4497) saveCfg mutated CFG/cfg and applied prefs before POSTing /settings, with no r.ok check and no catch. A rejected write (e.g. across the engine restart the base_url/api_key rows warn about) left the theme button dead, toggles diverged from stored state, and setTextSize toasted success while every write failed. saveCfg now persists first, mutates only on r.ok, toasts on failure, and returns a boolean. apply() and setTextSize gate their visible effects on it. Co-authored-by: MervinPraison --- .../frontend/tests/settings-effects.test.mjs | 61 ++++++++++++++++++- src/praisonai-desktop/ui/index.html | 34 +++++++---- 2 files changed, 83 insertions(+), 12 deletions(-) diff --git a/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs b/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs index 080ef4148..e2c342697 100644 --- a/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs +++ b/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs @@ -40,7 +40,8 @@ function sseReader(frames) { : { done: true, value: undefined }) }; } -async function boot(cfg = {}, { confirmAnswer = true, sse = [], prefersReducedMotion = false } = {}) { +async function boot(cfg = {}, { confirmAnswer = true, sse = [], prefersReducedMotion = false, + failSettingsWrite = false } = {}) { const calls = []; const bodies = []; const settings = { @@ -52,6 +53,11 @@ async function boot(cfg = {}, { confirmAnswer = true, sse = [], prefersReducedMo calls.push(`${opts.method || 'GET'} ${path}`); if (opts.body) { try { bodies.push(JSON.parse(opts.body)); } catch {} } const u = String(url); + // Simulate the engine rejecting a settings write -- the very case the UI + // itself warns about across a base_url/api_key change that restarts it. + if (failSettingsWrite && (opts.method || 'GET') === 'POST' && u.includes('/settings')) { + return { ok: false, status: 500, json: async () => ({}), text: async () => '' }; + } const body = u.includes('/settings') ? settings : u.includes('/chats/') ? { id: 'c1', title: 'Hi', messages: [] } @@ -166,6 +172,59 @@ test('theme "system" leaves the attribute off so the OS decides', async () => { assert.equal(b.doc.documentElement.getAttribute('data-theme'), null); }); +// --- a settings write the engine rejects ------------------------------------ + +/** Open Settings and switch to the named section, waiting for it to render. */ +async function openSettings(b, section) { + click(b.doc.getElementById('settings')); + for (let i = 0; i < 40; i++) { + await new Promise((r) => setTimeout(r, 25)); + if (b.doc.getElementById('setbody')) break; + } + assert.ok(b.doc.querySelector('#setbody .srow'), 'settings never opened'); + const nav = [...b.doc.querySelectorAll('#setside button')] + .find((x) => x.textContent.includes(section)); + assert.ok(nav, `the ${section} section is missing`); + click(nav); + await new Promise((r) => setTimeout(r, 40)); +} + +test('a rejected theme write applies nothing and tells the user', async () => { + // saveCfg used to mutate first and never check r.ok, so a 500 left the button + // un-highlighted, no data-theme, and no error -- a completely dead button. + const b = await boot({ theme: 'system' }, { failSettingsWrite: true }); + await openSettings(b, 'Appearance'); + const light = [...b.doc.querySelectorAll('#setting-theme .seg button')] + .find((x) => x.textContent === 'Light'); + assert.ok(light, 'the Light option is missing'); + click(light); + await new Promise((r) => setTimeout(r, 80)); + assert.equal(b.doc.documentElement.getAttribute('data-theme'), null, + 'a failed write still applied the theme'); + const t = b.doc.getElementById('toast'); + assert.ok(t && t.classList.contains('show'), 'no error was surfaced'); +}); + +test('a rejected toggle write leaves the switch where it was', async () => { + // The toggle mutated cfg before the write, so a rejection left the switch and + // the stored value disagreeing and the next click flipped the wrong one back. + // The observable contract: a failed write must not move the switch, and the + // engine must never see a value the user did not manage to persist. + const b = await boot({ confirm_delete: true }, { failSettingsWrite: true }); + await openSettings(b, 'Safety'); + const sw = b.doc.querySelector('#setting-confirm_delete .sw'); + assert.ok(sw, 'the confirm_delete toggle is missing'); + assert.equal(sw.classList.contains('on'), true, 'toggle did not start on'); + b.bodies.length = 0; + click(sw); + await new Promise((r) => setTimeout(r, 80)); + const now = b.doc.querySelector('#setting-confirm_delete .sw'); + assert.equal(now.classList.contains('on'), true, + 'a failed write moved the switch to a state that was never persisted'); + assert.equal(b.bodies.some((x) => x && 'confirm_delete' in x && x.confirm_delete === false), + true, 'the attempted write should still have been sent'); +}); + // --- safety ------------------------------------------------------------------ /** Answer the in-app confirmation, and fail loudly if none appeared. */ diff --git a/src/praisonai-desktop/ui/index.html b/src/praisonai-desktop/ui/index.html index a413a9fe3..d8732b9e9 100644 --- a/src/praisonai-desktop/ui/index.html +++ b/src/praisonai-desktop/ui/index.html @@ -1812,9 +1812,10 @@

Fine-tune a model

const def=SETTINGS.find(d=>d.key==='font_size'); return def.control.options.map(o=>Number(o.value)); } -function setTextSize(px){ - saveCfg({font_size:px}); - toast('Text size '+px+' px'); +async function setTextSize(px){ + // Await the write and only announce the size that actually stuck; toasting + // "Text size N px" while every write failed is the same lie as the toggles. + if(await saveCfg({font_size:px})) toast('Text size '+px+' px'); } function stepTextSize(dir){ const sizes=textSizes(); @@ -1832,21 +1833,28 @@

Fine-tune a model

const cfgGet = k => (k in cfg ? cfg[k] : DEFAULTS[k]); async function saveCfg(patch){ - CFG={...CFG,...patch}; applyPrefs(CFG); - cfg = {...cfg, ...patch}; - let saved=null; + // Persist first, mutate only on success. Mutating before the write and never + // checking it is how a toggle moved in memory, never moved on screen, and + // un-flipped itself on the next click. + let r=null; try{ - saved=await (await fetch('http://127.0.0.1:'+PORT+'/settings',{method:'POST', - headers:{'content-type':'application/json'},body:JSON.stringify(patch)})).json(); - }catch{} + r=await fetch('http://127.0.0.1:'+PORT+'/settings',{method:'POST', + headers:{'content-type':'application/json'},body:JSON.stringify(patch)}); + }catch(_){} + if(!r||!r.ok){ toast('Could not save that setting.'); return false; } + let saved=null; + try{ saved=await r.json(); }catch{} + CFG={...CFG,...patch}; cfg={...cfg,...patch}; // The server persists what actually happened, not what we asked. Reconcile // any key it wrote back so the current session never shows a value that was // never saved -- e.g. "Open at login" the OS refused to register. if(saved && typeof saved==='object'){ for(const k of Object.keys(patch)) if(k in saved){ CFG[k]=saved[k]; cfg[k]=saved[k]; } - applyPrefs(CFG); } - return saved; + applyPrefs(CFG); + // Return the server object (truthy) so callers can both gate on success and + // read reconciliation details like launch_at_login_result. + return saved && typeof saved==='object' ? saved : true; } function control(def, onChange){ @@ -1998,7 +2006,11 @@

Fine-tune a model

err.style.display='none'; if(def.confirm && def.confirm.when(cfgGet(def.key),nv) && !await askConfirm(def.confirm.message,{ok:'Continue'})) { renderSettings(); return; } + // Don't advance the visible state on a write that failed: saveCfg has + // already told the user, and applying the theme or re-rendering here would + // show a value that was never persisted. const saved=await saveCfg({[def.key]:nv}); + if(!saved) return; if(def.key==='theme') applyTheme(nv); if(def.key==='model') modelName.textContent=nv; renderSettings(); From 56ace2979932920f431d20af9b4f162018a48be6 Mon Sep 17 00:00:00 2001 From: "praisonai-triage-agent[bot]" <272766704+praisonai-triage-agent[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 14:40:55 +0000 Subject: [PATCH 2/3] fix: serialize settings writes and step text size from pending target Moving the CFG mutation to after the await made rapid text-size steps read stale state, collapsing bursts and letting overlapping writes land out of order. Queue writes FIFO and step from the pending target so bursts accumulate (fixes Greptile P1). Co-authored-by: Mervin Praison --- .../frontend/tests/settings-effects.test.mjs | 17 ++++++++++++ src/praisonai-desktop/ui/index.html | 27 ++++++++++++++++--- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs b/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs index e2c342697..406e61dc3 100644 --- a/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs +++ b/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs @@ -111,6 +111,23 @@ test('font_size changes the variable the message text is sized from', async () = assert.equal(large.doc.documentElement.style.getPropertyValue('--fs'), '20px'); }); +test('rapid text-size steps accumulate instead of collapsing to one', async () => { + // saveCfg only updates CFG once the write resolves. Two quick zoom-in presses + // fired back-to-back would otherwise both read the same starting size and + // land on the same next value; the pending-target bookkeeping must let them + // walk 13 -> 14 -> 15 across the two writes. + const b = await boot({ font_size: 13 }); + const zoom = () => b.doc.dispatchEvent(new b.window.KeyboardEvent('keydown', + { key: '=', metaKey: true, bubbles: true, cancelable: true })); + zoom(); + zoom(); + await new Promise((r) => setTimeout(r, 120)); + assert.equal(b.doc.documentElement.style.getPropertyValue('--fs'), '15px', + 'two quick steps collapsed to a single step'); + const sent = b.bodies.filter((x) => x && 'font_size' in x).map((x) => x.font_size); + assert.deepEqual(sent, [14, 15], `expected 14 then 15 to be persisted, got ${sent}`); +}); + test('code_font_size is its own setting, and scales with the interface', async () => { // It used to be an absolute px value, so at text size 18 the prose grew and // code blocks stayed put. It is now multiplied by the same scale -- which diff --git a/src/praisonai-desktop/ui/index.html b/src/praisonai-desktop/ui/index.html index d8732b9e9..0f0026e41 100644 --- a/src/praisonai-desktop/ui/index.html +++ b/src/praisonai-desktop/ui/index.html @@ -1815,16 +1815,25 @@

Fine-tune a model

async function setTextSize(px){ // Await the write and only announce the size that actually stuck; toasting // "Text size N px" while every write failed is the same lie as the toggles. - if(await saveCfg({font_size:px})) toast('Text size '+px+' px'); + const ok=await saveCfg({font_size:px}); + if(ok) toast('Text size '+px+' px'); + // Once the queue drains, drop the optimistic target so the next burst starts + // from CFG again -- and a failed write does not leave us stepping from a size + // that never persisted. + if(pendingTextSize===px) pendingTextSize=null; } +// The size a rapid burst of steps is walking toward. saveCfg only updates CFG +// once the write resolves, so consecutive presses would otherwise read the same +// stale value and collapse to one step; track the pending target here instead. +let pendingTextSize=null; function stepTextSize(dir){ const sizes=textSizes(); - const now=Number(CFG.font_size)||BASE_TEXT_PX; + const now=pendingTextSize!=null?pendingTextSize:(Number(CFG.font_size)||BASE_TEXT_PX); let i=sizes.indexOf(now); if(i<0){ i=sizes.reduce((best,v,k)=> Math.abs(v-now)Fine-tune a model let cfg = {}; const cfgGet = k => (k in cfg ? cfg[k] : DEFAULTS[k]); -async function saveCfg(patch){ +// Writes run one at a time, in the order they were requested. Without this a +// burst of steps could resolve out of order and leave CFG -- and the persisted +// value -- at an older selection than the last one the user asked for. +let saveQueue=Promise.resolve(); +function saveCfg(patch){ + const run=saveQueue.then(()=>persistCfg(patch)); + // Keep the chain alive even if one write throws, so later writes still run. + saveQueue=run.catch(()=>{}); + return run; +} +async function persistCfg(patch){ // Persist first, mutate only on success. Mutating before the write and never // checking it is how a toggle moved in memory, never moved on screen, and // un-flipped itself on the next click. From e8b82ad9abfcec5891243e54ffc325f09da1f1c0 Mon Sep 17 00:00:00 2001 From: "praisonai-triage-agent[bot]" <272766704+praisonai-triage-agent[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 03:23:22 +0000 Subject: [PATCH 3/3] test: reflect POST /settings echo in stub so reconciliation reads the write After rebasing onto main, saveCfg reconciles the server's echoed settings back into CFG. The stub returned the original settings for both GET and POST, so the persisted step was overwritten and rapid text-size steps collapsed. Echo the written patch like the real engine does. Co-authored-by: Mervin Praison --- .../frontend/tests/settings-effects.test.mjs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs b/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs index 406e61dc3..35fb81cd2 100644 --- a/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs +++ b/src/praisonai-desktop/frontend/tests/settings-effects.test.mjs @@ -58,6 +58,11 @@ async function boot(cfg = {}, { confirmAnswer = true, sse = [], prefersReducedMo if (failSettingsWrite && (opts.method || 'GET') === 'POST' && u.includes('/settings')) { return { ok: false, status: 500, json: async () => ({}), text: async () => '' }; } + // The real engine persists the write and echoes the stored settings back; + // reconciliation reads that echo, so the stub must reflect what it was sent. + if ((opts.method || 'GET') === 'POST' && u.includes('/settings') && opts.body) { + try { Object.assign(settings, JSON.parse(opts.body)); } catch {} + } const body = u.includes('/settings') ? settings : u.includes('/chats/') ? { id: 'c1', title: 'Hi', messages: [] }