Skip to content

Commit 10ab958

Browse files
authored
refactor(core): extract ExecutionLifecycleService for tool backgrounding (#21717)
1 parent 949e85c commit 10ab958

13 files changed

Lines changed: 1545 additions & 767 deletions

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export const useShellCommandProcessor = (
8080
setShellInputFocused: (value: boolean) => void,
8181
terminalWidth?: number,
8282
terminalHeight?: number,
83-
activeToolPtyId?: number,
83+
activeBackgroundExecutionId?: number,
8484
isWaitingForConfirmation?: boolean,
8585
) => {
8686
const [state, dispatch] = useReducer(shellReducer, initialState);
@@ -103,7 +103,8 @@ export const useShellCommandProcessor = (
103103
}
104104
const m = manager.current;
105105

106-
const activePtyId = state.activeShellPtyId || activeToolPtyId;
106+
const activePtyId =
107+
state.activeShellPtyId ?? activeBackgroundExecutionId ?? undefined;
107108

108109
useEffect(() => {
109110
const isForegroundActive = !!activePtyId || !!isWaitingForConfirmation;
@@ -191,7 +192,8 @@ export const useShellCommandProcessor = (
191192
]);
192193

193194
const backgroundCurrentShell = useCallback(() => {
194-
const pidToBackground = state.activeShellPtyId || activeToolPtyId;
195+
const pidToBackground =
196+
state.activeShellPtyId ?? activeBackgroundExecutionId;
195197
if (pidToBackground) {
196198
ShellExecutionService.background(pidToBackground);
197199
m.backgroundedPids.add(pidToBackground);
@@ -202,7 +204,7 @@ export const useShellCommandProcessor = (
202204
m.restoreTimeout = null;
203205
}
204206
}
205-
}, [state.activeShellPtyId, activeToolPtyId, m]);
207+
}, [state.activeShellPtyId, activeBackgroundExecutionId, m]);
206208

207209
const dismissBackgroundShell = useCallback(
208210
async (pid: number) => {

packages/cli/src/ui/hooks/useGeminiStream.test.tsx

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,25 @@ const MockedUserPromptEvent = vi.hoisted(() =>
103103
vi.fn().mockImplementation(() => {}),
104104
);
105105
const mockParseAndFormatApiError = vi.hoisted(() => vi.fn());
106+
const mockIsBackgroundExecutionData = vi.hoisted(
107+
() =>
108+
(data: unknown): data is { pid?: number } => {
109+
if (typeof data !== 'object' || data === null) {
110+
return false;
111+
}
112+
const value = data as {
113+
pid?: unknown;
114+
command?: unknown;
115+
initialOutput?: unknown;
116+
};
117+
return (
118+
(value.pid === undefined || typeof value.pid === 'number') &&
119+
(value.command === undefined || typeof value.command === 'string') &&
120+
(value.initialOutput === undefined ||
121+
typeof value.initialOutput === 'string')
122+
);
123+
},
124+
);
106125

107126
const MockValidationRequiredError = vi.hoisted(
108127
() =>
@@ -128,6 +147,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
128147
const actualCoreModule = (await importOriginal()) as any;
129148
return {
130149
...actualCoreModule,
150+
isBackgroundExecutionData: mockIsBackgroundExecutionData,
131151
GitService: vi.fn(),
132152
GeminiClient: MockedGeminiClientClass,
133153
UserPromptEvent: MockedUserPromptEvent,
@@ -606,6 +626,35 @@ describe('useGeminiStream', () => {
606626
expect(mockSendMessageStream).not.toHaveBeenCalled(); // submitQuery uses this
607627
});
608628

629+
it('should expose activePtyId for non-shell executing tools that report an execution ID', () => {
630+
const remoteExecutingTool: TrackedExecutingToolCall = {
631+
request: {
632+
callId: 'remote-call-1',
633+
name: 'remote_agent_call',
634+
args: {},
635+
isClientInitiated: false,
636+
prompt_id: 'prompt-id-remote',
637+
},
638+
status: CoreToolCallStatus.Executing,
639+
responseSubmittedToGemini: false,
640+
tool: {
641+
name: 'remote_agent_call',
642+
displayName: 'Remote Agent',
643+
description: 'Remote agent execution',
644+
build: vi.fn(),
645+
} as any,
646+
invocation: {
647+
getDescription: () => 'Calling remote agent',
648+
} as unknown as AnyToolInvocation,
649+
startTime: Date.now(),
650+
liveOutput: 'working...',
651+
pid: 4242,
652+
};
653+
654+
const { result } = renderTestHook([remoteExecutingTool]);
655+
expect(result.current.activePtyId).toBe(4242);
656+
});
657+
609658
it('should submit tool responses when all tool calls are completed and ready', async () => {
610659
const toolCall1ResponseParts: Part[] = [{ text: 'tool 1 final response' }];
611660
const toolCall2ResponseParts: Part[] = [{ text: 'tool 2 final response' }];

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

Lines changed: 45 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
buildUserSteeringHintPrompt,
3838
GeminiCliOperation,
3939
getPlanModeExitMessage,
40+
isBackgroundExecutionData,
4041
} from '@google/gemini-cli-core';
4142
import type {
4243
Config,
@@ -94,10 +95,10 @@ type ToolResponseWithParts = ToolCallResponseInfo & {
9495
llmContent?: PartListUnion;
9596
};
9697

97-
interface ShellToolData {
98-
pid?: number;
99-
command?: string;
100-
initialOutput?: string;
98+
interface BackgroundedToolInfo {
99+
pid: number;
100+
command: string;
101+
initialOutput: string;
101102
}
102103

103104
enum StreamProcessingStatus {
@@ -111,15 +112,32 @@ const SUPPRESSED_TOOL_ERRORS_NOTE =
111112
const LOW_VERBOSITY_FAILURE_NOTE =
112113
'This request failed. Press F12 for diagnostics, or run /settings and change "Error Verbosity" to full for full details.';
113114

114-
function isShellToolData(data: unknown): data is ShellToolData {
115-
if (typeof data !== 'object' || data === null) {
116-
return false;
115+
function getBackgroundedToolInfo(
116+
toolCall: TrackedCompletedToolCall | TrackedCancelledToolCall,
117+
): BackgroundedToolInfo | undefined {
118+
const response = toolCall.response as ToolResponseWithParts;
119+
const rawData: unknown = response?.data;
120+
if (!isBackgroundExecutionData(rawData)) {
121+
return undefined;
122+
}
123+
124+
if (rawData.pid === undefined) {
125+
return undefined;
117126
}
118-
const d = data as Partial<ShellToolData>;
127+
128+
return {
129+
pid: rawData.pid,
130+
command: rawData.command ?? toolCall.request.name,
131+
initialOutput: rawData.initialOutput ?? '',
132+
};
133+
}
134+
135+
function isBackgroundableExecutingToolCall(
136+
toolCall: TrackedToolCall,
137+
): toolCall is TrackedExecutingToolCall {
119138
return (
120-
(d.pid === undefined || typeof d.pid === 'number') &&
121-
(d.command === undefined || typeof d.command === 'string') &&
122-
(d.initialOutput === undefined || typeof d.initialOutput === 'string')
139+
toolCall.status === CoreToolCallStatus.Executing &&
140+
typeof toolCall.pid === 'number'
123141
);
124142
}
125143

@@ -319,13 +337,11 @@ export const useGeminiStream = (
319337
getPreferredEditor,
320338
);
321339

322-
const activeToolPtyId = useMemo(() => {
323-
const executingShellTool = toolCalls.find(
324-
(tc) =>
325-
tc.status === 'executing' && tc.request.name === 'run_shell_command',
340+
const activeBackgroundExecutionId = useMemo(() => {
341+
const executingBackgroundableTool = toolCalls.find(
342+
isBackgroundableExecutingToolCall,
326343
);
327-
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
328-
return (executingShellTool as TrackedExecutingToolCall | undefined)?.pid;
344+
return executingBackgroundableTool?.pid;
329345
}, [toolCalls]);
330346

331347
const onExec = useCallback(
@@ -358,7 +374,7 @@ export const useGeminiStream = (
358374
setShellInputFocused,
359375
terminalWidth,
360376
terminalHeight,
361-
activeToolPtyId,
377+
activeBackgroundExecutionId,
362378
);
363379

364380
const streamingState = useMemo(
@@ -536,7 +552,8 @@ export const useGeminiStream = (
536552
onComplete: (result: { userSelection: 'disable' | 'keep' }) => void;
537553
} | null>(null);
538554

539-
const activePtyId = activeShellPtyId || activeToolPtyId;
555+
const activePtyId =
556+
activeShellPtyId ?? activeBackgroundExecutionId ?? undefined;
540557

541558
const prevActiveShellPtyIdRef = useRef<number | null>(null);
542559
useEffect(() => {
@@ -1678,26 +1695,16 @@ export const useGeminiStream = (
16781695
!processedMemoryToolsRef.current.has(t.request.callId),
16791696
);
16801697

1681-
// Handle backgrounded shell tools
1682-
completedAndReadyToSubmitTools.forEach((t) => {
1683-
const isShell = t.request.name === 'run_shell_command';
1684-
// Access result from the tracked tool call response
1685-
const response = t.response as ToolResponseWithParts;
1686-
const rawData = response?.data;
1687-
const data = isShellToolData(rawData) ? rawData : undefined;
1688-
1689-
// Use data.pid for shell commands moved to the background.
1690-
const pid = data?.pid;
1691-
1692-
if (isShell && pid) {
1693-
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
1694-
const command = (data?.['command'] as string) ?? 'shell';
1695-
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
1696-
const initialOutput = (data?.['initialOutput'] as string) ?? '';
1697-
1698-
registerBackgroundShell(pid, command, initialOutput);
1698+
for (const toolCall of completedAndReadyToSubmitTools) {
1699+
const backgroundedTool = getBackgroundedToolInfo(toolCall);
1700+
if (backgroundedTool) {
1701+
registerBackgroundShell(
1702+
backgroundedTool.pid,
1703+
backgroundedTool.command,
1704+
backgroundedTool.initialOutput,
1705+
);
16991706
}
1700-
});
1707+
}
17011708

17021709
if (newSuccessfulMemorySaves.length > 0) {
17031710
// Perform the refresh only if there are new ones.

packages/core/src/core/coreToolHookTriggers.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
BaseToolInvocation,
1212
type ToolResult,
1313
type AnyDeclarativeTool,
14+
type ToolLiveOutput,
1415
} from '../tools/tools.js';
1516
import type { MessageBus } from '../confirmation-bus/message-bus.js';
1617
import type { HookSystem } from '../hooks/hookSystem.js';
@@ -37,6 +38,30 @@ class MockInvocation extends BaseToolInvocation<{ key?: string }, ToolResult> {
3738
}
3839
}
3940

41+
class MockBackgroundableInvocation extends BaseToolInvocation<
42+
{ key?: string },
43+
ToolResult
44+
> {
45+
constructor(params: { key?: string }, messageBus: MessageBus) {
46+
super(params, messageBus);
47+
}
48+
getDescription() {
49+
return 'mock-pid';
50+
}
51+
async execute(
52+
_signal: AbortSignal,
53+
_updateOutput?: (output: ToolLiveOutput) => void,
54+
_shellExecutionConfig?: unknown,
55+
setExecutionIdCallback?: (executionId: number) => void,
56+
) {
57+
setExecutionIdCallback?.(4242);
58+
return {
59+
llmContent: 'pid',
60+
returnDisplay: 'pid',
61+
};
62+
}
63+
}
64+
4065
describe('executeToolWithHooks', () => {
4166
let messageBus: MessageBus;
4267
let mockTool: AnyDeclarativeTool;
@@ -258,4 +283,26 @@ describe('executeToolWithHooks', () => {
258283
expect(invocation.params.key).toBe('original');
259284
expect(mockTool.build).not.toHaveBeenCalled();
260285
});
286+
287+
it('should pass execution ID callback through for non-shell invocations', async () => {
288+
const invocation = new MockBackgroundableInvocation({}, messageBus);
289+
const abortSignal = new AbortController().signal;
290+
const setExecutionIdCallback = vi.fn();
291+
292+
vi.mocked(mockHookSystem.fireBeforeToolEvent).mockResolvedValue(undefined);
293+
vi.mocked(mockHookSystem.fireAfterToolEvent).mockResolvedValue(undefined);
294+
295+
await executeToolWithHooks(
296+
invocation,
297+
'test_tool',
298+
abortSignal,
299+
mockTool,
300+
undefined,
301+
undefined,
302+
setExecutionIdCallback,
303+
mockConfig,
304+
);
305+
306+
expect(setExecutionIdCallback).toHaveBeenCalledWith(4242);
307+
});
261308
});

packages/core/src/core/coreToolHookTriggers.ts

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import type {
1515
import { ToolErrorType } from '../tools/tool-error.js';
1616
import { debugLogger } from '../utils/debugLogger.js';
1717
import type { ShellExecutionConfig } from '../index.js';
18-
import { ShellToolInvocation } from '../tools/shell.js';
1918
import { DiscoveredMCPToolInvocation } from '../tools/mcp-tool.js';
2019

2120
/**
@@ -26,7 +25,7 @@ import { DiscoveredMCPToolInvocation } from '../tools/mcp-tool.js';
2625
* @returns MCP context if this is an MCP tool, undefined otherwise
2726
*/
2827
function extractMcpContext(
29-
invocation: ShellToolInvocation | AnyToolInvocation,
28+
invocation: AnyToolInvocation,
3029
config: Config,
3130
): McpToolContext | undefined {
3231
if (!(invocation instanceof DiscoveredMCPToolInvocation)) {
@@ -63,18 +62,18 @@ function extractMcpContext(
6362
* @param signal Abort signal for cancellation
6463
* @param liveOutputCallback Optional callback for live output updates
6564
* @param shellExecutionConfig Optional shell execution config
66-
* @param setPidCallback Optional callback to set the PID for shell invocations
65+
* @param setExecutionIdCallback Optional callback to set an execution ID for backgroundable invocations
6766
* @param config Config to look up MCP server details for hook context
6867
* @returns The tool result
6968
*/
7069
export async function executeToolWithHooks(
71-
invocation: ShellToolInvocation | AnyToolInvocation,
70+
invocation: AnyToolInvocation,
7271
toolName: string,
7372
signal: AbortSignal,
7473
tool: AnyDeclarativeTool,
7574
liveOutputCallback?: (outputChunk: ToolLiveOutput) => void,
7675
shellExecutionConfig?: ShellExecutionConfig,
77-
setPidCallback?: (pid: number) => void,
76+
setExecutionIdCallback?: (executionId: number) => void,
7877
config?: Config,
7978
originalRequestName?: string,
8079
): Promise<ToolResult> {
@@ -154,22 +153,14 @@ export async function executeToolWithHooks(
154153
}
155154
}
156155

157-
// Execute the actual tool
158-
let toolResult: ToolResult;
159-
if (setPidCallback && invocation instanceof ShellToolInvocation) {
160-
toolResult = await invocation.execute(
161-
signal,
162-
liveOutputCallback,
163-
shellExecutionConfig,
164-
setPidCallback,
165-
);
166-
} else {
167-
toolResult = await invocation.execute(
168-
signal,
169-
liveOutputCallback,
170-
shellExecutionConfig,
171-
);
172-
}
156+
// Execute the actual tool. Tools that support backgrounding can optionally
157+
// surface an execution ID via the callback.
158+
const toolResult: ToolResult = await invocation.execute(
159+
signal,
160+
liveOutputCallback,
161+
shellExecutionConfig,
162+
setExecutionIdCallback,
163+
);
173164

174165
// Append notification if parameters were modified
175166
if (inputWasModified) {

0 commit comments

Comments
 (0)