From 3de2a6561132bda44b35959b7c5f4638ca49d9de Mon Sep 17 00:00:00 2001 From: vastsa Date: Mon, 21 Sep 2026 14:40:43 +0800 Subject: [PATCH] fix(agent-runtime): dedupe tool calls in subagents Use one shared request-boundary filter for the parent runtime and Task subagents. This keeps resumed delegate history valid when append-only snapshots repeat a tool-call id, while preserving the existing no-duplicate fast path and diagnostic log. --- packages/agent-runtime/src/runtime.ts | 62 ++----------- packages/agent-runtime/src/subagent.test.ts | 54 +++++++++++ packages/agent-runtime/src/subagent.ts | 12 ++- .../agent-runtime/src/tool-call-dedupe.ts | 92 +++++++++++++++++++ 4 files changed, 164 insertions(+), 56 deletions(-) create mode 100644 packages/agent-runtime/src/tool-call-dedupe.ts diff --git a/packages/agent-runtime/src/runtime.ts b/packages/agent-runtime/src/runtime.ts index 269b4fc208..894c431d64 100644 --- a/packages/agent-runtime/src/runtime.ts +++ b/packages/agent-runtime/src/runtime.ts @@ -122,6 +122,10 @@ import { usageToPi, } from "./agent-messages.js"; import { buildSessionContext } from "./session-context.js"; +import { + dedupeToolCallMessages, + reportDuplicateToolCallDrop, +} from "./tool-call-dedupe.js"; import { apiBindingForProviderModel, buildProviderModel, @@ -602,8 +606,6 @@ const AGENT_CORE_TOOL_NAMES = new Set([ const MAX_ON_DEMAND_TOOL_PROMPT_ENTRIES = 64; const MAX_TOOL_SEARCH_RESULT_NAMES = 24; -/** Ids named in the one log line a duplicate-collision rebuild writes. */ -const MAX_LOGGED_DUPLICATE_TOOL_CALLS = 8; /** Tools that ask the host to switch this session into a contract mode (D198). */ const ENTER_TOOL_NAMES: Record = { @@ -2715,59 +2717,9 @@ Delegation rules: * sentence. */ private dropDuplicateToolCalls(messages: AgentMessage[]): AgentMessage[] { - const claimedCalls = new Set(); - const claimedResults = new Set(); - const droppedIds = new Set(); - let droppedCount = 0; - const note = (id: string) => { - droppedCount += 1; - if (droppedIds.size < MAX_LOGGED_DUPLICATE_TOOL_CALLS) droppedIds.add(id); - }; - const next: AgentMessage[] = []; - for (const message of messages) { - if (message.role === "assistant") { - const content = (message as AssistantMessage).content; - if (!Array.isArray(content)) { - next.push(message); - continue; - } - const kept = content.filter((block) => { - if (!isRecord(block) || block.type !== "toolCall") return true; - const id = typeof block.id === "string" ? block.id : undefined; - if (!id) return true; - if (claimedCalls.has(id)) { - note(id); - return false; - } - claimedCalls.add(id); - return true; - }); - next.push( - kept.length === content.length - ? message - : { ...(message as AssistantMessage), content: kept }, - ); - continue; - } - if (message.role === "toolResult") { - const id = (message as { toolCallId?: unknown }).toolCallId; - if (typeof id === "string" && id) { - if (claimedResults.has(id)) { - note(id); - continue; - } - claimedResults.add(id); - } - } - next.push(message); - } - if (droppedCount === 0) return messages; - process.stderr.write( - `[agent-runtime] dropped ${droppedCount} duplicate tool call ${ - droppedCount === 1 ? "entry" : "entries" - } before the request (session=${this.sessionId} ids=${[...droppedIds].join(",")})\n`, - ); - return next; + const drop = dedupeToolCallMessages(messages); + reportDuplicateToolCallDrop(this.sessionId, drop); + return drop.messages; } private entriesWithCompaction( diff --git a/packages/agent-runtime/src/subagent.test.ts b/packages/agent-runtime/src/subagent.test.ts index 90b28518d0..e4aca6d57c 100644 --- a/packages/agent-runtime/src/subagent.test.ts +++ b/packages/agent-runtime/src/subagent.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import type { AgentEventEnvelope, SubagentDefinition } from "@pi-desktop/shared"; +import type { Message } from "@earendil-works/pi-ai"; import { composeSubagentSystemPrompt, MAX_SUBAGENT_REPORT_CHARS, @@ -136,6 +137,59 @@ describe("SubagentRun event forwarding", () => { "models.stream(omitThinkingModel, context, retryOptions)", ); }); + it("deduplicates repeated tool calls before a subagent request", () => { + const stderr = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + try { + const { run } = createRun({ + initialMessages: [ + assistantMessage({ + content: [ + { type: "toolCall", id: "child-read", name: "Read", arguments: {} }, + ], + }), + { + role: "toolResult", + toolCallId: "child-read", + toolName: "Read", + content: [{ type: "text", text: "first" }], + isError: false, + timestamp: 2, + }, + assistantMessage({ + content: [ + { type: "toolCall", id: "child-read", name: "Read", arguments: {} }, + ], + }), + { + role: "toolResult", + toolCallId: "child-read", + toolName: "Read", + content: [{ type: "text", text: "retry" }], + isError: false, + timestamp: 3, + }, + ] as unknown as NonNullable, + }); + const outgoing = run.agent.convertToLlm(run.agent.state.messages) as Message[]; + const toolCalls = outgoing.flatMap((message) => + message.role === "assistant" + ? message.content.filter((block) => block.type === "toolCall") + : [], + ); + + expect(toolCalls.map((call) => call.id)).toEqual(["child-read"]); + expect(outgoing.filter((message) => message.role === "toolResult")).toHaveLength(1); + expect( + outgoing.filter( + (message) => message.role === "assistant" && message.content.length === 0, + ), + ).toHaveLength(0); + expect(stderr.mock.calls.map(([chunk]) => String(chunk)).join("")) + .toContain("session=session-1"); + } finally { + stderr.mockRestore(); + } + }); it("does not synthesize a Responses reasoning setting when omitted", async () => { const responseProvider: RuntimeProviderConfig = { diff --git a/packages/agent-runtime/src/subagent.ts b/packages/agent-runtime/src/subagent.ts index 323a316fdc..8cf0183979 100644 --- a/packages/agent-runtime/src/subagent.ts +++ b/packages/agent-runtime/src/subagent.ts @@ -62,6 +62,10 @@ import { prepareDelegateTurnContext, subagentContextOverflowError, } from "./subagent-context.js"; +import { + dedupeToolCallMessages, + reportDuplicateToolCallDrop, +} from "./tool-call-dedupe.js"; import { classifyProviderError, delayWithAbort, @@ -242,7 +246,8 @@ export class SubagentRun { this.agent = new Agent({ streamFn: binding.streamFn, getApiKey: binding.getApiKey, - convertToLlm, + convertToLlm: (messages) => + convertToLlm(this.dedupeToolCalls(messages)), // The same turn-boundary context protection the session has (ADR 0299): // re-estimate at each boundary, compact before the next request, degrade // before failing. The budget derives from this run's resolved model. @@ -316,6 +321,11 @@ export class SubagentRun { private modelBinding() { return this.bindingFor(this.provider, this.thinkingLevel); } + private dedupeToolCalls(messages: AgentMessage[]): AgentMessage[] { + const drop = dedupeToolCallMessages(messages); + reportDuplicateToolCallDrop(this.opts.sessionId, drop); + return drop.messages; + } private bindingFor( provider: RuntimeProviderConfig, diff --git a/packages/agent-runtime/src/tool-call-dedupe.ts b/packages/agent-runtime/src/tool-call-dedupe.ts new file mode 100644 index 0000000000..7dc35e825c --- /dev/null +++ b/packages/agent-runtime/src/tool-call-dedupe.ts @@ -0,0 +1,92 @@ +import type { AgentMessage } from "@earendil-works/pi-agent-core"; + +type RecordLike = Record; + +const MAX_LOGGED_DUPLICATE_TOOL_CALLS = 8; + +export type DuplicateToolCallDrop = { + messages: AgentMessage[]; + droppedCount: number; + droppedIds: string[]; +}; + +function isRecord(value: unknown): value is RecordLike { + return typeof value === "object" && value !== null; +} + +/** + * Keep the first call and result for every tool-call id in a provider request. + * + * The transcript can contain repeated snapshots of one call. Returning the + * original array when nothing changes preserves the common path's identity and + * avoids copying messages that are already valid. + */ +export function dedupeToolCallMessages( + messages: AgentMessage[], +): DuplicateToolCallDrop { + const claimedCalls = new Set(); + const claimedResults = new Set(); + const droppedIds = new Set(); + let droppedCount = 0; + const note = (id: string) => { + droppedCount += 1; + if (droppedIds.size < MAX_LOGGED_DUPLICATE_TOOL_CALLS) { + droppedIds.add(id); + } + }; + const next: AgentMessage[] = []; + + for (const message of messages) { + if (message.role === "assistant") { + const content = (message as { content?: unknown }).content; + if (!Array.isArray(content)) { + next.push(message); + continue; + } + const kept = content.filter((block) => { + if (!isRecord(block) || block.type !== "toolCall") return true; + const id = typeof block.id === "string" ? block.id : undefined; + if (id === undefined) return true; + if (claimedCalls.has(id)) { + note(id); + return false; + } + claimedCalls.add(id); + return true; + }); + if (kept.length === 0 && content.length > 0) continue; + next.push(kept.length === content.length ? message : ({ ...message, content: kept } as AgentMessage)); + continue; + } + + if (message.role === "toolResult") { + const id = (message as { toolCallId?: unknown }).toolCallId; + if (typeof id === "string") { + if (claimedResults.has(id)) { + note(id); + continue; + } + claimedResults.add(id); + } + } + next.push(message); + } + + return { + messages: droppedCount === 0 ? messages : next, + droppedCount, + droppedIds: [...droppedIds], + }; +} + +export function reportDuplicateToolCallDrop( + sessionId: string, + drop: DuplicateToolCallDrop, +): void { + if (drop.droppedCount === 0) return; + process.stderr.write( + `[agent-runtime] dropped ${drop.droppedCount} duplicate tool call ${ + drop.droppedCount === 1 ? "entry" : "entries" + } before the request (session=${sessionId} ids=${drop.droppedIds.join(",")})\n`, + ); +}