feat: shellcheck Docker fallback for systems without native binary; re-enable in MCP

docs/adr/49826-shellcheck-docker-fallback-and-mcp-reenable.md

@@ -0,0 +1,60 @@
+# ADR-49826: Shellcheck Docker Fallback for Systems Without Native Binary; Re-enable in MCP
+
+**Date**: 2026-08-02
+**Status**: Draft
+**Deciders**: pelikhan, copilot-swe-agent
+
+---
+
+### Context
+
+The shellcheck integration in the compile pipeline (introduced in ADR-49762) relied exclusively on a native shellcheck binary being present in PATH. On platforms that lack a native package (notably Windows), shellcheck was silently skipped, leaving run: step linting inoperative for those users. Separately, MCP compilation explicitly disabled shellcheck via `--no-shellcheck` on the assumption that shellcheck output would not reach the LLM — but this also prevented any shell script validation from occurring in that context.
+
+The goal is to make shellcheck linting reliable across all supported platforms and re-enable it in MCP compilation, without requiring users to install shellcheck natively.
+
+### Decision
+
+We will add a lazy Docker container fallback for shellcheck (`koalaman/shellcheck:v0.10.0`, SHA-pinned) that is invoked only when the native binary is absent and Docker is available. The Docker path pipes scripts via stdin (no volume mount required). In parallel, we will remove `--no-shellcheck` from MCP compilation so that shellcheck runs during MCP compilation via whichever path (binary or Docker) is available.
+
+The fallback precedence is: native binary → Docker container → silent skip. Warnings and errors in `--strict`/`--validate` mode fire only when neither path is available.
+
+### Alternatives Considered
+
+#### Alternative 1: Require Native Binary Installation
+
+Continue requiring users to install shellcheck natively on every platform. Document the requirement more prominently, and surface a clear error on unsupported platforms.
+
+Rejected because it creates a hard barrier for Windows users and automated MCP environments where installing system packages is not feasible. The user experience degrades from "linting is silently skipped" to "an error is shown", with no practical linting benefit on those platforms.
+
+#### Alternative 2: Keep Shellcheck Disabled in MCP
+
+Continue passing `--no-shellcheck` during MCP compilation, accepting that LLM-generated workflows are never linted for shell script quality.
+
+Rejected because the original rationale (shellcheck output not reaching the LLM via JSON response) was addressed by the Docker fallback design: shellcheck now has a reliable execution path in any environment that has Docker running, which covers the MCP use case.
+
+#### Alternative 3: Require Docker Pull at Startup
+
+Pull the shellcheck Docker image eagerly at compile startup rather than lazily on first use.
+
+Rejected in favor of the lazy pull: Docker is only contacted when there are actual bash/sh run: steps to lint. An eager pull would add latency to all compilations, including those with no shell scripts.
+
+### Consequences
+
+#### Positive
+- Shellcheck linting is now available on all platforms that have Docker, including Windows.
+- MCP-compiled workflows receive shell script validation, improving quality of AI-generated workflow steps.
+- The Docker image is SHA-pinned, preventing unexpected behavior from upstream image changes.
+- No temporary files or volume mounts are required; scripts are piped via stdin.
+
+#### Negative
+- Docker must be running for the fallback to work; users without Docker on non-PATH platforms still get silent skips.
+- The first use of the fallback triggers a Docker image pull, adding latency that is invisible to the user.
+- Adding Docker as an implicit runtime dependency for shellcheck increases the surface area of the tool's external dependencies.
+
+#### Neutral
+- MCP shellcheck output goes to stderr (not the JSON response body), so the LLM does not see individual findings — only whether compilation succeeded or failed.
+- The existing `--no-shellcheck` flag remains available for users who want to opt out of linting entirely.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/cli/compile_external_tools.go

