[MarcoDotIO]

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

[macroscopeapp]

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

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: b50d8a1
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1ccaa694-95d0-415b-937b-b1f4083d517e

Commits

Reviewing files that changed from the base of the PR and between 40e4849 and 2329328.

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; 4 remain after this review.

Walkthrough

The unlink handler 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.

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

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to 23293

The dismissed-link unlink behavior is correctly preserved without introducing actionable merge risk.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely identifies the main fix: correcting unlink_pull_request behavior for dismissed stack PRs.
Description check		The description includes all required sections. It explains the problem, change, scope rationale, focused verification, observed results, and limitations.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 40e48497d2cc047dde7382c2a84473bef332b6a8 on this linux boat vm. reproduced the bug first on github/main at 305a90dce44b8b7813d951662f13ce8510a3c669 in a separate worktree. installed each checkout with vp i and ran one background dev server at a time with isolated state.

used playwright from the repo with /usr/bin/google-chrome. created real repository projects and threads through the web ui, selected codex gpt-6.1-sol, and sent real turns through the local proxy. established the target pr link with source: "stack" through the public websocket orchestration.dispatchCommand api, then clicked "dismiss from thread" in the app. no mocks, database writes, fixtures, demo data, or changes to another pr.

• main: the list omitted the dismissed link, but the agent's first unlink returned wasLinked: true. persisted event sequence 82 dispatched an mcp unlink and removed the tombstone. the second call returned false because the record was already gone.
• pr: the agent's two unlink calls both returned wasLinked: false. the stack-dismissed record remained persisted, and neither call dispatched an unlink command. checked the provider-native completed mcp results as well as the app display.
• nearby behavior: the agent restored the dismissed link (alreadyLinked: false), linked again (alreadyLinked: true), and listed its source as agent. ordinary unlink then returned true once, false on repeat, and left the list empty.

vp test run apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts passed all 19 tests through blacksmith-check. the unchanged head has 28 successful ci checks, zero failing or pending. no code fix or push was needed.

                          main               pr
list after dismissal      []                 []
unlink first              wasLinked=true     wasLinked=false
unlink second             wasLinked=false    wasLinked=false
dismissed record          deleted            preserved
mcp unlink events         1                  0

short command and provider transcript

Not verified

automatic discovery or rediscovery from a real forge stack: this target pr has no stack, so the stack source was supplied through the public server api. the ui dismissal, persisted tombstone, provider calls, and relink/unlink behavior were exercised against the real running server.

native ios/android, electron, macos/windows, other providers, and relay/tunnel connections were not exercised on this linux vm. this pass used the direct web client and codex path.

merge readiness pass (2026-11-01)

merged current main into 232932862ca5608b59dbf5c7ec92481e9821269c without rebasing. resolved the test insertion conflict by retaining the dismissed-link regression and both upstream bulk-operation cases. the final feature diff remains two files, 19 additions and one deletion. pushed to the existing contributor branch without force.

blacksmith passed vp run check:changed against main, including format, lint, knip, graphql, affected-package typechecks and 23 tests. the explicit handler run passed all 22 tests. two independent read-only reviewers approved this head with no findings. no actionable bot findings were present. the earlier real-provider evidence above was not repeated in this merge-only pass. ci on the new head is pending.