Skip to content

Commit 42bfbf9

Browse files
committed
fix(gateway): drop mcpmux_describe_workspace meta tool
User feedback: the describe_* surface was redundant — `list_all_tools` and `list_feature_sets` already give an LLM enough to introspect the resolved state, and trimming the toolbar reduces visual noise on the client side. * Remove `DescribeWorkspaceTool` from `tools.rs` and its registration in `meta_tools/mod.rs`. * Drop the two `describe_workspace_*` integration tests; the resolver behavior they exercised is already covered by the `feature_set_resolver` integration suite. * Re-target the audit-emission test to `mcpmux_list_all_tools` (still a read tool, same `decision = "read"` audit path). * Update `registry_advertises_every_default_tool_with_annotations` to assert both describe_* tools are NOT advertised. Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
1 parent 02bd7b9 commit 42bfbf9

3 files changed

Lines changed: 17 additions & 170 deletions

File tree

crates/mcpmux-gateway/src/services/meta_tools/mod.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,9 @@ pub fn build_default_registry(
7777
// Reads — no approval needed.
7878
registry.register(Box::new(tools::ListAllToolsTool));
7979
registry.register(Box::new(tools::ListFeatureSetsTool));
80-
// `describe_workspace` also returns the resolution fields the older
81-
// `describe_resolution` tool used to expose — the split was confusing
82-
// for LLMs (two reads with overlapping output) and trimming it shrinks
83-
// the toolbar visible to the caller.
84-
registry.register(Box::new(tools::DescribeWorkspaceTool));
80+
// Both `describe_resolution` and `describe_workspace` were removed by
81+
// user request — the read surface is just the two list_* tools above,
82+
// which an LLM can stitch into the same picture without an extra hop.
8583
// Writes — gated by ApprovalBroker.
8684
registry.register(Box::new(tools::CreateFeatureSetTool));
8785
registry.register(Box::new(tools::BindCurrentWorkspaceTool));

crates/mcpmux-gateway/src/services/meta_tools/tools.rs

Lines changed: 0 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -154,95 +154,6 @@ impl MetaTool for ListFeatureSetsTool {
154154
}
155155
}
156156

157-
// ---------------------------------------------------------------------------
158-
// mcpmux_describe_workspace — read
159-
//
160-
// Single read endpoint that combines:
161-
// • workspace roots reported by the caller
162-
// • the matched WorkspaceBinding (if any)
163-
// • the resolved FeatureSet (id, name, source, tool count)
164-
// — replacing the older two-tool split where `describe_resolution` and
165-
// `describe_workspace` returned overlapping fragments.
166-
// ---------------------------------------------------------------------------
167-
168-
pub struct DescribeWorkspaceTool;
169-
170-
#[async_trait]
171-
impl MetaTool for DescribeWorkspaceTool {
172-
fn name(&self) -> &'static str {
173-
"mcpmux_describe_workspace"
174-
}
175-
176-
fn description(&self) -> &'static str {
177-
"Report everything that determines this caller's effective tool set: \
178-
the workspace roots declared via MCP `roots`, the matched \
179-
WorkspaceBinding (if any), and the resolved FeatureSet (id, name, \
180-
source, tool count). Always call this before a write tool so you \
181-
know the baseline."
182-
}
183-
184-
fn input_schema(&self) -> Value {
185-
json!({ "type": "object", "properties": {} })
186-
}
187-
188-
async fn call(&self, call: MetaToolCall<'_>) -> Result<CallToolResult, MetaToolError> {
189-
let space_id = caller_space_id(&call).await?;
190-
let roots = call
191-
.session_id
192-
.and_then(|sid| call.ctx.session_roots.get(sid))
193-
.unwrap_or_default();
194-
let matched = if !roots.is_empty() {
195-
call.ctx
196-
.binding_repo
197-
.find_longest_prefix_match(&space_id, &roots)
198-
.await?
199-
} else {
200-
None
201-
};
202-
203-
// Walk the resolver too — that's the authoritative answer for "which
204-
// FS would actually apply right now". A binding may exist but the
205-
// caller's resolution chain may still pick something else (e.g. a
206-
// session pin, when those exist again).
207-
let resolved = call.ctx.resolver.resolve(call.session_id).await?;
208-
let fs_id = resolved.feature_set_id.clone();
209-
let fs_name = if let Some(id) = fs_id.as_deref() {
210-
call.ctx.feature_set_repo.get(id).await?.map(|fs| fs.name)
211-
} else {
212-
None
213-
};
214-
let tool_count = if let Some(id) = fs_id.as_deref() {
215-
let resolved_space = resolved.space_id.unwrap_or(space_id);
216-
call.ctx
217-
.feature_service
218-
.get_tools_for_grants(&resolved_space.to_string(), &[id.to_string()])
219-
.await?
220-
.iter()
221-
.filter(|f| f.is_available)
222-
.count()
223-
} else {
224-
0
225-
};
226-
227-
Ok(text_result(json!({
228-
"space_id": space_id,
229-
"reported_roots": roots,
230-
"matched_binding": matched.map(|b| json!({
231-
"id": b.id,
232-
"workspace_root": b.workspace_root,
233-
"space_id": b.space_id,
234-
"feature_set_id": b.feature_set_id,
235-
})),
236-
"resolution": {
237-
"feature_set_id": fs_id,
238-
"feature_set_name": fs_name,
239-
"source": resolved.source,
240-
"resolved_tool_count": tool_count,
241-
},
242-
})))
243-
}
244-
}
245-
246157
// ---------------------------------------------------------------------------
247158
// Writes — each goes through the ApprovalBroker before mutating state.
248159
// ---------------------------------------------------------------------------

tests/rust/tests/integration/meta_tools.rs

Lines changed: 14 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -248,68 +248,11 @@ async fn list_feature_sets_returns_space_contents() {
248248
assert_eq!(sets.len(), 3, "Default + 2 custom expected");
249249
}
250250

251-
#[tokio::test(flavor = "multi_thread")]
252-
async fn describe_workspace_reports_default_resolution() {
253-
// With no bindings and no reported roots, the resolver falls through
254-
// to the Default tier and returns the space's auto-seeded
255-
// `fs_default_<space>` FS. `describe_workspace` surfaces this in its
256-
// `resolution` block (the field used to live on a separate
257-
// `describe_resolution` tool that we collapsed into this one).
258-
let f = Fixture::new().await;
259-
let result = f
260-
.registry
261-
.call(
262-
"mcpmux_describe_workspace",
263-
&f.client_id,
264-
Some(&f.session_id),
265-
json!({}),
266-
)
267-
.await
268-
.unwrap();
269-
let body = Fixture::result_json(&result);
270-
let resolution = body.get("resolution").unwrap();
271-
assert_eq!(
272-
resolution.get("source").unwrap().as_str().unwrap(),
273-
"default"
274-
);
275-
let fs_id = resolution.get("feature_set_id").unwrap().as_str().unwrap();
276-
assert!(
277-
fs_id.starts_with("fs_default_"),
278-
"default tier should surface the Default FS, got {fs_id}"
279-
);
280-
}
281-
282-
#[tokio::test(flavor = "multi_thread")]
283-
async fn describe_workspace_reports_reported_roots() {
284-
let f = Fixture::new().await;
285-
let path = if cfg!(windows) {
286-
"d:\\android\\myapp"
287-
} else {
288-
"/android/myapp"
289-
};
290-
f.session_roots.set(&f.session_id, [path]);
291-
292-
let result = f
293-
.registry
294-
.call(
295-
"mcpmux_describe_workspace",
296-
&f.client_id,
297-
Some(&f.session_id),
298-
json!({}),
299-
)
300-
.await
301-
.unwrap();
302-
let body = Fixture::result_json(&result);
303-
let roots = body.get("reported_roots").unwrap().as_array().unwrap();
304-
assert_eq!(roots.len(), 1);
305-
assert!(body.get("matched_binding").unwrap().is_null());
306-
// Resolution still reports the default tier — no binding matched.
307-
let resolution = body.get("resolution").unwrap();
308-
assert_eq!(
309-
resolution.get("source").unwrap().as_str().unwrap(),
310-
"default"
311-
);
312-
}
251+
// `describe_resolution` and `describe_workspace` were both removed at the
252+
// user's request — the read surface is now just `list_all_tools` and
253+
// `list_feature_sets`. Behavior previously asserted here is covered by
254+
// `FeatureSetResolverService`'s own tests in
255+
// `tests/rust/tests/integration/feature_set_resolver.rs`.
313256

314257
// ---------------------------------------------------------------------------
315258
// Writes — gated by ApprovalBroker
@@ -490,18 +433,18 @@ async fn registry_advertises_every_default_tool_with_annotations() {
490433
for expected in [
491434
"mcpmux_list_all_tools",
492435
"mcpmux_list_feature_sets",
493-
"mcpmux_describe_workspace",
494436
"mcpmux_create_feature_set",
495437
"mcpmux_bind_current_workspace",
496438
] {
497439
assert!(names.iter().any(|n| n == expected), "missing {expected}");
498440
}
499-
// describe_resolution was collapsed into describe_workspace — the
500-
// registry must NOT advertise it any more.
501-
assert!(
502-
!names.iter().any(|n| n == "mcpmux_describe_resolution"),
503-
"describe_resolution should be removed; got {names:?}"
504-
);
441+
// Both describe_* tools were removed — they must NOT be advertised.
442+
for removed in ["mcpmux_describe_resolution", "mcpmux_describe_workspace"] {
443+
assert!(
444+
!names.iter().any(|n| n == removed),
445+
"{removed} should be removed; got {names:?}"
446+
);
447+
}
505448
// Writes carry the destructive_hint annotation.
506449
let bind = tools
507450
.iter()
@@ -577,12 +520,7 @@ async fn read_tool_emits_meta_tool_invoked_with_decision_read() {
577520
let (registry, client_id, _tx, mut rx) = bare_registry(None).await;
578521

579522
registry
580-
.call(
581-
"mcpmux_describe_workspace",
582-
&client_id,
583-
Some("s"),
584-
json!({}),
585-
)
523+
.call("mcpmux_list_all_tools", &client_id, Some("s"), json!({}))
586524
.await
587525
.unwrap();
588526

@@ -596,7 +534,7 @@ async fn read_tool_emits_meta_tool_invoked_with_decision_read() {
596534
decision,
597535
..
598536
} => {
599-
assert_eq!(tool_name, "mcpmux_describe_workspace");
537+
assert_eq!(tool_name, "mcpmux_list_all_tools");
600538
assert_eq!(decision, "read");
601539
}
602540
other => panic!("unexpected event: {other:?}"),

0 commit comments

Comments
 (0)