From 6020ab53e0d5c9692855f2cd68197d1f79c66313 Mon Sep 17 00:00:00 2001
From: Jon Jackson <jonjacks@redhat.com>
Date: Fri, 17 Apr 2026 10:07:02 -0400
Subject: [PATCH 1/3] CONSOLE-5195: Add AI tooling configuration with Claude
 Code skills

Configure AI-assisted development and review tooling for console-operator:

- Add 6 Claude Code skills (1,142 lines) for operator-specific reviews:
  - controller-review: Validate controller factory patterns and ManagementState
  - sync-handler-review: Check incremental reconciliation logic
  - manifest-review: Review RBAC and cluster profile annotations
  - e2e-test-review: Validate test patterns, cleanup, wait logic
  - e2e-test-create: Scaffold new e2e tests with best practices
  - go-quality-review: Detect deprecated APIs and code smells

- Update CodeRabbit config to use pattern-based skill triggering:
  - Triggers based on code patterns (function signatures, types) not just file paths
  - More accurate detection of controller code, sync handlers, e2e tests
  - References all skills in knowledge base

- Add comprehensive documentation:
  - .claude/README.md: Skills overview and usage
  - .github/CODERABBIT_SETUP.md: Integration setup guide

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
---
 .claude/README.md                     | 156 +++++++++++++
 .claude/skills/controller-review.md   |  93 ++++++++
 .claude/skills/e2e-test-create.md     | 280 +++++++++++++++++++++++
 .claude/skills/e2e-test-review.md     | 212 +++++++++++++++++
 .claude/skills/go-quality-review.md   | 318 ++++++++++++++++++++++++++
 .claude/skills/manifest-review.md     |  88 +++++++
 .claude/skills/sync-handler-review.md | 151 ++++++++++++
 .coderabbit.yaml                      | 103 +++++++--
 .github/CODERABBIT_SETUP.md           | 118 ++++++++++
 9 files changed, 1505 insertions(+), 14 deletions(-)
 create mode 100644 .claude/README.md
 create mode 100644 .claude/skills/controller-review.md
 create mode 100644 .claude/skills/e2e-test-create.md
 create mode 100644 .claude/skills/e2e-test-review.md
 create mode 100644 .claude/skills/go-quality-review.md
 create mode 100644 .claude/skills/manifest-review.md
 create mode 100644 .claude/skills/sync-handler-review.md
 create mode 100644 .github/CODERABBIT_SETUP.md

