refactor(tui): resolve the color env overrides at the CLI entry point

AGENTS.md

@@ -122,10 +122,13 @@ allowed — use an exhaustive `enum` match.
 - Commands emit results through `&mut dyn mergify_core::output::Output`, not
   `println!`. Call `emit` / `emit_json_value` once.
 - **Color goes through `mergify_tui::theme`**, never hardcoded SGR escapes. The
-  theme honors `--color <auto|always|never>` (resolved once via
-  `set_color_choice`), then `NO_COLOR`, then `FORCE_COLOR`/`CLICOLOR_FORCE`, then
-  the TTY. Cursor-movement / erase escapes in the progress renderer are not color
-  and are fine.
+  precedence is `--color <auto|always|never>`, then `NO_COLOR`, then
+  `FORCE_COLOR`/`CLICOLOR_FORCE`, then the TTY, and it is resolved once at the
+  CLI entry point: `resolve_color_choice` in `main.rs` folds the three variables
+  into the `ColorChoice` it hands `set_color_choice`. `mergify-tui` itself reads
+  no environment variable, which is why a consumer crate's tests no longer touch
+  the environment just by rendering a themed line. Cursor-movement / erase
+  escapes in the progress renderer are not color and are fine.
 
 ```rust
 // GOOD

README.md

@@ -213,7 +213,8 @@ These are accepted on every command:
 | `GITHUB_REPOSITORY` | Default `owner/repo` when `--repository` is omitted. |
 | `MERGIFY_API_URL` | API base URL (default `https://api.mergify.com`). |
 | `RUST_LOG` | Fine-grained log filtering; overrides `--verbose`. |
-| `NO_COLOR` | Disable colored output. |
+| `NO_COLOR` | Disable colored output. Must be set to a non-empty value; exported-but-empty counts as unset. |
+| `FORCE_COLOR`, `CLICOLOR_FORCE` | Force colored output when it would otherwise be off. Same non-empty rule, and `NO_COLOR` wins over both. All three apply to `--color auto` only. |
 | `MERGIFY_INSTALL_DIR`, `MERGIFY_VERSION` | Install-script target directory / pinned version. |
 
 ## Exit codes

crates/mergify-cli/src/main.rs

@@ -688,8 +688,17 @@ fn detect_dispatch(argv: &[String]) -> Dispatch {
     };
     // Resolve the color preference once, before any command builds a
     // theme via `Theme::detect`.
-    mergify_tui::set_color_choice(parsed.color.into());
-    init_tracing(parsed.verbose, parsed.debug);
+    // Exported-but-empty is not set: a workflow writing
+    // `NO_COLOR: ${{ inputs.no_color }}` with no input exports the
+    // empty string, and <https://no-color.org> says that does not
+    // count. Same rule as every other variable this CLI reads.
+    let color = resolve_color_choice(
+        parsed.color,
+        non_empty("NO_COLOR"),
+        non_empty("FORCE_COLOR") || non_empty("CLICOLOR_FORCE"),
+    );
+    mergify_tui::set_color_choice(color);
+    init_tracing(parsed.verbose, parsed.debug, color);
     dispatch_from_parsed(parsed)
 }
 
