feat: mount MCP servers as local CLIs after gateway starts#25928
feat: mount MCP servers as local CLIs after gateway starts#25928
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f6e50b8d-7b6d-4cef-a64b-c7b0f9f8878d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Adds automatic mounting of MCP servers as local CLI tools in agentic workflows. This behavior is opt-in via a new `tools.mount-as-clis: true` flag (default `false`). When enabled, all user-facing MCP servers are exposed as standalone shell commands and excluded from the agent's final MCP config so the agent uses the CLI wrappers exclusively. Reconstructs the feature from PR #24503. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f6e50b8d-7b6d-4cef-a64b-c7b0f9f8878d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot ignore the github MCP server, it is handled differently. |
The github MCP server is handled differently and should not be mounted as a CLI tool. Added it to the exclusion lists in both Go and JavaScript code, updated prompt template examples, and restored smoke test instructions to use github as a normal MCP tool. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/35a490ef-753b-4f4e-8ab3-0d10a2d44b21 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Excluded the 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 enable flag for smoke-copilot |
|
@copilot always enabled for safeoutputs, mcpscripts. Update the built prompt generator to reflect this change too. |
The |
Removed safeoutputs, mcp-scripts, and mcpscripts from the internalMCPServerNames exclusion list in both Go and JavaScript. Added explicit logic to always include safeoutputs and mcpscripts in getMCPCLIServerNames() when they are enabled. Updated the prompt template with safeoutputs/mcpscripts examples and updated smoke-copilot test instructions accordingly. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/20bf245a-f76f-48bb-9b17-8597b84ee941 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 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:
|
|
✅ All tools validated successfully! Agent Container Smoke Test confirms agent container is ready. |
|
|
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
|
This comment has been minimized.
This comment has been minimized.
|
Smoke Test Results — Run §24424530245
Overall:
|
There was a problem hiding this comment.
🤖 Smoke Test Review — PR #25928 adds opt-in MCP CLI mounting via mount-as-clis: true. The feature is well-structured with clear separation between gateway config and agent config. Left 2 minor inline suggestions on mcp_cli_mount.go. Overall the implementation looks solid with good security hardening (shell escaping, server name validation, URL validation).
📰 BREAKING: Report filed by Smoke Copilot · ● 898.5K
| if data.ParsedTools != nil && data.ParsedTools.MountAsCLIs { | ||
| // Collect user-facing standard MCP tools from the raw Tools map | ||
| for toolName, toolValue := range data.Tools { | ||
| if toolValue == false { |
There was a problem hiding this comment.
🤖 Smoke Test Review — The toolValue == false check here works but is a bit surprising — false is a valid Go zero-value for bool, but toolValue is any. It might be clearer to use toolValue == nil || toolValue == false to also guard against nil entries, or to add a comment explaining that false means "tool disabled".
| // (regardless of mount-as-clis setting), as they provide safe-output and script tools | ||
| // that the agent should invoke via CLI wrappers. | ||
| var internalMCPServerNames = map[string]bool{ | ||
| "github": true, // GitHub MCP server is handled differently and should not be CLI-mounted |
There was a problem hiding this comment.
🤖 Smoke Test Review — The internalMCPServerNames map correctly excludes the github server. The comment on line 29 nicely explains the rationale. One suggestion: consider adding an integration test that asserts github is never returned from getMCPCLIServerNames so this invariant is enforced by tests.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal! Run 24424530300 verified the Claude engine end-to-end. Core tests (GitHub MCP, Serena, Make build, Playwright, Tavily, file I/O) and PR review safe outputs all passing.
💥 [THE END] — Illustrated by Smoke Claude · ● 210.7K
| // buildMCPCLIPromptSection returns a PromptSection describing the CLI tools available | ||
| // to the agent, or nil if there are no servers to mount. | ||
| // The prompt is loaded from actions/setup/md/mcp_cli_tools_prompt.md at runtime, | ||
| // with the __GH_AW_MCP_CLI_SERVERS_LIST__ placeholder substituted by the substitution step. |
There was a problem hiding this comment.
💥 Smoke Test Review — The --help hint in the prompt section is a great UX improvement. Consider also adding a brief example invocation in the generated prompt so agents understand the CLI wrapper syntax at a glance (e.g., playwright browser_navigate --url (example.com/redacted)).
| @@ -0,0 +1,5 @@ | |||
| --- | |||
There was a problem hiding this comment.
✨ Smoke Test Review — Good changeset entry! Clear and concise summary of the feature. The note about github MCP server exclusion is helpful for reviewers understanding the scope of the change.
|
@copilot review gateway logs, the cli calls are failing https://github.com/github/gh-aw/actions/runs/24424530245/job/71355310322#step:45:1 |
…eature-24503 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Both mcp_cli_bridge.cjs and mount_mcp_as_cli.cjs were missing the required Accept: application/json, text/event-stream header, causing HTTP 400 errors from the MCP gateway with 'Accept must contain both application/json and text/event-stream'. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Merged main (no conflicts) and recompiled all 191 lock files in
Fixed in 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:
|
|
✅ All tools validated successfully! Agent Container Smoke Test confirms agent container is ready. |
|
|
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
|
Smoke Test Results — Run §24427887182 Core tests #1-12: ✅✅✅✅✅✅✅✅✅✅✅✅ Overall: PARTIAL (2 skipped: #16 thread resolve - no thread node ID accessible, #19 close PR - no safe test PR)
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal! This PR introduces MCP CLI mounting and RUNNER_TEMP fixes — both well-implemented. Tests #14-15 complete.
💥 [THE END] — Illustrated by Smoke Claude · ● 236.9K
| "gh-aw": patch | ||
| --- | ||
|
|
||
| Added opt-in MCP CLI mounting via `tools.mount-as-clis: true`, which exposes eligible MCP servers as local CLI wrappers on `PATH` and updates prompt/config wiring so agents use those wrappers. The `github` MCP server remains a normal MCP tool, while `safeoutputs` and `mcpscripts` are included in CLI mounting when enabled. |
There was a problem hiding this comment.
✅ Good changeset description! The opt-in nature via tools.mount-as-clis: true is clearly documented. Consider also mentioning any performance impact of mounting MCP servers as CLIs vs. direct MCP tool calls.
| run: | | ||
| set -eo pipefail | ||
| mkdir -p /tmp/gh-aw/mcp-config | ||
| mkdir -p "${RUNNER_TEMP}/gh-aw/mcp-config" |
There was a problem hiding this comment.
✅ Good fix — switching from hardcoded /tmp/gh-aw/mcp-config to \$\{RUNNER_TEMP}/gh-aw/mcp-config improves portability across different runner environments. This is consistent with GitHub Actions best practices for using RUNNER_TEMP.
|
🤖 Smoke Test Results — Run §24427887202
Overall: PR author:
|
There was a problem hiding this comment.
🤖 Smoke Test Review (Run §24427887202)
This PR introduces a well-architected MCP CLI mounting system. The separation between internalMCPServerNames, alwaysCLIMountedServers, and user-opted servers is clean. Security hardening (shell injection prevention, server name validation) is thorough. The dual-access pattern for safeoutputs/mcpscripts is pragmatic. Build verified ✅
📰 BREAKING: Report filed by Smoke Copilot · ● 1.8M
| --- | ||
| "gh-aw": patch | ||
| --- | ||
|
|
There was a problem hiding this comment.
🤖 Smoke Test Review — Changeset entry clearly describes the feature. Consider mentioning the security hardening aspects (shell injection prevention, server name validation) in the changeset description as they're significant improvements worth surfacing in the changelog.
| // Note: safeoutputs and mcpscripts are NOT excluded — they are always CLI-mounted | ||
| // (regardless of mount-as-clis setting), as they provide safe-output and script tools | ||
| // that the agent should invoke via CLI wrappers. | ||
| var internalMCPServerNames = map[string]bool{ |
There was a problem hiding this comment.
🤖 Smoke Test Review — The internalMCPServerNames map elegantly centralizes exclusion logic. One suggestion: consider adding a doc comment explaining what makes a server "internal" vs eligible for CLI mounting, which would help future contributors understand the distinction between this map and alwaysCLIMountedServers.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
@copilot merge main, recompile, deep review |
…eature-24503 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Reconstructs the feature from #24503. Adds opt-in mounting of MCP servers as standalone CLI tools on
PATHviatools.mount-as-clis: true(defaultfalse). When enabled, user-facing MCP servers are exposed as shell commands and excluded from the agent's final MCP config, forcing the agent to use CLI wrappers exclusively. ThegithubMCP server is excluded from CLI mounting as it is handled differently and remains available as a normal MCP tool.The
safeoutputsandmcpscriptsservers are always CLI-mounted when enabled — regardless of themount-as-clissetting — and remain available as MCP tools too (dual access). The injected prompt instructs the agent to prefer the CLI wrappers over the MCP tools for these servers.How it works
The gateway starts containers for all configured servers (so they're reachable), then filtering happens in two stages:
${RUNNER_TEMP}/gh-aw/mcp-cli/manifest.jsonGH_AW_MCP_CLI_SERVERSenv var in eachconvert_gateway_config_*.cjsscript — exceptsafeoutputsandmcpscripts, which stay in the agent MCP config for dual accessAfter the gateway starts,
mount_mcp_as_cli.cjsreads the manifest, queries each server'stools/listvia the full MCP session protocol (initialize→notifications/initialized→tools/list), and generates locked-down CLI wrappers under${RUNNER_TEMP}/gh-aw/mcp-cli/bin/.Note: The
githubMCP server is not CLI-mounted — it is handled differently and remains available as a normal MCP tool even whenmount-as-clis: trueis set.Changes
New files
pkg/workflow/mcp_cli_mount.go—getMCPCLIServerNames(),getMCPCLIExcludeFromAgentConfig(),generateMCPCLIMountStep(),buildMCPCLIPromptSection(),GetMCPCLIPathSetup()actions/setup/js/mount_mcp_as_cli.cjs— manifest reader, MCP handshake, CLI wrapper generator; generates thin bash stubs thatexecinto the Node.js bridgeactions/setup/js/mcp_cli_bridge.cjs— Node.js CLI-to-MCP bridge handling full MCP session protocol (initialize→notifications/initialized→tools/call), argument parsing,--helpdisplay, response formatting, and JSONL audit logging; usesshim.cjsandcore.*for consistent GitHub Actions loggingactions/setup/md/mcp_cli_tools_prompt.md—<mcp-clis>prompt section templateactions/setup/js/convert_gateway_config_copilot.cjs— JavaScript gateway config converter for Copilot (filters CLI servers, addstools:["*"], rewrites URLs)actions/setup/js/convert_gateway_config_claude.cjs— JavaScript gateway config converter for Claude (filters CLI servers, setstype:"http", rewrites URLs)actions/setup/js/convert_gateway_config_codex.cjs— JavaScript gateway config converter for Codex (filters CLI servers, resolveshost.docker.internalto172.30.0.1, generates TOML)actions/setup/js/convert_gateway_config_gemini.cjs— JavaScript gateway config converter for Gemini (filters CLI servers, removestypefield for transport auto-detection, rewrites URLs, addscontext.includeDirectories)actions/setup/js/start_mcp_gateway.cjs— JavaScript gateway startup script (reads MCP config from stdin, launches Docker container, health check polling, engine-specific converter dispatch, CLI manifest creation, GitHub Actions outputs)CLI wrapper architecture (bash → Node.js bridge)
exec node mcp_cli_bridge.cjswith server config passed as arguments — no jq/curl/awk dependenciesmcp_cli_bridge.cjshandles all MCP protocol logic in Node.js:initialize→notifications/initialized→tools/callwith proper session ID tracking--key valueand--key=valuestyles, with--jsonflag for raw JSON output--help/-hsupport: displays available tools with parameter schemasshim.cjsandcore.*for all logging (consistent GitHub Actions annotation output)/tmp/gh-aw/mcp-cli-audit/<server>.jsonlwith timestamps, duration, request/response details, and error tracking — audit directory/file failures logged as warnings without blocking tool executionGo pipeline integration
ToolsConfig.MountAsCLIsfield + parser + schema + validationcompiler_yaml_main_job.gocallsgenerateMCPCLIMountStep()after gateway setupmcp_setup_generator.goexportsGH_AW_MCP_CLI_SERVERSJSON array to$GITHUB_ENVusinggetMCPCLIExcludeFromAgentConfig()(excludessafeoutputs/mcpscriptsso they remain as MCP tools)unified_prompt_step.goinjects CLI tools prompt section (8a, after safe outputs)mcp_rendering.godocuments why CLI servers stay in gateway configmcp_renderer.goinvokesnode start_mcp_gateway.cjs(converted frombash start_mcp_gateway.sh)getMCPCLIServerNames()always includessafeoutputsandmcpscriptswhen they are enabled (HasSafeOutputsEnabled/IsMCPScriptsEnabled), regardless ofmount-as-clissettinggetMCPCLIExcludeFromAgentConfig()returns only servers that should be removed from agent MCP config (does not includesafeoutputs/mcpscripts— they get dual access)MCP CLI bin directory on PATH inside AWF container
GetMCPCLIPathSetup()inmcp_cli_mount.goreturns theexport PATH=snippet that prepends${RUNNER_TEMP}/gh-aw/mcp-cli/binto$PATHcopilot_engine_execution.go,claude_engine.go,codex_engine.go,gemini_engine.go) inject this PATH setup into the AWF container's environment so CLI wrappers are discoverable without manualexportby the agentDual access for safeoutputs and mcpscripts
safeoutputsandmcpscriptsare always CLI-mounted when enabled, even withoutmount-as-clis: truealwaysCLIMountedServersmap in Go controls which servers get this treatmentGateway scripts (bash → JavaScript)
start_mcp_gateway.cjs— full JavaScript port of the gateway startup script usingchild_process.spawnfor Docker container management, Node.jshttpmodule for health checks, andchild_process.execSyncfor converter/checker dispatch. Maintains all security controls (umask, symlink attack prevention, 0o600 file permissions)convert_gateway_config_{copilot,claude,codex,gemini}.cjs— all four engine converters are JavaScript for CLI server filtering, URL rewriting, and engine-specific config generationvalidate_prompt_placeholders.sh— strips backtick code spans before checking for unreplaced__GH_AW_*__placeholdersConfig storage hardening (
/tmp→RUNNER_TEMP)mcp-config/) and CLI manifest (mcp-cli/) moved from/tmp/gh-aw/to${RUNNER_TEMP}/gh-aw/to prevent tampering —/tmpis world-writable,RUNNER_TEMPis per-runner and not world-accessiblestart_mcp_gateway.cjs, converter scripts,mount_mcp_as_cli.cjs,redact_secrets.cjs) and Go engine code (claude_engine.go,codex_engine.go,codex_mcp.go,claude_mcp.go,gemini_mcp.go,mcp_setup_generator.go,threat_detection.go,step_order_validation.go)/tmp/gh-aw/mcp-logs/since the Docker gateway container writes there via-v /tmp:/tmp:rwSecurity hardening (shell injection, input validation)
shellEscapeDoubleQuoted()inmount_mcp_as_cli.cjs— escapes$ `` \ " !and strips\r\nfor all values embedded in generated CLI wrapper bash scripts (server name, URL, tools file path, API key), preventing shell injection and line injection attacksisValidServerName()inmount_mcp_as_cli.cjs— validates server names against[a-zA-Z0-9_-]with 64-char max length, preventing path traversal and shell metacharacter injection via malicious server names; servers failing validation are skipped with a warningnew URL()before processinggatewayPortverified to be strictly numeric before interpolation intoexecSyncshell commands instart_mcp_gateway.cjscheck_mcp_servers.shcall uses$MCP_GATEWAY_API_KEYenv var reference instead of direct JS string interpolation to avoid shell metacharacter risksstart_mcp_gateway.cjsfilters invalid server names when generating the CLI manifestObservability improvements
core.*logging viashim.cjs(core.info,core.error,core.warning) instead ofconsole.*, ensuring consistent GitHub Actions annotation outputstart_mcp_gateway.cjslogs startup banner (engine, port, domain, runner temp), server names in CLI manifest; top-level error handler preserves stack traces viacore.error(err.stack)beforecore.setFailed()mount_mcp_as_cli.cjslogs manifest server count after internal filtering, skipped servers with validation error reasonsmcp_cli_bridge.cjslogs every tool invocation with timing, arguments, and response status; appends JSONL audit entries to/tmp/gh-aw/mcp-cli-audit/<server>.jsonlfor post-run analysisServer exclusions
githubMCP server is excluded from CLI mounting in both Go (internalMCPServerNames) and JavaScript (INTERNAL_SERVERS) — it is handled differently and remains a normal MCP toolsafeoutputsandmcpscriptsare always CLI-mounted when enabled (dual access with MCP), with usage examples and preference guidance in the prompt templateSmoke test
smoke-copilot.mdenablesmount-as-clis: trueand updates test instructions to use CLI syntax for playwright/serena/safeoutputs/mcpscripts while keeping github as a normal MCP tool✨ PR Review Safe Output Test - Run 24424530300
✨ PR Review Safe Output Test - Run 24427887182