From c3a3cf5f57aa84a6c6731f85927f4c0ef344ae1e Mon Sep 17 00:00:00 2001 From: Nitzan Volman Date: Sun, 3 May 2026 16:01:32 +0300 Subject: [PATCH] Fix TUI startup foreground tty race --- go.mod | 2 +- internal/statusui/statusui.go | 14 ++++ internal/statusui/tty_foreground_other.go | 14 ++++ internal/statusui/tty_foreground_unix.go | 61 ++++++++++++++++ internal/statusui/tty_foreground_unix_test.go | 73 +++++++++++++++++++ 5 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 internal/statusui/tty_foreground_other.go create mode 100644 internal/statusui/tty_foreground_unix.go create mode 100644 internal/statusui/tty_foreground_unix_test.go diff --git a/go.mod b/go.mod index 8a6d205..27ebd5a 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/charmbracelet/log v1.0.0 github.com/go-logfmt/logfmt v0.6.1 // indirect golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect + golang.org/x/sys v0.38.0 ) require ( @@ -53,7 +54,6 @@ require ( github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect golang.org/x/mod v0.29.0 // indirect golang.org/x/sync v0.18.0 // indirect - golang.org/x/sys v0.38.0 // indirect golang.org/x/text v0.30.0 // indirect golang.org/x/tools v0.38.0 // indirect gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect diff --git a/internal/statusui/statusui.go b/internal/statusui/statusui.go index d61b400..652d210 100644 --- a/internal/statusui/statusui.go +++ b/internal/statusui/statusui.go @@ -2,6 +2,7 @@ package statusui import ( "context" + "errors" "log/slog" "os" @@ -15,6 +16,19 @@ import ( // cancelled or the user presses q. buf may be nil (log pane disabled). // cancelFn is called when the user presses x to kill the selected session; nil disables it. func Run(ctx context.Context, snap func() server.StateSnapshot, buf *logbuffer.Buffer, cfg Config, cancelFn func(string) bool) { + if err := waitForForegroundTTY(ctx); err != nil { + switch { + case errors.Is(err, context.Canceled), errors.Is(err, context.DeadlineExceeded): + return + case errors.Is(err, errNoControllingTTY): + slog.Info("statusui: no controlling tty; terminal UI disabled", "error", err) + return + default: + slog.Warn("statusui: foreground tty check failed; terminal UI disabled", "error", err) + return + } + } + m := New(snap, buf, cfg, cancelFn) p := tea.NewProgram(m, tea.WithOutput(os.Stderr), diff --git a/internal/statusui/tty_foreground_other.go b/internal/statusui/tty_foreground_other.go new file mode 100644 index 0000000..7a5432a --- /dev/null +++ b/internal/statusui/tty_foreground_other.go @@ -0,0 +1,14 @@ +//go:build !darwin && !dragonfly && !freebsd && !linux && !netbsd && !openbsd && !solaris && !aix && !zos + +package statusui + +import ( + "context" + "errors" +) + +var errNoControllingTTY = errors.New("statusui: no controlling tty") + +func waitForForegroundTTY(context.Context) error { + return nil +} diff --git a/internal/statusui/tty_foreground_unix.go b/internal/statusui/tty_foreground_unix.go new file mode 100644 index 0000000..f02b203 --- /dev/null +++ b/internal/statusui/tty_foreground_unix.go @@ -0,0 +1,61 @@ +//go:build darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris || aix || zos + +package statusui + +import ( + "context" + "errors" + "fmt" + "os" + "time" + + "golang.org/x/sys/unix" +) + +var errNoControllingTTY = errors.New("statusui: no controlling tty") + +const foregroundTTYCheckInterval = 10 * time.Millisecond + +func waitForForegroundTTY(ctx context.Context) error { + tty, err := os.Open("/dev/tty") + if err != nil { + return fmt.Errorf("%w: %w", errNoControllingTTY, err) + } + defer tty.Close() //nolint:errcheck + + return waitForForegroundProcessGroup( + ctx, + unix.Getpgrp, + func() (int, error) { + return unix.IoctlGetInt(int(tty.Fd()), unix.TIOCGPGRP) + }, + foregroundTTYCheckInterval, + ) +} + +func waitForForegroundProcessGroup( + ctx context.Context, + currentPGID func() int, + foregroundPGID func() (int, error), + interval time.Duration, +) error { + if interval <= 0 { + interval = foregroundTTYCheckInterval + } + + for { + foreground, err := foregroundPGID() + if err != nil { + return fmt.Errorf("statusui: foreground process group: %w", err) + } + if foreground == currentPGID() { + return nil + } + + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(interval): + } + } +} diff --git a/internal/statusui/tty_foreground_unix_test.go b/internal/statusui/tty_foreground_unix_test.go new file mode 100644 index 0000000..fdafbf5 --- /dev/null +++ b/internal/statusui/tty_foreground_unix_test.go @@ -0,0 +1,73 @@ +//go:build darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris || aix || zos + +package statusui + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestWaitForForegroundProcessGroupReturnsImmediatelyWhenForeground(t *testing.T) { + var calls int + err := waitForForegroundProcessGroup( + context.Background(), + func() int { return 42 }, + func() (int, error) { + calls++ + return 42, nil + }, + time.Millisecond, + ) + + require.NoError(t, err) + require.Equal(t, 1, calls) +} + +func TestWaitForForegroundProcessGroupPollsUntilForeground(t *testing.T) { + var calls int + err := waitForForegroundProcessGroup( + context.Background(), + func() int { return 42 }, + func() (int, error) { + calls++ + if calls < 3 { + return 7, nil + } + return 42, nil + }, + time.Millisecond, + ) + + require.NoError(t, err) + require.Equal(t, 3, calls) +} + +func TestWaitForForegroundProcessGroupExitsOnContextCancel(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := waitForForegroundProcessGroup( + ctx, + func() int { return 42 }, + func() (int, error) { return 7, nil }, + time.Millisecond, + ) + + require.ErrorIs(t, err, context.Canceled) +} + +func TestWaitForForegroundProcessGroupReturnsForegroundError(t *testing.T) { + want := errors.New("ioctl failed") + err := waitForForegroundProcessGroup( + context.Background(), + func() int { return 42 }, + func() (int, error) { return 0, want }, + time.Millisecond, + ) + + require.ErrorIs(t, err, want) +}