feat: redeploy controller#990
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 24 minutes and 52 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdded a complete force-deploy reconciliation system to workspace-engine that processes force-deploy work items, creates jobs via agent matching and configuration merging, and dispatches them to agents. Refactored the existing redeploy route to use the new Changes
Sequence DiagramsequenceDiagram
actor API
participant Queue as Reconcile Queue
participant Worker as Workspace Engine<br/>Force-Deploy Worker
participant DB as Database
participant Dispatch as Job Dispatch<br/>Queue
API->>DB: enqueueForceDeploy(workspaceId,<br/>deploymentId, environmentId,<br/>resourceId)
DB->>Queue: Enqueue force-deploy item
Worker->>Queue: Poll work item
Queue-->>Worker: force-deploy item
Worker->>DB: Fetch desired release
alt No desired release
Worker-->>Worker: Return (no action)
else Has desired release
Worker->>DB: Get active jobs for<br/>release target
alt Active jobs exist
Worker-->>Worker: Requeue after 5s
else No active jobs
Worker->>DB: Load deployment &<br/>resource
Worker->>DB: List job agents for<br/>workspace
Worker->>Worker: Match agents via<br/>selector
Worker->>Worker: Merge job-agent<br/>configs
loop For each matched agent
Worker->>DB: CreateJob(agent config)
DB->>DB: Insert job & job-release
DB-->>Worker: Job created
Worker->>Dispatch: EnqueueJobDispatch
end
Worker-->>Worker: Return success
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds a new “force-deploy” reconcile workflow in workspace-engine and updates the tRPC redeploy routes to enqueue that work, shifting redeploy behavior from “clone last job” logic to a dedicated controller that creates and dispatches fresh jobs for the desired release target.
Changes:
- Replace tRPC redeploy logic with
enqueueForceDeployreconcile enqueuing. - Add
enqueueForceDeployreconciler helper inpackages/dband export it. - Implement a new
forcedeploycontroller inworkspace-engine(getter/setter, reconcile + tests) and wire it intomain.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/trpc/src/routes/redeploy.ts | Switch redeploy endpoints to enqueue force-deploy reconcile work. |
| packages/db/src/reconcilers/index.ts | Export the new force-deploy reconciler helper. |
| packages/db/src/reconcilers/force-deploy.ts | Add enqueueForceDeploy helper to enqueue force-deploy work for a release-target scope. |
| apps/workspace-engine/svc/controllers/forcedeploy/setters_postgres.go | Persist created jobs + release_job rows and enqueue job-dispatch work. |
| apps/workspace-engine/svc/controllers/forcedeploy/setters.go | Define setter interface for job persistence + dispatch enqueueing. |
| apps/workspace-engine/svc/controllers/forcedeploy/releasetarget.go | Parse release-target scope IDs and convert to OAPI model. |
| apps/workspace-engine/svc/controllers/forcedeploy/reconcile_test.go | Add unit tests for forcedeploy reconcile/controller behavior. |
| apps/workspace-engine/svc/controllers/forcedeploy/reconcile.go | Implement reconcile logic: block on active jobs, select agents, create + dispatch jobs. |
| apps/workspace-engine/svc/controllers/forcedeploy/getters_postgres.go | Implement Postgres-backed getters for release target existence, desired release, active jobs, etc. |
| apps/workspace-engine/svc/controllers/forcedeploy/getters.go | Define getter interface for forcedeploy reconcile. |
| apps/workspace-engine/svc/controllers/forcedeploy/controller.go | Add reconcile worker/processor wiring for the force-deploy kind. |
| apps/workspace-engine/pkg/reconcile/events/forcedeploy.go | Define ForceDeployKind constant. |
| apps/workspace-engine/main.go | Register the new forcedeploy worker in the service startup list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const redeployRouter = router({ | ||
| releaseTarget: protectedProcedure |
There was a problem hiding this comment.
In this router, the releaseTargets mutation (defined further down in this file) returns releaseTargets.map(...), which produces an array of Promises rather than awaiting them. tRPC will attempt to serialize the Promise objects instead of resolved ReconcileWorkScopes, and enqueue failures won’t be propagated. Make the resolver async and await Promise.all(...) so results/errors are handled correctly.
| export const redeployRouter = router({ | ||
| releaseTarget: protectedProcedure |
There was a problem hiding this comment.
The releaseTargets mutation input (defined further down in this file) validates workspaceId and the IDs inside releaseTargets as plain strings. Since these IDs are used to build the reconcile scopeId and later parsed as UUIDs by the workspace-engine controller, invalid input will turn into retrying reconcile errors. Validate these as UUIDs (e.g. z.uuid()) to fail fast at the API boundary.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/workspace-engine/svc/controllers/forcedeploy/getters_postgres.go (1)
15-15: Document the exportedPostgresGettertype.Please add a doc comment describing that this is the Postgres-backed read side for force-deploy reconciliation.
As per coding guidelines, "Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/svc/controllers/forcedeploy/getters_postgres.go` at line 15, Add a doc comment immediately above the exported type PostgresGetter describing that it implements the Postgres-backed read side for force-deploy reconciliation (e.g., "PostgresGetter is the Postgres-backed read side used by the force-deploy reconciliation logic."). Keep the comment concise, present tense, and document the purpose/role of PostgresGetter so it meets the exported-type documentation guideline.apps/workspace-engine/svc/controllers/forcedeploy/getters.go (1)
10-18: Document the exportedGetterinterface.A short doc comment is needed here to capture that this interface loads the fresh state used to rebuild a force-deploy, since that behavior is central to the new redeploy semantics.
As per coding guidelines, "Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/svc/controllers/forcedeploy/getters.go` around lines 10 - 18, Add a short exported doc comment for the Getter interface explaining its purpose: state that Getter loads the fresh runtime state used to reconstruct/rebuild a force-deploy (redeploy) operation and that its methods (ReleaseTargetExists, GetDesiredRelease, GetActiveJobsForReleaseTarget, GetDeployment, GetEnvironment, GetResource, ListJobAgentsByWorkspaceID) are used to gather the canonical data required to perform the new redeploy semantics; keep the comment concise and placed immediately above the Getter type declaration.packages/db/src/reconcilers/force-deploy.ts (1)
7-14: Extract the params shape into an interface.Since this is part of the package surface, giving the params object a named interface will make the contract reusable and keep the public API aligned with the repo’s TS conventions.
♻️ Suggested refactor
+export interface EnqueueForceDeployParams { + workspaceId: string; + deploymentId: string; + environmentId: string; + resourceId: string; +} + export async function enqueueForceDeploy( db: Tx, - params: { - workspaceId: string; - deploymentId: string; - environmentId: string; - resourceId: string; - }, + params: EnqueueForceDeployParams, ): Promise<ReconcileWorkScope> {As per coding guidelines, "Prefer interfaces for public APIs in TypeScript code".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/db/src/reconcilers/force-deploy.ts` around lines 7 - 14, Extract the inline params object into a named exported interface (e.g., ForceDeployParams) and use that interface in enqueueForceDeploy's signature instead of the inline type; update the function declaration (enqueueForceDeploy) to accept params: ForceDeployParams and export the interface so callers can import and reuse it, and update any local call sites or types that referenced the previous inline shape to reference the new interface.apps/workspace-engine/svc/controllers/forcedeploy/setters.go (1)
9-12: Document the exportedSetterinterface.Please add a package-style doc comment that explains the role of this abstraction in the force-deploy flow, not just its method names.
As per coding guidelines, "Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/svc/controllers/forcedeploy/setters.go` around lines 9 - 12, Add a proper exported doc comment for the Setter interface describing its role in the force-deploy flow: explain that Setter is the abstraction responsible for persisting deployment jobs (CreateJob) and scheduling their dispatch (EnqueueJobDispatch) within the force-deploy controller, describe expected behavior/side-effects (e.g., persistence, enqueueing to dispatch queue) and any guarantees or failure semantics callers should expect; place this package-style comment immediately above the "type Setter interface" declaration and reference the methods CreateJob and EnqueueJobDispatch in the prose.apps/workspace-engine/svc/controllers/forcedeploy/reconcile_test.go (1)
15-17: Drop the section-banner comments.These headings just restate the declarations that immediately follow and add noise in a file that's already small enough to scan directly.
As per coding guidelines "Do not add extraneous inline comments that state the obvious" and "Do not add comments that simply restate what the code does."
Also applies to: 77-79, 112-114, 190-192
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/svc/controllers/forcedeploy/reconcile_test.go` around lines 15 - 17, Remove the redundant section-banner comments that simply restate nearby declarations—specifically delete the banner comment containing "Mock Getter" and the other similar header blocks that precede the mock declarations and test helpers (the ones noted in the review). Edit reconcile_test.go to remove those extraneous comment blocks so only the actual mock/type/test declarations (e.g., the mocked getter and its related mocks/helpers) remain, leaving concise, self-explanatory code consistent with the project's inline-commenting guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/workspace-engine/svc/controllers/forcedeploy/reconcile.go`:
- Around line 141-147: CreateJob and EnqueueJobDispatch must be made atomic:
currently CreateJob can succeed and EnqueueJobDispatch fail, leaving a persisted
active job that never gets dispatched. Fix by performing both writes inside a
single DB transaction or expose a single setter API that does both as an outbox
operation (e.g., add setter.CreateAndEnqueueJob or a transactional wrapper) and
call that from reconcile.go instead of calling setter.CreateJob and
setter.EnqueueJobDispatch separately; ensure the transaction/error handling
rolls back the job persist if enqueue fails and reference the existing
setter.CreateJob and setter.EnqueueJobDispatch functions when implementing the
combined transactional operation.
In `@apps/workspace-engine/svc/controllers/forcedeploy/releasetarget.go`:
- Around line 11-18: The ReleaseTarget currently only carries
DeploymentID/EnvironmentID/ResourceID so NewReleaseTarget loses the originating
job identity; extend the ReleaseTarget struct to include a JobID (or
JobAgentID/jobKey) field and update NewReleaseTarget to parse and populate that
field from the force-deploy key (and ensure callers include that job identity
when constructing the key); use this preserved JobID in buildAndDispatchJob (or
the reconciliation path) to restrict redeploys to the original job/agent instead
of recomputing against all matched agents.
---
Nitpick comments:
In `@apps/workspace-engine/svc/controllers/forcedeploy/getters_postgres.go`:
- Line 15: Add a doc comment immediately above the exported type PostgresGetter
describing that it implements the Postgres-backed read side for force-deploy
reconciliation (e.g., "PostgresGetter is the Postgres-backed read side used by
the force-deploy reconciliation logic."). Keep the comment concise, present
tense, and document the purpose/role of PostgresGetter so it meets the
exported-type documentation guideline.
In `@apps/workspace-engine/svc/controllers/forcedeploy/getters.go`:
- Around line 10-18: Add a short exported doc comment for the Getter interface
explaining its purpose: state that Getter loads the fresh runtime state used to
reconstruct/rebuild a force-deploy (redeploy) operation and that its methods
(ReleaseTargetExists, GetDesiredRelease, GetActiveJobsForReleaseTarget,
GetDeployment, GetEnvironment, GetResource, ListJobAgentsByWorkspaceID) are used
to gather the canonical data required to perform the new redeploy semantics;
keep the comment concise and placed immediately above the Getter type
declaration.
In `@apps/workspace-engine/svc/controllers/forcedeploy/reconcile_test.go`:
- Around line 15-17: Remove the redundant section-banner comments that simply
restate nearby declarations—specifically delete the banner comment containing
"Mock Getter" and the other similar header blocks that precede the mock
declarations and test helpers (the ones noted in the review). Edit
reconcile_test.go to remove those extraneous comment blocks so only the actual
mock/type/test declarations (e.g., the mocked getter and its related
mocks/helpers) remain, leaving concise, self-explanatory code consistent with
the project's inline-commenting guidelines.
In `@apps/workspace-engine/svc/controllers/forcedeploy/setters.go`:
- Around line 9-12: Add a proper exported doc comment for the Setter interface
describing its role in the force-deploy flow: explain that Setter is the
abstraction responsible for persisting deployment jobs (CreateJob) and
scheduling their dispatch (EnqueueJobDispatch) within the force-deploy
controller, describe expected behavior/side-effects (e.g., persistence,
enqueueing to dispatch queue) and any guarantees or failure semantics callers
should expect; place this package-style comment immediately above the "type
Setter interface" declaration and reference the methods CreateJob and
EnqueueJobDispatch in the prose.
In `@packages/db/src/reconcilers/force-deploy.ts`:
- Around line 7-14: Extract the inline params object into a named exported
interface (e.g., ForceDeployParams) and use that interface in
enqueueForceDeploy's signature instead of the inline type; update the function
declaration (enqueueForceDeploy) to accept params: ForceDeployParams and export
the interface so callers can import and reuse it, and update any local call
sites or types that referenced the previous inline shape to reference the new
interface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d408c02a-acfd-4895-ac4e-3323fb759654
📒 Files selected for processing (13)
apps/workspace-engine/main.goapps/workspace-engine/pkg/reconcile/events/forcedeploy.goapps/workspace-engine/svc/controllers/forcedeploy/controller.goapps/workspace-engine/svc/controllers/forcedeploy/getters.goapps/workspace-engine/svc/controllers/forcedeploy/getters_postgres.goapps/workspace-engine/svc/controllers/forcedeploy/reconcile.goapps/workspace-engine/svc/controllers/forcedeploy/reconcile_test.goapps/workspace-engine/svc/controllers/forcedeploy/releasetarget.goapps/workspace-engine/svc/controllers/forcedeploy/setters.goapps/workspace-engine/svc/controllers/forcedeploy/setters_postgres.gopackages/db/src/reconcilers/force-deploy.tspackages/db/src/reconcilers/index.tspackages/trpc/src/routes/redeploy.ts
| type ReleaseTarget struct { | ||
| WorkspaceID uuid.UUID | ||
| DeploymentID uuid.UUID | ||
| EnvironmentID uuid.UUID | ||
| ResourceID uuid.UUID | ||
| } | ||
|
|
||
| func NewReleaseTarget(key string) (*ReleaseTarget, error) { |
There was a problem hiding this comment.
Preserve the source job identity in the force-deploy key.
This scope only carries deployment/environment/resource, so a redeploy request for one job loses which job/job-agent it came from. Downstream, buildAndDispatchJob recomputes against all currently matched agents and can create extra jobs unrelated to the job the user asked to redeploy. The work item needs enough identity to reconcile the same job target, not the whole release target.
Also applies to: 18-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/workspace-engine/svc/controllers/forcedeploy/releasetarget.go` around
lines 11 - 18, The ReleaseTarget currently only carries
DeploymentID/EnvironmentID/ResourceID so NewReleaseTarget loses the originating
job identity; extend the ReleaseTarget struct to include a JobID (or
JobAgentID/jobKey) field and update NewReleaseTarget to parse and populate that
field from the force-deploy key (and ensure callers include that job identity
when constructing the key); use this preserved JobID in buildAndDispatchJob (or
the reconciliation path) to restrict redeploys to the original job/agent instead
of recomputing against all matched agents.
There was a problem hiding this comment.
this is release target scoped? this seems irrelevant
There was a problem hiding this comment.
@adityachoudhari26, you're right — I missed the intent from issue #985. The whole point of the refactor is to recompute everything (agent, deployment config, environment, resource) from the latest state and only preserve the desired release. Being release-target scoped is correct by design; buildAndDispatchJob recomputing against all currently matched agents is the intended behavior, not a bug. I'll withdraw this comment.
✏️ Learnings added
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 990
File: apps/workspace-engine/svc/controllers/forcedeploy/releasetarget.go:11-18
Timestamp: 2026-04-14T22:50:11.856Z
Learning: In ctrlplanedev/ctrlplane, the `forcedeploy` controller (`apps/workspace-engine/svc/controllers/forcedeploy/`) is intentionally **release-target scoped** (DeploymentID + EnvironmentID + ResourceID). When redeploying, only the desired release is preserved; all other dispatch context (job agent, config, resource state, deployment state, environment state) is recomputed from the latest state. This is per the design in issue `#985`. Do not flag the absence of a JobID/JobAgentID on `ReleaseTarget` as a missing field.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 908
File: apps/workspace-engine/pkg/db/computed_relationships.sql.go:85-95
Timestamp: 2026-04-02T22:36:23.420Z
Learning: In `apps/workspace-engine/pkg/db/computed_relationships.sql.go`, `GetDeploymentForRelEval` and `ListDeploymentsByWorkspace` intentionally omit `job_agent_id` and `job_agent_config` from their projections after the refactor that moved job-agent data to a separate `deployment_job_agent` table. No existing CEL relationship-eval rules reference these fields, so backward compatibility is not a concern. Do not flag the absence of job-agent fields in these projections.
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.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 908
File: packages/db/drizzle/0182_rainy_changeling.sql:10-21
Timestamp: 2026-04-02T22:38:30.986Z
Learning: In `packages/db/drizzle/0182_rainy_changeling.sql` (PR `#908`), the legacy `deployment.job_agent_id` and `deployment.job_agent_config` columns were already migrated to the `deployment.job_agents` JSONB array in a prior migration before this PR ran. Therefore, the backfill INSERT only needs to read from `job_agents` — no need to also handle the singular `job_agent_id`/`job_agent_config` columns. Do not flag the absence of those columns in the backfill query.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 237
File: packages/api/src/router/job.ts:382-390
Timestamp: 2024-11-27T23:18:59.456Z
Learning: In the codebase, releases are not scoped to a specific resource.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 181
File: packages/auth/src/utils/rbac.ts:102-118
Timestamp: 2024-10-29T02:05:46.185Z
Learning: The `releaseChannel` scope type is included in the `scopeType` enum in `packages/db/src/schema/rbac.ts`.
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.
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.
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 818
File: apps/workspace-engine/pkg/db/queries/schema.sql:252-282
Timestamp: 2026-02-26T23:01:30.641Z
Learning: User adityachoudhari26 prefers to defer adding database indexes on foreign keys until there's evidence they're needed, considering such additions premature optimization at the initial schema design stage.
Resolves #985
Summary by CodeRabbit
New Features
Refactor