fix: body-level {{#import shared/X.md}} rewritten with wrong cross-repo path in gh aw add by Copilot · Pull Request #23817 · github/gh-aw

Copilot · 2026-04-01T04:11:56Z

gh aw add had two bugs when rewriting body-level {{#import shared/X.md}} directives: it resolved the path from the repo root instead of the workflow file's directory (.github/workflows/), and it unconditionally rewrote imports even when the consuming repo already had the file locally.

Changes

processIncludesWithWorkflowSpec — path resolution fix: Replace bare filePath with resolveImportPath(filePath, workflow.WorkflowPath) when building the cross-repo ref, matching what processIncludesInContent (used by update) already does.
```
# before
{{#import github/source-repo/shared/config.md@abc123}}   ❌ (repo-root relative)

# after
{{#import github/source-repo/.github/workflows/shared/config.md@abc123}}  ✅
```
processIncludesWithWorkflowSpec — local preservation: Add a localWorkflowDir parameter (same pattern as processIncludesInContent). Imports whose targets exist under localWorkflowDir are left unchanged rather than pinned to the source repo.
add_command.go: Pass githubWorkflowsDir as localWorkflowDir so consuming repos with their own shared/ files don't get those imports overwritten on every gh aw add.
Tests: Three targeted regression tests (PathResolution, PreservesLocalIncludes, RewritesBodyWhenLocalMissing) plus two comprehensive table-driven tests covering every import path variant:
- TestProcessIncludesWithWorkflowSpec_PathTypes (15 sub-tests): simple relative, optional flag, deep nested, absolute (/tools/…), path+section, already a workflowspec (pass-through), section-only, parent dir (../), current dir (./), legacy @include syntax, three-segment path, and plain filename.
- TestProcessImportsWithWorkflowSpec_PathTypes (8 sub-tests): same path categories for the frontmatter imports: field processor.

- Use resolveImportPath in processIncludesWithWorkflowSpec so body-level {{#import shared/X.md}} paths are resolved relative to the workflow file's directory (.github/workflows/) rather than the repo root. Before: owner/repo/shared/config.md@SHA After: owner/repo/.github/workflows/shared/config.md@SHA - Add localWorkflowDir parameter to processIncludesWithWorkflowSpec so imports whose target files already exist locally are preserved as-is, matching the behaviour already in processIncludesInContent (update cmd). - Update add_command.go to pass githubWorkflowsDir as localWorkflowDir so consuming repos with their own local shared/ files are not forced onto cross-repo references after every gh aw add. - Add 3 new tests: PathResolution, PreservesLocalIncludes, RewritesBodyWhenLocalMissing. Closes #(issue) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/106643c3-a705-4a25-9619-35c55a98d102 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

pelikhan · 2026-04-01T04:39:54Z

@copilot review and add tests to try various types of import paths

@include

Add two table-driven tests covering every meaningful import-path variant: TestProcessIncludesWithWorkflowSpec_PathTypes (15 sub-tests): - simple relative (shared/config.md) - optional flag preserved ({{#import?}}) - deep nested path - absolute path (/tools/config.md) - path with section reference (#Section) - optional + section - already a workflowspec (passed through) - already workflowspec with section - section-only reference (#SectionName) - parent dir (../shared/config.md) - current dir (./config.md) - legacy @include syntax - legacy optional @include? syntax - three-segment path (no @ sign, treated as local) - plain filename TestProcessImportsWithWorkflowSpec_PathTypes (8 sub-tests): - same path categories for the frontmatter imports: field Agent-Logs-Url: https://github.com/github/gh-aw/sessions/47f3fec7-c300-4ecb-9103-6c1d1a96903a Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot · 2026-04-01T04:48:08Z

@copilot review and add tests to try various types of import paths

Added two table-driven tests in commit 8c43068:

TestProcessIncludesWithWorkflowSpec_PathTypes (15 sub-tests for body-level {{#import}}):

simple relative (shared/config.md)
optional flag ({{#import?}})
deep nested (shared/mcp/deep/file.md)
absolute (/tools/config.md → repo-root relative)
path with section (shared/config.md#Introduction)
optional + section
already a workflowspec (pass-through)
already workflowspec with section (pass-through)
section-only (#SectionName — preserved as-is)
parent dir traversal (../shared/config.md → .github/shared/config.md)
current dir prefix (./config.md)
legacy @include syntax (output uses {{#import}})
legacy optional @include?
three-segment path with no @ (treated as local, not a workflowspec)
plain filename

TestProcessImportsWithWorkflowSpec_PathTypes (8 sub-tests for frontmatter imports:): same path categories applied to the frontmatter field processor.

Copilot

Pull request overview

Fixes gh aw add rewriting of body-level {{#import ...}} directives by resolving paths relative to the workflow file location and preserving local imports when the consuming repo already has the referenced files.

Changes:

Update processIncludesWithWorkflowSpec to (a) skip already-workflowspec paths, (b) resolve include paths relative to workflow.WorkflowPath, and (c) preserve local includes when localWorkflowDir contains the target.
Wire localWorkflowDir through add_command.go so gh aw add preserves locally-present shared files.
Add regression + table-driven tests for body includes and frontmatter imports: path variants; add a changeset entry.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
`pkg/cli/imports.go`	Fixes path resolution for body imports, adds local-preservation behavior, and avoids rewriting already-workflowspec imports.
`pkg/cli/imports_test.go`	Adds regression and table-driven coverage for import path variants and preservation behavior.
`pkg/cli/add_command.go`	Passes `.github/workflows` as `localWorkflowDir` to preserve local body imports during `gh aw add`.
`.changeset/patch-fix-body-import-path-resolution.md`	Documents the patch-level behavior change for release notes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T05:05:17Z

pkg/cli/imports.go

 			// Mark as visited
 			visited[filePath] = true



Because visited[filePath] = true is set before emitting the rewritten directive, subsequent occurrences of the same include will hit the earlier if visited[filePath] { continue } and be omitted from the output entirely. This changes workflow content by dropping repeated include lines. Consider changing the cycle-detection behavior to still write the original line (or rewritten directive) while only skipping re-enqueueing/nested traversal for already-visited paths.

Copilot · 2026-04-01T05:05:18Z

pkg/cli/imports_test.go

+	if err := os.MkdirAll(tmpDir+"/shared", 0755); err != nil {
+		t.Fatalf("Failed to create shared dir: %v", err)
+	}
+	if err := os.WriteFile(tmpDir+"/shared/config.md", []byte("# Local config"), 0644); err != nil {
+		t.Fatalf("Failed to create local config file: %v", err)
+	}


In these tests, building paths via string concatenation (e.g. tmpDir+"/shared") can be brittle on Windows. Prefer filepath.Join(tmpDir, "shared") / filepath.Join(tmpDir, "shared", "config.md") and add the path/filepath import to keep the tests OS-agnostic.

pelikhan · 2026-04-01T05:09:44Z

@copilot review comments

- Fix cycle-detection in processIncludesWithWorkflowSpec: mark visited and skip re-enqueueing only on second occurrence, but still write the rewritten directive so duplicate {{#import}} lines are preserved in the output - Add TestProcessIncludesWithWorkflowSpec_DuplicateInclude to cover the fix - Replace tmpDir+"/..." string concatenation in tests with filepath.Join(...) for OS-agnosticism (addresses code review feedback) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f99a3393-1a0c-42bb-acb7-68920cee5eb6 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot · 2026-04-01T05:17:17Z

@copilot review comments

Both review comments addressed in commit e40ad58:

Cycle-detection dropping duplicate imports (imports.go:249-251): Moved visited[filePath] = true and the queue append inside a !visited guard so they only fire on the first occurrence. The rewritten directive is now written unconditionally, preserving every duplicate {{#import}} line in the output. Added TestProcessIncludesWithWorkflowSpec_DuplicateInclude to cover this.
String concatenation in tests (imports_test.go:315-320): Replaced all tmpDir+"/"... instances with filepath.Join(...) and added the path/filepath import.

github-actions · 2026-04-01T05:43:57Z

Hey @Copilot 👋 — great work on this bug fix! Squashing both the path-resolution issue and the local-preservation oversight in processIncludesWithWorkflowSpec in one clean sweep is exactly the kind of targeted fix this codebase benefits from.

Contribution Guidelines Check — ✅ All clear

Check	Result
On-topic	✅ Yes — focused bug fix in core `gh aw add` CLI path
Follows process	✅ Yes — Copilot agent PR with core team member `@pelikhan` as assignee/reviewer, matching the agentic-development model
Focused	✅ Yes — both changes address closely related bugs in the same function
New dependencies	✅ No
Tests	✅ Yes — 3 regression tests + 2 comprehensive table-driven suites (23 sub-tests total, +410 lines in `imports_test.go`)
Description	✅ Yes — thorough write-up with before/after examples
Diff size	484 lines (454 additions, 30 deletions across 4 files)
Changeset entry	✅ Present

This PR looks well-structured and ready for maintainer review. The test coverage across all import path variants (simple relative, optional flag, deep nested, absolute /tools/…, legacy @include syntax, ../, ./, etc.) is particularly thorough. 🎉

Generated by Contribution Check · ◷

- Replace fileToProcess struct with []string queue in processIncludesWithWorkflowSpec - Extract splitImportPath helper using strings.Cut, eliminating 3 duplicate section-split blocks - Extract writeImportDirective helper, eliminating 2 duplicate directive-write blocks - Merge two adjacent if opts.Verbose blocks in addWorkflowWithTracking No behavioral changes; all tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Initial plan

8772685

Copilot AI assigned Copilot and pelikhan Apr 1, 2026

Copilot started work on behalf of pelikhan April 1, 2026 04:12 View session

Copilot AI linked an issue Apr 1, 2026 that may be closed by this pull request

gh aw add rewrites {{#import shared/X.md}} with incorrect cross-repo path (resolves from repo root instead of .github/workflows/) #23212

Closed

Copilot AI changed the title ~~[WIP] Fix incorrect cross-repo path rewrites for imports~~ fix: body-level {{#import shared/X.md}} rewritten with wrong cross-repo path in gh aw add Apr 1, 2026

Copilot AI requested a review from pelikhan April 1, 2026 04:38

Copilot finished work on behalf of pelikhan April 1, 2026 04:38

Copilot started work on behalf of pelikhan April 1, 2026 04:40 View session

Copilot finished work on behalf of pelikhan April 1, 2026 04:49

pelikhan marked this pull request as ready for review April 1, 2026 05:01

Copilot AI review requested due to automatic review settings April 1, 2026 05:01

Copilot started reviewing on behalf of pelikhan April 1, 2026 05:01 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Copilot started work on behalf of pelikhan April 1, 2026 05:09 View session

Copilot finished work on behalf of pelikhan April 1, 2026 05:18

github-actions bot mentioned this pull request Apr 1, 2026

Smoke Test: Claude - 23833156202 #23834

Closed

pelikhan merged commit f5a2c2a into main Apr 1, 2026
2 checks passed

pelikhan deleted the copilot/fix-import-rewrites-bug branch April 1, 2026 05:37

github-actions bot added the lgtm label Apr 1, 2026

github-actions bot mentioned this pull request Apr 1, 2026

[Contribution Check Report] Contribution Check — 2026-04-01 #23805

Closed

github-actions bot mentioned this pull request Apr 1, 2026

[code-simplifier] refactor: simplify imports.go and add_command.go from PR #23817 #23844

Merged

pelikhan pushed a commit that referenced this pull request Apr 1, 2026

refactor: simplify imports.go and add_command.go from PR #23817 (#23844)

6e097a6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: body-level {{#import shared/X.md}} rewritten with wrong cross-repo path in gh aw add#23817

fix: body-level {{#import shared/X.md}} rewritten with wrong cross-repo path in gh aw add#23817
pelikhan merged 4 commits intomainfrom
copilot/fix-import-rewrites-bug

Copilot AI commented Apr 1, 2026 •

edited

Loading

Uh oh!

pelikhan commented Apr 1, 2026

Uh oh!

Copilot AI commented Apr 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

pelikhan commented Apr 1, 2026

Uh oh!

Copilot AI commented Apr 1, 2026

Uh oh!

Uh oh!

github-actions bot commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

pelikhan commented Apr 1, 2026

Uh oh!

Copilot AI commented Apr 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

pelikhan commented Apr 1, 2026

Uh oh!

Copilot AI commented Apr 1, 2026

Uh oh!

Uh oh!

github-actions bot commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Apr 1, 2026 •

edited

Loading