fix: gh aw add resolves uses: references transitively in imported files

pkg/cli/includes.go

@@ -184,6 +184,10 @@ type frontmatterImportsOpts struct {
 	force           bool
 	tracker         *FileTracker
 	seen            map[string]struct{}
+	// downloadFn is the function used to fetch file content from the source repository.
+	// When nil, parser.DownloadFileFromGitHub is used. Tests may inject a stub to avoid
+	// network calls and observe which paths were requested.
+	downloadFn func(ctx context.Context, owner, repo, path, ref string) ([]byte, error)
 }
 
 // fetchFrontmatterImportsRecursive is the internal worker for fetchAndSaveRemoteFrontmatterImports.
@@ -275,15 +279,28 @@ func fetchFrontmatterImportsRecursive(ctx context.Context, content, currentBaseD
 				remoteFilePath = filePath
 			}
 		} else {
-			// 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 := opts.originalBaseDir
+			// Non-explicit relative path: resolution strategy mirrors the compiler's
+			// determineNestedBaseDir logic — it depends on whether the path contains a
+			// "/" separator.
+			//
+			// Paths WITHOUT "/" (e.g. "helper.md"): the importing file treats them as
+			// siblings in its own directory, so resolve relative to currentBaseDir.
+			// Example: "control-precompute.md" from ".github/workflows/shared/control.md"
+			//          → ".github/workflows/shared/control-precompute.md"
+			//
+			// Paths WITH "/" (e.g. "shared/foo.md"): workflows in this repository write
+			// these paths relative to the workflow root (.github/workflows), not relative to
+			// the importing file's directory, so resolve against originalBaseDir.
+			// Example: "shared/reporting.md" from ".github/workflows/shared/daily-audit-base.md"
+			//          → ".github/workflows/shared/reporting.md"
+			var baseDir string
+			if !strings.Contains(filePath, "/") {
+				baseDir = currentBaseDir
+			} else {
+				baseDir = opts.originalBaseDir
+			}
+			// Fallback: if the selected baseDir is empty (e.g. workflow at repo root
+			// with no originalBaseDir set), use currentBaseDir as a last resort.
 			if baseDir == "" {
 				baseDir = currentBaseDir
 			}
@@ -349,20 +366,35 @@ func fetchFrontmatterImportsRecursive(ctx context.Context, content, currentBaseD
 		}
 
 		// Check existence before downloading: if the file already exists and force=false,
-		// skip the download entirely (no unnecessary network round-trip).
+		// skip the download entirely (no unnecessary network round-trip). However, still
+		// recurse into the existing file's imports so that any transitive dependencies
+		// it references are fetched even when the parent file was already present.
 		fileExists := false
 		if fileutil.FileExists(targetPath) {
 			fileExists = true
 			if !opts.force {
 				if opts.verbose {
 					fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Import file already exists, skipping: "+targetPath))
 				}
+				// Read the existing file so we can recurse into its own imports.
+				// If the read fails, log it and skip — the compiler will report
+				// any missing transitive dependencies.
+				if existingContent, readErr := os.ReadFile(targetPath); readErr == nil {
+					importedBaseDir := path.Dir(remoteFilePath)
+					fetchFrontmatterImportsRecursive(ctx, string(existingContent), importedBaseDir, opts)
+				} else {
+					remoteWorkflowLog.Printf("Failed to read existing import %s for recursion: %v", targetPath, readErr)
+				}
 				continue
 			}
 		}
 
 		// Download from the source repository
-		importContent, err := parser.DownloadFileFromGitHub(ctx, opts.owner, opts.repo, remoteFilePath, opts.ref)
+		downloadFn := opts.downloadFn
+		if downloadFn == nil {
+			downloadFn = parser.DownloadFileFromGitHub
+		}
+		importContent, err := downloadFn(ctx, opts.owner, opts.repo, remoteFilePath, opts.ref)
 		if err != nil {
 			remoteWorkflowLog.Printf("Failed to download import %s from %s/%s@%s: %v", remoteFilePath, opts.owner, opts.repo, opts.ref, err)
 			if opts.verbose {

pkg/cli/remote_workflow_test.go

@@ -686,6 +686,126 @@ imports:
 	assert.Empty(t, entries, "no files should be created for an invalid RepoSlug")
 }
 
+// TestFetchFrontmatterImportsRecursive_SiblingPathResolution verifies that when a
+// shared file imports a sibling using a bare filename (no "/" character), the path
+// is resolved relative to the importing file's directory rather than the original
+// workflow base directory. This mirrors the compiler's determineNestedBaseDir logic.
+func TestFetchFrontmatterImportsRecursive_SiblingPathResolution(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Track which remote paths the downloader is asked for, and return minimal content.
+	var downloadedPaths []string
+	mockDownload := func(_ context.Context, _, _, remoteFilePath, _ string) ([]byte, error) {
+		downloadedPaths = append(downloadedPaths, remoteFilePath)
+		return []byte("# stub\n"), nil
+	}
+
+	// shared/control.md imports a sibling via a bare filename (no "/").
+	controlContent := `---
+imports:
+  - control-aux.md
+---
+# Control
+`
+
+	seen := make(map[string]struct{})
+	opts := frontmatterImportsOpts{
+		owner:           "github",
+		repo:            "gh-aw",
+		ref:             "main",
+		originalBaseDir: ".github/workflows",
+		targetDir:       tmpDir,
+		verbose:         false,
+		force:           true, // force=true so the download step is always reached
+		tracker:         &FileTracker{OriginalContent: make(map[string][]byte)},
+		seen:            seen,
+		downloadFn:      mockDownload,
+	}
+
+	// Process shared/control.md; currentBaseDir mirrors its location in the source repo.
+	fetchFrontmatterImportsRecursive(t.Context(), controlContent, ".github/workflows/shared", opts)
+
+	// "control-aux.md" (bare filename, no "/") must resolve relative to the importing
+	// file's directory → ".github/workflows/shared/control-aux.md".
+	require.Len(t, downloadedPaths, 1, "downloader must be called exactly once")
+	assert.Equal(t, ".github/workflows/shared/control-aux.md", downloadedPaths[0],
+		"sibling import must resolve to the shared/ subdir, not the workflow root")
+
+	// The file must be saved at the correct local path inside targetDir.
+	assert.FileExists(t, filepath.Join(tmpDir, "shared", "control-aux.md"),
+		"file must be written to shared/control-aux.md")
+	assert.NoFileExists(t, filepath.Join(tmpDir, "control-aux.md"),
+		"file must not be written at the root level (wrong path resolution)")
+}
+
+// TestFetchFrontmatterImportsRecursive_RecurseIntoExistingFile verifies that when a
+// top-level import already exists on disk (force=false), the function still recurses
+// into that file's imports so that transitive dependencies are fetched. This test
+// also covers sibling-path resolution: the transitive dep uses a bare filename, so it
+// should resolve to the shared file's directory, not the workflow root.
+func TestFetchFrontmatterImportsRecursive_RecurseIntoExistingFile(t *testing.T) {
+	tmpDir := t.TempDir()
+	sharedDir := filepath.Join(tmpDir, "shared")
+	require.NoError(t, os.MkdirAll(sharedDir, 0755))
+
+	// shared/control.md already exists locally; it imports control-aux.md (a sibling).
+	controlContent := `---
+imports:
+  - control-aux.md
+---
+# Control
+`
+	require.NoError(t, os.WriteFile(filepath.Join(sharedDir, "control.md"), []byte(controlContent), 0600))
+
+	// Track which remote paths are downloaded.
+	var downloadedPaths []string
+	mockDownload := func(_ context.Context, _, _, remoteFilePath, _ string) ([]byte, error) {
+		downloadedPaths = append(downloadedPaths, remoteFilePath)
+		return []byte("# stub\n"), nil
+	}
+
+	// Top-level content imports shared/control.md (which already exists on disk).
+	topContent := `---
+engine: copilot
+imports:
+  - shared/control.md
+---
+# Orchestrator
+`
+
+	seen := make(map[string]struct{})
+	opts := frontmatterImportsOpts{
+		owner:           "github",
+		repo:            "gh-aw",
+		ref:             "v1.0.0",
+		originalBaseDir: ".github/workflows",
+		targetDir:       tmpDir,
+		verbose:         false,
+		force:           false, // force=false so pre-existing files trigger the "skip + recurse" path
+		tracker:         &FileTracker{OriginalContent: make(map[string][]byte)},
+		seen:            seen,
+		downloadFn:      mockDownload,
+	}
+
+	fetchFrontmatterImportsRecursive(t.Context(), topContent, ".github/workflows", opts)
+
+	// shared/control.md existed on disk → must NOT be downloaded.
+	assert.NotContains(t, downloadedPaths, ".github/workflows/shared/control.md",
+		"pre-existing shared/control.md must not be re-downloaded")
+
+	// The function must recurse into the existing file and discover its sibling dep.
+	// control-aux.md (bare filename, no "/") must resolve to shared/control-aux.md.
+	require.Len(t, downloadedPaths, 1, "transitive dep control-aux.md must be fetched exactly once")
+	assert.Equal(t, ".github/workflows/shared/control-aux.md", downloadedPaths[0],
+		"transitive dep must be fetched from the shared/ subdir")
+
+	// The transitive dep must be saved at the correct local path.
+	assert.FileExists(t, filepath.Join(tmpDir, "shared", "control-aux.md"),
+		"transitive dep must be written to shared/control-aux.md")
+	assert.NoFileExists(t, filepath.Join(tmpDir, "control-aux.md"),
+		"transitive dep must not be written at the root level")
+}
+
 // --- extractDispatchWorkflowNames tests ---
 
 // TestExtractDispatchWorkflowNames_ArrayFormat verifies that workflow names are extracted