diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 0f613b605..a30ff0ce8 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -78,7 +78,7 @@ ### Fixed - Fixed four Cursor exec frames answering with a result whose oneof was never set. In proto3 that is not an empty result — the server reads it as "the tool ran and produced nothing", indistinguishable from real success. `listMcpResourcesExecResult`, `readMcpResourceExecResult`, `recordScreenResult` and `computerUseResult` now send `ListMcpResourcesSuccess{resources: []}`, `ReadMcpResourceNotFound{uri}`, `RecordScreenFailure` and `ComputerUseError` respectively. -- The MCP resource frames now answer from the host instead of a fixed verdict. `CursorExecHandlers` gained `listMcpResources`/`readMcpResource`, so a host holding live MCP connections advertises them; the empty catalog and `not_found` above remain the answer when no handler is supplied. A handler that throws surfaces as `ListMcpResourcesError`/`ReadMcpResourceError` rather than collapsing into "none exist", which the model cannot retry. +- The MCP resource frames now answer from the host instead of a fixed verdict. `CursorExecHandlers` gained `listMcpResources`/`readMcpResource`, so a host holding live MCP connections advertises them; the empty catalog and `not_found` above remain the answer when no handler is supplied. A handler that throws surfaces as `ListMcpResourcesError`/`ReadMcpResourceError` rather than collapsing into "none exist", which the model cannot retry. A read carrying `download_path` forwards it and answers with `ReadMcpResourceSuccess.download_path` and no content, which is what that mode means. - 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. diff --git a/packages/ai/src/providers/cursor.ts b/packages/ai/src/providers/cursor.ts index 19b58f5dc..94f2b5a4c 100644 --- a/packages/ai/src/providers/cursor.ts +++ b/packages/ai/src/providers/cursor.ts @@ -1584,7 +1584,11 @@ async function handleExecServerMessage( // `null` is the handler's "no such server or uri", which is exactly // `not_found`; a throw is a real failure and must not masquerade as // a missing resource. - const content = await execHandlers?.readMcpResource?.({ server: args.server, uri: args.uri }); + const content = await execHandlers?.readMcpResource?.({ + server: args.server, + uri: args.uri, + downloadPath: args.downloadPath, + }); execResult = content ? create(ReadMcpResourceExecResultSchema, { result: { @@ -1594,14 +1598,19 @@ async function handleExecServerMessage( name: content.name, description: content.description, mimeType: content.mimeType, - // The wire's content oneof carries one of the two; text - // wins when a host supplies both. + downloadPath: content.downloadPath, + // A download returns no content to the model: the file is + // on disk and the path is the answer. Otherwise the wire's + // content oneof carries one of the two, text winning when + // a host supplies both. content: - content.text !== undefined - ? { case: "text", value: content.text } - : content.blob !== undefined - ? { case: "blob", value: content.blob } - : { case: undefined }, + content.downloadPath !== undefined + ? { case: undefined } + : content.text !== undefined + ? { case: "text", value: content.text } + : content.blob !== undefined + ? { case: "blob", value: content.blob } + : { case: undefined }, }), }, }) diff --git a/packages/ai/src/types.ts b/packages/ai/src/types.ts index f18041f8c..56f23ea8f 100644 --- a/packages/ai/src/types.ts +++ b/packages/ai/src/types.ts @@ -1032,7 +1032,9 @@ export interface CursorMcpResource { * The content of one resource read. * * `text` and `blob` are the wire's content oneof: exactly one is sent, with - * `text` winning when a host supplies both. + * `text` winning when a host supplies both. A download instead sets + * `downloadPath` and no content at all — the model is told where the file + * landed rather than being handed its bytes. */ export interface CursorMcpResourceContent { uri: string; @@ -1041,6 +1043,12 @@ export interface CursorMcpResourceContent { mimeType?: string; text?: string; blob?: Uint8Array; + /** + * Where the host wrote the resource, workspace-relative, when the frame + * asked for a download. Set this INSTEAD of `text`/`blob`: the wire + * contract is that a download returns no content to the model. + */ + downloadPath?: string; } export interface CursorExecHandlers { @@ -1078,7 +1086,15 @@ export interface CursorExecHandlers { * Read one resource. `null` means the server or uri is genuinely unknown, * which the provider answers as `not_found`; throwing surfaces as `error`. */ - readMcpResource?: (args: { server: string; uri: string }) => Promise; + readMcpResource?: (args: { + server: string; + uri: string; + /** + * When set, write the resource here (workspace-relative) and return + * `downloadPath` instead of content. + */ + downloadPath?: string; + }) => Promise; /** Mirror Cursor's server-owned todo list into local session state. */ todoSync?: CursorTodoSyncHandler; onToolResult?: CursorToolResultHandler; diff --git a/packages/ai/test/cursor-exec-modern.test.ts b/packages/ai/test/cursor-exec-modern.test.ts index f3fd50ecf..8ff570343 100644 --- a/packages/ai/test/cursor-exec-modern.test.ts +++ b/packages/ai/test/cursor-exec-modern.test.ts @@ -581,6 +581,40 @@ describe("Cursor MCP resource frames answer from the host's servers", () => { expect(answer.value.result.value.content).toEqual({ case: "text", value: "# Title" }); }); + it("forwards a download request and answers with the path, not the content", async () => { + // `download_path` is a different contract: the host writes the resource + // to that workspace-relative path and the model is told where it landed. + // Returning content anyway would put the payload back in context, which + // is exactly what the download mode exists to avoid. + let sawDownloadPath: string | undefined; + const { frames } = await dispatchExec( + buildExecMessage({ + case: "readMcpResourceExecArgs", + value: create(ReadMcpResourceExecArgsSchema, { + server: "files", + uri: "files://logo", + downloadPath: "assets/logo.png", + }), + }), + { + execHandlers: { + readMcpResource: async ({ uri, downloadPath }) => { + sawDownloadPath = downloadPath; + // A host that also has the bytes on hand must still not have + // them forwarded: the download path is the whole answer. + return { uri, mimeType: "image/png", downloadPath, text: "inline payload" }; + }, + }, + }, + ); + expect(sawDownloadPath).toBe("assets/logo.png"); + const answer = soleResult(frames); + if (answer.case !== "readMcpResourceExecResult") throw new Error(`got ${answer.case}`); + if (answer.value.result.case !== "success") throw new Error(`got ${answer.value.result.case}`); + expect(answer.value.result.value.downloadPath).toBe("assets/logo.png"); + expect(answer.value.result.value.content.case).toBeUndefined(); + }); + it("distinguishes a missing resource from a failing host", async () => { // `null` is "no such server or uri", which is `not_found`. A throw is a // real failure and must not masquerade as a missing resource — the model diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1f8e7e740..60a873dbb 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -143,7 +143,7 @@ - Fixed `pi_bash` killing commands that explicitly asked for no deadline. `timeout` is `optional int32` and `bash` documents `0` as "disables the command deadline", but a truthiness check folded a supplied `0` into unset, applying the 300s default instead. A present `0` now passes through; negatives, which have no local meaning and would otherwise clamp to the 1s floor, still fall back to the default. - Fixed the Cursor exec bridge granting `edit` and `grep` to sessions that withheld them. Both bridge-only tools are constructed rather than looked up, and `executeTool` prefers a constructed override over the registry, so a restricted tool set (`toolNames` without them, or `restrictToolNames`) still got a working `pi_edit`/`pi_grep` — native frames arrive regardless of the advertised catalog. Both are now gated on the session having actually granted the tool, matching the `delete` frame's existing check (issue #5680). - Fixed Cursor advisor bridge tools bypassing approval settings. The advisor's `pi_edit`/`pi_grep` instances are approval-wrapped, but the wrapper reads `tools.approvalMode`, per-tool `tools.approval.` policies and `autoApprove` only from the execute-time tool context — which the advisor bridge never supplied, so every native advisor frame resolved as `yolo` with empty policies and ran past a configured `ask` or `deny`. Advisors now receive the same context store as the primary bridge. -- Fixed Cursor's `list_mcp_resources`/`read_mcp_resource` frames answering as though the client hosted no MCP servers. The bridge hardcoded an empty catalog and `not_found`, so resources from servers the session held live connections to were invisible to the model even while the same session read them through `mcp://`. Both frames now answer from the session's `MCPManager`; a lookup failure surfaces as an error rather than an empty catalog, which would read as "asked, none exist". +- Fixed Cursor's `list_mcp_resources`/`read_mcp_resource` frames answering as though the client hosted no MCP servers. The bridge hardcoded an empty catalog and `not_found`, so resources from servers the session held live connections to were invisible to the model even while the same session read them through `mcp://`. Both frames now answer from the session's `MCPManager`; a lookup failure surfaces as an error rather than an empty catalog, which would read as "asked, none exist". A read carrying `download_path` writes the resource to that workspace-relative path and answers with the path alone, per the wire contract, instead of putting the payload back in the model's context. - Fixed the Cursor native `delete` frame bypassing approval settings. Unlike every other frame it removes the file directly instead of running a registry tool, so no approval wrapper sat in front of it — `allowNativeDelete` answers whether a mutating tool was granted, which is a different question from whether the user's policy allows the call. A configured `tools.approval.delete: deny`, or an `always-ask` session that this channel cannot prompt in, now refuses the frame and keeps the file. - Fixed `pi_ls` never reporting that a listing was clipped. The bridge read the entry cap from a flat `details.resultLimitReached`, which `glob` sets but `read` — the tool serving `pi_ls` — does not: it records the cap through `OutputMeta` at `details.meta.limits.resultLimit.reached`. Every capped listing therefore reached Cursor with `entry_limit_reached` unset, reading as complete. Both shapes are now checked, the same way the truncation translation already handles its two producers. diff --git a/packages/coding-agent/src/cursor.ts b/packages/coding-agent/src/cursor.ts index 72d99a491..5962e08a5 100644 --- a/packages/coding-agent/src/cursor.ts +++ b/packages/coding-agent/src/cursor.ts @@ -620,19 +620,43 @@ export class CursorExecHandlers implements ICursorExecHandlers { * blob. Text items are joined, since a multi-part text resource is one * document; otherwise the first blob stands in. `blob` arrives base64 and * the wire wants bytes. + * + * A `downloadPath` frame is a different contract: write the bytes to that + * workspace-relative path and answer with the path alone, so a large binary + * lands on disk instead of in the model's context. */ - async readMcpResource({ server, uri }: { server: string; uri: string }): Promise { + async readMcpResource({ + server, + uri, + downloadPath, + }: { + server: string; + uri: string; + downloadPath?: string; + }): Promise { const mcp = this.options.mcpResources; if (!mcp) return null; const read = await mcp.readServerResource(server, uri); if (!read) return null; + const mimeType = read.contents[0]?.mimeType; const texts = read.contents.filter(item => item.text !== undefined).map(item => item.text as string); - if (texts.length > 0) { - return { uri, mimeType: read.contents[0]?.mimeType, text: texts.join("\n") }; - } const blob = read.contents.find(item => item.blob !== undefined)?.blob; + + if (downloadPath) { + // Text resources download as their own bytes; a blob decodes first. + const payload = + texts.length > 0 ? texts.join("\n") : blob !== undefined ? Buffer.from(blob, "base64") : undefined; + if (payload === undefined) return null; + const absolutePath = resolveToCwd(downloadPath, this.options.getCwd?.() ?? this.options.cwd); + await Bun.write(absolutePath, payload); + // The path echoed back is the one the frame asked for: it is + // workspace-relative by contract, and the model addresses it that way. + return { uri, mimeType, downloadPath }; + } + + if (texts.length > 0) return { uri, mimeType, text: texts.join("\n") }; if (blob === undefined) return null; - return { uri, mimeType: read.contents[0]?.mimeType, blob: Buffer.from(blob, "base64") }; + return { uri, mimeType, blob: Buffer.from(blob, "base64") }; } /** diff --git a/packages/coding-agent/test/cursor-exec.test.ts b/packages/coding-agent/test/cursor-exec.test.ts index 8376dced4..de0866a5c 100644 --- a/packages/coding-agent/test/cursor-exec.test.ts +++ b/packages/coding-agent/test/cursor-exec.test.ts @@ -695,6 +695,41 @@ describe("CursorExecHandlers mounted tool bridge", () => { expect(await handlers.readMcpResource({ server: "files", uri: "files://missing" })).toBeNull(); }); + it("writes a download to the workspace path and returns no content", async () => { + // The frame's `download_path` means "put it on disk, don't hand me the + // bytes". Reporting success without creating the file leaves the model + // pointing at nothing. + const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "cursor-mcp-download-")); + try { + const handlers = new CursorExecHandlers({ + cwd: workspace, + tools: new Map(), + mcpResources: { + serverNames: () => ["files"], + getServerResources: () => undefined, + readServerResource: async (_name, uri) => ({ + contents: [{ uri, mimeType: "image/png", blob: Buffer.from("PNG-BYTES").toString("base64") }], + }), + }, + }); + + const read = await handlers.readMcpResource({ + server: "files", + uri: "files://logo", + downloadPath: "assets/logo.png", + }); + + expect(read?.downloadPath).toBe("assets/logo.png"); + // No content: that is the whole point of the download mode. + expect(read?.text).toBeUndefined(); + expect(read?.blob).toBeUndefined(); + // The file is really there, decoded from base64, under the workspace. + expect(await Bun.file(path.join(workspace, "assets/logo.png")).text()).toBe("PNG-BYTES"); + } finally { + await removeWithRetries(workspace); + } + }); + it("answers nothing when the session has no MCP manager", async () => { // A host without MCP must still answer truthfully rather than throwing: // an empty catalog and `not_found` are the honest responses.