diff --git a/.github/workflows/ci-coach.lock.yml b/.github/workflows/ci-coach.lock.yml index 3649a9d481f..793c3b844d7 100644 --- a/.github/workflows/ci-coach.lock.yml +++ b/.github/workflows/ci-coach.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v3","frontmatter_hash":"87ef52d4045a4717505db1be172d02d8e8263433d50c75a27088775647fbc806","strict":true,"agent_id":"copilot"} +# gh-aw-metadata: {"schema_version":"v3","frontmatter_hash":"e136939b07fd346de09cb6bdb589a64fb7ffe45d31d23f433859b10038571c30","strict":true,"agent_id":"copilot"} # gh-aw-manifest: {"version":1,"secrets":["GH_AW_CI_TRIGGER_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GITHUB_TOKEN"],"actions":[{"repo":"actions/cache/restore","sha":"27d5ce7f107fe9357f9df03efb73ab90386fccae","version":"v5.0.5"},{"repo":"actions/cache/save","sha":"27d5ce7f107fe9357f9df03efb73ab90386fccae","version":"v5.0.5"},{"repo":"actions/checkout","sha":"de0fac2e4500dabe0009e67214ff5f5447ce83dd","version":"v6.0.2"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"373c709c69115d41ff229c7e5df9f8788daa9553","version":"v9"},{"repo":"actions/setup-go","sha":"4a3601121dd01d1626a1e23e37211e3254c1c06c","version":"v6.4.0"},{"repo":"actions/setup-node","sha":"48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e","version":"v6.4.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.25.26"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.25.26"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.25.26"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.25.26"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.2.26"},{"image":"ghcr.io/github/github-mcp-server:v1.0.0"},{"image":"node:lts-alpine","digest":"sha256:01743339035a5c3c11a373cd7c83aeab6ed1457b55da6a69e014a95ac4e4700b","pinned_image":"node:lts-alpine@sha256:01743339035a5c3c11a373cd7c83aeab6ed1457b55da6a69e014a95ac4e4700b"}]} # ___ _ _ # / _ \ | | (_) @@ -180,24 +180,24 @@ jobs: run: | bash "${RUNNER_TEMP}/gh-aw/actions/create_prompt_first.sh" { - cat << 'GH_AW_PROMPT_94a00665cb9be77b_EOF' + cat << 'GH_AW_PROMPT_b695ae7e2ca86425_EOF' - GH_AW_PROMPT_94a00665cb9be77b_EOF + GH_AW_PROMPT_b695ae7e2ca86425_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/xpia.md" cat "${RUNNER_TEMP}/gh-aw/prompts/temp_folder_prompt.md" cat "${RUNNER_TEMP}/gh-aw/prompts/markdown.md" cat "${RUNNER_TEMP}/gh-aw/prompts/cache_memory_prompt.md" cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_prompt.md" - cat << 'GH_AW_PROMPT_94a00665cb9be77b_EOF' + cat << 'GH_AW_PROMPT_b695ae7e2ca86425_EOF' Tools: create_pull_request, missing_tool, missing_data, noop - GH_AW_PROMPT_94a00665cb9be77b_EOF + GH_AW_PROMPT_b695ae7e2ca86425_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_create_pull_request.md" - cat << 'GH_AW_PROMPT_94a00665cb9be77b_EOF' + cat << 'GH_AW_PROMPT_b695ae7e2ca86425_EOF' - GH_AW_PROMPT_94a00665cb9be77b_EOF + GH_AW_PROMPT_b695ae7e2ca86425_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/mcp_cli_tools_prompt.md" - cat << 'GH_AW_PROMPT_94a00665cb9be77b_EOF' + cat << 'GH_AW_PROMPT_b695ae7e2ca86425_EOF' The following GitHub context information is available for this workflow: {{#if __GH_AW_GITHUB_ACTOR__ }} @@ -226,16 +226,16 @@ jobs: {{/if}} - GH_AW_PROMPT_94a00665cb9be77b_EOF + GH_AW_PROMPT_b695ae7e2ca86425_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/cli_proxy_with_safeoutputs_prompt.md" - cat << 'GH_AW_PROMPT_94a00665cb9be77b_EOF' + cat << 'GH_AW_PROMPT_b695ae7e2ca86425_EOF' {{#runtime-import .github/workflows/shared/ci-data-analysis.md}} {{#runtime-import .github/workflows/shared/ci-optimization-strategies.md}} {{#runtime-import .github/workflows/shared/reporting.md}} {{#runtime-import .github/workflows/shared/jqschema.md}} {{#runtime-import .github/workflows/ci-coach.md}} - GH_AW_PROMPT_94a00665cb9be77b_EOF + GH_AW_PROMPT_b695ae7e2ca86425_EOF } > "$GH_AW_PROMPT" - name: Interpolate variables and render templates uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9 @@ -484,9 +484,9 @@ jobs: mkdir -p "${RUNNER_TEMP}/gh-aw/safeoutputs" mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs - cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_cf6ad31dfdb35060_EOF' + cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_e29b2de446ecd11f_EOF' {"create_pull_request":{"expires":48,"max":1,"max_patch_size":1024,"protected_files":["package.json","bun.lockb","bunfig.toml","deno.json","deno.jsonc","deno.lock","global.json","NuGet.Config","Directory.Packages.props","mix.exs","mix.lock","go.mod","go.sum","stack.yaml","stack.yaml.lock","pom.xml","build.gradle","build.gradle.kts","settings.gradle","settings.gradle.kts","gradle.properties","package-lock.json","yarn.lock","pnpm-lock.yaml","npm-shrinkwrap.json","requirements.txt","Pipfile","Pipfile.lock","pyproject.toml","setup.py","setup.cfg","Gemfile","Gemfile.lock","uv.lock","CODEOWNERS","AGENTS.md","CLAUDE.md","GEMINI.md"],"protected_files_policy":"fallback-to-issue","protected_path_prefixes":[".github/",".agents/"],"title_prefix":"[ci-coach] "},"create_report_incomplete_issue":{},"missing_data":{},"missing_tool":{},"noop":{"max":1,"report-as-issue":"true"},"report_incomplete":{}} - GH_AW_SAFE_OUTPUTS_CONFIG_cf6ad31dfdb35060_EOF + GH_AW_SAFE_OUTPUTS_CONFIG_e29b2de446ecd11f_EOF - name: Write Safe Outputs Tools env: GH_AW_TOOLS_META_JSON: | @@ -690,7 +690,7 @@ jobs: mkdir -p /home/runner/.copilot GH_AW_NODE=$(which node 2>/dev/null || command -v node 2>/dev/null || echo node) - cat << GH_AW_MCP_CONFIG_309bc2fefb7ad12f_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.cjs" + cat << GH_AW_MCP_CONFIG_c1c59fbc56831ea4_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.cjs" { "mcpServers": { "safeoutputs": { @@ -715,7 +715,7 @@ jobs: "payloadDir": "${MCP_GATEWAY_PAYLOAD_DIR}" } } - GH_AW_MCP_CONFIG_309bc2fefb7ad12f_EOF + GH_AW_MCP_CONFIG_c1c59fbc56831ea4_EOF - name: Mount MCP servers as CLIs id: mount-mcp-clis continue-on-error: true @@ -1154,7 +1154,7 @@ jobs: rm -rf /tmp/gh-aw/sandbox/firewall/logs rm -rf /tmp/gh-aw/sandbox/firewall/audit - name: Download container images - run: bash "${RUNNER_TEMP}/gh-aw/actions/download_docker_images.sh" ghcr.io/github/gh-aw-firewall/agent:0.25.26 ghcr.io/github/gh-aw-firewall/api-proxy:0.25.26 ghcr.io/github/gh-aw-firewall/cli-proxy:0.25.26 ghcr.io/github/gh-aw-firewall/squid:0.25.26 + run: bash "${RUNNER_TEMP}/gh-aw/actions/download_docker_images.sh" ghcr.io/github/gh-aw-firewall/agent:0.25.26 ghcr.io/github/gh-aw-firewall/api-proxy:0.25.26 ghcr.io/github/gh-aw-firewall/squid:0.25.26 - name: Check if detection needed id: detection_guard if: always() @@ -1225,7 +1225,7 @@ jobs: export GH_AW_NODE_BIN (umask 177 && touch /tmp/gh-aw/threat-detection/detection.log) # shellcheck disable=SC1003 - sudo -E awf --container-workdir "${GITHUB_WORKSPACE}" --mount "${RUNNER_TEMP}/gh-aw:${RUNNER_TEMP}/gh-aw:ro" --mount "${RUNNER_TEMP}/gh-aw:/host${RUNNER_TEMP}/gh-aw:ro" --env-all --exclude-env COPILOT_GITHUB_TOKEN --exclude-env GH_TOKEN --allow-domains api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,github.com,host.docker.internal,telemetry.enterprise.githubcopilot.com --log-level info --proxy-logs-dir /tmp/gh-aw/sandbox/firewall/logs --audit-dir /tmp/gh-aw/sandbox/firewall/audit --enable-host-access --allow-host-ports 80,443,8080 --image-tag 0.25.26 --skip-pull --enable-api-proxy --difc-proxy-host host.docker.internal:18443 --difc-proxy-ca-cert /tmp/gh-aw/difc-proxy-tls/ca.crt \ + sudo -E awf --container-workdir "${GITHUB_WORKSPACE}" --mount "${RUNNER_TEMP}/gh-aw:${RUNNER_TEMP}/gh-aw:ro" --mount "${RUNNER_TEMP}/gh-aw:/host${RUNNER_TEMP}/gh-aw:ro" --env-all --exclude-env COPILOT_GITHUB_TOKEN --allow-domains api.business.githubcopilot.com,api.enterprise.githubcopilot.com,api.github.com,api.githubcopilot.com,api.individual.githubcopilot.com,github.com,host.docker.internal,telemetry.enterprise.githubcopilot.com --log-level info --proxy-logs-dir /tmp/gh-aw/sandbox/firewall/logs --audit-dir /tmp/gh-aw/sandbox/firewall/audit --enable-host-access --allow-host-ports 80,443,8080 --image-tag 0.25.26 --skip-pull --enable-api-proxy \ -- /bin/bash -c 'GH_AW_NODE_EXEC="${GH_AW_NODE_BIN:-}"; if [ -z "$GH_AW_NODE_EXEC" ] || [ ! -x "$GH_AW_NODE_EXEC" ]; then GH_AW_NODE_EXEC="$(command -v node 2>/dev/null || echo node)"; fi; "$GH_AW_NODE_EXEC" ${RUNNER_TEMP}/gh-aw/actions/copilot_driver.cjs /usr/local/bin/copilot --add-dir /tmp/gh-aw/ --log-level all --log-dir /tmp/gh-aw/sandbox/agent/logs/ --disable-builtin-mcps --no-ask-user --allow-all-tools --add-dir "${GITHUB_WORKSPACE}" --prompt-file /tmp/gh-aw/aw-prompts/prompt.txt' 2>&1 | tee -a /tmp/gh-aw/threat-detection/detection.log env: COPILOT_AGENT_RUNNER_TYPE: STANDALONE @@ -1234,7 +1234,6 @@ jobs: GH_AW_PHASE: detection GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt GH_AW_VERSION: dev - GH_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN || github.token }} GITHUB_API_URL: ${{ github.api_url }} GITHUB_AW: true GITHUB_COPILOT_INTEGRATION_ID: agentic-workflows diff --git a/.github/workflows/ci-coach.md b/.github/workflows/ci-coach.md index ac0c9ac0194..9a7e75227da 100644 --- a/.github/workflows/ci-coach.md +++ b/.github/workflows/ci-coach.md @@ -14,6 +14,7 @@ engine: copilot tools: mount-as-clis: true github: + mode: gh-proxy toolsets: [issues, pull_requests] edit: safe-outputs: @@ -28,7 +29,6 @@ imports: - shared/reporting.md features: mcp-cli: true - cli-proxy: true copilot-requests: true --- diff --git a/.github/workflows/copilot-token-optimizer.lock.yml b/.github/workflows/copilot-token-optimizer.lock.yml index 570a95b45d2..e55a5b7e1c5 100644 --- a/.github/workflows/copilot-token-optimizer.lock.yml +++ b/.github/workflows/copilot-token-optimizer.lock.yml @@ -1,4 +1,4 @@ -# gh-aw-metadata: {"schema_version":"v3","frontmatter_hash":"40fb326f0a5898f90136a1deab92f2a2e692b3699c12bde4c9238abd8a6079ea","strict":true,"agent_id":"copilot"} +# gh-aw-metadata: {"schema_version":"v3","frontmatter_hash":"7e0d43b5343d78a0f01e14145fa97b05d592ccf425264140a2e8dc7095420fdc","strict":true,"agent_id":"copilot"} # gh-aw-manifest: {"version":1,"secrets":["COPILOT_GITHUB_TOKEN","GH_AW_GITHUB_MCP_SERVER_TOKEN","GH_AW_GITHUB_TOKEN","GITHUB_TOKEN"],"actions":[{"repo":"actions/checkout","sha":"de0fac2e4500dabe0009e67214ff5f5447ce83dd","version":"v6.0.2"},{"repo":"actions/download-artifact","sha":"3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c","version":"v8.0.1"},{"repo":"actions/github-script","sha":"373c709c69115d41ff229c7e5df9f8788daa9553","version":"v9"},{"repo":"actions/setup-go","sha":"4a3601121dd01d1626a1e23e37211e3254c1c06c","version":"v6.4.0"},{"repo":"actions/setup-node","sha":"48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e","version":"v6.4.0"},{"repo":"actions/upload-artifact","sha":"043fb46d1a93c77aae656e7c1c64a875d1fc6a0a","version":"v7.0.1"},{"repo":"astral-sh/setup-uv","sha":"eac588ad8def6316056a12d4907a9d4d84ff7a3b","version":"eac588ad8def6316056a12d4907a9d4d84ff7a3b"}],"containers":[{"image":"ghcr.io/github/gh-aw-firewall/agent:0.25.26"},{"image":"ghcr.io/github/gh-aw-firewall/api-proxy:0.25.26"},{"image":"ghcr.io/github/gh-aw-firewall/cli-proxy:0.25.26"},{"image":"ghcr.io/github/gh-aw-firewall/squid:0.25.26"},{"image":"ghcr.io/github/gh-aw-mcpg:v0.2.26"},{"image":"ghcr.io/github/github-mcp-server:v1.0.0"},{"image":"node:lts-alpine","digest":"sha256:01743339035a5c3c11a373cd7c83aeab6ed1457b55da6a69e014a95ac4e4700b","pinned_image":"node:lts-alpine@sha256:01743339035a5c3c11a373cd7c83aeab6ed1457b55da6a69e014a95ac4e4700b"}]} # ___ _ _ # / _ \ | | (_) @@ -182,21 +182,21 @@ jobs: run: | bash "${RUNNER_TEMP}/gh-aw/actions/create_prompt_first.sh" { - cat << 'GH_AW_PROMPT_66aeb9478fc96e44_EOF' + cat << 'GH_AW_PROMPT_fb7a896802031a54_EOF' - GH_AW_PROMPT_66aeb9478fc96e44_EOF + GH_AW_PROMPT_fb7a896802031a54_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/xpia.md" cat "${RUNNER_TEMP}/gh-aw/prompts/temp_folder_prompt.md" cat "${RUNNER_TEMP}/gh-aw/prompts/markdown.md" cat "${RUNNER_TEMP}/gh-aw/prompts/repo_memory_prompt.md" cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_prompt.md" - cat << 'GH_AW_PROMPT_66aeb9478fc96e44_EOF' + cat << 'GH_AW_PROMPT_fb7a896802031a54_EOF' Tools: create_issue, missing_tool, missing_data, noop - GH_AW_PROMPT_66aeb9478fc96e44_EOF + GH_AW_PROMPT_fb7a896802031a54_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/mcp_cli_tools_prompt.md" - cat << 'GH_AW_PROMPT_66aeb9478fc96e44_EOF' + cat << 'GH_AW_PROMPT_fb7a896802031a54_EOF' The following GitHub context information is available for this workflow: {{#if __GH_AW_GITHUB_ACTOR__ }} @@ -225,13 +225,13 @@ jobs: {{/if}} - GH_AW_PROMPT_66aeb9478fc96e44_EOF + GH_AW_PROMPT_fb7a896802031a54_EOF cat "${RUNNER_TEMP}/gh-aw/prompts/cli_proxy_with_safeoutputs_prompt.md" - cat << 'GH_AW_PROMPT_66aeb9478fc96e44_EOF' + cat << 'GH_AW_PROMPT_fb7a896802031a54_EOF' {{#runtime-import .github/workflows/shared/reporting.md}} {{#runtime-import .github/workflows/copilot-token-optimizer.md}} - GH_AW_PROMPT_66aeb9478fc96e44_EOF + GH_AW_PROMPT_fb7a896802031a54_EOF } > "$GH_AW_PROMPT" - name: Interpolate variables and render templates uses: actions/github-script@373c709c69115d41ff229c7e5df9f8788daa9553 # v9 @@ -476,9 +476,9 @@ jobs: mkdir -p "${RUNNER_TEMP}/gh-aw/safeoutputs" mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs - cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_08970fa336cef046_EOF' + cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_c2524c983d8ad50d_EOF' {"create_issue":{"close_older_issues":true,"expires":168,"max":1,"title_prefix":"[copilot-token-optimizer] "},"create_report_incomplete_issue":{},"missing_data":{},"missing_tool":{},"noop":{"max":1,"report-as-issue":"true"},"push_repo_memory":{"memories":[{"dir":"/tmp/gh-aw/repo-memory/default","id":"default","max_file_count":100,"max_file_size":102400,"max_patch_size":51200}]},"report_incomplete":{}} - GH_AW_SAFE_OUTPUTS_CONFIG_08970fa336cef046_EOF + GH_AW_SAFE_OUTPUTS_CONFIG_c2524c983d8ad50d_EOF - name: Write Safe Outputs Tools env: GH_AW_TOOLS_META_JSON: | @@ -674,7 +674,7 @@ jobs: mkdir -p /home/runner/.copilot GH_AW_NODE=$(which node 2>/dev/null || command -v node 2>/dev/null || echo node) - cat << GH_AW_MCP_CONFIG_6638e68b137dda0f_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.cjs" + cat << GH_AW_MCP_CONFIG_076e6d50d41d08f0_EOF | "$GH_AW_NODE" "${RUNNER_TEMP}/gh-aw/actions/start_mcp_gateway.cjs" { "mcpServers": { "safeoutputs": { @@ -699,7 +699,7 @@ jobs: "payloadDir": "${MCP_GATEWAY_PAYLOAD_DIR}" } } - GH_AW_MCP_CONFIG_6638e68b137dda0f_EOF + GH_AW_MCP_CONFIG_076e6d50d41d08f0_EOF - name: Mount MCP servers as CLIs id: mount-mcp-clis continue-on-error: true diff --git a/.github/workflows/copilot-token-optimizer.md b/.github/workflows/copilot-token-optimizer.md index e2028e4c88e..d9c67bac20c 100644 --- a/.github/workflows/copilot-token-optimizer.md +++ b/.github/workflows/copilot-token-optimizer.md @@ -13,6 +13,7 @@ tracker-id: copilot-token-optimizer engine: copilot tools: github: + mode: gh-proxy toolsets: [issues] bash: - "*" @@ -34,7 +35,6 @@ imports: - shared/reporting.md features: mcp-cli: true - cli-proxy: true steps: - name: Download recent Copilot workflow logs env: @@ -114,6 +114,7 @@ steps: echo "ℹ️ No previous optimization history found." fi --- + {{#runtime-import? .github/shared-instructions.md}} # Copilot Token Usage Optimizer diff --git a/docs/adr/27707-unify-github-access-mode-under-tools-github-mode.md b/docs/adr/27707-unify-github-access-mode-under-tools-github-mode.md new file mode 100644 index 00000000000..b263b2214d1 --- /dev/null +++ b/docs/adr/27707-unify-github-access-mode-under-tools-github-mode.md @@ -0,0 +1,75 @@ +# ADR-27707: Unify GitHub Access Mode Under `tools.github.mode` + +**Date**: 2026-04-21 +**Status**: Draft +**Deciders**: pelikhan + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +The gh-aw workflow compiler historically controlled GitHub access behavior through a `features.cli-proxy` boolean flag. When `true`, the agent used a pre-authenticated `gh` CLI for GitHub reads instead of the GitHub MCP server. Separately, `tools.github.mode` controlled only MCP transport type (`local` vs `remote`). This split created semantic ambiguity: one concept (how the agent talks to GitHub) was expressed in two different configuration places with incompatible value spaces. As the platform matured, the MCP server gained a `remote` hosted variant at `api.githubcopilot.com`, and a cleaner abstraction was needed that could express all three access modes — CLI, local MCP, and remote MCP — in one unified location. + +### Decision + +We will consolidate GitHub agent-access semantics into `tools.github.mode` with three values: `gh-proxy` (pre-authenticated `gh` CLI guidance, replacing `features.cli-proxy: true`), `local` (Docker-based MCP server, previously the implicit default), and `remote` (hosted MCP server at api.githubcopilot.com). We will additionally introduce `tools.github.type` to carry the MCP transport type (`local|remote`) independently of the new CLI/MCP semantic distinction, allowing them to evolve separately. Legacy `features.cli-proxy: true` configurations remain functional through a backward-compatibility fallback; a codemod migrates them automatically to `tools.github.mode: gh-proxy`. + +### Alternatives Considered + +#### Alternative 1: Keep `features.cli-proxy` as the primary flag + +The boolean flag could have been extended with a third `remote-mcp` option or companion flags. This was rejected because the `features.*` namespace is intended for unstable/experimental toggles, and CLI-vs-MCP is now a stable, first-class configuration concern. Mixing stable mode semantics into the feature-flag namespace would increase confusion and make schema documentation harder to maintain. + +#### Alternative 2: Overload `tools.github.mode` with both access mode and transport type in a flat value set + +`tools.github.mode` could have accepted all four combinations as individual string values (e.g., `gh-proxy`, `local-mcp`, `remote-mcp`). This was rejected because `local` and `remote` already had documented meanings as MCP transport values; renaming them would be a breaking change. Separating concerns into `mode` (access paradigm) and `type` (transport) is more expressive and forwards-compatible. + +### Consequences + +#### Positive +- Single, canonical location for GitHub access configuration, reducing cognitive load for workflow authors. +- `features.cli-proxy` removal cleans up the feature-flag namespace; the codemod ensures a smooth migration path. +- `tools.github.type` provides an independent dimension for future MCP transport evolution without re-breaking `mode` semantics. + +#### Negative +- `tools.github.mode` now carries two overlapping semantic layers: the new `gh-proxy` value has a distinct meaning from the legacy `local`/`remote` values, which now describe only transport. This requires careful documentation and can confuse readers who encounter a `mode: local` value and expect it to mean "not CLI mode." +- The backward-compatibility fallback means the compiler must maintain both code paths until all existing workflows have been migrated, increasing maintenance surface. + +#### Neutral +- All existing lock files are regenerated as a side effect of the schema change (frontmatter hash churn across `.lock.yml` files). +- The new `tools.github.type` field is plumbed through the schema and compiler but has no user-visible prompt behavior change; it is effectively a no-op for current workflows that don't set it explicitly. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### GitHub Access Mode (`tools.github.mode`) + +1. Implementations **MUST** treat `tools.github.mode: gh-proxy` as equivalent to the legacy `features.cli-proxy: true` behavior: the agent **MUST** receive pre-authenticated `gh` CLI prompt guidance and **MUST NOT** register a GitHub MCP server for reads. +2. Implementations **MUST** treat `tools.github.mode: local` and `tools.github.mode: remote` as MCP transport selectors; these values **MUST NOT** activate CLI-proxy prompt behavior. +3. When `tools.github.mode` is absent, implementations **MUST** fall back to the value of `features.cli-proxy` for backward compatibility. +4. Implementations **MUST NOT** silently ignore unrecognized `tools.github.mode` values; they **SHOULD** log a warning and fall back to legacy behavior. + +### GitHub MCP Transport Type (`tools.github.type`) + +1. Implementations **MUST** accept `tools.github.type: local` and `tools.github.type: remote` to specify MCP transport independently of `tools.github.mode`. +2. When `tools.github.type` is present, implementations **MUST** prefer it over the legacy `tools.github.mode` transport interpretation for determining MCP transport. +3. Implementations **MAY** accept `tools.github.mode: local` and `tools.github.mode: remote` as a fallback transport selector when `tools.github.type` is absent, for backward compatibility. + +### Migration (Codemod) + +1. The codemod **MUST** transform `features.cli-proxy: true` into `tools.github.mode: gh-proxy` in workflow frontmatter. +2. The codemod **MUST NOT** alter any other frontmatter keys or values. +3. The codemod **SHOULD** be idempotent: re-running it on an already-migrated file **MUST NOT** produce additional changes. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24751474414) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 6931997c17e..0c5b380c1d8 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -1872,10 +1872,17 @@ tools: allowed: [] # Array of strings - # MCP server mode: 'local' (Docker-based, default) or 'remote' (hosted at - # api.githubcopilot.com) + # GitHub access mode. Prefer 'gh-proxy' for better performance (uses + # pre-authenticated gh CLI prompt guidance). Legacy MCP transport values 'local' + # and 'remote' are accepted for backward compatibility and use GitHub MCP server + # prompt guidance. # (optional) - mode: "local" + mode: "gh-proxy" + + # GitHub MCP transport type: 'local' (Docker-based, default) or 'remote' (hosted + # at api.githubcopilot.com) + # (optional) + type: "local" # Optional version specification for the GitHub MCP server (used with 'local' # type). Can be a string (e.g., 'v1.0.0', 'latest') or number (e.g., 20, 3.11). diff --git a/pkg/cli/codemod_cli_proxy_mode.go b/pkg/cli/codemod_cli_proxy_mode.go new file mode 100644 index 00000000000..937ff95315c --- /dev/null +++ b/pkg/cli/codemod_cli_proxy_mode.go @@ -0,0 +1,147 @@ +package cli + +import ( + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var cliProxyModeCodemodLog = logger.New("cli:codemod_cli_proxy_mode") + +// getCliProxyFeatureToGitHubModeCodemod migrates features.cli-proxy: true to tools.github.mode: gh-proxy. +func getCliProxyFeatureToGitHubModeCodemod() Codemod { + return Codemod{ + ID: "features-cli-proxy-to-tools-github-mode", + Name: "Migrate 'features.cli-proxy: true' to 'tools.github.mode: gh-proxy'", + Description: "Removes deprecated features.cli-proxy: true and sets tools.github.mode: gh-proxy (equivalent behavior).", + IntroducedIn: "1.0.0", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + if !hasLegacyCliProxyFeatureEnabled(frontmatter) { + return content, false, nil + } + hasMode := hasToolsGitHubMode(frontmatter) + + newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) { + result, modified := removeFieldFromBlock(lines, "cli-proxy", "features") + if !modified { + return lines, false + } + if !hasMode { + result = addGitHubModeGhProxyToTools(result) + } + return result, true + }) + if applied { + cliProxyModeCodemodLog.Print("Migrated features.cli-proxy: true to tools.github.mode: gh-proxy") + } + return newContent, applied, err + }, + } +} + +func hasLegacyCliProxyFeatureEnabled(frontmatter map[string]any) bool { + featuresAny, ok := frontmatter["features"] + if !ok { + return false + } + featuresMap, ok := featuresAny.(map[string]any) + if !ok { + return false + } + value, has := featuresMap["cli-proxy"] + if !has { + return false + } + enabled, ok := value.(bool) + return ok && enabled +} + +func hasToolsGitHubMode(frontmatter map[string]any) bool { + toolsAny, hasTools := frontmatter["tools"] + if !hasTools { + return false + } + toolsMap, ok := toolsAny.(map[string]any) + if !ok { + return false + } + githubAny, hasGitHub := toolsMap["github"] + if !hasGitHub { + return false + } + githubMap, ok := githubAny.(map[string]any) + if !ok { + return false + } + _, hasMode := githubMap["mode"] + return hasMode +} + +func addGitHubModeGhProxyToTools(lines []string) []string { + toolsLine := -1 + toolsIndent := "" + + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "tools:") { + toolsLine = i + toolsIndent = getIndentation(line) + break + } + } + + if toolsLine == -1 { + return append(lines, "tools:", " github:", " mode: gh-proxy") + } + + githubLine := -1 + githubIndent := "" + toolsEnd := len(lines) + for i := toolsLine + 1; i < len(lines); i++ { + trimmed := strings.TrimSpace(lines[i]) + if len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") && hasExitedBlock(lines[i], toolsIndent) { + toolsEnd = i + break + } + if strings.HasPrefix(trimmed, "github:") && strings.HasPrefix(getIndentation(lines[i]), toolsIndent+" ") { + githubLine = i + githubIndent = getIndentation(lines[i]) + break + } + } + + if githubLine == -1 { + result := make([]string, 0, len(lines)+2) + result = append(result, lines[:toolsEnd]...) + result = append(result, toolsIndent+" github:") + result = append(result, toolsIndent+" mode: gh-proxy") + result = append(result, lines[toolsEnd:]...) + return result + } + + if strings.TrimSpace(lines[githubLine]) != "github:" { + cliProxyModeCodemodLog.Print("Skipping mode addition: github line has inline content") + return lines + } + + fieldIndent := githubIndent + " " + insertAt := githubLine + 1 + for i := githubLine + 1; i < len(lines); i++ { + trimmed := strings.TrimSpace(lines[i]) + if len(trimmed) > 0 && !strings.HasPrefix(trimmed, "#") { + if hasExitedBlock(lines[i], githubIndent) { + insertAt = i + } else { + fieldIndent = getIndentation(lines[i]) + insertAt = i + } + break + } + } + + result := make([]string, 0, len(lines)+1) + result = append(result, lines[:insertAt]...) + result = append(result, fieldIndent+"mode: gh-proxy") + result = append(result, lines[insertAt:]...) + return result +} diff --git a/pkg/cli/codemod_cli_proxy_mode_test.go b/pkg/cli/codemod_cli_proxy_mode_test.go new file mode 100644 index 00000000000..1d32fa440d9 --- /dev/null +++ b/pkg/cli/codemod_cli_proxy_mode_test.go @@ -0,0 +1,117 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCliProxyFeatureToGitHubModeCodemod(t *testing.T) { + codemod := getCliProxyFeatureToGitHubModeCodemod() + + t.Run("migrates features.cli-proxy true and adds tools.github.mode gh-proxy", func(t *testing.T) { + content := `--- +features: + cli-proxy: true +--- + +# Test +` + frontmatter := map[string]any{ + "features": map[string]any{ + "cli-proxy": true, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.NotContains(t, result, "cli-proxy:") + assert.Contains(t, result, "tools:") + assert.Contains(t, result, "github:") + assert.Contains(t, result, "mode: gh-proxy") + }) + + t.Run("does not apply when cli-proxy is false", func(t *testing.T) { + content := `--- +features: + cli-proxy: false +--- + +# Test +` + frontmatter := map[string]any{ + "features": map[string]any{ + "cli-proxy": false, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) + }) + + t.Run("removes legacy flag but preserves existing github mode", func(t *testing.T) { + content := `--- +features: + cli-proxy: true +tools: + github: + mode: local +--- + +# Test +` + frontmatter := map[string]any{ + "features": map[string]any{ + "cli-proxy": true, + }, + "tools": map[string]any{ + "github": map[string]any{ + "mode": "local", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.NotContains(t, result, "cli-proxy:") + assert.Contains(t, result, "mode: local") + assert.NotContains(t, result, "mode: gh-proxy") + }) + + t.Run("adds github block under existing tools block", func(t *testing.T) { + content := `--- +features: + cli-proxy: true +tools: + playwright: + version: v1.50.0 +--- + +# Test +` + frontmatter := map[string]any{ + "features": map[string]any{ + "cli-proxy": true, + }, + "tools": map[string]any{ + "playwright": map[string]any{ + "version": "v1.50.0", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "playwright:") + assert.Contains(t, result, "github:") + assert.Contains(t, result, "mode: gh-proxy") + }) +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index c7a64fb3472..1dc9f9e0e0b 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -54,6 +54,7 @@ func GetAllCodemods() []Codemod { getPluginsToDependenciesCodemod(), // Migrate plugins to dependencies (plugins removed in favour of APM) getSerenaToSharedImportCodemod(), // Migrate removed tools.serena to shared/mcp/serena.md import getGitHubReposToAllowedReposCodemod(), // Rename deprecated tools.github.repos to tools.github.allowed-repos + getCliProxyFeatureToGitHubModeCodemod(), // Migrate features.cli-proxy: true to tools.github.mode: gh-proxy getDIFCProxyToIntegrityProxyCodemod(), // Migrate deprecated features.difc-proxy to tools.github.integrity-proxy } fixCodemodsLog.Printf("Loaded codemod registry: %d codemods available", len(codemods)) diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 6bf98eab310..64f7b2a388d 100644 --- a/pkg/cli/fix_codemods_test.go +++ b/pkg/cli/fix_codemods_test.go @@ -43,7 +43,7 @@ func TestGetAllCodemods_ReturnsAllCodemods(t *testing.T) { codemods := GetAllCodemods() // Verify we have the expected number of codemods - expectedCount := 33 + expectedCount := 34 assert.Len(t, codemods, expectedCount, "Should return all %d codemods", expectedCount) // Verify all codemods have required fields @@ -140,6 +140,7 @@ func TestGetAllCodemods_InExpectedOrder(t *testing.T) { "plugins-to-dependencies", "serena-tools-to-shared-import", "github-repos-to-allowed-repos", + "features-cli-proxy-to-tools-github-mode", "features-difc-proxy-to-tools-github", } diff --git a/pkg/cli/workflows/test-copilot-gh-proxy.md b/pkg/cli/workflows/test-copilot-gh-proxy.md new file mode 100644 index 00000000000..0ccf56aaa13 --- /dev/null +++ b/pkg/cli/workflows/test-copilot-gh-proxy.md @@ -0,0 +1,17 @@ +--- +on: + issues: + types: [opened] +engine: copilot +permissions: + contents: read + issues: read + pull-requests: read +tools: + github: + mode: gh-proxy +--- + +# Test Copilot GH Proxy + +Verify that `tools.github.mode: gh-proxy` uses CLI proxy guidance and does not register GitHub MCP. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 06bcedfe8ea..8a6e2e4505a 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -3431,12 +3431,12 @@ }, { "github": { - "mode": "remote" + "mode": "local" } }, { "github": { - "mode": "local", + "type": "local", "version": "latest" } }, @@ -3472,9 +3472,14 @@ } }, "mode": { + "type": "string", + "enum": ["gh-proxy", "local", "remote"], + "description": "GitHub access mode. Prefer 'gh-proxy' for better performance (uses pre-authenticated gh CLI prompt guidance). Legacy MCP transport values 'local' and 'remote' are accepted for backward compatibility and use GitHub MCP server prompt guidance." + }, + "type": { "type": "string", "enum": ["local", "remote"], - "description": "MCP server mode: 'local' (Docker-based, default) or 'remote' (hosted at api.githubcopilot.com)" + "description": "GitHub MCP transport type: 'local' (Docker-based, default) or 'remote' (hosted at api.githubcopilot.com)" }, "version": { "type": ["string", "number"], diff --git a/pkg/workflow/awf_helpers.go b/pkg/workflow/awf_helpers.go index ab731c643f4..11ca3aeab4e 100644 --- a/pkg/workflow/awf_helpers.go +++ b/pkg/workflow/awf_helpers.go @@ -285,10 +285,10 @@ func BuildAWFArgs(config AWFCommandConfig) []string { awfArgs = append(awfArgs, "--enable-api-proxy") awfHelpersLog.Print("Added --enable-api-proxy for LLM API proxying") - // Enable CLI proxy sidecar when the cli-proxy feature flag is set. + // Enable CLI proxy sidecar when GitHub mode is gh-proxy. // Start the difc-proxy on the host and tell AWF where to connect - // (firewall v0.26.0+). - if isFeatureEnabled(constants.CliProxyFeatureFlag, config.WorkflowData) { + // (firewall v0.25.17+). + if isGitHubCLIModeEnabled(config.WorkflowData) { if awfSupportsCliProxy(firewallConfig) { awfArgs = append(awfArgs, "--difc-proxy-host", "host.docker.internal:18443") awfArgs = append(awfArgs, "--difc-proxy-ca-cert", "/tmp/gh-aw/difc-proxy-tls/ca.crt") @@ -491,9 +491,9 @@ func ComputeAWFExcludeEnvVarNames(workflowData *WorkflowData, coreSecretVarNames } } - // GH_TOKEN when cli-proxy is enabled: the token is passed in the AWF step env for the + // GH_TOKEN when GitHub mode is gh-proxy: the token is passed in the AWF step env for the // host difc-proxy but must be excluded from the agent container. - if isFeatureEnabled(constants.CliProxyFeatureFlag, workflowData) { + if isGitHubCLIModeEnabled(workflowData) { addUnique("GH_TOKEN") } @@ -501,8 +501,8 @@ func ComputeAWFExcludeEnvVarNames(workflowData *WorkflowData, coreSecretVarNames return names } -// addCliProxyGHTokenToEnv adds GH_TOKEN to the AWF step environment when the -// cli-proxy feature is enabled. The token is NOT used by AWF or its cli-proxy +// addCliProxyGHTokenToEnv adds GH_TOKEN to the AWF step environment when GitHub +// mode is gh-proxy. The token is NOT used by AWF or its cli-proxy // sidecar directly — the host difc-proxy (started by start_cli_proxy.sh) already // has it. However, --env-all passes all step env vars into the agent container, // so we explicitly set GH_TOKEN here to ensure --exclude-env GH_TOKEN can @@ -515,7 +515,7 @@ func ComputeAWFExcludeEnvVarNames(workflowData *WorkflowData, coreSecretVarNames // template that is resolved at runtime by the GitHub Actions runner. func addCliProxyGHTokenToEnv(env map[string]string, workflowData *WorkflowData) { firewallConfig := getFirewallConfig(workflowData) - if isFeatureEnabled(constants.CliProxyFeatureFlag, workflowData) && + if isGitHubCLIModeEnabled(workflowData) && isFirewallEnabled(workflowData) && awfSupportsCliProxy(firewallConfig) && awfSupportsExcludeEnv(firewallConfig) { diff --git a/pkg/workflow/compiler_difc_proxy.go b/pkg/workflow/compiler_difc_proxy.go index a6592ebe8c7..c701efc63f1 100644 --- a/pkg/workflow/compiler_difc_proxy.go +++ b/pkg/workflow/compiler_difc_proxy.go @@ -408,14 +408,14 @@ func (c *Compiler) generateStopDIFCProxyStep(yaml *strings.Builder, data *Workfl // isCliProxyNeeded returns true if the CLI proxy should be started on the host. // // The CLI proxy is needed when: -// 1. The cli-proxy feature flag is enabled (explicitly or implicitly), and +// 1. tools.github.mode is set to gh-proxy (or legacy cli-proxy behavior is enabled), and // 2. The AWF sandbox (firewall) is enabled, and // 3. The AWF version supports CLI proxy flags // // The cli-proxy feature is implicitly enabled when integrity-reactions is enabled, // because reaction-based integrity decisions require the proxy to identify reaction authors. func isCliProxyNeeded(data *WorkflowData) bool { - cliProxyEnabled := isFeatureEnabled(constants.CliProxyFeatureFlag, data) + cliProxyEnabled := isGitHubCLIModeEnabled(data) integrityReactionsEnabled := isFeatureEnabled(constants.IntegrityReactionsFeatureFlag, data) if !cliProxyEnabled && !integrityReactionsEnabled { diff --git a/pkg/workflow/compiler_difc_proxy_test.go b/pkg/workflow/compiler_difc_proxy_test.go index 3f4ce331d44..bd11ff41a2f 100644 --- a/pkg/workflow/compiler_difc_proxy_test.go +++ b/pkg/workflow/compiler_difc_proxy_test.go @@ -1003,6 +1003,43 @@ func TestIsCliProxyNeeded_IntegrityReactionsImplicitEnable(t *testing.T) { expected: true, desc: "both flags together should enable the CLI proxy", }, + { + name: "tools.github.mode local overrides legacy cli-proxy feature", + data: &WorkflowData{ + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + Version: awfVersion, + }, + }, + Tools: map[string]any{ + "github": map[string]any{ + "mode": "local", + }, + }, + Features: map[string]any{"cli-proxy": true}, + }, + expected: false, + desc: "explicit tools.github.mode=local should disable cli proxy even when legacy feature is set", + }, + { + name: "tools.github.mode gh-proxy enables cli proxy without legacy feature", + data: &WorkflowData{ + NetworkPermissions: &NetworkPermissions{ + Firewall: &FirewallConfig{ + Enabled: true, + Version: awfVersion, + }, + }, + Tools: map[string]any{ + "github": map[string]any{ + "mode": "gh-proxy", + }, + }, + }, + expected: true, + desc: "explicit tools.github.mode=gh-proxy should enable cli proxy without legacy feature", + }, { name: "neither flag set", data: &WorkflowData{ diff --git a/pkg/workflow/github_mode_gh_proxy_integration_test.go b/pkg/workflow/github_mode_gh_proxy_integration_test.go new file mode 100644 index 00000000000..53a1205eb98 --- /dev/null +++ b/pkg/workflow/github_mode_gh_proxy_integration_test.go @@ -0,0 +1,49 @@ +//go:build integration + +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/github/gh-aw/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGitHubProxyModeIntegration(t *testing.T) { + tmpDir := testutil.TempDir(t, "github-mode-gh-proxy-workflow-file-test") + workflowFile := "../cli/workflows/test-copilot-gh-proxy.md" + + src, err := os.ReadFile(workflowFile) + require.NoError(t, err, "Failed to read workflow file %s", workflowFile) + + baseName := filepath.Base(workflowFile) + mdDst := filepath.Join(tmpDir, baseName) + require.NoError(t, os.WriteFile(mdDst, src, 0o600), + "Failed to write workflow file to temporary directory") + + compiler := NewCompiler() + err = compiler.CompileWorkflow(mdDst) + require.NoError(t, err, "Workflow file %s should compile successfully", workflowFile) + + lockName := strings.TrimSuffix(baseName, ".md") + ".lock.yml" + lockPath := filepath.Join(tmpDir, lockName) + compiledBytes, err := os.ReadFile(lockPath) + require.NoError(t, err) + compiled := string(compiledBytes) + + assert.Contains(t, compiled, "start_cli_proxy.sh", + "Compiled workflow should include host CLI proxy startup for tools.github.mode=gh-proxy") + assert.Contains(t, compiled, "stop_cli_proxy.sh", + "Compiled workflow should include host CLI proxy cleanup for tools.github.mode=gh-proxy") + assert.Contains(t, compiled, "cli_proxy_prompt.md", + "Compiled workflow should include CLI proxy guidance prompt for tools.github.mode=gh-proxy") + + assert.NotContains(t, compiled, "github_mcp_tools_prompt.md", + "Compiled workflow should not include GitHub MCP prompt guidance for tools.github.mode=gh-proxy") + assert.NotContains(t, compiled, "api.githubcopilot.com/mcp/", + "Compiled workflow should not register remote GitHub MCP server for tools.github.mode=gh-proxy") +} diff --git a/pkg/workflow/github_remote_mode_test.go b/pkg/workflow/github_remote_mode_test.go index 7d149875d38..e18562403ca 100644 --- a/pkg/workflow/github_remote_mode_test.go +++ b/pkg/workflow/github_remote_mode_test.go @@ -349,6 +349,43 @@ func TestGitHubRemoteModeHelperFunctions(t *testing.T) { } }) + t.Run("getGitHubType normalizes explicit type", func(t *testing.T) { + githubTool := map[string]any{ + "type": " Remote ", + "allowed": []string{"list_issues"}, + } + + githubType := getGitHubType(githubTool) + if githubType != "remote" { + t.Errorf("Expected normalized type 'remote', got '%s'", githubType) + } + }) + + t.Run("getGitHubType normalizes explicit local type", func(t *testing.T) { + githubTool := map[string]any{ + "type": " LoCaL ", + "allowed": []string{"list_issues"}, + } + + githubType := getGitHubType(githubTool) + if githubType != "local" { + t.Errorf("Expected normalized type 'local', got '%s'", githubType) + } + }) + + t.Run("getGitHubType falls back to local for invalid type and mode", func(t *testing.T) { + githubTool := map[string]any{ + "type": "invalid", + "mode": "unknown", + "allowed": []string{"list_issues"}, + } + + githubType := getGitHubType(githubTool) + if githubType != "local" { + t.Errorf("Expected fallback type 'local', got '%s'", githubType) + } + }) + t.Run("getGitHubToken extracts custom token correctly", func(t *testing.T) { githubTool := map[string]any{ "github-token": "${{ secrets.CUSTOM_PAT }}", diff --git a/pkg/workflow/importable_tools_test.go b/pkg/workflow/importable_tools_test.go index 1b2d4880e55..59f3438ced7 100644 --- a/pkg/workflow/importable_tools_test.go +++ b/pkg/workflow/importable_tools_test.go @@ -1002,3 +1002,60 @@ This workflow explicitly opts out of GitHub MCP tools. } } } + +// TestGitHubModeTopLevelOverridesImport verifies that tools.github.mode in the main +// workflow overrides imported tools.github.mode values. +func TestGitHubModeTopLevelOverridesImport(t *testing.T) { + tempDir := testutil.TempDir(t, "test-*") + + sharedPath := filepath.Join(tempDir, "shared.md") + sharedContent := `--- +tools: + github: + mode: gh-proxy +--- +` + if err := os.WriteFile(sharedPath, []byte(sharedContent), 0644); err != nil { + t.Fatalf("Failed to write shared file: %v", err) + } + + workflowPath := filepath.Join(tempDir, "main.md") + workflowContent := `--- +on: issues +engine: copilot +strict: false +permissions: + contents: read + issues: read +tools: + github: + mode: local +imports: + - shared.md +--- + +# Main +` + 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) + if strings.Contains(workflowData, "cli_proxy_prompt.md") { + t.Error("Expected top-level mode:local to win over imported mode:gh-proxy (cli_proxy_prompt.md should not be present)") + } + if !strings.Contains(workflowData, "github_mcp_tools_prompt.md") { + t.Error("Expected GitHub MCP prompt guidance when top-level mode is local") + } +} diff --git a/pkg/workflow/mcp_config_validation.go b/pkg/workflow/mcp_config_validation.go index 4a36f0ca2a9..0716b2b8e5e 100644 --- a/pkg/workflow/mcp_config_validation.go +++ b/pkg/workflow/mcp_config_validation.go @@ -156,29 +156,30 @@ func getRawMCPConfig(toolConfig map[string]any) (map[string]any, error) { // List of all known tool config fields (not just MCP) knownToolFields := map[string]bool{ - "type": true, - "url": true, - "command": true, - "container": true, - "env": true, - "headers": true, - "auth": true, // upstream OIDC authentication (HTTP servers only) - "version": true, - "args": true, - "entrypoint": true, - "entrypointArgs": true, - "mounts": true, - "proxy-args": true, - "registry": true, - "allowed": true, - "mode": true, // for github tool - "github-token": true, // for github tool - "read-only": true, // for github tool - "toolsets": true, // for github tool - "id": true, // for cache-memory (array notation) - "key": true, // for cache-memory - "description": true, // for cache-memory - "retention-days": true, // for cache-memory + "type": true, + "url": true, + "command": true, + "container": true, + "env": true, + "headers": true, + "auth": true, // upstream OIDC authentication (HTTP servers only) + "version": true, + "args": true, + "entrypoint": true, + "entrypointArgs": true, + "mounts": true, + "proxy-args": true, + "registry": true, + "allowed": true, + "mode": true, // for github tool: prompt/runtime mode (cli) or legacy MCP transport (local/remote) + "github-token": true, // for github tool + "read-only": true, // for github tool + "toolsets": true, // for github tool + "integrity-proxy": true, // for github tool + "id": true, // for cache-memory (array notation) + "key": true, // for cache-memory + "description": true, // for cache-memory + "retention-days": true, // for cache-memory } // Check new format: direct fields in tool config diff --git a/pkg/workflow/mcp_github_config.go b/pkg/workflow/mcp_github_config.go index 4764af24d45..437cbf16942 100644 --- a/pkg/workflow/mcp_github_config.go +++ b/pkg/workflow/mcp_github_config.go @@ -90,13 +90,69 @@ func hasGitHubApp(githubTool any) bool { return false } -// getGitHubType extracts the mode from GitHub tool configuration (local or remote) +// isGitHubCLIModeEnabled returns true when GitHub prompt/runtime mode is explicitly set +// to `tools.github.mode: gh-proxy`. If mode is explicitly set to `local` or `remote`, it +// takes precedence over the legacy features.cli-proxy flag (treated as MCP mode). +// When mode is not explicitly set, this returns the legacy `features.cli-proxy` flag +// value for backward compatibility. +func isGitHubCLIModeEnabled(data *WorkflowData) bool { + if data == nil { + return false + } + githubTool, hasGitHub := data.Tools["github"] + if hasGitHub && githubTool == false { + return false + } + if hasGitHub { + if toolConfig, ok := githubTool.(map[string]any); ok { + if modeSetting, exists := toolConfig["mode"]; exists { + if stringValue, ok := modeSetting.(string); ok { + switch modeValue := strings.ToLower(strings.TrimSpace(stringValue)); modeValue { + case "gh-proxy", "cli": + return true + case "local", "remote": + return false + default: + githubConfigLog.Printf("Unrecognized tools.github.mode value: %s, falling back to legacy behavior", modeValue) + } + } + } + } + } + return isFeatureEnabled(constants.CliProxyFeatureFlag, data) +} + +// normalizeGitHubType normalizes and validates GitHub MCP transport values. +// Supported values are `local` and `remote`. +func normalizeGitHubType(value string) (string, bool) { + normalizedValue := strings.ToLower(strings.TrimSpace(value)) + switch normalizedValue { + case "local", "remote": + return normalizedValue, true + default: + return "", false + } +} + +// getGitHubType extracts the MCP transport type from GitHub tool configuration +// (local or remote). Supports both `type` (preferred) and legacy `mode` values. func getGitHubType(githubTool any) string { if toolConfig, ok := githubTool.(map[string]any); ok { + if typeSetting, exists := toolConfig["type"]; exists { + if stringValue, ok := typeSetting.(string); ok { + if normalizedValue, valid := normalizeGitHubType(stringValue); valid { + githubConfigLog.Printf("GitHub MCP type set explicitly: %s", normalizedValue) + return normalizedValue + } + githubConfigLog.Printf("Unrecognized tools.github.type value: %q, falling back to default", stringValue) + } + } if modeSetting, exists := toolConfig["mode"]; exists { if stringValue, ok := modeSetting.(string); ok { - githubConfigLog.Printf("GitHub MCP mode set explicitly: %s", stringValue) - return stringValue + if normalizedValue, valid := normalizeGitHubType(stringValue); valid { + githubConfigLog.Printf("GitHub MCP type read from legacy mode field: %s", normalizedValue) + return normalizedValue + } } } } diff --git a/pkg/workflow/mcp_setup_generator.go b/pkg/workflow/mcp_setup_generator.go index b75eee3b1bd..41ef4b06015 100644 --- a/pkg/workflow/mcp_setup_generator.go +++ b/pkg/workflow/mcp_setup_generator.go @@ -91,10 +91,10 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, if toolValue == false { continue } - // When cli-proxy is enabled, agents use the pre-authenticated gh CLI for GitHub + // When GitHub mode is gh-proxy, agents use the pre-authenticated gh CLI for GitHub // reads instead of the GitHub MCP server. Skip so it is not configured with the gateway. - if toolName == "github" && isFeatureEnabled(constants.CliProxyFeatureFlag, workflowData) { - mcpSetupGeneratorLog.Print("Skipping GitHub MCP server registration: cli-proxy feature flag is enabled") + if toolName == "github" && isGitHubCLIModeEnabled(workflowData) { + mcpSetupGeneratorLog.Print("Skipping GitHub MCP server registration: tools.github.mode is gh-proxy") continue } // Standard MCP tools diff --git a/pkg/workflow/tools_parser.go b/pkg/workflow/tools_parser.go index 2ccf5ca62c7..73ade376ef8 100644 --- a/pkg/workflow/tools_parser.go +++ b/pkg/workflow/tools_parser.go @@ -221,6 +221,9 @@ func parseGitHubTool(val any) *GitHubToolConfig { if mode, ok := configMap["mode"].(string); ok { config.Mode = mode } + if mcpType, ok := configMap["type"].(string); ok { + config.Type = mcpType + } if version, ok := configMap["version"].(string); ok { config.Version = version diff --git a/pkg/workflow/tools_types.go b/pkg/workflow/tools_types.go index 857672b6da6..b54dfc17d46 100644 --- a/pkg/workflow/tools_types.go +++ b/pkg/workflow/tools_types.go @@ -291,6 +291,7 @@ type GitHubReposScope any // string or []any (YAML-parsed arrays are []any) type GitHubToolConfig struct { Allowed GitHubAllowedTools `yaml:"allowed,omitempty"` Mode string `yaml:"mode,omitempty"` + Type string `yaml:"type,omitempty"` Version string `yaml:"version,omitempty"` Args []string `yaml:"args,omitempty"` ReadOnly bool `yaml:"read-only,omitempty"` diff --git a/pkg/workflow/unified_prompt_step.go b/pkg/workflow/unified_prompt_step.go index 6ea9eb250d6..c087e3a146f 100644 --- a/pkg/workflow/unified_prompt_step.go +++ b/pkg/workflow/unified_prompt_step.go @@ -233,10 +233,10 @@ func (c *Compiler) collectPromptSections(data *WorkflowData) []PromptSection { // 10. GitHub tool-use guidance: directs the model to the correct mechanism for // GitHub reads (and writes when safe-outputs is also enabled). - // When cli-proxy is enabled, the agent uses the pre-authenticated gh CLI for reads + // When GitHub mode is gh-proxy, the agent uses the pre-authenticated gh CLI for reads // instead of a GitHub MCP server (which is not registered). Otherwise, the GitHub // MCP server is used for reads. - if isFeatureEnabled(constants.CliProxyFeatureFlag, data) { + if isGitHubCLIModeEnabled(data) { unifiedPromptLog.Print("Adding cli-proxy tool-use guidance (gh CLI for reads, no GitHub MCP server)") cliProxyFile := cliProxyPromptFile if HasSafeOutputsEnabled(data.SafeOutputs) { diff --git a/pkg/workflow/unified_prompt_step_test.go b/pkg/workflow/unified_prompt_step_test.go index 45a584b32ac..dea1b3e779c 100644 --- a/pkg/workflow/unified_prompt_step_test.go +++ b/pkg/workflow/unified_prompt_step_test.go @@ -410,6 +410,30 @@ func TestCollectPromptSections_CliProxy(t *testing.T) { } }) + t.Run("tools.github.mode gh-proxy uses cli_proxy_prompt without legacy feature flag", func(t *testing.T) { + compiler := &Compiler{} + + data := &WorkflowData{ + Tools: map[string]any{ + "github": map[string]any{"mode": "gh-proxy"}, + }, + ParsedTools: NewTools(map[string]any{"github": map[string]any{"mode": "gh-proxy"}}), + SafeOutputs: nil, + } + + sections := compiler.collectPromptSections(data) + require.NotEmpty(t, sections, "Should collect sections") + + var cliProxySection *PromptSection + for i := range sections { + if sections[i].IsFile && sections[i].Content == cliProxyPromptFile { + cliProxySection = §ions[i] + break + } + } + require.NotNil(t, cliProxySection, "Should include cli_proxy_prompt.md when tools.github.mode is gh-proxy") + }) + t.Run("cli-proxy enabled with safe-outputs uses cli_proxy_with_safeoutputs_prompt", func(t *testing.T) { compiler := &Compiler{}