From 0b99421dff1ec308ba8be8ffef5e9491cb7e0491 Mon Sep 17 00:00:00 2001 From: Grey Newell Date: Thu, 30 Apr 2026 10:30:27 -0400 Subject: [PATCH 1/3] test: failing test for MinTTY /dev/tty fallback in stdinIsTerminal (#154) Co-Authored-By: Claude Sonnet 4.6 --- cmd/root.go | 8 ++++++++ cmd/root_test.go | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/cmd/root.go b/cmd/root.go index 374d1c6..0cca7f7 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -14,6 +14,14 @@ import ( "github.com/supermodeltools/cli/internal/shards" ) +// openDevTty opens /dev/tty (the process's controlling terminal). It is a +// package-level var so tests can replace it with a mock. On Windows, +// os.Open("/dev/tty") will fail, which is expected — Windows Console API +// (used by term.IsTerminal above) handles Windows Terminal/PowerShell. +var openDevTty = func() (*os.File, error) { + return nil, fmt.Errorf("not implemented") +} + // stdinIsTerminal reports whether stdin is connected to an interactive // terminal. Pulled into a var so tests can stub it. var stdinIsTerminal = func() bool { diff --git a/cmd/root_test.go b/cmd/root_test.go index bceac9a..32fec50 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,6 +1,38 @@ package cmd -import "testing" +import ( + "os" + "testing" +) + +// TestStdinIsTerminalUsesDevTtyFallback verifies that stdinIsTerminal falls +// back to opening /dev/tty when term.IsTerminal returns false (e.g. in CI or +// MinTTY/Git Bash on Windows). The fix is tracked in issue #154. +func TestStdinIsTerminalUsesDevTtyFallback(t *testing.T) { + // Save and restore the real openDevTty hook. + orig := openDevTty + t.Cleanup(func() { openDevTty = orig }) + + called := false + openDevTty = func() (*os.File, error) { + called = true + // Return a real, harmless file so the caller can call f.Close(). + return os.Open(os.DevNull) + } + + // In CI stdin is not a TTY, so term.IsTerminal returns false. + // The fixed stdinIsTerminal must call openDevTty as a fallback and + // return true because our mock succeeds. + got := stdinIsTerminal() + + if !called { + t.Error("stdinIsTerminal did not call openDevTty — /dev/tty fallback is missing (fix #154)") + } + // If openDevTty was called and succeeded, the result must be true. + if called && !got { + t.Error("openDevTty returned a file but stdinIsTerminal returned false — fix #154") + } +} func TestPickRootAction(t *testing.T) { cases := []struct { From 84ef73520a82fac26cac81968365bdd3fffab67b Mon Sep 17 00:00:00 2001 From: Grey Newell Date: Thu, 30 Apr 2026 10:36:51 -0400 Subject: [PATCH 2/3] fix: add /dev/tty fallback so MinTTY/Git Bash is detected as interactive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit term.IsTerminal(syscall.Stdin) returns false in MinTTY even at an interactive terminal. Fall back to opening /dev/tty (POSIX TTY device) — if it opens, stdin is interactive. Closes #154. Co-Authored-By: Claude Sonnet 4.6 --- cmd/root.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 0cca7f7..72ce0f8 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -19,13 +19,25 @@ import ( // os.Open("/dev/tty") will fail, which is expected — Windows Console API // (used by term.IsTerminal above) handles Windows Terminal/PowerShell. var openDevTty = func() (*os.File, error) { - return nil, fmt.Errorf("not implemented") + return os.Open("/dev/tty") } // stdinIsTerminal reports whether stdin is connected to an interactive // terminal. Pulled into a var so tests can stub it. +// +// MinTTY (Windows Git Bash) reports a non-terminal stdin fd even though the +// user is at an interactive prompt, so we fall back to opening /dev/tty — the +// POSIX controlling-terminal device. If it opens, stdin is interactive. +// See issue #154. var stdinIsTerminal = func() bool { - return term.IsTerminal(int(syscall.Stdin)) //nolint:unconvert // syscall.Stdin is uintptr on Windows + if term.IsTerminal(int(syscall.Stdin)) { //nolint:unconvert // syscall.Stdin is uintptr on Windows + return true + } + if f, err := openDevTty(); err == nil { + f.Close() + return true + } + return false } // rootAction enumerates the three branches the bare `supermodel` command From c66477c312cfd403ad7aed4d44e443d276a71959 Mon Sep 17 00:00:00 2001 From: Grey Newell Date: Thu, 30 Apr 2026 10:43:23 -0400 Subject: [PATCH 3/3] fix: make /dev/tty fallback test deterministic; extract termIsTerminal hook Extract term.IsTerminal into a stubable `termIsTerminal` var so the /dev/tty fallback test is deterministic regardless of whether the test process itself has an interactive stdin. Addresses CodeRabbit review. Co-Authored-By: Claude Sonnet 4.6 --- cmd/root.go | 8 +++++++- cmd/root_test.go | 11 ++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 72ce0f8..ee1b4cc 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -14,6 +14,12 @@ import ( "github.com/supermodeltools/cli/internal/shards" ) +// termIsTerminal wraps term.IsTerminal so tests can stub the syscall check +// independently of the /dev/tty fallback. +var termIsTerminal = func() bool { + return term.IsTerminal(int(syscall.Stdin)) //nolint:unconvert // syscall.Stdin is uintptr on Windows +} + // openDevTty opens /dev/tty (the process's controlling terminal). It is a // package-level var so tests can replace it with a mock. On Windows, // os.Open("/dev/tty") will fail, which is expected — Windows Console API @@ -30,7 +36,7 @@ var openDevTty = func() (*os.File, error) { // POSIX controlling-terminal device. If it opens, stdin is interactive. // See issue #154. var stdinIsTerminal = func() bool { - if term.IsTerminal(int(syscall.Stdin)) { //nolint:unconvert // syscall.Stdin is uintptr on Windows + if termIsTerminal() { return true } if f, err := openDevTty(); err == nil { diff --git a/cmd/root_test.go b/cmd/root_test.go index 32fec50..3a20d09 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -9,6 +9,12 @@ import ( // back to opening /dev/tty when term.IsTerminal returns false (e.g. in CI or // MinTTY/Git Bash on Windows). The fix is tracked in issue #154. func TestStdinIsTerminalUsesDevTtyFallback(t *testing.T) { + // Force termIsTerminal to return false so the test is deterministic + // regardless of whether the test process itself has an interactive stdin. + origTermIsTerminal := termIsTerminal + t.Cleanup(func() { termIsTerminal = origTermIsTerminal }) + termIsTerminal = func() bool { return false } + // Save and restore the real openDevTty hook. orig := openDevTty t.Cleanup(func() { openDevTty = orig }) @@ -20,9 +26,8 @@ func TestStdinIsTerminalUsesDevTtyFallback(t *testing.T) { return os.Open(os.DevNull) } - // In CI stdin is not a TTY, so term.IsTerminal returns false. - // The fixed stdinIsTerminal must call openDevTty as a fallback and - // return true because our mock succeeds. + // With termIsTerminal stubbed to false, stdinIsTerminal must call + // openDevTty as a fallback and return true because our mock succeeds. got := stdinIsTerminal() if !called {