Skip to content

Orchestration: reject duplicate task IDs (shadow task deadlocks phase completion) #402

Description

@arul28

Severity: High · Verify confidence: high

File: apps/desktop/src/main/services/orchestration/manifestNormalization.ts:288 (validateTasks) + apps/desktop/src/main/services/orchestration/applyPatches.ts:96-104 (/tasks/- append)

What

Task IDs are author-supplied by the lead agent via raw add /tasks/- patches; nothing enforces uniqueness. validateTasks() (manifestNormalization.ts:288-316) only checks each task.id is a non-empty string, never that the set of ids is unique, and applyPatches' '-' append path (applyPatches.ts:96-104) does a bare arr.push(value) with no dedup (only id-PREDICATE adds are guarded against collisions at applyPatches.ts:88-91). All lookups use manifest.tasks.find((t)=>t.id===taskId) and predicate paths /tasks/{id:X}/... resolve via findIndex to the FIRST match (orchestrationService.ts:1086, 1156; applyPatches.ts:47-49,74). Failure mode: if the lead emits two tasks with id 'T-1' (e.g. a re-plan that re-adds an existing task, or an LLM hallucinating a repeated id), the second is a permanent phantom — it can never be claimed, released, or patched (every operation hits the first copy), yet it still counts toward phase completion. reconcileActivePhaseProgress (manifestNormalization.ts:381-383) and buildPhaseTransitionOpsAfterTaskRelease (manifestNormalization.ts:428) gate phase advance on activePhaseTasks.every(status==='done'). The unreachable duplicate stays 'pending' forever, so the phase never completes and the whole run deadlocks (never reaches validating/wrapup). This is a run-graph step collision with data-loss-grade impact (run becomes permanently stuck).

Trigger

Lead sends manifestPatch with two add /tasks/- ops whose values both have id 'T-1' (or re-adds an existing task id during re-planning). Both pass validateManifestShape. Claim/release/status patches only ever affect the first; the second stays pending and blocks every-done phase advance forever.

Verification (adversarial)

Independently confirmed every link in the causal chain against the real code.

NO UNIQUENESS GUARD: validateTasks (apps/desktop/src/main/services/orchestration/manifestNormalization.ts:288-316) validates each task.id is a non-empty string but never checks set uniqueness. No new-Set-based task-id dedup exists anywhere in the orchestration code (grep for duplicate/unique/dedup found only enum sets and supersede tracking). normalizeManifestShape (manifestNormalization.ts:38) maps tasks 1:1 with no dedup, so disk reloads preserve duplicates too.

APPEND DOES BARE PUSH: applyPatches.ts:96-104 does (parent as unknown[]).push(op.value) for the trailing - segment with zero collision check. The only id-collision guard (applyPatches.ts:88-91, "add op against existing entry... use replace") is in the {id:X} PREDICATE add path, which is not taken for /tasks/- appends.

REACHABLE IN PRODUCTION: The lead agent's manifestPatch AI tool (apps/desktop/src/main/services/ai/tools/orchestrationTools.ts:548-604) accepts arbitrary add ops with author-supplied path/value. Runs start with tasks:[] (orchestrationService.ts:774); tasks are seeded EXCLUSIVELY via add /tasks/- (confirmed in orchestrationTools.test.ts:772,835,908,987). Lead passes LEAD_DENY_PATTERNS (patchPolicy.ts:110-126; /tasks/- not denied) and validateManifestShape (orchestrationService.ts:918-919). This is a genuine in-process disk-backed service wired at registerIpc.ts:5872-5880 — NOT a daemon-routed null-service path, so the runtime-backed-null-service bug class does not apply.

FIRST-MATCH RESOLUTION: claimTask (orchestrationService.ts:1086) and releaseTask (1156) use .find(t=>t.id===id) → first match, and emit /tasks/{id:X}/... ops resolved by findIndex → first match (applyPatches.ts:74). The duplicate is never mutated.

GATE DEADLOCK: reconcileActivePhaseProgress (manifestNormalization.ts:378-384) and buildPhaseTransitionOpsAfterTaskRelease (425-428) include the phantom (matching phaseId, not superseded, pending) and require every(status==='done'). It stays pending forever → phase never advances → run deadlocks before validating/wrapup.

NO RECOVERY: numeric indices rejected everywhere (patchPolicy.ts:45-49; applyPatches.ts:56-57,106-107); remove /tasks/{id:X} splices the FIRST match (applyPatches.ts:75-78), deleting the real task and leaving the phantom — the lead cannot target the duplicate. No watchdog/stuck-detection/timeout exists (grep found nothing). Recovery requires hand-editing manifest.json. No test covers duplicate ids, confirming it is unhandled, not intentionally guarded.

Severity: downgraded from critical to high. Mechanics and reachability are certain, and impact is an unrecoverable run deadlock. But the trigger depends on the lead LLM actually emitting a duplicate id (hallucination, a naive re-plan that re-adds instead of using replace, or a retry after etag_conflict that re-appends). That is plausible and unprevented but not guaranteed on every run, making this a high-severity latent correctness bug rather than a guaranteed-on-every-run critical.


Filed from the 2026-05-29 ADE codebase audit (adversarially verified).

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions