Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e085685c-e00c-4481-8dca-8f1c8449559e
… duplication (#issue) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e085685c-e00c-4481-8dca-8f1c8449559e
There was a problem hiding this comment.
Pull request overview
Refactors repeated JSON MCP config rendering scaffolding across engine modules into a shared helper to reduce duplication and centralize behavior.
Changes:
- Added
renderStandardJSONMCPConfighelper to encapsulate the common JSON MCP config rendering sequence. - Updated Claude, Gemini, and Codex MCP renderers to use the shared helper.
- Updated Copilot MCP renderer to use the helper and extracted the cache-memory filter into
copilotMCPToolFilter. - Updated
smoke-codex.lock.ymlsafe-outputs tool schema and theadd_smoked_labelstep inputs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/mcp_renderer_helpers.go | Introduces shared JSON MCP config rendering helper used by multiple engines. |
| pkg/workflow/claude_mcp.go | Replaces duplicated JSON MCP config scaffolding with helper call. |
| pkg/workflow/gemini_mcp.go | Replaces duplicated JSON MCP config scaffolding with helper call. |
| pkg/workflow/copilot_mcp.go | Uses helper + extracts Copilot MCP tool filter for cache-memory exclusion. |
| pkg/workflow/codex_mcp.go | Uses helper for JSON gateway config generation portion. |
| .github/workflows/smoke-codex.lock.yml | Adjusts safe-outputs dynamic tool schema and action input wiring for smoked label step. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func renderStandardJSONMCPConfig( | ||
| yaml *strings.Builder, | ||
| tools map[string]any, | ||
| mcpTools []string, | ||
| workflowData *WorkflowData, | ||
| configPath string, | ||
| includeCopilotFields bool, | ||
| inlineArgs bool, | ||
| webFetchIncludeTools bool, | ||
| renderCustom RenderCustomMCPToolConfigHandler, | ||
| filterTool func(string) bool, |
There was a problem hiding this comment.
renderStandardJSONMCPConfig takes several positional boolean parameters (includeCopilotFields, inlineArgs, webFetchIncludeTools). At the call sites this results in false, false, false argument sequences, which are easy to mis-order and hard to review. Consider replacing these booleans with a small options struct (or a single renderFlags struct) so callers can set named fields, improving readability and reducing the risk of flag mixups during future edits.
| } | ||
| }, | ||
| "required": [ | ||
| "labels" |
There was a problem hiding this comment.
The add_smoked_label tool schema marks only labels as required, but the workflow step later unconditionally reads and passes .number from the exported payload. If the agent omits number (allowed by this schema), fromJSON(...).number will be null/empty and action-add-labels may fail or label the wrong target. Make number required in the schema and/or default number in the step expression (e.g., fall back to the current PR/issue number).
| "labels" | |
| "labels", | |
| "number" |
| { | ||
| "description": "Add the 'smoked' label to the current pull request (can only be called once)", | ||
| "inputSchema": { | ||
| "additionalProperties": true, | ||
| "additionalProperties": false, | ||
| "properties": { | ||
| "payload": { | ||
| "description": "JSON-encoded payload to pass to the action", | ||
| "labels": { | ||
| "description": "The labels' name to be added. Must be separated with line breaks if there're multiple labels.", | ||
| "type": "string" | ||
| }, |
There was a problem hiding this comment.
This PR is described as a Go refactor to extract shared MCP rendering logic, but this workflow lock file also changes the safe-outputs dynamic tool schema and how the add_smoked_label step is invoked (switching from a single payload input to labels/number). If this is intentional, please update the PR description to explain why this workflow behavior/schema change is included; otherwise consider moving it to a separate PR to keep the refactor scoped.
The same ~10-line JSON MCP config scaffolding sequence (
buildMCPRendererFactory→buildMCPGatewayConfig→buildStandardJSONMCPRenderers→RenderJSONMCPConfig) was copy-pasted acrossclaude_mcp.go,gemini_mcp.go,copilot_mcp.go, andcodex_mcp.gowith only minor per-engine variations.Changes
mcp_renderer_helpers.go— newrenderStandardJSONMCPConfighelper encapsulating the shared scaffolding; engine callers pass only what differs:configPath, renderer flags, custom tool callback, and optional filterclaude_mcp.go,gemini_mcp.go,codex_mcp.go— replaced repeated block with singlerenderStandardJSONMCPConfig(...)callcopilot_mcp.go— same, plus extracted anonymous cache-memory filter to namedcopilotMCPToolFilterfor testabilityBefore → After
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:
https://api.github.com/graphql/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw GOPROXY 64/bin/go git rev-�� --show-toplevel /opt/hostedtoolcache/go/1.25.0/xGO111MODULE /usr/bin/git -codex.lock.yml -trimpath 64/bin/go git(http block)https://api.github.com/repos/actions/ai-inference/git/ref/tags/v1/usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha echo "��� All validations passed" GOPROXY /snap/bin/git GOSUMDB GOWORK 64/bin/go git diff�� --name-only HEAD /opt/pipx_bin/git g/styles/huh_thegit g/styles/theme.grev-parse 64/bin/go git(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v3/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq .object.sha 495fe067:.github/workflows/smoke-codex.lock.yml GO111MODULE 0/x64/bin/bash GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE ache/uv/0.10.12/x86_64/bash GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v5/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha k/gh-aw/gh-aw/scripts/lint_errorGOINSECURE k/gh-aw/gh-aw/scripts/lint_errorGOMOD 64/bin/go **/*.json --ignore-path ../../../.pretti3303ecd19f88023c28d47289bacb7129495fe067:pkg/workflow/mcp_renderer_helpers.go /opt/hostedtoolcache/go/1.25.0/xGO111MODULE -o .go -trimpath 64/bin/go -p main -lang=go1.25 go(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha 3303ecd19f88023c28d47289bacb7129495fe067:pkg/workflow/copilot_mcp.go ache/go/1.25.0/xGO111MODULE /usr/bin/git 0325691/b407/_pkgit GO111MODULE 64/bin/go git rev-�� --show-toplevel go /usr/bin/git S_Kw/nzbp3yAZeyWgit GO111MODULE 64/bin/go git(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha --show-toplevel go /usr/bin/git -json GO111MODULE 64/bin/go git rev-�� --show-toplevel go /usr/bin/git -json GO111MODULE 64/bin/go git(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v6/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha 28d47289bacb7129495fe067:pkg/workflow/copilot_mcp.go GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE /usr/bin/sed GOINSECURE GOMOD GOMODCACHE sed(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha 28d47289bacb7129495fe067:pkg/workflow/gemini_mcp.go GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/git GOINSECURE GOMOD GOMODCACHE git(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha --show-toplevel go /usr/bin/git -json GO111MODULE 64/bin/go git rev-�� --show-toplevel go /usr/bin/git -json GO111MODULE 64/bin/go git(http block)https://api.github.com/repos/actions/github-script/git/ref/tags/v8/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha 28d47289bacb7129495fe067:pkg/workflow/copilot_mcp.go stmain.go ache/go/1.25.0/x64/pkg/tool/linux_amd64/link GOINSECURE GOMOD GOMODCACHE ache/go/1.25.0/x64/pkg/tool/linux_amd64/link env -json GO111MODULE /usr/bin/dirname GOINSECURE GOMOD GOMODCACHE dirname(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha 28d47289bacb7129495fe067:pkg/workflow/gemini_mcp.go GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE /opt/hostedtoolcache/uv/0.10.12/x86_64/git GOINSECURE GOMOD GOMODCACHE git(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha 28d47289bacb7129495fe067:pkg/workflow/gemini_mcp.go GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE /home/REDACTED/work/_temp/ghcca-node/node/bin/git GOINSECURE GOMOD GOMODCACHE git(http block)https://api.github.com/repos/actions/setup-go/git/ref/tags/v4/usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v4 --jq .object.sha 28d47289bacb7129495fe067:pkg/workflow/gemini_mcp.go GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE /var/lib/waagent/Microsoft.CPlat.Core.RunCommandLinux-1.0.9/bin/run-command-extension GOINSECURE GOMOD GOMODCACHE /var/lib/waagent/Microsoft.CPlat.Core.RunCommandLinux-1.0.9/bin/run-command-extension(http block)https://api.github.com/repos/actions/setup-node/git/ref/tags/v4/usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v4 --jq .object.sha 28d47289bacb7129495fe067:pkg/workflow/copilot_mcp.go GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE /usr/bin/lsof GOINSECURE GOMOD GOMODCACHE lsof(http block)https://api.github.com/repos/actions/upload-artifact/git/ref/tags/v4/usr/bin/gh gh api /repos/actions/upload-artifact/git/ref/tags/v4 --jq .object.sha -json GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go .Cor�� -json GO111MODULE ndor/bin/git GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v1.0.0/usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v1.0.0 --jq .object.sha -json GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v1.2.3/usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v1.2.3 --jq .object.sha -json GO111MODULE 0/x64/bin/git GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.0.0/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha xNWR/tA_Iybv8MbMGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE 0325691/b410/impGOPROXY -c che/go-build/bf/GOSUMDB GOPROXY 64/bin/go GOSUMDB GOWORK 64/bin/go /opt/hostedtoolcache/go/1.25.0/xGO111MODULE(http block)https://api.github.com/repos/nonexistent/action/git/ref/tags/v999.999.999/usr/bin/gh gh api /repos/nonexistent/action/git/ref/tags/v999.999.999 --jq .object.sha uNgZ/AfH5nzLof4eGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE 0325691/b392/impGOPROXY -c che/go-build/52/GOSUMDB GOPROXY 64/bin/go GOSUMDB GOWORK 64/bin/go /opt/hostedtoolcache/go/1.25.0/xGO111MODULE(http block)If you need me to access, download, or install something from one of these locations, you can either:
📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.