From 0f5e7fc7f66139abe17ef33ea14ffa1062f3bb4c Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Tue, 22 Sep 2026 05:57:23 +0530 Subject: [PATCH] Park a server whose credential was rejected instead of retrying it forever MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An MCP server answering 401 was penalised on the backoff curve like any other failure. The curve caps at BACKOFF_MAX, so the server was re-dialled every five minutes for the life of the session — 18+ attempts in one reported session, each writing a warning, against an endpoint that had already said no (openhuman#6412). Waiting does not turn a 401 into a 200. Only the user supplying a working credential does, and that arrives through the store, not through time. This is the same shape as a missing runtime, which the supervisor already parks with exactly that reasoning, so route a rejected credential into the same terminal path rather than inventing a second one. The existing park machinery then covers the rest of the report for free: a parked server is reported once and stays quiet, so the log noise stops at one line; `terminal` is already cleared when a server is disabled, so toggling it off and on after setting a token is the way back, the same gesture that already clears a backoff penalty; and `ServerStatus::Unauthorized` already reaches the host, so the state is visible rather than merely silent. --- .../tinymcp/src/registry/supervisor/test.rs | 70 +++++++++++++++++++ .../tinymcp/src/registry/supervisor/types.rs | 9 ++- 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/crates/tinymcp/src/registry/supervisor/test.rs b/crates/tinymcp/src/registry/supervisor/test.rs index 3af5f570..dbcf08d5 100644 --- a/crates/tinymcp/src/registry/supervisor/test.rs +++ b/crates/tinymcp/src/registry/supervisor/test.rs @@ -1285,3 +1285,73 @@ fn every_event_names_its_server_and_its_kind() { } assert!(TickReport::default().is_empty()); } + +/// Binds a loopback port and serves a server that rejects every request with +/// HTTP 401, as an MCP server does when its bearer token is missing or wrong. +async fn serve_unauthorized_server() -> String { + let app = Router::new().route( + "/", + post(|| async { (axum::http::StatusCode::UNAUTHORIZED, "unauthorized") }), + ); + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + format!("http://{addr}/") +} + +#[tokio::test] +async fn a_401_is_terminal_and_earns_no_backoff_penalty() { + // A 401 does not become a 200 by waiting: the credential is missing or + // wrong and only the user can change it. Retrying on the backoff curve + // means a request every BACKOFF_MAX forever, which is the reported storm + // (18+ failures in one session, every 300s, openhuman#6412). + let url = serve_unauthorized_server().await; + let store = Store::open_in_memory().unwrap(); + store + .insert_server(&install("srv-1", Transport::HttpRemote { url }, true)) + .unwrap(); + + let connections = Connections::new(); + let oauth = OAuthFlow::new(None).unwrap(); + let mut supervisor = supervisor(); + let base = Instant::now(); + + supervisor.tick(&store, &connections, &oauth, base).await; + + assert!(!connections.is_connected("srv-1").await); + assert_eq!( + supervisor.backed_off_count(), + 0, + "a backoff promises that waiting helps; a 401 needs a new credential" + ); + assert_eq!( + supervisor.terminally_failed_count(), + 1, + "the unauthorized server must be parked, not penalised" + ); + + // Far past any backoff window: a penalised server would certainly have + // been retried by now. A parked one must not be. + supervisor + .tick(&store, &connections, &oauth, base + BACKOFF_MAX * 2) + .await; + + assert_eq!(supervisor.backed_off_count(), 0); + assert_eq!( + supervisor.terminally_failed_count(), + 1, + "still parked — nothing about the credential changed" + ); + + // Toggling the server is the way back, so a user who sets the token can + // reconnect without restarting the app. + store.update_enabled("srv-1", false).unwrap(); + supervisor + .tick(&store, &connections, &oauth, base + BACKOFF_MAX * 3) + .await; + + assert_eq!(supervisor.terminally_failed_count(), 0); +} diff --git a/crates/tinymcp/src/registry/supervisor/types.rs b/crates/tinymcp/src/registry/supervisor/types.rs index e9d342c6..e49c9000 100644 --- a/crates/tinymcp/src/registry/supervisor/types.rs +++ b/crates/tinymcp/src/registry/supervisor/types.rs @@ -264,10 +264,17 @@ impl Supervisor { after_failures, } } - Err(error) if error.is_missing_runtime() => { + Err(error) if error.is_missing_runtime() || error.is_unauthorized() => { // No backoff entry: a penalty says "wait, then try again", // and there is nothing to wait for. The server is parked // until the user disables and re-enables it. + // + // A rejected credential is terminal for the same reason a + // missing runtime is: waiting does not turn a 401 into a 200, + // only the user supplying a working token does. Retrying it on + // the curve is a request every BACKOFF_MAX for the life of the + // session against someone else's endpoint, and one warning per + // retry in the log (openhuman#6412). self.backoff.remove(&server_id); self.terminal.insert(server_id.clone()); tracing::warn!(