Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 13, 2025

This PR expands the original MCP server support implementation to include HTTP transport capability validation for different agentic engines, addressing the limitation that some engines only support stdio transport. Additionally, it enhances the test-codex workflow with a time MCP server for timestamping functionality.

Key Changes

1. HTTP Transport Capability System

Added a new engine capability SupportsHTTPTransport() to track which engines support HTTP MCP servers:

  • Codex: Only supports stdio transport (set to false)
  • Claude: Supports both stdio and HTTP transport (set to true)
  • Gemini: Only supports stdio for custom MCP servers (set to false)

2. Enhanced CLI Display

Updated the engine listing to show HTTP transport capabilities:

ID              Display Name         Status       MCP      HTTP Transport
--              ------------         ------       ---      -------------
claude          Claude Code          Stable       Yes      Yes         
codex           Codex                Experimental Yes      No          
gemini          Gemini CLI           Stable       Yes      No          

3. Compiler Validation

Added validation in the workflow compiler to prevent HTTP MCP server configurations when the engine doesn't support them:

---
engine: codex  # Only supports stdio
tools:
  custom-server:
    mcp:
      type: "http"  # This will now be rejected
      url: "http://localhost:3000"
---

Results in: tool 'custom-server' uses HTTP transport which is not supported by engine 'codex' (only stdio transport is supported)

4. Enhanced Test Codex Workflow

Added comprehensive MCP server support to the test-codex workflow using best practices for container configuration:

tools:
  time:
    mcp:
      type: stdio
      container: "mcp/time"
      env:
        LOCAL_TIMEZONE: "${LOCAL_TIMEZONE}"
    allowed: ["get_current_time"]
  • Time MCP Server: Container-based time server for timestamping analysis
  • get_current_time Tool: Enabled for timestamping workflow execution
  • Updated Prompt: Modified to request current time at analysis start
  • Enhanced Output: Added timestamp field to PR summary template
  • Proper Container Configuration: Uses the documented container field for clean Docker configuration

5. Maintained Original Features

All original MCP functionality remains intact:

  • History configuration for Codex (persistence = "none")
  • Full MCP server support with supportsToolsWhitelist: true
  • Proper TOML configuration generation
  • Backward compatibility with existing workflows

6. Testing

  • All existing tests continue to pass
  • New validation correctly rejects unsupported configurations
  • HTTP transport works correctly with compatible engines (Claude)
  • Time MCP server configuration compiles correctly for Codex using proper container field
  • Container field properly transforms to Docker command structure in generated TOML

This ensures that workflows fail fast with clear error messages when attempting to use HTTP MCP servers with engines that only support stdio transport, while providing enhanced functionality for timestamp-aware workflow execution using best practices for container configuration.

Fixes #18.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Enable MCP tools whitelist support for Codex engine
- Add history configuration with persistence=none to config.toml
- Update engine description to reflect MCP support
- Add comprehensive tests for custom MCP servers with Codex
- Validate config.toml generation includes history and MCP sections
- Ensure compatibility with existing Claude and Gemini engines

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] code mcp server Add MCP server support and history configuration for Codex engine Aug 13, 2025
Copilot AI requested a review from pelikhan August 13, 2025 01:00
command: "python"
args: ["-m", "my_server"]
env:
API_KEY: "{{ secrets.API_KEY }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot missing $

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in commit 1ea81dc. The missing $ has been added to both the test input and assertion to use the correct GitHub Actions template syntax: ${{ secrets.API_KEY }}.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 13, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.contoso.com
    • Triggering command: /tmp/go-build1463307643/b255/cli.test -test.testlogfile=/tmp/go-build1463307643/b255/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from pelikhan August 13, 2025 02:29
@pelikhan
Copy link
Contributor

pelikhan commented Aug 13, 2025

@copilot OpenAI codex only supports stdio transport for mcp servers.

  • Add a agentic engine capability for http transport support and set it to false for codex.
  • update other engines
  • add a configuration validation in the compiler to refuse http MCP server configurations when not supported by the engine.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 13, 2025

@copilot OpenAI codex only supports stdio transport for mcp servers.

  • Add a agentic engine capability for http transport support and set it to false for codex.
  • update other engines ...

I've implemented all the requested changes in commit 45e3d53:

  1. Added HTTP transport capability - New SupportsHTTPTransport() method in the agentic engine interface
  2. Set capabilities per engine:
    • Codex: supportsHTTPTransport = false (only stdio transport)
    • Claude: supportsHTTPTransport = true (both stdio and HTTP)
    • Gemini: supportsHTTPTransport = false (only stdio for custom MCP)
  3. Updated CLI display - Replaced "Node.js" column with "HTTP Transport" showing each engine's capabilities
  4. Added compiler validation - HTTP MCP servers are now rejected with clear error messages when used with engines that don't support them

The validation works correctly:

  • Codex with HTTP MCP: ❌ tool 'custom-server' uses HTTP transport which is not supported by engine 'codex' (only stdio transport is supported)
  • Claude with HTTP MCP: ✅ Compiles successfully
  • Gemini with HTTP MCP: ❌ Same error as Codex

All existing tests pass and the new functionality is working as requested.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.contoso.com
    • Triggering command: /tmp/go-build1193402773/b255/cli.test -test.testlogfile=/tmp/go-build1193402773/b255/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)
    • Triggering command: /tmp/go-build2581049781/b255/cli.test -test.testlogfile=/tmp/go-build2581049781/b255/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Add MCP server support and history configuration for Codex engine Add HTTP transport capability and validation for MCP servers in agentic engines Aug 13, 2025
@pelikhan
Copy link
Contributor

pelikhan commented Aug 13, 2025

.

@pelikhan
Copy link
Contributor

@copilot - Update the test-codex.yml with a simple mcp stdio server

