Skip to content

perf(mcp): skip preflight probe on reconnect#40548

Closed
mohamedorigami-jpg wants to merge 1 commit into
NousResearch:mainfrom
mohamedorigami-jpg:fix/mcp-preflight-reconnect
Closed

perf(mcp): skip preflight probe on reconnect#40548
mohamedorigami-jpg wants to merge 1 commit into
NousResearch:mainfrom
mohamedorigami-jpg:fix/mcp-preflight-reconnect

Conversation

@mohamedorigami-jpg

Copy link
Copy Markdown
Contributor

Skip the MCP preflight content-type probe when reconnecting to a server that already connected successfully.

The preflight sends an initial HEAD request (which returns 405) followed by a GET (which returns 406). This adds about 3 seconds of network round-trip time on every reconnect with no actionable information, since the URL was already validated during the first connection.

The fix changes the condition from:
if config.get("transport") != "sse":
to:
if config.get("transport") != "sse" and not self._ready.is_set():

This ensures the preflight probe only runs on first connection, not on reconnects.

Closes #40366

@alpindiay alpindiay left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: PR #40548 -- perf(mcp): skip preflight probe on reconnect

Summary: Small, targeted change that skips the MCP preflight content-type probe when self._ready is already set (i.e., on reconnect).

What works well:

  • The logic is correct: if the connection is already established (_ready.is_set()), there is no need to re-probe the content type.
  • The change is minimal (one-line condition addition) and low-risk.
  • The comment block above the code already documents the rationale for the probe, and this change is consistent with that intent -- probe on first connect only.

Considerations:

  • No tests. While this is a small perf optimization that is difficult to test without mocking the ready event, a unit test verifying that _preflight_content_type is not called when _ready.is_set() would be ideal.
  • The condition order is fine: short-circuit evaluation means self._ready.is_set() is not checked for SSE transports, which is correct.

Verdict: LGTM. Approve as-is; the lack of a test is acceptable given the nature of the change.

@alt-glitch alt-glitch added type/perf Performance improvement or optimization P3 Low — cosmetic, nice to have tool/mcp MCP client and OAuth labels Jun 6, 2026
@teknium1

teknium1 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Salvaged into #40604 with your authorship credited (Co-authored-by). Re-verified on current main, tightened, and tested. Thanks!

#40604

@teknium1 teknium1 closed this Jun 6, 2026
teknium1 added a commit that referenced this pull request Jun 7, 2026
…ready (#40604)

Closes #40366.

Salvaged from #40548; re-verified on main, tightened, tested.

Co-authored-by: mohamedorigami-jpg <mohamedorigami-jpg@users.noreply.github.com>
changman pushed a commit to changman/hermes-agent that referenced this pull request Jun 10, 2026
…ready (NousResearch#40604)

Closes NousResearch#40366.

Salvaged from NousResearch#40548; re-verified on main, tightened, tested.

Co-authored-by: mohamedorigami-jpg <mohamedorigami-jpg@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have tool/mcp MCP client and OAuth type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(mcp): skip HTTP preflight probe on reconnect

4 participants