fix(web): detect duplicate object keys, which JSON.parse drops silently

Copilot's eleventh pass: three comments, one defect in three places.

`JSON.parse` accepts duplicate member names and keeps the last without
complaint, so `{"role":"user","role":"admin"}` becomes `{"role":"admin"}` —
a document that renders, and submits, as *less* than it says. In the read-only
viewer that is the Inspector hiding what the server sent; in a draft it is the
editor showing more than the wire will carry. Nothing downstream could catch
it, because by the time `JSON.parse` returns the evidence is gone.

New `findDuplicateObjectKey` scans the source, scoped per object so
`{"a":{"a":1}}` and `[{"a":1},{"a":2}]` are fine, and compares names by decoded
value so `"a"` and `"a"` collide as they should. It shares its
escape-handling with the number-literal scanner, which now reads strings
through the same helper rather than skipping them inline.

Wired into all four JSON-draft parsers and into the viewer's faithfulness
check, so the same document is refused, or shown unindented, everywhere.

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

clients/web/src/components/elements/ContentViewer/contentViewerUtils.test.ts

@@ -116,6 +116,15 @@ describe("formatJson", () => {
     expect(formatJson('{"limit":1e400}')).toBe('{"limit":1e400}');
   });
 
+  // The starkest instance of the same class: only the *second* `role` survives
+  // `JSON.parse`, so reindenting would show the reader less than the server
+  // sent.
+  it("leaves a payload alone when it names the same member twice", () => {
+    expect(formatJson('{"role":"user","role":"admin"}')).toBe(
+      '{"role":"user","role":"admin"}',
+    );
+  });
+
   // The spelling of a number changes on a round trip and its *value* does not,
   // so these still get indented: `0.10000000000000001` and `0.1` are the same
   // double, and `1e2` and `100` are the same number.

clients/web/src/components/elements/ContentViewer/contentViewerUtils.ts

@@ -2,12 +2,15 @@
  * Pure helpers backing the per-MIME dispatch in {@link ContentViewer}. Kept
  * free of component dependencies so they can be unit-tested in isolation and
  * reused by the blob renderers (PDF / CSV / HTML) without dragging in React.
- * The two pure predicates imported below are the exception that proves it:
- * plain functions over a string, shared so the viewer and the JSON editors
- * cannot disagree about which payloads survive a round trip.
+ * The predicates imported below are the exception that proves it: plain
+ * functions over a string, shared so the viewer and the JSON editors cannot
+ * disagree about which payloads survive a round trip.
  */
 import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
-import { hasImpreciseIntegerLiteral } from "../../../utils/jsonObjectDraft";
+import {
+  findDuplicateObjectKey,
+  hasImpreciseIntegerLiteral,
+} from "../../../utils/jsonObjectDraft";
 
 /**
  * The renderer family a MIME type maps to. `ContentViewer` switches on this to
@@ -160,11 +163,18 @@ export function formatJson(content: string): string {
  * as it arrived — unindented, but honest. The copy button already copied the
  * original text either way; this makes what is on screen agree with it.
  *
- * The same two checks the draft parsers use, which is why they live in
+ * The same checks the draft parsers use, which is why they live in
  * `utils/jsonObjectDraft` rather than beside either caller.
  */
 function reformatsFaithfully(text: string, parsed: unknown): boolean {
-  return isSerializableJson(parsed) && !hasImpreciseIntegerLiteral(text);
+  return (
+    isSerializableJson(parsed) &&
+    !hasImpreciseIntegerLiteral(text) &&
+    // A duplicate member name is the same class and the starkest instance:
+    // `{"role":"user","role":"admin"}` would be *shown* as only the second,
+    // which is the viewer hiding what the server sent.
+    findDuplicateObjectKey(text) === null
+  );
 }
 
 /** Heuristic: does this plain text (no MIME) look like a JSON document? */

clients/web/src/components/groups/EditReplayModal/EditReplayModal.test.tsx

@@ -265,6 +265,15 @@ describe("EditReplayModal", () => {
     expect(screen.getByRole("button", { name: "Send" })).not.toBeDisabled();
   });
 
