fix(code): suppress expected MCP reauth logs (#4359)

Expected MCP OAuth re-auth failures in non-interactive mode runs now
produce the Deep Agents login hint without also dumping noisy SDK
warning tracebacks. Transient refresh failures still stay visible so
provider outages are not mistaken for expired credentials.

## Changes
- Added a scoped OAuth log filter that suppresses only expected refresh
failures (`400`, `401`, `403`) and nested re-auth errors while the
non-interactive auth flow is delegating to the MCP SDK.
- Wired `build_oauth_provider(interactive=False)` to enable that
suppression, while preserving the SDK diagnostics for interactive
sessions.
- Changed MCP tool discovery to log expected re-auth skips as concise
warnings with the full exception moved to debug logs, while keeping
unexpected discovery failures at warning level with tracebacks.
- Reformatted untrusted project MCP server warnings as a multiline list
so multiple skipped servers are readable.
This commit is contained in:
Mason Daugherty
2026-06-28 23:56:04 -04:00
committed by GitHub
parent db441ed306
commit 15ee384117
4 changed files with 281 additions and 16 deletions
+52
View File
@@ -13,6 +13,7 @@ from __future__ import annotations
import concurrent.futures
import contextlib
import contextvars
import hashlib
import html
import json
@@ -43,6 +44,7 @@ from mcp.shared.auth import (
OAuthToken,
)
from pydantic import BaseModel, ConfigDict, ValidationError
from typing_extensions import override
if TYPE_CHECKING:
from pathlib import Path
@@ -109,6 +111,47 @@ class McpServerSpec(TypedDict, total=False):
logger = logging.getLogger(__name__)
_SUPPRESS_EXPECTED_REAUTH_LOGS: contextvars.ContextVar[bool] = contextvars.ContextVar(
"suppress_expected_mcp_reauth_logs",
default=False,
)
_TOKEN_REFRESH_FAILED_PREFIX = "Token refresh failed: " # noqa: S105 # log-message prefix, not a credential
"""Prefix of the SDK's `Token refresh failed: <status>` warning (`oauth2.py`)."""
_EXPECTED_REAUTH_REFRESH_STATUSES = frozenset({"400", "401", "403"})
"""Refresh-endpoint statuses that mean the grant was rejected (token stale).
The SDK logs `Token refresh failed: <status>` and clears tokens for *any*
non-200 on the refresh endpoint. Only these statuses indicate the refresh
token itself is expired/revoked — i.e. the expected re-auth cases our hint
replaces. Transient failures (`429`, `5xx`, gateway timeouts) must stay
visible so a provider outage isn't silently relabeled as "go re-login".
"""
class _ExpectedReauthLogFilter(logging.Filter):
"""Drop SDK OAuth log records that are replaced by our reauth hint."""
@override
def filter(self, record: logging.LogRecord) -> bool:
"""Return whether the SDK OAuth log record should be emitted."""
if not _SUPPRESS_EXPECTED_REAUTH_LOGS.get():
return True
message = record.getMessage()
if message.startswith(_TOKEN_REFRESH_FAILED_PREFIX):
status = message.removeprefix(_TOKEN_REFRESH_FAILED_PREFIX)
if status in _EXPECTED_REAUTH_REFRESH_STATUSES:
return False
if message == "OAuth flow error" and record.exc_info is not None:
exc = record.exc_info[1]
if exc is not None and find_reauth_required(exc) is not None:
return False
return True
logging.getLogger("mcp.client.auth.oauth2").addFilter(_ExpectedReauthLogFilter())
_REF_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}")
"""Matches `${VAR}` placeholders inside config strings for env-var substitution."""
@@ -1095,6 +1138,9 @@ class _ExpiryAwareOAuthClientProvider(OAuthClientProvider):
"""
def __init__(self, *args: Any, **kwargs: Any) -> None:
self._suppress_expected_reauth_logs = bool(
kwargs.pop("suppress_expected_reauth_logs", False)
)
super().__init__(*args, **kwargs)
_strip_duplicate_client_id_under_basic_auth(self.context)
@@ -1245,6 +1291,9 @@ class _ExpiryAwareOAuthClientProvider(OAuthClientProvider):
# `asend`/`aclose` (never `athrow`), so forwarding sent values and
# closing the inner generator on `GeneratorExit` is sufficient — no
# `athrow` forwarding needed.
token: contextvars.Token[bool] | None = None
if self._suppress_expected_reauth_logs:
token = _SUPPRESS_EXPECTED_REAUTH_LOGS.set(True)
inner = super().async_auth_flow(request)
try:
# Prime with `anext()` (no response to send yet); thereafter every
@@ -1257,6 +1306,8 @@ class _ExpiryAwareOAuthClientProvider(OAuthClientProvider):
return
finally:
await inner.aclose()
if token is not None:
_SUPPRESS_EXPECTED_REAUTH_LOGS.reset(token)
def build_oauth_provider(
@@ -1347,6 +1398,7 @@ def build_oauth_provider(
storage=storage,
redirect_handler=redirect,
callback_handler=callback,
suppress_expected_reauth_logs=not interactive,
)
+23 -12
View File
@@ -1558,21 +1558,31 @@ async def _load_tools_from_config(
# Tokens existed (we checked above) but the OAuth provider
# fell back to interactive reauth — the refresh attempt
# failed. Flag unauthenticated so the user is prompted to
# re-login, and keep the original exception in the log so
# debugging a real provider outage is possible.
# re-login, and keep the original exception only in debug logs
# so expected re-auth skips don't flood non-interactive output.
status = "unauthenticated"
error = (
f"{reauth} "
"(token refresh failed; the original error is in debug logs)"
)
logger.warning(
"MCP server '%s' skipped: %s",
server_name,
error,
)
logger.debug(
"MCP server '%s' skipped: tool discovery failed",
server_name,
exc_info=True,
)
else:
status = "error"
error = str(exc)
logger.warning(
"MCP server '%s' skipped: tool discovery failed",
server_name,
exc_info=True,
)
logger.warning(
"MCP server '%s' skipped: tool discovery failed",
server_name,
exc_info=True,
)
server_infos.append(
MCPServerInfo(
name=server_name,
@@ -1791,18 +1801,19 @@ async def resolve_and_load_mcp_tools(
# `${VAR}` references in their `headers` would exfiltrate the value
# to the attacker URL during the discovery handshake.
skipped = [
f"{name} [{kind}]: {summary}" for name, kind, summary in project_servers
f"- {name} [{kind}]: {summary}" for name, kind, summary in project_servers
]
skipped_list = "\n".join(skipped)
if trust_project_mcp is False:
logger.warning(
"Skipped untrusted project MCP servers: %s",
"; ".join(skipped),
"Skipped untrusted project MCP servers:\n%s",
skipped_list,
)
else:
logger.warning(
"Skipped untrusted project MCP servers "
"(config changed or not yet approved): %s",
"; ".join(skipped),
"(config changed or not yet approved):\n%s",
skipped_list,
)
if explicit_config_path:
+178 -1
View File
@@ -8,7 +8,7 @@ import logging
import re
import time
from pathlib import Path
from typing import Any, Literal
from typing import Any, Literal, cast
from unittest.mock import patch
import pytest
@@ -512,6 +512,66 @@ class TestExpiryAwareOAuthClientProvider:
assert "/.well-known/oauth-protected-resource" in str(discovery_request.url)
await flow.aclose()
@pytest.mark.parametrize(
("interactive", "expected"),
[(False, True), (True, False)],
)
async def test_delegated_flow_toggles_reauth_log_suppression(
self,
fake_home: Path,
monkeypatch: pytest.MonkeyPatch,
*,
interactive: bool,
expected: bool,
) -> None:
"""The contextvar is set during delegation only for non-interactive runs.
Guards the wiring between `build_oauth_provider(interactive=...)` and the
filter: the SDK flow logs synchronously inside the delegated generator,
so the suppression flag must be visible there. A fake SDK flow records
what the contextvar reads at that point.
"""
del fake_home
import httpx
from mcp.client.auth import OAuthClientProvider
from deepagents_code.mcp_auth import (
_SUPPRESS_EXPECTED_REAUTH_LOGS,
build_oauth_provider,
)
observed: dict[str, bool] = {}
async def fake_flow(
self: OAuthClientProvider,
request: httpx.Request,
):
del self
observed["suppressed"] = _SUPPRESS_EXPECTED_REAUTH_LOGS.get()
_ = yield request
monkeypatch.setattr(OAuthClientProvider, "async_auth_flow", fake_flow)
storage = FileTokenStorage("notion")
await storage.set_client_info(_make_client_info())
await storage.set_tokens(_make_tokens())
provider = build_oauth_provider(
server_name="notion",
server_url="https://mcp.notion.com/mcp",
storage=storage,
interactive=interactive,
)
flow = provider.async_auth_flow(
httpx.Request("POST", "https://mcp.notion.com/mcp")
)
await anext(flow)
await flow.aclose()
assert observed["suppressed"] is expected
# The flag never leaks past the flow.
assert _SUPPRESS_EXPECTED_REAUTH_LOGS.get() is False
async def test_delegated_flow_forwards_responses_on_every_iteration(
self,
fake_home: Path,
@@ -664,6 +724,95 @@ class TestBasicAuthClientIdStripping:
assert "client_secret" not in data
class TestExpectedReauthLogFilter:
"""Tests for suppressing noisy SDK OAuth logs during non-interactive reauth."""
def test_suppresses_expected_sdk_oauth_logs(
self,
caplog: pytest.LogCaptureFixture,
) -> None:
"""Expected non-interactive reauth logs are replaced by our login hint."""
from deepagents_code.mcp_auth import _SUPPRESS_EXPECTED_REAUTH_LOGS
sdk_logger = logging.getLogger("mcp.client.auth.oauth2")
caplog.set_level(logging.WARNING, logger="mcp.client.auth.oauth2")
server = "notion"
reauth = MCPReauthRequiredError(server)
msg = "boom"
unexpected = RuntimeError(msg)
token = _SUPPRESS_EXPECTED_REAUTH_LOGS.set(True)
try:
sdk_logger.warning("Token refresh failed: 400")
sdk_logger.error(
"OAuth flow error",
exc_info=(type(reauth), reauth, reauth.__traceback__),
)
sdk_logger.error(
"OAuth flow error",
exc_info=(type(unexpected), unexpected, unexpected.__traceback__),
)
finally:
_SUPPRESS_EXPECTED_REAUTH_LOGS.reset(token)
messages = [record.getMessage() for record in caplog.records]
assert messages == ["OAuth flow error"]
exc_info = caplog.records[0].exc_info
assert exc_info is not None
assert isinstance(exc_info[1], RuntimeError)
def test_transient_refresh_failure_is_not_suppressed(
self,
caplog: pytest.LogCaptureFixture,
) -> None:
"""Transient refresh statuses (5xx/429) stay visible, not relabeled reauth.
The SDK logs `Token refresh failed: <status>` for any non-200. A `503`
means the provider is down and the refresh token is still valid, so the
operator must see it rather than be steered toward a pointless re-login.
"""
from deepagents_code.mcp_auth import _SUPPRESS_EXPECTED_REAUTH_LOGS
sdk_logger = logging.getLogger("mcp.client.auth.oauth2")
caplog.set_level(logging.WARNING, logger="mcp.client.auth.oauth2")
token = _SUPPRESS_EXPECTED_REAUTH_LOGS.set(True)
try:
sdk_logger.warning("Token refresh failed: 503")
sdk_logger.warning("Token refresh failed: 429")
sdk_logger.warning("Token refresh failed: 400")
finally:
_SUPPRESS_EXPECTED_REAUTH_LOGS.reset(token)
messages = [record.getMessage() for record in caplog.records]
assert messages == [
"Token refresh failed: 503",
"Token refresh failed: 429",
]
def test_passes_through_when_not_suppressing(
self,
caplog: pytest.LogCaptureFixture,
) -> None:
"""With the contextvar unset, the process-wide filter is inert.
The filter is installed on the SDK logger for every consumer of that
logger, so its default-off behavior guards against globally swallowing
real OAuth errors outside a non-interactive reauth window.
"""
sdk_logger = logging.getLogger("mcp.client.auth.oauth2")
caplog.set_level(logging.WARNING, logger="mcp.client.auth.oauth2")
reauth = MCPReauthRequiredError("notion")
# No `_SUPPRESS_EXPECTED_REAUTH_LOGS.set(...)`: contextvar at default.
sdk_logger.warning("Token refresh failed: 400")
sdk_logger.error(
"OAuth flow error",
exc_info=(type(reauth), reauth, reauth.__traceback__),
)
messages = [record.getMessage() for record in caplog.records]
assert messages == ["Token refresh failed: 400", "OAuth flow error"]
class TestFindReauthRequired:
"""Tests for unwrapping nested re-auth errors."""
@@ -895,6 +1044,34 @@ class TestBuildOAuthProvider:
assert metadata.redirect_uris is not None
assert [str(uri) for uri in metadata.redirect_uris] == [_SLACK_REDIRECT_URI]
def test_interactive_mode_maps_to_reauth_log_suppression(
self,
fake_home: Path,
) -> None:
"""Only non-interactive providers suppress expected reauth SDK logs.
Interactive sessions keep the SDK's OAuth diagnostics; non-interactive
runs replace the expected reauth noise with our login hint.
"""
del fake_home
from deepagents_code.mcp_auth import build_oauth_provider
non_interactive = build_oauth_provider(
server_name="notion",
server_url="https://mcp.notion.com/mcp",
storage=FileTokenStorage("notion"),
interactive=False,
)
interactive = build_oauth_provider(
server_name="notion",
server_url="https://mcp.notion.com/mcp",
storage=FileTokenStorage("notion"),
interactive=True,
)
assert cast("Any", non_interactive)._suppress_expected_reauth_logs is True
assert cast("Any", interactive)._suppress_expected_reauth_logs is False
async def test_refresh_uses_cached_oauth_metadata_endpoint(
self,
fake_home: Path,
+28 -3
View File
@@ -4,6 +4,7 @@ from __future__ import annotations
import asyncio
import json
import logging
from contextlib import asynccontextmanager
from pathlib import Path
from typing import TYPE_CHECKING, Any, cast
@@ -1323,8 +1324,11 @@ class TestLoadToolsFromConfigOAuth:
assert isinstance(recorded[0].get("auth"), OAuthClientProvider)
await manager.cleanup()
async def test_discovery_reauth_marks_server_unauthenticated(self) -> None:
"""OAuth re-auth during discovery is surfaced as unauthenticated."""
async def test_discovery_reauth_marks_server_unauthenticated(
self,
caplog: pytest.LogCaptureFixture,
) -> None:
"""OAuth re-auth during discovery is surfaced without warning tracebacks."""
from mcp.shared.auth import OAuthToken
storage = FileTokenStorage(
@@ -1344,6 +1348,8 @@ class TestLoadToolsFromConfigOAuth:
raise ExceptionGroup(msg, [MCPReauthRequiredError("notion")])
yield
caplog.set_level(logging.WARNING, logger="deepagents_code.mcp_tools")
with patch("langchain_mcp_adapters.sessions.create_session", _fake):
config = {
"mcpServers": {
@@ -1360,6 +1366,15 @@ class TestLoadToolsFromConfigOAuth:
assert isinstance(manager, MCPSessionManager)
assert server_infos[0].status == "unauthenticated"
assert "re-authentication" in (server_infos[0].error or "")
warning_records = [
record
for record in caplog.records
if record.name == "deepagents_code.mcp_tools"
and record.levelno == logging.WARNING
]
assert warning_records
assert all(record.exc_info is None for record in warning_records)
assert "Exception Group Traceback" not in caplog.text
await manager.cleanup()
@@ -1452,6 +1467,7 @@ class TestResolveAndLoadMcpTools:
mock_classify: MagicMock,
mock_load: AsyncMock,
tmp_path: Path,
caplog: pytest.LogCaptureFixture,
) -> None:
"""Project remote MCP entries do not reach the loader without trust.
@@ -1467,7 +1483,11 @@ class TestResolveAndLoadMcpTools:
"transport": "http",
"url": "http://169.254.169.254",
"headers": {"X-Token": "${OPENAI_API_KEY}"},
}
},
"docs-langchain": {
"transport": "http",
"url": "https://docs.langchain.com/mcp",
},
}
}
)
@@ -1475,6 +1495,7 @@ class TestResolveAndLoadMcpTools:
mock_discover.return_value = [project_cfg]
mock_classify.return_value = ([], [project_cfg])
mock_load.return_value = ([], None, [])
caplog.set_level(logging.WARNING, logger="deepagents_code.mcp_tools")
tools, _manager, _infos = await resolve_and_load_mcp_tools(
trust_project_mcp=False,
@@ -1482,6 +1503,10 @@ class TestResolveAndLoadMcpTools:
assert tools == []
assert mock_load.call_count == 0
assert "Skipped untrusted project MCP servers:\n" in caplog.text
assert "- evil [http]: http://169.254.169.254" in caplog.text
assert "- docs-langchain [http]: https://docs.langchain.com/mcp" in caplog.text
assert "; docs-langchain" not in caplog.text
@patch("deepagents_code.mcp_tools._load_tools_from_config")
@patch("deepagents_code.mcp_tools.classify_discovered_configs")