{
"mcpServers": {
"time": {
"command": "docker",
"args": ["run", "-i", "--rm", "-e", "LOCAL_TIMEZONE", "mcp/time"]
}
}
}

  • Update test-codex.yml to allow the get_current_time tool
  • update prompt to request the current time

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 13, 2025

@copilot - Update the test-codex.yml with a simple mcp stdio server

{...

I've implemented all the requested changes in commit ad97bf3:

  1. Added time MCP server with stdio transport using the specified Docker configuration:

    tools:
      time:
        mcp:
          type: stdio
          command: "docker"
          args: ["run", "-i", "--rm", "-e", "LOCAL_TIMEZONE", "mcp/time"]
        allowed: ["get_current_time"]
  2. Enabled get_current_time tool in the allowed tools list for the time MCP server

  3. Updated prompt to request current time at the start:

    • Added instruction: "First, get the current time using the get_current_time tool to timestamp your analysis"
    • Added "Analysis Time" field to the PR summary template

The workflow now compiles correctly and generates the proper TOML configuration with both the GitHub MCP server (for repository operations) and the time MCP server (for timestamping). All tests continue to pass.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.contoso.com
    • Triggering command: /tmp/go-build878271565/b255/cli.test -test.testlogfile=/tmp/go-build878271565/b255/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Add HTTP transport capability and validation for MCP servers in agentic engines Add HTTP transport capability validation and time MCP server for agentic engines Aug 13, 2025
time:
mcp:
type: stdio
command: "docker"
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot use the "container" field to configure this MCP server (see docs)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in commit dfcff98. Changed to use the container field as documented instead of manual Docker command configuration. The time MCP server now uses proper container-based setup that automatically generates the correct Docker command structure.

…orkflow

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 13, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.contoso.com
    • Triggering command: /tmp/go-build2430576817/b255/cli.test -test.testlogfile=/tmp/go-build2430576817/b255/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)
    • Triggering command: /tmp/go-build2937509441/b255/cli.test -test.testlogfile=/tmp/go-build2937509441/b255/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from pelikhan August 13, 2025 04:12
@pelikhan pelikhan marked this pull request as ready for review August 13, 2025 04:20
@github-actions
Copy link
Contributor

🤖 Codex PR Summary

Branch: copilot/fix-18
Files Changed: 11 files
Analysis Time: 2025-08-13T04:36:27Z

📋 Change Overview

  • Adds HTTP transport capability and validation for agentic engines.
  • Introduces time MCP server for timestamping analyses.
  • Type of changes: feature, refactor, tests.

📁 Files Modified

  • .github/workflows/ci.yml (Modified): Adds check for generated lock file changes.
  • .github/workflows/test-codex.lock.yml (Modified): Inserts history persistence setting; adds time MCP server config; reorders env vars.
  • .github/workflows/test-codex.md (Modified): Mirrors lock workflow changes; enables get_current_time tool.
  • pkg/cli/commands.go (Modified): Extends engine listing table to include HTTP transport column.
  • pkg/workflow/agentic_engine.go (Modified): Adds SupportsHTTPTransport method to interface and base struct.
  • pkg/workflow/claude_engine.go (Modified): Enables HTTP transport support for Claude engine.
  • pkg/workflow/codex_engine.go (Modified): Updates description; enables tool whitelist; sets HTTP transport to false; injects history config.
  • pkg/workflow/codex_engine_test.go (Modified): Corrects test assertion for tool whitelist support.
  • pkg/workflow/codex_test.go (Modified): Adds tests for history persistence and custom MCP server scenarios.
  • pkg/workflow/compiler.go (Modified): Adds HTTP transport validation; sorts environment keys for consistency.
  • pkg/workflow/gemini_engine.go (Modified): Disables HTTP transport support for Gemini engine.

🔍 Key Changes

  • Tracks HTTP transport capability per engine and surfaces it in CLI.
  • Compiler now rejects HTTP MCP servers when engine lacks support.
  • Adds time-based MCP server in test workflows using proper container configuration.

🎯 Recommendations

  • Update repository documentation for HTTP transport support and configuration.
  • Add integration tests covering HTTP transport paths across engines.
  • Review readme to include new CLI table column.
  • Confirm existing workflows still compile without HTTP MCP configs.

🔗 Related

AI-generated content by Test Codex may contain mistakes.

@pelikhan pelikhan merged commit 0d8250d into main Aug 13, 2025
9 checks passed
@pelikhan pelikhan deleted the copilot/fix-18 branch August 13, 2025 04:39
github-actions bot added a commit that referenced this pull request Oct 14, 2025
Replace single-quote string formatting with %q format specifier to properly
escape user-controlled input in error messages. This prevents potential quote
injection vulnerabilities where malicious YAML key names containing single
quotes could break out of the string literal.

Security Impact:
- Prevents potential command injection or SQL injection if error messages
  are used in contexts that interpret quotes specially
- Follows Go best practices for safely embedding untrusted data in strings

Changes:
- Changed format string from '%s' to %q in validateExpressionSizes function
- The %q specifier automatically escapes special characters including quotes

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
pelikhan pushed a commit that referenced this pull request Oct 14, 2025
Replace single-quote string formatting with %q format specifier to properly
escape user-controlled input in error messages. This prevents potential quote
injection vulnerabilities where malicious YAML key names containing single
quotes could break out of the string literal.

Security Impact:
- Prevents potential command injection or SQL injection if error messages
  are used in contexts that interpret quotes specially
- Follows Go best practices for safely embedding untrusted data in strings

Changes:
- Changed format string from '%s' to %q in validateExpressionSizes function
- The %q specifier automatically escapes special characters including quotes

Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Security Fix PR <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openai codex mcp server

3 participants