Skip to content

fix(mcp): do not ask for roots without the event stream - #42300

Closed
Yury Semikhatsky (yury-s) wants to merge 2 commits into
microsoft:mainfrom
yury-s:fix-42256
Closed

fix(mcp): do not ask for roots without the event stream#42300
Yury Semikhatsky (yury-s) wants to merge 2 commits into
microsoft:mainfrom
yury-s:fix-42256

Conversation

@yury-s

Copy link
Copy Markdown
Member

Summary

  • Server-to-client requests go over the standalone GET event stream. A streamable HTTP client that advertises roots but never opens that stream can't be delivered roots/list, so the request was dropped by the transport and the first tools/call stalled for the SDK's 60s default timeout.
  • Skip listRoots() when the transport never became bidirectionally ready, instead of sending a request that could only time out.

Fixes #42256

Pretty printing round-tripped the body through JSON.parse/JSON.stringify,
which turns every number into a double. Integers beyond
Number.MAX_SAFE_INTEGER were silently rounded and number literals were
normalized.

Keep the source text of each number via JSON.rawJSON, falling back to the
old behavior on engines that do not support it.

References microsoft#42261
@yury-s

Copy link
Copy Markdown
Member Author

If the client advertises capabilities?.roots => both server->client transport and list/roots requests should work.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

19 flaky ⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/popup.spec.ts:260 › should not throw when click closes popup `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@realtime-time-library-chromium-linux`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@realtime-time-library-chromium-linux`
⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/beforeunload.spec.ts:130 › should support dismissing the dialog multiple times `@chromium-ubuntu-22.04-node22`
⚠️ [chromium-library] › library/global-fetch.spec.ts:293 › should return security details from response `@chromium-ubuntu-22.04-node22`
⚠️ [chromium-library] › library/har.spec.ts:639 › should have security details `@chromium-ubuntu-22.04-node22`
⚠️ [firefox-library] › library/global-fetch.spec.ts:293 › should return security details from response `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-library] › library/har.spec.ts:639 › should have security details `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-page] › page/page-emulate-media.spec.ts:144 › should keep reduced motion and color emulation after reload `@firefox-ubuntu-22.04-node20`
⚠️ [webkit-library] › library/global-fetch.spec.ts:293 › should return security details from response `@webkit-ubuntu-22.04-node20`
⚠️ [webkit-library] › library/har.spec.ts:639 › should have security details `@webkit-ubuntu-22.04-node20`

51183 passed, 1233 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

1 failed
❌ [chrome] › mcp/config.spec.ts:203 › chromiumSandbox › --sandbox enables the sandbox @mcp-ubuntu-latest-chrome

8122 passed, 1341 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Hi, I'm the Playwright bot and I took a look at the CI failures here.

🟢 CI is clear — the one failure is a pre-existing flake

[chrome] › mcp/config.spec.ts:203 › --sandbox enables the sandbox fails on pushes to main and on unrelated PRs with the same signature, so it isn't caused by this PR. The "tests 1" report has no real failures (19 flaky, 0 failed).

Details

Overall: This PR changes MCP roots/list handling (server.ts) and trace-viewer JSON pretty-printing (networkResourceDetails.tsx). Neither touches browser channel selection or sandbox launch, which is what the failing test asserts on. No group is attributable to this PR.

Pre-existing flake / infra

Not failures

  • The 19 entries in the "tests 1" report (global-fetch.spec.ts:293, har.spec.ts:639 security details, popup.spec.ts:260, beforeunload.spec.ts:130, page-emulate-media.spec.ts:144) are all flaky (passed on retry), across chromium/firefox/webkit — nothing to triage, and none overlap the MCP/trace-viewer code this PR touches.

Triaged by the Playwright bot - agent run

Sign up for free to 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.

[MCP]: first tools/call stalls ~60s for Streamable HTTP clients that never open the standalone SSE stream

1 participant