From 4178e7d32559a7e69964037b840e50a641a02896 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 2 Jul 2026 05:01:35 -0700 Subject: [PATCH] FN-7415: Delete stale executor pause quarantine Delete the expired executor pause quarantine and its stale direct-dispatch coverage. - Remove the obsolete executor-pause test suite after the graph runtime cutover. - Clear the matching Vitest exclude and quarantine-ledger entry. - Refresh test audit, timing, line-count, and planning references for the deleted suite. Files changed: ...7-001-refactor-workflow-runtime-cutover-plan.md | 2 +- docs/test-value-audit.json | 102 - .../engine/src/__tests__/executor-pause.test.ts | 3061 -------------------- packages/engine/vitest.config.ts | 5 - scripts/__tests__/test-velocity-baseline.test.mjs | 2 +- scripts/lib/test-quarantine.json | 8 +- scripts/line-count-baseline.json | 1 - scripts/test-timings.json | 1 - 8 files changed, 3 insertions(+), 3179 deletions(-) Fusion-Task-Id: FN-7415 Fusion-Task-Lineage: 3d1551c8-1353-47cb-a70f-d88e733a6652 Co-authored-by: Fusion (runfusion.ai) --- ...-refactor-workflow-runtime-cutover-plan.md | 2 +- docs/test-value-audit.json | 102 - .../src/__tests__/executor-pause.test.ts | 3061 ----------------- packages/engine/vitest.config.ts | 5 - .../__tests__/test-velocity-baseline.test.mjs | 2 +- scripts/lib/test-quarantine.json | 8 +- scripts/line-count-baseline.json | 1 - scripts/test-timings.json | 1 - 8 files changed, 3 insertions(+), 3179 deletions(-) delete mode 100644 packages/engine/src/__tests__/executor-pause.test.ts diff --git a/docs/plans/2026-06-07-001-refactor-workflow-runtime-cutover-plan.md b/docs/plans/2026-06-07-001-refactor-workflow-runtime-cutover-plan.md index cdd1321460..cd12aa8aa9 100644 --- a/docs/plans/2026-06-07-001-refactor-workflow-runtime-cutover-plan.md +++ b/docs/plans/2026-06-07-001-refactor-workflow-runtime-cutover-plan.md @@ -167,7 +167,7 @@ The runtime should have one mental model: resolve a workflow, execute graph node - **Goal:** Route scheduler, task-moved, heartbeat resume, pause/unpause, and self-healing triggers through the workflow runtime. - **Requirements:** R1, R3, R7, R9. - **Dependencies:** U5. -- **Files:** `packages/engine/src/scheduler.ts`, `packages/engine/src/executor.ts`, `packages/engine/src/project-engine.ts`, `packages/engine/src/self-healing-manager.ts` or adjacent recovery modules, `packages/engine/src/__tests__/executor-abort-all-in-flight.test.ts`, `packages/engine/src/__tests__/executor-pause.test.ts`, `packages/engine/src/__tests__/reliability-interactions/engine-stop-aborts-execution.test.ts`, `packages/engine/src/__tests__/reliability-interactions/in-review-automerge-off.test.ts`, `packages/engine/src/__tests__/reliability-interactions/workflow-interpreter-dual-observe.test.ts`. +- **Files:** `packages/engine/src/scheduler.ts`, `packages/engine/src/executor.ts`, `packages/engine/src/project-engine.ts`, `packages/engine/src/self-healing-manager.ts` or adjacent recovery modules, `packages/engine/src/__tests__/executor-abort-all-in-flight.test.ts`, `packages/engine/src/__tests__/reliability-interactions/engine-stop-aborts-execution.test.ts`, `packages/engine/src/__tests__/reliability-interactions/in-review-automerge-off.test.ts`, `packages/engine/src/__tests__/reliability-interactions/workflow-interpreter-dual-observe.test.ts`. - **Approach:** Make runtime run claims the authority for "currently executing". Existing `executingTaskLock`, active session registries, configured command controllers, and heartbeat gates should move behind runtime primitives or runtime ownership checks. Recovery sweeps should inspect workflow-run state before mutating active tasks. - **Execution note:** Characterization-first for cancel/pause and auto-merge-off interactions. - **Patterns to follow:** `executingTaskLock`, `maybeExecuteWorkflowGraph` duplicate guard, `ProjectEngine.requestInterpreterMerge`, reliability-interactions suites. diff --git a/docs/test-value-audit.json b/docs/test-value-audit.json index 618bfabaee..972cc9f22c 100644 --- a/docs/test-value-audit.json +++ b/docs/test-value-audit.json @@ -16822,108 +16822,6 @@ } ] }, - { - "file": "packages/engine/src/__tests__/executor-pause.test.ts", - "package": "packages/engine", - "valueScore": -1, - "positiveCount": 8, - "negativeCount": 6, - "neutralCount": 1, - "timesTouched": 15, - "testOnlyCount": 5, - "testOnlyRatio": 0.333, - "signalTally": { - "added": 1, - "test-only-churn": 5, - "churn-keyword": 5, - "test+source": 6, - "fix+source": 3 - }, - "durationMs": 300, - "testCount": 86, - "quarantined": false, - "recommendation": "delete", - "safeDelete": false, - "deletionPriority": 315, - "ageDays": 49, - "lastTouched": "2026-06-25", - "evidence": [ - { - "sha": "f240683fc", - "time": 1778221265000, - "subject": "feat(FN-3720): split executor.test.ts into eight focused suites and add she", - "status": "A", - "touchedSource": false, - "category": "positive", - "weight": 1, - "signals": [ - "added" - ] - }, - { - "sha": "234c2a594", - "time": 1778481370000, - "subject": "feat(FN-3968): add tests for task env inheritance and isolation in spawn/ch", - "status": "M", - "touchedSource": false, - "category": "negative", - "weight": -1, - "signals": [ - "test-only-churn" - ] - }, - { - "sha": "7298c9607", - "time": 1778597230000, - "subject": "feat(FN-4087): replace sleep() with deterministic waits in tests", - "status": "M", - "touchedSource": false, - "category": "negative", - "weight": -4, - "signals": [ - "churn-keyword", - "test-only-churn" - ] - }, - { - "sha": "4e6440a12", - "time": 1778735018000, - "subject": "feat(FN-4389): complete Step 8 — run verification and fix regressions", - "status": "M", - "touchedSource": true, - "category": "positive", - "weight": 1.5, - "signals": [ - "test+source" - ] - }, - { - "sha": "af1660881", - "time": 1779232524000, - "subject": "feat(FN-5168): complete Step 2 — executor feeds ignored step-update churn", - "status": "M", - "touchedSource": true, - "category": "positive", - "weight": 1.5, - "signals": [ - "test+source" - ] - }, - { - "sha": "cf0101be7", - "time": 1779379583000, - "subject": "fix(FN-5256): keep live task worktrees through pause/resume races", - "status": "M", - "touchedSource": true, - "category": "neutral", - "weight": 0, - "signals": [ - "churn-keyword", - "fix+source" - ] - } - ] - }, { "file": "packages/engine/src/__tests__/reliability-interactions/todo-inprogress-flapping.test.ts", "package": "packages/engine", diff --git a/packages/engine/src/__tests__/executor-pause.test.ts b/packages/engine/src/__tests__/executor-pause.test.ts deleted file mode 100644 index ad5587ff8d..0000000000 --- a/packages/engine/src/__tests__/executor-pause.test.ts +++ /dev/null @@ -1,3061 +0,0 @@ -// -nocheck -/* eslint-disable -eslint/no-unused-vars */ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import "./executor-test-helpers.js"; -import { AgentSemaphore } from "../concurrency.js"; -import { detectReviewHandoffIntent, determineRevisionResetStart } from "../executor.js"; -import { TaskExecutor, buildExecutionPrompt } from "../executor.js"; -import { createFnAgent } from "../pi.js"; -import { reviewStep as mockedReviewStepFn } from "../reviewer.js"; -import { execSync } from "node:child_process"; -import { findWorktreeUser, aiMergeTask } from "../merger.js"; -import { WorktreePool } from "../worktree-pool.js"; -import { generateWorktreeName, slugify } from "../worktree-names.js"; -import type { Task, TaskDetail } from "@fusion/core"; -import { SessionManager } from "@earendil-works/pi-coding-agent"; -import { StepSessionExecutor } from "../step-session-executor.js"; -import { activeSessionRegistry } from "../active-session-registry.js"; -import { executorLog } from "../logger.js"; -import { withRateLimitRetry } from "../rate-limit-retry.js"; -import { runVerificationCommand as mockedRunVerificationCommand } from "../verification-utils.js"; -import { - createMockStore, - mockedCreateFnAgent, - mockedSessionManager, - mockedGenerateWorktreeName, - mockedFindWorktreeUser, - mockedStepSessionExecutor, - mockedWithRateLimitRetry, - mockedExecSync, - mockedExistsSync, - mockExecuteAll, - mockTerminateAllSessions, - mockCleanup, - resetExecutorMocks, -} from "./executor-test-helpers.js"; - -const mockedReviewStep = vi.mocked(mockedReviewStepFn); - -const WAIT_FOR_ASYNC_OPTIONS = { timeout: 2000, interval: 5 }; - -async function waitForAsyncExpectation(assertion: () => void | Promise) { - await vi.waitFor(assertion, WAIT_FOR_ASYNC_OPTIONS); -} - -async function waitForStepExecutorRegistration(executor: TaskExecutor, taskId: string) { - await waitForAsyncExpectation(() => { - expect((executor as any).activeStepExecutors.has(taskId)).toBe(true); - }); -} - -afterEach(() => { - vi.useRealTimers(); -}); - -describe("TaskExecutor context limit error recovery", () => { - beforeEach(() => { - resetExecutorMocks(); - }); - - function createMockSessionForContextRecovery() { - return { - prompt: vi.fn(async () => {}), - dispose: vi.fn(), - subscribe: vi.fn(), - setThinkingLevel: vi.fn(), - steer: vi.fn(async () => {}), - sessionFile: "/tmp/test-session.json", - model: { provider: "mock", id: "mock-model", name: "Mock" }, - sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, - state: {}, - }; - } - - it("does NOT mark task as failed when context limit error is detected and recovery succeeds", async () => { - const mockSession = createMockSessionForContextRecovery(); - - // Mock compactSessionContext to succeed - const { compactSessionContext } = await import("../pi.js"); - vi.mocked(compactSessionContext).mockResolvedValueOnce({ - summary: "Compacted conversation", - tokensBefore: 150000, - }); - - const store = createMockStore(); - (store.getSettings as any).mockResolvedValue({ - maxConcurrent: 2, - maxWorktrees: 4, - pollIntervalMs: 15000, - groupOverlappingFiles: false, - autoMerge: false, - }); - - const executor = new TaskExecutor(store, "/tmp/test"); - - // Directly inject an active session - (executor as any).activeSessions.set("FN-001", { - session: mockSession, - seenSteeringIds: new Set(), - }); - - // Simulate the catch block being invoked with a context limit error - // This would normally happen when prompt() throws - const contextError = new Error("invalid params, context window exceeds limit (2013)"); - - // The executor should catch this error and attempt recovery - // We can't directly test the catch block, but we can test that isContextLimitError - // now correctly identifies this error - const { isContextLimitError } = await import("../context-limit-detector.js"); - expect(isContextLimitError(contextError.message)).toBe(true); - }); - - it("recognizes 'context window exceeds limit' as context limit error", async () => { - const { isContextLimitError } = await import("../context-limit-detector.js"); - - // These are the specific error formats that should be recognized - expect(isContextLimitError("invalid params, context window exceeds limit (2013)")).toBe(true); - expect(isContextLimitError("context window exceeds limit")).toBe(true); - expect(isContextLimitError("context window exceeds limit (2003)")).toBe(true); - expect(isContextLimitError("Context Window Exceeds limit")).toBe(true); - }); - - it("does NOT recognize generic 'limit exceeded' without context keywords", async () => { - const { isContextLimitError } = await import("../context-limit-detector.js"); - - // These should NOT be recognized as context limit errors - expect(isContextLimitError("limit exceeded")).toBe(false); - expect(isContextLimitError("quota exceeded")).toBe(false); - expect(isContextLimitError("rate limit exceeded")).toBe(false); - }); - - it("reduced-prompt retry is attempted when compact returns null", async () => { - // This test verifies the recovery flow when compactSessionContext returns null - // (no history to compact) - the code should fall through to reduced-prompt retry - const { isContextLimitError } = await import("../context-limit-detector.js"); - - // These error formats should trigger reduced-prompt recovery - const contextError = "context window exceeds limit (2013)"; - expect(isContextLimitError(contextError)).toBe(true); - // The test passes if isContextLimitError returns true, which means - // the reduced-prompt retry path would be triggered - }); - - it("reduced-prompt retry prompt focuses on completing efficiently", async () => { - // Verify the reduced prompt template includes key instructions - const reducedPrompt = [ - "Your previous attempt hit the context window limit.", - "Focus on completing the task efficiently with minimal context:", - "1. Review git status and git log to see what's been done", - "2. Identify the most critical remaining work", - "3. Complete it with a simpler, more focused approach", - "", - "Do not repeat what's already been done. Just complete the task and call fn_task_done.", - ].join("\n"); - - expect(reducedPrompt).toContain("context window limit"); - expect(reducedPrompt).toContain("git status"); - expect(reducedPrompt).toContain("fn_task_done"); - expect(reducedPrompt).toContain("Do not repeat what's already been done"); - }); -}); - -// ── Agent Spawning Tests ───────────────────────────────────────────────── - -function createMockAgentStore() { - let nextId = 1; - const agents = new Map(); - - return { - createAgent: vi.fn(async (input: any) => { - const agentId = `agent-${String(nextId++).padStart(8, "0")}`; - const agent = { - id: agentId, - name: input.name, - role: input.role, - state: "idle" as string, - reportsTo: input.reportsTo, - metadata: input.metadata ?? {}, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - agents.set(agentId, agent); - return agent; - }), - updateAgentState: vi.fn(async (agentId: string, newState: string) => { - const agent = agents.get(agentId); - if (agent) { - agent.state = newState; - agent.updatedAt = new Date().toISOString(); - } - return agent; - }), - _agents: agents, - }; -} - -async function captureToolsWithAgentStore(agentStore?: any, settingsOverride?: any): Promise<{ - tools: Record Promise>; - store: ReturnType; - executor: TaskExecutor; -}> { - const store = createMockStore(); - store.updateStep.mockResolvedValue({ - steps: [ - { name: "Preflight", status: "done" }, - { name: "Implement", status: "in-progress" }, - { name: "Testing", status: "pending" }, - ], - }); - const mergedSettings = { - maxConcurrent: 2, - maxWorktrees: 4, - pollIntervalMs: 15000, - groupOverlappingFiles: false, - autoMerge: false, - worktreeInitCommand: undefined, - ...settingsOverride, - }; - store.getSettings.mockResolvedValue(mergedSettings); - - let capturedTools: any[] = []; - mockedCreateFnAgent.mockImplementation(async (opts: any) => { - capturedTools = opts.customTools || []; - // Child agent sessions get a never-resolving prompt so runSpawnedChild - // doesn't complete and decrement totalSpawnedCount before limit checks. - const isChildAgent = opts.systemPrompt?.includes("child agent spawned"); - return { - session: { - prompt: isChildAgent ? vi.fn(() => new Promise(() => {})) : vi.fn().mockResolvedValue(undefined), - dispose: vi.fn(), - sessionManager: { - getLeafId: vi.fn().mockReturnValue("leaf-id"), - branchWithSummary: vi.fn(), - }, - navigateTree: vi.fn().mockResolvedValue({ cancelled: false }), - }, - } as any; - }); - - mockedExistsSync.mockReturnValue(true); - // Mock execSync for worktree operations - vi.mocked(execSync).mockReturnValue(""); - - const options: any = {}; - if (agentStore) { - options.agentStore = agentStore; - } - - const executor = new TaskExecutor(store, "/tmp/test", options); - - await executor.execute({ - id: "FN-SPAWN", - title: "Spawn Test", - description: "Spawn test task", - column: "in-progress", - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const tools: Record = {}; - for (const t of capturedTools) { - tools[t.name] = t.execute; - } - return { tools, store, executor }; -} - -describe("Agent Spawning", () => { - beforeEach(() => { - resetExecutorMocks(); - mockedExistsSync.mockReturnValue(true); - vi.mocked(execSync).mockReturnValue(""); - }); - - it("fn_spawn_agent tool is registered in customTools", async () => { - const { tools } = await captureToolsWithAgentStore(); - expect(tools.fn_spawn_agent).toBeDefined(); - expect(typeof tools.fn_spawn_agent).toBe("function"); - }); - - it("returns error when AgentStore is not configured", async () => { - const { tools } = await captureToolsWithAgentStore(undefined); - const result = await tools.fn_spawn_agent("call1", { - name: "researcher", - role: "engineer", - task: "Research something", - }); - - expect(result.content[0].text).toContain("not available"); - expect(result.content[0].text).toContain("no AgentStore configured"); - expect(result.details.state).toBe("error"); - }); - - it("creates agent in AgentStore with correct reportsTo", async () => { - const agentStore = createMockAgentStore(); - const { tools } = await captureToolsWithAgentStore(agentStore); - - const result = await tools.fn_spawn_agent("call1", { - name: "researcher", - role: "engineer", - task: "Research authentication patterns", - }); - - expect(agentStore.createAgent).toHaveBeenCalledOnce(); - const createInput = agentStore.createAgent.mock.calls[0][0]; - expect(createInput.name).toBe("researcher"); - expect(createInput.role).toBe("engineer"); - expect(createInput.reportsTo).toBe("FN-SPAWN"); - expect(createInput.metadata.type).toBe("spawned"); - expect(createInput.metadata.parentTaskId).toBe("FN-SPAWN"); - }); - - it("returns correct SpawnAgentResult structure with state", async () => { - const agentStore = createMockAgentStore(); - const { tools } = await captureToolsWithAgentStore(agentStore); - - const result = await tools.fn_spawn_agent("call1", { - name: "researcher", - role: "engineer", - task: "Research authentication patterns", - }); - - // Parse the JSON from the text content - const parsed = JSON.parse(result.content[0].text); - expect(parsed).toHaveProperty("agentId"); - expect(parsed).toHaveProperty("name", "researcher"); - expect(parsed).toHaveProperty("state", "running"); - expect(parsed).toHaveProperty("role", "engineer"); - expect(parsed).toHaveProperty("message"); - expect(parsed.message).toContain("researcher"); - expect(parsed.message).toContain("Research authentication patterns"); - - // Also check details object - expect(result.details.agentId).toBe(parsed.agentId); - expect(result.details.state).toBe("running"); - }); - - it("transitions agent to active state after creation", async () => { - const agentStore = createMockAgentStore(); - const { tools } = await captureToolsWithAgentStore(agentStore); - - await tools.fn_spawn_agent("call1", { - name: "worker", - role: "custom", - task: "Do some work", - }); - - // Agent is created in idle, then transitioned to active - const agentId = agentStore.createAgent.mock.calls[0][0]; - expect(agentStore.updateAgentState).toHaveBeenCalledWith( - expect.any(String), - "active" - ); - }); - - it("creates child agent session via createFnAgent", async () => { - const agentStore = createMockAgentStore(); - const { tools } = await captureToolsWithAgentStore(agentStore); - - await tools.fn_spawn_agent("call1", { - name: "worker", - role: "engineer", - task: "Do some work", - }); - - // createFnAgent is called at least twice: once for parent, once for child - expect(mockedCreateFnAgent.mock.calls.length).toBeGreaterThanOrEqual(2); - - const parentCall = mockedCreateFnAgent.mock.calls.find( - (call: any) => !call[0].systemPrompt?.includes("child agent spawned") - ); - // Find the child session call - const childCall = mockedCreateFnAgent.mock.calls.find( - (call: any) => call[0].systemPrompt?.includes("child agent spawned") - ); - - expect(parentCall).toBeDefined(); - expect(childCall).toBeDefined(); - expect(childCall![0].tools).toBe("coding"); - expect(childCall![0].cwd).toContain("/.worktrees/swift-falcon"); - expect(childCall![0].systemPrompt).toContain("FN-SPAWN"); - expect(childCall![0].taskEnv).toEqual(parentCall![0].taskEnv); - }); - - it("respects per-parent maxSpawnedAgentsPerParent limit", async () => { - const agentStore = createMockAgentStore(); - const { tools } = await captureToolsWithAgentStore(agentStore, { - maxSpawnedAgentsPerParent: 2, - }); - - // Spawn 2 agents (limit) - await tools.fn_spawn_agent("call1", { name: "a1", role: "engineer", task: "task 1" }); - await tools.fn_spawn_agent("call2", { name: "a2", role: "engineer", task: "task 2" }); - - // 3rd should be rejected - const result = await tools.fn_spawn_agent("call3", { name: "a3", role: "engineer", task: "task 3" }); - expect(result.content[0].text).toContain("Per-parent spawn limit reached"); - expect(result.content[0].text).toContain("2/2"); - expect(result.details.state).toBe("error"); - }); - - it("respects global maxSpawnedAgentsGlobal limit", async () => { - const agentStore = createMockAgentStore(); - const { tools } = await captureToolsWithAgentStore(agentStore, { - maxSpawnedAgentsGlobal: 3, - }); - - // Spawn 3 agents (global limit) - await tools.fn_spawn_agent("call1", { name: "a1", role: "engineer", task: "task 1" }); - await tools.fn_spawn_agent("call2", { name: "a2", role: "engineer", task: "task 2" }); - await tools.fn_spawn_agent("call3", { name: "a3", role: "engineer", task: "task 3" }); - - // 4th should hit global limit - const result = await tools.fn_spawn_agent("call4", { name: "a4", role: "engineer", task: "task 4" }); - expect(result.content[0].text).toContain("Global spawn limit reached"); - expect(result.content[0].text).toContain("3/3"); - expect(result.details.state).toBe("error"); - }); - - it("uses default limits when settings are not specified", async () => { - const agentStore = createMockAgentStore(); - // No spawn settings in the store — defaults should apply - const { tools } = await captureToolsWithAgentStore(agentStore); - - // Should be able to spawn (defaults: 5 per parent, 20 global) - const result = await tools.fn_spawn_agent("call1", { - name: "worker", - role: "engineer", - task: "task 1", - }); - const parsed = JSON.parse(result.content[0].text); - expect(parsed.state).toBe("running"); - }); - - it("handles errors during agent creation gracefully", async () => { - const agentStore = createMockAgentStore(); - agentStore.createAgent.mockRejectedValue(new Error("DB connection failed")); - - const { tools } = await captureToolsWithAgentStore(agentStore); - - const result = await tools.fn_spawn_agent("call1", { - name: "worker", - role: "engineer", - task: "task 1", - }); - - expect(result.content[0].text).toContain("Failed to spawn agent"); - expect(result.content[0].text).toContain("DB connection failed"); - expect(result.details.state).toBe("error"); - }); - - it("trims whitespace from agent name", async () => { - const agentStore = createMockAgentStore(); - const { tools } = await captureToolsWithAgentStore(agentStore); - - await tools.fn_spawn_agent("call1", { - name: " researcher ", - role: "engineer", - task: "task 1", - }); - - const createInput = agentStore.createAgent.mock.calls[0][0]; - expect(createInput.name).toBe("researcher"); - }); - - it("truncates long task descriptions in result message", async () => { - const agentStore = createMockAgentStore(); - const { tools } = await captureToolsWithAgentStore(agentStore); - - const longTask = "A".repeat(200); - const result = await tools.fn_spawn_agent("call1", { - name: "worker", - role: "engineer", - task: longTask, - }); - - const parsed = JSON.parse(result.content[0].text); - expect(parsed.message).toContain("..."); - // The message should contain the first 100 chars - expect(parsed.message.length).toBeLessThan(longTask.length + 50); - }); -}); - -describe("Agent Spawning - Child Termination", () => { - beforeEach(() => { - resetExecutorMocks(); - mockedExistsSync.mockReturnValue(true); - vi.mocked(execSync).mockReturnValue(""); - }); - - it("parent termination triggers all child terminations", async () => { - const agentStore = createMockAgentStore(); - const mockDispose = vi.fn(); - - mockedCreateFnAgent.mockImplementation(async (opts: any) => { - return { - session: { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: mockDispose, - sessionManager: { - getLeafId: vi.fn().mockReturnValue("leaf-id"), - branchWithSummary: vi.fn(), - }, - navigateTree: vi.fn().mockResolvedValue({ cancelled: false }), - }, - } as any; - }); - - const store = createMockStore(); - store.getSettings.mockResolvedValue({ - maxConcurrent: 2, - maxWorktrees: 4, - pollIntervalMs: 15000, - groupOverlappingFiles: false, - autoMerge: false, - }); - - const executor = new TaskExecutor(store, "/tmp/test", { agentStore } as any); - - await executor.execute({ - id: "FN-PARENT", - title: "Parent Task", - description: "Parent", - column: "in-progress", - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - // execute() should have completed, disposing the parent session - // and any child sessions that were spawned - expect(mockDispose).toHaveBeenCalled(); - }); - - it("terminateChildAgent cleans up maps and decrements count", async () => { - const agentStore = createMockAgentStore(); - const store = createMockStore(); - store.getSettings.mockResolvedValue({ - maxConcurrent: 2, - maxWorktrees: 4, - pollIntervalMs: 15000, - groupOverlappingFiles: false, - autoMerge: false, - }); - - const mockDispose = vi.fn(); - mockedCreateFnAgent.mockResolvedValue({ - session: { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: mockDispose, - sessionManager: { - getLeafId: vi.fn().mockReturnValue("leaf-id"), - branchWithSummary: vi.fn(), - }, - navigateTree: vi.fn().mockResolvedValue({ cancelled: false }), - }, - } as any); - - const executor = new TaskExecutor(store, "/tmp/test", { agentStore } as any); - - // Access internal state via any for testing - const internals = executor as any; - - // Simulate spawned agent tracking state - const childId = "agent-test-child"; - const mockSession = { dispose: vi.fn() }; - internals.childSessions.set(childId, mockSession); - internals.spawnedAgents.set("FN-PARENT", new Set([childId])); - internals.totalSpawnedCount = 1; - - // Terminate the child - await internals.terminateChildAgent(childId); - - expect(mockSession.dispose).toHaveBeenCalled(); - expect(internals.childSessions.has(childId)).toBe(false); - // Note: spawnedAgents cleanup is done by terminateAllChildren, not terminateChildAgent - expect(internals.totalSpawnedCount).toBe(0); - expect(agentStore.updateAgentState).toHaveBeenCalledWith(childId, "paused"); - }); - - it("terminateChildAgent handles missing session gracefully", async () => { - const agentStore = createMockAgentStore(); - const store = createMockStore(); - - const executor = new TaskExecutor(store, "/tmp/test", { agentStore } as any); - const internals = executor as any; - - internals.totalSpawnedCount = 1; - - // Terminate a child that doesn't have a session in the map - await internals.terminateChildAgent("nonexistent-agent"); - - // Should still decrement counter and attempt state update - expect(internals.totalSpawnedCount).toBe(0); - expect(agentStore.updateAgentState).toHaveBeenCalledWith("nonexistent-agent", "paused"); - }); - - it("terminateAllChildren handles no children gracefully", async () => { - const agentStore = createMockAgentStore(); - const store = createMockStore(); - - const executor = new TaskExecutor(store, "/tmp/test", { agentStore } as any); - const internals = executor as any; - - // Should not throw when there are no children - await internals.terminateAllChildren("FN-NONE"); - expect(agentStore.updateAgentState).not.toHaveBeenCalled(); - }); - - it("terminateAllChildren terminates all children and cleans up", async () => { - const agentStore = createMockAgentStore(); - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test", { agentStore } as any); - const internals = executor as any; - - // Set up multiple children - const child1 = { dispose: vi.fn() }; - const child2 = { dispose: vi.fn() }; - internals.childSessions.set("c1", child1); - internals.childSessions.set("c2", child2); - internals.spawnedAgents.set("FN-PARENT", new Set(["c1", "c2"])); - internals.totalSpawnedCount = 2; - - await internals.terminateAllChildren("FN-PARENT"); - - expect(child1.dispose).toHaveBeenCalled(); - expect(child2.dispose).toHaveBeenCalled(); - expect(internals.spawnedAgents.has("FN-PARENT")).toBe(false); - expect(internals.totalSpawnedCount).toBe(0); - expect(agentStore.updateAgentState).toHaveBeenCalledWith("c1", "paused"); - expect(agentStore.updateAgentState).toHaveBeenCalledWith("c2", "paused"); - }); - - it("terminateChildAgent handles AgentStore errors gracefully", async () => { - const agentStore = createMockAgentStore(); - agentStore.updateAgentState.mockRejectedValue(new Error("DB error")); - const store = createMockStore(); - - const executor = new TaskExecutor(store, "/tmp/test", { agentStore } as any); - const internals = executor as any; - - const mockSession = { dispose: vi.fn() }; - internals.childSessions.set("c1", mockSession); - internals.totalSpawnedCount = 1; - - // Should not throw even when AgentStore fails - await internals.terminateChildAgent("c1"); - expect(mockSession.dispose).toHaveBeenCalled(); - expect(internals.totalSpawnedCount).toBe(0); - }); - - it("terminateChildAgent auto-deletes agent immediately", async () => { - const agentStore = createMockAgentStore() as any; - agentStore.deleteAgent = vi.fn().mockResolvedValue(undefined); - const store = createMockStore(); - - const executor = new TaskExecutor(store, "/tmp/test", { agentStore } as any); - const internals = executor as any; - - const mockSession = { dispose: vi.fn() }; - const childId = "agent-auto-delete-test"; - internals.childSessions.set(childId, mockSession); - internals.totalSpawnedCount = 1; - - await internals.terminateChildAgent(childId); - - expect(mockSession.dispose).toHaveBeenCalled(); - expect(agentStore.deleteAgent).toHaveBeenCalledTimes(1); - expect(agentStore.deleteAgent).toHaveBeenCalledWith(childId); - expect(internals.pendingEphemeralDeletions.has(childId)).toBe(false); - }); - - it("disposeEphemeralTimers clears pending deletion bookkeeping", async () => { - const agentStore = createMockAgentStore() as any; - agentStore.deleteAgent = vi.fn().mockResolvedValue(undefined); - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test", { agentStore } as any); - const internals = executor as any; - - internals.pendingEphemeralDeletions.add("agent-dispose-test"); - executor.disposeEphemeralTimers(); - - expect(internals.pendingEphemeralDeletions.size).toBe(0); - }); -}); - -describe("Agent Spawning - runSpawnedChild", () => { - beforeEach(() => { - resetExecutorMocks(); - mockedExistsSync.mockReturnValue(true); - }); - - it("updates agent state to running then active on success", async () => { - const agentStore = createMockAgentStore(); - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test", { agentStore } as any); - const internals = executor as any; - - const mockSession = { dispose: vi.fn(), prompt: vi.fn().mockResolvedValue(undefined) }; - internals.childSessions.set("agent-test", mockSession); - internals.totalSpawnedCount = 1; - - await internals.runSpawnedChild("agent-test", mockSession, "Do the research"); - - // Should transition: running → active - expect(agentStore.updateAgentState).toHaveBeenCalledWith("agent-test", "running"); - expect(agentStore.updateAgentState).toHaveBeenCalledWith("agent-test", "active"); - // FNXC:AgentSpawning 2026-06-23-09:52: Completed spawned child sessions must dispose during cleanup so execution memory is released on the normal success path. - expect(mockSession.dispose).toHaveBeenCalledOnce(); - expect(internals.childSessions.has("agent-test")).toBe(false); - expect(internals.totalSpawnedCount).toBe(0); - }); - - it("updates agent state to error on failure", async () => { - const agentStore = createMockAgentStore(); - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test", { agentStore } as any); - const internals = executor as any; - - const mockSession = { dispose: vi.fn() }; - internals.childSessions.set("agent-test", mockSession); - internals.totalSpawnedCount = 1; - - // Make promptWithFallback throw - const { promptWithFallback } = await import("../pi.js"); - vi.mocked(promptWithFallback).mockRejectedValueOnce(new Error("API error")); - - await internals.runSpawnedChild("agent-test", mockSession, "Do the research"); - - expect(agentStore.updateAgentState).toHaveBeenCalledWith("agent-test", "running"); - expect(agentStore.updateAgentState).toHaveBeenCalledWith("agent-test", "error"); - // FNXC:AgentSpawning 2026-06-23-09:52: Failed spawned child sessions must still dispose during cleanup so error paths do not retain provider/runtime state. - expect(mockSession.dispose).toHaveBeenCalledOnce(); - expect(internals.childSessions.has("agent-test")).toBe(false); - expect(internals.totalSpawnedCount).toBe(0); - }); - - it("cleans up even when state update fails", async () => { - const agentStore = createMockAgentStore(); - agentStore.updateAgentState.mockRejectedValue(new Error("DB down")); - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test", { agentStore } as any); - const internals = executor as any; - - const mockSession = { dispose: vi.fn() }; - internals.childSessions.set("agent-test", mockSession); - internals.totalSpawnedCount = 1; - - // Should not throw even when state updates fail - await internals.runSpawnedChild("agent-test", mockSession, "Do the research"); - - expect(mockSession.dispose).toHaveBeenCalledOnce(); - expect(internals.childSessions.has("agent-test")).toBe(false); - expect(internals.totalSpawnedCount).toBe(0); - }); -}); - -// ─── Agent Execution Flow Integration Tests (FN-978) ──────────────────────────── -// -// These tests verify the complete execution flow: event listener registration, -// session creation, stuck detector tracking, and heartbeat recording. -describe("TaskExecutor agent execution flow (FN-978)", () => { - beforeEach(() => { - resetExecutorMocks(); - mockedExistsSync.mockReturnValue(true); - }); - - it("registers task:moved event listener in constructor", () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - // Verify the store.on was called with "task:moved" - expect(store.on).toHaveBeenCalledWith("task:moved", expect.any(Function)); - }); - - it("executes task when task:moved event fires with to='in-progress'", async () => { - const store = createMockStore(); - const session = { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: vi.fn(), - }; - - mockedCreateFnAgent.mockResolvedValue({ session } as any); - - const executor = new TaskExecutor(store, "/tmp/test"); - - const task = { - id: "FN-978", - title: "Test Task", - description: "Test", - column: "in-progress" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - // Trigger the task:moved event manually - store._trigger("task:moved", { task, from: "todo", to: "in-progress" }); - - await waitForAsyncExpectation(() => { - expect(session.prompt).toHaveBeenCalled(); - }); - - // Verify the agent was created and prompt was called - expect(mockedCreateFnAgent).toHaveBeenCalledWith( - expect.objectContaining({ - cwd: expect.any(String), - systemPrompt: expect.any(String), - tools: "coding", - }), - ); - expect(session.prompt).toHaveBeenCalled(); - }); - - it("does not execute task when task:moved event fires with to!='in-progress'", async () => { - const store = createMockStore(); - - mockedCreateFnAgent.mockResolvedValue({ - session: { prompt: vi.fn(), dispose: vi.fn() }, - } as any); - - const executor = new TaskExecutor(store, "/tmp/test"); - - const task = { - id: "FN-978", - title: "Test Task", - description: "Test", - column: "todo", - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - // Trigger the task:moved event with to='done' (should not execute) - store._trigger("task:moved", { task, from: "in-progress", to: "done" }); - - await Promise.resolve(); - - // Verify no agent was created - expect(mockedCreateFnAgent).not.toHaveBeenCalled(); - }); - - describe("merge-state reset when returning to in-progress (FN-2883)", () => { - /* - FNXC:ExecutorReverification 2026-06-29-17:20: - Post-cutover re-verification must follow cleanupMergeStateForReverification() through reopenLastStepForRevision(): a completed task reopens the nearest preceding non-pending work step plus any trailing verification/delivery suffix instead of hard-coding only the previously current legacy step. - */ - it("resets merge state on in-review → in-progress move", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - const executeSpy = vi.spyOn(executor, "execute").mockResolvedValue(undefined); - - const movedTask = { - id: "FN-2883-A", - title: "Merge retry", - description: "desc", - column: "in-progress" as const, - dependencies: [], - steps: [ - { name: "Step 0: Preflight", status: "done" }, - { name: "Step 1: Implementation", status: "done" }, - { name: "Step 2: Testing & Verification", status: "done" }, - { name: "Step 3: Documentation & Delivery", status: "done" }, - ], - currentStep: 3, - log: [], - mergeDetails: { strategy: "manual" } as any, - mergeRetries: 2, - verificationFailureCount: 0, - workflowStepResults: [{ id: "wf-1", status: "passed" }], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - store.getTask.mockResolvedValue(movedTask); - store._trigger("task:moved", { task: movedTask, from: "in-review", to: "in-progress" }); - await waitForAsyncExpectation(() => { - expect(store.updateTask).toHaveBeenCalledWith("FN-2883-A", expect.objectContaining({ - mergeDetails: null, - mergeRetries: 0, - verificationFailureCount: 0, - workflowStepResults: [], - })); - }); - - expect(store.updateTask).toHaveBeenCalledWith("FN-2883-A", expect.objectContaining({ - mergeDetails: null, - mergeRetries: 0, - verificationFailureCount: 0, - workflowStepResults: [], - })); - expect(store.updateStep).toHaveBeenCalledWith("FN-2883-A", 1, "pending"); - expect(store.updateStep).toHaveBeenCalledWith("FN-2883-A", 2, "pending"); - expect(store.updateStep).toHaveBeenCalledWith("FN-2883-A", 3, "pending"); - expect(store.updateTask).toHaveBeenCalledWith("FN-2883-A", expect.objectContaining({ currentStep: 1 })); - await waitForAsyncExpectation(() => { - expect(store.logEntry).toHaveBeenCalledWith( - "FN-2883-A", - expect.stringContaining("Task returned to in-progress from in-review column"), - undefined, - undefined, - ); - }); - expect(executeSpy).toHaveBeenCalled(); - }); - - it("resets merge state on done → in-progress move", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - vi.spyOn(executor, "execute").mockResolvedValue(undefined); - - const movedTask = { - id: "FN-2883-B", - title: "Done rollback", - description: "desc", - column: "in-progress" as const, - dependencies: [], - steps: [ - { name: "Step 0: Preflight", status: "done" }, - { name: "Step 1: Testing & Verification", status: "done" }, - { name: "Step 2: Documentation & Delivery", status: "done" }, - ], - currentStep: 2, - log: [], - mergeDetails: { strategy: "ours" } as any, - mergeRetries: 1, - verificationFailureCount: 0, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - store.getTask.mockResolvedValue(movedTask); - store._trigger("task:moved", { task: movedTask, from: "done", to: "in-progress" }); - await waitForAsyncExpectation(() => { - expect(store.updateTask).toHaveBeenCalledWith("FN-2883-B", expect.objectContaining({ - mergeDetails: null, - mergeRetries: 0, - verificationFailureCount: 0, - workflowStepResults: [], - })); - }); - - expect(store.updateTask).toHaveBeenCalledWith("FN-2883-B", expect.objectContaining({ - mergeDetails: null, - mergeRetries: 0, - verificationFailureCount: 0, - workflowStepResults: [], - })); - expect(store.updateStep).toHaveBeenCalledWith("FN-2883-B", 0, "pending"); - expect(store.updateStep).toHaveBeenCalledWith("FN-2883-B", 1, "pending"); - expect(store.updateStep).toHaveBeenCalledWith("FN-2883-B", 2, "pending"); - expect(store.updateTask).toHaveBeenCalledWith("FN-2883-B", expect.objectContaining({ currentStep: 0 })); - await waitForAsyncExpectation(() => { - expect(store.logEntry).toHaveBeenCalledWith( - "FN-2883-B", - expect.stringContaining("Task returned to in-progress from done column"), - undefined, - undefined, - ); - }); - }); - - it("preserves verificationFailureCount for merge remediation cycles even if status was cleared", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - vi.spyOn(executor, "execute").mockResolvedValue(undefined); - - const movedTask = { - id: "FN-2883-D", - title: "Verification remediation", - description: "desc", - column: "in-progress" as const, - dependencies: [], - steps: [{ name: "Step 2: Testing & Verification", status: "done" }], - currentStep: 0, - log: [], - mergeDetails: { strategy: "manual" } as any, - mergeRetries: 0, - status: null, - verificationFailureCount: 2, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - store.getTask.mockResolvedValue(movedTask); - store._trigger("task:moved", { task: movedTask, from: "in-review", to: "in-progress" }); - await waitForAsyncExpectation(() => { - expect(store.updateTask).toHaveBeenCalledWith("FN-2883-D", expect.objectContaining({ - mergeDetails: null, - mergeRetries: 0, - verificationFailureCount: 2, - workflowStepResults: [], - })); - }); - - expect(store.updateTask).toHaveBeenCalledWith("FN-2883-D", expect.objectContaining({ - mergeDetails: null, - mergeRetries: 0, - verificationFailureCount: 2, - workflowStepResults: [], - })); - }); - - it("does not reset merge state on todo → in-progress move", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - vi.spyOn(executor, "execute").mockResolvedValue(undefined); - - const movedTask = { - id: "FN-2883-C", - title: "Fresh start", - description: "desc", - column: "in-progress" as const, - dependencies: [], - steps: [{ name: "Step 0: Preflight", status: "pending" }], - currentStep: 0, - log: [], - mergeDetails: null, - mergeRetries: 0, - verificationFailureCount: 0, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - store._trigger("task:moved", { task: movedTask, from: "todo", to: "in-progress" }); - await Promise.resolve(); - - expect(store.updateTask).not.toHaveBeenCalledWith("FN-2883-C", expect.objectContaining({ mergeDetails: null })); - expect(store.updateStep).not.toHaveBeenCalled(); - }); - }); - - describe("when task is moved away from in-progress", () => { - it("terminates active session and removes from activeSessions map", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - - const disposeSpy = vi.fn(); - const mockSession = { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: disposeSpy, - steer: vi.fn(), - }; - - const worktreePath = "/tmp/test/.worktrees/fn-001"; - (executor as any).addActiveWorktree("FN-001", worktreePath); - (executor as any).setActiveSession("FN-001", { - session: mockSession, - seenSteeringIds: new Set(), - }, worktreePath); - expect(activeSessionRegistry.isPathActive(worktreePath)).toBe(true); - - const task = { - id: "FN-001", - title: "Test Task", - description: "Test", - column: "todo" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - // Trigger task:moved away from in-progress - store._trigger("task:moved", { task, from: "in-progress", to: "todo" }); - - await waitForAsyncExpectation(() => { - expect(disposeSpy).toHaveBeenCalled(); - }); - - // Verify session was disposed and removed from map - expect(disposeSpy).toHaveBeenCalled(); - expect((executor as any).activeSessions.has("FN-001")).toBe(false); - expect(activeSessionRegistry.isPathActive(worktreePath)).toBe(false); - // Verify task was added to pausedAborted set - expect((executor as any).pausedAborted.has("FN-001")).toBe(true); - }); - - it("terminates active step executor when task is moved away", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - - const mockTerminateAllSessions = vi.fn().mockResolvedValue(undefined); - const mockStepExecutor = { - executeAll: vi.fn().mockResolvedValue([]), - terminateAllSessions: mockTerminateAllSessions, - cleanup: vi.fn().mockResolvedValue(undefined), - }; - - const worktreePath = "/tmp/test/.worktrees/fn-002"; - (executor as any).addActiveWorktree("FN-002", worktreePath); - (executor as any).setActiveStepExecutor("FN-002", mockStepExecutor as any, worktreePath); - expect(activeSessionRegistry.isPathActive(worktreePath)).toBe(true); - - const task = { - id: "FN-002", - title: "Test Task", - description: "Test", - column: "todo" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - // Trigger task:moved away from in-progress - store._trigger("task:moved", { task, from: "in-progress", to: "triage" }); - - await waitForAsyncExpectation(() => { - expect(mockTerminateAllSessions).toHaveBeenCalled(); - }); - - // Verify terminateAllSessions was called - expect(mockTerminateAllSessions).toHaveBeenCalled(); - // Verify removed from map - expect((executor as any).activeStepExecutors.has("FN-002")).toBe(false); - expect(activeSessionRegistry.isPathActive(worktreePath)).toBe(false); - }); - - it("terminates active workflow-step session when task is moved away", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - const worktreePath = "/tmp/test/.worktrees/fn-006"; - const abort = vi.fn().mockResolvedValue(undefined); - const dispose = vi.fn(); - - (executor as any).addActiveWorktree("FN-006", worktreePath); - (executor as any).setActiveWorkflowStepSession("FN-006", { abort, dispose } as any, worktreePath); - expect(activeSessionRegistry.isPathActive(worktreePath)).toBe(true); - - const task = { - id: "FN-006", - title: "Test Task", - description: "Test", - column: "todo" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - store._trigger("task:moved", { task, from: "in-progress", to: "triage" }); - - await waitForAsyncExpectation(() => { - expect(abort).toHaveBeenCalled(); - }); - - expect(dispose).toHaveBeenCalled(); - expect((executor as any).activeWorkflowStepSessions.has("FN-006")).toBe(false); - expect(activeSessionRegistry.isPathActive(worktreePath)).toBe(false); - }); - - it("handles graceful no-op when no active session exists", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - - // No active session set - - const task = { - id: "FN-003", - title: "Test Task", - description: "Test", - column: "todo" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - // Should not throw - expect(() => { - store._trigger("task:moved", { task, from: "in-progress", to: "triage" }); - }).not.toThrow(); - }); - - it("untracks task from stuck detector when moved away", async () => { - const store = createMockStore(); - const untrackSpy = vi.fn(); - const stuckDetector = { - trackTask: vi.fn(), - recordActivity: vi.fn(), - recordProgress: vi.fn(), - untrackTask: untrackSpy, - }; - - const executor = new TaskExecutor(store, "/tmp/test", { - stuckTaskDetector: stuckDetector as any, - }); - - const disposeSpy = vi.fn(); - const mockSession = { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: disposeSpy, - steer: vi.fn(), - }; - - (executor as any).activeSessions.set("FN-004", { - session: mockSession, - seenSteeringIds: new Set(), - }); - - const task = { - id: "FN-004", - title: "Test Task", - description: "Test", - column: "todo" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - store._trigger("task:moved", { task, from: "in-progress", to: "todo" }); - - await waitForAsyncExpectation(() => { - expect(untrackSpy).toHaveBeenCalledWith("FN-004"); - }); - }); - - it("adds task to pausedAborted set to prevent re-execution", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - - const disposeSpy = vi.fn(); - const mockSession = { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: disposeSpy, - steer: vi.fn(), - }; - - (executor as any).activeSessions.set("FN-005", { - session: mockSession, - seenSteeringIds: new Set(), - }); - - const task = { - id: "FN-005", - title: "Test Task", - description: "Test", - column: "triage" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - store._trigger("task:moved", { task, from: "in-progress", to: "triage" }); - - await waitForAsyncExpectation(() => { - expect((executor as any).pausedAborted.has("FN-005")).toBe(true); - }); - }); - }); - - it("tracks task with stuck detector after session creation", async () => { - const store = createMockStore(); - const stuckDetector = { - trackTask: vi.fn(), - recordActivity: vi.fn(), - recordProgress: vi.fn(), - untrackTask: vi.fn(), - }; - - const session = { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: vi.fn(), - }; - - mockedCreateFnAgent.mockResolvedValue({ session } as any); - - const executor = new TaskExecutor(store, "/tmp/test", { - stuckTaskDetector: stuckDetector as any, - }); - - const task = { - id: "FN-978", - title: "Test Task", - description: "Test", - column: "in-progress" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - await executor.execute(task); - - // Verify trackTask was called with task ID - expect(stuckDetector.trackTask).toHaveBeenCalledWith("FN-978", expect.anything()); - // Verify recordActivity was called (heartbeat on prompt start) - expect(stuckDetector.recordActivity).toHaveBeenCalledWith("FN-978"); - // Verify untrackTask was called in the finally block - expect(stuckDetector.untrackTask).toHaveBeenCalledWith("FN-978"); - }); - - it("records activity via AgentLogger onText callbacks", async () => { - const store = createMockStore(); - const stuckDetector = { - trackTask: vi.fn(), - recordActivity: vi.fn(), - recordProgress: vi.fn(), - untrackTask: vi.fn(), - }; - - let capturedOnText: ((delta: string) => void) | undefined; - - mockedCreateFnAgent.mockImplementation(async (opts: any) => { - // Capture the onText callback that's passed to createFnAgent - capturedOnText = opts.onText; - return { - session: { - prompt: vi.fn().mockImplementation(async () => { - // Simulate the agent producing text output - if (capturedOnText) { - capturedOnText("Hello world"); - } - }), - dispose: vi.fn(), - }, - } as any; - }); - - const onAgentText = vi.fn(); - const executor = new TaskExecutor(store, "/tmp/test", { - stuckTaskDetector: stuckDetector as any, - onAgentText, - }); - - const task = { - id: "FN-978", - title: "Test Task", - description: "Test", - column: "in-progress" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - await executor.execute(task); - - // Verify that recordActivity was called (at least once for the initial heartbeat - // and possibly more for the simulated text output) - expect(stuckDetector.recordActivity).toHaveBeenCalledWith("FN-978"); - // The initial recordActivity + text callback should result in multiple calls - expect(stuckDetector.recordActivity.mock.calls.length).toBeGreaterThanOrEqual(2); - // Verify onAgentText callback was called with the delta - expect(onAgentText).toHaveBeenCalledWith("FN-978", "Hello world"); - }); - - it("records activity via AgentLogger onToolStart callbacks", async () => { - const store = createMockStore(); - const stuckDetector = { - trackTask: vi.fn(), - recordActivity: vi.fn(), - recordProgress: vi.fn(), - untrackTask: vi.fn(), - }; - - let capturedOnToolStart: ((name: string, args?: Record) => void) | undefined; - - mockedCreateFnAgent.mockImplementation(async (opts: any) => { - capturedOnToolStart = opts.onToolStart; - return { - session: { - prompt: vi.fn().mockImplementation(async () => { - // Simulate the agent calling a tool - if (capturedOnToolStart) { - capturedOnToolStart("bash", { command: "echo test" }); - } - }), - dispose: vi.fn(), - }, - } as any; - }); - - const onAgentTool = vi.fn(); - const executor = new TaskExecutor(store, "/tmp/test", { - stuckTaskDetector: stuckDetector as any, - onAgentTool, - }); - - const task = { - id: "FN-978", - title: "Test Task", - description: "Test", - column: "in-progress" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - await executor.execute(task); - - // Verify that recordActivity was called for the tool usage - expect(stuckDetector.recordActivity).toHaveBeenCalledWith("FN-978"); - // Verify onAgentTool callback was called with the tool name - expect(onAgentTool).toHaveBeenCalledWith("FN-978", "bash"); - }); - - it("feeds ignored fn_task_update rebuffs into the stuck detector", async () => { - const store = createMockStore(); - store.getTask.mockResolvedValue({ - id: "FN-001", - steps: [{ name: "Implement", status: "done" }], - }); - store.updateStep.mockResolvedValue({ - stuckKillCount: 2, - steps: [{ name: "Implement", status: "done" }], - }); - - const stuckDetector = { - recordProgress: vi.fn(), - recordIgnoredStepUpdate: vi.fn(), - getIgnoredStepUpdateCount: vi.fn().mockReturnValue(25), - }; - - const executor = new TaskExecutor(store, "/tmp/test", { - stuckTaskDetector: stuckDetector as any, - }); - (executor as any).loopRecoveryState.set("FN-001", { attempts: 1, pending: false }); - - const tool = (executor as any).createTaskUpdateTool( - "FN-001", - new Map(), - { current: null }, - new Map(), - stuckDetector, - ); - - const result = await tool.execute("call-1", { step: 0, status: "in-progress" }); - - expect(stuckDetector.recordIgnoredStepUpdate).toHaveBeenCalledWith("FN-001"); - expect(result.content[0].text).toContain("already done"); - expect(executorLog.warn).toHaveBeenCalledWith( - expect.stringContaining( - "FN-001: no-progress churn detected (ignoredStepUpdates=25, stuckKillStreak=2) — escalating to STUCK_NO_PROGRESS_CHURN", - ), - ); - }); - - it("marks loop recovery as observed after successful compaction", async () => { - const store = createMockStore(); - const stuckDetector = { - markLoopObserved: vi.fn(), - }; - const executor = new TaskExecutor(store, "/tmp/test", { - stuckTaskDetector: stuckDetector as any, - }); - - const session = { - compact: vi.fn(async () => ({ summary: "Compacted conversation", tokensBefore: 150000 })), - steer: vi.fn(async () => {}), - sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, - state: {}, - }; - (executor as any).activeSessions.set("FN-001", { - session, - seenSteeringIds: new Set(), - }); - - const result = await executor.handleLoopDetected({ - taskId: "FN-001", - reason: "loop", - noProgressMs: 600000, - inactivityMs: 0, - activitySinceProgress: 100, - ignoredStepUpdateCount: 0, - shouldRequeue: true, - }); - - expect(result).toBe(true); - expect(stuckDetector.markLoopObserved).toHaveBeenCalledWith("FN-001"); - }); - - it("prevents duplicate execution when task:moved fires twice for same task", async () => { - const store = createMockStore(); - let resolvePrompt: (() => void) | undefined; - const session = { - prompt: vi.fn().mockImplementation( - () => new Promise((resolve) => { - resolvePrompt = resolve; - }), - ), - dispose: vi.fn(), - }; - - mockedCreateFnAgent.mockResolvedValue({ session } as any); - - const executor = new TaskExecutor(store, "/tmp/test"); - - const task = { - id: "FN-978", - title: "Test Task", - description: "Test", - column: "in-progress" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - // Trigger the event twice quickly while the first execution is still in flight. - store._trigger("task:moved", { task, from: "todo", to: "in-progress" }); - store._trigger("task:moved", { task, from: "todo", to: "in-progress" }); - - await waitForAsyncExpectation(() => { - expect(mockedCreateFnAgent).toHaveBeenCalledTimes(1); - expect(session.prompt).toHaveBeenCalledTimes(1); - }); - - resolvePrompt?.(); - await Promise.resolve(); - - // The executing guard prevents duplicate execution from the event handler. - // Verify that store.on was called with task:moved (listener registered) - expect(store.on).toHaveBeenCalledWith("task:moved", expect.any(Function)); - expect(mockedCreateFnAgent).toHaveBeenCalledTimes(1); - expect(session.prompt).toHaveBeenCalledTimes(1); - }); - - it("logs error when execute() fails in task:moved handler", async () => { - const store = createMockStore(); - mockedCreateFnAgent.mockRejectedValue(new Error("model not found")); - - const onError = vi.fn(); - const executor = new TaskExecutor(store, "/tmp/test", { onError }); - - const task = { - id: "FN-978", - title: "Test Task", - description: "Test", - column: "in-progress" as const, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - // Trigger the event - store._trigger("task:moved", { task, from: "todo", to: "in-progress" }); - - await waitForAsyncExpectation(() => { - expect(onError).toHaveBeenCalledWith( - expect.objectContaining({ id: "FN-978" }), - expect.any(Error), - ); - }); - - // Verify the error handler was called - expect(onError).toHaveBeenCalledWith( - expect.objectContaining({ id: "FN-978" }), - expect.any(Error), - ); - }); -}); - -describe("FN-2883 fast-path guards", () => { - beforeEach(() => { - resetExecutorMocks(); - mockedExistsSync.mockReturnValue(true); - }); - - it("execute() defensively clears stale mergeDetails for in-progress tasks", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - - const cleanupSpy = vi.spyOn(executor as any, "cleanupMergeStateForReverification") - .mockResolvedValue({ - id: "FN-2883-D", - title: "stale merge", - description: "desc", - column: "in-progress", - dependencies: [], - steps: [{ name: "Step 0", status: "pending" }], - currentStep: 0, - log: [], - mergeDetails: null, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const task = { - id: "FN-2883-D", - title: "stale merge", - description: "desc", - column: "in-progress" as const, - dependencies: [], - steps: [{ name: "Step 0", status: "pending" }], - currentStep: 0, - log: [], - mergeDetails: { strategy: "theirs" } as any, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - await executor.execute(task as any); - - expect(cleanupSpy).toHaveBeenCalledWith( - expect.objectContaining({ id: "FN-2883-D" }), - expect.stringContaining("stale merge state"), - ); - }); - - it("stale merge cleanup preserves passed plan review status while clearing post-execution gates", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - const task = { - id: "FN-7228", - title: "stale merge", - description: "desc", - column: "in-progress" as const, - dependencies: [], - steps: [{ name: "Step 0", status: "in-progress" }], - currentStep: 0, - log: [], - mergeDetails: { strategy: "manual" }, - workflowStepResults: [ - { - workflowStepId: "plan-review", - workflowStepName: "Plan Review", - phase: "pre-merge", - status: "passed", - }, - { - workflowStepId: "code-review", - workflowStepName: "Code Review", - phase: "pre-merge", - status: "passed", - }, - ], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask.mockResolvedValue({ ...task, mergeDetails: null }); - - await (executor as any).cleanupMergeStateForReverification(task, "cleanup stale merge state"); - - expect(store.updateTask).toHaveBeenCalledWith( - "FN-7228", - expect.objectContaining({ - workflowStepResults: [ - expect.objectContaining({ - workflowStepId: "plan-review", - workflowStepName: "Plan Review", - status: "passed", - }), - ], - }), - ); - expect(store.updateTask).not.toHaveBeenCalledWith( - "FN-7228", - expect.objectContaining({ - workflowStepResults: expect.arrayContaining([ - expect.objectContaining({ workflowStepId: "code-review" }), - ]), - }), - ); - }); - - it("stale merge cleanup reconstructs passed plan review status from logs when the row is missing", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - const task = { - id: "FN-7228", - title: "stale merge", - description: "desc", - column: "in-progress" as const, - dependencies: [], - steps: [{ name: "Step 0", status: "in-progress" }], - currentStep: 0, - log: [ - { - timestamp: "2026-06-29T10:34:21.917Z", - action: "[pre-merge] Workflow step completed: Plan Review", - outcome: "Plan Review approved the revised specification.", - }, - ], - mergeDetails: { strategy: "manual" }, - workflowStepResults: undefined, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask.mockResolvedValue({ ...task, mergeDetails: null }); - - await (executor as any).cleanupMergeStateForReverification(task, "cleanup stale merge state"); - - expect(store.updateTask).toHaveBeenCalledWith( - "FN-7228", - expect.objectContaining({ - workflowStepResults: [ - expect.objectContaining({ - workflowStepId: "plan-review", - workflowStepName: "Plan Review", - status: "passed", - verdict: "APPROVE", - output: "Plan Review approved the revised specification.", - }), - ], - }), - ); - }); - - it("resumeOrphaned does not fast-path completed tasks that still have mergeDetails", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - - store.listTasks.mockResolvedValue([ - { - id: "FN-2883-E", - title: "orphan", - description: "desc", - column: "in-progress", - paused: false, - dependencies: [], - steps: [ - { name: "Step 0", status: "done" }, - { name: "Step 1", status: "done" }, - ], - currentStep: 1, - log: [], - mergeDetails: { strategy: "manual" }, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }, - ]); - - const executeSpy = vi.spyOn(executor, "execute").mockResolvedValue(undefined); - const recoverSpy = vi.spyOn(executor, "recoverCompletedTask").mockResolvedValue(false); - - await executor.resumeOrphaned(); - - expect(recoverSpy).not.toHaveBeenCalled(); - expect(executeSpy).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-2883-E" })); - }); -}); - -describe("TaskExecutor watchdogs", () => { - beforeEach(() => { - resetExecutorMocks(); - mockedExistsSync.mockReturnValue(true); - vi.useFakeTimers(); - }); - - afterEach(() => { - vi.useRealTimers(); - }); - - it("recovers a completed task still stuck in-progress after the completion watchdog delay", async () => { - const store = createMockStore(); - const stuckTask = { - id: "FN-WD-1", - title: "Watchdog test", - description: "desc", - column: "in-progress" as const, - paused: false, - dependencies: [], - steps: [{ name: "Step 0", status: "done" as const }], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask.mockImplementation(async () => stuckTask); - - const executor = new TaskExecutor(store, "/tmp/test"); - const recoverSpy = vi.spyOn(executor, "recoverCompletedTask").mockResolvedValue(true); - - (executor as any).scheduleCompletedTaskWatchdog("FN-WD-1", "fn_task_done"); - await vi.advanceTimersByTimeAsync(60_000); - - expect(recoverSpy).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-WD-1" })); - expect(store.logEntry).toHaveBeenCalledWith( - "FN-WD-1", - expect.stringContaining("Watchdog: task remained in-progress 60s after fn_task_done"), - ); - }); - - it("clears the completion watchdog once the task leaves in-progress", async () => { - const store = createMockStore(); - const stuckTask = { - id: "FN-WD-2", - title: "Watchdog clear test", - description: "desc", - column: "in-progress" as const, - paused: false, - dependencies: [], - steps: [{ name: "Step 0", status: "done" as const }], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask.mockImplementation(async () => stuckTask); - - const executor = new TaskExecutor(store, "/tmp/test"); - const recoverSpy = vi.spyOn(executor, "recoverCompletedTask").mockResolvedValue(true); - - (executor as any).scheduleCompletedTaskWatchdog("FN-WD-2", "fn_task_done"); - store._trigger("task:moved", { task: { id: "FN-WD-2" }, from: "in-progress", to: "in-review" }); - await vi.advanceTimersByTimeAsync(60_000); - - expect(recoverSpy).not.toHaveBeenCalled(); - }); - - it("retries a stalled workflow rerun handoff once after the watchdog delay", async () => { - const store = createMockStore(); - const mutableTask: { - id: string; - title: string; - description: string; - column: "in-progress" | "todo"; - paused: boolean; - worktree: string; - dependencies: string[]; - steps: { name: string; status: "done" }[]; - currentStep: number; - log: unknown[]; - createdAt: string; - updatedAt: string; - } = { - id: "FN-WD-3", - title: "Workflow watchdog test", - description: "desc", - column: "in-progress", - paused: false, - worktree: "/tmp/fn-wd-3", - dependencies: [], - steps: [{ name: "Step 0", status: "done" as const }], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - let inProgressAttempts = 0; - - store.getTask.mockImplementation(async () => mutableTask as any); - store.updateTask.mockImplementation(async (_taskId: string, patch: any) => { - if (patch.worktree !== undefined) { - mutableTask.worktree = patch.worktree; - } - return {}; - }); - store.moveTask.mockImplementation(async (_taskId: string, column: string) => { - if (column === "todo") { - mutableTask.column = "todo"; - return {}; - } - if (column === "in-progress") { - inProgressAttempts += 1; - if (inProgressAttempts === 1) { - throw new Error("guard still unwinding"); - } - mutableTask.column = "in-progress"; - } - return {}; - }); - - const executor = new TaskExecutor(store, "/tmp/test"); - - (executor as any).scheduleWorkflowRerun( - "FN-WD-3", - "/tmp/fn-wd-3", - "FN-WD-3: workflow step retry scheduled — moved to todo then in-progress", - ); - - await vi.advanceTimersByTimeAsync(0); - expect(mutableTask.column).toBe("todo"); - - await vi.advanceTimersByTimeAsync(15_000); - - expect(inProgressAttempts).toBe(2); - expect(mutableTask.column).toBe("in-progress"); - expect(store.logEntry).toHaveBeenCalledWith( - "FN-WD-3", - expect.stringContaining("Watchdog: workflow rerun handoff stalled for 15s"), - ); - }); - - it("defers workflow rerun bounce while global pause is active", async () => { - const store = createMockStore(); - store.getSettings.mockResolvedValue({ - maxConcurrent: 2, - maxWorktrees: 4, - pollIntervalMs: 15000, - autoMerge: false, - globalPause: true, - enginePaused: false, - }); - - const executor = new TaskExecutor(store, "/tmp/test"); - const outcome = await (executor as any).performWorkflowRerunBounce("FN-WD-PAUSE", "/tmp/fn-wd-pause"); - - expect(outcome).toBe("deferred-paused"); - expect(store.getTask).not.toHaveBeenCalled(); - expect(store.moveTask).not.toHaveBeenCalled(); - }); - - it("preserves the original executionStartedAt during a workflow rerun bounce", async () => { - const store = createMockStore(); - const originalExecutionStartedAt = "2026-04-30T05:06:43.781Z"; - const mutableTask: { - id: string; - title: string; - description: string; - column: "in-progress" | "todo"; - paused: boolean; - worktree: string; - executionStartedAt: string; - dependencies: string[]; - steps: { name: string; status: string }[]; - currentStep: number; - log: unknown[]; - createdAt: string; - updatedAt: string; - } = { - id: "FN-WD-4", - title: "Workflow rerun timing", - description: "desc", - column: "in-progress", - paused: false, - worktree: "/tmp/fn-wd-4", - executionStartedAt: originalExecutionStartedAt, - dependencies: [], - steps: [{ name: "Step 0", status: "done" as const }], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - store.getTask.mockResolvedValue(mutableTask as any); - store.moveTask.mockImplementation(async (_taskId: string, column: string) => { - mutableTask.column = column as "in-progress" | "todo"; - return { ...mutableTask }; - }); - - const executor = new TaskExecutor(store, "/tmp/test"); - const outcome = await (executor as any).performWorkflowRerunBounce("FN-WD-4", "/tmp/fn-wd-4"); - - expect(outcome).toBe("bounced"); - expect(store.updateTask).toHaveBeenCalledWith("FN-WD-4", { - worktree: "/tmp/fn-wd-4", - executionStartedAt: originalExecutionStartedAt, - }); - expect(store.moveTask.mock.calls).toEqual([ - ["FN-WD-4", "todo", { preserveResumeState: true, preserveWorktree: true }], - ["FN-WD-4", "in-progress"], - ]); - }); -}); - -// ── StepSessionExecutor integration tests ────────────────────────────────── - -describe("StepSessionExecutor integration", () => { - beforeEach(() => { - resetExecutorMocks(); - mockExecuteAll.mockResolvedValue([]); - mockTerminateAllSessions.mockResolvedValue(undefined); - mockCleanup.mockResolvedValue(undefined); - }); - - /** Helper to create a task with steps for step-session mode testing */ - function createTaskWithSteps(overrides: Partial = {}): Task { - return { - id: "FN-200", - title: "Step-session test task", - description: "Test task with steps for step-session execution", - column: "in-progress", - dependencies: [], - steps: [ - { name: "Step 0", status: "pending" }, - { name: "Step 1", status: "pending" }, - ], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - ...overrides, - }; - } - - /** Helper to create a store configured for step-session mode */ - function createStepSessionStore() { - const store = createMockStore(); - store.getSettings.mockResolvedValue({ - maxConcurrent: 2, - maxWorktrees: 4, - pollIntervalMs: 15000, - groupOverlappingFiles: false, - autoMerge: false, - runStepsInNewSessions: true, - maxParallelSteps: 2, - }); - store.getTask.mockResolvedValue({ - id: "FN-200", - title: "Step-session test task", - description: "Test task with steps for step-session execution", - column: "in-progress", - dependencies: [], - steps: [ - { name: "Step 0", status: "pending" }, - { name: "Step 1", status: "pending" }, - ], - currentStep: 0, - log: [], - prompt: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check\n### Step 1: Implement\n- [ ] code", - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - baseCommitSha: "abc123", - enabledWorkflowSteps: [], - }); - return store; - } - - function createTokenUsageStepSessionStore(overrides: Partial = {}) { - const store = createStepSessionStore(); - const taskState: Task = { - ...createTaskWithSteps({ - assignedAgentId: "agent-001", - baseCommitSha: "abc123", - enabledWorkflowSteps: [], - }), - ...overrides, - }; - - store.getTask.mockImplementation(async () => ({ ...taskState })); - store.updateTask.mockImplementation(async (_taskId: string, updates: Record) => { - if (updates.tokenUsage !== undefined) { - (taskState as Task).tokenUsage = updates.tokenUsage as Task["tokenUsage"]; - } - if (updates.status !== undefined) { - (taskState as Task).status = updates.status as Task["status"]; - } - if (updates.error !== undefined) { - (taskState as Task).error = updates.error as Task["error"]; - } - return {}; - }); - - return { store, taskState }; - } - - it("uses step-session path when runStepsInNewSessions is true", async () => { - const store = createStepSessionStore(); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - - await executor.execute(createTaskWithSteps()); - - // StepSessionExecutor constructor should have been called - expect(mockedStepSessionExecutor).toHaveBeenCalled(); - // executeAll should have been called - expect(mockExecuteAll).toHaveBeenCalledOnce(); - // createFnAgent should NOT have been called for step-session path - expect(mockedCreateFnAgent).not.toHaveBeenCalled(); - }); - - it("uses single-session path when runStepsInNewSessions is false (default)", async () => { - const store = createMockStore(); - // Default settings: no runStepsInNewSessions - store.getSettings.mockResolvedValue({ - maxConcurrent: 2, - maxWorktrees: 4, - pollIntervalMs: 15000, - groupOverlappingFiles: false, - autoMerge: false, - }); - store.getTask.mockResolvedValue({ - id: "FN-200", - title: "Test", - description: "Test", - column: "in-progress", - dependencies: [], - steps: [{ name: "Step 0", status: "pending" }], - currentStep: 0, - log: [], - prompt: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check", - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - mockedCreateFnAgent.mockResolvedValue({ - session: { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: vi.fn(), - subscribe: vi.fn(), - on: vi.fn(), - sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, - state: {}, - }, - } as any); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - await executor.execute(createTaskWithSteps()); - - // Should NOT use step-session executor - expect(mockedStepSessionExecutor).not.toHaveBeenCalled(); - // Should use the traditional single-session agent - expect(mockedCreateFnAgent).toHaveBeenCalled(); - }); - - it("success path moves task to in-review and calls onComplete", async () => { - const store = createStepSessionStore(); - - mockExecuteAll.mockResolvedValue([ - { stepIndex: 0, success: true, retries: 0 }, - { stepIndex: 1, success: true, retries: 0 }, - ]); - - const onComplete = vi.fn(); - const onError = vi.fn(); - const executor = new TaskExecutor(store, "/tmp/test", { onComplete, onError }); - - await executor.execute(createTaskWithSteps()); - - expect(mockExecuteAll).toHaveBeenCalledOnce(); - expect(store.moveTask).toHaveBeenCalledWith("FN-200", "in-review"); - expect(onComplete).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-200" })); - expect(onError).not.toHaveBeenCalled(); - }); - - it("failure path marks task as failed with step error summary", async () => { - const store = createStepSessionStore(); - - mockExecuteAll.mockResolvedValue([ - { stepIndex: 0, success: true, retries: 0 }, - { stepIndex: 1, success: false, error: "compilation error", retries: 3 }, - ]); - - const onComplete = vi.fn(); - const onError = vi.fn(); - const executor = new TaskExecutor(store, "/tmp/test", { onComplete, onError }); - - await executor.execute(createTaskWithSteps()); - - expect(store.updateTask).toHaveBeenCalledWith("FN-200", { - status: "failed", - error: "Step 1: compilation error", - }); - expect(store.moveTask).toHaveBeenCalledWith("FN-200", "in-review"); - expect(onError).toHaveBeenCalledWith( - expect.objectContaining({ id: "FN-200" }), - expect.objectContaining({ message: "Step 1: compilation error" }), - ); - expect(onComplete).not.toHaveBeenCalled(); - }); - - it("persists aggregated tokenUsage from step-session results", async () => { - const { store } = createTokenUsageStepSessionStore(); - mockExecuteAll.mockResolvedValue([ - { - stepIndex: 0, - success: true, - retries: 0, - tokenUsage: { inputTokens: 22, outputTokens: 8, cachedTokens: 3, totalTokens: 33 }, - }, - { - stepIndex: 1, - success: true, - retries: 0, - tokenUsage: { inputTokens: 28, outputTokens: 13, cachedTokens: 1, totalTokens: 42 }, - }, - ]); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - await executor.execute(createTaskWithSteps()); - - const tokenUsageUpdates = store.updateTask.mock.calls - .filter(([, updates]: [string, Record]) => updates.tokenUsage) - .map(([, updates]: [string, Record]) => updates.tokenUsage as Record); - - expect(tokenUsageUpdates.length).toBeGreaterThan(0); - expect(tokenUsageUpdates[tokenUsageUpdates.length - 1]).toEqual( - expect.objectContaining({ - inputTokens: 50, - outputTokens: 21, - cachedTokens: 4, - totalTokens: 75, - firstUsedAt: expect.any(String), - lastUsedAt: expect.any(String), - }), - ); - }); - - it("persists tokenUsage incrementally during step execution before in-review transition", async () => { - const { store } = createTokenUsageStepSessionStore(); - - mockedStepSessionExecutor.mockImplementationOnce(function (options: any) { - return { - executeAll: vi.fn(async () => { - options.onStepComplete(0, { - stepIndex: 0, - success: true, - retries: 0, - tokenUsage: { inputTokens: 20, outputTokens: 10, cachedTokens: 2, totalTokens: 32 }, - }); - await Promise.resolve(); - - options.onStepComplete(1, { - stepIndex: 1, - success: true, - retries: 0, - tokenUsage: { inputTokens: 30, outputTokens: 5, cachedTokens: 1, totalTokens: 36 }, - }); - await Promise.resolve(); - - return [ - { - stepIndex: 0, - success: true, - retries: 0, - tokenUsage: { inputTokens: 20, outputTokens: 10, cachedTokens: 2, totalTokens: 32 }, - }, - { - stepIndex: 1, - success: true, - retries: 0, - tokenUsage: { inputTokens: 30, outputTokens: 5, cachedTokens: 1, totalTokens: 36 }, - }, - ]; - }), - terminateAllSessions: mockTerminateAllSessions, - cleanup: mockCleanup, - }; - } as any); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - await executor.execute(createTaskWithSteps()); - - const tokenUsageUpdates = store.updateTask.mock.calls - .filter(([, updates]: [string, Record]) => updates.tokenUsage) - .map(([, updates]: [string, Record]) => updates.tokenUsage as Record); - - expect(tokenUsageUpdates).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - inputTokens: 20, - outputTokens: 10, - cachedTokens: 2, - totalTokens: 32, - }), - expect.objectContaining({ - inputTokens: 50, - outputTokens: 15, - cachedTokens: 3, - totalTokens: 68, - }), - ]), - ); - expect(store.moveTask).toHaveBeenCalledWith("FN-200", "in-review"); - }); - - it("persists task tokenUsage on step-session failure paths so partial usage is visible", async () => { - const { store } = createTokenUsageStepSessionStore(); - mockExecuteAll.mockResolvedValue([ - { - stepIndex: 0, - success: true, - retries: 0, - tokenUsage: { inputTokens: 14, outputTokens: 6, cachedTokens: 2, totalTokens: 22 }, - }, - { - stepIndex: 1, - success: false, - error: "lint failed", - retries: 1, - tokenUsage: { inputTokens: 7, outputTokens: 3, cachedTokens: 1, totalTokens: 11 }, - }, - ]); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - await executor.execute(createTaskWithSteps()); - - const tokenUsageUpdates = store.updateTask.mock.calls - .filter(([, updates]: [string, Record]) => updates.tokenUsage) - .map(([, updates]: [string, Record]) => updates.tokenUsage as Record); - - expect(tokenUsageUpdates[tokenUsageUpdates.length - 1]).toEqual( - expect.objectContaining({ - inputTokens: 21, - outputTokens: 9, - cachedTokens: 3, - totalTokens: 33, - }), - ); - }); - - it("persists tokenUsage from session.getSessionStats in single-session mode", async () => { - const store = createMockStore(); - const taskState = createTaskWithSteps({ - description: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check", - steps: [{ name: "Step 0", status: "done" }], - currentStep: 0, - }); - - store.getSettings.mockResolvedValue({ - maxConcurrent: 2, - maxWorktrees: 4, - pollIntervalMs: 15000, - groupOverlappingFiles: false, - autoMerge: false, - runStepsInNewSessions: false, - }); - store.getTask.mockImplementation(async () => ({ ...taskState })); - store.updateTask.mockImplementation(async (_taskId: string, updates: Record) => { - if (updates.tokenUsage !== undefined) { - (taskState as Task).tokenUsage = updates.tokenUsage as Task["tokenUsage"]; - } - if (updates.status !== undefined) { - (taskState as Task).status = updates.status as Task["status"]; - } - return {}; - }); - - const session = { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: vi.fn(), - subscribe: vi.fn(), - on: vi.fn(), - abortBash: vi.fn(), - state: {}, - sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, - getSessionStats: vi.fn().mockReturnValue({ - tokens: { - input: 31, - output: 17, - cacheRead: 5, - cacheWrite: 2, - total: 55, - }, - }), - }; - - mockedCreateFnAgent.mockResolvedValue({ session } as any); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - await executor.execute(taskState); - - expect(store.updateTask).toHaveBeenCalledWith( - "FN-200", - expect.objectContaining({ - tokenUsage: expect.objectContaining({ - inputTokens: 31, - outputTokens: 17, - cachedTokens: 5, - cacheWriteTokens: 2, - totalTokens: 55, - }), - }), - ); - expect(session.getSessionStats).toHaveBeenCalled(); - }); - - it("persists single-session tokenUsage on failure so partial usage is visible", async () => { - const store = createMockStore(); - const taskState = createTaskWithSteps({ - description: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check", - steps: [{ name: "Step 0", status: "pending" }], - currentStep: 0, - }); - - store.getSettings.mockResolvedValue({ - maxConcurrent: 2, - maxWorktrees: 4, - pollIntervalMs: 15000, - groupOverlappingFiles: false, - autoMerge: false, - runStepsInNewSessions: false, - }); - store.getTask.mockImplementation(async () => ({ ...taskState })); - store.updateTask.mockImplementation(async (_taskId: string, updates: Record) => { - if (updates.tokenUsage !== undefined) { - (taskState as Task).tokenUsage = updates.tokenUsage as Task["tokenUsage"]; - } - if (updates.status !== undefined) { - (taskState as Task).status = updates.status as Task["status"]; - } - if (updates.error !== undefined) { - (taskState as Task).error = updates.error as Task["error"]; - } - return {}; - }); - - const session = { - prompt: vi.fn().mockRejectedValue(new Error("session failed")), - dispose: vi.fn(), - subscribe: vi.fn(), - on: vi.fn(), - abortBash: vi.fn(), - state: {}, - sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, - getSessionStats: vi.fn().mockReturnValue({ - tokens: { - input: 12, - output: 4, - cacheRead: 1, - cacheWrite: 0, - total: 17, - }, - }), - }; - - mockedCreateFnAgent.mockResolvedValue({ session } as any); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - await executor.execute(taskState); - - const tokenUsageUpdates = store.updateTask.mock.calls - .filter(([, updates]: [string, Record]) => updates.tokenUsage) - .map(([, updates]: [string, Record]) => updates.tokenUsage as Record); - - expect(tokenUsageUpdates[tokenUsageUpdates.length - 1]).toEqual( - expect.objectContaining({ - inputTokens: 12, - outputTokens: 4, - cachedTokens: 1, - totalTokens: 17, - }), - ); - expect(session.getSessionStats).toHaveBeenCalled(); - }); - - it("moves task to in-review when step-session execution fails", async () => { - const store = createStepSessionStore(); - - mockExecuteAll.mockRejectedValue(new Error("Infrastructure failure")); - - const onError = vi.fn(); - const executor = new TaskExecutor(store, "/tmp/test", { onError }); - - await executor.execute(createTaskWithSteps()); - - expect(store.updateTask).toHaveBeenCalledWith("FN-200", expect.objectContaining({ - status: "failed", - error: "Infrastructure failure", - })); - expect(store.moveTask).toHaveBeenCalledWith("FN-200", "in-review"); - expect(onError).toHaveBeenCalled(); - }); - - it("moves task to in-review when transient retries are exhausted (step-session)", async () => { - const store = createStepSessionStore(); - - mockExecuteAll.mockRejectedValue(new Error("socket hang up")); - - const onError = vi.fn(); - const executor = new TaskExecutor(store, "/tmp/test", { onError }); - - await executor.execute(createTaskWithSteps({ recoveryRetryCount: 3 })); - - expect(store.updateTask).toHaveBeenCalledWith("FN-200", { - status: "failed", - error: "socket hang up", - recoveryRetryCount: null, - nextRecoveryAt: null, - }); - expect(store.moveTask).toHaveBeenCalledWith("FN-200", "in-review"); - expect(store.moveTask).not.toHaveBeenCalledWith("FN-200", "todo"); - expect(onError).toHaveBeenCalled(); - }); - - it("pause terminates step sessions", async () => { - const store = createStepSessionStore(); - const stuckDetector = { - trackTask: vi.fn(), - untrackTask: vi.fn(), - recordProgress: vi.fn(), - } as any; - - // Make executeAll hang until we trigger pause - let resolveExecuteAll: () => void; - mockExecuteAll.mockReturnValue(new Promise((resolve) => { - resolveExecuteAll = resolve; - })); - - const executor = new TaskExecutor(store, "/tmp/test", { stuckTaskDetector: stuckDetector }); - - const task = createTaskWithSteps(); - - // Start execution (don't await — it will hang) - const executePromise = executor.execute(task); - - await waitForStepExecutorRegistration(executor, "FN-200"); - - // Trigger pause - store._trigger("task:updated", { ...task, paused: true }); - - // Resolve executeAll so the execution can complete - resolveExecuteAll!(); - await executePromise; - - expect(mockTerminateAllSessions).toHaveBeenCalled(); - expect(stuckDetector.untrackTask).toHaveBeenCalledWith("FN-200"); - }); - - it("stuck-kill terminates step sessions", async () => { - const store = createStepSessionStore(); - - // Make executeAll hang - let resolveExecuteAll: () => void; - mockExecuteAll.mockReturnValue(new Promise((resolve) => { - resolveExecuteAll = resolve; - })); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - - const task = createTaskWithSteps(); - const executePromise = executor.execute(task); - - await waitForStepExecutorRegistration(executor, "FN-200"); - - // Trigger stuck kill - executor.markStuckAborted("FN-200"); - - // Resolve executeAll so the execution can complete - resolveExecuteAll!(); - await executePromise; - - expect(mockTerminateAllSessions).toHaveBeenCalled(); - }); - - // ── FN-1461: Step-session stuck retry regression tests ───────────────────────────────────── - - it("REGRESSION: stuck-kill with bare task ID properly requeues step-session task to todo", async () => { - const store = createStepSessionStore(); - - // Make executeAll hang initially - let resolveExecuteAll: (() => void) | null = null; - mockExecuteAll.mockReturnValue(new Promise((resolve) => { - resolveExecuteAll = resolve; - })); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - - const task = createTaskWithSteps(); - const executePromise = executor.execute(task); - - await waitForStepExecutorRegistration(executor, "FN-200"); - - // Verify step executor is registered - expect((executor as any).activeStepExecutors.has("FN-200")).toBe(true); - - // Trigger stuck kill with bare task ID (as StuckTaskDetector.onStuck would call) - executor.markStuckAborted("FN-200", true); - - // Resolve executeAll to complete the execution - resolveExecuteAll!(); - await executePromise; - - // Verify: task should be requeued and moved to todo for retry. - expect(store.updateTask).toHaveBeenCalledWith("FN-200", expect.objectContaining({ - status: "queued", - error: null, - worktree: null, - branch: null, - })); - expect(store.moveTask).toHaveBeenCalledWith("FN-200", "todo", { preserveProgress: true }); - }); - - it("REGRESSION: stuck-kill with exhausted budget does not requeue step-session task", async () => { - const store = createStepSessionStore(); - - let resolveExecuteAll: (() => void) | null = null; - mockExecuteAll.mockReturnValue(new Promise((resolve) => { - resolveExecuteAll = resolve; - })); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - - const task = createTaskWithSteps(); - const executePromise = executor.execute(task); - - await waitForStepExecutorRegistration(executor, "FN-200"); - - // Budget exhausted — should NOT requeue - executor.markStuckAborted("FN-200", false); - - resolveExecuteAll!(); - await executePromise; - - // Should NOT move to todo or mark as stuck-killed - expect(store.moveTask).not.toHaveBeenCalledWith("FN-200", "todo"); - expect(store.moveTask).not.toHaveBeenCalledWith("FN-200", "todo", expect.anything()); - expect(store.updateTask).not.toHaveBeenCalledWith("FN-200", expect.objectContaining({ - status: "stuck-killed", - })); - }); - - it("REGRESSION (FN-6722): pause-abort with step progress preserves branch + resume state on requeue to todo", async () => { - // A mid-run abort on a task that already completed steps (with commits on its - // branch) must not reset progress or drop the branch when bounced to todo — - // otherwise the next dispatch re-plans from Step 0 and the committed work is - // stranded (observed as FN-6722 "lost all progress / stuck"). The teardown - // must move with preserveResumeState and keep the branch pointer. - // Single-session mode — this is the teardown that logs the exact FN-6722 - // string "Execution paused — agent terminated, moved to todo" (executor.ts - // ~9280). Reject the session work with pausedAborted set so execute() enters - // that catch-block teardown, exactly as the FN-6722 hard-cancel did. - const store = createMockStore(); - const taskState = createTaskWithSteps({ - description: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check", - steps: [ - { name: "Step 0", status: "done" }, - { name: "Step 1", status: "pending" }, - ], - currentStep: 1, - branch: "fusion/fn-200", - }); - - store.getSettings.mockResolvedValue({ - maxConcurrent: 2, - maxWorktrees: 4, - pollIntervalMs: 15000, - groupOverlappingFiles: false, - autoMerge: false, - runStepsInNewSessions: false, - }); - store.getTask.mockImplementation(async () => ({ ...taskState })); - - const session = { - prompt: vi.fn().mockRejectedValue(new Error("aborted by hard-cancel")), - dispose: vi.fn(), - subscribe: vi.fn(), - on: vi.fn(), - abortBash: vi.fn(), - state: {}, - sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, - getSessionStats: vi.fn().mockReturnValue({ tokens: {} }), - }; - mockedCreateFnAgent.mockResolvedValue({ session } as any); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - // Mark the in-flight run hard-cancelled so the rejection routes through the - // pausedAborted teardown branch rather than the generic failure sink. - (executor as any).pausedAborted.add("FN-200"); - await executor.execute(taskState); - - // Confirm we exercised the single-session catch-block teardown. - expect(store.logEntry).toHaveBeenCalledWith( - "FN-200", - "Execution paused — agent terminated, moved to todo", - undefined, - expect.anything(), - ); - // Resume state preserved (steps NOT reset to pending)... - expect(store.moveTask).toHaveBeenCalledWith("FN-200", "todo", { preserveResumeState: true }); - // ...and the branch pointer to the committed work is NOT cleared. - expect(store.updateTask).not.toHaveBeenCalledWith( - "FN-200", - expect.objectContaining({ branch: undefined }), - ); - expect(store.updateTask).not.toHaveBeenCalledWith( - "FN-200", - expect.objectContaining({ branch: null }), - ); - }); - - it("REGRESSION (FN-6722): pause-abort preserves progress committed mid-session by a freshly-dispatched task", async () => { - // The dispatch-time `task` snapshot is frozen — a task dispatched fresh - // (currentStep 0, all steps pending) that commits step progress to the store - // mid-session shows that progress ONLY via the store read (`latestTask`), - // never via `task`. The teardown must read `latestTask` so this first-run - // case is preserved too; reading the stale `task` here would clear the branch - // and reset steps — the same FN-6722 failure mode. - const store = createMockStore(); - - // Dispatch-time snapshot: no progress yet. - const task = createTaskWithSteps({ - description: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check", - steps: [ - { name: "Step 0", status: "pending" }, - { name: "Step 1", status: "pending" }, - ], - currentStep: 0, - }); - - store.getSettings.mockResolvedValue({ - maxConcurrent: 2, - maxWorktrees: 4, - pollIntervalMs: 15000, - groupOverlappingFiles: false, - autoMerge: false, - runStepsInNewSessions: false, - }); - // Store snapshot the teardown re-reads: the agent committed Step 0 + a branch - // during the session. column stays in-progress so the 9227 early-return guard - // (which needs column === "todo") does not fire and we reach the teardown. - store.getTask.mockResolvedValue({ - ...task, - column: "in-progress", - currentStep: 1, - branch: "fusion/fn-200", - steps: [ - { name: "Step 0", status: "done" }, - { name: "Step 1", status: "pending" }, - ], - } as any); - - const session = { - prompt: vi.fn().mockRejectedValue(new Error("aborted by hard-cancel")), - dispose: vi.fn(), - subscribe: vi.fn(), - on: vi.fn(), - abortBash: vi.fn(), - state: {}, - sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, - getSessionStats: vi.fn().mockReturnValue({ tokens: {} }), - }; - mockedCreateFnAgent.mockResolvedValue({ session } as any); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - (executor as any).pausedAborted.add("FN-200"); - await executor.execute(task); - - // Progress visible only via latestTask must still be preserved. - expect(store.moveTask).toHaveBeenCalledWith("FN-200", "todo", { preserveResumeState: true }); - expect(store.updateTask).not.toHaveBeenCalledWith( - "FN-200", - expect.objectContaining({ branch: undefined }), - ); - expect(store.updateTask).not.toHaveBeenCalledWith( - "FN-200", - expect.objectContaining({ branch: null }), - ); - }); - - it("REGRESSION: untrackTask called with bare task ID during pause in step-session mode", async () => { - const store = createStepSessionStore(); - - const stuckDetector = { - trackTask: vi.fn(), - untrackTask: vi.fn(), - recordProgress: vi.fn(), - } as any; - - let resolveExecuteAll: (() => void) | null = null; - mockExecuteAll.mockReturnValue(new Promise((resolve) => { - resolveExecuteAll = resolve; - })); - - const executor = new TaskExecutor(store, "/tmp/test", { stuckTaskDetector: stuckDetector }); - - const task = createTaskWithSteps(); - const executePromise = executor.execute(task); - - await waitForStepExecutorRegistration(executor, "FN-200"); - - // Trigger pause - store._trigger("task:updated", { ...task, paused: true }); - - resolveExecuteAll!(); - await executePromise; - - // In step-session mode, untrackTask is called with bare task ID "FN-200" - // But tracking was done with step-scoped keys like "FN-200-step-0" - // BUG: This test captures that the current implementation passes bare ID, - // which won't match the step-scoped tracking keys - expect(stuckDetector.untrackTask).toHaveBeenCalledWith("FN-200"); - // After fix: untrackTask should also clean up any step-scoped entries - }); - - it("REGRESSION: StepSessionExecutor should pass bare task ID for recordProgress in onStepStart", async () => { - // Note: This test verifies the expected contract between StepSessionExecutor and StuckTaskDetector. - // In step-session mode: - // - StepSessionExecutor tracks with step-scoped keys (e.g., "FN-200-step-0") - // - But recordProgress should be called with the bare task ID for consistency - // - StuckTaskDetector.recordProgress should handle this by finding the canonical task ID - // - // Currently, StepSessionExecutor calls recordProgress(task.id) where task.id is "FN-200" - // But the entry was tracked with key "FN-200-step-0", so the lookup fails. - // - // After fix: recordProgress should handle both bare task IDs and step-scoped keys - // by extracting the canonical task ID from step-scoped keys. - // - // This test is informational - the actual fix will be in StuckTaskDetector.recordProgress() - // which should find entries by canonicalTaskId when looking up by bare task ID. - expect(true).toBe(true); // Placeholder - real test verifies StuckTaskDetector behavior - }); - - it("cleanup called in finally block even on error", async () => { - const store = createStepSessionStore(); - - mockExecuteAll.mockRejectedValue(new Error("Fatal error")); - mockCleanup.mockResolvedValue(undefined); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - - await executor.execute(createTaskWithSteps()); - - // cleanup should still have been called despite the error - expect(mockCleanup).toHaveBeenCalledOnce(); - }); - - it("respects semaphore for concurrency control", async () => { - const store = createStepSessionStore(); - const sem = new AgentSemaphore(2); - const runSpy = vi.spyOn(sem, "run"); - - mockExecuteAll.mockResolvedValue([ - { stepIndex: 0, success: true, retries: 0 }, - ]); - - const executor = new TaskExecutor(store, "/tmp/test", { semaphore: sem }); - await executor.execute(createTaskWithSteps()); - - expect(runSpy).toHaveBeenCalledOnce(); - expect(sem.activeCount).toBe(0); - }); - - it("runs without semaphore when not provided", async () => { - const store = createStepSessionStore(); - - mockExecuteAll.mockResolvedValue([ - { stepIndex: 0, success: true, retries: 0 }, - ]); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - await executor.execute(createTaskWithSteps()); - - // Should still work fine without a semaphore - expect(mockExecuteAll).toHaveBeenCalledOnce(); - expect(store.moveTask).toHaveBeenCalledWith("FN-200", "in-review"); - }); - - it("dep-abort during step-session execution triggers cleanup", async () => { - const store = createStepSessionStore(); - - // Make executeAll hang so we can trigger dep-abort mid-execution - let resolveExecuteAll: () => void; - mockExecuteAll.mockReturnValue(new Promise((resolve) => { - resolveExecuteAll = resolve; - })); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - - const task = createTaskWithSteps(); - const executePromise = executor.execute(task); - - await waitForStepExecutorRegistration(executor, "FN-200"); - - // Simulate dep-abort by directly triggering the fn_task_add_dep cleanup logic - // The dep-abort flag should cause the step-session path to handle cleanup - // We can test this by checking that when depAborted is set, cleanup is called - // For now, just resolve and verify cleanup runs - resolveExecuteAll!(); - await executePromise; - - // Verify cleanup was called in finally - expect(mockCleanup).toHaveBeenCalled(); - }); - - // U4 (KTD-2): removed "workflow steps run on success and block on failure" — - // it drove the deleted legacy runWorkflowSteps loop via execute() on a minimal - // store (no getTaskWorkflowSelection). That path now fails closed (KTD-5), and - // the run-on-success / block-on-failure behavior is covered through the graph by - // builtin-coding-workflow-step-results.test.ts. - - it("onStepStart callback updates step status to in-progress", async () => { - const store = createStepSessionStore(); - store.updateStep.mockResolvedValue({} as any); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - await executor.execute(createTaskWithSteps()); - - // Capture the StepSessionExecutor constructor options - expect(mockedStepSessionExecutor).toHaveBeenCalled(); - const ctorOptions = mockedStepSessionExecutor.mock.calls[mockedStepSessionExecutor.mock.calls.length - 1][0]; - - // Invoke the onStepStart callback - ctorOptions.onStepStart!(0); - - // Should update step status in store - expect(store.updateStep).toHaveBeenCalledWith("FN-200", 0, "in-progress"); - }); - - it("onStepComplete callback updates step status to done on success", async () => { - const store = createStepSessionStore(); - store.updateStep.mockResolvedValue({} as any); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - await executor.execute(createTaskWithSteps()); - - const ctorOptions = mockedStepSessionExecutor.mock.calls[mockedStepSessionExecutor.mock.calls.length - 1][0]; - - ctorOptions.onStepComplete!(0, { stepIndex: 0, success: true, retries: 0 }); - - expect(store.updateStep).toHaveBeenCalledWith("FN-200", 0, "done"); - }); - - it("onStepComplete callback updates step status to skipped on failure", async () => { - const store = createStepSessionStore(); - store.updateStep.mockResolvedValue({} as any); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - await executor.execute(createTaskWithSteps()); - - const ctorOptions = mockedStepSessionExecutor.mock.calls[mockedStepSessionExecutor.mock.calls.length - 1][0]; - - ctorOptions.onStepComplete!(1, { stepIndex: 1, success: false, retries: 3 }); - - expect(store.updateStep).toHaveBeenCalledWith("FN-200", 1, "skipped"); - }); - - it("step status update errors do not block execution", async () => { - const store = createStepSessionStore(); - store.updateStep.mockRejectedValue(new Error("DB error")); - - const executor = new TaskExecutor(store, "/tmp/test", {}); - // Should not throw even when updateStep rejects - await executor.execute(createTaskWithSteps()); - - const ctorOptions = mockedStepSessionExecutor.mock.calls[mockedStepSessionExecutor.mock.calls.length - 1][0]; - - // Invoking callbacks should not throw - expect(() => ctorOptions.onStepStart!(0)).not.toThrow(); - expect(() => ctorOptions.onStepComplete!(0, { stepIndex: 0, success: true, retries: 0 })).not.toThrow(); - }); -}); - -describe("FN-5256 awaitAbortInFlightTaskWork pause synchronization", () => { - it("synchronously disposes activeSessions on pause via task:updated", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - - let abortResolve: (() => void) | undefined; - const abortPromise = new Promise((resolve) => { - abortResolve = resolve; - }); - const abortSpy = vi.fn(() => abortPromise); - const disposeSpy = vi.fn(); - (executor as any).activeSessions.set("FN-PAUSE-1", { - session: { abort: abortSpy, dispose: disposeSpy, steer: vi.fn() }, - seenSteeringIds: new Set(), - }); - - const taskUpdatedHandler = (store.on as unknown as ReturnType).mock.calls - .find((call: any[]) => call[0] === "task:updated")?.[1] as (task: any) => Promise; - expect(taskUpdatedHandler).toBeTypeOf("function"); - - const settled = taskUpdatedHandler({ - id: "FN-PAUSE-1", - title: "pause", - description: "pause", - column: "in-progress", - paused: true, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - } satisfies Task); - - // Before abort resolves, dispose must NOT have been called — i.e. handler is - // truly awaiting the abort, not fire-and-forgetting it. - await new Promise((resolve) => setImmediate(resolve)); - expect(disposeSpy).not.toHaveBeenCalled(); - - abortResolve?.(); - await settled; - - expect(abortSpy).toHaveBeenCalledTimes(1); - expect(disposeSpy).toHaveBeenCalledTimes(1); - expect((executor as any).activeSessions.has("FN-PAUSE-1")).toBe(false); - }); - - it("synchronously reaps workflow-step + step-session surfaces on pause", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - - const stepTerminate = vi.fn().mockResolvedValue(undefined); - (executor as any).activeStepExecutors.set("FN-PAUSE-2", { - terminateAllSessions: stepTerminate, - }); - - const workflowAbort = vi.fn().mockResolvedValue(undefined); - const workflowDispose = vi.fn(); - (executor as any).activeWorkflowStepSessions.set("FN-PAUSE-2", { - abort: workflowAbort, - dispose: workflowDispose, - }); - - const taskUpdatedHandler = (store.on as unknown as ReturnType).mock.calls - .find((call: any[]) => call[0] === "task:updated")?.[1] as (task: any) => Promise; - - await taskUpdatedHandler({ - id: "FN-PAUSE-2", - title: "pause", - description: "pause", - column: "in-progress", - paused: true, - dependencies: [], - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - } satisfies Task); - - expect(stepTerminate).toHaveBeenCalledTimes(1); - expect(workflowAbort).toHaveBeenCalledTimes(1); - expect(workflowDispose).toHaveBeenCalledTimes(1); - expect((executor as any).activeStepExecutors.has("FN-PAUSE-2")).toBe(false); - expect((executor as any).activeWorkflowStepSessions.has("FN-PAUSE-2")).toBe(false); - }); - - it("awaitAbortInFlightTaskWork awaits abort before dispose for all surfaces", async () => { - const store = createMockStore(); - const executor = new TaskExecutor(store, "/tmp/test"); - - const order: string[] = []; - const sessionAbort = vi.fn().mockImplementation(async () => { - await new Promise((r) => setTimeout(r, 5)); - order.push("session-abort"); - }); - const sessionDispose = vi.fn(() => order.push("session-dispose")); - (executor as any).activeSessions.set("FN-PAUSE-3", { - session: { abort: sessionAbort, dispose: sessionDispose, steer: vi.fn() }, - seenSteeringIds: new Set(), - }); - - await (executor as any).awaitAbortInFlightTaskWork("FN-PAUSE-3", "test reason"); - - expect(order).toEqual(["session-abort", "session-dispose"]); - expect((executor as any).activeSessions.has("FN-PAUSE-3")).toBe(false); - }); -}); diff --git a/packages/engine/vitest.config.ts b/packages/engine/vitest.config.ts index cc20dec8ce..c8ace669b9 100644 --- a/packages/engine/vitest.config.ts +++ b/packages/engine/vitest.config.ts @@ -149,11 +149,6 @@ export default defineConfig({ FNXC:EngineTests 2026-06-14-02:11: FN-6433 rescued the AI-merge suites by replacing broad activeSessionRegistry cleanup with path-scoped cleanup, so the default engine lane should execute them again. The soft-delete blocker residue suite was deleted under the ratchet because deterministic soft-delete deadlock coverage already owns that invariant. */ - /* - FNXC:EngineTests 2026-06-29-13:55: - FN-7239 quarantines executor-pause.test.ts under the deletion ratchet because it still asserts obsolete direct-dispatch StepSessionExecutor and legacy pause paths after builtin:coding moved to graph execution. Keep graph-path equivalents active in step-session-executor, executor-paused-abort-todo-benign, and workflow-graph-step-rerun before rescuing or deleting this file. - */ - "src/__tests__/executor-pause.test.ts", ], }, }, diff --git a/scripts/__tests__/test-velocity-baseline.test.mjs b/scripts/__tests__/test-velocity-baseline.test.mjs index de0f07fcee..0b857593f8 100644 --- a/scripts/__tests__/test-velocity-baseline.test.mjs +++ b/scripts/__tests__/test-velocity-baseline.test.mjs @@ -287,7 +287,7 @@ describe("main", () => { writeJson(rootDir, "scripts/lib/test-quarantine.json", { entries: [ { - file: "packages/engine/src/__tests__/executor-pause.test.ts", + file: "packages/engine/src/__tests__/hypothetical-quarantined.test.ts", reason: "FN-7239: stale post-cutover direct-dispatch StepSessionExecutor/legacy pause assertions fail after builtin:coding graph cutover.", quarantinedAt: "2026-06-29", }, diff --git a/scripts/lib/test-quarantine.json b/scripts/lib/test-quarantine.json index 921784d05e..39eac9c428 100644 --- a/scripts/lib/test-quarantine.json +++ b/scripts/lib/test-quarantine.json @@ -1,10 +1,4 @@ { "$comment": "Flaky-test quarantine ledger (deletion ratchet — see AGENTS.md 'Flaky tests: quarantine on sight' and docs/testing.md 'Quarantine ledger and the deletion ratchet'). A test observed failing without a corresponding real bug is quarantined ON SIGHT: add an entry here AND a matching one-line `exclude` entry in that package's vitest config, in the same commit. Every entry needs a non-empty `reason` (link the failing run) and a `quarantinedAt` date — the entry expires 14 days later, at which point the test file is DELETED unless someone rescues it with evidence it catches real regressions plus a root-cause fix (never appeasement). There is deliberately no loader module and no automation around this file: it is a dated record, the vitest config exclude is the mechanism, and the sweep is policy executed by whoever touches the suite.", - "entries": [ - { - "file": "packages/engine/src/__tests__/executor-pause.test.ts", - "reason": "FN-7239: stale post-cutover direct-dispatch StepSessionExecutor/legacy pause assertions fail after builtin:coding graph cutover; equivalent direct step-session, pause-abort graph, and graph rerun coverage remains active. Local targeted run: pnpm --filter @fusion/engine exec vitest run --project engine-default src/__tests__/workflow-graph-executor-retry-coding-workflow.test.ts src/__tests__/executor-pause.test.ts --silent=passed-only --reporter=dot (2026-06-29).", - "quarantinedAt": "2026-06-29" - } - ] + "entries": [] } diff --git a/scripts/line-count-baseline.json b/scripts/line-count-baseline.json index 26e42c9a56..b58e323196 100644 --- a/scripts/line-count-baseline.json +++ b/scripts/line-count-baseline.json @@ -76,7 +76,6 @@ "packages/dashboard/src/routes/register-settings-memory-routes.ts": 2382, "packages/dashboard/src/routes/register-task-workflow-routes.ts": 3902, "packages/dashboard/src/server.ts": 2467, - "packages/engine/src/__tests__/executor-pause.test.ts": 3019, "packages/engine/src/__tests__/executor-prompt.test.ts": 2573, "packages/engine/src/__tests__/executor-step-session.test.ts": 3779, "packages/engine/src/__tests__/executor-worktree.test.ts": 2536, diff --git a/scripts/test-timings.json b/scripts/test-timings.json index 76472796d7..fa9509ce3d 100644 --- a/scripts/test-timings.json +++ b/scripts/test-timings.json @@ -1402,7 +1402,6 @@ "packages/engine/src/__tests__/executor-implicit-task-done-revise-guard.test.ts": 100, "packages/engine/src/__tests__/executor-lease-renewal.test.ts": 100, "packages/engine/src/__tests__/executor-missing-task-json-transient.test.ts": 100, - "packages/engine/src/__tests__/executor-pause.test.ts": 300, "packages/engine/src/__tests__/executor-paused-abort-todo-benign.test.ts": 100, "packages/engine/src/__tests__/executor-plan-only-scope-leak.test.ts": 100, "packages/engine/src/__tests__/executor-prompt.test.ts": 500,