Skip to content

ADE-48: Orchestration: reject duplicate task IDs (shadow task deadlocks phase completion)#436

Merged
arul28 merged 1 commit into
mainfrom
ade-48-orchestration-reject-duplicate-task-ids-shadow-task-deadlocks-phase-completion
May 30, 2026
Merged

ADE-48: Orchestration: reject duplicate task IDs (shadow task deadlocks phase completion)#436
arul28 merged 1 commit into
mainfrom
ade-48-orchestration-reject-duplicate-task-ids-shadow-task-deadlocks-phase-completion

Conversation

@arul28
Copy link
Copy Markdown
Owner

@arul28 arul28 commented May 30, 2026

Fixes ADE-48

Summary

Describe the change.

What Changed

Key files and behaviors.

Validation

How you tested.

Risks

Anything to watch.

Linked Linear issues

ADE   Open in ADE  ·  ade-48-orchestration-reject-duplicate-task-ids-shadow-task-deadlocks-phase-completion branch  ·  PR #436

Greptile Summary

Adds duplicate-task-ID rejection to the manifest validation layer so that a shadow task can no longer silently occupy an ID and prevent phase completion. The guard runs at validation time, before any write is committed, and works across both same-patch and cross-patch duplication scenarios.

  • validateTasks now maintains a Set<string> of trimmed IDs and returns an early error on the first duplicate found, preventing the poisoned manifest from ever reaching disk.
  • Two new integration tests verify that manifestPatch returns validation_failed and leaves on-disk state unchanged (or at the pre-duplicate length) for both scenarios.

Confidence Score: 5/5

Safe to merge — the change is a targeted, additive validation guard with no side effects on the happy path.

The implementation is a minimal, focused addition: a Set is allocated once per validateTasks call, the duplicate check fires before any write, and the existing validation chain is otherwise untouched. Both same-patch and cross-patch duplicate scenarios are covered by integration tests that also verify on-disk state. No regressions were introduced.

No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/src/main/services/orchestration/manifestNormalization.ts Adds duplicate task ID detection to validateTasks using a Set; trims IDs consistently across all error messages in the loop.
apps/desktop/src/main/services/orchestration/orchestrationService.test.ts Adds makeTask helper and two integration tests covering same-patch duplicates and cross-patch duplicates; on-disk state verified after each rejection.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[manifestPatch called] --> B[Apply JSON patches in memory]
    B --> C[validateManifestShape]
    C --> D[validateTasks]
    D --> E{task.id empty?}
    E -- yes --> F[return error: non-empty id required]
    E -- no --> G[taskId = task.id.trim]
    G --> H{seenIds.has taskId?}
    H -- yes --> I[return error: duplicate id]
    H -- no --> J[seenIds.add taskId]
    J --> K[validate phaseId, title, description, status, validationGate]
    K --> L{more tasks?}
    L -- yes --> E
    L -- no --> M[return null — valid]
    M --> N[Persist manifest to disk]
    I --> O[Reject patch — no write]
    F --> O
Loading

Reviews (5): Last reviewed commit: "Fix duplicate orchestration task id reje..." | Re-trigger Greptile

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 30, 2026

ADE-48

@vercel
Copy link
Copy Markdown

vercel Bot commented May 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview May 30, 2026 7:55am

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

Warning

Review limit reached

@arul28, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 26 minutes and 10 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4dfc90ab-1f2a-4c7c-a4ca-46216d5cba49

📥 Commits

Reviewing files that changed from the base of the PR and between 8dc8834 and 29391cb.

📒 Files selected for processing (2)
  • apps/desktop/src/main/services/orchestration/manifestNormalization.ts
  • apps/desktop/src/main/services/orchestration/orchestrationService.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade-48-orchestration-reject-duplicate-task-ids-shadow-task-deadlocks-phase-completion

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@capy-ai
Copy link
Copy Markdown

capy-ai Bot commented May 30, 2026

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 30, 2026

@copilot review but do not make fixes

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 30, 2026

@copilot review but do not make fixes

@arul28 arul28 force-pushed the ade-48-orchestration-reject-duplicate-task-ids-shadow-task-deadlocks-phase-completion branch from 4c5d11f to f005304 Compare May 30, 2026 07:06
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 30, 2026

@copilot review but do not make fixes

@arul28 arul28 force-pushed the ade-48-orchestration-reject-duplicate-task-ids-shadow-task-deadlocks-phase-completion branch from f005304 to fe7623d Compare May 30, 2026 07:32
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 30, 2026

@copilot review but do not make fixes

@arul28 arul28 force-pushed the ade-48-orchestration-reject-duplicate-task-ids-shadow-task-deadlocks-phase-completion branch from fe7623d to 29391cb Compare May 30, 2026 07:46
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 30, 2026

@copilot review but do not make fixes

@arul28 arul28 merged commit 1c030de into main May 30, 2026
28 of 29 checks passed
@arul28 arul28 deleted the ade-48-orchestration-reject-duplicate-task-ids-shadow-task-deadlocks-phase-completion branch May 30, 2026 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant