Improve composite tool configuration ergonomics

pkg/vmcp/config/validator.go

@@ -396,8 +396,9 @@ func (v *DefaultValidator) validateCompositeTools(tools []*CompositeToolConfig)
 			return fmt.Errorf("composite_tools[%d].description is required", i)
 		}
 
-		if tool.Timeout <= 0 {
-			return fmt.Errorf("composite_tools[%d].timeout must be positive", i)
+		// Timeout can be 0 (uses default) or positive (explicit timeout)
+		if tool.Timeout < 0 {
+			return fmt.Errorf("composite_tools[%d].timeout cannot be negative", i)
 		}
 
 		// Validate steps
@@ -451,8 +452,20 @@ func (*DefaultValidator) validateStepBasics(step *WorkflowStepConfig, index int,
 	return nil
 }
 
-// validateStepType validates step type and type-specific requirements
+// validateStepType validates step type and type-specific requirements.
+// The type should have been inferred during loading if the 'tool' field is present.
+// Elicitation steps must always specify type explicitly for clarity.
 func (*DefaultValidator) validateStepType(step *WorkflowStepConfig, index int) error {
+	// Check for ambiguous configuration: both tool and message fields present
+	if step.Tool != "" && step.Message != "" {
+		return fmt.Errorf("step[%d] cannot have both tool and message fields - use explicit type to clarify intent", index)
+	}
+
+	// Type is required at this point (should have been inferred during loading)
+	if step.Type == "" {
+		return fmt.Errorf("step[%d].type is required (or omit for tool steps with 'tool' field present)", index)
+	}
+
 	validTypes := []string{"tool", "elicitation"}
 	if !contains(validTypes, step.Type) {
 		return fmt.Errorf("step[%d].type must be one of: %s", index, strings.Join(validTypes, ", "))

pkg/vmcp/config/validator_test.go

@@ -553,6 +553,104 @@ func TestValidator_ValidateCompositeTools(t *testing.T) {
 			wantErr: true,
 			errMsg:  "duplicate composite tool name",
 		},
+		{
+			name: "type inferred from tool field",
+			tools: []*CompositeToolConfig{
+				{
+					Name:        "fetch_data",
+					Description: "Fetch data workflow",
+					Timeout:     Duration(5 * time.Minute),
+					Steps: []*WorkflowStepConfig{
+						{
+							ID:   "fetch",
+							Type: "tool", // Type would be inferred by loader from tool field
+							Tool: "fetch_fetch",
+							Arguments: map[string]any{
+								"url": "https://example.com",
+							},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "timeout omitted uses default",
+			tools: []*CompositeToolConfig{
+				{
+					Name:        "no_timeout",
+					Description: "Workflow without explicit timeout",
+					Timeout:     0, // Omitted - should use default (30 minutes)
+					Steps: []*WorkflowStepConfig{
+						{
+							ID:   "step1",
+							Type: "tool", // Type would be inferred by loader from tool field
+							Tool: "some_tool",
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "elicitation requires explicit type",
+			tools: []*CompositeToolConfig{
+				{
+					Name:        "confirm_action",
+					Description: "Confirmation workflow",
+					Timeout:     Duration(5 * time.Minute),
+					Steps: []*WorkflowStepConfig{
+						{
+							ID:      "confirm",
+							Message: "Proceed?", // Elicitation field present
+							Schema:  map[string]any{"type": "object"},
+							// Type is missing - should fail
+						},
+					},
+				},
+			},
+			wantErr: true,
+			errMsg:  "type is required",
+		},
+		{
+			name: "missing both type and identifying fields",
+			tools: []*CompositeToolConfig{
+				{
+					Name:        "invalid_step",
+					Description: "Invalid step workflow",
+					Timeout:     Duration(5 * time.Minute),
+					Steps: []*WorkflowStepConfig{
+						{
+							ID: "step1",
+							// No type, no tool, no message - cannot infer
+						},
+					},
+				},
+			},
+			wantErr: true,
+			errMsg:  "type is required",
+		},
+		{
+			name: "both tool and message fields present",
+			tools: []*CompositeToolConfig{
+				{
+					Name:        "ambiguous_step",
+					Description: "Step with both tool and message",
+					Timeout:     Duration(5 * time.Minute),
+					Steps: []*WorkflowStepConfig{
+						{
+							ID:      "step1",
+							Tool:    "some_tool",    // Tool field present
+							Message: "Some message", // Message field also present
+							// Type will be inferred as "tool" during loading
+							// This should fail validation due to ambiguity
+						},
+					},
+				},
+			},
+			wantErr: true,
+			errMsg:  "cannot have both tool and message fields",
+		},
 	}
 
 	for _, tt := range tests {

pkg/vmcp/config/yaml_loader.go

@@ -495,9 +495,14 @@ func (l *YAMLLoader) transformCompositeTools(raw []*rawCompositeTool) ([]*Compos
 	var tools []*CompositeToolConfig
 
 	for _, rawTool := range raw {
-		timeout, err := time.ParseDuration(rawTool.Timeout)
-		if err != nil {
-			return nil, fmt.Errorf("tool %s: invalid timeout: %w", rawTool.Name, err)
+		// Parse timeout - empty string means use default (0 duration)
+		var timeout time.Duration
+		if rawTool.Timeout != "" {
+			var err error
+			timeout, err = time.ParseDuration(rawTool.Timeout)
+			if err != nil {
+				return nil, fmt.Errorf("tool %s: invalid timeout: %w", rawTool.Name, err)
+			}
 		}
 
 		tool := &CompositeToolConfig{
@@ -553,6 +558,11 @@ func (*YAMLLoader) transformWorkflowStep(raw *rawWorkflowStep) (*WorkflowStepCon
 		Schema:    raw.Schema,
 	}
 
+	// Apply type inference: if type is empty and tool field is present, infer as "tool"
+	if step.Type == "" && step.Tool != "" {
+		step.Type = "tool"
+	}
+
 	if raw.Timeout != "" {
 		timeout, err := time.ParseDuration(raw.Timeout)
 		if err != nil {