diff --git a/.claude/README.md b/.claude/README.md
new file mode 100644
index 000000000..10447ad0d
--- /dev/null
+++ b/.claude/README.md
@@ -0,0 +1,156 @@
+# Claude Code Configuration for Console Operator
+
+This directory contains Claude Code configuration and custom skills for the OpenShift console-operator.
+
+## Directory Structure
+
+```
+.claude/
+├── README.md           # This file
+└── skills/             # Custom operator-specific skills
+    ├── controller-review.md      # Controller pattern review
+    ├── sync-handler-review.md    # Sync handler review
+    ├── manifest-review.md        # RBAC/manifest review
+    ├── e2e-test-review.md        # E2E test review
+    ├── e2e-test-create.md        # E2E test scaffolding
+    └── go-quality-review.md      # Code quality review
+```
+
+## Skills Overview
+
+### Operator Development Skills
+
+**controller-review** - Review controller implementations
+- Checks library-go factory pattern usage
+- Validates ManagementState handling (Managed/Unmanaged/Removed)
+- Verifies status condition handling
+- Ensures proper import organization
+- Validates error handling and resource application
+
+**sync-handler-review** - Review sync/reconciliation handlers
+- Validates incremental sync pattern
+- Checks dependency ordering
+- Verifies status updates
+- Ensures early returns on errors
+- Validates resource creation vs update logic
+- Checks feature gate handling
+
+**manifest-review** - Review Kubernetes manifests
+- Validates cluster profile annotations
+- Checks RBAC least privilege
+- Verifies namespace consistency
+- Ensures proper YAML formatting
+- Validates service account references
+
+### Testing Skills
+
+**e2e-test-review** - Review end-to-end tests
+- Validates test structure and cleanup
+- Checks for proper wait patterns (wait.Poll, not time.Sleep)
+- Verifies retry patterns for config updates
+- Ensures context usage with timeouts
+- Validates framework usage
+- Checks assertion quality
+
+**e2e-test-create** - Scaffold new e2e tests
+- Provides complete test templates
+- Includes proper framework setup
+- Shows cleanup patterns
+- Demonstrates wait and retry patterns
+- Includes table-driven test examples
+
+### Code Quality Skills
+
+**go-quality-review** - General Go code quality
+- Detects deprecated APIs (ioutil.ReadFile, etc.)
+- Validates error handling and wrapping
+- Checks context propagation
+- Identifies code smells (god functions, magic values, deep nesting)
+- Reviews performance issues
+- Checks concurrency safety
+
+## Using Skills
+
+### In Claude Code
+
+```bash
+# Review a controller
+/controller-review
+
+# Review sync handler
+/sync-handler-review
+
+# Review e2e tests
+/e2e-test-review
+
+# Create new e2e test
+/e2e-test-create
+
+# Review manifests
+/manifest-review
+
+# General code quality review
+/go-quality-review
+```
+
+### In CodeRabbit PR Reviews
+
+CodeRabbit automatically references these skills based on code patterns (configured in `.coderabbit.yaml`):
+- Controller factory patterns (`factory.New()`, `ToController()`) → controller-review
+- Sequential sync logic with early returns → sync-handler-review
+- E2E framework usage (`framework.MustNewClientset`, `wait.Poll`) → e2e-test-review
+- YAML with RBAC kinds or missing annotations → manifest-review
+- All Go code → go-quality-review (deprecated APIs, code smells)
+
+## Project Context
+
+These skills reference and enforce patterns documented in:
+- `ARCHITECTURE.md` - System architecture and components
+- `CONVENTIONS.md` - Go coding standards and patterns
+- `TESTING.md` - Testing patterns and commands
+- `AGENTS.md` - AI context hub
+
+## Extending Skills
+
+To add a new skill:
+
+1. Create a new `.md` file in `.claude/skills/`
+2. Use this frontmatter:
+   ```yaml
+   ---
+   name: skill-name
+   description: What this skill does
+   tags: [relevant, tags]
+   ---
+   ```
+3. Document what to check, patterns to follow, and output format
+4. Update `.coderabbit.yaml` to reference the skill in path_instructions
+5. Update this README with the new skill
+
+## Integration with CodeRabbit
+
+The `.coderabbit.yaml` configuration:
+- References all skills in `knowledge_base.code_guidelines.filePatterns`
+- Maps file paths to relevant skills in `reviews.path_instructions`
+- Provides operator-specific review guidelines
+- Links to openshift/console repository for cross-repo context
+
+See `.github/CODERABBIT_SETUP.md` for complete integration documentation.
+
+## Common Review Patterns
+
+Based on recent PRs, watch for:
+- OwnerReference management (ensure only one controller=true)
+- Service account lifecycle and RBAC changes
+- API version bumps and deprecated field usage
+- Feature flag addition/removal
+- Manifest cluster profile annotations
+- Cleanup logic for Removed state
+- Status condition updates
+
+## Questions?
+
+See project documentation:
+- Main docs: `ARCHITECTURE.md`, `CONVENTIONS.md`, `TESTING.md`
+- Integration: `.github/CODERABBIT_SETUP.md`
+- Skills: Individual `.md` files in this directory
diff --git a/.claude/skills/controller-review.md b/.claude/skills/controller-review.md
new file mode 100644
index 000000000..1c2da6692
--- /dev/null
+++ b/.claude/skills/controller-review.md
@@ -0,0 +1,93 @@
+---
+name: controller-review
+description: Review controller code for OpenShift operator patterns and library-go conventions
+tags: [review, controller, operator]
+---
+
+# Controller Review Skill
+
+Review controller implementation code following OpenShift console-operator patterns.
+
+## What to Check
+
+### 1. Controller Factory Pattern
+- Uses `factory.New().WithFilteredEventsInformers()` pattern
+- Proper informer filtering with `util.IncludeNamesFilter()`
+- ToController() call with descriptive name and recorder
+
+### 2. ManagementState Handling
+All controllers should handle three states:
+```go
+switch operatorConfig.Spec.ManagementState {
+case operatorv1.Managed:
+    // sync logic
+case operatorv1.Unmanaged:
+    // skip sync, return nil
+case operatorv1.Removed:
+    // cleanup/removal logic
+default:
+    return fmt.Errorf("unknown state: %v", ...)
+}
+```
+
+### 3. Status Condition Handling
+- Use `status.NewStatusHandler(c.operatorClient)`
+- Set conditions with appropriate types:
+  - `status.HandleProgressingOrDegraded()` for transient errors
+  - `status.HandleDegraded()` for degraded conditions
+  - `status.HandleProgressing()` for in-progress state
+  - `status.HandleAvailable()` for availability
+- Always `FlushAndReturn()` at end of sync
+
+### 4. Import Organization
+Imports must be grouped with comments:
+```go
+import (
+    // standard lib
+    "context"
+    "fmt"
+
+    // 3rd party
+    "github.com/spf13/cobra"
+
+    // kube
+    "k8s.io/client-go/kubernetes"
+
+    // openshift
+    "github.com/openshift/api/config/v1"
+
+    // operator (internal)
+    "github.com/openshift/console-operator/pkg/api"
+)
+```
+
+### 5. Error Handling
+- Wrap errors with context: `fmt.Errorf("failed to X: %w", err)`
+- Check for `apierrors.IsNotFound()` when appropriate
+- Return meaningful error messages
+
+### 6. Resource Application
+- Use `resourceapply.Apply*()` functions from library-go
+- Pass controller recorder for events
+- Handle returned error properly
+
+### 7. OwnerReference Management
+- Set owner references using `subresourceutil.OwnerRefFrom(cr)`
+- Be careful with multiple owner references (only one controller=true)
+- Handle OwnerRef cleanup when replacing existing resources
+
+## Output Format
+
+For each issue found, report:
+- **Location**: File:line or function name
+- **Issue**: What pattern is missing or incorrect
+- **Fix**: How to correct it
+- **Severity**: Critical / Warning / Suggestion
+
+## Example Review Comments
+
+**Critical**: Missing ManagementState.Removed handling in Sync() function. Controller will not clean up resources when console is removed.
+
+**Warning**: Status conditions not flushed. Add `statusHandler.FlushAndReturn(err)` at end of Sync().
+
+**Suggestion**: Consider grouping imports with standard comments for better readability.
diff --git a/.claude/skills/e2e-test-create.md b/.claude/skills/e2e-test-create.md
new file mode 100644
index 000000000..ba67600b4
--- /dev/null
+++ b/.claude/skills/e2e-test-create.md
@@ -0,0 +1,280 @@
+---
+name: e2e-test-create
+description: Scaffold new e2e tests following console-operator patterns and best practices
+tags: [create, testing, e2e, scaffold]
+---
+
+# E2E Test Creation Skill
+
+Create new end-to-end tests in `test/e2e/` directory following established patterns.
+
+## Test Template
+
+```go
+package e2e
+
+import (
+    "context"
+    "testing"
+    "time"
+
+    operatorv1 "github.com/openshift/api/operator/v1"
+    "github.com/openshift/console-operator/pkg/api"
+    "github.com/openshift/console-operator/test/e2e/framework"
+    metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+    "k8s.io/apimachinery/pkg/util/wait"
+    "k8s.io/client-go/util/retry"
+)
+
+func TestYourFeature(t *testing.T) {
+    // Setup client
+    client := framework.MustNewClientset(t, nil)
+    ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+    defer cancel()
+
+    // Get initial operator config
+    operatorConfig := framework.GetOperatorConfig(t, client.OperatorClient)
+    if operatorConfig.Spec.ManagementState != operatorv1.Managed {
+        t.Skip("Skipping test - operator not in Managed state")
+    }
+
+    // Test implementation
+    t.Run("SubtestName", func(t *testing.T) {
+        // Setup test resources
+        defer cleanupResources(t, client)
+
+        // Perform operations
+        err := updateOperatorConfig(ctx, client, func(config *operatorv1.Console) {
+            // Modify config
+        })
+        if err != nil {
+            t.Fatalf("failed to update config: %v", err)
+        }
+
+        // Wait for expected state
+        err = waitForCondition(ctx, t, client)
+        if err != nil {
+            t.Fatalf("expected condition not met: %v", err)
+        }
+
+        // Verify result
+        verifyExpectedState(t, client)
+    })
+}
+
+// Helper functions
+func updateOperatorConfig(
+    ctx context.Context,
+    client *framework.Clientset,
+    modify func(*operatorv1.Console),
+) error {
+    return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+        config, err := client.OperatorClient.Consoles().Get(
+            ctx, api.ConfigResourceName, metav1.GetOptions{},
+        )
+        if err != nil {
+            return err
+        }
+        modify(config)
+        _, err = client.OperatorClient.Consoles().Update(
+            ctx, config, metav1.UpdateOptions{},
+        )
+        return err
+    })
+}
+
+func waitForCondition(
+    ctx context.Context,
+    t *testing.T,
+    client *framework.Clientset,
+) error {
+    return wait.Poll(1*time.Second, 2*time.Minute, func() (bool, error) {
+        // Check for expected condition
+        deployment, err := client.KubeClient.AppsV1().Deployments(
+            api.OpenShiftConsoleNamespace,
+        ).Get(ctx, api.OpenShiftConsoleName, metav1.GetOptions{})
+        if err != nil {
+            return false, err
+        }
+        return deployment.Status.ReadyReplicas > 0, nil
+    })
+}
+
+func cleanupResources(t *testing.T, client *framework.Clientset) {
+    ctx := context.Background()
+    // Clean up in reverse order of creation
+    // Ignore NotFound errors
+}
+
+func verifyExpectedState(t *testing.T, client *framework.Clientset) {
+    // Make assertions about final state
+}
+```
+
+## Table-Driven Test Template
+
+```go
+func TestMultipleScenarios(t *testing.T) {
+    testCases := []struct {
+        name          string
+        setup         func(*operatorv1.Console)
+        expectedValue string
+        expectError   bool
+    }{
+        {
+            name: "scenario 1",
+            setup: func(config *operatorv1.Console) {
+                config.Spec.SomeField = "value1"
+            },
+            expectedValue: "result1",
+            expectError:   false,
+        },
+        {
+            name: "scenario 2",
+            setup: func(config *operatorv1.Console) {
+                config.Spec.SomeField = "value2"
+            },
+            expectedValue: "result2",
+            expectError:   false,
+        },
+    }
+
+    client := framework.MustNewClientset(t, nil)
+    ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
+    defer cancel()
+
+    for _, tc := range testCases {
+        t.Run(tc.name, func(t *testing.T) {
+            // Setup
+            err := updateOperatorConfig(ctx, client, tc.setup)
+            if err != nil {
+                t.Fatalf("setup failed: %v", err)
+            }
+            defer resetConfig(t, client)
+
+            // Execute test
+            result, err := performOperation(ctx, client)
+
+            // Verify
+            if tc.expectError && err == nil {
+                t.Error("expected error but got none")
+            }
+            if !tc.expectError && err != nil {
+                t.Errorf("unexpected error: %v", err)
+            }
+            if result != tc.expectedValue {
+                t.Errorf("expected %v, got %v", tc.expectedValue, result)
+            }
+        })
+    }
+}
+```
+
+## Plugin Test Pattern
+
+```go
+func TestConsolePlugin(t *testing.T) {
+    client := framework.MustNewClientset(t, nil)
+    ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+    defer cancel()
+
+    pluginName := "test-plugin"
+    
+    // Create plugin
+    plugin := &consolev1.ConsolePlugin{
+        ObjectMeta: metav1.ObjectMeta{
+            Name: pluginName,
+        },
+        Spec: consolev1.ConsolePluginSpec{
+            DisplayName: "Test Plugin",
+            Service: consolev1.ConsolePluginService{
+                Name:      "plugin-service",
+                Namespace: "plugin-namespace",
+                Port:      8443,
+                BasePath:  "/",
+            },
+        },
+    }
+
+    _, err := client.ConsoleClient.ConsoleV1().ConsolePlugins().Create(
+        ctx, plugin, metav1.CreateOptions{},
+    )
+    if err != nil {
+        t.Fatalf("failed to create plugin: %v", err)
+    }
+
+    defer func() {
+        err := client.ConsoleClient.ConsoleV1().ConsolePlugins().Delete(
+            ctx, pluginName, metav1.DeleteOptions{},
+        )
+        if err != nil {
+            t.Logf("cleanup warning: %v", err)
+        }
+    }()
+
+    // Wait for plugin to be processed
+    err = wait.Poll(1*time.Second, 1*time.Minute, func() (bool, error) {
+        deployment, err := client.KubeClient.AppsV1().Deployments(
+            api.OpenShiftConsoleNamespace,
+        ).Get(ctx, api.OpenShiftConsoleName, metav1.GetOptions{})
+        if err != nil {
+            return false, err
+        }
+        // Check for plugin in deployment
+        return containsPlugin(deployment, pluginName), nil
+    })
+    if err != nil {
+        t.Fatalf("plugin not reflected in deployment: %v", err)
+    }
+}
+```
+
+## Best Practices Checklist
+
+When creating e2e tests:
+
+- [ ] Use `framework.MustNewClientset(t, nil)` for client setup
+- [ ] Create context with timeout (`context.WithTimeout`)
+- [ ] Add cleanup with `defer`
+- [ ] Use `retry.RetryOnConflict` for config updates
+- [ ] Use `wait.Poll` for async operations (not `time.Sleep`)
+- [ ] Write helpful error messages with context
+- [ ] Add subtests for different scenarios
+- [ ] Test both success and failure paths
+- [ ] Verify operator responds to config changes
+- [ ] Check ClusterOperator status conditions
+- [ ] Clean up in reverse order of creation
+- [ ] Handle `IsNotFound` errors in cleanup
+- [ ] Use table-driven tests for multiple similar cases
+
+## Common Imports
+
+```go
+import (
+    "context"
+    "testing"
+    "time"
+
+    configv1 "github.com/openshift/api/config/v1"
+    consolev1 "github.com/openshift/api/console/v1"
+    operatorv1 "github.com/openshift/api/operator/v1"
+    "github.com/openshift/console-operator/pkg/api"
+    "github.com/openshift/console-operator/test/e2e/framework"
+    apierrors "k8s.io/apimachinery/pkg/api/errors"
+    metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+    "k8s.io/apimachinery/pkg/util/wait"
+    "k8s.io/client-go/util/retry"
+)
+```
+
+## Output
+
+Generate complete, runnable test code with:
+1. Proper imports
+2. Framework setup
+3. Resource creation
+4. Cleanup logic
+5. Wait patterns
+6. Assertions
+7. Helpful error messages
+8. Comments explaining the test flow
diff --git a/.claude/skills/e2e-test-review.md b/.claude/skills/e2e-test-review.md
new file mode 100644
index 000000000..ab1dd00c4
--- /dev/null
+++ b/.claude/skills/e2e-test-review.md
@@ -0,0 +1,212 @@
+---
+name: e2e-test-review
+description: Review e2e tests for best practices, proper cleanup, and wait patterns
+tags: [review, testing, e2e]
+---
+
+# E2E Test Review Skill
+
+Review end-to-end tests in `test/e2e/` directory for correctness and best practices.
+
+## What to Check
+
+### 1. Test Structure
+Tests should follow this pattern:
+```go
+func TestFeatureName(t *testing.T) {
+    client := framework.MustNewClientset(t, nil)
+    
+    // Setup
+    defer cleanupFunction(t, client)
+    
+    // Test logic with subtests
+    t.Run("subtest1", func(t *testing.T) {
+        // test code
+    })
+}
+```
+
+### 2. Table-Driven Tests
+For testing multiple scenarios, use table-driven pattern:
+```go
+func TestMultipleScenarios(t *testing.T) {
+    testCases := []struct {
+        name     string
+        input    string
+        expected string
+    }{
+        {name: "scenario1", input: "a", expected: "b"},
+        {name: "scenario2", input: "c", expected: "d"},
+    }
+    
+    for _, tc := range testCases {
+        t.Run(tc.name, func(t *testing.T) {
+            result := doSomething(tc.input)
+            if result != tc.expected {
+                t.Errorf("expected %v, got %v", tc.expected, result)
+            }
+        })
+    }
+}
+```
+
+### 3. Cleanup and Teardown
+Always clean up created resources:
+```go
+// Use defer for cleanup
+defer func() {
+    err := client.Delete(ctx, name, metav1.DeleteOptions{})
+    if err != nil && !apierrors.IsNotFound(err) {
+        t.Logf("cleanup failed: %v", err)
+    }
+}()
+```
+
+### 4. Wait Patterns
+Use `wait.Poll` for eventual consistency:
+```go
+err := wait.Poll(1*time.Second, 5*time.Minute, func() (bool, error) {
+    deployment, err := client.AppsV1().Deployments(ns).Get(
+        ctx, name, metav1.GetOptions{},
+    )
+    if err != nil {
+        return false, err
+    }
+    return deployment.Status.ReadyReplicas > 0, nil
+})
+if err != nil {
+    t.Fatalf("deployment never became ready: %v", err)
+}
+```
+
+### 5. Retry Patterns
+Use `retry.RetryOnConflict` for updates:
+```go
+err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+    config, err := client.Get(ctx, name, metav1.GetOptions{})
+    if err != nil {
+        return err
+    }
+    config.Spec.SomeField = "newValue"
+    _, err = client.Update(ctx, config, metav1.UpdateOptions{})
+    return err
+})
+```
+
+### 6. Context Usage
+Always use context with timeout:
+```go
+ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+defer cancel()
+```
+
+### 7. Framework Usage
+Use test framework helpers:
+```go
+// Good - uses framework
+client := framework.MustNewClientset(t, nil)
+operatorConfig := framework.GetOperatorConfig(t, client)
+
+// Bad - manual setup
+config, _ := clientcmd.BuildConfigFromFlags("", kubeconfig)
+client, _ := kubernetes.NewForConfig(config)
+```
+
+### 8. Error Assertions
+Be specific about expected errors:
+```go
+// Good - checks specific error
+if !apierrors.IsNotFound(err) {
+    t.Fatalf("expected NotFound, got %v", err)
+}
+
+// Bad - vague
+if err != nil {
+    t.Fatal("got error")
+}
+```
+
+### 9. Test Isolation
+Tests should not depend on each other:
+- Each test creates its own resources
+- Cleanup removes all test resources
+- No shared mutable state between tests
+
+### 10. Assertion Quality
+```go
+// Good - clear message with context
+if actual != expected {
+    t.Errorf("plugin config mismatch: expected %v, got %v", expected, actual)
+}
+
+// Bad - no context
+if actual != expected {
+    t.Error("mismatch")
+}
+```
+
+## Red Flags
+
+- Tests without cleanup (resource leaks)
+- Missing `wait.Poll` when checking async operations
+- Direct sleeps instead of wait conditions
+- Tests that modify global state
+- Tests without timeout contexts
+- Assertions without helpful error messages
+- Tests that require specific ordering to pass
+
+## E2E-Specific Patterns
+
+### Console Config Updates
+```go
+err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+    config, err := client.ConfigV1().Consoles().Get(
+        ctx, "cluster", metav1.GetOptions{},
+    )
+    if err != nil {
+        return err
+    }
+    config.Spec.Customization = &configv1.ConsoleCustomization{
+        Brand: configv1.BrandOKD,
+    }
+    _, err = client.ConfigV1().Consoles().Update(
+        ctx, config, metav1.UpdateOptions{},
+    )
+    return err
+})
+```
+
+### Plugin Testing
+```go
+// Create plugin
+plugin := &consolev1.ConsolePlugin{
+    ObjectMeta: metav1.ObjectMeta{Name: "test-plugin"},
+    Spec: consolev1.ConsolePluginSpec{
+        DisplayName: "Test Plugin",
+        Service: consolev1.ConsolePluginService{
+            Name:      "plugin-service",
+            Namespace: "plugin-ns",
+            Port:      8443,
+        },
+    },
+}
+defer cleanupPlugin(t, client, plugin.Name)
+```
+
+## Output Format
+
+For each issue:
+- **Test**: Test function name
+- **Issue**: What's wrong
+- **Risk**: Why it matters
+- **Fix**: How to improve
+
+## Example Review Comments
+
+**Critical**: Test has no cleanup - will leak ConsolePlugin resource. Add defer with cleanup.
+
+**Warning**: Using time.Sleep instead of wait.Poll. This makes test flaky and slower than necessary.
+
+**Suggestion**: Consider using table-driven tests to cover multiple plugin configurations.
+
+**Info**: Good use of retry.RetryOnConflict for config updates.
diff --git a/.claude/skills/go-quality-review.md b/.claude/skills/go-quality-review.md
new file mode 100644
index 000000000..edf930efc
--- /dev/null
+++ b/.claude/skills/go-quality-review.md
@@ -0,0 +1,318 @@
+---
+name: go-quality-review
+description: Review Go code for quality, deprecated APIs, and common anti-patterns
+tags: [review, go, quality, deprecated]
+---
+
+# Go Code Quality Review Skill
+
+Review Go code for quality issues, deprecated APIs, and common problems.
+
+## Deprecated API Detection
+
+### Check for Deprecated Imports and Functions
+
+**Deprecated (Do NOT use):**
+```go
+// DEPRECATED: Use os.ReadFile
+ioutil.ReadFile(filename)
+ioutil.WriteFile(filename, data, perm)
+ioutil.ReadAll(reader)
+
+// DEPRECATED: Use DialContext
+Dial: func(network, addr string) (net.Conn, error) {
+    return net.Dial(network, addr)
+}
+
+// DEPRECATED: Various k8s.io/utils functions
+// Check specific package documentation
+```
+
+**Modern alternatives:**
+```go
+// Use os package (Go 1.16+)
+os.ReadFile(filename)
+os.WriteFile(filename, data, perm)
+io.ReadAll(reader)
+
+// Use DialContext
+DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
+    return (&net.Dialer{}).DialContext(ctx, network, addr)
+}
+```
+
+## Error Handling
+
+### Error Wrapping
+**Good:**
+```go
+if err != nil {
+    return fmt.Errorf("failed to get config: %w", err)
+}
+```
+
+**Bad:**
+```go
+if err != nil {
+    return fmt.Errorf("failed to get config: %v", err) // Lost error chain
+}
+```
+
+### Error Context
+Add context to errors:
+```go
+// Good - explains what was happening
+return fmt.Errorf("failed to sync deployment %s in namespace %s: %w", 
+    name, namespace, err)
+
+// Bad - no context
+return err
+```
+
+### Checking Specific Errors
+```go
+// Good - specific error check
+if apierrors.IsNotFound(err) {
+    // Handle not found
+}
+
+// Bad - string matching
+if strings.Contains(err.Error(), "not found") {
+    // Fragile
+}
+```
+
+## Resource Management
+
+### Context Propagation
+**Good:**
+```go
+func (c *Controller) Sync(ctx context.Context, ...) error {
+    deployment, err := c.client.Get(ctx, name, metav1.GetOptions{})
+    // ...
+}
+```
+
+**Bad:**
+```go
+func (c *Controller) Sync(ctx context.Context, ...) error {
+    deployment, err := c.client.Get(context.Background(), name, metav1.GetOptions{})
+    // Ignores parent context cancellation
+}
+```
+
+### Defer Usage
+```go
+// Good - cleanup on all paths
+func doWork(ctx context.Context) error {
+    resource, err := acquire()
+    if err != nil {
+        return err
+    }
+    defer release(resource)
+    
+    return performWork(resource)
+}
+
+// Bad - leaks on early return
+func doWork(ctx context.Context) error {
+    resource, err := acquire()
+    if err != nil {
+        return err // LEAK!
+    }
+    
+    if someCondition {
+        return errors.New("failed") // LEAK!
+    }
+    
+    release(resource)
+    return nil
+}
+```
+
+## Code Smells
+
+### God Functions
+Flag functions longer than ~100 lines or with too many responsibilities:
+```go
+// BAD - doing too much
+func (c *Controller) Sync(...) error {
+    // 50 lines of config validation
+    // 100 lines of deployment logic
+    // 75 lines of service setup
+    // 80 lines of route configuration
+    // = 305 lines, should be split
+}
+
+// GOOD - extracted
+func (c *Controller) Sync(...) error {
+    if err := c.validateConfig(); err != nil {
+        return err
+    }
+    if err := c.syncDeployment(); err != nil {
+        return err
+    }
+    if err := c.syncService(); err != nil {
+        return err
+    }
+    return c.syncRoute()
+}
+```
+
+### Magic Values
+```go
+// Bad - magic numbers/strings
+if replicas == 2 {
+    // Why 2?
+}
+timeout := 30 * time.Second // Why 30?
+if mode == "special" {
+    // What makes it special?
+}
+
+// Good - named constants
+const (
+    DefaultReplicaCount = 2 // HA requirement
+    DefaultTimeout = 30 * time.Second
+    SpecialMode = "special" // Tech preview mode
+)
+```
+
+### Deep Nesting
+```go
+// Bad - deeply nested
+if condition1 {
+    if condition2 {
+        if condition3 {
+            if condition4 {
+                // business logic buried here
+            }
+        }
+    }
+}
+
+// Good - early returns
+if !condition1 {
+    return nil
+}
+if !condition2 {
+    return nil
+}
+if !condition3 {
+    return nil
+}
+if !condition4 {
+    return nil
+}
+// business logic at top level
+```
+
+## Performance
+
+### String Building
+```go
+// Bad - inefficient string concatenation
+var result string
+for _, item := range items {
+    result += item + "\n"
+}
+
+// Good - strings.Builder
+var builder strings.Builder
+for _, item := range items {
+    builder.WriteString(item)
+    builder.WriteString("\n")
+}
+result := builder.String()
+```
+
+### Unnecessary Allocations
+```go
+// Bad - allocates on every call
+func (c *Controller) getNamespace() string {
+    return []byte("openshift-console")[0:] // Unnecessary allocation
+}
+
+// Good - use constant
+const namespace = "openshift-console"
+```
+
+## Concurrency
+
+### Missing Mutex Protection
+```go
+// Bad - race condition
+type Cache struct {
+    data map[string]string
+}
+func (c *Cache) Set(k, v string) {
+    c.data[k] = v // RACE if called from multiple goroutines
+}
+
+// Good - protected
+type Cache struct {
+    mu   sync.RWMutex
+    data map[string]string
+}
+func (c *Cache) Set(k, v string) {
+    c.mu.Lock()
+    defer c.mu.Unlock()
+    c.data[k] = v
+}
+```
+
+## Testing
+
+### Missing Error Checks
+```go
+// Bad - ignoring errors in tests
+result, _ := doSomething()
+if result != expected {
+    t.Error("mismatch")
+}
+
+// Good - checking errors
+result, err := doSomething()
+if err != nil {
+    t.Fatalf("unexpected error: %v", err)
+}
+if result != expected {
+    t.Errorf("expected %v, got %v", expected, result)
+}
+```
+
+## Documentation
+
+### Exported Functions
+```go
+// Good - documented export
+// NewController creates a new console controller.
+// It requires a valid operatorClient and informer factory.
+func NewController(...) factory.Controller {
+    // ...
+}
+
+// Bad - no doc
+func NewController(...) factory.Controller {
+    // ...
+}
+```
+
+## Output Format
+
+For each issue:
+- **File:Line**: Location
+- **Issue**: What's wrong
+- **Category**: Deprecated / Error Handling / Code Smell / Performance / etc.
+- **Fix**: How to improve
+- **Priority**: High / Medium / Low
+
+## Example Review Comments
+
+**High Priority**: pkg/console/operator/sync.go:45 - Using deprecated ioutil.ReadFile. Replace with os.ReadFile.
+
+**Medium Priority**: pkg/console/controllers/route/controller.go:123 - Deep nesting (5 levels). Consider early returns.
+
+**Low Priority**: Consider adding godoc comment for exported function NewRouteController.
+
+**Info**: Good use of context propagation throughout sync handler.
diff --git a/.claude/skills/manifest-review.md b/.claude/skills/manifest-review.md
new file mode 100644
index 000000000..9b2a03249
--- /dev/null
+++ b/.claude/skills/manifest-review.md
@@ -0,0 +1,88 @@
+---
+name: manifest-review
+description: Review RBAC manifests and CVO manifest files for cluster profile annotations and correctness
+tags: [review, manifest, rbac, cvo]
+---
+
+# Manifest Review Skill
+
+Review Kubernetes manifest files in `manifests/` and `bindata/assets/` directories.
+
+## What to Check
+
+### 1. Cluster Profile Annotations
+All CVO manifests MUST include appropriate cluster profile annotations:
+
+```yaml
+annotations:
+  include.release.openshift.io/hypershift: "true"
+  include.release.openshift.io/ibm-cloud-managed: "true"
+  include.release.openshift.io/self-managed-high-availability: "true"
+  include.release.openshift.io/single-node-developer: "true"
+  capability.openshift.io/name: Console
+```
+
+**Common profiles:**
+- `hypershift` - HyperShift hosted control planes
+- `ibm-cloud-managed` - IBM Cloud managed OpenShift
+- `self-managed-high-availability` - Standard self-managed HA clusters
+- `single-node-developer` - Single-node OpenShift (SNO)
+
+### 2. Capability Annotations
+Console resources should include:
+```yaml
+capability.openshift.io/name: Console
+```
+
+### 3. RBAC Rules
+Check RBAC manifests for:
+- **Principle of least privilege**: Only grant necessary permissions
+- **Resource specificity**: Avoid wildcards unless truly needed
+- **Verb appropriateness**: Match verbs to actual needs (get/list/watch vs create/update/delete)
+- **apiGroups correctness**: Use proper API groups ("" for core, specific groups for CRDs)
+
+### 4. Namespace Consistency
+- Resources in `openshift-console` namespace for console workload
+- Resources in `openshift-console-operator` namespace for operator
+- Cross-namespace references are explicit and intentional
+
+### 5. YAML Formatting
+- Proper indentation (2 spaces)
+- Consistent ordering of fields
+- Include `---` separator between multiple resources in one file
+
+### 6. Service Account References
+When binding roles to service accounts:
+```yaml
+subjects:
+  - kind: ServiceAccount
+    name: console
+    namespace: openshift-console
+```
+
+### 7. Profile-Specific Patches
+Check `profile-patches/` for overrides that should only apply to specific deployments.
+
+## Red Flags
+
+- Missing cluster profile annotations on new manifests
+- RBAC with `*` wildcards without clear justification
+- Cross-namespace permissions without clear need
+- Missing capability annotations
+- Hardcoded values that should be configurable
+
+## Output Format
+
+For each issue:
+- **File**: manifest filename
+- **Issue**: What's wrong
+- **Impact**: Which clusters/profiles are affected
+- **Fix**: Recommended correction
+
+## Example Review Comments
+
+**Critical**: manifests/new-role.yaml missing cluster profile annotations. This resource won't be deployed to any clusters.
+
+**Warning**: RBAC grants `*` verbs on configmaps. Consider restricting to specific verbs needed.
+
+**Info**: Consider adding hypershift annotation if this resource is needed for hosted control planes.
diff --git a/.claude/skills/sync-handler-review.md b/.claude/skills/sync-handler-review.md
new file mode 100644
index 000000000..824ba3a0a
--- /dev/null
+++ b/.claude/skills/sync-handler-review.md
@@ -0,0 +1,151 @@
+---
+name: sync-handler-review
+description: Review operator sync handler logic for incremental reconciliation patterns
+tags: [review, sync, reconciliation, operator]
+---
+
+# Sync Handler Review Skill
+
+Review sync handler implementations (especially `sync_v400.go` and controller Sync methods).
+
+## What to Check
+
+### 1. Incremental Sync Pattern
+The operator uses an incremental sync pattern where each loop picks up where the previous left off:
+
+**Good Pattern:**
+```go
+func (c *OperatorController) Sync(ctx context.Context, ...) error {
+    // 1. Check prerequisite resources
+    requiredConfig, err := c.getRequiredConfig()
+    if err != nil {
+        return err // Will retry
+    }
+
+    // 2. Sync first dependency
+    if err := c.syncConfigMaps(ctx, requiredConfig); err != nil {
+        return err // Stop here, retry next loop
+    }
+
+    // 3. Sync second dependency (only if step 2 succeeded)
+    if err := c.syncSecrets(ctx); err != nil {
+        return err
+    }
+
+    // 4. Sync final resource
+    return c.syncDeployment(ctx, requiredConfig)
+}
+```
+
+**Bad Pattern:**
+```go
+func (c *OperatorController) Sync(ctx context.Context, ...) error {
+    // DON'T collect all errors and continue
+    var errs []error
+    errs = append(errs, c.syncConfigMaps(ctx))
+    errs = append(errs, c.syncSecrets(ctx))
+    errs = append(errs, c.syncDeployment(ctx))
+    return errors.Join(errs...)
+}
+```
+
+### 2. Dependency Ordering
+Resources should be synced in dependency order:
+1. ConfigMaps and Secrets (prerequisites)
+2. Service Accounts
+3. RBAC (Roles, RoleBindings)
+4. Services
+5. Deployments (depend on above)
+6. Routes (depend on Services)
+
+### 3. Status Updates
+Status should reflect actual reconciliation state:
+```go
+statusHandler := status.NewStatusHandler(c.operatorClient)
+
+// Add conditions as operations complete
+if err := c.syncResource(ctx); err != nil {
+    statusHandler.AddConditions(
+        status.HandleProgressingOrDegraded("ResourceSync", "FailedApply", err)
+    )
+    return statusHandler.FlushAndReturn(err)
+}
+
+// Mark available when fully synced
+statusHandler.AddCondition(
+    status.HandleAvailable("DeploymentAvailable", "", nil)
+)
+return statusHandler.FlushAndReturn(nil)
+```
+
+### 4. Early Returns
+Return early on errors to maintain incremental behavior:
+```go
+// GOOD - returns immediately
+if err := c.doSomething(); err != nil {
+    return err
+}
+
+// BAD - continues after error
+if err := c.doSomething(); err != nil {
+    klog.Errorf("failed: %v", err)
+}
+```
+
+### 5. Resource Creation vs Update
+Use `resourceapply.Apply*()` functions - they handle both create and update:
+```go
+_, _, err = resourceapply.ApplyConfigMap(
+    ctx,
+    c.configMapClient,
+    recorder,
+    requiredConfigMap,
+)
+```
+
+### 6. Deleted Resource Detection
+When resources are removed from config, ensure they're deleted from cluster:
+```go
+if !shouldExist {
+    err := c.client.Delete(ctx, name, metav1.DeleteOptions{})
+    if apierrors.IsNotFound(err) {
+        return nil // Already gone, success
+    }
+    return err
+}
+```
+
+### 7. Feature Gate Handling
+Check feature gates before syncing gated resources:
+```go
+if featureGates.Enabled(features.ExternalOIDC) {
+    if err := c.syncOIDCResources(ctx); err != nil {
+        return err
+    }
+}
+```
+
+## Anti-Patterns
+
+- Syncing all resources regardless of errors
+- Continuing after failure instead of returning
+- Not respecting ManagementState
+- Missing cleanup logic for Removed state
+- Mutating live objects instead of building desired state
+- Lack of status condition updates
+
+## Output Format
+
+For each issue:
+- **Location**: Function or line range
+- **Pattern**: Which pattern is violated
+- **Risk**: What could go wrong
+- **Fix**: How to correct it
+
+## Example Review Comments
+
+**Critical**: Sync continues after configmap error. This violates incremental sync pattern and could deploy with incomplete config.
+
+**Warning**: No cleanup logic for ManagementState.Removed. Resources will leak when console is removed.
+
+**Suggestion**: Consider extracting this sync logic into a separate controller for better separation of concerns.
diff --git a/.coderabbit.yaml b/.coderabbit.yaml
index 9dae7c411..1a975d0d5 100644
--- a/.coderabbit.yaml
+++ b/.coderabbit.yaml
@@ -1,5 +1,18 @@
 # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
 
