fix(desktop): surface approval fallback prompt - #45828
Closed
helix4u wants to merge 1 commit into
Closed
Conversation
helix4u
marked this pull request as ready for review
June 13, 2026 21:27
3 tasks
Collaborator
|
Closing in favor of #45853, which landed on an even simpler shape than my earlier comment described: rather than a separate pill + Genuinely appreciate the report and repro @helix4u — the symptom you flagged (a blocked approval scrolled off-screen looking like a dead session) is exactly what #45853 fixes. Thank you! |
OutThisLife
added a commit
that referenced
this pull request
Jun 13, 2026
…trol (#45853) * fix(desktop): jump-to-approval pill for off-screen approvals A blocked approval's only response surface is the inline Run/Reject bar on the pending tool row. When that row is scrolled out of view the session looks stalled with no visible action. Surface a composer-anchored "Approval needed" pill only when an approval is pending AND its inline bar is scrolled away; clicking scrolls the bar back into view. Preserves the deliberate inline (not modal) approval design — the pill never duplicates the approve/reject controls. The inline bar mirrors its own viewport visibility via IntersectionObserver (tracks scroll/resize/layout) and registers a scroll-into-view handler the pill fires, mirroring the existing thread-scroll jump-button bridge. Supersedes #45828. * fix(desktop): morph jump-to-bottom into approval prompt; drop scroll bridge Collapse the separate "jump to approval" pill into the existing scroll-to-bottom control: when scrolled away from the bottom while an approval is pending, it relabels to "Approval needed". A parked approval's inline Run/Reject bar is always the bottom-most content, so the existing scroll-to-bottom action lands the user right on it — one control, no collision. This also fixes the layout corruption from the first cut: the pill called native el.scrollIntoView(), which scrolls every scrollable ancestor including the overflow:hidden chat shell containers. Those have no scrollbar to scroll back and don't remount on session switch, so the composer stayed shoved and the breakage persisted across sessions. Reusing requestScrollToBottom() (the use-stick-to-bottom path) only touches the one designated scroll container. Removes the now-unused approval-scroll store + IntersectionObserver wiring.
AIalliAI
pushed a commit
to AIalliAI/Hermes
that referenced
this pull request
Jun 14, 2026
…trol (NousResearch#45853) * fix(desktop): jump-to-approval pill for off-screen approvals A blocked approval's only response surface is the inline Run/Reject bar on the pending tool row. When that row is scrolled out of view the session looks stalled with no visible action. Surface a composer-anchored "Approval needed" pill only when an approval is pending AND its inline bar is scrolled away; clicking scrolls the bar back into view. Preserves the deliberate inline (not modal) approval design — the pill never duplicates the approve/reject controls. The inline bar mirrors its own viewport visibility via IntersectionObserver (tracks scroll/resize/layout) and registers a scroll-into-view handler the pill fires, mirroring the existing thread-scroll jump-button bridge. Supersedes NousResearch#45828. * fix(desktop): morph jump-to-bottom into approval prompt; drop scroll bridge Collapse the separate "jump to approval" pill into the existing scroll-to-bottom control: when scrolled away from the bottom while an approval is pending, it relabels to "Approval needed". A parked approval's inline Run/Reject bar is always the bottom-most content, so the existing scroll-to-bottom action lands the user right on it — one control, no collision. This also fixes the layout corruption from the first cut: the pill called native el.scrollIntoView(), which scrolls every scrollable ancestor including the overflow:hidden chat shell containers. Those have no scrollbar to scroll back and don't remount on session switch, so the composer stayed shoved and the breakage persisted across sessions. Reusing requestScrollToBottom() (the use-stick-to-bottom path) only touches the one designated scroll container. Removes the now-unused approval-scroll store + IntersectionObserver wiring.
T02200059
pushed a commit
to T02200059/hermes-agent
that referenced
this pull request
Jun 18, 2026
…trol (NousResearch#45853) * fix(desktop): jump-to-approval pill for off-screen approvals A blocked approval's only response surface is the inline Run/Reject bar on the pending tool row. When that row is scrolled out of view the session looks stalled with no visible action. Surface a composer-anchored "Approval needed" pill only when an approval is pending AND its inline bar is scrolled away; clicking scrolls the bar back into view. Preserves the deliberate inline (not modal) approval design — the pill never duplicates the approve/reject controls. The inline bar mirrors its own viewport visibility via IntersectionObserver (tracks scroll/resize/layout) and registers a scroll-into-view handler the pill fires, mirroring the existing thread-scroll jump-button bridge. Supersedes NousResearch#45828. * fix(desktop): morph jump-to-bottom into approval prompt; drop scroll bridge Collapse the separate "jump to approval" pill into the existing scroll-to-bottom control: when scrolled away from the bottom while an approval is pending, it relabels to "Approval needed". A parked approval's inline Run/Reject bar is always the bottom-most content, so the existing scroll-to-bottom action lands the user right on it — one control, no collision. This also fixes the layout corruption from the first cut: the pill called native el.scrollIntoView(), which scrolls every scrollable ancestor including the overflow:hidden chat shell containers. Those have no scrollbar to scroll back and don't remount on session switch, so the composer stayed shoved and the breakage persisted across sessions. Reusing requestScrollToBottom() (the use-stick-to-bottom path) only touches the one designated scroll container. Removes the now-unused approval-scroll store + IntersectionObserver wiring.
waefrebeorn
pushed a commit
to waefrebeorn/slermes
that referenced
this pull request
Jul 2, 2026
…trol (NousResearch#45853) * fix(desktop): jump-to-approval pill for off-screen approvals A blocked approval's only response surface is the inline Run/Reject bar on the pending tool row. When that row is scrolled out of view the session looks stalled with no visible action. Surface a composer-anchored "Approval needed" pill only when an approval is pending AND its inline bar is scrolled away; clicking scrolls the bar back into view. Preserves the deliberate inline (not modal) approval design — the pill never duplicates the approve/reject controls. The inline bar mirrors its own viewport visibility via IntersectionObserver (tracks scroll/resize/layout) and registers a scroll-into-view handler the pill fires, mirroring the existing thread-scroll jump-button bridge. Supersedes NousResearch#45828. * fix(desktop): morph jump-to-bottom into approval prompt; drop scroll bridge Collapse the separate "jump to approval" pill into the existing scroll-to-bottom control: when scrolled away from the bottom while an approval is pending, it relabels to "Approval needed". A parked approval's inline Run/Reject bar is always the bottom-most content, so the existing scroll-to-bottom action lands the user right on it — one control, no collision. This also fixes the layout corruption from the first cut: the pill called native el.scrollIntoView(), which scrolls every scrollable ancestor including the overflow:hidden chat shell containers. Those have no scrollbar to scroll back and don't remount on session switch, so the composer stayed shoved and the breakage persisted across sessions. Reusing requestScrollToBottom() (the use-stick-to-bottom path) only touches the one designated scroll container. Removes the now-unused approval-scroll store + IntersectionObserver wiring.
Methodician
added a commit
to Methodician/hermes-agent
that referenced
this pull request
Jul 4, 2026
…trol (NousResearch#45853) * fix(desktop): jump-to-approval pill for off-screen approvals A blocked approval's only response surface is the inline Run/Reject bar on the pending tool row. When that row is scrolled out of view the session looks stalled with no visible action. Surface a composer-anchored "Approval needed" pill only when an approval is pending AND its inline bar is scrolled away; clicking scrolls the bar back into view. Preserves the deliberate inline (not modal) approval design — the pill never duplicates the approve/reject controls. The inline bar mirrors its own viewport visibility via IntersectionObserver (tracks scroll/resize/layout) and registers a scroll-into-view handler the pill fires, mirroring the existing thread-scroll jump-button bridge. Supersedes NousResearch#45828. * fix(desktop): morph jump-to-bottom into approval prompt; drop scroll bridge Collapse the separate "jump to approval" pill into the existing scroll-to-bottom control: when scrolled away from the bottom while an approval is pending, it relabels to "Approval needed". A parked approval's inline Run/Reject bar is always the bottom-most content, so the existing scroll-to-bottom action lands the user right on it — one control, no collision. This also fixes the layout corruption from the first cut: the pill called native el.scrollIntoView(), which scrolls every scrollable ancestor including the overflow:hidden chat shell containers. Those have no scrollbar to scroll back and don't remount on session switch, so the composer stayed shoved and the breakage persisted across sessions. Reusing requestScrollToBottom() (the use-stick-to-bottom path) only touches the one designated scroll container. Removes the now-unused approval-scroll store + IntersectionObserver wiring.
habarmc1223-sudo
pushed a commit
to habarmc1223-sudo/hermes-agent-fluxmem
that referenced
this pull request
Jul 8, 2026
…trol (NousResearch#45853) * fix(desktop): jump-to-approval pill for off-screen approvals A blocked approval's only response surface is the inline Run/Reject bar on the pending tool row. When that row is scrolled out of view the session looks stalled with no visible action. Surface a composer-anchored "Approval needed" pill only when an approval is pending AND its inline bar is scrolled away; clicking scrolls the bar back into view. Preserves the deliberate inline (not modal) approval design — the pill never duplicates the approve/reject controls. The inline bar mirrors its own viewport visibility via IntersectionObserver (tracks scroll/resize/layout) and registers a scroll-into-view handler the pill fires, mirroring the existing thread-scroll jump-button bridge. Supersedes NousResearch#45828. * fix(desktop): morph jump-to-bottom into approval prompt; drop scroll bridge Collapse the separate "jump to approval" pill into the existing scroll-to-bottom control: when scrolled away from the bottom while an approval is pending, it relabels to "Approval needed". A parked approval's inline Run/Reject bar is always the bottom-most content, so the existing scroll-to-bottom action lands the user right on it — one control, no collision. This also fixes the layout corruption from the first cut: the pill called native el.scrollIntoView(), which scrolls every scrollable ancestor including the overflow:hidden chat shell containers. Those have no scrollbar to scroll back and don't remount on session switch, so the composer stayed shoved and the breakage persisted across sessions. Reusing requestScrollToBottom() (the use-stick-to-bottom path) only touches the one designated scroll container. Removes the now-unused approval-scroll store + IntersectionObserver wiring.
4 tasks
santhreal
pushed a commit
to santhreal/hermes-agent
that referenced
this pull request
Jul 13, 2026
…trol (NousResearch#45853) * fix(desktop): jump-to-approval pill for off-screen approvals A blocked approval's only response surface is the inline Run/Reject bar on the pending tool row. When that row is scrolled out of view the session looks stalled with no visible action. Surface a composer-anchored "Approval needed" pill only when an approval is pending AND its inline bar is scrolled away; clicking scrolls the bar back into view. Preserves the deliberate inline (not modal) approval design — the pill never duplicates the approve/reject controls. The inline bar mirrors its own viewport visibility via IntersectionObserver (tracks scroll/resize/layout) and registers a scroll-into-view handler the pill fires, mirroring the existing thread-scroll jump-button bridge. Supersedes NousResearch#45828. * fix(desktop): morph jump-to-bottom into approval prompt; drop scroll bridge Collapse the separate "jump to approval" pill into the existing scroll-to-bottom control: when scrolled away from the bottom while an approval is pending, it relabels to "Approval needed". A parked approval's inline Run/Reject bar is always the bottom-most content, so the existing scroll-to-bottom action lands the user right on it — one control, no collision. This also fixes the layout corruption from the first cut: the pill called native el.scrollIntoView(), which scrolls every scrollable ancestor including the overflow:hidden chat shell containers. Those have no scrollbar to scroll back and don't remount on session switch, so the composer stayed shoved and the breakage persisted across sessions. Reusing requestScrollToBottom() (the use-stick-to-bottom path) only touches the one designated scroll container. Removes the now-unused approval-scroll store + IntersectionObserver wiring.
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
…trol (NousResearch#45853) * fix(desktop): jump-to-approval pill for off-screen approvals A blocked approval's only response surface is the inline Run/Reject bar on the pending tool row. When that row is scrolled out of view the session looks stalled with no visible action. Surface a composer-anchored "Approval needed" pill only when an approval is pending AND its inline bar is scrolled away; clicking scrolls the bar back into view. Preserves the deliberate inline (not modal) approval design — the pill never duplicates the approve/reject controls. The inline bar mirrors its own viewport visibility via IntersectionObserver (tracks scroll/resize/layout) and registers a scroll-into-view handler the pill fires, mirroring the existing thread-scroll jump-button bridge. Supersedes NousResearch#45828. * fix(desktop): morph jump-to-bottom into approval prompt; drop scroll bridge Collapse the separate "jump to approval" pill into the existing scroll-to-bottom control: when scrolled away from the bottom while an approval is pending, it relabels to "Approval needed". A parked approval's inline Run/Reject bar is always the bottom-most content, so the existing scroll-to-bottom action lands the user right on it — one control, no collision. This also fixes the layout corruption from the first cut: the pill called native el.scrollIntoView(), which scrolls every scrollable ancestor including the overflow:hidden chat shell containers. Those have no scrollbar to scroll back and don't remount on session switch, so the composer stayed shoved and the breakage persisted across sessions. Reusing requestScrollToBottom() (the use-stick-to-bottom path) only touches the one designated scroll container. Removes the now-unused approval-scroll store + IntersectionObserver wiring.
leewenjie
pushed a commit
to leewenjie/hermes-agent
that referenced
this pull request
Aug 7, 2026
…trol (NousResearch#45853) * fix(desktop): jump-to-approval pill for off-screen approvals A blocked approval's only response surface is the inline Run/Reject bar on the pending tool row. When that row is scrolled out of view the session looks stalled with no visible action. Surface a composer-anchored "Approval needed" pill only when an approval is pending AND its inline bar is scrolled away; clicking scrolls the bar back into view. Preserves the deliberate inline (not modal) approval design — the pill never duplicates the approve/reject controls. The inline bar mirrors its own viewport visibility via IntersectionObserver (tracks scroll/resize/layout) and registers a scroll-into-view handler the pill fires, mirroring the existing thread-scroll jump-button bridge. Supersedes NousResearch#45828. * fix(desktop): morph jump-to-bottom into approval prompt; drop scroll bridge Collapse the separate "jump to approval" pill into the existing scroll-to-bottom control: when scrolled away from the bottom while an approval is pending, it relabels to "Approval needed". A parked approval's inline Run/Reject bar is always the bottom-most content, so the existing scroll-to-bottom action lands the user right on it — one control, no collision. This also fixes the layout corruption from the first cut: the pill called native el.scrollIntoView(), which scrolls every scrollable ancestor including the overflow:hidden chat shell containers. Those have no scrollbar to scroll back and don't remount on session switch, so the composer stayed shoved and the breakage persisted across sessions. Reusing requestScrollToBottom() (the use-stick-to-bottom path) only touches the one designated scroll container. Removes the now-unused approval-scroll store + IntersectionObserver wiring.
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.
What does this PR do?
Desktop already parks
approval.requestevents per session and renders approval controls inline under pendingterminal/execute_codetool rows. That leaves a failure mode where the backend is waiting forapproval.respond, but the active Desktop view has no actionable approval UI because the inline row is missing, hidden, or outside the visible viewport.This PR adds a Desktop fallback approval dialog mounted with the existing prompt overlays. It reuses the same approval action component as the inline row, and it only appears when a live approval request exists and the inline approval bar is not visibly available.
Root cause: approval state was durable, but the only renderer was tied to one transcript row. If that row was not mounted or visible, the user could only see the session needing input while the agent kept waiting.
Related Issue
Related support thread: https://discord.com/channels/1053877538025386074/1515049553194389564
Type of Change
Changes Made
ApprovalPromptFallbackinapps/desktop/src/components/assistant-ui/tool-approval.tsx.apps/desktop/src/components/prompt-overlays.tsxnext to sudo/secret prompts.ApprovalBarresponse path so fallback approvals still callapproval.respondwith the same choices.How to Test
approval.requestwhile no visible inline approval bar is mounted.approval.respondand clears the request.Focused test run:
npm run test:ui -- src/components/assistant-ui/tool-approval.test.tsxResult: 1 file passed, 11 tests passed.
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
N/A
Screenshots / Logs
Focused test output:
Full Python test suite was not run; this is a Desktop UI-only change and the focused Desktop UI test above covers the changed approval surface.