fix: handle graceful shutdown in McpServer.run()

[qchuchu]

Summary

• Register SIGTERM and SIGINT signal handlers in McpServer.run() to gracefully close the HTTP server before process exit
• Prevents EADDRINUSE errors when using nodemon or other process managers that restart the server
• Adds a 3-second forced exit timeout as a safety net if server.close() hangs

Fixes #481

Test plan

• Start a Skybridge server with nodemon, verify restarts no longer cause EADDRINUSE
• Send SIGTERM to a running server, verify it exits cleanly
• Send SIGINT (Ctrl+C) to a running server, verify it exits cleanly
• Verify the build passes (pnpm --filter skybridge build)

🤖 Generated with Claude Code

Greptile Summary

This PR adds graceful shutdown handling to McpServer.run() by registering SIGTERM and SIGINT signal handlers that close the HTTP server before process exit. This prevents EADDRINUSE errors when using process managers like nodemon that restart the server. A 3-second forced-exit timeout acts as a safety net if server.close() hangs.

• Registers SIGTERM and SIGINT handlers that call server.close() followed by process.exit(0)
• Adds a 3-second setTimeout fallback that calls process.exit(1) if server.close() doesn't complete
• The change is small, focused, and follows a well-established Node.js graceful shutdown pattern

Confidence Score: 4/5

• This PR is safe to merge — it adds a standard graceful shutdown pattern with minimal risk.
• The change is small (8 lines), follows a well-known Node.js pattern for graceful shutdown, and only affects the run() method's lifecycle behavior. The logic is straightforward: close the server on signal, force-exit after 3 seconds if it hangs. One minor style suggestion (.unref() on the timer) but no functional issues.
• No files require special attention.

_{Last reviewed commit: 75daca4}

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Consider calling .unref() on the safety-net timer

The setTimeout timer will keep the Node.js event loop alive for 3 seconds even after server.close() completes (though process.exit(0) in the callback makes this moot in the happy path). Calling .unref() is a common best practice for safety-net timers so they don't prevent natural process exit in edge cases.

Suggested change

	setTimeout(() => process.exit(1), 3000);
	setTimeout(() => process.exit(1), 3000).unref();

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/core/src/server/server.ts
Line: 273:273

Comment:
**Consider calling `.unref()` on the safety-net timer**

The `setTimeout` timer will keep the Node.js event loop alive for 3 seconds even after `server.close()` completes (though `process.exit(0)` in the callback makes this moot in the happy path). Calling `.unref()` is a common best practice for safety-net timers so they don't prevent natural process exit in edge cases.

```suggestion
        setTimeout(() => process.exit(1), 3000).unref();
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

[fredericbarthelet]

I agree with greptile, and I'm not sure why you even need the setTimeout

[fredericbarthelet]

Can you use reject instead of process.exit(1) by the way ?

[greptile-apps]

Looking at the context, the setTimeout is a safety net for when server.close() hangs or takes too long. Using reject() here wouldn't work as expected because:

1. Wrong scope: The reject is already out of scope - it belongs to the Promise created at the start of run() that was resolved when the server started listening successfully (lines before this code)

2. Already resolved: By the time we're in the shutdown handler, the Promise has already been resolved (server is running), so calling reject would have no effect

3. Process is exiting anyway: This is a signal handler for SIGTERM/SIGINT - the process is meant to exit, not reject a promise

However, I do see an issue: the force-exit timer should probably use process.exit(0) instead of process.exit(1), since a timeout during graceful shutdown isn't necessarily an error state - it's just taking longer than expected. Or if you want to keep it as exit code 1 to signal the timeout issue, that's also reasonable.

The real question is whether you even need the timeout at all. For a simple HTTP server without persistent connections or complex cleanup, server.close() should complete quickly. You could simplify to:

Suggested change

	setTimeout(() => process.exit(1), 3000);
	const shutdown = () => {
	server.close(() => process.exit(0));
	};

Unless you have long-lived connections or complex cleanup logic that might cause server.close() to hang, the timeout is probably unnecessary.