-
Notifications
You must be signed in to change notification settings - Fork 1
fix: v7 routes failing in v7 #75
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 | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
claim-db-worker | e2b92b1 | Commit Preview URL Branch Preview URL |
Jan 07 2026, 06:00 PM |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughSchema API route handlers refactored to use isolated temporary Prisma projects per request instead of shared state. API endpoints in schemaApi.ts centralized via constant. Prisma dependency updated from 5.10.0 to 7.2.0, and server bootstrapping added to index.ts. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 npx create-db@pr75
npx create-pg@pr75
npx create-postgres@pr75Worker 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: 3
🤖 Fix all issues with AI agents
In @schema-api-routes/src/routes/schema/pull.ts:
- Around line 56-66: The code currently calls execSync('npm install --no-save',
{ cwd: projectDir, encoding: 'utf8', stdio: 'pipe', env: { ... } }) which blocks
the event loop; replace this synchronous call with an asynchronous promisified
exec (e.g., util.promisify(child_process.exec) or child_process.exec with a
Promise) so npm install runs without blocking, await the promise where this
occurs (same pattern used in push-force.ts), propagate or handle errors and
capture stdout/stderr from the returned result instead of relying on execSync.
In @schema-api-routes/src/routes/schema/push-force.ts:
- Around line 56-66: Replace the blocking execSync call used to run npm install
with a non-blocking async exec variant: import and promisify child_process.exec
(or use util.promisify) and replace execSync('npm install --no-save', { cwd:
projectDir, env: { ...process.env, HOME: tempDir, npm_config_cache:
`${tempDir}/.npm-cache`, npm_config_userconfig: `${tempDir}/.npmrc` } }) with an
awaited execAsync('npm install --no-save', { cwd: projectDir, env: {
...process.env, HOME: tempDir, npm_config_cache: `${tempDir}/.npm-cache`,
npm_config_userconfig: `${tempDir}/.npmrc` } }); ensure relevant imports are
added and error handling/await is present in the async function that contains
this logic (refer to execSync, projectDir, tempDir, and the push-force handler).
🧹 Nitpick comments (8)
schema-api-routes/src/index.ts (1)
54-63: Server bootstrapping looks good.The addition of
serve()with console logs for available routes is a sensible approach for the Node.js server. Consider extracting the port to an environment variable for flexibility across environments:-serve({ fetch: app.fetch, port: 4141 }); +const PORT = process.env.PORT ? parseInt(process.env.PORT, 10) : 4141; +serve({ fetch: app.fetch, port: PORT }); -console.log('Server running on http://localhost:4141'); +console.log(`Server running on http://localhost:${PORT}`);claim-db-worker/lib/prismaSchemaEditor/schemaApi.ts (1)
101-102: Minor indentation issue.Line 102 has extra leading whitespace before the template literal.
🔧 Suggested fix
const response = await fetch( - `${API_URL}/push-force`, + `${API_URL}/push-force`, {schema-api-routes/src/routes/schema/push-force.ts (3)
3-3: Remove unused import.
spawnis imported but not used in this file.🔧 Suggested fix
-import { spawn } from "child_process"; -import { execSync } from "child_process"; +import { execSync } from "child_process";
81-83: Redundant catch block.This catch block only rethrows the error without any transformation or logging. It can be removed entirely since the outer try-catch will handle it.
🔧 Suggested fix
- try { const result = execSync( `npx prisma db push --schema=${schemaPath} --accept-data-loss --force-reset`, { env: { ...process.env, DATABASE_URL: connectionString, }, cwd: projectDir, encoding: "utf8", stdio: "pipe", } ); - } catch (error) { - throw error; - }
28-54: Consider extracting temp project setup to a shared utility.The temporary Prisma project creation pattern (mkdir, write schema, write prisma.config.ts, write package.json, npm install) is duplicated across
push.ts,push-force.ts, andpull.ts. Extracting this to a shared utility would reduce duplication and simplify maintenance.schema-api-routes/src/routes/schema/pull.ts (2)
3-3: Remove unused import.
spawnis imported but not used in this file.🔧 Suggested fix
-import { spawn } from "child_process"; -import { execSync } from "child_process"; +import { execSync } from "child_process";
79-80: Remove unused variables.
stdoutandstderrare assigned but never used.🔧 Suggested fix
- const stdout = result; - const stderr = ""; + // result is not needed; execution success is sufficientschema-api-routes/src/routes/schema/push.ts (1)
3-3: Remove unused import.
spawnis imported but not used in this file.🔧 Suggested fix
-import { spawn } from "child_process"; -import { execSync } from "child_process"; +import { execSync } from "child_process";
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
schema-api-routes/package-lock.jsonis excluded by!**/package-lock.json
📒 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 (3)
claim-db-worker/lib/prismaSchemaEditor/schemaApi.ts (1)
1-2: Good centralization of the API endpoint.Extracting
API_URLto a constant improves maintainability. The commented localhost alternative is helpful for local development.schema-api-routes/src/routes/schema/push.ts (1)
28-66: Per-request temporary project approach is sound.The isolated temporary Prisma project pattern properly addresses the v7 requirements. The cleanup in the
finallyblock ensures resources are released even on failure. Once the synchronous blocking is addressed, this implementation should work well.schema-api-routes/package.json (1)
7-7: Prisma 7.2.0 is a stable, public release available on npm.This major version upgrade from 5.x to 7.x aligns with the codebase's use of
prisma/configwithdefineConfigandenv()in the schema route handlers. The package-lock.json confirms proper dependency resolution at v7.2.0.
|
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr75
npx create-pg@pr75
npx create-postgres@pr75Worker URLs
|
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.