fix(#60): pre-aborted signal short-circuits before run; guard onProgress relay

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>
This commit is contained in:
BenStullsBets
2026-06-26 04:49:02 -07:00
parent 21bbf6b114
commit a3ea6c65ca
3 changed files with 30 additions and 17 deletions
+15 -4
View File
@@ -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>`,
);
+4 -3
View File
@@ -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();
+11 -10
View File
@@ -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();
});