feat(github): support cross-fork PRs by passing HeadRepositoryId#515
Open
jucor wants to merge 2 commits into
Open
feat(github): support cross-fork PRs by passing HeadRepositoryId#515jucor wants to merge 2 commits into
jucor wants to merge 2 commits into
Conversation
goreleaser v2.16.0 (2026-05-24) turned the deprecated `brews:` key in .goreleaser.yml into a hard error. With `version: latest`, every push since fails CI's `goreleaser check` step regardless of the code change. v2.15.4 (2026-04-21) is the latest release that still accepts `brews:` with only a warning and also understands the current `archives.format_overrides.formats` syntax. Pinning to it restores green CI without touching .goreleaser.yml. ## Migration path (for whoever unpins) The goreleaser deprecation page recommends migrating `brews:` → `homebrew_casks:`. For simple cases this is a key rename, plus a `hooks.post.install` block to strip the macOS quarantine xattr (because the binaries aren't signed/notarized). End-user impact: - macOS: transparent. The binary still lands at `/opt/homebrew/bin/git-spr` (via cask `binary` stanza) — same PATH, same usage. The install command may need `--cask` explicitly in the readme. - Linux: regression. Casks are macOS-only; Linux brew users currently installing via `brew install ejoffe/tap/spr` would have to switch to another channel (binary release, apt, Nix, source). - Tap repo: `Formula/spr.rb` becomes `Casks/spr.rb`. The `ejoffe/homebrew-tap` repo would need a transition plan. Alternative: drop `brews:` entirely and remove the brew install path from the readme. Simpler but breaks the channel outright. Either way it's a deliberate distribution decision, not a mechanical CI fix, which is why this PR pins rather than migrates. Unpin once the `brews:` block is migrated or removed.
Before this change spr could only open PRs whose head branch lived in the
same repository as the PR target. That collapsed the (fork, upstream)
distinction: GetInfo resolved a single RepositoryID and CreatePullRequest
passed only HeadRefName, so GitHub looked for the head branch inside the
upstream repo even when the user had pushed it to a fork.
We now detect cross-fork operation from the URL of cfg.Repo.GitHubRemote
(the actual push target) vs cfg.Repo.GitHubRepoOwner/Name (the PR target).
When they differ, spr resolves the fork's GitHub node ID via StarGetRepo
and threads it through info.HeadRepositoryID. CreatePullRequest sets the
GraphQL HeadRepositoryId field, which is GitHub's documented way to route
a same-named branch through the fork.
When the URL is unparseable, the remote is missing, or StarGetRepo fails,
detection silently falls back to same-fork behavior and GitHub surfaces
the resulting error on the create call, matching pre-change semantics.
MaintainerCanModify is now a UserConfig option (default true) and is
forwarded only in cross-fork mode; it has no meaning for same-fork PRs.
URL parsing is factored out as git.ParseRepoURL and shared with the
existing config_parser remote source, which previously had its own
inline regex.
Tests cover:
- ParseRepoURL: HTTPS / SSH / no-suffix / enterprise host / dashes /
underscores / case / trailing slash / whitespace / malformed.
- buildCreatePullRequestInput: same-fork omits HeadRepositoryId and
MaintainerCanModify; cross-fork sets both; MaintainerCanModify=false
is honoured; HeadRepositoryID equal to RepositoryID is treated as
same-fork (defensive).
No behavior change for users whose origin is the PR target.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before this change spr could only open PRs whose head branch lived in the same repository as the PR target. That collapsed the (fork, upstream) distinction:
GetInforesolved a singleRepositoryIDandCreatePullRequestpassed onlyHeadRefName, so GitHub looked for the head branch inside the upstream repo even when the user had pushed it to a fork.We now detect cross-fork operation from the URL of
cfg.Repo.GitHubRemote(the actual push target) vscfg.Repo.GitHubRepoOwner/Name(the PR target). When they differ, spr resolves the fork's GitHub node ID viaStarGetRepoand threads it throughinfo.HeadRepositoryID.CreatePullRequestsets the GraphQLHeadRepositoryIdfield, which is GitHub's documented way to route a same-named branch through the fork.When the URL is unparseable, the remote is missing, or
StarGetRepofails, detection silently falls back to same-fork behavior and GitHub surfaces the resulting error on the create call, matching pre-change semantics.MaintainerCanModifyis now aUserConfigoption (defaulttrue) and is forwarded only in cross-fork mode; it has no meaning for same-fork PRs.URL parsing is factored out as
git.ParseRepoURLand shared with the existingconfig_parserremote source, which previously had its own inline regex.Tests
ParseRepoURL: HTTPS / SSH / no-suffix / enterprise host / dashes / underscores / case / trailing slash / whitespace / malformed.buildCreatePullRequestInput: same-fork omitsHeadRepositoryIdandMaintainerCanModify; cross-fork sets both;MaintainerCanModify=falseis honoured;HeadRepositoryIDequal toRepositoryIDis treated as same-fork (defensive).No behavior change for users whose origin is the PR target.
Known trade-off: cross-fork stacked PRs lose the per-commit diff view
A cross-fork PR's base branch must live in the upstream repo — that's a GitHub constraint, not a spr one. For a stack of PRs that means every PR in the stack must base on
master(the only branch in upstream the contributor can reference) rather than on the previous PR's head (which lives in the fork). Consequence: each subsequent PR's "Files changed" tab shows the cumulative diff from all predecessors, not just its own commit.In spr's native same-fork stacked PRs (where
originis the PR target), PR N's base is PR N-1's head branch — same repo, so it works — and each PR's diff is exactly one commit. Cross-fork sacrifices that.For reviewers:
/compare/SHA1...SHA2) pasted into the PR body can point at the per-commit diff.The only way to recover proper stacked PRs across fork boundaries is for the upstream maintainer to grant the contributor write access to the upstream repo (collaborator role with
WriteorMaintainpermission). That lets the contributor push feature branches directly to upstream, which can then be valid PR bases. This is unrelated to themaintainerCanModifyflag this PR adds — that goes the opposite direction (maintainer can push to contributor's PR branch on the fork). GitHub has no in-between for cross-fork base permissions.Full transparency: I made heavy use of Claude Code (Opus 4.7, high effort) for this PR, as I do for my own work these days.