diff --git a/crates/pet/src/jsonrpc.rs b/crates/pet/src/jsonrpc.rs index 3b056a21..83318c1c 100644 --- a/crates/pet/src/jsonrpc.rs +++ b/crates/pet/src/jsonrpc.rs @@ -204,57 +204,27 @@ pub fn handle_refresh(context: Arc, id: u32, params: Value) { // Ensure we can have only one refresh at a time. let lock = REFRESH_LOCK.lock().expect("REFRESH_LOCK mutex poisoned"); - let mut config = context.configuration.read().unwrap().clone(); + let config = context.configuration.read().unwrap().clone(); let reporter = Arc::new(CacheReporter::new(Arc::new(jsonrpc::create_reporter( refresh_options.search_kind, )))); - let mut search_scope = None; - - // If search kind is provided and no search_paths, then we will only search in the global locations. - if refresh_options.search_kind.is_some() || refresh_options.search_paths.is_some() { - // Always clear this, as we will either serach in specified folder or a specific kind in global locations. - config.workspace_directories = None; - if let Some(search_paths) = refresh_options.search_paths { - // Expand any glob patterns in the search paths - let expanded_paths = expand_glob_patterns(&search_paths); - trace!( - "Expanded {} search paths to {} paths", - search_paths.len(), - expanded_paths.len() - ); - // These workspace folders are only for this refresh. - config.workspace_directories = Some( - expanded_paths - .iter() - .filter(|p| p.is_dir()) - .cloned() - .collect(), - ); - config.executables = Some( - expanded_paths - .iter() - .filter(|p| p.is_file()) - .cloned() - .collect(), - ); - search_scope = Some(SearchScope::Workspace); - } else if let Some(search_kind) = refresh_options.search_kind { - config.executables = None; - search_scope = Some(SearchScope::Global(search_kind)); - } + let (config, search_scope) = build_refresh_config(&refresh_options, config); + if refresh_options.search_paths.is_some() { + trace!( + "Expanded search paths to {} workspace dirs, {} executables", + config + .workspace_directories + .as_ref() + .map(|v| v.len()) + .unwrap_or(0), + config.executables.as_ref().map(|v| v.len()).unwrap_or(0) + ); + } - // Configure the locators with the modified config. - for locator in context.locators.iter() { - locator.configure(&config); - } - } else { - // Re-configure the locators with an un-modified config. - // Possible we congirued the locators with a modified config in the in the previous request. - // & the config was scoped to a particular search folder, executables or kind. - for locator in context.locators.iter() { - locator.configure(&config); - } + // Configure the locators with the (possibly modified) config. + for locator in context.locators.iter() { + locator.configure(&config); } trace!("Start refreshing environments, config: {:?}", config); @@ -497,3 +467,155 @@ pub fn handle_clear_cache(_context: Arc, id: u32, _params: Value) { } }); } + +/// Builds the configuration and search scope based on refresh options. +/// This is extracted from handle_refresh to enable unit testing. +/// +/// Returns (modified_config, search_scope) +pub(crate) fn build_refresh_config( + refresh_options: &RefreshOptions, + mut config: Configuration, +) -> (Configuration, Option) { + let mut search_scope = None; + + // If search_paths is provided, limit search to those paths. + // If only search_kind is provided (without search_paths), we still search + // workspace directories because many environment types (like Venv, VirtualEnv) + // don't have global locations - they only exist in workspace folders. + // The reporter will filter results to only report the requested kind. + if let Some(ref search_paths) = refresh_options.search_paths { + // Clear workspace directories when explicit search paths are provided. + config.workspace_directories = None; + // Expand any glob patterns in the search paths + let expanded_paths = expand_glob_patterns(search_paths); + // These workspace folders are only for this refresh. + config.workspace_directories = Some( + expanded_paths + .iter() + .filter(|p| p.is_dir()) + .cloned() + .collect(), + ); + config.executables = Some( + expanded_paths + .iter() + .filter(|p| p.is_file()) + .cloned() + .collect(), + ); + search_scope = Some(SearchScope::Workspace); + } else if let Some(search_kind) = refresh_options.search_kind { + // When only search_kind is provided, keep workspace directories so that + // workspace-based environments (Venv, VirtualEnv, etc.) can be found. + // The search_scope tells find_and_report_envs to filter locators by kind. + search_scope = Some(SearchScope::Global(search_kind)); + } + + (config, search_scope) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + /// Test for https://github.com/microsoft/python-environment-tools/issues/151 + /// Verifies that when searchKind is provided (without searchPaths), + /// workspace_directories are NOT cleared. + /// + /// The bug was that handle_refresh cleared workspace_directories when searchKind + /// was provided, preventing discovery of workspace-based environments like venvs. + #[test] + fn test_search_kind_preserves_workspace_directories() { + let workspace = PathBuf::from("/test/workspace"); + let config = Configuration { + workspace_directories: Some(vec![workspace.clone()]), + ..Default::default() + }; + + let refresh_options = RefreshOptions { + search_kind: Some(PythonEnvironmentKind::Venv), + search_paths: None, + }; + + let (result_config, search_scope) = build_refresh_config(&refresh_options, config); + + // CRITICAL: workspace_directories must be preserved when only search_kind is provided + assert_eq!( + result_config.workspace_directories, + Some(vec![workspace]), + "workspace_directories should NOT be cleared when only searchKind is provided" + ); + + // search_scope should be Global with the requested kind + assert!( + matches!( + search_scope, + Some(SearchScope::Global(PythonEnvironmentKind::Venv)) + ), + "search_scope should be Global(Venv)" + ); + } + + /// Test that when searchPaths is provided, workspace_directories ARE replaced. + #[test] + fn test_search_paths_replaces_workspace_directories() { + let temp_dir = tempfile::tempdir().unwrap(); + let search_dir = temp_dir.path().join("search_path"); + std::fs::create_dir(&search_dir).unwrap(); + + let original_workspace = PathBuf::from("/original/workspace"); + let config = Configuration { + workspace_directories: Some(vec![original_workspace]), + ..Default::default() + }; + + let refresh_options = RefreshOptions { + search_kind: None, + search_paths: Some(vec![search_dir.clone()]), + }; + + let (result_config, search_scope) = build_refresh_config(&refresh_options, config); + + // workspace_directories should be replaced with the search_paths directory + assert_eq!( + result_config.workspace_directories, + Some(vec![search_dir]), + "workspace_directories should be replaced by search_paths" + ); + + assert!( + matches!(search_scope, Some(SearchScope::Workspace)), + "search_scope should be Workspace" + ); + } + + /// Test that when neither searchKind nor searchPaths is provided, + /// configuration is unchanged. + #[test] + fn test_no_options_preserves_config() { + let workspace = PathBuf::from("/test/workspace"); + let config = Configuration { + workspace_directories: Some(vec![workspace.clone()]), + ..Default::default() + }; + + let refresh_options = RefreshOptions { + search_kind: None, + search_paths: None, + }; + + let (result_config, search_scope) = build_refresh_config(&refresh_options, config); + + assert_eq!( + result_config.workspace_directories, + Some(vec![workspace]), + "workspace_directories should be preserved when no options provided" + ); + + assert!( + search_scope.is_none(), + "search_scope should be None when no options provided" + ); + } +} diff --git a/docs/JSONRPC.md b/docs/JSONRPC.md index e4483e10..d4ed0e5f 100644 --- a/docs/JSONRPC.md +++ b/docs/JSONRPC.md @@ -97,14 +97,15 @@ _Response_: interface RefreshParams { /** * Limits the search to a specific kind of Python environment. - * Ignores workspace folders passed in configuration request. + * Workspace folders from the configuration request are still searched + * to discover workspace-based environments (e.g., venvs, virtualenvs). */ searchKind?: PythonEnvironmentKind; } | { /** * Limits the search to a specific set of paths. * searchPaths can either by directories or Python prefixes/executables or combination of both. - * Ignores workspace folders passed in configuration request. + * Replaces workspace folders from the configuration request. * * Glob patterns are supported: * - `*` matches any sequence of characters in a path component