+  // `JSON.parse` keeps only the last occurrence, so the draft would show two
+  // cursors and Send would dispatch one.
+  it("disables Send for a document that names the same member twice", async () => {
+    renderWithMantine(<EditReplayModal {...baseProps} method="tools/list" />);
+    await setAceText('{"cursor":"first","cursor":"second"}');
+    expect(screen.getByRole("button", { name: "Send" })).toBeDisabled();
+    expect(screen.getByText(/`cursor` appears twice/)).toBeInTheDocument();
+  });
+
   it("closes without sending when cancelled", async () => {
     const user = userEvent.setup();
     const onSend = vi.fn();

clients/web/src/components/groups/EditReplayModal/EditReplayModal.tsx

@@ -3,6 +3,8 @@ import type { Tool } from "@modelcontextprotocol/client";
 import { Alert, Button, Group, Modal, Stack, Text } from "@mantine/core";
 import { JsonEditor } from "../../elements/JsonEditor/JsonEditor";
 import {
+  duplicateKeyError,
+  findDuplicateObjectKey,
   hasImpreciseIntegerLiteral,
   isJsonObject,
   IMPRECISE_INTEGER_ERROR,
@@ -114,6 +116,10 @@ function parseParamsDraft(
   if (hasImpreciseIntegerLiteral(text)) {
     return { ok: false, error: IMPRECISE_INTEGER_ERROR };
   }
+  const duplicate = findDuplicateObjectKey(text);
+  if (duplicate !== null) {
+    return { ok: false, error: duplicateKeyError(duplicate) };
+  }
   // Seeding from the projection is only half of it: a key the dispatcher does
   // not read can also be *added* to the draft, and Send would then discard an
   // edit the user was looking at. Checked with the same function the seed is

clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx

@@ -792,6 +792,9 @@ describe("SchemaForm", () => {
       screen.getByText(/^This whole number cannot be represented exactly/),
     ).toBeInTheDocument();
 
+    await setAceTextByLabel(/Config/, '[{"a":1,"a":2}]');
+    expect(screen.getByText(/^`a` appears twice/)).toBeInTheDocument();
+
     await setAceTextByLabel(/Config/, "[1,2]");
     expect(screen.queryByText(/this field will be omitted/)).toBeNull();
   });
@@ -1978,6 +1981,22 @@ describe("SchemaForm raw JSON (#2151)", () => {
     ).toBeInTheDocument();
   });
 
+  // `JSON.parse` keeps only the last occurrence, so the editor would show a
+  // document that submits as less than it says.
+  it("blocks submission for a document that names the same member twice", async () => {
+    const user = userEvent.setup();
+    const onValidityChange = vi.fn();
+    renderWithMantine(<RawHarness onValidityChange={onValidityChange} />);
+    await enableRawJson(user);
+
+    await setAceTextByLabel(
+      /Arguments JSON/,
+      '{"mode":"safe","mode":"unsafe"}',
+    );
+    expect(onValidityChange).toHaveBeenLastCalledWith(true);
+    expect(screen.getByText(/`mode` appears twice/)).toBeInTheDocument();
+  });
+
   // Nothing in `values` names a branch, so the picker's index is held by the
   // form. Editing the discriminator in the raw document is a change to which
   // branch is in effect, and it carries no other signal — without the re-derive

clients/web/src/components/groups/SchemaForm/SchemaForm.tsx

@@ -25,6 +25,8 @@ import { ClearButton } from "../../elements/ClearButton/ClearButton";
 import { EnlargeButton } from "../../elements/EnlargeButton/EnlargeButton";
 import { JsonEditor } from "../../elements/JsonEditor/JsonEditor";
 import {
+  duplicateKeyError,
+  findDuplicateObjectKey,
   hasImpreciseIntegerLiteral,
   isJsonObject,
   IMPRECISE_INTEGER_ERROR,
@@ -332,6 +334,10 @@ function describeJsonDraftProblem(text: string): string | null {
   if (hasImpreciseIntegerLiteral(text)) {
     return `${IMPRECISE_INTEGER_ERROR} — this field will be omitted`;
   }
+  const duplicate = findDuplicateObjectKey(text);
+  if (duplicate !== null) {
+    return `${duplicateKeyError(duplicate)} — this field will be omitted`;
+  }
   return null;
 }
 
@@ -649,6 +655,10 @@ function parseRawArgumentsDraft(
   if (hasImpreciseIntegerLiteral(text)) {
     return { ok: false, error: IMPRECISE_INTEGER_ERROR };
   }
+  const duplicate = findDuplicateObjectKey(text);
+  if (duplicate !== null) {
+    return { ok: false, error: duplicateKeyError(duplicate) };
+  }
   return { ok: true, value: parsed };
 }
 

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

@@ -1,5 +1,6 @@
 import { describe, it, expect } from "vitest";
 import {
+  findDuplicateObjectKey,
   hasImpreciseIntegerLiteral,
   parseJsonObjectDraft,
 } from "./jsonObjectDraft";
@@ -70,6 +71,51 @@ describe("hasImpreciseIntegerLiteral", () => {
   });
 });
 
+// `JSON.parse` accepts duplicate member names and keeps the last silently, so
+// the evidence is gone by the time it returns — nothing downstream can see that
+// the document said more than the value carries.
+describe("findDuplicateObjectKey", () => {
+  it("names the repeated member", () => {
+    expect(findDuplicateObjectKey('{"role":"user","role":"admin"}')).toBe(
+      "role",
+    );
+  });
+
+  it("accepts a document whose names are all distinct", () => {
+    expect(findDuplicateObjectKey('{"a":1,"b":2}')).toBeNull();
+  });
+
+  // The repeat has to be within one set of braces.
+  it("scopes the check per object", () => {
+    expect(findDuplicateObjectKey('{"a":{"a":1}}')).toBeNull();
+    expect(findDuplicateObjectKey('[{"a":1},{"a":2}]')).toBeNull();
+  });
+
+  it("finds a repeat nested inside another object or an array", () => {
+    expect(findDuplicateObjectKey('{"outer":{"a":1,"a":2}}')).toBe("a");
+    expect(findDuplicateObjectKey('[{"a":1,"a":2}]')).toBe("a");
+  });
+
+  // Compared by decoded value: `"a"` and `"\u0061"` name the same member, so
+  // comparing the raw source would miss a duplicate written the second way.
+  it("compares names by value, not by spelling", () => {
+    expect(findDuplicateObjectKey('{"a":1,"\\u0061":2}')).toBe("a");
+  });
+
+  // A string *value* that repeats is not a duplicate name, and a name that
+  // merely appears inside one is not either.
+  it("does not confuse values with names", () => {
+    expect(findDuplicateObjectKey('{"a":"x","b":"x"}')).toBeNull();
+    expect(findDuplicateObjectKey('{"a":"b","c":1}')).toBeNull();
+    expect(findDuplicateObjectKey('{"a":"}\\"a\\":1{","b":2}')).toBeNull();
+  });
+
+  it("says nothing about a half-typed draft", () => {
+    expect(findDuplicateObjectKey('{"a":1,"a')).toBeNull();
+    expect(findDuplicateObjectKey("")).toBeNull();
+  });
+});
+
 describe("parseJsonObjectDraft", () => {
   it("reads empty text as the empty object rather than an error", () => {
     expect(parseJsonObjectDraft("")).toEqual({ ok: true, value: {} });
@@ -112,6 +158,12 @@ describe("parseJsonObjectDraft", () => {
     });
   });
 
+  it("rejects a document that names the same member twice", () => {
+    const result = parseJsonObjectDraft('{"role":"user","role":"admin"}');
+    expect(result.ok).toBe(false);
+    expect(result.ok === false && result.error).toMatch(/`role` appears twice/);
+  });
+
   it("rejects text that is not JSON", () => {
     expect(parseJsonObjectDraft('{"a":')).toEqual({
       ok: false,

clients/web/src/utils/jsonObjectDraft.ts

@@ -67,18 +67,10 @@ const JSON_NUMBER = /-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?/y;
  */
 function* jsonNumberLiterals(text: string): Generator<string> {
   let index = 0;
-  let inString = false;
   while (index < text.length) {
     const char = text[index];
-    if (inString) {
-      // A backslash escapes the next character, including a closing quote.
-      index += char === "\\" ? 2 : 1;
-      if (char === '"') inString = false;
-      continue;
-    }
     if (char === '"') {
-      inString = true;
-      index += 1;
+      index = readStringToken(text, index).next;
       continue;
     }
     if (char === "-" || (char >= "0" && char <= "9")) {
@@ -95,6 +87,97 @@ function* jsonNumberLiterals(text: string): Generator<string> {
   }
 }
 
+/**
+ * Read one JSON string token starting at the opening quote.
+ *
+ * Returns where the token ends and its raw source, escapes intact — the caller
+ * decodes only when it needs the *value*, which for a key it does: `"a"` and
+ * `"\u0061"` name the same member, so comparing the raw forms would miss a
+ * duplicate written the second way.
+ *
+ * Shared by both scanners here so the escape handling — a backslash consumes
+ * the next character, including a closing quote — is written once.
+ */
+function readStringToken(
+  text: string,
+  start: number,
+): { raw: string; next: number } {
+  let index = start + 1;
+  while (index < text.length) {
+    const char = text[index];
+    if (char === "\\") {
+      index += 2;
+      continue;
+    }
+    index += 1;
+    if (char === '"') break;
+  }
+  return { raw: text.slice(start, index), next: index };
+}
+
+/**
+ * The first object member name this document declares twice, or `null`.
+ *
+ * `JSON.parse` accepts duplicate names and keeps the last silently, so
+ * `{"role":"user","role":"admin"}` becomes `{"role":"admin"}` — a document that
+ * renders, and submits, as *less* than it says. In a read-only view that is the
+ * Inspector hiding what the server sent; in a draft it is the editor showing
+ * more than the wire will carry. Neither is acceptable, and nothing else here
+ * detects it, because by the time `JSON.parse` returns the evidence is gone.
+ *
+ * Scoped per object, so `{"a":{"a":1}}` and `[{"a":1},{"a":2}]` are fine — the
+ * repeat has to be within one set of braces.
+ */
+export function findDuplicateObjectKey(text: string): string | null {
+  // One frame per open brace or bracket; `null` marks an array, whose members
+  // are not named and so cannot collide.
+  const stack: (Set<string> | null)[] = [];
+  let expectKey = false;
+  let index = 0;
+  while (index < text.length) {
+    const char = text[index];
+    if (char === '"') {
+      const { raw, next } = readStringToken(text, index);
+      const frame = stack[stack.length - 1];
+      if (expectKey && frame) {
+        let name: string;
+        try {
+          name = JSON.parse(raw) as string;
+        } catch {
+          // An unterminated string at the end of a half-typed draft. There is
+          // no key here to compare, and the parse will report the real problem.
+          return null;
+        }
+        if (frame.has(name)) return name;
+        frame.add(name);
+        expectKey = false;
+      }
+      index = next;
+      continue;
+    }
+    if (char === "{") {
+      stack.push(new Set());
+      expectKey = true;
+    } else if (char === "[") {
+      stack.push(null);
+      expectKey = false;
+    } else if (char === "}" || char === "]") {
+      stack.pop();
+      expectKey = false;
+    } else if (char === ",") {
+      // Only an object's separator introduces a name; an array's does not.
+      expectKey = stack[stack.length - 1] != null;
+    }
+    index += 1;
+  }
+  return null;
+}
+
+/** The message every draft parser uses for {@link findDuplicateObjectKey}. */
+export function duplicateKeyError(key: string): string {
+  return `\`${key}\` appears twice in the same object — JSON keeps only the last, so part of this would not be sent`;
+}
+
 /**
  * The message every draft parser uses for {@link hasImpreciseIntegerLiteral}.
  *
@@ -148,5 +231,9 @@ export function parseJsonObjectDraft(text: string): JsonObjectDraft {
   if (hasImpreciseIntegerLiteral(text)) {
     return { ok: false, error: IMPRECISE_INTEGER_ERROR };
   }
+  const duplicate = findDuplicateObjectKey(text);
+  if (duplicate !== null) {
+    return { ok: false, error: duplicateKeyError(duplicate) };
+  }
   return { ok: true, value: parsed };
 }