@@ -85,8 +85,14 @@ func RunYamllintOnFiles(lockFiles []string, verbose bool, strict bool) error {
 // from the provided lock files. Shellcheck must be installed as a system binary;
 // unlike other tools it does not use Docker. When shellcheck is not available
 // the function returns nil (callers are responsible for warning the user).
-func RunShellcheckOnLockFiles(lockFiles []string, verbose bool, strict bool) error {
-	return runBatchLockFileTool("shellcheck", lockFiles, verbose, strict, runShellcheckOnLockFiles)
+func RunShellcheckOnLockFiles(ctx context.Context, lockFiles []string, verbose bool, strict bool) error {
+	if len(lockFiles) == 0 {
+		compileExternalToolsLog.Printf("No lock files to process with shellcheck")
+		return nil
+	}
+
+	compileExternalToolsLog.Printf("Running batch shellcheck on %d lock files", len(lockFiles))
+	return handleBatchToolError("shellcheck", runShellcheckOnLockFiles(ctx, lockFiles, verbose, strict), strict, verbose)
 }
 
 // RunSyftOnLockFiles runs the syft SBOM scanner on container images extracted

pkg/cli/compile_orchestrator.go

@@ -67,11 +67,15 @@ func CompileWorkflows(ctx context.Context, config CompileConfig) ([]*workflow.Wo
 	// Warn or error when shellcheck is enabled (the default) 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() {
-		if config.Strict {
-			return nil, errors.New("shellcheck binary not found in PATH; run step linting requires shellcheck to be installed (use --no-shellcheck to skip)")
-		} else if config.Validate {
-			fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("shellcheck binary not found in PATH; run step linting will be skipped. Install shellcheck to enable run step linting."))
+		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 {
+				fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("shellcheck binary not found in PATH and Docker is not running; run step linting will be skipped"))
+			}
 		}
 	}
 

pkg/cli/compile_pipeline.go

