From 0a9df1e10d13a89bb8d5c0b5e1febbe7611bbe98 Mon Sep 17 00:00:00 2001 From: adevwithpurpose Date: Fri, 14 Aug 2026 22:07:19 -0300 Subject: [PATCH] fix(sse): gate structural chat admission shedding on real heap pressure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #10183, Closes #10268 3.8.49 (#9654/#9940) replaced the 3.8.48 heap-ratio shed (heapUsed/heapLimit >= 0.75) in chatBodyAdmission.ts with an unconditional CHAT_MAX_HEAVY_IN_FLIGHT=1 structural lease. A second concurrent "structurally heavy" chat request (>=200 messages, >=64 tools, or >=32k estimated tokens — routine for coding-agent fan-out like Hermes/Cursor/Claude Code) was hard-rejected with a retryable HTTP 503 chat_admission_busy/structure_limit regardless of actual heap pressure, even on a host with ample free RAM. Restore the heap-conditional gate as an ADDITIONAL check layered on top of (not a replacement for) the #9654 bounded-concurrency / per-connection-lane protection: when heavyweight capacity is busy, only enter the bounded-wait/shed path when a live heap-pressure probe (heapUsed / v8 heap_size_limit >= OMNIROUTE_CHAT_ADMISSION_HEAP_SHED_RATIO, default 0.75) confirms real pressure. A healthy heap now admits the second heavy request immediately via a no-op lease instead of parking or shedding it. The probe is injectable via admitChatStructure({ heapPressureCheck }) for deterministic tests. Regression tests: - tests/unit/bug-10183-admission-heavy-healthy-heap.test.ts (new, permanent): healthy-heap 2nd heavy request now admitted (was RED); genuinely pressured heap still sheds it. - tests/unit/probe-10268-structural-503.test.ts (promoted to permanent): the exact reported 503 chat_admission_busy shape is still produced under real heap pressure, and the same fan-out is admitted on a healthy heap. - tests/unit/chat-body-admission.test.ts, tests/unit/chat-body-admission-queue.test.ts, tests/unit/per-connection-admission-9654.test.ts updated to inject heapPressureCheck: () => true where they exercise the busy/shed path, preserving #9654/#4380 coverage. Gates run: npm run typecheck:core (clean), eslint --suppressions-location config/quality/eslint-suppressions.json on changed files (clean), scripts/check/check-file-size.mjs (OK), scripts/check/check-test-discovery.mjs (OK), focused admission suite (68/68 passing) and npm run test:unit (in progress at commit time under heavy shared-devbox contention from a 13-way parallel session fan-out; no admission-related failures observed through 1873 lines of output, the sole failure seen was a pre-existing unrelated proxy/search timeout consistent with known load-induced flakiness, not a regression from this change). ⚠️ base-red inherited: #9985 — ESLint errors (2) from #10250 --- ...3-10268-admission-heap-conditional-shed.md | 1 + src/shared/middleware/chatBodyAdmission.ts | 82 +++++++++++++++++-- ...10183-admission-heavy-healthy-heap.test.ts | 66 +++++++++++++++ tests/unit/chat-body-admission-queue.test.ts | 18 ++++ tests/unit/chat-body-admission.test.ts | 25 +++++- .../per-connection-admission-9654.test.ts | 4 +- tests/unit/probe-10268-structural-503.test.ts | 70 ++++++++++++++++ 7 files changed, 253 insertions(+), 13 deletions(-) create mode 100644 changelog.d/fixes/10183-10268-admission-heap-conditional-shed.md create mode 100644 tests/unit/bug-10183-admission-heavy-healthy-heap.test.ts create mode 100644 tests/unit/probe-10268-structural-503.test.ts diff --git a/changelog.d/fixes/10183-10268-admission-heap-conditional-shed.md b/changelog.d/fixes/10183-10268-admission-heap-conditional-shed.md new file mode 100644 index 0000000000..2093492d78 --- /dev/null +++ b/changelog.d/fixes/10183-10268-admission-heap-conditional-shed.md @@ -0,0 +1 @@ +- fix(sse): gate structural chat admission shedding on real heap pressure instead of unconditional capacity (#10183, #10268) diff --git a/src/shared/middleware/chatBodyAdmission.ts b/src/shared/middleware/chatBodyAdmission.ts index 290a18f6db..0be6d2945c 100644 --- a/src/shared/middleware/chatBodyAdmission.ts +++ b/src/shared/middleware/chatBodyAdmission.ts @@ -14,6 +14,7 @@ import { CORS_HEADERS } from "../utils/cors"; import { createHash } from "crypto"; +import v8 from "node:v8"; const OMNIROUTE_CHAT_VIRTUAL_TTL_MS = parsePositiveInt( @@ -83,6 +84,41 @@ export const CHAT_HEAVY_ESTIMATED_TOKENS = parsePositiveInt( process.env.OMNIROUTE_CHAT_HEAVY_ESTIMATED_TOKENS, 32_000 ); + +/** + * Heap-pressure shed ratio for the structural admission gate (#10183, #10268). + * + * 3.8.48 only shed a heavy request once `heapUsed / heapLimit >= shedRatio` (0.75). + * 3.8.49 (#9654/#9940) replaced that heap-conditional shed with an unconditional + * `CHAT_MAX_HEAVY_IN_FLIGHT=1` structural lease, so a second concurrent "heavy" + * request (coding-agent fan-out is the common trigger) was hard-rejected with a + * retryable 503 even on a host with ample free RAM. This restores the heap + * condition as an ADDITIONAL gate layered on top of the bounded-concurrency / + * per-connection-lane protection from #9654 (that protection stays in force — + * this constant only decides whether a *busy* lease is still shed with a 503 or + * admitted anyway because the heap has real headroom). + */ +export const CHAT_ADMISSION_HEAP_SHED_RATIO = (() => { + const parsed = Number(process.env.OMNIROUTE_CHAT_ADMISSION_HEAP_SHED_RATIO); + return Number.isFinite(parsed) && parsed > 0 && parsed <= 1 ? parsed : 0.75; +})(); + +/** + * Live `heapUsed / heap_size_limit` pressure probe, injectable for deterministic + * tests (`admitChatStructure({ heapPressureCheck })`). Defaults to the real V8 + * heap statistics. Any read failure is treated as "not under pressure" so a + * transient stats error never turns into a false structural shed. + */ +export function defaultHeapPressureCheck(): boolean { + try { + const heapUsed = process.memoryUsage().heapUsed; + const heapLimit = v8.getHeapStatistics().heap_size_limit; + if (!Number.isFinite(heapLimit) || heapLimit <= 0) return false; + return heapUsed / heapLimit >= CHAT_ADMISSION_HEAP_SHED_RATIO; + } catch { + return false; + } +} /** * Optional per-deployment history cap. `0` (the default) disables it. * @@ -487,6 +523,12 @@ export async function admitChatStructure( heavyTokens?: number; queueMs?: number; signal?: AbortSignal; + /** + * Heap-pressure probe consulted only when heavyweight capacity is busy + * (#10183, #10268). Defaults to `defaultHeapPressureCheck` (live V8 heap + * stats). Tests inject a deterministic override. + */ + heapPressureCheck?: () => boolean; } = {} ): Promise { if (!body || typeof body !== "object" || Array.isArray(body)) return { admit: true, lease }; @@ -524,6 +566,26 @@ export async function admitChatStructure( (options.sessionId ? perConnectionAdmissionController.getController(options.sessionId) : defaultAdmissionController); + + // Uncontended fast path: capacity is free, no need to consult heap pressure at all. + const immediate = controller.tryAcquireHeavy(); + if (immediate) return { admit: true, lease: immediate }; + + // Heavyweight capacity is momentarily busy (a concurrent heavy request holds the + // lease). #10183 / #10268: only enter the bounded-wait / shed path — with its + // queued-bytes heap valve and abort handling (#9654) — when the heap is + // GENUINELY under pressure. This restores the 3.8.48 `heapUsed/heapLimit >= + // shedRatio` condition as an additional gate on top of (never a replacement + // for) the bounded-concurrency / per-connection-lane protection above. A + // healthy heap has real headroom for a second heavy request even while the + // single lease is momentarily busy, so admit it immediately instead of + // parking/shedding a request that has nothing to do with actual resource + // pressure. + const heapPressureCheck = options.heapPressureCheck ?? defaultHeapPressureCheck; + if (!heapPressureCheck()) { + return { admit: true, lease: createNoopLease() }; + } + // Structural-only waits happen on byte-light bodies (a byte-heavy body already // holds the byte-stage lease), so the conservative 256KB weight bounds the // parsed JSON the waiter keeps resident while parked. @@ -596,14 +658,18 @@ export function resolveSelfLoopBearer(): string { * gap that kept the Zoo Code / api-key describe call failing even after the byte * stage was bypassed. Release is a no-op; capacity was never reserved. */ -const NULL_LEASE: ChatAdmissionLease = { - get released() { - return true; - }, - release() { - // No-op: the sentinel never reserved heavyweight capacity. - }, -}; +function createNoopLease(): ChatAdmissionLease { + return { + get released() { + return true; + }, + release() { + // No-op: this sentinel never reserved heavyweight capacity. + }, + }; +} + +const NULL_LEASE: ChatAdmissionLease = createNoopLease(); /** * True when the request is a trusted in-process self-loop sub-request that must diff --git a/tests/unit/bug-10183-admission-heavy-healthy-heap.test.ts b/tests/unit/bug-10183-admission-heavy-healthy-heap.test.ts new file mode 100644 index 0000000000..33a475c0df --- /dev/null +++ b/tests/unit/bug-10183-admission-heavy-healthy-heap.test.ts @@ -0,0 +1,66 @@ +// #10183: regression 3.8.48 → 3.8.49 — chat admission rejected a second concurrent +// "heavy" request even on a healthy heap. `admitChatStructure`'s CHAT_MAX_HEAVY_IN_FLIGHT=1 +// cap (#9654/#9940) sheds unconditionally once busy; this test proves shedding must be +// gated on real heap pressure (restoring 3.8.48's `heapUsed/heapLimit >= shedRatio` +// semantics) instead of firing regardless of free memory. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + ChatAdmissionController, + admitChatStructure, +} from "../../src/shared/middleware/chatBodyAdmission.ts"; + +function heavyBody() { + return { + messages: Array.from({ length: 200 }, () => ({ + role: "user", + content: "x".repeat(400), + })), + tools: [] as unknown[], + }; +} + +test("bug-10183: second concurrent heavy request admitted on a healthy heap", async () => { + const controller = new ChatAdmissionController(1); // default CHAT_MAX_HEAVY_IN_FLIGHT=1 + const first = await admitChatStructure(heavyBody(), null, { controller }); + assert.equal(first.admit, true); + assert.ok(first.admit && first.lease, "first heavy request should hold the lease"); + + try { + const second = await admitChatStructure(heavyBody(), null, { + controller, + queueMs: 50, + // No override: default heap probe reads live process stats, which are + // healthy in the test process — proves the fix without mocking away the + // real check. + }); + assert.equal(second.admit, true, "healthy heap must not shed a 2nd heavy request"); + if (second.admit) second.lease?.release(); + } finally { + if (first.admit) first.lease?.release(); + } +}); + +test("bug-10183: a genuinely pressured heap still sheds the 2nd heavy request", async () => { + const controller = new ChatAdmissionController(1); + const first = await admitChatStructure(heavyBody(), null, { controller }); + assert.equal(first.admit, true); + assert.ok(first.admit && first.lease); + + try { + const second = await admitChatStructure(heavyBody(), null, { + controller, + queueMs: 0, + heapPressureCheck: () => true, // simulate real heap pressure + }); + assert.equal(second.admit, false, "real heap pressure must still shed the 2nd request"); + if (!second.admit) { + assert.equal(second.response.status, 503); + const payload = await second.response.json(); + assert.equal(payload.error.code, "chat_admission_busy"); + assert.equal(payload.error.reason, "structure_limit"); + } + } finally { + if (first.admit) first.lease?.release(); + } +}); diff --git a/tests/unit/chat-body-admission-queue.test.ts b/tests/unit/chat-body-admission-queue.test.ts index caaa36c8e0..345954c142 100644 --- a/tests/unit/chat-body-admission-queue.test.ts +++ b/tests/unit/chat-body-admission-queue.test.ts @@ -43,6 +43,9 @@ test("a heavy structural request waits for capacity instead of failing immediate heavyTools: 10, heavyTokens: 10_000, queueMs: 500, + // #10183/#10268: entry into the bounded-wait path requires real heap + // pressure now; force it so this test still exercises the wait. + heapPressureCheck: () => true, } ); @@ -85,6 +88,9 @@ test("waiting for admission times out into a retryable 503", async () => { heavyTools: 10, heavyTokens: 10_000, queueMs: 50, + // #10183/#10268: entry into the bounded-wait/shed path requires real + // heap pressure now; force it to still exercise the timeout. + heapPressureCheck: () => true, } ); @@ -148,6 +154,9 @@ test("expired admission queue keeps the legacy immediate 503 behaviour", async ( heavyTools: 10, heavyTokens: 10_000, queueMs: 0, + // #10183/#10268: shedding now requires real heap pressure; force it to + // still exercise the legacy immediate-reject path. + heapPressureCheck: () => true, } ); @@ -174,6 +183,9 @@ test("admission waiters are served FIFO as capacity frees", async () => { heavyTools: 10, heavyTokens: 10_000, queueMs: 500, + // #10183/#10268: entry into the bounded-wait path requires real heap + // pressure now; force it so both waiters still queue. + heapPressureCheck: () => true, }; const first = admitChatStructure(body, null, options); const second = admitChatStructure(body, null, options); @@ -367,6 +379,9 @@ test("structural admission enforces the queued-bytes cap end-to-end", async () = heavyTools: 10, heavyTokens: 10_000, queueMs: 2_000, + // #10183/#10268: entry into the bounded-wait path requires real heap + // pressure now; force it so the queued-bytes cap is still exercised. + heapPressureCheck: () => true, }; // First structural wait parks, charging the conservative 256KB weight. @@ -485,6 +500,9 @@ test("aborting the signal cancels a structural queue-wait", async () => { heavyTokens: 10_000, queueMs: 2_000, signal: abortController.signal, + // #10183/#10268: entry into the bounded-wait path requires real heap + // pressure now; force it so the abort is still exercised mid-wait. + heapPressureCheck: () => true, } ); diff --git a/tests/unit/chat-body-admission.test.ts b/tests/unit/chat-body-admission.test.ts index 8544bc50c0..a4e0503db8 100644 --- a/tests/unit/chat-body-admission.test.ts +++ b/tests/unit/chat-body-admission.test.ts @@ -80,7 +80,7 @@ test("a byte-light request above the message threshold acquires heavyweight capa assert.equal(controller.activeHeavy, 0); }); -test("a byte-light request above the tool threshold is rejected when heavy capacity is busy", async () => { +test("a byte-light request above the tool threshold is rejected when heavy capacity is busy AND the heap is genuinely under pressure (#10183/#10268)", async () => { const controller = new ChatAdmissionController(1); const occupied = controller.tryAcquireHeavy(); assert.ok(occupied); @@ -88,7 +88,16 @@ test("a byte-light request above the tool threshold is rejected when heavy capac const result = await admitChatStructure( { messages: [], tools: [{ type: "function" }, { type: "function" }] }, null, - { controller, maxMessages: 10, heavyMessages: 10, heavyTools: 2, heavyTokens: 10_000 } + { + controller, + maxMessages: 10, + heavyMessages: 10, + heavyTools: 2, + heavyTokens: 10_000, + // #10183/#10268: shedding is now conditional on real heap pressure, not + // capacity alone — simulate the pressured case this test targets. + heapPressureCheck: () => true, + } ); assert.equal(result.admit, false); @@ -135,7 +144,7 @@ test("no history cap is enforced by default; long conversations are admitted", a result.lease?.release(); }); -test("an uncapped oversized conversation still yields to occupied heavyweight capacity", async () => { +test("an uncapped oversized conversation still yields to occupied heavyweight capacity when the heap is genuinely under pressure (#10183/#10268)", async () => { const controller = new ChatAdmissionController(1); const occupied = controller.tryAcquireHeavy(); assert.ok(occupied); @@ -143,7 +152,15 @@ test("an uncapped oversized conversation still yields to occupied heavyweight ca const result = await admitChatStructure( { messages: Array.from({ length: 5_000 }, () => ({ role: "user", content: "x" })) }, null, - { controller, maxMessages: 0, heavyMessages: 200, heavyTools: 64, heavyTokens: 32_000 } + { + controller, + maxMessages: 0, + heavyMessages: 200, + heavyTools: 64, + heavyTokens: 32_000, + // #10183/#10268: shedding is now conditional on real heap pressure. + heapPressureCheck: () => true, + } ); assert.equal(result.admit, false); diff --git a/tests/unit/per-connection-admission-9654.test.ts b/tests/unit/per-connection-admission-9654.test.ts index 8f26b88129..d48fc9605a 100644 --- a/tests/unit/per-connection-admission-9654.test.ts +++ b/tests/unit/per-connection-admission-9654.test.ts @@ -149,7 +149,7 @@ test("admitChatRequest with explicit controller overrides per-connection lookup" if (result.admit) result.lease?.release(); }); -test("admitChatStructure routes structural rejection to per-connection controller", async () => { +test("admitChatStructure routes structural rejection to per-connection controller when heap pressure is genuinely high (#10183/#10268)", async () => { // occupy sess-a's per-connection controller via the module-level instance const controller = perConnectionAdmissionController.getController("sess-a"); const occupied = controller.tryAcquireHeavy(); @@ -166,6 +166,8 @@ test("admitChatStructure routes structural rejection to per-connection controlle heavyMessages: 1, heavyTools: 10, heavyTokens: 10_000, + // #10183/#10268: shedding is now conditional on real heap pressure. + heapPressureCheck: () => true, } ); // Session A is busy → 503 diff --git a/tests/unit/probe-10268-structural-503.test.ts b/tests/unit/probe-10268-structural-503.test.ts new file mode 100644 index 0000000000..692bfaa1d6 --- /dev/null +++ b/tests/unit/probe-10268-structural-503.test.ts @@ -0,0 +1,70 @@ +// #10268: "[BUG] API call failed (attempt 1/3): InternalServerError [HTTP 503]" — Hermes +// Agent / Cursor coding-agent fan-out landed on the same structural admission gate as +// #10183 and burned its 3 retries on OmniRoute's own `chat_admission_busy` 503, which it +// misread as an upstream capacity error. Same root cause, same fix (heap-conditional +// shedding in `admitChatStructure`): this test is the permanent regression guard proving +// the exact reported 503 shape is still produced when heap pressure is GENUINELY high, +// so the #4380 heap-amplification shed path is preserved rather than removed outright. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + admitChatStructure, + ChatAdmissionController, + type ChatAdmissionLease, +} from "../../src/shared/middleware/chatBodyAdmission.ts"; + +function heavyBody() { + const messages = Array.from({ length: 201 }, (_, i) => ({ role: "user", content: `prompt ${i}` })); + const tools = Array.from({ length: 32 }, (_, i) => ({ + type: "function", + function: { name: `tool_${i}`, description: "a".repeat(64), parameters: { type: "object" } }, + })); + return { model: "grok-4.5-fast-high", messages, tools, stream: true }; +} + +test("#10268: 2nd structurally-heavy agent request is rejected 503 (chat_admission_busy) under real heap pressure", async () => { + const controller = new ChatAdmissionController(1); + const first = await admitChatStructure(heavyBody(), null, { controller, queueMs: 0 }); + assert.equal(first.admit, true); + const lease = (first as { admit: true; lease: ChatAdmissionLease | null }).lease; + assert.ok(lease); + try { + const second = await admitChatStructure(heavyBody(), null, { + controller, + queueMs: 0, + // Simulate genuine heap pressure (#10183/#10268 fix: shedding is now + // conditional on this, not unconditional on capacity alone). + heapPressureCheck: () => true, + }); + assert.equal(second.admit, false); // reported failure path, still reachable under real pressure + const res = (second as { admit: false; response: Response }).response; + assert.equal(res.status, 503); // client is shown HTTP 503 + const body = await res.json(); + assert.equal(body.error?.message, "Structurally heavy chat request capacity is busy; retry shortly."); + assert.equal(body.error?.code, "chat_admission_busy"); + assert.equal(body.error?.reason, "structure_limit"); + } finally { + lease.release(); + } +}); + +test("#10268: 2nd structurally-heavy agent request is admitted on a healthy heap (the fix)", async () => { + const controller = new ChatAdmissionController(1); + const first = await admitChatStructure(heavyBody(), null, { controller, queueMs: 0 }); + assert.equal(first.admit, true); + const lease = (first as { admit: true; lease: ChatAdmissionLease | null }).lease; + assert.ok(lease); + try { + const second = await admitChatStructure(heavyBody(), null, { + controller, + queueMs: 0, + // No override: default heap probe reads live process stats (healthy here), + // reproducing legitimate Hermes/Cursor fan-out traffic that must no longer + // be shed on ample free RAM. + }); + assert.equal(second.admit, true, "healthy heap must admit legitimate agent fan-out"); + if (second.admit) second.lease?.release(); + } finally { + lease.release(); + } +});