fix: check settings write result before applying in desktop UI

[praisonai-triage-agent]

Fixes #4497

Problem

saveCfg mutated CFG/cfg and called applyPrefs before await fetch(POST /settings), with no r.ok check and no catch. apply awaited it and unconditionally ran applyTheme/renderSettings; setTextSize didn't even await.

When the write was rejected — which the base_url/api_key rows explicitly warn happens across an engine restart:

• Theme became a completely dead button: no data-theme, no highlight, no error.
• Toggles diverged — cfg moved in memory but the switch never moved, and the next click flipped the invisible one back.
• Text size toasted "Text size N px" while every write silently failed.

Fix (ui/index.html)

• saveCfg now persists first, mutates only on r.ok, toasts Could not save that setting. on failure, and returns a boolean.
• apply gates applyTheme/model name/renderSettings on that boolean (if(!await saveCfg(...)) return;).
• setTextSize awaits and only toasts the size that actually stuck.

Minimal, backward-compatible; no new params, deps, or exports.

Tests (frontend/tests/settings-effects.test.mjs)

Added a failSettingsWrite option to the harness (stubs POST /settings as 500) and two tests:

• A rejected theme write applies nothing (data-theme stays off) and surfaces a toast.
• A rejected toggle write leaves the switch where it was, while still sending the attempted write.

Verified these two fail against the pre-fix code and pass with the fix. Full desktop suite: 178/178 pass.

Generated with Claude Code

[praisonai-triage-agent]

@coderabbitai review

[praisonai-triage-agent]

/review

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[greptile-apps]

Greptile Summary

The PR makes desktop settings writes persist before updating local state, reports rejected writes, serializes concurrent writes, and gates UI effects on successful persistence.

• Adds failed-write coverage for theme and toggle controls.
• Adds queued text-size updates and pending-target bookkeeping for rapid keyboard steps.
• Reconciles successful responses with the settings returned by the engine.

Confidence Score: 4/5

The PR is not yet safe to merge because the outstanding reset and pending-target interaction can persist and display the wrong text size.

The reset shortcut queues 15 px without replacing the pending zoom target, so a subsequent rapid zoom step advances from the stale value and can leave both persisted and displayed size at 18 px rather than 16 px.

Files Needing Attention: src/praisonai-desktop/ui/index.html

Important Files Changed

Filename	Overview
src/praisonai-desktop/ui/index.html	Settings writes are now serialized and failure-aware, but the reset shortcut still bypasses pending text-size synchronization and can persist the wrong final size.
src/praisonai-desktop/frontend/tests/settings-effects.test.mjs	Adds focused rejected-write and rapid-step coverage using an isolated settings endpoint stub.

_{Reviews (3): Last reviewed commit: "test: reflect POST /settings echo in stu..." | Re-trigger Greptile}

[MervinPraison]

@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Review changes in this PR. Python SDK: praisonaiagents, praisonai. TypeScript SDK: src/praisonai-ts/. Do NOT modify src/praisonai-rust. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding.

Phase 1: Review per AGENTS.md

1. Protocol-driven: check heavy implementations vs core SDK
2. Backward compatible: ensure zero feature regressions
3. Performance: no hot-path regressions
4. SDK value: review in depth whether the change genuinely adds value to the SDK — never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep
5. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.
6. Repo routing: agent-callable tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins; optional sandbox backends → PraisonAI-Plugins (praisonai.sandbox entry point) — request changes if wrongly added to praisonaiagents/

Phase 2: FIX Valid Issues
7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix
8. Also independently identify and fix any gaps or issues you find in the changed code — do not rely only on prior reviewer feedback
9. Push all code fixes directly to THIS branch (do NOT create a new PR)
10. Comment a summary of exact files modified and what you skipped

Phase 3: Final Verdict
11. If all issues are resolved, approve the PR / close the Issue
12. If blocking issues remain, request changes / leave clear action items

[praisonai-triage-agent]

Claude finished @MervinPraison's task —— View job

---

Final Architecture Review — PR #4520 ✅

Verdict on the original change: Correct and valuable. The persist-then-mutate refactor makes desktop settings writes transactional — theme, toggles, and text size no longer diverge from what actually persisted. In scope (desktop ui/index.html only), backward-compatible, no SDK surface changes, no Agent bloat, no new deps. Aligns with AGENTS.md.

Valid issue fixed (Greptile P1 — a regression this PR introduced)

