fix(FN-4811): process-wide executingTaskLock blocks parallel execute() across instances
Investigating FN-4814 + FN-4811 re-failures after commit8bef30655(which added per-instance synchronous this.executing.add) revealed the per-instance guard was insufficient. FN-4809 log at 02:48:17-18 UTC: 02:48:17 [-] Resuming execution after unpause 02:48:17 [-] Step 4 (Testing & Verification) -> pending 02:48:17 [-] Step 4 (Testing & Verification) -> pending 02:48:17 [6097725-y2nb] Executor detected stale merge state ... 02:48:18 [6097816-9gde] Executor detected stale merge state ... Both runs y2nb and 9gde reached executor.ts:2661 (which is INSIDE execute(), past the synchronous this.executing.add claim). The only viable explanation is that there is more than one TaskExecutor instance in the same Node process (engine restart race, multi-project hybrid runtime, or similar code path). Each instance has its own executing Set, so the per-instance guard doesn't help. Fix: module-level singleton executingTaskLock in active-session-registry.ts, shared across all TaskExecutor instances. execute() synchronously tryClaim()s the lock; if false, bails. Every existing this.executing.delete() site also calls executingTaskLock.release(). Per-instance this.executing kept because many other call sites use it (this.executing.has at handler gates, stuck-detector, resumeTaskForAgent, etc.). Test setup (resetExecutorMocks in executor-test-helpers.ts) clears the lock between tests so process-wide state doesn't leak (executor-pause and executor-prompt tests would otherwise show 'expected 2 createFnAgent calls but got 0' / 'expected not called but called 3 times' flakes). Tests: - executing-task-lock.test.ts: 2 cases. Key case creates TWO TaskExecutor instances and races them on the same task ID, asserts only ONE actually runs. Verified FAILS on prior code (8bef30655) and PASSES on fix. Verification: - Targeted suite (4 files, 170 tests): pass. - pnpm --filter @fusion/engine build: clean. - pnpm lint: clean. Fusion-Task-Id: FN-4811
This commit is contained in:
@@ -248,6 +248,7 @@ import { existsSync, realpathSync } from "node:fs";
|
||||
import { hydrateWorktreeDb } from "../worktree-db-hydrate.js";
|
||||
import { isUsableTaskWorktree } from "../worktree-pool.js";
|
||||
import { classifyStaleLock, tryRemoveStaleLock } from "../worktree-stale-lock.js";
|
||||
import { executingTaskLock } from "../active-session-registry.js";
|
||||
|
||||
export const mockedCreateFnAgent = vi.mocked(createFnAgent);
|
||||
export const mockedSessionManager = vi.mocked(SessionManager);
|
||||
@@ -343,4 +344,9 @@ export function resetExecutorMocks() {
|
||||
mockExecuteAll.mockResolvedValue([]);
|
||||
mockTerminateAllSessions.mockResolvedValue(undefined);
|
||||
mockCleanup.mockResolvedValue(undefined);
|
||||
// FN-4811 follow-up: the executingTaskLock is process-wide module state, so it must
|
||||
// be cleared between tests or earlier tests' claims will block later tests' execute()
|
||||
// calls ("expected at least 2 createFnAgent calls but got 0" / "expected not called
|
||||
// but called 3 times" symptoms in executor-pause / executor-prompt tests).
|
||||
executingTaskLock._clearForTest();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user