refactor: strictly type job agent configs and use oapi discriminator #730
Conversation
WalkthroughReplace untyped job-agent config maps with discriminated OpenAPI union schemas and generated Go union types; propagate typed Deployment/Full/JobAgent configs through API, DB scanning, merge logic, dispatcher routing, diffing, forms, and many tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as "API (typed OpenAPI)"
participant DB
participant Factory
participant Dispatcher
participant Ext as "External Service (GH/Argo/TF)"
Client->>API: Create/Upsert Deployment or Job (typed JobAgentConfig)
API->>DB: Persist typed config (JSON)
DB-->>Factory: Read Deployment + JobAgent defaults
Factory->>Factory: mergeJobAgentConfig (JSON/map deep-merge, inject discriminator)
Factory->>DB: Create Job with FullJobAgentConfig
Factory->>Dispatcher: Enqueue/Dispatch Job
Dispatcher->>Dispatcher: discriminator := Job.JobAgentConfig.Discriminator()
alt github-app
Dispatcher->>Ext: Dispatch GitHub workflow (owner/repo/workflowId/ref)
else argo-cd
Dispatcher->>Ext: Create/Sync ArgoCD application (template/serverUrl)
else tfe
Dispatcher->>Ext: Create Terraform Cloud run
else test-runner
Dispatcher->>Dispatcher: Run local test-runner flow
else custom
Dispatcher->>Dispatcher: Passthrough / no-op
end
Dispatcher->>DB: Emit JobUpdateEvent (FieldsToUpdate)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (15)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go (2)
63-72: Minor: Redundant type assertion formap[string]interface{}.In Go 1.18+,
anyis an alias forinterface{}, somap[string]anyandmap[string]interface{}are the same type. The second branch (lines 66-72) is unreachable.toMap := func(v any) (map[string]any, error) { if v == nil { return map[string]any{}, nil } if m, ok := v.(map[string]any); ok { return m, nil } - if m, ok := v.(map[string]interface{}); ok { - out := make(map[string]any, len(m)) - for k, vv := range m { - out[k] = vv - } - return out, nil - } b, err := json.Marshal(v)
206-209: Misleading log message.The message "Applied default job agent configuration" is inaccurate since
mergeJobAgentConfigmerges agent defaults with deployment overrides. Consider updating to reflect the actual behavior.if action != nil { - configMsg := "Applied default job agent configuration" - - action.AddStep("Configure job", trace.StepResultPass, configMsg) + action.AddStep("Configure job", trace.StepResultPass, "Merged job agent configuration with deployment overrides") }apps/workspace-engine/test/e2e/engine_jobs_test.go (1)
562-572: Consider extracting empty config check to a helper.The JSON marshal/unmarshal pattern for verifying an empty config is correct but verbose. If this pattern is needed in multiple tests, consider adding a test helper like
assertEmptyJobAgentConfig(t, cfg).// Example helper that could be added to test utilities: func assertEmptyJobAgentConfig(t *testing.T, cfg oapi.FullJobAgentConfig) { t.Helper() b, err := cfg.MarshalJSON() if err != nil { t.Fatalf("failed to marshal job agent config: %v", err) } var m map[string]any if err := json.Unmarshal(b, &m); err != nil { t.Fatalf("failed to unmarshal job agent config: %v", err) } if len(m) != 0 { t.Errorf("expected empty job agent config, got %v", m) } }apps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.go (1)
15-30: Consider consolidating duplicate test helpers.This helper function creates typed DeploymentJobAgentConfig objects via JSON round-tripping, setting the discriminator to "custom". The implementation is correct, but similar helpers appear in multiple test files across this PR:
customJobAgentConfigin this filecustomJobAgentConfiginpkg/db/persistence/store_test.gocreateJobAgentConfiginpkg/workspace/store/diffcheck/deployment_test.goConsider consolidating these into a shared test utility package (e.g.,
pkg/db/common.goortest/helpers/) to reduce duplication and ensure consistency. The existing pattern inapps/workspace-engine/pkg/db/common.gocould be expanded or made exported for broader test usage.apps/workspace-engine/pkg/workspace/store/diffcheck/deployment_test.go (1)
12-25: Well-implemented helper with better error handling.This helper correctly constructs typed
DeploymentJobAgentConfigobjects and usest.Fatalf()for error reporting, which is more idiomatic for Go test helpers thanpanic(). However, this creates an inconsistency with similar helpers in other test files:
- This file: uses
t.Fatalf()incremental_test.goandstore_test.go: usepanic(err)Consider standardizing error handling across all similar test helpers. The
t.Fatalf()approach used here is generally preferred for test helpers as it provides clearer test failure messages.apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_test.go (1)
65-66: Ignored unmarshal error could hide test setup failures.The error from
UnmarshalJSONis silently discarded. If the JSON is malformed or the type discriminator is invalid, the test would proceed with a zero-value config, potentially causing confusing test failures.Consider panicking on error to fail fast during test setup:
customJobAgentConfig := oapi.DeploymentJobAgentConfig{} - _ = customJobAgentConfig.UnmarshalJSON([]byte(`{"type": "custom"}`)) + if err := customJobAgentConfig.UnmarshalJSON([]byte(`{"type": "custom"}`)); err != nil { + panic(fmt.Sprintf("failed to create test job agent config: %v", err)) + }Based on learnings, the user prefers simplicity in test code, so this is optional.
apps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_test.go (2)
171-175: Function name implies panic behavior but errors are ignored.The
mustprefix in Go conventionally indicates the function will panic on error. However, the error fromUnmarshalJSONis silently ignored. This could mask test setup issues.Align behavior with the naming convention:
func mustCreateDeploymentJobAgentConfig() oapi.DeploymentJobAgentConfig { jobAgentConfig := oapi.DeploymentJobAgentConfig{} - _ = jobAgentConfig.UnmarshalJSON([]byte(`{"type": "custom"}`)) + if err := jobAgentConfig.UnmarshalJSON([]byte(`{"type": "custom"}`)); err != nil { + panic(fmt.Sprintf("mustCreateDeploymentJobAgentConfig: %v", err)) + } return jobAgentConfig }Based on learnings, the user prefers simplicity in test code, so this is optional.
45-46: Same ignored error issue in setupStoreWithDeployment.The helper also ignores the unmarshal error. Consider handling it consistently with
mustCreateDeploymentJobAgentConfigif you choose to update that helper.apps/workspace-engine/pkg/db/job_agents.go (1)
80-117: Good backwards-compatible type inference with one minor error-handling nit.The logic for inferring the type discriminator from
jobAgent.Typewhen not present in the config is a solid approach for handling legacy data.However, at line 113, if
cfg.UnmarshalJSON(b2)fails (err2), the function returns the originalerrinstead oferr2, which could make debugging confusing since the error message won't match the actual failure.if err2 := cfg.UnmarshalJSON(b2); err2 != nil { - return nil, err + return nil, err2 }apps/workspace-engine/pkg/selector/match_test.go (1)
55-72: Consider reusing the existing helper fromcreatorspackage.This helper duplicates
c.CustomDeploymentJobAgentConfigfromworkspace-engine/test/integration/creators/deployment.go(lines 10-27). Consider importing and reusing it to reduce duplication:-func customJobAgentConfig(m map[string]interface{}) oapi.DeploymentJobAgentConfig { - payload := map[string]interface{}{} - for k, v := range m { - payload[k] = v - } - payload["type"] = "custom" - - b, err := json.Marshal(payload) - if err != nil { - panic(err) - } - - var cfg oapi.DeploymentJobAgentConfig - if err := cfg.UnmarshalJSON(b); err != nil { - panic(err) - } - return cfg -}Then use
c.CustomDeploymentJobAgentConfig(...)with an import alias for the creators package.apps/workspace-engine/pkg/workspace/store/diffcheck/deployment.go (1)
115-133: Early returns may mask other field changes on JSON errors.If
MarshalJSONorUnmarshalfails forJobAgentConfig, the function returns immediately with only the changes detected so far, potentially missing changes toResourceSelector(checked on line 148). Consider continuing comparison even if JobAgentConfig comparison fails:oldJobAgentConfigJSON, err := old.JobAgentConfig.MarshalJSON() - if err != nil { - return changed - } newJobAgentConfigJSON, err := new.JobAgentConfig.MarshalJSON() - if err != nil { - return changed - } - - var oldJobAgentConfigMap map[string]any - err = json.Unmarshal(oldJobAgentConfigJSON, &oldJobAgentConfigMap) - if err != nil { - return changed - } - var newJobAgentConfigMap map[string]any - err = json.Unmarshal(newJobAgentConfigJSON, &newJobAgentConfigMap) - if err != nil { - return changed + if err == nil { + // ... JobAgentConfig comparison logic ... } + + // Continue to ResourceSelector comparison regardlessThis would ensure
ResourceSelectorchanges are always detected.apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.go (1)
45-46: Consider extracting config creation to a test helper.The pattern of creating typed job agent configs via JSON unmarshalling is repeated many times throughout this test file. Extracting this to a helper function (similar to
createJobAgentConfigin testrunner_test.go) would reduce duplication and improve maintainability.Example helper:
func createCustomJobAgentConfig(t *testing.T) oapi.FullJobAgentConfig { cfg := oapi.FullJobAgentConfig{} if err := cfg.UnmarshalJSON([]byte(`{"type": "custom"}`)); err != nil { t.Fatalf("Failed to create job agent config: %v", err) } return cfg }This would also provide better error messages if the unmarshalling fails, rather than silently ignoring errors with
_ =.Based on coding guidelines, which recommend following existing test structure.
Also applies to: 187-188
apps/workspace-engine/pkg/workspace/jobdispatch/terraformcloud.go (1)
354-361: Consider the explicit FieldsToUpdate construction.The explicit construction of the
FieldsToUpdateslice is more verbose but clearer than a composite literal. However, this could be extracted to a helper function if this pattern is repeated in other dispatchers.If this pattern appears in multiple dispatchers, consider:
func allJobUpdateFields() []oapi.JobUpdateEventFieldsToUpdate { return []oapi.JobUpdateEventFieldsToUpdate{ oapi.JobUpdateEventFieldsToUpdateStatus, oapi.JobUpdateEventFieldsToUpdateMetadata, oapi.JobUpdateEventFieldsToUpdateCompletedAt, oapi.JobUpdateEventFieldsToUpdateUpdatedAt, } }apps/workspace-engine/pkg/workspace/jobdispatch/github_test.go (1)
57-72: Consider consistency in config creation pattern.These tests create a struct first, then marshal to JSON, then unmarshal to the union type. This differs from other test files (like jobtracker_test.go) which unmarshal directly from a JSON string. While both approaches work, using a consistent pattern across the codebase improves maintainability.
Consider creating a helper similar to testrunner_test.go:
func createGithubJobAgentConfig(t *testing.T, cfg oapi.FullGithubJobAgentConfig) oapi.FullJobAgentConfig { configJSON, err := json.Marshal(cfg) if err != nil { t.Fatalf("Failed to marshal github job agent config: %v", err) } config := oapi.FullJobAgentConfig{} if err := config.UnmarshalJSON(configJSON); err != nil { t.Fatalf("Failed to unmarshal github job agent config: %v", err) } return config }Based on coding guidelines, which recommend following existing test structure.
Also applies to: 123-138, 168-182
apps/workspace-engine/test/e2e/engine_deployment_test.go (1)
370-381: Config update + retriggering flow is well covered; consider a tiny helper for map assertionsThe test correctly:
- Reads typed deployment/job configs via the new
As*CustomJobAgentConfighelpers,- Marks the first job successful with the new
JobUpdateEventFieldsToUpdate*constants, and- Uses
CustomDeploymentJobAgentConfigwhen updating the deployment so the new pending job reflects the updatednamespace/timeout.If you find yourself adding more assertions on
AdditionalProperties["namespace"]/["timeout"]elsewhere, a small helper in the test package to assert these keys would reduce duplication, but it’s not required here.Also applies to: 394-406, 419-421, 428-431, 451-462
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (60)
apps/workspace-engine/oapi/openapi.json(13 hunks)apps/workspace-engine/oapi/spec/schemas/deployments.jsonnet(2 hunks)apps/workspace-engine/oapi/spec/schemas/entities.jsonnet(1 hunks)apps/workspace-engine/oapi/spec/schemas/jobs.jsonnet(2 hunks)apps/workspace-engine/pkg/db/common.go(1 hunks)apps/workspace-engine/pkg/db/deployment_variables_test.go(4 hunks)apps/workspace-engine/pkg/db/deployment_versions_test.go(4 hunks)apps/workspace-engine/pkg/db/deployments_test.go(10 hunks)apps/workspace-engine/pkg/db/job_agents.go(3 hunks)apps/workspace-engine/pkg/db/job_agents_test.go(10 hunks)apps/workspace-engine/pkg/db/persistence/store_test.go(9 hunks)apps/workspace-engine/pkg/db/release_targets_test.go(1 hunks)apps/workspace-engine/pkg/db/releases_test.go(1 hunks)apps/workspace-engine/pkg/events/handler/deployment/deployment.go(1 hunks)apps/workspace-engine/pkg/events/handler/resources/resourceproviders_bench_test.go(2 hunks)apps/workspace-engine/pkg/oapi/oapi.gen.go(16 hunks)apps/workspace-engine/pkg/selector/match_test.go(7 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/argocd.go(4 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/github.go(2 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/github_test.go(8 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/terraformcloud.go(4 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/testrunner.go(4 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.go(8 hunks)apps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.go(9 hunks)apps/workspace-engine/pkg/workspace/relationships/compute/rule_bench_test.go(3 hunks)apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.go(12 hunks)apps/workspace-engine/pkg/workspace/relationships/matcher_test.go(3 hunks)apps/workspace-engine/pkg/workspace/relationships/property.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor_test.go(2 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.go(4 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/dispatcher.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go(6 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_bench_test.go(2 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_test.go(2 hunks)apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.go(20 hunks)apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.go(40 hunks)apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.go(14 hunks)apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.go(19 hunks)apps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.go(6 hunks)apps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_bench_test.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_test.go(13 hunks)apps/workspace-engine/pkg/workspace/store/diffcheck/deployment.go(3 hunks)apps/workspace-engine/pkg/workspace/store/diffcheck/deployment_test.go(29 hunks)apps/workspace-engine/test/e2e/engine_bench_test.go(2 hunks)apps/workspace-engine/test/e2e/engine_deployment_test.go(6 hunks)apps/workspace-engine/test/e2e/engine_deployment_version_test.go(1 hunks)apps/workspace-engine/test/e2e/engine_entity_timestamps_test.go(1 hunks)apps/workspace-engine/test/e2e/engine_job_agent_retrigger_test.go(2 hunks)apps/workspace-engine/test/e2e/engine_job_agent_test.go(8 hunks)apps/workspace-engine/test/e2e/engine_job_state_machine_test.go(19 hunks)apps/workspace-engine/test/e2e/engine_jobs_test.go(4 hunks)apps/workspace-engine/test/e2e/engine_nil_release_handling_test.go(4 hunks)apps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.go(1 hunks)apps/workspace-engine/test/e2e/engine_redeploy_test.go(1 hunks)apps/workspace-engine/test/e2e/engine_releasemanager_test.go(1 hunks)apps/workspace-engine/test/e2e/engine_resource_variable_test.go(2 hunks)apps/workspace-engine/test/e2e/engine_verification_hooks_test.go(2 hunks)apps/workspace-engine/test/integration/creators/deployment.go(2 hunks)apps/workspace-engine/test/integration/creators/job_agent.go(1 hunks)apps/workspace-engine/test/integration/opts.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/test/integration/opts.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/pkg/db/release_targets_test.goapps/workspace-engine/pkg/db/deployment_variables_test.goapps/workspace-engine/test/e2e/engine_verification_hooks_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_test.goapps/workspace-engine/test/e2e/engine_nil_release_handling_test.goapps/workspace-engine/test/e2e/engine_redeploy_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_bench_test.goapps/workspace-engine/pkg/db/releases_test.goapps/workspace-engine/pkg/db/common.goapps/workspace-engine/pkg/workspace/store/diffcheck/deployment.goapps/workspace-engine/test/e2e/engine_deployment_version_test.goapps/workspace-engine/pkg/db/deployments_test.goapps/workspace-engine/pkg/workspace/relationships/property.goapps/workspace-engine/test/integration/creators/deployment.goapps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_test.goapps/workspace-engine/pkg/db/job_agents.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/dispatcher.goapps/workspace-engine/pkg/db/deployment_versions_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/executor_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_bench_test.goapps/workspace-engine/test/e2e/engine_bench_test.goapps/workspace-engine/pkg/workspace/store/diffcheck/deployment_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/test/e2e/engine_releasemanager_test.goapps/workspace-engine/test/e2e/engine_job_state_machine_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.goapps/workspace-engine/test/e2e/engine_job_agent_retrigger_test.goapps/workspace-engine/pkg/workspace/relationships/matcher_test.goapps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/test/e2e/engine_entity_timestamps_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.goapps/workspace-engine/pkg/events/handler/resources/resourceproviders_bench_test.goapps/workspace-engine/pkg/events/handler/deployment/deployment.goapps/workspace-engine/pkg/workspace/jobdispatch/terraformcloud.goapps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.goapps/workspace-engine/test/e2e/engine_resource_variable_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner.goapps/workspace-engine/pkg/workspace/jobdispatch/argocd.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/selector/match_test.goapps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_bench_test.goapps/workspace-engine/test/e2e/engine_jobs_test.goapps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.goapps/workspace-engine/test/e2e/engine_deployment_test.goapps/workspace-engine/test/e2e/engine_job_agent_test.goapps/workspace-engine/pkg/oapi/oapi.gen.goapps/workspace-engine/test/integration/creators/job_agent.goapps/workspace-engine/pkg/workspace/jobdispatch/github.go
apps/workspace-engine/**/*_test.go
📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)
Follow the existing test structure used in *_test.go files
Files:
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/pkg/db/release_targets_test.goapps/workspace-engine/pkg/db/deployment_variables_test.goapps/workspace-engine/test/e2e/engine_verification_hooks_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_test.goapps/workspace-engine/test/e2e/engine_nil_release_handling_test.goapps/workspace-engine/test/e2e/engine_redeploy_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_bench_test.goapps/workspace-engine/pkg/db/releases_test.goapps/workspace-engine/test/e2e/engine_deployment_version_test.goapps/workspace-engine/pkg/db/deployments_test.goapps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_test.goapps/workspace-engine/pkg/db/deployment_versions_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/executor_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_bench_test.goapps/workspace-engine/test/e2e/engine_bench_test.goapps/workspace-engine/pkg/workspace/store/diffcheck/deployment_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/test/e2e/engine_releasemanager_test.goapps/workspace-engine/test/e2e/engine_job_state_machine_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.goapps/workspace-engine/test/e2e/engine_job_agent_retrigger_test.goapps/workspace-engine/pkg/workspace/relationships/matcher_test.goapps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/test/e2e/engine_entity_timestamps_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/pkg/events/handler/resources/resourceproviders_bench_test.goapps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.goapps/workspace-engine/test/e2e/engine_resource_variable_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/selector/match_test.goapps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_bench_test.goapps/workspace-engine/test/e2e/engine_jobs_test.goapps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.goapps/workspace-engine/test/e2e/engine_deployment_test.goapps/workspace-engine/test/e2e/engine_job_agent_test.go
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
apps/workspace-engine/oapi/openapi.json
🧠 Learnings (14)
📓 Common learnings
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 637
File: packages/events/src/kafka/client.ts:10-16
Timestamp: 2025-08-01T04:41:41.345Z
Learning: User adityachoudhari26 prefers not to add null safety checks for required environment variables when they are guaranteed to be present in their deployment configuration, similar to their preference for simplicity over defensive programming in test code.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/**/*_test.go : Follow the existing test structure used in *_test.go files
Applied to files:
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/test/e2e/engine_verification_hooks_test.goapps/workspace-engine/test/e2e/engine_nil_release_handling_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_bench_test.goapps/workspace-engine/test/e2e/engine_deployment_version_test.goapps/workspace-engine/pkg/db/deployments_test.goapps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_test.goapps/workspace-engine/pkg/db/deployment_versions_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/executor_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/workspace/store/diffcheck/deployment_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/test/e2e/engine_job_state_machine_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.goapps/workspace-engine/pkg/workspace/relationships/matcher_test.goapps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.goapps/workspace-engine/test/e2e/engine_resource_variable_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/selector/match_test.goapps/workspace-engine/test/e2e/engine_jobs_test.goapps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.goapps/workspace-engine/test/e2e/engine_deployment_test.goapps/workspace-engine/test/e2e/engine_job_agent_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Write comprehensive, data-driven tests for new condition types
Applied to files:
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_bench_test.goapps/workspace-engine/pkg/db/deployments_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/pkg/workspace/relationships/matcher_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/selector/match_test.goapps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Use table-driven tests for all condition types
Applied to files:
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_bench_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/pkg/workspace/relationships/matcher_test.goapps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/selector/match_test.goapps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Include edge cases in tests (empty values, special characters, unicode) for condition types
Applied to files:
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/test/e2e/engine_verification_hooks_test.goapps/workspace-engine/test/e2e/engine_nil_release_handling_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_bench_test.goapps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/pkg/workspace/relationships/matcher_test.goapps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.goapps/workspace-engine/test/e2e/engine_resource_variable_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/selector/match_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.go
📚 Learning: 2025-08-12T20:49:05.086Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 655
File: apps/workspace-engine/pkg/engine/workspace/fluent.go:166-171
Timestamp: 2025-08-12T20:49:05.086Z
Learning: The UpdateDeploymentVersions() method with OperationCreate case in apps/workspace-engine/pkg/engine/workspace/fluent.go is specifically designed only for creating new deployment versions, not for handling potential duplicates or existing versions.
Applied to files:
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_test.goapps/workspace-engine/test/e2e/engine_nil_release_handling_test.goapps/workspace-engine/pkg/workspace/store/diffcheck/deployment.goapps/workspace-engine/test/integration/creators/deployment.goapps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_test.goapps/workspace-engine/oapi/spec/schemas/deployments.jsonnetapps/workspace-engine/pkg/db/deployment_versions_test.goapps/workspace-engine/test/e2e/engine_bench_test.goapps/workspace-engine/pkg/workspace/store/diffcheck/deployment_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.goapps/workspace-engine/test/e2e/engine_entity_timestamps_test.goapps/workspace-engine/pkg/events/handler/deployment/deployment.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_bench_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.go
📚 Learning: 2025-10-24T00:02:29.723Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 696
File: apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor.go:63-67
Timestamp: 2025-10-24T00:02:29.723Z
Learning: In workspace-engine replay mode (apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor.go), jobs are created with Pending status even during replay because job updates are still accepted and processed. If a job remains Pending after replay completes, it genuinely is still pending because no updates occurred during replay.
Applied to files:
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/test/e2e/engine_jobs_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.go
📚 Learning: 2025-10-07T16:44:54.938Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 673
File: apps/workspace-engine/pkg/db/deployment_variables.go:51-65
Timestamp: 2025-10-07T16:44:54.938Z
Learning: In `apps/workspace-engine/pkg/db/deployment_variables.go`, the `default_value_id` field is intentionally scanned from the database but not assigned to the `DefaultValueId` field in the proto struct. This is a temporary decision and the field assignment is deferred for later implementation.
Applied to files:
apps/workspace-engine/pkg/db/deployment_variables_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Test validation and matching logic separately for condition types
Applied to files:
apps/workspace-engine/test/e2e/engine_nil_release_handling_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_bench_test.goapps/workspace-engine/pkg/db/deployments_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/pkg/workspace/relationships/matcher_test.goapps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/pkg/selector/match_test.goapps/workspace-engine/pkg/db/job_agents_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/mapping/protobuf_mappings.go : Add mapping functions for the new condition in pkg/mapping/protobuf_mappings.go
Applied to files:
apps/workspace-engine/pkg/db/common.goapps/workspace-engine/test/integration/creators/deployment.goapps/workspace-engine/test/e2e/engine_bench_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/pkg/workspace/relationships/matcher_test.goapps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.go
📚 Learning: 2025-08-12T21:56:42.349Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 655
File: apps/workspace-engine/pkg/events/handler/deployment-version/new.go:15-17
Timestamp: 2025-08-12T21:56:42.349Z
Learning: In Go constructor naming conventions, when a struct is named with a "New" prefix (like NewDeploymentVersionHandler), the constructor function should be named "New" + the struct name, resulting in names like NewNewDeploymentVersionHandler(). This is not redundant but follows the standard Go pattern of New<TypeName>() for constructors.
Applied to files:
apps/workspace-engine/test/integration/creators/deployment.go
📚 Learning: 2025-04-28T18:41:58.813Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 515
File: apps/webservice/src/app/api/v1/release-targets/[releaseTargetId]/releases/route.ts:103-108
Timestamp: 2025-04-28T18:41:58.813Z
Learning: In this project, full records from the `deployment` and `deployment_version` tables are considered safe for public API consumption, and there's no need to create restricted DTOs for them.
Applied to files:
apps/workspace-engine/oapi/spec/schemas/deployments.jsonnet
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/**/*.go : Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods
Applied to files:
apps/workspace-engine/pkg/db/job_agents.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.goapps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/types.go : Add the new condition type to pkg/model/selector/types.go
Applied to files:
apps/workspace-engine/pkg/selector/match_test.go
🧬 Code graph analysis (49)
apps/workspace-engine/test/integration/opts.go (3)
apps/workspace-engine/test/integration/creators/deployment.go (1)
CustomDeploymentJobAgentConfig(11-28)apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
JobAgentConfig(683-685)CustomJobAgentConfig(281-285)apps/workspace-engine/test/integration/creators/job_agent.go (1)
CustomJobAgentConfig(10-28)
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (5)
DeploymentJobAgentConfig(393-395)Deployment(327-336)JobAgentConfig(683-685)FullJobAgentConfig(567-569)Job(657-671)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/pkg/db/release_targets_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/db/deployment_variables_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_verification_hooks_test.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateCompletedAt(140-140)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
DeploymentJobAgentConfig(393-395)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_redeploy_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/relationships/compute/rule_bench_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/db/releases_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/db/common.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
DeploymentJobAgentConfig(393-395)apps/workspace-engine/test/integration/opts.go (1)
DeploymentJobAgentConfig(403-411)
apps/workspace-engine/test/e2e/engine_deployment_version_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/db/deployments_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/relationships/property.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/test/integration/creators/deployment.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (3)
DeploymentJobAgentConfig(393-395)Deployment(327-336)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_test.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (3)
DeploymentJobAgentConfig(393-395)Deployment(327-336)JobAgentConfig(683-685)
apps/workspace-engine/pkg/db/job_agents.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
JobAgentConfig(683-685)JobAgent(674-680)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/dispatcher.go (6)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)apps/workspace-engine/pkg/workspace/jobdispatch/github.go (1)
NewGithubDispatcher(54-59)apps/workspace-engine/pkg/workspace/jobdispatch/argocd.go (1)
NewArgoCDDispatcher(39-44)apps/workspace-engine/pkg/workspace/jobdispatch/terraformcloud.go (1)
NewTerraformCloudDispatcher(184-186)apps/workspace-engine/pkg/workspace/jobdispatch/testrunner.go (1)
NewTestRunnerDispatcher(36-42)
apps/workspace-engine/pkg/db/deployment_versions_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (5)
JobAgentConfig(683-685)JobAgent(674-680)Deployment(327-336)Selector(1026-1028)DeploymentJobAgentConfig(393-395)apps/workspace-engine/test/integration/opts.go (2)
JobAgentConfig(702-707)DeploymentJobAgentConfig(403-411)
apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
JobAgentConfig(683-685)Deployment(327-336)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_bench_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
DeploymentJobAgentConfig(393-395)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_bench_test.go (3)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)apps/workspace-engine/test/integration/creators/deployment.go (1)
CustomDeploymentJobAgentConfig(11-28)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (3)
DeploymentJobAgentConfig(393-395)Deployment(327-336)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_job_state_machine_test.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateStartedAt(149-149)JobUpdateEventFieldsToUpdateCompletedAt(140-140)JobUpdateEventFieldsToUpdate(701-701)
apps/workspace-engine/pkg/db/persistence/store_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (3)
DeploymentJobAgentConfig(393-395)FullJobAgentConfig(567-569)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
DeploymentJobAgentConfig(393-395)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateCompletedAt(140-140)
apps/workspace-engine/test/e2e/engine_job_agent_retrigger_test.go (4)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
CustomJobAgentConfig(281-285)JobAgentConfig(683-685)apps/workspace-engine/test/integration/creators/job_agent.go (1)
CustomJobAgentConfig(10-28)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)apps/workspace-engine/test/integration/creators/deployment.go (1)
CustomDeploymentJobAgentConfig(11-28)
apps/workspace-engine/pkg/workspace/jobdispatch/github_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
FullGithubJobAgentConfig(546-561)FullJobAgentConfig(567-569)Job(657-671)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_entity_timestamps_test.go (3)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)apps/workspace-engine/test/integration/creators/deployment.go (1)
CustomDeploymentJobAgentConfig(11-28)
apps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (7)
FullJobAgentConfig(567-569)TestRunner(226-226)JobStatusPending(133-133)JobAgentConfig(683-685)Job(657-671)Failure(221-221)JobStatusInProgress(130-130)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (5)
Deployment(327-336)JobAgent(674-680)FullJobAgentConfig(567-569)JobAgentConfig(683-685)Job(657-671)
apps/workspace-engine/pkg/events/handler/resources/resourceproviders_bench_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
DeploymentJobAgentConfig(393-395)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/jobdispatch/terraformcloud.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
FullTerraformCloudJobAgentConfig(572-588)Job(657-671)JobUpdateEventFieldsToUpdate(701-701)JobAgentConfig(683-685)
apps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (3)
DeploymentJobAgentConfig(393-395)JobAgentConfig(683-685)Deployment(327-336)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_resource_variable_test.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateCompletedAt(140-140)
apps/workspace-engine/pkg/workspace/jobdispatch/testrunner.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (9)
TestRunnerJobAgentConfig(1098-1110)JobStatus(688-688)Failure(221-221)JobAgentConfig(683-685)JobUpdateEventFieldsToUpdate(701-701)JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateMessage(146-146)JobUpdateEventFieldsToUpdateUpdatedAt(152-152)JobUpdateEventFieldsToUpdateCompletedAt(140-140)
apps/workspace-engine/pkg/workspace/jobdispatch/argocd.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (8)
JobAgentConfig(683-685)Job(657-671)FullArgoCDJobAgentConfig(525-537)JobUpdateEventFieldsToUpdate(701-701)JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateMetadata(147-147)JobUpdateEventFieldsToUpdateCompletedAt(140-140)JobUpdateEventFieldsToUpdateUpdatedAt(152-152)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
DeploymentJobAgentConfig(393-395)Deployment(327-336)JobAgentConfig(683-685)FullJobAgentConfig(567-569)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
DeploymentJobAgentConfig(393-395)Deployment(327-336)JobAgentConfig(683-685)FullJobAgentConfig(567-569)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/pkg/selector/match_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
DeploymentJobAgentConfig(393-395)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_bench_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_jobs_test.go (3)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)apps/workspace-engine/test/integration/creators/deployment.go (1)
CustomDeploymentJobAgentConfig(11-28)
apps/workspace-engine/pkg/db/job_agents_test.go (2)
apps/workspace-engine/pkg/memsql/memsql_test.go (9)
Config(2720-2723)JobAgent(1259-1263)JobAgent(1331-1335)JobAgent(1374-1378)JobAgent(1539-1543)JobAgent(1604-1608)JobAgent(1651-1655)JobAgent(1874-1878)JobAgent(2172-2176)apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgent(674-680)
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
DeploymentJobAgentConfig(393-395)Deployment(327-336)JobAgentConfig(683-685)FullJobAgentConfig(567-569)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_deployment_test.go (4)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
JobAgentConfig(683-685)Deployment(327-336)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)apps/workspace-engine/pkg/events/handler/handler.go (1)
JobUpdate(68-68)apps/workspace-engine/test/integration/creators/deployment.go (1)
CustomDeploymentJobAgentConfig(11-28)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
apps/workspace-engine/test/integration/creators/job_agent.go (1)
CustomJobAgentConfig(10-28)
apps/workspace-engine/test/integration/creators/job_agent.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (3)
CustomJobAgentConfig(281-285)JobAgentConfig(683-685)JobAgent(674-680)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/jobdispatch/github.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
Job(657-671)FullGithubJobAgentConfig(546-561)GithubEntity(597-600)JobAgentConfig(683-685)
⏰ 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). (5)
- GitHub Check: Lint
- GitHub Check: workspace-engine-tests
- GitHub Check: Typecheck
- GitHub Check: build (linux/amd64)
- GitHub Check: tests
apps/workspace-engine/pkg/events/handler/deployment/deployment.go
Outdated
Show resolved
Hide resolved
apps/workspace-engine/pkg/events/handler/resources/resourceproviders_bench_test.go
Show resolved
Hide resolved
apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/api/openapi/schemas/jobs.jsonnet (1)
123-133: Consider consistency for TestRunner and Custom variants.
FullTestRunnerJobAgentConfigandFullCustomJobAgentConfigonly wrap their base configs without deployment variants, unlike the other three. This may be intentional (no deployment-level overrides needed), but for consistency, consider whetherDeploymentTestRunnerJobAgentConfigand corresponding full composition might be needed in the future.apps/api/openapi/schemas/job-agents.jsonnet (2)
61-61: Verifyformat: 'int'is the intended integer format.OpenAPI 3.x typically uses
format: 'int32'orformat: 'int64'for integers. Using'int'may not be recognized by all code generators. Consider using'int32'for consistency with other schemas (e.g.,workflowIduses'int64'in deployments.jsonnet).- installationId: { type: 'integer', format: 'int' }, + installationId: { type: 'integer', format: 'int64' },
106-106: Sameformat: 'int'concern fordelaySeconds.Consider using a standard format like
'int32'for consistency.- delaySeconds: { type: 'integer', format: 'int', description: 'Delay before resolving the job.' }, + delaySeconds: { type: 'integer', format: 'int32', description: 'Delay before resolving the job.' },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/api/openapi/schemas/deployments.jsonnet(3 hunks)apps/api/openapi/schemas/job-agents.jsonnet(2 hunks)apps/api/openapi/schemas/jobs.jsonnet(2 hunks)apps/api/src/routes/v1/workspaces/job-agents.ts(1 hunks)apps/api/src/types/openapi.ts(12 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript with explicit types (prefer interfaces for public APIs)
Import styles: Use named imports, group imports by source (std lib > external > internal)
Consistent type imports:import type { Type } from "module"
Prefer async/await over raw promises
Handle errors explicitly (use try/catch and typed error responses)
Files:
apps/api/src/routes/v1/workspaces/job-agents.tsapps/api/src/types/openapi.ts
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
Files:
apps/api/src/routes/v1/workspaces/job-agents.tsapps/api/src/types/openapi.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
apps/api/src/routes/v1/workspaces/job-agents.tsapps/api/src/types/openapi.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 637
File: packages/events/src/kafka/client.ts:10-16
Timestamp: 2025-08-01T04:41:41.345Z
Learning: User adityachoudhari26 prefers not to add null safety checks for required environment variables when they are guaranteed to be present in their deployment configuration, similar to their preference for simplicity over defensive programming in test code.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.
📚 Learning: 2025-04-28T18:41:58.813Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 515
File: apps/webservice/src/app/api/v1/release-targets/[releaseTargetId]/releases/route.ts:103-108
Timestamp: 2025-04-28T18:41:58.813Z
Learning: In this project, full records from the `deployment` and `deployment_version` tables are considered safe for public API consumption, and there's no need to create restricted DTOs for them.
Applied to files:
apps/api/openapi/schemas/deployments.jsonnet
🧬 Code graph analysis (1)
apps/api/src/types/openapi.ts (2)
packages/workspace-engine-sdk/src/schema.ts (1)
components(1037-1951)apps/web/app/api/openapi.ts (1)
components(283-599)
🔇 Additional comments (8)
apps/api/src/routes/v1/workspaces/job-agents.ts (1)
31-38: LGTM - Config is now required in schema.The removal of
?? {}fallback is correct sinceconfigis now a required field inUpsertJobAgentRequest(per the updated OpenAPI schema injob-agents.jsonnet). The typed handler will enforce this at request validation.apps/api/openapi/schemas/deployments.jsonnet (2)
45-61: Well-structured discriminated union.The
DeploymentJobAgentConfigunion schema is properly defined with:
- Correct
oneOfcomposition of all deployment-specific variants- Discriminator on
typeproperty with explicit mappings- Consistent type values matching the enum constraints in each variant
63-115: Deployment-specific config variants are well-defined.Each variant:
- Has
typeas required with a single-value enum for discriminator matching- Includes clear descriptions for each property
- Uses appropriate types (
int64forworkflowId,stringfor templates)CustomJobAgentConfigcorrectly allowsadditionalPropertiesfor extensibilityapps/api/src/types/openapi.ts (1)
1-4: Auto-generated file - no manual changes required.This file is auto-generated by
openapi-typescript. The generated types correctly reflect the discriminated union schemas defined in the jsonnet files, with proper literal type discriminators and intersection types forallOfcompositions.apps/api/openapi/schemas/jobs.jsonnet (1)
82-100: FullJobAgentConfig union is correctly structured.The discriminated union properly maps all five agent types with consistent discriminator values matching the base schemas.
apps/api/openapi/schemas/job-agents.jsonnet (3)
4-16: Breaking change:configis now required.Adding
configto the required array is a breaking change for existing API clients that previously could omit it. Ensure this is documented in migration notes and that clients are updated accordingly.
107-107: Clarifystatusenum values for TestRunner.The
statusenum uses'completed'and'failure', but the mainJobStatusenum uses'successful'(not'completed'). Ensure these values are intentional for the test runner's internal semantics and won't cause confusion when compared to actual job statuses.
32-50: JobAgentConfig discriminated union is well-defined.The oneOf union with explicit discriminator mappings is correctly structured. All type values match their corresponding schema enum constraints.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/workspace-engine/oapi/openapi.json (3)
758-781: Non-standardformatvalue forinstallationId.The
format: "int"on line 761 is not a standard OpenAPI format. Standard integer formats areint32orint64. While some code generators may tolerate this, using a standard format improves compatibility."GithubJobAgentConfig": { "properties": { "installationId": { - "format": "int", + "format": "int64", "type": "integer" },
1961-1995: Clarify the purpose oftemplatein base config.
TerraformCloudJobAgentConfigincludes atemplatefield (line 1972) with the same description as inDeploymentTerraformCloudJobAgentConfig. When merged viaFullTerraformCloudJobAgentConfig'sallOf, both contribute the same property. Consider whethertemplateshould only exist in the deployment-level config (since templates are typically deployment-specific), or add distinct descriptions clarifying base vs override behavior.
2029-2060: Non-standardformatvalue fordelaySeconds.Similar to the
GithubJobAgentConfigissue,format: "int"on line 2033 is not a standard OpenAPI format."delaySeconds": { "description": "Delay before resolving the job.", - "format": "int", + "format": "int32", "type": "integer" },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/api/src/types/openapi.ts(12 hunks)apps/workspace-engine/oapi/openapi.json(13 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
apps/workspace-engine/oapi/openapi.jsonapps/api/src/types/openapi.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript with explicit types (prefer interfaces for public APIs)
Import styles: Use named imports, group imports by source (std lib > external > internal)
Consistent type imports:import type { Type } from "module"
Prefer async/await over raw promises
Handle errors explicitly (use try/catch and typed error responses)
Files:
apps/api/src/types/openapi.ts
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
Files:
apps/api/src/types/openapi.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 637
File: packages/events/src/kafka/client.ts:10-16
Timestamp: 2025-08-01T04:41:41.345Z
Learning: User adityachoudhari26 prefers not to add null safety checks for required environment variables when they are guaranteed to be present in their deployment configuration, similar to their preference for simplicity over defensive programming in test code.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.
⏰ 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). (7)
- GitHub Check: Typecheck
- GitHub Check: build-and-push (linux/amd64)
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
- GitHub Check: build (linux/amd64)
- GitHub Check: workspace-engine-tests
- GitHub Check: tests
🔇 Additional comments (7)
apps/api/src/types/openapi.ts (3)
1-4: Auto-generated file — changes look correct.This file is properly marked as auto-generated. Any necessary changes to type definitions should be made in the source OpenAPI schema, not directly in this file.
762-1537: Well-structured discriminated union implementation.The three-tier config hierarchy (base, deployment, full) cleanly separates agent-level configuration from deployment-specific overrides, and the discriminated unions enable type-safe routing and validation. Usage is consistent across schemas:
JobAgent.config→JobAgentConfig(base)Deployment.jobAgentConfig→DeploymentJobAgentConfig(deployment overrides)Job.jobAgentConfig→FullJobAgentConfig(merged)
1413-1429: Remove concern about TerraformCloudJobAgentConfig additional properties.The OpenAPI schema explicitly defines
"additionalProperties": truefor TerraformCloudJobAgentConfig. The generated TypeScript& { [key: string]: unknown; }correctly reflects this intentional schema design, allowing extensibility for future Terraform Cloud API enhancements. This is not a type safety issue.Likely an incorrect or invalid review comment.
apps/workspace-engine/oapi/openapi.json (4)
34-99: LGTM - Base job agent config schemas are well-defined.The discriminator pattern with
typeas the property name and appropriate enum values is correctly implemented. TheadditionalProperties: trueonCustomJobAgentConfigappropriately allows for extensibility.
316-340: Discriminated union for deployment-level configs is correctly structured.The
DeploymentJobAgentConfigexcludestest-runnerwhileJobAgentConfigandFullJobAgentConfiginclude it. This appears intentional sinceTestRunnerJobAgentConfigis for testing purposes and doesn't require deployment-level overrides. The schema design is consistent with this intent.
670-742: Well-designed composite config schemas usingallOfcomposition.The
Full*JobAgentConfigschemas correctly merge base job agent configs with their deployment-level counterparts. The discriminator mapping andoneOfreferences are consistent across all five agent types.
192-194: Schema references correctly implement the typed config hierarchy.The updated references form a coherent config merging pattern:
JobAgent.config→JobAgentConfig(base agent configuration)Deployment.jobAgentConfig→DeploymentJobAgentConfig(deployment-level overrides)Job.jobAgentConfig→FullJobAgentConfig(merged config for execution)The discriminator-based unions enable type-safe routing to appropriate config handlers.
Also applies to: 875-877, 922-924, 947-975
adityachoudhari26
force-pushed
the
job-agent-config-refactor
branch
from
de79509 to
a4770d4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
apps/workspace-engine/pkg/workspace/relationships/property.go (1)
113-124: Critical: Missing error check aftergetMapValue(duplicate).This issue was flagged in a previous review and remains unresolved. Line 123 captures the error from
getMapValue, but line 124 proceeds to callconvertValue(value)without checking it. IfgetMapValuefails (e.g., nested property not found), the error is silently ignored and an invalid or nilvalueis passed toconvertValue, leading to incorrect behavior.Apply this diff to properly handle the error:
jobAgentConfigJSON, err := deployment.JobAgentConfig.MarshalJSON() if err != nil { return nil, fmt.Errorf("failed to marshal deployment job agent config: %v", err) } var jobAgentConfigMap map[string]any err = json.Unmarshal(jobAgentConfigJSON, &jobAgentConfigMap) if err != nil { return nil, fmt.Errorf("failed to unmarshal deployment job agent config: %v", err) } value, err := getMapValue(jobAgentConfigMap, propertyPath[1:]) + if err != nil { + return nil, err + } return convertValue(value)Note: This matches the error-handling pattern used for
resource.Configat lines 73-77.apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go (1)
229-234: Redundant JSON round-trip and silently ignored error.
mergedConfigreturned frommergeJobAgentConfig(line 206) is already anoapi.FullJobAgentConfig. Re-marshalling and unmarshalling it is unnecessary, and line 234 silently discards the unmarshal error.- mergedJobAgentConfig := oapi.FullJobAgentConfig{} - mergedJobAgentConfigJSON, err := json.Marshal(mergedConfig) - if err != nil { - return nil, fmt.Errorf("failed to marshal merged job agent config: %v", err) - } - _ = mergedJobAgentConfig.UnmarshalJSON(mergedJobAgentConfigJSON) - return &oapi.Job{ Id: jobId, ReleaseId: release.ID(), JobAgentId: *jobAgentId, - JobAgentConfig: mergedJobAgentConfig, + JobAgentConfig: mergedConfig, Status: oapi.JobStatusPending, CreatedAt: time.Now(), UpdatedAt: time.Now(), Metadata: make(map[string]string), }, nilapps/workspace-engine/test/e2e/engine_job_agent_test.go (1)
255-261: Fix format specifiers for float64 values.Lines 256 and 260 use
%d(integer format) but the values arefloat64after JSON unmarshaling. This produces incorrect error messages if assertions fail.if updatedConfig.AdditionalProperties["timeout"] != float64(600) { - t.Fatalf("job agent config timeout mismatch: got %d, want 600", updatedConfig.AdditionalProperties["timeout"]) + t.Fatalf("job agent config timeout mismatch: got %v, want 600", updatedConfig.AdditionalProperties["timeout"]) } if updatedConfig.AdditionalProperties["retries"] != float64(5) { - t.Fatalf("job agent config retries mismatch: got %d, want 5", updatedConfig.AdditionalProperties["retries"]) + t.Fatalf("job agent config retries mismatch: got %v, want 5", updatedConfig.AdditionalProperties["retries"]) }apps/workspace-engine/pkg/workspace/jobdispatch/testrunner.go (1)
99-102: Fixdelay_secondsattribute to report seconds, not nanoseconds.
time.Durationis stored as nanoseconds, soint64(delay)makesdelay_secondsmisleading. Line 117 correctly usesdelay.Seconds()for the same purpose.span.SetAttributes( - attribute.Int64("delay_seconds", int64(delay)), + attribute.Int64("delay_seconds", int64(delay.Seconds())), attribute.String("final_status", string(finalStatus)), )
🧹 Nitpick comments (11)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.go (2)
30-38: LGTM! Typed config correctly implemented.The typed
DeploymentJobAgentConfigpattern correctly implements the discriminated union refactor. The JSON unmarshalling and struct assignment are appropriate.Optional: Consider two refinements for test robustness:
Use
require.NoError(t, customJobAgentConfig.UnmarshalJSON(...))instead of ignoring the error. This would fail fast with clear messages if the JSON is malformed, rather than proceeding with a zero-value config.Extract a helper function since this pattern repeats 4 times (lines 30-31, 145-146, 275-276, 368-369):
func createCustomJobAgentConfig(t *testing.T) oapi.DeploymentJobAgentConfig { t.Helper() config := oapi.DeploymentJobAgentConfig{} require.NoError(t, config.UnmarshalJSON([]byte(`{"type": "custom"}`))) return config }Then use:
customJobAgentConfig := createCustomJobAgentConfig(t)
153-153: Redundant explicit nil assignment.The explicit
JobAgentId: nilis redundant since pointer fields default to nil in Go struct literals. This is the only test that sets this field explicitly, while the other three tests omit it.Consider removing this line for consistency with the other tests:
SystemId: "system-1", JobAgentConfig: customJobAgentConfig, - JobAgentId: nil, }apps/api/src/types/openapi.ts (1)
988-990: Note: DeploymentVersion types retain untyped jobAgentConfig.
DeploymentVersion,CreateDeploymentVersionRequest,UpdateDeploymentVersionRequest, andUpsertDeploymentVersionRequeststill use{ [key: string]: unknown }forjobAgentConfiginstead of the new discriminated unions. This might be intentional if version configs need flexibility beyond the typed schema, but it creates an inconsistency worth noting. Consider whether version configs should also use the typed union for consistency and type safety.Also applies to: 795-797, 1477-1479, 1520-1522
apps/workspace-engine/pkg/db/persistence/store_test.go (1)
40-58: Helper constructors correctly build typed job agent config unions for testsThe three helpers (
customJobAgentConfig,customFullJobAgentConfig,customJobAgentDefaults) safely:
- Clone the input map (including nil case),
- Inject the
"type": "custom"discriminator, and- Round-trip through
UnmarshalJSONon the generated union wrappers.This is a minimal, test-only way to exercise the discriminator-based union behavior without leaking complexity into the tests themselves. Panicking on marshal/unmarshal failure is fine in this context.
If you ever need more variants (argo-cd/github/tfe) in these tests, consider factoring a tiny internal helper (
makeCustomPayloador similar) to share the clone+marshal logic, but it isn’t necessary right now.Also applies to: 60-78, 80-98
apps/workspace-engine/oapi/openapi.json (1)
34-58: Job agent config discriminated unions and wiring look structurally soundThe new schemas and unions for job agent configs are consistent:
- Base agent configs (
ArgoCDJobAgentConfig,GithubJobAgentConfig,TerraformCloudJobAgentConfig,TestRunnerJobAgentConfig,CustomJobAgentConfig) all define a requiredtypediscriminator with enums that match the union mappings.- Deployment-scoped variants (
Deployment*JobAgentConfig) correctly focus on per-deployment fields (e.g., templates, repo/workflowId) and share the sametypevalues.- Union wrappers:
JobAgentConfigdiscriminates default agent configs,DeploymentJobAgentConfigdiscriminates deployment-level overrides, andFullJobAgentConfigdiscriminates resolved/merged configs via the appropriateFull*JobAgentConfigcomposites.- Model wiring is updated coherently:
Deployment.jobAgentConfig→DeploymentJobAgentConfig,Job.jobAgentConfig→FullJobAgentConfig,JobAgent.config→JobAgentConfig,DeploymentVersion.jobAgentConfigremains a free-form object but now documents its relationship to the typed shapes.This should play nicely with the generated Go union wrappers and oapi-codegen’s discriminator handling.
You may want to normalize
GithubJobAgentConfig.installationId’s format to"int64"(to matchworkflowIdand other integer IDs) for better tooling compatibility, but it’s optional and non-blocking.Also applies to: 84-99, 185-219, 235-360, 432-483, 670-725, 758-781, 875-877, 922-975, 1961-1995, 2029-2060
apps/workspace-engine/pkg/db/job_agents.go (1)
46-63: Consider clarifying function name and usage context.The function name
runnerJobAgentConfigsuggests it creates a "runner" config, but it actually forces the type to "custom". This could be confusing.Additionally, the panic-on-error behavior suggests this is intended as a test helper, but it's in a non-test file. Consider:
- Renaming to reflect its actual behavior (e.g.,
customJobAgentConfigOrPanic)- Moving to a test-only file (e.g.,
common_test.go)- Or documenting that it's specifically for test scaffolding
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.go (1)
472-473: Consider extracting repeated config creation to a helper.The same
jobAgentConfiginitialization pattern (oapi.FullJobAgentConfig{}+UnmarshalJSON) is repeated across multiple test functions. A test helper would reduce duplication.Example helper:
func testFullJobAgentConfig() oapi.FullJobAgentConfig { cfg := oapi.FullJobAgentConfig{} _ = cfg.UnmarshalJSON([]byte(`{"type": "custom"}`)) return cfg }Also applies to: 482-482, 496-496, 510-510
apps/workspace-engine/pkg/workspace/jobdispatch/github_test.go (1)
318-321: Unused helper function.The
strPtrhelper function appears to be unused in this file.#!/bin/bash # Verify if strPtr is used anywhere in this file rg -n 'strPtr' apps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go (1)
40-56: Consider adding a comment explaining the TestRunner special case.The empty
case string(oapi.TestRunner):branch silently skips deployment config validation and merging for test runners. While this appears intentional (test runners don't need deployment overrides), a brief comment would clarify this design decision for future maintainers.switch runnerDiscriminator { case string(oapi.TestRunner): + // TestRunner configs don't support deployment-level overrides; skip validation. default:apps/workspace-engine/pkg/workspace/jobdispatch/github.go (1)
189-196: Remove redundant int64 cast.Per
oapi.gen.go,FullGithubJobAgentConfig.WorkflowIdis alreadyint64. The castint64(cfg.WorkflowId)is unnecessary.err = client.DispatchWorkflow( ctx, cfg.Owner, cfg.Repo, - int64(cfg.WorkflowId), + cfg.WorkflowId, ref, inputs, )apps/api/openapi/schemas/job-agents.jsonnet (1)
52-64: Consider using standard OpenAPI integer format.
format: 'int'on line 61 is not a standard OpenAPI format. Standard formats areint32orint64. This may cause inconsistent code generation across different OpenAPI tooling.- installationId: { type: 'integer', format: 'int' }, + installationId: { type: 'integer', format: 'int32' },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (66)
apps/api/openapi/schemas/deployments.jsonnet(3 hunks)apps/api/openapi/schemas/job-agents.jsonnet(2 hunks)apps/api/openapi/schemas/jobs.jsonnet(2 hunks)apps/api/src/routes/v1/workspaces/job-agents.ts(1 hunks)apps/api/src/types/openapi.ts(12 hunks)apps/workspace-engine/oapi/openapi.json(13 hunks)apps/workspace-engine/oapi/spec/schemas/deployments.jsonnet(2 hunks)apps/workspace-engine/oapi/spec/schemas/entities.jsonnet(1 hunks)apps/workspace-engine/oapi/spec/schemas/jobs.jsonnet(2 hunks)apps/workspace-engine/pkg/db/common.go(1 hunks)apps/workspace-engine/pkg/db/deployment_variables_test.go(4 hunks)apps/workspace-engine/pkg/db/deployment_versions_test.go(4 hunks)apps/workspace-engine/pkg/db/deployments_test.go(10 hunks)apps/workspace-engine/pkg/db/job_agents.go(3 hunks)apps/workspace-engine/pkg/db/job_agents_test.go(10 hunks)apps/workspace-engine/pkg/db/persistence/store_test.go(9 hunks)apps/workspace-engine/pkg/db/release_targets_test.go(1 hunks)apps/workspace-engine/pkg/db/releases_test.go(1 hunks)apps/workspace-engine/pkg/events/handler/deployment/deployment.go(1 hunks)apps/workspace-engine/pkg/events/handler/resources/resourceproviders_bench_test.go(2 hunks)apps/workspace-engine/pkg/oapi/oapi.gen.go(16 hunks)apps/workspace-engine/pkg/selector/match_test.go(7 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/argocd.go(4 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/github.go(2 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/github_test.go(8 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/terraformcloud.go(4 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/testrunner.go(4 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.go(8 hunks)apps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.go(9 hunks)apps/workspace-engine/pkg/workspace/relationships/compute/rule_bench_test.go(3 hunks)apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.go(12 hunks)apps/workspace-engine/pkg/workspace/relationships/matcher_test.go(3 hunks)apps/workspace-engine/pkg/workspace/relationships/property.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor_test.go(2 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.go(4 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/dispatcher.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go(6 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_bench_test.go(2 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_test.go(2 hunks)apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.go(20 hunks)apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.go(40 hunks)apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.go(14 hunks)apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.go(19 hunks)apps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.go(6 hunks)apps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_bench_test.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_test.go(13 hunks)apps/workspace-engine/pkg/workspace/store/diffcheck/deployment.go(3 hunks)apps/workspace-engine/pkg/workspace/store/diffcheck/deployment_test.go(29 hunks)apps/workspace-engine/test/e2e/engine_bench_test.go(2 hunks)apps/workspace-engine/test/e2e/engine_deployment_test.go(6 hunks)apps/workspace-engine/test/e2e/engine_deployment_version_test.go(1 hunks)apps/workspace-engine/test/e2e/engine_entity_timestamps_test.go(1 hunks)apps/workspace-engine/test/e2e/engine_job_agent_retrigger_test.go(2 hunks)apps/workspace-engine/test/e2e/engine_job_agent_test.go(8 hunks)apps/workspace-engine/test/e2e/engine_job_state_machine_test.go(19 hunks)apps/workspace-engine/test/e2e/engine_jobs_test.go(4 hunks)apps/workspace-engine/test/e2e/engine_nil_release_handling_test.go(4 hunks)apps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.go(1 hunks)apps/workspace-engine/test/e2e/engine_redeploy_test.go(1 hunks)apps/workspace-engine/test/e2e/engine_releasemanager_test.go(1 hunks)apps/workspace-engine/test/e2e/engine_resource_variable_test.go(2 hunks)apps/workspace-engine/test/e2e/engine_verification_hooks_test.go(2 hunks)apps/workspace-engine/test/integration/creators/deployment.go(2 hunks)apps/workspace-engine/test/integration/creators/job_agent.go(1 hunks)apps/workspace-engine/test/integration/opts.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (26)
- apps/workspace-engine/pkg/db/deployments_test.go
- apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_bench_test.go
- apps/workspace-engine/test/e2e/engine_policy_deployment_dependency_test.go
- apps/workspace-engine/pkg/workspace/relationships/matcher_test.go
- apps/workspace-engine/test/integration/creators/deployment.go
- apps/workspace-engine/pkg/workspace/relationships/compute/incremental_test.go
- apps/workspace-engine/test/e2e/engine_nil_release_handling_test.go
- apps/workspace-engine/test/e2e/engine_jobs_test.go
- apps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_test.go
- apps/workspace-engine/test/e2e/engine_verification_hooks_test.go
- apps/workspace-engine/test/e2e/engine_redeploy_test.go
- apps/workspace-engine/pkg/workspace/relationships/compute/rule_bench_test.go
- apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor_test.go
- apps/workspace-engine/pkg/workspace/releasemanager/deployment/planner_test.go
- apps/api/src/routes/v1/workspaces/job-agents.ts
- apps/workspace-engine/test/e2e/engine_bench_test.go
- apps/workspace-engine/pkg/db/common.go
- apps/workspace-engine/test/integration/opts.go
- apps/workspace-engine/pkg/workspace/store/diffcheck/deployment.go
- apps/workspace-engine/test/integration/creators/job_agent.go
- apps/workspace-engine/pkg/workspace/releasemanager/variables/variablemanager_bench_test.go
- apps/workspace-engine/pkg/events/handler/resources/resourceproviders_bench_test.go
- apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/dispatcher.go
- apps/workspace-engine/pkg/events/handler/deployment/deployment.go
- apps/workspace-engine/pkg/selector/match_test.go
- apps/workspace-engine/pkg/workspace/store/diffcheck/deployment_test.go
🧰 Additional context used
📓 Path-based instructions (4)
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/db/releases_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/test/e2e/engine_job_state_machine_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/test/e2e/engine_entity_timestamps_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/test/e2e/engine_deployment_version_test.goapps/workspace-engine/test/e2e/engine_resource_variable_test.goapps/workspace-engine/test/e2e/engine_releasemanager_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/test/e2e/engine_job_agent_retrigger_test.goapps/workspace-engine/pkg/db/release_targets_test.goapps/workspace-engine/pkg/workspace/jobdispatch/github.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.goapps/workspace-engine/pkg/workspace/jobdispatch/terraformcloud.goapps/workspace-engine/test/e2e/engine_job_agent_test.goapps/workspace-engine/test/e2e/engine_deployment_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.goapps/workspace-engine/pkg/db/deployment_variables_test.goapps/workspace-engine/pkg/workspace/relationships/property.goapps/workspace-engine/pkg/db/job_agents.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.goapps/workspace-engine/pkg/workspace/jobdispatch/argocd.goapps/workspace-engine/pkg/db/deployment_versions_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.goapps/workspace-engine/pkg/oapi/oapi.gen.go
apps/workspace-engine/**/*_test.go
📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)
Follow the existing test structure used in *_test.go files
Files:
apps/workspace-engine/pkg/db/releases_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/test/e2e/engine_job_state_machine_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/test/e2e/engine_entity_timestamps_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/test/e2e/engine_deployment_version_test.goapps/workspace-engine/test/e2e/engine_resource_variable_test.goapps/workspace-engine/test/e2e/engine_releasemanager_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/test/e2e/engine_job_agent_retrigger_test.goapps/workspace-engine/pkg/db/release_targets_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.goapps/workspace-engine/test/e2e/engine_job_agent_test.goapps/workspace-engine/test/e2e/engine_deployment_test.goapps/workspace-engine/pkg/db/deployment_variables_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.goapps/workspace-engine/pkg/db/deployment_versions_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
apps/workspace-engine/oapi/openapi.jsonapps/api/src/types/openapi.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript with explicit types (prefer interfaces for public APIs)
Import styles: Use named imports, group imports by source (std lib > external > internal)
Consistent type imports:import type { Type } from "module"
Prefer async/await over raw promises
Handle errors explicitly (use try/catch and typed error responses)
Files:
apps/api/src/types/openapi.ts
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
Files:
apps/api/src/types/openapi.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 637
File: packages/events/src/kafka/client.ts:10-16
Timestamp: 2025-08-01T04:41:41.345Z
Learning: User adityachoudhari26 prefers not to add null safety checks for required environment variables when they are guaranteed to be present in their deployment configuration, similar to their preference for simplicity over defensive programming in test code.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/**/*_test.go : Follow the existing test structure used in *_test.go files
Applied to files:
apps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/test/e2e/engine_job_state_machine_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/test/e2e/engine_resource_variable_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/test/e2e/engine_job_agent_retrigger_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.goapps/workspace-engine/test/e2e/engine_job_agent_test.goapps/workspace-engine/test/e2e/engine_deployment_test.goapps/workspace-engine/pkg/db/deployment_variables_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.goapps/workspace-engine/pkg/db/deployment_versions_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Write comprehensive, data-driven tests for new condition types
Applied to files:
apps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/pkg/db/persistence/store_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Include edge cases in tests (empty values, special characters, unicode) for condition types
Applied to files:
apps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/test/e2e/engine_resource_variable_test.goapps/workspace-engine/test/e2e/engine_job_agent_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-08-12T20:49:05.086Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 655
File: apps/workspace-engine/pkg/engine/workspace/fluent.go:166-171
Timestamp: 2025-08-12T20:49:05.086Z
Learning: The UpdateDeploymentVersions() method with OperationCreate case in apps/workspace-engine/pkg/engine/workspace/fluent.go is specifically designed only for creating new deployment versions, not for handling potential duplicates or existing versions.
Applied to files:
apps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.goapps/workspace-engine/test/e2e/engine_entity_timestamps_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/oapi/spec/schemas/deployments.jsonnetapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.goapps/workspace-engine/pkg/db/deployment_versions_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Test validation and matching logic separately for condition types
Applied to files:
apps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Use table-driven tests for all condition types
Applied to files:
apps/workspace-engine/pkg/workspace/jobdispatch/github_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.goapps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/resolver/resolver_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-10-24T00:02:29.723Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 696
File: apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor.go:63-67
Timestamp: 2025-10-24T00:02:29.723Z
Learning: In workspace-engine replay mode (apps/workspace-engine/pkg/workspace/releasemanager/deployment/executor.go), jobs are created with Pending status even during replay because job updates are still accepted and processed. If a job remains Pending after replay completes, it genuinely is still pending because no updates occurred during replay.
Applied to files:
apps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/soaktime_test.goapps/workspace-engine/test/e2e/engine_job_agent_retrigger_test.goapps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.go
📚 Learning: 2025-04-28T18:41:58.813Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 515
File: apps/webservice/src/app/api/v1/release-targets/[releaseTargetId]/releases/route.ts:103-108
Timestamp: 2025-04-28T18:41:58.813Z
Learning: In this project, full records from the `deployment` and `deployment_version` tables are considered safe for public API consumption, and there's no need to create restricted DTOs for them.
Applied to files:
apps/workspace-engine/oapi/spec/schemas/deployments.jsonnetapps/api/openapi/schemas/deployments.jsonnet
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/**/*.go : Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods
Applied to files:
apps/workspace-engine/pkg/db/job_agents_test.goapps/workspace-engine/test/e2e/engine_job_agent_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.goapps/workspace-engine/pkg/db/job_agents.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/mapping/protobuf_mappings.go : Add mapping functions for the new condition in pkg/mapping/protobuf_mappings.go
Applied to files:
apps/workspace-engine/pkg/db/persistence/store_test.go
📚 Learning: 2025-10-07T16:44:54.938Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 673
File: apps/workspace-engine/pkg/db/deployment_variables.go:51-65
Timestamp: 2025-10-07T16:44:54.938Z
Learning: In `apps/workspace-engine/pkg/db/deployment_variables.go`, the `default_value_id` field is intentionally scanned from the database but not assigned to the `DefaultValueId` field in the proto struct. This is a temporary decision and the field assignment is deferred for later implementation.
Applied to files:
apps/workspace-engine/pkg/db/deployment_variables_test.go
🧬 Code graph analysis (25)
apps/workspace-engine/pkg/db/releases_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/job_eligibility_retry_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (3)
DeploymentJobAgentConfig(393-395)Deployment(327-336)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_job_state_machine_test.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (3)
JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateStartedAt(149-149)JobUpdateEventFieldsToUpdate(701-701)
apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
JobAgentConfig(683-685)Deployment(327-336)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/jobdispatch/testrunner_test.go (3)
apps/workspace-engine/pkg/oapi/oapi.gen.go (6)
FullJobAgentConfig(567-569)TestRunner(226-226)JobStatusPending(133-133)JobAgentConfig(683-685)Job(657-671)JobStatusInProgress(130-130)apps/workspace-engine/pkg/workspace/releasemanager/trace/types.go (1)
Status(68-68)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/environmentprogression_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
DeploymentJobAgentConfig(393-395)Deployment(327-336)JobAgentConfig(683-685)FullJobAgentConfig(567-569)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/pkg/db/job_agents_test.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgent(674-680)
apps/workspace-engine/test/e2e/engine_deployment_version_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_resource_variable_test.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateCompletedAt(140-140)
apps/workspace-engine/pkg/db/persistence/store_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (3)
DeploymentJobAgentConfig(393-395)FullJobAgentConfig(567-569)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/test/e2e/engine_job_agent_retrigger_test.go (4)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
CustomJobAgentConfig(281-285)JobAgentConfig(683-685)apps/workspace-engine/test/integration/creators/job_agent.go (1)
CustomJobAgentConfig(10-28)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)apps/workspace-engine/test/integration/creators/deployment.go (1)
CustomDeploymentJobAgentConfig(11-28)
apps/workspace-engine/pkg/db/release_targets_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/jobdispatch/github.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
Job(657-671)FullGithubJobAgentConfig(546-561)GithubEntity(597-600)JobAgentConfig(683-685)apps/workspace-engine/pkg/workspace/store/github_entities.go (1)
GithubEntities(10-13)
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/jobtracker_test.go (3)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
DeploymentJobAgentConfig(393-395)Deployment(327-336)JobAgentConfig(683-685)FullJobAgentConfig(567-569)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)apps/workspace-engine/pkg/memsql/memsql_test.go (1)
Deployment(1444-1448)
apps/workspace-engine/pkg/workspace/jobdispatch/terraformcloud.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (8)
FullTerraformCloudJobAgentConfig(572-588)Job(657-671)JobUpdateEventFieldsToUpdate(701-701)JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateMetadata(147-147)JobUpdateEventFieldsToUpdateCompletedAt(140-140)JobUpdateEventFieldsToUpdateUpdatedAt(152-152)JobAgentConfig(683-685)
apps/workspace-engine/test/e2e/engine_deployment_test.go (3)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
JobAgentConfig(683-685)JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateCompletedAt(140-140)Deployment(327-336)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)apps/workspace-engine/test/integration/creators/deployment.go (1)
CustomDeploymentJobAgentConfig(11-28)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (6)
Deployment(327-336)JobAgent(674-680)FullJobAgentConfig(567-569)TestRunner(226-226)JobAgentConfig(683-685)Job(657-671)
apps/workspace-engine/pkg/db/deployment_variables_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/relationships/property.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/db/job_agents.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
JobAgentConfig(683-685)JobAgent(674-680)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/releasemanager/policy/evaluator/environmentprogression/passrate_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4)
DeploymentJobAgentConfig(393-395)Deployment(327-336)JobAgentConfig(683-685)FullJobAgentConfig(567-569)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/jobdispatch/argocd.go (1)
apps/workspace-engine/pkg/oapi/oapi.gen.go (8)
JobAgentConfig(683-685)Job(657-671)FullArgoCDJobAgentConfig(525-537)JobUpdateEventFieldsToUpdate(701-701)JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateMetadata(147-147)JobUpdateEventFieldsToUpdateCompletedAt(140-140)JobUpdateEventFieldsToUpdateUpdatedAt(152-152)
apps/workspace-engine/pkg/db/deployment_versions_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (1)
JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/api/src/types/openapi.ts (2)
packages/workspace-engine-sdk/src/schema.ts (1)
components(1037-1951)apps/web/app/api/openapi.ts (1)
components(283-599)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go (4)
apps/workspace-engine/pkg/statechange/statechange.go (1)
NewChangeSet(39-43)apps/workspace-engine/pkg/oapi/oapi.gen.go (6)
Release(878-885)JobStatusPending(133-133)FullGithubJobAgentConfigType(564-564)FullArgoCDJobAgentConfigType(540-540)FullTerraformCloudJobAgentConfigType(591-591)TestRunnerJobAgentConfigStatus(1113-1113)apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go (1)
NewFactory(27-31)apps/workspace-engine/pkg/workspace/releasemanager/trace/types.go (1)
Status(68-68)
⏰ 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). (7)
- GitHub Check: Typecheck
- GitHub Check: Lint
- GitHub Check: tests
- GitHub Check: build (linux/amd64)
- GitHub Check: workspace-engine-tests
- GitHub Check: build-and-push (linux/amd64)
- GitHub Check: build (linux/amd64)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/trpc/src/routes/job-agents.ts (1)
68-76: Validate consistency between outer type and config.type fields.The input schema has both a
typefield (line 73) and aconfig.typediscriminator (line 74). These fields could mismatch, allowing inconsistent data liketype: "github-app"withconfig.type: "argo-cd".Consider one of these approaches:
Option 1: Remove the outer type field and derive it from config
create: protectedProcedure .input( z.object({ workspaceId: z.uuid(), name: z.string(), - type: z.string(), config: jobAgentConfig, }), ) .mutation(async ({ input }) => { - const data = { ...input, id: uuidv4() }; + const data = { ...input, id: uuidv4(), type: input.config.type };Option 2: Add refinement to validate consistency
create: protectedProcedure .input( z.object({ workspaceId: z.uuid(), name: z.string(), type: z.string(), config: jobAgentConfig, - }), + }).refine((data) => data.type === data.config.type, { + message: "type must match config.type", + }), )apps/web/app/routes/ws/deployments/settings/_components/GithubAgentConfig.tsx (1)
104-119: Type mismatch:workflowIdshould be stored as a number.The
githubAppJobAgentConfigschema definesworkflowIdasz.number(), butSelect'sonValueChangeprovides a string. The value isn't being converted back to a number before callingonChange.Apply this diff to fix the type mismatch:
<Select value={String(value)} onValueChange={onChange}> + <Select value={String(value)} onValueChange={(v) => onChange(Number(v))}>
🧹 Nitpick comments (2)
packages/trpc/src/routes/job-agents.ts (1)
9-36: Document the rationale for maintaining a separate Zod schema.This local discriminated union provides client-side validation, which is valuable. However, since the PR introduces OpenAPI-generated types for job agent configs, maintaining both schemas creates a risk of divergence over time.
If the workspace-engine SDK exports Zod schemas or type guards based on the OpenAPI spec, consider importing and reusing them. Otherwise, add a comment explaining that this schema must be kept in sync with the OpenAPI definitions.
+// Local validation schema for job agent configs. +// IMPORTANT: Keep in sync with OpenAPI schema definitions in workspace-engine. const jobAgentConfig = z.discriminatedUnion("type", [packages/trpc/src/routes/deployments.ts (1)
260-260: Consider applying the same typed validation to version job agent configs.The
createVersionmutation still usesz.record(z.string(), z.any())forjobAgentConfig, whileupdateJobAgentnow uses the discriminated union schema. If version-level job agent configs should follow the same structure, apply the same schema for consistency.If the structures should match, apply this diff:
- jobAgentConfig: z.record(z.string(), z.any()).default({}), + jobAgentConfig: deploymentJobAgentConfig.default({ type: "custom" }),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
apps/api/src/routes/v1/workspaces/deployments.ts(2 hunks)apps/web/app/routes/ws/deployments/settings/_components/ArgoCD.tsx(3 hunks)apps/web/app/routes/ws/deployments/settings/_components/GithubAgentConfig.tsx(4 hunks)apps/web/app/routes/ws/deployments/settings/_hooks/job-agents.ts(2 hunks)apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.ts(1 hunks)apps/web/app/routes/ws/deployments/settings/page.$deploymentId.job-agent.tsx(2 hunks)apps/web/app/routes/ws/runners/ArgoCD.tsx(1 hunks)packages/trpc/src/routes/deployments.ts(4 hunks)packages/trpc/src/routes/github.ts(1 hunks)packages/trpc/src/routes/job-agents.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript with explicit types (prefer interfaces for public APIs)
Import styles: Use named imports, group imports by source (std lib > external > internal)
Consistent type imports:import type { Type } from "module"
Prefer async/await over raw promises
Handle errors explicitly (use try/catch and typed error responses)
Files:
apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.tspackages/trpc/src/routes/job-agents.tsapps/web/app/routes/ws/runners/ArgoCD.tsxapps/web/app/routes/ws/deployments/settings/page.$deploymentId.job-agent.tsxapps/api/src/routes/v1/workspaces/deployments.tsapps/web/app/routes/ws/deployments/settings/_components/ArgoCD.tsxapps/web/app/routes/ws/deployments/settings/_components/GithubAgentConfig.tsxpackages/trpc/src/routes/github.tspackages/trpc/src/routes/deployments.tsapps/web/app/routes/ws/deployments/settings/_hooks/job-agents.ts
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
Files:
apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.tspackages/trpc/src/routes/job-agents.tsapps/web/app/routes/ws/runners/ArgoCD.tsxapps/web/app/routes/ws/deployments/settings/page.$deploymentId.job-agent.tsxapps/api/src/routes/v1/workspaces/deployments.tsapps/web/app/routes/ws/deployments/settings/_components/ArgoCD.tsxapps/web/app/routes/ws/deployments/settings/_components/GithubAgentConfig.tsxpackages/trpc/src/routes/github.tspackages/trpc/src/routes/deployments.tsapps/web/app/routes/ws/deployments/settings/_hooks/job-agents.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.tspackages/trpc/src/routes/job-agents.tsapps/web/app/routes/ws/runners/ArgoCD.tsxapps/web/app/routes/ws/deployments/settings/page.$deploymentId.job-agent.tsxapps/api/src/routes/v1/workspaces/deployments.tsapps/web/app/routes/ws/deployments/settings/_components/ArgoCD.tsxapps/web/app/routes/ws/deployments/settings/_components/GithubAgentConfig.tsxpackages/trpc/src/routes/github.tspackages/trpc/src/routes/deployments.tsapps/web/app/routes/ws/deployments/settings/_hooks/job-agents.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 637
File: packages/events/src/kafka/client.ts:10-16
Timestamp: 2025-08-01T04:41:41.345Z
Learning: User adityachoudhari26 prefers not to add null safety checks for required environment variables when they are guaranteed to be present in their deployment configuration, similar to their preference for simplicity over defensive programming in test code.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.
📚 Learning: 2025-04-12T22:08:13.790Z
Learnt from: zacharyblasczyk
Repo: ctrlplanedev/ctrlplane PR: 408
File: apps/webservice/src/components/form/job-agent/JobAgentJenkinsPipelineConfig.tsx:26-31
Timestamp: 2025-04-12T22:08:13.790Z
Learning: For Jenkins job configuration, two approaches are needed: (1) a simple URL input form for airgapped environments (current focus) and (2) a dropdown selection interface for non-airgapped environments where the Jenkins server is accessible. A component similar to DeploymentJobAgentGithubConfig.tsx is preferred.
Applied to files:
apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.tsapps/web/app/routes/ws/deployments/settings/page.$deploymentId.job-agent.tsxapps/web/app/routes/ws/deployments/settings/_components/ArgoCD.tsxapps/web/app/routes/ws/deployments/settings/_components/GithubAgentConfig.tsxapps/web/app/routes/ws/deployments/settings/_hooks/job-agents.ts
📚 Learning: 2024-10-30T23:10:58.869Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 187
File: apps/jobs/src/ephemeral-env-checker/index.ts:57-0
Timestamp: 2024-10-30T23:10:58.869Z
Learning: In the codebase, deployments are decoupled from environments. When deleting environments (e.g., in `apps/jobs/src/ephemeral-env-checker/index.ts`), associated deployments should not be deleted.
Applied to files:
apps/api/src/routes/v1/workspaces/deployments.tspackages/trpc/src/routes/deployments.ts
📚 Learning: 2025-04-28T18:41:58.813Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 515
File: apps/webservice/src/app/api/v1/release-targets/[releaseTargetId]/releases/route.ts:103-108
Timestamp: 2025-04-28T18:41:58.813Z
Learning: In this project, full records from the `deployment` and `deployment_version` tables are considered safe for public API consumption, and there's no need to create restricted DTOs for them.
Applied to files:
apps/api/src/routes/v1/workspaces/deployments.ts
📚 Learning: 2025-05-30T21:48:48.868Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 578
File: apps/webservice/src/app/[workspaceSlug]/(app)/resources/(sidebar)/providers/integrations/github/GithubDialog.tsx:58-58
Timestamp: 2025-05-30T21:48:48.868Z
Learning: In the ctrlplane codebase, the shadcn form UI version allows initializing forms with `useForm({ schema: formSchema })` directly, without needing to import and use `zodResolver` from 'hookform/resolvers/zod'. This is different from standard react-hook-form usage.
Applied to files:
apps/web/app/routes/ws/deployments/settings/_components/ArgoCD.tsxapps/web/app/routes/ws/deployments/settings/_components/GithubAgentConfig.tsxapps/web/app/routes/ws/deployments/settings/_hooks/job-agents.ts
📚 Learning: 2024-11-03T00:15:33.442Z
Learnt from: zacharyblasczyk
Repo: ctrlplanedev/ctrlplane PR: 196
File: integrations/github-get-job-inputs/src/index.ts:39-39
Timestamp: 2024-11-03T00:15:33.442Z
Learning: In `integrations/github-get-job-inputs/src/sdk.ts`, the API client is properly configured with the `baseUrl` input.
Applied to files:
apps/web/app/routes/ws/deployments/settings/_components/GithubAgentConfig.tsx
📚 Learning: 2024-11-27T23:18:42.055Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 237
File: packages/api/src/router/job.ts:362-365
Timestamp: 2024-11-27T23:18:42.055Z
Learning: In the file `packages/api/src/router/job.ts`, the function `releaseMatchesCondition` returns undefined if the `filter` parameter is null. This behavior ensures that when constructing the query with `and(...)`, the condition is omitted, allowing the query to function correctly even if there is no release channel associated with the environment.
Applied to files:
packages/trpc/src/routes/github.ts
📚 Learning: 2024-10-29T02:04:50.312Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 181
File: packages/api/src/router/deployment.ts:116-131
Timestamp: 2024-10-29T02:04:50.312Z
Learning: In `packages/api/src/router/deployment.ts`, the `list.byDeploymentId` procedure requires multiple database queries due to limitations of the `releaseMatchesCondition` function.
Applied to files:
packages/trpc/src/routes/deployments.ts
📚 Learning: 2024-11-01T02:37:25.510Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 188
File: packages/api/src/router/deployment.ts:144-161
Timestamp: 2024-11-01T02:37:25.510Z
Learning: In `packages/api/src/router/deployment.ts`, when using Drizzle ORM, there is a limitation when referencing the same table twice in a relational builder query (rbq), requiring separate queries to avoid issues.
Applied to files:
packages/trpc/src/routes/deployments.ts
🧬 Code graph analysis (4)
apps/web/app/routes/ws/deployments/settings/page.$deploymentId.job-agent.tsx (1)
apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.ts (1)
deploymentJobAgentConfig(24-29)
apps/web/app/routes/ws/deployments/settings/_components/GithubAgentConfig.tsx (1)
apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.ts (2)
deploymentJobAgentConfig(24-29)githubAppJobAgentConfig(3-8)
packages/trpc/src/routes/github.ts (1)
packages/db/src/schema/job-agent.ts (1)
jobAgent(10-22)
packages/trpc/src/routes/deployments.ts (2)
apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.ts (1)
deploymentJobAgentConfig(24-29)packages/db/src/schema/deployment.ts (1)
deployment(69-93)
⏰ 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). (7)
- GitHub Check: workspace-engine-tests
- GitHub Check: tests
- GitHub Check: build (linux/amd64)
- GitHub Check: build-and-push (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: build (linux/amd64)
- GitHub Check: Lint
🔇 Additional comments (21)
apps/api/src/routes/v1/workspaces/deployments.ts (3)
83-83: LGTM: jobAgentConfig now uses typed discriminator.The change from
{}to{ type: "custom" }correctly provides a valid discriminated union default for the DeploymentJobAgentConfig schema.
101-101: LGTM: Fallback now provides valid discriminated union default.The change from
body.jobAgentConfig ?? {}tobody.jobAgentConfig ?? { type: "custom" }ensures the fallback value is a valid DeploymentJobAgentConfig when not provided in the request body.
177-177: No changes needed. Line 177 is correct as-is.Deployment versions use a different schema than deployments. While deployments require a discriminated union with
{ type: "custom" }(line 101), deployment versions accept a generic record that defaults to{}(line 260). Both fallback values are correct for their respective validators.Likely an incorrect or invalid review comment.
apps/web/app/routes/ws/runners/ArgoCD.tsx (1)
47-51: LGTM!The config object now correctly includes the
type: "argo-cd"discriminator field, aligning with the new discriminated union schema for job agent configurations.apps/web/app/routes/ws/deployments/settings/page.$deploymentId.job-agent.tsx (3)
31-35: LGTM!The import and schema update correctly adopt the discriminated union type for job agent configurations.
79-104: LGTM!Removing the field destructuring is appropriate since child components (
GithubAgentConfig,ArgoCDConfig) handle their own form field bindings internally.
110-116: Verify empty default aligns with discriminated union.The fallback
deployment.jobAgentConfig ?? {}doesn't include the requiredtypediscriminator field thatdeploymentJobAgentConfigexpects. While child components may handle this gracefully, consider whether an explicit nullable type or conditional initialization would be safer.apps/web/app/routes/ws/deployments/settings/_components/ArgoCD.tsx (4)
8-11: LGTM!Importing both schemas enables proper typing for the form and the ArgoCD-specific narrowed type.
56-68: LGTM!The separate
argoFormSchemaandArgoFormtype enable type-safe access to ArgoCD-specific config fields liketemplate.
72-75: Type narrowing via cast is acceptable here.The
as unknown as ArgoFormcast is safe in this context sinceArgoCDConfigis only rendered whenselectedJobAgent?.type === "argo-cd", guaranteeing the config shape matches.
174-175: LGTM!Correctly emitting the full discriminated object
{ type: "argo-cd", template: newValue }ensures the config maintains its discriminator when updated.apps/web/app/routes/ws/deployments/settings/_components/GithubAgentConfig.tsx (2)
21-38: LGTM!The schema definitions and
GithubFormtype correctly enable type-safe access to GitHub-specific config fields.
128-135: Type narrowing via cast is acceptable here.Similar to the ArgoCD component, this cast is safe since
GithubAgentConfigis only rendered whenselectedJobAgent?.type === "github-app".apps/web/app/routes/ws/deployments/settings/_hooks/job-agents.ts (2)
1-1: LGTM!Updated imports support the new generic type parameters.
26-36: LGTM!Making
useSelectedJobAgentgeneric with the constraintT extends FieldValues & { jobAgentId: string }is a clean solution that allows the hook to work with multiple form schemas while ensuring type safety for the requiredjobAgentIdfield.packages/trpc/src/routes/deployments.ts (2)
220-220: LGTM: Improved type safety for job agent configuration.The replacement of
z.record(z.string(), z.any())with the discriminated union provides proper validation and type safety. The direct passthrough ofjobAgentConfigis cleaner than the previous transformation approach.Also applies to: 235-235
152-152: Align event data with discriminated union schema requirements.The
jobAgentConfighas been updated from{}(invalid for the discriminated union) to{ type: "custom" }(valid default variant). This change aligns the DeploymentCreated event with the API's existing behavior and matches the schema definition. No downstream impact expected—consumers already validate this field against their own requirements.apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.ts (4)
10-13: LGTM: ArgoCD config schema is appropriately minimal.The schema correctly defines the ArgoCD job agent configuration with a template field.
15-18: LGTM: Terraform Cloud config schema is clear.The schema appropriately defines the Terraform Cloud job agent configuration.
20-29: LGTM: Well-structured discriminated union with appropriate custom type handling.The use of
.passthrough()on the custom config variant correctly allows arbitrary additional properties while maintaining type discrimination. The discriminated union is properly structured and provides type-safe validation for all job agent configuration variants.
3-8: No precision issue withworkflowIdasz.number().GitHub workflow IDs in API responses use numeric values well within JavaScript's safe integer range, with no indication from GitHub's documentation that this will change. The safe integer concern is theoretical; practical examples show no risk.
| const deploymentJobAgentConfig = z.discriminatedUnion("type", [ | ||
| z.object({ | ||
| type: z.literal("github-app"), | ||
| repo: z.string(), | ||
| workflowId: z.number(), | ||
| ref: z.string().optional(), | ||
| }), | ||
| z.object({ | ||
| type: z.literal("argo-cd"), | ||
| template: z.string(), | ||
| }), | ||
| z.object({ | ||
| type: z.literal("tfe"), | ||
| template: z.string(), | ||
| }), | ||
| z.object({ type: z.literal("custom") }).passthrough(), | ||
| ]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Eliminate code duplication by importing the shared schema.
This schema is duplicated from apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.ts. Import the exported schema instead to maintain a single source of truth.
Apply this diff:
+import { deploymentJobAgentConfig } from "apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.js";
+
-const deploymentJobAgentConfig = z.discriminatedUnion("type", [
- z.object({
- type: z.literal("github-app"),
- repo: z.string(),
- workflowId: z.number(),
- ref: z.string().optional(),
- }),
- z.object({
- type: z.literal("argo-cd"),
- template: z.string(),
- }),
- z.object({
- type: z.literal("tfe"),
- template: z.string(),
- }),
- z.object({ type: z.literal("custom") }).passthrough(),
-]);
-📝 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.
| const deploymentJobAgentConfig = z.discriminatedUnion("type", [ | |
| z.object({ | |
| type: z.literal("github-app"), | |
| repo: z.string(), | |
| workflowId: z.number(), | |
| ref: z.string().optional(), | |
| }), | |
| z.object({ | |
| type: z.literal("argo-cd"), | |
| template: z.string(), | |
| }), | |
| z.object({ | |
| type: z.literal("tfe"), | |
| template: z.string(), | |
| }), | |
| z.object({ type: z.literal("custom") }).passthrough(), | |
| ]); | |
| import { deploymentJobAgentConfig } from "apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.js"; | |
| // ... rest of the file |
🤖 Prompt for AI Agents
In packages/trpc/src/routes/deployments.ts lines 12-28, the
deploymentJobAgentConfig zod schema is duplicated; replace this inline schema
with an import of the shared schema exported from
apps/web/app/routes/ws/deployments/settings/deploymentJobAgentConfig.ts. Remove
the local declaration and add a single import statement that references the
exported schema (ensure correct relative path and exported name), then use the
imported symbol in place of the current variable so the module relies on the
single source-of-truth schema.
packages/trpc/src/routes/github.ts
Outdated
| const config = jobAgent.config as { installationId: number }; | ||
| const { installationId } = config; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and examine the github.ts file
find . -name "github.ts" -path "*/trpc/*" | head -5Repository: ctrlplanedev/ctrlplane
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Read the github.ts file with line numbers
cat -n ./packages/trpc/src/routes/github.ts | head -80Repository: ctrlplanedev/ctrlplane
Length of output: 3312
🏁 Script executed:
#!/bin/bash
# Search for FullGithubJobAgentConfig or GithubJobAgentConfig type definitions
rg "FullGithubJobAgentConfig|GithubJobAgentConfig" --type=ts --type=tsx -nRepository: ctrlplanedev/ctrlplane
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Search for FullGithubJobAgentConfig or GithubJobAgentConfig type definitions
rg "FullGithubJobAgentConfig|GithubJobAgentConfig" -n --type tsRepository: ctrlplanedev/ctrlplane
Length of output: 2065
🏁 Script executed:
#!/bin/bash
# Read the schema.ts file to see the actual type definitions
sed -n '1320,1370p' ./packages/workspace-engine-sdk/src/schema.tsRepository: ctrlplanedev/ctrlplane
Length of output: 2152
🏁 Script executed:
#!/bin/bash
# Check the exports from workspace-engine-sdk
head -100 ./packages/workspace-engine-sdk/src/schema.tsRepository: ctrlplanedev/ctrlplane
Length of output: 2403
🏁 Script executed:
#!/bin/bash
# Check what WorkspaceEngine exports from workspace-engine-sdk
grep -n "export" ./packages/workspace-engine-sdk/src/index.ts | head -30Repository: ctrlplanedev/ctrlplane
Length of output: 381
🏁 Script executed:
#!/bin/bash
# Check what type the GET endpoint returns
rg -A 20 "/job-agents/{jobAgentId}" ./packages/workspace-engine-sdk/src/schema.ts | head -40Repository: ctrlplanedev/ctrlplane
Length of output: 185
🏁 Script executed:
#!/bin/bash
# Search for job-agents endpoint in schema
rg "job-agents" ./packages/workspace-engine-sdk/src/schema.ts -A 20 | head -60Repository: ctrlplanedev/ctrlplane
Length of output: 1383
🏁 Script executed:
#!/bin/bash
# Search for getJobAgent operation definition
rg -A 30 '"getJobAgent"' ./packages/workspace-engine-sdk/src/schema.ts | head -50Repository: ctrlplanedev/ctrlplane
Length of output: 760
🏁 Script executed:
#!/bin/bash
# Search for the getJobAgent operation in the operations section
rg "getJobAgent:" ./packages/workspace-engine-sdk/src/schema.ts -A 25 | head -50Repository: ctrlplanedev/ctrlplane
Length of output: 696
🏁 Script executed:
#!/bin/bash
# Search for JobAgent schema definition
rg "JobAgent:" ./packages/workspace-engine-sdk/src/schema.ts -A 30 | grep -A 30 "^\s*JobAgent:" | head -50Repository: ctrlplanedev/ctrlplane
Length of output: 943
🏁 Script executed:
#!/bin/bash
# Verify FullGithubJobAgentConfig is accessible through WorkspaceEngine
grep -n "export.*components" ./packages/workspace-engine-sdk/src/index.tsRepository: ctrlplanedev/ctrlplane
Length of output: 128
Use generated OpenAPI types instead of unsafe type assertion.
The inline type assertion as { installationId: number } defeats the purpose of using strongly-typed discriminated unions from the OpenAPI schema. Replace it with the proper type:
- const config = jobAgent.config as { installationId: number };
+ const config = jobAgent.config as WorkspaceEngine["schemas"]["FullGithubJobAgentConfig"];
const { installationId } = config;The FullGithubJobAgentConfig type is generated from the OpenAPI schema and provides complete type safety, including the discriminator guarantee that type === "github-app" and the installationId property.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/trpc/src/routes/github.ts around lines 59-60, replace the unsafe
inline assertion by using the generated FullGithubJobAgentConfig type: import
the FullGithubJobAgentConfig type from the OpenAPI-generated types, narrow or
guard that jobAgent.config.type === "github-app" (or declare config:
FullGithubJobAgentConfig when that is already guaranteed) and then extract const
{ installationId } = config so you rely on the generated discriminated-union
type instead of using "as { installationId: number }".
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/trpc/src/routes/github.ts (1)
59-62: Add type assertion to leverage discriminated union types.The runtime guard on line 60 doesn't provide TypeScript type narrowing without a proper type assertion. To fully leverage the discriminated union types from the OpenAPI schema migration, assert the config type:
- const config = jobAgent.config; - if (config.type !== "github-app") - throw new Error("Job agent is not a GitHub app"); + const config = jobAgent.config as WorkspaceEngine["schemas"]["FullGithubJobAgentConfig"]; + if (config.type !== "github-app") + throw new Error("Job agent is not a GitHub app"); const { installationId } = config;This ensures TypeScript recognizes
installationIdas a valid property and provides compile-time type safety consistent with the PR's objectives.Based on prior review feedback suggesting the same approach.
🧹 Nitpick comments (2)
apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go (1)
33-120: Well-structured config merging with proper discriminator handling.The
mergeJobAgentConfigmethod cleanly consolidates the merge logic with appropriate error propagation. The special-casing forTestRunner(lines 40-42) that skips deployment config validation is clear and matches the test expectations.One minor observation: the
toMaphelper (lines 64-90) handles bothmap[string]anyandmap[string]interface{}separately, but in Go these are identical types. The second branch (lines 71-77) will never execute.toMap := func(v any) (map[string]any, error) { if v == nil { return map[string]any{}, nil } if m, ok := v.(map[string]any); ok { return m, nil } - if m, ok := v.(map[string]interface{}); ok { - out := make(map[string]any, len(m)) - for k, vv := range m { - out[k] = vv - } - return out, nil - } b, err := json.Marshal(v)packages/trpc/src/routes/github.ts (1)
56-61: Remove redundant type check.Both
jobAgent.type(line 56) andconfig.type(line 60) are checked sequentially. Sinceconfigis directly extracted fromjobAgent.config, these checks should always yield the same result. One check is sufficient.Apply this diff to remove the redundant check:
if (jobAgent?.type !== "github-app") throw new Error("Job agent is not a GitHub app"); const config = jobAgent.config; - if (config.type !== "github-app") - throw new Error("Job agent is not a GitHub app"); const { installationId } = config;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
apps/workspace-engine/pkg/events/handler/deployment/deployment.go(1 hunks)apps/workspace-engine/pkg/events/handler/resources/resourceproviders_bench_test.go(2 hunks)apps/workspace-engine/pkg/workspace/jobdispatch/testrunner.go(3 hunks)apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.go(12 hunks)apps/workspace-engine/pkg/workspace/relationships/property.go(1 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go(5 hunks)apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go(1 hunks)apps/workspace-engine/test/e2e/engine_job_agent_test.go(8 hunks)packages/trpc/src/routes/github.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/workspace-engine/pkg/workspace/relationships/property.go
🧰 Additional context used
📓 Path-based instructions (4)
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/events/handler/resources/resourceproviders_bench_test.goapps/workspace-engine/pkg/workspace/jobdispatch/testrunner.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/events/handler/deployment/deployment.goapps/workspace-engine/test/e2e/engine_job_agent_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go
apps/workspace-engine/**/*_test.go
📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)
Follow the existing test structure used in *_test.go files
Files:
apps/workspace-engine/pkg/events/handler/resources/resourceproviders_bench_test.goapps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/test/e2e/engine_job_agent_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript with explicit types (prefer interfaces for public APIs)
Import styles: Use named imports, group imports by source (std lib > external > internal)
Consistent type imports:import type { Type } from "module"
Prefer async/await over raw promises
Handle errors explicitly (use try/catch and typed error responses)
Files:
packages/trpc/src/routes/github.ts
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
Files:
packages/trpc/src/routes/github.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Formatting: Prettier is used with
@ctrlplane/prettier-config
Files:
packages/trpc/src/routes/github.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 637
File: packages/events/src/kafka/client.ts:10-16
Timestamp: 2025-08-01T04:41:41.345Z
Learning: User adityachoudhari26 prefers not to add null safety checks for required environment variables when they are guaranteed to be present in their deployment configuration, similar to their preference for simplicity over defensive programming in test code.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/**/*_test.go : Follow the existing test structure used in *_test.go files
Applied to files:
apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/test/e2e/engine_job_agent_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Test validation and matching logic separately for condition types
Applied to files:
apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Write comprehensive, data-driven tests for new condition types
Applied to files:
apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Include edge cases in tests (empty values, special characters, unicode) for condition types
Applied to files:
apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/test/e2e/engine_job_agent_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/pkg/model/selector/**/*_test.go : Use table-driven tests for all condition types
Applied to files:
apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2025-08-12T20:49:05.086Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 655
File: apps/workspace-engine/pkg/engine/workspace/fluent.go:166-171
Timestamp: 2025-08-12T20:49:05.086Z
Learning: The UpdateDeploymentVersions() method with OperationCreate case in apps/workspace-engine/pkg/engine/workspace/fluent.go is specifically designed only for creating new deployment versions, not for handling potential duplicates or existing versions.
Applied to files:
apps/workspace-engine/pkg/events/handler/deployment/deployment.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go
📚 Learning: 2024-10-30T23:10:58.869Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 187
File: apps/jobs/src/ephemeral-env-checker/index.ts:57-0
Timestamp: 2024-10-30T23:10:58.869Z
Learning: In the codebase, deployments are decoupled from environments. When deleting environments (e.g., in `apps/jobs/src/ephemeral-env-checker/index.ts`), associated deployments should not be deleted.
Applied to files:
apps/workspace-engine/pkg/events/handler/deployment/deployment.go
📚 Learning: 2024-11-27T23:16:35.580Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 237
File: apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx:43-50
Timestamp: 2024-11-27T23:16:35.580Z
Learning: In `DeploymentResourceDrawer.tsx`, the `isOpen` variable already checks whether `deploymentId`, `environmentId`, and `resourceId` are non-null, so additional null checks in query `enabled` conditions are not necessary.
Applied to files:
apps/workspace-engine/pkg/events/handler/deployment/deployment.go
📚 Learning: 2025-08-12T18:13:54.630Z
Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: apps/workspace-engine/CLAUDE.md:0-0
Timestamp: 2025-08-12T18:13:54.630Z
Learning: Applies to apps/workspace-engine/**/*.go : Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods
Applied to files:
apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.goapps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go
📚 Learning: 2024-11-03T00:15:33.442Z
Learnt from: zacharyblasczyk
Repo: ctrlplanedev/ctrlplane PR: 196
File: integrations/github-get-job-inputs/src/index.ts:39-39
Timestamp: 2024-11-03T00:15:33.442Z
Learning: In `integrations/github-get-job-inputs/src/sdk.ts`, the API client is properly configured with the `baseUrl` input.
Applied to files:
packages/trpc/src/routes/github.ts
📚 Learning: 2025-07-23T22:42:20.933Z
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 627
File: apps/event-worker/src/workers/job-update/trigger-dependendent-targets.ts:0-0
Timestamp: 2025-07-23T22:42:20.933Z
Learning: In apps/event-worker/src/workers/job-update/trigger-dependendent-targets.ts, when implementing error handling for the triggerDependentTargets function, only a try-catch wrapper is needed around the entire function body. Additional null checks after takeFirst() calls are unnecessary since takeFirst throws an error if no record is found.
Applied to files:
packages/trpc/src/routes/github.ts
📚 Learning: 2025-04-12T22:08:13.790Z
Learnt from: zacharyblasczyk
Repo: ctrlplanedev/ctrlplane PR: 408
File: apps/webservice/src/components/form/job-agent/JobAgentJenkinsPipelineConfig.tsx:26-31
Timestamp: 2025-04-12T22:08:13.790Z
Learning: For Jenkins job configuration, two approaches are needed: (1) a simple URL input form for airgapped environments (current focus) and (2) a dropdown selection interface for non-airgapped environments where the Jenkins server is accessible. A component similar to DeploymentJobAgentGithubConfig.tsx is preferred.
Applied to files:
packages/trpc/src/routes/github.ts
🧬 Code graph analysis (4)
apps/workspace-engine/pkg/events/handler/resources/resourceproviders_bench_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
DeploymentJobAgentConfig(393-395)JobAgentConfig(683-685)apps/workspace-engine/test/integration/opts.go (2)
DeploymentJobAgentConfig(403-411)JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/jobdispatch/testrunner.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (9)
TestRunnerJobAgentConfig(1098-1110)JobStatus(688-688)Failure(221-221)JobAgentConfig(683-685)JobUpdateEventFieldsToUpdate(701-701)JobUpdateEventFieldsToUpdateStatus(150-150)JobUpdateEventFieldsToUpdateMessage(146-146)JobUpdateEventFieldsToUpdateUpdatedAt(152-152)JobUpdateEventFieldsToUpdateCompletedAt(140-140)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.go (2)
apps/workspace-engine/pkg/oapi/oapi.gen.go (2)
JobAgentConfig(683-685)Deployment(327-336)apps/workspace-engine/test/integration/opts.go (1)
JobAgentConfig(702-707)
packages/trpc/src/routes/github.ts (1)
packages/db/src/schema/job-agent.ts (1)
jobAgent(10-22)
⏰ 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). (7)
- GitHub Check: Lint
- GitHub Check: build (linux/amd64)
- GitHub Check: Typecheck
- GitHub Check: build (linux/amd64)
- GitHub Check: tests
- GitHub Check: build-and-push (linux/amd64)
- GitHub Check: workspace-engine-tests
🔇 Additional comments (26)
apps/workspace-engine/pkg/events/handler/deployment/deployment.go (1)
162-172: Behavioral change appears intentional; redundant condition removed.The redundant
if oldDeployment == nil && newDeployment == nilbranch from the past review has been removed. The simplified logic now returnstruewhenoldDeployment == nil(andnewDeployment != nil), treating any initial deployment as a config change regardless of theJobAgentConfigvalue.This broader detection aligns with the PR's typed discriminated-union approach for
JobAgentConfig. The downstreamredeployTargetsfunction already guards withdeployment.JobAgentId != nilchecks, so no unintended actions occur for deployments without a job agent.apps/workspace-engine/pkg/workspace/jobdispatch/testrunner.go (5)
63-68: LGTM: Clean migration to typed config.The signature change from the untyped
testRunnerJobConfigto the OpenAPI-generatedoapi.TestRunnerJobAgentConfigimproves type safety while preserving the original logic.
70-75: LGTM: Type-safe status mapping.The refactoring correctly uses the generated
oapi.Failureconstant and preserves the original status resolution logic.
85-90: LGTM: Discriminated union accessor replaces JSON parsing.The migration from manual JSON unmarshalling to the type-safe
AsFullTestRunnerJobAgentConfig()method aligns with the PR objective and includes proper error handling.
99-102: LGTM: Delay attribute now correctly reports seconds.The
delay_secondsattribute now usesint64(delay.Seconds()), which correctly reports the delay in seconds and addresses the past review comment.
194-201: LGTM: Explicit field constants improve maintainability.Using the generated
oapi.JobUpdateEventFieldsToUpdate*constants instead of string literals reduces the risk of typos and improves refactoring safety.apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory.go (2)
206-233: Previous review concern addressed.The redundant JSON round-trip has been removed.
mergedConfigis now used directly in theJobstruct (line 233), which is the correct approach.
242-252: LGTM!The
deepMergeimplementation correctly handles recursive map merging while overwriting non-map values. The behavior of overwriting slices (rather than merging them) is the expected default for config merging.apps/workspace-engine/pkg/workspace/releasemanager/deployment/jobs/factory_test.go (6)
19-90: LGTM!Well-structured test helpers following existing test patterns. The use of
t.Helper()ensures proper error attribution, and the data builders are appropriately minimal.
96-191: LGTM!Good coverage of GitHub App config merging scenarios with proper verification of merged fields from both JobAgent and Deployment sources.
459-521: LGTM!The deep nested merge test effectively validates the recursive merge behavior, ensuring nested fields are properly merged while preserving values not overridden by the deployment config.
627-675: Previous review concern addressed.The TestRunner status value is now correctly set to
"completed"(line 637) and properly asserted (line 672), matching the valid enum values defined in the schema (['completed', 'failure']).
527-621: LGTM!Comprehensive error case coverage including discriminator mismatch, missing job agent configuration, non-existent job agent, and missing deployment scenarios. The assertions properly verify both error returns and status assignments.
681-802: LGTM!Thorough testing of job field initialization, UUID uniqueness across multiple creations, and edge case handling for empty job agent IDs.
apps/workspace-engine/test/e2e/engine_job_agent_test.go (7)
195-214: LGTM!The typed config accessor pattern with error handling and appropriate format specifiers is correct. Using
AsFullCustomJobAgentConfig()for job-level config access is consistent with the discriminator-based union schema.
221-225: LGTM!The config construction using
c.CustomJobAgentConfig()wrapper aligns with the typed config pattern introduced in this refactor.
246-261: LGTM!The format specifier issue from the previous review has been addressed. Using
%vfor the float64 values (timeout, retries) will now produce correct error messages if assertions fail.
310-320: LGTM!The nested config structure with
resourcesmap is properly wrapped using the typed constructor, demonstrating that the pattern handles complex configurations correctly.
330-363: LGTM!The use of
AsCustomJobAgentConfig()forJobAgent.Config(vsAsFullCustomJobAgentConfig()forJob.JobAgentConfig) is consistent with the discriminated union schema design. The type assertions withokchecks provide clear test failure messages.
377-409: LGTM!The config update test properly exercises the full lifecycle with typed config wrappers, including verification that new fields can be added during updates.
641-658: LGTM!Testing the empty/nil config edge case ensures the typed wrapper handles this scenario gracefully. The assertion correctly verifies that
AdditionalPropertiesremains empty.apps/workspace-engine/pkg/events/handler/resources/resourceproviders_bench_test.go (2)
61-77: LGTM! Critical issue resolved.The infinite recursion issue identified in the previous review has been fixed. The function now correctly marshals the payload with the "custom" discriminator and unmarshals it into a
DeploymentJobAgentConfig, providing proper support for strongly-typed job agent configurations in benchmark tests.
94-94: LGTM!The use of
customJobAgentConfig(nil)properly creates a strongly-typed deployment config with the "custom" discriminator, aligning with the PR's objective to replace untyped maps with discriminated union schemas.apps/workspace-engine/pkg/workspace/relationships/compute/rule_test.go (3)
39-39: LGTM: Consistent use of helper for test deployments.The refactoring to use
customJobAgentConfig(nil)provides a clean, consistent pattern across all test cases that don't require specific configuration values. This improves maintainability and aligns with the typed configuration approach.Also applies to: 46-46, 120-120, 249-249, 357-357, 473-473, 480-480, 555-555, 604-604, 650-650, 729-729, 782-782
178-178: LGTM: Correct use of helper with test-specific configuration.The helper is properly used with specific
workspaceIdvalues needed for the CEL matcher test, enabling validation of the expression"from.workspaceId == to.jobAgentConfig.workspaceId".Also applies to: 185-185
218-220: LGTM: Proper error handling and typed access pattern.The error handling correctly uses
require.NoError(t, err)(addressing the previous review concern), and the typed accessorAsDeploymentCustomJobAgentConfig()followed byAdditionalPropertiesaccess properly implements the new discriminated config API.
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.