From 9d437504f8143445295cebed4e53b0b5e3e2f5bc Mon Sep 17 00:00:00 2001 From: "m.kindritskiy" Date: Wed, 15 Jul 2026 22:26:46 +0300 Subject: [PATCH 1/3] Fix hang in source <(lets completion -s zsh) under job control Every invocation queried the terminal background color (OSC 11) via lipgloss.HasDarkBackground(os.Stdin, stderr) when stderr is a TTY -- in logging.InitLogging and in progress bar setup. Inside process substitution under an interactive (job-control) shell, lets runs in a background process group; reading/reconfiguring the controlling terminal suspends it with SIGTTIN/SIGTTOU, the fifo never reaches EOF, and `source` blocks forever. Skip background-color detection when the process is not in the terminal's foreground process group (TIOCGPGRP vs getpgrp), defaulting to a dark background, lipgloss's own fallback. Windows has no terminal process groups, so it always queries. Regression test runs zsh with job control under a PTY (script) in bats; it failed with a timeout before the fix. --- docs/docs/changelog.md | 1 + go.mod | 2 +- internal/logging/formatter.go | 5 ++--- internal/theme/theme.go | 17 +++++++++++++++-- internal/util/foreground_unix.go | 21 +++++++++++++++++++++ internal/util/foreground_windows.go | 10 ++++++++++ tests/completion.bats | 10 ++++++++++ 7 files changed, 60 insertions(+), 6 deletions(-) create mode 100644 internal/util/foreground_unix.go create mode 100644 internal/util/foreground_windows.go diff --git a/docs/docs/changelog.md b/docs/docs/changelog.md index 562383f2..ba1ffc9a 100644 --- a/docs/docs/changelog.md +++ b/docs/docs/changelog.md @@ -5,6 +5,7 @@ title: Changelog ## [Unreleased](https://github.com/lets-cli/lets/releases/tag/v0.0.X) +* `[Fixed]` `source <(lets completion -s zsh)` no longer hangs: skip terminal background-color detection when lets runs outside the terminal's foreground process group (e.g. inside process substitution), where the query would suspend the process with SIGTTIN/SIGTTOU. * `[Fixed]` Add spacing between the `EXAMPLES` help title and its examples. * `[Fixed]` Align styled error output to the left while preserving the padded error badge. * `[Docs]` Convert design specs into ADRs. diff --git a/go.mod b/go.mod index 4aa5c593..1e552c65 100644 --- a/go.mod +++ b/go.mod @@ -69,7 +69,7 @@ require ( github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/lithammer/dedent v1.1.0 github.com/spf13/pflag v1.0.9 - golang.org/x/sys v0.42.0 // indirect + golang.org/x/sys v0.42.0 gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b // indirect gopkg.in/yaml.v3 v3.0.1 ) diff --git a/internal/logging/formatter.go b/internal/logging/formatter.go index 20508b7d..fc995437 100644 --- a/internal/logging/formatter.go +++ b/internal/logging/formatter.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "io" - "os" "strings" "unicode" @@ -12,6 +11,7 @@ import ( "github.com/charmbracelet/x/term" "github.com/fatih/color" "github.com/lets-cli/fang" + "github.com/lets-cli/lets/internal/theme" log "github.com/sirupsen/logrus" ) @@ -44,8 +44,7 @@ func newFormatter(errWriter io.Writer, cs fang.ColorSchemeFunc) *Formatter { return f } - isDark := lipgloss.HasDarkBackground(os.Stdin, file) - scheme := cs(lipgloss.LightDark(isDark)) + scheme := cs(lipgloss.LightDark(theme.IsDarkBackground(file))) w, _, err := term.GetSize(file.Fd()) if err != nil || w == 0 { diff --git a/internal/theme/theme.go b/internal/theme/theme.go index 599c9381..6cec9abd 100644 --- a/internal/theme/theme.go +++ b/internal/theme/theme.go @@ -8,6 +8,7 @@ import ( "github.com/charmbracelet/x/exp/charmtone" "github.com/charmbracelet/x/term" "github.com/lets-cli/fang" + "github.com/lets-cli/lets/internal/util" ) // Supported theme names. @@ -39,10 +40,22 @@ func ColorSchemeByName(name string) fang.ColorSchemeFunc { } } +// IsDarkBackground reports whether the terminal behind out has a dark background. +// Detection queries the terminal (writes OSC 11 to out, reads the reply from stdin), +// which suspends a background process group with SIGTTIN/SIGTTOU — so when the +// process is not in the foreground, skip the query and assume dark (lipgloss's +// own fallback). +func IsDarkBackground(out term.File) bool { + if !util.IsForegroundProcess(out.Fd()) { + return true + } + + return lipgloss.HasDarkBackground(os.Stdin, out) +} + // ProgressColorsByName resolves a theme name to fill and empty colors for download progress. func ProgressColorsByName(name string, out term.File) (color.Color, color.Color) { - isDark := lipgloss.HasDarkBackground(os.Stdin, out) - scheme := ColorSchemeByName(name)(lipgloss.LightDark(isDark)) + scheme := ColorSchemeByName(name)(lipgloss.LightDark(IsDarkBackground(out))) return ProgressColors(scheme) } diff --git a/internal/util/foreground_unix.go b/internal/util/foreground_unix.go new file mode 100644 index 00000000..d12e444c --- /dev/null +++ b/internal/util/foreground_unix.go @@ -0,0 +1,21 @@ +//go:build !windows + +package util + +import ( + "golang.org/x/sys/unix" +) + +// IsForegroundProcess reports whether this process belongs to the foreground +// process group of the terminal referred to by fd. Reading or reconfiguring a +// terminal from a background process group suspends the process with +// SIGTTIN/SIGTTOU — e.g. inside process substitution: +// `source <(lets completion -s zsh)`. +func IsForegroundProcess(fd uintptr) bool { + pgrp, err := unix.IoctlGetInt(int(fd), unix.TIOCGPGRP) + if err != nil { + return false + } + + return pgrp == unix.Getpgrp() +} diff --git a/internal/util/foreground_windows.go b/internal/util/foreground_windows.go new file mode 100644 index 00000000..95fc73cf --- /dev/null +++ b/internal/util/foreground_windows.go @@ -0,0 +1,10 @@ +//go:build windows + +package util + +// IsForegroundProcess reports whether this process may safely query the +// terminal referred to by fd. Windows has no terminal process groups, so +// querying never suspends the process. +func IsForegroundProcess(_ uintptr) bool { + return true +} diff --git a/tests/completion.bats b/tests/completion.bats index df5dfbf4..200c968c 100644 --- a/tests/completion.bats +++ b/tests/completion.bats @@ -51,3 +51,13 @@ setup() { assert_line --index 0 "--debug[Run with debug]" assert_line --index 1 "--env[Set env]" } + +# Regression: querying the terminal background color from a background process +# group suspends the process with SIGTTOU, so `source <(lets completion -s zsh)` +# in an interactive (job-control) shell hung forever. `script` allocates a PTY, +# `set -m` enables job control like an interactive shell. +@test "completion: source <(lets completion -s zsh) must not hang under job control" { + run timeout 10 script -qec "zsh -c 'set -m; source <(lets completion -s zsh); echo SOURCED_OK'" /dev/null + assert_success + assert_output --partial "SOURCED_OK" +} From 285b877e265b03c36ca1d75b3ec176e0c6b6b270 Mon Sep 17 00:00:00 2001 From: "m.kindritskiy" Date: Wed, 15 Jul 2026 22:26:56 +0300 Subject: [PATCH 2/3] Use %w for cached-read error in readMixin Applied by golangci-lint --fix (errorlint); lets errors.Is/As unwrap the read error alongside the download error. --- internal/config/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/config/config.go b/internal/config/config/config.go index 6d893d1b..90d0509b 100644 --- a/internal/config/config/config.go +++ b/internal/config/config/config.go @@ -259,7 +259,7 @@ func (c *Config) readMixin(mixin *Mixin) error { // ensureRemoteConfig's behavior for the top-level remote config. cachedData, readErr := rm.tryRead() if readErr != nil { - return fmt.Errorf("download failed (%w); also failed to read cached version: %v", downloadErr, readErr) + return fmt.Errorf("download failed (%w); also failed to read cached version: %w", downloadErr, readErr) } if cachedData != nil { From b64bdb1806d36b8d4de5771f5e8212d76c7ac36f Mon Sep 17 00:00:00 2001 From: "m.kindritskiy" Date: Thu, 16 Jul 2026 10:02:23 +0300 Subject: [PATCH 3/3] Drop Windows stub for foreground process check; Windows is not supported --- internal/util/{foreground_unix.go => foreground.go} | 2 -- internal/util/foreground_windows.go | 10 ---------- 2 files changed, 12 deletions(-) rename internal/util/{foreground_unix.go => foreground.go} (96%) delete mode 100644 internal/util/foreground_windows.go diff --git a/internal/util/foreground_unix.go b/internal/util/foreground.go similarity index 96% rename from internal/util/foreground_unix.go rename to internal/util/foreground.go index d12e444c..a8600f6b 100644 --- a/internal/util/foreground_unix.go +++ b/internal/util/foreground.go @@ -1,5 +1,3 @@ -//go:build !windows - package util import ( diff --git a/internal/util/foreground_windows.go b/internal/util/foreground_windows.go deleted file mode 100644 index 95fc73cf..00000000 --- a/internal/util/foreground_windows.go +++ /dev/null @@ -1,10 +0,0 @@ -//go:build windows - -package util - -// IsForegroundProcess reports whether this process may safely query the -// terminal referred to by fd. Windows has no terminal process groups, so -// querying never suspends the process. -func IsForegroundProcess(_ uintptr) bool { - return true -}