From 0225a0a02dc340fe5a8337b1a3d6d74787f3b09e Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Sat, 20 Jun 2026 20:24:36 -0300 Subject: [PATCH] fix(translator): sanitize Read tool args from non-Anthropic models (#4451) Integrated into release/v3.8.32 --- CHANGELOG.md | 1 + open-sse/translator/helpers/toolCallShim.ts | 47 ++++++++-- tests/unit/translator-tool-call-shim.test.ts | 90 ++++++++++++++++++++ 3 files changed, 132 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cd83665d3..104260da2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ _In development — bullets added per PR; finalized at release._ ### 🐛 Fixed - **fix(embeddings):** forward output dimensions to Gemini for consistent embedding dims. (thanks @nguyenha935) +- **fix(translator):** sanitize Read tool args from non-Anthropic models to prevent retry loops. (thanks @GodrezJr2) - **fix(combo): round-robin members fail over faster under concurrency saturation via a configurable queue depth** — when a round-robin combo member was saturated, requests sat in the per-model semaphore's **unbounded** queue and only failed over to the next member after the full `queueTimeoutMs` (default 30s) elapsed — so a burst of agentic requests deep-queued one hot member instead of spilling to healthy ones. The per-model semaphore now accepts a bounded queue depth and emits `SEMAPHORE_QUEUE_FULL` once it is full (the round-robin loop already cascades on that code), so a configured low depth fails over immediately. A new `queueDepth` combo-config knob (global default / provider override / per-combo, default **20** for backward compatibility; **0** = never queue → fail over now) is exposed in Settings → Combo Defaults. ([#3872](https://github.com/diegosouzapw/OmniRoute/issues/3872) — thanks @KooshaPari) --- diff --git a/open-sse/translator/helpers/toolCallShim.ts b/open-sse/translator/helpers/toolCallShim.ts index cc92174386..43ca09481d 100644 --- a/open-sse/translator/helpers/toolCallShim.ts +++ b/open-sse/translator/helpers/toolCallShim.ts @@ -27,16 +27,51 @@ function coerceToArray(v: unknown): unknown[] { return []; } +// Claude Code's Read tool caps `limit` at 2000 lines per call. Non-Anthropic models +// (GPT-5.5, DeepSeek …) occasionally emit absurd values (e.g. `limit: 25999999999999999`) +// that Claude Code rejects, causing a retry loop that wastes tokens. Clamp here. +const READ_MAX_LIMIT = 2000; + +// `pages` is only meaningful for PDFs and only as `"N"` or `"N-M"` (1-based). +// Reference: claude-code-tools docs + upstream decolua/9router#1144. +function isValidPdfPagesArg(filePath: unknown, pages: unknown): boolean { + return ( + typeof filePath === "string" && + filePath.toLowerCase().endsWith(".pdf") && + typeof pages === "string" && + /^\d+(?:-\d+)?$/.test(pages) + ); +} + +function sanitizeReadArgs(args: Record): void { + // Coerce numeric-string limit/offset (some non-Anthropic models stringify everything). + if (typeof args.limit === "string" && /^\d+$/.test(args.limit)) { + args.limit = Number(args.limit); + } + if (typeof args.offset === "string" && /^-?\d+$/.test(args.offset)) { + args.offset = Number(args.offset); + } + + if (typeof args.limit === "number") { + if (args.limit > READ_MAX_LIMIT) args.limit = READ_MAX_LIMIT; + if (args.limit < 1) delete args.limit; + } + if (typeof args.offset === "number" && args.offset < 0) args.offset = 0; + + if ("pages" in args && !isValidPdfPagesArg(args.file_path, args.pages)) { + delete args.pages; + } +} + const TOOL_SHIMS: Record = { - // Claude Code Read accepts `pages` only for PDFs and rejects an empty string. - // Some non-Anthropic models emit optional `pages: ""` for ordinary files. - // Buffer and emit one cleaned JSON delta so the client never sees the bad field. + // Claude Code Read rejects bad params and retries — wasting tokens with non-Anthropic + // models that emit oversized limits, negative offsets, stringified numbers, or stray + // `pages` on non-PDF files. Buffer and emit one cleaned JSON delta so the client never + // sees the bad fields. See `sanitizeReadArgs` for the per-field rules. Read: (input) => { if (typeof input !== "object" || input === null || Array.isArray(input)) return input; const patched = { ...(input as Record) }; - if (patched.pages === "" || (Array.isArray(patched.pages) && patched.pages.length === 0)) { - delete patched.pages; - } + sanitizeReadArgs(patched); return patched; }, submit_pr_review: (input) => { diff --git a/tests/unit/translator-tool-call-shim.test.ts b/tests/unit/translator-tool-call-shim.test.ts index b5da99d951..2c5bde48df 100644 --- a/tests/unit/translator-tool-call-shim.test.ts +++ b/tests/unit/translator-tool-call-shim.test.ts @@ -75,6 +75,96 @@ test("applyToolCallShimToBuffer: Read removes empty pages but preserves valid ra assert.deepEqual(withValidPages, { file_path: "/tmp/a.pdf", pages: "1-5" }); }); +// Port of decolua/9router#1144: non-Anthropic models (GPT-5.5, DeepSeek …) sometimes +// emit absurd Read-tool args (e.g. limit: 99999999999) that Claude Code rejects and +// retries, wasting tokens. The shim clamps/normalizes those args before re-emitting. +test("applyToolCallShimToBuffer: Read clamps limit to 2000 (non-Anthropic models)", () => { + const out = JSON.parse( + applyToolCallShimToBuffer( + "Read", + JSON.stringify({ file_path: "/etc/hosts", limit: 99999999999 }) + ) + ); + assert.equal(out.limit, 2000); +}); + +test("applyToolCallShimToBuffer: Read drops non-positive limit", () => { + const zero = JSON.parse( + applyToolCallShimToBuffer("Read", JSON.stringify({ file_path: "/etc/hosts", limit: 0 })) + ); + assert.equal("limit" in zero, false); + + const negative = JSON.parse( + applyToolCallShimToBuffer("Read", JSON.stringify({ file_path: "/etc/hosts", limit: -50 })) + ); + assert.equal("limit" in negative, false); +}); + +test("applyToolCallShimToBuffer: Read clamps negative offset to 0", () => { + const out = JSON.parse( + applyToolCallShimToBuffer("Read", JSON.stringify({ file_path: "/etc/hosts", offset: -5 })) + ); + assert.equal(out.offset, 0); +}); + +test("applyToolCallShimToBuffer: Read coerces numeric-string limit/offset", () => { + const out = JSON.parse( + applyToolCallShimToBuffer( + "Read", + JSON.stringify({ file_path: "/etc/hosts", limit: "100", offset: "5" }) + ) + ); + assert.equal(out.limit, 100); + assert.equal(out.offset, 5); +}); + +test("applyToolCallShimToBuffer: Read strips pages for non-PDF files", () => { + const out = JSON.parse( + applyToolCallShimToBuffer( + "Read", + JSON.stringify({ file_path: "/etc/hosts", pages: "1-3" }) + ) + ); + assert.equal("pages" in out, false); +}); + +test("applyToolCallShimToBuffer: Read strips malformed pages even on PDFs", () => { + const out = JSON.parse( + applyToolCallShimToBuffer( + "Read", + JSON.stringify({ file_path: "/tmp/doc.pdf", pages: "abc" }) + ) + ); + assert.equal("pages" in out, false); +}); + +test("applyToolCallShimToBuffer: Read accepts a single page on PDFs", () => { + const out = JSON.parse( + applyToolCallShimToBuffer( + "Read", + JSON.stringify({ file_path: "/tmp/doc.PDF", pages: "7" }) + ) + ); + assert.equal(out.pages, "7"); +}); + +test("applyToolCallShimToBuffer: Read combined absurd args from non-Anthropic model", () => { + // Simulates the upstream issue exactly: GPT-5.5-style giant limit, negative offset, + // and a stray empty-string pages on a non-PDF. + const out = JSON.parse( + applyToolCallShimToBuffer( + "Read", + JSON.stringify({ + file_path: "F:/repo/file.js", + offset: -5, + limit: 25999999999999999, + pages: "", + }) + ) + ); + assert.deepEqual(out, { file_path: "F:/repo/file.js", offset: 0, limit: 2000 }); +}); + test("applyToolCallShimToBuffer: submit_pr_review with valid arrays preserved", () => { const raw = JSON.stringify({ summary: "ok",