Merge remote-tracking branch 'origin/v2/feat/2144-oauth-revocation' into v2/feat/2144-oauth-revocation
This commit is contained in:
commit
c7b18adabf
4 changed files with 3326 additions and 1516 deletions
|
|
@ -649,9 +649,12 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
>();
|
||||
private rawWireRequestCounter = 0;
|
||||
// Abort controller for the in-flight ordinary (non-task) tool call. Aborting
|
||||
// it makes the SDK send a `notifications/cancelled` for that request (the MCP
|
||||
// cancellation flow) and reject the pending call, which `callTool` surfaces as
|
||||
// a `ToolCallCancelledError`. Undefined when no ordinary call is in flight.
|
||||
// it hands the SDK the MCP cancellation flow for that request and rejects the
|
||||
// pending call, which `callTool` surfaces as a `ToolCallCancelledError`. Which
|
||||
// signal reaches the server is the transport's business, not ours: a
|
||||
// per-request-stream transport on a 2026-era connection has that request's
|
||||
// stream aborted, and everything else gets `notifications/cancelled` (#2140).
|
||||
// Undefined when no ordinary call is in flight.
|
||||
// Task-augmented calls have a server-side task and are cancelled via
|
||||
// `cancelRequestorTask` instead, so they don't use this (#1458).
|
||||
private activeToolCallAbortController?: AbortController;
|
||||
|
|
@ -1171,8 +1174,10 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
if (this.requestTimeout !== undefined) {
|
||||
opts.timeout = this.requestTimeout;
|
||||
}
|
||||
// When provided, aborting this signal makes the SDK send a
|
||||
// `notifications/cancelled` for the request and reject it (#1458).
|
||||
// When provided, aborting this signal cancels the request and rejects it
|
||||
// (#1458). The SDK picks the wire signal from the transport: an aborted
|
||||
// per-request SSE stream on a 2026-era Streamable HTTP connection, and
|
||||
// `notifications/cancelled` everywhere else (#2140).
|
||||
if (signal) {
|
||||
opts.signal = signal;
|
||||
}
|
||||
|
|
@ -2978,10 +2983,13 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
|
||||
/**
|
||||
* Cancel the in-flight ordinary (non-task) tool call started by
|
||||
* {@link callTool}. Aborting its request makes the SDK send a
|
||||
* `notifications/cancelled` to the server (the MCP cancellation flow) and
|
||||
* reject the pending call, which `callTool` surfaces as a
|
||||
* {@link ToolCallCancelledError}.
|
||||
* {@link callTool}. Aborting its request runs the MCP cancellation flow and
|
||||
* rejects the pending call, which `callTool` surfaces as a
|
||||
* {@link ToolCallCancelledError}. The SDK chooses the wire signal from the
|
||||
* transport: on a 2026-era Streamable HTTP connection it aborts that
|
||||
* request's own SSE response stream — the spec's cancellation signal —
|
||||
* rather than sending `notifications/cancelled`, which is the stdio
|
||||
* mechanism and remains in use there (#2140).
|
||||
*
|
||||
* Task-augmented calls have a server-side task and are cancelled via
|
||||
* {@link cancelRequestorTask} instead — this is a no-op for them (and whenever
|
||||
|
|
|
|||
|
|
@ -203,6 +203,30 @@ export class MessageTrackingTransport implements Transport {
|
|||
return this.baseTransport.sessionId;
|
||||
}
|
||||
|
||||
/**
|
||||
* Forward the base transport's per-request-stream capability (#2140).
|
||||
*
|
||||
* The SDK's `Protocol.request` reads this off the transport it was handed —
|
||||
* which is always this wrapper — to decide how a cancelled request is
|
||||
* signalled: abort that request's own stream (the 2026-07-28 signal for
|
||||
* Streamable HTTP) or POST a `notifications/cancelled` (the stdio one).
|
||||
*
|
||||
* Not forwarding it made the wrapper *answer* the question, `undefined`, for
|
||||
* every client. So the Cancel button POSTed a notification even on a modern
|
||||
* Streamable HTTP connection, where a spec-compliant server acknowledges it
|
||||
* `202` and drops it — the tool kept running and only a disconnect really
|
||||
* cancelled anything. That hit the CLI and TUI as much as the web client:
|
||||
* their real `StreamableHTTPClientTransport` advertises the flag correctly
|
||||
* and this wrapper hid it.
|
||||
*
|
||||
* A base transport that says nothing stays `undefined` rather than becoming
|
||||
* `false`, since the SDK's check is `=== true` either way and inventing a
|
||||
* value here would misreport what the transport claimed.
|
||||
*/
|
||||
get hasPerRequestStream(): boolean | undefined {
|
||||
return this.baseTransport.hasPerRequestStream;
|
||||
}
|
||||
|
||||
// Implemented as a concrete method (rather than delegating the base
|
||||
// transport's optional `setProtocolVersion`) so the SDK Client always
|
||||
// invokes it after the initialize handshake — including for stdio, whose
|
||||
|
|
|
|||
|
|
@ -1022,6 +1022,36 @@ export function createRemoteApp(
|
|||
// Auth errors may reject the wait via onerror while send() also throws.
|
||||
void responseWait.catch(() => {});
|
||||
}
|
||||
|
||||
// #2140: this route is held open for the whole call, so the browser
|
||||
// aborting its `POST /api/mcp/send` mid-flight *is* the client's
|
||||
// cancellation signal on a per-request-stream upstream. Forward it as
|
||||
// `requestSignal` so the upstream StreamableHTTP transport closes that
|
||||
// request's SSE response stream, which is what the 2026-07-28 spec makes
|
||||
// the cancellation signal. A transport with no per-request stream (stdio,
|
||||
// SSE) ignores the option, and a *cancel* never reaches it as an abort
|
||||
// anyway: `RemoteClientTransport` withholds `hasPerRequestStream` there, so
|
||||
// the SDK keeps sending `notifications/cancelled`. Forwarding the
|
||||
// disconnect unconditionally is still right — a browser that went away
|
||||
// mid-call is not waiting for the answer whatever the transport is.
|
||||
const upstreamAbort = new AbortController();
|
||||
const clientSignal = c.req.raw.signal;
|
||||
const onClientDisconnect = () => {
|
||||
upstreamAbort.abort(clientSignal.reason);
|
||||
// The response will never arrive now; release the wait so this handler
|
||||
// unwinds instead of sitting on a promise nothing can settle.
|
||||
if (requestId !== undefined) {
|
||||
session.cancelRequestWait(requestId);
|
||||
}
|
||||
};
|
||||
if (clientSignal.aborted) {
|
||||
onClientDisconnect();
|
||||
} else {
|
||||
clientSignal.addEventListener("abort", onClientDisconnect, {
|
||||
once: true,
|
||||
});
|
||||
}
|
||||
|
||||
try {
|
||||
// SEP-2243: apply the client's mirrored headers to the upstream request,
|
||||
// restricted to the `Mcp-Param-` prefix so a client can't inject other
|
||||
|
|
@ -1031,6 +1061,7 @@ export function createRemoteApp(
|
|||
await session.transport.send(message, {
|
||||
relatedRequestId: relatedRequestId as string | number | undefined,
|
||||
...(paramHeaders && { headers: paramHeaders }),
|
||||
requestSignal: upstreamAbort.signal,
|
||||
});
|
||||
if (responseWait) {
|
||||
await responseWait;
|
||||
|
|
@ -1051,6 +1082,7 @@ export function createRemoteApp(
|
|||
const msg = err instanceof Error ? err.message : String(err);
|
||||
return c.json({ ok: false, kind: "transport_error", error: msg });
|
||||
} finally {
|
||||
clientSignal.removeEventListener("abort", onClientDisconnect);
|
||||
session.endSend();
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -261,6 +261,25 @@ export class RemoteClientTransport implements Transport {
|
|||
private readonly options: RemoteTransportOptions;
|
||||
private readonly config: import("../types.js").MCPServerConfig;
|
||||
|
||||
/**
|
||||
* Whether the upstream connection gives each request its own response stream
|
||||
* (#2140). The SDK's `Protocol.request` reads this: on a 2026-era connection
|
||||
* a transport advertising it has its per-request stream **aborted** as the
|
||||
* spec's cancellation signal, and no `notifications/cancelled` is sent; a
|
||||
* transport that does not gets the stdio mechanism instead.
|
||||
*
|
||||
* The real upstream transport lives on the backend, so this transport has to
|
||||
* answer for it. Streamable HTTP opens one POST — and one SSE response
|
||||
* stream — per request, so it qualifies; stdio and SSE multiplex every
|
||||
* request over one shared channel and must keep sending the notification.
|
||||
*
|
||||
* Advertising it is only half the fix: `requestSend` applies the SDK's
|
||||
* `requestSignal` to the browser-to-backend fetch, and `/api/mcp/send`
|
||||
* forwards that disconnect to the upstream `transport.send`, which is what
|
||||
* actually closes the stream the server is watching.
|
||||
*/
|
||||
readonly hasPerRequestStream: boolean;
|
||||
|
||||
/**
|
||||
* Intentionally returns undefined. The MCP Client checks transport.sessionId to detect
|
||||
* reconnects and skip initialize. Our _sessionId is the remote server's session ID, not
|
||||
|
|
@ -312,6 +331,7 @@ export class RemoteClientTransport implements Transport {
|
|||
) {
|
||||
this.options = options;
|
||||
this.config = config;
|
||||
this.hasPerRequestStream = config.type === "streamable-http";
|
||||
}
|
||||
|
||||
setAuthRecovery(handlers: AuthRecoveryHandlers | undefined): void {
|
||||
|
|
@ -757,10 +777,17 @@ export class RemoteClientTransport implements Transport {
|
|||
}),
|
||||
};
|
||||
|
||||
// #2140: aborting this fetch is the cancellation signal for a
|
||||
// per-request-stream upstream. The backend holds `/api/mcp/send` open for
|
||||
// the whole call (it awaits the JSON-RPC response), so the disconnect
|
||||
// reaches it mid-flight and it aborts the upstream request's SSE stream.
|
||||
// For stdio/SSE `hasPerRequestStream` is false, so the SDK never supplies
|
||||
// a `requestSignal` and this is undefined.
|
||||
const res = await this.fetchFn(`${this.baseUrl}/api/mcp/send`, {
|
||||
method: "POST",
|
||||
headers: this.headers,
|
||||
body: JSON.stringify(body),
|
||||
...(options?.requestSignal && { signal: options.requestSignal }),
|
||||
});
|
||||
|
||||
if (!res.ok) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue