Merge pull request #5918 from Hmbown/fix/plugin-esm-multifile-launch

fix(mcp): launch multi-file Node ESM plugin servers by staged path on macOS

Author: Hmbown

crates/tui/src/mcp.rs

@@ -679,23 +679,46 @@ impl ReviewedPluginMcpSource {
         if Path::new(command).is_absolute() {
             launch.bind_command(staged_root, Path::new(command), &validated.file_hashes)?;
         }
+        // Darwin cannot hand Node an ESM entry by descriptor when that entry
+        // imports sibling modules: `/dev/fd/N` has no directory, so every
+        // relative specifier resolves against `/dev/` (#5916). Such an entry
+        // keeps its staged path — its bytes are still hash-verified by
+        // `bind_file` below, and the siblings were always read by path.
+        #[cfg(target_os = "macos")]
+        let esm_entry_index = is_node_command(command)
+            .then(|| {
+                args.iter().position(|argument| {
+                    let path = Path::new(argument);
+                    path.is_absolute()
+                        && path.starts_with(staged_root)
+                        && path.extension().is_some_and(|extension| extension == "mjs")
+                })
+            })
+            .flatten();
+        #[cfg(target_os = "macos")]
+        let esm_entry_keeps_path = esm_entry_index.is_some_and(|index| {
+            esm_entry_has_module_siblings(
+                staged_root,
+                Path::new(&args[index]),
+                &validated.file_hashes,
+            )
+        });
         for (index, argument) in args.iter().enumerate() {
             let path = Path::new(argument);
             if path.is_absolute() && path.starts_with(staged_root) && path.is_file() {
-                launch.args[index] = launch.bind_file(staged_root, path, &validated.file_hashes)?;
+                let bound = launch.bind_file(staged_root, path, &validated.file_hashes)?;
+                #[cfg(target_os = "macos")]
+                if esm_entry_keeps_path && esm_entry_index == Some(index) {
+                    continue;
+                }
+                launch.args[index] = bound;
             }
         }
         #[cfg(target_os = "macos")]
-        if is_node_command(command) {
-            let entry_index = args.iter().position(|argument| {
-                let path = Path::new(argument);
-                path.is_absolute()
-                    && path.starts_with(staged_root)
-                    && path.extension().is_some_and(|extension| extension == "mjs")
-            });
-            if let Some(entry_index) = entry_index {
-                launch.args = node_esm_descriptor_args(&launch.args, entry_index);
-            }
+        if let Some(entry_index) = esm_entry_index
+            && !esm_entry_keeps_path
+        {
+            launch.args = node_esm_descriptor_args(&launch.args, entry_index);
         }
         if let Some(cwd) = cwd {
             if !cwd.starts_with(staged_root) {
@@ -759,6 +782,30 @@ impl ReviewedPluginMcpSource {
     }
 }
 
+/// Whether a reviewed `.mjs` entry shares its stage with other module files.
+/// Such an entry must launch by path: Node resolves its relative imports
+/// against the entry's own URL, and a `/dev/fd/N` URL has no directory to
+/// resolve against (#5916). Manifests and data files do not count.
+#[cfg(target_os = "macos")]
+fn esm_entry_has_module_siblings(
+    staged_root: &Path,
+    entry: &Path,
+    file_hashes: &std::collections::BTreeMap<PathBuf, String>,
+) -> bool {
+    let Ok(entry) = entry.strip_prefix(staged_root) else {
+        return false;
+    };
+    file_hashes.keys().any(|path| {
+        path != entry
+            && path.extension().is_some_and(|extension| {
+                matches!(
+                    extension.to_string_lossy().as_ref(),
+                    "mjs" | "js" | "cjs" | "node" | "wasm"
+                )
+            })
+    })
+}
+
 #[cfg(target_os = "macos")]
 fn is_node_command(command: &str) -> bool {
     Path::new(command)

crates/tui/src/mcp/stdio.rs

@@ -321,10 +321,17 @@ impl McpTransport for StdioTransport {
                     }
                 };
             if bytes == 0 {
+                // Let the stderr drain task catch up before snapshotting, and
+                // name the exit status: a reviewed plugin's stderr is never
+                // retained, so the status is the only reason the operator
+                // gets when the child dies before the handshake (#5916).
+                tokio::task::yield_now().await;
+                let exit = self.child.lock().await.try_wait().ok().flatten();
+                let exit = exit.map_or_else(String::new, |status| format!(" ({status})"));
                 if let Some(stderr) = format_stderr_context(&self.stderr_tail).await {
-                    anyhow::bail!("Stdio transport closed\n{stderr}");
+                    anyhow::bail!("Stdio transport closed{exit}\n{stderr}");
                 }
-                anyhow::bail!("Stdio transport closed");
+                anyhow::bail!("Stdio transport closed{exit}");
             }
 
             let line = String::from_utf8_lossy(&line_bytes);

crates/tui/src/mcp/tests.rs

@@ -1121,6 +1121,155 @@ connect_timeout = 2
     assert_eq!(connection.tools()[0].name, "ready");
 }
 
+#[cfg(target_os = "macos")]
+#[tokio::test]
+async fn reviewed_multi_file_node_mjs_plugin_launches_by_staged_path() {
+    if std::process::Command::new("node")
+        .arg("--version")
+        .output()
+        .is_err()
+    {
+        eprintln!("skipping reviewed multi-file Node ESM launch test because node is unavailable");
+        return;
+    }
+
+    // The entry imports a sibling module, exactly like the computer-use
+    // bundle (#5916). Launched by descriptor, Node would resolve `./lib/...`
+    // against `/dev/` and the child would die before the handshake.
+    let dir = tempfile::tempdir().unwrap();
+    let plugins_root = dir.path().join("plugins");
+    let plugin_base = plugins_root.join("node-esm-multi");
+    fs::create_dir_all(plugin_base.join("mcp")).unwrap();
+    fs::create_dir_all(plugin_base.join("lib")).unwrap();
+    fs::write(
+        plugin_base.join("lib").join("reply.mjs"),
+        r#"import path from 'node:path';
+import url from 'node:url';
+export const TOOL = 'ready-from-sibling';
+export const ENTRY_DIR = path.basename(path.dirname(url.fileURLToPath(import.meta.url)));
+"#,
+    )
+    .unwrap();
+    fs::write(
+        plugin_base.join("mcp").join("server.mjs"),
+        r#"import readline from 'node:readline';
+import { TOOL, ENTRY_DIR } from '../lib/reply.mjs';
+const lines = readline.createInterface({ input: process.stdin });
+lines.on('line', (line) => {
+  const request = JSON.parse(line);
+  if (request.id === undefined) return;
+  let result;
+  if (request.method === 'initialize') {
+    result = {
+      protocolVersion: '2025-06-18',
+      capabilities: { tools: {} },
+      serverInfo: { name: 'node-esm-multi', version: '1.0.0' }
+    };
+  } else if (request.method === 'tools/list') {
+    result = {
+      tools: [{ name: TOOL, description: ENTRY_DIR, inputSchema: { type: 'object' } }]
+    };
+  } else {
+    result = {};
+  }
+  process.stdout.write(JSON.stringify({ jsonrpc: '2.0', id: request.id, result }) + '\n');
+});
+"#,
+    )
+    .unwrap();
+    fs::write(
+        plugin_base.join("plugin.toml"),
+        r#"
+schema_version = 1
+[plugin]
+name = "node-esm-multi"
+version = "1.0.0"
+
+[mcp_servers.local]
+command = "node"
+args = ["mcp/server.mjs"]
+connect_timeout = 2
+"#,
+    )
+    .unwrap();
+
+    let discovery = crate::plugins::discovery::DiscoveryConfig {
+        workspace: dir.path().join("project"),
+        user_plugins_dir: plugins_root,
+        workspace_plugins_dir: dir.path().join("workspace-plugins-unused"),
+        builtin_plugin_dirs: Vec::new(),
+        state_path: dir.path().join("plugin-state/state.json"),
+    };
+    let mut registry = crate::plugins::discovery::discover_with_config(&discovery);
+    registry.trust("node-esm-multi").unwrap();
+    registry.enable("node-esm-multi").unwrap();
+    let active = registry.active_plugins()[0].clone();
+    let authority = registry.authority_for("node-esm-multi").unwrap();
+    let merged = merge_plugin_mcp_servers_from_plugins(
+        McpConfig::default(),
+        vec![("node-esm-multi".to_string(), active, authority)],
+    )
+    .unwrap();
+    let mut pool = McpPool::new(merged);
+
+    let connection = pool
+        .get_or_connect("plugin-14-node-esm-multi-local")
+        .await
+        .unwrap();
+    assert_eq!(connection.tools().len(), 1);
+    assert_eq!(connection.tools()[0].name, "ready-from-sibling");
+    // The sibling resolved from the staged tree, not from `/dev/`.
+    assert_eq!(connection.tools()[0].description.as_deref(), Some("lib"));
+}
+
+#[cfg(target_os = "macos")]
+#[test]
+fn esm_entry_with_module_siblings_keeps_its_staged_path() {
+    use std::collections::BTreeMap;
+    let staged_root = Path::new("/stage/plugin");
+    let entry = staged_root.join("mcp/server.mjs");
+    let hash = |paths: &[&str]| {
+        paths
+            .iter()
+            .map(|path| (PathBuf::from(path), "h".to_string()))
+            .collect::<BTreeMap<_, _>>()
+    };
+    // Manifests, docs, and data files are not modules.
+    assert!(!esm_entry_has_module_siblings(
+        staged_root,
+        &entry,
+        &hash(&[
+            "mcp/server.mjs",
+            "plugin.json",
+            "mcp.json",
+            "README.md",
+            "skills/a/SKILL.md"
+        ]),
+    ));
+    for sibling in [
+        "src/tools.mjs",
+        "lib/x.js",
+        "lib/x.cjs",
+        "native/x.node",
+        "wasm/x.wasm",
+    ] {
+        assert!(
+            esm_entry_has_module_siblings(
+                staged_root,
+                &entry,
+                &hash(&["mcp/server.mjs", "plugin.json", sibling]),
+            ),
+            "{sibling} must force a path launch"
+        );
+    }
+    // An entry outside the stage never qualifies.
+    assert!(!esm_entry_has_module_siblings(
+        Path::new("/elsewhere"),
+        &entry,
+        &hash(&["mcp/server.mjs", "src/tools.mjs"]),
+    ));
+}
+
 #[cfg(target_os = "macos")]
 #[test]
 fn node_esm_descriptor_launch_keeps_options_argv_shape_and_script_arguments() {