From 61b5d85d197f0f1fb3b6b88867694be81f90dd22 Mon Sep 17 00:00:00 2001 From: March-7 <155554077+March-7@users.noreply.github.com> Date: Fri, 2 Oct 2026 22:13:42 +0800 Subject: [PATCH] fix: preserve tool results when call IDs are reused --- README.md | 2 + lib/commands/sweep.ts | 4 +- lib/compress/protected-content.ts | 6 +- lib/messages/inject/subagent-results.ts | 6 +- lib/messages/prune.ts | 3 + lib/state/tool-cache.ts | 9 +- lib/strategies/deduplication.ts | 6 +- lib/strategies/purge-errors.ts | 6 +- lib/tool-ids.ts | 21 ++ lib/v2/messages.ts | 21 +- tests/tool-id-collisions.test.ts | 337 ++++++++++++++++++++++++ 11 files changed, 406 insertions(+), 15 deletions(-) create mode 100644 lib/tool-ids.ts create mode 100644 tests/tool-id-collisions.test.ts diff --git a/README.md b/README.md index d5b3ec87..b4ddc39e 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,8 @@ In `range` mode, when a new compression overlaps an earlier one, the earlier sum Identifies repeated tool calls (same tool, same arguments) and keeps only the most recent output. Recalculated when the compress tool runs, so prompt cache is only impacted alongside compression. +When a provider reuses a tool call ID, DCP retains those calls instead of automatically pruning or sweeping an ambiguous ID. Unique IDs keep the normal pruning behavior. Original provider call/result IDs are preserved. + ### Purge Errors Prunes inputs from errored tool calls after a configurable number of turns. Error messages are preserved; only the potentially large input content is removed. Recalculated on compress tool use. diff --git a/lib/commands/sweep.ts b/lib/commands/sweep.ts index 15c4a5c8..b4c90352 100644 --- a/lib/commands/sweep.ts +++ b/lib/commands/sweep.ts @@ -19,6 +19,7 @@ import { saveSessionState } from "../state/persistence" import { isMessageCompacted } from "../state/utils" import { isToolProtected } from "../protected-patterns" import { syncToolCache } from "../state/tool-cache" +import { getAmbiguousToolIds } from "../tool-ids" export interface SweepCommandContext { client: any @@ -164,8 +165,9 @@ export async function handleSweepCommand(ctx: SweepCommandContext): Promise { - if (state.prune.tools.has(id)) { + if (state.prune.tools.has(id) || ambiguousIds.has(id)) { return false } const entry = state.toolParameters.get(id) diff --git a/lib/compress/protected-content.ts b/lib/compress/protected-content.ts index c6d3e7ae..103c205c 100644 --- a/lib/compress/protected-content.ts +++ b/lib/compress/protected-content.ts @@ -8,6 +8,7 @@ import { } from "../subagents/subagent-results" import { fetchSessionMessages } from "./search" import type { SearchContext, SelectionResolution } from "./types" +import { toolOccurrenceKey } from "../tool-ids" export function appendProtectedUserMessages( summary: string, @@ -152,7 +153,8 @@ export async function appendProtectedTools( part.state?.status === "completed" && typeof part.state?.output === "string" ) { - const cachedSubAgentResult = state.subAgentResultCache.get(part.callID) + const cacheKey = toolOccurrenceKey(message, part) + const cachedSubAgentResult = state.subAgentResultCache.get(cacheKey) if (cachedSubAgentResult !== undefined) { if (cachedSubAgentResult) { @@ -176,7 +178,7 @@ export async function appendProtectedTools( } if (subAgentResultText) { - state.subAgentResultCache.set(part.callID, subAgentResultText) + state.subAgentResultCache.set(cacheKey, subAgentResultText) output = mergeSubagentResult( part.state.output, subAgentResultText, diff --git a/lib/messages/inject/subagent-results.ts b/lib/messages/inject/subagent-results.ts index 967121f8..f7ab70d5 100644 --- a/lib/messages/inject/subagent-results.ts +++ b/lib/messages/inject/subagent-results.ts @@ -7,6 +7,7 @@ import { mergeSubagentResult, } from "../../subagents/subagent-results" import { stripHallucinationsFromString } from "../utils" +import { toolOccurrenceKey } from "../../tool-ids" async function fetchSubAgentMessages(client: any, sessionId: string): Promise { const response = await client.session.messages({ @@ -41,7 +42,8 @@ export const injectExtendedSubAgentResults = async ( continue } - const cachedResult = state.subAgentResultCache.get(part.callID) + const cacheKey = toolOccurrenceKey(message, part) + const cachedResult = state.subAgentResultCache.get(cacheKey) if (cachedResult !== undefined) { if (cachedResult) { part.state.output = stripHallucinationsFromString( @@ -74,7 +76,7 @@ export const injectExtendedSubAgentResults = async ( continue } - state.subAgentResultCache.set(part.callID, subAgentResultText) + state.subAgentResultCache.set(cacheKey, subAgentResultText) part.state.output = stripHallucinationsFromString( mergeSubagentResult(part.state.output, subAgentResultText), state.idFormat, diff --git a/lib/messages/prune.ts b/lib/messages/prune.ts index dc394524..728a6acc 100644 --- a/lib/messages/prune.ts +++ b/lib/messages/prune.ts @@ -6,6 +6,7 @@ import { createSyntheticUserMessage, replaceBlockIdsWithBlocked } from "./utils" import { getLastUserMessage } from "./query" import type { UserMessage } from "@opencode-ai/sdk/v2" import { formatBlockRef } from "../message-ids" +import { getAmbiguousToolIds } from "../tool-ids" const PRUNED_TOOL_OUTPUT_REPLACEMENT = "[Output removed to save context - information superseded or no longer needed]" @@ -19,6 +20,8 @@ export const prune = ( messages: WithParts[], summaryBase?: WithParts, ): void => { + // Legacy persisted call IDs must not remove multiple tool occurrences. + for (const id of getAmbiguousToolIds(messages)) state.prune.tools.delete(id) filterCompressedRanges(state, logger, config, messages, summaryBase) // pruneFullTool(state, logger, messages) pruneToolOutputs(state, logger, messages) diff --git a/lib/state/tool-cache.ts b/lib/state/tool-cache.ts index aab67f1c..d9c8fbd3 100644 --- a/lib/state/tool-cache.ts +++ b/lib/state/tool-cache.ts @@ -3,6 +3,7 @@ import type { Logger } from "../logger" import { PluginConfig } from "../config" import { isMessageCompacted } from "./utils" import { countToolTokens } from "../token-utils" +import { getAmbiguousToolIds } from "../tool-ids" const MAX_TOOL_CACHE_SIZE = 1000 @@ -18,6 +19,12 @@ export function syncToolCache( try { logger.info("Syncing tool parameters from OpenCode messages") + const ambiguousIds = getAmbiguousToolIds(messages) + for (const id of ambiguousIds) { + state.toolParameters.delete(id) + state.prune.tools.delete(id) + } + let turnCounter = 0 for (const msg of messages) { @@ -32,7 +39,7 @@ export function syncToolCache( continue } - if (part.type !== "tool" || !part.callID) { + if (part.type !== "tool" || !part.callID || ambiguousIds.has(part.callID)) { continue } diff --git a/lib/strategies/deduplication.ts b/lib/strategies/deduplication.ts index 9cda5fbf..6ae03825 100644 --- a/lib/strategies/deduplication.ts +++ b/lib/strategies/deduplication.ts @@ -3,6 +3,7 @@ import { Logger } from "../logger" import type { SessionState, WithParts } from "../state" import { isToolProtected } from "../protected-patterns" import { getTotalToolTokens } from "../token-utils" +import { getAmbiguousToolIds } from "../tool-ids" /** * Deduplication strategy - prunes older tool calls that have identical @@ -29,7 +30,10 @@ export const deduplicate = ( } // Filter out IDs already pruned - const unprunedIds = allToolIds.filter((id) => !state.prune.tools.has(id)) + const ambiguousIds = getAmbiguousToolIds(messages) + const unprunedIds = allToolIds.filter( + (id) => !state.prune.tools.has(id) && !ambiguousIds.has(id), + ) if (unprunedIds.length === 0) { return diff --git a/lib/strategies/purge-errors.ts b/lib/strategies/purge-errors.ts index 40303616..63d20f3d 100644 --- a/lib/strategies/purge-errors.ts +++ b/lib/strategies/purge-errors.ts @@ -3,6 +3,7 @@ import { Logger } from "../logger" import type { SessionState, WithParts } from "../state" import { isToolProtected } from "../protected-patterns" import { getTotalToolTokens } from "../token-utils" +import { getAmbiguousToolIds } from "../tool-ids" /** * Purge Errors strategy - prunes tool inputs for tools that errored @@ -32,7 +33,10 @@ export const purgeErrors = ( } // Filter out IDs already pruned - const unprunedIds = allToolIds.filter((id) => !state.prune.tools.has(id)) + const ambiguousIds = getAmbiguousToolIds(messages) + const unprunedIds = allToolIds.filter( + (id) => !state.prune.tools.has(id) && !ambiguousIds.has(id), + ) if (unprunedIds.length === 0) { return diff --git a/lib/tool-ids.ts b/lib/tool-ids.ts new file mode 100644 index 00000000..7b3a86ce --- /dev/null +++ b/lib/tool-ids.ts @@ -0,0 +1,21 @@ +import type { WithParts } from "./state" + +type ToolPart = Extract + +export function getAmbiguousToolIds(messages: WithParts[]): Set { + const seen = new Set() + const ambiguous = new Set() + for (const message of messages) { + const parts = Array.isArray(message.parts) ? message.parts : [] + for (const part of parts) { + if (part.type !== "tool" || !part.callID) continue + if (seen.has(part.callID)) ambiguous.add(part.callID) + seen.add(part.callID) + } + } + return ambiguous +} + +export function toolOccurrenceKey(message: WithParts, part: ToolPart): string { + return JSON.stringify([message.info.id, part.id ?? part.callID]) +} diff --git a/lib/v2/messages.ts b/lib/v2/messages.ts index 62f0c737..c9d5a408 100644 --- a/lib/v2/messages.ts +++ b/lib/v2/messages.ts @@ -90,15 +90,22 @@ function output(content: ReadonlyArray<{ type: string; text?: string }>): unknow export function project(native: Message[], entries: History, session: Session) { const byID = new Map(entries.map((entry) => [entry.id, entry])) - const results = new Map() - const owners = new Map() + const calls = new Map() + const results = new Map() + const owners = new Map() const links = new Map() const originals = new Map() const messages: WithParts[] = [] for (const message of native) for (const part of message.content) { - if (part.type === "tool-result") results.set(part.id, part) - if (part.type === "tool-call" && message.id) owners.set(part.id, message.id) + if (part.type === "tool-call") calls.set(part.id, { part, messageId: message.id }) + if (part.type === "tool-result") { + const call = calls.get(part.id) + if (!call) continue + results.set(call.part, part) + if (call.messageId) owners.set(part, call.messageId) + calls.delete(part.id) + } } for (const message of native) { if (!message.id || !["user", "assistant"].includes(message.role)) continue @@ -121,7 +128,7 @@ export function project(native: Message[], entries: History, session: Session) { url: typeof part.data === "string" ? part.data : "", } if (part.type === "tool-call") { - const result = results.get(part.id) + const result = results.get(part) const entry = byID.get(message.id) const durable = entry?.type === "assistant" @@ -205,8 +212,8 @@ export function project(native: Message[], entries: History, session: Session) { if (part.type === "tool-call") content.push(...added.splice(0)) if ( part.type === "tool-result" && - originals.has(owners.get(part.id)!) && - !retained.has(owners.get(part.id)!) + originals.has(owners.get(part)!) && + !retained.has(owners.get(part)!) ) continue const edit = links.get(part) diff --git a/tests/tool-id-collisions.test.ts b/tests/tool-id-collisions.test.ts new file mode 100644 index 00000000..15cc8aad --- /dev/null +++ b/tests/tool-id-collisions.test.ts @@ -0,0 +1,337 @@ +import assert from "node:assert/strict" +import { mkdtempSync, rmSync } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" +import test, { after } from "node:test" +import type { PluginConfig } from "../lib/config" +import type { WithParts } from "../lib/state" + +const dataHome = mkdtempSync(join(tmpdir(), "dcp-tool-ids-")) +const previousDataHome = process.env.XDG_DATA_HOME +process.env.XDG_DATA_HOME = dataHome +const { createSessionState, syncToolCache, saveSessionState, ensureSessionInitialized } = + await import("../lib/state") +const { buildToolIdList } = await import("../lib/messages/utils") +const { deduplicate } = await import("../lib/strategies/deduplication") +const { purgeErrors } = await import("../lib/strategies/purge-errors") +const { prune } = await import("../lib/messages/prune") +const { injectExtendedSubAgentResults } = await import("../lib/messages/inject/subagent-results") +const { appendProtectedTools } = await import("../lib/compress/protected-content") +const { buildSearchContext, resolveSelection } = await import("../lib/compress/search") +const { handleSweepCommand } = await import("../lib/commands/sweep") +const { project } = await import("../lib/v2/messages") +const logger = { info() {}, warn() {}, error() {}, debug() {} } as any +const config = { + manualMode: { enabled: false, automaticStrategies: true }, + turnProtection: { enabled: false, turns: 1 }, + strategies: { + deduplication: { enabled: true, protectedTools: [] }, + purgeErrors: { enabled: true, turns: 1, protectedTools: [] }, + }, + commands: { enabled: true, protectedTools: [] }, + protectedFilePatterns: [], + compress: { mode: "range" }, +} as PluginConfig + +after(() => { + if (previousDataHome === undefined) delete process.env.XDG_DATA_HOME + else process.env.XDG_DATA_HOME = previousDataHome + rmSync(dataHome, { recursive: true, force: true }) +}) + +function toolMessage(id: string, callID: string, path = id): WithParts { + return { + info: { id, sessionID: "ses_collisions", role: "assistant", time: { created: 2 } }, + parts: [ + { type: "step-start", id: `step_${id}` }, + { + type: "tool", + id: `part_${id}`, + messageID: id, + sessionID: "ses_collisions", + tool: "read", + callID, + state: { + status: "completed", + input: { filePath: path }, + output: `contents:${path}`, + metadata: {}, + time: { start: 1, end: 2 }, + }, + }, + ], + } as WithParts +} +const tools = (messages: WithParts[]) => + messages.flatMap((m) => m.parts.filter((p) => p.type === "tool")) +const outputs = (messages: WithParts[]) => + tools(messages).map((p) => (p.state.status === "completed" ? p.state.output : undefined)) +function run(messages: WithParts[], state = createSessionState(), settings = config) { + syncToolCache(state, settings, logger, messages) + buildToolIdList(state, messages) + deduplicate(state, logger, settings, messages) + purgeErrors(state, logger, settings, messages) + prune(state, logger, settings, messages) + return state +} + +// #615: a reused provider ID cannot safely address one tool occurrence. +for (const sameInput of [false, true]) { + test(`reused IDs retain both results with ${sameInput ? "identical" : "different"} inputs`, () => { + const messages = [ + toolMessage("old", "read:0", "alpha"), + toolMessage("new", "read:0", sameInput ? "alpha" : "beta"), + ] + const expected = outputs(messages) + const state = run(messages) + assert.deepEqual(outputs(messages), expected) + assert.equal(state.prune.tools.size, 0) + assert.equal(state.stats.totalPruneTokens, 0) + }) +} + +test("two parts in one message with the same ID are retained", () => { + const message = toolMessage("old", "read:0", "alpha") + message.parts.push( + ...toolMessage("new", "read:0", "beta").parts.filter((p) => p.type === "tool"), + ) + const expected = outputs([message]) + run([message]) + assert.deepEqual(outputs([message]), expected) +}) + +test("unique IDs still remove only the older repeated read", () => { + const messages = [toolMessage("old", "first", "alpha"), toolMessage("new", "second", "alpha")] + const state = run(messages) + assert.deepEqual([...state.prune.tools.keys()], ["first"]) + assert.match(String(outputs(messages)[0]), /Output removed/) + assert.equal(outputs(messages)[1], "contents:alpha") +}) + +test("unique IDs with different inputs remain independent", () => { + const messages = [toolMessage("old", "first"), toolMessage("new", "second")] + const expected = outputs(messages) + run(messages) + assert.deepEqual(outputs(messages), expected) +}) + +test("a collision introduced after caching clears the old metadata and prune entry", () => { + const state = createSessionState() + const old = toolMessage("old", "read:0") + syncToolCache(state, config, logger, [old]) + state.prune.tools.set("read:0", 100) + const messages = [old, toolMessage("new", "read:0")] + const expected = outputs(messages) + run(messages, state) + assert.deepEqual(outputs(messages), expected) + assert.equal(state.toolParameters.has("read:0"), false) + assert.equal(state.prune.tools.has("read:0"), false) +}) + +test("compacted occurrences still make a retained call ID ambiguous", () => { + const state = createSessionState() + state.lastCompaction = 2 + const messages = [toolMessage("old", "read:0"), toolMessage("new", "read:0")] + messages[0]!.info.time.created = 1 + state.prune.tools.set("read:0", 100) + run(messages, state) + assert.equal(outputs(messages)[1], "contents:new") +}) + +test("errored calls with reused IDs retain their separate inputs", () => { + const state = createSessionState() + state.currentTurn = 10 + const messages = [toolMessage("old", "read:0"), toolMessage("new", "read:0")] + for (const part of tools(messages)) { + part.state = { + status: "error", + input: part.state.input, + error: "not found", + time: { start: 1, end: 2 }, + } + } + const expected = structuredClone(messages) + run(messages, state) + assert.deepEqual(messages, expected) +}) + +test("pruning restored legacy state never removes both colliding results", async () => { + const state = createSessionState() + state.sessionId = "ses_collisions" + state.prune.tools.set("read:0", 100) + await saveSessionState(state, logger) + const messages = [toolMessage("old", "read:0"), toolMessage("new", "read:0")] + const user = { + info: { id: "user", sessionID: "ses_collisions", role: "user", time: { created: 1 } }, + parts: [{ type: "text", text: "Continue" }], + } as WithParts + const restored = createSessionState() + await ensureSessionInitialized( + { session: { get: async () => ({ data: {} }) } }, + restored, + "ses_collisions", + logger, + [user, ...messages], + false, + ) + assert.equal(restored.prune.tools.has("read:0"), true, "legacy file actually loaded") + // The final pruning boundary must also be safe before a cache refresh. + prune(restored, logger, config, messages) + assert.deepEqual(outputs(messages), ["contents:old", "contents:new"]) +}) + +for (const args of [[], ["1"]]) { + test(`manual sweep retains ambiguous IDs (${args.length ? "last-n" : "since-user"})`, async () => { + const state = createSessionState() + const messages = [ + toolMessage("unique", "unique"), + toolMessage("old", "read:0"), + { + info: { + id: "user", + sessionID: "ses_collisions", + role: "user", + agent: "build", + model: { providerID: "local", modelID: "fixed" }, + time: { created: 1 }, + }, + parts: [{ type: "text", text: "Continue" }], + } as WithParts, + toolMessage("new", "read:0"), + ] + await handleSweepCommand({ + client: { session: { prompt: async () => ({}) } }, + state, + config, + logger, + sessionId: "ses_collisions", + messages, + args, + workingDirectory: dataHome, + }) + assert.equal(state.prune.tools.size, 0) + assert.equal(state.stats.totalPruneTokens, 0) + assert.deepEqual(outputs(messages), ["contents:unique", "contents:old", "contents:new"]) + }) +} + +function taskMessages() { + const messages = [toolMessage("old", "task:0"), toolMessage("new", "task:0")] + for (const [i, part] of tools(messages).entries()) { + part.tool = "task" + if (part.state.status === "completed") { + part.state.metadata = { sessionId: `child_${i}` } + part.state.output = "placeholder" + } + } + return messages +} +const childClient = { + session: { + messages: async ({ path }: any) => ({ + data: [ + { + info: { + id: path.id, + sessionID: path.id, + role: "assistant", + time: { created: 1 }, + }, + parts: [{ type: "text", text: `result:${path.id}` }], + }, + ], + }), + }, +} + +test("subagent caches keep reused IDs separate across messages", async () => { + const messages = taskMessages() + await injectExtendedSubAgentResults(childClient, createSessionState(), logger, messages, true) + assert.deepEqual(outputs(messages), [ + "result:child_0", + "result:child_1", + ]) +}) + +test("protected summaries keep both subagents' actual results", async () => { + const messages = taskMessages() + const state = createSessionState() + const context = buildSearchContext(state, messages) + const selection = resolveSelection( + context, + { kind: "message", rawIndex: 0, messageId: "old" }, + { kind: "message", rawIndex: 1, messageId: "new" }, + ) + const summary = await appendProtectedTools( + childClient, + state, + true, + "summary", + selection, + context, + ["task"], + ) + assert.match(summary, /result:child_0/) + assert.match(summary, /result:child_1/) +}) + +test("V2 projection pairs each reused ID with the following turn's result", () => { + const native = [ + { + id: "old", + role: "assistant", + content: [{ type: "tool-call", id: "read:0", name: "read", input: { path: "alpha" } }], + }, + { + role: "tool", + content: [ + { + type: "tool-result", + id: "read:0", + name: "read", + result: { type: "text", value: "alpha" }, + }, + ], + }, + { + id: "new", + role: "assistant", + content: [{ type: "tool-call", id: "read:0", name: "read", input: { path: "beta" } }], + }, + { + role: "tool", + content: [ + { + type: "tool-result", + id: "read:0", + name: "read", + result: { type: "text", value: "beta" }, + }, + ], + }, + ] as any + const entries = ["old", "new"].map((id) => ({ + id, + type: "assistant", + time: { created: 1 }, + model: {}, + content: [], + })) as any + const view = project(native, entries, { + id: "ses_collisions", + model: {}, + agent: "build", + } as any) + assert.deepEqual(outputs(view.messages), ["alpha", "beta"]) + run(view.messages) + assert.deepEqual(view.restore(), native) + // Compressing the old message must remove only its paired result. + const state = createSessionState() + state.prune.messages.byMessageId.set("old", { + tokenCount: 1, + allBlockIds: [1], + activeBlockIds: [1], + }) + prune(state, logger, config, view.messages) + assert.deepEqual(view.restore(), native.slice(2)) +})