Skip to content

Commit f3d24fe

Browse files
CopilotCopilot
authored
fix: preserve local relative imports during gh aw update (#23809)
1 parent 6187853 commit f3d24fe

6 files changed

Lines changed: 415 additions & 23 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2570,6 +2570,170 @@ jobs:
25702570
echo "- Success count: ${{ steps.add-workflows.outputs.success_count }}"
25712571
echo "- Failure count: ${{ steps.add-workflows.outputs.failure_count }}"
25722572
2573+
integration-update:
2574+
name: Integration Update - Preserve Local Imports
2575+
runs-on: ubuntu-latest
2576+
timeout-minutes: 15
2577+
permissions:
2578+
contents: read
2579+
concurrency:
2580+
group: ci-${{ github.ref }}-integration-update
2581+
cancel-in-progress: true
2582+
steps:
2583+
- name: Checkout code
2584+
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
2585+
2586+
- name: Set up Go
2587+
id: setup-go
2588+
uses: actions/setup-go@4dc6199c7b1a012772edbd06daecab0f50c9053c # v6
2589+
with:
2590+
go-version-file: go.mod
2591+
cache: true
2592+
2593+
- name: Report Go cache status
2594+
run: |
2595+
if [ "${{ steps.setup-go.outputs.cache-hit }}" == "true" ]; then
2596+
echo "✅ Go cache hit" >> $GITHUB_STEP_SUMMARY
2597+
else
2598+
echo "⚠️ Go cache miss" >> $GITHUB_STEP_SUMMARY
2599+
fi
2600+
2601+
- name: Download dependencies
2602+
run: go mod download
2603+
2604+
- name: Verify dependencies
2605+
run: go mod verify
2606+
2607+
- name: Build gh-aw binary
2608+
run: make build
2609+
2610+
- name: Set up isolated test workspace
2611+
run: |
2612+
mkdir -p /tmp/test-update-workspace/.github/workflows
2613+
cd /tmp/test-update-workspace
2614+
git init -q
2615+
git config user.email "test@example.com"
2616+
git config user.name "Test"
2617+
echo "✅ Test workspace initialised at /tmp/test-update-workspace"
2618+
2619+
- name: Add a workflow from githubnext/agentics
2620+
id: add-workflow
2621+
env:
2622+
GH_TOKEN: ${{ github.token }}
2623+
run: |
2624+
cd /tmp/test-update-workspace
2625+
2626+
echo "## Integration Update Test: Preserve Local Imports" >> $GITHUB_STEP_SUMMARY
2627+
echo "" >> $GITHUB_STEP_SUMMARY
2628+
2629+
# Add daily-team-status which uses shared imports in githubnext/agentics
2630+
if /home/runner/work/gh-aw/gh-aw/gh-aw add githubnext/agentics/workflows/daily-team-status.md --force 2>&1; then
2631+
echo "✅ Added workflow successfully" >> $GITHUB_STEP_SUMMARY
2632+
else
2633+
echo "❌ Failed to add workflow" >> $GITHUB_STEP_SUMMARY
2634+
exit 1
2635+
fi
2636+
2637+
WORKFLOW=".github/workflows/daily-team-status.md"
2638+
if [ ! -f "$WORKFLOW" ]; then
2639+
echo "❌ Workflow file not found after add"
2640+
exit 1
2641+
fi
2642+
2643+
# Check if the workflow has relative imports
2644+
if grep -q "^imports:" "$WORKFLOW"; then
2645+
echo "✅ Workflow has an imports field" >> $GITHUB_STEP_SUMMARY
2646+
# Collect relative import paths (those without "@" — not yet cross-repo refs).
2647+
# Python is used for robust YAML-aware parsing instead of fragile AWK patterns.
2648+
RELATIVE_IMPORTS=$(python3 - "$WORKFLOW" <<'PYEOF'
2649+
import sys, re
2650+
2651+
content = open(sys.argv[1]).read()
2652+
# Find the imports: block (list items under the key, indented with 2+ spaces)
2653+
m = re.search(r'^imports:\n((?:[ \t]+-[ \t]+.+\n)+)', content, re.MULTILINE)
2654+
if m:
2655+
for line in m.group(1).splitlines():
2656+
val = line.strip().lstrip('- ').strip()
2657+
if val and '@' not in val:
2658+
print(val)
2659+
PYEOF
2660+
)
2661+
if [ -n "$RELATIVE_IMPORTS" ]; then
2662+
echo "has_relative_imports=true" >> $GITHUB_OUTPUT
2663+
echo "$RELATIVE_IMPORTS" > /tmp/relative-imports.txt
2664+
echo "Relative import paths: $RELATIVE_IMPORTS" >> $GITHUB_STEP_SUMMARY
2665+
else
2666+
echo "has_relative_imports=false" >> $GITHUB_OUTPUT
2667+
echo "⚠️ All imports already use cross-repo refs; skipping local-file preservation check" >> $GITHUB_STEP_SUMMARY
2668+
fi
2669+
else
2670+
echo "has_relative_imports=false" >> $GITHUB_OUTPUT
2671+
echo "⚠️ No imports field found in added workflow; skipping preservation check" >> $GITHUB_STEP_SUMMARY
2672+
fi
2673+
2674+
- name: Create local copies of the shared import files
2675+
if: steps.add-workflow.outputs.has_relative_imports == 'true'
2676+
run: |
2677+
cd /tmp/test-update-workspace
2678+
echo "Creating local shared files..." >> $GITHUB_STEP_SUMMARY
2679+
while IFS= read -r import_path; do
2680+
local_file=".github/workflows/$import_path"
2681+
mkdir -p "$(dirname "$local_file")"
2682+
echo "# Local shared file (simulating user-copied content)" > "$local_file"
2683+
echo "✅ Created: $local_file" >> $GITHUB_STEP_SUMMARY
2684+
done < /tmp/relative-imports.txt
2685+
2686+
- name: Run gh aw update --force
2687+
if: steps.add-workflow.outputs.has_relative_imports == 'true'
2688+
env:
2689+
GH_TOKEN: ${{ github.token }}
2690+
run: |
2691+
cd /tmp/test-update-workspace
2692+
echo "Running gh aw update --force --no-compile daily-team-status ..."
2693+
/home/runner/work/gh-aw/gh-aw/gh-aw update daily-team-status --force --no-compile 2>&1
2694+
echo "✅ Update completed" >> $GITHUB_STEP_SUMMARY
2695+
2696+
- name: Verify local import paths are preserved after update
2697+
if: steps.add-workflow.outputs.has_relative_imports == 'true'
2698+
run: |
2699+
cd /tmp/test-update-workspace
2700+
WORKFLOW=".github/workflows/daily-team-status.md"
2701+
2702+
echo "" >> $GITHUB_STEP_SUMMARY
2703+
echo "### Import paths after \`gh aw update\`" >> $GITHUB_STEP_SUMMARY
2704+
echo '```' >> $GITHUB_STEP_SUMMARY
2705+
grep -A10 "^imports:" "$WORKFLOW" >> $GITHUB_STEP_SUMMARY || true
2706+
echo '```' >> $GITHUB_STEP_SUMMARY
2707+
2708+
FAILED=false
2709+
while IFS= read -r import_path; do
2710+
local_file=".github/workflows/$import_path"
2711+
2712+
# Confirm the local file still exists (sanity check)
2713+
if [ ! -f "$local_file" ]; then
2714+
echo "⚠️ Local file not found (was it deleted?): $local_file"
2715+
continue
2716+
fi
2717+
2718+
# The import entry in the workflow file must still be the relative path,
2719+
# NOT a cross-repo reference (which would contain "@" and "owner/repo/").
2720+
if grep -qF "- $import_path" "$WORKFLOW"; then
2721+
echo "✅ Relative import preserved: $import_path" >> $GITHUB_STEP_SUMMARY
2722+
else
2723+
echo "❌ FAIL: '$import_path' was rewritten even though the local file exists" >> $GITHUB_STEP_SUMMARY
2724+
echo "❌ Current imports section:"
2725+
grep -A10 "^imports:" "$WORKFLOW" || true
2726+
FAILED=true
2727+
fi
2728+
done < /tmp/relative-imports.txt
2729+
2730+
if [ "$FAILED" = "true" ]; then
2731+
echo "❌ **FAILURE**: gh aw update rewrote local relative imports to cross-repo paths" >> $GITHUB_STEP_SUMMARY
2732+
exit 1
2733+
fi
2734+
2735+
echo "✅ **SUCCESS**: All local relative import paths were preserved by gh aw update" >> $GITHUB_STEP_SUMMARY
2736+
25732737
integration-unauthenticated-add:
25742738
name: Integration Unauthenticated Add (Public Repo)
25752739
runs-on: ubuntu-latest

‎pkg/cli/imports.go‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ func resolveImportPath(importPath string, workflowPath string) string {
5858
// processImportsWithWorkflowSpec processes imports field in frontmatter and replaces local file references
5959
// with workflowspec format (owner/repo/path@sha) for all imports found.
6060
// Handles both array form and object form (with 'aw' subfield) of the imports field.
61-
func processImportsWithWorkflowSpec(content string, workflow *WorkflowSpec, commitSHA string, verbose bool) (string, error) {
62-
importsLog.Printf("Processing imports with workflowspec: repo=%s, sha=%s", workflow.RepoSlug, commitSHA)
61+
// If localWorkflowDir is non-empty, any import path whose file exists under that directory is
62+
// left as a local relative path rather than being rewritten to a cross-repo reference.
63+
func processImportsWithWorkflowSpec(content string, workflow *WorkflowSpec, commitSHA string, localWorkflowDir string, verbose bool) (string, error) {
64+
importsLog.Printf("Processing imports with workflowspec: repo=%s, sha=%s, localWorkflowDir=%s", workflow.RepoSlug, commitSHA, localWorkflowDir)
6365
if verbose {
6466
fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Processing imports field to replace with workflowspec"))
6567
}
@@ -79,6 +81,10 @@ func processImportsWithWorkflowSpec(content string, workflow *WorkflowSpec, comm
7981
}
8082

8183
// processImportPaths converts a list of raw import paths to workflowspec format.
84+
// Paths that already use the workflowspec format (contain "@") are left unchanged.
85+
// When localWorkflowDir is set, relative paths whose files exist locally are also
86+
// preserved as-is so that consumers who have copied shared files into their own repo
87+
// are not forced onto cross-repo references after every `gh aw update`.
8288
processImportPaths := func(imports []string) []string {
8389
processed := make([]string, 0, len(imports))
8490
for _, importPath := range imports {
@@ -87,6 +93,16 @@ func processImportsWithWorkflowSpec(content string, workflow *WorkflowSpec, comm
8793
processed = append(processed, importPath)
8894
continue
8995
}
96+
// Preserve relative paths whose files exist in the local workflow directory.
97+
// Absolute paths (starting with "/") are not checked — they are always resolved
98+
// relative to the repo root and cannot be reliably tested here.
99+
if localWorkflowDir != "" && !strings.HasPrefix(importPath, "/") {
100+
if isLocalFileForUpdate(localWorkflowDir, importPath) {
101+
importsLog.Printf("Import path exists locally, preserving relative path: %s", importPath)
102+
processed = append(processed, importPath)
103+
continue
104+
}
105+
}
90106
resolvedPath := resolveImportPath(importPath, workflow.WorkflowPath)
91107
importsLog.Printf("Resolved import path: %s -> %s (workflow: %s)", importPath, resolvedPath, workflow.WorkflowPath)
92108
workflowSpec := buildWorkflowSpecRef(workflow.RepoSlug, resolvedPath, commitSHA, workflow.Version)
@@ -317,10 +333,12 @@ func processIncludesWithWorkflowSpec(content string, workflow *WorkflowSpec, com
317333
}
318334

319335
// processIncludesInContent processes @include directives in workflow content for update command
320-
// and also processes imports field in frontmatter
321-
func processIncludesInContent(content string, workflow *WorkflowSpec, commitSHA string, verbose bool) (string, error) {
336+
// and also processes imports field in frontmatter.
337+
// If localWorkflowDir is non-empty, any relative import/include path whose file exists under
338+
// that directory is left as-is rather than being rewritten to a cross-repo reference.
339+
func processIncludesInContent(content string, workflow *WorkflowSpec, commitSHA string, localWorkflowDir string, verbose bool) (string, error) {
322340
// First process imports field in frontmatter
323-
processedImportsContent, err := processImportsWithWorkflowSpec(content, workflow, commitSHA, verbose)
341+
processedImportsContent, err := processImportsWithWorkflowSpec(content, workflow, commitSHA, localWorkflowDir, verbose)
324342
if err != nil {
325343
if verbose {
326344
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to process imports: %v", err)))
@@ -367,6 +385,15 @@ func processIncludesInContent(content string, workflow *WorkflowSpec, commitSHA
367385
continue
368386
}
369387

388+
// Preserve relative @include paths whose files exist in the local workflow directory.
389+
if localWorkflowDir != "" && !strings.HasPrefix(filePath, "/") {
390+
if isLocalFileForUpdate(localWorkflowDir, filePath) {
391+
importsLog.Printf("Include path exists locally, preserving: %s", filePath)
392+
result.WriteString(line + "\n")
393+
continue
394+
}
395+
}
396+
370397
// Resolve the file path relative to the workflow file's directory
371398
resolvedPath := resolveImportPath(filePath, workflow.WorkflowPath)
372399

@@ -393,7 +420,28 @@ func processIncludesInContent(content string, workflow *WorkflowSpec, commitSHA
393420
return result.String(), scanner.Err()
394421
}
395422

396-
// isWorkflowSpecFormat checks if a path already looks like a workflowspec
423+
// isLocalFileForUpdate returns true when importPath resolves to an existing file
424+
// within localWorkflowDir. The resolved absolute path must stay inside localWorkflowDir
425+
// to guard against path traversal (e.g. "../../etc/passwd" in import paths).
426+
// importPath must be a relative path — callers must not pass absolute paths here.
427+
func isLocalFileForUpdate(localWorkflowDir, importPath string) bool {
428+
if localWorkflowDir == "" || importPath == "" {
429+
return false
430+
}
431+
localPath := filepath.Join(localWorkflowDir, importPath)
432+
absDir, err1 := filepath.Abs(localWorkflowDir)
433+
absPath, err2 := filepath.Abs(localPath)
434+
if err1 != nil || err2 != nil {
435+
return false
436+
}
437+
// Reject traversal attempts: the resolved path must be a child of localWorkflowDir
438+
if !strings.HasPrefix(absPath, absDir+string(filepath.Separator)) {
439+
return false
440+
}
441+
_, statErr := os.Stat(localPath)
442+
return statErr == nil
443+
}
444+
397445
// A workflowspec is identified by having an @ version indicator (e.g., owner/repo/path@sha)
398446
// Simple paths like "shared/mcp/file.md" are NOT workflowspecs and should be processed
399447
func isWorkflowSpecFormat(path string) bool {

0 commit comments

Comments
 (0)