refactor: derive StderrIsTerminal as a precomputed field

Mirror the existing IsTerminal field instead of a live method, computed once
in NewIOStreams. This drops the stderrIsTerminal package-level test seam (the
gate now reads the field directly); tests inject the field like bind_test.go
does for IsTerminal. The warnIfProxied seam stays, since the real function is
guarded by an internal sync.Once and cannot be counted across subtests.

Author: albertnusouo

internal/cmdutil/factory_default.go

@@ -100,19 +100,17 @@ func safeRedirectPolicy(req *http.Request, via []*http.Request) error {
 	return nil
 }
 
-// Seams for unit-testing the proxy-warning gate. Production wires them to the
-// real implementations; tests substitute a predicate and a spy to verify the
-// warning fires only on an interactive stderr (see factory_proxy_warn_test.go).
-// StderrIsTerminal is a concrete method (real TTY only) and WarnIfProxied is a
-// package function, so neither is otherwise injectable.
-var (
-	warnIfProxied    = transport.WarnIfProxied
-	stderrIsTerminal = func(s *IOStreams) bool { return s.StderrIsTerminal() }
-)
+// warnIfProxied is a test seam for the proxy-warning gate. Production wires it
+// to transport.WarnIfProxied; tests swap in a spy to count invocations. It is
+// needed because the real function is guarded by an internal sync.Once, so
+// calling it directly would only fire on the first test (see
+// factory_proxy_warn_test.go). The terminal check is the IOStreams
+// .StderrIsTerminal field, which tests set directly.
+var warnIfProxied = transport.WarnIfProxied
 
 func cachedHttpClientFunc(f *Factory) func() (*http.Client, error) {
 	return sync.OnceValues(func() (*http.Client, error) {
-		if stderrIsTerminal(f.IOStreams) {
+		if f.IOStreams.StderrIsTerminal {
 			warnIfProxied(f.IOStreams.ErrOut)
 		}
 
@@ -141,7 +139,7 @@ func cachedLarkClientFunc(f *Factory) func() (*lark.Client, error) {
 			lark.WithLogLevel(larkcore.LogLevelError),
 			lark.WithHeaders(BaseSecurityHeaders()),
 		}
-		if stderrIsTerminal(f.IOStreams) {
+		if f.IOStreams.StderrIsTerminal {
 			warnIfProxied(f.IOStreams.ErrOut)
 		}
 		opts = append(opts, lark.WithHttpClient(&http.Client{

internal/cmdutil/factory_proxy_warn_test.go

@@ -11,19 +11,16 @@ import (
 	"github.com/larksuite/cli/internal/envvars"
 )
 
-// installProxyWarnSpy swaps the package-level proxy-warning seams for one test:
-// stderrIsTerminal is forced to `terminal`, and warnIfProxied is replaced with a
-// counter. The real implementations are restored on cleanup. Returns a pointer
-// to the call count so the caller can assert how many times the warning fired.
-func installProxyWarnSpy(t *testing.T, terminal bool) *int {
+// installProxyWarnSpy replaces warnIfProxied with a counter for one test and
+// restores it on cleanup. Returns a pointer to the call count so the caller can
+// assert how many times the warning fired. The terminal state is controlled via
+// the IOStreams.StderrIsTerminal field, not a seam.
+func installProxyWarnSpy(t *testing.T) *int {
 	t.Helper()
-	prevWarn, prevTTY := warnIfProxied, stderrIsTerminal
-	t.Cleanup(func() {
-		warnIfProxied, stderrIsTerminal = prevWarn, prevTTY
-	})
+	prevWarn := warnIfProxied
+	t.Cleanup(func() { warnIfProxied = prevWarn })
 	calls := 0
 	warnIfProxied = func(io.Writer) { calls++ }
-	stderrIsTerminal = func(*IOStreams) bool { return terminal }
 	return &calls
 }
 
@@ -41,9 +38,11 @@ var proxyWarnGateCases = []struct {
 func TestCachedHttpClientFunc_ProxyWarnGate(t *testing.T) {
 	for _, tc := range proxyWarnGateCases {
 		t.Run(tc.name, func(t *testing.T) {
-			calls := installProxyWarnSpy(t, tc.terminal)
+			calls := installProxyWarnSpy(t)
 
-			fn := cachedHttpClientFunc(&Factory{IOStreams: &IOStreams{ErrOut: io.Discard}})
+			fn := cachedHttpClientFunc(&Factory{IOStreams: &IOStreams{
+				ErrOut: io.Discard, StderrIsTerminal: tc.terminal,
+			}})
 			if _, err := fn(); err != nil {
 				t.Fatalf("http client init: %v", err)
 			}
@@ -69,9 +68,11 @@ func TestCachedLarkClientFunc_ProxyWarnGate(t *testing.T) {
 			t.Setenv(envvars.CliTenantAccessToken, "")
 			t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
 
-			calls := installProxyWarnSpy(t, tc.terminal)
+			calls := installProxyWarnSpy(t)
 
-			f := NewDefault(&IOStreams{ErrOut: io.Discard}, InvocationContext{})
+			// normalizeStreams copies the struct (out := *s), so the
+			// StderrIsTerminal field survives into f.IOStreams.
+			f := NewDefault(&IOStreams{ErrOut: io.Discard, StderrIsTerminal: tc.terminal}, InvocationContext{})
 			if _, err := cachedLarkClientFunc(f)(); err != nil {
 				t.Fatalf("lark client init: %v", err)
 			}

internal/cmdutil/iostreams.go

@@ -18,26 +18,28 @@ type IOStreams struct {
 	Out        io.Writer
 	ErrOut     io.Writer
 	IsTerminal bool
+	// StderrIsTerminal reports whether ErrOut is an interactive terminal.
+	// Advisory warnings written to stderr (e.g. the proxy notice) gate on this
+	// so they stay out of non-interactive output (pipes, CI, agent runs).
+	// Computed once in NewIOStreams, mirroring IsTerminal; tests assign it
+	// directly like cmd/config/bind_test.go does for IsTerminal.
+	StderrIsTerminal bool
 }
 
 // NewIOStreams builds an IOStreams from arbitrary readers/writers.
-// IsTerminal is derived from in's underlying *os.File, if any; non-file
-// readers (bytes.Buffer, strings.Reader, …) yield IsTerminal=false.
+// IsTerminal / StderrIsTerminal are derived from in's / errOut's underlying
+// *os.File, if any; non-file streams (bytes.Buffer, strings.Reader, …) yield
+// false.
 func NewIOStreams(in io.Reader, out, errOut io.Writer) *IOStreams {
 	isTerminal := false
 	if f, ok := in.(*os.File); ok {
 		isTerminal = term.IsTerminal(int(f.Fd()))
 	}
-	return &IOStreams{In: in, Out: out, ErrOut: errOut, IsTerminal: isTerminal}
-}
-
-// StderrIsTerminal reports whether ErrOut is an interactive terminal. Advisory
-// warnings written to stderr (e.g. the proxy notice) gate on this so they stay
-// out of non-interactive output (pipes, CI, agent runs). Buffers (tests) and
-// redirects are not *os.File terminals, so they yield false.
-func (s *IOStreams) StderrIsTerminal() bool {
-	f, ok := s.ErrOut.(*os.File)
-	return ok && term.IsTerminal(int(f.Fd()))
+	stderrIsTerminal := false
+	if f, ok := errOut.(*os.File); ok {
+		stderrIsTerminal = term.IsTerminal(int(f.Fd()))
+	}
+	return &IOStreams{In: in, Out: out, ErrOut: errOut, IsTerminal: isTerminal, StderrIsTerminal: stderrIsTerminal}
 }
 
 // SystemIO creates an IOStreams wired to the process's standard file descriptors.