diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index 9bf241470..c2c87b60a 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -8,6 +8,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 db0692d1e..d74f8a6cb 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; @@ -1685,7 +1700,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..aee4d8f06 100644 --- a/packages/agent/test/agent-loop.test.ts +++ b/packages/agent/test/agent-loop.test.ts @@ -2717,3 +2717,201 @@ 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: { command: params.command }, + }; + }, + }; + + 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: { value: params.value }, + }; + }, + }; + + 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 48513de60..78758c301 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -20,6 +20,7 @@ ### Fixed - Fixed OpenAI-compatible streaming usage parsing to prefer non-zero nested cached token counts when root `cached_tokens` is zero ([#4337](https://github.com/can1357/oh-my-pi/issues/4337)). +- 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..b760e53e3 100644 --- a/packages/ai/src/utils/block-symbols.ts +++ b/packages/ai/src/utils/block-symbols.ts @@ -30,3 +30,19 @@ 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 };