diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index b3f1badc1..e835d1981 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -69,7 +69,7 @@ - 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 a Cursor `read_mcp_resource` call leaving no transcript block. The frame runs locally — and in download mode writes a workspace file — but synthesized no tool call and paired no result, so the read was invisible in the UI and absent from every rebuilt history; a resource download could mutate the workspace with nothing on record. The frame now synthesizes a `read_mcp_resource` block (not `read`: it is a remote MCP operation, and the name drives rendering and prune semantics) and pairs a result on success, not-found and error alike. Frames answered without a handler still synthesize nothing, since nothing ran. -- Fixed a Cursor MCP approval probe actually running the tool. A modern `mcpArgs` frame carrying `smart_mode_approval_only` asks only whether a call would be permitted — the server resolves the smart-mode decision before the real invocation and expects the dedicated `approved` reply. The decoder dropped the flag, so the frame ran a side-effecting MCP tool the user had not been asked about, then ran it again when the real call followed. The flag is now carried through and answered without executing or synthesizing a transcript block, since nothing ran. +- Fixed a Cursor MCP approval probe actually running the tool. A modern `mcpArgs` frame carrying `smart_mode_approval_only` asks only whether a call would be permitted, not for the call itself. The decoder dropped the flag, so the frame ran a side-effecting MCP tool the user had not been asked about, then ran it again when the real call followed. The flag is now carried through and the probe is answered from the host's policy without executing: approved only for a definite allow, refused for a deny, for a mode that demands a prompt the frame cannot raise, and for a tool the session does not have. No transcript block is synthesized either, since nothing ran. - 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 a Cursor stream dying mid-turn stranding the call it left open. `connect_scm` and native todo blocks are stamped resolved the moment they open, so the agent loop synthesizes no placeholder and only their completion frame pairs a result — a transport that closed first left the card animating and the call unpaired, which takes the whole interaction out of every rebuilt transcript. The terminal-error path now closes open blocks and pairs those server-owned calls with an interrupted result; the flush ran only on clean completion before, which is not the path a dying stream takes. Exec-settled MCP blocks are left alone, since the dispatch that ran them owns their result. - 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. diff --git a/packages/ai/src/providers/cursor-pi-args.ts b/packages/ai/src/providers/cursor-pi-args.ts index b546691b5..d7b759f5a 100644 --- a/packages/ai/src/providers/cursor-pi-args.ts +++ b/packages/ai/src/providers/cursor-pi-args.ts @@ -47,6 +47,39 @@ export function piReadPath(readPath: string, offset?: number, limit?: number): s return count === undefined ? `${readPath}:raw:${start}-` : `${readPath}:raw:${start}+${count}`; } +/** + * The same range as {@link piReadPath}, rendered for a transcript block rather + * than for execution. + * + * Differs only at `limit: 0`, where `piReadPath` returns `null` because no + * selector reads zero lines and the frame is answered with empty output + * directly. The block still has to say so: falling back to the bare path there + * would record a whole-file read whose result is empty, which is the widest + * possible gap between what a rebuilt transcript shows and what happened. + * `+0` is never executed — it exists to be read. + */ +export function piReadDisplayPath(readPath: string, offset?: number, limit?: number): string { + const composed = piReadPath(readPath, offset, limit); + if (composed !== null) return composed; + const start = offset !== undefined ? Math.max(1, Math.floor(offset)) : 1; + return `${readPath}:raw:${start}+0`; +} + +/** + * A legacy `grep` frame's pagination `offset` as the local tool's file `skip`. + * + * `grep` paginates by file and reports "use skip=N for the next page" in that + * same unit, so the offset maps across directly. A present `0` means "start at + * the beginning", which is the unskipped search rather than a skip of zero. + * + * Shared because both the executing bridge and the provider's transcript + * synthesis need it: a block showing an unskipped search beside a result from + * a later file window misreports what was searched. + */ +export function piGrepSkip(offset?: number): number | undefined { + return offset !== undefined && offset > 0 ? Math.floor(offset) : undefined; +} + /** * Join a Pi frame's optional `path` with the `glob`/`pattern` it scopes. * diff --git a/packages/ai/src/providers/cursor.ts b/packages/ai/src/providers/cursor.ts index a4d9e0681..0834af9d5 100644 --- a/packages/ai/src/providers/cursor.ts +++ b/packages/ai/src/providers/cursor.ts @@ -78,6 +78,7 @@ import { McpApprovedSchema, McpErrorSchema, McpImageContentSchema, + McpRejectedSchema, McpResultSchema, McpSuccessSchema, McpTextContentSchema, @@ -200,10 +201,11 @@ import { buildPiWriteError, buildPiWriteResult, piEscapeRegexLiteral, + piGrepSkip, piJoinPath, piLimit, piLsPath, - piReadPath, + piReadDisplayPath, piTimeout, } from "./cursor/exec-modern"; @@ -1295,7 +1297,12 @@ async function handleExecServerMessage( case "readArgs": { const args = execMsg.message.value; if (!args.toolCallId) args.toolCallId = crypto.randomUUID(); - synthesizeCursorExecToolCall(output, stream, state, args.toolCallId, "read", { path: args.path }); + // The same composed selector the bridge executes: showing a bare path + // for a ranged read makes the returned slice look like the whole + // file in every rebuilt transcript. + synthesizeCursorExecToolCall(output, stream, state, args.toolCallId, "read", { + path: piReadDisplayPath(args.path, args.offset, args.limit), + }); const { execResult } = await resolveExecHandler( args, execHandlers?.read?.bind(execHandlers), @@ -1354,6 +1361,7 @@ async function handleExecServerMessage( pattern: args.pattern, path: searchPath, case: args.caseInsensitive === true ? false : undefined, + skip: piGrepSkip(args.offset), }); const { execResult } = await resolveExecHandler( args, @@ -1516,19 +1524,33 @@ async function handleExecServerMessage( case "mcpArgs": { const args = execMsg.message.value; const mcpCall = decodeMcpCall(args); - // An approval probe, not an invocation: the server is resolving a - // smart-mode permission decision before the real call and answers - // with the dedicated `approved` variant. Executing here would fire a - // side-effecting MCP tool the user has not been asked about yet — - // and fire it a second time when the real frame follows. No block is - // synthesized either: nothing ran, so a transcript entry would claim - // work that never happened. + // An approval probe, not an invocation: the frame asks whether the + // call would be permitted. Running the tool to find out fires a side + // effect the user has not been asked about, and fires it again when + // the real frame follows — so this must answer without executing. + // + // The host resolves it against the same policy the wrapper applies at + // execution time. Only a definite allow is approved: a pending prompt + // cannot be asked through this frame, and answering yes on its behalf + // would pre-authorize a call the user never saw. Without a handler + // there is nothing to decide with, so it is refused. Either way no + // block is synthesized — nothing ran. if (mcpCall.approvalOnly) { + const approved = (await execHandlers?.mcpApprovalPreflight?.(mcpCall)) === true; sendExecClientMessage( h2Request, execMsg, "mcpResult", - create(McpResultSchema, { result: { case: "approved", value: create(McpApprovedSchema, {}) } }), + create(McpResultSchema, { + result: approved + ? { case: "approved", value: create(McpApprovedSchema, {}) } + : { + case: "rejected", + value: create(McpRejectedSchema, { + reason: `Tool "${mcpCall.toolName || mcpCall.name}" is not approved to run without asking.`, + }), + }, + }), ); return; } @@ -1720,7 +1742,7 @@ async function handleExecServerMessage( // 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, + path: piReadDisplayPath(args.path, args.offset, args.limit), }); const { execResult } = await resolveExecHandler( { args, toolCallId }, @@ -2358,9 +2380,10 @@ function buildReadResultFromToolResult(path: string, toolResult: ToolResultMessa fileSize: BigInt(Buffer.byteLength(text, "utf-8")), truncated: toolResultWasTruncated(toolResult), output: { case: "content", value: text }, - // Whether the frame's window was honored. Left false for an - // unranged read: the server reads it as "this is the whole file", - // which is exactly true when no range was asked for. + // Set when this client composed the frame's window onto the read, + // left false when it read the file whole. The proto names the + // field but nothing here pins the server's use of it, so the only + // safe contract is that it describes what we actually did. rangeApplied, }), }, @@ -2615,10 +2638,10 @@ function buildGrepResultFromToolResult( totalFiles: files.length, clientTruncated, ripgrepTruncated: false, - // Echoes the frame's own optional field: present means "this - // page starts where you asked", absent means no offset was - // requested. Without it the server cannot tell a honored - // offset from a client that ignored it, and re-paginates. + // Echoes the offset this client actually applied; absent when + // the frame requested none. The proto names the field but + // nothing here pins the server's use of it, so it reports what + // we did rather than asserting a pagination protocol. offsetApplied: args.offset, }), }, diff --git a/packages/ai/src/providers/cursor/exec-modern.ts b/packages/ai/src/providers/cursor/exec-modern.ts index 4d0c589c7..1bf56f446 100644 --- a/packages/ai/src/providers/cursor/exec-modern.ts +++ b/packages/ai/src/providers/cursor/exec-modern.ts @@ -73,9 +73,11 @@ import type { ToolResultMessage } from "../../types"; */ export { piEscapeRegexLiteral, + piGrepSkip, piJoinPath, piLimit, piLsPath, + piReadDisplayPath, piReadPath, piTimeout, } from "../cursor-pi-args"; diff --git a/packages/ai/src/types.ts b/packages/ai/src/types.ts index 2b30830ff..fc822e798 100644 --- a/packages/ai/src/types.ts +++ b/packages/ai/src/types.ts @@ -1072,6 +1072,20 @@ export interface CursorExecHandlers { ) => Promise>; diagnostics?: (args: DiagnosticsArgs) => Promise>; mcp?: (call: CursorMcpCall) => Promise>; + /** + * Answers "would this MCP call be permitted", without running it. + * + * A modern `mcpArgs` frame carrying `smart_mode_approval_only` asks for the + * permission decision alone, ahead of the real invocation. Executing the + * tool to answer it would fire a side effect the user never approved — and + * fire it twice once the real call arrives. + * + * `true` only when the host's policy resolves to a definite allow. A pending + * prompt is `false`: it can only be answered interactively at execution + * time, and there is no "ask me later" reply in this frame's result. When no + * handler is registered the provider refuses, since it cannot decide. + */ + mcpApprovalPreflight?: (call: CursorMcpCall) => Promise; /** * Modern Cursor CLI Pi tool frames (`ExecServerMessage` 45-51). They are a * distinct frame family from the legacy `readArgs`/`shellArgs`/... set, not diff --git a/packages/ai/test/cursor-exec-modern.test.ts b/packages/ai/test/cursor-exec-modern.test.ts index ff3da4b2a..d798d73c1 100644 --- a/packages/ai/test/cursor-exec-modern.test.ts +++ b/packages/ai/test/cursor-exec-modern.test.ts @@ -1538,10 +1538,9 @@ describe("Cursor modern exec frames: server-resolved tool calls leave a paired b describe("Cursor legacy read frame: range reporting", () => { it("reports rangeApplied only when the frame asked for a window", async () => { - // `range_applied: false` tells the server "this is the whole file", which - // is true for an unranged read and a lie for a paginated one — a model - // walking a large file would stop after the first window believing it had - // seen everything. + // The field reports whether the frame's window was actually composed + // onto the read. Reporting true for an unranged whole-file read, or + // false for a paginated one, misdescribes what the client did. const handlers: CursorExecHandlers = { async read() { return toolResult("line1\nline2"); @@ -1572,13 +1571,52 @@ describe("Cursor legacy read frame: range reporting", () => { if (wholeAnswer.value.result.case !== "success") throw new Error(`got ${wholeAnswer.value.result.case}`); expect(wholeAnswer.value.result.value.rangeApplied).toBe(false); }); + + it("carries the composed selector into the synthesized call", async () => { + // A bare path beside a ranged result makes the slice look like the whole + // file in every rebuilt transcript. + const { output } = await dispatchExec( + buildExecMessage({ + case: "readArgs", + value: create(ReadArgsSchema, { path: "/repo/big.ts", toolCallId: "c1", offset: 5, limit: 20 }), + }), + { + execHandlers: { + async read() { + return toolResult("line1\nline2"); + }, + }, + }, + ); + const call = output.content.find(block => block.type === "toolCall"); + expect(call?.arguments).toMatchObject({ path: "/repo/big.ts:raw:5+20" }); + }); + it("records a zero-line read as zero lines, not the whole file", async () => { + // `limit: 0` composes no selector — nothing reads zero lines — and the + // frame is answered with empty output directly. A bare path in the block + // would replay as a whole-file read that came back empty. + const { output } = await dispatchExec( + buildExecMessage({ + case: "readArgs", + value: create(ReadArgsSchema, { path: "/repo/big.ts", toolCallId: "c1", offset: 5, limit: 0 }), + }), + { + execHandlers: { + async read() { + return toolResult(""); + }, + }, + }, + ); + const call = output.content.find(block => block.type === "toolCall"); + expect(call?.arguments).toMatchObject({ path: "/repo/big.ts:raw:5+0" }); + }); }); describe("Cursor legacy grep frame: offset reporting", () => { it("echoes the frame's offset back as offsetApplied", async () => { - // `offset_applied` is how the server learns the page it asked for is the - // page it got. Left unset, a honored offset is indistinguishable from a - // client that ignored it, so Cursor re-paginates from the same place. + // The field reports the offset this client actually applied, so it must + // track the frame's request rather than being left unset. const handlers: CursorExecHandlers = { async grep() { return toolResult("a.ts:1:needle"); @@ -1614,14 +1652,35 @@ describe("Cursor legacy grep frame: offset reporting", () => { // Absent, not 0: no offset was requested, so none was applied. expect(firstUnion.value.offsetApplied).toBeUndefined(); }); + + it("carries the executed page into the synthesized call", async () => { + // The block is what a reloaded transcript replays. Showing an unskipped + // search beside a result taken from a later file window tells the next + // turn the wrong thing about what was searched. + const { output } = await dispatchExec( + buildExecMessage({ + case: "grepArgs", + value: create(GrepArgsSchema, { pattern: "needle", path: "src", toolCallId: "c1", offset: 20 }), + }), + { + execHandlers: { + async grep() { + return toolResult("a.ts:1:needle"); + }, + }, + }, + ); + const call = output.content.find(block => block.type === "toolCall"); + expect(call?.arguments).toMatchObject({ pattern: "needle", path: "src", skip: 20 }); + }); }); describe("Cursor MCP frame: approval-only probes", () => { - it("answers an approval-only frame without running the tool", async () => { - // The server sends this to resolve a smart-mode permission decision - // BEFORE the real call. Running the tool here fires a side effect the - // user has not been asked about, and fires it again when the real frame - // arrives. + it("answers from the host's policy without running the tool", async () => { + // The server sends this to resolve a permission decision BEFORE the real + // call. Running the tool to answer it fires a side effect the user has + // not been asked about, and fires it again when the real frame arrives. + // The verdict comes from the host's policy instead. let invocations = 0; const handlers: CursorExecHandlers = { async mcp() { @@ -1647,12 +1706,81 @@ describe("Cursor MCP frame: approval-only probes", () => { expect(invocations).toBe(0); const answer = soleResult(frames); if (answer.case !== "mcpResult") throw new Error(`got ${answer.case}`); - expect(answer.value.result.case).toBe("approved"); + // No preflight handler is registered here: with nothing to decide + // against, the probe cannot be approved. + expect(answer.value.result.case).toBe("rejected"); // Nothing ran, so nothing may appear in the transcript either. expect(output.content.filter(block => block.type === "toolCall")).toHaveLength(0); expect(results).toHaveLength(0); }); + it("approves a probe the host's policy allows", async () => { + // A definite allow must still be approved, or a permitted call loses the + // fast path on every turn. + let invocations = 0; + const { frames, output } = await dispatchExec( + buildExecMessage({ + case: "mcpArgs", + value: create(McpArgsSchema, { + name: "lookup", + toolName: "lookup", + toolCallId: "c1", + providerIdentifier: "ops", + smartModeApprovalOnly: true, + }), + }), + { + execHandlers: { + async mcp() { + invocations += 1; + return toolResult("ran"); + }, + async mcpApprovalPreflight() { + return true; + }, + }, + }, + ); + + // Approved, but still not executed: the call itself comes later. + expect(invocations).toBe(0); + const answer = soleResult(frames); + if (answer.case !== "mcpResult") throw new Error(`got ${answer.case}`); + expect(answer.value.result.case).toBe("approved"); + expect(output.content.filter(block => block.type === "toolCall")).toHaveLength(0); + }); + + it("refuses a probe the host declines to approve", async () => { + // A pending prompt resolves to false: it can only be answered at + // execution time, and approving on the user's behalf pre-authorizes a + // call they never saw. + const { frames } = await dispatchExec( + buildExecMessage({ + case: "mcpArgs", + value: create(McpArgsSchema, { + name: "deploy", + toolName: "deploy", + toolCallId: "c1", + providerIdentifier: "ops", + smartModeApprovalOnly: true, + }), + }), + { + execHandlers: { + async mcp() { + return toolResult("ran"); + }, + async mcpApprovalPreflight() { + return false; + }, + }, + }, + ); + const answer = soleResult(frames); + if (answer.case !== "mcpResult") throw new Error(`got ${answer.case}`); + expect(answer.value.result.case).toBe("rejected"); + }); + it("still runs an ordinary MCP frame", async () => { // The guard keys on the flag alone: a normal call must be unaffected. let invocations = 0; diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 5bb5b03c0..114c0aa62 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -152,8 +152,9 @@ - Fixed `pi_grep` returning fewer matches than it asked for when they spread across many files. The local `grep` windows results to the first 20 files and tells the caller to paginate with `skip`, but `PiGrepExecArgs` has no `skip` field — so a frame asking for 100 matches over 25 one-match files got 20, `match_limit_reached` unset, and advice it could not act on: output silently short and labelled complete. A search carrying a total match cap now reads enough files to satisfy it (cap+1, so a result landing exactly on the cap is distinguishable from a clipped one) and reports the cap when it actually bites. - Fixed every native `pi_edit` failing after a session switched onto Cursor. The replace-mode `edit` instance the frame needs was built only for sessions *created* on Cursor, and the tool roster is not rebuilt on a model switch — so a session that started elsewhere kept its configured-mode `edit` in the registry, which the bridge resolves before its fallback, and the frame's `old_text`/`new_text` pairs failed validation against a `hashline` schema. The instance is now built from the `edit` grant regardless of the session's initial provider (lazily, so a session that never reaches Cursor never constructs one) and `pi_edit` asks for it explicitly through a dedicated accessor. A session that was never granted `edit` is still refused. - Fixed the Cursor bridge's tool resolver being able to execute an unadvertised `edit`. That resolver doubles as the agent loop's fallback for any call outside the advertised set, so serving `edit` from it meant a hallucinated call — or one naming a tool the session deselected after startup — could run a replace-mode edit the model was never offered. It is device-only again; `pi_edit` uses its own accessor. -- Fixed the legacy Cursor `read` frame ignoring the `offset`/`limit` modern builds paginate with. Only the Pi variant composed a range, so every page of a legacy read returned the whole file (or its own truncation) and a model walking a large file never advanced past the first window. Both frames now translate a range through the same helper, and the answer reports `range_applied` — left false for an unranged read, which is the server's "this is the whole file". -- Fixed the legacy Cursor `grep` frame ignoring its pagination `offset`. The local `grep` paginates by file through `skip` and advertises exactly that in its own "use skip=N" advice, so an unforwarded offset re-ran the identical search and answered page one for every page. The answer now echoes `offset_applied`, without which a honored offset is indistinguishable from a client that ignored it. +- Fixed the legacy Cursor `read` frame ignoring the `offset`/`limit` modern builds paginate with. Only the Pi variant composed a range, so every page of a legacy read returned the whole file (or its own truncation) and a model walking a large file never advanced past the first window. Both frames now translate a range through the same helper, and the answer sets `range_applied` to describe whether a window was actually composed. +- Fixed the legacy Cursor `grep` frame ignoring its pagination `offset`. The local `grep` paginates by file through `skip` and advertises exactly that in its own "use skip=N" advice, so an unforwarded offset re-ran the identical search and answered page one for every page. The answer now reports the offset it applied in `offset_applied`. +- Fixed a paginated Cursor `read` or `grep` frame being recorded as an unpaginated one. The executed call and the transcript block are built separately, so forwarding the frame's range and page fixed only the execution: the block still showed a bare path and an unskipped search, which is what a reloaded session replays and what the next turn reasons from — a slice of a file presented as the whole thing, and results from a later window presented as page one. Both are now synthesized from the same translation that runs them, including a `limit: 0` read, which is recorded as the zero lines it returns rather than a whole-file read. - Fixed advisor tools bypassing the approval gate. They are built straight from the builtin table, outside the loop that wraps every registry tool, and both the advisor's own agent loop and its Cursor exec bridge (`pi_write`, `pi_bash`) run those instances directly — so an advisor granted `write` or `bash` executed them regardless of a configured `ask` or `deny`. They now carry the same `ExtensionToolWrapper` as every other tool. ## [17.1.5] - 2026-07-27 diff --git a/packages/coding-agent/src/cursor.ts b/packages/coding-agent/src/cursor.ts index 2e1d96413..87568cfb2 100644 --- a/packages/coding-agent/src/cursor.ts +++ b/packages/coding-agent/src/cursor.ts @@ -19,6 +19,7 @@ import type { } from "@oh-my-pi/pi-ai"; import { piEscapeRegexLiteral, + piGrepSkip, piJoinPath, piLimit, piLsPath, @@ -434,12 +435,11 @@ export class CursorExecHandlers implements ICursorExecHandlers { async grep(args: Parameters>[0]) { const toolCallId = decodeToolCallId(args.toolCallId); const searchPath = args.glob ? `${args.path || "."}/${args.glob}` : args.path || "."; - const skip = args.offset !== undefined && args.offset > 0 ? Math.floor(args.offset) : undefined; const toolResultMessage = await executeTool(this.options, "grep", toolCallId, { pattern: args.pattern, path: searchPath, case: args.caseInsensitive === true ? false : undefined, - skip, + skip: piGrepSkip(args.offset), }); return toolResultMessage; } @@ -910,4 +910,30 @@ export class CursorExecHandlers implements ICursorExecHandlers { const toolResultMessage = await executeTool(this.options, toolName, toolCallId, args); return toolResultMessage; } + + /** + * Resolve an MCP call's approval without running it. + * + * Same resolution the wrapper applies at execution time, minus the + * execution: an unknown tool is not approvable, and a `prompt` is not an + * approval — the frame has no way to carry an interactive question, and the + * user is asked for real when the call itself arrives. + */ + async mcpApprovalPreflight(call: CursorMcpCall) { + const toolName = call.toolName || call.name; + const tool = this.options.tools.get(toolName) ?? this.options.getTool?.(toolName); + if (!tool) return false; + const context = this.options.getToolContext?.(); + const settings = context?.settings; + const approvalMode: ApprovalMode = + context?.autoApprove === true ? "yolo" : (settings?.get("tools.approvalMode") ?? "yolo"); + const args = Object.keys(call.args ?? {}).length > 0 ? call.args : decodeMcpArgs(call.rawArgs ?? {}); + const approval = resolveApproval( + tool, + args, + approvalMode, + (settings?.get("tools.approval") ?? {}) as Record, + ); + return approval.policy === "allow"; + } } diff --git a/packages/coding-agent/test/cursor-exec.test.ts b/packages/coding-agent/test/cursor-exec.test.ts index 5d819a92f..18365e7d4 100644 --- a/packages/coding-agent/test/cursor-exec.test.ts +++ b/packages/coding-agent/test/cursor-exec.test.ts @@ -1385,6 +1385,84 @@ describe("CursorExecHandlers native delete gating (issue #5680)", () => { }); }); +// A `smart_mode_approval_only` frame asks whether an MCP call would be allowed, +// ahead of the call itself. The verdict has to come from the same policy that +// gates execution — without running the tool to find out. +describe("CursorExecHandlers MCP approval preflight", () => { + let cwd: string; + + beforeEach(async () => { + cwd = await fs.mkdtemp(path.join(os.tmpdir(), "cursor-preflight-test-")); + }); + + afterEach(async () => { + await removeWithRetries(cwd); + }); + + function mcpHandlers(settings: Settings): { handlers: CursorExecHandlers; executed: () => number } { + let executed = 0; + const tool: AgentTool = { + name: "mcp__ops__deploy", + label: "deploy", + description: "", + parameters: type({}), + execute: async () => { + executed += 1; + return { content: [{ type: "text", text: "ran" }] }; + }, + } as unknown as AgentTool; + const handlers = new CursorExecHandlers({ + cwd, + tools: new Map([[tool.name, tool]]), + getToolContext: () => ({ settings }) as AgentToolContext, + }); + return { handlers, executed: () => executed }; + } + + const call = { + name: "mcp__ops__deploy", + toolName: "mcp__ops__deploy", + toolCallId: "c1", + providerIdentifier: "ops", + args: {}, + rawArgs: {}, + }; + + it("approves a call the policy allows, without running it", async () => { + const { handlers, executed } = mcpHandlers(Settings.isolated({ "tools.approvalMode": "yolo" })); + + expect(await handlers.mcpApprovalPreflight(call)).toBe(true); + // Approval is the answer; the invocation is a separate frame. + expect(executed()).toBe(0); + }); + + it("refuses a call the user's policy denies", async () => { + const { handlers, executed } = mcpHandlers( + Settings.isolated({ "tools.approvalMode": "yolo", "tools.approval": { mcp__ops__deploy: "deny" } }), + ); + + // Approving here would launder the deny into a server-side blessing. + expect(await handlers.mcpApprovalPreflight(call)).toBe(false); + expect(executed()).toBe(0); + }); + + it("refuses when the policy demands a prompt this frame cannot raise", async () => { + const { handlers } = mcpHandlers(Settings.isolated({ "tools.approvalMode": "always-ask" })); + + // The user is asked for real when the call arrives; answering yes on + // their behalf pre-authorizes something they never saw. + expect(await handlers.mcpApprovalPreflight(call)).toBe(false); + }); + + it("refuses a tool the session does not have", async () => { + const { handlers } = mcpHandlers(Settings.isolated({ "tools.approvalMode": "yolo" })); + + expect( + await handlers.mcpApprovalPreflight({ ...call, name: "mcp__ops__absent", toolName: "mcp__ops__absent" }), + ).toBe(false); + }); +}); + // The Pi frames (`ExecServerMessage` 45-51) are a separate wire family from the // legacy `read`/`shell`/`grep` args, with different field names and different // semantics. Each bridge handler therefore performs a real translation, and a