fix(cursor): surface built-in tool calls instead of dropping the turn - #410
fix(cursor): surface built-in tool calls instead of dropping the turn#410r-uben wants to merge 5 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves tool-call handling in the antigravity-cli and cursor adapters by explicitly failing requests with clear errors when tool bridging is not supported, rather than silently ignoring them or truncating the turn. Specifically, antigravity-cli now rejects caller-supplied tools with a 400 error, and the cursor adapter detects and fails on unbridged built-in tool calls with a 502 error. The code review feedback recommends optimizing the tool ID prefix check in the Cursor adapter by avoiding full UTF-8 validation on byte slices, and suggests using the concat! macro for multi-line string literals in both adapters to prevent issues with IDEs stripping fragile trailing whitespaces.
| let message = "The deprecated `antigravity-cli` transport cannot use caller-supplied \ | ||
| tools. It runs the local `agy` binary, which resolves its own tool calls and never \ | ||
| returns a tool_use block, so this request would otherwise get a text-only reply \ | ||
| that silently ignored them. Send the task as a plain prompt and let agy do the \ | ||
| work, or route this model at the native `antigravity` provider (or `gemini`), \ | ||
| which do forward tools." | ||
| .to_string(); |
There was a problem hiding this comment.
[MEDIUM] Fragile trailing whitespace in multi-line string literal
Problem: The multi-line string literal uses trailing spaces before the line-continuation backslashes (\) to separate words across lines. This is extremely fragile because many IDEs and code formatters are configured to automatically strip trailing whitespace on save, which would silently remove these spaces and cause words to run together (e.g., "caller-suppliedtools").
Rationale: Organization Style Guide §1 (Maintainability) and §12.2 (R1 - Cognitive Overload / Accidental Complexity).
Suggestion: Use concat! to safely construct the multi-line string with explicit spaces at the end of each segment, making it robust against trailing whitespace stripping.
let message = concat!(
"The deprecated `antigravity-cli` transport cannot use caller-supplied tools. ",
"It runs the local `agy` binary, which resolves its own tool calls and never ",
"returns a tool_use block, so this request would otherwise get a text-only reply ",
"that silently ignored them. Send the task as a plain prompt and let agy do the ",
"work, or route this model at the native `antigravity` provider (or `gemini`), ",
"which do forward tools."
)
.to_string();References
- Organization Style Guide §1 (Maintainability) and §12.2 (R1 - Cognitive Overload / Accidental Complexity) (link)
Greptile SummaryThe PR changes Cursor’s response decoding so otherwise-unbridgeable built-in tool calls produce an explicit error instead of a silent successful turn.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/adapters/cursor/agent.rs | Adds bounded detection of unbridgeable Cursor built-in tool calls, explicit error handling, and focused decoder tests. |
| site/src/content/docs/providers/cursor.mdx | Documents how built-in calls are surfaced and that retrying is the available mitigation. |
| README.md | Updates the English Cursor overview with the new explicit failure behavior. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Cursor ExecServerMessage] --> B{MCP field 11?}
B -->|Yes| C[Emit bridged tool call]
B -->|No| D{Nested tool_* call id?}
D -->|Yes| E[Emit explicit gateway error]
D -->|No| F[Continue decoding text, reasoning, or turn end]
Reviews (6): Last reviewed commit: "docs(cursor): note the unbridgeable buil..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| **Antigravity has two transports.** The `antigravity` provider talks to the Google Antigravity backend over HTTP, authenticated with `shunt login antigravity` — a Google authorization-code flow using Antigravity's own OAuth client and scopes, so a Gemini CLI login cannot be reused for it. It speaks the same Code Assist protocol as the `gemini` provider and currently serves the Gemini-family Antigravity models; the Claude models Antigravity also offers need request rewrites that are not implemented yet (#368). | ||
|
|
||
| **`antigravity-cli` is deprecated and is arbitrary code execution.** It runs the local `agy` binary in agentic mode: the CLI uses its own tools to do the work and shunt streams its progress back as Anthropic SSE. A `tools` array on the request is therefore not forwarded and no `tool_use` block is ever returned. Because a non-interactive run cannot answer a permission prompt, `agy` runs with `--dangerously-skip-permissions`, so **treat this provider as arbitrary code execution as the user running shunt**. Two settings bound it: `sandbox` (default `true`) passes `--sandbox`, which keeps reads and writes inside the workspace and is what actually contains the agent; `workspace_roots` decides only where it may *start*, gating the `Working directory:` path taken from the request's system prompt (client-controlled text) to canonicalized paths under roots you list. Keep the sandbox on and the bind on loopback. Prefer the `antigravity` provider above, which needs none of this. See the [providers guide](https://shunt.dev/guides/providers/). | ||
| **`antigravity-cli` is deprecated and is arbitrary code execution.** It runs the local `agy` binary in agentic mode: the CLI uses its own tools to do the work and shunt streams its progress back as Anthropic SSE. It can therefore never return a `tool_use` block, so a request that actually asks for one — a non-empty `tools` array, or a `tool_choice` of `any`/`tool` — is refused with a `400` rather than silently answered as text. `tool_choice: none` is exempt even alongside `tools`, and so is `auto` with no tools, since neither obliges a tool call. Because a non-interactive run cannot answer a permission prompt, `agy` runs with `--dangerously-skip-permissions`, so **treat this provider as arbitrary code execution as the user running shunt**. Two settings bound it: `sandbox` (default `true`) passes `--sandbox`, which keeps reads and writes inside the workspace and is what actually contains the agent; `workspace_roots` decides only where it may *start*, gating the `Working directory:` path taken from the request's system prompt (client-controlled text) to canonicalized paths under roots you list. Keep the sandbox on and the bind on loopback. Prefer the `antigravity` provider above, which needs none of this. See the [providers guide](https://shunt.dev/guides/providers/). |
There was a problem hiding this comment.
The English README now documents that tool-bearing antigravity-cli requests return 400, but the maintained Korean, Japanese, and Chinese root READMEs retain the previous behavior. Update all three translations in this PR so localized documentation does not continue telling users that caller tools are merely ignored.
Context Used: CLAUDE.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: README.md
Line: 154
Comment:
**Root translations remain stale**
The English README now documents that tool-bearing `antigravity-cli` requests return `400`, but the maintained Korean, Japanese, and Chinese root READMEs retain the previous behavior. Update all three translations in this PR so localized documentation does not continue telling users that caller tools are merely ignored.
**Context Used:** CLAUDE.md ([source](https://github.com/pleaseai/shunt/blob/main/CLAUDE.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f432ead3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| **Antigravity has two transports.** The `antigravity` provider talks to the Google Antigravity backend over HTTP, authenticated with `shunt login antigravity` — a Google authorization-code flow using Antigravity's own OAuth client and scopes, so a Gemini CLI login cannot be reused for it. It speaks the same Code Assist protocol as the `gemini` provider and currently serves the Gemini-family Antigravity models; the Claude models Antigravity also offers need request rewrites that are not implemented yet (#368). | ||
|
|
||
| **`antigravity-cli` is deprecated and is arbitrary code execution.** It runs the local `agy` binary in agentic mode: the CLI uses its own tools to do the work and shunt streams its progress back as Anthropic SSE. A `tools` array on the request is therefore not forwarded and no `tool_use` block is ever returned. Because a non-interactive run cannot answer a permission prompt, `agy` runs with `--dangerously-skip-permissions`, so **treat this provider as arbitrary code execution as the user running shunt**. Two settings bound it: `sandbox` (default `true`) passes `--sandbox`, which keeps reads and writes inside the workspace and is what actually contains the agent; `workspace_roots` decides only where it may *start*, gating the `Working directory:` path taken from the request's system prompt (client-controlled text) to canonicalized paths under roots you list. Keep the sandbox on and the bind on loopback. Prefer the `antigravity` provider above, which needs none of this. See the [providers guide](https://shunt.dev/guides/providers/). | ||
| **`antigravity-cli` is deprecated and is arbitrary code execution.** It runs the local `agy` binary in agentic mode: the CLI uses its own tools to do the work and shunt streams its progress back as Anthropic SSE. It can therefore never return a `tool_use` block, so a request that actually asks for one — a non-empty `tools` array, or a `tool_choice` of `any`/`tool` — is refused with a `400` rather than silently answered as text. `tool_choice: none` is exempt even alongside `tools`, and so is `auto` with no tools, since neither obliges a tool call. Because a non-interactive run cannot answer a permission prompt, `agy` runs with `--dangerously-skip-permissions`, so **treat this provider as arbitrary code execution as the user running shunt**. Two settings bound it: `sandbox` (default `true`) passes `--sandbox`, which keeps reads and writes inside the workspace and is what actually contains the agent; `workspace_roots` decides only where it may *start*, gating the `Working directory:` path taken from the request's system prompt (client-controlled text) to canonicalized paths under roots you list. Keep the sandbox on and the bind on loopback. Prefer the `antigravity` provider above, which needs none of this. See the [providers guide](https://shunt.dev/guides/providers/). |
There was a problem hiding this comment.
This documents the new antigravity-cli 400 behavior only in README.md; README.ja.md, README.ko.md, and README.zh-CN.md were not updated, so users of the maintained translations will not know that requests advertising tools are now rejected. Apply the behavioral update to all three translated READMEs in the same change.
AGENTS.md reference: AGENTS.md:L57-L62
Useful? React with 👍 / 👎.
| - **Tools are `agy`'s, not the caller's, and a request asking for one is refused.** This | ||
| adapter can never emit a `tool_use` block, so a non-empty `tools` array, or a | ||
| `tool_choice` of `any` or `tool`, is rejected with a `400 invalid_request_error` naming |
There was a problem hiding this comment.
The English providers guide now explains that antigravity-cli rejects these requests, but the maintained ja, ko, and zh-cn copies of guides/providers.mdx were left unchanged and do not contain this behavior, leaving the published locale guides stale. Add the equivalent section to each locale copy alongside this update.
AGENTS.md reference: AGENTS.md:L57-L64
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/adapters/antigravity/mod.rs">
<violation number="1" location="src/adapters/antigravity/mod.rs:612">
P3: The added doc block is inserted before the existing `agy_not_found` item, so the binary-lookup documentation now annotates `reject_caller_tools` and `agy_not_found` loses it. Move `agy_not_found` above the new helper, or move the new helper block below `agy_not_found` so each function keeps the correct documentation.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:154">
P3: Update `README.ko.md`, `README.ja.md`, and `README.zh-CN.md` to document the new `400` rejection; otherwise localized users are still told that caller tools are ignored.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as Anthropic Client
participant Adapter as Shunt Adapter Layer
participant Cursor as Cursor Agent Service
participant AGY as Antigravity CLI (agy)
Note over Client,AGY: Request Validation Path (antigravity-cli)
Client->>Adapter: Messages request with tools/tool_choice
Adapter->>Adapter: NEW: Check tool_choice and tools fields
alt tool_choice = none OR (no tools AND tool_choice = auto) OR empty tools
Adapter->>AGY: Forward text-only prompt
AGY-->>Adapter: Text response stream
Adapter-->>Client: 200 text-only response (end_turn)
else non-empty tools OR tool_choice = any/tool
Adapter-->>Client: 400 invalid_request_error (reject)
Note over Client,Adapter: Explicit failure instead of silent text-only 200
end
Note over Client,Cursor: Cursor Adapter - Built-in Tool Detection
Client->>Adapter: Agentic request with caller-supplied tools
Adapter->>Cursor: ConnectRPC agent request
Cursor-->>Adapter: ExecServerMessage stream
loop Per turn
Cursor-->>Adapter: Frame (field 7 or 11)
Adapter->>Adapter: NEW: Check field 11 (bridged MCP) first
alt Field 11 contains tool call (bridged path)
Adapter->>Adapter: Extract tool name + args
Adapter-->>Client: Stream tool_use block
else Field 7+ with tool_<uuid> id (built-in path)
Adapter->>Adapter: NEW: Detect tool_ prefix outside field 11
Adapter-->>Client: 502 error (explicit failure)
Note over Adapter,Client: No silent end_turn - caller sees the failure
else No tool ID present
Adapter-->>Client: Stream text/reasoning content
end
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| /// which excludes `~/.local/bin` — the default install location for `agy` — so | ||
| /// a provider that works in a shell returns 503 under the service with no | ||
| /// indication why. `AGY_BIN` is the fix, and the message has to say so. | ||
| /// Reject a request that carries caller-supplied tools. |
There was a problem hiding this comment.
P3: The added doc block is inserted before the existing agy_not_found item, so the binary-lookup documentation now annotates reject_caller_tools and agy_not_found loses it. Move agy_not_found above the new helper, or move the new helper block below agy_not_found so each function keeps the correct documentation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/adapters/antigravity/mod.rs, line 612:
<comment>The added doc block is inserted before the existing `agy_not_found` item, so the binary-lookup documentation now annotates `reject_caller_tools` and `agy_not_found` loses it. Move `agy_not_found` above the new helper, or move the new helper block below `agy_not_found` so each function keeps the correct documentation.</comment>
<file context>
@@ -608,6 +609,64 @@ fn terminal_failure(end: Option<&AgyEnd>, timed_out: bool, stderr: &StderrLog) -
/// which excludes `~/.local/bin` — the default install location for `agy` — so
/// a provider that works in a shell returns 503 under the service with no
/// indication why. `AGY_BIN` is the fix, and the message has to say so.
+/// Reject a request that carries caller-supplied tools.
+///
+/// `agy` resolves its own tool calls internally and has no mode that hands them
</file context>
| **Antigravity has two transports.** The `antigravity` provider talks to the Google Antigravity backend over HTTP, authenticated with `shunt login antigravity` — a Google authorization-code flow using Antigravity's own OAuth client and scopes, so a Gemini CLI login cannot be reused for it. It speaks the same Code Assist protocol as the `gemini` provider and currently serves the Gemini-family Antigravity models; the Claude models Antigravity also offers need request rewrites that are not implemented yet (#368). | ||
|
|
||
| **`antigravity-cli` is deprecated and is arbitrary code execution.** It runs the local `agy` binary in agentic mode: the CLI uses its own tools to do the work and shunt streams its progress back as Anthropic SSE. A `tools` array on the request is therefore not forwarded and no `tool_use` block is ever returned. Because a non-interactive run cannot answer a permission prompt, `agy` runs with `--dangerously-skip-permissions`, so **treat this provider as arbitrary code execution as the user running shunt**. Two settings bound it: `sandbox` (default `true`) passes `--sandbox`, which keeps reads and writes inside the workspace and is what actually contains the agent; `workspace_roots` decides only where it may *start*, gating the `Working directory:` path taken from the request's system prompt (client-controlled text) to canonicalized paths under roots you list. Keep the sandbox on and the bind on loopback. Prefer the `antigravity` provider above, which needs none of this. See the [providers guide](https://shunt.dev/guides/providers/). | ||
| **`antigravity-cli` is deprecated and is arbitrary code execution.** It runs the local `agy` binary in agentic mode: the CLI uses its own tools to do the work and shunt streams its progress back as Anthropic SSE. It can therefore never return a `tool_use` block, so a request that actually asks for one — a non-empty `tools` array, or a `tool_choice` of `any`/`tool` — is refused with a `400` rather than silently answered as text. `tool_choice: none` is exempt even alongside `tools`, and so is `auto` with no tools, since neither obliges a tool call. Because a non-interactive run cannot answer a permission prompt, `agy` runs with `--dangerously-skip-permissions`, so **treat this provider as arbitrary code execution as the user running shunt**. Two settings bound it: `sandbox` (default `true`) passes `--sandbox`, which keeps reads and writes inside the workspace and is what actually contains the agent; `workspace_roots` decides only where it may *start*, gating the `Working directory:` path taken from the request's system prompt (client-controlled text) to canonicalized paths under roots you list. Keep the sandbox on and the bind on loopback. Prefer the `antigravity` provider above, which needs none of this. See the [providers guide](https://shunt.dev/guides/providers/). |
There was a problem hiding this comment.
P3: Update README.ko.md, README.ja.md, and README.zh-CN.md to document the new 400 rejection; otherwise localized users are still told that caller tools are ignored.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 154:
<comment>Update `README.ko.md`, `README.ja.md`, and `README.zh-CN.md` to document the new `400` rejection; otherwise localized users are still told that caller tools are ignored.</comment>
<file context>
@@ -151,7 +151,7 @@ xAI may gate OAuth access by subscription tier — if `grok` returns 403, use th
**Antigravity has two transports.** The `antigravity` provider talks to the Google Antigravity backend over HTTP, authenticated with `shunt login antigravity` — a Google authorization-code flow using Antigravity's own OAuth client and scopes, so a Gemini CLI login cannot be reused for it. It speaks the same Code Assist protocol as the `gemini` provider and currently serves the Gemini-family Antigravity models; the Claude models Antigravity also offers need request rewrites that are not implemented yet (#368).
-**`antigravity-cli` is deprecated and is arbitrary code execution.** It runs the local `agy` binary in agentic mode: the CLI uses its own tools to do the work and shunt streams its progress back as Anthropic SSE. A `tools` array on the request is therefore not forwarded and no `tool_use` block is ever returned. Because a non-interactive run cannot answer a permission prompt, `agy` runs with `--dangerously-skip-permissions`, so **treat this provider as arbitrary code execution as the user running shunt**. Two settings bound it: `sandbox` (default `true`) passes `--sandbox`, which keeps reads and writes inside the workspace and is what actually contains the agent; `workspace_roots` decides only where it may *start*, gating the `Working directory:` path taken from the request's system prompt (client-controlled text) to canonicalized paths under roots you list. Keep the sandbox on and the bind on loopback. Prefer the `antigravity` provider above, which needs none of this. See the [providers guide](https://shunt.dev/guides/providers/).
+**`antigravity-cli` is deprecated and is arbitrary code execution.** It runs the local `agy` binary in agentic mode: the CLI uses its own tools to do the work and shunt streams its progress back as Anthropic SSE. It can therefore never return a `tool_use` block, so a request that actually asks for one — a non-empty `tools` array, or a `tool_choice` of `any`/`tool` — is refused with a `400` rather than silently answered as text. `tool_choice: none` is exempt even alongside `tools`, and so is `auto` with no tools, since neither obliges a tool call. Because a non-interactive run cannot answer a permission prompt, `agy` runs with `--dangerously-skip-permissions`, so **treat this provider as arbitrary code execution as the user running shunt**. Two settings bound it: `sandbox` (default `true`) passes `--sandbox`, which keeps reads and writes inside the workspace and is what actually contains the agent; `workspace_roots` decides only where it may *start*, gating the `Working directory:` path taken from the request's system prompt (client-controlled text) to canonicalized paths under roots you list. Keep the sandbox on and the bind on loopback. Prefer the `antigravity` provider above, which needs none of this. See the [providers guide](https://shunt.dev/guides/providers/).
**Migrating from the old `antigravity`.** `kind = "antigravity"` used to mean the local CLI. A config still carrying that meaning is refused by name rather than silently retargeted, and a routed `antigravity` provider with no credential refuses to start — switching transport, credentials, and egress underneath a green startup would be worse than failing. Run `shunt login antigravity`, or point the route at `antigravity-cli`.
</file context>
Composer chooses per turn whether to invoke a bridged caller tool or one of
Cursor's own built-in tools. A bridged call arrives as
`ExecServerMessage.mcp_args` (field 11) and decodes correctly. A built-in call
arrives under a tool-specific field carrying only its arguments and a
`tool_<uuid>` call id, with no tool name -- observed for the built-in file read
as `ExecServerMessage` field 7: `{1: "<path>", 2: "tool_<uuid>"}`.
`extract_tool_call` matched field 11 only, so a built-in call fell through to
the `End` branch: the turn closed with `end_turn` and no `tool_use`, and the
model appeared to announce a tool call and then do nothing. Captured against
the live upstream over six turns, the correlation was exact -- every failing
turn carried field 7 and no field 11, every passing turn the reverse.
shunt cannot bridge these: the caller owns the filesystem, and the frame
carries no name to map onto a caller tool. Detect them via the `tool_` call-id
prefix outside field 11 (across captured turns only fields 7 and 11 ever carry
one) and fail the turn with an explicit error, turning a silent truncation into
a visible one. Verified over ten live turns: five bridged tool calls, five
explicit errors, zero silent `end_turn`s.
6f432ea to
8970e7e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8970e7e4ec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review feedback: the message relied on trailing whitespace before a line continuation, so whitespace cleanup could join `tool,` and `which` into a malformed string. Also record why the call-id scan stays wide. Two narrower variants were built and measured against the live upstream on an identical request: pinning detection to the captured field position leaked silent turns (2 of 22), and additionally requiring a uuid-shaped id body leaked badly (8 of 16), even though every captured id satisfies that shape. The wide scan showed no silent turn in 38.
|
Thanks — addressed the On the P2 suggestion (restrict detection to call-ID field positions rather than scanning nested strings): I built it and measured it, and it makes things worse, so I've kept the wide scan and documented why in the code. Two narrower variants, each A/B'd against the live upstream on an identical tool-bearing request, counting turns that returned neither a
The uuid-shape variant is the surprising one: all six call ids captured from real frames satisfy that shape ( The tradeoff isn't symmetric either way, but it is a tradeoff: a false positive fails a turn that was working, while a miss just restores the pre-existing silent truncation. I agree the prefix check is loose in principle — I'd happily replace the whole heuristic with an explicit field allowlist if anyone has the real Re the README translation gap and the |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 988ff3ed88
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 336a124ca5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- match the `tool_` call-id prefix on raw bytes instead of running a full UTF-8 validation over every nested field of every streamed frame - drop the "or send a request with no tools" remedy from the 502 message and the matching claim from the provider page: `ReadState` carries no request-tool context, so detection fires regardless of what was advertised - correct the `MAX_TOOL_ID_SCAN_DEPTH` doc, which claimed to mirror a constant that is 64 - cover the byte-prefix semantics with a call id that has a non-UTF-8 tail
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds detection and explicit error handling for unbridged built-in tool calls from Cursor, along with corresponding documentation and tests. A high-severity issue was identified in the recursive scanning of protobuf fields, which could cause false positives on arbitrary user content (such as file paths or contents) that happens to match the protobuf wire format and contain the 'tool_' prefix. It is recommended to restrict the scan to direct fields of the tool message to prevent these false positives.
The README described the Cursor adapter as bridging caller tools with no mention that a turn can now fail when Composer answers through one of Cursor's own built-in tools instead. The ko/ja/zh-CN READMEs need no matching change: their Cursor paragraphs never carried the tool-bridging claim — they abbreviate to the prefix/suffix rule and defer to the providers page.
2 participants
Summary
Composer chooses per turn whether to invoke a bridged caller tool or one of Cursor's own built-in tools. A bridged call arrives as
ExecServerMessage.mcp_args(field 11) and decodes correctly. A built-in call arrives under a tool-specific field carrying only its arguments and atool_<uuid>call id, with no tool name — observed for the built-in file read asExecServerMessagefield 7:{1: "<path>", 2: "tool_<uuid>"}.extract_tool_callmatched field 11 only, so a built-in call fell through to theEndbranch: the turn closed withend_turnand notool_use. The visible symptom is a model that announces a tool call ("I'll read /etc/hostname using the Read tool") and then does nothing — which makes the Cursor adapter unusable for agentic clients, since roughly half of tool-bearing turns silently no-op.shunt cannot bridge these calls: the caller owns the filesystem, and the frame carries no name to map onto a caller tool. This PR detects them and fails the turn with an explicit error, converting a silent truncation into a visible one. Detection keys on the
tool_call-id prefix appearing outside field 11.Evidence
Captured raw Connect frames against the live upstream (throwaway instrumented build, not included here). Correlation over six turns was exact:
Across all captured turns, only fields 7 and 11 ever carry a
tool_*id — which is what makes the prefix a safe discriminator without enumerating Cursor's built-in catalog.After the fix, ten live turns on the same request: 5 bridged tool calls, 5 explicit errors, 0 silent
end_turns.Milestone / spec
None — this is a wire-behavior bug in the
agent.v1Cursor path, not a spec deviation. Related open issues: #269, #29, #59 (all Cursor adapter, none covering this).Checklist
cargo buildpassescargo testpasses (1701 passed, 0 failed; three new unit tests, no network)cargo clippy --all-targets --all-features -- -D warningscleancargo fmt --all --checkcleansrc/adapters/cursor/agent.rswas already ~1450 lines before this change (tracked by Split oversized Cursor adapter files (>500 lines) into focused modules #59); this adds to it rather than fixing it. Happy to land it elsewhere if you'd prefer.docs/updated if this change deviates from it (n/a)site/src/content/docs/providers/cursor.mdxgained a note on the built-in-tool error (no locale copies of that page exist)Notes for reviewers
tool_*id outside field 11 as an unbridged built-in call. It's backed by observation, not by a schema. If the realExecServerMessagedescriptor is available, an explicit field allowlist would be strictly better.Summary by cubic
Surfaces Cursor built-in tool calls as explicit 502 errors instead of closing the turn with end_turn and no tool_use. We detect a
tool_*call id outsideExecServerMessage.mcp_args(field 11) and fail the turn so the issue is visible.tool_prefix outside field 11 with bounded recursion; ignores plain-text frames and excludes the bridgedmcp_argspath.concat!.site/src/content/docs/providers/cursor.mdxandREADME.mdto document the explicit error and the non-deterministic built-in path.Written for commit cb364f8. Summary will update on new commits.