From a7035a056873523d48bae84af1e26a03e87a5d90 Mon Sep 17 00:00:00 2001 From: Kevin Date: Fri, 21 Aug 2026 20:47:03 +0900 Subject: [PATCH] fix(cache): exclude untrackedEnv names from tracked bulk env queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tracked `getEnvs` query was validated against the full unfiltered parent environment, so a broad query (an empty prefix matches every variable) cache-missed whenever any ambient variable changed — on GitHub Actions, per-run variables like ACTIONS_ORCHESTRATION_ID made such tasks miss every run (#505). `untrackedEnv` already declares variables non-build-affecting for spawn-env filtering (passed through, not fingerprinted). Apply the same contract to bulk queries: names matching the task's untracked patterns are excluded from the recorded match-set and from both sides of validation, while the runner still serves them to the tool. Filtering the stored side too lets entries recorded before this change keep hitting. Explicit single-name `getEnv` reads stay fingerprinted: naming a variable is a direct dependency declaration. Also add ACTIONS_* to DEFAULT_UNTRACKED_ENV next to GITHUB_*/RUNNER_*. Co-Authored-By: Claude Fable 5 --- crates/vt/src/session/cache/mod.rs | 17 +- crates/vt/src/session/execute/cache_update.rs | 19 ++- crates/vt/src/session/execute/fingerprint.rs | 154 ++++++++++++++++-- .../fixtures/ipc_client_test/snapshots.toml | 61 +++++++ ...s_tracked_query_ignores_untracked_names.md | 46 ++++++ .../fixtures/ipc_client_test/vite-task.json | 5 + crates/vt_graph/src/config/mod.rs | 4 +- 7 files changed, 282 insertions(+), 24 deletions(-) create mode 100644 crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/fetch_envs_tracked_query_ignores_untracked_names.md 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