Skip to content

docs: converge README Test servers section with main's restructure - #1880

Merged
cliffhall merged 2 commits into
v2/mainfrom
v2/docs/1874-readme-convergence
Aug 1, 2026
Merged

docs: converge README Test servers section with main's restructure#1880
cliffhall merged 2 commits into
v2/mainfrom
v2/docs/1874-readme-convergence

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #1874

Step 2 of the ordered plan in #1821. Docs-only.

What was transplanted

main restructured the whole "Test servers" section in #1836; v2/main never got it. This brings that structure over — and only that section:

  • New ### Serving the modern protocol era heading, with the transport.modern / ServerConfig options as a bullet list instead of a wall-of-text paragraph.
  • New ### Showcase configs heading with a config → demonstrates → issue table indexing all nine showcase servers.
  • Eight #### per-config subsections (MRTR, Network tab, x-mcp-header, page-by-page fetching, advertised extensions, logging, resource subscriptions, tasks), replacing the eight single long paragraphs v2/main still carried. Sub-tables for the MRTR presets and the trigger_* error taxonomy; numbered steps for the advertised-extensions walkthrough; legacy/modern bullet pairs for the era-fork configs.

No content was dropped — every test-servers/configs/*.json and behavior described on v2/main is still described, just under headings.

The #1846 correction is preserved

main's restructured copy still carries the stale claim. Kept main's blockquote-callout shape, corrected wording:

Before (on main):

Mcp-Param-* mirroring is skipped by the SDK in the browser (detectProbeEnvironment() !== "browser"). So calling get_weather from the web client omits Mcp-Param-City and the strict server answers -32020. The same tool is callable from the Node CLI/TUI, where mirroring is active.

After (this PR):

Mcp-Param-* mirroring is built by the Inspector, not the SDK. The SDK only mirrors inside client.callTool(), and skips it in the browser (detectProbeEnvironment() !== "browser"). The Inspector routes tools/call through client.request() to drive MRTR manually, so it builds the mirrored headers itself (#1846) — on every client, web included, since the web client's upstream request is issued by the Node backend rather than the browser. So get_weather is callable from web, CLI, and TUI alike, in both the plain and "Run as task" forms.

Not a whole-file copy

git checkout origin/main -- README.md was not used — it would have reverted v2/main-only content. The diff against v2/main is confined to lines 118–132, i.e. entirely inside the Test-servers section:

@@ -118 +118 @@   …   @@ -132 +143,97 @@

Nothing in #1866's regions (the "Repo status" callout near the top, the Project-layout tree, and the "Cutting a release" section) is touched, so this stays mergeable alongside it.

Residual diff vs main

git diff origin/main -- README.md now shows exactly three things, all expected:

  1. The tools/call omits SEP-2243 Mcp-Param-* headers (x-mcp-header mirroring), rejected by strict modern servers #1846 correction above — the intended, deliberate divergence. This is the single 3-line hunk the first milestone merge will conflict on, where you take v2/main's side.
  2. The quality-gate script table, prettier-reformatted on main — untouched here, merges cleanly (v2/main hasn't edited it since the merge base).
  3. The trusted-publishing paragraph in "Cutting a release" — inside docs: post-swap branch model, PR policy, milestone rule, and Copilot review instructions #1866's region, deliberately left alone.

Payoff

The first v2/mainmain milestone merge (#1821 step 5) drops from a whole-section README conflict to one 3-line hunk.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU

Transplants main's restructured "Test servers" section (#1836) onto
v2/main — the "Serving the modern protocol era" and "Showcase configs"
headings, the config table, and the per-config subsections — replacing
the eight long paragraphs v2/main still carried.

Folds in v2/main's #1846 correction: main's copy still claims
Mcp-Param-* mirroring is skipped in the browser, which stopped being
true when the Inspector took over building the mirrored headers itself.
The blockquote callout keeps main's shape with the corrected wording.

Scoped to that section only — the whole file is deliberately NOT copied
from main, so #1866's repo-status callout and release-section edits (and
every other v2/main-only line) are untouched.

Shrinks the first v2/main -> main milestone merge from a whole-section
conflict to a single 3-line hunk.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Aug 1, 2026
@cliffhall
cliffhall requested a review from Copilot August 1, 2026 00:30

Copilot AI 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.

Pull request overview

Updates the repository’s README.md Test servers documentation on v2/main to match the restructured layout introduced on main, while preserving the corrected explanation of Mcp-Param-* header mirroring behavior (#1846 context).

Changes:

  • Replaces a long “modern protocol era” paragraph with a structured “Serving the modern protocol era” subsection and bullets.
  • Adds a “Showcase configs” index table and breaks the previous wall-of-text descriptions into focused per-config subsections.
  • Keeps (and re-words) the Mcp-Param-* mirroring callout to reflect Inspector behavior rather than the SDK.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread README.md Outdated
#1846 is the issue; the fix shipped as PR #1847. The callout's link used
/pull/1846, which 404s. Per Copilot review.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YAt8rqxysNbhYWLhoRm3fU

Copilot AI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

README.md:129

  • The global guidance here says “unless noted, connect with Protocol Era = Modern”, but several listed showcase configs (e.g. pagination-http.json, advertised-extensions-http.json) do not set transport.modern and therefore can’t be reached when the client pins Modern (it will try to negotiate 2026-07-28 and fail). The intro should distinguish legacy-only configs from modern-serving ones so readers don’t hit a connection error following the instructions.
Each config below is a ready-made server for exercising one feature by hand. Load one with `--config`, and unless noted, connect with **Protocol Era = Modern**.

@cliffhall

Copy link
Copy Markdown
Member Author

Round 2 of the Copilot loop came back clean — "reviewed 1 out of 1 changed files … generated no new comments". Round 1's only finding (the #1846 link pointing at /pull/1846, which 404s since #1846 is the issue and the fix shipped as PR #1847) is fixed in f7decc7.

Noting the one suppressed comment for the record, and why it is deliberately not addressed here:

The global guidance here says "unless noted, connect with Protocol Era = Modern", but several listed showcase configs (e.g. pagination-http.json, advertised-extensions-http.json) do not set transport.modern

Fair point, and partly already covered — the advertised-extensions subsection does say "Legacy stateful leg only". But that sentence is verbatim transplanted text from main, not authored in this PR. Rewording it here would create a second divergence between the two copies of this section, which is exactly what #1874 exists to eliminate: the whole point is to get the first v2/mainmain milestone merge down to the single intended 3-line hunk. Better addressed after the merge, when there is one copy of the section to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Converge README.md on v2/main with main's restructured Test servers section

2 participants