fix(mcp): force OAuth login for preregistered clients - #37840
Open
zapabob wants to merge 1 commit into
Open
Conversation
alt-glitch
added
type/bug
Collaborator
|
Duplicate of #35965 — same fix (force OAuth login for preregistered MCP clients). Consolidating on the earlier open PR. |
teknium1
reviewed
Jul 13, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for isolating the anonymous-tools/list OAuth failure. The premise remains present on current main: _reauth_oauth_server still expects _probe_single_server() to initiate OAuth and only detects the missing-token condition afterward (hermes_cli/mcp_config.py:787-815).
Problems
- The added helper unconditionally calls
_stop_mcp_loop()(hermes_cli/mcp_config.py:255in this PR). Current main documents that temporary probes must use_stop_mcp_loop_if_idle()so they do not terminate active MCP sessions or orphan their children (tools/mcp_tool.py:5592-5626). - Current main routes
loginand bothreauthforms through_reauth_oauth_server(hermes_cli/mcp_config.py:759,865,899,915). The explicit-login step should be integrated there, retaining the current 315-second interactive timeout floor (hermes_cli/mcp_config.py:787-802).
Suggested changes
- Port the logic into the shared reauth helper, use idle-only loop cleanup, and cover both
loginandreauth --all.
Automated hermes-sweeper review.
| ) | ||
| ) | ||
| finally: | ||
| _stop_mcp_loop() |
Contributor
There was a problem hiding this comment.
Use _stop_mcp_loop_if_idle() here. Current tools/mcp_tool.py:5592-5626 documents that unconditional shutdown can terminate active registered MCP servers and orphan their subprocesses; this helper is a temporary CLI flow and must not tear down a live shared loop.
| # list tools unauthenticated, so no SDK auth flow is triggered otherwise. | ||
| try: | ||
| if _has_preregistered_oauth_client(server_config): | ||
| _force_oauth_login(name, server_config) |
Contributor
There was a problem hiding this comment.
Current main funnels login and both reauth forms through _reauth_oauth_server. Port this shared explicit-login step there during salvage so reauth --all receives the same behavior and the existing 315-second browser-flow timeout is preserved.
teknium1
added
sweeper:risk-security-boundary
teknium1
added a commit
that referenced
this pull request
Jul 27, 2026
…s; add new overflow shapes Port from anomalyco/opencode#37848 (+ dev-branch twin #37840): expand context-overflow patterns and guard against rate-limit messages that mention tokens. - 'Throttling error: Too many tokens, please wait before trying again.' (AWS Bedrock / proxy shape) classified as context_overflow and routed a healthy session into compression on every throttle. Added 'throttling' to _RATE_LIMIT_PATTERNS, which the message-only path checks BEFORE the overflow list. - 'Input length N exceeds the maximum allowed input length of M tokens.' (Together/Fireworks shape) fell through to unknown — no compression recovery. Added 'maximum allowed input length' to overflow patterns. - 'request_too_large' / 'Request exceeds the maximum size' (Anthropic 413 type re-wrapped without a status code by aggregators/proxies) fell through to unknown. Added to _PAYLOAD_TOO_LARGE_PATTERNS. All three shapes proven live on main before the fix; 265 classifier + bedrock tests and 238 sibling rate-guard/compression tests pass.
1 task
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…s; add new overflow shapes Port from anomalyco/opencode#37848 (+ dev-branch twin NousResearch#37840): expand context-overflow patterns and guard against rate-limit messages that mention tokens. - 'Throttling error: Too many tokens, please wait before trying again.' (AWS Bedrock / proxy shape) classified as context_overflow and routed a healthy session into compression on every throttle. Added 'throttling' to _RATE_LIMIT_PATTERNS, which the message-only path checks BEFORE the overflow list. - 'Input length N exceeds the maximum allowed input length of M tokens.' (Together/Fireworks shape) fell through to unknown — no compression recovery. Added 'maximum allowed input length' to overflow patterns. - 'request_too_large' / 'Request exceeds the maximum size' (Anthropic 413 type re-wrapped without a status code by aggregators/proxies) fell through to unknown. Added to _PAYLOAD_TOO_LARGE_PATTERNS. All three shapes proven live on main before the fix; 265 classifier + bedrock tests and 238 sibling rate-guard/compression tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hermes mcp logindoes not depend on the server first returning HTTP 401.Verification
uv run --extra dev python -m pytest tests/hermes_cli/test_mcp_config.py tests/tools/test_mcp_oauth_manager.py -o addopts=''-> 43 passeduv run ruff check hermes_cli/mcp_config.py tools/mcp_oauth_manager.py tests/hermes_cli/test_mcp_config.py tests/tools/test_mcp_oauth_manager.py-> passedgit diff --check-> passedDuplicate check
Searched
NousResearch/hermes-agentPRs forforce oauth login preregistered clientsandmcp oauth preregistered clients; no duplicate PR was found.