From bd3b749069c73625fcbedc46014c5e6b0b21324f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 3 Aug 2026 01:36:05 +0000 Subject: [PATCH 1/3] feat: disable shellcheck by default, enable via --shellcheck or --validate, run in parallel Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- cmd/gh-aw/main.go | 10 +-- pkg/cli/compile_command_test.go | 4 +- pkg/cli/compile_config.go | 2 +- pkg/cli/compile_orchestrator.go | 13 ++-- pkg/cli/compile_pipeline.go | 8 +-- pkg/cli/compile_pipeline_yamllint_test.go | 2 +- pkg/cli/mcp_tools_readonly.go | 19 ------ pkg/cli/shellcheck.go | 80 ++++++++++++++--------- 8 files changed, 69 insertions(+), 69 deletions(-) diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 557c3516bf8..2573a53aec0 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -422,7 +422,7 @@ type compileCmdOptions struct { grype bool grant bool yamllint bool - noShellcheck bool + shellcheck bool jsonOutput bool showAllErrors bool fix bool @@ -465,7 +465,7 @@ func getCompileCmdOptions(cmd *cobra.Command) compileCmdOptions { grype, _ := cmd.Flags().GetBool("grype") grant, _ := cmd.Flags().GetBool("grant") yamllint, _ := cmd.Flags().GetBool("yamllint") - noShellcheck, _ := cmd.Flags().GetBool("no-shellcheck") + shellcheck, _ := cmd.Flags().GetBool("shellcheck") jsonOutput, _ := cmd.Flags().GetBool("json") showAllErrors, _ := cmd.Flags().GetBool("show-all") fix, _ := cmd.Flags().GetBool("fix") @@ -485,7 +485,7 @@ func getCompileCmdOptions(cmd *cobra.Command) compileCmdOptions { dir: dir, workflowsDir: workflowsDir, logicalRepo: logicalRepo, scheduleSeed: scheduleSeed, priorManifestFile: priorManifestFile, validate: validate, watch: watch, noEmit: noEmit, purge: purge, strict: strict, trial: trial, dependabot: dependabot, forceOverwrite: forceOverwrite, refreshStopTime: refreshStopTime, forceRefreshActionPins: forceRefreshActionPins, allowActionRefs: allowActionRefs, - zizmor: zizmor, poutine: poutine, actionlint: actionlint, runnerGuard: runnerGuard, syft: syft, grype: grype, grant: grant, yamllint: yamllint, noShellcheck: noShellcheck, + zizmor: zizmor, poutine: poutine, actionlint: actionlint, runnerGuard: runnerGuard, syft: syft, grype: grype, grant: grant, yamllint: yamllint, shellcheck: shellcheck, jsonOutput: jsonOutput, showAllErrors: showAllErrors, fix: fix, stats: stats, failFast: failFast, noCheckUpdate: noCheckUpdate, staged: staged, approve: approve, validateImages: validateImages, ghes: ghes, verbose: verbose, useSamples: useSamples, } @@ -518,7 +518,7 @@ func (o *compileCmdOptions) toCompileConfig(args []string) cli.CompileConfig { NoEmit: o.noEmit, Purge: o.purge, TrialMode: o.trial, TrialLogicalRepoSlug: o.logicalRepo, Strict: o.strict, Dependabot: o.dependabot, ForceOverwrite: o.forceOverwrite, RefreshStopTime: o.refreshStopTime, ForceRefreshActionPins: o.forceRefreshActionPins, AllowActionRefs: o.allowActionRefs, Zizmor: o.zizmor, Poutine: o.poutine, Actionlint: o.actionlint, RunnerGuard: o.runnerGuard, - Syft: o.syft, Grype: o.grype, Grant: o.grant, Yamllint: o.yamllint, NoShellcheck: o.noShellcheck, JSONOutput: o.jsonOutput, ShowAllErrors: o.showAllErrors, + Syft: o.syft, Grype: o.grype, Grant: o.grant, Yamllint: o.yamllint, Shellcheck: o.shellcheck, JSONOutput: o.jsonOutput, ShowAllErrors: o.showAllErrors, Stats: o.stats, FailFast: o.failFast, ScheduleSeed: o.scheduleSeed, Staged: o.staged, Approve: o.approve, ValidateImages: o.validateImages, PriorManifestFile: o.priorManifestFile, GHESCompat: o.ghes, UseSamples: o.useSamples, } @@ -775,7 +775,7 @@ func configureCompileToolFlags() { compileCmd.Flags().Bool("grype", false, "Run grype vulnerability scanner on container images referenced in compiled .lock.yml files (uses Docker image "+cli.GrypeImage+")") compileCmd.Flags().Bool("grant", false, "Run grant license scanner on container images referenced in compiled .lock.yml files (uses Docker image "+cli.GrantImage+")") compileCmd.Flags().Bool("yamllint", false, "Run yamllint YAML linter on generated .lock.yml files (uses Docker image "+cli.YamllintImage+")") - compileCmd.Flags().Bool("no-shellcheck", false, "Disable shellcheck linting of run step scripts (shellcheck runs by default when available)") + compileCmd.Flags().Bool("shellcheck", false, "Run shellcheck linting of run step scripts (also enabled by --validate)") compileCmd.Flags().Bool("fix", false, "Apply automatic codemod fixes to workflows before compiling") compileCmd.Flags().BoolP("json", "j", false, "Output results in JSON format") compileCmd.Flags().Bool("show-all", false, "Display all compilation errors instead of only the highest-priority subset (default: top 5)") diff --git a/pkg/cli/compile_command_test.go b/pkg/cli/compile_command_test.go index 6913b86058c..eb4e43b2917 100644 --- a/pkg/cli/compile_command_test.go +++ b/pkg/cli/compile_command_test.go @@ -509,8 +509,8 @@ func TestCompileConfig_DefaultValues(t *testing.T) { if config.Actionlint { t.Error("Expected Actionlint to default to false") } - if config.NoShellcheck { - t.Error("Expected NoShellcheck to default to false (shellcheck runs by default when available)") + if config.Shellcheck { + t.Error("Expected Shellcheck to default to false (shellcheck is disabled by default)") } } diff --git a/pkg/cli/compile_config.go b/pkg/cli/compile_config.go index f30963d19ba..aaf56505fbf 100644 --- a/pkg/cli/compile_config.go +++ b/pkg/cli/compile_config.go @@ -29,7 +29,7 @@ type CompileConfig struct { Grype bool // Run grype vulnerability scanner on container images referenced in compiled .lock.yml files Grant bool // Run grant license scanner on container images referenced in compiled .lock.yml files Yamllint bool // Run yamllint YAML linter on generated .lock.yml files - NoShellcheck bool // Skip shellcheck linting of run step scripts (shellcheck runs by default when available) + Shellcheck bool // Run shellcheck linting of run step scripts (disabled by default; enabled by --shellcheck or --validate) JSONOutput bool // Output validation results as JSON ShowAllErrors bool // Display all prioritized errors instead of the default top five ActionMode string // How action scripts are referenced: dev, release, or action. Auto-detected if empty. diff --git a/pkg/cli/compile_orchestrator.go b/pkg/cli/compile_orchestrator.go index 3a1d3d92db9..0fa1d7bcac7 100644 --- a/pkg/cli/compile_orchestrator.go +++ b/pkg/cli/compile_orchestrator.go @@ -64,16 +64,15 @@ func CompileWorkflows(ctx context.Context, config CompileConfig) ([]*workflow.Wo initActionlintStats() } - // Warn or error when shellcheck is enabled (the default) but not installed. + // Warn or error when shellcheck is explicitly requested via --shellcheck but not installed. // Skip this check when --no-emit is set: no lock files are written so shellcheck - // is never invoked, regardless of the --strict or --validate flags. - // When the binary is absent, Docker is used as a fallback (lazy — only when - // there are scripts to lint). Only warn/error when neither is available. - if !config.NoShellcheck && !config.NoEmit && !isShellcheckAvailable() { + // is never invoked. When the binary is absent, Docker is used as a fallback (lazy + // — only when there are scripts to lint). Only warn/error when neither is available. + if config.Shellcheck && !config.NoEmit && !isShellcheckAvailable() { if !IsDockerAvailable(ctx) { if config.Strict { - return nil, errors.New("shellcheck not available: binary not found in PATH and Docker is not running; install shellcheck or start Docker to enable run step linting, or use --no-shellcheck to skip") - } else if config.Validate { + return nil, errors.New("shellcheck not available: binary not found in PATH and Docker is not running; install shellcheck or start Docker to enable run step linting") + } else { fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("shellcheck binary not found in PATH and Docker is not running; run step linting will be skipped")) } } diff --git a/pkg/cli/compile_pipeline.go b/pkg/cli/compile_pipeline.go index a1ac3125375..308f97ccf6f 100644 --- a/pkg/cli/compile_pipeline.go +++ b/pkg/cli/compile_pipeline.go @@ -174,7 +174,7 @@ func compileSpecificFiles( if config.Yamllint { lockFilesForYamllint = append(lockFilesForYamllint, fileResult.lockFile) } - if !config.NoShellcheck { + if config.Shellcheck || config.Validate { lockFilesForShellcheck = append(lockFilesForShellcheck, fileResult.lockFile) } } @@ -297,7 +297,7 @@ func compileSpecificFiles( } // Run shellcheck on run step scripts in all collected lock files. - if !config.NoShellcheck && !config.NoEmit && len(lockFilesForShellcheck) > 0 { + if (config.Shellcheck || config.Validate) && !config.NoEmit && len(lockFilesForShellcheck) > 0 { if err := ctx.Err(); err != nil { return workflowDataList, err } @@ -486,7 +486,7 @@ func compileAllFilesInDirectory( if config.Yamllint { lockFilesForYamllint = append(lockFilesForYamllint, fileResult.lockFile) } - if !config.NoShellcheck { + if config.Shellcheck || config.Validate { lockFilesForShellcheck = append(lockFilesForShellcheck, fileResult.lockFile) } } @@ -605,7 +605,7 @@ func compileAllFilesInDirectory( } // Run shellcheck on run step scripts in all collected lock files. - if !config.NoShellcheck && !config.NoEmit && len(lockFilesForShellcheck) > 0 { + if (config.Shellcheck || config.Validate) && !config.NoEmit && len(lockFilesForShellcheck) > 0 { if err := ctx.Err(); err != nil { return workflowDataList, err } diff --git a/pkg/cli/compile_pipeline_yamllint_test.go b/pkg/cli/compile_pipeline_yamllint_test.go index 03aed007f44..89ec8b91156 100644 --- a/pkg/cli/compile_pipeline_yamllint_test.go +++ b/pkg/cli/compile_pipeline_yamllint_test.go @@ -85,7 +85,7 @@ This is a test workflow for yamllint batch execution. NoEmit: false, Yamllint: true, Strict: strict, - NoShellcheck: true, // yamllint test is independent of shellcheck + // Shellcheck is disabled by default; yamllint test is independent of shellcheck. } _, err = CompileWorkflows(context.Background(), config) diff --git a/pkg/cli/mcp_tools_readonly.go b/pkg/cli/mcp_tools_readonly.go index bf5e8533e2d..f4336e8b855 100644 --- a/pkg/cli/mcp_tools_readonly.go +++ b/pkg/cli/mcp_tools_readonly.go @@ -141,7 +141,6 @@ Returns JSON array with validation results for each workflow: // the caller knows linting was skipped, while preserving each workflow's // valid/invalid status. var dockerUnavailableWarning string - var shellcheckUnavailableWarning string // Check if any static analysis tools are requested that require Docker images if args.Zizmor || args.Poutine || args.Actionlint || args.RunnerGuard || args.Syft || args.Grype || args.Grant || args.Yamllint { @@ -197,14 +196,6 @@ Returns JSON array with validation results for each workflow: // Always validate workflows during compilation and use JSON output for MCP. cmdArgs := []string{"compile", "--validate", "--json"} - // Keep compile JSON responses usable in MCP on hosts without shellcheck support. - // When neither a native shellcheck binary nor Docker fallback is available, - // skip shellcheck for this subprocess and return a structured warning. - if !isShellcheckAvailable() && !IsDockerAvailable(ctx) { - cmdArgs = append(cmdArgs, "--no-shellcheck") - shellcheckUnavailableWarning = "shellcheck binary not found in PATH and Docker is not running; run step linting was skipped" - } - // Add fix flag if requested if args.Fix { cmdArgs = append(cmdArgs, "--fix") @@ -301,9 +292,6 @@ Returns JSON array with validation results for each workflow: if dockerUnavailableWarning != "" { outputStr = injectDockerUnavailableWarning(outputStr, dockerUnavailableWarning) } - if shellcheckUnavailableWarning != "" { - outputStr = injectShellcheckUnavailableWarning(outputStr, shellcheckUnavailableWarning) - } outputStr = injectShellcheckDiagnostics(outputStr, string(stderr)) return &mcp.CallToolResult{ @@ -522,13 +510,6 @@ func injectDockerUnavailableWarning(outputStr, warningMsg string) string { }) } -func injectShellcheckUnavailableWarning(outputStr, warningMsg string) string { - return injectValidationWarning(outputStr, CompileValidationError{ - Type: "shellcheck_unavailable", - Message: warningMsg, - }) -} - func injectShellcheckDiagnostics(outputStr, stderrOutput string) string { diagnostics := extractShellcheckDiagnostics(stderrOutput) if len(diagnostics) == 0 { diff --git a/pkg/cli/shellcheck.go b/pkg/cli/shellcheck.go index 0a5353c57b5..0728fbe484b 100644 --- a/pkg/cli/shellcheck.go +++ b/pkg/cli/shellcheck.go @@ -23,6 +23,7 @@ import ( "path/filepath" "regexp" "strings" + "sync" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" @@ -366,8 +367,8 @@ func runShellcheckOnScriptViaDocker(ctx context.Context, info runStepInfo, ignor } // runShellcheckOnLockFiles extracts run: steps from each lock file and runs -// shellcheck on the shell snippets. It uses shellcheckDefaultIgnoreCodes to -// suppress known false positives from GitHub Actions expression syntax. +// shellcheck on the shell snippets in parallel. It uses shellcheckDefaultIgnoreCodes +// to suppress known false positives from GitHub Actions expression syntax. // // When the shellcheck binary is not installed, it falls back to the Docker // container (ShellcheckImage) if Docker is available. This allows shellcheck @@ -375,9 +376,10 @@ func runShellcheckOnScriptViaDocker(ctx context.Context, info runStepInfo, ignor // The fallback is lazy: the Docker image is only invoked when there are scripts // to lint and the binary is absent. // -// When strict is false, individual step failures are printed as warnings and -// the function returns nil. When strict is true, the first step failure causes -// an error to be returned immediately (fail fast). +// Steps across all lock files are collected first, then run in parallel using +// goroutines. When strict is true, the first step failure causes an error to be +// returned. In non-strict mode, all failures are printed as warnings and nil is +// returned. func runShellcheckOnLockFiles(ctx context.Context, lockFiles []string, verbose bool, strict bool) error { if len(lockFiles) == 0 { return nil @@ -393,18 +395,8 @@ func runShellcheckOnLockFiles(ctx context.Context, lockFiles []string, verbose b shellcheckLog.Print("shellcheck binary not found in PATH; using Docker container fallback") } - shellcheckLog.Printf("Running shellcheck on run steps in %d lock file(s) (strict=%t, docker=%t)", len(lockFiles), strict, useDocker) - - if len(lockFiles) == 1 { - fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("Running shellcheck on run steps in "+filepath.Base(lockFiles[0]))) - } else { - fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage(fmt.Sprintf("Running shellcheck on run steps in %d files", len(lockFiles)))) - } - - var totalSteps, totalIssues int - var firstErr error - -outer: + // Collect all steps from all lock files first. + var allSteps []runStepInfo for _, lockFile := range lockFiles { steps, err := extractRunStepsFromLockFile(lockFile) if err != nil { @@ -412,29 +404,57 @@ outer: fmt.Fprintf(os.Stderr, "%s\n", console.FormatWarningMessage("shellcheck: could not parse "+filepath.Base(lockFile)+": "+err.Error())) continue } + allSteps = append(allSteps, steps...) + } + + if len(allSteps) == 0 { + return nil + } + + shellcheckLog.Printf("Running shellcheck on run steps in %d lock file(s) (strict=%t, docker=%t)", len(lockFiles), strict, useDocker) - for _, step := range steps { - totalSteps++ + if len(lockFiles) == 1 { + fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("Running shellcheck on run steps in "+filepath.Base(lockFiles[0]))) + } else { + fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage(fmt.Sprintf("Running shellcheck on run steps in %d files", len(lockFiles)))) + } + + // Run shellcheck on all steps in parallel. + type result struct { + err error + } + results := make([]result, len(allSteps)) + var wg sync.WaitGroup + for i, step := range allSteps { + wg.Add(1) + go func(idx int, s runStepInfo) { + defer wg.Done() var stepErr error if useDocker { - stepErr = runShellcheckOnScriptViaDocker(ctx, step, shellcheckDefaultIgnoreCodes, verbose) + stepErr = runShellcheckOnScriptViaDocker(ctx, s, shellcheckDefaultIgnoreCodes, verbose) } else { - stepErr = runShellcheckOnScript(step, shellcheckDefaultIgnoreCodes, verbose) + stepErr = runShellcheckOnScript(s, shellcheckDefaultIgnoreCodes, verbose) } - if stepErr != nil { - totalIssues++ - shellcheckLog.Printf("shellcheck issue in %s step %q: %v", lockFile, step.Name, stepErr) - if strict { - firstErr = stepErr - break outer // fail fast in strict mode - } + results[idx] = result{err: stepErr} + }(i, step) + } + wg.Wait() + + var totalIssues int + var firstErr error + for i, r := range results { + if r.err != nil { + totalIssues++ + shellcheckLog.Printf("shellcheck issue in step %q: %v", allSteps[i].Name, r.err) + if firstErr == nil { + firstErr = r.err } } } - shellcheckLog.Printf("shellcheck complete: steps=%d, issues=%d", totalSteps, totalIssues) + shellcheckLog.Printf("shellcheck complete: steps=%d, issues=%d", len(allSteps), totalIssues) - if firstErr != nil { + if strict && firstErr != nil { return fmt.Errorf("strict mode: shellcheck found issues in run steps: %w", firstErr) } return nil From 09a6d657d34101d5577f85aa9869f0545b9104d9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 3 Aug 2026 02:41:24 +0000 Subject: [PATCH 2/3] fix: bounded concurrency, buffered output, availability warning, --no-shellcheck no-op, multi-step tests, changeset Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .changeset/shellcheck-opt-in-default.md | 12 +++ cmd/gh-aw/main.go | 4 +- pkg/cli/compile_orchestrator.go | 4 +- pkg/cli/shellcheck.go | 98 +++++++++++++++++-------- pkg/cli/shellcheck_test.go | 80 +++++++++++++++++++- 5 files changed, 161 insertions(+), 37 deletions(-) create mode 100644 .changeset/shellcheck-opt-in-default.md diff --git a/.changeset/shellcheck-opt-in-default.md b/.changeset/shellcheck-opt-in-default.md new file mode 100644 index 00000000000..77dde3a523d --- /dev/null +++ b/.changeset/shellcheck-opt-in-default.md @@ -0,0 +1,12 @@ +--- +"gh-aw": major +--- + +`shellcheck` is now opt-in via `--shellcheck` (or `--validate`); the `--no-shellcheck` flag is deprecated. + +**Breaking Change**: shellcheck previously ran by default on every `gh aw compile` invocation (opt-out via `--no-shellcheck`). It is now disabled by default, consistent with all other optional checkers (`--poutine`, `--zizmor`, `--actionlint`, etc.). + +**Migration guide:** +- If you relied on shellcheck running by default, add `--shellcheck` to your `gh aw compile` invocations. +- Alternatively, use `--validate` to enable shellcheck together with schema, container-image, and action-SHA validation. +- The `--no-shellcheck` flag is retained as a deprecated no-op for script compatibility; it will be removed in a future release. diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 2573a53aec0..efc479e8253 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -747,7 +747,7 @@ func configureCompileBuildFlags() { compileCmd.Flags().String("action-tag", "", "Pin compiled workflows to a specific version of gh-aw actions. Accepts a full commit SHA or a version tag (e.g. v1, v1.2.3). Sets --action-mode to 'release' unless --action-mode action is also specified. Cannot be combined with --gh-aw-ref; use --gh-aw-ref when you want to resolve a branch or tag name to its current SHA") compileCmd.Flags().String("actions-repo", "", "Override the external actions repository used in action mode (default: github/gh-aw-actions)") compileCmd.Flags().String("gh-aw-ref", "", "Pin compiled workflows to a specific branch, tag, or commit SHA of github/gh-aw (e.g. main, my-feature, abc123). Branch and tag names are resolved to their full commit SHA at compile time so the baked-in ref is immutable. Equivalent to --action-mode release --action-tag . Cannot be combined with --action-tag or --action-mode. Use this to E2E-test workflows against a specific gh-aw revision") - compileCmd.Flags().Bool("validate", false, "Enable GitHub Actions workflow schema validation, container image validation, and action SHA validation") + compileCmd.Flags().Bool("validate", false, "Enable GitHub Actions workflow schema validation, container image validation, action SHA validation, and run step shell linting (shellcheck)") compileCmd.Flags().BoolP("watch", "w", false, "Watch for changes to workflow files and recompile automatically") compileCmd.Flags().StringP("dir", "d", "", "Workflow directory (default: $GH_AW_WORKFLOWS_DIR or .github/workflows)") compileCmd.Flags().String("workflows-dir", "", "Deprecated: use --dir instead") @@ -776,6 +776,8 @@ func configureCompileToolFlags() { compileCmd.Flags().Bool("grant", false, "Run grant license scanner on container images referenced in compiled .lock.yml files (uses Docker image "+cli.GrantImage+")") compileCmd.Flags().Bool("yamllint", false, "Run yamllint YAML linter on generated .lock.yml files (uses Docker image "+cli.YamllintImage+")") compileCmd.Flags().Bool("shellcheck", false, "Run shellcheck linting of run step scripts (also enabled by --validate)") + compileCmd.Flags().Bool("no-shellcheck", false, "Deprecated: shellcheck is now opt-in via --shellcheck; this flag is a no-op and will be removed in a future release") + _ = compileCmd.Flags().MarkDeprecated("no-shellcheck", "shellcheck is now opt-in; use --shellcheck to enable it. This flag has no effect and will be removed in a future release") compileCmd.Flags().Bool("fix", false, "Apply automatic codemod fixes to workflows before compiling") compileCmd.Flags().BoolP("json", "j", false, "Output results in JSON format") compileCmd.Flags().Bool("show-all", false, "Display all compilation errors instead of only the highest-priority subset (default: top 5)") diff --git a/pkg/cli/compile_orchestrator.go b/pkg/cli/compile_orchestrator.go index 0fa1d7bcac7..28df2bfed1c 100644 --- a/pkg/cli/compile_orchestrator.go +++ b/pkg/cli/compile_orchestrator.go @@ -64,11 +64,11 @@ func CompileWorkflows(ctx context.Context, config CompileConfig) ([]*workflow.Wo initActionlintStats() } - // Warn or error when shellcheck is explicitly requested via --shellcheck but not installed. + // Warn or error when shellcheck is requested (via --shellcheck or --validate) but not installed. // Skip this check when --no-emit is set: no lock files are written so shellcheck // is never invoked. When the binary is absent, Docker is used as a fallback (lazy // — only when there are scripts to lint). Only warn/error when neither is available. - if config.Shellcheck && !config.NoEmit && !isShellcheckAvailable() { + if (config.Shellcheck || config.Validate) && !config.NoEmit && !isShellcheckAvailable() { if !IsDockerAvailable(ctx) { if config.Strict { return nil, errors.New("shellcheck not available: binary not found in PATH and Docker is not running; install shellcheck or start Docker to enable run step linting") diff --git a/pkg/cli/shellcheck.go b/pkg/cli/shellcheck.go index 0728fbe484b..90d49e5a6c0 100644 --- a/pkg/cli/shellcheck.go +++ b/pkg/cli/shellcheck.go @@ -74,6 +74,12 @@ func sanitizeGHAExpressions(script string) string { // body strings. var shellcheckDefaultIgnoreCodes = []string{"SC2016", "SC1090", "SC1091", "SC2002", "SC2129", "SC2153", "SC2154"} +// shellcheckMaxConcurrency is the maximum number of shellcheck processes (or +// Docker containers) that may run simultaneously. It bounds resource usage on +// large workflow sets that would otherwise exhaust process or file-descriptor +// limits. +const shellcheckMaxConcurrency = 8 + // runStepInfo captures the information from a single run: step in a lock file // that is needed to run shellcheck on the script snippet. type runStepInfo struct { @@ -217,11 +223,15 @@ func extractRunStepsFromLockFile(lockFile string) ([]runStepInfo, error) { } // runShellcheckOnScript writes script to a temporary file and invokes shellcheck. -// It prints any findings to stderr and returns a non-nil error when shellcheck -// reports one or more issues. -func runShellcheckOnScript(info runStepInfo, ignoreCodes []string, verbose bool) error { +// It returns any findings as a byte slice (ready to write to stderr) and a +// non-nil error when shellcheck reports one or more issues. Callers are +// responsible for writing the returned output to stderr so that concurrent +// calls do not interleave their diagnostic blocks. +func runShellcheckOnScript(info runStepInfo, ignoreCodes []string, verbose bool) ([]byte, error) { shellcheckLog.Printf("Running shellcheck on step %q (shell=%s)", info.Name, info.Shell) + var out bytes.Buffer + // Sanitize GitHub Actions ${{ ... }} expressions before writing the script. // Without this, shellcheck emits parse errors (SC1073, SC1083) because // ${{ is not valid POSIX/bash substitution syntax. @@ -230,13 +240,13 @@ func runShellcheckOnScript(info runStepInfo, ignoreCodes []string, verbose bool) // Write script to a temp file so shellcheck can lint it. tmpFile, err := os.CreateTemp("", "gh-aw-shellcheck-*.sh") if err != nil { - return fmt.Errorf("failed to create temp file for shellcheck: %w", err) + return nil, fmt.Errorf("failed to create temp file for shellcheck: %w", err) } defer os.Remove(tmpFile.Name()) if _, err := tmpFile.WriteString(sanitizedScript); err != nil { tmpFile.Close() - return fmt.Errorf("failed to write shellcheck temp file: %w", err) + return nil, fmt.Errorf("failed to write shellcheck temp file: %w", err) } tmpFile.Close() @@ -251,7 +261,7 @@ func runShellcheckOnScript(info runStepInfo, ignoreCodes []string, verbose bool) if verbose { shellcheckLog.Printf("Invoking: shellcheck %s", strings.Join(args, " ")) - fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage("shellcheck "+strings.Join(args[:len(args)-1], " ")+"