diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 50901183dd..ce62ddd674 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -225,6 +225,12 @@ const DefaultMCPStartupTimeout = 120 * time.Second // MCPSessionTimeoutMin is the minimum allowed value for engine.mcp.session-timeout (5 minutes). const MCPSessionTimeoutMin = 5 * time.Minute +// MCPToolTimeoutMin is the minimum allowed value for engine.mcp.tool-timeout (10 seconds). +const MCPToolTimeoutMin = 10 * time.Second + +// MCPToolTimeoutMax is the maximum allowed value for engine.mcp.tool-timeout (600 seconds). +const MCPToolTimeoutMax = 600 * time.Second + // DefaultActivationJobRunnerImage is the default runner image for activation and pre-activation jobs const DefaultActivationJobRunnerImage = "ubuntu-slim" diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index f0d83fd97f..3aaa67b6b8 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -9902,6 +9902,11 @@ "type": "string", "description": "Session timeout for MCP gateway sessions as a Go duration string (e.g. \"30m\", \"4h\", \"24h\"). Must be at least 5m (no upper bound). Omitted or empty uses the effective gateway default (precedence: this field > MCP_GATEWAY_SESSION_TIMEOUT env var > built-in default 6h). Longer timeouts benefit multi-hour workflows such as large-scale migrations; shorter values free gateway resources sooner.", "examples": ["30m", "1h", "4h", "6h", "12h"] + }, + "tool-timeout": { + "type": "string", + "description": "Timeout for individual MCP tool calls as a Go duration string (e.g. \"30s\", \"2m\", \"10m\"). Must be between 10s and 600s inclusive. Omitted or empty uses the gateway built-in default (60s). Use a higher value for slow MCP backends such as full-text search over large indexes.", + "examples": ["30s", "2m", "5m", "10m"] } }, "additionalProperties": false diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 7e9598f173..8e3ab89a17 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -86,6 +86,11 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) return nil, fmt.Errorf("%s: %w", cleanPath, err) } + // Validate optional engine.mcp.tool-timeout configuration. + if err := c.validateEngineMCPToolTimeout(workflowData); err != nil { + return nil, fmt.Errorf("%s: %w", cleanPath, err) + } + // Validate that inlined-imports is not used with agent file imports. // Agent files require runtime access and cannot be resolved without sources. if workflowData.InlinedImports && engineSetup.importsResult.AgentFile != "" { diff --git a/pkg/workflow/compiler_string_api.go b/pkg/workflow/compiler_string_api.go index df69f946db..d90c13e4b2 100644 --- a/pkg/workflow/compiler_string_api.go +++ b/pkg/workflow/compiler_string_api.go @@ -140,6 +140,11 @@ func (c *Compiler) ParseWorkflowString(content string, virtualPath string) (*Wor return nil, fmt.Errorf("%s: %w", cleanPath, err) } + // Validate optional engine.mcp.tool-timeout configuration. + if err := c.validateEngineMCPToolTimeout(workflowData); err != nil { + return nil, fmt.Errorf("%s: %w", cleanPath, err) + } + // Validate GitHub tool configuration if err := validateGitHubToolConfig(workflowData.ParsedTools, workflowData.Name); err != nil { return nil, fmt.Errorf("%s: %w", cleanPath, err) diff --git a/pkg/workflow/engine.go b/pkg/workflow/engine.go index 0ebd28f32d..3a8fbb4979 100644 --- a/pkg/workflow/engine.go +++ b/pkg/workflow/engine.go @@ -49,6 +49,7 @@ type EngineConfig struct { // MCP gateway configuration from engine.mcp sub-object MCPSessionTimeout string // session-timeout: Go duration string for MCP gateway sessions (e.g. "4h", "30m") + MCPToolTimeout string // tool-timeout: Go duration string for individual MCP tool calls (e.g. "2m", "30s") } // NetworkPermissions represents network access permissions for workflow execution @@ -331,6 +332,13 @@ func (c *Compiler) ExtractEngineConfig(frontmatter map[string]any) (string, *Eng engineLog.Printf("Extracted engine.mcp.session-timeout: %s", config.MCPSessionTimeout) } } + // Extract tool-timeout (kebab-case only; camelCase is not supported) + if ttVal, hasToolTimeout := mcpObj["tool-timeout"]; hasToolTimeout { + if ttStr, ok := ttVal.(string); ok && ttStr != "" { + config.MCPToolTimeout = ttStr + engineLog.Printf("Extracted engine.mcp.tool-timeout: %s", config.MCPToolTimeout) + } + } } } diff --git a/pkg/workflow/engine_config_test.go b/pkg/workflow/engine_config_test.go index d12630baa4..6c8b8ea74c 100644 --- a/pkg/workflow/engine_config_test.go +++ b/pkg/workflow/engine_config_test.go @@ -925,3 +925,71 @@ func TestEngineMCPSessionTimeoutExtraction(t *testing.T) { }) } } + +// TestEngineMCPToolTimeoutExtraction tests extraction of engine.mcp.tool-timeout. +func TestEngineMCPToolTimeoutExtraction(t *testing.T) { + compiler := NewCompiler() + + tests := []struct { + name string + frontmatter map[string]any + expectedTimeout string + }{ + { + name: "extracts tool-timeout from engine.mcp", + frontmatter: map[string]any{ + "engine": map[string]any{ + "id": "copilot", + "mcp": map[string]any{ + "tool-timeout": "2m", + }, + }, + }, + expectedTimeout: "2m", + }, + { + name: "no mcp section - empty tool timeout", + frontmatter: map[string]any{ + "engine": map[string]any{ + "id": "copilot", + }, + }, + expectedTimeout: "", + }, + { + name: "mcp section without tool-timeout - empty", + frontmatter: map[string]any{ + "engine": map[string]any{ + "id": "copilot", + "mcp": map[string]any{}, + }, + }, + expectedTimeout: "", + }, + { + name: "mcp section with both session-timeout and tool-timeout", + frontmatter: map[string]any{ + "engine": map[string]any{ + "id": "copilot", + "mcp": map[string]any{ + "session-timeout": "4h", + "tool-timeout": "5m", + }, + }, + }, + expectedTimeout: "5m", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, config := compiler.ExtractEngineConfig(tt.frontmatter) + if config == nil { + t.Fatal("Expected non-nil config") + } + if config.MCPToolTimeout != tt.expectedTimeout { + t.Errorf("MCPToolTimeout = %q, want %q", config.MCPToolTimeout, tt.expectedTimeout) + } + }) + } +} diff --git a/pkg/workflow/engine_validation.go b/pkg/workflow/engine_validation.go index e289649a33..5935ab5a29 100644 --- a/pkg/workflow/engine_validation.go +++ b/pkg/workflow/engine_validation.go @@ -131,6 +131,32 @@ func (c *Compiler) validateEngineMCPSessionTimeout(workflowData *WorkflowData) e return nil } +// validateEngineMCPToolTimeout validates optional engine.mcp.tool-timeout configuration. +// The value must be a valid Go duration string between 10s and 600s inclusive. +func (c *Compiler) validateEngineMCPToolTimeout(workflowData *WorkflowData) error { + if workflowData == nil || workflowData.EngineConfig == nil || workflowData.EngineConfig.MCPToolTimeout == "" { + return nil + } + + raw := workflowData.EngineConfig.MCPToolTimeout + + d, err := time.ParseDuration(raw) + if err != nil { + return fmt.Errorf("engine.mcp.tool-timeout: invalid duration %q. Must be a valid Go duration string (e.g. \"30s\", \"2m\", \"10m\").\n\nExamples:\n engine:\n mcp:\n tool-timeout: 2m\n\nSee: %s", raw, constants.DocsEnginesURL) + } + + if d < constants.MCPToolTimeoutMin { + return fmt.Errorf("engine.mcp.tool-timeout: %q is too short (minimum is 10s).\n\nExamples:\n tool-timeout: 30s\n tool-timeout: 2m\n\nSee: %s", raw, constants.DocsEnginesURL) + } + + if d > constants.MCPToolTimeoutMax { + return fmt.Errorf("engine.mcp.tool-timeout: %q exceeds the maximum allowed value (600s / 10m).\n\nExamples:\n tool-timeout: 2m\n tool-timeout: 10m\n\nSee: %s", raw, constants.DocsEnginesURL) + } + + engineValidationLog.Printf("engine.mcp.tool-timeout validated: %s (%s)", raw, d) + return nil +} + // validateEngineInlineDefinition validates an inline engine definition parsed from // engine.runtime + optional engine.provider in the workflow frontmatter. // Returns an error if: diff --git a/pkg/workflow/engine_validation_test.go b/pkg/workflow/engine_validation_test.go index 6743a59f3b..13b10f146f 100644 --- a/pkg/workflow/engine_validation_test.go +++ b/pkg/workflow/engine_validation_test.go @@ -505,3 +505,123 @@ func TestValidateEngineMCPSessionTimeout(t *testing.T) { }) } } + +// TestValidateEngineMCPToolTimeout tests the validateEngineMCPToolTimeout function. +func TestValidateEngineMCPToolTimeout(t *testing.T) { + tests := []struct { + name string + workflow *WorkflowData + expectError bool + errorSubstr string + }{ + { + name: "nil workflow data", + workflow: nil, + expectError: false, + }, + { + name: "nil engine config", + workflow: &WorkflowData{}, + expectError: false, + }, + { + name: "empty tool timeout - no error", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot"}, + }, + expectError: false, + }, + { + name: "valid duration 2m", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPToolTimeout: "2m"}, + }, + expectError: false, + }, + { + name: "valid duration 30s", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPToolTimeout: "30s"}, + }, + expectError: false, + }, + { + name: "valid duration 10s (minimum)", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPToolTimeout: "10s"}, + }, + expectError: false, + }, + { + name: "valid duration 600s (maximum)", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPToolTimeout: "600s"}, + }, + expectError: false, + }, + { + name: "valid duration 10m (maximum)", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPToolTimeout: "10m"}, + }, + expectError: false, + }, + { + name: "invalid duration string", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPToolTimeout: "2hours"}, + }, + expectError: true, + errorSubstr: "invalid duration", + }, + { + name: "too short - 5s", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPToolTimeout: "5s"}, + }, + expectError: true, + errorSubstr: "too short", + }, + { + name: "too long - 601s", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPToolTimeout: "601s"}, + }, + expectError: true, + errorSubstr: "exceeds the maximum", + }, + { + name: "too long - 11m", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPToolTimeout: "11m"}, + }, + expectError: true, + errorSubstr: "exceeds the maximum", + }, + { + name: "plain integer - not valid Go duration", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPToolTimeout: "120"}, + }, + expectError: true, + errorSubstr: "invalid duration", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + compiler := NewCompiler() + err := compiler.validateEngineMCPToolTimeout(tt.workflow) + + if tt.expectError { + require.Error(t, err, "Expected validation error") + if tt.errorSubstr != "" { + assert.Contains(t, err.Error(), tt.errorSubstr, "Expected error substring mismatch") + } + return + } + + assert.NoError(t, err, "Expected tool-timeout validation to pass") + }) + } +} diff --git a/pkg/workflow/mcp_gateway_config.go b/pkg/workflow/mcp_gateway_config.go index 8ef7453650..69c271a7f8 100644 --- a/pkg/workflow/mcp_gateway_config.go +++ b/pkg/workflow/mcp_gateway_config.go @@ -136,9 +136,11 @@ func buildMCPGatewayConfig(workflowData *WorkflowData) *MCPGatewayRuntimeConfig // are written directly into the gateway config JSON. // // SessionTimeout comes from engine.mcp.session-timeout in frontmatter (via EngineConfig). - var sessionTimeout string + // ToolTimeout comes from engine.mcp.tool-timeout in frontmatter (via EngineConfig). + var sessionTimeout, toolTimeout string if workflowData.EngineConfig != nil { sessionTimeout = workflowData.EngineConfig.MCPSessionTimeout + toolTimeout = workflowData.EngineConfig.MCPToolTimeout } return &MCPGatewayRuntimeConfig{ Port: int(DefaultMCPGatewayPort), // Will be formatted as "${MCP_GATEWAY_PORT}" in renderer @@ -150,6 +152,7 @@ func buildMCPGatewayConfig(workflowData *WorkflowData) *MCPGatewayRuntimeConfig TrustedBots: workflowData.SandboxConfig.MCP.TrustedBots, // Additional trusted bot identities from frontmatter KeepaliveInterval: workflowData.SandboxConfig.MCP.KeepaliveInterval, // Keepalive interval from frontmatter (0=default, -1=disabled, >0=custom) SessionTimeout: sessionTimeout, // Session timeout from engine.mcp.session-timeout (empty = gateway default 6h) + ToolTimeout: toolTimeout, // Tool timeout from engine.mcp.tool-timeout (empty = gateway built-in default 60s) // OTLPEndpoint and OTLPHeaders are set from workflowData by injectOTLPConfig, which is // the fully resolved OTLP config (including imports). Using these fields ensures gateway // OTLP config honours observability defined in imported shared workflows. diff --git a/pkg/workflow/mcp_gateway_config_test.go b/pkg/workflow/mcp_gateway_config_test.go index 5aa150af8b..bc299b460f 100644 --- a/pkg/workflow/mcp_gateway_config_test.go +++ b/pkg/workflow/mcp_gateway_config_test.go @@ -401,6 +401,44 @@ func TestBuildMCPGatewayConfig(t *testing.T) { SessionTimeout: "", }, }, + { + name: "propagates tool-timeout from engine config", + workflowData: &WorkflowData{ + EngineConfig: &EngineConfig{ + ID: "copilot", + MCPToolTimeout: "2m", + }, + SandboxConfig: &SandboxConfig{}, + }, + expected: &MCPGatewayRuntimeConfig{ + Port: int(DefaultMCPGatewayPort), + Domain: "${MCP_GATEWAY_DOMAIN}", + APIKey: "${MCP_GATEWAY_API_KEY}", + PayloadDir: "${MCP_GATEWAY_PAYLOAD_DIR}", + PayloadSizeThreshold: constants.DefaultMCPGatewayPayloadSizeThreshold, + ToolTimeout: "2m", + }, + }, + { + name: "propagates both session-timeout and tool-timeout from engine config", + workflowData: &WorkflowData{ + EngineConfig: &EngineConfig{ + ID: "copilot", + MCPSessionTimeout: "4h", + MCPToolTimeout: "5m", + }, + SandboxConfig: &SandboxConfig{}, + }, + expected: &MCPGatewayRuntimeConfig{ + Port: int(DefaultMCPGatewayPort), + Domain: "${MCP_GATEWAY_DOMAIN}", + APIKey: "${MCP_GATEWAY_API_KEY}", + PayloadDir: "${MCP_GATEWAY_PAYLOAD_DIR}", + PayloadSizeThreshold: constants.DefaultMCPGatewayPayloadSizeThreshold, + SessionTimeout: "4h", + ToolTimeout: "5m", + }, + }, } for _, tt := range tests { @@ -419,6 +457,7 @@ func TestBuildMCPGatewayConfig(t *testing.T) { assert.Equal(t, tt.expected.TrustedBots, result.TrustedBots, "TrustedBots should match") assert.Equal(t, tt.expected.KeepaliveInterval, result.KeepaliveInterval, "KeepaliveInterval should match") assert.Equal(t, tt.expected.SessionTimeout, result.SessionTimeout, "SessionTimeout should match") + assert.Equal(t, tt.expected.ToolTimeout, result.ToolTimeout, "ToolTimeout should match") } }) } diff --git a/pkg/workflow/mcp_renderer.go b/pkg/workflow/mcp_renderer.go index 112cdc98ce..0ac86c47cc 100644 --- a/pkg/workflow/mcp_renderer.go +++ b/pkg/workflow/mcp_renderer.go @@ -191,6 +191,9 @@ func RenderJSONMCPConfig( if options.GatewayConfig.SessionTimeout != "" { fmt.Fprintf(&configBuilder, ",\n \"sessionTimeout\": %q", options.GatewayConfig.SessionTimeout) } + if options.GatewayConfig.ToolTimeout != "" { + fmt.Fprintf(&configBuilder, ",\n \"toolTimeout\": %q", options.GatewayConfig.ToolTimeout) + } // When OTLP tracing is configured, add the opentelemetry section directly to the // gateway config. The endpoint is passed via the OTEL_EXPORTER_OTLP_ENDPOINT env var // (injected by injectOTLPConfig) so that secrets are never interpolated directly into diff --git a/pkg/workflow/mcp_renderer_test.go b/pkg/workflow/mcp_renderer_test.go index 8e50ca8fa9..49d3f736bc 100644 --- a/pkg/workflow/mcp_renderer_test.go +++ b/pkg/workflow/mcp_renderer_test.go @@ -668,3 +668,74 @@ func TestRenderJSONMCPConfig_SessionTimeout(t *testing.T) { }) } } + +// TestRenderJSONMCPConfig_ToolTimeout verifies that toolTimeout is emitted +// in the gateway JSON section when set on the MCPGatewayRuntimeConfig. +func TestRenderJSONMCPConfig_ToolTimeout(t *testing.T) { + tests := []struct { + name string + toolTimeout string + wantField bool + }{ + { + name: "includes toolTimeout when set", + toolTimeout: "2m", + wantField: true, + }, + { + name: "omits toolTimeout when empty", + toolTimeout: "", + wantField: false, + }, + { + name: "includes toolTimeout 30s", + toolTimeout: "30s", + wantField: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gatewayConfig := &MCPGatewayRuntimeConfig{ + Domain: "localhost", + APIKey: "test-api-key", + ToolTimeout: tt.toolTimeout, + } + + workflowData := &WorkflowData{ + Name: "test-workflow", + FrontmatterHash: "abc123", + } + + var output strings.Builder + err := RenderJSONMCPConfig( + &output, + map[string]any{}, + []string{}, + workflowData, + JSONMCPConfigOptions{ + ConfigPath: "/tmp/test/mcp-servers.json", + GatewayConfig: gatewayConfig, + Renderers: MCPToolRenderers{}, + }, + ) + + if err != nil { + t.Fatalf("RenderJSONMCPConfig returned error: %v", err) + } + + result := output.String() + hasField := strings.Contains(result, `"toolTimeout":`) + if hasField != tt.wantField { + t.Errorf("toolTimeout field presence = %v, want %v\noutput:\n%s", hasField, tt.wantField, result) + } + + if tt.wantField && tt.toolTimeout != "" { + expected := `"toolTimeout": "` + tt.toolTimeout + `"` + if !strings.Contains(result, expected) { + t.Errorf("expected %q in output\noutput:\n%s", expected, result) + } + } + }) + } +} diff --git a/pkg/workflow/tools_types.go b/pkg/workflow/tools_types.go index 0199deddc2..000139bb3a 100644 --- a/pkg/workflow/tools_types.go +++ b/pkg/workflow/tools_types.go @@ -442,6 +442,7 @@ type MCPGatewayRuntimeConfig struct { TrustedBots []string `yaml:"trusted-bots,omitempty"` // Additional bot identity strings to pass to the gateway, merged with its built-in list KeepaliveInterval int `yaml:"keepalive-interval,omitempty"` // Keepalive ping interval in seconds for HTTP MCP backends (0=default 1500s, -1=disabled, >0=custom) SessionTimeout string `yaml:"session-timeout,omitempty"` // Session timeout for MCP gateway sessions as a Go duration string (e.g. "4h", "30m"); empty = gateway default (precedence: stdin config > MCP_GATEWAY_SESSION_TIMEOUT env var > built-in default 6h) + ToolTimeout string `yaml:"tool-timeout,omitempty"` // Timeout for individual MCP tool calls as a Go duration string (e.g. "2m", "30s"); empty = gateway built-in default (60s) OTLPEndpoint string `yaml:"-"` // OTLP collector endpoint (derived from observability.otlp, not user-settable) OTLPHeaders string `yaml:"-"` // Raw OTLP HTTP headers string (derived from observability.otlp, not user-settable) }