[raycicmd] Add validation guards for array expansion

Detect three classes of errors early during array expansion:
- Duplicate base key: two array steps sharing the same key/name silently overwrote configs
- Duplicate element from adjustment: an addition adjustment that duplicates an existing element would produce duplicate pipeline steps
- Key collision from sanitization: values like "1.2.1" and "121" that produce the same sanitized key part

Also adds a test for partial-dimension skip behavior (skip by one dim removes all matching combinations).

Topic: array-validation-guards
Labels: draft
Signed-off-by: andrew <andrew@anyscale.com>

Author: andrew-anyscale

raycicmd/array_expand.go

@@ -154,9 +154,16 @@ func expandSingleArrayStep(
 		)
 	}
 
+	if _, exists := configs[baseKey]; exists {
+		return nil, fmt.Errorf(
+			"duplicate array step key %q", baseKey,
+		)
+	}
+
 	cfg.elements = elements
 	configs[baseKey] = cfg
 
+	seenKeys := make(map[string]struct{}, len(elements))
 	var result []map[string]any
 	for _, elem := range elements {
 		// Deep-copy step, replacing {{array.X}} placeholders with
@@ -166,6 +173,15 @@ func expandSingleArrayStep(
 		//    → "build-step--cuda1211-python311"
 		expandedKey := elem.generateKey(baseKey)
 
+		if _, dup := seenKeys[expandedKey]; dup {
+			return nil, fmt.Errorf(
+				"array step %q: duplicate generated key %q "+
+					"(values may collide after sanitization)",
+				baseKey, expandedKey,
+			)
+		}
+		seenKeys[expandedKey] = struct{}{}
+
 		if _, hasName := expandedStep["name"]; hasName {
 			expandedStep["name"] = expandedKey
 		} else {
@@ -199,6 +215,12 @@ func applyAdjustments(
 				)
 			}
 		}
+		if hasElement(elements, adj.with) {
+			return nil, fmt.Errorf(
+				"addition adjustment with=%v duplicates "+
+					"an existing element", adj.with,
+			)
+		}
 		elements = append(elements, &arrayElement{
 			values: maps.Clone(adj.with),
 		})

raycicmd/array_expand_test.go

@@ -844,6 +844,135 @@ func TestExpandArraySteps_GroupDependsOnNonArrayStep(t *testing.T) {
 	}
 }
 
+func TestExpandArraySteps_DuplicateBaseKey(t *testing.T) {
+	groups := []*pipelineGroup{
+		{
+			Group: "build",
+			Steps: []map[string]any{{
+				"label":    "Build {{array.python}}",
+				"name":     "shared-name",
+				"commands": []any{"echo build"},
+				"array": map[string]any{
+					"python": []any{"3.10"},
+				},
+			}},
+		},
+		{
+			Group: "test",
+			Steps: []map[string]any{{
+				"label":    "Test {{array.python}}",
+				"name":     "shared-name",
+				"commands": []any{"echo test"},
+				"array": map[string]any{
+					"python": []any{"3.11"},
+				},
+			}},
+		},
+	}
+
+	err := expandArraySteps(groups)
+	if err == nil {
+		t.Fatal("expected error for duplicate base key, got nil")
+	}
+	if !strings.Contains(err.Error(), "duplicate array step key") {
+		t.Errorf(
+			"error = %q, want to contain \"duplicate array step key\"",
+			err.Error(),
+		)
+	}
+}
+
+func TestExpandArraySteps_DuplicateElementFromAdjustment(t *testing.T) {
+	groups := []*pipelineGroup{{
+		Group: "build",
+		Steps: []map[string]any{{
+			"label":    "Build {{array.python}}",
+			"key":      "build-step",
+			"commands": []any{"echo build"},
+			"array": map[string]any{
+				"python": []any{"3.10", "3.11"},
+				"adjustments": []any{
+					map[string]any{
+						"with": map[string]any{"python": "3.10"},
+					},
+				},
+			},
+		}},
+	}}
+
+	err := expandArraySteps(groups)
+	if err == nil {
+		t.Fatal("expected error for duplicate element, got nil")
+	}
+	if !strings.Contains(err.Error(), "duplicates") {
+		t.Errorf(
+			"error = %q, want to contain \"duplicates\"",
+			err.Error(),
+		)
+	}
+}
+
+func TestExpandArraySteps_KeyCollisionFromSanitization(t *testing.T) {
+	groups := []*pipelineGroup{{
+		Group: "build",
+		Steps: []map[string]any{{
+			"label":    "Build {{array.ver}}",
+			"key":      "build-step",
+			"commands": []any{"echo build"},
+			"array": map[string]any{
+				"ver": []any{"1.2.1", "121"},
+			},
+		}},
+	}}
+
+	err := expandArraySteps(groups)
+	if err == nil {
+		t.Fatal("expected error for key collision, got nil")
+	}
+	if !strings.Contains(err.Error(), "duplicate generated key") {
+		t.Errorf(
+			"error = %q, want to contain \"duplicate generated key\"",
+			err.Error(),
+		)
+	}
+}
+
+func TestExpandArraySteps_PartialDimensionSkip(t *testing.T) {
+	groups := []*pipelineGroup{{
+		Group: "build",
+		Steps: []map[string]any{{
+			"label":    "Build {{array.os}} {{array.arch}}",
+			"key":      "build-step",
+			"commands": []any{"echo {{array.os}} {{array.arch}}"},
+			"array": map[string]any{
+				"os":   []any{"linux", "windows"},
+				"arch": []any{"amd64", "arm64"},
+				"adjustments": []any{
+					map[string]any{
+						"with": map[string]any{"os": "windows"},
+						"skip": true,
+					},
+				},
+			},
+		}},
+	}}
+
+	if err := expandArraySteps(groups); err != nil {
+		t.Fatalf("expandArraySteps() error = %v", err)
+	}
+
+	rs := groups[0].resolvedSteps
+	if len(rs) != 2 {
+		t.Fatalf("got %d resolvedSteps, want 2", len(rs))
+	}
+	for _, r := range rs {
+		key := r.src["key"].(string)
+		if strings.Contains(key, "oswindows") {
+			t.Errorf("skipped os=windows should not appear: %q", key)
+		}
+	}
+}
+
 func TestParseArrayDependsOnErrors(t *testing.T) {
 	tests := []struct {
 		name    string