diff --git a/tests/tools/test_mcp_oauth_manager.py b/tests/tools/test_mcp_oauth_manager.py index 87e454a970..d3e3fcc87e 100644 --- a/tests/tools/test_mcp_oauth_manager.py +++ b/tests/tools/test_mcp_oauth_manager.py @@ -136,6 +136,8 @@ async def test_first_observation_401_still_allows_in_place_refresh(tmp_path, mon (token_dir / "srv.json").write_text(json.dumps({"access_token": "OLD"}), encoding="utf-8") class _Ctx: + current_tokens = object() # tokens already in memory (in-process sign-in) + def can_refresh_token(self): return True @@ -154,6 +156,33 @@ async def test_first_observation_401_still_allows_in_place_refresh(tmp_path, mon assert mgr._entries[mgr._key("srv")].last_mtime_ns != 0 +@pytest.mark.asyncio +async def test_external_login_after_absent_tokens_file_forces_reload(tmp_path, monkeypatch): + """Provider built before login (no file, no in-memory tokens) must reload + once another process writes the tokens file (GH#39551 review).""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + from tools.mcp_oauth_manager import MCPOAuthManager, _ProviderEntry + + class _Ctx: + current_tokens = None + + class _Provider: + context = _Ctx() + _initialized = True + + mgr = MCPOAuthManager() + provider = _Provider() + mgr._entries[mgr._key("srv")] = _ProviderEntry( + server_url="https://example.com/mcp", oauth_config=None, provider=provider, + ) + assert await mgr.invalidate_if_disk_changed("srv") is False # file absent + token_dir = tmp_path / "mcp-tokens" + token_dir.mkdir(parents=True) + (token_dir / "srv.json").write_text(json.dumps({"access_token": "NEW"}), encoding="utf-8") + assert await mgr.invalidate_if_disk_changed("srv") is True + assert provider._initialized is False + + @pytest.mark.asyncio async def test_handle_401_dedup_survives_even_if_task_reference_dropped(tmp_path, monkeypatch): """Concurrent 401s share one handler task and all callers resolve. diff --git a/tools/mcp_oauth_manager.py b/tools/mcp_oauth_manager.py index e40ff4663d..9f3a2a8339 100644 --- a/tools/mcp_oauth_manager.py +++ b/tools/mcp_oauth_manager.py @@ -355,10 +355,12 @@ class MCPOAuthManager: if mtime_ns == entry.last_mtime_ns: return False old, entry.last_mtime_ns = entry.last_mtime_ns, mtime_ns - if old == 0: - # First observation only seeds the baseline. Forcing a - # reload here can reset the provider in the middle of its - # first auth handshake and tear down HTTP MCP discovery. + if old == 0 and getattr(getattr(entry.provider, "context", None), "current_tokens", None) is not None: + # First observation with tokens already in memory only seeds the + # baseline: the file was written by this process's own first + # sign-in, and reloading on the next request would tear down the + # live HTTP MCP session. With no tokens in memory (started before + # an external `hermes mcp login`), fall through and reload. return False # `_initialized` is private SDK API but stable across the pinned versions (>=1.26.0). if hasattr(entry.provider, "_initialized"):