Target pull requests at upstream repositories#3900
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
vouch:unvouched
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces new fork-to-upstream PR targeting functionality with significant runtime behavior changes. Multiple unresolved P1/P2 review comments identify potential bugs in CLI argument handling, GitHub Enterprise support, and error handling that should be addressed before merging. You can customize Macroscope's approvability policy. Learn more. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb9cf66da7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "--head", | ||
| input.headSelector, |
There was a problem hiding this comment.
When origin is a fork, resolveBranchHeadContext still treats the branch as non-cross-repo and passes a bare branch name as input.headSelector; after this change the same gh pr create call now adds --repo for the upstream parent, so --head feature is resolved against the upstream owner instead of the fork. The GitHub CLI docs say --head uses <user>:<branch> to select a head repo, and the CLI maintainers clarify that a bare head defaults to the --repo owner, so branches pushed only to the fork will fail to create a PR (or target an upstream branch with the same name).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 7f560fc — bare --head values are qualified with the fork owner before gh pr create/pr list against the upstream --repo.
| "number,title,url,baseRefName,headRefName,state,mergedAt,isCrossRepository,headRepository,headRepositoryOwner", | ||
| "nameWithOwner,parent", | ||
| "--jq", | ||
| 'if .parent then "\\(.parent.owner.login)/\\(.parent.name)" else .nameWithOwner end', |
There was a problem hiding this comment.
For GitHub Enterprise checkouts, this resolver returns only owner/repo for both fork parents and same-repo cases, but the later --repo override needs the documented [HOST/]OWNER/REPO form to keep commands on the enterprise host. In an environment authenticated to both github.com and github.company.com (or where github.com is the default host), gh pr list/create and gh repo view will resolve this hostless value against the wrong host, regressing PR creation/default-branch lookup for GHES repositories.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in f4f6e83 — base/head repository resolution now emits HOST/OWNER/REPO from the repo URL so --repo stays on the enterprise host.
When --repo targets a fork parent, bare --head values resolve against the upstream owner. Qualify with owner:branch from the local head repository so create/list hit the fork branch. Co-authored-by: Cursor <cursoragent@cursor.com>
listChangeRequests for non-open states, getPullRequest, and checkoutPullRequest still used the fork cwd. Route them through the resolved base repository so fork checkouts can list and open upstream PRs. Co-authored-by: Cursor <cursoragent@cursor.com>
Fork checkouts are not local cross-repo remotes, so open PRs against the upstream parent were filtered out as isCrossRepository. Keep matching when the PR head repository aligns with the expected fork head. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve base/head repositories as HOST/OWNER/REPO from the repo URL so gh commands stay on the authenticated enterprise host instead of defaulting to github.com. Co-authored-by: Cursor <cursoragent@cursor.com>
github-actions
Bot
added
size:L
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4f6e834bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "--base", | ||
| input.baseBranch, | ||
| "--head", | ||
| qualifyPullRequestHead(context, input.headSelector), |
There was a problem hiding this comment.
When the fork is owned by an organization, this path now always sends --head org:branch after qualifyPullRequestHead prefixes the fork owner. The GitHub CLI manual for gh pr create says --head supports <user>:<branch> and that using an organization as <user> is not supported, so upstream PR creation from org-owned forks fails even though the branch was pushed and --repo points at the correct base repository; consider detecting the owner type or using a fallback that can specify an org fork head repository.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 10aeb3888 — cross-repository PR creation now uses the GitHub API with explicit head_repo, which supports organization-owned forks.
| } else if (pr.isCrossRepository === true) { | ||
| if (!expectedHeadRepository || !prHeadRepository) { | ||
| return false; |
There was a problem hiding this comment.
For self-hosted GitHub fork checkouts, resolveRemoteRepositoryContext leaves expectedHeadRepository empty because parseGitHubRepositoryNameWithOwnerFromRemoteUrl only recognizes github.com remotes, while the new upstream-targeted listPullRequests can still return isCrossRepository: true PRs from the fork. This branch rejects those matches whenever the expected repo is missing, so create_pr on an existing GHES upstream PR will miss it and proceed to gh pr create (or, after creation, fail to recover the PR URL/number).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 849a0d275 — GitHub Enterprise remote URLs now resolve their owner/repository identity and existing cross-repository PRs are matched without duplicate creation.
| "pr", | ||
| "list", | ||
| "--head", | ||
| qualifyPullRequestHead(context, input.headSelector), |
There was a problem hiding this comment.
gh pr list heads
For fork checkouts this rewrites even the bare branch selector from findOpenPr/findLatestPr to owner:branch, but I checked the gh pr list manual and its --head option explicitly says the <owner>:<branch> syntax is not supported. That means the upstream PR lookup for fork-origin branches can fail or return no matches before create_pr runs, so existing upstream PRs are missed and the post-create lookup may not recover the PR URL/number; use an unqualified head plus another filter/search for the head repository instead of sharing the gh pr create selector here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in e9cd27e52 — gh pr list --head now receives the original unqualified branch selector, with repository identity still verified from the returned PR metadata.
| args: [ | ||
| "repo", | ||
| "view", | ||
| context.baseRepository, |
There was a problem hiding this comment.
For a fork checkout whose origin/main is stale behind the upstream repository, this now returns the upstream default branch name, but GitManager.resolveBaseRangeRef still resolves that bare branch through resolvePrimaryRemoteName (normally origin) before generating the PR title/body. The PR is created against the upstream repo, yet the generated range is based on the fork's remote-tracking branch, so the body can include unrelated upstream commits or omit the actual upstream diff; the upstream base ref needs to be fetched/resolved from the same repository used here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 9860b6e65 — PR content generation now fetches and resolves the target repository's base branch instead of comparing against a stale fork origin.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| }) | ||
| .pipe(Effect.map((resolved) => resolved.commitSha)); | ||
| }); | ||
| const targetBaseRangeRef = targetMatchesOrigin |
There was a problem hiding this comment.
🟡 Medium git/GitManager.ts:1137
When targetMatchesOrigin is false (fork workflow), resolveBaseRangeRef lets failures from adding/fetching the upstream remote abort the entire operation — the primary-remote fallback below is never reached. In the matching-origin case the same fetch is wrapped with Effect.orElseSucceed(() => null), so a transient network or auth failure degrades gracefully, but in the fork case a similar failure propagates and prevents PR creation even though the fallback could still resolve the base range. Consider applying the same Effect.orElseSucceed(() => null) wrapper to resolveTargetBaseRangeRef in the non-matching branch.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/git/GitManager.ts around line 1137:
When `targetMatchesOrigin` is `false` (fork workflow), `resolveBaseRangeRef` lets failures from adding/fetching the `upstream` remote abort the entire operation — the primary-remote fallback below is never reached. In the matching-origin case the same fetch is wrapped with `Effect.orElseSucceed(() => null)`, so a transient network or auth failure degrades gracefully, but in the fork case a similar failure propagates and prevents PR creation even though the fallback could still resolve the base range. Consider applying the same `Effect.orElseSucceed(() => null)` wrapper to `resolveTargetBaseRangeRef` in the non-matching branch.
There was a problem hiding this comment.
Addressed in a6246954b — the regression test now makes the intended fork behavior explicit: if the upstream base cannot be fetched, PR content generation stops rather than silently using a stale fork origin. Applying the suggested fallback would reintroduce the incorrect-base bug identified in the preceding review.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
There was a problem hiding this comment.
One convention issue found in the new fork-detection path added to GitHubCli.make. See the inline comment.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2edec75. Configure here.
| }); | ||
| const targetBaseRangeRef = targetMatchesOrigin | ||
| ? yield* resolveTargetBaseRangeRef.pipe(Effect.orElseSucceed(() => null)) | ||
| : yield* resolveTargetBaseRangeRef; |
There was a problem hiding this comment.
Same-repo check misses SSH aliases
Medium Severity
targetMatchesOrigin equates remotes via full normalizeGitRemoteUrl keys, so an SSH config host alias on origin (for example git@github.com-work:owner/repo) never matches GitHub’s canonical clone URLs. That same-repo checkout is treated like a fork, and a failed fetch of the canonical remote no longer falls back to the local tracking ref—create_pr fails instead.
Reviewed by Cursor Bugbot for commit 2edec75. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2edec75de1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "-f", | ||
| `head_repo=${head.name}`, | ||
| "-f", | ||
| `base=${input.baseBranch}`, |
There was a problem hiding this comment.
When a fork checkout has a local branch tracking a differently named fork branch (for example local-work with upstream origin/feature-x), GitManager.resolveBaseBranch still returns that upstream branch name because the head context is considered non-cross-repo relative to origin. This new API path then sends that value as base against the parent repository, so PR creation fails if the parent has no feature-x branch or targets the wrong parent branch if it does; fork-parent PRs should ignore the fork tracking branch and use the target repository's default/configured base.
Useful? React with 👍 / 👎.
| const targetBaseRangeRef = targetMatchesOrigin | ||
| ? yield* resolveTargetBaseRangeRef.pipe(Effect.orElseSucceed(() => null)) | ||
| : yield* resolveTargetBaseRangeRef; |
There was a problem hiding this comment.
For same-repo GitHub checkouts whose origin is a valid alternate URL, such as ssh://git@ssh.github.com:443/owner/repo.git or a corporate mirror/rewrite, targetMatchesOrigin is false because the comparison only recognizes the two clone URLs returned by gh repo view. This branch then treats a failed fetch of the canonical target URL as fatal instead of falling back to the existing origin/<base> tracking ref, so create_pr can abort before content generation even though the configured origin is usable; only fork-parent targets need this strict no-fallback behavior.
Useful? React with 👍 / 👎.
| "-f", | ||
| `title=${input.title}`, | ||
| "-f", | ||
| `head=${head.owner}:${headBranch}`, |
There was a problem hiding this comment.
When origin is a fork but the current branch is pushed to a different remote (for example an upstream/parent or another fork remote), GitManager passes a qualified selector for that actual head repository, but this API path discards the selector owner and always uses context.headRepository from gh repo view of the checkout. In that setup the request is sent as originOwner:branch, so PR creation fails if the branch only exists on the selected remote or opens from the wrong fork if a same-named branch exists on origin.
Useful? React with 👍 / 👎.
1 participant
Summary
Why
When a T3 Code checkout used a fork as
origin, the commit/push/create-PR flow and the native create-PR action allowed GitHub CLI to infer the target repository from that fork. As a result, new PRs could be opened against the fork instead of its upstream source repository.This change uses GitHub's parent repository metadata and passes the resolved repository explicitly, while preserving same-repository behavior for non-forks.
Validation
vp checkvp run typecheckjakeleventhal/t3codetopingdotgg/t3codeNote
Target pull requests at upstream repositories when origin is a fork
gh api) to POST against the upstream base repository, with the head selector qualified by owner to support organization-owned forks.ghCLI calls (list, view, checkout, default branch) are scoped to the resolved base repository via--repo, using a cachedgh repo viewcontext resolution.resolveBaseRangeRefin GitManager.ts fetches the base commit from the upstream remote when the origin URL differs from the target repository's clone URLs, falling back to local tracking if the fetch fails.getTargetRepositoryCloneUrlsis added to theSourceControlProviderinterface and implemented inGitHubSourceControlProviderto supply upstream clone URLs to consumers.parseGitHubRepositoryNameWithOwnerFromRemoteUrlnow supports GitHub Enterprise URL formats and rejects non-GitHub remotes.listPullRequestsnow fetches up to 100 open PR candidates (previously 1), increasing gh API call cost when searching for an existing open PR.Macroscope summarized 2edec75.
Note
Medium Risk
Changes core PR create/list/checkout and diff-base resolution for fork workflows; mistakes could open PRs on the wrong repo or break PR generation, but behavior is heavily regression-tested.
Overview
When
originis a fork, commit/push/create-PR flows now resolve the parent (upstream) repository and run GitHub CLI against that repo instead of inferring targets from the fork.GitHubClicaches per-cwd base/head context fromgh repo view(parent vs current repo). PR list, view, checkout, and default branch pass--repoto the base repo. Fork PR creation usesgh apiwith explicithead,head_repo, and host; same-repo creation still usesgh pr createwith qualified heads.listOpenPullRequestsbecomeslistPullRequestswith configurable state; list decoding is centralized.GitManagerraises open-PR candidate scans to 100, tightensmatchesBranchHeadContextfor cross-repo mismatches, andresolveBaseRangeReffetches the upstream base viagetTargetRepositoryCloneUrls(new on the GitHub provider/registry) so generated PR content uses the upstream diff range; failed upstream fetch on forks aborts before generating content.parseGitHubRepositoryNameWithOwnerFromRemoteUrlin shared git now supports GitHub Enterprise hosts and ignores non-GitHub remotes. Tests cover fork create/reuse, GHE, and upstream range behavior.Reviewed by Cursor Bugbot for commit 2edec75. Bugbot is set up for automated code reviews on this repo. Configure here.