PR feedback

Author: peteski22

cmd/config/plugins/add.go

@@ -85,7 +85,10 @@ in the category, the command fails with an error. To update an existing plugin,
 		&c.flows,
 		flagFlow,
 		nil,
-		"Flow to execute plugin in: request, response (can be repeated)",
+		fmt.Sprintf(
+			"Flow during which, the plugin should execute (%s) (can be repeated)",
+			strings.Join(config.OrderedFlowNames(), ", "),
+		),
 	)
 	_ = cobraCmd.MarkFlagRequired(flagFlow)
 
@@ -128,16 +131,17 @@ func (c *AddCmd) run(cmd *cobra.Command, args []string) error {
 		)
 	}
 
-	flowsMap := config.ParseFlows(c.flows)
-	if len(flowsMap) == 0 {
-		return fmt.Errorf("at least one valid flow is required (must be 'request' or 'response')")
+	flows := config.ParseFlowsDistinct(c.flows)
+	if len(flows) == 0 {
+		return fmt.Errorf(
+			"at least one valid flow is required (%s)",
+			strings.Join(config.OrderedFlowNames(), ", "),
+		)
 	}
 
-	parsedFlows := slices.Sorted(maps.Keys(flowsMap))
-
 	entry := config.PluginEntry{
 		Name:  pluginName,
-		Flows: parsedFlows,
+		Flows: slices.Sorted(maps.Keys(flows)),
 	}
 
 	// Set optional fields only if they were provided.

internal/config/plugin_config.go

@@ -99,10 +99,10 @@ func (f Flow) IsValid() bool {
 	return ok
 }
 
-// ParseFlows validates and reduces flow strings to a distinct set.
+// ParseFlowsDistinct validates and reduces flow strings to a distinct set.
 // Flow strings are normalized before validation.
 // Invalid flows are silently ignored. Returns an empty map if no valid flows are found.
-func ParseFlows(flags []string) map[Flow]struct{} {
+func ParseFlowsDistinct(flags []string) map[Flow]struct{} {
 	valid := make(map[Flow]struct{}, len(flows))
 
 	for _, s := range flags {
@@ -244,12 +244,10 @@ func (e *PluginEntry) Validate() error {
 	} else {
 		seen := make(map[Flow]struct{})
 		for _, flow := range e.Flows {
-			// Check for valid flow values.
 			if !flow.IsValid() {
-				validationErrors = append(
-					validationErrors,
-					fmt.Errorf("invalid flow '%s', must be '%s' or '%s'", flow, FlowRequest, FlowResponse),
-				)
+				allowedFlows := strings.Join(OrderedFlowNames(), ", ")
+				err := fmt.Errorf("invalid flow '%s' (allowed: %s)", flow, allowedFlows)
+				validationErrors = append(validationErrors, err)
 			}
 
 			// Check for duplicates.
@@ -526,6 +524,18 @@ func OrderedCategories() Categories {
 	return slices.Clone(orderedCategories)
 }
 
+// OrderedFlowNames returns the names of allowed flows in order.
+func OrderedFlowNames() []string {
+	flows := slices.Sorted(maps.Keys(flows))
+
+	flowNames := make([]string, len(flows))
+	for i, f := range flows {
+		flowNames[i] = string(f)
+	}
+
+	return flowNames
+}
+
 // Set is used by Cobra to set the category value from a string.
 // NOTE: This is also required by Cobra as part of implementing flag.Value.
 func (c *Category) Set(v string) error {

internal/config/plugin_config_test.go

@@ -8,6 +8,16 @@ import (
 	"github.com/mozilla-ai/mcpd/v2/internal/context"
 )
 
+func testPluginStringPtr(t *testing.T, s string) *string {
+	t.Helper()
+	return &s
+}
+
+func testPluginBoolPtr(t *testing.T, b bool) *bool {
+	t.Helper()
+	return &b
+}
+
 func TestPluginEntry_Validate(t *testing.T) {
 	t.Parallel()
 
@@ -790,7 +800,7 @@ func TestFlow_IsValid(t *testing.T) {
 	}
 }
 
-func TestParseFlows(t *testing.T) {
+func TestParseFlowsDistinct(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
@@ -866,18 +876,15 @@ func TestParseFlows(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			result := ParseFlows(tc.input)
+			result := ParseFlowsDistinct(tc.input)
 			require.Equal(t, tc.expected, result)
 		})
 	}
 }
 
-func testPluginStringPtr(t *testing.T, s string) *string {
-	t.Helper()
-	return &s
-}
-
-func testPluginBoolPtr(t *testing.T, b bool) *bool {
-	t.Helper()
-	return &b
+func TestAddCmd_OrderedFlowNames(t *testing.T) {
+	flows := OrderedFlowNames()
+	require.Len(t, flows, 2)
+	require.Equal(t, "request", flows[0])
+	require.Equal(t, "response", flows[1])
 }