+# CodeRabbit configuration for console-operator
+# This config integrates Claude Code skills for operator-specific reviews.
+# Skills are triggered based on CODE PATTERNS (function signatures, types, YAML kinds)
+# rather than file paths alone.
+#
+# Available skills (defined in .claude/skills/):
+#   /controller-review - When code has Controller types, factory.New() patterns
+#   /sync-handler-review - When code has sequential sync logic with early returns
+#   /manifest-review - When YAML has RBAC kinds or missing annotations
+#   /e2e-test-review - When tests use framework, wait.Poll, retry patterns
+#   /e2e-test-create - Scaffold new e2e tests with best practices
+#   /go-quality-review - Always applied; checks deprecated APIs, code smells
+
 language: en-US
 inheritance: true
 
@@ -17,25 +30,86 @@ reviews:
   - "!vendor/**"
   - "!**/*_test.go"
   path_instructions:
-    - path: "pkg/**/*.go"
+    - path: "**/*.go"
       instructions: |
         Review Go code following OpenShift operator patterns.
         See CONVENTIONS.md for coding standards and patterns.
-        Controllers should use the library-go factory pattern.
-        Status conditions should use status.Handle* functions.
-    - path: "bindata/assets/**/*.yaml"
-      instructions: |
-        Review YAML assets for Kubernetes resource correctness.
-        Ensure proper annotations for cluster profiles.
-    - path: "manifests/**/*.yaml"
+
+        Apply skills based on CODE PATTERNS, not just file paths:
+
+        **Apply /controller-review when code contains:**
+        - Controller struct types (e.g., `type *Controller struct`)
+        - `func New*Controller(` factory functions
+        - `factory.New().WithFilteredEventsInformers(` pattern
+        - `.ToController(` method calls
+        - `Sync(ctx context.Context, controllerContext factory.SyncContext)` methods
+        - `operatorConfig.Spec.ManagementState` checks
+        - `status.NewStatusHandler` or `status.Handle*` functions
+
+        **Apply /sync-handler-review when code contains:**
+        - Main operator sync functions (e.g., `sync_v400.go` content)
+        - Sequential resource syncing with early returns
+        - Incremental reconciliation loops
+        - Multiple `resourceapply.Apply*()` calls in sequence
+        - Dependency ordering of ConfigMaps → Secrets → Deployments → Routes
+        - Feature gate conditional logic
+
+        **Apply /go-quality-review for all Go code to check:**
+        - Deprecated imports: `ioutil.ReadFile`, `ioutil.WriteFile`, `ioutil.ReadAll`
+        - Deprecated patterns: `Dial` without `DialContext`
+        - Error handling: missing `%w` in fmt.Errorf
+        - Code smells: deep nesting (4+ levels), functions >100 lines
+        - Magic values: unexplained numbers/strings
+        - Context propagation: `context.Background()` instead of passed ctx
+        - Missing godoc on exported functions
+
+    - path: "**/*_test.go"
       instructions: |
