Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
17 changes: 10 additions & 7 deletions docs/adr/41159-honor-gh-host-in-trial-repository-urls.md
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
# ADR-41159: Honor the active GitHub host for trial repository URLs

**Date**: 2026-06-24
**Status**: Draft
**Status**: Accepted
**Deciders**: pelikhan (PR author), gh-aw maintainers

---

### Context

`gh aw trial` (including `--clone-repo`) constructs several URLs for the host and source repositories: clone URLs, force-push remote URLs, displayed repository links, and Actions settings links. These were all hard-coded against `https://github.com/`, so trials run against a GitHub Enterprise Server (GHES) host failed even when `gh` was authenticated to the enterprise host. Worse, a fully-qualified GHES repo spec (`https://example.ghe.com/owner/repo`) was normalized to `owner/repo` and then rebuilt against public GitHub, silently targeting the wrong host. The codebase already exposes `getGitHubHost()`, which resolves the active host from `GITHUB_SERVER_URL` / `GITHUB_ENTERPRISE_HOST` / `GITHUB_HOST` / `GH_HOST`.
`gh aw trial` (including `--clone-repo`) constructs several URLs for the host and source repositories: clone URLs, force-push remote URLs, displayed repository links, and Actions settings links. These were all hard-coded against `https://github.com/`, so trials run against a GitHub Enterprise Server (GHES) host failed even when `gh` was authenticated to the enterprise host. Worse, a fully-qualified GHES repo spec (`https://example.ghe.com/owner/repo`) was normalized to `owner/repo` and then rebuilt against public GitHub, silently targeting the wrong host. Additionally, `parseIssueSpec` used a regex that hard-coded `https://github.com/` for matching `--trigger-context` issue URLs, so GHES issue URLs (e.g. `https://example.ghe.com/owner/repo/issues/123`) were silently ignored. The codebase already exposes `getGitHubHost()`, which resolves the active host from `GITHUB_SERVER_URL` / `GITHUB_ENTERPRISE_HOST` / `GITHUB_HOST` / `GH_HOST`.

### Decision

We will centralize all trial-mode repository URL construction on the active GitHub host. We introduce three small helpers in `pkg/cli/trial_repository.go` — `trialRepositoryURL`, `trialRepositoryGitURL`, and `trialRepositoryActionsSettingsURL` — that build URLs from `getGitHubHost()` plus the repo slug, and we route every previously hard-coded `https://github.com/...` string in the trial flow through them. Clone-mode repo specs continue to parse down to `owner/repo`, but clone/push/display operations now rebuild URLs against the resolved host.
We centralize all trial-mode repository URL construction on the active GitHub host. Three small helpers in `pkg/cli/trial_repository.go` — `trialRepositoryURL`, `trialRepositoryGitURL`, and `trialRepositoryActionsSettingsURL` — build URLs from `getGitHubHost()` plus the repo slug, and every previously hard-coded `https://github.com/...` string in the trial flow is routed through them. Clone-mode repo specs continue to parse down to `owner/repo`, but clone/push/display operations now rebuild URLs against the resolved host.

For `--trigger-context` issue URL parsing, we replace the hard-coded `issueURLPattern` regex with URL-based matching in `parseIssueSpec`: the URL host is compared against the normalized host from `getGitHubHost()`, so only issue URLs on the configured GitHub host (public or GHES) are accepted.

### Alternatives Considered

Expand All @@ -24,10 +26,15 @@ Replace each hard-coded literal with an inline `fmt.Sprintf("%s/%s.git", getGitH

Pass the resolved host (or a host-aware URL builder) down through `ensureTrialRepository`, `cloneTrialHostRepository`, and `cloneRepoContentsIntoHost` instead of reading it from ambient environment inside the helpers. This is more testable and explicit, but it widens several function signatures and is a larger change than the bug fix warrants. Rejected for scope; the helpers keep the existing `getGitHubHost()` resolution that the rest of the CLI already relies on.

#### Alternative 3: Build a dynamic `issueURLPattern` from `getGitHubHost()`

Compile the issue URL regex dynamically from the configured host on each call. Rejected because compiling a regex on each `parseIssueSpec` invocation is wasteful; URL parsing with `net/url` is both idiomatic and efficient.

### Consequences

#### Positive
- `gh aw trial --clone-repo` now works correctly against GHES hosts.
- `gh aw trial --trigger-context` now accepts issue URLs on the configured GHES host.
- A single source of truth for trial URL construction reduces the chance of a future hard-coded `github.com` regression.

#### Negative
Expand All @@ -37,7 +44,3 @@ Pass the resolved host (or a host-aware URL builder) down through `ensureTrialRe
#### Neutral
- Behavior on public GitHub is unchanged: with no host env set, `getGitHubHost()` resolves to `github.com`.
- The precedence order among `GITHUB_SERVER_URL`, `GITHUB_ENTERPRISE_HOST`, `GITHUB_HOST`, and `GH_HOST` is inherited unchanged from `getGitHubHost()`.

---

*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*
21 changes: 15 additions & 6 deletions pkg/cli/trial_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"net/url"
"os"
"os/exec"
"path/filepath"
Expand All @@ -22,8 +23,8 @@ import (
"github.com/github/gh-aw/pkg/workflow"
)

