Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions pkg/cli/add_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
181 changes: 181 additions & 0 deletions pkg/cli/add_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
}
11 changes: 11 additions & 0 deletions pkg/cli/trial_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down