fix: refresh repository identity after SSH translation by benekuehn · Pull Request #274 · cli/go-gh

benekuehn · 2026-07-30T06:41:35Z

Fixes #310

Description

Some people point a git remote at an SSH host alias from their SSH config, like [email protected]:cli/go-gh.git. repository.Current() already runs ssh -G to translate the alias to its real host. However, it kept the Host, Owner and Repo it parsed from the untranslated URL.

So the remote looked like it pointed at github.com-work. That isn't a known host, and Current() failed. Extensions like gh stack hit this in github/gh-stack#45 and github/gh-stack#337.

This PR refreshes those fields after translation. It also makes Current() pick a URL the way gh does when push URLs are configured.

gh reads only the last push URL, so earlier push URLs are never used. The acceptance criteria on #310 list each case.

How did you test this change?

I created a Go extension with gh extension create, which uses go-gh v2.16.1. It adds a github.com-work alias to the SSH config and runs the extension in a repository whose remote uses that alias. Then it points the extension at this branch with a replace directive, rebuilds, and runs it again.

It ran with a temporary HOME. ssh on PATH there is a wrapper that runs /usr/bin/ssh -F "$HOME/.ssh/config", because real ssh finds the home directory through the user database, not $HOME. The recording was built from 1117f89. The only later commit, 393ceb0, moves code without changing behaviour.

Key points

Anyone with several push URLs may see a different repository. I chose to match gh so extensions and gh agree on the current repository. It is kind of breaking, but not so much that I really care. Let someone complain.
A known host that SSH maps somewhere else, like an IP address, is rejected. This also matches gh.

Notes for reviewers

Start with Current() in pkg/repository/repository.go and RemoteSet.Translate in internal/git/remote.go. Then read pkg/repository/repository_test.go. It has one Current() test per acceptance criterion on #310.

The first commit is the original fix by @benekuehn. The later commits align push URL handling with gh, rewrite the tests, and move identity resolution onto git.Remote.

Authorship and follow-up

Who wrote this:

A human wrote it.
An agent wrote it under close human direction.
An agent wrote it independently, and no human has guided the implementation beyond the initial prompt.

Who answers review comments:

@williammartin will read and reply directly.
An agent will draft replies and @username will read them before they are posted.
Nobody has explicitly committed to replying.

RishabJaiswal · 2026-08-03T08:10:13Z

Request to merge this urgently

Quite a common usecase for users with multiple github accounts

Copilot

Pull request overview

This PR fixes incorrect remote host/owner/repo identity caching when SSH hostname aliases are translated, ensuring host-based filtering works against the post-translation canonical hostname (e.g., github.com).

Changes:

Refactors SSH URL translation into translateRemotes and refreshes Remote{Host,Owner,Repo} after translation.
Preserves fetch URL metadata precedence, falling back to push URL metadata only when fetch repo info cannot be derived.
Adds unit tests covering SSH alias translation and the fetch-to-push fallback behavior.

Show a summary per file

File	Description
pkg/repository/repository.go	Translates remotes via a helper and refreshes cached remote identity fields after SSH alias resolution, with fetch-first fallback semantics.
pkg/repository/repository_test.go	Adds tests validating remote identity refresh after translation and fallback to push URL when fetch repo info is invalid.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 2/2 changed files
Comments generated: 0
Review effort level: Lite

benekuehn · 2026-10-06T13:08:52Z

@williammartin any update?

williammartin · 2026-10-07T11:08:46Z

Hey @benekuehn, sorry, let me take a look today. I also just stumbled across your comment on github/gh-stack#45

Copilot

Copilot review overview

🟢 Approval recommended

The focused fix has no blocking issues; additional fetch-precedence coverage is optional.

Review effort: Balanced
Findings: 1

Open (1)

Add test coverage for valid push/fetch URL precedence · New

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.

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 cli#310, using the test binary as a fake `ssh -G` so SSH host aliases are exercised on all platforms. Co-authored-by: Copilot App <[email protected]>

Test Current() through the shared fake ssh instead of translateRemotes, and isolate tests from GH_HOST. Co-authored-by: Copilot App <[email protected]>

Copilot

🟢 Approval recommended

The focused identity refresh preserves the intended precedence and fallback behavior, with targeted regression coverage and no unresolved issues.

0 open findings

1 resolved since last review

Add test coverage for valid push/fetch URL precedence

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Co-authored-by: Copilot App <[email protected]>

williammartin · 2026-10-08T16:06:35Z

@benekuehn I made some more changes here so I've requested a review from @babakks. Hopefully we can get this released and then into a gh-stack release tomorrow.

williammartin · 2026-10-08T15:58:44Z

 		}
 	}
