Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis pull request adds a new database reset command to the CLI. Changes include bumping the Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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. ✨ 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 |
📊 Benchmark resultsComparing with f70e4b4
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/commands/database/database.ts (1)
101-104: Add an explicit confirmation path fordb reset.This subcommand drops the entire local schema immediately, but there’s no confirmation or
--forceescape hatch. Prompting in interactive shells and requiring--forcefor non-interactive runs would make accidental data loss much harder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/database/database.ts` around lines 101 - 104, Add an explicit confirmation flow to the 'reset' subcommand: introduce a boolean option '--force' and, inside the .action handler for the reset command (the async (options: { json?: boolean }, command: BaseCommand) => { ... } block), if stdin is a TTY prompt the user to type an explicit confirmation (e.g., "type RESET to confirm") before proceeding; if stdin is not a TTY, require that options.force is true or abort with a clear message. Ensure the prompt branch prevents deletion on cancel and that the non-interactive branch only proceeds when --force is present, and reuse the existing command/console utilities for prompting and aborting so behavior integrates with the rest of the CLI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/commands/database/database.ts`:
- Around line 101-104: Add an explicit confirmation flow to the 'reset'
subcommand: introduce a boolean option '--force' and, inside the .action handler
for the reset command (the async (options: { json?: boolean }, command:
BaseCommand) => { ... } block), if stdin is a TTY prompt the user to type an
explicit confirmation (e.g., "type RESET to confirm") before proceeding; if
stdin is not a TTY, require that options.force is true or abort with a clear
message. Ensure the prompt branch prevents deletion on cancel and that the
non-interactive branch only proceeds when --force is present, and reuse the
existing command/console utilities for prompting and aborting so behavior
integrates with the rest of the CLI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6b3fd70-8750-495b-a021-91a8a7a6f335
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.jsonsrc/commands/database/database.tssrc/commands/database/reset.tstests/unit/commands/database/reset.test.ts
| const netlifyDev = new NetlifyDev({ | ||
| projectRoot: buildDir, | ||
| aiGateway: { enabled: false }, | ||
| blobs: { enabled: false }, | ||
| edgeFunctions: { enabled: false }, | ||
| environmentVariables: { enabled: false }, | ||
| functions: { enabled: false }, | ||
| geolocation: { enabled: false }, | ||
| headers: { enabled: false }, | ||
| images: { enabled: false }, | ||
| redirects: { enabled: false }, | ||
| staticFiles: { enabled: false }, | ||
| serverAddress: null, | ||
| }) | ||
|
|
||
| try { | ||
| await netlifyDev.start() | ||
|
|
||
| const { db } = netlifyDev | ||
| if (!db) { | ||
| throw new Error('Local database failed to start. Set EXPERIMENTAL_NETLIFY_DB_ENABLED=1 to enable.') | ||
| } | ||
|
|
||
| await db.reset() |
There was a problem hiding this comment.
Block db reset while another netlify dev session is using the local DB.
The PR description already calls out that running reset against a separate PGlite process can leave the data directory corrupted when a dev server is active. This code path always spins up a new NetlifyDev instance and proceeds to db.reset() without checking for that state, so the known corruption case is still user-reachable. Please fail fast here until the command can coordinate with the running dev server.
There was a problem hiding this comment.
Ohh interesting, the bot detected it as well. I'll address this in a follow-up, because it also affects the migrations.
There was a problem hiding this comment.
@paulo — Sounds good, thanks for confirming! Since this affects both db reset and migrations, it might be worth tracking as a GitHub issue so it doesn't get lost. Would you like me to open one for it?
1 participant
Summary
This adds the reset command from the primitives repo. However, I've noted a couple of issues that I'll address in a follow-up: