From 7b62fef36612c7fd950a7ac9b158d83764e1eaea Mon Sep 17 00:00:00 2001 From: Diogo Soares Rodrigues Date: Mon, 27 Jul 2026 09:08:38 -0300 Subject: [PATCH] fix(cursor): honor Pi frame arguments and preserve open-block args MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the modern exec wire protocol surfaced defects the committed suite did not pin. The Pi bridge dropped frame arguments: `pi_read`'s offset/limit (ranged reads returned whole files), `pi_grep`'s literal (fixed strings ran as regexes), and the path/glob join emitted `./`-prefixed specs. These are `optional int32`, so a present `0` is a value, not "unset" — `limit: 0` now answers empty rather than reading everything, and `pi_find` clamps to 1 like the reference client. The provider synthesized its transcript block from a second, divergent translation of the same frame, so the displayed operation differed from the executed one. Both sides now share one mapper in `exec-modern.ts`. End-of-transport cleanup reparsed every open block's streamed argument buffer; blocks whose args arrive whole never set that buffer, and `parseStreamingJson(undefined)` is `{}`, so a truncated turn erased their arguments. All fixes are mutation-verified: reverting each one fails a test. (cherry picked from commit bb7bcfebce4200d436e17d6e39320da13fc85ca8) --- packages/ai/CHANGELOG.md | 2 + packages/ai/src/providers/cursor.ts | 66 +++++--- .../ai/src/providers/cursor/exec-modern.ts | 58 +++++++ packages/ai/test/cursor-exec-modern.test.ts | 154 +++++++++++++++++- .../ai/test/cursor-mcp-tool-catalog.test.ts | 12 ++ packages/coding-agent/CHANGELOG.md | 4 + packages/coding-agent/src/cursor.ts | 45 ++++- .../coding-agent/test/cursor-exec.test.ts | 63 ++++++- 8 files changed, 367 insertions(+), 37 deletions(-) diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 823950063..23e8f2717 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -74,6 +74,8 @@ - Fixed Cursor `connect_scm` calls losing their repository and settling on a fabricated verdict. The target rides in the `ConnectScmArgs.target` oneof, so reading a flat `github` property always saw `undefined`; and the authoritative `success`/`error`/`rejected` result only arrives on the completion frame, so answering at the announcement persisted a fixed failure for every call — including the ones the server went on to accept. The block now opens on the start frame and settles from the completion's decoded result. - Fixed interleaved Cursor tool calls corrupting each other. The stream decoder tracked a single "current" block and settled it on any `toolCallCompleted`, ignoring the envelope's `call_id`: a completion for one call closed whichever block happened to be open and paired it with the wrong result, and `start A, start B` orphaned A entirely so its own completion settled B while A was never paired — which strips the whole interaction from every rebuilt transcript. Open blocks are now retained per envelope `call_id`, and end-of-stream closes all of them rather than only the last. - Fixed a Cursor `search_conversations` call leaving no transcript block. The frame is answered from a fixed verdict, so nothing downstream pairs a result for it, and an unpaired call takes its whole interaction out of every rebuilt transcript. +- Fixed the Cursor stream's end-of-transport cleanup erasing the arguments of every block still open. Blocks whose args arrive whole (todo, connect-SCM, MCP) never feed the streamed partial-JSON buffer, and reparsing an absent buffer yields `{}`, so a truncated or disconnected turn rebuilt those calls with no arguments at all. Only blocks that actually streamed their args are reparsed now. +- Fixed the Pi exec frames displaying a different operation than the one they run. The provider synthesized its transcript block from a second, hand-rolled translation of the frame args, so `pi_read`'s `offset`/`limit` were shown as a whole-file read, `pi_grep`'s `literal` pattern as an unescaped regex, and `pi_find`'s path/glob join differed from the executed one. Both sides now share a single translation. - Fixed the streamed `pi_*_tool_call` announcements that modern builds send alongside each exec frame being unrecognized. The exec channel already synthesizes those blocks when it runs the tool; the duplicate was avoided only because the decoder recognized none of the variants, which would have started double-rendering as soon as any one was added. ## [17.1.4] - 2026-07-26 diff --git a/packages/ai/src/providers/cursor.ts b/packages/ai/src/providers/cursor.ts index f613c3ff2..8e10cba54 100644 --- a/packages/ai/src/providers/cursor.ts +++ b/packages/ai/src/providers/cursor.ts @@ -192,6 +192,10 @@ import { buildPiReadResult, buildPiWriteError, buildPiWriteResult, + piEscapeRegexLiteral, + piJoinPath, + piLimit, + piReadPath, } from "./cursor/exec-modern"; export const CURSOR_API_URL = "https://api2.cursor.sh"; @@ -729,20 +733,7 @@ export const streamCursor: StreamFunction<"cursor-agent"> = ( endCurrentTextBlock(output, stream, state); endCurrentThinkingBlock(output, stream, state); - // Every block still open when the transport ends must be closed, not - // just the last one started: with interleaved calls several can be - // open at once, and an unclosed block leaves its live card animating - // and its call unpaired. - const openBlocks = new Set(state.openToolCalls.values()); - if (state.currentToolCall) openBlocks.add(state.currentToolCall); - for (const block of openBlocks) { - const idx = output.content.indexOf(block); - block.arguments = parseStreamingJson(block[kStreamingPartialJson]); - clearStreamingPartialJson(block); - stream.push({ type: "toolcall_end", contentIndex: idx, toolCall: block, partial: output }); - } - state.openToolCalls.clear(); - state.setToolCall(null); + flushOpenToolCalls(output, stream, state); calculateCost(model, output.usage); @@ -1559,7 +1550,11 @@ async function handleExecServerMessage( case "piReadArgs": { const args = execMsg.message.value; const toolCallId = crypto.randomUUID(); - synthesizeCursorExecToolCall(output, stream, state, toolCallId, "read", { path: args.path }); + // The displayed block must show the operation that actually runs: the + // bridge composes the same range selector onto the path. + synthesizeCursorExecToolCall(output, stream, state, toolCallId, "read", { + path: piReadPath(args.path, args.offset, args.limit) ?? args.path, + }); const { execResult } = await resolveExecHandler( { args, toolCallId }, execHandlers?.piRead?.bind(execHandlers), @@ -1635,8 +1630,8 @@ async function handleExecServerMessage( const args = execMsg.message.value; const toolCallId = crypto.randomUUID(); synthesizeCursorExecToolCall(output, stream, state, toolCallId, "grep", { - pattern: args.pattern, - path: args.glob ? `${args.path || "."}/${args.glob}` : args.path || ".", + pattern: args.literal === true ? piEscapeRegexLiteral(args.pattern) : args.pattern, + path: args.glob ? piJoinPath(args.path, args.glob) : args.path || ".", case: args.ignoreCase === true ? false : undefined, }); const { execResult } = await resolveExecHandler( @@ -1655,8 +1650,8 @@ async function handleExecServerMessage( const args = execMsg.message.value; const toolCallId = crypto.randomUUID(); synthesizeCursorExecToolCall(output, stream, state, toolCallId, "glob", { - path: args.path ? `${args.path}/${args.pattern}` : args.pattern, - limit: args.limit, + path: piJoinPath(args.path, args.pattern), + limit: piLimit(args.limit), }); const { execResult } = await resolveExecHandler( { args, toolCallId }, @@ -2814,6 +2809,39 @@ function isExecOwnedToolCall(toolCall: { tool?: { case?: string } } | undefined) * `currentToolCall` is still set, as the fallback for frames that carry no * `call_id` (proto3-optional, and unset on what older builds send). */ +/** + * Close every tool-call block still open when the stream ends. + * + * Not just the last one started: with interleaved calls several can be open at + * once, and an unclosed block leaves its live card animating and its call + * unpaired. + * + * Only blocks fed by a streamed argument buffer get reparsed. Todo, + * connect-SCM and MCP-settled frames arrive with complete `arguments` and + * never set the partial buffer; `parseStreamingJson(undefined)` returns `{}`, + * so reparsing unconditionally would erase the arguments of every such block + * caught open by a truncated stream. + */ +export function flushOpenToolCalls( + output: AssistantMessage, + stream: AssistantMessageEventStream, + state: BlockState, +): void { + const openBlocks = new Set(state.openToolCalls.values()); + if (state.currentToolCall) openBlocks.add(state.currentToolCall); + for (const block of openBlocks) { + const idx = output.content.indexOf(block); + const partialJson = block[kStreamingPartialJson]; + if (partialJson !== undefined) { + block.arguments = parseStreamingJson(partialJson); + clearStreamingPartialJson(block); + } + stream.push({ type: "toolcall_end", contentIndex: idx, toolCall: block, partial: output }); + } + state.openToolCalls.clear(); + state.setToolCall(null); +} + function retainStreamedCall(state: BlockState, block: ToolCallState, envelopeId: string | undefined): void { if (envelopeId) state.openToolCalls.set(envelopeId, block); state.setToolCall(block); diff --git a/packages/ai/src/providers/cursor/exec-modern.ts b/packages/ai/src/providers/cursor/exec-modern.ts index bd8d96254..d61876c41 100644 --- a/packages/ai/src/providers/cursor/exec-modern.ts +++ b/packages/ai/src/providers/cursor/exec-modern.ts @@ -10,6 +10,7 @@ * the server reads as "the tool ran and produced nothing". */ +import * as path from "node:path"; import { create } from "@bufbuild/protobuf"; import { AfterAgentResponseRequestResponseSchema, @@ -65,6 +66,63 @@ import { } from "@oh-my-pi/pi-catalog/discovery/cursor-gen/agent_pb"; import type { ToolResultMessage } from "../../types"; +/** + * Translate a Pi frame's args into the local tool kwargs that run it. + * + * Shared deliberately: the provider synthesizes a display block from these and + * the coding-agent bridge executes with them. Two hand-rolled translations of + * one frame drift, and the drift is invisible — the transcript shows one + * operation while a different one runs. + * + * Every `optional int32` here is presence-sensitive: `0` is a supplied value, + * not "unset", so it must never be folded into a default. + */ + +/** + * A `pi_read` range composed onto the path as `read`'s inline `:N+K` selector. + * + * `read` exposes no range kwargs, so an uncomposed range reads the whole file. + * `offset` is a 1-indexed start clamped like the reference's + * `Math.max(0, offset - 1)` over 0-indexed lines; `limit` is a line count. + * `null` marks a present `limit: 0` — zero lines, which no selector expresses + * and which must not degrade into a whole-file read. + */ +export function piReadPath(path: string, offset?: number, limit?: number): string | null { + if (limit !== undefined && Math.floor(limit) <= 0) return null; + const start = offset !== undefined ? Math.max(1, Math.floor(offset)) : undefined; + const count = limit !== undefined ? Math.floor(limit) : undefined; + if (start === undefined && count === undefined) return path; + if (start === undefined) return `${path}:1+${count}`; + return count === undefined ? `${path}:${start}-` : `${path}:${start}+${count}`; +} + +/** + * Join a Pi frame's optional `path` with the `glob`/`pattern` it scopes. + * + * The local `grep`/`glob` tools take one combined path spec. An absolute + * pattern ignores the path, and an absent or `.` path leaves the pattern + * standing alone rather than building a `./`- or `//`-prefixed spec. + * + * Uses `node:path` rather than string surgery so Windows absolutes (`C:\…`, + * UNC) are recognised and separators stay normalized — the same treatment + * `joinLegacyGlob` gives the legacy pi shim's identical path/glob pair. + */ +export function piJoinPath(basePath: string | undefined, pattern: string): string { + if (path.isAbsolute(pattern)) return pattern; + if (!basePath || basePath === ".") return pattern; + return path.join(basePath, pattern); +} + +/** Escape a literal string so the regex-only local `grep` tool matches it verbatim. */ +export function piEscapeRegexLiteral(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +/** Clamp a present `optional int32` result cap the way the reference does; `undefined` stays unset. */ +export function piLimit(limit: number | undefined): number | undefined { + return limit === undefined ? undefined : Math.max(1, Math.floor(limit)); +} + /** Flatten a tool result's content into the single `output` string the Pi frames carry. */ export function piOutputText(toolResult: ToolResultMessage): string { return toolResult.content.map(item => (item.type === "text" ? item.text : `[${item.mimeType} image]`)).join("\n"); diff --git a/packages/ai/test/cursor-exec-modern.test.ts b/packages/ai/test/cursor-exec-modern.test.ts index 64f3754a5..22e33636f 100644 --- a/packages/ai/test/cursor-exec-modern.test.ts +++ b/packages/ai/test/cursor-exec-modern.test.ts @@ -2,12 +2,13 @@ import { describe, expect, it } from "bun:test"; import { create, fromBinary, toBinary } from "@bufbuild/protobuf"; import { type BlockState, + flushOpenToolCalls, handleServerMessage, processInteractionUpdate, type ToolCallState, } from "@oh-my-pi/pi-ai/providers/cursor"; import type { AssistantMessage, CursorExecHandlers, ToolResultMessage } from "@oh-my-pi/pi-ai/types"; -import { kCursorExecResolved } from "@oh-my-pi/pi-ai/utils/block-symbols"; +import { kCursorExecResolved, setStreamingPartialJson } from "@oh-my-pi/pi-ai/utils/block-symbols"; import { AssistantMessageEventStream } from "@oh-my-pi/pi-ai/utils/event-stream"; import { type AgentClientMessage, @@ -188,6 +189,90 @@ function soleResult(frames: AgentClientMessage[]) { return frame.value.message; } +describe("Cursor stream teardown", () => { + it("keeps whole arguments on a block still open when the transport ends", async () => { + // A connect-SCM block arrives with complete `arguments` and never feeds + // the streamed partial-JSON buffer. `parseStreamingJson(undefined)` + // returns `{}`, so reparsing every open block on teardown erased exactly + // those args — the interaction then rebuilt as an empty call. + const wire = create(ToolCallSchema, { + tool: { + case: "connectScmToolCall", + value: create(ConnectScmToolCallSchema, { + args: create(ConnectScmArgsSchema, { + toolCallId: "inner-id", + target: { + case: "github", + value: create(ConnectScmGithubSchema, { + repository: create(ConnectScmGithubRepositorySchema, { owner: "can1357", repo: "oh-my-pi" }), + }), + }, + }), + }), + }, + }); + const toolCall = fromBinary(ToolCallSchema, toBinary(ToolCallSchema, wire)); + + const output = cursorAssistantMessage(); + const stream = new AssistantMessageEventStream(); + const state = newBlockState(); + + // Start the call and leave it open: the transport dies before completion. + processInteractionUpdate( + { message: { case: "toolCallStarted", value: { callId: "envelope-a", toolCall } } }, + output, + stream, + state, + { sawTokenDelta: false }, + ); + + const block = output.content.find((b): b is ToolCallState => b.type === "toolCall"); + if (!block) throw new Error("expected an open tool-call block"); + const argsBeforeTeardown = block.arguments; + expect(argsBeforeTeardown).not.toEqual({}); + + flushOpenToolCalls(output, stream, state); + + expect(block.arguments).toEqual(argsBeforeTeardown); + // The block is closed, so no live card is left animating. + expect(state.openToolCalls.size).toBe(0); + expect(state.currentToolCall).toBeNull(); + }); + + it("salvages a truncated streamed argument buffer into partial arguments", async () => { + // The other half of the same contract: blocks that *do* stream their args + // must still be reparsed on teardown, or a cut-short call renders with no + // arguments at all. + const output = cursorAssistantMessage(); + const stream = new AssistantMessageEventStream(); + const state = newBlockState(); + + processInteractionUpdate( + { + message: { + case: "toolCallStarted", + value: { + callId: "envelope-mcp", + toolCall: { tool: { case: "mcpToolCall", value: { args: { toolCallId: "mcp-1" } } } }, + }, + }, + }, + output, + stream, + state, + { sawTokenDelta: false }, + ); + + const block = output.content.find((b): b is ToolCallState => b.type === "toolCall"); + if (!block) throw new Error("expected an open tool-call block"); + setStreamingPartialJson(block, '{"path":"/repo/a.ts"'); + + flushOpenToolCalls(output, stream, state); + + expect(block.arguments).toEqual({ path: "/repo/a.ts" }); + }); +}); + describe("Cursor modern exec frames: failure channel", () => { it("throws on a frame whose oneof this build does not model, instead of stranding the exec id", async () => { // A oneof number absent from agent.proto decodes into unknown fields and @@ -443,6 +528,38 @@ describe("Cursor modern exec frames: hooks", () => { expect(answer.value.response.response.value.additionalContext).toBeUndefined(); }); + it("maps every modelled hook request onto its parallel response case", async () => { + // The request and response oneofs are parallel by field number. A single + // mis-wired branch answers a different hook than the one asked, which the + // server reads as a stalled or misrouted hook — invisible unless every + // variant is exercised. + const requestCases = [ + "preCompact", + "subagentStart", + "subagentStop", + "preToolUse", + "postToolUse", + "postToolUseFailure", + "beforeSubmitPrompt", + "afterAgentResponse", + "afterAgentThought", + "stop", + ] as const; + + for (const requestCase of requestCases) { + const request = create(ExecuteHookRequestSchema, { + request: { case: requestCase, value: {} }, + } as never); + const { frames } = await dispatchExec( + buildExecMessage({ case: "executeHookArgs", value: create(ExecuteHookArgsSchema, { request }) }), + ); + + const answer = soleResult(frames); + if (answer.case !== "executeHookResult") throw new Error(`${requestCase}: got ${answer.case}`); + expect(answer.value.response?.response.case).toBe(requestCase); + } + }); + it("throws for a hook request whose case this build does not model", async () => { const { frames } = await dispatchExec( buildExecMessage({ @@ -557,6 +674,10 @@ describe("Cursor modern exec frames: Pi tools", () => { expect(blocks).toHaveLength(1); expect(blocks[0].name).toBe("read"); expect(results.map(r => r.toolCallId)).toEqual([blocks[0].id]); + // The displayed args must be the operation that actually runs. The bridge + // composes offset/limit into `read`'s `:N+K` selector, so a block showing + // the bare path claims a whole-file read that never happened. + expect(blocks[0].arguments).toEqual({ path: "/repo/a.ts:5+20" }); }); it("maps a failing Pi handler onto the frame's own error variant", async () => { @@ -669,19 +790,22 @@ describe("Cursor modern exec frames: Pi tools", () => { expect(block?.arguments).toEqual({ path: "/repo/a.ts", edits: [{ old_text: "before", new_text: "after" }] }); }); - it("answers each remaining Pi frame with its own matching result case", async () => { + it("answers each remaining Pi frame with a populated success payload", async () => { + // The outer result case alone is not proof: an unset inner oneof reads as + // "the tool ran and produced nothing", and dropped output or limit + // metadata is invisible at the discriminator level. const handlers: CursorExecHandlers = { async piWrite() { return toolResult("wrote"); }, async piGrep() { - return toolResult("a.ts:1:hit"); + return toolResult("a.ts:1:hit", { details: { perFileLimitReached: 20, linesTruncated: true } }); }, async piFind() { - return toolResult("a.ts"); + return toolResult("a.ts", { details: { resultLimitReached: 200 } }); }, async piLs() { - return toolResult("a.ts\nb.ts"); + return toolResult("a.ts\nb.ts", { details: { resultLimitReached: 500 } }); }, }; @@ -696,7 +820,9 @@ describe("Cursor modern exec frames: Pi tools", () => { ) ).frames, ); - expect(write.case).toBe("piWriteResult"); + if (write.case !== "piWriteResult") throw new Error(`got ${write.case}`); + if (write.value.result.case !== "success") throw new Error("expected success"); + expect(write.value.result.value.output).toBe("wrote"); const grep = soleResult( ( @@ -706,7 +832,11 @@ describe("Cursor modern exec frames: Pi tools", () => { ) ).frames, ); - expect(grep.case).toBe("piGrepResult"); + if (grep.case !== "piGrepResult") throw new Error(`got ${grep.case}`); + if (grep.value.result.case !== "success") throw new Error("expected success"); + expect(grep.value.result.value.output).toBe("a.ts:1:hit"); + expect(grep.value.result.value.matchLimitReached).toBe(20); + expect(grep.value.result.value.linesTruncated).toBe(true); const find = soleResult( ( @@ -716,7 +846,10 @@ describe("Cursor modern exec frames: Pi tools", () => { ) ).frames, ); - expect(find.case).toBe("piFindResult"); + if (find.case !== "piFindResult") throw new Error(`got ${find.case}`); + if (find.value.result.case !== "success") throw new Error("expected success"); + expect(find.value.result.value.output).toBe("a.ts"); + expect(find.value.result.value.resultLimitReached).toBe(200); const ls = soleResult( ( @@ -725,7 +858,10 @@ describe("Cursor modern exec frames: Pi tools", () => { }) ).frames, ); - expect(ls.case).toBe("piLsResult"); + if (ls.case !== "piLsResult") throw new Error(`got ${ls.case}`); + if (ls.value.result.case !== "success") throw new Error("expected success"); + expect(ls.value.result.value.output).toBe("a.ts\nb.ts"); + expect(ls.value.result.value.entryLimitReached).toBe(500); }); it("serves miniSweAgentBash from the existing shell handler under its own frame", async () => { diff --git a/packages/ai/test/cursor-mcp-tool-catalog.test.ts b/packages/ai/test/cursor-mcp-tool-catalog.test.ts index 1c8dda9a2..286883ec3 100644 --- a/packages/ai/test/cursor-mcp-tool-catalog.test.ts +++ b/packages/ai/test/cursor-mcp-tool-catalog.test.ts @@ -44,4 +44,16 @@ describe("cursor buildMcpToolDefinitions", () => { const names = buildMcpToolDefinitions([tool("read"), tool("write"), tool("bash")]).map(def => def.name); expect(names).toEqual([]); }); + + it("advertises lsp, which Cursor has no native equivalent for", () => { + // `lsp` is deliberately absent from the native-filtered set: Cursor's own + // tools cover none of definition/references/rename, so filtering it out + // leaves the model with no way to reach them at all. + const defs = buildMcpToolDefinitions([tool("read"), tool("bash"), tool("lsp")]); + + const lspDef = defs.find(def => def.name === "lsp"); + expect(lspDef).toBeDefined(); + expect(lspDef?.providerIdentifier).toBe("pi-agent"); + expect(lspDef?.toolName).toBe("lsp"); + }); }); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a175d40cf..73021a173 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -165,6 +165,10 @@ - The Cursor exec bridge serves the seven modern Pi tool frames, mapping each to its local equivalent: `pi_read`/`pi_ls` → `read`, `pi_bash` → `bash`, `pi_edit` → `edit`, `pi_write` → `write`, `pi_grep` → `grep`, and `pi_find` → `glob`. The frames are a separate wire family from the legacy args, not aliases, so each mapping is a real translation — `pi_grep`'s `ignore_case` is the inverse of the local tool's case-sensitivity flag, `pi_find` searches filenames rather than contents, and `pi_edit`'s replacements are renamed to the local snake_case pairs. +### Fixed + +- Fixed the Cursor Pi exec bridge silently dropping frame arguments. `pi_read`'s `offset`/`limit` were ignored, so a ranged read returned the whole file; `pi_grep`'s `literal` was ignored, so a fixed-string search ran as a regex and matched the wrong lines; and the path/glob join produced a `./`-prefixed spec. Ranges are now composed onto `read`'s `:N+K` inline selector, literal patterns are escaped, and the join uses `node:path`. These are `optional int32` fields, so a present `0` is honored rather than folded into a default: `pi_read` with `limit: 0` answers with empty output instead of the entire file, and `pi_find` with `limit: 0` clamps to 1 the way the reference client does. + ## [17.1.4] - 2026-07-26 ### Added diff --git a/packages/coding-agent/src/cursor.ts b/packages/coding-agent/src/cursor.ts index e06b72866..39ae87cba 100644 --- a/packages/coding-agent/src/cursor.ts +++ b/packages/coding-agent/src/cursor.ts @@ -14,6 +14,7 @@ import type { CursorExecHandlers as ICursorExecHandlers, ToolResultMessage, } from "@oh-my-pi/pi-ai"; +import { piEscapeRegexLiteral, piJoinPath, piLimit, piReadPath } from "@oh-my-pi/pi-ai/providers/cursor/exec-modern"; import { sanitizeText } from "@oh-my-pi/pi-utils"; import { resolveToCwd } from "./tools/path-utils"; import type { TodoPhase, TodoStatus } from "./tools/todo"; @@ -410,8 +411,23 @@ export class CursorExecHandlers implements ICursorExecHandlers { * the local tool with matching semantics, so the same approval, sandboxing * and event plumbing applies as for a model-issued call. */ + /** + * `offset`/`limit` are a 1-indexed start line plus a line count (verified + * against the reference `LocalPiReadExecutor`), which is exactly the local + * `read` tool's `:N+K` inline selector — the tool takes no range kwargs, so + * the range has to be composed onto the path or ranged reads silently + * return the whole file. + */ async piRead(call: Parameters>[0]) { - return await executeTool(this.options, "read", call.toolCallId, { path: call.args.path }); + const { path: readPath, offset, limit } = call.args; + const composed = piReadPath(readPath, offset, limit); + // A present `limit: 0` asks for zero lines. The reference slices an empty + // string for it; no `read` selector expresses that, so answer directly + // rather than falling back to a whole-file read. + if (composed === null) { + return createToolResultMessage(call.toolCallId, "read", { content: [{ type: "text", text: "" }] }, false); + } + return await executeTool(this.options, "read", call.toolCallId, { path: composed }); } async piBash(call: Parameters>[0]) { @@ -441,14 +457,21 @@ export class CursorExecHandlers implements ICursorExecHandlers { }); } + /** + * `literal` makes the pattern a fixed string; the local tool is regex-only, + * so the pattern is escaped on the way in (same translation the legacy pi + * shim does). `context` and `limit` have no tool-level equivalent — context + * width comes from `grep.contextBefore`/`grep.contextAfter` settings and the + * result count is bounded by the tool's own file/match caps. + */ async piGrep(call: Parameters>[0]) { - const { pattern, path, glob, ignoreCase } = call.args; + const { pattern, path, glob, ignoreCase, literal } = call.args; // Same arg mapping as the legacy `grep` handler: the local tool takes one // path spec, and its `case` flag is case-SENSITIVITY, the inverse of the // frame's `ignore_case`. return await executeTool(this.options, "grep", call.toolCallId, { - pattern, - path: glob ? `${path || "."}/${glob}` : path || ".", + pattern: literal === true ? piEscapeRegexLiteral(pattern) : pattern, + path: glob ? piJoinPath(path, glob) : path || ".", case: ignoreCase === true ? false : undefined, }); } @@ -457,16 +480,24 @@ export class CursorExecHandlers implements ICursorExecHandlers { * `pi_find` is a filename search, which is the local `glob` tool — not * `grep`. Its `pattern` is a glob, joined onto `path` because `glob` takes a * single combined path spec. + * + * `limit` is `optional int32`, so `0` is present rather than unset; the + * reference clamps it with `Math.max(1, limit ?? 1000)`, and an unset limit + * leaves the local tool's own default in place. */ async piFind(call: Parameters>[0]) { const { pattern, path, limit } = call.args; return await executeTool(this.options, "glob", call.toolCallId, { - path: path ? `${path}/${pattern}` : pattern, - limit: limit && limit > 0 ? limit : undefined, + path: piJoinPath(path, pattern), + limit: piLimit(limit), }); } - /** Redirected to `read`, which lists directories — same as the legacy `ls`. */ + /** + * Redirected to `read`, which lists directories — same as the legacy `ls`. + * `limit` has no tool-level equivalent; the listing is bounded by `read`'s + * own directory-entry caps. + */ async piLs(call: Parameters>[0]) { return await executeTool(this.options, "read", call.toolCallId, { path: call.args.path || "." }); } diff --git a/packages/coding-agent/test/cursor-exec.test.ts b/packages/coding-agent/test/cursor-exec.test.ts index 74a386147..60454c23c 100644 --- a/packages/coding-agent/test/cursor-exec.test.ts +++ b/packages/coding-agent/test/cursor-exec.test.ts @@ -485,9 +485,64 @@ describe("CursorExecHandlers Pi frame translation", () => { await handlers.piGrep({ toolCallId: "c1", args: { pattern: "x", path: "src", glob: "**/*.ts" } } as never); await handlers.piGrep({ toolCallId: "c2", args: { pattern: "x", glob: "**/*.ts" } } as never); + await handlers.piGrep({ toolCallId: "c3", args: { pattern: "x", path: ".", glob: "**/*.ts" } } as never); + await handlers.piGrep({ toolCallId: "c4", args: { pattern: "x", path: "src", glob: "/abs/**/*.ts" } } as never); expect((calls[0] as { path: string }).path).toBe("src/**/*.ts"); - expect((calls[1] as { path: string }).path).toBe("./**/*.ts"); + // An absent or "." path leaves the glob standing alone: a "./"-prefixed + // spec is a needlessly different path expression for the same scope. + expect((calls[1] as { path: string }).path).toBe("**/*.ts"); + expect((calls[2] as { path: string }).path).toBe("**/*.ts"); + // An absolute glob ignores the frame's path entirely. + expect((calls[3] as { path: string }).path).toBe("/abs/**/*.ts"); + }); + + it("composes pi_read's offset/limit onto the path as the read tool's range selector", async () => { + // `read` takes no range kwargs, so a dropped offset/limit silently returns + // the whole file. `offset` is a 1-indexed start and `limit` a line count, + // which is exactly the `:N+K` selector. + const { handlers, calls } = recordingHandlers("read"); + + await handlers.piRead({ toolCallId: "c1", args: { path: "a.ts", offset: 5, limit: 20 } } as never); + await handlers.piRead({ toolCallId: "c2", args: { path: "a.ts", offset: 5 } } as never); + await handlers.piRead({ toolCallId: "c3", args: { path: "a.ts", limit: 20 } } as never); + await handlers.piRead({ toolCallId: "c4", args: { path: "a.ts" } } as never); + // `optional int32`: a present 0 offset is not "unset". The reference + // clamps it to the first line rather than falling back to no range. + await handlers.piRead({ toolCallId: "c5", args: { path: "a.ts", offset: 0, limit: 20 } } as never); + + expect(calls).toEqual([ + { path: "a.ts:5+20" }, + { path: "a.ts:5-" }, + { path: "a.ts:1+20" }, + { path: "a.ts" }, + { path: "a.ts:1+20" }, + ]); + }); + + it("answers a present pi_read limit of zero with empty output instead of the whole file", async () => { + // `limit: 0` is present, not unset: the reference slices zero lines. No + // `read` selector expresses an empty range, so treating it as unset would + // return the entire file — the opposite of what was asked. + const { handlers, calls } = recordingHandlers("read"); + + const result = await handlers.piRead({ toolCallId: "c1", args: { path: "a.ts", limit: 0 } } as never); + + expect(calls).toEqual([]); + expect(result.isError).toBe(false); + expect(result.content).toEqual([{ type: "text", text: "" }]); + }); + + it("escapes pi_grep's pattern when the frame asks for a literal search", async () => { + // The local tool is regex-only, so an unescaped literal turns regex + // metacharacters into operators and matches the wrong lines. + const { handlers, calls } = recordingHandlers("grep"); + + await handlers.piGrep({ toolCallId: "c1", args: { pattern: "a.b(c)", literal: true } } as never); + await handlers.piGrep({ toolCallId: "c2", args: { pattern: "a.b(c)" } } as never); + + expect((calls[0] as { pattern: string }).pattern).toBe("a\\.b\\(c\\)"); + expect((calls[1] as { pattern: string }).pattern).toBe("a.b(c)"); }); it("routes pi_find to glob, not grep, joining its pattern onto the path", async () => { @@ -497,10 +552,14 @@ describe("CursorExecHandlers Pi frame translation", () => { await handlers.piFind({ toolCallId: "c1", args: { pattern: "*.ts", path: "src", limit: 10 } } as never); await handlers.piFind({ toolCallId: "c2", args: { pattern: "*.ts", limit: 0 } } as never); + await handlers.piFind({ toolCallId: "c3", args: { pattern: "*.ts" } } as never); expect(calls).toEqual([ { path: "src/*.ts", limit: 10 }, - // A zero limit is protobuf's unset, not a request for zero results. + // `optional int32`: a present 0 is clamped to 1 (as the reference + // does), not silently widened to the tool's default. + { path: "*.ts", limit: 1 }, + // Genuinely unset leaves the local tool's own default in place. { path: "*.ts", limit: undefined }, ]); });