Skip to content

Initial implementation of PR checkout#97

Merged
andrew merged 2 commits into
git-pkgs:mainfrom
untitaker:pr-checkout
May 23, 2026
Merged

Initial implementation of PR checkout#97
andrew merged 2 commits into
git-pkgs:mainfrom
untitaker:pr-checkout

Conversation

@untitaker
Copy link
Copy Markdown
Contributor

forge pr checkout 123 will now check out PR/MR locally:

  1. Bunch of API calls (forge-specific)
  2. Add fork as remote if necessary
  3. Make new branch with upstream from fork

I tested this manually on Codeberg and GitHub. An agent also tested it on GitLab and Bitbucket.

Since there are no e2e tests in this repo, I mainly constrained myself to testing the CLI with a mock forge. There are no CLI tests that spawn HTTP servers and run CLI, so I refrained from that.

Existing fields on the structs were preserved for backwards compat, let me know if you want me to remove them since this library is young, or whether PRBranch should be flattened into its parent structs.

Fix #92

`forge pr checkout 123` will now check out PR/MR locally:

1. Bunch of API calls (forge-specific)
2. Add fork as remote if necessary
3. Make new branch with upstream from fork

I tested this manually on Codeberg and GitHub. An agent also tested it
on GitLab and Bitbucket.

Since there are no e2e tests in this repo, I mainly constrained myself
to testing the CLI with a mock forge. There are no CLI tests that spawn
HTTP servers and run CLI, so I refrained from that.

Existing fields on the structs were preserved for backwards compat, let
me know if you want me to remove them since this library is young, or
whether PRBranch should be flattened into its parent structs.
Copy link
Copy Markdown
Contributor

@andrew andrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, the backend plumbing is the hard part and it looks right across all four forges. A few things to sort before merging.

Should fix

checkoutSameRepoPR hardcodes "origin" (internal/cli/pr.go). The CLI has a global --remote flag wired through resolve.SetRemote, so forge --remote upstream pr checkout 42 resolves the PR against upstream but then fetches from origin. It should use whichever remote resolve.Repo actually read. resolve doesn't currently export that name; a small resolve.RemoteName() accessor would do.

GitLab silently drops the fork lookup error (gitlab/prs.go). When GetProject(mr.SourceProjectID) fails (private fork, deleted project) the error is discarded and Fork stays nil, so checkout falls through to the same-repo path and git fails with "couldn't find remote ref" instead of telling the user the source project isn't accessible. Returning the error would be better.

Gitea: pr.Head.Repository.Owner is *User in the SDK and can be nil (deleted owner), so Owner.UserName will panic. Needs a guard.

On the Head/Base question

Drop the old string fields rather than keep both. We're pre-1.0 and nothing imports this yet; having pr.Head and pr.HeadBranch.Ref hold the same value will drift. I'd make Head and Base into PRBranch values (non-pointer, they're always populated) and update the callers.

Smaller stuff

--force only actually gates the dirty-tree check. An existing local branch just prints a warning and gets reset via -B regardless. Either make existing-branch a hard error without --force, or drop "or existing branch" from the flag help.

ensureRemote does exact-string URL matching, so a fork already added via SSH won't match the HTTPS URL the API returns (and Bitbucket's https://user@... form won't match either). Fine to leave for now but it'll come up.

Tests: use t.Chdir(dir) instead of os.Getwd/os.Chdir with manual cleanup, to match resolve_test.go and repo_test.go.

deadcode flags SetTestForge/ResetTestForge because they're test-only exports. Same situation as setBitbucketAPI so not new noise, just FYI.

@untitaker
Copy link
Copy Markdown
Contributor Author

I addressed everything but the ensureRemote concern, since I didn't find an easy way to fix it.

--force only actually gates the dirty-tree check. An existing local branch just prints a warning and gets reset via -B regardless. Either make existing-branch a hard error without --force, or drop "or existing branch" from the flag help.

I got rid of that behavior, the code now roughly matches what the gh cli does. Unfortunately I find the gh cli's behavior somewhat confusing: It doesn't refuse to check out PRs if the worktree is dirty, it just sort of fails halfway through if any files conflict. But I think unless we can show that this behavior can be improved without fundamentally breaking people's existing gh cli workflow, it's better to be bug-compatible.

Copy link
Copy Markdown
Contributor

@andrew andrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andrew andrew merged commit d7789ac into git-pkgs:main May 23, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for forge pr checkout

2 participants