run: signal every concurrent child on Ctrl-C, and make --color take effect

crates/nub-cli/src/pm_engine/mod.rs

@@ -1511,14 +1511,12 @@ fn emit_scope_warnings(role: config_scope::Role, ignored: &[config_scope::Ignore
     }
 }
 
-/// Whether the scoping warning should be dim-styled: stderr is a terminal
-/// (or `FORCE_COLOR` is set) AND `NO_COLOR` is unset.
+/// Whether the scoping warning should be dim-styled. Delegates to the CLI's single
+/// color predicate so an explicit `--color`/`--no-color` governs the engine's
+/// warnings too, and so `FORCE_COLOR=0` reads as OFF rather than as merely set.
 pub(crate) fn scope_warning_uses_dim() -> bool {
     use std::io::IsTerminal;
-    if std::env::var_os("NO_COLOR").is_some() {
-        return false;
-    }
-    std::io::stderr().is_terminal() || std::env::var_os("FORCE_COLOR").is_some()
+    crate::cli::color_enabled(std::io::stderr().is_terminal())
 }
 
 /// The pnpm version the role-first UA advertises for a pnpm-role project with

crates/nub-cli/tests/integration.rs

@@ -4697,6 +4697,279 @@ fn reporter_hide_prefix_strips_per_line_prefix() {
     }
 }
 
