fix: use full paths for ACP diff payloads (#19539)

Signed-off-by: Jagjeevan Kashid <jagjeevandev97@gmail.com>
This commit is contained in:
Jagjeevan Kashid
2026-02-28 21:24:44 +05:30
committed by GitHub
parent 76f70d65ff
commit fae0639ba2
2 changed files with 131 additions and 2 deletions

View File

@@ -625,6 +625,133 @@ describe('Session', () => {
);
});
it('should use filePath for ACP diff content in permission request', async () => {
const confirmationDetails = {
type: 'edit',
title: 'Confirm Write: test.txt',
fileName: 'test.txt',
filePath: '/tmp/test.txt',
originalContent: 'old',
newContent: 'new',
onConfirm: vi.fn(),
};
mockTool.build.mockReturnValue({
getDescription: () => 'Test Tool',
toolLocations: () => [],
shouldConfirmExecute: vi.fn().mockResolvedValue(confirmationDetails),
execute: vi.fn().mockResolvedValue({ llmContent: 'Tool Result' }),
});
mockConnection.requestPermission.mockResolvedValue({
outcome: {
outcome: 'selected',
optionId: ToolConfirmationOutcome.ProceedOnce,
},
});
const stream1 = createMockStream([
{
type: StreamEventType.CHUNK,
value: {
functionCalls: [{ name: 'test_tool', args: {} }],
},
},
]);
const stream2 = createMockStream([
{
type: StreamEventType.CHUNK,
value: { candidates: [] },
},
]);
mockChat.sendMessageStream
.mockResolvedValueOnce(stream1)
.mockResolvedValueOnce(stream2);
await session.prompt({
sessionId: 'session-1',
prompt: [{ type: 'text', text: 'Call tool' }],
});
expect(mockConnection.requestPermission).toHaveBeenCalledWith(
expect.objectContaining({
toolCall: expect.objectContaining({
content: expect.arrayContaining([
expect.objectContaining({
type: 'diff',
path: '/tmp/test.txt',
oldText: 'old',
newText: 'new',
}),
]),
}),
}),
);
});
it('should use filePath for ACP diff content in tool result', async () => {
mockTool.build.mockReturnValue({
getDescription: () => 'Test Tool',
toolLocations: () => [],
shouldConfirmExecute: vi.fn().mockResolvedValue(null),
execute: vi.fn().mockResolvedValue({
llmContent: 'Tool Result',
returnDisplay: {
fileName: 'test.txt',
filePath: '/tmp/test.txt',
originalContent: 'old',
newContent: 'new',
},
}),
});
const stream1 = createMockStream([
{
type: StreamEventType.CHUNK,
value: {
functionCalls: [{ name: 'test_tool', args: {} }],
},
},
]);
const stream2 = createMockStream([
{
type: StreamEventType.CHUNK,
value: { candidates: [] },
},
]);
mockChat.sendMessageStream
.mockResolvedValueOnce(stream1)
.mockResolvedValueOnce(stream2);
await session.prompt({
sessionId: 'session-1',
prompt: [{ type: 'text', text: 'Call tool' }],
});
const updateCalls = mockConnection.sessionUpdate.mock.calls.map(
(call) => call[0],
);
const toolCallUpdate = updateCalls.find(
(call) => call.update?.sessionUpdate === 'tool_call_update',
);
expect(toolCallUpdate).toEqual(
expect.objectContaining({
update: expect.objectContaining({
content: expect.arrayContaining([
expect.objectContaining({
type: 'diff',
path: '/tmp/test.txt',
oldText: 'old',
newText: 'new',
}),
]),
}),
}),
);
});
it('should handle tool call cancellation by user', async () => {
const confirmationDetails = {
type: 'info',

View File

@@ -700,7 +700,7 @@ export class Session {
if (confirmationDetails.type === 'edit') {
content.push({
type: 'diff',
path: confirmationDetails.fileName,
path: confirmationDetails.filePath,
oldText: confirmationDetails.originalContent,
newText: confirmationDetails.newContent,
_meta: {
@@ -1228,7 +1228,9 @@ function toToolCallContent(toolResult: ToolResult): acp.ToolCallContent | null {
if ('fileName' in toolResult.returnDisplay) {
return {
type: 'diff',
path: toolResult.returnDisplay.fileName,
path:
toolResult.returnDisplay.filePath ??
toolResult.returnDisplay.fileName,
oldText: toolResult.returnDisplay.originalContent,
newText: toolResult.returnDisplay.newContent,
_meta: {