fix(agentctl): inject HTTP MCP server for Codex ACP support (#641)
* fix(agentctl): inject HTTP MCP server for Codex ACP support Codex ACP only supports HTTP-based MCP servers (not SSE). The kandev MCP server was only injected with SSE transport, causing it to be filtered out by Codex ACP's capability check. Now inject both SSE and HTTP variants of the kandev MCP server: - SSE at /sse (for Claude Code) - HTTP at /mcp (for Codex ACP) The agent's capability filtering automatically selects the appropriate transport based on what the agent supports. * refactor(agentctl): address code review feedback - Add name deduplication to filterMcpServersByCapabilities to prevent duplicate MCP server entries if an agent supports both transports - Extract injectKandevMcpServers helper to centralize dual-transport injection logic in agent.go - Add constants for MCP server name and transport types - Add test for duplicate name deduplication * fix(agentctl): add MCP injection to handleWSResetSession Apply injectKandevMcpServers to session reset handler, ensuring MCP tools remain available after context window reset.
This commit is contained in:
@@ -294,8 +294,11 @@ func (a *Adapter) NewSession(ctx context.Context, mcpServers []types.McpServer)
|
||||
|
||||
// filterMcpServersByCapabilities removes MCP servers that the agent doesn't support.
|
||||
// Stdio servers are always allowed; SSE/HTTP servers require the corresponding capability.
|
||||
// If multiple servers share the same name (e.g., dual SSE+HTTP injection), only the first
|
||||
// surviving entry is kept to prevent duplicate tool registration.
|
||||
func filterMcpServersByCapabilities(servers []types.McpServer, caps acp.McpCapabilities, logger *logger.Logger) []types.McpServer {
|
||||
filtered := make([]types.McpServer, 0, len(servers))
|
||||
seenNames := make(map[string]bool)
|
||||
for _, s := range servers {
|
||||
switch s.Type {
|
||||
case "sse":
|
||||
@@ -309,6 +312,12 @@ func filterMcpServersByCapabilities(servers []types.McpServer, caps acp.McpCapab
|
||||
continue
|
||||
}
|
||||
}
|
||||
// Skip duplicate names - first surviving entry wins
|
||||
if seenNames[s.Name] {
|
||||
logger.Debug("skipping duplicate MCP server name", zap.String("name", s.Name), zap.String("type", s.Type))
|
||||
continue
|
||||
}
|
||||
seenNames[s.Name] = true
|
||||
filtered = append(filtered, s)
|
||||
}
|
||||
return filtered
|
||||
|
||||
@@ -346,6 +346,46 @@ func TestFilterMcpServersByCapabilities_Mixed(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestFilterMcpServersByCapabilities_DeduplicatesSameName(t *testing.T) {
|
||||
log := newTestLoggerForMcp()
|
||||
// Simulate dual injection: same name "kandev" with different types
|
||||
servers := []types.McpServer{
|
||||
{Name: "kandev", Type: "sse", URL: "http://localhost:10005/sse"},
|
||||
{Name: "kandev", Type: "http", URL: "http://localhost:10005/mcp"},
|
||||
{Name: "other", Type: "stdio", Command: "cmd"},
|
||||
}
|
||||
|
||||
// Agent supports both SSE and HTTP - should only get first kandev entry
|
||||
result := filterMcpServersByCapabilities(servers, acp.McpCapabilities{Sse: true, Http: true}, log)
|
||||
if len(result) != 2 {
|
||||
t.Fatalf("expected 2 (first kandev + other), got %d", len(result))
|
||||
}
|
||||
if result[0].Name != "kandev" || result[0].Type != "sse" {
|
||||
t.Errorf("expected first entry to be kandev/sse, got %s/%s", result[0].Name, result[0].Type)
|
||||
}
|
||||
if result[1].Name != "other" {
|
||||
t.Errorf("expected second entry to be other, got %s", result[1].Name)
|
||||
}
|
||||
|
||||
// Agent supports only HTTP - should get kandev/http (SSE filtered, then HTTP kept)
|
||||
result = filterMcpServersByCapabilities(servers, acp.McpCapabilities{Sse: false, Http: true}, log)
|
||||
if len(result) != 2 {
|
||||
t.Fatalf("expected 2 (kandev/http + other), got %d", len(result))
|
||||
}
|
||||
if result[0].Name != "kandev" || result[0].Type != "http" {
|
||||
t.Errorf("expected first entry to be kandev/http, got %s/%s", result[0].Name, result[0].Type)
|
||||
}
|
||||
|
||||
// Agent supports only SSE - should get kandev/sse (HTTP filtered)
|
||||
result = filterMcpServersByCapabilities(servers, acp.McpCapabilities{Sse: true, Http: false}, log)
|
||||
if len(result) != 2 {
|
||||
t.Fatalf("expected 2 (kandev/sse + other), got %d", len(result))
|
||||
}
|
||||
if result[0].Name != "kandev" || result[0].Type != "sse" {
|
||||
t.Errorf("expected first entry to be kandev/sse, got %s/%s", result[0].Name, result[0].Type)
|
||||
}
|
||||
}
|
||||
|
||||
// newTestLoggerForMcp creates a logger for MCP tests.
|
||||
func newTestLoggerForMcp() *logger.Logger {
|
||||
return newTestAdapter().logger
|
||||
|
||||
@@ -19,6 +19,15 @@ import (
|
||||
"go.uber.org/zap"
|
||||
)
|
||||
|
||||
// MCP server constants for kandev injection.
|
||||
const (
|
||||
kandevMcpServerName = "kandev"
|
||||
mcpTransportSSE = "sse"
|
||||
mcpTransportHTTP = "http"
|
||||
mcpPathSSE = "/sse"
|
||||
mcpPathHTTP = "/mcp"
|
||||
)
|
||||
|
||||
// InitializeRequest is a request to initialize the agent session.
|
||||
type InitializeRequest struct {
|
||||
ClientName string `json:"client_name"`
|
||||
@@ -301,6 +310,35 @@ func (s *Server) handleWSInitialize(ctx context.Context, msg *ws.Message) *ws.Me
|
||||
return resp
|
||||
}
|
||||
|
||||
// injectKandevMcpServers prepends the local kandev MCP server to the list of MCP servers.
|
||||
// Both SSE and HTTP variants are injected - the agent's capability filtering will select
|
||||
// the appropriate one based on what the agent supports (SSE for Claude Code, HTTP for Codex ACP).
|
||||
// Any existing kandev server in the list is filtered out to avoid duplicates.
|
||||
func (s *Server) injectKandevMcpServers(mcpServers []types.McpServer) []types.McpServer {
|
||||
kandevMcpSse := types.McpServer{
|
||||
Name: kandevMcpServerName,
|
||||
Type: mcpTransportSSE,
|
||||
URL: fmt.Sprintf("http://localhost:%d%s", s.cfg.Port, mcpPathSSE),
|
||||
}
|
||||
kandevMcpHttp := types.McpServer{
|
||||
Name: kandevMcpServerName,
|
||||
Type: mcpTransportHTTP,
|
||||
URL: fmt.Sprintf("http://localhost:%d%s", s.cfg.Port, mcpPathHTTP),
|
||||
}
|
||||
filtered := make([]types.McpServer, 0, len(mcpServers)+2)
|
||||
filtered = append(filtered, kandevMcpSse, kandevMcpHttp)
|
||||
for _, srv := range mcpServers {
|
||||
if srv.Name != kandevMcpServerName {
|
||||
filtered = append(filtered, srv)
|
||||
}
|
||||
}
|
||||
s.logger.Debug("injected local kandev MCP servers (sse+http)",
|
||||
zap.String("sse_url", kandevMcpSse.URL),
|
||||
zap.String("http_url", kandevMcpHttp.URL),
|
||||
zap.Int("total_servers", len(filtered)))
|
||||
return filtered
|
||||
}
|
||||
|
||||
func (s *Server) handleWSNewSession(ctx context.Context, msg *ws.Message) *ws.Message {
|
||||
var req NewSessionRequest
|
||||
if err := msg.ParsePayload(&req); err != nil {
|
||||
@@ -327,22 +365,7 @@ func (s *Server) handleWSNewSession(ctx context.Context, msg *ws.Message) *ws.Me
|
||||
// If MCP server is enabled, prepend the local kandev MCP server to the list.
|
||||
mcpServers := req.McpServers
|
||||
if s.mcpServer != nil {
|
||||
localKandevMcp := types.McpServer{
|
||||
Name: "kandev",
|
||||
Type: "sse",
|
||||
URL: fmt.Sprintf("http://localhost:%d/sse", s.cfg.Port),
|
||||
}
|
||||
filtered := make([]types.McpServer, 0, len(mcpServers)+1)
|
||||
filtered = append(filtered, localKandevMcp)
|
||||
for _, srv := range mcpServers {
|
||||
if srv.Name != "kandev" {
|
||||
filtered = append(filtered, srv)
|
||||
}
|
||||
}
|
||||
mcpServers = filtered
|
||||
s.logger.Debug("injected local kandev MCP server",
|
||||
zap.String("url", localKandevMcp.URL),
|
||||
zap.Int("total_servers", len(mcpServers)))
|
||||
mcpServers = s.injectKandevMcpServers(mcpServers)
|
||||
}
|
||||
|
||||
sessionID, err := adapter.NewSession(ctx, mcpServers)
|
||||
@@ -387,25 +410,9 @@ func (s *Server) handleWSLoadSession(ctx context.Context, msg *ws.Message) *ws.M
|
||||
}
|
||||
|
||||
// If MCP server is enabled, prepend the local kandev MCP server to the list.
|
||||
// This mirrors handleWSNewSession to ensure MCP tools are available after resume.
|
||||
mcpServers := req.McpServers
|
||||
if s.mcpServer != nil {
|
||||
localKandevMcp := types.McpServer{
|
||||
Name: "kandev",
|
||||
Type: "sse",
|
||||
URL: fmt.Sprintf("http://localhost:%d/sse", s.cfg.Port),
|
||||
}
|
||||
filtered := make([]types.McpServer, 0, len(mcpServers)+1)
|
||||
filtered = append(filtered, localKandevMcp)
|
||||
for _, srv := range mcpServers {
|
||||
if srv.Name != "kandev" {
|
||||
filtered = append(filtered, srv)
|
||||
}
|
||||
}
|
||||
mcpServers = filtered
|
||||
s.logger.Debug("injected local kandev MCP server for loaded session",
|
||||
zap.String("url", localKandevMcp.URL),
|
||||
zap.Int("total_servers", len(mcpServers)))
|
||||
mcpServers = s.injectKandevMcpServers(mcpServers)
|
||||
}
|
||||
|
||||
if err := adapter.LoadSession(ctx, req.SessionID, mcpServers); err != nil {
|
||||
@@ -602,7 +609,13 @@ func (s *Server) handleWSResetSession(ctx context.Context, msg *ws.Message) *ws.
|
||||
return resp
|
||||
}
|
||||
|
||||
sessionID, err := sr.ResetSession(ctx, req.McpServers)
|
||||
// If MCP server is enabled, prepend the local kandev MCP server to the list.
|
||||
mcpServers := req.McpServers
|
||||
if s.mcpServer != nil {
|
||||
mcpServers = s.injectKandevMcpServers(mcpServers)
|
||||
}
|
||||
|
||||
sessionID, err := sr.ResetSession(ctx, mcpServers)
|
||||
if err != nil {
|
||||
s.logger.Error("session reset failed", zap.Error(err))
|
||||
resp, _ := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, err.Error(), nil)
|
||||
|
||||
@@ -386,21 +386,31 @@ func getEnvBool(key string, defaultValue bool) bool {
|
||||
return defaultValue
|
||||
}
|
||||
|
||||
// kandevMcpServerName is the name used for the local kandev MCP server.
|
||||
const kandevMcpServerName = "kandev"
|
||||
|
||||
// injectKandevMcpServer prepends the local kandev MCP server to the list of MCP servers.
|
||||
// This replaces any existing kandev server to avoid duplicates.
|
||||
// The kandev MCP server provides tools like ask_user_question to the agent.
|
||||
// Both SSE and HTTP variants are injected - agent capability filtering will select the appropriate one.
|
||||
func injectKandevMcpServer(servers []McpServerConfig, port int) []McpServerConfig {
|
||||
localKandevMcp := McpServerConfig{
|
||||
Name: "kandev",
|
||||
portStr := strconv.Itoa(port)
|
||||
kandevMcpSse := McpServerConfig{
|
||||
Name: kandevMcpServerName,
|
||||
Type: "sse",
|
||||
URL: "http://localhost:" + strconv.Itoa(port) + "/sse",
|
||||
URL: "http://localhost:" + portStr + "/sse",
|
||||
}
|
||||
kandevMcpHttp := McpServerConfig{
|
||||
Name: kandevMcpServerName,
|
||||
Type: "http",
|
||||
URL: "http://localhost:" + portStr + "/mcp",
|
||||
}
|
||||
|
||||
// Filter out any existing kandev server and prepend the local one
|
||||
result := make([]McpServerConfig, 0, len(servers)+1)
|
||||
result = append(result, localKandevMcp)
|
||||
// Filter out any existing kandev server and prepend the local ones
|
||||
result := make([]McpServerConfig, 0, len(servers)+2)
|
||||
result = append(result, kandevMcpSse, kandevMcpHttp)
|
||||
for _, srv := range servers {
|
||||
if srv.Name != "kandev" {
|
||||
if srv.Name != kandevMcpServerName {
|
||||
result = append(result, srv)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user