From dbb8c40245c6af72a1ad0bca39f11316910018c7 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 15 Jul 2026 14:47:10 -0400 Subject: [PATCH] Address review: justify/refine SDK-gap casts (`as unknown as`) Per human review on #1688: for each `as unknown as` in the PR, either refactor or justify with a comment naming the SDK gap (so it can inform SDK-side type improvements). - Sampling/elicit receiver-task returns: route through a typed `CreateTaskResult` so the object shape IS checked; the remaining cast to CreateMessageResult / ElicitResult is the honest SDK gap (the 2-arg setRequestHandler overload types the handler return as the spec result only, not the task-augmented variant). - `_requestHandlers` / `_progressHandlers` private-field casts: comments now name the exact missing public API (opt a handler out of result validation; keep a progress subscription alive across a resolved request). - test-server `_requestHandlers` cast: same SDK gap on the server side, noted. - serverList object->Record casts (pre-existing): documented as plain structural widening (no index signature -> TS requires the `unknown` step), not an SDK workaround. Co-Authored-By: Claude Opus 4.8 (1M context) --- core/mcp/inspectorClient.ts | 40 ++++++++++++++++------ core/mcp/serverList.ts | 5 +++ test-servers/src/composable-test-server.ts | 6 ++++ 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index 478a4182..6d34e504 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -63,6 +63,7 @@ import type { Root, CreateMessageRequest, CreateMessageResult, + CreateTaskResult, ElicitRequest, ElicitResult, ElicitRequestURLParams, @@ -671,6 +672,12 @@ export class InspectorClient extends InspectorClientEventTarget { ) => Promise | Promise, ): void { if (!this.client) return; + // SDK gap: `Client` exposes no public way to (a) read a registered request + // handler or (b) opt one out of the result validation its `_wrapHandler` + // installs, so we reach the private `_requestHandlers` map through a + // narrowed cast. A public "register a raw/unvalidated handler" API — or a + // handler-result type that includes `CreateTaskResult` — would remove both + // this cast and the ones on the sampling/elicit returns above. const internal = this.client as unknown as { _requestHandlers: Map< string, @@ -1045,11 +1052,18 @@ export class InspectorClient extends InspectorClientEventTarget { // The v2 Client validates a spec handler's result and would reject // `{ task }` with -32602; `installReceiverTaskResponseBypass` below // routes this task-augmented branch around that validation so the - // legacy `{ task }` response reaches the wire. The cast satisfies the - // handler's declared `CreateMessageResult` return type. - return Promise.resolve({ - task: record.task, - } as unknown as CreateMessageResult); + // legacy `{ task }` response reaches the wire. `taskResult` is typed + // as `CreateTaskResult` so its shape IS checked; the unavoidable + // `as unknown as CreateMessageResult` bridges the SDK gap — the 2-arg + // `setRequestHandler` overload types a sampling handler's return as + // `CreateMessageResult` only and doesn't model the (deprecated but + // wire-valid) task-augmented `CreateTaskResult`. A handler-result + // union `CreateMessageResult | CreateTaskResult` on the SDK side + // would remove this cast. + const taskResult: CreateTaskResult = { task: record.task }; + return Promise.resolve( + taskResult as unknown as CreateMessageResult, + ); } return new Promise((resolve, reject) => { const samplingRequest = new SamplingCreateMessage( @@ -1123,11 +1137,13 @@ export class InspectorClient extends InspectorClientEventTarget { // Task-augmented (2025-11-25) response — see the sampling handler // above. Reply with a `CreateTaskResult` (`{ task }`), routed around // the v2 Client's result validation by - // `installReceiverTaskResponseBypass` below; the cast satisfies the - // handler's declared `ElicitResult` return type. - return Promise.resolve({ - task: record.task, - } as unknown as ElicitResult); + // `installReceiverTaskResponseBypass` below. `taskResult` is typed so + // its shape is checked; the `as unknown as ElicitResult` bridges the + // same SDK gap as the sampling handler — the 2-arg `setRequestHandler` + // overload types an elicitation handler's return as `ElicitResult` + // only and doesn't model the task-augmented `CreateTaskResult`. + const taskResult: CreateTaskResult = { task: record.task }; + return Promise.resolve(taskResult as unknown as ElicitResult); } return new Promise((resolve) => { const elicitationRequest = new ElicitationCreateMessage( @@ -2222,6 +2238,10 @@ export class InspectorClient extends InspectorClientEventTarget { // id the SDK registers for this request (the only new key in the private // `_progressHandlers` map) so we can keep the caller's `onprogress` alive // through the poll and clean it up when the task terminates. + // SDK gap: `Client` exposes no public API to keep a progress subscription + // alive across a resolved request (or to subscribe to progress by token), so + // we reach the private `_progressHandlers` map through a narrowed cast. A + // public "durable progress subscription" hook would remove this cast. const progressHandlers = ( client as unknown as { _progressHandlers: Map; diff --git a/core/mcp/serverList.ts b/core/mcp/serverList.ts index 58f7646f..becedb13 100644 --- a/core/mcp/serverList.ts +++ b/core/mcp/serverList.ts @@ -355,6 +355,11 @@ export function mcpConfigToServerEntries(config: MCPConfig): ServerEntry[] { // `InspectorServerSettings` only. const inspectorFields: StoredInspectorFields = {}; const sdkOnly: Record = {}; + // Widen the typed config object to a generic record to iterate its keys. + // `StoredMCPServer` has no index signature, so TS requires the `unknown` + // step (`as Record` alone is TS2352). This is a plain + // structural widening, not an SDK-shape workaround. (Pre-existing pattern, + // also at the `serverEntryToStored` / oauth-strip casts in this file.) for (const [k, v] of Object.entries( raw as unknown as Record, )) { diff --git a/test-servers/src/composable-test-server.ts b/test-servers/src/composable-test-server.ts index a8bcc298..44ac2fe6 100644 --- a/test-servers/src/composable-test-server.ts +++ b/test-servers/src/composable-test-server.ts @@ -1215,6 +1215,12 @@ function wireTaskHandlers( taskTools: Map, ): void { const lowLevel = mcpServer.server; + // SDK gap (server side): `Server` exposes no public API to override an already + // registered handler or to send a `tools/call` response that bypasses its + // result-schema validation, so we reach the private `_requestHandlers` map via + // a narrowed cast (`RawHandlerHost`). Mirrors the client-side bypass in + // `core/mcp/inspectorClient.ts`; both go away when the SDK models task-augmented + // results natively. const registry = (lowLevel as unknown as RawHandlerHost)._requestHandlers; const sdkToolsCall = registry.get("tools/call"); // Map each created task back to the tool that owns it, so tasks/get and