-        Review CVO manifests carefully.
-        These are deployed to clusters and changes have wide impact.
-    - path: "quickstarts/**/*.yaml"
+        Review test code for quality and patterns.
+
+        **Apply /e2e-test-review when test contains:**
+        - `framework.MustNewClientset(t, nil)` or similar e2e framework usage
+        - `wait.Poll` or `wait.PollImmediate` patterns
+        - `retry.RetryOnConflict` for updates
+        - Cleanup via `defer` functions
+        - Console/operator CR manipulations
+        - Test assertions on cluster state
+
+        **Suggest /e2e-test-create when:**
+        - PR adds new feature requiring e2e coverage
+        - Test file is empty or skeleton
+        - Comments indicate "TODO: add test"
+
+        **Review for common issues:**
+        - Missing cleanup (defer statements)
+        - Using `time.Sleep` instead of `wait.Poll`
+        - Missing context timeouts
+        - Vague error messages in assertions
+        - Tests without table-driven structure when testing multiple cases
+
+    - path: "**/*.yaml"
       instructions: |
-        Review quickstart YAML definitions.
-        Ensure cluster profile annotations are present.
-        See quickstarts/README.md for guidelines.
+        Review YAML manifests based on content and kind.
+
+        **Apply /manifest-review when YAML contains:**
+        - `kind: Role` or `kind: ClusterRole` (RBAC review)
+        - `kind: RoleBinding` or `kind: ClusterRoleBinding`
+        - `annotations:` section (check for cluster profiles)
+        - `verbs: ["*"]` or wildcard permissions
+        - `apiGroups: ["*"]` or overly broad permissions
+        - ServiceAccount references in subjects
+
+        **Check for required annotations in manifests/:**
+        - `include.release.openshift.io/hypershift`
+        - `include.release.openshift.io/ibm-cloud-managed`
+        - `include.release.openshift.io/self-managed-high-availability`
+        - `include.release.openshift.io/single-node-developer`
+        - `capability.openshift.io/name: Console`
+
+        **For quickstarts/, additionally check:**
+        - QuickStart spec structure
+        - Task descriptions and prerequisites
+        - See quickstarts/README.md for guidelines
 
 chat:
   auto_reply: true
@@ -51,6 +125,7 @@ knowledge_base:
       - "ARCHITECTURE.md"
       - "CONVENTIONS.md"
       - "TESTING.md"
+      - ".claude/skills/*.md"
   linked_repositories:
     - repository: "openshift/console"
       instructions: >-
diff --git a/.github/CODERABBIT_SETUP.md b/.github/CODERABBIT_SETUP.md
new file mode 100644
index 000000000..fd5f52261
--- /dev/null
+++ b/.github/CODERABBIT_SETUP.md
@@ -0,0 +1,118 @@
+# CodeRabbit and Claude Code Integration Setup
+
+This document describes the AI tooling integration for the console-operator repository.
+
+## Overview
+
+The console-operator uses two complementary AI tools:
+- **Claude Code**: Interactive development assistant with custom operator skills
+- **CodeRabbit**: Automated PR review bot that references Claude skills
+
+## Claude Code Skills
+
+Custom skills are defined in `.claude/skills/`:
+
+| Skill | Purpose | Use When |
+|-------|---------|----------|
+| `controller-review` | Review controller patterns | Reviewing controller implementations |
+| `sync-handler-review` | Review sync/reconciliation logic | Reviewing operator sync handlers |
+| `manifest-review` | Review RBAC/CVO manifests | Reviewing manifest changes |
+| `e2e-test-review` | Review e2e tests | Reviewing test code |
+| `e2e-test-create` | Scaffold new e2e tests | Creating new e2e tests |
+| `go-quality-review` | Review code quality | Checking for deprecated APIs, code smells |
+
+### Using Skills
+
+In Claude Code CLI or IDE extensions:
+```
+/controller-review
+/e2e-test-review
+```
+
+In PR reviews (CodeRabbit will reference these automatically based on code patterns, not just file paths).
+
+## CodeRabbit Configuration
+
+Configuration is in `.coderabbit.yaml`. It:
+- References project documentation (ARCHITECTURE.md, CONVENTIONS.md, etc.)
+- Triggers skills based on **code patterns** (function signatures, types, YAML kinds) not just file paths
+- Integrates with openshift/console repository context
+- Provides operator-specific review guidelines
+
+### Pattern-Based Skill Triggering
+
+Skills are applied when specific code patterns are detected:
+
+**controller-review** triggers when code contains:
+- `type *Controller struct` definitions
+- `factory.New().WithFilteredEventsInformers()` pattern
+- `Sync(ctx context.Context, controllerContext factory.SyncContext)` methods
+- `status.NewStatusHandler` usage
+
+**sync-handler-review** triggers when code contains:
+- Sequential `resourceapply.Apply*()` calls with early returns
+- Feature gate conditional logic
+- Incremental reconciliation patterns
+
+**manifest-review** triggers when YAML contains:
+- `kind: Role` or `kind: ClusterRole`
+- Missing cluster profile annotations in `manifests/`
+- Wildcard permissions (`verbs: ["*"]`)
+
+**e2e-test-review** triggers when tests contain:
+- `framework.MustNewClientset(t, nil)`
+- `wait.Poll` or `retry.RetryOnConflict`
+- Console/operator CR manipulations
+
+This approach is more reliable than path-based triggering, as it recognizes what the code actually does.
+
+## CI/CD Integration
+
+### Skip E2E Tests for Tooling Changes
+
+Changes to AI tooling configuration should not trigger e2e test runs.
+
+**This repository (console-operator):**
+- OWNERS file documents the intent (see comments)
+- `.coderabbit.yaml` contains the configuration
+- `.claude/skills/` contains Claude skills
+
+**openshift/release repository:**
+
+To skip e2e tests for tooling changes, add to the ci-operator configuration for console-operator:
+
+```yaml
+skip_if_only_changed: "^(\\.coderabbit\\.yaml|\\.claude/|.*\\.md|OWNERS)$"
+```
+
+Or in Prow configuration:
+```yaml
+- name: pull-ci-openshift-console-operator-main-e2e
+  skip_if_only_changed: "^(\\.coderabbit\\.yaml|\\.claude/|.*\\.md|OWNERS)$"
+```
+
+**Action Required:** Update openshift/release configuration to implement this skip pattern.
+
+## Updating Skills
+
+To add or modify Claude skills:
+
+1. Edit or create skill files in `.claude/skills/`
+2. Update `.coderabbit.yaml` path_instructions to reference new skills
+3. Test the skill locally with Claude Code: `/skill-name`
+4. Commit changes (will not trigger e2e tests once openshift/release is updated)
+
+## Syncing with openshift/console
+
+The console repository has similar AI tooling. When adding patterns here, consider:
+- Are there equivalent patterns for TypeScript/React code?
+- Should frontend-specific skills be added to console repo?
+- Do both repos need to reference the same conventions?
+
+Keep tooling configuration aligned between repos where patterns overlap.
+
+## Reference
+
+- [Claude Code Documentation](https://docs.anthropic.com/claude/docs)
+- [CodeRabbit Documentation](https://coderabbit.ai/docs)
+- [OpenShift CI/CD Docs](https://docs.ci.openshift.org/)

From 5fae84905efd79e274f52464b9bc37ee08cb5ef3 Mon Sep 17 00:00:00 2001
From: Jon Jackson <jonjacks@redhat.com>
Date: Thu, 23 Apr 2026 16:38:02 -0400
Subject: [PATCH 2/3] CONSOLE-5195: Address review feedback and add
 unit-test-review skill

Fix issues identified by CodeRabbit and jhadvig:
- Remove test file exclusion from .coderabbit.yaml (enables skill application)
- Replace non-existent framework.GetOperatorConfig with actual pattern
- Update ServiceAccount examples to show both operator and console SA patterns
- Add quickstarts/ and examples/ directories to manifest-review scope
- Replace hardcoded timeouts with framework.AsyncOperationTimeout
- Fix go-quality-review type mismatch in allocation example
- Clarify CodeRabbit integration language (applies vs triggers)
- Remove redundant .claude/README.md (already deleted)

Add comprehensive unit-test-review skill:
- Table-driven test patterns matching repo conventions
- Deep equality checks using go-test/deep
- Error handling with wantErr pattern
- Edge case coverage guidelines
- Test isolation and assertion quality
- Integrated into .coderabbit.yaml and CODERABBIT_SETUP.md

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
---
 .claude/README.md                   | 156 --------------
 .claude/skills/e2e-test-create.md   |  49 ++++-
 .claude/skills/e2e-test-review.md   |   4 +-
 .claude/skills/go-quality-review.md |   2 +-
 .claude/skills/manifest-review.md   |  18 +-
 .claude/skills/unit-test-review.md  | 301 ++++++++++++++++++++++++++++
 .coderabbit.yaml                    |  13 +-
 .github/CODERABBIT_SETUP.md         |  24 ++-
 8 files changed, 389 insertions(+), 178 deletions(-)
 delete mode 100644 .claude/README.md
 create mode 100644 .claude/skills/unit-test-review.md

diff --git a/.claude/README.md b/.claude/README.md
deleted file mode 100644
index 10447ad0d..000000000
--- a/.claude/README.md
+++ /dev/null
@@ -1,156 +0,0 @@
-# Claude Code Configuration for Console Operator
-
-This directory contains Claude Code configuration and custom skills for the OpenShift console-operator.
-
-## Directory Structure
-
-```
-.claude/
-├── README.md           # This file
-└── skills/             # Custom operator-specific skills
-    ├── controller-review.md      # Controller pattern review
-    ├── sync-handler-review.md    # Sync handler review
-    ├── manifest-review.md        # RBAC/manifest review
-    ├── e2e-test-review.md        # E2E test review
-    ├── e2e-test-create.md        # E2E test scaffolding
-    └── go-quality-review.md      # Code quality review
-```
-
-## Skills Overview
-
-### Operator Development Skills
-
-**controller-review** - Review controller implementations
-- Checks library-go factory pattern usage
-- Validates ManagementState handling (Managed/Unmanaged/Removed)
-- Verifies status condition handling
-- Ensures proper import organization
-- Validates error handling and resource application
-
-**sync-handler-review** - Review sync/reconciliation handlers
-- Validates incremental sync pattern
-- Checks dependency ordering
-- Verifies status updates
-- Ensures early returns on errors
-- Validates resource creation vs update logic
-- Checks feature gate handling
-
-**manifest-review** - Review Kubernetes manifests
-- Validates cluster profile annotations
-- Checks RBAC least privilege
-- Verifies namespace consistency
-- Ensures proper YAML formatting
-- Validates service account references
-
-### Testing Skills
-
-**e2e-test-review** - Review end-to-end tests
-- Validates test structure and cleanup
-- Checks for proper wait patterns (wait.Poll, not time.Sleep)
-- Verifies retry patterns for config updates
-- Ensures context usage with timeouts
-- Validates framework usage
-- Checks assertion quality
-
-**e2e-test-create** - Scaffold new e2e tests
-- Provides complete test templates
-- Includes proper framework setup
-- Shows cleanup patterns
-- Demonstrates wait and retry patterns
-- Includes table-driven test examples
-
-### Code Quality Skills
-
-**go-quality-review** - General Go code quality
-- Detects deprecated APIs (ioutil.ReadFile, etc.)
-- Validates error handling and wrapping
-- Checks context propagation
-- Identifies code smells (god functions, magic values, deep nesting)
-- Reviews performance issues
-- Checks concurrency safety
-
-## Using Skills
-
-### In Claude Code
-
-```bash
-# Review a controller
-/controller-review
-
-# Review sync handler
-/sync-handler-review
-
-# Review e2e tests
-/e2e-test-review
-
-# Create new e2e test
-/e2e-test-create
-
-# Review manifests
-/manifest-review
-
-# General code quality review
-/go-quality-review
-```
-
-### In CodeRabbit PR Reviews
-
-CodeRabbit automatically references these skills based on code patterns (configured in `.coderabbit.yaml`):
-- Controller factory patterns (`factory.New()`, `ToController()`) → controller-review
-- Sequential sync logic with early returns → sync-handler-review
-- E2E framework usage (`framework.MustNewClientset`, `wait.Poll`) → e2e-test-review
-- YAML with RBAC kinds or missing annotations → manifest-review
-- All Go code → go-quality-review (deprecated APIs, code smells)
-
-## Project Context
-
-These skills reference and enforce patterns documented in:
-- `ARCHITECTURE.md` - System architecture and components
-- `CONVENTIONS.md` - Go coding standards and patterns
-- `TESTING.md` - Testing patterns and commands
-- `AGENTS.md` - AI context hub
-
-## Extending Skills
-
-To add a new skill:
-
-1. Create a new `.md` file in `.claude/skills/`
-2. Use this frontmatter:
-   ```yaml
-   ---
-   name: skill-name
-   description: What this skill does
-   tags: [relevant, tags]
-   ---
-   ```
-3. Document what to check, patterns to follow, and output format
-4. Update `.coderabbit.yaml` to reference the skill in path_instructions
-5. Update this README with the new skill
-
-## Integration with CodeRabbit
-
-The `.coderabbit.yaml` configuration:
-- References all skills in `knowledge_base.code_guidelines.filePatterns`
-- Maps file paths to relevant skills in `reviews.path_instructions`
-- Provides operator-specific review guidelines
-- Links to openshift/console repository for cross-repo context
-
-See `.github/CODERABBIT_SETUP.md` for complete integration documentation.
-
-## Common Review Patterns
-
-Based on recent PRs, watch for:
-- OwnerReference management (ensure only one controller=true)
-- Service account lifecycle and RBAC changes
-- API version bumps and deprecated field usage
-- Feature flag addition/removal
-- Manifest cluster profile annotations
-- Cleanup logic for Removed state
-- Status condition updates
-
-## Questions?
-
-See project documentation:
-- Main docs: `ARCHITECTURE.md`, `CONVENTIONS.md`, `TESTING.md`
-- Integration: `.github/CODERABBIT_SETUP.md`
-- Skills: Individual `.md` files in this directory
diff --git a/.claude/skills/e2e-test-create.md b/.claude/skills/e2e-test-create.md
index ba67600b4..aaa8a0370 100644
--- a/.claude/skills/e2e-test-create.md
+++ b/.claude/skills/e2e-test-create.md
@@ -33,7 +33,12 @@ func TestYourFeature(t *testing.T) {
     defer cancel()
 
     // Get initial operator config
-    operatorConfig := framework.GetOperatorConfig(t, client.OperatorClient)
+    operatorConfig, err := client.Operator.Consoles().Get(
+        ctx, api.ConfigResourceName, metav1.GetOptions{},
+    )
+    if err != nil {
+        t.Fatalf("failed to get operator config: %v", err)
+    }
     if operatorConfig.Spec.ManagementState != operatorv1.Managed {
         t.Skip("Skipping test - operator not in Managed state")
     }
@@ -65,7 +70,7 @@ func TestYourFeature(t *testing.T) {
 // Helper functions
 func updateOperatorConfig(
     ctx context.Context,
-    client *framework.Clientset,
+    client *framework.ClientSet,
     modify func(*operatorv1.Console),
 ) error {
     return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
@@ -86,7 +91,7 @@ func updateOperatorConfig(
 func waitForCondition(
     ctx context.Context,
     t *testing.T,
-    client *framework.Clientset,
+    client *framework.ClientSet,
 ) error {
     return wait.Poll(1*time.Second, 2*time.Minute, func() (bool, error) {
         // Check for expected condition
@@ -100,13 +105,45 @@ func waitForCondition(
     })
 }
 
-func cleanupResources(t *testing.T, client *framework.Clientset) {
+func cleanupResources(t *testing.T, client *framework.ClientSet) {
     ctx := context.Background()
+    
     // Clean up in reverse order of creation
-    // Ignore NotFound errors
+    // Example: if you created namespace -> deployment -> service -> configmap
+    // Delete in reverse: configmap -> service -> deployment -> namespace
+    
+    // Delete ConfigMap
+    err := client.KubeClient.CoreV1().ConfigMaps(api.OpenShiftConsoleNamespace).Delete(
+        ctx, "test-configmap", metav1.DeleteOptions{},
+    )
+    if err != nil && !apierrors.IsNotFound(err) {
+        t.Errorf("failed to delete configmap: %v", err)
+    } else if apierrors.IsNotFound(err) {
+        t.Logf("configmap already deleted or not found")
+    }
+    
+    // Delete Service
+    err = client.KubeClient.CoreV1().Services(api.OpenShiftConsoleNamespace).Delete(
+        ctx, "test-service", metav1.DeleteOptions{},
+    )
+    if err != nil && !apierrors.IsNotFound(err) {
+        t.Errorf("failed to delete service: %v", err)
+    } else if apierrors.IsNotFound(err) {
+        t.Logf("service already deleted or not found")
+    }
+    
+    // Delete Deployment
+    err = client.KubeClient.AppsV1().Deployments(api.OpenShiftConsoleNamespace).Delete(
+        ctx, "test-deployment", metav1.DeleteOptions{},
+    )
+    if err != nil && !apierrors.IsNotFound(err) {
+        t.Errorf("failed to delete deployment: %v", err)
+    } else if apierrors.IsNotFound(err) {
+        t.Logf("deployment already deleted or not found")
+    }
 }
 
-func verifyExpectedState(t *testing.T, client *framework.Clientset) {
+func verifyExpectedState(t *testing.T, client *framework.ClientSet) {
     // Make assertions about final state
 }
 ```
diff --git a/.claude/skills/e2e-test-review.md b/.claude/skills/e2e-test-review.md
index ab1dd00c4..57d593931 100644
--- a/.claude/skills/e2e-test-review.md
+++ b/.claude/skills/e2e-test-review.md
@@ -65,7 +65,7 @@ defer func() {
 ### 4. Wait Patterns
 Use `wait.Poll` for eventual consistency:
 ```go
-err := wait.Poll(1*time.Second, 5*time.Minute, func() (bool, error) {
+err := wait.Poll(1*time.Second, framework.AsyncOperationTimeout, func() (bool, error) {
     deployment, err := client.AppsV1().Deployments(ns).Get(
         ctx, name, metav1.GetOptions{},
     )
@@ -96,7 +96,7 @@ err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
 ### 6. Context Usage
 Always use context with timeout:
 ```go
-ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+ctx, cancel := context.WithTimeout(context.Background(), framework.AsyncOperationTimeout)
 defer cancel()
 ```
 
diff --git a/.claude/skills/go-quality-review.md b/.claude/skills/go-quality-review.md
index edf930efc..819f11789 100644
--- a/.claude/skills/go-quality-review.md
+++ b/.claude/skills/go-quality-review.md
@@ -230,7 +230,7 @@ result := builder.String()
 ```go
 // Bad - allocates on every call
 func (c *Controller) getNamespace() string {
-    return []byte("openshift-console")[0:] // Unnecessary allocation
+    return string([]byte("openshift-console")) // Unnecessary byte slice allocation
 }
 
 // Good - use constant
diff --git a/.claude/skills/manifest-review.md b/.claude/skills/manifest-review.md
index 9b2a03249..68f686381 100644
--- a/.claude/skills/manifest-review.md
+++ b/.claude/skills/manifest-review.md
@@ -6,7 +6,7 @@ tags: [review, manifest, rbac, cvo]
 
 # Manifest Review Skill
 
-Review Kubernetes manifest files in `manifests/` and `bindata/assets/` directories.
+Review Kubernetes manifest files in `manifests/`, `bindata/assets/`, `quickstarts/`, and `examples/` directories.
 
 ## What to Check
 
@@ -52,7 +52,17 @@ Check RBAC manifests for:
 - Include `---` separator between multiple resources in one file
 
 ### 6. Service Account References
-When binding roles to service accounts:
+When binding roles to service accounts, verify which SA is appropriate:
+
+**Operator SA (cross-namespace bindings):**
+```yaml
+subjects:
+  - kind: ServiceAccount
+    name: console-operator
+    namespace: openshift-console-operator
+```
+
+**Console workload SA (same-namespace bindings):**
 ```yaml
 subjects:
   - kind: ServiceAccount
@@ -60,6 +70,10 @@ subjects:
     namespace: openshift-console
 ```
 
+> Verify which SA is appropriate based on what the role grants access to:
+> use `console-operator` when the operator needs cross-namespace permissions,
+> and `console` when the binding is scoped to the console workload in `openshift-console`.
+
 ### 7. Profile-Specific Patches
 Check `profile-patches/` for overrides that should only apply to specific deployments.
 
diff --git a/.claude/skills/unit-test-review.md b/.claude/skills/unit-test-review.md
new file mode 100644
index 000000000..3c0f5ec35
--- /dev/null
+++ b/.claude/skills/unit-test-review.md
@@ -0,0 +1,301 @@
+---
+name: unit-test-review
+description: Review Go unit tests for best practices, table-driven patterns, and proper assertions
+tags: [review, testing, unit, go]
+---
+
+# Unit Test Review Skill
+
+Review unit tests in `pkg/**/*_test.go` files for correctness and best practices.
+
+## What to Check
+
+### 1. Table-Driven Test Pattern
+Most unit tests should use table-driven pattern for clarity and completeness:
+```go
+func TestFeature(t *testing.T) {
+    tests := []struct {
+        name     string
+        input    InputType
+        expected OutputType
+        wantErr  bool
+    }{
+        {
+            name:     "happy path",
+            input:    validInput,
+            expected: expectedOutput,
+            wantErr:  false,
+        },
+        {
+            name:     "error case",
+            input:    invalidInput,
+            expected: OutputType{},
+            wantErr:  true,
+        },
+    }
+    for _, tt := range tests {
+        t.Run(tt.name, func(t *testing.T) {
+            result, err := FunctionUnderTest(tt.input)
+            
+            if (err != nil) != tt.wantErr {
+                t.Errorf("expected error: %v, got: %v", tt.wantErr, err)
+            }
+            
+            if diff := deep.Equal(result, tt.expected); diff != nil {
+                t.Error(diff)
+            }
+        })
+    }
+}
+```
+
+### 2. Test Naming
+**Good test names:**
+- `TestGetNodeComputeEnvironments` - describes what function is tested
+- `TestNewRouteConfig` - clear and descriptive
+- Test case names: `"Custom hostname and TLS secret set"` - explains scenario
+
+**Bad test names:**
+- `TestFunc1` - not descriptive
+- `TestIt` - what is "it"?
+- Test case names: `"test1"` - doesn't explain what's being tested
+
+### 3. Deep Equality Checks
+Use `github.com/go-test/deep` for struct comparisons:
+```go
+// Good - shows exact differences
+if diff := deep.Equal(actual, expected); diff != nil {
+    t.Error(diff)
+}
+
+// Bad - unhelpful error message
+if actual != expected {
+    t.Error("mismatch")
+}
+
+// Bad - manual field-by-field comparison (fragile)
+if actual.Field1 != expected.Field1 || actual.Field2 != expected.Field2 {
+    t.Error("fields don't match")
+}
+```
+
+### 4. Test Coverage
+**Test both success and failure paths:**
+```go
+tests := []struct {
+    name    string
+    input   string
+    wantErr bool
+}{
+    {name: "valid input", input: "valid", wantErr: false},
+    {name: "empty input", input: "", wantErr: true},
+    {name: "invalid format", input: "bad", wantErr: true},
+}
+```
+
+**Edge cases to consider:**
+- Empty inputs (nil, "", empty slices/maps)
+- Boundary values (0, -1, max int)
+- Missing labels/fields
+- Duplicate values
+- Large inputs
+
+### 5. Test Structure (Arrange-Act-Assert)
+```go
+t.Run("test scenario", func(t *testing.T) {
+    // Arrange - setup test data
+    config := &operatorv1.Console{
+        Spec: operatorv1.ConsoleSpec{
+            Route: operatorv1.ConsoleConfigRoute{
+                Hostname: "custom.example.com",
+            },
+        },
+    }
+    
+    // Act - call the function
+    result := ProcessConfig(config)
+    
+    // Assert - verify results
+    if diff := deep.Equal(result, expected); diff != nil {
+        t.Error(diff)
+    }
+})
+```
+
+### 6. Error Handling
+**Check error presence:**
+```go
+// Good - checks if error occurred when expected
+if (err != nil) != tt.wantErr {
+    t.Errorf("wantErr %v, got error: %v", tt.wantErr, err)
+}
+
+// Bad - ignores error
+result, _ := FunctionUnderTest(input)
+```
+
+**Check error messages (when specific):**
+```go
+if err != nil && !strings.Contains(err.Error(), "expected substring") {
+    t.Errorf("unexpected error message: %v", err)
+}
+```
+
+### 7. Mocking and Test Isolation
+**Prefer interfaces for testability:**
+```go
+// Good - uses interface, easy to mock
+type ConfigGetter interface {
+    GetConfig() (*Config, error)
+}
+
+func ProcessData(getter ConfigGetter) error {
+    config, err := getter.GetConfig()
+    // ...
+}
+
+// Bad - hard-coded dependency
+func ProcessData() error {
+    config := getConfigFromKubernetes() // can't test without cluster
+    // ...
+}
+```
+
+**Keep tests isolated:**
+- Tests should not depend on execution order
+- Tests should not share mutable state
+- Each test case should be independent
+
+### 8. Helper Functions
+Extract common setup into helpers:
+```go
+func testConsole() *operatorv1.Console {
+    return &operatorv1.Console{
+        Spec: operatorv1.ConsoleSpec{
+            // common test config
+        },
+    }
+}
+
+func TestMultipleFunctions(t *testing.T) {
+    console := testConsole()
+    // modify as needed for specific test
+}
+```
+
+### 9. Assertion Quality
+**Good assertions:**
+```go
+// Specific error message with context
+if result != expected {
+    t.Errorf("expected %d nodes, got %d", expected, result)
+}
+
+// Shows what differs
+if diff := deep.Equal(actual, expected); diff != nil {
+    t.Errorf("config mismatch: %v", diff)
+}
+```
+
+**Bad assertions:**
+```go
+// Vague - what failed?
+if result != expected {
+    t.Error("failed")
+}
+
+// Silent failure
+if result != expected {
+    // no error reported
+}
+```
+
+### 10. Test Data Management
+**Inline simple data:**
+```go
+tests := []struct {
+    name  string
+    input int
+}{
+    {name: "zero", input: 0},
+    {name: "one", input: 1},
+}
+```
+
+**Extract complex fixtures:**
+```go
+// testdata/valid_config.yaml
+// or helper functions for complex objects
+func validConsoleConfig() *operatorv1.Console {
+    return &operatorv1.Console{
+        // complex config
+    }
+}
+```
+
+## Red Flags
+
+- No table-driven tests for functions with multiple scenarios
+- Tests without subtests (can't see which case failed)
+- Tests that depend on execution order
+- Global mutable state between tests
+- Hardcoded sleeps (use mocks/fakes instead)
+- Tests without assertions (just calling the function)
+- Ignoring errors with `_`
+- Testing implementation details instead of behavior
+- Overly complex test setup (refactor the code, not the test)
+
+## Common Patterns in This Codebase
+
+### Using go-test/deep
+```go
+import "github.com/go-test/deep"
+
+if diff := deep.Equal(actual, expected); diff != nil {
+    t.Error(diff)
+}
+```
+
+### Testing Functions That Return Errors
+```go
+result, err := FunctionUnderTest(input)
+
+if (err != nil) != tt.wantErr {
+    t.Errorf("expected error: %v, got: %v", tt.wantErr, err)
+    return
+}
+
+if !tt.wantErr && diff := deep.Equal(result, tt.expected); diff != nil {
+    t.Error(diff)
+}
+```
+
+### Testing with Kubernetes Objects
+```go
+node := &v1.Node{
+    ObjectMeta: metav1.ObjectMeta{
+        Name: "test-node",
+        Labels: map[string]string{
+            "key": "value",
+        },
+    },
+}
+```
+
+## Output Format
+
+For each issue:
+- **Test**: Test function name
+- **Issue**: What's wrong
+- **Impact**: Why it matters
+- **Fix**: How to improve
+
+## Example Review Comments
+
+**Critical**: TestProcessConfig has no assertions - test always passes even if function is broken.
+
+**Warning**: Using `==` to compare structs instead of `deep.Equal` - test won't show what differs.
+
+**Suggestion**: Consider adding test case for empty input to TestGetNodes - edge case not covered.
+
+**Info**: Good use of table-driven tests with clear scenario names.
diff --git a/.coderabbit.yaml b/.coderabbit.yaml
index 1a975d0d5..5a30c882f 100644
--- a/.coderabbit.yaml
+++ b/.coderabbit.yaml
@@ -9,6 +9,7 @@
 #   /controller-review - When code has Controller types, factory.New() patterns
 #   /sync-handler-review - When code has sequential sync logic with early returns
 #   /manifest-review - When YAML has RBAC kinds or missing annotations
+#   /unit-test-review - When pkg/**/*_test.go has table-driven tests, go-test/deep
 #   /e2e-test-review - When tests use framework, wait.Poll, retry patterns
 #   /e2e-test-create - Scaffold new e2e tests with best practices
 #   /go-quality-review - Always applied; checks deprecated APIs, code smells
@@ -28,7 +29,6 @@ reviews:
   review_details: true
   path_filters:
   - "!vendor/**"
-  - "!**/*_test.go"
   path_instructions:
     - path: "**/*.go"
       instructions: |
@@ -67,6 +67,15 @@ reviews:
       instructions: |
         Review test code for quality and patterns.
 
+        **Apply /unit-test-review when test is in pkg/**/*_test.go:**
+        - Table-driven test structure with test cases
+        - Use of `go-test/deep` for struct comparisons
+        - Test naming conventions (TestFunctionName)
+        - Error handling with `wantErr` pattern
+        - Edge case coverage (nil, empty, boundary values)
+        - Proper assertions with helpful error messages
+        - Test isolation (no shared mutable state)
+
         **Apply /e2e-test-review when test contains:**
         - `framework.MustNewClientset(t, nil)` or similar e2e framework usage
         - `wait.Poll` or `wait.PollImmediate` patterns
@@ -86,6 +95,8 @@ reviews:
         - Missing context timeouts
         - Vague error messages in assertions
         - Tests without table-driven structure when testing multiple cases
+        - Ignoring errors with `_`
+        - Tests without assertions
 
     - path: "**/*.yaml"
       instructions: |
diff --git a/.github/CODERABBIT_SETUP.md b/.github/CODERABBIT_SETUP.md
index fd5f52261..a8401e01a 100644
--- a/.github/CODERABBIT_SETUP.md
+++ b/.github/CODERABBIT_SETUP.md
@@ -17,14 +17,15 @@ Custom skills are defined in `.claude/skills/`:
 | `controller-review` | Review controller patterns | Reviewing controller implementations |
 | `sync-handler-review` | Review sync/reconciliation logic | Reviewing operator sync handlers |
 | `manifest-review` | Review RBAC/CVO manifests | Reviewing manifest changes |
-| `e2e-test-review` | Review e2e tests | Reviewing test code |
+| `unit-test-review` | Review unit tests | Reviewing pkg/**/*_test.go files |
+| `e2e-test-review` | Review e2e tests | Reviewing test/e2e/**/*_test.go files |
 | `e2e-test-create` | Scaffold new e2e tests | Creating new e2e tests |
 | `go-quality-review` | Review code quality | Checking for deprecated APIs, code smells |
 
 ### Using Skills
 
 In Claude Code CLI or IDE extensions:
-```
+```bash
 /controller-review
 /e2e-test-review
 ```
@@ -35,36 +36,39 @@ In PR reviews (CodeRabbit will reference these automatically based on code patte
 
 Configuration is in `.coderabbit.yaml`. It:
 - References project documentation (ARCHITECTURE.md, CONVENTIONS.md, etc.)
-- Triggers skills based on **code patterns** (function signatures, types, YAML kinds) not just file paths
+- Applies review guidelines from skills based on **code patterns** (function signatures, types, YAML kinds) not just file paths
 - Integrates with openshift/console repository context
 - Provides operator-specific review guidelines
 
-### Pattern-Based Skill Triggering
+> **Note**: CodeRabbit embeds skill content as reviewer instructions during PR review.
+> Skills are also usable interactively via Claude Code (e.g., `/controller-review`).
+
+### Pattern-Based Review Guidelines
 
-Skills are applied when specific code patterns are detected:
+Review guidelines from skills are applied when specific code patterns are detected:
 
-**controller-review** triggers when code contains:
+**controller-review** applies when code contains:
 - `type *Controller struct` definitions
 - `factory.New().WithFilteredEventsInformers()` pattern
 - `Sync(ctx context.Context, controllerContext factory.SyncContext)` methods
 - `status.NewStatusHandler` usage
 
-**sync-handler-review** triggers when code contains:
+**sync-handler-review** applies when code contains:
 - Sequential `resourceapply.Apply*()` calls with early returns
 - Feature gate conditional logic
 - Incremental reconciliation patterns
 
-**manifest-review** triggers when YAML contains:
+**manifest-review** applies when YAML contains:
 - `kind: Role` or `kind: ClusterRole`
 - Missing cluster profile annotations in `manifests/`
 - Wildcard permissions (`verbs: ["*"]`)
 
-**e2e-test-review** triggers when tests contain:
+**e2e-test-review** applies when tests contain:
 - `framework.MustNewClientset(t, nil)`
 - `wait.Poll` or `retry.RetryOnConflict`
 - Console/operator CR manipulations
 
-This approach is more reliable than path-based triggering, as it recognizes what the code actually does.
+This approach is more reliable than path-based matching, as it recognizes what the code actually does.
 
 ## CI/CD Integration
 

From 932ae124a2ecaeabd6070fec4125f60d45b54173 Mon Sep 17 00:00:00 2001
From: Jon Jackson <jonjacks@redhat.com>
Date: Fri, 1 May 2026 09:49:03 -0400
Subject: [PATCH 3/3] Fix e2e-test-create skill to use actual framework APIs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Corrected hallucinated APIs to match test/e2e/framework:
- client.OperatorClient → client.Operator
- client.KubeClient.AppsV1() → client.Apps
- client.KubeClient.CoreV1() → client.Core
- client.ConsoleClient.ConsoleV1() → client.ConsolePluginV1

Updated patterns to use framework.StandardSetup/StandardCleanup
instead of manual client setup. Added comprehensive Framework API
Reference documenting all ClientSet interfaces and helper functions.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
---
 .claude/skills/e2e-test-create.md | 191 +++++++++++++++---------------
 1 file changed, 97 insertions(+), 94 deletions(-)

diff --git a/.claude/skills/e2e-test-create.md b/.claude/skills/e2e-test-create.md
index aaa8a0370..acf81c2a6 100644
--- a/.claude/skills/e2e-test-create.md
+++ b/.claude/skills/e2e-test-create.md
@@ -27,44 +27,40 @@ import (
 )
 
 func TestYourFeature(t *testing.T) {
-    // Setup client
-    client := framework.MustNewClientset(t, nil)
-    ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+    // Setup client and get pristine operator config
+    client, operatorConfig := framework.StandardSetup(t)
+    defer framework.StandardCleanup(t, client)
+
+    ctx, cancel := context.WithTimeout(context.TODO(), 5*time.Minute)
     defer cancel()
 
-    // Get initial operator config
-    operatorConfig, err := client.Operator.Consoles().Get(
-        ctx, api.ConfigResourceName, metav1.GetOptions{},
-    )
-    if err != nil {
-        t.Fatalf("failed to get operator config: %v", err)
-    }
+    // Check management state
     if operatorConfig.Spec.ManagementState != operatorv1.Managed {
         t.Skip("Skipping test - operator not in Managed state")
     }
 
-    // Test implementation
-    t.Run("SubtestName", func(t *testing.T) {
-        // Setup test resources
-        defer cleanupResources(t, client)
-
-        // Perform operations
-        err := updateOperatorConfig(ctx, client, func(config *operatorv1.Console) {
-            // Modify config
-        })
-        if err != nil {
-            t.Fatalf("failed to update config: %v", err)
-        }
+    // Perform operations
+    err := updateOperatorConfig(ctx, client, func(config *operatorv1.Console) {
+        // Modify config
+    })
+    if err != nil {
+        t.Fatalf("failed to update config: %v", err)
+    }
 
-        // Wait for expected state
-        err = waitForCondition(ctx, t, client)
-        if err != nil {
-            t.Fatalf("expected condition not met: %v", err)
-        }
+    // Wait for expected state
+    err = waitForCondition(ctx, t, client)
+    if err != nil {
+        t.Fatalf("expected condition not met: %v", err)
+    }
 
-        // Verify result
-        verifyExpectedState(t, client)
-    })
+    // Verify result using framework helpers
+    deployment, err := framework.GetConsoleDeployment(client)
+    if err != nil {
+        t.Fatalf("failed to get deployment: %v", err)
+    }
+    if deployment.Status.ReadyReplicas == 0 {
+        t.Fatal("expected ready replicas > 0")
+    }
 }
 
 // Helper functions
@@ -74,14 +70,14 @@ func updateOperatorConfig(
     modify func(*operatorv1.Console),
 ) error {
     return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
-        config, err := client.OperatorClient.Consoles().Get(
+        config, err := client.Operator.Consoles().Get(
             ctx, api.ConfigResourceName, metav1.GetOptions{},
         )
         if err != nil {
             return err
         }
         modify(config)
-        _, err = client.OperatorClient.Consoles().Update(
+        _, err = client.Operator.Consoles().Update(
             ctx, config, metav1.UpdateOptions{},
         )
         return err
@@ -94,64 +90,23 @@ func waitForCondition(
     client *framework.ClientSet,
 ) error {
     return wait.Poll(1*time.Second, 2*time.Minute, func() (bool, error) {
-        // Check for expected condition
-        deployment, err := client.KubeClient.AppsV1().Deployments(
-            api.OpenShiftConsoleNamespace,
-        ).Get(ctx, api.OpenShiftConsoleName, metav1.GetOptions{})
+        // Use framework helper instead of manual client access
+        deployment, err := framework.GetConsoleDeployment(client)
         if err != nil {
             return false, err
         }
         return deployment.Status.ReadyReplicas > 0, nil
     })
 }
-
-func cleanupResources(t *testing.T, client *framework.ClientSet) {
-    ctx := context.Background()
-    
-    // Clean up in reverse order of creation
-    // Example: if you created namespace -> deployment -> service -> configmap
-    // Delete in reverse: configmap -> service -> deployment -> namespace
-    
-    // Delete ConfigMap
-    err := client.KubeClient.CoreV1().ConfigMaps(api.OpenShiftConsoleNamespace).Delete(
-        ctx, "test-configmap", metav1.DeleteOptions{},
-    )
-    if err != nil && !apierrors.IsNotFound(err) {
-        t.Errorf("failed to delete configmap: %v", err)
-    } else if apierrors.IsNotFound(err) {
-        t.Logf("configmap already deleted or not found")
-    }
-    
-    // Delete Service
-    err = client.KubeClient.CoreV1().Services(api.OpenShiftConsoleNamespace).Delete(
-        ctx, "test-service", metav1.DeleteOptions{},
-    )
-    if err != nil && !apierrors.IsNotFound(err) {
-        t.Errorf("failed to delete service: %v", err)
-    } else if apierrors.IsNotFound(err) {
-        t.Logf("service already deleted or not found")
-    }
-    
-    // Delete Deployment
-    err = client.KubeClient.AppsV1().Deployments(api.OpenShiftConsoleNamespace).Delete(
-        ctx, "test-deployment", metav1.DeleteOptions{},
-    )
-    if err != nil && !apierrors.IsNotFound(err) {
-        t.Errorf("failed to delete deployment: %v", err)
-    } else if apierrors.IsNotFound(err) {
-        t.Logf("deployment already deleted or not found")
-    }
-}
-
-func verifyExpectedState(t *testing.T, client *framework.ClientSet) {
-    // Make assertions about final state
-}
 ```
 
 ## Table-Driven Test Template
 
 ```go
 func TestMultipleScenarios(t *testing.T) {
+    client, _ := framework.StandardSetup(t)
+    defer framework.StandardCleanup(t, client)
+
     testCases := []struct {
         name          string
         setup         func(*operatorv1.Console)
@@ -176,8 +131,7 @@ func TestMultipleScenarios(t *testing.T) {
         },
     }
 
-    client := framework.MustNewClientset(t, nil)
-    ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
+    ctx, cancel := context.WithTimeout(context.TODO(), 10*time.Minute)
     defer cancel()
 
     for _, tc := range testCases {
@@ -187,7 +141,6 @@ func TestMultipleScenarios(t *testing.T) {
             if err != nil {
                 t.Fatalf("setup failed: %v", err)
             }
-            defer resetConfig(t, client)
 
             // Execute test
             result, err := performOperation(ctx, client)
@@ -211,8 +164,10 @@ func TestMultipleScenarios(t *testing.T) {
 
 ```go
 func TestConsolePlugin(t *testing.T) {
-    client := framework.MustNewClientset(t, nil)
-    ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+    client, _ := framework.StandardSetup(t)
+    defer framework.StandardCleanup(t, client)
+
+    ctx, cancel := context.WithTimeout(context.TODO(), 5*time.Minute)
     defer cancel()
 
     pluginName := "test-plugin"
@@ -233,7 +188,7 @@ func TestConsolePlugin(t *testing.T) {
         },
     }
 
-    _, err := client.ConsoleClient.ConsoleV1().ConsolePlugins().Create(
+    _, err := client.ConsolePluginV1.Create(
         ctx, plugin, metav1.CreateOptions{},
     )
     if err != nil {
@@ -241,7 +196,7 @@ func TestConsolePlugin(t *testing.T) {
     }
 
     defer func() {
-        err := client.ConsoleClient.ConsoleV1().ConsolePlugins().Delete(
+        err := client.ConsolePluginV1.Delete(
             ctx, pluginName, metav1.DeleteOptions{},
         )
         if err != nil {
@@ -249,11 +204,9 @@ func TestConsolePlugin(t *testing.T) {
         }
     }()
 
-    // Wait for plugin to be processed
+    // Wait for plugin to be processed using framework helper
     err = wait.Poll(1*time.Second, 1*time.Minute, func() (bool, error) {
-        deployment, err := client.KubeClient.AppsV1().Deployments(
-            api.OpenShiftConsoleNamespace,
-        ).Get(ctx, api.OpenShiftConsoleName, metav1.GetOptions{})
+        deployment, err := framework.GetConsoleDeployment(client)
         if err != nil {
             return false, err
         }
@@ -270,18 +223,17 @@ func TestConsolePlugin(t *testing.T) {
 
 When creating e2e tests:
 
-- [ ] Use `framework.MustNewClientset(t, nil)` for client setup
+- [ ] Use `framework.StandardSetup(t)` for test setup (not `MustNewClientset`)
+- [ ] Use `framework.StandardCleanup(t, client)` with defer for cleanup
 - [ ] Create context with timeout (`context.WithTimeout`)
-- [ ] Add cleanup with `defer`
 - [ ] Use `retry.RetryOnConflict` for config updates
 - [ ] Use `wait.Poll` for async operations (not `time.Sleep`)
+- [ ] Use framework helpers (`GetConsoleDeployment`, `GetConsoleConfigMap`, etc.)
 - [ ] Write helpful error messages with context
-- [ ] Add subtests for different scenarios
 - [ ] Test both success and failure paths
 - [ ] Verify operator responds to config changes
 - [ ] Check ClusterOperator status conditions
-- [ ] Clean up in reverse order of creation
-- [ ] Handle `IsNotFound` errors in cleanup
+- [ ] Handle `IsNotFound` errors in manual cleanup
 - [ ] Use table-driven tests for multiple similar cases
 
 ## Common Imports
@@ -304,6 +256,57 @@ import (
 )
 ```
 
+## Framework API Reference
+
+### Setup and Cleanup
+
+- `framework.StandardSetup(t)` - Returns `(*ClientSet, *operatorv1.Console)`. Sets up client and pristine operator config.
+- `framework.StandardCleanup(t, client)` - Restores operator to pristine state after test.
+
+### Resource Helpers
+
+Prefer these helpers over manual client access:
+
+- `framework.GetConsoleDeployment(client)` - Returns `(*appv1.Deployment, error)`
+- `framework.GetDownloadsDeployment(client)` - Returns `(*appv1.Deployment, error)`
+- `framework.GetConsoleConfigMap(client)` - Returns `(*corev1.ConfigMap, error)`
+- `framework.GetCustomLogoConfigMap(client, name)` - Returns `(*corev1.ConfigMap, error)`
+- `framework.GetConsoleService(client)` - Returns `(*corev1.Service, error)`
+- `framework.GetConsoleRoute(client)` - Returns `(*routev1.Route, error)`
+- `framework.GetConsoleCLIDownloads(client, name)` - Returns `(*consolev1.ConsoleCLIDownload, error)`
+- `framework.GetConsolePodDisruptionBudget(client, pdbName)` - Returns `(*policyv1.PodDisruptionBudget, error)`
+
+### ClientSet Interfaces
+
+When helper functions don't exist, use `framework.ClientSet` typed interfaces:
+
+**Core Kubernetes:**
+- `client.Core` - CoreV1 resources (ConfigMaps, Services, Secrets)
+- `client.Apps` - AppsV1 resources (Deployments, DaemonSets, StatefulSets)
+- `client.PodDisruptionBudget` - PodDisruptionBudgets
+
+**OpenShift Routing:**
+- `client.Routes` - OpenShift Routes
+
+**Console Operator:**
+- `client.Operator` - Console operator configs (operatorv1.Console)
+
+**Console Config Resources:**
+- `client.Console` - Cluster console configs (configv1.Console)
+- `client.ConsolePluginV1` - Console plugins
+- `client.ConsoleCliDownloads` - Console CLI downloads
+- `client.ConsoleLink` - Console links
+- `client.ConsoleNotification` - Console notifications
+- `client.ConsoleExternalLogLink` - External log links
+- `client.ConsoleYAMLSample` - YAML samples
+
+**Cluster Config:**
+- `client.ClusterOperator` - ClusterOperator status
+- `client.Infrastructure` - Cluster infrastructure config
+- `client.Proxy` - Cluster proxy config
+- `client.Ingress` - Cluster ingress config
+- `client.FeatureGate` - Feature gates
+
 ## Output
 
 Generate complete, runnable test code with: