Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions agent-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down Expand Up @@ -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,
Expand Down
29 changes: 29 additions & 0 deletions examples/toolset-working-dir.yaml
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions pkg/config/latest/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions pkg/config/latest/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM: Validation gap — working_dir not rejected for ref:-based MCPs that may resolve to remote at runtime

This check only blocks working_dir when Remote.URL is set explicitly:

if t.WorkingDir != "" && t.Type == "mcp" && t.Remote.URL != "" {
    return errors.New("working_dir is not valid for remote MCP toolsets (no local subprocess)")
}

ref:-based MCPs (e.g. ref: docker:context7) are not covered. Their serverSpec.Type is only known at runtime (via the MCP Catalog API), so if the catalog entry resolves to a remote server, working_dir is silently dropped with no user-visible error.

The companion misleading comment in registry.go ("The remote branch never reaches here because working_dir is rejected by validation for toolsets with a remote.url") reinforces this false assumption.

Suggested fix: At minimum, update the comment in registry.go to accurately describe that only explicit-remote.url toolsets are guarded at validation time, not Ref-based ones. Optionally emit a warning when a Ref-based MCP with working_dir resolves to remote at runtime.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit af057f34.

  • Updated the comment on this validation rule to clarify it only blocks explicit remote.url toolsets; ref-based MCPs are not covered at parse time because their transport type is only known at runtime.
  • Added a runtime check in the serverSpec.Type == "remote" branch inside createMCPTool: if toolset.WorkingDir != "", an explicit error is returned ("working_dir is not supported for MCP toolset ... ref ... resolves to a remote server") instead of silently discarding the field.
  • Also deferred the checkDirExists call for ref-based toolsets so it only runs after the server spec is resolved (avoids unnecessary work when the ref turns out to be remote).
  • Added gateway.OverrideCatalogForTesting helper + TestMain in pkg/teamloader to enable unit tests without network, plus TestCreateMCPTool_RefRemote_WorkingDir_ReturnsError and TestCreateMCPTool_RefRemote_NoWorkingDir_Succeeds.

return errors.New("working_dir is not valid for remote MCP toolsets (no local subprocess)")
}

switch t.Type {
case "shell":
Expand Down
101 changes: 101 additions & 0 deletions pkg/config/latest/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
})
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/config/mcps.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 22 additions & 0 deletions pkg/config/mcps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
23 changes: 23 additions & 0 deletions pkg/config/testdata/mcp_definitions_working_dir.yaml
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions pkg/gateway/testing.go
Original file line number Diff line number Diff line change
@@ -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
}
}
29 changes: 29 additions & 0 deletions pkg/teamloader/main_test.go
Original file line number Diff line number Diff line change
@@ -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())
}
Loading
Loading