From a3ea6c65cadae0c405eac0140ec0caeeb452d521 Mon Sep 17 00:00:00 2001
From: BenStullsBets <ben@benbetson.us>
Date: Fri, 26 Jun 2026 04:49:02 -0700
Subject: [PATCH] fix(#60): pre-aborted signal short-circuits before run; guard
 onProgress relay
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review findings (Medium+Low): a signal already aborted before runEditTurn is
called could not be honored by agent.abort() (the SDK AbortController isn't
created until run()), so the turn ran to completion — now it throws before
run() (INV-47). And a throwing onProgress relay is swallowed so observability
can never fail the turn (INV-44). Strengthened the unit + E2E to prove both.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/liveTurn.ts                     | 19 +++++++++++++++----
 test/e2e/suite/liveProgress.test.ts |  7 ++++---
 test/liveTurn.test.ts               | 21 +++++++++++----------
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/src/liveTurn.ts b/src/liveTurn.ts
index 0965cae..3edac9e 100644
--- a/src/liveTurn.ts
+++ b/src/liveTurn.ts
@@ -73,16 +73,27 @@ export async function runEditTurn(
     ? agent.subscribe((event) => {
         const next = reduceTurnProgress(state, event);
         state = next.state;
-        if (next.snapshot) opts.onProgress!(next.snapshot);
+        // Observability must never affect the result (INV-44): a throwing relay
+        // is swallowed, not allowed to propagate into the SDK and fail the turn.
+        if (next.snapshot) {
+          try {
+            opts.onProgress!(next.snapshot);
+          } catch {
+            /* progress is best-effort */
+          }
+        }
       })
     : undefined;
   const onAbort = () => agent.abort();
   opts?.signal?.addEventListener("abort", onAbort);
-  // Handle a signal that was already aborted before the turn started (the
-  // "abort" event won't re-fire for an already-aborted signal).
-  if (opts?.signal?.aborted) onAbort();
 
   try {
+    // A signal already aborted before the turn starts can't be honored by
+    // agent.abort() (the SDK's AbortController isn't created until run()), so
+    // short-circuit to the same aborted outcome the call site reflects (INV-47).
+    if (opts?.signal?.aborted) {
+      throw new Error("claude-code turn aborted: cancelled before start");
+    }
     const result = await agent.run(
       `<instruction>\n${instruction}\n</instruction>\n<text>\n${selectedText}\n</text>`,
     );
diff --git a/test/e2e/suite/liveProgress.test.ts b/test/e2e/suite/liveProgress.test.ts
index 8e339bd..a10bbe7 100644
--- a/test/e2e/suite/liveProgress.test.ts
+++ b/test/e2e/suite/liveProgress.test.ts
@@ -60,11 +60,12 @@ suite("#60 live turn progress (additive + cancel)", () => {
     await vscode.commands.executeCommand("cowriting.showTrackChangesPreview");
     await settle();
 
-    // Simulate the SDK honoring the abort signal: throw as runEditTurn does on a
-    // non-"completed" (aborted) status.
+    // The stub throws ONLY when the aborted signal reached it — so if opts.signal
+    // failed to thread through runEditAndPropose, the stub would instead return a
+    // rewrite and create a proposal, failing this test. That proves propagation.
     ctl.setEditTurnForTest(async (_i, _text, opts) => {
       if (opts?.signal?.aborted) throw new Error("claude-code turn aborted");
-      throw new Error("claude-code turn aborted");
+      return { replacement: "# Title\n\nSHOULD NOT BE PROPOSED.\n", model: "sonnet", sessionId: "e2e-live60-nope" };
     });
 
     const ac = new AbortController();
diff --git a/test/liveTurn.test.ts b/test/liveTurn.test.ts
index 805b2ed..32f022d 100644
--- a/test/liveTurn.test.ts
+++ b/test/liveTurn.test.ts
@@ -3,11 +3,11 @@ import { extractReplacement, runEditTurn } from "../src/liveTurn";
 
 // A fake Agent that replays a scripted event list to subscribers, supports abort,
 // and resolves agent.run() with a completed (or aborted) result.
-function fakeSdk(events: any[], opts?: { abortAware?: boolean }) {
+const runs = { count: 0 };
+function fakeSdk(events: any[]) {
   return {
     Agent: class {
       private listeners: ((e: any) => void)[] = [];
-      private aborted = false;
       constructor(_cfg: unknown) {}
       subscribe(fn: (e: any) => void) {
         this.listeners.push(fn);
@@ -15,14 +15,13 @@ function fakeSdk(events: any[], opts?: { abortAware?: boolean }) {
           this.listeners = this.listeners.filter((l) => l !== fn);
         };
       }
-      abort() {
-        this.aborted = true;
-      }
+      // The real SDK's abort() is a no-op before run() creates its AbortController,
+      // so this fake does NOT cooperate with a pre-abort — proving runEditTurn's own
+      // short-circuit, not the fake's leniency.
+      abort() {}
       async run(_input: string) {
+        runs.count += 1;
         for (const e of events) for (const l of this.listeners) l(e);
-        if (opts?.abortAware && this.aborted) {
-          return { status: "aborted", outputText: "", runId: "r1", error: { message: "aborted" } };
-        }
         return { status: "completed", outputText: "EDITED", runId: "r1" };
       }
     },
@@ -70,12 +69,14 @@ describe("runEditTurn progress + cancel", () => {
     vi.resetModules();
   });
 
-  it("a fired AbortSignal aborts the agent and the turn throws (INV-47)", async () => {
+  it("a pre-aborted AbortSignal short-circuits before run() and throws (INV-47)", async () => {
     vi.resetModules();
-    vi.doMock("@cline/sdk", () => fakeSdk([], { abortAware: true }));
+    vi.doMock("@cline/sdk", () => fakeSdk([]));
+    runs.count = 0;
     const ac = new AbortController();
     ac.abort();
     await expect(runEditTurn("do it", "old", { signal: ac.signal })).rejects.toThrow(/aborted/);
+    expect(runs.count).toBe(0); // the turn never ran — no proposal could be produced
     vi.doUnmock("@cline/sdk");
     vi.resetModules();
   });