From c77fd3fc7ae838ac9a1a0a7c248b6ff63a687d2c Mon Sep 17 00:00:00 2001 From: Adam Weidman Date: Mon, 9 Mar 2026 01:10:42 -0400 Subject: [PATCH] refactor: standardize on pid as canonical background execution field Remove the dual executionId/pid fields from BackgroundExecutionData. Use pid everywhere for consistency with existing types (ExecutingToolCall, ExecutionHandle). A codebase-wide rename to executionId is planned as a follow-up once all consumers are migrated. --- .../cli/src/ui/hooks/useGeminiStream.test.tsx | 10 +------ packages/cli/src/ui/hooks/useGeminiStream.ts | 16 +++++------- packages/core/src/tools/shell.ts | 1 - packages/core/src/tools/tools.ts | 26 ++++--------------- 4 files changed, 12 insertions(+), 41 deletions(-) diff --git a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx index e60ff906fe..70f6a6ef37 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx +++ b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx @@ -97,18 +97,16 @@ const MockedUserPromptEvent = vi.hoisted(() => ); const mockParseAndFormatApiError = vi.hoisted(() => vi.fn()); const mockIsBackgroundExecutionData = vi.hoisted( - () => (data: unknown): data is { executionId?: number; pid?: number } => { + () => (data: unknown): data is { pid?: number } => { if (typeof data !== 'object' || data === null) { return false; } const value = data as { - executionId?: unknown; pid?: unknown; command?: unknown; initialOutput?: unknown; }; return ( - (value.executionId === undefined || typeof value.executionId === 'number') && (value.pid === undefined || typeof value.pid === 'number') && (value.command === undefined || typeof value.command === 'string') && (value.initialOutput === undefined || @@ -116,11 +114,6 @@ const mockIsBackgroundExecutionData = vi.hoisted( ); }, ); -const mockGetBackgroundExecutionId = vi.hoisted( - () => - (data: { executionId?: number; pid?: number }): number | undefined => - data.executionId ?? data.pid, -); const MockValidationRequiredError = vi.hoisted( () => @@ -147,7 +140,6 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { return { ...actualCoreModule, isBackgroundExecutionData: mockIsBackgroundExecutionData, - getBackgroundExecutionId: mockGetBackgroundExecutionId, GitService: vi.fn(), GeminiClient: MockedGeminiClientClass, UserPromptEvent: MockedUserPromptEvent, diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index 05af263300..3aee149fb9 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -37,11 +37,9 @@ import { buildUserSteeringHintPrompt, GeminiCliOperation, getPlanModeExitMessage, - getBackgroundExecutionId, isBackgroundExecutionData, } from '@google/gemini-cli-core'; import type { - BackgroundExecutionData, Config, EditorType, GeminiClient, @@ -98,7 +96,7 @@ type ToolResponseWithParts = ToolCallResponseInfo & { }; interface BackgroundedToolInfo { - executionId: number; + pid: number; command: string; initialOutput: string; } @@ -123,16 +121,14 @@ function getBackgroundedToolInfo( return undefined; } - const data: BackgroundExecutionData = rawData; - const executionId = getBackgroundExecutionId(data); - if (executionId === undefined) { + if (rawData.pid === undefined) { return undefined; } return { - executionId, - command: data.command ?? toolCall.request.name, - initialOutput: data.initialOutput ?? '', + pid: rawData.pid, + command: rawData.command ?? toolCall.request.name, + initialOutput: rawData.initialOutput ?? '', }; } @@ -1676,7 +1672,7 @@ export const useGeminiStream = ( const backgroundedTool = getBackgroundedToolInfo(toolCall); if (backgroundedTool) { registerBackgroundShell( - backgroundedTool.executionId, + backgroundedTool.pid, backgroundedTool.command, backgroundedTool.initialOutput, ); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 701d2df713..c21a2bddc8 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -347,7 +347,6 @@ export class ShellToolInvocation extends BaseToolInvocation< } else if (this.params.is_background || result.backgrounded) { llmContent = `Command moved to background (PID: ${result.pid}). Output hidden. Press Ctrl+B to view.`; data = { - executionId: result.pid, pid: result.pid, command: this.params.command, initialOutput: result.output, diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 585805dd5f..730a40a435 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -75,15 +75,13 @@ export interface ToolInvocation< /** * Structured payload used by tools to surface background execution metadata to * the CLI UI. + * + * NOTE: `pid` is used as the canonical identifier for now to stay consistent + * with existing types (ExecutingToolCall.pid, ExecutionHandle.pid, etc.). + * A future rename to `executionId` is planned once the codebase is fully + * migrated — not done in this PR to keep the diff focused on the abstraction. */ export interface BackgroundExecutionData extends Record { - /** - * Neutral execution identifier for background lifecycle tracking. - */ - executionId?: number; - /** - * Backwards-compatible alias for executionId. - */ pid?: number; command?: string; initialOutput?: string; @@ -96,32 +94,18 @@ export function isBackgroundExecutionData( return false; } - const executionId = 'executionId' in data ? data.executionId : undefined; const pid = 'pid' in data ? data.pid : undefined; const command = 'command' in data ? data.command : undefined; const initialOutput = 'initialOutput' in data ? data.initialOutput : undefined; return ( - (executionId === undefined || typeof executionId === 'number') && (pid === undefined || typeof pid === 'number') && (command === undefined || typeof command === 'string') && (initialOutput === undefined || typeof initialOutput === 'string') ); } -export function getBackgroundExecutionId( - data: BackgroundExecutionData, -): number | undefined { - if (typeof data.executionId === 'number') { - return data.executionId; - } - if (typeof data.pid === 'number') { - return data.pid; - } - return undefined; -} - /** * Options for policy updates that can be customized by tool invocations. */