Skip to content

fix(server): MCP thread list and read report every linked pull request - #17454

Open
ScottN-PV wants to merge 1 commit into
pingdotgg:mainfrom
ScottN-PV:fix/17103-thread-pull-requests
Open

ScottN-PV wants to merge 1 commit into
pingdotgg:mainfrom
ScottN-PV:fix/17103-thread-pull-requests

Conversation

@ScottN-PV

Copy link
Copy Markdown
Contributor

Problem

t3_thread_list and t3_thread_read report only the legacy linkedPullRequest. Links added through link_pull_request go to the thread's pullRequests and leave that field unchanged, so a thread linked to a PR in another repository, or in a project without a remote, reads as linkedPullRequest: null. An orchestrator that filters on it sees no link for a thread that is tracking PRs and may settle it. list_thread_pull_requests and the sidebar show the links, so the three disagree.

Change

Both results now include pullRequests: the thread's visible links (dismissed stack members excluded) as {host, repository, number, url, source, state}, with state null until the host has been read. It is the list list_thread_pull_requests returns, without snapshot details. The field is optional in the contract, as new wire fields are here, and the server always sets it. linkedPullRequest is unchanged; both tool descriptions now call it legacy and point to pullRequests.

Scope and approval

Closes #17103. The triage comment suggests this additive array as its first option. I took it over a per-thread count because six short fields per link stays small; a count on list items is a small follow-up if you prefer smaller pages. No change to linking, watches, host reads, or clients.

Verification

Linux, apps/server:

  • vp test run src/mcp/OrchestratorMcpToolkit.integration.test.ts: 2 passed. The test project has no remote. The added case links a PR in another repository and a dismissed stack member through the orchestrator, then decodes real t3_thread_read and t3_thread_list results: linkedPullRequest is null, pullRequests holds only the visible link, and a thread without links reports []. With the fix reverted the same run fails because pullRequests is missing.
  • Real run on a dev server, in a project with a remote: an agent linked Effect-TS/effect#5000. Before the fix, read and list returned linkedPullRequest: null and no pullRequests while list_thread_pull_requests listed the PR. After the fix both returned it in pullRequests with state: null (no GitHub credential on that server). After unlink_pull_request, both returned pullRequests: [].
  • vp test run src/mcp (268 passed), packages/contracts tests (643 passed), server and contracts typecheck, vp lint and vp fmt --check on the changed files: clean.

Model: Claude Opus 5.5, Claude Fable 5.1 (review), GPT-6-Astra (review). Harness: Claude Code, Codex.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 9, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 9, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at c4a3bdf

Macroscope's review found this PR approvable — The change adds an optional pullRequests array to existing MCP thread list/read responses, reusing established filtering and normalization without changing persistence or other runtime workflows. It is backward-compatible, narrowly scoped, and covered by integration tests.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

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

@coderabbitai

coderabbitai Bot commented Oct 9, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 54e0eaa3-f6a8-486f-bf0c-7761e3491ed2

📥 Commits

Reviewing files that changed from the base of the PR and between 101f8b2 and c4a3bdf.


📒 Files selected for processing (4)
  • apps/server/src/mcp/OrchestratorMcpService.ts
  • apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts
  • apps/server/src/mcp/toolkits/orchestrator/tools.ts
  • packages/contracts/src/orchestratorMcp.ts

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



📝 Walkthrough

Walkthrough

MCP thread list and detail responses now include an optional pullRequests array. The service filters visible links and maps their details. The existing linkedPullRequest field remains unchanged.

Changes

Thread pull-request responses

Layer / File(s) Summary
Pull-request response contract
packages/contracts/src/orchestratorMcp.ts
Defines the linked pull-request schema and adds an optional pullRequests array to thread list and detail contracts.
Thread link mapping and responses
apps/server/src/mcp/OrchestratorMcpService.ts, apps/server/src/mcp/toolkits/orchestrator/tools.ts, apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts
Filters and maps visible links into thread list and detail responses. Tool descriptions clarify how pullRequests relates to the legacy field. Integration tests check visible agent links, excluded stack-dismissed links, and parent threads without links.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge


Merge Risk

Merge Risk: ⚪ Minimal · up to c4a3b

The new pull-request list appears ready to merge after normal checks; no actionable risk remains identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c4a3b

The change exposes stored link metadata without adding repository access, changing stored links, or granting mutation authority. The remaining question is whether thread-reading access and pull-request metadata access are intentionally separable permissions.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The new output covers visible links on threads returned by the existing project-list and environment-level live-thread read paths. Linked repositories need not match the thread's project repository. The returned metadata does not itself grant access to those repositories.

Trust Boundaries and Controls

  • observed — Thread list/read require orchestration capability through loadCaller. The dedicated pull-request list requires pull-requests capability. Consequently, the new response projection has a different capability gate; whether supported credentials can receive orchestration without pull-requests remains unresolved.



🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly describes the main change: MCP thread list and read results now expose linked pull requests. It is slightly broad because dismissed stack members remain excluded, but it is still dir…
Description check Passed The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the behavior, linked issue, implementation scope, focused integration coverage, manual ver…
Linked Issues check Passed Issue #17103 requires consistent PR-link visibility in t3_thread_list, and notes the same defect in t3_thread_read. The server now maps visible thread links into pullRequests for both results. T…
Out of Scope Changes check Passed The reviewed changes are limited to the server response mapping, the MCP contract, tool descriptions, and integration coverage for issue #17103. The existing linkedPullRequest field remains unchange…


✨ Finishing Touches
🧪 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.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 11, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 11, 2026 07:00

Dismissing prior approval to re-evaluate c4a3bdf

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: t3_thread_list reports linkedPullRequest: null for threads whose PRs are in another repo (or project has no git remote)

2 participants