Two minor fixes for vMCP retryDelay handling

cmd/thv-operator/api/v1alpha1/virtualmcpcompositetooldefinition_webhook.go

@@ -84,8 +84,8 @@ func (r *VirtualMCPCompositeToolDefinition) Validate() error {
 	// Validate failure mode
 	if r.Spec.FailureMode != "" {
 		validModes := map[string]bool{
-			"abort":    true,
-			"continue": true,
+			ErrorActionAbort:    true,
+			ErrorActionContinue: true,
 		}
 		if !validModes[r.Spec.FailureMode] {
 			errors = append(errors, "spec.failureMode must be one of: abort, continue")
@@ -270,19 +270,19 @@ func (*VirtualMCPCompositeToolDefinition) validateStepDependencies(index int, st
 // validateErrorHandling validates error handling configuration
 func (*VirtualMCPCompositeToolDefinition) validateErrorHandling(stepIndex int, errorHandling *ErrorHandling) error {
 	if errorHandling.Action == "" {
-		return nil // Action is optional, defaults to "abort"
+		return nil // Action is optional, defaults to ErrorActionAbort
 	}
 
 	validActions := map[string]bool{
-		"abort":    true,
-		"continue": true,
-		"retry":    true,
+		ErrorActionAbort:    true,
+		ErrorActionContinue: true,
+		ErrorActionRetry:    true,
 	}
 	if !validActions[errorHandling.Action] {
 		return fmt.Errorf("spec.steps[%d].onError.action must be one of: abort, continue, retry", stepIndex)
 	}
 
-	if errorHandling.Action == "retry" {
+	if errorHandling.Action == ErrorActionRetry {
 		if errorHandling.MaxRetries < 1 {
 			return fmt.Errorf("spec.steps[%d].onError.maxRetries must be at least 1 when action is retry", stepIndex)
 		}

cmd/thv-operator/api/v1alpha1/virtualmcpcompositetooldefinition_webhook_test.go

@@ -363,7 +363,7 @@ func TestVirtualMCPCompositeToolDefinitionValidate(t *testing.T) {
 							ID:   "deploy",
 							Tool: "kubectl.apply",
 							OnError: &ErrorHandling{
-								Action:     "retry",
+								Action:     ErrorActionRetry,
 								MaxRetries: 3,
 							},
 						},
@@ -383,7 +383,7 @@ func TestVirtualMCPCompositeToolDefinitionValidate(t *testing.T) {
 							ID:   "deploy",
 							Tool: "kubectl.apply",
 							OnError: &ErrorHandling{
-								Action:     "retry",
+								Action:     ErrorActionRetry,
 								MaxRetries: 0,
 							},
 						},
@@ -455,7 +455,7 @@ func TestVirtualMCPCompositeToolDefinitionValidate(t *testing.T) {
 				Spec: VirtualMCPCompositeToolDefinitionSpec{
 					Name:        "continue_deploy",
 					Description: "Deploy with continue mode",
-					FailureMode: "continue",
+					FailureMode: ErrorActionContinue,
 					Steps: []WorkflowStep{
 						{
 							ID:   "deploy1",

cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go

@@ -541,6 +541,18 @@ const (
 	WorkflowStepTypeElicitation = "elicitation"
 )
 
+// Error handling actions
+const (
+	// ErrorActionAbort aborts the workflow on error
+	ErrorActionAbort = "abort"
+
+	// ErrorActionContinue continues the workflow on error
+	ErrorActionContinue = "continue"
+
+	// ErrorActionRetry retries the step on error
+	ErrorActionRetry = "retry"
+)
+
 //+kubebuilder:object:root=true
 //+kubebuilder:subresource:status
 //+kubebuilder:resource:shortName=vmcp;virtualmcp

cmd/thv-operator/api/v1alpha1/virtualmcpserver_webhook.go

@@ -332,20 +332,27 @@ func validateStepErrorHandling(toolIndex, stepIndex int, step WorkflowStep) erro
 	}
 
 	validActions := map[string]bool{
-		"abort":    true,
-		"continue": true,
-		"retry":    true,
+		ErrorActionAbort:    true,
+		ErrorActionContinue: true,
+		ErrorActionRetry:    true,
 	}
 	if !validActions[step.OnError.Action] {
 		return fmt.Errorf("spec.compositeTools[%d].steps[%d].onError.action must be abort, continue, or retry",
 			toolIndex, stepIndex)
 	}
 
-	if step.OnError.Action == "retry" && step.OnError.MaxRetries < 1 {
+	if step.OnError.Action == ErrorActionRetry && step.OnError.MaxRetries < 1 {
 		return fmt.Errorf("spec.compositeTools[%d].steps[%d].onError.maxRetries must be at least 1 when action is retry",
 			toolIndex, stepIndex)
 	}
 
+	if step.OnError.Action == ErrorActionRetry && step.OnError.RetryDelay != "" {
+		if err := validateDuration(step.OnError.RetryDelay); err != nil {
+			return fmt.Errorf("spec.compositeTools[%d].steps[%d].onError.retryDelay: %w",
+				toolIndex, stepIndex, err)
+		}
+	}
+
 	return nil
 }
 

cmd/thv-operator/api/v1alpha1/virtualmcpserver_webhook_test.go

@@ -508,7 +508,7 @@ func TestValidateCompositeToolsWithDependencies(t *testing.T) {
 									ID:   "step1",
 									Tool: "backend.tool1",
 									OnError: &ErrorHandling{
-										Action:     "retry",
+										Action:     ErrorActionRetry,
 										MaxRetries: 3,
 									},
 								},
@@ -533,7 +533,7 @@ func TestValidateCompositeToolsWithDependencies(t *testing.T) {
 									ID:   "step1",
 									Tool: "backend.tool1",
 									OnError: &ErrorHandling{
-										Action:     "retry",
+										Action:     ErrorActionRetry,
 										MaxRetries: 0,
 									},
 								},
@@ -545,6 +545,85 @@ func TestValidateCompositeToolsWithDependencies(t *testing.T) {
 			wantErr: true,
 			errMsg:  "spec.compositeTools[0].steps[0].onError.maxRetries must be at least 1 when action is retry",
 		},
+		{
+			name: "valid error handling with retryDelay",
+			vmcp: &VirtualMCPServer{
+				Spec: VirtualMCPServerSpec{
+					GroupRef: GroupRef{Name: "test-group"},
+					CompositeTools: []CompositeToolSpec{
+						{
+							Name:        "test-tool",
+							Description: "Test composite tool",
+							Steps: []WorkflowStep{
+								{
+									ID:   "step1",
+									Tool: "backend.tool1",
+									OnError: &ErrorHandling{
+										Action:     ErrorActionRetry,
+										MaxRetries: 3,
+										RetryDelay: "5s",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid error handling with complex retryDelay",
+			vmcp: &VirtualMCPServer{
+				Spec: VirtualMCPServerSpec{
+					GroupRef: GroupRef{Name: "test-group"},
+					CompositeTools: []CompositeToolSpec{
+						{
+							Name:        "test-tool",
+							Description: "Test composite tool",
+							Steps: []WorkflowStep{
+								{
+									ID:   "step1",
+									Tool: "backend.tool1",
+									OnError: &ErrorHandling{
+										Action:     ErrorActionRetry,
+										MaxRetries: 3,
+										RetryDelay: "1m30s",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "invalid error handling - invalid retryDelay format",
+			vmcp: &VirtualMCPServer{
+				Spec: VirtualMCPServerSpec{
+					GroupRef: GroupRef{Name: "test-group"},
+					CompositeTools: []CompositeToolSpec{
+						{
+							Name:        "test-tool",
+							Description: "Test composite tool",
+							Steps: []WorkflowStep{
+								{
+									ID:   "step1",
+									Tool: "backend.tool1",
+									OnError: &ErrorHandling{
+										Action:     ErrorActionRetry,
+										MaxRetries: 3,
+										RetryDelay: "invalid",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: true,
+			errMsg:  "spec.compositeTools[0].steps[0].onError.retryDelay: invalid duration format \"invalid\", expected format like '30s', '5m', '1h', '1h30m'",
+		},
 	}
 
 	for _, tt := range tests {

cmd/thv-operator/pkg/vmcpconfig/converter.go

@@ -309,7 +309,12 @@ func (*Converter) convertCompositeTools(
 					RetryCount: crdStep.OnError.MaxRetries,
 				}
 				if crdStep.OnError.RetryDelay != "" {
-					if duration, err := time.ParseDuration(crdStep.OnError.RetryDelay); err == nil {
+					if duration, err := time.ParseDuration(crdStep.OnError.RetryDelay); err != nil {
+						// Log warning but continue - validation should have caught this at admission time
+						ctxLogger := log.FromContext(ctx)
+						ctxLogger.Error(err, "failed to parse retry delay",
+							"step", crdStep.ID, "retryDelay", crdStep.OnError.RetryDelay)
+					} else {
 						stepError.RetryDelay = vmcpconfig.Duration(duration)
 					}
 				}

cmd/thv-operator/pkg/vmcpconfig/converter_test.go

@@ -253,72 +253,72 @@ func TestConverter_ConvertCompositeTools_ErrorHandling(t *testing.T) {
 		{
 			name: "with retry delay",
 			errorHandling: &mcpv1alpha1.ErrorHandling{
-				Action:     "retry",
+				Action:     mcpv1alpha1.ErrorActionRetry,
 				MaxRetries: 3,
 				RetryDelay: "5s",
 			},
-			expectedAction: "retry",
+			expectedAction: mcpv1alpha1.ErrorActionRetry,
 			expectedRetry:  3,
 			expectedDelay:  vmcpconfig.Duration(5 * time.Second),
 		},
 		{
 			name: "with millisecond retry delay",
 			errorHandling: &mcpv1alpha1.ErrorHandling{
-				Action:     "retry",
+				Action:     mcpv1alpha1.ErrorActionRetry,
 				MaxRetries: 5,
 				RetryDelay: "500ms",
 			},
-			expectedAction: "retry",
+			expectedAction: mcpv1alpha1.ErrorActionRetry,
 			expectedRetry:  5,
 			expectedDelay:  vmcpconfig.Duration(500 * time.Millisecond),
 		},
 		{
 			name: "with minute retry delay",
 			errorHandling: &mcpv1alpha1.ErrorHandling{
-				Action:     "retry",
+				Action:     mcpv1alpha1.ErrorActionRetry,
 				MaxRetries: 2,
 				RetryDelay: "1m",
 			},
-			expectedAction: "retry",
+			expectedAction: mcpv1alpha1.ErrorActionRetry,
 			expectedRetry:  2,
 			expectedDelay:  vmcpconfig.Duration(1 * time.Minute),
 		},
 		{
 			name: "without retry delay",
 			errorHandling: &mcpv1alpha1.ErrorHandling{
-				Action:     "retry",
+				Action:     mcpv1alpha1.ErrorActionRetry,
 				MaxRetries: 3,
 			},
-			expectedAction: "retry",
+			expectedAction: mcpv1alpha1.ErrorActionRetry,
 			expectedRetry:  3,
 			expectedDelay:  vmcpconfig.Duration(0),
 		},
 		{
 			name: "abort action",
 			errorHandling: &mcpv1alpha1.ErrorHandling{
-				Action: "abort",
+				Action: mcpv1alpha1.ErrorActionAbort,
 			},
-			expectedAction: "abort",
+			expectedAction: mcpv1alpha1.ErrorActionAbort,
 			expectedRetry:  0,
 			expectedDelay:  vmcpconfig.Duration(0),
 		},
 		{
 			name: "continue action",
 			errorHandling: &mcpv1alpha1.ErrorHandling{
-				Action: "continue",
+				Action: mcpv1alpha1.ErrorActionContinue,
 			},
-			expectedAction: "continue",
+			expectedAction: mcpv1alpha1.ErrorActionContinue,
 			expectedRetry:  0,
 			expectedDelay:  vmcpconfig.Duration(0),
 		},
 		{
 			name: "invalid retry delay format is ignored",
 			errorHandling: &mcpv1alpha1.ErrorHandling{
-				Action:     "retry",
+				Action:     mcpv1alpha1.ErrorActionRetry,
 				MaxRetries: 3,
 				RetryDelay: "invalid",
 			},
-			expectedAction: "retry",
+			expectedAction: mcpv1alpha1.ErrorActionRetry,
 			expectedRetry:  3,
 			expectedDelay:  vmcpconfig.Duration(0),
 		},