From 059e1b5550123f292730bc7ac1b66aafa3819d82 Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Wed, 11 Nov 2026 21:27:16 -0600 Subject: [PATCH 01/11] fix(mcp): follow the stdio and 2026-07-28 rules clients check Results of server/discover and the list and read methods carry ttlMs and cacheScope. A request cancelled with notifications/cancelled gets no reply, also when a synchronous read held the event loop while the cancel arrived. A cursor on tools/list, prompts/list or resources/templates/list is -32602, a request with a null id gets -32600 instead of no answer, a 2025-03-26 session accepts batches, and error text that quotes a huge argument is cut to 4,000 characters around the middle. --- packages/coding.sh/src/mcp/protocol.ts | 128 +++++++-- .../coding.sh/test/mcp/_blocking-server.ts | 29 +++ packages/coding.sh/test/mcp/protocol.test.ts | 243 ++++++++++++++++++ 3 files changed, 376 insertions(+), 24 deletions(-) create mode 100644 packages/coding.sh/test/mcp/_blocking-server.ts create mode 100644 packages/coding.sh/test/mcp/protocol.test.ts diff --git a/packages/coding.sh/src/mcp/protocol.ts b/packages/coding.sh/src/mcp/protocol.ts index d635435a..1701129f 100644 --- a/packages/coding.sh/src/mcp/protocol.ts +++ b/packages/coding.sh/src/mcp/protocol.ts @@ -144,9 +144,39 @@ export interface RpcResponse { const errorResponse = (id: RpcId | null, code: number, message: string, data?: unknown): RpcResponse => ({ jsonrpc: "2.0", id, - error: data === undefined ? { code, message } : { code, message, data }, + error: data === undefined ? { code, message: clipText(message) } : { code, message: clipText(message), data }, }); +/** Longest error text sent back: an error that quotes a huge argument keeps its start and its advice. */ +export const ERROR_TEXT_MAX = 4000; + +/** `s` within `max` characters: its start and end, with how much was cut between them. */ +export function clipText(s: string, max = ERROR_TEXT_MAX): string { + if (s.length <= max) return s; + const keep = Math.floor((max - 40) / 2); + return `${s.slice(0, keep)} … [${s.length - 2 * keep} characters cut] … ${s.slice(-keep)}`; +} + +/** + * Caching hints on 2026-07-28 results (https://modelcontextprotocol.io/specification/2026-07-28/server/utilities/caching): + * the tool, prompt and template lists are fixed for the life of the process and hold no user data; + * resources are the user's sessions and change with every capture. + */ +const CACHE_HINTS: Record = { + "server/discover": { ttlMs: 3_600_000, cacheScope: "public" }, + "tools/list": { ttlMs: 3_600_000, cacheScope: "public" }, + "prompts/list": { ttlMs: 3_600_000, cacheScope: "public" }, + "resources/templates/list": { ttlMs: 3_600_000, cacheScope: "public" }, + "resources/list": { ttlMs: 0, cacheScope: "private" }, + "resources/read": { ttlMs: 0, cacheScope: "private" }, +}; + +/** These lists come back in one page, so any cursor is one this server never issued. */ +function noCursor(method: string, params: Obj): void { + if (params.cursor === undefined || params.cursor === null || params.cursor === "") return; + throw new RpcFailure(RPC.INVALID_PARAMS, `Invalid cursor: ${method} returns everything in one page; call it without a cursor.`); +} + /** Compact text for a structured tool result (the spec's backward-compatible TextContent). */ export function toolText(structured: unknown): string { return JSON.stringify(structured); @@ -172,11 +202,31 @@ export class McpServer { }; } - /** Handles one decoded message. Returns the response, or null for notifications and stray responses. */ - async handle(msg: unknown): Promise { - if (Array.isArray(msg)) return errorResponse(null, RPC.INVALID_REQUEST, "JSON-RPC batches are not supported; send one message per line"); + /** + * Handles one decoded message. Returns the response, or null for notifications and stray responses. + * A batch (an array) is served only in a 2025-03-26 session, the one version whose spec has them; + * its reply is the array of its responses, or null when it held only notifications. + */ + handle(msg: Obj): Promise; + handle(msg: unknown): Promise; + async handle(msg: unknown): Promise { + if (Array.isArray(msg)) { + if (this.legacyVersion !== "2025-03-26") return errorResponse(null, RPC.INVALID_REQUEST, "JSON-RPC batches are not supported; send one message per line"); + if (!msg.length) return errorResponse(null, RPC.INVALID_REQUEST, "Invalid request: empty batch"); + const replies: RpcResponse[] = []; + for (const m of msg) { + const r = Array.isArray(m) ? errorResponse(null, RPC.INVALID_REQUEST, "Invalid request: a batch holds JSON-RPC objects") : await this.handleOne(m); + if (r) replies.push(r); + } + return replies.length ? replies : null; + } + return this.handleOne(msg); + } + + private async handleOne(msg: unknown): Promise { if (!isObj(msg)) return errorResponse(null, RPC.INVALID_REQUEST, "Invalid request: expected a JSON-RPC object"); - const hasId = "id" in msg && msg.id !== undefined && msg.id !== null; + // A null id is still a request (JSON-RPC 2.0) and gets an answer; MCP ids are strings or numbers. + const hasId = "id" in msg && msg.id !== undefined; const idOk = typeof msg.id === "string" || (typeof msg.id === "number" && Number.isFinite(msg.id)); if (typeof msg.method !== "string") { // A response to a server→client request (we send none) or garbage. @@ -208,7 +258,7 @@ export class McpServer { } } const result = await this.dispatch(msg.method, params, modern); - return { jsonrpc: "2.0", id, result: modern ? this.modernResult(result) : result }; + return { jsonrpc: "2.0", id, result: modern ? this.modernResult(msg.method, result) : result }; } catch (e) { if (e instanceof RpcFailure) return errorResponse(id, e.code, e.message, e.data); this.o.log?.(`internal error in ${msg.method}: ${e instanceof Error ? (e.stack ?? e.message) : String(e)}`); @@ -218,12 +268,12 @@ export class McpServer { private notification(method: string): void { if (method === "notifications/initialized") this.initialized = true; - // notifications/cancelled, roots/list_changed…: requests finish quickly; nothing to do. + // notifications/cancelled is handled by the transport (serveStdio); roots/list_changed…: nothing to do. } - private modernResult(r: Obj): Obj { + private modernResult(method: string, r: Obj): Obj { const meta = isObj(r._meta) ? r._meta : {}; - return { resultType: "complete", ...r, _meta: { ...meta, [`${META}serverInfo`]: this.o.serverInfo } }; + return { resultType: "complete", ...CACHE_HINTS[method], ...r, _meta: { ...meta, [`${META}serverInfo`]: this.o.serverInfo } }; } private async dispatch(method: string, params: Obj, modern: boolean): Promise { @@ -240,6 +290,7 @@ export class McpServer { case "ping": return {}; case "tools/list": + noCursor(method, params); return { tools: this.o.tools.map((t) => ({ name: t.name, @@ -261,6 +312,7 @@ export class McpServer { case "resources/templates/list": { const r = this.o.resources; if (!r) throw new RpcFailure(RPC.METHOD_NOT_FOUND, "Method not found: resources/templates/list"); + noCursor(method, params); return { resourceTemplates: r.templates() }; } case "resources/read": { @@ -268,10 +320,11 @@ export class McpServer { if (!r) throw new RpcFailure(RPC.METHOD_NOT_FOUND, "Method not found: resources/read"); if (typeof params.uri !== "string" || !params.uri) throw new RpcFailure(RPC.INVALID_PARAMS, "resources/read needs a uri"); const c = await r.read(params.uri); - if (!c) throw new RpcFailure(modern ? RPC.INVALID_PARAMS : RPC.RESOURCE_NOT_FOUND, `Resource not found: ${params.uri}`, { uri: params.uri }); + if (!c) throw new RpcFailure(modern ? RPC.INVALID_PARAMS : RPC.RESOURCE_NOT_FOUND, `Resource not found: ${params.uri}`, { uri: clipText(params.uri, 1000) }); return { contents: [c] }; } case "prompts/list": + noCursor(method, params); return { prompts: (this.o.prompts ?? []).map((p) => ({ name: p.name, ...(p.title ? { title: p.title } : {}), description: p.description, ...(p.arguments ? { arguments: p.arguments } : {}) })) }; case "prompts/get": return this.getPrompt(params); @@ -308,7 +361,7 @@ export class McpServer { const structured = await tool.run(args, this.o.context()); return { content: [{ type: "text", text: toolText(structured) }], structuredContent: structured }; } catch (e) { - if (e instanceof ToolError) return e.error ? { ...errorResult(e.message), structuredContent: { error: { message: e.message, ...e.error } } } : errorResult(e.message); + if (e instanceof ToolError) return e.error ? { ...errorResult(e.message), structuredContent: { error: { message: clipText(e.message), ...e.error } } } : errorResult(e.message); this.o.log?.(`tool ${name} failed: ${e instanceof Error ? (e.stack ?? e.message) : String(e)}`); return errorResult(`${name} failed: ${e instanceof Error ? e.message : String(e)}`); } @@ -316,7 +369,7 @@ export class McpServer { } function errorResult(text: string): Obj { - return { content: [{ type: "text", text }], isError: true }; + return { content: [{ type: "text", text: clipText(text) }], isError: true }; } // ------------------------------------------------------------ stdio @@ -329,31 +382,58 @@ export interface StdioStreams { /** * Serves newline-delimited JSON-RPC on the streams until input ends. Requests * run one at a time in arrival order (handlers share one store connection). + * `notifications/cancelled` takes effect on arrival: a waiting request is + * skipped and a running one's reply is dropped, since the stdio transport says + * a cancelled request gets no further messages + * (https://modelcontextprotocol.io/specification/2026-07-28/basic/transports/stdio). */ export function serveStdio(server: McpServer, io: StdioStreams, log: (m: string) => void = () => {}, signal?: AbortSignal): Promise { return new Promise((resolve) => { let buf = ""; let chain = Promise.resolve(); let ended = false; - const write = (m: RpcResponse) => { + const write = (m: RpcResponse | RpcResponse[]) => { try { io.output.write(JSON.stringify(m) + "\n"); } catch (e) { log(`write failed: ${(e as Error).message}`); } }; + // Requests waiting or running, by id. A cancel marks the entry; a marked request is never answered. + const pending = new Map(); + // A synchronous store read holds the event loop, so a cancel sent meanwhile is still unread in the + // pipe. Two turns of the loop include one poll for input, which reads it before we start or answer. + const readInput = () => new Promise((r) => setImmediate(() => setImmediate(r))); const onLine = (line: string) => { - chain = chain.then(async () => { - let msg: unknown; - try { - msg = JSON.parse(line); - } catch { - write(errorResponse(null, RPC.PARSE_ERROR, "Parse error: each line must be one JSON-RPC message")); - return; - } - const r = await server.handle(msg); - if (r) write(r); - }); + let msg: unknown; + let parsed = true; + try { + msg = JSON.parse(line); + } catch { + parsed = false; + } + if (parsed && isObj(msg) && msg.method === "notifications/cancelled" && msg.id === undefined) { + const id = isObj(msg.params) ? msg.params.requestId : undefined; + const entry = typeof id === "string" || typeof id === "number" ? pending.get(JSON.stringify(id)) : undefined; + if (entry) entry.cancelled = true; + return; + } + const key = parsed && isObj(msg) && typeof msg.method === "string" && (typeof msg.id === "string" || typeof msg.id === "number") ? JSON.stringify(msg.id) : undefined; + const entry = { cancelled: false }; + if (key) pending.set(key, entry); + chain = chain + .then(async () => { + if (!parsed) return write(errorResponse(null, RPC.PARSE_ERROR, "Parse error: each line must be one JSON-RPC message")); + if (key) await readInput(); + if (entry.cancelled) return; + const r = await server.handle(msg); + if (key && r) await readInput(); + if (r && !entry.cancelled) write(r); + }) + .catch((e: unknown) => log(`request failed: ${e instanceof Error ? e.message : String(e)}`)) + .finally(() => { + if (key && pending.get(key) === entry) pending.delete(key); + }); }; io.output.on?.("error", (e: Error) => log(`stdout: ${e.message}`)); io.input.setEncoding?.("utf8"); diff --git a/packages/coding.sh/test/mcp/_blocking-server.ts b/packages/coding.sh/test/mcp/_blocking-server.ts new file mode 100644 index 00000000..6da4a05e --- /dev/null +++ b/packages/coding.sh/test/mcp/_blocking-server.ts @@ -0,0 +1,29 @@ +// An MCP server on stdio whose `block` tool holds the event loop for `ms` +// milliseconds, the way a synchronous SQLite read does. protocol.test.ts sends +// a cancel while it blocks. +import { McpServer, serveStdio } from "../../src/mcp/protocol.ts"; + +const server = new McpServer({ + serverInfo: { name: "blocking", version: "0" }, + instructions: () => "", + tools: [ + { + name: "block", + title: "Block", + description: "Busy-waits for ms milliseconds.", + inputSchema: { type: "object", properties: { ms: { type: "number" } }, additionalProperties: false }, + outputSchema: { type: "object" }, + annotations: { readOnlyHint: true }, + run: async (a) => { + const end = Date.now() + Number(a.ms ?? 0); + while (Date.now() < end) { + // hold the event loop + } + return { done: true }; + }, + }, + ], + context: () => undefined, +}); + +await serveStdio(server, { input: process.stdin, output: process.stdout }); diff --git a/packages/coding.sh/test/mcp/protocol.test.ts b/packages/coding.sh/test/mcp/protocol.test.ts new file mode 100644 index 00000000..bcf34224 --- /dev/null +++ b/packages/coding.sh/test/mcp/protocol.test.ts @@ -0,0 +1,243 @@ +// The MCP core against the spec rules a client can observe: caching hints on +// 2026-07-28 list and read results, cancellation, invalid cursors, a null id, +// 2025-03-26 batches and error text kept short when an argument is huge. +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { PassThrough } from "node:stream"; +import { describe, it } from "node:test"; +import { McpServer, serveStdio, ToolError, type ToolDef } from "../../src/mcp/protocol.ts"; + +type Msg = { jsonrpc: string; id: unknown; result?: Record; error?: { code: number; message: string; data?: Record } }; + +const META = "io.modelcontextprotocol/"; +const modern = { [`${META}protocolVersion`]: "2026-07-28", [`${META}clientCapabilities`]: {} }; + +/** A server with a `slow` tool that waits for release(), an `echo` tool that fails with its input, and one resource. */ +function world() { + const gates: (() => void)[] = []; + const slow: ToolDef = { + name: "slow", + title: "Slow", + description: "Waits until the test releases it.", + inputSchema: { type: "object", properties: {}, additionalProperties: false }, + outputSchema: { type: "object" }, + annotations: { readOnlyHint: true }, + run: () => new Promise((r) => gates.push(() => r({ done: true }))), + }; + const echo: ToolDef = { + name: "echo", + title: "Echo", + description: "Fails with a message that quotes its input.", + inputSchema: { type: "object", properties: { text: { type: "string" } }, additionalProperties: false }, + outputSchema: { type: "object" }, + annotations: { readOnlyHint: true }, + run: (a) => Promise.reject(new ToolError(`No session with id ${JSON.stringify(a.text)}. Find ids with sessions_list.`)), + }; + const server = new McpServer({ + serverInfo: { name: "t", version: "0" }, + instructions: () => "test", + tools: [slow, echo], + resources: { + list: async () => ({ resources: [{ uri: "t://one", name: "one" }] }), + templates: () => [{ uriTemplate: "t://{id}", name: "by id" }], + read: async (uri) => (uri === "t://one" ? { uri, mimeType: "text/plain", text: "one" } : undefined), + }, + prompts: [{ name: "p", description: "A prompt.", render: () => "hi" }], + context: () => undefined, + }); + const input = new PassThrough(); + const output = new PassThrough(); + const out: Msg[] = []; + /** Batch replies: one array line each. */ + const batches: Msg[][] = []; + let buf = ""; + output.setEncoding("utf8"); + output.on("data", (c: string) => { + buf += c; + let i: number; + while ((i = buf.indexOf("\n")) !== -1) { + const m = JSON.parse(buf.slice(0, i)) as Msg | Msg[]; + if (Array.isArray(m)) batches.push(m); + else out.push(m); + buf = buf.slice(i + 1); + } + }); + const done = serveStdio(server, { input, output }); + const send = (m: unknown) => input.write((typeof m === "string" ? m : JSON.stringify(m)) + "\n"); + const tick = () => new Promise((r) => setTimeout(r, 20)); + /** Waits until a reply with this id arrives (or 2 s pass). */ + const reply = async (id: unknown): Promise => { + for (let i = 0; i < 100; i++) { + const m = out.find((x) => JSON.stringify(x.id) === JSON.stringify(id)); + if (m) return m; + await tick(); + } + return undefined; + }; + const release = async () => { + for (let i = 0; i < 100 && !gates.length; i++) await tick(); + gates.shift()!(); + }; + return { server, send, out, batches, reply, tick, release, close: async () => (input.end(), done) }; +} + +describe("caching hints (2026-07-28)", () => { + it("every list, read and discover result carries ttlMs and cacheScope", async () => { + const w = world(); + const cases: [string, Record, string][] = [ + ["server/discover", {}, "public"], + ["tools/list", {}, "public"], + ["prompts/list", {}, "public"], + ["resources/templates/list", {}, "public"], + ["resources/list", {}, "private"], + ["resources/read", { uri: "t://one" }, "private"], + ]; + let id = 0; + for (const [method, params, scope] of cases) { + w.send({ jsonrpc: "2.0", id: ++id, method, params: { ...params, _meta: modern } }); + const r = await w.reply(id); + assert.equal(r?.result?.resultType, "complete", method); + assert.equal(typeof r?.result?.ttlMs, "number", `${method} ttlMs`); + assert.ok((r!.result!.ttlMs as number) >= 0, `${method} ttlMs >= 0`); + assert.equal(r?.result?.cacheScope, scope, `${method} cacheScope`); + } + // Legacy results stay as they were. + w.send({ jsonrpc: "2.0", id: "legacy", method: "tools/list" }); + const legacy = await w.reply("legacy"); + assert.equal(legacy?.result?.ttlMs, undefined); + await w.close(); + }); +}); + +describe("cancellation (notifications/cancelled)", () => { + it("a request cancelled before it runs gets no reply", async () => { + const w = world(); + w.send({ jsonrpc: "2.0", id: 1, method: "tools/call", params: { name: "slow" } }); + w.send({ jsonrpc: "2.0", id: 2, method: "ping" }); + w.send({ jsonrpc: "2.0", method: "notifications/cancelled", params: { requestId: 2, reason: "test" } }); + await w.release(); + assert.ok(await w.reply(1), "the running request is answered"); + w.send({ jsonrpc: "2.0", id: 3, method: "ping" }); + assert.ok(await w.reply(3), "later requests are answered"); + assert.equal(w.out.find((m) => m.id === 2), undefined, "the cancelled request got no reply"); + await w.close(); + }); + + it("a request cancelled while it runs gets no reply", async () => { + const w = world(); + w.send({ jsonrpc: "2.0", id: "a", method: "tools/call", params: { name: "slow" } }); + await w.tick(); + w.send({ jsonrpc: "2.0", method: "notifications/cancelled", params: { requestId: "a" } }); + await w.tick(); + await w.release(); + w.send({ jsonrpc: "2.0", id: "b", method: "ping" }); + assert.ok(await w.reply("b")); + assert.equal(w.out.find((m) => m.id === "a"), undefined, "the cancelled request got no reply"); + // A cancel for an unknown or finished request is ignored. + w.send({ jsonrpc: "2.0", method: "notifications/cancelled", params: { requestId: "b" } }); + w.send({ jsonrpc: "2.0", id: "c", method: "ping" }); + assert.ok(await w.reply("c")); + await w.close(); + }); +}); + +describe("cancellation while a tool holds the event loop", () => { + // A synchronous store read blocks the loop, so a cancel sits unread in the pipe until it ends. + it("reads the cancel before it answers or starts the next request", async () => { + const child = spawn(process.execPath, ["--no-warnings", new URL("./_blocking-server.ts", import.meta.url).pathname], { stdio: ["pipe", "pipe", "inherit"] }); + const ids: unknown[] = []; + let buf = ""; + child.stdout.setEncoding("utf8"); + child.stdout.on("data", (c: string) => { + buf += c; + let i: number; + while ((i = buf.indexOf("\n")) !== -1) { + ids.push((JSON.parse(buf.slice(0, i)) as Msg).id); + buf = buf.slice(i + 1); + } + }); + const send = (m: unknown) => child.stdin.write(JSON.stringify(m) + "\n"); + const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)); + const exited = new Promise((r) => child.on("exit", r)); + send({ jsonrpc: "2.0", id: "warm", method: "ping" }); + for (let i = 0; i < 200 && !ids.includes("warm"); i++) await wait(10); + // "run" is cancelled while it blocks; "queued" is cancelled while it waits behind "first". + send({ jsonrpc: "2.0", id: "run", method: "tools/call", params: { name: "block", arguments: { ms: 400 } } }); + await wait(100); + send({ jsonrpc: "2.0", method: "notifications/cancelled", params: { requestId: "run" } }); + await wait(500); + send({ jsonrpc: "2.0", id: "first", method: "tools/call", params: { name: "block", arguments: { ms: 400 } } }); + await wait(100); + send({ jsonrpc: "2.0", id: "queued", method: "ping" }); + await wait(50); + send({ jsonrpc: "2.0", method: "notifications/cancelled", params: { requestId: "queued" } }); + await wait(500); + send({ jsonrpc: "2.0", id: "last", method: "ping" }); + for (let i = 0; i < 200 && !ids.includes("last"); i++) await wait(10); + child.stdin.end(); + await exited; + assert.deepEqual(ids, ["warm", "first", "last"]); + }); +}); + +describe("JSON-RPC edges", () => { + it("rejects a cursor on lists that have one page", async () => { + const w = world(); + for (const [i, method] of ["tools/list", "prompts/list", "resources/templates/list"].entries()) { + w.send({ jsonrpc: "2.0", id: i, method, params: { cursor: "not-a-cursor" } }); + const r = await w.reply(i); + assert.equal(r?.error?.code, -32602, `${method}: ${JSON.stringify(r)}`); + } + await w.close(); + }); + + it("answers a request whose id is null with an invalid-request error", async () => { + const w = world(); + w.send({ jsonrpc: "2.0", id: null, method: "tools/list" }); + const r = await w.reply(null); + assert.equal(r?.error?.code, -32600); + await w.close(); + }); + + it("accepts batches after a 2025-03-26 handshake, and only then", async () => { + const w = world(); + w.send([{ jsonrpc: "2.0", id: "x", method: "ping" }]); + const before = await w.reply(null); + assert.equal(before?.error?.code, -32600, "no batches before the handshake"); + w.send({ jsonrpc: "2.0", id: 0, method: "initialize", params: { protocolVersion: "2025-03-26", capabilities: {}, clientInfo: { name: "t", version: "0" } } }); + assert.equal((await w.reply(0))?.result?.protocolVersion, "2025-03-26"); + const n = w.out.length; + w.send([{ jsonrpc: "2.0", id: 1, method: "ping" }, { jsonrpc: "2.0", method: "notifications/initialized" }, { jsonrpc: "2.0", id: 2, method: "no/such" }]); + for (let i = 0; i < 100 && !w.batches.length; i++) await w.tick(); + // The reply is one array line with a response per request, none for the notification. + const batch = w.batches[0]!; + assert.ok(batch, "a batch reply"); + assert.deepEqual(batch.map((m) => m.id), [1, 2]); + assert.deepEqual(batch[0]!.result, {}); + assert.equal(batch[1]!.error!.code, -32601); + w.send([]); + for (let i = 0; i < 100 && w.out.length === n; i++) await w.tick(); + assert.equal(w.out[n]!.error?.code, -32600, "an empty batch is invalid"); + await w.close(); + }); + + it("keeps error text short when an argument is huge", async () => { + const w = world(); + const huge = "h".repeat(1_000_000); + w.send({ jsonrpc: "2.0", id: 1, method: "tools/call", params: { name: "echo", arguments: { text: huge } } }); + const tool = await w.reply(1); + const text = (tool!.result!.content as { text: string }[])[0]!.text; + assert.equal(tool!.result!.isError, true); + assert.ok(text.length < 5000, `tool error is ${text.length} chars`); + assert.match(text, /^No session with id "hhh/); + assert.match(text, /Find ids with sessions_list\.$/, "the advice at the end survives"); + w.send({ jsonrpc: "2.0", id: 2, method: "tools/call", params: { name: huge } }); + const rpc = await w.reply(2); + assert.equal(rpc!.error!.code, -32602); + assert.ok(rpc!.error!.message.length < 5000, `rpc error is ${rpc!.error!.message.length} chars`); + w.send({ jsonrpc: "2.0", id: 3, method: "resources/read", params: { uri: `t://${huge}` } }); + const res = await w.reply(3); + assert.ok(JSON.stringify(res).length < 10_000, `resource error is ${JSON.stringify(res).length} chars`); + await w.close(); + }); +}); From ad896cb8597ba95ebbd2749e0e01290fc0bcf5d6 Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Wed, 11 Nov 2026 21:27:55 -0600 Subject: [PATCH 02/11] fix(mcp): keep tool answers under 25,000 characters A read tool with a limit runs again with fewer rows while its text is over 25,000 characters, and the answer carries limitUsed. Rows are dropped, never cut, so nextCursor still starts at the first row left out. session_get at limit 200 was 40,000 characters on a long session and session_replay's step list 227,000; session_replay now shows 16 KB of a file or diff (was 64 KB), and eval_candidates returns at most limit rejected sessions. --- packages/coding.sh/src/mcp/server.ts | 1 + packages/coding.sh/src/mcp/tools.ts | 50 ++++++- packages/coding.sh/test/mcp/limits.test.ts | 127 ++++++++++++++++++ .../coding.sh/test/mcp/session-tree.test.ts | 13 +- 4 files changed, 181 insertions(+), 10 deletions(-) create mode 100644 packages/coding.sh/test/mcp/limits.test.ts diff --git a/packages/coding.sh/src/mcp/server.ts b/packages/coding.sh/src/mcp/server.ts index b22c73b0..fe0df128 100644 --- a/packages/coding.sh/src/mcp/server.ts +++ b/packages/coding.sh/src/mcp/server.ts @@ -44,6 +44,7 @@ const BASE_INSTRUCTIONS = [ "To build evals for the user, call eval_init (a suite in evals// from failed sessions, a skill's trigger prompts, or cases you write), then tell them its run command.", "eval_from_session, eval_init and exp_session_note write; ask the user before calling eval_from_session or eval_init with confirm: true.", 'A result with pending: {files, indexing} was answered before every capture was indexed: it covers what is indexed so far; call again shortly for the rest.', + "Answers stay under 25,000 characters: one that would be longer has fewer rows and limitUsed; follow nextCursor for the rest.", ].join(" "); /** Server instructions, with a one-line notice when the update checker found a newer release. */ diff --git a/packages/coding.sh/src/mcp/tools.ts b/packages/coding.sh/src/mcp/tools.ts index 54a00451..6f391411 100644 --- a/packages/coding.sh/src/mcp/tools.ts +++ b/packages/coding.sh/src/mcp/tools.ts @@ -39,7 +39,7 @@ import { import { sessionDigest } from "./digest.ts"; import { facetCounts, searchWhere } from "../query/run.ts"; import { andQuery, FACET_KEYS, parseWhere, QueryError, QueryUnsupported, type Query } from "../query/where.ts"; -import { ToolError, type ToolDef } from "./protocol.ts"; +import { toolText, ToolError, type ToolDef } from "./protocol.ts"; import { scoreTools } from "../scores/mcp.ts"; type Obj = Record; @@ -131,6 +131,42 @@ function treeNodeJson(n: TreeNode, top = false): Obj { // ------------------------------------------------------------ helpers +/** The longest answer a tool sends, in characters of its text (mcp-builder's CHARACTER_LIMIT). */ +export const CHARACTER_LIMIT = 25_000; + +/** + * The page `render(limit)` makes, with fewer rows while its text is over CHARACTER_LIMIT. Rows are + * dropped, never cut, so a cursor of offset + n still points at the first row left out. A shortened + * page carries `limitUsed`. When no smaller limit makes it shorter (the size is not in the rows, or + * one row is that long), the shortest page comes back whole. + */ +export async function fitPage(limit: number, render: (n: number) => Obj | Promise): Promise { + let page = await render(limit); + let len = toolText(page).length; + let used: number | undefined; + // n can start above the rows left (the last page), so a try that is no shorter only halves n. + for (let n = limit; len > CHARACTER_LIMIT && n > 1; ) { + n = Math.max(1, Math.min(Math.floor(n / 2), Math.floor((n * CHARACTER_LIMIT * 0.9) / len))); + const smaller = await render(n); + const smallerLen = toolText(smaller).length; + if (smallerLen < len) [page, len, used] = [smaller, smallerLen, n]; + } + return used === undefined ? page : { ...page, limitUsed: used }; +} + +/** A read-only tool with a `limit` argument, run again with a smaller limit when its answer is too long. */ +function withCharacterLimit(t: ToolDef): ToolDef { + const limit = (t.inputSchema.properties as Record | undefined)?.limit; + if (!limit || t.annotations.readOnlyHint !== true || t.annotations.idempotentHint !== true) return t; + return { + ...t, + run(a, c) { + const asked = typeof a.limit === "number" ? a.limit : typeof limit.default === "number" ? limit.default : undefined; + return asked === undefined ? t.run(a, c) : fitPage(asked, (n) => t.run(n === asked ? a : { ...a, limit: n }, c)); + }, + }; +} + const READ = { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false } as const; function strArg(a: Obj, k: string): string | undefined { @@ -300,6 +336,10 @@ function lastSpoolError(file: string): Obj | undefined { // ------------------------------------------------------------ tools export function buildTools(): ToolDef[] { + return toolDefs().map(withCharacterLimit); +} + +function toolDefs(): ToolDef[] { return [ { name: "sessions_list", @@ -428,7 +468,8 @@ export function buildTools(): ToolDef[] { const ctx = await c.store(); const s = await resolveSession(ctx, strArg(a, "sessionId")!); const redactor = createRedactor({ redact: ctx.config.capture.redact, patterns: ctx.config.capture.redactPatterns, env: ctx.env }); - const replay = await loadReplay(ctx, s, { baseline: snapshotBaseline(s, ctx.env, redactor), maxContentBytes: 64 * 1024, maxDiffBytes: 64 * 1024 }); + // 16 KB of a file and of each diff keeps a step's view near CHARACTER_LIMIT; contentTruncated and diffTruncated say when they were cut. + const replay = await loadReplay(ctx, s, { baseline: snapshotBaseline(s, ctx.env, redactor), maxContentBytes: 16 * 1024, maxDiffBytes: 16 * 1024 }); const total = replay.steps.length; const stepArg = (k: string): number | undefined => { const v = a[k]; @@ -453,8 +494,7 @@ export function buildTools(): ToolDef[] { const index = replay.index(); const offset = cursorArg(a); const limit = numArg(a, "limit", 100); - const page = index.steps.slice(offset, offset + limit); - return { ...index, steps: page, total, ...(offset + limit < total ? { nextCursor: String(offset + limit) } : {}) }; + return fitPage(limit, (n) => ({ ...index, steps: index.steps.slice(offset, offset + n), total, ...(offset + n < total ? { nextCursor: String(offset + n) } : {}) })); }, }, { @@ -898,7 +938,7 @@ export function buildTools(): ToolDef[] { const ranked = await rankSessions(ctx.index, pool, { now }); const eligible = ranked.filter((x) => x.eligible); const limit = numArg(a, "limit", 10); - const shown = [...eligible.slice(0, limit), ...(a.includeRejected === true ? ranked.filter((x) => !x.eligible) : [])]; + const shown = [...eligible.slice(0, limit), ...(a.includeRejected === true ? ranked.filter((x) => !x.eligible).slice(0, limit) : [])]; return { candidates: shown.map((x) => ({ ...x, title: clip(x.title, 100), startedAt: new Date(x.startedAt).toISOString() })), considered: ranked.length, diff --git a/packages/coding.sh/test/mcp/limits.test.ts b/packages/coding.sh/test/mcp/limits.test.ts new file mode 100644 index 00000000..e4231a84 --- /dev/null +++ b/packages/coding.sh/test/mcp/limits.test.ts @@ -0,0 +1,127 @@ +// Tool answers stay under the 25,000-character limit by returning fewer rows +// (never by cutting text), the cursor still walks every row, and errors that +// quote a huge argument stay short. +import assert from "node:assert/strict"; +import { after, before, describe, it } from "node:test"; +import { openContext } from "../../src/core/context.ts"; +import { buildSession } from "../../src/core/sessions.ts"; +import { CHARACTER_LIMIT } from "../../src/mcp/tools.ts"; +import type { AgentEvent } from "../../src/schema/index.ts"; +import { RawClient, cleanup, makeWorld, seed, type World } from "./helpers.ts"; + +const BIG = "sess-big"; +const CALLS = 600; + +let w: World; +let c: RawClient; + +type Result = { structuredContent?: Record; content: { type: string; text: string }[]; isError?: boolean }; + +async function call(name: string, args: Record): Promise { + const r = await c.call(name, args); + assert.notEqual(r.isError, true, `${name}: ${r.content[0]?.text.slice(0, 300)}`); + return r; +} + +before(async () => { + w = makeWorld(); + await seed(w); + // One long session: CALLS Bash calls with 4 KB outputs. + const ctx = await openContext({ env: w.env, cwd: w.root }); + const t0 = w.now - 3 * 3600e3; + const base = { v: 0 as const, sessionId: BIG, agent: "claude-code", cwd: w.repo, model: "claude-opus-4-5" }; + const evs: AgentEvent[] = [ + { ...base, id: "big-0", ts: t0, kind: "session.start", payload: {} }, + { ...base, id: "big-1", ts: t0 + 1, kind: "user.message", payload: { text: "Run the migration" } }, + ]; + for (let i = 0; i < CALLS; i++) { + evs.push({ ...base, id: `big-s${i}`, ts: t0 + 10 + i * 10, kind: "tool.start", payload: { tool: "Bash", toolUseId: `b${i}`, input: { command: `echo needle-${i} ${"y".repeat(2000)}` } } }); + evs.push({ ...base, id: `big-e${i}`, ts: t0 + 15 + i * 10, kind: "tool.end", payload: { tool: "Bash", toolUseId: `b${i}`, ok: true, output: `needle out ${i} ${"z".repeat(4000)}` }, durationMs: 5 }); + } + evs.push({ ...base, id: "big-end", ts: t0 + 20 * CALLS, kind: "session.end", payload: {} }); + await ctx.index.putEvents(evs); + await ctx.index.upsertSession(buildSession(evs)); + await ctx.close(); + c = new RawClient(w.env, w.repo); + await c.initialize(); +}); + +after(async () => { + await c?.close(); + cleanup(); +}); + +describe("the 25,000-character limit", () => { + it("session_get returns fewer timeline entries, and the cursor walks all of them", async () => { + const seen: number[] = []; + let cursor: string | undefined; + for (let page = 0; page < 200; page++) { + const args: Record = { sessionId: BIG, limit: 200 }; + if (cursor) args.cursor = cursor; + const r = await call("session_get", args); + assert.ok(r.content[0]!.text.length <= CHARACTER_LIMIT, `page ${page} is ${r.content[0]!.text.length} chars`); + const sc = r.structuredContent!; + for (const e of sc.timeline as { at: number }[]) seen.push(e.at); + cursor = sc.nextCursor as string | undefined; + if (!cursor) break; + assert.ok((sc.limitUsed as number) < 200, "a shortened page says which limit it used"); + } + assert.equal(cursor, undefined, "the cursor reached the end"); + assert.equal(new Set(seen).size, seen.length, "no entry is repeated"); + const all = await call("session_get", { sessionId: BIG, limit: 200, tail: true }); + assert.ok(all.content[0]!.text.length <= CHARACTER_LIMIT); + assert.ok(seen.length > CALLS, `every tool call is in the walk (${seen.length})`); + }); + + it("session_replay pages its step list under the limit", async () => { + let cursor: string | undefined; + let steps = 0; + let total = 0; + for (let page = 0; page < 200; page++) { + const args: Record = { sessionId: BIG, limit: 1000 }; + if (cursor) args.cursor = cursor; + const r = await call("session_replay", args); + assert.ok(r.content[0]!.text.length <= CHARACTER_LIMIT, `page ${page} is ${r.content[0]!.text.length} chars`); + const sc = r.structuredContent!; + steps += (sc.steps as unknown[]).length; + total = sc.total as number; + cursor = sc.nextCursor as string | undefined; + if (!cursor) break; + } + assert.ok(total > 0); + assert.equal(steps, total, "the pages hold every step once"); + }); + + it("search, sessions_list and session_current stay under the limit", async () => { + for (const [name, args] of [ + ["search", { query: "needle", limit: 50 }], + ["sessions_list", { limit: 100, includeSubagents: true }], + ["session_current", { limit: 100 }], + ] as const) { + const r = await call(name, args); + assert.ok(r.content[0]!.text.length <= CHARACTER_LIMIT, `${name} is ${r.content[0]!.text.length} chars`); + } + }); + + it("eval_candidates returns at most limit rejected sessions", async () => { + const r = await call("eval_candidates", { limit: 1, includeRejected: true, repo: "all", since: "30d" }); + const sc = r.structuredContent as { candidates: { eligible: boolean }[]; considered: number; eligible: number }; + assert.ok(sc.considered - sc.eligible > 1, `the seed has more than one rejected session (${sc.considered - sc.eligible})`); + assert.ok(sc.candidates.filter((x) => !x.eligible).length <= 1, `${sc.candidates.filter((x) => !x.eligible).length} rejected sessions for limit 1`); + }); +}); + +describe("tool errors", () => { + it("quote a huge argument in a short message", async () => { + const huge = "h".repeat(1_000_000); + for (const [name, args] of [ + ["session_get", { sessionId: huge }], + ["benchmarks", { name: huge }], + ["harness_compare", { experiment: huge }], + ] as const) { + const r = await c.call(name, args); + assert.equal(r.isError, true, name); + assert.ok(r.content[0]!.text.length < 5000, `${name} error is ${r.content[0]!.text.length} chars`); + } + }); +}); diff --git a/packages/coding.sh/test/mcp/session-tree.test.ts b/packages/coding.sh/test/mcp/session-tree.test.ts index 911f7020..fea679dc 100644 --- a/packages/coding.sh/test/mcp/session-tree.test.ts +++ b/packages/coding.sh/test/mcp/session-tree.test.ts @@ -10,6 +10,7 @@ import { openContext } from "../../src/core/context.ts"; import { buildSession } from "../../src/core/sessions.ts"; import { pageTree, type TreeNode } from "../../src/core/tree.ts"; import { createCodingShMcp, type CodingShMcp } from "../../src/mcp/server.ts"; +import { CHARACTER_LIMIT } from "../../src/mcp/tools.ts"; import type { AgentEvent, Session } from "../../src/schema/index.ts"; import { schemaViolations } from "../../src/toolevals/jsonschema.ts"; import { PKG, cleanup, makeWorld, type World } from "./helpers.ts"; @@ -58,14 +59,14 @@ let w: World; let mcp: CodingShMcp; let outputSchema: Record; -async function tree(args: Record): Promise<{ page: Page; bytes: number }> { +async function tree(args: Record): Promise<{ page: Page; bytes: number; chars: number }> { const r = await mcp.server.handle({ jsonrpc: "2.0", id: 1, method: "tools/call", params: { name: "session_tree", arguments: args } }); const res = r!.result as { structuredContent: Page; isError?: boolean; content: { text: string }[] }; assert.notEqual(res.isError, true, res.content?.[0]?.text); const page = JSON.parse(JSON.stringify(res.structuredContent)) as Page; assert.deepEqual(schemaViolations(page, outputSchema), [], "session_tree output matches its outputSchema"); // What the agent reads: the whole tool result, text and structured content. - return { page, bytes: Buffer.byteLength(JSON.stringify(r!.result)) }; + return { page, bytes: Buffer.byteLength(JSON.stringify(r!.result)), chars: res.content[0]!.text.length }; } before(async () => { @@ -114,15 +115,17 @@ describe("session_tree on a run with 800 subagents", () => { let cursor: string | undefined; let calls = 0; do { - const { page } = await tree(cursor ? { sessionId: ROOT, limit: 200, cursor } : { sessionId: ROOT, limit: 200 }); + const { page, chars } = await tree(cursor ? { sessionId: ROOT, limit: 200, cursor } : { sessionId: ROOT, limit: 200 }); calls++; + assert.ok(chars <= CHARACTER_LIMIT, `page ${calls} is ${chars} characters`); assert.equal(page.totals.sessions, NODES); if (cursor) for (const top of page.tree) assert.deepEqual({ parent: top.parentSessionId, depth: top.depth }, { parent: ROOT, depth: 1 }); seen.push(...flat(page.tree).map((x) => x.sessionId!)); cursor = page.nextCursor; assert.equal(page.truncated, cursor ? true : undefined); - } while (cursor && calls < 10); - assert.equal(calls, 5, "802 nodes at 200 a page"); + } while (cursor && calls < 50); + // 200 nodes are over the 25,000-character limit, so each page holds fewer. + assert.ok(calls > 5, `802 nodes in ${calls} pages`); assert.equal(seen.length, NODES); assert.equal(new Set(seen).size, NODES, "no node twice"); assert.deepEqual(seen.slice(0, 3), [ROOT, child(0), GRAND], "depth-first, as coding.sh tree prints it"); From ac0f36afec03568cc444eb9f1a1acbe64bcda259 Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Wed, 11 Nov 2026 21:28:05 -0600 Subject: [PATCH 03/11] fix(mcp): mark eval_init and score_add destructive eval_init with force replaces a suite, and score_add replaces the score stored for the same target, name, source and person, so both now say destructiveHint: true. score_list's error for a bad since names the argument instead of the CLI's --since. --- packages/coding.sh/src/mcp/tools.ts | 3 ++- packages/coding.sh/src/scores/mcp.ts | 7 ++++--- packages/coding.sh/test/mcp/server.test.ts | 3 ++- packages/coding.sh/test/scores/cli.test.ts | 3 ++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/coding.sh/src/mcp/tools.ts b/packages/coding.sh/src/mcp/tools.ts index 6f391411..3795a49a 100644 --- a/packages/coding.sh/src/mcp/tools.ts +++ b/packages/coding.sh/src/mcp/tools.ts @@ -847,7 +847,8 @@ function toolDefs(): ToolDef[] { }, ["name", "dir", "kind", "files", "run"], ), - annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: false }, + // destructive: force replaces an existing suite. + annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: false, openWorldHint: false }, async run(a, c) { if (a.confirm !== true) throw new ToolError("Not written: this writes an eval suite into the user's project. Ask the user, then call again with confirm: true."); const { initSuite, InitError } = await import("../eval/init.ts"); diff --git a/packages/coding.sh/src/scores/mcp.ts b/packages/coding.sh/src/scores/mcp.ts index 9ff502f5..a6cc3b2b 100644 --- a/packages/coding.sh/src/scores/mcp.ts +++ b/packages/coding.sh/src/scores/mcp.ts @@ -60,8 +60,8 @@ export function scoreTools(): ToolDef[] { if (typeof a.since === "string" && a.since) { try { f.since = parseSince(a.since, c.now()); - } catch (e) { - throw new ToolError(e instanceof Error ? e.message : String(e)); + } catch { + throw new ToolError(`Invalid since ${JSON.stringify(a.since)}: use a duration like 30m, 12h, 7d, today, yesterday or a date like 2026-01-31 (local time).`); } } return { items: await listScores(ctx.index, f).catch((e) => Promise.reject(toolError(e))) }; @@ -88,7 +88,8 @@ export function scoreTools(): ToolDef[] { additionalProperties: false, }, outputSchema: { type: "object", properties: { score: SCORE_OUT, created: B, changed: B }, required: ["score"] }, - annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: false }, + // destructive: a score for the same target, name, source and person replaces the one stored. + annotations: { readOnlyHint: false, destructiveHint: true, idempotentHint: true, openWorldHint: false }, async run(a: Obj, c: ToolCtx) { const ctx = await c.store(); const body: Obj = { ...a }; diff --git a/packages/coding.sh/test/mcp/server.test.ts b/packages/coding.sh/test/mcp/server.test.ts index da39cd89..43afd68e 100644 --- a/packages/coding.sh/test/mcp/server.test.ts +++ b/packages/coding.sh/test/mcp/server.test.ts @@ -104,7 +104,8 @@ describe("handshake (legacy era)", () => { for (const [k, p] of Object.entries((t.inputSchema.properties ?? {}) as Record)) assert.ok(p.description, `${t.name}.${k} has a description`); assert.equal(t.outputSchema.type, "object", `${t.name} outputSchema`); assert.equal(t.annotations.readOnlyHint, !["eval_from_session", "eval_init", "exp_session_note", "score_add"].includes(t.name), `${t.name} readOnlyHint`); - assert.equal(t.annotations.destructiveHint, false); + // eval_init's force replaces a suite; score_add replaces the score for the same target and name. + assert.equal(t.annotations.destructiveHint, ["eval_init", "score_add"].includes(t.name), `${t.name} destructiveHint`); // session_replay's fork calls the model API. assert.equal(t.annotations.openWorldHint, t.name === "session_replay", `${t.name} openWorldHint`); } diff --git a/packages/coding.sh/test/scores/cli.test.ts b/packages/coding.sh/test/scores/cli.test.ts index 88241393..dfd94b6b 100644 --- a/packages/coding.sh/test/scores/cli.test.ts +++ b/packages/coding.sh/test/scores/cli.test.ts @@ -149,6 +149,7 @@ describe("MCP score tools", () => { const items = ((await run(list!, { sessionId: "latest", name: "helpfulness", source: "api" })) as { items: Score[] }).items; assert.deepEqual(items.map((s) => s.value), [3]); assert.equal(((await run(list!, { limit: 1 })) as { items: Score[] }).items.length, 1); - await assert.rejects(run(list!, { since: "yesterday-ish" }), ToolError); + // The agent passes since, not the CLI's --since. + await assert.rejects(run(list!, { since: "yesterday-ish" }), (e) => e instanceof ToolError && /^Invalid since "yesterday-ish"/.test(e.message) && !e.message.includes("--since")); }); }); From bba99b54f320d1b701fa1f9f3d9e2db5bd088bf3 Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Wed, 11 Nov 2026 21:28:06 -0600 Subject: [PATCH 04/11] docs(mcp): the protocol versions, caching hints, cancellation and the answer size limit --- CHANGELOG.md | 3 +++ apps/site/src/pages/docs/mcp.astro | 22 +++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2d50201..2e4d329b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,9 @@ grouped by area, with links to the docs for the details. ### Fixed +- **MCP.** In 2026-07-28 requests, list, read and discover results carry `ttlMs` and `cacheScope`. A request cancelled with `notifications/cancelled` gets no reply, a cursor on `tools/list`, `prompts/list` or `resources/templates/list` is an error, a request with a null id gets an error instead of no answer, and a 2025-03-26 session accepts batches. [MCP server](https://coding.sh/docs/mcp#protocol) +- **MCP.** Tool answers stay under 25,000 characters. A page that would be longer has fewer rows and `limitUsed`, and `nextCursor` goes on from the first row left out: `session_get` at limit 200 was 40,000 characters and `session_replay`'s step list 227,000. An error that quotes a huge argument is cut to 4,000 characters, and `eval_candidates` returns at most `limit` rejected sessions. [MCP server](https://coding.sh/docs/mcp#tools) +- **MCP.** `eval_init` and `score_add` are marked destructive, since both can replace stored data, and `score_list`'s error names its `since` argument instead of the CLI's `--since`. [MCP server](https://coding.sh/docs/mcp#tools) - **Capture.** Model calls no longer show impossible speeds such as 3750 tok/s. A Claude Code hook that read a transcript from the middle of a response, or Codex lines written seconds late, gave a call a start time during the response. Such a call now has no duration, time to first token or speed. A Claude Code call gets them back when the import reads the whole transcript, and the re-parse after the upgrade repairs calls older versions stored. [Adapters](https://github.com/jackulau/coding.sh/blob/main/docs/ADAPTERS.md#known-breakage-to-design-around) - **Tests.** The viewer's browser tests no longer hang when Chrome closes its DevTools connection before it answers: a waiting call fails at once, and any call fails after 30 s. The helper also lets go of Chrome's pipe after closing it, and the CI runner makes each test file exit once its tests are done, so a handle left open can no longer hold a Windows job until its time limit. [Contributing](https://github.com/jackulau/coding.sh/blob/main/CONTRIBUTING.md) - **Site.** The docs sidebar and the phone menu open on the current page, and the phone menu fits the screen. A terminal block no longer touches the paragraph above it, wide code blocks scroll by keyboard, code copy selects the code when the browser blocks the clipboard, and search results open without a redirect. [Docs](https://coding.sh/docs) diff --git a/apps/site/src/pages/docs/mcp.astro b/apps/site/src/pages/docs/mcp.astro index a4007766..9d6654e3 100644 --- a/apps/site/src/pages/docs/mcp.astro +++ b/apps/site/src/pages/docs/mcp.astro @@ -109,7 +109,27 @@ const gemini = `// ~/.gemini/settings.json

Only eval_from_session, eval_init, score_add and exp_session_note write. Agents ask - you before eval_from_session and eval_init. Notes have secrets redacted. + you before eval_from_session and eval_init. Notes have secrets redacted. eval_init and + score_add are marked destructive, because eval_init with force replaces a suite and a new + score replaces the one stored for the same target, name, source and person. +

+

+ A tool answer stays under 25,000 characters. When the rows asked for would be longer, the answer has fewer rows and a + limitUsed field, and nextCursor starts at the first row left out. The limit drops rows; it does not cut text. + session_replay shows at most 16 KB of a file and of each diff, and says when it cut one. +

+ +

Protocol

+

+ The server speaks MCP over stdio. It accepts the stateless 2026-07-28 revision (server/discover, no handshake) and + the initialize handshake for 2025-11-25, 2025-06-18 and 2025-03-26. An initialize that asks for another + version gets 2025-11-25. In a 2025-03-26 session it accepts JSON-RPC batches; in later versions batches are an error. +

+

+ In 2026-07-28, list and read results carry ttlMs and cacheScope: the tool, prompt and template lists + are public for an hour, and resources are private and stale at once. A request cancelled with + notifications/cancelled gets no reply. Requests run one at a time, so a cancelled request that has not started + never runs.

Prompts and resources

From 5d8692106373916c92fae361deb7a35e5ee6c07d Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Wed, 11 Nov 2026 22:03:30 -0600 Subject: [PATCH 05/11] fix(mcp): treat a null-id notification as one, ignore cancel for initialize, cache discover for 5 minutes, cut text on a character boundary, and wait on signals in the cancel test --- packages/coding.sh/src/mcp/protocol.ts | 25 +++++-- .../coding.sh/test/mcp/_blocking-server.ts | 5 +- packages/coding.sh/test/mcp/protocol.test.ts | 74 ++++++++++++++++--- 3 files changed, 84 insertions(+), 20 deletions(-) diff --git a/packages/coding.sh/src/mcp/protocol.ts b/packages/coding.sh/src/mcp/protocol.ts index 1701129f..b1382f1c 100644 --- a/packages/coding.sh/src/mcp/protocol.ts +++ b/packages/coding.sh/src/mcp/protocol.ts @@ -154,16 +154,25 @@ export const ERROR_TEXT_MAX = 4000; export function clipText(s: string, max = ERROR_TEXT_MAX): string { if (s.length <= max) return s; const keep = Math.floor((max - 40) / 2); - return `${s.slice(0, keep)} … [${s.length - 2 * keep} characters cut] … ${s.slice(-keep)}`; + const head = cutChars(s, keep); + const tail = s.slice(-keep).replace(/^[\uDC00-\uDFFF]/, ""); + return `${head} … [${s.length - head.length - tail.length} characters cut] … ${tail}`; +} + +/** The first `max` characters of `s`, without half of a surrogate pair at the cut. */ +export function cutChars(s: string, max: number): string { + if (s.length <= max) return s; + return s.slice(0, max).replace(/[\uD800-\uDBFF]$/, ""); } /** * Caching hints on 2026-07-28 results (https://modelcontextprotocol.io/specification/2026-07-28/server/utilities/caching): * the tool, prompt and template lists are fixed for the life of the process and hold no user data; - * resources are the user's sessions and change with every capture. + * resources are the user's sessions and change with every capture. server/discover carries the + * instructions, which hold a per-call update notice, so it is cached for less time. */ const CACHE_HINTS: Record = { - "server/discover": { ttlMs: 3_600_000, cacheScope: "public" }, + "server/discover": { ttlMs: 300_000, cacheScope: "public" }, "tools/list": { ttlMs: 3_600_000, cacheScope: "public" }, "prompts/list": { ttlMs: 3_600_000, cacheScope: "public" }, "resources/templates/list": { ttlMs: 3_600_000, cacheScope: "public" }, @@ -233,7 +242,8 @@ export class McpServer { if ("result" in msg || "error" in msg) return null; return errorResponse(idOk ? (msg.id as RpcId) : null, RPC.INVALID_REQUEST, "Invalid request: missing method"); } - if (!hasId) { + // A null id on a notifications/ method is still a notification: no reply. + if (!hasId || (msg.id === null && msg.method.startsWith("notifications/"))) { this.notification(msg.method); return null; } @@ -400,7 +410,7 @@ export function serveStdio(server: McpServer, io: StdioStreams, log: (m: s } }; // Requests waiting or running, by id. A cancel marks the entry; a marked request is never answered. - const pending = new Map(); + const pending = new Map(); // A synchronous store read holds the event loop, so a cancel sent meanwhile is still unread in the // pipe. Two turns of the loop include one poll for input, which reads it before we start or answer. const readInput = () => new Promise((r) => setImmediate(() => setImmediate(r))); @@ -415,11 +425,12 @@ export function serveStdio(server: McpServer, io: StdioStreams, log: (m: s if (parsed && isObj(msg) && msg.method === "notifications/cancelled" && msg.id === undefined) { const id = isObj(msg.params) ? msg.params.requestId : undefined; const entry = typeof id === "string" || typeof id === "number" ? pending.get(JSON.stringify(id)) : undefined; - if (entry) entry.cancelled = true; + // initialize can not be cancelled (lifecycle spec). + if (entry && entry.method !== "initialize") entry.cancelled = true; return; } const key = parsed && isObj(msg) && typeof msg.method === "string" && (typeof msg.id === "string" || typeof msg.id === "number") ? JSON.stringify(msg.id) : undefined; - const entry = { cancelled: false }; + const entry = { cancelled: false, method: isObj(msg) && typeof msg.method === "string" ? msg.method : "" }; if (key) pending.set(key, entry); chain = chain .then(async () => { diff --git a/packages/coding.sh/test/mcp/_blocking-server.ts b/packages/coding.sh/test/mcp/_blocking-server.ts index 6da4a05e..7aa0c86a 100644 --- a/packages/coding.sh/test/mcp/_blocking-server.ts +++ b/packages/coding.sh/test/mcp/_blocking-server.ts @@ -1,6 +1,7 @@ // An MCP server on stdio whose `block` tool holds the event loop for `ms` // milliseconds, the way a synchronous SQLite read does. protocol.test.ts sends -// a cancel while it blocks. +// a cancel while it blocks. It writes "block-start" to stderr as it starts blocking and +// "block-end" when it stops, so the test waits on those lines, not on timers. import { McpServer, serveStdio } from "../../src/mcp/protocol.ts"; const server = new McpServer({ @@ -16,9 +17,11 @@ const server = new McpServer({ annotations: { readOnlyHint: true }, run: async (a) => { const end = Date.now() + Number(a.ms ?? 0); + process.stderr.write("block-start\n"); while (Date.now() < end) { // hold the event loop } + process.stderr.write("block-end\n"); return { done: true }; }, }, diff --git a/packages/coding.sh/test/mcp/protocol.test.ts b/packages/coding.sh/test/mcp/protocol.test.ts index bcf34224..925071d5 100644 --- a/packages/coding.sh/test/mcp/protocol.test.ts +++ b/packages/coding.sh/test/mcp/protocol.test.ts @@ -4,8 +4,9 @@ import assert from "node:assert/strict"; import { spawn } from "node:child_process"; import { PassThrough } from "node:stream"; +import { fileURLToPath } from "node:url"; import { describe, it } from "node:test"; -import { McpServer, serveStdio, ToolError, type ToolDef } from "../../src/mcp/protocol.ts"; +import { clipText, cutChars, McpServer, serveStdio, ToolError, type ToolDef } from "../../src/mcp/protocol.ts"; type Msg = { jsonrpc: string; id: unknown; result?: Record; error?: { code: number; message: string; data?: Record } }; @@ -100,6 +101,7 @@ describe("caching hints (2026-07-28)", () => { assert.equal(typeof r?.result?.ttlMs, "number", `${method} ttlMs`); assert.ok((r!.result!.ttlMs as number) >= 0, `${method} ttlMs >= 0`); assert.equal(r?.result?.cacheScope, scope, `${method} cacheScope`); + if (method === "server/discover") assert.equal(r?.result?.ttlMs, 300_000, "discover carries a per-call update notice, so it is cached for 5 minutes"); } // Legacy results stay as they were. w.send({ jsonrpc: "2.0", id: "legacy", method: "tools/list" }); @@ -144,9 +146,11 @@ describe("cancellation (notifications/cancelled)", () => { describe("cancellation while a tool holds the event loop", () => { // A synchronous store read blocks the loop, so a cancel sits unread in the pipe until it ends. it("reads the cancel before it answers or starts the next request", async () => { - const child = spawn(process.execPath, ["--no-warnings", new URL("./_blocking-server.ts", import.meta.url).pathname], { stdio: ["pipe", "pipe", "inherit"] }); + const child = spawn(process.execPath, ["--no-warnings", fileURLToPath(new URL("./_blocking-server.ts", import.meta.url))], { stdio: ["pipe", "pipe", "pipe"] }); const ids: unknown[] = []; + const marks: string[] = []; let buf = ""; + let errBuf = ""; child.stdout.setEncoding("utf8"); child.stdout.on("data", (c: string) => { buf += c; @@ -156,24 +160,37 @@ describe("cancellation while a tool holds the event loop", () => { buf = buf.slice(i + 1); } }); + child.stderr.setEncoding("utf8"); + child.stderr.on("data", (c: string) => { + errBuf += c; + let i: number; + while ((i = errBuf.indexOf("\n")) !== -1) { + marks.push(errBuf.slice(0, i)); + errBuf = errBuf.slice(i + 1); + } + }); const send = (m: unknown) => child.stdin.write(JSON.stringify(m) + "\n"); - const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)); + // Waits for the child's signal, not for a time: the machine may be loaded. + const until = async (what: string, done: () => boolean) => { + for (let i = 0; i < 3000 && !done(); i++) await new Promise((r) => setTimeout(r, 10)); + assert.ok(done(), `timed out waiting for ${what}`); + }; const exited = new Promise((r) => child.on("exit", r)); send({ jsonrpc: "2.0", id: "warm", method: "ping" }); - for (let i = 0; i < 200 && !ids.includes("warm"); i++) await wait(10); + await until("warm", () => ids.includes("warm")); // "run" is cancelled while it blocks; "queued" is cancelled while it waits behind "first". - send({ jsonrpc: "2.0", id: "run", method: "tools/call", params: { name: "block", arguments: { ms: 400 } } }); - await wait(100); + send({ jsonrpc: "2.0", id: "run", method: "tools/call", params: { name: "block", arguments: { ms: 1500 } } }); + await until("run to block", () => marks.length === 1); send({ jsonrpc: "2.0", method: "notifications/cancelled", params: { requestId: "run" } }); - await wait(500); - send({ jsonrpc: "2.0", id: "first", method: "tools/call", params: { name: "block", arguments: { ms: 400 } } }); - await wait(100); + await until("run to end", () => marks.length === 2); + send({ jsonrpc: "2.0", id: "first", method: "tools/call", params: { name: "block", arguments: { ms: 1500 } } }); + await until("first to block", () => marks.length === 3); send({ jsonrpc: "2.0", id: "queued", method: "ping" }); - await wait(50); send({ jsonrpc: "2.0", method: "notifications/cancelled", params: { requestId: "queued" } }); - await wait(500); + await until("first to end", () => marks.length === 4); + await until("first's reply", () => ids.includes("first")); send({ jsonrpc: "2.0", id: "last", method: "ping" }); - for (let i = 0; i < 200 && !ids.includes("last"); i++) await wait(10); + await until("last", () => ids.includes("last")); child.stdin.end(); await exited; assert.deepEqual(ids, ["warm", "first", "last"]); @@ -199,6 +216,39 @@ describe("JSON-RPC edges", () => { await w.close(); }); + it("handles a null id on a notifications/ method as a notification", async () => { + const w = world(); + const n = w.out.length; + w.send({ jsonrpc: "2.0", id: null, method: "notifications/initialized" }); + w.send({ jsonrpc: "2.0", id: null, method: "notifications/cancelled", params: { requestId: 9 } }); + w.send({ jsonrpc: "2.0", id: "after", method: "ping" }); + assert.ok(await w.reply("after")); + assert.equal(w.out.length, n + 1, "the notifications got no reply"); + assert.equal(w.server.initialized, true); + w.send({ jsonrpc: "2.0", id: null, method: "ping" }); + assert.equal((await w.reply(null))?.error?.code, -32600, "other null-id requests stay invalid"); + await w.close(); + }); + + it("ignores a cancel for initialize", async () => { + const w = world(); + w.send({ jsonrpc: "2.0", id: "init", method: "initialize", params: { protocolVersion: "2025-03-26", capabilities: {}, clientInfo: { name: "t", version: "0" } } }); + w.send({ jsonrpc: "2.0", method: "notifications/cancelled", params: { requestId: "init" } }); + assert.ok((await w.reply("init"))?.result?.protocolVersion, "initialize is still answered"); + await w.close(); + }); + + it("clips text without splitting a surrogate pair", () => { + const e = "\u{1F600}"; + for (let pad = 0; pad < 2; pad++) { + const s = "a".repeat(pad) + e.repeat(100); + const clipped = clipText(s, 100); + assert.ok(!/[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? { const w = world(); w.send([{ jsonrpc: "2.0", id: "x", method: "ping" }]); From edc12825341b01c0cba8cfc3a112d451b6abfddb Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Wed, 11 Nov 2026 22:22:48 -0600 Subject: [PATCH 06/11] fix(mcp): keep every session_replay view under 25,000 characters The step list, a step, a step with a file, a range and a fork page their files with limit and cursor and cut the text that does not fit, saying so with truncated. fitPage returns the largest page that fits, or the page whole with truncated when the fixed part is the long one. benchmarks has a default limit, and every tool that can return limitUsed declares it. --- packages/coding.sh/src/mcp/replay-page.ts | 135 ++++++++++++++++++ packages/coding.sh/src/mcp/server.ts | 4 +- packages/coding.sh/src/mcp/tools.ts | 75 ++++++---- packages/coding.sh/test/mcp/fit-page.test.ts | 65 +++++++++ packages/coding.sh/test/mcp/limits.test.ts | 24 +++- .../coding.sh/test/mcp/replay-limit.test.ts | 117 +++++++++++++++ packages/coding.sh/test/mcp/toolevals.test.ts | 6 +- 7 files changed, 395 insertions(+), 31 deletions(-) create mode 100644 packages/coding.sh/src/mcp/replay-page.ts create mode 100644 packages/coding.sh/test/mcp/fit-page.test.ts create mode 100644 packages/coding.sh/test/mcp/replay-limit.test.ts diff --git a/packages/coding.sh/src/mcp/replay-page.ts b/packages/coding.sh/src/mcp/replay-page.ts new file mode 100644 index 00000000..2cc955a9 --- /dev/null +++ b/packages/coding.sh/src/mcp/replay-page.ts @@ -0,0 +1,135 @@ +// session_replay's answers kept under the character limit. The file lists (`tree`, `changes`, `files`, +// `unchanged`) are paged with limit and cursor; the text in them (diffs, file content, tool output) is +// cut to fit what is left, and the answer says so (`truncated`, plus the field's own flag). +import type { RangeView, StepView } from "../core/replay.ts"; +import { cutChars, toolText } from "./protocol.ts"; + +type Obj = Record; + +/** Text is never cut below this many characters when it sits in a row of a list, or outside one. */ +const ROW_FLOOR = 600; +const FIXED_FLOOR = 2000; +/** The most a diff or file shows (the replay is loaded with the same cap). */ +export const TEXT_MAX = 16 * 1024; + +const FLAG: Record = { diff: "diffTruncated", content: "contentTruncated", output: "outputTruncated", text: "textTruncated" }; + +const NOTE = "Text was cut to keep the answer under 25,000 characters (the fields marked truncated). A smaller limit gives each diff more room (limit 1: up to 16 KB); step with file gives one file's content."; + +interface Slot { + owner: Obj; + key: string; + floor: number; +} + +function collect(v: unknown, inRow: boolean, out: Slot[], depth = 0): void { + if (depth > 8 || v === null || typeof v !== "object") return; + if (Array.isArray(v)) { + for (const x of v) collect(x, true, out, depth + 1); + return; + } + const o = v as Obj; + for (const [k, x] of Object.entries(o)) { + if (typeof x === "string") { + const floor = inRow ? ROW_FLOOR : FIXED_FLOOR; + if (x.length > floor) out.push({ owner: o, key: k, floor }); + } else collect(x, inRow, out, depth + 1); + } +} + +/** Whether any field in `v` is flagged as cut (`diffTruncated`, `contentTruncated`, …). */ +function hasFlag(v: unknown, depth = 0): boolean { + if (depth > 8 || v === null || typeof v !== "object") return false; + if (Array.isArray(v)) return v.some((x) => hasFlag(x, depth + 1)); + return Object.entries(v as Obj).some(([k, x]) => (k.endsWith("Truncated") && x === true) || hasFlag(x, depth + 1)); +} + +/** + * Cuts the longest strings in `page` (in place) until its text is at most `limit` characters, or no string + * is left above its floor. The longest are cut down to a common length, so short text stays whole. + * Sets `truncated` and `note` on the page, and the field's own flag (`diffTruncated`, …) beside a cut one. + */ +export function fitTexts(page: Obj, limit: number): Obj { + let len = toolText(page).length; + if (len <= limit) return hasFlag(page) ? { ...page, truncated: true, note: NOTE } : page; + const slots: Slot[] = []; + collect(page, false, slots); + let margin = 1.1; + for (let round = 0; round < 6 && len > limit && slots.length; round++, margin += 0.4) { + const need = Math.ceil((len - limit) * margin) + 64; + const size = (s: Slot) => (s.owner[s.key] as string).length; + const after = (s: Slot, level: number) => Math.max(s.floor, Math.min(size(s), level)); + const saved = (level: number) => slots.reduce((sum, s) => sum + size(s) - after(s, level), 0); + let lo = 0; + let hi = Math.max(...slots.map(size)); + // The highest level that still saves `need` characters (or the lowest there is). + while (lo < hi) { + const mid = Math.ceil((lo + hi) / 2); + if (saved(mid) >= need) lo = mid; + else hi = mid - 1; + } + let cut = false; + for (const s of slots) { + const to = after(s, lo); + if (to >= size(s)) continue; + s.owner[s.key] = cutChars(s.owner[s.key] as string, to); + const flag = FLAG[s.key]; + if (flag) s.owner[flag] = true; + cut = true; + } + if (!cut) break; + len = toolText({ ...page, truncated: true, note: NOTE }).length; + } + return { ...page, truncated: true, note: NOTE }; +} + +/** A diff cut to `max` characters at a line end, flagged when cut. */ +function capDiff(o: { diff?: string; diffTruncated?: boolean }, max: number): void { + if (o.diff === undefined || o.diff.length <= max) return; + let cut = cutChars(o.diff, max); + const nl = cut.lastIndexOf("\n"); + if (nl > 0) cut = cut.slice(0, nl + 1); + o.diff = cut; + o.diffTruncated = true; +} + +/** Characters each row's diff may use on a page of n rows. */ +const perRow = (n: number) => Math.min(TEXT_MAX, Math.max(ROW_FLOOR, Math.floor(18_000 / n))); + +/** A page of a step's view: the file lists from `offset`; the step's own detail and file only on the first page. */ +export function pageStep(view: StepView, offset: number, n: number, limit: number): Obj { + const first: Obj = {}; + if (offset === 0) { + first.detail = view.detail; + if (view.file) first.file = view.file; + } + const page = structuredClone({ + sessionId: view.sessionId, + steps: view.steps, + step: view.step, + ...first, + tree: view.tree.slice(offset, offset + n), + changes: view.changes.slice(offset, offset + n), + treeTotal: view.tree.length, + changesTotal: view.changes.length, + }) as Obj & { changes: { diff?: string; diffTruncated?: boolean }[]; nextCursor?: string }; + for (const c of page.changes) capDiff(c, perRow(n)); + if (offset + n < Math.max(view.tree.length, view.changes.length)) page.nextCursor = String(offset + n); + return fitTexts(page, limit); +} + +/** A page of a range's view. */ +export function pageRange(view: RangeView, offset: number, n: number, limit: number): Obj { + const page = structuredClone({ + sessionId: view.sessionId, + from: view.from, + to: view.to, + files: view.files.slice(offset, offset + n), + unchanged: view.unchanged.slice(offset, offset + n), + filesTotal: view.files.length, + unchangedTotal: view.unchanged.length, + }) as Obj & { files: { diff?: string; diffTruncated?: boolean }[]; nextCursor?: string }; + for (const f of page.files) capDiff(f, perRow(n)); + if (offset + n < Math.max(view.files.length, view.unchanged.length)) page.nextCursor = String(offset + n); + return fitTexts(page, limit); +} diff --git a/packages/coding.sh/src/mcp/server.ts b/packages/coding.sh/src/mcp/server.ts index 150d44f1..a2613ebe 100644 --- a/packages/coding.sh/src/mcp/server.ts +++ b/packages/coding.sh/src/mcp/server.ts @@ -42,9 +42,9 @@ const BASE_INSTRUCTIONS = [ "For trends use failures (raw tool errors and flags), latency, usage (tokens/cost) and subagents. experiments_list and experiment_compare read eval results.", "In an experiment session, call exp_session_note with the hypothesis before each run and the reading after it; exp_session_summary returns the timeline.", "To build evals for the user, call eval_init (a suite in evals// from failed sessions, a skill's trigger prompts, or cases you write), then tell them its run command.", - "eval_from_session, eval_init and exp_session_note write; ask the user before calling eval_from_session or eval_init with confirm: true.", + "eval_from_session, eval_init, score_add and exp_session_note write; ask the user before calling eval_from_session or eval_init with confirm: true.", 'A result with pending: {files, indexing} was answered before every capture was indexed: it covers what is indexed so far; call again shortly for the rest.', - "Answers stay under 25,000 characters: one that would be longer has fewer rows and limitUsed; follow nextCursor for the rest.", + "Answers stay under 25,000 characters. A tool that pages returns nextCursor for the rest; the others return fewer rows, with limitUsed.", ].join(" "); /** Server instructions, with a one-line notice when the update checker found a newer release. */ diff --git a/packages/coding.sh/src/mcp/tools.ts b/packages/coding.sh/src/mcp/tools.ts index 3795a49a..90c6332a 100644 --- a/packages/coding.sh/src/mcp/tools.ts +++ b/packages/coding.sh/src/mcp/tools.ts @@ -39,6 +39,7 @@ import { import { sessionDigest } from "./digest.ts"; import { facetCounts, searchWhere } from "../query/run.ts"; import { andQuery, FACET_KEYS, parseWhere, QueryError, QueryUnsupported, type Query } from "../query/where.ts"; +import { fitTexts, pageRange, pageStep } from "./replay-page.ts"; import { toolText, ToolError, type ToolDef } from "./protocol.ts"; import { scoreTools } from "../scores/mcp.ts"; @@ -135,38 +136,57 @@ function treeNodeJson(n: TreeNode, top = false): Obj { export const CHARACTER_LIMIT = 25_000; /** - * The page `render(limit)` makes, with fewer rows while its text is over CHARACTER_LIMIT. Rows are - * dropped, never cut, so a cursor of offset + n still points at the first row left out. A shortened - * page carries `limitUsed`. When no smaller limit makes it shorter (the size is not in the rows, or - * one row is that long), the shortest page comes back whole. + * The page `render(limit)` makes, with fewer rows while its text is over CHARACTER_LIMIT: the largest + * n whose page fits. Rows are dropped, never cut, so a cursor of offset + n still points at the first + * row left out. A shortened page carries `limitUsed`; one that already fit is returned as it is. When + * not even one row fits (the size is in the fixed part of the answer), the page for `limit` comes back + * whole with `truncated: true`. */ export async function fitPage(limit: number, render: (n: number) => Obj | Promise): Promise { - let page = await render(limit); - let len = toolText(page).length; - let used: number | undefined; - // n can start above the rows left (the last page), so a try that is no shorter only halves n. - for (let n = limit; len > CHARACTER_LIMIT && n > 1; ) { - n = Math.max(1, Math.min(Math.floor(n / 2), Math.floor((n * CHARACTER_LIMIT * 0.9) / len))); - const smaller = await render(n); - const smallerLen = toolText(smaller).length; - if (smallerLen < len) [page, len, used] = [smaller, smallerLen, n]; + const first = await render(limit); + const len = toolText(first).length; + if (len <= CHARACTER_LIMIT) return first; + const tooLong = (): Obj => ({ ...first, truncated: true, note: `This answer is over ${CHARACTER_LIMIT.toLocaleString("en-US")} characters even with one row: most of it is not in the rows.` }); + if (limit <= 1) return tooLong(); + const fits = async (n: number): Promise => { + const page = { ...(await render(n)), limitUsed: n }; + return toolText(page).length <= CHARACTER_LIMIT ? page : undefined; + }; + // One row first: when it does not fit, no page does. + let best = await fits(1); + if (!best) return tooLong(); + // The length grows with n: bisect for the largest n that fits, probing an estimate first. + let lo = 1; + let hi = limit; + for (let n = Math.min(limit - 1, Math.max(2, Math.floor((limit * CHARACTER_LIMIT * 0.9) / len))); hi - lo > 1; n = lo + Math.floor((hi - lo) / 2)) { + const page = await fits(n); + if (page) [lo, best] = [n, page]; + else hi = n; } - return used === undefined ? page : { ...page, limitUsed: used }; + return best; } +/** The rows a tool with a `limit` returns when none is passed, where the schema's default does not say. */ +const LIMIT_DEFAULTS: Record number> = { benchmarks: (a) => (a.metric ? 30 : 5) }; + /** A read-only tool with a `limit` argument, run again with a smaller limit when its answer is too long. */ function withCharacterLimit(t: ToolDef): ToolDef { const limit = (t.inputSchema.properties as Record | undefined)?.limit; if (!limit || t.annotations.readOnlyHint !== true || t.annotations.idempotentHint !== true) return t; + const outProps = (t.outputSchema.properties ?? {}) as Obj; return { ...t, + outputSchema: { ...t.outputSchema, properties: { ...outProps, limitUsed: I } }, run(a, c) { - const asked = typeof a.limit === "number" ? a.limit : typeof limit.default === "number" ? limit.default : undefined; - return asked === undefined ? t.run(a, c) : fitPage(asked, (n) => t.run(n === asked ? a : { ...a, limit: n }, c)); + const asked = typeof a.limit === "number" ? a.limit : typeof limit.default === "number" ? limit.default : LIMIT_DEFAULTS[t.name]?.(a); + return asked === undefined ? t.run(a, c) : fitPage(asked, (n) => t.run(n === a.limit ? a : { ...a, limit: n }, c)); }, }; } +/** What session_replay's text may take of CHARACTER_LIMIT: the rest is for `limitUsed` and the page's own keys. */ +const REPLAY_TEXT_LIMIT = CHARACTER_LIMIT - 60; + const READ = { readOnlyHint: true, destructiveHint: false, idempotentHint: true, openWorldHint: false } as const; function strArg(a: Obj, k: string): string | undefined { @@ -448,7 +468,7 @@ function toolDefs(): ToolDef[] { name: "session_replay", title: "Replay a session", description: - "A session as numbered steps, with the files the agent touched rebuilt from its tool calls as of any step. No step: the step list.", + "A session as numbered steps, with the files the agent touched rebuilt from its tool calls as of any step. No step: the step list. Paged by files; cut text says truncated.", inputSchema: input( { sessionId: SESSION_ID, @@ -457,12 +477,12 @@ function toolDefs(): ToolDef[] { from: { type: "integer", minimum: 1, description: "With to: the first step of a range." }, to: { type: "integer", minimum: 1, description: "With from: the last step (included); each file's diff over the range." }, fork: { type: "object", description: 'With step: send its model call again with these files ({"AGENTS.md": text}); recorded vs forked next action. Calls the model.' }, - limit: LIMIT(100, 1000), + limit: { ...LIMIT(100, 1000), description: "Max files per page; fewer give each diff more room." }, cursor: CURSOR, }, ["sessionId"], ), - outputSchema: out({ sessionId: S, steps: { description: "The step list, or (with step) how many steps there are." }, files: { type: "array" }, total: I, step: { type: "object" }, changes: { type: "array" }, tree: { type: "array" }, file: { type: "object" }, from: I, to: I, nextCursor: S, trials: { type: "array" } }, ["sessionId"]), + outputSchema: out({ sessionId: S, steps: { description: "The step list, or (with step) how many steps there are." }, files: { type: "array" }, total: I, treeTotal: I, changesTotal: I, filesTotal: I, unchanged: arr(S), unchangedTotal: I, step: { type: "object" }, changes: { type: "array" }, tree: { type: "array" }, file: { type: "object" }, from: I, to: I, nextCursor: S, limitUsed: I, truncated: { type: "boolean" }, note: S, trials: { type: "array" } }, ["sessionId"]), annotations: { ...READ, idempotentHint: false, openWorldHint: true }, async run(a, c) { const ctx = await c.store(); @@ -480,21 +500,26 @@ function toolDefs(): ToolDef[] { const step = stepArg("step"); const from = stepArg("from"); const to = stepArg("to"); + const offset = cursorArg(a); + const limit = numArg(a, "limit", 100); if (from !== undefined || to !== undefined) { if (from === undefined || to === undefined) throw new ToolError("Pass both from and to for a range."); - return { ...(await replay.range(from, to)) }; + const range = await replay.range(from, to); + return fitPage(limit, (n) => pageRange(range, offset, n, REPLAY_TEXT_LIMIT)); } if (a.fork !== undefined) { if (step === undefined) throw new ToolError("fork needs a step."); - return { ...(await forkTool(ctx, c.env, s, replay, step, a.fork)) }; + return fitTexts({ ...(await forkTool(ctx, c.env, s, replay, step, a.fork)) }, REPLAY_TEXT_LIMIT); } const file = strArg(a, "file"); - if (step !== undefined) return { ...(await replay.at(step, { file })) }; + if (step !== undefined) { + const view = await replay.at(step, { file }); + return fitPage(limit, (n) => pageStep(view, offset, n, REPLAY_TEXT_LIMIT)); + } if (file) throw new ToolError("file needs a step."); const index = replay.index(); - const offset = cursorArg(a); - const limit = numArg(a, "limit", 100); - return fitPage(limit, (n) => ({ ...index, steps: index.steps.slice(offset, offset + n), total, ...(offset + n < total ? { nextCursor: String(offset + n) } : {}) })); + const more = (n: number) => offset + n < Math.max(total, index.files.length); + return fitPage(limit, (n) => ({ ...index, steps: index.steps.slice(offset, offset + n), files: index.files.slice(offset, offset + n), total, filesTotal: index.files.length, ...(more(n) ? { nextCursor: String(offset + n) } : {}) })); }, }, { diff --git a/packages/coding.sh/test/mcp/fit-page.test.ts b/packages/coding.sh/test/mcp/fit-page.test.ts new file mode 100644 index 00000000..d4a3a1c9 --- /dev/null +++ b/packages/coding.sh/test/mcp/fit-page.test.ts @@ -0,0 +1,65 @@ +// fitPage: the largest page that fits CHARACTER_LIMIT, `limitUsed` only when it shrank, and a +// `truncated` page when the fixed part of the answer is the part that is too long. +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { CHARACTER_LIMIT, fitPage } from "../../src/mcp/tools.ts"; +import { toolText } from "../../src/mcp/protocol.ts"; + +type Obj = Record; + +/** A page of `n` rows of `rowLen` characters out of `total`, after `fixed` characters that are always there. */ +const pager = + (total: number, rowLen: number, fixed = 0, tail = false) => + (n: number): Obj => { + const rows = Array.from({ length: Math.min(n, total) }, (_, i) => `${tail ? total - Math.min(n, total) + i : i}`.padEnd(rowLen, "x")); + const page: Obj = { head: "h".repeat(fixed), rows }; + if (n < total) page.nextCursor = String(n); + return page; + }; + +describe("fitPage", () => { + it("leaves a page that fits alone, without limitUsed, and calls render once", async () => { + let calls = 0; + const render = pager(10, 100); + const page = await fitPage(10, (n) => (calls++, render(n))); + assert.equal(calls, 1); + assert.equal("limitUsed" in page, false); + assert.equal((page.rows as string[]).length, 10); + }); + + it("returns the largest page that fits, and says which limit it used", async () => { + const render = pager(500, 1000); + const page = await fitPage(500, render); + const n = page.limitUsed as number; + assert.ok(toolText(page).length <= CHARACTER_LIMIT); + assert.equal((page.rows as string[]).length, n); + assert.ok(toolText({ ...render(n + 1), limitUsed: n + 1 }).length > CHARACTER_LIMIT, "one more row would not fit"); + assert.ok(n >= 20 && n < 26, `about 25 rows of 1,000 characters (${n})`); + }); + + it("works on a page taken from the tail", async () => { + const render = pager(500, 1000, 0, true); + const page = await fitPage(500, render); + const rows = page.rows as string[]; + assert.ok(toolText(page).length <= CHARACTER_LIMIT); + assert.equal(rows.length, page.limitUsed); + assert.ok(rows[0]!.startsWith(String(500 - rows.length)), "the newest rows stay"); + }); + + it("a last page shorter than the limit still counts as fitting", async () => { + const page = await fitPage(1000, pager(30, 100)); + assert.equal("limitUsed" in page, false); + }); + + it("returns the page whole with truncated when the fixed part is over the limit", async () => { + let calls = 0; + const render = pager(100, 10, CHARACTER_LIMIT + 5000); + const page = await fitPage(100, (n) => (calls++, render(n))); + assert.equal(page.truncated, true); + assert.equal("limitUsed" in page, false); + assert.equal((page.rows as string[]).length, 100, "the page that was asked for"); + assert.ok(calls <= 2, `no pointless retries (${calls} calls)`); + const one = await fitPage(1, render); + assert.equal(one.truncated, true); + }); +}); diff --git a/packages/coding.sh/test/mcp/limits.test.ts b/packages/coding.sh/test/mcp/limits.test.ts index e4231a84..bbaae65d 100644 --- a/packages/coding.sh/test/mcp/limits.test.ts +++ b/packages/coding.sh/test/mcp/limits.test.ts @@ -41,6 +41,18 @@ before(async () => { evs.push({ ...base, id: "big-end", ts: t0 + 20 * CALLS, kind: "session.end", payload: {} }); await ctx.index.putEvents(evs); await ctx.index.upsertSession(buildSession(evs)); + // Sixty sessions with long ids and long prompts: a search for "needle" finds one hit in each, with rows + // too long for 50 of them to fit. + for (let k = 0; k < 60; k++) { + const sid = `sess-long-${k}-${"q".repeat(150)}`; + const lb = { ...base, sessionId: sid }; + const lev: AgentEvent[] = [ + { ...lb, id: `l${k}-0`, ts: t0 + k, kind: "session.start", payload: {} }, + { ...lb, id: `l${k}-1`, ts: t0 + k + 1, kind: "user.message", payload: { text: `needle ${"w".repeat(900)}` } }, + ]; + await ctx.index.putEvents(lev); + await ctx.index.upsertSession(buildSession(lev)); + } await ctx.close(); c = new RawClient(w.env, w.repo); await c.initialize(); @@ -92,9 +104,17 @@ describe("the 25,000-character limit", () => { assert.equal(steps, total, "the pages hold every step once"); }); - it("search, sessions_list and session_current stay under the limit", async () => { + it("search returns fewer hits when the rows are long, and the cursor walks all of them", async () => { + const first = await call("search", { query: "needle", limit: 50 }); + const sc = first.structuredContent!; + assert.ok(first.content[0]!.text.length <= CHARACTER_LIMIT, `search is ${first.content[0]!.text.length} chars`); + assert.ok((sc.limitUsed as number) < 50, `the page was shortened (limitUsed ${String(sc.limitUsed)})`); + assert.equal((sc.hits as unknown[]).length, sc.limitUsed, "limitUsed is the number of hits"); + assert.equal(typeof sc.nextCursor, "string", "the rest is behind nextCursor"); + }); + + it("sessions_list and session_current stay under the limit", async () => { for (const [name, args] of [ - ["search", { query: "needle", limit: 50 }], ["sessions_list", { limit: 100, includeSubagents: true }], ["session_current", { limit: 100 }], ] as const) { diff --git a/packages/coding.sh/test/mcp/replay-limit.test.ts b/packages/coding.sh/test/mcp/replay-limit.test.ts new file mode 100644 index 00000000..99bc1a15 --- /dev/null +++ b/packages/coding.sh/test/mcp/replay-limit.test.ts @@ -0,0 +1,117 @@ +// session_replay stays under the 25,000-character limit in every view, with a session that touched +// 300 files of about 50 KB each: the step list, a step, a step with a file, and ranges. Pages by +// limit and cursor reach every file exactly once, and cut text says so. +import assert from "node:assert/strict"; +import { after, before, describe, it } from "node:test"; +import { openContext } from "../../src/core/context.ts"; +import { buildSession } from "../../src/core/sessions.ts"; +import { CHARACTER_LIMIT } from "../../src/mcp/tools.ts"; +import type { AgentEvent } from "../../src/schema/index.ts"; +import { RawClient, cleanup, makeWorld, seed, type World } from "./helpers.ts"; + +const ID = "sess-files"; +const FILES = 300; +const path = (i: number) => `src/gen/file-${String(i).padStart(3, "0")}.ts`; + +let w: World; +let c: RawClient; + +type Sc = Record; + +async function call(args: Record): Promise<{ sc: Sc; len: number }> { + const r = await c.call("session_replay", { sessionId: ID, ...args }); + assert.notEqual(r.isError, true, r.content[0]?.text.slice(0, 300)); + return { sc: r.structuredContent as Sc, len: r.content[0]!.text.length }; +} + +/** Every page of a view, following nextCursor. Each page is checked against the limit. */ +async function pages(args: Record): Promise { + const out: Sc[] = []; + let cursor: string | undefined; + for (let i = 0; i < 400; i++) { + const next = { ...args }; + if (cursor) next.cursor = cursor; + const { sc, len } = await call(next); + assert.ok(len <= CHARACTER_LIMIT, `page ${i} of ${JSON.stringify(args)} is ${len} characters`); + out.push(sc); + cursor = sc.nextCursor; + if (!cursor) return out; + } + throw new Error("the cursor never ended"); +} + +const once = (xs: string[], n: number, what: string) => { + assert.equal(xs.length, n, `${what}: ${xs.length} entries`); + assert.equal(new Set(xs).size, n, `${what}: each exactly once`); +}; + +before(async () => { + w = makeWorld(); + await seed(w); + const ctx = await openContext({ env: w.env, cwd: w.root }); + const t0 = w.now - 3 * 3600e3; + const base = { v: 0 as const, sessionId: ID, agent: "claude-code", cwd: w.repo, model: "claude-opus-4-5" }; + const evs: AgentEvent[] = [{ ...base, id: "f-0", ts: t0, kind: "session.start", payload: {} }, { ...base, id: "f-1", ts: t0 + 1, kind: "user.message", payload: { text: "Generate the files" } }]; + for (let i = 0; i < FILES; i++) { + // About 50 KB of distinct lines per file. + const content = Array.from({ length: 1500 }, (_, l) => `export const v${i}_${l} = ${l * 7919 + i};`).join("\n") + "\n"; + evs.push({ ...base, id: `f-s${i}`, ts: t0 + 10 + i * 10, kind: "tool.start", payload: { tool: "Write", toolUseId: `w${i}`, input: { file_path: path(i), content } } }); + evs.push({ ...base, id: `f-e${i}`, ts: t0 + 15 + i * 10, kind: "tool.end", payload: { tool: "Write", toolUseId: `w${i}`, ok: true }, durationMs: 5 }); + evs.push({ ...base, id: `f-c${i}`, ts: t0 + 16 + i * 10, kind: "file.change", payload: { path: path(i), op: "add" } }); + } + evs.push({ ...base, id: "f-end", ts: t0 + 20 * FILES, kind: "session.end", payload: {} }); + await ctx.index.putEvents(evs); + await ctx.index.upsertSession(buildSession(evs)); + await ctx.close(); + c = new RawClient(w.env, w.repo); + await c.initialize(); +}); + +after(async () => { + await c?.close(); + cleanup(); +}); + +describe("session_replay under the 25,000-character limit", () => { + it("pages the step list, with its files, once each", async () => { + const all = await pages({}); + const steps = all.flatMap((p) => (p.steps as { n: number }[]).map((s) => String(s.n))); + const first = all[0]!; + once(steps, first.total, "steps"); + once(all.flatMap((p) => p.files as string[]), FILES, "files"); + assert.ok(all.length > 1, "a long list takes more than one page"); + }); + + it("keeps a step, and a step with a file, under the limit and pages its tree", async () => { + const total = (await call({})).sc.total as number; + assert.equal(total, FILES + 1, "a prompt, then one step per file"); + const step = (await pages({ step: 5 }))[0]!; + assert.ok(step.step, "the step is there"); + const last = await pages({ step: total, file: path(FILES - 1) }); + assert.ok(last[0]!.file, "the first page has the file"); + assert.equal(last[0]!.file.path, path(FILES - 1)); + assert.equal(last[1]?.file, undefined, "later pages hold only the next files"); + once(last.flatMap((p) => (p.tree as { path: string }[]).map((t) => t.path)), FILES, "tree"); + for (const p of last) assert.equal(p.treeTotal, FILES); + assert.ok(last[0]!.file.content.length > 1000, "the file keeps a useful start"); + assert.equal(last[0]!.truncated, true, "cut text is reported"); + assert.equal(last[0]!.file.contentTruncated, true); + }); + + it("keeps ranges under the limit and reaches every changed file once", async () => { + const few = await pages({ from: 1, to: 20 }); + once(few.flatMap((p) => (p.files as { path: string }[]).map((f) => f.path)), 19, "files of steps 1..20 (step 1 is the prompt)"); + const wide = await pages({ from: 1, to: FILES + 1 }); + once(wide.flatMap((p) => (p.files as { path: string }[]).map((f) => f.path)), FILES, "files of 1..300"); + assert.ok(wide.every((p) => p.filesTotal === FILES)); + assert.ok(wide[0]!.truncated === true && wide[0]!.files[0].diffTruncated === true, "cut diffs are flagged"); + assert.ok(wide[0]!.files[0].diff.length > 100, "a diff keeps its start"); + }); + + it("gives one file's diff more room with limit 1", async () => { + const one = await call({ from: 1, to: FILES + 1, limit: 1 }); + assert.ok(one.len <= CHARACTER_LIMIT); + assert.ok(one.sc.files[0].diff.length > 10_000, `a lone diff is ${one.sc.files[0].diff.length} characters`); + assert.equal(one.sc.nextCursor, "1"); + }); +}); diff --git a/packages/coding.sh/test/mcp/toolevals.test.ts b/packages/coding.sh/test/mcp/toolevals.test.ts index c37d25aa..3afb152d 100644 --- a/packages/coding.sh/test/mcp/toolevals.test.ts +++ b/packages/coding.sh/test/mcp/toolevals.test.ts @@ -71,8 +71,10 @@ describe("tool evals against the coding.sh MCP server", () => { // session_tree's paging (limit, cursor, nextCursor, truncated and a page-top node's // parentSessionId) adds about 0.2 KB within the same budget; without it an 800-subagent // tree was one 679 KB reply. - assert.ok(s.surfaceBytes < 42_900, `surface is ${s.surfaceBytes} bytes`); - assert.ok(s.descriptionBytes < 11_750, `descriptions are ${s.descriptionBytes} bytes`); + // limitUsed in the output schema of the 21 tools that can return it, and session_replay's paging + // fields (treeTotal, changesTotal, filesTotal, unchanged, truncated, note), add about 0.95 KB, 0.1 KB of it descriptions. + assert.ok(s.surfaceBytes < 43_900, `surface is ${s.surfaceBytes} bytes`); + assert.ok(s.descriptionBytes < 11_850, `descriptions are ${s.descriptionBytes} bytes`); for (const it of s.items) assert.ok(it.bytes < 4_000, `${it.name} is ${it.bytes} bytes`); }); From b4996711d20f47f32deef25f3139b8706e07b0ad Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Wed, 11 Nov 2026 22:22:48 -0600 Subject: [PATCH 07/11] docs(mcp): say which tools page and which return fewer rows, and list score_add as a writing tool --- CHANGELOG.md | 5 +++-- apps/site/src/pages/docs/mcp.astro | 11 ++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 847215dd..f467265f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ grouped by area, with links to the docs for the details. - **Doctor.** `coding.sh doctor` counts recent model calls past a physical limit (speed, time to first token, duration) and names the session with the most. [Troubleshooting](https://github.com/jackulau/coding.sh/blob/main/docs/troubleshooting.md) - **SDK.** `coding_sh.weights_edit()` in the Python SDK records each weight edit a script makes as one `notification` event with `payload.kind: "weights.edit"`: tensor name, shape, weights changed, L2 and max-abs of the change, and the base and edited sha256. It adds no event kind, and `coding_sh.weights_delta()` computes the counts and norms for small arrays without numpy. [Docs](https://coding.sh/docs/api) - **Evals.** `coding.sh exp compare A B --given ` compares only the cases where that grader passes in both runs, and prints how many it kept and dropped. With a `terminated` grader this separates "finished more answers" from "got more of the finished answers right". [Compare an edited model with its base](https://github.com/jackulau/coding.sh/blob/main/docs/guides/evals.md#compare-an-edited-model-with-its-base) +- **MCP.** In 2026-07-28 requests, list, read and discover results carry `ttlMs` and `cacheScope`: the tool, prompt and template lists are cached for an hour, `server/discover` for five minutes, and sessions are not cached. [MCP server](https://coding.sh/docs/mcp#protocol) ### Changed @@ -36,8 +37,8 @@ grouped by area, with links to the docs for the details. ### Fixed -- **MCP.** In 2026-07-28 requests, list, read and discover results carry `ttlMs` and `cacheScope`. A request cancelled with `notifications/cancelled` gets no reply, a cursor on `tools/list`, `prompts/list` or `resources/templates/list` is an error, a request with a null id gets an error instead of no answer, and a 2025-03-26 session accepts batches. [MCP server](https://coding.sh/docs/mcp#protocol) -- **MCP.** Tool answers stay under 25,000 characters: a longer page returns fewer rows with `limitUsed`, and `nextCursor` goes on from the first row left out. `session_replay`'s step list was 227,000. An error quoting a huge argument is cut to 4,000 characters, and `eval_candidates` returns at most `limit` rejected sessions. [MCP server](https://coding.sh/docs/mcp#tools) +- **MCP.** A request cancelled with `notifications/cancelled` gets no reply (a cancel for `initialize` is ignored), a cursor on `tools/list`, `prompts/list` or `resources/templates/list` is an error, a request with a null id gets an error instead of no answer (a null-id `notifications/` message is still a notification), and a 2025-03-26 session accepts batches. [MCP server](https://coding.sh/docs/mcp#protocol) +- **MCP.** Tool answers stay under 25,000 characters. A tool that pages returns `nextCursor`; the others return fewer rows with `limitUsed`. `session_replay` pages its files, cuts diff and file text that does not fit and says so with `truncated` (a range was 5 million characters). `benchmarks` has a default limit. [MCP server](https://coding.sh/docs/mcp#tools) - **MCP.** `eval_init` and `score_add` are marked destructive, since both can replace stored data, and `score_list`'s error names its `since` argument instead of the CLI's `--since`. [MCP server](https://coding.sh/docs/mcp#tools) - **Site.** The changelog and quickstart pages no longer shift on a phone while loading: release area chips sit in a fixed grid and the quickstart tab list is laid out from first paint. Layout shift fell from 0.052 to 0 and from 0.023 to 0. [Site speed](https://github.com/jackulau/coding.sh/blob/main/docs/perf.md#font-swap-and-image-sizes) - **Stores.** A `file:///C:/...` blob URL and a `sqlite://user:password@/path` index URL open the path they name on Windows, and the user and password are no longer read as a folder. See [stores](https://github.com/jackulau/coding.sh/blob/main/docs/reference/stores.md). diff --git a/apps/site/src/pages/docs/mcp.astro b/apps/site/src/pages/docs/mcp.astro index 9d6654e3..6d4e47ee 100644 --- a/apps/site/src/pages/docs/mcp.astro +++ b/apps/site/src/pages/docs/mcp.astro @@ -114,9 +114,14 @@ const gemini = `// ~/.gemini/settings.json score replaces the one stored for the same target, name, source and person.

- A tool answer stays under 25,000 characters. When the rows asked for would be longer, the answer has fewer rows and a - limitUsed field, and nextCursor starts at the first row left out. The limit drops rows; it does not cut text. - session_replay shows at most 16 KB of a file and of each diff, and says when it cut one. + A tool answer stays under 25,000 characters. A tool that pages (it has a cursor argument) returns + nextCursor, which starts at the first row left out. The others return fewer rows when the answer would be longer, with a + limitUsed field. Rows are dropped, not cut. +

+

+ session_replay pages the files of a step, of a range and of the step list with limit and + cursor. It cuts the text of a diff, a file or a tool output that does not fit, and says so with + truncated: true and the field's own flag. limit: 1 gives one file's diff up to 16 KB.

Protocol

From d0a25918e50b464c0bce89571449e12bb1557232 Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Wed, 11 Nov 2026 22:58:12 -0600 Subject: [PATCH 08/11] test(mcp): wait longer for replies when the machine is loaded --- packages/coding.sh/test/mcp/protocol.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/coding.sh/test/mcp/protocol.test.ts b/packages/coding.sh/test/mcp/protocol.test.ts index 925071d5..362159b1 100644 --- a/packages/coding.sh/test/mcp/protocol.test.ts +++ b/packages/coding.sh/test/mcp/protocol.test.ts @@ -66,9 +66,9 @@ function world() { const done = serveStdio(server, { input, output }); const send = (m: unknown) => input.write((typeof m === "string" ? m : JSON.stringify(m)) + "\n"); const tick = () => new Promise((r) => setTimeout(r, 20)); - /** Waits until a reply with this id arrives (or 2 s pass). */ + /** Waits until a reply with this id arrives (or 20 s pass: the machine may be loaded). */ const reply = async (id: unknown): Promise => { - for (let i = 0; i < 100; i++) { + for (let i = 0; i < 1000; i++) { const m = out.find((x) => JSON.stringify(x.id) === JSON.stringify(id)); if (m) return m; await tick(); @@ -76,7 +76,7 @@ function world() { return undefined; }; const release = async () => { - for (let i = 0; i < 100 && !gates.length; i++) await tick(); + for (let i = 0; i < 1000 && !gates.length; i++) await tick(); gates.shift()!(); }; return { server, send, out, batches, reply, tick, release, close: async () => (input.end(), done) }; From 8f25477a53e0cc04a144ea8c9281fa3dc378ff18 Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Wed, 11 Nov 2026 23:43:24 -0600 Subject: [PATCH 09/11] fix(mcp): fit session_replay answers with one pass over the final answer Cut only when the answer does not fit: unpaged lists (a step's own files, edits in a tool input) keep their first items with a Total, the longest strings are cut to a common length, and the note and flags are counted in the length. A page takes rows by their size, so a small diff is not cut by the default limit. A cut inside a tool input is flagged on detail. --- CHANGELOG.md | 2 +- apps/site/src/pages/docs/mcp.astro | 31 ++- packages/coding.sh/src/mcp/replay-page.ts | 225 +++++++++++------- packages/coding.sh/src/mcp/server.ts | 2 +- packages/coding.sh/src/mcp/tools.ts | 11 +- packages/coding.sh/test/mcp/protocol.test.ts | 13 +- .../coding.sh/test/mcp/replay-limit.test.ts | 96 +++++++- scripts/lint/allowlist.json | 2 +- 8 files changed, 274 insertions(+), 108 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f467265f..d0c7ba07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,7 +38,7 @@ grouped by area, with links to the docs for the details. ### Fixed - **MCP.** A request cancelled with `notifications/cancelled` gets no reply (a cancel for `initialize` is ignored), a cursor on `tools/list`, `prompts/list` or `resources/templates/list` is an error, a request with a null id gets an error instead of no answer (a null-id `notifications/` message is still a notification), and a 2025-03-26 session accepts batches. [MCP server](https://coding.sh/docs/mcp#protocol) -- **MCP.** Tool answers stay under 25,000 characters. A tool that pages returns `nextCursor`; the others return fewer rows with `limitUsed`. `session_replay` pages its files, cuts diff and file text that does not fit and says so with `truncated` (a range was 5 million characters). `benchmarks` has a default limit. [MCP server](https://coding.sh/docs/mcp#tools) +- **MCP.** Tool answers stay under 25,000 characters: a longer page has fewer rows and `limitUsed`, and a tool with a `cursor` also returns `nextCursor`. `session_replay` pages its files and cuts text and lists only when an answer would not fit, saying so with `truncated` (a range was 5 million characters). [MCP server](https://coding.sh/docs/mcp#tools) - **MCP.** `eval_init` and `score_add` are marked destructive, since both can replace stored data, and `score_list`'s error names its `since` argument instead of the CLI's `--since`. [MCP server](https://coding.sh/docs/mcp#tools) - **Site.** The changelog and quickstart pages no longer shift on a phone while loading: release area chips sit in a fixed grid and the quickstart tab list is laid out from first paint. Layout shift fell from 0.052 to 0 and from 0.023 to 0. [Site speed](https://github.com/jackulau/coding.sh/blob/main/docs/perf.md#font-swap-and-image-sizes) - **Stores.** A `file:///C:/...` blob URL and a `sqlite://user:password@/path` index URL open the path they name on Windows, and the user and password are no longer read as a folder. See [stores](https://github.com/jackulau/coding.sh/blob/main/docs/reference/stores.md). diff --git a/apps/site/src/pages/docs/mcp.astro b/apps/site/src/pages/docs/mcp.astro index 6d4e47ee..82f773ff 100644 --- a/apps/site/src/pages/docs/mcp.astro +++ b/apps/site/src/pages/docs/mcp.astro @@ -114,14 +114,22 @@ const gemini = `// ~/.gemini/settings.json score replaces the one stored for the same target, name, source and person.

- A tool answer stays under 25,000 characters. A tool that pages (it has a cursor argument) returns - nextCursor, which starts at the first row left out. The others return fewer rows when the answer would be longer, with a - limitUsed field. Rows are dropped, not cut. + A tool answer stays under 25,000 characters. When the rows asked for would be longer, the answer has fewer rows and a + limitUsed field. A tool with a cursor argument also returns nextCursor, which starts at the + first row left out. Rows are dropped, not cut.

session_replay pages the files of a step, of a range and of the step list with limit and - cursor. It cuts the text of a diff, a file or a tool output that does not fit, and says so with - truncated: true and the field's own flag. limit: 1 gives one file's diff up to 16 KB. + cursor. limit: 1 gives one file's diff up to 16 KB. +

+

+ It cuts only when an answer would still be too long. A list that is not paged (a step's own files, the edits in a tool input) + keeps its first items, and the longest text is cut to a common length. +

+

+ The answer then has truncated: true, a note, and a flag such as diffTruncated or + filesTruncated (with filesTotal) on what was cut. A cut inside a tool input is flagged on + detail as inputTruncated.

Protocol

@@ -132,9 +140,16 @@ const gemini = `// ~/.gemini/settings.json

In 2026-07-28, list and read results carry ttlMs and cacheScope: the tool, prompt and template lists - are public for an hour, and resources are private and stale at once. A request cancelled with - notifications/cancelled gets no reply. Requests run one at a time, so a cancelled request that has not started - never runs. + are public for an hour, server/discover for 5 minutes (ttlMs 300000, because its + instructions carry an update notice), and resources are private and stale at once. +

+

+ A request cancelled with notifications/cancelled gets no reply, except initialize, which can't be + cancelled. Requests run one at a time, so a cancelled request that has not started never runs. +

+

+ A message with a null id whose method starts with notifications/ is a notification and gets no + reply. Any other request with a null id gets an invalid-request error.

Prompts and resources

diff --git a/packages/coding.sh/src/mcp/replay-page.ts b/packages/coding.sh/src/mcp/replay-page.ts index 2cc955a9..935104a6 100644 --- a/packages/coding.sh/src/mcp/replay-page.ts +++ b/packages/coding.sh/src/mcp/replay-page.ts @@ -1,135 +1,184 @@ // session_replay's answers kept under the character limit. The file lists (`tree`, `changes`, `files`, -// `unchanged`) are paged with limit and cursor; the text in them (diffs, file content, tool output) is -// cut to fit what is left, and the answer says so (`truncated`, plus the field's own flag). +// `unchanged`, the step list) are paged with limit and cursor. Then one pass over the final answer +// cuts what is still too long: arrays that are not paged are cut to the length that fits (with a +// `Total`), and the longest strings are cut to a common length. Each cut is flagged on the +// object that holds it (`diffTruncated`, `filesTruncated`, …); a cut inside a recorded tool input is +// flagged on `detail` as `inputTruncated`, never inside the input. Nothing is cut when the answer fits. import type { RangeView, StepView } from "../core/replay.ts"; import { cutChars, toolText } from "./protocol.ts"; type Obj = Record; -/** Text is never cut below this many characters when it sits in a row of a list, or outside one. */ -const ROW_FLOOR = 600; -const FIXED_FLOOR = 2000; -/** The most a diff or file shows (the replay is loaded with the same cap). */ -export const TEXT_MAX = 16 * 1024; +/** Text is never cut below this many characters. */ +const FLOOR = 64; +/** A page takes rows until they add up to this many characters, each counted at most ROW_SHARE. */ +const PAGE_BUDGET = 20_000; +const ROW_SHARE = 2_500; -const FLAG: Record = { diff: "diffTruncated", content: "contentTruncated", output: "outputTruncated", text: "textTruncated" }; +const NOTE = "Text and lists were cut to keep the answer under 25,000 characters (the fields marked truncated, with their Total). A smaller limit gives each diff more room; step with file gives one file's content."; -const NOTE = "Text was cut to keep the answer under 25,000 characters (the fields marked truncated). A smaller limit gives each diff more room (limit 1: up to 16 KB); step with file gives one file's content."; +const jlen = (v: unknown): number => JSON.stringify(v)?.length ?? 0; -interface Slot { +interface Hit { owner: Obj; key: string; - floor: number; + /** The object that gets the flag, and its name. */ + flagObj: Obj; + flagKey: string; + /** False inside a recorded tool input: no `Total` is written there. */ + total: boolean; } -function collect(v: unknown, inRow: boolean, out: Slot[], depth = 0): void { - if (depth > 8 || v === null || typeof v !== "object") return; - if (Array.isArray(v)) { - for (const x of v) collect(x, true, out, depth + 1); - return; - } - const o = v as Obj; - for (const [k, x] of Object.entries(o)) { - if (typeof x === "string") { - const floor = inRow ? ROW_FLOOR : FIXED_FLOOR; - if (x.length > floor) out.push({ owner: o, key: k, floor }); - } else collect(x, inRow, out, depth + 1); - } +/** Strings and arrays in `v` that may be cut: everything but the paged arrays at the top. */ +function collect(root: Obj, exempt: ReadonlySet): { strings: Hit[]; arrays: Hit[] } { + const strings: Hit[] = []; + const arrays: Hit[] = []; + const walk = (v: unknown, input: Obj | undefined, top: boolean, depth: number): void => { + if (depth > 10 || v === null || typeof v !== "object") return; + if (Array.isArray(v)) { + for (const x of v) walk(x, input, false, depth + 1); + return; + } + const o = v as Obj; + for (const [k, x] of Object.entries(o)) { + const inInput = input ?? (k === "input" ? o : undefined); + const hit = { owner: o, key: k, flagObj: inInput ?? o, flagKey: inInput ? "inputTruncated" : `${k}Truncated`, total: !inInput }; + if (typeof x === "string") strings.push(hit); + else if (Array.isArray(x)) { + if (!(top && exempt.has(k))) arrays.push(hit); + walk(x, inInput, false, depth + 1); + } else walk(x, inInput, false, depth + 1); + } + }; + walk(root, undefined, true, 0); + return { strings, arrays }; } -/** Whether any field in `v` is flagged as cut (`diffTruncated`, `contentTruncated`, …). */ -function hasFlag(v: unknown, depth = 0): boolean { - if (depth > 8 || v === null || typeof v !== "object") return false; - if (Array.isArray(v)) return v.some((x) => hasFlag(x, depth + 1)); - return Object.entries(v as Obj).some(([k, x]) => (k.endsWith("Truncated") && x === true) || hasFlag(x, depth + 1)); +/** `a`'s array cut to its first `keep` items. */ +function capArray(a: Hit, keep: number): void { + const list = a.owner[a.key] as unknown[]; + if (a.total && a.owner[`${a.key}Total`] === undefined) a.owner[`${a.key}Total`] = list.length; + a.owner[a.key] = list.slice(0, keep); + a.flagObj[a.flagKey] = true; +} + +/** The most items of `list` whose JSON fits `budget` characters. */ +function itemsWithin(list: unknown[], budget: number): number { + let used = 2; + let k = 0; + while (k < list.length && used + jlen(list[k]) + 1 <= budget) used += jlen(list[k++]) + 1; + return k; } /** - * Cuts the longest strings in `page` (in place) until its text is at most `limit` characters, or no string - * is left above its floor. The longest are cut down to a common length, so short text stays whole. - * Sets `truncated` and `note` on the page, and the field's own flag (`diffTruncated`, …) beside a cut one. + * Cuts `page` (in place) until its text is at most `limit` characters: arrays over a quarter of the + * limit first, then the longest strings down to a common length (not below FLOOR), then the longest + * arrays by halves. `exempt` names the top-level arrays that are already paged. `truncated` and + * `note` are set only when this pass cut something, and are counted in the length. */ -export function fitTexts(page: Obj, limit: number): Obj { - let len = toolText(page).length; - if (len <= limit) return hasFlag(page) ? { ...page, truncated: true, note: NOTE } : page; - const slots: Slot[] = []; - collect(page, false, slots); - let margin = 1.1; - for (let round = 0; round < 6 && len > limit && slots.length; round++, margin += 0.4) { - const need = Math.ceil((len - limit) * margin) + 64; - const size = (s: Slot) => (s.owner[s.key] as string).length; - const after = (s: Slot, level: number) => Math.max(s.floor, Math.min(size(s), level)); +export function fitAnswer(page: Obj, limit: number, exempt: readonly string[] = []): Obj { + if (jlen(page) <= limit) return page; + const ex = new Set(exempt); + page.truncated = true; + page.note = NOTE; + let cut = false; + const over = () => jlen(page) - limit; + const quarter = Math.floor(limit / 4); + for (const a of collect(page, ex).arrays) { + const list = a.owner[a.key] as unknown[]; + if (jlen(list) > quarter) { + capArray(a, itemsWithin(list, quarter)); + cut = true; + } + } + for (let round = 0, margin = 1.1; round < 6 && over() > 0; round++, margin += 0.4) { + const slots = collect(page, ex).strings.filter((s) => (s.owner[s.key] as string).length > FLOOR); + if (!slots.length) break; + const need = Math.ceil(over() * margin) + 64; + const size = (s: Hit) => (s.owner[s.key] as string).length; + const after = (s: Hit, level: number) => Math.max(FLOOR, Math.min(size(s), level)); const saved = (level: number) => slots.reduce((sum, s) => sum + size(s) - after(s, level), 0); - let lo = 0; + let lo = FLOOR; let hi = Math.max(...slots.map(size)); - // The highest level that still saves `need` characters (or the lowest there is). + // The highest common length that still saves `need` characters (or the lowest there is). while (lo < hi) { const mid = Math.ceil((lo + hi) / 2); if (saved(mid) >= need) lo = mid; else hi = mid - 1; } - let cut = false; for (const s of slots) { const to = after(s, lo); if (to >= size(s)) continue; s.owner[s.key] = cutChars(s.owner[s.key] as string, to); - const flag = FLAG[s.key]; - if (flag) s.owner[flag] = true; + s.flagObj[s.flagKey] = true; cut = true; } - if (!cut) break; - len = toolText({ ...page, truncated: true, note: NOTE }).length; } - return { ...page, truncated: true, note: NOTE }; + while (over() > 0) { + const arrays = collect(page, ex).arrays.filter((a) => (a.owner[a.key] as unknown[]).length > 0); + if (!arrays.length) break; + const big = arrays.reduce((m, a) => (jlen(a.owner[a.key]) > jlen(m.owner[m.key]) ? a : m)); + capArray(big, Math.floor((big.owner[big.key] as unknown[]).length / 2)); + cut = true; + } + if (!cut) { + delete page.truncated; + delete page.note; + } + return page; +} + +/** How many rows a page takes from `offset`: up to n, while their text adds up to PAGE_BUDGET (at least one). */ +function takeRows(n: number, offset: number, total: number, cost: (i: number) => number): number { + let used = 0; + let k = 0; + for (let i = offset; i < total && k < n; i++) { + const c = cost(i); + if (k > 0 && used + c > PAGE_BUDGET) break; + used += c; + k++; + } + return k; } -/** A diff cut to `max` characters at a line end, flagged when cut. */ -function capDiff(o: { diff?: string; diffTruncated?: boolean }, max: number): void { - if (o.diff === undefined || o.diff.length <= max) return; - let cut = cutChars(o.diff, max); - const nl = cut.lastIndexOf("\n"); - if (nl > 0) cut = cut.slice(0, nl + 1); - o.diff = cut; - o.diffTruncated = true; +const costs = new WeakMap(); +/** The size of each row of a view (its list entries side by side), computed once. */ +function rowCosts(key: object, count: number, row: (i: number) => number): number[] { + let c = costs.get(key); + if (!c) costs.set(key, (c = Array.from({ length: count }, (_, i) => row(i)))); + return c; } -/** Characters each row's diff may use on a page of n rows. */ -const perRow = (n: number) => Math.min(TEXT_MAX, Math.max(ROW_FLOOR, Math.floor(18_000 / n))); +const share = (v: unknown): number => Math.min(ROW_SHARE, jlen(v)); /** A page of a step's view: the file lists from `offset`; the step's own detail and file only on the first page. */ export function pageStep(view: StepView, offset: number, n: number, limit: number): Obj { - const first: Obj = {}; + const total = Math.max(view.tree.length, view.changes.length); + const c = rowCosts(view, total, (i) => (i < view.tree.length ? jlen(view.tree[i]) : 0) + (i < view.changes.length ? share(view.changes[i]) : 0)); + const k = takeRows(n, offset, total, (i) => c[i]!); + const page: Obj = { sessionId: view.sessionId, steps: view.steps, step: view.step }; if (offset === 0) { - first.detail = view.detail; - if (view.file) first.file = view.file; + page.detail = view.detail; + if (view.file) page.file = view.file; } - const page = structuredClone({ - sessionId: view.sessionId, - steps: view.steps, - step: view.step, - ...first, - tree: view.tree.slice(offset, offset + n), - changes: view.changes.slice(offset, offset + n), - treeTotal: view.tree.length, - changesTotal: view.changes.length, - }) as Obj & { changes: { diff?: string; diffTruncated?: boolean }[]; nextCursor?: string }; - for (const c of page.changes) capDiff(c, perRow(n)); - if (offset + n < Math.max(view.tree.length, view.changes.length)) page.nextCursor = String(offset + n); - return fitTexts(page, limit); + page.tree = view.tree.slice(offset, offset + k); + page.changes = view.changes.slice(offset, offset + k); + page.treeTotal = view.tree.length; + page.changesTotal = view.changes.length; + if (offset + k < total) page.nextCursor = String(offset + k); + return fitAnswer(structuredClone(page), limit, ["tree", "changes"]); } /** A page of a range's view. */ export function pageRange(view: RangeView, offset: number, n: number, limit: number): Obj { - const page = structuredClone({ - sessionId: view.sessionId, - from: view.from, - to: view.to, - files: view.files.slice(offset, offset + n), - unchanged: view.unchanged.slice(offset, offset + n), - filesTotal: view.files.length, - unchangedTotal: view.unchanged.length, - }) as Obj & { files: { diff?: string; diffTruncated?: boolean }[]; nextCursor?: string }; - for (const f of page.files) capDiff(f, perRow(n)); - if (offset + n < Math.max(view.files.length, view.unchanged.length)) page.nextCursor = String(offset + n); - return fitTexts(page, limit); + const total = Math.max(view.files.length, view.unchanged.length); + const c = rowCosts(view, total, (i) => (i < view.files.length ? share(view.files[i]) : 0) + (i < view.unchanged.length ? jlen(view.unchanged[i]) : 0)); + const k = takeRows(n, offset, total, (i) => c[i]!); + const page: Obj = { sessionId: view.sessionId, from: view.from, to: view.to }; + page.files = view.files.slice(offset, offset + k); + page.unchanged = view.unchanged.slice(offset, offset + k); + page.filesTotal = view.files.length; + page.unchangedTotal = view.unchanged.length; + if (offset + k < total) page.nextCursor = String(offset + k); + return fitAnswer(structuredClone(page), limit, ["files", "unchanged"]); } diff --git a/packages/coding.sh/src/mcp/server.ts b/packages/coding.sh/src/mcp/server.ts index a2613ebe..3d80d4a4 100644 --- a/packages/coding.sh/src/mcp/server.ts +++ b/packages/coding.sh/src/mcp/server.ts @@ -44,7 +44,7 @@ const BASE_INSTRUCTIONS = [ "To build evals for the user, call eval_init (a suite in evals// from failed sessions, a skill's trigger prompts, or cases you write), then tell them its run command.", "eval_from_session, eval_init, score_add and exp_session_note write; ask the user before calling eval_from_session or eval_init with confirm: true.", 'A result with pending: {files, indexing} was answered before every capture was indexed: it covers what is indexed so far; call again shortly for the rest.', - "Answers stay under 25,000 characters. A tool that pages returns nextCursor for the rest; the others return fewer rows, with limitUsed.", + "Answers stay under 25,000 characters. A page that would be longer has fewer rows and limitUsed; a tool with a cursor argument also returns nextCursor for the rest.", ].join(" "); /** Server instructions, with a one-line notice when the update checker found a newer release. */ diff --git a/packages/coding.sh/src/mcp/tools.ts b/packages/coding.sh/src/mcp/tools.ts index 90c6332a..2c5ae8b9 100644 --- a/packages/coding.sh/src/mcp/tools.ts +++ b/packages/coding.sh/src/mcp/tools.ts @@ -39,7 +39,7 @@ import { import { sessionDigest } from "./digest.ts"; import { facetCounts, searchWhere } from "../query/run.ts"; import { andQuery, FACET_KEYS, parseWhere, QueryError, QueryUnsupported, type Query } from "../query/where.ts"; -import { fitTexts, pageRange, pageStep } from "./replay-page.ts"; +import { fitAnswer, pageRange, pageStep } from "./replay-page.ts"; import { toolText, ToolError, type ToolDef } from "./protocol.ts"; import { scoreTools } from "../scores/mcp.ts"; @@ -509,7 +509,7 @@ function toolDefs(): ToolDef[] { } if (a.fork !== undefined) { if (step === undefined) throw new ToolError("fork needs a step."); - return fitTexts({ ...(await forkTool(ctx, c.env, s, replay, step, a.fork)) }, REPLAY_TEXT_LIMIT); + return fitAnswer({ ...(await forkTool(ctx, c.env, s, replay, step, a.fork)) }, REPLAY_TEXT_LIMIT); } const file = strArg(a, "file"); if (step !== undefined) { @@ -518,8 +518,11 @@ function toolDefs(): ToolDef[] { } if (file) throw new ToolError("file needs a step."); const index = replay.index(); - const more = (n: number) => offset + n < Math.max(total, index.files.length); - return fitPage(limit, (n) => ({ ...index, steps: index.steps.slice(offset, offset + n), files: index.files.slice(offset, offset + n), total, filesTotal: index.files.length, ...(more(n) ? { nextCursor: String(offset + n) } : {}) })); + return fitPage(limit, (n) => { + const page: Obj = { ...index, steps: structuredClone(index.steps.slice(offset, offset + n)), files: index.files.slice(offset, offset + n), total, filesTotal: index.files.length }; + if (offset + n < Math.max(total, index.files.length)) page.nextCursor = String(offset + n); + return fitAnswer(page, REPLAY_TEXT_LIMIT, ["steps", "files"]); + }); }, }, { diff --git a/packages/coding.sh/test/mcp/protocol.test.ts b/packages/coding.sh/test/mcp/protocol.test.ts index 362159b1..b86fed1a 100644 --- a/packages/coding.sh/test/mcp/protocol.test.ts +++ b/packages/coding.sh/test/mcp/protocol.test.ts @@ -75,11 +75,18 @@ function world() { } return undefined; }; + /** Waits for the signal that a tool is running: it has registered its gate. */ + const started = async () => { + for (let i = 0; i < 1000 && !gates.length; i++) await tick(); + assert.ok(gates.length, "the tool started"); + }; + /** Two turns of the event loop: input written so far has been read. */ + const settle = () => new Promise((r) => setImmediate(() => setImmediate(r))); const release = async () => { for (let i = 0; i < 1000 && !gates.length; i++) await tick(); gates.shift()!(); }; - return { server, send, out, batches, reply, tick, release, close: async () => (input.end(), done) }; + return { server, send, out, batches, reply, tick, started, settle, release, close: async () => (input.end(), done) }; } describe("caching hints (2026-07-28)", () => { @@ -128,9 +135,9 @@ describe("cancellation (notifications/cancelled)", () => { it("a request cancelled while it runs gets no reply", async () => { const w = world(); w.send({ jsonrpc: "2.0", id: "a", method: "tools/call", params: { name: "slow" } }); - await w.tick(); + await w.started(); w.send({ jsonrpc: "2.0", method: "notifications/cancelled", params: { requestId: "a" } }); - await w.tick(); + await w.settle(); await w.release(); w.send({ jsonrpc: "2.0", id: "b", method: "ping" }); assert.ok(await w.reply("b")); diff --git a/packages/coding.sh/test/mcp/replay-limit.test.ts b/packages/coding.sh/test/mcp/replay-limit.test.ts index 99bc1a15..1a42d46d 100644 --- a/packages/coding.sh/test/mcp/replay-limit.test.ts +++ b/packages/coding.sh/test/mcp/replay-limit.test.ts @@ -18,8 +18,8 @@ let c: RawClient; type Sc = Record; -async function call(args: Record): Promise<{ sc: Sc; len: number }> { - const r = await c.call("session_replay", { sessionId: ID, ...args }); +async function call(args: Record, sessionId = ID): Promise<{ sc: Sc; len: number }> { + const r = await c.call("session_replay", { sessionId, ...args }); assert.notEqual(r.isError, true, r.content[0]?.text.slice(0, 300)); return { sc: r.structuredContent as Sc, len: r.content[0]!.text.length }; } @@ -62,6 +62,32 @@ before(async () => { evs.push({ ...base, id: "f-end", ts: t0 + 20 * FILES, kind: "session.end", payload: {} }); await ctx.index.putEvents(evs); await ctx.index.upsertSession(buildSession(evs)); + // More shapes: a small diff, a MultiEdit of 60 edits, an apply_patch that adds 500 files, and a 20 KB + // file followed by tool steps whose output grows, so that some step is near the limit. + const put = async (sid: string, agent: string, calls: { tool: string; input: Record; output?: string }[]) => { + const b = { ...base, sessionId: sid, agent }; + const list: AgentEvent[] = [{ ...b, id: `${sid}-0`, ts: t0, kind: "session.start", payload: {} }, { ...b, id: `${sid}-1`, ts: t0 + 1, kind: "user.message", payload: { text: "go" } }]; + calls.forEach((x, i) => { + list.push({ ...b, id: `${sid}-s${i}`, ts: t0 + 10 + i * 2, kind: "tool.start", payload: { tool: x.tool, toolUseId: `u${i}`, input: x.input } }); + list.push({ ...b, id: `${sid}-e${i}`, ts: t0 + 11 + i * 2, kind: "tool.end", payload: { tool: x.tool, toolUseId: `u${i}`, ok: true, output: x.output } }); + }); + list.push({ ...b, id: `${sid}-end`, ts: t0 + 1000, kind: "session.end", payload: {} }); + await ctx.index.putEvents(list); + await ctx.index.upsertSession(buildSession(list)); + }; + const body = Array.from({ length: 150 }, (_, l) => `export const v${l} = ${l * 7919};`).join("\n") + "\n"; + await put("rv-small", "claude-code", [ + { tool: "Write", input: { file_path: "src/a.ts", content: "short\n" } }, + { tool: "Edit", input: { file_path: "src/a.ts", old_string: "short\n", new_string: body.slice(0, 1900) } }, + ]); + const edits = Array.from({ length: 60 }, (_, i) => ({ old_string: `old${i} `.padEnd(500, "o"), new_string: `new${i} `.padEnd(500, "n") })); + await put("rv-multi", "claude-code", [{ tool: "MultiEdit", input: { file_path: "src/b.ts", edits } }]); + const patch = "*** Begin Patch\n" + Array.from({ length: 500 }, (_, i) => `*** Add File: src/generated/models/entity-${String(i).padStart(4, "0")}.generated.ts\n+export const x = ${i};\n`).join("") + "*** End Patch\n"; + await put("rv-patch", "codex", [{ tool: "apply_patch", input: { input: patch }, output: "Done" }]); + await put("rv-grow", "claude-code", [ + { tool: "Write", input: { file_path: "src/d.ts", content: "x".repeat(20000) } }, + ...Array.from({ length: 40 }, (_, k) => ({ tool: "Grep", input: { pattern: "x" }, output: "y".repeat(7700 + k * 20) })), + ]); await ctx.close(); c = new RawClient(w.env, w.repo); await c.initialize(); @@ -115,3 +141,69 @@ describe("session_replay under the 25,000-character limit", () => { assert.equal(one.sc.nextCursor, "1"); }); }); + +describe("session_replay cuts only what does not fit", () => { + it("cuts many small strings in one step (a MultiEdit of 60 edits), whatever the limit", async () => { + for (const limit of [undefined, 1]) { + const { sc, len } = await call({ step: 2, limit }, "rv-multi"); + assert.ok(len <= CHARACTER_LIMIT, `limit ${String(limit)}: ${len} characters`); + assert.equal(sc.truncated, true); + assert.match(sc.note, /cut/); + assert.equal(sc.detail.inputTruncated, true, "the cut input is flagged on detail"); + assert.equal(sc.detail.input.editsTotal, undefined, "nothing is written inside the recorded input"); + assert.equal(sc.detail.input.editsTruncated, undefined); + assert.ok(sc.detail.input.edits.length >= 1 && sc.detail.input.edits.length < 60); + } + }); + + it("caps a step's own file list, in the step and in the step list", async () => { + for (const limit of [undefined, 1]) { + const { sc, len } = await call({ step: 2, limit }, "rv-patch"); + assert.ok(len <= CHARACTER_LIMIT, `step, limit ${String(limit)}: ${len} characters`); + assert.equal(sc.step.filesTruncated, true); + assert.equal(sc.step.filesTotal, 500); + assert.ok(sc.step.files.length < 500); + } + const list = await call({}, "rv-patch"); + assert.ok(list.len <= CHARACTER_LIMIT, `step list: ${list.len} characters`); + const row = (list.sc.steps as { files?: unknown[]; filesTotal?: number }[]).find((x) => x.filesTotal === 500); + assert.ok(row && row.files!.length < 500, "a step's file list in the step list is capped, with its total"); + const pages = await pages_({ sessionId: "rv-patch" }); + assert.ok(pages.every((p) => p.len <= CHARACTER_LIMIT)); + }); + + it("stays under the limit, note included, for a step whose content and output are near it", async () => { + let cut = 0; + for (let k = 0; k < 40; k++) { + const { sc, len } = await call({ step: 3 + k, file: "src/d.ts" }, "rv-grow"); + assert.ok(len <= CHARACTER_LIMIT, `step ${3 + k}: ${len} characters`); + if (sc.truncated) { + cut++; + assert.match(sc.note, /cut/); + } else assert.equal(sc.note, undefined, "no note when this answer cut nothing"); + } + assert.ok(cut > 0, "some step needed a cut"); + }); + + it("does not cut a small diff at the default limit, nor add a note", async () => { + const { sc } = await call({ step: 3 }, "rv-small"); + assert.equal(sc.changes[0].diff.length > 1900, true, `the whole diff (${sc.changes[0].diff.length})`); + assert.equal(sc.changes[0].diffTruncated, undefined); + assert.equal(sc.truncated, undefined); + assert.equal(sc.note, undefined); + const range = await call({ from: 3, to: 3 }, "rv-small"); + assert.equal(range.sc.files[0].diffTruncated, undefined); + }); +}); + +async function pages_(args: { sessionId: string }): Promise<{ len: number }[]> { + const out: { len: number }[] = []; + let cursor: string | undefined; + for (let i = 0; i < 50; i++) { + const r = await call(cursor ? { cursor } : {}, args.sessionId); + out.push({ len: r.len }); + cursor = r.sc.nextCursor; + if (!cursor) break; + } + return out; +} diff --git a/scripts/lint/allowlist.json b/scripts/lint/allowlist.json index fde1b1c7..3749eab4 100644 --- a/scripts/lint/allowlist.json +++ b/scripts/lint/allowlist.json @@ -1845,7 +1845,7 @@ { "rule": "conditional-empty-spread", "path": "packages/coding.sh/src/mcp/tools.ts", - "max": 68, + "max": 67, "reason": "baseline 2026-11-09: existing code, do not add more" }, { From d8bed8575802d2a20491c9a6f412d57518cea573 Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Wed, 11 Nov 2026 23:56:29 -0600 Subject: [PATCH 10/11] fix(mcp): fit session_replay answers with one budget allocation, rows first A page takes whole rows by their measured size and sets limitUsed when it has fewer than asked. Only a row or the rest of the answer that is still too long is cut, by a top-down allocation that shares the room among the longest children at any depth, counts JSON escapes, keeps ids and paths whole, flags each cut, and ends with a guaranteed fit. --- apps/site/src/pages/docs/mcp.astro | 7 +- packages/coding.sh/src/mcp/replay-page.ts | 371 ++++++++++++------ packages/coding.sh/src/mcp/tools.ts | 12 +- .../coding.sh/test/mcp/fit-answer.test.ts | 162 ++++++++ .../coding.sh/test/mcp/replay-limit.test.ts | 55 ++- 5 files changed, 434 insertions(+), 173 deletions(-) create mode 100644 packages/coding.sh/test/mcp/fit-answer.test.ts diff --git a/apps/site/src/pages/docs/mcp.astro b/apps/site/src/pages/docs/mcp.astro index 82f773ff..38b906dc 100644 --- a/apps/site/src/pages/docs/mcp.astro +++ b/apps/site/src/pages/docs/mcp.astro @@ -123,12 +123,13 @@ const gemini = `// ~/.gemini/settings.json cursor. limit: 1 gives one file's diff up to 16 KB.

- It cuts only when an answer would still be too long. A list that is not paged (a step's own files, the edits in a tool input) - keeps its first items, and the longest text is cut to a common length. + A page takes whole rows while they fit, so a short page has limitUsed and a nextCursor. It cuts only + when one row, or the rest of the answer, is still too long. Then the room is shared out: short text stays whole, the longest is + cut, and a long list keeps its first items. Ids and paths are never cut.

The answer then has truncated: true, a note, and a flag such as diffTruncated or - filesTruncated (with filesTotal) on what was cut. A cut inside a tool input is flagged on + filesTruncated (with filesTotal, or truncatedKeys on an object) on what was cut. A cut inside a tool input is flagged on detail as inputTruncated.

diff --git a/packages/coding.sh/src/mcp/replay-page.ts b/packages/coding.sh/src/mcp/replay-page.ts index 935104a6..2c5f073d 100644 --- a/packages/coding.sh/src/mcp/replay-page.ts +++ b/packages/coding.sh/src/mcp/replay-page.ts @@ -1,184 +1,295 @@ -// session_replay's answers kept under the character limit. The file lists (`tree`, `changes`, `files`, -// `unchanged`, the step list) are paged with limit and cursor. Then one pass over the final answer -// cuts what is still too long: arrays that are not paged are cut to the length that fits (with a -// `Total`), and the longest strings are cut to a common length. Each cut is flagged on the -// object that holds it (`diffTruncated`, `filesTruncated`, …); a cut inside a recorded tool input is -// flagged on `detail` as `inputTruncated`, never inside the input. Nothing is cut when the answer fits. -import type { RangeView, StepView } from "../core/replay.ts"; -import { cutChars, toolText } from "./protocol.ts"; +// session_replay's answers kept under the character limit, in two steps. +// +// 1. Rows first, uncut. A page takes whole rows (file lists, the step list) by their measured size, +// as many as fit beside the rest of the answer, at least one. A page with fewer rows than asked +// says so with `limitUsed`, and `nextCursor` starts at the first row left out. +// 2. `fitAnswer` cuts only what is still too long: one budget allocation over the answer, top down. +// Each object or array gives the children that fit under their fair share their full size, and +// splits the rest among the larger ones. Strings are cut at a character boundary by their escaped +// length. When there are too many children to give each a useful share, the first ones are kept +// whole in order and the rest dropped. Ids, paths, cursors and flags are never cut. Each cut is +// flagged on the object that holds it (`diffTruncated`, `filesTruncated` with `filesTotal`, +// `truncatedKeys`); a cut inside a recorded tool input is flagged on its holder as `inputTruncated`, +// never inside the input. `truncated` and `note` are set only when this step cut something. +import type { RangeView, ReplayIndex, StepView } from "../core/replay.ts"; +import { toolText } from "./protocol.ts"; type Obj = Record; -/** Text is never cut below this many characters. */ -const FLOOR = 64; -/** A page takes rows until they add up to this many characters, each counted at most ROW_SHARE. */ -const PAGE_BUDGET = 20_000; -const ROW_SHARE = 2_500; +/** The smallest share of characters worth cutting a child to; below it the child is dropped instead. */ +const MIN_SHARE = 200; +/** Room kept free for the page's cursor and `limitUsed`. */ +const SLACK = 80; const NOTE = "Text and lists were cut to keep the answer under 25,000 characters (the fields marked truncated, with their Total). A smaller limit gives each diff more room; step with file gives one file's content."; -const jlen = (v: unknown): number => JSON.stringify(v)?.length ?? 0; +/** Keys whose text is never cut (ids, paths, names the reader needs to follow the answer). */ +const PROTECTED = new Set(["note", "truncated", "nextCursor", "limitUsed", "sessionId", "id", "eventId", "toolUseId", "n", "path", "file", "kind", "tool", "op", "status", "root", "baseline", "from", "to"]); -interface Hit { - owner: Obj; - key: string; - /** The object that gets the flag, and its name. */ - flagObj: Obj; - flagKey: string; - /** False inside a recorded tool input: no `Total` is written there. */ - total: boolean; +const jlen = (v: unknown): number => toolText(v)?.length ?? 0; +const isContainer = (v: unknown): v is object => typeof v === "object" && v !== null; + +interface Info { + keys?: string[]; + vals: unknown[]; + /** Serialized size of each value. */ + sizes: number[]; + /** What each entry costs besides its value: the key and a comma. */ + over: number[]; + /** Serialized size of the whole container. */ + total: number; } -/** Strings and arrays in `v` that may be cut: everything but the paged arrays at the top. */ -function collect(root: Obj, exempt: ReadonlySet): { strings: Hit[]; arrays: Hit[] } { - const strings: Hit[] = []; - const arrays: Hit[] = []; - const walk = (v: unknown, input: Obj | undefined, top: boolean, depth: number): void => { - if (depth > 10 || v === null || typeof v !== "object") return; - if (Array.isArray(v)) { - for (const x of v) walk(x, input, false, depth + 1); - return; - } - const o = v as Obj; - for (const [k, x] of Object.entries(o)) { - const inInput = input ?? (k === "input" ? o : undefined); - const hit = { owner: o, key: k, flagObj: inInput ?? o, flagKey: inInput ? "inputTruncated" : `${k}Truncated`, total: !inInput }; - if (typeof x === "string") strings.push(hit); - else if (Array.isArray(x)) { - if (!(top && exempt.has(k))) arrays.push(hit); - walk(x, inInput, false, depth + 1); - } else walk(x, inInput, false, depth + 1); - } +/** The serialized size of every object and array under `root`, bottom up with an explicit stack (any depth). */ +function measure(root: object): Map { + const infos = new Map(); + const open = (v: object) => { + if (Array.isArray(v)) return { v, keys: undefined as string[] | undefined, vals: v as unknown[], sizes: [] as number[], i: 0 }; + const entries = Object.entries(v).filter(([, x]) => x !== undefined && typeof x !== "function" && typeof x !== "symbol"); + return { v, keys: entries.map(([k]) => k), vals: entries.map(([, x]) => x), sizes: [] as number[], i: 0 }; }; - walk(root, undefined, true, 0); - return { strings, arrays }; + const stack = [open(root)]; + while (stack.length) { + const f = stack[stack.length - 1]!; + if (f.i < f.vals.length) { + const c = f.vals[f.i]; + if (isContainer(c)) { + const known = infos.get(c); + if (!known) { + stack.push(open(c)); + continue; + } + f.sizes.push(known.total); + } else f.sizes.push(jlen(c === undefined ? null : c)); + f.i++; + continue; + } + const over = f.vals.map((_, i) => (f.keys ? jlen(f.keys[i]!) + 1 : 0) + 1); + const n = f.vals.length; + const total = 2 + over.reduce((a, b) => a + b, 0) + f.sizes.reduce((a, b) => a + b, 0) - (n ? 1 : 0); + infos.set(f.v, { keys: f.keys, vals: f.vals, sizes: f.sizes, over, total }); + stack.pop(); + } + return infos; } -/** `a`'s array cut to its first `keep` items. */ -function capArray(a: Hit, keep: number): void { - const list = a.owner[a.key] as unknown[]; - if (a.total && a.owner[`${a.key}Total`] === undefined) a.owner[`${a.key}Total`] = list.length; - a.owner[a.key] = list.slice(0, keep); - a.flagObj[a.flagKey] = true; +/** The longest prefix of `s` whose JSON text (with its quotes) is at most `budget` characters, cut between characters. */ +function cutString(s: string, budget: number): string { + const room = Math.max(0, budget - 2); + let used = 0; + let i = 0; + while (i < s.length) { + const c = s.charCodeAt(i); + let cost = 1; + let step = 1; + if (c >= 0xd800 && c <= 0xdbff && i + 1 < s.length && (s.charCodeAt(i + 1) & 0xfc00) === 0xdc00) step = 2; + else if (c >= 0xd800 && c <= 0xdfff) cost = 6; + else if (c < 0x20) cost = c === 8 || c === 9 || c === 10 || c === 12 || c === 13 ? 2 : 6; + else if (c === 34 || c === 92) cost = 2; + if (step === 2) cost = 2; + if (used + cost > room) break; + used += cost; + i += step; + } + return s.slice(0, i); } -/** The most items of `list` whose JSON fits `budget` characters. */ -function itemsWithin(list: unknown[], budget: number): number { - let used = 2; - let k = 0; - while (k < list.length && used + jlen(list[k]) + 1 <= budget) used += jlen(list[k++]) + 1; - return k; +interface Flag { + obj: Obj; + key: string; +} +interface Task { + info: Info; + out: Obj | unknown[]; + budget: number; + /** Where a cut in here is flagged. */ + flag: Flag | undefined; + /** Inside a recorded tool input: nothing but `inputTruncated` is written. */ + inInput: boolean; + /** For an array held by an object key: that object and key, for `Total`. */ + self: Flag | undefined; } -/** - * Cuts `page` (in place) until its text is at most `limit` characters: arrays over a quarter of the - * limit first, then the longest strings down to a common length (not below FLOOR), then the longest - * arrays by halves. `exempt` names the top-level arrays that are already paged. `truncated` and - * `note` are set only when this pass cut something, and are counted in the length. - */ -export function fitAnswer(page: Obj, limit: number, exempt: readonly string[] = []): Obj { - if (jlen(page) <= limit) return page; - const ex = new Set(exempt); - page.truncated = true; - page.note = NOTE; +/** A copy of `root` that serializes to at most `budget` characters (when the protected text allows). Originals are not changed. */ +function allocate(root: Obj, infos: Map, budget: number): { out: Obj; cut: boolean } { + const out: Obj = {}; let cut = false; - const over = () => jlen(page) - limit; - const quarter = Math.floor(limit / 4); - for (const a of collect(page, ex).arrays) { - const list = a.owner[a.key] as unknown[]; - if (jlen(list) > quarter) { - capArray(a, itemsWithin(list, quarter)); - cut = true; + const stack: Task[] = [{ info: infos.get(root)!, out, budget, flag: undefined, inInput: false, self: undefined }]; + while (stack.length) { + const t = stack.pop()!; + const { info: I } = t; + const n = I.vals.length; + const cost = (i: number) => I.over[i]! + I.sizes[i]!; + // decide[i]: Infinity keeps it whole, -1 drops it, anything else is the budget for its value. + const decide: number[] = new Array(n).fill(Infinity); + let remaining = t.budget - 1; + const rest: number[] = []; + for (let i = 0; i < n; i++) { + const v = I.vals[i]; + // Ids and paths, and numbers, booleans and null (a few characters each), are never cut or dropped. + if (!isContainer(v) && (typeof v !== "string" || (I.keys && PROTECTED.has(I.keys[i]!)))) remaining -= cost(i); + else rest.push(i); } - } - for (let round = 0, margin = 1.1; round < 6 && over() > 0; round++, margin += 0.4) { - const slots = collect(page, ex).strings.filter((s) => (s.owner[s.key] as string).length > FLOOR); - if (!slots.length) break; - const need = Math.ceil(over() * margin) + 64; - const size = (s: Hit) => (s.owner[s.key] as string).length; - const after = (s: Hit, level: number) => Math.max(FLOOR, Math.min(size(s), level)); - const saved = (level: number) => slots.reduce((sum, s) => sum + size(s) - after(s, level), 0); - let lo = FLOOR; - let hi = Math.max(...slots.map(size)); - // The highest common length that still saves `need` characters (or the lowest there is). - while (lo < hi) { - const mid = Math.ceil((lo + hi) / 2); - if (saved(mid) >= need) lo = mid; - else hi = mid - 1; + let sum = 0; + for (const i of rest) sum += cost(i); + if (sum > remaining) { + const order = [...rest].sort((a, b) => cost(a) - cost(b)); + let rem = remaining; + let left = order.length; + let pos = 0; + while (pos < order.length && cost(order[pos]!) <= rem / left) rem -= cost(order[pos++]!), left--; + if (left > 0 && rem / left >= MIN_SHARE) { + for (; pos < order.length; pos++) decide[order[pos]!] = Math.max(2, Math.floor(rem / left) - I.over[order[pos]!]!); + } else if (left > 0) { + // Too many children for each to get a useful share: keep the first ones whole, cut the next, drop the rest. + let r = remaining; + let dropping = false; + for (const i of rest) { + if (dropping) decide[i] = -1; + else if (cost(i) <= r) r -= cost(i); + else { + const vb = r - I.over[i]!; + decide[i] = vb >= MIN_SHARE ? vb : -1; + dropping = true; + } + } + } + } + const isArr = Array.isArray(t.out); + let dropped = 0; + for (let i = 0; i < n; i++) { + const d = decide[i]!; + if (d === -1) { + dropped++; + continue; + } + const val = I.vals[i]; + const key = I.keys?.[i]; + // Where a cut at this entry is flagged. + const flag: Flag | undefined = isArr ? t.flag : t.inInput ? t.flag : key === "input" ? { obj: t.out as Obj, key: "inputTruncated" } : { obj: t.out as Obj, key: `${key}Truncated` }; + const inInput = t.inInput || key === "input"; + let nv = val; + if (d !== Infinity && typeof val === "string") { + nv = cutString(val, d); + cut = true; + if (flag) flag.obj[flag.key] = true; + } else if (d !== Infinity && isContainer(val)) { + const child: Obj | unknown[] = Array.isArray(val) ? [] : {}; + cut = true; + stack.push({ info: infos.get(val)!, out: child, budget: d, flag, inInput, self: Array.isArray(val) && key !== undefined && !inInput ? { obj: t.out as Obj, key } : undefined }); + nv = child; + } + if (isArr) (t.out as unknown[]).push(nv); + else (t.out as Obj)[key!] = nv; } - for (const s of slots) { - const to = after(s, lo); - if (to >= size(s)) continue; - s.owner[s.key] = cutChars(s.owner[s.key] as string, to); - s.flagObj[s.flagKey] = true; + if (dropped) { cut = true; + if (t.flag) t.flag.obj[t.flag.key] = true; + if (!t.inInput) { + if (t.self) t.self.obj[`${t.self.key}Total`] = n; + else if (!isArr) (t.out as Obj).truncatedKeys = dropped; + } } } - while (over() > 0) { - const arrays = collect(page, ex).arrays.filter((a) => (a.owner[a.key] as unknown[]).length > 0); - if (!arrays.length) break; - const big = arrays.reduce((m, a) => (jlen(a.owner[a.key]) > jlen(m.owner[m.key]) ? a : m)); - capArray(big, Math.floor((big.owner[big.key] as unknown[]).length / 2)); - cut = true; + return { out, cut }; +} + +/** + * `page`, or a copy of it cut to at most `cap` characters. Nothing is cut when it fits. `truncated` + * and `note` are set when something was cut, and counted in the length. A last resort replaces the + * largest top-level entries with `{truncated: true, chars}` when protected text alone is too long. + */ +export function fitAnswer(page: Obj, cap: number): Obj { + const infos = measure(page); + if (infos.get(page)!.total <= cap) return page; + let budget = cap - jlen(NOTE) - 40 - 256; + let out: Obj = page; + for (let attempt = 0; attempt < 3; attempt++) { + const r = allocate(page, infos, budget); + out = r.out; + if (r.cut) { + out.truncated = true; + out.note = NOTE; + } + const len = jlen(out); + if (len <= cap) return out; + budget -= len - cap + 32; } - if (!cut) { - delete page.truncated; - delete page.note; + out.truncated = true; + out.note = NOTE; + const replaced = new Set(); + for (let len = jlen(out); len > cap; len = jlen(out)) { + let big: string | undefined; + let bigSize = 0; + for (const [k, v] of Object.entries(out)) { + if (replaced.has(k) || k === "note" || k === "truncated" || k === "nextCursor" || k === "limitUsed") continue; + const s = jlen(v); + if (s > bigSize) [big, bigSize] = [k, s]; + } + if (big === undefined) break; + out[big] = { truncated: true, chars: bigSize }; + replaced.add(big); } - return page; + return out; } -/** How many rows a page takes from `offset`: up to n, while their text adds up to PAGE_BUDGET (at least one). */ -function takeRows(n: number, offset: number, total: number, cost: (i: number) => number): number { - let used = 0; +/** How many rows a page takes from `offset`: up to n, while the answer (`fixed` characters besides the rows) fits `cap`; at least one. */ +function takeRows(n: number, offset: number, total: number, fixed: number, cap: number, cost: (i: number) => number): number { + let used = fixed + SLACK; let k = 0; for (let i = offset; i < total && k < n; i++) { const c = cost(i); - if (k > 0 && used + c > PAGE_BUDGET) break; + if (k > 0 && used + c > cap) break; used += c; k++; } return k; } -const costs = new WeakMap(); -/** The size of each row of a view (its list entries side by side), computed once. */ -function rowCosts(key: object, count: number, row: (i: number) => number): number[] { - let c = costs.get(key); - if (!c) costs.set(key, (c = Array.from({ length: count }, (_, i) => row(i)))); - return c; +/** Cursor and `limitUsed` for a page of k rows, out of n asked, from `offset`. */ +function paging(page: Obj, offset: number, k: number, n: number, total: number): void { + if (offset + k < total) page.nextCursor = String(offset + k); + if (k < Math.min(n, total - offset)) page.limitUsed = k; } -const share = (v: unknown): number => Math.min(ROW_SHARE, jlen(v)); +const rowLen = (list: unknown[], i: number): number => (i < list.length ? jlen(list[i]) + 1 : 0); /** A page of a step's view: the file lists from `offset`; the step's own detail and file only on the first page. */ -export function pageStep(view: StepView, offset: number, n: number, limit: number): Obj { +export function pageStep(view: StepView, offset: number, n: number, cap: number): Obj { const total = Math.max(view.tree.length, view.changes.length); - const c = rowCosts(view, total, (i) => (i < view.tree.length ? jlen(view.tree[i]) : 0) + (i < view.changes.length ? share(view.changes[i]) : 0)); - const k = takeRows(n, offset, total, (i) => c[i]!); const page: Obj = { sessionId: view.sessionId, steps: view.steps, step: view.step }; if (offset === 0) { page.detail = view.detail; if (view.file) page.file = view.file; } - page.tree = view.tree.slice(offset, offset + k); - page.changes = view.changes.slice(offset, offset + k); page.treeTotal = view.tree.length; page.changesTotal = view.changes.length; - if (offset + k < total) page.nextCursor = String(offset + k); - return fitAnswer(structuredClone(page), limit, ["tree", "changes"]); + const k = takeRows(n, offset, total, jlen(page) + 24, cap, (i) => rowLen(view.tree, i) + rowLen(view.changes, i)); + page.tree = view.tree.slice(offset, offset + k); + page.changes = view.changes.slice(offset, offset + k); + paging(page, offset, k, n, total); + return fitAnswer(page, cap); } /** A page of a range's view. */ -export function pageRange(view: RangeView, offset: number, n: number, limit: number): Obj { +export function pageRange(view: RangeView, offset: number, n: number, cap: number): Obj { const total = Math.max(view.files.length, view.unchanged.length); - const c = rowCosts(view, total, (i) => (i < view.files.length ? share(view.files[i]) : 0) + (i < view.unchanged.length ? jlen(view.unchanged[i]) : 0)); - const k = takeRows(n, offset, total, (i) => c[i]!); - const page: Obj = { sessionId: view.sessionId, from: view.from, to: view.to }; + const page: Obj = { sessionId: view.sessionId, from: view.from, to: view.to, filesTotal: view.files.length, unchangedTotal: view.unchanged.length }; + const k = takeRows(n, offset, total, jlen(page) + 28, cap, (i) => rowLen(view.files, i) + rowLen(view.unchanged, i)); page.files = view.files.slice(offset, offset + k); page.unchanged = view.unchanged.slice(offset, offset + k); - page.filesTotal = view.files.length; - page.unchangedTotal = view.unchanged.length; - if (offset + k < total) page.nextCursor = String(offset + k); - return fitAnswer(structuredClone(page), limit, ["files", "unchanged"]); + paging(page, offset, k, n, total); + return fitAnswer(page, cap); +} + +/** A page of the step list: steps and the session's file paths, side by side. */ +export function pageIndex(index: ReplayIndex, offset: number, n: number, cap: number): Obj { + const total = Math.max(index.steps.length, index.files.length); + const page: Obj = { sessionId: index.sessionId, total: index.steps.length, filesTotal: index.files.length, baseline: index.baseline }; + if (index.root !== undefined) page.root = index.root; + const k = takeRows(n, offset, total, jlen(page) + 24, cap, (i) => rowLen(index.steps, i) + rowLen(index.files, i)); + page.steps = index.steps.slice(offset, offset + k); + page.files = index.files.slice(offset, offset + k); + paging(page, offset, k, n, total); + return fitAnswer(page, cap); } diff --git a/packages/coding.sh/src/mcp/tools.ts b/packages/coding.sh/src/mcp/tools.ts index 2c5ae8b9..412e13a0 100644 --- a/packages/coding.sh/src/mcp/tools.ts +++ b/packages/coding.sh/src/mcp/tools.ts @@ -39,7 +39,7 @@ import { import { sessionDigest } from "./digest.ts"; import { facetCounts, searchWhere } from "../query/run.ts"; import { andQuery, FACET_KEYS, parseWhere, QueryError, QueryUnsupported, type Query } from "../query/where.ts"; -import { fitAnswer, pageRange, pageStep } from "./replay-page.ts"; +import { fitAnswer, pageIndex, pageRange, pageStep } from "./replay-page.ts"; import { toolText, ToolError, type ToolDef } from "./protocol.ts"; import { scoreTools } from "../scores/mcp.ts"; @@ -505,7 +505,7 @@ function toolDefs(): ToolDef[] { if (from !== undefined || to !== undefined) { if (from === undefined || to === undefined) throw new ToolError("Pass both from and to for a range."); const range = await replay.range(from, to); - return fitPage(limit, (n) => pageRange(range, offset, n, REPLAY_TEXT_LIMIT)); + return pageRange(range, offset, limit, REPLAY_TEXT_LIMIT); } if (a.fork !== undefined) { if (step === undefined) throw new ToolError("fork needs a step."); @@ -514,15 +514,11 @@ function toolDefs(): ToolDef[] { const file = strArg(a, "file"); if (step !== undefined) { const view = await replay.at(step, { file }); - return fitPage(limit, (n) => pageStep(view, offset, n, REPLAY_TEXT_LIMIT)); + return pageStep(view, offset, limit, REPLAY_TEXT_LIMIT); } if (file) throw new ToolError("file needs a step."); const index = replay.index(); - return fitPage(limit, (n) => { - const page: Obj = { ...index, steps: structuredClone(index.steps.slice(offset, offset + n)), files: index.files.slice(offset, offset + n), total, filesTotal: index.files.length }; - if (offset + n < Math.max(total, index.files.length)) page.nextCursor = String(offset + n); - return fitAnswer(page, REPLAY_TEXT_LIMIT, ["steps", "files"]); - }); + return pageIndex(index, offset, limit, REPLAY_TEXT_LIMIT); }, }, { diff --git a/packages/coding.sh/test/mcp/fit-answer.test.ts b/packages/coding.sh/test/mcp/fit-answer.test.ts new file mode 100644 index 00000000..b46604ab --- /dev/null +++ b/packages/coding.sh/test/mcp/fit-answer.test.ts @@ -0,0 +1,162 @@ +// fitAnswer and the replay pagers: an answer is cut to the character limit by one budget allocation, +// only when it does not fit, never in the ids and paths, and without a retry loop that can stall. +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { fitAnswer, pageIndex, pageRange, pageStep } from "../../src/mcp/replay-page.ts"; + +type Obj = Record; +const CAP = 24_940; +const len = (v: unknown) => JSON.stringify(v).length; +const step = { n: 2, ts: 1, kind: "edit", label: "Edit src/a.ts", eventId: "e" }; +const view = (detail: Obj, extra: Obj = {}) => ({ sessionId: "s", steps: 3, step, detail, tree: [], changes: [], ...extra }) as never; +const timed = (f: () => T): [T, number] => { + const t = performance.now(); + const r = f(); + return [r, performance.now() - t]; +}; + +describe("fitAnswer", () => { + it("leaves an answer that fits alone, with no note", () => { + const a = { x: "y".repeat(1000), list: [1, 2, 3] }; + assert.equal(fitAnswer(a, CAP), a); + }); + + it("fits 2,000 short keys, whatever their count, without cutting ids or the note", () => { + const input: Obj = {}; + for (let i = 0; i < 2000; i++) input[`key${i}`] = "v".repeat(20); + const tree = [{ path: `src/${"d".repeat(80)}.ts`, status: "modified", edits: 1, lastStep: 2 }]; + const p = pageStep(view({ input }, { tree }), 0, 100, CAP) as Obj; + assert.ok(len(p) <= CAP, `${len(p)} characters`); + assert.equal(p.truncated, true); + assert.match(p.note, /^Text and lists were cut .* one file's content\.$/); + assert.equal(p.noteTruncated, undefined); + assert.equal(p.tree[0].path, tree[0]!.path, "a path is never cut"); + assert.equal(p.detail.inputTruncated, true); + assert.equal(p.detail.input.truncatedKeys, undefined, "nothing is written inside a recorded input"); + assert.ok(Object.keys(p.detail.input).length > 100, "the first keys are kept whole"); + }); + + it("cuts text nested 12 levels (and 100 KB strings in nested arrays)", () => { + for (const depth of [1, 9, 10, 12, 40]) { + let o: unknown = { text: "z".repeat(100_000) }; + for (let i = 0; i < depth; i++) o = { child: o }; + const p = pageStep(view({ input: o as Obj }), 0, 100, CAP) as Obj; + assert.ok(len(p) <= CAP, `depth ${depth}: ${len(p)}`); + assert.equal(p.detail.inputTruncated, true); + } + let a: unknown = "z".repeat(100_000); + for (let i = 0; i < 12; i++) a = [a]; + assert.ok(len(pageStep(view({ input: { a } }), 0, 100, CAP)) <= CAP); + }); + + it("stays fast with thousands of keys holding small arrays (no quadratic retry)", () => { + for (const n of [1000, 4000, 20_000]) { + const input: Obj = {}; + for (let i = 0; i < n; i++) input[`k${i}`] = { tags: ["a", "b", "c", "d"] }; + const [p, ms] = timed(() => pageStep(view({ input }), 0, 100, CAP)); + assert.ok(len(p) <= CAP, `${n} keys: ${len(p)}`); + assert.ok(ms < 5000, `${n} keys took ${Math.round(ms)} ms`); + } + const [p, ms] = timed(() => pageStep(view({ input: { content: "x".repeat(5_000_000) } }), 0, 100, CAP)); + assert.ok(len(p) <= CAP && ms < 5000, `5 MB string: ${Math.round(ms)} ms`); + }); + + it("keeps a cut item of a list whose first item is large (two 8 KB edits; an 18 KB fork trial)", () => { + const edits = [0, 1].map((i) => ({ old_string: `old${i}`.padEnd(8000, "o"), new_string: `new${i}`.padEnd(8000, "n") })); + const p = pageStep(view({ input: { file_path: "src/b.ts", edits } }), 0, 100, CAP) as Obj; + assert.ok(len(p) <= CAP); + assert.equal(p.detail.input.edits.length, 2, "both edits stay, cut"); + assert.ok(p.detail.input.edits[0].old_string.length > 1000); + const fork = { + sessionId: "s", + recorded: { action: { name: "Write", input: { file_path: "src/a.ts", content: "r".repeat(9000) } } }, + trials: [{ action: { name: "Write", input: { file_path: "src/a.ts", content: "f".repeat(18_000) } }, label: "Write src/a.ts", differs: true }], + }; + const q = fitAnswer(structuredClone(fork), CAP) as Obj; + assert.ok(len(q) <= CAP); + assert.equal(q.trials.length, 1, "the trial stays, cut"); + assert.equal(q.trials[0].action.input.content.length > 1000, true); + assert.equal(q.trials[0].action.input.file_path, "src/a.ts"); + }); + + it("uses the budget: eight 16 KB diffs get about a third of the answer each", () => { + const diffs = Array.from({ length: 8 }, (_, i) => ({ name: `d${i}`, diff: "d".repeat(16_000) })); + const p = fitAnswer({ sessionId: "s", diffs }, CAP) as Obj; + assert.ok(len(p) <= CAP); + assert.ok(len(p) >= CAP * 0.9, `${len(p)} of ${CAP} used`); + const each = p.diffs.map((d: Obj) => d.diff.length); + assert.ok(Math.min(...each) >= 2800, `each diff keeps ${Math.min(...each)} characters`); + assert.ok(p.diffs.every((d: Obj) => d.diffTruncated === true)); + }); + + it("counts JSON escapes: 40 KB of lines is cut to what fits, not far below it", () => { + const p = pageStep(view({ output: "a\n".repeat(20_000) }), 0, 100, CAP) as Obj; + assert.ok(len(p) <= CAP); + assert.ok(len(p) >= CAP - 700, `${len(p)} characters of ${CAP}`); + assert.ok(p.detail.output.length > 8000, `kept ${p.detail.output.length} characters of 40,000`); + assert.equal(p.detail.outputTruncated, true); + }); + + it("cuts between characters: no half of an emoji", () => { + for (const s of ["😀".repeat(20_000), "漢".repeat(40_000), "a😀".repeat(15_000)]) { + const p = pageStep(view({ output: s }), 0, 100, CAP) as Obj; + assert.ok(len(p) <= CAP); + assert.doesNotMatch(p.detail.output as string, /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? { + const p = fitAnswer({ sessionId: "s".repeat(30_000), detail: { output: "x".repeat(500) }, n: 3 }, CAP) as Obj; + assert.ok(len(p) <= CAP, `${len(p)} characters`); + assert.deepEqual(p.sessionId, { truncated: true, chars: 30_002 }); + assert.equal(p.truncated, true); + assert.equal(p.n, 3); + }); +}); + +describe("the replay pagers choose whole rows first", () => { + it("a step list with fewer rows than asked keeps its rows uncut and says limitUsed", () => { + const steps = Array.from({ length: 150 }, (_, i) => ({ n: i + 1, ts: 1, kind: "shell", label: `$ cd packages/app && npm run test -- --filter ${"some-long-feature-name/".repeat(3)}Widget${i}.test.tsx --reporter=dot`.padEnd(400, "!"), eventId: `e${i}`, files: [{ path: `src/f${i}.ts`, op: "add" }] })); + const files = Array.from({ length: 150 }, (_, i) => `packages/app/src/features/some-long-feature-name/components/Widget${i}.tsx`); + const index = { sessionId: "s", steps, files, baseline: "none" } as never; + const seen: number[] = []; + let offset = 0; + for (let g = 0; g < 20; g++) { + const p = pageIndex(index, offset, 100, CAP) as Obj; + assert.ok(len(p) <= CAP); + if (g === 0) assert.ok(p.limitUsed > 0 && p.limitUsed < 100, `limitUsed ${p.limitUsed}`); + assert.equal(p.truncated, undefined, "no cut when whole rows fit"); + assert.ok(p.steps.every((s: Obj, i: number) => s.label === steps[offset + i]!.label && s.labelTruncated === undefined)); + seen.push(...p.steps.map((s: Obj) => s.n)); + if (!p.nextCursor) break; + offset = Number(p.nextCursor); + } + assert.equal(new Set(seen).size, 150); + }); + + it("a step page never cuts a tree path, and later pages are short", () => { + const tree = Array.from({ length: 150 }, (_, i) => ({ path: `packages/app/src/features/some-long-feature-name/components/Widget${String(i).padStart(4, "0")}.tsx`, status: "modified", edits: 1, lastStep: 2 })); + const v = view({ output: "o".repeat(9000) }, { tree }); + const p = pageStep(v, 0, 1000, CAP) as Obj; + assert.ok(len(p) <= CAP); + assert.ok(p.limitUsed > 0 && p.limitUsed < 150); + assert.equal(p.truncated, undefined, "fewer rows, not cut text"); + assert.ok(p.tree.every((t: Obj, i: number) => t.path === tree[i]!.path)); + assert.equal(p.detail.output.length, 9000); + }); + + it("a range page takes as many whole diffs as fit, and one that is too long alone is cut", () => { + const files = Array.from({ length: 30 }, (_, i) => ({ path: `src/f${i}.ts`, op: "modify", steps: [2], diff: "d".repeat(6000), exact: true })); + const p = pageRange({ sessionId: "s", from: 1, to: 3, files, unchanged: [] } as never, 0, 100, CAP) as Obj; + assert.equal(p.files.length, 4); + assert.equal(p.limitUsed, 4); + assert.equal(p.nextCursor, "4"); + assert.equal(p.truncated, undefined); + const big = [{ path: "src/big.ts", op: "modify", steps: [2], diff: "d".repeat(60_000), exact: true }]; + const q = pageRange({ sessionId: "s", from: 1, to: 3, files: big, unchanged: [] } as never, 0, 100, CAP) as Obj; + assert.ok(len(q) <= CAP); + assert.equal(q.files[0].diffTruncated, true); + assert.equal(q.files[0].path, "src/big.ts"); + assert.equal(q.truncated, true); + }); +}); diff --git a/packages/coding.sh/test/mcp/replay-limit.test.ts b/packages/coding.sh/test/mcp/replay-limit.test.ts index 1a42d46d..18e91dde 100644 --- a/packages/coding.sh/test/mcp/replay-limit.test.ts +++ b/packages/coding.sh/test/mcp/replay-limit.test.ts @@ -1,5 +1,5 @@ // session_replay stays under the 25,000-character limit in every view, with a session that touched -// 300 files of about 50 KB each: the step list, a step, a step with a file, and ranges. Pages by +// 300 files of about 2 KB each (and a few of 50 KB): the step list, a step, a step with a file, and ranges. Pages by // limit and cursor reach every file exactly once, and cut text says so. import assert from "node:assert/strict"; import { after, before, describe, it } from "node:test"; @@ -25,13 +25,13 @@ async function call(args: Record, sessionId = ID): Promise<{ sc } /** Every page of a view, following nextCursor. Each page is checked against the limit. */ -async function pages(args: Record): Promise { +async function pages(args: Record, sessionId = ID): Promise { const out: Sc[] = []; let cursor: string | undefined; for (let i = 0; i < 400; i++) { const next = { ...args }; if (cursor) next.cursor = cursor; - const { sc, len } = await call(next); + const { sc, len } = await call(next, sessionId); assert.ok(len <= CHARACTER_LIMIT, `page ${i} of ${JSON.stringify(args)} is ${len} characters`); out.push(sc); cursor = sc.nextCursor; @@ -53,8 +53,8 @@ before(async () => { const base = { v: 0 as const, sessionId: ID, agent: "claude-code", cwd: w.repo, model: "claude-opus-4-5" }; const evs: AgentEvent[] = [{ ...base, id: "f-0", ts: t0, kind: "session.start", payload: {} }, { ...base, id: "f-1", ts: t0 + 1, kind: "user.message", payload: { text: "Generate the files" } }]; for (let i = 0; i < FILES; i++) { - // About 50 KB of distinct lines per file. - const content = Array.from({ length: 1500 }, (_, l) => `export const v${i}_${l} = ${l * 7919 + i};`).join("\n") + "\n"; + // About 2 KB of distinct lines per file. + const content = Array.from({ length: 60 }, (_, l) => `export const v${i}_${l} = ${l * 7919 + i};`).join("\n") + "\n"; evs.push({ ...base, id: `f-s${i}`, ts: t0 + 10 + i * 10, kind: "tool.start", payload: { tool: "Write", toolUseId: `w${i}`, input: { file_path: path(i), content } } }); evs.push({ ...base, id: `f-e${i}`, ts: t0 + 15 + i * 10, kind: "tool.end", payload: { tool: "Write", toolUseId: `w${i}`, ok: true }, durationMs: 5 }); evs.push({ ...base, id: `f-c${i}`, ts: t0 + 16 + i * 10, kind: "file.change", payload: { path: path(i), op: "add" } }); @@ -88,6 +88,7 @@ before(async () => { { tool: "Write", input: { file_path: "src/d.ts", content: "x".repeat(20000) } }, ...Array.from({ length: 40 }, (_, k) => ({ tool: "Grep", input: { pattern: "x" }, output: "y".repeat(7700 + k * 20) })), ]); + await put("rv-big", "claude-code", Array.from({ length: 6 }, (_, i) => ({ tool: "Write", input: { file_path: `src/big${i}.ts`, content: Array.from({ length: 1500 }, (_, l) => `export const v${i}_${l} = ${l * 7919 + i};`).join("\n") + "\n" } }))); await ctx.close(); c = new RawClient(w.env, w.repo); await c.initialize(); @@ -120,8 +121,10 @@ describe("session_replay under the 25,000-character limit", () => { once(last.flatMap((p) => (p.tree as { path: string }[]).map((t) => t.path)), FILES, "tree"); for (const p of last) assert.equal(p.treeTotal, FILES); assert.ok(last[0]!.file.content.length > 1000, "the file keeps a useful start"); - assert.equal(last[0]!.truncated, true, "cut text is reported"); - assert.equal(last[0]!.file.contentTruncated, true); + assert.equal(last[0]!.truncated, undefined, "nothing was cut: whole rows fit"); + const shrunk = await pages({ step: total, file: path(FILES - 1), limit: 1000 }); + assert.ok(shrunk[0]!.limitUsed > 0 && shrunk[0]!.limitUsed < 1000, "a page with fewer rows than asked says limitUsed"); + once(shrunk.flatMap((p) => (p.tree as { path: string }[]).map((t) => t.path)), FILES, "tree, limit 1000"); }); it("keeps ranges under the limit and reaches every changed file once", async () => { @@ -130,15 +133,17 @@ describe("session_replay under the 25,000-character limit", () => { const wide = await pages({ from: 1, to: FILES + 1 }); once(wide.flatMap((p) => (p.files as { path: string }[]).map((f) => f.path)), FILES, "files of 1..300"); assert.ok(wide.every((p) => p.filesTotal === FILES)); - assert.ok(wide[0]!.truncated === true && wide[0]!.files[0].diffTruncated === true, "cut diffs are flagged"); - assert.ok(wide[0]!.files[0].diff.length > 100, "a diff keeps its start"); + assert.ok(wide.length > 1 && wide[0]!.limitUsed > 0, "whole diffs, fewer per page"); + assert.equal(wide[0]!.truncated, undefined); }); - it("gives one file's diff more room with limit 1", async () => { - const one = await call({ from: 1, to: FILES + 1, limit: 1 }); - assert.ok(one.len <= CHARACTER_LIMIT); - assert.ok(one.sc.files[0].diff.length > 10_000, `a lone diff is ${one.sc.files[0].diff.length} characters`); - assert.equal(one.sc.nextCursor, "1"); + it("returns each 50 KB file's diff as the replay recorded it (16 KB, flagged), one per page", async () => { + const all = await pages({ from: 1, to: 7 }, "rv-big"); + once(all.flatMap((p) => (p.files as { path: string }[]).map((f) => f.path)), 6, "big files"); + for (const p of all) { + assert.equal(p.files[0].diffTruncated, true, "the replay's own 16 KB cap is flagged"); + assert.ok(p.files[0].diff.length > 15_000, `a diff is ${p.files[0].diff.length} characters`); + } }); }); @@ -152,7 +157,7 @@ describe("session_replay cuts only what does not fit", () => { assert.equal(sc.detail.inputTruncated, true, "the cut input is flagged on detail"); assert.equal(sc.detail.input.editsTotal, undefined, "nothing is written inside the recorded input"); assert.equal(sc.detail.input.editsTruncated, undefined); - assert.ok(sc.detail.input.edits.length >= 1 && sc.detail.input.edits.length < 60); + assert.ok(sc.detail.input.edits.length >= 1, "at least one edit stays"); } }); @@ -164,12 +169,10 @@ describe("session_replay cuts only what does not fit", () => { assert.equal(sc.step.filesTotal, 500); assert.ok(sc.step.files.length < 500); } - const list = await call({}, "rv-patch"); - assert.ok(list.len <= CHARACTER_LIMIT, `step list: ${list.len} characters`); - const row = (list.sc.steps as { files?: unknown[]; filesTotal?: number }[]).find((x) => x.filesTotal === 500); + const rows: { files?: unknown[]; filesTotal?: number }[] = []; + for (const p of await pages({}, "rv-patch")) rows.push(...p.steps); + const row = rows.find((x) => x.filesTotal === 500); assert.ok(row && row.files!.length < 500, "a step's file list in the step list is capped, with its total"); - const pages = await pages_({ sessionId: "rv-patch" }); - assert.ok(pages.every((p) => p.len <= CHARACTER_LIMIT)); }); it("stays under the limit, note included, for a step whose content and output are near it", async () => { @@ -195,15 +198,3 @@ describe("session_replay cuts only what does not fit", () => { assert.equal(range.sc.files[0].diffTruncated, undefined); }); }); - -async function pages_(args: { sessionId: string }): Promise<{ len: number }[]> { - const out: { len: number }[] = []; - let cursor: string | undefined; - for (let i = 0; i < 50; i++) { - const r = await call(cursor ? { cursor } : {}, args.sessionId); - out.push({ len: r.len }); - cursor = r.sc.nextCursor; - if (!cursor) break; - } - return out; -} From 7a77649f1497261ede1afb0ead933e48f80de76e Mon Sep 17 00:00:00 2001 From: Jack <72348727+Jack-GitHub12@users.noreply.github.com> Date: Thu, 12 Nov 2026 00:07:17 -0600 Subject: [PATCH 11/11] fix(mcp): keep session_replay answers whole when one value is too big to cut --- apps/site/src/pages/docs/mcp.astro | 6 +-- packages/coding.sh/src/mcp/replay-page.ts | 35 +++++++++----- packages/coding.sh/src/mcp/tools.ts | 2 +- .../coding.sh/test/mcp/fit-answer.test.ts | 47 +++++++++++++++++-- 4 files changed, 71 insertions(+), 19 deletions(-) diff --git a/apps/site/src/pages/docs/mcp.astro b/apps/site/src/pages/docs/mcp.astro index 38b906dc..18c1e81e 100644 --- a/apps/site/src/pages/docs/mcp.astro +++ b/apps/site/src/pages/docs/mcp.astro @@ -120,7 +120,7 @@ const gemini = `// ~/.gemini/settings.json

session_replay pages the files of a step, of a range and of the step list with limit and - cursor. limit: 1 gives one file's diff up to 16 KB. + cursor. A diff or file over 16 KB is cut when the replay is built, and flagged diffTruncated or contentTruncated.

A page takes whole rows while they fit, so a short page has limitUsed and a nextCursor. It cuts only @@ -129,8 +129,8 @@ const gemini = `// ~/.gemini/settings.json

The answer then has truncated: true, a note, and a flag such as diffTruncated or - filesTruncated (with filesTotal, or truncatedKeys on an object) on what was cut. A cut inside a tool input is flagged on - detail as inputTruncated. + filesTruncated (with filesTotal, or truncatedKeys on an object) on what was cut. A cut inside a tool input is flagged as inputTruncated on the + object that holds the input (detail, or action in a fork answer), never inside the input.

Protocol

diff --git a/packages/coding.sh/src/mcp/replay-page.ts b/packages/coding.sh/src/mcp/replay-page.ts index 2c5f073d..2a6d3b92 100644 --- a/packages/coding.sh/src/mcp/replay-page.ts +++ b/packages/coding.sh/src/mcp/replay-page.ts @@ -16,12 +16,16 @@ import { toolText } from "./protocol.ts"; type Obj = Record; -/** The smallest share of characters worth cutting a child to; below it the child is dropped instead. */ +/** The smallest budget worth cutting one child to; below it the child is dropped instead. */ const MIN_SHARE = 200; +/** When the larger children would each get less than this, the first ones are kept whole instead of every one cut to a stub. */ +const SPLIT_SHARE = 400; +/** The budget never drops below this on a retry: below it the answer would lose its other entries. */ +const MIN_BUDGET = 1000; /** Room kept free for the page's cursor and `limitUsed`. */ const SLACK = 80; -const NOTE = "Text and lists were cut to keep the answer under 25,000 characters (the fields marked truncated, with their Total). A smaller limit gives each diff more room; step with file gives one file's content."; +const NOTE = "Text and lists were cut to keep the answer under 25,000 characters (the fields marked truncated, with their Total). One row, or the step itself, was over that on its own; the full text is in the source."; /** Keys whose text is never cut (ids, paths, names the reader needs to follow the answer). */ const PROTECTED = new Set(["note", "truncated", "nextCursor", "limitUsed", "sessionId", "id", "eventId", "toolUseId", "n", "path", "file", "kind", "tool", "op", "status", "root", "baseline", "from", "to"]); @@ -126,8 +130,9 @@ function allocate(root: Obj, infos: Map, budget: number): { out: O const rest: number[] = []; for (let i = 0; i < n; i++) { const v = I.vals[i]; - // Ids and paths, and numbers, booleans and null (a few characters each), are never cut or dropped. - if (!isContainer(v) && (typeof v !== "string" || (I.keys && PROTECTED.has(I.keys[i]!)))) remaining -= cost(i); + // At the answer's own levels, ids and paths, and numbers, booleans and null (a few characters each), are never + // cut or dropped. Inside a recorded tool input everything may be: it is data of any size. + if (!t.inInput && I.keys && !isContainer(v) && (typeof v !== "string" || PROTECTED.has(I.keys[i]!))) remaining -= cost(i); else rest.push(i); } let sum = 0; @@ -138,7 +143,7 @@ function allocate(root: Obj, infos: Map, budget: number): { out: O let left = order.length; let pos = 0; while (pos < order.length && cost(order[pos]!) <= rem / left) rem -= cost(order[pos++]!), left--; - if (left > 0 && rem / left >= MIN_SHARE) { + if (left > 0 && rem / left >= SPLIT_SHARE) { for (; pos < order.length; pos++) decide[order[pos]!] = Math.max(2, Math.floor(rem / left) - I.over[order[pos]!]!); } else if (left > 0) { // Too many children for each to get a useful share: keep the first ones whole, cut the next, drop the rest. @@ -213,21 +218,29 @@ export function fitAnswer(page: Obj, cap: number): Obj { } const len = jlen(out); if (len <= cap) return out; - budget -= len - cap + 32; + budget = Math.max(MIN_BUDGET, budget - (len - cap + 32)); } - out.truncated = true; - out.note = NOTE; + out = { ...page, truncated: true, note: NOTE }; + // Protected text alone is over the cap: shrink the largest top-level entries, keeping each one's type. const replaced = new Set(); for (let len = jlen(out); len > cap; len = jlen(out)) { let big: string | undefined; let bigSize = 0; for (const [k, v] of Object.entries(out)) { if (replaced.has(k) || k === "note" || k === "truncated" || k === "nextCursor" || k === "limitUsed") continue; - const s = jlen(v); - if (s > bigSize) [big, bigSize] = [k, s]; + const sz = jlen(v); + if (sz > bigSize) [big, bigSize] = [k, sz]; } if (big === undefined) break; - out[big] = { truncated: true, chars: bigSize }; + const v = out[big]; + if (Array.isArray(v)) { + if (out[`${big}Total`] === undefined) out[`${big}Total`] = v.length; + out[big] = []; + out[`${big}Truncated`] = true; + } else if (typeof v === "string") { + out[big] = cutString(v, 256); + out[`${big}Truncated`] = true; + } else out[big] = { truncated: true, chars: bigSize }; replaced.add(big); } return out; diff --git a/packages/coding.sh/src/mcp/tools.ts b/packages/coding.sh/src/mcp/tools.ts index 412e13a0..64f2b145 100644 --- a/packages/coding.sh/src/mcp/tools.ts +++ b/packages/coding.sh/src/mcp/tools.ts @@ -477,7 +477,7 @@ function toolDefs(): ToolDef[] { from: { type: "integer", minimum: 1, description: "With to: the first step of a range." }, to: { type: "integer", minimum: 1, description: "With from: the last step (included); each file's diff over the range." }, fork: { type: "object", description: 'With step: send its model call again with these files ({"AGENTS.md": text}); recorded vs forked next action. Calls the model.' }, - limit: { ...LIMIT(100, 1000), description: "Max files per page; fewer give each diff more room." }, + limit: { ...LIMIT(100, 1000), description: "Max files per page (fewer when they are long)." }, cursor: CURSOR, }, ["sessionId"], diff --git a/packages/coding.sh/test/mcp/fit-answer.test.ts b/packages/coding.sh/test/mcp/fit-answer.test.ts index b46604ab..574a405f 100644 --- a/packages/coding.sh/test/mcp/fit-answer.test.ts +++ b/packages/coding.sh/test/mcp/fit-answer.test.ts @@ -28,7 +28,7 @@ describe("fitAnswer", () => { const p = pageStep(view({ input }, { tree }), 0, 100, CAP) as Obj; assert.ok(len(p) <= CAP, `${len(p)} characters`); assert.equal(p.truncated, true); - assert.match(p.note, /^Text and lists were cut .* one file's content\.$/); + assert.match(p.note, /^Text and lists were cut .* the full text is in the source\.$/); assert.equal(p.noteTruncated, undefined); assert.equal(p.tree[0].path, tree[0]!.path, "a path is never cut"); assert.equal(p.detail.inputTruncated, true); @@ -105,12 +105,51 @@ describe("fitAnswer", () => { } }); - it("falls back to replacing the largest entry when protected text alone is too long", () => { - const p = fitAnswer({ sessionId: "s".repeat(30_000), detail: { output: "x".repeat(500) }, n: 3 }, CAP) as Obj; + it("keeps a recorded input's data cuttable: 9,000 numbers or a 40 KB string leave the rest of the answer whole", () => { + const tree = [{ path: "a.ts", status: "modified", edits: 1, lastStep: 2 }]; + const cases: Obj[] = [ + { input: { data: Array.from({ length: 9000 }, (_, i) => 1000 + i) }, output: "ok" }, + { input: { from: "a", to: "x".repeat(40_000) }, output: "ok" }, + { input: { note: "n".repeat(40_000), file: "f".repeat(40_000), id: "i".repeat(40_000), truncated: "t".repeat(40_000) }, output: "ok" }, + { input: { rows: Array.from({ length: 9000 }, (_, i) => i % 2 === 0) }, output: "ok" }, + ]; + for (const [i, detail] of cases.entries()) { + const p = pageStep(view(detail, { tree, changes: [{ path: "a.ts", op: "modify", exact: true, diff: "+x\n" }] }), 0, 100, CAP) as Obj; + assert.ok(len(p) <= CAP, `case ${i}: ${len(p)} characters`); + assert.deepEqual(p.step, step, `case ${i}: the step stays`); + assert.equal(p.tree[0].path, "a.ts", `case ${i}: the tree stays`); + assert.equal(p.changes[0].diff, "+x\n", `case ${i}: the changes stay`); + assert.equal(p.detail.output, "ok", `case ${i}: the output stays`); + assert.equal(p.detail.inputTruncated, true); + } + }); + + it("falls back to shrinking the largest entries, keeping their types, when protected text alone is too long", () => { + const p = fitAnswer({ sessionId: "s".repeat(30_000), detail: { output: "x".repeat(500) }, n: 3, tree: [{ path: "p" }] }, CAP) as Obj; assert.ok(len(p) <= CAP, `${len(p)} characters`); - assert.deepEqual(p.sessionId, { truncated: true, chars: 30_002 }); + assert.equal(typeof p.sessionId, "string", "a string stays a string"); + assert.equal(p.sessionIdTruncated, true); assert.equal(p.truncated, true); assert.equal(p.n, 3); + assert.ok(Array.isArray(p.tree)); + const rows = pageStep(view({ output: "ok" }, { tree: [{ path: "p".repeat(1_000_000), status: "added", edits: 1, lastStep: 2 }] }), 0, 100, CAP) as Obj; + assert.ok(len(rows) <= CAP, `a 1 MB path: ${len(rows)} characters`); + assert.ok(Array.isArray(rows.tree), "a list stays a list"); + assert.equal(rows.treeTruncated, true); + assert.deepEqual(rows.step, step, "the step stays"); + assert.equal(rows.detail.output, "ok"); + }); + + it("keeps fewer items whole when every item would lose most of its text", () => { + for (const n of [105, 124, 200]) { + const todos = Array.from({ length: n }, (_, i) => ({ id: i, content: "c".repeat(2000) })); + const p = pageStep(view({ input: { todos } }), 0, 100, CAP) as Obj; + assert.ok(len(p) <= CAP); + const kept = p.detail.input.todos as Obj[]; + assert.ok(kept.length < n && kept.length >= 8, `${kept.length} of ${n}`); + assert.ok(kept.slice(0, kept.length - 1).every((x) => x.content.length === 2000), "the items that stay are whole"); + assert.ok(len(p) >= CAP * 0.85, `${len(p)} of ${CAP} used`); + } }); });