Skip to content

Fix safe-inputs MCP server to use HTTP transport for Codex engine #5782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 5 commits into from
Dec 7, 2025
Merged

Fix safe-inputs MCP server to use HTTP transport for Codex engine #5782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8a6e128
Initial plan
Copilot Dec 7, 2025
96d0e47
Initial analysis of safe-inputs MCP support across engines
Copilot Dec 7, 2025
8a8ca86
Fix safe-inputs to use HTTP transport for Codex engine
Copilot Dec 7, 2025
eb81a0c
Add type field to TOML safe-inputs config and improve tests
Copilot Dec 7, 2025
c7ab08b
Add changeset [skip-ci]
actions-user Dec 7, 2025
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
5 changes: 5 additions & 0 deletions .changeset/patch-add-changeset-pr-5782.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 4 additions & 5 deletions .github/workflows/daily-performance-summary.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .github/workflows/release.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 10 additions & 7 deletions pkg/workflow/mcp_renderer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,17 +219,20 @@ func (r *MCPConfigRendererUnified) RenderSafeInputsMCP(yaml *strings.Builder, sa
}

// renderSafeInputsTOML generates Safe Inputs MCP configuration in TOML format
// Uses HTTP transport for consistency with JSON format (Copilot/Claude)
func (r *MCPConfigRendererUnified) renderSafeInputsTOML(yaml *strings.Builder, safeInputs *SafeInputsConfig) {
envVars := getSafeInputsEnvVars(safeInputs)

yaml.WriteString(" \n")
yaml.WriteString(" [mcp_servers." + constants.SafeInputsMCPServerID + "]\n")
yaml.WriteString(" command = \"node\"\n")
yaml.WriteString(" args = [\n")
yaml.WriteString(" \"/tmp/gh-aw/safe-inputs/mcp-server.cjs\",\n")
yaml.WriteString(" ]\n")
// Add environment variables from safe-inputs config
envVars := getSafeInputsEnvVars(safeInputs)
yaml.WriteString(" type = \"http\"\n")
yaml.WriteString(" url = \"http://localhost:$GH_AW_SAFE_INPUTS_PORT\"\n")
yaml.WriteString(" headers = { Authorization = \"Bearer $GH_AW_SAFE_INPUTS_API_KEY\" }\n")

// Add environment variables: server config + tool-specific vars
envVarsWithServerConfig := append([]string{"GH_AW_SAFE_INPUTS_PORT", "GH_AW_SAFE_INPUTS_API_KEY"}, envVars...)
yaml.WriteString(" env_vars = [")
for i, envVar := range envVars {
for i, envVar := range envVarsWithServerConfig {
if i > 0 {
yaml.WriteString(", ")
}
Expand Down
183 changes: 183 additions & 0 deletions pkg/workflow/safe_inputs_http_codex_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
package workflow

import (
"os"
"path/filepath"
"strings"
"testing"
)

// TestCodexSafeInputsHTTPTransport verifies that Codex engine uses HTTP transport for safe-inputs
// (not stdio transport) to be consistent with Copilot and Claude engines
func TestCodexSafeInputsHTTPTransport(t *testing.T) {
// Create a temporary workflow file
tempDir := t.TempDir()
workflowPath := filepath.Join(tempDir, "test-workflow.md")

workflowContent := `---
on: workflow_dispatch
engine: codex
safe-inputs:
test-tool:
description: Test tool
script: |
return { result: "test" };
---

Test safe-inputs HTTP transport for Codex
`

err := os.WriteFile(workflowPath, []byte(workflowContent), 0644)
if err != nil {
t.Fatalf("Failed to write workflow file: %v", err)
}

// Compile the workflow
compiler := NewCompiler(false, "", "test")
err = compiler.CompileWorkflow(workflowPath)
if err != nil {
t.Fatalf("Failed to compile workflow: %v", err)
}

// Read the generated lock file
lockPath := strings.TrimSuffix(workflowPath, ".md") + ".lock.yml"
lockContent, err := os.ReadFile(lockPath)
if err != nil {
t.Fatalf("Failed to read lock file: %v", err)
}

yamlStr := string(lockContent)

// Verify that the HTTP server configuration steps are generated
expectedSteps := []string{
"Generate Safe Inputs MCP Server Config",
"Start Safe Inputs MCP HTTP Server",
"Setup MCPs",
}

for _, stepName := range expectedSteps {
if !strings.Contains(yamlStr, stepName) {
t.Errorf("Expected step not found in workflow: %q", stepName)
}
}

// Verify HTTP transport in TOML config (not stdio)
if !strings.Contains(yamlStr, "[mcp_servers.safeinputs]") {
t.Error("Safe-inputs MCP server config section not found")
}

// Should have explicit type field
codexConfigSection := extractCodexConfigSection(yamlStr)
if !strings.Contains(codexConfigSection, `type = "http"`) {
t.Error("Expected type field set to 'http' in TOML format")
}

// Should use HTTP transport (url + headers)
if !strings.Contains(yamlStr, `url = "http://localhost:$GH_AW_SAFE_INPUTS_PORT"`) {
t.Error("Expected HTTP URL config not found in TOML format")
}

if !strings.Contains(yamlStr, `headers = { Authorization = "Bearer $GH_AW_SAFE_INPUTS_API_KEY" }`) {
t.Error("Expected HTTP headers config not found in TOML format")
}

// Should NOT use stdio transport (command + args to node)
if strings.Contains(codexConfigSection, `command = "node"`) {
t.Error("Codex config should not use stdio transport (command = 'node'), should use HTTP")
}

if strings.Contains(codexConfigSection, `args = [`) && strings.Contains(codexConfigSection, `/tmp/gh-aw/safe-inputs/mcp-server.cjs`) {
t.Error("Codex config should not use stdio transport with mcp-server.cjs args, should use HTTP")
}

// Verify environment variables are included
if !strings.Contains(codexConfigSection, "GH_AW_SAFE_INPUTS_PORT") {
t.Error("Expected GH_AW_SAFE_INPUTS_PORT env var in config")
}

if !strings.Contains(codexConfigSection, "GH_AW_SAFE_INPUTS_API_KEY") {
t.Error("Expected GH_AW_SAFE_INPUTS_API_KEY env var in config")
}

t.Logf("✓ Codex engine correctly uses HTTP transport for safe-inputs")
}

// extractCodexConfigSection extracts the Codex MCP config section from the workflow YAML
func extractCodexConfigSection(yamlContent string) string {
// Find the start of the safeinputs config
start := strings.Index(yamlContent, "[mcp_servers.safeinputs]")
if start == -1 {
return ""
}

// Find the end (next section or EOF)
end := strings.Index(yamlContent[start:], "EOF")
if end == -1 {
return yamlContent[start:]
}

return yamlContent[start : start+end]
}

// TestCodexSafeInputsWithSecretsHTTPTransport verifies that environment variables
// from safe-inputs tools are properly passed through with HTTP transport
func TestCodexSafeInputsWithSecretsHTTPTransport(t *testing.T) {
tempDir := t.TempDir()
workflowPath := filepath.Join(tempDir, "test-workflow.md")

workflowContent := `---
on: workflow_dispatch
engine: codex
safe-inputs:
api-call:
description: Call an API
env:
API_KEY: ${{ secrets.API_KEY }}
GH_TOKEN: ${{ github.token }}
script: |
return { result: "test" };
---

Test safe-inputs with secrets
`

err := os.WriteFile(workflowPath, []byte(workflowContent), 0644)
if err != nil {
t.Fatalf("Failed to write workflow file: %v", err)
}

compiler := NewCompiler(false, "", "test")
err = compiler.CompileWorkflow(workflowPath)
if err != nil {
t.Fatalf("Failed to compile workflow: %v", err)
}

lockPath := strings.TrimSuffix(workflowPath, ".md") + ".lock.yml"
lockContent, err := os.ReadFile(lockPath)
if err != nil {
t.Fatalf("Failed to read lock file: %v", err)
}

yamlStr := string(lockContent)
codexConfigSection := extractCodexConfigSection(yamlStr)

// Verify tool-specific env vars are included in HTTP transport config
if !strings.Contains(codexConfigSection, "API_KEY") {
t.Error("Expected API_KEY env var in safe-inputs config")
}

if !strings.Contains(codexConfigSection, "GH_TOKEN") {
t.Error("Expected GH_TOKEN env var in safe-inputs config")
}

// Verify env vars are set in Setup MCPs step
if !strings.Contains(yamlStr, "API_KEY: ${{ secrets.API_KEY }}") {
t.Error("Expected API_KEY secret in Setup MCPs env section")
}

if !strings.Contains(yamlStr, "GH_TOKEN: ${{ github.token }}") {
t.Error("Expected GH_TOKEN in Setup MCPs env section")
}

t.Logf("✓ Codex engine correctly passes secrets through HTTP transport")
}
Loading