From c9d48026c418ddc5a6fe177a5f2e4d9169ef7ada Mon Sep 17 00:00:00 2001 From: Valery Teplyakov <31941254+Mervap@users.noreply.github.com> Date: Thu, 19 Mar 2026 01:02:07 +0100 Subject: [PATCH] fix(acp): provide more meta in tool_call_update (#22663) Co-authored-by: Mervap Co-authored-by: Sri Pasumarthi --- packages/cli/src/acp/acpClient.test.ts | 75 ++++++++++++++++++++++++++ packages/cli/src/acp/acpClient.ts | 8 +++ 2 files changed, 83 insertions(+) diff --git a/packages/cli/src/acp/acpClient.test.ts b/packages/cli/src/acp/acpClient.test.ts index 65b23247ef..abad9d374d 100644 --- a/packages/cli/src/acp/acpClient.test.ts +++ b/packages/cli/src/acp/acpClient.test.ts @@ -894,6 +894,9 @@ describe('Session', () => { update: expect.objectContaining({ sessionUpdate: 'tool_call_update', status: 'completed', + title: 'Test Tool', + locations: [], + kind: 'read', }), }), ); @@ -1306,6 +1309,18 @@ describe('Session', () => { expect(path.resolve).toHaveBeenCalled(); expect(fs.stat).toHaveBeenCalled(); + expect(mockConnection.sessionUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + update: expect.objectContaining({ + sessionUpdate: 'tool_call_update', + status: 'completed', + title: 'Read files', + locations: [], + kind: 'read', + }), + }), + ); + // Verify ReadManyFilesTool was used (implicitly by checking if sendMessageStream was called with resolved content) // Since we mocked ReadManyFilesTool to return specific content, we can check the args passed to sendMessageStream expect(mockChat.sendMessageStream).toHaveBeenCalledWith( @@ -1321,6 +1336,65 @@ describe('Session', () => { ); }); + it('should handle @path resolution error', async () => { + (path.resolve as unknown as Mock).mockReturnValue('/tmp/error.txt'); + (fs.stat as unknown as Mock).mockResolvedValue({ + isDirectory: () => false, + }); + (isWithinRoot as unknown as Mock).mockReturnValue(true); + + const MockReadManyFilesTool = ReadManyFilesTool as unknown as Mock; + MockReadManyFilesTool.mockImplementationOnce(() => ({ + name: 'read_many_files', + kind: 'read', + build: vi.fn().mockReturnValue({ + getDescription: () => 'Read files', + toolLocations: () => [], + execute: vi.fn().mockRejectedValue(new Error('File read failed')), + }), + })); + + const stream = createMockStream([ + { + type: StreamEventType.CHUNK, + value: { candidates: [] }, + }, + ]); + mockChat.sendMessageStream.mockResolvedValue(stream); + + await expect( + session.prompt({ + sessionId: 'session-1', + prompt: [ + { type: 'text', text: 'Read' }, + { + type: 'resource_link', + uri: 'file://error.txt', + mimeType: 'text/plain', + name: 'error.txt', + }, + ], + }), + ).rejects.toThrow('File read failed'); + + expect(mockConnection.sessionUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + update: expect.objectContaining({ + sessionUpdate: 'tool_call_update', + status: 'failed', + content: expect.arrayContaining([ + expect.objectContaining({ + content: expect.objectContaining({ + text: expect.stringMatching(/File read failed/), + }), + }), + ]), + kind: 'read', + }), + }), + ); + }); + it('should handle cancellation during prompt', async () => { let streamController: ReadableStreamDefaultController; const stream = new ReadableStream({ @@ -1434,6 +1508,7 @@ describe('Session', () => { content: expect.objectContaining({ text: 'Tool failed' }), }), ]), + kind: 'read', }), }), ); diff --git a/packages/cli/src/acp/acpClient.ts b/packages/cli/src/acp/acpClient.ts index 072d91c20a..44c0373515 100644 --- a/packages/cli/src/acp/acpClient.ts +++ b/packages/cli/src/acp/acpClient.ts @@ -966,7 +966,10 @@ export class Session { sessionUpdate: 'tool_call_update', toolCallId: callId, status: 'completed', + title: invocation.getDescription(), content: content ? [content] : [], + locations: invocation.toolLocations(), + kind: toAcpToolKind(tool.kind), }); const durationMs = Date.now() - startTime; @@ -1030,6 +1033,7 @@ export class Session { content: [ { type: 'content', content: { type: 'text', text: error.message } }, ], + kind: toAcpToolKind(tool.kind), }); this.chat.recordCompletedToolCalls(this.config.getActiveModel(), [ @@ -1324,7 +1328,10 @@ export class Session { sessionUpdate: 'tool_call_update', toolCallId: callId, status: 'completed', + title: invocation.getDescription(), content: content ? [content] : [], + locations: invocation.toolLocations(), + kind: toAcpToolKind(readManyFilesTool.kind), }); if (Array.isArray(result.llmContent)) { const fileContentRegex = /^--- (.*?) ---\n\n([\s\S]*?)\n\n$/; @@ -1368,6 +1375,7 @@ export class Session { }, }, ], + kind: toAcpToolKind(readManyFilesTool.kind), }); throw error;