Skip to content

Commit 3eb678c

Browse files
committed
fix(gateway): Phase 4 — gateway fixes + consent polish
Autonomous decisions: - Adapted non-localhost consent note to fork's minimal launcher page — shows fallback immediately on network bind instead of upstream's full consent HTML - Extracted load_gateway_auth_disabled_from_repo helper shared by manual start and auto-start paths — matches upstream #205 pattern Signed-off-by: crimsonsunset <jsangio1@gmail.com>
1 parent 3401053 commit 3eb678c

5 files changed

Lines changed: 176 additions & 57 deletions

File tree

apps/desktop/src-tauri/src/commands/gateway.rs

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,24 @@ pub(crate) async fn load_network_access(app_state: &AppState) -> bool {
231231
load_network_access_from_repo(&app_state.settings_repository).await
232232
}
233233

234+
/// Load the persisted inbound-auth toggle (`gateway.auth_disabled`).
235+
pub(crate) async fn load_gateway_auth_disabled_from_repo(
236+
settings_repository: &Arc<dyn mcpmux_core::AppSettingsRepository>,
237+
) -> bool {
238+
settings_repository
239+
.get(GATEWAY_AUTH_DISABLED_KEY)
240+
.await
241+
.ok()
242+
.flatten()
243+
.map(|value| value == "true")
244+
.unwrap_or(false)
245+
}
246+
247+
/// Load the persisted inbound-auth toggle for the running app instance.
248+
pub(crate) async fn load_gateway_auth_disabled(app_state: &AppState) -> bool {
249+
load_gateway_auth_disabled_from_repo(&app_state.settings_repository).await
250+
}
251+
234252
pub(crate) fn advertised_base_url(public_base_url: Option<&str>, port: u16) -> String {
235253
public_base_url
236254
.map(str::trim)
@@ -1108,18 +1126,8 @@ pub async fn start_gateway(
11081126
// Seed the system-wide inbound-auth toggle into the running gateway from
11091127
// persisted settings (default: auth required). Live changes go through
11101128
// `set_gateway_auth_disabled`.
1111-
{
1112-
let disabled = app_state
1113-
.settings_repository
1114-
.get(GATEWAY_AUTH_DISABLED_KEY)
1115-
.await
1116-
.ok()
1117-
.flatten()
1118-
.map(|v| v == "true")
1119-
.unwrap_or(false);
1120-
if disabled {
1121-
gw_state.write().await.set_auth_disabled(true);
1122-
}
1129+
if load_gateway_auth_disabled(&app_state).await {
1130+
gw_state.write().await.set_auth_disabled(true);
11231131
}
11241132

11251133
// Subscribe to OAuth completions BEFORE spawn so we don't miss early
@@ -2068,3 +2076,37 @@ mod public_base_url_tests {
20682076
assert_eq!(super::bind_host_for(true), "0.0.0.0");
20692077
}
20702078
}
2079+
2080+
#[cfg(test)]
2081+
mod gateway_auth_settings_tests {
2082+
use super::{load_gateway_auth_disabled_from_repo, GATEWAY_AUTH_DISABLED_KEY};
2083+
use mcpmux_core::AppSettingsRepository;
2084+
use mcpmux_storage::{Database, SqliteAppSettingsRepository};
2085+
use std::sync::Arc;
2086+
use tokio::sync::Mutex;
2087+
2088+
fn settings_repo() -> Arc<dyn AppSettingsRepository> {
2089+
let database = Database::open_in_memory().expect("create in-memory database");
2090+
Arc::new(SqliteAppSettingsRepository::new(Arc::new(Mutex::new(
2091+
database,
2092+
))))
2093+
}
2094+
2095+
#[tokio::test]
2096+
async fn auth_remains_required_when_disable_setting_is_missing() {
2097+
let repository = settings_repo();
2098+
2099+
assert!(!load_gateway_auth_disabled_from_repo(&repository).await);
2100+
}
2101+
2102+
#[tokio::test]
2103+
async fn persisted_disable_setting_is_restored_on_gateway_start() {
2104+
let repository = settings_repo();
2105+
repository
2106+
.set(GATEWAY_AUTH_DISABLED_KEY, "true")
2107+
.await
2108+
.unwrap();
2109+
2110+
assert!(load_gateway_auth_disabled_from_repo(&repository).await);
2111+
}
2112+
}

apps/desktop/src-tauri/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,9 @@ pub fn run() {
441441
// devices on the LAN can reach the gateway; loopback-only otherwise.
442442
let network_access =
443443
crate::commands::gateway::load_network_access_from_repo(&settings_repo).await;
444+
let auth_disabled =
445+
crate::commands::gateway::load_gateway_auth_disabled_from_repo(&settings_repo)
446+
.await;
444447
let local_url = format!("http://localhost:{}", final_port);
445448
info!("Auto-starting gateway on {} (advertising {})", local_url, url);
446449

@@ -500,6 +503,10 @@ pub fn run() {
500503
let server = mcpmux_gateway::GatewayServer::new(config, dependencies);
501504
let gw_inner_state = server.state();
502505

506+
if auth_disabled {
507+
gw_inner_state.write().await.set_auth_disabled(true);
508+
}
509+
503510
// Get services from gateway
504511
let pool_service = server.pool_service();
505512
let feature_service = server.feature_service();

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

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ impl McpMuxGatewayHandler {
106106
let resolver = &services.feature_set_resolver;
107107
match resolver
108108
.resolve(session_id, Some(client_id), request_machine_id)
109-
.await {
109+
.await
110+
{
110111
Ok(resolved) => {
111112
info!(
112113
%client_id,
@@ -826,14 +827,23 @@ impl ServerHandler for McpMuxGatewayHandler {
826827
// connection). Without the probe it resolves to empty FS ids and
827828
// fails "not allowed by the current grants" — breaking the
828829
// list==call invariant the list handlers already uphold.
829-
self.ensure_roots_probed(&context.peer, session_id, &oauth_ctx.client_id, oauth_ctx.request_machine_id)
830-
.await;
830+
self.ensure_roots_probed(
831+
&context.peer,
832+
session_id,
833+
&oauth_ctx.client_id,
834+
oauth_ctx.request_machine_id,
835+
)
836+
.await;
831837

832838
// Resolve routing once — the binding's target space is authoritative
833839
// (may differ from oauth_ctx.space_id). Needed both to gate the
834840
// per-Space meta tools below and to route a normal tool call.
835841
let (space_id, feature_set_ids) = self
836-
.resolve_routing(session_id, &oauth_ctx.client_id, oauth_ctx.request_machine_id)
842+
.resolve_routing(
843+
session_id,
844+
&oauth_ctx.client_id,
845+
oauth_ctx.request_machine_id,
846+
)
837847
.await?;
838848

839849
// Intercept meta tools (mcpmux_*) BEFORE feature-set filtering, gated
@@ -979,15 +989,13 @@ impl ServerHandler for McpMuxGatewayHandler {
979989
.await
980990
.map_err(|e| McpError::internal_error(format!("Tool call failed: {}", e), None))?;
981991

982-
// Convert ToolCallResult to MCP CallToolResult
983-
let content: Vec<Content> = tool_result
984-
.content
985-
.into_iter()
986-
.filter_map(|v| serde_json::from_value(v).ok())
987-
.collect();
992+
// Convert ToolCallResult to MCP CallToolResult without dropping
993+
// structuredContent or protocol-level _meta from the upstream server.
994+
let result = tool_result.into_mcp_result();
988995

989996
// Log result summary - show content types and approximate sizes
990-
let content_summary: Vec<String> = content
997+
let content_summary: Vec<String> = result
998+
.content
991999
.iter()
9921000
.map(|c| {
9931001
// Content is Annotated<RawContent>, serialize to inspect type
@@ -1026,18 +1034,11 @@ impl ServerHandler for McpMuxGatewayHandler {
10261034
.collect();
10271035
debug!(
10281036
tool = %params.name,
1029-
is_error = tool_result.is_error,
1037+
is_error = result.is_error.unwrap_or(false),
10301038
content = ?content_summary,
10311039
"call_tool result"
10321040
);
10331041

1034-
let mut result = if tool_result.is_error {
1035-
CallToolResult::error(content)
1036-
} else {
1037-
CallToolResult::success(content)
1038-
};
1039-
result.structured_content = tool_result.structured_content;
1040-
10411042
Ok(result)
10421043
}
10431044

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

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use std::time::Duration;
1212

1313
use anyhow::{anyhow, Result};
1414
use mcpmux_core::{FeatureType, LogLevel, LogSource, ServerLog, ServerLogManager};
15-
use rmcp::model::CallToolRequestParams;
15+
use rmcp::model::{CallToolRequestParams, CallToolResult, Content, Meta};
1616
use serde_json::Value;
1717
use tracing::{debug, info, warn};
1818
use uuid::Uuid;
@@ -53,6 +53,38 @@ pub struct ToolCallResult {
5353
pub content: Vec<Value>,
5454
pub structured_content: Option<Value>,
5555
pub is_error: bool,
56+
pub meta: Option<Meta>,
57+
}
58+
59+
impl ToolCallResult {
60+
fn from_mcp_result(result: CallToolResult) -> Self {
61+
Self {
62+
content: result
63+
.content
64+
.into_iter()
65+
.map(|item| serde_json::to_value(item).unwrap_or(Value::Null))
66+
.collect(),
67+
structured_content: result.structured_content,
68+
is_error: result.is_error.unwrap_or(false),
69+
meta: result.meta,
70+
}
71+
}
72+
73+
pub(crate) fn into_mcp_result(self) -> CallToolResult {
74+
let content: Vec<Content> = self
75+
.content
76+
.into_iter()
77+
.filter_map(|item| serde_json::from_value(item).ok())
78+
.collect();
79+
let mut result = if self.is_error {
80+
CallToolResult::error(content)
81+
} else {
82+
CallToolResult::success(content)
83+
};
84+
result.structured_content = self.structured_content;
85+
result.meta = self.meta;
86+
result
87+
}
5688
}
5789

5890
/// Default timeout for MCP tool calls (60 seconds)
@@ -377,17 +409,7 @@ impl RoutingService {
377409
.map_err(|_| anyhow!("Tool call timed out after {:?}", TOOL_CALL_TIMEOUT))?
378410
.map_err(|e| anyhow!("MCP call failed: {}", e))?;
379411

380-
let content: Vec<Value> = res
381-
.content
382-
.into_iter()
383-
.map(|c| serde_json::to_value(c).unwrap_or(Value::Null))
384-
.collect();
385-
386-
Ok(ToolCallResult {
387-
content,
388-
structured_content: res.structured_content,
389-
is_error: res.is_error.unwrap_or(false),
390-
})
412+
Ok(ToolCallResult::from_mcp_result(res))
391413
}
392414
None => Err(anyhow!("Server instance has no active client")),
393415
}
@@ -823,7 +845,9 @@ impl RoutingService {
823845

824846
#[cfg(test)]
825847
mod redirect_tests {
826-
use super::format_direct_call_redirect;
848+
use super::{format_direct_call_redirect, ToolCallResult};
849+
use rmcp::model::{CallToolResult, Content, Meta};
850+
use serde_json::json;
827851

828852
#[test]
829853
fn direct_call_redirect_points_at_invoke_tool() {
@@ -832,4 +856,23 @@ mod redirect_tests {
832856
assert!(message.contains("\"server_id\": \"github\""));
833857
assert!(message.contains("\"tool\": \"create_issue\""));
834858
}
859+
860+
#[test]
861+
fn tool_result_round_trip_preserves_structured_content_and_meta() {
862+
let structured = json!({ "matches": [{ "message": "found" }] });
863+
let mut meta = Meta::new();
864+
meta.0.insert("traceId".to_string(), json!("trace-123"));
865+
866+
let mut upstream = CallToolResult::structured(structured.clone());
867+
upstream.content = vec![Content::text("search completed")];
868+
upstream.meta = Some(meta.clone());
869+
870+
let routed = ToolCallResult::from_mcp_result(upstream);
871+
let forwarded = routed.into_mcp_result();
872+
873+
assert_eq!(forwarded.content, vec![Content::text("search completed")]);
874+
assert_eq!(forwarded.structured_content, Some(structured));
875+
assert_eq!(forwarded.meta, Some(meta));
876+
assert_eq!(forwarded.is_error, Some(false));
877+
}
835878
}

crates/mcpmux-gateway/src/server/handlers.rs

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -481,10 +481,42 @@ pub async fn oauth_authorize(
481481

482482
let app_name = branding::DISPLAY_NAME;
483483

484-
// Minimal launcher page — fires the deep link and closes immediately.
485-
// No visible UI: consent lives entirely in the McpMux app (desktop modal
486-
// or web admin SSE modal). The fallback section only appears if the
487-
// browser blocks window.close() (tab was not script-opened).
484+
// When the gateway is exposed beyond loopback, a client that reached this
485+
// page from another machine can't complete the desktop consent (the
486+
// mcpmux:// deep link fires only on the host). Surface the API-key path so a
487+
// remote user isn't left at a dead end.
488+
let network_bind = state.read().await.network_bind;
489+
let network_note = if network_bind {
490+
format!(
491+
r#"<p style="margin:0 0 1rem;padding:0.85rem 1rem;border-radius:8px;background:rgba(218,119,86,0.12);border:1px solid rgba(218,119,86,0.3);color:#d8b08c;font-size:0.85rem;line-height:1.45;text-align:left;"><strong style="color:#DA7756;">Connecting from another machine?</strong> This approval only completes on the computer running {app_name}. For a remote or headless client, register an <strong>API-key client</strong> in {app_name} (Clients tab) and connect with that key instead of this browser flow.</p>"#
492+
)
493+
} else {
494+
String::new()
495+
};
496+
let body_style = if network_bind {
497+
String::new()
498+
} else {
499+
"display: none;".to_string()
500+
};
501+
let redirect_script = if network_bind {
502+
String::new()
503+
} else {
504+
format!(
505+
r#"
506+
<script>
507+
window.location.href = "{deep_link_url}";
508+
setTimeout(function() {{
509+
try {{ window.close(); }} catch(e) {{}}
510+
// If window.close() was blocked, reveal the fallback.
511+
document.body.style.display = '';
512+
}}, 300);
513+
</script>"#
514+
)
515+
};
516+
517+
// Minimal launcher page — fires the deep link and closes immediately on
518+
// loopback binds. On network binds the fallback stays visible with guidance
519+
// toward API-key clients because the deep link only works on the host.
488520
let html = format!(
489521
r##"<!DOCTYPE html>
490522
<html lang="en">
@@ -494,7 +526,7 @@ pub async fn oauth_authorize(
494526
<title>{app_name}</title>
495527
<style>
496528
body {{
497-
display: none;
529+
{body_style}
498530
}}
499531
.fallback {{
500532
display: flex;
@@ -522,18 +554,12 @@ pub async fn oauth_authorize(
522554
<body>
523555
<div class="fallback">
524556
<div>
557+
{network_note}
525558
<p>Check {app_name} to complete authorization.</p>
526559
<a href="{deep_link_url}">Open {app_name}</a>
527560
</div>
528561
</div>
529-
<script>
530-
window.location.href = "{deep_link_url}";
531-
setTimeout(function() {{
532-
try {{ window.close(); }} catch(e) {{}}
533-
// If window.close() was blocked, reveal the fallback.
534-
document.body.style.display = '';
535-
}}, 300);
536-
</script>
562+
{redirect_script}
537563
</body>
538564
</html>"##
539565
);

0 commit comments

Comments
 (0)