From 9714a6f4c215f54e84c318f25d7c92b45858f4fd Mon Sep 17 00:00:00 2001 From: Abhisek Datta Date: Wed, 10 Jun 2026 09:57:08 +0530 Subject: [PATCH] fix(pty): treat background jobs as non-interactive to avoid SIGTTOU stop (#324) * fix(pty): treat background jobs as non-interactive to avoid SIGTTOU stop IsInteractiveTerminal only checked that stdin/stdout are TTYs. A background job (pmg npm run test &) still has the TTY on stdin/stdout, so pmg picked PTY mode and called tcsetattr to enter raw mode. Changing terminal modes from a background process group makes the kernel stop the process with SIGTTOU, leaving the job hanging in Stopped state. Check that the process group is the terminal's foreground process group (tcgetpgrp == getpgrp) before treating the terminal as interactive, so background jobs fall through to direct execution. Fixes #322 https://claude.ai/code/session_01PBBo5CKkzg68MMGrgCTQcY * test(pty): fail on output copy timeout to avoid racy buffer read Reading the output buffer after a silent select timeout races with the io.Copy goroutine still writing to it. Fail the test on timeout instead. Also fix a grammar nit in the IsInteractiveTerminal doc comment. https://claude.ai/code/session_01PBBo5CKkzg68MMGrgCTQcY --------- Co-authored-by: Claude --- internal/pty/foreground_unix.go | 21 ++++++ internal/pty/foreground_windows.go | 9 +++ internal/pty/session.go | 10 ++- internal/pty/session_unix_test.go | 113 +++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 4 deletions(-) create mode 100644 internal/pty/foreground_unix.go create mode 100644 internal/pty/foreground_windows.go create mode 100644 internal/pty/session_unix_test.go diff --git a/internal/pty/foreground_unix.go b/internal/pty/foreground_unix.go new file mode 100644 index 0000000..ee612e1 --- /dev/null +++ b/internal/pty/foreground_unix.go @@ -0,0 +1,21 @@ +//go:build !windows + +package pty + +import ( + "golang.org/x/sys/unix" +) + +// isForegroundProcess reports whether this process belongs to the terminal's +// foreground process group. A background job (e.g. `pmg npm install &`) still +// has the TTY on stdin/stdout, but changing terminal modes or reading from +// the TTY in a background process group triggers SIGTTOU/SIGTTIN, which stops +// the process. +func isForegroundProcess(fd uintptr) bool { + foregroundPgrp, err := unix.IoctlGetInt(int(fd), unix.TIOCGPGRP) + if err != nil { + return false + } + + return foregroundPgrp == unix.Getpgrp() +} diff --git a/internal/pty/foreground_windows.go b/internal/pty/foreground_windows.go new file mode 100644 index 0000000..332c668 --- /dev/null +++ b/internal/pty/foreground_windows.go @@ -0,0 +1,9 @@ +//go:build windows + +package pty + +// Windows has no Unix-style terminal job control, so a process is never +// stopped for touching the console from the "background". +func isForegroundProcess(_ uintptr) bool { + return true +} diff --git a/internal/pty/session.go b/internal/pty/session.go index 6c73db1..a5dcf39 100644 --- a/internal/pty/session.go +++ b/internal/pty/session.go @@ -42,9 +42,11 @@ type InteractiveSession interface { Close() error } -// IsInteractiveTerminal returns true if stdin is a real terminal (TTY). -// Returns false in CI environments (when the "CI" env var set to "true"), -// when input or output is piped, or in non-interactive shells. +// IsInteractiveTerminal returns true if stdin is a real terminal (TTY) and +// the process can safely drive it. Returns false in CI environments (when the +// "CI" env var is set to "true"), when input or output is piped, in +// non-interactive shells, or when running as a background job (where putting +// the terminal into raw mode would stop the process with SIGTTOU). func IsInteractiveTerminal() bool { if ci := os.Getenv("CI"); ci != "" && strings.ToLower(ci) == "true" { return false @@ -58,7 +60,7 @@ func IsInteractiveTerminal() bool { return false } - return true + return isForegroundProcess(os.Stdin.Fd()) } var _ InteractiveSession = &session{} diff --git a/internal/pty/session_unix_test.go b/internal/pty/session_unix_test.go new file mode 100644 index 0000000..e84938e --- /dev/null +++ b/internal/pty/session_unix_test.go @@ -0,0 +1,113 @@ +//go:build !windows + +package pty + +import ( + "bytes" + "context" + "fmt" + "io" + "os" + "os/exec" + "strings" + "syscall" + "testing" + "time" + + "github.com/safedep/ptyx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ttyHelperEnv = "PMG_TEST_TTY_HELPER" + +// Reproduces https://github.com/safedep/pmg/issues/322: a pmg process started +// as a background job (`pmg npm run test &`) still has the TTY on +// stdin/stdout, but it is not in the terminal's foreground process group. +// Treating it as interactive makes the PTY session call tcsetattr, which +// stops the process with SIGTTOU. IsInteractiveTerminal must report false in +// that situation. +func TestIsInteractiveTerminalBackgroundJob(t *testing.T) { + exe, err := os.Executable() + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + // The helper runs on a fresh PTY as the foreground process group, then + // moves itself to the background, checking IsInteractiveTerminal in both + // states. + sess, err := ptyx.Spawn(ctx, ptyx.SpawnOpts{ + Prog: exe, + Args: []string{"-test.run", "^TestIsInteractiveTerminalTTYHelper$", "-test.v"}, + Env: append(envWithoutCI(), ttyHelperEnv+"=1"), + Cols: 80, + Rows: 24, + }) + require.NoError(t, err) + defer func() { + require.NoError(t, sess.Close()) + }() + + var output bytes.Buffer + copyDone := make(chan struct{}) + go func() { + defer close(copyDone) + _, _ = io.Copy(&output, sess.PtyReader()) + }() + + waitErr := sess.Wait() + + // The buffer must not be read until the copy goroutine is done, both to + // avoid a data race and to ensure all helper output has been captured. + select { + case <-copyDone: + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for pty output copy to finish") + } + + out := output.String() + require.NoError(t, waitErr, "helper failed, output:\n%s", out) + assert.Contains(t, out, "foreground_interactive=true", "expected interactive in foreground, output:\n%s", out) + assert.Contains(t, out, "background_interactive=false", "expected non-interactive in background, output:\n%s", out) +} + +// TestIsInteractiveTerminalTTYHelper is not a real test. It is re-executed by +// TestIsInteractiveTerminalBackgroundJob on a PTY where it starts as the +// foreground process group. +func TestIsInteractiveTerminalTTYHelper(t *testing.T) { + if os.Getenv(ttyHelperEnv) != "1" { + t.Skip("helper for TestIsInteractiveTerminalBackgroundJob") + } + + fmt.Printf("foreground_interactive=%v\n", IsInteractiveTerminal()) + + // Hand the terminal's foreground process group to a child, like a shell + // does for the foreground job. This process then becomes a background job + // on the TTY, matching `pmg npm run test &`. + cmd := exec.Command("sleep", "30") + cmd.SysProcAttr = &syscall.SysProcAttr{ + Setpgid: true, + Foreground: true, + Ctty: int(os.Stdin.Fd()), + } + require.NoError(t, cmd.Start()) + defer func() { + _ = cmd.Process.Kill() + _, _ = cmd.Process.Wait() + }() + + fmt.Printf("background_interactive=%v\n", IsInteractiveTerminal()) +} + +func envWithoutCI() []string { + env := make([]string, 0, len(os.Environ())) + for _, entry := range os.Environ() { + if strings.HasPrefix(entry, "CI=") { + continue + } + env = append(env, entry) + } + + return env +}