From 218d18eb6f0dfd16db0f5f12ffb23f532f4e5278 Mon Sep 17 00:00:00 2001 From: Simon Ferquel's Clanker Date: Fri, 17 Apr 2026 14:02:46 +0000 Subject: [PATCH 1/3] feat: add optional working_dir to MCP and LSP toolset configs - Add WorkingDir field to Toolset struct (pkg/config/latest/types.go) - Validate that working_dir is only used with type 'mcp' or 'lsp' - Resolve working_dir relative to agent's working directory at process start - Propagate working_dir from top-level mcps: definitions to agent toolsets - Add resolveToolsetWorkingDir helper in registry.go - Add tests: validate_test.go, mcps_test.go, registry_test.go - Add example: examples/toolset-working-dir.yaml - Update agent-schema.json for Toolset and MCPToolset Closes #2459 Assisted-By: docker-agent --- agent-schema.json | 8 ++ examples/toolset-working-dir.yaml | 29 +++++++ pkg/config/latest/types.go | 5 ++ pkg/config/latest/validate.go | 3 + pkg/config/latest/validate_test.go | 85 +++++++++++++++++++ pkg/config/mcps.go | 3 + pkg/config/mcps_test.go | 22 +++++ .../testdata/mcp_definitions_working_dir.yaml | 23 +++++ pkg/teamloader/registry.go | 22 ++++- pkg/teamloader/registry_test.go | 56 ++++++++++++ 10 files changed, 254 insertions(+), 2 deletions(-) create mode 100644 examples/toolset-working-dir.yaml create mode 100644 pkg/config/testdata/mcp_definitions_working_dir.yaml diff --git a/agent-schema.json b/agent-schema.json index 88f54e0b0..70276d110 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." } }, "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'." } }, "additionalProperties": false, diff --git a/examples/toolset-working-dir.yaml b/examples/toolset-working-dir.yaml new file mode 100644 index 000000000..6a8046780 --- /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-4o + 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..c296e596b 100644 --- a/pkg/config/latest/validate.go +++ b/pkg/config/latest/validate.go @@ -114,6 +114,9 @@ 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'") + } switch t.Type { case "shell": diff --git a/pkg/config/latest/validate_test.go b/pkg/config/latest/validate_test.go index 6c90b9be7..aa24b5b2a 100644 --- a/pkg/config/latest/validate_test.go +++ b/pkg/config/latest/validate_test.go @@ -97,6 +97,90 @@ 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: "", + }, } for _, tt := range tests { @@ -111,6 +195,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..83959d3b7 100644 --- a/pkg/config/mcps.go +++ b/pkg/config/mcps.go @@ -86,6 +86,9 @@ func applyMCPDefaults(ts, def *latest.Toolset) { if ts.Defer.IsEmpty() { ts.Defer = def.Defer } + if ts.WorkingDir == "" { + 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/teamloader/registry.go b/pkg/teamloader/registry.go index df3dfb7a8..7ea9ec106 100644 --- a/pkg/teamloader/registry.go +++ b/pkg/teamloader/registry.go @@ -86,6 +86,22 @@ func NewDefaultToolsetRegistry() *ToolsetRegistry { return r } +// resolveToolsetWorkingDir returns the effective working directory for a toolset process. +// If toolsetWorkingDir is set, it is resolved relative to agentWorkingDir (when relative). +// If toolsetWorkingDir is empty, agentWorkingDir is returned unchanged. +func resolveToolsetWorkingDir(toolsetWorkingDir, agentWorkingDir string) string { + if toolsetWorkingDir == "" { + return agentWorkingDir + } + if filepath.IsAbs(toolsetWorkingDir) { + return toolsetWorkingDir + } + if agentWorkingDir != "" { + return filepath.Join(agentWorkingDir, toolsetWorkingDir) + } + 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) { @@ -289,7 +305,8 @@ 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 + cwd := resolveToolsetWorkingDir(toolset.WorkingDir, runConfig.WorkingDir) + return mcp.NewToolsetCommand(toolset.Name, resolvedCommand, toolset.Args, env, cwd), nil // Remote MCP Server case toolset.Remote.URL != "": @@ -329,7 +346,8 @@ 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) + 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..347c4e1eb 100644 --- a/pkg/teamloader/registry_test.go +++ b/pkg/teamloader/registry_test.go @@ -70,3 +70,59 @@ 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() + + 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: "relative toolset dir without leading dot is 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: "", + }, + } + + 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) + }) + } +} From bac0d30e316ac777822231a9037a9e37d58eccfe Mon Sep 17 00:00:00 2001 From: Simon Ferquel's Clanker Date: Fri, 17 Apr 2026 14:19:32 +0000 Subject: [PATCH 2/3] fix: address reviewer feedback on working_dir feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B1: pass resolved CWD to NewGatewayToolset; gateway-based MCPs now honour working_dir. Remote MCP toolsets reject working_dir at validation time (no local subprocess). B2: call path.ExpandPath in resolveToolsetWorkingDir so ~ and ${VAR}/$VAR are expanded before path operations. Use filepath.Abs when joining a relative path with the agent dir (fixes file://./backend LSP root URI). S1: add checkDirExists helper; createMCPTool and createLSPTool now surface a clear error if working_dir does not exist at tool-creation time. S2: doc comment on resolveToolsetWorkingDir explains no-containment- check posture (working_dir is treated like command/args). S3: filepath.Abs on relative+non-empty-agentDir path ensures LSP rootURI is always absolute. S4: validation rejects working_dir on remote MCP toolsets. S5: comment in applyMCPDefaults explains empty-string inheritance semantics. S6: doc comment covers the empty-agent-dir+relative edge case. Nits: example model → gpt-5-mini; test name → 'bare relative dir'; schema descriptions aligned; import groups tidied; hard-coded non-existent path in test → t.TempDir()+'/missing'. N5: integration tests added: - TestCreateMCPTool_WorkingDir_ReachesSubprocess - TestCreateMCPTool_RelativeWorkingDir_ResolvedAgainstAgentDir - TestCreateMCPTool_NonexistentWorkingDir_ReturnsError - TestCreateLSPTool_WorkingDir_ReachesHandler Also add WorkingDir() accessors on *mcp.Toolset and *builtin.LSPTool to support the above integration tests. Assisted-By: docker-agent --- agent-schema.json | 4 +- examples/toolset-working-dir.yaml | 2 +- pkg/config/latest/validate.go | 4 + pkg/config/latest/validate_test.go | 16 ++++ pkg/config/mcps.go | 4 + pkg/teamloader/registry.go | 72 +++++++++++++- pkg/teamloader/registry_test.go | 149 ++++++++++++++++++++++++++++- pkg/tools/builtin/lsp.go | 6 ++ pkg/tools/mcp/mcp.go | 10 ++ 9 files changed, 258 insertions(+), 9 deletions(-) diff --git a/agent-schema.json b/agent-schema.json index 70276d110..7713f5eb4 100644 --- a/agent-schema.json +++ b/agent-schema.json @@ -904,7 +904,7 @@ }, "working_dir": { "type": "string", - "description": "Optional working directory for the MCP server process. Relative paths are resolved relative to the agent's working directory." + "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": [ @@ -1148,7 +1148,7 @@ }, "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'." + "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/examples/toolset-working-dir.yaml b/examples/toolset-working-dir.yaml index 6a8046780..8b91e44eb 100644 --- a/examples/toolset-working-dir.yaml +++ b/examples/toolset-working-dir.yaml @@ -12,7 +12,7 @@ agents: root: - model: openai/gpt-4o + 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 diff --git a/pkg/config/latest/validate.go b/pkg/config/latest/validate.go index c296e596b..5564e8f11 100644 --- a/pkg/config/latest/validate.go +++ b/pkg/config/latest/validate.go @@ -117,6 +117,10 @@ func (t *Toolset) validate() error { 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 aa24b5b2a..d6a41b919 100644 --- a/pkg/config/latest/validate_test.go +++ b/pkg/config/latest/validate_test.go @@ -181,6 +181,22 @@ agents: 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 { diff --git a/pkg/config/mcps.go b/pkg/config/mcps.go index 83959d3b7..d65dd0858 100644 --- a/pkg/config/mcps.go +++ b/pkg/config/mcps.go @@ -87,6 +87,10 @@ func applyMCPDefaults(ts, def *latest.Toolset) { 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 { diff --git a/pkg/teamloader/registry.go b/pkg/teamloader/registry.go index 7ea9ec106..6aeafbd34 100644 --- a/pkg/teamloader/registry.go +++ b/pkg/teamloader/registry.go @@ -86,19 +86,59 @@ 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. -// If toolsetWorkingDir is set, it is resolved relative to agentWorkingDir (when relative). -// If toolsetWorkingDir is empty, agentWorkingDir is returned unchanged. +// +// 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 } @@ -257,6 +297,19 @@ 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. + // The remote branch never reaches here because working_dir is rejected by + // validation for toolsets with a remote.url. + 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, "mcp"); err != nil { + return nil, err + } + } + switch { // MCP Server from the MCP Catalog, running with the MCP Gateway case toolset.Ref != "": @@ -281,7 +334,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 != "": @@ -305,10 +359,9 @@ 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) - cwd := resolveToolsetWorkingDir(toolset.WorkingDir, runConfig.WorkingDir) 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. case toolset.Remote.URL != "": expander := js.NewJsExpander(envProvider) @@ -347,6 +400,15 @@ func createLSPTool(ctx context.Context, toolset latest.Toolset, _ string, runCon env = toolinstall.PrependBinDirToEnv(env) 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 347c4e1eb..fe63f751c 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) { @@ -74,6 +78,9 @@ func TestCreateMCPTool_BareCommandNotFound_CreatesToolsetAnyway(t *testing.T) { func TestResolveToolsetWorkingDir(t *testing.T) { t.Parallel() + home, err := os.UserHomeDir() + require.NoError(t, err) + tests := []struct { name string toolsetWorkingDir string @@ -99,7 +106,7 @@ func TestResolveToolsetWorkingDir(t *testing.T) { want: "/workspace/backend", }, { - name: "relative toolset dir without leading dot is joined with agent dir", + name: "bare relative dir joined with agent dir", toolsetWorkingDir: "tools/mcp", agentWorkingDir: "/workspace", want: "/workspace/tools/mcp", @@ -116,6 +123,19 @@ func TestResolveToolsetWorkingDir(t *testing.T) { 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 { @@ -126,3 +146,130 @@ func TestResolveToolsetWorkingDir(t *testing.T) { }) } } + +// 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()) +} 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 { From 983470a57253d59363c26ee96119e9b6ea6389c3 Mon Sep 17 00:00:00 2001 From: Simon Ferquel's Clanker Date: Fri, 17 Apr 2026 14:24:01 +0000 Subject: [PATCH 3/3] fix: use filepath.Join with separate segments to satisfy gocritic filepathJoin lint rule Assisted-By: docker-agent --- pkg/teamloader/registry_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/teamloader/registry_test.go b/pkg/teamloader/registry_test.go index fe63f751c..9afc3e95c 100644 --- a/pkg/teamloader/registry_test.go +++ b/pkg/teamloader/registry_test.go @@ -128,7 +128,7 @@ func TestResolveToolsetWorkingDir(t *testing.T) { name: "tilde expands to home dir", toolsetWorkingDir: "~/projects/app", agentWorkingDir: "/workspace", - want: filepath.Join(home, "projects/app"), + want: filepath.Join(home, "projects", "app"), }, { name: "bare tilde expands to home dir",