Skip to content

Commit a4a6100

Browse files
Merge branch 'main' into fweinberger/browser-stdio-conditional-export
2 parents c2b85db + 9ed62fe commit a4a6100

15 files changed

Lines changed: 450 additions & 13 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@modelcontextprotocol/client': minor
3+
---
4+
5+
Add `validateClientMetadataUrl()` utility for early validation of `clientMetadataUrl`
6+
7+
Exports a `validateClientMetadataUrl()` function that `OAuthClientProvider` implementations
8+
can call in their constructors to fail fast on invalid URL-based client IDs, instead of
9+
discovering the error deep in the auth flow.

.github/workflows/release.yml

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,17 +63,20 @@ jobs:
6363
node-version: 24
6464
cache: pnpm
6565
cache-dependency-path: pnpm-lock.yaml
66-
registry-url: 'https://registry.npmjs.org'
6766

6867
- name: Install dependencies
6968
run: pnpm install
7069

70+
# pnpm@10 delegates `pnpm publish` to the npm CLI; OIDC trusted publishing
71+
# requires npm >=11.5.1, which Node 24's bundled npm only satisfies from
72+
# ~24.6 onward. Install a recent-enough npm so we don't depend on which Node patch resolves.
73+
- name: Ensure npm CLI supports OIDC trusted publishing
74+
run: npm install -g npm@11.5.1
75+
7176
- name: Publish to npm
7277
uses: changesets/action@6a0a831ff30acef54f2c6aa1cbbc1096b066edaf # v1
7378
with:
7479
publish: pnpm run ci:publish
7580
env:
7681
GITHUB_TOKEN: ${{ github.token }}
77-
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
78-
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
7982
NPM_CONFIG_PROVENANCE: 'true'

.github/workflows/update-spec-types.yml

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,17 @@ jobs:
5151
if: steps.check_changes.outputs.has_changes == 'true'
5252
env:
5353
GH_TOKEN: ${{ github.token }}
54+
# Skip lefthook pre-push (typecheck/lint/build); spec drift that breaks
55+
# typecheck should still open a PR so it can be fixed there.
56+
LEFTHOOK: 0
5457
run: |
5558
git config user.name "github-actions[bot]"
5659
git config user.email "github-actions[bot]@users.noreply.github.com"
5760
5861
git checkout -B update-spec-types
5962
git add packages/core/src/types/spec.types.ts
6063
git commit -m "chore: update spec.types.ts from upstream"
61-
git push -f origin update-spec-types
64+
git push -f --no-verify origin update-spec-types
6265
6366
# Create PR if it doesn't exist, or update if it does
6467
PR_BODY="This PR updates \`packages/core/src/types/spec.types.ts\` from the Model Context Protocol specification.
@@ -67,9 +70,11 @@ jobs:
6770
6871
This is an automated update triggered by the nightly cron job."
6972
70-
if gh pr view update-spec-types &>/dev/null; then
71-
echo "PR already exists, updating description..."
72-
gh pr edit update-spec-types --body "$PR_BODY"
73+
# `gh pr view <branch>` matches closed PRs too, so check for an *open* PR explicitly.
74+
EXISTING_PR=$(gh pr list --head update-spec-types --state open --json number --jq '.[0].number // empty')
75+
if [ -n "$EXISTING_PR" ]; then
76+
echo "PR #$EXISTING_PR already exists, updating description..."
77+
gh pr edit "$EXISTING_PR" --body "$PR_BODY"
7378
else
7479
gh pr create \
7580
--title "chore: update spec.types.ts from upstream" \

