Skip to content

fix(server): read_pull_request_guide reads no-checkout hosts through the caller's project - #35913

Open
juliusmarminge wants to merge 2 commits into
mainfrom
factory/issue-35891
Open

juliusmarminge wants to merge 2 commits into
mainfrom
factory/issue-35891

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Nov 13, 2026 •

Copy link
Copy Markdown
Member

read_pull_request_guide threw PullRequestProjectNotFoundError whenever projectForPullRequest found no project whose repository identity matched the PR, before calling PullRequestReviewGuideService.get. PullRequestService's readsPublicHostWithoutCheckout fallback (GitHub, Bitbucket, Forgejo), which the app's PR panel already uses, was therefore never reached. locateAll now also returns the caller's project, and read_pull_request_guide falls back to it when no checkout matches, letting PullRequestService decide whether the host can be read without a checkout. When it cannot (PullRequestUnavailableError with reason provider-unsupported, and no matching project), the handler still returns PullRequestProjectNotFoundError with the existing message, so hosts that need a checkout behave as before. link_pull_request was not affected and is unchanged. No new regex or parser. I did not reproduce the bug myself and relied on the repro branch's failing test.

Fixes #35891

Reproduction on main

On origin/main 7fb6098: started T3CODE_DEV_SEED=0 vp run dev (fresh database), added one project via node src/bin.ts project add whose checkout's origin is github.com/pingdotgg/t3code, then from a scratch driver issued a short-lived MCP client session (as t3 thread does) and called read_pull_request_guide over /mcp. With url #35755 (the project's own repo) it succeeded and returned currentHeadSha. With url Effect-TS/effect#5000 (public, no project) it returned isError with: "No project in this environment is a checkout of effect-ts/effect, so its pull requests cannot be read here. Add one with t3_project_create." GitHub sets readsPublicHostWithoutCheckout like Bitbucket, so this is the same code path the reporter hit; no Bitbucket account is available on the VM. link_pull_request does not go through projectForPullRequest and links a URL from any repository (an existing handler test covers that), so only reading the guide fails. Driver: https://gh-file-drop-api-prod-mi5fy3sowv63ufte.pinglabs.workers.dev/f/e7dfd36e33a6f11d/driver.ts Transcript: https://gh-file-drop-api-prod-mi5fy3sowv63ufte.pinglabs.workers.dev/f/b338a5798c396439/run-main.txt

Evidence

Call read_pull_request_guide from an actual replay-agent thread on fresh real servers. Each environment contains only a T3 Code checkout. Read the own-repository control, then Effect PR #5000, without adding an Effect checkout.

Before After
7fb6098cfd 84f0ab93cf
Captured MCP transcript: isError:true, “No project in this environment is a checkout of effect-ts/effect…” Captured MCP transcript: isError:false, currentHeadSha:46f78576843b3ee8d706337d8c4fe23ccf8db516

The own-repository control succeeds on both commits. MCP driver, transparent replay-agent relay, reproduction instructions.

Not shown:

  • Permanent blocker: direct Bitbucket verification requires a Bitbucket account unavailable on this VM. The supplied GitHub reproduction demonstrates the same no-checkout path.
  • Forgejo-specific reads and unsupported-host error mapping were not exercised; this proof covers the supplied GitHub reproduction.

Tests

vp test run src/mcp/toolkits/pullRequests/handlers.test.ts (apps/server): 20/20 pass, including the repro test from factory/repro-35891. vp run check:changed: format, lint, knip, graphql, typecheck (3 packages), tests (2 files, 22 tests) all pass.

Not in this change

  • write_pull_request_guides still refuses a PR with no matching project; generating a guide needs a checkout to fetch the head, so that refusal is intended and was left alone.

Who did the work

  • Written by Claude Opus 5.5, in Claude Code
  • Reviewed by GPT-6.1 Sol, in Codex, and by Claude Opus 5.5, each on its own
  • Proof captured by GPT-6.1 Sol and checked by Claude Opus 5.5

Run by the t3code factory in T3 Code.

juliusmarminge and others added 2 commits November 13, 2026 03:31
…ough the caller's project

Fails on main: the handler refuses with PullRequestProjectNotFoundError before
PullRequestService's readsPublicHostWithoutCheckout fallback can read it (#35891).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the caller's project

