fix: return explicit error when ref-based MCP resolves to remote server with working_dir

When a toolset uses ref: to point to a catalog entry that resolves to a
remote server at runtime, working_dir cannot be validated at config-parse
time (the transport type is only known after the catalog API call). The
previous code silently discarded working_dir in this case.

- Fix the misleading comment in createMCPTool that claimed the remote
  branch was unreachable due to validation; ref-based MCPs are not
  blocked at validation time.
- Defer the checkDirExists call for ref-based toolsets until after the
  server spec is resolved (avoids unnecessary work for remote refs).
- Return a clear error when serverSpec.Type == "remote" and
  working_dir is set, instead of silently discarding the field.
- Update the Remote.URL branch comment to accurately describe what it
  does and does not cover.
- Add gateway.OverrideCatalogForTesting helper so teamloader tests can
  seed a fake catalog without a live network call.
- Add TestMain in pkg/teamloader that seeds remote and local catalog
  entries for use by tests.
- Add TestCreateMCPTool_RefRemote_WorkingDir_ReturnsError and
  TestCreateMCPTool_RefRemote_NoWorkingDir_Succeeds.

Closes #2461

Assisted-By: docker-agent

Author: simonferquel-clanker

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
+	}
+}

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())
+}

pkg/teamloader/registry.go

@@ -298,13 +298,18 @@ func createMCPTool(ctx context.Context, toolset latest.Toolset, _ string, runCon
 	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.
+	// 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.
-	if toolset.WorkingDir != "" {
+	// 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
 		}
@@ -321,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)
@@ -361,7 +380,9 @@ func createMCPTool(ctx context.Context, toolset latest.Toolset, _ string, runCon
 
 		return mcp.NewToolsetCommand(toolset.Name, resolvedCommand, toolset.Args, env, cwd), nil
 
-	// Remote MCP Server — working_dir is rejected at validation time for this branch.
+	// 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)
 

pkg/teamloader/registry_test.go

@@ -273,3 +273,47 @@ func TestCreateLSPTool_WorkingDir_ReachesHandler(t *testing.T) {
 	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)
+}