fix(workspace): avoid holding workspace file handles across session (#335)

## Summary

- Refactor `FileWithPath` to pre-read file contents into memory and
close the OS handle immediately, rather than keeping a live
`std::fs::File` for the entire lifetime of `WorkspaceRoot`.
- Update `load_package_graph` to deserialize via
`serde_*::from_slice(content())` instead of `from_reader(file())`; drop
the now-unnecessary seek-back step in `find_workspace_root`.
- Add regression tests: a cross-platform rename-over-target test
modeling pnpm's atomic update flow while `WorkspaceRoot` is alive, a
Linux-only `/proc/self/fd` check, and basic correctness tests for the
new API.

## Why

On Windows, `vp run dev` keeps a `Session` (and its `WorkspaceRoot`)
alive for the full duration of the dev server. Holding an open handle to
`pnpm-workspace.yaml` in that `WorkspaceRoot` could prevent pnpm in a
second terminal from atomically replacing the file
(write-temp-then-rename), surfacing as `EPERM: operation not permitted`
when pnpm tried to add a catalog entry — see
voidzero-dev/vite-plus#1357. Even where current Rust's default
`FILE_SHARE_DELETE` makes the rename succeed, long-lived handles to
config files are fragile across OS/AV/filesystem edge cases. Reading
content once and closing the handle is strictly simpler.

## Test plan

- [x] `cargo test -p vite_workspace` — 78 passing (including 4 new
tests)
- [x] `cargo test -p vite_task -p vite_task_plan -p vite_task_graph` —
all pass
- [x] `cargo clippy -p vite_workspace --all-targets -- -D warnings` —
clean
- [x] Verified end-to-end against vite-plus via local `[patch]`: all
vite-plus own crate tests pass.
- [x] Windows CI rerun to exercise the new regression test on the target
platform.

## Follow-up

Once merged, bump the `vite-task` git rev in `voidzero-dev/vite-plus`
`Cargo.toml` (entries at lines 92, 197, 203–206) to pick up the fix.

Fixes voidzero-dev/vite-plus#1357

Author: fengmk2

crates/vite_workspace/src/lib.rs

@@ -248,24 +248,24 @@ pub fn load_package_graph(
     let mut graph_builder = PackageGraphBuilder::default();
     let workspaces = match &workspace_root.workspace_file {
         WorkspaceFile::PnpmWorkspaceYaml(file_with_path) => {
-            let workspace: PnpmWorkspace =
-                serde_yml::from_reader(file_with_path.file()).map_err(|e| Error::SerdeYml {
+            let workspace: PnpmWorkspace = serde_yml::from_slice(file_with_path.content())
+                .map_err(|e| Error::SerdeYml {
                     file_path: Arc::clone(file_with_path.path()),
                     serde_yml_error: e,
                 })?;
             workspace.packages
         }
         WorkspaceFile::NpmWorkspaceJson(file_with_path) => {
-            let workspace: NpmWorkspace =
-                serde_json::from_reader(file_with_path.file()).map_err(|e| Error::SerdeJson {
+            let workspace: NpmWorkspace = serde_json::from_slice(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()
         }
         WorkspaceFile::NonWorkspacePackage(file_with_path) => {
             // For non-workspace packages, add the package.json to the graph as a root package
-            let package_json: PackageJson = serde_json::from_reader(file_with_path.file())
+            let package_json: PackageJson = serde_json::from_slice(file_with_path.content())
                 .map_err(|e| Error::SerdeJson {
                     file_path: Arc::clone(file_with_path.path()),
                     serde_json_error: e,

crates/vite_workspace/src/package_manager.rs

@@ -1,51 +1,49 @@
-use std::{
-    fs::File,
-    io::{BufReader, Seek, SeekFrom},
-    sync::Arc,
-};
+use std::{fs, sync::Arc};
 
 use vite_path::{AbsolutePath, RelativePathBuf};
 
 use crate::Error;
 
-/// A file handle bundled with its absolute path for error context.
+/// The contents of a file bundled with its absolute path for error context.
+///
+/// The file is read to memory on construction and its handle closed
+/// immediately — the struct itself never holds a live OS file handle. This
+/// keeps long-lived `WorkspaceRoot`s (held across an entire `vp run` session)
+/// from pinning files like `pnpm-workspace.yaml`, which on Windows could
+/// otherwise block pnpm's atomic write-and-rename and fail with EPERM
+/// (<https://github.com/voidzero-dev/vite-plus/issues/1357>).
 #[derive(Debug)]
 pub struct FileWithPath {
-    file: File,
+    content: Vec<u8>,
     path: Arc<AbsolutePath>,
 }
 
 impl FileWithPath {
-    /// Open a file at the given path.
+    /// Open a file at the given path and read its contents into memory.
     ///
     /// # Errors
-    /// Returns an error if the file cannot be opened.
+    /// Returns an error if the file cannot be read.
     pub fn open(path: Arc<AbsolutePath>) -> Result<Self, Error> {
-        let file = File::open(&*path)?;
-        Ok(Self { file, path })
+        let content = fs::read(&*path)?;
+        Ok(Self { content, path })
     }
 
-    /// Try to open a file, returning None if it doesn't exist.
+    /// Try to read a file, returning None if it doesn't exist.
     ///
     /// # Errors
-    /// Returns an error if the file exists but cannot be opened.
+    /// Returns an error if the file exists but cannot be read.
     pub fn open_if_exists(path: Arc<AbsolutePath>) -> Result<Option<Self>, Error> {
-        match File::open(&*path) {
-            Ok(file) => Ok(Some(Self { file, path })),
+        match fs::read(&*path) {
+            Ok(content) => Ok(Some(Self { content, path })),
             Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
             Err(e) => Err(e.into()),
         }
     }
 
-    /// Get a reference to the file handle.
+    /// Get the file contents as a byte slice.
     #[must_use]
-    pub const fn file(&self) -> &File {
-        &self.file
-    }
-
-    /// Get a mutable reference to the file handle.
-    pub const fn file_mut(&mut self) -> &mut File {
-        &mut self.file
+    pub fn content(&self) -> &[u8] {
+        &self.content
     }
 
     /// Get the file path.
@@ -156,17 +154,13 @@ pub fn find_workspace_root(
 
         // Check for package.json with workspaces field for npm/yarn workspace
         let package_json_path: Arc<AbsolutePath> = cwd.join("package.json").into();
-        if let Some(mut file_with_path) = FileWithPath::open_if_exists(package_json_path)? {
-            let package_json: serde_json::Value =
-                serde_json::from_reader(BufReader::new(file_with_path.file())).map_err(|e| {
-                    Error::SerdeJson {
-                        file_path: Arc::clone(file_with_path.path()),
-                        serde_json_error: e,
-                    }
+        if let Some(file_with_path) = FileWithPath::open_if_exists(package_json_path)? {
+            let package_json: serde_json::Value = serde_json::from_slice(file_with_path.content())
+                .map_err(|e| Error::SerdeJson {
+                    file_path: Arc::clone(file_with_path.path()),
+                    serde_json_error: e,
                 })?;
             if package_json.get("workspaces").is_some() {
-                // Reset the file cursor since we consumed it reading
-                file_with_path.file_mut().seek(SeekFrom::Start(0))?;
                 let relative_cwd =
                     original_cwd.strip_prefix(cwd)?.expect("cwd must be within the workspace");
                 return Ok((
@@ -195,3 +189,80 @@ pub fn find_workspace_root(
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use tempfile::TempDir;
+
+    use super::*;
+
+    /// Regression test for <https://github.com/voidzero-dev/vite-plus/issues/1357>:
+    /// on Windows, an open handle to `pnpm-workspace.yaml` without
+    /// `FILE_SHARE_DELETE` blocks pnpm's atomic write-tmp-then-rename.
+    #[test]
+    fn find_workspace_root_does_not_lock_pnpm_workspace_yaml() {
+        let temp_dir = TempDir::new().unwrap();
+        let temp_dir_path = AbsolutePath::new(temp_dir.path()).unwrap();
+        let ws_yaml = temp_dir_path.join("pnpm-workspace.yaml");
+        let ws_yaml_tmp = temp_dir_path.join("pnpm-workspace.yaml.tmp");
+
+        fs::write(&ws_yaml, b"packages:\n  - apps/*\n").unwrap();
+
+        let (workspace_root, _) = find_workspace_root(temp_dir_path).unwrap();
+
+        fs::write(&ws_yaml_tmp, b"packages:\n  - apps/*\n  - packages/*\n").unwrap();
+        fs::rename(&ws_yaml_tmp, &ws_yaml)
+            .expect("rename over pnpm-workspace.yaml must succeed while WorkspaceRoot is alive");
+
+        drop(workspace_root);
+    }
+
+    /// Linux-only: `/proc/self/fd` lets us verify no descriptor remains
+    /// pointing at `pnpm-workspace.yaml` regardless of Rust's default
+    /// share mode on the platform.
+    #[cfg(target_os = "linux")]
+    #[test]
+    fn find_workspace_root_releases_pnpm_workspace_yaml_fd_on_linux() {
+        let temp_dir = TempDir::new().unwrap();
+        let temp_dir_path = AbsolutePath::new(temp_dir.path()).unwrap();
+        let ws_yaml = temp_dir_path.join("pnpm-workspace.yaml");
+        fs::write(&ws_yaml, b"packages:\n  - apps/*\n").unwrap();
+
+        let (workspace_root, _) = find_workspace_root(temp_dir_path).unwrap();
+
+        let ws_yaml_canonical = fs::canonicalize(&ws_yaml).unwrap();
+        let mut open_to_target = 0;
+        for entry in fs::read_dir("/proc/self/fd").unwrap().flatten() {
+            if let Ok(link) = fs::read_link(entry.path())
+                && link == ws_yaml_canonical
+            {
+                open_to_target += 1;
+            }
+        }
+        assert_eq!(
+            open_to_target, 0,
+            "expected no open file descriptor for pnpm-workspace.yaml, got {open_to_target}",
+        );
+        drop(workspace_root);
+    }
+
+    #[test]
+    fn file_with_path_content_matches_file_on_disk() {
+        let temp_dir = TempDir::new().unwrap();
+        let temp_dir_path = AbsolutePath::new(temp_dir.path()).unwrap();
+        let path: Arc<AbsolutePath> = temp_dir_path.join("pnpm-workspace.yaml").into();
+        fs::write(&*path, b"packages:\n  - apps/*\n").unwrap();
+
+        let file_with_path = FileWithPath::open(Arc::clone(&path)).unwrap();
+        assert_eq!(file_with_path.content(), b"packages:\n  - apps/*\n");
+        assert_eq!(&**file_with_path.path(), &*path);
+    }
+
+    #[test]
+    fn file_with_path_open_if_exists_returns_none_when_missing() {
+        let temp_dir = TempDir::new().unwrap();
+        let temp_dir_path = AbsolutePath::new(temp_dir.path()).unwrap();
+        let path: Arc<AbsolutePath> = temp_dir_path.join("missing.yaml").into();
+        assert!(FileWithPath::open_if_exists(path).unwrap().is_none());
+    }
+}