fix(tui): restore desktop approval prompts#37856
Closed
LeonSGP43 wants to merge 1 commit into
Closed
Conversation
3 tasks
Collaborator
|
Thanks @LeonSGP43 — closing as superseded by #44534, which carries your work forward with credit ( Two things shifted since this was opened:
The genuinely valuable, correct part of this PR was the |
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
allow_permanentthrough the approval event path so the UI only offersalways allowwhen the backend actually permits itTesting
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.pygit diff --checkNotes
ui-tuiVitest coverage for the new prompt-state plumbing was not runnable locally in this environment becauseui-tui/node_moduleswas absent andnpm --prefix ui-tui installdid not complete.