From c04dd5fb0883a5727707ff77c20883ed5d0a7da4 Mon Sep 17 00:00:00 2001 From: benekuehn <41262108+benekuehn@users.noreply.github.com> Date: Thu, 30 Jul 2026 08:31:25 +0200 Subject: [PATCH 1/5] fix: refresh repository identity after SSH translation Refresh cached remote metadata after translating SSH host aliases. Preserve go-gh's fetch-then-push fallback when extracting repository identity. Relates to github/gh-stack#45 and github/gh-stack#337. --- pkg/repository/repository.go | 37 ++++++++++++++++----- pkg/repository/repository_test.go | 54 +++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/pkg/repository/repository.go b/pkg/repository/repository.go index 008dd43..1ea663f 100644 --- a/pkg/repository/repository.go +++ b/pkg/repository/repository.go @@ -5,6 +5,7 @@ package repository import ( "errors" "fmt" + "net/url" "os" "strings" @@ -133,14 +134,7 @@ func Current() (Repository, error) { } translator := ssh.NewTranslator() - for _, r := range remotes { - if r.FetchURL != nil { - r.FetchURL = translator.Translate(r.FetchURL) - } - if r.PushURL != nil { - r.PushURL = translator.Translate(r.PushURL) - } - } + translateRemotes(remotes, translator.Translate) hosts := auth.KnownHosts() @@ -172,3 +166,30 @@ func Current() (Repository, error) { return r, nil } + +func translateRemotes(remotes git.RemoteSet, translate func(*url.URL) *url.URL) { + for _, remote := range remotes { + hasFetchInfo := false + if remote.FetchURL != nil { + remote.FetchURL = translate(remote.FetchURL) + hasFetchInfo = updateRemoteInfo(remote, remote.FetchURL) + } + if remote.PushURL != nil { + remote.PushURL = translate(remote.PushURL) + if !hasFetchInfo { + updateRemoteInfo(remote, remote.PushURL) + } + } + } +} + +func updateRemoteInfo(remote *git.Remote, u *url.URL) bool { + host, owner, repo, err := git.RepoInfoFromURL(u) + if err != nil { + return false + } + remote.Host = host + remote.Owner = owner + remote.Repo = repo + return true +} diff --git a/pkg/repository/repository_test.go b/pkg/repository/repository_test.go index f441a19..2b78e1a 100644 --- a/pkg/repository/repository_test.go +++ b/pkg/repository/repository_test.go @@ -1,6 +1,7 @@ package repository import ( + "net/url" "testing" "github.com/cli/go-gh/v2/internal/git" @@ -9,6 +10,59 @@ import ( "github.com/stretchr/testify/require" ) +func TestTranslateRemotesRefreshesRepositoryInfo(t *testing.T) { + translate := func(u *url.URL) *url.URL { + translated := *u + translated.Host = "github.com" + return &translated + } + + t.Run("SSH host alias", func(t *testing.T) { + u, err := url.Parse("ssh://git@github.com-work/owner/repo.git") + if err != nil { + t.Fatal(err) + } + remotes := git.RemoteSet{&git.Remote{ + Name: "origin", + FetchURL: u, + Host: "github.com-work", + Owner: "owner", + Repo: "repo", + }} + + translateRemotes(remotes, translate) + + filtered := remotes.FilterByHosts([]string{"github.com"}) + assert.Len(t, filtered, 1) + assert.Equal(t, "github.com", remotes[0].Host) + assert.Equal(t, "owner", remotes[0].Owner) + assert.Equal(t, "repo", remotes[0].Repo) + }) + + t.Run("invalid fetch URL falls back to push URL", func(t *testing.T) { + fetchURL, err := url.Parse("ssh://git@github.com-work/") + if err != nil { + t.Fatal(err) + } + pushURL, err := url.Parse("ssh://git@github.com-work/owner/repo.git") + if err != nil { + t.Fatal(err) + } + remotes := git.RemoteSet{&git.Remote{ + Name: "origin", + FetchURL: fetchURL, + PushURL: pushURL, + Host: "github.com-work", + }} + + translateRemotes(remotes, translate) + + assert.Equal(t, "github.com", remotes[0].Host) + assert.Equal(t, "owner", remotes[0].Owner) + assert.Equal(t, "repo", remotes[0].Repo) + }) +} + func TestParse(t *testing.T) { testutils.StubConfig(t, "") From 0a8aabdd0b27731cacd7ce6130b8b02c540fee7d Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 8 Oct 2026 15:49:10 +0200 Subject: [PATCH 2/5] Align Current() with gh for last push URL Drop a remote's parse-time identity when neither its fetch URL nor its last push URL names a repository, matching gh's TranslateRemotes. Replace the translateRemotes unit tests with Current() scenarios for every acceptance criterion in #310, using the test binary as a fake `ssh -G` so SSH host aliases are exercised on all platforms. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/repository/repository.go | 13 +- pkg/repository/repository_test.go | 252 +++++++++++++++++++++++------- 2 files changed, 207 insertions(+), 58 deletions(-) diff --git a/pkg/repository/repository.go b/pkg/repository/repository.go index 1ea663f..80e1bff 100644 --- a/pkg/repository/repository.go +++ b/pkg/repository/repository.go @@ -169,17 +169,22 @@ func Current() (Repository, error) { func translateRemotes(remotes git.RemoteSet, translate func(*url.URL) *url.URL) { for _, remote := range remotes { - hasFetchInfo := false + hasRepoInfo := false if remote.FetchURL != nil { remote.FetchURL = translate(remote.FetchURL) - hasFetchInfo = updateRemoteInfo(remote, remote.FetchURL) + hasRepoInfo = updateRemoteInfo(remote, remote.FetchURL) } if remote.PushURL != nil { remote.PushURL = translate(remote.PushURL) - if !hasFetchInfo { - updateRemoteInfo(remote, remote.PushURL) + if !hasRepoInfo { + hasRepoInfo = updateRemoteInfo(remote, remote.PushURL) } } + // Like gh, a remote whose URLs don't name a repository has no identity, + // even if an earlier push URL did when the remote was parsed. + if !hasRepoInfo { + remote.Host, remote.Owner, remote.Repo = "", "", "" + } } } diff --git a/pkg/repository/repository_test.go b/pkg/repository/repository_test.go index 2b78e1a..63c0a31 100644 --- a/pkg/repository/repository_test.go +++ b/pkg/repository/repository_test.go @@ -1,7 +1,12 @@ package repository import ( - "net/url" + "fmt" + "io" + "os" + "path/filepath" + "runtime" + "strings" "testing" "github.com/cli/go-gh/v2/internal/git" @@ -10,59 +15,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestTranslateRemotesRefreshesRepositoryInfo(t *testing.T) { - translate := func(u *url.URL) *url.URL { - translated := *u - translated.Host = "github.com" - return &translated - } - - t.Run("SSH host alias", func(t *testing.T) { - u, err := url.Parse("ssh://git@github.com-work/owner/repo.git") - if err != nil { - t.Fatal(err) - } - remotes := git.RemoteSet{&git.Remote{ - Name: "origin", - FetchURL: u, - Host: "github.com-work", - Owner: "owner", - Repo: "repo", - }} - - translateRemotes(remotes, translate) - - filtered := remotes.FilterByHosts([]string{"github.com"}) - assert.Len(t, filtered, 1) - assert.Equal(t, "github.com", remotes[0].Host) - assert.Equal(t, "owner", remotes[0].Owner) - assert.Equal(t, "repo", remotes[0].Repo) - }) - - t.Run("invalid fetch URL falls back to push URL", func(t *testing.T) { - fetchURL, err := url.Parse("ssh://git@github.com-work/") - if err != nil { - t.Fatal(err) - } - pushURL, err := url.Parse("ssh://git@github.com-work/owner/repo.git") - if err != nil { - t.Fatal(err) - } - remotes := git.RemoteSet{&git.Remote{ - Name: "origin", - FetchURL: fetchURL, - PushURL: pushURL, - Host: "github.com-work", - }} - - translateRemotes(remotes, translate) - - assert.Equal(t, "github.com", remotes[0].Host) - assert.Equal(t, "owner", remotes[0].Owner) - assert.Equal(t, "repo", remotes[0].Repo) - }) -} - func TestParse(t *testing.T) { testutils.StubConfig(t, "") @@ -299,3 +251,195 @@ hosts: assert.Equal(t, "parent-org", repository.Owner) assert.Equal(t, "example", repository.Name) } + +const noKnownHostRemoteErr = "unable to determine current repository, none of the git remotes configured for this repository point to a known GitHub host" + +func TestCurrentResolvesFetchURLThroughSSHHostAlias(t *testing.T) { + // Given a fetch URL that uses an SSH host alias for github.com + loggedInTo(t, "github.com") + sshConfigMapsHost(t, "github.com-work", "github.com") + inRepoWithOrigin(t, "git@github.com-work:acme/widgets.git") + + // When the current repository is resolved + repository, err := Current() + + // Then the repository is on the aliased host + require.NoError(t, err) + assert.Equal(t, "github.com", repository.Host) + assert.Equal(t, "acme", repository.Owner) + assert.Equal(t, "widgets", repository.Name) +} + +func TestCurrentPrefersFetchURLOverPushURL(t *testing.T) { + // Given fetch and push URLs that name different repositories + loggedInTo(t, "github.com") + inRepoWithOrigin(t, "https://github.com/acme/widgets", "https://github.com/my-user/widgets") + + // When the current repository is resolved + repository, err := Current() + + // Then the fetch URL's repository is used + require.NoError(t, err) + assert.Equal(t, "github.com", repository.Host) + assert.Equal(t, "acme", repository.Owner) + assert.Equal(t, "widgets", repository.Name) +} + +func TestCurrentFallsBackToPushURLWhenFetchURLIsLocalPath(t *testing.T) { + // Given a fetch URL that is a local path + loggedInTo(t, "github.com") + inRepoWithOrigin(t, "/srv/git/widgets.git", "https://github.com/acme/widgets") + + // When the current repository is resolved + repository, err := Current() + + // Then the push URL's repository is used + require.NoError(t, err) + assert.Equal(t, "github.com", repository.Host) + assert.Equal(t, "acme", repository.Owner) + assert.Equal(t, "widgets", repository.Name) +} + +func TestCurrentResolvesPushURLThroughSSHHostAlias(t *testing.T) { + // Given a local fetch path and a push URL that uses an SSH host alias for github.com + loggedInTo(t, "github.com") + sshConfigMapsHost(t, "github.com-work", "github.com") + inRepoWithOrigin(t, "/srv/git/widgets.git", "git@github.com-work:acme/widgets.git") + + // When the current repository is resolved + repository, err := Current() + + // Then the push URL's repository is on the aliased host + require.NoError(t, err) + assert.Equal(t, "github.com", repository.Host) + assert.Equal(t, "acme", repository.Owner) + assert.Equal(t, "widgets", repository.Name) +} + +func TestCurrentIgnoresPushURLWhenFetchURLIsOnUnknownHost(t *testing.T) { + // Given a fetch URL on a host I'm not logged in to and a push URL on one I am + loggedInTo(t, "github.com") + inRepoWithOrigin(t, "https://gitlab.com/acme/widgets", "https://github.com/acme/widgets") + + // When the current repository is resolved + _, err := Current() + + // Then the remote is not treated as pointing to a known host + require.EqualError(t, err, noKnownHostRemoteErr) +} + +func TestCurrentUsesLastPushURL(t *testing.T) { + // Given a local fetch path and several push URLs + loggedInTo(t, "github.com") + inRepoWithOrigin(t, "/srv/git/widgets.git", "https://github.com/acme/widgets", "https://github.com/backup/mirror") + + // When the current repository is resolved + repository, err := Current() + + // Then the last push URL's repository is used + require.NoError(t, err) + assert.Equal(t, "github.com", repository.Host) + assert.Equal(t, "backup", repository.Owner) + assert.Equal(t, "mirror", repository.Name) +} + +func TestCurrentIgnoresEarlierPushURLsWhenLastIsLocalPath(t *testing.T) { + // Given a local fetch path and a last push URL that is also a local path + loggedInTo(t, "github.com") + inRepoWithOrigin(t, "/srv/git/widgets.git", "https://github.com/acme/widgets", "/srv/git/mirror.git") + + // When the current repository is resolved + _, err := Current() + + // Then the earlier push URL is not used + require.EqualError(t, err, noKnownHostRemoteErr) +} + +func TestCurrentRejectsKnownHostThatSSHMapsToAnotherHost(t *testing.T) { + // Given the only known host is one that SSH config maps to an IP address + loggedInTo(t, "github.company.example") + sshConfigMapsHost(t, "github.company.example", "192.0.2.10") + inRepoWithOrigin(t, "git@github.company.example:acme/widgets.git") + + // When the current repository is resolved + _, err := Current() + + // Then the remote is not treated as pointing to a known host + require.EqualError(t, err, noKnownHostRemoteErr) +} + +// loggedInTo makes host the only known GitHub host, ignoring any host or token from the environment. +func loggedInTo(t *testing.T, host string) { + t.Helper() + for _, name := range []string{"GH_HOST", "GH_TOKEN", "GITHUB_TOKEN", "GH_ENTERPRISE_TOKEN", "GITHUB_ENTERPRISE_TOKEN"} { + t.Setenv(name, "") + } + testutils.StubConfig(t, fmt.Sprintf("hosts:\n %s:\n oauth_token: token\n", host)) +} + +// inRepoWithOrigin changes into a new git repository whose origin fetches from fetchURL and pushes to pushURLs in order. +func inRepoWithOrigin(t *testing.T, fetchURL string, pushURLs ...string) { + t.Helper() + t.Setenv("GH_REPO", "") + t.Chdir(t.TempDir()) + _, _, err := git.Exec("init", "--quiet") + require.NoError(t, err) + _, _, err = git.Exec("remote", "add", "origin", fetchURL) + require.NoError(t, err) + for _, pushURL := range pushURLs { + _, _, err = git.Exec("remote", "set-url", "--add", "--push", "origin", pushURL) + require.NoError(t, err) + } +} + +const fakeSSHHostnameEnv = "GO_GH_TEST_FAKE_SSH_HOSTNAME" + +// sshConfigMapsHost puts a fake ssh first on PATH whose "ssh -G" reports hostname for alias. +// The fake is this test binary, which TestMain runs as ssh when fakeSSHHostnameEnv is set. +func sshConfigMapsHost(t *testing.T, alias, hostname string) { + t.Helper() + dir := t.TempDir() + name := "ssh" + if runtime.GOOS == "windows" { + name += ".exe" + } + copyTestBinary(t, filepath.Join(dir, name)) + t.Setenv(fakeSSHHostnameEnv, alias+"="+hostname) + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +func copyTestBinary(t *testing.T, dst string) { + t.Helper() + src, err := os.Executable() + require.NoError(t, err) + in, err := os.Open(src) + require.NoError(t, err) + defer in.Close() + out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY, 0o755) + require.NoError(t, err) + _, err = io.Copy(out, in) + require.NoError(t, err) + require.NoError(t, out.Close()) +} + +func TestMain(m *testing.M) { + if mapping, ok := os.LookupEnv(fakeSSHHostnameEnv); ok { + runFakeSSH(mapping, os.Args[1:]) + os.Exit(0) + } + os.Exit(m.Run()) +} + +// runFakeSSH mimics "ssh -G HOST", printing the mapped hostname for the alias and HOST itself otherwise. +func runFakeSSH(mapping string, args []string) { + if len(args) != 2 || args[0] != "-G" { + fmt.Fprintf(os.Stderr, "fake ssh: unexpected arguments %q\n", args) + os.Exit(1) + } + host := args[1] + alias, hostname, _ := strings.Cut(mapping, "=") + if strings.EqualFold(host, alias) { + host = hostname + } + fmt.Printf("hostname %s\n", host) +} From 125442e6c297158523daad0820549c6eec15604b Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 8 Oct 2026 17:18:22 +0200 Subject: [PATCH 3/5] Make repository tests behavioural Test Current() through the shared fake ssh instead of translateRemotes, and isolate tests from GH_HOST. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/repository/repository_test.go | 331 +++++++++++++++--------------- 1 file changed, 160 insertions(+), 171 deletions(-) diff --git a/pkg/repository/repository_test.go b/pkg/repository/repository_test.go index 63c0a31..6c80bb4 100644 --- a/pkg/repository/repository_test.go +++ b/pkg/repository/repository_test.go @@ -2,11 +2,7 @@ package repository import ( "fmt" - "io" "os" - "path/filepath" - "runtime" - "strings" "testing" "github.com/cli/go-gh/v2/internal/git" @@ -16,48 +12,29 @@ import ( ) func TestParse(t *testing.T) { - testutils.StubConfig(t, "") - tests := []struct { - name string - input string - hostOverride string - wantOwner string - wantName string - wantHost string - wantErr string + name string + input string + wantHost string + wantOwner string + wantName string }{ { - name: "OWNER/REPO combo", + name: "OWNER/REPO uses github.com by default", input: "OWNER/REPO", wantHost: "github.com", wantOwner: "OWNER", wantName: "REPO", }, { - name: "too few elements", - input: "OWNER", - wantErr: `expected the "[HOST/]OWNER/REPO" format, got "OWNER"`, - }, - { - name: "too many elements", - input: "a/b/c/d", - wantErr: `expected the "[HOST/]OWNER/REPO" format, got "a/b/c/d"`, - }, - { - name: "blank value", - input: "a/", - wantErr: `expected the "[HOST/]OWNER/REPO" format, got "a/"`, - }, - { - name: "with hostname", + name: "HOST/OWNER/REPO", input: "example.org/OWNER/REPO", wantHost: "example.org", wantOwner: "OWNER", wantName: "REPO", }, { - name: "full URL", + name: "HTTPS URL", input: "https://example.org/OWNER/REPO.git", wantHost: "example.org", wantOwner: "OWNER", @@ -70,114 +47,148 @@ func TestParse(t *testing.T) { wantOwner: "OWNER", wantName: "REPO", }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Given no GH_HOST and no configured hosts + t.Setenv("GH_HOST", "") + testutils.StubConfig(t, "") + + // When the repository is parsed + r, err := Parse(tt.input) + + // Then its host, owner, and name are returned + require.NoError(t, err) + assert.Equal(t, tt.wantHost, r.Host) + assert.Equal(t, tt.wantOwner, r.Owner) + assert.Equal(t, tt.wantName, r.Name) + }) + } +} + +func TestParseWithGHHost(t *testing.T) { + tests := []struct { + name string + input string + wantHost string + }{ { - name: "OWNER/REPO with default host override", - input: "OWNER/REPO", - hostOverride: "override.com", - wantHost: "override.com", - wantOwner: "OWNER", - wantName: "REPO", + name: "OWNER/REPO uses GH_HOST", + input: "OWNER/REPO", + wantHost: "override.com", }, { - name: "HOST/OWNER/REPO with default host override", - input: "example.com/OWNER/REPO", - hostOverride: "override.com", - wantHost: "example.com", - wantOwner: "OWNER", - wantName: "REPO", + name: "HOST/OWNER/REPO ignores GH_HOST", + input: "example.com/OWNER/REPO", + wantHost: "example.com", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - t.Setenv("GH_CONFIG_DIR", "nonexistant") - if tt.hostOverride != "" { - t.Setenv("GH_HOST", tt.hostOverride) - } + // Given GH_HOST names a host + t.Setenv("GH_HOST", "override.com") + testutils.StubConfig(t, "") + + // When the repository is parsed r, err := Parse(tt.input) - if tt.wantErr != "" { - assert.EqualError(t, err, tt.wantErr) - return - } - assert.NoError(t, err) + + // Then the host comes from the input when it has one, and GH_HOST otherwise + require.NoError(t, err) assert.Equal(t, tt.wantHost, r.Host) - assert.Equal(t, tt.wantOwner, r.Owner) - assert.Equal(t, tt.wantName, r.Name) + assert.Equal(t, "OWNER", r.Owner) + assert.Equal(t, "REPO", r.Name) }) } } -func TestParse_hostFromConfig(t *testing.T) { - var cfgStr = ` +func TestParseUsesTheOnlyConfiguredHost(t *testing.T) { + // Given no GH_HOST and a single configured host + t.Setenv("GH_HOST", "") + testutils.StubConfig(t, ` hosts: enterprise.com: user: user2 oauth_token: yyyyyyyyyyyyyyyyyyyy git_protocol: https -` - testutils.StubConfig(t, cfgStr) +`) + + // When a repository without a host is parsed r, err := Parse("OWNER/REPO") - assert.NoError(t, err) + + // Then the configured host is used + require.NoError(t, err) assert.Equal(t, "enterprise.com", r.Host) assert.Equal(t, "OWNER", r.Owner) assert.Equal(t, "REPO", r.Name) } -func TestParseWithHost(t *testing.T) { +func TestParseRejectsMalformedRepository(t *testing.T) { tests := []struct { - name string - input string - host string - wantOwner string - wantName string - wantHost string - wantErr string + name string + input string + wantErr string }{ - { - name: "OWNER/REPO combo", - input: "OWNER/REPO", - host: "github.com", - wantHost: "github.com", - wantOwner: "OWNER", - wantName: "REPO", - }, { name: "too few elements", input: "OWNER", - host: "github.com", wantErr: `expected the "[HOST/]OWNER/REPO" format, got "OWNER"`, }, { name: "too many elements", input: "a/b/c/d", - host: "github.com", wantErr: `expected the "[HOST/]OWNER/REPO" format, got "a/b/c/d"`, }, { name: "blank value", input: "a/", - host: "github.com", wantErr: `expected the "[HOST/]OWNER/REPO" format, got "a/"`, }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // When a malformed repository is parsed + _, err := Parse(tt.input) + + // Then the expected format is explained + require.EqualError(t, err, tt.wantErr) + }) + } +} + +func TestParseWithHost(t *testing.T) { + tests := []struct { + name string + input string + wantHost string + wantOwner string + wantName string + }{ { - name: "with hostname", + name: "OWNER/REPO uses the given host", + input: "OWNER/REPO", + wantHost: "ghe.example", + wantOwner: "OWNER", + wantName: "REPO", + }, + { + name: "HOST/OWNER/REPO ignores the given host", input: "example.org/OWNER/REPO", - host: "github.com", wantHost: "example.org", wantOwner: "OWNER", wantName: "REPO", }, { - name: "full URL", + name: "HTTPS URL ignores the given host", input: "https://example.org/OWNER/REPO.git", - host: "github.com", wantHost: "example.org", wantOwner: "OWNER", wantName: "REPO", }, { - name: "SSH URL", + name: "SSH URL ignores the given host", input: "git@example.org:OWNER/REPO.git", - host: "github.com", wantHost: "example.org", wantOwner: "OWNER", wantName: "REPO", @@ -185,12 +196,13 @@ func TestParseWithHost(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - r, err := ParseWithHost(tt.input, tt.host) - if tt.wantErr != "" { - assert.EqualError(t, err, tt.wantErr) - return - } - assert.NoError(t, err) + t.Parallel() + + // When the repository is parsed with a fallback host + r, err := ParseWithHost(tt.input, "ghe.example") + + // Then its host, owner, and name are returned + require.NoError(t, err) assert.Equal(t, tt.wantHost, r.Host) assert.Equal(t, tt.wantOwner, r.Owner) assert.Equal(t, tt.wantName, r.Name) @@ -198,23 +210,48 @@ func TestParseWithHost(t *testing.T) { } } +func TestParseWithHostRejectsMalformedRepository(t *testing.T) { + tests := []struct { + name string + input string + wantErr string + }{ + { + name: "too few elements", + input: "OWNER", + wantErr: `expected the "[HOST/]OWNER/REPO" format, got "OWNER"`, + }, + { + name: "too many elements", + input: "a/b/c/d", + wantErr: `expected the "[HOST/]OWNER/REPO" format, got "a/b/c/d"`, + }, + { + name: "blank value", + input: "a/", + wantErr: `expected the "[HOST/]OWNER/REPO" format, got "a/"`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // When a malformed repository is parsed with a fallback host + _, err := ParseWithHost(tt.input, "github.com") + + // Then the expected format is explained + require.EqualError(t, err, tt.wantErr) + }) + } +} + func TestCurrentUsesResolvedBaseRemote(t *testing.T) { // Given a higher-ranked fork remote and a parent remote selected by gh - t.Setenv("GH_REPO", "") - testutils.StubConfig(t, ` -hosts: - github.com: - oauth_token: token -`) - t.Chdir(t.TempDir()) - _, _, err := git.Exec("init", "--quiet") - require.NoError(t, err) - _, _, err = git.Exec("remote", "add", "origin", "git@github.com:parent-org/example.git") - require.NoError(t, err) - _, _, err = git.Exec("remote", "add", "github", "git@github.com:my-user/example.git") - require.NoError(t, err) - _, _, err = git.Exec("config", "remote.origin.gh-resolved", "base") - require.NoError(t, err) + loggedInTo(t, "github.com") + inNewRepo(t) + runGit(t, "remote", "add", "origin", "https://github.com/parent-org/example.git") + runGit(t, "remote", "add", "github", "https://github.com/my-user/example.git") + runGit(t, "config", "remote.origin.gh-resolved", "base") // When the current repository is resolved repository, err := Current() @@ -228,19 +265,9 @@ hosts: func TestCurrentUsesExplicitResolvedRepository(t *testing.T) { // Given a remote whose gh resolution names a repository on a different host - t.Setenv("GH_REPO", "") - testutils.StubConfig(t, ` -hosts: - github.com: - oauth_token: token -`) - t.Chdir(t.TempDir()) - _, _, err := git.Exec("init", "--quiet") - require.NoError(t, err) - _, _, err = git.Exec("remote", "add", "origin", "git@github.com:my-user/example.git") - require.NoError(t, err) - _, _, err = git.Exec("config", "remote.origin.gh-resolved", "ghe.example/parent-org/example") - require.NoError(t, err) + loggedInTo(t, "github.com") + inRepoWithOrigin(t, "https://github.com/my-user/example.git") + runGit(t, "config", "remote.origin.gh-resolved", "ghe.example/parent-org/example") // When the current repository is resolved repository, err := Current() @@ -257,7 +284,7 @@ const noKnownHostRemoteErr = "unable to determine current repository, none of th func TestCurrentResolvesFetchURLThroughSSHHostAlias(t *testing.T) { // Given a fetch URL that uses an SSH host alias for github.com loggedInTo(t, "github.com") - sshConfigMapsHost(t, "github.com-work", "github.com") + testutils.StubSSH(t, map[string]testutils.SSHResponse{"github.com-work": testutils.SSHReportsHostname("github.com")}) inRepoWithOrigin(t, "git@github.com-work:acme/widgets.git") // When the current repository is resolved @@ -303,7 +330,7 @@ func TestCurrentFallsBackToPushURLWhenFetchURLIsLocalPath(t *testing.T) { func TestCurrentResolvesPushURLThroughSSHHostAlias(t *testing.T) { // Given a local fetch path and a push URL that uses an SSH host alias for github.com loggedInTo(t, "github.com") - sshConfigMapsHost(t, "github.com-work", "github.com") + testutils.StubSSH(t, map[string]testutils.SSHResponse{"github.com-work": testutils.SSHReportsHostname("github.com")}) inRepoWithOrigin(t, "/srv/git/widgets.git", "git@github.com-work:acme/widgets.git") // When the current repository is resolved @@ -358,7 +385,7 @@ func TestCurrentIgnoresEarlierPushURLsWhenLastIsLocalPath(t *testing.T) { func TestCurrentRejectsKnownHostThatSSHMapsToAnotherHost(t *testing.T) { // Given the only known host is one that SSH config maps to an IP address loggedInTo(t, "github.company.example") - sshConfigMapsHost(t, "github.company.example", "192.0.2.10") + testutils.StubSSH(t, map[string]testutils.SSHResponse{"github.company.example": testutils.SSHReportsHostname("192.0.2.10")}) inRepoWithOrigin(t, "git@github.company.example:acme/widgets.git") // When the current repository is resolved @@ -377,69 +404,31 @@ func loggedInTo(t *testing.T, host string) { testutils.StubConfig(t, fmt.Sprintf("hosts:\n %s:\n oauth_token: token\n", host)) } -// inRepoWithOrigin changes into a new git repository whose origin fetches from fetchURL and pushes to pushURLs in order. -func inRepoWithOrigin(t *testing.T, fetchURL string, pushURLs ...string) { +// inNewRepo changes into a new git repository with no remotes, ignoring any GH_REPO from the environment. +func inNewRepo(t *testing.T) { t.Helper() t.Setenv("GH_REPO", "") t.Chdir(t.TempDir()) - _, _, err := git.Exec("init", "--quiet") - require.NoError(t, err) - _, _, err = git.Exec("remote", "add", "origin", fetchURL) - require.NoError(t, err) - for _, pushURL := range pushURLs { - _, _, err = git.Exec("remote", "set-url", "--add", "--push", "origin", pushURL) - require.NoError(t, err) - } + runGit(t, "init", "--quiet") } -const fakeSSHHostnameEnv = "GO_GH_TEST_FAKE_SSH_HOSTNAME" - -// sshConfigMapsHost puts a fake ssh first on PATH whose "ssh -G" reports hostname for alias. -// The fake is this test binary, which TestMain runs as ssh when fakeSSHHostnameEnv is set. -func sshConfigMapsHost(t *testing.T, alias, hostname string) { +// inRepoWithOrigin changes into a new git repository whose origin fetches from fetchURL and pushes to pushURLs in order. +func inRepoWithOrigin(t *testing.T, fetchURL string, pushURLs ...string) { t.Helper() - dir := t.TempDir() - name := "ssh" - if runtime.GOOS == "windows" { - name += ".exe" + inNewRepo(t) + runGit(t, "remote", "add", "origin", fetchURL) + for _, pushURL := range pushURLs { + runGit(t, "remote", "set-url", "--add", "--push", "origin", pushURL) } - copyTestBinary(t, filepath.Join(dir, name)) - t.Setenv(fakeSSHHostnameEnv, alias+"="+hostname) - t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) } -func copyTestBinary(t *testing.T, dst string) { +func runGit(t *testing.T, args ...string) { t.Helper() - src, err := os.Executable() - require.NoError(t, err) - in, err := os.Open(src) - require.NoError(t, err) - defer in.Close() - out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY, 0o755) + _, _, err := git.Exec(args...) require.NoError(t, err) - _, err = io.Copy(out, in) - require.NoError(t, err) - require.NoError(t, out.Close()) } func TestMain(m *testing.M) { - if mapping, ok := os.LookupEnv(fakeSSHHostnameEnv); ok { - runFakeSSH(mapping, os.Args[1:]) - os.Exit(0) - } + testutils.RunFakeSSHIfRequested() os.Exit(m.Run()) } - -// runFakeSSH mimics "ssh -G HOST", printing the mapped hostname for the alias and HOST itself otherwise. -func runFakeSSH(mapping string, args []string) { - if len(args) != 2 || args[0] != "-G" { - fmt.Fprintf(os.Stderr, "fake ssh: unexpected arguments %q\n", args) - os.Exit(1) - } - host := args[1] - alias, hostname, _ := strings.Cut(mapping, "=") - if strings.EqualFold(host, alias) { - host = hostname - } - fmt.Printf("hostname %s\n", host) -} From 1117f898975a6920b4eab909a97a2329236da6fa Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 8 Oct 2026 17:46:07 +0200 Subject: [PATCH 4/5] Inline remote translation into Current() Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/repository/repository.go | 53 ++++++++++++++---------------------- 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/pkg/repository/repository.go b/pkg/repository/repository.go index 80e1bff..f3375b1 100644 --- a/pkg/repository/repository.go +++ b/pkg/repository/repository.go @@ -134,7 +134,26 @@ func Current() (Repository, error) { } translator := ssh.NewTranslator() - translateRemotes(remotes, translator.Translate) + for _, remote := range remotes { + if remote.FetchURL != nil { + remote.FetchURL = translator.Translate(remote.FetchURL) + } + if remote.PushURL != nil { + remote.PushURL = translator.Translate(remote.PushURL) + } + // Like gh, a remote whose URLs don't name a repository has no identity, + // even if an earlier push URL did when the remote was parsed. + remote.Host, remote.Owner, remote.Repo = "", "", "" + for _, u := range []*url.URL{remote.FetchURL, remote.PushURL} { + if u == nil { + continue + } + if host, owner, repo, err := git.RepoInfoFromURL(u); err == nil { + remote.Host, remote.Owner, remote.Repo = host, owner, repo + break + } + } + } hosts := auth.KnownHosts() @@ -166,35 +185,3 @@ func Current() (Repository, error) { return r, nil } - -func translateRemotes(remotes git.RemoteSet, translate func(*url.URL) *url.URL) { - for _, remote := range remotes { - hasRepoInfo := false - if remote.FetchURL != nil { - remote.FetchURL = translate(remote.FetchURL) - hasRepoInfo = updateRemoteInfo(remote, remote.FetchURL) - } - if remote.PushURL != nil { - remote.PushURL = translate(remote.PushURL) - if !hasRepoInfo { - hasRepoInfo = updateRemoteInfo(remote, remote.PushURL) - } - } - // Like gh, a remote whose URLs don't name a repository has no identity, - // even if an earlier push URL did when the remote was parsed. - if !hasRepoInfo { - remote.Host, remote.Owner, remote.Repo = "", "", "" - } - } -} - -func updateRemoteInfo(remote *git.Remote, u *url.URL) bool { - host, owner, repo, err := git.RepoInfoFromURL(u) - if err != nil { - return false - } - remote.Host = host - remote.Owner = owner - remote.Repo = repo - return true -} From 393ceb0de625637679c4cb815f1b9925af43a875 Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 8 Oct 2026 17:55:45 +0200 Subject: [PATCH 5/5] Move remote identity resolution onto git.Remote Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- internal/git/remote.go | 52 +++++++++++++++++++++++++++--------- pkg/repository/repository.go | 23 +--------------- 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/internal/git/remote.go b/internal/git/remote.go index c969f36..a486f47 100644 --- a/internal/git/remote.go +++ b/internal/git/remote.go @@ -65,6 +65,42 @@ func (rs RemoteSet) FilterByHosts(hosts []string) RemoteSet { return filtered } +// Translate rewrites the URLs of every remote and re-derives each remote's +// repository identity from the result. +func (rs RemoteSet) Translate(translate func(*url.URL) *url.URL) { + for _, r := range rs { + r.Translate(translate) + } +} + +// Translate rewrites the remote's URLs and re-derives its repository identity +// from the result. +func (r *Remote) Translate(translate func(*url.URL) *url.URL) { + if r.FetchURL != nil { + r.FetchURL = translate(r.FetchURL) + } + if r.PushURL != nil { + r.PushURL = translate(r.PushURL) + } + r.resolveIdentity() +} + +// resolveIdentity sets the remote's repository from its fetch URL, falling back +// to its push URL. Like gh, a remote whose URLs don't name a repository has no +// identity. +func (r *Remote) resolveIdentity() { + r.Host, r.Owner, r.Repo = "", "", "" + for _, u := range []*url.URL{r.FetchURL, r.PushURL} { + if u == nil { + continue + } + if host, owner, repo, err := RepoInfoFromURL(u); err == nil { + r.Host, r.Owner, r.Repo = host, owner, repo + return + } + } +} + func listRemotes() ([]string, error) { stdOut, _, err := Exec("remote", "-v") if err != nil { @@ -88,7 +124,6 @@ func parseRemotes(gitRemotes []string) RemoteSet { if err != nil { continue } - host, owner, repo, _ := RepoInfoFromURL(url) var rem *Remote if len(remotes) > 0 { @@ -105,22 +140,13 @@ func parseRemotes(gitRemotes []string) RemoteSet { switch urlType { case "fetch": rem.FetchURL = url - rem.Host = host - rem.Owner = owner - rem.Repo = repo case "push": rem.PushURL = url - if rem.Host == "" { - rem.Host = host - } - if rem.Owner == "" { - rem.Owner = owner - } - if rem.Repo == "" { - rem.Repo = repo - } } } + for _, rem := range remotes { + rem.resolveIdentity() + } return remotes } diff --git a/pkg/repository/repository.go b/pkg/repository/repository.go index f3375b1..4f0521c 100644 --- a/pkg/repository/repository.go +++ b/pkg/repository/repository.go @@ -5,7 +5,6 @@ package repository import ( "errors" "fmt" - "net/url" "os" "strings" @@ -133,27 +132,7 @@ func Current() (Repository, error) { return r, errors.New("unable to determine current repository, no git remotes configured for this repository") } - translator := ssh.NewTranslator() - for _, remote := range remotes { - if remote.FetchURL != nil { - remote.FetchURL = translator.Translate(remote.FetchURL) - } - if remote.PushURL != nil { - remote.PushURL = translator.Translate(remote.PushURL) - } - // Like gh, a remote whose URLs don't name a repository has no identity, - // even if an earlier push URL did when the remote was parsed. - remote.Host, remote.Owner, remote.Repo = "", "", "" - for _, u := range []*url.URL{remote.FetchURL, remote.PushURL} { - if u == nil { - continue - } - if host, owner, repo, err := git.RepoInfoFromURL(u); err == nil { - remote.Host, remote.Owner, remote.Repo = host, owner, repo - break - } - } - } + remotes.Translate(ssh.NewTranslator().Translate) hosts := auth.KnownHosts()