From 49ef08a5429ff0ead603efc92fbcb39676861290 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Wed, 8 Apr 2026 06:06:10 +0000
Subject: [PATCH 1/3] Initial plan


From 691b06f53b45be606411fe678d273b9ca0e04a69 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Wed, 8 Apr 2026 06:20:19 +0000
Subject: [PATCH 2/3] fix: when Docker is unavailable, compile still runs and
 adds warnings instead of marking valid workflows as invalid

When actionlint/zizmor/poutine is requested but Docker is unavailable,
the MCP compile tool previously returned valid:false for all workflows
without even attempting compilation. This was misleading because the
workflow itself compiled correctly.

Changes:
- Add DockerUnavailableError type in docker_images.go to distinguish
  persistent Docker unavailability from transient image-downloading errors
- CheckAndPrepareDockerImages now returns *DockerUnavailableError when
  Docker daemon is not running (vs errors.New for downloading)
- MCP compile tool handler detects DockerUnavailableError and falls back
  to running compile without the Docker tools, then injects a
  docker_unavailable warning into each result
- Add injectDockerUnavailableWarning helper to parse compile JSON output
  and append the warning without changing valid/invalid status
- Add unit tests for DockerUnavailableError type and injectDockerUnavailableWarning

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/efda6809-e1ec-497c-855d-ac6ff4161347

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
---
 pkg/cli/docker_images.go      | 17 ++++++-
 pkg/cli/docker_images_test.go | 96 +++++++++++++++++++++++++++++++++++
 pkg/cli/mcp_tools_readonly.go | 75 +++++++++++++++++++++++----
 3 files changed, 176 insertions(+), 12 deletions(-)

