Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 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
8 changes: 8 additions & 0 deletions cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ func runServer(ctx context.Context, logger *slog.Logger, argsToPass []string) er
}

experimentalACP := viper.GetBool(FlagExperimentalACP)
acpMCPFile := viper.GetString(FlagMCPFile)

if acpMCPFile != "" && !experimentalACP {
return xerrors.Errorf("--mcp-file requires --experimental-acp requires to be set")
Comment thread
35C4n0r marked this conversation as resolved.
Outdated
}

if experimentalACP && (saveState || loadState) {
return xerrors.Errorf("ACP mode doesn't support state persistence")
Expand Down Expand Up @@ -169,6 +174,7 @@ func runServer(ctx context.Context, logger *slog.Logger, argsToPass []string) er
acpResult, err = httpapi.SetupACP(ctx, httpapi.SetupACPConfig{
Program: agent,
ProgramArgs: argsToPass[1:],
MCPFilePath: acpMCPFile,
})
if err != nil {
return xerrors.Errorf("failed to setup ACP: %w", err)
Expand Down Expand Up @@ -382,6 +388,7 @@ const (
FlagSaveState = "save-state"
FlagPidFile = "pid-file"
FlagExperimentalACP = "experimental-acp"
FlagMCPFile = "mcp-file"
)

func CreateServerCmd() *cobra.Command {
Expand Down Expand Up @@ -425,6 +432,7 @@ func CreateServerCmd() *cobra.Command {
{FlagSaveState, "", false, "Save state to state-file on shutdown (defaults to true when state-file is set)", "bool"},
{FlagPidFile, "", "", "Path to file where the server process ID will be written for shutdown scripts", "string"},
{FlagExperimentalACP, "", false, "Use experimental ACP transport instead of PTY", "bool"},
{FlagMCPFile, "", "", "MCP file for the ACP server", "string"},
Comment thread
35C4n0r marked this conversation as resolved.
}

for _, spec := range flagSpecs {
Expand Down
52 changes: 52 additions & 0 deletions cmd/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -799,3 +799,55 @@ func TestServerCmd_AllowedOrigins(t *testing.T) {
})
}
}

func TestServerCmd_MCPFileFlag(t *testing.T) {
t.Run("mcp-file default is empty", func(t *testing.T) {
isolateViper(t)

serverCmd := CreateServerCmd()
setupCommandOutput(t, serverCmd)
serverCmd.SetArgs([]string{"--exit", "dummy-command"})
err := serverCmd.Execute()
require.NoError(t, err)

assert.Equal(t, "", viper.GetString(FlagMCPFile))
})

t.Run("mcp-file can be set via CLI flag", func(t *testing.T) {
isolateViper(t)

serverCmd := CreateServerCmd()
setupCommandOutput(t, serverCmd)
serverCmd.SetArgs([]string{"--mcp-file", "/path/to/mcp.json", "--exit", "dummy-command"})
err := serverCmd.Execute()
require.NoError(t, err)

assert.Equal(t, "/path/to/mcp.json", viper.GetString(FlagMCPFile))
})

t.Run("mcp-file can be set via environment variable", func(t *testing.T) {
isolateViper(t)
t.Setenv("AGENTAPI_MCP_FILE", "/env/path/to/mcp.json")

serverCmd := CreateServerCmd()
setupCommandOutput(t, serverCmd)
serverCmd.SetArgs([]string{"--exit", "dummy-command"})
err := serverCmd.Execute()
require.NoError(t, err)

assert.Equal(t, "/env/path/to/mcp.json", viper.GetString(FlagMCPFile))
})

t.Run("CLI flag overrides environment variable", func(t *testing.T) {
isolateViper(t)
t.Setenv("AGENTAPI_MCP_FILE", "/env/path/to/mcp.json")

serverCmd := CreateServerCmd()
setupCommandOutput(t, serverCmd)
serverCmd.SetArgs([]string{"--mcp-file", "/cli/path/to/mcp.json", "--exit", "dummy-command"})
err := serverCmd.Execute()
require.NoError(t, err)

assert.Equal(t, "/cli/path/to/mcp.json", viper.GetString(FlagMCPFile))
})
}
38 changes: 38 additions & 0 deletions e2e/echo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,3 +556,41 @@ func getFreePort() (int, error) {

return l.Addr().(*net.TCPAddr).Port, nil
}

func TestServerFlagValidation(t *testing.T) {
if testing.Short() {
t.Skip("Skipping integration test in short mode")
}

t.Run("mcp-file requires experimental-acp", func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
defer cancel()

binaryPath := os.Getenv("AGENTAPI_BINARY_PATH")
if binaryPath == "" {
cwd, err := os.Getwd()
require.NoError(t, err, "Failed to get current working directory")
binaryPath = filepath.Join(cwd, "..", "out", "agentapi")
t.Logf("Building binary at %s", binaryPath)
buildCmd := exec.CommandContext(ctx, "go", "build", "-o", binaryPath, ".")
buildCmd.Dir = filepath.Join(cwd, "..")
require.NoError(t, buildCmd.Run(), "Failed to build binary")
}

// Create a temporary MCP file
tmpDir := t.TempDir()
mcpFile := filepath.Join(tmpDir, "mcp.json")
err := os.WriteFile(mcpFile, []byte(`{"mcpServers": []}`), 0o644)
Comment thread
35C4n0r marked this conversation as resolved.
Outdated
require.NoError(t, err, "Failed to create temp MCP file")

// Run the server with --mcp-file but WITHOUT --experimental-acp
cmd := exec.CommandContext(ctx, binaryPath, "server",
"--mcp-file", mcpFile,
"--", "echo", "test")

output, err := cmd.CombinedOutput()
require.Error(t, err, "Expected server to fail when --mcp-file is used without --experimental-acp")
require.Contains(t, string(output), "--mcp-file requires --experimental-acp",
"Expected error message about --mcp-file requiring --experimental-acp, got: %s", string(output))
})
}
3 changes: 2 additions & 1 deletion lib/httpapi/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ func SetupProcess(ctx context.Context, config SetupProcessConfig) (*termexec.Pro
type SetupACPConfig struct {
Program string
ProgramArgs []string
MCPFilePath string
Clock quartz.Clock
}

Expand Down Expand Up @@ -88,7 +89,7 @@ func SetupACP(ctx context.Context, config SetupACPConfig) (*SetupACPResult, erro
return nil, fmt.Errorf("failed to start process: %w", err)
}

agentIO, err := acpio.NewWithPipes(ctx, stdin, stdout, logger, os.Getwd)
agentIO, err := acpio.NewWithPipes(ctx, stdin, stdout, logger, os.Getwd, config.MCPFilePath)
if err != nil {
_ = cmd.Process.Kill()
return nil, fmt.Errorf("failed to initialize ACP connection: %w", err)
Expand Down
51 changes: 49 additions & 2 deletions x/acpio/acpio.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@ package acpio

import (
"context"
"encoding/json"
"fmt"
"io"
"log/slog"
"os"
"strings"
"sync"

acp "github.com/coder/acp-go-sdk"
st "github.com/coder/agentapi/lib/screentracker"
"golang.org/x/xerrors"
)

// Compile-time assertion that ACPAgentIO implements st.AgentIO
Expand All @@ -31,6 +34,10 @@ type acpClient struct {
agentIO *ACPAgentIO
}

type McpConfig struct {
McpServers []acp.McpServer `json:"mcpServers"`
}

var _ acp.Client = (*acpClient)(nil)

func (c *acpClient) SessionUpdate(ctx context.Context, params acp.SessionNotification) error {
Expand Down Expand Up @@ -131,7 +138,7 @@ func (a *ACPAgentIO) SetOnChunk(fn func(chunk string)) {
}

// NewWithPipes creates an ACPAgentIO connected via the provided pipes
func NewWithPipes(ctx context.Context, toAgent io.Writer, fromAgent io.Reader, logger *slog.Logger, getwd func() (string, error)) (*ACPAgentIO, error) {
func NewWithPipes(ctx context.Context, toAgent io.Writer, fromAgent io.Reader, logger *slog.Logger, getwd func() (string, error), mcpFilePath string) (*ACPAgentIO, error) {
Comment thread
35C4n0r marked this conversation as resolved.
if logger == nil {
logger = slog.Default()
}
Expand All @@ -154,6 +161,12 @@ func NewWithPipes(ctx context.Context, toAgent io.Writer, fromAgent io.Reader, l
}
logger.Debug("ACP initialized", "protocolVersion", initResp.ProtocolVersion)

// Prepare the MCPs for the session
supportedMCPList, err := getSupportedMCPConfig(mcpFilePath, logger, &initResp)
if err != nil {
return nil, err
}

// Create a session
cwd, err := getwd()
if err != nil {
Expand All @@ -162,7 +175,7 @@ func NewWithPipes(ctx context.Context, toAgent io.Writer, fromAgent io.Reader, l
}
sessResp, err := conn.NewSession(ctx, acp.NewSessionRequest{
Cwd: cwd,
McpServers: []acp.McpServer{},
McpServers: supportedMCPList,
})
if err != nil {
logger.Error("Failed to create ACP session", "error", err)
Expand All @@ -174,6 +187,40 @@ func NewWithPipes(ctx context.Context, toAgent io.Writer, fromAgent io.Reader, l
return agentIO, nil
}

func getSupportedMCPConfig(mcpFilePath string, logger *slog.Logger, initResp *acp.InitializeResponse) ([]acp.McpServer, error) {
if mcpFilePath == "" {
return []acp.McpServer{}, nil
}

mcpFile, err := os.Open(mcpFilePath)
if err != nil {
return nil, xerrors.Errorf("Failed to open mcp file: %v", err)
}
Comment thread
35C4n0r marked this conversation as resolved.

defer func() {
if closeErr := mcpFile.Close(); closeErr != nil {
logger.Error("Failed to close mcp file", "error", err)
Comment thread
35C4n0r marked this conversation as resolved.
Outdated
}
}()

var allMcpList McpConfig
decoder := json.NewDecoder(mcpFile)

if err = decoder.Decode(&allMcpList); err != nil {
return nil, xerrors.Errorf("Failed to decode mcp file: %v", err)
}
Comment thread
35C4n0r marked this conversation as resolved.
Outdated
Comment thread
35C4n0r marked this conversation as resolved.

// Only send the MCPs that are supported by the agents
var supportedMCPList []acp.McpServer
for _, mcp := range allMcpList.McpServers {
if (mcp.Http != nil && !initResp.AgentCapabilities.McpCapabilities.Http) || (mcp.Sse != nil && !initResp.AgentCapabilities.McpCapabilities.Sse) {
continue
}
supportedMCPList = append(supportedMCPList, mcp)
}
return supportedMCPList, err
}

// Write sends a message to the agent via ACP prompt
func (a *ACPAgentIO) Write(data []byte) (int, error) {
text := string(data)
Expand Down
1 change: 1 addition & 0 deletions x/acpio/acpio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func newTestPair(t *testing.T, agent *testAgent) *acpio.ACPAgentIO {
clientToAgentW, agentToClientR,
nil,
func() (string, error) { return os.TempDir(), nil },
"", // no MCP file
)
require.NoError(t, err)

Expand Down
Loading
Loading