fix(unclaim): make the resting status predictable for every status - #873
Conversation
unclaim previously only flipped status/in-progress to status/ready and left every other status untouched, so unclaiming a status/blocked issue left it unowned and still parked — contradicting the tool's documented "immediately re-claimable" contract. Now the route decides one behaviour per status: - in-progress → ready (as before) - blocked with a null blockedReason → ready (nothing recorded a reason to stay parked) - blocked with a blockedReason → unchanged, and the response says so (the block was a deliberate decision) - in-review → unchanged while the linked PR is still open (do not drag work backwards while a PR is in flight) The response body now reports the resulting status label plus a statusNote explaining any retention, so a caller can tell whether the issue is workable instead of inferring it from the labels. The MCP tool description in src/mcp/server.ts is updated so the "immediately re-claimable" claim is conditional, not absolute. Tests cover unclaim from in-progress, blocked with and without a reason, and in-review with an open linked PR, asserting both the resulting labels and the reported status. Fixes #869 Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com>
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — primary route
Recommendation: Approve
This PR correctly fixes the unclaim endpoint's status handling contract per issue PR 869. All acceptance criteria are met and tests pass.
Change-by-Change Findings
src/app/api/issues/unclaim/route.ts
The implementation correctly handles all four status cases per the issue's acceptance criteria:
in-progress→ready(preserves existing behavior)blockedwithnullblockedReason→ready(new — addresses the core bug)blockedwith ablockedReason→ unchanged, returnsstatusNoteexplaining whyin-reviewwith open linked PR → unchanged, checks PR state viafetchPullRequestState
The response now includes status (the resulting status label) and statusNote (context when status was retained), satisfying the requirement that callers can determine workability without inferring from labels.
src/app/api/issues/unclaim/route.test.ts
Four new test cases cover all status transitions, asserting both label outcomes and response fields. The tests correctly mock fetchPullRequestState and verify statusNote content.
src/mcp/server.ts
The tool description is updated to accurately reflect the conditional "immediately re-claimable" claim. It now describes all four status behaviors explicitly.
Must-Check: Route Access Controls
Verified. The route uses:
enforceRateLimitwith a 30 requests per 10 second limit (line ~11)- Bearer token authentication via
getAuthorizedActor(present in the imported auth module; auth middleware pattern is consistent with the rest of the API)
The auth model is unchanged from before — the PR does not modify access controls, which is appropriate since the issue scope is behavior-only.
Must-Check: Unintended Public Endpoints
Not applicable. This PR does not add any new routes. The changed endpoint (POST /api/issues/unclaim) existed prior to this PR. The MCP description change in src/mcp/server.ts is documentation only and does not alter runtime behavior.
Linked Issue Fit
This PR fully satisfies issue PR 869's acceptance criteria:
| Criterion | Status |
|---|---|
in-progress → ready |
✅ Preserved |
blocked with null blockedReason → ready |
✅ Implemented |
blocked with blockedReason → unchanged |
✅ Implemented, with statusNote |
in-review with open PR → unchanged |
✅ Implemented, checks fetchPullRequestState |
Response includes resulting status |
✅ Added status and statusNote fields |
| MCP description updated | ✅ Updated in src/mcp/server.ts |
| Tests for all cases | ✅ Four new test cases |
Standards Compliance
Per AGENTS.md conventions:
- API routes return appropriate HTTP status codes: ✅ Returns 200 for success, preserves error handling paths
- Validation before database operations: ✅ The
issueguard checks existence before proceeding - Error handling via
error instanceof Error: ✅ Pattern preserved in the catch block
CI Results
All checks passed (12/12): Lint, Typecheck, Build, Tests, Coverage, Docker Build, Docker Build (MCP), Database migrations, Database integration, npm audit, smoke — no blockers.
The unclaim endpoint now correctly handles all issue statuses (in-progress→ready, blocked→ready when no reason, in-review unchanged), adds corresponding tests, and updates the MCP server description to match.
Fixes #869
Opened by foreman on review GO (workload wl-misospace-dispatch-869).