Make it clearer which toolsets can use which configuration

Signed-off-by: David Gageot <david.gageot@docker.com>

Author: dgageot

…config/testdata/invalid_mcp_path_v2.yaml pkg/config/testdata/invalid_path_v2.yamlpkg/config/testdata/invalid_mcp_path_v2.yaml renamed to pkg/config/testdata/invalid_path_v2.yaml pkg/config/testdata/invalid_mcp_path_v2.yaml renamed to pkg/config/testdata/invalid_path_v2.yaml

@@ -0,0 +1,9 @@
+version: "2"
+
+agents:
+  root:
+    model: openai/gpt-4o
+    toolsets:
+      - type: mcp
+        post_edit:
+          - path: /path

pkg/config/testdata/invalid_post_edit_v2.yaml

@@ -67,24 +67,29 @@ type PostEditConfig struct {
 
 // Toolset represents a tool configuration
 type Toolset struct {
-	Type    string            `json:"type,omitempty" yaml:"type,omitempty"`
-	Ref     string            `json:"ref,omitempty" yaml:"ref,omitempty"`
-	Config  any               `json:"config,omitempty" yaml:"config,omitempty"`
-	Command string            `json:"command,omitempty" yaml:"command,omitempty"`
-	Remote  Remote            `json:"remote,omitempty" yaml:"remote,omitempty"`
-	Args    []string          `json:"args,omitempty" yaml:"args,omitempty"`
-	Tools   []string          `json:"tools,omitempty" yaml:"tools,omitempty"`
-	Env     map[string]string `json:"env,omitempty" yaml:"env,omitempty"`
-
-	// For the think tool
+	Type  string   `json:"type,omitempty" yaml:"type,omitempty"`
+	Tools []string `json:"tools,omitempty" yaml:"tools,omitempty"`
+
+	// For the `mcp` tool
+	Command string   `json:"command,omitempty" yaml:"command,omitempty"`
+	Args    []string `json:"args,omitempty" yaml:"args,omitempty"`
+	Ref     string   `json:"ref,omitempty" yaml:"ref,omitempty"`
+	Remote  Remote   `json:"remote,omitempty" yaml:"remote,omitempty"`
+	Config  any      `json:"config,omitempty" yaml:"config,omitempty"`
+
+	// For `shell`, `script` or `mcp` tools
+	Env map[string]string `json:"env,omitempty" yaml:"env,omitempty"`
+
+	// For the `todo` tool
 	Shared bool `json:"shared,omitempty" yaml:"shared,omitempty"`
-	// For the memory tool
+
+	// For the `memory` tool
 	Path string `json:"path,omitempty" yaml:"path,omitempty"`
 
-	// For the script tool
+	// For the `script` tool
 	Shell map[string]ScriptShellToolConfig `json:"shell,omitempty" yaml:"shell,omitempty"`
 
-	// For the filesystem tool - post-edit commands
+	// For the `filesystem` tool - post-edit commands
 	PostEdit []PostEditConfig `json:"post_edit,omitempty" yaml:"post_edit,omitempty"`
 }
 

pkg/config/v2/types.go

@@ -17,8 +17,8 @@ func (t *Config) UnmarshalYAML(unmarshal func(any) error) error {
 
 func (t *Config) validate() error {
 	for _, agent := range t.Agents {
-		for _, toolSet := range agent.Toolsets {
-			if err := toolSet.validate(); err != nil {
+		for i := range agent.Toolsets {
+			if err := agent.Toolsets[i].validate(); err != nil {
 				return err
 			}
 		}
@@ -27,19 +27,42 @@ func (t *Config) validate() error {
 	return nil
 }
 
-// Ensure that either Command, Remote or Ref is set, but not all empty
 func (t *Toolset) validate() error {
 	// Attributes used on the wrong toolset type.
 	if len(t.Shell) > 0 && t.Type != "script" {
 		return errors.New("shell can only be used with type 'script'")
 	}
-	if len(t.Path) > 0 && t.Type != "memory" {
+	if t.Path != "" && t.Type != "memory" {
 		return errors.New("path can only be used with type 'memory'")
 	}
+	if len(t.PostEdit) > 0 && t.Type != "filesystem" {
+		return errors.New("post_edit can only be used with type 'filesystem'")
+	}
+	if len(t.Env) > 0 && (t.Type != "shell" && t.Type != "script" && t.Type != "mcp") {
+		return errors.New("env can only be used with type 'shell', 'script' or 'mcp'")
+	}
+	if t.Shared && t.Type != "todo" {
+		return errors.New("shared can only be used with type 'todo'")
+	}
+	if t.Command != "" && t.Type != "mcp" {
+		return errors.New("command can only be used with type 'mcp'")
+	}
+	if len(t.Args) > 0 && t.Type != "mcp" {
+		return errors.New("args can only be used with type 'mcp'")
+	}
+	if t.Ref != "" && t.Type != "mcp" {
+		return errors.New("ref can only be used with type 'mcp'")
+	}
+	if (t.Remote.URL != "" || t.Remote.TransportType != "" || len(t.Remote.Headers) > 0) && t.Type != "mcp" {
+		return errors.New("remote can only be used with type 'mcp'")
+	}
+	if t.Config != nil && t.Type != "mcp" {
+		return errors.New("config can only be used with type 'mcp'")
+	}
 
 	switch t.Type {
 	case "memory":
-		if len(t.Path) == 0 {
+		if t.Path == "" {
 			return errors.New("memory toolset requires a path to be set")
 		}
 	case "mcp":

pkg/config/v2/validate.go

@@ -120,15 +120,19 @@ func TestValidationErrors(t *testing.T) {
 		},
 		{
 			name: "path in non memory toolset",
-			path: "testdata/invalid_mcp_path_v2.yaml",
+			path: "testdata/invalid_path_v2.yaml",
+		},
+		{
+			name: "post_edit in non filesystem toolset",
+			path: "testdata/invalid_post_edit_v2.yaml",
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			_, err := loadConfig("testdata/missing_memory_path.yaml")
+			_, err := loadConfig(tt.path)
 			require.Error(t, err)
 		})
 	}