Conversation
…ommands Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…prove error message Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes incorrect API routing when GH_HOST points to a GitHub Enterprise instance but the workflow source URL explicitly targets a different host (e.g. github.com), ensuring add/add-wizard can fetch from the correct host.
Changes:
- Add explicit
Hosttracking toWorkflowSpecwhen parsing full URLs. - Make remote ref resolution and file fetching host-aware (REST client host override +
gh api --hostname). - Update call sites and tests to use the new host-aware behavior and the updated symlink-resolution signature.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/parser/remote_fetch.go | Threads explicit host through ref resolution and download paths; reworks symlink resolution to reuse a caller-provided REST client; adds host-specific exported helpers. |
| pkg/parser/import_remote_nested_test.go | Adjusts unit test call site for the new resolveRemoteSymlinks(*api.RESTClient, ...) signature. |
| pkg/cli/spec_test.go | Extends workflow spec parsing tests to assert host parsing from full URLs. |
| pkg/cli/spec.go | Adds Host to WorkflowSpec and captures hostname from parsed URLs (including mapping raw.githubusercontent.com → github.com). |
| pkg/cli/fetch.go | Switches remote workflow fetching to the new host-aware parser APIs based on spec.Host. |
Comments suppressed due to low confidence (1)
pkg/parser/remote_fetch.go:272
downloadIncludeFromWorkflowSpecstill hard-codes an empty host when resolving the ref for caching. With the new host-aware fetch path, this means workflows fetched from an explicit host (e.g.github.comwhileGH_HOSTpoints to GHE) can still fail later when they import remote files via workflowspec, because include downloads/ref resolution will be routed to the configured default host. Consider plumbing the parent workflow’s host throughResolveIncludePath/downloadIncludeFromWorkflowSpecand using the new host-awareresolveRefToSHA/downloadFileFromGitHubWithDepthvariants there as well.
// Resolve ref to SHA for cache lookup
var sha string
if cache != nil {
// Only resolve SHA if we're using the cache
resolvedSHA, err := resolveRefToSHA(owner, repo, ref, "")
if err != nil {
// SHA resolution failure (including auth errors) only means we cannot cache; the
// actual file download will be attempted below and may succeed via git fallback for
// public repositories. Do not propagate this error - just skip caching.
remoteLog.Printf("Failed to resolve ref to SHA, will skip cache: %v", err)
// Continue without caching if SHA resolution fails
} else {
sha = resolvedSHA
// Check cache using SHA
if cachedPath, found := cache.Get(owner, repo, filePath, sha); found {
remoteLog.Printf("Using cached import: %s/%s/%s@%s (SHA: %s)", owner, repo, filePath, ref, sha)
return cachedPath, nil
}
}
}
// Download the file content from GitHub
remoteLog.Printf("Fetching file from GitHub: %s/%s/%s@%s", owner, repo, filePath, ref)
content, err := downloadFileFromGitHub(owner, repo, filePath, ref)
if err != nil {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| if spec.Version != tt.wantVersion { | ||
| t.Errorf("parseWorkflowSpec() version = %q, want %q", spec.Version, tt.wantVersion) | ||
| } | ||
| if tt.wantHost != "" && spec.Host != tt.wantHost { |
| @@ -169,7 +168,7 @@ func parseGitHubURL(spec string) (*WorkflowSpec, error) { | |||
| return nil, err | |||
| } | |||
|
|
|||
| specLog.Printf("Parsed GitHub URL: owner=%s, repo=%s, ref=%s, path=%s", owner, repo, ref, filePath) | |||
| specLog.Printf("Parsed GitHub URL: owner=%s, repo=%s, ref=%s, path=%s, host=%s", owner, repo, ref, filePath, parsedURL.Host) | |||
|
|
|||
| // Ensure the file path ends with .md | |||
| if !strings.HasSuffix(filePath, ".md") { | |||
| @@ -181,13 +180,21 @@ func parseGitHubURL(spec string) (*WorkflowSpec, error) { | |||
| return nil, fmt.Errorf("invalid GitHub URL: '%s/%s' does not look like a valid GitHub repository", owner, repo) | |||
| } | |||
|
|
|||
| // For raw.githubusercontent.com content, the API host is github.com. | |||
| // For all other hosts (github.com, GHE), use the URL's host as-is. | |||
| host := parsedURL.Host | |||
| if host == "raw.githubusercontent.com" { | |||
| host = "github.com" | |||
| } | |||
|
|
|||
| apiPath := fmt.Sprintf("/repos/%s/%s/commits/%s", owner, repo, ref) | ||
| var args []string | ||
| if host != "" { | ||
| args = []string{"api", "--hostname", host, apiPath, "--jq", ".sha"} | ||
| } else { | ||
| args = []string{"api", apiPath, "--jq", ".sha"} | ||
| } | ||
| stdout, stderr, err := gh.Exec(args...) |
| } | ||
| return "", fmt.Errorf("failed to create REST client: %w", err) | ||
| if client == nil { | ||
| return "", fmt.Errorf("no REST client available for symlink resolution of %s/%s/%s@%s", owner, repo, filePath, ref) |
…re change Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in f1dfb80 — |
…henticated Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in a1fb96d — |
…like gitlab.com Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in e4b4aea — |
When
GH_HOSTpoints to a GHE instance (e.g.contoso-aw.ghe.com),add/add-wizardwith a fullgithub.comURL incorrectly routed all API calls to the GHE instance, producing HTTP 404.Root cause
api.DefaultRESTClient()andgh.Exec("api", ...)both honourGH_HOST. When the source URL explicitly names a different host, the default client targets the wrong endpoint.Changes
pkg/cli/spec.goHost stringtoWorkflowSpec— populated from the URL's hostname when a full URL is supplied.github.com-only restriction inparseGitHubURL; GHE URLs (e.g.https://myorg.ghe.com/owner/repo/blob/...) are now accepted and their host captured.pkg/parser/remote_fetch.gohost stringparameter through all internal fetch/resolve functions.host != "", useapi.NewRESTClient(api.ClientOptions{Host: host})instead ofapi.DefaultRESTClient(), and pass--hostname <host>togh.Exec("api", ...).downloadFileViaGitClone,resolveRefToSHAViaGit) build the remote URL from the explicit host rather thanGetGitHubHostForRepo.downloadFileViaGitskips theraw.githubusercontent.comshortcut for non-github.comhosts (GHE has no raw-content equivalent).resolveRemoteSymlinksnow accepts the already-created*api.RESTClientfrom the caller instead of creating its own — avoids a second client targeting the wrong host.DownloadFileFromGitHubForHostandResolveRefToSHAForHost.pkg/cli/fetch.gofetchRemoteWorkflowcalls the new host-aware functions whenspec.Hostis set.