The tool refused with PullRequestProjectNotFoundError whenever no project
was a checkout of the pull request's exact repository, so PullRequestService
never reached its readsPublicHostWithoutCheckout path (GitHub, Bitbucket,
Forgejo). Fall back to the calling thread's project and let the service
decide; a host it cannot read that way still reports the missing project.

Fixes #35891

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted Author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.5 KiB 21.5 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 1 1 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 1 1 0 (0.0%) 8 ✅

Other scenarios

All 64 scenario measurements match main.

All 64 scenario measurements
Provider Scenario Metric Main baseline This PR Impact PR ceiling
Codex short turn resume WebSocket decoded 10.5 KiB 10.5 KiB 0 B (0.0%) 12.7 KiB ✅
Codex short turn resume WebSocket messages 2 2 0 (0.0%) 2 ✅
Codex long turn running, cold open HTTP decoded 281.7 KiB 281.7 KiB 0 B (0.0%) 302.7 KiB ✅
Codex long turn running, cold open rows 75 75 0 (0.0%) 75 ✅
Codex long turn running, history page HTTP decoded 92.4 KiB 92.4 KiB 0 B (0.0%) 107.4 KiB ✅
Codex long turn running, history page rows 75 75 0 (0.0%) 75 ✅
Codex long turn finished, cold open HTTP decoded 381.5 KiB 381.5 KiB 0 B (0.0%) 410.2 KiB ✅
Codex long turn finished, cold open rows 151 151 0 (0.0%) 151 ✅
Codex long turn finished, socket cold open WebSocket decoded 381.6 KiB 381.6 KiB 0 B (0.0%) 410.2 KiB ✅
Codex long turn finished, socket cold open WebSocket messages 2 2 0 (0.0%) 2 ✅
Codex long turn finished, socket cold open rows 151 151 0 (0.0%) 151 ✅
Codex long turn finished, work page HTTP decoded 186.2 KiB 186.2 KiB 0 B (0.0%) 200.2 KiB ✅
Codex long turn finished, work page rows 150 150 0 (0.0%) 150 ✅
Codex idle thread resume WebSocket decoded 65 B 65 B 0 B (0.0%) 200 B ✅
Codex idle thread resume WebSocket messages 1 1 0 (0.0%) 1 ✅
Codex fleet shell snapshot HTTP decoded 42.4 KiB 42.4 KiB 0 B (0.0%) 53.7 KiB ✅
Codex fleet shell snapshot rows 27 27 0 (0.0%) 27 ✅
Codex fleet shell subscribe WebSocket decoded 65 B 65 B 0 B (0.0%) 200 B ✅
Codex fleet shell subscribe WebSocket messages 1 1 0 (0.0%) 1 ✅
Codex fleet shell resume check HTTP decoded 18 B 18 B 0 B (0.0%) 100 B ✅
Codex fleet shell resume check rows 0 0 0 0 ✅
Codex stale shell reload HTTP decoded 42.4 KiB 42.4 KiB 0 B (0.0%) 53.7 KiB ✅
Codex stale shell reload rows 27 27 0 (0.0%) 27 ✅
Codex stale shell resume WebSocket decoded 140 B 140 B 0 B (0.0%) 400 B ✅
Codex stale shell resume WebSocket messages 2 2 0 (0.0%) 2 ✅
Codex stale todo list cold open HTTP decoded 72.4 KiB 72.4 KiB 0 B (0.0%) 83.0 KiB ✅
Codex stale todo list cold open rows 75 75 0 (0.0%) 75 ✅
Codex stale todo list cold open nodes 2 2 0 (0.0%) 4 ✅
Codex stale todo list cold open subagents 2 2 0 (0.0%) 4 ✅
Codex stale todo list cold open plans 1 1 0 (0.0%) 2 ✅
Codex stale todo list cold open providerTurns 1 1 0 (0.0%) 2 ✅
Codex stale todo list cold open checkpoints 1 1 0 (0.0%) 2 ✅
Claude short turn resume WebSocket decoded 10.6 KiB 10.6 KiB 0 B (0.0%) 12.7 KiB ✅
Claude short turn resume WebSocket messages 2 2 0 (0.0%) 2 ✅
Claude long turn running, cold open HTTP decoded 283.3 KiB 283.3 KiB 0 B (0.0%) 302.7 KiB ✅
Claude long turn running, cold open rows 75 75 0 (0.0%) 75 ✅
Claude long turn running, history page HTTP decoded 93.4 KiB 93.4 KiB 0 B (0.0%) 107.4 KiB ✅
Claude long turn running, history page rows 75 75 0 (0.0%) 75 ✅
Claude long turn finished, cold open HTTP decoded 384.2 KiB 384.2 KiB 0 B (0.0%) 410.2 KiB ✅
Claude long turn finished, cold open rows 151 151 0 (0.0%) 151 ✅
Claude long turn finished, socket cold open WebSocket decoded 384.3 KiB 384.3 KiB 0 B (0.0%) 410.2 KiB ✅
Claude long turn finished, socket cold open WebSocket messages 2 2 0 (0.0%) 2 ✅
Claude long turn finished, socket cold open rows 151 151 0 (0.0%) 151 ✅
Claude long turn finished, work page HTTP decoded 188.1 KiB 188.1 KiB 0 B (0.0%) 200.2 KiB ✅
Claude long turn finished, work page rows 150 150 0 (0.0%) 150 ✅
Claude idle thread resume WebSocket decoded 66 B 66 B 0 B (0.0%) 200 B ✅
Claude idle thread resume WebSocket messages 1 1 0 (0.0%) 1 ✅
Claude fleet shell snapshot HTTP decoded 43.0 KiB 43.0 KiB 0 B (0.0%) 53.7 KiB ✅
Claude fleet shell snapshot rows 27 27 0 (0.0%) 27 ✅
Claude fleet shell subscribe WebSocket decoded 66 B 66 B 0 B (0.0%) 200 B ✅
Claude fleet shell subscribe WebSocket messages 1 1 0 (0.0%) 1 ✅
Claude fleet shell resume check HTTP decoded 18 B 18 B 0 B (0.0%) 100 B ✅
Claude fleet shell resume check rows 0 0 0 0 ✅
Claude stale shell reload HTTP decoded 43.0 KiB 43.0 KiB 0 B (0.0%) 53.7 KiB ✅
Claude stale shell reload rows 27 27 0 (0.0%) 27 ✅
Claude stale shell resume WebSocket decoded 140 B 140 B 0 B (0.0%) 400 B ✅
Claude stale shell resume WebSocket messages 2 2 0 (0.0%) 2 ✅
Claude stale todo list cold open HTTP decoded 73.4 KiB 73.4 KiB 0 B (0.0%) 83.0 KiB ✅
Claude stale todo list cold open rows 75 75 0 (0.0%) 75 ✅
Claude stale todo list cold open nodes 2 2 0 (0.0%) 4 ✅
Claude stale todo list cold open subagents 2 2 0 (0.0%) 4 ✅
Claude stale todo list cold open plans 1 1 0 (0.0%) 2 ✅
Claude stale todo list cold open providerTurns 1 1 0 (0.0%) 2 ✅
Claude stale todo list cold open checkpoints 1 1 0 (0.0%) 2 ✅

Baseline: be03b93 · PR result: 84f0ab9 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The pull request guide handler now uses the caller’s project when no project matches the pull request repository. The handler maps an unsupported provider error in this fallback path to PullRequestProjectNotFoundError. A test covers reading a Bitbucket pull request without a repository checkout.

Changes: Bitbucket pull request guide fallback

Layer / File(s) Summary
Pass caller project through guide lookup
apps/server/src/mcp/toolkits/pullRequests/handlers.ts, apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts
locateAll carries the caller’s project into its results. read_pull_request_guide uses that project when no project matches the target. When the fallback project’s provider is unsupported, the handler returns PullRequestProjectNotFoundError. The test checks the returned pull request identity, null guide, and head SHA.

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

Change: Bug fix · Severity of issue fixed: Low

Suggested labels: bugfix

Suggested reviewers: maria-rcks, t3dotgg, macroscopeapp

Merge Risk: Minimal · up to 84f0a

Reading a pull request guide for a repository with no checkout now falls back to the caller's project, which fixes the reported Bitbucket read case. No merge-blocking risk was found. Linking pull requests without a checkout is not covered by this change.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the fix and its scope: read_pull_request_guide reading no-checkout hosts through the caller's project. The changeset adds a caller-project fallback in read_pull_request_guide and a tes…
Linked Issues check Passed [#35891] Read path: the reviewed head now falls back to the caller's project in read_pull_request_guide (handlers.ts). locateAll returns callerProject per entry, and the guide read runs with `entr…
Out of Scope Changes check Passed All changes are in apps/server/src/mcp/toolkits/pullRequests/handlers.ts and its handlers.test.ts. The locateAll change adds callerProject to each entry and the read handler uses it only as a fall…
Description check Passed The description covers the required template areas in substance. Problem: read_pull_request_guide threw PullRequestProjectNotFoundError before PullRequestReviewGuideService.get ran, so the public-host…
Full details: Title check

Explanation

The title names the fix and its scope: read_pull_request_guide reading no-checkout hosts through the caller's project. The changeset adds a caller-project fallback in read_pull_request_guide and a test for it, so the title matches the main change. It is long at 89 characters, but it is specific and conventional.

Full details: Linked Issues check

Explanation

[#35891] Read path: the reviewed head now falls back to the caller's project in read_pull_request_guide (handlers.ts). locateAll returns callerProject per entry, and the guide read runs with entry?.project ?? entry?.callerProject. A PR with no matching project therefore reaches PullRequestReviewGuideService.get and the readsPublicHostWithoutCheckout logic in PullRequestService. The added handler test covers reading a Bitbucket PR for a repository with no checkout and asserts host, repository, number, null guide, and current head SHA. Link path: link_pull_request resolves its target with the caller's project (projectOf(thread)) and does not call projectForPullRequest, so the link behavior the issue requests is already present at the base and is unchanged. Not verified: the PullRequestService fallback itself was not re-read in this assessment; the read fix depends on it. Non-coding items (manual before/after evidence capture) are excluded. The PR objectives text says linking is not included, but the code at head shows linking does not hit the reported failure, so no linking work is missing.

Full details: Out of Scope Changes check

Explanation

All changes are in apps/server/src/mcp/toolkits/pullRequests/handlers.ts and its handlers.test.ts. The locateAll change adds callerProject to each entry and the read handler uses it only as a fallback. The error mapping converts only PullRequestUnavailableError with reason provider-unsupported, and only when no matching project exists, into the existing PullRequestProjectNotFoundError. Other errors keep PullRequestGuideFailedError. The test supports the read fix. No unrelated changes were identified in the diff reviewed. The diff was not read in full beyond the handlers.ts hunks and the test summary, so this conclusion is limited to those sources.

Full details: Description check

Explanation

The description covers the required template areas in substance. Problem: read_pull_request_guide threw PullRequestProjectNotFoundError before PullRequestReviewGuideService.get ran, so the public-host no-checkout fallback in PullRequestService was never reached. Change: locateAll also returns the caller's project, and the handler falls back to it. The provider-unsupported case still returns the existing error, and link_pull_request is unchanged. Scope: it links a bug (issue #35891) and gives a reason for the fix. Verification: it reports a reproduction on main with before/after transcripts for a GitHub public PR, 20/20 handler tests passing, and check:changed passing. It also states what was not checked (direct Bitbucket verification and Forgejo reads). The 'Who did the work' section names the model and harness, as the template asks. Two items need attention: the Evidence table's 'after' column cites a commit hash and transcript links, which the reviewer should confirm are reachable. The 'Before' and 'After' transcript links point to external file-drop hosts that a reviewer may not be able to verify. The description also says 'The before-and-after evidence section is still marked as being captured' in the PR objectives, which conflicts with the populated table. Overall the description is mostly complete and specific.

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between f39d38b and 84f0ab9.

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. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.

@macroscopeapp

macroscopeapp Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 84f0ab9

Macroscope's review found this PR approvable — Guide reads can now use the caller's project for hosts that already support reading without a checkout. Existing provider restrictions remain in place, and guide generation is unchanged.

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

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

size:S 10-29 changed lines (additions + deletions). vouch:trusted Author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Agent PR tools refuse Bitbucket PRs the server can read without a checkout

1 participant