diff --git a/pkg/cli/docker_images.go b/pkg/cli/docker_images.go
index cb42f9eb914..83654675a42 100644
--- a/pkg/cli/docker_images.go
+++ b/pkg/cli/docker_images.go
@@ -14,6 +14,19 @@ import (
 
 var dockerImagesLog = logger.New("cli:docker_images")
 
+// DockerUnavailableError is returned when the Docker daemon is not accessible.
+// This is distinct from transient errors (e.g., images being downloaded) and signals
+// that Docker is not installed or not running on the host system.
+// Callers can use errors.As to check for this type and take appropriate action,
+// such as skipping static analysis but still running the compile step.
+type DockerUnavailableError struct {
+	Message string
+}
+
+func (e *DockerUnavailableError) Error() string {
+	return e.Message
+}
+
 // DockerImages defines the Docker images used by the compile tool's static analysis scanners
 const (
 	ZizmorImage     = "ghcr.io/zizmorcore/zizmor:latest"
@@ -220,7 +233,9 @@ func CheckAndPrepareDockerImages(ctx context.Context, useZizmor, usePoutine, use
 		if len(requestedTools) > 1 {
 			verb = "require"
 		}
-		return fmt.Errorf("docker is not available (cannot connect to Docker daemon). %s %s Docker. Please install and start Docker, or set %s to skip static analysis", strings.Join(requestedTools, " and "), verb, strings.Join(paramsList, " and "))
+		return &DockerUnavailableError{
+			Message: fmt.Sprintf("docker is not available (cannot connect to Docker daemon). %s %s Docker. Please install and start Docker, or set %s to skip static analysis", strings.Join(requestedTools, " and "), verb, strings.Join(paramsList, " and ")),
+		}
 	}
 
 	var missingImages []string
diff --git a/pkg/cli/docker_images_test.go b/pkg/cli/docker_images_test.go
index f7e2db4b9c3..d55c2dda053 100644
--- a/pkg/cli/docker_images_test.go
+++ b/pkg/cli/docker_images_test.go
@@ -4,6 +4,8 @@ package cli
 
 import (
 	"context"
+	"encoding/json"
+	"errors"
 	"strings"
 	"testing"
 	"time"
@@ -562,3 +564,97 @@ func TestIsDockerAvailable_MockFalse(t *testing.T) {
 	}
 	ResetDockerPullState()
 }
+
+func TestCheckAndPrepareDockerImages_DockerUnavailable_ReturnsTypedError(t *testing.T) {
+	// Reset state before test
+	ResetDockerPullState()
+	SetMockDockerAvailable(false)
+
+	err := CheckAndPrepareDockerImages(context.Background(), false, false, true)
+	if err == nil {
+		t.Fatal("Expected error when Docker is unavailable, got nil")
+	}
+
+	// Verify the error is the typed DockerUnavailableError so callers can distinguish
+	// it from transient errors (e.g., images downloading).
+	var dockerUnavailableErr *DockerUnavailableError
+	if !errors.As(err, &dockerUnavailableErr) {
+		t.Errorf("Expected error to be *DockerUnavailableError, got %T: %v", err, err)
+	}
+
+	// Clean up
+	ResetDockerPullState()
+}
+
+func TestInjectDockerUnavailableWarning_AddsWarningToValidResults(t *testing.T) {
+	// Simulate compile output where both workflows compiled successfully.
+	inputJSON := `[{"workflow":"a.md","valid":true,"errors":[],"warnings":[]},{"workflow":"b.md","valid":true,"errors":[],"warnings":[]}]`
+	warningMsg := "docker is not available (cannot connect to Docker daemon). actionlint requires Docker."
+
+	output := injectDockerUnavailableWarning(inputJSON, warningMsg)
+
+	var results []ValidationResult
+	if err := json.Unmarshal([]byte(output), &results); err != nil {
+		t.Fatalf("Failed to parse injected output: %v", err)
+	}
+
+	if len(results) != 2 {
+		t.Fatalf("Expected 2 results, got %d", len(results))
+	}
+
+	for _, r := range results {
+		if !r.Valid {
+			t.Errorf("Workflow %s should still be valid after Docker unavailable warning", r.Workflow)
+		}
+		if len(r.Warnings) != 1 {
+			t.Errorf("Workflow %s should have 1 warning, got %d", r.Workflow, len(r.Warnings))
+			continue
+		}
+		if r.Warnings[0].Type != "docker_unavailable" {
+			t.Errorf("Expected warning type 'docker_unavailable', got '%s'", r.Warnings[0].Type)
+		}
+		if r.Warnings[0].Message != warningMsg {
+			t.Errorf("Expected warning message %q, got %q", warningMsg, r.Warnings[0].Message)
+		}
+	}
+}
+
+func TestInjectDockerUnavailableWarning_PreservesInvalidResults(t *testing.T) {
+	// One workflow failed to compile; the other succeeded.
+	inputJSON := `[{"workflow":"bad.md","valid":false,"errors":[{"type":"parse_error","message":"syntax error"}],"warnings":[]},{"workflow":"good.md","valid":true,"errors":[],"warnings":[]}]`
+	warningMsg := "docker is not available"
+
+	output := injectDockerUnavailableWarning(inputJSON, warningMsg)
+
+	var results []ValidationResult
+	if err := json.Unmarshal([]byte(output), &results); err != nil {
+		t.Fatalf("Failed to parse injected output: %v", err)
+	}
+
+	if len(results) != 2 {
+		t.Fatalf("Expected 2 results, got %d", len(results))
+	}
+
+	// bad.md should remain invalid and still carry its original error.
+	if results[0].Valid {
+		t.Error("bad.md should remain invalid")
+	}
+	if len(results[0].Errors) != 1 || results[0].Errors[0].Type != "parse_error" {
+		t.Error("bad.md should still have its original parse_error")
+	}
+	// good.md should be valid with the warning appended.
+	if !results[1].Valid {
+		t.Error("good.md should still be valid")
+	}
+	if len(results[1].Warnings) != 1 || results[1].Warnings[0].Type != "docker_unavailable" {
+		t.Error("good.md should have the docker_unavailable warning")
+	}
+}
+
+func TestInjectDockerUnavailableWarning_InvalidJSONReturnedUnchanged(t *testing.T) {
+	invalidJSON := "not-valid-json"
+	output := injectDockerUnavailableWarning(invalidJSON, "some warning")
+	if output != invalidJSON {
+		t.Errorf("Expected original output to be returned unchanged for invalid JSON, got: %s", output)
+	}
+}
diff --git a/pkg/cli/mcp_tools_readonly.go b/pkg/cli/mcp_tools_readonly.go
index 0385ac2febe..8eadb2b456d 100644
--- a/pkg/cli/mcp_tools_readonly.go
+++ b/pkg/cli/mcp_tools_readonly.go
@@ -130,20 +130,38 @@ Returns JSON array with validation results for each workflow:
 		default:
 		}
 
+		// dockerUnavailableWarning is set when Docker is not accessible but the compile
+		// should still proceed without the static-analysis tools.  After a successful
+		// compile the warning is appended to every workflow result so the caller
+		// knows linting was skipped — without marking valid workflows as invalid.
+		var dockerUnavailableWarning string
+
 		// Check if any static analysis tools are requested that require Docker images
 		if args.Zizmor || args.Poutine || args.Actionlint {
 			// Check if Docker images are available; if not, start downloading and return retry message
 			if err := CheckAndPrepareDockerImages(ctx, args.Zizmor, args.Poutine, args.Actionlint); err != nil {
-				// 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())
-				jsonBytes, jsonErr := json.Marshal(results)
-				if jsonErr != nil {
-					return nil, nil, newMCPError(jsonrpc.CodeInternalError, "failed to marshal docker error results", jsonErr.Error())
+				var dockerUnavailableErr *DockerUnavailableError
+				if errors.As(err, &dockerUnavailableErr) {
+					// Docker daemon is not running.  Instead of failing every workflow,
+					// compile without the Docker-based tools and surface a warning so
+					// the caller knows static analysis was skipped.
+					dockerUnavailableWarning = err.Error()
+					args.Zizmor = false
+					args.Poutine = false
+					args.Actionlint = false
+				} else {
+					// 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())
+					jsonBytes, jsonErr := json.Marshal(results)
+					if jsonErr != nil {
+						return nil, nil, newMCPError(jsonrpc.CodeInternalError, "failed to marshal docker error results", jsonErr.Error())
+					}
+					return &mcp.CallToolResult{
+						Content: []mcp.Content{&mcp.TextContent{Text: string(jsonBytes)}},
+					}, nil, nil
 				}
-				return &mcp.CallToolResult{
-					Content: []mcp.Content{&mcp.TextContent{Text: string(jsonBytes)}},
-				}, nil, nil
 			}
 
 			// Check for cancellation after Docker image preparation
@@ -220,6 +238,13 @@ Returns JSON array with validation results for each workflow:
 			// and return it to the LLM
 		}
 
+		// When Docker was unavailable, inject a warning into every workflow result so the
+		// caller knows that static analysis was skipped — but does NOT mark valid
+		// workflows as invalid.
+		if dockerUnavailableWarning != "" {
+			outputStr = injectDockerUnavailableWarning(outputStr, dockerUnavailableWarning)
+		}
+
 		return &mcp.CallToolResult{
 			Content: []mcp.Content{
 				&mcp.TextContent{Text: outputStr},
@@ -380,8 +405,9 @@ 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 is unavailable so the compile tool returns consistent
-// structured JSON instead of a protocol-level error.
+// 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 {
 	// Determine which workflow names to report
 	var workflowNames []string
@@ -426,3 +452,30 @@ func buildDockerErrorResults(requestedWorkflows []string, errMsg string) []Valid
 	}
 	return results
 }
+
+// injectDockerUnavailableWarning parses the JSON compile output and appends a
+// "docker_unavailable" warning to every workflow result.  It is used when Docker
+// is not running so the caller knows static analysis was skipped, while preserving
+// 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 {
+	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)
+	}
+
+	jsonBytes, err := json.Marshal(results)
+	if err != nil {
+		return outputStr
+	}
+	return string(jsonBytes)
+}

