Skip to content

Commit e0eb7d6

Browse files
committed
fix(connector-mcp): a credentialed instance's missing pinned tool takes the credential path
Contract review on PR 28515: the pin's carve-out from the include refusal ignored whether the instance's credential reached the server, so a tool hidden from an under-scoped token was degraded as an upstream change and retried against the same credential. Under a sent credential a missing pinned tool (with or without include) now takes the credential path, both causes named; a served tool that hashes differently stays CONNECTOR_UPSTREAM_CHANGED. Also: the drift message no longer claims no tool is exposed (false while a previous connector keeps serving), and the connector-provider-errors header names the four outcomes and three markers. Claude-Session: https://claude.ai/code/session_017xRBcSf5BEfcbq4uCnjEXu Co-authored-by: Claude <noreply@anthropic.com>
1 parent fdacc67 commit e0eb7d6

5 files changed

Lines changed: 166 additions & 33 deletions

File tree

‎.changeset/27401-mcp-tools-snapshot.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Metadata-protocol: added
1010
An `mcp` instance's `include` decides which tools it exposes. Until now nothing noticed when the server rewrote one of them, for example a new description that steers an agent or an `inputSchema` that asks for more than it did. `providerConfig.toolsSnapshot` is a record of tool name → the sha256 of that tool's definition. It is keyed by tool name, like `include`, and the two compose.
1111

1212
- **The digest.** sha256, lowercase hex, of the UTF-8 canonical JSON of `{ name, description, inputSchema }` from the tool's `tools/list` entry. A member the server does not send is left out. Canonical JSON sorts object keys in UTF-16 code-unit order at every depth, keeps array order, writes primitives as `JSON.stringify` does and has no whitespace: the RFC 8785 (JCS) form for values read from JSON. `computeMcpToolsSnapshot(tools)` and `mcpToolDigest(tool)` are exported and are the functions the check uses.
13-
- **The check.** On every materialization each pinned tool is hashed and compared on its own. A pinned tool served with a different description or `inputSchema`, or no longer served, degrades the instance: the provider throws `ConnectorUpstreamChangedError` (`CONNECTOR_UPSTREAM_CHANGED`), not the upstream-unavailable marker, so the logs say the pin did not match rather than that the server is down. Its reason names each such tool and the digest the server now serves, and none of the instance's tools are exposed. The instance is re-checked on the retry backoff and recovers once the server serves the pinned definitions again. An edited pin re-materializes it at once. A pinned tool that `include` also lists is reported by the pin when the server stops serving it, not as an `include` typo.
13+
- **The check.** On every materialization each pinned tool is hashed and compared on its own. A pinned tool served with a different description or `inputSchema`, or no longer served, degrades the instance: the provider throws `ConnectorUpstreamChangedError` (`CONNECTOR_UPSTREAM_CHANGED`), not the upstream-unavailable marker, so the logs say the pin did not match rather than that the server is down. Its reason names each such tool and the digest the server now serves, and none of the instance's tools are exposed. The instance is re-checked on the retry backoff and recovers once the server serves the pinned definitions again. An edited pin re-materializes it at once. A pinned tool that `include` also lists is reported by the pin when the server stops serving it, not as an `include` typo. When the instance's own credential reached the server (its `auth` over http), a pinned tool the server does not list takes the credential path instead (`ConnectorCredentialRejectedError`, 403, both causes named, a stored credential marked `rejected`), since the server may list only the tools that credential may use; a served pinned tool that hashes differently is still `CONNECTOR_UPSTREAM_CHANGED`.
1414
- **Refused before connecting.** A `toolsSnapshot` that is not a record of tool name → 64 lowercase hex characters fails the boot, and so does a pin on a tool `include` leaves out, since that pin would never be checked.
1515
- Without `toolsSnapshot`, an instance behaves exactly as before. A tool the pin does not name is not checked.
1616
- `McpConnectorOptions.include` now also receives the tool's `tools/list` entry as a second argument. An existing one-argument predicate is unaffected.

