diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index 56e4348652a..707bcc5a3e4 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -417,6 +417,15 @@ func addWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTracker, o if sourceInfo != nil { commitSHA = sourceInfo.CommitSHA } + // When the fetch used a fallback path (e.g. .github/workflows/my-workflow.md instead + // of the short-form my-workflow.md), SourcePath holds the actual repo-root-relative + // path. Propagate it to workflowSpec so all downstream processing (source field, + // include/import resolution) uses the canonical path. + if sourceInfo != nil && !sourceInfo.IsLocal && sourceInfo.SourcePath != "" && sourceInfo.SourcePath != workflowSpec.WorkflowPath { + specCopy := *workflowSpec + specCopy.WorkflowPath = sourceInfo.SourcePath + workflowSpec = &specCopy + } sourceString := buildSourceStringWithCommitSHA(workflowSpec, commitSHA) if sourceString != "" { updatedContent, err := addSourceToWorkflow(content, sourceString) diff --git a/pkg/cli/add_command_test.go b/pkg/cli/add_command_test.go index 2428107d986..fe5db735769 100644 --- a/pkg/cli/add_command_test.go +++ b/pkg/cli/add_command_test.go @@ -4,8 +4,12 @@ package cli import ( "context" + "os" + "os/exec" + "path/filepath" "testing" + "github.com/github/gh-aw/pkg/testutil" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -349,3 +353,180 @@ func TestAddMultipleWorkflowsNameFlag(t *testing.T) { require.Error(t, err, "Should error when --name is used with multiple workflows") assert.Contains(t, err.Error(), "--name flag cannot be used when adding multiple workflows", "Error should mention --name restriction") } + +// setupMinimalGitRepo initialises a bare-minimum git repo in dir and returns the +// path to the .github/workflows directory so callers can write/read workflow files. +func setupMinimalGitRepo(t *testing.T, dir string) string { + t.Helper() + + t.Setenv("HOME", dir) + t.Chdir(dir) + + initCmd := exec.Command("git", "init") + initCmd.Dir = dir + require.NoError(t, initCmd.Run(), "git init should succeed") + + gitConfigName := exec.Command("git", "config", "user.name", "Test User") + gitConfigName.Dir = dir + _ = gitConfigName.Run() + gitConfigEmail := exec.Command("git", "config", "user.email", "test@example.com") + gitConfigEmail.Dir = dir + _ = gitConfigEmail.Run() + + workflowsDir := filepath.Join(dir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowsDir, 0755), "should create workflows dir") + + return workflowsDir +} + +// TestAddWorkflowWithTracking_SourceFieldVariants covers the main combinations of local / +// remote specs and fallback-path resolution for the source: frontmatter field written by +// addWorkflowWithTracking. +func TestAddWorkflowWithTracking_SourceFieldVariants(t *testing.T) { + simpleContent := []byte("---\non: push\n---\n\n# Workflow\n") + const sha = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + tests := []struct { + name string + spec *WorkflowSpec + sourceInfo *FetchedWorkflow + wantContains string + wantNotContains string + }{ + { + // Local workflows must NOT get a source: field — the code guards on !sourceInfo.IsLocal. + name: "local workflow — no source field written", + spec: &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: ""}, + WorkflowPath: "./local-workflow.md", + WorkflowName: "local-workflow", + }, + sourceInfo: &FetchedWorkflow{ + Content: simpleContent, + CommitSHA: "", + IsLocal: true, + SourcePath: "./local-workflow.md", + }, + wantContains: "", + wantNotContains: "source:", + }, + { + // Remote workflow where the parsed spec path already matches SourcePath + // (no fallback triggered). The source: field must use the original path. + name: "remote workflow — no fallback, path matches SourcePath", + spec: &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "owner/repo", Version: "main"}, + WorkflowPath: ".github/workflows/my-workflow.md", + WorkflowName: "my-workflow", + }, + sourceInfo: &FetchedWorkflow{ + Content: simpleContent, + CommitSHA: sha, + IsLocal: false, + SourcePath: ".github/workflows/my-workflow.md", // identical — no fallback + }, + wantContains: "source: owner/repo/.github/workflows/my-workflow.md@" + sha, + wantNotContains: "", + }, + { + // Remote workflow from the *current* repository (self-referential) where + // the spec only carries the short name but the file lives under + // .github/workflows/. Fallback resolution must be reflected in source:. + name: "self-referential remote — fallback path resolution", + spec: &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "current-org/current-repo", Version: "main"}, + WorkflowPath: "my-workflow.md", // short-form from parsed spec + WorkflowName: "my-workflow", + }, + sourceInfo: &FetchedWorkflow{ + Content: simpleContent, + CommitSHA: sha, + IsLocal: false, + SourcePath: ".github/workflows/my-workflow.md", // resolved via fallback + }, + wantContains: "source: current-org/current-repo/.github/workflows/my-workflow.md@" + sha, + wantNotContains: "source: current-org/current-repo/my-workflow.md", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := testutil.TempDir(t, "test-source-field-variant-*") + workflowsDir := setupMinimalGitRepo(t, tempDir) + + resolved := &ResolvedWorkflow{ + Spec: tt.spec, + Content: tt.sourceInfo.Content, + SourceInfo: tt.sourceInfo, + } + opts := AddOptions{DisableSecurityScanner: true} + + err := addWorkflowWithTracking(resolved, nil, opts) + require.NoError(t, err, "addWorkflowWithTracking should succeed") + + written, err := os.ReadFile(filepath.Join(workflowsDir, tt.spec.WorkflowName+".md")) + require.NoError(t, err, "written file should be readable") + body := string(written) + + if tt.wantContains != "" { + assert.Contains(t, body, tt.wantContains, "source field should contain expected path") + } + if tt.wantNotContains != "" { + assert.NotContains(t, body, tt.wantNotContains, "source field must not contain forbidden path") + } + }) + } +} + +// TestAddWorkflowWithTracking_UsesActualFetchedPath verifies that when a remote workflow is +// fetched via a fallback path (e.g. .github/workflows/my-workflow.md instead of the +// short-form my-workflow.md), the written source: field reflects the actual fetched path +// so that gh aw update can later re-fetch from the correct location. +func TestAddWorkflowWithTracking_UsesActualFetchedPath(t *testing.T) { + // Set up a temp git repo + tempDir := testutil.TempDir(t, "test-add-source-path-*") + workflowsDir := setupMinimalGitRepo(t, tempDir) + + // Simple workflow content with no remote dependencies (avoids network calls) + content := []byte("---\non: push\n---\n\n# Test Workflow\n") + + // Simulate: spec parsed from short-form "owner/repo/my-workflow.md@main" + // The file was actually found at .github/workflows/my-workflow.md via fallback. + spec := &WorkflowSpec{ + RepoSpec: RepoSpec{ + RepoSlug: "owner/repo", + Version: "main", + }, + WorkflowPath: "my-workflow.md", // short-form path from parsed spec + WorkflowName: "my-workflow", + } + sourceInfo := &FetchedWorkflow{ + Content: content, + CommitSHA: "abc123def456789012345678901234567890abcd", + IsLocal: false, + SourcePath: ".github/workflows/my-workflow.md", // actual path found via fallback + } + resolved := &ResolvedWorkflow{ + Spec: spec, + Content: content, + SourceInfo: sourceInfo, + } + + opts := AddOptions{ + DisableSecurityScanner: true, + } + err := addWorkflowWithTracking(resolved, nil, opts) + require.NoError(t, err, "addWorkflowWithTracking should succeed") + + // Read the written file + written, err := os.ReadFile(filepath.Join(workflowsDir, "my-workflow.md")) + require.NoError(t, err, "written file should be readable") + + // The source: field must use the full path, not the short-form WorkflowPath + assert.Contains(t, string(written), + "source: owner/repo/.github/workflows/my-workflow.md@abc123def456789012345678901234567890abcd", + "source field should use the actual fetched path so gh aw update can find the file") + assert.NotContains(t, string(written), + "source: owner/repo/my-workflow.md", + "source field must NOT use the short-form path that causes 404 on gh aw update") +} diff --git a/pkg/cli/trial_repository.go b/pkg/cli/trial_repository.go index 22f19427057..556b5eb753d 100644 --- a/pkg/cli/trial_repository.go +++ b/pkg/cli/trial_repository.go @@ -248,6 +248,17 @@ func installWorkflowInTrialMode(ctx context.Context, tempDir string, parsedSpec content := fetched.Content + // When the fetch used a fallback path (e.g. .github/workflows/my-workflow.md + // instead of the short-form my-workflow.md), SourcePath holds the actual + // repo-root-relative path. Normalize parsedSpec so all downstream dependency + // resolution (source field, includes, imports, dispatch workflows, resources) + // uses the same effective workflow path. + if !fetched.IsLocal && fetched.SourcePath != "" && fetched.SourcePath != parsedSpec.WorkflowPath { + specCopy := *parsedSpec + specCopy.WorkflowPath = fetched.SourcePath + parsedSpec = &specCopy + } + // Add source field to frontmatter for remote workflows if !fetched.IsLocal && fetched.CommitSHA != "" { sourceString := buildSourceStringWithCommitSHA(parsedSpec, fetched.CommitSHA)