fix(gateway): close PR 11 review gaps on pins, reconnect, and probe

Empty headers no longer keep a sibling window's pin, and window pins now require a one-folder set. Also lock reconnect_fresh, quote the hook path, and stop the env-probe from dumping secrets.

Signed-off-by: crimsonsunset <jsangio1@gmail.com>

Author: crimsonsunset

crates/mcpmux-gateway/src/cursor_hook.rs

@@ -47,8 +47,20 @@ fn script_path() -> Result<PathBuf, String> {
     Ok(cursor_dir()?.join("hooks").join(SCRIPT_NAME))
 }
 
+fn shell_quote(path: &Path) -> String {
+    let raw = path.to_string_lossy();
+    #[cfg(windows)]
+    {
+        format!("\"{}\"", raw.replace('"', "\"\""))
+    }
+    #[cfg(not(windows))]
+    {
+        format!("'{}'", raw.replace('\'', "'\\''"))
+    }
+}
+
 fn hook_command(script: &Path) -> String {
-    format!("node {}", script.display())
+    format!("node {}", shell_quote(script))
 }
 
 fn managed_entry(script: &Path) -> Value {
@@ -308,3 +320,92 @@ pub fn uninstall() -> CursorHookResult {
         manual_entry: manual_entry_text(&script),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use serde_json::Value;
+    use std::path::PathBuf;
+
+    fn script() -> PathBuf {
+        PathBuf::from("/Users/Test User/.cursor/hooks/mcpmux-workspace-context.js")
+    }
+
+    #[test]
+    fn hook_command_quotes_spaces() {
+        let cmd = hook_command(&script());
+        assert!(
+            cmd.contains("'") || cmd.contains('"'),
+            "path with a space must be quoted: {cmd}"
+        );
+        assert!(cmd.contains(SCRIPT_NAME));
+    }
+
+    #[test]
+    fn merge_preserves_unrelated_hooks() {
+        let existing = r#"{
+            "version": 1,
+            "hooks": {
+                "preToolUse": [
+                    { "command": "npx wakatime-hook", "matcher": ".*" }
+                ]
+            }
+        }"#;
+        let out = merge_pre_tool_use(Some(existing), &script()).unwrap();
+        let v: Value = serde_json::from_str(&out).unwrap();
+        let list = v["hooks"]["preToolUse"].as_array().unwrap();
+        assert_eq!(list.len(), 2);
+        assert_eq!(list[0]["command"], "npx wakatime-hook");
+        assert!(list[1]["command"].as_str().unwrap().contains(SCRIPT_NAME));
+    }
+
+    #[test]
+    fn merge_replaces_managed_entry_once() {
+        let existing = merge_pre_tool_use(None, &script()).unwrap();
+        let again = merge_pre_tool_use(Some(&existing), &script()).unwrap();
+        let v: Value = serde_json::from_str(&again).unwrap();
+        assert_eq!(v["hooks"]["preToolUse"].as_array().unwrap().len(), 1);
+    }
+
+    #[test]
+    fn merge_rejects_jsonc_and_non_object_shapes() {
+        let script = script();
+        assert!(merge_pre_tool_use(Some("{ // comment\n}"), &script).is_err());
+        assert!(merge_pre_tool_use(Some("[1]"), &script).is_err());
+        assert!(merge_pre_tool_use(Some(r#"{ "hooks": [] }"#), &script).is_err());
+        assert!(merge_pre_tool_use(Some(r#"{ "hooks": { "preToolUse": {} } }"#), &script).is_err());
+    }
+
+    #[test]
+    fn uninstall_keeps_other_entries() {
+        let existing = merge_pre_tool_use(
+            Some(r#"{ "hooks": { "preToolUse": [{ "command": "other" }] } }"#),
+            &script(),
+        )
+        .unwrap();
+        let removed = remove_managed_entry(&existing, &script()).unwrap();
+        let v: Value = serde_json::from_str(&removed).unwrap();
+        let list = v["hooks"]["preToolUse"].as_array().unwrap();
+        assert_eq!(list.len(), 1);
+        assert_eq!(list[0]["command"], "other");
+    }
+
+    #[test]
+    fn write_with_backup_copies_existing() {
+        let tmp = std::env::temp_dir().join(format!(
+            "mcpmux-hook-{}-{}",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos()
+        ));
+        std::fs::create_dir_all(&tmp).unwrap();
+        let path = tmp.join("hooks.json");
+        std::fs::write(&path, "{}").unwrap();
+        let bak = write_with_backup(&path, "{\"ok\":true}").unwrap();
+        assert!(bak.is_some());
+        assert_eq!(std::fs::read_to_string(&path).unwrap(), "{\"ok\":true}");
+        let _ = std::fs::remove_dir_all(&tmp);
+    }
+}

crates/mcpmux-gateway/src/mcp/oauth_middleware.rs

@@ -481,37 +481,40 @@ fn attach_window_identity(services: &ServiceContainer, session_id: &str, peer: O
 
 /// Promote an explicit session pin to the window, or inherit a remembered one.
 ///
-/// The empty-header warn only fires when inheritance also failed — a reused
-/// window pin is the recovery, not another prompt.
+/// An empty header is a failed substitution, not "reuse the last pin." Drop
+/// the session claim and skip window inherit so a sibling window on the
+/// shared `mcp-session-id` cannot keep resolving through the previous root.
 fn resolve_window_pin(
     services: &ServiceContainer,
     session_id: &str,
     empty_workspace_header: bool,
     trace_id: &str,
 ) {
-    if services.session_roots.session_pin(session_id).is_some() {
-        services.session_roots.promote_pin_to_window(session_id);
-        return;
-    }
-    if services
-        .session_roots
-        .inherit_window_pin(session_id)
-        .is_some()
-    {
-        return;
-    }
     if empty_workspace_header {
+        services.session_roots.forget_empty_header_claim(session_id);
         warn!(
             trace_id = %trace_id,
             session_id,
-            "[SessionRoots] X-Mcpmux-Workspace present but empty — pin skipped \
+            "[SessionRoots] X-Mcpmux-Workspace present but empty — pin cleared \
              (Cursor did not substitute ${{workspaceFolder}} before spawning \
              mcp-remote, which then expanded the unresolved literal to empty). \
              Affects editor and Agents windows alike; recover with \
              mcpmux_set_workspace_root, or install a per-repo static header to \
              avoid substitution entirely — \
              see docs/manual/cursor-workspace-bridge.md Fallback",
         );
+        return;
+    }
+    if services.session_roots.session_pin(session_id).is_some() {
+        services.session_roots.promote_pin_to_window(session_id);
+        return;
+    }
+    if services
+        .session_roots
+        .inherit_window_pin(session_id)
+        .is_some()
+    {
+        return;
     }
 }
 

crates/mcpmux-gateway/src/pool/service.rs

@@ -277,19 +277,24 @@ impl PoolService {
         }
     }
 
+    /// Per-(space, server) lock shared by connect and reconnect_fresh.
+    fn connect_lock(&self, space_id: Uuid, server_id: &str) -> Arc<tokio::sync::Mutex<()>> {
+        self.connect_locks
+            .entry((space_id, server_id.to_string()))
+            .or_insert_with(|| Arc::new(tokio::sync::Mutex::new(())))
+            .clone()
+    }
+
     /// Connect a server for a space
     pub async fn connect_server(&self, ctx: &ConnectionContext) -> ConnectionResult {
-        let key = (ctx.space_id, ctx.server_id.to_string());
-
-        // Single-flight per key: the late caller waits, then reuses the
-        // winner's instance via the health check below instead of racing it.
-        // (Clone the Arc out so the DashMap entry guard drops before .await.)
-        let connect_lock = self
-            .connect_locks
-            .entry(key.clone())
-            .or_insert_with(|| Arc::new(tokio::sync::Mutex::new(())))
-            .clone();
+        let connect_lock = self.connect_lock(ctx.space_id, &ctx.server_id);
         let _connect_guard = connect_lock.lock().await;
+        self.connect_server_locked(ctx).await
+    }
+
+    /// Body of [`Self::connect_server`] after the per-key lock is held.
+    async fn connect_server_locked(&self, ctx: &ConnectionContext) -> ConnectionResult {
+        let key = (ctx.space_id, ctx.server_id.to_string());
 
         // Check for existing instance. Clone the Arc out of the DashMap so
         // no shard guard is held across the reconnect `.await` below.
@@ -482,8 +487,10 @@ impl PoolService {
     /// `reconnect_after_oauth` and will not mis-route stdio backends to HTTP.
     pub async fn reconnect_fresh(&self, ctx: &ConnectionContext) -> ConnectionResult {
         let started = Instant::now();
+        let connect_lock = self.connect_lock(ctx.space_id, &ctx.server_id);
+        let _connect_guard = connect_lock.lock().await;
         self.remove_instance(ctx.space_id, &ctx.server_id);
-        let result = self.connect_server(ctx).await;
+        let result = self.connect_server_locked(ctx).await;
         info!(
             server_id = %ctx.server_id,
             space_id = %ctx.space_id,