mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-12 21:03:05 -07:00
Refine execution lifecycle facade follow-ups
This commit is contained in:
@@ -80,7 +80,7 @@ export const useShellCommandProcessor = (
|
|||||||
setShellInputFocused: (value: boolean) => void,
|
setShellInputFocused: (value: boolean) => void,
|
||||||
terminalWidth?: number,
|
terminalWidth?: number,
|
||||||
terminalHeight?: number,
|
terminalHeight?: number,
|
||||||
activeToolPtyId?: number,
|
activeBackgroundExecutionId?: number,
|
||||||
isWaitingForConfirmation?: boolean,
|
isWaitingForConfirmation?: boolean,
|
||||||
) => {
|
) => {
|
||||||
const [state, dispatch] = useReducer(shellReducer, initialState);
|
const [state, dispatch] = useReducer(shellReducer, initialState);
|
||||||
@@ -103,7 +103,8 @@ export const useShellCommandProcessor = (
|
|||||||
}
|
}
|
||||||
const m = manager.current;
|
const m = manager.current;
|
||||||
|
|
||||||
const activePtyId = state.activeShellPtyId || activeToolPtyId;
|
const activePtyId =
|
||||||
|
state.activeShellPtyId ?? activeBackgroundExecutionId ?? undefined;
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const isForegroundActive = !!activePtyId || !!isWaitingForConfirmation;
|
const isForegroundActive = !!activePtyId || !!isWaitingForConfirmation;
|
||||||
@@ -191,7 +192,8 @@ export const useShellCommandProcessor = (
|
|||||||
]);
|
]);
|
||||||
|
|
||||||
const backgroundCurrentShell = useCallback(() => {
|
const backgroundCurrentShell = useCallback(() => {
|
||||||
const pidToBackground = state.activeShellPtyId || activeToolPtyId;
|
const pidToBackground =
|
||||||
|
state.activeShellPtyId ?? activeBackgroundExecutionId;
|
||||||
if (pidToBackground) {
|
if (pidToBackground) {
|
||||||
ShellExecutionService.background(pidToBackground);
|
ShellExecutionService.background(pidToBackground);
|
||||||
m.backgroundedPids.add(pidToBackground);
|
m.backgroundedPids.add(pidToBackground);
|
||||||
@@ -202,7 +204,7 @@ export const useShellCommandProcessor = (
|
|||||||
m.restoreTimeout = null;
|
m.restoreTimeout = null;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}, [state.activeShellPtyId, activeToolPtyId, m]);
|
}, [state.activeShellPtyId, activeBackgroundExecutionId, m]);
|
||||||
|
|
||||||
const dismissBackgroundShell = useCallback(
|
const dismissBackgroundShell = useCallback(
|
||||||
(pid: number) => {
|
(pid: number) => {
|
||||||
|
|||||||
@@ -96,6 +96,31 @@ const MockedUserPromptEvent = vi.hoisted(() =>
|
|||||||
vi.fn().mockImplementation(() => {}),
|
vi.fn().mockImplementation(() => {}),
|
||||||
);
|
);
|
||||||
const mockParseAndFormatApiError = vi.hoisted(() => vi.fn());
|
const mockParseAndFormatApiError = vi.hoisted(() => vi.fn());
|
||||||
|
const mockIsBackgroundExecutionData = vi.hoisted(
|
||||||
|
() => (data: unknown): data is { executionId?: number; 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 ||
|
||||||
|
typeof value.initialOutput === 'string')
|
||||||
|
);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
const mockGetBackgroundExecutionId = vi.hoisted(
|
||||||
|
() =>
|
||||||
|
(data: { executionId?: number; pid?: number }): number | undefined =>
|
||||||
|
data.executionId ?? data.pid,
|
||||||
|
);
|
||||||
|
|
||||||
const MockValidationRequiredError = vi.hoisted(
|
const MockValidationRequiredError = vi.hoisted(
|
||||||
() =>
|
() =>
|
||||||
@@ -121,6 +146,11 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
|
|||||||
const actualCoreModule = (await importOriginal()) as any;
|
const actualCoreModule = (await importOriginal()) as any;
|
||||||
return {
|
return {
|
||||||
...actualCoreModule,
|
...actualCoreModule,
|
||||||
|
isBackgroundExecutionData:
|
||||||
|
actualCoreModule.isBackgroundExecutionData ??
|
||||||
|
mockIsBackgroundExecutionData,
|
||||||
|
getBackgroundExecutionId:
|
||||||
|
actualCoreModule.getBackgroundExecutionId ?? mockGetBackgroundExecutionId,
|
||||||
GitService: vi.fn(),
|
GitService: vi.fn(),
|
||||||
GeminiClient: MockedGeminiClientClass,
|
GeminiClient: MockedGeminiClientClass,
|
||||||
UserPromptEvent: MockedUserPromptEvent,
|
UserPromptEvent: MockedUserPromptEvent,
|
||||||
|
|||||||
@@ -37,6 +37,8 @@ import {
|
|||||||
buildUserSteeringHintPrompt,
|
buildUserSteeringHintPrompt,
|
||||||
GeminiCliOperation,
|
GeminiCliOperation,
|
||||||
getPlanModeExitMessage,
|
getPlanModeExitMessage,
|
||||||
|
getBackgroundExecutionId,
|
||||||
|
isBackgroundExecutionData,
|
||||||
} from '@google/gemini-cli-core';
|
} from '@google/gemini-cli-core';
|
||||||
import type {
|
import type {
|
||||||
Config,
|
Config,
|
||||||
@@ -100,12 +102,12 @@ interface BackgroundedToolInfo {
|
|||||||
initialOutput: string;
|
initialOutput: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
interface BackgroundExecutionData {
|
type BackgroundExecutionDataLike = {
|
||||||
executionId?: number;
|
executionId?: number;
|
||||||
pid?: number;
|
pid?: number;
|
||||||
command?: string;
|
command?: string;
|
||||||
initialOutput?: string;
|
initialOutput?: string;
|
||||||
}
|
} & Record<string, unknown>;
|
||||||
|
|
||||||
enum StreamProcessingStatus {
|
enum StreamProcessingStatus {
|
||||||
Completed,
|
Completed,
|
||||||
@@ -118,55 +120,58 @@ const SUPPRESSED_TOOL_ERRORS_NOTE =
|
|||||||
const LOW_VERBOSITY_FAILURE_NOTE =
|
const LOW_VERBOSITY_FAILURE_NOTE =
|
||||||
'This request failed. Press F12 for diagnostics, or run /settings and change "Error Verbosity" to full for full details.';
|
'This request failed. Press F12 for diagnostics, or run /settings and change "Error Verbosity" to full for full details.';
|
||||||
|
|
||||||
function isBackgroundExecutionData(
|
function isBackgroundExecutionDataValidator(
|
||||||
data: unknown,
|
candidate: unknown,
|
||||||
): data is BackgroundExecutionData {
|
): candidate is (data: unknown) => data is BackgroundExecutionDataLike {
|
||||||
if (typeof data !== 'object' || data === null) {
|
return typeof candidate === 'function';
|
||||||
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')
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function getBackgroundExecutionId(
|
function isBackgroundExecutionIdGetter(
|
||||||
data: BackgroundExecutionData,
|
candidate: unknown,
|
||||||
|
): candidate is (data: BackgroundExecutionDataLike) => number | undefined {
|
||||||
|
return typeof candidate === 'function';
|
||||||
|
}
|
||||||
|
|
||||||
|
function isBackgroundExecutionDataFromCore(
|
||||||
|
data: unknown,
|
||||||
|
): data is BackgroundExecutionDataLike {
|
||||||
|
const candidate: unknown = isBackgroundExecutionData;
|
||||||
|
if (isBackgroundExecutionDataValidator(candidate)) {
|
||||||
|
return candidate(data);
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
function getBackgroundExecutionIdFromCore(
|
||||||
|
data: BackgroundExecutionDataLike,
|
||||||
): number | undefined {
|
): number | undefined {
|
||||||
if (typeof data.executionId === 'number') {
|
const candidate: unknown = getBackgroundExecutionId;
|
||||||
return data.executionId;
|
if (isBackgroundExecutionIdGetter(candidate)) {
|
||||||
|
return candidate(data);
|
||||||
}
|
}
|
||||||
if (typeof data.pid === 'number') {
|
|
||||||
return data.pid;
|
return data.executionId ?? data.pid;
|
||||||
}
|
|
||||||
return undefined;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function getBackgroundedToolInfo(
|
function getBackgroundedToolInfo(
|
||||||
toolCall: TrackedCompletedToolCall | TrackedCancelledToolCall,
|
toolCall: TrackedCompletedToolCall | TrackedCancelledToolCall,
|
||||||
): BackgroundedToolInfo | undefined {
|
): BackgroundedToolInfo | undefined {
|
||||||
const response = toolCall.response as ToolResponseWithParts;
|
const response = toolCall.response as ToolResponseWithParts;
|
||||||
const rawData = response?.data;
|
const rawData: unknown = response?.data;
|
||||||
const data = isBackgroundExecutionData(rawData) ? rawData : undefined;
|
if (!isBackgroundExecutionDataFromCore(rawData)) {
|
||||||
const executionId = data ? getBackgroundExecutionId(data) : undefined;
|
return undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
const executionId = getBackgroundExecutionIdFromCore(rawData);
|
||||||
if (executionId === undefined) {
|
if (executionId === undefined) {
|
||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
executionId,
|
executionId,
|
||||||
command: data.command ?? toolCall.request.name,
|
command: rawData.command ?? toolCall.request.name,
|
||||||
initialOutput: data.initialOutput ?? '',
|
initialOutput: rawData.initialOutput ?? '',
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -133,6 +133,21 @@ describe('ExecutionLifecycleService', () => {
|
|||||||
expect(result.error?.message).toContain('Operation cancelled by user');
|
expect(result.error?.message).toContain('Operation cancelled by user');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('does not probe OS process state for completed non-process execution IDs', async () => {
|
||||||
|
const handle = ExecutionLifecycleService.createExecution();
|
||||||
|
if (handle.pid === undefined) {
|
||||||
|
throw new Error('Expected execution ID.');
|
||||||
|
}
|
||||||
|
|
||||||
|
ExecutionLifecycleService.completeExecution(handle.pid, { exitCode: 0 });
|
||||||
|
await handle.result;
|
||||||
|
|
||||||
|
const processKillSpy = vi.spyOn(process, 'kill');
|
||||||
|
expect(ExecutionLifecycleService.isActive(handle.pid)).toBe(false);
|
||||||
|
expect(processKillSpy).not.toHaveBeenCalled();
|
||||||
|
processKillSpy.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
it('manages external executions through registration hooks', async () => {
|
it('manages external executions through registration hooks', async () => {
|
||||||
const writeInput = vi.fn();
|
const writeInput = vi.fn();
|
||||||
const isActive = vi.fn().mockReturnValue(true);
|
const isActive = vi.fn().mockReturnValue(true);
|
||||||
|
|||||||
@@ -312,16 +312,6 @@ export class ExecutionLifecycleService {
|
|||||||
this.settleExecution(executionId, result);
|
this.settleExecution(executionId, result);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* @deprecated Use completeWithResult() for new call sites.
|
|
||||||
*/
|
|
||||||
static finalizeExecution(
|
|
||||||
executionId: number,
|
|
||||||
result: ExecutionResult,
|
|
||||||
): void {
|
|
||||||
this.completeWithResult(executionId, result);
|
|
||||||
}
|
|
||||||
|
|
||||||
static background(executionId: number): void {
|
static background(executionId: number): void {
|
||||||
const resolve = this.activeResolvers.get(executionId);
|
const resolve = this.activeResolvers.get(executionId);
|
||||||
if (!resolve) {
|
if (!resolve) {
|
||||||
@@ -423,6 +413,9 @@ export class ExecutionLifecycleService {
|
|||||||
static isActive(executionId: number): boolean {
|
static isActive(executionId: number): boolean {
|
||||||
const execution = this.activeExecutions.get(executionId);
|
const execution = this.activeExecutions.get(executionId);
|
||||||
if (!execution) {
|
if (!execution) {
|
||||||
|
if (executionId >= NON_PROCESS_EXECUTION_ID_START) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
return process.kill(executionId, 0);
|
return process.kill(executionId, 0);
|
||||||
} catch {
|
} catch {
|
||||||
|
|||||||
Reference in New Issue
Block a user