-
Notifications
You must be signed in to change notification settings - Fork 1k
fix(cursor): name the invocation inside a replayed tool result #2900
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 |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| # 000 — RCA: Cursor external tool-continuation replays an orphaned tool result | ||
|
|
||
| Unit: `devlog/_plan/260829_cursor_tool_continuation_pairing/` | ||
| Date: 2026-08-29 | ||
| Class: C3 (single adapter file + focused regression tests; public wire behavior for every Cursor | ||
| external model, so a durable record is warranted) | ||
|
|
||
| ## 1. Symptom as reported | ||
|
|
||
| A Cursor-backed Codex session "does not receive tool output and outputs infinitely": the model | ||
| re-runs a command it already ran, narrates that the previous attempt was interrupted, and the turn | ||
| does not terminate. | ||
|
|
||
| ## 2. Live reproduction (this session) | ||
|
|
||
| Environment: opencodex proxy 2.35.0, pid 62773, port 10100 (`/healthz` confirmed); | ||
| codex-cli 0.150.1; model `cursor/grok-4.6` resolved to wire id `cursor-grok-4.6-xhigh`. | ||
|
|
||
| ### run1 — three sequential echoes | ||
|
|
||
| Prompt asked for `echo STEP1`, `echo STEP2`, `echo STEP3` one at a time, reading each | ||
| result, then `ALLDONE`. Observed NDJSON (`/tmp/cursor-repro/run1.ndjson`): | ||
|
|
||
| | # | item | observation | | ||
| |---|------|-------------| | ||
| | item_3 | command_execution `echo STEP1` | `exit_code: 0`, `aggregated_output: "STEP1\n"` | | ||
| | item_4 | agent_message | "STEP1 **was interrupted**, so I'm running it again with the command output captured." | | ||
| | item_5 | command_execution `echo STEP1` | duplicate run of a command that already succeeded | | ||
| | item_7 | command_execution `echo STEP2` | succeeded, `exit_code: 0` | | ||
| | item_8 | agent_message | "STEP1 finished. Next I'll run `echo STEP2`..." — re-announces work already done | | ||
| | item_9 | command_execution `echo STEP2` | duplicate again | | ||
|
|
||
| The turn never reached `turn.completed` and was terminated manually. The phantom "was interrupted" | ||
| claim is the load-bearing detail: the tool call had `exit_code: 0` and real stdout, so the model | ||
| was not reacting to a failure — it was reacting to a history in which its own call is missing. | ||
|
|
||
| ### run2 — two echoes, provider debug on | ||
|
|
||
| Prompt asked for `echo AAA` then `echo BBB`, then `DONE2`. Observed: | ||
| `echo AAA` ran **twice**, `echo BBB` ran **twice**, and the model asserted the first command | ||
| "printed `AAA_DONE`" when the actual output was `AAA`. It did finally emit `DONE2` and exit 0 — | ||
| four tool calls for two requested commands. | ||
|
|
||
| Provider diagnostics for the same run (`ocx debug provider logs`) show each continuation: | ||
|
|
||
| ``` | ||
| [ocx:cursor:run-request] {"wireModel":"cursor-grok-4.6-xhigh","action":"userMessageAction", | ||
| "turnType":"tool-continuation","externalModel":true,"rawMessages":8,"continuationMode":"full-replay", | ||
| "checkpointPresent":false,"checkpointInvalidationReason":"missing_ref","rootBlobs":10,"turnBlobs":6} | ||
| ``` | ||
|
|
||
| So the transport is healthy: the tool result IS being sent (`rootBlobs` grows every turn, 10 → 12 → | ||
| 14 → 16). This is not a dropped-output or backlog bug. The payload is wrong in *shape*. | ||
|
|
||
| ## 3. Root cause — decoded from the wire, not inferred | ||
|
|
||
| Probe: `.tmp/cursorprobe/wire.ts` builds a tool-result continuation through the real | ||
| `encodeCursorRunRequest` and resolves every `rootPromptMessagesJson` blob through the real | ||
| `handleCursorNativeKv` blob store. History: user prompt → assistant text + `toolCall` | ||
| (`fc_abc123`, `exec_command`, `{"cmd":"echo AAA"}`) → `toolResult` (same id, output `AAA`). | ||
|
|
||
| Decoded roots for `grok-4.6-high`: | ||
|
|
||
| ``` | ||
| root[0] {"role":"system", ...} | ||
| root[1] {"role":"user","content":[{"type":"text","text":"Run echo AAA then echo BBB."}]} | ||
| root[2] {"role":"assistant","content":[{"type":"text","text":"I will run echo AAA."}]} | ||
| root[3] {"role":"assistant","content":[{"type":"text","text":"[Tool Result]\n[tool_result]\n | ||
| call_id: fc_abc123\nname: exec_command\nis_error: false\noutput:\nAAA"}]} | ||
| ACTION: userMessageAction | ||
| ACTION TEXT: "Continue: the requested tool results are provided in the conversation history above." | ||
| ``` | ||
|
|
||
| **The assistant tool CALL is absent.** `root[2]` keeps only the assistant's prose; the | ||
| `toolCall` content part is dropped. `root[3]` then presents a *result* — complete with a | ||
| `call_id` that refers to a call the model cannot see anywhere in its context. | ||
|
|
||
| The omission is deliberate and documented in `src/adapters/cursor/protobuf-request.ts` | ||
| (`rootPromptMessages`, external branch): | ||
|
|
||
| ``` | ||
| // Assistant tool CALLS are intentionally NOT replayed as visible "[Tool Call]" text here. | ||
| ``` | ||
|
|
||
| The same asymmetry exists in `conversationTurns`: for `externalModel` it replays only | ||
| `part.type === "text"` and explicitly skips `toolCall` parts, while the `toolResult` branch | ||
| pushes a `[Tool Result]` assistant step. | ||
|
|
||
| ### Why an orphaned result produces exactly these symptoms | ||
|
|
||
| From the model's point of view the transcript reads: *I said I would run a command. Then a tool | ||
| result appeared for a call with an id I never issued. Now a user message tells me to continue.* | ||
| Both observed behaviors are the natural completion of that context: | ||
|
|
||
| 1. **Duplicate execution.** The intended call is not in the transcript, so the most probable | ||
| continuation is to issue it — which is exactly what a well-behaved agent does when it announced | ||
| an action it cannot see itself having taken. | ||
| 2. **Phantom "was interrupted".** The model must explain a result with no originating call. The | ||
| available story is that the earlier attempt was cut off. It then "re-runs it properly". | ||
|
|
||
| The infinite-output case is the same loop without a lucky exit: every continuation re-adds an | ||
| orphaned result, so the same reasoning fires again. This is also why the existing repetition | ||
| breaker does not save the turn — the repeated entries are *not byte-identical* (each carries a | ||
| different `call_id`), so the `pushDeduped` collapse never triggers. | ||
|
|
||
| ## 4. Why existing mitigations do not cover it | ||
|
|
||
| | Mechanism | Why it misses this defect | | ||
| |-----------|---------------------------| | ||
| | `CursorEnvelopeEchoSniffer` | Watches the model's **output** for an echoed envelope. Here the output is legitimate prose; the defect is in the **input**. | | ||
| | `CursorMidstreamEchoObserver` | Diagnostic-only, never mutates the request. | | ||
| | Repetition breaker (gap-9) | Collapses byte-identical consecutive entries. Distinct `call_id`s defeat it. | | ||
| | `normalizeCursorToolResultText` (#1920) | Fixes result *text* for empty/failed output. Says nothing about the missing call. | | ||
| | `CURSOR_EXTERNAL_TOOL_CONTINUATION_TEXT` | Tells the model results are "in the conversation history above" — true, but the *call* is not, which is what makes the reference unresolvable. | | ||
|
|
||
| ## 5. Conclusion | ||
|
|
||
| The external replay path is internally inconsistent: it drops assistant tool calls but keeps tool | ||
| results that reference them by id. The fix is to make the replayed transcript self-consistent by | ||
| emitting the call immediately before its result, keyed by call id, without touching the native | ||
| composer path (which carries real `mcpToolCall` structures on `turns[]` and must stay untouched — | ||
| replaying native structures for external workers is what caused the earlier `invalid_argument` | ||
| rejections documented in the same file). | ||
|
Comment on lines
+118
to
+123
Contributor
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. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win Use one precise replay contract in both planning documents. The final behavior embeds an
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
|
|
||
| Implementation phases: `010` (pairing in root replay + conversation turns), `020` (regression | ||
| tests, remote verification, delivery). | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| # 001 — Audit round 1 (direct independent audit, blockers folded) | ||
|
|
||
| Auditor: dispatched `explorer` lane (`cursor-plan-auditor`, agent `01a04d5c`) produced no | ||
| output across five bounded `wait_agent` cycles (~10 minutes). Per DISPATCH-RETIRE-01 it was | ||
| retired, and the audit was performed directly against the codebase with an executable probe | ||
| (`.tmp/cursorprobe/audit.ts`) instead of a second paper review. The probe is stronger evidence | ||
| than the paper audit would have been: it decodes the real wire payload for both model classes. | ||
|
|
||
| ## Probe output (verbatim) | ||
|
|
||
| ``` | ||
| ### grok-4.6-high roots=4 turns=1 | ||
| root[0] role=system :: You are a helpful assistant. | ||
| root[1] role=user :: Run echo AAA. | ||
| root[2] role=assistant :: I will run echo AAA. | ||
| root[3] role=assistant :: [Tool Result] | [tool_result] | call_id: call_echo_1 | name: exec_command | is_error: false | output: | AAA | ||
| turn steps=2 | ||
| step assistantMessage :: I will run echo AAA. | ||
| step assistantMessage :: [Tool Result] | AAA | ||
| ### composer-2.5-fast roots=3 turns=1 | ||
| root[0] role=system :: You are a helpful assistant. | ||
| root[1] role=user :: Run echo AAA. | ||
| root[2] role=assistant :: I will run echo AAA. | ||
| turn steps=2 | ||
| step assistantMessage :: I will run echo AAA. | ||
| step toolCall :: <toolCall> | ||
| ``` | ||
|
|
||
| ## Findings | ||
|
|
||
| ### F1 — Root cause CONFIRMED (was claim 1) | ||
|
|
||
| For `grok-4.6-high` the tool call is absent from **both** surfaces: roots carry an orphaned | ||
| `[Tool Result]` (with `call_id: call_echo_1`) and the turn steps carry only `assistantMessage` | ||
| text. For `composer-2.5-fast` the turn carries a real `toolCall` step. The external path is | ||
| therefore the only one that loses the call. `000_rca.md` §3 stands. | ||
|
|
||
| ### F2 — BLOCKER (High): the planned guard is wrong, not merely redundant | ||
|
|
||
| `010` §3.2 gated the new emission on `externalModel && echoToolResultInRoot`. Reading | ||
| `discovery.ts:212`: | ||
|
|
||
| ```ts | ||
| export function cursorNeedsExternalToolContinuation(modelId: string): boolean { | ||
| if (isCursorExternalWireModel(modelId)) return true; | ||
| const wire = cursorCodexToWireModelId(modelId).trim().toLowerCase(); | ||
| return wire === "composer-2.5"; | ||
| } | ||
| ``` | ||
|
|
||
| `externalModel === true` implies `echoToolResultInRoot === true`, so the second conjunct is dead | ||
| in that direction. The live case it *excludes* is the one that matters: `composer-2.5` | ||
| (non-fast) is native (`externalModel === false`) yet `echoToolResultInRoot === true`, so | ||
| `rootPromptMessages` DOES write an orphaned `[Tool Result]` into its root prompt while the | ||
| planned guard would have skipped emitting the pairing call for it. | ||
|
|
||
| That is not hypothetical. `discovery.ts:200-210` documents `composer-2.5` misbehaving with | ||
| exactly the symptom class in `000_rca.md`: it "resumes a tool-result turn with server-side | ||
| native tool calls (read/grep/exec) instead of answering, or completes with zero text". The | ||
| existing mitigation switched its action shape; it never fixed the orphaned root. | ||
|
|
||
| **Fold:** gate the root emission on `echoToolResultInRoot` alone. The invariant is *wherever a | ||
| tool result is echoed into root as text, its call must be there too* — which is exactly the set | ||
| `echoToolResultInRoot` describes. The `conversationTurns` change stays keyed on | ||
| `externalModel`, because the native branch already emits a real `toolCall` step (F1). | ||
|
|
||
| ### F3 — Which surface the model actually reads | ||
|
|
||
| `protobuf-request.ts:186`: "Cursor builds the actual model prompt from | ||
| `rootPromptMessagesJson` (`turns[]` is UI/display metadata)". The root change is therefore the | ||
| load-bearing fix; the `conversationTurns` change is consistency for the display/structure | ||
| surface. Recorded so the test weighting reflects it: the root assertions are the ones that prove | ||
| the defect fixed. | ||
|
|
||
| ### F4 — `arguments` is an object, not a string (was claim 6) | ||
|
|
||
| `src/types/request.ts:211-215`: | ||
|
|
||
| ```ts | ||
| export interface OcxToolCall { | ||
| type: "toolCall"; | ||
| id: string; | ||
| name: string; | ||
| arguments: Record<string, unknown>; | ||
| ``` | ||
|
|
||
| **Fold:** drop the "string or object" branch from `010` §3.1. Serialize with `JSON.stringify` | ||
| inside a `try`, falling back to `"[unserializable arguments]"` — a cyclic or `BigInt`-bearing | ||
| argument object must not be able to throw inside request encoding. | ||
|
|
||
| ### F5 — Helpers exist as assumed (was claim 5) | ||
|
|
||
| | Helper | Location | | ||
| |--------|----------| | ||
| | `decodeCursorCallId` | `src/adapters/cursor/call-id.ts:32` | | ||
| | `namespacedToolName(namespace, name)` | `src/types/tools.ts:30` | | ||
| | `toolResultRootPayload(text)` | `src/adapters/cursor/protobuf-request.ts:137` | | ||
| | `assistantRootText` | `src/adapters/cursor/protobuf-request.ts:180` | | ||
| | `rootBlobCandidate` | `src/adapters/cursor/protobuf-request.ts:121` | | ||
|
|
||
| `OcxToolCall.namespace` exists (`request.ts:226`), so `namespacedToolName(part.namespace, part.name)` | ||
| is correct and mirrors the result formatter's `namespacedToolName(message.toolNamespace, message.toolName)`. | ||
|
|
||
| ### F6 — Pruner bookkeeping is safe (was claim 3) | ||
|
|
||
| `messageIndex` is used only for (a) `truncateToolResultBlob` carry-over and (b) | ||
| `historyMessageStart = firstKept?.messageIndex` (`:372-373`), which feeds `conversationTurns`'s | ||
| `start`. A call entry carries the SAME `messageIndex` as its assistant message, so the | ||
| computed `historyMessageStart` can only equal a value the assistant entry would already have | ||
| produced — it cannot point past a retained message, and `conversationTurns` slices by message | ||
| index, not entry count, so no turn is duplicated. Classing the entry `toolResult` also makes the | ||
| `activeStart` walk (`:322`) keep a call attached to its result as one active block, which is | ||
| the desired behavior. | ||
|
|
||
| One real consequence: `truncateToolResultBlob` will now also truncate an oversized CALL entry | ||
| (it accepts any `role === "toolResult"` entry with `text`). That is acceptable — a call whose | ||
| arguments exceed the budget is better truncated than dropped — and is noted rather than changed. | ||
|
|
||
| ### F7 — Repetition breaker (was claim 4) | ||
|
|
||
| `pushDeduped` collapses only *consecutive byte-identical* entries. Two different calls differ by | ||
| `call_id`, so no collapse. Two identical retried calls (same id, same arguments) would collapse | ||
| with a `produced N times in a row` note, which is the intended signal. No conflict. | ||
|
|
||
| ### F8 — BLOCKER (Medium): a documented prior rejection of this exact rendering | ||
|
|
||
| `src/adapters/cursor/request-builder.ts:223-235`, `contentPartToText`: | ||
|
|
||
| ```ts | ||
| case "toolCall": | ||
| // Cursor does not accept OpenAI Responses assistant tool-call parts as native history here. | ||
| // Rendering them as visible "[tool_call]" text leaks synthetic protocol markers back into | ||
| // model output and can halt multi-tool continuations. The paired tool result carries the | ||
| // call id/name/output Cursor needs for the next action. | ||
| return undefined; | ||
| ``` | ||
|
|
||
| This is a THIRD site (the `messages` text channel, `CursorRequestMessage`) and it explicitly | ||
| rejects rendering tool calls as `[tool_call]` text. Two honest observations: | ||
|
|
||
| 1. That channel is not the wire root replay — it feeds `activePromptText` and omission-marker | ||
| reconstruction. This phase does not touch it, and the plan must say so instead of pretending | ||
| the concern does not exist. | ||
| 2. Its stated risk — the model echoing synthetic markers back — is REAL and applies to the new | ||
| root entries. Note that root already carries `[tool_result]` markers, so the risk is already | ||
| accepted for results; adding the paired call is symmetric, not novel. | ||
|
|
||
| **Fold:** convert that residual risk into a covered one. Add `"[Tool Call]"` to `ECHO_MARKERS` in | ||
| `src/adapters/cursor/envelope-echo.ts` so the existing prefix sniffer and mid-stream observer | ||
| treat an echoed call envelope exactly like an echoed result envelope (retry with the existing | ||
| continuation text). This also answers audit question 10, and it means `010` §3.1's "recorded as | ||
| residual risk, not fixed here" is superseded — it IS fixed here. | ||
|
|
||
| ### F9 — Verifier reality (was claim 7) | ||
|
|
||
| `bun run .tmp/cursorprobe/wire.ts` and `.tmp/cursorprobe/audit.ts` both ran with exit 0 and both | ||
| import `encodeCursorRunRequest` from the change target directly. `bun x tsc --noEmit` is strict | ||
| and project-wide. `tests/cursor-blob.test.ts` decodes `rootPromptMessagesJson` from the same | ||
| function. All four observe this change. The live `codex exec` run traverses it (provider log | ||
| confirmed `turnType: tool-continuation`). | ||
|
|
||
| ## Disposition | ||
|
|
||
| | Finding | Severity | Disposition | | ||
| |---------|----------|-------------| | ||
| | F2 guard excludes `composer-2.5` | High | FOLDED into `010` §3.2 — gate on `echoToolResultInRoot` | | ||
| | F8 echoed-marker risk uncovered | Medium | FOLDED into `010` §3.1 — add `[Tool Call]` to `ECHO_MARKERS` | | ||
| | F4 `arguments` type mismatch | Medium | FOLDED into `010` §3.1 — object-only serialization with throw guard | | ||
| | F3 turns[] is display metadata | Low | Recorded; test weighting reflects it | | ||
| | F6 truncation now applies to calls | Low | Accepted, documented | | ||
| | F1/F5/F7/F9 | — | Confirmed, no change needed | | ||
|
|
||
| VERDICT: GO-WITH-FIXES (blockers=3) | ||
|
|
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.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to every reported Markdown fence.
markdownlint-cli2reports MD040 for four fences. Add an explicit language to each block:000_rca.md#L46:text000_rca.md#L64:text000_rca.md#L81:ts001_audit_round1.md#L11:text📍 Affects 1 file
devlog/_plan/260829_cursor_tool_continuation_pairing/000_rca.md#L46-L46(this comment)devlog/_plan/260829_cursor_tool_continuation_pairing/000_rca.md#L46-L46🤖 Prompt for AI Agents
Source: Linters/SAST tools