Conversation
5 tasks
- Modify importAccumulator to extract engine.mcp.tool-timeout and engine.mcp.session-timeout from imported workflows (first-wins strategy) - Skip adding mcp-only engine configs to engines list to avoid 'multiple engine fields' validation error for shared workflows that declare engine.mcp.* without an engine ID - Add MergedEngineMCPToolTimeout/MergedEngineMCPSessionTimeout to ImportsResult - Merge imported values into engineConfig with consumer-wins precedence in compiler_orchestrator_engine.go - Add new oneOf variant to engine_config schema for mcp-only configs - Update TestEngineCatalogMatchesSchema for 5 variants - Add 3 integration tests covering: import without override, import with consumer override, and validation of imported values" Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3274e4c1-0fe4-4b86-9d03-21e01057c064 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3274e4c1-0fe4-4b86-9d03-21e01057c064 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix import of engine.mcp.tool-timeout from shared workflows
feat: support importing engine.mcp.tool-timeout from shared workflows
May 6, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Enables shared workflows to propagate engine.mcp.tool-timeout / engine.mcp.session-timeout to importing workflows, while avoiding false “multiple engine fields” errors when an imported workflow’s engine block is MCP-only (no id/runtime).
Changes:
- Parser: extract and first-win merge imported
engine.mcp.tool-timeout/engine.mcp.session-timeout, and exclude MCP-onlyengineobjects from “engine specification” conflict detection. - Compiler: apply imported MCP timeout values after engine resolution with consumer-wins precedence.
- Schema/tests: add an
engine_configoneOf variant forengine: { mcp: {...} }and expand tests for importingengine.mcp.tool-timeout.
Show a summary per file
| File | Description |
|---|---|
| pkg/parser/import_field_extractor.go | Extracts imported engine.mcp.* timeouts and prevents MCP-only engine objects from being treated as engine specs. |
| pkg/parser/import_processor.go | Extends ImportsResult to carry merged MCP timeout values from imports. |
| pkg/workflow/compiler_orchestrator_engine.go | Applies merged imported MCP timeout values to the resolved engine config (consumer override wins). |
| pkg/parser/schemas/main_workflow_schema.json | Adds schema support for MCP-only engine objects (no required id). |
| pkg/workflow/engine_catalog_test.go | Updates schema-structure assertions for the new engine_config variant. |
| pkg/workflow/importable_tools_test.go | Adds coverage for importing engine.mcp.tool-timeout, consumer override, and validation. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 2
Comment on lines
+236
to
+244
| engineContent, err := extractFieldJSONFromMap(fm, "engine", "") | ||
| if err == nil && engineContent != "" { | ||
| log.Printf("Found engine config in import: %s", item.fullPath) | ||
| acc.engines = append(acc.engines, engineContent) | ||
|
|
||
| // Parse the engine content to determine whether it specifies an engine ID | ||
| // and to extract any engine.mcp.* settings. | ||
| var engineData any | ||
| if jsonErr := json.Unmarshal([]byte(engineContent), &engineData); jsonErr == nil { | ||
| switch v := engineData.(type) { |
Comment on lines
+1065
to
+1069
| // 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-*") | ||
|
|
Collaborator
|
@copilot merge main and recompile |
…t-engine-mcp-tool-timeout # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done — merged main and recompiled all 215 workflow lock files in commit a51ac2e. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Collaborator
|
@copilot Review all comments. |
… tests Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5950e8d4-cd6a-4351-8734-9bfd1005fa8d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Both review comments addressed in commit 6b70a59:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
feat: support importing engine.mcp.tool-timeout from shared workflows
feat: support importing engine.mcp.tool-timeout and session-timeout from shared workflows
May 6, 2026
github-actions Bot
added a commit
that referenced
this pull request
May 7, 2026
Add engine.mcp (tool-timeout, session-timeout) to the Allowed Import Fields list, the Field-Specific Merge Semantics table, and a new "Importing MCP Gateway Settings" section with a worked example. Closes gap introduced by feat: support importing engine.mcp.tool-timeout and session-timeout from shared workflows (#30686). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Shared workflows wrapping slow HTTP MCP servers (e.g. Repo Mind Light) couldn't propagate
engine.mcp.tool-timeoutto consumers — every consumer had to manually duplicate bothtools.timeoutandengine.mcp.tool-timeout, even though the shared workflow author owns the correct value.A secondary blocker: shared workflows declaring
engine: { mcp: { tool-timeout: "10m" } }(no engineid) triggered a "multiple engine fields" compilation error, making the feature entirely unusable before this fix.Changes
Parser — import accumulator (
pkg/parser/import_field_extractor.go)engine.mcp.tool-timeoutandengine.mcp.session-timeoutfrom each imported workflow'sengineblock (first-wins across imports) using a direct type-switch on the parsed frontmatter value — no JSON encode/decode round-tripmcpsub-keys (noid/runtime) are no longer counted as engine specifications, eliminating the false "multiple engine fields" errorParser — result type (
pkg/parser/import_processor.go)MergedEngineMCPToolTimeoutandMergedEngineMCPSessionTimeouttoImportsResultCompiler (
pkg/workflow/compiler_orchestrator_engine.go)Schema (
pkg/parser/schemas/main_workflow_schema.json)oneOfvariant forengine_configthat allowsengine: { mcp: {...} }without a requiredid, removing the false "Unknown property: mcp" schema warningExample
Shared workflow (
shared/repo-mind-light.md):Consumer (no extra config needed):
Consumer override (explicit value wins):
Validation is applied to imported values just as it is for directly declared values:
tool-timeout: 10s–600s boundssession-timeout: minimum 5m, no upper bound