fix(FN-5256): await disposal across task:moved-away and task:deleted
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>
This commit is contained in:
@@ -66,7 +66,7 @@ describe("TaskExecutor soft-delete aborts", () => {
|
||||
});
|
||||
|
||||
emit("task:deleted", makeTask("FN-TEST-1"));
|
||||
await Promise.resolve();
|
||||
await (executor as any).pendingTaskDisposals.get("FN-TEST-1");
|
||||
|
||||
expect(abort).toHaveBeenCalledTimes(1);
|
||||
expect(dispose).toHaveBeenCalledTimes(1);
|
||||
@@ -88,7 +88,7 @@ describe("TaskExecutor soft-delete aborts", () => {
|
||||
});
|
||||
|
||||
emit("task:deleted", makeTask("FN-TEST-2"));
|
||||
await Promise.resolve();
|
||||
await (executor as any).pendingTaskDisposals.get("FN-TEST-2");
|
||||
|
||||
expect(abortAllSessionBash).toHaveBeenCalledTimes(1);
|
||||
expect(terminateAllSessions).toHaveBeenCalledTimes(1);
|
||||
@@ -104,7 +104,7 @@ describe("TaskExecutor soft-delete aborts", () => {
|
||||
(executor as any).activeWorkflowStepSessions.set("FN-TEST-3", { abort, dispose });
|
||||
|
||||
emit("task:deleted", makeTask("FN-TEST-3"));
|
||||
await Promise.resolve();
|
||||
await (executor as any).pendingTaskDisposals.get("FN-TEST-3");
|
||||
|
||||
expect(abort).toHaveBeenCalledTimes(1);
|
||||
expect(dispose).toHaveBeenCalledTimes(1);
|
||||
|
||||
@@ -40,7 +40,7 @@ describe("TaskExecutor user cancel handling", () => {
|
||||
source: "user",
|
||||
});
|
||||
|
||||
await Promise.resolve();
|
||||
await (executor as any).pendingTaskDisposals.get("FN-001");
|
||||
|
||||
expect(callOrder[0]).toBe("abort");
|
||||
expect(callOrder[1]).toBe("dispose");
|
||||
@@ -94,4 +94,66 @@ describe("TaskExecutor user cancel handling", () => {
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -194,7 +194,7 @@ describe("reliability interactions: soft-delete in-flight abort", () => {
|
||||
});
|
||||
|
||||
await store.deleteTask("FN-EXEC");
|
||||
await Promise.resolve();
|
||||
await (executor as any).pendingTaskDisposals.get("FN-EXEC");
|
||||
|
||||
expect(abort).toHaveBeenCalledTimes(1);
|
||||
expect(dispose).toHaveBeenCalledTimes(1);
|
||||
@@ -241,7 +241,7 @@ describe("reliability interactions: soft-delete in-flight abort", () => {
|
||||
|
||||
await store.moveTask("FN-RACE", "todo");
|
||||
await store.deleteTask("FN-RACE");
|
||||
await Promise.resolve();
|
||||
await (executor as any).pendingTaskDisposals.get("FN-RACE");
|
||||
|
||||
expect(abort).toHaveBeenCalledTimes(1);
|
||||
expect(dispose).toHaveBeenCalledTimes(1);
|
||||
|
||||
Reference in New Issue
Block a user