test(engine): fix executor step-session/liveness-gate/checkout/ce-workflow mocks (FN-7229 retry-cap + workflow verdict wiring)
This commit is contained in:
@@ -462,9 +462,16 @@ describe("CE workflow-step executor integration", () => {
|
||||
value: "implementation-incomplete",
|
||||
}));
|
||||
expect(mergeRequester).not.toHaveBeenCalled();
|
||||
// FNXC:WorkflowMerge 2026-07-07-08:38: The merge boundary (executor.ts:6305, 6fc50d8d9e) now moves the task to in-review and logs the boundary move BEFORE the implementation-proof gate runs, then the proof failure is logged separately. The proof-failure text (executor.ts:6345) changed from the static "implementation steps are incomplete" to the parse-step-aware "implementation did not run: parsed coding steps are missing or incomplete". Assert both log entries so the new two-stage merge-boundary behavior is pinned.
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-CE-1",
|
||||
"Workflow merge blocked before requester: implementation steps are incomplete",
|
||||
"Workflow merge boundary moved task to in-review before requesting merge",
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-CE-1",
|
||||
"Workflow merge blocked before requester: implementation did not run: parsed coding steps are missing or incomplete",
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
|
||||
@@ -187,7 +187,7 @@ describe("Workflow Steps Execution", () => {
|
||||
expect(onComplete).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("moves task to in-review once fn_task_done requeue budget is exhausted", async () => {
|
||||
it("marks task failed in-place once fn_task_done requeue budget is exhausted (FN-7229)", async () => {
|
||||
const store = createMockStore();
|
||||
store.getTask.mockResolvedValue({
|
||||
id: "FN-001",
|
||||
@@ -237,7 +237,8 @@ describe("Workflow Steps Execution", () => {
|
||||
status: "failed",
|
||||
error: "Agent finished without calling fn_task_done (after 3 retries)",
|
||||
});
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-review");
|
||||
// FNXC:ExecutorMoveTask 2026-07-07-08:38: FN-7229 (984e36255d) stopped parking execution errors in review — an exhausted fn_task_done budget now marks the task failed in-place (executor.ts:11179) instead of moveTask→in-review. `in-review` is reserved for clean completion handoffs, so the task must NOT be moved there. (Line 236 already asserts status=failed.)
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "in-review");
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "todo");
|
||||
expect(onError).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: "FN-001" }),
|
||||
@@ -557,18 +558,20 @@ describe("Workflow Steps Execution", () => {
|
||||
// non-deterministic; calling performWorkflowRerunBounce directly is
|
||||
// exactly what the timer would have done after the next event-loop
|
||||
// tick and removes the timing dependency entirely.
|
||||
// Cast once to a named handle: these are private executor methods the
|
||||
// compiler cannot see; assign to a typed const rather than inlining the
|
||||
// cast into each member access.
|
||||
const executorInternals = executor as unknown as {
|
||||
scheduleWorkflowRerun: (taskId: string, worktreePath: string, successMessage: string) => void;
|
||||
performWorkflowRerunBounce: (taskId: string, worktreePath: string) => Promise<unknown>;
|
||||
};
|
||||
let bouncePromise: Promise<unknown> | undefined;
|
||||
const scheduleSpy = vi
|
||||
.spyOn(executor as unknown as {
|
||||
scheduleWorkflowRerun: (
|
||||
taskId: string,
|
||||
worktreePath: string,
|
||||
successMessage: string,
|
||||
) => void;
|
||||
}, "scheduleWorkflowRerun")
|
||||
.spyOn(executorInternals, "scheduleWorkflowRerun")
|
||||
.mockImplementation((taskId, worktreePath) => {
|
||||
void (executor as unknown as {
|
||||
performWorkflowRerunBounce: (taskId: string, worktreePath: string) => Promise<unknown>;
|
||||
}).performWorkflowRerunBounce(taskId, worktreePath);
|
||||
// Capture the bounce promise so the test can await it to completion
|
||||
// (see FNXC below) instead of flushing a fixed microtask count.
|
||||
bouncePromise = executorInternals.performWorkflowRerunBounce(taskId, worktreePath);
|
||||
});
|
||||
|
||||
const stepName = "Frontend UX Design";
|
||||
@@ -604,15 +607,11 @@ describe("Workflow Steps Execution", () => {
|
||||
.map((call: any[]) => call[1]);
|
||||
expect(reopenedStepIndexes).toEqual([0, 1]);
|
||||
|
||||
// performWorkflowRerunBounce was invoked synchronously by the spy
|
||||
// above; flush microtasks so its awaited store calls settle before
|
||||
// we assert.
|
||||
await new Promise<void>((resolve) => queueMicrotask(resolve));
|
||||
await new Promise<void>((resolve) => queueMicrotask(resolve));
|
||||
await new Promise<void>((resolve) => queueMicrotask(resolve));
|
||||
// FNXC:ExecutorMoveTask 2026-07-07-08:38: Await the captured rerun-bounce promise instead of flushing a fixed number of microtasks. 3167dbc83 inserted clearTerminalStepFailuresForRetry (an extra awaited hop) between the todo and in-progress moves inside performWorkflowRerunBounce, so a fixed microtask count no longer deterministically drains the bounce to the final in-progress moveTask. Awaiting the promise is exact and survives future awaited hops; the bounce still performs the todo→in-progress hop (executor.ts:3650 then 3674).
|
||||
await bouncePromise;
|
||||
|
||||
// (2) bounce uses preserveResumeState so step progress + worktree survive
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo", { preserveResumeState: true, preserveWorktree: true });
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo", expect.objectContaining({ preserveResumeState: true, preserveWorktree: true }));
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-progress");
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "in-review");
|
||||
expect(onError).not.toHaveBeenCalled();
|
||||
|
||||
@@ -114,7 +114,12 @@ describe("FN-4115 wrong-checkout completion rejection", () => {
|
||||
const result = await tool.execute("id", {});
|
||||
expect(result.content[0].text).toContain("Task marked complete");
|
||||
expect(store.updateStep).toHaveBeenCalled();
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-4115", "todo", { preserveProgress: true });
|
||||
// FNXC:ExecutorMoveTask 2026-07-07-08:38: A valid fn_task_done completion is distinguished from a wrong-checkout REFUSAL by its success log, not by the absence of a todo moveTask. setup() runs execute() with a mocked agent that never calls fn_task_done, so the FN-4806 silent worktree-reclaim path (executor.ts:11149, 3f8a5e6839) legitimately requeues to todo with { preserveProgress: true } — the same signature the refusal path (handleImplicitTaskDoneRefusal, executor.ts:13030) emits — so a moveTask-shape assertion cannot tell a valid completion from a refusal. Pin the positive success marker instead: a valid completion logs "Task marked done by agent" (executor.ts:13306), which the refusal test at line 82 proves a wrong-checkout rejection never emits. (Filter on id+message so the runContext arg / arity don't make this brittle.)
|
||||
expect(
|
||||
store.logEntry.mock.calls.some(
|
||||
([id, msg]) => id === "FN-4115" && typeof msg === "string" && msg === "Task marked done by agent",
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("FN-4115: pre-session liveness rejects missing worktree before createFnAgent", async () => {
|
||||
|
||||
@@ -163,7 +163,7 @@ describe("reliability interactions: FN-4935 executor liveness gate", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("parks in-review at retry cap", async () => {
|
||||
it("fails in-place at retry cap (FN-7229)", async () => {
|
||||
vi.spyOn(worktreeAcquisition, "acquireTaskWorktree").mockResolvedValue({
|
||||
worktreePath: "/repo/.worktrees/new-path",
|
||||
branch: "fusion/fn-4935-t",
|
||||
@@ -180,7 +180,9 @@ describe("reliability interactions: FN-4935 executor liveness gate", () => {
|
||||
const executor = new TaskExecutor(store as any, "/repo");
|
||||
await executor.execute(makeTask({ taskDoneRetryCount: 999, sessionFile: null }));
|
||||
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-4935-T", "in-review");
|
||||
// FNXC:ExecutorMoveTask 2026-07-07-08:38: FN-7229 (984e36255d) stopped parking worktree-liveness failures in review — at the retry cap the task is now marked failed in-place via updateTask(status=failed) (executor.ts:9608) instead of moveTask→in-review. `in-review` is reserved for clean completion handoffs, so assert the task is NOT moved there and IS marked failed. The worktree:incomplete-detected audit event below still carries the forensic `terminalAction: "park-in-review"` label (executor.ts:9554), which records what the gate detected, not the (changed) terminal action.
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-4935-T", "in-review");
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-4935-T", expect.objectContaining({ status: "failed", error: expect.any(String) }));
|
||||
expect(events.some((event) => (event.type === "worktree:incomplete-detected" || event.mutationType === "worktree:incomplete-detected") && event.metadata?.terminalAction === "park-in-review")).toBe(true);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user