feat(mcp): handle task change links

[yattdev]

Task MCP can now link, unlink, and replace GitHub pull request or GitLab merge request associations using provider-qualified repository identity, so agents can repair stale task links without conflating fork numbers.

Important Changes

• Route the three task MCP mutations through one provider-neutral coordinator with same-workspace task reach checks.
• Return the resulting active GitHub/GitLab association set and document the historical-receipt rule.

Validation

• go test ./internal/backendapp ./internal/mcp/handlers ./internal/mcp/server
• node scripts/validate-public-docs.mjs
• Commit hooks: Go lint, formatting, and Conventional Commits validation.

Checklist

• I have performed a self-review of my code.
• I have manually tested my changes and they work as expected.
• My changes have tests that cover the new functionality and edge cases.
• If my change touches UI files (apps/web/), I have added or updated Playwright e2e tests in apps/web/e2e/ and verified them with make test-e2e.
• I checked whether this affects public docs in docs/public/** and updated them or noted why no docs change is needed.

Preview Environment

URL	https://kandev-pr-3506-bwo7.sprites.app
Commit	974a919
Agent	Mock agent

Updates automatically on each push. Destroyed when the PR is closed.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

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.}

[github-actions]

Claude finished @yattdev's task in 5m 44s —— View job

---

Findings

Blocker (must fix before merge)

1. Missing coordinator business-logic tests — apps/backend/internal/backendapp/task_change_link_coordinator_test.go:20

   • Issue: The test file only covers githubChangeURL / gitlabChangeURL. The coordinator's link, unlink, list, ReplaceTaskChange, and the workspace reach check in taskRepository have zero test coverage.
   • Why: AGENTS.md is explicit: "Missing tests for new or changed logic is a blocker." The workspace check, provider dispatch, and partial-failure behaviour in ReplaceTaskChange all contain material logic.
   • Fix: Add unit tests using stub implementations of github.Service / gitlab.Service. Cover: workspace mismatch rejection, per-provider dispatch, the link-succeeds-unlink-fails partial failure, and list aggregating both providers.

2. Missing handler request-logic tests — apps/backend/internal/mcp/handlers/task_change_link_test.go:20

   • Issue: TestValidateTaskChangeLinkRequiresProviderRepositoryAndNumber only exercises the pure validation helper. taskChangeLinkRequest (workspace isolation check, replace-mode old-link parsing) is untested.
   • Why: Same blocker rule. The workspace check path (ErrorCodeForbidden) and the replace=true old-link parsing are non-trivial paths with real bugs if broken.
   • Fix: Add table-driven tests for the handler using a stub TaskService: cross-workspace caller → Forbidden, absent task → NotFound, valid replace round-trip, absent caller_task_id behaviour.

Suggestion (recommended, doesn't block)

1. ReplaceTaskChange partial-failure contract is undocumented — apps/backend/internal/backendapp/task_change_link_coordinator.go:56

   • If link succeeds but unlink fails, the task holds both old and new associations while the caller receives an error. A retry should converge, but the interim state and the retry contract are undocumented. Add a comment explaining the partial-failure contract (inline comment left on the diff).

2. githubChangeURL host check accepts crafted substrings — apps/backend/internal/backendapp/task_change_link_coordinator.go:188

   • strings.Contains(host, "github.com") passes for github.com.evil.com. Because the generated URL is always hardcoded to github.com this cannot corrupt output, but the check is misleading. Use strings.HasSuffix or exact matching (inline suggestion left on the diff).

3. Silent no-op when PR to unlink is not found — apps/backend/internal/backendapp/task_change_link_coordinator.go:109

   • When (repository_id, number) matches nothing in the stored PRs, unlink returns nil with no log. A debug-level log would help diagnose stale-link repair failures without changing the no-op semantics.

4. Workspace isolation check is conditional on caller_task_id being present — apps/backend/internal/mcp/handlers/task_change_link.go:110

   • When caller_task_id is absent the workspace check is skipped. In task (kanban) mode the server always injects s.taskID, so the MCP path is safe. But the WS action is registered unconditionally; a raw WS message can omit the field. With auth enabled the service-layer GetTask is the real guard, so this is defence-in-depth — but the PR description promises "same-workspace task reach checks" unconditionally. Consider requiring caller_task_id to close the raw-WS gap (inline suggestion left on the diff).

---

Summary

Severity	Count
Blocker	2
Suggestion	4

Verdict: Blocked — fix blockers first.

The feature design (provider-neutral coordinator, canonical (repository_id, number) identity, same-workspace reach checks) is sound and the GitHub sync context fix bundled here is correct. The two blockers are both about test coverage: the coordinator's core linking logic and the handler's workspace isolation path are both untested, which doesn't meet the repo's stated quality bar.