Uh oh!
There was an error while loading. Please reload this page.
Add structured content to the TypeScript weather server and client and update to v2 - #165
Open
olaservo wants to merge 9 commits into
Open
Conversation
Both tools declare an outputSchema and return structuredContent alongside the
text.
get-alerts declares z.array(...), so it answers with a top-level JSON array
rather than an array nested in an object, which protocol revision 2026-07-28
is the first to allow. "No alerts" is simply []. get-forecast returns an
object, for contrast.
This costs older clients nothing. The server declares the array schema once
and never branches on protocol version: serveStdio serves both eras from one
factory, and the SDK projects the schema down to
{"type":"object","properties":{"result":...}} for a 2025-11-25 client, wrapping
the structured content to match. Verified against both eras.
Error paths throw: a tool declaring an outputSchema MUST return conforming
structured content, so a path with no data has to fail.
The client passes versionNegotiation {mode:"auto"}; the SDK default is
"legacy". The SDK validates every result against the declared schema, so the
client-side SHOULD needs no code. Each channel goes to its stated reader:
content is forwarded to the model, structuredContent is used as data,
reporting how many items came back.
Moves to the 2.0 beta packages, where the 2026-07-28 support lives:
@modelcontextprotocol/sdk is replaced by @modelcontextprotocol/server and
@modelcontextprotocol/client. Model identifier moves to claude-sonnet-5.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Jul 26, 2026
There was a problem hiding this comment.
Pull request overview
Updates the TypeScript weather server/client examples to MCP SDK v2 (split client/server packages) and adds declared tool outputSchema with matching structuredContent, including an array-rooted schema for get-alerts to align with protocol revision 2026-07-28.
Changes:
- Migrate server to
@modelcontextprotocol/serverand useserveStdio(buildServer)with per-tooloutputSchema+structuredContentforget-alerts(array) andget-forecast(object). - Migrate client to
@modelcontextprotocol/client, enable automatic protocol version negotiation, and surface basic structured-output handling. - Update READMEs and dependency manifests/lockfiles for the new package split and Zod v4 usage.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| weather-server-typescript/src/index.ts | Switches to v2 server APIs, adds output schemas + structured results, and refactors server startup via serveStdio. |
| weather-server-typescript/README.md | Documents structured output behavior and backward-compat projection. |
| weather-server-typescript/package.json | Replaces @modelcontextprotocol/sdk with @modelcontextprotocol/server and adds zod. |
| weather-server-typescript/package-lock.json | Locks updated dependency graph for server v2 + Zod v4. |
| mcp-client-typescript/index.ts | Switches to v2 client APIs, enables versionNegotiation: auto, and distinguishes content vs structuredContent. |
| mcp-client-typescript/README.md | Documents structured output handling and version negotiation. |
| mcp-client-typescript/package.json | Replaces @modelcontextprotocol/sdk with @modelcontextprotocol/client. |
| mcp-client-typescript/package-lock.json | Locks updated dependency graph for client v2. |
Files not reviewed (2)
- mcp-client-typescript/package-lock.json: Generated file
- weather-server-typescript/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The 2.0 packages left beta on 2026-07-27, so the dependency ranges move off 2.0.0-beta.5. Also from review: engines.node goes to 20, which is what the 2.0 packages require, and the client README no longer implies every structured result is counted — only array-rooted ones are. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
get-alerts asked for /alerts?area=XX, which returns every alert NWS has on file for the state including expired ones. The Python, Go and Rust servers all ask for /alerts/active/area/XX. Measured against the live API for TX: 372 alerts before, 8 after, and the tools/call response drops from roughly 270KB to 5KB. The other four servers return the same 8. Pre-existing on main rather than introduced by the structured-output work, but it is the one thing that stopped the examples being comparable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
get-alerts published status and headline where Python, Go, Rust and Ruby all
publish description and instructions, and get-forecast periods used the NWS
camelCase spelling with shortForecast instead of detailed_forecast. The
divergence predates this round, but declaring an outputSchema turns it from a
difference in prose formatting into a difference in the published contract,
which defeats the point of a five-language example.
All five now advertise:
get_alerts -> [{event, area, severity, description, instructions}]
get_forecast -> {latitude, longitude, periods[{name, temperature,
temperature_unit, wind_speed, wind_direction,
detailed_forecast}]}
Also limits the forecast to the next 5 periods, which the other four already
did, and uses `??` on the alert fields because NWS sends explicit nulls
rather than omitting them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>The spec never uses the phrase "structured output". It defines two separate things under Tool Result: "Structured Content" (the `structuredContent` field) and "Output Schema" (the `outputSchema` field). Collapsing them into "structured output" conflates the two. It is also actively confusing here. In LLM tooling "structured output" means constrained decoding — making the *model* emit conforming JSON. These clients call a model API, so a reader could reasonably take the phrase to mean the tool constrains the model's response, which is the opposite of what is going on: the tool describes the shape of its own result. Headings and prose now say "structured content". References to real identifiers are left alone: the Python SDK's own docs page is called Structured Output and lives at docs/servers/structured-output.md, and its decorator parameter is `structured_output`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 8 changed files in this pull request and generated no new comments.
Files not reviewed (2)
- mcp-client-typescript/package-lock.json: Generated file
- weather-server-typescript/package-lock.json: Generated file
Suppressed comments (3)
weather-server-typescript/src/index.ts:226
- The new structured result turns a missing temperature into the valid-looking value
0°F.ForecastPeriod.temperatureis explicitly optional, so an incomplete upstream period will now publish incorrect machine-readable weather data rather than fail. Preserve the non-null output contract by rejecting a period that lacks its temperature (or unit) before constructing the result.
periods: rawPeriods.slice(0, 5).map((period) => ({
name: period.name ?? "Unknown",
temperature: period.temperature ?? 0,
temperature_unit: period.temperatureUnit ?? "F",
mcp-client-typescript/package.json:23
- The manifest now requires Node 20, but the root entry in
mcp-client-typescript/package-lock.jsonstill recordsengines.nodeas>=16.0.0. Regenerate/update the lockfile so installs and tooling see the same runtime contract.
"node": ">=20.0.0"
weather-server-typescript/package.json:25
- This dependency requires Node 20+, but unlike the updated client, the weather server package does not declare an
engines.nodefloor. Consumers on Node 18 can therefore select/install this example as if it were supported and only encounter the incompatibility through a transitive engine warning or at runtime. Add the Node 20 engine requirement here and refresh the lockfile root metadata.
"@modelcontextprotocol/server": "^2.0.0",
"zod": "^4.4.3"
| ## Structured content | ||
| Both tools declare an `outputSchema` and return `structuredContent`. `get-forecast` returns an object; `get-alerts` returns a top-level JSON array, which protocol revision `2026-07-28` is the first to allow — see [Structured Content](https://modelcontextprotocol.io/specification/draft/server/tools#structured-content) in the spec. |
Contributor
There was a problem hiding this comment.
Let's change the url from draft to https://modelcontextprotocol.io/specification/2026-07-28/server/tools#structured-content ?
| .array( | ||
| z.object({ | ||
| name: z.string(), | ||
| temperature: z.number(), |
Contributor
There was a problem hiding this comment.
Suggest making it nullable so we can set it to null later (not to 0)
temperature: z.number().nullable(),
| // Only show the next 5 periods. | ||
| periods: rawPeriods.slice(0, 5).map((period) => ({ | ||
| name: period.name ?? "Unknown", | ||
| temperature: period.temperature ?? 0, |
Contributor
There was a problem hiding this comment.
Suggest chaning it to:
temperature: period.temperature ?? null,
| process.exit(1); | ||
| }); | ||
| // One factory serves both protocol eras. | ||
| serveStdio(buildServer); |
Contributor
There was a problem hiding this comment.
Suggest error handling:
serveStdio(buildServer, {
onerror: (error) => {
console.error("Weather MCP Server error:", error);
},
});
| temperature: period.temperature ?? 0, | ||
| temperature_unit: period.temperatureUnit ?? "F", | ||
| wind_speed: period.windSpeed ?? "Unknown", | ||
| wind_direction: period.windDirection ?? "", |
Contributor
There was a problem hiding this comment.
Should we also set it to Unknown?
wind_direction: period.windDirection ?? "Unknown",
- Point the structured-content link at the dated 2026-07-28 spec rather than /draft, since the surrounding text names that revision. - Model a missing temperature as null instead of 0. `ForecastPeriod.temperature` is optional upstream, so the old `?? 0` published an incorrect but valid-looking 0degF rather than admitting the gap. The output schema now declares `z.number().nullable()`, and formatPeriod renders "Temperature: Unknown" for the null case instead of "null degF". - Fall back to "Unknown" for a missing wind direction, matching the neighbouring fields. - Report out-of-band errors via serveStdio's onerror callback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The client declared `>=20.0.0` and the server declared no floor at all, while its lockfile root still recorded `>=16.0.0`. Node 20 reached end of life on 2026-04-30, so it should not be what the examples ask people to install. Both packages now require `>=24.0.0`, the Active LTS, matching the Node 24 that ci.yml already provisions and the reasoning its comment already gives. The SDK's own floor is `>=20`, but that is the oldest runtime the library tolerates, not what an example should recommend. Lockfile roots regenerated; no dependency versions move. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Updates the TypeScript examples to MCP SDK 2.0.0 and protocol revision
2026-07-28, and gives both tools a declaredoutputSchemawith matchingstructuredContent.get-alertsnow returns a top-level JSON array. Before2026-07-28an output schema had to be object-rooted, so a tool returning a list had to invent a key to hang it off.get-forecastreturns an object, for contrast.Backwards compatibility note: The server declares the array schema once and never branches on protocol version — the SDK projects it for whichever era connects, so a
2025-11-25client still sees{"type":"object","properties":{"result":{...}}}and gets{"result":[...]}. Verified against both.Two pre-existing bugs fixed here
Both stopped the five examples being comparable. Declaring an
outputSchematurns them from formatting differences into contract differences, which is why they're fixed rather than left.get-alertsqueried/alerts?area=XX(every alert ever, including expired) where the other four query/alerts/active/area/XX. For TX: 372 alerts before, 8 after, and the response drops from ~270KB to ~5KB.get-alertspublished{event, area, severity, status, headline}against{event, area, severity, description, instructions}everywhere else, and forecast periods used NWS camelCase and weren't capped at 5. All five examples now advertise identical shapes.Verification
A real
get-alerts("TX")call against live NWS, captured off the raw wire with no SDK on the client side:outputSchema.type = "array",resultType: "complete",structuredContentan 8-item array,contentone block of prose. Called against all five servers in the same minute — identical values from every one.One of a set, one PR per language: #164 (Python, merged), #165 (TypeScript), #166 (Go), #167 (Rust); #163 was the shared test prerequisite and has merged. Ruby is not in the set — the
mcpgem never emits the mandatoryresultType, so a spec-strict client rejects every response. That needs an upstream fix.🤖 Generated with Claude Code