chore(web): address Copilot review on #2127

- Move REPLAYABLE_PROTOCOL_METHODS / isReplayableProtocolMethod out of
  components/groups/protocolUtils.ts into utils/replayableProtocolMethods.ts.
  It is a pure predicate over a string set, so by the project's own lib-vs-utils
  rule it never belonged under components/ — and keeping it there was what made
  lib/protocolReplay.ts import 'up' into the component graph. Both consumers now
  import from utils/; the predicate keeps its own test.

- Give replayProtocolRequest an exported ReplayClient = Pick<InspectorClient, ...>
  naming the nine methods it calls, so a caller (and the test) can satisfy the
  real contract instead of casting a stub into a whole client.

- Rebuild the protocolReplay fixtures as plain typed MessageEntry values. The
  double cast had been hiding a real defect: MessageEntry.timestamp is a Date
  and the fixture passed a number, so the assertion confirmed a shape
  messagesToLogEntries cannot produce.

- Replace the window casts in authToken.test.ts with Reflect.set /
  Reflect.deleteProperty. No 'as unknown as' remains in either test.

- Drop the stale 'floats top-right' sentence on ReAuthBannerBar, which moved
  verbatim and arrived contradicting the paragraph below it.

Declined: re-homing ReAuthBannerBar's transform/zIndex as a theme variant.
'styles' is the Styles API rather than an inline style, Mantine exposes neither
property as a prop, and a styling change is out of scope for a phase whose
safety argument is that nothing was edited.

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-26 18:24:09 -04:00
commit 4b119c9a95
8 changed files with 156 additions and 87 deletions

View file

