From c3954788dea43baf30013070621ec448b2d9b4a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Apr 2026 01:20:45 +0000 Subject: [PATCH 1/2] Initial plan From 78a4f16fbc21397e4d4537dc04abb97ef12edfdd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Apr 2026 01:38:51 +0000 Subject: [PATCH 2/2] Fix recursive shared import downloads in gh aw add When a workflow imports a shared file that itself imports other shared files, those nested imports were not downloaded. The bug was in fetchFrontmatterImportsRecursive which resolved all relative paths against currentBaseDir (the importing file's directory), producing wrong paths like .github/workflows/shared/shared/daily-audit-discussion.md instead of .github/workflows/shared/daily-audit-discussion.md. Fix: non-explicit relative paths (not starting with ./ or ../) are now resolved against originalBaseDir (the top-level workflow root), matching the convention where shared imports are always written relative to .github/workflows/. Explicitly-relative paths (./file.md) continue to resolve against currentBaseDir. Also adds TestAddWorkflowWithRecursiveSharedImports integration test that verifies 2-3 level deep import trees are fully downloaded. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4dde7cb4-1fe9-4b3a-b756-d5ce0653102f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/add_integration_test.go | 73 +++++++++++++++++++++++++++++++++ pkg/cli/includes.go | 31 ++++++++++++-- 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/pkg/cli/add_integration_test.go b/pkg/cli/add_integration_test.go index de9555b6639..a90c38bc60c 100644 --- a/pkg/cli/add_integration_test.go +++ b/pkg/cli/add_integration_test.go @@ -1059,3 +1059,76 @@ func TestAddWorkflowWithDispatchWorkflowFromSharedImport(t *testing.T) { assert.Contains(t, string(lockContent), "haiku-printer", "lock file should reference the haiku-printer dispatch-workflow target") } + +// TestAddWorkflowWithRecursiveSharedImports verifies that `gh aw add` recursively +// downloads all transitively-imported shared markdown files. +// +// daily-compiler-quality.md (at commit 8d26856) has this two-level import tree: +// +// daily-compiler-quality.md +// ├── shared/daily-audit-base.md (direct) +// │ ├── shared/daily-audit-discussion.md (nested level 2) +// │ ├── shared/reporting.md (nested level 2) +// │ └── shared/observability-otlp.md (nested level 2) +// └── shared/go-source-analysis.md (direct) +// ├── shared/mcp/serena-go.md (nested level 2) +// │ └── shared/mcp/serena.md (nested level 3, via "./serena.md") +// └── shared/reporting.md (nested level 2, shared with above) +// +// This test would fail without the fix to fetchFrontmatterImportsRecursive that +// resolves non-explicit relative paths (e.g. "shared/foo.md") against originalBaseDir +// rather than currentBaseDir. +// +// This test requires GitHub authentication. +func TestAddWorkflowWithRecursiveSharedImports(t *testing.T) { + 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() + + // Pin to commit 8d26856 so the import tree is stable and reproducible. + workflowSpec := "github/gh-aw/.github/workflows/daily-compiler-quality.md@8d26856" + + 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. Main workflow must be present. + require.FileExists(t, filepath.Join(workflowsDir, "daily-compiler-quality.md"), + "main workflow daily-compiler-quality.md should exist") + + // 2. Direct imports must be present. + assert.FileExists(t, filepath.Join(workflowsDir, "shared", "daily-audit-base.md"), + "direct import shared/daily-audit-base.md should be fetched") + assert.FileExists(t, filepath.Join(workflowsDir, "shared", "go-source-analysis.md"), + "direct import shared/go-source-analysis.md should be fetched") + + // 3. Transitive imports via shared/daily-audit-base.md must be present. + assert.FileExists(t, filepath.Join(workflowsDir, "shared", "daily-audit-discussion.md"), + "transitive import shared/daily-audit-discussion.md (via daily-audit-base) should be fetched") + assert.FileExists(t, filepath.Join(workflowsDir, "shared", "reporting.md"), + "transitive import shared/reporting.md (via daily-audit-base) should be fetched") + assert.FileExists(t, filepath.Join(workflowsDir, "shared", "observability-otlp.md"), + "transitive import shared/observability-otlp.md (via daily-audit-base) should be fetched") + + // 4. Transitive imports via shared/go-source-analysis.md must be present. + assert.FileExists(t, filepath.Join(workflowsDir, "shared", "mcp", "serena-go.md"), + "transitive import shared/mcp/serena-go.md (via go-source-analysis) should be fetched") + // serena-go.md imports ./serena.md (explicitly-relative), which should resolve to + // shared/mcp/serena.md and be fetched correctly. + assert.FileExists(t, filepath.Join(workflowsDir, "shared", "mcp", "serena.md"), + "deep transitive import shared/mcp/serena.md (via go-source-analysis → serena-go.md) should be fetched") + + // 5. Compilation must have succeeded (lock file present). + assert.FileExists(t, filepath.Join(workflowsDir, "daily-compiler-quality.lock.yml"), + "compiled lock file daily-compiler-quality.lock.yml should exist") +} diff --git a/pkg/cli/includes.go b/pkg/cli/includes.go index 007f480d171..d6301864a5f 100644 --- a/pkg/cli/includes.go +++ b/pkg/cli/includes.go @@ -227,16 +227,39 @@ func fetchFrontmatterImportsRecursive(content, owner, repo, ref, currentBaseDir, continue } - // Resolve the remote file path relative to the current file's directory. + // Resolve the remote file path to an absolute repo path. // Use path (not filepath) because this is always a forward-slash URL/API path. var remoteFilePath string if rest, ok := strings.CutPrefix(filePath, "/"); ok { // Absolute path from repo root (e.g. "/scripts/helper.md") remoteFilePath = rest - } else if currentBaseDir != "" { - remoteFilePath = path.Join(currentBaseDir, filePath) + } else if strings.HasPrefix(filePath, "./") || strings.HasPrefix(filePath, "../") { + // Explicitly-relative path (e.g. "./serena.md"): resolve relative to the + // current importing file's directory so that sibling-file references work + // correctly regardless of nesting depth. + if currentBaseDir != "" { + remoteFilePath = path.Join(currentBaseDir, filePath) + } else { + remoteFilePath = filePath + } } else { - remoteFilePath = filePath + // Non-explicit relative path (e.g. "shared/foo.md"): resolve relative to the + // original base directory (the top-level workflow's directory). Workflows in + // this repository write shared import paths relative to the workflow root + // (e.g. ".github/workflows"), not relative to the importing file's own + // directory. Resolving against originalBaseDir instead of currentBaseDir + // ensures that a file at ".github/workflows/shared/base.md" can import + // "shared/helper.md" and have it resolve to ".github/workflows/shared/helper.md" + // rather than the incorrect ".github/workflows/shared/shared/helper.md". + baseDir := originalBaseDir + if baseDir == "" { + baseDir = currentBaseDir + } + if baseDir != "" { + remoteFilePath = path.Join(baseDir, filePath) + } else { + remoteFilePath = filePath + } } remoteFilePath = path.Clean(remoteFilePath)