Address PR #1236 review feedback

- Rename RequestMethod → MessageMethod. The type covers notifications and
  results, not just requests; the new name matches the JSDoc.
- Normalize imports to the @inspector/core/mcp/types.js path alias in
  HistoryListPanel, historyUtils, and HistoryScreen (HistoryControls was
  already using the alias).
- Document the cast invariant in extractMethod: SDK types message.method
  as string, but every MessageEntry in this app originates from SDK
  schemas, so the narrowing is safe.
- Type SAMPLE_METHODS as MessageMethod[] in HistoryControls stories so
  the literal-type narrowing isn't lost across the spread.

Skipped two reviewer suggestions: the Mantine Select inline cast (reviewer
self-resolved) and the stale-methodFilter-on-clear UX edge case (deferred
as a separate follow-up — not a Phase 5 audit gap).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
cliffhall 2026-04-27 10:24:30 -04:00
commit 6b3cecd736
7 changed files with 21 additions and 27 deletions

View file

@ -1,8 +1,9 @@
import type { Meta, StoryObj } from "@storybook/react-vite"; import type { Meta, StoryObj } from "@storybook/react-vite";
import { fn } from "storybook/test"; import { fn } from "storybook/test";
import type { MessageMethod } from "@inspector/core/mcp/types.js";
import { HistoryControls } from "./HistoryControls"; import { HistoryControls } from "./HistoryControls";
const SAMPLE_METHODS = [ const SAMPLE_METHODS: MessageMethod[] = [
"tools/call", "tools/call",
"tools/list", "tools/list",
"resources/read", "resources/read",
@ -11,14 +12,14 @@ const SAMPLE_METHODS = [
"prompts/list", "prompts/list",
"sampling/createMessage", "sampling/createMessage",
"elicitation/create", "elicitation/create",
] as const; ];
const meta: Meta<typeof HistoryControls> = { const meta: Meta<typeof HistoryControls> = {
title: "Groups/HistoryControls", title: "Groups/HistoryControls",
component: HistoryControls, component: HistoryControls,
args: { args: {
searchText: "", searchText: "",
availableMethods: [...SAMPLE_METHODS], availableMethods: SAMPLE_METHODS,
onSearchChange: fn(), onSearchChange: fn(),
onMethodFilterChange: fn(), onMethodFilterChange: fn(),
}, },

View file

@ -1,12 +1,12 @@
import { Select, Stack, TextInput, Title } from "@mantine/core"; import { Select, Stack, TextInput, Title } from "@mantine/core";
import type { RequestMethod } from "@inspector/core/mcp/types.js"; import type { MessageMethod } from "@inspector/core/mcp/types.js";
export interface HistoryControlsProps { export interface HistoryControlsProps {
searchText: string; searchText: string;
methodFilter?: RequestMethod; methodFilter?: MessageMethod;
availableMethods: RequestMethod[]; availableMethods: MessageMethod[];
onSearchChange: (text: string) => void; onSearchChange: (text: string) => void;
onMethodFilterChange: (method: RequestMethod | undefined) => void; onMethodFilterChange: (method: MessageMethod | undefined) => void;
} }
export function HistoryControls({ export function HistoryControls({
@ -31,7 +31,7 @@ export function HistoryControls({
data={availableMethods} data={availableMethods}
value={methodFilter ?? null} value={methodFilter ?? null}
onChange={(value) => onChange={(value) =>
onMethodFilterChange((value as RequestMethod | null) ?? undefined) onMethodFilterChange((value as MessageMethod | null) ?? undefined)
} }
clearable clearable
/> />

View file

@ -8,10 +8,7 @@ import {
Text, Text,
Title, Title,
} from "@mantine/core"; } from "@mantine/core";
import type { import type { MessageEntry, MessageMethod } from "@inspector/core/mcp/types.js";
MessageEntry,
RequestMethod,
} from "../../../../../../core/mcp/types.js";
import { HistoryEntry } from "../HistoryEntry/HistoryEntry"; import { HistoryEntry } from "../HistoryEntry/HistoryEntry";
import { ListToggle } from "../../elements/ListToggle/ListToggle"; import { ListToggle } from "../../elements/ListToggle/ListToggle";
import { extractMethod } from "../historyUtils.js"; import { extractMethod } from "../historyUtils.js";
@ -20,7 +17,7 @@ export interface HistoryListPanelProps {
entries: MessageEntry[]; entries: MessageEntry[];
pinnedIds: Set<string>; pinnedIds: Set<string>;
searchText: string; searchText: string;
methodFilter?: RequestMethod; methodFilter?: MessageMethod;
onClearAll: () => void; onClearAll: () => void;
onExport: () => void; onExport: () => void;
onReplay: (id: string) => void; onReplay: (id: string) => void;
@ -56,7 +53,7 @@ function formatHistoryTitle(count: number): string {
function matchesFilters( function matchesFilters(
entry: MessageEntry, entry: MessageEntry,
searchText: string, searchText: string,
methodFilter?: RequestMethod, methodFilter?: MessageMethod,
): boolean { ): boolean {
const method = extractMethod(entry); const method = extractMethod(entry);
if (methodFilter && method !== methodFilter) return false; if (methodFilter && method !== methodFilter) return false;

View file

@ -1,11 +1,10 @@
import type { import type { MessageEntry, MessageMethod } from "@inspector/core/mcp/types.js";
MessageEntry,
RequestMethod,
} from "../../../../../core/mcp/types.js";
export function extractMethod(entry: MessageEntry): RequestMethod { export function extractMethod(entry: MessageEntry): MessageMethod {
if ("method" in entry.message) { if ("method" in entry.message) {
return entry.message.method as RequestMethod; // Cast: SDK types message.method as `string`, but every entry in this
// app's MessageEntry log originates from MCP SDK schemas.
return entry.message.method as MessageMethod;
} }
return "response"; return "response";
} }

View file

@ -1,9 +1,6 @@
import { useMemo, useState } from "react"; import { useMemo, useState } from "react";
import { Card, Flex, Stack } from "@mantine/core"; import { Card, Flex, Stack } from "@mantine/core";
import type { import type { MessageEntry, MessageMethod } from "@inspector/core/mcp/types.js";
MessageEntry,
RequestMethod,
} from "../../../../../../core/mcp/types.js";
import { HistoryControls } from "../../groups/HistoryControls/HistoryControls"; import { HistoryControls } from "../../groups/HistoryControls/HistoryControls";
import { HistoryListPanel } from "../../groups/HistoryListPanel/HistoryListPanel.js"; import { HistoryListPanel } from "../../groups/HistoryListPanel/HistoryListPanel.js";
import { extractMethod } from "../../groups/historyUtils.js"; import { extractMethod } from "../../groups/historyUtils.js";
@ -43,7 +40,7 @@ export function HistoryScreen({
onTogglePin, onTogglePin,
}: HistoryScreenProps) { }: HistoryScreenProps) {
const [searchText, setSearchText] = useState(""); const [searchText, setSearchText] = useState("");
const [methodFilter, setMethodFilter] = useState<RequestMethod | undefined>(); const [methodFilter, setMethodFilter] = useState<MessageMethod | undefined>();
const availableMethods = useMemo( const availableMethods = useMemo(
() => Array.from(new Set(entries.map(extractMethod))).sort(), () => Array.from(new Set(entries.map(extractMethod))).sort(),

View file

@ -103,7 +103,7 @@ export interface MessageEntry {
} }
/** Method name for any MessageEntry traffic, plus synthetic "response" for result/error entries. */ /** Method name for any MessageEntry traffic, plus synthetic "response" for result/error entries. */
export type RequestMethod = export type MessageMethod =
| ClientRequest["method"] | ClientRequest["method"]
| ClientNotification["method"] | ClientNotification["method"]
| ServerRequest["method"] | ServerRequest["method"]

View file

@ -356,7 +356,7 @@ will close out as part of that work.
- **Purpose**: Search + method filter sidebar for the history screen. - **Purpose**: Search + method filter sidebar for the history screen.
- **Current props**: `searchText`, `methodFilter`, `onSearchChange`, `onMethodFilterChange`. - **Current props**: `searchText`, `methodFilter`, `onSearchChange`, `onMethodFilterChange`.
- **MCP schema touch points**: Method filter values are MCP `Request.method` literals. - **MCP schema touch points**: Method filter values are MCP `Request.method` literals.
- **Target props**: `searchText`, `methodFilter?: RequestMethod`, `availableMethods: RequestMethod[]`, `onSearchChange`, `onMethodFilterChange`. - **Target props**: `searchText`, `methodFilter?: MessageMethod`, `availableMethods: MessageMethod[]`, `onSearchChange`, `onMethodFilterChange`.
- **Callbacks → core hook**: `useMessageLog`. - **Callbacks → core hook**: `useMessageLog`.
- **Internal refactors**: Type the method filter as a union of MCP method strings. - **Internal refactors**: Type the method filter as a union of MCP method strings.