fix: Add SIGHUP handler and stdin close detection to prevent orphaned processes#54
fix: Add SIGHUP handler and stdin close detection to prevent orphaned processes#54shahabzohaib wants to merge 1 commit intoobra:mainfrom
Conversation
… processes When Claude sessions end through terminal closure, crashes, or SIGKILL, the MCP server processes were becoming orphaned because only SIGTERM and SIGINT were being handled. Changes: - Add SIGHUP signal handler for terminal hangup (closing terminal window) - Add stdin 'end' event listener to detect parent process death This ensures child processes are properly cleaned up in all exit scenarios: - Terminal window closed (SIGHUP) - Claude crashes (stdin closes) - Claude killed with SIGKILL (stdin closes) - Normal /exit (SIGTERM - already worked) Fixes obra#53 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds signal forwarding and parent process death detection to the MCP server wrapper to prevent child processes from becoming orphaned when Claude sessions terminate unexpectedly or abnormally. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cli/mcp-server-wrapper.js`:
- Around line 89-94: The 'end' listener on process.stdin won't fire because
stdin is paused by default; call process.stdin.resume() before attaching the
'end' handler so the stream enters flowing mode and the parent-death detection
works. Update the block that sets process.stdin.on('end', ...) (where
child.kill() and process.exit(0) are invoked) to call process.stdin.resume()
first; ensure this doesn't conflict with other code reading stdin (or gate it
behind a flag) and optionally add an 'error' listener to handle stream errors
gracefully.
| // Detect parent process death via stdin close | ||
| // When Claude exits (normally or abnormally), stdin will close | ||
| process.stdin.on('end', () => { | ||
| child.kill(); | ||
| process.exit(0); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Node.js process.stdin 'end' event listener paused mode resume
💡 Result:
In Node.js, a Readable stream’s 'end' event won’t fire unless the stream is fully consumed. If process.stdin stays in paused mode (no consumption), Node won’t read to EOF, so 'end' may never emit. [2]
To make 'end' fire, you must consume or explicitly drain stdin by doing one of the following:
Option A: Switch to flowing mode (common)
Add a 'data' handler (consumes data) or call .resume() (drains data even if you ignore it):
process.stdin.resume(); // start flowing/draining
process.stdin.on('end', () => {
console.log('stdin ended');
});All Readable streams start in paused mode; calling resume() switches them to flowing mode. [1][4]
Option B: Use 'readable' + read() until EOF
process.stdin.on('readable', () => {
while (process.stdin.read() !== null) {}
});
process.stdin.on('end', () => console.log('stdin ended'));Reading until read() returns null ensures the stream is consumed so 'end' can emit. [2]
Note about process.stdin and “old mode”
Node docs note that process.stdin.resume() can switch stdin into the legacy (“old streams”) mode; in that mode stdin is paused by default unless resumed. [3]
Sources:
[1] Node.js Stream docs (paused vs flowing; resume()) (r2.nodejs.org)
[2] Node.js Stream docs ('end' requires complete consumption) (r2.nodejs.org)
[3] Node.js Process docs (process.stdin, old mode note) (nodejs.org)
[4] Readable.resume() behavior/example (bun.com)
Citations:
- 1: https://r2.nodejs.org/docs/v24.0.1/api/stream.html?utm_source=openai
- 2: https://r2.nodejs.org/docs/v0.12.3/api/stream.html?utm_source=openai
- 3: https://nodejs.org/api/process.html?utm_source=openai
- 4: https://bun.com/reference/node/stream/default/Readable/resume?utm_source=openai
process.stdin.resume() is required for the 'end' event to fire.
In Node.js, process.stdin (like all Readable streams) starts in paused mode. The 'end' event only fires when the stream is fully consumed. Simply attaching an 'end' listener does not enable flowing mode, so this handler will never trigger. Without calling process.stdin.resume(), the parent process death detection won't work.
🐛 Proposed fix
// Detect parent process death via stdin close
// When Claude exits (normally or abnormally), stdin will close
+ process.stdin.resume();
process.stdin.on('end', () => {
child.kill();
process.exit(0);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Detect parent process death via stdin close | |
| // When Claude exits (normally or abnormally), stdin will close | |
| process.stdin.on('end', () => { | |
| child.kill(); | |
| process.exit(0); | |
| }); | |
| // Detect parent process death via stdin close | |
| // When Claude exits (normally or abnormally), stdin will close | |
| process.stdin.resume(); | |
| process.stdin.on('end', () => { | |
| child.kill(); | |
| process.exit(0); | |
| }); |
🤖 Prompt for AI Agents
In `@cli/mcp-server-wrapper.js` around lines 89 - 94, The 'end' listener on
process.stdin won't fire because stdin is paused by default; call
process.stdin.resume() before attaching the 'end' handler so the stream enters
flowing mode and the parent-death detection works. Update the block that sets
process.stdin.on('end', ...) (where child.kill() and process.exit(0) are
invoked) to call process.stdin.resume() first; ensure this doesn't conflict with
other code reading stdin (or gate it behind a flag) and optionally add an
'error' listener to handle stream errors gracefully.
- Redirect embedding model log messages from stdout to stderr and suppress @xenova/transformers progress callbacks. Stdout is reserved for MCP JSON-RPC communication and any non-JSON output corrupts the protocol. (credit: obra#48) - Add 'clear' to the SessionStart hook matcher so conversations are archived when users run /clear, not just on startup/resume. (credit: obra#51) - Add SIGHUP handler and stdin close detection to the MCP server wrapper so child processes are cleaned up when the terminal closes or Claude crashes. (credit: obra#54)
Summary
endevent listener to detect parent process deathProblem
MCP server processes become orphaned when Claude sessions end through:
kill -9on Claude process (no signal sent)Over time, users accumulate dozens of orphaned
mcp-server.jsprocesses consuming memory.Root Cause
The wrapper only handled SIGTERM and SIGINT:
Solution
Add two additional cleanup mechanisms:
The stdin detection is particularly important because it catches all abnormal parent exits, including crashes and SIGKILL.
Test Plan
/exitstill works (SIGTERM path)kill -9on Claude kills child process (stdin close path)Fixes #53
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.