From 46bf487dc832fa02662e5ea722e8de005953a66e Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Wed, 8 Apr 2026 12:09:20 +0000
Subject: [PATCH 3/3] refactor: address review feedback - fix comment wording
 and move tests to mcp_tools_readonly_test.go
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Fix misleading comment on dockerUnavailableWarning: "after a successful compile"
  → "after the compile attempt, preserving each workflow's valid/invalid status"
- Move injectDockerUnavailableWarning tests from docker_images_test.go to new
  mcp_tools_readonly_test.go to match the file-per-feature test layout convention

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/adcf9dad-a2cd-496a-9de7-dfab6dc8c2a7

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
---
 pkg/cli/docker_images_test.go      | 74 ---------------------------
 pkg/cli/mcp_tools_readonly.go      |  7 +--
 pkg/cli/mcp_tools_readonly_test.go | 81 ++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 77 deletions(-)
 create mode 100644 pkg/cli/mcp_tools_readonly_test.go

diff --git a/pkg/cli/docker_images_test.go b/pkg/cli/docker_images_test.go
index d55c2dda053..0b9b9d776d2 100644
--- a/pkg/cli/docker_images_test.go
+++ b/pkg/cli/docker_images_test.go
@@ -4,7 +4,6 @@ package cli
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"strings"
 	"testing"
