feat(acp): Implemented the structured terminal lifecycle for ACP.

-   Modified `packages/core/src/tools/shell.ts` to always include `exitCode` and `signal` in `ToolResult.data`.
-   Modified `packages/cli/src/acp/acpClient.ts` to send `_meta.terminal_info` on start, `_meta.terminal_output` during streaming (deltas), and `_meta.terminal_exit` on completion.
-   Added verification tests in `packages/cli/src/acp/acpClient.test.ts`
This commit is contained in:
Sri Pasumarthi
2026-03-26 16:45:54 -07:00
parent 765fb67011
commit 4dbe7f1833
3 changed files with 227 additions and 7 deletions
+121 -1
View File
@@ -39,6 +39,7 @@ import { loadCliConfig, type CliArgs } from '../config/config.js';
import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import { ApprovalMode } from '@google/gemini-cli-core/src/policy/types.js';
import { SHELL_TOOL_NAME } from '@google/gemini-cli-core';
vi.mock('../config/config.js', () => ({
loadCliConfig: vi.fn(),
@@ -656,8 +657,32 @@ describe('Session', () => {
execute: vi.fn().mockResolvedValue({ llmContent: 'Tool Result' }),
}),
};
const mockShellTool = {
name: SHELL_TOOL_NAME,
kind: 'execute',
build: vi.fn().mockReturnValue({
getDescription: () => 'Execute shell command',
toolLocations: () => [],
shouldConfirmExecute: vi.fn().mockResolvedValue(null),
execute: vi.fn().mockImplementation(async (abortSignal, onOutput) => {
if (onOutput) {
onOutput([[{ text: 'chunk 1' }]]);
onOutput([[{ text: 'chunk 2' }]]);
}
return {
llmContent: 'Command finished',
data: { exitCode: 0 },
};
}),
}),
};
mockToolRegistry = {
getTool: vi.fn().mockReturnValue(mockTool),
getTool: vi.fn().mockImplementation((name) => {
if (name === SHELL_TOOL_NAME || name === 'shell') {
return mockShellTool;
}
return mockTool;
}),
};
mockMessageBus = {
publish: vi.fn(),
@@ -971,6 +996,101 @@ describe('Session', () => {
expect(result).toMatchObject({ stopReason: 'end_turn' });
});
it('should emit terminal lifecycle events for ShellTool', async () => {
const stream1 = createMockStream([
{
type: StreamEventType.CHUNK,
value: {
functionCalls: [{ name: SHELL_TOOL_NAME, args: { command: 'ls' } }],
},
},
]);
const stream2 = createMockStream([
{
type: StreamEventType.CHUNK,
value: {
candidates: [{ content: { parts: [{ text: 'Result' }] } }],
},
},
]);
mockChat.sendMessageStream
.mockResolvedValueOnce(stream1)
.mockResolvedValueOnce(stream2);
const result = await session.prompt({
sessionId: 'session-1',
prompt: [{ type: 'text', text: 'Run ls' }],
});
expect(mockToolRegistry.getTool).toHaveBeenCalledWith(SHELL_TOOL_NAME);
// Verify terminal_info
expect(mockConnection.sessionUpdate).toHaveBeenCalledWith(
expect.objectContaining({
update: expect.objectContaining({
sessionUpdate: 'tool_call',
status: 'in_progress',
_meta: expect.objectContaining({
terminal_info: expect.objectContaining({
cwd: '/tmp',
terminal_id: expect.any(String),
}),
}),
}),
}),
);
// Verify terminal_output (2 chunks)
expect(mockConnection.sessionUpdate).toHaveBeenCalledWith(
expect.objectContaining({
update: expect.objectContaining({
sessionUpdate: 'tool_call_update',
status: 'in_progress',
_meta: expect.objectContaining({
terminal_output: expect.objectContaining({
data: 'chunk 1',
terminal_id: expect.any(String),
}),
}),
}),
}),
);
expect(mockConnection.sessionUpdate).toHaveBeenCalledWith(
expect.objectContaining({
update: expect.objectContaining({
sessionUpdate: 'tool_call_update',
status: 'in_progress',
_meta: expect.objectContaining({
terminal_output: expect.objectContaining({
data: 'chunk 2',
terminal_id: expect.any(String),
}),
}),
}),
}),
);
// Verify terminal_exit
expect(mockConnection.sessionUpdate).toHaveBeenCalledWith(
expect.objectContaining({
update: expect.objectContaining({
sessionUpdate: 'tool_call_update',
status: 'completed',
_meta: expect.objectContaining({
terminal_exit: expect.objectContaining({
exit_code: 0,
signal: null,
terminal_id: expect.any(String),
}),
}),
}),
}),
);
expect(result).toMatchObject({ stopReason: 'end_turn' });
});
it('should handle tool call permission request', async () => {
const confirmationDetails = {
type: 'info',
+99 -2
View File
@@ -51,6 +51,7 @@ import {
InvalidStreamError,
type AgentLoopContext,
updatePolicy,
SHELL_TOOL_NAME,
} from '@google/gemini-cli-core';
import * as acp from '@agentclientprotocol/sdk';
import { AcpFileSystemService } from './fileSystemService.js';
@@ -1106,8 +1107,34 @@ export class Session {
throw new Error(`Unexpected: ${resultOutcome}`);
}
}
if (tool.name === SHELL_TOOL_NAME) {
await this.sendUpdate({
sessionUpdate: 'tool_call_update',
toolCallId: callId,
status: 'in_progress',
title: displayTitle,
content: [],
_meta: {
terminal_info: {
cwd: this.context.config.getTargetDir(),
terminal_id: callId,
},
},
});
}
} else {
const content: acp.ToolCallContent[] = [];
let meta = undefined;
if (tool.name === SHELL_TOOL_NAME) {
meta = {
terminal_info: {
cwd: this.context.config.getTargetDir(),
terminal_id: callId,
},
};
}
await this.sendUpdate({
sessionUpdate: 'tool_call',
@@ -1117,22 +1144,92 @@ export class Session {
content,
locations: invocation.toolLocations(),
kind: toAcpToolKind(tool.kind),
_meta: meta ?? undefined,
});
}
const toolResult: ToolResult = await invocation.execute(abortSignal);
const updateOutput = async (output: unknown) => {
if (tool.name === SHELL_TOOL_NAME) {
let data = '';
if (typeof output === 'string') {
data = output;
} else if (Array.isArray(output)) {
data = output
.map((line) => {
if (Array.isArray(line)) {
return line
.map((token) => {
if (
typeof token === 'object' &&
token !== null &&
'text' in token
) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
return (token as { text?: string }).text || '';
}
return '';
})
.join('');
}
return '';
})
.join('\n');
}
await this.sendUpdate({
sessionUpdate: 'tool_call_update',
toolCallId: callId,
status: 'in_progress',
content: [],
_meta: {
terminal_output: {
data,
terminal_id: callId,
},
},
});
}
};
const toolResult: ToolResult = await invocation.execute(
abortSignal,
updateOutput,
);
const content = toToolCallContent(toolResult);
const updateContent: acp.ToolCallContent[] = content ? [content] : [];
let meta: Record<string, unknown> | undefined = undefined;
const isShellTool = tool.name === SHELL_TOOL_NAME;
const isShellError = !!(isShellTool && toolResult.data?.['isError']);
if (isShellTool) {
const rawExitCode = toolResult.data?.['exitCode'];
const exitCode: number | undefined =
typeof rawExitCode === 'number' ? rawExitCode : undefined;
const rawSignal = toolResult.data?.['signal'];
const signal: string | null =
typeof rawSignal === 'string' ? rawSignal : null;
meta = {
terminal_exit: {
exit_code:
typeof exitCode === 'number' ? exitCode : isShellError ? 1 : 0,
signal,
terminal_id: callId,
},
};
}
await this.sendUpdate({
sessionUpdate: 'tool_call_update',
toolCallId: callId,
status: 'completed',
status: toolResult.error || isShellError ? 'failed' : 'completed',
title: displayTitle,
content: updateContent,
locations: invocation.toolLocations(),
kind: toAcpToolKind(tool.kind),
_meta: meta ?? undefined,
});
const durationMs = Date.now() - startTime;
+7 -4
View File
@@ -452,12 +452,15 @@ export class ShellToolInvocation extends BaseToolInvocation<
if (result.exitCode !== null && result.exitCode !== 0) {
llmContentParts.push(`Exit Code: ${result.exitCode}`);
data = {
exitCode: result.exitCode,
isError: true,
};
}
data = {
exitCode: result.exitCode,
signal: result.signal,
isError: result.exitCode !== 0 || !!result.error || !!result.signal,
pid: result.pid,
};
if (result.signal) {
llmContentParts.push(`Signal: ${result.signal}`);
}