fix(web): reject non-finite numbers, and keep the branch picker with the raw draft

Two more from Copilot's second pass, both real.

- Every JSON-draft parser now rejects a value `JSON.stringify` cannot write
  back. `JSON.parse("1e400")` yields `Infinity`, which serializes as `null` —
  so an editor showing `1e400` would have sent `null`, which is exactly the
  misreport `isSerializableJson` was written for. `parseJsonObjectDraft`
  already guarded; the raw-arguments editor and the Edit-and-replay modal did
  not, and neither did `SchemaJsonField`'s `parseJsonDraft` — the last
  predates this PR but is the same defect in the same family, so it is fixed
  with them rather than left as the odd one out.

- Editing a root union's discriminator in the raw document now moves the
  Variant picker. Nothing in `values` names a branch, so the index is held by
  the form and was only re-derived on `resetKey` or a rewritten union; a raw
  edit had no other signal, so `{"kind":"email"}` → `{"kind":"sms",…}` left
  the picker on Email and switching back to the widgets rendered the Email
  branch over SMS values. The current branch is kept when the values identify
  none, since the discriminator is absent every time it is retyped.

The raw editor is keyed by entity only, not by entity-and-branch like the
per-field editors: it holds the whole arguments object, so a branch move must
re-sync it rather than remount it — a remount would reformat the text being
typed and drop the caret to the top.

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>
This commit is contained in:
cliffhall 2026-08-27 03:01:47 -04:00
commit b4083ee5fb
4 changed files with 218 additions and 8 deletions

View file