@@ -301,7 +301,7 @@ func compileSpecificFiles(
 		if err := ctx.Err(); err != nil {
 			return workflowDataList, err
 		}
-		if err := RunShellcheckOnLockFiles(lockFilesForShellcheck, config.Verbose && !config.JSONOutput, config.Strict); err != nil {
+		if err := RunShellcheckOnLockFiles(ctx, lockFilesForShellcheck, config.Verbose && !config.JSONOutput, config.Strict); err != nil {
 			if config.Strict {
 				return workflowDataList, err
 			}
@@ -609,7 +609,7 @@ func compileAllFilesInDirectory(
 		if err := ctx.Err(); err != nil {
 			return workflowDataList, err
 		}
-		if err := RunShellcheckOnLockFiles(lockFilesForShellcheck, config.Verbose && !config.JSONOutput, config.Strict); err != nil {
+		if err := RunShellcheckOnLockFiles(ctx, lockFilesForShellcheck, config.Verbose && !config.JSONOutput, config.Strict); err != nil {
 			if config.Strict {
 				return workflowDataList, err
 			}

pkg/cli/docker_images.go

@@ -37,6 +37,9 @@ const (
 	GrypeImage       = "anchore/grype:v0.116.1@sha256:1e71065c0a4cff3e6bd3b8add525ffac4343eb4971694eb90a31cf6d4d3e85db"
 	GrantImage       = "anchore/grant:v0.6.8@sha256:172463611795f43b77302cdfbd7b3f81295492a7330e0820cfe41c3674920237"
 	YamllintImage    = "pipelinecomponents/yamllint:latest@sha256:5ab5eb7da0ed5e606b07c1723fc8b275e925189f70ac259b26b7329cb5f8f44d"
+	// ShellcheckImage is the Docker fallback for shellcheck when the binary is not installed.
+	// Used automatically when the shellcheck binary is not found in PATH (e.g. on Windows).
+	ShellcheckImage = "koalaman/shellcheck:v0.10.0@sha256:2097951f02e735b613f4a34de20c40f937a6c8f18ecb170612c88c34517221fb"
 )
 
 // inflightDownload holds the join channel and result for an in-progress pull.

pkg/cli/docker_images_test.go

@@ -113,6 +113,9 @@ func TestDockerImageConstants(t *testing.T) {
 	if GrantImage == "" {
 		t.Error("GrantImage constant should not be empty")
 	}
+	if ShellcheckImage == "" {
+		t.Error("ShellcheckImage constant should not be empty")
+	}
 
 	// Verify they are docker image references
 	expectedImages := map[string]string{
@@ -123,6 +126,7 @@ func TestDockerImageConstants(t *testing.T) {
 		"syft":         SyftImage,
 		"grype":        GrypeImage,
 		"grant":        GrantImage,
+		"shellcheck":   ShellcheckImage,
 	}
 
 	for name, image := range expectedImages {

pkg/cli/mcp_subprocess_guardrail.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"bytes"
 	"context"
 	"os/exec"
 
@@ -85,3 +86,19 @@ func runMCPExecOutput(ctx context.Context, execCmd execCmdFunc, args ...string)
 func runMCPExecCombinedOutput(ctx context.Context, execCmd execCmdFunc, args ...string) ([]byte, error) {
 	return runMCPSubprocessCombinedOutput(ctx, execCmd(ctx, args...))
 }
+
+func runMCPExecOutputWithStderr(ctx context.Context, execCmd execCmdFunc, args ...string) ([]byte, []byte, error) {
+	cmd := execCmd(ctx, args...)
+
+	if err := defaultMCPSubprocessGuardrail.acquire(ctx); err != nil {
+		return nil, nil, err
+	}
+	defer defaultMCPSubprocessGuardrail.release()
+
+	var stdout, stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+
+	err := cmd.Run()
+	return stdout.Bytes(), stderr.Bytes(), err
+}

pkg/cli/mcp_tools_readonly.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"os/exec"
 	"path/filepath"
+	"strings"
 
 	"github.com/modelcontextprotocol/go-sdk/jsonrpc"
 	"github.com/modelcontextprotocol/go-sdk/mcp"
@@ -140,6 +141,7 @@ 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 {
@@ -172,7 +174,7 @@ Returns JSON array with validation results for each workflow:
 					// Images are still downloading — ask the caller to retry.
 					// Build per-workflow validation errors instead of throwing an MCP protocol error,
 					// so callers always receive consistent JSON regardless of the failure mode.
-					results := buildDockerErrorResults(args.Workflows, err.Error())
+					results := buildCompileErrorResults(args.Workflows, err.Error())
 					jsonBytes, jsonErr := json.Marshal(results)
 					if jsonErr != nil {
 						return nil, nil, newMCPError(jsonrpc.CodeInternalError, "failed to marshal docker error results", jsonErr.Error())
@@ -193,9 +195,15 @@ Returns JSON array with validation results for each workflow:
 
 		// Build command arguments
 		// Always validate workflows during compilation and use JSON output for MCP.
-		// Shellcheck output goes to stderr (captured in subprocess error buffer only) and
-		// therefore never reaches the LLM via the JSON response — always disable it.
-		cmdArgs := []string{"compile", "--validate", "--json", "--no-shellcheck"}
+		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 {
@@ -248,7 +256,7 @@ Returns JSON array with validation results for each workflow:
 		// Use separate stdout/stderr capture instead of CombinedOutput because:
 		// - Stdout contains JSON output (--json flag)
 		// - Stderr contains console messages that shouldn't be mixed with JSON
-		stdout, err := runMCPExecOutput(ctx, execCmd, cmdArgs...)
+		stdout, stderr, err := runMCPExecOutputWithStderr(ctx, execCmd, cmdArgs...)
 
 		// The compile command always outputs JSON to stdout when --json flag is used, even on error.
 		// We should return the JSON output to the LLM so it can see validation errors.
@@ -262,12 +270,26 @@ Returns JSON array with validation results for each workflow:
 			// If we have no output, this is a real execution failure
 			if outputStr == "" {
 				// Try to get stderr for error details
-				var stderr string
+				var stderrText string
 				var exitErr *exec.ExitError
 				if errors.As(err, &exitErr) {
-					stderr = string(exitErr.Stderr)
+					stderrText = string(exitErr.Stderr)
+				}
+				if strings.TrimSpace(stderrText) == "" {
+					stderrText = string(stderr)
+				}
+				errMsg := strings.TrimSpace(stderrText)
+				if errMsg == "" {
+					errMsg = err.Error()
 				}
-				return nil, nil, newMCPError(jsonrpc.CodeInternalError, "failed to compile workflows", map[string]any{"error": err.Error(), "stderr": stderr})
+				results := buildCompileErrorResults(args.Workflows, errMsg)
+				jsonBytes, jsonErr := json.Marshal(results)
+				if jsonErr != nil {
+					return nil, nil, newMCPError(jsonrpc.CodeInternalError, "failed to marshal compile error results", jsonErr.Error())
+				}
+				return &mcp.CallToolResult{
+					Content: []mcp.Content{&mcp.TextContent{Text: string(jsonBytes)}},
+				}, nil, nil
 			}
 			// Otherwise, we have output (likely validation errors in JSON), so continue
 			// and return it to the LLM
@@ -279,6 +301,10 @@ 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{
 			Content: []mcp.Content{
@@ -436,11 +462,10 @@ Also returns pr_number, head_sha, check_runs, statuses, and total_count.`,
 	})
 }
 
-// buildDockerErrorResults builds a []ValidationResult with a config_error for each target
-// workflow. It is used when Docker images are still being downloaded (transient error) so
-// the compile tool returns consistent structured JSON instead of a protocol-level error.
-// For the persistent case where Docker is not available at all, see injectDockerUnavailableWarning.
-func buildDockerErrorResults(requestedWorkflows []string, errMsg string) []ValidationResult {
+// buildCompileErrorResults builds a []ValidationResult with a config_error for each target
+// workflow. It is used when the compile subprocess cannot return JSON output, so the compile
+// tool can still return consistent structured JSON instead of a protocol-level error.
+func buildCompileErrorResults(requestedWorkflows []string, errMsg string) []ValidationResult {
 	// Determine which workflow names to report
 	var workflowNames []string
 	if len(requestedWorkflows) > 0 {
@@ -491,18 +516,81 @@ func buildDockerErrorResults(requestedWorkflows []string, errMsg string) []Valid
 // the compile-time valid/invalid status of each workflow.
 // If the JSON cannot be parsed the original output is returned unchanged.
 func injectDockerUnavailableWarning(outputStr, warningMsg string) string {
+	return injectValidationWarning(outputStr, CompileValidationError{
+		Type:    "docker_unavailable",
+		Message: warningMsg,
+	})
+}
+
+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 {
+		return outputStr
+	}
+
+	warnings := make([]CompileValidationError, 0, len(diagnostics))
+	for _, diagnostic := range diagnostics {
+		warnings = append(warnings, CompileValidationError{
+			Type:    "shellcheck",
+			Message: diagnostic,
+		})
+	}
+	return injectValidationWarnings(outputStr, warnings)
+}
+
+func extractShellcheckDiagnostics(stderrOutput string) []string {
+	if stderrOutput == "" || !strings.Contains(stderrOutput, "shellcheck findings in ") {
+		return nil
+	}
+
+	lines := strings.Split(stderrOutput, "\n")
+	diagnostics := make([]string, 0)
+	current := ""
+
+	flush := func() {
+		if strings.TrimSpace(current) != "" {
+			diagnostics = append(diagnostics, strings.TrimSpace(current))
+		}
+		current = ""
+	}
+
+	for _, line := range lines {
+		trimmed := strings.TrimSpace(line)
+		switch {
+		case strings.Contains(trimmed, "shellcheck findings in "):
+			flush()
+			current = trimmed
+		case current != "" && (strings.Contains(trimmed, "script:") || strings.HasPrefix(trimmed, "script ")):
+			current += "\n" + trimmed
+		case current != "" && trimmed == "":
+			flush()
+		}
+	}
+	flush()
+
+	return diagnostics
+}
+
+func injectValidationWarnings(outputStr string, warnings []CompileValidationError) string {
+	if len(warnings) == 0 {
+		return outputStr
+	}
+
 	var results []ValidationResult
 	if err := json.Unmarshal([]byte(outputStr), &results); err != nil {
 		// Can't parse — return original output so we don't lose information.
 		return outputStr
 	}
 
-	warning := CompileValidationError{
-		Type:    "docker_unavailable",
-		Message: warningMsg,
-	}
 	for i := range results {
-		results[i].Warnings = append(results[i].Warnings, warning)
+		results[i].Warnings = append(results[i].Warnings, warnings...)
 	}
 
 	jsonBytes, err := json.Marshal(results)
@@ -511,3 +599,7 @@ func injectDockerUnavailableWarning(outputStr, warningMsg string) string {
 	}
 	return string(jsonBytes)
 }
+
+func injectValidationWarning(outputStr string, warning CompileValidationError) string {
+	return injectValidationWarnings(outputStr, []CompileValidationError{warning})
+}