fix(workspace): only use a workspace root that includes the current package

CHANGELOG.md

@@ -1,5 +1,6 @@
 # Changelog
 
+- **Fixed** Running `vp run` in a package that sits inside a workspace folder but isn't listed in its `pnpm-workspace.yaml` `packages` or `package.json` `workspaces` now runs that package's own tasks, instead of treating the enclosing workspace as its root. When both configurations exist, `pnpm-workspace.yaml` takes precedence ([#859](https://github.com/voidzero-dev/vite-task/pull/859)).
 - **Fixed** A mistyped or misplaced setting in a task now gets an error that names it. For example, `cache: { outputs: [...] }` now fails with an `unknown field` error that names `outputs` and lists the accepted settings, instead of `data did not match any variant of untagged enum UserTaskDefinition` ([vite-plus#2635](https://github.com/voidzero-dev/vite-plus/issues/2635), [#815](https://github.com/voidzero-dev/vite-task/pull/815)).
 - **Fixed** In cached tasks on Linux and macOS, programs that pass `-1` as a directory descriptor to calls such as `openat` no longer abort. For example, `ps -p <pid>` from procps-ng 4.0.6 or later aborted when the process had already exited ([#862](https://github.com/voidzero-dev/vite-task/pull/862)).
 - **Fixed** `vp run` no longer panics before running a script that contains non-ASCII characters, such as `echo こんにちは`, and no longer cuts off such commands where it displays them ([#861](https://github.com/voidzero-dev/vite-task/pull/861)).

crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/examples/demo/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "demo",
+  "scripts": {
+    "build": "vt tool print demo"
+  }
+}

crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "root",
+  "workspaces": [
+    "packages/*",
+    "examples/*"
+  ],
+  "scripts": {
+    "build": "vt tool print root"
+  }
+}

crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/packages/app/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "app",
+  "scripts": {
+    "build": "vt tool print app"
+  }
+}

crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/pnpm-workspace.yaml

@@ -0,0 +1,2 @@
+packages:
+  - 'packages/*'

crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots.toml

@@ -0,0 +1,10 @@
+# A package excluded by pnpm stays standalone even when package.json workspaces includes it.
+
+[fields]
+key = true
+execution_item_display = { command = true }
+
+[[plan]]
+name = "non_member_package_is_its_own_workspace"
+args = ["run", "--recursive", "build"]
+cwd = "examples/demo"

crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots/query_non_member_package_is_its_own_workspace.jsonc

@@ -0,0 +1,20 @@
+// run --recursive build
+{
+  "graph": [
+    {
+      "key": [
+        "<workspace>/examples/demo/",
+        "build"
+      ],
+      "node": {
+        "items": [
+          {
+            "execution_item_display": {
+              "command": "vt tool print demo"
+            }
+          }
+        ]
+      }
+    }
+  ]
+}

crates/vt_plan/tests/plan_snapshots/fixtures/workspace_non_member_package/snapshots/task_graph.md

@@ -0,0 +1,88 @@
+# task graph
+
+```mermaid
+flowchart TD
+  task_0["<workspace>/#build"]
+  task_1["<workspace>/packages/app#build"]
+```
+
+## `<workspace>/#build`
+
+```json
+{
+  "task_display": {
+    "package_name": "root",
+    "task_name": "build",
+    "package_path": "<workspace>/"
+  },
+  "resolved_config": {
+    "commands": [
+      "vt tool print root"
+    ],
+    "resolved_options": {
+      "cwd": "<workspace>/",
+      "cache_config": {
+        "remote_cache_allowed": true,
+        "env_config": {
+          "fingerprinted_envs": [],
+          "untracked_env": [
+            "<default untracked envs>"
+          ]
+        },
+        "input_config": {
+          "includes_auto": true,
+          "positive_globs": [],
+          "negative_globs": []
+        },
+        "output_config": {
+          "includes_auto": true,
+          "positive_globs": [],
+          "negative_globs": []
+        }
+      }
+    }
+  },
+  "source": "PackageJsonScript"
+}
+```
+
+## `<workspace>/packages/app#build`
+
+```json
+{
+  "task_display": {
+    "package_name": "app",
+    "task_name": "build",
+    "package_path": "<workspace>/packages/app"
+  },
+  "resolved_config": {
+    "commands": [
+      "vt tool print app"
+    ],
+    "resolved_options": {
+      "cwd": "<workspace>/packages/app",
+      "cache_config": {
+        "remote_cache_allowed": true,
+        "env_config": {
+          "fingerprinted_envs": [],
+          "untracked_env": [
+            "<default untracked envs>"
+          ]
+        },
+        "input_config": {
+          "includes_auto": true,
+          "positive_globs": [],
+          "negative_globs": []
+        },
+        "output_config": {
+          "includes_auto": true,
+          "positive_globs": [],
+          "negative_globs": []
+        }
+      }
+    }
+  },
+  "source": "PackageJsonScript"
+}
+```
+

crates/vt_workspace/src/lib.rs

@@ -11,7 +11,7 @@ use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet};
 use serde::Deserialize;
 use vec1::smallvec_v1::SmallVec1;
 use vt_glob::path::PathGlobSet;
-use vt_path::{AbsolutePath, AbsolutePathBuf, RelativePathBuf};
+use vt_path::{AbsolutePath, AbsolutePathBuf, RelativePath, RelativePathBuf};
 use vt_str::Str;
 use wax::{
     Glob,
@@ -88,20 +88,29 @@ struct WorkspaceMemberGlobs {
     workspaces: Vec<Str>,
 }
 impl WorkspaceMemberGlobs {
-    const fn new(workspaces: Vec<Str>) -> Self {
-        Self { workspaces }
+    fn from_pnpm_workspace_yaml(file_with_path: &FileWithPath) -> Result<Self, Error> {
+        let workspace: PnpmWorkspace =
+            serde_norway::from_slice(strip_bom(file_with_path.content())).map_err(|e| {
+                Error::SerdeYaml {
+                    file_path: Arc::clone(file_with_path.path()),
+                    serde_yaml_error: e,
+                }
+            })?;
+        Ok(Self { workspaces: workspace.packages })
     }
 
-    fn get_package_json_paths(
-        self,
-        workspace_root: impl AsRef<AbsolutePath>,
-    ) -> Result<impl IntoIterator<Item = AbsolutePathBuf>, Error> {
-        let workspace_root = workspace_root.as_ref();
-        let mut package_json_paths = HashSet::<AbsolutePathBuf>::default();
-        let mut has_negated = false;
-        let mut inclusions = Vec::<Str>::new();
-        let mut all = Vec::<Str>::new();
-        for pattern in self.workspaces {
+    fn from_npm_workspace_json(file_with_path: &FileWithPath) -> Result<Self, Error> {
+        let workspace: NpmWorkspace = serde_json::from_slice(strip_bom(file_with_path.content()))
+            .map_err(|e| Error::SerdeJson {
+            file_path: Arc::clone(file_with_path.path()),
+            serde_json_error: e,
+        })?;
+        Ok(Self { workspaces: workspace.workspaces.into_packages() })
+    }
+
+    /// Converts each member glob into a glob of `package.json` files, paired with whether it is negated.
+    fn package_json_patterns(&self) -> impl Iterator<Item = (Str, bool)> + '_ {
+        self.workspaces.iter().map(|pattern| {
             // Ported from npm's workspace pattern normalization:
             // https://github.com/npm/map-workspaces/blob/76ea3c852171790e245052e006329de15b09e60e/lib/index.js#L17
             let exclusion_count = pattern.bytes().take_while(|byte| *byte == b'!').count();
@@ -124,7 +133,31 @@ impl WorkspaceMemberGlobs {
             } else {
                 "/package.json"
             });
+            (pattern, is_negated)
+        })
+    }
 
+    /// Checks whether the package at `package_path`, relative to the workspace root, is a member.
+    fn includes(&self, package_path: &RelativePath) -> Result<bool, Error> {
+        // `get_package_json_paths` never walks into `node_modules`.
+        if package_path.as_str().split('/').any(|component| component == "node_modules") {
+            return Ok(false);
+        }
+        let patterns = self.package_json_patterns().map(|(pattern, _)| pattern).collect::<Vec<_>>();
+        let package_json_path = vt_str::format!("{}/package.json", package_path.as_str());
+        Ok(PathGlobSet::new(&patterns)?.is_match(package_json_path.as_str()))
+    }
+
+    fn get_package_json_paths(
+        self,
+        workspace_root: impl AsRef<AbsolutePath>,
+    ) -> Result<impl IntoIterator<Item = AbsolutePathBuf>, Error> {
+        let workspace_root = workspace_root.as_ref();
+        let mut package_json_paths = HashSet::<AbsolutePathBuf>::default();
+        let mut has_negated = false;
+        let mut inclusions = Vec::<Str>::new();
+        let mut all = Vec::<Str>::new();
+        for (pattern, is_negated) in self.package_json_patterns() {
             if is_negated {
                 has_negated = true;
             } else {
@@ -283,26 +316,12 @@ pub fn load_package_graph(
     workspace_root: &WorkspaceRoot,
 ) -> Result<DiGraph<PackageInfo, DependencyType, PackageIx>, Error> {
     let mut graph_builder = PackageGraphBuilder::default();
-    let workspaces = match &workspace_root.workspace_file {
+    let member_globs = match &workspace_root.workspace_file {
         WorkspaceFile::PnpmWorkspaceYaml(file_with_path) => {
-            let workspace: PnpmWorkspace =
-                serde_norway::from_slice(strip_bom(file_with_path.content())).map_err(|e| {
-                    Error::SerdeYaml {
-                        file_path: Arc::clone(file_with_path.path()),
-                        serde_yaml_error: e,
-                    }
-                })?;
-            workspace.packages
+            WorkspaceMemberGlobs::from_pnpm_workspace_yaml(file_with_path)?
         }
         WorkspaceFile::NpmWorkspaceJson(file_with_path) => {
-            let workspace: NpmWorkspace =
-                serde_json::from_slice(strip_bom(file_with_path.content())).map_err(|e| {
-                    Error::SerdeJson {
-                        file_path: Arc::clone(file_with_path.path()),
-                        serde_json_error: e,
-                    }
-                })?;
-            workspace.workspaces.into_packages()
+            WorkspaceMemberGlobs::from_npm_workspace_json(file_with_path)?
         }
         WorkspaceFile::NonWorkspacePackage(file_with_path) => {
             // For non-workspace packages, add the package.json to the graph as a root package
@@ -323,7 +342,6 @@ pub fn load_package_graph(
         }
     };
 
-    let member_globs = WorkspaceMemberGlobs::new(workspaces);
     let mut has_root_package = false;
     for package_json_path in member_globs.get_package_json_paths(&*workspace_root.path)? {
         let package_json_path: Arc<AbsolutePath> = package_json_path.clone().into();

crates/vt_workspace/src/package_manager.rs

@@ -2,7 +2,7 @@ use std::{fs, sync::Arc};
 
 use vt_path::{AbsolutePath, RelativePathBuf};
 
-use crate::Error;
+use crate::{Error, WorkspaceMemberGlobs};
 
 /// The contents of a file bundled with its absolute path for error context.
 ///
@@ -123,6 +123,9 @@ pub struct WorkspaceRoot {
 ///
 /// Returns the workspace root and the relative path from the workspace root to the original cwd.
 ///
+/// Like npm, a workspace is only used if its member globs include the package nearest to `original_cwd`, so a package
+/// nested in a workspace directory without being declared as a member is not treated as part of that workspace.
+///
 /// If the workspace file is not found, but a package is found, `workspace_file` will be `NonWorkspacePackage` with the `package.json` File.
 ///
 /// If neither workspace nor package is found, will return `PackageJsonNotFound` error.
@@ -136,11 +139,18 @@ pub fn find_workspace_root(
     original_cwd: &AbsolutePath,
 ) -> Result<(WorkspaceRoot, RelativePathBuf), Error> {
     let mut cwd = original_cwd;
+    let mut nearest_package: Option<&AbsolutePath> = None;
 
     loop {
         // Check for pnpm-workspace.yaml for pnpm workspace
         let pnpm_workspace_path: Arc<AbsolutePath> = cwd.join("pnpm-workspace.yaml").into();
-        if let Some(file_with_path) = FileWithPath::open_if_exists(pnpm_workspace_path)? {
+        let pnpm_workspace = FileWithPath::open_if_exists(pnpm_workspace_path)?;
+        let is_pnpm_workspace = pnpm_workspace.is_some();
+        if let Some(file_with_path) = pnpm_workspace
+            && includes_package(cwd, nearest_package, || {
+                WorkspaceMemberGlobs::from_pnpm_workspace_yaml(&file_with_path)
+            })?
+        {
             let relative_cwd =
                 original_cwd.strip_prefix(cwd)?.expect("cwd must be within the pnpm workspace");
             return Ok((
@@ -162,7 +172,13 @@ pub fn find_workspace_root(
                 file_path: Arc::clone(file_with_path.path()),
                 serde_json_error: e,
             })?;
-            if package_json.get("workspaces").is_some() {
+            // pnpm defines membership even when its globs exclude the current package.
+            if !is_pnpm_workspace
+                && package_json.get("workspaces").is_some()
+                && includes_package(cwd, nearest_package, || {
+                    WorkspaceMemberGlobs::from_npm_workspace_json(&file_with_path)
+                })?
+            {
                 let relative_cwd =
                     original_cwd.strip_prefix(cwd)?.expect("cwd must be within the workspace");
                 return Ok((
@@ -173,6 +189,7 @@ pub fn find_workspace_root(
                     relative_cwd,
                 ));
             }
+            nearest_package.get_or_insert(cwd);
         }
 
         // TODO(@fengmk2): other package manager support
@@ -192,6 +209,24 @@ pub fn find_workspace_root(
     }
 }
 
+/// Checks whether the workspace at `workspace_root` includes the nearest package found below it.
+///
+/// Without a package below it, `original_cwd` belongs to the workspace root package itself.
+fn includes_package(
+    workspace_root: &AbsolutePath,
+    nearest_package: Option<&AbsolutePath>,
+    member_globs: impl FnOnce() -> Result<WorkspaceMemberGlobs, Error>,
+) -> Result<bool, Error> {
+    let Some(nearest_package) = nearest_package else {
+        return Ok(true);
+    };
+    // SAFETY: `nearest_package` is found while walking up from `original_cwd` to `workspace_root`.
+    let package_path = nearest_package
+        .strip_prefix(workspace_root)?
+        .expect("nearest package must be within the workspace root");
+    member_globs()?.includes(&package_path)
+}
+
 #[cfg(test)]
 mod tests {
     use tempfile::TempDir;