diff --git a/packages/engine/src/__tests__/ce-workflow-step-executor.test.ts b/packages/engine/src/__tests__/ce-workflow-step-executor.test.ts index 7c2d012ef8..07cef58f7d 100644 --- a/packages/engine/src/__tests__/ce-workflow-step-executor.test.ts +++ b/packages/engine/src/__tests__/ce-workflow-step-executor.test.ts @@ -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, ); diff --git a/packages/engine/src/__tests__/executor-step-session.test.ts b/packages/engine/src/__tests__/executor-step-session.test.ts index 4b9241bfdf..45fc39ccf5 100644 --- a/packages/engine/src/__tests__/executor-step-session.test.ts +++ b/packages/engine/src/__tests__/executor-step-session.test.ts @@ -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; + }; + let bouncePromise: Promise | 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; - }).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((resolve) => queueMicrotask(resolve)); - await new Promise((resolve) => queueMicrotask(resolve)); - await new Promise((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(); diff --git a/packages/engine/src/__tests__/invariant-wrong-checkout-completion.test.ts b/packages/engine/src/__tests__/invariant-wrong-checkout-completion.test.ts index 80545edd8c..7b4d48027d 100644 --- a/packages/engine/src/__tests__/invariant-wrong-checkout-completion.test.ts +++ b/packages/engine/src/__tests__/invariant-wrong-checkout-completion.test.ts @@ -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 () => { diff --git a/packages/engine/src/__tests__/reliability-interactions/executor-liveness-gate.test.ts b/packages/engine/src/__tests__/reliability-interactions/executor-liveness-gate.test.ts index 9d7cffc33c..ff54c535f7 100644 --- a/packages/engine/src/__tests__/reliability-interactions/executor-liveness-gate.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/executor-liveness-gate.test.ts @@ -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); });