From bb7b7b8d6ab36631076dcbd3d20061d98276e126 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Thu, 26 Mar 2026 19:25:03 +0000 Subject: [PATCH 1/7] improve message --- actions/setup/js/extra_empty_commit.cjs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/actions/setup/js/extra_empty_commit.cjs b/actions/setup/js/extra_empty_commit.cjs index 101a0a2aee..183ad487cb 100644 --- a/actions/setup/js/extra_empty_commit.cjs +++ b/actions/setup/js/extra_empty_commit.cjs @@ -147,11 +147,12 @@ async function pushExtraEmptyCommit({ branchName, repoOwner, repoName, commitMes try { await exec.exec("git", ["fetch", "ci-trigger", branchName]); await exec.exec("git", ["reset", "--hard", `ci-trigger/${branchName}`]); - } catch { + } catch (error) { // Non-fatal: if fetch/reset fails (e.g. branch not yet on remote), continue + // with the local HEAD and attempt the push anyway. // Non-fatal: if fetch/reset fails (e.g. branch not yet on remote), continue // with the local HEAD and attempt the push anyway. - core.info(`Could not sync local branch with remote ${branchName} - will attempt push with local HEAD`); - } + const syncErrorMessage = error instanceof Error ? error.message : String(error); + core.warning(`Could not sync local branch with remote ${branchName} - will attempt push with local HEAD. Underlying error: ${syncErrorMessage}`); } // Create and push an empty commit const message = commitMessage || "ci: trigger checks"; From 3e7d94187fe7c6a57bcec61cb69fa83920d7a082 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Thu, 26 Mar 2026 20:03:30 +0000 Subject: [PATCH 2/7] Update actions/setup/js/extra_empty_commit.cjs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- actions/setup/js/extra_empty_commit.cjs | 2 -- 1 file changed, 2 deletions(-) diff --git a/actions/setup/js/extra_empty_commit.cjs b/actions/setup/js/extra_empty_commit.cjs index 183ad487cb..ed6a7d9e9e 100644 --- a/actions/setup/js/extra_empty_commit.cjs +++ b/actions/setup/js/extra_empty_commit.cjs @@ -148,8 +148,6 @@ async function pushExtraEmptyCommit({ branchName, repoOwner, repoName, commitMes await exec.exec("git", ["fetch", "ci-trigger", branchName]); await exec.exec("git", ["reset", "--hard", `ci-trigger/${branchName}`]); } catch (error) { // Non-fatal: if fetch/reset fails (e.g. branch not yet on remote), continue - // with the local HEAD and attempt the push anyway. - // Non-fatal: if fetch/reset fails (e.g. branch not yet on remote), continue // with the local HEAD and attempt the push anyway. const syncErrorMessage = error instanceof Error ? error.message : String(error); core.warning(`Could not sync local branch with remote ${branchName} - will attempt push with local HEAD. Underlying error: ${syncErrorMessage}`); } From bb013a04e62ea4c247f8df9c5b2eac64a613827d Mon Sep 17 00:00:00 2001 From: Don Syme Date: Thu, 26 Mar 2026 20:04:19 +0000 Subject: [PATCH 3/7] commit --- actions/setup/js/extra_empty_commit.cjs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/extra_empty_commit.cjs b/actions/setup/js/extra_empty_commit.cjs index ed6a7d9e9e..33a38e8797 100644 --- a/actions/setup/js/extra_empty_commit.cjs +++ b/actions/setup/js/extra_empty_commit.cjs @@ -147,10 +147,12 @@ async function pushExtraEmptyCommit({ branchName, repoOwner, repoName, commitMes try { await exec.exec("git", ["fetch", "ci-trigger", branchName]); await exec.exec("git", ["reset", "--hard", `ci-trigger/${branchName}`]); - } catch (error) { // Non-fatal: if fetch/reset fails (e.g. branch not yet on remote), continue + } catch (error) { + // Non-fatal: if fetch/reset fails (e.g. branch not yet on remote), continue // with the local HEAD and attempt the push anyway. const syncErrorMessage = error instanceof Error ? error.message : String(error); - core.warning(`Could not sync local branch with remote ${branchName} - will attempt push with local HEAD. Underlying error: ${syncErrorMessage}`); } + core.warning(`Could not sync local branch with remote ${branchName} - will attempt push with local HEAD. Underlying error: ${syncErrorMessage}`); + } // Create and push an empty commit const message = commitMessage || "ci: trigger checks"; From 98376647f992c580290653e5c955f518886a5d38 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Thu, 26 Mar 2026 20:20:11 +0000 Subject: [PATCH 4/7] fix: trial command missing remote dependency fetching for imports, dispatch workflows, and resources The trial command only called fetchAndSaveRemoteIncludes but not fetchAndSaveRemoteFrontmatterImports, fetchAndSaveRemoteDispatchWorkflows, or fetchAndSaveRemoteResources. This caused 'import file not found' errors when running 'gh aw trial' with workflows that use frontmatter imports (e.g. shared/formatting.md), while 'gh aw add' worked fine. Extract a shared fetchAllRemoteDependencies function in includes.go that both add_command.go and trial_repository.go now call, ensuring parity and preventing future drift. --- pkg/cli/add_command.go | 24 ++---------------------- pkg/cli/includes.go | 31 +++++++++++++++++++++++++++++++ pkg/cli/trial_repository.go | 8 +++----- 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index 2d721b34ac..b27193c037 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -358,29 +358,9 @@ func addWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTracker, o return fmt.Errorf("workflow '%s' already exists in .github/workflows/. Use a different name with -n flag, remove the existing workflow first, or use --force to overwrite", workflowName) } - // For remote workflows, fetch and save include dependencies directly from the source + // For remote workflows, fetch and save all dependencies (includes, imports, dispatch workflows, resources) if !isLocalWorkflowPath(workflowSpec.WorkflowPath) { - if err := fetchAndSaveRemoteIncludes(string(sourceContent), workflowSpec, githubWorkflowsDir, opts.Verbose, opts.Force, tracker); err != nil { - if opts.Verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch include dependencies: %v", err))) - } - } - // Also fetch and save frontmatter 'imports:' dependencies so they are available - // locally during compilation. Keeping these as relative paths (not workflowspecs) - // ensures the compiler resolves them from disk rather than downloading from GitHub. - if err := fetchAndSaveRemoteFrontmatterImports(string(sourceContent), workflowSpec, githubWorkflowsDir, opts.Verbose, opts.Force, tracker); err != nil { - if opts.Verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch frontmatter import dependencies: %v", err))) - } - } - // 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(context.Background(), string(sourceContent), workflowSpec, githubWorkflowsDir, opts.Verbose, opts.Force, tracker); err != nil { - return err - } - // Fetch files listed in the 'resources:' frontmatter field (additional workflow or - // action files that should be present alongside this workflow). - if err := fetchAndSaveRemoteResources(string(sourceContent), workflowSpec, githubWorkflowsDir, opts.Verbose, opts.Force, tracker); err != nil { + if err := fetchAllRemoteDependencies(context.Background(), string(sourceContent), workflowSpec, githubWorkflowsDir, opts.Verbose, opts.Force, tracker); err != nil { return err } } else if sourceInfo != nil && sourceInfo.IsLocal { diff --git a/pkg/cli/includes.go b/pkg/cli/includes.go index d01a0daa60..755b18c358 100644 --- a/pkg/cli/includes.go +++ b/pkg/cli/includes.go @@ -442,3 +442,34 @@ func fetchAndSaveRemoteIncludes(content string, spec *WorkflowSpec, targetDir st return nil } + +// fetchAllRemoteDependencies fetches all remote dependencies for a workflow: +// includes (@include directives), frontmatter imports, dispatch workflows, and resources. +// This is the single entry point shared by both the add and trial commands. +func fetchAllRemoteDependencies(ctx context.Context, content string, spec *WorkflowSpec, targetDir string, verbose bool, force bool, tracker *FileTracker) error { + // Fetch and save @include directive dependencies + if err := fetchAndSaveRemoteIncludes(content, spec, targetDir, verbose, force, tracker); err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch include dependencies: %v", err))) + } + } + // Fetch and save frontmatter 'imports:' dependencies so they are available + // locally during compilation. Keeping these as relative paths (not workflowspecs) + // ensures the compiler resolves them from disk rather than downloading from GitHub. + if err := fetchAndSaveRemoteFrontmatterImports(content, spec, targetDir, verbose, force, tracker); err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch frontmatter import dependencies: %v", err))) + } + } + // 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 { + return err + } + // Fetch files listed in the 'resources:' frontmatter field (additional workflow or + // action files that should be present alongside this workflow). + if err := fetchAndSaveRemoteResources(content, spec, targetDir, verbose, force, tracker); err != nil { + return err + } + return nil +} diff --git a/pkg/cli/trial_repository.go b/pkg/cli/trial_repository.go index daf018608b..ba35f9a6dd 100644 --- a/pkg/cli/trial_repository.go +++ b/pkg/cli/trial_repository.go @@ -277,12 +277,10 @@ func installWorkflowInTrialMode(ctx context.Context, tempDir string, parsedSpec } } - // Fetch and save include dependencies for remote workflows + // Fetch and save all remote dependencies (includes, imports, dispatch workflows, resources) if !fetched.IsLocal { - if err := fetchAndSaveRemoteIncludes(string(content), parsedSpec, result.WorkflowsDir, opts.Verbose, true, nil); err != nil { - if opts.Verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch include dependencies: %v", err))) - } + 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) } } From ef2dda40870116ed0bbf5e86bd0f7250b0317198 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Thu, 26 Mar 2026 20:24:28 +0000 Subject: [PATCH 5/7] Update pkg/cli/includes.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/cli/includes.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/cli/includes.go b/pkg/cli/includes.go index 755b18c358..2f846613c5 100644 --- a/pkg/cli/includes.go +++ b/pkg/cli/includes.go @@ -464,12 +464,18 @@ func fetchAllRemoteDependencies(ctx context.Context, content string, spec *Workf // 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 { - return err + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch dispatch workflow dependencies: %v", err))) + } + return fmt.Errorf("failed to fetch dispatch workflow dependencies: %w", err) } // Fetch files listed in the 'resources:' frontmatter field (additional workflow or // action files that should be present alongside this workflow). if err := fetchAndSaveRemoteResources(content, spec, targetDir, verbose, force, tracker); err != nil { - return err + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch resource dependencies: %v", err))) + } + return fmt.Errorf("failed to fetch resource dependencies: %w", err) } return nil } From c8d8c72bda938aed81e9fdd2e719c667481815d4 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Thu, 26 Mar 2026 20:25:00 +0000 Subject: [PATCH 6/7] fix: increase timeout for live GitHub API integration test The test was timing out at 10s default. Increase to 30s to handle slow API responses. --- actions/setup/js/frontmatter_hash_github_api.test.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/setup/js/frontmatter_hash_github_api.test.cjs b/actions/setup/js/frontmatter_hash_github_api.test.cjs index 4d4a5ecf9e..78a5f73bec 100644 --- a/actions/setup/js/frontmatter_hash_github_api.test.cjs +++ b/actions/setup/js/frontmatter_hash_github_api.test.cjs @@ -371,7 +371,7 @@ describe("frontmatter_hash with GitHub API", () => { }); describe("live GitHub API integration", () => { - it("should compute hash using real GitHub API (no mocks)", async () => { + it("should compute hash using real GitHub API (no mocks)", { timeout: 30000 }, async () => { // Skip this test if no GitHub token is available // Check multiple possible token environment variables const token = process.env.GITHUB_TOKEN || process.env.GH_TOKEN; From c877da62ad4c94196789e0bdbe0722a8e0a807a0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 28 Mar 2026 11:23:11 +0000 Subject: [PATCH 7/7] fix: clarify best-effort semantics in fetchAllRemoteDependencies docstring and add tests Agent-Logs-Url: https://github.com/github/gh-aw/sessions/78861777-a15b-4057-82d1-a28adf6deaba Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/includes.go | 8 +- pkg/cli/remote_workflow_test.go | 132 ++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 1 deletion(-) diff --git a/pkg/cli/includes.go b/pkg/cli/includes.go index 2f846613c5..e1767ad87c 100644 --- a/pkg/cli/includes.go +++ b/pkg/cli/includes.go @@ -446,8 +446,13 @@ func fetchAndSaveRemoteIncludes(content string, spec *WorkflowSpec, targetDir st // fetchAllRemoteDependencies fetches all remote dependencies for a workflow: // includes (@include directives), frontmatter imports, dispatch workflows, and resources. // This is the single entry point shared by both the add and trial commands. +// +// Error handling is intentionally asymmetric: +// - @include and frontmatter import errors are best-effort: failures emit a warning when +// verbose is true but do not stop the overall operation. +// - Dispatch-workflow and resource errors are fatal and are returned to the caller. func fetchAllRemoteDependencies(ctx context.Context, content string, spec *WorkflowSpec, targetDir string, verbose bool, force bool, tracker *FileTracker) error { - // Fetch and save @include directive dependencies + // Fetch and save @include directive dependencies (best-effort: errors are not fatal). if err := fetchAndSaveRemoteIncludes(content, spec, targetDir, verbose, force, tracker); err != nil { if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch include dependencies: %v", err))) @@ -456,6 +461,7 @@ func fetchAllRemoteDependencies(ctx context.Context, content string, spec *Workf // Fetch and save frontmatter 'imports:' dependencies so they are available // locally during compilation. Keeping these as relative paths (not workflowspecs) // ensures the compiler resolves them from disk rather than downloading from GitHub. + // Best-effort: errors are not fatal. if err := fetchAndSaveRemoteFrontmatterImports(content, spec, targetDir, verbose, force, tracker); err != nil { if verbose { fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch frontmatter import dependencies: %v", err))) diff --git a/pkg/cli/remote_workflow_test.go b/pkg/cli/remote_workflow_test.go index d9df711905..6d34b98549 100644 --- a/pkg/cli/remote_workflow_test.go +++ b/pkg/cli/remote_workflow_test.go @@ -1737,3 +1737,135 @@ source: otherorg/other-repo/.github/workflows/triage-issue.md@v1 require.NoError(t, readErr) assert.Equal(t, conflictContent, string(got), "conflict file must not be overwritten in post-write phase") } + +// --- fetchAllRemoteDependencies tests --- + +// TestFetchAllRemoteDependencies_NoDependencies verifies that a workflow with no +// includes, imports, dispatch workflows, or resources succeeds with no files created. +func TestFetchAllRemoteDependencies_NoDependencies(t *testing.T) { + content := `--- +engine: copilot +--- + +# Workflow with no remote dependencies +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "", Version: ""}, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAllRemoteDependencies(context.Background(), content, spec, tmpDir, false, false, nil) + require.NoError(t, err, "should not error when there are no remote dependencies") + + entries, readErr := os.ReadDir(tmpDir) + require.NoError(t, readErr) + assert.Empty(t, entries, "no files should be created when there are no remote dependencies") +} + +// TestFetchAllRemoteDependencies_LocalSpecNoOp verifies that when the spec has an empty +// RepoSlug (a local workflow), all fetch operations are skipped and the function succeeds. +func TestFetchAllRemoteDependencies_LocalSpecNoOp(t *testing.T) { + content := `--- +engine: copilot +safe-outputs: + dispatch-workflow: + - dependent-workflow +--- + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "", Version: ""}, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAllRemoteDependencies(context.Background(), content, spec, tmpDir, false, false, nil) + require.NoError(t, err, "should not error for a local (no RepoSlug) workflow") + + entries, readErr := os.ReadDir(tmpDir) + require.NoError(t, readErr) + assert.Empty(t, entries, "no files should be created for a local workflow") +} + +// TestFetchAllRemoteDependencies_IncludeErrorSwallowed verifies that include-fetch errors +// are treated as best-effort: a non-optional @include that cannot be resolved does not +// cause fetchAllRemoteDependencies to return an error. +func TestFetchAllRemoteDependencies_IncludeErrorSwallowed(t *testing.T) { + // A non-optional @include with a relative path and no RepoSlug will fail to resolve, + // but that error should be swallowed by fetchAllRemoteDependencies. + content := `--- +engine: copilot +--- + +@include shared/helpers.md + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "", Version: ""}, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAllRemoteDependencies(context.Background(), content, spec, tmpDir, false, false, nil) + require.NoError(t, err, "include errors should be swallowed (best-effort)") +} + +// TestFetchAllRemoteDependencies_DispatchConflictPropagated verifies that a conflict +// detected when fetching dispatch-workflow dependencies is returned to the caller. +// The conflict is detected before any network call, so no real download occurs. +func TestFetchAllRemoteDependencies_DispatchConflictPropagated(t *testing.T) { + tmpDir := t.TempDir() + + // Pre-existing dispatch workflow from a DIFFERENT source repo. + conflictPath := filepath.Join(tmpDir, "triage-issue.md") + conflictContent := `--- +source: otherorg/other-repo/.github/workflows/triage-issue.md@v1 +--- +# Triage from other repo +` + require.NoError(t, os.WriteFile(conflictPath, []byte(conflictContent), 0644)) + + content := `--- +engine: copilot +safe-outputs: + dispatch-workflow: + - triage-issue +--- + +# Main Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/main.md", + } + + err := fetchAllRemoteDependencies(context.Background(), content, spec, tmpDir, false, false, nil) + require.Error(t, err, "dispatch workflow conflict should be propagated") + assert.Contains(t, err.Error(), "dispatch workflow", "error should mention 'dispatch workflow'") +} + +// TestFetchAllRemoteDependencies_ResourceMacroErrorPropagated verifies that a resource +// path containing GitHub Actions expression syntax causes an error that is propagated. +// This check occurs before any network call, so no download is attempted. +func TestFetchAllRemoteDependencies_ResourceMacroErrorPropagated(t *testing.T) { + content := `--- +engine: copilot +resources: + - "${{ env.DYNAMIC_FILE }}" +--- + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAllRemoteDependencies(context.Background(), content, spec, tmpDir, false, false, nil) + require.Error(t, err, "resource macro error should be propagated") + assert.Contains(t, err.Error(), "failed to fetch resource dependencies", "error should be wrapped with dependency context") +}