From 67948df8cb998575f8ff2bf37737746d5b18e544 Mon Sep 17 00:00:00 2001 From: vikingowl Date: Wed, 20 May 2026 03:34:00 +0200 Subject: [PATCH] fix(mcp): make transport cross-compile on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `internal/mcp/transport.go` used syscall.Setpgid and syscall.Kill unconditionally, both Unix-only. Split the platform bits into `transport_unix.go` (build tag `!windows`) keeping the existing process-group semantics, and `transport_windows.go` (build tag `windows`) falling back to `os.Process.Kill` (kills only the immediate process — full process-tree kill on Windows would need golang.org/x/sys/windows + job objects, deferred). Caught by `goreleaser release --snapshot` cross-compiling for windows/amd64 and windows/arm64. --- internal/mcp/transport.go | 14 +++++++------- internal/mcp/transport_unix.go | 18 ++++++++++++++++++ internal/mcp/transport_windows.go | 18 ++++++++++++++++++ 3 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 internal/mcp/transport_unix.go create mode 100644 internal/mcp/transport_windows.go diff --git a/internal/mcp/transport.go b/internal/mcp/transport.go index 48699d4..1f77eb8 100644 --- a/internal/mcp/transport.go +++ b/internal/mcp/transport.go @@ -12,7 +12,6 @@ import ( "os/exec" "sync" "sync/atomic" - "syscall" "time" ) @@ -49,8 +48,9 @@ func NewTransport(command string, args []string, env map[string]string, logger * func (t *Transport) Start(ctx context.Context) error { t.cmd = exec.CommandContext(ctx, t.command, t.args...) t.cmd.Env = t.buildEnv() - // Create a new process group so Close can kill the entire tree. - t.cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + // Platform-specific: on Unix, isolate the child in a new process group + // so Close can kill the entire tree. + setProcessGroup(t.cmd) t.stderr = limitedWriter{max: maxStderrCapture} t.cmd.Stderr = &t.stderr @@ -146,10 +146,10 @@ func (t *Transport) Close() error { case <-time.After(2 * time.Second): } - // Graceful didn't work — kill the entire process group. - t.logger.Warn("mcp: server did not exit, killing process group", "command", t.command) - if err := syscall.Kill(-t.cmd.Process.Pid, syscall.SIGKILL); err != nil { - t.logger.Warn("mcp: SIGKILL to process group failed", + // Graceful didn't work — kill the process (Unix: whole group; Windows: just the process). + t.logger.Warn("mcp: server did not exit, killing", "command", t.command) + if err := killProcessTree(t.cmd.Process); err != nil { + t.logger.Warn("mcp: kill failed", "command", t.command, "pid", t.cmd.Process.Pid, "error", err) } return <-done diff --git a/internal/mcp/transport_unix.go b/internal/mcp/transport_unix.go new file mode 100644 index 0000000..aefd81c --- /dev/null +++ b/internal/mcp/transport_unix.go @@ -0,0 +1,18 @@ +//go:build !windows + +package mcp + +import ( + "os" + "os/exec" + "syscall" +) + +func setProcessGroup(cmd *exec.Cmd) { + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} +} + +func killProcessTree(p *os.Process) error { + // Negative PID targets the process group created via Setpgid. + return syscall.Kill(-p.Pid, syscall.SIGKILL) +} diff --git a/internal/mcp/transport_windows.go b/internal/mcp/transport_windows.go new file mode 100644 index 0000000..a1d1b85 --- /dev/null +++ b/internal/mcp/transport_windows.go @@ -0,0 +1,18 @@ +//go:build windows + +package mcp + +import ( + "os" + "os/exec" +) + +// setProcessGroup is a no-op on Windows; we rely on os.Process.Kill in Close. +// Pulling in golang.org/x/sys/windows + job objects to kill the full process +// tree would be the upgrade path if MCP servers ever spawn children we need +// to reap. +func setProcessGroup(cmd *exec.Cmd) {} + +func killProcessTree(p *os.Process) error { + return p.Kill() +}