Skip to content

Commit f289742

Browse files
abhipatel12BryanBradfo
authored andcommitted
fix(cli): allow sub-agent confirmation requests in UI while preventing background flicker (google-gemini#20722)
1 parent f45b112 commit f289742

File tree

2 files changed

+168
-25
lines changed

2 files changed

+168
-25
lines changed

packages/cli/src/ui/hooks/useToolScheduler.test.ts

Lines changed: 132 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
type AnyToolInvocation,
2121
ROOT_SCHEDULER_ID,
2222
CoreToolCallStatus,
23+
type WaitingToolCall,
2324
} from '@google/gemini-cli-core';
2425
import { createMockMessageBus } from '@google/gemini-cli-core/src/test-utils/mock-message-bus.js';
2526

@@ -32,6 +33,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
3233
Scheduler: vi.fn().mockImplementation(() => ({
3334
schedule: vi.fn().mockResolvedValue([]),
3435
cancelAll: vi.fn(),
36+
dispose: vi.fn(),
3537
})),
3638
};
3739
});
@@ -341,7 +343,9 @@ describe('useToolScheduler', () => {
341343
const callSub = {
342344
...callRoot,
343345
request: { ...callRoot.request, callId: 'call-sub' },
346+
status: CoreToolCallStatus.AwaitingApproval as const, // Must be awaiting approval to be tracked
344347
schedulerId: 'subagent-1',
348+
confirmationDetails: { type: 'info', title: 'Confirm', prompt: 'Yes?' },
345349
};
346350

347351
// 1. Populate state with multiple schedulers
@@ -360,9 +364,13 @@ describe('useToolScheduler', () => {
360364
});
361365

362366
const [toolCalls] = result.current;
363-
expect(toolCalls).toHaveLength(1);
364-
expect(toolCalls[0].request.callId).toBe('call-root');
365-
expect(toolCalls[0].schedulerId).toBe(ROOT_SCHEDULER_ID);
367+
expect(toolCalls).toHaveLength(2);
368+
expect(
369+
toolCalls.find((t) => t.request.callId === 'call-root'),
370+
).toBeDefined();
371+
expect(
372+
toolCalls.find((t) => t.request.callId === 'call-sub'),
373+
).toBeDefined();
366374

367375
// 2. Call setToolCallsForDisplay (e.g., simulate a manual update or clear)
368376
act(() => {
@@ -374,12 +382,11 @@ describe('useToolScheduler', () => {
374382

375383
// 3. Verify that tools are still present and maintain their scheduler IDs
376384
const [toolCalls2] = result.current;
377-
expect(toolCalls2).toHaveLength(1);
378-
expect(toolCalls2[0].responseSubmittedToGemini).toBe(true);
379-
expect(toolCalls2[0].schedulerId).toBe(ROOT_SCHEDULER_ID);
385+
expect(toolCalls2).toHaveLength(2);
386+
expect(toolCalls2.every((t) => t.responseSubmittedToGemini)).toBe(true);
380387
});
381388

382-
it('ignores TOOL_CALLS_UPDATE from non-root schedulers', () => {
389+
it('ignores TOOL_CALLS_UPDATE from non-root schedulers when no tools await approval', () => {
383390
const { result } = renderHook(() =>
384391
useToolScheduler(
385392
vi.fn().mockResolvedValue(undefined),
@@ -410,8 +417,125 @@ describe('useToolScheduler', () => {
410417
} as ToolCallsUpdateMessage);
411418
});
412419

420+
expect(result.current[0]).toHaveLength(0);
421+
});
422+
423+
it('allows TOOL_CALLS_UPDATE from non-root schedulers when tools are awaiting approval', () => {
424+
const { result } = renderHook(() =>
425+
useToolScheduler(
426+
vi.fn().mockResolvedValue(undefined),
427+
mockConfig,
428+
() => undefined,
429+
),
430+
);
431+
432+
const subagentCall = {
433+
status: CoreToolCallStatus.AwaitingApproval as const,
434+
request: {
435+
callId: 'call-sub',
436+
name: 'test',
437+
args: {},
438+
isClientInitiated: false,
439+
prompt_id: 'p1',
440+
},
441+
tool: createMockTool(),
442+
invocation: createMockInvocation(),
443+
schedulerId: 'subagent-1',
444+
confirmationDetails: { type: 'info', title: 'Confirm', prompt: 'Yes?' },
445+
} as WaitingToolCall;
446+
447+
act(() => {
448+
void mockMessageBus.publish({
449+
type: MessageBusType.TOOL_CALLS_UPDATE,
450+
toolCalls: [subagentCall],
451+
schedulerId: 'subagent-1',
452+
} as ToolCallsUpdateMessage);
453+
});
454+
413455
const [toolCalls] = result.current;
414-
expect(toolCalls).toHaveLength(0);
456+
expect(toolCalls).toHaveLength(1);
457+
expect(toolCalls[0].request.callId).toBe('call-sub');
458+
expect(toolCalls[0].status).toBe(CoreToolCallStatus.AwaitingApproval);
459+
});
460+
461+
it('preserves subagent tools in the UI after they have been approved', () => {
462+
const { result } = renderHook(() =>
463+
useToolScheduler(
464+
vi.fn().mockResolvedValue(undefined),
465+
mockConfig,
466+
() => undefined,
467+
),
468+
);
469+
470+
const subagentCall = {
471+
status: CoreToolCallStatus.AwaitingApproval as const,
472+
request: {
473+
callId: 'call-sub',
474+
name: 'test',
475+
args: {},
476+
isClientInitiated: false,
477+
prompt_id: 'p1',
478+
},
479+
tool: createMockTool(),
480+
invocation: createMockInvocation(),
481+
schedulerId: 'subagent-1',
482+
confirmationDetails: { type: 'info', title: 'Confirm', prompt: 'Yes?' },
483+
} as WaitingToolCall;
484+
485+
// 1. Initial approval request
486+
act(() => {
487+
void mockMessageBus.publish({
488+
type: MessageBusType.TOOL_CALLS_UPDATE,
489+
toolCalls: [subagentCall],
490+
schedulerId: 'subagent-1',
491+
} as ToolCallsUpdateMessage);
492+
});
493+
494+
expect(result.current[0]).toHaveLength(1);
495+
496+
// 2. Approved and executing
497+
const approvedCall = {
498+
...subagentCall,
499+
status: CoreToolCallStatus.Executing as const,
500+
} as unknown as ExecutingToolCall;
501+
502+
act(() => {
503+
void mockMessageBus.publish({
504+
type: MessageBusType.TOOL_CALLS_UPDATE,
505+
toolCalls: [approvedCall],
506+
schedulerId: 'subagent-1',
507+
} as ToolCallsUpdateMessage);
508+
});
509+
510+
expect(result.current[0]).toHaveLength(1);
511+
expect(result.current[0][0].status).toBe(CoreToolCallStatus.Executing);
512+
513+
// 3. New turn with a background tool (should NOT be shown)
514+
const backgroundTool = {
515+
status: CoreToolCallStatus.Executing as const,
516+
request: {
517+
callId: 'call-background',
518+
name: 'read_file',
519+
args: {},
520+
isClientInitiated: false,
521+
prompt_id: 'p1',
522+
},
523+
tool: createMockTool(),
524+
invocation: createMockInvocation(),
525+
schedulerId: 'subagent-1',
526+
} as ExecutingToolCall;
527+
528+
act(() => {
529+
void mockMessageBus.publish({
530+
type: MessageBusType.TOOL_CALLS_UPDATE,
531+
toolCalls: [backgroundTool],
532+
schedulerId: 'subagent-1',
533+
} as ToolCallsUpdateMessage);
534+
});
535+
536+
// The subagent list should now be empty because the previously approved tool
537+
// is gone from the current list, and the new tool doesn't need approval.
538+
expect(result.current[0]).toHaveLength(0);
415539
});
416540

417541
it('adapts success/error status to executing when a tail call is present', () => {

packages/cli/src/ui/hooks/useToolScheduler.ts

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,42 @@ export function useToolScheduler(
115115

116116
useEffect(() => {
117117
const handler = (event: ToolCallsUpdateMessage) => {
118-
// Only process updates for the root scheduler.
119-
// Subagent internal tools should not be displayed in the main tool list.
120-
if (event.schedulerId !== ROOT_SCHEDULER_ID) {
121-
return;
122-
}
118+
const isRoot = event.schedulerId === ROOT_SCHEDULER_ID;
119+
120+
setToolCallsMap((prev) => {
121+
const prevCalls = prev[event.schedulerId] ?? [];
122+
const prevCallIds = new Set(prevCalls.map((tc) => tc.request.callId));
123+
124+
// For non-root schedulers, we only show tool calls that:
125+
// 1. Are currently awaiting approval.
126+
// 2. Were previously shown (e.g., they are now executing or completed).
127+
// This prevents "thinking" tools (reads/searches) from flickering in the UI
128+
// unless they specifically required user interaction.
129+
const filteredToolCalls = isRoot
130+
? event.toolCalls
131+
: event.toolCalls.filter(
132+
(tc) =>
133+
tc.status === CoreToolCallStatus.AwaitingApproval ||
134+
prevCallIds.has(tc.request.callId),
135+
);
136+
137+
// If this is a subagent and we have no tools to show and weren't showing any,
138+
// we can skip the update entirely to avoid unnecessary re-renders.
139+
if (
140+
!isRoot &&
141+
filteredToolCalls.length === 0 &&
142+
prevCalls.length === 0
143+
) {
144+
return prev;
145+
}
146+
147+
const adapted = internalAdaptToolCalls(filteredToolCalls, prevCalls);
148+
149+
return {
150+
...prev,
151+
[event.schedulerId]: adapted,
152+
};
153+
});
123154

124155
// Update output timer for UI spinners (Side Effect)
125156
const hasExecuting = event.toolCalls.some(
@@ -134,18 +165,6 @@ export function useToolScheduler(
134165
if (hasExecuting) {
135166
setLastToolOutputTime(Date.now());
136167
}
137-
138-
setToolCallsMap((prev) => {
139-
const adapted = internalAdaptToolCalls(
140-
event.toolCalls,
141-
prev[event.schedulerId] ?? [],
142-
);
143-
144-
return {
145-
...prev,
146-
[event.schedulerId]: adapted,
147-
};
148-
});
149168
};
150169

151170
messageBus.subscribe(MessageBusType.TOOL_CALLS_UPDATE, handler);

0 commit comments

Comments
 (0)