Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

216 changes: 216 additions & 0 deletions packages/cli/src/acp-integration/session/Session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ describe('Session', () => {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue([]),
getHistoryShallow: vi.fn().mockReturnValue([]),
getHistoryFunctionResponseIds: vi.fn().mockReturnValue(new Set<string>()),
getLastModelMessageText: vi.fn().mockReturnValue(''),
setHistory: vi.fn(),
truncateHistory: vi.fn(),
Expand Down Expand Up @@ -7385,6 +7386,221 @@ describe('Session', () => {
expect(mockChatRecordingService.recordToolResult).toHaveBeenCalledOnce();
});

it('suppresses duplicate provider functionCall ids already answered in history', async () => {
const execute = vi.fn().mockResolvedValue({
llmContent: 'should not run',
returnDisplay: 'should not run',
});
const build = vi.fn().mockReturnValue({
params: { file_path: 'b.ts' },
execute,
getDefaultPermission: vi.fn().mockResolvedValue('allow'),
getDescription: vi.fn().mockReturnValue('Read file'),
toolLocations: vi.fn().mockReturnValue([]),
});
mockToolRegistry.getTool.mockReturnValue({
name: 'read_file',
kind: core.Kind.Read,
displayName: 'Read File',
description: 'Read file',
build,
canUpdateOutput: false,
isOutputMarkdown: true,
});
vi.mocked(mockChat.getHistoryFunctionResponseIds).mockReturnValue(
new Set(['shell_1']),
);
const [duplicatePart] = core.normalizeModelToolCallIds(
[
{
functionCall: {
id: 'shell_1',
name: 'read_file',
args: { file_path: 'b.ts' },
},
},
],
new Set(['shell_1']),
new Set<string>(),
);
const duplicateCall = duplicatePart.functionCall!;

const result = await (
session as unknown as ToolCallInternals
).runToolCalls(new AbortController().signal, 'prompt-history-dup', [
duplicateCall,
]);

expect(mockToolRegistry.getTool).not.toHaveBeenCalled();
expect(build).not.toHaveBeenCalled();
expect(execute).not.toHaveBeenCalled();
const { parts } = result;
expect(parts).toHaveLength(1);
expect(result.stopAfterUserQuestionCancel).toBe(false);
expect(parts[0].functionResponse?.id).toBe('shell_1__qwen_dup_2');
expect(parts[0].functionResponse?.response).toEqual({
error: expect.stringContaining(
'Duplicate provider tool call id "shell_1"',
),
});
expect(mockChatRecordingService.recordToolResult).toHaveBeenCalledWith(
parts,
expect.objectContaining({
callId: 'shell_1__qwen_dup_2',
status: 'error',
resultDisplay: expect.stringContaining(
'Duplicate provider tool call id "shell_1"',
),
error: expect.any(Error),
}),
);
expect(mockClient.sessionUpdate).toHaveBeenCalledWith(
expect.objectContaining({
update: expect.objectContaining({
sessionUpdate: 'tool_call_update',
toolCallId: 'shell_1__qwen_dup_2',
status: 'failed',
}),
}),
);
expect(mockClient.sessionUpdate).not.toHaveBeenCalledWith(
expect.objectContaining({
update: expect.objectContaining({
sessionUpdate: 'tool_call',
toolCallId: 'shell_1__qwen_dup_2',
}),
}),
);
});

it('suppresses duplicate TodoWrite calls without emitting plan updates', async () => {
vi.mocked(mockChat.getHistoryFunctionResponseIds).mockReturnValue(
new Set(['todo_1']),
);
const [duplicatePart] = core.normalizeModelToolCallIds(
[
{
functionCall: {
id: 'todo_1',
name: core.ToolNames.TODO_WRITE,
args: {
todos: [
{
id: 'task-1',
content: 'Do not replay this',
status: 'pending',
},
],
},
},
},
],
new Set(['todo_1']),
new Set<string>(),
);

const result = await (
session as unknown as ToolCallInternals
).runToolCalls(new AbortController().signal, 'prompt-todo-dup', [
duplicatePart.functionCall!,
]);

expect(mockToolRegistry.getTool).not.toHaveBeenCalled();
const { parts } = result;
expect(result.stopAfterUserQuestionCancel).toBe(false);
expect(parts[0].functionResponse?.id).toBe('todo_1__qwen_dup_2');
expect(parts[0].functionResponse?.response).toEqual({
error: expect.stringContaining(
'Duplicate provider tool call id "todo_1"',
),
});
expect(mockClient.sessionUpdate).toHaveBeenCalledWith(
expect.objectContaining({
update: expect.objectContaining({
sessionUpdate: 'tool_call_update',
toolCallId: 'todo_1__qwen_dup_2',
status: 'failed',
}),
}),
);
expect(mockClient.sessionUpdate).not.toHaveBeenCalledWith(
expect.objectContaining({
update: expect.objectContaining({
sessionUpdate: 'plan',
}),
}),
);
expect(mockChatRecordingService.recordToolResult).toHaveBeenCalledWith(
parts,
expect.objectContaining({
callId: 'todo_1__qwen_dup_2',
status: 'error',
}),
);
});

it('keeps duplicate synthetic responses ordered with executable calls', async () => {
const execute = vi.fn(async () => ({
llmContent: 'ran',
returnDisplay: 'ran',
}));
mockToolRegistry.getTool.mockReturnValue({
name: 'read_file',
kind: core.Kind.Read,
displayName: 'Read File',
description: 'Read file',
build: vi.fn().mockReturnValue({
params: { file_path: 'x.ts' },
execute,
getDefaultPermission: vi.fn().mockResolvedValue('allow'),
getDescription: vi.fn().mockReturnValue('Read file'),
toolLocations: vi.fn().mockReturnValue([]),
}),
canUpdateOutput: false,
isOutputMarkdown: true,
});
const historyIds = new Set(['dup_mid']);
vi.mocked(mockChat.getHistoryFunctionResponseIds).mockReturnValue(
historyIds,
);
const [duplicatePart] = core.normalizeModelToolCallIds(
[
{
functionCall: {
id: 'dup_mid',
name: 'read_file',
args: { file_path: 'b.ts' },
},
},
],
new Set(['dup_mid']),
new Set<string>(),
);

const result = await (
session as unknown as ToolCallInternals
).runToolCalls(new AbortController().signal, 'prompt-mixed-dup', [
{ id: 'call_a', name: 'read_file', args: { file_path: 'a.ts' } },
duplicatePart.functionCall!,
{ id: 'call_c', name: 'read_file', args: { file_path: 'c.ts' } },
]);

expect(execute).toHaveBeenCalledTimes(2);
const { parts } = result;
expect(result.stopAfterUserQuestionCancel).toBe(false);
expect(parts.map((part) => part.functionResponse?.id)).toEqual([
'call_a',
'dup_mid__qwen_dup_2',
'call_c',
]);
expect(parts[1].functionResponse?.response).toEqual({
error: expect.stringContaining(
'Duplicate provider tool call id "dup_mid"',
),
});
expect(historyIds).toEqual(new Set(['dup_mid']));
});

it('does not dedupe function calls with empty ids in one batch', async () => {
const execute = vi.fn().mockResolvedValue({
llmContent: 'result',
Expand Down
101 changes: 98 additions & 3 deletions packages/cli/src/acp-integration/session/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@ import type {
AutoModeDecision,
AutoModeOutcome,
GoalTerminalEvent,
ToolCallRequestInfo,
ToolCallResponseInfo,
} from '@qwen-code/qwen-code-core';
import {
AuthType,
ApprovalMode,
CompressionStatus,
convertToFunctionResponse,
createDuplicateProviderToolCallResponse,
createDebugLogger,
DiscoveredMCPTool,
StreamEventType,
Expand Down Expand Up @@ -95,6 +98,7 @@ import {
setGoalTerminalObserver,
sessionIdContext,
dedupeToolCallsById,
getProviderToolCallId,
} from '@qwen-code/qwen-code-core';
import { NOT_CURRENTLY_GENERATING_CANCEL_MESSAGE } from '@qwen-code/acp-bridge/bridgeErrors';
// Single source of truth shared with the daemon-side answerer (BridgeClient),
Expand Down Expand Up @@ -3189,15 +3193,101 @@ export class Session implements SessionContext {
functionCalls: FunctionCall[],
): Promise<RunToolResult> {
const dedupedFunctionCalls = dedupeToolCallsById(functionCalls);
type Batch = { concurrent: boolean; calls: FunctionCall[] };
type ExecutableBatch = {
kind: 'execute';
concurrent: boolean;
calls: FunctionCall[];
};
type DuplicateBatch = {
kind: 'duplicate';
request: ToolCallRequestInfo;
response: ToolCallResponseInfo;
};
type Batch = ExecutableBatch | DuplicateBatch;
const batches: Batch[] = [];
const handledProviderToolCallIds = new Set(
this.#getCurrentChat().getHistoryFunctionResponseIds(),
);

const pushDuplicateBatch = (request: ToolCallRequestInfo): void => {
const response = createDuplicateProviderToolCallResponse(request);
debugLogger.debug(
`[Session.runToolCalls] Suppressing duplicate provider tool-call id: ` +
`${request.providerCallId} (tool: ${request.name})`,
);
batches.push({ kind: 'duplicate', request, response });
};

const emitDuplicateBatch = async (batch: DuplicateBatch): Promise<void> => {
const { request, response } = batch;
if (request.name === ToolNames.TODO_WRITE) {
const provenance = ToolCallEmitter.resolveToolProvenance(request.name);
await this.sendUpdate({
sessionUpdate: 'tool_call_update',
toolCallId: response.callId,
status: 'failed',
content: [
{
type: 'content',
content: {
type: 'text',
text: response.error?.message ?? String(response.resultDisplay),
},
},
],
rawOutput: response.resultDisplay,
_meta: {
toolName: request.name,
provenance: provenance.provenance,
...(provenance.serverId ? { serverId: provenance.serverId } : {}),
},
});
} else {
await this.toolCallEmitter.emitResult({
callId: response.callId,
toolName: request.name,
args: request.args,
message: response.responseParts,
resultDisplay: response.resultDisplay,
error: response.error,
success: false,
});
}
this.config
.getChatRecordingService()
?.recordToolResult(response.responseParts, {
callId: response.callId,
status: 'error',
resultDisplay: response.resultDisplay,
error: response.error,
errorType: response.errorType,
});
};

for (const fc of dedupedFunctionCalls) {
const providerCallId = getProviderToolCallId(fc) ?? fc.id;
if (providerCallId) {
if (handledProviderToolCallIds.has(providerCallId)) {
const callId = fc.id ?? `${fc.name}-${Date.now()}`;
pushDuplicateBatch({
callId,
providerCallId,
name: fc.name ?? 'unknown_tool',
args: (fc.args ?? {}) as Record<string, unknown>,
isClientInitiated: false,
prompt_id: promptId,
});
continue;
}
handledProviderToolCallIds.add(providerCallId);
}

const isAgent = fc.name === ToolNames.AGENT;
const last = batches[batches.length - 1];
if (isAgent && last?.concurrent) {
if (isAgent && last?.kind === 'execute' && last.concurrent) {
last.calls.push(fc);
} else {
batches.push({ concurrent: isAgent, calls: [fc] });
batches.push({ kind: 'execute', concurrent: isAgent, calls: [fc] });
}
}

Expand Down Expand Up @@ -3291,6 +3381,11 @@ export class Session implements SessionContext {

const parts: Part[] = [];
for (const batch of batches) {
if (batch.kind === 'duplicate') {
await emitDuplicateBatch(batch);
parts.push(...batch.response.responseParts);
continue;
}
if (batch.concurrent && batch.calls.length > 1) {
const batchAbortController = new AbortController();
let batchStopAfterUserQuestionCancel = false;
Expand Down
Loading
Loading