diff --git a/devlog/_plan/260829_cursor_tool_continuation_pairing/000_rca.md b/devlog/_plan/260829_cursor_tool_continuation_pairing/000_rca.md new file mode 100644 index 0000000000..b7dd694485 --- /dev/null +++ b/devlog/_plan/260829_cursor_tool_continuation_pairing/000_rca.md @@ -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). + +Implementation phases: `010` (pairing in root replay + conversation turns), `020` (regression +tests, remote verification, delivery). + diff --git a/devlog/_plan/260829_cursor_tool_continuation_pairing/001_audit_round1.md b/devlog/_plan/260829_cursor_tool_continuation_pairing/001_audit_round1.md new file mode 100644 index 0000000000..b65f47c362 --- /dev/null +++ b/devlog/_plan/260829_cursor_tool_continuation_pairing/001_audit_round1.md @@ -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 :: +``` + +## 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; +``` + +**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) + diff --git a/devlog/_plan/260829_cursor_tool_continuation_pairing/002_audit_round2_redesign.md b/devlog/_plan/260829_cursor_tool_continuation_pairing/002_audit_round2_redesign.md new file mode 100644 index 0000000000..87746c6804 --- /dev/null +++ b/devlog/_plan/260829_cursor_tool_continuation_pairing/002_audit_round2_redesign.md @@ -0,0 +1,103 @@ +# 002 — Audit round 2: the remote suite rejected the approach, and it was right + +Trigger: the full `bun run test` suite on `ssh lidge` at commit `2aaf7c7ad` failed with exactly one +test, twice (the suite runs it in two groups): + +``` +(fail) 363-B: tool result reaches the model via rootPromptMessagesJson > + assistant tool CALL is NOT replayed as [Tool Call] text (model-prompt leak guard) +``` + +## What that guard says + +`tests/cursor-tool-continuation.test.ts:87-103`: + +```ts + // Regression: a prior assistant tool call MUST NOT leak into the model-visible prompt as literal + // "[Tool Call]" text. The model few-shot-mimics that marker and emits later parallel/mixed tool + // calls as inert text instead of real tool frames (halting multi-tool continuations). + expect(serialized).not.toContain("[Tool Call]"); + // composer-2.5 still needs the paired tool RESULT echo in the model-visible prompt. + expect(serialized).toContain("FILE CONTENTS HERE"); +``` + +This is a **prior fix for the opposite failure mode of the same defect class**, and the mechanism +is the same one my patch relied on: models imitate replayed envelope shapes. My change would have +traded "the model re-runs a tool" for "the model stops emitting real tool frames" — arguably worse, +since a turn that emits inert text never calls a tool at all. + +`src/adapters/cursor/request-builder.ts:223-235` records the same rejection independently, which +`001` F8 noticed but mis-dispositioned: I read it as a risk to *cover with a sniffer* when it was +in fact a constraint to *obey*. Adding `[Tool Call]` to `ECHO_MARKERS` would only have retried the +turn after the damage, not prevented the mimicry. + +## Why the RCA still stands + +The defect in `000_rca.md` is real and reproduced: a replayed result carries a `call_id` for an +invocation the model cannot see, and it re-runs the command while narrating a phantom interrupt. +What `010` got wrong was the *remedy*, not the diagnosis. Two requirements must hold at once: + +1. The model must be able to see WHICH invocation produced a replayed result (fixes 260829). +2. There must be no standalone call-shaped template for it to copy (preserves 363-B). + +## Redesign + +Name the invocation as one descriptive line INSIDE the result envelope, instead of emitting a +separate entry: + +``` +[Tool Result] +[tool_result] +call_id: call_echo_1 +name: exec_command +invoked: exec_command with {"cmd":"echo AAA"} +is_error: false +output: +AAA +``` + +- Requirement 1 holds: the result is self-describing, so no `call_id` dangles. +- Requirement 2 holds: `invoked: …` is prose inside a result the model already never emits itself. + There is no `[Tool Call]` block anywhere in the payload — asserted for all three model classes. + +Implementation (`src/adapters/cursor/protobuf-request.ts`): + +| Element | Role | +|---------|------| +| `toolInvocationLine(call)` | renders the single `invoked: with ` line | +| `toolCallArgumentsText(args)` | `JSON.stringify` in a `try`, `[unserializable arguments]` on throw | +| `toolCallsByCallId(messages)` | indexes assistant calls by decoded call id, once per request | +| `toolResultToText(message, call?)` | inserts the line when a call matched; unchanged output when none did | + +Both replay surfaces consume it: `rootPromptMessages` (gated on `echoToolResultInRoot`, so +`composer-2.5` is covered per `001` F2) and the `conversationTurns` external branch. `envelope-echo.ts` +is reverted to its original three markers — no new marker exists to sniff. + +An unmatched `call_id` produces no invocation line rather than a fabricated one; inventing an +invocation the transcript cannot support would be a different lie than the one being fixed. + +## Verification after redesign + +| Check | Result | +|-------|--------| +| `bun x tsc --noEmit` (local) | exit 0 | +| `tests/cursor-tool-result-invocation.test.ts` (7 new) | 7 pass | +| `tests/cursor-tool-continuation.test.ts` (incl. 363-B) | pass | +| 6 Cursor suites (blob, repetition-breaker, envelope-echo-retry, request-builder, tool-continuation, new) | 184 pass / 0 fail | +| `tests/cursor-blob.test.ts` | reverted to untouched — the redesign needs no edit to an existing expectation | + +That last row matters: the first approach required weakening an existing test's step count. The +redesign changes no existing assertion, which is the honest signal that it fits the invariants +already encoded in the suite rather than renegotiating them. + +## Process note (LOOP-PESSIMIST-01) + +The dispatched plan auditor produced nothing and was retired (`001`). My direct audit confirmed the +root cause but missed this blocker, because it searched for tool-result replay sites and helper +signatures rather than asking whether the repository had already REJECTED the remedy. The remote +full-suite run caught it. Concretely: a grep for the literal string being introduced +(`rg '\[Tool Call\]' tests/`) would have found the guard in one step, before any code was written. +Recorded as the cheap check to run whenever a change introduces a model-visible marker. + +VERDICT (round 2): PASS — approach replaced, both invariants satisfied, no existing expectation weakened. + diff --git a/devlog/_plan/260829_cursor_tool_continuation_pairing/010_phase1_call_result_pairing.md b/devlog/_plan/260829_cursor_tool_continuation_pairing/010_phase1_call_result_pairing.md new file mode 100644 index 0000000000..11c8ad967d --- /dev/null +++ b/devlog/_plan/260829_cursor_tool_continuation_pairing/010_phase1_call_result_pairing.md @@ -0,0 +1,207 @@ +# 010 — Phase 1: pair assistant tool calls with their results in external replay + +Depends on: `000_rca.md` +Target file: `src/adapters/cursor/protobuf-request.ts` (MODIFY, only file changed in this phase) +Out of scope: native/composer replay, checkpoint suffix mechanics, `conversationTurns` native +branch, catalog/effort mapping, GUI, docs-site. + +## 1. Objective + +Make the external-model replayed transcript self-consistent: every `[Tool Result]` entry is +immediately preceded by a visible record of the assistant tool CALL that produced it, matched by +call id. A result whose call is missing must still be replayed (never dropped), and the native +path must be byte-for-byte untouched. + +## 2. Current code (before) + +### 2.1 `rootPromptMessages` — assistant branch (~line 285) + +```ts + } else if (message.role === "assistant") { + const text = assistantRootText(message, !externalModel).trim(); + if (text.length > 0) { + pushDeduped( + { role: "assistant", content: [{ type: "text", text }] }, + "assistant", + { messageIndex: i }, + text, + ); + } + // Assistant tool CALLS are intentionally NOT replayed as visible "[Tool Call]" text here. + } else if (message.role === "toolResult") { + if (!echoToolResultInRoot) continue; + const prefix = normalizedToolResult(message, contentToText(message.content)).isError ? "[Tool Error]" : "[Tool Result]"; + const text = `${prefix}\n${toolResultToText(message)}`; + pushDeduped(toolResultRootPayload(text), "toolResult", { messageIndex: i, text }, text); + } +``` + +### 2.2 `conversationTurns` — external assistant branch (~line 742) + +```ts + if (externalModel) { + if (part.type === "text" && part.text.length > 0) { + current.steps.push(storeCursorBlob(...AssistantMessageSchema, { text: part.text }...)); + } + continue; + } +``` + +A `toolCall` part hits `continue` and vanishes. + +## 3. Change (after) + +### 3.1 NEW: a call-record formatter + +Add next to `toolResultToText` (which owns the mirror-image format for results): + +```ts +/** + * External replay must show the CALL that produced a replayed "[Tool Result]" entry. Without it the + * result is orphaned: its call_id refers to nothing the model can see, and live grok-4.6 turns then + * re-issue the same call while narrating a phantom interrupt (devlog 260829 000_rca). + * Mirrors toolResultToText so a call/result pair reads as one record. + */ +function toolCallToText(part: Extract): string { + return [ + "[tool_call]", + `call_id: ${decodeCursorCallId(part.id)}`, + `name: ${namespacedToolName(part.namespace, part.name)}`, + "arguments:", + cursorToolCallArgumentsText(part.arguments), + ].join("\n"); +} +``` + +**AMENDED by `001_audit_round1.md` F4:** `OcxToolCall.arguments` is `Record` +(`src/types/request.ts:215`) — always an object, never a string. Serialize with `JSON.stringify` +inside a `try`, falling back to `"[unserializable arguments]"`, so a cyclic or `BigInt`-bearing +argument object cannot throw inside request encoding. + +Prefix wording: `[Tool Call]` on the first line to match the `[Tool Result]`/`[Tool Error]` +family. **AMENDED by `001_audit_round1.md` F8:** `ECHO_MARKERS` in `envelope-echo.ts` must gain +`"[Tool Call]"` in the same change, so the existing prefix sniffer and mid-stream observer treat an +echo of a call envelope exactly like an echoed result envelope. `request-builder.ts:223` records a +prior rejection of rendering tool calls as visible text precisely because a model may echo the +marker back; covering the marker is what makes this emission safe rather than a repeat of that +mistake. The `messages` text channel that comment governs is NOT touched by this phase. + +### 3.2 MODIFY `rootPromptMessages` assistant branch + +Replace the comment-only omission with an emission that is *conditional on the call having a +replayed result in this same history slice*, so a call whose result was pruned does not reintroduce +an orphan in the other direction: + +```ts + } else if (message.role === "assistant") { + const text = assistantRootText(message, !externalModel).trim(); + if (text.length > 0) { /* unchanged pushDeduped */ } + // Replay the tool CALL so the paired "[Tool Result]" below is not orphaned (000_rca). + // Native models receive real mcpToolCall structures on turns[]; only the external + // text-replay path needs this, and only when results are echoed into root at all. + // AMENDED by 001_audit_round1.md F2: gate on echoToolResultInRoot ALONE, not on + // externalModel. externalModel implies echoToolResultInRoot, so the conjunct was dead in one + // direction and wrong in the other: composer-2.5 (non-fast) is NATIVE yet has + // echoToolResultInRoot === true, so it writes an orphaned [Tool Result] into root and needs + // the pairing call too. Invariant: wherever a result is echoed into root as text, its call + // must be there as well. + if (echoToolResultInRoot && Array.isArray(message.content)) { + for (const part of message.content) { + if (part.type !== "toolCall") continue; + const callText = `[Tool Call]\n${toolCallToText(part)}`; + pushDeduped(toolResultRootPayload(callText), "toolResult", { messageIndex: i, text: callText }, callText); + } + } + } +``` + +`toolResultRootPayload` is reused because it already produces the `{role:"assistant"}` wire shape +that external workers accept (the `role` label passed to `pushDeduped` is internal bookkeeping used +by the pruner, and `toolResult` is the correct class for "part of the active tool block" so the +pruner's `activeStart` walk keeps a call attached to its result). + +**Ordering guarantee:** the call is emitted while processing the assistant message at index `i`, +and its result arrives at a later index, so call-before-result ordering follows from the existing +loop order — no sorting needed. + +### 3.3 MODIFY `conversationTurns` external branch + +```ts + if (externalModel) { + if (part.type === "text" && part.text.length > 0) { /* unchanged */ } + else if (part.type === "toolCall") { + current.steps.push(storeCursorBlob(toBinary(ConversationStepSchema, create(ConversationStepSchema, { + message: { case: "assistantMessage", value: create(AssistantMessageSchema, { + text: `[Tool Call]\n${toolCallToText(part)}`, + }) }, + })), requestScope)); + } + continue; + } +``` + +Still an `assistantMessage` step — never a native `mcpToolCall` — preserving the constraint the +existing comment records (native structures make external workers reject the turn with +`invalid_argument` after `stepCompleted`). + +### 3.4 Pruner interaction (`activeStart` walk, ~line 322) + +```ts + while (activeStart > 0 && history[activeStart - 1]?.role === "toolResult") activeStart -= 1; +``` + +Because call entries are classed `toolResult`, this walk already treats a call+result block as one +active unit. The orphan guard below it (`while (historyEntries[0]?.role === "assistant" || ... === "toolResult")`) +also keeps behaving correctly: a leading call with no result is shifted off with the rest. + +Byte-budget note: each call record adds roughly the length of the arguments JSON. The existing +`CURSOR_EXTERNAL_ROOT_BYTE_LIMIT` (512 KiB) and `CURSOR_EXTERNAL_ROOT_BLOB_LIMIT` (192) still +bound it, and `truncateToolResultBlob` applies to the active block. No limit change in this phase. + +## 4. Accept criteria (testable) + +| # | Criterion | Activation scenario (C-ACTIVATION-GROUNDING-01) | +|---|-----------|--------------------------------------------------| +| A1 | External continuation roots contain a `[Tool Call]` entry carrying the call id, tool name and arguments, positioned before its `[Tool Result]` entry | Decode `rootPromptMessagesJson` for a grok history with one call+result; assert index(call) < index(result) and both share the call id | +| A2 | A result with no matching call is still replayed | History with a `toolResult` whose id matches no call → result entry still present | +| A3 | A call with no result does not crash and does not desync ordering | Assistant `toolCall` as the last message → encode succeeds, call entry present | +| A4 | Native/composer replay is unaffected | Same history encoded with `composer-2.5-fast`: no `[Tool Call]` text entry appears in roots | +| A5 | Live behavior | `codex exec --json -m cursor/grok-4.6` with two sequential echo commands → exactly one `command_execution` per command, zero "was interrupted" strings | + +A1-A4 are logic assertions in `tests/`. A5 is the live grounding that closes the reported symptom. + +## 5. Verifier commands (PLAN-VERIFIER-REAL-01) + +Verified before writing this doc: + +| Command | Exit | Reads this change target? | +|---------|------|---------------------------| +| `bun run .tmp/cursorprobe/wire.ts` | 0 | Yes — imports `encodeCursorRunRequest` from the target file directly; this is the probe that produced §3 of `000_rca.md` | +| `bun test tests/cursor-blob.test.ts` | to run on lidge | Yes — decodes `rootPromptMessagesJson` from `encodeCursorRunRequest` | +| `bun x tsc --noEmit` | to run on lidge | Yes — strict project-wide typecheck includes `src/adapters/cursor/**` | +| `codex exec --json -m cursor/grok-4.6` | 0 in run2 | Yes — traverses the live proxy through this exact encode path (`turnType: tool-continuation` confirmed in provider logs) | + +Per the user's instruction the bun suites run on `ssh lidge`, not locally. + +## 6. Field chain (PLAN-FIELD-CHAIN-01) + +No new type field or enum value is introduced. The chain for the value that IS added (a replayed +call record) is: + +| Stage | Location | +|-------|----------| +| Creation | `toolCallToText` (new) fed from existing `OcxAssistantContentPart` `toolCall` parts already present in `rawMessages` | +| Serialization | `toolResultRootPayload` → `rootBlobCandidate` → `storeCursorBlob` (existing) | +| Deserialization | N/A — the blob is consumed by Cursor upstream, not re-read by opencodex. Tests decode it via `handleCursorNativeKv`, the same path `tests/cursor-blob.test.ts` already uses | +| Consumers | Root pruner (`activeStart` walk, orphan guard, byte budget) and the token estimate via `serialized` — both handled in §3.4 | + +## 7. Bypass / enforcement (PLAN-BYPASS-NAMED-01) + +This phase adds no enforcement gate; it changes request construction. For completeness: +tier E1 (unit test), executing surface `bun test` in CI, known bypass — a caller constructing a +Cursor request without `rawMessages` skips replay entirely (unchanged pre-existing behavior), +residual risk — none for the echoed-marker case, which F8 closed by adding `[Tool Call]` to +`ECHO_MARKERS`; the remaining residual is that an oversized call record can be truncated by +`truncateToolResultBlob` (accepted, 001 F6), wording downgrade — none. Final enforcement layer: +none beyond CI tests. + diff --git a/devlog/_plan/260829_cursor_tool_continuation_pairing/020_phase2_tests_and_delivery.md b/devlog/_plan/260829_cursor_tool_continuation_pairing/020_phase2_tests_and_delivery.md new file mode 100644 index 0000000000..5846385344 --- /dev/null +++ b/devlog/_plan/260829_cursor_tool_continuation_pairing/020_phase2_tests_and_delivery.md @@ -0,0 +1,97 @@ +# 020 — Phase 2: regression tests, remote verification, delivery + +Depends on: `010_phase1_call_result_pairing.md` (landed) +Targets: `tests/cursor-tool-call-replay.test.ts` (NEW), PR against `dev` +Out of scope: touching `src/` again except to fix a defect this phase's tests expose. + +## 1. NEW test file — `tests/cursor-tool-call-replay.test.ts` + +Follows the decode harness already established by `tests/cursor-repetition-breaker.test.ts` and +`tests/cursor-blob.test.ts`: encode through `encodeCursorRunRequest`, then resolve every +`rootPromptMessagesJson` blob through the real `handleCursorNativeKv` blob store. No mocks — +a mocked blob store would not prove what the wire carries. + +```ts +import { describe, expect, test } from "bun:test"; +import { create, fromBinary } from "@bufbuild/protobuf"; +import { encodeCursorRunRequest } from "../src/adapters/cursor/protobuf-request"; +import { handleCursorNativeKv } from "../src/adapters/cursor/native-exec"; +import { AgentClientMessageSchema, GetBlobArgsSchema, KvServerMessageSchema } from "../src/adapters/cursor/gen/agent_pb"; +import type { OcxMessage } from "../src/types"; + +function blobData(blobId: Uint8Array): Uint8Array { /* same helper as cursor-repetition-breaker */ } +function rootTexts(bytes: Uint8Array): string[] { /* JSON.parse each root, return content[0].text */ } + +const CALL_ID = "call_echo_1"; +function historyWithCall(resultId = CALL_ID): OcxMessage[] { + return [ + { role: "user", content: "Run echo AAA.", timestamp: 1 }, + { role: "assistant", content: [ + { type: "text", text: "I will run echo AAA." }, + { type: "toolCall", id: CALL_ID, name: "exec_command", arguments: { cmd: "echo AAA" } }, + ], timestamp: 2 }, + { role: "toolResult", toolCallId: resultId, toolName: "exec_command", content: "AAA", isError: false, timestamp: 3 }, + ]; +} +``` + +### Test cases + +| Test | Asserts | Maps to | +|------|---------|---------| +| `external replay pairs a tool call with its result` | a root contains `[Tool Call]` + `call_id: call_echo_1` + `exec_command` + `echo AAA`; its index is LESS than the index of the `[Tool Result]` root | A1 | +| `a replayed tool result is never orphaned` | for every root matching `[Tool Result]` with `call_id: X`, some earlier root carries `[Tool Call]` with the same `X` | A1 (the invariant, stated directly) | +| `an unmatched result is still replayed` | `historyWithCall("call_other")` → the `[Tool Result]` root is still present (no silent drop) | A2 | +| `a trailing call with no result encodes` | history ending at the assistant `toolCall` → no throw, `[Tool Call]` root present | A3 | +| `native replay does not gain a tool-call text entry` | same history at `composer-2.5-fast` → no root contains `[Tool Call]` | A4 | +| `arguments serialize for string and object forms` | `arguments` given as a JSON string and as an object both surface the `cmd` value | §3.1 defensive serialization | + +The orphan-invariant test is the load-bearing one: it is written to FAIL on the pre-fix code +(run it before the `src` change to confirm red), which is what makes it a regression test rather +than a restatement of current behavior. + +## 2. Remote verification (user instruction: never run the local suite) + +```bash +ssh lidge 'cd && git fetch origin && git checkout && bun install --frozen-lockfile' +ssh lidge 'cd && bun x tsc --noEmit' +ssh lidge 'cd && bun test tests/cursor-tool-call-replay.test.ts tests/cursor-blob.test.ts \ + tests/cursor-repetition-breaker.test.ts tests/cursor-request-builder.test.ts' +``` + +Because the change touches shared request construction for every Cursor model, the full +`bun run test` suite also runs on lidge before the PR is marked review-ready (AGENTS.md requires +typecheck + test before a non-trivial PR is review-ready). + +## 3. Live grounding (A5) + +Re-run the exact reproduction from `000_rca.md` against the patched proxy: + +```bash +OPENAI_BASE_URL=http://127.0.0.1:10100/v1 codex exec --json --skip-git-repo-check \ + -m cursor/grok-4.6 '...echo AAA then echo BBB... reply DONE2' +``` + +Pass condition: exactly one `command_execution` item per requested command, and zero occurrences of +`interrupted` in `agent_message` text. The proxy must be restarted onto the patched code first — +the running service is a separate long-lived process (pid observed at 62773), so an unrestarted +proxy would test the old bytes. Record the restarted pid alongside the transcript. + +## 4. Delivery + +- Branch `codex/cursor-tool-call-replay-pairing` off current `dev`. +- PR targets `dev` (never `main`), fills Summary / Verification / Checklist from + `.github/PULL_REQUEST_TEMPLATE.md`. No `gui` mention, so no screenshot requirement. +- Commits are pushed with `--no-verify` per the user's explicit instruction; the independent + gates are the remote lidge runs plus repository CI at the exact head SHA. +- Merge: `--admin` once CI is green at the exact head SHA, per the user's explicit + authorization for this task. Verify CI is reported for the SHA that is actually being merged, + not an earlier push. + +## 5. Terminal outcomes + +- `DONE` — A1-A5 green, remote gates green, PR merged into `dev` with the merge commit recorded. +- `BLOCKED` — lidge unreachable or CI infrastructure failure. +- `NEEDS_HUMAN` — the live re-run still shows duplicate calls after the fix, meaning the root + cause is broader than replay pairing (would reopen at P with the new trace, not be patched blind). + diff --git a/src/adapters/cursor/protobuf-request.ts b/src/adapters/cursor/protobuf-request.ts index 256af86ab9..de62882d7f 100644 --- a/src/adapters/cursor/protobuf-request.ts +++ b/src/adapters/cursor/protobuf-request.ts @@ -218,6 +218,9 @@ function rootPromptMessages(request: CursorRunRequest, requestScope: CursorBlobR const externalModel = isCursorExternalWireModel(request.modelId); const echoToolResultInRoot = cursorNeedsExternalToolContinuation(request.modelId); + // Replayed results name the invocation that produced them; without it the result is orphaned + // (devlog 260829 000_rca). Indexed once per request rather than rescanned per result. + const replayedCalls = echoToolResultInRoot ? toolCallsByCallId(messages) : undefined; const lastRawIsToolResult = messages.at(-1)?.role === "toolResult"; const activeUserIndex = lastRawIsToolResult ? -1 : lastActionIndex(messages); // Repetition breaker (devlog 260826 gap-9): external full-replay flattens history to text, @@ -287,7 +290,11 @@ function rootPromptMessages(request: CursorRunRequest, requestScope: CursorBlobR text, ); } - // Assistant tool CALLS are intentionally NOT replayed as visible "[Tool Call]" text here. + // Assistant tool CALLS are NOT replayed as a separate visible "[Tool Call]" entry: a model + // few-shot-mimics that marker and emits later tool calls as inert text (363-B guard in + // tests/cursor-tool-continuation.test.ts). The invocation is instead named INSIDE the paired + // "[Tool Result]" envelope below, which carries the same information without a mimickable + // call template (devlog 260829 002_audit_round2). } else if (message.role === "toolResult") { // Native resume models already receive the paired MCP result through turns[]. Replaying // the same payload as assistant-role "[Tool Result]" / "[tool_result]" text teaches Auto @@ -296,7 +303,7 @@ function rootPromptMessages(request: CursorRunRequest, requestScope: CursorBlobR // #1920: the prefix must reflect the NORMALIZED error state (an empty // node_repl result is an error even when the runtime said isError=false). const prefix = normalizedToolResult(message, contentToText(message.content)).isError ? "[Tool Error]" : "[Tool Result]"; - const text = `${prefix}\n${toolResultToText(message)}`; + const text = `${prefix}\n${toolResultToText(message, replayedCalls?.get(decodeCursorCallId(message.toolCallId)))}`; pushDeduped(toolResultRootPayload(text), "toolResult", { messageIndex: i, text }, text); } } @@ -660,12 +667,85 @@ function toolResultContentItems( return items; } -function toolResultToText(message: OcxToolResultMessage): string { +/** + * Serialize tool-call arguments for the replayed transcript. `OcxToolCall.arguments` is always an + * object, but it originates in provider JSON, so a cyclic or BigInt-bearing value must degrade to a + * marker instead of throwing inside request encoding. + */ +function toolCallArgumentsText(args: Record): string { + try { + return JSON.stringify(args) ?? "[unserializable arguments]"; + } catch { + return "[unserializable arguments]"; + } +} + +/** + * The invocation that produced a replayed tool result, rendered as ONE descriptive line inside the + * result envelope. + * + * Why not a separate "[Tool Call]" entry: a model few-shot-mimics that marker and starts emitting + * later tool calls as inert text instead of real tool frames, which halts multi-tool continuations + * (363-B guard, tests/cursor-tool-continuation.test.ts). Why it must exist at all: without any + * record of the invocation, the replayed result is orphaned — its `call_id` refers to nothing the + * model can see — and live cursor/grok-4.6 turns re-ran commands that had already succeeded while + * narrating a phantom interrupt (devlog 260829 000_rca). A prose line inside the result satisfies + * both: the invocation is visible, but there is no call-shaped template to copy. + */ +function toolInvocationLine(call: Extract): string { + return `invoked: ${namespacedToolName(call.namespace, call.name)} with ${toolCallArgumentsText(call.arguments)}`; +} + +/** + * Index assistant tool calls by decoded call id so a replayed result can name its invocation. + * + * A call id is supposed to be unique, but nothing upstream guarantees it across a long history, and + * `decodeCursorCallId` can map distinct wire ids onto the same decoded id. Two calls sharing one id + * would make the LAST one describe every result bearing it, so an early result could be labelled + * with a later command — a wrong invocation is worse than none, since it is the kind of mislabel the + * model cannot detect. Keep the FIRST call for an id (results follow their call, so the first + * binding is the one an earlier result belongs to) and drop the ambiguous id entirely once a second + * distinct call claims it, which degrades to the honest no-invocation-line path. + */ +function toolCallsByCallId(messages: readonly OcxMessage[]): Map> { + const calls = new Map>(); + const ambiguous = new Set(); + for (const message of messages) { + if (message.role !== "assistant" || !Array.isArray(message.content)) continue; + for (const part of message.content) { + if (part.type !== "toolCall") continue; + const callId = decodeCursorCallId(part.id); + if (ambiguous.has(callId)) continue; + const existing = calls.get(callId); + if (!existing) { + calls.set(callId, part); + continue; + } + // Same id, and not the same invocation: neither claim can be trusted for a given result. + if (existing.name !== part.name || toolCallArgumentsText(existing.arguments) !== toolCallArgumentsText(part.arguments)) { + calls.delete(callId); + ambiguous.add(callId); + } + } + } + return calls; +} + +/** + * The replayed text of one tool result. When `call` is supplied, the invocation that produced it is + * named inline so the result is not orphaned; when it is absent (no match, or an ambiguous call id) + * the envelope is emitted unchanged rather than guessing. + */ +function toolResultToText( + message: OcxToolResultMessage, + call?: Extract, +): string { const normalized = normalizedToolResult(message, contentToText(message.content)); return [ "[tool_result]", `call_id: ${decodeCursorCallId(message.toolCallId)}`, `name: ${namespacedToolName(message.toolNamespace, message.toolName)}`, + ...(call ? [toolInvocationLine(call)] : []), `is_error: ${normalized.isError}`, "output:", normalized.text, @@ -806,6 +886,7 @@ function conversationTurns( const externalModel = isCursorExternalWireModel(request.modelId); const historyEnd = messages.at(-1)?.role === "toolResult" ? messages.length : Math.max(0, end); const start = externalModel ? Math.max(0, historyMessageStart) : 0; + const turnCalls = externalModel ? toolCallsByCallId(messages) : undefined; const turns: Uint8Array[] = []; let current: { userMessage: Uint8Array; steps: Uint8Array[] } | undefined; const pendingToolCalls = new Map>(); @@ -857,10 +938,14 @@ function conversationTurns( // reported repro path for empty Computer Use results. const normalized = normalizedToolResult(message, contentToText(message.content)); const prefix = normalized.isError ? "[Tool Error]" : "[Tool Result]"; + // Name the invocation here as well, for the same reason the root replay does: a result with + // no visible originating call reads as an interrupted attempt (devlog 260829 000_rca). +const call = turnCalls?.get(decodeCursorCallId(message.toolCallId)); + const invocation = call ? `${toolInvocationLine(call)}\n` : ""; current.steps.push(storeCursorBlob(toBinary(ConversationStepSchema, create(ConversationStepSchema, { message: { case: "assistantMessage", - value: create(AssistantMessageSchema, { text: `${prefix}\n${normalized.text}` }), + value: create(AssistantMessageSchema, { text: `${prefix}\n${invocation}${normalized.text}` }), }, })), requestScope)); continue; diff --git a/tests/cursor-tool-result-invocation.test.ts b/tests/cursor-tool-result-invocation.test.ts new file mode 100644 index 0000000000..f81ed1e29a --- /dev/null +++ b/tests/cursor-tool-result-invocation.test.ts @@ -0,0 +1,212 @@ +import { describe, expect, test } from "bun:test"; +import { create, fromBinary } from "@bufbuild/protobuf"; +import { encodeCursorRunRequest } from "../src/adapters/cursor/protobuf-request"; +import { handleCursorNativeKv } from "../src/adapters/cursor/native-exec"; +import { + AgentClientMessageSchema, + ConversationStepSchema, + ConversationTurnStructureSchema, + GetBlobArgsSchema, + KvServerMessageSchema, +} from "../src/adapters/cursor/gen/agent_pb"; +import type { OcxMessage } from "../src/types"; + +function blobData(blobId: Uint8Array): Uint8Array { + const reply = fromBinary(AgentClientMessageSchema, handleCursorNativeKv(create(KvServerMessageSchema, { + id: 1, + message: { case: "getBlobArgs", value: create(GetBlobArgsSchema, { blobId }) }, + }))); + if (reply.message.case !== "kvClientMessage" || reply.message.value.message.case !== "getBlobResult") { + throw new Error("expected getBlobResult"); + } + return reply.message.value.message.value.blobData!; +} + +function runRequest(bytes: Uint8Array) { + const msg = fromBinary(AgentClientMessageSchema, bytes); + return msg.message.case === "runRequest" ? msg.message.value : undefined; +} + +/** Model-visible text of every root prompt blob, in wire order. */ +function rootTexts(bytes: Uint8Array): string[] { + return (runRequest(bytes)?.conversationState?.rootPromptMessagesJson ?? []).map(blobId => { + const parsed = JSON.parse(new TextDecoder().decode(blobData(blobId))) as { + content?: string | [{ text?: string }]; + }; + const content = parsed.content; + if (typeof content === "string") return content; + return content?.[0]?.text ?? ""; + }); +} + +/** Assistant text of every conversation-turn step, in wire order. */ +function turnStepTexts(bytes: Uint8Array): string[] { + const texts: string[] = []; + for (const turnId of runRequest(bytes)?.conversationState?.turns ?? []) { + const turn = fromBinary(ConversationTurnStructureSchema, blobData(turnId)); + if (turn.turn.case !== "agentConversationTurn") continue; + for (const stepId of turn.turn.value.steps) { + const step = fromBinary(ConversationStepSchema, blobData(stepId)); + if (step.message.case === "assistantMessage") texts.push(step.message.value.text); + } + } + return texts; +} + +const CALL_ID = "call_echo_1"; + +function history(options: { resultCallId?: string } = {}): OcxMessage[] { + return [ + { role: "user", content: "Run echo AAA.", timestamp: 1 }, + { + role: "assistant", + content: [ + { type: "text", text: "I will run echo AAA." }, + { type: "toolCall", id: CALL_ID, name: "exec_command", arguments: { cmd: "echo AAA" } }, + ], + timestamp: 2, + }, + { + role: "toolResult", + toolCallId: options.resultCallId ?? CALL_ID, + toolName: "exec_command", + content: "AAA", + isError: false, + timestamp: 3, + }, + ]; +} + +function encode(messages: OcxMessage[], modelId: string): Uint8Array { + return encodeCursorRunRequest({ + modelId, + conversationId: "c_pairing", + system: [], + messages: [], + rawMessages: messages, + }); +} + +function resultRoot(bytes: Uint8Array): string | undefined { + return rootTexts(bytes).find(text => text.startsWith("[Tool Result]") || text.startsWith("[Tool Error]")); +} + +/** + * devlog 260829: a replayed tool result used to carry no record of the invocation that produced it, + * so the model saw a `call_id` referring to nothing it could see. Live cursor/grok-4.6 turns then + * re-ran commands that had already succeeded (exit 0) and narrated a phantom "was interrupted". + * + * The invocation is named INSIDE the result envelope rather than as a separate "[Tool Call]" entry: + * the 363-B guard in cursor-tool-continuation.test.ts shows a standalone call marker gets + * few-shot-mimicked, after which the model emits later tool calls as inert text. These assertions + * decode the real wire payload, since roots are what Cursor builds the model prompt from. + */ +describe("cursor replayed tool results name their invocation", () => { + test("the result envelope names the tool and arguments that produced it", () => { + const root = resultRoot(encode(history(), "grok-4.6-high")); + expect(root).toBeDefined(); + expect(root).toContain(`call_id: ${CALL_ID}`); + expect(root).toContain("invoked: exec_command with"); + expect(root).toContain("echo AAA"); + }); + + test("no standalone [Tool Call] entry is ever emitted (363-B mimicry guard)", () => { + for (const modelId of ["grok-4.6-high", "composer-2.5", "composer-2.5-fast"]) { + const bytes = encode(history(), modelId); + expect(rootTexts(bytes).some(text => text.includes("[Tool Call]"))).toBe(false); + expect(turnStepTexts(bytes).some(text => text.includes("[Tool Call]"))).toBe(false); + } + }); + + test("the invocation line also reaches the conversation-turn step", () => { + const step = turnStepTexts(encode(history(), "grok-4.6-high")) + .find(text => text.startsWith("[Tool Result]")); + expect(step).toBeDefined(); + expect(step).toContain("invoked: exec_command with"); + }); + + // composer-2.5 (non-fast) is a NATIVE wire model that still routes through the external + // tool-continuation path (discovery.ts cursorNeedsExternalToolContinuation), so it echoes results + // into root as text and needs the invocation named too. Gating on `externalModel` would have + // skipped exactly this model (audit 001 F2). + test("composer-2.5 root replay names the invocation too", () => { + const root = resultRoot(encode(history(), "composer-2.5")); + expect(root).toBeDefined(); + expect(root).toContain("invoked: exec_command with"); + }); + + test("a result whose call id matches nothing is still replayed, without an invocation line", () => { + const root = resultRoot(encode(history({ resultCallId: "call_other" }), "grok-4.6-high")); + expect(root).toBeDefined(); + expect(root).toContain("call_id: call_other"); + expect(root).not.toContain("invoked:"); + }); + + test("native composer replay keeps results off the root prompt entirely", () => { + const bytes = encode(history(), "composer-2.5-fast"); + expect(rootTexts(bytes).some(text => text.startsWith("[Tool Result]"))).toBe(false); + expect(rootTexts(bytes).some(text => text.includes("invoked:"))).toBe(false); + }); + + // A reused call id must not let a LATER command describe an EARLIER result: a confidently wrong + // invocation line is worse than none, because nothing downstream can detect the mislabel. + test("a call id claimed by two different invocations yields no invocation line", () => { + const messages: OcxMessage[] = [ + { role: "user", content: "Run both.", timestamp: 1 }, + { + role: "assistant", + content: [{ type: "toolCall", id: CALL_ID, name: "exec_command", arguments: { cmd: "echo FIRST" } }], + timestamp: 2, + }, + { role: "toolResult", toolCallId: CALL_ID, toolName: "exec_command", content: "FIRST", isError: false, timestamp: 3 }, + { + role: "assistant", + content: [{ type: "toolCall", id: CALL_ID, name: "exec_command", arguments: { cmd: "echo SECOND" } }], + timestamp: 4, + }, + { role: "toolResult", toolCallId: CALL_ID, toolName: "exec_command", content: "SECOND", isError: false, timestamp: 5 }, + ]; + const roots = rootTexts(encode(messages, "grok-4.6-high")); + const results = roots.filter(text => text.startsWith("[Tool Result]")); + expect(results.length).toBeGreaterThan(0); + // Neither result may claim an invocation, and in particular none may name the wrong command. + for (const result of results) expect(result).not.toContain("invoked:"); + }); + + test("a call id repeated for the SAME invocation still names it", () => { + const messages: OcxMessage[] = [ + { role: "user", content: "Run it twice.", timestamp: 1 }, + { + role: "assistant", + content: [{ type: "toolCall", id: CALL_ID, name: "exec_command", arguments: { cmd: "echo AAA" } }], + timestamp: 2, + }, + { role: "toolResult", toolCallId: CALL_ID, toolName: "exec_command", content: "AAA", isError: false, timestamp: 3 }, + { + role: "assistant", + content: [{ type: "toolCall", id: CALL_ID, name: "exec_command", arguments: { cmd: "echo AAA" } }], + timestamp: 4, + }, + { role: "toolResult", toolCallId: CALL_ID, toolName: "exec_command", content: "AAA", isError: false, timestamp: 5 }, + ]; + const root = resultRoot(encode(messages, "grok-4.6-high")); + expect(root).toContain("invoked: exec_command with"); + }); + + test("unserializable arguments do not break request encoding", () => { + const cyclic: Record = {}; + cyclic.self = cyclic; + const messages: OcxMessage[] = [ + { role: "user", content: "Run it.", timestamp: 1 }, + { + role: "assistant", + content: [{ type: "toolCall", id: CALL_ID, name: "exec_command", arguments: cyclic }], + timestamp: 2, + }, + { role: "toolResult", toolCallId: CALL_ID, toolName: "exec_command", content: "ok", isError: false, timestamp: 3 }, + ]; + const root = resultRoot(encode(messages, "grok-4.6-high")); + expect(root).toBeDefined(); + expect(root).toContain("[unserializable arguments]"); + }); +});