Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/model/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ type Action struct {
Inputs map[string]Input `yaml:"inputs"`
Outputs map[string]Output `yaml:"outputs"`
Runs ActionRuns `yaml:"runs"`
ActionPath string `yaml:"-"`
Branding struct {
Color string `yaml:"color"`
Icon string `yaml:"icon"`
Expand Down
84 changes: 81 additions & 3 deletions pkg/runner/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ func readActionImpl(ctx context.Context, step *model.Step, actionDir string, act
Using: "docker",
Image: "Dockerfile",
},
ActionPath: actionPath,
}
logger.Debugf("Using synthetic action %v for Dockerfile", action)
return action, nil
Expand Down Expand Up @@ -99,6 +100,7 @@ func readActionImpl(ctx context.Context, step *model.Step, actionDir string, act
Using: "node12",
Main: "trampoline.js",
},
ActionPath: actionPath,
}
logger.Debugf("Using synthetic action %v", action)
return action, nil
Expand All @@ -107,11 +109,75 @@ func readActionImpl(ctx context.Context, step *model.Step, actionDir string, act
}
}
if allErrors != nil {
baseDir := filepath.Join(actionDir, actionPath)
// ActionCache case: actionDir is a SHA hash, not a real directory.
// os.Stat/ReadDir won't work with tar-backed readers.
// We explicitly check and return a clear error so users know why discovery failed.
info, statErr := os.Stat(baseDir)
if statErr != nil {
// baseDir doesn't exist at all — likely ActionCache is enabled.
// Return original errors plus a clear explanation.
return nil, errors.Join(
errors.Join(allErrors...),
fmt.Errorf("subdir discovery failed for %q: directory not accessible (ActionCache may be enabled, which uses a tar-backed reader incompatible with filesystem discovery): %w", baseDir, statErr),
)
}
if info.IsDir() {
entries, readErr := os.ReadDir(baseDir)
if readErr != nil {
return nil, errors.Join(
errors.Join(allErrors...),
fmt.Errorf("failed to read action directory %q: %w", baseDir, readErr),
)
}
var discoveredFile string
var discoveredSubdir string
for _, entry := range entries {
if !entry.IsDir() {
continue
}
subdir := entry.Name()
actionYml := filepath.Join(baseDir, subdir, "action.yml")
actionYaml := filepath.Join(baseDir, subdir, "action.yaml")
if _, err := os.Stat(actionYml); err == nil {
if discoveredFile != "" {
return nil, fmt.Errorf("ambiguous action: found action.yml in multiple subdirectories: %q and %q (in %q)", discoveredFile, actionYml, baseDir)
}
discoveredFile = actionYml
discoveredSubdir = subdir
continue
}
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)
}
discoveredFile = actionYaml
discoveredSubdir = subdir
}
}
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)
logger.Debugf("Read action %v from '%s'", action, discoveredFile)
return action, nil
}
}
return nil, errors.Join(allErrors...)
}
defer closer.Close()

action, err := model.ReadAction(reader)
if err == nil {
action.ActionPath = actionPath
}
logger.Debugf("Read action %v from '%s'", action, "Unknown")
return action, err
}
Expand Down Expand Up @@ -191,7 +257,11 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction
actionDir = ""
actionPath = containerActionDir
}
return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "entrypoint")
dockerSubpath := actionPath
if remoteAction != nil && action.ActionPath != "" {
dockerSubpath = action.ActionPath
}
return execAsDocker(ctx, step, actionName, actionDir, dockerSubpath, remoteAction == nil, "entrypoint")
case x.IsComposite():
if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil {
return err
Expand Down Expand Up @@ -567,7 +637,11 @@ func runPreStep(step actionStep) common.Executor {
actionDir = ""
actionPath = containerActionDir
}
return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "pre-entrypoint")
dockerSubpath := actionPath
if remoteAction != nil && action.ActionPath != "" {
dockerSubpath = action.ActionPath
}
return execAsDocker(ctx, step, actionName, actionDir, dockerSubpath, remoteAction == nil, "pre-entrypoint")

