From deef273d13616c7649c5aa8cc4ded84d28c3d8e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 06:32:02 +0000 Subject: [PATCH 1/4] Initial plan From cfdd57550ab10bda84ad98113a61ce82b3c8e557 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 06:59:21 +0000 Subject: [PATCH 2/4] fix(trial): fetch entire .github folder to include runtime-import dependencies When `gh aw trial` installs a remote workflow, it only fetched individual declared dependencies (frontmatter `imports:`, `@include` directives, `dispatch-workflow`, etc.) but missed files referenced by `{{#runtime-import ...}}` macros in the workflow body (e.g. skills under `.github/skills/`). Add `copySourceRepoGitHubFolder` which performs a sparse git checkout of the entire `.github/` directory from the source repository at the exact same ref as the workflow and merges it into the trial host directory. Existing files (e.g. the trial-modified workflow with `source:` frontmatter already injected) are preserved via a skip-if-exists strategy in the new `mergeDirectory` helper. `fetchAllRemoteDependencies` is kept to handle cross-repo `@include`, dispatch-workflow, call-workflow, and resources dependencies. Closes #47147 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/trial_repository.go | 151 +++++++++++++++++++++++++++++++ pkg/cli/trial_repository_test.go | 138 ++++++++++++++++++++++++++++ 2 files changed, 289 insertions(+) diff --git a/pkg/cli/trial_repository.go b/pkg/cli/trial_repository.go index 2aafd7be8b7..ec5bea73462 100644 --- a/pkg/cli/trial_repository.go +++ b/pkg/cli/trial_repository.go @@ -3,6 +3,7 @@ package cli import ( "context" "fmt" + "io/fs" "os" "os/exec" "path/filepath" @@ -13,6 +14,7 @@ import ( "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/constants" "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/workflow" ) @@ -308,6 +310,16 @@ func installWorkflowInTrialMode(ctx context.Context, tempDir string, parsedSpec // Fetch and save all remote dependencies (includes, imports, dispatch workflows, resources) if !fetched.IsLocal { + // Copy the entire .github/ folder from the source repo so that runtime-import + // files (e.g. .github/skills/*/SKILL.md) and other local dependencies are + // present in the trial host before compilation. Files already written to tempDir + // (such as the trial-modified workflow) are preserved. + if err := copySourceRepoGitHubFolder(ctx, parsedSpec, tempDir, opts.Verbose); err != nil { + if opts.Verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to copy .github folder from source: %v", err))) + } + // Non-fatal: fall through to individual dependency fetching below + } if err := fetchAllRemoteDependencies(ctx, string(content), parsedSpec, result.WorkflowsDir, opts.Verbose, true, nil); err != nil { return fmt.Errorf("failed to fetch remote dependencies: %w", err) } @@ -606,3 +618,142 @@ func cloneRepoContentsIntoHost(cloneRepoSlug string, cloneRepoVersion string, ho return nil } + +// copySourceRepoGitHubFolder performs a sparse checkout of the .github/ directory +// from the source repository at the workflow's ref and merges it into destDir. +// Files that already exist in destDir are not overwritten, so the trial-modified +// workflow file is preserved. +// This ensures runtime-import files (e.g. .github/skills/*/SKILL.md) and any other +// .github/ resources are present in the trial host before compilation. +func copySourceRepoGitHubFolder(ctx context.Context, spec *WorkflowSpec, destDir string, verbose bool) error { + if spec.RepoSlug == "" { + return nil + } + + ref := spec.Version + if ref == "" { + defaultBranch, err := getRepoDefaultBranch(ctx, spec.RepoSlug) + if err != nil { + trialRepoLog.Printf("Failed to resolve default branch for %s, falling back to 'main': %v", spec.RepoSlug, err) + ref = "main" + } else { + ref = defaultBranch + } + } + + trialRepoLog.Printf("Copying .github folder from source: repo=%s, ref=%s, destDir=%s", spec.RepoSlug, ref, destDir) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Fetching .github folder from %s@%s", spec.RepoSlug, ref))) + } + + tmpDir, err := os.MkdirTemp("", "gh-aw-github-folder-*") + if err != nil { + return fmt.Errorf("failed to create temp directory: %w", err) + } + defer os.RemoveAll(tmpDir) + + githubHost := getGitHubHostForRepo(spec.RepoSlug) + // #nosec G204 -- repoURL and ref are parsed from a developer-authored workflow spec URL; + // exec.CommandContext with separate args (not shell execution) prevents shell injection. + repoURL := fmt.Sprintf("%s/%s.git", githubHost, spec.RepoSlug) + + // Initialize a bare git repository + if out, err := exec.CommandContext(ctx, "git", "-C", tmpDir, "init").CombinedOutput(); err != nil { + return fmt.Errorf("failed to initialize git repository: %w\nOutput: %s", err, string(out)) + } + + // Add remote + if out, err := exec.CommandContext(ctx, "git", "-C", tmpDir, "remote", "add", "origin", repoURL).CombinedOutput(); err != nil { + return fmt.Errorf("failed to add remote: %w\nOutput: %s", err, string(out)) + } + + // Enable sparse checkout so only .github/ is checked out (saves bandwidth) + if out, err := exec.CommandContext(ctx, "git", "-C", tmpDir, "config", "core.sparseCheckout", "true").CombinedOutput(); err != nil { + return fmt.Errorf("failed to enable sparse checkout: %w\nOutput: %s", err, string(out)) + } + sparseInfoDir := filepath.Join(tmpDir, ".git", "info") + if err := os.MkdirAll(sparseInfoDir, constants.DirPermPublic); err != nil { + return fmt.Errorf("failed to create sparse-checkout info directory: %w", err) + } + // Use owner-only read/write permissions (0600) for the sparse-checkout config file + if err := os.WriteFile(filepath.Join(sparseInfoDir, "sparse-checkout"), []byte(".github/\n"), constants.FilePermSensitive); err != nil { + return fmt.Errorf("failed to write sparse-checkout file: %w", err) + } + + // Fetch and checkout at the exact ref used by the workflow + isSHA := len(ref) == 40 && gitutil.IsHexString(ref) + if isSHA { + // For commit SHAs: attempt direct SHA fetch; fall back to full shallow fetch + fetchCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref) + if _, err := fetchCmd.CombinedOutput(); err != nil { + fetchCmd = exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin") + if out, err := fetchCmd.CombinedOutput(); err != nil { + return fmt.Errorf("failed to fetch repository: %w\nOutput: %s", err, string(out)) + } + } + checkoutCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "checkout", ref) + if out, err := checkoutCmd.CombinedOutput(); err != nil { + return fmt.Errorf("failed to checkout commit %s: %w\nOutput: %s", ref, err, string(out)) + } + } else { + fetchCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref) + if out, err := fetchCmd.CombinedOutput(); err != nil { + return fmt.Errorf("failed to fetch ref %s: %w\nOutput: %s", ref, err, string(out)) + } + checkoutCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "checkout", "FETCH_HEAD") + if out, err := checkoutCmd.CombinedOutput(); err != nil { + return fmt.Errorf("failed to checkout FETCH_HEAD: %w\nOutput: %s", err, string(out)) + } + } + + // Merge the .github/ directory from the clone into destDir, skipping existing files + srcGitHubDir := filepath.Join(tmpDir, ".github") + if _, err := os.Stat(srcGitHubDir); os.IsNotExist(err) { + trialRepoLog.Printf("No .github directory found in source repo %s@%s", spec.RepoSlug, ref) + return nil + } + + dstGitHubDir := filepath.Join(destDir, ".github") + if err := mergeDirectory(srcGitHubDir, dstGitHubDir); err != nil { + return fmt.Errorf("failed to merge .github folder into trial host: %w", err) + } + + trialRepoLog.Printf("Successfully copied .github folder from %s@%s to %s", spec.RepoSlug, ref, destDir) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Fetched .github folder from "+spec.RepoSlug)) + } + return nil +} + +// mergeDirectory copies all files from src to dst recursively, creating directories +// as needed. Files that already exist in dst are not overwritten. +func mergeDirectory(src, dst string) error { + return filepath.WalkDir(src, func(srcPath string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + relPath, err := filepath.Rel(src, srcPath) + if err != nil { + return fmt.Errorf("failed to compute relative path for %s: %w", srcPath, err) + } + + dstPath := filepath.Join(dst, relPath) + + // Security: ensure the destination path does not escape the destination directory + if validateErr := fileutil.ValidatePathWithinBase(dst, dstPath); validateErr != nil { + return fmt.Errorf("refusing to copy file outside destination directory: %w", validateErr) + } + + if d.IsDir() { + return os.MkdirAll(dstPath, constants.DirPermPublic) + } + + // Skip files that already exist in the destination (preserve trial-modified versions) + if _, statErr := os.Stat(dstPath); statErr == nil { + return nil + } + + return fileutil.CopyFile(srcPath, dstPath) + }) +} diff --git a/pkg/cli/trial_repository_test.go b/pkg/cli/trial_repository_test.go index 37f8e289378..cf793fd95ff 100644 --- a/pkg/cli/trial_repository_test.go +++ b/pkg/cli/trial_repository_test.go @@ -3,7 +3,9 @@ package cli import ( + "os" "os/exec" + "path/filepath" "strings" "testing" ) @@ -151,3 +153,139 @@ func TestGetCurrentBranchIn(t *testing.T) { } }) } + +// TestMergeDirectory verifies the mergeDirectory helper that copies the source .github/ +// folder into the trial host directory. +func TestMergeDirectory(t *testing.T) { + t.Run("copies new files from src to dst", func(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + + if err := os.WriteFile(filepath.Join(src, "file.md"), []byte("hello"), 0644); err != nil { + t.Fatal(err) + } + + if err := mergeDirectory(src, dst); err != nil { + t.Fatalf("mergeDirectory() unexpected error: %v", err) + } + + content, err := os.ReadFile(filepath.Join(dst, "file.md")) + if err != nil { + t.Fatalf("expected file.md to exist in dst: %v", err) + } + if string(content) != "hello" { + t.Fatalf("expected 'hello', got %q", string(content)) + } + }) + + t.Run("does not overwrite existing files in dst", func(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + + if err := os.WriteFile(filepath.Join(src, "workflow.md"), []byte("source version"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dst, "workflow.md"), []byte("trial version"), 0644); err != nil { + t.Fatal(err) + } + + if err := mergeDirectory(src, dst); err != nil { + t.Fatalf("mergeDirectory() unexpected error: %v", err) + } + + content, err := os.ReadFile(filepath.Join(dst, "workflow.md")) + if err != nil { + t.Fatal(err) + } + if string(content) != "trial version" { + t.Fatalf("expected dst file to be preserved, got %q", string(content)) + } + }) + + t.Run("creates nested directories and copies files", func(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + + skillDir := filepath.Join(src, "skills", "my-skill") + if err := os.MkdirAll(skillDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("skill content"), 0644); err != nil { + t.Fatal(err) + } + + if err := mergeDirectory(src, dst); err != nil { + t.Fatalf("mergeDirectory() unexpected error: %v", err) + } + + dstSkillFile := filepath.Join(dst, "skills", "my-skill", "SKILL.md") + content, err := os.ReadFile(dstSkillFile) + if err != nil { + t.Fatalf("expected SKILL.md to exist in dst: %v", err) + } + if string(content) != "skill content" { + t.Fatalf("expected 'skill content', got %q", string(content)) + } + }) + + t.Run("preserves existing workflow while copying new skill files", func(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + + // Source has both a workflow file and a skill file + workflowDir := filepath.Join(src, "workflows") + if err := os.MkdirAll(workflowDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(workflowDir, "example.md"), []byte("source workflow"), 0644); err != nil { + t.Fatal(err) + } + skillDir := filepath.Join(src, "skills", "example") + if err := os.MkdirAll(skillDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("skill content"), 0644); err != nil { + t.Fatal(err) + } + + // Destination already has a trial-modified version of the workflow + dstWorkflowDir := filepath.Join(dst, "workflows") + if err := os.MkdirAll(dstWorkflowDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dstWorkflowDir, "example.md"), []byte("trial workflow with source field"), 0644); err != nil { + t.Fatal(err) + } + + if err := mergeDirectory(src, dst); err != nil { + t.Fatalf("mergeDirectory() unexpected error: %v", err) + } + + // Workflow must not be overwritten + wfContent, err := os.ReadFile(filepath.Join(dst, "workflows", "example.md")) + if err != nil { + t.Fatal(err) + } + if string(wfContent) != "trial workflow with source field" { + t.Fatalf("trial workflow was overwritten; got %q", string(wfContent)) + } + + // Skill file must be copied + skillContent, err := os.ReadFile(filepath.Join(dst, "skills", "example", "SKILL.md")) + if err != nil { + t.Fatalf("expected SKILL.md to be copied: %v", err) + } + if string(skillContent) != "skill content" { + t.Fatalf("expected 'skill content', got %q", string(skillContent)) + } + }) + + t.Run("returns nil for empty src directory", func(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + + if err := mergeDirectory(src, dst); err != nil { + t.Fatalf("mergeDirectory() unexpected error for empty src: %v", err) + } + }) +} From 12d90e1eef0afd57e1c87c211e13b2b40bb2010c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:35:20 +0000 Subject: [PATCH 3/4] docs(adr): draft ADR-48787 for .github/ sparse-checkout approach in trial mode --- ...ub-folder-for-runtime-import-resolution.md | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 docs/adr/48787-fetch-github-folder-for-runtime-import-resolution.md diff --git a/docs/adr/48787-fetch-github-folder-for-runtime-import-resolution.md b/docs/adr/48787-fetch-github-folder-for-runtime-import-resolution.md new file mode 100644 index 00000000000..9cec7f58aca --- /dev/null +++ b/docs/adr/48787-fetch-github-folder-for-runtime-import-resolution.md @@ -0,0 +1,44 @@ +# ADR-48787: Fetch Entire `.github/` Folder for Runtime-Import Dependency Resolution + +**Date**: 2026-07-29 +**Status**: Draft +**Deciders**: pelikhan, copilot-swe-agent + +--- + +### Context + +The `gh aw trial` command installs a workflow in trial mode by fetching its declared dependencies: frontmatter `imports:`, `@include` directives, dispatch/call workflows, and resources. However, workflows may also reference files via `{{#runtime-import .github/path/to/file}}` macros in the workflow body. These macro-resolved paths were never traversed by the explicit dependency scanner, so any workflow using `runtime-import` would fail during template interpolation with "Runtime import file not found." The root cause is that `.github/`-local files are an implicit, untracked dependency class with no manifest. + +### Decision + +We will perform a sparse checkout of the entire `.github/` directory from the source repository at the workflow's exact ref before calling `fetchAllRemoteDependencies`. The sparse-checked-out directory is merged into the trial host using a non-destructive copy that skips existing files, preserving the already-injected trial workflow. The `.github/` copy step is non-fatal: if it fails, trial install falls through to the existing individual dependency fetching path. + +### Alternatives Considered + +#### Alternative 1: Extend the dependency scanner to parse `{{#runtime-import}}` macros + +The dependency scanner could be taught to recognize `{{#runtime-import ...}}` syntax and add the resolved paths to the explicit fetch list handled by `fetchAllRemoteDependencies`. This would be surgical — only the referenced files are fetched — but requires maintaining a parser for macro syntax. It would also miss any future macro or implicit reference pattern not yet in the scanner, meaning the fix would be fragile against new macro types. The current diff shows no attempt to extend the scanner, indicating this was considered and rejected in favor of a more complete solution. + +#### Alternative 2: Fail fast and require authors to declare all `runtime-import` files as explicit dependencies + +Trial mode could require workflow authors to also list `runtime-import` targets in the frontmatter `imports:` field. This places the burden on the author, breaks existing workflows that already use the macro without redundant declarations, and would make the UX inconsistent (why does `@include` work automatically but `runtime-import` doesn't?). The bug report (issue #47147) confirms this degraded experience is the problem being fixed. + +### Consequences + +#### Positive +- All `.github/`-local resources — including `runtime-import` files, skills, and any future implicit dependency patterns — are automatically available in the trial host without requiring workflow authors to redeclare them. +- The non-fatal fallback preserves the existing individual dependency fetch path, so trial install degrades gracefully if the sparse checkout fails (e.g., network issue or unsupported host). +- Path traversal is guarded via `fileutil.ValidatePathWithinBase`, preventing symlink or `..`-escape attacks during the merge. + +#### Negative +- Every non-local trial activation now performs an additional git sparse checkout, increasing network I/O even for workflows that do not use `runtime-import`. Repositories with large `.github/` directories will incur non-trivial latency. +- The sparse checkout creates and immediately removes a temporary directory per activation, adding filesystem churn. + +#### Neutral +- `fetchAllRemoteDependencies` is retained alongside the new step: cross-repo `@include`, dispatch, and resource dependencies still require individual fetch logic, so the two mechanisms coexist rather than one replacing the other. +- The `mergeDirectory` skip-existing semantics mean that future `.github/` changes at the ref are not visible once the trial-modified workflow has been written, which is intentional but may surprise debuggers. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 492cdfb7147765ec4fd63f92d15dedb90b868a8b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:54:56 +0000 Subject: [PATCH 4/4] fix(trial): fetch pinned runtime-import dependencies Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- ...ub-folder-for-runtime-import-resolution.md | 44 ----- pkg/cli/includes.go | 124 ++++++++++++++ pkg/cli/remote_workflow_test.go | 73 ++++++++ pkg/cli/trial_repository.go | 158 +----------------- pkg/cli/trial_repository_test.go | 138 --------------- 5 files changed, 203 insertions(+), 334 deletions(-) delete mode 100644 docs/adr/48787-fetch-github-folder-for-runtime-import-resolution.md diff --git a/docs/adr/48787-fetch-github-folder-for-runtime-import-resolution.md b/docs/adr/48787-fetch-github-folder-for-runtime-import-resolution.md deleted file mode 100644 index 9cec7f58aca..00000000000 --- a/docs/adr/48787-fetch-github-folder-for-runtime-import-resolution.md +++ /dev/null @@ -1,44 +0,0 @@ -# ADR-48787: Fetch Entire `.github/` Folder for Runtime-Import Dependency Resolution - -**Date**: 2026-07-29 -**Status**: Draft -**Deciders**: pelikhan, copilot-swe-agent - ---- - -### Context - -The `gh aw trial` command installs a workflow in trial mode by fetching its declared dependencies: frontmatter `imports:`, `@include` directives, dispatch/call workflows, and resources. However, workflows may also reference files via `{{#runtime-import .github/path/to/file}}` macros in the workflow body. These macro-resolved paths were never traversed by the explicit dependency scanner, so any workflow using `runtime-import` would fail during template interpolation with "Runtime import file not found." The root cause is that `.github/`-local files are an implicit, untracked dependency class with no manifest. - -### Decision - -We will perform a sparse checkout of the entire `.github/` directory from the source repository at the workflow's exact ref before calling `fetchAllRemoteDependencies`. The sparse-checked-out directory is merged into the trial host using a non-destructive copy that skips existing files, preserving the already-injected trial workflow. The `.github/` copy step is non-fatal: if it fails, trial install falls through to the existing individual dependency fetching path. - -### Alternatives Considered - -#### Alternative 1: Extend the dependency scanner to parse `{{#runtime-import}}` macros - -The dependency scanner could be taught to recognize `{{#runtime-import ...}}` syntax and add the resolved paths to the explicit fetch list handled by `fetchAllRemoteDependencies`. This would be surgical — only the referenced files are fetched — but requires maintaining a parser for macro syntax. It would also miss any future macro or implicit reference pattern not yet in the scanner, meaning the fix would be fragile against new macro types. The current diff shows no attempt to extend the scanner, indicating this was considered and rejected in favor of a more complete solution. - -#### Alternative 2: Fail fast and require authors to declare all `runtime-import` files as explicit dependencies - -Trial mode could require workflow authors to also list `runtime-import` targets in the frontmatter `imports:` field. This places the burden on the author, breaks existing workflows that already use the macro without redundant declarations, and would make the UX inconsistent (why does `@include` work automatically but `runtime-import` doesn't?). The bug report (issue #47147) confirms this degraded experience is the problem being fixed. - -### Consequences - -#### Positive -- All `.github/`-local resources — including `runtime-import` files, skills, and any future implicit dependency patterns — are automatically available in the trial host without requiring workflow authors to redeclare them. -- The non-fatal fallback preserves the existing individual dependency fetch path, so trial install degrades gracefully if the sparse checkout fails (e.g., network issue or unsupported host). -- Path traversal is guarded via `fileutil.ValidatePathWithinBase`, preventing symlink or `..`-escape attacks during the merge. - -#### Negative -- Every non-local trial activation now performs an additional git sparse checkout, increasing network I/O even for workflows that do not use `runtime-import`. Repositories with large `.github/` directories will incur non-trivial latency. -- The sparse checkout creates and immediately removes a temporary directory per activation, adding filesystem churn. - -#### Neutral -- `fetchAllRemoteDependencies` is retained alongside the new step: cross-repo `@include`, dispatch, and resource dependencies still require individual fetch logic, so the two mechanisms coexist rather than one replacing the other. -- The `mergeDirectory` skip-existing semantics mean that future `.github/` changes at the ref are not visible once the trial-modified workflow has been written, which is intentional but may surprise debuggers. - ---- - -*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/cli/includes.go b/pkg/cli/includes.go index 3aedb871723..bf6a0d9baa4 100644 --- a/pkg/cli/includes.go +++ b/pkg/cli/includes.go @@ -22,12 +22,128 @@ import ( // includeDirectivePattern matches @include or @include? directives with their path argument var includeDirectivePattern = regexp.MustCompile(`^@include(\?)?\s+(.+)$`) var downloadRemoteImportFile = parser.DownloadFileFromGitHub +var downloadRemoteRuntimeImportFile = parser.DownloadFileFromGitHubForHost // includesFetcher is the function type used by fetchAndSaveRemoteIncludes to retrieve // a single include file. Passing a non-nil value overrides the default FetchIncludeFromSource // implementation; this is used in tests to avoid real network calls. type includesFetcher func(ctx context.Context, includePath string, baseSpec *WorkflowSpec, verbose bool) ([]byte, string, error) +type runtimeImportFetcher func(ctx context.Context, owner, repo, path, ref, host string) ([]byte, error) + +type runtimeImportOpts struct { + owner string + repo string + ref string + host string + repoRoot string + verbose bool + force bool + tracker *FileTracker + seen map[string]struct{} + downloadFn runtimeImportFetcher +} + +func fetchAndSaveRemoteRuntimeImports(ctx context.Context, content string, spec *WorkflowSpec, targetDir string, verbose bool, force bool, tracker *FileTracker) error { + if spec.RepoSlug == "" { + return nil + } + + parts := strings.SplitN(spec.RepoSlug, "/", 2) + if len(parts) != 2 { + return nil + } + + ref := spec.Version + if ref == "" { + defaultBranch, err := getRepoDefaultBranch(ctx, spec.RepoSlug) + if err != nil { + remoteWorkflowLog.Printf("Failed to resolve default branch for %s, falling back to 'main': %v", spec.RepoSlug, err) + ref = "main" + } else { + ref = defaultBranch + } + spec.Version = ref + } + + opts := runtimeImportOpts{ + owner: parts[0], + repo: parts[1], + ref: ref, + host: spec.Host, + repoRoot: filepath.Dir(filepath.Dir(targetDir)), + verbose: verbose, + force: force, + tracker: tracker, + seen: make(map[string]struct{}), + downloadFn: downloadRemoteRuntimeImportFile, + } + + return fetchRemoteRuntimeImportsRecursive(ctx, content, getParentDir(spec.WorkflowPath), opts) +} + +func fetchRemoteRuntimeImportsRecursive(ctx context.Context, content, currentBaseDir string, opts runtimeImportOpts) error { + body := content + if parsed, err := parser.ExtractFrontmatterFromContent(content); err == nil { + body = parsed.Markdown + } + + for _, imp := range parser.ExtractBodyLevelImportPaths(body, currentBaseDir) { + remoteFilePath := strings.TrimSpace(imp.Path) + if remoteFilePath == "" { + continue + } + if rest, ok := strings.CutPrefix(remoteFilePath, "/"); ok { + remoteFilePath = rest + } + remoteFilePath = path.Clean(remoteFilePath) + if remoteFilePath == "." || remoteFilePath == ".." || strings.HasPrefix(remoteFilePath, "../") { + return fmt.Errorf("runtime-import path %q escapes repository root", imp.Path) + } + if setutil.Contains(opts.seen, remoteFilePath) { + continue + } + opts.seen[remoteFilePath] = struct{}{} + + targetPath := filepath.Join(opts.repoRoot, filepath.FromSlash(remoteFilePath)) + if err := fileutil.ValidatePathWithinBase(opts.repoRoot, targetPath); err != nil { + return fmt.Errorf("refusing to write runtime import outside repository root: %w", err) + } + + fileExists := false + if fileutil.FileExists(targetPath) { + fileExists = true + if !opts.force { + continue + } + } + + importContent, err := opts.downloadFn(ctx, opts.owner, opts.repo, remoteFilePath, opts.ref, opts.host) + if err != nil { + return fmt.Errorf("failed to fetch runtime import %s: %w", remoteFilePath, err) + } + if err := os.MkdirAll(filepath.Dir(targetPath), constants.DirPermPublic); err != nil { + return fmt.Errorf("failed to create directory for runtime import %s: %w", remoteFilePath, err) + } + if err := os.WriteFile(targetPath, importContent, constants.FilePermSensitive); err != nil { + return fmt.Errorf("failed to write runtime import %s: %w", remoteFilePath, err) + } + if opts.tracker != nil { + if fileExists { + opts.tracker.TrackModified(targetPath) + } else { + opts.tracker.TrackCreated(targetPath) + } + } + + if err := fetchRemoteRuntimeImportsRecursive(ctx, string(importContent), path.Dir(remoteFilePath), opts); err != nil { + return err + } + } + + return nil +} + // FetchIncludeFromSource fetches an include file from GitHub directly using a workflowspec format path. // The includePath should be in the format: owner/repo/path/to/file.md[@ref] // If the includePath is a relative path, it's resolved relative to the baseSpec. @@ -586,6 +702,14 @@ func fetchAllRemoteDependencies(ctx context.Context, content string, spec *Workf fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch frontmatter import dependencies: %v", err))) } } + // Fetch and save required runtime-import dependencies so installs include the + // explicit runtime-import closure without copying unrelated .github contents. + if err := fetchAndSaveRemoteRuntimeImports(ctx, content, spec, targetDir, verbose, force, tracker); err != nil { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Failed to fetch runtime-import dependencies; activation may fail")) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(err.Error())) + } + } // Fetch and save workflows referenced in safe-outputs.dispatch-workflow so they are // available locally. Workflow names using GitHub Actions expression syntax are skipped. if err := fetchAndSaveRemoteDispatchWorkflows(ctx, content, spec, targetDir, verbose, force, tracker); err != nil { diff --git a/pkg/cli/remote_workflow_test.go b/pkg/cli/remote_workflow_test.go index d136d65935f..40119e4ce00 100644 --- a/pkg/cli/remote_workflow_test.go +++ b/pkg/cli/remote_workflow_test.go @@ -858,6 +858,79 @@ imports: "transitive dep must not be written at the root level") } +func TestFetchAndSaveRemoteRuntimeImports_EmptyRepoSlug(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0o755)) + + err := fetchAndSaveRemoteRuntimeImports( + t.Context(), + "{{#runtime-import .github/skills/example/SKILL.md}}\n", + &WorkflowSpec{}, + workflowsDir, + false, + false, + nil, + ) + require.NoError(t, err) + assert.NoFileExists(t, filepath.Join(tmpDir, ".github", "skills", "example", "SKILL.md")) +} + +func TestFetchAndSaveRemoteRuntimeImports_FetchesPinnedRecursiveClosure(t *testing.T) { + original := downloadRemoteRuntimeImportFile + defer func() { downloadRemoteRuntimeImportFile = original }() + + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0o755)) + + type request struct { + path string + ref string + host string + } + var requests []request + downloadRemoteRuntimeImportFile = func(_ context.Context, owner, repo, remotePath, ref, host string) ([]byte, error) { + assert.Equal(t, "github", owner) + assert.Equal(t, "gh-aw", repo) + requests = append(requests, request{path: remotePath, ref: ref, host: host}) + switch remotePath { + case ".github/skills/example/SKILL.md": + return []byte("---\nname: Example\n---\n{{#runtime-import ./nested.md}}\n"), nil + case ".github/skills/example/nested.md": + return []byte("nested\n"), nil + default: + return nil, fmt.Errorf("unexpected path %s", remotePath) + } + } + + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "github/gh-aw", + Version: "0123456789abcdef0123456789abcdef01234567", + }, + WorkflowPath: ".github/workflows/example.md", + Host: "github.com", + } + + err := fetchAndSaveRemoteRuntimeImports( + t.Context(), + "{{#runtime-import .github/skills/example/SKILL.md}}\n", + spec, + workflowsDir, + false, + true, + nil, + ) + require.NoError(t, err) + assert.Equal(t, []request{ + {path: ".github/skills/example/SKILL.md", ref: "0123456789abcdef0123456789abcdef01234567", host: "github.com"}, + {path: ".github/skills/example/nested.md", ref: "0123456789abcdef0123456789abcdef01234567", host: "github.com"}, + }, requests) + assert.FileExists(t, filepath.Join(tmpDir, ".github", "skills", "example", "SKILL.md")) + assert.FileExists(t, filepath.Join(tmpDir, ".github", "skills", "example", "nested.md")) +} + // TestFetchFrontmatterImportsRecursive_RepoRootSlashPath verifies that when // originalBaseDir is empty (workflow lives at the repo root) and an import // path contains a "/" (e.g. "shared/helper.md"), the path is used as-is diff --git a/pkg/cli/trial_repository.go b/pkg/cli/trial_repository.go index ec5bea73462..085de5bb6ee 100644 --- a/pkg/cli/trial_repository.go +++ b/pkg/cli/trial_repository.go @@ -3,7 +3,6 @@ package cli import ( "context" "fmt" - "io/fs" "os" "os/exec" "path/filepath" @@ -14,7 +13,6 @@ import ( "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/constants" "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/workflow" ) @@ -308,19 +306,14 @@ func installWorkflowInTrialMode(ctx context.Context, tempDir string, parsedSpec } } - // Fetch and save all remote dependencies (includes, imports, dispatch workflows, resources) + // Fetch and save all remote dependencies (includes, imports, runtime imports, + // dispatch workflows, resources) using the exact commit resolved for this install. if !fetched.IsLocal { - // Copy the entire .github/ folder from the source repo so that runtime-import - // files (e.g. .github/skills/*/SKILL.md) and other local dependencies are - // present in the trial host before compilation. Files already written to tempDir - // (such as the trial-modified workflow) are preserved. - if err := copySourceRepoGitHubFolder(ctx, parsedSpec, tempDir, opts.Verbose); err != nil { - if opts.Verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to copy .github folder from source: %v", err))) - } - // Non-fatal: fall through to individual dependency fetching below + depSpec := *parsedSpec + if fetched.CommitSHA != "" { + depSpec.Version = fetched.CommitSHA } - if err := fetchAllRemoteDependencies(ctx, string(content), parsedSpec, result.WorkflowsDir, opts.Verbose, true, nil); err != nil { + if err := fetchAllRemoteDependencies(ctx, string(content), &depSpec, result.WorkflowsDir, opts.Verbose, true, nil); err != nil { return fmt.Errorf("failed to fetch remote dependencies: %w", err) } } @@ -618,142 +611,3 @@ func cloneRepoContentsIntoHost(cloneRepoSlug string, cloneRepoVersion string, ho return nil } - -// copySourceRepoGitHubFolder performs a sparse checkout of the .github/ directory -// from the source repository at the workflow's ref and merges it into destDir. -// Files that already exist in destDir are not overwritten, so the trial-modified -// workflow file is preserved. -// This ensures runtime-import files (e.g. .github/skills/*/SKILL.md) and any other -// .github/ resources are present in the trial host before compilation. -func copySourceRepoGitHubFolder(ctx context.Context, spec *WorkflowSpec, destDir string, verbose bool) error { - if spec.RepoSlug == "" { - return nil - } - - ref := spec.Version - if ref == "" { - defaultBranch, err := getRepoDefaultBranch(ctx, spec.RepoSlug) - if err != nil { - trialRepoLog.Printf("Failed to resolve default branch for %s, falling back to 'main': %v", spec.RepoSlug, err) - ref = "main" - } else { - ref = defaultBranch - } - } - - trialRepoLog.Printf("Copying .github folder from source: repo=%s, ref=%s, destDir=%s", spec.RepoSlug, ref, destDir) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Fetching .github folder from %s@%s", spec.RepoSlug, ref))) - } - - tmpDir, err := os.MkdirTemp("", "gh-aw-github-folder-*") - if err != nil { - return fmt.Errorf("failed to create temp directory: %w", err) - } - defer os.RemoveAll(tmpDir) - - githubHost := getGitHubHostForRepo(spec.RepoSlug) - // #nosec G204 -- repoURL and ref are parsed from a developer-authored workflow spec URL; - // exec.CommandContext with separate args (not shell execution) prevents shell injection. - repoURL := fmt.Sprintf("%s/%s.git", githubHost, spec.RepoSlug) - - // Initialize a bare git repository - if out, err := exec.CommandContext(ctx, "git", "-C", tmpDir, "init").CombinedOutput(); err != nil { - return fmt.Errorf("failed to initialize git repository: %w\nOutput: %s", err, string(out)) - } - - // Add remote - if out, err := exec.CommandContext(ctx, "git", "-C", tmpDir, "remote", "add", "origin", repoURL).CombinedOutput(); err != nil { - return fmt.Errorf("failed to add remote: %w\nOutput: %s", err, string(out)) - } - - // Enable sparse checkout so only .github/ is checked out (saves bandwidth) - if out, err := exec.CommandContext(ctx, "git", "-C", tmpDir, "config", "core.sparseCheckout", "true").CombinedOutput(); err != nil { - return fmt.Errorf("failed to enable sparse checkout: %w\nOutput: %s", err, string(out)) - } - sparseInfoDir := filepath.Join(tmpDir, ".git", "info") - if err := os.MkdirAll(sparseInfoDir, constants.DirPermPublic); err != nil { - return fmt.Errorf("failed to create sparse-checkout info directory: %w", err) - } - // Use owner-only read/write permissions (0600) for the sparse-checkout config file - if err := os.WriteFile(filepath.Join(sparseInfoDir, "sparse-checkout"), []byte(".github/\n"), constants.FilePermSensitive); err != nil { - return fmt.Errorf("failed to write sparse-checkout file: %w", err) - } - - // Fetch and checkout at the exact ref used by the workflow - isSHA := len(ref) == 40 && gitutil.IsHexString(ref) - if isSHA { - // For commit SHAs: attempt direct SHA fetch; fall back to full shallow fetch - fetchCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref) - if _, err := fetchCmd.CombinedOutput(); err != nil { - fetchCmd = exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin") - if out, err := fetchCmd.CombinedOutput(); err != nil { - return fmt.Errorf("failed to fetch repository: %w\nOutput: %s", err, string(out)) - } - } - checkoutCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "checkout", ref) - if out, err := checkoutCmd.CombinedOutput(); err != nil { - return fmt.Errorf("failed to checkout commit %s: %w\nOutput: %s", ref, err, string(out)) - } - } else { - fetchCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "fetch", "--depth", "1", "origin", ref) - if out, err := fetchCmd.CombinedOutput(); err != nil { - return fmt.Errorf("failed to fetch ref %s: %w\nOutput: %s", ref, err, string(out)) - } - checkoutCmd := exec.CommandContext(ctx, "git", "-C", tmpDir, "checkout", "FETCH_HEAD") - if out, err := checkoutCmd.CombinedOutput(); err != nil { - return fmt.Errorf("failed to checkout FETCH_HEAD: %w\nOutput: %s", err, string(out)) - } - } - - // Merge the .github/ directory from the clone into destDir, skipping existing files - srcGitHubDir := filepath.Join(tmpDir, ".github") - if _, err := os.Stat(srcGitHubDir); os.IsNotExist(err) { - trialRepoLog.Printf("No .github directory found in source repo %s@%s", spec.RepoSlug, ref) - return nil - } - - dstGitHubDir := filepath.Join(destDir, ".github") - if err := mergeDirectory(srcGitHubDir, dstGitHubDir); err != nil { - return fmt.Errorf("failed to merge .github folder into trial host: %w", err) - } - - trialRepoLog.Printf("Successfully copied .github folder from %s@%s to %s", spec.RepoSlug, ref, destDir) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Fetched .github folder from "+spec.RepoSlug)) - } - return nil -} - -// mergeDirectory copies all files from src to dst recursively, creating directories -// as needed. Files that already exist in dst are not overwritten. -func mergeDirectory(src, dst string) error { - return filepath.WalkDir(src, func(srcPath string, d fs.DirEntry, err error) error { - if err != nil { - return err - } - - relPath, err := filepath.Rel(src, srcPath) - if err != nil { - return fmt.Errorf("failed to compute relative path for %s: %w", srcPath, err) - } - - dstPath := filepath.Join(dst, relPath) - - // Security: ensure the destination path does not escape the destination directory - if validateErr := fileutil.ValidatePathWithinBase(dst, dstPath); validateErr != nil { - return fmt.Errorf("refusing to copy file outside destination directory: %w", validateErr) - } - - if d.IsDir() { - return os.MkdirAll(dstPath, constants.DirPermPublic) - } - - // Skip files that already exist in the destination (preserve trial-modified versions) - if _, statErr := os.Stat(dstPath); statErr == nil { - return nil - } - - return fileutil.CopyFile(srcPath, dstPath) - }) -} diff --git a/pkg/cli/trial_repository_test.go b/pkg/cli/trial_repository_test.go index cf793fd95ff..37f8e289378 100644 --- a/pkg/cli/trial_repository_test.go +++ b/pkg/cli/trial_repository_test.go @@ -3,9 +3,7 @@ package cli import ( - "os" "os/exec" - "path/filepath" "strings" "testing" ) @@ -153,139 +151,3 @@ func TestGetCurrentBranchIn(t *testing.T) { } }) } - -// TestMergeDirectory verifies the mergeDirectory helper that copies the source .github/ -// folder into the trial host directory. -func TestMergeDirectory(t *testing.T) { - t.Run("copies new files from src to dst", func(t *testing.T) { - src := t.TempDir() - dst := t.TempDir() - - if err := os.WriteFile(filepath.Join(src, "file.md"), []byte("hello"), 0644); err != nil { - t.Fatal(err) - } - - if err := mergeDirectory(src, dst); err != nil { - t.Fatalf("mergeDirectory() unexpected error: %v", err) - } - - content, err := os.ReadFile(filepath.Join(dst, "file.md")) - if err != nil { - t.Fatalf("expected file.md to exist in dst: %v", err) - } - if string(content) != "hello" { - t.Fatalf("expected 'hello', got %q", string(content)) - } - }) - - t.Run("does not overwrite existing files in dst", func(t *testing.T) { - src := t.TempDir() - dst := t.TempDir() - - if err := os.WriteFile(filepath.Join(src, "workflow.md"), []byte("source version"), 0644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(dst, "workflow.md"), []byte("trial version"), 0644); err != nil { - t.Fatal(err) - } - - if err := mergeDirectory(src, dst); err != nil { - t.Fatalf("mergeDirectory() unexpected error: %v", err) - } - - content, err := os.ReadFile(filepath.Join(dst, "workflow.md")) - if err != nil { - t.Fatal(err) - } - if string(content) != "trial version" { - t.Fatalf("expected dst file to be preserved, got %q", string(content)) - } - }) - - t.Run("creates nested directories and copies files", func(t *testing.T) { - src := t.TempDir() - dst := t.TempDir() - - skillDir := filepath.Join(src, "skills", "my-skill") - if err := os.MkdirAll(skillDir, 0755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("skill content"), 0644); err != nil { - t.Fatal(err) - } - - if err := mergeDirectory(src, dst); err != nil { - t.Fatalf("mergeDirectory() unexpected error: %v", err) - } - - dstSkillFile := filepath.Join(dst, "skills", "my-skill", "SKILL.md") - content, err := os.ReadFile(dstSkillFile) - if err != nil { - t.Fatalf("expected SKILL.md to exist in dst: %v", err) - } - if string(content) != "skill content" { - t.Fatalf("expected 'skill content', got %q", string(content)) - } - }) - - t.Run("preserves existing workflow while copying new skill files", func(t *testing.T) { - src := t.TempDir() - dst := t.TempDir() - - // Source has both a workflow file and a skill file - workflowDir := filepath.Join(src, "workflows") - if err := os.MkdirAll(workflowDir, 0755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(workflowDir, "example.md"), []byte("source workflow"), 0644); err != nil { - t.Fatal(err) - } - skillDir := filepath.Join(src, "skills", "example") - if err := os.MkdirAll(skillDir, 0755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("skill content"), 0644); err != nil { - t.Fatal(err) - } - - // Destination already has a trial-modified version of the workflow - dstWorkflowDir := filepath.Join(dst, "workflows") - if err := os.MkdirAll(dstWorkflowDir, 0755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(dstWorkflowDir, "example.md"), []byte("trial workflow with source field"), 0644); err != nil { - t.Fatal(err) - } - - if err := mergeDirectory(src, dst); err != nil { - t.Fatalf("mergeDirectory() unexpected error: %v", err) - } - - // Workflow must not be overwritten - wfContent, err := os.ReadFile(filepath.Join(dst, "workflows", "example.md")) - if err != nil { - t.Fatal(err) - } - if string(wfContent) != "trial workflow with source field" { - t.Fatalf("trial workflow was overwritten; got %q", string(wfContent)) - } - - // Skill file must be copied - skillContent, err := os.ReadFile(filepath.Join(dst, "skills", "example", "SKILL.md")) - if err != nil { - t.Fatalf("expected SKILL.md to be copied: %v", err) - } - if string(skillContent) != "skill content" { - t.Fatalf("expected 'skill content', got %q", string(skillContent)) - } - }) - - t.Run("returns nil for empty src directory", func(t *testing.T) { - src := t.TempDir() - dst := t.TempDir() - - if err := mergeDirectory(src, dst); err != nil { - t.Fatalf("mergeDirectory() unexpected error for empty src: %v", err) - } - }) -}