diff --git a/actions/setup/sh/check_mcp_servers.sh b/actions/setup/sh/check_mcp_servers.sh index dca59b00a6c..79d61390567 100755 --- a/actions/setup/sh/check_mcp_servers.sh +++ b/actions/setup/sh/check_mcp_servers.sh @@ -33,8 +33,8 @@ print_timing() { # GATEWAY_API_KEY : API key for gateway authentication # # Exit codes: -# 0 - All HTTP servers successfully checked (skipped servers logged as warnings) -# 1 - Invalid arguments, configuration file issues, or server connection failures +# 0 - At least one server connected and no required servers failed (optional server failures logged as warnings) +# 1 - Invalid arguments, configuration file issues, no successful connections, or required server failures if [ "$#" -ne 3 ]; then echo "Usage: $0 GATEWAY_CONFIG_PATH GATEWAY_URL GATEWAY_API_KEY" >&2 @@ -86,6 +86,7 @@ SERVERS_CHECKED=0 SERVERS_SUCCEEDED=0 SERVERS_FAILED=0 SERVERS_SKIPPED=0 +REQUIRED_SERVERS_FAILED=0 # Retry configuration for slow-starting servers # Gateway may take 40-50 seconds to start all MCP servers (per start_mcp_gateway.sh) @@ -104,6 +105,13 @@ while IFS= read -r SERVER_NAME; do SERVERS_FAILED=$((SERVERS_FAILED + 1)) continue fi + + # Check whether server is marked optional in configuration JSON. + # Servers are required by default; set required: false to degrade failures to warnings. + REQUIRED=true + if echo "$SERVER_CONFIG" | jq -e '.required == false' >/dev/null 2>&1; then + REQUIRED=false + fi # Extract server URL (should be HTTP URL pointing to gateway) SERVER_URL=$(echo "$SERVER_CONFIG" | jq -r '.url // empty' 2>/dev/null) @@ -220,7 +228,12 @@ while IFS= read -r SERVER_NAME; do echo "✓ $SERVER_NAME: connected" SERVERS_SUCCEEDED=$((SERVERS_SUCCEEDED + 1)) else - echo "✗ $SERVER_NAME: failed to connect" + if [ "$REQUIRED" = true ]; then + echo "✗ $SERVER_NAME: failed to connect (required)" + REQUIRED_SERVERS_FAILED=$((REQUIRED_SERVERS_FAILED + 1)) + else + echo "⚠ $SERVER_NAME: failed to connect (optional)" + fi echo " URL: ${SERVER_URL@Q}" echo " Last error: ${LAST_ERROR@Q}" echo " Retries attempted: $MAX_RETRIES" @@ -235,24 +248,23 @@ done <<< "$SERVER_NAMES" # Print summary print_timing $SCRIPT_START_TIME "Overall MCP server checks" echo "" -if [ $SERVERS_FAILED -gt 0 ]; then - echo "ERROR: $SERVERS_FAILED of $SERVERS_CHECKED server(s) failed connectivity check" +if [ $REQUIRED_SERVERS_FAILED -gt 0 ]; then + echo "ERROR: $REQUIRED_SERVERS_FAILED required server(s) failed connectivity check" echo "Succeeded: $SERVERS_SUCCEEDED, Failed: $SERVERS_FAILED, Skipped: $SERVERS_SKIPPED" echo "" - echo "This indicates that one or more MCP servers failed to respond to MCP ping/initialize/tools/list requests" + echo "One or more startup-critical MCP servers failed ping/initialize/tools/list" echo "after multiple retry attempts with progressive timeouts (10s, 20s, 30s)." echo "" echo "Common causes:" - echo " - MCP server container failed to start or crashed" - echo " - Network connectivity issues between gateway and server" - echo " - Server initialization taking longer than expected (>30s)" - echo " - Server port not properly exposed or accessible" + echo " - Invalid or expired credentials (for example HTTP 401/403 on initialize)" + echo " - MCP server unavailable or rejecting requests" + echo " - Network connectivity or DNS issues" echo "" echo "Check the gateway logs and individual server logs for more details:" echo " /tmp/gh-aw/mcp-logs/stderr.log" echo " /tmp/gh-aw/mcp-logs/start-gateway.log" exit 1 -elif [ $SERVERS_SUCCEEDED -eq 0 ]; then +elif [ $SERVERS_SUCCEEDED -eq 0 ] && [ $SERVERS_FAILED -eq 0 ]; then echo "ERROR: No HTTP servers were successfully checked" echo "This could indicate:" echo " - No HTTP-type MCP servers were configured" @@ -260,7 +272,23 @@ elif [ $SERVERS_SUCCEEDED -eq 0 ]; then echo "" echo "If you expected HTTP servers to be configured, check the gateway configuration." exit 1 +elif [ $SERVERS_SUCCEEDED -eq 0 ]; then + echo "ERROR: All $SERVERS_FAILED optional server(s) failed; no successful connections" + echo "Succeeded: 0, Failed: $SERVERS_FAILED, Skipped: $SERVERS_SKIPPED" + echo "" + echo "All configured HTTP MCP servers are optional but none connected successfully." + echo "At least one server must connect for the gateway to be considered healthy." + echo "" + echo "Check the gateway logs and individual server logs for more details:" + echo " /tmp/gh-aw/mcp-logs/stderr.log" + echo " /tmp/gh-aw/mcp-logs/start-gateway.log" + exit 1 else - echo "✓ All checks passed ($SERVERS_SUCCEEDED succeeded, $SERVERS_SKIPPED skipped)" + if [ $SERVERS_FAILED -gt 0 ]; then + echo "WARNING: $SERVERS_FAILED optional server(s) failed connectivity check; continuing startup" + echo "✓ Checks completed with warnings ($SERVERS_SUCCEEDED succeeded, $SERVERS_FAILED failed, $SERVERS_SKIPPED skipped)" + else + echo "✓ All checks passed ($SERVERS_SUCCEEDED succeeded, $SERVERS_SKIPPED skipped)" + fi exit 0 fi diff --git a/actions/setup/sh/check_mcp_servers_test.sh b/actions/setup/sh/check_mcp_servers_test.sh index d156117b763..f517dc86a1f 100755 --- a/actions/setup/sh/check_mcp_servers_test.sh +++ b/actions/setup/sh/check_mcp_servers_test.sh @@ -33,6 +33,104 @@ print_result() { fi } +start_mock_mcp_server() { + local port_file="$1" + local log_file="$2" + + node - "$port_file" >"$log_file" 2>&1 <<'NODE' & +const fs = require("fs"); +const http = require("http"); + +const portFile = process.argv[2]; + +const send = (res, code, payload, sessionId) => { + const body = JSON.stringify(payload); + res.writeHead(code, { + "Content-Type": "application/json", + "Content-Length": Buffer.byteLength(body), + ...(sessionId ? { "Mcp-Session-Id": sessionId } : {}), + }); + res.end(body); +}; + +const server = http.createServer((req, res) => { + let raw = ""; + req.on("data", chunk => { + raw += chunk; + }); + req.on("end", () => { + let data = {}; + try { + data = JSON.parse(raw || "{}"); + } catch { + data = {}; + } + + const method = data.method; + const reqId = data.id ?? 1; + + if (req.url.endsWith("/github")) { + if (method === "initialize") { + send(res, 200, { jsonrpc: "2.0", id: reqId, result: { protocolVersion: "2024-11-05", capabilities: {}, serverInfo: { name: "github", version: "1.0.0" } } }, "s1"); + } else if (method === "tools/list") { + send(res, 200, { jsonrpc: "2.0", id: reqId, result: { tools: [] } }); + } else { + send(res, 200, { jsonrpc: "2.0", id: reqId, result: {} }); + } + return; + } + + if (req.url.endsWith("/datadog")) { + if (method === "initialize") { + send(res, 403, { errors: ["Forbidden"] }); + } else { + send(res, 200, { jsonrpc: "2.0", id: reqId, result: {} }); + } + return; + } + + send(res, 404, { error: "not found" }); + }); +}); + +server.listen(0, "127.0.0.1", () => { + fs.writeFileSync(portFile, String(server.address().port), "utf8"); +}); +NODE + echo "$!" +} + +wait_for_port_file() { + local port_file="$1" + local i=0 + while [ ! -s "$port_file" ] && [ $i -lt 50 ]; do + sleep 0.1 + i=$((i + 1)) + done +} + +start_and_validate_mock_server() { + local port_file="$1" + local log_file="$2" + local server_pid + + server_pid=$(start_mock_mcp_server "$port_file" "$log_file") + if [ -z "$server_pid" ] || ! kill -0 "$server_pid" 2>/dev/null; then + return 1 + fi + + wait_for_port_file "$port_file" + + if [ ! -s "$port_file" ]; then + if kill -0 "$server_pid" 2>/dev/null; then + kill "$server_pid" 2>/dev/null || true + fi + return 1 + fi + + echo "$server_pid" +} + # Test 1: Script syntax is valid test_script_syntax() { echo "" @@ -352,6 +450,166 @@ test_validation_functions_exist() { fi } +# Test 11: Optional failing server should not fail startup when another server is healthy +test_optional_server_failure_degrades_to_warning() { + echo "" + echo "Test 11: Optional server failure degrades to warning" + + local tmpdir + tmpdir=$(mktemp -d) + local port_file="$tmpdir/port" + local config_file="$tmpdir/config.json" + + local server_pid + if ! server_pid=$(start_and_validate_mock_server "$port_file" "$tmpdir/mock.log"); then + print_result "Mock MCP server failed to start (check $tmpdir/mock.log)" "FAIL" + return + fi + + local port + port=$(cat "$port_file") + + cat > "$config_file" </dev/null 2>&1; then + print_result "Optional failing server does not fail startup" "PASS" + else + print_result "Optional failing server should not fail startup" "FAIL" + fi + + kill "$server_pid" 2>/dev/null || true + wait "$server_pid" 2>/dev/null || true + rm -rf "$tmpdir" +} + +# Test 12: Server with no required field should fail startup by default (required is the default) +test_required_server_failure_is_fatal() { + echo "" + echo "Test 12: Server failure is fatal by default (required is the default)" + + local tmpdir + tmpdir=$(mktemp -d) + local port_file="$tmpdir/port" + local config_file="$tmpdir/config.json" + + local server_pid + if ! server_pid=$(start_and_validate_mock_server "$port_file" "$tmpdir/mock.log"); then + print_result "Mock MCP server failed to start (check $tmpdir/mock.log)" "FAIL" + return + fi + + local port + port=$(cat "$port_file") + + cat > "$config_file" </dev/null 2>&1; then + print_result "Failing server without required field defaults to fatal" "PASS" + else + print_result "Failing server without required field should default to fatal" "FAIL" + fi + + kill "$server_pid" 2>/dev/null || true + wait "$server_pid" 2>/dev/null || true + rm -rf "$tmpdir" +} + +# Test 13: All optional servers fail => startup should fail (no successful connections) +test_all_optional_servers_fail_is_error() { + echo "" + echo "Test 13: All optional servers fail => startup fails with clear error" + + local tmpdir + tmpdir=$(mktemp -d) + local port_file="$tmpdir/port" + local config_file="$tmpdir/config.json" + + local server_pid + if ! server_pid=$(start_and_validate_mock_server "$port_file" "$tmpdir/mock.log"); then + print_result "Mock MCP server failed to start (check $tmpdir/mock.log)" "FAIL" + return + fi + + local port + port=$(cat "$port_file") + + # Only the datadog server (returns 403 on initialize), marked optional + cat > "$config_file" <"$output_file" 2>&1 || run_result=$? + + if [ $run_result -ne 0 ]; then + print_result "All optional servers failing causes startup to fail" "PASS" + else + print_result "All optional servers failing should cause startup to fail" "FAIL" + fi + + # Check that the error message is about all optional servers failing, not about missing config + if grep -q "optional server" "$output_file"; then + print_result "Error message correctly references optional server failure" "PASS" + else + print_result "Error message should reference optional server failure (not 'no HTTP servers configured')" "FAIL" + fi + + kill "$server_pid" 2>/dev/null || true + wait "$server_pid" 2>/dev/null || true + rm -rf "$tmpdir" +} + # Run all tests echo "=== Testing check_mcp_servers.sh ===" echo "Script: $SCRIPT_PATH" @@ -366,6 +624,9 @@ test_valid_http_server test_server_without_url test_mixed_servers test_validation_functions_exist +test_optional_server_failure_degrades_to_warning +test_required_server_failure_is_fatal +test_all_optional_servers_fail_is_error # Print summary echo "" diff --git a/pkg/parser/mcp.go b/pkg/parser/mcp.go index fa4229789bc..a9535b54c1c 100644 --- a/pkg/parser/mcp.go +++ b/pkg/parser/mcp.go @@ -44,6 +44,10 @@ type RegistryMCPServerConfig struct { Registry string `json:"registry"` // URI to installation location from registry ProxyArgs []string `json:"proxy-args"` // custom proxy arguments for container-based tools Allowed []string `json:"allowed"` // allowed tools + + // Gateway startup behavior: when Required is explicitly false the server is optional + // and startup failures degrade to warnings. nil means the default (required). + Required *bool `json:"required,omitempty"` } // MCPServerInfo contains the inspection results for an MCP server diff --git a/pkg/workflow/mcp_config_custom.go b/pkg/workflow/mcp_config_custom.go index d3832893c98..36bc4a0b4ce 100644 --- a/pkg/workflow/mcp_config_custom.go +++ b/pkg/workflow/mcp_config_custom.go @@ -132,7 +132,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma // JSON format - use MCP Gateway schema format (container-based) OR legacy command-based // Per MCP Gateway Specification v1.0.0 section 3.2.1, stdio servers SHOULD be containerized // But we also support legacy command-based tools for backwards compatibility - propertyOrder = []string{"type", "container", "entrypoint", "entrypointArgs", "mounts", "command", "args", "tools", "env", "proxy-args", "registry"} + propertyOrder = []string{"type", "container", "entrypoint", "entrypointArgs", "mounts", "command", "args", "tools", "env", "proxy-args", "registry", "required"} } case "http": if renderer.Format == "toml" { @@ -142,9 +142,9 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma // JSON format - include tools field for MCP gateway tool filtering (all engines) // For HTTP MCP with secrets in headers, env passthrough is needed if len(headerSecrets) > 0 { - propertyOrder = []string{"type", "url", "headers", "auth", "tools", "env"} + propertyOrder = []string{"type", "url", "headers", "auth", "tools", "env", "required"} } else { - propertyOrder = []string{"type", "url", "headers", "auth", "tools"} + propertyOrder = []string{"type", "url", "headers", "auth", "tools", "required"} } } default: @@ -220,6 +220,11 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma if mcpConfig.Registry != "" { existingProperties = append(existingProperties, prop) } + case "required": + // Only emit when explicitly set to false (false = optional; default/absent = required) + if mcpConfig.Required != nil && !*mcpConfig.Required { + existingProperties = append(existingProperties, prop) + } } } @@ -499,6 +504,16 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma } fmt.Fprintf(yaml, "%s\"registry\": \"%s\"%s\n", renderer.IndentLevel, mcpConfig.Registry, comma) } + case "required": + // Only emitted for JSON format when Required is explicitly false (optional server). + // Absent field means the default (required), so we never emit "required": true. + if renderer.Format == "json" && mcpConfig.Required != nil && !*mcpConfig.Required { + comma := "," + if isLast { + comma = "" + } + fmt.Fprintf(yaml, "%s\"required\": false%s\n", renderer.IndentLevel, comma) + } } } @@ -568,6 +583,7 @@ func getMCPConfig(toolConfig map[string]any, toolName string) (*parser.RegistryM "registry": {}, "allowed": {}, "toolsets": {}, // Added for MCPServerConfig struct + "required": {}, // Whether the server is required at startup (false = optional, default = required) } for key := range toolConfig { @@ -712,6 +728,13 @@ func getMCPConfig(toolConfig map[string]any, toolName string) (*parser.RegistryM result.Allowed = allowed } + // Extract required field: when explicitly set to false the server is optional at startup + if requiredVal, hasRequired := config.GetAny("required"); hasRequired { + if requiredBool, ok := requiredVal.(bool); ok { + result.Required = &requiredBool + } + } + // Automatically assign well-known containers for stdio MCP servers based on command // This ensures all stdio servers work with the MCP Gateway which requires containerization if result.Type == "stdio" && result.Container == "" && result.Command != "" { diff --git a/pkg/workflow/schemas/mcp-gateway-config.schema.json b/pkg/workflow/schemas/mcp-gateway-config.schema.json index 463f2f702b6..d08d8f91a98 100644 --- a/pkg/workflow/schemas/mcp-gateway-config.schema.json +++ b/pkg/workflow/schemas/mcp-gateway-config.schema.json @@ -103,6 +103,11 @@ } }, "additionalProperties": true + }, + "required": { + "type": "boolean", + "description": "Whether this server is required for a successful gateway startup. When absent or true (the default), a connectivity failure causes startup to fail. Set to false to make the server optional: failures are logged as warnings and startup continues if at least one other server connects successfully.", + "default": true } }, "required": ["container"], @@ -175,6 +180,11 @@ }, "required": ["type"], "additionalProperties": false + }, + "required": { + "type": "boolean", + "description": "Whether this server is required for a successful gateway startup. When absent or true (the default), a connectivity failure causes startup to fail. Set to false to make the server optional: failures are logged as warnings and startup continues if at least one other server connects successfully.", + "default": true } }, "required": ["type", "url"],