diff --git a/agent-schema.json b/agent-schema.json index 88f54e0b0..7713f5eb4 100644 --- a/agent-schema.json +++ b/agent-schema.json @@ -901,6 +901,10 @@ } } ] + }, + "working_dir": { + "type": "string", + "description": "Optional working directory for the MCP server process. Relative paths are resolved relative to the agent's working directory. Only valid for subprocess-based MCP types (command or ref); not supported for remote MCP toolsets." } }, "anyOf": [ @@ -1141,6 +1145,10 @@ "version": { "type": "string", "description": "Package reference for auto-installation of MCP/LSP tool binaries. Format: 'owner/repo' or 'owner/repo@version'. Set to 'false' to disable auto-install for this toolset." + }, + "working_dir": { + "type": "string", + "description": "Optional working directory for MCP/LSP toolset processes. Relative paths are resolved relative to the agent's working directory. Only valid for type 'mcp' or 'lsp', and not supported for remote MCP toolsets (no local subprocess)." } }, "additionalProperties": false, diff --git a/docs/configuration/tools/index.md b/docs/configuration/tools/index.md index 3bcc57999..8659f814b 100644 --- a/docs/configuration/tools/index.md +++ b/docs/configuration/tools/index.md @@ -65,6 +65,7 @@ Browse available tools at the [Docker MCP Catalog](https://hub.docker.com/search | `tools` | array | Optional: only expose these tools | | `instruction` | string | Custom instructions injected into the agent's context | | `config` | any | MCP server-specific configuration (passed during initialization) | +| `working_dir` | string | Working directory for the MCP gateway subprocess. Only applies when the catalog entry runs as a local process (not remote). Relative paths are resolved against the agent's working directory. | ### Local MCP (stdio) @@ -86,6 +87,7 @@ toolsets: | `args` | array | Command arguments | | `tools` | array | Optional: only expose these tools | | `env` | object | Environment variables (key-value pairs) | +| `working_dir` | string | Working directory for the MCP server process. Relative paths are resolved against the agent's working directory. Defaults to the agent's working directory when omitted. | | `instruction` | string | Custom instructions injected into the agent's context | | `version` | string | Package reference for [auto-installing](#auto-installing-tools) the command binary | diff --git a/docs/tools/lsp/index.md b/docs/tools/lsp/index.md index f292d7d02..bb67e6932 100644 --- a/docs/tools/lsp/index.md +++ b/docs/tools/lsp/index.md @@ -65,6 +65,7 @@ agents: | `args` | array | ✗ | Command-line arguments for the LSP server | | `env` | object | ✗ | Environment variables for the LSP process | | `file_types` | array | ✗ | File extensions this LSP handles (e.g., `[".go", ".mod"]`) | +| `working_dir` | string | ✗ | Working directory for the LSP server process. Relative paths are resolved against the agent's working directory. Defaults to the agent's working directory when omitted. | | `version` | string | ✗ | Package reference for [auto-installing]({{ '/configuration/tools/#auto-installing-tools' | relative_url }}) the command binary | ## Common LSP Servers @@ -81,6 +82,16 @@ toolsets: file_types: [".go"] ``` +If your Go module lives in a subdirectory (e.g. a monorepo where `go.mod` is under `./backend`), set `working_dir` so `gopls` is started from the module root: + +```yaml +toolsets: + - type: lsp + command: gopls + file_types: [".go"] + working_dir: ./backend # gopls must be started from the module root +``` + ### TypeScript/JavaScript (typescript-language-server) ```yaml diff --git a/examples/toolset-working-dir.yaml b/examples/toolset-working-dir.yaml new file mode 100644 index 000000000..8b91e44eb --- /dev/null +++ b/examples/toolset-working-dir.yaml @@ -0,0 +1,29 @@ +#!/usr/bin/env docker agent run + +# Example: using working_dir for MCP and LSP toolsets +# +# Some language servers and MCP tools must be started from a specific directory. +# For example, gopls must be started from the Go module root. Use `working_dir` +# to configure the launch directory for any MCP or LSP toolset. +# +# `working_dir` is: +# - Optional (defaults to the agent's working directory when omitted) +# - Resolved relative to the agent's working directory if it is a relative path + +agents: + root: + model: openai/gpt-5-mini + description: Example agent demonstrating working_dir for MCP and LSP toolsets + instruction: | + You are a helpful coding assistant with access to language server and MCP tools + launched from their respective project directories. + toolsets: + # LSP server started from a subdirectory (e.g. a Go module in ./backend) + - type: lsp + command: gopls + working_dir: ./backend + + # MCP server started from a specific tools directory + - type: mcp + command: my-mcp-server + working_dir: ./tools/mcp diff --git a/pkg/config/latest/types.go b/pkg/config/latest/types.go index fc936f5ad..120353e4d 100644 --- a/pkg/config/latest/types.go +++ b/pkg/config/latest/types.go @@ -741,6 +741,11 @@ type Toolset struct { // For the `model_picker` tool Models []string `json:"models,omitempty"` + + // For `mcp` and `lsp` tools - optional working directory override. + // When set, the toolset process is started from this directory. + // Relative paths are resolved relative to the agent's working directory. + WorkingDir string `json:"working_dir,omitempty"` } func (t *Toolset) UnmarshalYAML(unmarshal func(any) error) error { diff --git a/pkg/config/latest/validate.go b/pkg/config/latest/validate.go index c99ce4748..5564e8f11 100644 --- a/pkg/config/latest/validate.go +++ b/pkg/config/latest/validate.go @@ -114,6 +114,13 @@ func (t *Toolset) validate() error { if t.RAGConfig != nil && t.Type != "rag" { return errors.New("rag_config can only be used with type 'rag'") } + if t.WorkingDir != "" && t.Type != "mcp" && t.Type != "lsp" { + return errors.New("working_dir can only be used with type 'mcp' or 'lsp'") + } + // working_dir requires a local subprocess; it is meaningless for remote MCP toolsets. + if t.WorkingDir != "" && t.Type == "mcp" && t.Remote.URL != "" { + return errors.New("working_dir is not valid for remote MCP toolsets (no local subprocess)") + } switch t.Type { case "shell": diff --git a/pkg/config/latest/validate_test.go b/pkg/config/latest/validate_test.go index 6c90b9be7..d6a41b919 100644 --- a/pkg/config/latest/validate_test.go +++ b/pkg/config/latest/validate_test.go @@ -97,6 +97,106 @@ agents: `, wantErr: "file_types can only be used with type 'lsp'", }, + { + name: "lsp with working_dir", + config: ` +version: "8" +agents: + root: + model: "openai/gpt-4" + toolsets: + - type: lsp + command: gopls + working_dir: ./backend +`, + wantErr: "", + }, + { + name: "working_dir on non-mcp-lsp toolset is rejected", + config: ` +version: "8" +agents: + root: + model: "openai/gpt-4" + toolsets: + - type: shell + working_dir: ./backend +`, + wantErr: "working_dir can only be used with type 'mcp' or 'lsp'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var cfg Config + err := yaml.Unmarshal([]byte(tt.config), &cfg) + + if tt.wantErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tt.wantErr) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestToolset_Validate_MCP_WorkingDir(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + config string + wantErr string + wantValue string + }{ + { + name: "mcp with working_dir", + config: ` +version: "8" +agents: + root: + model: "openai/gpt-4" + toolsets: + - type: mcp + command: my-mcp-server + working_dir: ./tools/mcp +`, + wantErr: "", + wantValue: "./tools/mcp", + }, + { + name: "mcp without working_dir defaults to empty", + config: ` +version: "8" +agents: + root: + model: "openai/gpt-4" + toolsets: + - type: mcp + command: my-mcp-server +`, + wantErr: "", + wantValue: "", + }, + { + name: "working_dir on remote mcp is rejected", + config: ` +version: "8" +agents: + root: + model: "openai/gpt-4" + toolsets: + - type: mcp + remote: + url: https://mcp.example.com/sse + working_dir: ./tools +`, + wantErr: "working_dir is not valid for remote MCP toolsets", + wantValue: "", + }, } for _, tt := range tests { @@ -111,6 +211,7 @@ agents: require.Contains(t, err.Error(), tt.wantErr) } else { require.NoError(t, err) + require.Equal(t, tt.wantValue, cfg.Agents.First().Toolsets[0].WorkingDir) } }) } diff --git a/pkg/config/mcps.go b/pkg/config/mcps.go index a47a2617a..d65dd0858 100644 --- a/pkg/config/mcps.go +++ b/pkg/config/mcps.go @@ -86,6 +86,13 @@ func applyMCPDefaults(ts, def *latest.Toolset) { if ts.Defer.IsEmpty() { ts.Defer = def.Defer } + if ts.WorkingDir == "" { + // An empty working_dir in the referencing toolset is treated as "unset": + // inherit the definition's value. This matches the semantics of all other + // string fields in this function. An explicit `working_dir: ""` in YAML + // is indistinguishable from omission and will therefore be overridden. + ts.WorkingDir = def.WorkingDir + } if len(def.Env) > 0 { merged := make(map[string]string, len(def.Env)+len(ts.Env)) maps.Copy(merged, def.Env) diff --git a/pkg/config/mcps_test.go b/pkg/config/mcps_test.go index cfe2fa922..6965d9a1b 100644 --- a/pkg/config/mcps_test.go +++ b/pkg/config/mcps_test.go @@ -136,3 +136,25 @@ func TestMCPDefinitions_EnvMerge(t *testing.T) { // Toolset-only key is preserved assert.Equal(t, "from_toolset", ts.Env["EXTRA"]) } + +func TestMCPDefinitions_WorkingDir(t *testing.T) { + t.Parallel() + + cfg, err := Load(t.Context(), NewFileSource("testdata/mcp_definitions_working_dir.yaml")) + require.NoError(t, err) + + // WorkingDir from the definition is inherited by the referencing toolset. + root, ok := cfg.Agents.Lookup("root") + require.True(t, ok) + require.Len(t, root.Toolsets, 1) + ts := root.Toolsets[0] + assert.Equal(t, "my-mcp-server", ts.Command) + assert.Equal(t, "./tools/mcp", ts.WorkingDir) + + // A toolset-level working_dir overrides the definition's value. + override, ok := cfg.Agents.Lookup("override") + require.True(t, ok) + require.Len(t, override.Toolsets, 1) + tsOverride := override.Toolsets[0] + assert.Equal(t, "./override/path", tsOverride.WorkingDir) +} diff --git a/pkg/config/testdata/mcp_definitions_working_dir.yaml b/pkg/config/testdata/mcp_definitions_working_dir.yaml new file mode 100644 index 000000000..c87ffd426 --- /dev/null +++ b/pkg/config/testdata/mcp_definitions_working_dir.yaml @@ -0,0 +1,23 @@ +version: "8" +models: + model: + provider: openai + model: gpt-4o + +mcps: + custom_mcp_with_dir: + command: my-mcp-server + working_dir: ./tools/mcp + +agents: + root: + model: model + toolsets: + - type: mcp + ref: custom_mcp_with_dir + override: + model: model + toolsets: + - type: mcp + ref: custom_mcp_with_dir + working_dir: ./override/path diff --git a/pkg/gateway/testing.go b/pkg/gateway/testing.go new file mode 100644 index 000000000..bd0e1ca3d --- /dev/null +++ b/pkg/gateway/testing.go @@ -0,0 +1,10 @@ +package gateway + +// OverrideCatalogForTesting replaces the catalog loader with a fixed function +// that returns the given catalog. It should only be called from TestMain or +// equivalent test-setup code, before any call to ServerSpec. +func OverrideCatalogForTesting(catalog Catalog) { + catalogOnce = func() (Catalog, error) { + return catalog, nil + } +} diff --git a/pkg/teamloader/main_test.go b/pkg/teamloader/main_test.go new file mode 100644 index 000000000..5f70b5f6a --- /dev/null +++ b/pkg/teamloader/main_test.go @@ -0,0 +1,29 @@ +package teamloader + +import ( + "os" + "testing" + + "github.com/docker/docker-agent/pkg/gateway" +) + +// TestMain seeds a fake MCP catalog so that teamloader tests that invoke +// createMCPTool with a ref can run without a live network call. +func TestMain(m *testing.M) { + gateway.OverrideCatalogForTesting(gateway.Catalog{ + // A local (subprocess-based) server entry. + "local-server": { + Type: "server", + }, + // A remote (no subprocess) server entry — used to test that + // working_dir is rejected at runtime for ref-based remote MCPs. + "remote-server": { + Type: "remote", + Remote: gateway.Remote{ + URL: "https://mcp.example.com/sse", + TransportType: "sse", + }, + }, + }) + os.Exit(m.Run()) +} diff --git a/pkg/teamloader/registry.go b/pkg/teamloader/registry.go index df3dfb7a8..bbb9ab03f 100644 --- a/pkg/teamloader/registry.go +++ b/pkg/teamloader/registry.go @@ -86,6 +86,62 @@ func NewDefaultToolsetRegistry() *ToolsetRegistry { return r } +// checkDirExists returns an error if the given directory does not exist or is +// not a directory. toolsetType is used only in the error message. +func checkDirExists(dir, toolsetType string) error { + info, err := os.Stat(dir) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("working_dir %q for %s toolset does not exist", dir, toolsetType) + } + return fmt.Errorf("working_dir %q for %s toolset: %w", dir, toolsetType, err) + } + if !info.IsDir() { + return fmt.Errorf("working_dir %q for %s toolset is not a directory", dir, toolsetType) + } + return nil +} + +// resolveToolsetWorkingDir returns the effective working directory for a toolset process. +// +// Resolution rules: +// - If toolsetWorkingDir is empty, agentWorkingDir is returned unchanged. +// - Shell patterns (~ and ${VAR}/$VAR) are expanded before any further processing. +// - If the expanded path is absolute, it is returned as-is. +// - If the expanded path is relative and agentWorkingDir is non-empty, +// it is joined with agentWorkingDir and made absolute via filepath.Abs. +// - If the expanded path is relative and agentWorkingDir is empty, +// the relative path is returned unchanged (caller will inherit the process cwd). +// +// Note: unlike resolveToolsetPath, this helper does not enforce containment +// within the agent working directory. working_dir is treated like command/args — +// a trusted, operator-authored value where cross-tree references (e.g. a sibling +// module root in a monorepo) are intentional and must not be silently blocked. +func resolveToolsetWorkingDir(toolsetWorkingDir, agentWorkingDir string) string { + if toolsetWorkingDir == "" { + return agentWorkingDir + } + // Expand ~ and environment variables before path operations. + toolsetWorkingDir = path.ExpandPath(toolsetWorkingDir) + if filepath.IsAbs(toolsetWorkingDir) { + return toolsetWorkingDir + } + if agentWorkingDir != "" { + // filepath.Abs cleans the result and anchors the URI correctly + // (avoids file://./backend-style LSP root URIs when the agent dir + // is itself absolute, which is the normal case). + abs, err := filepath.Abs(filepath.Join(agentWorkingDir, toolsetWorkingDir)) + if err == nil { + return abs + } + // Fallback: return the joined path without Abs (should not happen in practice). + return filepath.Join(agentWorkingDir, toolsetWorkingDir) + } + // agentWorkingDir is empty and path is relative: return as-is. + // The child process will inherit the OS working directory. + return toolsetWorkingDir +} + // resolveToolsetPath expands shell patterns (~, env vars) in the given path, // then validates it relative to the working directory or parent directory. func resolveToolsetPath(toolsetPath, parentDir string, runConfig *config.RuntimeConfig) (string, error) { @@ -241,6 +297,24 @@ func createFetchTool(_ context.Context, toolset latest.Toolset, _ string, _ *con func createMCPTool(ctx context.Context, toolset latest.Toolset, _ string, runConfig *config.RuntimeConfig, _ string) (tools.ToolSet, error) { envProvider := runConfig.EnvProvider() + // Resolve the working directory once; used for all subprocess-based branches. + // Note: validation only rejects working_dir for toolsets with an explicit + // remote.url. Ref-based MCPs (e.g. ref: docker:context7) pass validation + // regardless, because their transport type is only known at runtime via the + // MCP Catalog API. If such a ref resolves to a remote server at runtime, we + // return an explicit error below rather than silently discarding the field. + cwd := resolveToolsetWorkingDir(toolset.WorkingDir, runConfig.WorkingDir) + + // S1: validate the resolved directory exists (if one was specified) so we + // surface a clear error now rather than a cryptic exec failure later. + // Skip this check for ref-based toolsets whose transport type is not yet + // known — the check would be premature and potentially wrong. + if toolset.WorkingDir != "" && toolset.Ref == "" { + if err := checkDirExists(cwd, "mcp"); err != nil { + return nil, err + } + } + switch { // MCP Server from the MCP Catalog, running with the MCP Gateway case toolset.Ref != "": @@ -252,9 +326,23 @@ func createMCPTool(ctx context.Context, toolset latest.Toolset, _ string, runCon // TODO(dga): until the MCP Gateway supports oauth with docker agent, we fetch the remote url and directly connect to it. if serverSpec.Type == "remote" { + // working_dir cannot be validated at config-parse time for ref-based + // MCPs because their transport type is only known here. Return a clear + // error rather than silently discarding the field. + if toolset.WorkingDir != "" { + return nil, fmt.Errorf("working_dir is not supported for MCP toolset %q: ref %q resolves to a remote server (no local subprocess)", + toolset.Name, toolset.Ref) + } return mcp.NewRemoteToolset(toolset.Name, serverSpec.Remote.URL, serverSpec.Remote.TransportType, nil, nil), nil } + // The ref resolves to a local subprocess — validate the working directory now. + if toolset.WorkingDir != "" { + if err := checkDirExists(cwd, "mcp"); err != nil { + return nil, err + } + } + env, err := environment.ExpandAll(ctx, environment.ToValues(toolset.Env), envProvider) if err != nil { return nil, fmt.Errorf("failed to expand the tool's environment variables: %w", err) @@ -265,7 +353,8 @@ func createMCPTool(ctx context.Context, toolset latest.Toolset, _ string, runCon envProvider, ) - return mcp.NewGatewayToolset(ctx, toolset.Name, mcpServerName, serverSpec.Secrets, toolset.Config, envProvider, runConfig.WorkingDir) + // Pass the resolved cwd so gateway-based MCPs also honour working_dir. + return mcp.NewGatewayToolset(ctx, toolset.Name, mcpServerName, serverSpec.Secrets, toolset.Config, envProvider, cwd) // STDIO MCP Server from shell command case toolset.Command != "": @@ -289,9 +378,11 @@ func createMCPTool(ctx context.Context, toolset latest.Toolset, _ string, runCon // Prepend tools bin dir to PATH so child processes can find installed tools env = toolinstall.PrependBinDirToEnv(env) - return mcp.NewToolsetCommand(toolset.Name, resolvedCommand, toolset.Args, env, runConfig.WorkingDir), nil + return mcp.NewToolsetCommand(toolset.Name, resolvedCommand, toolset.Args, env, cwd), nil - // Remote MCP Server + // Remote MCP Server — working_dir is rejected at validation time for this + // branch (explicit remote.url in config). Ref-based MCPs that resolve to + // remote at runtime are handled with an explicit error in the Ref branch above. case toolset.Remote.URL != "": expander := js.NewJsExpander(envProvider) @@ -329,7 +420,17 @@ func createLSPTool(ctx context.Context, toolset latest.Toolset, _ string, runCon // Prepend tools bin dir to PATH so child processes can find installed tools env = toolinstall.PrependBinDirToEnv(env) - tool := builtin.NewLSPTool(resolvedCommand, toolset.Args, env, runConfig.WorkingDir) + cwd := resolveToolsetWorkingDir(toolset.WorkingDir, runConfig.WorkingDir) + + // S1: validate the resolved directory exists (if one was specified) so we + // surface a clear error now rather than a cryptic exec failure later. + if toolset.WorkingDir != "" { + if err := checkDirExists(cwd, "lsp"); err != nil { + return nil, err + } + } + + tool := builtin.NewLSPTool(resolvedCommand, toolset.Args, env, cwd) if len(toolset.FileTypes) > 0 { tool.SetFileTypes(toolset.FileTypes) } diff --git a/pkg/teamloader/registry_test.go b/pkg/teamloader/registry_test.go index 75c9fde86..33be35ef0 100644 --- a/pkg/teamloader/registry_test.go +++ b/pkg/teamloader/registry_test.go @@ -1,6 +1,8 @@ package teamloader import ( + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -10,6 +12,8 @@ import ( "github.com/docker/docker-agent/pkg/config/latest" "github.com/docker/docker-agent/pkg/environment" "github.com/docker/docker-agent/pkg/tools" + "github.com/docker/docker-agent/pkg/tools/builtin" + mcptool "github.com/docker/docker-agent/pkg/tools/mcp" ) func TestCreateShellTool(t *testing.T) { @@ -70,3 +74,246 @@ func TestCreateMCPTool_BareCommandNotFound_CreatesToolsetAnyway(t *testing.T) { require.NotNil(t, tool) assert.Equal(t, "mcp(stdio cmd=some-nonexistent-mcp-binary)", tools.DescribeToolSet(tool)) } + +func TestResolveToolsetWorkingDir(t *testing.T) { + t.Parallel() + + home, err := os.UserHomeDir() + require.NoError(t, err) + + tests := []struct { + name string + toolsetWorkingDir string + agentWorkingDir string + want string + }{ + { + name: "empty toolset dir returns agent dir", + toolsetWorkingDir: "", + agentWorkingDir: "/workspace", + want: "/workspace", + }, + { + name: "absolute toolset dir is returned as-is", + toolsetWorkingDir: "/tmp/mcp", + agentWorkingDir: "/workspace", + want: "/tmp/mcp", + }, + { + name: "relative toolset dir is joined with agent dir", + toolsetWorkingDir: "./backend", + agentWorkingDir: "/workspace", + want: "/workspace/backend", + }, + { + name: "bare relative dir joined with agent dir", + toolsetWorkingDir: "tools/mcp", + agentWorkingDir: "/workspace", + want: "/workspace/tools/mcp", + }, + { + name: "relative toolset dir with empty agent dir returns toolset dir unchanged", + toolsetWorkingDir: "./backend", + agentWorkingDir: "", + want: "./backend", + }, + { + name: "both empty returns empty", + toolsetWorkingDir: "", + agentWorkingDir: "", + want: "", + }, + // Tilde expansion tests (B2) + { + name: "tilde expands to home dir", + toolsetWorkingDir: "~/projects/app", + agentWorkingDir: "/workspace", + want: filepath.Join(home, "projects", "app"), + }, + { + name: "bare tilde expands to home dir", + toolsetWorkingDir: "~", + agentWorkingDir: "/workspace", + want: home, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := resolveToolsetWorkingDir(tt.toolsetWorkingDir, tt.agentWorkingDir) + assert.Equal(t, tt.want, got) + }) + } +} + +// TestResolveToolsetWorkingDir_EnvVarExpansion tests env-var expansion separately +// because t.Setenv is incompatible with t.Parallel on the parent test. +func TestResolveToolsetWorkingDir_EnvVarExpansion(t *testing.T) { + t.Setenv("TEST_REGISTRY_CWD_VAR", "/custom/path") + + got := resolveToolsetWorkingDir("${TEST_REGISTRY_CWD_VAR}/app", "/workspace") + assert.Equal(t, "/custom/path/app", got) +} + +// TestCreateMCPTool_WorkingDir_ReachesSubprocess verifies that working_dir is +// wired all the way through createMCPTool to the underlying stdio command (N5). +func TestCreateMCPTool_WorkingDir_ReachesSubprocess(t *testing.T) { + t.Setenv("DOCKER_AGENT_TOOLS_DIR", t.TempDir()) + + // Create a real temporary directory so the existence check passes. + customDir := t.TempDir() + agentDir := t.TempDir() + + toolset := latest.Toolset{ + Type: "mcp", + Command: "some-nonexistent-mcp-binary", + WorkingDir: customDir, + } + + registry := NewDefaultToolsetRegistry() + runConfig := &config.RuntimeConfig{ + Config: config.Config{WorkingDir: agentDir}, + EnvProviderForTests: environment.NewOsEnvProvider(), + } + + rawTool, err := registry.CreateTool(t.Context(), toolset, ".", runConfig, "test-agent") + require.NoError(t, err) + require.NotNil(t, rawTool) + + // Assert the CWD reached the inner stdio command. + ts, ok := rawTool.(*mcptool.Toolset) + require.True(t, ok, "expected *mcp.Toolset") + assert.Equal(t, customDir, ts.WorkingDir()) +} + +// TestCreateMCPTool_RelativeWorkingDir_ResolvedAgainstAgentDir verifies that a +// relative working_dir is resolved against the agent's working directory. +func TestCreateMCPTool_RelativeWorkingDir_ResolvedAgainstAgentDir(t *testing.T) { + t.Setenv("DOCKER_AGENT_TOOLS_DIR", t.TempDir()) + + agentDir := t.TempDir() + // Create the subdirectory so the existence check passes. + subDir := filepath.Join(agentDir, "tools", "mcp") + require.NoError(t, os.MkdirAll(subDir, 0o700)) + + toolset := latest.Toolset{ + Type: "mcp", + Command: "some-nonexistent-mcp-binary", + WorkingDir: "tools/mcp", + } + + registry := NewDefaultToolsetRegistry() + runConfig := &config.RuntimeConfig{ + Config: config.Config{WorkingDir: agentDir}, + EnvProviderForTests: environment.NewOsEnvProvider(), + } + + rawTool, err := registry.CreateTool(t.Context(), toolset, ".", runConfig, "test-agent") + require.NoError(t, err) + require.NotNil(t, rawTool) + + ts, ok := rawTool.(*mcptool.Toolset) + require.True(t, ok, "expected *mcp.Toolset") + assert.Equal(t, subDir, ts.WorkingDir()) +} + +// TestCreateMCPTool_NonexistentWorkingDir_ReturnsError verifies that a +// non-existent working_dir surfaces a clear error at tool-creation time (S1). +func TestCreateMCPTool_NonexistentWorkingDir_ReturnsError(t *testing.T) { + t.Setenv("DOCKER_AGENT_TOOLS_DIR", t.TempDir()) + + // Use a path that is guaranteed not to exist by creating a tempdir and + // appending a non-existent subdir (avoids flakes on hosts where a + // hard-coded path might coincidentally exist). + nonExistent := filepath.Join(t.TempDir(), "missing") + + toolset := latest.Toolset{ + Type: "mcp", + Command: "some-nonexistent-mcp-binary", + WorkingDir: nonExistent, + } + + registry := NewDefaultToolsetRegistry() + runConfig := &config.RuntimeConfig{ + Config: config.Config{WorkingDir: t.TempDir()}, + EnvProviderForTests: environment.NewOsEnvProvider(), + } + + _, err := registry.CreateTool(t.Context(), toolset, ".", runConfig, "test-agent") + require.Error(t, err) + assert.Contains(t, err.Error(), "working_dir") + assert.Contains(t, err.Error(), "does not exist") +} + +// TestCreateLSPTool_WorkingDir_ReachesHandler verifies that working_dir is +// wired all the way through createLSPTool to the LSP handler (N5). +func TestCreateLSPTool_WorkingDir_ReachesHandler(t *testing.T) { + // Create a real temporary directory so the existence check passes. + customDir := t.TempDir() + agentDir := t.TempDir() + + toolset := latest.Toolset{ + Type: "lsp", + Command: "gopls", + WorkingDir: customDir, + } + + registry := NewDefaultToolsetRegistry() + runConfig := &config.RuntimeConfig{ + Config: config.Config{WorkingDir: agentDir}, + EnvProviderForTests: environment.NewOsEnvProvider(), + } + + rawTool, err := registry.CreateTool(t.Context(), toolset, ".", runConfig, "test-agent") + require.NoError(t, err) + require.NotNil(t, rawTool) + + lsp, ok := rawTool.(*builtin.LSPTool) + require.True(t, ok, "expected *builtin.LSPTool") + assert.Equal(t, customDir, lsp.WorkingDir()) +} + +// TestCreateMCPTool_RefRemote_WorkingDir_ReturnsError verifies that when a +// ref-based MCP resolves to a remote server at runtime, setting working_dir +// returns a clear error rather than silently discarding the field. +func TestCreateMCPTool_RefRemote_WorkingDir_ReturnsError(t *testing.T) { + // The "docker:remote-server" ref is seeded as type "remote" in TestMain. + toolset := latest.Toolset{ + Type: "mcp", + Ref: "docker:remote-server", + WorkingDir: "./workspace", + } + + registry := NewDefaultToolsetRegistry() + runConfig := &config.RuntimeConfig{ + Config: config.Config{WorkingDir: t.TempDir()}, + EnvProviderForTests: environment.NewOsEnvProvider(), + } + + _, err := registry.CreateTool(t.Context(), toolset, ".", runConfig, "test-agent") + require.Error(t, err) + assert.Contains(t, err.Error(), "working_dir is not supported") + assert.Contains(t, err.Error(), "remote server") +} + +// TestCreateMCPTool_RefRemote_NoWorkingDir_Succeeds verifies that a ref-based +// MCP that resolves to a remote server still works fine when working_dir is +// not set (the common case — regression guard). +func TestCreateMCPTool_RefRemote_NoWorkingDir_Succeeds(t *testing.T) { + // The "docker:remote-server" ref is seeded as type "remote" in TestMain. + toolset := latest.Toolset{ + Type: "mcp", + Ref: "docker:remote-server", + } + + registry := NewDefaultToolsetRegistry() + runConfig := &config.RuntimeConfig{ + Config: config.Config{WorkingDir: t.TempDir()}, + EnvProviderForTests: environment.NewOsEnvProvider(), + } + + tool, err := registry.CreateTool(t.Context(), toolset, ".", runConfig, "test-agent") + require.NoError(t, err) + require.NotNil(t, tool) +} diff --git a/pkg/tools/builtin/lsp.go b/pkg/tools/builtin/lsp.go index 28a4a3998..0cbb708f5 100644 --- a/pkg/tools/builtin/lsp.go +++ b/pkg/tools/builtin/lsp.go @@ -346,6 +346,12 @@ func (t *LSPTool) SetFileTypes(fileTypes []string) { t.handler.fileTypes = fileTypes } +// WorkingDir returns the working directory of the LSP server process. +// This is intended for testing only. +func (t *LSPTool) WorkingDir() string { + return t.handler.workingDir +} + // HandlesFile checks if this LSP handles the given file based on its extension. func (t *LSPTool) HandlesFile(path string) bool { return t.handler.handlesFile(path) diff --git a/pkg/tools/mcp/mcp.go b/pkg/tools/mcp/mcp.go index 24204b7af..d8a27fa2b 100644 --- a/pkg/tools/mcp/mcp.go +++ b/pkg/tools/mcp/mcp.go @@ -126,6 +126,16 @@ func NewRemoteToolset(name, urlString, transport string, headers map[string]stri // retries via ensureToolSetsAreStarted on the next conversation turn. var errServerUnavailable = errors.New("MCP server unavailable") +// WorkingDir returns the working directory of the underlying stdio client, +// or an empty string if this toolset uses a remote transport. +// This is intended for testing only. +func (ts *Toolset) WorkingDir() string { + if c, ok := ts.mcpClient.(*stdioMCPClient); ok { + return c.cwd + } + return "" +} + // Describe returns a short, user-visible description of this toolset instance. // It never includes secrets. func (ts *Toolset) Describe() string {