From 4d97e26718ce686ba7eb134877114bab31b018e4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 05:43:38 +0000 Subject: [PATCH 1/5] Initial plan From adddbb5042c8203cb0ba28953b12616bbc3a7b4c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 06:16:15 +0000 Subject: [PATCH 2/5] fix: pass GITHUB_REPOSITORY as --repo in MCP logs and audit tools to avoid git dependency The gh aw logs MCP tool failed in audit-workflows agent environment with: 'unable to find git executable in PATH' This happened because gh run list (called internally) needs to determine the base repository from git when no --repo flag is provided. Fix: In registerLogsTool and registerAuditTool, read GITHUB_REPOSITORY env var (already passed to the MCP server container via env_vars config) and append --repo to the spawned subprocess arguments. Also adds unit tests using mcp.NewInMemoryTransports() to verify the flag is correctly appended/omitted based on the environment variable. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7436e648-aeeb-4b58-8bf6-1d4a3adf15e7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/mcp_tools_privileged.go | 17 +++ pkg/cli/mcp_tools_privileged_test.go | 152 +++++++++++++++++++++++++++ 2 files changed, 169 insertions(+) diff --git a/pkg/cli/mcp_tools_privileged.go b/pkg/cli/mcp_tools_privileged.go index 60cf91c48d8..5f4377a6e24 100644 --- a/pkg/cli/mcp_tools_privileged.go +++ b/pkg/cli/mcp_tools_privileged.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "os" "os/exec" "strconv" "strings" @@ -156,6 +157,14 @@ from where the previous request stopped due to timeout.`, cmdArgs = append(cmdArgs, "--artifacts", strings.Join(args.Artifacts, ",")) } + // Pass --repo from GITHUB_REPOSITORY to avoid git dependency in restricted environments + // (e.g., MCP server containers where git is not installed). + // GITHUB_REPOSITORY is set as an env_var in the agentic-workflows MCP server configuration + // and is passed through to subprocesses spawned by the MCP server. + if repo := os.Getenv("GITHUB_REPOSITORY"); repo != "" { + cmdArgs = append(cmdArgs, "--repo", repo) + } + // Set timeout to 1 minute for MCP server if not explicitly specified timeoutValue := args.Timeout if timeoutValue == 0 { @@ -301,6 +310,14 @@ Returns JSON with the following structure: cmdArgs = append(cmdArgs, "--artifacts", strings.Join(args.Artifacts, ",")) } + // Pass --repo from GITHUB_REPOSITORY to avoid git dependency in restricted environments + // (e.g., MCP server containers where git is not installed). + // Only needed when the input is a bare numeric run ID (not a URL with embedded owner/repo), + // but passing it is safe since the audit command ignores --repo when owner/repo is in the URL. + if repo := os.Getenv("GITHUB_REPOSITORY"); repo != "" { + cmdArgs = append(cmdArgs, "--repo", repo) + } + // Execute the CLI command // Use separate stdout/stderr capture instead of CombinedOutput because: // - Stdout contains JSON output (--json flag) diff --git a/pkg/cli/mcp_tools_privileged_test.go b/pkg/cli/mcp_tools_privileged_test.go index ae4977122f7..8bc828125d5 100644 --- a/pkg/cli/mcp_tools_privileged_test.go +++ b/pkg/cli/mcp_tools_privileged_test.go @@ -3,9 +3,13 @@ package cli import ( + "context" + "os/exec" "testing" + "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestExtractLastConsoleMessage verifies that extractLastConsoleMessage correctly @@ -95,3 +99,151 @@ workflow:foo Cleanup +50ms`, }) } } + +// connectInMemory creates an in-memory MCP client-server connection for testing. +// The caller is responsible for closing the returned session. +func connectInMemory(t *testing.T, server *mcp.Server) *mcp.ClientSession { + t.Helper() + ctx := context.Background() + t1, t2 := mcp.NewInMemoryTransports() + _, err := server.Connect(ctx, t1, nil) + require.NoError(t, err, "server.Connect should succeed") + client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "1.0"}, nil) + session, err := client.Connect(ctx, t2, nil) + require.NoError(t, err, "client.Connect should succeed") + t.Cleanup(func() { session.Close() }) + return session +} + +// TestLogsToolPassesGithubRepositoryAsRepoFlag verifies that the logs MCP tool +// appends --repo to the subprocess command when GITHUB_REPOSITORY +// is set, allowing gh run list to work in environments without git installed. +func TestLogsToolPassesGithubRepositoryAsRepoFlag(t *testing.T) { + tests := []struct { + name string + githubRepository string + wantRepoFlag bool + }{ + { + name: "passes --repo when GITHUB_REPOSITORY is set", + githubRepository: "github/gh-aw", + wantRepoFlag: true, + }, + { + name: "omits --repo when GITHUB_REPOSITORY is empty", + githubRepository: "", + wantRepoFlag: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("GITHUB_REPOSITORY", tt.githubRepository) + + var capturedArgs []string + mockExecCmd := func(ctx context.Context, args ...string) *exec.Cmd { + capturedArgs = append([]string(nil), args...) + // Return a command that exits with code 1 so the handler returns an + // error without writing any files, keeping the test self-contained. + return exec.CommandContext(ctx, "false") + } + + server := mcp.NewServer(&mcp.Implementation{Name: "test", Version: "1.0"}, nil) + err := registerLogsTool(server, mockExecCmd, "", false) + require.NoError(t, err, "registerLogsTool should succeed") + + session := connectInMemory(t, server) + + // Call the tool — it will fail because the mock command exits with code 1, + // but we only care about the captured args. + ctx := context.Background() + _, _ = session.CallTool(ctx, &mcp.CallToolParams{ + Name: "logs", + Arguments: map[string]any{}, + }) + + require.NotNil(t, capturedArgs, "execCmd should have been called") + + // Locate --repo flag in captured args + var repoValue string + for i, arg := range capturedArgs { + if arg == "--repo" && i+1 < len(capturedArgs) { + repoValue = capturedArgs[i+1] + break + } + } + + if tt.wantRepoFlag { + assert.Equal(t, tt.githubRepository, repoValue, + "--repo flag should be set to GITHUB_REPOSITORY value; args: %v", capturedArgs) + } else { + assert.Empty(t, repoValue, + "--repo flag should not be present when GITHUB_REPOSITORY is empty; args: %v", capturedArgs) + } + }) + } +} + +// TestAuditToolPassesGithubRepositoryAsRepoFlag verifies that the audit MCP tool +// appends --repo to the subprocess command when GITHUB_REPOSITORY +// is set, allowing the audit command to resolve the repository without git. +func TestAuditToolPassesGithubRepositoryAsRepoFlag(t *testing.T) { + tests := []struct { + name string + githubRepository string + wantRepoFlag bool + }{ + { + name: "passes --repo when GITHUB_REPOSITORY is set", + githubRepository: "github/gh-aw", + wantRepoFlag: true, + }, + { + name: "omits --repo when GITHUB_REPOSITORY is empty", + githubRepository: "", + wantRepoFlag: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("GITHUB_REPOSITORY", tt.githubRepository) + + var capturedArgs []string + mockExecCmd := func(ctx context.Context, args ...string) *exec.Cmd { + capturedArgs = append([]string(nil), args...) + return exec.CommandContext(ctx, "false") + } + + server := mcp.NewServer(&mcp.Implementation{Name: "test", Version: "1.0"}, nil) + err := registerAuditTool(server, mockExecCmd, "", false) + require.NoError(t, err, "registerAuditTool should succeed") + + session := connectInMemory(t, server) + + ctx := context.Background() + _, _ = session.CallTool(ctx, &mcp.CallToolParams{ + Name: "audit", + Arguments: map[string]any{"run_id_or_url": "1234567890"}, + }) + + require.NotNil(t, capturedArgs, "execCmd should have been called") + + var repoValue string + for i, arg := range capturedArgs { + if arg == "--repo" && i+1 < len(capturedArgs) { + repoValue = capturedArgs[i+1] + break + } + } + + if tt.wantRepoFlag { + assert.Equal(t, tt.githubRepository, repoValue, + "--repo flag should be set to GITHUB_REPOSITORY value; args: %v", capturedArgs) + } else { + assert.Empty(t, repoValue, + "--repo flag should not be present when GITHUB_REPOSITORY is empty; args: %v", capturedArgs) + } + }) + } +} From 99b5896ded2ac03687089bd9750051047483c47f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 06:19:23 +0000 Subject: [PATCH 3/5] fix: use portable mock command in privileged tool tests Replace Unix-specific 'false' command with a non-existent command name that fails portably on all platforms. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7436e648-aeeb-4b58-8bf6-1d4a3adf15e7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/mcp_tools_privileged_test.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/cli/mcp_tools_privileged_test.go b/pkg/cli/mcp_tools_privileged_test.go index 8bc828125d5..f7576b1a714 100644 --- a/pkg/cli/mcp_tools_privileged_test.go +++ b/pkg/cli/mcp_tools_privileged_test.go @@ -143,9 +143,11 @@ func TestLogsToolPassesGithubRepositoryAsRepoFlag(t *testing.T) { var capturedArgs []string mockExecCmd := func(ctx context.Context, args ...string) *exec.Cmd { capturedArgs = append([]string(nil), args...) - // Return a command that exits with code 1 so the handler returns an - // error without writing any files, keeping the test self-contained. - return exec.CommandContext(ctx, "false") + // Use a non-existent command so the subprocess fails on all platforms + // without depending on Unix-specific commands like "false". + // cmd.Output() will return a "executable file not found" error, which + // the handler treats as a failure — we only care about the captured args. + return exec.CommandContext(ctx, "nonexistent-command-for-testing-only") } server := mcp.NewServer(&mcp.Implementation{Name: "test", Version: "1.0"}, nil) @@ -154,7 +156,7 @@ func TestLogsToolPassesGithubRepositoryAsRepoFlag(t *testing.T) { session := connectInMemory(t, server) - // Call the tool — it will fail because the mock command exits with code 1, + // Call the tool — it will fail because the mock command is not found, // but we only care about the captured args. ctx := context.Background() _, _ = session.CallTool(ctx, &mcp.CallToolParams{ @@ -212,7 +214,9 @@ func TestAuditToolPassesGithubRepositoryAsRepoFlag(t *testing.T) { var capturedArgs []string mockExecCmd := func(ctx context.Context, args ...string) *exec.Cmd { capturedArgs = append([]string(nil), args...) - return exec.CommandContext(ctx, "false") + // Use a non-existent command so the subprocess fails on all platforms + // without depending on Unix-specific commands like "false". + return exec.CommandContext(ctx, "nonexistent-command-for-testing-only") } server := mcp.NewServer(&mcp.Implementation{Name: "test", Version: "1.0"}, nil) From f4b91938e0563f8f8c67063837dc0836ed4e71f2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 15 Apr 2026 06:29:48 +0000 Subject: [PATCH 4/5] docs(adr): add draft ADR-26377 for GITHUB_REPOSITORY env var MCP repo resolution Co-Authored-By: Claude Sonnet 4.6 --- ...-var-for-mcp-subprocess-repo-resolution.md | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 docs/adr/26377-use-github-repository-env-var-for-mcp-subprocess-repo-resolution.md diff --git a/docs/adr/26377-use-github-repository-env-var-for-mcp-subprocess-repo-resolution.md b/docs/adr/26377-use-github-repository-env-var-for-mcp-subprocess-repo-resolution.md new file mode 100644 index 00000000000..aebb526ce57 --- /dev/null +++ b/docs/adr/26377-use-github-repository-env-var-for-mcp-subprocess-repo-resolution.md @@ -0,0 +1,78 @@ +# ADR-26377: Use GITHUB_REPOSITORY Env Var for MCP Subprocess Repo Resolution + +**Date**: 2026-04-15 +**Status**: Draft +**Deciders**: pelikhan, Copilot + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +The `logs` and `audit` MCP tools registered by `registerLogsTool` and `registerAuditTool` in `pkg/cli/mcp_tools_privileged.go` spawn subprocesses that call `gh run list` and related commands. When `--repo` is not explicitly supplied, `gh` falls back to git to auto-detect the repository from the local working directory. MCP server containers in the agentic-workflows sandbox do not have git installed, causing these tool invocations to fail with `unable to find git executable in PATH`. The `GITHUB_REPOSITORY` environment variable (`owner/repo` format) is already forwarded to the MCP server container via `env_vars` in the agentic-workflows configuration, making it a zero-cost source of truth for the target repository. + +### Decision + +We will conditionally append `--repo ` to the subprocess argument list of the `logs` and `audit` MCP tools whenever the `GITHUB_REPOSITORY` environment variable is non-empty. This allows `gh` to resolve the repository without invoking git, making the tools functional in sandboxed environments that lack a git binary. The injection is conditional so that the tools continue to behave identically in local development environments where `GITHUB_REPOSITORY` is typically unset. + +### Alternatives Considered + +#### Alternative 1: Install git in the MCP server container + +Git could be added as a dependency of the MCP server container image so that `gh`'s auto-detection works as designed. This was rejected because it adds a heavyweight, security-relevant binary to a minimal sandbox container for the sole purpose of supporting a fallback path in `gh`; it also increases the container attack surface and image size without addressing the root cause. + +#### Alternative 2: Add an explicit `repo` parameter to the MCP tool schema + +The `logs` and `audit` tool schemas could expose a `repo` parameter that callers must supply. This was rejected because it breaks backward compatibility for every existing caller and shifts repository-resolution responsibility to the AI agent (which already has `GITHUB_REPOSITORY` available implicitly). The env-var approach achieves the same result transparently. + +#### Alternative 3: Always require `--repo` by reading from configuration at server startup + +The MCP server startup code could resolve the repository once at initialization time and bake it into the tool handlers as a closure. This was not chosen because it complicates the server initialization path and makes unit-testing harder; the conditional env-var read at call time is simpler and equally correct. + +### Consequences + +#### Positive +- MCP tools work correctly in git-less sandbox containers without any changes to caller code or tool schemas. +- `GITHUB_REPOSITORY` is a standard GitHub Actions environment variable, so the fix is idiomatic and requires no new infrastructure. +- The change is backward-compatible: when `GITHUB_REPOSITORY` is empty, behavior is unchanged. + +#### Negative +- The tools now have an implicit, undocumented dependency on the `GITHUB_REPOSITORY` environment variable; callers in non-Actions environments must be aware that omitting this variable forces git-based fallback. +- If `GITHUB_REPOSITORY` is set incorrectly (e.g., wrong value injected), it will be silently propagated to the subprocess without validation. + +#### Neutral +- Two new integration-style unit tests (`TestLogsToolPassesGithubRepositoryAsRepoFlag`, `TestAuditToolPassesGithubRepositoryAsRepoFlag`) use `mcp.NewInMemoryTransports()` and a mock `execCmd` to capture subprocess arguments, establishing a test pattern for future MCP tool behavior verification. +- The same fix is applied symmetrically to both the `logs` and `audit` tool handlers; any future MCP tools that spawn `gh` subprocesses should follow the same pattern. + +--- + +## 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). + +### Repository Resolution in MCP Tool Subprocess Calls + +1. MCP tool handlers that spawn `gh` subprocesses **MUST** append `--repo ` to the subprocess argument list when the `GITHUB_REPOSITORY` environment variable is non-empty. +2. MCP tool handlers **MUST NOT** append `--repo` when `GITHUB_REPOSITORY` is empty or unset, preserving existing git-based fallback behavior. +3. The value appended as `--repo` **MUST** be the verbatim value of `GITHUB_REPOSITORY` (in `owner/repo` format) without modification or validation. +4. Implementations **MUST NOT** install or invoke `git` solely to work around the absence of the `--repo` flag; the env-var injection pattern is the approved resolution mechanism for sandboxed environments. + +### Environment Variable Contract + +1. The `GITHUB_REPOSITORY` environment variable **MUST** be forwarded to MCP server containers via the agentic-workflows `env_vars` configuration when the server is deployed in a GitHub Actions sandbox. +2. Callers running MCP tools outside of GitHub Actions **SHOULD** set `GITHUB_REPOSITORY` explicitly if git is unavailable in the execution environment. +3. Implementations **MAY** log a debug-level message when `GITHUB_REPOSITORY` is used for `--repo` injection to aid in diagnosing unexpected repository resolution. + +### Testing + +1. Every MCP tool handler that conditionally injects `--repo` **MUST** have a unit test that verifies the flag is appended when `GITHUB_REPOSITORY` is set and omitted when it is empty. +2. Tests **SHOULD** use `mcp.NewInMemoryTransports()` with a mock `execCmd` to capture subprocess arguments without spawning a real `gh` process. + +### 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/24439752420) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* From 53ca779f991247cb7304003da56b100b8d8e348f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 12:52:12 +0000 Subject: [PATCH 5/5] refactor: extract appendRepoFlagFromEnv helper and fix connectInMemory comment - Extract appendRepoFlagFromEnv() to eliminate duplication between logs and audit tools (addresses PR review feedback) - Fix connectInMemory() comment to say the session is closed via t.Cleanup rather than incorrectly saying the caller is responsible Agent-Logs-Url: https://github.com/github/gh-aw/sessions/fe7378b9-10c8-45ac-9281-7022f836a06b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/mcp_tools_privileged.go | 30 ++++++++++++++-------------- pkg/cli/mcp_tools_privileged_test.go | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/cli/mcp_tools_privileged.go b/pkg/cli/mcp_tools_privileged.go index 5f4377a6e24..cb9f0759ee3 100644 --- a/pkg/cli/mcp_tools_privileged.go +++ b/pkg/cli/mcp_tools_privileged.go @@ -13,7 +13,19 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" ) -// registerLogsTool registers the logs tool with the MCP server. +// appendRepoFlagFromEnv appends "--repo " to args when GITHUB_REPOSITORY +// is set in the environment. This allows gh CLI subcommands to identify the repository +// without falling back to git-based detection, which fails in sandboxed environments +// (e.g., MCP server containers where git is not installed). +// GITHUB_REPOSITORY is forwarded to the agentic-workflows MCP server container via +// env_vars in the MCP configuration and inherited by spawned subprocesses. +func appendRepoFlagFromEnv(args []string) []string { + if repo := os.Getenv("GITHUB_REPOSITORY"); repo != "" { + return append(args, "--repo", repo) + } + return args +} + // The logs tool requires write+ access and checks actor permissions. // Returns an error if schema generation fails. func registerLogsTool(server *mcp.Server, execCmd execCmdFunc, actor string, validateActor bool) error { @@ -157,13 +169,7 @@ from where the previous request stopped due to timeout.`, cmdArgs = append(cmdArgs, "--artifacts", strings.Join(args.Artifacts, ",")) } - // Pass --repo from GITHUB_REPOSITORY to avoid git dependency in restricted environments - // (e.g., MCP server containers where git is not installed). - // GITHUB_REPOSITORY is set as an env_var in the agentic-workflows MCP server configuration - // and is passed through to subprocesses spawned by the MCP server. - if repo := os.Getenv("GITHUB_REPOSITORY"); repo != "" { - cmdArgs = append(cmdArgs, "--repo", repo) - } + cmdArgs = appendRepoFlagFromEnv(cmdArgs) // Set timeout to 1 minute for MCP server if not explicitly specified timeoutValue := args.Timeout @@ -310,13 +316,7 @@ Returns JSON with the following structure: cmdArgs = append(cmdArgs, "--artifacts", strings.Join(args.Artifacts, ",")) } - // Pass --repo from GITHUB_REPOSITORY to avoid git dependency in restricted environments - // (e.g., MCP server containers where git is not installed). - // Only needed when the input is a bare numeric run ID (not a URL with embedded owner/repo), - // but passing it is safe since the audit command ignores --repo when owner/repo is in the URL. - if repo := os.Getenv("GITHUB_REPOSITORY"); repo != "" { - cmdArgs = append(cmdArgs, "--repo", repo) - } + cmdArgs = appendRepoFlagFromEnv(cmdArgs) // Execute the CLI command // Use separate stdout/stderr capture instead of CombinedOutput because: diff --git a/pkg/cli/mcp_tools_privileged_test.go b/pkg/cli/mcp_tools_privileged_test.go index f7576b1a714..c9fbca10913 100644 --- a/pkg/cli/mcp_tools_privileged_test.go +++ b/pkg/cli/mcp_tools_privileged_test.go @@ -101,7 +101,7 @@ workflow:foo Cleanup +50ms`, } // connectInMemory creates an in-memory MCP client-server connection for testing. -// The caller is responsible for closing the returned session. +// The session is closed automatically when the test ends via t.Cleanup. func connectInMemory(t *testing.T, server *mcp.Server) *mcp.ClientSession { t.Helper() ctx := context.Background()