Migrate MCP gateway compiler output to agentId - #56990
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Configured agent IDs permit shell injection, while legacy references leave a test and user-facing artifacts inconsistent.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Migrates MCP gateway configuration and runtime wiring from legacy API-key terminology to agentId, including generated workflows and documentation.
Changes:
- Renames gateway configuration, environment variables, outputs, and runtime handoffs.
- Updates gateway scripts, schema, tests, and generated fixtures.
- Preserves unrelated MCP Scripts API-key behavior.
File summaries
| File | Description |
|---|---|
pkg/workflow/tools_types.go |
Renames runtime configuration to AgentID. |
pkg/workflow/testdata/wasm_golden/WasmBinary/with-imports.golden |
Refreshes WASM snapshot. |
pkg/workflow/testdata/wasm_golden/WasmBinary/smoke-copilot.golden |
Refreshes Copilot WASM snapshot. |
pkg/workflow/testdata/wasm_golden/WasmBinary/playwright-cli-mode.golden |
Refreshes Playwright WASM snapshot. |
pkg/workflow/testdata/wasm_golden/WasmBinary/basic-copilot.golden |
Refreshes basic WASM snapshot. |
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden |
Updates import fixture output. |
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-test-tools.golden |
Updates tools fixture output. |
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden |
Updates Copilot fixture output. |
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden |
Updates Playwright fixture output. |
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/claude-with-network.golden |
Updates Claude fixture output. |
pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden |
Updates basic fixture output. |
pkg/workflow/testdata/TestWasmGolden_AllEngines/pi.golden |
Updates Pi engine snapshot. |
pkg/workflow/testdata/TestWasmGolden_AllEngines/gemini.golden |
Updates Gemini engine snapshot. |
pkg/workflow/testdata/TestWasmGolden_AllEngines/copilot.golden |
Updates Copilot engine snapshot. |
pkg/workflow/testdata/TestWasmGolden_AllEngines/codex.golden |
Updates Codex engine snapshot. |
pkg/workflow/test-yaml-import.lock.yml |
Regenerates import workflow output. |
pkg/workflow/template_injection_validation_test.go |
Updates gateway output references. |
pkg/workflow/strict_mode_sandbox_validation_test.go |
Tests AgentID in strict mode. |
pkg/workflow/sandbox_agent_disabled_test.go |
Updates disabled-sandbox assertions. |
pkg/workflow/mcp_setup_gateway.go |
Generates and forwards gateway agent IDs. |
pkg/workflow/mcp_renderer.go |
Emits gateway.agentId. |
pkg/workflow/mcp_renderer_test.go |
Adds JSON/schema validation coverage. |
pkg/workflow/mcp_gateway_config.go |
Builds agent-ID gateway configuration. |
pkg/workflow/mcp_gateway_config_test.go |
Updates gateway configuration tests. |
pkg/workflow/mcp_cli_mount.go |
Passes agent ID to CLI mounts. |
pkg/workflow/mcp_api_key_masking_test.go |
Updates masking assertions. |
pkg/workflow/gemini_engine.go |
Updates Gemini secret terminology. |
pkg/workflow/gemini_engine_test.go |
Updates Gemini secret assertions. |
pkg/workflow/frontmatter_extraction_security.go |
Parses sandbox.mcp.agent-id. |
pkg/workflow/frontmatter_extraction_security_test.go |
Tests configured agent-ID extraction. |
pkg/workflow/engine_helpers.go |
Updates common gateway secret name. |
pkg/workflow/engine_helpers_secrets_test.go |
Updates engine secret tests. |
pkg/workflow/enclaves_test.go |
Updates enclave handoff assertions. |
pkg/workflow/enclave_github_proxy_test.go |
Updates enclave proxy assertions. |
pkg/workflow/copilot_engine.go |
Updates Copilot gateway secret handling. |
pkg/workflow/compiler_yaml_ai_execution.go |
Passes agent ID during shutdown. |
pkg/workflow/codex_engine_test.go |
Updates Codex rendering expectations. |
pkg/workflow/awf_env.go |
Excludes agent ID from agent environments. |
pkg/workflow/agentic_engine.go |
Updates security-provider documentation. |
pkg/parser/schemas/main_workflow_schema.json |
Renames the frontmatter schema field. |
pkg/cli/workflows/example-blocked-domains.lock.yml |
Regenerates example workflow output. |
docs/src/content/docs/troubleshooting/common-issues.md |
Updates OpenCode gateway guidance. |
docs/src/content/docs/reference/sandbox.md |
Documents agent-id. |
docs/src/content/docs/introduction/architecture.mdx |
Updates gateway security terminology. |
docs/adr/55531-add-read-only-github-issues-access-to-agent-enclaves.md |
Updates enclave terminology. |
actions/setup/sh/stop_mcp_gateway.sh |
Uses agent ID for graceful shutdown. |
actions/setup/sh/start_mcp_gateway.sh |
Validates and outputs agent IDs. |
actions/setup/sh/check_mcp_servers.sh |
Renames gateway-check argument. |
actions/setup/sh/check_mcp_servers_test.sh |
Updates gateway test configurations. |
actions/setup/js/start_mcp_gateway.test.cjs |
Tests agent-ID redaction. |
actions/setup/js/start_mcp_gateway.cjs |
Migrates gateway startup runtime wiring. |
actions/setup/js/mount_mcp_as_cli.test.cjs |
Updates CLI mount credential tests. |
actions/setup/js/mount_mcp_as_cli.cjs |
Uses agent ID for CLI wrappers. |
.github/workflows/smoke-opencode.lock.yml |
Regenerates OpenCode smoke workflow. |
.github/workflows/smoke-cursor.lock.yml |
Regenerates Cursor smoke workflow. |
.github/workflows/smoke-crush.lock.yml |
Regenerates Crush smoke workflow. |
.github/workflows/smoke-ci.lock.yml |
Regenerates CI smoke workflow. |
.github/workflows/daily-token-consumption-report.lock.yml |
Regenerates token report workflow. |
.github/workflows/daily-team-evolution-insights.lock.yml |
Regenerates team insights workflow. |
.github/workflows/daily-security-red-team.lock.yml |
Regenerates security workflow. |
.github/workflows/daily-reliability-review.lock.yml |
Regenerates reliability workflow. |
Review details
- Files reviewed: 88/353 changed files
- Comments generated: 4
- Review effort level: Balanced
| @@ -455,7 +455,7 @@ func TestCodexEngineRenderMCPConfig(t *testing.T) { | |||
| "\"gateway\": {", | |||
| "\"port\": $MCP_GATEWAY_PORT,", | |||
| "\"domain\": \"${MCP_GATEWAY_DOMAIN}\",", | |||
| "\"apiKey\": \"${MCP_GATEWAY_API_KEY}\",", | |||
| "\"apiKey\": \"${MCP_GATEWAY_AGENT_ID}\",", | |||
| } else { | ||
| yaml.WriteString(" export MCP_GATEWAY_API_KEY=\"" + gatewayConfig.APIKey + "\"\n") | ||
| yaml.WriteString(" echo \"::add-mask::${MCP_GATEWAY_API_KEY}\"\n") | ||
| yaml.WriteString(" export MCP_GATEWAY_AGENT_ID=\"" + gatewayConfig.AgentID + "\"\n") |
| "agent-id": { | ||
| "type": "string", | ||
| "description": "API key for authenticating with the MCP gateway (supports ${{ secrets.* }} syntax)" | ||
| "description": "Agent/session identifier for authenticating with the MCP gateway (supports ${{ secrets.* }} syntax)" |
| @@ -25,12 +25,12 @@ print_timing() { | |||
| echo "⏱️ TIMING: $label took ${duration}ms" | |||
| } | |||
|
|
|||
| # Usage: check_mcp_servers.sh GATEWAY_CONFIG_PATH GATEWAY_URL GATEWAY_API_KEY | |||
| # Usage: check_mcp_servers.sh GATEWAY_CONFIG_PATH GATEWAY_URL GATEWAY_AGENT_ID | |||
|
@copilot Please address the latest blocking review feedback on this PR, then run the Priority items from the newest review comments:
Failed checks to fix:
Please also refresh the branch if needed while addressing the review comments.
|
|
No PR diff was available at /tmp/gh-aw/agent/pr-diff.patch, so there was nothing to review for over-engineering. Warning Firewall blocked 4 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"
- "api.github.com"
- "chatgpt.com"
- "github.com"See Network Configuration for more information.
|
|
❌ Design Decision Gate 🏗️ failed to deliver outputs during design decision gate check. Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"
- "proxy.golang.org"See Network Configuration for more information.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch PR file list
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Request changes
This rename lands in the compiler/runtime, but several derived artifacts and one test still pin the old api-key contract.
Blocking themes
pkg/workflow/codex_engine_test.gostill expectsgateway.apiKey, so the renamed renderer contract is not reflected in test coverage.- Generated user-facing artifacts still advertise
sandbox.mcp.api-key, which now conflicts with the schema and will push users toward invalid frontmatter. - The MCP server checker usage text still tells callers to pass
GATEWAY_API_KEY, so even the failure path is documenting the deprecated name.
🔎 Code quality review by PR Code Quality Reviewer · pi · gpt54 · 21.2 AIC · ⌖ 7.47 AIC · ⊞ 7.4K
Comment /review to run again
| "type": "string", | ||
| "description": "API key for authenticating with the MCP gateway (supports ${{ secrets.* }} syntax)" | ||
| "description": "Agent/session identifier for authenticating with the MCP gateway (supports ${{ secrets.* }} syntax)" |
There was a problem hiding this comment.
This rename is incomplete: the generated reference docs and editor autocomplete payload still advertise sandbox.mcp.api-key, so the schema and user-facing artifacts now disagree and will mislead anyone authoring workflows.
💡 Why this blocks the change
docs/src/content/docs/reference/frontmatter-full.md still shows:
api-key: "example-value"and docs/public/editor/autocomplete-data.json still exposes the same api-key field/description. This PR changes the accepted field to agent-id, so leaving those generated artifacts stale breaks discoverability and drives users toward invalid config.
Please regenerate the derived docs/editor assets from the updated schema so they publish agent-id consistently.
| @@ -455,7 +455,7 @@ func TestCodexEngineRenderMCPConfig(t *testing.T) { | |||
| "\"gateway\": {", | |||
| "\"port\": $MCP_GATEWAY_PORT,", | |||
| "\"domain\": \"${MCP_GATEWAY_DOMAIN}\",", | |||
| "\"apiKey\": \"${MCP_GATEWAY_API_KEY}\",", | |||
| "\"apiKey\": \"${MCP_GATEWAY_AGENT_ID}\",", | |||
There was a problem hiding this comment.
This test is still asserting the old "apiKey": "${MCP_GATEWAY_AGENT_ID}" payload even though the renderer now emits agentId, so the suite should fail as soon as this path is exercised.
💡 What to fix
The expectation block in TestCodexEngine...RenderMCPConfig still contains the legacy JSON fragment:
"gateway": {
"apiKey": "${MCP_GATEWAY_AGENT_ID}"
}but the rest of the PR migrates the gateway schema/output to agentId. Update the expected substring to the new key so the test actually validates the renamed contract instead of pinning the obsolete one.
| @@ -25,12 +25,12 @@ print_timing() { | |||
| echo "⏱️ TIMING: $label took ${duration}ms" | |||
| } | |||
|
|
|||
| # Usage: check_mcp_servers.sh GATEWAY_CONFIG_PATH GATEWAY_URL GATEWAY_API_KEY | |||
| # Usage: check_mcp_servers.sh GATEWAY_CONFIG_PATH GATEWAY_URL GATEWAY_AGENT_ID | |||
There was a problem hiding this comment.
The script contract was renamed to GATEWAY_AGENT_ID, but the argument validation error still prints GATEWAY_API_KEY; that is stale guidance in the one path users read when invocation fails.
💡 Why this matters
The comment header and variable assignment below already use GATEWAY_AGENT_ID, so this usage string is now the odd one out:
echo "Usage: $0 GATEWAY_CONFIG_PATH GATEWAY_URL GATEWAY_API_KEY" >&2When a caller passes the wrong arguments, this sends them back to the deprecated name. Please update the usage text to match the renamed parameter.
|
Warning The 🏗️ ADR required — draft added for PR #56990I enforced the design-decision gate for this PR because the prefetch summary shows 307 added lines in business-logic directories ( Evidence reviewed
Decision inferred from the PRThis PR makes an architectural contract change: the MCP gateway compiler/runtime interface should use
Action takenI added a draft ADR to the PR branch:
Next action for the authorReview the draft ADR, adjust any rationale or trade-offs that need team context, and keep it with the PR so the architectural rename is explicitly documented before merge. Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"
- "proxy.golang.org"See Network Configuration for more information.
|
Addressed all blocking review comments in commit
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /developer-security — requesting changes on a shell-injection risk and a broken test assertion.
📋 Key Themes & Highlights
Key Issues (4 inline comments already posted)
-
[Security / shell injection]
mcp_setup_gateway.go:275—gatewayConfig.AgentIDis interpolated directly into shell source without sanitisation. A value likefoo"; malicious-cmd; #executes arbitrary code at workflow runtime. The random-generation path is safe; only the configured-value path is affected. -
[Correctness / broken test]
codex_engine_test.go:458— The test expects the substringapiKeyin the rendered gateway JSON, butRenderJSONMCPConfignow emitsagentId. This assertion will fail and block CI. -
[Correctness / stale usage string]
actions/setup/sh/check_mcp_servers.sh:28— The error-path usage line still saysGATEWAY_API_KEY. Users who mis-invoke the script receive misleading guidance. -
[Maintainability / stale docs]
pkg/parser/schemas/main_workflow_schema.json:4037— Thesandbox.mcp.agent-idrename has not been propagated todocs/src/content/docs/reference/frontmatter-full.mdanddocs/public/editor/autocomplete-data.json. Autocomplete will still suggest the removedapi-keyfield.
Positive Highlights
- ✅ Clean, systematic rename across compiler, runtime, env-var masking, and lock files.
- ✅ Random-generation path for
MCP_GATEWAY_AGENT_IDis correctly masked via::add-mask::. - ✅ MCP Scripts API-key path intentionally left unchanged — good separation of concerns.
- ✅ PR description is thorough and accurately describes preserved vs. changed behaviour.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet46 · 38.8 AIC · ⌖ 11.6 AIC · ⊞ 7.6K
Comment /matt to run again
There was a problem hiding this comment.
Impeccable Review — harden + audit (refactor/rename)
The migration from APIKey/MCP_GATEWAY_API_KEY to AgentID/MCP_GATEWAY_AGENT_ID is coherent and covers all the key paths: env var export, container flag, CLI mount step, enclave handoff, JSON renderer, and gateway schema. The env-var indirection pattern for configured agent IDs (GH_AW_MCP_GATEWAY_CONFIGURED_AGENT_ID) is a security improvement over the old direct string interpolation.
Issues already filed (confirmed real)
| File | Issue |
|---|---|
pkg/workflow/codex_engine_test.go:458 |
Test asserts "apiKey": "${MCP_GATEWAY_AGENT_ID}" — JSON key is still apiKey, will cause test failure |
actions/setup/sh/check_mcp_servers.sh:28 |
Usage error message still prints GATEWAY_API_KEY — stale help text |
pkg/parser/schemas/main_workflow_schema.json:4037 |
Schema rename may leave generated reference docs stale |
Additional observations
mcp_setup_gateway.goinjection concern is resolved — The newelsebranch emits"${GH_AW_MCP_GATEWAY_CONFIGURED_AGENT_ID}"(variable reference), not the literal value inline. This is safer than the old code; the existing Copilot comment marking this as an injection risk is incorrect for the new code.- Golden fixtures and lock files are mechanically consistent — all
apiKey→agentIdandMCP_GATEWAY_API_KEY→MCP_GATEWAY_AGENT_IDsubstitutions in testdata match. - MCP Scripts API key is explicitly preserved and unmodified — correct.
nolint:largefuncadditions are appropriate suppressions for pre-existing large functions.
The blocking issue is codex_engine_test.go:458 which will fail tests.
Warning
Firewall blocked 2 domains
The following domains were blocked by the firewall during workflow execution:
api.github.comgithub.com
[!TIP]
api.github.com is blocked because GitHub API access uses the built-in GitHub tools by default. Instead of adding api.github.com to network.allowed, use tools.github.mode: gh-proxy for direct pre-authenticated GitHub CLI access without requiring network access to api.github.com:
tools:
github:
mode: gh-proxySee GitHub Tools for more information on gh-proxy mode.
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "api.github.com"
- "github.com"See Network Configuration for more information.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet46 · 99.5 AIC · ⌖ 10.8 AIC · ⊞ 6.2K
|
@copilot Please refresh the branch if needed, confirm all review threads are actually resolved, and then run the Please double-check the previously called out items remain fixed together:
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
…ateway-compiler-output # Conflicts: # .github/workflows/pr-code-quality-reviewer.lock.yml Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Refreshed branch with latest
|
|
/smoke-copilot |
|
📰 BREAKING: Smoke Copilot is now investigating this issue comment. Sources say the story is developing... |
|
PR: Migrate MCP gateway compiler output to agentId (#56990) Smoke test results:
Overall status: FAIL cc Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "www.google.com"
- "www.gstatic.com"See Network Configuration for more information.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "www.google.com"
- "www.gstatic.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
Caveman review done. Test comment small.
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comwww.google.comwww.gstatic.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "www.google.com"
- "www.gstatic.com"See Network Configuration for more information.
📰 BREAKING: Report filed by Smoke Copilot · copilot · auto · 34.3 AIC · ⌖ 2.55 AIC · ⊞ 9.2K
Comment /smoke-copilot to run again
Add label smoke to run again
| MCP_GATEWAY_API_KEY=$(openssl rand -base64 45 | tr -d '/+=') | ||
| echo "::add-mask::${MCP_GATEWAY_API_KEY}" | ||
| export MCP_GATEWAY_API_KEY | ||
| MCP_GATEWAY_AGENT_ID=$(openssl rand -base64 45 | tr -d '/+=') |
There was a problem hiding this comment.
Caveman say: line good. Test comment one.
| echo "::add-mask::${MCP_GATEWAY_API_KEY}" | ||
| export MCP_GATEWAY_API_KEY | ||
| MCP_GATEWAY_AGENT_ID=$(openssl rand -base64 45 | tr -d '/+=') | ||
| echo "::add-mask::${MCP_GATEWAY_AGENT_ID}" |
There was a problem hiding this comment.
Caveman say: mask good. Test comment two.
|
@copilot PR #56990 still needs forward progress.
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
Confirmed that all review feedback items remain fully addressed on this PR:
|
The MCP gateway schema now accepts
gateway.agentId/gateway.agentIdsand rejects legacygateway.apiKey. This updates compiler output and runtime wiring to useagentIdterminology while preserving unrelated MCP Scripts API-key authorization.Compiler/runtime config
APIKeytoAgentID.sandbox.mcp.api-keytosandbox.mcp.agent-id.MCP_GATEWAY_API_KEY.Generated gateway JSON and environment
agentId.MCP_GATEWAY_AGENT_ID.{ "gateway": { "agentId": "${MCP_GATEWAY_AGENT_ID}" } }Schemas, fixtures, and docs
sandbox.mcp.agent-id.Preserved behavior
Overview
Processed the current compact PR candidate set and nudged the highest-priority eligible pull requests. PRs with actively running recent checks were skipped to avoid duplicate churn.
Counters
Nudged PRs
Skipped details
on.cooldownworkflow gating #56998 — checks_running_recentWarning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
Run: https://github.com/github/gh-aw/actions/runs/33289099553
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.