tests: address CodeRabbit review comments from downstream PR

- Rename local variable `tag` to `bundleTag` in catalog.Build to avoid
  shadowing the method's `tag` parameter
- Fail the step with an error on invalid LargeCRD field counts in the
  bundle option parser; use strconv.ParseInt with bitSize 0 to also
  reject non-positive values
- Look up channels by name in TestCatalog_FBCGeneration instead of
  assuming insertion order
- Use errgroup.Group with SetLimit(8) in ScenarioCleanup deletion loop
  to bound concurrent goroutines and kubectl calls
- Clarify README that ClusterObjectSet deletion is conditional on the
  BoxcutterRuntime feature gate
- Add missing `text` language specifier to fenced code block in
  e2e-isolation design doc
- Remove extra blank line in setup.sh

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: tmshort

docs/designs/testing/2026-04-13-e2e-isolation/design.md

@@ -12,7 +12,7 @@ Each scenario dynamically builds and pushes its own bundle and catalog OCI image
 parameterized by scenario ID. All cluster-scoped resource names include the scenario ID, making
 conflicts structurally impossible.
 
-```
+```text
 Scenario starts
   -> Generate parameterized bundle manifests (CRD names, deployments, etc. include scenario ID)
   -> Build + push bundle OCI images to e2e registry via go-containerregistry

test/e2e/README.md

@@ -292,7 +292,7 @@ Each scenario runs in its own namespace with unique resource names, ensuring com
 
 The `ScenarioCleanup` hook ensures all resources are deleted after each scenario:
 
-- Deletes ClusterExtensions and ClusterObjectSets
+- Deletes ClusterExtensions and (when BoxcutterRuntime gate is enabled) ClusterObjectSets
 - Deletes ClusterCatalogs
 - Deletes namespaces
 - Deletes added resources

test/e2e/steps/hooks.go

@@ -8,11 +8,11 @@ import (
 	"os/exec"
 	"regexp"
 	"strconv"
-	"sync"
 
 	"github.com/cucumber/godog"
 	"github.com/go-logr/logr"
 	"github.com/spf13/pflag"
+	"golang.org/x/sync/errgroup"
 	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/component-base/featuregate"
@@ -222,20 +222,20 @@ func ScenarioCleanup(ctx context.Context, _ *godog.Scenario, err error) (context
 	}
 	forDeletion = append(forDeletion, resource{name: sc.namespace, kind: "namespace"})
 
-	var wg sync.WaitGroup
+	g := new(errgroup.Group)
+	g.SetLimit(8)
 	for _, r := range forDeletion {
-		wg.Add(1)
-		go func(res resource) {
-			defer wg.Done()
-			args := []string{"delete", res.kind, res.name, "--ignore-not-found=true"}
-			if res.namespace != "" {
-				args = append(args, "-n", res.namespace)
+		g.Go(func() error {
+			args := []string{"delete", r.kind, r.name, "--ignore-not-found=true"}
+			if r.namespace != "" {
+				args = append(args, "-n", r.namespace)
 			}
 			if _, err := k8sClient(args...); err != nil {
-				logger.Info("Error deleting resource", "name", res.name, "namespace", res.namespace, "stderr", stderrOutput(err))
+				logger.Info("Error deleting resource", "name", r.name, "namespace", r.namespace, "stderr", stderrOutput(err))
 			}
-		}(r)
+			return nil
+		})
 	}
-	wg.Wait()
+	_ = g.Wait()
 	return ctx, nil
 }

test/e2e/steps/steps.go

@@ -1564,7 +1564,10 @@ func parseCatalogTable(table *godog.Table) ([]catalog.PackageOption, error) {
 				return nil, fmt.Errorf("duplicate bundle %s/%s: contents must be empty for repeated versions", pkg, version)
 			}
 		} else {
-			opts := parseContents(contents)
+			opts, err := parseContents(contents)
+			if err != nil {
+				return nil, fmt.Errorf("bundle %s/%s: %w", pkg, version, err)
+			}
 			bundleDefs[bk] = &bundleEntry{opts: opts}
 			bundleOrder = append(bundleOrder, bk)
 		}
@@ -1651,13 +1654,13 @@ spec:
 	return nil
 }
 
