chore: deployment dependency api endpoints#1083
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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 (4)
📝 WalkthroughWalkthroughThis PR introduces REST API endpoints for managing deployment dependency relationships. Three endpoints are added under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 0/1 reviews remaining, refill in 44 minutes and 26 seconds.Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds REST + OpenAPI support in apps/api for managing deployment-to-deployment dependency edges backed by the existing deployment_dependency table, enabling external integrations to declare and list deployment dependencies.
Changes:
- Add REST handlers + routes to list, upsert, and delete deployment dependency edges.
- Extend OpenAPI path + schema definitions for the new dependency endpoints.
- Regenerate/extend OpenAPI TypeScript types to include the new operations and schemas.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/api/src/types/openapi.ts | Adds typed OpenAPI path + operation + schema entries for dependency endpoints. |
| apps/api/src/routes/v1/workspaces/deployments.ts | Implements GET list + PUT upsert + DELETE dependency handlers and wires them into the deployments router. |
| apps/api/openapi/schemas/deployments.jsonnet | Defines DeploymentDependency and UpsertDeploymentDependencyRequest schemas. |
| apps/api/openapi/paths/deployments.jsonnet | Defines /dependencies and /dependencies/{dependencyDeploymentId} OpenAPI paths/operations. |
| apps/api/openapi/openapi.json | Updates generated OpenAPI output with new schemas and endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+659
to
+663
| .get("/:deploymentId/dependencies", asyncHandler(listDeploymentDependencies)) | ||
| .put( | ||
| "/:deploymentId/dependencies/:dependencyDeploymentId", | ||
| asyncHandler(upsertDeploymentDependency), | ||
| ) |
| if (deploymentId === dependencyDeploymentId) | ||
| throw new ApiError("A deployment cannot depend on itself", 400); | ||
|
|
||
| if (!validResourceSelector(versionSelector)) |
Comment on lines
+308
to
+310
| .select() | ||
| .from(schema.deploymentDependency) | ||
| .where(eq(schema.deploymentDependency.deploymentId, deploymentId)) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
apps/api/openapi/paths/deployments.jsonnet (1)
107-140: Reuse the deployment ID param helper here.
dependencyDeploymentIdis also a deployment identifier, soopenapi.deploymentIdParam()would keep the OpenAPI validation and docs consistent with the rest of this file. I’d also mirror thebadRequestResponse()coverage used by the other deployment routes.Possible cleanup
- openapi.stringParam('dependencyDeploymentId', 'ID of the dependency deployment'), + openapi.deploymentIdParam(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/openapi/paths/deployments.jsonnet` around lines 107 - 140, Replace the explicit string param for dependencyDeploymentId with the deployment param helper and add the same badRequestResponse coverage to the delete route: use openapi.deploymentIdParam() for the dependencyDeploymentId parameter in both the put and delete operations (keeping the param name dependencyDeploymentId) and append openapi.badRequestResponse() to the delete responses to match other deployment routes.
🤖 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/api/openapi/openapi.json`:
- Around line 4949-4950: The OpenAPI contract currently identifies dependency
mutations by the pair (deploymentId, dependencyDeploymentId) but the requirement
says the edge is (deploymentId, dependencyDeploymentId, versionSelector); update
the operation and relevant schemas so identity includes versionSelector: modify
operationId requestDeploymentDependencyUpsert and its description to state the
tuple includes versionSelector, add versionSelector as a required parameter (or
part of the request body) to the upsert and delete operations, and update any
request/response schemas (e.g., the dependency resource/identifier object) so
uniqueness and ID fields reflect (deploymentId, dependencyDeploymentId,
versionSelector).
In `@apps/api/src/routes/v1/workspaces/deployments.ts`:
- Around line 369-397: The current preflight check (selecting schema.deployment
for deploymentId and dependencyDeploymentId) races with deletes and can still
cause the subsequent insert on schema.deploymentDependency to fail; make the
upsert atomic by performing the existence check and insert in a single DB
operation (or inside a transaction) using db (e.g., an INSERT ... SELECT or CTE
that selects from schema.deployment filtered by workspaceId and both ids) and
then run ON CONFLICT DO UPDATE on schema.deploymentDependency with set: {
versionSelector }; if the insert/CTE affects 0 rows, throw ApiError("Deployment
not found", 404) or ApiError("Dependency deployment not found", 404)
accordingly. Use the existing symbols deploymentId, dependencyDeploymentId,
versionSelector, schema.deployment, schema.deploymentDependency and db to locate
and implement the change.
In `@apps/api/src/types/openapi.ts`:
- Around line 212-252: The path parameter definitions for the two new routes are
missing (they currently have path?: never), so update the OpenAPI spec to add
path parameter objects for workspaceId and deploymentId on
"/v1/workspaces/{workspaceId}/deployments/{deploymentId}/dependencies" and for
workspaceId, deploymentId, dependencyDeploymentId on
"/v1/workspaces/{workspaceId}/deployments/{deploymentId}/dependencies/{dependencyDeploymentId}";
each parameter should be in: "path", have a name matching the placeholder,
required: true, and an appropriate schema (e.g., type: "string" / format: "uuid"
if applicable), then regenerate the types so operations
listDeploymentDependencies, requestDeploymentDependencyUpsert, and
requestDeploymentDependencyDeletion accept those path params.
- Around line 3385-3419: The requestDeploymentDependencyDeletion operation is
missing a 400 response for validation failures; add a 400 response entry under
requestDeploymentDependencyDeletion.responses (alongside 202 and 404) with a
descriptive comment (e.g., "Bad request / Validation error"), headers object
like the others, and content "application/json" pointing to the same error
schema used by sibling endpoints (e.g., components["schemas"]["ErrorResponse"])
so the contract consistently exposes client validation errors.
---
Nitpick comments:
In `@apps/api/openapi/paths/deployments.jsonnet`:
- Around line 107-140: Replace the explicit string param for
dependencyDeploymentId with the deployment param helper and add the same
badRequestResponse coverage to the delete route: use openapi.deploymentIdParam()
for the dependencyDeploymentId parameter in both the put and delete operations
(keeping the param name dependencyDeploymentId) and append
openapi.badRequestResponse() to the delete responses to match other deployment
routes.
🪄 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: eb9a0c58-560b-4c3d-a96e-ad649be2e12f
📒 Files selected for processing (5)
apps/api/openapi/openapi.jsonapps/api/openapi/paths/deployments.jsonnetapps/api/openapi/schemas/deployments.jsonnetapps/api/src/routes/v1/workspaces/deployments.tsapps/api/src/types/openapi.ts
Comment on lines
+4949
to
+4950
| "description": "Declare or update a version-selector dependency from this deployment to another deployment. Identified by the (deploymentId, dependencyDeploymentId) pair.", | ||
| "operationId": "requestDeploymentDependencyUpsert", |
Contributor
There was a problem hiding this comment.
Edge identity in the API contract does not match the stated requirement.
Line 4949 identifies dependencies by (deploymentId, dependencyDeploymentId), but the requirement defines an edge by (deploymentId, dependencyDeploymentId, versionSelector). This contract cannot uniquely update/delete multiple selectors for the same deployment pair. Please align mutation identity with the required tuple (or explicitly redefine uniqueness as pair everywhere).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/openapi/openapi.json` around lines 4949 - 4950, The OpenAPI contract
currently identifies dependency mutations by the pair (deploymentId,
dependencyDeploymentId) but the requirement says the edge is (deploymentId,
dependencyDeploymentId, versionSelector); update the operation and relevant
schemas so identity includes versionSelector: modify operationId
requestDeploymentDependencyUpsert and its description to state the tuple
includes versionSelector, add versionSelector as a required parameter (or part
of the request body) to the upsert and delete operations, and update any
request/response schemas (e.g., the dependency resource/identifier object) so
uniqueness and ID fields reflect (deploymentId, dependencyDeploymentId,
versionSelector).
Comment on lines
+369
to
+397
| const deployments = await db | ||
| .select({ id: schema.deployment.id }) | ||
| .from(schema.deployment) | ||
| .where( | ||
| and( | ||
| eq(schema.deployment.workspaceId, workspaceId), | ||
| inArray(schema.deployment.id, [deploymentId, dependencyDeploymentId]), | ||
| ), | ||
| ); | ||
|
|
||
| const found = new Set(deployments.map((d) => d.id)); | ||
| if (!found.has(deploymentId)) throw new ApiError("Deployment not found", 404); | ||
| if (!found.has(dependencyDeploymentId)) | ||
| throw new ApiError("Dependency deployment not found", 404); | ||
|
|
||
| await db | ||
| .insert(schema.deploymentDependency) | ||
| .values({ | ||
| deploymentId, | ||
| dependencyDeploymentId, | ||
| versionSelector, | ||
| }) | ||
| .onConflictDoUpdate({ | ||
| target: [ | ||
| schema.deploymentDependency.deploymentId, | ||
| schema.deploymentDependency.dependencyDeploymentId, | ||
| ], | ||
| set: { versionSelector }, | ||
| }); |
Contributor
There was a problem hiding this comment.
Make the dependency upsert atomic.
The preflight existence check can race with a concurrent delete, so the subsequent insert may fail with an uncaught DB error even though both deployments looked valid a moment earlier.
Recommended fix
- const deployments = await db
- .select({ id: schema.deployment.id })
- .from(schema.deployment)
- .where(
- and(
- eq(schema.deployment.workspaceId, workspaceId),
- inArray(schema.deployment.id, [deploymentId, dependencyDeploymentId]),
- ),
- );
-
- const found = new Set(deployments.map((d) => d.id));
- if (!found.has(deploymentId)) throw new ApiError("Deployment not found", 404);
- if (!found.has(dependencyDeploymentId))
- throw new ApiError("Dependency deployment not found", 404);
-
- await db
- .insert(schema.deploymentDependency)
- .values({
- deploymentId,
- dependencyDeploymentId,
- versionSelector,
- })
- .onConflictDoUpdate({
- target: [
- schema.deploymentDependency.deploymentId,
- schema.deploymentDependency.dependencyDeploymentId,
- ],
- set: { versionSelector },
- });
+ await db.transaction(async (tx) => {
+ const deployments = await tx
+ .select({ id: schema.deployment.id })
+ .from(schema.deployment)
+ .where(
+ and(
+ eq(schema.deployment.workspaceId, workspaceId),
+ inArray(schema.deployment.id, [deploymentId, dependencyDeploymentId]),
+ ),
+ );
+
+ const found = new Set(deployments.map((d) => d.id));
+ if (!found.has(deploymentId)) throw new ApiError("Deployment not found", 404);
+ if (!found.has(dependencyDeploymentId))
+ throw new ApiError("Dependency deployment not found", 404);
+
+ await tx
+ .insert(schema.deploymentDependency)
+ .values({
+ deploymentId,
+ dependencyDeploymentId,
+ versionSelector,
+ })
+ .onConflictDoUpdate({
+ target: [
+ schema.deploymentDependency.deploymentId,
+ schema.deploymentDependency.dependencyDeploymentId,
+ ],
+ set: { versionSelector },
+ });
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/routes/v1/workspaces/deployments.ts` around lines 369 - 397, The
current preflight check (selecting schema.deployment for deploymentId and
dependencyDeploymentId) races with deletes and can still cause the subsequent
insert on schema.deploymentDependency to fail; make the upsert atomic by
performing the existence check and insert in a single DB operation (or inside a
transaction) using db (e.g., an INSERT ... SELECT or CTE that selects from
schema.deployment filtered by workspaceId and both ids) and then run ON CONFLICT
DO UPDATE on schema.deploymentDependency with set: { versionSelector }; if the
insert/CTE affects 0 rows, throw ApiError("Deployment not found", 404) or
ApiError("Dependency deployment not found", 404) accordingly. Use the existing
symbols deploymentId, dependencyDeploymentId, versionSelector,
schema.deployment, schema.deploymentDependency and db to locate and implement
the change.
Comment on lines
+212
to
+252
| "/v1/workspaces/{workspaceId}/deployments/{deploymentId}/dependencies": { | ||
| parameters: { | ||
| query?: never; | ||
| header?: never; | ||
| path?: never; | ||
| cookie?: never; | ||
| }; | ||
| /** | ||
| * List deployment dependencies | ||
| * @description Returns the dependency edges declared by this deployment. | ||
| */ | ||
| get: operations["listDeploymentDependencies"]; | ||
| put?: never; | ||
| post?: never; | ||
| delete?: never; | ||
| options?: never; | ||
| head?: never; | ||
| patch?: never; | ||
| trace?: never; | ||
| }; | ||
| "/v1/workspaces/{workspaceId}/deployments/{deploymentId}/dependencies/{dependencyDeploymentId}": { | ||
| parameters: { | ||
| query?: never; | ||
| header?: never; | ||
| path?: never; | ||
| cookie?: never; | ||
| }; | ||
| get?: never; | ||
| /** | ||
| * Upsert deployment dependency | ||
| * @description Declare or update a version-selector dependency from this deployment to another deployment. Identified by the (deploymentId, dependencyDeploymentId) pair. | ||
| */ | ||
| put: operations["requestDeploymentDependencyUpsert"]; | ||
| post?: never; | ||
| /** Delete deployment dependency */ | ||
| delete: operations["requestDeploymentDependencyDeletion"]; | ||
| options?: never; | ||
| head?: never; | ||
| patch?: never; | ||
| trace?: never; | ||
| }; |
Contributor
There was a problem hiding this comment.
Populate the path parameters for the new dependency routes.
Both new paths entries still declare path?: never, so generated clients cannot supply workspaceId, deploymentId, or dependencyDeploymentId for these endpoints. Please fix the source OpenAPI spec and regenerate this file.
🔧 Proposed fix
"/v1/workspaces/{workspaceId}/deployments/{deploymentId}/dependencies": {
parameters: {
query?: never;
header?: never;
- path?: never;
+ path: {
+ workspaceId: string;
+ deploymentId: string;
+ };
cookie?: never;
};
@@
"/v1/workspaces/{workspaceId}/deployments/{deploymentId}/dependencies/{dependencyDeploymentId}": {
parameters: {
query?: never;
header?: never;
- path?: never;
+ path: {
+ workspaceId: string;
+ deploymentId: string;
+ dependencyDeploymentId: string;
+ };
cookie?: never;
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/types/openapi.ts` around lines 212 - 252, The path parameter
definitions for the two new routes are missing (they currently have path?:
never), so update the OpenAPI spec to add path parameter objects for workspaceId
and deploymentId on
"/v1/workspaces/{workspaceId}/deployments/{deploymentId}/dependencies" and for
workspaceId, deploymentId, dependencyDeploymentId on
"/v1/workspaces/{workspaceId}/deployments/{deploymentId}/dependencies/{dependencyDeploymentId}";
each parameter should be in: "path", have a name matching the placeholder,
required: true, and an appropriate schema (e.g., type: "string" / format: "uuid"
if applicable), then regenerate the types so operations
listDeploymentDependencies, requestDeploymentDependencyUpsert, and
requestDeploymentDependencyDeletion accept those path params.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #1078
Summary by CodeRabbit