@@ -698,7 +707,7 @@ fn detect_dispatch(argv: &[String]) -> Dispatch {
 /// trace), with `--debug` flooring at debug; an explicit `RUST_LOG`
 /// overrides both. Only our own crates are raised — third-party deps
 /// stay at `warn` so `-vv` doesn't drown in hyper/reqwest noise.
-fn init_tracing(verbose: u8, debug: bool) {
+fn init_tracing(verbose: u8, debug: bool, color: mergify_tui::ColorChoice) {
     use tracing_subscriber::EnvFilter;
 
     let level = match verbose {
@@ -717,7 +726,7 @@ fn init_tracing(verbose: u8, debug: bool) {
     let _ = tracing_subscriber::fmt()
         .with_env_filter(filter)
         .with_writer(std::io::stderr)
-        .with_ansi(std::io::stderr().is_terminal())
+        .with_ansi(ansi_on_stderr(color))
         .try_init();
 }
 
@@ -2800,13 +2809,56 @@ enum ColorArg {
     Never,
 }
 
-impl From<ColorArg> for mergify_tui::ColorChoice {
-    fn from(c: ColorArg) -> Self {
-        match c {
-            ColorArg::Auto => Self::Auto,
-            ColorArg::Always => Self::Always,
-            ColorArg::Never => Self::Never,
-        }
+/// `true` when `name` is exported to something other than the empty
+/// string, which is what <https://no-color.org> means by "present".
+///
+/// The rule itself lives in `mergify_core::env::var_non_empty`, which
+/// is where it is documented and tested; spelling it out a second time
+/// here is how the color variables would drift away from every other
+/// variable this CLI reads.
+fn non_empty(name: &str) -> bool {
+    mergify_core::env::var_non_empty(name).is_some()
+}
+
+/// Whether the log subscriber may emit ANSI on stderr.
+///
+/// The same decision `Theme::detect` makes for stdout, against
+/// stderr, because that is where the logs go. Without this
+/// `--color never` and `NO_COLOR` styled stdout and left the `-vv`
+/// stream on stderr full of escapes, which is the one place a user
+/// redirects to a file.
+fn ansi_on_stderr(color: mergify_tui::ColorChoice) -> bool {
+    match color {
+        mergify_tui::ColorChoice::Always => true,
+        mergify_tui::ColorChoice::Never => false,
+        mergify_tui::ColorChoice::Auto => std::io::stderr().is_terminal(),
+    }
+}
+
+/// Map `--color` to the choice `mergify-tui` records, folding in
+/// `NO_COLOR` / `FORCE_COLOR` / `CLICOLOR_FORCE`.
+///
+/// An explicit `--color always|never` wins over all three, which is
+/// why only the `Auto` arms consult them. `NO_COLOR` beats the two
+/// force flags, per <https://no-color.org>.
+///
+/// This lives here rather than in `mergify-tui` so that crate never
+/// reads the environment: it is the whole reason a consumer crate's
+/// test binary used to touch the environment just by rendering a
+/// themed line.
+fn resolve_color_choice(
+    arg: ColorArg,
+    no_color: bool,
+    force_color: bool,
+) -> mergify_tui::ColorChoice {
+    use mergify_tui::ColorChoice;
+
+    match arg {
+        ColorArg::Always => ColorChoice::Always,
+        ColorArg::Never => ColorChoice::Never,
+        ColorArg::Auto if no_color => ColorChoice::Never,
+        ColorArg::Auto if force_color => ColorChoice::Always,
+        ColorArg::Auto => ColorChoice::Auto,
     }
 }
 
@@ -4800,6 +4852,51 @@ mod tests {
         );
     }
 
+    #[test]
+    fn color_choice_folds_env_overrides_into_auto_only() {
+        use mergify_tui::ColorChoice;
+
+        // `--color always|never` is the user being explicit; no env
+        // var may override it.
+        assert_eq!(
+            resolve_color_choice(ColorArg::Always, true, false),
+            ColorChoice::Always
+        );
+        assert_eq!(
+            resolve_color_choice(ColorArg::Never, false, true),
+            ColorChoice::Never
+        );
+        // Auto: NO_COLOR wins over FORCE_COLOR / CLICOLOR_FORCE.
+        assert_eq!(
+            resolve_color_choice(ColorArg::Auto, true, true),
+            ColorChoice::Never
+        );
+        assert_eq!(
+            resolve_color_choice(ColorArg::Auto, false, true),
+            ColorChoice::Always
+        );
+        // Auto with neither set stays Auto, i.e. defers to the TTY.
+        assert_eq!(
+            resolve_color_choice(ColorArg::Auto, false, false),
+            ColorChoice::Auto
+        );
+    }
+
+    #[test]
+    fn stderr_ansi_follows_the_resolved_color_choice() {
+        use mergify_tui::ColorChoice;
+
+        // The log stream obeys the same decision as stdout: before
+        // this, `--color never` and `NO_COLOR` gave a plain stdout and
+        // a `-vv` stream still full of escapes.
+        assert!(ansi_on_stderr(ColorChoice::Always));
+        assert!(!ansi_on_stderr(ColorChoice::Never));
+        // `Auto` is deliberately not asserted: it is
+        // `stderr().is_terminal()`, which is whatever the harness was
+        // given, so an assertion here would either restate the branch
+        // or test the terminal the suite happens to run under.
+    }
+
     #[test]
     fn ci_scopes_parses_when_mergify_config_path_env_var_is_empty() {
         // Regression for monorepo#33423 / gha-mergify-ci:

crates/mergify-stack/src/progress.rs

@@ -109,8 +109,14 @@ pub struct Progress {
 impl Progress {
     /// Detect the output mode from stdout. Interactive (cursor
     /// redraw + spinner) only on a real terminal whose size we can
-    /// read. Color is delegated to [`Theme::detect`] (TTY-and-
-    /// `NO_COLOR`-aware, suppressed under tests).
+    /// read.
+    ///
+    /// Color follows that same interactivity, so it is narrower than
+    /// the recorded `--color` choice: a non-interactive `Progress` is
+    /// always plain, including under `--color always`. Only the
+    /// interactive branch consults [`Theme::detect`] (the recorded
+    /// choice, else the TTY; off outside the CLI entry point, so
+    /// tests are never colored). See the comment in the body.
     #[must_use]
     pub fn new() -> Self {
         let stdout = std::io::stdout();

crates/mergify-tui/src/lib.rs

@@ -11,8 +11,10 @@
 //! Modules:
 //!
 //! - [`theme`]: [`Theme`] struct that wraps `anstyle::Style` with
-//!   TTY-and-`NO_COLOR`-aware enable/disable, plus a named-color
-//!   palette. The same closure-based emit code paths produce
+//!   choice-and-TTY-aware enable/disable, plus a named-color
+//!   palette. This crate reads no environment variables; the
+//!   `NO_COLOR` family reaches it folded into the recorded
+//!   [`ColorChoice`]. The same closure-based emit code paths produce
 //!   styled output on a TTY and plain text everywhere else with no
 //!   conditional branching at every write.
 //! - [`glyph`]: [`StyledGlyph`] — pairs a Unicode icon with the

crates/mergify-tui/src/select.rs

@@ -67,9 +67,9 @@ pub fn fuzzy_select(prompt: &str, items: &[String], default: usize) -> io::Resul
     install_sigint_handler();
     let colorful = ColorfulTheme::default();
     let simple = SimpleTheme;
-    // Same color policy as every other renderer (`--color` override
-    // > `NO_COLOR` > `FORCE_COLOR`/`CLICOLOR_FORCE` > TTY), reused
-    // from `theme.rs` rather than re-derived.
+    // Same color policy as every other renderer — the choice the
+    // entry point recorded, else the TTY — reused from `theme.rs`
+    // rather than re-derived.
     let theme: &dyn Theme = if colors_enabled() { &colorful } else { &simple };
     PICKER_ACTIVE.store(true, Ordering::SeqCst);
     let result = FuzzySelect::with_theme(theme)

crates/mergify-tui/src/theme.rs

@@ -1,4 +1,8 @@
-//! ANSI styling wrapped with TTY/`NO_COLOR` detection.
+//! ANSI styling, enabled by the recorded [`ColorChoice`] and the TTY.
+//!
+//! This crate reads no environment variable of its own: the
+//! `NO_COLOR` family arrives folded into that choice. See
+//! [`set_color_choice`].
 //!
 //! The intent is to write normal `format!` / `write!` code paths
 //! that emit styled output on an interactive terminal and produce
@@ -15,8 +19,9 @@ use std::sync::OnceLock;
 use anstyle::AnsiColor;
 use anstyle::Style;
 
-/// The user's `--color` preference. `Auto` defers to env vars and TTY
-/// detection; `Always`/`Never` override both.
+/// The user's resolved color preference. `Auto` defers to TTY
+/// detection; `Always`/`Never` override it. The `NO_COLOR` family is
+/// folded in by the caller of [`set_color_choice`], not here.
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
 pub enum ColorChoice {
     #[default]
@@ -27,14 +32,23 @@ pub enum ColorChoice {
 
 static COLOR_CHOICE: OnceLock<ColorChoice> = OnceLock::new();
 
-/// Record the process-wide color preference (from `--color`), once,
-/// at startup before any [`Theme::detect`]. Subsequent calls are
-/// ignored, so a stray second call can't flip colors mid-run.
+/// Record the process-wide color preference, once, at startup before
+/// any [`Theme::detect`]. Subsequent calls are ignored, so a stray
+/// second call can't flip colors mid-run.
 ///
 /// Calling this is also what makes color possible at all: until it
 /// does, [`Theme::detect`] reports disabled. Only the CLI entry point
 /// calls it, so a process that never went through `main` — a test
 /// harness, a doctest, an embedder — is never colored.
+///
+/// The caller passes the *resolved* choice: `--color` with the
+/// `NO_COLOR` / `FORCE_COLOR` / `CLICOLOR_FORCE` overrides already
+/// folded in (`mergify-cli`'s `resolve_color_choice`). This crate
+/// deliberately never reads the environment itself. Two reasons:
+/// it keeps `mergify-tui` dependency-light and free of the
+/// workspace's env funnel, and it means no consumer crate's test
+/// binary reads the environment on a worker thread just by
+/// rendering a themed line.
 pub fn set_color_choice(choice: ColorChoice) {
     let _ = COLOR_CHOICE.set(choice);
 }
@@ -44,8 +58,8 @@ pub fn set_color_choice(choice: ColorChoice) {
 /// colors are enabled) or `Style::new()` (when disabled — emits
 /// nothing); `reset` mirrors that with `"\x1b[0m"` vs `""`.
 ///
-/// Construct via [`Theme::detect`] for the production policy
-/// (TTY-only, `NO_COLOR`-aware, off outside the CLI entry point).
+/// Construct via [`Theme::detect`] for the production policy (the
+/// recorded choice, else the TTY; off outside the CLI entry point).
 /// Tests that need to assert on styled output explicitly can pass
 /// `enabled = true` to [`Theme::new`].
 pub struct Theme {
@@ -77,11 +91,12 @@ impl Theme {
     /// 1. No [`set_color_choice`] yet ⇒ disabled. Only the CLI entry
     ///    point records one, so this is a test harness or an embedder:
     ///    it asserts on in-memory buffers and must not take a
-    ///    dependency on the developer's terminal or environment.
+    ///    dependency on the developer's terminal.
     /// 2. `--color always`/`never` (via [`set_color_choice`]) wins.
-    /// 3. Otherwise (`auto`): `NO_COLOR` forces off; `FORCE_COLOR` /
-    ///    `CLICOLOR_FORCE` force on (e.g. through a pager or CI
-    ///    viewer); else `stdout` must be a terminal.
+    /// 3. Otherwise (`auto`): `stdout` must be a terminal.
+    ///
+    /// `NO_COLOR` / `FORCE_COLOR` / `CLICOLOR_FORCE` are **not** read
+    /// here — see [`set_color_choice`].
     #[must_use]
     pub fn detect() -> Self {
         Self::new(colors_enabled())
@@ -121,28 +136,15 @@ impl Theme {
 }
 
 /// Pure color decision, factored out of [`colors_enabled`] so the
-/// precedence is unit-testable without touching global state, env, or
-/// the real TTY.
-fn resolve_enabled(
-    choice: Option<ColorChoice>,
-    no_color: bool,
-    force_color: bool,
-    is_tty: bool,
-) -> bool {
+/// precedence is unit-testable without touching global state or the
+/// real TTY.
+fn resolve_enabled(choice: Option<ColorChoice>, is_tty: bool) -> bool {
     match choice {
         Some(ColorChoice::Always) => true,
         // `None` is nobody having recorded a preference, which means
         // nobody is watching a terminal — see [`set_color_choice`].
         None | Some(ColorChoice::Never) => false,
-        Some(ColorChoice::Auto) => {
-            if no_color {
-                false
-            } else if force_color {
-                true
-            } else {
-                is_tty
-            }
-        }
+        Some(ColorChoice::Auto) => is_tty,
     }
 }
 
@@ -153,16 +155,7 @@ pub(crate) fn colors_enabled() -> bool {
     // *consumer* crate's tests were reading the developer's
     // environment after all. `FORCE_COLOR=1 cargo test` failed on the
     // escape sequences that leaked into asserted output.
-    let choice = COLOR_CHOICE.get().copied();
-    let no_color = std::env::var_os("NO_COLOR").is_some();
-    let force_color =
-        std::env::var_os("FORCE_COLOR").is_some() || std::env::var_os("CLICOLOR_FORCE").is_some();
-    resolve_enabled(
-        choice,
-        no_color,
-        force_color,
-        std::io::stdout().is_terminal(),
-    )
+    resolve_enabled(COLOR_CHOICE.get().copied(), std::io::stdout().is_terminal())
 }
 
 #[cfg(test)]
@@ -172,34 +165,17 @@ mod tests {
     #[test]
     fn color_precedence() {
         // No recorded choice: not the CLI, so nothing is colored —
-        // not even with a TTY and FORCE_COLOR both saying yes. This
-        // is what keeps a consumer crate's tests reproducible.
-        assert!(!resolve_enabled(None, false, true, true));
-        // Explicit choice overrides everything.
-        assert!(resolve_enabled(
-            Some(ColorChoice::Always),
-            true,
-            false,
-            false
-        ));
-        assert!(!resolve_enabled(
-            Some(ColorChoice::Never),
-            false,
-            true,
-            true
-        ));
-        // Auto: NO_COLOR wins over FORCE_COLOR and TTY.
-        assert!(!resolve_enabled(Some(ColorChoice::Auto), true, true, true));
-        // Auto: FORCE_COLOR turns it on without a TTY.
-        assert!(resolve_enabled(Some(ColorChoice::Auto), false, true, false));
-        // Auto: otherwise follow the TTY.
-        assert!(resolve_enabled(Some(ColorChoice::Auto), false, false, true));
-        assert!(!resolve_enabled(
-            Some(ColorChoice::Auto),
-            false,
-            false,
-            false
-        ));
+        // not even on a TTY. This is what keeps a consumer crate's
+        // tests reproducible.
+        assert!(!resolve_enabled(None, true));
+        // Explicit choice overrides the TTY in both directions.
+        assert!(resolve_enabled(Some(ColorChoice::Always), false));
+        assert!(!resolve_enabled(Some(ColorChoice::Never), true));
+        // Auto follows the TTY. The env overrides that used to be
+        // decided here now reach us already folded into the choice —
+        // see `set_color_choice`.
+        assert!(resolve_enabled(Some(ColorChoice::Auto), true));
+        assert!(!resolve_enabled(Some(ColorChoice::Auto), false));
     }
 
     #[test]