chore: init variable manager template #662
Conversation
WalkthroughAdds deployment- and resource-level variable models, in-memory repositories (skeleton implementations), and pluggable variable managers (workspace, deployment, resource) for resolution. Removes older model Variable/ResourceVariable interfaces and deletes the resource variable repository test suite. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant WVM as WorkspaceVariableManager
participant DVM as DeploymentVariableManager
participant RVM as ResourceVariableManager
participant DRepo as DeploymentVariableRepository
participant RRepo as ResourceVariableRepository
Caller->>WVM: ResolveDeploymentVariables(ctx, resource, deploymentID)
note right of WVM: obtain keys via provided getKeys()
WVM->>WVM: for each key
alt try Resource manager first
WVM->>RVM: Resolve(ctx, resource, deploymentID, key)
RVM->>RRepo: GetByResourceIDAndKey(resourceID, key)
RRepo-->>RVM: *ResourceVariable
RVM->>RVM: variable.Resolve(ctx)
RVM-->>WVM: *string or nil / error
else try Deployment manager
WVM->>DVM: Resolve(ctx, resource, deploymentID, key)
DVM->>DRepo: GetByDeploymentIDAndKey(deploymentID, key)
DRepo-->>DVM: *DeploymentVariable
DVM->>DVM: variable.Resolve(ctx, resource)
DVM-->>WVM: *string or nil / error
end
alt resolved value
WVM->>WVM: store result[key]=value
else error
WVM-->>Caller: wrapped error (includes key/resource)
end
WVM-->>Caller: map[string]string
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
apps/workspace-engine/pkg/engine/variable/manager_deployment_variable.go (2)
9-19: Add doc comments for exported types/ctorsPlease add short package-style docs for DeploymentVariableManager and NewDeploymentVariableManager to clarify ownership and precedence when multiple managers are registered.
Example (non-functional guidance):
// DeploymentVariableManager resolves variables defined at the deployment scope. // It looks up variables by (deploymentID, key) and delegates value computation // to the underlying DeploymentVariable implementation.
21-35: *Prefer (string, bool, error) over string for presence semanticsReturning a pointer to string to signal “not found” forces double indirection across the codebase and invites pointer-to-interface patterns downstream. A more idiomatic API is (value string, ok bool, err error).
Minimal diff to show intent (interface change will ripple):
-type VariableManager interface { - Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error) -} +type VariableManager interface { + Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (string, bool, error) +}And here:
- resolved, err := deploymentVariable.Resolve(ctx, resource) - if err != nil { - return nil, fmt.Errorf("failed to resolve deployment variable for key: %s, deploymentID: %s, err: %w", key, deploymentID, err) - } - return &resolved, nil + resolved, err := deploymentVariable.Resolve(ctx, resource) + if err != nil { + return "", false, fmt.Errorf("failed to resolve deployment variable for key: %s, deploymentID: %s, err: %w", key, deploymentID, err) + } + return resolved, true, nilapps/workspace-engine/pkg/engine/variable/manager_resource_variable.go (1)
9-19: Add doc commentsPlease document ResourceVariableManager and its constructor, including how it participates in precedence with other VariableManager implementations.
apps/workspace-engine/pkg/engine/variable/manager_variable.go (3)
28-40: Guard against nil repository and pre-allocate keys capacityIf v.deploymentVariables is nil, getKeys will panic. Also, pre-allocating keys improves perf slightly.
func (v *WorkspaceVariableManager) getKeys(ctx context.Context, deploymentID string) []string { - deploymentVariables := v.deploymentVariables.GetAllByDeploymentID(ctx, deploymentID) - keys := make([]string, 0) + if v.deploymentVariables == nil { + return nil + } + deploymentVariables := v.deploymentVariables.GetAllByDeploymentID(ctx, deploymentID) + keys := make([]string, 0, len(deploymentVariables))
13-26: Document precedence and failure semanticsSince resolution stops at the first manager that returns a value, the order of managers defines precedence. Please document this on WorkspaceVariableManager and NewWorkspaceVariableManager so callers know how to construct the slice.
42-61: Consider parallelizing per-key resolution (optional)If keys/managers grow, serial resolution could become a bottleneck. An errgroup with bounded concurrency per key can keep latency predictable while retaining short-circuit-on-first-hit semantics.
If helpful, I can sketch a safe pattern respecting early-exit per key.
apps/workspace-engine/pkg/engine/variable/deployment_variable_repository.go (1)
19-26: Pointer-to-interface storage increases complexityvariables is map[string]*DeploymentVariable (pointer to interface). This creates double-nil states, forces awkward checks (ptr nil vs interface nil), and rarely adds value in Go. Prefer storing the interface value (map[string]DeploymentVariable) or, better, a concrete entity type that implements DeploymentVariable.
If generics in model.Repository require pointer returns, consider returning a concrete pointer type (e.g., *deploymentVariableImpl) instead of *DeploymentVariable.
apps/workspace-engine/pkg/engine/variable/resource_variable_repository.go (1)
18-25: Avoid pointer-to-interface in storageSimilar to the deployment repo, map[string]*ResourceVariable invites double-nil states and awkward checks. Prefer storing interface values or concrete types and returning concrete pointers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
apps/workspace-engine/pkg/engine/variable/deployment_variable_repository.go(1 hunks)apps/workspace-engine/pkg/engine/variable/manager_deployment_variable.go(1 hunks)apps/workspace-engine/pkg/engine/variable/manager_resource_variable.go(1 hunks)apps/workspace-engine/pkg/engine/variable/manager_variable.go(1 hunks)apps/workspace-engine/pkg/engine/variable/resource_variable_repository.go(1 hunks)apps/workspace-engine/pkg/engine/variable/resource_variable_repository_test.go(0 hunks)apps/workspace-engine/pkg/model/variable/resource_variable.go(0 hunks)apps/workspace-engine/pkg/model/variable/variable.go(0 hunks)
💤 Files with no reviewable changes (3)
- apps/workspace-engine/pkg/engine/variable/resource_variable_repository_test.go
- apps/workspace-engine/pkg/model/variable/variable.go
- apps/workspace-engine/pkg/model/variable/resource_variable.go
🧰 Additional context used
📓 Path-based instructions (1)
apps/workspace-engine/**/*.go
📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)
apps/workspace-engine/**/*.go: Do not add extraneous inline comments that state the obvious
Do not add comments that simply restate what the code does
Do not add comments for standard Go patterns (e.g., noting WaitGroup or semaphore usage)
Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods
Files:
apps/workspace-engine/pkg/engine/variable/manager_variable.goapps/workspace-engine/pkg/engine/variable/manager_deployment_variable.goapps/workspace-engine/pkg/engine/variable/manager_resource_variable.goapps/workspace-engine/pkg/engine/variable/deployment_variable_repository.goapps/workspace-engine/pkg/engine/variable/resource_variable_repository.go
🧬 Code graph analysis (1)
apps/workspace-engine/pkg/engine/variable/resource_variable_repository.go (1)
apps/workspace-engine/pkg/model/repository.go (1)
Repository(7-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint
- GitHub Check: Typecheck
🔇 Additional comments (1)
apps/workspace-engine/pkg/engine/variable/manager_deployment_variable.go (1)
21-35: Core resolve flow looks correctNil-safe fetch, deref, resolve, and error wrapping are all reasonable for the intended template.
| func (r *DeploymentVariableRepository) GetAll(ctx context.Context) []*DeploymentVariable { | ||
| r.mu.RLock() | ||
| defer r.mu.RUnlock() | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *DeploymentVariableRepository) GetAllByDeploymentID(ctx context.Context, deploymentID string) []*DeploymentVariable { | ||
| r.mu.RLock() | ||
| defer r.mu.RUnlock() | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *DeploymentVariableRepository) GetByDeploymentIDAndKey(ctx context.Context, deploymentID, key string) *DeploymentVariable { | ||
| r.mu.RLock() | ||
| defer r.mu.RUnlock() | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *DeploymentVariableRepository) Get(ctx context.Context, id string) *DeploymentVariable { | ||
| r.mu.RLock() | ||
| defer r.mu.RUnlock() | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *DeploymentVariableRepository) Create(ctx context.Context, variable *DeploymentVariable) error { | ||
| r.mu.Lock() | ||
| defer r.mu.Unlock() | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *DeploymentVariableRepository) Update(ctx context.Context, variable *DeploymentVariable) error { | ||
| r.mu.Lock() | ||
| defer r.mu.Unlock() | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *DeploymentVariableRepository) Delete(ctx context.Context, id string) error { | ||
| r.mu.Lock() | ||
| defer r.mu.Unlock() | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *DeploymentVariableRepository) Exists(ctx context.Context, id string) bool { | ||
| r.mu.RLock() | ||
| defer r.mu.RUnlock() | ||
|
|
||
| return false | ||
| } |
There was a problem hiding this comment.
Repository methods are no-ops; managers will never resolve anything
Every method currently returns zero values. As a result, getKeys returns no keys and Resolve paths will never find variables. Provide a minimal in-memory implementation to make the template usable and testable.
Apply a minimal implementation:
@@
-import (
- "context"
- "sync"
- "workspace-engine/pkg/model"
- "workspace-engine/pkg/model/resource"
-)
+import (
+ "context"
+ "fmt"
+ "sync"
+ "workspace-engine/pkg/model"
+ "workspace-engine/pkg/model/resource"
+)
@@
func (r *DeploymentVariableRepository) GetAll(ctx context.Context) []*DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
-
- return nil
+ out := make([]*DeploymentVariable, 0, len(r.variables))
+ for _, v := range r.variables {
+ out = append(out, v)
+ }
+ return out
}
@@
func (r *DeploymentVariableRepository) GetAllByDeploymentID(ctx context.Context, deploymentID string) []*DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
-
- return nil
+ out := make([]*DeploymentVariable, 0)
+ for _, vptr := range r.variables {
+ if vptr == nil || *vptr == nil {
+ continue
+ }
+ v := *vptr
+ if v.GetDeploymentID() == deploymentID {
+ out = append(out, vptr)
+ }
+ }
+ return out
}
@@
func (r *DeploymentVariableRepository) GetByDeploymentIDAndKey(ctx context.Context, deploymentID, key string) *DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
-
- return nil
+ for _, vptr := range r.variables {
+ if vptr == nil || *vptr == nil {
+ continue
+ }
+ v := *vptr
+ if v.GetDeploymentID() == deploymentID && v.GetKey() == key {
+ return vptr
+ }
+ }
+ return nil
}
@@
func (r *DeploymentVariableRepository) Get(ctx context.Context, id string) *DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
-
- return nil
+ return r.variables[id]
}
@@
func (r *DeploymentVariableRepository) Create(ctx context.Context, variable *DeploymentVariable) error {
r.mu.Lock()
defer r.mu.Unlock()
-
- return nil
+ if variable == nil || *variable == nil {
+ return fmt.Errorf("variable is nil")
+ }
+ id := (*variable).GetID()
+ if id == "" {
+ return fmt.Errorf("id cannot be empty")
+ }
+ if r.variables == nil {
+ r.variables = make(map[string]*DeploymentVariable)
+ }
+ if _, exists := r.variables[id]; exists {
+ return fmt.Errorf("deployment variable %q already exists", id)
+ }
+ r.variables[id] = variable
+ return nil
}
@@
func (r *DeploymentVariableRepository) Update(ctx context.Context, variable *DeploymentVariable) error {
r.mu.Lock()
defer r.mu.Unlock()
-
- return nil
+ if variable == nil || *variable == nil {
+ return fmt.Errorf("variable is nil")
+ }
+ id := (*variable).GetID()
+ if id == "" {
+ return fmt.Errorf("id cannot be empty")
+ }
+ r.variables[id] = variable
+ return nil
}
@@
func (r *DeploymentVariableRepository) Delete(ctx context.Context, id string) error {
r.mu.Lock()
defer r.mu.Unlock()
-
- return nil
+ delete(r.variables, id)
+ return nil
}
@@
func (r *DeploymentVariableRepository) Exists(ctx context.Context, id string) bool {
r.mu.RLock()
defer r.mu.RUnlock()
-
- return false
+ _, ok := r.variables[id]
+ return ok
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (r *DeploymentVariableRepository) GetAll(ctx context.Context) []*DeploymentVariable { | |
| r.mu.RLock() | |
| defer r.mu.RUnlock() | |
| return nil | |
| } | |
| func (r *DeploymentVariableRepository) GetAllByDeploymentID(ctx context.Context, deploymentID string) []*DeploymentVariable { | |
| r.mu.RLock() | |
| defer r.mu.RUnlock() | |
| return nil | |
| } | |
| func (r *DeploymentVariableRepository) GetByDeploymentIDAndKey(ctx context.Context, deploymentID, key string) *DeploymentVariable { | |
| r.mu.RLock() | |
| defer r.mu.RUnlock() | |
| return nil | |
| } | |
| func (r *DeploymentVariableRepository) Get(ctx context.Context, id string) *DeploymentVariable { | |
| r.mu.RLock() | |
| defer r.mu.RUnlock() | |
| return nil | |
| } | |
| func (r *DeploymentVariableRepository) Create(ctx context.Context, variable *DeploymentVariable) error { | |
| r.mu.Lock() | |
| defer r.mu.Unlock() | |
| return nil | |
| } | |
| func (r *DeploymentVariableRepository) Update(ctx context.Context, variable *DeploymentVariable) error { | |
| r.mu.Lock() | |
| defer r.mu.Unlock() | |
| return nil | |
| } | |
| func (r *DeploymentVariableRepository) Delete(ctx context.Context, id string) error { | |
| r.mu.Lock() | |
| defer r.mu.Unlock() | |
| return nil | |
| } | |
| func (r *DeploymentVariableRepository) Exists(ctx context.Context, id string) bool { | |
| r.mu.RLock() | |
| defer r.mu.RUnlock() | |
| return false | |
| } | |
| import ( | |
| "context" | |
| "fmt" | |
| "sync" | |
| "workspace-engine/pkg/model" | |
| "workspace-engine/pkg/model/resource" | |
| ) | |
| func (r *DeploymentVariableRepository) GetAll(ctx context.Context) []*DeploymentVariable { | |
| r.mu.RLock() | |
| defer r.mu.RUnlock() | |
| out := make([]*DeploymentVariable, 0, len(r.variables)) | |
| for _, v := range r.variables { | |
| out = append(out, v) | |
| } | |
| return out | |
| } | |
| func (r *DeploymentVariableRepository) GetAllByDeploymentID(ctx context.Context, deploymentID string) []*DeploymentVariable { | |
| r.mu.RLock() | |
| defer r.mu.RUnlock() | |
| out := make([]*DeploymentVariable, 0) | |
| for _, vptr := range r.variables { | |
| if vptr == nil || *vptr == nil { | |
| continue | |
| } | |
| v := *vptr | |
| if v.GetDeploymentID() == deploymentID { | |
| out = append(out, vptr) | |
| } | |
| } | |
| return out | |
| } | |
| func (r *DeploymentVariableRepository) GetByDeploymentIDAndKey(ctx context.Context, deploymentID, key string) *DeploymentVariable { | |
| r.mu.RLock() | |
| defer r.mu.RUnlock() | |
| for _, vptr := range r.variables { | |
| if vptr == nil || *vptr == nil { | |
| continue | |
| } | |
| v := *vptr | |
| if v.GetDeploymentID() == deploymentID && v.GetKey() == key { | |
| return vptr | |
| } | |
| } | |
| return nil | |
| } | |
| func (r *DeploymentVariableRepository) Get(ctx context.Context, id string) *DeploymentVariable { | |
| r.mu.RLock() | |
| defer r.mu.RUnlock() | |
| return r.variables[id] | |
| } | |
| func (r *DeploymentVariableRepository) Create(ctx context.Context, variable *DeploymentVariable) error { | |
| r.mu.Lock() | |
| defer r.mu.Unlock() | |
| if variable == nil || *variable == nil { | |
| return fmt.Errorf("variable is nil") | |
| } | |
| id := (*variable).GetID() | |
| if id == "" { | |
| return fmt.Errorf("id cannot be empty") | |
| } | |
| if r.variables == nil { | |
| r.variables = make(map[string]*DeploymentVariable) | |
| } | |
| if _, exists := r.variables[id]; exists { | |
| return fmt.Errorf("deployment variable %q already exists", id) | |
| } | |
| r.variables[id] = variable | |
| return nil | |
| } | |
| func (r *DeploymentVariableRepository) Update(ctx context.Context, variable *DeploymentVariable) error { | |
| r.mu.Lock() | |
| defer r.mu.Unlock() | |
| if variable == nil || *variable == nil { | |
| return fmt.Errorf("variable is nil") | |
| } | |
| id := (*variable).GetID() | |
| if id == "" { | |
| return fmt.Errorf("id cannot be empty") | |
| } | |
| r.variables[id] = variable | |
| return nil | |
| } | |
| func (r *DeploymentVariableRepository) Delete(ctx context.Context, id string) error { | |
| r.mu.Lock() | |
| defer r.mu.Unlock() | |
| delete(r.variables, id) | |
| return nil | |
| } | |
| func (r *DeploymentVariableRepository) Exists(ctx context.Context, id string) bool { | |
| r.mu.RLock() | |
| defer r.mu.RUnlock() | |
| _, ok := r.variables[id] | |
| return ok | |
| } |
🤖 Prompt for AI Agents
In apps/workspace-engine/pkg/engine/variable/deployment_variable_repository.go
around lines 28 to 82, all repository methods are stubs returning zero values;
implement a minimal in-memory backing store using the existing r.mu (read/write
mutex) and a map field (e.g., r.store map[string]*DeploymentVariable) so
managers can resolve variables. Implement GetAll to return all values,
GetAllByDeploymentID to filter by DeploymentID, GetByDeploymentIDAndKey to find
the first matching variable by deploymentID and key, Get to return the entry by
id, Create to add a new variable (error if id already exists), Update to replace
an existing variable (error if not found), Delete to remove by id, and Exists to
check presence; use RLock for read methods and Lock for write methods and ensure
any returned slices contain the stored pointers (or copies if desired) and that
Create/Update/Delete return appropriate errors.
| func (m *ResourceVariableManager) Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error) { | ||
| resourceVariablePtr := m.resourceVariables.GetByResourceIDAndKey(ctx, resource.GetID(), key) | ||
| if resourceVariablePtr == nil || *resourceVariablePtr == nil { | ||
| return nil, nil | ||
| } | ||
|
|
||
| resourceVariable := *resourceVariablePtr | ||
|
|
||
| resolved, err := resourceVariable.Resolve(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to resolve resource variable for key: %s, resource: %s, err: %w", key, resource.GetID(), err) | ||
| } | ||
|
|
||
| return &resolved, nil | ||
| } |
There was a problem hiding this comment.
Nil deref risk when resource is nil
Resolve dereferences resource twice: once to fetch resource.GetID() and again in the error message. This will panic if callers pass a nil resource.
Apply this guard and make the error path nil-safe:
func (m *ResourceVariableManager) Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error) {
- resourceVariablePtr := m.resourceVariables.GetByResourceIDAndKey(ctx, resource.GetID(), key)
+ if resource == nil {
+ return nil, fmt.Errorf("resource is nil while resolving key: %s", key)
+ }
+ resourceVariablePtr := m.resourceVariables.GetByResourceIDAndKey(ctx, resource.GetID(), key)
@@
- if err != nil {
- return nil, fmt.Errorf("failed to resolve resource variable for key: %s, resource: %s, err: %w", key, resource.GetID(), err)
- }
+ if err != nil {
+ return nil, fmt.Errorf("failed to resolve resource variable for key: %s, resource: %s, err: %w", key, resource.GetID(), err)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (m *ResourceVariableManager) Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error) { | |
| resourceVariablePtr := m.resourceVariables.GetByResourceIDAndKey(ctx, resource.GetID(), key) | |
| if resourceVariablePtr == nil || *resourceVariablePtr == nil { | |
| return nil, nil | |
| } | |
| resourceVariable := *resourceVariablePtr | |
| resolved, err := resourceVariable.Resolve(ctx) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to resolve resource variable for key: %s, resource: %s, err: %w", key, resource.GetID(), err) | |
| } | |
| return &resolved, nil | |
| } | |
| func (m *ResourceVariableManager) Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error) { | |
| if resource == nil { | |
| return nil, fmt.Errorf("resource is nil while resolving key: %s", key) | |
| } | |
| resourceVariablePtr := m.resourceVariables.GetByResourceIDAndKey(ctx, resource.GetID(), key) | |
| if resourceVariablePtr == nil || *resourceVariablePtr == nil { | |
| return nil, nil | |
| } | |
| resourceVariable := *resourceVariablePtr | |
| resolved, err := resourceVariable.Resolve(ctx) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to resolve resource variable for key: %s, resource: %s, err: %w", key, resource.GetID(), err) | |
| } | |
| return &resolved, nil | |
| } |
🤖 Prompt for AI Agents
In apps/workspace-engine/pkg/engine/variable/manager_resource_variable.go around
lines 21-35, the code dereferences resource (resource.GetID()) before checking
for nil which can panic; add a nil guard at the top (if resource == nil) and
handle that case immediately (either return a clear error or nil result as
appropriate for callers). Also capture resourceID into a local variable only
after confirming resource != nil and use that variable in the error message; if
you choose to keep an error path when resource is nil, ensure the error message
uses a safe placeholder like "<nil>" or the deploymentID instead of
dereferencing resource.
| if err != nil { | ||
| return nil, fmt.Errorf("failed to resolve for key: %s, resource: %s, err: %w", key, resource.GetID(), err) | ||
| } |
There was a problem hiding this comment.
Nil deref risk in error formatting
The error path unconditionally calls resource.GetID(). If Resolve was invoked with a nil resource, this will panic before the error can be returned.
Apply a nil-safe resource ID for error messages:
- return nil, fmt.Errorf("failed to resolve for key: %s, resource: %s, err: %w", key, resource.GetID(), err)
+ rid := "<nil>"
+ if resource != nil {
+ rid = resource.GetID()
+ }
+ return nil, fmt.Errorf("failed to resolve for key: %s, resource: %s, err: %w", key, rid, err)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if err != nil { | |
| return nil, fmt.Errorf("failed to resolve for key: %s, resource: %s, err: %w", key, resource.GetID(), err) | |
| } | |
| if err != nil { | |
| rid := "<nil>" | |
| if resource != nil { | |
| rid = resource.GetID() | |
| } | |
| return nil, fmt.Errorf("failed to resolve for key: %s, resource: %s, err: %w", key, rid, err) | |
| } |
🤖 Prompt for AI Agents
In apps/workspace-engine/pkg/engine/variable/manager_variable.go around lines 49
to 51, the error path unconditionally calls resource.GetID(), which will panic
if resource is nil; change the error construction to be nil-safe by computing an
ID string first (e.g. id := "<nil>"; if resource != nil { id = resource.GetID()
}) and then use that id in fmt.Errorf so the error path never dereferences a nil
resource.
| func (r *ResourceVariableRepository) GetAll(ctx context.Context) []*ResourceVariable { | ||
| r.mu.RLock() | ||
| defer r.mu.RUnlock() | ||
|
|
||
| var variables []*modelvariable.ResourceVariable | ||
| for _, resourceVariables := range r.variables { | ||
| for _, variable := range resourceVariables { | ||
| if variable == nil || *variable == nil { | ||
| continue | ||
| } | ||
|
|
||
| variableCopy := *variable | ||
| variables = append(variables, &variableCopy) | ||
| } | ||
| } | ||
|
|
||
| return variables | ||
| return nil | ||
| } | ||
|
|
||
| func (r *ResourceVariableRepository) Get(ctx context.Context, id string) *modelvariable.ResourceVariable { | ||
| func (r *ResourceVariableRepository) GetAllByResourceID(ctx context.Context, resourceID string) []*ResourceVariable { | ||
| r.mu.RLock() | ||
| defer r.mu.RUnlock() | ||
|
|
||
| for _, resourceVariables := range r.variables { | ||
| for _, variable := range resourceVariables { | ||
| if variable == nil || *variable == nil { | ||
| continue | ||
| } | ||
| variableCopy := *variable | ||
| if variableCopy.GetID() == id { | ||
| return &variableCopy | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *ResourceVariableRepository) GetAllByResourceID(ctx context.Context, resourceID string) []*modelvariable.ResourceVariable { | ||
| func (r *ResourceVariableRepository) GetByResourceIDAndKey(ctx context.Context, resourceID, key string) *ResourceVariable { | ||
| r.mu.RLock() | ||
| defer r.mu.RUnlock() | ||
|
|
||
| var variables []*modelvariable.ResourceVariable | ||
| if resourceVariables, ok := r.variables[resourceID]; ok { | ||
| for _, variable := range resourceVariables { | ||
| if variable == nil || *variable == nil { | ||
| continue | ||
| } | ||
| variableCopy := *variable | ||
| variables = append(variables, &variableCopy) | ||
| } | ||
| } | ||
|
|
||
| return variables | ||
| return nil | ||
| } | ||
|
|
||
| func (r *ResourceVariableRepository) GetByResourceIDAndKey(ctx context.Context, resourceID, key string) *modelvariable.ResourceVariable { | ||
| func (r *ResourceVariableRepository) Get(ctx context.Context, id string) *ResourceVariable { | ||
| r.mu.RLock() | ||
| defer r.mu.RUnlock() | ||
|
|
||
| if resourceVariables, ok := r.variables[resourceID]; ok { | ||
| if variable, ok := resourceVariables[key]; ok { | ||
| if variable == nil || *variable == nil { | ||
| return nil | ||
| } | ||
| variableCopy := *variable | ||
| return &variableCopy | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *ResourceVariableRepository) ensureVariableUniqueness(resourceID string, variable *modelvariable.ResourceVariable) error { | ||
| variableCopy := *variable | ||
| if _, ok := r.variables[resourceID][variableCopy.GetKey()]; ok { | ||
| return fmt.Errorf("variable already exists for resource %s and key %s", resourceID, variableCopy.GetKey()) | ||
| } | ||
|
|
||
| for _, resourceVariables := range r.variables { | ||
| for _, existingVariable := range resourceVariables { | ||
| if existingVariable == nil || *existingVariable == nil { | ||
| continue | ||
| } | ||
| existingVariableCopy := *existingVariable | ||
| if existingVariableCopy.GetID() == variableCopy.GetID() { | ||
| return fmt.Errorf("variable already exists with ID %s", variableCopy.GetID()) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *ResourceVariableRepository) Create(ctx context.Context, variable *modelvariable.ResourceVariable) error { | ||
| func (r *ResourceVariableRepository) Create(ctx context.Context, variable *ResourceVariable) error { | ||
| r.mu.Lock() | ||
| defer r.mu.Unlock() | ||
|
|
||
| if variable == nil || *variable == nil { | ||
| return errors.New("variable is nil") | ||
| } | ||
|
|
||
| variableCopy := *variable | ||
| resourceID := variableCopy.GetResourceID() | ||
| key := variableCopy.GetKey() | ||
|
|
||
| if _, ok := r.variables[resourceID]; !ok { | ||
| r.variables[resourceID] = make(map[string]*modelvariable.ResourceVariable) | ||
| } | ||
|
|
||
| if err := r.ensureVariableUniqueness(resourceID, variable); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| r.variables[resourceID][key] = &variableCopy | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *ResourceVariableRepository) Update(ctx context.Context, variable *modelvariable.ResourceVariable) error { | ||
| func (r *ResourceVariableRepository) Update(ctx context.Context, variable *ResourceVariable) error { | ||
| r.mu.Lock() | ||
| defer r.mu.Unlock() | ||
|
|
||
| if variable == nil || *variable == nil { | ||
| return errors.New("variable is nil") | ||
| } | ||
|
|
||
| variableCopy := *variable | ||
| resourceID := variableCopy.GetResourceID() | ||
| key := variableCopy.GetKey() | ||
|
|
||
| if _, ok := r.variables[resourceID]; !ok { | ||
| return errors.New("resource not found") | ||
| } | ||
|
|
||
| current, ok := r.variables[resourceID][key] | ||
| if !ok { | ||
| return errors.New("variable key not found") | ||
| } | ||
| currentCopy := *current | ||
| if currentCopy == nil { | ||
| return errors.New("current variable is nil") | ||
| } | ||
|
|
||
| if currentCopy.GetID() != variableCopy.GetID() { | ||
| return errors.New("variable ID mismatch") | ||
| } | ||
|
|
||
| r.variables[resourceID][key] = &variableCopy | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *ResourceVariableRepository) Delete(ctx context.Context, id string) error { | ||
| r.mu.Lock() | ||
| defer r.mu.Unlock() | ||
|
|
||
| for _, resourceVariables := range r.variables { | ||
| for _, variable := range resourceVariables { | ||
| if variable == nil || *variable == nil { | ||
| continue | ||
| } | ||
| variableCopy := *variable | ||
| if variableCopy.GetID() == id { | ||
| delete(resourceVariables, variableCopy.GetKey()) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (r *ResourceVariableRepository) Exists(ctx context.Context, id string) bool { | ||
| r.mu.RLock() | ||
| defer r.mu.RUnlock() | ||
|
|
||
| for _, resourceVariables := range r.variables { | ||
| for _, variable := range resourceVariables { | ||
| if variable == nil || *variable == nil { | ||
| continue | ||
| } | ||
| variableCopy := *variable | ||
| if variableCopy.GetID() == id { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
Repository is a stub; implement minimal in-memory behavior
As written, all methods return zero values; the ResourceVariableManager will never resolve anything. Provide a basic implementation to make the template functional.
Suggested minimal implementation:
@@
-import (
- "context"
- "sync"
- "workspace-engine/pkg/model"
-)
+import (
+ "context"
+ "fmt"
+ "sync"
+ "workspace-engine/pkg/model"
+)
@@
func (r *ResourceVariableRepository) GetAll(ctx context.Context) []*ResourceVariable {
r.mu.RLock()
defer r.mu.RUnlock()
-
- return nil
+ out := make([]*ResourceVariable, 0, len(r.variables))
+ for _, v := range r.variables {
+ out = append(out, v)
+ }
+ return out
}
@@
func (r *ResourceVariableRepository) GetAllByResourceID(ctx context.Context, resourceID string) []*ResourceVariable {
r.mu.RLock()
defer r.mu.RUnlock()
- return nil
+ out := make([]*ResourceVariable, 0)
+ for _, vptr := range r.variables {
+ if vptr == nil || *vptr == nil {
+ continue
+ }
+ v := *vptr
+ if v.GetResourceID() == resourceID {
+ out = append(out, vptr)
+ }
+ }
+ return out
}
@@
func (r *ResourceVariableRepository) GetByResourceIDAndKey(ctx context.Context, resourceID, key string) *ResourceVariable {
r.mu.RLock()
defer r.mu.RUnlock()
- return nil
+ for _, vptr := range r.variables {
+ if vptr == nil || *vptr == nil {
+ continue
+ }
+ v := *vptr
+ if v.GetResourceID() == resourceID && v.GetKey() == key {
+ return vptr
+ }
+ }
+ return nil
}
@@
func (r *ResourceVariableRepository) Get(ctx context.Context, id string) *ResourceVariable {
r.mu.RLock()
defer r.mu.RUnlock()
- return nil
+ return r.variables[id]
}
@@
func (r *ResourceVariableRepository) Create(ctx context.Context, variable *ResourceVariable) error {
r.mu.Lock()
defer r.mu.Unlock()
- return nil
+ if variable == nil || *variable == nil {
+ return fmt.Errorf("variable is nil")
+ }
+ id := (*variable).GetID()
+ if id == "" {
+ return fmt.Errorf("id cannot be empty")
+ }
+ if r.variables == nil {
+ r.variables = make(map[string]*ResourceVariable)
+ }
+ if _, exists := r.variables[id]; exists {
+ return fmt.Errorf("resource variable %q already exists", id)
+ }
+ r.variables[id] = variable
+ return nil
}
@@
func (r *ResourceVariableRepository) Update(ctx context.Context, variable *ResourceVariable) error {
r.mu.Lock()
defer r.mu.Unlock()
- return nil
+ if variable == nil || *variable == nil {
+ return fmt.Errorf("variable is nil")
+ }
+ id := (*variable).GetID()
+ if id == "" {
+ return fmt.Errorf("id cannot be empty")
+ }
+ r.variables[id] = variable
+ return nil
}
@@
func (r *ResourceVariableRepository) Delete(ctx context.Context, id string) error {
r.mu.Lock()
defer r.mu.Unlock()
- return nil
+ delete(r.variables, id)
+ return nil
}
@@
func (r *ResourceVariableRepository) Exists(ctx context.Context, id string) bool {
r.mu.RLock()
defer r.mu.RUnlock()
- return false
+ _, ok := r.variables[id]
+ return ok
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/workspace-engine/pkg/engine/variable/resource_variable_repository.go
around lines 27 to 81 the repository methods are stubs returning zero values;
implement a minimal in-memory store using a map and the existing mutex to make
the repository functional. Add a field like store map[string]*ResourceVariable
(keyed by ID) if not present, then implement: GetAll returns a slice of all
values; GetAllByResourceID filters by ResourceID; GetByResourceIDAndKey searches
store for matching resourceID and key and returns the first match; Get returns
store[id]; Create adds the variable to store (generate or validate ID) and
returns nil; Update replaces the entry if it exists (return error on missing if
desired) and Delete removes the key; Exists checks presence in the map. Use
r.mu.RLock/RUnlock for read paths and r.mu.Lock/Unlock for write paths as
already structured.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/workspace-engine/pkg/engine/variable/manager_variable.go (1)
35-37: Fix panic when resource is nil in error pathDereferencing resource.GetID() will panic when Resolve is called with a nil resource (workspace-level scenarios). Make the error construction nil-safe and include deploymentID for context.
- if err != nil { - return nil, fmt.Errorf("failed to resolve for key: %s, resource: %s, err: %w", key, resource.GetID(), err) - } + if err != nil { + rid := "<nil>" + if resource != nil { + rid = resource.GetID() + } + return nil, fmt.Errorf("failed to resolve for key: %s, deployment: %s, resource: %s, err: %w", key, deploymentID, rid, err) + }
🧹 Nitpick comments (6)
apps/workspace-engine/pkg/engine/variable/manager_variable.go (4)
22-25: Defensive copy of managers to prevent external mutationThe managers slice is stored by reference; callers can mutate it after construction. Make a defensive copy in the constructor.
return &WorkspaceVariableManager{ - getKeys: getKeys, - managers: managers, + getKeys: getKeys, + managers: append([]VariableManager(nil), managers...), }
31-31: Guard against nil getKeysThe struct is exported and can be instantiated without the constructor, making getKeys nil and causing a panic here. Add a simple guard.
- keys := v.getKeys(ctx, deploymentID) + if v.getKeys == nil { + return nil, fmt.Errorf("workspace variable manager misconfigured: getKeys is nil") + } + keys := v.getKeys(ctx, deploymentID)
9-11: Add doc comments for exported API and clarify manager precedencePublic symbols lack Go doc comments. Please add brief comments noting that managers are evaluated in order and the first non-nil value wins.
+// VariableManager resolves a value for a key within a deployment/resource context. +// Return a nil value (with nil error) to indicate "not handled". type VariableManager interface { Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error) } +// WorkspaceVariableManager fans out key resolution to a prioritized set of VariableManagers. type WorkspaceVariableManager struct { getKeys func(ctx context.Context, deploymentID string) []string managers []VariableManager } +// NewWorkspaceVariableManager constructs a manager; managers are evaluated in order. func NewWorkspaceVariableManager( getKeys func(ctx context.Context, deploymentID string) []string, managers []VariableManager, ) *WorkspaceVariableManager { return &WorkspaceVariableManager{ getKeys: getKeys, managers: managers, } } +// ResolveDeploymentVariables resolves all keys returned by getKeys. func (v *WorkspaceVariableManager) ResolveDeploymentVariables(ctx context.Context, resource *resource.Resource, deploymentID string) (map[string]string, error) {Also applies to: 13-16, 18-26, 28-28
31-44: Confirm behavior for unresolved keysIf no manager returns a value for a key, it’s omitted from the result. Is this intended, or should unresolved keys be returned with defaults or an explicit error/list? If needed, we can collect and return an "unresolved" slice for observability.
apps/workspace-engine/pkg/model/variable/deployment_variable.go (2)
10-18: Add concise doc comments for exported typesExported types lack documentation. Add minimal comments to satisfy linters and communicate intent (especially around selector semantics and sensitivity).
+// DeploymentVariable models a deployment-scoped variable and its candidate values. type DeploymentVariable struct { @@ +// DirectDeploymentVariableValue represents a literal value with optional resource selector. type DirectDeploymentVariableValue struct { @@ +// ReferenceDeploymentVariableValue represents an indirect value (reference + path) with optional selector. type ReferenceDeploymentVariableValue struct {Also applies to: 36-43, 76-83
69-74: Reduce duplication in Selector implementationsConsider factoring the entity-kind check into a small helper to keep both Selectors consistent and easy to maintain.
Example (outside selected lines, for illustration only):
func isResourceEntity(e model.MatchableEntity) bool { switch e.(type) { case resource.Resource, *resource.Resource: return true default: return false } }Then both Selectors can call isResourceEntity(entity).
Also applies to: 109-114
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/workspace-engine/pkg/engine/variable/manager_variable.go(1 hunks)apps/workspace-engine/pkg/model/variable/deployment_variable.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/workspace-engine/**/*.go
📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)
apps/workspace-engine/**/*.go: Do not add extraneous inline comments that state the obvious
Do not add comments that simply restate what the code does
Do not add comments for standard Go patterns (e.g., noting WaitGroup or semaphore usage)
Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods
Files:
apps/workspace-engine/pkg/engine/variable/manager_variable.goapps/workspace-engine/pkg/model/variable/deployment_variable.go
🧬 Code graph analysis (1)
apps/workspace-engine/pkg/model/variable/deployment_variable.go (2)
apps/workspace-engine/pkg/model/conditions/condition.go (1)
JSONCondition(53-59)apps/workspace-engine/pkg/model/types.go (1)
MatchableEntity(17-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Typecheck
- GitHub Check: Lint
🔇 Additional comments (1)
apps/workspace-engine/pkg/model/variable/deployment_variable.go (1)
65-67: Please confirm nil‐selector matching semantics across all SelectorEntity implementationsI noticed that for most entities (DirectDeploymentVariableValue, ReferenceDeploymentVariableValue, Environment, Deployment), MatchAllIfNullSelector always returns false, whereas PolicyTarget returns true when its ResourceSelector is nil. To ensure consistency and avoid surprises in the selector engine:
• Direct/ReferenceDeploymentVariableValue.MatchAllIfNullSelector (lines 65–67 and 105–107 of deployment_variable.go)
func (v *DirectDeploymentVariableValue) MatchAllIfNullSelector(entity model.MatchableEntity) bool { return false }• Environment.MatchAllIfNullSelector (environment.go:24–26) returns false
• Deployment.MatchAllIfNullSelector (deployment.go:28–30) returns false
• PolicyTarget.MatchAllIfNullSelector (policy_target.go:26–28) returns truePlease verify the intended contract:
- Should variable values (Direct/ReferenceDeploymentVariableValue) treat a nil ResourceSelector as “match none” (current) or “match all” (policy style)?
- If “match all” is desired, update MatchAllIfNullSelector to return
truewhenResourceSelector == nil.- Otherwise, consider adding comments to each implementation to document why semantics differ.
| func (v *DirectDeploymentVariableValue) Selector(entity model.MatchableEntity) (*conditions.JSONCondition, error) { | ||
| if _, ok := entity.(resource.Resource); ok { | ||
| return v.ResourceSelector, nil | ||
| } | ||
| return nil, fmt.Errorf("entity is not a supported selector option") | ||
| } |
There was a problem hiding this comment.
Selector rejects pointer-to-resource; support both pointer and value
Type assertion checks for resource.Resource (value). Call sites typically pass pointers (*resource.Resource), so this will incorrectly error. Support both forms.
-func (v *DirectDeploymentVariableValue) Selector(entity model.MatchableEntity) (*conditions.JSONCondition, error) {
- if _, ok := entity.(resource.Resource); ok {
- return v.ResourceSelector, nil
- }
- return nil, fmt.Errorf("entity is not a supported selector option")
-}
+func (v *DirectDeploymentVariableValue) Selector(entity model.MatchableEntity) (*conditions.JSONCondition, error) {
+ switch entity.(type) {
+ case resource.Resource, *resource.Resource:
+ return v.ResourceSelector, nil
+ default:
+ return nil, fmt.Errorf("entity is not a supported selector option")
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (v *DirectDeploymentVariableValue) Selector(entity model.MatchableEntity) (*conditions.JSONCondition, error) { | |
| if _, ok := entity.(resource.Resource); ok { | |
| return v.ResourceSelector, nil | |
| } | |
| return nil, fmt.Errorf("entity is not a supported selector option") | |
| } | |
| func (v *DirectDeploymentVariableValue) Selector(entity model.MatchableEntity) (*conditions.JSONCondition, error) { | |
| switch entity.(type) { | |
| case resource.Resource, *resource.Resource: | |
| return v.ResourceSelector, nil | |
| default: | |
| return nil, fmt.Errorf("entity is not a supported selector option") | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/workspace-engine/pkg/model/variable/deployment_variable.go around lines
69 to 74, the Selector method only type-asserts entity as the value type
resource.Resource so it fails when callers pass a pointer; update the check to
accept both resource.Resource and *resource.Resource (e.g., use a type switch or
two assertions for the value and pointer types) and return v.ResourceSelector
for either case, otherwise return the same error.
| func (v *ReferenceDeploymentVariableValue) Selector(entity model.MatchableEntity) (*conditions.JSONCondition, error) { | ||
| if _, ok := entity.(resource.Resource); ok { | ||
| return v.ResourceSelector, nil | ||
| } | ||
| return nil, fmt.Errorf("entity is not a supported selector option") | ||
| } |
There was a problem hiding this comment.
Same pointer vs value issue for ReferenceDeploymentVariableValue.Selector
Mirror the fix here to avoid false negatives when a pointer is provided.
-func (v *ReferenceDeploymentVariableValue) Selector(entity model.MatchableEntity) (*conditions.JSONCondition, error) {
- if _, ok := entity.(resource.Resource); ok {
- return v.ResourceSelector, nil
- }
- return nil, fmt.Errorf("entity is not a supported selector option")
-}
+func (v *ReferenceDeploymentVariableValue) Selector(entity model.MatchableEntity) (*conditions.JSONCondition, error) {
+ switch entity.(type) {
+ case resource.Resource, *resource.Resource:
+ return v.ResourceSelector, nil
+ default:
+ return nil, fmt.Errorf("entity is not a supported selector option")
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (v *ReferenceDeploymentVariableValue) Selector(entity model.MatchableEntity) (*conditions.JSONCondition, error) { | |
| if _, ok := entity.(resource.Resource); ok { | |
| return v.ResourceSelector, nil | |
| } | |
| return nil, fmt.Errorf("entity is not a supported selector option") | |
| } | |
| func (v *ReferenceDeploymentVariableValue) Selector(entity model.MatchableEntity) (*conditions.JSONCondition, error) { | |
| switch entity.(type) { | |
| case resource.Resource, *resource.Resource: | |
| return v.ResourceSelector, nil | |
| default: | |
| return nil, fmt.Errorf("entity is not a supported selector option") | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/workspace-engine/pkg/model/variable/deployment_variable.go around lines
109 to 114, the current type assertion only checks the dynamic type as-is and
returns false negatives when entity is a pointer to a type that implements
resource.Resource; change the check to detect and accept both pointer and value
receivers by using reflection to inspect and, if entity is a pointer,
dereference it and test whether the underlying value implements
resource.Resource (return v.ResourceSelector if so), otherwise return the
existing error.
Summary by CodeRabbit
New Features
Refactor
Tests