@@ -585,76 +584,3 @@ func TestCheckAndPrepareDockerImages_DockerUnavailable_ReturnsTypedError(t *test
 	// Clean up
 	ResetDockerPullState()
 }
-
-func TestInjectDockerUnavailableWarning_AddsWarningToValidResults(t *testing.T) {
-	// Simulate compile output where both workflows compiled successfully.
-	inputJSON := `[{"workflow":"a.md","valid":true,"errors":[],"warnings":[]},{"workflow":"b.md","valid":true,"errors":[],"warnings":[]}]`
-	warningMsg := "docker is not available (cannot connect to Docker daemon). actionlint requires Docker."
-
-	output := injectDockerUnavailableWarning(inputJSON, warningMsg)
-
-	var results []ValidationResult
-	if err := json.Unmarshal([]byte(output), &results); err != nil {
-		t.Fatalf("Failed to parse injected output: %v", err)
-	}
-
-	if len(results) != 2 {
-		t.Fatalf("Expected 2 results, got %d", len(results))
-	}
-
-	for _, r := range results {
-		if !r.Valid {
-			t.Errorf("Workflow %s should still be valid after Docker unavailable warning", r.Workflow)
-		}
-		if len(r.Warnings) != 1 {
-			t.Errorf("Workflow %s should have 1 warning, got %d", r.Workflow, len(r.Warnings))
-			continue
-		}
-		if r.Warnings[0].Type != "docker_unavailable" {
-			t.Errorf("Expected warning type 'docker_unavailable', got '%s'", r.Warnings[0].Type)
-		}
-		if r.Warnings[0].Message != warningMsg {
-			t.Errorf("Expected warning message %q, got %q", warningMsg, r.Warnings[0].Message)
-		}
-	}
-}
-
-func TestInjectDockerUnavailableWarning_PreservesInvalidResults(t *testing.T) {
-	// One workflow failed to compile; the other succeeded.
-	inputJSON := `[{"workflow":"bad.md","valid":false,"errors":[{"type":"parse_error","message":"syntax error"}],"warnings":[]},{"workflow":"good.md","valid":true,"errors":[],"warnings":[]}]`
-	warningMsg := "docker is not available"
-
-	output := injectDockerUnavailableWarning(inputJSON, warningMsg)
-
-	var results []ValidationResult
-	if err := json.Unmarshal([]byte(output), &results); err != nil {
-		t.Fatalf("Failed to parse injected output: %v", err)
-	}
-
-	if len(results) != 2 {
-		t.Fatalf("Expected 2 results, got %d", len(results))
-	}
-
-	// bad.md should remain invalid and still carry its original error.
-	if results[0].Valid {
-		t.Error("bad.md should remain invalid")
-	}
-	if len(results[0].Errors) != 1 || results[0].Errors[0].Type != "parse_error" {
-		t.Error("bad.md should still have its original parse_error")
-	}
-	// good.md should be valid with the warning appended.
-	if !results[1].Valid {
-		t.Error("good.md should still be valid")
-	}
-	if len(results[1].Warnings) != 1 || results[1].Warnings[0].Type != "docker_unavailable" {
-		t.Error("good.md should have the docker_unavailable warning")
-	}
-}
-
-func TestInjectDockerUnavailableWarning_InvalidJSONReturnedUnchanged(t *testing.T) {
-	invalidJSON := "not-valid-json"
-	output := injectDockerUnavailableWarning(invalidJSON, "some warning")
-	if output != invalidJSON {
-		t.Errorf("Expected original output to be returned unchanged for invalid JSON, got: %s", output)
-	}
-}
diff --git a/pkg/cli/mcp_tools_readonly.go b/pkg/cli/mcp_tools_readonly.go
index 8eadb2b456d..65ba0d3725b 100644
--- a/pkg/cli/mcp_tools_readonly.go
+++ b/pkg/cli/mcp_tools_readonly.go
@@ -131,9 +131,10 @@ Returns JSON array with validation results for each workflow:
 		}
 
 		// dockerUnavailableWarning is set when Docker is not accessible but the compile
