fix(cursor): honor download_path on MCP resource reads

`ReadMcpResourceExecArgs.download_path` means "write the resource to
this workspace-relative path and return no model content". The handler
I added forwarded only server and uri, so a download reported success
while creating no file and leaving `ReadMcpResourceSuccess.download_path`
unset - the model was pointed at a path that did not exist.

The path now reaches the handler, the bridge writes the bytes (decoding
a base64 blob, or the joined text) under the session cwd, and the
answer carries the path with the content oneof deliberately unset: a
host that also has the payload on hand must not have it forwarded, or
the download mode puts it right back in context.

(cherry picked from commit f0a6784533201f412529fb0ae5a6542531012197)
This commit is contained in:
Diogo Soares Rodrigues
2026-07-27 13:50:31 -03:00
committed by can1357
parent 6d42f6e52e
commit 01be80b9ea
7 changed files with 135 additions and 17 deletions
+1 -1
View File
@@ -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.
+17 -8
View File
@@ -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 },
}),
},
})
+18 -2
View File
@@ -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<CursorMcpResourceContent | null>;
readMcpResource?: (args: {
server: string;
uri: string;
/**
* When set, write the resource here (workspace-relative) and return
* `downloadPath` instead of content.
*/
downloadPath?: string;
}) => Promise<CursorMcpResourceContent | null>;
/** Mirror Cursor's server-owned todo list into local session state. */
todoSync?: CursorTodoSyncHandler;
onToolResult?: CursorToolResultHandler;
@@ -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
+1 -1
View File
@@ -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.<tool>` 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.
+29 -5
View File
@@ -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<CursorMcpResourceContent | null> {
async readMcpResource({
server,
uri,
downloadPath,
}: {
server: string;
uri: string;
downloadPath?: string;
}): Promise<CursorMcpResourceContent | null> {
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") };
}
/**
@@ -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.