fix(cursor): honor Pi frame arguments and preserve open-block args
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)
This commit is contained in:
committed by
can1357
parent
b6e01c8a3c
commit
7b62fef366
@@ -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
|
||||
|
||||
@@ -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<ToolCallState>(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<ToolCallState>(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);
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<NonNullable<ICursorExecHandlers["piRead"]>>[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<NonNullable<ICursorExecHandlers["piBash"]>>[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<NonNullable<ICursorExecHandlers["piGrep"]>>[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<NonNullable<ICursorExecHandlers["piFind"]>>[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<NonNullable<ICursorExecHandlers["piLs"]>>[0]) {
|
||||
return await executeTool(this.options, "read", call.toolCallId, { path: call.args.path || "." });
|
||||
}
|
||||
|
||||
@@ -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 },
|
||||
]);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user