-		// should still proceed without the static-analysis tools.  After a successful
-		// compile the warning is appended to every workflow result so the caller
-		// knows linting was skipped — without marking valid workflows as invalid.
+		// should still proceed without the static-analysis tools.  After the compile
+		// attempt, the warning is appended to workflow results in the JSON output so
+		// the caller knows linting was skipped, while preserving each workflow's
+		// valid/invalid status.
 		var dockerUnavailableWarning string
 
 		// Check if any static analysis tools are requested that require Docker images
diff --git a/pkg/cli/mcp_tools_readonly_test.go b/pkg/cli/mcp_tools_readonly_test.go
new file mode 100644
index 00000000000..f7a5be2188a
--- /dev/null
+++ b/pkg/cli/mcp_tools_readonly_test.go
@@ -0,0 +1,81 @@
+//go:build !integration
+
+package cli
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+func TestInjectDockerUnavailableWarning_AddsWarningToValidResults(t *testing.T) {
+	// Simulate compile output where both workflows compiled successfully.
+	inputJSON := `[{"workflow":"a.md","valid":true,"errors":[],"warnings":[]},{"workflow":"b.md","valid":true,"errors":[],"warnings":[]}]`
+	warningMsg := "docker is not available (cannot connect to Docker daemon). actionlint requires Docker."
+
+	output := injectDockerUnavailableWarning(inputJSON, warningMsg)
+
+	var results []ValidationResult
+	if err := json.Unmarshal([]byte(output), &results); err != nil {
+		t.Fatalf("Failed to parse injected output: %v", err)
+	}
+
+	if len(results) != 2 {
+		t.Fatalf("Expected 2 results, got %d", len(results))
+	}
+
+	for _, r := range results {
+		if !r.Valid {
+			t.Errorf("Workflow %s should still be valid after Docker unavailable warning", r.Workflow)
+		}
+		if len(r.Warnings) != 1 {
+			t.Errorf("Workflow %s should have 1 warning, got %d", r.Workflow, len(r.Warnings))
+			continue
+		}
+		if r.Warnings[0].Type != "docker_unavailable" {
+			t.Errorf("Expected warning type 'docker_unavailable', got '%s'", r.Warnings[0].Type)
+		}
+		if r.Warnings[0].Message != warningMsg {
+			t.Errorf("Expected warning message %q, got %q", warningMsg, r.Warnings[0].Message)
+		}
+	}
+}
+
+func TestInjectDockerUnavailableWarning_PreservesInvalidResults(t *testing.T) {
+	// One workflow failed to compile; the other succeeded.
+	inputJSON := `[{"workflow":"bad.md","valid":false,"errors":[{"type":"parse_error","message":"syntax error"}],"warnings":[]},{"workflow":"good.md","valid":true,"errors":[],"warnings":[]}]`
+	warningMsg := "docker is not available"
+
+	output := injectDockerUnavailableWarning(inputJSON, warningMsg)
+
+	var results []ValidationResult
+	if err := json.Unmarshal([]byte(output), &results); err != nil {
+		t.Fatalf("Failed to parse injected output: %v", err)
+	}
+
+	if len(results) != 2 {
+		t.Fatalf("Expected 2 results, got %d", len(results))
+	}
+
+	// bad.md should remain invalid and still carry its original error.
+	if results[0].Valid {
+		t.Error("bad.md should remain invalid")
+	}
+	if len(results[0].Errors) != 1 || results[0].Errors[0].Type != "parse_error" {
+		t.Error("bad.md should still have its original parse_error")
+	}
+	// good.md should be valid with the warning appended.
+	if !results[1].Valid {
+		t.Error("good.md should still be valid")
+	}
+	if len(results[1].Warnings) != 1 || results[1].Warnings[0].Type != "docker_unavailable" {
+		t.Error("good.md should have the docker_unavailable warning")
+	}
+}
+
+func TestInjectDockerUnavailableWarning_InvalidJSONReturnedUnchanged(t *testing.T) {
+	invalidJSON := "not-valid-json"
+	output := injectDockerUnavailableWarning(invalidJSON, "some warning")
+	if output != invalidJSON {
+		t.Errorf("Expected original output to be returned unchanged for invalid JSON, got: %s", output)
+	}
+}
