Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -651,18 +651,18 @@ function selectTemplate(t: { name: string; source: string }) {
<Label class="text-sm">Ref Type</Label>
<Select.Root type="single" bind:value={refType}>
<Select.Trigger class="h-8 w-full rounded-lg">
{refType === "branch" ? "Branch" : refType === "commit" ? "Commit" : "Pull Request"}
{refType === "branch" ? "Branch" : refType === "commit" ? "Commit" : "Pull / Merge Request"}
</Select.Trigger>
<Select.Content>
<Select.Item value="branch">Branch</Select.Item>
<Select.Item value="commit">Commit</Select.Item>
<Select.Item value="pr">Pull Request</Select.Item>
<Select.Item value="pr">Pull / Merge Request</Select.Item>
</Select.Content>
</Select.Root>
</div>
<div class="space-y-1.5">
<Label class="text-sm">
{refType === "branch" ? "Branch name" : refType === "commit" ? "Commit SHA" : "PR number"}
{refType === "branch" ? "Branch name" : refType === "commit" ? "Commit SHA" : "PR / MR number"}
</Label>
<Input
placeholder={refType === "branch" ? "main" : refType === "commit" ? "abc123…" : "42"}
Expand Down Expand Up @@ -1022,7 +1022,7 @@ function selectTemplate(t: { name: string; source: string }) {
{#if sourceType === "git" && refValue.trim()}
<div class="flex justify-between gap-3">
<span class="text-muted-foreground">
{refType === "branch" ? "Branch" : refType === "commit" ? "Commit" : "Pull request"}
{refType === "branch" ? "Branch" : refType === "commit" ? "Commit" : "Pull / Merge request"}
</span>
<span class="font-medium truncate">{refValue}</span>
</div>
Expand Down
16 changes: 15 additions & 1 deletion desktop/src/renderer/src/lib/utils/workspace-source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,27 @@ describe("buildWorkspaceSource", () => {
expect(out.source).toBe("github.com/org/repo@sha256:abc123")
})

it("git: PR ref appends @pull/N/head", () => {
it("git: PR ref appends @pull/N/head for GitHub", () => {
const out = buildWorkspaceSource(
gitForm({ repoUrl: "github.com/org/repo", refType: "pr", refValue: "42" }),
)
expect(out.source).toBe("github.com/org/repo@pull/42/head")
})

it("git: MR ref appends @merge-requests/N/head for GitLab", () => {
const out = buildWorkspaceSource(
gitForm({ repoUrl: "gitlab.com/org/repo", refType: "pr", refValue: "7125" }),
)
expect(out.source).toBe("gitlab.com/org/repo@merge-requests/7125/head")
})

it("git: MR ref detects self-hosted GitLab by hostname", () => {
const out = buildWorkspaceSource(
gitForm({ repoUrl: "git@gitlab.example.com:org/repo.git", refType: "pr", refValue: "7" }),
)
expect(out.source).toBe("git@gitlab.example.com:org/repo.git@merge-requests/7/head")
})

it("git: subpath appends @subpath: after ref", () => {
const out = buildWorkspaceSource(
gitForm({
Expand Down
19 changes: 16 additions & 3 deletions desktop/src/renderer/src/lib/utils/workspace-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,19 @@ export interface WorkspaceSourceResult {
prebuildRepository?: string
}

function refSuffix(refType: GitRefType, refValue: string): string {
// GitLab exposes merge requests at merge-requests/N/head; every other host
// uses pull/N/head. Detection is best-effort — the CLI fetch falls back to the
// other convention when the detected one has no such ref.
function prRefspec(repoUrl: string, number: string): string {
const segment = /gitlab/i.test(repoUrl) ? "merge-requests" : "pull"
return `@${segment}/${number}/head`
}

function refSuffix(
refType: GitRefType,
refValue: string,
repoUrl: string,
): string {
const value = refValue.trim()
if (!value) return ""
switch (refType) {
Expand All @@ -43,7 +55,7 @@ function refSuffix(refType: GitRefType, refValue: string): string {
case "commit":
return `@sha256:${value}`
case "pr":
return `@pull/${value}/head`
return prRefspec(repoUrl, value)
}
}

Expand Down Expand Up @@ -72,7 +84,8 @@ export function buildWorkspaceSource(
}
}

const source = `${form.repoUrl.trim()}${refSuffix(form.refType, form.refValue)}${subPathSuffix(form.subPath)}`
const repoUrl = form.repoUrl.trim()
const source = `${repoUrl}${refSuffix(form.refType, form.refValue, repoUrl)}${subPathSuffix(form.subPath)}`

return {
source,
Expand Down
28 changes: 20 additions & 8 deletions pkg/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ import (
)

const (
CommitDelimiter string = "@sha256:"
PullRequestReference string = "pull/([0-9]+)/head"
CommitDelimiter string = "@sha256:"
// PullRequestReference matches a pull-request (GitHub) or merge-request
// (GitLab) refspec, capturing the request number.
PullRequestReference string = "(?:pull|merge-requests)/([0-9]+)/head"
SubPathDelimiter string = "@subpath:"
repoBaseRegEx string = `((?:(?:https?|git|ssh|file):\/\/)?\/?(?:[^@\/\n]+@)?` +
`(?:[^:\/\n]+)(?:[:\/][^\/\n]+)+(?:\.git)?)`
)

// WARN: Make sure this matches the regex in /desktop/src/views/Workspaces/CreateWorkspace/CreateWorkspaceInput.tsx!
// WARN: Keep these in sync with the ref parsing in desktop's workspace-source.ts.
var (
branchRegEx = regexp.MustCompile(`^` + repoBaseRegEx + `@([a-zA-Z0-9\./\-\_]+)$`)
commitRegEx = regexp.MustCompile(
Expand All @@ -36,7 +38,8 @@ var recognizedSchemes = []string{"ssh://", "git@", "http://", "https://", "file:

// NormalizeRepository parses a repository reference into its structured parts.
// Accepts plain URLs, the "git:<url>" workspace-source scheme, and references
// suffixed with @branch, @subpath:<path>, @sha256:<commit>, or @pull/N/head.
// suffixed with @branch, @subpath:<path>, @sha256:<commit>, or a pull/merge
// request ref (@pull/N/head or @merge-requests/N/head).
// Bare host[/path] inputs are upgraded to https://.
func NormalizeRepository(str string) *GitInfo {
str = canonicalizeURL(str)
Expand Down Expand Up @@ -87,14 +90,23 @@ func PingRepository(str string, extraEnv []string) bool {
return At("", WithEnv(extraEnv)).LsRemote(timeoutCtx, str) == nil
}

// GetBranchNameForPR returns the local branch name for a request ref, using the
// provider convention encoded in the ref (PR<n> for GitHub, MR<n> for GitLab).
func GetBranchNameForPR(ref string) string {
regex := regexp.MustCompile(PullRequestReference)
return regex.ReplaceAllString(ref, "PR${1}")
number := prNumber(ref)
if number == "" {
return ref
}
return hostForRef(ref).BranchName(number)
}

// GetIDForPR returns the lowercased request identifier used in workspace IDs.
func GetIDForPR(ref string) string {
regex := regexp.MustCompile(PullRequestReference)
return regex.ReplaceAllString(ref, "pr${1}")
number := prNumber(ref)
if number == "" {
return ref
}
return strings.ToLower(hostForRef(ref).BranchName(number))
}

// GitInfo is the parsed form of a repository reference. Branch, Commit, PR,
Expand Down
12 changes: 12 additions & 0 deletions pkg/git/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ var normalizeRepositoryCases = []testCaseNormalizeRepository{
expectedCommit: "",
expectedSubpath: "",
},
{
in: "git@gitlab.com:h3upperbounds/data/data-team.git@merge-requests/7125/head",
expectedRepo: "git@gitlab.com:h3upperbounds/data/data-team.git",
expectedPRReference: "merge-requests/7125/head",
expectedBranch: "",
expectedCommit: "",
expectedSubpath: "",
},
{
in: "github.com/devsy-org/devsy-without-protocol-with-slash.git@subpath:/test/path",
expectedRepo: repoDevsyNoProtoSlash,
Expand Down Expand Up @@ -260,6 +268,10 @@ func TestGetBranchNameForPRReference(t *testing.T) {
in: testPRRef,
expectedBranch: "PR996",
},
{
in: "merge-requests/7125/head",
expectedBranch: "MR7125",
},
{
in: "pull/abc/head",
expectedBranch: "pull/abc/head",
Expand Down
90 changes: 90 additions & 0 deletions pkg/git/host.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package git

import (
"regexp"
"strings"
)

var prNumberRegEx = regexp.MustCompile(`(?:pull|merge-requests)/([0-9]+)/head`)

const (
hostNameGitHub = "github"
hostNameGitLab = "gitlab"
)

// Host is a git provider's convention for referencing pull/merge requests:
// GitHub exposes them at pull/N/head, GitLab at merge-requests/N/head.
type Host struct {
Name string
refPrefix string
branchAbbr string
hostHint string // substring identifying the provider in a repository URL
}

var (
HostGitHub = Host{
Name: hostNameGitHub,
refPrefix: "pull",
branchAbbr: "PR",
hostHint: hostNameGitHub,
}
HostGitLab = Host{
Name: hostNameGitLab,
refPrefix: "merge-requests",
branchAbbr: "MR",
hostHint: hostNameGitLab,
}

// GitHub first: it is both the detection default and the first fallback.
knownHosts = []Host{HostGitHub, HostGitLab}
)

func (h Host) Refspec(number string) string {
return h.refPrefix + "/" + number + "/head"
}

func (h Host) BranchName(number string) string {
return h.branchAbbr + number
}

// DetectHost picks the provider from a repository URL, defaulting to GitHub.
// It is best-effort: self-hosted instances on custom domains won't be
// recognized, which is why the fetch path falls back to the other convention.
func DetectHost(repoURL string) Host {
lower := strings.ToLower(repoURL)
for _, h := range knownHosts {
if strings.Contains(lower, h.hostHint) {
return h
}
}
return HostGitHub
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// hostForRef infers the provider from the ref itself, which already encodes the
// convention exactly (unlike DetectHost's URL heuristic).
func hostForRef(ref string) Host {
if strings.Contains(ref, HostGitLab.refPrefix+"/") {
return HostGitLab
}
return HostGitHub
}

func prNumber(ref string) string {
if m := prNumberRegEx.FindStringSubmatch(ref); m != nil {
return m[1]
}
return ""
}

// prCandidates orders the hosts to try for a checkout: detected host first,
// remaining conventions as fallbacks.
func prCandidates(repoURL string) []Host {
primary := DetectHost(repoURL)
candidates := []Host{primary}
for _, h := range knownHosts {
if h.Name != primary.Name {
candidates = append(candidates, h)
}
}
return candidates
}
47 changes: 47 additions & 0 deletions pkg/git/host_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package git

import (
"testing"

"gotest.tools/assert"
"gotest.tools/assert/cmp"
)

func TestDetectHost(t *testing.T) {
cases := []struct {
url string
want string
}{
{"git@github.com:org/repo.git", hostNameGitHub},
{"https://github.com/org/repo.git", hostNameGitHub},
{"git@gitlab.com:org/repo.git", hostNameGitLab},
{"https://gitlab.example.com/org/repo.git", hostNameGitLab},
{
"https://git.internal.example/org/repo.git",
hostNameGitHub,
}, // unknown host defaults to GitHub
}
for _, c := range cases {
assert.Check(t, cmp.Equal(c.want, DetectHost(c.url).Name), "url=%s", c.url)
}
}

func TestHostRefspecAndBranch(t *testing.T) {
assert.Equal(t, "pull/42/head", HostGitHub.Refspec("42"))
assert.Equal(t, "PR42", HostGitHub.BranchName("42"))
assert.Equal(t, "merge-requests/42/head", HostGitLab.Refspec("42"))
assert.Equal(t, "MR42", HostGitLab.BranchName("42"))
}

func TestPRNumber(t *testing.T) {
assert.Equal(t, "996", prNumber("pull/996/head"))
assert.Equal(t, "7125", prNumber("merge-requests/7125/head"))
assert.Equal(t, "", prNumber("refs/heads/main"))
}

func TestPRCandidatesOrder(t *testing.T) {
got := prCandidates("git@gitlab.com:org/repo.git")
assert.Equal(t, 2, len(got))
assert.Equal(t, hostNameGitLab, got[0].Name) // detected host first
assert.Equal(t, hostNameGitHub, got[1].Name) // fallback
}
33 changes: 24 additions & 9 deletions pkg/git/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,30 @@ func (r *Repo) Switch(ctx context.Context, branch string) error {
return nil
}

// CheckoutPR fetches the given pull request refspec and switches to a local branch for it.
func (r *Repo) CheckoutPR(ctx context.Context, prRef string) error {
log.Debugf("fetching pull request: %s", prRef)

prBranch := GetBranchNameForPR(prRef)
if err := r.Fetch(ctx, prRef+":"+prBranch); err != nil {
return err
// CheckoutPR fetches a pull/merge request into a local branch and switches to
// it. The request number is taken from prRef; the remote refspec is resolved
// against repoURL's hosting provider, falling back to the other known
// conventions when the detected one has no such ref (e.g. self-hosted GitLab on
// a custom domain that URL detection can't recognize).
func (r *Repo) CheckoutPR(ctx context.Context, repoURL, prRef string) error {
number := prNumber(prRef)
if number == "" {
return fmt.Errorf("not a pull/merge request reference: %q", prRef)
}

var lastErr error
for _, host := range prCandidates(repoURL) {
refspec := host.Refspec(number)
prBranch := host.BranchName(number)
log.Debugf("fetching %s request: %s", host.Name, refspec)

if err := r.Fetch(ctx, refspec+":"+prBranch); err != nil {
lastErr = err
continue
}
return r.Switch(ctx, prBranch)
}
return r.Switch(ctx, prBranch)
return lastErr
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

// Reset moves HEAD to commit using the given mode.
Expand Down Expand Up @@ -140,7 +155,7 @@ func (r *Repo) CloneFromInfo(

switch {
case gitInfo.PR != "":
if err := r.CheckoutPR(ctx, gitInfo.PR); err != nil {
if err := r.CheckoutPR(ctx, gitInfo.Repository, gitInfo.PR); err != nil {
return err
}
case gitInfo.Commit != "":
Expand Down
Loading
Loading