@ -30,8 +30,8 @@ import {
extractMethod,
extractResultType,
extractSubscriptionId,
isReplayableProtocolMethod,
} from "../protocolUtils.js";
import { isReplayableProtocolMethod } from "../../../utils/replayableProtocolMethods";
export interface ProtocolEntryProps {
entry: MessageEntry;

View file

@ -3,12 +3,12 @@ import { Paper } from "@mantine/core";
// The re-auth popup. A `Paper` so every static style is a prop; the stacking
// order goes through `styles.root` since Mantine has no `z` prop.
//
// Floats top-right rather than spanning the top as a sticky full-bleed bar.
// The bar cost the whole view a band of vertical space for what is a
// notification about one server, and it sat directly above the monitoring
// sidebar that an OAuth failure now opens (#2108) — the two things a user needs
// at once here are this affordance and those requests, so it must not push them
// around. `fixed`, not `sticky`, so scrolling the server list leaves it put.
// Floats rather than spanning the top as a sticky full-bleed bar. The bar cost
// the whole view a band of vertical space for what is a notification about one
// server, and it sat directly above the monitoring sidebar that an OAuth
// failure now opens (#2108) — the two things a user needs at once here are this
// affordance and those requests, so it must not push them around. `fixed`, not
// `sticky`, so scrolling the server list leaves it put.
//
// Centered, and deliberately WITHOUT an overlay. Every corner is spoken for —
// the toast stack owns bottom-right (see `main.tsx`), the right edge is the

View file

@ -13,30 +13,6 @@ export function extractMethod(entry: MessageEntry): MessageMethod {
return "response";
}
/**
* Request methods the Protocol Replay action can re-issue (client→server reads
* and calls). Server→client requests (roots/list, sampling, elicitation) and
* side-effectful methods (logging/setLevel, subscribe) are intentionally
* excluded. Single source of truth: `ProtocolEntry` hides the Replay button for
* anything not listed here, and App's `replayProtocolRequest` gates dispatch on
* the same set.
*/
export const REPLAYABLE_PROTOCOL_METHODS: ReadonlySet<string> = new Set([
"tools/call",
"prompts/get",
"resources/read",
"tools/list",
"prompts/list",
"resources/list",
"resources/templates/list",
"tasks/list",
"ping",
]);
export function isReplayableProtocolMethod(method: string): boolean {
return REPLAYABLE_PROTOCOL_METHODS.has(method);
}
// --- Modern-era (2026-07-28) message vocabulary -----------------------------
//
// The modern era changes the over-the-wire conversation (spec §7.2–7.4): every

View file

@ -44,9 +44,7 @@ describe("getAuthToken", () => {
beforeEach(() => {
setSearch("");
window.sessionStorage.clear();
delete (window as unknown as Record<string, unknown>)[
INSPECTOR_API_TOKEN_GLOBAL
];
Reflect.deleteProperty(window, INSPECTOR_API_TOKEN_GLOBAL);
});
afterEach(() => {
@ -54,14 +52,11 @@ describe("getAuthToken", () => {
restoreSessionStorage();
setSearch("");
window.sessionStorage.clear();
delete (window as unknown as Record<string, unknown>)[
INSPECTOR_API_TOKEN_GLOBAL
];
Reflect.deleteProperty(window, INSPECTOR_API_TOKEN_GLOBAL);
});
it("prefers the injected global and persists it", () => {
(window as unknown as Record<string, unknown>)[INSPECTOR_API_TOKEN_GLOBAL] =
"from-global";
Reflect.set(window, INSPECTOR_API_TOKEN_GLOBAL, "from-global");
setSearch(`?${KEY}=from-url`);
window.sessionStorage.setItem(KEY, "from-storage");
@ -70,14 +65,12 @@ describe("getAuthToken", () => {
});
it("ignores a non-string or empty global and falls through to the URL", () => {
(window as unknown as Record<string, unknown>)[INSPECTOR_API_TOKEN_GLOBAL] =
"";
Reflect.set(window, INSPECTOR_API_TOKEN_GLOBAL, "");
setSearch(`?${KEY}=from-url`);
expect(getAuthToken()).toBe("from-url");
(window as unknown as Record<string, unknown>)[INSPECTOR_API_TOKEN_GLOBAL] =
123;
Reflect.set(window, INSPECTOR_API_TOKEN_GLOBAL, 123);
expect(getAuthToken()).toBe("from-url");
expect(window.sessionStorage.getItem(KEY)).toBe("from-url");
});
@ -96,8 +89,7 @@ describe("getAuthToken", () => {
throw new Error("blocked");
});
stubSessionStorage({ setItem });
(window as unknown as Record<string, unknown>)[INSPECTOR_API_TOKEN_GLOBAL] =
"from-global";
Reflect.set(window, INSPECTOR_API_TOKEN_GLOBAL, "from-global");
expect(getAuthToken()).toBe("from-global");
expect(setItem).toHaveBeenCalledWith(KEY, "from-global");

View file

@ -1,16 +1,24 @@
import { describe, expect, it, vi } from "vitest";
import type { Tool } from "@modelcontextprotocol/client";
import type { InspectorClient } from "@inspector/core/mcp/index.js";
import type { MessageEntry } from "@inspector/core/mcp/types.js";
import { messagesToLogEntries, replayProtocolRequest } from "./protocolReplay";
import {
messagesToLogEntries,
replayProtocolRequest,
type ReplayClient,
} from "./protocolReplay";
describe("messagesToLogEntries", () => {
const notification = (method: string, params: unknown): MessageEntry =>
({
direction: "notification",
timestamp: 1000,
message: { jsonrpc: "2.0", method, params },
}) as unknown as MessageEntry;
const at = new Date("2026-01-01T00:00:00Z");
const notification = (
method: string,
params: Record<string, unknown>,
): MessageEntry => ({
id: `n-${method}`,
timestamp: at,
direction: "notification",
message: { jsonrpc: "2.0", method, params },
});
it("keeps only notifications/message and carries the timestamp", () => {
const entries = messagesToLogEntries([
@ -18,49 +26,62 @@ describe("messagesToLogEntries", () => {
notification("notifications/progress", { progress: 1 }),
]);
expect(entries).toEqual([
{ receivedAt: 1000, params: { level: "info", data: "hello" } },
{ receivedAt: at, params: { level: "info", data: "hello" } },
]);
});
it("skips requests and responses", () => {
const request = {
const request: MessageEntry = {
id: "r-1",
timestamp: at,
direction: "request",
timestamp: 1,
// A request frame carrying the same method — only `direction` should
// decide, so this must still be skipped.
message: { jsonrpc: "2.0", id: 1, method: "notifications/message" },
} as unknown as MessageEntry;
const response = {
};
const response: MessageEntry = {
id: "r-2",
timestamp: at,
direction: "response",
timestamp: 2,
message: { jsonrpc: "2.0", id: 1, result: {} },
} as unknown as MessageEntry;
};
expect(messagesToLogEntries([request, response])).toEqual([]);
});
it("skips a notification carrying no method", () => {
const malformed = {
it("skips a frame labelled a notification that carries no method", () => {
// `direction` and the frame shape can disagree — the log records what
// arrived, so the `"method" in message` guard has to hold on its own.
const malformed: MessageEntry = {
id: "m-1",
timestamp: at,
direction: "notification",
timestamp: 3,
message: { jsonrpc: "2.0", result: {} },
} as unknown as MessageEntry;
message: { jsonrpc: "2.0", id: 1, result: {} },
};
expect(messagesToLogEntries([malformed])).toEqual([]);
});
});
describe("replayProtocolRequest", () => {
const tool = { name: "echo", inputSchema: { type: "object" } } as Tool;
const tool: Tool = { name: "echo", inputSchema: { type: "object" } };
// Typed against `ReplayClient`, so the mock has to satisfy the same contract
// the function declares rather than being cast into a whole InspectorClient.
// Each mock is typed against the real method signature, so a drift in
// `InspectorClient` surfaces here rather than being absorbed by a cast. None
// of them needs a resolved value: `replayProtocolRequest` awaits the call and
// discards the result, so what is asserted is the dispatch, not the payload.
function makeClient() {
return {
callTool: vi.fn().mockResolvedValue({}),
getPrompt: vi.fn().mockResolvedValue({}),
readResource: vi.fn().mockResolvedValue({}),
listTools: vi.fn().mockResolvedValue({}),
listPrompts: vi.fn().mockResolvedValue({}),
listResources: vi.fn().mockResolvedValue({}),
listResourceTemplates: vi.fn().mockResolvedValue({}),
listRequestorTasks: vi.fn().mockResolvedValue({}),
ping: vi.fn().mockResolvedValue({}),
};
callTool: vi.fn<ReplayClient["callTool"]>(),
getPrompt: vi.fn<ReplayClient["getPrompt"]>(),
readResource: vi.fn<ReplayClient["readResource"]>(),
listTools: vi.fn<ReplayClient["listTools"]>(),
listPrompts: vi.fn<ReplayClient["listPrompts"]>(),
listResources: vi.fn<ReplayClient["listResources"]>(),
listResourceTemplates: vi.fn<ReplayClient["listResourceTemplates"]>(),
listRequestorTasks: vi.fn<ReplayClient["listRequestorTasks"]>(),
ping: vi.fn<ReplayClient["ping"]>(),
} satisfies ReplayClient;
}
const replay = (
@ -68,13 +89,7 @@ describe("replayProtocolRequest", () => {
method: string,
params?: Record<string, unknown>,
tools: Tool[] = [tool],
) =>
replayProtocolRequest(
client as unknown as InspectorClient,
method,
params,
tools,
);
) => replayProtocolRequest(client, method, params, tools);
it("refuses a method outside the shared replayable set", async () => {
const client = makeClient();
@ -153,7 +168,7 @@ describe("replayProtocolRequest", () => {
expect(client[fn]).toHaveBeenCalledWith("page-2");
});
it("passes no cursor when the entry carried none", async () => {
it("passes no cursor when the entry carried none, or carried a non-string", async () => {
const client = makeClient();
await expect(replay(client, "tools/list", undefined)).resolves.toBeNull();
expect(client.listTools).toHaveBeenCalledWith(undefined);

View file

@ -5,8 +5,11 @@ import type {
import type { InspectorClient } from "@inspector/core/mcp/index.js";
import type { JsonValue } from "@inspector/core/mcp/index.js";
import type { MessageEntry } from "@inspector/core/mcp/types.js";
// Type-only, so this creates no runtime edge into `components/` and the
// `components → lib → utils` direction still holds: `LogEntryData` is the shape
// the Logs screen renders, and this module exists to produce exactly that.
import type { LogEntryData } from "../components/elements/LogEntry/LogEntry";
import { isReplayableProtocolMethod } from "../components/groups/protocolUtils.js";
import { isReplayableProtocolMethod } from "../utils/replayableProtocolMethods";
// Derive `LogEntryData[]` from the MessageLog by filtering for the
// `notifications/message` notifications the server emits in response to
@ -37,8 +40,26 @@ export function messagesToLogEntries(messages: MessageEntry[]): LogEntryData[] {
// intentionally does NOT touch the Tools/Prompts/Resources panels. Returns a
// human-readable reason when the entry can't be replayed (unsupported method,
// or a tool that's no longer present), or null on a dispatched replay.
/**
* The slice of `InspectorClient` a replay actually reaches. Naming it — rather
* than taking the whole client — is what lets a caller (and a test) supply a
* value that satisfies the real contract instead of casting one in.
*/
export type ReplayClient = Pick<
InspectorClient,
| "callTool"
| "getPrompt"
| "readResource"
| "listTools"
| "listPrompts"
| "listResources"
| "listResourceTemplates"
| "listRequestorTasks"
| "ping"
>;
export async function replayProtocolRequest(
client: InspectorClient,
client: ReplayClient,
method: string,
params: Record<string, unknown> | undefined,
tools: Tool[],

View file

@ -0,0 +1,37 @@
import { describe, expect, it } from "vitest";
import {
isReplayableProtocolMethod,
REPLAYABLE_PROTOCOL_METHODS,
} from "./replayableProtocolMethods";
describe("isReplayableProtocolMethod", () => {
it("admits every client→server read and call in the set", () => {
for (const method of REPLAYABLE_PROTOCOL_METHODS) {
expect(isReplayableProtocolMethod(method)).toBe(true);
}
expect([...REPLAYABLE_PROTOCOL_METHODS].sort()).toEqual([
"ping",
"prompts/get",
"prompts/list",
"resources/list",
"resources/read",
"resources/templates/list",
"tasks/list",
"tools/call",
"tools/list",
]);
});
it("excludes server→client requests and side-effectful methods", () => {
for (const method of [
"roots/list",
"sampling/createMessage",
"elicitation/create",
"logging/setLevel",
"resources/subscribe",
"initialize",
]) {
expect(isReplayableProtocolMethod(method)).toBe(false);
}
});
});

View file

@ -0,0 +1,28 @@
/**
* Request methods the Protocol Replay action can re-issue (client→server reads
* and calls). Server→client requests (roots/list, sampling, elicitation) and
* side-effectful methods (logging/setLevel, subscribe) are intentionally
* excluded. Single source of truth: `ProtocolEntry` hides the Replay button for
* anything not listed here, and `replayProtocolRequest` gates dispatch on the
* same set.
*
* Lives in `utils/` rather than beside the Protocol components because it is a
* pure predicate over a string set with two consumers on opposite sides of the
* layering: a component and `lib/protocolReplay.ts`. Keeping it here is what
* lets the adapter reach it without importing "up" into `components/`.
*/
export const REPLAYABLE_PROTOCOL_METHODS: ReadonlySet<string> = new Set([
"tools/call",
"prompts/get",
"resources/read",
"tools/list",
"prompts/list",
"resources/list",
"resources/templates/list",
"tasks/list",
"ping",
]);
export function isReplayableProtocolMethod(method: string): boolean {
return REPLAYABLE_PROTOCOL_METHODS.has(method);
}