From 5e57ab6448e1557b8ebcaec119f39b5956c8bedb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 16:34:34 +0000 Subject: [PATCH 1/6] Initial plan From f8097bd162d30e995f75f7c027400c909f89cb63 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 16:53:02 +0000 Subject: [PATCH 2/6] Add engine.mcp.session-timeout support for configurable MCP gateway session timeout - Add MCPSessionTimeout field to EngineConfig - Parse engine.mcp.session-timeout in ExtractEngineConfig - Add SessionTimeout field to MCPGatewayRuntimeConfig - Propagate session timeout from engine config in buildMCPGatewayConfig - Render sessionTimeout in gateway JSON config (mcp_renderer.go) - Add validateEngineMCPSessionTimeout (5m-12h bounds) in engine_validation.go - Wire validation in compiler_orchestrator_workflow.go - Update JSON schema with engine.mcp.session-timeout property - Add docs section in mcp-gateway.md - Add tests for extraction, validation, config building, and rendering Agent-Logs-Url: https://github.com/github/gh-aw/sessions/28633935-f9d4-454d-b410-68ff66ca2853 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- .../src/content/docs/reference/mcp-gateway.md | 53 ++++++++- pkg/parser/schemas/main_workflow_schema.json | 12 ++ .../compiler_orchestrator_workflow.go | 5 + pkg/workflow/engine.go | 19 ++++ pkg/workflow/engine_config_test.go | 67 +++++++++++ pkg/workflow/engine_validation.go | 29 +++++ pkg/workflow/engine_validation_test.go | 105 ++++++++++++++++++ pkg/workflow/mcp_gateway_config.go | 7 ++ pkg/workflow/mcp_gateway_config_test.go | 33 ++++++ pkg/workflow/mcp_renderer.go | 3 + pkg/workflow/mcp_renderer_test.go | 71 ++++++++++++ pkg/workflow/tools_types.go | 1 + 12 files changed, 404 insertions(+), 1 deletion(-) diff --git a/docs/src/content/docs/reference/mcp-gateway.md b/docs/src/content/docs/reference/mcp-gateway.md index e71ce439a4f..d4634fdcd57 100644 --- a/docs/src/content/docs/reference/mcp-gateway.md +++ b/docs/src/content/docs/reference/mcp-gateway.md @@ -251,6 +251,7 @@ The `gateway` section is required and configures gateway-specific behavior: | `payloadSizeThreshold` | integer | No | Size threshold in bytes for storing payloads to disk (default: 524288 = 512KB) | | `trustedBots` | array[string] | No | Additional GitHub bot identity strings (e.g., `github-actions[bot]`) passed to the gateway and merged with its built-in trusted identity list. This field is additive — it extends the internal list but cannot remove built-in entries. | | `keepaliveInterval` | integer | No | Keepalive ping interval in seconds for HTTP MCP backends. Prevents session expiry during long-running tasks. Use `-1` to disable, `0` or unset for gateway default (1500s = 25 min), or a positive integer for a custom interval. | +| `sessionTimeout` | string | No | Session timeout for MCP gateway sessions as a Go duration string (e.g. `"30m"`, `"4h"`, `"6h"`). Empty or omitted uses the gateway default (6h). Must be between 5m and 12h when set by the workflow compiler (infrastructure operators may override via `MCP_GATEWAY_SESSION_TIMEOUT` env var). | | `opentelemetry` | object | No | OpenTelemetry configuration for emitting distributed tracing events for MCP calls. See Section 4.1.3.6 for details. | #### 4.1.3.1 Payload Directory Path Validation @@ -449,7 +450,47 @@ sandbox: - A value of `-1` disables keepalive pings entirely - Any positive integer sets the keepalive interval in seconds -#### 4.1.3.6 OpenTelemetry Configuration +#### 4.1.3.6 Session Timeout Configuration + +The optional `sessionTimeout` field in the gateway configuration controls how long stateful MCP sessions survive in both unified and routed modes. + +| Value | Behavior | +|-------|----------| +| Unset / `""` | Gateway default: 6 hours (or `MCP_GATEWAY_SESSION_TIMEOUT` env var) | +| Go duration string | Custom session lifetime (e.g. `"30m"`, `"4h"`, `"6h"`, `"12h"`) | + +**Precedence**: `sessionTimeout` in the stdin config JSON **>** `MCP_GATEWAY_SESSION_TIMEOUT` environment variable **>** gateway built-in default (6h). + +**Configuration example (JSON)**: + +```json +{ + "gateway": { + "port": 8080, + "domain": "localhost", + "apiKey": "${MCP_GATEWAY_API_KEY}", + "sessionTimeout": "4h" + } +} +``` + +**Workflow frontmatter** (via `engine.mcp.session-timeout`): + +```yaml +engine: + id: copilot + mcp: + session-timeout: 4h # 4-hour sessions for long-running migrations +``` + +**Compliance rules**: + +- `sessionTimeout` MUST be a valid Go duration string when present (e.g. `"30m"`, `"4h"`) +- The workflow compiler enforces a minimum of `5m` and a maximum of `12h` for author-specified values +- Infrastructure operators may set `MCP_GATEWAY_SESSION_TIMEOUT` on the gateway container to override the default for all workflows; a per-workflow `sessionTimeout` in the stdin config takes precedence +- When unset, the gateway uses its built-in default (6h) + +#### 4.1.3.7 OpenTelemetry Configuration The optional `opentelemetry` object in the gateway configuration enables the gateway to emit distributed tracing events for MCP calls using the [OpenTelemetry](https://opentelemetry.io/) standard. When configured, the gateway creates spans for each MCP tool invocation and exports them to the designated collector endpoint. @@ -1852,6 +1893,16 @@ Content-Type: application/json - **Added**: Normative references for W3C Trace Context and OTLP - **Updated**: JSON Schema with `opentelemetry` property and `opentelemetryConfig` definition in `gatewayConfig` +### Version 1.11.0 (Draft) + +- **Added**: `sessionTimeout` field to gateway configuration (Section 4.1.3, 4.1.3.6) + - Optional Go duration string for controlling MCP session lifetime (e.g. `"4h"`, `"30m"`) + - Precedence: stdin config `sessionTimeout` > `MCP_GATEWAY_SESSION_TIMEOUT` env var > gateway default (6h) + - Workflow authors configure via `engine.mcp.session-timeout` in frontmatter; the compiler validates (min 5m, max 12h) and emits it into the gateway config JSON +- **Added**: Section 4.1.3.6 — Session Timeout Configuration +- **Renumbered**: Former Section 4.1.3.6 (OpenTelemetry) to 4.1.3.7 +- **Updated**: JSON Schema with `sessionTimeout` property in `gatewayConfig` definition + ### Version 1.10.0 (Draft) - **Added**: `keepaliveInterval` field to gateway configuration (Section 4.1.3, 4.1.3.5) diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index f827f962ecb..d3d9afaf7d7 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -9819,6 +9819,18 @@ "type": "boolean", "description": "When true, disables automatic loading of context and custom instructions by the AI engine. The engine-specific flag depends on the engine: copilot uses --no-custom-instructions (suppresses .github/AGENTS.md and user-level custom instructions), claude uses --bare (suppresses CLAUDE.md memory files), codex uses --no-system-prompt (suppresses the default system prompt), gemini sets GEMINI_SYSTEM_MD=/dev/null (overrides the built-in system prompt with an empty one). Defaults to false.", "default": false + }, + "mcp": { + "type": "object", + "description": "Engine-level MCP gateway configuration. Settings here apply to the MCP gateway used by this engine.", + "properties": { + "session-timeout": { + "type": "string", + "description": "Session timeout for MCP gateway sessions as a Go duration string (e.g. \"30m\", \"4h\", \"6h\"). Must be between 5m and 12h. Empty or omitted uses the gateway 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"] + } + }, + "additionalProperties": false } }, "required": ["id"], diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index e6e0984809d..d341a4404cb 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -81,6 +81,11 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) return nil, fmt.Errorf("%s: %w", cleanPath, err) } + // Validate optional engine.mcp.session-timeout configuration. + if err := c.validateEngineMCPSessionTimeout(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/engine.go b/pkg/workflow/engine.go index b1805bc04ec..249b49ad96f 100644 --- a/pkg/workflow/engine.go +++ b/pkg/workflow/engine.go @@ -46,6 +46,9 @@ type EngineConfig struct { // Extended inline request shaping fields (engine.provider.request.*) InlineProviderRequest *RequestShape // request shaping parsed from engine.provider.request + + // MCP gateway configuration from engine.mcp sub-object + MCPSessionTimeout string // session-timeout: Go duration string for MCP gateway sessions (e.g. "4h", "30m") } // NetworkPermissions represents network access permissions for workflow execution @@ -318,6 +321,22 @@ func (c *Compiler) ExtractEngineConfig(frontmatter map[string]any) (string, *Eng } } + // Extract optional 'mcp' sub-object (engine-level MCP gateway configuration) + if mcpVal, hasMCP := engineObj["mcp"]; hasMCP { + if mcpObj, ok := mcpVal.(map[string]any); ok { + // Extract session-timeout + for _, key := range []string{"session-timeout", "sessionTimeout"} { + if stVal, hasSessionTimeout := mcpObj[key]; hasSessionTimeout { + if stStr, ok := stVal.(string); ok && stStr != "" { + config.MCPSessionTimeout = stStr + engineLog.Printf("Extracted engine.mcp.session-timeout: %s", config.MCPSessionTimeout) + } + break + } + } + } + } + // Return the ID as the engineSetting for backwards compatibility engineLog.Printf("Extracted engine configuration: ID=%s", config.ID) return config.ID, config diff --git a/pkg/workflow/engine_config_test.go b/pkg/workflow/engine_config_test.go index 9fbc67d70b2..5bbfe3c2102 100644 --- a/pkg/workflow/engine_config_test.go +++ b/pkg/workflow/engine_config_test.go @@ -870,3 +870,70 @@ func TestBareMode_UnsupportedEngineNoFlag(t *testing.T) { } }) } + +// TestEngineMCPSessionTimeoutExtraction tests extraction of engine.mcp.session-timeout. +func TestEngineMCPSessionTimeoutExtraction(t *testing.T) { + compiler := NewCompiler() + + tests := []struct { + name string + frontmatter map[string]any + expectedTimeout string + }{ + { + name: "extracts session-timeout from engine.mcp", + frontmatter: map[string]any{ + "engine": map[string]any{ + "id": "copilot", + "mcp": map[string]any{ + "session-timeout": "4h", + }, + }, + }, + expectedTimeout: "4h", + }, + { + name: "extracts session-timeout using camelCase key", + frontmatter: map[string]any{ + "engine": map[string]any{ + "id": "copilot", + "mcp": map[string]any{ + "sessionTimeout": "30m", + }, + }, + }, + expectedTimeout: "30m", + }, + { + name: "no mcp section - empty session timeout", + frontmatter: map[string]any{ + "engine": map[string]any{ + "id": "copilot", + }, + }, + expectedTimeout: "", + }, + { + name: "mcp section without session-timeout - empty", + frontmatter: map[string]any{ + "engine": map[string]any{ + "id": "copilot", + "mcp": map[string]any{}, + }, + }, + expectedTimeout: "", + }, + } + + 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.MCPSessionTimeout != tt.expectedTimeout { + t.Errorf("MCPSessionTimeout = %q, want %q", config.MCPSessionTimeout, tt.expectedTimeout) + } + }) + } +} diff --git a/pkg/workflow/engine_validation.go b/pkg/workflow/engine_validation.go index 7e8d542dccb..0260cc9cf8b 100644 --- a/pkg/workflow/engine_validation.go +++ b/pkg/workflow/engine_validation.go @@ -41,6 +41,7 @@ import ( "path/filepath" "regexp" "strings" + "time" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/constants" @@ -108,6 +109,34 @@ func (c *Compiler) validateEngineHarnessScript(workflowData *WorkflowData) error } } +// validateEngineMCPSessionTimeout validates optional engine.mcp.session-timeout configuration. +// The value must be a valid Go duration string between 5m and 12h. +func (c *Compiler) validateEngineMCPSessionTimeout(workflowData *WorkflowData) error { + if workflowData == nil || workflowData.EngineConfig == nil || workflowData.EngineConfig.MCPSessionTimeout == "" { + return nil + } + + raw := workflowData.EngineConfig.MCPSessionTimeout + + d, err := time.ParseDuration(raw) + if err != nil { + return fmt.Errorf("engine.mcp.session-timeout: invalid duration %q. Must be a valid Go duration string (e.g. \"30m\", \"4h\", \"6h\").\n\nExamples:\n engine:\n mcp:\n session-timeout: 4h\n\nSee: %s", raw, constants.DocsEnginesURL) + } + + const minTimeout = 5 * time.Minute + const maxTimeout = 12 * time.Hour + + if d < minTimeout { + return fmt.Errorf("engine.mcp.session-timeout: %q is too short (minimum is 5m). Use a value between 5m and 12h.\n\nExamples:\n session-timeout: 30m\n session-timeout: 4h\n\nSee: %s", raw, constants.DocsEnginesURL) + } + if d > maxTimeout { + return fmt.Errorf("engine.mcp.session-timeout: %q is too long (maximum is 12h). Use a value between 5m and 12h.\n\nExamples:\n session-timeout: 4h\n session-timeout: 12h\n\nSee: %s", raw, constants.DocsEnginesURL) + } + + engineValidationLog.Printf("engine.mcp.session-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 f3248c045d7..d98961357f2 100644 --- a/pkg/workflow/engine_validation_test.go +++ b/pkg/workflow/engine_validation_test.go @@ -401,3 +401,108 @@ func TestValidateEngineHarnessScript(t *testing.T) { }) } } + +// TestValidateEngineMCPSessionTimeout tests the validateEngineMCPSessionTimeout function. +func TestValidateEngineMCPSessionTimeout(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 session timeout - no error", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot"}, + }, + expectError: false, + }, + { + name: "valid duration 4h", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPSessionTimeout: "4h"}, + }, + expectError: false, + }, + { + name: "valid duration 30m", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPSessionTimeout: "30m"}, + }, + expectError: false, + }, + { + name: "valid duration 12h", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPSessionTimeout: "12h"}, + }, + expectError: false, + }, + { + name: "valid duration 5m (minimum)", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPSessionTimeout: "5m"}, + }, + expectError: false, + }, + { + name: "invalid duration string", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPSessionTimeout: "2hours"}, + }, + expectError: true, + errorSubstr: "invalid duration", + }, + { + name: "too short - 4m", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPSessionTimeout: "4m"}, + }, + expectError: true, + errorSubstr: "too short", + }, + { + name: "too long - 13h", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPSessionTimeout: "13h"}, + }, + expectError: true, + errorSubstr: "too long", + }, + { + name: "plain integer - not valid Go duration", + workflow: &WorkflowData{ + EngineConfig: &EngineConfig{ID: "copilot", MCPSessionTimeout: "3600"}, + }, + expectError: true, + errorSubstr: "invalid duration", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + compiler := NewCompiler() + err := compiler.validateEngineMCPSessionTimeout(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 session-timeout validation to pass") + }) + } +} diff --git a/pkg/workflow/mcp_gateway_config.go b/pkg/workflow/mcp_gateway_config.go index fd234c8b300..8ef74536506 100644 --- a/pkg/workflow/mcp_gateway_config.go +++ b/pkg/workflow/mcp_gateway_config.go @@ -134,6 +134,12 @@ func buildMCPGatewayConfig(workflowData *WorkflowData) *MCPGatewayRuntimeConfig // OTLPEndpoint and OTLPHeaders are read from workflowData fields set by injectOTLPConfig. // These compile-time values (including GitHub Actions expressions such as ${{ secrets.X }}) // are written directly into the gateway config JSON. + // + // SessionTimeout comes from engine.mcp.session-timeout in frontmatter (via EngineConfig). + var sessionTimeout string + if workflowData.EngineConfig != nil { + sessionTimeout = workflowData.EngineConfig.MCPSessionTimeout + } return &MCPGatewayRuntimeConfig{ Port: int(DefaultMCPGatewayPort), // Will be formatted as "${MCP_GATEWAY_PORT}" in renderer Domain: "${MCP_GATEWAY_DOMAIN}", // Gateway variable expression @@ -143,6 +149,7 @@ func buildMCPGatewayConfig(workflowData *WorkflowData) *MCPGatewayRuntimeConfig PayloadSizeThreshold: payloadSizeThreshold, // Size threshold in bytes 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) // 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 840796557ca..5aa150af8bd 100644 --- a/pkg/workflow/mcp_gateway_config_test.go +++ b/pkg/workflow/mcp_gateway_config_test.go @@ -369,6 +369,38 @@ func TestBuildMCPGatewayConfig(t *testing.T) { KeepaliveInterval: -1, }, }, + { + name: "propagates session-timeout from engine config", + workflowData: &WorkflowData{ + EngineConfig: &EngineConfig{ + ID: "copilot", + MCPSessionTimeout: "4h", + }, + 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", + }, + }, + { + name: "empty session-timeout when engine config is nil", + workflowData: &WorkflowData{ + 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: "", + }, + }, } for _, tt := range tests { @@ -386,6 +418,7 @@ func TestBuildMCPGatewayConfig(t *testing.T) { assert.Equal(t, tt.expected.PayloadSizeThreshold, result.PayloadSizeThreshold, "PayloadSizeThreshold should match") 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") } }) } diff --git a/pkg/workflow/mcp_renderer.go b/pkg/workflow/mcp_renderer.go index ad9efaa1dd2..112cdc98ceb 100644 --- a/pkg/workflow/mcp_renderer.go +++ b/pkg/workflow/mcp_renderer.go @@ -188,6 +188,9 @@ func RenderJSONMCPConfig( if options.GatewayConfig.KeepaliveInterval != 0 { fmt.Fprintf(&configBuilder, ",\n \"keepaliveInterval\": %d", options.GatewayConfig.KeepaliveInterval) } + if options.GatewayConfig.SessionTimeout != "" { + fmt.Fprintf(&configBuilder, ",\n \"sessionTimeout\": %q", options.GatewayConfig.SessionTimeout) + } // 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 c5aeb5c2738..8e50ca8fa9d 100644 --- a/pkg/workflow/mcp_renderer_test.go +++ b/pkg/workflow/mcp_renderer_test.go @@ -597,3 +597,74 @@ func TestRenderJSONMCPConfig_OTLPGateway(t *testing.T) { }) } } + +// TestRenderJSONMCPConfig_SessionTimeout verifies that sessionTimeout is emitted +// in the gateway JSON section when set on the MCPGatewayRuntimeConfig. +func TestRenderJSONMCPConfig_SessionTimeout(t *testing.T) { + tests := []struct { + name string + sessionTimeout string + wantField bool + }{ + { + name: "includes sessionTimeout when set", + sessionTimeout: "4h", + wantField: true, + }, + { + name: "omits sessionTimeout when empty", + sessionTimeout: "", + wantField: false, + }, + { + name: "includes sessionTimeout 30m", + sessionTimeout: "30m", + wantField: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gatewayConfig := &MCPGatewayRuntimeConfig{ + Domain: "localhost", + APIKey: "test-api-key", + SessionTimeout: tt.sessionTimeout, + } + + 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, `"sessionTimeout":`) + if hasField != tt.wantField { + t.Errorf("sessionTimeout field presence = %v, want %v\noutput:\n%s", hasField, tt.wantField, result) + } + + if tt.wantField && tt.sessionTimeout != "" { + expected := `"sessionTimeout": "` + tt.sessionTimeout + `"` + 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 aaccca06e29..e0a825ae800 100644 --- a/pkg/workflow/tools_types.go +++ b/pkg/workflow/tools_types.go @@ -441,6 +441,7 @@ type MCPGatewayRuntimeConfig struct { PayloadSizeThreshold int `yaml:"payload-size-threshold,omitempty"` // Size threshold in bytes for storing payloads to disk (default: 524288 = 512KB) 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 (6h) 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) } From 93bc34a48bfa2a2777ab05c2321a3a09902fe981 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 16:55:05 +0000 Subject: [PATCH 3/6] Extract MCPSessionTimeoutMin/Max to named constants in pkg/constants Agent-Logs-Url: https://github.com/github/gh-aw/sessions/28633935-f9d4-454d-b410-68ff66ca2853 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- pkg/constants/constants.go | 6 ++++++ pkg/workflow/engine_validation.go | 7 ++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index f3e7323eb79..69a2f94aa0e 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -216,6 +216,12 @@ const DefaultToolTimeout = 60 * time.Second // DefaultMCPStartupTimeout is the default timeout for MCP server startup const DefaultMCPStartupTimeout = 120 * time.Second +// MCPSessionTimeoutMin is the minimum allowed value for engine.mcp.session-timeout (5 minutes). +const MCPSessionTimeoutMin = 5 * time.Minute + +// MCPSessionTimeoutMax is the maximum allowed value for engine.mcp.session-timeout (12 hours). +const MCPSessionTimeoutMax = 12 * time.Hour + // DefaultActivationJobRunnerImage is the default runner image for activation and pre-activation jobs const DefaultActivationJobRunnerImage = "ubuntu-slim" diff --git a/pkg/workflow/engine_validation.go b/pkg/workflow/engine_validation.go index 0260cc9cf8b..5c8b71f649f 100644 --- a/pkg/workflow/engine_validation.go +++ b/pkg/workflow/engine_validation.go @@ -123,13 +123,10 @@ func (c *Compiler) validateEngineMCPSessionTimeout(workflowData *WorkflowData) e return fmt.Errorf("engine.mcp.session-timeout: invalid duration %q. Must be a valid Go duration string (e.g. \"30m\", \"4h\", \"6h\").\n\nExamples:\n engine:\n mcp:\n session-timeout: 4h\n\nSee: %s", raw, constants.DocsEnginesURL) } - const minTimeout = 5 * time.Minute - const maxTimeout = 12 * time.Hour - - if d < minTimeout { + if d < constants.MCPSessionTimeoutMin { return fmt.Errorf("engine.mcp.session-timeout: %q is too short (minimum is 5m). Use a value between 5m and 12h.\n\nExamples:\n session-timeout: 30m\n session-timeout: 4h\n\nSee: %s", raw, constants.DocsEnginesURL) } - if d > maxTimeout { + if d > constants.MCPSessionTimeoutMax { return fmt.Errorf("engine.mcp.session-timeout: %q is too long (maximum is 12h). Use a value between 5m and 12h.\n\nExamples:\n session-timeout: 4h\n session-timeout: 12h\n\nSee: %s", raw, constants.DocsEnginesURL) } From 47618f6626fae205fab2831e0818ead8b545aa25 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 30 Apr 2026 17:13:33 +0000 Subject: [PATCH 4/6] docs(adr): add draft ADR-29354 for per-workflow MCP session timeout Co-Authored-By: Claude Sonnet 4.6 --- ...-session-timeout-via-engine-frontmatter.md | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 docs/adr/29354-per-workflow-mcp-session-timeout-via-engine-frontmatter.md diff --git a/docs/adr/29354-per-workflow-mcp-session-timeout-via-engine-frontmatter.md b/docs/adr/29354-per-workflow-mcp-session-timeout-via-engine-frontmatter.md new file mode 100644 index 00000000000..8752e6b5a9b --- /dev/null +++ b/docs/adr/29354-per-workflow-mcp-session-timeout-via-engine-frontmatter.md @@ -0,0 +1,84 @@ +# ADR-29354: Per-Workflow MCP Session Timeout via `engine.mcp` Frontmatter + +**Date**: 2026-04-30 +**Status**: Draft +**Deciders**: lpcox, Copilot SWE Agent + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +All workflows sharing a single MCP gateway instance inherit the same session lifetime, controlled globally by the `MCP_GATEWAY_SESSION_TIMEOUT` environment variable set on the gateway container. There is no mechanism for individual workflow authors to declare the session lifetime their workflow needs. This is a problem because workflows have wildly different runtime profiles: a short code-review workflow may finish in minutes while a large-scale migration may run for several hours and require a long-lived MCP session to avoid mid-task reconnection failures. + +### Decision + +We will add an optional `engine.mcp.session-timeout` frontmatter field that workflow authors can use to declare the MCP gateway session lifetime required by their workflow. The compiler will parse the value, validate it is a Go duration string in the range 5m–12h, and propagate it as `sessionTimeout` into the gateway's stdin configuration JSON. The field coexists with the existing `MCP_GATEWAY_SESSION_TIMEOUT` env-var mechanism: the per-workflow value in the stdin config takes precedence over the env var, which takes precedence over the gateway's built-in default of 6h. + +### Alternatives Considered + +#### Alternative 1: `MCP_GATEWAY_SESSION_TIMEOUT` env-var-only (status quo) + +The existing approach relies on a single environment variable that applies uniformly to all workflows on the gateway instance. It was considered acceptable because infrastructure operators can tune it. It was not chosen as the long-term solution because it requires infrastructure-level changes (redeploy) to accommodate per-workflow requirements and cannot express that two workflows running on the same gateway need different lifetimes. + +#### Alternative 2: Place the field under `sandbox.mcp` instead of `engine.mcp` + +The `sandbox.mcp` sub-object already hosts gateway-level settings like `trusted-bots` and `keepalive-interval`, which are operational concerns about how the gateway behaves. Session timeout was considered for placement there too. However, `engine.mcp` was chosen because session lifetime is an engine-level concern: it describes how long the engine's connection to the MCP gateway must remain alive, not a property of the sandbox environment itself. Keeping engine-specific knobs under `engine.*` maintains a cleaner frontmatter taxonomy. + +#### Alternative 3: Integer seconds field instead of a Go duration string + +Using an integer (seconds) would be simpler to parse and validate. A Go duration string was chosen instead because the rest of the codebase uses `time.Duration`-compatible strings for timeout configuration, it is more readable in YAML (`4h` versus `14400`), and it aligns with the format already used by `MCP_GATEWAY_SESSION_TIMEOUT` on the gateway side. + +### Consequences + +#### Positive +- Workflow authors can express session lifetime requirements declaratively alongside the workflow definition, without requiring infrastructure changes. +- Long-running workflows (multi-hour migrations, large batch operations) get appropriately long sessions; short workflows can use shorter values, freeing gateway resources sooner. +- Compile-time validation (5m–12h range, valid Go duration format) surfaces misconfiguration early with actionable error messages. + +#### Negative +- Adds another frontmatter field and a new `engine.mcp` sub-object, increasing frontmatter surface area. +- The three-level precedence rule (stdin config > env var > gateway default) is non-obvious and must be documented; operators may be surprised that a per-workflow value overrides their env-var setting. +- Any workflow that sets `session-timeout` to a very long value (approaching 12h) could hold gateway resources for extended periods if a run hangs or is abandoned. + +#### Neutral +- Constants `MCPSessionTimeoutMin` (5m) and `MCPSessionTimeoutMax` (12h) are added to `pkg/constants`, establishing a named home for MCP session bounds that future policies can reference. +- Both kebab-case (`session-timeout`) and camelCase (`sessionTimeout`) key spellings are accepted during extraction for consistency with other frontmatter fields that support dual spellings. +- The JSON Schema for `engine_config` gains a new `mcp` object definition with `additionalProperties: false`, which prevents unrecognized MCP sub-keys from silently passing validation. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### Frontmatter Field + +1. The `engine.mcp.session-timeout` frontmatter field is **OPTIONAL**. +2. When present and non-empty, the value **MUST** be a valid Go duration string parseable by `time.ParseDuration`. +3. When present, the value **MUST** be greater than or equal to `5m` (as defined by `constants.MCPSessionTimeoutMin`). +4. When present, the value **MUST** be less than or equal to `12h` (as defined by `constants.MCPSessionTimeoutMax`). +5. Implementations **MUST** accept both kebab-case (`session-timeout`) and camelCase (`sessionTimeout`) as equivalent spellings for this key. +6. Implementations **MUST NOT** accept any other spellings or sibling keys under `engine.mcp`; the `mcp` sub-object **MUST** be declared with `additionalProperties: false` in the JSON Schema. + +### Validation + +1. The compiler **MUST** validate `engine.mcp.session-timeout` during `ParseWorkflowFile`, before the workflow is further processed. +2. A validation error **MUST** produce a human-readable message that includes the offending value, the valid range, and a corrected example. +3. Workflows that omit `engine.mcp.session-timeout` or set it to an empty string **MUST** be treated as if the field were absent; no default value **SHALL** be injected by the compiler. + +### Propagation + +1. When `engine.mcp.session-timeout` is set to a valid non-empty value, the compiler **MUST** propagate it into `MCPGatewayRuntimeConfig.SessionTimeout`. +2. The renderer **MUST** emit `"sessionTimeout": ""` inside the `gateway` JSON block when `MCPGatewayRuntimeConfig.SessionTimeout` is non-empty. +3. The renderer **MUST NOT** emit `"sessionTimeout"` when `MCPGatewayRuntimeConfig.SessionTimeout` is empty. +4. The emitted value **MUST** take precedence over the `MCP_GATEWAY_SESSION_TIMEOUT` environment variable at the gateway runtime. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/25178770914) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* From b432096375584a82330688f1722cce53ad753769 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 17:23:21 +0000 Subject: [PATCH 5/6] Address review feedback: drop camelCase alias, add string-API validation, fix docs references - Drop sessionTimeout camelCase alias in engine.go (schema only allows session-timeout) - Remove camelCase test case from engine_config_test.go - Add validateEngineMCPSessionTimeout call to ParseWorkflowString (compiler_string_api.go) - Fix OpenTelemetry table reference: Section 4.1.3.6 -> 4.1.3.7 in mcp-gateway.md - Update tools_types.go comment to reflect full precedence chain - Update schema description to mention MCP_GATEWAY_SESSION_TIMEOUT env var precedence Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5441c30c-eea8-47fe-991f-07f56d21b15a Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- docs/src/content/docs/reference/mcp-gateway.md | 2 +- pkg/parser/schemas/main_workflow_schema.json | 2 +- pkg/workflow/compiler_string_api.go | 5 +++++ pkg/workflow/engine.go | 13 +++++-------- pkg/workflow/engine_config_test.go | 12 ------------ pkg/workflow/tools_types.go | 2 +- 6 files changed, 13 insertions(+), 23 deletions(-) diff --git a/docs/src/content/docs/reference/mcp-gateway.md b/docs/src/content/docs/reference/mcp-gateway.md index d4634fdcd57..2695166452f 100644 --- a/docs/src/content/docs/reference/mcp-gateway.md +++ b/docs/src/content/docs/reference/mcp-gateway.md @@ -252,7 +252,7 @@ The `gateway` section is required and configures gateway-specific behavior: | `trustedBots` | array[string] | No | Additional GitHub bot identity strings (e.g., `github-actions[bot]`) passed to the gateway and merged with its built-in trusted identity list. This field is additive — it extends the internal list but cannot remove built-in entries. | | `keepaliveInterval` | integer | No | Keepalive ping interval in seconds for HTTP MCP backends. Prevents session expiry during long-running tasks. Use `-1` to disable, `0` or unset for gateway default (1500s = 25 min), or a positive integer for a custom interval. | | `sessionTimeout` | string | No | Session timeout for MCP gateway sessions as a Go duration string (e.g. `"30m"`, `"4h"`, `"6h"`). Empty or omitted uses the gateway default (6h). Must be between 5m and 12h when set by the workflow compiler (infrastructure operators may override via `MCP_GATEWAY_SESSION_TIMEOUT` env var). | -| `opentelemetry` | object | No | OpenTelemetry configuration for emitting distributed tracing events for MCP calls. See Section 4.1.3.6 for details. | +| `opentelemetry` | object | No | OpenTelemetry configuration for emitting distributed tracing events for MCP calls. See Section 4.1.3.7 for details. | #### 4.1.3.1 Payload Directory Path Validation diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index d3d9afaf7d7..f3354e592e7 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -9826,7 +9826,7 @@ "properties": { "session-timeout": { "type": "string", - "description": "Session timeout for MCP gateway sessions as a Go duration string (e.g. \"30m\", \"4h\", \"6h\"). Must be between 5m and 12h. Empty or omitted uses the gateway default (6h). Longer timeouts benefit multi-hour workflows such as large-scale migrations; shorter values free gateway resources sooner.", + "description": "Session timeout for MCP gateway sessions as a Go duration string (e.g. \"30m\", \"4h\", \"6h\"). Must be between 5m and 12h. 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"] } }, diff --git a/pkg/workflow/compiler_string_api.go b/pkg/workflow/compiler_string_api.go index 6d824874a3d..df69f946db4 100644 --- a/pkg/workflow/compiler_string_api.go +++ b/pkg/workflow/compiler_string_api.go @@ -135,6 +135,11 @@ func (c *Compiler) ParseWorkflowString(content string, virtualPath string) (*Wor return nil, fmt.Errorf("%s: %w", cleanPath, err) } + // Validate optional engine.mcp.session-timeout configuration. + if err := c.validateEngineMCPSessionTimeout(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 249b49ad96f..0ebd28f32dd 100644 --- a/pkg/workflow/engine.go +++ b/pkg/workflow/engine.go @@ -324,14 +324,11 @@ func (c *Compiler) ExtractEngineConfig(frontmatter map[string]any) (string, *Eng // Extract optional 'mcp' sub-object (engine-level MCP gateway configuration) if mcpVal, hasMCP := engineObj["mcp"]; hasMCP { if mcpObj, ok := mcpVal.(map[string]any); ok { - // Extract session-timeout - for _, key := range []string{"session-timeout", "sessionTimeout"} { - if stVal, hasSessionTimeout := mcpObj[key]; hasSessionTimeout { - if stStr, ok := stVal.(string); ok && stStr != "" { - config.MCPSessionTimeout = stStr - engineLog.Printf("Extracted engine.mcp.session-timeout: %s", config.MCPSessionTimeout) - } - break + // Extract session-timeout (kebab-case only; camelCase is not supported) + if stVal, hasSessionTimeout := mcpObj["session-timeout"]; hasSessionTimeout { + if stStr, ok := stVal.(string); ok && stStr != "" { + config.MCPSessionTimeout = stStr + engineLog.Printf("Extracted engine.mcp.session-timeout: %s", config.MCPSessionTimeout) } } } diff --git a/pkg/workflow/engine_config_test.go b/pkg/workflow/engine_config_test.go index 5bbfe3c2102..d12630baa4b 100644 --- a/pkg/workflow/engine_config_test.go +++ b/pkg/workflow/engine_config_test.go @@ -892,18 +892,6 @@ func TestEngineMCPSessionTimeoutExtraction(t *testing.T) { }, expectedTimeout: "4h", }, - { - name: "extracts session-timeout using camelCase key", - frontmatter: map[string]any{ - "engine": map[string]any{ - "id": "copilot", - "mcp": map[string]any{ - "sessionTimeout": "30m", - }, - }, - }, - expectedTimeout: "30m", - }, { name: "no mcp section - empty session timeout", frontmatter: map[string]any{ diff --git a/pkg/workflow/tools_types.go b/pkg/workflow/tools_types.go index e0a825ae800..0199deddc2d 100644 --- a/pkg/workflow/tools_types.go +++ b/pkg/workflow/tools_types.go @@ -441,7 +441,7 @@ type MCPGatewayRuntimeConfig struct { PayloadSizeThreshold int `yaml:"payload-size-threshold,omitempty"` // Size threshold in bytes for storing payloads to disk (default: 524288 = 512KB) 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 (6h) + 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) 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) } From b4f3831fdde508afd429792d17fad18c75631081 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 17:34:01 +0000 Subject: [PATCH 6/6] Remove MCPSessionTimeoutMax: no upper bound on session-timeout - Remove MCPSessionTimeoutMax constant from pkg/constants - Drop upper-bound check from validateEngineMCPSessionTimeout (min 5m only) - Replace "too long - 13h" test case with "valid duration 24h (no upper bound)" - Update schema description, docs table, compliance rules, changelog, and ADR to reflect no upper bound Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4b9e19de-6066-4b7e-a4a9-340a513242e1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- ...-mcp-session-timeout-via-engine-frontmatter.md | 15 +++++++-------- docs/src/content/docs/reference/mcp-gateway.md | 6 +++--- pkg/constants/constants.go | 3 --- pkg/parser/schemas/main_workflow_schema.json | 2 +- pkg/workflow/engine_validation.go | 9 +++------ pkg/workflow/engine_validation_test.go | 7 +++---- 6 files changed, 17 insertions(+), 25 deletions(-) diff --git a/docs/adr/29354-per-workflow-mcp-session-timeout-via-engine-frontmatter.md b/docs/adr/29354-per-workflow-mcp-session-timeout-via-engine-frontmatter.md index 8752e6b5a9b..ffa457f1fc8 100644 --- a/docs/adr/29354-per-workflow-mcp-session-timeout-via-engine-frontmatter.md +++ b/docs/adr/29354-per-workflow-mcp-session-timeout-via-engine-frontmatter.md @@ -40,11 +40,11 @@ Using an integer (seconds) would be simpler to parse and validate. A Go duration #### Negative - Adds another frontmatter field and a new `engine.mcp` sub-object, increasing frontmatter surface area. - The three-level precedence rule (stdin config > env var > gateway default) is non-obvious and must be documented; operators may be surprised that a per-workflow value overrides their env-var setting. -- Any workflow that sets `session-timeout` to a very long value (approaching 12h) could hold gateway resources for extended periods if a run hangs or is abandoned. +- Any workflow that sets `session-timeout` to a very long value could hold gateway resources for extended periods if a run hangs or is abandoned. #### Neutral -- Constants `MCPSessionTimeoutMin` (5m) and `MCPSessionTimeoutMax` (12h) are added to `pkg/constants`, establishing a named home for MCP session bounds that future policies can reference. -- Both kebab-case (`session-timeout`) and camelCase (`sessionTimeout`) key spellings are accepted during extraction for consistency with other frontmatter fields that support dual spellings. +- Constants `MCPSessionTimeoutMin` (5m) is added to `pkg/constants`, establishing a named home for the MCP session minimum bound that future policies can reference. +- Only the kebab-case (`session-timeout`) key spelling is accepted during extraction; the JSON Schema uses `additionalProperties: false` to reject any other keys under `engine.mcp`. - The JSON Schema for `engine_config` gains a new `mcp` object definition with `additionalProperties: false`, which prevents unrecognized MCP sub-keys from silently passing validation. --- @@ -57,15 +57,14 @@ Using an integer (seconds) would be simpler to parse and validate. A Go duration 1. The `engine.mcp.session-timeout` frontmatter field is **OPTIONAL**. 2. When present and non-empty, the value **MUST** be a valid Go duration string parseable by `time.ParseDuration`. -3. When present, the value **MUST** be greater than or equal to `5m` (as defined by `constants.MCPSessionTimeoutMin`). -4. When present, the value **MUST** be less than or equal to `12h` (as defined by `constants.MCPSessionTimeoutMax`). -5. Implementations **MUST** accept both kebab-case (`session-timeout`) and camelCase (`sessionTimeout`) as equivalent spellings for this key. -6. Implementations **MUST NOT** accept any other spellings or sibling keys under `engine.mcp`; the `mcp` sub-object **MUST** be declared with `additionalProperties: false` in the JSON Schema. +3. When present, the value **MUST** be greater than or equal to `5m` (as defined by `constants.MCPSessionTimeoutMin`); there is no upper bound. +4. Implementations **MUST** accept only kebab-case (`session-timeout`) for this key; camelCase is not supported. +5. Implementations **MUST NOT** accept any other spellings or sibling keys under `engine.mcp`; the `mcp` sub-object **MUST** be declared with `additionalProperties: false` in the JSON Schema. ### Validation 1. The compiler **MUST** validate `engine.mcp.session-timeout` during `ParseWorkflowFile`, before the workflow is further processed. -2. A validation error **MUST** produce a human-readable message that includes the offending value, the valid range, and a corrected example. +2. A validation error **MUST** produce a human-readable message that includes the offending value, the minimum bound, and a corrected example. 3. Workflows that omit `engine.mcp.session-timeout` or set it to an empty string **MUST** be treated as if the field were absent; no default value **SHALL** be injected by the compiler. ### Propagation diff --git a/docs/src/content/docs/reference/mcp-gateway.md b/docs/src/content/docs/reference/mcp-gateway.md index 2695166452f..887a78a34ce 100644 --- a/docs/src/content/docs/reference/mcp-gateway.md +++ b/docs/src/content/docs/reference/mcp-gateway.md @@ -251,7 +251,7 @@ The `gateway` section is required and configures gateway-specific behavior: | `payloadSizeThreshold` | integer | No | Size threshold in bytes for storing payloads to disk (default: 524288 = 512KB) | | `trustedBots` | array[string] | No | Additional GitHub bot identity strings (e.g., `github-actions[bot]`) passed to the gateway and merged with its built-in trusted identity list. This field is additive — it extends the internal list but cannot remove built-in entries. | | `keepaliveInterval` | integer | No | Keepalive ping interval in seconds for HTTP MCP backends. Prevents session expiry during long-running tasks. Use `-1` to disable, `0` or unset for gateway default (1500s = 25 min), or a positive integer for a custom interval. | -| `sessionTimeout` | string | No | Session timeout for MCP gateway sessions as a Go duration string (e.g. `"30m"`, `"4h"`, `"6h"`). Empty or omitted uses the gateway default (6h). Must be between 5m and 12h when set by the workflow compiler (infrastructure operators may override via `MCP_GATEWAY_SESSION_TIMEOUT` env var). | +| `sessionTimeout` | string | No | Session timeout for MCP gateway sessions as a Go duration string (e.g. `"30m"`, `"4h"`, `"24h"`). Empty or omitted uses the gateway default (6h). Must be at least 5m when set by the workflow compiler (no upper bound; infrastructure operators may override via `MCP_GATEWAY_SESSION_TIMEOUT` env var). | | `opentelemetry` | object | No | OpenTelemetry configuration for emitting distributed tracing events for MCP calls. See Section 4.1.3.7 for details. | #### 4.1.3.1 Payload Directory Path Validation @@ -486,7 +486,7 @@ engine: **Compliance rules**: - `sessionTimeout` MUST be a valid Go duration string when present (e.g. `"30m"`, `"4h"`) -- The workflow compiler enforces a minimum of `5m` and a maximum of `12h` for author-specified values +- The workflow compiler enforces a minimum of `5m` for author-specified values (no upper bound) - Infrastructure operators may set `MCP_GATEWAY_SESSION_TIMEOUT` on the gateway container to override the default for all workflows; a per-workflow `sessionTimeout` in the stdin config takes precedence - When unset, the gateway uses its built-in default (6h) @@ -1898,7 +1898,7 @@ Content-Type: application/json - **Added**: `sessionTimeout` field to gateway configuration (Section 4.1.3, 4.1.3.6) - Optional Go duration string for controlling MCP session lifetime (e.g. `"4h"`, `"30m"`) - Precedence: stdin config `sessionTimeout` > `MCP_GATEWAY_SESSION_TIMEOUT` env var > gateway default (6h) - - Workflow authors configure via `engine.mcp.session-timeout` in frontmatter; the compiler validates (min 5m, max 12h) and emits it into the gateway config JSON + - Workflow authors configure via `engine.mcp.session-timeout` in frontmatter; the compiler validates (min 5m, no upper bound) and emits it into the gateway config JSON - **Added**: Section 4.1.3.6 — Session Timeout Configuration - **Renumbered**: Former Section 4.1.3.6 (OpenTelemetry) to 4.1.3.7 - **Updated**: JSON Schema with `sessionTimeout` property in `gatewayConfig` definition diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 69a2f94aa0e..4921583d8f7 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -219,9 +219,6 @@ const DefaultMCPStartupTimeout = 120 * time.Second // MCPSessionTimeoutMin is the minimum allowed value for engine.mcp.session-timeout (5 minutes). const MCPSessionTimeoutMin = 5 * time.Minute -// MCPSessionTimeoutMax is the maximum allowed value for engine.mcp.session-timeout (12 hours). -const MCPSessionTimeoutMax = 12 * time.Hour - // 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 f3354e592e7..5eab3b20bd1 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -9826,7 +9826,7 @@ "properties": { "session-timeout": { "type": "string", - "description": "Session timeout for MCP gateway sessions as a Go duration string (e.g. \"30m\", \"4h\", \"6h\"). Must be between 5m and 12h. 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.", + "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"] } }, diff --git a/pkg/workflow/engine_validation.go b/pkg/workflow/engine_validation.go index 5c8b71f649f..e289649a333 100644 --- a/pkg/workflow/engine_validation.go +++ b/pkg/workflow/engine_validation.go @@ -110,7 +110,7 @@ func (c *Compiler) validateEngineHarnessScript(workflowData *WorkflowData) error } // validateEngineMCPSessionTimeout validates optional engine.mcp.session-timeout configuration. -// The value must be a valid Go duration string between 5m and 12h. +// The value must be a valid Go duration string of at least 5m (no upper bound). func (c *Compiler) validateEngineMCPSessionTimeout(workflowData *WorkflowData) error { if workflowData == nil || workflowData.EngineConfig == nil || workflowData.EngineConfig.MCPSessionTimeout == "" { return nil @@ -120,14 +120,11 @@ func (c *Compiler) validateEngineMCPSessionTimeout(workflowData *WorkflowData) e d, err := time.ParseDuration(raw) if err != nil { - return fmt.Errorf("engine.mcp.session-timeout: invalid duration %q. Must be a valid Go duration string (e.g. \"30m\", \"4h\", \"6h\").\n\nExamples:\n engine:\n mcp:\n session-timeout: 4h\n\nSee: %s", raw, constants.DocsEnginesURL) + return fmt.Errorf("engine.mcp.session-timeout: invalid duration %q. Must be a valid Go duration string (e.g. \"30m\", \"4h\", \"24h\").\n\nExamples:\n engine:\n mcp:\n session-timeout: 4h\n\nSee: %s", raw, constants.DocsEnginesURL) } if d < constants.MCPSessionTimeoutMin { - return fmt.Errorf("engine.mcp.session-timeout: %q is too short (minimum is 5m). Use a value between 5m and 12h.\n\nExamples:\n session-timeout: 30m\n session-timeout: 4h\n\nSee: %s", raw, constants.DocsEnginesURL) - } - if d > constants.MCPSessionTimeoutMax { - return fmt.Errorf("engine.mcp.session-timeout: %q is too long (maximum is 12h). Use a value between 5m and 12h.\n\nExamples:\n session-timeout: 4h\n session-timeout: 12h\n\nSee: %s", raw, constants.DocsEnginesURL) + return fmt.Errorf("engine.mcp.session-timeout: %q is too short (minimum is 5m).\n\nExamples:\n session-timeout: 30m\n session-timeout: 4h\n\nSee: %s", raw, constants.DocsEnginesURL) } engineValidationLog.Printf("engine.mcp.session-timeout validated: %s (%s)", raw, d) diff --git a/pkg/workflow/engine_validation_test.go b/pkg/workflow/engine_validation_test.go index d98961357f2..6743a59f3bb 100644 --- a/pkg/workflow/engine_validation_test.go +++ b/pkg/workflow/engine_validation_test.go @@ -472,12 +472,11 @@ func TestValidateEngineMCPSessionTimeout(t *testing.T) { errorSubstr: "too short", }, { - name: "too long - 13h", + name: "valid duration 24h (no upper bound)", workflow: &WorkflowData{ - EngineConfig: &EngineConfig{ID: "copilot", MCPSessionTimeout: "13h"}, + EngineConfig: &EngineConfig{ID: "copilot", MCPSessionTimeout: "24h"}, }, - expectError: true, - errorSubstr: "too long", + expectError: false, }, { name: "plain integer - not valid Go duration",