case x.IsComposite():
if step.getCompositeSteps() == nil {
Expand Down Expand Up @@ -670,7 +744,11 @@ func runPostStep(step actionStep) common.Executor {
actionDir = ""
actionPath = containerActionDir
}
return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "post-entrypoint")
dockerSubpath := actionPath
if remoteAction != nil && action.ActionPath != "" {
dockerSubpath = action.ActionPath
}
return execAsDocker(ctx, step, actionName, actionDir, dockerSubpath, remoteAction == nil, "post-entrypoint")

case x.IsComposite():
if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil {
Expand Down
105 changes: 105 additions & 0 deletions pkg/runner/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"io"
"io/fs"
"os"
"path/filepath"
"strings"
"testing"

Expand Down Expand Up @@ -49,6 +51,7 @@ runs:
PreIf: "always()",
PostIf: "always()",
},
ActionPath: "actionPath",
},
},
{
Expand All @@ -64,6 +67,7 @@ runs:
PreIf: "always()",
PostIf: "always()",
},
ActionPath: "actionPath",
},
},
{
Expand All @@ -77,6 +81,7 @@ runs:
Using: "docker",
Image: "Dockerfile",
},
ActionPath: "actionPath",
},
},
{
Expand Down Expand Up @@ -104,6 +109,7 @@ runs:
Using: "node12",
Main: "trampoline.js",
},
ActionPath: "actionPath",
},
},
}
Expand Down Expand Up @@ -140,6 +146,38 @@ runs:
}
}

func TestActionReaderDiscoverSubdir(t *testing.T) {
yaml := `
name: 'name'
runs:
using: 'node16'
main: 'main.js'
`

baseDir := t.TempDir()
subdir := filepath.Join(baseDir, "github-action")
if err := os.MkdirAll(subdir, 0o755); err != nil {
t.Fatalf("failed to create subdir: %v", err)
}
if err := os.WriteFile(filepath.Join(subdir, "action.yml"), []byte(yaml), 0o644); err != nil {
t.Fatalf("failed to write action.yml: %v", err)
}

readFile := func(filename string) (io.Reader, io.Closer, error) {
return nil, nil, fs.ErrNotExist
}

writeFile := func(filename string, _ []byte, perm fs.FileMode) error {
t.Fatalf("unexpected writeFile call: %s %v", filename, perm)
return nil
}

action, err := readActionImpl(context.Background(), &model.Step{}, baseDir, "", readFile, writeFile)

assert.Nil(t, err)
assert.Equal(t, "github-action", action.ActionPath)
}

Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new discovery behavior is only tested for a single subdir match on the real filesystem. It would be useful to add coverage for (1) multiple subdir matches (to ensure a clear error is returned) and (2) the ActionCache/tar-backed reader path (to ensure discovery works or fails with an intentional, actionable error when ActionCache is enabled).

Suggested change
func TestActionReaderDiscoverSubdirMultipleMatches(t *testing.T) {
yaml := `
name: 'name'
runs:
using: 'node16'
main: 'main.js'
`
baseDir := t.TempDir()
for _, dir := range []string{"github-action", "nested-action"} {
subdir := filepath.Join(baseDir, dir)
if err := os.MkdirAll(subdir, 0o755); err != nil {
t.Fatalf("failed to create subdir %q: %v", dir, err)
}
if err := os.WriteFile(filepath.Join(subdir, "action.yml"), []byte(yaml), 0o644); err != nil {
t.Fatalf("failed to write action.yml in %q: %v", dir, err)
}
}
readFile := func(filename string) (io.Reader, io.Closer, error) {
return nil, nil, fs.ErrNotExist
}
writeFile := func(filename string, _ []byte, perm fs.FileMode) error {
t.Fatalf("unexpected writeFile call: %s %v", filename, perm)
return nil
}
action, err := readActionImpl(context.Background(), &model.Step{}, baseDir, "", readFile, writeFile)
assert.Nil(t, action)
if assert.Error(t, err) {
assert.NotEmpty(t, err.Error())
assert.True(t,
strings.Contains(strings.ToLower(err.Error()), "multiple") ||
strings.Contains(strings.ToLower(err.Error()), "more than one") ||
strings.Contains(strings.ToLower(err.Error()), "ambiguous"),
"expected a clear error for multiple discovered action subdirectories, got: %v", err,
)
}
}
func TestActionReaderDiscoverSubdirActionCacheReader(t *testing.T) {
yaml := `
name: 'name'
runs:
using: 'node16'
main: 'main.js'
`
baseDir := t.TempDir()
discoveredPath := filepath.Join(baseDir, "github-action", "action.yml")
readFile := func(filename string) (io.Reader, io.Closer, error) {
switch filename {
case filepath.Join(baseDir, "action.yml"), filepath.Join(baseDir, "action.yaml"):
return nil, nil, fs.ErrNotExist
case discoveredPath:
return strings.NewReader(yaml), io.NopCloser(strings.NewReader("")), nil
default:
return nil, nil, fs.ErrNotExist
}
}
writeFile := func(filename string, _ []byte, perm fs.FileMode) error {
t.Fatalf("unexpected writeFile call: %s %v", filename, perm)
return nil
}
action, err := readActionImpl(context.Background(), &model.Step{}, baseDir, "", readFile, writeFile)
if err == nil {
assert.Equal(t, "github-action", action.ActionPath)
return
}
assert.Nil(t, action)
assert.True(t,
strings.Contains(strings.ToLower(err.Error()), "actioncache") ||
strings.Contains(strings.ToLower(err.Error()), "cache") ||
strings.Contains(strings.ToLower(err.Error()), "tar") ||
strings.Contains(strings.ToLower(err.Error()), "discover") ||
strings.Contains(strings.ToLower(err.Error()), "subdir"),
"expected action-cache/tar-backed discovery to either work or fail with an actionable error, got: %v", err,
)
}

