fix(engine): close executor/merger concurrency races and reviewer pause TOCTOU
FN-2910 surfaced concurrent reviewer + merger activity on the same task. Root cause: asymmetric in-flight guards let an unpause-resume kick off a fresh executor session while a recovery path was already running, and the auto-merge handoff fired before the executor's finally block finished cleanup. This sweeps the surrounding lifecycle paths for similar races and tightens the reviewer pause gate against TOCTOU through runtime setup. - Symmetric in-flight tracking across `executing`, `recoveringCompleted`, and `resumingUnpaused`; `recoverCompletedTask` bails when any are set. - Atomic claim of the recovery slot in the completed-task watchdog before any awaited work. - Workflow-rerun bounce returns "bounced" | "skipped-pending" so the watchdog can no longer log a false-success retry when the original bounce is still mid-flight. - Self-healing's completed-task scan re-checks executing IDs inside the loop instead of trusting a pre-await snapshot. - 300ms grace period before auto-merge enqueue, giving the executor's finally block (session disposal, child cleanup) time to drain and eliminating the residual log-overlap symptom from FN-2910. Test uses fake timers, no real sleep added. - New AgentSemaphore.runNested for synchronously nested helper agents (reviewers): bumps activeCount for honest observability while bypassing the wait queue, preserving forward-progress fairness for the parent at low maxConcurrent. Both createReviewStepTool and triage's createReviewSpecTool now use it. - New beforeSpawnSession hook on AgentRuntimeOptions/AgentOptions fired inside createFnAgent immediately before createAgentSession, past every awaited setup step. Reviewer wires a pause re-check that throws a sentinel error converted to UNAVAILABLE, closing the TOCTOU window where pause flipped during runtime resolution or resource loading. All 2887 engine tests pass; engine + core + cli + dashboard + plugin-sdk + pi-claude-cli + desktop typecheck clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1702,7 +1702,12 @@ export class TriageProcessor {
|
||||
// model changes made after the session started.
|
||||
const currentSettings = await store.getSettings();
|
||||
|
||||
const result = await reviewStep(
|
||||
// Spec reviewer runs via semaphore.runNested so it transiently
|
||||
// bumps activeCount for honest observability while bypassing the
|
||||
// wait queue (no fairness regression at low maxConcurrent). See
|
||||
// concurrency.ts:runNested for the contract.
|
||||
const sem = options.semaphore;
|
||||
const invokeReviewer = () => reviewStep(
|
||||
rootDir,
|
||||
taskId,
|
||||
0,
|
||||
@@ -1736,6 +1741,9 @@ export class TriageProcessor {
|
||||
rootDir,
|
||||
},
|
||||
);
|
||||
const result = sem
|
||||
? await sem.runNested(invokeReviewer)
|
||||
: await invokeReviewer();
|
||||
|
||||
// Track verdict for post-session enforcement
|
||||
specReviewVerdictRef.current = result.verdict;
|
||||
|
||||
Reference in New Issue
Block a user