From dde203c5c3e3f95177ba36fb38ed287c85753f22 Mon Sep 17 00:00:00 2001 From: luvs01 Date: Sat, 29 Aug 2026 10:53:31 +0900 Subject: [PATCH 1/3] fix(cursor): preserve reserved-prefix call ids --- src/adapters/cursor/call-id.ts | 10 +++++--- tests/cursor-call-id.test.ts | 47 ++++++++++++++++++++++++++-------- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/src/adapters/cursor/call-id.ts b/src/adapters/cursor/call-id.ts index a76707d00a..c74de39054 100644 --- a/src/adapters/cursor/call-id.ts +++ b/src/adapters/cursor/call-id.ts @@ -5,9 +5,10 @@ * literal newline ("call--\nfc__"). OpenCodex forwards ids * verbatim, so that newline leaked into Responses-visible `call_id` values, * where line-oriented clients (logging, splitting, validation) break. The codec - * encodes only ids containing CR/LF into a versioned single-line form and - * decodes both that form and legacy raw multi-line ids back to the exact - * upstream bytes before anything is serialized toward Cursor. + * encodes ids containing CR/LF into a versioned single-line form. It also + * escapes ids already in that form's reserved namespace so encoding remains + * injective. Both forms decode back to the exact upstream bytes before + * anything is serialized toward Cursor. */ const CALL_ID_PREFIX = "ocxc1_"; @@ -19,7 +20,7 @@ function needsEncoding(id: string): boolean { /** Encode a Cursor wire call id into a single-line Responses-safe id. */ export function encodeCursorCallId(id: string): string { - if (!needsEncoding(id)) return id; + if (!needsEncoding(id) && !id.startsWith(CALL_ID_PREFIX)) return id; return CALL_ID_PREFIX + Buffer.from(id, "utf8").toString("base64url"); } @@ -37,6 +38,7 @@ export function decodeCursorCallId(id: string): string { const decoded = Buffer.from(payload, "base64url").toString("utf8"); // Round-trip guard: only trust payloads our encoder could have produced. if (Buffer.from(decoded, "utf8").toString("base64url") !== payload) return id; + if (!needsEncoding(decoded) && !decoded.startsWith(CALL_ID_PREFIX)) return id; return decoded; } catch { return id; diff --git a/tests/cursor-call-id.test.ts b/tests/cursor-call-id.test.ts index def2ece14c..068f324359 100644 --- a/tests/cursor-call-id.test.ts +++ b/tests/cursor-call-id.test.ts @@ -17,6 +17,30 @@ describe("cursor call-id codec", () => { expect(decodeCursorCallId("call_abc123")).toBe("call_abc123"); }); + test("reserved-prefix ids are escaped and round-trip", () => { + for (const id of ["ocxc1_", "ocxc1_Y2FsbF8x", "ocxc1_!!not-base64url!!", "ocxc1_raw\nwire"]) { + const encoded = encodeCursorCallId(id); + expect(encoded).not.toBe(id); + expect(encoded.startsWith("ocxc1_")).toBe(true); + expect(encoded).not.toContain("\n"); + expect(encoded).not.toContain("\r"); + expect(decodeCursorCallId(encoded)).toBe(id); + } + }); + + test("reserved-prefix ids resembling legacy newline encodings stay opaque", () => { + const id = "ocxc1_YQpi"; + const encoded = encodeCursorCallId(id); + expect(encoded).not.toBe(id); + expect(decodeCursorCallId(encoded)).toBe(id); + }); + + test("legacy encoded line breaks remain decodable", () => { + expect(decodeCursorCallId("ocxc1_YQpi")).toBe("a\nb"); + expect(decodeCursorCallId("ocxc1_DQ")).toBe("\r"); + expect(decodeCursorCallId("ocxc1_DQo")).toBe("\r\n"); + }); + test("newline composite id round-trips through a single-line form", () => { const encoded = encodeCursorCallId(COMPOSITE); expect(encoded).not.toContain("\n"); @@ -34,15 +58,18 @@ describe("cursor call-id codec", () => { expect(decodeCursorCallId("ocxc1_!!not-base64url!!")).toBe("ocxc1_!!not-base64url!!"); }); - test("tool_call_start ids are single-line at the adapter boundary", () => { - const events = mapCursorServerMessage( - { type: "tool_call_start", id: COMPOSITE, name: "get_weather" }, - mapperState(), - ); - expect(events).toHaveLength(1); - const event = events[0]!; - if (event.type !== "tool_call_start") throw new Error("expected tool_call_start"); - expect(event.id).not.toContain("\n"); - expect(decodeCursorCallId(event.id)).toBe(COMPOSITE); + test("tool_call_start ids are reversible and single-line at the adapter boundary", () => { + for (const id of [COMPOSITE, "ocxc1_YQpi"]) { + const events = mapCursorServerMessage( + { type: "tool_call_start", id, name: "get_weather" }, + mapperState(), + ); + expect(events).toHaveLength(1); + const event = events[0]!; + if (event.type !== "tool_call_start") throw new Error("expected tool_call_start"); + expect(event.id).not.toContain("\n"); + expect(event.id).not.toContain("\r"); + expect(decodeCursorCallId(event.id)).toBe(id); + } }); }); From 7135dcd08508ba5510c71d9457bf9e86e3655546 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 29 Aug 2026 11:32:57 +0900 Subject: [PATCH 2/3] test(cursor): prove reserved call-id nesting --- tests/cursor-call-id.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/cursor-call-id.test.ts b/tests/cursor-call-id.test.ts index 068f324359..c4a9a129ab 100644 --- a/tests/cursor-call-id.test.ts +++ b/tests/cursor-call-id.test.ts @@ -33,12 +33,27 @@ describe("cursor call-id codec", () => { const encoded = encodeCursorCallId(id); expect(encoded).not.toBe(id); expect(decodeCursorCallId(encoded)).toBe(id); + expect(decodeCursorCallId("ocxc1_Y2FsbF8x")).toBe("ocxc1_Y2FsbF8x"); + }); + + test("adversarial reserved-prefix ids escape one layer at a time", () => { + const cases = [ + ["ocxc1_Y2FsbF8x", "ocxc1_b2N4YzFfWTJGc2JGOHg"], + ["ocxc1_Y2FsbF8xCg", "ocxc1_b2N4YzFfWTJGc2JGOHhDZw"], + ["ocxc1_b2N4YzFfWTJGc2JGOHhDZw", "ocxc1_b2N4YzFfYjJONFl6RmZXVEpHYzJKR09IaERadw"], + ] as const; + + for (const [id, encoded] of cases) { + expect(encodeCursorCallId(id)).toBe(encoded); + expect(decodeCursorCallId(encoded)).toBe(id); + } }); test("legacy encoded line breaks remain decodable", () => { expect(decodeCursorCallId("ocxc1_YQpi")).toBe("a\nb"); expect(decodeCursorCallId("ocxc1_DQ")).toBe("\r"); expect(decodeCursorCallId("ocxc1_DQo")).toBe("\r\n"); + expect(decodeCursorCallId("ocxc1_Y2FsbF8xCg")).toBe("call_1\n"); }); test("newline composite id round-trips through a single-line form", () => { From 74a8ccba2f3725c64cd5551fcb154d44527b1923 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 29 Aug 2026 11:55:46 +0900 Subject: [PATCH 3/3] fix(cursor): give the reserved-id escape its own namespace CodeRabbit found a real regression in the escape added here: with one shared prefix the decoder had to guess whether a payload was our own output or an upstream id that merely looked like it, and it guessed wrong. `ocxc1_b2N4YzFf` is a legal opaque Cursor id whose base64url payload decodes to the literal text `ocxc1_`. The decoder unwrapped it and produced a bare `ocxc1_`, so a different id reached Cursor through request-builder and protobuf-request - breaking call/result pairing for any pre-change call or replayed history. The parent codec preserved that id, so this was a regression, not an incomplete fix. Two prefixes remove the ambiguity. `ocxc1_` stays the encoding namespace and admits only CR/LF-bearing content, which is the codec's actual job. `ocxc1e_` is the escape namespace and admits only ids already sitting in a namespace this codec owns. Each decoder branch accepts exactly what its encoder produces, so no payload has to be classified by guesswork. Verified: 273 pass across the eight suites that touch call ids, tsc clean, and three mutations red - restoring the shared-prefix decode fails the new regression test specifically, and removing the escape namespace or the escape entirely fails six. --- src/adapters/cursor/call-id.ts | 33 ++++++++++++++++++++++++++++----- tests/cursor-call-id.test.ts | 32 ++++++++++++++++++++++++++++---- 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/src/adapters/cursor/call-id.ts b/src/adapters/cursor/call-id.ts index c74de39054..5c98ab31b8 100644 --- a/src/adapters/cursor/call-id.ts +++ b/src/adapters/cursor/call-id.ts @@ -9,19 +9,38 @@ * escapes ids already in that form's reserved namespace so encoding remains * injective. Both forms decode back to the exact upstream bytes before * anything is serialized toward Cursor. + * + * The escape uses its OWN prefix rather than reusing the encoding one. Sharing a + * prefix made the decoder guess: `ocxc1_b2N4YzFf` is a legal opaque upstream id + * whose payload happens to decode to the literal text `ocxc1_`, so a decoder that + * unwraps any payload beginning with the prefix turned that id into a bare + * `ocxc1_` and sent the wrong id to Cursor, breaking call/result pairing for a + * pre-change call or replayed history. Two prefixes remove the ambiguity: a + * payload under `ocxc1_` is only ever CR/LF-bearing wire content, and a payload + * under `ocxc1e_` is only ever an escaped reserved id. */ const CALL_ID_PREFIX = "ocxc1_"; +/** Escape namespace for ids that already sit in a reserved namespace. */ +const CALL_ID_ESCAPE_PREFIX = "ocxc1e_"; /** True when the id needs encoding to survive line-oriented consumers. */ function needsEncoding(id: string): boolean { return id.includes("\n") || id.includes("\r"); } +/** True when the id sits in a namespace this codec owns and must be escaped. */ +function isReserved(id: string): boolean { + return id.startsWith(CALL_ID_PREFIX) || id.startsWith(CALL_ID_ESCAPE_PREFIX); +} + /** Encode a Cursor wire call id into a single-line Responses-safe id. */ export function encodeCursorCallId(id: string): string { - if (!needsEncoding(id) && !id.startsWith(CALL_ID_PREFIX)) return id; - return CALL_ID_PREFIX + Buffer.from(id, "utf8").toString("base64url"); + // CR/LF content is the codec's actual job, so it wins the primary namespace. + if (needsEncoding(id)) return CALL_ID_PREFIX + Buffer.from(id, "utf8").toString("base64url"); + // A reserved id carries no newline; it only needs to stop looking like our output. + if (isReserved(id)) return CALL_ID_ESCAPE_PREFIX + Buffer.from(id, "utf8").toString("base64url"); + return id; } /** @@ -31,14 +50,18 @@ export function encodeCursorCallId(id: string): string { * through rather than corrupting pairing. */ export function decodeCursorCallId(id: string): string { - if (!id.startsWith(CALL_ID_PREFIX)) return id; - const payload = id.slice(CALL_ID_PREFIX.length); + const escaped = id.startsWith(CALL_ID_ESCAPE_PREFIX); + if (!escaped && !id.startsWith(CALL_ID_PREFIX)) return id; + const payload = id.slice((escaped ? CALL_ID_ESCAPE_PREFIX : CALL_ID_PREFIX).length); if (payload.length === 0) return id; try { const decoded = Buffer.from(payload, "base64url").toString("utf8"); // Round-trip guard: only trust payloads our encoder could have produced. if (Buffer.from(decoded, "utf8").toString("base64url") !== payload) return id; - if (!needsEncoding(decoded) && !decoded.startsWith(CALL_ID_PREFIX)) return id; + // Each namespace admits exactly what its encoder puts there. An `ocxc1_` payload + // that decodes to newline-free text is NOT our output — it is an opaque upstream + // id that merely looks like ours, and unwrapping it would change the id. + if (escaped ? !isReserved(decoded) : !needsEncoding(decoded)) return id; return decoded; } catch { return id; diff --git a/tests/cursor-call-id.test.ts b/tests/cursor-call-id.test.ts index c4a9a129ab..c0aee14136 100644 --- a/tests/cursor-call-id.test.ts +++ b/tests/cursor-call-id.test.ts @@ -21,13 +21,37 @@ describe("cursor call-id codec", () => { for (const id of ["ocxc1_", "ocxc1_Y2FsbF8x", "ocxc1_!!not-base64url!!", "ocxc1_raw\nwire"]) { const encoded = encodeCursorCallId(id); expect(encoded).not.toBe(id); - expect(encoded.startsWith("ocxc1_")).toBe(true); + // Newline-bearing ids take the encoding namespace; newline-free reserved ids take the + // escape namespace. Both are single-line and both reverse exactly. + expect(encoded.startsWith(id.includes("\n") ? "ocxc1_" : "ocxc1e_")).toBe(true); expect(encoded).not.toContain("\n"); expect(encoded).not.toContain("\r"); expect(decodeCursorCallId(encoded)).toBe(id); } }); + // CodeRabbit on PR #2868: with one shared prefix the decoder had to guess, and it guessed + // wrong here. `ocxc1_b2N4YzFf` is a legal opaque upstream id whose payload decodes to the + // literal text `ocxc1_`, so unwrapping it produced a bare `ocxc1_` and sent a DIFFERENT id + // to Cursor — breaking pairing for any pre-change call or replayed history. The parent codec + // preserved it; a fix that regresses it is not a fix. + test("an opaque id whose payload merely looks encoded is preserved", () => { + expect(decodeCursorCallId("ocxc1_b2N4YzFf")).toBe("ocxc1_b2N4YzFf"); + // And it still survives a full round trip, via the escape namespace. + const encoded = encodeCursorCallId("ocxc1_b2N4YzFf"); + expect(encoded.startsWith("ocxc1e_")).toBe(true); + expect(decodeCursorCallId(encoded)).toBe("ocxc1_b2N4YzFf"); + }); + + test("ids already in the escape namespace are themselves escaped", () => { + const id = "ocxc1e_YQpi"; + const encoded = encodeCursorCallId(id); + expect(encoded).not.toBe(id); + expect(decodeCursorCallId(encoded)).toBe(id); + // Untouched when it is not our output: the payload decodes to newline-free non-reserved text. + expect(decodeCursorCallId("ocxc1e_Y2FsbF8x")).toBe("ocxc1e_Y2FsbF8x"); + }); + test("reserved-prefix ids resembling legacy newline encodings stay opaque", () => { const id = "ocxc1_YQpi"; const encoded = encodeCursorCallId(id); @@ -38,9 +62,9 @@ describe("cursor call-id codec", () => { test("adversarial reserved-prefix ids escape one layer at a time", () => { const cases = [ - ["ocxc1_Y2FsbF8x", "ocxc1_b2N4YzFfWTJGc2JGOHg"], - ["ocxc1_Y2FsbF8xCg", "ocxc1_b2N4YzFfWTJGc2JGOHhDZw"], - ["ocxc1_b2N4YzFfWTJGc2JGOHhDZw", "ocxc1_b2N4YzFfYjJONFl6RmZXVEpHYzJKR09IaERadw"], + ["ocxc1_Y2FsbF8x", "ocxc1e_b2N4YzFfWTJGc2JGOHg"], + ["ocxc1_Y2FsbF8xCg", "ocxc1e_b2N4YzFfWTJGc2JGOHhDZw"], + ["ocxc1e_b2N4YzFfWTJGc2JGOHhDZw", "ocxc1e_b2N4YzFlX2IyTjRZekZmV1RKR2MySkdPSGhEWnc"], ] as const; for (const [id, encoded] of cases) {