Address @claude review: format core/, typed output-validation error, document progress heuristic
- Run Prettier on the 60 core/ files this PR touched — the deterministic import rewrite had collapsed multi-line imports and left throws unwrapped; core/ is in no format gate so it slipped past CI. Now Prettier-clean. - Default-path output-schema violation now throws ProtocolError(ProtocolErrorCode.InvalidParams, …) instead of a bare Error, matching the prior v1 contract (SDK threw a typed McpError with a code) so downstream `instanceof`/`.code` branching keeps working. - Document the "assumes serial construction" invariant on pollTaskToolCall's progress-subscription revival (the `_progressHandlers` new-key heuristic). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
19a6df7ad3
commit
f9f79fff54
14 changed files with 296 additions and 95 deletions
|
|
@ -6,7 +6,17 @@
|
|||
*/
|
||||
|
||||
import { vi } from "vitest";
|
||||
import type { ClientCapabilities, Implementation, LoggingLevel, Prompt, Resource, ResourceTemplateType as ResourceTemplate, ServerCapabilities, Task, Tool } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
ClientCapabilities,
|
||||
Implementation,
|
||||
LoggingLevel,
|
||||
Prompt,
|
||||
Resource,
|
||||
ResourceTemplateType as ResourceTemplate,
|
||||
ServerCapabilities,
|
||||
Task,
|
||||
Tool,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import { InspectorClientEventTarget } from "../inspectorClientEventTarget.js";
|
||||
import type {
|
||||
AppRendererClient,
|
||||
|
|
|
|||
|
|
@ -46,7 +46,38 @@ import {
|
|||
MessageTrackingTransport,
|
||||
type MessageTrackingCallbacks,
|
||||
} from "./messageTrackingTransport.js";
|
||||
import type { CallToolRequest, JSONRPCRequest, JSONRPCNotification, JSONRPCResultResponse, JSONRPCErrorResponse, ServerCapabilities, ClientCapabilities, Implementation, LoggingLevel, Tool, Resource, ResourceTemplateType as ResourceTemplate, Prompt, Root, CreateMessageRequest, CreateMessageResult, ElicitRequest, ElicitResult, ElicitRequestURLParams, CallToolResult, Task, Progress, ProgressToken, ListToolsRequest, ListResourcesRequest, ListResourceTemplatesRequest, ListPromptsRequest, ReadResourceRequest, GetPromptRequest, CompleteRequest } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
CallToolRequest,
|
||||
JSONRPCRequest,
|
||||
JSONRPCNotification,
|
||||
JSONRPCResultResponse,
|
||||
JSONRPCErrorResponse,
|
||||
ServerCapabilities,
|
||||
ClientCapabilities,
|
||||
Implementation,
|
||||
LoggingLevel,
|
||||
Tool,
|
||||
Resource,
|
||||
ResourceTemplateType as ResourceTemplate,
|
||||
Prompt,
|
||||
Root,
|
||||
CreateMessageRequest,
|
||||
CreateMessageResult,
|
||||
ElicitRequest,
|
||||
ElicitResult,
|
||||
ElicitRequestURLParams,
|
||||
CallToolResult,
|
||||
Task,
|
||||
Progress,
|
||||
ProgressToken,
|
||||
ListToolsRequest,
|
||||
ListResourcesRequest,
|
||||
ListResourceTemplatesRequest,
|
||||
ListPromptsRequest,
|
||||
ReadResourceRequest,
|
||||
GetPromptRequest,
|
||||
CompleteRequest,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type { Transport } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
RequestOptions,
|
||||
|
|
@ -633,9 +664,7 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
internal._requestHandlers.set(method, (request, ctx) => {
|
||||
const task = (request as { params?: { task?: unknown } })?.params?.task;
|
||||
if (this.receiverTasks && task != null) {
|
||||
return rawHandler(
|
||||
request as CreateMessageRequest & ElicitRequest,
|
||||
);
|
||||
return rawHandler(request as CreateMessageRequest & ElicitRequest);
|
||||
}
|
||||
return validating(request, ctx);
|
||||
});
|
||||
|
|
@ -723,7 +752,10 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
private async getReceiverTaskPayload(taskId: string): Promise<ClientResult> {
|
||||
const record = this.receiverTaskRecords.get(taskId);
|
||||
if (!record) {
|
||||
throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Unknown taskId: ${taskId}`);
|
||||
throw new ProtocolError(
|
||||
ProtocolErrorCode.InvalidParams,
|
||||
`Unknown taskId: ${taskId}`,
|
||||
);
|
||||
}
|
||||
return record.payloadPromise;
|
||||
}
|
||||
|
|
@ -731,7 +763,10 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
private cancelReceiverTask(taskId: string): Task {
|
||||
const record = this.receiverTaskRecords.get(taskId);
|
||||
if (!record) {
|
||||
throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Unknown taskId: ${taskId}`);
|
||||
throw new ProtocolError(
|
||||
ProtocolErrorCode.InvalidParams,
|
||||
`Unknown taskId: ${taskId}`,
|
||||
);
|
||||
}
|
||||
if (InspectorClient.isTerminalTaskStatus(record.task.status)) {
|
||||
return record.task;
|
||||
|
|
@ -1012,7 +1047,10 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
this.addPendingSample(samplingRequest);
|
||||
});
|
||||
};
|
||||
this.client.setRequestHandler("sampling/createMessage", samplingHandler);
|
||||
this.client.setRequestHandler(
|
||||
"sampling/createMessage",
|
||||
samplingHandler,
|
||||
);
|
||||
if (this.receiverTasks) {
|
||||
this.installReceiverTaskResponseBypass(
|
||||
"sampling/createMessage",
|
||||
|
|
@ -2001,7 +2039,14 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
result as CallToolResult,
|
||||
);
|
||||
if (outputValidationError && !options?.skipOutputValidation) {
|
||||
throw new Error(outputValidationError);
|
||||
// Match the prior contract: on v1 a strict output-schema violation
|
||||
// surfaced as the SDK's typed `McpError`/`ProtocolError` (code
|
||||
// InvalidParams), not a bare Error — so downstream code that branches on
|
||||
// `instanceof ProtocolError` / `error.code` keeps working.
|
||||
throw new ProtocolError(
|
||||
ProtocolErrorCode.InvalidParams,
|
||||
outputValidationError,
|
||||
);
|
||||
}
|
||||
|
||||
const invocation: ToolCallInvocation = {
|
||||
|
|
@ -2161,6 +2206,14 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
);
|
||||
// The SDK registers the progress handler synchronously while constructing
|
||||
// the request promise (before this await), so the new key is present now.
|
||||
// ASSUMES SERIAL CONSTRUCTION: `find` takes the first key not present in the
|
||||
// pre-request snapshot, which is unambiguous only because no OTHER request
|
||||
// registers a progress handler between the snapshot and this request's
|
||||
// synchronous registration. Tool calls are user-driven and serial, so that
|
||||
// holds today; if concurrent task-augmented calls are ever constructed in
|
||||
// the same microtask window, two subscription ids could cross-wire and this
|
||||
// must move to an SDK-supported correlation (see the delete-when-native note
|
||||
// on `installReceiverTaskResponseBypass`).
|
||||
const progressSubscriptionId = requestOptions.onprogress
|
||||
? [...progressHandlers.keys()].find((k) => !keysBeforeRequest.has(k))
|
||||
: undefined;
|
||||
|
|
|
|||
|
|
@ -24,7 +24,17 @@ import type {
|
|||
ResourceReadInvocation,
|
||||
ResourceTemplateReadInvocation,
|
||||
} from "./types.js";
|
||||
import type { Tool, ServerCapabilities, Implementation, Root, Progress, ProgressToken, Task, CallToolResult, ProtocolError } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
Tool,
|
||||
ServerCapabilities,
|
||||
Implementation,
|
||||
Root,
|
||||
Progress,
|
||||
ProgressToken,
|
||||
Task,
|
||||
CallToolResult,
|
||||
ProtocolError,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type { SamplingCreateMessage } from "./samplingCreateMessage.js";
|
||||
import type { ElicitationCreateMessage } from "./elicitationCreateMessage.js";
|
||||
import type { JsonValue } from "../json/jsonUtils.js";
|
||||
|
|
|
|||
|
|
@ -20,7 +20,17 @@ import type {
|
|||
PromptGetInvocation,
|
||||
ToolCallInvocation,
|
||||
} from "./types.js";
|
||||
import type { ClientCapabilities, Implementation, LoggingLevel, Prompt, Resource, ResourceTemplateType as ResourceTemplate, ServerCapabilities, Task, Tool } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
ClientCapabilities,
|
||||
Implementation,
|
||||
LoggingLevel,
|
||||
Prompt,
|
||||
Resource,
|
||||
ResourceTemplateType as ResourceTemplate,
|
||||
ServerCapabilities,
|
||||
Task,
|
||||
Tool,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type { JsonValue } from "../json/jsonUtils.js";
|
||||
import type { InspectorClientEventTarget } from "./inspectorClientEventTarget.js";
|
||||
import type { SamplingCreateMessage } from "./samplingCreateMessage.js";
|
||||
|
|
|
|||
|
|
@ -1,6 +1,17 @@
|
|||
import type { Transport, TransportSendOptions } from "@modelcontextprotocol/client";
|
||||
import type { JSONRPCMessage, MessageExtraInfo } from "@modelcontextprotocol/client";
|
||||
import type { JSONRPCRequest, JSONRPCNotification, JSONRPCResultResponse, JSONRPCErrorResponse } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
Transport,
|
||||
TransportSendOptions,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
JSONRPCMessage,
|
||||
MessageExtraInfo,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
JSONRPCRequest,
|
||||
JSONRPCNotification,
|
||||
JSONRPCResultResponse,
|
||||
JSONRPCErrorResponse,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type { MessageOrigin } from "./types.js";
|
||||
|
||||
export interface MessageTrackingCallbacks {
|
||||
|
|
@ -21,10 +32,7 @@ export class MessageTrackingTransport implements Transport {
|
|||
private callbacks: MessageTrackingCallbacks;
|
||||
private negotiatedProtocolVersion?: string;
|
||||
|
||||
constructor(
|
||||
baseTransport: Transport,
|
||||
callbacks: MessageTrackingCallbacks,
|
||||
) {
|
||||
constructor(baseTransport: Transport, callbacks: MessageTrackingCallbacks) {
|
||||
this.baseTransport = baseTransport;
|
||||
this.callbacks = callbacks;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -286,8 +286,7 @@ export class OAuthManager {
|
|||
try {
|
||||
if (this.isEnterpriseManaged()) {
|
||||
const emaConfig = await this.getEmaFlowConfig();
|
||||
const scopeForMint =
|
||||
this.pendingAuthorizationScope ?? emaConfig.scope;
|
||||
const scopeForMint = this.pendingAuthorizationScope ?? emaConfig.scope;
|
||||
const config = scopeForMint
|
||||
? { ...emaConfig, scope: scopeForMint }
|
||||
: emaConfig;
|
||||
|
|
@ -449,7 +448,9 @@ export class OAuthManager {
|
|||
*/
|
||||
async refreshEnterpriseManagedTokens(): Promise<boolean> {
|
||||
if (!this.isEnterpriseManaged()) return false;
|
||||
const tokens = await refreshEmaResourceTokens(await this.getEmaFlowConfig());
|
||||
const tokens = await refreshEmaResourceTokens(
|
||||
await this.getEmaFlowConfig(),
|
||||
);
|
||||
return tokens !== undefined;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,9 @@
|
|||
import type { OAuthClientProvider } from "@modelcontextprotocol/client";
|
||||
import type { OAuthClientInformation, OAuthClientMetadata, OAuthTokens } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
OAuthClientInformation,
|
||||
OAuthClientMetadata,
|
||||
OAuthTokens,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type { RemoteAuthState } from "../types.js";
|
||||
|
||||
const REMOTE_OAUTH_STUB_METADATA: OAuthClientMetadata = {
|
||||
|
|
|
|||
|
|
@ -3,8 +3,14 @@
|
|||
* Pure TypeScript; works in browser, Deno, or Node.
|
||||
*/
|
||||
|
||||
import type { Transport, TransportSendOptions } from "@modelcontextprotocol/client";
|
||||
import type { JSONRPCMessage, MessageExtraInfo } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
Transport,
|
||||
TransportSendOptions,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
JSONRPCMessage,
|
||||
MessageExtraInfo,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type { InspectorServerSettings, StderrLogEntry } from "../types.js";
|
||||
import type { FetchRequestEntryBase } from "../types.js";
|
||||
import type {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,7 @@
|
|||
import type { CreateMessageRequest, CreateMessageResult } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
CreateMessageRequest,
|
||||
CreateMessageResult,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import { RELATED_TASK_META_KEY } from "@modelcontextprotocol/client";
|
||||
|
||||
export type { CreateMessageRequest, CreateMessageResult };
|
||||
|
|
|
|||
|
|
@ -106,9 +106,7 @@ type StoredInspectorFields = Pick<
|
|||
export function envRecordToPairs(
|
||||
env: Record<string, string> | undefined,
|
||||
): { key: string; value: string }[] {
|
||||
return env
|
||||
? Object.entries(env).map(([key, value]) => ({ key, value }))
|
||||
: [];
|
||||
return env ? Object.entries(env).map(([key, value]) => ({ key, value })) : [];
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -232,9 +230,7 @@ export function inspectorSettingsToStoredFields(
|
|||
out.headers = headersRecord;
|
||||
}
|
||||
|
||||
const metadataFiltered = settings.metadata.filter(
|
||||
(m) => m.key.trim() !== "",
|
||||
);
|
||||
const metadataFiltered = settings.metadata.filter((m) => m.key.trim() !== "");
|
||||
if (metadataFiltered.length > 0) {
|
||||
out.metadata = metadataFiltered;
|
||||
}
|
||||
|
|
@ -333,9 +329,7 @@ export const INSPECTOR_FIELD_KEYS = new Set(
|
|||
* `satisfies` constraint above forces the map update, which propagates
|
||||
* here.
|
||||
*/
|
||||
export function stripInspectorFields(
|
||||
stored: StoredMCPServer,
|
||||
): MCPServerConfig {
|
||||
export function stripInspectorFields(stored: StoredMCPServer): MCPServerConfig {
|
||||
const out: Record<string, unknown> = {};
|
||||
for (const [k, v] of Object.entries(
|
||||
stored as unknown as Record<string, unknown>,
|
||||
|
|
@ -361,7 +355,9 @@ export function mcpConfigToServerEntries(config: MCPConfig): ServerEntry[] {
|
|||
// `InspectorServerSettings` only.
|
||||
const inspectorFields: StoredInspectorFields = {};
|
||||
const sdkOnly: Record<string, unknown> = {};
|
||||
for (const [k, v] of Object.entries(raw as unknown as Record<string, unknown>)) {
|
||||
for (const [k, v] of Object.entries(
|
||||
raw as unknown as Record<string, unknown>,
|
||||
)) {
|
||||
if (INSPECTOR_FIELD_KEYS.has(k as keyof StoredInspectorFields)) {
|
||||
(inspectorFields as Record<string, unknown>)[k] = v;
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -194,9 +194,10 @@ export abstract class ManagedListState<
|
|||
* concrete subclass event maps keep the call sites type-safe.
|
||||
*/
|
||||
private emit(type: keyof M, detail: unknown): void {
|
||||
(
|
||||
this.dispatchTypedEvent as unknown as (t: keyof M, d: unknown) => void
|
||||
)(type, detail);
|
||||
(this.dispatchTypedEvent as unknown as (t: keyof M, d: unknown) => void)(
|
||||
type,
|
||||
detail,
|
||||
);
|
||||
}
|
||||
|
||||
private setListChanged(value: boolean): void {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,8 @@
|
|||
import type { CallToolResult, Tool } from "@modelcontextprotocol/client";
|
||||
import type { JsonSchemaType, jsonSchemaValidator } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
JsonSchemaType,
|
||||
jsonSchemaValidator,
|
||||
} from "@modelcontextprotocol/client";
|
||||
|
||||
/**
|
||||
* Validate a delivered tool result's `structuredContent` against the tool's
|
||||
|
|
|
|||
|
|
@ -1,4 +1,23 @@
|
|||
import type { CallToolResult, ClientNotification, ClientRequest, GetPromptResult, Implementation, JSONRPCErrorResponse, JSONRPCNotification, JSONRPCRequest, JSONRPCResultResponse, LoggingLevel, Prompt, ReadResourceResult, Resource, Root, ServerCapabilities, ServerNotification, ServerRequest, Tool } from "@modelcontextprotocol/client";
|
||||
import type {
|
||||
CallToolResult,
|
||||
ClientNotification,
|
||||
ClientRequest,
|
||||
GetPromptResult,
|
||||
Implementation,
|
||||
JSONRPCErrorResponse,
|
||||
JSONRPCNotification,
|
||||
JSONRPCRequest,
|
||||
JSONRPCResultResponse,
|
||||
LoggingLevel,
|
||||
Prompt,
|
||||
ReadResourceResult,
|
||||
Resource,
|
||||
Root,
|
||||
ServerCapabilities,
|
||||
ServerNotification,
|
||||
ServerRequest,
|
||||
Tool,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type { Client } from "@modelcontextprotocol/client";
|
||||
import type { OAuthClientProvider } from "@modelcontextprotocol/client";
|
||||
import type { Transport } from "@modelcontextprotocol/client";
|
||||
|
|
@ -149,7 +168,9 @@ export type ConnectionStatus =
|
|||
* (see InspectorClient's `onclose` handler, #1490). A bare `=== "disconnected"`
|
||||
* check would therefore tear down in one ordering but not the other.
|
||||
*/
|
||||
export function isTerminalStatus(status: ConnectionStatus | undefined): boolean {
|
||||
export function isTerminalStatus(
|
||||
status: ConnectionStatus | undefined,
|
||||
): boolean {
|
||||
return status === "disconnected" || status === "error";
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,8 @@
|
|||
import { ProtocolErrorCode, ProtocolError, UrlElicitationRequiredError } from "@modelcontextprotocol/client";
|
||||
import {
|
||||
ProtocolErrorCode,
|
||||
ProtocolError,
|
||||
UrlElicitationRequiredError,
|
||||
} from "@modelcontextprotocol/client";
|
||||
import type { ElicitRequestURLParams } from "@modelcontextprotocol/client";
|
||||
|
||||
export type { ElicitRequestURLParams };
|
||||
|
|
@ -46,7 +50,10 @@ export function getUrlElicitationsFromError(
|
|||
if (error instanceof UrlElicitationRequiredError) {
|
||||
return error.elicitations ?? [];
|
||||
}
|
||||
if (error instanceof ProtocolError && error.code === ProtocolErrorCode.UrlElicitationRequired) {
|
||||
if (
|
||||
error instanceof ProtocolError &&
|
||||
error.code === ProtocolErrorCode.UrlElicitationRequired
|
||||
) {
|
||||
const data = error.data as
|
||||
| { elicitations?: ElicitRequestURLParams[] }
|
||||
| undefined;
|
||||
|
|
|
|||
Loading…
Reference in a new issue