diff --git a/integration-tests/acp-filesystem.read.responses b/integration-tests/acp-filesystem.read.responses new file mode 100644 index 00000000000..c96c72fc0ba --- /dev/null +++ b/integration-tests/acp-filesystem.read.responses @@ -0,0 +1,2 @@ +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"**Preparing File Read**\n\nI'll use the read_file tool to fetch the contents of test.txt.\n","thought":true}],"role":"model"},"index":0}],"usageMetadata":{"promptTokenCount":100,"totalTokenCount":110,"thoughtsTokenCount":10}},{"candidates":[{"content":{"parts":[{"functionCall":{"name":"read_file","args":{"file_path":"test.txt"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":12,"totalTokenCount":122}}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"The file contains: client content"}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":120,"candidatesTokenCount":8,"totalTokenCount":128}}]} diff --git a/integration-tests/acp-filesystem.test.ts b/integration-tests/acp-filesystem.test.ts new file mode 100644 index 00000000000..33b7cc2a12c --- /dev/null +++ b/integration-tests/acp-filesystem.test.ts @@ -0,0 +1,212 @@ +/** + * @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 { 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; + +const READ_RESPONSES_PATH = 'acp-filesystem.read.responses'; +const WRITE_RESPONSES_PATH = 'acp-filesystem.write.responses'; + +function collectMessages(updates: acp.SessionNotification[]): string { + return updates + .filter((u) => u.update.sessionUpdate === 'agent_message_chunk') + .map((u) => { + const upd = u.update; + if (upd.sessionUpdate === 'agent_message_chunk') { + const content = upd.content; + return content && 'text' in content ? content.text : ''; + } + return ''; + }) + .join(''); +} + +describe('ACP filesystem', () => { + let rig: TestRig; + let child: ChildProcess | undefined; + + beforeEach(() => { + rig = new TestRig(); + }); + + afterEach(async () => { + child?.kill(); + child = undefined; + await rig.cleanup(); + }); + + itMaybe('delegates read_file to ACP client', async () => { + rig.setup('acp-filesystem-read', { + fakeResponsesPath: join(import.meta.dirname, READ_RESPONSES_PATH), + settings: { tools: { core: ['read_file'] } }, + }); + + rig.createFile('test.txt', 'local content'); + + 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!, + }, + }, + ); + + const updates: acp.SessionNotification[] = []; + let readTextFilePath: string | null = null; + + const client: acp.Client = { + sessionUpdate: async (params) => { + updates.push(params); + }, + requestPermission: async () => ({ + outcome: { outcome: 'selected', optionId: 'proceed_once' }, + }), + readTextFile: async (params) => { + readTextFilePath = params.path; + return { content: 'client content' }; + }, + writeTextFile: async () => {}, + }; + + const input = Writable.toWeb(child.stdin!); + const output = Readable.toWeb(child.stdout!) as ReadableStream; + const stream = acp.ndJsonStream(input, output); + const connection = new acp.ClientSideConnection(() => client, stream); + + await connection.initialize({ + protocolVersion: acp.PROTOCOL_VERSION, + clientCapabilities: { fs: { readTextFile: true, writeTextFile: true } }, + }); + + const { sessionId } = await connection.newSession({ + cwd: rig.testDir!, + mcpServers: [], + }); + + const result = await connection.prompt({ + sessionId, + prompt: [{ type: 'text', text: 'Read test.txt' }], + }); + + expect(result).toEqual({ stopReason: 'end_turn' }); + expect(readTextFilePath).toBeTruthy(); + expect(readTextFilePath).toContain('test.txt'); + expect(collectMessages(updates)).toContain('client content'); + }); + + itMaybe( + 'treats ACP RESOURCE_NOT_FOUND as ENOENT during write_file', + async () => { + rig.setup('acp-filesystem-write', { + fakeResponsesPath: join(import.meta.dirname, WRITE_RESPONSES_PATH), + settings: { tools: { core: ['write_file'] } }, + }); + + 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!, + }, + }, + ); + + const updates: acp.SessionNotification[] = []; + let writeTextFileCalled = false; + let writeTextFilePath: string | null = null; + let writeTextFileContent: string | null = null; + + const client: acp.Client = { + sessionUpdate: async (params) => { + updates.push(params); + }, + requestPermission: async () => ({ + outcome: { outcome: 'selected', optionId: 'proceed_once' }, + }), + readTextFile: async (params) => { + throw new acp.RequestError( + -32002, + `Resource not found: ${params.path}`, + { uri: params.path }, + ); + }, + writeTextFile: async (params) => { + writeTextFileCalled = true; + writeTextFilePath = params.path; + writeTextFileContent = params.content; + }, + }; + + const input = Writable.toWeb(child.stdin!); + const output = Readable.toWeb( + child.stdout!, + ) as ReadableStream; + const stream = acp.ndJsonStream(input, output); + const connection = new acp.ClientSideConnection(() => client, stream); + + await connection.initialize({ + protocolVersion: acp.PROTOCOL_VERSION, + clientCapabilities: { fs: { readTextFile: true, writeTextFile: true } }, + }); + + const { sessionId } = await connection.newSession({ + cwd: rig.testDir!, + mcpServers: [], + }); + + const result = await connection.prompt({ + sessionId, + prompt: [{ type: 'text', text: 'Write hello to new-file.txt' }], + }); + + expect(result).toEqual({ stopReason: 'end_turn' }); + expect(writeTextFileCalled).toBe(true); + expect(writeTextFilePath).toBeTruthy(); + expect(writeTextFilePath).toContain('new-file.txt'); + expect(writeTextFileContent).toBe('hello'); + + const toolCompleted = updates.find((u) => { + const upd = u.update; + return ( + upd.sessionUpdate === 'tool_call_update' && upd.status === 'completed' + ); + }); + expect(toolCompleted).toBeDefined(); + }, + ); +}); diff --git a/integration-tests/acp-filesystem.write.responses b/integration-tests/acp-filesystem.write.responses new file mode 100644 index 00000000000..726050ad316 --- /dev/null +++ b/integration-tests/acp-filesystem.write.responses @@ -0,0 +1,2 @@ +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"**Preparing File Write**\n\nI'll write to new-file.txt using write_file.\n","thought":true}],"role":"model"},"index":0}],"usageMetadata":{"promptTokenCount":100,"totalTokenCount":110,"thoughtsTokenCount":10}},{"candidates":[{"content":{"parts":[{"functionCall":{"name":"write_file","args":{"file_path":"new-file.txt","content":"hello"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":12,"totalTokenCount":122}}]} +{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"Wrote the file."}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":120,"candidatesTokenCount":6,"totalTokenCount":126}}]} diff --git a/packages/cli/src/zed-integration/fileSystemService.test.ts b/packages/cli/src/zed-integration/fileSystemService.test.ts index 66624d54491..9830f7a9a39 100644 --- a/packages/cli/src/zed-integration/fileSystemService.test.ts +++ b/packages/cli/src/zed-integration/fileSystemService.test.ts @@ -7,6 +7,7 @@ import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest'; import { AcpFileSystemService } from './fileSystemService.js'; import type { AgentSideConnection } from '@agentclientprotocol/sdk'; +import { RequestError } from '@agentclientprotocol/sdk'; import type { FileSystemService } from '@google/gemini-cli-core'; describe('AcpFileSystemService', () => { @@ -24,6 +25,7 @@ describe('AcpFileSystemService', () => { mockFallback = { readTextFile: vi.fn(), writeTextFile: vi.fn(), + findFiles: vi.fn(), }; }); @@ -70,6 +72,28 @@ describe('AcpFileSystemService', () => { expect(result).toBe('content'); verify(); }); + + it('should convert RESOURCE_NOT_FOUND to ENOENT', async () => { + service = new AcpFileSystemService( + mockConnection, + 'session-1', + { readTextFile: true, writeTextFile: true }, + mockFallback, + ); + mockConnection.readTextFile.mockRejectedValue( + new RequestError(-32002, 'File not found', { + uri: '/missing/file', + }), + ); + + await expect(service.readTextFile('/missing/file')).rejects.toMatchObject( + { + code: 'ENOENT', + syscall: 'open', + path: '/missing/file', + }, + ); + }); }); describe('writeTextFile', () => { diff --git a/packages/cli/src/zed-integration/fileSystemService.ts b/packages/cli/src/zed-integration/fileSystemService.ts index 51a32f27796..810e3ee2fe9 100644 --- a/packages/cli/src/zed-integration/fileSystemService.ts +++ b/packages/cli/src/zed-integration/fileSystemService.ts @@ -5,7 +5,7 @@ */ import type { FileSystemService } from '@google/gemini-cli-core'; -import type * as acp from '@agentclientprotocol/sdk'; +import * as acp from '@agentclientprotocol/sdk'; /** * ACP client-based implementation of FileSystemService @@ -14,7 +14,7 @@ export class AcpFileSystemService implements FileSystemService { constructor( private readonly connection: acp.AgentSideConnection, private readonly sessionId: string, - private readonly capabilities: acp.FileSystemCapability, + readonly capabilities: acp.FileSystemCapability, private readonly fallback: FileSystemService, ) {} @@ -23,12 +23,32 @@ export class AcpFileSystemService implements FileSystemService { return this.fallback.readTextFile(filePath); } - const response = await this.connection.readTextFile({ - path: filePath, - sessionId: this.sessionId, - }); + try { + const response = await this.connection.readTextFile({ + path: filePath, + sessionId: this.sessionId, + }); - return response.content; + return response.content; + } catch (err) { + // Convert ACP error to Node.js ENOENT for file not found + const requestErrorCode = + err instanceof acp.RequestError + ? err.code + : typeof err === 'object' && err !== null && 'code' in err + ? (err as { code?: unknown }).code + : undefined; + if (requestErrorCode === -32002 || requestErrorCode === '-32002') { + const nodeErr = new Error( + `ENOENT: open '${filePath}'`, + ) as NodeJS.ErrnoException; + nodeErr.code = 'ENOENT'; + nodeErr.syscall = 'open'; + nodeErr.path = filePath; + throw nodeErr; + } + throw err; + } } async writeTextFile(filePath: string, content: string): Promise { @@ -42,4 +62,8 @@ export class AcpFileSystemService implements FileSystemService { sessionId: this.sessionId, }); } + + findFiles(fileName: string, searchPaths: readonly string[]): string[] { + return this.fallback.findFiles(fileName, searchPaths); + } } diff --git a/packages/core/src/services/fileSystemService.test.ts b/packages/core/src/services/fileSystemService.test.ts index 4ca5c3329ef..61003f27129 100644 --- a/packages/core/src/services/fileSystemService.test.ts +++ b/packages/core/src/services/fileSystemService.test.ts @@ -7,8 +7,12 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import fs from 'node:fs/promises'; import { StandardFileSystemService } from './fileSystemService.js'; +import * as fileUtils from '../utils/fileUtils.js'; vi.mock('fs/promises'); +vi.mock('../utils/fileUtils.js', () => ({ + readFileWithEncoding: vi.fn(), +})); describe('StandardFileSystemService', () => { let fileSystem: StandardFileSystemService; @@ -23,19 +27,21 @@ describe('StandardFileSystemService', () => { }); describe('readTextFile', () => { - it('should read file content using fs', async () => { + it('should read file content using BOM-aware reader', async () => { const testContent = 'Hello, World!'; - vi.mocked(fs.readFile).mockResolvedValue(testContent); + vi.mocked(fileUtils.readFileWithEncoding).mockResolvedValue(testContent); const result = await fileSystem.readTextFile('/test/file.txt'); - expect(fs.readFile).toHaveBeenCalledWith('/test/file.txt', 'utf-8'); + expect(fileUtils.readFileWithEncoding).toHaveBeenCalledWith( + '/test/file.txt', + ); expect(result).toBe(testContent); }); - it('should propagate fs.readFile errors', async () => { + it('should propagate readFileWithEncoding errors', async () => { const error = new Error('ENOENT: File not found'); - vi.mocked(fs.readFile).mockRejectedValue(error); + vi.mocked(fileUtils.readFileWithEncoding).mockRejectedValue(error); await expect(fileSystem.readTextFile('/test/file.txt')).rejects.toThrow( 'ENOENT: File not found', diff --git a/packages/core/src/services/fileSystemService.ts b/packages/core/src/services/fileSystemService.ts index 946c227ab6a..eb3a107d00d 100644 --- a/packages/core/src/services/fileSystemService.ts +++ b/packages/core/src/services/fileSystemService.ts @@ -5,6 +5,9 @@ */ import fs from 'node:fs/promises'; +import * as path from 'node:path'; +import { globSync } from 'glob'; +import { readFileWithEncoding } from '../utils/fileUtils.js'; /** * Interface for file system operations that may be delegated to different implementations @@ -25,6 +28,15 @@ export interface FileSystemService { * @param content - The content to write */ writeTextFile(filePath: string, content: string): Promise; + + /** + * Finds files with a given name within specified search paths. + * + * @param fileName - The name of the file to find. + * @param searchPaths - An array of directory paths to search within. + * @returns An array of absolute paths to the found files. + */ + findFiles(fileName: string, searchPaths: readonly string[]): string[]; } /** @@ -32,10 +44,21 @@ export interface FileSystemService { */ export class StandardFileSystemService implements FileSystemService { async readTextFile(filePath: string): Promise { - return fs.readFile(filePath, 'utf-8'); + // Use BOM-aware reader to handle UTF-8/16/32 encodings + return readFileWithEncoding(filePath); } async writeTextFile(filePath: string, content: string): Promise { await fs.writeFile(filePath, content, 'utf-8'); } + + findFiles(fileName: string, searchPaths: readonly string[]): string[] { + return searchPaths.flatMap((searchPath) => { + const pattern = path.posix.join(searchPath, '**', fileName); + return globSync(pattern, { + nodir: true, + absolute: true, + }); + }); + } } diff --git a/packages/core/src/utils/fileUtils.ts b/packages/core/src/utils/fileUtils.ts index 5525f98d06f..9e349689b1f 100644 --- a/packages/core/src/utils/fileUtils.ts +++ b/packages/core/src/utils/fileUtils.ts @@ -416,8 +416,8 @@ export async function processSingleFileContent( }; } case 'text': { - // Use BOM-aware reader to avoid leaving a BOM character in content and to support UTF-16/32 transparently - const content = await readFileWithEncoding(filePath); + // Delegate to fileSystemService for ACP client-side reading and BOM handling + const content = await fileSystemService.readTextFile(filePath); const lines = content.split('\n'); const originalLineCount = lines.length;