-
Notifications
You must be signed in to change notification settings - Fork 915
fix(cursor): explain empty exec results + forbid native probing in code mode #2662
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,25 @@ function isNodeReplOrComputerUseTool(toolName?: string, toolNamespace?: string): | |
| return lower.startsWith("mcp__node_repl") || lower.startsWith("mcp__computer_use"); | ||
| } | ||
|
|
||
| /** | ||
| * Codex exec / shell-bridge tool names (flat and MCP-prefixed display aliases). An empty result | ||
| * here is almost always a code-mode cell that never called text()/notify() — the cursor model | ||
| * reads the blank [tool_result], concludes prior results were lost, and spirals into | ||
| * re-orientation retries (devlog 260826_cursor_responses_gap, live subagent transcripts). | ||
| */ | ||
| function isCodexExecBridgeTool(toolName?: string, toolNamespace?: string): boolean { | ||
| if (toolNamespace && toolNamespace.includes("opencodex-responses")) return true; | ||
| if (!toolName) return false; | ||
| const lower = toolName.toLowerCase(); | ||
| return ( | ||
| lower === "exec" | ||
| || lower === "exec_command" | ||
| || lower === "shell_command" | ||
| || lower.startsWith("mcp_opencodex-responses_") | ||
| || lower.startsWith("mcp__opencodex-responses__") | ||
| ); | ||
| } | ||
|
|
||
| /** Failure states the Computer Use / node_repl runtime reports as PLAIN TEXT inside a non-error result. */ | ||
| const RUNTIME_FAILURE_GUIDANCE: ReadonlyArray<{ marker: string; guidance: string }> = [ | ||
| { | ||
|
|
@@ -80,6 +99,13 @@ export function normalizeCursorToolResultText( | |
| changed: true, | ||
| }; | ||
| } | ||
| if (isCodexExecBridgeTool(options.toolName, options.toolNamespace) && EMPTY_EXEC_OUTPUT_REGEX.test(text.trim())) { | ||
| return { | ||
| text: "[empty output: the exec cell completed but emitted nothing. This is NOT lost context and NOT a blocked tool — in code mode call text(...) or notify(...) on any value you need to see (a bare await tools.exec_command(...) is not echoed automatically); in shell mode the command simply printed nothing. Do not re-run the same call expecting different output.]", | ||
| isError: false, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an exec or shell-bridge result arrives with AGENTS.md reference: src/AGENTS.md:L19-L19 Useful? React with 👍 / 👎. |
||
| changed: true, | ||
| }; | ||
| } | ||
| if (!isError) { | ||
| for (const { marker, guidance } of RUNTIME_FAILURE_GUIDANCE) { | ||
| if (text.includes(marker)) { | ||
|
|
@@ -89,4 +115,3 @@ export function normalizeCursorToolResultText( | |
| } | ||
| return { text, isError, changed: false }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { normalizeCursorToolResultText } from "../src/adapters/cursor/tool-result-normalize"; | ||
|
|
||
| describe("codex exec bridge empty-result normalization (devlog 260826 gap-7)", () => { | ||
| test("empty exec cell output becomes explanatory text, not an error", () => { | ||
| const out = normalizeCursorToolResultText("Script completed\nWall time 0.1 seconds\nOutput:\n", { toolName: "exec" }); | ||
| expect(out.changed).toBe(true); | ||
| expect(out.isError).toBe(false); | ||
| expect(out.text).toContain("NOT lost context"); | ||
| expect(out.text).toContain("text(...)"); | ||
| }); | ||
|
|
||
| test("mcp display alias names route the same way", () => { | ||
| const out = normalizeCursorToolResultText("", { toolName: "mcp_opencodex-responses_exec" }); | ||
| expect(out.changed).toBe(true); | ||
| expect(out.text).toContain("empty output"); | ||
| }); | ||
|
|
||
| test("shell_command empty output routes too", () => { | ||
| const out = normalizeCursorToolResultText("<empty>", { toolName: "shell_command" }); | ||
| expect(out.changed).toBe(true); | ||
| }); | ||
|
|
||
| test("non-empty exec output passes through byte-identical", () => { | ||
| const out = normalizeCursorToolResultText("Output:\nhello", { toolName: "exec" }); | ||
| expect(out.changed).toBe(false); | ||
| expect(out.text).toBe("Output:\nhello"); | ||
| }); | ||
|
|
||
| test("computer-use empties keep the original error semantics", () => { | ||
| const out = normalizeCursorToolResultText("", { toolName: "screenshot" }); | ||
| expect(out.isError).toBe(true); | ||
| expect(out.text).toContain("get_app_state"); | ||
| }); | ||
|
|
||
| test("unrelated tools with empty output stay untouched", () => { | ||
| const out = normalizeCursorToolResultText("", { toolName: "get_weather" }); | ||
| expect(out.changed).toBe(false); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When any unrelated Responses-owned tool returns empty output, this namespace check classifies it as an exec bridge before inspecting its name. For example,
waitwith namespaceopencodex-responsesis recognized byisCursorWaitTool, while display aliases such asmcp_opencodex-responses_apply_patchshare the same generic prefix; both now receive misleading exec-cell guidance rather than retaining their actual result semantics. Require the normalized tool name to be exactlyexec,exec_command, orshell_commandinstead of accepting the provider namespace or every prefixed tool.AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.