fix(web): reject drafts whose digits or types would not survive being sent
Copilot's seventh pass, three comments, all the same class: the editor showing
one value while the wire carries another.
- `JSON.parse` rounds a whole number past 2^53−1 to the nearest double, so a
draft reading `{"id":9007199254740993}` submits `…992`. New
`hasRoundedInteger`, applied by all four JSON-draft parsers — the raw
arguments editor, `SchemaJsonField`, the Edit-and-replay modal, and
`parseJsonObjectDraft`, which Copilot did not flag but shares the defect.
It is narrower than "unsafe integer": it also requires the number's shortest
form to be written out in full, because JS stringifies from 1e21 upward in
exponent form and such a value was typed that way — `{"n":1e308}` is a
legitimate `_meta` value that `parseJsonObjectDraft`'s tests already pin as
accepted. It still refuses the rare full-form integer that happens to be
exactly representable, erring toward refusing a sendable value rather than
misreporting an unsendable one — the trade `toNumericValue` already makes.
- `getPrompt` runs `convertPromptArguments`, which JSON-stringifies every
non-string argument, so `{"count":2}` leaves as `{"count":"2"}` while the
editor still shows a number. `reshapedReplayParam` now rejects a non-string
prompt argument. Not a workaround: the spec types
`GetPromptRequest.params.arguments` as `Record<string, string>`, so a string
is the only thing a prompt argument can be. `tools/call` is untouched —
`callTool` sends its arguments as given.
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
0e9069a78d
commit
8f4aa7f8af
2 changed files with 216 additions and 13 deletions
|
|
@ -167,6 +167,38 @@ describe("reshapedReplayParam", () => {
|
|||
).toMatch(/must be a JSON object/);
|
||||
});
|
||||
|
||||
// `getPrompt` runs `convertPromptArguments`, which JSON-stringifies anything
|
||||
// that is not already a string — so `{count: 2}` goes out as `{count: "2"}`
|
||||
// while the editor still shows the number. The spec types a prompt argument
|
||||
// as a string, so this is the protocol, not a quirk to route around.
|
||||
it("rejects a non-string prompt argument, which is stringified on the way out", () => {
|
||||
expect(
|
||||
reshapedReplayParam("prompts/get", {
|
||||
name: "greet",
|
||||
arguments: { count: 2, who: "ada" },
|
||||
}),
|
||||
).toMatch(/`count` would be sent as JSON text/);
|
||||
});
|
||||
|
||||
it("accepts string prompt arguments", () => {
|
||||
expect(
|
||||
reshapedReplayParam("prompts/get", {
|
||||
name: "greet",
|
||||
arguments: { count: "2", who: "ada" },
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
// `callTool` sends its arguments as given, so the same shape is fine there.
|
||||
it("leaves a tool call's non-string arguments alone", () => {
|
||||
expect(
|
||||
reshapedReplayParam("tools/call", {
|
||||
name: "add",
|
||||
arguments: { a: 1, b: 2 },
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("has nothing to say about methods that take no arguments", () => {
|
||||
expect(reshapedReplayParam("tools/list", { cursor: "a" })).toBeNull();
|
||||
expect(reshapedReplayParam("resources/read", { uri: "x://y" })).toBeNull();
|
||||
|
|
|
|||
|
|
@ -138,8 +138,8 @@ export function replayableParams(
|
|||
* kept param is sent as written.
|
||||
*
|
||||
* {@link replayableParams} answers which keys survive; this answers whether the
|
||||
* surviving ones survive *intact*. Only `arguments` can fail that today, and it
|
||||
* fails two ways, both of which the Edit-and-replay editor has to catch before
|
||||
* surviving ones survive *intact*. Only `arguments` can fail that, and it fails
|
||||
* three ways, each of which the Edit-and-replay editor has to catch before
|
||||
* Send:
|
||||
*
|
||||
* - `arguments: null` is nullish, so `?? {}` replaces it — the editor shows
|
||||
|
|
@ -147,6 +147,13 @@ export function replayableParams(
|
|||
* - `arguments: [1,2]` or `arguments: 4` is not nullish, so the cast carries it
|
||||
* through unchanged into a call whose type says it is a named-argument
|
||||
* record. Nothing reshapes it, but nothing can send it either.
|
||||
* - For **`prompts/get` only**, a non-string *value*. `getPrompt` runs
|
||||
* `convertPromptArguments`, which `JSON.stringify`s anything that is not
|
||||
* already a string — so `{"count": 2}` is sent as `{"count": "2"}` while the
|
||||
* editor still shows the number. This is not a quirk to route around: the
|
||||
* spec types `GetPromptRequest.params.arguments` as `Record<string, string>`,
|
||||
* so a string is the only thing a prompt argument can be. `tools/call` is
|
||||
* unaffected — `callTool` sends its arguments as given.
|
||||
*
|
||||
* `name` and `uri` are deliberately not checked here: the dispatch already
|
||||
* refuses a missing or non-string one with a reason the caller surfaces as a
|
||||
|
|
@ -162,10 +169,20 @@ export function reshapedReplayParam(
|
|||
const args = params.arguments;
|
||||
const isRecord =
|
||||
args !== null && typeof args === "object" && !Array.isArray(args);
|
||||
if (isRecord) return null;
|
||||
return args === null
|
||||
? "`arguments` is sent as `{}` when null — remove it, or give it an object"
|
||||
: "`arguments` must be a JSON object (`{ … }`)";
|
||||
if (!isRecord) {
|
||||
return args === null
|
||||
? "`arguments` is sent as `{}` when null — remove it, or give it an object"
|
||||
: "`arguments` must be a JSON object (`{ … }`)";
|
||||
}
|
||||
if (method === "prompts/get") {
|
||||
const coerced = Object.entries(args as Record<string, unknown>)
|
||||
.filter(([, value]) => typeof value !== "string")
|
||||
.map(([key]) => `\`${key}\``);
|
||||
if (coerced.length > 0) {
|
||||
return `A prompt argument is always a string — ${coerced.join(", ")} would be sent as JSON text`;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export async function replayProtocolRequest(
|
||||
|
|
|
|||
Loading…
Reference in a new issue