Skip to content

chore(mcp): remove temporary HTTP diagnostics - #5795

Merged
waleedlatif1 merged 1 commit into
stagingfrom
chore/remove-mcp-http-diagnostics
Jul 21, 2026
Merged

chore(mcp): remove temporary HTTP diagnostics#5795
waleedlatif1 merged 1 commit into
stagingfrom
chore/remove-mcp-http-diagnostics

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Removes the temporary McpHttpDiag instrumentation added in diag(mcp): comprehensive HTTP logging for OAuth + streamable-HTTP transport #5782. It did its job: it confirmed the OAuth fetch fix (fix(mcp): bound and tear down one-shot OAuth fetches so auth can't hang #5789) makes the flow reach and complete the token exchange (200, tokens saved, "Server Authorized"), and it isolated a separate, pre-existing transport-initialize stall — Gauge returns 200 application/json + Mcp-Session-Id then sends no body for 30s, reproducible only from the staging egress IP (a fresh IP + the real SDK + h2 + pinning + even the diagnostic's body tee() all connect in ~600ms).
  • Reverts client.ts and oauth/auth.ts to their exact pre-diagnostic form and deletes http-diagnostics.ts. Also removes the tee() from the transport hot path so the streamable-HTTP connection runs clean while the transport stall is investigated on its own.

What changed

Type of Change

  • Chore / cleanup

Testing

  • Full lib/mcp suite green (371 tests); tsc clean; biome clean.
  • Verified client.ts/client.test.ts are byte-identical to the pre-diagnostic originals; auth.ts differs only in biome line-wrapping.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Reverts the temporary McpHttpDiag instrumentation from #5782 now that it has
served its purpose: it confirmed the OAuth fetch fix (#5789) makes the flow reach
and complete the token exchange, and it isolated a separate, pre-existing
transport-initialize stall (Gauge returns 200 + session then no body for 30s,
reproducible only from the staging egress — not from a fresh IP, and not with the
SDK/h2/pinning/tee in isolation).
- Delete apps/sim/lib/mcp/http-diagnostics.ts.
- Restore client.ts transport fetch to `...(pinned ? { fetch: pinned.fetch } : {})`.
- Restore oauth/auth.ts fetchFn to the plain SSRF-guarded default.
- Drop the diagnostic-specific test assertion.
Removes the body tee() from the transport hot path so the streamable-HTTP
connection runs clean while the transport stall is investigated separately.
@vercel

vercelBot commented Jul 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
docsSkippedSkippedJul 21, 2026 3:42am

Request Review

@cursor

cursorBot commented Jul 21, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Cleanup that reverts temporary logging/instrumentation on MCP HTTP paths; no new auth or SSRF behavior beyond restoring the prior fetch wiring.

Overview
Removes the temporary McpHttpDiag layer added for OAuth/streamable-HTTP debugging by deleting http-diagnostics.ts and all withMcpHttpDiagnostics usage.

McpClient again passes a custom fetch only when IP pinning is active (...(pinned ? { fetch: pinned.fetch } : {})), so unpinned transports use the SDK default and no longer run the diagnostic tee() on the initialize response path. mcpAuthGuarded defaults OAuth fetchFn back to createSsrfGuardedMcpFetch() only, without the OAuth-phase wrapper.

The OAuth transport test expectation drops the diagnostic-only fetch: expect.any(Function) assertion.

Reviewed by Cursor Bugbot for commit be2038d. Configure here.

@greptile-apps

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes temporary MCP HTTP diagnostics and restores the earlier fetch paths.

  • Deletes the diagnostic fetch wrapper and logging module.
  • Uses the pinned fetch only for pinned MCP transports.
  • Keeps the SSRF-guarded default for OAuth requests.
  • Removes the diagnostic-specific transport test assertion.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • The deleted module has no remaining references.
  • Existing transport pinning and OAuth SSRF protection remain in place.

Important Files Changed

FilenameOverview
apps/sim/lib/mcp/client.tsRemoves diagnostic wrapping while retaining the pinned fetch for resolved MCP server addresses.
apps/sim/lib/mcp/oauth/auth.tsRemoves diagnostic wrapping while preserving the caller override and SSRF-guarded default.
apps/sim/lib/mcp/http-diagnostics.tsDeletes the temporary diagnostic module after all consumers were removed.
apps/sim/lib/mcp/client.test.tsRemoves the assertion that diagnostics always provide an explicit transport fetch.

Reviews (1): Last reviewed commit: "chore(mcp): remove temporary HTTP diagno..." | Re-trigger Greptile

@waleedlatif1
waleedlatif1 merged commit 1b2ee26 into stagingJul 21, 2026
15 checks passed
@waleedlatif1
waleedlatif1 deleted the chore/remove-mcp-http-diagnostics branch July 21, 2026 03:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1