From 60526d7aff32ab54958042ee814e982e69a20105 Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 19 Aug 2026 21:53:22 +0900 Subject: [PATCH 1/2] fix(cursor): do not re-label a completed turn as failed when the stream is aborted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only cancelCursorRun() sets expectedClose, so an ordinary completed turn never qualified for the benign-close path. The abort listener then failed the turn with 'Cursor request was aborted' — which is deliberately NOT a benign cancel, since a mid-turn abort is a real failure — so a turn whose terminal frame had already been emitted and whose messages had already been yielded still surfaced as turn-failed with expectedClose:false. Return instead of throwing when a terminal frame was already emitted AND the failure is an abort. Both halves matter: post-terminal alone would change what the adapter sees for genuine faults, and abort alone would swallow a real mid-turn abort where nothing was delivered. Deliberately narrow. A benign cancel after a terminal is already swallowed one layer up (cursor.ts:183), and the existing contract test that pins 'the transport still throws the raw cancel after a terminal' keeps passing — this does not widen that path. Refs #1527. This is the teardown-misclassification slice only; the kimi-k3 collapse and the 429 asymmetry are separate and need live acceptance work that cannot start until #2054 lands. --- src/adapters/cursor/cursor-errors.ts | 15 ++++++++++++++ src/adapters/cursor/live-transport.ts | 15 +++++++++++++- tests/cursor-cancel-provenance.test.ts | 27 +++++++++++++++++++++++++- 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/adapters/cursor/cursor-errors.ts b/src/adapters/cursor/cursor-errors.ts index 294adc4a7c..f2e13c578c 100644 --- a/src/adapters/cursor/cursor-errors.ts +++ b/src/adapters/cursor/cursor-errors.ts @@ -85,6 +85,21 @@ export function isCursorBenignCancelError(value: unknown): boolean { return false; } +/** + * True when the turn was torn down by an `AbortSignal` rather than by a transport fault. + * + * This is deliberately NOT part of `isCursorBenignCancelError`: an abort mid-turn is a real + * failure and must still surface. It is only meaningful in combination with a terminal frame + * having already been emitted, where it means "the answer landed and then the connection went + * away" (#1527). + */ +export function isCursorAbortError(value: unknown): boolean { + const message = errorMessage(value).toLowerCase(); + if (message.includes("cursor request was aborted")) return true; + const name = (value as { name?: unknown })?.name; + return typeof name === "string" && name === "AbortError"; +} + /** * True when Cursor Connect rejected the turn with invalid_argument. * Seen after stepCompleted on brittle external-model continuations. diff --git a/src/adapters/cursor/live-transport.ts b/src/adapters/cursor/live-transport.ts index 3f8ce957a9..5775de025e 100644 --- a/src/adapters/cursor/live-transport.ts +++ b/src/adapters/cursor/live-transport.ts @@ -48,7 +48,7 @@ import { type InteractionResponse, } from "./gen/agent_pb"; import { debugProviderDiagnostic } from "../../lib/debug"; -import { classifyCursorError, CursorUnexpectedCancelError, isCursorBenignCancelError, safeCursorErrorMessage } from "./cursor-errors"; +import { classifyCursorError, CursorUnexpectedCancelError, isCursorAbortError, isCursorBenignCancelError, safeCursorErrorMessage } from "./cursor-errors"; import { mcpArgsFromToolCall } from "./protobuf-events"; import { OCX_RESPONSES_TOOL_PROVIDER } from "./tool-definitions"; import { @@ -650,6 +650,18 @@ class LiveCursorTransport implements CursorTransport { // A CANCEL is benign only on the client-tool suspend path (expectedClose); an // unexpected server-side NGHTTP2_CANCEL must surface as a real transport error. if (this.expectedClose && isCursorBenignCancelError(failure)) return; + // A teardown error arriving AFTER the turn's terminal frame describes the connection, + // not the turn: the answer is committed and every queued message has been yielded. + // + // Narrow on purpose. A benign cancel after a terminal is already swallowed one layer + // up (`cursor.ts:183`), so widening this to every post-terminal error would change + // what the adapter sees for genuine faults. What it does cover is the abort case + // from #1527: `signal.abort` fires `failAndClear(new Error("Cursor request was + // aborted"))`, which is NOT benign (`cursor-errors.ts:74`), so an ordinary completed + // turn that is then torn down still surfaced as `turn-failed` with + // `expectedClose:false`. Only `cancelCursorRun()` sets `expectedClose`, so a normal + // completion never qualified for the branch above. + if (this.emittedTerminal && isCursorAbortError(failure)) return; throw attachPartialUsage(classifyTurnFailure(failure), state); } if (done) break; @@ -659,6 +671,7 @@ class LiveCursorTransport implements CursorTransport { } if (failure) { if (this.expectedClose && isCursorBenignCancelError(failure)) return; + if (this.emittedTerminal && isCursorAbortError(failure)) return; throw attachPartialUsage(classifyTurnFailure(failure), state); } } diff --git a/tests/cursor-cancel-provenance.test.ts b/tests/cursor-cancel-provenance.test.ts index 548bbd3f05..c954f64184 100644 --- a/tests/cursor-cancel-provenance.test.ts +++ b/tests/cursor-cancel-provenance.test.ts @@ -33,6 +33,7 @@ function cancelError(): Error { async function runCancelTurn(opts: { emitTerminalFirst?: boolean; suspendFirst?: boolean; + failWith?: Error; }): Promise<{ messages: CursorServerMessage[]; failure?: Error }> { resetCursorBlobStateForTests(); const transport = createLiveCursorTransport({ @@ -73,7 +74,7 @@ async function runCancelTurn(opts: { if (opts.emitTerminalFirst) pushEvent({ type: "done", usage: { inputTokens: 0, outputTokens: 0, totalTokens: 0 } }); // The client-tool suspend path cancels our own stream, which sets expectedClose. if (opts.suspendFirst) (transport as unknown as { cancelCursorRun(): void }).cancelCursorRun(); - failTurn(cancelError()); + failTurn(opts.failWith ?? cancelError()); await drain; transport.close?.(); return { messages, failure }; @@ -112,6 +113,30 @@ describe("Cursor cancel provenance", () => { expect(failure).not.toBeInstanceOf(CursorUnexpectedCancelError); expect(isCursorBenignCancelError(failure)).toBe(true); }); + + test("an abort after a terminal frame does not re-label a completed turn as failed (#1527)", async () => { + // Only cancelCursorRun() sets expectedClose, so an ordinary completed turn never + // qualified for the benign path. The abort listener then failed the turn with + // "Cursor request was aborted" — which is deliberately NOT a benign cancel — so a turn + // whose answer had already been delivered still surfaced as turn-failed with + // expectedClose:false in the request log. + const { messages, failure } = await runCancelTurn({ + emitTerminalFirst: true, + failWith: new Error("Cursor request was aborted"), + }); + + expect(messages.some(m => m.type === "done")).toBe(true); + expect(failure).toBeUndefined(); + }); + + test("an abort BEFORE any terminal frame still fails the turn (#1527 guard)", async () => { + // The narrowing must not swallow a genuine mid-turn abort: nothing was delivered, so + // the caller has to hear about it. + const { failure } = await runCancelTurn({ failWith: new Error("Cursor request was aborted") }); + + expect(failure).toBeDefined(); + expect(failure?.message).toContain("aborted"); + }); }); describe("isCursorBenignCancelError provenance", () => { From b5712840282bd01fd7ebc87220dd273f2688656a Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 19 Aug 2026 21:54:40 +0900 Subject: [PATCH 2/2] docs(devlog): record the 1527 abort slice and the wrong-site edit the ablation caught --- .../090_1527_abort_slice.md | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 devlog/_plan/260819_unclaimed_bug_selection/090_1527_abort_slice.md diff --git a/devlog/_plan/260819_unclaimed_bug_selection/090_1527_abort_slice.md b/devlog/_plan/260819_unclaimed_bug_selection/090_1527_abort_slice.md new file mode 100644 index 0000000000..11f5760661 --- /dev/null +++ b/devlog/_plan/260819_unclaimed_bug_selection/090_1527_abort_slice.md @@ -0,0 +1,67 @@ +# 090 — #1527 abort-teardown slice + +Branch: `fix/cursor-abort-teardown` off `fix/tray-registry-encoding`. +Commit: `346eaa80d`. PR: **#2118** → #2117 → #2116 → `dev`. + +## The plan pointed at the wrong line, and the ablation is what caught it + +`000` said the fix site was the abort listener calling +`failAndClear("Cursor request was aborted")` at `live-transport.ts:1157`. That +reads correctly and is wrong. + +Patching `failAndClear` left **all ten existing tests passing**. That is the +signal: a change to the actual failure path could not have been invisible. The +injected failure never reaches that helper — the `open()` seam's `fail` callback +at `:627` writes a local `failure` variable, and the throw happens later inside +`run()` at `:651` and `:661`. + +Without the ablation this would have shipped as a green no-op. It is the same +lesson the campaign already recorded twice, arriving a third time in a new +costume: **a passing suite after a change proves nothing until you have seen +that suite fail.** + +## The second correction: unconditional was too wide + +First working version returned on `emittedTerminal` alone. That broke an +existing contract test: + +> "a cancel after a terminal was already emitted does not add a second one" — +> asserts the transport **still throws** the raw cancel. + +That test is deliberate. The adapter's benign check (`cursor.ts:183`) swallows a +raw cancel one layer up, so the transport throwing it is how provenance stays +intact without a second terminal reaching the bridge. + +Narrowed to `emittedTerminal && isCursorAbortError(failure)`. Both halves carry +weight: + +| Condition | Without it | +|---|---| +| `emittedTerminal` | a mid-turn abort would be swallowed — nothing delivered, caller never told | +| `isCursorAbortError` | every post-terminal fault would change shape for the adapter | + +## Why an abort was not already covered + +`isCursorBenignCancelError` deliberately excludes aborts — a mid-turn abort *is* +a real failure. And `expectedClose` is set only by `cancelCursorRun()`, so an +ordinary completion never qualified. A completed turn torn down afterwards fell +through both guards and was reported `turn-failed` with `expectedClose: false`. + +## Verification + +``` +bun test cursor-cancel-provenance + cursor-eof-terminal + + cursor-adapter + cursor-errors 46 pass / 0 fail +bun x tsc --noEmit exit 0 +``` + +Ablation: removing both guards gives `7 pass / 1 fail` — exactly the new +post-terminal-abort test, nothing else. + +## Scope + +This is one of five residual parts of #1527 after #2054. The other four — +`kimi-k3` collapse at 79-95k, the `claude-fable-5` 429 asymmetry, full-replay on +first turn/restart/compaction, and request-shape parity — are acceptance work +that cannot start until #2054 lands, and the 429 half may be unprovable while +Connect hides `cache_read_tokens`. Hence `Refs`, not `Closes`.