Skip to content

fix(server): detect existing PR when remote URL uses a renamed repository slug - #20027

Closed
jorotodorovv wants to merge 2 commits into
pingdotgg:mainfrom
jorotodorovv:fix/git-pr-lookup-renamed-repository
Closed

jorotodorovv wants to merge 2 commits into
pingdotgg:mainfrom
jorotodorovv:fix/git-pr-lookup-renamed-repository

Conversation

@jorotodorovv

Copy link
Copy Markdown

Problem

When a repository has been renamed (or transferred under the same owner), a checkout's origin URL often keeps the old slug (git@github.com:octo/old-name.git). GitHub redirects git traffic, so push/fetch keep working, but the GitHub API reports the canonical name (octo/new-name).

matchesBranchHeadContext compares the remote-derived owner/name to the PR's headRepository.nameWithOwner with strict equality, so every PR is rejected even though branch and owner match. Effects:

  • An open PR is never detected for the branch (no PR status / "View PR" CTA).
  • Running "Commit, push & PR" again does not see the existing PR, tries to create a new one, and fails with GitHub's A pull request already exists for <owner>:<branch>, even though the commit was pushed.

Reproduction: rename a GitHub repo, keep the old remote URL, push a branch, create a PR, then commit again with "Commit, push & PR".

Change

In matchesBranchHeadContext (apps/server/src/git/GitManager.ts), a repository-name mismatch is tolerated only when all of these hold: the owner logins are equal, the head context is not cross-repository, and the PR reports isCrossRepository === false. Cross-repository heads, differing owners, and PRs flagged cross-repository keep the existing strict comparison.

Scope and approval

Very small, focused bug fix in one predicate with no new settings or behavior beyond correctly recognizing an already-open PR. It qualifies for the no-prior-issue route under CONTRIBUTING.md.

Verification

  • Added a regression test in GitManager.test.ts (matches same-repo PR metadata when the remote URL uses a renamed repository slug) covering: renamed slug with same owner matches; different owner does not; cross-repository head context and cross-repository PR keep rejecting a name mismatch.
  • vp test run in apps/server on src/git/GitManager.test.ts: 141 passed.
  • Manually observed the original failure on a real repo renamed from zahlas-puzzles to zahlas with an old-slug remote, using a locally patched build with an equivalent change: existing PR was detected and repeat commits no longer attempted a duplicate PR.
  • Not checked: other providers (GitLab, Bitbucket, Forgejo) beyond the existing unit tests.

Authored with Claude Sonnet 5.5 via Antigravity.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 15, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at fc1eecd

Macroscope's review found this PR approvable — The PR makes a narrowly scoped change to existing PR matching so same-owner repository renames are recognized, while preserving strict cross-repository checks. It includes focused regression coverage and introduces no schema, default, infrastructure, security, or static-analysis changes.

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

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

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

⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 46d8cd8c-49ea-4f61-a784-e3ff19b55a85
📥 Commits

Reviewing files that changed from the base of the PR and between fc1eecd and fb42959.

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

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3a068bde-40aa-4509-8fc0-b75f7a346d65
📥 Commits

Reviewing files that changed from the base of the PR and between ede3f2d and fc1eecd.

📒 Files selected for processing: 2
  • apps/server/src/git/GitManager.test.ts
  • apps/server/src/git/GitManager.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

matchesBranchHeadContext now accepts repository name differences when both sides have the same non-null owner and neither side is cross-repository. Tests cover these conditions and verify that other mismatches remain rejected.

Changes: Branch head matching

Layer / File(s) Summary
Matching rules and tests
apps/server/src/git/GitManager.ts, apps/server/src/git/GitManager.test.ts
matchesBranchHeadContext accepts repository name differences for matching owners when neither side is cross-repository. Tests cover renamed repositories, owner mismatches, and cross-repository cases.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to fc1ee

The rename-matching change appears ready to merge after normal checks.

### Architecture Summary

Architecture risk: 🔵 Low · up to fc1ee

The change affects 1 system.

Changed systems: apps/server

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/server (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/git/GitManager.test.ts: Added coverage for renamed-repository matching: same-owner, same-branch same-repository PR metadata matches despite differing repository slugs; a different owner does not match, and cross-repository metadata is rejected whether both sides or only the PR are marked cross-repository.
  • observed — Modified behavior in apps/server/src/git/GitManager.ts: matchesBranchHeadContext no longer rejects every mismatch between known repository names. It now accepts a mismatch only when the owner logins match and both sides are non-cross-repository; otherwise it returns false.
Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main fix: detecting existing pull requests when the remote URL contains a renamed repository slug.
Description check Passed The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the bug, the guarded fix, the no-prior-issue justification, focused test results, manual v…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed 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

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

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Thanks for this. I'm closing it as part of PR triage. The approach here looks wrong or too risky to merge as is.

From the review: Allowing repository names to differ for the same owner can select an unrelated repository's PR with the same branch name. Resolve the canonical repository identity before relaxing this check.

If you think this was a mistake, message me on Twitter at @maria_rcks and I'll take another look.

@maria-rcks maria-rcks closed this Oct 17, 2026
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:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants