From 8c24522b9459c3cb5c3d9df78fc49f9fd52e9a38 Mon Sep 17 00:00:00 2001 From: Dominic Black Date: Fri, 9 Oct 2026 13:03:07 +0100 Subject: [PATCH] Keep children of a TTY-less runner away from the terminal An MCP stack_create in a repo with commit.gpgsign on and an SSH signing key that wasn't loaded into ssh-agent never came back. Capture mode gives git a closed stdin, but ssh-keygen asks for the passphrase on /dev/tty (the controlling terminal), and every child inherits that from whatever terminal the MCP server was started in. So git sat on a prompt nobody could see or answer until someone killed ssh-keygen by hand. gpg's pinentry and git's own credential prompt can do exactly the same. A runner built without a TTY now starts each child in its own session (setsid), so there is no controlling terminal and opening /dev/tty fails; it also sets GIT_TERMINAL_PROMPT=0 and drops GPG_TTY. The prompt turns into an immediate error the tool can return instead of a hang. The CLI runner with a terminal is untouched, so a passphrase prompt in an ordinary git stack create still works as before. --- docs/architecture.md | 13 ++++++-- pkg/exec/detach_other.go | 11 +++++++ pkg/exec/detach_unix.go | 12 ++++++++ pkg/exec/runner.go | 20 ++++++++++++ pkg/exec/runner_test.go | 66 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 pkg/exec/detach_other.go create mode 100644 pkg/exec/detach_unix.go diff --git a/docs/architecture.md b/docs/architecture.md index 5cca428..069abc0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -59,8 +59,17 @@ and nothing outside `pkg/exec` touches `os/exec`. Everything goes through `pkg/exec.Runner`. Capture is the default. Passthrough (a real TTY handed to the child) is opt in, only exists on the CLI runner, and is only used for `git add -p`, editors, and `gh stack submit`'s interactive editor. The MCP runner is built -without a TTY so it physically can't enter passthrough mode, which is how we guarantee an -MCP tool never blocks waiting on a terminal. +without a TTY so it physically can't enter passthrough mode. + +That alone wasn't enough to keep an MCP tool from blocking on a terminal. Capture mode +hands the child a closed stdin, but ssh-keygen, gpg and git's credential code prompt on +`/dev/tty`, the controlling terminal, which every child inherits from the terminal the MCP +server was started in; a `stack_create` with `commit.gpgsign` and a locked SSH key sat on a +passphrase prompt nobody could see. So a runner without a TTY starts each child in its own +session (`setsid`, so there is no controlling terminal and `/dev/tty` fails to open), sets +`GIT_TERMINAL_PROMPT=0` and drops `GPG_TTY`. The prompt turns into an immediate error that +the use case can explain. A CLI runner with a terminal does none of this: its user can +answer the prompt. `pkg/git` also retries a command that failed because another process held `.git/index.lock`, backing off from 25ms up to 250ms for about 2 seconds in total before diff --git a/pkg/exec/detach_other.go b/pkg/exec/detach_other.go new file mode 100644 index 0000000..9c3490f --- /dev/null +++ b/pkg/exec/detach_other.go @@ -0,0 +1,11 @@ +//go:build !unix + +package exec + +import "syscall" + +// detached is a no-op where sessions don't exist; there is no /dev/tty to +// keep a child away from either. +func detached() *syscall.SysProcAttr { + return nil +} diff --git a/pkg/exec/detach_unix.go b/pkg/exec/detach_unix.go new file mode 100644 index 0000000..cf1031b --- /dev/null +++ b/pkg/exec/detach_unix.go @@ -0,0 +1,12 @@ +//go:build unix + +package exec + +import "syscall" + +// detached returns the process attributes that start a child in its own +// session, so it has no controlling terminal: opening /dev/tty fails +// instead of reaching the terminal this process was started from. +func detached() *syscall.SysProcAttr { + return &syscall.SysProcAttr{Setsid: true} +} diff --git a/pkg/exec/runner.go b/pkg/exec/runner.go index a0a1c5b..63ae505 100644 --- a/pkg/exec/runner.go +++ b/pkg/exec/runner.go @@ -6,6 +6,13 @@ // whatever the caller supplies (never the parent's stdin). Passthrough mode // attaches the real terminal and is only available to runners built with // WithTTY; the MCP server never constructs such a runner. +// +// A runner without a TTY also keeps its children away from the terminal the +// process happens to have been started from: they run in their own session +// (no controlling terminal, so /dev/tty cannot be opened), git's terminal +// prompts are disabled and GPG_TTY is dropped. Otherwise a passphrase or +// credential prompt from ssh-keygen, gpg or git would wait on a terminal +// nobody is watching, which is how the MCP server used to hang. package exec import ( @@ -139,6 +146,15 @@ func WithTTY(t TTY) Option { // context is cancelled before killing it. const waitDelay = 2 * time.Second +// noPromptEnv and noPromptUnset apply to every child of a runner without a +// TTY. GIT_TERMINAL_PROMPT=0 makes git fail a credential prompt outright +// rather than look for a terminal; dropping GPG_TTY stops gpg pointing its +// pinentry at the terminal this process inherited. +var ( + noPromptEnv = []string{"GIT_TERMINAL_PROMPT=0"} + noPromptUnset = []string{"GPG_TTY"} +) + type runner struct { debug io.Writer tty *TTY @@ -161,6 +177,10 @@ func (r *runner) Run(ctx context.Context, c Cmd) (Result, error) { cmd := osexec.CommandContext(ctx, c.Name, c.Args...) cmd.Dir = c.Dir cmd.Env = BuildEnv(os.Environ(), c.Env, c.Unset) + if r.tty == nil { + cmd.Env = BuildEnv(cmd.Env, noPromptEnv, noPromptUnset) + cmd.SysProcAttr = detached() + } cmd.WaitDelay = waitDelay cmd.Cancel = func() error { // Give the child a chance to clean up (e.g. git rebase state) before diff --git a/pkg/exec/runner_test.go b/pkg/exec/runner_test.go index d9208f9..b1e7494 100644 --- a/pkg/exec/runner_test.go +++ b/pkg/exec/runner_test.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "errors" + "os" + "runtime" "strings" "testing" "time" @@ -120,3 +122,67 @@ func TestBuildEnv(t *testing.T) { t.Errorf("BuildEnv = %v, want %v", got, want) } } + +// hasControllingTerminal reports whether this test process can open +// /dev/tty, which is what ssh-keygen and gpg do to prompt for a passphrase. +func hasControllingTerminal(t *testing.T) bool { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("no /dev/tty on windows") + } + _, err := exec.New(exec.WithTTY(exec.TTY{In: os.Stdin, Out: os.Stdout, Err: os.Stderr})). + Run(context.Background(), exec.Cmd{Name: "sh", Args: []string{"-c", "exec 3