fix: replace non-functional alert()/prompt() in desktop UI by praisonai-triage-agent[bot] · Pull Request #4521 · MervinPraison/PraisonAI

praisonai-triage-agent · 2026-08-27T13:54:49Z

What

A WKWebView under Tauri implements no JS dialog panel, so prompt() returns null and alert() shows nothing. This made three sites non-functional in a page that already documents JS dialogs as unavailable (the reason askConfirm exists):

ui/index.html "Move to project" right-click used prompt() → issued zero requests.
MCP add error paths used alert() → showed nothing for a blank name or an engine rejection.

Changes

Add askText() — an in-app text prompt mirroring askConfirm() (promise-based, resolves to the string or null), plus matching .confirm-box .txt styling.
Route the three sites through askText / toast:
- "Move to project" → await askText(...)
- blank MCP name → toast('A name is required.')
- MCP add error → toast(r.error)

Test

Updated frontend/tests/buttons.test.mjs stubs to model the real platform: w.prompt = () => null and w.alert = () => { throw ... } (the old () => 'x' / no-op stubs hid the bug).
Added a test that fires contextmenu on a chat row, fills the in-app prompt, and asserts a POST /project/<id> request is issued.

All 177 desktop tests pass (npm run test:ci).

Generated with Claude Code

WKWebView under Tauri has no JS dialog panel, so prompt() returns null and alert() shows nothing. This made "Move to project" inert and hid MCP add errors. Add an askText() helper mirroring askConfirm(), and route the three sites through askText/toast. Update the button test stubs to model the real platform (prompt->null, alert throws) and add a contextmenu test asserting a POST /project request is issued. Co-authored-by: MervinPraison <454862+MervinPraison@users.noreply.github.com>

praisonai-triage-agent · 2026-08-27T13:55:06Z

@coderabbitai review

praisonai-triage-agent · 2026-08-27T13:55:07Z

/review

qodo-code-review · 2026-08-27T13:55:10Z

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 · 2026-08-27T13:57:36Z

Greptile Summary

The PR replaces unsupported native webview dialogs with in-app prompt and confirmation UI. It adds a text-entry dialog for moving chats, routes MCP errors through visible dialogs, and updates desktop tests to model WKWebView behavior; however, dismissing an MCP error with Escape also closes the underlying MCP form.

Confidence Score: 4/5

The PR needs a fix before merging because dismissing an MCP error with Escape also closes the underlying MCP form.

The new MCP error dialog and the existing overlay both process the same document-level Escape event, causing users to lose their in-progress MCP form when they dismiss only the foreground error.

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

Important Files Changed

Filename	Overview
src/praisonai-desktop/ui/index.html	Adds askText and visible MCP error dialogs, but the new nested-dialog path lets one Escape event close both the error and MCP overlays.
src/praisonai-desktop/frontend/tests/buttons.test.mjs	Models unavailable native dialogs and verifies that the in-app text prompt issues a project-move request.

_{Reviews (2): Last reviewed commit: "fix: surface MCP add errors via in-app d..." | Re-trigger Greptile}

MervinPraison · 2026-08-27T14:30:49Z

@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

Protocol-driven: check heavy implementations vs core SDK
Backward compatible: ensure zero feature regressions
Performance: no hot-path regressions
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
Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.
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 · 2026-08-27T14:41:01Z

Claude finished @MervinPraison's task —— View job

Final Architecture Review — PR #4521 ✅

Read changed files & test harness
Validate Greptile's z-index/toast finding
Independently review changes
Fix valid issue (pushed)
Run desktop tests (177/177 pass)
Push & summarize verdict

Phase 1 — AGENTS.md Review

Check	Verdict
Scope/routing	✅ Desktop UI (`src/praisonai-desktop/`) only. No changes to `praisonaiagents`, `praisonai`, `praisonai-ts`, or `praisonai-rust`. No Agent-class bloat. Out of core SDK surface — correct.
Backward compatible	✅ No feature regressions. Native `prompt()`/`alert()` were already dead in the WKWebView; this restores broken behaviour.
Performance	✅ No hot-path impact.
SDK value	✅ Genuine bug fix, not scope creep. `askText()` mirrors the existing `askConfirm()` pattern the page already documents as necessary; three dead sites now function. Minimal surface area.

Phase 2 — Issue Found & Fixed

Greptile's finding was valid, but its suggested fix (raise toast z-index) was wrong. Raising #toast above the overlay breaks an existing tested invariant — layout.test.mjs:222 "the settings scrim covers the toast" deliberately keeps the toast dimmed under the scrim. My first attempt (z-index 95) failed that test.

