mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-21 20:28:24 +08:00
fix(browse): gate #994 SIGTERM-ignore to normal mode only
PR #994 made browse persist across Claude Code Bash calls by ignoring SIGTERM and parent-PID death, relying on the 30-min idle timeout for eventual cleanup. Codex outside-voice review caught that the idle timeout doesn't apply in two modes: headed mode (/open-gstack-browser) and tunnel mode (/pair-agent). Both early-return from idleCheckInterval. Combined with #994's ignore-SIGTERM, those sessions would leak forever after the user disconnects — a real resource leak on shared machines where multiple /pair-agent sessions come and go. Fix: gate SIGTERM-ignore and parent-PID-watchdog-ignore to normal (headless) mode only. Headed + tunnel modes respect both signals and shutdown cleanly. Idle timeout behavior unchanged. Also documents the deliberate contract change for future contributors — don't re-add global SIGTERM shutdown thinking it's missing; it's intentionally scoped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -764,10 +764,19 @@ if (BROWSE_PARENT_PID > 0) {
|
|||||||
try {
|
try {
|
||||||
process.kill(BROWSE_PARENT_PID, 0); // signal 0 = existence check only, no signal sent
|
process.kill(BROWSE_PARENT_PID, 0); // signal 0 = existence check only, no signal sent
|
||||||
} catch {
|
} catch {
|
||||||
// Parent exited. Don't shutdown — the server should persist across
|
// Parent exited. Behavior depends on mode:
|
||||||
// Bash calls (e.g. Claude Code sandbox kills the parent shell).
|
// - Normal (headless) mode: stay alive. Claude Code's Bash tool kills the
|
||||||
// The idle timeout (30 min) handles eventual cleanup instead.
|
// parent shell between invocations, so the server must survive. The
|
||||||
if (!parentGone) {
|
// idle timeout (30 min) handles eventual cleanup.
|
||||||
|
// - Headed / tunnel mode: the idle timeout DOESN'T apply (see idleCheckInterval
|
||||||
|
// above — both modes early-return). If we ignored parent death here too,
|
||||||
|
// orphan daemons would accumulate forever after /pair-agent or
|
||||||
|
// /open-gstack-browser sessions end. Shutdown instead.
|
||||||
|
const headed = browserManager.getConnectionMode() === 'headed';
|
||||||
|
if (headed || tunnelActive) {
|
||||||
|
console.log(`[browse] Parent process ${BROWSE_PARENT_PID} exited in ${headed ? 'headed' : 'tunnel'} mode, shutting down`);
|
||||||
|
shutdown();
|
||||||
|
} else if (!parentGone) {
|
||||||
parentGone = true;
|
parentGone = true;
|
||||||
console.log(`[browse] Parent process ${BROWSE_PARENT_PID} exited (server stays alive, idle timeout will clean up)`);
|
console.log(`[browse] Parent process ${BROWSE_PARENT_PID} exited (server stays alive, idle timeout will clean up)`);
|
||||||
}
|
}
|
||||||
@@ -1233,10 +1242,21 @@ async function shutdown() {
|
|||||||
// Handle signals
|
// Handle signals
|
||||||
// SIGINT (Ctrl+C): user intentionally stopping → shutdown
|
// SIGINT (Ctrl+C): user intentionally stopping → shutdown
|
||||||
process.on('SIGINT', shutdown);
|
process.on('SIGINT', shutdown);
|
||||||
// SIGTERM: may come from sandbox killing parent → ignore and let idle timeout handle cleanup.
|
// SIGTERM behavior depends on mode:
|
||||||
// Explicit shutdown is available via the /stop command or SIGINT.
|
// - Normal (headless) mode: Claude Code's Bash sandbox fires SIGTERM when the
|
||||||
|
// parent shell exits between tool invocations. Ignoring it keeps the server
|
||||||
|
// alive across $B calls. Idle timeout (30 min) handles eventual cleanup.
|
||||||
|
// - Headed / tunnel mode: idle timeout doesn't apply in these modes. Respect
|
||||||
|
// SIGTERM so external tooling (systemd, supervisord, CI) can shut cleanly
|
||||||
|
// without waiting forever. Ctrl+C and /stop still work either way.
|
||||||
process.on('SIGTERM', () => {
|
process.on('SIGTERM', () => {
|
||||||
console.log('[browse] Received SIGTERM (ignoring — use /stop or Ctrl+C for intentional shutdown)');
|
const headed = browserManager.getConnectionMode() === 'headed';
|
||||||
|
if (headed || tunnelActive) {
|
||||||
|
console.log(`[browse] Received SIGTERM in ${headed ? 'headed' : 'tunnel'} mode, shutting down`);
|
||||||
|
shutdown();
|
||||||
|
} else {
|
||||||
|
console.log('[browse] Received SIGTERM (ignoring — use /stop or Ctrl+C for intentional shutdown)');
|
||||||
|
}
|
||||||
});
|
});
|
||||||
// Windows: taskkill /F bypasses SIGTERM, but 'exit' fires for some shutdown paths.
|
// Windows: taskkill /F bypasses SIGTERM, but 'exit' fires for some shutdown paths.
|
||||||
// Defense-in-depth — primary cleanup is the CLI's stale-state detection via health check.
|
// Defense-in-depth — primary cleanup is the CLI's stale-state detection via health check.
|
||||||
|
|||||||
Reference in New Issue
Block a user