fix(plugins): confine resolved write destinations

crates/ide-host/src/ops/lsp.rs

@@ -922,12 +922,13 @@ fn plugin_file_edits(
     use ide_plugin::FileOperation;
     let confine = !a.operations.is_empty() || a.open.is_some();
     let confined = |path: &Path| ide_plugin::resolve_new_in_root(root, &path.to_string_lossy());
+    let writable = |path: &Path| ide_plugin::resolve_writable_in_root(root, &path.to_string_lossy());
     let mut files = Vec::new();
     let mut created_with_text = Vec::new();
     for operation in &a.operations {
         match operation {
             FileOperation::Create { path, text, overwrite } => {
-                let path = confined(path)?;
+                let path = writable(path)?;
                 files.push(FileEdit::Create { path: path.clone().into(), overwrite: *overwrite });
                 if !text.is_empty() {
                     let start = Range::point(Pos::new(0, 0));
@@ -948,7 +949,7 @@ fn plugin_file_edits(
         }
     }
     for (path, edits) in &a.edits {
-        let path = if confine { confined(path)? } else { path.clone() };
+        let path = if confine { writable(path)? } else { path.clone() };
         if created_with_text.contains(&path) {
             return Err("put the text of a created file in its `create`, not in an edit".into());
         }
@@ -2228,6 +2229,55 @@ mod tests {
         assert!(without_generated_helpers(Vec::new()).is_empty());
     }
 
+    #[cfg(unix)]
+    #[test]
+    fn plugins_file_actions_confine_content_writes_and_preserve_link_entries() {
+        use ide_core::FileEdit;
+        use ide_plugin::{CodeAction, FileOperation};
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path().join("proj");
+        std::fs::create_dir(&root).unwrap();
+        let root = dunce::canonicalize(root).unwrap();
+        let outside = tmp.path().join("outside.txt");
+        std::fs::write(&outside, "unchanged").unwrap();
+        std::fs::write(root.join("target.txt"), "inside").unwrap();
+        for (name, target) in [
+            ("outside", outside.clone()),
+            ("dangling", tmp.path().join("missing.txt")),
+            ("inside", root.join("target.txt")),
+        ] {
+            std::os::unix::fs::symlink(target, root.join(name)).unwrap();
+        }
+        let action = |operations, edits| CodeAction {
+            title: "Update files".into(),
+            kind: None,
+            preferred: false,
+            edits,
+            operations,
+            open: Some(root.join("target.txt")),
+            data: None,
+            file_action: true,
+        };
+        for name in ["outside", "dangling"] {
+            let create = FileOperation::Create { path: root.join(name), text: "text".into(), overwrite: true };
+            assert!(plugin_file_edits(&root, &action(vec![create], Vec::new())).is_err());
+            let edits = vec![(root.join(name), vec![TextEdit { range: Range::default(), new_text: "text".into() }])];
+            assert!(plugin_file_edits(&root, &action(Vec::new(), edits)).is_err());
+        }
+        let create = FileOperation::Create { path: root.join("inside"), text: "text".into(), overwrite: true };
+        let (files, _) = plugin_file_edits(&root, &action(vec![create], Vec::new())).unwrap();
+        assert!(matches!(&files[0], FileEdit::Create { path, .. } if **path == root.join("target.txt")));
+        let operations = vec![
+            FileOperation::Rename { from: root.join("outside"), to: root.join("dangling") },
+            FileOperation::Delete { path: root.join("outside") },
+        ];
+        let (files, _) = plugin_file_edits(&root, &action(operations, Vec::new())).unwrap();
+        assert!(matches!(&files[0], FileEdit::Rename { from, to }
+            if **from == root.join("outside") && **to == root.join("dangling")));
+        assert!(matches!(&files[1], FileEdit::Delete { path, .. } if **path == root.join("outside")));
+        assert_eq!(std::fs::read_to_string(outside).unwrap(), "unchanged");
+    }
+
     #[test]
     fn a_plugin_action_maps_its_file_operations_before_its_edits_and_stays_in_the_project() {
         use ide_core::FileEdit;

crates/ide-host/src/plugins/api.rs

@@ -163,16 +163,10 @@ impl SecretStore for FileSecretStore {
             None => map.remove(name),
         };
         let file = self.file(plugin);
-        std::fs::create_dir_all(file.parent().expect("has parent")).map_err(|e| e.to_string())?;
+        std::fs::create_dir_all(&self.dir).map_err(|e| e.to_string())?;
+        ide_vfs::create_private_dir(file.parent().expect("has parent")).map_err(|e| e.to_string())?;
         let bytes = serde_json::to_vec(&map).map_err(|e| e.to_string())?;
-        let tmp = file.with_extension("tmp");
-        std::fs::write(&tmp, bytes).map_err(|e| e.to_string())?;
-        #[cfg(unix)]
-        {
-            use std::os::unix::fs::PermissionsExt;
-            let _ = std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o600));
-        }
-        std::fs::rename(&tmp, &file).map_err(|e| e.to_string())
+        replace_entry(&file, &bytes).map_err(|e| e.to_string())
     }
 }
 
@@ -924,6 +918,15 @@ fn storage_io(e: std::io::Error) -> HostError {
     HostError::Failed(format!("storage: {e}"))
 }
 
+fn replace_entry(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
+    use std::io::Write;
+    let mut tmp =
+        tempfile::Builder::new().prefix(".entry-").suffix(".tmp").tempfile_in(path.parent().expect("has parent"))?;
+    tmp.write_all(bytes)?;
+    tmp.persist(path).map_err(|e| e.error)?;
+    Ok(())
+}
+
 /// A key's file name: its hash, since keys may hold any character.
 fn entry_name(key: &str) -> String {
     blake3::hash(key.as_bytes()).to_hex().to_string()
@@ -945,8 +948,7 @@ fn write_entry(dir: &Path, key: &str, value: &str) -> Result<(), HostError> {
     bytes.extend_from_slice(&len.to_le_bytes());
     bytes.extend_from_slice(key.as_bytes());
     bytes.extend_from_slice(value.as_bytes());
-    let tmp = file.with_extension("tmp");
-    std::fs::write(&tmp, bytes).and_then(|()| std::fs::rename(&tmp, &file)).map_err(storage_io)
+    replace_entry(&file, &bytes).map_err(storage_io)
 }
 
 /// The stored entries' files and the bytes of their keys and values.
@@ -983,6 +985,9 @@ impl PluginApi {
     /// older version into it.
     fn storage(&self) -> Result<PathBuf, HostError> {
         let dir = self.shared.storage_dir(self.id());
+        std::fs::create_dir_all(&self.shared.data_dir).map_err(storage_io)?;
+        ide_vfs::create_private_dir(dir.parent().expect("has parent")).map_err(failed)?;
+        ide_vfs::create_private_dir(&dir).map_err(failed)?;
         let legacy = self.shared.data_dir.join(self.id()).join("storage.json");
         match std::fs::read(&legacy) {
             Ok(bytes) => {
@@ -2682,7 +2687,9 @@ impl HostApi for PluginApi {
             Err(e) if e.kind == ide_core::error::ErrorKind::NotFound => ("utf-8".to_string(), false),
             Err(e) => return Err(not_writable(real, e)),
         };
-        if self.plugin_dir().is_ok_and(|dir| real.starts_with(dir)) {
+        let private = self.plugin_dir().is_ok_and(|dir| real.starts_with(dir));
+        check_write_path(real)?;
+        if private {
             let bytes = ide_vfs::encode(text, &encoding, has_bom).map_err(|e| not_writable(real, e))?;
             ide_vfs::write_atomic_private(real, &bytes).map(drop).map_err(|e| not_writable(real, e))
         } else {
@@ -2692,7 +2699,9 @@ impl HostApi for PluginApi {
 
     fn write_bytes_file(&self, real: &Path, bytes: &[u8]) -> Result<(), HostError> {
         check_unsaved(self.shared.workspace(), real)?;
-        if self.plugin_dir().is_ok_and(|dir| real.starts_with(dir)) {
+        let private = self.plugin_dir().is_ok_and(|dir| real.starts_with(dir));
+        check_write_path(real)?;
+        if private {
             ide_vfs::write_atomic_private(real, bytes).map(drop).map_err(|e| not_writable(real, e))
         } else {
             ide_vfs::write_atomic(real, bytes).map(drop).map_err(|e| not_writable(real, e))
@@ -2700,6 +2709,16 @@ impl HostApi for PluginApi {
     }
 }
 
+/// Approved write paths are already resolved. Refuse substitutions since approval.
+fn check_write_path(path: &Path) -> Result<(), HostError> {
+    let parent = path.parent().ok_or_else(|| HostError::Invalid("no directory to put it in".into()))?;
+    let checked = ide_plugin::resolve_writable_in_root(parent, &to_ipc(path)).map_err(HostError::Invalid)?;
+    if checked != path {
+        return Err(HostError::Invalid(format!("{}: file path changed before writing", path.display())));
+    }
+    Ok(())
+}
+
 /// A file that is not there, or cannot be read, as the plugin sees it.
 fn not_a_file(path: &Path, e: std::io::Error) -> HostError {
     match e.kind() {
@@ -3103,6 +3122,72 @@ mod tests {
         PluginApi { pkg: Package::new(manifest, dir.to_path_buf()), shared }
     }
 
+    #[cfg(unix)]
+    #[test]
+    fn writes_recheck_approved_paths_after_symlink_substitution() {
+        use std::os::unix::fs::symlink;
+        let tmp = tempfile::tempdir().unwrap();
+        let api = api(tmp.path(), Arc::default());
+        let root = tmp.path().join("project");
+        std::fs::create_dir_all(root.join("nested")).unwrap();
+        let path = ide_plugin::resolve_writable_in_root(&root, "nested/file.txt").unwrap();
+        let outside = tmp.path().join("outside.txt");
+        std::fs::write(&outside, "unchanged").unwrap();
+        symlink(&outside, &path).unwrap();
+        assert!(api.write_text_file(&path, "text").is_err());
+        assert!(api.write_bytes_file(&path, b"bytes").is_err());
+        assert_eq!(std::fs::read_to_string(&outside).unwrap(), "unchanged");
+
+        std::fs::remove_file(&path).unwrap();
+        std::fs::remove_dir(root.join("nested")).unwrap();
+        symlink(tmp.path(), root.join("nested")).unwrap();
+        assert!(api.write_text_file(&path, "text").is_err());
+        assert!(api.write_bytes_file(&path, b"bytes").is_err());
+        assert!(!tmp.path().join("file.txt").exists());
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn storage_and_secrets_do_not_write_through_temporary_symlinks() {
+        use std::os::unix::fs::symlink;
+        let tmp = tempfile::tempdir().unwrap();
+        let api = api(tmp.path(), Arc::default());
+        let outside = tmp.path().join("outside.txt");
+        std::fs::write(&outside, "unchanged").unwrap();
+        let dir = api.storage().unwrap();
+        let entry = dir.join(entry_name("key"));
+        symlink(&outside, entry.with_extension("tmp")).unwrap();
+        symlink(&outside, &entry).unwrap();
+        api.storage_set("key", Some("value".into())).unwrap();
+        assert_eq!(api.storage_get("key").unwrap().as_deref(), Some("value"));
+        assert_eq!(std::fs::read_to_string(&outside).unwrap(), "unchanged");
+
+        let secrets = FileSecretStore { dir: tmp.path().join("secrets") };
+        secrets.set("acme.lint", "key", Some("before")).unwrap();
+        symlink(&outside, secrets.file("acme.lint").with_extension("tmp")).unwrap();
+        secrets.set("acme.lint", "key", Some("after")).unwrap();
+        assert_eq!(secrets.get("acme.lint", "key").as_deref(), Some("after"));
+        assert_eq!(std::fs::read_to_string(&outside).unwrap(), "unchanged");
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn storage_and_secrets_refuse_symlinked_private_directories() {
+        let tmp = tempfile::tempdir().unwrap();
+        let api = api(tmp.path(), Arc::default());
+        let dir = api.storage().unwrap();
+        std::fs::remove_dir(&dir).unwrap();
+        let outside = tmp.path().join("outside");
+        std::fs::create_dir(&outside).unwrap();
+        std::os::unix::fs::symlink(&outside, &dir).unwrap();
+        assert!(api.storage_set("key", Some("value".into())).is_err());
+        let secrets = FileSecretStore { dir: tmp.path().join("secrets") };
+        std::fs::create_dir(&secrets.dir).unwrap();
+        std::os::unix::fs::symlink(&outside, secrets.dir.join("acme.lint")).unwrap();
+        assert!(secrets.set("acme.lint", "key", Some("value")).is_err());
+        assert_eq!(std::fs::read_dir(outside).unwrap().count(), 0);
+    }
+
     #[cfg(windows)]
     #[test]
     fn private_directory_matches_resolved_write_paths() {

crates/ide-host/tests/plugins.rs

@@ -542,6 +542,30 @@ fn project_write_stays_inside_the_project() {
     assert_eq!((info.status, info.stats.crashes, info.stats.errors), (PluginStatus::Active, 0, 0), "{info:?}");
 }
 
+#[cfg(unix)]
+#[test]
+fn project_writes_confine_symlink_targets() {
+    use std::os::unix::fs::symlink;
+    let f = fixture(Limits::default());
+    f.install(&misbehave_with(f._tmp.path(), "misbehave-symlink-writer", "project.write"));
+    let outside = f._tmp.path().join("secret.txt");
+    let missing = f._tmp.path().join("missing.txt");
+    symlink(&outside, f.project.join("outside.txt")).unwrap();
+    symlink(&missing, f.project.join("dangling.txt")).unwrap();
+    for name in ["outside.txt", "dangling.txt"] {
+        let error = f.run("misbehave", "projwrite", Some(name)).unwrap_err();
+        assert!(error.contains("access outside the project root is not allowed"), "{error}");
+    }
+    assert_eq!(std::fs::read_to_string(outside).unwrap(), "top secret");
+    assert!(!missing.exists());
+    for (name, target) in [("inside.txt", "src/notes.txt"), ("new.txt", "new/deep.txt")] {
+        symlink(target, f.project.join(name)).unwrap();
+        f.run("misbehave", "projwrite", Some(name)).unwrap();
+        assert_eq!(std::fs::read_to_string(f.project.join(target)).unwrap(), "written inside the project\n");
+        assert!(std::fs::symlink_metadata(f.project.join(name)).unwrap().is_symlink());
+    }
+}
+
 /// `fs.write` replaces files the grant covers and nothing else, through the
 /// real component.
 #[test]

crates/ide-plugin/src/lib.rs

@@ -20,7 +20,10 @@ pub use runtime::{
     API_VERSION, CANCELLED, CallCancel, CallError, GuestError, HostApi, PluginEngine, Runtime, RuntimeSetup, check_api,
     component_api, host_api,
 };
-pub use sandbox::{MAX_READ_BYTES, inside_root, read_text, resolve_in_root, resolve_new_in_root, resolve_real};
+pub use sandbox::{
+    MAX_READ_BYTES, inside_root, read_text, resolve_in_root, resolve_new_in_root, resolve_real,
+    resolve_writable_in_root,
+};
 pub use types::*;
 pub use wasmtime::component::Component;
 

crates/ide-plugin/src/sandbox.rs

@@ -117,6 +117,45 @@ pub fn resolve_new_in_root(root: &Path, path: &str) -> Result<PathBuf, String> {
     Ok(real)
 }
 
+/// Resolves a content write's destination, including existing and dangling
+/// symlinks in the final component. Entry operations use [`resolve_new_in_root`].
+pub fn resolve_writable_in_root(root: &Path, path: &str) -> Result<PathBuf, String> {
+    let (root, joined) = joined_path(root, path)?;
+    let real = resolve_write_target(&joined, 40).map_err(|e| format!("{path}: {e}"))?;
+    if !real.starts_with(&root) {
+        return Err(format!("{path}: access outside the project root is not allowed"));
+    }
+    Ok(real)
+}
+
+fn resolve_write_target(path: &Path, links_left: usize) -> Result<PathBuf, std::io::Error> {
+    match dunce::canonicalize(path) {
+        Ok(real) => return Ok(real),
+        Err(error) => match std::fs::symlink_metadata(path) {
+            Ok(meta) if meta.is_symlink() => {
+                if links_left == 0 {
+                    return Err(std::io::Error::other("too many symbolic links"));
+                }
+                let target = std::fs::read_link(path)?;
+                let target = if target.is_absolute() { target } else { path.parent().unwrap().join(target) };
+                return resolve_write_target(&target, links_left - 1);
+            }
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
+            _ => return Err(error),
+        },
+    }
+    let parent = path.parent().ok_or_else(|| std::io::Error::other("no directory to put it in"))?;
+    let mut real = resolve_write_target(parent, links_left)?;
+    match path.components().next_back() {
+        Some(std::path::Component::ParentDir) => {
+            real.pop();
+        }
+        Some(std::path::Component::Normal(name)) => real.push(name),
+        _ => {}
+    }
+    if real != path { resolve_write_target(&real, links_left) } else { Ok(real) }
+}
+
 /// Reads a UTF-8 file that `resolve_in_root` already approved.
 pub fn read_text(real: &Path) -> Result<String, String> {
     let meta = std::fs::metadata(real).map_err(|e| e.to_string())?;
@@ -194,9 +233,9 @@ pub fn resolve_granted(granted: &[Capability], path: &str) -> Result<PathBuf, Re
 /// The real path `path` would be written to, when an `fs.write:<path>` grant
 /// covers it.
 ///
-/// A file that does not exist yet cannot be canonicalized, so its directory is
-/// canonicalized instead and the name appended; an existing file (or link) is
-/// followed first, so a link cannot be used to write outside the grant.
+/// Missing paths and dangling links are resolved to their target location;
+/// an existing file (or link) is followed first. The destination's directory
+/// must exist.
 pub fn resolve_writable(granted: &[Capability], path: &str) -> Result<PathBuf, Refusal> {
     resolve(granted, path, Wanted::Write)
 }
@@ -241,12 +280,14 @@ fn resolve(granted: &[Capability], path: &str, wanted: Wanted) -> Result<PathBuf
     if covered { Ok(real) } else { Err(Refusal::Denied(cap.to_string())) }
 }
 
-/// Where `asked` would be created: its directory, resolved, plus its name.
+/// Where `asked` would be created, including a dangling link's target.
 fn resolve_unsaved(asked: &Path) -> Result<PathBuf, String> {
-    let name = asked.file_name().ok_or_else(|| "not a file name".to_string())?;
-    let parent = asked.parent().ok_or_else(|| "no directory to put it in".to_string())?;
-    let dir = dunce::canonicalize(parent).map_err(|_| "its directory does not exist".to_string())?;
-    Ok(dir.join(name))
+    let real = resolve_write_target(asked, 40).map_err(|e| e.to_string())?;
+    let parent = real.parent().ok_or_else(|| "no directory to put it in".to_string())?;
+    if !parent.is_dir() {
+        return Err("its directory does not exist".into());
+    }
+    Ok(real)
 }
 
 /// Simplifies Windows drive paths only when the plain form preserves their meaning.
@@ -387,6 +428,82 @@ mod tests {
         assert!(resolve_new_in_root(&root, "out/x.txt").unwrap_err().contains("outside"));
     }
 
+    #[cfg(unix)]
+    #[test]
+    fn writable_paths_follow_existing_and_dangling_symlinks() {
+        use std::os::unix::fs::symlink;
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path().join("proj");
+        std::fs::create_dir(&root).unwrap();
+        let root = dunce::canonicalize(root).unwrap();
+        let outside = tmp.path().join("outside.txt");
+        std::fs::write(&outside, "unchanged").unwrap();
+        std::fs::write(root.join("inside.txt"), "before").unwrap();
+        for (name, target) in [
+            ("outside", outside.clone()),
+            ("dangling-outside", tmp.path().join("missing.txt")),
+            ("inside", PathBuf::from("inside.txt")),
+            ("dangling-inside", PathBuf::from("new/deep.txt")),
+            ("chain", PathBuf::from("dangling-outside")),
+            ("cycle", PathBuf::from("cycle")),
+        ] {
+            symlink(target, root.join(name)).unwrap();
+        }
+        for name in ["outside", "dangling-outside", "chain"] {
+            assert!(resolve_writable_in_root(&root, name).unwrap_err().contains("outside the project root"));
+            assert!(
+                resolve_writable_in_root(&root, &format!("missing/../{name}"))
+                    .unwrap_err()
+                    .contains("outside the project root")
+            );
+        }
+        assert!(resolve_writable_in_root(&root, "cycle").is_err());
+        let inside = resolve_writable_in_root(&root, "inside").unwrap();
+        assert_eq!(inside, root.join("inside.txt"));
+        std::fs::write(inside, "after").unwrap();
+        assert_eq!(std::fs::read_to_string(root.join("inside.txt")).unwrap(), "after");
+        assert_eq!(resolve_writable_in_root(&root, "dangling-inside").unwrap(), root.join("new/deep.txt"));
+        assert_eq!(std::fs::read_to_string(outside).unwrap(), "unchanged");
+        assert!(!tmp.path().join("missing.txt").exists());
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn writable_grants_confine_dangling_symlink_targets() {
+        use std::os::unix::fs::symlink;
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path().join("granted");
+        std::fs::create_dir(&root).unwrap();
+        let root = dunce::canonicalize(root).unwrap();
+        let grant = Capability::FsWrite(strip_verbatim(&root));
+        symlink("../missing.txt", root.join("outside")).unwrap();
+        symlink("new.txt", root.join("inside")).unwrap();
+        assert!(matches!(
+            resolve_writable(std::slice::from_ref(&grant), root.join("outside").to_str().unwrap()),
+            Err(Refusal::Denied(_))
+        ));
+        assert_eq!(resolve_writable(&[grant], root.join("inside").to_str().unwrap()).unwrap(), root.join("new.txt"));
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn entry_operations_do_not_follow_outside_symlinks() {
+        let tmp = tempfile::tempdir().unwrap();
+        let root = tmp.path().join("proj");
+        std::fs::create_dir(&root).unwrap();
+        let outside = tmp.path().join("outside.txt");
+        std::fs::write(&outside, "unchanged").unwrap();
+        std::os::unix::fs::symlink(&outside, root.join("link")).unwrap();
+        let from = resolve_new_in_root(&root, "link").unwrap();
+        let to = resolve_new_in_root(&root, "renamed").unwrap();
+        std::fs::rename(&from, &to).unwrap();
+        assert!(!from.try_exists().unwrap());
+        assert!(std::fs::symlink_metadata(&to).unwrap().is_symlink());
+        std::fs::remove_file(resolve_new_in_root(&root, "renamed").unwrap()).unwrap();
+        assert!(std::fs::symlink_metadata(&to).is_err());
+        assert_eq!(std::fs::read_to_string(outside).unwrap(), "unchanged");
+    }
+
     #[cfg(unix)]
     #[test]
     fn symlinks_cannot_escape() {

crates/ide-plugin/src/wit_host.rs

@@ -761,7 +761,7 @@ impl workspace::HostProject for StoreData {
             return Ok(Err(too_much_to_write(&path)));
         }
         let root = self.table.get(&this)?.root.clone();
-        let real = match sandbox::resolve_new_in_root(&root, &path) {
+        let real = match sandbox::resolve_writable_in_root(&root, &path) {
             Ok(real) => real,
             Err(e) => return Ok(Err(wit::Error::Invalid(e))),
         };

e2e/updateProject.spec.ts

@@ -1,73 +0,0 @@
-import { expect, ipcCalls, stubDesktop, test } from "./fixtures";
-
-test("shows Update failures from the branch menu even when VCS notifications are muted", async ({
-  workbench: page,
-}) => {
-  await stubDesktop(page);
-  await page.evaluate(async () => {
-    const root = "/dev/acme-api";
-    const branch = { head: "main", upstream: "origin/main", behind: 68, ahead: 0, operation: null, headCommit: "c1" };
-    const branches = [
-      {
-        name: "main",
-        current: true,
-        remote: false,
-        upstream: "origin/main",
-        behind: 68,
-        ahead: 0,
-        commit: "c1",
-        lastCommitMs: 0,
-      },
-    ];
-    const h = window.__ipc.handlers;
-    h.vcs_branch_info = () => branch;
-    h.vcs_branches = () => branches;
-    h.vcs_roots = () => [{ root, head: "main", headCommit: "c1", operation: null }];
-    h.vcs_status = () => [];
-    h.vcs_log = () => [];
-    h.vcs_tags = () => [];
-    h.vcs_config_get = () => null;
-    h.vcs_merge_base = () => "c1";
-    h.vcs_remote = () => {
-      throw {
-        kind: "invalid",
-        message:
-          "The following untracked working tree files would be overwritten by merge: e2e/settingsFormLayout.spec.ts",
-      };
-    };
-    const projectModule = "/src/platform/project.ts";
-    const { useProject } = await import(/* @vite-ignore */ projectModule);
-    useProject.setState({
-      settings: {
-        ...useProject.getState().settings,
-        notifications: { groups: { vcs: { display: "none", log: true } } },
-      },
-    });
-    const storeModule = "/src/features/vcs/store.ts";
-    const { useVcs } = await import(/* @vite-ignore */ storeModule);
-    useVcs.setState({
-      available: true,
-      root,
-      repo: root,
-      roots: [root],
-      branch,
-      branches,
-      branchesLoaded: true,
-    });
-    const contextModule = "/src/platform/context.ts";
-    const { setContext } = await import(/* @vite-ignore */ contextModule);
-    setContext("vcsAvailable", true);
-    const actionsModule = "/src/platform/actions.ts";
-    const { executeAction } = await import(/* @vite-ignore */ actionsModule);
-    await executeAction("vcs.showLog");
-  });
-  const branches = page.getByRole("tree", { name: "Branches" });
-  await branches.getByRole("treeitem", { name: /main current/ }).click({ button: "right" });
-  await page.getByRole("menuitem", { name: "Update", exact: true }).click();
-  const dialog = page.getByRole("dialog", { name: "Update Project", exact: true });
-  await expect(dialog).toBeVisible();
-  await dialog.getByRole("button", { name: "OK", exact: true }).click();
-  await expect(page.getByText("Update failed", { exact: true })).toBeVisible();
-  await expect(page.getByText(/untracked working tree files would be overwritten by merge/)).toBeVisible();
-  expect(await ipcCalls(page, "vcs_remote")).toHaveLength(1);
-});

src/features/vcs/__tests__/remoteResults.test.ts

@@ -1,11 +1,9 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
-import type { BranchInfo, RemoteOp } from "@/bindings";
+import type { BranchInfo } from "@/bindings";
 
 let branch: Partial<BranchInfo> = {};
 /** What a pull brings in: the head moves to `c2`, with these changed files and commits. */
 let pulled = { files: 0, commits: 0 };
-let failure: { kind: string; message: string } | null = null;
-let remoteGate: Promise<void> | null = null;
 
 vi.mock("@tauri-apps/api/core", async (importOriginal) => ({
   ...(await importOriginal<typeof import("@tauri-apps/api/core")>()),
@@ -25,12 +23,9 @@ vi.mock("@/platform/ipc", async (importOriginal) => {
         data: { head: "main", headCommit: "c1", upstream: null, ahead: 0, behind: 0, operation: null, ...branch },
       }),
       vcsRemote: async () => {
-        await remoteGate;
-        if (failure) return { status: "error", error: failure };
         if (pulled.commits) branch = { ...branch, headCommit: "c2" };
         return { status: "ok", data: null };
       },
-      vcsOperation: async () => (failure ? { status: "error", error: failure } : { status: "ok", data: null }),
       vcsDiffRevisions: async () => ({ status: "ok", data: Array.from({ length: pulled.files }, (_, i) => ({ i })) }),
       vcsLog: async () => ({ status: "ok", data: Array.from({ length: pulled.commits }, (_, i) => ({ i })) }),
       cancelRequest: async () => ({ status: "ok", data: null }),
@@ -40,10 +35,7 @@ vi.mock("@/platform/ipc", async (importOriginal) => {
 
 import { _resetI18nForTests, setLocale, txTemplate } from "@/platform/i18n";
 import { provideNotificationService } from "@/workbench/contributions";
-import { useProject } from "@/platform/project";
-import { expireNotification, notify, useNotifications } from "@/workbench/notificationStore";
-import { runGitOperation, runOp, runRemote } from "../ops";
-import { useVcs } from "../store";
+import { runRemote } from "../ops";
 
 type Note = { title: string; level: string };
 let notes: Note[] = [];
@@ -56,11 +48,6 @@ beforeEach(() => {
 afterEach(() => {
   branch = {};
   pulled = { files: 0, commits: 0 };
-  failure = null;
-  remoteGate = null;
-  useVcs.setState({ remoteBusy: null });
-  useNotifications.setState({ items: [] });
-  useProject.setState({ settings: null });
   _resetI18nForTests();
 });
 
@@ -118,97 +105,3 @@ describe("Update Project result notification", () => {
     expect(txTemplate(await updateTitle())).toBe("已在 2 个提交中更新 3 个文件");
   });
 });
-
-describe.each([
-  ["VCS popups disabled", { groups: { vcs: { display: "none", log: true } } }],
-  ["all balloons disabled", { balloons: false }],
-  ["VCS popups and logging disabled", { groups: { vcs: { display: "none", log: false } } }],
-])("Git command feedback with %s", (_name, prefs) => {
-  beforeEach(() => {
-    useProject.setState({ settings: { notifications: prefs } as never });
-    provideNotificationService({ notify, expire: expireNotification });
-  });
-
-  it("shows the untracked-file error that prevented Update", async () => {
-    failure = {
-      kind: "invalid",
-      message:
-        "The following untracked working tree files would be overwritten by merge: e2e/settingsFormLayout.spec.ts",
-    };
-    expect(await runRemote({ op: "pull", rebase: false }, undefined, ["/p"])).toBe(false);
-    expect(useNotifications.getState().items.find((n) => n.title === "Update failed")).toMatchObject({
-      balloon: true,
-      body: failure.message,
-    });
-  });
-
-  it.each<[RemoteOp, string]>([
-    [{ op: "fetch" }, "Fetch successful"],
-    [{ op: "push", force_with_lease: false, set_upstream: false, tags: null, no_verify: false }, "Push successful"],
-    [{ op: "pull", rebase: false }, "All files are up-to-date"],
-  ])("shows progress and the result of $op", async (op, title) => {
-    let finish!: () => void;
-    remoteGate = new Promise((resolve) => (finish = resolve));
-    const running = runRemote(op, undefined, ["/p"]);
-    try {
-      expect(useNotifications.getState().items.find((n) => n.key === "vcs.remote")).toMatchObject({
-        balloon: true,
-        sticky: true,
-      });
-    } finally {
-      finish();
-    }
-    expect(await running).toBe(true);
-    expect(useNotifications.getState().items.find((n) => n.title === title)?.balloon).toBe(true);
-  });
-
-  it("shows the files and commits that Update brought in", async () => {
-    pulled = { files: 3, commits: 2 };
-    expect(await runRemote({ op: "pull", rebase: false }, undefined, ["/p"])).toBe(true);
-    expect(useNotifications.getState().items.find((n) => n.title === "3 files updated in 2 commits")?.balloon).toBe(
-      true,
-    );
-  });
-
-  it("explains why another remote operation cannot start", async () => {
-    useVcs.setState({ remoteBusy: "Fetching" });
-    expect(await runRemote({ op: "fetch" })).toBe(false);
-    expect(useNotifications.getState().items[0]).toMatchObject({
-      title: "Another remote operation is in progress",
-      balloon: true,
-    });
-  });
-
-  it("keeps cancelled and caller-handled failures quiet", async () => {
-    failure = { kind: "cancelled", message: "cancelled" };
-    expect(await runRemote({ op: "fetch" })).toBe(false);
-    expect(useNotifications.getState().items.some((n) => n.balloon)).toBe(false);
-    failure = { kind: "invalid", message: "rejected" };
-    expect(await runRemote({ op: "fetch" }, undefined, ["/p"], () => true)).toBe(false);
-    expect(useNotifications.getState().items.some((n) => n.balloon)).toBe(false);
-  });
-
-  it("shows local command failures but respects quiet callers", async () => {
-    const fail = () => Promise.reject(new Error("index locked"));
-    await runOp("Commit", fail);
-    expect(useNotifications.getState().items[0]).toMatchObject({ title: "Commit failed", balloon: true });
-    useNotifications.setState({ items: [] });
-    await runOp("Commit", fail, { quiet: true });
-    expect(useNotifications.getState().items).toEqual([]);
-  });
-
-  it("shows long local operation results", async () => {
-    const op = { op: "cherryPick", commits: ["c1"] } as const;
-    const options = { progress: "Cherry-picking", success: "Cherry-picked c1", failure: "Cherry-pick failed" };
-    expect(await runGitOperation({ ...op, commits: [...op.commits] }, options)).toBe(true);
-    expect(useNotifications.getState().items[0]).toMatchObject({ title: options.success, balloon: true });
-    failure = { kind: "invalid", message: "invalid revision" };
-    expect(await runGitOperation({ ...op, commits: [...op.commits] }, options)).toBe(false);
-    expect(useNotifications.getState().items[0]).toMatchObject({ title: options.failure, balloon: true });
-  });
-
-  it("still suppresses unsolicited VCS notifications", () => {
-    notify({ group: "vcs", level: "info", title: "Background refresh" });
-    expect(useNotifications.getState().items.some((n) => n.balloon)).toBe(false);
-  });
-});

src/features/vcs/ops.ts

@@ -9,7 +9,7 @@ import { executeAction } from "@/platform/actions";
 import { tx } from "@/platform/i18n";
 import { call, commands, isCancelled, nextRequestId } from "@/platform/ipc";
 import { attachDocument, editorsFor } from "@/editor/api";
-import { maybeEditorService, notifications, type NotificationService } from "@/workbench/contributions";
+import { maybeEditorService, notifications } from "@/workbench/contributions";
 import { cancelTask, upsertTaskSoon, useTasks } from "@/workbench/tasks";
 import { progressFraction } from "./model";
 import { beginGitWork, conflictedFiles, currentRepo, refreshNow, repoName, useVcs } from "./store";
@@ -55,11 +55,6 @@ export function errorMessage(e: unknown): string {
   return e instanceof Error ? e.message : String(e);
 }
 
-/** Feedback for an explicit Git command stays visible when background VCS notifications are muted. */
-function notify(n: Omit<Parameters<NotificationService["notify"]>[0], "group" | "userInitiated">) {
-  notifications().notify({ ...n, group: "vcs", userInitiated: true });
-}
-
 // ---------------------------------------------------------------------------
 // Local operations
 
@@ -85,7 +80,7 @@ export async function runOp<T>(title: string, fn: () => Promise<T>, opts: OpOpti
     const msg = typeof opts.success === "function" ? opts.success(value) : opts.success;
     if (msg) {
       logConsole("info", msg);
-      notify({ level: "info", title: msg });
+      notifications().notify({ group: "vcs", level: "info", title: msg });
     }
     return value;
   } catch (e) {
@@ -93,7 +88,8 @@ export async function runOp<T>(title: string, fn: () => Promise<T>, opts: OpOpti
     logConsole("err", message);
     const handled = opts.onError ? await opts.onError(message) : false;
     if (!opts.quiet && !handled)
-      notify({
+      notifications().notify({
+        group: "vcs",
         level: "error",
         title: tx`${title} failed`,
         body: message,
@@ -164,7 +160,7 @@ export async function runRemote(
   handled?: (message: string) => boolean,
 ): Promise<boolean> {
   if (useVcs.getState().remoteBusy) {
-    notify({ level: "warning", title: "Another remote operation is in progress" });
+    notifications().notify({ group: "vcs", level: "warning", title: "Another remote operation is in progress" });
     return false;
   }
   const [progressText, doneTitle] = REMOTE_TITLE[op.op];
@@ -177,7 +173,8 @@ export async function runRemote(
   const task = (message: string | null, fraction: number | null, done: boolean) =>
     upsertTaskSoon({ id: taskId, title: progressTitle, message, fraction, done, cancel });
   task(detail ?? null, null, false);
-  notify({
+  notifications().notify({
+    group: "vcs",
     level: "info",
     key: "vcs.remote",
     title: `${progressTitle}…`,
@@ -234,7 +231,8 @@ export async function runRemote(
     else {
       // One repository's push names what went out; a partial multi-repo push stays generic.
       const one = pushed.length === 1 ? pushed[0] : null;
-      notify({
+      notifications().notify({
+        group: "vcs",
         level: "info",
         key: "vcs.remote",
         title:
@@ -258,7 +256,8 @@ export async function runRemote(
       notifications().expire?.("vcs.remote");
       void executeAction("vcs.resolveConflicts");
     } else
-      notify({
+      notifications().notify({
+        group: "vcs",
         level: "error",
         key: "vcs.remote",
         title: `${doneTitle} failed`,
@@ -320,7 +319,7 @@ const pushedTitle = (n: number, to: string) =>
 /** IDEA's Update Project result: "N files updated in M commits" with View Commits. */
 async function notifyUpdated(updated: readonly UpdatedRange[]) {
   if (!updated.length) {
-    notify({ level: "info", key: "vcs.remote", title: "All files are up-to-date" });
+    notifications().notify({ group: "vcs", level: "info", key: "vcs.remote", title: "All files are up-to-date" });
     return;
   }
   let files = 0;
@@ -345,7 +344,8 @@ async function notifyUpdated(updated: readonly UpdatedRange[]) {
       commits += log.length;
     }),
   );
-  notify({
+  notifications().notify({
+    group: "vcs",
     level: "info",
     key: "vcs.remote",
     title: `${filesText(files)} updated in ${commitsText(commits)}`,
@@ -389,7 +389,7 @@ export async function runGitOperation(
   repo = currentRepo(),
 ): Promise<boolean> {
   if (useVcs.getState().remoteBusy) {
-    notify({ level: "warning", title: "Another Git operation is in progress" });
+    notifications().notify({ group: "vcs", level: "warning", title: "Another Git operation is in progress" });
     return false;
   }
   await saveAll();
@@ -402,7 +402,8 @@ export async function runGitOperation(
   const task = (message: string | null, done: boolean) =>
     upsertTaskSoon({ id: taskId, title: progress, message, fraction: done ? 1 : null, done, cancel });
   task(null, false);
-  notify({
+  notifications().notify({
+    group: "vcs",
     level: "info",
     key: "vcs.operation",
     title: `${progress}…`,
@@ -424,7 +425,7 @@ export async function runGitOperation(
     }
     logConsole("info", success);
     const actions = opts.successActions ? await opts.successActions() : undefined;
-    notify({ level: "info", key: "vcs.operation", title: success, actions });
+    notifications().notify({ group: "vcs", level: "info", key: "vcs.operation", title: success, actions });
     return true;
   } catch (e) {
     const cancelled = isCancelled(e);
@@ -434,7 +435,8 @@ export async function runGitOperation(
     await refreshNow();
     const handled = cancelled || (opts.onError ? await opts.onError(message) : false);
     if (!handled)
-      notify({
+      notifications().notify({
+        group: "vcs",
         level: "error",
         key: "vcs.operation",
         title: opts.failure,