Moving the CFG mutation to after await fetch meant stepTextSize read a stale CFG.font_size while a write was in flight, so a rapid Cmd+=/Cmd+- burst collapsed to one step, and overlapping writes could land out of order.

Fix (src/praisonai-desktop/ui/index.html):

• saveCfg now enqueues onto a FIFO saveQueue (persistCfg does the actual work) → overlapping writes persist/apply in order, so CFG never regresses to an older selection.
• stepTextSize steps from a pendingTextSize optimistic target when a write is in flight → rapid presses accumulate (13 → 14 → 15) instead of collapsing.
• pendingTextSize is cleared once its write resolves, so a failed write doesn't leave the UI stepping from a size that never persisted.

Test (frontend/tests/settings-effects.test.mjs):

• Added "rapid text-size steps accumulate instead of collapsing to one" — fires two back-to-back Cmd+= presses and asserts --fs reaches 15px and [14, 15] were persisted in order.

Testing

Full desktop suite: 179/179 pass (npm run test:ci), including the new race test.

Skipped / not changed

• No Python/TS SDK changes needed — issue is entirely desktop UI.
• Did not touch src/praisonai-rust (out of scope).
• No new params/exports/deps — kept minimal and backward-compatible per AGENTS.md.

Recommendation: Approve & merge. The originally-reported bug is fixed and the one valid reviewer concern (Greptile P1) is resolved with test coverage.

• Read AGENTS.md & changed files
• Validate Greptile P1 (real regression)
• Fix: serialize writes + step from pending target
• Run desktop test suite (179/179)
• Push to branch & final verdict
  • Branch

[greptile-apps]

Reset bypasses pending target

When the user rapidly presses zoom-in, reset, then zoom-in again, reset queues 15 without updating pendingTextSize, so the final step advances the stale pending value from 16 to 18. The writes persist 16, 15, then 18, leaving the displayed and stored size at 18 px instead of the expected 16 px.

[MervinPraison]

@claude this PR has merge conflicts with main. Please rebase onto latest main, resolve conflicts (keep this PR's intent, merge in newer main logic), run targeted tests, and force-push with --force-with-lease. Comment which files you resolved. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params. Keep the resolution minimal — the aim is a lightweight and powerful package; do not introduce scope creep while resolving conflicts.

[praisonai-triage-agent]

Claude finished @MervinPraison's task —— View job

---

Rebase complete ✅

Rebased claude/issue-4497-20260827-1341 onto latest main (6e8005137), resolved conflicts, and force-pushed with --force-with-lease.

• Setup git identity & auth
• Fetch latest main and rebase
• Resolve conflicts (keep PR intent + newer main logic)
• Run targeted desktop tests
• Force-push with --force-with-lease
• Comment resolved files

Files resolved

src/praisonai-desktop/ui/index.html — the only conflicting file. Merged both intents into persistCfg:

• This PR's fix (desktop: Every settings write reports success unchecked; one failure mode makes Theme a completely dead button #4497): persist first, gate on r.ok, toast Could not save that setting. and return falsy on failure.
• Newer main logic: reconcile the server's echoed settings back into CFG/cfg (e.g. launch_at_login the OS refused), reading r.json() from the same response instead of a second fetch.
• Return value is the server object (truthy) on success / false on failure, so callers can both gate (if(!saved) return;) and read saved.launch_at_login_result.
• Kept the second commit's FIFO saveQueue + pendingTextSize (Greptile P1) intact on top.

src/praisonai-desktop/frontend/tests/settings-effects.test.mjs — small harness fix (no conflict, but needed after the merge): the POST /settings stub now echoes the written patch like the real engine. Previously it returned the original settings for both GET and POST, so reconciliation overwrote the just-persisted value and the "rapid text-size steps" test collapsed 13→13.

Tests

Full desktop suite: 180/180 pass (npm run test:ci), including the rapid-step race test and the rejected-write tests.

Skipped / not changed

• No Python/TS SDK changes — issue is entirely desktop UI. No new params/exports/deps; Agent class untouched.
• Did not touch src/praisonai-rust (out of scope).
• The open Greptile "reset bypasses pending target" comment was not in scope of this rebase request; the merge preserves the existing behavior without expanding surface area.
  • Branch

[praisonai-triage-agent]

Merge gate scan — eligible for assessment. Claude merge gate will assess and may auto-merge if MERGE_GATE_VERDICT: APPROVE.

[praisonai-triage-agent]

Merged by Claude PR merge gate (claude-merge-gate.yml).
Verdict: MERGE_GATE_VERDICT: APPROVE
SHA: e8b82ad
Method: merge