Merge remote-tracking branch 'origin/v2/main' into v2/fix/2071-oauth-resource-metadata

Signed-off-by: cliffhall <cliff@futurescale.com>

# Conflicts:
#	core/mcp/node/transport.ts
This commit is contained in:
cliffhall 2026-08-24 21:26:59 -04:00
commit baf4fe954f
30 changed files with 9048 additions and 1271 deletions

View file

@ -0,0 +1,58 @@
import { act } from "@testing-library/react";
import type { Ace } from "ace-builds";
/**
* Reach the live Ace editor a component mounted, and drive it.
*
* Typing is deliberately **not** simulated. Ace reads keystrokes through a 1px
* offscreen textarea plus selection and composition state that happy-dom does
* not implement, so `userEvent.type` reaches the element and produces no edit
* at all — a test written that way asserts nothing while looking like it
* asserts a lot. Real keyboard behaviour (auto-closing a brace on Enter, and
* the draft surviving invalid text) is covered by the `JsonObjectInput`
* Storybook play functions, which run in Chromium.
*
* Ace hangs the instance off its container element as `env.editor`; that is the
* supported way back to it from the DOM.
*/
export function getAceEditor(container: ParentNode = document): Ace.Editor {
const node = container.querySelector(".ace_editor");
const editor = (
node as (HTMLElement & { env?: { editor?: Ace.Editor } }) | null
)?.env?.editor;
if (!editor) throw new Error("Ace editor not mounted");
return editor;
}
/** The document the editor currently holds. */
export function getAceText(container?: ParentNode): string {
return getAceEditor(container).getValue();
}
/**
* Replace the document the way a select-all-and-retype would, and let React
* flush.
*
* Async because `JsonObjectInput` coalesces the paired remove/insert events
* Ace fires for a replace; an `await act` is what drains that microtask.
*/
export async function setAceText(
text: string,
container?: ParentNode,
): Promise<void> {
await act(async () => {
// `1` parks the cursor at the end, matching where typing would leave it.
getAceEditor(container).setValue(text, 1);
});
}
/**
* Matcher for an Ace-labelled control.
*
* Ace composes its text input's `aria-label` as `"<label>, Cursor at row N"`,
* so an exact-string `getByLabelText` never matches — the position readout is
* Ace's, only the prefix is ours.
*/
export function aceLabel(label: string): RegExp {
return new RegExp(label.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"));
}

View file

@ -2,6 +2,8 @@ import { describe, it, expect, vi } from "vitest";
import {
buildOAuthConnectionState,
hasPersistedOAuthServerState,
isAccessTokenProvablyUnexpired,
isAccessTokenUsable,
isServerOAuthConfigured,
protocolFromOAuthConfig,
} from "@inspector/core/auth/connection-state.js";
@ -257,3 +259,102 @@ describe("buildOAuthConnectionState", () => {
expect(state.ema).not.toHaveProperty("idToken");
});
});
/** Minimal unsigned JWT carrying only `exp` (epoch seconds). */
function jwtWithExp(expSec: number): string {
const payload = btoa(JSON.stringify({ exp: expSec }))
.replace(/\+/g, "-")
.replace(/\//g, "_")
.replace(/=+$/, "");
return `header.${payload}.sig`;
}
const nowSec = () => Math.floor(Date.now() / 1000);
describe("isAccessTokenProvablyUnexpired", () => {
it("is true for a JWT whose exp is in the future", () => {
expect(
isAccessTokenProvablyUnexpired({
access_token: jwtWithExp(nowSec() + 3600),
token_type: "Bearer",
}),
).toBe(true);
});
it("is false for a JWT whose exp has passed", () => {
expect(
isAccessTokenProvablyUnexpired({
access_token: jwtWithExp(nowSec() - 60),
token_type: "Bearer",
}),
).toBe(false);
});
it("is false inside the 60s expiry skew window", () => {
expect(
isAccessTokenProvablyUnexpired({
access_token: jwtWithExp(nowSec() + 30),
token_type: "Bearer",
}),
).toBe(false);
});
it("is false for an opaque token, where isAccessTokenUsable is true", () => {
// The #2051 regression: no local expiry evidence must not read as "valid".
const tokens = { access_token: "opaque-tok", token_type: "Bearer" };
expect(isAccessTokenProvablyUnexpired(tokens)).toBe(false);
expect(isAccessTokenUsable(tokens)).toBe(true);
});
it("is false for a JWT carrying no exp claim", () => {
const payload = btoa(JSON.stringify({ sub: "user" }))
.replace(/\+/g, "-")
.replace(/\//g, "_")
.replace(/=+$/, "");
expect(
isAccessTokenProvablyUnexpired({
access_token: `header.${payload}.sig`,
token_type: "Bearer",
}),
).toBe(false);
});
it("is false for a two-segment token whose payload decodes to an exp", () => {
// jwtExpiresAtMs only requires two dot-separated segments, so a structured
// opaque token carrying a decodable `{"exp":...}` would otherwise read as
// proof of validity. isJwtFormat is what rejects it.
const payload = btoa(JSON.stringify({ exp: nowSec() + 3600 }))
.replace(/\+/g, "-")
.replace(/\//g, "_")
.replace(/=+$/, "");
expect(
isAccessTokenProvablyUnexpired({
access_token: `opaque.${payload}`,
token_type: "Bearer",
}),
).toBe(false);
});
it("is false for a four-segment token carrying a decodable exp", () => {
const payload = btoa(JSON.stringify({ exp: nowSec() + 3600 }))
.replace(/\+/g, "-")
.replace(/\//g, "_")
.replace(/=+$/, "");
expect(
isAccessTokenProvablyUnexpired({
access_token: `header.${payload}.sig.extra`,
token_type: "Bearer",
}),
).toBe(false);
});
it("is false when there is no access token", () => {
expect(isAccessTokenProvablyUnexpired(undefined)).toBe(false);
expect(
isAccessTokenProvablyUnexpired({
access_token: "",
token_type: "Bearer",
}),
).toBe(false);
});
});

View file

@ -76,7 +76,7 @@ describe("runner client auth options", () => {
taskTtl: 60000,
maxFetchRequests: 10,
autoRefreshOnListChanged: false,
metadata: [],
metadata: {},
headers: [],
env: [],
roots: [],
@ -99,7 +99,7 @@ describe("runner client auth options", () => {
taskTtl: 60000,
maxFetchRequests: 10,
autoRefreshOnListChanged: false,
metadata: [],
metadata: {},
headers: [],
env: [],
roots: [],
@ -116,7 +116,7 @@ describe("runner client auth options", () => {
taskTtl: 60000,
maxFetchRequests: 10,
autoRefreshOnListChanged: false,
metadata: [],
metadata: {},
headers: [],
env: [],
roots: [],
@ -135,7 +135,7 @@ describe("runner client auth options", () => {
taskTtl: 60000,
maxFetchRequests: 10,
autoRefreshOnListChanged: false,
metadata: [],
metadata: {},
headers: [],
env: [],
roots: [],
@ -156,7 +156,7 @@ describe("runner client auth options", () => {
taskTtl: 60000,
maxFetchRequests: 10,
autoRefreshOnListChanged: false,
metadata: [],
metadata: {},
headers: [],
env: [],
roots: [],
@ -187,7 +187,7 @@ describe("runner client auth options", () => {
taskTtl: 60000,
maxFetchRequests: 10,
autoRefreshOnListChanged: false,
metadata: [],
metadata: {},
headers: [],
env: [],
roots: [],

View file

@ -0,0 +1,47 @@
import { describe, it, expect } from "vitest";
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
describe("isSerializableJson", () => {
it.each([
["a string", "a"],
["a finite number", 1.5],
["zero", 0],
["a negative number", -42],
["a boolean", true],
["null", null],
["a nested object", { a: { b: [1, 2, { c: "d" }] } }],
["an empty array", []],
])("accepts %s", (_label, value) => {
expect(isSerializableJson(value)).toBe(true);
});
it.each([
["NaN", NaN],
["Infinity", Infinity],
["-Infinity", -Infinity],
])("rejects %s, which JSON.stringify rewrites to null", (_label, value) => {
expect(isSerializableJson(value)).toBe(false);
// The reason it matters, asserted rather than asserted-about.
expect(JSON.stringify({ v: value })).toBe('{"v":null}');
});
it.each([
["undefined", undefined],
["a function", () => {}],
["a symbol", Symbol("s")],
["a bigint", 10n],
])("rejects %s, which is not JSON at all", (_label, value) => {
expect(isSerializableJson(value)).toBe(false);
});
it("descends into arrays and objects", () => {
expect(isSerializableJson({ a: [1, { b: Infinity }] })).toBe(false);
expect(isSerializableJson([[[NaN]]])).toBe(false);
expect(isSerializableJson({ a: [1, { b: 2 }] })).toBe(true);
});
it("rejects what `JSON.parse` produces for an overflowing literal", () => {
// The reachable path: valid JSON text that parses to an unsendable value.
expect(isSerializableJson(JSON.parse('{"n":1e400}'))).toBe(false);
});
});

View file

@ -854,7 +854,7 @@ describe("InspectorClient peer-handler timing (#1797)", () => {
serverSettings: {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 0,

View file

@ -30,7 +30,7 @@ describe("headersToServerSettings", () => {
expect(settings?.headers).toEqual([
{ key: "Authorization", value: "Bearer t" },
]);
expect(settings?.metadata).toEqual([]);
expect(settings?.metadata).toEqual({});
expect(settings?.roots).toEqual([]);
});
});

View file

@ -27,6 +27,15 @@ const mockedMcpAuth = vi.mocked(mcpAuth);
const SERVER_URL = "https://example.com/mcp";
/** Minimal unsigned JWT carrying only `exp` (epoch seconds). */
function jwtWithExp(expSec: number): string {
const payload = btoa(JSON.stringify({ exp: expSec }))
.replace(/\+/g, "-")
.replace(/\//g, "_")
.replace(/=+$/, "");
return `header.${payload}.sig`;
}
function createMockParams(
overrides?: Partial<OAuthManagerParams>,
): OAuthManagerParams {
@ -988,10 +997,10 @@ describe("OAuthManager", () => {
).toBe(false);
});
it("returns true for token_expired when a usable access token exists", async () => {
it("returns true for token_expired when a provably unexpired JWT exists", async () => {
const params = createMockParams();
storageOf(params).getTokens.mockResolvedValue({
access_token: "tok",
access_token: jwtWithExp(Math.floor(Date.now() / 1000) + 3600),
token_type: "Bearer",
});
const manager = new OAuthManager(params);
@ -1001,6 +1010,36 @@ describe("OAuthManager", () => {
).toBe(true);
});
it("returns false for token_expired when the stored JWT has expired", async () => {
const params = createMockParams();
storageOf(params).getTokens.mockResolvedValue({
access_token: jwtWithExp(Math.floor(Date.now() / 1000) - 60),
token_type: "Bearer",
});
const manager = new OAuthManager(params);
expect(
await manager.checkAuthChallengeSatisfied({ reason: "token_expired" }),
).toBe(false);
});
it("returns false for token_expired when the stored token is opaque", async () => {
// Regression guard for #2051: an opaque token has no local expiry
// evidence, so it must not outvote the resource server's verdict and
// short-circuit re-authorization into replaying a dead credential.
const params = createMockParams();
storageOf(params).getTokens.mockResolvedValue({
access_token: "opaque-tok",
token_type: "Bearer",
expires_in: 900,
});
const manager = new OAuthManager(params);
expect(
await manager.checkAuthChallengeSatisfied({ reason: "token_expired" }),
).toBe(false);
});
it("returns false for invalid_token even when a locally valid token exists", async () => {
const params = createMockParams();
storageOf(params).getTokens.mockResolvedValue({

View file

@ -361,7 +361,7 @@ describe("RemoteClientTransport", () => {
const settings = {
headers: [{ key: "X-Tenant", value: "acme" }],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 0,

View file

@ -11,6 +11,7 @@ import {
mcpConfigToServerEntries,
mergeSecretsIntoStored,
normalizeServerType,
normalizeStoredMetadata,
oauthAuthorizationParamsFromSettings,
oauthEndpointOverridesFromSettings,
serverEntriesToMcpConfig,
@ -240,7 +241,7 @@ describe("serverEntriesToMcpConfig", () => {
});
it("round-trips a populated set of Inspector-extension fields (post-#1358 flat shape)", () => {
// Disk shape: top-level `headers` (Record), `metadata` (pair-array),
// Disk shape: top-level `headers` (Record), `metadata` (JSON object),
// numeric timeouts, nested `oauth`. Round-trip must preserve the on-
// disk shape byte-equivalent so a hand-edited file is stable.
const original: MCPConfig = {
@ -249,7 +250,7 @@ describe("serverEntriesToMcpConfig", () => {
type: "streamable-http",
url: "https://x.test/mcp",
headers: { Authorization: "Bearer xyz" },
metadata: [{ key: "tenant", value: "acme" }],
metadata: { tenant: "acme", limits: { rps: 10 } },
connectionTimeout: 30000,
requestTimeout: 60000,
oauth: {
@ -581,7 +582,7 @@ describe("serverEntriesToMcpConfig", () => {
headers: [{ key: "X-Tenant", value: "acme" }],
// Non-stdio server → empty env mirror in memory (for the form)
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
// Absent taskTtl on disk → product default in memory (for the form)
@ -663,7 +664,7 @@ describe("serverEntriesToMcpConfig", () => {
{ key: " ", value: "whitespace" },
],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 0,
@ -690,7 +691,7 @@ describe("serverEntriesToMcpConfig", () => {
settings: {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 0,
@ -719,7 +720,7 @@ describe("serverEntriesToMcpConfig", () => {
settings: {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 0,
@ -758,7 +759,7 @@ describe("serverEntriesToMcpConfig", () => {
settings: {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 0,
@ -785,7 +786,7 @@ describe("serverEntriesToMcpConfig", () => {
settings: {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 0,
@ -1163,7 +1164,7 @@ describe("enterpriseManaged oauth settings", () => {
const stored = inspectorSettingsToStoredFields({
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,
@ -1180,7 +1181,7 @@ describe("enterpriseManaged oauth settings", () => {
const stored = inspectorSettingsToStoredFields({
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,
@ -1196,7 +1197,7 @@ describe("oauthOnInsufficientScope (SEP-2350)", () => {
const baseSettings = {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,
@ -1374,6 +1375,185 @@ describe("oauthOnInsufficientScope (SEP-2350)", () => {
});
});
describe("normalizeStoredMetadata (#1910)", () => {
let warn: ReturnType<typeof vi.spyOn>;
beforeEach(() => {
warn = vi.spyOn(console, "warn").mockImplementation(() => {});
});
afterEach(() => {
warn.mockRestore();
});
it("passes an object through, values of any JSON type intact", () => {
const meta = {
s: "a",
n: 1,
b: false,
z: null,
arr: [1, { k: 2 }],
o: { deep: true },
};
expect(normalizeStoredMetadata(meta)).toEqual(meta);
});
it("copies rather than aliasing the stored object", () => {
// The settings object is edited in place by the form; sharing the
// reference with the parsed catalog would mutate it behind the caller.
const meta = { a: 1 };
const out = normalizeStoredMetadata(meta);
out.b = 2;
expect(meta).toEqual({ a: 1 });
});
it("reads the pre-#1910 pair array so an existing mcp.json keeps working", () => {
expect(
normalizeStoredMetadata([
{ key: "tenant", value: "acme" },
{ key: "n", value: 3 },
]),
).toEqual({ tenant: "acme", n: 3 });
});
it("skips a blank-key legacy pair", () => {
expect(
normalizeStoredMetadata([
{ key: "", value: "orphan" },
{ key: " ", value: "orphan" },
{ key: "kept", value: "yes" },
]),
).toEqual({ kept: "yes" });
});
it("preserves an explicit null in a legacy pair rather than blanking it", () => {
// `null` is a legal `_meta` value, so the migration must not fold it into
// `""` — only a genuinely absent `value` defaults.
expect(normalizeStoredMetadata([{ key: "z", value: null }])).toEqual({
z: null,
});
});
it.each([
["the object form", { __proto__: { polluted: true } } as unknown],
[
"a hand-edited file's object form",
JSON.parse('{"__proto__":{"polluted":true}}'),
],
["the legacy pair form", [{ key: "__proto__", value: { polluted: true } }]],
])("stores a `__proto__` key from %s as an own property", (_label, input) => {
// Plain assignment hits the prototype setter: the entry vanishes and, for
// an object value, the result gets a caller-controlled prototype. Both
// branches must define rather than set — the object branch regressed once
// when it changed from a spread to a per-key loop.
const out = normalizeStoredMetadata(input);
expect(Object.getPrototypeOf(out)).toBe(Object.prototype);
expect(({} as Record<string, unknown>).polluted).toBeUndefined();
});
it("stores a legacy `__proto__` key as an own property", () => {
// Plain assignment would hit the prototype setter: the entry would vanish
// (and an object value would mutate the prototype) instead of being stored.
const out = normalizeStoredMetadata([
{ key: "__proto__", value: { polluted: true } },
]);
expect(Object.hasOwn(out, "__proto__")).toBe(true);
expect(out["__proto__"]).toEqual({ polluted: true });
expect(Object.getPrototypeOf(out)).toBe(Object.prototype);
expect(({} as Record<string, unknown>).polluted).toBeUndefined();
});
it("reads a legacy pair with no value as an empty string", () => {
expect(normalizeStoredMetadata([{ key: "k" }])).toEqual({ k: "" });
});
it("drops a malformed legacy entry with a warning, keeping its siblings", () => {
expect(
normalizeStoredMetadata([
"nope",
null,
{ value: "no key" },
{ key: "kept", value: 1 },
]),
).toEqual({ kept: 1 });
expect(warn).toHaveBeenCalledTimes(3);
});
it.each([
["undefined", undefined],
["null", null],
])("reads %s as the empty object without warning", (_label, input) => {
expect(normalizeStoredMetadata(input)).toEqual({});
expect(warn).not.toHaveBeenCalled();
});
it.each([
["a string", "oops"],
["a number", 7],
["a boolean", true],
])("drops %s with a warning", (_label, input) => {
expect(normalizeStoredMetadata(input)).toEqual({});
expect(warn).toHaveBeenCalledTimes(1);
});
it("drops a value that parses but cannot be sent, keeping its siblings", () => {
// A hand-edited catalog reaches this untouched by any editor, so it is a
// real source of `1e400` — which parses to Infinity and serializes to null.
const out = normalizeStoredMetadata(
JSON.parse('{"good":1,"bad":1e400,"alsoGood":{"n":2}}'),
);
expect(out).toEqual({ good: 1, alsoGood: { n: 2 } });
expect(warn).toHaveBeenCalledTimes(1);
});
it("drops a non-serializable value nested inside an object", () => {
const out = normalizeStoredMetadata(JSON.parse('{"a":{"deep":[1e400]}}'));
expect(out).toEqual({});
});
it("applies the same check to a legacy pair value", () => {
const out = normalizeStoredMetadata([
{ key: "ok", value: 1 },
{ key: "bad", value: JSON.parse("1e400") },
]);
expect(out).toEqual({ ok: 1 });
});
it("never prints the offending value in a warning", () => {
// `_meta` can hold credentials, and these warnings go to ordinary console
// output — so a diagnostic may name the key and the type, never the value.
normalizeStoredMetadata([
{ value: { accessToken: "sk-live-nope" } },
{ key: "leaky", value: JSON.parse('{"token":1e400}') },
]);
normalizeStoredMetadata("sk-live-also-nope");
const printed = JSON.stringify(warn.mock.calls);
expect(printed).not.toContain("sk-live");
expect(printed).not.toContain("accessToken");
// The key is still named, so the warning stays actionable.
expect(printed).toContain("leaky");
});
it("migrates a legacy file to the object shape on the next write", () => {
// Read side accepts the pair array; write side emits only the object, so
// one round-trip is the migration.
// The legacy on-disk shape no longer matches `MCPConfig` — that is what
// the test is about — so the fixture is untyped and cast once where it
// crosses into the typed parser, the same boundary a real file crosses.
const legacyFile: unknown = {
mcpServers: {
legacy: {
type: "streamable-http",
url: "https://x.test/mcp",
metadata: [{ key: "tenant", value: "acme" }],
},
},
};
const entries = mcpConfigToServerEntries(legacyFile as MCPConfig);
expect(entries[0]?.settings?.metadata).toEqual({ tenant: "acme" });
const round = serverEntriesToMcpConfig(entries);
expect(round.mcpServers.legacy?.metadata).toEqual({ tenant: "acme" });
});
});
describe("envRecordToPairs / envPairsToRecord", () => {
it("envRecordToPairs preserves key insertion order", () => {
expect(envRecordToPairs({ B: "2", A: "1" })).toEqual([
@ -1469,7 +1649,7 @@ describe("stdio env / cwd mirroring", () => {
headers: [],
env: [{ key: "API_KEY", value: "secret" }],
cwd: "/srv/app",
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,

View file

@ -13,7 +13,7 @@ function prompt(name: string): Prompt {
const AUTO_REFRESH_SETTINGS: InspectorServerSettings = {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,

View file

@ -13,7 +13,7 @@ function template(name: string): ResourceTemplate {
const AUTO_REFRESH_SETTINGS: InspectorServerSettings = {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,

View file

@ -13,7 +13,7 @@ function resource(uri: string): Resource {
const AUTO_REFRESH_SETTINGS: InspectorServerSettings = {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,

View file

@ -13,7 +13,7 @@ function tool(name: string): Tool {
const AUTO_REFRESH_SETTINGS: InspectorServerSettings = {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,

View file

@ -11,7 +11,7 @@ function prompt(name: string): Prompt {
const PAGINATED_SETTINGS: InspectorServerSettings = {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,

View file

@ -11,7 +11,7 @@ function resource(uri: string): Resource {
const PAGINATED_SETTINGS: InspectorServerSettings = {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,

View file

@ -11,7 +11,7 @@ function tool(name: string): Tool {
const PAGINATED_SETTINGS: InspectorServerSettings = {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 60000,

View file

@ -10,7 +10,10 @@ import { act, renderHook, waitFor } from "@testing-library/react";
import { mkdtempSync, rmSync, writeFileSync, readFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { useServers } from "@inspector/core/react/useServers";
import {
ServerListReloadError,
useServers,
} from "@inspector/core/react/useServers";
import { createRemoteApp } from "@inspector/core/mcp/remote/node/server";
import { DEFAULT_SEED_CONFIG } from "@inspector/core/mcp/serverList";
import { InMemorySecretStore } from "@inspector/core/auth/node/secret-store";
@ -412,7 +415,7 @@ describe("useServers", () => {
await result.current.updateServerSettings("alpha", {
headers: [{ key: "X-Tenant", value: "acme" }],
env: [],
metadata: [{ key: "trace", value: "abc" }],
metadata: { trace: "abc" },
connectionTimeout: 5000,
requestTimeout: 30000,
taskTtl: 30000,
@ -425,7 +428,7 @@ describe("useServers", () => {
expect(result.current.servers[0]?.settings).toEqual({
headers: [{ key: "X-Tenant", value: "acme" }],
env: [],
metadata: [{ key: "trace", value: "abc" }],
metadata: { trace: "abc" },
connectionTimeout: 5000,
requestTimeout: 30000,
taskTtl: 30000,
@ -444,7 +447,7 @@ describe("useServers", () => {
expect(stored.url).toBe("https://x.test/mcp");
expect(stored).not.toHaveProperty("settings");
expect(stored.headers).toEqual({ "X-Tenant": "acme" });
expect(stored.metadata).toEqual([{ key: "trace", value: "abc" }]);
expect(stored.metadata).toEqual({ trace: "abc" });
expect(stored.connectionTimeout).toBe(5000);
expect(stored.requestTimeout).toBe(30000);
expect(stored.taskTtl).toBe(30000);
@ -461,7 +464,7 @@ describe("useServers", () => {
await result.current.updateServerSettings("nonexistent", {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 0,
@ -511,7 +514,7 @@ describe("useServers", () => {
expect(result.current.servers[0]?.settings).toEqual({
headers: [{ key: "X-Keep", value: "yes" }],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
// Absent taskTtl on disk reads back as the product default for the form.
@ -1201,6 +1204,186 @@ describe("useServers", () => {
}
});
/**
* #1914 — a write that lands but whose list reload fails must reject.
*
* `ServerConfigModal` renders `submitError` only for something `onSubmit`
* threw, so while the post-write refresh swallowed its failure into the
* `error` state the Add Server modal closed as though nothing had gone
* wrong. That is the "silent failure" in the report: the POST succeeded,
* the GET 500'd, and the user saw neither the new row nor an error.
*
* `failGetAfterWrite` lets the seed load and the write itself through, then
* fails every subsequent list read — the shape a container with no
* reachable keychain produced before #1848.
*/
function failGetAfterWrite(message: string): {
fetchFn: typeof fetch;
startFailing: () => void;
} {
let failing = false;
const fetchFn: typeof fetch = async (input, init) => {
const req =
input instanceof Request
? input
: new Request(input as string | URL, init);
if (failing && req.method === "GET" && req.url.endsWith("/api/servers")) {
return new Response(JSON.stringify({ error: message }), {
status: 500,
headers: { "Content-Type": "application/json" },
});
}
return h.fetchFn(req);
};
return {
fetchFn,
startFailing: () => {
failing = true;
},
};
}
it("addServer rejects when the post-write list reload fails (#1914)", async () => {
const { fetchFn, startFailing } = failGetAfterWrite("keyring unavailable");
const { result } = renderHook(() =>
useServers({ baseUrl: "http://test.local", fetchFn }),
);
await waitFor(() => expect(result.current.loading).toBe(false));
startFailing();
await act(async () => {
await expect(
result.current.addServer("alpha", { type: "stdio", command: "node" }),
).rejects.toThrow(
/The server was added, but the server list could not be reloaded: keyring unavailable/,
);
});
// The write itself landed, and the message says so — a bare failure would
// send the user into a retry that trips the duplicate-id check.
expect(readConfig(h.configPath).mcpServers.alpha).toEqual({
type: "stdio",
command: "node",
});
// Still recorded in `error` for any list-level UI reading it.
await waitFor(() =>
expect(result.current.error).toBe("keyring unavailable"),
);
expect(result.current.loading).toBe(false);
});
it("updateServer rejects when the post-write list reload fails (#1914)", async () => {
const { fetchFn, startFailing } = failGetAfterWrite("disk full");
const { result } = renderHook(() =>
useServers({ baseUrl: "http://test.local", fetchFn }),
);
await waitFor(() => expect(result.current.loading).toBe(false));
const existing = result.current.servers[0]!.id;
startFailing();
await act(async () => {
await expect(
result.current.updateServer(existing, existing, {
type: "stdio",
command: "node",
}),
).rejects.toThrow(
/The server was saved, but the server list could not be reloaded: disk full/,
);
});
});
it("updateServerSettings rejects when the post-write list reload fails (#1914)", async () => {
const { fetchFn, startFailing } = failGetAfterWrite("disk full");
const { result } = renderHook(() =>
useServers({ baseUrl: "http://test.local", fetchFn }),
);
await waitFor(() => expect(result.current.loading).toBe(false));
const existing = result.current.servers[0]!.id;
startFailing();
await act(async () => {
await expect(
result.current.updateServerSettings(existing, {
headers: [],
env: [],
metadata: {},
roots: [],
connectionTimeout: 5000,
requestTimeout: 30000,
taskTtl: 30000,
maxFetchRequests: 1000,
paginatedLists: true,
}),
).rejects.toThrow(
/The server was saved, but the server list could not be reloaded: disk full/,
);
});
});
it("tags a post-write reload failure as ServerListReloadError, not a failed write (#1914)", async () => {
// The pagination toggle updates optimistically and reverts the live
// client on a rejection. That is right for a failed PUT and wrong for a
// failed reload — the setting IS on disk — so the two must be tellable
// apart by type, not by message matching.
const { fetchFn, startFailing } = failGetAfterWrite("disk full");
const { result } = renderHook(() =>
useServers({ baseUrl: "http://test.local", fetchFn }),
);
await waitFor(() => expect(result.current.loading).toBe(false));
startFailing();
await act(async () => {
await expect(
result.current.addServer("alpha", { type: "stdio", command: "node" }),
).rejects.toBeInstanceOf(ServerListReloadError);
});
// A rejected *write* stays a plain Error, so a caller branching on the
// type still rolls back for the case the rollback was written for.
await act(async () => {
const err = await result.current
.addServer("alpha", { type: "stdio", command: "node" })
.catch((e: unknown) => e);
expect(err).toBeInstanceOf(Error);
expect(err).not.toBeInstanceOf(ServerListReloadError);
});
});
it("removeServer rejects when the post-write list reload fails (#1914)", async () => {
const { fetchFn, startFailing } = failGetAfterWrite("disk full");
const { result } = renderHook(() =>
useServers({ baseUrl: "http://test.local", fetchFn }),
);
await waitFor(() => expect(result.current.loading).toBe(false));
const existing = result.current.servers[0]!.id;
startFailing();
await act(async () => {
await expect(result.current.removeServer(existing)).rejects.toThrow(
/The server was removed, but the server list could not be reloaded: disk full/,
);
});
expect(readConfig(h.configPath).mcpServers[existing]).toBeUndefined();
});
it("a plain refresh() still resolves on a failed list read, recording the error (#1914)", async () => {
// The rethrow is scoped to post-write reloads. `refresh()` is awaited by
// the mount effect and the SSE loop, neither of which has anywhere to put
// a rejection, so it must keep swallowing into `error`.
const { fetchFn, startFailing } = failGetAfterWrite("keyring unavailable");
const { result } = renderHook(() =>
useServers({ baseUrl: "http://test.local", fetchFn }),
);
await waitFor(() => expect(result.current.loading).toBe(false));
startFailing();
await act(async () => {
await expect(result.current.refresh()).resolves.toBeUndefined();
});
expect(result.current.error).toBe("keyring unavailable");
});
it("uses DEFAULT_SEED_CONFIG keys on the first load (seed-write contract)", async () => {
const { result } = renderHook(() =>
useServers({ baseUrl: "http://test.local", fetchFn: h.fetchFn }),

View file

@ -389,7 +389,7 @@ describe("InspectorClient coverage backfill", () => {
serverSettings: {
headers: [],
env: [],
metadata: [{ key: "x-test", value: "1" }],
metadata: { "x-test": "1" },
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 0,
@ -655,7 +655,7 @@ describe("InspectorClient coverage backfill", () => {
serverSettings: {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 1,
requestTimeout: 0,
taskTtl: 0,

View file

@ -506,7 +506,7 @@ describe("modern-era negotiation (2026-07-28)", () => {
versionNegotiation: eraToVersionNegotiation("modern"),
serverSettings: {
headers: [],
metadata: [],
metadata: {},
env: [],
connectionTimeout: 0,
requestTimeout: 0,
@ -535,7 +535,7 @@ describe("modern-era negotiation (2026-07-28)", () => {
versionNegotiation: eraToVersionNegotiation("modern"),
serverSettings: {
headers: [],
metadata: [],
metadata: {},
env: [],
connectionTimeout: 0,
requestTimeout: 0,
@ -586,7 +586,7 @@ describe("modern-era negotiation (2026-07-28)", () => {
versionNegotiation: eraToVersionNegotiation("modern"),
serverSettings: {
headers: [],
metadata: [],
metadata: {},
env: [],
connectionTimeout: 0,
requestTimeout: 0,

View file

@ -53,7 +53,9 @@ import type {
MessageEntry,
ConnectionStatus,
FetchRequestEntryBase,
RequestMetadata,
} from "@inspector/core/mcp/types.js";
import type { InspectorLogger } from "@inspector/core/logging/logger.js";
import type { JsonValue } from "@inspector/core/json/jsonUtils.js";
import type {
TypedEvent,
@ -164,7 +166,7 @@ function settleInFlight(call: Promise<unknown>): InFlightCall {
/** Get all resources from the client via listResources() (paginates if needed). */
async function getAllResources(
client: InspectorClient,
metadata?: Record<string, string>,
metadata?: RequestMetadata,
): Promise<Resource[]> {
const collected: Resource[] = [];
let cursor: string | undefined;
@ -180,7 +182,7 @@ async function getAllResources(
/** Get all resource templates via listResourceTemplates() (paginates if needed). */
async function getAllResourceTemplates(
client: InspectorClient,
metadata?: Record<string, string>,
metadata?: RequestMetadata,
): Promise<ResourceTemplate[]> {
const collected: ResourceTemplate[] = [];
let cursor: string | undefined;
@ -196,7 +198,7 @@ async function getAllResourceTemplates(
/** Get all prompts via listPrompts() (paginates if needed). */
async function getAllPrompts(
client: InspectorClient,
metadata?: Record<string, string>,
metadata?: RequestMetadata,
): Promise<Prompt[]> {
const collected: Prompt[] = [];
let cursor: string | undefined;
@ -386,7 +388,7 @@ describe("InspectorClient", () => {
serverSettings: {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 50,
requestTimeout: 0,
taskTtl: 0,
@ -756,7 +758,7 @@ describe("InspectorClient", () => {
const initial = {
headers: [],
env: [],
metadata: [],
metadata: {},
connectionTimeout: 0,
requestTimeout: 0,
taskTtl: 0,
@ -1372,6 +1374,256 @@ describe("InspectorClient", () => {
messageLogState.destroy();
});
it("sends a structured default _meta value as JSON, not as a string (#1910)", async () => {
// The whole point of #1910: a `_meta` value may be an object, an array,
// a number, a boolean or `null`. The Inspector used to type this map as
// `Record<string, string>`, so a nested payload could only be sent by
// stringifying it — which is a different wire message than the one the
// user configured.
const structured = {
trace: { id: "abc123", sampled: true, hops: [1, 2] },
retries: 3,
beta: false,
unset: null,
};
client = new InspectorClient(
{
type: "stdio",
command: serverCommand.command,
args: serverCommand.args,
},
{
environment: { transport: createTransportNode },
defaultMetadata: structured,
},
);
const messageLogState = new MessageLogState(client);
await client.connect();
await client.listTools();
const listToolsReq = messageLogState
.getMessages()
.find(
(m) =>
m.direction === "request" &&
(m.message as { method?: string }).method === "tools/list",
);
expect(listToolsReq).toBeDefined();
expect(metaOf(listToolsReq!)).toMatchObject(structured);
messageLogState.destroy();
});
it.each([
["a float", 3.5],
["an object", { id: 1 }],
["an array", [1]],
["null", null],
["a boolean", true],
// Past `Number.MAX_SAFE_INTEGER` the float64 value is no longer the
// integer it was written as, and zod 4's `.int()` rejects it — so
// `Number.isInteger` is too loose a check here.
["an unsafe integer", Number.MAX_SAFE_INTEGER + 1],
])(
"drops a reserved progressToken that is %s rather than sending it",
async (_label, badToken) => {
// Every other `_meta` value may be any JSON (#1910), but the spec
// constrains `progressToken` to `string | integer`
// (`ProgressTokenSchema`). Sending a bad one would make a conforming
// server reject the whole request, so the member is dropped — the key
// is optional, so what goes out is still well-formed.
//
// `progress: false` is what makes this observable. With progress on
// (the default) the SDK sets `onprogress` and stamps its own message
// id over `_meta.progressToken`, so a caller's value never reaches the
// wire either way; with it off nothing overwrites the member and a bad
// one would ship.
client = new InspectorClient(
{
type: "stdio",
command: serverCommand.command,
args: serverCommand.args,
},
{
environment: { transport: createTransportNode },
progress: false,
defaultMetadata: { progressToken: badToken, keep: "me" },
},
);
const messageLogState = new MessageLogState(client);
await client.connect();
await client.listTools();
const listToolsReq = messageLogState
.getMessages()
.find(
(m) =>
m.direction === "request" &&
(m.message as { method?: string }).method === "tools/list",
);
expect(listToolsReq).toBeDefined();
const meta = metaOf(listToolsReq!);
expect(meta).not.toHaveProperty("progressToken");
// Only the offending member is dropped, not the whole payload.
expect(meta.keep).toBe("me");
messageLogState.destroy();
},
);
it.each([
["an array", [1]],
["a string", "t1"],
["an object with a non-string taskId", { taskId: 5 }],
])(
"drops a related-task member that is %s, keeping its siblings",
async (_label, badRelatedTask) => {
// `progressToken` is not the only reserved member: the pinned SDK also
// constrains `io.modelcontextprotocol/related-task` to
// `{ taskId: string }`. A bad one makes a conforming server reject the
// whole request, so it is dropped the same way.
client = new InspectorClient(
{
type: "stdio",
command: serverCommand.command,
args: serverCommand.args,
},
{
environment: { transport: createTransportNode },
progress: false,
defaultMetadata: {
"io.modelcontextprotocol/related-task": badRelatedTask,
keep: "me",
},
},
);
const messageLogState = new MessageLogState(client);
await client.connect();
await client.listTools();
const req = messageLogState
.getMessages()
.find(
(m) =>
m.direction === "request" &&
(m.message as { method?: string }).method === "tools/list",
);
const meta = metaOf(req!);
expect(meta).not.toHaveProperty("io.modelcontextprotocol/related-task");
expect(meta.keep).toBe("me");
messageLogState.destroy();
},
);
it("keeps a well-formed related-task member", async () => {
client = new InspectorClient(
{
type: "stdio",
command: serverCommand.command,
args: serverCommand.args,
},
{
environment: { transport: createTransportNode },
progress: false,
defaultMetadata: {
"io.modelcontextprotocol/related-task": { taskId: "task-1" },
},
},
);
const messageLogState = new MessageLogState(client);
await client.connect();
await client.listTools();
const req = messageLogState
.getMessages()
.find(
(m) =>
m.direction === "request" &&
(m.message as { method?: string }).method === "tools/list",
);
expect(metaOf(req!)["io.modelcontextprotocol/related-task"]).toEqual({
taskId: "task-1",
});
messageLogState.destroy();
});
it("warns with the rejected progressToken's type, never its value", async () => {
// This logger is persisted by real clients (the TUI writes it to
// `~/.mcp-inspector/auth.log`), and an invalid `progressToken` can now be
// an object holding credentials — so the warning must not echo it.
const warn = vi.fn();
const noop = () => {};
const logger: InspectorLogger = {
level: "warn",
fatal: noop,
error: noop,
warn,
info: noop,
debug: noop,
trace: noop,
silent: noop,
child: () => logger,
};
client = new InspectorClient(
{
type: "stdio",
command: serverCommand.command,
args: serverCommand.args,
},
{
environment: { transport: createTransportNode, logger },
progress: false,
defaultMetadata: {
progressToken: { accessToken: "sk-live-should-not-appear" },
},
},
);
await client.connect();
await client.listTools();
// The offending key travels in the log's bindings, not in the message —
// the message is shared by every reserved member the sanitizer drops.
const call = warn.mock.calls.find(
(c) => (c[0] as { key?: string })?.key === "progressToken",
);
expect(call).toBeDefined();
expect(call![0]).toEqual({ key: "progressToken", received: "object" });
// Belt and braces: the secret must not appear anywhere in the record.
expect(JSON.stringify(warn.mock.calls)).not.toContain("sk-live");
});
it.each([
["a string", "tok-1"],
["an integer", 7],
// The boundary itself is still valid.
["the largest safe integer", Number.MAX_SAFE_INTEGER],
])("keeps a valid progressToken that is %s", async (_label, token) => {
client = new InspectorClient(
{
type: "stdio",
command: serverCommand.command,
args: serverCommand.args,
},
{
environment: { transport: createTransportNode },
// See the note above: with progress on, the SDK overwrites this.
progress: false,
defaultMetadata: { progressToken: token },
},
);
const messageLogState = new MessageLogState(client);
await client.connect();
await client.listTools();
const listToolsReq = messageLogState
.getMessages()
.find(
(m) =>
m.direction === "request" &&
(m.message as { method?: string }).method === "tools/list",
);
expect(metaOf(listToolsReq!).progressToken).toBe(token);
messageLogState.destroy();
});
it("call-time metadata overrides defaultMetadata on key collision", async () => {
client = new InspectorClient(
{
@ -4241,7 +4493,7 @@ describe("InspectorClient", () => {
timestamp: Date;
success: boolean;
error?: string;
metadata?: Record<string, string>;
metadata?: RequestMetadata;
}> = [];
client!.addEventListener(

Some files were not shown because too many files have changed in this diff Show more