Imported `steps:` run in traversal order, and import cycles in subdirectories are not detected

[Etienne-M]

Summary

Imported steps: are concatenated in BFS discovery order rather than dependency order, so a shared workflow's own steps can run before the prerequisites it imports. Separately, circular imports between files in a subdirectory compile successfully.

Both stem from the same area: topologicalSortImports in pkg/parser/import_topological.go already implements Kahn's algorithm, but its result is computed after all field merging has happened and is only used to populate a manifest list, and the dependency graph it builds silently drops every edge to an import that lives outside the workflow's base directory.

Reproduction

Six shared files:

# .github/workflows/shared/b.md
---
steps: [{ name: STEP-B, run: echo B }]
---

# .github/workflows/shared/a.md   (same imports+steps as p1-a-as-root.md, minus `on:`)
---
imports: [{ uses: b.md }]
steps: [{ name: STEP-A, run: echo A }]
---

# .github/workflows/shared/dep.md
---
steps: [{ name: STEP-DEP, run: echo DEP }]
---

# .github/workflows/shared/lib.md
---
imports: [{ uses: dep.md }]
steps: [{ name: STEP-LIB, run: echo LIB }]
---

# .github/workflows/shared/sub-a.md
---
imports: [{ uses: sub-b.md }]
steps: [{ name: STEP-SUBA, run: echo SUBA }]
---

# .github/workflows/shared/sub-b.md
---
imports: [{ uses: sub-a.md }]
steps: [{ name: STEP-SUBB, run: echo SUBB }]
---

Five root workflows, each carrying the same header (on: pull_request: types: [opened], permissions: contents: read, engine: copilot) and differing only in imports::

File	imports:	Plus own steps:
p1-a-as-root.md	shared/b.md	STEP-A
p1-a-as-import.md	shared/a.md	—
p2-dep-declared-first.md	shared/dep.md, shared/lib.md	—
p2-lib-declared-first.md	shared/lib.md, shared/dep.md	—
p3-cycle-in-subdir.md	shared/sub-a.md	—

Compiling all of them together:

$ gh aw compile
✓ Compiled 5 workflows: 5 succeeded, 0 warnings

$ for f in .github/workflows/p*.lock.yml; do echo "$(basename $f .lock.yml): $(grep -o 'STEP-[A-Z]*' $f | tr '\n' ' ')"; done
p1-a-as-import: STEP-A STEP-B
p1-a-as-root: STEP-B STEP-A
p2-dep-declared-first: STEP-DEP STEP-LIB
p2-lib-declared-first: STEP-LIB STEP-DEP
p3-cycle-in-subdir: STEP-SUBA STEP-SUBB

Expected:

Workflow	Actual	Expected
p1-a-as-root	STEP-B STEP-A	STEP-B STEP-A — prerequisite first
p1-a-as-import	STEP-A STEP-B	STEP-B STEP-A — order must not depend on the file's role
p2-dep-declared-first	STEP-DEP STEP-LIB	STEP-DEP STEP-LIB — prerequisite first
p2-lib-declared-first	STEP-LIB STEP-DEP	STEP-DEP STEP-LIB — order must not depend on sibling declaration order
p3-cycle-in-subdir	compiles successfully	ImportCycleError naming the cycle

The same cycle placed in the workflow directory rather than shared/ is correctly rejected, which isolates the second defect to path resolution:

$ gh aw compile          # flat-a.md ⟷ flat-b.md, both alongside the root workflow
✗ Compiled 3 workflows: 2 succeeded, 1 failed, 1 error, 0 warnings

Analysis / Root cause

1. The sorted order is produced after merging and used only as metadata

processImports runs the whole BFS traversal, then sorts:

// pkg/parser/import_bfs.go:46
if err := processImportQueue(baseDir, cache, workflowFilePath, yamlContent, state); err != nil {
    return nil, err
}
topologicalOrder, err := topologicalSortImports(state.processedOrder, baseDir, cache, workflowFilePath)
...
return state.acc.toImportsResult(topologicalOrder), nil

By that point every file's steps: has already been appended to acc.stepsBuilder in queue-pop order, via processQueueItem → handleStandardImportItem → extractAllImportFields (pkg/parser/import_field_extractor.go:413, pkg/parser/import_field_extractor.go:536). The sorted slice feeds exactly one field:

// pkg/parser/import_field_extractor.go — toImportsResult
result := acc.buildImportsResult()
result.ImportedFiles = topologicalOrder

So it never reaches MergedSteps, MergedPreAgentSteps, or MergedPostSteps.

2. Dependency edges are dropped for imports outside the base directory

buildImportDependencies resolves every import against the top-level baseDir, and extractImportPaths returns nested paths exactly as written in frontmatter. Those raw strings are then matched by exact equality:

// pkg/parser/import_topological.go — calculateInDegree
if setutil.Contains(allImportsSet, dep) {
    inDegree[imp]++
}

For shared/lib.md containing uses: dep.md:

Recorded as a dependency of shared/lib.md	Key present in processedOrder	Match?
"dep.md"	"shared/dep.md"	❌