+/// `--reporter-hide-prefix` hides the CHILD's per-line label, not Nub's own framing.
+/// pnpm 10.15.1 still emits `<dir> <script>: Done` with the flag set, and a bare
+/// `Done` names no package — a workspace run ended in a stack of identical,
+/// unattributable lines (#685).
+#[test]
+fn reporter_hide_prefix_keeps_the_label_on_nubs_status_line() {
+    let fixture = fixtures_dir().join("monorepo-deps");
+    let output = Command::new(nub_binary())
+        .args(["run", "-r", "--stream", "--reporter-hide-prefix", "build"])
+        .current_dir(&fixture)
+        .output()
+        .expect("spawn nub");
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.lines().any(|l| l.contains("build: Done")),
+        "the per-member status line must keep its `<dir> <script>: ` label: {stderr}"
+    );
+    assert!(
+        !stderr.lines().any(|l| l.trim() == "Done"),
+        "no unattributable bare `Done` may remain: {stderr}"
+    );
+}
+
+/// `--color=always` has to reach the CHILD, not just Nub's own framing — that is the
+/// whole point of asking for it. A workspace run pipes child stdio to prefix each
+/// line, so the child sees no TTY and disables its own color; `FORCE_COLOR` is the
+/// override tools honor. Measured parity: pnpm 10.15.1 exports `FORCE_COLOR=1` here.
+/// Before #685 the flag parsed into a field nothing ever read.
+#[test]
+fn color_always_exports_force_color_to_the_script() {
+    let fixture = fixtures_dir().join("monorepo-deps");
+    let output = Command::new(nub_binary())
+        .args(["--color=always", "run", "-r", "color-probe"])
+        .current_dir(&fixture)
+        .env_remove("FORCE_COLOR")
+        .env_remove("NO_COLOR")
+        .output()
+        .expect("spawn nub");
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    assert!(
+        stdout.contains("FC=1"),
+        "--color=always must export FORCE_COLOR=1 to the script: {stdout}"
+    );
+}
+
+/// The opt-OUT half, and pnpm's exact spelling: `--no-color` exports `FORCE_COLOR=0`
+/// (10.15.1 measured), not an unset var — a child that inherited an ambient
+/// `FORCE_COLOR=1` must still be told to stop.
+#[test]
+fn no_color_exports_force_color_zero_to_the_script() {
+    let fixture = fixtures_dir().join("monorepo-deps");
+    let output = Command::new(nub_binary())
+        .args(["--no-color", "run", "-r", "color-probe"])
+        .current_dir(&fixture)
+        .env("FORCE_COLOR", "1")
+        .output()
+        .expect("spawn nub");
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    assert!(
+        stdout.contains("FC=0"),
+        "--no-color must export FORCE_COLOR=0, overriding the ambient value: {stdout}"
+    );
+}
+
+/// Default `auto` forces nothing either way, so a plain run stays byte-identical to
+/// pnpm's (which also leaves the child's `FORCE_COLOR` unset). This is the control
+/// that keeps the two tests above from passing for the wrong reason.
+#[test]
+fn color_auto_leaves_the_scripts_force_color_untouched() {
+    let fixture = fixtures_dir().join("monorepo-deps");
+    let output = Command::new(nub_binary())
+        .args(["run", "-r", "color-probe"])
+        .current_dir(&fixture)
+        .env_remove("FORCE_COLOR")
+        .env_remove("NO_COLOR")
+        .output()
+        .expect("spawn nub");
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    assert!(
+        stdout.contains("FC=unset"),
+        "a default run must not force color on the child: {stdout}"
+    );
+}
+
+/// `nub --help` advertises `NO_COLOR`, but the stream-prefix path never consulted it,
+/// so a colored prefix survived the opt-out. `FORCE_COLOR=1` stands in for a TTY here
+/// (the test harness has none), which is also what makes this a real control: the
+/// first case proves the prefix CAN be colored, so the second failing to color is
+/// attributable to `NO_COLOR` rather than to there being no color anywhere.
+#[test]
+fn no_color_env_suppresses_nubs_own_prefix_color() {
+    let fixture = fixtures_dir().join("monorepo-deps");
+    let colored = Command::new(nub_binary())
+        .args(["run", "-r", "--stream", "build"])
+        .current_dir(&fixture)
+        .env("FORCE_COLOR", "1")
+        .env_remove("NO_COLOR")
+        .output()
+        .expect("spawn nub");
+    assert!(
+        String::from_utf8_lossy(&colored.stderr).contains('\x1b'),
+        "positive control: FORCE_COLOR=1 must colorize the prefix"
+    );
+
+    let plain = Command::new(nub_binary())
+        .args(["run", "-r", "--stream", "build"])
+        .current_dir(&fixture)
+        .env("FORCE_COLOR", "1")
+        .env("NO_COLOR", "1")
+        .output()
+        .expect("spawn nub");
+    assert!(
+        !String::from_utf8_lossy(&plain.stderr).contains('\x1b'),
+        "NO_COLOR must win over FORCE_COLOR: {}",
+        String::from_utf8_lossy(&plain.stderr)
+    );
+}
+
+/// `FORCE_COLOR=0` means OFF. The old predicate tested only for the variable's
+/// PRESENCE, so the conventional disable spelling switched color on — and that now
+/// matters directly, because `--no-color` exports exactly `FORCE_COLOR=0` to
+/// children, so a nested `nub` would have colorized on its parent's opt-out.
+#[test]
+fn force_color_zero_disables_nubs_own_prefix_color() {
+    let fixture = fixtures_dir().join("monorepo-deps");
+    let output = Command::new(nub_binary())
+        .args(["run", "-r", "--stream", "build"])
+        .current_dir(&fixture)
+        .env("FORCE_COLOR", "0")
+        .env_remove("NO_COLOR")
+        .output()
+        .expect("spawn nub");
+    assert!(
+        !String::from_utf8_lossy(&output.stderr).contains('\x1b'),
+        "FORCE_COLOR=0 must disable color: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+}
+
+/// pnpm accepts `--no-color` both before and after the verb; Nub's pre-subcommand
+/// scan only sees the pre-verb position, so the post-verb spelling used to exit 2
+/// with `unexpected argument`. Both must work, and both must actually disable color.
+#[test]
+fn no_color_is_accepted_before_and_after_the_verb() {
+    let fixture = fixtures_dir().join("monorepo-deps");
+    for args in [
+        vec!["--no-color", "run", "-r", "--stream", "build"],
+        vec!["run", "-r", "--stream", "--no-color", "build"],
+    ] {
+        let output = Command::new(nub_binary())
+            .args(&args)
+            .current_dir(&fixture)
+            // FORCE_COLOR=1 makes the run colored to begin with, so the assertion
+            // below can only pass because `--no-color` turned it off. Clearing
+            // NO_COLOR matters for the same reason: an ambient one would satisfy it
+            // without the flag doing anything.
+            .env("FORCE_COLOR", "1")
+            .env_remove("NO_COLOR")
+            .output()
+            .expect("spawn nub");
+        assert!(
+            output.status.success(),
+            "`nub {}` must parse: {}",
+            args.join(" "),
+            String::from_utf8_lossy(&output.stderr)
+        );
+        assert!(
+            !String::from_utf8_lossy(&output.stderr).contains('\x1b'),
+            "`nub {}` must disable color: {}",
+            args.join(" "),
+            String::from_utf8_lossy(&output.stderr)
+        );
+    }
+}
+
+/// An EMPTY `FORCE_COLOR` is Node's shortest spelling of ON — `getColorDepth` lists
+/// `case ''` alongside `'1'` and `'true'` (lib/internal/tty.js). Reading it as OFF
+/// split Nub from the very children it hands the variable to. `'false'` is the other
+/// half of the same table: Node falls through to monochrome, so Nub must too.
+#[test]
+fn force_color_values_follow_nodes_table() {
+    let fixture = fixtures_dir().join("monorepo-deps");
+    for (value, want_color) in [
+        ("", true),
+        ("1", true),
+        ("true", true),
+        ("0", false),
+        ("false", false),
+    ] {
+        let output = Command::new(nub_binary())
+            .args(["run", "-r", "--stream", "build"])
+            .current_dir(&fixture)
+            .env("FORCE_COLOR", value)
+            .env_remove("NO_COLOR")
+            .output()
+            .expect("spawn nub");
+        let got = String::from_utf8_lossy(&output.stderr).contains('\x1b');
+        assert_eq!(
+            got, want_color,
+            "FORCE_COLOR={value:?} should give color={want_color}, matching Node's getColorDepth"
+        );
+    }
+}
+
+/// Nub resolves `NO_COLOR` over `FORCE_COLOR`; Node resolves them the other way. With
+/// both set, that split a run against itself — Nub's label plain while the child
+/// colorized, because the child applied Node's order to the same two variables. In
+/// `auto` Nub now exports `FORCE_COLOR=0` for exactly that contradictory pair, so
+/// both halves of a line agree. The `FC=` probe reads what the child actually got.
+#[test]
+fn contradictory_color_env_does_not_split_nub_from_its_child() {
+    let fixture = fixtures_dir().join("monorepo-deps");
+    let output = Command::new(nub_binary())
+        .args(["run", "-r", "--stream", "color-probe"])
+        .current_dir(&fixture)
+        .env("FORCE_COLOR", "1")
+        .env("NO_COLOR", "1")
+        .output()
+        .expect("spawn nub");
+    assert!(
+        !String::from_utf8_lossy(&output.stderr).contains('\x1b'),
+        "NO_COLOR keeps Nub's own prefix plain: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+    assert!(
+        String::from_utf8_lossy(&output.stdout).contains("FC=0"),
+        "the child must be pinned to Nub's answer, not left to resolve the pair \
+         with Node's opposite precedence: {}",
+        String::from_utf8_lossy(&output.stdout)
+    );
+}
+
+/// The scope control for the case above. Nub only wraps a child's output on the
+/// prefixed path; a plain `nub run <script>` inherits stdio and frames nothing, so
+/// there is no contradiction to repair and the script must receive the environment
+/// it was given. pnpm, npm and a bare shell all pass `FORCE_COLOR=1` straight
+/// through here, and briefly nub did not.
+#[test]
+fn an_unprefixed_run_passes_the_color_env_through_untouched() {
+    let fixture = fixtures_dir().join("monorepo-deps").join("packages/core");
+    let output = Command::new(nub_binary())
+        .args(["run", "color-probe"])
+        .current_dir(&fixture)
+        .env("FORCE_COLOR", "1")
+        .env("NO_COLOR", "1")
+        .output()
+        .expect("spawn nub");
+    assert!(
+        String::from_utf8_lossy(&output.stdout).contains("FC=1"),
+        "an inherited-stdio run must not rewrite the script's FORCE_COLOR: {}",
+        String::from_utf8_lossy(&output.stdout)
+    );
+}
+
+/// The control for the case above: with only ONE of the two set there is no
+/// contradiction, so `auto` must go on forcing nothing on the child.
+#[test]
+fn auto_still_forces_nothing_when_only_no_color_is_set() {
+    let fixture = fixtures_dir().join("monorepo-deps");
+    let output = Command::new(nub_binary())
+        .args(["run", "-r", "color-probe"])
+        .current_dir(&fixture)
+        .env("NO_COLOR", "1")
+        .env_remove("FORCE_COLOR")
+        .output()
+        .expect("spawn nub");
+    assert!(
+        String::from_utf8_lossy(&output.stdout).contains("FC=unset"),
+        "NO_COLOR alone already disables the child; nothing needs forcing: {}",
+        String::from_utf8_lossy(&output.stdout)
+    );
+}
+
 #[test]
 fn ndjson_reporter_emits_valid_json_events() {
     // `--reporter=ndjson` emits one JSON object per line on stdout, covering