From f45a90009ab9f4fcf92a72673c2d7cf14710db63 Mon Sep 17 00:00:00 2001 From: adevwithpurpose Date: Sat, 15 Aug 2026 19:11:53 -0300 Subject: [PATCH] fix(providers): emit Cursor kv_after_text before tool calls instead of truncating them (#10215) --- .../10215-cursor-kv-after-text-toolcalls.md | 1 + open-sse/executors/cursor.ts | 25 +++++-- tests/unit/cursor-streaming.test.ts | 71 ++++++++++++++++++- 3 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 changelog.d/fixes/10215-cursor-kv-after-text-toolcalls.md diff --git a/changelog.d/fixes/10215-cursor-kv-after-text-toolcalls.md b/changelog.d/fixes/10215-cursor-kv-after-text-toolcalls.md new file mode 100644 index 0000000000..db0ea1df9a --- /dev/null +++ b/changelog.d/fixes/10215-cursor-kv-after-text-toolcalls.md @@ -0,0 +1 @@ +- **fix(cursor):** Stop truncating pending tool calls on non-composer models when a KV checkpoint arrives after text but before the `exec_mcp` frame — the KV short-circuit is now gated to the composer family where it was verified ([#10215](https://github.com/diegosouzapw/OmniRoute/issues/10215)). \ No newline at end of file diff --git a/open-sse/executors/cursor.ts b/open-sse/executors/cursor.ts index 42c7bf0055..89f7799b03 100644 --- a/open-sse/executors/cursor.ts +++ b/open-sse/executors/cursor.ts @@ -681,13 +681,26 @@ export function processFrame( // after text means the model finished and the server is saving the // turn. Phase 8 keeps both signals as defense-in-depth. // - // Safe vs tool calls: when the model invokes a tool, the exec_mcp event - // always arrives at or before this kv checkpoint (verified across many - // live composer-2.5 trials — a tool call never follows kv_after_text), so - // endReason is already "tool_calls" by the time we get here. Ending on - // kv_after_text therefore never truncates a pending tool call. + // Safe vs tool calls (composer family only): when the model invokes a + // tool, the exec_mcp event always arrives at or before this kv + // checkpoint (verified across many live composer-2.5 trials — a tool call + // never follows kv_after_text), so endReason is already "tool_calls" by + // the time we get here. Ending on kv_after_text therefore never truncates + // a pending tool call on composer. + // + // Non-composer models (cursor/grok-4.5-high, auto, ...) emit the KV + // checkpoint as a blob-store side-channel frame (envelope field 4, + // kv_get_blob/kv_set_blob) with NO turn-completion semantics, and it can + // arrive while the model is still streaming a long preamble BEFORE a + // pending exec_mcp. Ending the turn there drops that exec_mcp, leaving a + // narration-only finish_reason "stop" with zero tool_calls (#10215). On + // this family only the real terminal signals (turn_ended, + // tool_call_completed, server_end) decide — kvAfterTextSeen is kept purely + // as an observational flag, never as the turn terminator. ctx.kvAfterTextSeen = true; - ctx.endReason = "kv_after_text"; + if (isComposerModel(ctx.model)) { + ctx.endReason = "kv_after_text"; + } } } } diff --git a/tests/unit/cursor-streaming.test.ts b/tests/unit/cursor-streaming.test.ts index 068c723001..0eb26e684f 100644 --- a/tests/unit/cursor-streaming.test.ts +++ b/tests/unit/cursor-streaming.test.ts @@ -57,6 +57,23 @@ function buildKvServerMessagePayload(): Buffer { return lenPrefixed(4, Buffer.alloc(0)); } +// AgentServerMessage { exec_server_message (2): { id (1): 9, mcp_args (11): { tool_name (5): str } } } +function buildExecMcpPayload(): Buffer { + const mcpArgs = lenPrefixed(5, Buffer.from("magic_tool")); + const esm = Buffer.concat([tag(1, 0), v(9), lenPrefixed(11, mcpArgs)]); + return lenPrefixed(2, esm); +} + +// Faithful model of driveH2's per-frame endReason teardown (cursor.ts): after +// each decoded frame a truthy endReason detaches listeners and stops reading, +// so any frame still buffered after it is dropped. +function driveFrames(ctx: StreamCtx, frames: Buffer[]): void { + for (const f of frames) { + processFrame(f, ctx, new Set()); + if (ctx.endReason) return; + } +} + // JSON error payload (Connect-RPC error envelope) function buildJsonErrorPayload(): Buffer { return Buffer.from( @@ -117,14 +134,64 @@ test("processFrame accumulates token_delta", () => { assert.equal(ctx.tokenDelta, 55); }); -test("processFrame sets endReason on kv_server_message after text", () => { - const ctx = newStreamCtx("auto", () => {}); +test("processFrame sets endReason on kv_server_message after text for composer models", () => { + // Composer family keeps the plain-chat short-circuit: KV is the verified + // early end-of-turn signal and a tool call never follows kv_after_text. + const ctx = newStreamCtx("cursor/composer-2.5", () => {}); processFrame(buildTextDeltaPayload("hi"), ctx, new Set()); processFrame(buildKvServerMessagePayload(), ctx, new Set()); assert.equal(ctx.endReason, "kv_after_text"); assert.equal(ctx.kvAfterTextSeen, true); }); +test("processFrame does not end turn on kv_server_message for non-composer models", () => { + // Non-composer models (cursor/grok-4.5-high, auto) emit the KV checkpoint as + // a blob-store side-channel frame with no turn-completion semantics — it can + // arrive mid-stream before a pending exec_mcp. It must never terminate here; + // only the real terminal signals (turn_ended / tool_call_completed) decide. + for (const model of ["cursor/grok-4.5-high", "auto"]) { + const ctx = newStreamCtx(model, () => {}); + processFrame(buildTextDeltaPayload("hi"), ctx, new Set()); + processFrame(buildKvServerMessagePayload(), ctx, new Set()); + assert.equal(ctx.endReason, null, `model ${model} must not end on kv_after_text`); + assert.equal(ctx.kvAfterTextSeen, true, `model ${model} still observes the KV checkpoint`); + } +}); + +test("REGRESSION #10215: non-composer kv_after_text before exec_mcp must not drop the tool call", () => { + // text → kv_server_message → exec_mcp must still process the tool call: + // the KV checkpoint (with no turn semantics on this family) must not tear the + // frame loop down before the pending exec_mcp is decoded. Prior to the fix + // this left ctx.toolCalls=0 → finish_reason "stop" (narration-only truncation). + for (const model of ["cursor/grok-4.5-high", "auto"]) { + const ctx = newStreamCtx(model, () => {}); + driveFrames(ctx, [ + buildTextDeltaPayload("a long preamble before the tool call"), + buildKvServerMessagePayload(), + buildExecMcpPayload(), + ]); + assert.equal(ctx.toolCalls.length, 1, `model ${model} must keep the pending tool call`); + assert.equal(ctx.endReason, "tool_calls", `model ${model} ends on the real tool signal`); + assert.equal(ctx.kvAfterTextSeen, true); + } +}); + +test("REGRESSION #10215: long preamble (>2.5K chars) then KV then exec_mcp keeps the tool call", () => { + // Covers the at-risk band the reporter identified (2505-2933 chars of text + // before the tool call on cursor/grok-4.5-high). A KV checkpoint arriving + // mid-preamble must not truncate the still-pending exec_mcp. + const longPreamble = + "The model streams a lengthy preamble before invoking a tool. ".repeat(60); + assert.ok(longPreamble.length > 2500); + for (const model of ["cursor/grok-4.5-high", "auto"]) { + const ctx = newStreamCtx(model, () => {}); + driveFrames(ctx, [buildTextDeltaPayload(longPreamble), buildKvServerMessagePayload(), buildExecMcpPayload()]); + assert.equal(ctx.toolCalls.length, 1, `model ${model} must keep the tool call`); + assert.equal(ctx.endReason, "tool_calls"); + assert.ok(ctx.totalText.length > 2500); + } +}); + test("buildCursorUsage degrades to prompt-only counts for an empty response", () => { // emitUsage now always emits on the success path (OpenAI streaming contract), // relying on buildCursorUsage producing a valid usage object even when the