-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(cursor): stop grok-4.6 tool-result echo from poisoning later turns #4929
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
6dbd352
98f70c9
4c52021
cced7bf
9dc23ab
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 |
|---|---|---|
|
|
@@ -34,9 +34,12 @@ import { estimateTokens } from "../lib/token-estimate"; | |
| import { | ||
| clearCursorIncompleteToolRemint, | ||
| cursorIncompleteToolRemintScopeKey, | ||
| clearCursorEnvelopeEchoRemint, | ||
| cursorEnvelopeEchoRemintScopeKey, | ||
| cursorOverflowRemintScopeKey, | ||
| markCursorOverflowSurfaced, | ||
| recordCursorIncompleteToolRemint, | ||
| recordCursorEnvelopeEchoRemint, | ||
| recordCursorOverflowRemint, | ||
| rememberCursorThreadConversation, | ||
| shouldSkipCursorOverflowRemint, | ||
|
|
@@ -206,6 +209,7 @@ export function createCursorAdapter(provider: OcxProviderConfig, deps: CursorAda | |
| let lastTransport: { captured?: Uint8Array } | undefined; | ||
| let emittedClientTool = false; | ||
| let sawIncompleteToolCall = false; | ||
| let sawMidstreamEnvelopeEcho = false; | ||
| // Ordering proof for tool-suspended checkpoints: true only when the newest captured | ||
| // checkpoint bytes arrived AFTER the turn emitted a client tool call, i.e. upstream | ||
| // serialized its suspended-on-tool-call state. Only that snapshot can safely resume | ||
|
|
@@ -390,7 +394,9 @@ export function createCursorAdapter(provider: OcxProviderConfig, deps: CursorAda | |
| } | ||
| if (event.type !== "heartbeat") emittedOutput = true; | ||
| if (event.type === "done") { | ||
| for (const finding of midstreamObserver?.findings() ?? []) { | ||
| const midstreamFindings = midstreamObserver?.findings() ?? []; | ||
| if (midstreamFindings.length > 0) sawMidstreamEnvelopeEcho = true; | ||
| for (const finding of midstreamFindings) { | ||
| debugProviderDiagnostic("cursor", "midstream-envelope-echo", { | ||
| wireModel: activeRequest.modelId, | ||
| conversationHash: activeRequest.conversationId.slice(0, 16), | ||
|
|
@@ -564,6 +570,41 @@ export function createCursorAdapter(provider: OcxProviderConfig, deps: CursorAda | |
| } else if (!sawIncompleteToolCall && completedNormally && incompleteToolRemintScopeKey) { | ||
| clearCursorIncompleteToolRemint(incompleteToolRemintScopeKey); | ||
| } | ||
| // A mid-stream envelope echo has ALREADY reached the client — the prefix sniffer only | ||
| // watches the first bytes of a turn, and grok-4.6 writes a real sentence before pasting | ||
| // the envelope. It cannot be quarantined, so the recovery is the same as the | ||
| // incomplete-tool case: leave this turn alone and rotate the next turn's id, otherwise | ||
| // the stored echo is replayed and primes the model to echo again. | ||
| // | ||
| // Its own budget, not the incomplete-tool one: echoing is cheap and repeatable while an | ||
| // incomplete client-tool stream is rare and structural, so a shared counter would let a | ||
| // persistently echoing model spend the allowance the other recovery needs. Skipped when | ||
| // the incomplete-tool arm already reminted this turn — one rotation is enough. | ||
| const envelopeEchoRemintScopeKey = | ||
| _parsed._cursorIsolateConversation !== true | ||
| && request.contextUsageStoreCheckpoints !== false | ||
| ? cursorEnvelopeEchoRemintScopeKey( | ||
| cursorClientThreadOwner(_parsed), | ||
| _parsed._cursorIdentityScope, | ||
| ) | ||
| : null; | ||
| if (sawMidstreamEnvelopeEcho && !sawIncompleteToolCall && envelopeEchoRemintScopeKey) { | ||
| if (recordCursorEnvelopeEchoRemint(envelopeEchoRemintScopeKey)) { | ||
| if (inheritedCheckpointRef) invalidateCursorCheckpoint(inheritedCheckpointRef); | ||
|
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 🔎 Supported by static analysis🏁 Script executed: sed -n '220,285p' src/adapters/cursor.ts
sed -n '430,465p' src/adapters/cursor.ts
sed -n '560,620p' src/adapters/cursor.ts
sed -n '430,490p' src/adapters/cursor/request-builder.ts
sed -n '300,345p' src/adapters/cursor/checkpoint-store.ts
rg -n "commitCapturedCheckpoint|checkpointRef|remintConversationId|resolveCursorCheckpoint" src/adapters/cursor.ts src/adapters/cursor/request-builder.ts src/adapters/cursorRepository: lidge-jun/opencodex Length of output: 15481 🏁 Script executed: sed -n '175,280p' src/adapters/cursor.ts
sed -n '390,455p' src/adapters/cursor.ts
sed -n '500,535p' src/adapters/cursor.ts
sed -n '615,640p' src/adapters/cursor.ts
sed -n '495,525p' src/adapters/cursor/request-builder.tsRepository: lidge-jun/opencodex Length of output: 14123 Invalidate and clear the current checkpoint before reminting. Invalidate the current continuation reference and remove 🤖 Prompt for AI Agents |
||
| debugProviderDiagnostic("cursor", "midstream-envelope-echo-remint", { | ||
| wireModel: request.modelId, | ||
| conversationHash: request.conversationId.slice(0, 16), | ||
| }); | ||
| remintConversationId(request.conversationId); | ||
| } else { | ||
| debugProviderDiagnostic("cursor", "midstream-envelope-echo-remint-exhausted", { | ||
| wireModel: request.modelId, | ||
| conversationHash: request.conversationId.slice(0, 16), | ||
| }); | ||
| } | ||
| } else if (!sawMidstreamEnvelopeEcho && completedNormally && envelopeEchoRemintScopeKey) { | ||
| clearCursorEnvelopeEchoRemint(envelopeEchoRemintScopeKey); | ||
| } | ||
| if ( | ||
| request.checkpointInvalidationReason | ||
| && request.checkpointInvalidationReason !== "missing_ref" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,58 @@ | |
| */ | ||
|
|
||
| const ECHO_MARKERS = ["[Tool Result]", "[Tool Error]", "[tool_result]"] as const; | ||
|
|
||
| function isEchoMarkerLine(line: string): boolean { | ||
| return (ECHO_MARKERS as readonly string[]).includes(line.replace(/^[ \t]+/, "")); | ||
|
Comment on lines
+17
to
+18
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 the model emits a marker such as Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| /** | ||
| * Drop echoed tool-result envelopes from assistant history before Cursor root replay. | ||
| * | ||
| * The prefix sniffer catches an echo that STARTS a turn, but grok-4.6 routinely writes a real | ||
| * sentence first and pastes the envelope after it. That text has already reached the client and | ||
| * is stored as assistant output, so replaying it verbatim re-primes the next turn with the very | ||
| * envelope the model is copying. | ||
| * | ||
| * Scope starts AT the marker line and runs to the next blank line, rather than to the end of | ||
| * the message. The echoed envelope has no terminator we can recognise — we build it as a marker | ||
| * line plus arbitrary result text (protobuf-request.ts), and the observed copies are not | ||
| * byte-exact, so matching against the replayed envelope is not available either. Truncating to | ||
| * the end of the message was the alternative, and it discards a genuine answer whenever the | ||
| * model resumes after the echo. A blank line is the one boundary the model reliably writes when | ||
| * it goes back to prose. | ||
| * | ||
| * The tradeoff is explicit: an echoed envelope whose pasted result itself contains a blank line | ||
| * leaves its remainder in replay. That is the safer direction to be wrong in — conversation | ||
| * remint, not this filter, is the primary defence against a poisoned conversation, and this only | ||
| * stops the transcript from feeding itself. | ||
| * | ||
| * Only whole-line markers count, so prose such as "the string [Tool Result] appeared" survives. | ||
| */ | ||
| export function stripAssistantEchoedToolEnvelope(text: string): string { | ||
| if (!text || !ECHO_MARKERS.some(marker => text.includes(marker))) return text; | ||
| const newline = text.includes("\r\n") ? "\r\n" : "\n"; | ||
| const lines = text.split(/\r?\n/); | ||
| const kept: string[] = []; | ||
| let dropped = false; | ||
| let index = 0; | ||
| while (index < lines.length) { | ||
| const line = lines[index] ?? ""; | ||
| if (!isEchoMarkerLine(line)) { | ||
| kept.push(line); | ||
| index += 1; | ||
| continue; | ||
| } | ||
| dropped = true; | ||
| index += 1; | ||
| // The envelope body is the contiguous non-blank run after the marker. The blank line that | ||
| // ends it is left in place, so surviving prose on either side stays separated. | ||
| while (index < lines.length && (lines[index] ?? "").trim() !== "") index += 1; | ||
| } | ||
| if (!dropped) return text; | ||
| return kept.join(newline).trimEnd(); | ||
| } | ||
|
|
||
| const MAX_SNIFF_BYTES = 40; | ||
| /** Mid-stream observer: max leading whitespace on a line before matching disarms. */ | ||
| const MAX_MIDSTREAM_LINE_INDENT = 128; | ||
|
|
@@ -66,8 +118,9 @@ export interface MidstreamEchoFinding { | |
| * MIDDLE of an agent message — after legitimate leading text — one of them | ||
| * carrying a whitespace-spliced call-id ("fc_x mar-y" instead of "fc_x-y"). | ||
| * Deltas at that point have already reached the client, so this observer | ||
| * never throws and never withholds output: it records findings so the | ||
| * adapter can emit a structured diagnostic at turn end. Only fixed marker | ||
| * never throws and never withholds output. It records findings so the adapter | ||
| * can emit a structured diagnostic and remint the conversation for the next | ||
| * turn at turn end. Only fixed marker | ||
| * enums, numeric offsets, and corruption booleans are retained — never | ||
| * content bytes. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import { namespacedToolName } from "../../types"; | |
| import type { CursorRunRequest } from "./types"; | ||
| import { decodeCursorCallId } from "./call-id"; | ||
| import { cursorNeedsExternalToolContinuation, isCursorExternalWireModel } from "./discovery"; | ||
| import { stripAssistantEchoedToolEnvelope } from "./envelope-echo"; | ||
| import { normalizeCursorToolResultText } from "./tool-result-normalize"; | ||
| import { debugProviderDiagnostic } from "../../lib/debug"; | ||
| import { | ||
|
|
@@ -208,11 +209,13 @@ function assistantRootText( | |
| message: Extract<OcxMessage, { role: "assistant" }>, | ||
| includeThinking: boolean, | ||
| ): string { | ||
| if (typeof message.content === "string") return message.content; | ||
| return message.content | ||
| .map(part => (part.type === "text" ? part.text : includeThinking && part.type === "thinking" ? part.thinking : undefined)) | ||
| .filter((value): value is string => typeof value === "string" && value.length > 0) | ||
| .join("\n"); | ||
| const raw = typeof message.content === "string" | ||
| ? message.content | ||
| : message.content | ||
| .map(part => (part.type === "text" ? part.text : includeThinking && part.type === "thinking" ? part.thinking : undefined)) | ||
| .filter((value): value is string => typeof value === "string" && value.length > 0) | ||
| .join("\n"); | ||
| return stripAssistantEchoedToolEnvelope(raw); | ||
|
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.
If an assistant legitimately shows an envelope example—for example, a fenced block containing a line exactly equal to Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| // Cursor builds the actual model prompt from rootPromptMessagesJson (turns[] is UI/display metadata), | ||
|
|
||
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.
This changes the runtime behavior of the mapped
src/adapters/area but updates onlystructure/providers/cursor.md;structure/INDEX.mdalso maps this area to the runtime, byte-accounting, Responses transport, transport inventory, inbound compatibility, chat compatibility, and adapter-registry documents. Reconcile those mapped contracts in this change, or narrow the manifest mapping if they do not actually describe this area.AGENTS.md reference: src/AGENTS.md:L11-L11
Useful? React with 👍 / 👎.