fix(desktop): surface command approval even when its tool is in a collapsed group (#38829)

The desktop command-approval ApprovalBar renders inline inside ToolEntry,
which lives inside ToolGroupSlot. When 2+ tools group, the group body is
hidden until expanded, so an approval raised by a pending terminal/
execute_code call was buried behind "Tool actions · N steps" and required
manual expansion to act on (sudo/secret were unaffected — they use modal
overlays).

ToolGroupSlot now subscribes to $approvalRequest and force-opens its body
while an approval targeting one of its pending approval-eligible tools is in
flight, so the inline controls surface with nothing expanded. The group
reverts to the user's stored collapse state once the approval resolves.
This commit is contained in:
Teknium
2026-06-04 02:29:46 -07:00
committed by GitHub
parent 0175be3aa7
commit b1b0f4b668
3 changed files with 182 additions and 3 deletions

View File

@ -0,0 +1,161 @@
import { AssistantRuntimeProvider, type ThreadMessage, useExternalStoreRuntime } from '@assistant-ui/react'
import { cleanup, render, screen, waitFor } from '@testing-library/react'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { $approvalRequest } from '@/store/prompts'
import { $toolDisclosureStates } from '@/store/tool-view'
import { Thread } from './thread'
// Regression coverage for the "approval buried behind a collapsed tool group"
// bug. When 2+ tools group into a collapsed "Tool actions · N steps" row, the
// pending tool's inline ApprovalBar lives inside the group body — which is
// `hidden` until expanded. A live approval must surface WITHOUT the user
// expanding anything, so ToolGroupSlot force-opens its body while an approval
// targeting one of its pending tools is in flight.
const createdAt = new Date('2026-06-03T00:00:00.000Z')
const resizeObservers = new Set<TestResizeObserver>()
class TestResizeObserver {
private target: Element | null = null
constructor(private readonly callback: ResizeObserverCallback) {
resizeObservers.add(this)
}
observe(target: Element) {
this.target = target
}
unobserve() {}
disconnect() {
resizeObservers.delete(this)
}
}
vi.stubGlobal('ResizeObserver', TestResizeObserver)
vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) =>
window.setTimeout(() => callback(performance.now()), 0)
)
vi.stubGlobal('cancelAnimationFrame', (id: number) => window.clearTimeout(id))
Element.prototype.scrollTo = function scrollTo() {}
Element.prototype.animate = function animate() {
return {
cancel: () => {},
finished: Promise.resolve()
} as unknown as Animation
}
function stubOffsetDimension(
prop: 'offsetHeight' | 'offsetWidth',
clientProp: 'clientHeight' | 'clientWidth',
fallback: number
) {
const previous = Object.getOwnPropertyDescriptor(HTMLElement.prototype, prop)
Object.defineProperty(HTMLElement.prototype, prop, {
configurable: true,
get() {
return previous?.get?.call(this) || (this as HTMLElement)[clientProp] || fallback
}
})
}
stubOffsetDimension('offsetWidth', 'clientWidth', 800)
stubOffsetDimension('offsetHeight', 'clientHeight', 600)
// A running assistant message with two tools: a completed read_file plus a
// pending terminal (no result). Two visible tools → ToolGroupSlot groups them
// behind a collapsed "Tool actions · 2 steps" header.
function groupedPendingMessage(): ThreadMessage {
return {
id: 'assistant-group-1',
role: 'assistant',
content: [
{
type: 'tool-call',
toolCallId: 'read-1',
toolName: 'read_file',
args: { path: '/etc/hosts' },
argsText: JSON.stringify({ path: '/etc/hosts' }),
result: { content: '127.0.0.1 localhost' }
},
{
type: 'tool-call',
toolCallId: 'term-1',
toolName: 'terminal',
args: { command: 'rm -rf /tmp/x' },
argsText: JSON.stringify({ command: 'rm -rf /tmp/x' })
}
],
status: { type: 'running' },
createdAt,
metadata: {
unstable_state: null,
unstable_annotations: [],
unstable_data: [],
steps: [],
custom: {}
}
} as ThreadMessage
}
function GroupHarness({ message }: { message: ThreadMessage }) {
const runtime = useExternalStoreRuntime<ThreadMessage>({
messages: [message],
isRunning: message.status?.type === 'running',
onNew: async () => {}
})
return (
<AssistantRuntimeProvider runtime={runtime}>
<Thread />
</AssistantRuntimeProvider>
)
}
beforeEach(() => {
$approvalRequest.set(null)
$toolDisclosureStates.set({})
})
afterEach(() => {
cleanup()
$approvalRequest.set(null)
})
describe('ToolGroupSlot approval surfacing', () => {
it('hides the grouped pending tool body when there is no approval', async () => {
const { container } = render(<GroupHarness message={groupedPendingMessage()} />)
// Group header renders collapsed; the inline approval strip lives in the
// hidden body, so with no live approval it must not render at all (the
// ApprovalBar returns null when $approvalRequest is empty).
await waitFor(() => {
expect(screen.getByText(/Tool actions/)).toBeTruthy()
})
expect(container.querySelector('[data-slot="tool-approval-inline"]')).toBeNull()
})
it('force-opens the group body so the approval surfaces without expanding', async () => {
$approvalRequest.set({ command: 'rm -rf /tmp/x', description: 'dangerous command', sessionId: 'sess-1' })
const { container } = render(<GroupHarness message={groupedPendingMessage()} />)
// Even though the group defaults collapsed, the live approval forces the
// body open so the inline controls are visible (and reachable, not in a
// hidden subtree) immediately.
await waitFor(() => {
const bar = container.querySelector('[data-slot="tool-approval-inline"]')
expect(bar).not.toBeNull()
// The forced-open group body must not be hidden — assert no ancestor
// carries the `hidden` attribute that would keep the bar off-screen.
expect(bar?.closest('[hidden]')).toBeNull()
})
})
})

