From 6f2dfa7912ed78f13a5c38bccfe325c7e18bc499 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 29 Jul 2026 08:51:40 -0700 Subject: [PATCH 1/8] feat: improve recursive environment glob diagnostics (Fixes #427) Warn before expanding recursive environmentDirectories and report exact per-pattern timings for slow configure globs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/pet-fs/src/glob.rs | 11 ++++++ crates/pet/src/jsonrpc.rs | 79 ++++++++++++++++++++++++++------------- docs/JSONRPC.md | 4 +- 3 files changed, 67 insertions(+), 27 deletions(-) diff --git a/crates/pet-fs/src/glob.rs b/crates/pet-fs/src/glob.rs index e4b9de1f..1175822c 100644 --- a/crates/pet-fs/src/glob.rs +++ b/crates/pet-fs/src/glob.rs @@ -19,6 +19,10 @@ pub fn is_glob_pattern(path: &str) -> bool { path.contains(GLOB_METACHARACTERS) || has_brace_pattern(path) } +/// Returns true when a glob can traverse an unbounded number of path components. +pub fn is_recursive_glob_pattern(path: &str) -> bool { + path.contains("**") +} /// Checks if a string contains a valid brace expansion pattern `{a,b}`. /// Requires an opening `{`, at least one `,`, and a closing `}`. fn has_brace_pattern(path: &str) -> bool { @@ -202,6 +206,13 @@ mod tests { assert!(is_glob_pattern("*.txt")); } + #[test] + fn test_is_recursive_glob_pattern() { + assert!(is_recursive_glob_pattern("**/.venv")); + assert!(is_recursive_glob_pattern("/home/user/**/venv")); + assert!(!is_recursive_glob_pattern(".venv")); + assert!(!is_recursive_glob_pattern("*/.venv")); + } #[test] fn test_is_glob_pattern_with_question_mark() { assert!(is_glob_pattern("/home/user/file?.txt")); diff --git a/crates/pet/src/jsonrpc.rs b/crates/pet/src/jsonrpc.rs index ee93fa46..025bdda4 100644 --- a/crates/pet/src/jsonrpc.rs +++ b/crates/pet/src/jsonrpc.rs @@ -21,7 +21,7 @@ use pet_core::{ Configuration, Locator, RefreshStatePersistence, RefreshStateSyncScope, }; use pet_env_var_path::get_search_paths_from_env_variables; -use pet_fs::glob::expand_glob_patterns; +use pet_fs::glob::{expand_glob_pattern, expand_glob_patterns, is_recursive_glob_pattern}; use pet_fs::path::norm_case; use pet_jsonrpc::{ send_error, send_reply, @@ -566,6 +566,43 @@ pub struct ConfigureOptions { /// The client has a 30-second timeout for configure requests. const GLOB_EXPANSION_WARN_THRESHOLD: Duration = Duration::from_secs(5); +fn expand_configure_directory_patterns(kind: &str, patterns: Vec) -> Vec { + patterns + .into_iter() + .flat_map(|pattern| { + let start = Instant::now(); + let expanded = expand_glob_pattern(&pattern.to_string_lossy()); + let elapsed = start.elapsed(); + trace!( + "Expanded {} pattern '{}' in {:?}", + kind, + pattern.display(), + elapsed + ); + if elapsed >= GLOB_EXPANSION_WARN_THRESHOLD { + warn!( + "Expanding {} pattern '{}' took {:?}, this may cause client timeouts", + kind, + pattern.display(), + elapsed + ); + } + expanded + }) + .filter(|path| path.is_dir()) + .collect() +} + +fn warn_for_recursive_environment_patterns(patterns: &[PathBuf]) { + for pattern in patterns { + if is_recursive_glob_pattern(&pattern.to_string_lossy()) { + warn!( + "Recursive environmentDirectories pattern '{}' can make configure slow; prefer bounded patterns such as '.venv' or '*/.venv'", + pattern.display() + ); + } + } +} pub fn handle_configure(context: Arc, id: u32, params: Value) { match serde_json::from_value::(params.clone()) { Ok(mut configure_options) => { @@ -574,38 +611,28 @@ pub fn handle_configure(context: Arc, id: u32, params: Value) { thread::spawn(move || { let now = Instant::now(); + // Warn before any expansion so a slow workspace pattern cannot delay + // the actionable environmentDirectories diagnostic. + if let Some(patterns) = configure_options.environment_directories.as_deref() { + warn_for_recursive_environment_patterns(patterns); + } + // Expand glob patterns before acquiring the write lock so we // don't block readers/writers while traversing the filesystem. let workspace_directories = - configure_options.workspace_directories.take().map(|dirs| { - let start = Instant::now(); - let result: Vec = expand_glob_patterns(&dirs) - .into_iter() - .filter(|p| p.is_dir()) - .collect(); - trace!( - "Expanded workspace directory patterns ({:?}) in {:?}", - dirs, - start.elapsed() - ); - result - }); + configure_options + .workspace_directories + .take() + .map(|patterns| { + expand_configure_directory_patterns("workspaceDirectories", patterns) + }); let environment_directories = configure_options .environment_directories .take() - .map(|dirs| { - let start = Instant::now(); - let result: Vec = expand_glob_patterns(&dirs) - .into_iter() - .filter(|p| p.is_dir()) - .collect(); - trace!( - "Expanded environment directory patterns ({:?}) in {:?}", - dirs, - start.elapsed() - ); - result + .map(|patterns| { + warn_for_recursive_environment_patterns(&patterns); + expand_configure_directory_patterns("environmentDirectories", patterns) }); let glob_elapsed = now.elapsed(); trace!("Glob expansion completed in {:?}", glob_elapsed); diff --git a/docs/JSONRPC.md b/docs/JSONRPC.md index 85292326..ef7e6bc4 100644 --- a/docs/JSONRPC.md +++ b/docs/JSONRPC.md @@ -83,7 +83,9 @@ interface ConfigureParams { * * Useful for VS Code so users can configure where they store virtual environments. * - * Glob patterns are supported (e.g., "/home/user/envs/*", "/home/user/*/venv"). + * Bounded glob patterns are supported (e.g., ".venv", "*/.venv", "/home/user/envs/*"). + * Avoid recursive workspace-wide patterns such as "**/.venv": they can traverse large directory trees, + * delay configure responses, and trigger client timeouts. */ environmentDirectories?: string[]; /** From cb9a1c030e73f99ca340b521cbfa872c739e28d9 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 5 Aug 2026 11:13:08 -0700 Subject: [PATCH 2/8] test: cover recursive glob diagnostics (PR #495) Remove the duplicate recursive warning and cover pattern filtering plus directory-only expansion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/pet/src/jsonrpc.rs | 55 +++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/crates/pet/src/jsonrpc.rs b/crates/pet/src/jsonrpc.rs index 025bdda4..c9d87306 100644 --- a/crates/pet/src/jsonrpc.rs +++ b/crates/pet/src/jsonrpc.rs @@ -593,14 +593,18 @@ fn expand_configure_directory_patterns(kind: &str, patterns: Vec) -> Ve .collect() } +fn recursive_environment_patterns(patterns: &[PathBuf]) -> impl Iterator { + patterns + .iter() + .filter(|pattern| is_recursive_glob_pattern(&pattern.to_string_lossy())) +} + fn warn_for_recursive_environment_patterns(patterns: &[PathBuf]) { - for pattern in patterns { - if is_recursive_glob_pattern(&pattern.to_string_lossy()) { - warn!( - "Recursive environmentDirectories pattern '{}' can make configure slow; prefer bounded patterns such as '.venv' or '*/.venv'", - pattern.display() - ); - } + for pattern in recursive_environment_patterns(patterns) { + warn!( + "Recursive environmentDirectories pattern '{}' can make configure slow; prefer bounded patterns such as '.venv' or '*/.venv'", + pattern.display() + ); } } pub fn handle_configure(context: Arc, id: u32, params: Value) { @@ -631,7 +635,6 @@ pub fn handle_configure(context: Arc, id: u32, params: Value) { .environment_directories .take() .map(|patterns| { - warn_for_recursive_environment_patterns(&patterns); expand_configure_directory_patterns("environmentDirectories", patterns) }); let glob_elapsed = now.elapsed(); @@ -1434,6 +1437,42 @@ mod tests { use std::sync::{mpsc, Barrier, Mutex}; use std::thread; + #[test] + fn recursive_environment_pattern_filter_only_returns_recursive_globs() { + let patterns = vec![ + PathBuf::from(".venv"), + PathBuf::from("*/.venv"), + PathBuf::from("**/.venv"), + PathBuf::from("/workspace/**/venv"), + ]; + + assert_eq!( + recursive_environment_patterns(&patterns) + .cloned() + .collect::>(), + vec![ + PathBuf::from("**/.venv"), + PathBuf::from("/workspace/**/venv") + ] + ); + } + + #[test] + fn configure_pattern_expansion_filters_non_directories() { + let temp = tempfile::tempdir().unwrap(); + let directory = temp.path().join("env"); + let file = temp.path().join("python.exe"); + std::fs::create_dir(&directory).unwrap(); + std::fs::write(&file, "").unwrap(); + + assert_eq!( + expand_configure_directory_patterns( + "environmentDirectories", + vec![temp.path().join("*")], + ), + vec![directory] + ); + } #[derive(Default)] struct RecordingReporter { environments: Mutex>, From 3d4cd5b5ccc0079a1842b1d6f4071e5169662458 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 5 Aug 2026 12:03:38 -0700 Subject: [PATCH 3/8] fix: tighten recursive glob diagnostics (PR #495) Match recursive wildcards only as path segments, remove duplicate warnings, and add helper coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/pet-fs/src/glob.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/pet-fs/src/glob.rs b/crates/pet-fs/src/glob.rs index 1175822c..a42bff0a 100644 --- a/crates/pet-fs/src/glob.rs +++ b/crates/pet-fs/src/glob.rs @@ -21,7 +21,7 @@ pub fn is_glob_pattern(path: &str) -> bool { /// Returns true when a glob can traverse an unbounded number of path components. pub fn is_recursive_glob_pattern(path: &str) -> bool { - path.contains("**") + path.split(['/', '\']).any(|segment| segment == "**") } /// Checks if a string contains a valid brace expansion pattern `{a,b}`. /// Requires an opening `{`, at least one `,`, and a closing `}`. @@ -212,6 +212,8 @@ mod tests { assert!(is_recursive_glob_pattern("/home/user/**/venv")); assert!(!is_recursive_glob_pattern(".venv")); assert!(!is_recursive_glob_pattern("*/.venv")); + assert!(!is_recursive_glob_pattern("foo**bar/.venv")); + assert!(is_recursive_glob_pattern("C:\\workspace\\**\\.venv")); } #[test] fn test_is_glob_pattern_with_question_mark() { From d4509b2f634825b40485db544c8131d19c4ea7a0 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 5 Aug 2026 14:06:43 -0700 Subject: [PATCH 4/8] fix: correct recursive glob guidance (PR #495) Treat globstar only as a complete path segment and document environmentDirectories as container directories. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/pet-fs/src/glob.rs | 3 ++- crates/pet/src/jsonrpc.rs | 2 +- docs/JSONRPC.md | 8 ++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/pet-fs/src/glob.rs b/crates/pet-fs/src/glob.rs index a42bff0a..689dda7b 100644 --- a/crates/pet-fs/src/glob.rs +++ b/crates/pet-fs/src/glob.rs @@ -21,8 +21,9 @@ pub fn is_glob_pattern(path: &str) -> bool { /// Returns true when a glob can traverse an unbounded number of path components. pub fn is_recursive_glob_pattern(path: &str) -> bool { - path.split(['/', '\']).any(|segment| segment == "**") + path.split(['/', '\\']).any(|segment| segment == "**") } + /// Checks if a string contains a valid brace expansion pattern `{a,b}`. /// Requires an opening `{`, at least one `,`, and a closing `}`. fn has_brace_pattern(path: &str) -> bool { diff --git a/crates/pet/src/jsonrpc.rs b/crates/pet/src/jsonrpc.rs index c9d87306..c259287c 100644 --- a/crates/pet/src/jsonrpc.rs +++ b/crates/pet/src/jsonrpc.rs @@ -602,7 +602,7 @@ fn recursive_environment_patterns(patterns: &[PathBuf]) -> impl Iterator Date: Wed, 5 Aug 2026 14:24:55 -0700 Subject: [PATCH 5/8] docs: clarify recursive glob guidance (PR #495) State that glob patterns remain supported while explicitly discouraging recursive globstar traversal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/JSONRPC.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/JSONRPC.md b/docs/JSONRPC.md index 3248879a..1a8e1776 100644 --- a/docs/JSONRPC.md +++ b/docs/JSONRPC.md @@ -74,7 +74,7 @@ interface ConfigureParams { * If not provided, then environments such as poetry, pipenv, and the like will not be reported. * This is because poetry, pipenv, and the like are project-specific environments. * - * Bounded glob patterns are supported (e.g., `/home/user/projects/*`). + * Glob patterns are supported (e.g., `/home/user/projects/*`). Avoid recursive `**` patterns when a single-level pattern is sufficient. */ workspaceDirectories?: string[]; /** @@ -83,7 +83,7 @@ interface ConfigureParams { * * Useful for VS Code so users can configure where they store virtual environments. * - * Values identify directories that contain environments. Bounded patterns are supported (e.g., `/home/user/envs`, `/home/user/*/envs`). + * Values identify directories that contain environments. Glob patterns are supported (e.g., `/home/user/envs`, `/home/user/*/envs`). * Avoid recursive patterns such as `/home/user/**/envs`: they can traverse large directory trees, * delay configure responses, and trigger client timeouts. */ From d1449f71bc5633f7e503b4036eacee75e128e55f Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 5 Aug 2026 15:31:46 -0700 Subject: [PATCH 6/8] fix: detect brace-expanded recursive globs (PR #495) Classify globstar alternatives after brace expansion and cover the recursive brace case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/pet-fs/src/glob.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/pet-fs/src/glob.rs b/crates/pet-fs/src/glob.rs index 689dda7b..41d6fba1 100644 --- a/crates/pet-fs/src/glob.rs +++ b/crates/pet-fs/src/glob.rs @@ -21,7 +21,9 @@ pub fn is_glob_pattern(path: &str) -> bool { /// Returns true when a glob can traverse an unbounded number of path components. pub fn is_recursive_glob_pattern(path: &str) -> bool { - path.split(['/', '\\']).any(|segment| segment == "**") + expand_braces(path) + .iter() + .any(|pattern| pattern.split(['/', '\\']).any(|segment| segment == "**")) } /// Checks if a string contains a valid brace expansion pattern `{a,b}`. @@ -215,6 +217,7 @@ mod tests { assert!(!is_recursive_glob_pattern("*/.venv")); assert!(!is_recursive_glob_pattern("foo**bar/.venv")); assert!(is_recursive_glob_pattern("C:\\workspace\\**\\.venv")); + assert!(is_recursive_glob_pattern("{foo,**}/.venv")); } #[test] fn test_is_glob_pattern_with_question_mark() { From 1bb3c51c572f3973295d92841dcb10d8355bc6ee Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 5 Aug 2026 15:47:25 -0700 Subject: [PATCH 7/8] docs: use platform-neutral glob examples (PR #495) Keep configure diagnostics clear on Windows, macOS, and Linux with relative container-directory examples. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/pet/src/jsonrpc.rs | 2 +- docs/JSONRPC.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/pet/src/jsonrpc.rs b/crates/pet/src/jsonrpc.rs index c259287c..5138f569 100644 --- a/crates/pet/src/jsonrpc.rs +++ b/crates/pet/src/jsonrpc.rs @@ -602,7 +602,7 @@ fn recursive_environment_patterns(patterns: &[PathBuf]) -> impl Iterator Date: Wed, 5 Aug 2026 15:53:32 -0700 Subject: [PATCH 8/8] docs: anchor glob examples to a root (PR #495) Avoid suggesting CWD-dependent relative environmentDirectories patterns while remaining platform neutral. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/pet/src/jsonrpc.rs | 2 +- docs/JSONRPC.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/pet/src/jsonrpc.rs b/crates/pet/src/jsonrpc.rs index 5138f569..ee41d5fd 100644 --- a/crates/pet/src/jsonrpc.rs +++ b/crates/pet/src/jsonrpc.rs @@ -602,7 +602,7 @@ fn recursive_environment_patterns(patterns: &[PathBuf]) -> impl Iterator/envs' or '/*/envs'", pattern.display() ); } diff --git a/docs/JSONRPC.md b/docs/JSONRPC.md index 2208ed66..e658ac81 100644 --- a/docs/JSONRPC.md +++ b/docs/JSONRPC.md @@ -83,8 +83,8 @@ interface ConfigureParams { * * Useful for VS Code so users can configure where they store virtual environments. * - * Values identify directories that contain environments. Glob patterns are supported (e.g., `envs`, `*/envs`). - * Avoid recursive patterns such as `**/envs`: they can traverse large directory trees, + * Values identify directories that contain environments. Glob patterns are supported (e.g., `/envs`, `/*/envs`). + * Avoid recursive patterns such as `/**/envs`: they can traverse large directory trees, * delay configure responses, and trigger client timeouts. */ environmentDirectories?: string[];