Merge remote-tracking branch 'origin/farm/c1eb4f67/cursor-persist-tool-calls'
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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<AssistantMessage["content"][number], { type: "toolCall" }>;
|
||||
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<AssistantMessage["content"][number], { type: "toolCall" }>;
|
||||
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<AssistantMessage["content"][number], { type: "toolCall" }>;
|
||||
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"}`;
|
||||
|
||||
@@ -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<typeof toolSchema, { command: string }> = {
|
||||
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<AgentEvent, { type: "message_end" }> =>
|
||||
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<typeof toolSchema, { value: string }> = {
|
||||
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");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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 };
|
||||
|
||||
Reference in New Issue
Block a user