Skip to content

Commit f4cf81c

Browse files
Merge pull request #10 from crimsonsunset/docs/aug14-gateway-ops-bugs
fix(gateway): close aug14 gateway ops bugs
2 parents 6f98ee7 + 2b4765b commit f4cf81c

32 files changed

Lines changed: 1033 additions & 111 deletions

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

apps/desktop/src-tauri/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ mcpmux-core.workspace = true
5353
mcpmux-gateway.workspace = true
5454
mcpmux-storage.workspace = true
5555

56+
[target.'cfg(unix)'.dependencies]
57+
libc = "0.2"
58+
5659
[target.'cfg(target_os = "macos")'.dependencies]
5760
objc2 = "0.6"
5861
objc2-foundation = "0.3"

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

Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ mod main_window;
1515
mod services;
1616
mod state;
1717
mod tray;
18+
#[cfg(unix)]
19+
mod unix_signal;
1820

1921
// Re-export deep link handler
2022
use commands::oauth::{route_or_buffer_deep_link, PendingInitialDeepLink};
@@ -80,17 +82,11 @@ fn init_tracing() -> tracing_appender::non_blocking::WorkerGuard {
8082
.expect("Failed to create log file appender");
8183
let (non_blocking_file, guard) = tracing_appender::non_blocking(file_appender);
8284

83-
// Environment filter for log levels
84-
// RUST_LOG takes precedence, with sensible defaults for our crates
85-
// Note: Rust crate names use underscores in tracing (e.g., mcpmux-core → mcpmux_core)
85+
// RUST_LOG / .env wins. Default is info; set e.g. RUST_LOG=mcpmux_gateway=debug
86+
// to opt a crate back into debug. Crate names use underscores in tracing
87+
// (mcpmux-core → mcpmux_core).
8688
let env_filter = EnvFilter::try_from_default_env().unwrap_or_else(|_| {
87-
// Default filter when RUST_LOG is not set
8889
EnvFilter::new("info")
89-
.add_directive("mcpmux_core=debug".parse().unwrap())
90-
.add_directive("mcpmux_gateway=debug".parse().unwrap())
91-
.add_directive("mcpmux_storage=debug".parse().unwrap())
92-
.add_directive("mcpmux_mcp=debug".parse().unwrap())
93-
.add_directive("mcpmux_lib=debug".parse().unwrap())
9490
.add_directive("tauri=info".parse().unwrap())
9591
.add_directive("tao=warn".parse().unwrap())
9692
.add_directive("wry=warn".parse().unwrap())
@@ -904,24 +900,8 @@ pub fn run() {
904900
tauri::async_runtime::spawn(async move {
905901
#[cfg(unix)]
906902
{
907-
use tokio::signal::unix::{signal, SignalKind};
908-
let mut sigterm = match signal(SignalKind::terminate()) {
909-
Ok(s) => s,
910-
Err(e) => {
911-
warn!("[Signal] Failed to install SIGTERM handler: {}", e);
912-
return;
913-
}
914-
};
915-
let mut sigint = match signal(SignalKind::interrupt()) {
916-
Ok(s) => s,
917-
Err(e) => {
918-
warn!("[Signal] Failed to install SIGINT handler: {}", e);
919-
return;
920-
}
921-
};
922-
tokio::select! {
923-
_ = sigterm.recv() => info!("[Signal] SIGTERM — requesting exit"),
924-
_ = sigint.recv() => info!("[Signal] SIGINT — requesting exit"),
903+
if !crate::unix_signal::wait_for_term().await {
904+
return;
925905
}
926906
}
927907
#[cfg(windows)]

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,15 @@ use tracing::{debug, info, warn};
3030
pub fn ensure_contacts_registered() {
3131
use objc2_contacts::{CNAuthorizationStatus, CNContactStore, CNEntityType};
3232

33+
// TCC never persists a decision for the unsigned `tauri dev` binary (its
34+
// code signature/path changes across rebuilds), so every dev launch
35+
// re-prompts and immediately reports Access Denied. Skip in debug builds;
36+
// production (signed .app) behavior is unaffected.
37+
if cfg!(debug_assertions) {
38+
debug!("[Permissions] Contacts: skipping request in debug build");
39+
return;
40+
}
41+
3342
// SAFETY: `authorizationStatusForEntityType` is a pure read of the
3443
// system TCC database — no side effects, no main-thread requirement.
3544
let status =
Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
//! Capture who sent SIGTERM/SIGINT (`si_pid`) without fighting tokio's waiter.
2+
//!
3+
//! A `SA_SIGINFO` handler records the sender, then writes one byte to a socket
4+
//! pair so the async setup task can log and exit. We do not use `tokio::signal`
5+
//! on Unix — the last `sigaction` wins, and we need `si_pid`.
6+
7+
use std::os::fd::AsRawFd;
8+
use std::os::unix::net::UnixStream;
9+
use std::sync::atomic::{AtomicI32, Ordering};
10+
11+
use tracing::info;
12+
13+
static SENDER_PID: AtomicI32 = AtomicI32::new(0);
14+
static SIGNAL_NO: AtomicI32 = AtomicI32::new(0);
15+
static SIGNAL_CODE: AtomicI32 = AtomicI32::new(0);
16+
static PIPE_WRITE_FD: AtomicI32 = AtomicI32::new(-1);
17+
18+
/// Install the recorder and wait until SIGTERM or SIGINT arrives.
19+
///
20+
/// Returns `true` after the first termination signal. Returns `false` if the
21+
/// handler could not be installed — the caller must not treat that as exit.
22+
pub async fn wait_for_term() -> bool {
23+
let read_end = match install() {
24+
Ok(fd) => fd,
25+
Err(e) => {
26+
tracing::warn!("[Signal] Failed to install SA_SIGINFO handler: {e}");
27+
return false;
28+
}
29+
};
30+
31+
let mut reader = match tokio::net::UnixStream::from_std(read_end) {
32+
Ok(s) => s,
33+
Err(e) => {
34+
tracing::warn!("[Signal] Failed to wrap self-pipe: {e}");
35+
return false;
36+
}
37+
};
38+
let mut buf = [0u8; 1];
39+
use tokio::io::AsyncReadExt;
40+
let _ = reader.read_exact(&mut buf).await;
41+
42+
let pid = std::process::id();
43+
let ppid = unsafe { libc::getppid() };
44+
let sender = SENDER_PID.load(Ordering::SeqCst);
45+
let sig = SIGNAL_NO.load(Ordering::SeqCst);
46+
let code = SIGNAL_CODE.load(Ordering::SeqCst);
47+
let name = if sig == libc::SIGINT {
48+
"SIGINT"
49+
} else {
50+
"SIGTERM"
51+
};
52+
53+
info!(
54+
pid,
55+
ppid,
56+
parent = %describe_pid(ppid),
57+
sender_pid = sender,
58+
sender = %describe_pid(sender),
59+
si_code = code,
60+
"[Signal] {name} — requesting exit"
61+
);
62+
true
63+
}
64+
65+
fn install() -> std::io::Result<UnixStream> {
66+
let (read, write) = UnixStream::pair()?;
67+
read.set_nonblocking(true)?;
68+
write.set_nonblocking(true)?;
69+
PIPE_WRITE_FD.store(write.as_raw_fd(), Ordering::SeqCst);
70+
// Handler writes to this fd for the life of the process.
71+
std::mem::forget(write);
72+
73+
unsafe {
74+
let mut sa: libc::sigaction = std::mem::zeroed();
75+
sa.sa_sigaction = record_sender as *const () as usize;
76+
sa.sa_flags = libc::SA_SIGINFO | libc::SA_RESTART;
77+
libc::sigemptyset(&mut sa.sa_mask);
78+
if libc::sigaction(libc::SIGTERM, &sa, std::ptr::null_mut()) != 0 {
79+
return Err(std::io::Error::last_os_error());
80+
}
81+
if libc::sigaction(libc::SIGINT, &sa, std::ptr::null_mut()) != 0 {
82+
return Err(std::io::Error::last_os_error());
83+
}
84+
}
85+
Ok(read)
86+
}
87+
88+
unsafe extern "C" fn record_sender(
89+
sig: libc::c_int,
90+
info: *mut libc::siginfo_t,
91+
_ctx: *mut libc::c_void,
92+
) {
93+
if !info.is_null() {
94+
// SAFETY: kernel-filled siginfo for this delivery.
95+
let info = unsafe { &*info };
96+
SENDER_PID.store(unsafe { info.si_pid() }, Ordering::SeqCst);
97+
SIGNAL_CODE.store(info.si_code, Ordering::SeqCst);
98+
}
99+
SIGNAL_NO.store(sig, Ordering::SeqCst);
100+
let fd = PIPE_WRITE_FD.load(Ordering::SeqCst);
101+
if fd >= 0 {
102+
let byte = [1u8];
103+
unsafe {
104+
libc::write(fd, byte.as_ptr() as *const libc::c_void, 1);
105+
}
106+
}
107+
}
108+
109+
/// Best-effort path/name for a live pid. `"unknown"` if pid is empty or gone.
110+
fn describe_pid(pid: libc::pid_t) -> String {
111+
if pid <= 0 {
112+
return "unknown".to_string();
113+
}
114+
#[cfg(target_os = "macos")]
115+
{
116+
let mut buf = [0u8; 4096];
117+
extern "C" {
118+
fn proc_pidpath(pid: i32, buffer: *mut libc::c_void, buffersize: u32) -> i32;
119+
}
120+
let n = unsafe { proc_pidpath(pid, buf.as_mut_ptr() as *mut _, buf.len() as u32) };
121+
if n > 0 {
122+
return String::from_utf8_lossy(&buf[..n as usize]).into_owned();
123+
}
124+
}
125+
#[cfg(target_os = "linux")]
126+
{
127+
if let Ok(path) = std::fs::read_link(format!("/proc/{pid}/exe")) {
128+
return path.display().to_string();
129+
}
130+
if let Ok(comm) = std::fs::read_to_string(format!("/proc/{pid}/comm")) {
131+
return comm.trim().to_string();
132+
}
133+
}
134+
format!("pid:{pid}")
135+
}
136+
137+
#[cfg(test)]
138+
mod tests {
139+
use super::*;
140+
141+
#[test]
142+
fn describe_pid_self_is_nonempty() {
143+
let me = std::process::id() as libc::pid_t;
144+
let desc = describe_pid(me);
145+
assert!(!desc.is_empty(), "self pid should resolve");
146+
assert_ne!(desc, "unknown");
147+
}
148+
149+
#[test]
150+
fn describe_pid_zero_is_unknown() {
151+
assert_eq!(describe_pid(0), "unknown");
152+
}
153+
}

crates/mcpmux-core/src/application/server.rs

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -402,14 +402,11 @@ impl ServerAppService {
402402
.get_definition()
403403
.ok_or_else(|| anyhow!("Server has no cached definition"))?;
404404

405-
let user_entry: UserServerEntry = serde_json::from_value(entry)
406-
.map_err(|e| anyhow!("Invalid server entry: {}", e))?;
405+
let user_entry: UserServerEntry =
406+
serde_json::from_value(entry).map_err(|e| anyhow!("Invalid server entry: {}", e))?;
407407

408-
let mut definition = user_entry.to_server_definition(
409-
server_id,
410-
&space_id_str,
411-
std::path::PathBuf::new(),
412-
);
408+
let mut definition =
409+
user_entry.to_server_definition(server_id, &space_id_str, std::path::PathBuf::new());
413410

414411
definition.id = existing.id.clone();
415412
definition.source = ServerSource::ManualEntry;
@@ -847,9 +844,7 @@ mod tests {
847844
name: "PostHog Personal".to_string(),
848845
description: None,
849846
alias: Some("posthog".to_string()),
850-
auth: Some(AuthConfig::ApiKey {
851-
instructions: None,
852-
}),
847+
auth: Some(AuthConfig::ApiKey { instructions: None }),
853848
icon: None,
854849
transport: TransportConfig::Http {
855850
url: "https://mcp.posthog.com/mcp".to_string(),
@@ -898,15 +893,10 @@ mod tests {
898893
"Authorization".to_string(),
899894
"Bearer phx_parent_token".to_string(),
900895
),
901-
(
902-
"x-posthog-project-id".to_string(),
903-
"345911".to_string(),
904-
),
896+
("x-posthog-project-id".to_string(), "345911".to_string()),
905897
]);
906-
let parent_inputs = HashMap::from([(
907-
"POSTHOG_API_KEY".to_string(),
908-
"phc_parent_key".to_string(),
909-
)]);
898+
let parent_inputs =
899+
HashMap::from([("POSTHOG_API_KEY".to_string(), "phc_parent_key".to_string())]);
910900

911901
let definition = user_space_http_definition("posthog-personal");
912902
let mut source = InstalledServer::new(space_id.to_string(), "posthog-personal")
@@ -918,12 +908,7 @@ mod tests {
918908
source.extra_headers = parent_headers.clone();
919909
repo.seed(source).await;
920910

921-
let service = ServerAppService::new(
922-
repo.clone(),
923-
None,
924-
None,
925-
event_bus.sender(),
926-
);
911+
let service = ServerAppService::new(repo.clone(), None, None, event_bus.sender());
927912

928913
let cloned = service
929914
.clone_server(space_id, "posthog-personal", "mesh", None, None)

crates/mcpmux-core/src/application/user_space_sync.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,10 @@ mod tests {
759759
.expect("sync should update existing server");
760760
assert_eq!(result.updated, vec!["alpha".to_string()]);
761761

762-
let event = receiver.recv().await.expect("ServerConfigUpdated should emit");
762+
let event = receiver
763+
.recv()
764+
.await
765+
.expect("ServerConfigUpdated should emit");
763766
assert_eq!(event.type_name(), "server_config_updated");
764767
assert_eq!(event.server_id(), Some("alpha"));
765768
}

crates/mcpmux-gateway/src/consumers/server_config_handler.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,11 @@ impl ServerConfigUpdatedHandler {
5555

5656
/// Handle one domain event, evicting the pool instance when applicable.
5757
async fn handle_event(&self, event: DomainEvent) -> anyhow::Result<()> {
58-
let DomainEvent::ServerConfigUpdated { space_id, server_id } = event else {
58+
let DomainEvent::ServerConfigUpdated {
59+
space_id,
60+
server_id,
61+
} = event
62+
else {
5963
return Ok(());
6064
};
6165

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,15 @@ pub async fn mcp_oauth_middleware(
258258
(sid, ws)
259259
};
260260
match (&session_id_header, &workspace_header) {
261+
(_, Some(ws)) if ws.trim().is_empty() => {
262+
warn!(
263+
trace_id = %trace_id,
264+
session_id = session_id_header.as_deref().unwrap_or("<none>"),
265+
"[SessionRoots] X-Mcpmux-Workspace present but empty — pin skipped \
266+
(Cursor Agents window often spawns mcp-remote without resolving \
267+
${{workspaceFolder}}; see docs/manual/cursor-workspace-bridge.md Fallback)",
268+
);
269+
}
261270
(Some(sid), Some(ws)) => {
262271
services.session_roots.set_pinned(sid, ws);
263272
}
@@ -271,6 +280,11 @@ pub async fn mcp_oauth_middleware(
271280
_ => {}
272281
}
273282

283+
// Captured before `request` is consumed below — needed to recognize the
284+
// spec-correct GET shapes (pre-init SSE, post-timeout reconnect) when
285+
// deciding whether to warn on the response status.
286+
let http_method = request.method().clone();
287+
274288
// Extract MCP method from body if POST
275289
let mcp_method = if request.method() == axum::http::Method::POST {
276290
use axum::body::to_bytes;
@@ -313,9 +327,17 @@ pub async fn mcp_oauth_middleware(
313327

314328
let response = next.run(request).await;
315329

316-
// Log errors only
330+
// Log errors only — except two rmcp spec-correct shapes that are not
331+
// gateway problems: a GET without Mcp-Session-Id (client opening the SSE
332+
// stream before initialize) and any request against a session rmcp has
333+
// already closed (idle keep-alive timeout or explicit termination). Both
334+
// are expected client reconnect behavior. See
335+
// docs/planning/aug14-gateway-ops-bugs.md Decision 4.
317336
let status = response.status();
318-
if status.is_server_error() || status.is_client_error() {
337+
let is_expected_session_noise = (http_method == axum::http::Method::GET
338+
&& status == StatusCode::BAD_REQUEST)
339+
|| status == StatusCode::NOT_FOUND;
340+
if (status.is_server_error() || status.is_client_error()) && !is_expected_session_noise {
319341
warn!(
320342
trace_id = %trace_id,
321343
status = %status,

crates/mcpmux-gateway/src/oauth/flow.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ impl OAuthFlow {
187187
fn generate_state() -> String {
188188
use rand::Rng;
189189
let mut rng = rand::thread_rng();
190-
let bytes: Vec<u8> = (0..16).map(|_| rng.gen()).collect();
190+
let bytes: Vec<u8> = (0..16).map(|_| rng.gen::<u8>()).collect();
191191
use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine};
192192
URL_SAFE_NO_PAD.encode(&bytes)
193193
}

0 commit comments

Comments
 (0)