## Red on main #2783 (batch-core) landed and put **5 failures** on `main`. Both causes are the bookkeeping half of correct changes, not defects in them. ## 1. The census baseline — 4 failures `census-baseline-corruption-guard` plus 3 `lifecycle-column-census` ratchet cases, all downstream of one thing: ``` lifecycle-column-census --strict: column-guard count ROSE packages/core/src/task-move-disposer.ts (DELIBERATE-LITERAL: in-progress): 0 -> 1 packages/core/src/task-move-disposer.ts (DELIBERATE-LITERAL: todo): 0 -> 1 packages/core/src/task-store/archive-lifecycle-2.ts (DELIBERATE-LITERAL: archived): 0 -> 1 packages/dashboard/src/github-tracking-comments.ts (DELIBERATE-LITERAL: done): 0 -> 1 packages/dashboard/src/gitlab-tracking-comments.ts (DELIBERATE-LITERAL: in-progress): 0 -> 1 packages/dashboard/src/server.ts (DELIBERATE-LITERAL: archived): 0 -> 1 ``` **The rise is legitimate.** #2783 *annotated* documented fast-path literals — e.g. `task-move-disposer.ts`'s *"a fast path, not the guard … the actual lane decision is the RESOLVED membership test inside this block"* — and the census tracks marked literals per file. Re-recorded with `--strict --update-baseline`; the same run also **tightened 20 entries whose counts dropped**, so this moves the ratchet down as well as up. ## 2. The disposal-order test — 1 failure ``` executor-user-cancel > re-dispatch (task:moved → in-progress) awaits prior disposal before execute() AssertionError: expected -1 to be greater than 2 ``` `-1` reads like the re-dispatch was **dropped**. It was not — that would be a real cancel-race bug, so I checked before touching the test: ``` PROBE_MICROTASK callOrder=["abort-started","abort-resolved","dispose","execute"] PROBE_AFTER_TIMER callOrder=["abort-started","abort-resolved","dispose","execute"] ``` Correct order, reached once drained, unchanged after a real 50ms timer. The test drained exactly **two** microtask turns and #2783's disposer refactor added await hops, so `execute` had not been recorded yet. A fixed turn count encodes today's await depth into the test: any added `await` on the product path fails it for a reason that has nothing to do with the invariant. It now waits on the **outcome** via `vi.waitFor`. The ordering assertion is untouched and is still the point. ## Evidence | mutation | result | |---|---| | `execute` never recorded (stands in for a dropped re-dispatch) | **fails** — `waitFor` times out | | `execute` observed *before* `dispose` | **fails** — `expected 'execute' to be 'dispose'` | | baseline: fresh `--strict` run | *"every file matches its baseline exactly"* | Engine **10985 passed / 0 failed** (was 5 failed) · gate **732 green** · lint clean. ## Method note, against myself I pre-flighted #2783 and **reported it clean** — but I ran only `@fusion/core` and the dashboard `api` group, because that is what the diff touches. The census and disposal tests live in `packages/engine`, which batch-core does not modify at all. **The suite that breaks is not always the suite the diff points at.** A cross-package ratchet like the census is exactly the case where scoping pre-flight to the changed packages produces a confident "clean" that is wrong. Pre-flight needs the engine suite regardless of which package a batch touches; I have adjusted accordingly for the remaining queue. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
265 lines
9.0 KiB
TypeScript
265 lines
9.0 KiB
TypeScript
import { describe, it, expect, vi } from "vitest";
|
|
import "./executor-test-helpers.js";
|
|
import { getTaskMoveDisposer } from "@fusion/core";
|
|
import { TaskExecutor } from "../executor.js";
|
|
import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js";
|
|
|
|
describe("TaskExecutor user cancel handling", () => {
|
|
/*
|
|
FNXC:WorkflowLifecycle 2026-07-18-14:32:
|
|
A user move from active execution to Todo must await every executor
|
|
cancellation surface so the card cannot keep processing in the background.
|
|
*/
|
|
it("registers an awaited user-move disposer that aborts all active work before Todo", async () => {
|
|
resetExecutorMocks();
|
|
const store = createMockStore();
|
|
const executor = new TaskExecutor(store as any, "/tmp/test");
|
|
const terminateChildren = vi.spyOn(executor as any, "terminateAllChildren").mockResolvedValue(undefined);
|
|
let resolveAbort: (() => void) | undefined;
|
|
const abortPending = new Promise<void>((resolve) => {
|
|
resolveAbort = resolve;
|
|
});
|
|
const session = {
|
|
prompt: vi.fn(),
|
|
abort: vi.fn(() => abortPending),
|
|
dispose: vi.fn(),
|
|
} as any;
|
|
(executor as any).activeSessions.set("FN-AWAITED", {
|
|
session,
|
|
seenSteeringIds: new Set<string>(),
|
|
});
|
|
|
|
const disposer = getTaskMoveDisposer(store as any);
|
|
expect(disposer).toBeTypeOf("function");
|
|
let disposed = false;
|
|
const disposal = disposer!({ id: "FN-AWAITED" } as any).then(() => {
|
|
disposed = true;
|
|
});
|
|
|
|
await Promise.resolve();
|
|
expect(terminateChildren).toHaveBeenCalledWith("FN-AWAITED");
|
|
expect(session.abort).toHaveBeenCalledOnce();
|
|
expect(disposed).toBe(false);
|
|
|
|
resolveAbort?.();
|
|
await disposal;
|
|
expect(session.dispose).toHaveBeenCalledOnce();
|
|
expect((executor as any).userCanceledTaskIds.has("FN-AWAITED")).toBe(true);
|
|
});
|
|
|
|
it("does not let late cancellation cleanup touch a replacement execution", async () => {
|
|
resetExecutorMocks();
|
|
let resolveChildStateUpdate: (() => void) | undefined;
|
|
const childStateUpdate = new Promise<void>((resolve) => {
|
|
resolveChildStateUpdate = resolve;
|
|
});
|
|
const agentStore = {
|
|
updateAgentState: vi.fn(() => childStateUpdate),
|
|
deleteAgent: vi.fn().mockResolvedValue(undefined),
|
|
};
|
|
const store = createMockStore();
|
|
const executor = new TaskExecutor(store as any, "/tmp/test", { agentStore } as any);
|
|
const oldSession = {
|
|
prompt: vi.fn(),
|
|
abort: vi.fn().mockResolvedValue(undefined),
|
|
dispose: vi.fn(),
|
|
} as any;
|
|
const oldChildSession = { dispose: vi.fn() } as any;
|
|
(executor as any).activeSessions.set("FN-GENERATION", {
|
|
session: oldSession,
|
|
seenSteeringIds: new Set<string>(),
|
|
});
|
|
(executor as any).spawnedAgents.set("FN-GENERATION", new Set(["old-child"]));
|
|
(executor as any).childSessions.set("old-child", oldChildSession);
|
|
|
|
const disposer = getTaskMoveDisposer(store as any)!;
|
|
const disposal = disposer({ id: "FN-GENERATION" } as any);
|
|
|
|
const replacementSession = { dispose: vi.fn() } as any;
|
|
const replacementChildren = new Set(["new-child"]);
|
|
(executor as any).activeSessions.set("FN-GENERATION", {
|
|
session: replacementSession,
|
|
seenSteeringIds: new Set<string>(),
|
|
});
|
|
(executor as any).spawnedAgents.set("FN-GENERATION", replacementChildren);
|
|
|
|
resolveChildStateUpdate?.();
|
|
await disposal;
|
|
|
|
expect(oldSession.abort).toHaveBeenCalledOnce();
|
|
expect(oldChildSession.dispose).toHaveBeenCalledOnce();
|
|
expect((executor as any).activeSessions.get("FN-GENERATION")?.session).toBe(replacementSession);
|
|
expect((executor as any).spawnedAgents.get("FN-GENERATION")).toBe(replacementChildren);
|
|
});
|
|
|
|
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");
|
|
/*
|
|
FNXC:EngineTests 2026-07-31-05:40:
|
|
WAIT FOR THE OUTCOME, do not count turns.
|
|
|
|
This drained exactly two microtask turns and then asserted the order. #2783's task-move-disposer
|
|
refactor added await hops to the re-dispatch path, so `execute` had not been recorded yet and
|
|
`indexOf` returned -1 — reported as "expected -1 to be greater than 2", which reads like the
|
|
re-dispatch was DROPPED rather than merely later. It was not: measured, the order is still
|
|
["abort-started","abort-resolved","dispose","execute"], reached well within the same tick budget
|
|
once drained properly, and unchanged after a real timer.
|
|
|
|
A fixed turn count encodes today's await depth into the test, so any added await on the product
|
|
path fails it for a reason that has nothing to do with the invariant. `vi.waitFor` on the actual
|
|
outcome is depth-independent. The ORDER assertion below is untouched and is still the point: if
|
|
the re-dispatch genuinely stopped happening, waitFor times out and this fails.
|
|
*/
|
|
await vi.waitFor(() => {
|
|
expect(callOrder).toContain("execute");
|
|
});
|
|
|
|
expect(callOrder.indexOf("execute")).toBeGreaterThan(callOrder.indexOf("dispose"));
|
|
executeSpy.mockRestore();
|
|
});
|
|
});
|