Skip to content

Commit d5d698d

Browse files
its-mashMohammod Al Amin Ashik
authored andcommitted
fix(P2): locked client honors in-Space retargeted clientId mapping
Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
1 parent 7df9736 commit d5d698d

2 files changed

Lines changed: 112 additions & 6 deletions

File tree

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

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -303,13 +303,15 @@ impl FeatureSetResolverService {
303303
}
304304

305305
/// Resolve a client locked to Space `locked`. The Space is fixed to
306-
/// `locked`; the header/roots may still pick the FeatureSet, but only when
307-
/// their binding lives in `locked`. A header whose binding resolves to a
308-
/// different Space — or no header at all — falls back to `locked`'s Starter.
309-
/// A locked client never touches the roots-pending or client-grant tiers.
306+
/// `locked`; the header/roots — or the client's own retargeted clientId
307+
/// mapping — may still pick the FeatureSet, but only when that binding lives
308+
/// in `locked`. A binding that resolves to a different Space (or no binding
309+
/// at all) falls back to `locked`'s Starter. A locked client never touches
310+
/// the roots-pending or client-grant tiers.
310311
async fn resolve_locked(
311312
&self,
312313
session_id: Option<&str>,
314+
client_id: Option<&str>,
313315
locked: Uuid,
314316
) -> Result<ResolvedFeatureSet> {
315317
if let Some(sid) = session_id {
@@ -337,7 +339,35 @@ impl FeatureSetResolverService {
337339
}
338340
}
339341
}
340-
// No header, or its binding is outside the locked Space → locked Starter.
342+
// A locked client may still have its clientId-keyed `id` mapping
343+
// retargeted from the Mapping tab. Honor that mapping's FeatureSet — but
344+
// only when it stays within the locked Space, preserving
345+
// lock-confinement. A mapping pointing out of the Space (or none) falls
346+
// through to the locked Starter.
347+
if let Some(cid) = client_id {
348+
if let Some(binding) = self.binding_repo.find_by_id_key(cid).await? {
349+
if binding.space_id == locked {
350+
debug!(
351+
%locked,
352+
client_id = %cid,
353+
"[FeatureSetResolver] locked client — clientId mapping within locked Space",
354+
);
355+
return Ok(ResolvedFeatureSet {
356+
feature_set_ids: binding.feature_set_ids,
357+
space_id: Some(locked),
358+
source: ResolutionSource::WorkspaceBinding,
359+
});
360+
}
361+
debug!(
362+
%locked,
363+
client_id = %cid,
364+
binding_space = %binding.space_id,
365+
"[FeatureSetResolver] locked client — clientId mapping in a different Space; ignored",
366+
);
367+
}
368+
}
369+
370+
// No header/mapping within the locked Space → locked Starter.
341371
self.default_fallback(locked).await
342372
}
343373

@@ -378,7 +408,11 @@ impl FeatureSetResolverService {
378408
if let Some(cid) = client_id {
379409
if let Some(locked) = self.client_repo.get_locked_space(cid).await? {
380410
match locked.parse::<Uuid>() {
381-
Ok(locked_uuid) => return self.resolve_locked(session_id, locked_uuid).await,
411+
Ok(locked_uuid) => {
412+
return self
413+
.resolve_locked(session_id, client_id, locked_uuid)
414+
.await
415+
}
382416
Err(e) => warn!(
383417
client_id = %cid,
384418
locked_space = %locked,

tests/rust/tests/integration/feature_set_resolver.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -975,3 +975,75 @@ async fn locked_client_with_no_header_gets_locked_space_starter() {
975975
assert_eq!(r.source, ResolutionSource::SpaceDefault);
976976
assert_eq!(r.feature_set_ids, vec![f.starter_fs_id]);
977977
}
978+
979+
#[tokio::test]
980+
async fn locked_client_honors_in_space_retargeted_client_id_mapping() {
981+
// A locked client with no header: its clientId-keyed mapping was retargeted
982+
// (from the auto-created Starter) to another FeatureSet that still lives in
983+
// the locked Space. The resolver must honor that mapping — the Space stays
984+
// locked, but the operator's FeatureSet choice is respected.
985+
let f = Fixture::new().await;
986+
f.make_client("locked-4").await;
987+
f.client_repo
988+
.set_locked_space("locked-4", Some(&f.space_id.to_string()))
989+
.await
990+
.unwrap();
991+
// Retargeted clientId mapping → a non-Starter FS within the locked Space.
992+
f.binding_repo
993+
.create(&WorkspaceBinding::new_id(
994+
"locked-4",
995+
f.space_id,
996+
vec![f.fs_a_id.clone()],
997+
))
998+
.await
999+
.unwrap();
1000+
// No header; explicitly rootless so we reach the clientId tier.
1001+
f.session_roots.set_roots_capable("s", false);
1002+
let r = f
1003+
.resolver
1004+
.resolve(Some("s"), Some("locked-4"))
1005+
.await
1006+
.unwrap();
1007+
assert_eq!(r.source, ResolutionSource::WorkspaceBinding);
1008+
assert_eq!(r.space_id, Some(f.space_id));
1009+
assert_eq!(r.feature_set_ids, vec![f.fs_a_id]);
1010+
}
1011+
1012+
#[tokio::test]
1013+
async fn locked_client_ignores_out_of_space_client_id_mapping() {
1014+
// A locked client whose clientId mapping points at a DIFFERENT Space must be
1015+
// confined to the locked Space: the out-of-Space mapping is ignored and the
1016+
// client falls back to the locked Space's Starter (lock-confinement holds).
1017+
let f = Fixture::new().await;
1018+
f.make_client("locked-5").await;
1019+
let other_base = if cfg!(windows) {
1020+
"d:\\elsewhere"
1021+
} else {
1022+
"/elsewhere"
1023+
};
1024+
let (other_space, other_starter) = f.make_space_with_base_dir("Elsewhere", other_base).await;
1025+
f.client_repo
1026+
.set_locked_space("locked-5", Some(&f.space_id.to_string()))
1027+
.await
1028+
.unwrap();
1029+
// clientId mapping points OUT of the locked Space.
1030+
f.binding_repo
1031+
.create(&WorkspaceBinding::new_id(
1032+
"locked-5",
1033+
other_space,
1034+
vec![other_starter],
1035+
))
1036+
.await
1037+
.unwrap();
1038+
// No header; explicitly rootless so we reach the clientId tier.
1039+
f.session_roots.set_roots_capable("s", false);
1040+
let r = f
1041+
.resolver
1042+
.resolve(Some("s"), Some("locked-5"))
1043+
.await
1044+
.unwrap();
1045+
// Confined to the locked (default) Space; the foreign mapping is ignored.
1046+
assert_eq!(r.space_id, Some(f.space_id));
1047+
assert_eq!(r.source, ResolutionSource::SpaceDefault);
1048+
assert_eq!(r.feature_set_ids, vec![f.starter_fs_id]);
1049+
}

0 commit comments

Comments
 (0)