Fix: actions-lock.json created relative to CWD instead of repository root#14727
Fix: actions-lock.json created relative to CWD instead of repository root#14727
Conversation
Add WithGitRoot compiler option to specify git repository root Update getSharedActionResolver to use gitRoot if provided Update CLI to find and pass git root to compiler Add integration test to verify fix This ensures actions-lock.json is created at <repo-root>/.github/aw/ regardless of current working directory when compiling workflows Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot not backward compatible |
I understand the concern. The current fix only applies git root detection to the main Would you prefer:
Which approach would you like me to take? |
|
@copilot Move git root detection into NewCompiler() itself (automatic but adds overhead to every compiler creation) |
- Add findGitRoot() helper to pkg/workflow/git_helpers.go - Auto-detect git root in NewCompiler() initialization - Remove git root detection from CLI compile_compiler_setup.go - All compiler instances now automatically get git root detection - Ensures consistent behavior across all commands (compile, add, mcp, etc.) This makes the fix truly backward compatible while ensuring actions-lock.json is always created at repo root regardless of CWD. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done! Git root detection is now in Changes:
The fix is truly backward compatible - if git detection fails, it falls back to cwd (original behavior). |
There was a problem hiding this comment.
Pull request overview
Fixes incorrect placement of .github/aw/actions-lock.json when running gh aw compile from a subdirectory by basing the action cache path on the git repository root instead of the current working directory.
Changes:
- Add git-root detection helper in
pkg/workflowand store detectedgitRooton the compiler. - Update shared
ActionCacheinitialization to usegitRoot(fallback to CWD when unavailable). - Add an integration test intended to cover compiling from
.github/workflows/.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/workflow/git_helpers.go | Adds git-root detection helper used by the compiler. |
| pkg/workflow/compiler_types.go | Stores auto-detected gitRoot and uses it when creating ActionCache. |
| pkg/cli/compile_integration_test.go | Adds integration test for compiling from a subdirectory. |
| pkg/cli/compile_compiler_setup.go | Updates comment to reflect new git-root auto-detection behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Create a simple test workflow | ||
| // Note: actions-lock.json is only created when actions need to be pinned, | ||
| // so it may or may not exist. The test verifies it's NOT created in the wrong location. | ||
| testWorkflow := `--- |
There was a problem hiding this comment.
This test is non-deterministic: it explicitly notes that actions-lock.json “may or may not exist”, and then only asserts the wrong path doesn’t exist. If the compile run doesn’t ever write the action cache, the test will pass even if the original bug regresses. Make the test force creation/update of actions-lock.json (e.g., ensure a code path that marks the ActionCache dirty and saves it) so it fails when the cache is written relative to CWD.
| // Change to the .github/workflows subdirectory | ||
| if err := os.Chdir(setup.workflowsDir); err != nil { | ||
| t.Fatalf("Failed to change to workflows subdirectory: %v", err) | ||
| } | ||
|
|
||
| // Run the compile command from the subdirectory using a relative path | ||
| cmd := exec.Command(setup.binaryPath, "compile", "test-action.md") | ||
| output, err := cmd.CombinedOutput() | ||
| if err != nil { | ||
| t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output)) | ||
| } | ||
|
|
||
| // Change back to the temp directory root | ||
| if err := os.Chdir(setup.tempDir); err != nil { | ||
| t.Fatalf("Failed to change back to temp directory: %v", err) | ||
| } | ||
|
|
||
| // Verify actions-lock.json is created at the repository root (.github/aw/actions-lock.json) | ||
| // NOT at .github/workflows/.github/aw/actions-lock.json | ||
| expectedLockPath := filepath.Join(setup.tempDir, ".github", "aw", "actions-lock.json") | ||
| wrongLockPath := filepath.Join(setup.workflowsDir, ".github", "aw", "actions-lock.json") |
There was a problem hiding this comment.
Using os.Chdir in a test changes the process-wide working directory and can create hard-to-debug coupling if other tests are added later (especially with t.Parallel). Prefer setting cmd.Dir = setup.workflowsDir for the subprocess to simulate running from a subdirectory, and keep the parent test process CWD unchanged.
| // Change to the .github/workflows subdirectory | |
| if err := os.Chdir(setup.workflowsDir); err != nil { | |
| t.Fatalf("Failed to change to workflows subdirectory: %v", err) | |
| } | |
| // Run the compile command from the subdirectory using a relative path | |
| cmd := exec.Command(setup.binaryPath, "compile", "test-action.md") | |
| output, err := cmd.CombinedOutput() | |
| if err != nil { | |
| t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output)) | |
| } | |
| // Change back to the temp directory root | |
| if err := os.Chdir(setup.tempDir); err != nil { | |
| t.Fatalf("Failed to change back to temp directory: %v", err) | |
| } | |
| // Verify actions-lock.json is created at the repository root (.github/aw/actions-lock.json) | |
| // NOT at .github/workflows/.github/aw/actions-lock.json | |
| expectedLockPath := filepath.Join(setup.tempDir, ".github", "aw", "actions-lock.json") | |
| wrongLockPath := filepath.Join(setup.workflowsDir, ".github", "aw", "actions-lock.json") | |
| // Run the compile command as if from the .github/workflows subdirectory using a relative path | |
| cmd := exec.Command(setup.binaryPath, "compile", "test-action.md") | |
| cmd.Dir = setup.workflowsDir | |
| output, err := cmd.CombinedOutput() | |
| if err != nil { | |
| t.Fatalf("CLI compile command failed: %v\nOutput: %s", err, string(output)) | |
| } | |
| // Verify actions-lock.json is created at the repository root (.github/aw/actions-lock.json) | |
| // NOT at .github/workflows/.github/aw/actions-lock.json | |
| expectedLockPath := filepath.Join(setup.tempDir, ".github", "aw", "actions-lock.json") | |
| wrongLockPath := filepath.Join(setup.workflowsDir, ".github", "aw", "actions-lock.json") | |
| // NOT at .github/workflows/.github/aw/actions-lock.json | |
| expectedLockPath := filepath.Join(setup.tempDir, ".github", "aw", "actions-lock.json") | |
| wrongLockPath := filepath.Join(setup.workflowsDir, ".github", "aw", "actions-lock.json") |
| // Auto-detect git repository root for action cache path resolution | ||
| // This ensures actions-lock.json is created at repo root regardless of CWD | ||
| gitRoot := findGitRoot() | ||
|
|
There was a problem hiding this comment.
NewCompiler() now spawns a git rev-parse --show-toplevel process for every compiler instantiation. This compiler is constructed in a large number of unit tests and likely in multiple code paths, so this eager detection can noticeably slow test/CLI execution. Consider making git-root detection lazy (e.g., perform it once inside getSharedActionResolver() when the action cache is first needed) and/or caching the computed root with sync.Once so repeated NewCompiler() calls don’t re-run git.
| // findGitRoot attempts to find the git repository root directory. | ||
| // Returns empty string if not in a git repository or if git command fails. | ||
| // This function is safe to call from any context and won't cause errors if git is not available. | ||
| func findGitRoot() string { | ||
| gitHelpersLog.Print("Attempting to find git root directory") | ||
| cmd := exec.Command("git", "rev-parse", "--show-toplevel") | ||
| output, err := cmd.Output() | ||
| if err != nil { | ||
| gitHelpersLog.Printf("Could not find git root (not a git repo or git not available): %v", err) | ||
| return "" | ||
| } | ||
| gitRoot := strings.TrimSpace(string(output)) | ||
| gitHelpersLog.Printf("Found git root: %s", gitRoot) | ||
| return gitRoot |
There was a problem hiding this comment.
This introduces a second git-root helper that duplicates the existing implementation in pkg/cli/git.go (findGitRoot / findGitRootForPath). Having parallel implementations across packages increases the chance they drift (flags, error handling, -C support). If possible, centralize git-root detection in one shared helper (or export one of them) and reuse it from here.
|
@pelikhan hey Peli, did you have a chance to look at Copilot's review comments in this PR before merging? |
Running
gh aw compilefrom a subdirectory (e.g.,.github/workflows/) createsactions-lock.jsonrelative to CWD, resulting in duplicate nested paths like.github/workflows/.github/aw/actions-lock.json.Root cause:
ActionCacheinitialization ingetSharedActionResolver()usesos.Getwd()instead of git repository root.Changes
findGitRoot()helper function inpkg/workflow/git_helpers.goto safely detect git repository rootNewCompiler()initialization for all compiler instancesgitRootingetSharedActionResolver(), fall back to CWD if unsetTestCompileFromSubdirectoryCreatesActionsLockAtRootintegration testImplementation
Git root detection now happens automatically in
NewCompiler(), ensuring consistent behavior across all commands (gh aw compile,gh aw add,gh aw mcp inspect, etc.):Benefits
Backwards compatible: git root detection failure falls back to CWD behavior.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.