Correct fix instead: the MCP blank-name and backend-error paths (ui/index.html:1992,1996) were calling toast() while the MCP overlay is open — so the feedback was painted beneath the translucent scrim, reintroducing the very invisible-feedback bug this PR fixes. I rerouted both through askConfirm, whose .confirm-back (z-index 90) paints above the overlay (z-index 80), consistent with the page's own "no native dialogs → in-app equivalents" philosophy. #toast z-index left untouched at 70.

Files modified (pushed to 8cf894817):

src/praisonai-desktop/ui/index.html — MCP blank-name & error paths now use askConfirm(...,{danger:false,ok:'OK',cancel:'Dismiss'}) instead of toast(...).

Independently verified: no live alert()/prompt() calls remain (only a comment reference); askText() has sound cancel/cleanup/keydown handling matching askConfirm().

Skipped: Greptile's literal z-index suggestion (would break layout.test.mjs).

Phase 3 — Verdict: ✅ Approve

All 177 desktop tests pass (npm run test:ci). The one valid reviewer finding is resolved without violating the existing scrim/toast layering contract. No blocking issues remain.

Job run · Branch

The toast intentionally paints below the settings scrim (see layout.test 'the settings scrim covers the toast'), so blank-name and backend-error feedback shown while the MCP overlay is open was dimmed and unreadable -- the same invisible-feedback bug this PR set out to fix. Route both MCP error paths through askConfirm, whose .confirm-back (z-index 90) paints above the overlay (z-index 80). Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>

greptile-apps · 2026-08-27T14:46:53Z

+      if(!name){ await askConfirm('A name is required.',{danger:false,ok:'OK',cancel:'Dismiss'}); return; }
      const r=await (await fetch('http://127.0.0.1:'+PORT+'/mcp',{method:'POST',
        headers:{'content-type':'application/json'},
        body:JSON.stringify({action:'add',name,command:cmd,enabled:false})})).json();
-      if(!r.ok){ alert(r.error); return; }
+      if(!r.ok){ await askConfirm(r.error,{danger:false,ok:'OK',cancel:'Dismiss'}); return; }


Escape closes both overlays

When the user presses Escape to dismiss an MCP error dialog, both document-level Escape handlers run, causing the foreground dialog and the underlying MCP overlay to close and discarding the in-progress add-server form.

praisonai-triage-agent Bot mentioned this pull request Aug 27, 2026

desktop: alert() and prompt() survive at three sites, in a page that documents JS dialogs as non-functional here #4503

Open

greptile-apps Bot reviewed Aug 27, 2026

View reviewed changes

Comment thread src/praisonai-desktop/ui/index.html Outdated

MervinPraison added pipeline/blocked:ci Blocked: CI not green on HEAD pipeline/blocked:no-final Blocked: no FINAL @claude trigger yet pipeline/final-claude-pending Reviews done; waiting for FINAL @claude labels Aug 27, 2026

greptile-apps Bot reviewed Aug 27, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix: replace non-functional alert()/prompt() in desktop UI - #4521

fix: replace non-functional alert()/prompt() in desktop UI#4521
praisonai-triage-agent[bot] wants to merge 2 commits into
mainfrom
claude/issue-4503-20260827-1351

praisonai-triage-agent Bot commented Aug 27, 2026

Uh oh!

praisonai-triage-agent Bot commented Aug 27, 2026

Uh oh!

praisonai-triage-agent Bot commented Aug 27, 2026

Uh oh!

qodo-code-review Bot commented Aug 27, 2026

Uh oh!

greptile-apps Bot commented Aug 27, 2026 •

edited

Loading

Important Files Changed

Uh oh!

Uh oh!

MervinPraison commented Aug 27, 2026

Uh oh!

praisonai-triage-agent Bot commented Aug 27, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

praisonai-triage-agent Bot commented Aug 27, 2026

What

Changes

Test

Uh oh!

praisonai-triage-agent Bot commented Aug 27, 2026

Uh oh!

praisonai-triage-agent Bot commented Aug 27, 2026

Uh oh!

qodo-code-review Bot commented Aug 27, 2026

Qodo reviews are paused for this user.

Uh oh!

greptile-apps Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

Uh oh!

MervinPraison commented Aug 27, 2026

Uh oh!

praisonai-triage-agent Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Final Architecture Review — PR #4521 ✅

Phase 1 — AGENTS.md Review

Phase 2 — Issue Found & Fixed

Phase 3 — Verdict: ✅ Approve

Uh oh!

greptile-apps Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Aug 27, 2026 •

edited

Loading

praisonai-triage-agent Bot commented Aug 27, 2026 •

edited

Loading