Sign up
View stargazersSign up

fix(server): unlink_pull_request reports a dismissed stack PR as not linked#25562

open2
main
GitHub fork: fix/mcp-unlink-dismissed-pr
opened3d agobyMMarcoDotIO
2 files changed
View on GitHubTwo-way sync enabledPR last imported 3d ago

Problem

When the user removes a pull request from a stack, the thread keeps it as a hidden stack-dismissed record so stack sync does not add it back. The other MCP pull request tools already treat that record as not linked: list_thread_pull_requests hides it, link_pull_request links it again, and watch_pull_request ignores it. unlink_pull_request was the only one that counted it. An agent that unlinks such a PR got wasLinked: true for a PR its own list never showed, and every repeat call did the same. Each call also sent a real thread.pull-request.unlink command. That command bumps the thread's updatedAt, and when the dismissed record no longer belongs to a stack it deletes the record, so stack sync can add the PR back.

Repro: a thread has #9 with source: "stack-dismissed". Call unlink_pull_request { repository, number: 9 }.

  • Before: wasLinked: true, and an unlink command is dispatched.
  • After: wasLinked: false, and nothing is dispatched, as the tool description says ("Succeeds with wasLinked=false when it was not linked").

Change

The unlink_pull_request handler (apps/server/src/mcp/toolkits/pullRequests/handlers.ts) now skips stack-dismissed records when it checks for a link, the same check setWatching and link_pull_request already use. The PR can still be shown again with link_pull_request, which un-dismisses it.

Scope and approval

This is a very small fix for an obvious bug, so it has no prior issue. The tool contract says wasLinked=false when the PR is not linked. The sibling tools in the same file already treat stack-dismissed as not linked. The change is one condition in one handler plus one focused test. Nothing changes in the client, contracts or orchestrator.

Verification

  • node_modules/.bin/vp test run apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts
    • Without the fix (handler reset to upstream/main), the new test fails: × reports a dismissed stack member as not linked and leaves it dismissed, AssertionError: expected true to be false. Result: 1 failed, 18 passed.
    • With the fix: 19 passed.
  • vp lint --report-unused-disable-directives apps/server/src/mcp/toolkits/pullRequests: no findings. vp fmt --check: clean.
  • tsc --noEmit in apps/server: exit 0.
  • Not checked: a live agent session against a real stacked PR. The handler test runs the toolkit through McpToolAccess with a mocked orchestrator.

Claude Opus 5.5 (Claude Code desktop app)

🤖 Generated with Claude Code

Checks

19 successful · 9 skipped
All checks passed
40e4849

Added 1 commit

  • fix(server): unlink_pull_request reports a dismissed stack PR as not linkedbcd6989
MmacroscopeappBot3d ago
Synced from GitHub

Approvability

Verdict: Approved at bcd6989

Macroscope's review found this PR approvable — This is a narrowly scoped bug fix that makes unlink_pull_request consistently treat dismissed stack members as unlinked and avoids removing their tombstones. Existing link behavior is unchanged, and the new case is covered by a focused test.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot3d ago
Synced from GitHub

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between bcd69897fdb3b515693b105e387d9b98aa4ae99b and 40e48497d2cc047dde7382c2a84473bef332b6a8.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 210b2e7c6a8be11441b7afc94742ce3c76860111 and bcd69897fdb3b515693b105e387d9b98aa4ae99b.

External image Files selected for processing: 2
  • apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts
  • apps/server/src/mcp/toolkits/pullRequests/handlers.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

Walkthrough

The unlink handler now excludes links with source stack-dismissed when checking whether a pull request is linked. A test verifies that this case returns wasLinked: false and dispatches no command.

Changes: Dismissed Stack Link Unlink Handling

Layer / File(s)Summary
Exclude dismissed links from unlink checks
apps/server/src/mcp/toolkits/pullRequests/handlers.ts, apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts		The unlink check ignores stack-dismissed links. The test verifies the result is wasLinked: false and no command is recorded.

Priority: External image Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: External image Minimal · previous review bcd69

The focused change matches the intended dismissed-link behavior and has direct regression coverage.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main fix: dismissed stack pull requests are reported as not linked by unlink_pull_request.
Description checkPassedThe description includes all required sections. It explains the problem, expected behavior, implementation, scope rationale, focused verification, and unverified live-session testing.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Added 1 commit