diff --git a/pkg/parser/import_field_extractor.go b/pkg/parser/import_field_extractor.go index daf4dd1eac8..d08031ac5ab 100644 --- a/pkg/parser/import_field_extractor.go +++ b/pkg/parser/import_field_extractor.go @@ -63,6 +63,9 @@ type importAccumulator struct { topLevelGitHubApp string // JSON-encoded GitHubAppConfig // Checkout configs from all imported files (append in order; main workflow's checkouts take precedence) checkouts []string // JSON-encoded checkout values, one per import + // First engine.mcp.tool-timeout / engine.mcp.session-timeout found across all imported files (first-wins strategy) + mergedEngineMCPToolTimeout string // Go duration string (e.g. "10m", "30s") + mergedEngineMCPSessionTimeout string // Go duration string (e.g. "4h", "30m") // Best-effort sub-agent frontmatter warnings collected during BFS traversal. warnings []string } @@ -225,11 +228,57 @@ func (acc *importAccumulator) extractAllImportFields(content []byte, item import } } - // Extract engines from imported file - engineContent, err := extractFieldJSONFromMap(fm, "engine", "") - if err == nil && engineContent != "" { + // Extract engines from imported file. + // Engine configs with only `mcp` sub-keys (no `id` or `runtime`) are not counted + // as engine specifications — they only carry MCP gateway settings. This prevents + // "multiple engine fields" errors when a shared workflow declares engine.mcp.* + // without specifying an engine ID. + if engineVal, hasEngine := fm["engine"]; hasEngine { log.Printf("Found engine config in import: %s", item.fullPath) - acc.engines = append(acc.engines, engineContent) + + switch v := engineVal.(type) { + case string: + // String engine (e.g. "copilot") — always counts as an engine spec. + if engineJSON, merr := json.Marshal(v); merr == nil { + acc.engines = append(acc.engines, string(engineJSON)) + } + case map[string]any: + // Object engine — extract engine.mcp.* settings first, then decide + // whether to add to engines based on whether an engine ID is present. + if mcpVal, hasMCP := v["mcp"]; hasMCP { + if mcpMap, ok := mcpVal.(map[string]any); ok { + // Extract tool-timeout (first-wins across all imports) + if acc.mergedEngineMCPToolTimeout == "" { + if ttStr, ok := mcpMap["tool-timeout"].(string); ok && ttStr != "" { + acc.mergedEngineMCPToolTimeout = ttStr + log.Printf("Extracted engine.mcp.tool-timeout from import %s: %s", item.fullPath, ttStr) + } + } + // Extract session-timeout (first-wins across all imports) + if acc.mergedEngineMCPSessionTimeout == "" { + if stStr, ok := mcpMap["session-timeout"].(string); ok && stStr != "" { + acc.mergedEngineMCPSessionTimeout = stStr + log.Printf("Extracted engine.mcp.session-timeout from import %s: %s", item.fullPath, stStr) + } + } + } + } + // Only add to engines list if this config specifies an actual engine. + // Configs with only `mcp` settings are purely MCP gateway configuration + // and must not trigger the "multiple engine fields" validation error. + _, hasMCP := v["mcp"] + isMCPOnly := hasMCP && len(v) == 1 + if !isMCPOnly { + if engineJSON, merr := json.Marshal(v); merr == nil { + acc.engines = append(acc.engines, string(engineJSON)) + } + } + default: + // Unexpected type — marshal and add to preserve existing behavior. + if engineJSON, merr := json.Marshal(engineVal); merr == nil { + acc.engines = append(acc.engines, string(engineJSON)) + } + } } // Extract mcp-servers from imported file @@ -506,45 +555,47 @@ func (acc *importAccumulator) toImportsResult(topologicalOrder []string) *Import log.Printf("Building ImportsResult: importedFiles=%d, importPaths=%d, engines=%d, bots=%d, labels=%d", len(topologicalOrder), len(acc.importPaths), len(acc.engines), len(acc.bots), len(acc.labels)) return &ImportsResult{ - MergedTools: acc.toolsBuilder.String(), - MergedMCPServers: acc.mcpServersBuilder.String(), - MergedEngines: acc.engines, - MergedSafeOutputs: acc.safeOutputs, - MergedMCPScripts: acc.mcpScripts, - MergedMarkdown: acc.markdownBuilder.String(), - ImportPaths: acc.importPaths, - MergedSteps: acc.stepsBuilder.String(), - CopilotSetupSteps: acc.copilotSetupStepsBuilder.String(), - MergedPreSteps: acc.preStepsBuilder.String(), - MergedPreAgentSteps: acc.preAgentStepsBuilder.String(), - MergedRuntimes: acc.runtimesBuilder.String(), - MergedRunInstallScripts: acc.runInstallScripts, - MergedServices: acc.servicesBuilder.String(), - MergedNetwork: acc.networkBuilder.String(), - MergedPermissions: acc.permissionsBuilder.String(), - MergedSecretMasking: acc.secretMaskingBuilder.String(), - MergedBots: acc.bots, - MergedSkipRoles: acc.skipRoles, - MergedSkipBots: acc.skipBots, - MergedPostSteps: acc.postStepsBuilder.String(), - MergedLabels: acc.labels, - MergedCaches: acc.caches, - MergedJobs: acc.jobsBuilder.String(), - MergedEnv: acc.envBuilder.String(), - MergedEnvSources: acc.envSources, - MergedFeatures: acc.features, - MergedModels: acc.models, - MergedObservability: mergeObservabilityConfigs(acc.observabilityConfigs), - ImportedFiles: topologicalOrder, - AgentFile: acc.agentFile, - AgentImportSpec: acc.agentImportSpec, - RepositoryImports: acc.repositoryImports, - ImportInputs: acc.importInputs, - MergedActivationGitHubToken: acc.activationGitHubToken, - MergedActivationGitHubApp: acc.activationGitHubApp, - MergedTopLevelGitHubApp: acc.topLevelGitHubApp, - MergedCheckout: strings.Join(acc.checkouts, "\n"), - Warnings: acc.warnings, + MergedTools: acc.toolsBuilder.String(), + MergedMCPServers: acc.mcpServersBuilder.String(), + MergedEngines: acc.engines, + MergedSafeOutputs: acc.safeOutputs, + MergedMCPScripts: acc.mcpScripts, + MergedMarkdown: acc.markdownBuilder.String(), + ImportPaths: acc.importPaths, + MergedSteps: acc.stepsBuilder.String(), + CopilotSetupSteps: acc.copilotSetupStepsBuilder.String(), + MergedPreSteps: acc.preStepsBuilder.String(), + MergedPreAgentSteps: acc.preAgentStepsBuilder.String(), + MergedRuntimes: acc.runtimesBuilder.String(), + MergedRunInstallScripts: acc.runInstallScripts, + MergedServices: acc.servicesBuilder.String(), + MergedNetwork: acc.networkBuilder.String(), + MergedPermissions: acc.permissionsBuilder.String(), + MergedSecretMasking: acc.secretMaskingBuilder.String(), + MergedBots: acc.bots, + MergedSkipRoles: acc.skipRoles, + MergedSkipBots: acc.skipBots, + MergedPostSteps: acc.postStepsBuilder.String(), + MergedLabels: acc.labels, + MergedCaches: acc.caches, + MergedJobs: acc.jobsBuilder.String(), + MergedEnv: acc.envBuilder.String(), + MergedEnvSources: acc.envSources, + MergedFeatures: acc.features, + MergedModels: acc.models, + MergedObservability: mergeObservabilityConfigs(acc.observabilityConfigs), + ImportedFiles: topologicalOrder, + AgentFile: acc.agentFile, + AgentImportSpec: acc.agentImportSpec, + RepositoryImports: acc.repositoryImports, + ImportInputs: acc.importInputs, + MergedActivationGitHubToken: acc.activationGitHubToken, + MergedActivationGitHubApp: acc.activationGitHubApp, + MergedTopLevelGitHubApp: acc.topLevelGitHubApp, + MergedCheckout: strings.Join(acc.checkouts, "\n"), + MergedEngineMCPToolTimeout: acc.mergedEngineMCPToolTimeout, + MergedEngineMCPSessionTimeout: acc.mergedEngineMCPSessionTimeout, + Warnings: acc.warnings, } } diff --git a/pkg/parser/import_processor.go b/pkg/parser/import_processor.go index 5e1995ef22f..1df3efdf6e4 100644 --- a/pkg/parser/import_processor.go +++ b/pkg/parser/import_processor.go @@ -14,43 +14,45 @@ var importLog = logger.New("parser:import_processor") // ImportsResult holds the result of processing imports from frontmatter type ImportsResult struct { - MergedTools string // Merged tools configuration from all imports - MergedMCPServers string // Merged mcp-servers configuration from all imports - MergedEngines []string // Merged engine configurations from all imports - MergedSafeOutputs []string // Merged safe-outputs configurations from all imports - MergedMCPScripts []string // Merged mcp-scripts configurations from all imports - MergedMarkdown string // Only contains imports WITH inputs (for compile-time substitution) - ImportPaths []string // List of import file paths for runtime-import macro generation (replaces MergedMarkdown) - MergedSteps string // Merged steps configuration from all imports (excluding copilot-setup-steps) - CopilotSetupSteps string // Steps from copilot-setup-steps.yml (inserted at start) - MergedPreSteps string // Merged pre-steps configuration from all imports (prepended in order) - MergedPreAgentSteps string // Merged pre-agent-steps configuration from all imports (prepended in order) - MergedRuntimes string // Merged runtimes configuration from all imports - MergedRunInstallScripts bool // true if any imported workflow sets run-install-scripts: true (global or node-level) - MergedServices string // Merged services configuration from all imports - MergedNetwork string // Merged network configuration from all imports - MergedPermissions string // Merged permissions configuration from all imports - MergedSecretMasking string // Merged secret-masking steps from all imports - MergedBots []string // Merged bots list from all imports (union of bot names) - MergedSkipRoles []string // Merged skip-roles list from all imports (union of role names) - MergedSkipBots []string // Merged skip-bots list from all imports (union of usernames) - MergedActivationGitHubToken string // GitHub token from on.github-token in first imported workflow that defines it - MergedActivationGitHubApp string // JSON-encoded on.github-app from first imported workflow that defines it - MergedTopLevelGitHubApp string // JSON-encoded top-level github-app from first imported workflow that defines it - MergedCheckout string // JSON-encoded checkout configurations from imported workflows (one JSON value per line) - MergedPostSteps string // Merged post-steps configuration from all imports (appended in order) - MergedLabels []string // Merged labels from all imports (union of label names) - MergedCaches []string // Merged cache configurations from all imports (appended in order) - MergedJobs string // Merged jobs from imported YAML workflows (JSON format) - MergedEnv string // Merged env configuration from all imports (JSON format) - MergedEnvSources map[string]string // env var name → source import path (for conflict detection and lock file header listing) - MergedFeatures []map[string]any // Merged features configuration from all imports (parsed YAML structures) - MergedModels []map[string][]string // Merged model alias definitions from all imports (first import to define a key wins among imports) - MergedObservability string // Merged observability config (JSON) from all imports as an endpoint array (deduped by URL) - ImportedFiles []string // List of imported file paths (for manifest) - AgentFile string // Path to custom agent file (if imported) - AgentImportSpec string // Original import specification for agent file (e.g., "owner/repo/path@ref") - RepositoryImports []string // List of repository imports (format: "owner/repo@ref") for .github folder merging + MergedTools string // Merged tools configuration from all imports + MergedMCPServers string // Merged mcp-servers configuration from all imports + MergedEngines []string // Merged engine configurations from all imports + MergedSafeOutputs []string // Merged safe-outputs configurations from all imports + MergedMCPScripts []string // Merged mcp-scripts configurations from all imports + MergedMarkdown string // Only contains imports WITH inputs (for compile-time substitution) + ImportPaths []string // List of import file paths for runtime-import macro generation (replaces MergedMarkdown) + MergedSteps string // Merged steps configuration from all imports (excluding copilot-setup-steps) + CopilotSetupSteps string // Steps from copilot-setup-steps.yml (inserted at start) + MergedPreSteps string // Merged pre-steps configuration from all imports (prepended in order) + MergedPreAgentSteps string // Merged pre-agent-steps configuration from all imports (prepended in order) + MergedRuntimes string // Merged runtimes configuration from all imports + MergedRunInstallScripts bool // true if any imported workflow sets run-install-scripts: true (global or node-level) + MergedServices string // Merged services configuration from all imports + MergedNetwork string // Merged network configuration from all imports + MergedPermissions string // Merged permissions configuration from all imports + MergedSecretMasking string // Merged secret-masking steps from all imports + MergedBots []string // Merged bots list from all imports (union of bot names) + MergedSkipRoles []string // Merged skip-roles list from all imports (union of role names) + MergedSkipBots []string // Merged skip-bots list from all imports (union of usernames) + MergedActivationGitHubToken string // GitHub token from on.github-token in first imported workflow that defines it + MergedActivationGitHubApp string // JSON-encoded on.github-app from first imported workflow that defines it + MergedTopLevelGitHubApp string // JSON-encoded top-level github-app from first imported workflow that defines it + MergedCheckout string // JSON-encoded checkout configurations from imported workflows (one JSON value per line) + MergedPostSteps string // Merged post-steps configuration from all imports (appended in order) + MergedLabels []string // Merged labels from all imports (union of label names) + MergedCaches []string // Merged cache configurations from all imports (appended in order) + MergedJobs string // Merged jobs from imported YAML workflows (JSON format) + MergedEnv string // Merged env configuration from all imports (JSON format) + MergedEnvSources map[string]string // env var name → source import path (for conflict detection and lock file header listing) + MergedFeatures []map[string]any // Merged features configuration from all imports (parsed YAML structures) + MergedModels []map[string][]string // Merged model alias definitions from all imports (first import to define a key wins among imports) + MergedObservability string // Merged observability config (JSON) from all imports as an endpoint array (deduped by URL) + MergedEngineMCPToolTimeout string // First engine.mcp.tool-timeout found across all imports (Go duration string, e.g. "10m") + MergedEngineMCPSessionTimeout string // First engine.mcp.session-timeout found across all imports (Go duration string, e.g. "4h") + ImportedFiles []string // List of imported file paths (for manifest) + AgentFile string // Path to custom agent file (if imported) + AgentImportSpec string // Original import specification for agent file (e.g., "owner/repo/path@ref") + RepositoryImports []string // List of repository imports (format: "owner/repo@ref") for .github folder merging // ImportInputs uses map[string]any because input values can be different types (string, number, boolean). // This is parsed from YAML frontmatter where the structure is dynamic and not known at compile time. // This is an appropriate use of 'any' for dynamic YAML/JSON data. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 635d6357cfa..b274e06ec34 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -10374,6 +10374,31 @@ }, "required": ["id", "display-name"], "additionalProperties": false + }, + { + "type": "object", + "description": "MCP gateway configuration for shared workflows. Declares engine.mcp settings (tool-timeout, session-timeout) that consumers inherit during import without specifying an engine identifier. The engine is always inherited from the importing workflow.", + "properties": { + "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\", \"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).", + "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 + } + }, + "required": ["mcp"], + "additionalProperties": false } ] }, diff --git a/pkg/workflow/compiler_orchestrator_engine.go b/pkg/workflow/compiler_orchestrator_engine.go index 6d47c6ebfb8..39c81734ea7 100644 --- a/pkg/workflow/compiler_orchestrator_engine.go +++ b/pkg/workflow/compiler_orchestrator_engine.go @@ -264,6 +264,23 @@ func (c *Compiler) setupEngineAndImports(result *parser.FrontmatterResult, clean } } + // Merge engine.mcp.* settings from imports (consumer-specified values take precedence). + // Shared workflows can declare engine.mcp.tool-timeout / engine.mcp.session-timeout to + // propagate MCP gateway timeout configuration to consumers without requiring consumers + // to also set these values explicitly. If the main workflow already set a value, it + // wins (consumer override). + if engineConfig == nil { + engineConfig = &EngineConfig{ID: engineSetting} + } + if engineConfig.MCPToolTimeout == "" && importsResult.MergedEngineMCPToolTimeout != "" { + engineConfig.MCPToolTimeout = importsResult.MergedEngineMCPToolTimeout + orchestratorEngineLog.Printf("Applied engine.mcp.tool-timeout from import: %s", engineConfig.MCPToolTimeout) + } + if engineConfig.MCPSessionTimeout == "" && importsResult.MergedEngineMCPSessionTimeout != "" { + engineConfig.MCPSessionTimeout = importsResult.MergedEngineMCPSessionTimeout + orchestratorEngineLog.Printf("Applied engine.mcp.session-timeout from import: %s", engineConfig.MCPSessionTimeout) + } + // Validate the engine setting and resolve the runtime adapter via the catalog. // This performs exact catalog lookup, prefix fallback, and returns a formatted // validation error for unknown engines — replacing the separate validateEngine diff --git a/pkg/workflow/engine_catalog_test.go b/pkg/workflow/engine_catalog_test.go index 40ad41431d8..b00848aafdc 100644 --- a/pkg/workflow/engine_catalog_test.go +++ b/pkg/workflow/engine_catalog_test.go @@ -97,7 +97,7 @@ func TestEngineCatalog_BuiltInsPresent(t *testing.T) { func TestEngineCatalogMatchesSchema(t *testing.T) { variants := engineSchemaOneOfVariants(t) - require.Len(t, variants, 4, "engine_config oneOf should have exactly 4 variants: string, object-with-id, object-with-runtime, engine-definition") + require.Len(t, variants, 5, "engine_config oneOf should have exactly 5 variants: string, object-with-id, object-with-runtime, engine-definition, mcp-only") // Variant 0: plain string (no enum — allows built-ins and custom named catalog entries) assert.Equal(t, "string", variants[0]["type"], @@ -135,4 +135,14 @@ func TestEngineCatalogMatchesSchema(t *testing.T) { assert.Contains(t, props3, "id", "engine definition variant should have an 'id' property") assert.Contains(t, props3, "display-name", "engine definition variant should have a 'display-name' property") assert.Contains(t, props3, "auth", "engine definition variant should have an 'auth' property") + + // Variant 4: mcp-only form for shared workflow imports (no engine id required) + assert.Equal(t, "object", variants[4]["type"], + "fifth variant should be type object (mcp-only for shared workflows)") + props4, ok := variants[4]["properties"].(map[string]any) + require.True(t, ok, "fifth variant should have properties") + assert.Contains(t, props4, "mcp", + "mcp-only variant should have an 'mcp' property") + assert.NotContains(t, props4, "id", + "mcp-only variant must NOT have an 'id' property") } diff --git a/pkg/workflow/importable_tools_test.go b/pkg/workflow/importable_tools_test.go index 8c7217a0f7e..b2a65e03882 100644 --- a/pkg/workflow/importable_tools_test.go +++ b/pkg/workflow/importable_tools_test.go @@ -1061,3 +1061,351 @@ imports: t.Error("Expected GitHub MCP prompt guidance (with safe outputs variant) when top-level mode is local") } } + +// TestImportEngineMCPToolTimeout tests that engine.mcp.tool-timeout can be imported +// from a shared workflow and is applied to the compiled workflow. +func TestImportEngineMCPToolTimeout(t *testing.T) { + tempDir := testutil.TempDir(t, "test-*") + + // Create a shared workflow with engine.mcp.tool-timeout (no engine id) + sharedPath := filepath.Join(tempDir, "shared-mcp-timeout.md") + sharedContent := `--- +description: "Shared MCP timeout configuration" +mcp-servers: + test-server: + url: http://127.0.0.1:8000/mcp + allowed: + - query +engine: + mcp: + tool-timeout: "2m" +tools: + timeout: 120 + startup-timeout: 120 +--- + +# Shared MCP Timeout Configuration +` + if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { + t.Fatalf("Failed to write shared file: %v", err) + } + + workflowPath := filepath.Join(tempDir, "main-workflow.md") + workflowContent := `--- +on: issues +engine: copilot +imports: + - shared-mcp-timeout.md +permissions: + contents: read +--- + +# Main Workflow + +Uses imported MCP timeout setting. +` + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := workflow.NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("CompileWorkflow failed: %v", err) + } + + lockFilePath := stringutil.MarkdownToLockFile(workflowPath) + lockFileContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + workflowData := string(lockFileContent) + + // The imported engine.mcp.tool-timeout: "2m" should appear as "toolTimeout": "2m" + // in the MCP gateway JSON config embedded in the lock file. + if !strings.Contains(workflowData, `"toolTimeout": "2m"`) { + t.Errorf("Expected lock file to contain toolTimeout 2m from imported shared workflow, got:\n%s", workflowData) + } +} + +// TestImportEngineMCPToolTimeoutConsumerOverride tests that a consumer-specified +// engine.mcp.tool-timeout takes precedence over the imported value. +func TestImportEngineMCPToolTimeoutConsumerOverride(t *testing.T) { + tempDir := testutil.TempDir(t, "test-*") + + // Shared workflow declares a 2m tool-timeout + sharedPath := filepath.Join(tempDir, "shared-mcp-timeout.md") + sharedContent := `--- +description: "Shared MCP timeout configuration" +engine: + mcp: + tool-timeout: "2m" +--- + +# Shared MCP Timeout Configuration +` + if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { + t.Fatalf("Failed to write shared file: %v", err) + } + + // Consumer explicitly overrides tool-timeout to 30s + workflowPath := filepath.Join(tempDir, "main-workflow.md") + workflowContent := `--- +on: issues +engine: + id: copilot + mcp: + tool-timeout: "30s" +imports: + - shared-mcp-timeout.md +permissions: + contents: read +--- + +# Main Workflow with Consumer Override + +Consumer overrides the imported MCP timeout. +` + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := workflow.NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("CompileWorkflow failed: %v", err) + } + + lockFilePath := stringutil.MarkdownToLockFile(workflowPath) + lockFileContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + workflowData := string(lockFileContent) + + // Consumer's 30s should win, not the imported 2m + if !strings.Contains(workflowData, `"toolTimeout": "30s"`) { + t.Errorf("Expected consumer override toolTimeout 30s to win, got:\n%s", workflowData) + } + if strings.Contains(workflowData, `"toolTimeout": "2m"`) { + t.Errorf("Expected imported toolTimeout 2m to be overridden by consumer 30s, got:\n%s", workflowData) + } +} + +// TestImportEngineMCPToolTimeoutValidation tests that validation (10s–600s bounds) +// applies to engine.mcp.tool-timeout values imported from shared workflows. +func TestImportEngineMCPToolTimeoutValidation(t *testing.T) { + tempDir := testutil.TempDir(t, "test-*") + + // Shared workflow declares an invalid tool-timeout (below the 10s minimum) + sharedPath := filepath.Join(tempDir, "shared-invalid-timeout.md") + sharedContent := `--- +description: "Shared workflow with invalid timeout" +engine: + mcp: + tool-timeout: "5s" +--- + +# Shared Workflow with Invalid Timeout +` + if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { + t.Fatalf("Failed to write shared file: %v", err) + } + + workflowPath := filepath.Join(tempDir, "main-workflow.md") + workflowContent := `--- +on: issues +engine: copilot +imports: + - shared-invalid-timeout.md +permissions: + contents: read +--- + +# Main Workflow + +Should fail because imported tool-timeout is invalid. +` + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := workflow.NewCompiler() + err := compiler.CompileWorkflow(workflowPath) + if err == nil { + t.Fatal("Expected CompileWorkflow to fail due to invalid imported tool-timeout, but it succeeded") + } + if !strings.Contains(err.Error(), "tool-timeout") { + t.Errorf("Expected error to mention tool-timeout, got: %v", err) + } +} + +// TestImportEngineMCPSessionTimeout tests that engine.mcp.session-timeout can be imported +// from a shared workflow and is applied to the compiled workflow. +func TestImportEngineMCPSessionTimeout(t *testing.T) { + tempDir := testutil.TempDir(t, "test-*") + + // Create a shared workflow with engine.mcp.session-timeout (no engine id) + sharedPath := filepath.Join(tempDir, "shared-session-timeout.md") + sharedContent := `--- +description: "Shared MCP session timeout configuration" +engine: + mcp: + session-timeout: "2h" +--- + +# Shared MCP Session Timeout Configuration +` + if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { + t.Fatalf("Failed to write shared file: %v", err) + } + + workflowPath := filepath.Join(tempDir, "main-workflow.md") + workflowContent := `--- +on: issues +engine: copilot +imports: + - shared-session-timeout.md +permissions: + contents: read +--- + +# Main Workflow + +Uses imported MCP session timeout setting. +` + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := workflow.NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("CompileWorkflow failed: %v", err) + } + + lockFilePath := stringutil.MarkdownToLockFile(workflowPath) + lockFileContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + workflowData := string(lockFileContent) + + // The imported engine.mcp.session-timeout: "2h" should appear as "sessionTimeout": "2h" + // in the MCP gateway JSON config embedded in the lock file. + if !strings.Contains(workflowData, `"sessionTimeout": "2h"`) { + t.Errorf("Expected lock file to contain sessionTimeout 2h from imported shared workflow, got:\n%s", workflowData) + } +} + +// TestImportEngineMCPSessionTimeoutConsumerOverride tests that a consumer-specified +// engine.mcp.session-timeout takes precedence over the imported value. +func TestImportEngineMCPSessionTimeoutConsumerOverride(t *testing.T) { + tempDir := testutil.TempDir(t, "test-*") + + // Shared workflow declares a 2h session-timeout + sharedPath := filepath.Join(tempDir, "shared-session-timeout.md") + sharedContent := `--- +description: "Shared MCP session timeout configuration" +engine: + mcp: + session-timeout: "2h" +--- + +# Shared MCP Session Timeout Configuration +` + if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { + t.Fatalf("Failed to write shared file: %v", err) + } + + // Consumer explicitly overrides session-timeout to 30m + workflowPath := filepath.Join(tempDir, "main-workflow.md") + workflowContent := `--- +on: issues +engine: + id: copilot + mcp: + session-timeout: "30m" +imports: + - shared-session-timeout.md +permissions: + contents: read +--- + +# Main Workflow with Consumer Override + +Consumer overrides the imported MCP session timeout. +` + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := workflow.NewCompiler() + if err := compiler.CompileWorkflow(workflowPath); err != nil { + t.Fatalf("CompileWorkflow failed: %v", err) + } + + lockFilePath := stringutil.MarkdownToLockFile(workflowPath) + lockFileContent, err := os.ReadFile(lockFilePath) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + workflowData := string(lockFileContent) + + // Consumer's 30m should win, not the imported 2h + if !strings.Contains(workflowData, `"sessionTimeout": "30m"`) { + t.Errorf("Expected consumer override sessionTimeout 30m to win, got:\n%s", workflowData) + } + if strings.Contains(workflowData, `"sessionTimeout": "2h"`) { + t.Errorf("Expected imported sessionTimeout 2h to be overridden by consumer 30m, got:\n%s", workflowData) + } +} + +// TestImportEngineMCPSessionTimeoutValidation tests that validation (minimum 5m) +// applies to engine.mcp.session-timeout values imported from shared workflows. +func TestImportEngineMCPSessionTimeoutValidation(t *testing.T) { + tempDir := testutil.TempDir(t, "test-*") + + // Shared workflow declares an invalid session-timeout (below the 5m minimum) + sharedPath := filepath.Join(tempDir, "shared-invalid-session-timeout.md") + sharedContent := `--- +description: "Shared workflow with invalid session timeout" +engine: + mcp: + session-timeout: "1m" +--- + +# Shared Workflow with Invalid Session Timeout +` + if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { + t.Fatalf("Failed to write shared file: %v", err) + } + + workflowPath := filepath.Join(tempDir, "main-workflow.md") + workflowContent := `--- +on: issues +engine: copilot +imports: + - shared-invalid-session-timeout.md +permissions: + contents: read +--- + +# Main Workflow + +Should fail because imported session-timeout is invalid. +` + if err := os.WriteFile(workflowPath, []byte(workflowContent), 0644); err != nil { + t.Fatalf("Failed to write workflow file: %v", err) + } + + compiler := workflow.NewCompiler() + err := compiler.CompileWorkflow(workflowPath) + if err == nil { + t.Fatal("Expected CompileWorkflow to fail due to invalid imported session-timeout, but it succeeded") + } + if !strings.Contains(err.Error(), "session-timeout") { + t.Errorf("Expected error to mention session-timeout, got: %v", err) + } +}