-func parseContents(contents string) []catalog.BundleOption {
+func parseContents(contents string) ([]catalog.BundleOption, error) {
 	contents = strings.TrimSpace(contents)
 	if contents == "" {
-		return nil
+		return nil, nil
 	}
 	if strings.EqualFold(contents, "BadImage") {
-		return []catalog.BundleOption{catalog.BadImage()}
+		return []catalog.BundleOption{catalog.BadImage()}, nil
 	}
 	var opts []catalog.BundleOption
 	for _, part := range strings.Split(contents, ",") {
@@ -1682,10 +1685,11 @@ func parseContents(contents string) []catalog.BundleOption {
 		case strings.HasPrefix(part, "LargeCRD(") && strings.HasSuffix(part, ")"):
 			// LargeCRD(250)
 			countStr := part[len("LargeCRD(") : len(part)-1]
-			count, err := strconv.Atoi(countStr)
-			if err == nil {
-				opts = append(opts, catalog.WithLargeCRD(count))
+			count, err := strconv.ParseInt(countStr, 10, 0)
+			if err != nil || count <= 0 {
+				return nil, fmt.Errorf("invalid LargeCRD field count %q: must be a positive integer", countStr)
 			}
+			opts = append(opts, catalog.WithLargeCRD(int(count)))
 		case strings.HasPrefix(part, "ClusterRegistry(") && strings.HasSuffix(part, ")"):
 			// ClusterRegistry(mirrored-registry.operator-controller-e2e.svc.cluster.local:5000)
 			host := part[len("ClusterRegistry(") : len(part)-1]
@@ -1697,7 +1701,7 @@ func parseContents(contents string) []catalog.BundleOption {
 			opts = append(opts, catalog.StaticBundleDir(absDir))
 		}
 	}
-	return opts
+	return opts, nil
 }
 
 // PrometheusMetricsAreReturned validates that each pod's stored metrics response is non-empty and parses as valid Prometheus text format.

test/extension-developer-e2e/setup.sh

@@ -92,7 +92,6 @@ reg_bundle_img="${cluster_registry_host}/bundles/registry-v1/registry-bundle:v0.
 # because docker push goes through the Docker daemon which
 # may be in a different network context (e.g. colima VM).
 
-
 ###############################
 # Create the FBC that contains
 # the registry+v1 extensions

test/internal/catalog/catalog.go

@@ -154,9 +154,9 @@ func (c *Catalog) Build(ctx context.Context, tag, localRegistry, clusterRegistry
 				return nil, fmt.Errorf("failed to set bundle labels for %s:%s: %w", paramPkgName, bd.version, err)
 			}
 
-			tag := fmt.Sprintf("%s/bundles/%s:v%s", localRegistry, paramPkgName, bd.version)
-			if err := crane.Push(img, tag, crane.Insecure, crane.WithContext(ctx)); err != nil {
-				return nil, fmt.Errorf("failed to push bundle image %s: %w", tag, err)
+			bundleTag := fmt.Sprintf("%s/bundles/%s:v%s", localRegistry, paramPkgName, bd.version)
+			if err := crane.Push(img, bundleTag, crane.Insecure, crane.WithContext(ctx)); err != nil {
+				return nil, fmt.Errorf("failed to push bundle image %s: %w", bundleTag, err)
 			}
 			bundleClusterRegistry := clusterRegistry
 			if spec.clusterRegistryOverride != "" {

test/internal/catalog/catalog_test.go

@@ -134,17 +134,29 @@ func TestCatalog_FBCGeneration(t *testing.T) {
 		t.Errorf("expected 2 channels, got %d", len(pkg.channels))
 	}
 
-	// Verify alpha channel has 1 entry
-	alpha := pkg.channels[0]
-	if alpha.name != "alpha" {
-		t.Errorf("expected channel 'alpha', got %q", alpha.name)
+	// Verify channels by name rather than by index so the test is robust to ordering changes.
+	var alpha, beta *channelDef
+	for i := range pkg.channels {
+		switch pkg.channels[i].name {
+		case "alpha":
+			alpha = &pkg.channels[i]
+		case "beta":
+			beta = &pkg.channels[i]
+		}
+	}
+	if alpha == nil {
+		t.Fatal("alpha channel not found")
+	}
+	if beta == nil {
+		t.Fatal("beta channel not found")
 	}
+
+	// Verify alpha channel has 1 entry
 	if len(alpha.entries) != 1 {
 		t.Errorf("expected 1 alpha entry, got %d", len(alpha.entries))
 	}
 
 	// Verify beta channel has replaces edge
-	beta := pkg.channels[1]
 	if len(beta.entries) != 2 {
 		t.Fatalf("expected 2 beta entries, got %d", len(beta.entries))
 	}