diff --git a/.env.example b/.env.example index de212a773d..800a7801c8 100644 --- a/.env.example +++ b/.env.example @@ -370,6 +370,16 @@ ALLOW_API_KEY_REVEAL=false # OMNIROUTE_CHAT_HARD_MAX_BODY_BYTES=52428800 # Maximum heavyweight requests simultaneously admitted in one process. Default 1. # OMNIROUTE_CHAT_MAX_HEAVY_IN_FLIGHT=1 +# Heap-pressure shed ratio (heapUsed/heap_size_limit) for the structural admission gate +# (#10183, #10268): a second concurrent heavyweight request past OMNIROUTE_CHAT_MAX_HEAVY_IN_FLIGHT +# is only shed with a retryable 503 when the heap is ALSO under this much pressure — on a +# healthy heap it is admitted instead. Range (0, 1]. Default 0.75. +# OMNIROUTE_CHAT_ADMISSION_HEAP_SHED_RATIO=0.75 +# Bounded extra capacity for the healthy-heap fast path above OMNIROUTE_CHAT_MAX_HEAVY_IN_FLIGHT +# (#10437): once this many concurrent leases are active through the healthy-heap bypass, +# further busy requests fall through to the same bounded-wait/shed path used under real heap +# pressure. 0 disables the bypass entirely. Default 1. +# OMNIROUTE_CHAT_ADMISSION_HEALTHY_HEADROOM=1 # Message count that classifies an otherwise small body as heavyweight. Default 200. # OMNIROUTE_CHAT_HEAVY_MESSAGE_COUNT=200 # Tool count that classifies an otherwise small body as heavyweight. Default 64. 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..f99bba5035 --- /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, with a bounded headroom budget so a healthy heap can no longer bypass admission control indefinitely (#10183, #10268) diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index ba1c50f0ba..91a21fdec8 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -197,6 +197,8 @@ OmniRoute uses **SQLite** (via `better-sqlite3`) for all persistence. These vari | `OMNIROUTE_CHAT_LARGE_BODY_BYTES` | `262144` (256 KB) | `src/shared/middleware/chatBodyAdmission.ts` | Actual request bodies at or above this threshold require an atomic process-local heavyweight admission lease before JSON parsing. | | `OMNIROUTE_CHAT_HARD_MAX_BODY_BYTES` | `52428800` (50 MB) | `src/shared/middleware/chatBodyAdmission.ts` | Chat-route hard cap enforced against bytes read during bounded ingestion, including requests with missing, invalid, or dishonest `Content-Length`; excess receives `413`. | | `OMNIROUTE_CHAT_MAX_HEAVY_IN_FLIGHT` | `1` | `src/shared/middleware/chatBodyAdmission.ts` | Maximum heavyweight chat requests admitted concurrently in one process. When capacity is unavailable, OmniRoute returns retryable `503` with `Retry-After`. | +| `OMNIROUTE_CHAT_ADMISSION_HEAP_SHED_RATIO` | `0.75` | `src/shared/middleware/chatBodyAdmission.ts` | Heap-pressure shed ratio (`heapUsed / heap_size_limit`) for the structural admission gate (#10183, #10268). A second concurrent heavyweight request past `OMNIROUTE_CHAT_MAX_HEAVY_IN_FLIGHT` is only shed with the retryable `503` when the heap is ALSO at or above this ratio; on a healthy heap it is admitted instead. | +| `OMNIROUTE_CHAT_ADMISSION_HEALTHY_HEADROOM` | `OMNIROUTE_CHAT_MAX_HEAVY_IN_FLIGHT` (default `1`) | `src/shared/middleware/chatBodyAdmission.ts` | Bounded extra capacity for the healthy-heap fast path above (#10437). Without this bound, every busy-but-healthy-heap request bypassed admission with no ceiling at all — a slow leak or a burst that never quite trips the heap-shed ratio could still pile up unlimited concurrent heavyweight work. Once this many concurrent leases are active through the healthy-heap path, further busy requests fall through to the SAME bounded-wait/shed path used under real heap pressure. `0` disables the bypass entirely. | | `OMNIROUTE_CHAT_HEAVY_MESSAGE_COUNT` | `200` | `src/shared/middleware/chatBodyAdmission.ts` | Message count that classifies a chat request as heavyweight even when its body is below the byte threshold. | | `OMNIROUTE_CHAT_HEAVY_TOOL_COUNT` | `64` | `src/shared/middleware/chatBodyAdmission.ts` | Tool count that classifies a chat request as heavyweight even when its body is below the byte threshold. | | `OMNIROUTE_CHAT_HEAVY_ESTIMATED_TOKENS` | `32000` | `src/shared/middleware/chatBodyAdmission.ts` | Conservative string-size token estimate that classifies a request as heavyweight; this is an admission-cost proxy, not provider billing tokenization. | diff --git a/src/shared/middleware/chatBodyAdmission.ts b/src/shared/middleware/chatBodyAdmission.ts index e85fce30ec..490022b612 100644 --- a/src/shared/middleware/chatBodyAdmission.ts +++ b/src/shared/middleware/chatBodyAdmission.ts @@ -17,6 +17,7 @@ import { CORS_HEADERS } from "../utils/cors"; import { createHash } from "crypto"; +import v8 from "node:v8"; function parsePositiveInt(value: string | undefined, fallback: number): number { const parsed = Number.parseInt(String(value), 10); @@ -80,6 +81,60 @@ 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; +})(); + +/** + * Bounded extra capacity for the "healthy heap" fast path (#10437). + * + * The #10183/#10268 fix above admits a busy heavyweight request immediately whenever + * `heapPressureCheck()` is false — but with no bound of its own, that path let an + * UNLIMITED number of "healthy heap" requests pile in ahead of the heap-pressure + * shed, defeating the point of admission control: a slow leak or a burst that never + * quite trips the heap-pressure ratio could still starve the process. This constant + * caps how many requests may bypass the primary `CHAT_MAX_HEAVY_IN_FLIGHT` lease via + * the healthy-heap path at once (tracked independently, per `ChatAdmissionController` + * instance — see `#activeHealthy` / `tryAcquireHealthyHeadroom`). Once this budget is + * also exhausted, requests fall through to the SAME bounded-wait/shed path used under + * real heap pressure, so there is still a real ceiling either way. + */ +export const CHAT_ADMISSION_HEALTHY_HEADROOM = parseNonNegativeInt( + process.env.OMNIROUTE_CHAT_ADMISSION_HEALTHY_HEADROOM, + CHAT_MAX_HEAVY_IN_FLIGHT +); + +/** + * 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. * @@ -122,6 +177,11 @@ interface AdmissionWaiter { export class ChatAdmissionController { #activeHeavy = 0; #queuedBytes = 0; + /** #10437: independent counter for the bounded "healthy heap" headroom budget — + * separate from `#activeHeavy` so it never inflates the documented + * `CHAT_MAX_HEAVY_IN_FLIGHT` bound, but still a real, finite ceiling instead of + * the unconditional bypass this replaces. */ + #activeHealthy = 0; /** Per-key FIFOs. A key groups one client's waiters so they are served * round-robin against the shared budget instead of monopolizing a strict * FIFO (see #dispatchFair). */ @@ -132,7 +192,11 @@ export class ChatAdmissionController { constructor( readonly maxHeavyInFlight = 1, - readonly maxQueuedBytes = CHAT_ADMISSION_MAX_QUEUED_BYTES + readonly maxQueuedBytes = CHAT_ADMISSION_MAX_QUEUED_BYTES, + /** #10437: bounded extra capacity for the healthy-heap fast path. `0` disables + * the bypass entirely — every busy request then falls through to the same + * bounded-wait/shed path used under real heap pressure. */ + readonly healthyHeadroom = CHAT_ADMISSION_HEALTHY_HEADROOM ) { if (!Number.isSafeInteger(maxHeavyInFlight) || maxHeavyInFlight < 1) { throw new RangeError("maxHeavyInFlight must be a positive integer"); @@ -140,12 +204,44 @@ export class ChatAdmissionController { if (!Number.isSafeInteger(maxQueuedBytes) || maxQueuedBytes < 0) { throw new RangeError("maxQueuedBytes must be a non-negative integer"); } + if (!Number.isSafeInteger(healthyHeadroom) || healthyHeadroom < 0) { + throw new RangeError("healthyHeadroom must be a non-negative integer"); + } } get activeHeavy(): number { return this.#activeHeavy; } + /** Active leases held through the bounded healthy-heap headroom budget (#10437). */ + get activeHealthyHeadroom(): number { + return this.#activeHealthy; + } + + /** + * Acquire one slot from the bounded, independent healthy-heap headroom budget + * (#10437). Unlike `tryAcquireHeavy()`, this never contends with the primary + * `maxHeavyInFlight` lease — it exists ONLY to give the "heap has real + * headroom" fast path a finite ceiling instead of an unconditional bypass. + * Returns `null` once `healthyHeadroom` concurrent leases are already active, + * at which point the caller must fall through to the bounded-wait/shed path. + */ + tryAcquireHealthyHeadroom(): ChatAdmissionLease | null { + if (this.#activeHealthy >= this.healthyHeadroom) return null; + this.#activeHealthy += 1; + let released = false; + return { + get released() { + return released; + }, + release: () => { + if (released) return; + released = true; + this.#activeHealthy = Math.max(0, this.#activeHealthy - 1); + }, + }; + } + /** Total buffered bytes currently parked across all queues (heap valve accounting). */ get queuedBytes(): number { return this.#queuedBytes; @@ -523,6 +619,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 }; @@ -560,6 +662,34 @@ 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()) { + // #10437: the healthy-heap fast path must still have a real ceiling — an + // unconditional bypass here let unlimited concurrent "healthy heap" + // requests pile in ahead of the heap-pressure shed, defeating admission + // control entirely. Reserve from a separate, bounded headroom budget + // instead of an unconditional no-op lease; only fall through to the + // bounded-wait/shed path below (identical to the real-pressure case) once + // that budget is also exhausted. + const headroomLease = controller.tryAcquireHealthyHeadroom(); + if (headroomLease) return { admit: true, lease: headroomLease }; + } + // 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. @@ -633,14 +763,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-admission-healthy-headroom-10437.test.ts b/tests/unit/chat-admission-healthy-headroom-10437.test.ts new file mode 100644 index 0000000000..f668e46759 --- /dev/null +++ b/tests/unit/chat-admission-healthy-headroom-10437.test.ts @@ -0,0 +1,117 @@ +// #10437: the #10183/#10268 fix admitted a busy heavyweight request immediately +// whenever the heap was healthy, via an unconditional no-op lease — with no bound +// of its own. That let an UNLIMITED number of "healthy heap" requests pile in ahead +// of the heap-pressure shed path, defeating the purpose of admission control: a +// slow leak (or a burst that never quite trips the heap-pressure ratio) could still +// starve the process. This is the permanent regression guard proving the +// healthy-heap fast path now has a real, finite ceiling (`healthyHeadroom`) and +// falls through to the SAME bounded-wait/shed path used under real heap pressure +// once that budget is exhausted — the existing #10183/#10268 heap-pressure gate is +// preserved unchanged; only the previously-unbounded healthy path is now bounded. +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(40) })), + tools: [] as unknown[], + }; +} + +const heapHealthy = () => false; // "not under pressure" — the healthy-heap fast path + +test("#10437: the healthy-heap fast path admits only a bounded headroom budget, never unlimited requests", async () => { + const HEALTHY_HEADROOM = 2; + // maxHeavyInFlight=1 (the primary structural lease); healthyHeadroom=2 is the + // ADDITIONAL bounded budget available only while the heap stays healthy. + const controller = new ChatAdmissionController(1, undefined, HEALTHY_HEADROOM); + + // Occupy the single primary lease directly, simulating one in-flight heavy + // request — every subsequent admission below must go through the healthy-heap + // fast path (busy primary capacity + healthy heap). + const primary = controller.tryAcquireHeavy(); + assert.ok(primary); + + // Fire 3 CONCURRENT structurally-heavy requests on a healthy heap while the + // primary lease is busy. Pre-fix, `admitChatStructure` returned a fresh no-op + // lease for every single one of them, unconditionally — no ceiling existed. + // Post-fix, only HEALTHY_HEADROOM (2) may bypass through the bounded headroom + // budget; the remaining request must fall through to the bounded-wait/shed + // path (queueMs=0 → immediate retryable 503), exactly like real heap pressure. + const results = await Promise.all( + Array.from({ length: 3 }, () => + admitChatStructure(heavyBody(), null, { + controller, + heapPressureCheck: heapHealthy, + queueMs: 0, + }) + ) + ); + + const admitted = results.filter((r) => r.admit); + const rejected = results.filter((r) => !r.admit); + + assert.equal( + admitted.length, + HEALTHY_HEADROOM, + "only the finite healthy-headroom budget may bypass a busy primary lease on a healthy heap" + ); + assert.equal( + rejected.length, + 3 - HEALTHY_HEADROOM, + "once the headroom budget is exhausted, further healthy-heap requests must be shed, not silently admitted" + ); + for (const r of rejected) { + if (r.admit) continue; + assert.equal(r.response.status, 503); + const payload = await r.response.json(); + assert.equal(payload.error.code, "chat_admission_busy"); + assert.equal(payload.error.reason, "structure_limit"); + } + + assert.equal( + controller.activeHealthyHeadroom, + HEALTHY_HEADROOM, + "the headroom budget tracks its own active count independently of the primary lease" + ); + + primary.release(); + for (const r of admitted) if (r.admit) r.lease?.release(); + assert.equal(controller.activeHealthyHeadroom, 0, "released headroom leases free the budget"); +}); + +test("#10437: healthyHeadroom=0 disables the fast-path bypass entirely — every busy healthy-heap request is bounded by the shed path", async () => { + const controller = new ChatAdmissionController(1, undefined, 0); + const primary = controller.tryAcquireHeavy(); + assert.ok(primary); + + const result = await admitChatStructure(heavyBody(), null, { + controller, + heapPressureCheck: heapHealthy, + queueMs: 0, + }); + + assert.equal(result.admit, false, "with a zero headroom budget, a busy healthy-heap request must be shed"); + if (!result.admit) assert.equal(result.response.status, 503); + primary.release(); +}); + +test("#10437: the healthy-heap headroom budget still lets legitimate agent fan-out through up to its bound", async () => { + // Default headroom (>= 1) must still admit at least one bypass, matching the + // #10183/#10268 fix's original intent — this is not a regression to always-shed. + const controller = new ChatAdmissionController(1); + const primary = controller.tryAcquireHeavy(); + assert.ok(primary); + + const result = await admitChatStructure(heavyBody(), null, { + controller, + heapPressureCheck: heapHealthy, + }); + assert.equal(result.admit, true, "at least the default headroom budget must admit a healthy-heap request"); + if (result.admit) result.lease?.release(); + primary.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 ccfd494c0a..05af2deda6 100644 --- a/tests/unit/per-connection-admission-9654.test.ts +++ b/tests/unit/per-connection-admission-9654.test.ts @@ -169,8 +169,8 @@ 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 () => { - // occupy sess-a's controller — which is the shared process-global budget +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(); assert.ok(occupied); @@ -186,6 +186,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, } ); // The process-wide slot is busy → 503 @@ -203,7 +205,11 @@ test("admitChatStructure with different sessionId shares the global budget", asy assert.ok(occupied); // Session B must NOT get independent capacity (pre-#10110 it did — that was - // the defect): it shares the one process-wide slot and must be rejected. + // the defect): it shares the one process-wide slot and must be rejected — + // under real heap pressure. #10183/#10268 layered a heap-conditional gate on + // top of this shed path (a healthy heap now gets a bounded headroom slot + // instead of an outright 503), so this test forces genuine pressure to keep + // exercising the #10110 shared-budget invariant it targets. const result = await admitChatStructure( { messages: Array.from({ length: 500 }, () => ({ role: "user", content: "x" })), @@ -215,6 +221,7 @@ test("admitChatStructure with different sessionId shares the global budget", asy heavyMessages: 200, heavyTools: 64, heavyTokens: 32_000, + heapPressureCheck: () => true, } ); assert.equal(result.admit, false); 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(); + } +});