diff --git a/crates/vt/src/session/cache/mod.rs b/crates/vt/src/session/cache/mod.rs index 06780e188..326ee5c51 100644 --- a/crates/vt/src/session/cache/mod.rs +++ b/crates/vt/src/session/cache/mod.rs @@ -341,11 +341,18 @@ impl ExecutionCache { return Ok(Err(CacheMiss::FingerprintMismatch(mismatch))); } - // Validate post-run fingerprint (inferred inputs + tracked envs) - if let Some(mismatch) = cache_value - .post_run_fingerprint - .validate(workspace_root, &cache_metadata.unfiltered_envs)? - { + // Validate post-run fingerprint (inferred inputs + tracked envs). + // Bulk-query validation excludes `untrackedEnv`-declared names on + // both sides; the untracked config is part of the spawn + // fingerprint, so it matches the one active at record time. + let untracked_env = vt_glob::env::EnvGlobSet::new( + spawn_fingerprint.env_fingerprints().untracked_env_config.iter(), + )?; + if let Some(mismatch) = cache_value.post_run_fingerprint.validate( + workspace_root, + &cache_metadata.unfiltered_envs, + &untracked_env, + )? { return Ok(Err(CacheMiss::FingerprintMismatch(mismatch.into()))); } // Associate the execution key to the cache entry key if not already, diff --git a/crates/vt/src/session/execute/cache_update.rs b/crates/vt/src/session/execute/cache_update.rs index 6d320a083..0487bbc76 100644 --- a/crates/vt/src/session/execute/cache_update.rs +++ b/crates/vt/src/session/execute/cache_update.rs @@ -4,6 +4,7 @@ use std::{collections::BTreeMap, sync::Arc, time::Duration}; use rustc_hash::FxHashSet; +use vt_glob::env::EnvGlobSet; use vt_path::{AbsolutePath, RelativePathBuf}; use vt_plan::cache_metadata::{CacheMetadata, EnvValueHash}; use vt_server::Reports; @@ -276,7 +277,7 @@ fn collect_tracked_reports( reports .map(|reports| { let tracked_envs = collect_tracked_envs(reports, metadata)?; - let tracked_env_queries = collect_tracked_env_queries(reports)?; + let tracked_env_queries = collect_tracked_env_queries(reports, metadata)?; Ok::<_, anyhow::Error>((tracked_envs, tracked_env_queries)) }) .transpose() @@ -337,8 +338,17 @@ fn collect_tracked_envs( } /// Select tool-reported bulk env query records to embed in the post-run -/// fingerprint. The full match-set is stored as value hashes. -fn collect_tracked_env_queries(reports: &Reports) -> anyhow::Result { +/// fingerprint. The match-set is stored as value hashes, minus names matching +/// the task's `untrackedEnv` patterns: those are declared non-build-affecting, +/// so a broad query sweeping them up (an empty prefix matches every ambient +/// variable) must not pin their values into the fingerprint. Validation +/// applies the same exclusion (see `PostRunFingerprint::validate`). +fn collect_tracked_env_queries( + reports: &Reports, + metadata: &CacheMetadata, +) -> anyhow::Result { + let untracked_env = + EnvGlobSet::new(metadata.spawn_fingerprint.env_fingerprints().untracked_env_config.iter())?; let mut tracked_env_queries = BTreeMap::new(); for (query, record) in &reports.tracked_get_envs { @@ -347,6 +357,9 @@ fn collect_tracked_env_queries(reports: &Reports) -> anyhow::Result, Arc>, + untracked_env: &EnvGlobSet, ) -> anyhow::Result> { let input_mismatch = self.inferred_inputs.par_iter().find_map_any( |(input_relative_path, path_fingerprint)| { @@ -203,7 +215,7 @@ impl PostRunFingerprint { } for (query, stored_matches) in &self.tracked_env_queries { - let current_matches = match match_env_query(query, unfiltered_envs)? { + let current_matches = match match_env_query(query, unfiltered_envs, untracked_env)? { EnvQueryValidation::Matches(matches) => matches, EnvQueryValidation::NonUtf8Value(mismatch) => { return Ok(Some(PostRunMismatch::TrackedEnvQuery { @@ -212,7 +224,17 @@ impl PostRunFingerprint { })); } }; - if let Some(mismatch) = first_env_glob_mismatch(stored_matches, ¤t_matches) { + // Filter the stored side too: entries recorded before untracked + // exclusion existed (or under a narrower `untrackedEnv`) may + // contain names the current config declares untracked. Comparing + // both sides post-filter lets those entries keep hitting instead + // of forcing a one-time miss. + let stored_matches: BTreeMap = stored_matches + .iter() + .filter(|(name, _)| !untracked_env.is_match(name.as_str())) + .map(|(name, value)| (name.clone(), *value)) + .collect(); + if let Some(mismatch) = first_env_glob_mismatch(&stored_matches, ¤t_matches) { return Ok(Some(PostRunMismatch::TrackedEnvQuery { query: query.clone(), mismatch, @@ -225,20 +247,22 @@ impl PostRunFingerprint { } /// Build the current match-set for `query` by enumerating the given env -/// snapshot and keeping matching UTF-8 names. If a matching env has a non-UTF-8 -/// value, return a changed mismatch so the stale cache entry is not replayed. +/// snapshot and keeping matching UTF-8 names, minus names the task declares +/// in `untrackedEnv`. If a matching env has a non-UTF-8 value, return a +/// changed mismatch so the stale cache entry is not replayed. fn match_env_query( query: &TrackedEnvQuery, envs: &FxHashMap, Arc>, + untracked_env: &EnvGlobSet, ) -> anyhow::Result { Ok(match query { TrackedEnvQuery::Glob(pattern) => { let glob = vt_glob::env::EnvGlob::new(pattern.as_str())?; - collect_matching_envs(envs, |name| glob.is_match(name)) - } - TrackedEnvQuery::Prefix(prefix) => { - collect_matching_envs(envs, |name| env_name_starts_with(name, prefix.as_str())) + collect_matching_envs(envs, |name| glob.is_match(name) && !untracked_env.is_match(name)) } + TrackedEnvQuery::Prefix(prefix) => collect_matching_envs(envs, |name| { + env_name_starts_with(name, prefix.as_str()) && !untracked_env.is_match(name) + }), }) } @@ -534,6 +558,103 @@ mod tests { OsString::from_wide(&[0xD800]) } + fn no_untracked() -> EnvGlobSet { + EnvGlobSet::new(std::iter::empty::<&str>()).expect("empty set compiles") + } + + #[test] + fn validate_excludes_untracked_names_from_bulk_query_current_side() { + // A broad (empty-prefix) tracked query must not miss when an ambient + // variable covered by `untrackedEnv` appears between runs. + let mut tracked_env_queries = BTreeMap::new(); + let mut stored_matches = BTreeMap::new(); + stored_matches.insert(Str::from("MY_VAR"), EnvValueHash::new("x")); + tracked_env_queries.insert(TrackedEnvQuery::Prefix(Str::from("")), stored_matches); + let fingerprint = + PostRunFingerprint { tracked_env_queries, ..PostRunFingerprint::default() }; + + let mut unfiltered_envs = FxHashMap::default(); + unfiltered_envs + .insert(Arc::::from(OsStr::new("MY_VAR")), Arc::::from(OsStr::new("x"))); + unfiltered_envs.insert( + Arc::::from(OsStr::new("ACTIONS_ORCHESTRATION_ID")), + Arc::::from(OsStr::new("run-12345")), + ); + + let workspace_root = vt_path::current_dir().expect("cwd"); + let untracked = EnvGlobSet::new(["ACTIONS_*"]).expect("set compiles"); + let mismatch = fingerprint + .validate(&workspace_root, &unfiltered_envs, &untracked) + .expect("validation succeeds"); + assert!(mismatch.is_none(), "untracked ambient var must not invalidate: {mismatch:?}"); + + // Sanity contrast: without the untracked exclusion the same state is + // a mismatch (the ambient variable was added to the match-set). + let mismatch = fingerprint + .validate(&workspace_root, &unfiltered_envs, &no_untracked()) + .expect("validation succeeds"); + assert!(matches!( + mismatch, + Some(PostRunMismatch::TrackedEnvQuery { mismatch: EnvMismatch::Added { .. }, .. }) + )); + } + + #[test] + fn validate_excludes_untracked_names_from_bulk_query_stored_side() { + // Entries recorded before the untracked exclusion existed may contain + // untracked names in the stored match-set; they must still hit. + let mut tracked_env_queries = BTreeMap::new(); + let mut stored_matches = BTreeMap::new(); + stored_matches.insert(Str::from("MY_VAR"), EnvValueHash::new("x")); + stored_matches.insert(Str::from("GITHUB_RUN_ID"), EnvValueHash::new("old-run")); + tracked_env_queries.insert(TrackedEnvQuery::Prefix(Str::from("")), stored_matches); + let fingerprint = + PostRunFingerprint { tracked_env_queries, ..PostRunFingerprint::default() }; + + let mut unfiltered_envs = FxHashMap::default(); + unfiltered_envs + .insert(Arc::::from(OsStr::new("MY_VAR")), Arc::::from(OsStr::new("x"))); + unfiltered_envs.insert( + Arc::::from(OsStr::new("GITHUB_RUN_ID")), + Arc::::from(OsStr::new("new-run")), + ); + + let workspace_root = vt_path::current_dir().expect("cwd"); + let untracked = EnvGlobSet::new(["GITHUB_*"]).expect("set compiles"); + let mismatch = fingerprint + .validate(&workspace_root, &unfiltered_envs, &untracked) + .expect("validation succeeds"); + assert!(mismatch.is_none(), "stored untracked names must be ignored: {mismatch:?}"); + } + + #[test] + fn validate_still_tracks_non_untracked_bulk_query_changes() { + // The exclusion must not weaken fingerprinting of variables the task + // does depend on: a changed tracked match still misses. + let mut tracked_env_queries = BTreeMap::new(); + let mut stored_matches = BTreeMap::new(); + stored_matches.insert(Str::from("MY_VAR"), EnvValueHash::new("x")); + tracked_env_queries.insert(TrackedEnvQuery::Prefix(Str::from("")), stored_matches); + let fingerprint = + PostRunFingerprint { tracked_env_queries, ..PostRunFingerprint::default() }; + + let mut unfiltered_envs = FxHashMap::default(); + unfiltered_envs.insert( + Arc::::from(OsStr::new("MY_VAR")), + Arc::::from(OsStr::new("changed")), + ); + + let workspace_root = vt_path::current_dir().expect("cwd"); + let untracked = EnvGlobSet::new(["ACTIONS_*"]).expect("set compiles"); + let mismatch = fingerprint + .validate(&workspace_root, &unfiltered_envs, &untracked) + .expect("validation succeeds"); + assert!(matches!( + mismatch, + Some(PostRunMismatch::TrackedEnvQuery { mismatch: EnvMismatch::Changed { .. }, .. }) + )); + } + #[test] fn validate_errors_on_current_non_utf8_tracked_env_value() { let mut tracked_envs = BTreeMap::new(); @@ -548,7 +669,7 @@ mod tests { let workspace_root = vt_path::current_dir().expect("cwd"); let err = fingerprint - .validate(&workspace_root, &unfiltered_envs) + .validate(&workspace_root, &unfiltered_envs, &no_untracked()) .expect_err("non-UTF-8 tracked env values must error"); assert!(err.to_string().contains("tracked env value for PROBE_ENV is not valid UTF-8")); @@ -568,8 +689,9 @@ mod tests { ); let workspace_root = vt_path::current_dir().expect("cwd"); - let mismatch = - fingerprint.validate(&workspace_root, &unfiltered_envs).expect("validation succeeds"); + let mismatch = fingerprint + .validate(&workspace_root, &unfiltered_envs, &no_untracked()) + .expect("validation succeeds"); match mismatch { Some(PostRunMismatch::TrackedEnvQuery { @@ -603,8 +725,9 @@ mod tests { ); let workspace_root = vt_path::current_dir().expect("cwd"); - let mismatch = - fingerprint.validate(&workspace_root, &unfiltered_envs).expect("validation succeeds"); + let mismatch = fingerprint + .validate(&workspace_root, &unfiltered_envs, &no_untracked()) + .expect("validation succeeds"); assert!(mismatch.is_none()); } @@ -623,8 +746,9 @@ mod tests { ); let workspace_root = vt_path::current_dir().expect("cwd"); - let mismatch = - fingerprint.validate(&workspace_root, &unfiltered_envs).expect("validation succeeds"); + let mismatch = fingerprint + .validate(&workspace_root, &unfiltered_envs, &no_untracked()) + .expect("validation succeeds"); assert!(mismatch.is_none()); } diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots.toml b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots.toml index 64f809c2f..c59d843a5 100644 --- a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots.toml +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots.toml @@ -670,3 +670,64 @@ steps = [ ], ], comment = "cache hit: changed match set was requested with tracked: false" }, ] + +[[e2e]] +name = "fetch_envs_tracked_query_ignores_untracked_names" +comment = """ +Exercises `getEnvs(pattern, { tracked: true })` combined with `untrackedEnv`. Names matching `untrackedEnv` are excluded from the tracked match-set at record and validation time, so an ambient variable swept up by a broad query (a CI orchestration id, for example) cannot invalidate the cache. The runner still serves those variables to the tool; a hit replays the recorded output, per the untracked contract. +""" +ignore = true +steps = [ + { argv = [ + "vt", + "run", + "fetch-envs-untracked-noise", + ], envs = [ + [ + "PROBE_A", + "a", + ], + ], comment = "populate: the tracked match-set records {PROBE_A}; PROBE_NOISE_* is excluded" }, + { argv = [ + "vt", + "run", + "fetch-envs-untracked-noise", + ], envs = [ + [ + "PROBE_A", + "a", + ], + [ + "PROBE_NOISE_ID", + "run-1", + ], + ], comment = "ambient noise matches the query but also untrackedEnv -> cache hit" }, + { argv = [ + "vt", + "run", + "fetch-envs-untracked-noise", + ], envs = [ + [ + "PROBE_A", + "a", + ], + [ + "PROBE_NOISE_ID", + "run-2", + ], + ], comment = "noise value changes -> still a hit" }, + { argv = [ + "vt", + "run", + "fetch-envs-untracked-noise", + ], envs = [ + [ + "PROBE_A", + "changed", + ], + [ + "PROBE_NOISE_ID", + "run-2", + ], + ], comment = "tracked match PROBE_A changes -> cache miss" }, +] diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/fetch_envs_tracked_query_ignores_untracked_names.md b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/fetch_envs_tracked_query_ignores_untracked_names.md new file mode 100644 index 000000000..3584a0eb0 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/fetch_envs_tracked_query_ignores_untracked_names.md @@ -0,0 +1,46 @@ +# fetch_envs_tracked_query_ignores_untracked_names + +Exercises `getEnvs(pattern, { tracked: true })` combined with `untrackedEnv`. Names matching `untrackedEnv` are excluded from the tracked match-set at record and validation time, so an ambient variable swept up by a broad query (a CI orchestration id, for example) cannot invalidate the cache. The runner still serves those variables to the tool; a hit replays the recorded output, per the untracked contract. + +## `PROBE_A=a vt run fetch-envs-untracked-noise` + +populate: the tracked match-set records {PROBE_A}; PROBE_NOISE_* is excluded + +``` +$ node scripts/fetch_envs.mjs +PROBE_A=a +``` + +## `PROBE_A=a PROBE_NOISE_ID=run-1 vt run fetch-envs-untracked-noise` + +ambient noise matches the query but also untrackedEnv -> cache hit + +``` +$ node scripts/fetch_envs.mjs ◉ cache hit, replaying +PROBE_A=a + +--- +vt run: cache hit. +``` + +## `PROBE_A=a PROBE_NOISE_ID=run-2 vt run fetch-envs-untracked-noise` + +noise value changes -> still a hit + +``` +$ node scripts/fetch_envs.mjs ◉ cache hit, replaying +PROBE_A=a + +--- +vt run: cache hit. +``` + +## `PROBE_A=changed PROBE_NOISE_ID=run-2 vt run fetch-envs-untracked-noise` + +tracked match PROBE_A changes -> cache miss + +``` +$ node scripts/fetch_envs.mjs ○ cache miss: env 'PROBE_A' changed, executing +PROBE_A=changed +PROBE_NOISE_ID=run-2 +``` diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/vite-task.json b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/vite-task.json index 4f00c3ceb..6428b400e 100644 --- a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/vite-task.json +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/vite-task.json @@ -84,6 +84,11 @@ "command": "node scripts/fetch_env.mjs --untracked PROBE_ENV", "cache": true }, + "fetch-envs-untracked-noise": { + "command": "node scripts/fetch_envs.mjs", + "untrackedEnv": ["PROBE_NOISE_*"], + "cache": true + }, "fetch-envs-untracked": { "command": "node scripts/fetch_envs.mjs --untracked", "cache": true diff --git a/crates/vt_graph/src/config/mod.rs b/crates/vt_graph/src/config/mod.rs index ef39165b6..a7df898c7 100644 --- a/crates/vt_graph/src/config/mod.rs +++ b/crates/vt_graph/src/config/mod.rs @@ -441,9 +441,11 @@ pub const DEFAULT_UNTRACKED_ENV: &[&str] = &[ "USE_OUTPUT_FOR_EDGE_FUNCTIONS", "NOW_BUILDER", "VC_MICROFRONTENDS_CONFIG_FILE_NAME", - // GitHub Actions + // GitHub Actions. `ACTIONS_*` covers runner-orchestration variables like + // `ACTIONS_ORCHESTRATION_ID` that change every run. "GITHUB_*", "RUNNER_*", + "ACTIONS_*", // Windows specific "APPDATA", // Node's compile cache uses LOCALAPPDATA to pick its cache directory