refactor: enforce unique environment and deployment names in a workspace#1058
refactor: enforce unique environment and deployment names in a workspace#1058adityachoudhari26 merged 4 commits intomainfrom
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 13 minutes and 54 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 (12)
📝 WalkthroughWalkthroughThis PR implements unique name constraints for deployments and environments within workspaces. It adds database-level unique constraints on Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
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. Comment |
There was a problem hiding this comment.
Pull request overview
Enforces uniqueness of environment and deployment names within a workspace by adding DB-level unique constraints and surfacing conflicts as HTTP 409 in the API/OpenAPI, with e2e coverage for duplicate creates.
Changes:
- Add Postgres UNIQUE(workspace_id, name) constraints for
environmentanddeployment. - Map unique-constraint violations to
409 Conflictin environment/deployment create & upsert APIs; update OpenAPI accordingly. - Add e2e tests asserting
409on duplicate-name POST requests.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/db/src/schema/environment.ts | Adds composite uniqueness on (workspaceId, name) in schema. |
| packages/db/src/schema/deployment.ts | Adds composite uniqueness on (workspaceId, name) in schema. |
| packages/db/drizzle/0189_yellow_the_twelve.sql | Migration adding the new unique constraints. |
| packages/db/drizzle/meta/_journal.json | Registers migration 0189 in Drizzle journal. |
| packages/db/drizzle/meta/0189_snapshot.json | Snapshot updated to include new unique constraints. |
| apps/api/src/routes/v1/workspaces/environments.ts | Converts unique violations into 409 for create/upsert. |
| apps/api/src/routes/v1/workspaces/deployments.ts | Converts unique violations into 409 for create/upsert. |
| apps/api/openapi/lib/openapi.libsonnet | Adds reusable conflictResponse() helper. |
| apps/api/openapi/paths/environments.jsonnet | Documents 409 conflicts for environment create/upsert. |
| apps/api/openapi/paths/deployments.jsonnet | Documents 409 conflicts for deployment create/upsert. |
| apps/api/openapi/openapi.json | Generated OpenAPI includes 409 responses. |
| apps/api/src/types/openapi.ts | Generated TS types include 409 responses. |
| e2e/tests/api/environments.spec.ts | Adds e2e test for duplicate environment name on POST. |
| e2e/tests/api/deployments.spec.ts | Adds e2e test for duplicate deployment name on POST. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| workspaceId: uuid("workspace_id").references(() => workspace.id), | ||
| }, | ||
| (t) => [index().on(t.workspaceId)], | ||
| (t) => [index().on(t.workspaceId), unique().on(t.workspaceId, t.name)], |
| @@ -34,7 +35,7 @@ export const deployment = pgTable( | |||
|
|
|||
| workspaceId: uuid("workspace_id").references(() => workspace.id), | |||
| workspaceId: uuid("workspace_id").references(() => workspace.id), | ||
| }, | ||
| (t) => [index().on(t.workspaceId)], | ||
| (t) => [index().on(t.workspaceId), unique().on(t.workspaceId, t.name)], |
| ALTER TABLE "deployment" ADD CONSTRAINT "deployment_workspace_id_name_unique" UNIQUE("workspace_id","name");--> statement-breakpoint | ||
| ALTER TABLE "environment" ADD CONSTRAINT "environment_workspace_id_name_unique" UNIQUE("workspace_id","name"); No newline at end of file |
| if (error.code === "23505") | ||
| throw new ApiError( | ||
| "Environment name already exists in this workspace", | ||
| 409, | ||
| "DUPLICATE_NAME", | ||
| ); |
| if (error.code === "23505") | ||
| throw new ApiError( | ||
| "Deployment name already exists in this workspace", | ||
| 409, | ||
| "DUPLICATE_NAME", | ||
| ); |
| @@ -33,7 +34,7 @@ export const environment = pgTable( | |||
|
|
|||
| workspaceId: uuid("workspace_id").references(() => workspace.id), | |||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/paths/deployments.jsonnet`:
- Around line 30-31: The OpenAPI responses for postDeployment and
upsertDeployment in deployments.jsonnet are missing the 400 response for invalid
resourceSelector; restore the bad-request response in each operation's responses
map (the same place where openapi.acceptedResponse(...) and
openapi.conflictResponse(...) are combined) by adding the OpenAPI 400 response
(e.g., openapi.badRequestResponse(...) or the project's equivalent) so the
contract advertises 202, 409, and 400; apply the same change to the other
occurrence noted (around the second responses block at lines 61-62).
In `@apps/api/src/routes/v1/workspaces/deployments.ts`:
- Around line 230-240: The upsert in onConflictDoUpdate only writes
jobAgentSelector/jobAgentConfig when body.jobAgentSelector != null, which
prevents clearing or updating job-agent settings independently; change the logic
to detect presence of request keys (e.g., using
Object.prototype.hasOwnProperty.call(body, 'jobAgentSelector') and ...call(body,
'jobAgentConfig')) and include those fields in the set payload when present: if
jobAgentSelector is present set jobAgentSelector to body.jobAgentSelector ??
"false" (allow explicit null to clear), and if jobAgentConfig is present set
jobAgentConfig to body.jobAgentConfig ?? {} so onConflictDoUpdate (target:
schema.deployment.id) correctly updates or clears job-agent settings even when
only one of the two is provided.
In `@apps/api/src/types/openapi.ts`:
- Around line 3021-3029: The 409 responses reference
components["schemas"]["ErrorResponse"] which is currently typed as { error?:
string } but your runtime throws ApiError that serializes { message, code,
details }, so update the OpenAPI components: either expand
components.schemas.ErrorResponse to include message, code, details (with code
allowing "DUPLICATE_NAME") or add a new schema (e.g., DuplicateNameError)
matching ApiError and change the 409 responses to reference that schema; after
updating the OpenAPI schema regenerate the types so handlers like the
duplicate-name 409 (seen in the added response blocks) correctly surface code
=== "DUPLICATE_NAME" to clients.
In `@packages/db/drizzle/0189_yellow_the_twelve.sql`:
- Around line 1-2: This migration will fail if duplicates exist; update the
migration to detect and deduplicate existing rows in "deployment" and
"environment" before adding the UNIQUE constraints
deployment_workspace_id_name_unique and environment_workspace_id_name_unique:
add a pre-check that identifies rows with the same (workspace_id, name) (e.g.,
using ROW_NUMBER() OVER (PARTITION BY workspace_id, name ORDER BY id)), rename
or adjust all rn>1 rows to a deterministic unique value (for example append
'-dup-{id}'), commit those updates, then run the ALTER TABLE ... ADD CONSTRAINT
statements; also add a short migration note or ops runbook text that users will
no longer be allowed to have same-named environments/deployments per workspace
so operators know to run the migration or prepare cleanup beforehand.
In `@packages/db/src/schema/environment.ts`:
- Around line 35-37: The workspaceId column in the environment schema (the
uuid("workspace_id") field used in the environment table and its unique index on
(workspaceId, name)) is currently nullable which allows multiple rows with
workspace_id = NULL to share the same name; change the column definition to be
NOT NULL (and make the foreign key reference non-nullable as well) so the unique
constraint enforces "unique per workspace" correctly, and apply the same change
to the workspaceId definition in packages/db/src/schema/deployment.ts (the
uuid("workspace_id") used in the deployment table and its unique/index
constraints).
🪄 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: e638d535-8ab7-469c-bd75-dd0ea53783ce
📒 Files selected for processing (14)
apps/api/openapi/lib/openapi.libsonnetapps/api/openapi/openapi.jsonapps/api/openapi/paths/deployments.jsonnetapps/api/openapi/paths/environments.jsonnetapps/api/src/routes/v1/workspaces/deployments.tsapps/api/src/routes/v1/workspaces/environments.tsapps/api/src/types/openapi.tse2e/tests/api/deployments.spec.tse2e/tests/api/environments.spec.tspackages/db/drizzle/0189_yellow_the_twelve.sqlpackages/db/drizzle/meta/0189_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/deployment.tspackages/db/src/schema/environment.ts
| .onConflictDoUpdate({ | ||
| target: schema.deployment.id, | ||
| set: { | ||
| name: body.name, | ||
| description: body.description ?? "", | ||
| resourceSelector: body.resourceSelector ?? "false", | ||
| metadata: body.metadata ?? {}, | ||
| ...(body.jobAgentSelector != null | ||
| ? { jobAgentSelector: body.jobAgentSelector, jobAgentConfig } | ||
| : {}), | ||
| }, |
There was a problem hiding this comment.
This PUT can no longer clear or independently update job-agent settings.
The update branch only writes jobAgentSelector / jobAgentConfig when body.jobAgentSelector != null. That means an existing deployment keeps stale job-agent settings when the selector is omitted, and a request that only changes jobAgentConfig is silently ignored. Since the insert path still defaults omitted values to "false" / {}, the upsert behavior is now inconsistent and can preserve the wrong dispatch config.
Suggested fix
.onConflictDoUpdate({
target: schema.deployment.id,
set: {
name: body.name,
description: body.description ?? "",
resourceSelector: body.resourceSelector ?? "false",
+ jobAgentSelector: body.jobAgentSelector ?? "false",
+ jobAgentConfig,
metadata: body.metadata ?? {},
- ...(body.jobAgentSelector != null
- ? { jobAgentSelector: body.jobAgentSelector, jobAgentConfig }
- : {}),
},
});🤖 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 230 - 240, The
upsert in onConflictDoUpdate only writes jobAgentSelector/jobAgentConfig when
body.jobAgentSelector != null, which prevents clearing or updating job-agent
settings independently; change the logic to detect presence of request keys
(e.g., using Object.prototype.hasOwnProperty.call(body, 'jobAgentSelector') and
...call(body, 'jobAgentConfig')) and include those fields in the set payload
when present: if jobAgentSelector is present set jobAgentSelector to
body.jobAgentSelector ?? "false" (allow explicit null to clear), and if
jobAgentConfig is present set jobAgentConfig to body.jobAgentConfig ?? {} so
onConflictDoUpdate (target: schema.deployment.id) correctly updates or clears
job-agent settings even when only one of the two is provided.
| /** @description Deployment name already exists in this workspace */ | ||
| 409: { | ||
| headers: { | ||
| [name: string]: unknown; | ||
| }; | ||
| content: { | ||
| "application/json": components["schemas"]["ErrorResponse"]; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
The new 409 responses are typed with the wrong error payload.
These additions point at components["schemas"]["ErrorResponse"], but in this generated file that schema is still just { error?: string } (Lines 1434-1437). The runtime duplicate-name handlers throw ApiError, which serializes { message, code, details }. Any client using these generated types for 409 handling will miss code === "DUPLICATE_NAME" unless the source OpenAPI schema is fixed and regenerated.
Also applies to: 3102-3110, 3492-3500, 3591-3599
🤖 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 3021 - 3029, The 409 responses
reference components["schemas"]["ErrorResponse"] which is currently typed as {
error?: string } but your runtime throws ApiError that serializes { message,
code, details }, so update the OpenAPI components: either expand
components.schemas.ErrorResponse to include message, code, details (with code
allowing "DUPLICATE_NAME") or add a new schema (e.g., DuplicateNameError)
matching ApiError and change the 409 responses to reference that schema; after
updating the OpenAPI schema regenerate the types so handlers like the
duplicate-name 409 (seen in the added response blocks) correctly surface code
=== "DUPLICATE_NAME" to clients.
| ALTER TABLE "deployment" ADD CONSTRAINT "deployment_workspace_id_name_unique" UNIQUE("workspace_id","name");--> statement-breakpoint | ||
| ALTER TABLE "environment" ADD CONSTRAINT "environment_workspace_id_name_unique" UNIQUE("workspace_id","name"); No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for any dedup/rename helpers referencing deployment/environment name collisions in the PR branch.
rg -n -C2 --type=sql 'deployment|environment' packages/db/drizzle | rg -n -i 'dup|duplicate|rename|row_number|partition by'
rg -n -C2 -i 'duplicate.*(deployment|environment)|rename.*(deployment|environment)' -g '!**/node_modules/**'Repository: ctrlplanedev/ctrlplane
Length of output: 14523
🏁 Script executed:
# Read the full migration file
cat -n packages/db/drizzle/0189_yellow_the_twelve.sqlRepository: ctrlplanedev/ctrlplane
Length of output: 323
🏁 Script executed:
# Check if constraints already exist in schema (Drizzle schema)
rg -n "deployment_workspace_id_name_unique|environment_workspace_id_name_unique" packages/db/Repository: ctrlplanedev/ctrlplane
Length of output: 817
🏁 Script executed:
# Check prior migrations for any dedup logic targeting deployment/environment names
rg -n -B5 -A5 "UPDATE.*deployment.*name|UPDATE.*environment.*name" packages/db/drizzle/*.sql | head -100Repository: ctrlplanedev/ctrlplane
Length of output: 48
🏁 Script executed:
# Look at the Drizzle schema definition to see if constraints are already defined
fd "schema|drizzle\.config" packages/db/ -type f | head -10Repository: ctrlplanedev/ctrlplane
Length of output: 236
🏁 Script executed:
# Check if this constraint addition is idempotent or if it's a fresh add
rg -n "IF NOT EXISTS|IF\s+[A-Z]" packages/db/drizzle/0189_yellow_the_twelve.sqlRepository: ctrlplanedev/ctrlplane
Length of output: 48
🏁 Script executed:
# Look for the schema definition files (Drizzle schema)
find packages/db -name "*.ts" -o -name "schema*" | grep -E "(schema|index)" | head -20Repository: ctrlplanedev/ctrlplane
Length of output: 821
🏁 Script executed:
# Check the snapshot before 0189 to see if these constraints already existed
cat -n packages/db/drizzle/meta/0188_snapshot.json | grep -C3 "deployment_workspace_id_name_unique\|environment_workspace_id_name_unique"Repository: ctrlplanedev/ctrlplane
Length of output: 48
🏁 Script executed:
# Check Drizzle schema definition to see what the constraints look like in code
rg -n -A10 "export const deployment\s*=" packages/db/src/ --type ts | head -50Repository: ctrlplanedev/ctrlplane
Length of output: 882
🏁 Script executed:
# Look at the git diff context to see if this is adding a new constraint or modifying existing ones
git log --all --oneline --grep="0189\|yellow_the_twelve" | head -5Repository: ctrlplanedev/ctrlplane
Length of output: 48
🏁 Script executed:
# Get the full deployment schema with unique constraints
cat -n packages/db/src/schema/deployment.ts | head -80Repository: ctrlplanedev/ctrlplane
Length of output: 2710
🏁 Script executed:
# Get the full environment schema with unique constraints
cat -n packages/db/src/schema/environment.ts | head -60Repository: ctrlplanedev/ctrlplane
Length of output: 2168
🏁 Script executed:
# Check the prior snapshot (0188) for the constraint state
cat packages/db/drizzle/meta/0188_snapshot.json | jq '.schemas["public"]["tables"]["deployment"]["uniqueConstraints"] // .schemas["public"]["tables"]["deployment"]["indexes"]' 2>/dev/null | head -30Repository: ctrlplanedev/ctrlplane
Length of output: 70
🏁 Script executed:
# Check if there are any migrations between 0182 and 0189 that might have deduped
ls -1 packages/db/drizzle/018*.sql | sortRepository: ctrlplanedev/ctrlplane
Length of output: 533
🏁 Script executed:
# Check the prior migration (0188) for any dedup logic
cat -n packages/db/drizzle/0188_backfill_variables.sql | head -50Repository: ctrlplanedev/ctrlplane
Length of output: 2116
🏁 Script executed:
# Search all recent migrations for any dedup/handling of deployment and environment names
rg -n "deployment.*name|environment.*name" packages/db/drizzle/018[3-9]*.sqlRepository: ctrlplanedev/ctrlplane
Length of output: 407
🏁 Script executed:
# Check if the constraint adding uses IF NOT EXISTS or any error handling
grep -r "IF NOT EXISTS\|ON CONFLICT\|CONSTRAINT" packages/db/drizzle/0189* | head -20Repository: ctrlplanedev/ctrlplane
Length of output: 309
🏁 Script executed:
# Search for any test or validation that checks for duplicate names
rg -n "duplicate.*name|name.*duplicate" packages/db/src/ packages/db/drizzle/ -A3 -B3 | head -50Repository: ctrlplanedev/ctrlplane
Length of output: 48
🏁 Script executed:
# Look at the full Drizzle config to understand the generation strategy
cat -n packages/db/drizzle.config.ts 2>/dev/null || cat -n packages/db/drizzle.config.js 2>/dev/null || find packages/db -name "drizzle*config*" -exec cat -n {} \;Repository: ctrlplanedev/ctrlplane
Length of output: 572
🏁 Script executed:
# Check if there are any validations in the application code that prevent duplicate names
rg -n "workspace.*name|deployment.*name.*unique|environment.*name.*unique" packages/web/ apps/ --type ts --type tsx -A2 -B2 | head -80Repository: ctrlplanedev/ctrlplane
Length of output: 93
🏁 Script executed:
# Look at migration history to see when deployment/environment name constraints were first introduced
git log --all --oneline -- packages/db/drizzle/01*.sql | grep -i "constraint\|unique\|dedup" | head -10Repository: ctrlplanedev/ctrlplane
Length of output: 178
🏁 Script executed:
# Check if Drizzle ever generated constraint-dropping migrations that might indicate pre-existing duplicates
rg -n "DROP.*CONSTRAINT.*deployment|DROP.*CONSTRAINT.*environment" packages/db/drizzle/ --type sqlRepository: ctrlplanedev/ctrlplane
Length of output: 4415
🏁 Script executed:
# Look at when the unique() constraint was added to the schema TypeScript files
git log --all --oneline -- packages/db/src/schema/deployment.ts packages/db/src/schema/environment.ts | head -15Repository: ctrlplanedev/ctrlplane
Length of output: 1085
🏁 Script executed:
# Check if there are any database tests or fixtures that validate this constraint
find packages/db -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "deployment.*name\|environment.*name" 2>/dev/null | head -5Repository: ctrlplanedev/ctrlplane
Length of output: 48
🏁 Script executed:
# Check git history to find when unique constraint was added to deployment schema
git log --all -p -- packages/db/src/schema/deployment.ts | grep -B10 -A10 "unique().on" | head -40Repository: ctrlplanedev/ctrlplane
Length of output: 663
🏁 Script executed:
# Check if migrations 0187 or 0188 contain any constraint handling
cat -n packages/db/drizzle/0187_dry_quicksilver.sqlRepository: ctrlplanedev/ctrlplane
Length of output: 5563
🏁 Script executed:
# Look for any migration that may have removed or recreated these constraints
rg -n "deployment_workspace_id_name_unique|environment_workspace_id_name_unique" packages/db/drizzle/ --type sqlRepository: ctrlplanedev/ctrlplane
Length of output: 407
🏁 Script executed:
# Check the snapshot from 0188 to see what constraints existed before 0189
jq '.schemas["public"]["tables"]["deployment"]["uniqueConstraints"] // .schemas["public"]["tables"]["deployment"] | keys' packages/db/drizzle/meta/0188_snapshot.json 2>/dev/null | head -30Repository: ctrlplanedev/ctrlplane
Length of output: 48
🏁 Script executed:
# Check if there's an operations guide or deployment instructions that might handle this
find . -name "*.md" -o -name "*.txt" | xargs grep -l "duplicate.*name\|migration.*constraint" 2>/dev/null | head -5Repository: ctrlplanedev/ctrlplane
Length of output: 48
🏁 Script executed:
# Check all previous migrations to see if this constraint was ever added before
rg -n "ADD CONSTRAINT.*deployment_workspace_id_name_unique|ADD CONSTRAINT.*environment_workspace_id_name_unique" packages/db/drizzle/ --type sqlRepository: ctrlplanedev/ctrlplane
Length of output: 407
🏁 Script executed:
# Check the 0188 snapshot to see if constraints already existed
cat packages/db/drizzle/meta/0188_snapshot.json | jq '.schemas["public"]["tables"]["deployment"]' | head -80Repository: ctrlplanedev/ctrlplane
Length of output: 70
🏁 Script executed:
# Look for any references to when constraints were first introduced
git log --all --oneline --grep="constraint\|unique" -- packages/db/drizzle/ | head -20Repository: ctrlplanedev/ctrlplane
Length of output: 264
🏁 Script executed:
# Check if there's a schema migration log or documentation
find . -name "CHANGELOG*" -o -name "MIGRATIONS*" -o -name "*.md" | xargs grep -l "0189\|constraint\|unique" 2>/dev/null | head -5Repository: ctrlplanedev/ctrlplane
Length of output: 96
🏁 Script executed:
# Check CHANGELOG for any entries about 0189 or constraints
cat CHANGELOG.md | grep -C10 "0189\|constraint\|unique.*deployment\|unique.*environment" | head -50Repository: ctrlplanedev/ctrlplane
Length of output: 4129
🏁 Script executed:
# Check if the constraint was conditionally added in any way (IF NOT EXISTS patterns)
git log --all -p -- packages/db/drizzle/0189* | head -100Repository: ctrlplanedev/ctrlplane
Length of output: 730
🏁 Script executed:
# Verify the exact status: was the unique constraint ever added before 0189?
# Count total occurrences across all migrations
git log --all --pretty=format: --name-only -- packages/db/drizzle/*.sql | sort -u | xargs -I {} sh -c 'echo "=== {} ===" && grep -n "deployment_workspace_id_name_unique\|environment_workspace_id_name_unique" "{}"' 2>/dev/nullRepository: ctrlplanedev/ctrlplane
Length of output: 10724
🏁 Script executed:
# Final check: search for any post-migration handling or rollback logic
rg -n "duplicate\|rollback\|0189\|operations\|runbook" packages/db/ docs/ --type md --type txt 2>/dev/null | head -50Repository: ctrlplanedev/ctrlplane
Length of output: 48
🏁 Script executed:
# Check if there's any data validation or cleanup in app code that might deduplicate before migrating
rg -n "deployment.*name|environment.*name" apps/workspace-engine/ --type go -B2 -A2 | head -100Repository: ctrlplanedev/ctrlplane
Length of output: 7710
🏁 Script executed:
# Look for any test fixtures that might indicate duplicate name handling expectations
find . -name "*.test.*" -o -name "*.fixture.*" -o -name "seed*" | xargs grep -l "deployment\|environment" 2>/dev/null | head -5Repository: ctrlplanedev/ctrlplane
Length of output: 48
Migration will fail on any workspace with pre-existing duplicate names.
This migration adds UNIQUE(workspace_id, name) constraints on both deployment and environment with no dedup or error handling. The constraints are not present in the prior schema snapshot (0188), confirming they are new. Historically:
deployment.namewas never unique per workspace.environment.namewas effectively scoped per system, so workspaces with multiple systems very likely have multiple environments nameddev/staging/production.
Running this migration against production data with such duplicates will abort with a unique-constraint violation and leave the constraints un-created. Please implement one of the following:
- Add a pre-check and dedup block (e.g., rename conflicts to
name-dup-{id}) before the constraint addition, or - Document how pre-existing duplicates are handled (e.g., via a separate ops runbook or prior cleanup step).
Example dedup approach:
Pre-dedup logic for migration
WITH dups AS (
SELECT id,
ROW_NUMBER() OVER (PARTITION BY workspace_id, name ORDER BY id) AS rn
FROM "deployment"
WHERE workspace_id IS NOT NULL
)
UPDATE "deployment" d
SET name = d.name || '-dup-' || d.id::text
FROM dups
WHERE d.id = dups.id AND dups.rn > 1;
-- repeat for "environment" ...
ALTER TABLE "deployment" ADD CONSTRAINT "deployment_workspace_id_name_unique" UNIQUE("workspace_id","name");
ALTER TABLE "environment" ADD CONSTRAINT "environment_workspace_id_name_unique" UNIQUE("workspace_id","name");Also communicate to users that same-named environments across systems within a workspace are no longer allowed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/drizzle/0189_yellow_the_twelve.sql` around lines 1 - 2, This
migration will fail if duplicates exist; update the migration to detect and
deduplicate existing rows in "deployment" and "environment" before adding the
UNIQUE constraints deployment_workspace_id_name_unique and
environment_workspace_id_name_unique: add a pre-check that identifies rows with
the same (workspace_id, name) (e.g., using ROW_NUMBER() OVER (PARTITION BY
workspace_id, name ORDER BY id)), rename or adjust all rn>1 rows to a
deterministic unique value (for example append '-dup-{id}'), commit those
updates, then run the ALTER TABLE ... ADD CONSTRAINT statements; also add a
short migration note or ops runbook text that users will no longer be allowed to
have same-named environments/deployments per workspace so operators know to run
the migration or prepare cleanup beforehand.
| workspaceId: uuid("workspace_id").references(() => workspace.id), | ||
| }, | ||
| (t) => [index().on(t.workspaceId)], | ||
| (t) => [index().on(t.workspaceId), unique().on(t.workspaceId, t.name)], |
There was a problem hiding this comment.
Nullable workspaceId weakens the uniqueness guarantee.
workspaceId is declared as nullable (uuid("workspace_id").references(() => workspace.id), no .notNull()). PostgreSQL's default UNIQUE treats NULLs as distinct (NULLS DISTINCT), so multiple environment rows with workspace_id = NULL and the same name would still be allowed, bypassing the intended "unique per workspace" invariant. In practice workspaceId should always be set, but if that's an invariant, consider making the column NOT NULL (and the FK too), otherwise the constraint is advisory for orphaned rows.
Same concern applies to packages/db/src/schema/deployment.ts line 36.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/src/schema/environment.ts` around lines 35 - 37, The workspaceId
column in the environment schema (the uuid("workspace_id") field used in the
environment table and its unique index on (workspaceId, name)) is currently
nullable which allows multiple rows with workspace_id = NULL to share the same
name; change the column definition to be NOT NULL (and make the foreign key
reference non-nullable as well) so the unique constraint enforces "unique per
workspace" correctly, and apply the same change to the workspaceId definition in
packages/db/src/schema/deployment.ts (the uuid("workspace_id") used in the
deployment table and its unique/index constraints).
fixes #1057
fixes #1019
Summary by CodeRabbit
New Features
Documentation
Tests