Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a graceful shutdown mechanism to address resource cleanup and resolves inconsistencies in the development server naming in documentation.
- Added a SIGINT handler in src/index.ts for graceful server shutdown.
- Updated the server name in CONTRIBUTING.md to reflect the new naming convention.
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/index.ts | Introduces a SIGINT-based shutdown; consider extending for other termination signals. |
| CONTRIBUTING.md | Renames the dev server to match the updated naming convention. |
Files not reviewed (1)
- .vscode/mcp.json: Language not supported
Comments suppressed due to low confidence (1)
CONTRIBUTING.md:69
- [nitpick] Ensure the updated server name 'xcodebuildmcp-dev' consistently matches naming conventions used in the project documentation and configuration files.
-1. **Start the MCP Server**: Open the `.vscode/mcp.json` file. You can start the `xcodebuildmcp-dev` server either by clicking the `Start` CodeLens that appears above the server definition, or by opening the Command Palette (`Cmd+Shift+P` or `Ctrl+Shift+P`), running `Mcp: List Servers`, selecting `xcodebuildmcp-dev`, and starting the server.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a graceful shutdown mechanism to address orphaned connections and potential error logs, and updates the dev server naming in the documentation.
- Adds SIGTERM and SIGINT handlers in src/index.ts to close the server gracefully
- Renames the dev server reference in CONTRIBUTING.md to ensure consistency
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/index.ts | Adds shutdown handlers for SIGTERM and SIGINT |
| CONTRIBUTING.md | Updates dev server name for consistency |
Files not reviewed (1)
- .vscode/mcp.json: Language not supported
src/index.ts
Outdated
| await server.close(); | ||
| process.exit(0); | ||
| }); | ||
|
|
||
| process.on('SIGINT', async () => { | ||
| await server.close(); | ||
| process.exit(0); |
There was a problem hiding this comment.
Consider wrapping the call to 'await server.close()' in a try-catch block within the signal handlers to gracefully handle potential shutdown errors and log any issues that arise.
| await server.close(); | |
| process.exit(0); | |
| }); | |
| process.on('SIGINT', async () => { | |
| await server.close(); | |
| process.exit(0); | |
| try { | |
| await server.close(); | |
| } catch (error) { | |
| log('error', 'Error during server shutdown on SIGTERM:', error); | |
| } finally { | |
| process.exit(0); | |
| } | |
| }); | |
| process.on('SIGINT', async () => { | |
| try { | |
| await server.close(); | |
| } catch (error) { | |
| log('error', 'Error during server shutdown on SIGINT:', error); | |
| } finally { | |
| process.exit(0); | |
| } |
| process.on('SIGTERM', async () => { | ||
| await server.close(); | ||
| process.exit(0); | ||
| }); | ||
|
|
||
| process.on('SIGINT', async () => { | ||
| await server.close(); | ||
| process.exit(0); | ||
| }); |
There was a problem hiding this comment.
[nitpick] The shutdown logic is duplicated for SIGTERM and SIGINT; consider refactoring into a common helper function to reduce code duplication and improve maintainability.
| process.on('SIGTERM', async () => { | |
| await server.close(); | |
| process.exit(0); | |
| }); | |
| process.on('SIGINT', async () => { | |
| await server.close(); | |
| process.exit(0); | |
| }); | |
| async function handleShutdown(server: any): Promise<void> { | |
| await server.close(); | |
| process.exit(0); | |
| } | |
| process.on('SIGTERM', () => handleShutdown(server)); | |
| process.on('SIGINT', () => handleShutdown(server)); |
There was a problem hiding this comment.
Yes that is a nitpick 🤣
.vscode/mcp.jsonandCONTRIBUTING.md