fix(signal): keep honoring signals after the force-quit window, add a hard shutdown deadline

CLAUDE.md

@@ -174,7 +174,7 @@ mcpproxy upstream logs <name> --follow    # per-server logs
 tail -f ~/Library/Logs/mcpproxy/main.log  # main log (macOS; Linux: ~/.mcpproxy/logs/main.log)
 ```
 
-**Exit codes**: 0 success · 1 general · 2 port conflict · 3 DB locked · 4 config · 5 permission.
+**Exit codes**: 0 success · 1 general · 2 port conflict · 3 DB locked · 4 config · 5 permission · 6 shutdown timeout (graceful shutdown exceeded its 60s hard deadline after SIGINT/SIGTERM).
 
 ## Development Guidelines
 

cmd/mcpproxy/exit_codes.go

@@ -28,8 +28,15 @@ const (
 	// ExitCodePermissionError indicates insufficient permissions (file access, port binding)
 	ExitCodePermissionError = 5
 
+	// ExitCodeShutdownTimeout indicates graceful shutdown exceeded its hard
+	// deadline after a SIGINT/SIGTERM and the process killed itself so a
+	// supervisor (launchd/systemd, or the tray's process monitor) can restart
+	// it. It is deliberately NOT produced by classifyError: it is not a
+	// startup outcome, it is a shutdown that never finished.
+	ExitCodeShutdownTimeout = 6
+
 	// Spec 098 preflight verdict codes. They are a SEPARATE band from the codes
-	// above on purpose: 0-5 describe whether mcpproxy could run, 10-12 describe
+	// above on purpose: 0-6 describe whether mcpproxy could run (or stop), 10-12 describe
 	// what a preflight found, so a cron wrapper can branch retry-vs-page-vs-fix
 	// on the exit code alone without parsing JSON (SC-003). Their values are the
 	// spec's, and preflight.ExitCode is the single mapping — these constants

cmd/mcpproxy/main.go

@@ -32,7 +32,6 @@ import (
 	"strings"
 	"sync/atomic"
 	"syscall"
-	"time"
 
 	"github.com/spf13/cobra"
 	bbolterrors "go.etcd.io/bbolt/errors"
@@ -623,35 +622,26 @@ func runServer(cmd *cobra.Command, _ []string) error {
 	var receivedSignal atomic.Value
 	receivedSignal.Store("")
 
-	// Setup signal handling for graceful shutdown with force quit on second signal
+	// Setup signal handling for graceful shutdown with force quit on a further
+	// signal and a hard deadline so a wedged shutdown cannot make the daemon
+	// unkillable. See runSignalHandler in signal_handler.go.
+	//
+	// This starts BEFORE the log line below on purpose: signal.Notify is
+	// already registered, so from here until something reads sigChan the
+	// runtime is swallowing SIGINT/SIGTERM into a one-slot buffer. A log write
+	// can block (a full disk, a stalled pipe to the tray), and a daemon that
+	// cannot be killed while it is logging is the bug this handler exists to
+	// prevent.
+	go runSignalHandler(signalHandlerDeps{
+		sigChan: sigChan,
+		cancel:  cancel,
+		// Spec 024: Store signal for activity logging.
+		onSignal: func(sig os.Signal) { receivedSignal.Store(sig.String()) },
+		exit:     os.Exit,
+		logger:   logger,
+	})
 	logger.Info("Signal handler goroutine starting - waiting for SIGINT or SIGTERM")
 	_ = logger.Sync()
-	go func() {
-		logger.Info("Signal handler goroutine is running, waiting for signal on channel")
-		_ = logger.Sync()
-		sig := <-sigChan
-		receivedSignal.Store(sig.String()) // Spec 024: Store signal for activity logging
-		logger.Info("Received signal, shutting down", zap.String("signal", sig.String()))
-		_ = logger.Sync() // Flush logs immediately so we can see shutdown messages
-		logger.Info("Press Ctrl+C again within 10 seconds to force quit")
-		_ = logger.Sync() // Flush again
-		cancel()
-
-		// Start a timer for force quit
-		forceQuitTimer := time.NewTimer(10 * time.Second)
-		defer forceQuitTimer.Stop()
-
-		// Wait for second signal or timeout
-		select {
-		case sig2 := <-sigChan:
-			logger.Warn("Received second signal, forcing immediate exit", zap.String("signal", sig2.String()))
-			_ = logger.Sync()
-			os.Exit(ExitCodeGeneralError)
-		case <-forceQuitTimer.C:
-			// Normal shutdown timeout - continue with graceful shutdown
-			logger.Debug("Force quit timer expired, continuing with graceful shutdown")
-		}
-	}()
 
 	// Start the server
 	logger.Info("Starting mcpproxy server")

cmd/mcpproxy/signal_handler.go

@@ -0,0 +1,146 @@
+package main
+
+import (
+	"context"
+	"os"
+	"sync"
+	"time"
+
+	"go.uber.org/zap"
+)
+
+const (
+	// shutdownGraceWindow is how long we wait before reminding the operator
+	// that the force-quit shortcut is still available. It does NOT bound the
+	// shutdown itself.
+	shutdownGraceWindow = 10 * time.Second
+
+	// shutdownHardDeadline bounds graceful shutdown as a whole, measured from
+	// the first signal. 60s is MCPProxy's chosen bound: long enough for Docker
+	// container cleanup, short enough that a supervisor can restart a wedged
+	// daemon instead of waiting for a human with SIGKILL. It sits under
+	// systemd's 90s DefaultTimeoutStopSec, so systemd's own SIGKILL stays a
+	// backstop rather than the normal outcome.
+	shutdownHardDeadline = 60 * time.Second
+)
+
+// signalHandlerDeps are the collaborators of runSignalHandler. Everything that
+// touches the process or the clock is injected so the handler can be unit
+// tested without real signals, real sleeps or a real os.Exit.
+type signalHandlerDeps struct {
+	// sigChan delivers SIGINT/SIGTERM (signal.Notify in production).
+	sigChan <-chan os.Signal
+	// cancel triggers the graceful shutdown path in runServer.
+	cancel context.CancelFunc
+	// onSignal records the first signal for Spec 024 activity logging. It MUST
+	// run before cancel(), because the ctx.Done() branch in runServer reads the
+	// stored value as soon as it wakes up.
+	onSignal func(os.Signal)
+	// exit terminates the process (os.Exit in production).
+	exit func(int)
+	// logger is never nil in production; tests pass zap.NewNop().
+	logger *zap.Logger
+	// newTimer returns a channel that fires after the given duration. It is
+	// called only AFTER the first signal arrives, so both windows are measured
+	// from that signal rather than from process start. nil means time.After.
+	newTimer func(time.Duration) <-chan time.Time
+}
+
+// runSignalHandler waits for the first shutdown signal, starts the graceful
+// shutdown, and then KEEPS LISTENING until the process dies.
+//
+// Staying alive is the whole point. signal.Notify stays registered on sigChan
+// for the lifetime of the process, so the moment nobody reads that channel the
+// Go runtime keeps intercepting SIGINT/SIGTERM (their default disposition is
+// disabled) and dropping them into a one-slot buffer. The old handler returned
+// as soon as its 10s force-quit timer expired; if shutdown then ran longer
+// than that - exactly when an operator is hammering Ctrl+C - the daemon became
+// unkillable by anything short of SIGKILL.
+//
+// The three ways out:
+//   - a further signal    -> immediate exit, ExitCodeGeneralError
+//   - the hard deadline   -> forced exit, ExitCodeShutdownTimeout
+//   - shutdown completing -> runServer returns and the process exits normally
+//
+// Both forced exits are decided by a small goroutine that touches nothing but
+// channels. Logging is deliberately kept off that path: the failure that wedges
+// shutdown (a full disk, a tray that stopped draining the core's stderr pipe)
+// is the same failure that wedges the log sink, so a forced exit that had to
+// get past a log write first would not be forced at all.
+func runSignalHandler(d signalHandlerDeps) {
+	newTimer := d.newTimer
+	if newTimer == nil {
+		newTimer = time.After
+	}
+
+	// Nothing may run before this receive - not even a log line. Whatever
+	// blocks here blocks the only reader of sigChan.
+	sig, open := <-d.sigChan
+	if !open {
+		return
+	}
+	started := time.Now()
+
+	// Arm both windows FIRST. Every instant spent before this line is an
+	// instant the "hard" deadline is not counting.
+	graceWindow := newTimer(shutdownGraceWindow)
+	hardDeadline := newTimer(shutdownHardDeadline)
+
+	// exiting is closed once some path has asked the process to die. In
+	// production d.exit never returns, so this only matters to the wait below
+	// (and to tests): once the decision is made there is nothing left to do.
+	exiting := make(chan struct{})
+	var exitOnce sync.Once
+	forceExit := func(code int) {
+		exitOnce.Do(func() {
+			d.exit(code)
+			close(exiting)
+		})
+	}
+
+	// handlerDone lets the forcer retire if this function returns first.
+	handlerDone := make(chan struct{})
+	defer close(handlerDone)
+
+	// The forcer: the only goroutine that may end the process, and the only
+	// reader of sigChan from here on. It never logs.
+	go func() {
+		select {
+		case _, ok := <-d.sigChan:
+			if !ok {
+				return
+			}
+			forceExit(ExitCodeGeneralError)
+		case <-hardDeadline:
+			forceExit(ExitCodeShutdownTimeout)
+		case <-handlerDone:
+		}
+	}()
+
+	d.onSignal(sig) // Spec 024: Store signal for activity logging (must precede cancel)
+	// Start the graceful shutdown before logging, for the same reason the
+	// timers are armed before logging: a blocked sink must not delay it.
+	d.cancel()
+
+	// From here on this goroutine only reports. If the sink is wedged it stalls
+	// here, and the forcer above still ends the process on time.
+	d.logger.Info("Received signal, shutting down", zap.String("signal", sig.String()))
+	_ = d.logger.Sync() // Flush logs immediately so we can see shutdown messages
+	d.logger.Info("Press Ctrl+C again to force quit",
+		zap.Int("force_quit_exit_code", ExitCodeGeneralError),
+		zap.Duration("forced_exit_after", shutdownHardDeadline),
+		zap.Int("forced_exit_code", ExitCodeShutdownTimeout))
+	_ = d.logger.Sync() // Flush again
+
+	select {
+	case <-graceWindow:
+		// Shutdown is taking a while. Remind the operator that Ctrl+C still
+		// works - and, unlike before, it really does.
+		d.logger.Info("Graceful shutdown still in progress, press Ctrl+C again to force quit",
+			zap.Duration("elapsed", time.Since(started)),
+			zap.Duration("forced_exit_after", shutdownHardDeadline))
+		_ = d.logger.Sync()
+		<-exiting
+	case <-exiting:
+	}
+}