diff --git a/apps/backend/internal/agent/agents/agent.go b/apps/backend/internal/agent/agents/agent.go index fca8b1310..c1a623c88 100644 --- a/apps/backend/internal/agent/agents/agent.go +++ b/apps/backend/internal/agent/agents/agent.go @@ -9,6 +9,7 @@ import ( "errors" "time" + "github.com/kandev/kandev/internal/agent/mcpconfig" "github.com/kandev/kandev/internal/agent/usage" "github.com/kandev/kandev/internal/agentruntime" "github.com/kandev/kandev/pkg/agent" @@ -157,7 +158,10 @@ type PassthroughOptions struct { Resume bool // generic "continue last session" (e.g. -c, --resume latest) PermissionValues map[string]bool // e.g. {"auto_approve": true} WorkDir string - MCPConfigPath string // path to generated MCP config file for passthrough CLIs that support it + // MCPArgs are extra argv tokens produced by the agent's MCP passthrough + // strategy (e.g. claude's "--mcp-config ", codex's repeated "-c + // mcp_servers.…" overrides). Appended to the built command. + MCPArgs []string // CLIFlagTokens are user-configured CLI flag argv tokens derived from // AgentProfile.CLIFlags (only Enabled entries, shell-tokenised). Appended // verbatim to the built passthrough command, mirroring CommandOptions. @@ -259,8 +263,11 @@ type PassthroughConfig struct { StabilityWindow time.Duration ResumeFlag Param // generic "continue last session" (e.g. NewParam("-c"), NewParam("--resume", "latest")) SessionResumeFlag Param // resume a specific session by ID (e.g. NewParam("--resume")) - MCPConfigFlag Param // flag used to load an MCP config file, e.g. NewParam("--mcp-config", "{mcp_config}") - WaitForTerminal bool + // MCPStrategy materializes resolved MCP servers into this CLI's passthrough + // shape (config file + flag, repeated -c overrides, project file, or env var) + // without touching the user's global config. Nil means no MCP injection. + MCPStrategy mcpconfig.PassthroughMCPStrategy + WaitForTerminal bool // AutoInjectPrompt enables writing the task description to the PTY stdin // after the first idle window. Default false preserves today's behavior. AutoInjectPrompt bool diff --git a/apps/backend/internal/agent/agents/claude_acp.go b/apps/backend/internal/agent/agents/claude_acp.go index 9b37ce73a..21c7817ca 100644 --- a/apps/backend/internal/agent/agents/claude_acp.go +++ b/apps/backend/internal/agent/agents/claude_acp.go @@ -5,6 +5,7 @@ import ( _ "embed" "time" + "github.com/kandev/kandev/internal/agent/mcpconfig" "github.com/kandev/kandev/internal/agent/usage" "github.com/kandev/kandev/pkg/agent" ) @@ -44,7 +45,7 @@ func NewClaudeACP() *ClaudeACP { BufferMaxBytes: DefaultBufferMaxBytes, ResumeFlag: NewParam("-c"), SessionResumeFlag: NewParam("--resume"), - MCPConfigFlag: NewParam("--mcp-config", "{mcp_config}"), + MCPStrategy: mcpconfig.ClaudeStrategy{}, AutoInjectPrompt: true, SubmitSequence: "\r", DisableBracketedPaste: true, diff --git a/apps/backend/internal/agent/agents/codex_acp.go b/apps/backend/internal/agent/agents/codex_acp.go index fe018c489..b69e001b8 100644 --- a/apps/backend/internal/agent/agents/codex_acp.go +++ b/apps/backend/internal/agent/agents/codex_acp.go @@ -5,6 +5,7 @@ import ( _ "embed" "time" + "github.com/kandev/kandev/internal/agent/mcpconfig" "github.com/kandev/kandev/internal/agent/usage" "github.com/kandev/kandev/pkg/agent" ) @@ -56,6 +57,7 @@ func NewCodexACP() *CodexACP { ModelFlag: NewParam("--model", "{model}"), IdleTimeout: 3 * time.Second, BufferMaxBytes: DefaultBufferMaxBytes, + MCPStrategy: mcpconfig.CodexStrategy{}, AutoInjectPrompt: true, SubmitSequence: "\r", }, diff --git a/apps/backend/internal/agent/agents/cursor_acp.go b/apps/backend/internal/agent/agents/cursor_acp.go index eaef0b56d..6d3dfd973 100644 --- a/apps/backend/internal/agent/agents/cursor_acp.go +++ b/apps/backend/internal/agent/agents/cursor_acp.go @@ -6,6 +6,7 @@ import ( _ "embed" "time" + "github.com/kandev/kandev/internal/agent/mcpconfig" "github.com/kandev/kandev/internal/agent/usage" "github.com/kandev/kandev/pkg/agent" ) @@ -60,6 +61,9 @@ func NewCursorACP() *CursorACP { ModelFlag: NewParam("--model", "{model}"), IdleTimeout: 3 * time.Second, BufferMaxBytes: DefaultBufferMaxBytes, + // cursor-agent has no MCP flag/env; write a project-local + // .cursor/mcp.json into the worktree (only if absent). + MCPStrategy: mcpconfig.CursorStrategy{}, }, }, } diff --git a/apps/backend/internal/agent/agents/helpers.go b/apps/backend/internal/agent/agents/helpers.go index 8acee1920..3e5061be8 100644 --- a/apps/backend/internal/agent/agents/helpers.go +++ b/apps/backend/internal/agent/agents/helpers.go @@ -104,25 +104,6 @@ func (b *CmdBuilder) Prompt(flag Param, prompt string) *CmdBuilder { return b } -// MCPConfig appends an MCP config flag when a passthrough CLI supports one. -// flag args support {mcp_config} placeholder, e.g. NewParam("--mcp-config", "{mcp_config}"). -func (b *CmdBuilder) MCPConfig(flag Param, path string) *CmdBuilder { - if flag.IsEmpty() || path == "" { - return b - } - hasPlaceholder := false - for _, arg := range flag.args { - if strings.Contains(arg, "{mcp_config}") { - hasPlaceholder = true - } - b.args = append(b.args, strings.ReplaceAll(arg, "{mcp_config}", path)) - } - if !hasPlaceholder { - b.args = append(b.args, path) - } - return b -} - // Flag appends arbitrary flag parts to the command. func (b *CmdBuilder) Flag(parts ...string) *CmdBuilder { b.args = append(b.args, parts...) diff --git a/apps/backend/internal/agent/agents/opencode_acp.go b/apps/backend/internal/agent/agents/opencode_acp.go index a1a2d8638..37ca089bf 100644 --- a/apps/backend/internal/agent/agents/opencode_acp.go +++ b/apps/backend/internal/agent/agents/opencode_acp.go @@ -5,6 +5,7 @@ import ( _ "embed" "time" + "github.com/kandev/kandev/internal/agent/mcpconfig" "github.com/kandev/kandev/internal/agent/usage" "github.com/kandev/kandev/pkg/agent" ) @@ -43,6 +44,10 @@ func NewOpenCodeACP() *OpenCodeACP { IdleTimeout: 3 * time.Second, BufferMaxBytes: DefaultBufferMaxBytes, ResumeFlag: NewParam("-c"), + // opencode has no MCP flag; write a temp opencode.json and point + // it there via the OPENCODE_CONFIG env var (merges, never writes + // ~/.config/opencode). + MCPStrategy: mcpconfig.OpenCodeStrategy{}, }, }, } diff --git a/apps/backend/internal/agent/agents/passthrough.go b/apps/backend/internal/agent/agents/passthrough.go index 733d0bc23..ceb16bfc1 100644 --- a/apps/backend/internal/agent/agents/passthrough.go +++ b/apps/backend/internal/agent/agents/passthrough.go @@ -27,7 +27,11 @@ func (p *StandardPassthrough) BuildPassthroughCommand(opts PassthroughOptions) C case opts.Prompt != "": b.Prompt(p.Cfg.PromptFlag, opts.Prompt) } - b.MCPConfig(p.Cfg.MCPConfigFlag, opts.MCPConfigPath) + + // MCP injection args go last so Claude Code's variadic --mcp-config does not + // swallow a positional prompt as an extra config path. Codex's `-c` overrides + // are order-insensitive, so trailing placement is safe for them too. + b.Flag(opts.MCPArgs...) return b.Build() } diff --git a/apps/backend/internal/agent/mcpconfig/passthrough.go b/apps/backend/internal/agent/mcpconfig/passthrough.go new file mode 100644 index 000000000..8d73c0c3a --- /dev/null +++ b/apps/backend/internal/agent/mcpconfig/passthrough.go @@ -0,0 +1,431 @@ +package mcpconfig + +import ( + "encoding/json" + "fmt" + "path/filepath" + + "github.com/kandev/kandev/internal/agentctl/types" +) + +// PassthroughPaths carries filesystem locations a strategy may use to +// materialize MCP config for a passthrough (raw CLI) launch without writing to +// the user's global config. +type PassthroughPaths struct { + // TempConfigPath is a kandev-owned temp file path a strategy may write a + // config file to (used by Claude and OpenCode). Empty when unavailable. + TempConfigPath string + // WorkspaceDir is the agent's working directory (worktree root). Used by + // strategies that write a project-local config file (Cursor). Empty when + // unavailable. + WorkspaceDir string +} + +// PassthroughConfigFile is a config file a strategy wants materialized on disk. +type PassthroughConfigFile struct { + Path string + Content []byte + // MergeKey, when non-empty, means: if a file already exists at Path, merge + // Content's object at this top-level JSON key into the existing file's same + // key (our entries win on name collision, all other user keys preserved) + // rather than overwriting the whole file. Used by Cursor to append kandev's + // servers to a user's existing .cursor/mcp.json instead of clobbering it. + // When the file does not exist, Content is written as-is. + MergeKey string +} + +// PassthroughArtifacts is what a strategy produces for a single passthrough launch. +type PassthroughArtifacts struct { + Files []PassthroughConfigFile // config files to materialize + Args []string // extra argv tokens appended to the passthrough command + Env map[string]string // extra environment variables for the agent process +} + +// PassthroughMCPStrategy materializes resolved MCP servers into the CLI-specific +// shape (config files, CLI args, and/or env vars) for a passthrough launch, +// without writing to the user's global config. Each passthrough-capable agent +// declares the strategy that matches how its CLI loads MCP servers. +type PassthroughMCPStrategy interface { + BuildPassthroughMCP(servers []types.McpServer, paths PassthroughPaths) (PassthroughArtifacts, error) + // Describe returns a short, human-readable phrase naming the mechanism this + // strategy uses to inject MCP servers into the CLI (surfaced in the UI so + // users understand how kandev wires MCP for a passthrough agent). + Describe() string +} + +// isStdioServer reports whether the server is a stdio (command-based) server. +// Type is authoritative when set (ToACPServers always sets it); otherwise a +// server with a command and no URL is treated as stdio. +func isStdioServer(s types.McpServer) bool { + if s.Type != "" { + return s.Type == string(ServerTypeStdio) + } + return s.Command != "" && s.URL == "" +} + +// marshalMCPFile renders a config struct as pretty-printed JSON with a trailing +// newline, matching the existing passthrough config writer. +func marshalMCPFile(v any) ([]byte, error) { + data, err := json.MarshalIndent(v, "", " ") + if err != nil { + return nil, err + } + return append(data, '\n'), nil +} + +// --- Claude ------------------------------------------------------------------ + +// claudeReservedServerName is skipped by Claude Code at load time (it warns and +// ignores it), so we never emit it. +const claudeReservedServerName = "workspace" + +// claudeStreamableHTTPType is Claude Code's spelling of streamable HTTP (hyphen, +// not underscore); Claude treats it as an alias of "http". +const claudeStreamableHTTPType = "streamable-http" + +// ClaudeStrategy writes an `mcpServers` JSON file and points Claude Code at it +// via --mcp-config. Without Strict the servers are merged additively with the +// user's ~/.claude.json and project .mcp.json. With Strict, --strict-mcp-config +// is added so ONLY these servers load (the user's other MCP sources are ignored). +type ClaudeStrategy struct { + Strict bool +} + +type claudeMCPFile struct { + MCPServers map[string]claudeServerEntry `json:"mcpServers"` +} + +type claudeServerEntry struct { + Type string `json:"type"` + Command string `json:"command,omitempty"` + Args []string `json:"args,omitempty"` + Env map[string]string `json:"env,omitempty"` + URL string `json:"url,omitempty"` + Headers map[string]string `json:"headers,omitempty"` +} + +func (s ClaudeStrategy) BuildPassthroughMCP(servers []types.McpServer, paths PassthroughPaths) (PassthroughArtifacts, error) { + if len(servers) == 0 || paths.TempConfigPath == "" { + return PassthroughArtifacts{}, nil + } + entries := make(map[string]claudeServerEntry, len(servers)) + for _, srv := range servers { + if srv.Name == "" || srv.Name == claudeReservedServerName { + continue + } + entries[srv.Name] = claudeServerEntryFromServer(srv) + } + if len(entries) == 0 { + // Every server was filtered out (blank/reserved names); emit nothing + // rather than a `--mcp-config` flag pointing at an empty config. + return PassthroughArtifacts{}, nil + } + content, err := marshalMCPFile(claudeMCPFile{MCPServers: entries}) + if err != nil { + return PassthroughArtifacts{}, err + } + args := []string{"--mcp-config", paths.TempConfigPath} + if s.Strict { + args = append(args, "--strict-mcp-config") + } + return PassthroughArtifacts{ + Files: []PassthroughConfigFile{{Path: paths.TempConfigPath, Content: content}}, + Args: args, + }, nil +} + +func (ClaudeStrategy) Describe() string { + return "an MCP config file passed via the --mcp-config flag" +} + +func claudeServerEntryFromServer(srv types.McpServer) claudeServerEntry { + if isStdioServer(srv) { + return claudeServerEntry{Type: string(ServerTypeStdio), Command: srv.Command, Args: srv.Args, Env: srv.Env} + } + return claudeServerEntry{Type: claudeRemoteType(srv.Type), URL: srv.URL, Headers: srv.Headers} +} + +// claudeRemoteType maps kandev's transport names onto Claude Code's. Claude +// spells streamable HTTP with a hyphen ("streamable-http") and treats it as an +// alias of "http". +func claudeRemoteType(t string) string { + switch t { + case string(ServerTypeSSE): + return string(ServerTypeSSE) + case string(ServerTypeStreamableHTTP): + return claudeStreamableHTTPType + default: + return string(ServerTypeHTTP) + } +} + +// --- Codex ------------------------------------------------------------------- + +// CodexStrategy injects MCP servers via repeated `-c mcp_servers..=` +// CLI overrides. Codex has no MCP-config-file flag; `-c` overrides sit at the top +// of the config precedence stack and never modify config.toml. Codex parses each +// value as JSON when possible, so values are emitted as JSON. Transport is +// inferred from the presence of `command` vs `url` (there is no `type` key). +type CodexStrategy struct{} + +func (CodexStrategy) BuildPassthroughMCP(servers []types.McpServer, _ PassthroughPaths) (PassthroughArtifacts, error) { + args := make([]string, 0, len(servers)*4) + for _, srv := range servers { + if srv.Name == "" { + continue + } + serverArgs, err := codexServerArgs(srv) + if err != nil { + return PassthroughArtifacts{}, err + } + args = append(args, serverArgs...) + } + if len(args) == 0 { + return PassthroughArtifacts{}, nil + } + return PassthroughArtifacts{Args: args}, nil +} + +func (CodexStrategy) Describe() string { + return "repeated -c mcp_servers.* command-line overrides" +} + +// codexServerArgs returns the flat ["-c", "key=json", ...] tokens for one server. +func codexServerArgs(srv types.McpServer) ([]string, error) { + type pair struct { + key string + value any + } + var pairs []pair + if isStdioServer(srv) { + pairs = append(pairs, pair{"command", srv.Command}) + if len(srv.Args) > 0 { + pairs = append(pairs, pair{"args", srv.Args}) + } + if len(srv.Env) > 0 { + pairs = append(pairs, pair{"env", srv.Env}) + } + } else { + // Remote (url-based) server. Current Codex loads streamable-HTTP MCP + // servers from `url` natively. Some intermediate Codex versions gated + // this behind `experimental_use_rmcp_client = true`; if a pinned Codex + // build ever stops loading the kandev server, emit that override here. + pairs = append(pairs, pair{"url", srv.URL}) + if len(srv.Headers) > 0 { + pairs = append(pairs, pair{"http_headers", srv.Headers}) + } + } + // NOTE: env vars and headers are JSON-encoded into `-c` overrides, which + // land in the process argument list (visible via `ps aux`) — a local user + // on the host could read tokens this way. Codex has no file-based MCP config + // injection (unlike Claude/OpenCode), so the `-c` path is the only option. + args := make([]string, 0, len(pairs)*2) + for _, p := range pairs { + enc, err := json.Marshal(p.value) + if err != nil { + return nil, err + } + args = append(args, "-c", "mcp_servers."+codexKeyName(srv.Name)+"."+p.key+"="+string(enc)) + } + return args, nil +} + +// codexKeyName renders an MCP server name as a single TOML key segment for a +// `-c mcp_servers..` override. Names that are valid TOML bare keys +// are emitted as-is; names containing dots (or other non-bare characters) are +// quoted so Codex does not misread an embedded dot as extra key nesting (which +// would silently inject the server under the wrong path). +func codexKeyName(name string) string { + if isTOMLBareKey(name) { + return name + } + // Non-bare key: emit a quoted key. JSON string encoding produces + // TOML-compatible escaping (\", \\, \n, \t, \uXXXX, …), covering dots and + // control characters alike. + enc, err := json.Marshal(name) + if err != nil { + return `""` + } + return string(enc) +} + +func isTOMLBareKey(s string) bool { + if s == "" { + return false + } + for _, r := range s { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '_', r == '-': + default: + return false + } + } + return true +} + +// --- Cursor ------------------------------------------------------------------ + +// CursorStrategy writes a project-local .cursor/mcp.json into the workspace. +// cursor-agent auto-discovers it from the working directory. cursor-agent has no +// MCP-config flag and no reliable MCP env var, so a project file is the only +// non-global mechanism. When the file already exists, kandev's servers are +// merged into the existing `mcpServers` object (user entries preserved) via +// MergeKey rather than overwriting; when absent it is created. +type CursorStrategy struct{} + +type cursorMCPFile struct { + MCPServers map[string]cursorServerEntry `json:"mcpServers"` +} + +type cursorServerEntry struct { + Type string `json:"type,omitempty"` + Command string `json:"command,omitempty"` + Args []string `json:"args,omitempty"` + Env map[string]string `json:"env,omitempty"` + URL string `json:"url,omitempty"` + Headers map[string]string `json:"headers,omitempty"` +} + +func (CursorStrategy) BuildPassthroughMCP(servers []types.McpServer, paths PassthroughPaths) (PassthroughArtifacts, error) { + if len(servers) == 0 || paths.WorkspaceDir == "" { + return PassthroughArtifacts{}, nil + } + entries := make(map[string]cursorServerEntry, len(servers)) + for _, srv := range servers { + if srv.Name == "" { + continue + } + if isStdioServer(srv) { + entries[srv.Name] = cursorServerEntry{Type: string(ServerTypeStdio), Command: srv.Command, Args: srv.Args, Env: srv.Env} + } else { + entries[srv.Name] = cursorServerEntry{URL: srv.URL, Headers: srv.Headers} + } + } + if len(entries) == 0 { + return PassthroughArtifacts{}, nil + } + content, err := marshalMCPFile(cursorMCPFile{MCPServers: entries}) + if err != nil { + return PassthroughArtifacts{}, err + } + return PassthroughArtifacts{ + Files: []PassthroughConfigFile{{ + Path: filepath.Join(paths.WorkspaceDir, ".cursor", "mcp.json"), + Content: content, + MergeKey: "mcpServers", + }}, + }, nil +} + +func (CursorStrategy) Describe() string { + return "a project-local .cursor/mcp.json file (merged into an existing one)" +} + +// --- OpenCode ---------------------------------------------------------------- + +const ( + opencodeConfigSchema = "https://opencode.ai/config.json" + opencodeConfigEnvVar = "OPENCODE_CONFIG" + // opencode's MCP server discriminator values (distinct from kandev's + // transport names): stdio servers are "local", all remote transports "remote". + opencodeServerTypeLocal = "local" + opencodeServerTypeRemote = "remote" +) + +// OpenCodeStrategy writes an opencode JSON config (an `mcp` block) to a temp file +// and points opencode at it via the OPENCODE_CONFIG env var. opencode has no +// config CLI flag; it merges OPENCODE_CONFIG over the global config without +// modifying ~/.config/opencode. opencode collapses all remote transports into a +// single "remote" type and uses "local" for stdio (command is a string array). +type OpenCodeStrategy struct{} + +type opencodeMCPFile struct { + Schema string `json:"$schema"` + MCP map[string]opencodeServerEntry `json:"mcp"` +} + +type opencodeServerEntry struct { + Type string `json:"type"` + Command []string `json:"command,omitempty"` + Environment map[string]string `json:"environment,omitempty"` + URL string `json:"url,omitempty"` + Headers map[string]string `json:"headers,omitempty"` + Enabled bool `json:"enabled"` +} + +func (OpenCodeStrategy) BuildPassthroughMCP(servers []types.McpServer, paths PassthroughPaths) (PassthroughArtifacts, error) { + if len(servers) == 0 || paths.TempConfigPath == "" { + return PassthroughArtifacts{}, nil + } + entries := make(map[string]opencodeServerEntry, len(servers)) + for _, srv := range servers { + if srv.Name == "" { + continue + } + if isStdioServer(srv) { + command := append([]string{srv.Command}, srv.Args...) + entries[srv.Name] = opencodeServerEntry{Type: opencodeServerTypeLocal, Command: command, Environment: srv.Env, Enabled: true} + } else { + entries[srv.Name] = opencodeServerEntry{Type: opencodeServerTypeRemote, URL: srv.URL, Headers: srv.Headers, Enabled: true} + } + } + if len(entries) == 0 { + return PassthroughArtifacts{}, nil + } + content, err := marshalMCPFile(opencodeMCPFile{Schema: opencodeConfigSchema, MCP: entries}) + if err != nil { + return PassthroughArtifacts{}, err + } + return PassthroughArtifacts{ + Files: []PassthroughConfigFile{{Path: paths.TempConfigPath, Content: content}}, + Env: map[string]string{opencodeConfigEnvVar: paths.TempConfigPath}, + }, nil +} + +func (OpenCodeStrategy) Describe() string { + return "a temp MCP config file referenced by the OPENCODE_CONFIG env var" +} + +// --- Merge helper ------------------------------------------------------------ + +// MergeJSONUnderKey merges the object at `key` from `ours` into the same key of +// `existing`, returning the updated `existing` document. All other top-level +// keys in `existing` are preserved; within `key`, every other entry is +// preserved and our entries win on name collision. Both inputs must be JSON +// objects, and `existing[key]` (if present) must be an object — otherwise an +// error is returned so the caller can leave the user's file untouched rather +// than clobber malformed content. Output is pretty-printed with a trailing +// newline to match the other writers. +func MergeJSONUnderKey(existing, ours []byte, key string) ([]byte, error) { + existingDoc := map[string]json.RawMessage{} + if err := json.Unmarshal(existing, &existingDoc); err != nil { + return nil, fmt.Errorf("existing config is not a JSON object: %w", err) + } + oursDoc := map[string]json.RawMessage{} + if err := json.Unmarshal(ours, &oursDoc); err != nil { + return nil, fmt.Errorf("generated config is not a JSON object: %w", err) + } + + entries := map[string]json.RawMessage{} + if raw, ok := existingDoc[key]; ok { + if err := json.Unmarshal(raw, &entries); err != nil { + return nil, fmt.Errorf("existing %q is not an object: %w", key, err) + } + } + ourEntries := map[string]json.RawMessage{} + if raw, ok := oursDoc[key]; ok { + if err := json.Unmarshal(raw, &ourEntries); err != nil { + return nil, fmt.Errorf("generated %q is not an object: %w", key, err) + } + } + for name, entry := range ourEntries { + entries[name] = entry + } + + mergedKey, err := json.Marshal(entries) + if err != nil { + return nil, err + } + existingDoc[key] = mergedKey + return marshalMCPFile(existingDoc) +} diff --git a/apps/backend/internal/agent/mcpconfig/passthrough_test.go b/apps/backend/internal/agent/mcpconfig/passthrough_test.go new file mode 100644 index 000000000..a8bb436ca --- /dev/null +++ b/apps/backend/internal/agent/mcpconfig/passthrough_test.go @@ -0,0 +1,359 @@ +package mcpconfig + +import ( + "encoding/json" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/kandev/kandev/internal/agentctl/types" +) + +var sampleServers = []types.McpServer{ + {Name: "kandev", Type: "http", URL: "http://localhost:1234/mcp"}, + {Name: "github", Type: "stdio", Command: "npx", Args: []string{"-y", "@mcp/github"}, Env: map[string]string{"GITHUB_TOKEN": "tok"}}, + {Name: "stream", Type: "streamable_http", URL: "https://x/mcp", Headers: map[string]string{"Authorization": "Bearer t"}}, +} + +// --- Claude ------------------------------------------------------------------ + +func TestClaudeStrategy_FileAndFlags(t *testing.T) { + art, err := ClaudeStrategy{}.BuildPassthroughMCP(sampleServers, PassthroughPaths{TempConfigPath: "/tmp/cfg.json"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(art.Files) != 1 || art.Files[0].Path != "/tmp/cfg.json" { + t.Fatalf("expected one file at /tmp/cfg.json, got %+v", art.Files) + } + if art.Files[0].MergeKey != "" { + t.Error("claude temp file should overwrite, not merge") + } + wantArgs := []string{"--mcp-config", "/tmp/cfg.json"} + if !reflect.DeepEqual(art.Args, wantArgs) { + t.Errorf("Args = %v, want %v", art.Args, wantArgs) + } + + var got claudeMCPFile + if err := json.Unmarshal(art.Files[0].Content, &got); err != nil { + t.Fatalf("content not valid JSON: %v", err) + } + if got.MCPServers["kandev"].Type != "http" || got.MCPServers["kandev"].URL != "http://localhost:1234/mcp" { + t.Errorf("kandev entry = %+v", got.MCPServers["kandev"]) + } + gh := got.MCPServers["github"] + if gh.Type != "stdio" || gh.Command != "npx" || gh.Env["GITHUB_TOKEN"] != "tok" { + t.Errorf("github entry = %+v", gh) + } + // kandev's streamable_http must normalize to the hyphenated claude spelling. + if got.MCPServers["stream"].Type != "streamable-http" { + t.Errorf("stream Type = %q, want streamable-http", got.MCPServers["stream"].Type) + } +} + +func TestClaudeStrategy_RemoteTransportMapping(t *testing.T) { + servers := []types.McpServer{ + {Name: "sse", Type: "sse", URL: "https://x/sse"}, + {Name: "http", Type: "http", URL: "https://x/http"}, + {Name: "stream", Type: "streamable_http", URL: "https://x/stream"}, + } + art, _ := ClaudeStrategy{}.BuildPassthroughMCP(servers, PassthroughPaths{TempConfigPath: "/tmp/c.json"}) + var got claudeMCPFile + if err := json.Unmarshal(art.Files[0].Content, &got); err != nil { + t.Fatalf("content not valid JSON: %v", err) + } + if got.MCPServers["sse"].Type != "sse" { + t.Errorf("sse Type = %q, want sse", got.MCPServers["sse"].Type) + } + if got.MCPServers["http"].Type != "http" { + t.Errorf("http Type = %q, want http", got.MCPServers["http"].Type) + } + // kandev spells streamable HTTP with a hyphen and treats it as an alias of http. + if got.MCPServers["stream"].Type != "streamable-http" { + t.Errorf("stream Type = %q, want streamable-http", got.MCPServers["stream"].Type) + } +} + +func TestClaudeStrategy_Strict(t *testing.T) { + art, _ := ClaudeStrategy{Strict: true}.BuildPassthroughMCP(sampleServers, PassthroughPaths{TempConfigPath: "/tmp/cfg.json"}) + if got := strings.Join(art.Args, " "); got != "--mcp-config /tmp/cfg.json --strict-mcp-config" { + t.Errorf("Args = %q", got) + } +} + +func TestClaudeStrategy_SkipsReservedAndEmpty(t *testing.T) { + servers := []types.McpServer{ + {Name: "workspace", Type: "stdio", Command: "x"}, + {Name: "", Type: "stdio", Command: "y"}, + {Name: "ok", Type: "stdio", Command: "z"}, + } + art, _ := ClaudeStrategy{}.BuildPassthroughMCP(servers, PassthroughPaths{TempConfigPath: "/tmp/c.json"}) + var got claudeMCPFile + _ = json.Unmarshal(art.Files[0].Content, &got) + if _, ok := got.MCPServers["workspace"]; ok { + t.Error("reserved name 'workspace' must be skipped") + } + if len(got.MCPServers) != 1 { + t.Errorf("expected only 'ok', got %v", got.MCPServers) + } +} + +func TestClaudeStrategy_NoPathOrNoServers(t *testing.T) { + if art, _ := (ClaudeStrategy{}).BuildPassthroughMCP(sampleServers, PassthroughPaths{}); len(art.Files) != 0 || len(art.Args) != 0 { + t.Error("no temp path should produce no artifacts") + } + if art, _ := (ClaudeStrategy{}).BuildPassthroughMCP(nil, PassthroughPaths{TempConfigPath: "/tmp/c.json"}); len(art.Files) != 0 { + t.Error("no servers should produce no artifacts") + } +} + +func TestIsStdioServer_TypeFallback(t *testing.T) { + cases := []struct { + name string + srv types.McpServer + want bool + }{ + {"explicit stdio", types.McpServer{Type: "stdio", Command: "x"}, true}, + {"explicit http", types.McpServer{Type: "http", URL: "https://x"}, false}, + {"untyped command only", types.McpServer{Command: "x"}, true}, + {"untyped url only", types.McpServer{URL: "https://x"}, false}, + {"untyped command+url treated as remote", types.McpServer{Command: "x", URL: "https://x"}, false}, + } + for _, tc := range cases { + if got := isStdioServer(tc.srv); got != tc.want { + t.Errorf("%s: isStdioServer = %v, want %v", tc.name, got, tc.want) + } + } +} + +// --- Codex ------------------------------------------------------------------- + +func TestCodexStrategy_Args(t *testing.T) { + art, err := CodexStrategy{}.BuildPassthroughMCP(sampleServers, PassthroughPaths{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(art.Files) != 0 || len(art.Env) != 0 { + t.Error("codex strategy must not write files or env") + } + joined := strings.Join(art.Args, " ") + // stdio server: command + args + env + for _, want := range []string{ + `-c mcp_servers.kandev.url="http://localhost:1234/mcp"`, + `-c mcp_servers.github.command="npx"`, + `-c mcp_servers.github.args=["-y","@mcp/github"]`, + `-c mcp_servers.github.env={"GITHUB_TOKEN":"tok"}`, + `-c mcp_servers.stream.url="https://x/mcp"`, + `-c mcp_servers.stream.http_headers={"Authorization":"Bearer t"}`, + } { + if !strings.Contains(joined, want) { + t.Errorf("missing arg %q in:\n%s", want, joined) + } + } + // Every value token must be preceded by a -c flag. + for i := 0; i < len(art.Args); i += 2 { + if art.Args[i] != "-c" { + t.Fatalf("arg %d = %q, want -c (args: %v)", i, art.Args[i], art.Args) + } + } +} + +func TestCodexStrategy_OmitsEmptyArgsAndEnv(t *testing.T) { + servers := []types.McpServer{{Name: "bare", Type: "stdio", Command: "run"}} + art, _ := CodexStrategy{}.BuildPassthroughMCP(servers, PassthroughPaths{}) + if got := strings.Join(art.Args, " "); got != `-c mcp_servers.bare.command="run"` { + t.Errorf("Args = %q", got) + } +} + +func TestCodexStrategy_QuotesServerNamesWithDots(t *testing.T) { + servers := []types.McpServer{{Name: "my.server", Type: "stdio", Command: "run"}} + art, _ := CodexStrategy{}.BuildPassthroughMCP(servers, PassthroughPaths{}) + // A dotted name must be TOML-quoted so codex doesn't read the dot as nesting. + if got := strings.Join(art.Args, " "); got != `-c mcp_servers."my.server".command="run"` { + t.Errorf("Args = %q", got) + } +} + +// --- Cursor ------------------------------------------------------------------ + +func TestCursorStrategy_ProjectFileMerge(t *testing.T) { + art, err := CursorStrategy{}.BuildPassthroughMCP(sampleServers, PassthroughPaths{WorkspaceDir: "/work"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(art.Args) != 0 || len(art.Env) != 0 { + t.Error("cursor strategy must not produce args or env") + } + if len(art.Files) != 1 { + t.Fatalf("expected one file, got %d", len(art.Files)) + } + f := art.Files[0] + if f.Path != filepath.Join("/work", ".cursor", "mcp.json") { + t.Errorf("Path = %q", f.Path) + } + if f.MergeKey != "mcpServers" { + t.Errorf("cursor file must merge under mcpServers, got MergeKey=%q", f.MergeKey) + } + var got cursorMCPFile + if err := json.Unmarshal(f.Content, &got); err != nil { + t.Fatalf("content not valid JSON: %v", err) + } + if got.MCPServers["github"].Type != "stdio" || got.MCPServers["github"].Command != "npx" { + t.Errorf("github entry = %+v", got.MCPServers["github"]) + } + // Remote entries carry no type, just url+headers. + if got.MCPServers["stream"].Type != "" || got.MCPServers["stream"].URL != "https://x/mcp" { + t.Errorf("stream entry = %+v", got.MCPServers["stream"]) + } +} + +func TestCursorStrategy_NoWorkspace(t *testing.T) { + if art, _ := (CursorStrategy{}).BuildPassthroughMCP(sampleServers, PassthroughPaths{}); len(art.Files) != 0 { + t.Error("no workspace dir should produce no artifacts") + } +} + +func TestMergeJSONUnderKey(t *testing.T) { + existing := []byte(`{"mcpServers":{"user-srv":{"url":"https://user"}},"otherTop":"keep"}`) + ours := []byte(`{"mcpServers":{"kandev":{"type":"http","url":"http://localhost:1/mcp"}}}`) + + merged, err := MergeJSONUnderKey(existing, ours, "mcpServers") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + var got map[string]json.RawMessage + if err := json.Unmarshal(merged, &got); err != nil { + t.Fatalf("merged output not JSON: %v", err) + } + if string(got["otherTop"]) != `"keep"` { + t.Errorf("top-level user key not preserved: %s", merged) + } + var servers map[string]map[string]string + if err := json.Unmarshal(got["mcpServers"], &servers); err != nil { + t.Fatalf("mcpServers not an object: %v", err) + } + if servers["user-srv"]["url"] != "https://user" { + t.Errorf("user server dropped: %s", merged) + } + if servers["kandev"]["url"] != "http://localhost:1/mcp" { + t.Errorf("kandev server not merged in: %s", merged) + } +} + +func TestMergeJSONUnderKey_OursWinOnCollision(t *testing.T) { + existing := []byte(`{"mcpServers":{"kandev":{"url":"http://stale"}}}`) + ours := []byte(`{"mcpServers":{"kandev":{"url":"http://fresh"}}}`) + merged, err := MergeJSONUnderKey(existing, ours, "mcpServers") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(string(merged), "http://fresh") || strings.Contains(string(merged), "http://stale") { + t.Errorf("our entry must win on name collision: %s", merged) + } +} + +func TestMergeJSONUnderKey_MalformedExistingErrors(t *testing.T) { + if _, err := MergeJSONUnderKey([]byte(`not json`), []byte(`{"mcpServers":{}}`), "mcpServers"); err == nil { + t.Error("expected error for malformed existing JSON") + } + if _, err := MergeJSONUnderKey([]byte(`{"mcpServers":[]}`), []byte(`{"mcpServers":{}}`), "mcpServers"); err == nil { + t.Error("expected error when existing mcpServers is not an object") + } +} + +// --- OpenCode ---------------------------------------------------------------- + +func TestOpenCodeStrategy_FileAndEnv(t *testing.T) { + art, err := OpenCodeStrategy{}.BuildPassthroughMCP(sampleServers, PassthroughPaths{TempConfigPath: "/tmp/oc.json"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(art.Args) != 0 { + t.Error("opencode strategy uses env, not args") + } + if art.Env[opencodeConfigEnvVar] != "/tmp/oc.json" { + t.Errorf("OPENCODE_CONFIG = %q, want /tmp/oc.json", art.Env[opencodeConfigEnvVar]) + } + if len(art.Files) != 1 || art.Files[0].Path != "/tmp/oc.json" { + t.Fatalf("expected one file at /tmp/oc.json, got %+v", art.Files) + } + var got opencodeMCPFile + if err := json.Unmarshal(art.Files[0].Content, &got); err != nil { + t.Fatalf("content not valid JSON: %v", err) + } + if got.Schema != opencodeConfigSchema { + t.Errorf("$schema = %q", got.Schema) + } + // remote http server -> type "remote" + if got.MCP["kandev"].Type != "remote" || got.MCP["kandev"].URL != "http://localhost:1234/mcp" || !got.MCP["kandev"].Enabled { + t.Errorf("kandev entry = %+v", got.MCP["kandev"]) + } + // stdio -> type "local", command is [cmd, ...args] + gh := got.MCP["github"] + if gh.Type != "local" || !reflect.DeepEqual(gh.Command, []string{"npx", "-y", "@mcp/github"}) { + t.Errorf("github entry = %+v", gh) + } + if gh.Environment["GITHUB_TOKEN"] != "tok" { + t.Errorf("github env = %+v", gh.Environment) + } +} + +func TestOpenCodeStrategy_NoPath(t *testing.T) { + if art, _ := (OpenCodeStrategy{}).BuildPassthroughMCP(sampleServers, PassthroughPaths{}); len(art.Files) != 0 || len(art.Env) != 0 { + t.Error("no temp path should produce no artifacts") + } +} + +// When every server is filtered out (blank/reserved names), the file-based +// strategies must emit nothing — no empty config file, flag, or env var. +func TestStrategies_NoArtifactsWhenAllServersFiltered(t *testing.T) { + filtered := []types.McpServer{{Name: "", Type: "stdio", Command: "x"}} + + if art, _ := (ClaudeStrategy{}).BuildPassthroughMCP(filtered, PassthroughPaths{TempConfigPath: "/tmp/c.json"}); len(art.Files) != 0 || len(art.Args) != 0 { + t.Errorf("claude: want no artifacts, got %+v", art) + } + if art, _ := (CursorStrategy{}).BuildPassthroughMCP(filtered, PassthroughPaths{WorkspaceDir: "/work"}); len(art.Files) != 0 { + t.Errorf("cursor: want no artifacts, got %+v", art) + } + if art, _ := (OpenCodeStrategy{}).BuildPassthroughMCP(filtered, PassthroughPaths{TempConfigPath: "/tmp/oc.json"}); len(art.Files) != 0 || len(art.Env) != 0 { + t.Errorf("opencode: want no artifacts, got %+v", art) + } + // Claude additionally filters the reserved "workspace" name. + reserved := []types.McpServer{{Name: "workspace", Type: "stdio", Command: "y"}} + if art, _ := (ClaudeStrategy{}).BuildPassthroughMCP(reserved, PassthroughPaths{TempConfigPath: "/tmp/c.json"}); len(art.Files) != 0 || len(art.Args) != 0 { + t.Errorf("claude reserved-only: want no artifacts, got %+v", art) + } +} + +// All four strategies satisfy the interface. +func TestStrategiesImplementInterface(t *testing.T) { + var _ PassthroughMCPStrategy = ClaudeStrategy{} + var _ PassthroughMCPStrategy = CodexStrategy{} + var _ PassthroughMCPStrategy = CursorStrategy{} + var _ PassthroughMCPStrategy = OpenCodeStrategy{} +} + +// Each strategy reports a non-empty, distinct human-readable injection mechanism. +func TestStrategiesDescribe(t *testing.T) { + cases := map[string]struct { + strategy PassthroughMCPStrategy + want string + }{ + "claude": {ClaudeStrategy{}, "an MCP config file passed via the --mcp-config flag"}, + "codex": {CodexStrategy{}, "repeated -c mcp_servers.* command-line overrides"}, + "cursor": {CursorStrategy{}, "a project-local .cursor/mcp.json file (merged into an existing one)"}, + "opencode": {OpenCodeStrategy{}, "a temp MCP config file referenced by the OPENCODE_CONFIG env var"}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := tc.strategy.Describe() + if got == "" { + t.Fatalf("%s: Describe() returned empty string", name) + } + if got != tc.want { + t.Errorf("%s: Describe() = %q, want %q", name, got, tc.want) + } + }) + } +} diff --git a/apps/backend/internal/agent/runtime/agentctl/agent.go b/apps/backend/internal/agent/runtime/agentctl/agent.go index 01235db88..964a99cab 100644 --- a/apps/backend/internal/agent/runtime/agentctl/agent.go +++ b/apps/backend/internal/agent/runtime/agentctl/agent.go @@ -319,6 +319,14 @@ func (c *Client) StreamUpdates(ctx context.Context, handler func(AgentEvent), mc return nil } +// HasAgentStream reports whether the agent updates WebSocket is currently +// connected. Used to avoid opening a second stream on resume/restart. +func (c *Client) HasAgentStream() bool { + c.mu.Lock() + defer c.mu.Unlock() + return c.agentStreamConn != nil +} + // readUpdatesStream is the read loop for the agent updates WebSocket stream. func (c *Client) readUpdatesStream( ctx context.Context, diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go index 5656e8736..096980801 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough.go @@ -2,7 +2,6 @@ package lifecycle import ( "context" - "encoding/json" "fmt" "os" "path/filepath" @@ -13,6 +12,7 @@ import ( "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/agent/executor" + "github.com/kandev/kandev/internal/agent/mcpconfig" agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" "github.com/kandev/kandev/internal/agent/settings/cliflags" "github.com/kandev/kandev/internal/agentctl/server/process" @@ -148,6 +148,11 @@ func (m *Manager) buildPassthroughEnv(ctx context.Context, execution *AgentExecu } } } + // Merge env vars contributed by the passthrough MCP strategy (e.g. opencode's + // OPENCODE_CONFIG). Set during command building in applyPassthroughMCP. + for key, value := range getPassthroughMCPEnv(execution) { + env[key] = value + } return env } @@ -170,11 +175,12 @@ func (m *Manager) startPassthroughShell(ctx context.Context, execution *AgentExe // resolvedPassthrough holds the agent config, passthrough config, runtime config, and profile // info resolved from an execution. Used as the basis for building passthrough commands. type resolvedPassthrough struct { - agentID string - agent agents.PassthroughAgent - pt agents.PassthroughConfig - rt *agents.RuntimeConfig - profile *AgentProfileInfo + agentID string + agentConfig agents.Agent + agent agents.PassthroughAgent + pt agents.PassthroughConfig + rt *agents.RuntimeConfig + profile *AgentProfileInfo } // resolvePassthroughAgent loads the agent config and profile for a passthrough execution. @@ -196,11 +202,12 @@ func (m *Manager) resolvePassthroughAgent(ctx context.Context, execution *AgentE } return &resolvedPassthrough{ - agentID: agentConfig.ID(), - agent: ptAgent, - pt: ptAgent.PassthroughConfig(), - rt: agentConfig.Runtime(), - profile: profileInfo, + agentID: agentConfig.ID(), + agentConfig: agentConfig, + agent: ptAgent, + pt: ptAgent.PassthroughConfig(), + rt: agentConfig.Runtime(), + profile: profileInfo, }, nil } @@ -217,13 +224,44 @@ func promptForPassthroughCommand(pt agents.PassthroughConfig, taskDescription st return taskDescription } -type passthroughMCPServerConfig struct { - Type string `json:"type"` - URL string `json:"url"` +const ( + // metadataKeyPassthroughMCPFiles tracks config files kandev wrote for the + // passthrough MCP injection so they can be removed when the execution ends. + metadataKeyPassthroughMCPFiles = "passthrough_mcp_files" + // metadataKeyPassthroughMCPEnv carries env vars the MCP strategy needs on the + // agent process (e.g. opencode's OPENCODE_CONFIG), merged in buildPassthroughEnv. + metadataKeyPassthroughMCPEnv = "passthrough_mcp_env" + // kandevMCPServerName is the reserved name of kandev's own HTTP MCP server, + // which exposes the task tools to the agent. + kandevMCPServerName = "kandev" +) + +// redactPassthroughArgs masks secret-bearing MCP override values before the +// command is logged. Codex injects MCP servers via `-c mcp_servers..=` +// argv (no file-based option), so env vars and HTTP headers — which commonly +// carry tokens — would otherwise be written verbatim into backend logs. The +// real (unredacted) args are still what's executed; only the log copy is masked. +func redactPassthroughArgs(args []string) []string { + out := make([]string, len(args)) + for i, a := range args { + out[i] = redactMCPArg(a) + } + return out } -type passthroughMCPConfigFile struct { - MCPServers map[string]passthroughMCPServerConfig `json:"mcpServers"` +func redactMCPArg(arg string) string { + if !strings.HasPrefix(arg, "mcp_servers.") { + return arg + } + eq := strings.IndexByte(arg, '=') + if eq < 0 { + return arg + } + key := arg[:eq] + if strings.HasSuffix(key, ".env") || strings.HasSuffix(key, ".http_headers") { + return key + "=" + } + return arg } func passthroughMCPConfigPort(execution *AgentExecution) int { @@ -269,76 +307,321 @@ func safePassthroughMCPConfigName(value string) string { return out.String() } -func passthroughMCPConfigPath(execution *AgentExecution) string { - if execution == nil || execution.Metadata == nil { - return "" +// applyPassthroughMCP resolves the session's MCP servers (kandev's own server +// plus the profile's configured servers), runs the agent's passthrough MCP +// strategy, materializes any config files, records them for cleanup, stores the +// strategy's env vars on the execution (merged later in buildPassthroughEnv), +// and returns the extra CLI args to append to the passthrough command. It is a +// no-op for agents that declare no strategy. +func (m *Manager) applyPassthroughMCP(ctx context.Context, execution *AgentExecution, pt agents.PassthroughConfig, agentConfig agents.Agent) ([]string, error) { + if pt.MCPStrategy == nil { + return nil, nil + } + // passthroughMCPServers always returns at least the kandev server (or an + // error when the port is unavailable), so the strategy receives a non-empty + // list; each strategy guards its own empty-after-filtering case. + servers, err := m.passthroughMCPServers(ctx, execution, agentConfig) + if err != nil { + return nil, err + } + artifacts, err := pt.MCPStrategy.BuildPassthroughMCP(servers, m.passthroughMCPPaths(execution)) + if err != nil { + return nil, fmt.Errorf("build passthrough MCP config: %w", err) + } + if err := m.writePassthroughMCPFiles(execution, artifacts.Files); err != nil { + return nil, err } - path, _ := execution.Metadata["passthrough_mcp_config_path"].(string) - return path + setPassthroughMCPEnv(execution, artifacts.Env) + return artifacts.Args, nil } -func (m *Manager) writePassthroughMCPConfig(execution *AgentExecution, pt agents.PassthroughConfig) (string, error) { - if pt.MCPConfigFlag.IsEmpty() { - return "", nil - } +// passthroughMCPServers returns kandev's own HTTP MCP server followed by the +// profile's resolved MCP servers. The kandev server requires the standalone +// port; a profile server named "kandev" is dropped so it cannot shadow ours. +func (m *Manager) passthroughMCPServers(ctx context.Context, execution *AgentExecution, agentConfig agents.Agent) ([]agentctltypes.McpServer, error) { port := passthroughMCPConfigPort(execution) if port <= 0 { - return "", fmt.Errorf("standalone port unavailable for passthrough MCP config") + return nil, fmt.Errorf("standalone port unavailable for passthrough MCP config") + } + servers := []agentctltypes.McpServer{{ + Name: kandevMCPServerName, + Type: string(mcpconfig.ServerTypeHTTP), + URL: fmt.Sprintf("http://localhost:%d/mcp", port), + }} + profileServers, err := m.resolveMcpServersWithParams(ctx, execution.AgentProfileID, execution.Metadata, agentConfig) + if err != nil { + return nil, err + } + for _, srv := range profileServers { + if srv.Name == kandevMCPServerName { + continue + } + servers = append(servers, srv) } + return servers, nil +} + +// passthroughMCPPaths computes the filesystem locations a strategy may use: a +// kandev-owned temp config path (for file+flag / file+env strategies) and the +// workspace dir (for project-local file strategies like Cursor). +func (m *Manager) passthroughMCPPaths(execution *AgentExecution) mcpconfig.PassthroughPaths { root := m.dataDir if root == "" { root = filepath.Join(os.TempDir(), "kandev") } - dir := filepath.Join(root, "passthrough-mcp") - if err := os.MkdirAll(dir, 0o700); err != nil { - return "", fmt.Errorf("create passthrough MCP config dir: %w", err) - } name := safePassthroughMCPConfigName(execution.SessionID) if execution.SessionID == "" { name = safePassthroughMCPConfigName(execution.ID) } - path := filepath.Join(dir, name+".json") - cfg := passthroughMCPConfigFile{ - MCPServers: map[string]passthroughMCPServerConfig{ - "kandev": { - Type: "http", - URL: fmt.Sprintf("http://localhost:%d/mcp", port), - }, - }, + return mcpconfig.PassthroughPaths{ + TempConfigPath: filepath.Join(root, "passthrough-mcp", name+".json"), + WorkspaceDir: execution.WorkspacePath, } - data, err := json.MarshalIndent(cfg, "", " ") +} + +// writePassthroughMCPFiles materializes the strategy's config files and records +// every file kandev OWNS (created or overwrote) for cleanup. Files merged into a +// pre-existing user file (Cursor) are not tracked — kandev must not delete the +// user's file on teardown. +func (m *Manager) writePassthroughMCPFiles(execution *AgentExecution, files []mcpconfig.PassthroughConfigFile) error { + written := getPassthroughMCPFiles(execution) + for _, f := range files { + if f.Path == "" { + continue + } + ok, err := m.materializePassthroughFile(execution, f) + if err != nil { + return err + } + if ok { + written = appendUnique(written, f.Path) + } + } + setPassthroughMCPFiles(execution, written) + return nil +} + +// materializePassthroughFile writes one config file and reports whether kandev +// OWNS the result (true = track for cleanup). It refuses to write through an +// existing symlink (a malicious repo could point it outside the worktree), +// guards against a symlinked parent escaping the worktree, and creates new files +// with O_EXCL. For MergeKey files (Cursor) that already exist, kandev's servers +// are merged into the user's file (preserving their entries) and the file is NOT +// tracked for cleanup since it is the user's. +func (m *Manager) materializePassthroughFile(execution *AgentExecution, f mcpconfig.PassthroughConfigFile) (bool, error) { + if escapes, err := workspacePathEscapes(execution.WorkspacePath, f.Path); err != nil { + return false, fmt.Errorf("validate passthrough MCP config path: %w", err) + } else if escapes { + m.logger.Warn("passthrough MCP config path escapes workspace via symlink; skipping", + zap.String("path", f.Path)) + return false, nil + } + + info, statErr := os.Lstat(f.Path) + switch { + case statErr == nil && info.Mode()&os.ModeSymlink != 0: + // Never write through an existing symlink — it could redirect the write + // outside the worktree. Applies to both merge and create. + m.logger.Warn("passthrough MCP config is a symlink; leaving it untouched", + zap.String("path", f.Path)) + return false, nil + case statErr == nil && f.MergeKey != "": + // Merge kandev's servers into the user's existing regular file. Not + // tracked — it's the user's file; we only appended our entries. + return false, m.mergePassthroughConfig(f) + case statErr == nil: + // Existing kandev-owned temp file (Claude/OpenCode) — overwrite it. + if err := os.WriteFile(f.Path, f.Content, 0o600); err != nil { + return false, fmt.Errorf("write passthrough MCP config: %w", err) + } + return true, nil + case !os.IsNotExist(statErr): + return false, fmt.Errorf("lstat passthrough MCP config: %w", statErr) + default: + if err := os.MkdirAll(filepath.Dir(f.Path), 0o700); err != nil { + return false, fmt.Errorf("create passthrough MCP config dir: %w", err) + } + return m.writeFileNoFollow(f.Path, f.Content) + } +} + +// mergePassthroughConfig merges kandev's servers (f.Content's f.MergeKey object) +// into the existing regular file at f.Path, preserving the user's other entries. +// A malformed or unreadable existing file is left untouched (logged), never +// clobbered. The file is confirmed to be a regular file (not a symlink) by the +// caller before this runs. +func (m *Manager) mergePassthroughConfig(f mcpconfig.PassthroughConfigFile) error { + existing, err := os.ReadFile(f.Path) if err != nil { - return "", fmt.Errorf("marshal passthrough MCP config: %w", err) + m.logger.Warn("cannot read existing MCP config to merge; leaving it untouched", + zap.String("path", f.Path), zap.Error(err)) + return nil } - data = append(data, '\n') - if err := os.WriteFile(path, data, 0o600); err != nil { - return "", fmt.Errorf("write passthrough MCP config: %w", err) + merged, err := mcpconfig.MergeJSONUnderKey(existing, f.Content, f.MergeKey) + if err != nil { + m.logger.Warn("cannot merge into existing MCP config; leaving it untouched", + zap.String("path", f.Path), zap.Error(err)) + return nil } + if err := os.WriteFile(f.Path, merged, 0o600); err != nil { + return fmt.Errorf("write merged passthrough MCP config: %w", err) + } + return nil +} + +// writeFileNoFollow creates path with O_EXCL so it never follows or overwrites +// an existing leaf (including a symlink). A concurrently-created file is treated +// as "leave it alone" rather than an error. +func (m *Manager) writeFileNoFollow(path string, content []byte) (bool, error) { + file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) + if err != nil { + if os.IsExist(err) { + m.logger.Info("passthrough MCP config appeared concurrently; leaving it untouched", + zap.String("path", path)) + return false, nil + } + return false, fmt.Errorf("create passthrough MCP config: %w", err) + } + if _, werr := file.Write(content); werr != nil { + _ = file.Close() + // Remove the empty file so a later SkipIfExists probe doesn't see it and + // silently skip writing the real config. + _ = os.Remove(path) + return false, fmt.Errorf("write passthrough MCP config: %w", werr) + } + if cerr := file.Close(); cerr != nil { + _ = os.Remove(path) + return false, fmt.Errorf("close passthrough MCP config: %w", cerr) + } + return true, nil +} + +// workspacePathEscapes reports whether path — assumed to live under +// workspaceDir — would, after resolving symlinks on its deepest existing +// ancestor, land outside workspaceDir. Files not lexically under workspaceDir +// (kandev's own temp configs) are exempt and return false. An empty +// workspaceDir disables the check. +func workspacePathEscapes(workspaceDir, path string) (bool, error) { + if workspaceDir == "" { + return false, nil + } + cleanWS := filepath.Clean(workspaceDir) + cleanPath := filepath.Clean(path) + if rel, err := filepath.Rel(cleanWS, cleanPath); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return false, nil // not a workspace-relative file; not our concern + } + canonWS, err := filepath.EvalSymlinks(cleanWS) + if err != nil { + return false, fmt.Errorf("resolve workspace dir: %w", err) + } + // Resolve the deepest existing ancestor of the target (the leaf and some + // parents may not exist yet — those get created as real dirs). + ancestor := filepath.Dir(cleanPath) + for { + resolved, err := filepath.EvalSymlinks(ancestor) + if err == nil { + rel, relErr := filepath.Rel(canonWS, resolved) + escaped := relErr != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) + return escaped, nil + } + if !os.IsNotExist(err) { + return false, fmt.Errorf("resolve ancestor %q: %w", ancestor, err) + } + parent := filepath.Dir(ancestor) + if parent == ancestor { + return false, nil // reached filesystem root with nothing existing + } + ancestor = parent + } +} + +func (m *Manager) cleanupPassthroughMCPConfig(execution *AgentExecution) { + for _, path := range getPassthroughMCPFiles(execution) { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + m.logger.Warn("failed to remove passthrough MCP config", + zap.String("path", path), + zap.Error(err)) + } + } + if execution.Metadata != nil { + delete(execution.Metadata, metadataKeyPassthroughMCPFiles) + delete(execution.Metadata, metadataKeyPassthroughMCPEnv) + } +} + +func appendUnique(list []string, value string) []string { + for _, v := range list { + if v == value { + return list + } + } + return append(list, value) +} + +// getPassthroughMCPFiles reads the recorded config-file list, tolerating both +// []string (in-memory) and []interface{} (JSON-decoded after a restart). +func getPassthroughMCPFiles(execution *AgentExecution) []string { + if execution == nil || execution.Metadata == nil { + return nil + } + switch v := execution.Metadata[metadataKeyPassthroughMCPFiles].(type) { + case []string: + return append([]string(nil), v...) + case []interface{}: + out := make([]string, 0, len(v)) + for _, item := range v { + if s, ok := item.(string); ok { + out = append(out, s) + } + } + return out + default: + return nil + } +} + +func setPassthroughMCPFiles(execution *AgentExecution, files []string) { if execution.Metadata == nil { execution.Metadata = map[string]interface{}{} } - execution.Metadata["passthrough_mcp_config_path"] = path - return path, nil + execution.Metadata[metadataKeyPassthroughMCPFiles] = files } -func (m *Manager) cleanupPassthroughMCPConfig(execution *AgentExecution) { - path := passthroughMCPConfigPath(execution) - if path == "" { - return +// getPassthroughMCPEnv reads the recorded MCP env map, tolerating both +// map[string]string and map[string]interface{} (JSON-decoded after a restart). +func getPassthroughMCPEnv(execution *AgentExecution) map[string]string { + if execution == nil || execution.Metadata == nil { + return nil } - if err := os.Remove(path); err != nil && !os.IsNotExist(err) { - m.logger.Warn("failed to remove passthrough MCP config", - zap.String("path", path), - zap.Error(err)) + switch v := execution.Metadata[metadataKeyPassthroughMCPEnv].(type) { + case map[string]string: + return v + case map[string]interface{}: + out := make(map[string]string, len(v)) + for key, item := range v { + if s, ok := item.(string); ok { + out[key] = s + } + } + return out + default: + return nil } - if execution.Metadata != nil { - delete(execution.Metadata, "passthrough_mcp_config_path") +} + +func setPassthroughMCPEnv(execution *AgentExecution, env map[string]string) { + if len(env) == 0 { + return } + if execution.Metadata == nil { + execution.Metadata = map[string]interface{}{} + } + execution.Metadata[metadataKeyPassthroughMCPEnv] = env } // passthroughAgentCommand validates passthrough support and builds the command for a passthrough session. // Returns the PassthroughAgent, PassthroughConfig, RuntimeConfig pointer, command, and any error. -func (m *Manager) passthroughAgentCommand(execution *AgentExecution, profileInfo *AgentProfileInfo) (agents.PassthroughAgent, agents.PassthroughConfig, *agents.RuntimeConfig, agents.Command, error) { +func (m *Manager) passthroughAgentCommand(ctx context.Context, execution *AgentExecution, profileInfo *AgentProfileInfo) (agents.PassthroughAgent, agents.PassthroughConfig, *agents.RuntimeConfig, agents.Command, error) { agentConfig, err := m.getAgentConfigForExecution(execution) if err != nil { return nil, agents.PassthroughConfig{}, nil, agents.Command{}, fmt.Errorf("failed to get agent config: %w", err) @@ -353,7 +636,7 @@ func (m *Manager) passthroughAgentCommand(execution *AgentExecution, profileInfo rt := agentConfig.Runtime() taskDescription := getTaskDescriptionFromMetadata(execution) promptForCmd := promptForPassthroughCommand(pt, taskDescription) - mcpConfigPath, err := m.writePassthroughMCPConfig(execution, pt) + mcpArgs, err := m.applyPassthroughMCP(ctx, execution, pt, agentConfig) if err != nil { return nil, agents.PassthroughConfig{}, nil, agents.Command{}, err } @@ -363,7 +646,7 @@ func (m *Manager) passthroughAgentCommand(execution *AgentExecution, profileInfo SessionID: execution.ACPSessionID, Prompt: promptForCmd, PermissionValues: profilePermissionValues(profileInfo), - MCPConfigPath: mcpConfigPath, + MCPArgs: mcpArgs, CLIFlagTokens: m.profileCLIFlagTokens(profileInfo), }) if cmd.IsEmpty() { @@ -395,8 +678,11 @@ func (m *Manager) profileCLIFlagTokens(p *AgentProfileInfo) []string { // terminal WebSocket is already connected). func buildInteractiveStartRequest(sessionID string, execution *AgentExecution, pt agents.PassthroughConfig, env map[string]string, cmd agents.Command, immediateStart bool) process.InteractiveStartRequest { return process.InteractiveStartRequest{ - SessionID: sessionID, - Command: cmd.Args(), + SessionID: sessionID, + Command: cmd.Args(), + // Redacted copy logged in place of Command by the interactive runner so + // Codex MCP `-c` overrides (env/headers tokens) never reach process logs. + LogCommand: redactPassthroughArgs(cmd.Args()), WorkingDir: execution.WorkspacePath, Env: env, PromptPattern: pt.PromptPattern, @@ -437,14 +723,14 @@ func (m *Manager) startInteractiveProcess(ctx context.Context, execution *AgentE // startPassthroughSession starts an agent in passthrough mode (direct terminal interaction). // Instead of using ACP protocol, the agent's stdin/stdout is passed through directly. func (m *Manager) startPassthroughSession(ctx context.Context, execution *AgentExecution, profileInfo *AgentProfileInfo) error { - _, pt, rt, cmd, err := m.passthroughAgentCommand(execution, profileInfo) + _, pt, rt, cmd, err := m.passthroughAgentCommand(ctx, execution, profileInfo) if err != nil { return err } m.logger.Info("passthrough command built", zap.String("session_id", execution.SessionID), - zap.Strings("full_command", cmd.Args())) + zap.Strings("full_command", redactPassthroughArgs(cmd.Args()))) env := m.buildPassthroughEnv(ctx, execution, rt.RequiredEnv) @@ -462,13 +748,19 @@ func (m *Manager) startPassthroughSession(ctx context.Context, execution *AgentE zap.String("task_id", execution.TaskID), zap.String("session_id", execution.SessionID), zap.String("process_id", processInfo.ID), - zap.Strings("command", cmd.Args())) + zap.Strings("command", redactPassthroughArgs(cmd.Args()))) m.eventPublisher.PublishAgentctlEvent(ctx, events.AgentctlReady, execution, "") m.startPassthroughShell(ctx, execution, "failed to start shell for passthrough session") if m.streamManager != nil && execution.agentctl != nil { m.streamManager.ConnectWorkspaceStream(execution, nil) + // Also open the agent updates stream so the agentctl instance can proxy + // kandev MCP tool calls to the backend (passthrough has no ACP stream + // otherwise, so MCP tool calls would hang). + if !execution.agentctl.HasAgentStream() { + m.streamManager.ConnectMCPStream(execution) + } } go m.autoInjectInitialPrompt(execution, pt) @@ -518,7 +810,7 @@ func (m *Manager) freshPassthroughCommand(ctx context.Context, execution *AgentE if err != nil { return agents.PassthroughConfig{}, nil, agents.Command{}, err } - mcpConfigPath, err := m.writePassthroughMCPConfig(execution, resolved.pt) + mcpArgs, err := m.applyPassthroughMCP(ctx, execution, resolved.pt, resolved.agentConfig) if err != nil { return agents.PassthroughConfig{}, nil, agents.Command{}, err } @@ -526,7 +818,7 @@ func (m *Manager) freshPassthroughCommand(ctx context.Context, execution *AgentE cmd := resolved.agent.BuildPassthroughCommand(agents.PassthroughOptions{ Model: effectivePassthroughModel(execution, resolved.profile), PermissionValues: profilePermissionValues(resolved.profile), - MCPConfigPath: mcpConfigPath, + MCPArgs: mcpArgs, CLIFlagTokens: m.profileCLIFlagTokens(resolved.profile), }) if cmd.IsEmpty() { @@ -536,8 +828,8 @@ func (m *Manager) freshPassthroughCommand(ctx context.Context, execution *AgentE return resolved.pt, resolved.rt, cmd, nil } -func (m *Manager) resumePassthroughCommand(execution *AgentExecution, resolved *resolvedPassthrough, useResume bool) (agents.Command, error) { - mcpConfigPath, err := m.writePassthroughMCPConfig(execution, resolved.pt) +func (m *Manager) resumePassthroughCommand(ctx context.Context, execution *AgentExecution, resolved *resolvedPassthrough, useResume bool) (agents.Command, error) { + mcpArgs, err := m.applyPassthroughMCP(ctx, execution, resolved.pt, resolved.agentConfig) if err != nil { return agents.Command{}, err } @@ -545,7 +837,7 @@ func (m *Manager) resumePassthroughCommand(execution *AgentExecution, resolved * Model: effectivePassthroughModel(execution, resolved.profile), Resume: useResume, PermissionValues: profilePermissionValues(resolved.profile), - MCPConfigPath: mcpConfigPath, + MCPArgs: mcpArgs, CLIFlagTokens: m.profileCLIFlagTokens(resolved.profile), }) if cmd.IsEmpty() { @@ -647,7 +939,7 @@ func (m *Manager) ResumePassthroughSession(ctx context.Context, sessionID string // backend restart. Once the sticky flag is set, every subsequent launch // for this execution starts fresh. useResume := !execution.passthroughResumeFailed - cmd, err := m.resumePassthroughCommand(execution, resolved, useResume) + cmd, err := m.resumePassthroughCommand(ctx, execution, resolved, useResume) if err != nil { return err } @@ -656,7 +948,7 @@ func (m *Manager) ResumePassthroughSession(ctx context.Context, sessionID string zap.String("session_id", sessionID), zap.String("execution_id", execution.ID), zap.Bool("use_resume", useResume), - zap.Strings("command", cmd.Args())) + zap.Strings("command", redactPassthroughArgs(cmd.Args()))) env := m.buildPassthroughEnv(ctx, execution, resolved.rt.RequiredEnv) @@ -690,6 +982,10 @@ func (m *Manager) ResumePassthroughSession(ctx context.Context, sessionID string if m.streamManager != nil && execution.agentctl != nil && execution.GetWorkspaceStream() == nil { m.streamManager.ConnectWorkspaceStream(execution, nil) } + // Re-open the MCP proxy stream too (drains kandev MCP tool calls). + if m.streamManager != nil && execution.agentctl != nil && !execution.agentctl.HasAgentStream() { + m.streamManager.ConnectMCPStream(execution) + } return nil } @@ -962,6 +1258,9 @@ func (m *Manager) attemptResumeFallback(execution *AgentExecution, runner *proce if m.streamManager != nil && execution.agentctl != nil && execution.GetWorkspaceStream() == nil { m.streamManager.ConnectWorkspaceStream(execution, nil) } + if m.streamManager != nil && execution.agentctl != nil && !execution.agentctl.HasAgentStream() { + m.streamManager.ConnectMCPStream(execution) + } // Fallback path is a fresh session (no --resume) — re-inject the prompt. go m.autoInjectInitialPrompt(execution, pt) diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_test.go index 55c9673f1..9551fa074 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_test.go @@ -5,11 +5,13 @@ import ( "encoding/json" "os" "path/filepath" + "strings" "testing" "testing/synctest" "time" "github.com/kandev/kandev/internal/agent/agents" + "github.com/kandev/kandev/internal/agent/mcpconfig" settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" agentctltypes "github.com/kandev/kandev/internal/agentctl/types" ) @@ -73,13 +75,13 @@ func assertClaudePassthroughMCPConfig(t *testing.T, cmd agents.Command, wantURL t.Fatalf("passthrough command missing --mcp-config: %v", args) } -func newClaudePassthroughMCPTestManager(t *testing.T) (*Manager, *AgentExecution, *AgentProfileInfo) { +func newPassthroughMCPTestManager(t *testing.T, agentName string) (*Manager, *AgentExecution, *AgentProfileInfo) { t.Helper() mgr := newTestManager(t) mgr.dataDir = t.TempDir() mgr.profileResolver = &mockPassthroughProfileResolver{ - agentName: "claude-acp", + agentName: agentName, cliPassthrough: true, } execution := &AgentExecution{ @@ -95,13 +97,18 @@ func newClaudePassthroughMCPTestManager(t *testing.T) (*Manager, *AgentExecution } profile := &AgentProfileInfo{ ProfileID: "profile-1", - AgentName: "claude-acp", + AgentName: agentName, Model: "default", CLIPassthrough: true, } return mgr, execution, profile } +func newClaudePassthroughMCPTestManager(t *testing.T) (*Manager, *AgentExecution, *AgentProfileInfo) { + t.Helper() + return newPassthroughMCPTestManager(t, "claude-acp") +} + func TestBuildPassthroughCommand(t *testing.T) { tests := []struct { name string @@ -302,39 +309,39 @@ func TestBuildPassthroughCommand(t *testing.T) { wantCmd: []string{"test-cli", "--model", "gpt-4", "--yes", "--debug", "--log-level", "trace", "-c"}, }, { - name: "mcp config flag goes after positional prompt because claude treats it as variadic", + name: "mcp args go after positional prompt because claude treats --mcp-config as variadic", agent: &testAgent{ id: "test-agent", StandardPassthrough: agents.StandardPassthrough{ Cfg: agents.PassthroughConfig{ Supported: true, PassthroughCmd: agents.NewCommand("test-cli"), - MCPConfigFlag: agents.NewParam("--mcp-config", "{mcp_config}"), }, }, }, opts: agents.PassthroughOptions{ - Prompt: "fix the bug", - MCPConfigPath: "/tmp/kandev-mcp.json", + Prompt: "fix the bug", + MCPArgs: []string{"--mcp-config", "/tmp/kandev-mcp.json"}, }, wantCmd: []string{"test-cli", "fix the bug", "--mcp-config", "/tmp/kandev-mcp.json"}, }, { - name: "mcp config flag without placeholder still appends path", + name: "mcp args are appended after the resume flag", agent: &testAgent{ id: "test-agent", StandardPassthrough: agents.StandardPassthrough{ Cfg: agents.PassthroughConfig{ Supported: true, PassthroughCmd: agents.NewCommand("test-cli"), - MCPConfigFlag: agents.NewParam("--mcp-config"), + ResumeFlag: agents.NewParam("--resume"), }, }, }, opts: agents.PassthroughOptions{ - MCPConfigPath: "/tmp/kandev-mcp.json", + Resume: true, + MCPArgs: []string{"-c", `mcp_servers.kandev.url="http://x/mcp"`}, }, - wantCmd: []string{"test-cli", "--mcp-config", "/tmp/kandev-mcp.json"}, + wantCmd: []string{"test-cli", "--resume", "-c", `mcp_servers.kandev.url="http://x/mcp"`}, }, } @@ -359,7 +366,7 @@ func TestBuildPassthroughCommand(t *testing.T) { func TestPassthroughAgentCommandInjectsKandevMCPConfig(t *testing.T) { mgr, execution, profile := newClaudePassthroughMCPTestManager(t) - _, _, _, cmd, err := mgr.passthroughAgentCommand(execution, profile) + _, _, _, cmd, err := mgr.passthroughAgentCommand(context.Background(), execution, profile) if err != nil { t.Fatalf("passthroughAgentCommand returned error: %v", err) } @@ -385,7 +392,7 @@ func TestResumePassthroughCommandInjectsKandevMCPConfig(t *testing.T) { if err != nil { t.Fatalf("resolvePassthroughAgent returned error: %v", err) } - cmd, err := mgr.resumePassthroughCommand(execution, resolved, true) + cmd, err := mgr.resumePassthroughCommand(context.Background(), execution, resolved, true) if err != nil { t.Fatalf("resumePassthroughCommand returned error: %v", err) } @@ -393,6 +400,321 @@ func TestResumePassthroughCommandInjectsKandevMCPConfig(t *testing.T) { assertClaudePassthroughMCPConfig(t, cmd, "http://localhost:45678/mcp") } +func TestPassthroughCodexInjectsKandevMCPArgs(t *testing.T) { + mgr, execution, profile := newPassthroughMCPTestManager(t, "codex-acp") + + _, _, _, cmd, err := mgr.passthroughAgentCommand(context.Background(), execution, profile) + if err != nil { + t.Fatalf("passthroughAgentCommand returned error: %v", err) + } + + joined := strings.Join(cmd.Args(), " ") + if want := `-c mcp_servers.kandev.url="http://localhost:45678/mcp"`; !strings.Contains(joined, want) { + t.Fatalf("codex command missing kandev override %q in: %v", want, cmd.Args()) + } + // Codex injects via CLI flags only — it must not write any config file. + if files := getPassthroughMCPFiles(execution); len(files) != 0 { + t.Fatalf("codex must not write MCP config files, got %v", files) + } +} + +func TestPassthroughOpenCodeInjectsConfigEnv(t *testing.T) { + mgr, execution, profile := newPassthroughMCPTestManager(t, "opencode-acp") + + _, _, _, _, err := mgr.passthroughAgentCommand(context.Background(), execution, profile) + if err != nil { + t.Fatalf("passthroughAgentCommand returned error: %v", err) + } + + files := getPassthroughMCPFiles(execution) + if len(files) != 1 { + t.Fatalf("expected one opencode config file, got %v", files) + } + if _, err := os.Stat(files[0]); err != nil { + t.Fatalf("opencode config not written: %v", err) + } + // OPENCODE_CONFIG must be merged into the passthrough environment. + env := mgr.buildPassthroughEnv(context.Background(), execution, nil) + if env["OPENCODE_CONFIG"] != files[0] { + t.Fatalf("OPENCODE_CONFIG = %q, want %q", env["OPENCODE_CONFIG"], files[0]) + } +} + +func TestPassthroughCursorWritesProjectFile(t *testing.T) { + mgr, execution, profile := newPassthroughMCPTestManager(t, "cursor-acp") + cursorPath := filepath.Join(execution.WorkspacePath, ".cursor", "mcp.json") + + _, _, _, _, err := mgr.passthroughAgentCommand(context.Background(), execution, profile) + if err != nil { + t.Fatalf("passthroughAgentCommand returned error: %v", err) + } + + data, err := os.ReadFile(cursorPath) + if err != nil { + t.Fatalf("cursor mcp.json not written: %v", err) + } + if !strings.Contains(string(data), kandevMCPServerName) { + t.Fatalf("cursor mcp.json missing kandev server: %s", data) + } +} + +func TestPassthroughCursorMergesIntoExistingProjectFile(t *testing.T) { + mgr, execution, profile := newPassthroughMCPTestManager(t, "cursor-acp") + cursorPath := filepath.Join(execution.WorkspacePath, ".cursor", "mcp.json") + if err := os.MkdirAll(filepath.Dir(cursorPath), 0o755); err != nil { + t.Fatal(err) + } + // A user file with their own top-level key AND an existing MCP server. + userContent := `{"user":"config","mcpServers":{"user-srv":{"url":"https://user"}}}` + if err := os.WriteFile(cursorPath, []byte(userContent), 0o644); err != nil { + t.Fatal(err) + } + + if _, _, _, _, err := mgr.passthroughAgentCommand(context.Background(), execution, profile); err != nil { + t.Fatalf("passthroughAgentCommand returned error: %v", err) + } + + data, err := os.ReadFile(cursorPath) + if err != nil { + t.Fatalf("read cursor mcp.json: %v", err) + } + var merged struct { + User string `json:"user"` + MCPServers map[string]struct { + Type string `json:"type"` + URL string `json:"url"` + } `json:"mcpServers"` + } + if err := json.Unmarshal(data, &merged); err != nil { + t.Fatalf("merged file not JSON: %v\n%s", err, data) + } + if merged.User != "config" { + t.Errorf("user top-level key lost: %s", data) + } + if merged.MCPServers["user-srv"].URL != "https://user" { + t.Errorf("user's existing server dropped: %s", data) + } + // Cursor emits remote servers as {url, headers} with no type field. + if merged.MCPServers[kandevMCPServerName].URL != "http://localhost:45678/mcp" { + t.Errorf("kandev server not merged in: %s", data) + } + // A file merged into the user's must NOT be tracked for cleanup (it's theirs). + if files := getPassthroughMCPFiles(execution); len(files) != 0 { + t.Fatalf("merged user file must not be tracked for cleanup, got %v", files) + } +} + +func TestGetPassthroughMCPFilesDecodesRestartShapes(t *testing.T) { + // After a backend restart, Metadata is rehydrated from JSON, so a []string + // becomes []interface{} of strings. The reader must tolerate both shapes. + t.Run("in-memory []string", func(t *testing.T) { + exec := &AgentExecution{Metadata: map[string]interface{}{ + metadataKeyPassthroughMCPFiles: []string{"/a.json", "/b.json"}, + }} + got := getPassthroughMCPFiles(exec) + if len(got) != 2 || got[0] != "/a.json" || got[1] != "/b.json" { + t.Fatalf("got %v, want [/a.json /b.json]", got) + } + }) + t.Run("JSON-decoded []interface{}", func(t *testing.T) { + exec := &AgentExecution{Metadata: map[string]interface{}{ + metadataKeyPassthroughMCPFiles: []interface{}{"/a.json", 42, "/b.json"}, + }} + got := getPassthroughMCPFiles(exec) + // Non-string entries are dropped, not panicked on. + if len(got) != 2 || got[0] != "/a.json" || got[1] != "/b.json" { + t.Fatalf("got %v, want [/a.json /b.json]", got) + } + }) + t.Run("nil metadata", func(t *testing.T) { + if got := getPassthroughMCPFiles(&AgentExecution{}); got != nil { + t.Fatalf("got %v, want nil", got) + } + }) +} + +func TestGetPassthroughMCPEnvDecodesRestartShapes(t *testing.T) { + t.Run("in-memory map[string]string", func(t *testing.T) { + exec := &AgentExecution{Metadata: map[string]interface{}{ + metadataKeyPassthroughMCPEnv: map[string]string{"OPENCODE_CONFIG": "/oc.json"}, + }} + if got := getPassthroughMCPEnv(exec); got["OPENCODE_CONFIG"] != "/oc.json" { + t.Fatalf("got %v, want OPENCODE_CONFIG=/oc.json", got) + } + }) + t.Run("JSON-decoded map[string]interface{}", func(t *testing.T) { + exec := &AgentExecution{Metadata: map[string]interface{}{ + metadataKeyPassthroughMCPEnv: map[string]interface{}{"OPENCODE_CONFIG": "/oc.json", "BAD": 1}, + }} + got := getPassthroughMCPEnv(exec) + if got["OPENCODE_CONFIG"] != "/oc.json" { + t.Fatalf("got %v, want OPENCODE_CONFIG=/oc.json", got) + } + if _, ok := got["BAD"]; ok { + t.Fatalf("non-string env value must be dropped, got %v", got) + } + }) +} + +func TestWritePassthroughMCPFilesUnionTrackingOnRelaunch(t *testing.T) { + mgr := newTestManager(t) + exec := &AgentExecution{Metadata: map[string]interface{}{}} + path := filepath.Join(t.TempDir(), "cfg.json") + file := mcpconfig.PassthroughConfigFile{Path: path, Content: []byte("{}\n")} + + // Two launches writing the same path must track it exactly once. + if err := mgr.writePassthroughMCPFiles(exec, []mcpconfig.PassthroughConfigFile{file}); err != nil { + t.Fatalf("first write: %v", err) + } + if err := mgr.writePassthroughMCPFiles(exec, []mcpconfig.PassthroughConfigFile{file}); err != nil { + t.Fatalf("second write: %v", err) + } + if files := getPassthroughMCPFiles(exec); len(files) != 1 || files[0] != path { + t.Fatalf("union tracking failed: got %v, want [%s]", files, path) + } +} + +type fakeMcpConfigProvider struct { + config *mcpconfig.ProfileConfig +} + +func (f *fakeMcpConfigProvider) GetConfigByProfileID(_ context.Context, _ string) (*mcpconfig.ProfileConfig, error) { + return f.config, nil +} + +// TestPassthroughMCPServersMergesProfileAndDropsKandevCollision verifies that +// profile-configured MCP servers are merged with kandev's own server, and that a +// profile server named "kandev" cannot shadow the real kandev tools server. +func TestPassthroughMCPServersMergesProfileAndDropsKandevCollision(t *testing.T) { + mgr, execution, profile := newClaudePassthroughMCPTestManager(t) + mgr.mcpProvider = &fakeMcpConfigProvider{config: &mcpconfig.ProfileConfig{ + ProfileID: "profile-1", + Enabled: true, + Servers: map[string]mcpconfig.ServerDef{ + "github": {Type: mcpconfig.ServerTypeStdio, Command: "npx", Args: []string{"-y", "gh"}}, + "kandev": {Type: mcpconfig.ServerTypeStdio, Command: "evil-shadow"}, + }, + }} + // The default policy for an unknown runtime denies all transports; allow + // stdio so the profile servers survive resolution. + execution.Metadata["executor_mcp_policy"] = `{"allow_stdio":true}` + + if _, _, _, _, err := mgr.passthroughAgentCommand(context.Background(), execution, profile); err != nil { + t.Fatalf("passthroughAgentCommand returned error: %v", err) + } + + files := getPassthroughMCPFiles(execution) + if len(files) != 1 { + t.Fatalf("expected one MCP config file, got %v", files) + } + data, err := os.ReadFile(files[0]) + if err != nil { + t.Fatalf("read MCP config: %v", err) + } + var payload struct { + MCPServers map[string]struct { + Type string `json:"type"` + Command string `json:"command"` + URL string `json:"url"` + } `json:"mcpServers"` + } + if err := json.Unmarshal(data, &payload); err != nil { + t.Fatalf("MCP config not JSON: %v\n%s", err, data) + } + // kandev's own HTTP server must win over the profile's "kandev" stdio entry. + kandev := payload.MCPServers["kandev"] + if kandev.Type != "http" || kandev.URL != "http://localhost:45678/mcp" { + t.Fatalf("kandev entry = %+v, want our http tools server", kandev) + } + if kandev.Command == "evil-shadow" { + t.Fatal("profile 'kandev' server shadowed the real kandev server") + } + // The profile's own server must be merged in. + gh := payload.MCPServers["github"] + if gh.Type != "stdio" || gh.Command != "npx" { + t.Fatalf("github entry = %+v, want merged stdio server", gh) + } +} + +func TestWorkspacePathEscapesViaSymlink(t *testing.T) { + ws := t.TempDir() + outside := t.TempDir() + + // A clean path inside the workspace does not escape. + if escaped, err := workspacePathEscapes(ws, filepath.Join(ws, ".cursor", "mcp.json")); err != nil || escaped { + t.Fatalf("clean workspace path: escaped=%v err=%v, want false/nil", escaped, err) + } + // A file outside the workspace (kandev's temp dir) is exempt from the check. + if escaped, err := workspacePathEscapes(ws, filepath.Join(outside, "cfg.json")); err != nil || escaped { + t.Fatalf("temp path: escaped=%v err=%v, want false/nil", escaped, err) + } + // A symlinked `.cursor` pointing outside the workspace escapes. + if err := os.Symlink(outside, filepath.Join(ws, ".cursor")); err != nil { + t.Fatal(err) + } + if escaped, err := workspacePathEscapes(ws, filepath.Join(ws, ".cursor", "mcp.json")); err != nil || !escaped { + t.Fatalf("symlinked path: escaped=%v err=%v, want true/nil", escaped, err) + } +} + +func TestRedactPassthroughArgsRedactsCodexSecrets(t *testing.T) { + args := []string{ + "npx", "-y", "@openai/codex", + "-c", `mcp_servers.gh.command="npx"`, + "-c", `mcp_servers.gh.env={"GITHUB_TOKEN":"s3cret"}`, + "-c", `mcp_servers.remote.http_headers={"Authorization":"Bearer tok"}`, + "-c", `mcp_servers.remote.url="http://x/mcp"`, + "--mcp-config", "/tmp/c.json", + } + joined := strings.Join(redactPassthroughArgs(args), " ") + + if strings.Contains(joined, "s3cret") || strings.Contains(joined, "Bearer tok") { + t.Fatalf("secrets not redacted: %s", joined) + } + if !strings.Contains(joined, "mcp_servers.gh.env=") || + !strings.Contains(joined, "mcp_servers.remote.http_headers=") { + t.Errorf("expected redaction markers: %s", joined) + } + // Non-secret tokens must be preserved verbatim. + for _, want := range []string{`mcp_servers.gh.command="npx"`, `mcp_servers.remote.url="http://x/mcp"`, "--mcp-config", "/tmp/c.json"} { + if !strings.Contains(joined, want) { + t.Errorf("redaction dropped non-secret %q: %s", want, joined) + } + } +} + +func TestWritePassthroughMCPFilesSkipsDanglingLeafSymlink(t *testing.T) { + mgr := newTestManager(t) + ws := t.TempDir() + outside := t.TempDir() + outsideTarget := filepath.Join(outside, "target.json") // intentionally never created + cursorDir := filepath.Join(ws, ".cursor") + if err := os.MkdirAll(cursorDir, 0o755); err != nil { + t.Fatal(err) + } + leaf := filepath.Join(cursorDir, "mcp.json") + if err := os.Symlink(outsideTarget, leaf); err != nil { + t.Fatal(err) + } + + execution := &AgentExecution{WorkspacePath: ws, Metadata: map[string]interface{}{}} + if err := mgr.writePassthroughMCPFiles(execution, []mcpconfig.PassthroughConfigFile{ + {Path: leaf, Content: []byte(`{"mcpServers":{}}`), MergeKey: "mcpServers"}, + }); err != nil { + t.Fatalf("writePassthroughMCPFiles: %v", err) + } + + // The write must NOT have followed the symlink to create the outside file + // (neither the merge-read nor the write may traverse it). + if _, err := os.Stat(outsideTarget); !os.IsNotExist(err) { + t.Fatalf("write followed dangling symlink to outside target (err=%v)", err) + } + // A skipped symlink must not be tracked for cleanup. + if files := getPassthroughMCPFiles(execution); len(files) != 0 { + t.Fatalf("skipped symlink leaf must not be tracked, got %v", files) + } +} + func TestResumePassthroughSessionWithoutRunnerDoesNotWriteMCPConfig(t *testing.T) { mgr, execution, _ := newClaudePassthroughMCPTestManager(t) mgr.executorRegistry = nil @@ -404,8 +726,8 @@ func TestResumePassthroughSessionWithoutRunnerDoesNotWriteMCPConfig(t *testing.T if err == nil { t.Fatal("ResumePassthroughSession returned nil, want missing runner error") } - if path := passthroughMCPConfigPath(execution); path != "" { - t.Fatalf("passthrough MCP config path = %q, want empty", path) + if files := getPassthroughMCPFiles(execution); len(files) != 0 { + t.Fatalf("passthrough MCP config files = %v, want none", files) } } @@ -414,7 +736,7 @@ func TestPassthroughAgentCommandErrorsWhenMCPPortMissing(t *testing.T) { execution.standalonePort = 0 delete(execution.Metadata, "standalone_port") - _, _, _, _, err := mgr.passthroughAgentCommand(execution, profile) + _, _, _, _, err := mgr.passthroughAgentCommand(context.Background(), execution, profile) if err == nil { t.Fatal("passthroughAgentCommand returned nil, want missing MCP port error") } @@ -423,6 +745,34 @@ func TestPassthroughAgentCommandErrorsWhenMCPPortMissing(t *testing.T) { } } +func TestFreshPassthroughCommandErrorsWhenMCPPortMissing(t *testing.T) { + mgr, execution, _ := newClaudePassthroughMCPTestManager(t) + execution.standalonePort = 0 + delete(execution.Metadata, "standalone_port") + + if _, _, _, err := mgr.freshPassthroughCommand(context.Background(), execution); err == nil { + t.Fatal("freshPassthroughCommand returned nil, want missing MCP port error") + } else if err.Error() != "standalone port unavailable for passthrough MCP config" { + t.Fatalf("error = %q, want missing MCP port error", err.Error()) + } +} + +func TestResumePassthroughCommandErrorsWhenMCPPortMissing(t *testing.T) { + mgr, execution, _ := newClaudePassthroughMCPTestManager(t) + execution.standalonePort = 0 + delete(execution.Metadata, "standalone_port") + + resolved, err := mgr.resolvePassthroughAgent(context.Background(), execution) + if err != nil { + t.Fatalf("resolvePassthroughAgent returned error: %v", err) + } + if _, err := mgr.resumePassthroughCommand(context.Background(), execution, resolved, true); err == nil { + t.Fatal("resumePassthroughCommand returned nil, want missing MCP port error") + } else if err.Error() != "standalone port unavailable for passthrough MCP config" { + t.Fatalf("error = %q, want missing MCP port error", err.Error()) + } +} + func TestRemoveExecutionCleansPassthroughMCPConfig(t *testing.T) { mgr, execution, profile := newClaudePassthroughMCPTestManager(t) execution.Metadata = nil @@ -430,23 +780,24 @@ func TestRemoveExecutionCleansPassthroughMCPConfig(t *testing.T) { t.Fatalf("add execution: %v", err) } - _, _, _, cmd, err := mgr.passthroughAgentCommand(execution, profile) + _, _, _, cmd, err := mgr.passthroughAgentCommand(context.Background(), execution, profile) if err != nil { t.Fatalf("passthroughAgentCommand returned error: %v", err) } assertClaudePassthroughMCPConfig(t, cmd, "http://localhost:45678/mcp") - path := passthroughMCPConfigPath(execution) - if path == "" { - t.Fatal("passthrough MCP config path was not stored") + files := getPassthroughMCPFiles(execution) + if len(files) == 0 { + t.Fatal("passthrough MCP config file was not stored") } + path := files[0] mgr.RemoveExecution(execution.ID) if _, err := os.Stat(path); !os.IsNotExist(err) { t.Fatalf("generated MCP config still exists or stat failed: %v", err) } - if path := passthroughMCPConfigPath(execution); path != "" { - t.Fatalf("passthrough MCP config path metadata = %q, want empty", path) + if files := getPassthroughMCPFiles(execution); len(files) != 0 { + t.Fatalf("passthrough MCP config files metadata = %v, want empty", files) } } @@ -454,13 +805,15 @@ func TestWritePassthroughMCPConfigKeepsLiteralSessionFilename(t *testing.T) { mgr, execution, profile := newClaudePassthroughMCPTestManager(t) execution.SessionID = "session" - _, _, _, cmd, err := mgr.passthroughAgentCommand(execution, profile) + _, _, _, cmd, err := mgr.passthroughAgentCommand(context.Background(), execution, profile) if err != nil { t.Fatalf("passthroughAgentCommand returned error: %v", err) } assertClaudePassthroughMCPConfig(t, cmd, "http://localhost:45678/mcp") - if got, want := passthroughMCPConfigPath(execution), filepath.Join(mgr.dataDir, "passthrough-mcp", "session.json"); got != want { - t.Fatalf("passthrough MCP config path = %q, want session filename", got) + files := getPassthroughMCPFiles(execution) + want := filepath.Join(mgr.dataDir, "passthrough-mcp", "session.json") + if len(files) != 1 || files[0] != want { + t.Fatalf("passthrough MCP config files = %v, want [%s]", files, want) } } diff --git a/apps/backend/internal/agent/runtime/lifecycle/streams.go b/apps/backend/internal/agent/runtime/lifecycle/streams.go index 67f1692d4..143df20b8 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/streams.go +++ b/apps/backend/internal/agent/runtime/lifecycle/streams.go @@ -104,6 +104,14 @@ func (sm *StreamManager) ConnectWorkspaceStream(execution *AgentExecution, ready } } +// ConnectMCPStream opens the passthrough MCP proxy stream under goroutine +// tracking so it drains cleanly on shutdown (mirrors ConnectWorkspaceStream). +func (sm *StreamManager) ConnectMCPStream(execution *AgentExecution) { + sm.start(func() { + sm.connectMCPStream(execution) + }) +} + // Wait blocks until all StreamManager-owned stream goroutines have exited. func (sm *StreamManager) Wait() { sm.wgMu.Lock() @@ -227,6 +235,30 @@ func (sm *StreamManager) connectUpdatesStream(execution *AgentExecution, ready c } } +// connectMCPStream opens the agent updates WebSocket for a PASSTHROUGH session +// purely to drain the MCP request channel: the agentctl instance serves /mcp and +// proxies tool calls to the backend over this stream, so without it kandev MCP +// tool calls hang. Passthrough agents don't speak ACP, so no agent events arrive +// here (the PTY drives the UI). On disconnect it only logs — it must NOT signal +// promptDoneCh or OnStreamDisconnect (which would mark the execution failed); +// passthrough completion is detected via PTY idle, and a normal session end +// closing this stream is expected, not an error. +func (sm *StreamManager) connectMCPStream(execution *AgentExecution) { + ctx := sm.streamContext(execution) + err := execution.agentctl.StreamUpdates(ctx, func(agentctl.AgentEvent) {}, sm.mcpHandler, func(disconnectErr error) { + if disconnectErr != nil { + sm.logger.Debug("passthrough MCP stream disconnected", + zap.String("execution_id", execution.ID), + zap.Error(disconnectErr)) + } + }) + if err != nil { + sm.logger.Error("failed to connect passthrough MCP stream", + zap.String("execution_id", execution.ID), + zap.Error(err)) + } +} + // buildWorkspaceCallbacks creates the WorkspaceStreamCallbacks for a given execution, // wiring each callback to the StreamManager's registered handlers. func (sm *StreamManager) buildWorkspaceCallbacks(execution *AgentExecution) agentctl.WorkspaceStreamCallbacks { diff --git a/apps/backend/internal/agent/settings/controller/agent_discovery.go b/apps/backend/internal/agent/settings/controller/agent_discovery.go index 37fec162c..d14ea746a 100644 --- a/apps/backend/internal/agent/settings/controller/agent_discovery.go +++ b/apps/backend/internal/agent/settings/controller/agent_discovery.go @@ -125,6 +125,9 @@ func (c *Controller) buildAvailableAgentDTO(ctx context.Context, ag agents.Agent AutoInjectPrompt: pt.AutoInjectPrompt, SubmitSequence: pt.SubmitSequence, } + if pt.MCPStrategy != nil { + passthroughConfig.MCPInjection = pt.MCPStrategy.Describe() + } } loginCommand := buildLoginCommandDTO(ag) diff --git a/apps/backend/internal/agent/settings/dto/dto.go b/apps/backend/internal/agent/settings/dto/dto.go index 337565b1d..50c9945a8 100644 --- a/apps/backend/internal/agent/settings/dto/dto.go +++ b/apps/backend/internal/agent/settings/dto/dto.go @@ -151,6 +151,11 @@ type PassthroughConfigDTO struct { Description string `json:"description"` AutoInjectPrompt bool `json:"auto_inject_prompt"` SubmitSequence string `json:"submit_sequence"` + // MCPInjection is a short human-readable phrase describing how kandev + // injects MCP servers into this agent's CLI in passthrough mode (e.g. + // "an MCP config file passed via the --mcp-config flag"). Empty when the + // agent declares no MCP strategy. + MCPInjection string `json:"mcp_injection,omitempty"` } type ToolStatusDTO struct { diff --git a/apps/backend/internal/agentctl/server/process/interactive_lifecycle.go b/apps/backend/internal/agentctl/server/process/interactive_lifecycle.go index 728c705d8..ee6e456d7 100644 --- a/apps/backend/internal/agentctl/server/process/interactive_lifecycle.go +++ b/apps/backend/internal/agentctl/server/process/interactive_lifecycle.go @@ -154,7 +154,7 @@ func (r *InteractiveRunner) Start(ctx context.Context, req InteractiveStartReque r.logger.Info("interactive process created (waiting for terminal dimensions)", zap.String("process_id", id), zap.String("session_id", req.SessionID), - zap.Strings("command", req.Command), + zap.Strings("command", req.commandForLog()), zap.String("working_dir", req.WorkingDir), ) } @@ -209,7 +209,7 @@ func (r *InteractiveRunner) immediateStartProcess(req InteractiveStartRequest, p r.logger.Info("interactive process started immediately", zap.String("process_id", id), zap.String("session_id", req.SessionID), - zap.Strings("command", req.Command), + zap.Strings("command", req.commandForLog()), zap.String("working_dir", req.WorkingDir), ) return nil diff --git a/apps/backend/internal/agentctl/server/process/interactive_runner.go b/apps/backend/internal/agentctl/server/process/interactive_runner.go index f528317cf..896eeecb2 100644 --- a/apps/backend/internal/agentctl/server/process/interactive_runner.go +++ b/apps/backend/internal/agentctl/server/process/interactive_runner.go @@ -22,6 +22,7 @@ import ( type InteractiveStartRequest struct { SessionID string `json:"session_id"` // Required: Agent session owning this process Command []string `json:"command"` // Required: Command and args to execute + LogCommand []string `json:"log_command,omitempty"` // Optional: redacted copy of Command for logging (e.g. MCP `-c` overrides with tokens); falls back to Command when unset WorkingDir string `json:"working_dir"` // Working directory ScopeID string `json:"scope_id,omitempty"` // User-shell scope (task environment ID) when applicable TerminalID string `json:"terminal_id,omitempty"` // User-shell terminal ID when applicable @@ -41,6 +42,15 @@ type InteractiveStartRequest struct { IsUserShell bool `json:"is_user_shell,omitempty"` // Mark as user shell process (excluded from session-level lookups) } +// commandForLog returns the command to log: the redacted LogCommand when the +// caller supplied one, otherwise the raw Command. +func (r InteractiveStartRequest) commandForLog() []string { + if len(r.LogCommand) > 0 { + return r.LogCommand + } + return r.Command +} + // InteractiveProcessInfo represents the state of an interactive process. type InteractiveProcessInfo struct { ID string `json:"id"` diff --git a/apps/web/app/settings/agents/[agentId]/agent-setup-parts.tsx b/apps/web/app/settings/agents/[agentId]/agent-setup-parts.tsx index 9d58bba8b..07f1db326 100644 --- a/apps/web/app/settings/agents/[agentId]/agent-setup-parts.tsx +++ b/apps/web/app/settings/agents/[agentId]/agent-setup-parts.tsx @@ -146,6 +146,8 @@ export function ProfileCardItem({ onProfileMcpChange(profile.id, patch)} onToastError={onToastError} diff --git a/apps/web/app/settings/agents/[agentId]/profile-mcp-config-card.tsx b/apps/web/app/settings/agents/[agentId]/profile-mcp-config-card.tsx index 0ad50ff44..4d7cf4326 100644 --- a/apps/web/app/settings/agents/[agentId]/profile-mcp-config-card.tsx +++ b/apps/web/app/settings/agents/[agentId]/profile-mcp-config-card.tsx @@ -12,6 +12,17 @@ import type { AgentProfileMcpConfig } from "@/lib/types/http"; type ProfileMcpConfigCardProps = { profileId: string; supportsMcp: boolean; + /** + * Whether the profile is in CLI passthrough mode. When true (and + * mcpInjection is set), the card explains how kandev injects MCP servers + * into the agent's CLI. + */ + cliPassthrough?: boolean; + /** + * Human-readable phrase describing the passthrough MCP injection mechanism + * (from PassthroughConfig.mcp_injection). Only rendered when cliPassthrough. + */ + mcpInjection?: string; initialConfig?: AgentProfileMcpConfig | null; draftState?: { enabled: boolean; @@ -131,12 +142,29 @@ function validateDraftServers(value: string): string | null { return null; } +function PassthroughMcpInjectionHint({ + cliPassthrough, + mcpInjection, +}: { + cliPassthrough?: boolean; + mcpInjection?: string; +}) { + if (!cliPassthrough || !mcpInjection) return null; + return ( +

+ In CLI passthrough mode, kandev injects these MCP servers via {mcpInjection}. +

+ ); +} + type McpServersEditorProps = { profileId: string; currentServers: string; currentError: string | null; isDraft: boolean; isEditableProfile: boolean; + cliPassthrough?: boolean; + mcpInjection?: string; onDraftStateChange?: (next: { servers?: string; dirty?: boolean; error?: string | null }) => void; handleMcpServersChange: (value: string) => void; }; @@ -147,6 +175,8 @@ function McpServersEditor({ currentError, isDraft, isEditableProfile, + cliPassthrough, + mcpInjection, onDraftStateChange, handleMcpServersChange, }: McpServersEditorProps) { @@ -186,6 +216,7 @@ function McpServersEditor({ MCP definitions are stored in the database and resolved per executor at runtime. This does not override your local agent config.

+

Built-in

@@ -314,6 +345,8 @@ function McpProfileHint({ export function ProfileMcpConfigCard({ profileId, supportsMcp, + cliPassthrough, + mcpInjection, initialConfig, draftState, onDraftStateChange, @@ -364,6 +397,8 @@ export function ProfileMcpConfigCard({ currentError={state.currentError} isDraft={state.isDraft} isEditableProfile={state.isEditableProfile} + cliPassthrough={cliPassthrough} + mcpInjection={mcpInjection} onDraftStateChange={onDraftStateChange} handleMcpServersChange={handleMcpServersChange} /> diff --git a/apps/web/components/settings/agent-profile-page.tsx b/apps/web/components/settings/agent-profile-page.tsx index a70d2750d..0f51a02c1 100644 --- a/apps/web/components/settings/agent-profile-page.tsx +++ b/apps/web/components/settings/agent-profile-page.tsx @@ -438,6 +438,8 @@ function ProfileEditorBody({ diff --git a/apps/web/lib/types/http-agents.ts b/apps/web/lib/types/http-agents.ts index a383885f8..001e6ca08 100644 --- a/apps/web/lib/types/http-agents.ts +++ b/apps/web/lib/types/http-agents.ts @@ -163,6 +163,12 @@ export type PassthroughConfig = { description: string; auto_inject_prompt?: boolean; submit_sequence?: string; + /** + * Short human-readable phrase describing how kandev injects MCP servers into + * this agent's CLI in passthrough mode (e.g. "an MCP config file passed via + * the --mcp-config flag"). Absent when the agent declares no MCP strategy. + */ + mcp_injection?: string; }; export type ToolStatus = { diff --git a/docs/add-agent-cli.md b/docs/add-agent-cli.md index c98fe6ed7..f2b533934 100644 --- a/docs/add-agent-cli.md +++ b/docs/add-agent-cli.md @@ -141,6 +141,7 @@ Key implementation notes: - **`BuildCommand()`** returns the CLI command for structured protocol mode (the agent communicates via stdin/stdout with a protocol adapter). Include the protocol flag here (e.g. `--acp`, `--stream-json`). - **`Runtime()`** returns configuration used when running the agent in Docker or as a standalone process. Set `Protocol` to tell the adapter factory which transport to use. - **`IsInstalled()`** uses the `Detect()` helper with `WithFileExists()` to check for known installation paths. Set `SupportsMCP` and `MCPConfigPaths` on the result. +- **MCP servers in passthrough** — set `MCPStrategy` on `PassthroughConfig` so the agent receives kandev's own tools server (plus any profile-configured MCP servers) when run in passthrough mode. Omitting it means **no MCP servers are injected in passthrough**. Pick the strategy that matches how your CLI loads MCP servers: `mcpconfig.ClaudeStrategy{}` (temp JSON file + `--mcp-config` flag), `mcpconfig.CodexStrategy{}` (repeated `-c mcp_servers.…` overrides), `mcpconfig.CursorStrategy{}` (project-local `.cursor/mcp.json`), or `mcpconfig.OpenCodeStrategy{}` (temp JSON + `OPENCODE_CONFIG` env var). To add a strategy for a CLI with a different mechanism, implement `mcpconfig.PassthroughMCPStrategy`. See ADR 0014 and `internal/agent/mcpconfig/passthrough.go`. - **`ListModels()`** can return a static list or dynamically query the CLI. Use `execAndParse()` from `helpers.go` for dynamic model discovery. - Use `Cmd()` builder for constructing commands fluently: `Cmd("myagent", "--flag").Model(...).Settings(...).Build()`. diff --git a/docs/decisions/0014-passthrough-mcp-injection-strategies.md b/docs/decisions/0014-passthrough-mcp-injection-strategies.md new file mode 100644 index 000000000..1afb713fc --- /dev/null +++ b/docs/decisions/0014-passthrough-mcp-injection-strategies.md @@ -0,0 +1,70 @@ +# 0014: Per-CLI MCP server injection for passthrough mode + +**Status:** accepted +**Date:** 2026-05-29 +**Area:** backend + +## Context + +Kandev runs agent CLIs in two modes: **ACP** (kandev speaks the Agent Client Protocol to the CLI and passes MCP servers via `session/new`) and **passthrough** (the raw CLI runs directly in a PTY and the user interacts with its native TUI). MCP servers configured on an agent profile's settings page were only wired into ACP mode. In passthrough mode the single `writePassthroughMCPConfig` helper hardcoded one server — kandev's own HTTP tools server — into a Claude-shaped JSON file, gated on a `MCPConfigFlag Param` that only `claude-acp` declared. Codex, Cursor, and OpenCode passthrough sessions received **no** MCP servers at all, and even Claude never received the profile's configured servers. + +The naive fix — "write a JSON file and pass `--mcp-config`" — only works for Claude. Each CLI loads MCP servers differently, uses a different config schema, and has a different "don't touch the user's global config" escape hatch: + +- **Claude Code** — `--mcp-config ` flag (variadic, additive); `{"mcpServers": {...}}` schema. +- **Codex** — no MCP-config-file flag at all; servers come from `~/.codex/config.toml` or repeatable `-c key=value` CLI overrides. +- **Cursor (`cursor-agent`)** — no MCP flag and no reliable MCP env var; only auto-discovers a project-local `.cursor/mcp.json`. +- **OpenCode** — no config CLI flag; config file is selected via the `OPENCODE_CONFIG` env var; schema uses an `mcp` block with `local`/`remote` types. + +A hard requirement was that kandev must **never modify the user's global CLI config** (`~/.claude.json`, `~/.codex/config.toml`, `~/.cursor/mcp.json`, `~/.config/opencode`). + +## Decision + +**Each passthrough-capable agent declares a `PassthroughMCPStrategy` that materializes the resolved MCP server list into that CLI's native shape (config files, CLI args, and/or env vars), without writing to the user's global config.** The runtime resolves servers once and delegates emission to the strategy. + +The strategy interface (`internal/agent/mcpconfig/passthrough.go`) is pure — it returns descriptors; the runtime performs all I/O: + +```go +type PassthroughMCPStrategy interface { + BuildPassthroughMCP(servers []types.McpServer, paths PassthroughPaths) (PassthroughArtifacts, error) +} +// PassthroughArtifacts: Files (to write, with SkipIfExists), Args (appended to cmd), Env (merged into process env) +``` + +Four implementations: + +| CLI | Mechanism | Schema | Global config touched | +|-----|-----------|--------|----------------------| +| Claude | temp file + `--mcp-config ` args | `{"mcpServers":{…}}`, `streamable_http`→`streamable-http` | No | +| Codex | repeated `-c mcp_servers..=` args | transport inferred from `command` vs `url`; no `type` key | No | +| Cursor | project-local `/.cursor/mcp.json` (merged into an existing file, else created) | `{"mcpServers":{…}}` | No | +| OpenCode | temp file + `OPENCODE_CONFIG` env var | `{"mcp":{name:{type:"local"\|"remote",…}}}` | No | + +Runtime wiring (`internal/agent/runtime/lifecycle/manager_passthrough.go`, `applyPassthroughMCP`): + +1. **Resolve once, reuse the ACP path.** Profile servers come from the same `resolveMcpServersWithParams` ACP already uses (transport policy, allow/deny, URL rewrite, env injection all apply identically). Kandev's own HTTP tools server is prepended; a profile server named `kandev` is dropped so it cannot shadow the real one. +2. **Strategy emits, runtime writes.** Files are written 0600 under a kandev-owned temp dir (Claude/OpenCode) or the worktree (Cursor); kandev-created files are tracked (unioned across relaunch/resume/restart) in `execution.Metadata` for cleanup. An existing symlink at a target path is never written or read through. `MergeKey` files (Cursor) merge into an existing user file and are not tracked for cleanup. Strategy env is merged in `buildPassthroughEnv`; strategy args are appended **last** to the command. Each strategy also `Describe()`s its mechanism, surfaced on the profile MCP card (`passthrough_config.mcp_injection`) so users see how MCP is wired. +3. **Cleanup** removes only kandev-written files when the execution is removed. + +The old `MCPConfigFlag Param` / `CmdBuilder.MCPConfig` / `PassthroughOptions.MCPConfigPath` mechanism was removed in favor of `MCPStrategy` and `PassthroughOptions.MCPArgs`. + +### Key sub-decisions + +- **MCP args are appended *last* in the built command.** Claude's `--mcp-config` is variadic and would otherwise swallow a positional prompt as an extra config path. Codex's `-c` overrides are order-insensitive, so trailing placement is safe for both. (Only Claude and Codex emit args; Cursor/OpenCode use files/env.) +- **Claude injection is additive (no `--strict-mcp-config`).** Kandev's servers merge with the user's `~/.claude.json` and project `.mcp.json` rather than replacing them. `ClaudeStrategy{Strict: true}` is available if isolation is ever wanted, but the friendlier default preserves the user's own servers. +- **Cursor merges into an existing `.cursor/mcp.json`.** When the file is absent kandev creates it; when present, kandev's servers are merged into its `mcpServers` (user entries preserved, ours win on name collision) via `PassthroughConfigFile.MergeKey`, so kandev tools are available even when the user has their own project config. An existing symlink at the path is refused (never written/read through), and a merged user file is not tracked for cleanup. (Earlier this skipped-if-exists, which left Cursor without kandev tools whenever a project file pre-existed.) +- **Passthrough must open the agent updates stream for MCP to work.** The agentctl instance serves `/mcp` and proxies tool calls to the backend over the agent updates WebSocket, drained only while that stream is open. The ACP path opens it; passthrough originally opened only the workspace stream, so `tools/list` worked (local) but `tools/call` hung. `applyPassthroughMCP`'s launch/resume/fallback paths now also open a passthrough-safe MCP stream (`connectMCPStream`) — it wires the MCP handler but is log-only on disconnect (it must not mark the execution failed, since passthrough completion is PTY-idle). + +## Consequences + +- All four CLIs receive kandev's tools server plus the profile's MCP servers in passthrough mode; previously three received nothing. +- Adding a passthrough CLI with a new MCP mechanism = implement one `PassthroughMCPStrategy` and set it on the agent's `PassthroughConfig`. No runtime changes. +- The kandev HTTP server requires the standalone port; passthrough launch still errors when the port is unavailable (preserved behavior). +- `execution.Metadata` stores the written-file list and env map; getters decode both in-memory (`[]string`, `map[string]string`) and JSON-rehydrated (`[]interface{}`, `map[string]interface{}`) shapes so cleanup survives a backend restart. +- A `.cursor/mcp.json` kandev merged into (rather than created) persists after the session with kandev's entry; it is re-merged (port refreshed) on the next launch. Un-merging on teardown was judged not worth the complexity. + +## Alternatives considered + +- **`CODEX_HOME` pointing at a kandev temp dir (rejected).** It relocates Codex's *entire* config dir — including `auth.json` — so a bare temp dir leaves Codex unauthenticated, forcing kandev to seed/symlink the user's credentials. Repeatable `-c` overrides inject only the MCP servers, touch nothing else, and never write to disk. +- **Keep the single Claude-shaped writer and bolt on per-CLI special cases (rejected).** The output schemas and delivery mechanisms diverge enough (TOML-via-flags vs JSON file vs env-selected JSON vs project file) that a strategy per agent is clearer than branching inside one writer. +- **A `kind` enum dispatched by the runtime (rejected).** A true strategy object per agent keeps the per-CLI emission logic with its schema and avoids a growing switch in the runtime. +- **Emit Codex's `experimental_use_rmcp_client` for remote servers (deferred).** Some intermediate Codex versions gated streamable-HTTP MCP behind that flag; current Codex loads `url` servers natively, and kandev launches the latest via `npx`. Emitting a global experimental flag risks affecting the user's other servers or erroring on versions where the key changed. Documented at the call site instead. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 4cde043ca..6086b1157 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -19,3 +19,4 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 0011 | [Transient provider errors (529 Overloaded) auto-retry with visible backoff](0011-transient-provider-error-retry.md) | accepted | backend, frontend | 2026-05-30 | | 0012 | [Service-only UI self-update](0012-service-only-self-update.md) | accepted | backend, frontend, cli | 2026-05-29 | | 0013 | [Multi-branch task support — N (repo, branch) pairs per task](0013-multi-branch-tasks.md) | accepted | backend, frontend | 2026-06-01 | +| 0014 | [Per-CLI MCP server injection for passthrough mode](0014-passthrough-mcp-injection-strategies.md) | accepted | backend | 2026-05-29 |