fix(acp): run exit cleanup when stdin closes (#14953)

Signed-off-by: Adrian Cole <adrian@tetrate.io>
Co-authored-by: Allen Hutchison <adh@google.com>
Co-authored-by: Allen Hutchison <allen@hutchison.org>
This commit is contained in:
Adrian Cole
2026-01-15 06:02:44 +08:00
committed by GitHub
parent b14cf1dc30
commit 7e6817da5b
6 changed files with 133 additions and 9 deletions

View File

@@ -0,0 +1,116 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { TestRig } from './test-helper.js';
import { spawn, ChildProcess } from 'node:child_process';
import { join } from 'node:path';
import { readFileSync, existsSync } from 'node:fs';
import { Writable, Readable } from 'node:stream';
import { env } from 'node:process';
import * as acp from '@agentclientprotocol/sdk';
// Skip in sandbox mode - test spawns CLI directly which behaves differently in containers
const sandboxEnv = env['GEMINI_SANDBOX'];
const itMaybe = sandboxEnv && sandboxEnv !== 'false' ? it.skip : it;
// Reuse existing fake responses that return a simple "Hello" response
const SIMPLE_RESPONSE_PATH = 'hooks-system.session-startup.responses';
class SessionUpdateCollector implements acp.Client {
updates: acp.SessionNotification[] = [];
sessionUpdate = async (params: acp.SessionNotification) => {
this.updates.push(params);
};
requestPermission = async (): Promise<acp.RequestPermissionResponse> => {
throw new Error('unexpected');
};
}
describe('ACP telemetry', () => {
let rig: TestRig;
let child: ChildProcess | undefined;
beforeEach(() => {
rig = new TestRig();
});
afterEach(async () => {
child?.kill();
child = undefined;
await rig.cleanup();
});
itMaybe('should flush telemetry when connection closes', async () => {
rig.setup('acp-telemetry-flush', {
fakeResponsesPath: join(import.meta.dirname, SIMPLE_RESPONSE_PATH),
});
const telemetryPath = join(rig.homeDir!, 'telemetry.log');
const bundlePath = join(import.meta.dirname, '..', 'bundle/gemini.js');
child = spawn(
'node',
[
bundlePath,
'--experimental-acp',
'--fake-responses',
join(rig.testDir!, 'fake-responses.json'),
],
{
cwd: rig.testDir!,
stdio: ['pipe', 'pipe', 'inherit'],
env: {
...process.env,
GEMINI_API_KEY: 'fake-key',
GEMINI_CLI_HOME: rig.homeDir!,
GEMINI_TELEMETRY_ENABLED: 'true',
GEMINI_TELEMETRY_TARGET: 'local',
GEMINI_TELEMETRY_OUTFILE: telemetryPath,
// GEMINI_DEV_TRACING not set: fake responses aren't instrumented for spans
},
},
);
const input = Writable.toWeb(child.stdin!);
const output = Readable.toWeb(child.stdout!) as ReadableStream<Uint8Array>;
const testClient = new SessionUpdateCollector();
const stream = acp.ndJsonStream(input, output);
const connection = new acp.ClientSideConnection(() => testClient, stream);
await connection.initialize({
protocolVersion: acp.PROTOCOL_VERSION,
clientCapabilities: { fs: { readTextFile: false, writeTextFile: false } },
});
const { sessionId } = await connection.newSession({
cwd: rig.testDir!,
mcpServers: [],
});
await connection.prompt({
sessionId,
prompt: [{ type: 'text', text: 'Say hello' }],
});
expect(JSON.stringify(testClient.updates)).toContain('Hello');
// Close stdin to trigger telemetry flush via runExitCleanup()
child.stdin!.end();
await new Promise<void>((resolve) => {
child!.on('close', () => resolve());
});
child = undefined;
// gen_ai.output.messages is the last OTEL log emitted (after prompt response)
expect(existsSync(telemetryPath)).toBe(true);
expect(readFileSync(telemetryPath, 'utf-8')).toContain(
'gen_ai.output.messages',
);
});
});

9
package-lock.json generated
View File

@@ -19,6 +19,7 @@
"gemini": "bundle/gemini.js" "gemini": "bundle/gemini.js"
}, },
"devDependencies": { "devDependencies": {
"@agentclientprotocol/sdk": "^0.12.0",
"@octokit/rest": "^22.0.0", "@octokit/rest": "^22.0.0",
"@types/marked": "^5.0.2", "@types/marked": "^5.0.2",
"@types/mime-types": "^3.0.1", "@types/mime-types": "^3.0.1",
@@ -110,9 +111,9 @@
} }
}, },
"node_modules/@agentclientprotocol/sdk": { "node_modules/@agentclientprotocol/sdk": {
"version": "0.11.0", "version": "0.12.0",
"resolved": "https://registry.npmjs.org/@agentclientprotocol/sdk/-/sdk-0.11.0.tgz", "resolved": "https://registry.npmjs.org/@agentclientprotocol/sdk/-/sdk-0.12.0.tgz",
"integrity": "sha512-hngnMwQ13DCC7oEr0BUnrx+vTDFf/ToCLhF0YcCMWRs+v4X60rKQyAENsx0PdbQF21jC1VjMFkh2+vwNBLh6fQ==", "integrity": "sha512-V8uH/KK1t7utqyJmTA7y7DzKu6+jKFIXM+ZVouz8E55j8Ej2RV42rEvPKn3/PpBJlliI5crcGk1qQhZ7VwaepA==",
"license": "Apache-2.0", "license": "Apache-2.0",
"peerDependencies": { "peerDependencies": {
"zod": "^3.25.0 || ^4.0.0" "zod": "^3.25.0 || ^4.0.0"
@@ -18695,7 +18696,7 @@
"version": "0.26.0-nightly.20260114.bb6c57414", "version": "0.26.0-nightly.20260114.bb6c57414",
"license": "Apache-2.0", "license": "Apache-2.0",
"dependencies": { "dependencies": {
"@agentclientprotocol/sdk": "^0.11.0", "@agentclientprotocol/sdk": "^0.12.0",
"@google/gemini-cli-core": "file:../core", "@google/gemini-cli-core": "file:../core",
"@google/genai": "1.30.0", "@google/genai": "1.30.0",
"@iarna/toml": "^2.2.5", "@iarna/toml": "^2.2.5",

View File

@@ -79,6 +79,7 @@
"LICENSE" "LICENSE"
], ],
"devDependencies": { "devDependencies": {
"@agentclientprotocol/sdk": "^0.12.0",
"@octokit/rest": "^22.0.0", "@octokit/rest": "^22.0.0",
"@types/marked": "^5.0.2", "@types/marked": "^5.0.2",
"@types/mime-types": "^3.0.1", "@types/mime-types": "^3.0.1",

View File

@@ -29,7 +29,7 @@
"sandboxImageUri": "us-docker.pkg.dev/gemini-code-dev/gemini-cli/sandbox:0.26.0-nightly.20260114.bb6c57414" "sandboxImageUri": "us-docker.pkg.dev/gemini-code-dev/gemini-cli/sandbox:0.26.0-nightly.20260114.bb6c57414"
}, },
"dependencies": { "dependencies": {
"@agentclientprotocol/sdk": "^0.11.0", "@agentclientprotocol/sdk": "^0.12.0",
"@google/gemini-cli-core": "file:../core", "@google/gemini-cli-core": "file:../core",
"@google/genai": "1.30.0", "@google/genai": "1.30.0",
"@iarna/toml": "^2.2.5", "@iarna/toml": "^2.2.5",

View File

@@ -44,6 +44,7 @@ import { z } from 'zod';
import { randomUUID } from 'node:crypto'; import { randomUUID } from 'node:crypto';
import type { CliArgs } from '../config/config.js'; import type { CliArgs } from '../config/config.js';
import { loadCliConfig } from '../config/config.js'; import { loadCliConfig } from '../config/config.js';
import { runExitCleanup } from '../utils/cleanup.js';
export async function runZedIntegration( export async function runZedIntegration(
config: Config, config: Config,
@@ -55,10 +56,15 @@ export async function runZedIntegration(
const stdin = Readable.toWeb(process.stdin) as ReadableStream<Uint8Array>; const stdin = Readable.toWeb(process.stdin) as ReadableStream<Uint8Array>;
const stream = acp.ndJsonStream(stdout, stdin); const stream = acp.ndJsonStream(stdout, stdin);
new acp.AgentSideConnection( const connection = new acp.AgentSideConnection(
(connection) => new GeminiAgent(config, settings, argv, connection), (connection) => new GeminiAgent(config, settings, argv, connection),
stream, stream,
); );
// SIGTERM/SIGINT handlers (in sdk.ts) don't fire when stdin closes.
// We must explicitly await the connection close to flush telemetry.
// Use finally() to ensure cleanup runs even on stream errors.
await connection.closed.finally(runExitCleanup);
} }
export class GeminiAgent { export class GeminiAgent {

View File

@@ -114,10 +114,10 @@ export async function createContentGenerator(
): Promise<ContentGenerator> { ): Promise<ContentGenerator> {
const generator = await (async () => { const generator = await (async () => {
if (gcConfig.fakeResponses) { if (gcConfig.fakeResponses) {
return new LoggingContentGenerator( const fakeGenerator = await FakeContentGenerator.fromFile(
await FakeContentGenerator.fromFile(gcConfig.fakeResponses), gcConfig.fakeResponses,
gcConfig,
); );
return new LoggingContentGenerator(fakeGenerator, gcConfig);
} }
const version = await getVersion(); const version = await getVersion();
const model = resolveModel( const model = resolveModel(