mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-16 04:03:02 +03:00
fix(providers): emit Cursor kv_after_text before tool calls instead of truncating them (#10215)
This commit is contained in:
@@ -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)).
|
||||
@@ -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";
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user