-
Notifications
You must be signed in to change notification settings - Fork 1
fix: fixed failed push for v7 and improved push/pull speed #74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
claim-db-worker | 2131802 | Jan 07 2026, 03:46 PM |
WalkthroughThe PR refactors API endpoint URLs to use a base constant, introduces a Prisma workspace abstraction for schema operations, updates Prisma to v7.2.0, and adds server bootstrap code with port 4141 to the schema-api-routes service. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag Worker URLs
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @schema-api-routes/src/index.ts:
- Around line 55-60: The unconditional call to serve(...) with port = 4141 will
run on import and can break Vercel serverless usage; wrap the console.log and
serve(...) invocation in a runtime check so the server only starts when executed
locally (e.g., guard with if (import.meta.main) { console.log(...); serve({
fetch: app.fetch, port }); } or, if using Node-compatible env vars, check
process.env.VERCEL !== '1' or NODE_ENV === 'development' before calling serve),
leaving the exported app.fetch untouched for serverless imports.
In @schema-api-routes/src/routes/schema/pull.ts:
- Around line 29-37: The interpolated connection string in the template assigned
to configContent is vulnerable to injection; replace the raw
"${connectionString}" interpolation with a safely escaped form by using
JSON.stringify(connectionString) so the url field becomes url:
${JSON.stringify(connectionString)} (this mirrors the fix used in push-force.ts
and ensures proper quoting/escaping of the connection string).
In @schema-api-routes/src/routes/schema/push-force.ts:
- Around line 7-41: The createPrismaWorkspace helper is duplicated; extract it
into a shared module (e.g., export async function createPrismaWorkspace(...) in
utils/prismaWorkspace.ts) and move the implementation (timestamped /tmp
directory, schemaPath/configPath, mkdir, writeFile, configContent, and cleanup
using rm with try/catch) into that file, ensuring you import the needed fs
promises (mkdir, writeFile, rm) and export the function; then replace the local
implementations in push.ts, pull.ts, and push-force.ts with an import {
createPrismaWorkspace } from "utils/prismaWorkspace" (adjust relative path) and
remove the duplicated code from those files so they all use the single shared
utility.
- Around line 20-28: The configContent template directly interpolates
connectionString which can break/enable injection if it contains backticks, ${,
or quotes; update the construction of configContent (variable name:
configContent) to inject a safely escaped string instead of raw connectionString
— e.g., serialize/escape connectionString (use JSON.stringify(connectionString)
or an equivalent escaping function) when interpolating into the defineConfig
template so the generated config is syntactically safe.
In @schema-api-routes/src/routes/schema/push.ts:
- Around line 20-28: The template literal building configContent injects
connectionString directly, creating an injection vulnerability; update the
interpolation to escape the value by wrapping connectionString with
JSON.stringify when embedding into the template (i.e., use
JSON.stringify(connectionString) inside the template literal that assigns to
configContent) so the generated config safely quotes/escapes the URL.
- Around line 73-78: The check in the if block that inspects the variable
message is too broad because message.includes("reset") can match unrelated texts
like "connection reset"; update the condition in the schema push error handling
(the if that references message.includes(...)) to use more specific
patterns—e.g., match whole-word "reset" with word boundaries or require context
like "force-reset" or "schema reset", or explicitly exclude "connection
reset"/"connection reset by peer" phrases; modify the logical condition
accordingly so only genuine schema-reset/data-loss errors trigger the branch.
🧹 Nitpick comments (1)
claim-db-worker/lib/prismaSchemaEditor/schemaApi.ts (1)
1-2: Consider environment-based API URL configuration.The commented localhost URL suggests manual toggling between environments. Using an environment variable would eliminate the need for code changes when switching contexts.
Suggested approach
-const API_URL = "https://create-db-schema-api-routes.vercel.app/api/schema"; -// const API_URL = "http://localhost:4141/api/schema"; +const API_URL = process.env.SCHEMA_API_URL ?? "https://create-db-schema-api-routes.vercel.app/api/schema";
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
claim-db-worker/lib/prismaSchemaEditor/schemaApi.tsschema-api-routes/package.jsonschema-api-routes/src/index.tsschema-api-routes/src/routes/schema/pull.tsschema-api-routes/src/routes/schema/push-force.tsschema-api-routes/src/routes/schema/push.ts
⏰ 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). (2)
- GitHub Check: Workers Builds: claim-db-worker
- GitHub Check: Workers Builds: create-db-worker
🔇 Additional comments (6)
schema-api-routes/src/index.ts (1)
44-46: LGTM!The updated message accurately describes the server's purpose.
claim-db-worker/lib/prismaSchemaEditor/schemaApi.ts (1)
4-25: LGTM!The API_URL refactoring is applied consistently across all endpoint calls, improving maintainability without changing functionality.
Also applies to: 27-57, 59-89, 91-111
schema-api-routes/src/routes/schema/push-force.ts (1)
43-80: LGTM!The handler properly validates inputs, uses a
finallyblock to guarantee cleanup, and provides meaningful error responses.schema-api-routes/src/routes/schema/pull.ts (1)
53-97: LGTM!The handler correctly handles the empty database edge case by returning a minimal schema, and the finally block ensures cleanup.
schema-api-routes/src/routes/schema/push.ts (1)
44-96: LGTM on the handler structure.The validation, workspace lifecycle, and cleanup pattern are well implemented.
schema-api-routes/package.json (1)
7-7: Prisma v7.2.0 is a valid, stable release.The major version bump from v5.10.0 to v7.2.0 is properly specified. The
defineConfigpattern used in the workspace aligns with Prisma v7's configuration approach.
| const port = 4141; | ||
| console.log(`Server is running on http://localhost:${port}`); | ||
| serve({ | ||
| fetch: app.fetch, | ||
| port, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Server bootstrap may break Vercel serverless deployment.
The serve() call executes unconditionally when this module is imported. Since vercel is listed as a dependency and Vercel typically imports the app export for serverless functions, this will attempt to bind port 4141 in the serverless environment, likely causing deployment failures.
Consider conditionally starting the server only in local development:
Proposed fix
export default app;
-const port = 4141;
-console.log(`Server is running on http://localhost:${port}`);
-serve({
- fetch: app.fetch,
- port,
-});
+if (process.env.NODE_ENV !== "production") {
+ const port = Number(process.env.PORT) || 4141;
+ console.log(`Server is running on http://localhost:${port}`);
+ serve({
+ fetch: app.fetch,
+ port,
+ });
+}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const port = 4141; | |
| console.log(`Server is running on http://localhost:${port}`); | |
| serve({ | |
| fetch: app.fetch, | |
| port, | |
| }); | |
| if (process.env.NODE_ENV !== "production") { | |
| const port = Number(process.env.PORT) || 4141; | |
| console.log(`Server is running on http://localhost:${port}`); | |
| serve({ | |
| fetch: app.fetch, | |
| port, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In @schema-api-routes/src/index.ts around lines 55 - 60, The unconditional call
to serve(...) with port = 4141 will run on import and can break Vercel
serverless usage; wrap the console.log and serve(...) invocation in a runtime
check so the server only starts when executed locally (e.g., guard with if
(import.meta.main) { console.log(...); serve({ fetch: app.fetch, port }); } or,
if using Node-compatible env vars, check process.env.VERCEL !== '1' or NODE_ENV
=== 'development' before calling serve), leaving the exported app.fetch
untouched for serverless imports.
| const configContent = `import { defineConfig } from "prisma/config"; | ||
|
|
||
| try { | ||
| await writeFile(envPath, `DATABASE_URL="${connectionString}"`); | ||
| await writeFile(schemaPath, minimalSchema); | ||
| export default defineConfig({ | ||
| schema: "./schema.prisma", | ||
| datasource: { | ||
| url: "${connectionString}", | ||
| }, | ||
| }); | ||
| `; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same injection vulnerability as in push-force.ts.
Apply the same JSON.stringify fix for safe connection string escaping.
Proposed fix
const configContent = `import { defineConfig } from "prisma/config";
export default defineConfig({
schema: "./schema.prisma",
datasource: {
- url: "${connectionString}",
+ url: ${JSON.stringify(connectionString)},
},
});
`;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const configContent = `import { defineConfig } from "prisma/config"; | |
| try { | |
| await writeFile(envPath, `DATABASE_URL="${connectionString}"`); | |
| await writeFile(schemaPath, minimalSchema); | |
| export default defineConfig({ | |
| schema: "./schema.prisma", | |
| datasource: { | |
| url: "${connectionString}", | |
| }, | |
| }); | |
| `; | |
| const configContent = `import { defineConfig } from "prisma/config"; | |
| export default defineConfig({ | |
| schema: "./schema.prisma", | |
| datasource: { | |
| url: ${JSON.stringify(connectionString)}, | |
| }, | |
| }); | |
| `; |
🤖 Prompt for AI Agents
In @schema-api-routes/src/routes/schema/pull.ts around lines 29 - 37, The
interpolated connection string in the template assigned to configContent is
vulnerable to injection; replace the raw "${connectionString}" interpolation
with a safely escaped form by using JSON.stringify(connectionString) so the url
field becomes url: ${JSON.stringify(connectionString)} (this mirrors the fix
used in push-force.ts and ensures proper quoting/escaping of the connection
string).
| async function createPrismaWorkspace( | ||
| connectionString: string, | ||
| schema: string | ||
| ): Promise<{ workDir: string; cleanup: () => Promise<void> }> { | ||
| const timestamp = Date.now(); | ||
| const workDir = `/tmp/prisma-${timestamp}`; | ||
| const schemaPath = `${workDir}/schema.prisma`; | ||
| const configPath = `${workDir}/prisma.config.ts`; | ||
|
|
||
| if (!schema) { | ||
| return c.json({ error: "Schema is required" }, 400); | ||
| } | ||
| await mkdir(workDir, { recursive: true }); | ||
|
|
||
| const connectionString = c.req.header("X-Connection-String"); | ||
| await writeFile(schemaPath, schema); | ||
|
|
||
| if (!connectionString) { | ||
| return c.json( | ||
| { error: "Connection string not provided in headers" }, | ||
| 400 | ||
| ); | ||
| } | ||
| const configContent = `import { defineConfig } from "prisma/config"; | ||
|
|
||
| const tempDir = "/tmp"; | ||
| const schemaPath = `${tempDir}/schema-${Date.now()}.prisma`; | ||
| const envPath = `${tempDir}/.env-${Date.now()}`; | ||
|
|
||
| try { | ||
| await writeFile(schemaPath, schema); | ||
| await writeFile(envPath, `DATABASE_URL="${connectionString}"`); | ||
| export default defineConfig({ | ||
| schema: "./schema.prisma", | ||
| datasource: { | ||
| url: "${connectionString}", | ||
| }, | ||
| }); | ||
| `; | ||
| await writeFile(configPath, configContent); | ||
|
|
||
| return { | ||
| workDir, | ||
| cleanup: async () => { | ||
| try { | ||
| const result = execSync( | ||
| `npx prisma db push --schema=${schemaPath} --accept-data-loss --force-reset`, | ||
| { | ||
| env: { | ||
| ...process.env, | ||
| DATABASE_URL: connectionString, | ||
| npm_config_cache: "/tmp/.npm", | ||
| npm_config_prefix: "/tmp/.npm", | ||
| }, | ||
| cwd: process.cwd(), | ||
| encoding: "utf8", | ||
| stdio: "pipe", | ||
| } | ||
| ); | ||
| } catch (error) { | ||
| throw error; | ||
| await rm(workDir, { recursive: true, force: true }); | ||
| } catch (e) { | ||
| console.error("Cleanup error:", e); | ||
| } | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Extract createPrismaWorkspace to a shared utility.
This helper is duplicated across push.ts, pull.ts, and push-force.ts. Extracting it to a shared module (e.g., utils/prismaWorkspace.ts) would reduce maintenance burden and ensure consistent behavior.
🤖 Prompt for AI Agents
In @schema-api-routes/src/routes/schema/push-force.ts around lines 7 - 41, The
createPrismaWorkspace helper is duplicated; extract it into a shared module
(e.g., export async function createPrismaWorkspace(...) in
utils/prismaWorkspace.ts) and move the implementation (timestamped /tmp
directory, schemaPath/configPath, mkdir, writeFile, configContent, and cleanup
using rm with try/catch) into that file, ensuring you import the needed fs
promises (mkdir, writeFile, rm) and export the function; then replace the local
implementations in push.ts, pull.ts, and push-force.ts with an import {
createPrismaWorkspace } from "utils/prismaWorkspace" (adjust relative path) and
remove the duplicated code from those files so they all use the single shared
utility.
| const configContent = `import { defineConfig } from "prisma/config"; | ||
|
|
||
| const tempDir = "/tmp"; | ||
| const schemaPath = `${tempDir}/schema-${Date.now()}.prisma`; | ||
| const envPath = `${tempDir}/.env-${Date.now()}`; | ||
|
|
||
| try { | ||
| await writeFile(schemaPath, schema); | ||
| await writeFile(envPath, `DATABASE_URL="${connectionString}"`); | ||
| export default defineConfig({ | ||
| schema: "./schema.prisma", | ||
| datasource: { | ||
| url: "${connectionString}", | ||
| }, | ||
| }); | ||
| `; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential injection vulnerability in config template.
The connectionString is directly interpolated into the template literal. If the connection string contains backticks, ${, or unescaped quotes, it could break the config syntax or enable injection.
Proposed fix using JSON.stringify for safe escaping
const configContent = `import { defineConfig } from "prisma/config";
export default defineConfig({
schema: "./schema.prisma",
datasource: {
- url: "${connectionString}",
+ url: ${JSON.stringify(connectionString)},
},
});
`;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const configContent = `import { defineConfig } from "prisma/config"; | |
| const tempDir = "/tmp"; | |
| const schemaPath = `${tempDir}/schema-${Date.now()}.prisma`; | |
| const envPath = `${tempDir}/.env-${Date.now()}`; | |
| try { | |
| await writeFile(schemaPath, schema); | |
| await writeFile(envPath, `DATABASE_URL="${connectionString}"`); | |
| export default defineConfig({ | |
| schema: "./schema.prisma", | |
| datasource: { | |
| url: "${connectionString}", | |
| }, | |
| }); | |
| `; | |
| const configContent = `import { defineConfig } from "prisma/config"; | |
| export default defineConfig({ | |
| schema: "./schema.prisma", | |
| datasource: { | |
| url: ${JSON.stringify(connectionString)}, | |
| }, | |
| }); | |
| `; |
🤖 Prompt for AI Agents
In @schema-api-routes/src/routes/schema/push-force.ts around lines 20 - 28, The
configContent template directly interpolates connectionString which can
break/enable injection if it contains backticks, ${, or quotes; update the
construction of configContent (variable name: configContent) to inject a safely
escaped string instead of raw connectionString — e.g., serialize/escape
connectionString (use JSON.stringify(connectionString) or an equivalent escaping
function) when interpolating into the defineConfig template so the generated
config is syntactically safe.
| const configContent = `import { defineConfig } from "prisma/config"; | ||
|
|
||
| try { | ||
| await writeFile(schemaPath, schema); | ||
| await writeFile(envPath, `DATABASE_URL="${connectionString}"`); | ||
| export default defineConfig({ | ||
| schema: "./schema.prisma", | ||
| datasource: { | ||
| url: "${connectionString}", | ||
| }, | ||
| }); | ||
| `; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same injection vulnerability as in other workspace files.
Apply the same JSON.stringify fix for safe connection string escaping.
Proposed fix
const configContent = `import { defineConfig } from "prisma/config";
export default defineConfig({
schema: "./schema.prisma",
datasource: {
- url: "${connectionString}",
+ url: ${JSON.stringify(connectionString)},
},
});
`;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const configContent = `import { defineConfig } from "prisma/config"; | |
| try { | |
| await writeFile(schemaPath, schema); | |
| await writeFile(envPath, `DATABASE_URL="${connectionString}"`); | |
| export default defineConfig({ | |
| schema: "./schema.prisma", | |
| datasource: { | |
| url: "${connectionString}", | |
| }, | |
| }); | |
| `; | |
| const configContent = `import { defineConfig } from "prisma/config"; | |
| export default defineConfig({ | |
| schema: "./schema.prisma", | |
| datasource: { | |
| url: ${JSON.stringify(connectionString)}, | |
| }, | |
| }); | |
| `; |
🤖 Prompt for AI Agents
In @schema-api-routes/src/routes/schema/push.ts around lines 20 - 28, The
template literal building configContent injects connectionString directly,
creating an injection vulnerability; update the interpolation to escape the
value by wrapping connectionString with JSON.stringify when embedding into the
template (i.e., use JSON.stringify(connectionString) inside the template literal
that assigns to configContent) so the generated config safely quotes/escapes the
URL.
| if ( | ||
| message.includes("data loss") || | ||
| message.includes("force-reset") || | ||
| message.includes("reset") || | ||
| message.includes("neither a built-in type") | ||
| ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broad "reset" check may cause false positives.
The pattern message.includes("reset") is quite broad and could match unrelated error messages (e.g., connection reset errors). Consider using more specific patterns or combining with additional context.
Suggested refinement
if (
message.includes("data loss") ||
message.includes("force-reset") ||
- message.includes("reset") ||
+ message.includes("requires a reset") ||
message.includes("neither a built-in type")
) {
🤖 Prompt for AI Agents
In @schema-api-routes/src/routes/schema/push.ts around lines 73 - 78, The check
in the if block that inspects the variable message is too broad because
message.includes("reset") can match unrelated texts like "connection reset";
update the condition in the schema push error handling (the if that references
message.includes(...)) to use more specific patterns—e.g., match whole-word
"reset" with word boundaries or require context like "force-reset" or "schema
reset", or explicitly exclude "connection reset"/"connection reset by peer"
phrases; modify the logical condition accordingly so only genuine
schema-reset/data-loss errors trigger the branch.
Summary by CodeRabbit
Updates
Infrastructure & Improvements
✏️ Tip: You can customize this high-level summary in your review settings.