Fix/local actions outside workspace#6087
Open
Raj-glitch-max wants to merge 2 commits intonektos:masterfrom
Open
Fix/local actions outside workspace#6087Raj-glitch-max wants to merge 2 commits intonektos:masterfrom
Raj-glitch-max wants to merge 2 commits intonektos:masterfrom
Conversation
added 2 commits
May 1, 2026 17:26
…ctions When a remote action declares runs.using: docker with a relative runs.image (e.g. image: Dockerfile), the build context must be resolved relative to the directory containing action.yml, not from the repository root. Previously readActionImpl dropped the actionPath after reading metadata, so execAsDocker always used the uses: subpath to resolve runs.image. If action.yml lives in a subdirectory but uses: has no explicit path component, the Dockerfile was resolved from the wrong location. Changes: - Add ActionPath string (yaml:"-") to model.Action to carry the resolved subpath through the runner without affecting YAML parsing - Set action.ActionPath in all return paths of readActionImpl - When action.yml is not found at the repo root, scan immediate subdirectories of actionDir/actionPath and read from the unique match; return an error if zero or multiple matches are found - Use action.ActionPath as dockerSubpath at all three execAsDocker call sites when running remote actions (main, pre, post) Local actions and the ActionCache tar-stream path are unaffected. Added TestActionReaderDiscoverSubdir to cover the new discovery path. Fixes nektos#739
Fixes nektos#2107 getContainerActionPaths() was using strings.TrimPrefix to compute container paths for local actions, which broke for paths outside $GITHUB_WORKSPACE. Now uses ToContainerPath(actionDir) directly. Also fixes getOsSafeRelativePath() to use filepath.Rel() with filepath.Base() fallback instead of TrimPrefix. Implements the approach from nektos#2108 by @jenseng, rebased onto current master with updated tests.
There was a problem hiding this comment.
Pull request overview
This PR aims to fix resolution of local uses: actions that live outside GITHUB_WORKSPACE, which is part of the runner’s action path mapping logic. It updates how host paths are translated into container paths and adds regression coverage around that edge case.
Changes:
- Replace manual workspace-prefix trimming with relative-path/container-path handling in
pkg/runner/action.go. - Add/update unit tests around action path handling and introduce an integration workflow covering
./../action-outside-workspace. - Extend the action model with runtime-only
ActionPathmetadata to carry resolved action locations.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/runner/testdata/local-action-outside-workspace/push.yml |
Adds an integration workflow reproducing a local action located outside the workspace. |
pkg/runner/runner_test.go |
Registers the new integration scenario in the runner test matrix. |
pkg/runner/action_test.go |
Updates unit expectations and adds tests around discovered subdirectories and path propagation. |
pkg/runner/action.go |
Changes action loading and container path resolution logic for local/remote actions. |
pkg/model/action.go |
Adds a non-YAML runtime field to track resolved action paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+158
to
+168
| if discoveredFile != "" { | ||
| file, openErr := os.Open(discoveredFile) | ||
| if openErr != nil { | ||
| return nil, fmt.Errorf("%s failed to open discovered action file %q (action path %q, base dir %q): %w", step.String(), discoveredFile, actionPath, baseDir, openErr) | ||
| } | ||
| defer file.Close() | ||
| action, readErr := model.ReadAction(file) | ||
| if readErr != nil { | ||
| return nil, fmt.Errorf("%s failed to read discovered action file %q (action path %q, base dir %q): %w", step.String(), discoveredFile, actionPath, baseDir, readErr) | ||
| } | ||
| action.ActionPath = path.Join(actionPath, discoveredSubdir) |
Comment on lines
+150
to
+152
| if _, err := os.Stat(actionYaml); err == nil { | ||
| if discoveredFile != "" { | ||
| return nil, fmt.Errorf("ambiguous action: found action.yml in multiple subdirectories: %q and %q (in %q)", discoveredFile, actionYaml, baseDir) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2107
Root cause
getContainerActionPaths()inpkg/runner/action.gowas computingthe container path using
strings.TrimPrefixofactionDiragainstrc.Config.Workdir, then joining manually:When the action lives outside the workspace (e.g.
./../action-outside-workspace),TrimPrefixreturns an empty or wrong string — so the container pathresolves incorrectly, causing
file does not exist.Fix
strings.TrimPrefixwithfilepath.Rel()(withfilepath.Base()fallback)rc.JobContainer.ToContainerPath(actionDir)directly instead ofmanually joining from the workdir
Testing
action_test.golocal-action-outside-workspacewhich reproduces the exact scenario from the issue
Credit: @jenseng for the original analysis in #2108 — this rebases
that approach cleanly onto current master.