Close the last fire-and-forget gap from the previous fixes: the task:moved (away from in-progress) and task:deleted listeners no longer call the synchronous fire-and-forget `abortInFlightTaskWork`. Instead they track an awaited disposal promise per task in `pendingTaskDisposals`. The task:moved (to in-progress) dispatch path awaits any in-flight disposal for the same task before calling `execute()`, so a fast bounce (in-progress → todo → in-progress) no longer races the conflict-cleanup path against a still-live shell. `awaitAbortInFlightTaskWork` now claims each session surface (activeSessions, activeStepExecutors, activeWorkflowStepSessions, activeSubagentSessions) synchronously before awaiting any async abort. This lets concurrent disposal calls for the same task dedupe naturally — the second call finds the maps empty and no-ops, preserving the existing single-abort/single-dispose contract that the soft-delete and user-cancel tests assert. Adds a regression test in executor-user-cancel covering the re-dispatch ordering: an immediate task:moved-to-in-progress that follows a still-running task:moved-away must wait for abort to complete before execute() runs. The legacy `abortInFlightTaskWork` is removed (no callers). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
160 lines
4.6 KiB
TypeScript
160 lines
4.6 KiB
TypeScript
import { describe, it, expect, vi } from "vitest";
|
|
import "./executor-test-helpers.js";
|
|
import { TaskExecutor } from "../executor.js";
|
|
import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js";
|
|
|
|
describe("TaskExecutor user cancel handling", () => {
|
|
it("aborts before dispose when user moves in-progress task back to todo", async () => {
|
|
resetExecutorMocks();
|
|
const store = createMockStore();
|
|
const executor = new TaskExecutor(store as any, "/tmp/test");
|
|
|
|
const callOrder: string[] = [];
|
|
const session = {
|
|
prompt: vi.fn(),
|
|
abort: vi.fn(() => {
|
|
callOrder.push("abort");
|
|
return Promise.resolve();
|
|
}),
|
|
dispose: vi.fn(() => {
|
|
callOrder.push("dispose");
|
|
}),
|
|
} as any;
|
|
|
|
(executor as any).activeSessions.set("FN-001", {
|
|
session,
|
|
seenSteeringIds: new Set<string>(),
|
|
});
|
|
|
|
(store as any)._trigger("task:moved", {
|
|
task: {
|
|
id: "FN-001",
|
|
column: "todo",
|
|
dependencies: [],
|
|
steps: [],
|
|
currentStep: 0,
|
|
log: [],
|
|
},
|
|
from: "in-progress",
|
|
to: "todo",
|
|
source: "user",
|
|
});
|
|
|
|
await (executor as any).pendingTaskDisposals.get("FN-001");
|
|
|
|
expect(callOrder[0]).toBe("abort");
|
|
expect(callOrder[1]).toBe("dispose");
|
|
expect((executor as any).activeSessions.has("FN-001")).toBe(false);
|
|
expect((executor as any).userCanceledTaskIds.has("FN-001")).toBe(true);
|
|
expect(store.moveTask).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("does not mark engine-initiated move as user cancel", () => {
|
|
resetExecutorMocks();
|
|
const store = createMockStore();
|
|
const executor = new TaskExecutor(store as any, "/tmp/test");
|
|
|
|
(store as any)._trigger("task:moved", {
|
|
task: {
|
|
id: "FN-002",
|
|
column: "todo",
|
|
dependencies: [],
|
|
steps: [],
|
|
currentStep: 0,
|
|
log: [],
|
|
},
|
|
from: "in-progress",
|
|
to: "todo",
|
|
source: "engine",
|
|
});
|
|
|
|
expect((executor as any).userCanceledTaskIds.has("FN-002")).toBe(false);
|
|
});
|
|
|
|
it("clears userCanceled marker when task is moved back to in-progress", () => {
|
|
resetExecutorMocks();
|
|
const store = createMockStore();
|
|
const executor = new TaskExecutor(store as any, "/tmp/test");
|
|
|
|
(executor as any).userCanceledTaskIds.add("FN-003");
|
|
|
|
(store as any)._trigger("task:moved", {
|
|
task: {
|
|
id: "FN-003",
|
|
column: "in-progress",
|
|
dependencies: [],
|
|
steps: [],
|
|
currentStep: 0,
|
|
log: [],
|
|
},
|
|
from: "todo",
|
|
to: "in-progress",
|
|
source: "user",
|
|
});
|
|
|
|
expect((executor as any).userCanceledTaskIds.has("FN-003")).toBe(false);
|
|
});
|
|
|
|
it("re-dispatch (task:moved → in-progress) awaits prior disposal before execute()", async () => {
|
|
resetExecutorMocks();
|
|
const store = createMockStore();
|
|
const executor = new TaskExecutor(store as any, "/tmp/test");
|
|
|
|
const callOrder: string[] = [];
|
|
let resolveAbort: (() => void) | null = null;
|
|
const abortPromise = new Promise<void>((resolve) => {
|
|
resolveAbort = () => {
|
|
callOrder.push("abort-resolved");
|
|
resolve();
|
|
};
|
|
});
|
|
|
|
const session = {
|
|
prompt: vi.fn(),
|
|
abort: vi.fn(() => {
|
|
callOrder.push("abort-started");
|
|
return abortPromise;
|
|
}),
|
|
dispose: vi.fn(() => {
|
|
callOrder.push("dispose");
|
|
}),
|
|
} as any;
|
|
|
|
(executor as any).activeSessions.set("FN-RACE", {
|
|
session,
|
|
seenSteeringIds: new Set<string>(),
|
|
});
|
|
const executeSpy = vi.spyOn(executor, "execute" as any).mockImplementation(async () => {
|
|
callOrder.push("execute");
|
|
});
|
|
|
|
// Move away first — kicks off async disposal.
|
|
(store as any)._trigger("task:moved", {
|
|
task: { id: "FN-RACE", column: "todo", dependencies: [], steps: [], currentStep: 0, log: [] },
|
|
from: "in-progress",
|
|
to: "todo",
|
|
source: "user",
|
|
});
|
|
// Immediate re-dispatch — must wait for the disposal above.
|
|
(store as any)._trigger("task:moved", {
|
|
task: { id: "FN-RACE", column: "in-progress", dependencies: [], steps: [], currentStep: 0, log: [] },
|
|
from: "todo",
|
|
to: "in-progress",
|
|
source: "user",
|
|
});
|
|
|
|
// execute() must not run yet — abort is still pending.
|
|
await Promise.resolve();
|
|
expect(callOrder).toEqual(["abort-started"]);
|
|
|
|
// Resolve abort. Dispose + execute should follow in order.
|
|
resolveAbort!();
|
|
await (executor as any).pendingTaskDisposals.get("FN-RACE");
|
|
await Promise.resolve();
|
|
await Promise.resolve();
|
|
|
|
expect(callOrder.indexOf("execute")).toBeGreaterThan(callOrder.indexOf("dispose"));
|
|
executeSpy.mockRestore();
|
|
});
|
|
});
|