diff --git a/crates/vite_task_graph/src/config/mod.rs b/crates/vite_task_graph/src/config/mod.rs index 4a165dcb..a68efbec 100644 --- a/crates/vite_task_graph/src/config/mod.rs +++ b/crates/vite_task_graph/src/config/mod.rs @@ -422,15 +422,14 @@ pub const DEFAULT_UNTRACKED_ENV: &[&str] = &[ "LIBPATH", // Terminal/display // - // The only color-related var allowed through by default is `FORCE_COLOR`, - // which the planner pre-injects with value `1` before env resolution so - // cached output is always colored. The reporter strips colors at the - // writer level when the user's terminal cannot render them. Other - // color-related vars (`NO_COLOR`, `COLORTERM`, `TERM`, `TERM_PROGRAM`) - // are intentionally NOT included — users may opt in to passing them - // through via a task's `env`/`untrackedEnv` config. + // No color-related vars are included by default. The planner ensures + // `FORCE_COLOR=1` is set on the child after env resolution (as a fallback + // when neither the parent env nor task config provides one), so cached + // output is always colored. The reporter strips colors at the writer + // level when the user's terminal cannot render them. Users wanting to + // pass through `NO_COLOR`, `COLORTERM`, `TERM`, `TERM_PROGRAM`, or + // override `FORCE_COLOR` can opt in via a task's `env`/`untrackedEnv`. "DISPLAY", - "FORCE_COLOR", // Temporary directories "TMP", "TEMP", diff --git a/crates/vite_task_plan/src/envs.rs b/crates/vite_task_plan/src/envs.rs index 12d5fdce..95e410af 100644 --- a/crates/vite_task_plan/src/envs.rs +++ b/crates/vite_task_plan/src/envs.rs @@ -76,16 +76,15 @@ impl EnvFingerprints { /// Before the call, `all_envs` is expected to contain all available envs. /// After the call, it will be modified to contain only envs to be passed to the execution (fingerprinted + untracked). /// - /// `FORCE_COLOR` is pre-inserted with value `"1"` so cached output is - /// always colored. Because `FORCE_COLOR` is part of `DEFAULT_UNTRACKED_ENV`, - /// the pattern filter below keeps it; its value (`"1"`) is left untracked - /// (not part of the cache fingerprint). + /// After pattern filtering, `FORCE_COLOR=1` is inserted as a fallback if + /// nothing else set it, so cached output is always colored by default. + /// Tasks that need a different value (e.g. `FORCE_COLOR=0` to suppress + /// ANSI for a misbehaving tool) can opt in to passthrough by listing + /// `FORCE_COLOR` in `env` or `untrackedEnv`. pub fn resolve( all_envs: &mut FxHashMap, Arc>, env_config: &EnvConfig, ) -> Result { - all_envs.insert(OsStr::new("FORCE_COLOR").into(), Arc::::from(OsStr::new("1"))); - // Collect all envs matching fingerprinted or untracked envs in env_config *all_envs = resolve_envs_with_patterns( all_envs.iter(), @@ -97,6 +96,14 @@ impl EnvFingerprints { .collect::>(), )?; + // Ensure cached output is colored by default. Skipped if the user + // opted into passing `FORCE_COLOR` through (via `env` / `untrackedEnv`) + // and the parent supplied a value — in that case the user's choice + // wins, even `FORCE_COLOR=0`. + all_envs + .entry(Arc::::from(OsStr::new("FORCE_COLOR"))) + .or_insert_with(|| Arc::::from(OsStr::new("1"))); + // Resolve fingerprinted envs let mut fingerprinted_envs = BTreeMap::>::new(); if !env_config.fingerprinted_envs.is_empty() { @@ -223,14 +230,13 @@ mod tests { } #[test] - fn test_force_color_always_set_to_one() { - // `FORCE_COLOR=1` is pre-injected before pattern filtering so cached - // output is always colored. Because the merged untracked-env list - // (config resolution adds DEFAULT_UNTRACKED_ENV, which includes - // `FORCE_COLOR`) keeps it, the child sees `FORCE_COLOR=1` regardless - // of the parent's value. + fn test_force_color_defaults_to_one_when_user_does_not_opt_in() { + // The user did not list `FORCE_COLOR` in `env` or `untrackedEnv`, so + // the parent's value is filtered out by the pattern step. The + // post-resolution fallback then inserts `FORCE_COLOR=1` so cached + // output is colored. let mut all_envs = create_test_envs(vec![("PATH", "/usr/bin"), ("FORCE_COLOR", "2")]); - let env_config = create_env_config(&[], &["PATH", "FORCE_COLOR"]); + let env_config = create_env_config(&[], &["PATH"]); let _result = EnvFingerprints::resolve(&mut all_envs, &env_config).unwrap(); @@ -241,32 +247,57 @@ mod tests { } #[test] - fn test_force_color_dropped_when_pattern_does_not_allow_it() { - // The resolver itself only pre-injects; it does not force-keep - // `FORCE_COLOR` through the filter. Real callers always provide - // patterns that include `FORCE_COLOR` (via `DEFAULT_UNTRACKED_ENV`), - // but this test pins the contract: if `FORCE_COLOR` is absent from - // the merged pattern list, the filter drops it. + fn test_force_color_defaults_to_one_when_absent_from_parent() { + // Parent env has no `FORCE_COLOR` at all. The fallback still inserts + // `FORCE_COLOR=1` so the child emits colored output. let mut all_envs = create_test_envs(vec![("PATH", "/usr/bin")]); let env_config = create_env_config(&[], &["PATH"]); let _result = EnvFingerprints::resolve(&mut all_envs, &env_config).unwrap(); - assert!(!all_envs.contains_key(OsStr::new("FORCE_COLOR"))); + assert_eq!(all_envs.get(OsStr::new("FORCE_COLOR")).unwrap().to_str().unwrap(), "1"); + } + + #[test] + fn test_force_color_passthrough_when_user_opts_in_via_untracked() { + // If the user lists `FORCE_COLOR` in `untrackedEnv`, the parent's + // value passes through verbatim and the fallback is skipped. + let mut all_envs = create_test_envs(vec![("FORCE_COLOR", "0")]); + let env_config = create_env_config(&[], &["FORCE_COLOR"]); + + let result = EnvFingerprints::resolve(&mut all_envs, &env_config).unwrap(); + + assert_eq!(all_envs.get(OsStr::new("FORCE_COLOR")).unwrap().to_str().unwrap(), "0"); + assert!(!result.fingerprinted_envs.contains_key("FORCE_COLOR")); } #[test] - fn test_force_color_value_one_overrides_user_fingerprinted_value() { - // A user can list `FORCE_COLOR` as a fingerprinted env, but the - // pre-injection still wins — fingerprint records `"1"`, not the - // parent's value. (`FORCE_COLOR` is the colour-pipeline contract; - // users wanting a different colour level should configure the tool - // they're running, not the runner.) + fn test_force_color_passthrough_when_user_opts_in_via_fingerprinted() { + // If the user lists `FORCE_COLOR` in `env` (fingerprinted), the + // parent's value passes through and is recorded in the cache key. let mut all_envs = create_test_envs(vec![("FORCE_COLOR", "3")]); let env_config = create_env_config(&["FORCE_COLOR"], &[]); let result = EnvFingerprints::resolve(&mut all_envs, &env_config).unwrap(); + assert_eq!(all_envs.get(OsStr::new("FORCE_COLOR")).unwrap().to_str().unwrap(), "3"); + assert_eq!( + result.fingerprinted_envs.get("FORCE_COLOR").map(std::convert::AsRef::as_ref), + Some("3") + ); + } + + #[test] + fn test_force_color_fallback_fingerprinted_when_opted_in_but_parent_absent() { + // User opts in to `FORCE_COLOR` as fingerprinted, but parent has no + // value. The fallback supplies `1`, and because the fingerprint scan + // runs after the fallback, `1` is recorded in the cache key — keeping + // the fingerprint consistent with what the child actually sees. + let mut all_envs = create_test_envs(vec![]); + let env_config = create_env_config(&["FORCE_COLOR"], &[]); + + let result = EnvFingerprints::resolve(&mut all_envs, &env_config).unwrap(); + assert_eq!(all_envs.get(OsStr::new("FORCE_COLOR")).unwrap().to_str().unwrap(), "1"); assert_eq!( result.fingerprinted_envs.get("FORCE_COLOR").map(std::convert::AsRef::as_ref),