Skip to content

fix(tui): restore desktop approval prompts#37856

Closed
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix-37812-desktop-approval-prompts
Closed

fix(tui): restore desktop approval prompts#37856
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix-37812-desktop-approval-prompts

Conversation

@LeonSGP43

Copy link
Copy Markdown
Contributor

Summary

  • bind approval and sudo callbacks inside the actual TUI prompt worker thread so Desktop and dashboard turns can surface manual confirmations again
  • pass allow_permanent through the approval event path so the UI only offers always allow when the backend actually permits it
  • add regression coverage for thread-local callback registration and reduced approval option sets

Testing

  • pytest -o addopts='' tests/test_tui_gateway_server.py -q -k 'prompt_submit_sets_approval_session_key or prompt_submit_registers_thread_local_approval_callbacks'
  • ruff check tui_gateway/server.py tests/test_tui_gateway_server.py
  • git diff --check

Notes

  • ui-tui Vitest coverage for the new prompt-state plumbing was not runnable locally in this environment because ui-tui/node_modules was absent and npm --prefix ui-tui install did not complete.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 3, 2026
@OutThisLife

Copy link
Copy Markdown
Collaborator

Thanks @LeonSGP43 — closing as superseded by #44534, which carries your work forward with credit (Co-authored-by).

Two things shifted since this was opened:

The genuinely valuable, correct part of this PR was the allow_permanent plumbing — which #44534 carries end-to-end (backend approval.request payload → ui-tui ApprovalPrompt → the desktop ApprovalBar), so neither surface offers an "Always allow" the backend would silently downgrade for tirith content-security findings. Appreciate the contribution!

pull Bot pushed a commit to HSKIMRobert/hermes-agent that referenced this pull request Jun 11, 2026
When a tirith content-security warning is present the approval backend
forces allow_permanent=False and silently downgrades an "always" choice to
session scope (the persistence loop in check_all_command_guards only honors
"always" → permanent when no tirith finding exists). But the gateway notify
payload that drives the TUI and the Electron desktop app never carried that
flag, so both surfaces always rendered "Always allow" — offering a permanent
allow the backend would quietly refuse to persist.

Plumb allow_permanent end-to-end:
- tools/approval.py: include `allow_permanent: not has_tirith` in the gateway
  approval_data the notify callback emits as `approval.request`.
- ui-tui: thread `allowPermanent` through the event handler, gateway types,
  and ApprovalReq; ApprovalPrompt drops the "always" option (and renumbers the
  quick-pick keys) when it's false.
- apps/desktop: thread `allow_permanent` through the gateway payload type, the
  per-session approval store, and the inline ApprovalBar, which now hides the
  "Always allow…" dropdown item when permanent allow is disallowed — reusing
  the existing DropdownMenu / confirm-Dialog UI.

The desktop/TUI render path for approvals already landed in NousResearch#38578 (the root
cause of approvals not surfacing in the GUI); this completes the salvage of
NousResearch#37856 by carrying allow_permanent across both surfaces. NousResearch#37856's original
thread-local _block() approach is dropped: desktop/TUI approvals resolve via
approval.respond → resolve_gateway_approval (the per-session queue), not the
_block()/request_id correlation, so a worker-thread callback waiting on _block
would never be released by the real UI.

Tests: gateway notify payload carries allow_permanent (True without tirith,
False with a tirith warning); ui-tui approvalAction reduced option set +
event-handler allowPermanent propagation; desktop store round-trip + the
ApprovalBar showing/hiding "Always allow".

Supersedes NousResearch#37856
Closes NousResearch#37812

Co-authored-by: LeonSGP43 <cine.dreamer.one@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants