Skip to content

Commit 7ac5dc1

Browse files
committed
fix(gateway): disambiguate multi-root sessions via filesystem existence
A phantom root (moved/deleted folder still reported by a stale client source, e.g. an orphaned background-agent worker) alongside a real one was holding sessions at PendingRoots indefinitely. If exactly one reported root still exists on disk, narrow to it and resolve normally instead of waiting on the client to pin a header. Genuine ambiguity — zero or multiple surviving roots — still holds at PendingRoots. Signed-off-by: crimsonsunset <jsangio1@gmail.com>
1 parent d88ee03 commit 7ac5dc1

2 files changed

Lines changed: 119 additions & 17 deletions

File tree

crates/mcpmux-gateway/src/services/feature_set_resolver.rs

Lines changed: 60 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
//! // Signal 1 — reported root (deprecated MCP primitive, SEP-2577)
1010
//! if session reported roots:
1111
//! if roots.len() > 1 (no pinned X-Mcpmux-Workspace header):
12-
//! return ([], default_space, PendingRoots) // ambiguous — never guess
12+
//! if exactly one reported root still exists on disk:
13+
//! narrow to that root and continue // disambiguated
14+
//! else:
15+
//! return ([], default_space, PendingRoots) // ambiguous — never guess
1316
//! if a binding matches the (single) root:
1417
//! return (binding.space_id, [binding.feature_set_id], WorkspaceBinding)
1518
//!
@@ -73,7 +76,16 @@
7376
//! Multi-root ambiguity is a separate, non-timed hold: when
7477
//! [`SessionRootsRegistry::get`](crate::services::session_roots::SessionRootsRegistry::get)
7578
//! returns more than one root (no pinned `X-Mcpmux-Workspace` header), the
76-
//! resolver stays at `PendingRoots` indefinitely until the client pins a
79+
//! resolver first checks whether the ambiguity is only apparent — clients can
80+
//! report roots from unrelated or stale sources alongside the caller's real
81+
//! workspace (e.g. an orphaned background-agent worker still pointed at a
82+
//! folder that was since moved or deleted). If exactly one reported root
83+
//! still exists on disk, the resolver narrows to that root and proceeds
84+
//! normally — filesystem existence, not binding presence, is the signal,
85+
//! since discarding a root just because it lacks a binding would silently
86+
//! route the request to a *different*, unrelated-but-bound root's
87+
//! FeatureSet. Otherwise — zero surviving roots, or more than one that still
88+
//! exists — it stays at `PendingRoots` indefinitely until the client pins a
7789
//! single root (header or `mcpmux_set_workspace_root`). Unlike the in-flight
7890
//! grace window, time alone cannot resolve which open folder the request
7991
//! belongs to — guessing would silently route to the wrong FeatureSet.
@@ -515,26 +527,57 @@ impl FeatureSetResolverService {
515527
// Tier 1: session reported roots — try an EXACT binding match
516528
// (no ancestor inheritance).
517529
if has_roots {
518-
let reported_roots = roots.expect("has_roots implies Some");
530+
let mut reported_roots = roots.expect("has_roots implies Some");
519531

520532
// Ambiguous multi-root session: SessionRootsRegistry::get() only
521533
// returns more than one entry when there's no pinned
522534
// X-Mcpmux-Workspace header collapsing it to a single root (see
523-
// SessionRootsRegistry::get). Never guess which open folder this
524-
// request belongs to — hold at PendingRoots (meta tools, incl.
525-
// mcpmux_set_workspace_root, remain reachable) until the client
526-
// pins one explicitly.
535+
// SessionRootsRegistry::get). Before giving up, check whether the
536+
// ambiguity is only apparent: clients can report roots from
537+
// unrelated or stale sources alongside the caller's real
538+
// workspace — e.g. an orphaned background-agent worker still
539+
// pointed at a folder that was since moved or deleted. If
540+
// exactly one reported root still exists on disk, narrow to it
541+
// and fall through to the normal Tier 1 lookup below.
542+
//
543+
// Filesystem existence — NOT binding presence — is the only
544+
// safe disambiguation signal here. Discarding a root just
545+
// because it lacks a binding would silently route the request
546+
// to a *different*, unrelated-but-bound root's FeatureSet
547+
// whenever two genuinely distinct open folders both got
548+
// reported together — exactly the cross-workspace bleed this
549+
// gate exists to prevent. A phantom root, by contrast, can
550+
// never itself hold or acquire a binding, so dropping it loses
551+
// no real ambiguity. Bounded to a handful of cheap local
552+
// `stat()` calls on this already-cold path.
527553
if reported_roots.len() > 1 {
528-
debug!(
529-
session_id = %sid,
530-
root_count = reported_roots.len(),
531-
"[FeatureSetResolver] multiple roots reported, no pinned header — PendingRoots",
532-
);
533-
return Ok(ResolvedFeatureSet {
534-
feature_set_ids: vec![],
535-
space_id: Some(deny_space_id),
536-
source: ResolutionSource::PendingRoots,
537-
});
554+
let existing_roots: Vec<String> = reported_roots
555+
.iter()
556+
.filter(|root| std::path::Path::new(root.as_str()).exists())
557+
.cloned()
558+
.collect();
559+
560+
if existing_roots.len() == 1 {
561+
debug!(
562+
session_id = %sid,
563+
root_count = reported_roots.len(),
564+
resolved_root = %existing_roots[0],
565+
"[FeatureSetResolver] disambiguated multi-root session — only one reported root exists on disk",
566+
);
567+
reported_roots = existing_roots;
568+
} else {
569+
debug!(
570+
session_id = %sid,
571+
root_count = reported_roots.len(),
572+
existing_count = existing_roots.len(),
573+
"[FeatureSetResolver] multiple roots reported, no pinned header — PendingRoots",
574+
);
575+
return Ok(ResolvedFeatureSet {
576+
feature_set_ids: vec![],
577+
space_id: Some(deny_space_id),
578+
source: ResolutionSource::PendingRoots,
579+
});
580+
}
538581
}
539582

540583
if let Some(binding) = self

tests/rust/tests/integration/feature_set_resolver.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,65 @@ async fn roots_capable_multi_root_session_unblocked_by_pinned_header() {
778778
assert_eq!(r.feature_set_ids, vec![f.fs_b_id]);
779779
}
780780

781+
#[tokio::test]
782+
async fn multi_root_session_disambiguated_by_phantom_root_no_longer_on_disk() {
783+
// Regression: Cursor can report a root from a stale/orphaned source (e.g.
784+
// a background-agent worker still pointed at a folder that was moved or
785+
// deleted) alongside the caller's real, live workspace. Since the
786+
// phantom root no longer exists on disk, the resolver narrows to the
787+
// real one instead of holding at PendingRoots forever.
788+
let f = Fixture::new().await;
789+
let real_dir = tempfile::tempdir().unwrap();
790+
let real_root = real_dir.path().to_str().unwrap();
791+
let phantom_root = real_dir.path().join("this-does-not-exist-on-disk");
792+
let phantom_root = phantom_root.to_str().unwrap();
793+
794+
f.binding_repo
795+
.create(&WorkspaceBinding::new(
796+
normalize_workspace_root(real_root),
797+
f.space_id,
798+
f.fs_a_id.clone(),
799+
))
800+
.await
801+
.unwrap();
802+
803+
f.session_roots.set("s", [phantom_root, real_root]);
804+
f.session_roots.set_roots_capable("s", true);
805+
let r = f.resolver.resolve(Some("s"), None, None).await.unwrap();
806+
assert_eq!(r.source, ResolutionSource::WorkspaceBinding);
807+
assert_eq!(r.feature_set_ids, vec![f.fs_a_id]);
808+
}
809+
810+
#[tokio::test]
811+
async fn multi_root_session_stays_pending_when_all_reported_roots_exist_on_disk() {
812+
// Counterpart to the phantom-root regression above: when every reported
813+
// root is a real, live folder (the ordinary multi-root-workspace case),
814+
// filesystem existence can't disambiguate anything — stay at
815+
// PendingRoots exactly like the no-binding-at-all case, even though one
816+
// of the two happens to be bound. Guards against silently leaking a
817+
// different, unrelated-but-bound folder's tools into this session.
818+
let f = Fixture::new().await;
819+
let dir_a = tempfile::tempdir().unwrap();
820+
let dir_b = tempfile::tempdir().unwrap();
821+
let root_a = dir_a.path().to_str().unwrap();
822+
let root_b = dir_b.path().to_str().unwrap();
823+
824+
f.binding_repo
825+
.create(&WorkspaceBinding::new(
826+
normalize_workspace_root(root_a),
827+
f.space_id,
828+
f.fs_a_id.clone(),
829+
))
830+
.await
831+
.unwrap();
832+
833+
f.session_roots.set("s", [root_a, root_b]);
834+
f.session_roots.set_roots_capable("s", true);
835+
let r = f.resolver.resolve(Some("s"), None, None).await.unwrap();
836+
assert_eq!(r.source, ResolutionSource::PendingRoots);
837+
assert!(r.feature_set_ids.is_empty());
838+
}
839+
781840
#[tokio::test]
782841
async fn rootless_client_without_grants_falls_back_to_default() {
783842
let f = Fixture::new().await;

0 commit comments

Comments
 (0)