Skip to content

Comments

feat: Add Workbench and WorkbenchTool CRDs#3217

Open
maciaszczykm wants to merge 40 commits intomasterfrom
marcin/prod-4432-workbench-operator-support
Open

feat: Add Workbench and WorkbenchTool CRDs#3217
maciaszczykm wants to merge 40 commits intomasterfrom
marcin/prod-4432-workbench-operator-support

Conversation

@maciaszczykm
Copy link
Member

@maciaszczykm maciaszczykm commented Feb 13, 2026

Built based on the project controller logic.

I had to make workbench tool input schema required, otherwise API was returning input_schema can't be blank.

Test Plan

Checklist

  • If required, I have updated the Plural documentation accordingly.
  • I have added tests to cover my changes.
  • I have added a meaningful title and summary to convey the impact of this PR to a user.

Plural Flow: console

@linear
Copy link

linear bot commented Feb 13, 2026

@maciaszczykm maciaszczykm changed the title feat: Add Wokbench and WorkbenchTool CRDs feat: Add Workbench and WorkbenchTool CRDs Feb 13, 2026
@maciaszczykm maciaszczykm added enhancement New feature or request go Pull requests that update Go code labels Feb 13, 2026
@maciaszczykm maciaszczykm force-pushed the marcin/prod-4432-workbench-operator-support branch from 8c6c6ec to 267bece Compare February 18, 2026 10:24
@maciaszczykm maciaszczykm force-pushed the marcin/prod-4432-workbench-operator-support branch from 267bece to 198d0b6 Compare February 18, 2026 10:35
@maciaszczykm maciaszczykm marked this pull request as ready for review February 18, 2026 15:11
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 18, 2026

Greptile Summary

Adds two new Kubernetes CRDs — Workbench and WorkbenchTool — along with their reconcilers, console client methods, and GraphQL query updates. The implementation follows the established Project controller pattern for CRUD, finalizer management, read-only mode, and drift detection. The GetAgentRuntimeByName query now requires a clusterId parameter, and the workbench/tool getter queries accept optional id/name parameters to support name-based lookups from the controller.

  • Nil pointer dereference risk: WorkbenchToolHTTPConfig.Attributes() accesses c.InputSchema.Raw without checking if InputSchema is nil, which will panic at runtime if the pointer field is unset.
  • Test mock mismatch: The workbench controller test uses agentRuntimeName = "test-agent-runtime" but the controller expects "clusterHandle/runtimeName" format. The test also omits the GetClusterByHandle mock and provides the wrong argument count for GetAgentRuntime. The create test likely passes coincidentally because the agent runtime error is swallowed by the requeue handler.
  • Tag inconsistency: Method and InputSchema fields in WorkbenchToolHTTPConfig are annotated +kubebuilder:validation:Required but use omitempty in their JSON tags.

Confidence Score: 3/5

  • Generally safe structural addition, but contains a nil dereference bug and test gaps that should be addressed before merge.
  • The controllers and client code follow established patterns well and are structurally sound. However, the nil pointer dereference in InputSchema handling is a runtime crash risk, and the workbench controller test does not correctly exercise the agent runtime resolution path, masking potential integration issues.
  • go/controller/api/v1alpha1/workbenchtool_types.go (nil pointer dereference), go/controller/internal/controller/workbench_controller_test.go (incorrect mocks)

Important Files Changed

Filename Overview
go/controller/api/v1alpha1/workbench_types.go New Workbench CRD type definitions with spec, configuration, skills, and attribute conversion methods. Follows established patterns well.
go/controller/api/v1alpha1/workbenchtool_types.go New WorkbenchTool CRD type definitions. Has a nil pointer dereference risk in Attributes() when InputSchema is nil, and Required vs omitempty tag inconsistencies.
go/controller/internal/controller/workbench_controller.go New Workbench reconciler following established controller patterns (Project controller). Handles CRUD, finalizers, read-only mode, and references to tools/repos/runtimes.
go/controller/internal/controller/workbench_tool_controller.go New WorkbenchTool reconciler following the same patterns as the Workbench controller. Clean implementation with proper finalizer and sync logic.
go/controller/internal/controller/workbench_controller_test.go Workbench controller tests with incorrect agent runtime mock setup: wrong format for agentRuntimeName, missing GetClusterByHandle mock, and wrong argument count for GetAgentRuntime.
go/controller/internal/client/workbench.go Console client methods for Workbench CRUD operations. Standard implementation matching other resources.
go/controller/internal/client/workbenchtool.go Console client methods for WorkbenchTool CRUD operations. Standard implementation matching other resources.
go/controller/internal/client/agentruntime.go New GetAgentRuntime client method that fetches runtime by name and cluster ID with standard error handling.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Workbench CR created/updated] --> B[WorkbenchReconciler.Reconcile]
    B --> C{Already exists in API?}
    C -->|Yes, read-only| D[handleExistingWorkbench - sync ID]
    C -->|No| E[addOrRemoveFinalizer]
    E --> F{Being deleted?}
    F -->|Yes| G[Delete from Console API & remove finalizer]
    F -->|No| H[Compute SHA diff]
    H --> I[Resolve ProjectRef]
    I --> J[Resolve RepositoryRef]
    J --> K[Resolve AgentRuntime - clusterHandle/runtimeName]
    K --> L[Resolve WorkbenchTool refs - collect IDs]
    L --> M[sync - Create or Update via Console API]
    M --> N[Update status: ID, SHA, conditions]

    O[WorkbenchTool CR created/updated] --> P[WorkbenchToolReconciler.Reconcile]
    P --> Q{Already exists in API?}
    Q -->|Yes, read-only| R[handleExistingWorkbenchTool - sync ID]
    Q -->|No| S[addOrRemoveFinalizer]
    S --> T{Being deleted?}
    T -->|Yes| U[Delete from Console API & remove finalizer]
    T -->|No| V[Compute SHA diff]
    V --> W[Resolve ProjectRef]
    W --> X[sync - Create or Update via Console API]
    X --> Y[Update status: ID, SHA, conditions]

    L -.->|owner ref| O
