Skip to content

Commit fc917f3

Browse files
committed
test: regression + multi-client coverage for workspace-root routing
- Storage: migration FK-cascade survival (oauth_tokens/client preserved across the 005/006/012 rebuilds), synthetic DROP-with-FK-off, post-migration foreign_key_check integrity. - Core: normalize_workspace_root idempotency, full-path case-fold, file:// drive/UNC/localhost host handling, and the doubled-slash-before-drive regression. - Resolver: roots-arrived-empty → ClientGrant (and → Deny without grants). - FeatureSet composition cycle terminates and returns the de-duplicated union. - Multi-client: same root → identical effective tools, doubled-slash root → canonical binding (effective_features); two roots-capable clients tracked independently over real HTTP (streamable_http; test client now answers roots/list). - Prune redundant cases (duplicate token-hash / disable-server tests, empty resolver test module) and fix stale longest-prefix comments + the content-dedup test (measure delta around the unchanged event, not absolute count, so the legitimate connect-time resolution flip doesn't mask it). Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
1 parent d74fa87 commit fc917f3

6 files changed

Lines changed: 256 additions & 52 deletions

File tree

tests/rust/tests/database/inbound_client.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -302,16 +302,8 @@ async fn test_authorization_code_not_found() {
302302
// Token Tests
303303
// =============================================================================
304304

305-
#[tokio::test]
306-
async fn test_token_hash_consistency() {
307-
let hash1 = InboundClientRepository::hash_token("my_secret_token");
308-
let hash2 = InboundClientRepository::hash_token("my_secret_token");
309-
let hash3 = InboundClientRepository::hash_token("different_token");
310-
311-
assert_eq!(hash1, hash2);
312-
assert_ne!(hash1, hash3);
313-
assert_eq!(hash1.len(), 64); // SHA-256 hex
314-
}
305+
// Note: `hash_token` determinism/length is unit-tested in the storage crate
306+
// (`inbound_client_repository.rs::test_hash_token`); not duplicated here.
315307

316308
#[tokio::test]
317309
async fn test_save_and_find_token() {

tests/rust/tests/gateway/server_manager.rs

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -169,38 +169,10 @@ async fn test_disable_increments_flow_id() {
169169
// Event Emission
170170
// ============================================================================
171171

172-
#[tokio::test]
173-
async fn test_enable_emits_server_status_changed() {
174-
let mut harness = ServerManagerTestHarness::new().await;
175-
let key = test_key("server-1");
176-
177-
harness.manager.enable_server(key.clone()).await.unwrap();
178-
179-
let events = harness.collect_events().await;
180-
181-
let status_changed = events.iter().find(|e| {
182-
matches!(e, DomainEvent::ServerStatusChanged { server_id, .. } if server_id == "server-1")
183-
});
184-
185-
assert!(
186-
status_changed.is_some(),
187-
"Should emit ServerStatusChanged event"
188-
);
189-
}
190-
191-
#[tokio::test]
192-
async fn test_disable_emits_disconnected_event() {
193-
let mut harness = ServerManagerTestHarness::new().await;
194-
let key = test_key("server-1");
195-
196-
harness.manager.enable_server(key.clone()).await.unwrap();
197-
harness.collect_events().await;
198-
199-
harness.manager.disable_server(&key).await.unwrap();
200-
201-
let events = harness.collect_events().await;
202-
assert_event_status(&events, "server-1", ConnectionStatus::Disconnected);
203-
}
172+
// Note: enable→Connecting and disable→Disconnected event emission are
173+
// covered by `test_enable_server_starts_connecting` and
174+
// `test_disable_server_transitions_to_disconnected` above (which assert the
175+
// specific status); only the space-id correctness case is unique here.
204176

205177
#[tokio::test]
206178
async fn test_event_contains_correct_space_id() {

tests/rust/tests/integration/effective_features.rs

Lines changed: 115 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@
1919
use std::sync::Arc;
2020

2121
use mcpmux_core::{
22-
normalize_workspace_root, FeatureSet, FeatureSetRepository, MemberMode, ServerFeature,
23-
ServerFeatureRepository, SpaceRepository, WorkspaceBinding, WorkspaceBindingRepository,
22+
normalize_workspace_root, FeatureSet, FeatureSetMember, FeatureSetRepository, MemberMode,
23+
MemberType, ServerFeature, ServerFeatureRepository, SpaceRepository, WorkspaceBinding,
24+
WorkspaceBindingRepository,
2425
};
2526
use mcpmux_gateway::services::{FeatureSetResolverService, ResolutionSource, SessionRootsRegistry};
2627
use mcpmux_gateway::{FeatureService, PrefixCacheService};
@@ -36,12 +37,16 @@ struct Ctx {
3637
feature_service: FeatureService,
3738
session_roots: Arc<SessionRootsRegistry>,
3839
binding_repo: Arc<dyn WorkspaceBindingRepository>,
40+
fs_repo: Arc<dyn FeatureSetRepository>,
3941
space_id: Uuid,
4042
space_id_str: String,
4143
/// FeatureSet whose members are the two `github` tools.
4244
fs_github: String,
4345
/// FeatureSet whose only member is the `firebase` tool.
4446
fs_firebase: String,
47+
/// Raw ServerFeature ids for composing custom FeatureSets in tests.
48+
gh_issue_id: String,
49+
fb_deploy_id: String,
4550
}
4651

4752
impl Ctx {
@@ -117,10 +122,13 @@ impl Ctx {
117122
feature_service,
118123
session_roots,
119124
binding_repo,
125+
fs_repo,
120126
space_id,
121127
space_id_str,
122128
fs_github: fs_github.id,
123129
fs_firebase: fs_firebase.id,
130+
gh_issue_id: gh_issue.id.to_string(),
131+
fb_deploy_id: fb_deploy.id.to_string(),
124132
}
125133
}
126134

@@ -176,6 +184,59 @@ async fn mapping_determines_effective_tools_per_session() {
176184
);
177185
}
178186

187+
/// Multi-client, SAME workspace root: two distinct client sessions (e.g. two
188+
/// editors opening the same folder) must resolve to the SAME binding and see
189+
/// an identical toolset — the root is the routing key, not the session/client.
190+
#[tokio::test(flavor = "multi_thread")]
191+
async fn two_clients_same_root_see_identical_tools() {
192+
let ctx = Ctx::new().await;
193+
let root = if cfg!(windows) {
194+
"d:\\work\\shared"
195+
} else {
196+
"/work/shared"
197+
};
198+
// First client opens the folder and binds it.
199+
ctx.bind("client-a", root, &ctx.fs_github).await;
200+
// Second client (different session) reports the very same root.
201+
ctx.session_roots.set("client-b", [root]);
202+
ctx.session_roots.set_roots_capable("client-b", true);
203+
204+
let a = ctx.effective_tools("client-a").await;
205+
let b = ctx.effective_tools("client-b").await;
206+
assert_eq!(
207+
a,
208+
vec!["create_issue".to_string(), "list_repos".to_string()]
209+
);
210+
assert_eq!(
211+
a, b,
212+
"two clients on the same root must see identical tools"
213+
);
214+
}
215+
216+
/// Multi-client, DIFFERENT-shaped roots for the SAME folder: a binding created
217+
/// from the canonical Windows drive path must still match a client that
218+
/// reports that folder with a doubled leading slash (`//d:/…`, seen live from
219+
/// a `file:////D:/…` URI). Regression for the normalization bug that stored
220+
/// `\\d:\…` and silently never matched the canonical form.
221+
#[tokio::test(flavor = "multi_thread")]
222+
async fn doubled_slash_root_resolves_to_canonical_binding() {
223+
let ctx = Ctx::new().await;
224+
// Binding created from the canonical drive form.
225+
ctx.bind("canon", "d:\\work\\proj", &ctx.fs_github).await;
226+
227+
// A second client reports the SAME folder via the doubled-slash form;
228+
// SessionRootsRegistry::set normalizes it, and it must collapse to the
229+
// canonical key so the resolver matches the existing binding.
230+
ctx.session_roots.set("dslash", ["//d:/work/proj"]);
231+
ctx.session_roots.set_roots_capable("dslash", true);
232+
233+
assert_eq!(
234+
ctx.effective_tools("dslash").await,
235+
vec!["create_issue".to_string(), "list_repos".to_string()],
236+
"doubled-slash root must route to the binding created from the canonical form"
237+
);
238+
}
239+
179240
/// An *empty* mapping (a binding with zero feature sets) is valid: the session
180241
/// routes to the Space (source = WorkspaceBinding) but sees zero Space tools.
181242
/// Built-in servers (gated per Space) are layered on by the request handler and
@@ -221,3 +282,55 @@ async fn unbound_session_sees_zero_effective_tools() {
221282

222283
assert!(ctx.effective_tools("sess").await.is_empty());
223284
}
285+
286+
/// Regression (resolution #9): a composition CYCLE (FS X ⊇ Y, FS Y ⊇ X) must
287+
/// not infinite-loop the resolver on the live tools/list path. The command
288+
/// layer now blocks creating such a cycle, but the repository can still hold
289+
/// one (legacy data / direct writes), so the resolver must terminate
290+
/// defensively — returning the de-duplicated union of both sets' features.
291+
#[tokio::test(flavor = "multi_thread")]
292+
async fn composition_cycle_terminates_and_returns_union() {
293+
let ctx = Ctx::new().await;
294+
295+
// Two custom FeatureSets that include each other.
296+
let mut fs_x = FeatureSet::new_custom("CycX", ctx.space_id.to_string());
297+
let mut fs_y = FeatureSet::new_custom("CycY", ctx.space_id.to_string());
298+
ctx.fs_repo.create(&fs_x).await.unwrap();
299+
ctx.fs_repo.create(&fs_y).await.unwrap();
300+
301+
let member = |fs_id: &str, mtype: MemberType, mid: String| FeatureSetMember {
302+
id: Uuid::new_v4().to_string(),
303+
feature_set_id: fs_id.to_string(),
304+
member_type: mtype,
305+
member_id: mid,
306+
mode: MemberMode::Include,
307+
};
308+
309+
// X ⊇ {gh_issue (feature), Y (featureset)}
310+
fs_x.members = vec![
311+
member(&fs_x.id, MemberType::Feature, ctx.gh_issue_id.clone()),
312+
member(&fs_x.id, MemberType::FeatureSet, fs_y.id.clone()),
313+
];
314+
// Y ⊇ {fb_deploy (feature), X (featureset)} ← closes the cycle
315+
fs_y.members = vec![
316+
member(&fs_y.id, MemberType::Feature, ctx.fb_deploy_id.clone()),
317+
member(&fs_y.id, MemberType::FeatureSet, fs_x.id.clone()),
318+
];
319+
ctx.fs_repo.update(&fs_x).await.unwrap();
320+
ctx.fs_repo.update(&fs_y).await.unwrap();
321+
322+
let root = if cfg!(windows) {
323+
"d:\\work\\cyc"
324+
} else {
325+
"/work/cyc"
326+
};
327+
ctx.bind("sess", root, &fs_x.id).await;
328+
329+
// Must return (not hang) — the union of both sets' features, de-duplicated.
330+
let tools = ctx.effective_tools("sess").await;
331+
assert_eq!(
332+
tools,
333+
vec!["create_issue".to_string(), "deploy".to_string()],
334+
"cyclic composition must resolve to the de-duplicated union and terminate"
335+
);
336+
}

tests/rust/tests/integration/feature_set_resolver.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,45 @@ async fn rootless_client_without_grants_denies() {
276276
assert!(r.feature_set_ids.is_empty());
277277
}
278278

279+
#[tokio::test]
280+
async fn roots_arrived_empty_falls_through_to_grants() {
281+
// Regression (resolver 3.1): a roots-capable client whose roots arrived
282+
// EMPTY (no folder open — Claude Desktop chat, empty editor window) is a
283+
// SETTLED rootless answer and must fall through to its client grants, not
284+
// hang forever in PendingRoots. Before the fix, `Some([])` was conflated
285+
// with `None` (not-yet-arrived) and stranded granted clients on
286+
// meta-tools-only with no recovery short of opening a folder.
287+
let f = Fixture::new().await;
288+
let client_id = "folderless.example/client";
289+
f.make_client(client_id).await;
290+
f.client_repo
291+
.grant_feature_set(client_id, &f.space_id.to_string(), &f.fs_a_id)
292+
.await
293+
.unwrap();
294+
295+
f.session_roots.set_roots_capable("s", true);
296+
f.session_roots.set("s", Vec::<String>::new()); // roots ARRIVED, but empty
297+
let r = f
298+
.resolver
299+
.resolve(Some("s"), Some(client_id))
300+
.await
301+
.unwrap();
302+
assert_eq!(r.source, ResolutionSource::ClientGrant);
303+
assert_eq!(r.feature_set_ids, vec![f.fs_a_id]);
304+
}
305+
306+
#[tokio::test]
307+
async fn roots_arrived_empty_without_grants_denies() {
308+
// Same arrived-empty state but no grants → Deny, NOT PendingRoots, so the
309+
// session settles instead of re-probing `roots/list` forever.
310+
let f = Fixture::new().await;
311+
f.session_roots.set_roots_capable("s", true);
312+
f.session_roots.set("s", Vec::<String>::new());
313+
let r = f.resolver.resolve(Some("s"), None).await.unwrap();
314+
assert_eq!(r.source, ResolutionSource::Deny);
315+
assert!(r.feature_set_ids.is_empty());
316+
}
317+
279318
#[tokio::test]
280319
async fn capable_session_does_not_fall_through_to_grants() {
281320
// Critical: the leak we set out to fix. A roots-capable session whose

tests/rust/tests/integration/workspace_binding_events.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ impl Ctx {
8080
async fn creating_binding_flips_next_resolution_source() {
8181
let ctx = Ctx::new().await;
8282

83-
// Normalize both sides so the longest-prefix lookup matches — the
84-
// resolver compares already-normalized strings from both stores.
83+
// Normalize both sides so the exact-match lookup matches — the resolver
84+
// compares already-normalized strings from both stores (no inheritance).
8585
let raw = if cfg!(windows) {
8686
"d:\\proj\\bind-me"
8787
} else {

0 commit comments

Comments
 (0)