Refactor which resolver: OS-specific workspace fallback

docs/netsuke-design.md

@@ -962,7 +962,7 @@ The resolver keeps a small LRU cache keyed by the command, a fingerprint of
 `PATH`/`PATHEXT`, the working directory, and the cache-relevant options (`all`,
 `canonical`, `cwd_mode`). Entries are validated once at insertion; cache reads
 no longer re-probe executability, keeping the hot path lean. Because `fresh`
-only controls bypass behaviour it is stripped from the cache key so fresh
+only controls bypass behaviour, it is stripped from the cache key so fresh
 lookups still repopulate the cache for subsequent calls. The fingerprint means
 environment changes invalidate keys without cloning large strings, and the
 helper remains pure because all inputs still derive from the manifest or
@@ -985,6 +985,11 @@ reuse, and list-all semantics. Behavioural MiniJinja fixtures exercise the
 filter in Stage 3/4 renders to prove determinism across repeated invocations
 with identical environments.
 
+Workspace fallback traversals are bounded to a depth of six, skip heavy
+directories such as `.git`, `target`, `node_modules`, `dist`, and `build`, and
+honour the `NETSUKE_WHICH_WORKSPACE` environment variable (set to
+`0`/`false`/`off` to disable) to avoid surprising latency on large trees.
+
 Sequence of the resolver when falling back to the workspace:
 
 ```mermaid

src/stdlib/config.rs

@@ -1,6 +1,6 @@
 //! Configuration types and defaults for wiring the stdlib into `MiniJinja`.
 
-use super::{command, network::NetworkPolicy, which::DEFAULT_WORKSPACE_SKIP_DIRS};
+use super::{command, network::NetworkPolicy, which::WORKSPACE_SKIP_DIRS};
 use anyhow::{anyhow, bail, ensure};
 use camino::{Utf8Component, Utf8Path, Utf8PathBuf};
 use cap_std::{ambient_authority, fs_utf8::Dir};
@@ -59,7 +59,7 @@ impl StdlibConfig {
             command_max_output_bytes: DEFAULT_COMMAND_MAX_OUTPUT_BYTES,
             command_max_stream_bytes: DEFAULT_COMMAND_MAX_STREAM_BYTES,
             which_cache_capacity,
-            workspace_skip_dirs: DEFAULT_WORKSPACE_SKIP_DIRS
+            workspace_skip_dirs: WORKSPACE_SKIP_DIRS
                 .iter()
                 .map(|dir| (*dir).to_owned())
                 .collect(),

src/stdlib/register.rs

@@ -8,7 +8,7 @@
 
 use super::{
     StdlibConfig, StdlibState, collections, command, network, path, time,
-    which::{self, WhichConfig},
+    which::{self, WhichConfig, WorkspaceSkipList},
 };
 use anyhow::Context;
 use camino::Utf8Path;
@@ -90,13 +90,11 @@ pub fn register_with_config(
     path::register_filters(env);
     collections::register_filters(env);
     let which_cache_capacity = config.which_cache_capacity();
-    let which_config = WhichConfig::new(
-        config
-            .workspace_root_path()
-            .map(|path| Arc::new(path.to_path_buf())),
-        which::WorkspaceSkipList::from_names(config.workspace_skip_dirs().iter()),
-        which_cache_capacity,
-    );
+    let which_skip_dirs = WorkspaceSkipList::from_names(config.workspace_skip_dirs());
+    let which_cwd = config
+        .workspace_root_path()
+        .map(|path| Arc::new(path.to_path_buf()));
+    let which_config = WhichConfig::new(which_cwd, which_skip_dirs, which_cache_capacity);
     which::register(env, which_config);
     let impure = state.impure_flag();
     let (network_config, command_config) = config.into_components();
@@ -127,7 +125,15 @@ const FILE_TESTS: &[FileTest] = &[
 ];
 
 #[cfg(not(unix))]
-const FILE_TESTS: &[FileTest] = &[("dir", is_dir), ("file", is_file), ("symlink", is_symlink)];
+const FILE_TESTS: &[FileTest] = &[
+    ("dir", is_dir),
+    ("file", is_file),
+    ("symlink", is_symlink),
+    ("pipe", is_fifo),
+    ("block_device", is_block_device),
+    ("char_device", is_char_device),
+    ("device", is_device),
+];
 
 fn register_file_tests(env: &mut Environment<'_>) {
     for &(name, pred) in FILE_TESTS {
@@ -158,68 +164,37 @@ fn is_fifo(ft: fs::FileType) -> bool {
     ft.is_fifo()
 }
 
+#[cfg(not(unix))]
+fn is_fifo(_ft: fs::FileType) -> bool {
+    false
+}
+
 #[cfg(unix)]
 fn is_block_device(ft: fs::FileType) -> bool {
     ft.is_block_device()
 }
 
+#[cfg(not(unix))]
+fn is_block_device(_ft: fs::FileType) -> bool {
+    false
+}
+
 #[cfg(unix)]
 fn is_char_device(ft: fs::FileType) -> bool {
     ft.is_char_device()
 }
 
+#[cfg(not(unix))]
+fn is_char_device(_ft: fs::FileType) -> bool {
+    false
+}
+
 #[cfg(unix)]
 fn is_device(ft: fs::FileType) -> bool {
     is_block_device(ft) || is_char_device(ft)
 }
 
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use anyhow::{Result, ensure};
-    use camino::Utf8PathBuf;
-    use minijinja::{Environment, context};
-    use std::ffi::OsStr;
-    use tempfile::TempDir;
-    use test_support::{env::VarGuard, write_exec};
-
-    #[test]
-    fn register_with_config_honours_workspace_skip_dirs() -> Result<()> {
-        let _guard = VarGuard::set("PATH", OsStr::new(""));
-        let temp = TempDir::new()?;
-        let root_path =
-            Utf8PathBuf::from_path_buf(temp.path().to_path_buf()).expect("utf8 temp path");
-        let root_dir = Dir::open_ambient_dir(root_path.as_path(), ambient_authority())?;
-
-        let target = root_path.join("target");
-        std::fs::create_dir_all(target.as_std_path())?;
-        let exec = write_exec(target.as_path(), "tool")?;
-
-        let mut env = Environment::new();
-        let default_config =
-            StdlibConfig::new(root_dir.try_clone()?)?.with_workspace_root_path(&root_path)?;
-        register_with_config(&mut env, default_config)?;
-
-        env.add_template("t", "{{ which('tool') }}")?;
-        let render_err = env
-            .get_template("t")?
-            .render(context! {})
-            .expect_err("default skips should block target");
-        ensure!(
-            render_err
-                .to_string()
-                .contains("netsuke::jinja::which::not_found"),
-            "expected not_found error, got {render_err}"
-        );
-
-        let mut env_custom = Environment::new();
-        let custom_config = StdlibConfig::new(root_dir)?
-            .with_workspace_root_path(&root_path)?
-            .with_workspace_skip_dirs([".git"])?;
-        register_with_config(&mut env_custom, custom_config)?;
-        env_custom.add_template("t", "{{ which('tool') }}")?;
-        let rendered = env_custom.get_template("t")?.render(context! {})?;
-        ensure!(rendered == exec.as_str(), "expected resolved path");
-        Ok(())
-    }
+#[cfg(not(unix))]
+fn is_device(_ft: fs::FileType) -> bool {
+    false
 }

src/stdlib/which/cache.rs

@@ -119,7 +119,7 @@ mod tests {
     use rstest::rstest;
     use std::{num::NonZeroUsize, sync::Arc};
     use tempfile::TempDir;
-    use test_support::env::VarGuard;
+    use test_support::env::{VarGuard, with_isolated_path};
 
     fn cache_key_for(command: &str) -> CacheKey {
         CacheKey {
@@ -183,47 +183,48 @@ mod tests {
 
     #[test]
     fn resolver_applies_skip_list_during_resolution() -> Result<()> {
-        let _guard = VarGuard::set("PATH", std::ffi::OsStr::new(""));
-        let temp = TempDir::new()?;
-        let cwd = Utf8PathBuf::from_path_buf(temp.path().to_path_buf())
-            .map_err(|path| anyhow!("temp path should be utf8: {path:?}"))?;
-
-        let target = cwd.join("target");
-        std::fs::create_dir_all(target.as_std_path())?;
-        std::fs::write(target.join("tool").as_std_path(), b"#!/bin/sh\n")?;
-        #[cfg(unix)]
-        {
-            use std::os::unix::fs::PermissionsExt;
-            let path = target.join("tool");
-            let mut perms = std::fs::metadata(path.as_std_path())?.permissions();
-            perms.set_mode(0o755);
-            std::fs::set_permissions(path.as_std_path(), perms)?;
-        }
-
-        let capacity = NonZeroUsize::new(64).expect("non-zero cache capacity");
-        let resolver = WhichResolver::new(
-            Some(Arc::new(cwd.clone())),
-            WorkspaceSkipList::default(),
-            capacity,
-        );
-        let options = WhichOptions::default();
-        let err = resolver
-            .resolve("tool", &options)
-            .expect_err("default skip should ignore target");
-
-        ensure!(matches!(err.kind(), ErrorKind::InvalidOperation));
-
-        let resolver_custom = WhichResolver::new(
-            Some(Arc::new(cwd.clone())),
-            WorkspaceSkipList::from_names([".git"]),
-            capacity,
-        );
-        let matches = resolver_custom.resolve("tool", &options)?;
-        ensure!(
-            matches == vec![target.join("tool")],
-            "expected executable discovery when target not skipped"
-        );
-
-        Ok(())
+        with_isolated_path(std::ffi::OsStr::new(""), || -> Result<()> {
+            let temp = TempDir::new()?;
+            let cwd = Utf8PathBuf::from_path_buf(temp.path().to_path_buf())
+                .map_err(|path| anyhow!("temp path should be utf8: {path:?}"))?;
+
+            let target = cwd.join("target");
+            std::fs::create_dir_all(target.as_std_path())?;
+            std::fs::write(target.join("tool").as_std_path(), b"#!/bin/sh\n")?;
+            #[cfg(unix)]
+            {
+                use std::os::unix::fs::PermissionsExt;
+                let path = target.join("tool");
+                let mut perms = std::fs::metadata(path.as_std_path())?.permissions();
+                perms.set_mode(0o755);
+                std::fs::set_permissions(path.as_std_path(), perms)?;
+            }
+
+            let capacity = NonZeroUsize::new(64).expect("non-zero cache capacity");
+            let resolver = WhichResolver::new(
+                Some(Arc::new(cwd.clone())),
+                WorkspaceSkipList::default(),
+                capacity,
+            );
+            let options = WhichOptions::default();
+            let err = resolver
+                .resolve("tool", &options)
+                .expect_err("default skip should ignore target");
+
+            ensure!(matches!(err.kind(), ErrorKind::InvalidOperation));
+
+            let resolver_custom = WhichResolver::new(
+                Some(Arc::new(cwd.clone())),
+                WorkspaceSkipList::from_names([".git"]),
+                capacity,
+            );
+            let matches = resolver_custom.resolve("tool", &options)?;
+            ensure!(
+                matches == vec![target.join("tool")],
+                "expected executable discovery when target not skipped"
+            );
+
+            Ok(())
+        })
     }
 }