diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 40bc43c1387..353440bd31e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2362,3 +2362,57 @@ jobs: name: test-result-integration-unauthenticated path: test-result-integration-unauthenticated.json retention-days: 14 + + integration-add-dispatch-workflow: + name: Integration Add with dispatch-workflow Dependencies + runs-on: ubuntu-latest + permissions: + contents: read + concurrency: + group: ci-${{ github.ref }}-integration-add-dispatch-workflow + cancel-in-progress: true + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Set up Go + id: setup-go + uses: actions/setup-go@4dc6199c7b1a012772edbd06daecab0f50c9053c # v6 + with: + go-version-file: go.mod + cache: true + + - name: Report Go cache status + run: | + if [ "${{ steps.setup-go.outputs.cache-hit }}" == "true" ]; then + echo "✅ Go cache hit" >> $GITHUB_STEP_SUMMARY + else + echo "⚠️ Go cache miss" >> $GITHUB_STEP_SUMMARY + fi + + - name: Download dependencies + run: go mod download + + - name: Verify dependencies + run: go mod verify + + - name: Build gh-aw binary + run: make build + + - name: Run dispatch-workflow add integration tests + env: + GH_TOKEN: ${{ github.token }} + run: | + set -o pipefail + go test -v -parallel=4 -timeout=10m -tags 'integration' -json \ + -run 'TestAddWorkflowWithDispatchWorkflow' \ + ./pkg/cli/ \ + | tee test-result-integration-add-dispatch-workflow.json + + - name: Upload test results + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: test-result-integration-add-dispatch-workflow + path: test-result-integration-add-dispatch-workflow.json + retention-days: 14 diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index 179d5f92752..2c2a6352278 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -368,6 +368,16 @@ func addWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTracker, o 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(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 { + return err + } } else if sourceInfo != nil && sourceInfo.IsLocal { // For local workflows, collect and copy include dependencies from local paths // The source directory is derived from the workflow's path @@ -507,6 +517,18 @@ func addWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTracker, o } } + // For remote workflows: now that the main workflow and all its imports are on disk, + // parse the fully merged safe-outputs configuration to discover any dispatch workflows + // that originate from imported shared workflows (not visible in the raw frontmatter). + if !isLocalWorkflowPath(workflowSpec.WorkflowPath) { + fetchAndSaveDispatchWorkflowsFromParsedFile(destFile, workflowSpec, githubWorkflowsDir, opts.Verbose, opts.Force, tracker) + } + + // Compile any dispatch-workflow .md dependencies that were just fetched and lack a + // .lock.yml. The dispatch-workflow validator requires every .md dispatch target to be + // compiled before the main workflow can be validated. + compileDispatchWorkflowDependencies(destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, tracker) + // Compile the workflow if tracker != nil { if err := compileWorkflowWithTracking(destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, tracker); err != nil { diff --git a/pkg/cli/add_integration_test.go b/pkg/cli/add_integration_test.go index f4021ee43df..53b28a7e290 100644 --- a/pkg/cli/add_integration_test.go +++ b/pkg/cli/add_integration_test.go @@ -886,3 +886,129 @@ func TestAddPublicWorkflowUnauthenticated(t *testing.T) { _, err = os.Stat(workflowFile) require.NoError(t, err, "downloaded workflow file should exist at %s", workflowFile) } + +// TestAddWorkflowWithDispatchWorkflowDependency tests that when a remote workflow is added +// that references dispatch-workflow dependencies, those dependency workflows are automatically +// fetched alongside the main workflow. +// +// The test installs test-dispatcher.md from the main branch of github/gh-aw. That workflow +// has: +// +// safe-outputs: +// dispatch-workflow: +// workflows: +// - test-workflow +// +// After `gh aw add`, both test-dispatcher.md AND test-workflow.md should be present locally. +// This test requires GitHub authentication. +func TestAddWorkflowWithDispatchWorkflowDependency(t *testing.T) { + // Skip if GitHub authentication is not available + authCmd := exec.Command("gh", "auth", "status") + if err := authCmd.Run(); err != nil { + t.Skip("Skipping test: GitHub authentication not available (gh auth status failed)") + } + + setup := setupAddIntegrationTest(t) + defer setup.cleanup() + + // Add test-dispatcher.md which has a dispatch-workflow dependency on test-workflow. + // Use an explicit path spec so the file resolves unambiguously from the main branch. + workflowSpec := "github/gh-aw/.github/workflows/test-dispatcher.md@main" + + cmd := exec.Command(setup.binaryPath, "add", workflowSpec, "--verbose") + cmd.Dir = setup.tempDir + output, err := cmd.CombinedOutput() + outputStr := string(output) + + t.Logf("Command output:\n%s", outputStr) + + require.NoError(t, err, "add command should succeed: %s", outputStr) + + workflowsDir := filepath.Join(setup.tempDir, ".github", "workflows") + + // 1. The main workflow must be present. + mainFile := filepath.Join(workflowsDir, "test-dispatcher.md") + _, err = os.Stat(mainFile) + require.NoError(t, err, "main workflow test-dispatcher.md should exist at %s", mainFile) + + // 2. The dispatch-workflow dependency (test-workflow.md) must be fetched automatically. + depFile := filepath.Join(workflowsDir, "test-workflow.md") + _, err = os.Stat(depFile) + require.NoError(t, err, + "dispatch-workflow dependency test-workflow.md should be auto-fetched alongside the main workflow") + + // 3. Both .lock.yml files should be present (compilation must succeed). + mainLock := filepath.Join(workflowsDir, "test-dispatcher.lock.yml") + _, err = os.Stat(mainLock) + require.NoError(t, err, "compiled lock file test-dispatcher.lock.yml should exist") + + depLock := filepath.Join(workflowsDir, "test-workflow.lock.yml") + _, err = os.Stat(depLock) + require.NoError(t, err, "compiled lock file test-workflow.lock.yml should exist") + + // 4. Verify the dependency file has valid frontmatter. + depContent, err := os.ReadFile(depFile) + require.NoError(t, err, "should be able to read test-workflow.md") + assert.Contains(t, string(depContent), "workflow_dispatch", + "test-workflow.md should have workflow_dispatch trigger") +} + +// TestAddWorkflowWithDispatchWorkflowFromSharedImport tests that dispatch-workflow +// configuration is fetched and preserved correctly when `gh aw add` is used. +// This exercises the post-write parse path (fetchAndSaveDispatchWorkflowsFromParsedFile) +// that re-parses the written workflow file to discover any remaining dependencies. +// +// smoke-copilot.md has `safe-outputs.dispatch-workflow: [haiku-printer]` in its own +// frontmatter. haiku-printer exists as a plain GitHub Actions workflow (.yml), not an +// agentic workflow (.md). The dispatch-workflow fetcher first tries haiku-printer.md +// (404), then falls back to haiku-printer.yml which succeeds. The test verifies that +// the overall add command succeeds and the compiled lock file references haiku-printer. +// This test requires GitHub authentication. +func TestAddWorkflowWithDispatchWorkflowFromSharedImport(t *testing.T) { + // Skip if GitHub authentication is not available + authCmd := exec.Command("gh", "auth", "status") + if err := authCmd.Run(); err != nil { + t.Skip("Skipping test: GitHub authentication not available (gh auth status failed)") + } + + setup := setupAddIntegrationTest(t) + defer setup.cleanup() + + // smoke-copilot.md has `safe-outputs.dispatch-workflow: [haiku-printer]` in its own + // frontmatter. haiku-printer lives as haiku-printer.yml (a plain GitHub Actions + // workflow). The fetcher falls back to .yml when .md is 404, so both the main + // workflow and the dispatch-workflow dependency are written to disk. + workflowSpec := "github/gh-aw/.github/workflows/smoke-copilot.md@main" + + cmd := exec.Command(setup.binaryPath, "add", workflowSpec, "--verbose") + cmd.Dir = setup.tempDir + output, err := cmd.CombinedOutput() + outputStr := string(output) + + t.Logf("Command output:\n%s", outputStr) + + require.NoError(t, err, "add command should succeed: %s", outputStr) + + workflowsDir := filepath.Join(setup.tempDir, ".github", "workflows") + + // 1. The main workflow must be present. + mainFile := filepath.Join(workflowsDir, "smoke-copilot.md") + _, err = os.Stat(mainFile) + require.NoError(t, err, "main workflow smoke-copilot.md should exist at %s", mainFile) + + // 2. haiku-printer.yml should have been fetched via the .yml fallback path. + haikuFile := filepath.Join(workflowsDir, "haiku-printer.yml") + _, err = os.Stat(haikuFile) + require.NoError(t, err, "dispatch-workflow dependency haiku-printer.yml should be fetched") + + // 3. Verify compilation succeeded (the lock file was created). + mainLock := filepath.Join(workflowsDir, "smoke-copilot.lock.yml") + _, err = os.Stat(mainLock) + require.NoError(t, err, "compiled lock file smoke-copilot.lock.yml should exist") + + // Verify the lock file references the dispatch-workflow configuration + lockContent, err := os.ReadFile(mainLock) + require.NoError(t, err, "should be able to read lock file") + assert.Contains(t, string(lockContent), "haiku-printer", + "lock file should reference the haiku-printer dispatch-workflow target") +} diff --git a/pkg/cli/add_workflow_compilation.go b/pkg/cli/add_workflow_compilation.go index 7d39ece0bee..21c2ad467a1 100644 --- a/pkg/cli/add_workflow_compilation.go +++ b/pkg/cli/add_workflow_compilation.go @@ -121,7 +121,52 @@ func compileWorkflowWithTrackingAndRefresh(filePath string, verbose bool, quiet return nil } -// addSourceToWorkflow adds the source field to the workflow's frontmatter. +// compileDispatchWorkflowDependencies compiles any dispatch-workflow .md dependencies of +// workflowFile that are present locally but lack a corresponding .lock.yml. This must be +// called before compiling the main workflow, because the dispatch-workflow validator +// requires every referenced .md workflow to have an up-to-date .lock.yml. +func compileDispatchWorkflowDependencies(workflowFile string, verbose, quiet bool, engineOverride string, tracker *FileTracker) { + // Parse the merged safe-outputs to get the canonical list of dispatch-workflow names. + compiler := workflow.NewCompiler() + data, err := compiler.ParseWorkflowFile(workflowFile) + if err != nil || data == nil || data.SafeOutputs == nil || data.SafeOutputs.DispatchWorkflow == nil { + return + } + + workflowsDir := filepath.Dir(workflowFile) + + for _, name := range data.SafeOutputs.DispatchWorkflow.Workflows { + mdPath := filepath.Join(workflowsDir, name+".md") + lockPath := stringutil.MarkdownToLockFile(mdPath) + + // Only compile if the .md is present but the .lock.yml is absent. + if _, mdErr := os.Stat(mdPath); mdErr != nil { + continue // .md doesn't exist locally + } + if _, lockErr := os.Stat(lockPath); lockErr == nil { + continue // .lock.yml already exists, nothing to do + } + + addWorkflowCompilationLog.Printf("Compiling dispatch-workflow dependency: %s", mdPath) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Compiling dispatch-workflow dependency: "+mdPath)) + } + + var compileErr error + if tracker != nil { + compileErr = compileWorkflowWithTracking(mdPath, verbose, quiet, engineOverride, tracker) + } else { + compileErr = compileWorkflow(mdPath, verbose, quiet, engineOverride) + } + if compileErr != nil { + // Best-effort: log and continue so the main workflow can still give a clear error. + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to compile dispatch-workflow dependency %s: %v", mdPath, compileErr))) + } + } + } +} + // This function preserves the existing frontmatter formatting while adding the source field. func addSourceToWorkflow(content, source string) (string, error) { // Use shared frontmatter logic that preserves formatting diff --git a/pkg/cli/remote_workflow.go b/pkg/cli/remote_workflow.go index 1ac587a881e..13e9b1a5f37 100644 --- a/pkg/cli/remote_workflow.go +++ b/pkg/cli/remote_workflow.go @@ -13,6 +13,7 @@ import ( "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/parser" + "github.com/github/gh-aw/pkg/workflow" ) var remoteWorkflowLog = logger.New("cli:remote_workflow") @@ -582,3 +583,688 @@ func getParentDir(path string) string { } return path[:idx] } + +// readSourceRepoFromFile reads the 'source' frontmatter field from a local workflow file +// and returns the "owner/repo" portion (e.g. "github/gh-aw"). Returns "" if the file +// cannot be read, has no source field, or the field is not in the expected format. +func readSourceRepoFromFile(path string) string { + content, err := os.ReadFile(path) + if err != nil { + return "" + } + result, err := parser.ExtractFrontmatterFromContent(string(content)) + if err != nil || result.Frontmatter == nil { + return "" + } + sourceRaw, ok := result.Frontmatter["source"] + if !ok { + return "" + } + source, ok := sourceRaw.(string) + if !ok || source == "" { + return "" + } + // source format: "owner/repo/path/to/file.md@ref" — extract just "owner/repo" + slashParts := strings.SplitN(source, "/", 3) + if len(slashParts) < 2 { + return "" + } + return slashParts[0] + "/" + slashParts[1] +} + +// sourceRepoLabel returns the source repo string for display in error messages. +// When the repo string is empty (file has no source field or is not a markdown file), +// a human-readable placeholder is returned so the error message is not confusing. +func sourceRepoLabel(repo string) string { + if repo == "" { + return "(no source field)" + } + return repo +} + +// extractDispatchWorkflowNames extracts workflow names from the safe-outputs.dispatch-workflow +// frontmatter field. It handles both array and map forms of the configuration. +// Workflow names that contain GitHub Actions expression syntax (e.g. "${{") are skipped. +func extractDispatchWorkflowNames(content string) []string { + result, err := parser.ExtractFrontmatterFromContent(content) + if err != nil || result.Frontmatter == nil { + return nil + } + + safeOutputs, exists := result.Frontmatter["safe-outputs"] + if !exists { + return nil + } + + safeOutputsMap, ok := safeOutputs.(map[string]any) + if !ok { + return nil + } + + dispatchWorkflow, exists := safeOutputsMap["dispatch-workflow"] + if !exists { + return nil + } + + var workflowNames []string + + switch v := dispatchWorkflow.(type) { + case []any: + // Array format: dispatch-workflow: [name1, name2] + for _, item := range v { + if name, ok := item.(string); ok { + workflowNames = append(workflowNames, name) + } + } + case map[string]any: + // Map format: dispatch-workflow: {workflows: [name1, name2]} + if workflows, exists := v["workflows"]; exists { + if workflowsArray, ok := workflows.([]any); ok { + for _, item := range workflowsArray { + if name, ok := item.(string); ok { + workflowNames = append(workflowNames, name) + } + } + } + } + } + + // Filter out GitHub Actions expression syntax (e.g. "${{ vars.WORKFLOW }}") + filtered := make([]string, 0, len(workflowNames)) + for _, name := range workflowNames { + if !strings.Contains(name, "${{") { + filtered = append(filtered, name) + } + } + + return filtered +} + +// fetchAndSaveRemoteDispatchWorkflows fetches and saves the workflow files referenced in the +// safe-outputs.dispatch-workflow configuration of a remote workflow. Each listed workflow name +// (without extension) is resolved as a sibling file (".md") in the same directory as +// the source workflow and downloaded from the same remote repository. +// +// Workflow names that use GitHub Actions expression syntax (e.g. "${{") are silently skipped +// because they are dynamic values that cannot be resolved at add-time. +// +// If a target file already exists from a different source (different owner/repo in its +// 'source:' frontmatter field, or no source field at all), an error is returned. +// Files from the same source are silently skipped. Download failures are non-fatal. +func fetchAndSaveRemoteDispatchWorkflows(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 + } + owner, repo := parts[0], parts[1] + ref := spec.Version + if ref == "" { + defaultBranch, err := getRepoDefaultBranch(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 + } + + workflowNames := extractDispatchWorkflowNames(content) + if len(workflowNames) == 0 { + return nil + } + + // workflowBaseDir is the directory of the source workflow in the remote repo + // (e.g. ".github/workflows"). Dispatch-workflow names are resolved relative to it. + workflowBaseDir := getParentDir(spec.WorkflowPath) + + // Pre-compute the absolute target directory for path-traversal boundary checks. + absTargetDir, err := filepath.Abs(targetDir) + if err != nil { + remoteWorkflowLog.Printf("Failed to resolve absolute path for target directory %s: %v", targetDir, err) + return nil + } + + for _, workflowName := range workflowNames { + // Build the remote file path for this dispatch workflow + var remoteFilePath string + if workflowBaseDir != "" { + remoteFilePath = path.Join(workflowBaseDir, workflowName+".md") + } else { + remoteFilePath = workflowName + ".md" + } + remoteFilePath = path.Clean(remoteFilePath) + + // The local path is just the workflow filename in targetDir + localRelPath := filepath.Clean(workflowName + ".md") + targetPath := filepath.Join(targetDir, localRelPath) + + // Belt-and-suspenders: verify the resolved path stays inside targetDir + absTargetPath, absErr := filepath.Abs(targetPath) + if absErr != nil { + remoteWorkflowLog.Printf("Failed to resolve absolute path for dispatch workflow %s: %v", workflowName, absErr) + continue + } + if rel, relErr := filepath.Rel(absTargetDir, absTargetPath); relErr != nil || strings.HasPrefix(rel, "..") { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Refusing to write dispatch workflow outside target directory: %q", workflowName))) + } + continue + } + + // Check whether the target file already exists. + fileExists := false + if _, statErr := os.Stat(targetPath); statErr == nil { + fileExists = true + if !force { + // Allow if the existing file comes from the same source repository. + existingSourceRepo := readSourceRepoFromFile(targetPath) + if existingSourceRepo == spec.RepoSlug { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Dispatch workflow from same source already exists, skipping: "+targetPath)) + } + continue + } + // Different or missing source — this is a conflict. + return fmt.Errorf( + "dispatch workflow %q already exists at %s (existing source: %q, installing from: %q); remove the file or use --force to overwrite", + workflowName, targetPath, sourceRepoLabel(existingSourceRepo), spec.RepoSlug, + ) + } + } + + // Download from the source repository — try .md first, then .yml as fallback + // (the dispatch-workflow validator accepts either .md or .yml files locally). + workflowContent, err := parser.DownloadFileFromGitHub(owner, repo, remoteFilePath, ref) + if err != nil { + // .md not found — try .yml fallback (e.g. plain GitHub Actions workflow) + ymlRemotePath := path.Clean(strings.TrimSuffix(remoteFilePath, ".md") + ".yml") + ymlLocalPath := filepath.Join(targetDir, filepath.Clean(workflowName+".yml")) + + ymlContent, ymlErr := parser.DownloadFileFromGitHub(owner, repo, ymlRemotePath, ref) + if ymlErr != nil { + // Neither .md nor .yml found — best-effort, continue + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch dispatch workflow %s: %v", remoteFilePath, err))) + } + continue + } + // .yml fallback succeeded — write it (no source field for yml) + if mkErr := os.MkdirAll(filepath.Dir(ymlLocalPath), 0755); mkErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to create directory for dispatch workflow %s: %v", ymlRemotePath, mkErr))) + } + continue + } + // Capture whether file exists before writing (for correct tracker classification). + _, ymlFileExistsErr := os.Stat(ymlLocalPath) + ymlFileExists := ymlFileExistsErr == nil + if writeErr := os.WriteFile(ymlLocalPath, ymlContent, 0600); writeErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to write dispatch workflow %s: %v", ymlRemotePath, writeErr))) + } + continue + } + if verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Fetched dispatch workflow (.yml): "+ymlLocalPath)) + } + if tracker != nil { + if ymlFileExists { + tracker.TrackModified(ymlLocalPath) + } else { + tracker.TrackCreated(ymlLocalPath) + } + } + continue + } + + // Embed the source field so future adds can detect same-source conflicts. + depSourceString := spec.RepoSlug + "/" + remoteFilePath + "@" + ref + if updated, srcErr := addSourceToWorkflow(string(workflowContent), depSourceString); srcErr == nil { + workflowContent = []byte(updated) + } + + // Create parent directory if needed + if err := os.MkdirAll(filepath.Dir(targetPath), 0755); err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to create directory for dispatch workflow %s: %v", remoteFilePath, err))) + } + continue + } + + // Write the file + if err := os.WriteFile(targetPath, workflowContent, 0600); err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to write dispatch workflow %s: %v", remoteFilePath, err))) + } + continue + } + + if verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Fetched dispatch workflow: "+targetPath)) + } + + // Track the file + if tracker != nil { + if fileExists { + tracker.TrackModified(targetPath) + } else { + tracker.TrackCreated(targetPath) + } + } + } + + return nil +} + +// extractResources extracts file paths from the top-level "resources" frontmatter field. +// Returns an error if any entry contains GitHub Actions expression syntax (e.g. "${{"), +// since macros are not permitted in resource paths. +func extractResources(content string) ([]string, error) { + result, err := parser.ExtractFrontmatterFromContent(content) + if err != nil { + remoteWorkflowLog.Printf("Failed to extract frontmatter for resources: %v", err) + return nil, nil + } + if result.Frontmatter == nil { + return nil, nil + } + + resourcesField, exists := result.Frontmatter["resources"] + if !exists { + return nil, nil + } + + var paths []string + switch v := resourcesField.(type) { + case []any: + for _, item := range v { + if s, ok := item.(string); ok { + paths = append(paths, s) + } + } + case []string: + paths = v + } + + // Reject entries that contain GitHub Actions expression syntax — macros are not allowed. + for _, p := range paths { + if strings.Contains(p, "${{") { + return nil, fmt.Errorf("resources entry %q contains GitHub Actions expression syntax (${{) which is not allowed; use static paths only", p) + } + } + + return paths, nil +} + +// fetchAndSaveRemoteResources fetches files listed in the top-level "resources" frontmatter +// field from the same remote repository and saves them locally. Resources are resolved as +// relative paths from the same directory as the source workflow in the remote repo. +// +// GitHub Actions expression syntax (e.g. "${{") is not allowed in resource paths and will +// cause an error. Download failures for individual files are non-fatal (best-effort). +// +// For Markdown resource files: if the target already exists from a different source repository +// (different 'source:' frontmatter field, or no source field), an error is returned. Files +// from the same source are silently skipped. +// For non-Markdown resource files: if the target already exists and force is false, an error +// is returned regardless of origin (non-markdown files have no source tracking). +func fetchAndSaveRemoteResources(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 + } + owner, repo := parts[0], parts[1] + ref := spec.Version + if ref == "" { + defaultBranch, err := getRepoDefaultBranch(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 + } + + resourcePaths, err := extractResources(content) + if err != nil { + return err + } + if len(resourcePaths) == 0 { + return nil + } + + // Resources are resolved relative to the source workflow's directory in the remote repo. + workflowBaseDir := getParentDir(spec.WorkflowPath) + + // Pre-compute the absolute target directory for path-traversal boundary checks. + absTargetDir, err := filepath.Abs(targetDir) + if err != nil { + remoteWorkflowLog.Printf("Failed to resolve absolute path for target directory %s: %v", targetDir, err) + return nil + } + + for _, resourcePath := range resourcePaths { + // Early rejection of path traversal patterns. This is a fast first-pass check; + // the filepath.Rel boundary check below is the authoritative security control. + if strings.Contains(resourcePath, "..") { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Skipping resource with unsafe path: %q", resourcePath))) + } + continue + } + + // Resolve the remote file path + var remoteFilePath string + if rest, ok := strings.CutPrefix(resourcePath, "/"); ok { + remoteFilePath = rest + } else if workflowBaseDir != "" { + remoteFilePath = path.Join(workflowBaseDir, resourcePath) + } else { + remoteFilePath = resourcePath + } + remoteFilePath = path.Clean(remoteFilePath) + + // Derive the local relative path by stripping the workflow base dir prefix + localRelPath := remoteFilePath + if workflowBaseDir != "" && strings.HasPrefix(remoteFilePath, workflowBaseDir+"/") { + localRelPath = remoteFilePath[len(workflowBaseDir)+1:] + } + localRelPath = filepath.Clean(filepath.FromSlash(localRelPath)) + localRelPath = strings.TrimLeft(localRelPath, string(filepath.Separator)) + if localRelPath == "" || localRelPath == "." { + continue + } + targetPath := filepath.Join(targetDir, localRelPath) + + // Belt-and-suspenders: verify the resolved path stays inside targetDir + absTargetPath, absErr := filepath.Abs(targetPath) + if absErr != nil { + remoteWorkflowLog.Printf("Failed to resolve absolute path for resource %s: %v", resourcePath, absErr) + continue + } + if rel, relErr := filepath.Rel(absTargetDir, absTargetPath); relErr != nil || strings.HasPrefix(rel, "..") { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Refusing to write resource outside target directory: %q", resourcePath))) + } + continue + } + + // Check whether the target file already exists. + fileExists := false + if _, statErr := os.Stat(targetPath); statErr == nil { + fileExists = true + if !force { + isMarkdown := strings.HasSuffix(strings.ToLower(targetPath), ".md") + if isMarkdown { + // For markdown files, allow same-source overwrites. + existingSourceRepo := readSourceRepoFromFile(targetPath) + if existingSourceRepo == spec.RepoSlug { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Resource file from same source already exists, skipping: "+targetPath)) + } + continue + } + return fmt.Errorf( + "resource %q already exists at %s (existing source: %q, installing from: %q); remove the file or use --force to overwrite", + resourcePath, targetPath, sourceRepoLabel(existingSourceRepo), spec.RepoSlug, + ) + } + // Non-markdown files have no source tracking — always conflict. + return fmt.Errorf( + "resource %q already exists at %s; remove the file or use --force to overwrite", + resourcePath, targetPath, + ) + } + } + + // Download from source repository + fileContent, err := parser.DownloadFileFromGitHub(owner, repo, remoteFilePath, ref) + if err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch resource %s: %v", remoteFilePath, err))) + } + continue + } + + // For markdown resources, embed the source field for future conflict detection. + if strings.HasSuffix(strings.ToLower(remoteFilePath), ".md") { + depSourceString := spec.RepoSlug + "/" + remoteFilePath + "@" + ref + if updated, srcErr := addSourceToWorkflow(string(fileContent), depSourceString); srcErr == nil { + fileContent = []byte(updated) + } + } + + // Create parent directory if needed + if err := os.MkdirAll(filepath.Dir(targetPath), 0755); err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to create directory for resource %s: %v", remoteFilePath, err))) + } + continue + } + + // Write the file + if err := os.WriteFile(targetPath, fileContent, 0600); err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to write resource %s: %v", remoteFilePath, err))) + } + continue + } + + if verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Fetched resource: "+targetPath)) + } + + // Track the file + if tracker != nil { + if fileExists { + tracker.TrackModified(targetPath) + } else { + tracker.TrackCreated(targetPath) + } + } + } + + return nil +} + +// fetchAndSaveDispatchWorkflowsFromParsedFile parses a locally-saved workflow file to obtain +// the fully merged safe-outputs configuration (including dispatch workflows that originate +// from imported shared workflows), then fetches any referenced dispatch workflow files that +// don't already exist locally. +// +// This is needed because import-derived dispatch workflows cannot be discovered by static +// frontmatter inspection alone — they only become visible after the compiler processes all +// imports and merges the safe-outputs configuration. +// +// All early returns (empty RepoSlug, invalid slug, parse failure, no dispatch workflows) are +// intentional no-ops: this function is best-effort and must never block the add workflow flow. +// Parse failures are logged at debug level so they can be investigated when needed. +// Source conflicts are reported as warnings (not errors) because the main file is already written. +func fetchAndSaveDispatchWorkflowsFromParsedFile(destFile string, spec *WorkflowSpec, targetDir string, verbose bool, force bool, tracker *FileTracker) { + if spec.RepoSlug == "" { + return + } + + parts := strings.SplitN(spec.RepoSlug, "/", 2) + if len(parts) != 2 { + return + } + owner, repo := parts[0], parts[1] + ref := spec.Version + if ref == "" { + ref = "main" + } + + // Parse the locally-saved workflow to get the full merged safe-outputs config. + compiler := workflow.NewCompiler() + data, err := compiler.ParseWorkflowFile(destFile) + if err != nil { + remoteWorkflowLog.Printf("Failed to parse workflow file %s for import-derived dispatch workflows: %v", destFile, err) + return + } + if data == nil || data.SafeOutputs == nil || data.SafeOutputs.DispatchWorkflow == nil { + return + } + + workflowNames := data.SafeOutputs.DispatchWorkflow.Workflows + if len(workflowNames) == 0 { + return + } + + // Filter out GitHub Actions expression syntax + filtered := make([]string, 0, len(workflowNames)) + for _, name := range workflowNames { + if !strings.Contains(name, "${{") { + filtered = append(filtered, name) + } + } + if len(filtered) == 0 { + return + } + + workflowBaseDir := getParentDir(spec.WorkflowPath) + + absTargetDir, absErr := filepath.Abs(targetDir) + if absErr != nil { + remoteWorkflowLog.Printf("Failed to resolve absolute path for target directory %s: %v", targetDir, absErr) + return + } + + for _, workflowName := range filtered { + // Early rejection of path traversal patterns (authoritative check is filepath.Rel below). + if strings.Contains(workflowName, "..") { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Skipping dispatch workflow with unsafe name: %q", workflowName))) + } + continue + } + + var remoteFilePath string + if workflowBaseDir != "" { + remoteFilePath = path.Join(workflowBaseDir, workflowName+".md") + } else { + remoteFilePath = workflowName + ".md" + } + remoteFilePath = path.Clean(remoteFilePath) + + localRelPath := filepath.Clean(workflowName + ".md") + targetPath := filepath.Join(targetDir, localRelPath) + + absTargetPath, absErr2 := filepath.Abs(targetPath) + if absErr2 != nil { + remoteWorkflowLog.Printf("Failed to resolve absolute path for dispatch workflow %s: %v", workflowName, absErr2) + continue + } + if rel, relErr := filepath.Rel(absTargetDir, absTargetPath); relErr != nil || strings.HasPrefix(rel, "..") { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Refusing to write dispatch workflow outside target directory: %q", workflowName))) + } + continue + } + + // Check whether the target file already exists. + fileExists := false + if _, statErr := os.Stat(targetPath); statErr == nil { + fileExists = true + if !force { + existingSourceRepo := readSourceRepoFromFile(targetPath) + if existingSourceRepo == spec.RepoSlug { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Dispatch workflow (from import) from same source already exists, skipping: "+targetPath)) + } + continue + } + // Different or missing source — warn and skip (post-write best-effort). + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf( + "Dispatch workflow %q already exists at %s from a different source (existing: %q, needed: %q); use --force to overwrite", + workflowName, targetPath, sourceRepoLabel(existingSourceRepo), spec.RepoSlug, + ))) + continue + } + } + + // Download from source repository — try .md first, then .yml as fallback + workflowContent, err := parser.DownloadFileFromGitHub(owner, repo, remoteFilePath, ref) + if err != nil { + // .md not found — try .yml fallback + ymlRemotePath := path.Clean(strings.TrimSuffix(remoteFilePath, ".md") + ".yml") + ymlLocalPath := filepath.Join(targetDir, filepath.Clean(workflowName+".yml")) + + ymlContent, ymlErr := parser.DownloadFileFromGitHub(owner, repo, ymlRemotePath, ref) + if ymlErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch dispatch workflow %s: %v", remoteFilePath, err))) + } + continue + } + if mkErr := os.MkdirAll(filepath.Dir(ymlLocalPath), 0755); mkErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to create directory for dispatch workflow %s: %v", ymlRemotePath, mkErr))) + } + continue + } + // Capture whether file exists before writing (for correct tracker classification). + _, ymlFileExistsErr := os.Stat(ymlLocalPath) + ymlFileExists := ymlFileExistsErr == nil + if writeErr := os.WriteFile(ymlLocalPath, ymlContent, 0600); writeErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to write dispatch workflow %s: %v", ymlRemotePath, writeErr))) + } + continue + } + if verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Fetched dispatch workflow (.yml, from import): "+ymlLocalPath)) + } + if tracker != nil { + if ymlFileExists { + tracker.TrackModified(ymlLocalPath) + } else { + tracker.TrackCreated(ymlLocalPath) + } + } + continue + } + + // Embed the source field for future conflict detection. + depSourceString := spec.RepoSlug + "/" + remoteFilePath + "@" + ref + if updated, srcErr := addSourceToWorkflow(string(workflowContent), depSourceString); srcErr == nil { + workflowContent = []byte(updated) + } + + if err := os.MkdirAll(filepath.Dir(targetPath), 0755); err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to create directory for dispatch workflow %s: %v", remoteFilePath, err))) + } + continue + } + + if err := os.WriteFile(targetPath, workflowContent, 0600); err != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to write dispatch workflow %s: %v", remoteFilePath, err))) + } + continue + } + + if verbose { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Fetched dispatch workflow (from import): "+targetPath)) + } + + if tracker != nil { + if fileExists { + tracker.TrackModified(targetPath) + } else { + tracker.TrackCreated(targetPath) + } + } + } +} diff --git a/pkg/cli/remote_workflow_test.go b/pkg/cli/remote_workflow_test.go index f03dd36efbd..5179da576f1 100644 --- a/pkg/cli/remote_workflow_test.go +++ b/pkg/cli/remote_workflow_test.go @@ -555,3 +555,1181 @@ imports: require.NoError(t, readErr) assert.Empty(t, entries, "no files should be created for an invalid RepoSlug") } + +// --- extractDispatchWorkflowNames tests --- + +// TestExtractDispatchWorkflowNames_ArrayFormat verifies that workflow names are extracted +// from the dispatch-workflow array (shorthand) format. +func TestExtractDispatchWorkflowNames_ArrayFormat(t *testing.T) { + content := `--- +engine: copilot +safe-outputs: + dispatch-workflow: + - workflow-a + - workflow-b +--- + +# Workflow +` + names := extractDispatchWorkflowNames(content) + assert.Equal(t, []string{"workflow-a", "workflow-b"}, names, "should extract workflow names from array format") +} + +// TestExtractDispatchWorkflowNames_MapFormat verifies that workflow names are extracted +// from the dispatch-workflow map format (with explicit workflows key). +func TestExtractDispatchWorkflowNames_MapFormat(t *testing.T) { + content := `--- +engine: copilot +safe-outputs: + dispatch-workflow: + workflows: + - workflow-x + - workflow-y +--- + +# Workflow +` + names := extractDispatchWorkflowNames(content) + assert.Equal(t, []string{"workflow-x", "workflow-y"}, names, "should extract workflow names from map format") +} + +// TestExtractDispatchWorkflowNames_SkipMacros verifies that workflow names containing +// GitHub Actions expression syntax are filtered out. +func TestExtractDispatchWorkflowNames_SkipMacros(t *testing.T) { + content := `--- +engine: copilot +safe-outputs: + dispatch-workflow: + - plain-workflow + - ${{ vars.WORKFLOW_NAME }} + - ${{ needs.step.outputs.workflow }} + - another-plain-workflow +--- + +# Workflow +` + names := extractDispatchWorkflowNames(content) + assert.Equal(t, []string{"plain-workflow", "another-plain-workflow"}, names, "should skip workflow names with GitHub Actions macro syntax") +} + +// TestExtractDispatchWorkflowNames_NoSafeOutputs verifies that an empty slice is returned +// when there is no safe-outputs section. +func TestExtractDispatchWorkflowNames_NoSafeOutputs(t *testing.T) { + content := `--- +engine: copilot +--- + +# Workflow +` + names := extractDispatchWorkflowNames(content) + assert.Empty(t, names, "should return empty slice when no safe-outputs section") +} + +// TestExtractDispatchWorkflowNames_NoDispatchWorkflow verifies that an empty slice is returned +// when safe-outputs exists but has no dispatch-workflow key. +func TestExtractDispatchWorkflowNames_NoDispatchWorkflow(t *testing.T) { + content := `--- +engine: copilot +safe-outputs: + add-comment: +--- + +# Workflow +` + names := extractDispatchWorkflowNames(content) + assert.Empty(t, names, "should return empty slice when no dispatch-workflow key") +} + +// TestExtractDispatchWorkflowNames_AllMacros verifies that all-macro lists return an empty slice. +func TestExtractDispatchWorkflowNames_AllMacros(t *testing.T) { + content := `--- +engine: copilot +safe-outputs: + dispatch-workflow: + - ${{ github.event.inputs.workflow }} + - ${{ vars.WORKFLOW }} +--- + +# Workflow +` + names := extractDispatchWorkflowNames(content) + assert.Empty(t, names, "should return empty slice when all workflow names are macros") +} + +// --- fetchAndSaveRemoteDispatchWorkflows tests --- + +// TestFetchAndSaveRemoteDispatchWorkflows_NoSafeOutputs verifies that the function is a +// no-op when the workflow has no safe-outputs section. +func TestFetchAndSaveRemoteDispatchWorkflows_NoSafeOutputs(t *testing.T) { + content := `--- +engine: copilot +--- + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "github/gh-aw", + Version: "main", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAndSaveRemoteDispatchWorkflows(content, spec, tmpDir, false, false, nil) + require.NoError(t, err, "should not error when no safe-outputs present") + + entries, readErr := os.ReadDir(tmpDir) + require.NoError(t, readErr) + assert.Empty(t, entries, "no files should be created when no dispatch-workflow configured") +} + +// TestFetchAndSaveRemoteDispatchWorkflows_EmptyRepoSlug verifies that the function is a +// no-op when the spec has no remote repo (local workflow). +func TestFetchAndSaveRemoteDispatchWorkflows_EmptyRepoSlug(t *testing.T) { + content := `--- +engine: copilot +safe-outputs: + dispatch-workflow: + - dependent-workflow +--- + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "", // local workflow + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAndSaveRemoteDispatchWorkflows(content, spec, tmpDir, false, false, nil) + require.NoError(t, err, "should not error for local workflow") + + entries, readErr := os.ReadDir(tmpDir) + require.NoError(t, readErr) + assert.Empty(t, entries, "no files should be created for local workflows") +} + +// TestFetchAndSaveRemoteDispatchWorkflows_OnlyMacros verifies that when all workflow names +// are GitHub Actions macro syntax, no download is attempted and the function is a no-op. +func TestFetchAndSaveRemoteDispatchWorkflows_OnlyMacros(t *testing.T) { + content := `--- +engine: copilot +safe-outputs: + dispatch-workflow: + - ${{ vars.WORKFLOW_TO_RUN }} + - ${{ github.event.inputs.workflow }} +--- + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "github/gh-aw", + Version: "main", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAndSaveRemoteDispatchWorkflows(content, spec, tmpDir, false, false, nil) + require.NoError(t, err, "should not error when all workflow names are macros") + + entries, readErr := os.ReadDir(tmpDir) + require.NoError(t, readErr) + assert.Empty(t, entries, "no files should be created when all workflow names are macros") +} + +// TestFetchAndSaveRemoteDispatchWorkflows_SkipExistingWithoutForce verifies that an existing +// dispatch workflow file is not re-downloaded when force=false. +func TestFetchAndSaveRemoteDispatchWorkflows_SkipExistingWithoutForce(t *testing.T) { + tmpDir := t.TempDir() + // Pre-existing file with a matching source field so it is treated as same-source (skip). + existingContent := []byte(`--- +source: github/gh-aw/.github/workflows/dependent-workflow.md@v1.0.0 +engine: copilot +--- +# Existing dependent workflow +`) + existingFile := filepath.Join(tmpDir, "dependent-workflow.md") + require.NoError(t, os.WriteFile(existingFile, existingContent, 0600)) + + content := `--- +engine: copilot +safe-outputs: + dispatch-workflow: + - dependent-workflow +--- + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "github/gh-aw", + Version: "v1.0.0", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + err := fetchAndSaveRemoteDispatchWorkflows(content, spec, tmpDir, false, false, nil) + require.NoError(t, err) + + // The existing file must be untouched (no network call attempted because file already exists) + gotContent, readErr := os.ReadFile(existingFile) + require.NoError(t, readErr) + assert.Equal(t, existingContent, gotContent, "pre-existing dispatch workflow file must not be modified when force=false") +} + +// TestFetchAndSaveRemoteDispatchWorkflows_TrackerUpdated verifies that a pre-existing file +// that is skipped due to force=false does NOT appear in any tracker list. +func TestFetchAndSaveRemoteDispatchWorkflows_TrackerNoOpOnExisting(t *testing.T) { + tmpDir := t.TempDir() + existingFile := filepath.Join(tmpDir, "dep.md") + // Pre-existing file with a matching source field so it is treated as same-source (skip). + existingContent := `--- +source: github/gh-aw/.github/workflows/dep.md@v1.0.0 +engine: copilot +--- +# Existing dep +` + require.NoError(t, os.WriteFile(existingFile, []byte(existingContent), 0600)) + + tracker := &FileTracker{ + OriginalContent: make(map[string][]byte), + gitRoot: tmpDir, + } + + content := `--- +engine: copilot +safe-outputs: + dispatch-workflow: + - dep +--- +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "github/gh-aw", + Version: "v1.0.0", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + err := fetchAndSaveRemoteDispatchWorkflows(content, spec, tmpDir, false, false, tracker) + require.NoError(t, err) + assert.Empty(t, tracker.CreatedFiles, "pre-existing file must not appear in CreatedFiles") + assert.Empty(t, tracker.ModifiedFiles, "pre-existing file must not appear in ModifiedFiles") +} + +// TestFetchAndSaveRemoteDispatchWorkflows_InvalidRepoSlug verifies that an invalid +// RepoSlug (not in owner/repo format) causes the function to return early without error. +func TestFetchAndSaveRemoteDispatchWorkflows_InvalidRepoSlug(t *testing.T) { + content := `--- +engine: copilot +safe-outputs: + dispatch-workflow: + - dep-workflow +--- +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "not-a-valid-slug", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAndSaveRemoteDispatchWorkflows(content, spec, tmpDir, false, false, nil) + require.NoError(t, err, "invalid RepoSlug should return nil without error") + + entries, readErr := os.ReadDir(tmpDir) + require.NoError(t, readErr) + assert.Empty(t, entries, "no files should be created for an invalid RepoSlug") +} + +// --- extractResources tests --- + +// TestExtractResources_BasicList verifies that resource paths are extracted from the resources field. +func TestExtractResources_BasicList(t *testing.T) { + content := `--- +engine: copilot +on: issues +resources: + - triage-issue.md + - close-stale.md + - my-action.yml +--- + +# Workflow +` + resources, err := extractResources(content) + require.NoError(t, err, "should not error for valid resources") + assert.Equal(t, []string{"triage-issue.md", "close-stale.md", "my-action.yml"}, resources, "should extract all listed resources") +} + +// TestExtractResources_MacroRejected verifies that an entry with GitHub Actions expression syntax causes an error. +func TestExtractResources_MacroRejected(t *testing.T) { + content := `--- +engine: copilot +on: issues +resources: + - plain-workflow.md + - ${{ vars.WORKFLOW }}.md +--- + +# Workflow +` + resources, err := extractResources(content) + require.Error(t, err, "should error when a resource entry contains macro syntax") + assert.Nil(t, resources, "should return nil resources on error") + assert.Contains(t, err.Error(), "${{", "error message should mention the disallowed syntax") +} + +// TestExtractResources_AllMacrosRejected verifies that all-macro lists return an error. +func TestExtractResources_AllMacrosRejected(t *testing.T) { + content := `--- +engine: copilot +on: issues +resources: + - ${{ vars.WORKFLOW_A }} + - ${{ vars.WORKFLOW_B }} +--- + +# Workflow +` + resources, err := extractResources(content) + require.Error(t, err, "should error when all resources are macros") + assert.Nil(t, resources) +} + +// TestExtractResources_NoResourcesField verifies that nil is returned when no resources field. +func TestExtractResources_NoResourcesField(t *testing.T) { + content := `--- +engine: copilot +on: issues +--- + +# Workflow +` + resources, err := extractResources(content) + require.NoError(t, err, "should not error when no resources field") + assert.Empty(t, resources, "should return nil when no resources field") +} + +// --- fetchAndSaveRemoteResources tests --- + +// TestFetchAndSaveRemoteResources_NoResources verifies that the function is a no-op when the +// workflow has no resources field. +func TestFetchAndSaveRemoteResources_NoResources(t *testing.T) { + content := `--- +engine: copilot +on: issues +--- + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "github/gh-aw", + Version: "main", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAndSaveRemoteResources(content, spec, tmpDir, false, false, nil) + require.NoError(t, err, "should not error when no resources field") + + entries, readErr := os.ReadDir(tmpDir) + require.NoError(t, readErr) + assert.Empty(t, entries, "no files should be created when no resources configured") +} + +// TestFetchAndSaveRemoteResources_EmptyRepoSlug verifies that the function is a no-op for local workflows. +func TestFetchAndSaveRemoteResources_EmptyRepoSlug(t *testing.T) { + content := `--- +engine: copilot +on: issues +resources: + - triage-issue.md +--- + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAndSaveRemoteResources(content, spec, tmpDir, false, false, nil) + require.NoError(t, err, "should not error for local workflow") + + entries, readErr := os.ReadDir(tmpDir) + require.NoError(t, readErr) + assert.Empty(t, entries, "no files should be created for local workflows") +} + +// TestFetchAndSaveRemoteResources_MacroRejected verifies that resources with macro syntax return an error. +func TestFetchAndSaveRemoteResources_MacroRejected(t *testing.T) { + content := `--- +engine: copilot +on: issues +resources: + - ${{ vars.WORKFLOW }} +--- + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "github/gh-aw", + Version: "main", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAndSaveRemoteResources(content, spec, tmpDir, false, false, nil) + require.Error(t, err, "should error when resources contain macro syntax") + assert.Contains(t, err.Error(), "${{", "error should mention the disallowed syntax") + + entries, readErr := os.ReadDir(tmpDir) + require.NoError(t, readErr) + assert.Empty(t, entries, "no files should be created when resources contain macros") +} + +// TestFetchAndSaveRemoteResources_SkipExistingWithoutForce verifies that an existing resource +// file is not re-downloaded when force=false. +func TestFetchAndSaveRemoteResources_SkipExistingWithoutForce(t *testing.T) { + tmpDir := t.TempDir() + // Pre-existing file with a matching source field so it is treated as same-source (skip). + existingContent := []byte(`--- +source: github/gh-aw/.github/workflows/triage-issue.md@v1.0.0 +engine: copilot +--- +# Existing triage issue workflow +`) + existingFile := filepath.Join(tmpDir, "triage-issue.md") + require.NoError(t, os.WriteFile(existingFile, existingContent, 0600)) + + content := `--- +engine: copilot +on: issues +resources: + - triage-issue.md +--- + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "github/gh-aw", + Version: "v1.0.0", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + err := fetchAndSaveRemoteResources(content, spec, tmpDir, false, false, nil) + require.NoError(t, err) + + gotContent, readErr := os.ReadFile(existingFile) + require.NoError(t, readErr) + assert.Equal(t, existingContent, gotContent, "pre-existing resource file must not be modified when force=false") +} + +// TestFetchAndSaveRemoteResources_PathTraversal verifies that path traversal attempts are rejected. +func TestFetchAndSaveRemoteResources_PathTraversal(t *testing.T) { + content := `--- +engine: copilot +on: issues +resources: + - ../etc/passwd + - ../../etc/shadow +--- + +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "github/gh-aw", + Version: "v1.0.0", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAndSaveRemoteResources(content, spec, tmpDir, false, false, nil) + require.NoError(t, err, "path traversal should be silently rejected") + + entries, readErr := os.ReadDir(tmpDir) + require.NoError(t, readErr) + assert.Empty(t, entries, "traversal resources must not write any file") +} + +// TestFetchAndSaveRemoteResources_InvalidRepoSlug verifies early return for invalid slug. +func TestFetchAndSaveRemoteResources_InvalidRepoSlug(t *testing.T) { + content := `--- +engine: copilot +on: issues +resources: + - triage-issue.md +--- +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "not-a-valid-slug", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + tmpDir := t.TempDir() + err := fetchAndSaveRemoteResources(content, spec, tmpDir, false, false, nil) + require.NoError(t, err, "invalid RepoSlug should return nil without error") + + entries, readErr := os.ReadDir(tmpDir) + require.NoError(t, readErr) + assert.Empty(t, entries, "no files should be created for an invalid RepoSlug") +} + +// TestFetchAndSaveRemoteResources_TrackerNoOpOnExisting verifies that a pre-existing resource +// that is skipped (force=false) does NOT appear in any tracker list. +func TestFetchAndSaveRemoteResources_TrackerNoOpOnExisting(t *testing.T) { + tmpDir := t.TempDir() + existingFile := filepath.Join(tmpDir, "resource.md") + // Pre-existing file with a matching source field so it is treated as same-source (skip). + existingContent := `--- +source: github/gh-aw/.github/workflows/resource.md@v1.0.0 +engine: copilot +--- +# Existing resource +` + require.NoError(t, os.WriteFile(existingFile, []byte(existingContent), 0600)) + + tracker := &FileTracker{ + OriginalContent: make(map[string][]byte), + gitRoot: tmpDir, + } + + content := `--- +engine: copilot +on: issues +resources: + - resource.md +--- +# Workflow +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "github/gh-aw", + Version: "v1.0.0", + }, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + err := fetchAndSaveRemoteResources(content, spec, tmpDir, false, false, tracker) + require.NoError(t, err) + assert.Empty(t, tracker.CreatedFiles, "pre-existing file must not appear in CreatedFiles") + assert.Empty(t, tracker.ModifiedFiles, "pre-existing file must not appear in ModifiedFiles") +} + +// --- fetchAndSaveDispatchWorkflowsFromParsedFile tests --- +// +// These tests verify that dispatch workflows discovered via parsed (compiled) workflow data are +// handled correctly — including the key shared agentic workflow scenario where dispatch-workflow +// config comes from an imported shared workflow rather than from the main workflow's own frontmatter. + +// TestFetchAndSaveDispatchWorkflowsFromParsedFile_EmptyRepoSlug verifies that a local +// (non-remote) workflow is a no-op. +func TestFetchAndSaveDispatchWorkflowsFromParsedFile_EmptyRepoSlug(t *testing.T) { + tmpDir := t.TempDir() + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: ""}, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + fetchAndSaveDispatchWorkflowsFromParsedFile(filepath.Join(tmpDir, "nonexistent.md"), spec, tmpDir, false, false, nil) + + entries, err := os.ReadDir(tmpDir) + require.NoError(t, err) + assert.Empty(t, entries, "no files should be created for local workflows") +} + +// TestFetchAndSaveDispatchWorkflowsFromParsedFile_InvalidRepoSlug verifies that an invalid +// RepoSlug (not owner/repo format) is a no-op. +func TestFetchAndSaveDispatchWorkflowsFromParsedFile_InvalidRepoSlug(t *testing.T) { + tmpDir := t.TempDir() + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "not-valid"}, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + fetchAndSaveDispatchWorkflowsFromParsedFile(filepath.Join(tmpDir, "nonexistent.md"), spec, tmpDir, false, false, nil) + + entries, err := os.ReadDir(tmpDir) + require.NoError(t, err) + assert.Empty(t, entries, "no files should be created for invalid RepoSlug") +} + +// TestFetchAndSaveDispatchWorkflowsFromParsedFile_ParseFailure verifies that a missing or +// unparseable workflow file is handled silently (best-effort no-op). +func TestFetchAndSaveDispatchWorkflowsFromParsedFile_ParseFailure(t *testing.T) { + tmpDir := t.TempDir() + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + // Point to a file that does not exist — ParseWorkflowFile will fail. + fetchAndSaveDispatchWorkflowsFromParsedFile(filepath.Join(tmpDir, "does-not-exist.md"), spec, tmpDir, false, false, nil) + + entries, err := os.ReadDir(tmpDir) + require.NoError(t, err) + assert.Empty(t, entries, "parse failures must not create any files") +} + +// TestFetchAndSaveDispatchWorkflowsFromParsedFile_NoDispatchWorkflows verifies that a +// workflow with no safe-outputs dispatch-workflow config is a no-op. +func TestFetchAndSaveDispatchWorkflowsFromParsedFile_NoDispatchWorkflows(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755)) + + mainPath := filepath.Join(workflowsDir, "my-workflow.md") + mainContent := `--- +on: issues +engine: copilot +permissions: + issues: read + contents: read +--- + +# Workflow with no dispatch-workflow +` + require.NoError(t, os.WriteFile(mainPath, []byte(mainContent), 0644)) + + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/my-workflow.md", + } + + fetchAndSaveDispatchWorkflowsFromParsedFile(mainPath, spec, workflowsDir, false, false, nil) + + // Only the main workflow itself should be in the directory. + entries, err := os.ReadDir(workflowsDir) + require.NoError(t, err) + assert.Len(t, entries, 1, "only the main workflow file should exist") +} + +// TestFetchAndSaveDispatchWorkflowsFromParsedFile_SharedWorkflow_SkipExisting is the key +// shared agentic workflow test: the dispatch-workflow list comes from an *imported* shared +// workflow, and the referenced dispatch workflow file already exists locally. +// +// Setup: +// +// .github/workflows/ +// main.md — imports shared/dispatch-helper.md +// shared/ +// dispatch-helper.md — shared workflow; defines safe-outputs.dispatch-workflow +// triage-issue.md — dispatch workflow that already exists locally +// +// Expected: fetchAndSaveDispatchWorkflowsFromParsedFile discovers "triage-issue" via the +// parsed (merged) safe-outputs config, finds the file already present, and skips without +// modifying it. +func TestFetchAndSaveDispatchWorkflowsFromParsedFile_SharedWorkflow_SkipExisting(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(workflowsDir, "shared") + require.NoError(t, os.MkdirAll(sharedDir, 0755)) + + // Shared workflow defines the dispatch-workflow config (no 'on:' field → treated as shared). + sharedPath := filepath.Join(sharedDir, "dispatch-helper.md") + sharedContent := `--- +safe-outputs: + dispatch-workflow: + workflows: + - triage-issue +--- + +Shared helper that configures dispatch-workflow. +` + require.NoError(t, os.WriteFile(sharedPath, []byte(sharedContent), 0644)) + + // Main workflow imports the shared workflow and provides its own triggers. + mainPath := filepath.Join(workflowsDir, "main.md") + mainContent := `--- +on: issues +engine: copilot +permissions: + issues: write + contents: read +imports: + - shared/dispatch-helper.md +--- + +# Main Workflow + +Process incoming issues. +` + require.NoError(t, os.WriteFile(mainPath, []byte(mainContent), 0644)) + + // The dispatch workflow file already exists locally with known content. + existingContent := []byte("# Triage Issue workflow") + triagePath := filepath.Join(workflowsDir, "triage-issue.md") + require.NoError(t, os.WriteFile(triagePath, existingContent, 0644)) + + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "v1.0.0"}, + WorkflowPath: ".github/workflows/main.md", + } + + fetchAndSaveDispatchWorkflowsFromParsedFile(mainPath, spec, workflowsDir, false, false, nil) + + // The pre-existing dispatch workflow must not be modified. + got, err := os.ReadFile(triagePath) + require.NoError(t, err) + assert.Equal(t, existingContent, got, "pre-existing dispatch workflow file must not be modified") +} + +// TestFetchAndSaveDispatchWorkflowsFromParsedFile_SharedWorkflow_TrackerNoOpOnExisting verifies +// that a pre-existing dispatch workflow discovered via a shared workflow import does NOT appear +// in the tracker's created or modified lists when force=false. +func TestFetchAndSaveDispatchWorkflowsFromParsedFile_SharedWorkflow_TrackerNoOpOnExisting(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(workflowsDir, "shared") + require.NoError(t, os.MkdirAll(sharedDir, 0755)) + + // Shared workflow with dispatch-workflow config. + sharedPath := filepath.Join(sharedDir, "dispatch-helper.md") + sharedContent := `--- +safe-outputs: + dispatch-workflow: + workflows: + - triage-issue +--- + +Shared dispatch configuration. +` + require.NoError(t, os.WriteFile(sharedPath, []byte(sharedContent), 0644)) + + // Main workflow imports the shared workflow. + mainPath := filepath.Join(workflowsDir, "main.md") + mainContent := `--- +on: issues +engine: copilot +permissions: + issues: write + contents: read +imports: + - shared/dispatch-helper.md +--- + +# Main Workflow +` + require.NoError(t, os.WriteFile(mainPath, []byte(mainContent), 0644)) + + // Dispatch workflow already on disk. + triagePath := filepath.Join(workflowsDir, "triage-issue.md") + require.NoError(t, os.WriteFile(triagePath, []byte("# Triage"), 0644)) + + tracker := &FileTracker{ + OriginalContent: make(map[string][]byte), + gitRoot: workflowsDir, + } + + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "v1.0.0"}, + WorkflowPath: ".github/workflows/main.md", + } + + fetchAndSaveDispatchWorkflowsFromParsedFile(mainPath, spec, workflowsDir, false, false, tracker) + + assert.Empty(t, tracker.CreatedFiles, "pre-existing dispatch workflow must not appear in CreatedFiles") + assert.Empty(t, tracker.ModifiedFiles, "pre-existing dispatch workflow must not appear in ModifiedFiles") +} + +// TestFetchAndSaveDispatchWorkflowsFromParsedFile_SharedWorkflow_MacroWorkflowSkipped verifies +// that workflow names containing GitHub Actions expression syntax (${{) in the merged +// dispatch-workflow list are silently skipped. +func TestFetchAndSaveDispatchWorkflowsFromParsedFile_SharedWorkflow_MacroWorkflowSkipped(t *testing.T) { + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + sharedDir := filepath.Join(workflowsDir, "shared") + require.NoError(t, os.MkdirAll(sharedDir, 0755)) + + // Shared workflow whose dispatch-workflow list contains a macro-valued entry. + sharedPath := filepath.Join(sharedDir, "dispatch-helper.md") + sharedContent := `--- +safe-outputs: + dispatch-workflow: + workflows: + - static-workflow + - ${{ vars.DYNAMIC_WORKFLOW }} +--- + +Shared dispatch helper with mixed static and macro workflow names. +` + require.NoError(t, os.WriteFile(sharedPath, []byte(sharedContent), 0644)) + + // Main workflow. + mainPath := filepath.Join(workflowsDir, "main.md") + mainContent := `--- +on: issues +engine: copilot +permissions: + issues: write + contents: read +imports: + - shared/dispatch-helper.md +--- + +# Main Workflow +` + require.NoError(t, os.WriteFile(mainPath, []byte(mainContent), 0644)) + + // Pre-create static-workflow.md so the fetch is skipped without a network call. + staticPath := filepath.Join(workflowsDir, "static-workflow.md") + require.NoError(t, os.WriteFile(staticPath, []byte("# Static"), 0644)) + + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "v1.0.0"}, + WorkflowPath: ".github/workflows/main.md", + } + + fetchAndSaveDispatchWorkflowsFromParsedFile(mainPath, spec, workflowsDir, false, false, nil) + + // No file named after the macro entry should have been created. + macroFile := filepath.Join(workflowsDir, "${{ vars.DYNAMIC_WORKFLOW }}.md") + _, err := os.Stat(macroFile) + assert.True(t, os.IsNotExist(err), "macro-valued dispatch workflow must not create a file") +} + +// --------------------------------------------------------------------------- +// readSourceRepoFromFile +// --------------------------------------------------------------------------- + +func TestSourceRepoLabel_NonEmpty(t *testing.T) { + assert.Equal(t, "github/gh-aw", sourceRepoLabel("github/gh-aw"), "non-empty repo should pass through") +} + +func TestSourceRepoLabel_Empty(t *testing.T) { + assert.Equal(t, "(no source field)", sourceRepoLabel(""), "empty repo should return placeholder label") +} + +func TestReadSourceRepoFromFile_ValidSource(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "wf.md") + content := `--- +source: github/gh-aw/.github/workflows/wf.md@abc123 +--- +# Workflow +` + require.NoError(t, os.WriteFile(f, []byte(content), 0644)) + assert.Equal(t, "github/gh-aw", readSourceRepoFromFile(f), "should return owner/repo") +} + +func TestReadSourceRepoFromFile_NoSource(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "wf.md") + content := "---\non: push\n---\n# No source\n" + require.NoError(t, os.WriteFile(f, []byte(content), 0644)) + assert.Empty(t, readSourceRepoFromFile(f), "should return empty string when no source field") +} + +func TestReadSourceRepoFromFile_MissingFile(t *testing.T) { + assert.Empty(t, readSourceRepoFromFile("/nonexistent/file.md"), "should return empty string for missing file") +} + +func TestReadSourceRepoFromFile_ShortSource(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "wf.md") + content := "---\nsource: only-one-segment\n---\n# Workflow\n" + require.NoError(t, os.WriteFile(f, []byte(content), 0644)) + assert.Empty(t, readSourceRepoFromFile(f), "should return empty string for malformed source") +} + +// --------------------------------------------------------------------------- +// fetchAndSaveRemoteDispatchWorkflows — conflict detection +// --------------------------------------------------------------------------- + +// TestFetchDispatchWorkflows_ConflictDifferentSource verifies that an error is returned +// when a dispatch-workflow target file already exists with a different source repo. +func TestFetchDispatchWorkflows_ConflictDifferentSource(t *testing.T) { + dir := t.TempDir() + workflowsDir := dir + + // Pre-existing file from a DIFFERENT source repo + existingPath := filepath.Join(dir, "target-workflow.md") + existingContent := `--- +source: otherorg/other-repo/.github/workflows/target-workflow.md@v1 +--- +# Target workflow from a different repo +` + require.NoError(t, os.WriteFile(existingPath, []byte(existingContent), 0644)) + + // Workflow content that references target-workflow as a dispatch-workflow + content := `--- +safe-outputs: + dispatch-workflow: + workflows: + - target-workflow +--- +# Main +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/main.md", + } + + err := fetchAndSaveRemoteDispatchWorkflows(content, spec, workflowsDir, false, false, nil) + require.Error(t, err, "should error when existing file has a different source repo") + assert.Contains(t, err.Error(), "target-workflow", "error should name the conflicting file") + assert.Contains(t, err.Error(), "otherorg/other-repo", "error should mention existing source") + assert.Contains(t, err.Error(), "github/gh-aw", "error should mention the intended source") +} + +// TestFetchDispatchWorkflows_SameSourceSkips verifies that an existing dispatch-workflow +// file from the SAME source repo is silently skipped without error. +func TestFetchDispatchWorkflows_SameSourceSkips(t *testing.T) { + dir := t.TempDir() + workflowsDir := dir + + // Pre-existing file from the SAME source repo + existingPath := filepath.Join(dir, "target-workflow.md") + existingContent := `--- +source: github/gh-aw/.github/workflows/target-workflow.md@v1 +--- +# Target workflow from the same repo +` + require.NoError(t, os.WriteFile(existingPath, []byte(existingContent), 0644)) + + content := `--- +safe-outputs: + dispatch-workflow: + workflows: + - target-workflow +--- +# Main +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/main.md", + } + + err := fetchAndSaveRemoteDispatchWorkflows(content, spec, workflowsDir, false, false, nil) + require.NoError(t, err, "should not error when existing file is from the same source repo") + + // File must not have been modified + got, readErr := os.ReadFile(existingPath) + require.NoError(t, readErr) + assert.Equal(t, existingContent, string(got), "existing same-source file must be left unchanged") +} + +// TestFetchDispatchWorkflows_NoSourceConflict verifies that a file with no source field +// is treated as a conflict (unknown origin). +func TestFetchDispatchWorkflows_NoSourceConflict(t *testing.T) { + dir := t.TempDir() + workflowsDir := dir + + // Pre-existing file with NO source field + existingPath := filepath.Join(dir, "target-workflow.md") + require.NoError(t, os.WriteFile(existingPath, []byte("# No source field\n"), 0644)) + + content := `--- +safe-outputs: + dispatch-workflow: + workflows: + - target-workflow +--- +# Main +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/main.md", + } + + err := fetchAndSaveRemoteDispatchWorkflows(content, spec, workflowsDir, false, false, nil) + require.Error(t, err, "should error when existing file has no source field") + assert.Contains(t, err.Error(), "target-workflow", "error should name the conflicting file") + assert.Contains(t, err.Error(), "(no source field)", "error should show placeholder for missing source") +} + +// TestFetchDispatchWorkflows_ForceOverwritesConflict verifies that --force bypasses conflict detection. +func TestFetchDispatchWorkflows_ForceOverwritesConflict(t *testing.T) { + dir := t.TempDir() + + // Pre-existing file from a DIFFERENT source — would conflict without force + existingPath := filepath.Join(dir, "target-workflow.md") + existingContent := `--- +source: otherorg/other-repo/.github/workflows/target-workflow.md@v1 +--- +# From other repo +` + require.NoError(t, os.WriteFile(existingPath, []byte(existingContent), 0644)) + + content := `--- +safe-outputs: + dispatch-workflow: + workflows: + - target-workflow +--- +# Main +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/main.md", + } + + // force=true should not error (even though there would normally be a conflict). + // Since there is no real network in unit tests, the download will fail; but the + // conflict check itself must NOT return an error. + // We simply verify the function is able to proceed past the conflict check. + // (The download failure is handled with a best-effort continue, so err==nil overall.) + err := fetchAndSaveRemoteDispatchWorkflows(content, spec, dir, false, true, nil) + assert.NoError(t, err, "force=true should bypass conflict detection and return nil (download fails silently)") +} + +// --------------------------------------------------------------------------- +// fetchAndSaveRemoteResources — conflict detection +// --------------------------------------------------------------------------- + +// TestFetchResources_MarkdownConflictDifferentSource verifies that an error is returned +// when a markdown resource file exists from a different source. +func TestFetchResources_MarkdownConflictDifferentSource(t *testing.T) { + dir := t.TempDir() + + // Pre-existing markdown resource from a DIFFERENT source + existingPath := filepath.Join(dir, "helper.md") + existingContent := `--- +source: otherorg/other-repo/.github/workflows/helper.md@v1 +--- +# Helper from different repo +` + require.NoError(t, os.WriteFile(existingPath, []byte(existingContent), 0644)) + + content := `--- +resources: + - helper.md +--- +# Main +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/main.md", + } + + err := fetchAndSaveRemoteResources(content, spec, dir, false, false, nil) + require.Error(t, err, "should error when markdown resource exists from a different source") + assert.Contains(t, err.Error(), "helper.md", "error should name the conflicting resource") +} + +// TestFetchResources_NonMarkdownConflict verifies that a non-markdown resource that already +// exists always triggers a conflict error (no source tracking for non-markdown files). +func TestFetchResources_NonMarkdownConflict(t *testing.T) { + dir := t.TempDir() + + // Pre-existing .yml resource (no source tracking) + existingPath := filepath.Join(dir, "helper.yml") + require.NoError(t, os.WriteFile(existingPath, []byte("name: Helper\n"), 0644)) + + content := `--- +resources: + - helper.yml +--- +# Main +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/main.md", + } + + err := fetchAndSaveRemoteResources(content, spec, dir, false, false, nil) + require.Error(t, err, "should error when non-markdown resource already exists") + assert.Contains(t, err.Error(), "helper.yml", "error should name the conflicting resource") +} + +// TestFetchResources_MarkdownSameSourceSkips verifies that an existing markdown resource +// from the same source repo is silently skipped without error. +func TestFetchResources_MarkdownSameSourceSkips(t *testing.T) { + dir := t.TempDir() + + // Pre-existing markdown resource from the SAME source + existingPath := filepath.Join(dir, "helper.md") + existingContent := `--- +source: github/gh-aw/.github/workflows/helper.md@main +--- +# Helper from same repo +` + require.NoError(t, os.WriteFile(existingPath, []byte(existingContent), 0644)) + + content := `--- +resources: + - helper.md +--- +# Main +` + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/main.md", + } + + err := fetchAndSaveRemoteResources(content, spec, dir, false, false, nil) + assert.NoError(t, err, "should not error when markdown resource is from the same source") +} + +// --------------------------------------------------------------------------- +// fetchAndSaveDispatchWorkflowsFromParsedFile — conflict detection (warning, not error) +// --------------------------------------------------------------------------- + +// TestFetchDispatchWorkflowsFromParsed_ConflictWarnsAndContinues verifies that a conflict +// in the post-write phase emits a warning but does NOT return an error (best-effort). +func TestFetchDispatchWorkflowsFromParsed_ConflictWarnsAndContinues(t *testing.T) { + workflowsDir := t.TempDir() + + // Main workflow: has dispatch-workflow: [triage-issue] + mainPath := filepath.Join(workflowsDir, "main.md") + mainContent := `--- +on: + issues: +permissions: + issues: write + contents: read +engine: copilot +safe-outputs: + dispatch-workflow: + workflows: + - triage-issue +--- +# Main Workflow +` + require.NoError(t, os.WriteFile(mainPath, []byte(mainContent), 0644)) + + // Pre-existing triage-issue.md from a DIFFERENT source + conflictPath := filepath.Join(workflowsDir, "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)) + + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "github/gh-aw", Version: "main"}, + WorkflowPath: ".github/workflows/main.md", + } + + // Must not panic or error — post-write is best-effort. + fetchAndSaveDispatchWorkflowsFromParsedFile(mainPath, spec, workflowsDir, false, false, nil) + + // The conflicting file must NOT have been overwritten. + got, readErr := os.ReadFile(conflictPath) + require.NoError(t, readErr) + assert.Equal(t, conflictContent, string(got), "conflict file must not be overwritten in post-write phase") +} diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 5534fcf5254..b6626ecb635 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -120,6 +120,18 @@ ] ] }, + "resources": { + "type": "array", + "description": "Optional list of additional workflow or action files that should be fetched alongside this workflow when running 'gh aw add'. Entries are relative paths (from the same directory as this workflow in the source repository) to agentic workflow .md files or GitHub Actions .yml/.yaml files. GitHub Actions expression syntax (${{) is not allowed in resource paths.", + "items": { + "type": "string", + "description": "Relative path to a workflow .md file or action .yml/.yaml file. Must be a static path; GitHub Actions expression syntax (${{) is not allowed.", + "not": { + "pattern": "\\$\\{\\{" + } + }, + "examples": [["triage-issue.md", "label-issue.md"], ["my-custom-action.yml"], ["shared/helper-action.yml", "close-stale.md"]] + }, "inlined-imports": { "type": "boolean", "default": false, diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 1d1c36d83ab..693f210868c 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -370,6 +370,17 @@ Ensure proper audience validation and trust policies are configured.` } } + // Validate resources field — GitHub Actions expression syntax is not allowed. + log.Printf("Validating resources field") + if workflowData.ParsedFrontmatter != nil { + for _, r := range workflowData.ParsedFrontmatter.Resources { + if strings.Contains(r, "${{") { + return formatCompilerError(markdownPath, "error", + fmt.Sprintf("resources entry %q contains GitHub Actions expression syntax (${{) which is not allowed; use static paths only", r), nil) + } + } + } + // Validate dispatch-workflow configuration (independent of agentic-workflows tool) log.Print("Validating dispatch-workflow configuration") if err := c.validateDispatchWorkflow(workflowData, markdownPath); err != nil { diff --git a/pkg/workflow/frontmatter_types.go b/pkg/workflow/frontmatter_types.go index 0ff346250cc..5c4d26d3c53 100644 --- a/pkg/workflow/frontmatter_types.go +++ b/pkg/workflow/frontmatter_types.go @@ -161,9 +161,10 @@ type FrontmatterConfig struct { Cache map[string]any `json:"cache,omitempty"` // Import and inclusion - Imports any `json:"imports,omitempty"` // Can be string or array - Include any `json:"include,omitempty"` // Can be string or array - InlinedImports bool `json:"inlined-imports,omitempty"` // If true, inline all imports at compile time instead of using runtime-import macros + Imports any `json:"imports,omitempty"` // Can be string or array + Include any `json:"include,omitempty"` // Can be string or array + InlinedImports bool `json:"inlined-imports,omitempty"` // If true, inline all imports at compile time instead of using runtime-import macros + Resources []string `json:"resources,omitempty"` // Additional workflow .md or action .yml files to fetch alongside this workflow // Metadata Metadata map[string]string `json:"metadata,omitempty"` // Custom metadata key-value pairs