diff --git a/.changeset/mcp-resolver-no-swallow.md b/.changeset/mcp-resolver-no-swallow.md new file mode 100644 index 0000000..8e655ab --- /dev/null +++ b/.changeset/mcp-resolver-no-swallow.md @@ -0,0 +1,7 @@ +--- +'@gemstack/mcp': minor +--- + +`resolveOrConstruct` no longer masks a genuine resolver failure with an un-wired instance. + +`McpResolver` gains an optional `has?(token)` hook. When a resolver implements it (the built-in `createResolver` now does), the runtime only routes tokens it owns through `resolve()` when constructing a primitive class, and lets a genuine construction failure (e.g. a missing constructor dependency) propagate loudly instead of silently falling back to `new Token()`. Resolvers without `has` keep the previous behavior — a `resolve` miss (throw or `undefined`) falls back to a plain constructor — so this is backward compatible. diff --git a/packages/mcp/src/resolver.ts b/packages/mcp/src/resolver.ts index 2fd17f4..7c7875f 100644 --- a/packages/mcp/src/resolver.ts +++ b/packages/mcp/src/resolver.ts @@ -19,6 +19,15 @@ export interface McpResolver { * runtime — a resolver must never silently inject `undefined`. */ resolve(token: unknown): unknown + /** + * Optional: report whether this resolver owns a binding for `token`. When + * present, the runtime only routes owned tokens through {@link resolve} when + * constructing a primitive class, and lets a genuine construction failure + * propagate instead of silently falling back to `new Token()`. A resolver + * without this hook keeps the legacy behavior (a `resolve` miss — throw or + * `undefined` — falls back to a plain constructor). + */ + has?(token: unknown): boolean } /** A {@link McpResolver} with imperative registration, returned by {@link createResolver}. */ @@ -55,6 +64,11 @@ export function createResolver(): MutableResolver { `Register it with createResolver().register(token, instance).`, ) }, + // A token is "owned" if it is registered or is a class this resolver can + // construct; string/symbol tokens with no registration are not. + has(token) { + return registry.has(token) || typeof token === 'function' + }, } return resolver } diff --git a/packages/mcp/src/runtime/handle-deps.test.ts b/packages/mcp/src/runtime/handle-deps.test.ts new file mode 100644 index 0000000..a225aea --- /dev/null +++ b/packages/mcp/src/runtime/handle-deps.test.ts @@ -0,0 +1,56 @@ +import { test } from 'node:test' +import assert from 'node:assert/strict' +import { resolveOrConstruct } from './handle-deps.js' +import { createResolver, type McpResolver } from '../resolver.js' + +class Widget { + readonly kind = 'widget' +} + +test('resolveOrConstruct plain-constructs with no resolver', () => { + assert.ok(resolveOrConstruct(Widget) instanceof Widget) +}) + +test('the built-in resolver constructs an unregistered class', () => { + const out = resolveOrConstruct(Widget, createResolver()) + assert.ok(out instanceof Widget) +}) + +test('a has()-aware resolver lets a genuine construction failure propagate', () => { + // The token IS owned (has → true) but building it throws: this is a real DI + // misconfiguration and must NOT be masked by a plain new Ctor(). + const resolver: McpResolver = { + has: () => true, + resolve: () => { + throw new Error('missing constructor dependency') + }, + } + assert.throws(() => resolveOrConstruct(Widget, resolver), /missing constructor dependency/) +}) + +test('a has()-aware resolver skips resolve() for tokens it does not own', () => { + let resolveCalls = 0 + const resolver: McpResolver = { + has: () => false, + resolve: () => { + resolveCalls++ + return undefined + }, + } + assert.ok(resolveOrConstruct(Widget, resolver) instanceof Widget) + assert.equal(resolveCalls, 0) +}) + +test('a legacy resolver without has() still falls back on a throw', () => { + const resolver: McpResolver = { + resolve: () => { + throw new Error('unknown token') + }, + } + assert.ok(resolveOrConstruct(Widget, resolver) instanceof Widget) +}) + +test('a legacy resolver returning undefined falls back to a plain constructor', () => { + const resolver: McpResolver = { resolve: () => undefined } + assert.ok(resolveOrConstruct(Widget, resolver) instanceof Widget) +}) diff --git a/packages/mcp/src/runtime/handle-deps.ts b/packages/mcp/src/runtime/handle-deps.ts index 31ae045..b5fdd1f 100644 --- a/packages/mcp/src/runtime/handle-deps.ts +++ b/packages/mcp/src/runtime/handle-deps.ts @@ -6,12 +6,24 @@ export type Ctor = new (...args: any[]) => T /** * Construct a tool / resource / prompt class. When a {@link McpResolver} is * supplied (off the owning server), it gets first refusal so a container can - * auto-wire constructor dependencies; on a miss (resolver throws, or returns - * `undefined`) we fall back to a plain `new Ctor()` so primitives with no DI - * needs always instantiate. + * auto-wire constructor dependencies; a primitive with no DI needs still + * instantiates via a plain `new Ctor()` fallback. + * + * If the resolver implements {@link McpResolver.has}, only tokens it owns go + * through `resolve()`, and a genuine construction failure propagates loudly + * rather than being masked by an un-wired `new Ctor()`. A resolver without + * `has` keeps the legacy behavior: a `resolve` miss (throw or `undefined`) + * falls back to a plain constructor. */ export function resolveOrConstruct(Ctor: Ctor, resolver?: McpResolver): T { if (resolver) { + if (resolver.has) { + // Precise path: don't swallow a real failure building an owned token. + if (!resolver.has(Ctor)) return new Ctor() + const resolved = resolver.resolve(Ctor) + return (resolved !== undefined ? resolved : new Ctor()) as T + } + // Legacy path: no `has` hook — a resolver miss falls back. try { const resolved = resolver.resolve(Ctor) if (resolved !== undefined) return resolved as T