fix: return the era-parsed tool, and hide aggregate reports in paginated mode (#1909)

Copilot round 8.

The era contract validated with the wire schema but returned the raw input,
so the wire schema's normalization was discarded — both era schemas are
z.object, which strips unknown top-level fields as the strict codec does, so
salvaged tools could carry fields the strict path drops. The parsed output is
now what gets normalized and returned. This also simplifies the bridge: issues
come straight from the wire parse with their paths intact, so the custom-issue
forwarding (and its early describeIssue application) is gone.

The malformed-entry report is written by the aggregate walk's salvage, but in
paginated mode the tools/prompts/resources panels render the paged stores,
which neither write nor clear it — leaving a stale warning above a freshly
fetched page it does not describe, with indices into an aggregate the user is
no longer looking at. Those reports are now suppressed in paginated mode;
resource templates keep theirs, having no paged store. Suppressing rather than
adding per-page salvage: pointing at the wrong entry costs more than staying
quiet, and per-page salvage is a feature, not a display fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RDXCSrnSBV7vZfxycG5Z6E
Signed-off-by: cliffhall <cliff@futurescale.com>

clients/web/src/App.tsx

@@ -163,6 +163,7 @@ import {
 import { buildExportFilename, downloadJsonFile } from "./lib/downloadFile";
 import { INSPECTOR_SERVERS_TAB } from "./utils/inspectorTabs";
 import { enrichProtocolEntries } from "./utils/correlateTransportErrors";
+import { visibleMalformedListItems } from "./utils/malformedListReport";
 import {
   parseDeepLink,
   deepLinkConfigEquals,
@@ -1056,6 +1057,14 @@ function App() {
     setPaginatedListsOverride(null);
   }, [persistedPaginatedLists, activeServerId]);
   const paginatedLists = paginatedListsOverride ?? persistedPaginatedLists;
+  // The malformed-entry report is written by the aggregate walk's salvage. In
+  // paginated mode the tools/prompts/resources panels render the paged stores
+  // instead, which never write or clear it — so it would linger above a page it
+  // does not describe. See `visibleMalformedListItems` (#1909 + #1721).
+  const shownMalformedListItems = useMemo(
+    () => visibleMalformedListItems(malformedListItems, paginatedLists),
+    [malformedListItems, paginatedLists],
+  );
   const connected = connectionStatus === "connected";
   const toolsPagination = usePaginatedList({
     connected,
@@ -4377,7 +4386,7 @@ function App() {
           latencyMs={latencyMs}
           tools={tools}
           excludedTools={excludedTools}
-          malformedListItems={malformedListItems}
+          malformedListItems={shownMalformedListItems}
           prompts={prompts}
           resources={resources}
           resourceTemplates={resourceTemplates}

clients/web/src/test/integration/mcp/listSalvage-era.test.ts

@@ -197,6 +197,29 @@ describe("salvage era item contract (#1909)", () => {
     );
   });
 
+  it("strips unknown fields the way the strict codec does", async () => {
+    // The contract validates through a wire schema; returning the ORIGINAL
+    // value rather than the parsed one would leave salvaged tools carrying
+    // fields the strict path drops, so the two paths would hand the UI
+    // differently-shaped tools for the same server.
+    const strayTool = {
+      name: "stray",
+      inputSchema: { type: "object" },
+      notAToolField: "should not survive",
+    };
+    const parsed = toolItemSchemaForEra(false).safeParse(strayTool);
+    expect(parsed.success).toBe(true);
+    expect(parsed.success && parsed.data).not.toHaveProperty("notAToolField");
+
+    // And the strict path agrees — this is the shape being matched, not an
+    // invented rule.
+    const server = await startToolsServer([strayTool], false);
+    stop = server.stop;
+    const connected = await connect(server.url, "legacy");
+    const { tools } = await connected.listAllTools();
+    expect(tools[0]).not.toHaveProperty("notAToolField");
+  });
+
   it("the era contract matches the strict path on both divergences", () => {
     // Legacy: rejected, exactly as the strict parse above rejected it.
     expect(

clients/web/src/utils/malformedListReport.test.ts

@@ -0,0 +1,44 @@
+import { describe, it, expect } from "vitest";
+import type { MalformedListItem } from "@inspector/core/mcp";
+import { visibleMalformedListItems } from "./malformedListReport";
+
+const entry = (method: string): MalformedListItem => ({
+  method,
+  index: 0,
+  reason: "annotations: expected object",
+});
+
+const TOOLS = entry("tools/list");
+const PROMPTS = entry("prompts/list");
+const RESOURCES = entry("resources/list");
+const TEMPLATES = entry("resources/templates/list");
+
+describe("visibleMalformedListItems", () => {
+  it("shows every report in aggregate mode", () => {
+    const items = [TOOLS, PROMPTS, RESOURCES, TEMPLATES];
+    expect(visibleMalformedListItems(items, false)).toEqual(items);
+  });
+
+  it("returns the same array in aggregate mode rather than a copy", () => {
+    // Identity is load-bearing: the value feeds a memo, and a fresh array every
+    // render would re-render the panels for a filter that removed nothing.
+    const items = [TOOLS];
+    expect(visibleMalformedListItems(items, false)).toBe(items);
+  });
+
+  it("suppresses the paged lists' reports in paginated mode", () => {
+    // Those panels render the paged store, which neither writes nor clears this
+    // report — so the entry would describe a list that is no longer on screen.
+    expect(
+      visibleMalformedListItems([TOOLS, PROMPTS, RESOURCES], true),
+    ).toEqual([]);
+  });
+
+  it("keeps resource templates, which are aggregate-backed in both modes", () => {
+    // Templates have no paged store, so their report always describes the list
+    // actually being rendered.
+    expect(
+      visibleMalformedListItems([TOOLS, TEMPLATES, PROMPTS], true),
+    ).toEqual([TEMPLATES]);
+  });
+});

clients/web/src/utils/malformedListReport.ts

@@ -0,0 +1,46 @@
+import type { MalformedListItem } from "@inspector/core/mcp";
+
+/**
+ * Which malformed-entry reports the UI may show, given the pagination mode
+ * (#1909 + #1721).
+ *
+ * The report is produced by the salvage fallback inside the client's `listAll*`
+ * aggregate walk. In **paginated** mode the tools, prompts, and resources
+ * panels do not render that aggregate at all — they render the paged store,
+ * which fetches one page at a time through a different path that neither writes
+ * nor clears this report. So a report written while the list was aggregated
+ * would simply persist, sitting above a freshly fetched page it does not
+ * describe: the same warning over a page that may be perfectly conforming, with
+ * indices that refer to positions in an aggregate the user is no longer
+ * looking at.
+ *
+ * Suppressing is the honest option of the two available. The alternative —
+ * giving the paged path its own per-page salvage and report — is a real feature
+ * rather than a display fix, and showing a stale warning is worse than showing
+ * none: this panel's whole purpose is to say which entry the server got wrong,
+ * so pointing at the wrong entry costs more than staying quiet.
+ *
+ * Resource templates are deliberately NOT suppressed. That list has no paged
+ * store — it is fetched through the aggregate in both modes — so its report is
+ * always about the list actually on screen.
+ */
+const PAGED_LIST_METHODS: readonly string[] = [
+  "tools/list",
+  "prompts/list",
+  "resources/list",
+];
+
+/**
+ * The reports to render for the current pagination mode.
+ *
+ * Returns the input untouched in aggregate mode — the common case — so the
+ * memoized identity is preserved and nothing re-renders for a filter that
+ * removes nothing.
+ */
+export function visibleMalformedListItems(
+  items: MalformedListItem[],
+  paginated: boolean,
+): MalformedListItem[] {
+  if (!paginated) return items;
+  return items.filter((item) => !PAGED_LIST_METHODS.includes(item.method));
+}

core/mcp/listSalvage.ts

@@ -229,21 +229,9 @@ export function describeIssues(error: z.ZodError): string {
   /* v8 ignore next -- zod never produces a failed parse with zero issues; guards against an empty message if it ever did */
   if (!issue) return "did not match the expected shape";
   const path = issue.path.join(".");
-  const message = describeIssue(issue);
-  return path.length > 0 ? `${path}: ${message}` : message;
-}
-
-/**
- * One issue's message, with the expected type folded in when it adds something.
- *
- * Split out from {@link describeIssues} because {@link eraToolItemSchema} has to
- * apply it EARLY: forwarding another schema's issues re-emits them as `custom`
- * ones, which carry a message and a path but no `expected` field, so the detail
- * has to be baked into the message while the original issue is still in hand.
- */
-function describeIssue(issue: z.core.$ZodIssue): string {
   const detail = expectedDetail(issue);
-  return detail ? `${issue.message} (${detail})` : issue.message;
+  const message = detail ? `${issue.message} (${detail})` : issue.message;
+  return path.length > 0 ? `${path}: ${message}` : message;
 }
 
 /**
@@ -398,40 +386,29 @@ function stripExecution(tool: Tool): Tool {
 /**
  * Bridge a per-era WIRE schema to the `Tool` the rest of the client works with.
  *
- * The two are deliberately separate, because on BOTH eras the wire schema
- * accepts values the exported `Tool` type cannot describe — `properties: 5` is
- * a valid modern `inputSchema` keyword set, and the SDK hands exactly that back
- * typed as a `Tool`. So the era rules are expressed as an ordinary schema whose
- * output type is irrelevant (it is only ever consulted for a verdict), and
- * `z.custom<Tool>` supplies the type the caller needs. That is what avoids the
- * `as unknown as` this would otherwise require — and it is the same assertion
- * the SDK itself makes, in the same place.
+ * The bridge exists because the two cannot be the same schema: on BOTH eras the
+ * wire shape accepts values the exported `Tool` type cannot describe —
+ * `properties: 5` is a valid modern `inputSchema` keyword set, and the SDK
+ * hands exactly that back typed as a `Tool`. So the era rules are expressed as
+ * an ordinary schema, and its parsed output is asserted to `Tool` here.
  *
- * The wire schema's issues are copied across one by one, each keeping its PATH,
- * rather than collapsed into a single "invalid tool" — so `describeIssues` can
- * still name the offending field (`inputSchema.type`) instead of reporting a
- * bare "Invalid input", which is the whole point of the report this module
- * produces. They are re-emitted as `custom` issues (the only code that can
- * carry an arbitrary forwarded message), which is why the expected-type detail
- * is folded into the message here by {@link describeIssue} rather than left for
- * `describeIssues` to recover from a field the new issue will not have.
+ * The parsed OUTPUT is what gets normalized and returned, not the raw input.
+ * That matters: both era schemas are `z.object`, so parsing strips unknown
+ * top-level fields (and unknown keys inside `annotations`) exactly as the
+ * strict codec does. Validating with the wire schema but returning the original
+ * value would leave salvaged tools carrying fields the strict path drops, so
+ * the two paths would hand the UI differently-shaped tools for the same server.
+ *
+ * The assertion is single and deliberate — `parsed` is `unknown`, so this is
+ * the narrow `as` the double-cast rule asks for rather than an `as unknown as`
+ * — and it is the same claim the SDK makes about its own era-parsed results.
+ * Issues come straight from the wire parse, so they keep their field paths and
+ * `describeIssues` can still name `inputSchema.type` rather than reporting a
+ * bare "Invalid input".
  */
 function eraToolItemSchema(
   wire: z.ZodType,
   normalize: (tool: Tool) => Tool,
 ): z.ZodType<Tool> {
-  return z
-    .custom<Tool>()
-    .superRefine((value, ctx) => {
-      const parsed = wire.safeParse(value);
-      if (parsed.success) return;
-      for (const issue of parsed.error.issues) {
-        ctx.addIssue({
-          code: "custom",
-          path: issue.path,
-          message: describeIssue(issue),
-        });
-      }
-    })
-    .transform(normalize);
+  return wire.transform((parsed) => normalize(parsed as Tool));
 }