diff --git a/actions/setup/js/frontmatter_hash_github_api.test.cjs b/actions/setup/js/frontmatter_hash_github_api.test.cjs index 4d4a5ecf9ec..78a5f73becd 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; diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index 2d721b34ac8..b27193c037e 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 d01a0daa609..e1767ad87cb 100644 --- a/pkg/cli/includes.go +++ b/pkg/cli/includes.go @@ -442,3 +442,46 @@ 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. +// +// 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 (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))) + } + } + // 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))) + } + } + // 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 { + 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 { + 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 +} diff --git a/pkg/cli/remote_workflow_test.go b/pkg/cli/remote_workflow_test.go index d9df711905f..6d34b98549e 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") +} diff --git a/pkg/cli/trial_repository.go b/pkg/cli/trial_repository.go index daf018608b0..ba35f9a6dde 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) } }