Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 99 additions & 30 deletions mcpjam-inspector/bin/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,23 +120,97 @@ function delay(ms) {
return new Promise((resolve) => setTimeout(resolve, ms, true));
}

function isPortAvailable(port) {
function isPortAvailable(port, host = "127.0.0.1") {
return new Promise((resolve) => {
const server = createServer();
let settled = false;
const onCloseError = () => {};
const timeout = setTimeout(() => {
if (settled) {
return;
}
settled = true;
cleanup();
server.once("error", onCloseError);
try {
server.close(() => {
server.removeListener("error", onCloseError);
resolve(false);
});
} catch {
server.removeListener("error", onCloseError);
resolve(false);
}
}, 1000);

server.listen(port, "127.0.0.1", () => {
const cleanup = () => {
clearTimeout(timeout);
server.removeListener("error", onError);
server.removeListener("listening", onListening);
};

const onListening = () => {
if (settled) {
return;
}
settled = true;
cleanup();
server.close(() => {
resolve(true);
});
});
};

server.on("error", () => {
// Port is not available
const onError = () => {
if (settled) {
return;
}
settled = true;
cleanup();
resolve(false);
});
};

server.once("error", onError);
server.once("listening", onListening);
server.listen(port, host);
});
}

function parsePort(value) {
const parsed = Number.parseInt(value, 10);
if (!Number.isFinite(parsed) || parsed <= 0 || parsed > 65535) {
throw new Error(`Invalid port value: ${value}`);
}
return parsed;
}

async function findAvailablePort(
startPort,
host,
maxPortOffset = 100,
verbose = false,
) {
const maxPort = Math.min(startPort + maxPortOffset, 65535);

if (maxPort < startPort) {
throw new Error(
`No available port found in range ${startPort}-${maxPort}`,
);
}

for (let port = startPort; port <= maxPort; port++) {
const isAvailable = await isPortAvailable(port, host);
if (isAvailable) {
return port;
}

if (verbose) {
logWarning(`Port ${port} unavailable; checking next port`);
}
}

throw new Error(`No available port found in range ${startPort}-${maxPort}`);
}

function spawnPromise(command, args, options) {
return new Promise((resolve, reject) => {
const child = spawn(command, args, {
Expand Down Expand Up @@ -357,11 +431,6 @@ async function main() {
if (parsingFlags && arg === "--port" && i + 1 < args.length) {
const port = args[++i];
envVars.PORT = port;
// Default: localhost in development, 127.0.0.1 in production
const defaultHost =
process.env.ENVIRONMENT === "dev" ? "localhost" : "127.0.0.1";
const baseHost = process.env.HOST || defaultHost;
envVars.BASE_URL = `http://${baseHost}:${port}`;
continue;
}

Expand Down Expand Up @@ -623,17 +692,26 @@ async function main() {
// Apply parsed environment variables to process.env first
Object.assign(process.env, envVars);

// Port configuration (fixed default to 6274)
const requestedPort = 6274;
const requestedPortCandidate =
process.env.SERVER_PORT !== undefined
? process.env.SERVER_PORT
: envVars.PORT;
const requestedPort = requestedPortCandidate
? parsePort(requestedPortCandidate)
: 6274;
let PORT;
const defaultHost =
process.env.ENVIRONMENT === "dev" ? "localhost" : "127.0.0.1";
const baseHost = process.env.HOST || defaultHost;
const host = baseHost;

try {
// Check if user explicitly set a port via --port flag
const hasExplicitPort = envVars.PORT !== undefined;
const hasExplicitPort = requestedPortCandidate !== undefined;
Comment on lines +695 to +710
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Truthiness vs. identity: "0" and "" silently become port 6274 yet are treated as explicit.

Line 699 uses a truthiness test — falsy strings like "0" or "" fall through to the default 6274. But line 710 uses !== undefined, so those same values are considered explicitly requested. The result: --port 0 or SERVER_PORT="" silently checks 6274 as if the user asked for it, yielding a confusing error ("Explicitly requested port 6274 is not available") or quiet success on a port the user never intended.

Align the two checks so parsePort is the single arbiter of validity:

Proposed fix
   const requestedPortCandidate =
     process.env.SERVER_PORT !== undefined
       ? process.env.SERVER_PORT
       : envVars.PORT;
-  const requestedPort = requestedPortCandidate
-    ? parsePort(requestedPortCandidate)
-    : 6274;
+  const requestedPort =
+    requestedPortCandidate !== undefined && requestedPortCandidate !== ""
+      ? parsePort(requestedPortCandidate)
+      : 6274;
   let PORT;
   const defaultHost =
     process.env.ENVIRONMENT === "dev" ? "localhost" : "127.0.0.1";
   const baseHost = process.env.HOST || defaultHost;
   const host = baseHost;
 
   try {
     // Check if user explicitly set a port via --port flag
-    const hasExplicitPort = requestedPortCandidate !== undefined;
+    const hasExplicitPort =
+      requestedPortCandidate !== undefined && requestedPortCandidate !== "";

This way --port 0 produces the clear "Invalid port value: 0" error from parsePort, and an empty SERVER_PORT="" falls through to the default scan — both matching user intent.

🤖 Prompt for AI Agents
In `@mcpjam-inspector/bin/start.js` around lines 695 - 710, The code treats falsy
strings like "" or "0" inconsistently: requestedPort is computed via
parsePort(requestedPortCandidate) but hasExplicitPort is based on
requestedPortCandidate !== undefined; change hasExplicitPort to rely on the
parse result so parsePort is the single arbiter. Concretely, after computing
requestedPort (via parsePort(requestedPortCandidate)), set hasExplicitPort =
requestedPort !== undefined (or !== null depending on parsePort) instead of
checking requestedPortCandidate, so invalid values cause parsePort errors and
empty strings fall through to the default behavior.


if (hasExplicitPort) {
if (await isPortAvailable(requestedPort)) {
PORT = requestedPort.toString();
if (await isPortAvailable(requestedPort, host)) {
PORT = String(requestedPort);
} else {
logError(`Explicitly requested port ${requestedPort} is not available`);
logInfo(
Expand All @@ -642,23 +720,17 @@ async function main() {
throw new Error(`Port ${requestedPort} is already in use`);
}
} else {
// Fixed port policy: use default port 6274 and fail fast if unavailable
if (await isPortAvailable(requestedPort)) {
PORT = requestedPort.toString();
} else {
logError(
`Default port ${requestedPort} is already in use. Please free the port`,
const resolvedPort = await findAvailablePort(requestedPort, host, 100, verboseLogs);
if (resolvedPort !== requestedPort) {
logInfo(
`Default port ${requestedPort} is busy. Using next available port ${resolvedPort}.`,
);
throw new Error(`Port ${requestedPort} is already in use`);
}
PORT = String(resolvedPort);
}

// Update environment variables with the final port
envVars.PORT = PORT;
// Default: localhost in development, 127.0.0.1 in production
const defaultHost =
process.env.ENVIRONMENT === "dev" ? "localhost" : "127.0.0.1";
const baseHost = process.env.HOST || defaultHost;
envVars.BASE_URL = `http://${baseHost}:${PORT}`;
Object.assign(process.env, envVars);
} catch (error) {
Expand Down Expand Up @@ -744,9 +816,6 @@ async function main() {
// Open the browser automatically
// Use BASE_URL if set, otherwise construct from HOST and PORT
// Default: localhost in development, 127.0.0.1 in production
const defaultHost =
process.env.ENVIRONMENT === "dev" ? "localhost" : "127.0.0.1";
const host = process.env.HOST || defaultHost;
let url = process.env.BASE_URL || `http://${host}:${PORT}`;

// Append initial tab hash if specified
Expand Down
Loading