fix(web): validate the edited replay draft, not just its seed
Copilot's fifth pass: one comment, and the completion of the previous round's fix. Seeding the editor from `replayableParams` stopped it *offering* a key the dispatcher would drop, but nothing stopped one being typed in — adding `_meta` to a `tools/list` draft still let Send close the modal and discard it. `parseParamsDraft` now takes the method and rejects any key the dispatch would not read, so Send is disabled with the offending names rather than silently dropping them. It asks the same `replayableParams` the seed is built from, so the editor and the dispatcher cannot disagree about which keys survive. 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:
parent
5abe289caa
commit
4e7537ea76
2 changed files with 62 additions and 3 deletions
|
|
@ -136,6 +136,48 @@ describe("EditReplayModal", () => {
|
||||||
).toBeInTheDocument();
|
).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Seeding from the projection is only half of it: a key the dispatcher does
|
||||||
|
// not read can be *added* to the draft, and Send would then discard an edit
|
||||||
|
// the user was looking at.
|
||||||
|
it("disables Send for a key this method's dispatch would not read", async () => {
|
||||||
|
renderWithMantine(
|
||||||
|
<EditReplayModal {...baseProps} method="tools/list" params={undefined} />,
|
||||||
|
);
|
||||||
|
await setAceText('{"cursor":"abc","_meta":{"progressToken":1}}');
|
||||||
|
expect(screen.getByRole("button", { name: "Send" })).toBeDisabled();
|
||||||
|
expect(
|
||||||
|
screen.getByText(/`_meta` is not carried by tools\/list/),
|
||||||
|
).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("re-enables Send once the unsupported key is removed", async () => {
|
||||||
|
renderWithMantine(
|
||||||
|
<EditReplayModal {...baseProps} method="tools/list" params={undefined} />,
|
||||||
|
);
|
||||||
|
await setAceText('{"cursor":"abc","_meta":{}}');
|
||||||
|
expect(screen.getByRole("button", { name: "Send" })).toBeDisabled();
|
||||||
|
|
||||||
|
await setAceText('{"cursor":"abc"}');
|
||||||
|
expect(screen.getByRole("button", { name: "Send" })).not.toBeDisabled();
|
||||||
|
expect(screen.queryByText(/not carried by/)).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
// The check is per method, not a fixed key list: `arguments` is read by a
|
||||||
|
// tool call and by nothing on a list request.
|
||||||
|
it("judges a key against the method it would be sent to", async () => {
|
||||||
|
const { rerender } = renderWithMantine(
|
||||||
|
<EditReplayModal {...baseProps} method="tools/call" />,
|
||||||
|
);
|
||||||
|
await setAceText('{"name":"echo","arguments":{"a":1}}');
|
||||||
|
expect(screen.getByRole("button", { name: "Send" })).not.toBeDisabled();
|
||||||
|
|
||||||
|
rerender(<EditReplayModal {...baseProps} method="tools/list" />);
|
||||||
|
expect(screen.getByRole("button", { name: "Send" })).toBeDisabled();
|
||||||
|
expect(
|
||||||
|
screen.getByText(/`name`, `arguments` are not carried by tools\/list/),
|
||||||
|
).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
it("closes without sending when cancelled", async () => {
|
it("closes without sending when cancelled", async () => {
|
||||||
const user = userEvent.setup();
|
const user = userEvent.setup();
|
||||||
const onSend = vi.fn();
|
const onSend = vi.fn();
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ import { useState } from "react";
|
||||||
import { Alert, Button, Group, Modal, Stack, Text } from "@mantine/core";
|
import { Alert, Button, Group, Modal, Stack, Text } from "@mantine/core";
|
||||||
import { JsonEditor } from "../../elements/JsonEditor/JsonEditor";
|
import { JsonEditor } from "../../elements/JsonEditor/JsonEditor";
|
||||||
import { isJsonObject } from "../../../utils/jsonObjectDraft";
|
import { isJsonObject } from "../../../utils/jsonObjectDraft";
|
||||||
|
import { replayableParams } from "../../../lib/protocolReplay";
|
||||||
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
|
import { isSerializableJson } from "@inspector/core/json/jsonUtils.js";
|
||||||
|
|
||||||
export interface EditReplayModalProps {
|
export interface EditReplayModalProps {
|
||||||
|
|
@ -60,10 +61,12 @@ function formatDroppedKeys(keys: string[]): string {
|
||||||
* genuinely carries none, and an empty editor wearing a red error would read as
|
* 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
|
* 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
|
* all: JSON-RPC `params` must be a structured value, so `42` is a shape that
|
||||||
* cannot be sent however finished it is. A number that overflows is a third
|
* cannot be sent however finished it is. A number that overflows, and a key
|
||||||
* case, and the subtlest — see the note on that branch.
|
* this method's dispatch does not read, are the two subtle cases — see the
|
||||||
|
* notes on those branches.
|
||||||
*/
|
*/
|
||||||
function parseParamsDraft(
|
function parseParamsDraft(
|
||||||
|
method: string,
|
||||||
text: string,
|
text: string,
|
||||||
):
|
):
|
||||||
| { ok: true; value: Record<string, unknown> | null }
|
| { ok: true; value: Record<string, unknown> | null }
|
||||||
|
|
@ -89,6 +92,20 @@ function parseParamsDraft(
|
||||||
"Numbers must be finite — a value like `1e400` overflows and would be sent as null",
|
"Numbers must be finite — a value like `1e400` overflows and would be sent as null",
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
// 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
|
||||||
|
// built from, so the editor and the dispatcher cannot disagree about which
|
||||||
|
// keys survive.
|
||||||
|
const { dropped } = replayableParams(method, parsed);
|
||||||
|
if (dropped.length > 0) {
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
error: `${formatDroppedKeys(dropped)} not carried by ${method} — remove ${
|
||||||
|
dropped.length === 1 ? "it" : "them"
|
||||||
|
} so this matches what is sent`,
|
||||||
|
};
|
||||||
|
}
|
||||||
return { ok: true, value: parsed };
|
return { ok: true, value: parsed };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -107,7 +124,7 @@ function EditReplayForm({
|
||||||
const [draft, setDraft] = useState(() =>
|
const [draft, setDraft] = useState(() =>
|
||||||
params === undefined ? "" : JSON.stringify(params, null, 2),
|
params === undefined ? "" : JSON.stringify(params, null, 2),
|
||||||
);
|
);
|
||||||
const parsed = parseParamsDraft(draft);
|
const parsed = parseParamsDraft(method, draft);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Stack gap="sm">
|
<Stack gap="sm">
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue