Repository navigation
fix(github): reject cached same-branch PRs with foreign heads - #32060
Conversation
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: rejecting cached same-branch pull requests whose heads are from foreign repositories. Description check The description explains the user impact, implementation, rationale, linked issue, verification results, visual impact, risks, and platform considerations. It does not reproduce every template heading… 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. Full details: Description check
-
Explanation
The description explains the user impact, implementation, rationale, linked issue, verification results, visual impact, risks, and platform considerations. It does not reproduce every template heading or checklist item, but the required substantive information is mostly present.
-
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
A stale origin URL after a repository rename or transfer made the foreign-head check reject the checkout's own same-repo PR and clear its cache. Treat a PR whose head matches its canonical base URL as local. Co-Authored-By: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
a2191403-70cb-4fe3-a1e8-5667c6eef380
📒 Files selected for processing: 2
src/main/github/client-pr-foreign-head-cache.test.tssrc/main/github/client/lookup/implicit-default-branch-pr.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
| if (headRepo && baseRepo && githubRepoIdentityKey(headRepo) === githubRepoIdentityKey(baseRepo)) { | ||
| return false |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '95,140p' src/main/github/github-api-repository.ts
sed -n '195,245p' src/main/github/client/lookup/branch-lookup-resolution.ts
sed -n '85,145p' src/main/github/client/lookup/pr-number-lookup.ts
sed -n '1,110p' src/main/github/client/lookup/implicit-default-branch-pr.ts
rg -n 'originHeadRepo|trackedUpstream|lookupPRByNumber|pre-rename|same-repository' src/main/github/client/lookup src/main/github/client-pr-foreign-head-cache.test.tsRepository: stablyai/orca
Length of output: 14428
🏁 Script executed:
sed -n '1,245p' src/main/github/client/lookup/branch-lookup-resolution.ts
printf '\n--- candidate resolver declaration and callers ---\n'
rg -n -F --glob '*.ts' -e 'resolveGitHubApiRepositoryCandidates' -e 'createImplicitDefaultBranchPRPolicy' -e 'originHeadRepo:' -e 'trackedUpstream =' src/main/github
printf '\n--- relevant tests ---\n'
sed -n '130,250p' src/main/github/client-pr-foreign-head-cache.test.ts
printf '\n--- policy tests and references ---\n'
rg -n -F --glob '*.{ts,tsx}' -e 'isForeignSameNamedHead' -e 'same-repository' -e 'fallbackPRNumber' -e 'upstream' src/main/github/client src/main/github/*test.tsRepository: stablyai/orca
Length of output: 41457
🏁 Script executed:
sed -n '1,225p' src/main/github/client-pr-foreign-head-cache.test.ts
printf '\n--- test fixture and lookup mocks ---\n'
rg -n -F --glob 'src/main/github/client-pr-foreign-head-cache.test.ts' -e 'function primeLookup' -e 'const upstream' -e 'const origin' -e 'const foreign' -e 'trackedFork' -e 'checkout:' -e 'head:' -e 'expected:'
printf '\n--- policy data contracts ---\n'
sed -n '120,180p' src/main/github/client/lookup/pull-request-lookup-data.ts
sed -n '1,125p' src/main/github/client/lookup/tracked-upstream-cache.tsRepository: stablyai/orca
Length of output: 14767
Restrict the same-repository exception to trusted candidate repositories.
lookupPRByNumber checks every candidate from resolveGitHubApiRepositoryCandidates. The current exception accepts a candidate when the PR head repository matches the repository in the PR URL. It does not require that candidate to match originHeadRepo or trackedUpstream.headRepo.
An untracked upstream candidate can therefore pass the fallback lookup for a same-named PR. Reject that candidate. Compare repository identities so renamed origin URLs retain the existing behavior.
🐛 Suggested fix
--- "a/src/main/github/client/lookup/implicit-default-branch-pr.ts"
+++ "b/src/main/github/client/lookup/implicit-default-branch-pr.ts"
@@ -66,12 +66,20 @@
if (data.headRefName !== context.branchName || expectedHeads.length === 0) {
return false
}
const headRepo = pullRequestHeadRepository(data, prRepo)
const baseRepo = data.url ? ownerRepoFromPullRequestUrl(data.url) : null
+ const trustedPRRepo =
+ prRepo !== null &&
+ expectedHeads.some((head) => githubRepoIdentityKey(head) === githubRepoIdentityKey(prRepo))
// Why: a same-repo PR lives in a checkout remote; its canonical URL survives stale renamed-remote URLs.
- if (headRepo && baseRepo && githubRepoIdentityKey(headRepo) === githubRepoIdentityKey(baseRepo)) {
+ if (
+ trustedPRRepo &&
+ headRepo &&
+ baseRepo &&
+ githubRepoIdentityKey(headRepo) === githubRepoIdentityKey(baseRepo)
+ ) {
return false
}
return expectedHeads.every((head) => !pullRequestMatchesHeadRepository(data, prRepo, head))
}
ELI5
If you open a worktree on
mainin a public repo, Orca could show somebody else's pull request on it: an outside contributor's PR opened from their fork'smainbranch. You'd see their failing checks, conflicts and review comments as if they were yours. Deleting the cache didn't help, because Orca remembers the PR number and fetches that PR straight back by number.Orca's normal branch search already rejects PRs from other forks. The "fetch by remembered number" path skipped that check. Now it runs the same check: a PR whose branch name matches yours only sticks if it comes from your
origin, from the fork your branch tracks, or from the base repository itself (a same-repo PR). If it's rejected, Orca also clears that remembered PR, so it doesn't come back on the next refresh.What stays the same:
originURL likezeit/next.jsthat now redirects tovercel/next.js) still show their own same-repo PR. The first version of this PR broke that case; it's fixed in 2b626a6.Related: #27811 (this fixes the cached-number path, which is the "it comes back after refresh" part of that report). Prior contributor attempt: #29330 by @AmoabaKelvin (closed, a broader rewrite of push-head resolution across providers). It proposed the same rule for remembered PR numbers; no code was taken from it.
Verification
Live against real GitHub, calling the real
getPRForBranchOutcomewith realghand real scratch git repos (no mocks):mainstablyai/orca, branchmain, remembered #28356 (fromRandomPixelStudios's forkmain)no-pr, rejected URL.../pull/28356RandomPixelStudios/orcaupdates+ upstreamstablyai/orca, branchmain, #28356 (the contributor's own view)stablyai/orca, branch of this PR, #32060 (your own same-repo PR)zeit/next.jsURL, branchfleet/prune-stale-dev-types, #100612 (same-repo PR in renamedvercel/next.js)no-prand cleared the cache; fixed in 2b626a6)Unit tests:
src/main/github/client-pr-foreign-head-cache.test.tshas 11 cases. The foreign-fork, same-owner-other-repo and branch-result-revival cases fail onmain, and the renamed-remote case fails on the original commit. Together with the GitHub suites (src/main/github,src/renderer/src/store/github), 113 files / 1340 tests pass.check:code-quality:changed, oxlint and oxfmt pass.Visual proof: N/A. No layout change; the effect is that the foreign PR stops showing, which the live table above covers.
Risks / tradeoffs
originURL is stale. Linking the PR explicitly brings it back.