The edge is discarded, every node keeps in-degree 0, and Kahn emits the input slice unchanged. The BFS itself resolves these correctly using the importing file's own directory (filepath.Dir(item.fullPath) in enqueueNestedImportEntry); the sort does not reuse that resolution.

Because the graph ends up edgeless, len(result) == len(imports) and the cycle branch is never reached — which is why p3-cycle-in-subdir compiles. TestImportCycleDetection_TwoFiles in pkg/parser/import_cycle_test.go passes only because its fixture places both files in the same directory.

3. Documentation states the opposite

• docs/src/content/docs/reference/imports.md:312 claims "circular imports fail at compile time" — they do not when the participants live in a subdirectory.
• docs/src/content/docs/reference/imports.md:379 claims "Steps from imports run before steps defined in the main workflow, in import declaration order" — true only for a flat, single-level import list; nested imports have no declaration order in the main workflow.

Impact

Any shared workflow that both contributes steps: and imports a file providing prerequisite steps is correct standalone and silently wrong when imported, so shared workflows cannot be layered on other shared workflows. Correctness also depends on the order two unrelated-looking - uses: lines appear in the consumer. Both failures compile clean and surface at runtime in a generated lock file.

Proposed fix

Steps 1, 2 and 4–6 are blocking. Step 3 needs a maintainer decision; default to Option C if none is given.

1. Dependency resolution (pkg/parser/import_topological.go)

Resolve each nested import against the importing file's own directory rather than the top-level baseDir, and key the dependency graph on resolved full paths so calculateInDegree compares like with like. Preferred approach: record the edges the BFS already resolved (for example an edges map[string][]string on importBFSState, populated in enqueueNestedImportEntry) and pass them to topologicalSortImports, instead of re-deriving the graph by re-reading every file. Update the signature accordingly and drop buildImportDependencies / resolveNestedImportPaths if unused.

2. Field merging (pkg/parser/import_bfs.go, pkg/parser/import_field_extractor.go)

Split discovery from merging: the BFS resolves the graph, topologicalSortImports produces the order, then a second pass calls extractAllImportFields over the sorted order. Move the state.acc.extractAllImportFields(...) call out of handleStandardImportItem, and cache each file's content and parsed frontmatter on the queue item during discovery so the merge pass does not re-read from disk. Keep result.ImportedFiles = topologicalOrder.

3. Sibling precedence (decision required)

Scalar conflicts currently resolve first-seen-wins over the BFS order (pkg/workflow/checkout_manager.go:295, the entry.ref == "" guard), meaning a directly declared import outranks a transitively reached one. Under a dependencies-first order, preserving that meaning requires later overrides earlier, which flips which of two sibling imports wins.

• Option A — adopt later overrides earlier uniformly. Simplest to document; "an importing workflow can always override what it imports" falls out of the ordering, and "the main workflow takes precedence" stops being a special case. Breaking for sibling imports.
• Option B — same end state behind a compatibility flag defaulting to current behavior for one or two releases, warning when a sibling conflict would resolve differently. Breaking, deferred.
• Option C — topological order governs list-valued fields only; scalar precedence keeps its own resolution. Not breaking, at the cost of two orderings to document.

4. Tests

New pkg/parser/import_topological_test.go, table-driven per scratchpad/testing.md (require for setup, assert for values):

• A nested import in a subdirectory produces a dependency edge (regression for defect 2).
• Diamond: two files sharing one dependency emit the dependency first under both sibling declaration orders.
• Role invariance: A importing B yields the same relative step order whether A is the entry point or is imported.
• Siblings with no dependency relationship keep declaration order.

Extend pkg/parser/import_cycle_test.go with a cycle whose participants both live in a subdirectory and are reached through a third entry file (currently compiles successfully), plus a three-file cycle spanning two directories.

5. Documentation (docs/src/content/docs/reference/imports.md)

Replace the traversal description at line 312 with the invariant — a file is processed after everything it imports and before anything that imports it, ties broken by declaration order, cycles fail at compile time. Correct line 379 in "Importing Steps". Update the merge-semantics rows for steps:, pre-agent-steps: and post-steps:, and record whichever option step 3 selects.

6. Changeset

patch for Options B or C; major for Option A with migration guidance per scratchpad/breaking-cli-rules.md. Note in CHANGELOG.md that step order changes for multi-level imports and that previously-accepted subdirectory cycles now fail.

Notes for the implementing agent

Cycle errors must follow the error message style guide — [what's wrong]. [what's expected]. [example], for example: circular import detected: shared/sub-a.md → shared/sub-b.md → shared/sub-a.md. Imports must form a directed acyclic graph. Example: remove the 'uses: sub-a.md' entry from shared/sub-b.md. Use pkg/console for CLI output and pkg/logger (namespace parser:) for debug instrumentation. Run make agent-finish before completing.

Done when the five workflows above produce the "Expected" column, p3-cycle-in-subdir.md fails with ImportCycleError, and TestImportCycleDetection_TwoFiles still passes.

Environment

• Affected files: pkg/parser/import_topological.go, pkg/parser/import_bfs.go, pkg/parser/import_field_extractor.go
• Reproduced with: gh aw version v0.86.0

Suggested labels

bug, workflow, priority-medium