From 301326ab3744dd9853bf3650e8e1131caed2b532 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 16:45:27 +0000 Subject: [PATCH 1/7] Initial plan From 7f6055881761acd4ce8d89a97a1131918906d07f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 17:05:02 +0000 Subject: [PATCH 2/7] refactor: split remote_fetch.go into focused modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split pkg/parser/remote_fetch.go (1553 lines) into 5 focused files: - remote_client.go: shared HTTP client utilities (publicAPIClient, createRESTClientForHost, buildContentsAPIPath, fetchRemoteFileContent, fetchPublicGitHubContentsAPI) - remote_resolve_path.go: include-path resolution & security validation (ResolveIncludePath and helpers) - remote_workflow_spec.go: WorkflowSpec parsing & caching (IsWorkflowSpec, downloadIncludeFromWorkflowSpec and helpers) - remote_resolve_sha.go: ref→SHA resolution chain with fallbacks (resolveRefToSHA, ResolveRefToSHAForHost and helpers) - remote_download_file.go: single-file download with auth/git/public fallbacks (DownloadFileFromGitHub, DownloadFileFromGitHubForHost and helpers) - remote_list_files.go: directory listing operations (ListWorkflowFiles, ListDirAllFilesForHost, etc.) All public APIs preserved, all tests pass, no logic changes. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/parser/remote_client.go | 88 ++ pkg/parser/remote_download_file.go | 416 ++++++++ pkg/parser/remote_fetch.go | 1553 ---------------------------- pkg/parser/remote_list_files.go | 575 ++++++++++ pkg/parser/remote_resolve_path.go | 192 ++++ pkg/parser/remote_resolve_sha.go | 178 ++++ pkg/parser/remote_workflow_spec.go | 175 ++++ 7 files changed, 1624 insertions(+), 1553 deletions(-) create mode 100644 pkg/parser/remote_client.go create mode 100644 pkg/parser/remote_download_file.go delete mode 100644 pkg/parser/remote_fetch.go create mode 100644 pkg/parser/remote_list_files.go create mode 100644 pkg/parser/remote_resolve_path.go create mode 100644 pkg/parser/remote_resolve_sha.go create mode 100644 pkg/parser/remote_workflow_spec.go diff --git a/pkg/parser/remote_client.go b/pkg/parser/remote_client.go new file mode 100644 index 00000000000..f6922aeebdc --- /dev/null +++ b/pkg/parser/remote_client.go @@ -0,0 +1,88 @@ +//go:build !js && !wasm + +package parser + +import ( + "context" + "fmt" + "io" + "net/http" + "net/url" + "strings" + + "github.com/cli/go-gh/v2/pkg/api" + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/logger" +) + +var remoteLog = logger.New("parser:remote_fetch") + +// publicAPIClient is a shared HTTP client used for unauthenticated GitHub API +// fallback calls. It carries a timeout to prevent indefinite hangs on slow or +// unresponsive hosts. +var publicAPIClient = &http.Client{Timeout: constants.DefaultHTTPClientTimeout} + +func createRESTClientForHost(host string) (*api.RESTClient, error) { + opts := api.ClientOptions{Timeout: constants.DefaultHTTPClientTimeout} + if host != "" { + opts.Host = host + } + return api.NewRESTClient(opts) +} + +func buildContentsAPIPath(owner, repo, path, ref string) string { + pathSegments := strings.Split(path, "/") + for i := range pathSegments { + pathSegments[i] = url.PathEscape(pathSegments[i]) + } + return fmt.Sprintf( + "repos/%s/%s/contents/%s?ref=%s", + owner, + repo, + strings.Join(pathSegments, "/"), + url.QueryEscape(ref), + ) +} + +func fetchRemoteFileContent(ctx context.Context, client *api.RESTClient, owner, repo, path, ref string, fileContent any) error { + return client.DoWithContext(ctx, http.MethodGet, buildContentsAPIPath(owner, repo, path, ref), nil, fileContent) +} + +// fetchPublicGitHubContentsAPI makes an unauthenticated GET request to the +// GitHub public REST API contents endpoint. This is used as a last-resort +// fallback when the current token (e.g. an enterprise SAML-enforced token) +// cannot access cross-organization public repositories and git clone also +// fails. Unauthenticated requests are subject to a lower rate limit +// (60 req/hour) but are sufficient for the handful of calls during update. +func fetchPublicGitHubContentsAPI(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { + // Encode each path segment independently so that '/' separators are + // preserved — url.PathEscape would turn them into '%2F', breaking nested + // paths like '.github/workflows/shared/foo.md'. + segments := strings.Split(path, "/") + encodedSegments := make([]string, len(segments)) + for i, s := range segments { + encodedSegments[i] = url.PathEscape(s) + } + endpoint := fmt.Sprintf("https://api.github.com/repos/%s/%s/contents/%s?ref=%s", + owner, repo, strings.Join(encodedSegments, "/"), url.QueryEscape(ref)) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + if err != nil { + return nil, err + } + req.Header.Set("Accept", "application/vnd.github+json") + + resp, err := publicAPIClient.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("GitHub API returned %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) + } + return body, nil +} diff --git a/pkg/parser/remote_download_file.go b/pkg/parser/remote_download_file.go new file mode 100644 index 00000000000..a8aa681f523 --- /dev/null +++ b/pkg/parser/remote_download_file.go @@ -0,0 +1,416 @@ +//go:build !js && !wasm + +package parser + +import ( + "context" + "encoding/base64" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "os/exec" + pathpkg "path" + "path/filepath" + "strings" + + "github.com/cli/go-gh/v2/pkg/api" + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/errorutil" + "github.com/github/gh-aw/pkg/fileutil" + "github.com/github/gh-aw/pkg/gitutil" +) + +// DownloadFileFromGitHub downloads a file from a GitHub repository using the GitHub API. +// This is the exported wrapper for downloadFileFromGitHub. +// Parameters: +// - owner: Repository owner (e.g., "github") +// - repo: Repository name (e.g., "gh-aw") +// - path: Path to the file within the repository (e.g., ".github/workflows/workflow.md") +// - ref: Git reference (branch, tag, or commit SHA) +// Returns the file content as bytes or an error if the file cannot be retrieved. +func DownloadFileFromGitHub(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { + return downloadFileFromGitHubWithDepth(ctx, owner, repo, path, ref, 0, "") +} + +// DownloadFileFromGitHubForHost downloads a file from a GitHub repository using the GitHub API, +// targeting a specific GitHub host. Use this when the target repository is on a different host +// than the one configured via GH_HOST (e.g., fetching from github.com while GH_HOST is a GHE instance). +// host is the hostname without scheme (e.g., "github.com", "myorg.ghe.com"). +// An empty host uses the default configured host (GH_HOST or github.com). +func DownloadFileFromGitHubForHost(ctx context.Context, owner, repo, path, ref, host string) ([]byte, error) { + return downloadFileFromGitHubWithDepth(ctx, owner, repo, path, ref, 0, host) +} + +func downloadFileFromGitHub(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { + return downloadFileFromGitHubWithDepth(ctx, owner, repo, path, ref, 0, "") +} + +func downloadFileFromGitHubWithDepth(ctx context.Context, owner, repo, path, ref string, symlinkDepth int, host string) ([]byte, error) { + client, err := createRESTClientForHost(host) + if err != nil { + if gitutil.IsAuthError(err.Error()) { + remoteLog.Printf("REST client creation failed due to auth error, attempting git fallback for %s/%s/%s@%s: %v", owner, repo, path, ref, err) + content, gitErr := downloadFileViaGit(ctx, owner, repo, path, ref, host) + if gitErr != nil { + remoteLog.Printf("Git fallback also failed for %s/%s/%s@%s: %v", owner, repo, path, ref, gitErr) + return nil, fmt.Errorf("failed to fetch file content: %w", err) + } + return content, nil + } + return nil, fmt.Errorf("failed to create REST client: %w", err) + } + + var fileContent struct { + Content string `json:"content"` + Encoding string `json:"encoding"` + Name string `json:"name"` + } + + err = fetchRemoteFileContent(ctx, client, owner, repo, path, ref, &fileContent) + if err != nil { + if gitutil.IsAuthError(err.Error()) { + remoteLog.Printf("GitHub API authentication failed, attempting git fallback for %s/%s/%s@%s", owner, repo, path, ref) + content, gitErr := downloadFileViaGit(ctx, owner, repo, path, ref, host) + if gitErr != nil { + if host == "" || host == "github.com" { + remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s/%s@%s", owner, repo, path, ref) + return downloadFileViaPublicAPI(ctx, owner, repo, path, ref) + } + return nil, fmt.Errorf("failed to fetch file content via GitHub API (auth error) and git fallback: API error: %w, Git error: %w", err, gitErr) + } + return content, nil + } + + if errorutil.IsNotFoundError(err) && symlinkDepth < constants.MaxSymlinkDepth { + if content, handled, resolveErr := retryDownloadViaResolvedSymlink(ctx, client, owner, repo, path, ref, symlinkDepth, host); handled { + return content, resolveErr + } + } + + return nil, fmt.Errorf("failed to fetch file content from %s/%s/%s@%s: %w", owner, repo, path, ref, err) + } + + if fileContent.Content == "" { + return nil, fmt.Errorf("empty content returned from GitHub API for %s/%s/%s@%s", owner, repo, path, ref) + } + + content, err := base64.StdEncoding.DecodeString(fileContent.Content) + if err != nil { + return nil, fmt.Errorf("failed to decode base64 content: %w", err) + } + + return content, nil +} + +// downloadFileViaPublicAPI downloads a file from a public GitHub repository +// using an unauthenticated API call. Used as a last-resort fallback when both +// authenticated API and git clone fail (e.g. enterprise SAML tokens). +func downloadFileViaPublicAPI(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { + remoteLog.Printf("Attempting unauthenticated public API download for %s/%s/%s@%s", owner, repo, path, ref) + body, err := fetchPublicGitHubContentsAPI(ctx, owner, repo, path, ref) + if err != nil { + return nil, fmt.Errorf("unauthenticated public API also failed for %s/%s/%s@%s: %w", owner, repo, path, ref, err) + } + + var fileContent struct { + Content string `json:"content"` + Encoding string `json:"encoding"` + } + if err := json.Unmarshal(body, &fileContent); err != nil { + return nil, fmt.Errorf("failed to parse public API file response: %w", err) + } + if fileContent.Content == "" { + return nil, fmt.Errorf("empty content returned from public API for %s/%s/%s@%s", owner, repo, path, ref) + } + + content, err := base64.StdEncoding.DecodeString(fileContent.Content) + if err != nil { + return nil, fmt.Errorf("failed to decode base64 content from public API: %w", err) + } + return content, nil +} + +func retryDownloadViaResolvedSymlink( + ctx context.Context, + client *api.RESTClient, + owner, repo, path, ref string, + symlinkDepth int, + host string, +) ([]byte, bool, error) { + remoteLog.Printf("File not found at %s/%s/%s@%s, checking for symlinks in path (depth: %d)", owner, repo, path, ref, symlinkDepth) + resolvedPath, resolveErr := resolveRemoteSymlinks(ctx, client, owner, repo, path, ref) + if resolveErr == nil && resolvedPath != path { + remoteLog.Printf("Retrying download with symlink-resolved path: %s -> %s", path, resolvedPath) + content, err := downloadFileFromGitHubWithDepth(ctx, owner, repo, resolvedPath, ref, symlinkDepth+1, host) + return content, true, err + } + return nil, false, nil +} + +// checkRemoteSymlink checks if a path in a remote GitHub repository is a symlink. +// Returns the symlink target and true if it is a symlink, or empty string and false otherwise. +// A nil error with false means the path is not a symlink (e.g., it's a directory or file). +func checkRemoteSymlink(ctx context.Context, client *api.RESTClient, owner, repo, dirPath, ref string) (string, bool, error) { + endpoint := buildContentsAPIPath(owner, repo, dirPath, ref) + remoteLog.Printf("Checking if path component is symlink: %s/%s/%s@%s", owner, repo, dirPath, ref) + + // The Contents API returns a JSON object for files/symlinks but a JSON array for directories. + // Decode into json.RawMessage first to distinguish these cases without error-driven control flow. + var raw json.RawMessage + err := client.DoWithContext(ctx, http.MethodGet, endpoint, nil, &raw) + if err != nil { + remoteLog.Printf("Contents API error for %s: %v", dirPath, err) + return "", false, err + } + + // If the response is an array, this is a directory listing — not a symlink + trimmed := strings.TrimSpace(string(raw)) + if trimmed != "" && trimmed[0] == '[' { + remoteLog.Printf("Path component %s is a directory (not a symlink)", dirPath) + return "", false, nil + } + + // Parse the object response to check the type + var result struct { + Type string `json:"type"` + Target string `json:"target"` + } + if err := json.Unmarshal(raw, &result); err != nil { + return "", false, fmt.Errorf("failed to parse contents response for %s: %w", dirPath, err) + } + + if result.Type == "symlink" && result.Target != "" { + remoteLog.Printf("Path component %s is a symlink -> %s", dirPath, result.Target) + return result.Target, true, nil + } + + remoteLog.Printf("Path component %s is type=%s (not a symlink)", dirPath, result.Type) + return "", false, nil +} + +// resolveRemoteSymlinks resolves symlinks in a remote GitHub repository path. +// The GitHub Contents API doesn't follow symlinks in path components. For example, +// if .github/workflows/shared is a symlink to ../../gh-agent-workflows/shared, +// fetching .github/workflows/shared/elastic-tools.md returns 404. +// This function walks the path components and resolves any symlinks found. +// The caller must provide a REST client (already authenticated for the correct host). +func resolveRemoteSymlinks(ctx context.Context, client *api.RESTClient, owner, repo, filePath, ref string) (string, error) { + parts := strings.Split(filePath, "/") + if len(parts) <= 1 { + return "", fmt.Errorf("no directory components to resolve in path: %s", filePath) + } + + if client == nil { + return "", fmt.Errorf("no REST client available for symlink resolution of %s/%s/%s@%s", owner, repo, filePath, ref) + } + + remoteLog.Printf("Attempting symlink resolution for %s/%s/%s@%s (%d path components)", owner, repo, filePath, ref, len(parts)) + + for i := 1; i < len(parts); i++ { + dirPath := strings.Join(parts[:i], "/") + resolvedPath, found, err := resolveRemoteSymlinkComponent(ctx, client, owner, repo, filePath, ref, parts, i, dirPath) + if err != nil { + return "", err + } + if found { + return resolvedPath, nil + } + } + + remoteLog.Printf("No symlinks found after checking all %d directory components of %s", len(parts)-1, filePath) + return "", fmt.Errorf("no symlinks found in path: %s", filePath) +} + +func resolveRemoteSymlinkComponent( + ctx context.Context, + client *api.RESTClient, + owner, repo, filePath, ref string, + parts []string, + index int, + dirPath string, +) (string, bool, error) { + target, isSymlink, err := checkRemoteSymlink(ctx, client, owner, repo, dirPath, ref) + if err != nil { + if errorutil.IsNotFoundError(err) { + remoteLog.Printf("Path component %s returned 404, skipping", dirPath) + return "", false, nil + } + return "", false, fmt.Errorf("failed to check path component %s for symlinks: %w", dirPath, err) + } + if !isSymlink { + return "", false, nil + } + parentDir := "" + if index > 1 { + parentDir = strings.Join(parts[:index-1], "/") + } + resolvedBase, err := resolveAndValidateRemoteSymlinkBase(parentDir, target, dirPath) + if err != nil { + return "", false, err + } + remaining := strings.Join(parts[index:], "/") + resolvedPath := resolvedBase + "/" + remaining + remoteLog.Printf("Resolved symlink in remote path: %s -> %s (full: %s -> %s)", dirPath, target, filePath, resolvedPath) + return resolvedPath, true, nil +} + +func resolveAndValidateRemoteSymlinkBase(parentDir, target, dirPath string) (string, error) { + remoteLog.Printf("Resolving symlink: component=%s target=%s parentDir=%s", dirPath, target, parentDir) + resolvedBase := pathpkg.Clean(target) + if parentDir != "" { + resolvedBase = pathpkg.Clean(pathpkg.Join(parentDir, target)) + } + remoteLog.Printf("Resolved base after path.Clean: %s", resolvedBase) + if resolvedBase == "" || resolvedBase == "." || pathpkg.IsAbs(resolvedBase) || strings.HasPrefix(resolvedBase, "..") { + remoteLog.Printf("Rejecting resolved base %q (escapes repository root)", resolvedBase) + return "", fmt.Errorf("symlink target %q at %s resolves outside repository root: %s", target, dirPath, resolvedBase) + } + return resolvedBase, nil +} + +// downloadFileViaGit downloads a file from a Git repository using git commands +// This is a fallback for when GitHub API authentication fails +func downloadFileViaGit(ctx context.Context, owner, repo, path, ref, host string) ([]byte, error) { + remoteLog.Printf("Attempting git fallback for %s/%s/%s@%s", owner, repo, path, ref) + + // First, try via raw.githubusercontent.com — no auth required for public repos and + // no dependency on git being installed. + // Only attempt raw URL for github.com repos (not GHE) since raw.githubusercontent.com + // only serves public GitHub content. + if host == "" || host == "github.com" { + content, rawErr := downloadFileViaRawURL(ctx, owner, repo, path, ref) + if rawErr == nil { + return content, nil + } + remoteLog.Printf("Raw URL download failed for %s/%s/%s@%s, trying git archive: %v", owner, repo, path, ref, rawErr) + } + + // Use git archive to get the file content without cloning + // This works for public repositories without authentication + var githubHost string + if host != "" { + githubHost = "https://" + host + } else { + githubHost = GetGitHubHostForRepo(owner, repo) + } + repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) + + // git archive command: git archive --remote= + // #nosec G204 -- repoURL, ref, and path are from workflow import configuration authored by the + // developer; exec.CommandContext with separate args (not shell execution) prevents shell injection. + cmd := exec.CommandContext(ctx, "git", "archive", "--remote="+repoURL, ref, path) + archiveOutput, err := cmd.Output() + if err != nil { + // If git archive fails, try with git clone + git show as a fallback + return downloadFileViaGitClone(owner, repo, path, ref, host) + } + + // Extract the file from the tar archive using Go's archive/tar (cross-platform) + content, err := fileutil.ExtractFileFromTar(archiveOutput, path) + if err != nil { + return nil, fmt.Errorf("failed to extract file from git archive: %w", err) + } + + remoteLog.Printf("Successfully downloaded file via git archive: %s/%s/%s@%s", owner, repo, path, ref) + return content, nil +} + +// downloadFileViaRawURL fetches a file using the raw.githubusercontent.com URL. +// This requires no authentication for public repositories and no git installation. +func downloadFileViaRawURL(ctx context.Context, owner, repo, filePath, ref string) ([]byte, error) { + rawURL := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s", owner, repo, ref, filePath) + remoteLog.Printf("Attempting raw URL download: %s", rawURL) + + // Use a client with a timeout to prevent indefinite hangs on slow/unresponsive hosts. + rawClient := &http.Client{Timeout: constants.DefaultHTTPClientTimeout} + + // #nosec G107 -- rawURL is constructed from workflow import configuration authored by + // the developer; the owner, repo, filePath, and ref are user-supplied workflow spec fields. + req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) + if err != nil { + return nil, fmt.Errorf("raw URL request failed for %s: %w", rawURL, err) + } + resp, err := rawClient.Do(req) + if err != nil { + return nil, fmt.Errorf("raw URL request failed for %s: %w", rawURL, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("raw URL returned HTTP %d for %s", resp.StatusCode, rawURL) + } + + content, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read raw URL response body for %s: %w", rawURL, err) + } + + remoteLog.Printf("Successfully downloaded file via raw URL: %s", rawURL) + return content, nil +} + +// downloadFileViaGitClone downloads a file by shallow cloning the repository +// This is used as a fallback when git archive doesn't work +func downloadFileViaGitClone(owner, repo, path, ref, host string) ([]byte, error) { + remoteLog.Printf("Attempting git clone fallback for %s/%s/%s@%s", owner, repo, path, ref) + + // Create a temporary directory for the shallow clone + tmpDir, err := os.MkdirTemp("", "gh-aw-git-clone-*") + if err != nil { + return nil, fmt.Errorf("failed to create temp directory: %w", err) + } + defer os.RemoveAll(tmpDir) + + var githubHost string + if host != "" { + githubHost = "https://" + host + } else { + githubHost = GetGitHubHostForRepo(owner, repo) + } + repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) + + // Check if ref is a SHA (40 hex characters) + isSHA := len(ref) == 40 && gitutil.IsHexString(ref) + + var cloneCmd *exec.Cmd + if isSHA { + // For SHA refs, we need to clone without --branch and then checkout the specific commit + // Clone with minimal depth and no branch specified + cloneCmd = exec.Command("git", "clone", "--depth", "1", "--no-single-branch", repoURL, tmpDir) + if output, err := cloneCmd.CombinedOutput(); err != nil { + // Try without --no-single-branch if the first attempt fails + remoteLog.Printf("Clone with --no-single-branch failed, trying full clone: %s", string(output)) + cloneCmd = exec.Command("git", "clone", repoURL, tmpDir) + if output, err := cloneCmd.CombinedOutput(); err != nil { + return nil, fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) + } + } + + // Now checkout the specific commit + checkoutCmd := exec.Command("git", "-C", tmpDir, "checkout", ref) + if output, err := checkoutCmd.CombinedOutput(); err != nil { + return nil, fmt.Errorf("failed to checkout commit %s: %w\nOutput: %s", ref, err, string(output)) + } + } else { + // For branch/tag refs, use --branch flag + cloneCmd = exec.Command("git", "clone", "--depth", "1", "--branch", ref, repoURL, tmpDir) + if output, err := cloneCmd.CombinedOutput(); err != nil { + return nil, fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) + } + } + + // Read the file from the cloned repository + filePath := filepath.Join(tmpDir, path) + if err := fileutil.ValidatePathWithinBase(tmpDir, filePath); err != nil { + return nil, fmt.Errorf("refusing to read file outside clone directory: %w", err) + } + content, err := os.ReadFile(filePath) + if err != nil { + return nil, fmt.Errorf("failed to read file from cloned repository: %w", err) + } + + remoteLog.Printf("Successfully downloaded file via git clone: %s/%s/%s@%s", owner, repo, path, ref) + return content, nil +} diff --git a/pkg/parser/remote_fetch.go b/pkg/parser/remote_fetch.go deleted file mode 100644 index 590cd06bd3e..00000000000 --- a/pkg/parser/remote_fetch.go +++ /dev/null @@ -1,1553 +0,0 @@ -//go:build !js && !wasm - -package parser - -import ( - "context" - "encoding/base64" - "encoding/json" - "errors" - "fmt" - "io" - "net/http" - "net/url" - "os" - "os/exec" - pathpkg "path" - "path/filepath" - "strings" - "sync" - - "github.com/cli/go-gh/v2" - "github.com/cli/go-gh/v2/pkg/api" - "github.com/github/gh-aw/pkg/constants" - "github.com/github/gh-aw/pkg/errorutil" - "github.com/github/gh-aw/pkg/fileutil" - "github.com/github/gh-aw/pkg/gitutil" - "github.com/github/gh-aw/pkg/logger" - "github.com/github/gh-aw/pkg/stringutil" -) - -var remoteLog = logger.New("parser:remote_fetch") - -// publicAPIClient is a shared HTTP client used for unauthenticated GitHub API -// fallback calls. It carries a timeout to prevent indefinite hangs on slow or -// unresponsive hosts. -var publicAPIClient = &http.Client{Timeout: constants.DefaultHTTPClientTimeout} - -// gitListCloneCache is a process-lifetime cache of shallow clones used by -// git-based directory listing fallbacks to avoid repeated clone operations for -// the same repository/ref tuple. Entries are not explicitly cleaned up because -// the CLI process is short-lived and temporary directories are OS-managed. -var gitListCloneCache = struct { - mu sync.Mutex - dirs map[string]string -}{ - dirs: make(map[string]string), -} - -func getOrCreateListRepoClone(owner, repo, ref, host string) (string, error) { - ref = strings.TrimSpace(ref) - if ref == "" { - return "", errors.New("git fallback requires a non-empty ref") - } - - githubHost := GetGitHubHostForRepo(owner, repo) - if host != "" { - githubHost = stringutil.NormalizeGitHubHostURL(host) - } - repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) - cacheKey := fmt.Sprintf("%s|%s|%s|%s", githubHost, owner, repo, ref) - - if cloneDir, found := func() (string, bool) { - gitListCloneCache.mu.Lock() - defer gitListCloneCache.mu.Unlock() - if cloneDir, ok := gitListCloneCache.dirs[cacheKey]; ok { - if stat, err := os.Stat(filepath.Join(cloneDir, ".git")); err == nil && stat.IsDir() { - return cloneDir, true - } - delete(gitListCloneCache.dirs, cacheKey) - } - return "", false - }(); found { - return cloneDir, nil - } - - tmpDir, err := os.MkdirTemp("", "gh-aw-list-*") - if err != nil { - return "", fmt.Errorf("failed to create temp directory: %w", err) - } - - cloneCmd := exec.Command("git", "clone", "--depth", "1", "--branch", ref, "--single-branch", "--filter=blob:none", "--no-checkout", repoURL, tmpDir) - cloneOutput, err := cloneCmd.CombinedOutput() - if err != nil { - if cleanupErr := os.RemoveAll(tmpDir); cleanupErr != nil { - remoteLog.Printf("Failed to clean up temp directory %q: %v", tmpDir, cleanupErr) - } - remoteLog.Printf("Failed to clone repository: %s", string(cloneOutput)) - return "", fmt.Errorf("failed to clone repository for %s/%s@%s: %w", owner, repo, ref, err) - } - - existingDir, found := func() (string, bool) { - gitListCloneCache.mu.Lock() - defer gitListCloneCache.mu.Unlock() - if existingDir, ok := gitListCloneCache.dirs[cacheKey]; ok { - if stat, statErr := os.Stat(filepath.Join(existingDir, ".git")); statErr == nil && stat.IsDir() { - return existingDir, true - } - } - gitListCloneCache.dirs[cacheKey] = tmpDir - return "", false - }() - if found { - if cleanupErr := os.RemoveAll(tmpDir); cleanupErr != nil { - remoteLog.Printf("Failed to clean up duplicate clone %q: %v", tmpDir, cleanupErr) - } - return existingDir, nil - } - return tmpDir, nil -} - -// isUnderWorkflowsDirectory checks if a file path is a top-level workflow file (not in shared subdirectory) -func isUnderWorkflowsDirectory(filePath string) bool { - // Normalize the path to use forward slashes - normalizedPath := filepath.ToSlash(filePath) - - // Check if the path contains .github/workflows/ - if !strings.Contains(normalizedPath, constants.WorkflowsDirSlash) { - return false - } - - // Extract the part after .github/workflows/ - parts := strings.Split(normalizedPath, constants.WorkflowsDirSlash) - if len(parts) < 2 { - return false - } - - afterWorkflows := parts[1] - - // Check if there are any slashes after .github/workflows/ (indicating subdirectory) - // If there are, it's in a subdirectory like "shared/" and should not be treated as a workflow file - return !strings.Contains(afterWorkflows, "/") -} - -// isCustomAgentFile checks if a file path is a custom agent file under .github/agents/ -// Custom agent files use GitHub Copilot's agent format, which differs from gh-aw workflow format. -// These files have a different schema for the 'tools' field (array vs object). -func isCustomAgentFile(filePath string) bool { - // Normalize the path to use forward slashes - normalizedPath := filepath.ToSlash(filePath) - - // Check if the path contains .github/agents/ and ends with .md - return strings.Contains(normalizedPath, constants.AgentsDir) && strings.HasSuffix(strings.ToLower(normalizedPath), ".md") -} - -// isRepositoryImport checks if an import spec is a repository-only import (no file path) -// Format: owner/repo@ref or owner/repo (downloads entire .github folder, no agent extraction) -func isRepositoryImport(importPath string) bool { - // Remove section reference if present - cleanPath := importPath - if before, _, ok := strings.Cut(importPath, "#"); ok { - cleanPath = before - } - - // Remove ref if present to check the path structure - pathWithoutRef := cleanPath - if before, _, ok := strings.Cut(cleanPath, "@"); ok { - pathWithoutRef = before - } - - // Split by slash to count parts - parts := strings.Split(pathWithoutRef, "/") - - // Repository import has exactly 2 parts: owner/repo - // File imports have 1 part (local file) or 3+ parts (owner/repo/path/to/file) - if len(parts) != 2 { - return false - } - - // Reject local paths - if strings.HasPrefix(pathWithoutRef, ".") || strings.HasPrefix(pathWithoutRef, "/") { - return false - } - - // Reject paths that start with common local directory names - if strings.HasPrefix(pathWithoutRef, "shared/") { - return false - } - - // Additional validation: check if it looks like a valid owner/repo format - // GitHub identifiers can't start with numbers, must be alphanumeric with hyphens/underscores - owner := parts[0] - repo := parts[1] - - // Basic validation - ensure they're not empty and don't look like file extensions - if owner == "" || repo == "" { - return false - } - - // Reject if repo part looks like a file extension (ends with .md, .yaml, etc.) - if strings.Contains(repo, ".") { - return false - } - - return true -} - -// ResolveIncludePath resolves include path based on workflowspec format or relative path -func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, error) { - remoteLog.Printf("Resolving include path: file_path=%s, base_dir=%s", filePath, baseDir) - - if builtinPath, handled, err := resolveBuiltinIncludePath(filePath); handled { - return builtinPath, err - } - - if isWorkflowSpec(filePath) { - remoteLog.Printf("Detected workflowspec format: %s", filePath) - return downloadIncludeFromWorkflowSpec(filePath, cache) - } - - remoteLog.Printf("Using local file resolution for: %s", filePath) - resolveBase, securityBase, normalizedFilePath := computeIncludeResolveAndSecurityBases(filePath, baseDir) - return resolveAndValidateLocalIncludePath(normalizedFilePath, resolveBase, securityBase) -} - -func resolveBuiltinIncludePath(filePath string) (string, bool, error) { - if !strings.HasPrefix(filePath, BuiltinPathPrefix) { - return "", false, nil - } - if !BuiltinVirtualFileExists(filePath) { - return "", true, fmt.Errorf("builtin file not found: %s", filePath) - } - remoteLog.Printf("Resolved builtin path: %s", filePath) - return filePath, true, nil -} - -func findGitHubFolder(baseDir string) string { - githubFolder := baseDir - for !strings.HasSuffix(githubFolder, ".github") { - parent := filepath.Dir(githubFolder) - if parent == githubFolder || parent == "." || parent == "/" { - githubFolder = baseDir - break - } - githubFolder = parent - } - return githubFolder -} - -func computeIncludeResolveAndSecurityBases(filePath, baseDir string) (string, string, string) { - githubFolder := findGitHubFolder(baseDir) - resolveBase := baseDir - securityBase := githubFolder - normalizedFilePath := filePath - if strings.HasSuffix(githubFolder, ".github") { - repoRoot := filepath.Dir(githubFolder) - filePathSlash := filepath.ToSlash(filePath) - if strings.HasPrefix(filePathSlash, constants.GithubDir) { - resolveBase = repoRoot - } else if stripped, ok := strings.CutPrefix(filePathSlash, "/"); ok { - if !strings.HasPrefix(stripped, constants.GithubDir) && !strings.HasPrefix(stripped, ".agents/") { - return "", "", filePath - } - normalizedFilePath = filepath.FromSlash(stripped) - resolveBase = repoRoot - if strings.HasPrefix(stripped, ".agents/") { - securityBase = filepath.Join(repoRoot, ".agents") - } else { - securityBase = githubFolder - } - } - } - return resolveBase, securityBase, normalizedFilePath -} - -func resolveAndValidateLocalIncludePath(filePath, resolveBase, securityBase string) (string, error) { - if stripped, ok := strings.CutPrefix(filepath.ToSlash(filePath), "/"); ok { - if !strings.HasPrefix(stripped, constants.GithubDir) && !strings.HasPrefix(stripped, ".agents/") { - remoteLog.Printf("Security: Path not within .github or .agents: %s", filePath) - return "", fmt.Errorf("security: path %s must be within .github or .agents folder", filePath) - } - } - fullPath := filepath.Join(resolveBase, filePath) - normalizedSecurityBase := filepath.Clean(securityBase) - normalizedFullPath := filepath.Clean(fullPath) - relativePath, err := filepath.Rel(normalizedSecurityBase, normalizedFullPath) - if err != nil || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) || filepath.IsAbs(relativePath) { - allowedFolder := filepath.Base(normalizedSecurityBase) - remoteLog.Printf("Security: Path escapes allowed folder: %s (resolves to: %s)", filePath, relativePath) - return "", fmt.Errorf("security: path %s must be within %s folder (resolves to: %s)", filePath, allowedFolder, relativePath) - } - - if _, err := os.Stat(fullPath); os.IsNotExist(err) { - remoteLog.Printf("Local file not found: %s", fullPath) - // Return a simple error that will be wrapped with source location by the caller - return "", fmt.Errorf("file not found: %s", fullPath) - } - remoteLog.Printf("Resolved to local file: %s", fullPath) - return fullPath, nil -} - -// IsWorkflowSpec checks if a path looks like a workflowspec (owner/repo/path[@ref]). -func IsWorkflowSpec(path string) bool { - // Remove section reference if present - cleanPath := path - if before, _, ok := strings.Cut(path, "#"); ok { - cleanPath = before - } - - // Remove ref if present - if idx := strings.Index(cleanPath, "@"); idx != -1 { - cleanPath = cleanPath[:idx] - } - - // Check if it has at least 3 parts (owner/repo/path) - parts := strings.Split(cleanPath, "/") - if len(parts) < 3 { - return false - } - - // Preserve legacy behavior expected by parser tests: URL-like paths are - // currently treated as workflowspecs because downstream parsing supports - // repository/path extraction from slash-delimited remote references. - if strings.Contains(cleanPath, "://") { - return true - } - - // Reject paths that start with "." (local paths like .github/workflows/...) - if strings.HasPrefix(cleanPath, ".") { - return false - } - - // Reject paths that start with "shared/" (local shared files) - if strings.HasPrefix(cleanPath, "shared/") { - return false - } - - // Reject absolute paths - if strings.HasPrefix(cleanPath, "/") { - return false - } - - // Safe indexing: len(parts) >= 3 is guaranteed above. - owner := parts[0] - repo := parts[1] - if owner == "" || repo == "" { - return false - } - - return true -} - -func isWorkflowSpec(path string) bool { - return IsWorkflowSpec(path) -} - -// downloadIncludeFromWorkflowSpec downloads an include file from GitHub using workflowspec. -// It first checks the cache, and only downloads if not cached. -// -// NOTE: This function is called from ResolveIncludePath which has no context.Context -// parameter. Threading ctx through ResolveIncludePath and its 6+ callers across multiple -// packages is tracked as a follow-up task; context.Background() is used in the interim. -func downloadIncludeFromWorkflowSpec(spec string, cache *ImportCache) (string, error) { - remoteLog.Printf("Downloading from workflowspec: %s", spec) - owner, repo, filePath, ref, err := parseWorkflowSpecParts(spec) - if err != nil { - return "", err - } - remoteLog.Printf("Parsed workflowspec: owner=%s, repo=%s, file=%s, ref=%s", owner, repo, filePath, ref) - - sha := resolveWorkflowSpecSHAForCache(owner, repo, ref, cache) - if cache != nil && 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 - } - } - - remoteLog.Printf("Fetching file from GitHub: %s/%s/%s@%s", owner, repo, filePath, ref) - content, err := downloadFileFromGitHub(context.Background(), owner, repo, filePath, ref) - if err != nil { - return "", fmt.Errorf("failed to download include from %s: %w", spec, err) - } - remoteLog.Printf("Successfully downloaded file: size=%d bytes", len(content)) - - if cache != nil && sha != "" { - cachedPath, err := cache.Set(owner, repo, filePath, sha, content) - if err != nil { - remoteLog.Printf("Failed to cache import: %v", err) - } else { - remoteLog.Printf("Successfully cached download at: %s", cachedPath) - return cachedPath, nil - } - } - return writeDownloadedIncludeToTempFile(content) -} - -func parseWorkflowSpecParts(spec string) (string, string, string, string, error) { - cleanSpec := spec - if before, _, ok := strings.Cut(spec, "#"); ok { - cleanSpec = before - } - parts := strings.SplitN(cleanSpec, "@", 2) - pathPart := parts[0] - ref := "main" - if len(parts) == 2 { - ref = parts[1] - } else { - remoteLog.Print("No ref specified, defaulting to 'main'") - } - slashParts := strings.Split(pathPart, "/") - if len(slashParts) < 3 { - remoteLog.Printf("Invalid workflowspec format: %s", spec) - return "", "", "", "", errors.New("invalid workflowspec: must be owner/repo/path[@ref]") - } - return slashParts[0], slashParts[1], strings.Join(slashParts[2:], "/"), ref, nil -} - -func resolveWorkflowSpecSHAForCache(owner, repo, ref string, cache *ImportCache) string { - if cache == nil { - return "" - } - resolvedSHA, err := resolveRefToSHA(context.Background(), owner, repo, ref, "") - if err != nil { - remoteLog.Printf("Failed to resolve ref to SHA, will skip cache: %v", err) - return "" - } - return resolvedSHA -} - -func writeDownloadedIncludeToTempFile(content []byte) (string, error) { - tempFile, err := os.CreateTemp("", "gh-aw-include-*.md") - if err != nil { - return "", fmt.Errorf("failed to create temp file: %w", err) - } - cleanupOnError := true - fileClosed := false - defer func() { - if cleanupOnError { - if !fileClosed { - if closeErr := tempFile.Close(); closeErr != nil { - remoteLog.Printf("Warning: failed to close temp file during deferred cleanup: %v", closeErr) - } - } - if rmErr := os.Remove(tempFile.Name()); rmErr != nil && !os.IsNotExist(rmErr) { - remoteLog.Printf("Warning: failed to remove temp file %s: %v", tempFile.Name(), rmErr) - } - } - }() - if _, err := tempFile.Write(content); err != nil { - if closeErr := tempFile.Close(); closeErr != nil { - remoteLog.Printf("Warning: failed to close temp file during cleanup: %v", closeErr) - } - fileClosed = true - return "", fmt.Errorf("failed to write temp file: %w", err) - } - if err := tempFile.Close(); err != nil { - fileClosed = true - return "", fmt.Errorf("failed to close temp file: %w", err) - } - cleanupOnError = false - fileClosed = true - return tempFile.Name(), nil -} - -// resolveRefToSHAViaGit resolves a git ref to SHA using git ls-remote -// This is a fallback for when GitHub API authentication fails -func resolveRefToSHAViaGit(owner, repo, ref, host string) (string, error) { - remoteLog.Printf("Attempting git ls-remote fallback for ref resolution: %s/%s@%s", owner, repo, ref) - - var githubHost string - if host != "" { - githubHost = "https://" + host - } else { - githubHost = GetGitHubHostForRepo(owner, repo) - } - repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) - - // Try to resolve the ref using git ls-remote - // Format: git ls-remote - cmd := exec.Command("git", "ls-remote", repoURL, ref) - output, err := cmd.Output() - if err != nil { - // If exact ref doesn't work, try with refs/heads/ and refs/tags/ prefixes - for _, prefix := range []string{"refs/heads/", "refs/tags/"} { - cmd = exec.Command("git", "ls-remote", repoURL, prefix+ref) - output, err = cmd.Output() - if err == nil && len(output) > 0 { - break - } - } - - if err != nil { - return "", fmt.Errorf("failed to resolve ref via git ls-remote: %w", err) - } - } - - // Parse the output: " " - lines := strings.Split(strings.TrimSpace(string(output)), "\n") - if len(lines) == 0 || lines[0] == "" { - return "", fmt.Errorf("no matching ref found for %s", ref) - } - - // Extract SHA from the first line - parts := strings.Fields(lines[0]) - if len(parts) < 1 { - return "", errors.New("invalid git ls-remote output format") - } - - sha := parts[0] - - // Validate it's a valid SHA - if len(sha) != 40 || !gitutil.IsHexString(sha) { - return "", fmt.Errorf("invalid SHA format from git ls-remote: %s", sha) - } - - remoteLog.Printf("Successfully resolved ref via git ls-remote: %s/%s@%s -> %s", owner, repo, ref, sha) - return sha, nil -} - -// resolveRefToSHA resolves a git ref (branch, tag, or SHA) to its commit SHA -func resolveRefToSHA(ctx context.Context, owner, repo, ref, host string) (string, error) { - // If ref is already a full SHA (40 hex characters), return it as-is - if len(ref) == 40 && gitutil.IsHexString(ref) { - return ref, nil - } - - // Use gh CLI to get the commit SHA for the ref - // This works for branches, tags, and short SHAs - // Using go-gh to properly handle enterprise GitHub instances via GH_HOST - apiPath := buildCommitLookupAPIPath(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...) - - if err != nil { - outputStr := stderr.String() - if gitutil.IsAuthError(outputStr) { - remoteLog.Printf("GitHub API authentication failed, attempting git ls-remote fallback for %s/%s@%s", owner, repo, ref) - // Try fallback using git ls-remote for public repositories - sha, gitErr := resolveRefToSHAViaGit(owner, repo, ref, host) - if gitErr != nil { - if host == "" || host == "github.com" { - remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) - return resolveRefToSHAViaPublicAPI(ctx, owner, repo, ref) - } - return "", fmt.Errorf("failed to resolve ref via GitHub API (auth error) and git ls-remote: API error: %w, Git error: %w", err, gitErr) - } - return sha, nil - } - - return "", fmt.Errorf("failed to resolve ref %s to SHA for %s/%s: %s: %w", ref, owner, repo, strings.TrimSpace(outputStr), err) - } - - sha := strings.TrimSpace(stdout.String()) - if sha == "" { - return "", fmt.Errorf("empty SHA returned for ref %s in %s/%s", ref, owner, repo) - } - - // Validate it's a valid SHA (40 hex characters) - if len(sha) != 40 || !gitutil.IsHexString(sha) { - return "", fmt.Errorf("invalid SHA format returned: %s", sha) - } - - return sha, nil -} - -// buildCommitLookupAPIPath returns the GitHub commits API path for a ref, -// URL-escaping the ref segment so branch names containing slashes are valid. -func buildCommitLookupAPIPath(owner, repo, ref string) string { - return fmt.Sprintf("/repos/%s/%s/commits/%s", owner, repo, url.PathEscape(ref)) -} - -// resolveRefToSHAViaPublicAPI resolves a git ref to its commit SHA using an -// unauthenticated call to the public GitHub API. Used as a last-resort fallback -// when both authenticated API and git ls-remote fail. -func resolveRefToSHAViaPublicAPI(ctx context.Context, owner, repo, ref string) (string, error) { - remoteLog.Printf("Attempting unauthenticated public API ref resolution for %s/%s@%s", owner, repo, ref) - apiURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/commits/%s", - owner, repo, url.PathEscape(ref)) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) - if err != nil { - return "", err - } - req.Header.Set("Accept", "application/vnd.github+json") - - resp, err := publicAPIClient.Do(req) - if err != nil { - return "", err - } - defer resp.Body.Close() - - body, err := io.ReadAll(resp.Body) - if err != nil { - return "", err - } - if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("unauthenticated public API failed for %s/%s@%s: HTTP %d: %s", owner, repo, ref, resp.StatusCode, strings.TrimSpace(string(body))) - } - - var result struct { - SHA string `json:"sha"` - } - if err := json.Unmarshal(body, &result); err != nil { - return "", fmt.Errorf("failed to parse commit response: %w", err) - } - if result.SHA == "" || len(result.SHA) != 40 || !gitutil.IsHexString(result.SHA) { - return "", fmt.Errorf("invalid SHA returned from public API: %q", result.SHA) - } - return result.SHA, nil -} - -// downloadFileViaGit downloads a file from a Git repository using git commands -// This is a fallback for when GitHub API authentication fails -func downloadFileViaGit(ctx context.Context, owner, repo, path, ref, host string) ([]byte, error) { - remoteLog.Printf("Attempting git fallback for %s/%s/%s@%s", owner, repo, path, ref) - - // First, try via raw.githubusercontent.com — no auth required for public repos and - // no dependency on git being installed. - // Only attempt raw URL for github.com repos (not GHE) since raw.githubusercontent.com - // only serves public GitHub content. - if host == "" || host == "github.com" { - content, rawErr := downloadFileViaRawURL(ctx, owner, repo, path, ref) - if rawErr == nil { - return content, nil - } - remoteLog.Printf("Raw URL download failed for %s/%s/%s@%s, trying git archive: %v", owner, repo, path, ref, rawErr) - } - - // Use git archive to get the file content without cloning - // This works for public repositories without authentication - var githubHost string - if host != "" { - githubHost = "https://" + host - } else { - githubHost = GetGitHubHostForRepo(owner, repo) - } - repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) - - // git archive command: git archive --remote= - // #nosec G204 -- repoURL, ref, and path are from workflow import configuration authored by the - // developer; exec.CommandContext with separate args (not shell execution) prevents shell injection. - cmd := exec.CommandContext(ctx, "git", "archive", "--remote="+repoURL, ref, path) - archiveOutput, err := cmd.Output() - if err != nil { - // If git archive fails, try with git clone + git show as a fallback - return downloadFileViaGitClone(owner, repo, path, ref, host) - } - - // Extract the file from the tar archive using Go's archive/tar (cross-platform) - content, err := fileutil.ExtractFileFromTar(archiveOutput, path) - if err != nil { - return nil, fmt.Errorf("failed to extract file from git archive: %w", err) - } - - remoteLog.Printf("Successfully downloaded file via git archive: %s/%s/%s@%s", owner, repo, path, ref) - return content, nil -} - -// downloadFileViaRawURL fetches a file using the raw.githubusercontent.com URL. -// This requires no authentication for public repositories and no git installation. -func downloadFileViaRawURL(ctx context.Context, owner, repo, filePath, ref string) ([]byte, error) { - rawURL := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s", owner, repo, ref, filePath) - remoteLog.Printf("Attempting raw URL download: %s", rawURL) - - // Use a client with a timeout to prevent indefinite hangs on slow/unresponsive hosts. - rawClient := &http.Client{Timeout: constants.DefaultHTTPClientTimeout} - - // #nosec G107 -- rawURL is constructed from workflow import configuration authored by - // the developer; the owner, repo, filePath, and ref are user-supplied workflow spec fields. - req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) - if err != nil { - return nil, fmt.Errorf("raw URL request failed for %s: %w", rawURL, err) - } - resp, err := rawClient.Do(req) - if err != nil { - return nil, fmt.Errorf("raw URL request failed for %s: %w", rawURL, err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("raw URL returned HTTP %d for %s", resp.StatusCode, rawURL) - } - - content, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read raw URL response body for %s: %w", rawURL, err) - } - - remoteLog.Printf("Successfully downloaded file via raw URL: %s", rawURL) - return content, nil -} - -// downloadFileViaGitClone downloads a file by shallow cloning the repository -// This is used as a fallback when git archive doesn't work -func downloadFileViaGitClone(owner, repo, path, ref, host string) ([]byte, error) { - remoteLog.Printf("Attempting git clone fallback for %s/%s/%s@%s", owner, repo, path, ref) - - // Create a temporary directory for the shallow clone - tmpDir, err := os.MkdirTemp("", "gh-aw-git-clone-*") - if err != nil { - return nil, fmt.Errorf("failed to create temp directory: %w", err) - } - defer os.RemoveAll(tmpDir) - - var githubHost string - if host != "" { - githubHost = "https://" + host - } else { - githubHost = GetGitHubHostForRepo(owner, repo) - } - repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) - - // Check if ref is a SHA (40 hex characters) - isSHA := len(ref) == 40 && gitutil.IsHexString(ref) - - var cloneCmd *exec.Cmd - if isSHA { - // For SHA refs, we need to clone without --branch and then checkout the specific commit - // Clone with minimal depth and no branch specified - cloneCmd = exec.Command("git", "clone", "--depth", "1", "--no-single-branch", repoURL, tmpDir) - if output, err := cloneCmd.CombinedOutput(); err != nil { - // Try without --no-single-branch if the first attempt fails - remoteLog.Printf("Clone with --no-single-branch failed, trying full clone: %s", string(output)) - cloneCmd = exec.Command("git", "clone", repoURL, tmpDir) - if output, err := cloneCmd.CombinedOutput(); err != nil { - return nil, fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) - } - } - - // Now checkout the specific commit - checkoutCmd := exec.Command("git", "-C", tmpDir, "checkout", ref) - if output, err := checkoutCmd.CombinedOutput(); err != nil { - return nil, fmt.Errorf("failed to checkout commit %s: %w\nOutput: %s", ref, err, string(output)) - } - } else { - // For branch/tag refs, use --branch flag - cloneCmd = exec.Command("git", "clone", "--depth", "1", "--branch", ref, repoURL, tmpDir) - if output, err := cloneCmd.CombinedOutput(); err != nil { - return nil, fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) - } - } - - // Read the file from the cloned repository - filePath := filepath.Join(tmpDir, path) - if err := fileutil.ValidatePathWithinBase(tmpDir, filePath); err != nil { - return nil, fmt.Errorf("refusing to read file outside clone directory: %w", err) - } - content, err := os.ReadFile(filePath) - if err != nil { - return nil, fmt.Errorf("failed to read file from cloned repository: %w", err) - } - - remoteLog.Printf("Successfully downloaded file via git clone: %s/%s/%s@%s", owner, repo, path, ref) - return content, nil -} - -// checkRemoteSymlink checks if a path in a remote GitHub repository is a symlink. -// Returns the symlink target and true if it is a symlink, or empty string and false otherwise. -// A nil error with false means the path is not a symlink (e.g., it's a directory or file). -func checkRemoteSymlink(ctx context.Context, client *api.RESTClient, owner, repo, dirPath, ref string) (string, bool, error) { - endpoint := buildContentsAPIPath(owner, repo, dirPath, ref) - remoteLog.Printf("Checking if path component is symlink: %s/%s/%s@%s", owner, repo, dirPath, ref) - - // The Contents API returns a JSON object for files/symlinks but a JSON array for directories. - // Decode into json.RawMessage first to distinguish these cases without error-driven control flow. - var raw json.RawMessage - err := client.DoWithContext(ctx, http.MethodGet, endpoint, nil, &raw) - if err != nil { - remoteLog.Printf("Contents API error for %s: %v", dirPath, err) - return "", false, err - } - - // If the response is an array, this is a directory listing — not a symlink - trimmed := strings.TrimSpace(string(raw)) - if trimmed != "" && trimmed[0] == '[' { - remoteLog.Printf("Path component %s is a directory (not a symlink)", dirPath) - return "", false, nil - } - - // Parse the object response to check the type - var result struct { - Type string `json:"type"` - Target string `json:"target"` - } - if err := json.Unmarshal(raw, &result); err != nil { - return "", false, fmt.Errorf("failed to parse contents response for %s: %w", dirPath, err) - } - - if result.Type == "symlink" && result.Target != "" { - remoteLog.Printf("Path component %s is a symlink -> %s", dirPath, result.Target) - return result.Target, true, nil - } - - remoteLog.Printf("Path component %s is type=%s (not a symlink)", dirPath, result.Type) - return "", false, nil -} - -// resolveRemoteSymlinks resolves symlinks in a remote GitHub repository path. -// The GitHub Contents API doesn't follow symlinks in path components. For example, -// if .github/workflows/shared is a symlink to ../../gh-agent-workflows/shared, -// fetching .github/workflows/shared/elastic-tools.md returns 404. -// This function walks the path components and resolves any symlinks found. -// The caller must provide a REST client (already authenticated for the correct host). -func resolveRemoteSymlinks(ctx context.Context, client *api.RESTClient, owner, repo, filePath, ref string) (string, error) { - parts := strings.Split(filePath, "/") - if len(parts) <= 1 { - return "", fmt.Errorf("no directory components to resolve in path: %s", filePath) - } - - if client == nil { - return "", fmt.Errorf("no REST client available for symlink resolution of %s/%s/%s@%s", owner, repo, filePath, ref) - } - - remoteLog.Printf("Attempting symlink resolution for %s/%s/%s@%s (%d path components)", owner, repo, filePath, ref, len(parts)) - - for i := 1; i < len(parts); i++ { - dirPath := strings.Join(parts[:i], "/") - resolvedPath, found, err := resolveRemoteSymlinkComponent(ctx, client, owner, repo, filePath, ref, parts, i, dirPath) - if err != nil { - return "", err - } - if found { - return resolvedPath, nil - } - } - - remoteLog.Printf("No symlinks found after checking all %d directory components of %s", len(parts)-1, filePath) - return "", fmt.Errorf("no symlinks found in path: %s", filePath) -} - -func resolveRemoteSymlinkComponent( - ctx context.Context, - client *api.RESTClient, - owner, repo, filePath, ref string, - parts []string, - index int, - dirPath string, -) (string, bool, error) { - target, isSymlink, err := checkRemoteSymlink(ctx, client, owner, repo, dirPath, ref) - if err != nil { - if errorutil.IsNotFoundError(err) { - remoteLog.Printf("Path component %s returned 404, skipping", dirPath) - return "", false, nil - } - return "", false, fmt.Errorf("failed to check path component %s for symlinks: %w", dirPath, err) - } - if !isSymlink { - return "", false, nil - } - parentDir := "" - if index > 1 { - parentDir = strings.Join(parts[:index-1], "/") - } - resolvedBase, err := resolveAndValidateRemoteSymlinkBase(parentDir, target, dirPath) - if err != nil { - return "", false, err - } - remaining := strings.Join(parts[index:], "/") - resolvedPath := resolvedBase + "/" + remaining - remoteLog.Printf("Resolved symlink in remote path: %s -> %s (full: %s -> %s)", dirPath, target, filePath, resolvedPath) - return resolvedPath, true, nil -} - -func resolveAndValidateRemoteSymlinkBase(parentDir, target, dirPath string) (string, error) { - remoteLog.Printf("Resolving symlink: component=%s target=%s parentDir=%s", dirPath, target, parentDir) - resolvedBase := pathpkg.Clean(target) - if parentDir != "" { - resolvedBase = pathpkg.Clean(pathpkg.Join(parentDir, target)) - } - remoteLog.Printf("Resolved base after path.Clean: %s", resolvedBase) - if resolvedBase == "" || resolvedBase == "." || pathpkg.IsAbs(resolvedBase) || strings.HasPrefix(resolvedBase, "..") { - remoteLog.Printf("Rejecting resolved base %q (escapes repository root)", resolvedBase) - return "", fmt.Errorf("symlink target %q at %s resolves outside repository root: %s", target, dirPath, resolvedBase) - } - return resolvedBase, nil -} - -// DownloadFileFromGitHub downloads a file from a GitHub repository using the GitHub API. -// This is the exported wrapper for downloadFileFromGitHub. -// Parameters: -// - owner: Repository owner (e.g., "github") -// - repo: Repository name (e.g., "gh-aw") -// - path: Path to the file within the repository (e.g., ".github/workflows/workflow.md") -// - ref: Git reference (branch, tag, or commit SHA) -// Returns the file content as bytes or an error if the file cannot be retrieved. -func DownloadFileFromGitHub(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { - return downloadFileFromGitHubWithDepth(ctx, owner, repo, path, ref, 0, "") -} - -// DownloadFileFromGitHubForHost downloads a file from a GitHub repository using the GitHub API, -// targeting a specific GitHub host. Use this when the target repository is on a different host -// than the one configured via GH_HOST (e.g., fetching from github.com while GH_HOST is a GHE instance). -// host is the hostname without scheme (e.g., "github.com", "myorg.ghe.com"). -// An empty host uses the default configured host (GH_HOST or github.com). -func DownloadFileFromGitHubForHost(ctx context.Context, owner, repo, path, ref, host string) ([]byte, error) { - return downloadFileFromGitHubWithDepth(ctx, owner, repo, path, ref, 0, host) -} - -// ResolveRefToSHAForHost resolves a git ref to its full commit SHA on a specific GitHub host. -// Use this when the target repository is on a different host than the one configured via GH_HOST. -// host is the hostname without scheme (e.g., "github.com", "myorg.ghe.com"). -// An empty host uses the default configured host (GH_HOST or github.com). -func ResolveRefToSHAForHost(ctx context.Context, owner, repo, ref, host string) (string, error) { - return resolveRefToSHA(ctx, owner, repo, ref, host) -} - -func downloadFileFromGitHub(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { - return downloadFileFromGitHubWithDepth(ctx, owner, repo, path, ref, 0, "") -} - -func downloadFileFromGitHubWithDepth(ctx context.Context, owner, repo, path, ref string, symlinkDepth int, host string) ([]byte, error) { - client, err := createRESTClientForHost(host) - if err != nil { - if gitutil.IsAuthError(err.Error()) { - remoteLog.Printf("REST client creation failed due to auth error, attempting git fallback for %s/%s/%s@%s: %v", owner, repo, path, ref, err) - content, gitErr := downloadFileViaGit(ctx, owner, repo, path, ref, host) - if gitErr != nil { - remoteLog.Printf("Git fallback also failed for %s/%s/%s@%s: %v", owner, repo, path, ref, gitErr) - return nil, fmt.Errorf("failed to fetch file content: %w", err) - } - return content, nil - } - return nil, fmt.Errorf("failed to create REST client: %w", err) - } - - var fileContent struct { - Content string `json:"content"` - Encoding string `json:"encoding"` - Name string `json:"name"` - } - - err = fetchRemoteFileContent(ctx, client, owner, repo, path, ref, &fileContent) - if err != nil { - if gitutil.IsAuthError(err.Error()) { - remoteLog.Printf("GitHub API authentication failed, attempting git fallback for %s/%s/%s@%s", owner, repo, path, ref) - content, gitErr := downloadFileViaGit(ctx, owner, repo, path, ref, host) - if gitErr != nil { - if host == "" || host == "github.com" { - remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s/%s@%s", owner, repo, path, ref) - return downloadFileViaPublicAPI(ctx, owner, repo, path, ref) - } - return nil, fmt.Errorf("failed to fetch file content via GitHub API (auth error) and git fallback: API error: %w, Git error: %w", err, gitErr) - } - return content, nil - } - - if errorutil.IsNotFoundError(err) && symlinkDepth < constants.MaxSymlinkDepth { - if content, handled, resolveErr := retryDownloadViaResolvedSymlink(ctx, client, owner, repo, path, ref, symlinkDepth, host); handled { - return content, resolveErr - } - } - - return nil, fmt.Errorf("failed to fetch file content from %s/%s/%s@%s: %w", owner, repo, path, ref, err) - } - - if fileContent.Content == "" { - return nil, fmt.Errorf("empty content returned from GitHub API for %s/%s/%s@%s", owner, repo, path, ref) - } - - content, err := base64.StdEncoding.DecodeString(fileContent.Content) - if err != nil { - return nil, fmt.Errorf("failed to decode base64 content: %w", err) - } - - return content, nil -} - -func createRESTClientForHost(host string) (*api.RESTClient, error) { - opts := api.ClientOptions{Timeout: constants.DefaultHTTPClientTimeout} - if host != "" { - opts.Host = host - } - return api.NewRESTClient(opts) -} - -func buildContentsAPIPath(owner, repo, path, ref string) string { - pathSegments := strings.Split(path, "/") - for i := range pathSegments { - pathSegments[i] = url.PathEscape(pathSegments[i]) - } - return fmt.Sprintf( - "repos/%s/%s/contents/%s?ref=%s", - owner, - repo, - strings.Join(pathSegments, "/"), - url.QueryEscape(ref), - ) -} - -func fetchRemoteFileContent(ctx context.Context, client *api.RESTClient, owner, repo, path, ref string, fileContent any) error { - return client.DoWithContext(ctx, http.MethodGet, buildContentsAPIPath(owner, repo, path, ref), nil, fileContent) -} - -// downloadFileViaPublicAPI downloads a file from a public GitHub repository -// using an unauthenticated API call. Used as a last-resort fallback when both -// authenticated API and git clone fail (e.g. enterprise SAML tokens). -func downloadFileViaPublicAPI(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { - remoteLog.Printf("Attempting unauthenticated public API download for %s/%s/%s@%s", owner, repo, path, ref) - body, err := fetchPublicGitHubContentsAPI(ctx, owner, repo, path, ref) - if err != nil { - return nil, fmt.Errorf("unauthenticated public API also failed for %s/%s/%s@%s: %w", owner, repo, path, ref, err) - } - - var fileContent struct { - Content string `json:"content"` - Encoding string `json:"encoding"` - } - if err := json.Unmarshal(body, &fileContent); err != nil { - return nil, fmt.Errorf("failed to parse public API file response: %w", err) - } - if fileContent.Content == "" { - return nil, fmt.Errorf("empty content returned from public API for %s/%s/%s@%s", owner, repo, path, ref) - } - - content, err := base64.StdEncoding.DecodeString(fileContent.Content) - if err != nil { - return nil, fmt.Errorf("failed to decode base64 content from public API: %w", err) - } - return content, nil -} - -func retryDownloadViaResolvedSymlink( - ctx context.Context, - client *api.RESTClient, - owner, repo, path, ref string, - symlinkDepth int, - host string, -) ([]byte, bool, error) { - remoteLog.Printf("File not found at %s/%s/%s@%s, checking for symlinks in path (depth: %d)", owner, repo, path, ref, symlinkDepth) - resolvedPath, resolveErr := resolveRemoteSymlinks(ctx, client, owner, repo, path, ref) - if resolveErr == nil && resolvedPath != path { - remoteLog.Printf("Retrying download with symlink-resolved path: %s -> %s", path, resolvedPath) - content, err := downloadFileFromGitHubWithDepth(ctx, owner, repo, resolvedPath, ref, symlinkDepth+1, host) - return content, true, err - } - return nil, false, nil -} - -// ListWorkflowFiles lists workflow files from a remote GitHub repository -// Returns a list of .md files in the specified directory (excluding subdirectories) -func ListWorkflowFiles(ctx context.Context, owner, repo, ref, workflowPath string) ([]string, error) { - return listWorkflowFilesForHost(ctx, owner, repo, ref, workflowPath, "") -} - -// ListWorkflowFilesForHost lists workflow files from a remote GitHub repository on an explicit host. -// Use this when the target repository is on a different host than the one configured via GH_HOST. -func ListWorkflowFilesForHost(ctx context.Context, owner, repo, ref, workflowPath, host string) ([]string, error) { - return listWorkflowFilesForHost(ctx, owner, repo, ref, workflowPath, host) -} - -func listWorkflowFilesForHost(ctx context.Context, owner, repo, ref, workflowPath, host string) ([]string, error) { - remoteLog.Printf("Listing workflow files for %s/%s@%s (path: %s)", owner, repo, ref, workflowPath) - - client, err := createRESTClientForHost(host) - if err != nil { - remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) - return listWorkflowFilesViaGitForHost(owner, repo, ref, workflowPath, host) - } - - // Define response struct for GitHub contents API (array of file objects) - var contents []struct { - Name string `json:"name"` - Path string `json:"path"` - Type string `json:"type"` - } - - // Fetch directory contents from GitHub API - endpoint := buildContentsAPIPath(owner, repo, workflowPath, ref) - err = client.DoWithContext(ctx, http.MethodGet, endpoint, nil, &contents) - if err != nil { - errStr := err.Error() - - // Check if this is an authentication error - if gitutil.IsAuthError(errStr) { - remoteLog.Printf("GitHub API authentication failed, attempting git fallback for %s/%s@%s", owner, repo, ref) - // Try fallback using git commands for public repositories - files, gitErr := listWorkflowFilesViaGitForHost(owner, repo, ref, workflowPath, host) - if gitErr != nil { - if host == "" || host == "github.com" { - remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) - return listWorkflowFilesViaPublicAPI(ctx, owner, repo, ref, workflowPath) - } - return nil, fmt.Errorf("failed to list workflow files via GitHub API (auth error) and git fallback: API error: %w, Git error: %w", err, gitErr) - } - return files, nil - } - - return nil, fmt.Errorf("failed to list workflow files from %s/%s@%s (path: %s): %w", owner, repo, ref, workflowPath, err) - } - - // Filter to only .md files (not in subdirectories) - var workflowFiles []string - for _, item := range contents { - if item.Type == "file" && strings.HasSuffix(strings.ToLower(item.Name), ".md") { - workflowFiles = append(workflowFiles, item.Path) - } - } - - remoteLog.Printf("Found %d workflow files in %s/%s@%s (path: %s)", len(workflowFiles), owner, repo, ref, workflowPath) - return workflowFiles, nil -} - -// ListDirAllFilesForHost lists all files (any extension) that are direct children of -// the given directory in a remote GitHub repository. Subdirectories and their contents -// are not included. This is used for skill file discovery. -func ListDirAllFilesForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { - return listDirAllFilesForHost(ctx, owner, repo, ref, dirPath, host) -} - -func listDirAllFilesForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { - remoteLog.Printf("Listing all files in dir for %s/%s@%s (path: %s)", owner, repo, ref, dirPath) - - client, err := createRESTClientForHost(host) - if err != nil { - remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) - return listDirAllFilesViaGitForHost(owner, repo, ref, dirPath, host) - } - - var contents []struct { - Name string `json:"name"` - Path string `json:"path"` - Type string `json:"type"` - } - - endpoint := buildContentsAPIPath(owner, repo, dirPath, ref) - err = client.DoWithContext(ctx, http.MethodGet, endpoint, nil, &contents) - if err != nil { - errStr := err.Error() - if gitutil.IsAuthError(errStr) { - remoteLog.Printf("GitHub API auth failed, attempting git fallback for %s/%s@%s", owner, repo, ref) - files, gitErr := listDirAllFilesViaGitForHost(owner, repo, ref, dirPath, host) - if gitErr != nil { - if host == "" || host == "github.com" { - remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) - return listDirAllFilesViaPublicAPI(ctx, owner, repo, ref, dirPath) - } - return nil, fmt.Errorf("failed to list dir files via API (auth error) and git fallback: API error: %w, Git error: %w", err, gitErr) - } - return files, nil - } - return nil, fmt.Errorf("failed to list dir files from %s/%s@%s (path: %s): %w", owner, repo, ref, dirPath, err) - } - - var files []string - for _, item := range contents { - if item.Type == "file" { - files = append(files, item.Path) - } - } - - remoteLog.Printf("Found %d files in dir %s/%s@%s (path: %s)", len(files), owner, repo, ref, dirPath) - return files, nil -} - -func listDirAllFilesViaGitForHost(owner, repo, ref, dirPath, host string) ([]string, error) { - remoteLog.Printf("Git fallback for listing all dir files: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) - - tmpDir, err := getOrCreateListRepoClone(owner, repo, ref, host) - if err != nil { - return nil, err - } - - lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", dirPath+"/") - lsTreeOutput, err := lsTreeCmd.CombinedOutput() - if err != nil { - remoteLog.Printf("Failed to list dir files: %s", string(lsTreeOutput)) - return nil, fmt.Errorf("failed to list dir files: %w", err) - } - - lines := strings.Split(strings.TrimSpace(string(lsTreeOutput)), "\n") - var files []string - for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" { - continue - } - // Only include direct children (no additional path separator after dirPath/) - afterDirPath := strings.TrimPrefix(line, dirPath+"/") - if !strings.Contains(afterDirPath, "/") && afterDirPath != "" { - files = append(files, line) - } - } - - remoteLog.Printf("Found %d files in dir via git for %s/%s@%s (path: %s)", len(files), owner, repo, ref, dirPath) - return files, nil -} - -// listDirAllFilesViaPublicAPI lists files in a directory using an unauthenticated -// call to the public GitHub API. Used as a last-resort fallback when both -// authenticated API and git clone fail. -func listDirAllFilesViaPublicAPI(ctx context.Context, owner, repo, ref, dirPath string) ([]string, error) { - remoteLog.Printf("Attempting unauthenticated public API for listing dir files: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) - body, err := fetchPublicGitHubContentsAPI(ctx, owner, repo, dirPath, ref) - if err != nil { - return nil, fmt.Errorf("unauthenticated public API also failed for %s/%s@%s (path: %s): %w", owner, repo, ref, dirPath, err) - } - - var contents []struct { - Path string `json:"path"` - Type string `json:"type"` - } - if err := json.Unmarshal(body, &contents); err != nil { - return nil, fmt.Errorf("failed to parse public API response: %w", err) - } - - var files []string - for _, item := range contents { - if item.Type == "file" { - files = append(files, item.Path) - } - } - remoteLog.Printf("Found %d files via public API for %s/%s@%s (path: %s)", len(files), owner, repo, ref, dirPath) - return files, nil -} - -// ListDirAllFilesRecursivelyForHost lists all files (any extension) that are under the -// given directory in a remote GitHub repository, including files in subdirectories at any -// depth. This is used for copying entire skill folders. -func ListDirAllFilesRecursivelyForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { - return listDirAllFilesRecursivelyForHost(ctx, owner, repo, ref, dirPath, host) -} - -func listDirAllFilesRecursivelyForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { - remoteLog.Printf("Listing all files recursively in dir for %s/%s@%s (path: %s)", owner, repo, ref, dirPath) - - client, err := createRESTClientForHost(host) - if err != nil { - remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) - return listDirAllFilesRecursivelyViaGitForHost(owner, repo, ref, dirPath, host) - } - - files, err := listContentsRecursively(ctx, client, owner, repo, ref, dirPath) - if err != nil { - errStr := err.Error() - if gitutil.IsAuthError(errStr) { - remoteLog.Printf("GitHub API auth failed, attempting git fallback for %s/%s@%s", owner, repo, ref) - gitFiles, gitErr := listDirAllFilesRecursivelyViaGitForHost(owner, repo, ref, dirPath, host) - if gitErr != nil { - // No public API fallback for recursive listing — would require - // multiple unauthenticated calls and is unlikely to stay within - // the 60 req/hour rate limit. Surface both errors. - return nil, fmt.Errorf("failed to list dir files recursively via API (auth error) and git fallback: API error: %w, Git error: %w", err, gitErr) - } - return gitFiles, nil - } - return nil, err - } - - remoteLog.Printf("Found %d files recursively in dir %s/%s@%s (path: %s)", len(files), owner, repo, ref, dirPath) - return files, nil -} - -// listContentsRecursively uses the GitHub Contents API to recursively enumerate all -// files under dirPath. Each subdirectory triggers an additional API call. -func listContentsRecursively(ctx context.Context, client *api.RESTClient, owner, repo, ref, dirPath string) ([]string, error) { - const maxSkillDirRecursionDepth = 10 - return listContentsRecursivelyWithDepth(ctx, client, owner, repo, ref, dirPath, 0, maxSkillDirRecursionDepth) -} - -func listContentsRecursivelyWithDepth(ctx context.Context, client *api.RESTClient, owner, repo, ref, dirPath string, depth, maxDepth int) ([]string, error) { - if depth > maxDepth { - return nil, fmt.Errorf("maximum skill directory recursion depth exceeded at %q (max depth: %d)", dirPath, maxDepth) - } - - var contents []struct { - Name string `json:"name"` - Path string `json:"path"` - Type string `json:"type"` - } - - endpoint := buildContentsAPIPath(owner, repo, dirPath, ref) - if err := client.DoWithContext(ctx, http.MethodGet, endpoint, nil, &contents); err != nil { - return nil, fmt.Errorf("failed to list dir files from %s/%s (path: %s): %w", owner, repo, dirPath, err) - } - - var files []string - for _, item := range contents { - switch item.Type { - case "file": - files = append(files, item.Path) - case "dir": - subFiles, err := listContentsRecursivelyWithDepth(ctx, client, owner, repo, ref, item.Path, depth+1, maxDepth) - if err != nil { - return nil, err - } - files = append(files, subFiles...) - } - } - return files, nil -} - -func listDirAllFilesRecursivelyViaGitForHost(owner, repo, ref, dirPath, host string) ([]string, error) { - remoteLog.Printf("Git fallback for listing all dir files recursively: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) - - tmpDir, err := getOrCreateListRepoClone(owner, repo, ref, host) - if err != nil { - return nil, err - } - - // Normalise dirPath so it never has a trailing slash before we append one. - cleanDirPath := strings.TrimRight(dirPath, "/") - lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", cleanDirPath+"/") - lsTreeOutput, err := lsTreeCmd.CombinedOutput() - if err != nil { - remoteLog.Printf("Failed to list dir files recursively: %s", string(lsTreeOutput)) - return nil, fmt.Errorf("failed to list dir files recursively: %w", err) - } - - lines := strings.Split(strings.TrimSpace(string(lsTreeOutput)), "\n") - var files []string - for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" { - continue - } - // git ls-tree already scopes results to dirPrefix; include every non-empty line. - files = append(files, line) - } - - remoteLog.Printf("Found %d files recursively in dir via git for %s/%s@%s (path: %s)", len(files), owner, repo, ref, dirPath) - return files, nil -} - -// fetchPublicGitHubContentsAPI makes an unauthenticated GET request to the -// GitHub public REST API contents endpoint. This is used as a last-resort -// fallback when the current token (e.g. an enterprise SAML-enforced token) -// cannot access cross-organization public repositories and git clone also -// fails. Unauthenticated requests are subject to a lower rate limit -// (60 req/hour) but are sufficient for the handful of calls during update. -func fetchPublicGitHubContentsAPI(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { - // Encode each path segment independently so that '/' separators are - // preserved — url.PathEscape would turn them into '%2F', breaking nested - // paths like '.github/workflows/shared/foo.md'. - segments := strings.Split(path, "/") - encodedSegments := make([]string, len(segments)) - for i, s := range segments { - encodedSegments[i] = url.PathEscape(s) - } - endpoint := fmt.Sprintf("https://api.github.com/repos/%s/%s/contents/%s?ref=%s", - owner, repo, strings.Join(encodedSegments, "/"), url.QueryEscape(ref)) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) - if err != nil { - return nil, err - } - req.Header.Set("Accept", "application/vnd.github+json") - - resp, err := publicAPIClient.Do(req) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, err - } - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("GitHub API returned %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) - } - return body, nil -} - -// ListDirSubdirsForHost lists subdirectory paths that are direct children of the given -// directory in a remote GitHub repository. This is used for auto-discovering skill dirs. -func ListDirSubdirsForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { - return listDirSubdirsForHost(ctx, owner, repo, ref, dirPath, host) -} - -func listDirSubdirsForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { - remoteLog.Printf("Listing subdirs in %s/%s@%s (path: %s)", owner, repo, ref, dirPath) - - client, err := createRESTClientForHost(host) - if err != nil { - remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) - return listDirSubdirsViaGitForHost(owner, repo, ref, dirPath, host) - } - - var contents []struct { - Name string `json:"name"` - Path string `json:"path"` - Type string `json:"type"` - } - - endpoint := buildContentsAPIPath(owner, repo, dirPath, ref) - err = client.DoWithContext(ctx, http.MethodGet, endpoint, nil, &contents) - if err != nil { - errStr := err.Error() - if gitutil.IsAuthError(errStr) { - remoteLog.Printf("GitHub API auth failed, attempting git fallback for %s/%s@%s", owner, repo, ref) - dirs, gitErr := listDirSubdirsViaGitForHost(owner, repo, ref, dirPath, host) - if gitErr != nil { - if host == "" || host == "github.com" { - remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) - return listDirSubdirsViaPublicAPI(ctx, owner, repo, ref, dirPath) - } - return nil, fmt.Errorf("failed to list subdirs via API (auth error) and git fallback: API error: %w, Git error: %w", err, gitErr) - } - return dirs, nil - } - return nil, fmt.Errorf("failed to list subdirs from %s/%s@%s (path: %s): %w", owner, repo, ref, dirPath, err) - } - - var dirs []string - for _, item := range contents { - if item.Type == "dir" { - dirs = append(dirs, item.Path) - } - } - - remoteLog.Printf("Found %d subdirs in %s/%s@%s (path: %s)", len(dirs), owner, repo, ref, dirPath) - return dirs, nil -} - -func listDirSubdirsViaGitForHost(owner, repo, ref, dirPath, host string) ([]string, error) { - remoteLog.Printf("Git fallback for listing subdirs: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) - - tmpDir, err := getOrCreateListRepoClone(owner, repo, ref, host) - if err != nil { - return nil, err - } - - // Use ls-tree -d to list only direct subdirectory entries. - lsTreeDirsCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "--name-only", "-d", "HEAD", dirPath+"/") - lsTreeDirsOutput, err := lsTreeDirsCmd.CombinedOutput() - if err != nil { - remoteLog.Printf("Failed to list tree subdirs: %s", string(lsTreeDirsOutput)) - return nil, fmt.Errorf("failed to list subdirs: %w", err) - } - - lines := strings.Split(strings.TrimSpace(string(lsTreeDirsOutput)), "\n") - var dirs []string - for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" { - continue - } - afterDirPath := strings.TrimPrefix(line, dirPath+"/") - if !strings.Contains(afterDirPath, "/") && afterDirPath != "" { - dirs = append(dirs, line) - } - } - - remoteLog.Printf("Found %d subdirs via git for %s/%s@%s (path: %s)", len(dirs), owner, repo, ref, dirPath) - return dirs, nil -} - -// listDirSubdirsViaPublicAPI lists subdirectories using an unauthenticated call -// to the public GitHub API. Used as a last-resort fallback when both -// authenticated API and git clone fail (e.g. enterprise SAML tokens). -func listDirSubdirsViaPublicAPI(ctx context.Context, owner, repo, ref, dirPath string) ([]string, error) { - remoteLog.Printf("Attempting unauthenticated public API for listing subdirs: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) - body, err := fetchPublicGitHubContentsAPI(ctx, owner, repo, dirPath, ref) - if err != nil { - return nil, fmt.Errorf("unauthenticated public API also failed for %s/%s@%s (path: %s): %w", owner, repo, ref, dirPath, err) - } - - var contents []struct { - Name string `json:"name"` - Path string `json:"path"` - Type string `json:"type"` - } - if err := json.Unmarshal(body, &contents); err != nil { - return nil, fmt.Errorf("failed to parse public API response: %w", err) - } - - var dirs []string - for _, item := range contents { - if item.Type == "dir" { - dirs = append(dirs, item.Path) - } - } - remoteLog.Printf("Found %d subdirs via public API for %s/%s@%s (path: %s)", len(dirs), owner, repo, ref, dirPath) - return dirs, nil -} - -func listWorkflowFilesViaGitForHost(owner, repo, ref, workflowPath, host string) ([]string, error) { - remoteLog.Printf("Attempting git fallback for listing workflow files: %s/%s@%s (path: %s)", owner, repo, ref, workflowPath) - - githubHost := GetGitHubHostForRepo(owner, repo) - if host != "" { - githubHost = stringutil.NormalizeGitHubHostURL(host) - } - repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) - - // Create a temporary directory for minimal clone - tmpDir, err := os.MkdirTemp("", "gh-aw-list-*") - if err != nil { - return nil, fmt.Errorf("failed to create temp directory: %w", err) - } - defer os.RemoveAll(tmpDir) - - // Do a minimal clone using filter=blob:none for faster cloning (metadata only, no blobs) - // Use --depth=1 for shallow clone and --no-checkout to skip checkout initially - cloneCmd := exec.Command("git", "clone", "--depth", "1", "--branch", ref, "--single-branch", "--filter=blob:none", "--no-checkout", repoURL, tmpDir) - cloneOutput, err := cloneCmd.CombinedOutput() - if err != nil { - remoteLog.Printf("Failed to clone repository: %s", string(cloneOutput)) - return nil, fmt.Errorf("failed to clone repository for %s/%s@%s: %w", owner, repo, ref, err) - } - - // Use git ls-tree to list files in the specified workflows directory - lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", workflowPath+"/") - lsTreeOutput, err := lsTreeCmd.CombinedOutput() - if err != nil { - remoteLog.Printf("Failed to list files: %s", string(lsTreeOutput)) - return nil, fmt.Errorf("failed to list workflow files: %w", err) - } - - // Parse output and filter for .md files (not in subdirectories) - lines := strings.Split(strings.TrimSpace(string(lsTreeOutput)), "\n") - var workflowFiles []string - for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" { - continue - } - // Only include .md files directly in the workflow path (not in subdirectories) - if strings.HasSuffix(strings.ToLower(line), ".md") { - // Check if it's a top-level file (no additional slashes after workflowPath/) - afterWorkflowPath := strings.TrimPrefix(line, workflowPath+"/") - if !strings.Contains(afterWorkflowPath, "/") { - workflowFiles = append(workflowFiles, line) - } - } - } - - remoteLog.Printf("Found %d workflow files via git for %s/%s@%s (path: %s)", len(workflowFiles), owner, repo, ref, workflowPath) - return workflowFiles, nil -} - -// listWorkflowFilesViaPublicAPI lists workflow .md files using an unauthenticated -// call to the public GitHub API. Used as a last-resort fallback when both -// authenticated API and git clone fail. -func listWorkflowFilesViaPublicAPI(ctx context.Context, owner, repo, ref, workflowPath string) ([]string, error) { - remoteLog.Printf("Attempting unauthenticated public API for listing workflow files: %s/%s@%s (path: %s)", owner, repo, ref, workflowPath) - body, err := fetchPublicGitHubContentsAPI(ctx, owner, repo, workflowPath, ref) - if err != nil { - return nil, fmt.Errorf("unauthenticated public API also failed for %s/%s@%s (path: %s): %w", owner, repo, ref, workflowPath, err) - } - - var contents []struct { - Name string `json:"name"` - Path string `json:"path"` - Type string `json:"type"` - } - if err := json.Unmarshal(body, &contents); err != nil { - return nil, fmt.Errorf("failed to parse public API response: %w", err) - } - - var workflowFiles []string - for _, item := range contents { - if item.Type == "file" && strings.HasSuffix(strings.ToLower(item.Name), ".md") { - workflowFiles = append(workflowFiles, item.Path) - } - } - remoteLog.Printf("Found %d workflow files via public API for %s/%s@%s (path: %s)", len(workflowFiles), owner, repo, ref, workflowPath) - return workflowFiles, nil -} diff --git a/pkg/parser/remote_list_files.go b/pkg/parser/remote_list_files.go new file mode 100644 index 00000000000..691b87345b5 --- /dev/null +++ b/pkg/parser/remote_list_files.go @@ -0,0 +1,575 @@ +//go:build !js && !wasm + +package parser + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + + "github.com/cli/go-gh/v2/pkg/api" + "github.com/github/gh-aw/pkg/gitutil" + "github.com/github/gh-aw/pkg/stringutil" +) + +// gitListCloneCache is a process-lifetime cache of shallow clones used by +// git-based directory listing fallbacks to avoid repeated clone operations for +// the same repository/ref tuple. Entries are not explicitly cleaned up because +// the CLI process is short-lived and temporary directories are OS-managed. +var gitListCloneCache = struct { + mu sync.Mutex + dirs map[string]string +}{ + dirs: make(map[string]string), +} + +func getOrCreateListRepoClone(owner, repo, ref, host string) (string, error) { + ref = strings.TrimSpace(ref) + if ref == "" { + return "", errors.New("git fallback requires a non-empty ref") + } + + githubHost := GetGitHubHostForRepo(owner, repo) + if host != "" { + githubHost = stringutil.NormalizeGitHubHostURL(host) + } + repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) + cacheKey := fmt.Sprintf("%s|%s|%s|%s", githubHost, owner, repo, ref) + + if cloneDir, found := func() (string, bool) { + gitListCloneCache.mu.Lock() + defer gitListCloneCache.mu.Unlock() + if cloneDir, ok := gitListCloneCache.dirs[cacheKey]; ok { + if stat, err := os.Stat(filepath.Join(cloneDir, ".git")); err == nil && stat.IsDir() { + return cloneDir, true + } + delete(gitListCloneCache.dirs, cacheKey) + } + return "", false + }(); found { + return cloneDir, nil + } + + tmpDir, err := os.MkdirTemp("", "gh-aw-list-*") + if err != nil { + return "", fmt.Errorf("failed to create temp directory: %w", err) + } + + cloneCmd := exec.Command("git", "clone", "--depth", "1", "--branch", ref, "--single-branch", "--filter=blob:none", "--no-checkout", repoURL, tmpDir) + cloneOutput, err := cloneCmd.CombinedOutput() + if err != nil { + if cleanupErr := os.RemoveAll(tmpDir); cleanupErr != nil { + remoteLog.Printf("Failed to clean up temp directory %q: %v", tmpDir, cleanupErr) + } + remoteLog.Printf("Failed to clone repository: %s", string(cloneOutput)) + return "", fmt.Errorf("failed to clone repository for %s/%s@%s: %w", owner, repo, ref, err) + } + + existingDir, found := func() (string, bool) { + gitListCloneCache.mu.Lock() + defer gitListCloneCache.mu.Unlock() + if existingDir, ok := gitListCloneCache.dirs[cacheKey]; ok { + if stat, statErr := os.Stat(filepath.Join(existingDir, ".git")); statErr == nil && stat.IsDir() { + return existingDir, true + } + } + gitListCloneCache.dirs[cacheKey] = tmpDir + return "", false + }() + if found { + if cleanupErr := os.RemoveAll(tmpDir); cleanupErr != nil { + remoteLog.Printf("Failed to clean up duplicate clone %q: %v", tmpDir, cleanupErr) + } + return existingDir, nil + } + return tmpDir, nil +} + +// ListWorkflowFiles lists workflow files from a remote GitHub repository +// Returns a list of .md files in the specified directory (excluding subdirectories) +func ListWorkflowFiles(ctx context.Context, owner, repo, ref, workflowPath string) ([]string, error) { + return listWorkflowFilesForHost(ctx, owner, repo, ref, workflowPath, "") +} + +// ListWorkflowFilesForHost lists workflow files from a remote GitHub repository on an explicit host. +// Use this when the target repository is on a different host than the one configured via GH_HOST. +func ListWorkflowFilesForHost(ctx context.Context, owner, repo, ref, workflowPath, host string) ([]string, error) { + return listWorkflowFilesForHost(ctx, owner, repo, ref, workflowPath, host) +} + +func listWorkflowFilesForHost(ctx context.Context, owner, repo, ref, workflowPath, host string) ([]string, error) { + remoteLog.Printf("Listing workflow files for %s/%s@%s (path: %s)", owner, repo, ref, workflowPath) + + client, err := createRESTClientForHost(host) + if err != nil { + remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) + return listWorkflowFilesViaGitForHost(owner, repo, ref, workflowPath, host) + } + + // Define response struct for GitHub contents API (array of file objects) + var contents []struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` + } + + // Fetch directory contents from GitHub API + endpoint := buildContentsAPIPath(owner, repo, workflowPath, ref) + err = client.DoWithContext(ctx, http.MethodGet, endpoint, nil, &contents) + if err != nil { + errStr := err.Error() + + // Check if this is an authentication error + if gitutil.IsAuthError(errStr) { + remoteLog.Printf("GitHub API authentication failed, attempting git fallback for %s/%s@%s", owner, repo, ref) + // Try fallback using git commands for public repositories + files, gitErr := listWorkflowFilesViaGitForHost(owner, repo, ref, workflowPath, host) + if gitErr != nil { + if host == "" || host == "github.com" { + remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) + return listWorkflowFilesViaPublicAPI(ctx, owner, repo, ref, workflowPath) + } + return nil, fmt.Errorf("failed to list workflow files via GitHub API (auth error) and git fallback: API error: %w, Git error: %w", err, gitErr) + } + return files, nil + } + + return nil, fmt.Errorf("failed to list workflow files from %s/%s@%s (path: %s): %w", owner, repo, ref, workflowPath, err) + } + + // Filter to only .md files (not in subdirectories) + var workflowFiles []string + for _, item := range contents { + if item.Type == "file" && strings.HasSuffix(strings.ToLower(item.Name), ".md") { + workflowFiles = append(workflowFiles, item.Path) + } + } + + remoteLog.Printf("Found %d workflow files in %s/%s@%s (path: %s)", len(workflowFiles), owner, repo, ref, workflowPath) + return workflowFiles, nil +} + +func listWorkflowFilesViaGitForHost(owner, repo, ref, workflowPath, host string) ([]string, error) { + remoteLog.Printf("Attempting git fallback for listing workflow files: %s/%s@%s (path: %s)", owner, repo, ref, workflowPath) + + githubHost := GetGitHubHostForRepo(owner, repo) + if host != "" { + githubHost = stringutil.NormalizeGitHubHostURL(host) + } + repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) + + // Create a temporary directory for minimal clone + tmpDir, err := os.MkdirTemp("", "gh-aw-list-*") + if err != nil { + return nil, fmt.Errorf("failed to create temp directory: %w", err) + } + defer os.RemoveAll(tmpDir) + + // Do a minimal clone using filter=blob:none for faster cloning (metadata only, no blobs) + // Use --depth=1 for shallow clone and --no-checkout to skip checkout initially + cloneCmd := exec.Command("git", "clone", "--depth", "1", "--branch", ref, "--single-branch", "--filter=blob:none", "--no-checkout", repoURL, tmpDir) + cloneOutput, err := cloneCmd.CombinedOutput() + if err != nil { + remoteLog.Printf("Failed to clone repository: %s", string(cloneOutput)) + return nil, fmt.Errorf("failed to clone repository for %s/%s@%s: %w", owner, repo, ref, err) + } + + // Use git ls-tree to list files in the specified workflows directory + lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", workflowPath+"/") + lsTreeOutput, err := lsTreeCmd.CombinedOutput() + if err != nil { + remoteLog.Printf("Failed to list files: %s", string(lsTreeOutput)) + return nil, fmt.Errorf("failed to list workflow files: %w", err) + } + + // Parse output and filter for .md files (not in subdirectories) + lines := strings.Split(strings.TrimSpace(string(lsTreeOutput)), "\n") + var workflowFiles []string + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" { + continue + } + // Only include .md files directly in the workflow path (not in subdirectories) + if strings.HasSuffix(strings.ToLower(line), ".md") { + // Check if it's a top-level file (no additional slashes after workflowPath/) + afterWorkflowPath := strings.TrimPrefix(line, workflowPath+"/") + if !strings.Contains(afterWorkflowPath, "/") { + workflowFiles = append(workflowFiles, line) + } + } + } + + remoteLog.Printf("Found %d workflow files via git for %s/%s@%s (path: %s)", len(workflowFiles), owner, repo, ref, workflowPath) + return workflowFiles, nil +} + +// listWorkflowFilesViaPublicAPI lists workflow .md files using an unauthenticated +// call to the public GitHub API. Used as a last-resort fallback when both +// authenticated API and git clone fail. +func listWorkflowFilesViaPublicAPI(ctx context.Context, owner, repo, ref, workflowPath string) ([]string, error) { + remoteLog.Printf("Attempting unauthenticated public API for listing workflow files: %s/%s@%s (path: %s)", owner, repo, ref, workflowPath) + body, err := fetchPublicGitHubContentsAPI(ctx, owner, repo, workflowPath, ref) + if err != nil { + return nil, fmt.Errorf("unauthenticated public API also failed for %s/%s@%s (path: %s): %w", owner, repo, ref, workflowPath, err) + } + + var contents []struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` + } + if err := json.Unmarshal(body, &contents); err != nil { + return nil, fmt.Errorf("failed to parse public API response: %w", err) + } + + var workflowFiles []string + for _, item := range contents { + if item.Type == "file" && strings.HasSuffix(strings.ToLower(item.Name), ".md") { + workflowFiles = append(workflowFiles, item.Path) + } + } + remoteLog.Printf("Found %d workflow files via public API for %s/%s@%s (path: %s)", len(workflowFiles), owner, repo, ref, workflowPath) + return workflowFiles, nil +} + +// ListDirAllFilesForHost lists all files (any extension) that are direct children of +// the given directory in a remote GitHub repository. Subdirectories and their contents +// are not included. This is used for skill file discovery. +func ListDirAllFilesForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { + return listDirAllFilesForHost(ctx, owner, repo, ref, dirPath, host) +} + +func listDirAllFilesForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { + remoteLog.Printf("Listing all files in dir for %s/%s@%s (path: %s)", owner, repo, ref, dirPath) + + client, err := createRESTClientForHost(host) + if err != nil { + remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) + return listDirAllFilesViaGitForHost(owner, repo, ref, dirPath, host) + } + + var contents []struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` + } + + endpoint := buildContentsAPIPath(owner, repo, dirPath, ref) + err = client.DoWithContext(ctx, http.MethodGet, endpoint, nil, &contents) + if err != nil { + errStr := err.Error() + if gitutil.IsAuthError(errStr) { + remoteLog.Printf("GitHub API auth failed, attempting git fallback for %s/%s@%s", owner, repo, ref) + files, gitErr := listDirAllFilesViaGitForHost(owner, repo, ref, dirPath, host) + if gitErr != nil { + if host == "" || host == "github.com" { + remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) + return listDirAllFilesViaPublicAPI(ctx, owner, repo, ref, dirPath) + } + return nil, fmt.Errorf("failed to list dir files via API (auth error) and git fallback: API error: %w, Git error: %w", err, gitErr) + } + return files, nil + } + return nil, fmt.Errorf("failed to list dir files from %s/%s@%s (path: %s): %w", owner, repo, ref, dirPath, err) + } + + var files []string + for _, item := range contents { + if item.Type == "file" { + files = append(files, item.Path) + } + } + + remoteLog.Printf("Found %d files in dir %s/%s@%s (path: %s)", len(files), owner, repo, ref, dirPath) + return files, nil +} + +func listDirAllFilesViaGitForHost(owner, repo, ref, dirPath, host string) ([]string, error) { + remoteLog.Printf("Git fallback for listing all dir files: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) + + tmpDir, err := getOrCreateListRepoClone(owner, repo, ref, host) + if err != nil { + return nil, err + } + + lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", dirPath+"/") + lsTreeOutput, err := lsTreeCmd.CombinedOutput() + if err != nil { + remoteLog.Printf("Failed to list dir files: %s", string(lsTreeOutput)) + return nil, fmt.Errorf("failed to list dir files: %w", err) + } + + lines := strings.Split(strings.TrimSpace(string(lsTreeOutput)), "\n") + var files []string + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" { + continue + } + // Only include direct children (no additional path separator after dirPath/) + afterDirPath := strings.TrimPrefix(line, dirPath+"/") + if !strings.Contains(afterDirPath, "/") && afterDirPath != "" { + files = append(files, line) + } + } + + remoteLog.Printf("Found %d files in dir via git for %s/%s@%s (path: %s)", len(files), owner, repo, ref, dirPath) + return files, nil +} + +// listDirAllFilesViaPublicAPI lists files in a directory using an unauthenticated +// call to the public GitHub API. Used as a last-resort fallback when both +// authenticated API and git clone fail. +func listDirAllFilesViaPublicAPI(ctx context.Context, owner, repo, ref, dirPath string) ([]string, error) { + remoteLog.Printf("Attempting unauthenticated public API for listing dir files: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) + body, err := fetchPublicGitHubContentsAPI(ctx, owner, repo, dirPath, ref) + if err != nil { + return nil, fmt.Errorf("unauthenticated public API also failed for %s/%s@%s (path: %s): %w", owner, repo, ref, dirPath, err) + } + + var contents []struct { + Path string `json:"path"` + Type string `json:"type"` + } + if err := json.Unmarshal(body, &contents); err != nil { + return nil, fmt.Errorf("failed to parse public API response: %w", err) + } + + var files []string + for _, item := range contents { + if item.Type == "file" { + files = append(files, item.Path) + } + } + remoteLog.Printf("Found %d files via public API for %s/%s@%s (path: %s)", len(files), owner, repo, ref, dirPath) + return files, nil +} + +// ListDirAllFilesRecursivelyForHost lists all files (any extension) that are under the +// given directory in a remote GitHub repository, including files in subdirectories at any +// depth. This is used for copying entire skill folders. +func ListDirAllFilesRecursivelyForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { + return listDirAllFilesRecursivelyForHost(ctx, owner, repo, ref, dirPath, host) +} + +func listDirAllFilesRecursivelyForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { + remoteLog.Printf("Listing all files recursively in dir for %s/%s@%s (path: %s)", owner, repo, ref, dirPath) + + client, err := createRESTClientForHost(host) + if err != nil { + remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) + return listDirAllFilesRecursivelyViaGitForHost(owner, repo, ref, dirPath, host) + } + + files, err := listContentsRecursively(ctx, client, owner, repo, ref, dirPath) + if err != nil { + errStr := err.Error() + if gitutil.IsAuthError(errStr) { + remoteLog.Printf("GitHub API auth failed, attempting git fallback for %s/%s@%s", owner, repo, ref) + gitFiles, gitErr := listDirAllFilesRecursivelyViaGitForHost(owner, repo, ref, dirPath, host) + if gitErr != nil { + // No public API fallback for recursive listing — would require + // multiple unauthenticated calls and is unlikely to stay within + // the 60 req/hour rate limit. Surface both errors. + return nil, fmt.Errorf("failed to list dir files recursively via API (auth error) and git fallback: API error: %w, Git error: %w", err, gitErr) + } + return gitFiles, nil + } + return nil, err + } + + remoteLog.Printf("Found %d files recursively in dir %s/%s@%s (path: %s)", len(files), owner, repo, ref, dirPath) + return files, nil +} + +// listContentsRecursively uses the GitHub Contents API to recursively enumerate all +// files under dirPath. Each subdirectory triggers an additional API call. +func listContentsRecursively(ctx context.Context, client *api.RESTClient, owner, repo, ref, dirPath string) ([]string, error) { + const maxSkillDirRecursionDepth = 10 + return listContentsRecursivelyWithDepth(ctx, client, owner, repo, ref, dirPath, 0, maxSkillDirRecursionDepth) +} + +func listContentsRecursivelyWithDepth(ctx context.Context, client *api.RESTClient, owner, repo, ref, dirPath string, depth, maxDepth int) ([]string, error) { + if depth > maxDepth { + return nil, fmt.Errorf("maximum skill directory recursion depth exceeded at %q (max depth: %d)", dirPath, maxDepth) + } + + var contents []struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` + } + + endpoint := buildContentsAPIPath(owner, repo, dirPath, ref) + if err := client.DoWithContext(ctx, http.MethodGet, endpoint, nil, &contents); err != nil { + return nil, fmt.Errorf("failed to list dir files from %s/%s (path: %s): %w", owner, repo, dirPath, err) + } + + var files []string + for _, item := range contents { + switch item.Type { + case "file": + files = append(files, item.Path) + case "dir": + subFiles, err := listContentsRecursivelyWithDepth(ctx, client, owner, repo, ref, item.Path, depth+1, maxDepth) + if err != nil { + return nil, err + } + files = append(files, subFiles...) + } + } + return files, nil +} + +func listDirAllFilesRecursivelyViaGitForHost(owner, repo, ref, dirPath, host string) ([]string, error) { + remoteLog.Printf("Git fallback for listing all dir files recursively: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) + + tmpDir, err := getOrCreateListRepoClone(owner, repo, ref, host) + if err != nil { + return nil, err + } + + // Normalise dirPath so it never has a trailing slash before we append one. + cleanDirPath := strings.TrimRight(dirPath, "/") + lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", cleanDirPath+"/") + lsTreeOutput, err := lsTreeCmd.CombinedOutput() + if err != nil { + remoteLog.Printf("Failed to list dir files recursively: %s", string(lsTreeOutput)) + return nil, fmt.Errorf("failed to list dir files recursively: %w", err) + } + + lines := strings.Split(strings.TrimSpace(string(lsTreeOutput)), "\n") + var files []string + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" { + continue + } + // git ls-tree already scopes results to dirPrefix; include every non-empty line. + files = append(files, line) + } + + remoteLog.Printf("Found %d files recursively in dir via git for %s/%s@%s (path: %s)", len(files), owner, repo, ref, dirPath) + return files, nil +} + +// ListDirSubdirsForHost lists subdirectory paths that are direct children of the given +// directory in a remote GitHub repository. This is used for auto-discovering skill dirs. +func ListDirSubdirsForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { + return listDirSubdirsForHost(ctx, owner, repo, ref, dirPath, host) +} + +func listDirSubdirsForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { + remoteLog.Printf("Listing subdirs in %s/%s@%s (path: %s)", owner, repo, ref, dirPath) + + client, err := createRESTClientForHost(host) + if err != nil { + remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) + return listDirSubdirsViaGitForHost(owner, repo, ref, dirPath, host) + } + + var contents []struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` + } + + endpoint := buildContentsAPIPath(owner, repo, dirPath, ref) + err = client.DoWithContext(ctx, http.MethodGet, endpoint, nil, &contents) + if err != nil { + errStr := err.Error() + if gitutil.IsAuthError(errStr) { + remoteLog.Printf("GitHub API auth failed, attempting git fallback for %s/%s@%s", owner, repo, ref) + dirs, gitErr := listDirSubdirsViaGitForHost(owner, repo, ref, dirPath, host) + if gitErr != nil { + if host == "" || host == "github.com" { + remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) + return listDirSubdirsViaPublicAPI(ctx, owner, repo, ref, dirPath) + } + return nil, fmt.Errorf("failed to list subdirs via API (auth error) and git fallback: API error: %w, Git error: %w", err, gitErr) + } + return dirs, nil + } + return nil, fmt.Errorf("failed to list subdirs from %s/%s@%s (path: %s): %w", owner, repo, ref, dirPath, err) + } + + var dirs []string + for _, item := range contents { + if item.Type == "dir" { + dirs = append(dirs, item.Path) + } + } + + remoteLog.Printf("Found %d subdirs in %s/%s@%s (path: %s)", len(dirs), owner, repo, ref, dirPath) + return dirs, nil +} + +func listDirSubdirsViaGitForHost(owner, repo, ref, dirPath, host string) ([]string, error) { + remoteLog.Printf("Git fallback for listing subdirs: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) + + tmpDir, err := getOrCreateListRepoClone(owner, repo, ref, host) + if err != nil { + return nil, err + } + + // Use ls-tree -d to list only direct subdirectory entries. + lsTreeDirsCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "--name-only", "-d", "HEAD", dirPath+"/") + lsTreeDirsOutput, err := lsTreeDirsCmd.CombinedOutput() + if err != nil { + remoteLog.Printf("Failed to list tree subdirs: %s", string(lsTreeDirsOutput)) + return nil, fmt.Errorf("failed to list subdirs: %w", err) + } + + lines := strings.Split(strings.TrimSpace(string(lsTreeDirsOutput)), "\n") + var dirs []string + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" { + continue + } + afterDirPath := strings.TrimPrefix(line, dirPath+"/") + if !strings.Contains(afterDirPath, "/") && afterDirPath != "" { + dirs = append(dirs, line) + } + } + + remoteLog.Printf("Found %d subdirs via git for %s/%s@%s (path: %s)", len(dirs), owner, repo, ref, dirPath) + return dirs, nil +} + +// listDirSubdirsViaPublicAPI lists subdirectories using an unauthenticated call +// to the public GitHub API. Used as a last-resort fallback when both +// authenticated API and git clone fail (e.g. enterprise SAML tokens). +func listDirSubdirsViaPublicAPI(ctx context.Context, owner, repo, ref, dirPath string) ([]string, error) { + remoteLog.Printf("Attempting unauthenticated public API for listing subdirs: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) + body, err := fetchPublicGitHubContentsAPI(ctx, owner, repo, dirPath, ref) + if err != nil { + return nil, fmt.Errorf("unauthenticated public API also failed for %s/%s@%s (path: %s): %w", owner, repo, ref, dirPath, err) + } + + var contents []struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` + } + if err := json.Unmarshal(body, &contents); err != nil { + return nil, fmt.Errorf("failed to parse public API response: %w", err) + } + + var dirs []string + for _, item := range contents { + if item.Type == "dir" { + dirs = append(dirs, item.Path) + } + } + remoteLog.Printf("Found %d subdirs via public API for %s/%s@%s (path: %s)", len(dirs), owner, repo, ref, dirPath) + return dirs, nil +} diff --git a/pkg/parser/remote_resolve_path.go b/pkg/parser/remote_resolve_path.go new file mode 100644 index 00000000000..1cf67db86ff --- /dev/null +++ b/pkg/parser/remote_resolve_path.go @@ -0,0 +1,192 @@ +//go:build !js && !wasm + +package parser + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/github/gh-aw/pkg/constants" +) + +// isUnderWorkflowsDirectory checks if a file path is a top-level workflow file (not in shared subdirectory) +func isUnderWorkflowsDirectory(filePath string) bool { + // Normalize the path to use forward slashes + normalizedPath := filepath.ToSlash(filePath) + + // Check if the path contains .github/workflows/ + if !strings.Contains(normalizedPath, constants.WorkflowsDirSlash) { + return false + } + + // Extract the part after .github/workflows/ + parts := strings.Split(normalizedPath, constants.WorkflowsDirSlash) + if len(parts) < 2 { + return false + } + + afterWorkflows := parts[1] + + // Check if there are any slashes after .github/workflows/ (indicating subdirectory) + // If there are, it's in a subdirectory like "shared/" and should not be treated as a workflow file + return !strings.Contains(afterWorkflows, "/") +} + +// isCustomAgentFile checks if a file path is a custom agent file under .github/agents/ +// Custom agent files use GitHub Copilot's agent format, which differs from gh-aw workflow format. +// These files have a different schema for the 'tools' field (array vs object). +func isCustomAgentFile(filePath string) bool { + // Normalize the path to use forward slashes + normalizedPath := filepath.ToSlash(filePath) + + // Check if the path contains .github/agents/ and ends with .md + return strings.Contains(normalizedPath, constants.AgentsDir) && strings.HasSuffix(strings.ToLower(normalizedPath), ".md") +} + +// isRepositoryImport checks if an import spec is a repository-only import (no file path) +// Format: owner/repo@ref or owner/repo (downloads entire .github folder, no agent extraction) +func isRepositoryImport(importPath string) bool { + // Remove section reference if present + cleanPath := importPath + if before, _, ok := strings.Cut(importPath, "#"); ok { + cleanPath = before + } + + // Remove ref if present to check the path structure + pathWithoutRef := cleanPath + if before, _, ok := strings.Cut(cleanPath, "@"); ok { + pathWithoutRef = before + } + + // Split by slash to count parts + parts := strings.Split(pathWithoutRef, "/") + + // Repository import has exactly 2 parts: owner/repo + // File imports have 1 part (local file) or 3+ parts (owner/repo/path/to/file) + if len(parts) != 2 { + return false + } + + // Reject local paths + if strings.HasPrefix(pathWithoutRef, ".") || strings.HasPrefix(pathWithoutRef, "/") { + return false + } + + // Reject paths that start with common local directory names + if strings.HasPrefix(pathWithoutRef, "shared/") { + return false + } + + // Additional validation: check if it looks like a valid owner/repo format + // GitHub identifiers can't start with numbers, must be alphanumeric with hyphens/underscores + owner := parts[0] + repo := parts[1] + + // Basic validation - ensure they're not empty and don't look like file extensions + if owner == "" || repo == "" { + return false + } + + // Reject if repo part looks like a file extension (ends with .md, .yaml, etc.) + if strings.Contains(repo, ".") { + return false + } + + return true +} + +// ResolveIncludePath resolves include path based on workflowspec format or relative path +func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, error) { + remoteLog.Printf("Resolving include path: file_path=%s, base_dir=%s", filePath, baseDir) + + if builtinPath, handled, err := resolveBuiltinIncludePath(filePath); handled { + return builtinPath, err + } + + if isWorkflowSpec(filePath) { + remoteLog.Printf("Detected workflowspec format: %s", filePath) + return downloadIncludeFromWorkflowSpec(filePath, cache) + } + + remoteLog.Printf("Using local file resolution for: %s", filePath) + resolveBase, securityBase, normalizedFilePath := computeIncludeResolveAndSecurityBases(filePath, baseDir) + return resolveAndValidateLocalIncludePath(normalizedFilePath, resolveBase, securityBase) +} + +func resolveBuiltinIncludePath(filePath string) (string, bool, error) { + if !strings.HasPrefix(filePath, BuiltinPathPrefix) { + return "", false, nil + } + if !BuiltinVirtualFileExists(filePath) { + return "", true, fmt.Errorf("builtin file not found: %s", filePath) + } + remoteLog.Printf("Resolved builtin path: %s", filePath) + return filePath, true, nil +} + +func findGitHubFolder(baseDir string) string { + githubFolder := baseDir + for !strings.HasSuffix(githubFolder, ".github") { + parent := filepath.Dir(githubFolder) + if parent == githubFolder || parent == "." || parent == "/" { + githubFolder = baseDir + break + } + githubFolder = parent + } + return githubFolder +} + +func computeIncludeResolveAndSecurityBases(filePath, baseDir string) (string, string, string) { + githubFolder := findGitHubFolder(baseDir) + resolveBase := baseDir + securityBase := githubFolder + normalizedFilePath := filePath + if strings.HasSuffix(githubFolder, ".github") { + repoRoot := filepath.Dir(githubFolder) + filePathSlash := filepath.ToSlash(filePath) + if strings.HasPrefix(filePathSlash, constants.GithubDir) { + resolveBase = repoRoot + } else if stripped, ok := strings.CutPrefix(filePathSlash, "/"); ok { + if !strings.HasPrefix(stripped, constants.GithubDir) && !strings.HasPrefix(stripped, ".agents/") { + return "", "", filePath + } + normalizedFilePath = filepath.FromSlash(stripped) + resolveBase = repoRoot + if strings.HasPrefix(stripped, ".agents/") { + securityBase = filepath.Join(repoRoot, ".agents") + } else { + securityBase = githubFolder + } + } + } + return resolveBase, securityBase, normalizedFilePath +} + +func resolveAndValidateLocalIncludePath(filePath, resolveBase, securityBase string) (string, error) { + if stripped, ok := strings.CutPrefix(filepath.ToSlash(filePath), "/"); ok { + if !strings.HasPrefix(stripped, constants.GithubDir) && !strings.HasPrefix(stripped, ".agents/") { + remoteLog.Printf("Security: Path not within .github or .agents: %s", filePath) + return "", fmt.Errorf("security: path %s must be within .github or .agents folder", filePath) + } + } + fullPath := filepath.Join(resolveBase, filePath) + normalizedSecurityBase := filepath.Clean(securityBase) + normalizedFullPath := filepath.Clean(fullPath) + relativePath, err := filepath.Rel(normalizedSecurityBase, normalizedFullPath) + if err != nil || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) || filepath.IsAbs(relativePath) { + allowedFolder := filepath.Base(normalizedSecurityBase) + remoteLog.Printf("Security: Path escapes allowed folder: %s (resolves to: %s)", filePath, relativePath) + return "", fmt.Errorf("security: path %s must be within %s folder (resolves to: %s)", filePath, allowedFolder, relativePath) + } + + if _, err := os.Stat(fullPath); os.IsNotExist(err) { + remoteLog.Printf("Local file not found: %s", fullPath) + // Return a simple error that will be wrapped with source location by the caller + return "", fmt.Errorf("file not found: %s", fullPath) + } + remoteLog.Printf("Resolved to local file: %s", fullPath) + return fullPath, nil +} diff --git a/pkg/parser/remote_resolve_sha.go b/pkg/parser/remote_resolve_sha.go new file mode 100644 index 00000000000..e4ad0c2628b --- /dev/null +++ b/pkg/parser/remote_resolve_sha.go @@ -0,0 +1,178 @@ +//go:build !js && !wasm + +package parser + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "os/exec" + "strings" + + "github.com/cli/go-gh/v2" + "github.com/github/gh-aw/pkg/gitutil" +) + +// resolveRefToSHAViaGit resolves a git ref to SHA using git ls-remote +// This is a fallback for when GitHub API authentication fails +func resolveRefToSHAViaGit(owner, repo, ref, host string) (string, error) { + remoteLog.Printf("Attempting git ls-remote fallback for ref resolution: %s/%s@%s", owner, repo, ref) + + var githubHost string + if host != "" { + githubHost = "https://" + host + } else { + githubHost = GetGitHubHostForRepo(owner, repo) + } + repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) + + // Try to resolve the ref using git ls-remote + // Format: git ls-remote + cmd := exec.Command("git", "ls-remote", repoURL, ref) + output, err := cmd.Output() + if err != nil { + // If exact ref doesn't work, try with refs/heads/ and refs/tags/ prefixes + for _, prefix := range []string{"refs/heads/", "refs/tags/"} { + cmd = exec.Command("git", "ls-remote", repoURL, prefix+ref) + output, err = cmd.Output() + if err == nil && len(output) > 0 { + break + } + } + + if err != nil { + return "", fmt.Errorf("failed to resolve ref via git ls-remote: %w", err) + } + } + + // Parse the output: " " + lines := strings.Split(strings.TrimSpace(string(output)), "\n") + if len(lines) == 0 || lines[0] == "" { + return "", fmt.Errorf("no matching ref found for %s", ref) + } + + // Extract SHA from the first line + parts := strings.Fields(lines[0]) + if len(parts) < 1 { + return "", errors.New("invalid git ls-remote output format") + } + + sha := parts[0] + + // Validate it's a valid SHA + if len(sha) != 40 || !gitutil.IsHexString(sha) { + return "", fmt.Errorf("invalid SHA format from git ls-remote: %s", sha) + } + + remoteLog.Printf("Successfully resolved ref via git ls-remote: %s/%s@%s -> %s", owner, repo, ref, sha) + return sha, nil +} + +// resolveRefToSHA resolves a git ref (branch, tag, or SHA) to its commit SHA +func resolveRefToSHA(ctx context.Context, owner, repo, ref, host string) (string, error) { + // If ref is already a full SHA (40 hex characters), return it as-is + if len(ref) == 40 && gitutil.IsHexString(ref) { + return ref, nil + } + + // Use gh CLI to get the commit SHA for the ref + // This works for branches, tags, and short SHAs + // Using go-gh to properly handle enterprise GitHub instances via GH_HOST + apiPath := buildCommitLookupAPIPath(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...) + + if err != nil { + outputStr := stderr.String() + if gitutil.IsAuthError(outputStr) { + remoteLog.Printf("GitHub API authentication failed, attempting git ls-remote fallback for %s/%s@%s", owner, repo, ref) + // Try fallback using git ls-remote for public repositories + sha, gitErr := resolveRefToSHAViaGit(owner, repo, ref, host) + if gitErr != nil { + if host == "" || host == "github.com" { + remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) + return resolveRefToSHAViaPublicAPI(ctx, owner, repo, ref) + } + return "", fmt.Errorf("failed to resolve ref via GitHub API (auth error) and git ls-remote: API error: %w, Git error: %w", err, gitErr) + } + return sha, nil + } + + return "", fmt.Errorf("failed to resolve ref %s to SHA for %s/%s: %s: %w", ref, owner, repo, strings.TrimSpace(outputStr), err) + } + + sha := strings.TrimSpace(stdout.String()) + if sha == "" { + return "", fmt.Errorf("empty SHA returned for ref %s in %s/%s", ref, owner, repo) + } + + // Validate it's a valid SHA (40 hex characters) + if len(sha) != 40 || !gitutil.IsHexString(sha) { + return "", fmt.Errorf("invalid SHA format returned: %s", sha) + } + + return sha, nil +} + +// buildCommitLookupAPIPath returns the GitHub commits API path for a ref, +// URL-escaping the ref segment so branch names containing slashes are valid. +func buildCommitLookupAPIPath(owner, repo, ref string) string { + return fmt.Sprintf("/repos/%s/%s/commits/%s", owner, repo, url.PathEscape(ref)) +} + +// resolveRefToSHAViaPublicAPI resolves a git ref to its commit SHA using an +// unauthenticated call to the public GitHub API. Used as a last-resort fallback +// when both authenticated API and git ls-remote fail. +func resolveRefToSHAViaPublicAPI(ctx context.Context, owner, repo, ref string) (string, error) { + remoteLog.Printf("Attempting unauthenticated public API ref resolution for %s/%s@%s", owner, repo, ref) + apiURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/commits/%s", + owner, repo, url.PathEscape(ref)) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return "", err + } + req.Header.Set("Accept", "application/vnd.github+json") + + resp, err := publicAPIClient.Do(req) + if err != nil { + return "", err + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return "", err + } + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("unauthenticated public API failed for %s/%s@%s: HTTP %d: %s", owner, repo, ref, resp.StatusCode, strings.TrimSpace(string(body))) + } + + var result struct { + SHA string `json:"sha"` + } + if err := json.Unmarshal(body, &result); err != nil { + return "", fmt.Errorf("failed to parse commit response: %w", err) + } + if result.SHA == "" || len(result.SHA) != 40 || !gitutil.IsHexString(result.SHA) { + return "", fmt.Errorf("invalid SHA returned from public API: %q", result.SHA) + } + return result.SHA, nil +} + +// ResolveRefToSHAForHost resolves a git ref to its full commit SHA on a specific GitHub host. +// Use this when the target repository is on a different host than the one configured via GH_HOST. +// host is the hostname without scheme (e.g., "github.com", "myorg.ghe.com"). +// An empty host uses the default configured host (GH_HOST or github.com). +func ResolveRefToSHAForHost(ctx context.Context, owner, repo, ref, host string) (string, error) { + return resolveRefToSHA(ctx, owner, repo, ref, host) +} diff --git a/pkg/parser/remote_workflow_spec.go b/pkg/parser/remote_workflow_spec.go new file mode 100644 index 00000000000..d4b577e65c4 --- /dev/null +++ b/pkg/parser/remote_workflow_spec.go @@ -0,0 +1,175 @@ +//go:build !js && !wasm + +package parser + +import ( + "context" + "errors" + "fmt" + "os" + "strings" +) + +// IsWorkflowSpec checks if a path looks like a workflowspec (owner/repo/path[@ref]). +func IsWorkflowSpec(path string) bool { + // Remove section reference if present + cleanPath := path + if before, _, ok := strings.Cut(path, "#"); ok { + cleanPath = before + } + + // Remove ref if present + if idx := strings.Index(cleanPath, "@"); idx != -1 { + cleanPath = cleanPath[:idx] + } + + // Check if it has at least 3 parts (owner/repo/path) + parts := strings.Split(cleanPath, "/") + if len(parts) < 3 { + return false + } + + // Preserve legacy behavior expected by parser tests: URL-like paths are + // currently treated as workflowspecs because downstream parsing supports + // repository/path extraction from slash-delimited remote references. + if strings.Contains(cleanPath, "://") { + return true + } + + // Reject paths that start with "." (local paths like .github/workflows/...) + if strings.HasPrefix(cleanPath, ".") { + return false + } + + // Reject paths that start with "shared/" (local shared files) + if strings.HasPrefix(cleanPath, "shared/") { + return false + } + + // Reject absolute paths + if strings.HasPrefix(cleanPath, "/") { + return false + } + + // Safe indexing: len(parts) >= 3 is guaranteed above. + owner := parts[0] + repo := parts[1] + if owner == "" || repo == "" { + return false + } + + return true +} + +func isWorkflowSpec(path string) bool { + return IsWorkflowSpec(path) +} + +// downloadIncludeFromWorkflowSpec downloads an include file from GitHub using workflowspec. +// It first checks the cache, and only downloads if not cached. +// +// NOTE: This function is called from ResolveIncludePath which has no context.Context +// parameter. Threading ctx through ResolveIncludePath and its 6+ callers across multiple +// packages is tracked as a follow-up task; context.Background() is used in the interim. +func downloadIncludeFromWorkflowSpec(spec string, cache *ImportCache) (string, error) { + remoteLog.Printf("Downloading from workflowspec: %s", spec) + owner, repo, filePath, ref, err := parseWorkflowSpecParts(spec) + if err != nil { + return "", err + } + remoteLog.Printf("Parsed workflowspec: owner=%s, repo=%s, file=%s, ref=%s", owner, repo, filePath, ref) + + sha := resolveWorkflowSpecSHAForCache(owner, repo, ref, cache) + if cache != nil && 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 + } + } + + remoteLog.Printf("Fetching file from GitHub: %s/%s/%s@%s", owner, repo, filePath, ref) + content, err := downloadFileFromGitHub(context.Background(), owner, repo, filePath, ref) + if err != nil { + return "", fmt.Errorf("failed to download include from %s: %w", spec, err) + } + remoteLog.Printf("Successfully downloaded file: size=%d bytes", len(content)) + + if cache != nil && sha != "" { + cachedPath, err := cache.Set(owner, repo, filePath, sha, content) + if err != nil { + remoteLog.Printf("Failed to cache import: %v", err) + } else { + remoteLog.Printf("Successfully cached download at: %s", cachedPath) + return cachedPath, nil + } + } + return writeDownloadedIncludeToTempFile(content) +} + +func parseWorkflowSpecParts(spec string) (string, string, string, string, error) { + cleanSpec := spec + if before, _, ok := strings.Cut(spec, "#"); ok { + cleanSpec = before + } + parts := strings.SplitN(cleanSpec, "@", 2) + pathPart := parts[0] + ref := "main" + if len(parts) == 2 { + ref = parts[1] + } else { + remoteLog.Print("No ref specified, defaulting to 'main'") + } + slashParts := strings.Split(pathPart, "/") + if len(slashParts) < 3 { + remoteLog.Printf("Invalid workflowspec format: %s", spec) + return "", "", "", "", errors.New("invalid workflowspec: must be owner/repo/path[@ref]") + } + return slashParts[0], slashParts[1], strings.Join(slashParts[2:], "/"), ref, nil +} + +func resolveWorkflowSpecSHAForCache(owner, repo, ref string, cache *ImportCache) string { + if cache == nil { + return "" + } + resolvedSHA, err := resolveRefToSHA(context.Background(), owner, repo, ref, "") + if err != nil { + remoteLog.Printf("Failed to resolve ref to SHA, will skip cache: %v", err) + return "" + } + return resolvedSHA +} + +func writeDownloadedIncludeToTempFile(content []byte) (string, error) { + tempFile, err := os.CreateTemp("", "gh-aw-include-*.md") + if err != nil { + return "", fmt.Errorf("failed to create temp file: %w", err) + } + cleanupOnError := true + fileClosed := false + defer func() { + if cleanupOnError { + if !fileClosed { + if closeErr := tempFile.Close(); closeErr != nil { + remoteLog.Printf("Warning: failed to close temp file during deferred cleanup: %v", closeErr) + } + } + if rmErr := os.Remove(tempFile.Name()); rmErr != nil && !os.IsNotExist(rmErr) { + remoteLog.Printf("Warning: failed to remove temp file %s: %v", tempFile.Name(), rmErr) + } + } + }() + if _, err := tempFile.Write(content); err != nil { + if closeErr := tempFile.Close(); closeErr != nil { + remoteLog.Printf("Warning: failed to close temp file during cleanup: %v", closeErr) + } + fileClosed = true + return "", fmt.Errorf("failed to write temp file: %w", err) + } + if err := tempFile.Close(); err != nil { + fileClosed = true + return "", fmt.Errorf("failed to close temp file: %w", err) + } + cleanupOnError = false + fileClosed = true + return tempFile.Name(), nil +} From f2f888da7c14d6cfe0933219561928215b965c25 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 6 Jul 2026 17:25:46 +0000 Subject: [PATCH 3/7] docs(adr): draft ADR-43834 for remote_fetch.go decomposition Captures the architectural decision to split the 1553-line pkg/parser/remote_fetch.go into six focused single-domain modules, preserving all exported symbols with no logic changes. Co-Authored-By: Claude Sonnet 4.6 --- ...split-remote-fetch-into-focused-modules.md | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 docs/adr/43834-split-remote-fetch-into-focused-modules.md diff --git a/docs/adr/43834-split-remote-fetch-into-focused-modules.md b/docs/adr/43834-split-remote-fetch-into-focused-modules.md new file mode 100644 index 00000000000..666387475ba --- /dev/null +++ b/docs/adr/43834-split-remote-fetch-into-focused-modules.md @@ -0,0 +1,45 @@ +# ADR-43834: Split remote_fetch.go into Focused Single-Responsibility Modules + +**Date**: 2026-07-06 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +`pkg/parser/remote_fetch.go` had grown to 1,553 lines across five unrelated functional domains: shared HTTP client utilities, local include-path resolution and security validation, WorkflowSpec parsing and download dispatch, ref-to-SHA resolution with multi-tier auth fallbacks, single-file download with symlink handling, and directory listing (workflow files, flat, recursive, subdirs). A single file spanning five concerns made it hard to navigate, review, and isolate in tests. Merge conflicts were more frequent because unrelated features competed for changes in the same file. + +### Decision + +We will split `remote_fetch.go` into six focused files, all residing in the same `parser` package, each under 450 lines and responsible for exactly one functional domain: `remote_client.go` (shared HTTP/REST utilities), `remote_resolve_path.go` (include-path resolution and security validation), `remote_workflow_spec.go` (WorkflowSpec parsing, SHA caching, download dispatch), `remote_resolve_sha.go` (ref→SHA resolution with auth/git/public-API fallback chain), `remote_download_file.go` (single-file download with symlink resolution and three-tier fallback), and `remote_list_files.go` (directory listing). All exported symbols are preserved unchanged; no logic is altered. + +### Alternatives Considered + +#### Alternative 1: Keep the monolithic file with internal comment sections + +Add region-style comments (e.g., `// --- HTTP Client ---`) to demarcate the five domains within `remote_fetch.go` without splitting files. This is zero-risk from a compilation and test perspective and avoids any directory-structure change. It was not chosen because comment markers are non-enforced conventions — they drift and get ignored under deadline pressure, and navigating a 1,500-line file still requires IDE search. The size problem recurs the moment a new domain is added. + +#### Alternative 2: Extract into separate sub-packages under `pkg/parser/remote/` + +Create sub-packages such as `pkg/parser/remote/client`, `pkg/parser/remote/download`, etc., moving each domain into its own package. This enforces separation at the language level (the compiler will catch circular imports) and makes inter-domain dependencies explicit. It was not chosen because it would require changing all call sites across the `parser` package, potentially introduce circular imports given the current tight coupling, and is a larger refactor scope than the immediate problem warrants. Same-package file splits achieve the navigability goal with zero API breakage. + +### Consequences + +#### Positive +- Each file is under 450 lines with a single clearly named responsibility, making targeted code review and debugging faster. +- Merge conflicts on `remote_fetch.go` are eliminated; changes to download logic no longer race with changes to SHA resolution or directory listing. +- Easier to unit-test each domain in isolation without loading the other four domains into view. +- New contributors can discover where a specific function lives by filename rather than grepping a 1,500-line file. + +#### Negative +- The `pkg/parser` directory now contains more files (six new files replace one), which may feel noisy at a glance before the reader understands the naming convention. +- The same-package split does not enforce domain boundaries at compile time — a function in `remote_download_file.go` can still call unexported helpers in `remote_list_files.go` without the compiler flagging it. + +#### Neutral +- No logic changes mean all existing tests pass without modification; behavior for external callers of exported symbols (`DownloadFileFromGitHub`, `ListWorkflowFiles`, etc.) is identical. +- The wasm build stub `remote_fetch_wasm.go` is untouched; the build-tag-gated split is consistent with that pattern. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From c1730c97975835712682dc6e0b9b1e40e0a10551 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 19:34:44 +0000 Subject: [PATCH 4/7] fix(parser): address review feedback on remote fetch modules Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/parser/remote_download_file.go | 21 ++--- pkg/parser/remote_fetch_test.go | 5 +- pkg/parser/remote_list_files.go | 122 ++++++++++++++--------------- pkg/parser/remote_resolve_path.go | 11 +-- pkg/parser/remote_resolve_sha.go | 8 +- pkg/parser/remote_workflow_spec.go | 29 ++++--- 6 files changed, 101 insertions(+), 95 deletions(-) diff --git a/pkg/parser/remote_download_file.go b/pkg/parser/remote_download_file.go index a8aa681f523..ad00a23a6f3 100644 --- a/pkg/parser/remote_download_file.go +++ b/pkg/parser/remote_download_file.go @@ -251,7 +251,10 @@ func resolveRemoteSymlinkComponent( return "", false, err } remaining := strings.Join(parts[index:], "/") - resolvedPath := resolvedBase + "/" + remaining + resolvedPath := pathpkg.Clean(pathpkg.Join(resolvedBase, remaining)) + if resolvedPath == "" || resolvedPath == "." || pathpkg.IsAbs(resolvedPath) || strings.HasPrefix(resolvedPath, "..") { + return "", false, fmt.Errorf("resolved symlink path escapes repository root: %s", resolvedPath) + } remoteLog.Printf("Resolved symlink in remote path: %s -> %s (full: %s -> %s)", dirPath, target, filePath, resolvedPath) return resolvedPath, true, nil } @@ -304,7 +307,7 @@ func downloadFileViaGit(ctx context.Context, owner, repo, path, ref, host string archiveOutput, err := cmd.Output() if err != nil { // If git archive fails, try with git clone + git show as a fallback - return downloadFileViaGitClone(owner, repo, path, ref, host) + return downloadFileViaGitClone(ctx, owner, repo, path, ref, host) } // Extract the file from the tar archive using Go's archive/tar (cross-platform) @@ -324,15 +327,13 @@ func downloadFileViaRawURL(ctx context.Context, owner, repo, filePath, ref strin remoteLog.Printf("Attempting raw URL download: %s", rawURL) // Use a client with a timeout to prevent indefinite hangs on slow/unresponsive hosts. - rawClient := &http.Client{Timeout: constants.DefaultHTTPClientTimeout} - // #nosec G107 -- rawURL is constructed from workflow import configuration authored by // the developer; the owner, repo, filePath, and ref are user-supplied workflow spec fields. req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) if err != nil { return nil, fmt.Errorf("raw URL request failed for %s: %w", rawURL, err) } - resp, err := rawClient.Do(req) + resp, err := publicAPIClient.Do(req) if err != nil { return nil, fmt.Errorf("raw URL request failed for %s: %w", rawURL, err) } @@ -353,7 +354,7 @@ func downloadFileViaRawURL(ctx context.Context, owner, repo, filePath, ref strin // downloadFileViaGitClone downloads a file by shallow cloning the repository // This is used as a fallback when git archive doesn't work -func downloadFileViaGitClone(owner, repo, path, ref, host string) ([]byte, error) { +func downloadFileViaGitClone(ctx context.Context, owner, repo, path, ref, host string) ([]byte, error) { remoteLog.Printf("Attempting git clone fallback for %s/%s/%s@%s", owner, repo, path, ref) // Create a temporary directory for the shallow clone @@ -378,24 +379,24 @@ func downloadFileViaGitClone(owner, repo, path, ref, host string) ([]byte, error if isSHA { // For SHA refs, we need to clone without --branch and then checkout the specific commit // Clone with minimal depth and no branch specified - cloneCmd = exec.Command("git", "clone", "--depth", "1", "--no-single-branch", repoURL, tmpDir) + cloneCmd = exec.CommandContext(ctx, "git", "clone", "--depth", "1", "--no-single-branch", repoURL, tmpDir) if output, err := cloneCmd.CombinedOutput(); err != nil { // Try without --no-single-branch if the first attempt fails remoteLog.Printf("Clone with --no-single-branch failed, trying full clone: %s", string(output)) - cloneCmd = exec.Command("git", "clone", repoURL, tmpDir) + cloneCmd = exec.CommandContext(ctx, "git", "clone", repoURL, tmpDir) if output, err := cloneCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) } } // Now checkout the specific commit - checkoutCmd := exec.Command("git", "-C", tmpDir, "checkout", ref) + checkoutCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "checkout", ref) if output, err := checkoutCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to checkout commit %s: %w\nOutput: %s", ref, err, string(output)) } } else { // For branch/tag refs, use --branch flag - cloneCmd = exec.Command("git", "clone", "--depth", "1", "--branch", ref, repoURL, tmpDir) + cloneCmd = exec.CommandContext(ctx, "git", "clone", "--depth", "1", "--branch", ref, repoURL, tmpDir) if output, err := cloneCmd.CombinedOutput(); err != nil { return nil, fmt.Errorf("failed to clone repository: %w\nOutput: %s", err, string(output)) } diff --git a/pkg/parser/remote_fetch_test.go b/pkg/parser/remote_fetch_test.go index 693938c8ba7..06c2089f051 100644 --- a/pkg/parser/remote_fetch_test.go +++ b/pkg/parser/remote_fetch_test.go @@ -3,6 +3,7 @@ package parser import ( + "context" "strings" "testing" ) @@ -53,7 +54,7 @@ func TestBuildContentsAPIPath(t *testing.T) { func TestGitFallbackRequiresNonEmptyRef(t *testing.T) { t.Run("all files fallback validates ref", func(t *testing.T) { - _, err := listDirAllFilesViaGitForHost("owner", "repo", "", "skills/demo", "") + _, err := listDirAllFilesViaGitForHost(context.Background(), "owner", "repo", "", "skills/demo", "") if err == nil { t.Fatal("expected error for empty ref") } @@ -63,7 +64,7 @@ func TestGitFallbackRequiresNonEmptyRef(t *testing.T) { }) t.Run("subdirs fallback validates ref", func(t *testing.T) { - _, err := listDirSubdirsViaGitForHost("owner", "repo", " ", "skills", "") + _, err := listDirSubdirsViaGitForHost(context.Background(), "owner", "repo", " ", "skills", "") if err == nil { t.Fatal("expected error for empty ref") } diff --git a/pkg/parser/remote_list_files.go b/pkg/parser/remote_list_files.go index 691b87345b5..1e451f310a9 100644 --- a/pkg/parser/remote_list_files.go +++ b/pkg/parser/remote_list_files.go @@ -17,6 +17,7 @@ import ( "github.com/cli/go-gh/v2/pkg/api" "github.com/github/gh-aw/pkg/gitutil" "github.com/github/gh-aw/pkg/stringutil" + "golang.org/x/sync/singleflight" ) // gitListCloneCache is a process-lifetime cache of shallow clones used by @@ -30,7 +31,9 @@ var gitListCloneCache = struct { dirs: make(map[string]string), } -func getOrCreateListRepoClone(owner, repo, ref, host string) (string, error) { +var gitListCloneGroup singleflight.Group + +func getOrCreateListRepoClone(ctx context.Context, owner, repo, ref, host string) (string, error) { ref = strings.TrimSpace(ref) if ref == "" { return "", errors.New("git fallback requires a non-empty ref") @@ -57,39 +60,45 @@ func getOrCreateListRepoClone(owner, repo, ref, host string) (string, error) { return cloneDir, nil } - tmpDir, err := os.MkdirTemp("", "gh-aw-list-*") - if err != nil { - return "", fmt.Errorf("failed to create temp directory: %w", err) - } + cloneDir, err, _ := gitListCloneGroup.Do(cacheKey, func() (any, error) { + if cloneDir, found := func() (string, bool) { + gitListCloneCache.mu.Lock() + defer gitListCloneCache.mu.Unlock() + if cloneDir, ok := gitListCloneCache.dirs[cacheKey]; ok { + if stat, err := os.Stat(filepath.Join(cloneDir, ".git")); err == nil && stat.IsDir() { + return cloneDir, true + } + delete(gitListCloneCache.dirs, cacheKey) + } + return "", false + }(); found { + return cloneDir, nil + } - cloneCmd := exec.Command("git", "clone", "--depth", "1", "--branch", ref, "--single-branch", "--filter=blob:none", "--no-checkout", repoURL, tmpDir) - cloneOutput, err := cloneCmd.CombinedOutput() - if err != nil { - if cleanupErr := os.RemoveAll(tmpDir); cleanupErr != nil { - remoteLog.Printf("Failed to clean up temp directory %q: %v", tmpDir, cleanupErr) + tmpDir, err := os.MkdirTemp("", "gh-aw-list-*") + if err != nil { + return "", fmt.Errorf("failed to create temp directory: %w", err) } - remoteLog.Printf("Failed to clone repository: %s", string(cloneOutput)) - return "", fmt.Errorf("failed to clone repository for %s/%s@%s: %w", owner, repo, ref, err) - } - existingDir, found := func() (string, bool) { - gitListCloneCache.mu.Lock() - defer gitListCloneCache.mu.Unlock() - if existingDir, ok := gitListCloneCache.dirs[cacheKey]; ok { - if stat, statErr := os.Stat(filepath.Join(existingDir, ".git")); statErr == nil && stat.IsDir() { - return existingDir, true + cloneCmd := exec.CommandContext(ctx, "git", "clone", "--depth", "1", "--branch", ref, "--single-branch", "--filter=blob:none", "--no-checkout", repoURL, tmpDir) + cloneOutput, err := cloneCmd.CombinedOutput() + if err != nil { + if cleanupErr := os.RemoveAll(tmpDir); cleanupErr != nil { + remoteLog.Printf("Failed to clean up temp directory %q: %v", tmpDir, cleanupErr) } + remoteLog.Printf("Failed to clone repository: %s", string(cloneOutput)) + return "", fmt.Errorf("failed to clone repository for %s/%s@%s: %w", owner, repo, ref, err) } + + gitListCloneCache.mu.Lock() gitListCloneCache.dirs[cacheKey] = tmpDir - return "", false - }() - if found { - if cleanupErr := os.RemoveAll(tmpDir); cleanupErr != nil { - remoteLog.Printf("Failed to clean up duplicate clone %q: %v", tmpDir, cleanupErr) - } - return existingDir, nil + gitListCloneCache.mu.Unlock() + return tmpDir, nil + }) + if err != nil { + return "", err } - return tmpDir, nil + return cloneDir.(string), nil } // ListWorkflowFiles lists workflow files from a remote GitHub repository @@ -110,7 +119,7 @@ func listWorkflowFilesForHost(ctx context.Context, owner, repo, ref, workflowPat client, err := createRESTClientForHost(host) if err != nil { remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) - return listWorkflowFilesViaGitForHost(owner, repo, ref, workflowPath, host) + return listWorkflowFilesViaGitForHost(ctx, owner, repo, ref, workflowPath, host) } // Define response struct for GitHub contents API (array of file objects) @@ -130,7 +139,7 @@ func listWorkflowFilesForHost(ctx context.Context, owner, repo, ref, workflowPat if gitutil.IsAuthError(errStr) { remoteLog.Printf("GitHub API authentication failed, attempting git fallback for %s/%s@%s", owner, repo, ref) // Try fallback using git commands for public repositories - files, gitErr := listWorkflowFilesViaGitForHost(owner, repo, ref, workflowPath, host) + files, gitErr := listWorkflowFilesViaGitForHost(ctx, owner, repo, ref, workflowPath, host) if gitErr != nil { if host == "" || host == "github.com" { remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) @@ -156,33 +165,16 @@ func listWorkflowFilesForHost(ctx context.Context, owner, repo, ref, workflowPat return workflowFiles, nil } -func listWorkflowFilesViaGitForHost(owner, repo, ref, workflowPath, host string) ([]string, error) { +func listWorkflowFilesViaGitForHost(ctx context.Context, owner, repo, ref, workflowPath, host string) ([]string, error) { remoteLog.Printf("Attempting git fallback for listing workflow files: %s/%s@%s (path: %s)", owner, repo, ref, workflowPath) - githubHost := GetGitHubHostForRepo(owner, repo) - if host != "" { - githubHost = stringutil.NormalizeGitHubHostURL(host) - } - repoURL := fmt.Sprintf("%s/%s/%s.git", githubHost, owner, repo) - - // Create a temporary directory for minimal clone - tmpDir, err := os.MkdirTemp("", "gh-aw-list-*") - if err != nil { - return nil, fmt.Errorf("failed to create temp directory: %w", err) - } - defer os.RemoveAll(tmpDir) - - // Do a minimal clone using filter=blob:none for faster cloning (metadata only, no blobs) - // Use --depth=1 for shallow clone and --no-checkout to skip checkout initially - cloneCmd := exec.Command("git", "clone", "--depth", "1", "--branch", ref, "--single-branch", "--filter=blob:none", "--no-checkout", repoURL, tmpDir) - cloneOutput, err := cloneCmd.CombinedOutput() + tmpDir, err := getOrCreateListRepoClone(ctx, owner, repo, ref, host) if err != nil { - remoteLog.Printf("Failed to clone repository: %s", string(cloneOutput)) - return nil, fmt.Errorf("failed to clone repository for %s/%s@%s: %w", owner, repo, ref, err) + return nil, err } // Use git ls-tree to list files in the specified workflows directory - lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", workflowPath+"/") + lsTreeCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", workflowPath+"/") lsTreeOutput, err := lsTreeCmd.CombinedOutput() if err != nil { remoteLog.Printf("Failed to list files: %s", string(lsTreeOutput)) @@ -253,7 +245,7 @@ func listDirAllFilesForHost(ctx context.Context, owner, repo, ref, dirPath, host client, err := createRESTClientForHost(host) if err != nil { remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) - return listDirAllFilesViaGitForHost(owner, repo, ref, dirPath, host) + return listDirAllFilesViaGitForHost(ctx, owner, repo, ref, dirPath, host) } var contents []struct { @@ -268,7 +260,7 @@ func listDirAllFilesForHost(ctx context.Context, owner, repo, ref, dirPath, host errStr := err.Error() if gitutil.IsAuthError(errStr) { remoteLog.Printf("GitHub API auth failed, attempting git fallback for %s/%s@%s", owner, repo, ref) - files, gitErr := listDirAllFilesViaGitForHost(owner, repo, ref, dirPath, host) + files, gitErr := listDirAllFilesViaGitForHost(ctx, owner, repo, ref, dirPath, host) if gitErr != nil { if host == "" || host == "github.com" { remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) @@ -292,15 +284,15 @@ func listDirAllFilesForHost(ctx context.Context, owner, repo, ref, dirPath, host return files, nil } -func listDirAllFilesViaGitForHost(owner, repo, ref, dirPath, host string) ([]string, error) { +func listDirAllFilesViaGitForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { remoteLog.Printf("Git fallback for listing all dir files: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) - tmpDir, err := getOrCreateListRepoClone(owner, repo, ref, host) + tmpDir, err := getOrCreateListRepoClone(ctx, owner, repo, ref, host) if err != nil { return nil, err } - lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", dirPath+"/") + lsTreeCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", dirPath+"/") lsTreeOutput, err := lsTreeCmd.CombinedOutput() if err != nil { remoteLog.Printf("Failed to list dir files: %s", string(lsTreeOutput)) @@ -366,7 +358,7 @@ func listDirAllFilesRecursivelyForHost(ctx context.Context, owner, repo, ref, di client, err := createRESTClientForHost(host) if err != nil { remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) - return listDirAllFilesRecursivelyViaGitForHost(owner, repo, ref, dirPath, host) + return listDirAllFilesRecursivelyViaGitForHost(ctx, owner, repo, ref, dirPath, host) } files, err := listContentsRecursively(ctx, client, owner, repo, ref, dirPath) @@ -374,7 +366,7 @@ func listDirAllFilesRecursivelyForHost(ctx context.Context, owner, repo, ref, di errStr := err.Error() if gitutil.IsAuthError(errStr) { remoteLog.Printf("GitHub API auth failed, attempting git fallback for %s/%s@%s", owner, repo, ref) - gitFiles, gitErr := listDirAllFilesRecursivelyViaGitForHost(owner, repo, ref, dirPath, host) + gitFiles, gitErr := listDirAllFilesRecursivelyViaGitForHost(ctx, owner, repo, ref, dirPath, host) if gitErr != nil { // No public API fallback for recursive listing — would require // multiple unauthenticated calls and is unlikely to stay within @@ -429,17 +421,17 @@ func listContentsRecursivelyWithDepth(ctx context.Context, client *api.RESTClien return files, nil } -func listDirAllFilesRecursivelyViaGitForHost(owner, repo, ref, dirPath, host string) ([]string, error) { +func listDirAllFilesRecursivelyViaGitForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { remoteLog.Printf("Git fallback for listing all dir files recursively: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) - tmpDir, err := getOrCreateListRepoClone(owner, repo, ref, host) + tmpDir, err := getOrCreateListRepoClone(ctx, owner, repo, ref, host) if err != nil { return nil, err } // Normalise dirPath so it never has a trailing slash before we append one. cleanDirPath := strings.TrimRight(dirPath, "/") - lsTreeCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", cleanDirPath+"/") + lsTreeCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "ls-tree", "-r", "--name-only", "HEAD", cleanDirPath+"/") lsTreeOutput, err := lsTreeCmd.CombinedOutput() if err != nil { remoteLog.Printf("Failed to list dir files recursively: %s", string(lsTreeOutput)) @@ -473,7 +465,7 @@ func listDirSubdirsForHost(ctx context.Context, owner, repo, ref, dirPath, host client, err := createRESTClientForHost(host) if err != nil { remoteLog.Printf("Failed to create REST client, attempting git fallback: %v", err) - return listDirSubdirsViaGitForHost(owner, repo, ref, dirPath, host) + return listDirSubdirsViaGitForHost(ctx, owner, repo, ref, dirPath, host) } var contents []struct { @@ -488,7 +480,7 @@ func listDirSubdirsForHost(ctx context.Context, owner, repo, ref, dirPath, host errStr := err.Error() if gitutil.IsAuthError(errStr) { remoteLog.Printf("GitHub API auth failed, attempting git fallback for %s/%s@%s", owner, repo, ref) - dirs, gitErr := listDirSubdirsViaGitForHost(owner, repo, ref, dirPath, host) + dirs, gitErr := listDirSubdirsViaGitForHost(ctx, owner, repo, ref, dirPath, host) if gitErr != nil { if host == "" || host == "github.com" { remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) @@ -512,16 +504,16 @@ func listDirSubdirsForHost(ctx context.Context, owner, repo, ref, dirPath, host return dirs, nil } -func listDirSubdirsViaGitForHost(owner, repo, ref, dirPath, host string) ([]string, error) { +func listDirSubdirsViaGitForHost(ctx context.Context, owner, repo, ref, dirPath, host string) ([]string, error) { remoteLog.Printf("Git fallback for listing subdirs: %s/%s@%s (path: %s)", owner, repo, ref, dirPath) - tmpDir, err := getOrCreateListRepoClone(owner, repo, ref, host) + tmpDir, err := getOrCreateListRepoClone(ctx, owner, repo, ref, host) if err != nil { return nil, err } // Use ls-tree -d to list only direct subdirectory entries. - lsTreeDirsCmd := exec.Command("git", "-C", tmpDir, "ls-tree", "--name-only", "-d", "HEAD", dirPath+"/") + lsTreeDirsCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "ls-tree", "--name-only", "-d", "HEAD", dirPath+"/") lsTreeDirsOutput, err := lsTreeDirsCmd.CombinedOutput() if err != nil { remoteLog.Printf("Failed to list tree subdirs: %s", string(lsTreeDirsOutput)) diff --git a/pkg/parser/remote_resolve_path.go b/pkg/parser/remote_resolve_path.go index 1cf67db86ff..7eff68a81e4 100644 --- a/pkg/parser/remote_resolve_path.go +++ b/pkg/parser/remote_resolve_path.go @@ -79,8 +79,7 @@ func isRepositoryImport(importPath string) bool { return false } - // Additional validation: check if it looks like a valid owner/repo format - // GitHub identifiers can't start with numbers, must be alphanumeric with hyphens/underscores + // Additional validation: check if it looks like a valid owner/repo format. owner := parts[0] repo := parts[1] @@ -89,9 +88,11 @@ func isRepositoryImport(importPath string) bool { return false } - // Reject if repo part looks like a file extension (ends with .md, .yaml, etc.) - if strings.Contains(repo, ".") { - return false + // Reject if repo part looks like a file path with a known workflow/data extension. + for _, ext := range []string{".md", ".yaml", ".yml", ".json"} { + if strings.HasSuffix(strings.ToLower(repo), ext) { + return false + } } return true diff --git a/pkg/parser/remote_resolve_sha.go b/pkg/parser/remote_resolve_sha.go index e4ad0c2628b..33918247254 100644 --- a/pkg/parser/remote_resolve_sha.go +++ b/pkg/parser/remote_resolve_sha.go @@ -19,7 +19,7 @@ import ( // resolveRefToSHAViaGit resolves a git ref to SHA using git ls-remote // This is a fallback for when GitHub API authentication fails -func resolveRefToSHAViaGit(owner, repo, ref, host string) (string, error) { +func resolveRefToSHAViaGit(ctx context.Context, owner, repo, ref, host string) (string, error) { remoteLog.Printf("Attempting git ls-remote fallback for ref resolution: %s/%s@%s", owner, repo, ref) var githubHost string @@ -32,12 +32,12 @@ func resolveRefToSHAViaGit(owner, repo, ref, host string) (string, error) { // Try to resolve the ref using git ls-remote // Format: git ls-remote - cmd := exec.Command("git", "ls-remote", repoURL, ref) + cmd := exec.CommandContext(ctx, "git", "ls-remote", repoURL, ref) output, err := cmd.Output() if err != nil { // If exact ref doesn't work, try with refs/heads/ and refs/tags/ prefixes for _, prefix := range []string{"refs/heads/", "refs/tags/"} { - cmd = exec.Command("git", "ls-remote", repoURL, prefix+ref) + cmd = exec.CommandContext(ctx, "git", "ls-remote", repoURL, prefix+ref) output, err = cmd.Output() if err == nil && len(output) > 0 { break @@ -97,7 +97,7 @@ func resolveRefToSHA(ctx context.Context, owner, repo, ref, host string) (string if gitutil.IsAuthError(outputStr) { remoteLog.Printf("GitHub API authentication failed, attempting git ls-remote fallback for %s/%s@%s", owner, repo, ref) // Try fallback using git ls-remote for public repositories - sha, gitErr := resolveRefToSHAViaGit(owner, repo, ref, host) + sha, gitErr := resolveRefToSHAViaGit(ctx, owner, repo, ref, host) if gitErr != nil { if host == "" || host == "github.com" { remoteLog.Printf("Git fallback also failed, attempting unauthenticated API for %s/%s@%s", owner, repo, ref) diff --git a/pkg/parser/remote_workflow_spec.go b/pkg/parser/remote_workflow_spec.go index d4b577e65c4..4c086b42b20 100644 --- a/pkg/parser/remote_workflow_spec.go +++ b/pkg/parser/remote_workflow_spec.go @@ -73,13 +73,13 @@ func isWorkflowSpec(path string) bool { // packages is tracked as a follow-up task; context.Background() is used in the interim. func downloadIncludeFromWorkflowSpec(spec string, cache *ImportCache) (string, error) { remoteLog.Printf("Downloading from workflowspec: %s", spec) - owner, repo, filePath, ref, err := parseWorkflowSpecParts(spec) + host, owner, repo, filePath, ref, err := parseWorkflowSpecParts(spec) if err != nil { return "", err } - remoteLog.Printf("Parsed workflowspec: owner=%s, repo=%s, file=%s, ref=%s", owner, repo, filePath, ref) + remoteLog.Printf("Parsed workflowspec: host=%s, owner=%s, repo=%s, file=%s, ref=%s", host, owner, repo, filePath, ref) - sha := resolveWorkflowSpecSHAForCache(owner, repo, ref, cache) + sha := resolveWorkflowSpecSHAForCache(owner, repo, ref, host, cache) if cache != nil && 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) @@ -88,7 +88,12 @@ func downloadIncludeFromWorkflowSpec(spec string, cache *ImportCache) (string, e } remoteLog.Printf("Fetching file from GitHub: %s/%s/%s@%s", owner, repo, filePath, ref) - content, err := downloadFileFromGitHub(context.Background(), owner, repo, filePath, ref) + var content []byte + if host == "" { + content, err = downloadFileFromGitHub(context.Background(), owner, repo, filePath, ref) + } else { + content, err = downloadFileFromGitHubWithDepth(context.Background(), owner, repo, filePath, ref, 0, host) + } if err != nil { return "", fmt.Errorf("failed to download include from %s: %w", spec, err) } @@ -106,7 +111,7 @@ func downloadIncludeFromWorkflowSpec(spec string, cache *ImportCache) (string, e return writeDownloadedIncludeToTempFile(content) } -func parseWorkflowSpecParts(spec string) (string, string, string, string, error) { +func parseWorkflowSpecParts(spec string) (string, string, string, string, string, error) { cleanSpec := spec if before, _, ok := strings.Cut(spec, "#"); ok { cleanSpec = before @@ -122,16 +127,22 @@ func parseWorkflowSpecParts(spec string) (string, string, string, string, error) slashParts := strings.Split(pathPart, "/") if len(slashParts) < 3 { remoteLog.Printf("Invalid workflowspec format: %s", spec) - return "", "", "", "", errors.New("invalid workflowspec: must be owner/repo/path[@ref]") + return "", "", "", "", "", errors.New("invalid workflowspec: must be owner/repo/path[@ref]") } - return slashParts[0], slashParts[1], strings.Join(slashParts[2:], "/"), ref, nil + + // Optional host-prefixed format: host/owner/repo/path[@ref] + if len(slashParts) >= 4 && strings.Contains(slashParts[0], ".") { + return slashParts[0], slashParts[1], slashParts[2], strings.Join(slashParts[3:], "/"), ref, nil + } + + return "", slashParts[0], slashParts[1], strings.Join(slashParts[2:], "/"), ref, nil } -func resolveWorkflowSpecSHAForCache(owner, repo, ref string, cache *ImportCache) string { +func resolveWorkflowSpecSHAForCache(owner, repo, ref, host string, cache *ImportCache) string { if cache == nil { return "" } - resolvedSHA, err := resolveRefToSHA(context.Background(), owner, repo, ref, "") + resolvedSHA, err := resolveRefToSHA(context.Background(), owner, repo, ref, host) if err != nil { remoteLog.Printf("Failed to resolve ref to SHA, will skip cache: %v", err) return "" From a13d1d6fffbeb56230dce4c49750ffc54a20af0b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 19:47:40 +0000 Subject: [PATCH 5/7] fix(parser): harden clone cache result typing Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/parser/remote_list_files.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/parser/remote_list_files.go b/pkg/parser/remote_list_files.go index 1e451f310a9..e62ff8e6d0e 100644 --- a/pkg/parser/remote_list_files.go +++ b/pkg/parser/remote_list_files.go @@ -98,7 +98,11 @@ func getOrCreateListRepoClone(ctx context.Context, owner, repo, ref, host string if err != nil { return "", err } - return cloneDir.(string), nil + cloneDirPath, ok := cloneDir.(string) + if !ok { + return "", errors.New("internal error: clone result was not a string") + } + return cloneDirPath, nil } // ListWorkflowFiles lists workflow files from a remote GitHub repository From 7b2f2363765240b0dbe7ffe3f2b2d193d4eb8f4f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:42:48 +0000 Subject: [PATCH 6/7] chore: outline pr-finisher plan Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 4469ec430d3..73610b6dc43 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( github.com/stretchr/testify v1.11.1 golang.org/x/crypto v0.53.0 golang.org/x/mod v0.37.0 + golang.org/x/sync v0.21.0 golang.org/x/term v0.44.0 golang.org/x/tools v0.47.0 golang.org/x/vuln v1.5.0 @@ -109,7 +110,6 @@ require ( golang.org/x/exp v0.0.0-20240909161429-701f63a606c0 // indirect golang.org/x/net v0.56.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect - golang.org/x/sync v0.21.0 // indirect golang.org/x/sys v0.46.0 // indirect golang.org/x/telemetry v0.0.0-20260625142307-59b4966ccb57 // indirect golang.org/x/text v0.38.0 // indirect From ea4dc325d6c86c42236600a4de612140f0b7507f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:57:10 +0000 Subject: [PATCH 7/7] refactor(parser): remove redundant workflowspec wrapper usage Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/parser/import_bfs.go | 8 ++++---- pkg/parser/import_remote_nested_test.go | 12 ++++++------ pkg/parser/remote_resolve_path.go | 2 +- pkg/parser/remote_workflow_spec.go | 4 ---- 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/pkg/parser/import_bfs.go b/pkg/parser/import_bfs.go index dafa0d3f406..9800551f911 100644 --- a/pkg/parser/import_bfs.go +++ b/pkg/parser/import_bfs.go @@ -184,7 +184,7 @@ func validateNoLockYMLImport(fullPath, importPath, workflowFilePath, yamlContent } func detectRemoteImportOrigin(filePath string) *remoteImportOrigin { - if !isWorkflowSpec(filePath) { + if !IsWorkflowSpec(filePath) { return nil } origin := parseRemoteOrigin(filePath) @@ -407,10 +407,10 @@ func enqueueNestedImportEntry(entry nestedImportEntry, item importQueueItem, bas } func resolveNestedImportPathAndOrigin(item importQueueItem, nestedFilePath string) (string, *remoteImportOrigin, error) { - if item.remoteOrigin != nil && !isWorkflowSpec(nestedFilePath) { + if item.remoteOrigin != nil && !IsWorkflowSpec(nestedFilePath) { return resolveRemoteNestedPath(item, nestedFilePath) } - if isWorkflowSpec(nestedFilePath) { + if IsWorkflowSpec(nestedFilePath) { nestedRemoteOrigin := parseRemoteOrigin(nestedFilePath) if nestedRemoteOrigin != nil { importLog.Printf("Nested workflowspec import detected: %s (origin: %s/%s@%s)", nestedFilePath, nestedRemoteOrigin.Owner, nestedRemoteOrigin.Repo, nestedRemoteOrigin.Ref) @@ -439,7 +439,7 @@ func resolveRemoteNestedPath(item importQueueItem, nestedFilePath string) (strin func determineNestedBaseDir(item importQueueItem, resolvedPath, baseDir string) string { isLocalRelative := !strings.Contains(resolvedPath, "/") || strings.HasPrefix(resolvedPath, "./") - if item.remoteOrigin == nil && !isWorkflowSpec(resolvedPath) && isLocalRelative { + if item.remoteOrigin == nil && !IsWorkflowSpec(resolvedPath) && isLocalRelative { return filepath.Dir(item.fullPath) } return baseDir diff --git a/pkg/parser/import_remote_nested_test.go b/pkg/parser/import_remote_nested_test.go index 87610be4aa3..8c3ef5a8f0b 100644 --- a/pkg/parser/import_remote_nested_test.go +++ b/pkg/parser/import_remote_nested_test.go @@ -277,7 +277,7 @@ func TestRemoteOriginPropagation(t *testing.T) { t.Run("workflowspec import gets remote origin", func(t *testing.T) { spec := "elastic/ai-github-actions/gh-agent-workflows/mention-in-pr/rwxp.md@main" - assert.True(t, isWorkflowSpec(spec), "Should be recognized as workflowspec") + assert.True(t, IsWorkflowSpec(spec), "Should be recognized as workflowspec") origin := parseRemoteOrigin(spec) require.NotNil(t, origin, "Should parse remote origin") @@ -289,7 +289,7 @@ func TestRemoteOriginPropagation(t *testing.T) { t.Run("local import does not get remote origin", func(t *testing.T) { localPath := "shared/tools.md" - assert.False(t, isWorkflowSpec(localPath), "Should not be recognized as workflowspec") + assert.False(t, IsWorkflowSpec(localPath), "Should not be recognized as workflowspec") origin := parseRemoteOrigin(localPath) assert.Nil(t, origin, "Local paths should not produce remote origin") @@ -320,7 +320,7 @@ func TestRemoteOriginPropagation(t *testing.T) { ) // The constructed spec should be recognized as a workflowspec - assert.True(t, isWorkflowSpec(expectedSpec), "Constructed path should be a valid workflowspec") + assert.True(t, IsWorkflowSpec(expectedSpec), "Constructed path should be a valid workflowspec") }) t.Run("nested relative path from remote parent without BasePath uses .github/workflows", func(t *testing.T) { @@ -347,7 +347,7 @@ func TestRemoteOriginPropagation(t *testing.T) { ) // The constructed spec should be recognized as a workflowspec - assert.True(t, isWorkflowSpec(expectedSpec), "Constructed path should be a valid workflowspec") + assert.True(t, IsWorkflowSpec(expectedSpec), "Constructed path should be a valid workflowspec") }) t.Run("nested relative path with ./ prefix is cleaned", func(t *testing.T) { @@ -383,7 +383,7 @@ func TestRemoteOriginPropagation(t *testing.T) { // If a remote file references another workflowspec, it should // get its own origin, not inherit the parent's nestedSpec := "other-org/other-repo/path/file.md@v2.0" - assert.True(t, isWorkflowSpec(nestedSpec), "Should be recognized as workflowspec") + assert.True(t, IsWorkflowSpec(nestedSpec), "Should be recognized as workflowspec") origin := parseRemoteOrigin(nestedSpec) require.NotNil(t, origin, "Should parse remote origin for nested workflowspec") @@ -694,7 +694,7 @@ func TestParseRemoteOriginWithURLFormats(t *testing.T) { for _, urlPath := range urlPaths { // Currently, isWorkflowSpec accepts URLs (they have >3 slash-separated parts) - isSpec := isWorkflowSpec(urlPath) + isSpec := IsWorkflowSpec(urlPath) assert.True(t, isSpec, "URL is currently accepted as workflowspec: %s", urlPath) // parseRemoteOrigin will parse the URL parts literally diff --git a/pkg/parser/remote_resolve_path.go b/pkg/parser/remote_resolve_path.go index 7eff68a81e4..4a20d813568 100644 --- a/pkg/parser/remote_resolve_path.go +++ b/pkg/parser/remote_resolve_path.go @@ -106,7 +106,7 @@ func ResolveIncludePath(filePath, baseDir string, cache *ImportCache) (string, e return builtinPath, err } - if isWorkflowSpec(filePath) { + if IsWorkflowSpec(filePath) { remoteLog.Printf("Detected workflowspec format: %s", filePath) return downloadIncludeFromWorkflowSpec(filePath, cache) } diff --git a/pkg/parser/remote_workflow_spec.go b/pkg/parser/remote_workflow_spec.go index 4c086b42b20..eda6738c9ba 100644 --- a/pkg/parser/remote_workflow_spec.go +++ b/pkg/parser/remote_workflow_spec.go @@ -61,10 +61,6 @@ func IsWorkflowSpec(path string) bool { return true } -func isWorkflowSpec(path string) bool { - return IsWorkflowSpec(path) -} - // downloadIncludeFromWorkflowSpec downloads an include file from GitHub using workflowspec. // It first checks the cache, and only downloads if not cached. //