diff --git a/Makefile b/Makefile index c734b053ab..19a3050ba7 100644 --- a/Makefile +++ b/Makefile @@ -525,6 +525,12 @@ actionlint: build @echo "Validating workflows with actionlint..." ./$(BINARY_NAME) compile --actionlint +# Run lock-file-only lint using gh aw lint +.PHONY: lint-lock +lint-lock: build + @echo "Linting committed lock files with gh aw lint..." + ./$(BINARY_NAME) lint + # Format code .PHONY: fmt fmt: fmt-go fmt-cjs fmt-json @@ -758,6 +764,11 @@ agent-finish: deps-dev fmt lint build build-wasm test-all fix recompile dependab agent-report-progress: build fmt test-unit @echo "Pre-PR validation passed. Safe to call report_progress." +# Extended pre-PR gate with lock-file-only linting. +.PHONY: agent-report-progress-lint +agent-report-progress-lint: agent-report-progress lint-lock + @echo "Pre-PR validation + lock-file lint passed. Safe to call report_progress." + # Help target .PHONY: help help: @@ -813,6 +824,7 @@ help: @echo " security-gosec - Run gosec Go security scanner" @echo " security-govulncheck - Run govulncheck for known vulnerabilities" @echo " actionlint - Validate workflows with actionlint (depends on build)" + @echo " lint-lock - Run lock-file-only lint with gh aw lint (depends on build)" @echo " validate-workflows - Validate compiled workflow lock files (depends on build)" @echo " install - Install binary locally" @echo " sync-action-pins - Sync actions-lock.json from .github/aw to pkg/actionpins/data and pkg/workflow/data (runs automatically during build)" @@ -832,5 +844,6 @@ help: @echo " agent-finish - Complete validation sequence (build, test, fix, recompile, fmt, lint, security-scan)" @echo " agent-report-progress - Lightweight pre-PR gate: build + fmt + test-unit (<30s)" + @echo " agent-report-progress-lint - Pre-PR gate + gh aw lint lock-file check" @echo " sbom - Generate SBOM in SPDX and CycloneDX formats (requires syft)" @echo " help - Show this help message" diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 9a5c6aa5c3..89f2c40f59 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -765,6 +765,7 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all projectCmd := cli.NewProjectCommand() checksCmd := cli.NewChecksCommand() validateCmd := cli.NewValidateCommand(validateEngine) + lintCmd := cli.NewLintCommand() domainsCmd := cli.NewDomainsCommand() experimentsCmd := cli.NewExperimentsCommand() @@ -782,6 +783,7 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all // Development Commands compileCmd.GroupID = "development" validateCmd.GroupID = "development" + lintCmd.GroupID = "development" mcpCmd.GroupID = "development" fixCmd.GroupID = "development" domainsCmd.GroupID = "development" @@ -836,6 +838,7 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all rootCmd.AddCommand(secretsCmd) rootCmd.AddCommand(fixCmd) rootCmd.AddCommand(validateCmd) + rootCmd.AddCommand(lintCmd) rootCmd.AddCommand(completionCmd) rootCmd.AddCommand(hashCmd) rootCmd.AddCommand(projectCmd) diff --git a/docs/src/content/docs/setup/cli.md b/docs/src/content/docs/setup/cli.md index e9526c7ce9..0c2bdaab33 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -296,6 +296,21 @@ gh aw validate --engine copilot # Override AI engine All linters (`zizmor`, `actionlint`, `poutine`), `--validate`, and `--no-emit` are always-on defaults and cannot be disabled. Accepts the same workflow ID format as `compile`. +#### `lint` + +Lint existing `.lock.yml` workflow files from disk with actionlint only. This command does not recompile Markdown workflows, and skips `zizmor`/`poutine`. + +```bash wrap +gh aw lint # Lint all .github/workflows/*.lock.yml +gh aw lint .github/workflows/foo.lock.yml # Lint a specific lock file +gh aw lint --dir .github/workflows # Lint all lock files in a directory +gh aw lint --shellcheck --pyflakes # Enable actionlint script integrations +``` + +**Options:** `--dir/-d`, `--shellcheck`, `--pyflakes` + +By default, shellcheck and pyflakes integrations are disabled to reduce noise for generated `run:` scripts. Built-in actionlint ignore patterns cover gh-aw-specific extensions such as `job.workflow_*` context properties and the `copilot-requests` permission scope. + ### Testing #### `trial` diff --git a/pkg/cli/actionlint.go b/pkg/cli/actionlint.go index 92f69cc584..c6c973abeb 100644 --- a/pkg/cli/actionlint.go +++ b/pkg/cli/actionlint.go @@ -22,6 +22,30 @@ var actionlintLog = logger.New("cli:actionlint") // actionlintVersion caches the actionlint version to avoid repeated Docker calls var actionlintVersion string +// actionlintRunOptions configures optional actionlint integrations and ignores. +type actionlintRunOptions struct { + IncludeShellcheck bool + IncludePyflakes bool + // IgnorePatterns contains regular expressions passed to actionlint via + // repeated -ignore flags to suppress known false positives. + IgnorePatterns []string +} + +// buildActionlintIntegrationStatus returns a human-readable description of the +// shellcheck/pyflakes integration state for actionlint execution messages. +func buildActionlintIntegrationStatus(includeShellcheck bool, includePyflakes bool) string { + switch { + case includeShellcheck && includePyflakes: + return "with shellcheck/pyflakes" + case includeShellcheck: + return "with shellcheck only" + case includePyflakes: + return "with pyflakes only" + default: + return "without shellcheck/pyflakes" + } +} + // getActionlintDocsURL returns the documentation URL for a given actionlint error kind // Error kinds map to documentation anchors at https://github.com/rhysd/actionlint/blob/main/docs/checks.md func getActionlintDocsURL(kind string) string { @@ -144,8 +168,9 @@ func displayActionlintSummary() { fmt.Fprintf(os.Stderr, "\n%s\n", separator) } -// getActionlintVersion fetches and caches the actionlint version from Docker -func getActionlintVersion() (string, error) { +// getActionlintVersion fetches and caches the actionlint version from Docker. +// The provided context allows caller-driven cancellation. +func getActionlintVersion(ctx context.Context) (string, error) { // Return cached version if already fetched if actionlintVersion != "" { return actionlintVersion, nil @@ -154,11 +179,11 @@ func getActionlintVersion() (string, error) { actionlintLog.Print("Fetching actionlint version from Docker") // Run docker command to get version with a 30 second timeout - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + versionCtx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() cmd := exec.CommandContext( - ctx, + versionCtx, "docker", "run", "--rm", @@ -185,17 +210,24 @@ func getActionlintVersion() (string, error) { return version, nil } -// runActionlintOnFiles runs the actionlint linter on one or more .lock.yml files using Docker -func runActionlintOnFiles(lockFiles []string, verbose bool, strict bool) error { +// runActionlintOnFiles runs the actionlint linter on one or more .lock.yml files using Docker. +// The provided context allows caller-driven cancellation. +func runActionlintOnFiles(ctx context.Context, lockFiles []string, verbose bool, strict bool) error { + return runActionlintOnFilesWithOptions(ctx, lockFiles, verbose, strict, actionlintRunOptions{ + IncludeShellcheck: true, + IncludePyflakes: true, + }) +} + +func runActionlintOnFilesWithOptions(ctx context.Context, lockFiles []string, verbose bool, strict bool, options actionlintRunOptions) error { if len(lockFiles) == 0 { return nil } - actionlintLog.Printf("Running actionlint on %d file(s): %v (verbose=%t, strict=%t)", len(lockFiles), lockFiles, verbose, strict) // Display actionlint version on first use if actionlintVersion == "" { - version, err := getActionlintVersion() + version, err := getActionlintVersion(ctx) if err != nil { // Log error but continue - version display is not critical actionlintLog.Printf("Could not fetch actionlint version: %v", err) @@ -224,7 +256,7 @@ func runActionlintOnFiles(lockFiles []string, verbose bool, strict bool) error { // docker run --rm -v "$(pwd)":/workdir -w /workdir rhysd/actionlint:latest -format '{{json .}}' ... // Adjust timeout based on number of files (1 minute per file, minimum 5 minutes) timeoutDuration := time.Duration(max(5, len(lockFiles))) * time.Minute - ctx, cancel := context.WithTimeout(context.Background(), timeoutDuration) + runCtx, cancel := context.WithTimeout(ctx, timeoutDuration) defer cancel() // Build Docker command arguments @@ -236,15 +268,27 @@ func runActionlintOnFiles(lockFiles []string, verbose bool, strict bool) error { "rhysd/actionlint:latest", "-format", "{{json .}}", } + if !options.IncludeShellcheck { + // Empty value disables shellcheck integration in actionlint. + dockerArgs = append(dockerArgs, "-shellcheck=") + } + if !options.IncludePyflakes { + // Empty value disables pyflakes integration in actionlint. + dockerArgs = append(dockerArgs, "-pyflakes=") + } + for _, ignorePattern := range options.IgnorePatterns { + dockerArgs = append(dockerArgs, "-ignore", ignorePattern) + } dockerArgs = append(dockerArgs, relPaths...) - cmd := exec.CommandContext(ctx, "docker", dockerArgs...) + cmd := exec.CommandContext(runCtx, "docker", dockerArgs...) // Always show that actionlint is running (regular verbosity) + integrationStatus := buildActionlintIntegrationStatus(options.IncludeShellcheck, options.IncludePyflakes) if len(lockFiles) == 1 { - fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("Running actionlint (includes shellcheck & pyflakes) on "+relPaths[0])) + fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("Running actionlint ("+integrationStatus+") on "+relPaths[0])) } else { - fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage(fmt.Sprintf("Running actionlint (includes shellcheck & pyflakes) on %d files", len(lockFiles)))) + fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage(fmt.Sprintf("Running actionlint (%s) on %d files", integrationStatus, len(lockFiles)))) } // In verbose mode, also show the command that users can run directly @@ -263,7 +307,7 @@ func runActionlintOnFiles(lockFiles []string, verbose bool, strict bool) error { err = cmd.Run() // Check for timeout - if ctx.Err() == context.DeadlineExceeded { + if runCtx.Err() == context.DeadlineExceeded { fileList := "files" if len(lockFiles) == 1 { fileList = filepath.Base(lockFiles[0]) @@ -273,6 +317,9 @@ func runActionlintOnFiles(lockFiles []string, verbose bool, strict bool) error { } return fmt.Errorf("actionlint timed out after %d minutes on %s - this may indicate a Docker or network issue", int(timeoutDuration.Minutes()), fileList) } + if runCtx.Err() == context.Canceled { + return fmt.Errorf("actionlint was canceled before completion (for example by Ctrl+C or caller cancellation)") + } // Track workflows in statistics (count number of files validated) if actionlintStats != nil { diff --git a/pkg/cli/actionlint_test.go b/pkg/cli/actionlint_test.go index 89a197d2be..a5936887c5 100644 --- a/pkg/cli/actionlint_test.go +++ b/pkg/cli/actionlint_test.go @@ -3,6 +3,7 @@ package cli import ( + "context" "testing" "github.com/github/gh-aw/pkg/testutil" @@ -125,7 +126,7 @@ func TestGetActionlintVersion(t *testing.T) { defer func() { actionlintVersion = original }() actionlintVersion = "1.7.9" - version, err := getActionlintVersion() + version, err := getActionlintVersion(context.Background()) require.NoError(t, err, "should not error when version is cached") assert.Equal(t, "1.7.9", version, "should return cached version") } @@ -324,3 +325,44 @@ func TestGetActionlintDocsURL(t *testing.T) { }) } } + +func TestBuildActionlintIntegrationStatus(t *testing.T) { + tests := []struct { + name string + includeShellcheck bool + includePyflakes bool + expected string + }{ + { + name: "both integrations enabled", + includeShellcheck: true, + includePyflakes: true, + expected: "with shellcheck/pyflakes", + }, + { + name: "only shellcheck enabled", + includeShellcheck: true, + includePyflakes: false, + expected: "with shellcheck only", + }, + { + name: "only pyflakes enabled", + includeShellcheck: false, + includePyflakes: true, + expected: "with pyflakes only", + }, + { + name: "both integrations disabled", + includeShellcheck: false, + includePyflakes: false, + expected: "without shellcheck/pyflakes", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := buildActionlintIntegrationStatus(tt.includeShellcheck, tt.includePyflakes) + assert.Equal(t, tt.expected, result, "integration status should match") + }) + } +} diff --git a/pkg/cli/compile_external_tools.go b/pkg/cli/compile_external_tools.go index 92ed71914b..04f295bf5a 100644 --- a/pkg/cli/compile_external_tools.go +++ b/pkg/cli/compile_external_tools.go @@ -22,6 +22,7 @@ package cli import ( + "context" "fmt" "os" @@ -34,7 +35,9 @@ var compileExternalToolsLog = logger.New("cli:compile_external_tools") // RunActionlintOnFiles runs actionlint on multiple lock files in a single batch. // This is more efficient than running actionlint once per file. func RunActionlintOnFiles(lockFiles []string, verbose bool, strict bool) error { - return runBatchLockFileTool("actionlint", lockFiles, verbose, strict, runActionlintOnFiles) + return runBatchLockFileTool("actionlint", lockFiles, verbose, strict, func(files []string, runVerbose bool, runStrict bool) error { + return runActionlintOnFiles(context.Background(), files, runVerbose, runStrict) + }) } // RunZizmorOnFiles runs zizmor on multiple lock files in a single batch. diff --git a/pkg/cli/compile_validation.go b/pkg/cli/compile_validation.go index 4148001188..4d8e1ab9c0 100644 --- a/pkg/cli/compile_validation.go +++ b/pkg/cli/compile_validation.go @@ -1,6 +1,7 @@ package cli import ( + "context" "errors" "fmt" "os" @@ -90,7 +91,7 @@ func CompileWorkflowWithValidation(compiler *workflow.Compiler, filePath string, // Run actionlint on the generated lock file if requested // Note: For batch processing, use RunActionlintOnFiles instead if runActionlintPerFile { - if err := runActionlintOnFiles([]string{lockFile}, verbose, strict); err != nil { + if err := runActionlintOnFiles(context.Background(), []string{lockFile}, verbose, strict); err != nil { return fmt.Errorf("actionlint linter failed: %w", err) } } @@ -158,7 +159,7 @@ func CompileWorkflowDataWithValidation(compiler *workflow.Compiler, workflowData // Run actionlint on the generated lock file if requested // Note: For batch processing, use RunActionlintOnFiles instead if runActionlintPerFile { - if err := runActionlintOnFiles([]string{lockFile}, verbose, strict); err != nil { + if err := runActionlintOnFiles(context.Background(), []string{lockFile}, verbose, strict); err != nil { return fmt.Errorf("actionlint linter failed: %w", err) } } diff --git a/pkg/cli/lint_command.go b/pkg/cli/lint_command.go new file mode 100644 index 0000000000..c71ebe4adc --- /dev/null +++ b/pkg/cli/lint_command.go @@ -0,0 +1,118 @@ +package cli + +import ( + "fmt" + "os" + "path/filepath" + "slices" + "strings" + + "github.com/github/gh-aw/pkg/constants" + "github.com/spf13/cobra" +) + +var defaultGhAwActionlintIgnorePatterns = []string{ + // gh-aw extends GitHub Actions permissions with copilot-requests. + `unknown permission scope "copilot-requests"`, + // gh-aw exposes additional job.workflow_* context properties. + `property "workflow_(repository|sha|ref|file_path)" is not defined in object type`, +} + +// NewLintCommand creates the lint command. +func NewLintCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "lint [lock-file-or-directory]...", + Short: "Lint existing .lock.yml workflows with actionlint only", + Long: `Lint existing .lock.yml workflow files from disk using actionlint only. + +This command does not recompile Markdown workflows and does not run zizmor or poutine. +By default, shellcheck and pyflakes integrations are disabled for generated run scripts. + +Examples: + ` + string(constants.CLIExtensionPrefix) + ` lint + ` + string(constants.CLIExtensionPrefix) + ` lint .github/workflows/foo.lock.yml + ` + string(constants.CLIExtensionPrefix) + ` lint --dir .github/workflows + ` + string(constants.CLIExtensionPrefix) + ` lint --shellcheck --pyflakes`, + RunE: func(cmd *cobra.Command, args []string) error { + workflowDir, _ := cmd.Flags().GetString("dir") + includeShellcheck, _ := cmd.Flags().GetBool("shellcheck") + includePyflakes, _ := cmd.Flags().GetBool("pyflakes") + verbose, _ := cmd.Flags().GetBool("verbose") + + lockFiles, err := resolveLockFilesForLint(args, workflowDir) + if err != nil { + return err + } + + initActionlintStats() + defer displayActionlintSummary() + + // Lint should fail on any actionlint error. + strictActionlint := true + return runActionlintOnFilesWithOptions(cmd.Context(), lockFiles, verbose, strictActionlint, actionlintRunOptions{ + IncludeShellcheck: includeShellcheck, + IncludePyflakes: includePyflakes, + IgnorePatterns: defaultGhAwActionlintIgnorePatterns, + }) + }, + } + + cmd.Flags().StringP("dir", "d", ".github/workflows", "Directory to scan for *.lock.yml files when no arguments are provided") + cmd.Flags().Bool("shellcheck", false, "Enable shellcheck integration in actionlint") + cmd.Flags().Bool("pyflakes", false, "Enable pyflakes integration in actionlint") + + RegisterDirFlagCompletion(cmd, "dir") + + return cmd +} + +func resolveLockFilesForLint(inputs []string, workflowDir string) ([]string, error) { + candidates := inputs + if len(candidates) == 0 { + candidates = []string{workflowDir} + } + + var lockFiles []string + for _, candidate := range candidates { + lockFilesFromCandidate, err := expandLintCandidate(candidate) + if err != nil { + return nil, err + } + lockFiles = append(lockFiles, lockFilesFromCandidate...) + } + + if len(lockFiles) == 0 { + return nil, fmt.Errorf("no .lock.yml files found to lint") + } + + slices.Sort(lockFiles) + lockFiles = slices.Compact(lockFiles) + + return lockFiles, nil +} + +func expandLintCandidate(candidate string) ([]string, error) { + absCandidate, err := filepath.Abs(candidate) + if err != nil { + return nil, fmt.Errorf("failed to resolve path %q: %w", candidate, err) + } + + info, err := os.Stat(absCandidate) + if err != nil { + return nil, fmt.Errorf("failed to access %q: %w", candidate, err) + } + + if info.IsDir() { + lockFiles, err := filepath.Glob(filepath.Join(absCandidate, "*.lock.yml")) + if err != nil { + return nil, fmt.Errorf("failed to scan %q for .lock.yml files: %w", candidate, err) + } + return lockFiles, nil + } + + if !strings.HasSuffix(absCandidate, ".lock.yml") { + return nil, fmt.Errorf("path %q is not a .lock.yml file or directory", candidate) + } + + return []string{absCandidate}, nil +} diff --git a/pkg/cli/lint_command_test.go b/pkg/cli/lint_command_test.go new file mode 100644 index 0000000000..8e48630c32 --- /dev/null +++ b/pkg/cli/lint_command_test.go @@ -0,0 +1,62 @@ +//go:build !integration + +package cli + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewLintCommand(t *testing.T) { + cmd := NewLintCommand() + + require.NotNil(t, cmd, "NewLintCommand should return a non-nil command") + assert.Equal(t, "lint", cmd.Name(), "Command name should be 'lint'") + require.NotNil(t, cmd.Flags().Lookup("dir"), "lint command should have a --dir flag") + assert.Equal(t, "d", cmd.Flags().Lookup("dir").Shorthand, "--dir should have -d shorthand") + require.NotNil(t, cmd.Flags().Lookup("shellcheck"), "lint command should have a --shellcheck flag") + require.NotNil(t, cmd.Flags().Lookup("pyflakes"), "lint command should have a --pyflakes flag") + assert.Contains(t, defaultGhAwActionlintIgnorePatterns, `unknown permission scope "copilot-requests"`, + "lint command should include built-in ignore for gh-aw permission extension") + assert.Contains(t, defaultGhAwActionlintIgnorePatterns, `property "workflow_(repository|sha|ref|file_path)" is not defined in object type`, + "lint command should include built-in ignore for gh-aw workflow context extensions") +} + +func TestResolveLockFilesForLint(t *testing.T) { + tempDir := t.TempDir() + lockA := filepath.Join(tempDir, "a.lock.yml") + lockB := filepath.Join(tempDir, "b.lock.yml") + nonLock := filepath.Join(tempDir, "workflow.md") + + require.NoError(t, os.WriteFile(lockA, []byte("name: a"), 0o644), "should create lock file a") + require.NoError(t, os.WriteFile(lockB, []byte("name: b"), 0o644), "should create lock file b") + require.NoError(t, os.WriteFile(nonLock, []byte("---"), 0o644), "should create non-lock file") + + t.Run("defaults to scanning dir when no args", func(t *testing.T) { + files, err := resolveLockFilesForLint(nil, tempDir) + require.NoError(t, err, "should resolve lock files from default dir") + assert.Equal(t, []string{lockA, lockB}, files, "should return sorted .lock.yml files only") + }) + + t.Run("accepts explicit lock file path", func(t *testing.T) { + files, err := resolveLockFilesForLint([]string{lockB}, tempDir) + require.NoError(t, err, "should accept explicit lock file") + assert.Equal(t, []string{lockB}, files, "should include explicit lock file") + }) + + t.Run("accepts explicit directory path", func(t *testing.T) { + files, err := resolveLockFilesForLint([]string{tempDir}, tempDir) + require.NoError(t, err, "should accept explicit directory") + assert.Equal(t, []string{lockA, lockB}, files, "should expand directory to lock files") + }) + + t.Run("rejects non lock file path", func(t *testing.T) { + _, err := resolveLockFilesForLint([]string{nonLock}, tempDir) + require.Error(t, err, "should reject non-lock file path") + assert.Contains(t, err.Error(), "is not a .lock.yml file or directory", "error should explain allowed path types") + }) +}