Refactor / justify codebase-wide 'as unknown as' casts (#1690) (#1756)

* core: refactor serverList.ts object↔Record casts into typed helper (#1690)

Remove the four pre-existing `as unknown as Record<string, unknown>` /
`as unknown as MCPServerConfig` casts in core/mcp/serverList.ts:

- stripInspectorFields: rewrite as clone + `delete` over INSPECTOR_FIELD_KEYS.
  Every Inspector-extension key is optional on StoredMCPServer, so the result
  is still (a subtype of) MCPServerConfig with no cast.
- oauth-strip in extractSecretsFromStored: `oauth` is optional, so a plain
  `delete stripped.oauth` is legal — drop the cast.
- mcpConfigToServerEntries: route the one genuinely-needed widening through a
  single documented `toRecord(value: object)` helper. Taking `object` makes the
  single `as Record<string, unknown>` legal (no TS2352), so the double cast
  disappears from the call site.

Net: zero `as unknown as` in serverList.ts; the one remaining structural cast
lives inside the audited toRecord helper. Behavior-neutral — existing coverage
(79 serverList tests) unchanged; full `npm run ci` passes.

Closes #1690

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* core+web+test-servers: eliminate/justify remaining `as unknown as` casts (#1690)

Codebase-wide follow-on to the serverList refactor: audit every `as unknown
as` double cast outside tests and either remove it or ensure it carries a
justifying comment (the review bar is "no *unjustified* double cast").

Removed (widened via a shared helper or narrowed to a single cast):
- Promote the serverList `toRecord(value: object)` widening helper to
  core/json/jsonUtils.ts and reuse it at core/mcp/node/config.ts (normalize
  loop), core/mcp/remote/node/server.ts (pino level dispatch), and
  clients/web/src/App.tsx (window global lookup).
- inspectorClient.rawWireRequest: type the frame as `JSONRPCRequest` and
  narrow only `params` with a single structural cast (the SDK's `_meta`-typed
  params is what blocked direct assignment).
- toolOutputValidation: SDK `outputSchema` → `JsonSchemaType` is a single cast.
- test-server-fixtures: the optional `task` handle is reachable with a single
  cast (target field is optional).

Justified in place (genuinely unavoidable — private SDK internals, partial
interface stubs, third-party/nominal bridges): the inspectorClient private-map
accesses, resourceContext + tokenAuthProvider partial `OAuthClientProvider`
stubs, the ext-apps peer bridge, modern/legacy `_requestHandlers` bypasses,
and the happy-dom `matchMedia` test mock.

Behavior-neutral. Full `npm run ci` passes (validate, ≥90 coverage gate,
smoke, Storybook).

Refs #1690

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* address @claude review nits: consistent cast justifications + drop reflow noise (#1690)

- test-servers/src/modern-tasks.ts: add an inline justification above the
  `_requestHandlers` double cast, mirroring the sibling in
  composable-test-server.ts (was leaning on the function JSDoc only).
- Annotate the partial-`AppBridge` mock double casts in the story fixtures
  (AppsScreen ×2, AppRenderer, InspectorView) so no non-test-file double cast
  is unjustified.
- inspectorClient.ts: revert unrelated Prettier reflow hunks (protocolVersion,
  task-content array, timeout-reject wrap, reconnect guard) so the diff is the
  single rawWireRequest cast change only.

Behavior-neutral (comment/format-only). `npm run ci` green apart from a known
flaky redaction test whose assertion collided with a random request id.

Refs #1690

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* actually revert the reflow noise this time (#1690)

The reflow reverts in cc187ea were silently undone by a `prettier --write` I
ran on the same file right after, so the four Prettier-only hunks from 6a3afa4a
were still in the diff (caught in re-review). Revert them for real now — and
without re-running Prettier on the file:

- core/mcp/inspectorClient.ts: restore the protocolVersion line, task-content
  array, timeout-reject call, and reconnect guard to their origin/v2/main form,
  so the file's diff is only the single rawWireRequest cast change.
- test-servers/src/modern-tasks.ts: restore the ModernTaskStatus union to its
  one-line form (the leading-`|` reflow was also Prettier churn), leaving only
  the new `_requestHandlers` justification comment.

Formatting-only; `npm run validate` green.

Refs #1690

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall 2026-07-24 09:34:32 -04:00 • committed by GitHub
commit 99ef731746
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 85 additions and 33 deletions

View file

@ -1892,12 +1892,16 @@ export class InspectorClient extends InspectorClientEventTarget {
throw new Error("Client is not connected");
}
const id = `inspector-ext-${(this.rawWireRequestCounter += 1)}`;
const message = {
jsonrpc: "2.0" as const,
// `params` is an arbitrary caller-supplied record; the SDK types request
// params with a specific optional `_meta` shape it can't satisfy, so widen
// it with a single structural cast. Typing `message` as `JSONRPCRequest`
// (a `JSONRPCMessage` member) then needs no further cast.
const message: JSONRPCRequest = {
jsonrpc: "2.0",
id,
method,
params,
} as unknown as JSONRPCMessage;
params: params as JSONRPCRequest["params"],
};
const timeoutMs = this.requestTimeout ?? 30_000;
const raw = await new Promise<unknown>((resolve, reject) => {
const timer = setTimeout(() => {

View file

@ -9,6 +9,7 @@ import type {
StreamableHttpServerConfig,
} from "../types.js";
import { normalizeServerType } from "../serverList.js";
import { toRecord } from "../../json/jsonUtils.js";
/**
* Options object passed to resolveServerConfigs by runners (parsed from argv).
@ -140,9 +141,7 @@ function loadMcpServersConfig(
const normalizedServers: Record<string, MCPServerConfig> = {};
for (const [name, raw] of Object.entries(config.mcpServers)) {
normalizedServers[name] = normalizeServerType(
raw as unknown as Record<string, unknown> & { type?: string },
);
normalizedServers[name] = normalizeServerType(toRecord(raw));
}
return { ...config, mcpServers: normalizedServers };
} catch (error) {

View file

@ -58,6 +58,7 @@ import {
storedFieldsToInspectorSettings,
stripInspectorFields,
} from "../../serverList.js";
import { toRecord } from "../../../json/jsonUtils.js";
import { resolveImportSource } from "../../import/resolveSource.js";
import { RemoteSession } from "./remote-session.js";
import { createRemoteAuthProvider } from "./tokenAuthProvider.js";
@ -298,7 +299,7 @@ function forwardLogEvent(
logEvent: Partial<LogEvent>,
): void {
const levelLabel = (logEvent?.level?.label ?? "info").toLowerCase();
const method = (logger as unknown as Record<string, unknown>)[levelLabel];
const method = toRecord(logger)[levelLabel];
if (typeof method !== "function") return;
const bindings = Object.assign(

View file

@ -93,6 +93,12 @@ export function createRemoteAuthProvider(
state() {
return "";
},
// This provider implements only the members the remote backend transport
// exercises (token get/save, client info, non-interactive redirect); the
// interactive-only members the SDK never calls on the server side are stubbed
// with narrower shapes (e.g. `codeVerifier()` returns `undefined`, not the
// interface's `string | Promise<string>`), so it can't be assigned to the
// full `OAuthClientProvider` without the double cast.
} as unknown as OAuthClientProvider;
return {

View file

@ -27,6 +27,7 @@ import {
SECRET_FIELD_OAUTH_CLIENT_SECRET,
envSecretField,
} from "../auth/secret-fields.js";
import { toRecord } from "../json/jsonUtils.js";
// The full set of valid `type` discriminator values, used to reject anything
// else read off disk so unknown strings can't propagate to narrowing sites.
@ -427,16 +428,17 @@ export const INSPECTOR_FIELD_KEYS = new Set(
* new extension field doesn't silently leak through this slice — the
* `satisfies` constraint above forces the map update, which propagates
* here.
*
* Every Inspector-extension key is optional on `StoredMCPServer`, so deleting
* them off a clone leaves a value still typed as (a subtype of)
* `MCPServerConfig` — no cast needed.
*/
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>,
)) {
if (INSPECTOR_FIELD_KEYS.has(k as keyof StoredInspectorFields)) continue;
out[k] = v;
const out = { ...stored };
for (const key of INSPECTOR_FIELD_KEYS) {
delete out[key];
}
return out as unknown as MCPServerConfig;
return out;
}
/**
@ -454,16 +456,13 @@ 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>,
)) {
// Partition the entry's keys into Inspector-extension vs SDK-only. Both
// `raw` and `inspectorFields` are widened through `toRecord` (see its doc)
// so the generic key iteration/assignment needs no per-site cast.
const inspectorRecord = toRecord(inspectorFields);
for (const [k, v] of Object.entries(toRecord(raw))) {
if (INSPECTOR_FIELD_KEYS.has(k as keyof StoredInspectorFields)) {
(inspectorFields as Record<string, unknown>)[k] = v;
inspectorRecord[k] = v;
} else {
sdkOnly[k] = v;
}
@ -589,7 +588,7 @@ export function extractSecretsFromStored(
if (Object.keys(restOauth).length > 0) {
stripped.oauth = restOauth;
} else {
delete (stripped as unknown as Record<string, unknown>).oauth;
delete stripped.oauth;
}
}

View file

@ -34,9 +34,11 @@ export function validateToolOutput(
: `Tool "${tool.name}" declares an output schema but returned no structured content`;
}
try {
const validate = provider.getValidator(
tool.outputSchema as unknown as JsonSchemaType,
);
// `tool.outputSchema` is the SDK's own object-schema shape; `JsonSchemaType`
// is the validator provider's third-party JSON Schema interface. The two are
// structurally compatible but nominally unrelated, so a cast is required to
// bridge them here.
const validate = provider.getValidator(tool.outputSchema as JsonSchemaType);
const validation = validate(structured);
return validation.valid ? undefined : validation.errorMessage;
} catch {