Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughAdds a new TypeScript migration script that backfills null workspace slugs by normalizing names, ensuring uniqueness with numeric suffixes, processing records in paginated batches, updating via drizzle, and logging progress and final counts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI
participant Script as backfill-workspace-slugs.ts
participant ORM as drizzle (mysqlDrizzle)
participant DB as MySQL (workspaces)
CLI->>Script: start
Script->>ORM: init connection (env: DATABASE_*)
loop until no more rows
Script->>DB: SELECT * FROM workspaces WHERE slug IS NULL ORDER BY id LIMIT 1000
DB-->>Script: batch
alt batch not empty
loop for each workspace
Script->>Script: compute normalized slug(name)
Script->>DB: SELECT 1 FROM workspaces WHERE slug = candidate
alt exists
Script->>Script: append numeric suffix (-1, -2, ...)
end
Script->>DB: UPDATE workspaces SET slug = finalSlug WHERE id = ...
DB-->>Script: update result
Script-->>CLI: log per-workspace result/warning
end
else no rows
Script-->>Script: break loop
end
end
Script->>ORM: close connection
Script-->>CLI: print summary (processed/updated/errors)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
MichaelUnkey
requested review from
Flo4604,
chronark,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tools/migrate/backfill-workspace-slugs.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Use Biome for formatting and linting in TypeScript/JavaScript projects
Prefer named exports over default exports in TypeScript/JavaScript, except for Next.js pages
Files:
tools/migrate/backfill-workspace-slugs.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow strict TypeScript configuration
Use Zod for runtime validation in TypeScript projects
Files:
tools/migrate/backfill-workspace-slugs.ts
**/*.{env,js,ts,go}
📄 CodeRabbit inference engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
tools/migrate/backfill-workspace-slugs.ts
🧬 Code graph analysis (1)
tools/migrate/backfill-workspace-slugs.ts (1)
internal/db/src/schema/workspaces.ts (1)
workspaces(16-104)
⏰ 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). (3)
- GitHub Check: Build / Build
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
🔇 Additional comments (2)
tools/migrate/backfill-workspace-slugs.ts (2)
42-49: Pagination on string PKs: OK, but confirm id collation is stableUsing
id > cursorwith varchar PKs is fine if collation is consistent and IDs are unique (they are). Just confirm IDs are not case-variant in practice; otherwise ordering could be surprising across collations.
35-41: Progress logging is clear and batch-sized; LGTMNice running totals and batch cursor logging—useful for long runs and resumability.
Also applies to: 112-123
| function generateSlug(name: string): string { | ||
| return name | ||
| .toLowerCase() | ||
| .trim() | ||
| .replace(/\s+/g, "-") // Replace spaces with hyphens | ||
| .replace(/[^a-z0-9-]/g, "") // Remove all special characters except hyphens | ||
| .replace(/-+/g, "-") // Replace multiple consecutive hyphens with single hyphen | ||
| .replace(/^-+|-+$/g, ""); // Remove leading and trailing hyphens | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Slug normalization: enforce 64-char limit, handle Unicode/diacritics, and collapse empty results predictably
Current regex drops all non [a-z0-9-], which turns many non‑ASCII names into empty slugs and can exceed DB limit (varchar(64)). Add diacritic stripping, predictable fallback, and length enforcement with room for numeric suffixes.
Apply:
-function generateSlug(name: string): string {
- return name
- .toLowerCase()
- .trim()
- .replace(/\s+/g, "-") // Replace spaces with hyphens
- .replace(/[^a-z0-9-]/g, "") // Remove all special characters except hyphens
- .replace(/-+/g, "-") // Replace multiple consecutive hyphens with single hyphen
- .replace(/^-+|-+$/g, ""); // Remove leading and trailing hyphens
-}
+function generateSlug(name: string): string {
+ // 1) normalize Unicode, strip diacritics; 2) canonicalize; 3) trim edges
+ const base = name
+ .toLowerCase()
+ .normalize("NFKD")
+ .replace(/[\u0300-\u036f]/g, "") // remove combining marks
+ .trim()
+ .replace(/\s+/g, "-")
+ .replace(/[^a-z0-9-]/g, "")
+ .replace(/-+/g, "-")
+ .replace(/^-+|-+$/g, "");
+
+ // reserve space for potential "-<n>" suffix later; keep at most 61 chars here
+ const MAX_BASE = 61;
+ const trimmed = base.slice(0, MAX_BASE);
+ return trimmed || "workspace";
+}📝 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.
| function generateSlug(name: string): string { | |
| return name | |
| .toLowerCase() | |
| .trim() | |
| .replace(/\s+/g, "-") // Replace spaces with hyphens | |
| .replace(/[^a-z0-9-]/g, "") // Remove all special characters except hyphens | |
| .replace(/-+/g, "-") // Replace multiple consecutive hyphens with single hyphen | |
| .replace(/^-+|-+$/g, ""); // Remove leading and trailing hyphens | |
| } | |
| function generateSlug(name: string): string { | |
| // 1) normalize Unicode, strip diacritics; 2) canonicalize; 3) trim edges | |
| const base = name | |
| .toLowerCase() | |
| .normalize("NFKD") | |
| .replace(/[\u0300-\u036f]/g, "") // remove combining marks | |
| .trim() | |
| .replace(/\s+/g, "-") // Replace spaces with hyphens | |
| .replace(/[^a-z0-9-]/g, "") // Remove all special characters except hyphens | |
| .replace(/-+/g, "-") // Collapse multiple hyphens | |
| .replace(/^-+|-+$/g, ""); // Trim leading/trailing hyphens | |
| // reserve space for a "-<n>" suffix later; limit to 61 chars for the base | |
| const MAX_BASE = 61; | |
| const trimmed = base.slice(0, MAX_BASE); | |
| // if slug is empty after stripping, fall back to a predictable constant | |
| return trimmed || "workspace"; | |
| } |
| const conn = await mysql.createConnection( | ||
| `mysql://${process.env.DATABASE_USERNAME}:${process.env.DATABASE_PASSWORD}@${process.env.DATABASE_HOST}:3306/unkey?ssl={}`, | ||
| ); | ||
|
|
||
| await conn.ping(); | ||
| const db = mysqlDrizzle(conn, { schema, mode: "default" }); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Env vars naming + validation; connection config should not embed credentials in a URL
- Violates guideline: envs must be UNKEY__VARNAME.
- No runtime validation; undefined yields malformed URL.
- Passing
?ssl={}is brittle; prefer typed options.
Apply:
+import { z } from "zod";
@@
- const conn = await mysql.createConnection(
- `mysql://${process.env.DATABASE_USERNAME}:${process.env.DATABASE_PASSWORD}@${process.env.DATABASE_HOST}:3306/unkey?ssl={}`,
- );
+ const Env = z
+ .object({
+ UNKEY_DB_HOST: z.string().min(1),
+ UNKEY_DB_USERNAME: z.string().min(1),
+ UNKEY_DB_PASSWORD: z.string().min(1),
+ UNKEY_DB_NAME: z.string().min(1).default("unkey"),
+ })
+ .parse(process.env);
+
+ const conn = await mysql.createConnection({
+ host: Env.UNKEY_DB_HOST,
+ user: Env.UNKEY_DB_USERNAME,
+ password: Env.UNKEY_DB_PASSWORD,
+ database: Env.UNKEY_DB_NAME,
+ // TODO: configure SSL per deployment; placeholder enables TLS negotiation without CA
+ ssl: {},
+ });Verification to run:
🏁 Script executed:
#!/bin/bash
# Ensure no legacy env names remain
rg -nP 'process\.env\.(DATABASE_(USERNAME|PASSWORD|HOST)|DATABASE_URL)'Length of output: 1386
Refactor all migration scripts to use validated UNKEY_DB_ env vars and object-based MySQL connections*
- Rename legacy
process.env.DATABASE_USERNAME,DATABASE_PASSWORD,DATABASE_HOST, (and anyDATABASE_URL) intools/migrate/*.tstoUNKEY_DB_USERNAME,UNKEY_DB_PASSWORD,UNKEY_DB_HOST,UNKEY_DB_NAMEand validate them (e.g., via zod). - Replace string-based URL connections
with object config:
mysql.createConnection( `mysql://${user}:${pass}@${host}:3306/${db}?ssl={}`, )
mysql.createConnection({ host: Env.UNKEY_DB_HOST, user: Env.UNKEY_DB_USERNAME, password: Env.UNKEY_DB_PASSWORD, database: Env.UNKEY_DB_NAME, ssl: {}, // configure per deployment });
🤖 Prompt for AI Agents
In tools/migrate/backfill-workspace-slugs.ts around lines 28 to 34, the script
still uses legacy process.env.DATABASE_* vars and a string-based MySQL URL;
update it to read and validate UNKEY_DB_USERNAME, UNKEY_DB_PASSWORD,
UNKEY_DB_HOST, and UNKEY_DB_NAME (use a small zod schema or equivalent
validation and throw with a clear message on failure), then call
mysql.createConnection with an object config (host, user, password, database,
ssl) using the validated Env values instead of composing a connection string;
ensure ssl is passed as an empty object or appropriate config per deployment.
| // Process each workspace | ||
| for (const workspace of workspaces) { | ||
| try { | ||
| const slug = generateSlug(workspace.name); | ||
|
|
||
| if (!slug) { | ||
| console.warn( | ||
| `Workspace ${workspace.id} (${workspace.name}) generated empty slug, skipping`, | ||
| ); | ||
| continue; | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Empty slugs: consider deterministic fallback instead of skipping
Currently logs and skips; that leaves holes. Prefer workspace-<shortId> or trunc(id) to guarantee progress.
Example change within this block after if (!slug) { ... }:
- if (!slug) {
- console.warn(
- `Workspace ${workspace.id} (${workspace.name}) generated empty slug, skipping`,
- );
- continue;
- }
+ if (!slug) {
+ // deterministic fallback from id
+ const fallback = `workspace-${workspace.id.slice(0, 8).toLowerCase().replace(/[^a-z0-9]/g, "")}`;
+ console.warn(`Empty slug from name; falling back to '${fallback}' for ${workspace.id}`);
+ // proceed with fallback
+ (slug as string) = fallback;
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tools/migrate/backfill-workspace-slugs.ts around lines 63 to 74, the script
currently logs and skips workspaces when generateSlug returns an empty value,
leaving gaps; instead generate a deterministic fallback slug (e.g.,
"workspace-<shortId>" using a stable prefix plus a truncated or hashed form of
workspace.id) and use that value so the migration always assigns a slug; ensure
the fallback is collision-resistant enough for your system (truncate id to 8-12
chars or hash+encode), and replace the continue with assigning the fallback slug
and proceeding with the existing save/update logic.
| await conn.end(); | ||
|
|
||
| console.info("Migration completed!"); | ||
| console.info({ | ||
| totalProcessed: processed, | ||
| totalUpdated: updated, | ||
| totalErrors: errors, | ||
| }); | ||
| } | ||
|
|
||
| main().catch((error) => { | ||
| console.error("Migration failed:", error); | ||
| process.exit(1); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Always close the DB connection with finally
If an error throws before Line 115, the connection is never closed.
Apply:
-async function main() {
+async function main() {
- const conn = await mysql.createConnection( /* ... */ );
- await conn.ping();
- const db = mysqlDrizzle(conn, { schema, mode: "default" });
- // work...
- await conn.end();
+ const conn = await mysql.createConnection( /* ... */ );
+ try {
+ await conn.ping();
+ const db = mysqlDrizzle(conn, { schema, mode: "default" });
+ // work...
+ } finally {
+ await conn.end().catch(() => {});
+ }
}📝 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.
| await conn.end(); | |
| console.info("Migration completed!"); | |
| console.info({ | |
| totalProcessed: processed, | |
| totalUpdated: updated, | |
| totalErrors: errors, | |
| }); | |
| } | |
| main().catch((error) => { | |
| console.error("Migration failed:", error); | |
| process.exit(1); | |
| }); | |
| async function main() { | |
| const conn = await mysql.createConnection( /* ... */ ); | |
| try { | |
| await conn.ping(); | |
| const db = mysqlDrizzle(conn, { schema, mode: "default" }); | |
| // work... | |
| } finally { | |
| // Always attempt to close the connection, but don't let errors here mask the original one | |
| await conn.end().catch(() => {}); | |
| } | |
| console.info("Migration completed!"); | |
| console.info({ | |
| totalProcessed: processed, | |
| totalUpdated: updated, | |
| totalErrors: errors, | |
| }); | |
| } | |
| main().catch((error) => { | |
| console.error("Migration failed:", error); | |
| process.exit(1); | |
| }); |
🤖 Prompt for AI Agents
In tools/migrate/backfill-workspace-slugs.ts around lines 115-128, the DB
connection is only closed after normal completion so if an error is thrown
earlier the connection is leaked; wrap the main logic that opens and uses conn
in a try/catch/finally (or add a finally to main()) and in the finally block
check that conn is defined and call await conn.end() (guarding against multiple
closes), then rethrow or handle errors as appropriate so the existing
main().catch(...) still logs and exits.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
tools/migrate/backfill-workspace-slugs.ts (5)
35-43: Use UNKEY_DB_ envs + validate with Zod; avoid URL credentials*Complies with repo guidelines; prevents malformed URLs and leaked secrets in connection strings.
-async function main() { - const conn = await mysql.createConnection( - `mysql://${process.env.DATABASE_USERNAME}:${process.env.DATABASE_PASSWORD}@${process.env.DATABASE_HOST}:3306/unkey?ssl={}`, - ); - - await conn.ping(); - const db = mysqlDrizzle(conn, { schema, mode: "default" }); +async function main() { + const Env = z + .object({ + UNKEY_DB_HOST: z.string().min(1), + UNKEY_DB_USERNAME: z.string().min(1), + UNKEY_DB_PASSWORD: z.string().min(1), + UNKEY_DB_NAME: z.string().min(1).default("unkey"), + }) + .parse(process.env); + + const conn = await mysql.createConnection({ + host: Env.UNKEY_DB_HOST, + user: Env.UNKEY_DB_USERNAME, + password: Env.UNKEY_DB_PASSWORD, + database: Env.UNKEY_DB_NAME, + ssl: {}, + }); + + await conn.ping(); + const db = mysqlDrizzle(conn, { schema, mode: "default" });Also add:
+import { z } from "zod";at the top of the file (after Line 2).
43-44: Always close the DB connection with finallyPrevents connection leaks on errors mid-run.
- console.log("Starting workspace slug backfill migration..."); + console.log("Starting workspace slug backfill migration..."); + try { @@ - await conn.end(); - - console.info("Migration completed!"); - console.info({ - totalProcessed: processed, - totalUpdated: updated, - totalErrors: errors, - }); -} + console.info("Migration completed!"); + console.info({ + totalProcessed: processed, + totalUpdated: updated, + totalErrors: errors, + }); + } finally { + await conn.end().catch(() => {}); + } +}Also applies to: 154-162
17-33: Slug normalization: explicitly strip diacriticsNormalization alone leaves combining marks until the later filter; strip them explicitly for clarity and fewer surprises on future regex changes.
const base = name .toLowerCase() .normalize("NFKD") + .replace(/[\u0300-\u036f]/g, "") // strip combining marks .trim() .replace(/\s+/g, "-") // Replace spaces with hyphens .replace(/[^a-z0-9-]/g, "") // Remove all special characters except hyphens .replace(/-+/g, "-") // Collapse multiple hyphens .replace(/^-+|-+$/g, ""); // Trim leading/trailing hyphens
74-81: Don’t skip empty slugs; use deterministic fallback from idEnsures forward progress for non‑ASCII names and avoids gaps.
- const slug = generateSlug(workspace.name); + let slug = generateSlug(workspace.name); @@ - if (!slug) { - console.warn( - `Workspace ${workspace.id} (${workspace.name}) generated empty slug, skipping`, - ); - continue; - } + if (!slug) { + const fallback = `workspace-${workspace.id + .slice(0, 8) + .toLowerCase() + .replace(/[^a-z0-9]/g, "")}`; + console.warn( + `Empty slug from name; falling back to '${fallback}' for ${workspace.id}`, + ); + slug = fallback; + }
125-136: Avoid extra SELECT by checking affectedRows from the UPDATESaves a round-trip; still safe with the IS NULL guard.
- await db + const result = await db .update(schema.workspaces) .set({ slug: finalSlug }) .where(and(eq(schema.workspaces.id, workspace.id), isNull(schema.workspaces.slug))); - - // Check if the update actually affected a row by verifying the database state - const updatedWorkspace = await db.query.workspaces.findFirst({ - where: (table, { eq }) => eq(table.id, workspace.id), - }); - const rowUpdated = updatedWorkspace?.slug === finalSlug; + // drizzle-mysql2 returns a ResultSetHeader/OkPacket with affectedRows + // @ts-expect-error drizzle mysql2 driver returns affectedRows + const rowUpdated = (result?.affectedRows ?? 0) > 0;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tools/migrate/backfill-workspace-slugs.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Use Biome for formatting and linting in TypeScript/JavaScript projects
Prefer named exports over default exports in TypeScript/JavaScript, except for Next.js pages
Files:
tools/migrate/backfill-workspace-slugs.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow strict TypeScript configuration
Use Zod for runtime validation in TypeScript projects
Files:
tools/migrate/backfill-workspace-slugs.ts
**/*.{env,js,ts,go}
📄 CodeRabbit inference engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
tools/migrate/backfill-workspace-slugs.ts
🧠 Learnings (1)
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.
Applied to files:
tools/migrate/backfill-workspace-slugs.ts
🧬 Code graph analysis (1)
tools/migrate/backfill-workspace-slugs.ts (1)
internal/db/src/schema/workspaces.ts (1)
workspaces(16-104)
⏰ 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). (3)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
🔇 Additional comments (2)
tools/migrate/backfill-workspace-slugs.ts (2)
96-117: Nice: uniqueness loop handles numeric suffixes and 64-char capGood job preventing “-1-1” and enforcing the varchar(64) limit.
50-56: Verify lexical ordering of workspace IDs
Ensure thatworkspaces.idis a lexicographically sortable string (e.g. ULID/KSUID) before usinggt(table.id, cursor)for pagination; if it’s a numeric or UUID that isn’t lex-ordered, switch to a numeric or timestamp cursor instead.
| import { and, eq, isNull, mysqlDrizzle, schema } from "@unkey/db"; | ||
| import mysql from "mysql2/promise"; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Imports: add Zod
Required for the env validation above.
import { and, eq, isNull, mysqlDrizzle, schema } from "@unkey/db";
import mysql from "mysql2/promise";
+import { z } from "zod";📝 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.
| import { and, eq, isNull, mysqlDrizzle, schema } from "@unkey/db"; | |
| import mysql from "mysql2/promise"; | |
| import { and, eq, isNull, mysqlDrizzle, schema } from "@unkey/db"; | |
| import mysql from "mysql2/promise"; | |
| import { z } from "zod"; |
🤖 Prompt for AI Agents
In tools/migrate/backfill-workspace-slugs.ts around lines 1 to 3, the file uses
Zod for env validation but does not import it; add the Zod import (for example
import { z } from "zod") to the existing imports at the top of the file so the
env validation code can reference Zod.
2 participants
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit