Skip to content

Commit 427549a

Browse files
committed
refactor(plan): make FORCE_COLOR=1 a post-resolution fallback
Previously FORCE_COLOR was pre-inserted into all_envs before pattern filtering and kept in DEFAULT_UNTRACKED_ENV, which unconditionally overwrote any parent or user-provided value. Move the insert to a post-resolution entry().or_insert() fallback and drop FORCE_COLOR from DEFAULT_UNTRACKED_ENV. The default path (no user config) is unchanged - the child still sees FORCE_COLOR=1 - but users opting in via env / untrackedEnv now get their parent's value passed through, providing an escape hatch for tools that need FORCE_COLOR=0. https://claude.ai/code/session_01JZDXjffYu9W5qykGdmkUXD
1 parent 51e35ea commit 427549a

2 files changed

Lines changed: 67 additions & 34 deletions

File tree

crates/vite_task_graph/src/config/mod.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -422,15 +422,14 @@ pub const DEFAULT_UNTRACKED_ENV: &[&str] = &[
422422
"LIBPATH",
423423
// Terminal/display
424424
//
425-
// The only color-related var allowed through by default is `FORCE_COLOR`,
426-
// which the planner pre-injects with value `1` before env resolution so
427-
// cached output is always colored. The reporter strips colors at the
428-
// writer level when the user's terminal cannot render them. Other
429-
// color-related vars (`NO_COLOR`, `COLORTERM`, `TERM`, `TERM_PROGRAM`)
430-
// are intentionally NOT included — users may opt in to passing them
431-
// through via a task's `env`/`untrackedEnv` config.
425+
// No color-related vars are included by default. The planner ensures
426+
// `FORCE_COLOR=1` is set on the child after env resolution (as a fallback
427+
// when neither the parent env nor task config provides one), so cached
428+
// output is always colored. The reporter strips colors at the writer
429+
// level when the user's terminal cannot render them. Users wanting to
430+
// pass through `NO_COLOR`, `COLORTERM`, `TERM`, `TERM_PROGRAM`, or
431+
// override `FORCE_COLOR` can opt in via a task's `env`/`untrackedEnv`.
432432
"DISPLAY",
433-
"FORCE_COLOR",
434433
// Temporary directories
435434
"TMP",
436435
"TEMP",

crates/vite_task_plan/src/envs.rs

Lines changed: 60 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,15 @@ impl EnvFingerprints {
7676
/// Before the call, `all_envs` is expected to contain all available envs.
7777
/// After the call, it will be modified to contain only envs to be passed to the execution (fingerprinted + untracked).
7878
///
79-
/// `FORCE_COLOR` is pre-inserted with value `"1"` so cached output is
80-
/// always colored. Because `FORCE_COLOR` is part of `DEFAULT_UNTRACKED_ENV`,
81-
/// the pattern filter below keeps it; its value (`"1"`) is left untracked
82-
/// (not part of the cache fingerprint).
79+
/// After pattern filtering, `FORCE_COLOR=1` is inserted as a fallback if
80+
/// nothing else set it, so cached output is always colored by default.
81+
/// Tasks that need a different value (e.g. `FORCE_COLOR=0` to suppress
82+
/// ANSI for a misbehaving tool) can opt in to passthrough by listing
83+
/// `FORCE_COLOR` in `env` or `untrackedEnv`.
8384
pub fn resolve(
8485
all_envs: &mut FxHashMap<Arc<OsStr>, Arc<OsStr>>,
8586
env_config: &EnvConfig,
8687
) -> Result<Self, ResolveEnvError> {
87-
all_envs.insert(OsStr::new("FORCE_COLOR").into(), Arc::<OsStr>::from(OsStr::new("1")));
88-
8988
// Collect all envs matching fingerprinted or untracked envs in env_config
9089
*all_envs = resolve_envs_with_patterns(
9190
all_envs.iter(),
@@ -97,6 +96,14 @@ impl EnvFingerprints {
9796
.collect::<Vec<&str>>(),
9897
)?;
9998

99+
// Ensure cached output is colored by default. Skipped if the user
100+
// opted into passing `FORCE_COLOR` through (via `env` / `untrackedEnv`)
101+
// and the parent supplied a value — in that case the user's choice
102+
// wins, even `FORCE_COLOR=0`.
103+
all_envs
104+
.entry(Arc::<OsStr>::from(OsStr::new("FORCE_COLOR")))
105+
.or_insert_with(|| Arc::<OsStr>::from(OsStr::new("1")));
106+
100107
// Resolve fingerprinted envs
101108
let mut fingerprinted_envs = BTreeMap::<Str, Arc<str>>::new();
102109
if !env_config.fingerprinted_envs.is_empty() {
@@ -223,14 +230,13 @@ mod tests {
223230
}
224231

225232
#[test]
226-
fn test_force_color_always_set_to_one() {
227-
// `FORCE_COLOR=1` is pre-injected before pattern filtering so cached
228-
// output is always colored. Because the merged untracked-env list
229-
// (config resolution adds DEFAULT_UNTRACKED_ENV, which includes
230-
// `FORCE_COLOR`) keeps it, the child sees `FORCE_COLOR=1` regardless
231-
// of the parent's value.
233+
fn test_force_color_defaults_to_one_when_user_does_not_opt_in() {
234+
// The user did not list `FORCE_COLOR` in `env` or `untrackedEnv`, so
235+
// the parent's value is filtered out by the pattern step. The
236+
// post-resolution fallback then inserts `FORCE_COLOR=1` so cached
237+
// output is colored.
232238
let mut all_envs = create_test_envs(vec![("PATH", "/usr/bin"), ("FORCE_COLOR", "2")]);
233-
let env_config = create_env_config(&[], &["PATH", "FORCE_COLOR"]);
239+
let env_config = create_env_config(&[], &["PATH"]);
234240

235241
let _result = EnvFingerprints::resolve(&mut all_envs, &env_config).unwrap();
236242

@@ -241,32 +247,60 @@ mod tests {
241247
}
242248

243249
#[test]
244-
fn test_force_color_dropped_when_pattern_does_not_allow_it() {
245-
// The resolver itself only pre-injects; it does not force-keep
246-
// `FORCE_COLOR` through the filter. Real callers always provide
247-
// patterns that include `FORCE_COLOR` (via `DEFAULT_UNTRACKED_ENV`),
248-
// but this test pins the contract: if `FORCE_COLOR` is absent from
249-
// the merged pattern list, the filter drops it.
250+
fn test_force_color_defaults_to_one_when_absent_from_parent() {
251+
// Parent env has no `FORCE_COLOR` at all. The fallback still inserts
252+
// `FORCE_COLOR=1` so the child emits colored output.
250253
let mut all_envs = create_test_envs(vec![("PATH", "/usr/bin")]);
251254
let env_config = create_env_config(&[], &["PATH"]);
252255

253256
let _result = EnvFingerprints::resolve(&mut all_envs, &env_config).unwrap();
254257

255-
assert!(!all_envs.contains_key(OsStr::new("FORCE_COLOR")));
258+
assert_eq!(
259+
all_envs.get(OsStr::new("FORCE_COLOR")).unwrap().to_str().unwrap(),
260+
"1"
261+
);
262+
}
263+
264+
#[test]
265+
fn test_force_color_passthrough_when_user_opts_in_via_untracked() {
266+
// If the user lists `FORCE_COLOR` in `untrackedEnv`, the parent's
267+
// value passes through verbatim and the fallback is skipped.
268+
let mut all_envs = create_test_envs(vec![("FORCE_COLOR", "0")]);
269+
let env_config = create_env_config(&[], &["FORCE_COLOR"]);
270+
271+
let result = EnvFingerprints::resolve(&mut all_envs, &env_config).unwrap();
272+
273+
assert_eq!(all_envs.get(OsStr::new("FORCE_COLOR")).unwrap().to_str().unwrap(), "0");
274+
assert!(!result.fingerprinted_envs.contains_key("FORCE_COLOR"));
256275
}
257276

258277
#[test]
259-
fn test_force_color_value_one_overrides_user_fingerprinted_value() {
260-
// A user can list `FORCE_COLOR` as a fingerprinted env, but the
261-
// pre-injection still wins — fingerprint records `"1"`, not the
262-
// parent's value. (`FORCE_COLOR` is the colour-pipeline contract;
263-
// users wanting a different colour level should configure the tool
264-
// they're running, not the runner.)
278+
fn test_force_color_passthrough_when_user_opts_in_via_fingerprinted() {
279+
// If the user lists `FORCE_COLOR` in `env` (fingerprinted), the
280+
// parent's value passes through and is recorded in the cache key.
265281
let mut all_envs = create_test_envs(vec![("FORCE_COLOR", "3")]);
266282
let env_config = create_env_config(&["FORCE_COLOR"], &[]);
267283

268284
let result = EnvFingerprints::resolve(&mut all_envs, &env_config).unwrap();
269285

286+
assert_eq!(all_envs.get(OsStr::new("FORCE_COLOR")).unwrap().to_str().unwrap(), "3");
287+
assert_eq!(
288+
result.fingerprinted_envs.get("FORCE_COLOR").map(std::convert::AsRef::as_ref),
289+
Some("3")
290+
);
291+
}
292+
293+
#[test]
294+
fn test_force_color_fallback_fingerprinted_when_opted_in_but_parent_absent() {
295+
// User opts in to `FORCE_COLOR` as fingerprinted, but parent has no
296+
// value. The fallback supplies `1`, and because the fingerprint scan
297+
// runs after the fallback, `1` is recorded in the cache key — keeping
298+
// the fingerprint consistent with what the child actually sees.
299+
let mut all_envs = create_test_envs(vec![]);
300+
let env_config = create_env_config(&["FORCE_COLOR"], &[]);
301+
302+
let result = EnvFingerprints::resolve(&mut all_envs, &env_config).unwrap();
303+
270304
assert_eq!(all_envs.get(OsStr::new("FORCE_COLOR")).unwrap().to_str().unwrap(), "1");
271305
assert_eq!(
272306
result.fingerprinted_envs.get("FORCE_COLOR").map(std::convert::AsRef::as_ref),

0 commit comments

Comments
 (0)