fix: persist launch_at_login's honest result, not the request

[praisonai-triage-agent]

Fixes #4498

Problem

The desktop /settings handler wrote the requested launch_at_login
value first, then only attached set_launch_at_login(...)'s result to the
response as launch_at_login_result — which nothing reads. On Windows/Linux,
and on macOS running from a checkout, set_launch_at_login always returns
{"enabled": false, "message": "Only available in the installed app."}. So
the toggle rendered on, persisted, and survived restarts while no login item
was ever registered.

Fix (engine/server.py)

Run the platform action first, coerce the persisted launch_at_login to
what actually happened (result["enabled"]), then save. The honest result is
still surfaced as launch_at_login_result so the UI can show the explanation
when enabled came back false.

Test (engine/test_portability.py)

New LaunchAtLogin class drives the real /settings POST handler with
set_launch_at_login stubbed to the non-bundle result and asserts the
persisted settings read back false and the response carries the
explanation. Verified this fails on the old code (persisted true) and passes
with the fix. Full suite: 49 tests 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 persists the effective launch-at-login result and reconciles the settings UI with server responses.

• Runs the platform login-item action before storing its reported state.
• Returns the platform result so the UI can display registration failures.
• Adds handler-level portability tests for successful and declined registration.
• Updates generic client reconciliation so returned setting values replace optimistic state.

Confidence Score: 3/5

The PR is not yet safe to merge because stale settings responses can override newer user choices and a failed settings write can still leave login registration inconsistent with durable state.

Settings requests are neither serialized nor checked for stale responses, while the login-item side effect still occurs before a fallible persistence operation without rollback.

Files Needing Attention: src/praisonai-desktop/ui/index.html and src/praisonai-desktop/engine/server.py

Important Files Changed

Filename	Overview
src/praisonai-desktop/engine/server.py	Persists the platform-reported login-item state, but the previously reported registration-before-persistence divergence remains outstanding.
src/praisonai-desktop/ui/index.html	Reconciles optimistic settings with server responses, but unsequenced responses can restore an older value after rapid repeated changes.
src/praisonai-desktop/engine/test_portability.py	Adds focused coverage for persisting successful and declined launch-at-login outcomes.

Sequence Diagram

sequenceDiagram
  participant U as User
  participant UI as Settings UI
  participant S as Desktop engine
  participant OS as Login-item service
  participant D as Settings file
  U->>UI: Change launch_at_login
  UI->>S: POST /settings
  S->>OS: set_launch_at_login(requested)
  OS-->>S: Effective enabled state
  S->>D: Persist effective state
  S-->>UI: Saved settings and platform result
  UI->>UI: Reconcile and rerender

Loading

_{Reviews (2): Last reviewed commit: "fix: reconcile launch_at_login toggle fr..." | Re-trigger Greptile}

[greptile-apps]

Registration can outlive persistence

If login-item registration succeeds and save_settings then raises, the new ordering leaves the login item registered without updating durable settings, causing the application to launch at login after restart while the saved option remains off or stale.

Knowledge Base Used: Persistence, security, and configuration

[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 #4507

• Read changed files (server.py, test_portability.py)
• Read UI settings client (ui/index.html)
• Evaluate Greptile P1 issues
• Fix valid issues
• Test & push
• Final verdict

---

Assessment

The core fix in server.py:1677-1687 is correct and valuable: run the platform action first, then persist result["enabled"] (what actually happened) rather than the request. This is the right, minimal fix for #4498 — no new params, no Agent bloat, scope stays inside the desktop app.

Reviewer feedback

Greptile P1 #1 — "Effective state stays stale" → VALID, FIXED.
saveCfg optimistically wrote launch_at_login=true into local CFG/cfg and discarded the POST response, so renderSettings() re-rendered from stale local state. The server persisted false, but the running session's toggle stayed on and no explanation showed. Fixed in ui/index.html:

• saveCfg now awaits and parses the response and reconciles every key the server wrote back (CFG[k]=saved[k]), then re-applies prefs. Generic, so any future server-coerced value stays honest too.
• The launch_at_login handler now surfaces launch_at_login_result.message in the row's existing .warn element when the OS declines, so the user sees why the toggle snapped back.

Greptile P1 #2 — "Registration can outlive persistence" → acknowledged, NOT fixed (low value / high risk).
The scenario: set_launch_at_login succeeds, then save_settings raises, leaving a real login item without matching settings. In practice save_settings writes a local JSON file and effectively never raises on the happy path; adding a rollback that re-invokes the platform action on a disk-write failure introduces a second failure mode (rollback itself can fail) for a case that isn't reachable in normal use. Adding that machinery would be scope creep against the "lightweight" mandate. Left as-is deliberately.

Verification

• python -m unittest test_portability → 49/49 pass (incl. the new LaunchAtLogin handler test that pins the server contract the UI now consumes).
• Modified JS logic validated for parse + branch correctness with node.

Files modified by me

• src/praisonai-desktop/ui/index.html — saveCfg reconciles from server response; launch_at_login explanation surfaced.

Skipped

• Greptile P1 Merge pull request #1 from MervinPraison/develop #2 rollback (justified above).
• server.py / test_portability.py — already correct, no changes needed.

Verdict: ✅ Approve

The server persists the honest result and the UI now reflects it in-session with a clear explanation. Backward-compatible, no hot-path impact, no core-SDK surface change.
· Branch: claude/issue-4498-20260827-1337

[greptile-apps]

Stale responses overwrite settings

If the same setting is changed again before its first request completes, each response unconditionally overwrites CFG and cfg. An older response arriving last therefore restores the earlier value and rerenders the UI against the user's latest choice.

[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: 590beb8
Method: merge