REVIEW.md

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
# typescript-sdk Review Conventions
2+
3+
Guidance for reviewing pull requests on this repository. The first three sections are
4+
stable principles; the **Recurring Catches** section is auto-maintained from past human
5+
review rounds and grows over time.
6+
7+
## Guiding Principles
8+
9+
1. **Minimalism** — The SDK should do less, not more. Protocol correctness, transport
10+
lifecycle, types, and clean handler context belong in the SDK. Middleware engines,
11+
registry managers, builder patterns, and content helpers belong in userland.
12+
2. **Burden of proof is on addition** — The default answer to "should we add this?" is
13+
no. Removing something from the public API is far harder than not adding it.
14+
3. **Justify with concrete evidence** — Every new abstraction needs a concrete consumer
15+
today. Ask for real issues, benchmarks, real-world examples; apply the same standard
16+
to your own review (link spec sections, link code, show the simpler alternative).
17+
4. **Spec is the anchor** — The SDK implements the protocol spec. The further a feature
18+
drifts from the spec, the stronger the justification needs to be.
19+
5. **Kill at the highest level** — If the design is wrong, don't review the
20+
implementation. Lead with the highest-level concern; specific bugs are supporting
21+
detail.
22+
6. **Decompose by default** — A PR doing multiple things should be multiple PRs unless
23+
there's a strong reason to bundle.
24+
25+
## Review Ordering
26+
27+
1. **Design justification** — Is the overall approach sound? Is the complexity warranted?
28+
2. **Structural concerns** — Is the architecture right? Are abstractions justified?
29+
3. **Correctness** — Bugs, regressions, missing functionality.
30+
4. **Style and naming** — Nits, conventions, documentation.
31+
32+
## Checklist
33+
34+
**Protocol & spec**
35+
- Types match [`schema.ts`](https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/draft/schema.ts) exactly (optional vs required fields)
36+
- Correct `ProtocolError` codes (enum `ProtocolErrorCode`); HTTP status codes match spec (e.g., 404 vs 410)
37+
- Works for both stdio and Streamable HTTP transports — no transport-specific assumptions
38+
- Cross-SDK consistency: check what `python-sdk` does for the same feature
39+
40+
**API surface**
41+
- Every new export is intentional (see CLAUDE.md § Public API Exports); helpers users can write themselves belong in a cookbook, not the SDK
42+
- New abstractions have at least one concrete callsite in the PR
43+
- One way to do things — improving an existing API beats adding a parallel one
44+
45+
**Correctness**
46+
- Async: race conditions, cleanup on cancellation, unhandled rejections, missing `await`
47+
- Error propagation: caught/rethrown properly, resources cleaned up on error paths
48+
- Type safety: no unjustified `any`, no unsafe `as` assertions
49+
- Backwards compat: public-interface changes, default changes, removed exports — flagged and justified
50+
51+
**Tests & docs**
52+
- New behavior has vitest coverage including error paths
53+
- Breaking changes documented in `docs/migration.md` and `docs/migration-SKILL.md`
54+
- Bugfix or behavior change: check whether `docs/**/*.md` describes the old behavior and needs updating; flag prose that now contradicts the implementation
55+
- New feature: verify prose documentation is added (not just JSDoc), and assess whether `examples/` needs a new or updated example
56+
- Behavior change: assess whether existing `examples/` still compile and demonstrate the current API
57+
58+
## Reference
59+
60+
When verifying spec compliance, consult the spec directly rather than relying on memory:
61+
62+
- MCP documentation server: `https://modelcontextprotocol.io/mcp`
63+
- Full spec text (single file, LLM-friendly): `https://modelcontextprotocol.io/llms-full.txt` — fetch to a temp file and grep for the relevant section
64+
- Schema source of truth: [`schema.ts`](https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/draft/schema.ts)
65+
66+
## Recurring Catches
67+
68+
### HTTP Transport
69+
70+
- When validating `Mcp-Session-Id`, return **400** for a missing header and **404** for an unknown/expired session — never conflate `!sessionId || !transports[sessionId]` into one status, because the client needs to distinguish "fix your request" from "start a new session". Flag any diff that branches on session-id presence/lookup with a single 4xx. (#1707, #1770)
71+
72+
### Error Handling
73+
74+
- Broad `catch` blocks must not emit client-fault JSON-RPC codes (`-32700` ParseError, `-32602` InvalidParams) for server-internal failures like stream setup, task-store misses, or polling errors — map those to `-32603` InternalError so clients don't retry/reformat pointlessly. Flag any catch-all that hard-codes ParseError/InvalidParams without discriminating the thrown cause. (#1752, #1769)
75+
76+
### Schema Compliance
77+
78+
- When editing Zod protocol schemas in `schemas.ts`, verify unknown-key handling matches the spec `schema.ts`: if the spec type has no `additionalProperties: false`, the SDK schema must use `z.looseObject()` / `.catchall(z.unknown())` rather than implicit strict — over-strict Zod (incl. `z.literal('object')` on `type`) rejects spec-valid payloads from other SDKs. Also confirm `spec.types.test.ts` still passes bidirectionally. (#1768, #1849, #1169)
79+
80+
### Async / Lifecycle
81+
82+
- In `close()` / shutdown paths, wrap user-supplied or chained callbacks (`onclose?.()`, cancel fns) in `try/finally` so a throw can't skip the remaining teardown (`abort()`, `_onclose()`, map clears) — otherwise the transport is left half-open. (#1735, #1763)
83+
- Deferred callbacks (`setTimeout`, `.finally()`, reconnect closures) must check closed/aborted state before mutating `this._*` or starting I/O — a callback scheduled pre-close can fire after close/reconnect and corrupt the new connection's state (e.g., delete the new request's `AbortController`). (#1735, #1763)
84+
85+
### Completeness
86+
87+
- When a PR replaces a pattern (error class, auth-flow step, catch shape), grep the package for surviving instances of the old form — partial migrations leave sibling code paths with the very bug the PR claims to fix. Flag every leftover site. (#1657, #1761, #1595)
88+
89+
### Documentation & Changesets
90+
91+
- Read added `.changeset/*.md` text and new inline comments against the implementation in the same diff — prose that promises behavior the code no longer ships misleads consumers and contradicts stated intent. Flag any claim the diff doesn't back. (#1718, #1838)
92+
93+
### CI & GitHub Actions
94+
95+
- Do **not** assert that a third-party GitHub Action or publish toolchain will fail or needs extra permissions/tokens without verifying its docs or source — `pnpm publish` delegates to the system npm CLI (so npm OIDC works), and `changesets/action` in publish mode has no PR-comment step requiring `pull-requests: write`. For diffs under `.github/workflows/`, confirm claimed behavior in the action's README/source before flagging. (#1838, #1836)

docs/migration-SKILL.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ Notes:
8686
| `JSONRPCError` | `JSONRPCErrorResponse` |
8787
| `JSONRPCErrorSchema` | `JSONRPCErrorResponseSchema` |
8888
| `isJSONRPCError` | `isJSONRPCErrorResponse` |
89-
| `isJSONRPCResponse` | `isJSONRPCResultResponse` |
89+
| `isJSONRPCResponse` (deprecated in v1) | `isJSONRPCResultResponse` (**not** v2's new `isJSONRPCResponse`, which correctly matches both result and error) |
9090
| `ResourceReference` | `ResourceTemplateReference` |
9191
| `ResourceReferenceSchema` | `ResourceTemplateReferenceSchema` |
9292
| `IsomorphicHeaders` | REMOVED (use Web Standard `Headers`) |
@@ -98,7 +98,7 @@ Notes:
9898
| `StreamableHTTPError` | REMOVED (use `SdkError` with `SdkErrorCode.ClientHttp*`) |
9999
| `WebSocketClientTransport` | REMOVED (use `StreamableHTTPClientTransport` or `StdioClientTransport`) |
100100

101-
All other symbols from `@modelcontextprotocol/sdk/types.js` retain their original names (e.g., `CallToolResultSchema`, `ListToolsResultSchema`, etc.).
101+
All other **type** symbols from `@modelcontextprotocol/sdk/types.js` retain their original names. **Zod schemas** (e.g., `CallToolResultSchema`, `ListToolsResultSchema`) are no longer part of the public API — they are internal to the SDK. For runtime validation, use type guard functions like `isCallToolResult` instead of `CallToolResultSchema.safeParse()`.
102102

103103
### Error class changes
104104

@@ -435,6 +435,13 @@ const tool = await client.callTool({ name: 'my-tool', arguments: {} });
435435

436436
Remove unused schema imports: `CallToolResultSchema`, `CompatibilityCallToolResultSchema`, `ElicitResultSchema`, `CreateMessageResultSchema`, etc., when they were only used in `request()`/`send()`/`callTool()` calls.
437437

438+
If `CallToolResultSchema` was used for **runtime validation** (not just as a `request()` argument), replace with the `isCallToolResult` type guard:
439+
440+
| v1 pattern | v2 replacement |
441+
| --------------------------------------------------- | -------------------------- |
442+
| `CallToolResultSchema.safeParse(value).success` | `isCallToolResult(value)` |
443+
| `CallToolResultSchema.parse(value)` | Use `isCallToolResult(value)` then cast, or use `CallToolResult` type |
444+
438445
## 12. Experimental: `TaskCreationParams.ttl` no longer accepts `null`
439446

440447
`TaskCreationParams.ttl` changed from `z.union([z.number(), z.null()]).optional()` to `z.number().optional()`. Per the MCP spec, `null` TTL (unlimited lifetime) is only valid in server responses (`Task.ttl`), not in client requests. Omit `ttl` to let the server decide.

docs/migration.md

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,18 @@ const result = await client.callTool({ name: 'my-tool', arguments: {} });
444444

445445
The return type is now inferred from the method name via `ResultTypeMap`. For example, `client.request({ method: 'tools/call', ... })` returns `Promise<CallToolResult | CreateTaskResult>`.
446446

447+
If you were using `CallToolResultSchema` for **runtime validation** (not just in `request()`/`callTool()` calls), use the new `isCallToolResult` type guard instead:
448+
449+
```typescript
450+
// v1: runtime validation with Zod schema
451+
import { CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js';
452+
if (CallToolResultSchema.safeParse(value).success) { /* ... */ }
453+
454+
// v2: use the type guard
455+
import { isCallToolResult } from '@modelcontextprotocol/client';
456+
if (isCallToolResult(value)) { /* ... */ }
457+
```
458+
447459
### Client list methods return empty results for missing capabilities
448460

449461
`Client.listPrompts()`, `listResources()`, `listResourceTemplates()`, and `listTools()` now return empty results when the server didn't advertise the corresponding capability, instead of sending the request. This respects the MCP spec's capability negotiation.
@@ -484,14 +496,16 @@ The following deprecated type aliases have been removed from `@modelcontextproto
484496
| `JSONRPCError` | `JSONRPCErrorResponse` |
485497
| `JSONRPCErrorSchema` | `JSONRPCErrorResponseSchema` |
486498
| `isJSONRPCError` | `isJSONRPCErrorResponse` |
487-
| `isJSONRPCResponse` | `isJSONRPCResultResponse` |
499+
| `isJSONRPCResponse` | `isJSONRPCResultResponse` (see note below) |
488500
| `ResourceReferenceSchema` | `ResourceTemplateReferenceSchema` |
489501
| `ResourceReference` | `ResourceTemplateReference` |
490502
| `IsomorphicHeaders` | Use Web Standard `Headers` |
491503
| `AuthInfo` (from `server/auth/types.js`) | `AuthInfo` (now re-exported by `@modelcontextprotocol/client` and `@modelcontextprotocol/server`) |
492504

493505
All other types and schemas exported from `@modelcontextprotocol/sdk/types.js` retain their original names — import them from `@modelcontextprotocol/client` or `@modelcontextprotocol/server`.
494506

507+
> **Note on `isJSONRPCResponse`:** v1's `isJSONRPCResponse` was a deprecated alias that only checked for *result* responses (it was equivalent to `isJSONRPCResultResponse`). v2 removes the deprecated alias and introduces a **new** `isJSONRPCResponse` with corrected semantics — it checks for *any* response (either result or error). If you are migrating v1 code that used `isJSONRPCResponse`, rename it to `isJSONRPCResultResponse` to preserve the original behavior. Use the new `isJSONRPCResponse` only when you want to match both result and error responses.
508+
495509
**Before (v1):**
496510

497511
```typescript

examples/client/src/simpleOAuthClientProvider.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { OAuthClientInformationMixed, OAuthClientMetadata, OAuthClientProvider, OAuthTokens } from '@modelcontextprotocol/client';
2+
import { validateClientMetadataUrl } from '@modelcontextprotocol/client';
23

34
/**
45
* In-memory OAuth client provider for demonstration purposes
@@ -15,6 +16,9 @@ export class InMemoryOAuthClientProvider implements OAuthClientProvider {
1516
onRedirect?: (url: URL) => void,
1617
public readonly clientMetadataUrl?: string
1718
) {
19+
// Validate clientMetadataUrl at construction time (fail-fast)
20+
validateClientMetadataUrl(clientMetadataUrl);
21+
1822
this._onRedirect =
1923
onRedirect ||
2024
(url => {

packages/client/src/client/auth.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,28 @@ async function authInternal(
804804
return 'REDIRECT';
805805
}
806806

807+
/**
808+
* Validates that the given `clientMetadataUrl` is a valid HTTPS URL with a non-root pathname.
809+
*
810+
* No-op when `url` is `undefined` or empty (providers that do not use URL-based client IDs
811+
* are unaffected). When the value is defined but invalid, throws an {@linkcode OAuthError}
812+
* with code {@linkcode OAuthErrorCode.InvalidClientMetadata}.
813+
*
814+
* {@linkcode OAuthClientProvider} implementations that accept a `clientMetadataUrl` should
815+
* call this in their constructors for early validation.
816+
*
817+
* @param url - The `clientMetadataUrl` value to validate (from `OAuthClientProvider.clientMetadataUrl`)
818+
* @throws {OAuthError} When `url` is defined but is not a valid HTTPS URL with a non-root pathname
819+
*/
820+
export function validateClientMetadataUrl(url: string | undefined): void {
821+
if (url && !isHttpsUrl(url)) {
822+
throw new OAuthError(
823+
OAuthErrorCode.InvalidClientMetadata,
824+
`clientMetadataUrl must be a valid HTTPS URL with a non-root pathname, got: ${url}`
825+
);
826+
}
827+
}
828+
807829
/**
808830
* SEP-991: URL-based Client IDs
809831
* Validate that the `client_id` is a valid URL with `https` scheme

packages/client/src/client/authExtensions.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,16 @@ export interface PrivateKeyJwtProviderOptions {
233233
* Space-separated scopes values requested by the client.
234234
*/
235235
scope?: string;
236+
237+
/**
238+
* Optional custom claims to include in the JWT assertion.
239+
* These are merged with the standard claims (`iss`, `sub`, `aud`, `exp`, `iat`, `jti`),
240+
* with custom claims taking precedence for any overlapping keys.
241+
*
242+
* Useful for including additional claims that help scope the access token
243+
* with finer granularity than what scopes alone allow.
244+
*/
245+
claims?: Record<string, unknown>;
236246
}
237247

238248
/**
@@ -277,7 +287,8 @@ export class PrivateKeyJwtProvider implements OAuthClientProvider {
277287
subject: options.clientId,
278288
privateKey: options.privateKey,
279289
alg: options.algorithm,
280-
lifetimeSeconds: options.jwtLifetimeSeconds
290+
lifetimeSeconds: options.jwtLifetimeSeconds,
291+
claims: options.claims
281292
});
282293
}
283294

packages/client/src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ export {
3434
selectClientAuthMethod,
3535
selectResourceURL,
3636
startAuthorization,
37-
UnauthorizedError
37+
UnauthorizedError,
38+
validateClientMetadataUrl
3839
} from './client/auth.js';
3940
export type {
4041
AssertionCallback,

0 commit comments

Comments
 (0)