diff --git a/pkg/cli/mcp_server_compile_test.go b/pkg/cli/mcp_server_compile_test.go index 1cf973b557d..01c01b13548 100644 --- a/pkg/cli/mcp_server_compile_test.go +++ b/pkg/cli/mcp_server_compile_test.go @@ -479,8 +479,8 @@ This workflow has an unknown field. t.Logf("Compile tool handled multiple workflows correctly: %d results", len(results)) } -// TestMCPServer_CompileToolWithStrictMode tests compile with strict mode flag - +// TestMCPServer_CompileToolWithStrictMode tests that compile refuses strict=true +// since the MCP tool does not support strict mode (use gh aw compile --strict from the CLI). func TestMCPServer_CompileToolWithStrictMode(t *testing.T) { // Skip if the binary doesn't exist binaryPath := "../../gh-aw" @@ -539,31 +539,24 @@ This workflow has strict mode disabled in frontmatter. } defer session.Close() - // Call compile tool with strict mode enabled + // Call compile tool with strict=true — the MCP server should refuse this. params := &mcp.CallToolParams{ Name: "compile", Arguments: map[string]any{ "strict": true, }, } - result, err := session.CallTool(ctx, params) + _, err = session.CallTool(ctx, params) - // Should not return MCP error - if err != nil { - t.Errorf("Compile tool should not return MCP error with strict flag, got: %v", err) - } - - // Verify we got results - if result == nil || len(result.Content) == 0 { - t.Fatal("Expected non-empty result content") - } - - textContent, ok := result.Content[0].(*mcp.TextContent) - if !ok { - t.Fatal("Expected text content from compile tool") + // Should return an MCP error because strict=true is not supported via the MCP tool. + if err == nil { + t.Error("Compile tool should return an error when strict=true is passed") + } else { + t.Logf("Compile tool correctly refused strict=true: %v", err) + if !strings.Contains(err.Error(), "strict") { + t.Errorf("Expected error message to mention 'strict', got: %v", err) + } } - - t.Logf("Compile tool with strict mode returned: %s", textContent.Text) } // TestMCPServer_CompileToolWithSpecificWorkflows tests compiling specific workflows by name diff --git a/pkg/cli/mcp_server_defaults_test.go b/pkg/cli/mcp_server_defaults_test.go index 9cd4f9f1d16..69e8ba3ea17 100644 --- a/pkg/cli/mcp_server_defaults_test.go +++ b/pkg/cli/mcp_server_defaults_test.go @@ -12,10 +12,10 @@ import ( // TestMCPToolElicitationDefaults verifies that MCP tools have appropriate // elicitation defaults configured according to SEP-1024. func TestMCPToolElicitationDefaults(t *testing.T) { - t.Run("compile tool has strict default", func(t *testing.T) { + t.Run("compile tool has no strict default (strict is not supported via MCP)", func(t *testing.T) { type compileArgs struct { Workflows []string `json:"workflows,omitempty" jsonschema:"Workflow files to compile (empty for all)"` - Strict bool `json:"strict,omitempty" jsonschema:"Override frontmatter to enforce strict mode validation for all workflows"` + Strict bool `json:"strict,omitempty" jsonschema:"Deprecated: not supported via the MCP tool"` Zizmor bool `json:"zizmor,omitempty" jsonschema:"Run zizmor security scanner on generated .lock.yml files"` Poutine bool `json:"poutine,omitempty" jsonschema:"Run poutine security scanner on generated .lock.yml files"` Actionlint bool `json:"actionlint,omitempty" jsonschema:"Run actionlint linter on generated .lock.yml files"` @@ -28,28 +28,14 @@ func TestMCPToolElicitationDefaults(t *testing.T) { t.Fatalf("Failed to generate schema: %v", err) } - // Add default as done in createMCPServer - if err := AddSchemaDefault(schema, "strict", true); err != nil { - t.Fatalf("Failed to add default: %v", err) - } - - // Verify the default was added + // Verify no default is set for strict (the MCP compile tool refuses strict=true at runtime) strictProp, ok := schema.Properties["strict"] if !ok { t.Fatal("Expected 'strict' property to exist") } - if len(strictProp.Default) == 0 { - t.Error("Expected 'strict' property to have a default value") - } - - var strictDefault bool - if err := json.Unmarshal(strictProp.Default, &strictDefault); err != nil { - t.Fatalf("Failed to unmarshal strict default: %v", err) - } - - if !strictDefault { - t.Errorf("Expected strict default to be true, got %v", strictDefault) + if len(strictProp.Default) != 0 { + t.Errorf("Expected 'strict' property to have no schema default, got %s", strictProp.Default) } }) diff --git a/pkg/cli/mcp_server_unit_test.go b/pkg/cli/mcp_server_unit_test.go index 48eaacb4bc1..312706a7d31 100644 --- a/pkg/cli/mcp_server_unit_test.go +++ b/pkg/cli/mcp_server_unit_test.go @@ -123,4 +123,28 @@ func TestMCPServerUnit_CompileTool(t *testing.T) { require.NotEmpty(t, capturedArgs, "execCmd should have been called") assert.Equal(t, "compile", capturedArgs[0], "first arg should be 'compile'") assert.Contains(t, strings.Join(capturedArgs, " "), "--json", "compile should pass --json flag") + assert.NotContains(t, strings.Join(capturedArgs, " "), "--strict", "compile should not pass --strict flag by default") +} + +// TestMCPServerUnit_CompileToolRejectsStrict verifies that the compile tool +// returns an error when strict=true is passed, since the MCP tool does not +// support strict mode compilation (use gh aw compile --strict from the CLI instead). +func TestMCPServerUnit_CompileToolRejectsStrict(t *testing.T) { + mockExecCmd := func(ctx context.Context, args ...string) *exec.Cmd { + // Should never be called when strict=true is rejected + t.Error("execCmd should not be called when strict=true is rejected") + return exec.CommandContext(ctx, "false") + } + + server := mcp.NewServer(&mcp.Implementation{Name: "gh-aw", Version: "test"}, nil) + require.NoError(t, registerCompileTool(server, mockExecCmd, ""), "registerCompileTool should succeed") + session := connectInMemory(t, server) + + ctx := context.Background() + _, err := session.CallTool(ctx, &mcp.CallToolParams{ + Name: "compile", + Arguments: map[string]any{"strict": true}, + }) + require.Error(t, err, "compile tool should return an error when strict=true is passed") + assert.Contains(t, err.Error(), "strict", "error message should mention strict") } diff --git a/pkg/cli/mcp_tools_readonly.go b/pkg/cli/mcp_tools_readonly.go index f4336e8b855..314e97a6fc7 100644 --- a/pkg/cli/mcp_tools_readonly.go +++ b/pkg/cli/mcp_tools_readonly.go @@ -71,7 +71,7 @@ Returns a JSON array where each element has the following structure: // compileArgs holds the input parameters for the compile tool. type compileArgs struct { Workflows []string `json:"workflows,omitempty" jsonschema:"Workflow files to compile (empty for all)"` - Strict bool `json:"strict,omitempty" jsonschema:"Override frontmatter to enforce strict mode validation for all workflows. Note: Workflows default to strict mode unless frontmatter sets strict: false"` + Strict bool `json:"strict,omitempty" jsonschema:"Deprecated: not supported via the MCP tool. Use gh aw compile --strict for strict mode compilation."` Zizmor bool `json:"zizmor,omitempty" jsonschema:"Run zizmor security scanner on generated .lock.yml files"` Poutine bool `json:"poutine,omitempty" jsonschema:"Run poutine security scanner on generated .lock.yml files"` Actionlint bool `json:"actionlint,omitempty" jsonschema:"Run actionlint linter on generated .lock.yml files"` @@ -91,16 +91,13 @@ type compileArgs struct { // enforcement. An empty string disables this feature. // Returns an error if schema generation fails, which causes the server to stop registering tools. func registerCompileTool(server *mcp.Server, execCmd execCmdFunc, manifestCacheFile string) error { - // Generate schema with elicitation defaults + // Generate schema without a strict default: the MCP compile tool does not + // support strict mode (strict: true is refused at runtime). compileSchema, err := GenerateSchema[compileArgs]() if err != nil { mcpLog.Printf("Failed to generate compile tool schema: %v", err) return err } - // Add elicitation default: strict defaults to true (most common case) - if err := AddSchemaDefault(compileSchema, "strict", true); err != nil { - mcpLog.Printf("Failed to add default for strict: %v", err) - } mcp.AddTool(server, &mcp.Tool{ Name: "compile", @@ -115,9 +112,8 @@ func registerCompileTool(server *mcp.Server, execCmd execCmdFunc, manifestCacheF This tool generates .lock.yml files from .md workflow files. The .lock.yml files are what GitHub Actions actually executes, so failing to compile after modifying a .md file means your changes won't take effect. -Workflows use strict mode validation by default (unless frontmatter sets strict: false). -Strict mode enforces: action pinning to SHAs, explicit network config, safe-outputs for write operations, -and refuses write permissions and deprecated fields. Use the strict parameter to override frontmatter settings. +Workflows use their own frontmatter strict setting. The strict parameter is not supported via the MCP tool; +use gh aw compile --strict from the CLI for strict mode compilation. Returns JSON array with validation results for each workflow: - workflow: Name of the workflow file @@ -135,6 +131,12 @@ Returns JSON array with validation results for each workflow: default: } + // Refuse strict=true: the MCP compile tool does not support strict mode. + // Strict compilation must be done via the CLI (gh aw compile --strict). + if args.Strict { + return nil, nil, newMCPError(jsonrpc.CodeInvalidParams, "compile with strict=true is not supported via the MCP tool; use gh aw compile --strict for strict mode compilation", nil) + } + // dockerUnavailableWarning is set when Docker is not accessible but the compile // should still proceed without the static-analysis tools. After the compile // attempt, the warning is appended to workflow results in the JSON output so @@ -201,11 +203,6 @@ Returns JSON array with validation results for each workflow: cmdArgs = append(cmdArgs, "--fix") } - // Add strict flag if requested - if args.Strict { - cmdArgs = append(cmdArgs, "--strict") - } - // Add static analysis flags if requested if args.Zizmor { cmdArgs = append(cmdArgs, "--zizmor") @@ -240,8 +237,8 @@ Returns JSON array with validation results for each workflow: cmdArgs = append(cmdArgs, "--prior-manifest-file", manifestCacheFile) } - mcpLog.Printf("Executing compile tool: workflows=%v, strict=%v, fix=%v, zizmor=%v, poutine=%v, actionlint=%v, runner-guard=%v, syft=%v, grype=%v, grant=%v, yamllint=%v", - args.Workflows, args.Strict, args.Fix, args.Zizmor, args.Poutine, args.Actionlint, args.RunnerGuard, args.Syft, args.Grype, args.Grant, args.Yamllint) + mcpLog.Printf("Executing compile tool: workflows=%v, fix=%v, zizmor=%v, poutine=%v, actionlint=%v, runner-guard=%v, syft=%v, grype=%v, grant=%v, yamllint=%v", + args.Workflows, args.Fix, args.Zizmor, args.Poutine, args.Actionlint, args.RunnerGuard, args.Syft, args.Grype, args.Grant, args.Yamllint) // Execute the CLI command // Use separate stdout/stderr capture instead of CombinedOutput because: