Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,10 @@ const DefaultMCPGatewayVersion Version = "v0.0.103"
// DefaultMCPGatewayContainer is the default container image for the MCP Gateway
const DefaultMCPGatewayContainer = "ghcr.io/github/gh-aw-mcpg"

// DefaultMCPGatewayPayloadDir is the default directory for MCP gateway payload files
// This directory is shared between the agent container and MCP gateway for large payload exchange
const DefaultMCPGatewayPayloadDir = "/tmp/gh-aw/mcp-payloads"

// DefaultFirewallRegistry is the container image registry for AWF (gh-aw-firewall) Docker images
const DefaultFirewallRegistry = "ghcr.io/github/gh-aw-firewall"

Expand Down
3 changes: 2 additions & 1 deletion pkg/workflow/codex_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,8 @@ func TestCodexEngineRenderMCPConfig(t *testing.T) {
"\"gateway\": {",
"\"port\": $MCP_GATEWAY_PORT,",
"\"domain\": \"${MCP_GATEWAY_DOMAIN}\",",
"\"apiKey\": \"${MCP_GATEWAY_API_KEY}\"",
"\"apiKey\": \"${MCP_GATEWAY_API_KEY}\",",
"\"payloadDir\": \"${MCP_GATEWAY_PAYLOAD_DIR}\"",
"}",
"}",
"MCPCONFIG_EOF",
Expand Down
13 changes: 10 additions & 3 deletions pkg/workflow/mcp_gateway_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ func ensureDefaultMCPGatewayConfig(workflowData *WorkflowData) {
"${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}:rw",
}
}

// Ensure default payloadDir is set if not provided
if workflowData.SandboxConfig.MCP.PayloadDir == "" {
mcpGatewayConfigLog.Print("Setting default gateway payloadDir")
workflowData.SandboxConfig.MCP.PayloadDir = constants.DefaultMCPGatewayPayloadDir
}
}

// buildMCPGatewayConfig builds the gateway configuration for inclusion in MCP config files
Expand All @@ -117,9 +123,10 @@ func buildMCPGatewayConfig(workflowData *WorkflowData) *MCPGatewayRuntimeConfig
// Use ${...} syntax for environment variable references that will be resolved by the gateway at runtime
// Per MCP Gateway Specification v1.0.0 section 4.2, variable expressions use "${VARIABLE_NAME}" syntax
return &MCPGatewayRuntimeConfig{
Port: int(DefaultMCPGatewayPort), // Will be formatted as "${MCP_GATEWAY_PORT}" in renderer
Domain: "${MCP_GATEWAY_DOMAIN}", // Gateway variable expression
APIKey: "${MCP_GATEWAY_API_KEY}", // Gateway variable expression
Port: int(DefaultMCPGatewayPort), // Will be formatted as "${MCP_GATEWAY_PORT}" in renderer
Domain: "${MCP_GATEWAY_DOMAIN}", // Gateway variable expression
APIKey: "${MCP_GATEWAY_API_KEY}", // Gateway variable expression
PayloadDir: "${MCP_GATEWAY_PAYLOAD_DIR}", // Gateway variable expression for payload directory
}
}

Expand Down
47 changes: 41 additions & 6 deletions pkg/workflow/mcp_gateway_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func TestEnsureDefaultMCPGatewayConfig(t *testing.T) {
assert.Equal(t, constants.DefaultMCPGatewayContainer, wd.SandboxConfig.MCP.Container, "Container should be default")
assert.Equal(t, string(constants.DefaultMCPGatewayVersion), wd.SandboxConfig.MCP.Version, "Version should be default")
assert.Equal(t, int(DefaultMCPGatewayPort), wd.SandboxConfig.MCP.Port, "Port should be default")
assert.Equal(t, constants.DefaultMCPGatewayPayloadDir, wd.SandboxConfig.MCP.PayloadDir, "PayloadDir should be default")
assert.Len(t, wd.SandboxConfig.MCP.Mounts, 3, "Should have 3 default mounts")
},
Comment on lines +35 to 37
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage: The test verifies that the gateway configuration has the PayloadDir set to the default value, but it doesn't verify that the agent sandbox configuration also receives the payloadDir mount with read-only (:ro) permissions. According to the PR description, the agent should have a read-only mount of the payloadDir added to AgentSandboxConfig.Mounts.

Add assertions to verify:

  1. workflowData.SandboxConfig.Agent.Mounts contains the payloadDir mount with ":ro" permissions
  2. The mount format is correct: "/tmp/gh-aw/mcp-payloads:/tmp/gh-aw/mcp-payloads:ro"