+	for _, rem := range remotes {
+		rem.resolveIdentity()


This is just a cleanup that allows the Remotes/parseRemotes and the Translate to share the same

williammartin · 2026-10-08T16:05:43Z

+
+// 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) {


Moved translation here rather than keeping it in Current because the pattern exhibited a bit of https://refactoring.guru/smells/feature-envy

babakks

LGTM. Just one question/callout.

babakks · 2026-10-09T08:43:05Z

+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
+		}
+	}
+}


question: I know this is meant to simplify the old switch-statement on URL type, but I'm just a bit wondering about any potential edge cases, because it's not a 100% refactor in terms of behaviour (looking at the early return in the loop).

In this old switch-statement, any of Host, Owner, or Repo values that were not already populated from the fetch URL, would have a chance to be assigned from the push URL. So, a configuration like this (which is not actually correct, but technically possible):

fetchurl=https:///owner/repo pushurl=https://some.host/owner/repo

would result in a different state with this PR's changes: some.host is not picked up as opposed to the current implementation.

I know it's super unlikely for a user to have such a weird config, so I think we're fine shipping the changes. Just wanted to call out in case you think otherwise.

I don't think your specific example is right because https:///owner/repo will return an error from RepoInfoFromURL since there is no hostname.

https://a.host/owner/.git would probably be an issue. However, I think this is so weird, and would also diverge from gh that I'm not worried about.

Thanks for your diligence!

williammartin · 2026-10-09T16:00:23Z

@benekuehn merged, released, merged into gh-stack and released: https://github.com/github/gh-stack/releases/tag/v0.2.1

Hopefully works for you. Thanks for your help.

This was referenced Jul 30, 2026

Remapping hostname prevents using gh stack submit github/gh-stack#337

Closed

gh stack submit fails when remote uses a custom SSH host alias github/gh-stack#45

Closed

benekuehn marked this pull request as ready for review July 30, 2026 12:14

benekuehn requested a review from a team as a code owner July 30, 2026 12:14

benekuehn requested review from tidy-dev and removed request for a team July 30, 2026 12:14

williammartin requested a lite review from Copilot August 4, 2026 08:00

Copilot started reviewing on behalf of williammartin August 4, 2026 08:00

View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

aspiers mentioned this pull request Aug 29, 2026

repository.Current() ignores gh-resolved and selects the wrong repo when a remote is named "github" #286

Closed

williammartin requested review from williammartin and a balanced review from Copilot and removed request for tidy-dev October 7, 2026 14:28

Copilot started reviewing on behalf of williammartin October 7, 2026 14:29

View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

Comment thread pkg/repository/repository_test.go Outdated

edjeffreys mentioned this pull request Oct 8, 2026

Add gh pr share command cli/cli#14618

Open

williammartin mentioned this pull request Oct 8, 2026

Make ssh tests behavioural with a shared fake ssh #311

Merged

williammartin force-pushed the fix/repository-ssh-alias branch from 3db916e to 7ab1165 Compare October 8, 2026 15:22

benekuehn and others added 3 commits October 8, 2026 17:33

fix: refresh repository identity after SSH translation

c04dd5f

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.

Make repository tests behavioural

125442e

Test Current() through the shared fake ssh instead of translateRemotes, and isolate tests from GH_HOST. Co-authored-by: Copilot App <[email protected]>

williammartin force-pushed the fix/repository-ssh-alias branch from 7ab1165 to 125442e Compare October 8, 2026 15:33

williammartin requested a balanced review from Copilot October 8, 2026 15:36

Copilot started reviewing on behalf of williammartin October 8, 2026 15:37

View session

Copilot AI reviewed Oct 8, 2026

View reviewed changes

williammartin and others added 2 commits October 8, 2026 17:46

Inline remote translation into Current()

1117f89

Co-authored-by: Copilot App <[email protected]>

Move remote identity resolution onto git.Remote

393ceb0

Co-authored-by: Copilot App <[email protected]>

williammartin requested a review from babakks October 8, 2026 16:06

williammartin reviewed Oct 8, 2026

View reviewed changes

babakks approved these changes Oct 9, 2026

View reviewed changes

williammartin merged commit aadf367 into cli:trunk Oct 9, 2026
7 checks passed

williammartin mentioned this pull request Oct 9, 2026

Fix repository recognition for SSH mapped configurations github/gh-stack#554

Merged

2 of 6 tasks

Conversation

benekuehn commented Jul 30, 2026 • edited by williammartin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How did you test this change?

Key points

Notes for reviewers

Authorship and follow-up

Uh oh!

RishabJaiswal commented Aug 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

benekuehn commented Oct 6, 2026

Uh oh!

williammartin commented Oct 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

williammartin commented Oct 8, 2026

Uh oh!

williammartin Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

williammartin Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

babakks left a comment

Choose a reason for hiding this comment

Uh oh!

babakks Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

williammartin Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

williammartin commented Oct 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

benekuehn commented Jul 30, 2026 •

edited by williammartin

Loading