// issueURLPattern matches GitHub issue URLs like https://github.com/owner/repo/issues/123
var issueURLPattern = regexp.MustCompile(`https://github\.com/[^/]+/[^/]+/issues/(\d+)`)
// issuePathPattern matches the path portion of a GitHub issue URL: /owner/repo/issues/NUMBER
var issuePathPattern = regexp.MustCompile(`^/[^/]+/[^/]+/issues/(\d+)`)

// issueRefPattern matches issue references like #123
var issueRefPattern = regexp.MustCompile(`^#(\d+)$`)
Expand Down Expand Up @@ -236,15 +237,23 @@ func triggerWorkflowRun(repoSlug, workflowName string, triggerContext string, ve

// parseIssueSpec extracts the issue number from various formats
// Supports:
// - GitHub issue URLs: https://github.com/owner/repo/issues/123
// - GitHub issue URLs: https://github.com/owner/repo/issues/123 (public GitHub)
// - GitHub Enterprise issue URLs: https://example.ghe.com/owner/repo/issues/123 (GHES, respects GH_HOST)
// - Issue references: #123
// - Plain numbers: 123
func parseIssueSpec(input string) string {
input = strings.TrimSpace(input)

// First try to match GitHub issue URLs
if matches := issueURLPattern.FindStringSubmatch(input); len(matches) >= 2 {
return matches[1]
// First try to match GitHub issue URLs (supports public GitHub and GHES via GH_HOST).
// Both the scheme and host are compared against the configured GitHub host so that
// HTTP-only GHES instances and unrelated hosts are handled correctly.
if u, err := url.Parse(input); err == nil && u.Host != "" {
configuredHostURL, err := url.Parse(getGitHubHost())
Comment thread
pelikhan marked this conversation as resolved.
if err == nil && u.Scheme == configuredHostURL.Scheme && u.Host == configuredHostURL.Host {
if matches := issuePathPattern.FindStringSubmatch(u.Path); len(matches) >= 2 {
return matches[1]
}
}
}

// Try to match issue references like #123
Expand Down
63 changes: 63 additions & 0 deletions pkg/cli/trial_issue_mode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
)

func TestExtractIssueNumberFromURL(t *testing.T) {
// All of these cases run with the default host (no GH_HOST set), so
// getGitHubHost() returns https://github.com.
testCases := []struct {
name string
url string
Expand Down Expand Up @@ -64,6 +66,67 @@ func TestExtractIssueNumberFromURL(t *testing.T) {
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
// Ensure no GH_HOST is set so getGitHubHost() returns the public github.com default.
t.Setenv("GITHUB_SERVER_URL", "")
t.Setenv("GITHUB_ENTERPRISE_HOST", "")
t.Setenv("GITHUB_HOST", "")
t.Setenv("GH_HOST", "")
result := parseIssueSpec(tc.url)
if result != tc.expected {
t.Errorf("parseIssueSpec(%q) = %q, expected %q", tc.url, result, tc.expected)
}
})
}
}

func TestExtractIssueNumberFromURL_GHES(t *testing.T) {
// When GH_HOST points to a GitHub Enterprise Server, parseIssueSpec must accept
// issue URLs on that host and must reject github.com URLs (which belong to a
// different host in this configuration).
t.Setenv("GITHUB_SERVER_URL", "")
t.Setenv("GITHUB_ENTERPRISE_HOST", "")
t.Setenv("GITHUB_HOST", "")
t.Setenv("GH_HOST", "example.ghe.com")
Comment thread
pelikhan marked this conversation as resolved.

testCases := []struct {
name string
url string
expected string
}{
{
name: "GHES issue URL",
url: "https://example.ghe.com/owner/repo/issues/42",
expected: "42",
},
{
name: "GHES issue URL with query parameters",
url: "https://example.ghe.com/owner/repo/issues/99?tab=comments",
expected: "99",
},
{
name: "GHES issue URL with fragment",
url: "https://example.ghe.com/owner/repo/issues/7#issuecomment-1",
expected: "7",
},
{
name: "github.com issue URL rejected when GH_HOST is GHES",
url: "https://github.com/owner/repo/issues/123",
expected: "",
},
{
name: "other host rejected",
url: "https://gitlab.com/owner/repo/issues/123",
expected: "",
},
{
name: "GHES non-issue URL rejected",
url: "https://example.ghe.com/owner/repo/pulls/5",
expected: "",
},
}

for _, tc := range testCases {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] The inner test loop in TestExtractIssueNumberFromURL_GHES does not add per-subtest t.Setenv guards. If a future test running in parallel resets GH_HOST, the subtests here would silently use the wrong host.

💡 Suggested fix

Move the env guards from the outer function into each subtest — the same pattern used in TestExtractIssueNumberFromURL:

for _, tc := range testCases {
    t.Run(tc.name, func(t *testing.T) {
        t.Setenv("GITHUB_SERVER_URL", "")
        t.Setenv("GITHUB_ENTERPRISE_HOST", "")
        t.Setenv("GITHUB_HOST", "")
        t.Setenv("GH_HOST", "example.ghe.com")
        result := parseIssueSpec(tc.url)
        // ...
    })
}

This makes each subtest hermetic and safe if t.Parallel() is ever added.

@copilot please address this.

t.Run(tc.name, func(t *testing.T) {
result := parseIssueSpec(tc.url)
Expand Down
Loading