Copilot uses AI. Check for mistakes.
},
Expand Down Expand Up @@ -133,6 +134,37 @@ func TestEnsureDefaultMCPGatewayConfig(t *testing.T) {
assert.Equal(t, "/custom:/mount:ro", wd.SandboxConfig.MCP.Mounts[0], "Custom mount should be preserved")
},
},
{
name: "fills in missing payloadDir field",
workflowData: &WorkflowData{
SandboxConfig: &SandboxConfig{
MCP: &MCPGatewayRuntimeConfig{
Container: "custom-container",
Version: "v1.0.0",
Port: 8080,
},
},
},
validate: func(t *testing.T, wd *WorkflowData) {
assert.Equal(t, constants.DefaultMCPGatewayPayloadDir, wd.SandboxConfig.MCP.PayloadDir, "PayloadDir should be filled with default")
},
},
{
name: "preserves custom payloadDir",
workflowData: &WorkflowData{
SandboxConfig: &SandboxConfig{
MCP: &MCPGatewayRuntimeConfig{
Container: "custom-container",
Version: "v1.0.0",
Port: 8080,
PayloadDir: "/custom/payloads",
},
},
},
validate: func(t *testing.T, wd *WorkflowData) {
assert.Equal(t, "/custom/payloads", wd.SandboxConfig.MCP.PayloadDir, "Custom payloadDir should be preserved")
},
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -169,9 +201,10 @@ func TestBuildMCPGatewayConfig(t *testing.T) {
name: "creates default gateway config",
workflowData: &WorkflowData{},
expected: &MCPGatewayRuntimeConfig{
Port: int(DefaultMCPGatewayPort),
Domain: "${MCP_GATEWAY_DOMAIN}",
APIKey: "${MCP_GATEWAY_API_KEY}",
Port: int(DefaultMCPGatewayPort),
Domain: "${MCP_GATEWAY_DOMAIN}",
APIKey: "${MCP_GATEWAY_API_KEY}",
PayloadDir: "${MCP_GATEWAY_PAYLOAD_DIR}",
},
},
{
Expand All @@ -184,9 +217,10 @@ func TestBuildMCPGatewayConfig(t *testing.T) {
},
},
expected: &MCPGatewayRuntimeConfig{
Port: int(DefaultMCPGatewayPort),
Domain: "${MCP_GATEWAY_DOMAIN}",
APIKey: "${MCP_GATEWAY_API_KEY}",
Port: int(DefaultMCPGatewayPort),
Domain: "${MCP_GATEWAY_DOMAIN}",
APIKey: "${MCP_GATEWAY_API_KEY}",
PayloadDir: "${MCP_GATEWAY_PAYLOAD_DIR}",
},
},
}
Expand All @@ -201,6 +235,7 @@ func TestBuildMCPGatewayConfig(t *testing.T) {
assert.Equal(t, tt.expected.Port, result.Port, "Port should match")
assert.Equal(t, tt.expected.Domain, result.Domain, "Domain should match")
assert.Equal(t, tt.expected.APIKey, result.APIKey, "APIKey should match")
assert.Equal(t, tt.expected.PayloadDir, result.PayloadDir, "PayloadDir should match")
}
})
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/workflow/mcp_renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,13 @@ func RenderJSONMCPConfig(
// Port as unquoted variable - shell expands to integer (e.g., 8080) for valid JSON
fmt.Fprintf(&configBuilder, " \"port\": $MCP_GATEWAY_PORT,\n")
fmt.Fprintf(&configBuilder, " \"domain\": \"%s\",\n", options.GatewayConfig.Domain)
fmt.Fprintf(&configBuilder, " \"apiKey\": \"%s\"\n", options.GatewayConfig.APIKey)
fmt.Fprintf(&configBuilder, " \"apiKey\": \"%s\"", options.GatewayConfig.APIKey)
// Add payloadDir if specified
if options.GatewayConfig.PayloadDir != "" {
fmt.Fprintf(&configBuilder, ",\n \"payloadDir\": \"%s\"\n", options.GatewayConfig.PayloadDir)
} else {
configBuilder.WriteString("\n")
}
configBuilder.WriteString(" }\n")
} else {
configBuilder.WriteString(" }\n")
Expand Down
18 changes: 17 additions & 1 deletion pkg/workflow/mcp_setup_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,15 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any,
} else {
yaml.WriteString(" export MCP_GATEWAY_API_KEY=\"" + apiKey + "\"\n")
}

// Export payload directory and ensure it exists
payloadDir := gatewayConfig.PayloadDir
if payloadDir == "" {
payloadDir = constants.DefaultMCPGatewayPayloadDir
}
yaml.WriteString(" export MCP_GATEWAY_PAYLOAD_DIR=\"" + payloadDir + "\"\n")
yaml.WriteString(" mkdir -p \"${MCP_GATEWAY_PAYLOAD_DIR}\"\n")

