Migrate Node.js CLI code to Bun #15
Conversation
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on January 16
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
cli/bin/postgres-ai.ts
Outdated
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : String(err); | ||
| console.error(`Failed to connect to API: ${message}`); | ||
| callbackServer.server.close(); |
There was a problem hiding this comment.
Server method mismatch causes auth flow runtime error
The CallbackServer interface in auth-server.ts was updated to expose server: { stop: () => void }, but the code in postgres-ai.ts still calls callbackServer.server.close() in three places. Since the server object only has a stop() method, these calls will throw a runtime error (undefined is not a function) during the OAuth authentication flow. All three occurrences need to use .stop() instead of .close().
Additional Locations (2)
|
@cursor review |
|
@claude review |
| </div> | ||
| </body> | ||
| </html> | ||
| `); |
There was a problem hiding this comment.
Missing promise rejection for invalid callback parameters
When the OAuth callback receives a request missing required code or state parameters, the code returns an HTTP 400 response to the browser but fails to reject the promise or stop the server. Unlike the OAuth error case (lines 95-123) and state mismatch case (lines 151-177), this code path doesn't set resolved = true, call clearTimeout(timeout), schedule server stop, or call rejectCallback(). This causes the CLI to hang for 2 minutes (the timeout duration) even though the browser shows an error page, then display a confusing "Authentication timeout" message instead of explaining that required parameters were missing.
- Replace Node.js child_process with Bun.spawn/Bun.spawnSync - Replace http/https modules with native fetch() - Replace http.createServer with Bun.serve for OAuth callback - Update shebangs to use #!/usr/bin/env bun - Convert test files from Node test runner to Bun test runner - Update package.json: type: module, scripts use bun - Update tsconfig.json for Bun bundler resolution - Add @types/bun to dev dependencies - Remove compiled dist/ artifacts (Bun runs TS directly) - Simplify pgai wrapper to use TypeScript directly
Add build step that compiles TypeScript to Node.js-compatible JavaScript, enabling both `bunx pgai` and `npx pgai` to work. Users without Bun installed can now use the CLI via npx with Node.js runtime. - Add bun build with --target node for Node.js compatibility - Update shebang replacement in build script - Fix path resolution for SQL templates in init.ts - Update pgai wrapper to detect .ts vs .js and use appropriate runtime
Replace broken Bun.stdin.stream() approach with a singleton readline interface. The previous implementation created a new stream reader on each call, which could fail on subsequent prompts due to stream state issues.
- Fix singleton readline interface for stdin prompts - Fix SQL directory resolution with proper fallback paths - Add error logging for unhandled spawn errors - Use any type for MCP SDK request handler (SDK types vary) - Use cross-platform shebang replacement in build script - Clarify auth server timing with comments
The Bun migration requires Bun to compile TypeScript to Node.js-compatible JavaScript. Update all CI jobs to: - Install Bun via curl - Run `bun install && bun run build` to create dist/ folder - Tests and e2e scripts can then use the compiled JS with Node.js The published npm package still works with Node.js only - Bun is only needed at build time.
- Fix pgai/package.json sed command for macOS compatibility - Add JSDoc documentation for auth server async stop behavior
Restore prepare-db integration tests that were removed during Bun migration. Tests cover: - URI/conninfo/psql-like connection parsing - Permission fixing idempotency - --verify mode - --reset-password functionality - Error reporting for insufficient permissions Tests are automatically skipped when: - PostgreSQL binaries (initdb, postgres) are not available - Running as root (initdb cannot run as root)
Replace Bun.spawn/spawnSync with Node.js child_process equivalents. Bun's --target node flag doesn't transform Bun-specific APIs to Node.js equivalents, causing runtime errors when running compiled JS with Node.js. This fixes the "Docker is not running" false positive in CI where Bun.spawnSync was undefined when running with Node.js.
NikolayS
force-pushed
the
claude/migrate-nodejs-to-bun-FTSWj
branch
from
202bb38 to
016cea0
Compare
The --set-key option is on 'auth login' subcommand, not 'auth' command
Closes https://gitlab.com/postgres-ai/postgres_ai/-/work_items/69
Summary
Bun.serve(),Bun.spawn(), nativefetch())bunxandnpx)Performance Improvements
Dual Runtime Support
bun ./bin/postgres-ai.ts(native TypeScript, no build)bunx pgaiandnpx pgaiTest Plan
bun testbun ./bin/postgres-ai.ts --helpworksnode ./dist/bin/postgres-ai.js --helpworksNote
Modernizes the CLI by adopting Bun while preserving Node compatibility for distribution and usage.
#!/usr/bin/env bun, ESM modules, and Bun-native APIs:fetch(HTTP),Bun.serve()(OAuth callback server), and spawn helpers; adds readline prompt utilities and cleans up child process usagelib/init.tsbun test; converts tests to.tsand removes legacy*.cjstestspackage.jsontotype: module, Bun-based build scripts that emit Node-compatible bundles; addsbun.lockand removespackage-lock.jsonpgaiwrapper (pgai/bin/pgai.ts) and build; removes old Node wrappernpm ci/build/testwithbun install,bun run build, andbun test; ensures dual-runtime by validatingnode ./dist/...still worksWritten by Cursor Bugbot for commit 202bb38. Configure here.