Loading

Last reviewed commit: 03068f8

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

30 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

}
}),
Body: c.Body,
InputSchema: lo.ToPtr(string(c.InputSchema.Raw)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nil pointer dereference on InputSchema

InputSchema is a *runtime.RawExtension (pointer type). If it is nil, accessing c.InputSchema.Raw will panic at runtime. While the field is annotated +kubebuilder:validation:Required, the JSON tag uses omitempty, which creates an inconsistency — and the kubebuilder validation only applies at admission time, not when the struct is used programmatically (e.g., in tests or internal logic).

Add a nil guard before accessing .Raw:

Suggested change
InputSchema: lo.ToPtr(string(c.InputSchema.Raw)),
InputSchema: lo.Ternary(c.InputSchema != nil, lo.ToPtr(string(c.InputSchema.Raw)), nil),

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recheck.

workbenchName = "test"
namespace = "default"
id = "123"
agentRuntimeName = "test-agent-runtime"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agent runtime test format mismatch

agentRuntimeName is set to "test-agent-runtime", but the controller's handleAgentRuntime method (workbench_controller.go:288-291) expects the format "clusterHandle/runtimeName" and splits on "/". With this value, strings.Split produces a single-element slice, causing the controller to return fmt.Errorf("invalid agent runtime reference: %s", ref).

Additionally, the controller calls GetClusterByHandle before GetAgentRuntime, but the test doesn't mock GetClusterByHandle. And the GetAgentRuntime mock on line 156 only provides 2 matcher arguments (mock.Anything, agentRuntimeName) while the actual signature requires 3 (ctx, name, clusterId).

The test likely passes only because the reconcile fails at the agent runtime step and the error is handled by common.HandleRequeue (which sets a condition and returns a requeue result without propagating the error). This means the "create" path assertion succeeds coincidentally — the workbench is created without an agent runtime, which may not be the intended behavior to test.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment on lines 162 to 164
// +kubebuilder:validation:Required
// +kubebuilder:validation:Enum:=GET;POST;PUT;DELETE;PATCH
Method console.WorkbenchToolHTTPMethod `json:"method,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required field uses omitempty JSON tag

Method is annotated +kubebuilder:validation:Required but the JSON struct tag is json:"method,omitempty". For a required field, the tag should be json:"method" (without omitempty) to ensure it is always serialized. The same inconsistency applies to the InputSchema field on line 177.

Suggested change
// +kubebuilder:validation:Required
// +kubebuilder:validation:Enum:=GET;POST;PUT;DELETE;PATCH
Method console.WorkbenchToolHTTPMethod `json:"method,omitempty"`
Method console.WorkbenchToolHTTPMethod `json:"method"`

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for Workbench and WorkbenchTool CRDs to the Plural Console controller. These CRDs enable users to define AI workbenches with associated tools, system prompts, configuration, and skills. The implementation follows the existing controller patterns in the codebase and includes complete CRUD operations with drift detection support.

Changes:

  • Added Workbench and WorkbenchTool CRD definitions with comprehensive configuration options for AI agent runs
  • Implemented reconcilers for both resources following established patterns (read-only mode, finalizers, drift detection)
  • Extended Console API client with workbench and workbench tool operations, plus agent runtime retrieval

Reviewed changes

Copilot reviewed 25 out of 30 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
go/controller/api/v1alpha1/workbench_types.go Defines Workbench CRD structure with system prompt, configuration, skills, and tool references
go/controller/api/v1alpha1/workbenchtool_types.go Defines WorkbenchTool CRD for HTTP tools with input schema, headers, and methods
go/controller/internal/controller/workbench_controller.go Implements reconciliation logic for Workbench resources, handling project/repository/agent runtime refs
go/controller/internal/controller/workbench_tool_controller.go Implements reconciliation logic for WorkbenchTool resources
go/controller/internal/client/workbench.go Client methods for Workbench CRUD operations
go/controller/internal/client/workbenchtool.go Client methods for WorkbenchTool CRUD operations
go/controller/internal/client/agentruntime.go Adds GetAgentRuntime method to fetch agent runtime by name and cluster ID
go/controller/internal/test/mocks/ConsoleClient_mock.go Auto-generated mock updates for new console client methods
go/controller/Makefile Modified test command - appears to be debugging code
go/client/graph/*.graphql Updated GraphQL queries to accept both ID and name parameters
go/client/client.go Updated generated GraphQL client code
config/crd/bases/*.yaml Generated CRD manifests for Workbench and WorkbenchTool
config/rbac/role.yaml Added RBAC permissions for new resources
cmd/register.go Registered both new reconcilers

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test: TOOL = envtest controller-gen mockery
test: --tool manifests generate genmock fmt vet ## run tests
@KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(BINARIES_DIR) -p path)" go test -p 1 $$(go list ./... | grep -vE '/e2e|/benchmark') -v
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(BINARIES_DIR) -p path)" echo ${KUBEBUILDER_ASSETS}
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test command has been replaced with an echo statement, which means tests are no longer being executed. This appears to be a debugging change that was accidentally committed. The original command should be restored to ensure tests run properly.

Suggested change
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(BINARIES_DIR) -p path)" echo ${KUBEBUILDER_ASSETS}
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(BINARIES_DIR) -p path)" go test ./...

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant