Code review of the Task Groups v2 changes (merged in #376) surfaced the following orchestrator findings. Filing as a pre-release tracker. None block the existing debate-style chains (single-root, sequential), but A-C1 affects the "library with many parallel tasks" use case and should be fixed before real load.
🔴 CRITICAL
A-C1 — Permanent group deadlock when independent root tasks > MAX_CONCURRENT_TASKS (5)
Where: server/services/task-orchestrator.ts — startGroup (launchBatch(ready, MAX_CONCURRENT_TASKS, …)) + onTaskCompleted (loop only over status === "blocked").
Detail: buildSeeds marks every dependency-free definition ready. On start, launchBatch runs at most 5. The remaining ready executions are never launched: onTaskCompleted builds newlyReady only from blocked → ready transitions, and there is no other picker for already-ready executions. checkGroupCompletion computes done = every(completed | cancelled), so the stuck ready rows keep the iteration running forever.
Impact: A group with >5 root tasks hangs permanently and is unrecoverable — re-start → 409 (RunActiveError, latest is running), DELETE → 409 ("Cancel the running iteration first"), edits → 409 (assertNotRunning). activeGroupIds leaks.
Fix hint: Real scheduler — after each completion, top up any ready (not only newly-unblocked) up to free slots; or a pending-ready queue. Reset activeGroupIds in a finally.
🟠 HIGH
A-H1 — POST /:id/start blocks the HTTP request for the entire run
Where: server/routes/task-groups.ts (start route await orchestrator.startGroup(...)) → task-orchestrator.ts startGroup await this.launchBatch(...).
Detail: launchBatch does await Promise.allSettled(launched.map(executeExecution)); executeExecution awaits the LLM call AND onTaskCompleted, which recursively awaits the next launchBatch. So startGroup resolves only after the whole reachable DAG settles, and the route returns {group, iteration} only then.
Impact: With taskTimeoutMs=600000 (10 min/task) and multi-round runs, the request hangs minutes → 504 at a proxy/LB/client, though the server keeps working.
Fix hint: Dispatch execution in the background — return {group, iteration} right after creating the iteration + launching; stream progress over the existing WebSocket events.
A-H2 — Failed/cancelled group keeps launching downstream and spending money (no cooperative cancellation)
Where: task-orchestrator.ts — onTaskFailed (markGroupSettled → claims.clear + activeGroupIds.delete) and cancelGroup; sibling onTaskCompleted does not check the group is already settled.
Detail: A and B run in parallel, C depends on B. A fails → group failed, claims cleared, cancelUnreachable does not touch C (its dep B is still running). B finishes → onTaskCompleted unblocks C → launchBatch (on a cleared claim set) starts C in an already-failed group. Likewise cancelGroup marks a running execution cancelled in the DB but does not abort the gateway call — on completion updateExecution overwrites it back to completed and re-fires onTaskCompleted.
Impact: Extra paid LLM/pipeline runs after fail/cancel; inconsistent state (completed executions appearing in a failed/cancelled group); activeGroupIds drift.
Fix hint: Thread an AbortSignal into gateway.completeStreaming/pipeline; in onTaskCompleted/executeExecution no-op when the iteration is no longer running.
🟡 MEDIUM
A-M1 — Retry returns the wrong data layer
server/routes/task-groups.ts retry route does await orchestrator.retryTask(taskId); res.json(await storage.getTask(taskId)). retryTask mutates the EXECUTION; the route returns the definition row (getTask), which in v2 holds only create-time ready/blocked/pending, not the re-run state. Fix: return the updated execution from the latest iteration.
A-M2 — Global concurrency limit not enforced under a completion race
onTaskCompleted computes slotsAvailable = MAX_CONCURRENT_TASKS - runningCount from a local execs snapshot. ExecutionClaims prevents double-launching one execution but not total parallelism: two concurrent onTaskCompleted calls can each launch up to the limit, exceeding it overall. Fix: track active launches in the claim registry / a semaphore instead of recomputing from a snapshot.
A-M3 — After retrying a failed task, cancelled downstream is not revived but the group still completes
On failure, descendants are cancelled. A successful retry of the parent does not return them to ready (onTaskCompleted only looks at blocked). checkGroupCompletion sees completed | cancelled → group completed, though the descendants should have run once the dependency recovered. Fix: on retry, re-initialize transitively-dependent cancelled executions to blocked/ready.
Source: PR #376 review. Severities are the reviewer's; confirm against the code before fixing.
Code review of the Task Groups v2 changes (merged in #376) surfaced the following orchestrator findings. Filing as a pre-release tracker. None block the existing debate-style chains (single-root, sequential), but A-C1 affects the "library with many parallel tasks" use case and should be fixed before real load.
🔴 CRITICAL
A-C1 — Permanent group deadlock when independent root tasks >
MAX_CONCURRENT_TASKS(5)Where:
server/services/task-orchestrator.ts—startGroup(launchBatch(ready, MAX_CONCURRENT_TASKS, …)) +onTaskCompleted(loop only overstatus === "blocked").Detail:
buildSeedsmarks every dependency-free definitionready. On start,launchBatchruns at most 5. The remainingreadyexecutions are never launched:onTaskCompletedbuildsnewlyReadyonly fromblocked → readytransitions, and there is no other picker for already-readyexecutions.checkGroupCompletioncomputesdone = every(completed | cancelled), so the stuckreadyrows keep the iterationrunningforever.Impact: A group with >5 root tasks hangs permanently and is unrecoverable — re-
start→ 409 (RunActiveError, latest isrunning),DELETE→ 409 ("Cancel the running iteration first"), edits → 409 (assertNotRunning).activeGroupIdsleaks.Fix hint: Real scheduler — after each completion, top up any
ready(not only newly-unblocked) up to free slots; or a pending-ready queue. ResetactiveGroupIdsin afinally.🟠 HIGH
A-H1 —
POST /:id/startblocks the HTTP request for the entire runWhere:
server/routes/task-groups.ts(start routeawait orchestrator.startGroup(...)) →task-orchestrator.tsstartGroupawait this.launchBatch(...).Detail:
launchBatchdoesawait Promise.allSettled(launched.map(executeExecution));executeExecutionawaits the LLM call ANDonTaskCompleted, which recursively awaits the nextlaunchBatch. SostartGroupresolves only after the whole reachable DAG settles, and the route returns{group, iteration}only then.Impact: With
taskTimeoutMs=600000(10 min/task) and multi-round runs, the request hangs minutes → 504 at a proxy/LB/client, though the server keeps working.Fix hint: Dispatch execution in the background — return
{group, iteration}right after creating the iteration + launching; stream progress over the existing WebSocket events.A-H2 — Failed/cancelled group keeps launching downstream and spending money (no cooperative cancellation)
Where:
task-orchestrator.ts—onTaskFailed(markGroupSettled→claims.clear+activeGroupIds.delete) andcancelGroup; siblingonTaskCompleteddoes not check the group is already settled.Detail: A and B run in parallel, C depends on B. A fails → group
failed, claims cleared,cancelUnreachabledoes not touch C (its dep B is stillrunning). B finishes →onTaskCompletedunblocks C →launchBatch(on a cleared claim set) starts C in an already-failedgroup. LikewisecancelGroupmarks a running executioncancelledin the DB but does not abort the gateway call — on completionupdateExecutionoverwrites it back tocompletedand re-firesonTaskCompleted.Impact: Extra paid LLM/pipeline runs after fail/cancel; inconsistent state (
completedexecutions appearing in afailed/cancelledgroup);activeGroupIdsdrift.Fix hint: Thread an
AbortSignalintogateway.completeStreaming/pipeline; inonTaskCompleted/executeExecutionno-op when the iteration is no longerrunning.🟡 MEDIUM
A-M1 — Retry returns the wrong data layer
server/routes/task-groups.tsretry route doesawait orchestrator.retryTask(taskId); res.json(await storage.getTask(taskId)).retryTaskmutates the EXECUTION; the route returns the definition row (getTask), which in v2 holds only create-timeready/blocked/pending, not the re-run state. Fix: return the updated execution from the latest iteration.A-M2 — Global concurrency limit not enforced under a completion race
onTaskCompletedcomputesslotsAvailable = MAX_CONCURRENT_TASKS - runningCountfrom a localexecssnapshot.ExecutionClaimsprevents double-launching one execution but not total parallelism: two concurrentonTaskCompletedcalls can each launch up to the limit, exceeding it overall. Fix: track active launches in the claim registry / a semaphore instead of recomputing from a snapshot.A-M3 — After retrying a failed task, cancelled downstream is not revived but the group still completes
On failure, descendants are
cancelled. A successful retry of the parent does not return them toready(onTaskCompletedonly looks atblocked).checkGroupCompletionseescompleted | cancelled→ groupcompleted, though the descendants should have run once the dependency recovered. Fix: on retry, re-initialize transitively-dependent cancelled executions toblocked/ready.Source: PR #376 review. Severities are the reviewer's; confirm against the code before fixing.