[RaviTharuma]

Summary

• atomically reserve heavyweight chat capacity before expensive parsing
• bound actual bytes rather than trusting Content-Length
• sniff unknown-length small bodies without consuming heavyweight capacity
• hold and release leases across normal, streaming, cancellation, error, and early-keepalive lifecycles
• return controlled 413 or retryable 503 instead of risking process termination

This PR intentionally adds no waiting queue; bounded queueing remains separate under #7849.

Validation

• 20 focused admission and early-stream tests pass
• npm run typecheck:core
• focused ESLint and git diff --check

Closes #7846

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[gemini-code-assist]

Code Review

This pull request replaces the heap-pressure-aware admission guard for POST /v1/chat/completions with an atomic, process-local heavyweight admission lease system that limits concurrent heavyweight requests and enforces hard byte bounds before parsing. It also updates the early stream keepalive utility to cancel the upstream response when a client aborts. The review feedback correctly identifies two critical issues: a potential lease leak if handleChat rejects during streaming, and a runtime TypeError in the keepalive utility if result.response.body is null when attempting to cancel the stream.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[RaviTharuma]

Addressed both review findings in 83cb305: handler rejection now releases admission through a tested lifecycle helper, and the pre-aborted/bodyless 204 path checks response.body before cancellation. Added regressions for both cases; 22 focused admission/keepalive tests plus core typecheck and ESLint pass.

[diegosouzapw]

Thanks for tackling #7846 — the direction here is exactly right: atomic reserve-before-parse, actual-byte ingestion that doesn't trust Content-Length, and a lease held through the full stream lifecycle. That's a real fix for the TOCTOU race described in the issue, and the test suite for the lifecycle paths (fast success/failure, non-streaming, stream close/cancel/read-error) is solid.

I did find one gap that I'd consider blocking before merge, and it's the same one gemini-code-assist flagged on this PR: the heavyweight lease leaks permanently when handleChat() rejects after withEarlyStreamKeepalive has already committed to its slow (200 SSE) path.

In src/app/api/v1/chat/completions/route.ts:

const handlerResponse = handleChat(request, null, parsedBody, reqId).then(finishAdmission);
const streamedResponse = await withEarlyStreamKeepalive(handlerResponse, {...});

.then(finishAdmission) has no rejection handler. If handleChat() rejects after the keepalive threshold has already opened the SSE stream, withEarlyStreamKeepalive's slow path converts that rejection into an in-band event: error frame instead of rethrowing — so the outer try { ... } catch (error) { admission.lease?.release(); throw error; } never runs, and the lease is never released.

I reproduced this directly against your ChatAdmissionController + withEarlyStreamKeepalive wiring: after a simulated handleChat rejection timed to land on the slow path, activeHeavy stayed at 1 even after the response settled. With the shipped default OMNIROUTE_CHAT_MAX_HEAVY_IN_FLIGHT=1, a single such failure (any upstream error, combo exhaustion, etc. during a slow-to-first-byte large request) permanently zeroes the only heavyweight slot process-wide — every subsequent large chat request gets 503 chat_admission_busy until the process restarts. Given handleChat() has no single top-level try/catch around most of its body, this is a reachable production path, not just a theoretical one.

Suggested fix:

const handlerResponse = handleChat(request, null, parsedBody, reqId).then(finishAdmission, (error) => {
  admission.lease?.release();
  throw error;
});

plus a regression test pinning "handler rejects on the slow keepalive path releases the lease" — happy to see that land alongside the existing chat-body-admission.test.ts suite.

Everything else here (the atomic reservation itself, the actual-byte hard cap, the .env.example/docs updates, ESLint cleanliness) looks good to me. Once the lease-release path is fixed and covered by a test, this should be ready.

(Separately, worth a quick look before/at merge time: the default OMNIROUTE_CHAT_MAX_HEAVY_IN_FLIGHT=1 caps concurrent >=256KB requests at 1 for the whole process regardless of heap health, stricter than the heap-pressure guard it replaces — you note the queueing follow-up is #7849, so this may be an intentional interim tradeoff, just flagging it since it's a new production default.)

[diegosouzapw]

Follow-up after reconciling this against #7862 — both PRs restructure the same block of src/app/api/v1/chat/completions/route.ts, and a simulated merge conflicts in both directions, so this is a real overlap, not stale-base noise.

Suggested land order for the cluster: #7852 → #7855 → #7862 → this PR last, rebased on top of #7862's tree.

Why this order: #7862 is a tiny, self-contained change whose regression test pins the parse-once contract, and this PR is the functional superset once reconciled. When you rebase, please also drop the request.clone() + .json() this branch still keeps: admitChatRequest() already buffers the body exactly once into a Uint8Array, so you can parse straight from those bytes (e.g. JSON.parse(new TextDecoder().decode(bodyBytes))) instead of cloning the rebuilt Request. That way the merged result keeps both improvements — your admission/capacity control and #7862's parse-once discipline — instead of one clobbering the other.

To be clear about severity: your current clone operates on an in-memory buffered body, so it does not resurrect the #4380/#7847 heap-amplification issue — it's just redundant once #7862 lands, and resolving it during your rebase avoids a needless conflict.

And thanks for the fast turnaround on the release-on-failure path (fix: release admission on handler failure) — that was the one blocking item from the review.

[diegosouzapw]

Merged — thanks @RaviTharuma for the whole trilogy (#7852/#7855/#7862/#7853 all landed today)! The atomic admission reservation closes the #7846 TOCTOU for real, your lease-release turnaround was fast, and the final rebase now composes it with the #7862 parse-once discipline (single materialization in the bounded byte reader, zero clones). Validated in the local merge-train on tomni-proxmox-113 @ 9acbe4266 (fast gates green; the 14-PR full-suite train earlier today covered the same route stack) then squash-merged.