fix(issue-claim): stop unclaim from overwriting GitHub labels from the stale local cache

[itsmiso-ai]

Fixes issue #921 by using targeted label removal in releaseIssueClaim to prevent overwriting GitHub labels with stale cache data.

Fixes #921

Opened by foreman on review GO (workload wl-misospace-dispatch-921).

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)

Recommendation

Approve. This PR directly fixes the bug described in issue PR 921 by replacing a full-set label write with targeted label primitives, preventing stale cache data from reverting GitHub-side label changes. The change is small, well-scoped, and accompanied by updated tests that explicitly assert the new (and old, now-banned) behavior.

Change-by-change findings

src/lib/issue-claim.ts

• Import updated: Drops updateIssueLabels from @/lib/github imports, keeping only removeIssueLabel. This is consistent with the new code path that no longer performs full-set writes.
• Comment-only change at the write site: The line await updateIssueLabels(repoFullName, issueNumber, updatedLabels); is removed, and the trailing defensive removeIssueLabel call remains. The new explanatory comment correctly documents why a full-set write is unsafe (cached row can be behind GitHub).
• Behavior change is consistent with the issue's suggested fix: The issue explicitly recommends "remove the agent label with removeIssueLabel, apply any status transition through its own targeted call, leave every other label untouched." Status transitions are already routed through transitionIssueStatus, which the diff notes uses add/remove primitives. The agent label removal is already a targeted removeIssueLabel call.
• No new variables introduced: The intermediate updatedLabels array computation is no longer needed for the GitHub write (though getAgentFromLabels(issue.labels) is still computed earlier to determine the agentLabel). No dead state is left behind that would cause typecheck or lint regressions.

src/app/api/issues/unclaim/route.test.ts

• Test assertions updated to match new behavior: Five test cases that previously asserted mocks.updateIssueLabels calls now assert targeted mocks.removeIssueLabel / mocks.addIssueLabel calls and expect(mocks.updateIssueLabels).not.toHaveBeenCalled(). This directly proves the fix and guards against regression.
• New regression test added: A dedicated test case "does not revert labels changed on GitHub since the last sync (stale cache)" explicitly simulates the scenario described in the issue — a stale cache row containing status/ready that has already been cleared on GitHub — and asserts that the unclaim flow does not perform a full-set write that would restore it.
• Failure-path test updated: The GitHub-API-failure test now mocks removeIssueLabel to reject, matching the new primary write path. This preserves coverage of the error-handling branch.
• File ends without a trailing newline: The diff shows \ No newline at end of file on the test file. This is a minor style issue but consistent with the prior state (the file was already missing a trailing newline before this change, and many other test files in the repo follow the same convention based on the diff hunk markers). Not a blocker.

Linked Issue Fit

Issue PR 921 describes the exact failure mode this PR fixes: releaseIssueClaim was using updateIssueLabels (full-set replacement) with a cached label set, causing unclaim to silently revert labels changed on GitHub since the last sync. The issue's suggested fix is:

1. Remove the agent label with removeIssueLabel ✅
2. Apply any status transition through its own targeted call ✅ (already routed through transitionIssueStatus)
3. Leave every other label untouched ✅

The issue also notes: "stale-work.ts also calls releaseIssueClaim, so it inherits the same behaviour and should be covered by whichever fix lands." The fix is at the source — inside releaseIssueClaim itself — so both call sites (src/app/api/issues/unclaim/route.ts and stale-work.ts) are covered by the single change. No separate fix is needed for the stale-work path.

The acceptance criteria (no full-set writes during unclaim; stale cache cannot revert GitHub-side label changes) are met by both the implementation and the new regression test.

CI Status

All CI checks (Coverage, Database integration, Database migrations, Lint, Build, Typecheck, Tests, Docker Build, smoke, Docker Build MCP, npm audit) are green. This provides authoritative evidence that the change typechecks, lints, and passes the full test suite including the new assertions.

Standards Compliance

The repository standards (AGENTS.md) state: "GitHub Issues and PRs remain the source of truth. Dispatch's Postgres is a cache; do not write back to it as if it were authoritative." This PR aligns with that principle — it stops Dispatch from overwriting GitHub's label set with cached data and instead uses targeted label primitives that respect GitHub as the source of truth.

Other applicable standards are satisfied:

• Error handling: The failure-path test confirms errors propagate correctly.
• API routes: No changes to route handlers; the fix is internal to the library.
• Validation: No new inputs; existing validation is untouched.

Summary

This is a correct, minimal, and well-tested fix for a real bug that had user-visible impact (issues getting parked due to silent label reverts). The implementation matches the issue's suggested fix, the tests cover both the happy path and the regression scenario, and CI is green.