View File

@ -39,7 +39,7 @@ import type { ToolPart } from './tool-fallback-model'
// approval at a time, so the single pending row of those tools IS the row that
// raised it. The command/description text comes from `$approvalRequest` (the
// event payload), which is the only place that data reliably exists.
const APPROVAL_TOOLS = new Set(['terminal', 'execute_code'])
export const APPROVAL_TOOLS = new Set(['terminal', 'execute_code'])
// Canonical gateway choices (ui-tui/src/components/prompts.tsx).
type ApprovalChoice = 'once' | 'session' | 'always' | 'deny'

View File

@ -21,10 +21,11 @@ import { PrettyLink, LinkifiedText as SharedLinkifiedText, urlSlugTitleLabel } f
import { AlertCircle, CheckCircle2 } from '@/lib/icons'
import { useEnterAnimation } from '@/lib/use-enter-animation'
import { cn } from '@/lib/utils'
import { $approvalRequest } from '@/store/prompts'
import { $toolInlineDiffs } from '@/store/tool-diffs'
import { $toolDisclosureOpen, $toolViewMode, setToolDisclosureOpen } from '@/store/tool-view'
import { PendingToolApproval } from './tool-approval'
import { APPROVAL_TOOLS, PendingToolApproval } from './tool-approval'
import {
groupCopyText as buildGroupCopyText,
buildToolView,
@ -458,7 +459,24 @@ export const ToolGroupSlot: FC<PropsWithChildren<{ endIndex: number; startIndex:
// tools append to the end), so user-driven open/close persists across
// streaming.
const disclosureId = `tool-group:${messageId}:${startIndex}`
const open = useDisclosureOpen(disclosureId)
const userOpen = useDisclosureOpen(disclosureId)
// A live approval request must NEVER be buried inside a collapsed group —
// the user has to be able to act on it without first expanding "Tool
// actions · N steps". When an approval is in flight and this group hosts
// the pending approval-eligible tool that raised it (terminal /
// execute_code with no result yet — see tool-approval.tsx for why the
// single pending row IS the one that raised it), force the body open so
// the inline ApprovalBar surfaces. The user can still collapse the group
// again once the approval resolves.
const approvalRequest = useStore($approvalRequest)
const hostsLiveApproval =
approvalRequest !== null &&
messageRunning &&
visibleParts.some(p => p.result === undefined && APPROVAL_TOOLS.has(p.toolName))
const open = userOpen || hostsLiveApproval
const enterRef = useEnterAnimation(messageRunning, disclosureId)
const status = groupStatus(visibleParts)