ADE-58: Orchestration claimTask: reject claims against terminal (done/failed) tasks

[arul28]

Refs ADE-58

Summary

Describe the change.

What Changed

Key files and behaviors.

Validation

How you tested.

Risks

Anything to watch.

Linked Linear issues

• ADE-58: Orchestration claimTask: reject claims against terminal (done/failed) tasks - referenced

  Open in ADE  ·  ade-58-orchestration-claimtask-reject-claims-against-terminal-done-failed-tasks branch  ·  PR #420

Summary by CodeRabbit

• Bug Fixes
  • Prevented claiming of tasks that are already in terminal states (done or failed). The system now rejects claim attempts on completed or failed tasks, ensuring task state integrity.

Greptile Summary

This PR adds a guard in claimTask to reject claim attempts on tasks that are already in a terminal state (done or failed), preventing re-assignment of completed work. A comprehensive parameterized test covers both terminal states and verifies rejection from both the original assignee and other workers.

• orchestrationService.ts: New guard inserted after the task-not-found check; returns ok: false with a descriptive reason string (\"task X is terminal (done|failed) and cannot be claimed\") before any lease or assignee mutation is attempted.
• orchestrationService.test.ts: it.each test seeds a task, transitions it to the target terminal state via releaseTask, then verifies two separate claimTask calls are rejected with the expected reason and that the task's status and claimLeaseUntil remain unchanged.

Confidence Score: 5/5

Safe to merge — the guard is minimal, correctly placed, and well-tested.

The change is a small, focused early-return that prevents state mutation on terminal tasks. It does not alter any existing code paths and is covered by a thorough parameterized test for both terminal states and both claimant types.

No files require special attention.

Important Files Changed

Filename	Overview
apps/desktop/src/main/services/orchestration/orchestrationService.ts	Adds a terminal-state guard (done/failed) in claimTask before the lease-conflict check; logic is correct and well-placed.
apps/desktop/src/main/services/orchestration/orchestrationService.test.ts	Parameterized test covers both terminal states and both same-worker and cross-worker claim attempts; releaseTask result is not guarded with ok check before proceeding.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[claimTask request] --> B{Session registered?}
    B -- No --> C[return ok:false\n'session not registered']
    B -- Yes --> D{Task exists?}
    D -- No --> E[return ok:false\n'task not found']
    D -- Yes --> F{Task status is\ndone or failed?}
    F -- Yes --> G[return ok:false\n'task is terminal']
    F -- No --> H{Claimed by another\nsession with active lease?}
    H -- Yes --> I[return ok:false\n'already claimed by X']
    H -- No --> J[Apply claim patches\nstatus=claimed, assignee, lease]
    J --> K[return ok:true]

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/desktop/src/main/services/orchestration/orchestrationService.test.ts:1175-1181
**Missing `ok` guard on `releaseTask` result**

`releaseTask` is called without checking whether it succeeded before asserting on its manifest. If `releaseTask` returns `ok: false` for any reason, the task remains `claimed`, the `expect(releasedTask?.status).toBe(terminalStatus)` assertion fires a confusing mismatch, and subsequent `claimTask` calls may succeed (for `S-worker`, which still holds the lease) — producing a misleading failure rather than a fast, clear one. The seeding step above uses the same guard pattern (`if (!seeded.ok) throw new Error(...)`) and the same should be applied here.

_{Reviews (2): Last reviewed commit: "Fix terminal orchestration task claims" | Re-trigger Greptile}

[linear-code]

ADE-58

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
ade	Ignored	Preview	May 30, 2026 2:07am

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds a guard to prevent claiming tasks in terminal statuses. The claimTask service method now rejects claim attempts when a task is already marked done or failed, returning a structured failure response. A new parameterized test validates this behavior for both terminal statuses across multiple claim scenarios.

Changes

Terminal task claim prevention

Layer / File(s)	Summary
Terminal task claim guard and validation apps/desktop/src/main/services/orchestration/orchestrationService.ts, apps/desktop/src/main/services/orchestration/orchestrationService.test.ts	Added early guard in claimTask to reject claim attempts for tasks with done or failed status. Parameterized test validates that both terminal statuses prevent claims from the original assignee and other workers, while preserving status and ensuring lease remains cleared.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• Orchestration claimTask: reject claims against terminal (done/failed) tasks #413: Both changes address the same guard implementation in claimTask to reject claims against terminal-status tasks (done/failed).

Suggested labels

desktop

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: preventing claim operations on tasks in terminal states (done/failed).
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ade-58-orchestration-claimtask-reject-claims-against-terminal-done-failed-tasks

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[arul28]

@copilot review but do not make fixes

[capy-ai]

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.

[cursor]

PR Review

Scope: 2 file(s), +129 / −0
Verdict: Looks good

This PR adds an early guard in claimTask so tasks in terminal done or failed status cannot be re-claimed, and adds parametrized service tests that release a task to each terminal state then assert both the original assignee and another worker get a stable rejection with an unchanged manifest.

---

Notes

• The guard is placed in the right spot: after task/agent lookup and before lease/assignee logic, so an expired claimLeaseUntil on a terminal task can no longer resurrect work through claimTask.
• Tests cover both done and failed, same-session vs other-worker claims, and assert manifest fields (status, claimLeaseUntil) stay unchanged on failure — good regression coverage for the reported bug.
• Workers are steered to claimTask in tool prompts; this closes the primary mutation path. manifestPatch still allows an owning worker to replace /tasks/{id}/status without a terminal check — that looks pre-existing and out of this PR’s stated scope, but worth a follow-up if you want terminal immutability everywhere.
• No renderer/IPC contract changes; trust boundary remains local desktop orchestration over lane worktrees.

  

_{Sent by Cursor Automation: BUGBOT in Versic}

[arul28]

@copilot review but do not make fixes