fix(cursor): do not re-label a completed turn as failed when the stream is aborted

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`.

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.

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);
     }
   }

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", () => {