@ -88,6 +88,16 @@ describe("EditReplayModal", () => {
expect(screen.getByText(/must be a JSON object/i)).toBeInTheDocument();
});
// `JSON.parse("1e400")` yields `Infinity`, which `JSON.stringify` writes back
// as `null` — so Send would dispatch a number the user never typed while the
// editor still showed what they wrote.
it("disables Send for a number that cannot survive being sent", async () => {
renderWithMantine(<EditReplayModal {...baseProps} />);
await setAceText('{"limit":1e400}');
expect(screen.getByRole("button", { name: "Send" })).toBeDisabled();
expect(screen.getByText(/Numbers must be finite/)).toBeInTheDocument();
});
it("closes without sending when cancelled", async () => {
const user = userEvent.setup();
const onSend = vi.fn();

View file

@ -2,6 +2,7 @@ import { useState } from "react";
import { Button, Group, Modal, Stack, Text } from "@mantine/core";
import { JsonEditor } from "../../elements/JsonEditor/JsonEditor";
import { isJsonObject } from "../../../utils/jsonObjectDraft";
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
export interface EditReplayModalProps {
opened: boolean;
@ -34,7 +35,8 @@ const Footer = Group.withProps({ justify: "flex-end", gap: "sm" });
* genuinely carries none, and an empty editor wearing a red error would read as
* broken. A non-object is reported separately from text that is not JSON at
* all: JSON-RPC `params` must be a structured value, so `42` is a shape that
* cannot be sent however finished it is.
* cannot be sent however finished it is. A number that overflows is a third
* case, and the subtlest — see the note on that branch.
*/
function parseParamsDraft(
text: string,
@ -51,6 +53,17 @@ function parseParamsDraft(
if (!isJsonObject(parsed)) {
return { ok: false, error: "Params must be a JSON object (`{ … }`)" };
}
// `JSON.parse` accepts numeric literals it cannot represent: `1e400` parses to
// `Infinity`, which `JSON.stringify` then writes as `null`. Accepting it here
// would show the user one value and send another — the one thing an inspector
// must not do. Same guard, same reason, as `parseJsonObjectDraft`.
if (!isSerializableJson(parsed)) {
return {
ok: false,
error:
"Numbers must be finite — a value like `1e400` overflows and would be sent as null",
};
}
return { ok: true, value: parsed };
}

View file

@ -1899,6 +1899,143 @@ describe("SchemaForm raw JSON (#2151)", () => {
expect(screen.queryByText(/Not valid JSON/)).toBeNull();
});
// `JSON.parse("1e400")` yields `Infinity`, which `JSON.stringify` writes back
// as `null`. Submitting it would send a number the user never typed while the
// editor still showed what they wrote.
it("blocks submission for a number that cannot survive being sent", async () => {
const user = userEvent.setup();
const onValidityChange = vi.fn();
renderWithMantine(<RawHarness onValidityChange={onValidityChange} />);
await enableRawJson(user);
await setAceTextByLabel(/Arguments JSON/, '{"count":1e400}');
expect(onValidityChange).toHaveBeenLastCalledWith(true);
expect(screen.getByText(/Numbers must be finite/)).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
// the picker keeps showing the outgoing branch over the incoming values.
it("moves the branch picker when the raw document changes the discriminator", async () => {
const user = userEvent.setup();
const union: InspectorFormSchema = {
type: "object",
anyOf: [
{
type: "object",
title: "Email",
properties: {
kind: { type: "string", const: "email" },
address: { type: "string", title: "Address" },
},
required: ["kind"],
},
{
type: "object",
title: "SMS",
properties: {
kind: { type: "string", const: "sms" },
phone: { type: "string", title: "Phone" },
},
required: ["kind"],
},
],
};
renderWithMantine(
<RawHarness schema={union} initial={{ kind: "email" }} />,
);
expect(
(screen.getByRole("textbox", { name: /Variant/ }) as HTMLInputElement)
.value,
).toBe("Email");
await enableRawJson(user);
await setAceTextByLabel(
/Arguments JSON/,
'{"kind":"sms","phone":"555-0100"}',
);
expect(
(screen.getByRole("textbox", { name: /Variant/ }) as HTMLInputElement)
.value,
).toBe("SMS");
// And the widgets agree with it on the way back.
await user.click(screen.getByLabelText("Edit as JSON"));
// Anchored: the branch picker's own options carry these words too.
expect(screen.getByLabelText(/^Phone$/)).toBeInTheDocument();
expect(screen.queryByLabelText(/^Address$/)).toBeNull();
});
// The editor holds the whole arguments object, not a per-branch slice, so a
// branch move must not remount it — that would reformat the text being typed
// and drop the caret to the top. Pinned by the text surviving verbatim: a
// remount reseeds from `values`, which is pretty-printed.
it("does not reformat the draft when the branch moves under it", async () => {
const user = userEvent.setup();
const union: InspectorFormSchema = {
type: "object",
anyOf: [
{
type: "object",
title: "Email",
properties: { kind: { type: "string", const: "email" } },
required: ["kind"],
},
{
type: "object",
title: "SMS",
properties: { kind: { type: "string", const: "sms" } },
required: ["kind"],
},
],
};
renderWithMantine(
<RawHarness schema={union} initial={{ kind: "email" }} />,
);
await enableRawJson(user);
await setAceTextByLabel(/Arguments JSON/, '{"kind":"sms"}');
expect(getAceTextByLabel(/Arguments JSON/)).toBe('{"kind":"sms"}');
});
// The discriminator is cleared and retyped on the way between branches, so
// snapping to the first branch the moment it stops matching would move the
// picker under the user mid-edit.
it("keeps the current branch while the discriminator is absent", async () => {
const user = userEvent.setup();
const union: InspectorFormSchema = {
type: "object",
anyOf: [
{
type: "object",
title: "Email",
properties: { kind: { type: "string", const: "email" } },
required: ["kind"],
},
{
type: "object",
title: "SMS",
properties: { kind: { type: "string", const: "sms" } },
required: ["kind"],
},
],
};
renderWithMantine(<RawHarness schema={union} initial={{ kind: "sms" }} />);
await enableRawJson(user);
expect(
(screen.getByRole("textbox", { name: /Variant/ }) as HTMLInputElement)
.value,
).toBe("SMS");
await setAceTextByLabel(/Arguments JSON/, '{"note":"mid-edit"}');
expect(
(screen.getByRole("textbox", { name: /Variant/ }) as HTMLInputElement)
.value,
).toBe("SMS");
});
// #2123: switching a root union drops the outgoing branch's values. The raw
// editor is seeded from what the form holds, so a round trip through it must
// not resurrect them.

View file

@ -25,6 +25,7 @@ import { ClearButton } from "../../elements/ClearButton/ClearButton";
import { EnlargeButton } from "../../elements/EnlargeButton/EnlargeButton";
import { JsonEditor } from "../../elements/JsonEditor/JsonEditor";
import { isJsonObject } from "../../../utils/jsonObjectDraft";
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
import { useValueChange } from "../../../hooks/useValueChange";
import type {
InspectorFormSchema,
@ -272,16 +273,29 @@ function toNumericValue(raw: string | number): number | undefined {
return Number.isSafeInteger(Math.trunc(parsed)) ? parsed : undefined;
}
/** Parse editor text, reporting `undefined` for anything that is not JSON yet. */
/**
* Parse editor text, reporting `undefined` for anything this client cannot
* send.
*
* That covers text that is not JSON *yet* — the mid-edit state this field's
* whole draft/value split exists for — and text that parses but does not
* survive being written back out: `JSON.parse("1e400")` yields `Infinity`,
* which `JSON.stringify` writes as `null`. Both report no value, so the field
* shows its error and `onValidityChange` blocks submission, rather than sending
* a number the user never typed. Same guard, same reason, as
* `parseJsonObjectDraft` and the raw-arguments editor above.
*/
function parseJsonDraft(text: string): unknown {
if (text.trim() === "") {
return undefined;
}
let parsed: unknown;
try {
return JSON.parse(text);
parsed = JSON.parse(text);
} catch {
return undefined;
}
return isSerializableJson(parsed) ? parsed : undefined;
}
/** Structural equality for the draft/value re-sync, via canonical JSON. */
@ -584,6 +598,17 @@ function parseRawArgumentsDraft(
error: "Arguments must be a JSON object (`{ … }`)",
};
}
// `JSON.parse` accepts numeric literals it cannot represent: `1e400` parses to
// `Infinity`, which `JSON.stringify` then writes as `null`. Accepting it here
// would show the user one value and send another — the one thing an inspector
// must not do. Same guard, same reason, as `parseJsonObjectDraft`.
if (!isSerializableJson(parsed)) {
return {
ok: false,
error:
"Numbers must be finite — a value like `1e400` overflows and would be sent as null",
};
}
return { ok: true, value: parsed };
}
@ -942,6 +967,27 @@ export function SchemaForm({
onChange({ ...values, [fieldName]: fieldValue });
}
/**
* Take the raw editor's object, and move the branch picker with it.
*
* Nothing in `values` names a branch, so the picker's index is held here and
* is only ever re-derived when something says the values changed underneath
* (`resetKey`, a rewritten union). Editing the discriminator in the raw
* document is exactly that, and it has no other signal: without this, turning
* `{"kind":"email"}` into `{"kind":"sms",…}` leaves the picker on Email, and
* switching back to the widgets renders the Email branch over SMS values —
* showing one shape while submitting another.
*
* The current branch is kept when the values identify none, rather than
* snapping to the first: the user passes through that state every time they
* clear the discriminator to retype it.
*/
function handleRawArgumentsChange(next: Record<string, unknown>) {
onChange(next);
if (branches.length === 0) return;
setBranchIndex(selectBranchIndex(branches, next) ?? branchIndex);
}
/**
* Switch branches, and move `values` with the form.
*
@ -1413,13 +1459,17 @@ export function SchemaForm({
/>
)}
{rawJsonMode ? (
// Keyed by the entity and branch, like every draft-holding field: text
// typed for one tool must not be left in the box for the next, and
// switching a root union prunes values the stale draft would restore.
// Keyed by the *entity* only, unlike the per-field editors, which are
// keyed by entity and branch. This editor holds the whole arguments
// object, which is not a per-branch thing: switching branches rewrites
// `values`, and the re-sync below picks that up without a remount.
// Keying it by branch too would remount it the moment an edit to the
// discriminator moved the picker — reformatting the text the user was
// typing and dropping their caret to the top.
<RawArgumentsField
key={draftKey ?? "raw-json"}
key={resetKey ?? "raw-json"}
values={values}
onChange={onChange}
onChange={handleRawArgumentsChange}
disabled={disabled}
onInvalidChange={reportRawJsonValidity}
/>