Bound JSON-RPC batch fan-out and stop retrying permanent Summarizer failures

[kbennett2000]

Bound JSON-RPC batch fan-out and stop retrying permanent Summarizer failures

Closes #277.

Today a single accepted POST to /mcp can dispatch an unbounded number of tool calls, and each brave_summarizer call can issue up to 20 outbound Brave Search API requests. The two compound: one HTTP request can consume a large share of a monthly quota.

This bounds the fan-out at both ends. It does not add a rate limiter — that needs a decision about scope and durability that belongs to the maintainers (see "Deliberately out of scope").

Changes

src/protocols/batch.ts (new) — cap JSON-RPC batch length. Middleware in the same shape as createDnsRebindingGuard, applied after the JSON parser and before the MCP SDK. Arrays longer than HTTP_LIMITS.maxBatchSize get a 400 with a JSON-RPC error and a null id, per the Streamable HTTP spec. Non-array bodies — the common single-message case — pass through untouched.

src/protocols/http.ts — state the body limit explicitly. express.json() was inheriting body-parser's 100kb default. It now declares HTTP_LIMITS.maxBodySize (64kb) so the bound is a decision rather than a side effect. This is defense in depth; the batch cap is what actually bounds fan-out.

src/BraveAPI/index.ts — BraveApiError carries the HTTP status. issueRequest threw a bare Error with the status interpolated into the message, so callers had no way to tell a 401 from a 503 without parsing strings. BraveApiError extends Error with the same message, so existing catch blocks are unaffected.

src/tools/summarizer/index.ts — retry only what retrying can fix. The poll loop now stops immediately on deterministic failures (401, 403, 422) and keeps retrying only 429 and 5xx. An invalid key costs one request instead of twenty. Errors with no status (network faults, parse failures) are still treated as transient. The loop also honors the AbortSignal the MCP SDK already passes to tool handlers, so a cancelled request stops polling.

One thing #277 didn't catch

The report describes the loop as polling "up to 20 times at 50ms intervals." That was only true on the error path. The await on pollInterval sat inside the catch block, so a well-formed response that wasn't yet status: 'complete' re-polled with no delay at all — all remaining attempts fired back to back as fast as the network allowed. The delay now sits between attempts regardless of outcome. There's a regression test for it.

Effect

	before	after
max messages per request	931 (body-limit bound)	10
requests per summarizer call, invalid key	20	1
worst case per HTTP request	18,620	200
pacing when summary is pending	none	50ms between polls

Tests

src/protocols/batch.test.ts and src/tools/summarizer/index.test.ts, in the existing node:test style. Includes the 931-message batch from the report, asserted to stay under the body limit so it proves the batch cap fires on its own rather than the parser catching it.

64/64 pass; tsc --noEmit and prettier --check clean.

Deliberately out of scope

• The rate limiter. checkRateLimit() is still commented out in issueRequest. As written it's process-global with no durable monthly accounting, so restoring it as-is would be misleading rather than protective. A real limiter needs a decision on scope (per key? per session?) and whether state must survive restarts and multiple replicas. Happy to follow up if there's a direction you'd like. This also overlaps Enforce rate limit locally (and support automatic retries) #238.
• Exponential backoff. Left the interval fixed to preserve current latency. Backoff changes worst-case tool duration and seemed worth deciding separately.

Choosing the constants

Both live in src/constants.ts as HTTP_LIMITS. maxBatchSize: 10 is the value proposed in #277; maxBodySize: '64kb' is well above any legitimate request these tools take while staying under the inherited default. Both are one-line changes if you'd rather be more permissive — the security property holds at any bounded value.