yaml.WriteString(" export DEBUG=\"*\"\n")
yaml.WriteString(" \n")
yaml.WriteString(" # Register API key as secret to mask it from logs\n")
Expand Down Expand Up @@ -544,6 +553,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any,
containerCmd += " -e MCP_GATEWAY_PORT"
containerCmd += " -e MCP_GATEWAY_DOMAIN"
containerCmd += " -e MCP_GATEWAY_API_KEY"
containerCmd += " -e MCP_GATEWAY_PAYLOAD_DIR"
containerCmd += " -e DEBUG"
// Pass environment variables that MCP servers reference in their config
// These are needed because awmg v0.0.12+ validates and resolves ${VAR} patterns at config load time
Expand Down Expand Up @@ -624,7 +634,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any,

// Mark standard environment variables as already added
standardEnvVars := []string{
"MCP_GATEWAY_PORT", "MCP_GATEWAY_DOMAIN", "MCP_GATEWAY_API_KEY", "DEBUG",
"MCP_GATEWAY_PORT", "MCP_GATEWAY_DOMAIN", "MCP_GATEWAY_API_KEY", "MCP_GATEWAY_PAYLOAD_DIR", "DEBUG",
"MCP_GATEWAY_LOG_DIR", "GH_AW_MCP_LOG_DIR", "GH_AW_SAFE_OUTPUTS",
"GH_AW_SAFE_OUTPUTS_CONFIG_PATH", "GH_AW_SAFE_OUTPUTS_TOOLS_PATH",
"GH_AW_ASSETS_BRANCH", "GH_AW_ASSETS_MAX_SIZE_KB", "GH_AW_ASSETS_ALLOWED_EXTS",
Expand Down Expand Up @@ -679,6 +689,12 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any,
}

// Add volume mounts
// First, add the payload directory mount (rw for both agent and gateway)
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect comment: The comment states "rw for both agent and gateway", but this is misleading. According to the PR description and repository conventions, the agent container should have read-only (:ro) access while the gateway container should have read-write (:rw) access to the payloadDir.

Additionally, this code only adds the mount to the gateway container. There is no corresponding code that adds the mount to the agent container (AgentSandboxConfig.Mounts).

The comment should be corrected to reflect the actual behavior, or better yet, the missing agent mount functionality should be implemented.

This issue also appears in the following locations of the same file:

  • line 691
  • line 692
Suggested change
// First, add the payload directory mount (rw for both agent and gateway)
// First, add the payload directory mount for the gateway container with read-write access

Copilot uses AI. Check for mistakes.
if payloadDir != "" {
containerCmd += " -v " + payloadDir + ":" + payloadDir + ":rw"
}

// Then add user-configured mounts
if len(gatewayConfig.Mounts) > 0 {
for _, mount := range gatewayConfig.Mounts {
containerCmd += " -v " + mount
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/tools_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
Serena *SerenaToolConfig `yaml:"serena,omitempty"`
AgenticWorkflows *AgenticWorkflowsToolConfig `yaml:"agentic-workflows,omitempty"`
CacheMemory *CacheMemoryToolConfig `yaml:"cache-memory,omitempty"`
RepoMemory *RepoMemoryToolConfig `yaml:"repo-memory,omitempty"`

Check failure on line 77 in pkg/workflow/tools_types.go

View workflow job for this annotation

GitHub Actions / copilot

undefined: RepoMemoryToolConfig
Timeout *int `yaml:"timeout,omitempty"`
StartupTimeout *int `yaml:"startup-timeout,omitempty"`

Expand Down Expand Up @@ -268,7 +268,7 @@
GitHubToken string `yaml:"github-token,omitempty"`
Toolset GitHubToolsets `yaml:"toolsets,omitempty"`
Lockdown bool `yaml:"lockdown,omitempty"`
App *GitHubAppConfig `yaml:"app,omitempty"` // GitHub App configuration for token minting

Check failure on line 271 in pkg/workflow/tools_types.go

View workflow job for this annotation

GitHub Actions / copilot

undefined: GitHubAppConfig
}

// PlaywrightDomain represents a domain name allowed for Playwright browser automation
Expand Down Expand Up @@ -374,6 +374,7 @@
APIKey string `yaml:"api-key,omitempty"` // API key for gateway authentication
Domain string `yaml:"domain,omitempty"` // Domain for gateway URL (localhost or host.docker.internal)
Mounts []string `yaml:"mounts,omitempty"` // Volume mounts for the gateway container (format: "source:dest:mode")
PayloadDir string `yaml:"payload-dir,omitempty"` // Directory path for storing large payload JSON files (must be absolute path)
}

// HasTool checks if a tool is present in the configuration
Expand Down
Loading