Skip to content

Commit 6b69726

Browse files
committed
fix: address code review 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
1 parent 4d5d587 commit 6b69726

5 files changed

Lines changed: 24 additions & 13 deletions

File tree

cli/bin/postgres-ai.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,11 @@ function spawn(cmd: string, args: string[], options?: { stdio?: "pipe" | "ignore
9494
handlers["close"]?.forEach((cb) => cb(code));
9595
handlers["exit"]?.forEach((cb) => cb(code));
9696
}).catch((err) => {
97-
handlers["error"]?.forEach((cb) => cb(null, String(err)));
97+
if (handlers["error"]?.length) {
98+
handlers["error"].forEach((cb) => cb(null, String(err)));
99+
} else {
100+
console.error(`Spawn error for ${cmd}:`, err);
101+
}
98102
});
99103

100104
return {

cli/lib/auth-server.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,10 @@ export function createCallbackServer(
174174
resolved = true;
175175
clearTimeout(timeout);
176176

177-
setTimeout(() => serverInstance?.stop(), 100);
177+
// Resolve first, then stop server asynchronously after response is sent.
178+
// The 100ms delay ensures the HTTP response is fully written before closing.
178179
resolveCallback({ code, state });
180+
setTimeout(() => serverInstance?.stop(), 100);
179181

180182
return new Response(`
181183
<!DOCTYPE html>

cli/lib/init.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -160,16 +160,20 @@ export type InitPlan = {
160160
};
161161

162162
function sqlDir(): string {
163-
// Handle both development (lib/init.ts) and production (dist/bin/postgres-ai.js) paths
164-
// Development: lib/ -> ../sql
165-
// Production (bundled): dist/bin/ -> ../sql (copied during build)
166-
const devPath = path.resolve(__dirname, "..", "sql");
167-
const prodPath = path.resolve(__dirname, "..", "sql");
168-
169-
if (fs.existsSync(devPath)) {
170-
return devPath;
163+
// Handle both development and production paths
164+
// Development: lib/init.ts -> ../sql
165+
// Production (bundled): dist/bin/postgres-ai.js -> ../sql (copied during build)
166+
const candidates = [
167+
path.resolve(__dirname, "..", "sql"), // bundled: dist/bin -> dist/sql
168+
path.resolve(__dirname, "..", "..", "sql"), // dev from lib: lib -> ../sql
169+
];
170+
171+
for (const candidate of candidates) {
172+
if (fs.existsSync(candidate)) {
173+
return candidate;
174+
}
171175
}
172-
return prodPath;
176+
throw new Error(`SQL directory not found. Searched: ${candidates.join(", ")}`);
173177
}
174178

175179
function loadSqlTemplate(filename: string): string {

cli/lib/mcp-server.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ export async function startMcpServer(rootOpts?: RootOptsLike, extra?: { debug?:
7575
};
7676
});
7777

78-
server.setRequestHandler(CallToolRequestSchema, async (req: { params: { name: string; arguments?: Record<string, unknown> } }) => {
78+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
79+
server.setRequestHandler(CallToolRequestSchema, async (req: any) => {
7980
const toolName = req.params.name;
8081
const args = (req.params.arguments as Record<string, unknown>) || {};
8182

cli/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
"node": ">=18"
2323
},
2424
"scripts": {
25-
"build": "bun build ./bin/postgres-ai.ts --outdir ./dist/bin --target node && sed -i '1s|#!/usr/bin/env bun|#!/usr/bin/env node|' ./dist/bin/postgres-ai.js && cp -r sql dist/",
25+
"build": "bun build ./bin/postgres-ai.ts --outdir ./dist/bin --target node && node -e \"const fs=require('fs');const f='./dist/bin/postgres-ai.js';fs.writeFileSync(f,fs.readFileSync(f,'utf8').replace('#!/usr/bin/env bun','#!/usr/bin/env node'))\" && cp -r sql dist/",
2626
"prepublishOnly": "npm run build",
2727
"start": "bun ./bin/postgres-ai.ts --help",
2828
"start:node": "node ./dist/bin/postgres-ai.js --help",

0 commit comments

Comments
 (0)