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) <noreply@anthropic.com>
This commit is contained in:
parent
39362dd5ff
commit
dbb8c40245
3 changed files with 41 additions and 10 deletions
|
|
@ -63,6 +63,7 @@ import type {
|
|||
Root,
|
||||
CreateMessageRequest,
|
||||
CreateMessageResult,
|
||||
CreateTaskResult,
|
||||
ElicitRequest,
|
||||
ElicitResult,
|
||||
ElicitRequestURLParams,
|
||||
|
|
@ -671,6 +672,12 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
) => Promise<CreateMessageResult> | Promise<ElicitResult>,
|
||||
): 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<CreateMessageResult>((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<ElicitResult>((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<number, ProgressCallback>;
|
||||
|
|
|
|||
|
|
@ -355,6 +355,11 @@ export function mcpConfigToServerEntries(config: MCPConfig): ServerEntry[] {
|
|||
// `InspectorServerSettings` only.
|
||||
const inspectorFields: StoredInspectorFields = {};
|
||||
const sdkOnly: Record<string, unknown> = {};
|
||||
// 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<string, unknown>` 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<string, unknown>,
|
||||
)) {
|
||||
|
|
|
|||
|
|
@ -1215,6 +1215,12 @@ function wireTaskHandlers(
|
|||
taskTools: Map<string, TaskToolDefinition>,
|
||||
): 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
|
||||
|
|
|
|||
Loading…
Reference in a new issue