From eb64e7e5d185d70540dbe2e89d2827be67aea549 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 2 Jul 2026 21:26:37 +0000 Subject: [PATCH 1/3] fix(agent-loop): skip cursor exec-resolved toolCall blocks to avoid double-execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review on PR #4351: synthesizing toolCall content blocks for Cursor's exec-channel native tools made the shared agent loop treat the finalized assistant message as a fresh runnable tool turn. Because executeToolCalls filters message.content for any toolCall block on stop/toolUse, bash/write/delete/etc. ran a second time after Cursor already executed them server-side via the bridge, duplicating side effects and appending conflicting toolResults. - packages/ai/src/utils/block-symbols.ts: add `kCursorExecResolved` symbol and `CursorExecResolvedCarrier` carrier type. Symbol-keyed so the marker never leaks into JSONL; rebuild pairs blocks with toolResult messages by id. - packages/ai/src/providers/cursor.ts: stamp the marker onto every block `synthesizeCursorExecToolCall` emits and extend `ToolCallState`. - packages/agent/src/agent-loop.ts: filter marked blocks out of the runnable-toolCall extraction in both the main runnable path and the error/aborted placeholder path, plus defense-in-depth inside `executeToolCalls`. Marked blocks stay in `assistantMessage.content` for persistence + rebuild rendering; they just never re-execute. - packages/agent/test/agent-loop.test.ts: two regression tests — one proves a marked block yields zero `tool.execute` calls and no `tool_execution_*` events from the loop, the other verifies mixed batches still run the unmarked blocks unchanged. Fixes #4348 --- packages/agent/CHANGELOG.md | 1 + packages/agent/src/agent-loop.ts | 27 +++- packages/agent/test/agent-loop.test.ts | 195 +++++++++++++++++++++++++ packages/ai/CHANGELOG.md | 2 +- packages/ai/src/providers/cursor.ts | 9 ++ packages/ai/src/utils/block-symbols.ts | 17 +++ 6 files changed, 247 insertions(+), 4 deletions(-) diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index 8421665cf..89c510789 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - Fixed cursor-agent assistant messages containing native tool calls being split on text length and duplicating text blocks on replay, by emitting the assistant message as-is followed by buffered tool results and pairing them by `toolCallId` in the transcript rebuild ([#4348](https://github.com/can1357/oh-my-pi/issues/4348)). +- Fixed cursor-agent exec-channel tools being executed a second time by the shared agent loop after Cursor's server-side execution: `agent-loop.ts` now skips `toolCall` blocks stamped with the `kCursorExecResolved` marker so `bash`/`write`/`delete`/etc. run once ([#4348](https://github.com/can1357/oh-my-pi/issues/4348)). ## [16.3.0] - 2026-07-02 diff --git a/packages/agent/src/agent-loop.ts b/packages/agent/src/agent-loop.ts index f10a7207e..a6c0418af 100644 --- a/packages/agent/src/agent-loop.ts +++ b/packages/agent/src/agent-loop.ts @@ -26,6 +26,7 @@ import { wrapInbandToolStream, } from "@oh-my-pi/pi-ai/dialect"; import * as AIError from "@oh-my-pi/pi-ai/error"; +import { type CursorExecResolvedCarrier, kCursorExecResolved } from "@oh-my-pi/pi-ai/utils/block-symbols"; import { createHarmonyAuditEvent, detectHarmonyLeakInAssistantMessage, @@ -910,7 +911,13 @@ async function runLoopBody( // Create placeholder tool results for any tool calls in the aborted message // This maintains the tool_use/tool_result pairing that the API requires type ToolCallContent = Extract; - const toolCalls = message.content.filter((c): c is ToolCallContent => c.type === "toolCall"); + // Cursor exec-resolved blocks already have their toolResult buffered + // for out-of-band emission; a placeholder aborted result here would + // pair a duplicate to the same toolCallId (issue #4348 codex review). + const toolCalls = message.content.filter( + (c): c is ToolCallContent => + c.type === "toolCall" && (c as CursorExecResolvedCarrier)[kCursorExecResolved] !== true, + ); const toolResults: ToolResultMessage[] = []; for (const toolCall of toolCalls) { const result = createAbortedToolResult(toolCall, stream, message.stopReason, message.errorMessage); @@ -949,7 +956,15 @@ async function runLoopBody( // trailing tool_use may be truncated with incomplete arguments — those calls // are abandoned below. (`error`/`aborted` already returned above.) type ToolCallContent = Extract; - const toolCalls = message.content.filter((c): c is ToolCallContent => c.type === "toolCall"); + // A Cursor exec-channel synthesized `toolCall` block carries + // `kCursorExecResolved` because Cursor already executed the tool + // server-side (via the bridge) and buffered the result for + // out-of-band emission — running it here again would duplicate the + // same side-effecting call (issue #4348 review by @chatgpt-codex-connector). + const toolCalls = message.content.filter( + (c): c is ToolCallContent => + c.type === "toolCall" && (c as CursorExecResolvedCarrier)[kCursorExecResolved] !== true, + ); const runnableStop = message.stopReason === "toolUse" || message.stopReason === "stop"; hasMoreToolCalls = runnableStop && toolCalls.length > 0; @@ -1679,7 +1694,13 @@ async function executeToolCalls( afterToolCall, } = config; type ToolCallContent = Extract; - const toolCalls = assistantMessage.content.filter((c): c is ToolCallContent => c.type === "toolCall"); + // Defensive: the outer loop already filters exec-resolved blocks before + // deciding to invoke `executeToolCalls`, but skip them here too so the + // guarantee lives with the code that would re-run the tool. + const toolCalls = assistantMessage.content.filter( + (c): c is ToolCallContent => + c.type === "toolCall" && (c as CursorExecResolvedCarrier)[kCursorExecResolved] !== true, + ); const emittedToolResults: ToolResultMessage[] = []; const toolCallInfos = toolCalls.map(call => ({ id: call.id, name: call.name })); const batchId = `${assistantMessage.timestamp ?? Date.now()}_${toolCalls[0]?.id ?? "batch"}`; diff --git a/packages/agent/test/agent-loop.test.ts b/packages/agent/test/agent-loop.test.ts index b04d64da5..8c61dba54 100644 --- a/packages/agent/test/agent-loop.test.ts +++ b/packages/agent/test/agent-loop.test.ts @@ -2717,3 +2717,198 @@ describe("agentLoop streaming snapshots", () => { expect(update.message).not.toBe(livePartial); }); }); + +describe("agentLoop kCursorExecResolved (issue #4348)", () => { + it("skips execute for a toolCall block marked as already run by Cursor's exec channel", async () => { + const { kCursorExecResolved } = await import("@oh-my-pi/pi-ai/utils/block-symbols"); + + const toolSchema = type({ command: "string" }); + let executeCalls = 0; + const tool: AgentTool = { + name: "bash", + label: "Bash", + description: "Run shell commands", + parameters: toolSchema, + async execute(_id, params) { + executeCalls += 1; + return { + content: [{ type: "text", text: `local run: ${params.command}` }], + details: {}, + }; + }, + }; + + const context: AgentContext = { systemPrompt: [""], messages: [], tools: [tool] }; + const config: AgentLoopConfig = { + model: createMockModel().model, + convertToLlm: identityConverter, + }; + + // Simulate the shape the Cursor provider now emits after + // `synthesizeCursorExecToolCall`: a `toolCall` content block stamped + // with `kCursorExecResolved` because the server-driven exec channel + // already ran the tool. `agent-loop.ts` MUST NOT invoke `tool.execute` + // again — that would double-execute bash/write/delete/etc. and append a + // second `toolResult` with the same `toolCallId`. + const streamFn = () => { + const stream = new AssistantMessageEventStream(); + queueMicrotask(() => { + const resolvedBlock = { + type: "toolCall" as const, + id: "cursor-exec-tc-1", + name: "bash", + arguments: { command: "rm -rf /tmp/cursor-once" }, + [kCursorExecResolved]: true as const, + }; + const finalMessage: AssistantMessage = { + role: "assistant", + content: [{ type: "text", text: "Ran the command." }, resolvedBlock], + api: "cursor-agent", + provider: "cursor", + model: "cursor-composer-2.5", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: Date.now(), + }; + stream.push({ type: "start", partial: finalMessage }); + stream.push({ type: "done", reason: "stop", message: finalMessage }); + }); + return stream; + }; + + const events: AgentEvent[] = []; + const stream = agentLoop([createUserMessage("run bash")], context, config, undefined, streamFn); + for await (const event of stream) { + events.push(event); + } + + // Tool MUST NOT have been executed — Cursor already ran it server-side. + expect(executeCalls).toBe(0); + + // No `tool_execution_start`/`tool_execution_end` from agent-loop — + // those live-events belong to the bridge that already ran the tool. + const startFromLoop = events.find(e => e.type === "tool_execution_start"); + const endFromLoop = events.find(e => e.type === "tool_execution_end"); + expect(startFromLoop).toBeUndefined(); + expect(endFromLoop).toBeUndefined(); + + // And no duplicate `toolResult` message emitted for the resolved block: + // the buffered result flows via the Agent-class path, not agent-loop. + const orphanToolResult = events.find( + (e): e is Extract => + e.type === "message_end" && e.message.role === "toolResult", + ); + expect(orphanToolResult).toBeUndefined(); + }); + + it("still runs a normal, unmarked toolCall block in the same turn", async () => { + // Guards against the filter over-matching: a mixed turn where only + // SOME blocks are Cursor-resolved must still execute the unmarked one. + const { kCursorExecResolved } = await import("@oh-my-pi/pi-ai/utils/block-symbols"); + + const toolSchema = type({ value: "string" }); + const executed: string[] = []; + const tool: AgentTool = { + name: "echo", + label: "Echo", + description: "Echo tool", + parameters: toolSchema, + async execute(_id, params) { + executed.push(params.value); + return { content: [{ type: "text", text: `echoed: ${params.value}` }], details: {} }; + }, + }; + + const context: AgentContext = { systemPrompt: [""], messages: [], tools: [tool] }; + const config: AgentLoopConfig = { + model: createMockModel().model, + convertToLlm: identityConverter, + }; + + let turn = 0; + const streamFn = () => { + const stream = new AssistantMessageEventStream(); + queueMicrotask(() => { + if (turn++ === 0) { + const resolvedBlock = { + type: "toolCall" as const, + id: "resolved-1", + name: "bash", + arguments: { command: "true" }, + [kCursorExecResolved]: true as const, + }; + const runnableBlock = { + type: "toolCall" as const, + id: "runnable-1", + name: "echo", + arguments: { value: "hi" }, + }; + const partial: AssistantMessage = { + role: "assistant", + content: [resolvedBlock, runnableBlock], + api: "cursor-agent", + provider: "cursor", + model: "cursor-composer-2.5", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "toolUse", + timestamp: Date.now(), + }; + stream.push({ type: "start", partial }); + stream.push({ type: "done", reason: "toolUse", message: partial }); + } else { + // Second turn: replay `done` closes the loop. + const partial: AssistantMessage = { + role: "assistant", + content: [{ type: "text", text: "finished" }], + api: "cursor-agent", + provider: "cursor", + model: "cursor-composer-2.5", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: Date.now(), + }; + stream.push({ type: "start", partial }); + stream.push({ type: "done", reason: "stop", message: partial }); + } + }); + return stream; + }; + + const events: AgentEvent[] = []; + const stream = agentLoop([createUserMessage("mixed")], context, config, undefined, streamFn); + for await (const event of stream) { + events.push(event); + } + + expect(executed).toEqual(["hi"]); + + const executionStarts = events.filter(e => e.type === "tool_execution_start"); + // Exactly one execution: the unmarked `echo` block. The resolved + // `bash` block is passed through untouched. + expect(executionStarts).toHaveLength(1); + if (executionStarts[0]?.type !== "tool_execution_start") throw new Error("expected tool_execution_start"); + expect(executionStarts[0].toolCallId).toBe("runnable-1"); + expect(executionStarts[0].toolName).toBe("echo"); + }); +}); diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index d8a2209e9..92dae3b57 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed cursor-agent persisted transcripts losing tool-call structure by synthesizing `toolCall` content blocks for exec-channel native tools (`bash`/`read`/`write`/`grep`/`ls`/`delete`/`lsp`), so replay pairs each tool result with its call instead of rendering header-less tool output beneath the last assistant text ([#4348](https://github.com/can1357/oh-my-pi/issues/4348)). +- Fixed cursor-agent persisted transcripts losing tool-call structure by synthesizing `toolCall` content blocks for exec-channel native tools (`bash`/`read`/`write`/`grep`/`ls`/`delete`/`lsp`), so replay pairs each tool result with its call instead of rendering header-less tool output beneath the last assistant text ([#4348](https://github.com/can1357/oh-my-pi/issues/4348)). Synthesized blocks carry a new `kCursorExecResolved` symbol marker so the shared agent loop skips executing them a second time. ## [16.3.1] - 2026-07-02 diff --git a/packages/ai/src/providers/cursor.ts b/packages/ai/src/providers/cursor.ts index 88e9ca1cf..84c366f55 100644 --- a/packages/ai/src/providers/cursor.ts +++ b/packages/ai/src/providers/cursor.ts @@ -133,6 +133,7 @@ import type { import { normalizeSystemPrompts } from "../utils"; import { clearStreamingPartialJson, + kCursorExecResolved, kStreamingBlockIndex, kStreamingBlockKind, kStreamingLastParseLen, @@ -629,6 +630,7 @@ export type ToolCallState = ToolCall & { [kStreamingPartialJson]?: string; [kStreamingLastParseLen]?: number; [kStreamingBlockKind]: "mcp" | "todo" | "cursor-exec"; + [kCursorExecResolved]?: true; }; export interface BlockState { @@ -2082,6 +2084,12 @@ function endCurrentThinkingBlock( * `renderSessionContext`, so they render as header-less `⎿` lines beneath the * last text block instead of proper tool components (issue #4348). * + * The block is stamped with {@link kCursorExecResolved} so the shared + * `agent-loop.ts` execution pass skips it — Cursor's server-driven exec + * channel already ran the tool via the bridge and buffered the result, so + * treating this block as runnable would re-execute the same side-effecting + * tool a second time. + * * Exported for tests to exercise ordering with adjacent text/thinking blocks. */ export function synthesizeCursorExecToolCall( @@ -2101,6 +2109,7 @@ export function synthesizeCursorExecToolCall( arguments: args, [kStreamingBlockIndex]: output.content.length, [kStreamingBlockKind]: "cursor-exec", + [kCursorExecResolved]: true, }; output.content.push(block); const idx = output.content.length - 1; diff --git a/packages/ai/src/utils/block-symbols.ts b/packages/ai/src/utils/block-symbols.ts index b0b8efb13..e95557062 100644 --- a/packages/ai/src/utils/block-symbols.ts +++ b/packages/ai/src/utils/block-symbols.ts @@ -30,3 +30,20 @@ export const kStreamingArgumentsDone = Symbol("provider.block.argumentsDone"); /** Classifies Cursor's in-flight tool-call kind without leaking provider-private state. */ export const kStreamingBlockKind = Symbol("provider.block.kind"); + +/** + * Marks a `toolCall` content block that Cursor's exec channel already + * executed server-side (via the coding-agent bridge) and whose result is + * buffered separately for emission via the assistant-loop stream. + * + * `agent-loop.ts` MUST skip execution of blocks carrying this marker — + * treating them as a fresh runnable tool call would run the same + * side-effecting tool (bash, write, delete, …) a second time. Symbol-keyed + * so it never persists across the JSONL round-trip, where rebuild instead + * pairs the block with its already-persisted `toolResult` message by id. + */ +export const kCursorExecResolved = Symbol("provider.block.cursorExecResolved"); + +/** Carries the resolved marker without exposing a string-keyed property. */ +export type CursorExecResolvedCarrier = object & { [kCursorExecResolved]?: true }; + From 76f8205ff53441f353be272ab4a07aa73d51a928 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 2 Jul 2026 21:26:45 +0000 Subject: [PATCH 2/3] style: bun run fix --- packages/ai/src/utils/block-symbols.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/ai/src/utils/block-symbols.ts b/packages/ai/src/utils/block-symbols.ts index e95557062..b760e53e3 100644 --- a/packages/ai/src/utils/block-symbols.ts +++ b/packages/ai/src/utils/block-symbols.ts @@ -46,4 +46,3 @@ export const kCursorExecResolved = Symbol("provider.block.cursorExecResolved"); /** Carries the resolved marker without exposing a string-keyed property. */ export type CursorExecResolvedCarrier = object & { [kCursorExecResolved]?: true }; - From 107503d201857d9395841695d462e3ed3f09f493 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 2 Jul 2026 21:27:32 +0000 Subject: [PATCH 3/3] test(agent-loop): align tool result details type with parameter shape --- packages/agent/test/agent-loop.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/agent/test/agent-loop.test.ts b/packages/agent/test/agent-loop.test.ts index 8c61dba54..aee4d8f06 100644 --- a/packages/agent/test/agent-loop.test.ts +++ b/packages/agent/test/agent-loop.test.ts @@ -2733,7 +2733,7 @@ describe("agentLoop kCursorExecResolved (issue #4348)", () => { executeCalls += 1; return { content: [{ type: "text", text: `local run: ${params.command}` }], - details: {}, + details: { command: params.command }, }; }, }; @@ -2822,7 +2822,10 @@ describe("agentLoop kCursorExecResolved (issue #4348)", () => { parameters: toolSchema, async execute(_id, params) { executed.push(params.value); - return { content: [{ type: "text", text: `echoed: ${params.value}` }], details: {} }; + return { + content: [{ type: "text", text: `echoed: ${params.value}` }], + details: { value: params.value }, + }; }, };