‎content/docs/automation/connectors.mdx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,11 @@ On every materialization each pinned tool is hashed and compared on its own:
319319
change, review it and set the pin to the served digest (the edit
320320
re-materializes the instance at once). This holds for a pinned tool
321321
`include` also lists: once pinned, a tool the server stops serving is
322-
reported by the pin, not as an `include` typo.
322+
reported by the pin, not as an `include` typo. When the instance's own
323+
credential reached the server (its `auth` over http), a pinned tool the
324+
server does not list may be one hidden from that credential, so it is
325+
degraded as a credential the upstream refuses instead, naming both causes —
326+
a served tool that hashes differently is still reported by the pin.
323327
- A tool the pin does not name is not checked. Pin every tool you depend on,
324328
and keep tools the server adds out with `include`.
325329

‎packages/connectors/connector-mcp/src/mcp-provider.ts‎

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,11 @@ async function credentialRejectionStatus(err: unknown): Promise<number | undefin
333333
* {@link toolsSnapshotDriftError}). A
334334
* pinned name's absence is the pin's to report even when `include` lists it
335335
* too: the pin says the tool existed with that definition, so its absence is
336-
* the server's change rather than a typo.
336+
* the server's change rather than a typo — unless the instance's credential
337+
* reached the server, which may hide a tool from that credential: then a
338+
* missing pinned tool takes the credential path ({@link ConnectorCredentialRejectedError},
339+
* both causes named), and only a served tool that hashes differently is
340+
* {@link ConnectorUpstreamChangedError}.
337341
*/
338342
export function createMcpProviderFactory(deps: McpProviderDeps = {}): ConnectorProviderFactory {
339343
return async (ctx) => {
@@ -517,10 +521,14 @@ export function createMcpProviderFactory(deps: McpProviderDeps = {}): ConnectorP
517521
// without the tool the author listed (a typo, a tool the server renamed
518522
// or dropped, or one hidden from this credential), and the first sign
519523
// would be a flow failing at run time.
520-
// A pinned entry is left to the pin below: its absence is the server's
521-
// change, reported with the pin's other findings, not a typo.
524+
// A pinned entry is left to the pin below — its absence is the server's
525+
// change, reported with the pin's other findings, not a typo — but only
526+
// when no instance credential reached the server. When one did, a
527+
// missing pinned entry may be a tool hidden from that credential, which
528+
// the answer cannot tell apart from a dropped one, so it takes the
529+
// credential's path below like any other entry (#27401 contract review).
522530
const offered = new Set(listedTools);
523-
const pinned = new Set(snapshot ? Object.keys(snapshot) : []);
531+
const pinned = new Set(snapshot && !credentialSent ? Object.keys(snapshot) : []);
524532
const unknown = [...new Set(includeList)].filter((entry) => !offered.has(entry) && !pinned.has(entry));
525533
if (unknown.length > 0) {
526534
await bundle.close().catch(() => {});
@@ -574,6 +582,16 @@ export function createMcpProviderFactory(deps: McpProviderDeps = {}): ConnectorP
574582
const drift = compareToolsSnapshot(snapshot, selectedTools);
575583
if (drift) {
576584
await bundle.close().catch(() => {});
585+
// A pinned tool the server does not list, on an instance whose
586+
// credential reached it (a pin with no `include`; an included one was
587+
// refused above), is the credential's or the server's change, and the
588+
// answer cannot say which — so it takes the credential's path, as an
589+
// `include` entry does: degraded naming both causes, a stored
590+
// credential marked `rejected`, no retry against the same value. A
591+
// served tool that hashes differently is the server's change alone.
592+
if (credentialSent && drift.missing.length > 0) {
593+
throw pinnedToolsHiddenError(ctx.name, drift.missing, listedTools);
594+
}
577595
throw toolsSnapshotDriftError(ctx.name, drift);
578596
}
579597
}
@@ -616,12 +634,41 @@ function toolsSnapshotDriftError(connectorName: string, drift: ToolsSnapshotDrif
616634
const more = findings.length > named.length ? `; … ${findings.length - named.length} more` : '';
617635
return new ConnectorUpstreamChangedError(
618636
`connector-mcp provider: connector '${connectorName}' providerConfig.toolsSnapshot does not match the tools ` +
619-
`its MCP server serves: ${named.join('; ')}${more}. None of the instance's tools are exposed while a pinned ` +
620-
`tool differs. If the change is expected, review it and set each pin to the digest the server now serves; ` +
637+
`its MCP server serves: ${named.join('; ')}${more}. This materialization exposes none of the server's ` +
638+
`tools. If the change is expected, review it and set each pin to the digest the server now serves; ` +
621639
`otherwise the instance comes back once the server serves the pinned definitions again.`,
622640
);
623641
}
624642

643+
/**
644+
* The fault for pinned tools the server does not list when the instance's
645+
* credential reached it: {@link ConnectorCredentialRejectedError} (403), the
646+
* same path and reasoning as an `include` entry the server does not list for
647+
* that credential — a server may list only the tools a credential may use, so
648+
* the message names both causes.
649+
*/
650+
function pinnedToolsHiddenError(
651+
connectorName: string,
652+
missing: readonly string[],
653+
listedTools: readonly string[],
654+
): ConnectorCredentialRejectedError {
655+
const named = missing.slice(0, DRIFT_NAMED_MAX).map((name) => `'${name}'`).join(', ');
656+
const moreMissing = missing.length > DRIFT_NAMED_MAX ? `, … ${missing.length - DRIFT_NAMED_MAX} more` : '';
657+
const offered = [...new Set(listedTools)];
658+
const sample = offered.slice(0, 20);
659+
const more = offered.length > sample.length ? `, … ${offered.length - sample.length} more` : '';
660+
return new ConnectorCredentialRejectedError(
661+
`connector-mcp provider: connector '${connectorName}' providerConfig.toolsSnapshot pins ${named}${moreMissing}, ` +
662+
`which the MCP server does not list for the credential this instance sent. A server may list only the tools ` +
663+
`a credential is allowed to use, and its tools/list answer cannot tell the two causes apart: either the ` +
664+
`credential lacks the scopes or permissions these tools need — fix the credential, then set it again — or ` +
665+
`the server no longer serves these tools — review the change and update providerConfig.toolsSnapshot. ` +
666+
`For this credential, the server lists ${offered.length} tool(s): ` +
667+
`${sample.map((name) => `'${name}'`).join(', ')}${more}.`,
668+
403,
669+
);
670+
}
671+
625672
/** Whether {@link applyAuthToHeaders} puts the instance's credential on the http transport. */
626673
function sendsInstanceAuth(auth: ResolvedConnectorAuth | undefined): boolean {
627674
if (!auth || auth.type === 'none') return false;

‎packages/connectors/connector-mcp/src/tool-snapshot.test.ts‎

Lines changed: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,23 @@ function server(tools: McpToolDescriptor[]) {
4545
return { clientFactory, state };
4646
}
4747

48-
function ctx(providerConfig: Record<string, unknown>): ConnectorProviderContext {
49-
return { name: 'vendor', label: 'Vendor', type: 'api', providerConfig };
48+
function ctx(providerConfig: Record<string, unknown>, auth?: ConnectorProviderContext['auth']): ConnectorProviderContext {
49+
return { name: 'vendor', label: 'Vendor', type: 'api', providerConfig, ...(auth ? { auth } : {}) };
5050
}
5151

52-
/** Materialize against `tools`; resolves to the materialization or the thrown value. */
53-
async function materialize(tools: McpToolDescriptor[], providerConfig: Record<string, unknown>) {
52+
/**
53+
* Materialize against `tools`; resolves to the materialization or the thrown
54+
* value. `auth` is the instance's resolved credential: over http a bearer one
55+
* reaches the server, which may then list only the tools it may use.
56+
*/
57+
async function materialize(
58+
tools: McpToolDescriptor[],
59+
providerConfig: Record<string, unknown>,
60+
auth?: ConnectorProviderContext['auth'],
61+
) {
5462
const { clientFactory, state } = server(tools);
5563
const factory = createMcpProviderFactory({ clientFactory });
56-
const outcome = await Promise.resolve(factory(ctx({ transport: http, ...providerConfig }))).then(
64+
const outcome = await Promise.resolve(factory(ctx({ transport: http, ...providerConfig }, auth))).then(
5765
(mat: ConnectorMaterialization) => ({ mat, err: undefined as unknown }),
5866
(err: unknown) => ({ mat: undefined, err }),
5967
);
@@ -209,6 +217,69 @@ describe('mcp provider: providerConfig.toolsSnapshot pins each tool', () => {
209217
});
210218
});
211219

220+
// A credentialed instance (a bearer `auth` over http reaches the server): a
221+
// server may list only the tools that credential may use, so a pinned tool it
222+
// does not list may be hidden from the credential rather than dropped — it
223+
// takes the credential path, both causes named, like an `include` entry. A
224+
// served tool that hashes differently is still the server's change.
225+
describe('mcp provider: toolsSnapshot on an instance whose credential reaches the server', () => {
226+
const PIN = computeMcpToolsSnapshot([CREATE_ISSUE, LIST_ISSUES]);
227+
const bearer = { type: 'bearer', token: 'narrow-token-value' } as unknown as ConnectorProviderContext['auth'];
228+
229+
function expectCredentialPath(err: unknown) {
230+
expect(isConnectorCredentialRejected(err)).toBe(true);
231+
expect((err as { status?: number }).status).toBe(403);
232+
expect(isConnectorUpstreamChanged(err)).toBe(false);
233+
expect(isConnectorUpstreamUnavailable(err)).toBe(false);
234+
}
235+
236+
it('an included and pinned tool the server does not list takes the credential path, naming both causes', async () => {
237+
const { err, state } = await materialize(
238+
[LIST_ISSUES, DELETE_REPO],
239+
{ include: ['create_issue', 'list_issues'], toolsSnapshot: PIN },
240+
bearer,
241+
);
242+
expectCredentialPath(err);
243+
const message = (err as Error).message;
244+
expect(message).toContain("providerConfig.include names 'create_issue', which the MCP server does not list for the credential this instance sent");
245+
expect(message).toContain('the credential lacks the scopes or permissions these tools need');
246+
expect(message).not.toContain('narrow-token-value');
247+
expect(state.closed).toBe(true);
248+
});
249+
250+
it('a pinned tool the server does not list, with no include, takes the credential path too', async () => {
251+
const { err, state } = await materialize([LIST_ISSUES, DELETE_REPO], { toolsSnapshot: PIN }, bearer);
252+
expectCredentialPath(err);
253+
const message = (err as Error).message;
254+
expect(message).toMatch(
255+
/^connector-mcp provider: connector 'vendor' providerConfig\.toolsSnapshot pins 'create_issue', which the MCP server does not list for the credential this instance sent\./,
256+
);
257+
expect(message).toContain('the credential lacks the scopes or permissions these tools need — fix the credential');
258+
expect(message).toContain('the server no longer serves these tools — review the change and update providerConfig.toolsSnapshot');
259+
expect(message).toContain("For this credential, the server lists 2 tool(s): 'list_issues', 'delete_repo'.");
260+
expect(message).not.toContain('narrow-token-value');
261+
expect(state.closed).toBe(true);
262+
});
263+
264+
it('a served pinned tool that hashes differently is still CONNECTOR_UPSTREAM_CHANGED', async () => {
265+
const rewritten = { ...CREATE_ISSUE, description: 'changed' };
266+
const { err } = await materialize(
267+
[rewritten, LIST_ISSUES],
268+
{ include: ['create_issue', 'list_issues'], toolsSnapshot: PIN },
269+
bearer,
270+
);
271+
expectUpstreamChanged(err);
272+
expect((err as Error).message).toContain("tool 'create_issue' changed");
273+
});
274+
275+
it('matching pins materialize, and the credential counts as verified', async () => {
276+
const { mat, err } = await materialize(SERVED, { include: ['create_issue', 'list_issues'], toolsSnapshot: PIN }, bearer);
277+
expect(err).toBeUndefined();
278+
expect(Object.keys(mat?.handlers ?? {})).toEqual(['create_issue', 'list_issues']);
279+
expect(mat?.credentialVerified).toBe(true);
280+
});
281+
});
282+
212283
describe('mcp provider: a malformed toolsSnapshot is refused before connecting', () => {
213284
const digest = mcpToolDigest(CREATE_ISSUE);
214285

‎packages/spec/src/integration/connector-provider-errors.ts‎

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,41 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
/**
4-
* Upstream-availability classification for connector provider factories
5-
* (ADR-0097 follow-up, #3017).
4+
* Failure classification for connector provider factories (ADR-0097
5+
* follow-ups: #3017, #27237, #27401).
66
*
7-
* A {@link ConnectorProviderFactory} can fail for two very different reasons:
7+
* A {@link ConnectorProviderFactory} can fail in four ways, and the
8+
* materializer treats each differently:
89
*
9-
* 1. **Configuration faults** — invalid `providerConfig`, an unresolvable
10-
* `credentialRef`, a name conflict. These are authoring mistakes; the
11-
* materializer keeps them **fatal at boot** (ADR-0097 "fail loudly").
12-
* 2. **Operational faults** — the upstream the factory must contact to
13-
* materialize (an MCP server's `tools/list`, a remote spec endpoint) is
14-
* *temporarily unreachable*. Aborting the whole app boot for a transient
15-
* network blip turns one degraded integration into a total outage.
10+
* 1. **Configuration faults** — invalid `providerConfig`, a name conflict, a
11+
* shape the provider refuses. These are authoring mistakes, thrown with no
12+
* marker; the materializer keeps them **fatal at boot** (ADR-0097 "fail
13+
* loudly"), a skipped entry on reload.
14+
* 2. **The upstream is unreachable** — the upstream the factory must contact
15+
* to materialize (an MCP server's `tools/list`, a remote spec endpoint) is
16+
* *temporarily unreachable*: {@link CONNECTOR_UPSTREAM_UNAVAILABLE}, most
17+
* conveniently a {@link ConnectorUpstreamUnavailableError}.
18+
* 3. **The upstream refused the credential** — it answered, and refused the
19+
* credential the instance sent (HTTP 401 / 403, or a tool list narrowed to
20+
* what that credential may use): {@link CONNECTOR_CREDENTIAL_REJECTED},
21+
* a {@link ConnectorCredentialRejectedError}.
22+
* 4. **The upstream changed** — it answered, and serves a definition that
23+
* differs from a pin the instance declares:
24+
* {@link CONNECTOR_UPSTREAM_CHANGED}, a {@link ConnectorUpstreamChangedError}.
1625
*
17-
* A factory signals case 2 by throwing an error whose `code` is
18-
* {@link CONNECTOR_UPSTREAM_UNAVAILABLE} — most conveniently a
19-
* {@link ConnectorUpstreamUnavailableError}. The materializer then **degrades**
20-
* that one instance instead of failing boot: it is registered in a `degraded`
21-
* state (visible via `GET /connectors`, dispatch fails with a clear error) and
22-
* re-materialization is retried with backoff and on every reconcile. Errors
23-
* without the marker keep the fail-loud contract unchanged.
26+
* Each of the three markers makes the materializer **degrade** that one
27+
* instance instead of failing boot: it is registered in a `degraded` state
28+
* (visible via `GET /connectors`, dispatch fails with the factory's message)
29+
* with log records worded for its cause. An unreachable or changed upstream is
30+
* re-materialized with backoff and on every reconcile; a refused credential is
31+
* retried on that backoff only when its value can change where the process
32+
* cannot see it (the marker's own TSDoc says when). Errors without a marker
33+
* keep the fail-loud contract unchanged.
2434
*
25-
* The check is **structural** ({@link isConnectorUpstreamUnavailable} reads
26-
* `code`, never `instanceof`), so classification survives package-manager
27-
* module duplication across the plugin/host boundary.
35+
* The checks are **structural** ({@link isConnectorUpstreamUnavailable},
36+
* {@link isConnectorCredentialRejected} and {@link isConnectorUpstreamChanged}
37+
* read `code`, never `instanceof`), so classification survives
38+
* package-manager module duplication across the plugin/host boundary.
2839
*/
2940

3041
/** Marker `code` for an operational (retryable) provider-factory failure. */

0 commit comments

Comments
 (0)