Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 12 additions & 19 deletions pkg/cli/mcp_server_compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
24 changes: 5 additions & 19 deletions pkg/cli/mcp_server_defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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)
}
})

Expand Down
24 changes: 24 additions & 0 deletions pkg/cli/mcp_server_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
29 changes: 13 additions & 16 deletions pkg/cli/mcp_tools_readonly.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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",
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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:
Expand Down