From 17f12d55658130d496db7c9e159fcc512f981963 Mon Sep 17 00:00:00 2001 From: James Brink Date: Wed, 15 Jul 2026 10:48:46 -0700 Subject: [PATCH 1/3] fix(env): wait for shell probe before agent spawn --- .../docs/features/shell-environment.mdx | 2 + src-tauri/src/commands/env.rs | 6 +- src-tauri/src/main.rs | 11 +- src/agent/codex_app_server.rs | 3 +- src/agent/environment.rs | 43 ++++- src/agent/process.rs | 2 +- src/agent/session.rs | 2 +- src/env.rs | 181 +++++++++++++++++- src/env_provider/mod.rs | 26 +-- 9 files changed, 238 insertions(+), 38 deletions(-) diff --git a/site/src/content/docs/features/shell-environment.mdx b/site/src/content/docs/features/shell-environment.mdx index 58cbc858a..dba0a74f2 100644 --- a/site/src/content/docs/features/shell-environment.mdx +++ b/site/src/content/docs/features/shell-environment.mdx @@ -11,6 +11,8 @@ Claudette captures that environment for you. At app launch, Claudette runs your shell as `$SHELL -l -i -c 'env -0'` (or the equivalent fish command), captures the result, diffs it against the launchd / desktop-launcher baseline, applies a denylist, and exposes the remaining vars as a low-precedence env tier. Per-project env from direnv, mise, and dotenv plugins layers on top — they always win on key collision. +The probe runs in the background, but the first agent or integrated terminal waits for that bounded probe before spawning. This ensures sessions opened immediately after a Finder, Dock, Spotlight, or desktop-launcher start receive the captured `PATH` instead of permanently inheriting the minimal GUI `PATH`. If the probe fails or times out, Claudette falls back to the environment supplied by the OS. + You can inspect and control what was captured at **Settings → Environment → Shell environment**. ## Denylist diff --git a/src-tauri/src/commands/env.rs b/src-tauri/src/commands/env.rs index 97470eca5..46205bd59 100644 --- a/src-tauri/src/commands/env.rs +++ b/src-tauri/src/commands/env.rs @@ -1474,7 +1474,7 @@ pub async fn set_shell_env_denylist( // can call list_shell_env again without restarting. claudette::env::invalidate_shell_env(); let cleaned: Vec = patterns.into_iter().filter(|s| !s.is_empty()).collect(); - std::thread::spawn(move || claudette::env::prewarm_shell_env(cleaned)); + claudette::env::prewarm_shell_env(cleaned); Ok(()) } @@ -1513,7 +1513,7 @@ pub async fn reload_shell_env(state: State<'_, AppState>) -> Result<(), String> }) .unwrap_or_default(); claudette::env::invalidate_shell_env(); - std::thread::spawn(move || claudette::env::prewarm_shell_env(user_deny)); + claudette::env::prewarm_shell_env(user_deny); Ok(()) } @@ -1630,7 +1630,7 @@ pub fn setup_env_watcher(app: AppHandle) { .collect() }) .unwrap_or_default(); - std::thread::spawn(move || claudette::env::prewarm_shell_env(user_deny)); + claudette::env::prewarm_shell_env(user_deny); let _ = app_for_cb.emit( "env-cache-invalidated", EnvCacheInvalidatedPayload { diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs index a7ea47dfa..a23fac710 100644 --- a/src-tauri/src/main.rs +++ b/src-tauri/src/main.rs @@ -841,11 +841,12 @@ fn main() { // (tokio runtime may not be available during setup). std::thread::spawn(usage::warm_user_agent_cache_sync); - // Pre-warm the shell-env cache. On Unix, this spawns + // Pre-warm the shell-env cache. On Unix, this schedules // `$SHELL -l -i -c ''` — a NUL-delimited env dump via - // `env -0` (or the fish equivalent) — with a 5-second timeout. - // Fine to pay once at startup on a std thread, but lethal if it - // ever runs inline on a Tokio worker. On Windows this is a no-op. + // `env -0` (or the fish equivalent) — on its own worker thread. + // Calling the scheduler directly marks the probe as running before + // setup continues, so immediate agent/terminal launches can await + // it without racing the worker. On Windows this is a no-op. // Read user-supplied shell-env deny patterns from app_settings // so the very first probe already filters per the user's policy. let user_deny: Vec = match claudette::db::Database::open(&db_path) { @@ -862,7 +863,7 @@ fn main() { .unwrap_or_default(), Err(_) => Vec::new(), }; - std::thread::spawn(move || claudette::env::prewarm_shell_env(user_deny)); + claudette::env::prewarm_shell_env(user_deny); // Set up the system tray icon (respects tray_enabled setting). if let Err(e) = tray::setup_tray(app.handle()) { diff --git a/src/agent/codex_app_server.rs b/src/agent/codex_app_server.rs index d7fc34744..79fa250fa 100644 --- a/src/agent/codex_app_server.rs +++ b/src/agent/codex_app_server.rs @@ -143,7 +143,8 @@ impl CodexAppServerSession { &args, working_dir, options.resolved_env.as_ref(), - ); + ) + .await; let mut cmd = built_command.command; cmd.stdin(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped()) diff --git a/src/agent/environment.rs b/src/agent/environment.rs index 7425f0c0c..1eb68bbee 100644 --- a/src/agent/environment.rs +++ b/src/agent/environment.rs @@ -41,12 +41,21 @@ pub(crate) struct AgentCommand { /// terminal's process model. That keeps Nix devshell agents independent /// from `env-direnv` and avoids importing the user's shell/profile /// environment into the agent process. -pub(crate) fn build_agent_command( +pub(crate) async fn build_agent_command( program: &OsStr, args: &[String], working_dir: &Path, resolved_env: Option<&ResolvedEnv>, ) -> AgentCommand { + // Some control-plane sessions do not resolve workspace providers first. + // Keep the command builder as the final readiness gate so every harness + // receives the enriched PATH even when launched immediately at startup. + // Workspace resolution records an explicit disabled source, allowing + // users who opted out of shell-env forwarding to skip the probe entirely. + if !resolved_env.is_some_and(shell_env_is_disabled) { + crate::env::wait_for_shell_env_probe().await; + } + let wrapped_argv = resolved_env.and_then(|env| { crate::env_provider::nix_develop_command_wrap(working_dir, env, program, args) }); @@ -85,6 +94,12 @@ pub(crate) fn build_agent_command( } } +fn shell_env_is_disabled(env: &ResolvedEnv) -> bool { + env.sources.iter().any(|source| { + source.plugin_name == "shell-env" && source.error.as_deref() == Some("disabled") + }) +} + fn agent_path(provider_path: Option<&Option>) -> OsString { // A provider that unsets PATH (`Some(None)`) or emits none at all // (`None`) leaves the agent on the app's enriched PATH; an emitted @@ -98,7 +113,7 @@ fn agent_path(provider_path: Option<&Option>) -> OsString { #[cfg(test)] mod tests { - use super::{agent_path, build_agent_command}; + use super::{agent_path, build_agent_command, shell_env_is_disabled}; use crate::env_provider::{ResolvedEnv, ResolvedSource}; use std::ffi::OsStr; use std::time::SystemTime; @@ -126,6 +141,23 @@ mod tests { assert_eq!(agent_path(Some(&None)), crate::env::enriched_path()); } + #[test] + fn disabled_shell_env_source_skips_readiness_gate() { + let resolved = ResolvedEnv { + vars: Default::default(), + sources: vec![ResolvedSource { + plugin_name: "shell-env".to_string(), + detected: false, + vars_contributed: 0, + cached: false, + evaluated_at: SystemTime::now(), + error: Some("disabled".to_string()), + }], + }; + + assert!(shell_env_is_disabled(&resolved)); + } + #[test] fn teammate_command_env_var_name_matches_claude_code() { assert_eq!( @@ -134,8 +166,8 @@ mod tests { ); } - #[test] - fn agent_command_does_not_wrap_for_direnv_only() { + #[tokio::test] + async fn agent_command_does_not_wrap_for_direnv_only() { let resolved = ResolvedEnv { vars: Default::default(), sources: vec![ResolvedSource { @@ -153,7 +185,8 @@ mod tests { &["--print".to_string()], std::path::Path::new("/tmp"), Some(&resolved), - ); + ) + .await; assert_eq!(built.invocation_program, OsStr::new("claude")); assert_eq!(built.invocation_args, ["--print"]); diff --git a/src/agent/process.rs b/src/agent/process.rs index 5a4db1d2c..966ad76b4 100644 --- a/src/agent/process.rs +++ b/src/agent/process.rs @@ -90,7 +90,7 @@ pub async fn run_turn( let claude_path = resolve_claude_path().await; let built_command = - build_agent_command(claude_path.as_os_str(), &args, working_dir, resolved_env); + build_agent_command(claude_path.as_os_str(), &args, working_dir, resolved_env).await; let invocation_program = built_command.invocation_program.clone(); let invocation_args = built_command.invocation_args.clone(); let mut cmd = built_command.command; diff --git a/src/agent/session.rs b/src/agent/session.rs index ed5ab1f26..985ed1337 100644 --- a/src/agent/session.rs +++ b/src/agent/session.rs @@ -93,7 +93,7 @@ impl PersistentSession { let claude_path = resolve_claude_path().await; let built_command = - build_agent_command(claude_path.as_os_str(), &args, working_dir, resolved_env); + build_agent_command(claude_path.as_os_str(), &args, working_dir, resolved_env).await; let invocation_program = built_command.invocation_program.clone(); let invocation_args = built_command.invocation_args.clone(); let mut cmd = built_command.command; diff --git a/src/env.rs b/src/env.rs index 13946db34..d144afbb1 100644 --- a/src/env.rs +++ b/src/env.rs @@ -14,8 +14,8 @@ use std::collections::BTreeMap; use std::ffi::OsString; use std::path::Path; -use std::sync::{Arc, OnceLock, RwLock}; -use std::time::SystemTime; +use std::sync::{Arc, Mutex, OnceLock, RwLock}; +use std::time::{Duration, SystemTime}; /// Snapshot of `std::env::vars_os()` captured before any other env /// mutation. Used to diff the shell-probe output and forward only @@ -29,6 +29,33 @@ static LAUNCH_ENV: OnceLock> = OnceLock::new(); /// downstream work. static SHELL_ENV: RwLock>> = RwLock::new(None); +/// Lifecycle of the asynchronous shell probe. This is separate from +/// `SHELL_ENV`: a completed probe may legitimately produce no environment +/// (`$SHELL` unset, invalid shell, timeout), and waiters must still be released +/// rather than delaying every future subprocess spawn. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ShellEnvProbePhase { + Idle, + Running, + Complete, +} + +#[derive(Clone, Copy, Debug)] +struct ShellEnvProbeState { + generation: u64, + phase: ShellEnvProbePhase, +} + +static SHELL_ENV_PROBE_STATE: Mutex = Mutex::new(ShellEnvProbeState { + generation: 0, + phase: ShellEnvProbePhase::Idle, +}); + +/// The interactive probe and its login-only fallback each have a five-second +/// process timeout. Keep the async readiness gate bounded slightly above both. +const SHELL_ENV_PROBE_WAIT_TIMEOUT: Duration = Duration::from_secs(11); +const SHELL_ENV_PROBE_POLL_INTERVAL: Duration = Duration::from_millis(25); + /// Captured set of env vars from the user's interactive shell init /// (after diff vs `LAUNCH_ENV` and after denylist filtering). #[derive(Debug, Clone)] @@ -72,6 +99,77 @@ pub fn shell_env_is_cached() -> bool { SHELL_ENV.read().map(|g| g.is_some()).unwrap_or(false) } +fn shell_env_probe_phase() -> ShellEnvProbePhase { + SHELL_ENV_PROBE_STATE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .phase +} + +fn begin_shell_env_probe() -> Option { + let mut state = SHELL_ENV_PROBE_STATE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if state.phase != ShellEnvProbePhase::Idle { + return None; + } + state.phase = ShellEnvProbePhase::Running; + Some(state.generation) +} + +/// Publish a worker's result only if it still belongs to the active probe. +/// Invalidation increments the generation, preventing a stale worker from +/// overwriting a newer reload or releasing its readiness gate early. +fn finish_shell_env_probe(generation: u64, env: Option>) { + let mut state = SHELL_ENV_PROBE_STATE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if state.generation != generation || state.phase != ShellEnvProbePhase::Running { + return; + } + if let Some(env) = env + && let Ok(mut guard) = SHELL_ENV.write() + { + *guard = Some(env); + } + state.phase = ShellEnvProbePhase::Complete; +} + +fn abandon_running_shell_env_probe() { + let mut state = SHELL_ENV_PROBE_STATE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if state.phase == ShellEnvProbePhase::Running { + // Invalidate the worker's generation so a result arriving after the + // readiness timeout cannot alter the fallback environment. + state.generation = state.generation.wrapping_add(1); + state.phase = ShellEnvProbePhase::Complete; + } +} + +/// Wait asynchronously for an active shell-environment probe to finish. +/// +/// Startup marks the probe as running before it launches the worker thread, so +/// subprocesses created immediately after app setup cannot race ahead with the +/// minimal GUI environment. A failed probe also transitions to `Complete`, +/// preserving the existing process-environment fallback without repeatedly +/// delaying later spawns. The hard timeout is defensive; the probe subprocess +/// itself is already bounded. +pub async fn wait_for_shell_env_probe() { + let deadline = tokio::time::Instant::now() + SHELL_ENV_PROBE_WAIT_TIMEOUT; + while shell_env_probe_phase() == ShellEnvProbePhase::Running { + if tokio::time::Instant::now() >= deadline { + tracing::warn!( + target: "claudette::env", + "timed out waiting for shell-env probe; using available process environment", + ); + abandon_running_shell_env_probe(); + break; + } + tokio::time::sleep(SHELL_ENV_PROBE_POLL_INTERVAL).await; + } +} + /// Parse a NUL-delimited `env`-style dump (`KEY=VALUE\0KEY=VALUE\0...`). /// /// Splits each chunk on the FIRST `=`. Malformed entries (no `=`, @@ -352,6 +450,11 @@ pub fn diff_against_baseline( /// Invalidate the cached shell env. Next call to `shell_env()` returns /// `None` until the probe re-runs. Used by the rc-file watcher. pub fn invalidate_shell_env() { + let mut state = SHELL_ENV_PROBE_STATE + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + state.generation = state.generation.wrapping_add(1); + state.phase = ShellEnvProbePhase::Idle; if let Ok(mut guard) = SHELL_ENV.write() { *guard = None; } @@ -389,6 +492,18 @@ pub fn run_probe_pipeline( shell: &std::path::Path, baseline: &BTreeMap, user_deny: &[String], +) -> Option> { + let env = capture_shell_env(shell, baseline, user_deny)?; + if let Ok(mut guard) = SHELL_ENV.write() { + *guard = Some(Arc::clone(&env)); + } + Some(env) +} + +fn capture_shell_env( + shell: &std::path::Path, + baseline: &BTreeMap, + user_deny: &[String], ) -> Option> { let raw = probe_shell_env_with_shell(shell)?; let (added, inherited_raw) = partition_against_baseline(&raw, baseline); @@ -401,15 +516,11 @@ pub fn run_probe_pipeline( n_denied = dropped_added.len(), "shell_env probe captured", ); - let env = Arc::new(ShellEnv { + Some(Arc::new(ShellEnv { vars: kept_added, inherited: kept_inherited, captured_at: SystemTime::now(), - }); - if let Ok(mut guard) = SHELL_ENV.write() { - *guard = Some(Arc::clone(&env)); - } - Some(env) + })) } /// Prewarm the shell-env cache on a `std::thread::spawn`. Idempotent: @@ -420,13 +531,20 @@ pub fn prewarm_shell_env(user_deny: Vec) { if shell_env_is_cached() { return; } + let Some(generation) = begin_shell_env_probe() else { + return; + }; let shell = match std::env::var("SHELL") { Ok(s) => s, - Err(_) => return, + Err(_) => { + finish_shell_env_probe(generation, None); + return; + } }; let baseline = LAUNCH_ENV.get().cloned().unwrap_or_default(); std::thread::spawn(move || { - let _ = run_probe_pipeline(std::path::Path::new(&shell), &baseline, &user_deny); + let env = capture_shell_env(std::path::Path::new(&shell), &baseline, &user_deny); + finish_shell_env_probe(generation, env); }); } @@ -967,6 +1085,9 @@ mod tests { /// probe. We assert both invariants here. #[test] fn prewarm_shell_path_is_idempotent() { + let _guard = SHELL_ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); prewarm_shell_path(); prewarm_shell_path(); // A follow-up direct call must still succeed without panicking @@ -984,6 +1105,9 @@ mod tests { #[cfg(unix)] #[test] fn prewarm_populates_shell_path_cache_on_unix() { + let _guard = SHELL_ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); // prewarm_shell_path is now a shim over prewarm_shell_env. // Give the background thread a moment to populate the cache. prewarm_shell_path(); @@ -1013,11 +1137,48 @@ mod tests { #[cfg(unix)] #[test] fn shell_path_is_cached_is_true_after_prewarm() { + let _guard = SHELL_ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); prewarm_shell_path(); // Non-panicking return is the invariant we can assert here. let _ = shell_path_is_cached(); } + #[cfg(unix)] + #[test] + fn subprocess_readiness_waits_for_running_probe() { + let _guard = SHELL_ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + + // A prewarm smoke test may have started the real shell worker on a + // parallel test thread. Let it finish before installing synthetic + // state for this timing assertion. + while shell_env_probe_phase() == ShellEnvProbePhase::Running { + std::thread::sleep(SHELL_ENV_PROBE_POLL_INTERVAL); + } + + invalidate_shell_env(); + let generation = begin_shell_env_probe().expect("synthetic probe should start"); + + let finisher = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(75)); + finish_shell_env_probe(generation, None); + }); + let started = std::time::Instant::now(); + tokio::runtime::Builder::new_current_thread() + .enable_time() + .build() + .unwrap() + .block_on(wait_for_shell_env_probe()); + + assert!(started.elapsed() >= Duration::from_millis(50)); + assert_eq!(shell_env_probe_phase(), ShellEnvProbePhase::Complete); + finisher.join().unwrap(); + invalidate_shell_env(); + } + // ---- parse_env_dump tests ----------------------------------------------- #[test] diff --git a/src/env_provider/mod.rs b/src/env_provider/mod.rs index 13e5facb8..2e6a38376 100644 --- a/src/env_provider/mod.rs +++ b/src/env_provider/mod.rs @@ -89,6 +89,14 @@ pub async fn resolve_with_registry_streaming( progress: Option<&dyn EnvProgressSink>, streaming: Option>, ) -> ResolvedEnv { + // App startup probes the user's login shell on a background thread. Wait + // for that bounded probe before snapshotting providers so the first agent + // or terminal cannot permanently inherit launchd's minimal GUI PATH. An + // explicitly disabled shell-env tier neither needs nor uses the result. + if !disabled.contains("shell-env") { + crate::env::wait_for_shell_env_probe().await; + } + let backend = PluginRegistryBackend::new(registry).with_streaming_sink(streaming); resolve_for_workspace_with_progress(&backend, cache, worktree, ws_info, disabled, progress) .await @@ -377,10 +385,11 @@ pub async fn resolve_for_workspace_with_progress( ) -> ResolvedEnv { let mut names = backend.env_provider_names(); // Inject shell-env as a synthetic precedence-0 source whenever the - // global probe has produced a cached value. It is NOT a plugin and - // does NOT go through the backend abstraction — it is handled inline - // in the loop below before any backend.is_plugin_disabled check. - if crate::env::shell_env_is_cached() { + // global probe has produced a cached value. Keep an explicitly-disabled + // cold source too: downstream command builders use that marker to avoid + // waiting for a probe whose result the user chose not to forward. It is + // NOT a plugin and does not go through the backend abstraction. + if crate::env::shell_env_is_cached() || disabled.contains("shell-env") { names.push("shell-env".to_string()); } // Sort: primary by precedence (ascending, so higher overwrites on @@ -1670,13 +1679,6 @@ mod tests { .lock() .unwrap_or_else(|e| e.into_inner()); crate::env::invalidate_shell_env(); - let mut vars = std::collections::BTreeMap::new(); - vars.insert("LEAKED".into(), "no".into()); - crate::env::install_shell_env_for_test(crate::env::ShellEnv { - vars, - inherited: std::collections::BTreeMap::new(), - captured_at: std::time::SystemTime::UNIX_EPOCH, - }); let backend = MockBackend::new(); let cache = EnvCache::new(); @@ -1694,7 +1696,7 @@ mod tests { crate::env::invalidate_shell_env(); assert!( - !resolved.vars.contains_key("LEAKED"), + resolved.vars.is_empty(), "disabled shell-env must not contribute vars", ); let shell_source = resolved From aecbdc567067647bdfd0aaeff1c10db910316686 Mon Sep 17 00:00:00 2001 From: James Brink Date: Tue, 21 Jul 2026 12:36:07 -0700 Subject: [PATCH 2/3] test(env): cover cached disabled shell environment --- src/env_provider/mod.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/env_provider/mod.rs b/src/env_provider/mod.rs index 2e6a38376..aa8d453e6 100644 --- a/src/env_provider/mod.rs +++ b/src/env_provider/mod.rs @@ -1679,6 +1679,13 @@ mod tests { .lock() .unwrap_or_else(|e| e.into_inner()); crate::env::invalidate_shell_env(); + let mut vars = std::collections::BTreeMap::new(); + vars.insert("LEAKED".into(), "no".into()); + crate::env::install_shell_env_for_test(crate::env::ShellEnv { + vars, + inherited: std::collections::BTreeMap::new(), + captured_at: std::time::SystemTime::UNIX_EPOCH, + }); let backend = MockBackend::new(); let cache = EnvCache::new(); @@ -1696,8 +1703,8 @@ mod tests { crate::env::invalidate_shell_env(); assert!( - resolved.vars.is_empty(), - "disabled shell-env must not contribute vars", + !resolved.vars.contains_key("LEAKED"), + "cached shell-env vars must not leak when the provider is disabled", ); let shell_source = resolved .sources From ea13cfeef88f38f53d52f22c4ceb252f27714861 Mon Sep 17 00:00:00 2001 From: James Brink Date: Tue, 21 Jul 2026 12:54:43 -0700 Subject: [PATCH 3/3] fix(env): cache shell probe results after wait timeout --- src/env.rs | 78 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 70 insertions(+), 8 deletions(-) diff --git a/src/env.rs b/src/env.rs index d144afbb1..afac18745 100644 --- a/src/env.rs +++ b/src/env.rs @@ -37,6 +37,7 @@ static SHELL_ENV: RwLock>> = RwLock::new(None); enum ShellEnvProbePhase { Idle, Running, + WaitExpired, Complete, } @@ -119,12 +120,19 @@ fn begin_shell_env_probe() -> Option { /// Publish a worker's result only if it still belongs to the active probe. /// Invalidation increments the generation, preventing a stale worker from -/// overwriting a newer reload or releasing its readiness gate early. +/// overwriting a newer reload. A worker may still publish after its readiness +/// wait expires so a transient startup delay does not disable shell-env for the +/// rest of the app session. fn finish_shell_env_probe(generation: u64, env: Option>) { let mut state = SHELL_ENV_PROBE_STATE .lock() .unwrap_or_else(|poisoned| poisoned.into_inner()); - if state.generation != generation || state.phase != ShellEnvProbePhase::Running { + if state.generation != generation + || !matches!( + state.phase, + ShellEnvProbePhase::Running | ShellEnvProbePhase::WaitExpired + ) + { return; } if let Some(env) = env @@ -135,15 +143,12 @@ fn finish_shell_env_probe(generation: u64, env: Option>) { state.phase = ShellEnvProbePhase::Complete; } -fn abandon_running_shell_env_probe() { +fn release_shell_env_probe_waiters() { let mut state = SHELL_ENV_PROBE_STATE .lock() .unwrap_or_else(|poisoned| poisoned.into_inner()); if state.phase == ShellEnvProbePhase::Running { - // Invalidate the worker's generation so a result arriving after the - // readiness timeout cannot alter the fallback environment. - state.generation = state.generation.wrapping_add(1); - state.phase = ShellEnvProbePhase::Complete; + state.phase = ShellEnvProbePhase::WaitExpired; } } @@ -163,7 +168,7 @@ pub async fn wait_for_shell_env_probe() { target: "claudette::env", "timed out waiting for shell-env probe; using available process environment", ); - abandon_running_shell_env_probe(); + release_shell_env_probe_waiters(); break; } tokio::time::sleep(SHELL_ENV_PROBE_POLL_INTERVAL).await; @@ -1179,6 +1184,63 @@ mod tests { invalidate_shell_env(); } + #[test] + fn late_probe_result_is_cached_after_readiness_wait_expires() { + let _guard = SHELL_ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + invalidate_shell_env(); + let generation = begin_shell_env_probe().expect("synthetic probe should start"); + + release_shell_env_probe_waiters(); + assert_eq!(shell_env_probe_phase(), ShellEnvProbePhase::WaitExpired); + + let mut vars = BTreeMap::new(); + vars.insert("LATE_PROBE".to_string(), "published".to_string()); + finish_shell_env_probe( + generation, + Some(Arc::new(ShellEnv { + vars, + inherited: BTreeMap::new(), + captured_at: SystemTime::UNIX_EPOCH, + })), + ); + + assert_eq!( + shell_env() + .and_then(|env| env.vars.get("LATE_PROBE").cloned()) + .as_deref(), + Some("published"), + ); + assert_eq!(shell_env_probe_phase(), ShellEnvProbePhase::Complete); + invalidate_shell_env(); + } + + #[test] + fn invalidation_rejects_late_probe_result_after_wait_expires() { + let _guard = SHELL_ENV_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + invalidate_shell_env(); + let generation = begin_shell_env_probe().expect("synthetic probe should start"); + release_shell_env_probe_waiters(); + + invalidate_shell_env(); + let mut vars = BTreeMap::new(); + vars.insert("STALE_PROBE".to_string(), "rejected".to_string()); + finish_shell_env_probe( + generation, + Some(Arc::new(ShellEnv { + vars, + inherited: BTreeMap::new(), + captured_at: SystemTime::UNIX_EPOCH, + })), + ); + + assert!(shell_env().is_none()); + assert_eq!(shell_env_probe_phase(), ShellEnvProbePhase::Idle); + } + // ---- parse_env_dump tests ----------------------------------------------- #[test]