diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index 886ffca816..2ac63576b5 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -6,6 +6,7 @@ - Custom themes can now define separate light and dark color overrides when automatic theme switching is enabled. (#837, thanks @aneym) ### Fixed +- Panes working inside an activated conda environment or virtualenv now come back into it after a session restore, including when an agent is resumed into the pane. (#2367) - Running named servers now activate remote agent-detection manifests downloaded by another server, preventing stale agent states and `agent explain` output until restart. (#2711) - New lifecycle event subscriptions now stream only events emitted after subscription begins instead of replaying retained history. (#1270) - Windows users whose endpoint security blocks the fileless PowerShell install command can now use a local `install.cmd` bootstrap; installer downloads use `curl.exe` while preserving package checksum verification. (#2751) diff --git a/src/app/agent_resume.rs b/src/app/agent_resume.rs index 29c2c31768..ba109978d1 100644 --- a/src/app/agent_resume.rs +++ b/src/app/agent_resume.rs @@ -226,9 +226,17 @@ impl App { ); return false; }; + // Resume runs the agent from a freshly spawned shell, so it needs the + // pane's interpreter environment for the same reason restore does. + let virtual_env = self + .state + .terminals + .get(&terminal_id) + .and_then(|terminal| terminal.virtual_env.clone()); let Some(launch_env) = self .find_pane(pane_id) .and_then(|(ws_idx, _)| self.pane_launch_env(ws_idx, pane_id, Vec::new())) + .map(|launch_env| launch_env.with_virtual_env(virtual_env)) else { return false; }; diff --git a/src/pane.rs b/src/pane.rs index 9f2416ab18..852aced21f 100644 --- a/src/pane.rs +++ b/src/pane.rs @@ -84,6 +84,7 @@ fn apply_pane_terminal_env(cmd: &mut CommandBuilder) { #[derive(Debug, Clone, Default, PartialEq, Eq)] pub(crate) struct PaneLaunchEnv { extra: Vec<(String, String)>, + virtual_env: Option, identity: PaneLaunchIdentity, } @@ -103,10 +104,24 @@ impl PaneLaunchEnv { pub(crate) fn from_extra(extra: Vec<(String, String)>) -> Self { Self { extra, + virtual_env: None, identity: PaneLaunchIdentity::Inherit, } } + /// Re-enter an interpreter environment in the spawned process. + /// + /// The activation is kept whole rather than expanded into `extra` here + /// because its `PATH` has to be built from the `PATH` the process actually + /// ends up with, which is not known until every other override is applied. + pub(crate) fn with_virtual_env( + mut self, + virtual_env: Option, + ) -> Self { + self.virtual_env = virtual_env; + self + } + pub(crate) fn with_identity( mut self, workspace_id: String, @@ -132,6 +147,15 @@ fn apply_pane_launch_env(cmd: &mut CommandBuilder, launch_env: &PaneLaunchEnv) { for (key, value) in &launch_env.extra { cmd.env(key, value); } + if let Some(activation) = &launch_env.virtual_env { + // The builder starts from the server's environment, so this reads the + // PATH the pane would otherwise launch with, including anything set + // above. The activation goes in front of that value rather than + // replacing it. + for (key, value) in activation.launch_env(cmd.get_env("PATH")) { + cmd.env(key, value); + } + } cmd.env(crate::HERDR_ENV_VAR, crate::HERDR_ENV_VALUE); crate::integration::apply_pane_base_env(cmd); crate::platform::apply_pane_runtime_marker(cmd); @@ -3011,6 +3035,32 @@ impl PaneRuntime { None } } + + /// Interpreter environment the pane is currently working in, if any. + /// + /// A shell mutates its own environment in place when it activates one, and + /// that mutation is not visible from outside the process. What is visible + /// is the environment a process was launched with, so this reads the + /// foreground process group leader — the command the shell started, which + /// carries any activation that was in effect when it began. + /// + /// Session saves call this for every pane, so it stays at one process + /// lookup plus one environment read. In particular it does not ask the PTY + /// actor for the foreground group: that is a round trip which blocks while + /// the reader is paused for a handoff. + pub fn foreground_virtual_env(&self) -> Option { + let pid = self.child_pid.load(Ordering::Acquire); + if pid == 0 { + return None; + } + let foreground = crate::platform::foreground_process_group_id(pid)?; + // An idle prompt leaves the shell itself in the foreground, and the + // shell's own activation is the part that cannot be read. + if foreground == pid { + return None; + } + crate::platform::process_virtual_env(foreground) + } } #[cfg(test)] @@ -3092,6 +3142,41 @@ impl PaneRuntime { mod tests { use super::*; + #[test] + fn pane_launch_env_builds_the_activation_path_on_the_effective_launch_path() { + // Anything that sets PATH ahead of the activation has to survive it, + // so the activation is composed against the value the pane would have + // launched with rather than against the server's own PATH. + let mut cmd = CommandBuilder::new("shell"); + let activation = crate::platform::VirtualEnvActivation { + kind: crate::platform::VirtualEnvKind::Conda, + prefix: "/opt/conda/envs/web".into(), + name: Some("web".to_string()), + }; + let inherited = std::path::PathBuf::from("/refreshed/bin"); + let launch_env = PaneLaunchEnv::from_extra(vec![( + "PATH".to_string(), + inherited.to_string_lossy().into_owned(), + )]) + .with_virtual_env(Some(activation.clone())); + + apply_pane_launch_env(&mut cmd, &launch_env); + + // The directory list is the host's, so it is taken from the activation + // rather than spelled out; what this pins is the order. + let expected = activation + .path_entries() + .into_iter() + .chain(std::iter::once(inherited)) + .collect::>(); + let path = cmd.get_env("PATH").expect("expected PATH"); + assert_eq!(std::env::split_paths(path).collect::>(), expected); + assert_eq!( + cmd.get_env("CONDA_PREFIX"), + Some(std::ffi::OsStr::new("/opt/conda/envs/web")) + ); + } + #[test] fn pane_launch_env_removes_outer_codex_thread_id() { let mut cmd = CommandBuilder::new("shell"); diff --git a/src/persist/restore.rs b/src/persist/restore.rs index 3c5775c9e1..a84100098e 100644 --- a/src/persist/restore.rs +++ b/src/persist/restore.rs @@ -517,15 +517,36 @@ fn restore_tab( let public_pane_id = old_pane_id .and_then(|old_id| public_pane_ids_by_old_raw.get(&old_id)) .map(String::as_str); + // The pane comes back from a shell that never activated anything, so + // re-enter the interpreter environment it was working in before. + // + // An environment that has since been removed is dropped rather than + // re-entered. Pointing PATH at a missing directory while also telling + // conda not to fall back to base would leave the pane with no working + // interpreter, which is worse than restoring without one. + let restored_virtual_env = saved_pane + .and_then(|pane| pane.virtual_env.as_ref()) + .and_then(|snapshot| snapshot.to_activation()) + .filter(|activation| { + let exists = activation.prefix.is_dir(); + if !exists { + warn!( + prefix = %activation.prefix.display(), + "saved pane environment is gone; restoring without it" + ); + } + exists + }); let launch_env = public_pane_id .map(|pane_id| { - PaneLaunchEnv::from_extra(Vec::new()).with_identity( + PaneLaunchEnv::default().with_identity( workspace_id.to_string(), crate::workspace::public_tab_id_for_number(workspace_id, number), pane_id.to_string(), ) }) - .unwrap_or_default(); + .unwrap_or_default() + .with_virtual_env(restored_virtual_env.clone()); let imported_runtime = old_pane_id.and_then(|old_id| imported_panes.remove(&old_id)); let was_imported = imported_runtime.is_some(); let pending_native_agent_restore = if was_imported { @@ -537,6 +558,7 @@ fn restore_tab( let terminal_id = TerminalId::alloc(); let mut terminal = TerminalState::new(terminal_id.clone(), cwd.clone()) .with_pending_agent_resume_plan(plan); + terminal.virtual_env = restored_virtual_env; if let Some(label) = saved_label { terminal.set_manual_label(label); } @@ -629,6 +651,7 @@ fn restore_tab( Ok(runtime) => { let terminal_id = TerminalId::alloc(); let mut terminal = TerminalState::new(terminal_id.clone(), cwd.clone()); + terminal.virtual_env = restored_virtual_env; if was_imported { if let Some(argv) = saved_launch_argv { terminal = terminal.with_launch_argv(argv).with_respawn_shell_on_exit(); @@ -1198,6 +1221,7 @@ mod tests { value: "opencode-session".into(), }), launch_argv: None, + virtual_env: None, }, )]), zoomed: false, @@ -1279,6 +1303,7 @@ mod tests { managed_agent_kind: None, agent_session: None, launch_argv: None, + virtual_env: None, }, ), ( @@ -1290,6 +1315,7 @@ mod tests { managed_agent_kind: None, agent_session: None, launch_argv: None, + virtual_env: None, }, ), ]), @@ -1343,6 +1369,7 @@ mod tests { managed_agent_kind: None, agent_session: None, launch_argv: None, + virtual_env: None, }, ) }; @@ -1358,6 +1385,7 @@ mod tests { value: "codex-session".into(), }), launch_argv: None, + virtual_env: None, }; let snapshot = SessionSnapshot { version: super::super::snapshot::SNAPSHOT_VERSION, @@ -1441,6 +1469,98 @@ mod tests { .all(|detail| detail.pane_id != agent_pane)); } + fn restore_pane_with_virtual_env( + prefix: &std::path::Path, + ) -> Option { + let cwd = std::env::current_dir().unwrap(); + let snapshot = SessionSnapshot { + version: super::super::snapshot::SNAPSHOT_VERSION, + workspaces: vec![WorkspaceSnapshot { + id: Some("w1".into()), + custom_name: None, + identity_cwd: cwd.clone(), + worktree_space: None, + public_pane_numbers: HashMap::from([(10, 1)]), + next_public_pane_number: 2, + public_tab_numbers: vec![1], + next_public_tab_number: 2, + tabs: vec![TabSnapshot { + custom_name: None, + layout: LayoutSnapshot::Pane(10), + panes: HashMap::from([( + 10, + super::super::snapshot::PaneSnapshot { + cwd: cwd.clone(), + label: None, + agent_name: None, + managed_agent_kind: None, + agent_session: None, + launch_argv: None, + virtual_env: Some(super::super::snapshot::PaneVirtualEnvSnapshot { + kind: "conda".into(), + prefix: prefix.to_path_buf(), + name: Some("web".into()), + }), + }, + )]), + zoomed: false, + focused: Some(10), + root_pane: Some(10), + }], + active_tab: 0, + }], + active: Some(0), + selected: 0, + sidebar_width: None, + sidebar_section_split: None, + collapsed_space_keys: Default::default(), + }; + let (events, _event_rx) = mpsc::channel(4); + + let (workspaces, terminals, _runtimes) = restore( + &snapshot, + None, + 24, + 80, + 0, + test_restore_shell(), + crate::config::ShellModeConfig::NonLogin, + false, + events, + Arc::new(Notify::new()), + Arc::new(RenderSignal::new()), + ); + + let workspace = workspaces.first().expect("workspace should restore"); + let pane = workspace.tabs[0].root_pane; + let terminal_id = &workspace.tabs[0].panes[&pane].attached_terminal_id; + terminals[terminal_id].virtual_env.clone() + } + + #[tokio::test] + async fn cold_restore_carries_the_saved_virtual_env_onto_the_terminal() { + let prefix = std::env::current_dir().unwrap(); + + let activation = restore_pane_with_virtual_env(&prefix) + .expect("restored pane should carry its environment"); + + assert_eq!(activation.kind, crate::platform::VirtualEnvKind::Conda); + assert_eq!(activation.prefix, prefix); + assert_eq!(activation.name.as_deref(), Some("web")); + } + + #[tokio::test] + async fn cold_restore_drops_a_virtual_env_that_no_longer_exists() { + // Re-entering a deleted environment would put a missing directory on + // PATH and suppress conda's fallback to base, leaving the pane with no + // interpreter at all. + let prefix = std::env::current_dir() + .unwrap() + .join("herdr-environment-that-does-not-exist"); + + assert!(restore_pane_with_virtual_env(&prefix).is_none()); + } + #[test] fn legacy_restore_precomputes_missing_public_pane_numbers() { let cwd = std::env::current_dir().unwrap(); @@ -1509,6 +1629,7 @@ mod tests { value: "codex-session".into(), }), launch_argv: None, + virtual_env: None, }, )]), zoomed: false, @@ -1670,6 +1791,7 @@ mod tests { managed_agent_kind: None, agent_session: None, launch_argv: None, + virtual_env: None, }, ); let history = SessionHistorySnapshot { diff --git a/src/persist/snapshot.rs b/src/persist/snapshot.rs index 39f790ddda..3690581438 100644 --- a/src/persist/snapshot.rs +++ b/src/persist/snapshot.rs @@ -107,6 +107,40 @@ pub struct PaneSnapshot { pub agent_session: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub launch_argv: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub virtual_env: Option, +} + +/// The interpreter environment a pane was working in. +/// +/// Only the prefix and its name are stored. Rebuilding `PATH` from the prefix +/// on restore keeps a restored pane on the current `PATH` instead of pinning it +/// to whatever the machine looked like when the snapshot was written. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct PaneVirtualEnvSnapshot { + /// `conda` or `venv`. + pub kind: String, + pub prefix: PathBuf, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub name: Option, +} + +impl PaneVirtualEnvSnapshot { + fn from_activation(activation: &crate::platform::VirtualEnvActivation) -> Self { + Self { + kind: activation.kind.as_str().to_string(), + prefix: activation.prefix.clone(), + name: activation.name.clone(), + } + } + + pub(crate) fn to_activation(&self) -> Option { + Some(crate::platform::VirtualEnvActivation { + kind: crate::platform::VirtualEnvKind::from_str(&self.kind)?, + prefix: self.prefix.clone(), + name: self.name.clone(), + }) + } } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -338,6 +372,20 @@ fn capture_tab( }) .unwrap_or_default(); let launch_argv = terminal.and_then(|terminal| terminal.launch_argv.clone()); + // A live pane only reports an activation while it is running + // something, so an idle pane falls back to the one it was restored + // into. Without that, saving a restored session at an idle prompt + // would lose the environment on the next restore. + // + // That fallback is sticky: deactivating and then saving at an idle + // prompt brings the environment back on the next restore. A shell + // deactivates in its own process, and that is not observable from + // outside it, so there is nothing here to notice the change. + let virtual_env = tab + .virtual_env_for_pane(*id, terminal_runtimes) + .or_else(|| terminal.and_then(|terminal| terminal.virtual_env.clone())) + .as_ref() + .map(PaneVirtualEnvSnapshot::from_activation); let agent_session = terminal.and_then(|terminal| { if let Some(authority) = terminal.hook_authority.as_ref() { if let Some(session_ref) = authority.session_ref.as_ref() { @@ -368,6 +416,7 @@ fn capture_tab( managed_agent_kind, agent_session, launch_argv, + virtual_env, }, ); } @@ -648,6 +697,7 @@ mod tests { managed_agent_kind: None, agent_session: None, launch_argv: None, + virtual_env: None, }, ); panes.insert( @@ -659,6 +709,7 @@ mod tests { managed_agent_kind: None, agent_session: None, launch_argv: None, + virtual_env: None, }, ); @@ -718,6 +769,77 @@ mod tests { assert_eq!(restored.sidebar_section_split, Some(0.5)); } + #[test] + fn pane_virtual_env_survives_a_snapshot_round_trip() { + let pane = PaneSnapshot { + cwd: PathBuf::from("/home/can/Projects/herdr"), + label: None, + agent_name: None, + managed_agent_kind: None, + agent_session: None, + launch_argv: None, + virtual_env: Some(PaneVirtualEnvSnapshot { + kind: "conda".into(), + prefix: PathBuf::from("/opt/conda/envs/web"), + name: Some("web".into()), + }), + }; + + let json = serde_json::to_string(&pane).unwrap(); + let restored: PaneSnapshot = serde_json::from_str(&json).unwrap(); + + let activation = restored + .virtual_env + .as_ref() + .and_then(PaneVirtualEnvSnapshot::to_activation) + .expect("expected an activation"); + assert_eq!(activation.kind, crate::platform::VirtualEnvKind::Conda); + assert_eq!(activation.prefix, PathBuf::from("/opt/conda/envs/web")); + assert_eq!(activation.name.as_deref(), Some("web")); + } + + #[test] + fn pane_without_a_virtual_env_stays_absent_from_the_snapshot() { + let pane = PaneSnapshot { + cwd: PathBuf::from("/home/can/Projects/herdr"), + label: None, + agent_name: None, + managed_agent_kind: None, + agent_session: None, + launch_argv: None, + virtual_env: None, + }; + + let json = serde_json::to_value(&pane).unwrap(); + + assert!(json.get("virtual_env").is_none()); + } + + #[test] + fn snapshot_written_before_virtual_env_still_parses() { + let pane: PaneSnapshot = serde_json::from_value(serde_json::json!({ + "cwd": "/home/can/Projects/herdr", + "label": "api", + })) + .unwrap(); + + assert!(pane.virtual_env.is_none()); + } + + #[test] + fn unknown_virtual_env_kind_restores_as_no_activation() { + // A newer herdr could record a tool this build does not know how to + // re-enter; dropping it leaves the pane on the inherited environment + // instead of building a broken PATH from it. + let snapshot = PaneVirtualEnvSnapshot { + kind: "poetry".into(), + prefix: PathBuf::from("/opt/poetry/envs/web"), + name: None, + }; + + assert!(snapshot.to_activation().is_none()); + } + #[test] fn current_session_fixture_parses() { let snap = parse_snapshot(session_fixture("current-herdr")).unwrap(); @@ -1207,6 +1329,7 @@ mod tests { managed_agent_kind: None, agent_session: None, launch_argv: None, + virtual_env: None, }, ); panes.insert( @@ -1220,6 +1343,7 @@ mod tests { managed_agent_kind: None, agent_session: None, launch_argv: None, + virtual_env: None, }, ); diff --git a/src/platform/fallback.rs b/src/platform/fallback.rs index 64e5467b1d..0337fc29d4 100644 --- a/src/platform/fallback.rs +++ b/src/platform/fallback.rs @@ -188,6 +188,11 @@ pub fn process_cwd(_pid: u32) -> Option { None } +/// Unsupported platform stub. +pub fn process_virtual_env(_pid: u32) -> Option { + None +} + /// Unsupported platform stub. pub fn session_processes(_child_pid: u32) -> Vec { Vec::new() diff --git a/src/platform/linux.rs b/src/platform/linux.rs index f3ba0429c6..ab1619c246 100644 --- a/src/platform/linux.rs +++ b/src/platform/linux.rs @@ -47,6 +47,11 @@ pub(crate) fn should_query_host_terminal_palette() -> bool { } fn running_inside_wsl() -> bool { + static RUNNING_INSIDE_WSL: OnceLock = OnceLock::new(); + *RUNNING_INSIDE_WSL.get_or_init(detect_running_inside_wsl) +} + +fn detect_running_inside_wsl() -> bool { proc_file_indicates_wsl("/proc/sys/kernel/osrelease") || proc_file_indicates_wsl("/proc/version") || WSL_MARKER_ENV_VARS @@ -181,8 +186,8 @@ fn foreground_job_for_group(child_pid: u32, process_group_id: u32) -> Option Option { - let shell_group_id = process_pgrp_and_comm(child_pid) - .map(|(pgrp, _)| pgrp) + let shell_group_id = process_pgrp_comm_and_state(child_pid) + .map(|(pgrp, _, _)| pgrp) .filter(|pgrp| *pgrp > 0)? as u32; child_groups_foreground_process_group_with( @@ -190,7 +195,7 @@ fn child_groups_foreground_process_group(child_pid: u32) -> Option { shell_group_id, process_task_ids, process_task_children, - |pid| process_pgrp_and_comm(pid).map(|(pgrp, _)| pgrp), + |pid| process_pgrp_comm_and_state(pid).map(|(pgrp, _, _)| pgrp), ) } @@ -311,12 +316,12 @@ fn numeric_file_name(entry: &std::fs::DirEntry) -> Option { } fn live_process_group_member(process_group_id: u32, pid: u32) -> Option { - let (pgrp, comm) = process_pgrp_and_comm(pid)?; + let (pgrp, comm, _) = process_pgrp_comm_and_state(pid)?; (pgrp > 0 && pgrp as u32 == process_group_id).then_some(ProcGroupMember { pid, comm }) } pub fn foreground_group_leader_job(process_group_id: u32) -> Option { - let (pgrp, name) = process_pgrp_and_comm(process_group_id)?; + let (pgrp, name, _) = process_pgrp_comm_and_state(process_group_id)?; if pgrp as u32 != process_group_id { return None; } @@ -350,18 +355,34 @@ pub fn foreground_process_group_id_for_tty_fd(fd: RawFd) -> Option { (pgid > 0).then_some(pgid as u32) } -fn process_pgrp_and_comm(pid: u32) -> Option<(i32, String)> { +fn process_pgrp_comm_and_state(pid: u32) -> Option<(i32, String, char)> { let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")).ok()?; - process_pgrp_and_comm_from_stat(&stat) + process_pgrp_comm_and_state_from_stat(&stat) } -fn process_pgrp_and_comm_from_stat(stat: &str) -> Option<(i32, String)> { +fn process_pgrp_comm_and_state_from_stat(stat: &str) -> Option<(i32, String, char)> { let close = stat.rfind(')')?; let comm = stat.get(1 + stat.find('(')?..close)?.to_string(); let rest = stat.get(close + 2..)?; let fields: Vec<&str> = rest.split_whitespace().collect(); + let state = fields.first()?.chars().next()?; let pgrp: i32 = fields.get(2)?.parse().ok()?; - Some((pgrp, comm)) + Some((pgrp, comm, state)) +} + +/// Whether `/proc//environ` can be read without risking a stall. +/// +/// That read enters `access_remote_vm`. A process that is exiting or in +/// uninterruptible sleep can hold it there, and on WSL it has been seen to +/// block indefinitely while a multithreaded agent exits. The environment is +/// optional, so it is skipped in both cases rather than blocking the caller. +fn process_state_allows_remote_memory_read(state: char) -> bool { + !matches!(state, 'D' | 'Z' | 'X' | 'x') +} + +fn process_allows_remote_memory_read(state: char, comm: &str, running_inside_wsl: bool) -> bool { + process_state_allows_remote_memory_read(state) + && (!running_inside_wsl || crate::detect::identify_agent(comm).is_none()) } fn process_argv(pid: u32) -> Option> { @@ -395,6 +416,22 @@ pub fn process_agent_hint(pid: u32) -> Option { super::parse_agent_env_hint(&environ) } +/// Read the interpreter environment a process was started in. +/// +/// A pane whose foreground process cannot be read safely reports no +/// environment; the pane is still restored, just without one. +pub fn process_virtual_env(pid: u32) -> Option { + if pid == 0 { + return None; + } + let (_, comm, state) = process_pgrp_comm_and_state(pid)?; + if !process_allows_remote_memory_read(state, &comm, running_inside_wsl()) { + return None; + } + let environ = std::fs::read(format!("/proc/{pid}/environ")).ok()?; + super::parse_virtual_env_activation(&environ) +} + pub fn session_processes(child_pid: u32) -> Vec { let Some(session_id) = process_session_id(child_pid) else { return Vec::new(); @@ -1051,11 +1088,37 @@ mod tests { #[test] fn proc_stat_parsing_keeps_group_leader_inputs_live() { assert_eq!( - process_pgrp_and_comm_from_stat("123 (name with ) paren) S 1 456 789 0 456"), - Some((456, "name with ) paren".to_string())) + process_pgrp_comm_and_state_from_stat("123 (name with ) paren) S 1 456 789 0 456"), + Some((456, "name with ) paren".to_string(), 'S')) ); } + #[test] + fn exiting_and_uninterruptible_processes_are_not_read_from_remote_memory() { + for state in ['D', 'Z', 'X', 'x'] { + assert!( + !process_allows_remote_memory_read(state, "python", false), + "{state}" + ); + } + for state in ['R', 'S', 'T', 'I'] { + assert!( + process_allows_remote_memory_read(state, "python", false), + "{state}" + ); + } + } + + #[test] + fn wsl_skips_remote_memory_reads_for_named_agents_but_not_for_wrappers() { + // The stall was seen while an identified agent was exiting, and a state + // check alone races with it, so on WSL the name is enough to skip. + assert!(!process_allows_remote_memory_read('S', "codex", true)); + assert!(process_allows_remote_memory_read('S', "codex", false)); + assert!(process_allows_remote_memory_read('S', "node", true)); + assert!(process_allows_remote_memory_read('S', "python", true)); + } + #[test] fn clipboard_commands_prefer_wayland_when_available() { let _guard = env_lock().lock().unwrap(); diff --git a/src/platform/macos.rs b/src/platform/macos.rs index 948468acbe..16bc88f4d5 100644 --- a/src/platform/macos.rs +++ b/src/platform/macos.rs @@ -804,6 +804,19 @@ pub fn process_agent_hint(pid: u32) -> Option { super::parse_agent_env_hint(procargs2_env(&buf)?) } +/// Read the interpreter environment a process was started in. +/// +/// The kernel withholds the environment block for platform binaries, so this +/// returns `None` for the pane's own `/bin/zsh`. Agents run from user-installed +/// binaries, which do report it. +pub fn process_virtual_env(pid: u32) -> Option { + if pid == 0 { + return None; + } + let buf = kern_procargs2(pid)?; + super::parse_virtual_env_activation(procargs2_env(&buf)?) +} + fn procargs2_argv_start(rest: &[u8]) -> Option { let exec_end = rest.iter().position(|&byte| byte == 0)?; let mut pos = exec_end; diff --git a/src/platform/mod.rs b/src/platform/mod.rs index a63b75654d..47721b332e 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -327,6 +327,177 @@ pub(crate) fn parse_agent_env_hint(environ: &[u8]) -> Option &'static str { + match self { + Self::Conda => "conda", + Self::Venv => "venv", + } + } + + pub(crate) fn from_str(value: &str) -> Option { + match value { + "conda" => Some(Self::Conda), + "venv" => Some(Self::Venv), + _ => None, + } + } +} + +/// An activated interpreter environment observed on a live process. +/// +/// `conda activate` and the `venv`/`virtualenv` activate scripts both work the +/// same way: export a prefix variable and put that prefix's binary directory +/// first on `PATH`. The prefix is therefore the whole activation — everything +/// else is derived from it, which is why only the prefix and its display name +/// are recorded here instead of a snapshot of the process `PATH`. A `PATH` +/// captured on one run goes stale as soon as the user installs, removes, or +/// upgrades anything outside the environment. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct VirtualEnvActivation { + pub kind: VirtualEnvKind, + pub prefix: std::path::PathBuf, + /// `CONDA_DEFAULT_ENV`, or the prompt label a venv was activated with. + pub name: Option, +} + +impl VirtualEnvActivation { + /// Directories the activation puts in front of `PATH`, nearest first. + /// + /// Conda spreads a Windows environment over several directories and its + /// activate script adds all of them, so a single `Scripts` entry is not + /// enough to make the environment usable there. + pub(crate) fn path_entries(&self) -> Vec { + self.path_entries_on(cfg!(windows)) + } + + /// The layout the host uses is taken as an argument so both layouts stay + /// covered by tests wherever they run. + fn path_entries_on(&self, windows: bool) -> Vec { + let prefix = &self.prefix; + match (self.kind, windows) { + (VirtualEnvKind::Conda, true) => vec![ + prefix.clone(), + prefix.join("Library").join("mingw-w64").join("bin"), + prefix.join("Library").join("usr").join("bin"), + prefix.join("Library").join("bin"), + prefix.join("Scripts"), + prefix.join("bin"), + ], + (VirtualEnvKind::Venv, true) => vec![prefix.join("Scripts")], + (_, false) => vec![prefix.join("bin")], + } + } + + /// Environment overrides that re-enter this environment in a fresh process. + /// + /// `base_path` is the `PATH` the new process would otherwise inherit. The + /// activation entries are prepended to it, skipping any that survived, so + /// repeated restores cannot grow `PATH` without bound. + /// + /// `CONDA_SHLVL` is pinned to 1 rather than restored from the observed + /// process: nesting depth belongs to the shell that stacked the + /// activations, and a restored pane starts from an unactivated shell. + /// + /// Conda's automatic base activation is turned off for the restored shell. + /// A pane comes back as an interactive login shell, so it re-runs the + /// user's rc files, and conda's init hook activates base by default. That + /// runs after this environment is handed in and replaces it — it shadows a + /// restored venv's interpreter too, because base lands ahead of the venv on + /// `PATH`. Both spellings are set because conda renamed the setting in 25.x + /// and older releases only understand the previous one. Panes without a + /// recorded environment are untouched and still auto-activate base. + pub(crate) fn launch_env( + &self, + base_path: Option<&std::ffi::OsStr>, + ) -> Vec<(&'static str, std::ffi::OsString)> { + let prefix = std::ffi::OsString::from(&self.prefix); + let name = self.name.clone().map(std::ffi::OsString::from); + let mut env = match self.kind { + VirtualEnvKind::Conda => { + let mut vars = vec![("CONDA_PREFIX", prefix)]; + if let Some(name) = name { + vars.push(("CONDA_DEFAULT_ENV", name)); + } + vars.push(("CONDA_SHLVL", "1".into())); + vars + } + VirtualEnvKind::Venv => { + let mut vars = vec![("VIRTUAL_ENV", prefix)]; + if let Some(name) = name { + vars.push(("VIRTUAL_ENV_PROMPT", name)); + } + vars + } + }; + + env.push(("CONDA_AUTO_ACTIVATE", "false".into())); + env.push(("CONDA_AUTO_ACTIVATE_BASE", "false".into())); + + let entries = self.path_entries(); + let inherited = base_path.unwrap_or_else(|| std::ffi::OsStr::new("")); + let kept = std::env::split_paths(inherited) + .filter(|dir| !entries.iter().any(|entry| entry == dir)) + .collect::>(); + if let Ok(path) = std::env::join_paths(entries.into_iter().chain(kept)) { + env.push(("PATH", path)); + } + env + } +} + +/// Read an activation out of a NUL-separated environment block. +/// +/// A venv nested inside a conda environment leaves both prefixes exported, and +/// `VIRTUAL_ENV` is the inner one, so it wins when both are present. +#[cfg(any(target_os = "linux", target_os = "macos"))] +pub(crate) fn parse_virtual_env_activation(environ: &[u8]) -> Option { + let mut conda_prefix = None; + let mut conda_name = None; + let mut venv_prefix = None; + let mut venv_name = None; + + for record in environ.split(|&byte| byte == 0) { + let Some(index) = record.iter().position(|&byte| byte == b'=') else { + continue; + }; + let (key, value) = record.split_at(index); + let Ok(value) = std::str::from_utf8(&value[1..]) else { + continue; + }; + if value.is_empty() { + continue; + } + match key { + b"CONDA_PREFIX" => conda_prefix = Some(value.to_string()), + b"CONDA_DEFAULT_ENV" => conda_name = Some(value.to_string()), + b"VIRTUAL_ENV" => venv_prefix = Some(value.to_string()), + b"VIRTUAL_ENV_PROMPT" => venv_name = Some(value.to_string()), + _ => {} + } + } + + if let Some(prefix) = venv_prefix { + return Some(VirtualEnvActivation { + kind: VirtualEnvKind::Venv, + prefix: prefix.into(), + name: venv_name, + }); + } + conda_prefix.map(|prefix| VirtualEnvActivation { + kind: VirtualEnvKind::Conda, + prefix: prefix.into(), + name: conda_name, + }) +} + #[cfg(not(any(target_os = "macos", target_os = "windows")))] #[derive(Debug)] pub(crate) struct InputSourceRestore; @@ -380,6 +551,208 @@ impl PrefixInputSource for RealPrefixInputSource { mod tests { use super::*; + #[cfg(any(target_os = "linux", target_os = "macos"))] + fn environ(records: &[&str]) -> Vec { + let mut block = Vec::new(); + for record in records { + block.extend_from_slice(record.as_bytes()); + block.push(0); + } + block + } + + fn path_value(env: &[(&str, std::ffi::OsString)]) -> Vec { + let path = env + .iter() + .find(|(key, _)| *key == "PATH") + .map(|(_, value)| value.clone()) + .expect("expected PATH"); + std::env::split_paths(&path).collect() + } + + fn launch_env_of( + activation: &VirtualEnvActivation, + base_path: Option<&str>, + ) -> Vec<(&'static str, std::ffi::OsString)> { + activation.launch_env(base_path.map(std::ffi::OsStr::new)) + } + + #[cfg(any(target_os = "linux", target_os = "macos"))] + #[test] + fn parse_virtual_env_activation_reads_conda_prefix_and_name() { + let activation = parse_virtual_env_activation(&environ(&[ + "PATH=/opt/conda/envs/web/bin:/usr/bin", + "CONDA_PREFIX=/opt/conda/envs/web", + "CONDA_DEFAULT_ENV=web", + "TERM=xterm-256color", + ])) + .expect("expected an activation"); + + assert_eq!(activation.kind, VirtualEnvKind::Conda); + assert_eq!( + activation.prefix, + std::path::PathBuf::from("/opt/conda/envs/web") + ); + assert_eq!(activation.name.as_deref(), Some("web")); + } + + #[cfg(any(target_os = "linux", target_os = "macos"))] + #[test] + fn parse_virtual_env_activation_prefers_venv_nested_in_conda() { + let activation = parse_virtual_env_activation(&environ(&[ + "CONDA_PREFIX=/opt/conda", + "CONDA_DEFAULT_ENV=base", + "VIRTUAL_ENV=/work/api/.venv", + "VIRTUAL_ENV_PROMPT=api", + ])) + .expect("expected an activation"); + + assert_eq!(activation.kind, VirtualEnvKind::Venv); + assert_eq!( + activation.prefix, + std::path::PathBuf::from("/work/api/.venv") + ); + assert_eq!(activation.name.as_deref(), Some("api")); + } + + #[cfg(any(target_os = "linux", target_os = "macos"))] + #[test] + fn parse_virtual_env_activation_ignores_unactivated_and_empty_environments() { + assert!(parse_virtual_env_activation(&environ(&["PATH=/usr/bin", "TERM=xterm"])).is_none()); + // conda exports an empty CONDA_PREFIX after the last `conda deactivate`. + assert!( + parse_virtual_env_activation(&environ(&["CONDA_PREFIX=", "VIRTUAL_ENV="])).is_none() + ); + } + + #[test] + fn launch_env_puts_the_environment_first_on_the_inherited_path() { + let activation = VirtualEnvActivation { + kind: VirtualEnvKind::Venv, + prefix: "/work/api/.venv".into(), + name: None, + }; + + let env = launch_env_of(&activation, Some("/usr/local/bin:/usr/bin")); + + assert!(env.contains(&("VIRTUAL_ENV", "/work/api/.venv".into()))); + assert_eq!( + path_value(&env), + [ + std::path::PathBuf::from("/work/api/.venv/bin"), + std::path::PathBuf::from("/usr/local/bin"), + std::path::PathBuf::from("/usr/bin"), + ] + ); + } + + #[test] + fn launch_env_does_not_repeat_entries_already_on_the_inherited_path() { + let activation = VirtualEnvActivation { + kind: VirtualEnvKind::Conda, + prefix: "/opt/conda/envs/web".into(), + name: Some("web".to_string()), + }; + + let env = launch_env_of(&activation, Some("/opt/conda/envs/web/bin:/usr/bin")); + + assert_eq!( + path_value(&env), + [ + std::path::PathBuf::from("/opt/conda/envs/web/bin"), + std::path::PathBuf::from("/usr/bin"), + ] + ); + } + + #[test] + fn launch_env_stops_conda_from_auto_activating_base_over_the_restored_env() { + // The restored shell re-runs the user's rc files, and conda's init hook + // activates base by default. Without this the pane comes back on base + // no matter what was handed in — including for a venv, which base + // shadows on PATH. + for kind in [VirtualEnvKind::Conda, VirtualEnvKind::Venv] { + let activation = VirtualEnvActivation { + kind, + prefix: "/work/api/.venv".into(), + name: None, + }; + + let env = launch_env_of(&activation, Some("/usr/bin")); + + assert!(env.contains(&("CONDA_AUTO_ACTIVATE", "false".into()))); + assert!(env.contains(&("CONDA_AUTO_ACTIVATE_BASE", "false".into()))); + } + } + + #[test] + fn launch_env_pins_conda_nesting_depth_to_a_single_activation() { + let activation = VirtualEnvActivation { + kind: VirtualEnvKind::Conda, + prefix: "/opt/conda/envs/web".into(), + name: Some("web".to_string()), + }; + + let env = launch_env_of(&activation, None); + + assert!(env.contains(&("CONDA_SHLVL", "1".into()))); + assert!(env.contains(&("CONDA_DEFAULT_ENV", "web".into()))); + } + + // `path_entries_on` is exercised for both layouts here rather than only for + // the host's, so the Windows directory list stays covered off Windows. + // Expectations are built with `join` because the separator follows the + // host, while the directories under test do not. + #[test] + fn windows_conda_activation_covers_every_directory_the_activate_script_adds() { + let prefix = std::path::PathBuf::from(r"C:\conda\envs\web"); + let activation = VirtualEnvActivation { + kind: VirtualEnvKind::Conda, + prefix: prefix.clone(), + name: Some("web".to_string()), + }; + + assert_eq!( + activation.path_entries_on(true), + [ + prefix.clone(), + prefix.join("Library").join("mingw-w64").join("bin"), + prefix.join("Library").join("usr").join("bin"), + prefix.join("Library").join("bin"), + prefix.join("Scripts"), + prefix.join("bin"), + ] + ); + } + + #[test] + fn windows_venv_activation_uses_the_scripts_directory() { + let prefix = std::path::PathBuf::from(r"C:\work\api\.venv"); + let activation = VirtualEnvActivation { + kind: VirtualEnvKind::Venv, + prefix: prefix.clone(), + name: None, + }; + + assert_eq!(activation.path_entries_on(true), [prefix.join("Scripts")]); + } + + #[test] + fn unix_activation_uses_the_bin_directory_for_both_kinds() { + for kind in [VirtualEnvKind::Conda, VirtualEnvKind::Venv] { + let activation = VirtualEnvActivation { + kind, + prefix: "/opt/conda/envs/web".into(), + name: None, + }; + + assert_eq!( + activation.path_entries_on(false), + [std::path::PathBuf::from("/opt/conda/envs/web/bin")] + ); + } + } + #[test] fn terminal_resize_signal_is_recorded_once_per_delivery() { watch_terminal_resize_signal(); diff --git a/src/platform/windows.rs b/src/platform/windows.rs index 5d5b014189..234ce8af78 100644 --- a/src/platform/windows.rs +++ b/src/platform/windows.rs @@ -1710,6 +1710,38 @@ fn read_process_environment(process: HANDLE, address: *const c_void) -> Option Option { + if pid == 0 { + return None; + } + let process = ProcessHandle::open(pid, PROCESS_QUERY_INFORMATION | PROCESS_VM_READ)?; + let parameters = read_process_parameters(process.0)?; + let environment = read_process_environment(process.0, parameters.environment)?; + virtual_env_from_utf16(&environment) +} + +/// A venv nested inside a conda environment leaves both prefixes exported, and +/// `VIRTUAL_ENV` is the inner one, so it wins when both are present. +fn virtual_env_from_utf16(environment: &[u16]) -> Option { + let read = |name: &str| { + environment_variable_from_utf16(environment, name).filter(|value| !value.is_empty()) + }; + + if let Some(prefix) = read("VIRTUAL_ENV") { + return Some(super::VirtualEnvActivation { + kind: super::VirtualEnvKind::Venv, + prefix: prefix.into(), + name: read("VIRTUAL_ENV_PROMPT"), + }); + } + read("CONDA_PREFIX").map(|prefix| super::VirtualEnvActivation { + kind: super::VirtualEnvKind::Conda, + prefix: prefix.into(), + name: read("CONDA_DEFAULT_ENV"), + }) +} + fn environment_variable_from_utf16(environment: &[u16], name: &str) -> Option { for variable in environment.split(|unit| *unit == 0) { if variable.is_empty() { @@ -3824,6 +3856,61 @@ mod tests { ); } + fn environment_block(records: &[&str]) -> Vec { + let mut block = Vec::new(); + for record in records { + block.extend(record.encode_utf16()); + block.push(0); + } + block.push(0); + block + } + + #[test] + fn windows_virtual_env_parser_reads_a_conda_prefix_and_name() { + let activation = super::virtual_env_from_utf16(&environment_block(&[ + "PATH=C:\\conda\\envs\\web\\Scripts;C:\\Windows", + "CONDA_PREFIX=C:\\conda\\envs\\web", + "CONDA_DEFAULT_ENV=web", + ])) + .expect("expected an activation"); + + assert_eq!(activation.kind, crate::platform::VirtualEnvKind::Conda); + assert_eq!( + activation.prefix, + std::path::PathBuf::from("C:\\conda\\envs\\web") + ); + assert_eq!(activation.name.as_deref(), Some("web")); + } + + #[test] + fn windows_virtual_env_parser_prefers_the_venv_nested_inside_a_conda_environment() { + let activation = super::virtual_env_from_utf16(&environment_block(&[ + "CONDA_PREFIX=C:\\conda\\envs\\web", + "CONDA_DEFAULT_ENV=web", + "VIRTUAL_ENV=C:\\work\\api\\.venv", + "VIRTUAL_ENV_PROMPT=api", + ])) + .expect("expected an activation"); + + assert_eq!(activation.kind, crate::platform::VirtualEnvKind::Venv); + assert_eq!( + activation.prefix, + std::path::PathBuf::from("C:\\work\\api\\.venv") + ); + assert_eq!(activation.name.as_deref(), Some("api")); + } + + #[test] + fn windows_virtual_env_parser_ignores_empty_and_absent_prefixes() { + assert!(super::virtual_env_from_utf16(&environment_block(&["PATH=C:\\Windows"])).is_none()); + assert!(super::virtual_env_from_utf16(&environment_block(&[ + "CONDA_PREFIX=", + "VIRTUAL_ENV=", + ])) + .is_none()); + } + #[test] fn pane_runtime_markers_are_distinct() { let first = super::next_pane_runtime_marker(); diff --git a/src/terminal/runtime.rs b/src/terminal/runtime.rs index 5051a46b3c..f8d1eef543 100644 --- a/src/terminal/runtime.rs +++ b/src/terminal/runtime.rs @@ -559,6 +559,10 @@ impl TerminalRuntime { self.0.foreground_cwd() } + pub fn foreground_virtual_env(&self) -> Option { + self.0.foreground_virtual_env() + } + pub fn child_pid(&self) -> Option { self.0.child_pid() } diff --git a/src/terminal/state.rs b/src/terminal/state.rs index 5c659eb9f4..84d693a844 100644 --- a/src/terminal/state.rs +++ b/src/terminal/state.rs @@ -128,6 +128,13 @@ pub struct TerminalState { pub agent_metadata: HashMap, pub metadata_tokens: crate::metadata_tokens::MetadataTokens, pub persisted_agent_session: Option, + /// Interpreter environment this pane was restored into. + /// + /// A live pane only reports one while it is running a command, so the + /// restored value is kept here for the moments it cannot be observed: a + /// deferred agent resume needs it after the pane is already back, and a + /// save taken at an idle prompt would otherwise drop it. + pub virtual_env: Option, pub terminal_title: Option, pub manual_label: Option, pub agent_name: Option, @@ -162,6 +169,7 @@ impl TerminalState { agent_metadata: HashMap::new(), metadata_tokens: crate::metadata_tokens::MetadataTokens::default(), persisted_agent_session: None, + virtual_env: None, terminal_title: None, manual_label: None, agent_name: None, diff --git a/src/workspace/tab.rs b/src/workspace/tab.rs index 6f0fa75e17..aafade6fbb 100644 --- a/src/workspace/tab.rs +++ b/src/workspace/tab.rs @@ -586,4 +586,15 @@ impl Tab { .get(terminal_id) .and_then(|rt| rt.foreground_cwd()) } + + pub fn virtual_env_for_pane( + &self, + pane_id: PaneId, + terminal_runtimes: &TerminalRuntimeRegistry, + ) -> Option { + let terminal_id = self.terminal_id(pane_id)?; + terminal_runtimes + .get(terminal_id) + .and_then(|rt| rt.foreground_virtual_env()) + } }