Copilot uses AI. Check for mistakes.
func TestActionRunner(t *testing.T) {
table := []struct {
name string
Expand Down Expand Up @@ -249,3 +287,70 @@ func TestActionRunner(t *testing.T) {
})
}
}

func TestActionReaderDiscoverSubdirMultipleMatches(t *testing.T) {
yaml := `
name: 'name'
runs:
using: 'node16'
main: 'main.js'
`
baseDir := t.TempDir()
for _, dir := range []string{"github-action", "nested-action"} {
subdir := filepath.Join(baseDir, dir)
if err := os.MkdirAll(subdir, 0o755); err != nil {
t.Fatalf("failed to create subdir %q: %v", dir, err)
}
if err := os.WriteFile(filepath.Join(subdir, "action.yml"), []byte(yaml), 0o644); err != nil {
t.Fatalf("failed to write action.yml in %q: %v", dir, err)
}
}

readFile := func(filename string) (io.Reader, io.Closer, error) {
return nil, nil, fs.ErrNotExist
}

writeFile := func(filename string, _ []byte, perm fs.FileMode) error {
t.Fatalf("unexpected writeFile call: %s %v", filename, perm)
return nil
}

action, err := readActionImpl(context.Background(), &model.Step{}, baseDir, "", readFile, writeFile)

assert.Nil(t, action)
if assert.Error(t, err) {
assert.True(t,
strings.Contains(strings.ToLower(err.Error()), "multiple") ||
strings.Contains(strings.ToLower(err.Error()), "ambiguous"),
"expected a clear error for multiple discovered action subdirectories, got: %v", err,
)
}
}

func TestActionReaderDiscoverSubdirActionCacheUnsupported(t *testing.T) {
// When ActionCache is enabled, actionDir is a SHA hash (not a real path).
// Discovery should fail with a clear, actionable error — not silently.
nonExistentDir := filepath.Join(t.TempDir(), "sha-abc123def456")

readFile := func(filename string) (io.Reader, io.Closer, error) {
return nil, nil, fs.ErrNotExist
}

writeFile := func(filename string, _ []byte, perm fs.FileMode) error {
t.Fatalf("unexpected writeFile call: %s %v", filename, perm)
return nil
}

action, err := readActionImpl(context.Background(), &model.Step{}, nonExistentDir, "", readFile, writeFile)

assert.Nil(t, action)
if assert.Error(t, err) {
assert.True(t,
strings.Contains(strings.ToLower(err.Error()), "actioncache") ||
strings.Contains(strings.ToLower(err.Error()), "tar") ||
strings.Contains(strings.ToLower(err.Error()), "subdir") ||
strings.Contains(strings.ToLower(err.Error()), "accessible"),
"expected a clear error about ActionCache/discovery failure, got: %v", err,
)
}
}