Skip to content

fix(admission): queue heavyweight chat requests before 503 busy - #9816

Closed
herjarsa wants to merge 1812 commits into
diegosouzapw:mainfrom
herjarsa:fix/chat-admission-queue
Closed

herjarsa wants to merge 1812 commits into
diegosouzapw:mainfrom
herjarsa:fix/chat-admission-queue

Conversation

@herjarsa

@herjarsa herjarsa commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Problem

Agent-style clients (OpenCode, Claude Code, Cursor) fan out heavy sub-requests that land on the chat admission gate together. With the single heavyweight slot (OMNIROUTE_CHAT_MAX_HEAVY_IN_FLIGHT=1), the second concurrent heavy request was rejected immediately with a retryable 503 chat_admission_busy.

Those clients retry on a ~1s cadence; a burst of 2-3 heavy sub-requests burns the whole retry budget in seconds and the agent dies mid-task with no user-visible error — the UI just goes blank/waiting while later requests log 200.

Fix

Heavy chat requests now wait for a slot instead of failing instantly:

  • New ChatAdmissionController.acquireHeavyWithin(timeoutMs) — bounded FIFO wait, re-acquiring atomically on each release; resolves
    ull on deadline.
  • �dmitChatRequest (byte-based path) and �dmitChatStructure (structure-based path, now async) use it with a configurable budget.
  • New env OMNIROUTE_CHAT_ADMISSION_QUEUE_MS (default 5000);

Zartharas and others added 30 commits August 5, 2026 23:51
Validated in local merge-train T4 (HouMinXi+Zartharas+Andrian+artickc)
…9184)

Validated in local merge-train T4 (HouMinXi+Zartharas+Andrian+artickc)
Validated in local merge-train T4 (HouMinXi+Zartharas+Andrian+artickc)
Validated in local merge-train T4 (HouMinXi+Zartharas+Andrian+artickc)
Validated in local merge-train T4 (HouMinXi+Zartharas+Andrian+artickc)
Validated in local merge-train T4 (HouMinXi+Zartharas+Andrian+artickc)
Validated in local merge-train T4 (HouMinXi+Zartharas+Andrian+artickc)
Validated in local merge-train T4 (HouMinXi+Zartharas+Andrian+artickc)
…iegosouzapw#8870)

Validated in local merge-train T4 (HouMinXi+Zartharas+Andrian+artickc)
…egosouzapw#8877)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…iegosouzapw#9077)

Validated in local merge-train T5 (base49+contributors+pacocartones)
)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…dflare Workers AI (diegosouzapw#8717) (diegosouzapw#8808)

Validated in local merge-train T5 (base49+contributors+pacocartones)
Validated in local merge-train T5 (base49+contributors+pacocartones)
…zapw#8878)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…diegosouzapw#8958) (diegosouzapw#8961)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…holders (diegosouzapw#9001)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…trict BYOK providers (diegosouzapw#9005)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…souzapw#9020)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…apw#9065)

Validated in local merge-train T5 (base49+contributors+pacocartones)
… /login (diegosouzapw#9097)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…uzapw#9063)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…ouzapw#9073)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…iegosouzapw#9083)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…ents (diegosouzapw#9088)

Validated in local merge-train T5 (base49+contributors+pacocartones)
…ublish resolver (diegosouzapw#9553)

* fix(build): resolve npm-cli.js on POSIX layouts in the shim-free prepublish resolver

The diegosouzapw#8858 resolver only tried <dir(node)>/node_modules/npm/bin — the
Windows layout. On POSIX (GitHub hosted runners, nvm, system installs)
npm lives at <prefix>/lib/node_modules/npm while node is <prefix>/bin/
node, so resolveBundledNpmEntry returned null and npm run build:cli
died installing @omniroute/opencode-plugin deps on every fresh checkout
('npm-cli.js not found next to the running Node binary') — redding Fast
Production Build and dast-smoke for the whole PR queue.

Extract the resolver to scripts/build/resolveNpmEntry.ts with injectable
seams and try, in order: npm_execpath (exported by npm run itself), the
Windows beside-the-binary layout, the POSIX <prefix>/lib layout.

TDD: tests/unit/build/resolve-npm-entry.test.ts — the POSIX-layout and
npm_execpath cases plus a live regression guard fail against the old
single-candidate logic (2/5) and pass with the fix (5/5).

* docs(env): register the 7 env vars orphaned by the 08-05 merge batch

The Docs Gates env/docs contract went red on the release tip: diegosouzapw#9260
added OMNIROUTE_INTERNAL_SERVICE_TOKEN(_FILE) and diegosouzapw#9324 added
OPENROUTER_PROVIDER_STATS_ENABLED/_TTL_MS without .env.example entries,
and the diegosouzapw#9286 Redis sidecar vars (REDIS_BIND_HOST, REDIS_PORT,
OMNIROUTE_REDIS_BIND_HOST) never reached ENVIRONMENT.md. Inherited
base-red on every open PR. Defaults and descriptions taken from the
consuming source files.

---------

Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
…iegosouzapw#9554)

* fix(quality): reconcile inherited file-size drift on the release tip

13 files sit above their frozen LOC on the clean tip 8180b49
(measured by the gate itself). The PR-mode base-relative check (diegosouzapw#8522)
correctly lets innocent PRs pass, but per-PR rebaselines were lost
across successive conflict resolutions of this hot file during the
08-05/06 merge batch — so the absolute mode (nightly, local runs) is
permanently red and stops distinguishing real growth from inherited
drift.

Frozen values updated to the measured tip, each annotated with the
merged PR that grew the file (diegosouzapw#9024 diegosouzapw#9324 diegosouzapw#9329 diegosouzapw#9193 diegosouzapw#9332 diegosouzapw#9228
diegosouzapw#9236 diegosouzapw#9314 diegosouzapw#9260 diegosouzapw#8934 diegosouzapw#9196 diegosouzapw#9163); executors default.ts and kiro.ts
(above the 1000 cap with no frozen entry) join the frozen set.

* fix(quality): prune orphaned ESLint suppressions and clear the 5 unsuppressed errors

The 'No new ESLint warnings' job reds the whole queue with exit 2:
'There are suppressions left that do not occur anymore' — the 08-05
merge batch removed code whose violations were frozen in
eslint-suppressions.json, leaving orphaned entries (673->670 files,
4338->4333 violations after eslint --prune-suppressions).

The full-tree run also surfaced 5 real unsuppressed errors merged with
the batch, fixed here instead of suppressed (new violations must be
fixed, per policy): 4x no-explicit-any in
tests/unit/catalog-order-contract.test.ts ((conn as any).id -> typed
cast) and 1x react/no-unescaped-entities in the agent-bridge
SetupWizard (diegosouzapw#9095).

Also restores the _comment policy header the successive hot-file
conflict resolutions had dropped (TS7 debt freeze provenance + prune
policy).

* fix(quality): absorb the two file-size growths merged while this PR was in CI

The base kept moving during the reconcile cycle: diegosouzapw#9184 grew
src/sse/handlers/chat.ts 1857->1877 and diegosouzapw#9005 grew
open-sse/executors/default.ts 1027->1042. Re-measured on the merged
tree; gate back to 0 violations.

* fix(tests): move the orphaned RTL ratchet test to a collected path as node:test

diegosouzapw#8828 added tests/unit/scripts/check-rtl-ratchet.test.ts — a path no
runner collects (the node:test globs enumerate an explicit subdir list
without scripts/, and vitest.config.ts never included it), so the file
NEVER ran and the test-discovery orphan gate reds the queue. Moved to
tests/unit/ (collected by node:test) and converted from vitest
describe/it/expect to node:test+assert to match the runner and the
sibling check-*.test.ts files. 5/5 green under the real runner.

---------

Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
…e from (diegosouzapw#9559)

* fix(mcp): give the audit tests a loader seam createRequire cannot hide from

Since diegosouzapw#8959 the audit DB loads better-sqlite3 via createRequire() (so
Electron/global-install resolution works) — which vi.doMock cannot
intercept: it only patches Vitest's ESM module graph. The audit.test.ts
better-sqlite3 mock therefore never engaged; the tests opened a REAL
empty sqlite file in the temp DATA_DIR ('no such table: mcp_tool_audit'
on stderr) and every mock assertion counted zero calls. The 3 failures
are deterministic (reproduced 3/3 locally), redding Vitest (fast-path)
for the entire PR queue — long misdiagnosed as a flake (diegosouzapw#9095 merge
notes call it 'pre-existing audit.test.ts flake').

- Shutdown tests inject the mock through the audit connection cache
  (globalThis.__omnirouteMcpAuditDb) — the module's own seam.
- The node:sqlite fallback test drives __setBetterSqliteLoaderForTests,
  a test-only loader override; the production createRequire path is
  untouched (node:sqlite itself is import()'d, so its doMock still
  works).

3/3 red -> 3/3 green; full open-sse/mcp-server vitest suite 88/88.

* chore: align changelog slug with the PR number (9559)

---------

Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
…moke base-red) (diegosouzapw#9558)

* fix(build): exec native tool binaries directly in runBuildTool

diegosouzapw#8858 routed every resolved local bin through process.execPath to avoid
Windows .cmd shims — but esbuild >=0.25 ships bin/esbuild as the NATIVE
platform executable (ELF on Linux), so Node parsed machine code as JS and
build:cli died with 'SyntaxError: Invalid or unexpected token', turning
dast-smoke red for every PR.

runBuildTool now sniffs the entry's magic bytes (ELF / Mach-O / PE) and
execs native binaries directly; JS entries keep going through this Node
binary (the .cmd-shim avoidance diegosouzapw#8858 wanted).

Validation (RED->GREEN on this box):
- RED: node node_modules/esbuild/bin/esbuild --version -> SyntaxError (ELF)
- GREEN: the exact failing CI step reproduced via the new logic bundles
  open-sse/mcp-server/server.ts successfully (4.2MB output, 1.3s).

* fix(docs): add MDX frontmatter to the 20 remaining docs without it

Same failure class as AGENTROUTER_WAF (diegosouzapw#9503) and DOCKER_RELEASE_CHANNELS
(this run's dast-smoke red): any doc without frontmatter breaks the
fumadocs MDX loader during next build, killing build:cli/dast-smoke for
every PR. Swept ALL of docs/ (i18n mirrors excluded) in one pass so this
class cannot recur one file at a time.

* docs(env): document OMNIROUTE_INTERNAL_SERVICE_TOKEN(+_FILE), OPENROUTER_PROVIDER_STATS_* and embedded-Redis binding vars

Pre-existing env/docs contract drift from recently merged features made
check:env-doc-sync red for any docs-touching PR. Values and defaults read
from the defining modules (internalServiceAuth.ts, openrouterProviderStats.ts).

* fix(build): resolve bundled npm-cli.js in the standard Unix layout + safe npm fallback off-Windows

The opencode-plugin step hard-failed on GitHub runners because
resolveBundledNpmEntry only looked next to the node binary (Windows zip
layout); hostedtoolcache Node keeps npm at <prefix>/lib/node_modules/npm.
Added that candidate, and when neither exists on non-Windows the step now
falls back to plain 'npm' — the .cmd-shim hazard diegosouzapw#8858 avoids is
Windows-only.

* test(mutation): register xai-agent-tools-passthrough.test.ts in stryker tap.testFiles

The test landed on release/v3.8.50 covering
open-sse/handlers/chatCore/passthroughHelpers.ts without the stryker
registration, so Fast Quality Gates' drift detection reds any PR that
carries it. Mechanical registration so its mutant kills count.

---------

Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
…8339)

Validated in local merge-train T6 (ungrouped batch 1)
…quality-validation-benign-error

fix(combo): ignore benign empty error fields in streaming quality validation (502 false-positive on opencode tool calls)
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks for the PR. Please address the mandatory items (tests and/or merge blockers) in this branch, then rerun checks before /merge-prs.

2 similar comments
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks for the PR. Please address the mandatory items (tests and/or merge blockers) in this branch, then rerun checks before /merge-prs.

@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks for the PR. Please address the mandatory items (tests and/or merge blockers) in this branch, then rerun checks before /merge-prs.

diegosouzapw and others added 3 commits August 8, 2026 23:24
Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
…pr-9296

# Conflicts:
#	open-sse/config/imageRegistry.ts
#	open-sse/handlers/imageGeneration/providers/adobeFirefly.ts
#	open-sse/services/adobeFireflyClient.ts
#	tests/unit/adobe-firefly.test.ts
@diegosouzapw

Copy link
Copy Markdown
Owner

Obrigado pelo PR. Mantive a revisão de fix-in-place e não foi possível concluir o ajuste completo aqui:

  • Para os PRs em fork: não consigo aplicar push de correção diretamente na sua branch.
    Por favor, faça um rebase/sync com release/v3.8.50, resolva conflitos se houver, e rode os checks dessa branch.
    Se preferir, posso aplicar a correção na próxima rodada assim que você mandar o branch atualizado ou confirmar que o PR está limpo pra esse merge.

…odel-capabilities

fix(adobe-firefly): sync discovered models and capabilities
dionjoshualobo and others added 4 commits August 9, 2026 02:18
Agent clients (OpenCode, Claude Code, Cursor) fan out heavy sub-requests
that land on the admission gate together. With the single heavyweight
slot, concurrent heavy requests were rejected immediately with a
retryable 503; clients burn their retry budget in seconds and the agent
dies mid-task.

Heavy requests now wait up to OMNIROUTE_CHAT_ADMISSION_QUEUE_MS (default
5000ms) for a slot before the 503, served FIFO; 0 restores the legacy
immediate-reject behaviour. Applied to both the byte-based path
(admitChatRequest) and the structure-based path (admitChatStructure, now
async).
Keeps the env/docs contract (check-env-doc-sync, diegosouzapw#7793) in sync with the
new admission queue budget introduced in this PR.
@herjarsa
herjarsa force-pushed the fix/chat-admission-queue branch from c065c12 to 5441321 Compare August 9, 2026 09:20
@herjarsa

herjarsa commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto the current release/v3.8.50 tip (no conflicts) and pushed.

Fixed in this update:

  • Registered OMNIROUTE_CHAT_ADMISSION_QUEUE_MS in .env.example — resolves the three env/docs contract failures: check-env-doc-sync (unit 1/4), Docs Gates (fast-path), and issue #7793 (unit 4/4). Verified locally: scripts/check/check-env-doc-sync.mjs exit 0, tests/unit/chat-body-admission.test.ts 39/39, npm run typecheck:core clean, full npm run lint exit 0.

Remaining CI failures are pre-existing base drift, not introduced by this PR (none of the affected modules/files are touched by the diff):

Check Failure Evidence
Unit 1/4 cli-ipv4-first-dns-2699 — asserts command: 'node' but runner reports /opt/hostedtoolcache/node/24.18.0/x64/bin/node; perplexity-web 401 session cookie log lines at end of job
Unit 2/4 Responses->Chat output_item.done, direct translation calls have English messages, #6848 cleanupDomainCostHistory (x2), triage-bugs-2026-08-02 job summary
Fast Quality Gates dead-code 228 > baseline 227 (regressão); mutation-test-coverage drift: accountFallback.ts/auth.ts/api-key-policy modules missing from stryker.conf.json tap.testFiles — all pre-existing modules, none in this diff; pack-policy gate logs

These look like the base branch's own accumulated ratchet drift (the release branch is mid-merge per the other open PRs). Happy to address any of them here if you'd prefer they be fixed in this PR rather than the base.

The base's diegosouzapw#9697 (radar referral links) and the New-API aggregator work
added keys to en.json and 42 locales but left vi.json behind: 23 missing
radarPage/providers/miniPlayground keys plus 8 providers.* entries still
carrying __MISSING__ markers. That broke i18n-vi-completeness and the
radar page-tab/key-input tests for every PR against release/v3.8.50.

Adds the missing translations and replaces the __MISSING__ placeholders,
restoring exact en<->vi key parity (11391/11391).
@herjarsa

herjarsa commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Third push — fixes the remaining unit-test failures, all of which were Vietnamese-locale drift inherited from the base (not from the admission change):

  • #9697 (radar referral links) added radarPage.* keys to en.json + 42 locales but missed vi.json; the New-API aggregator work left 8 providers.* entries in vi.json with __MISSING__ markers.
  • Fixed src/i18n/messages/vi.json: 23 missing keys translated + 8 __MISSING__ placeholders replaced → exact en↔vi parity restored (11391/11391). This unblocks the three failing jobs (i18n-vi-completeness, radar-referrals-page-tab, radar-key-input).

Verified locally: all 18 i18n tests pass, prettier clean, ypecheck:core clean, full
pm run lint exit 0.

@herjarsa

Copy link
Copy Markdown
Contributor Author

Superseded by #NEW — cherry-pick of the 3 commits onto current main.

@herjarsa herjarsa closed this Aug 10, 2026
@herjarsa herjarsa reopened this Aug 10, 2026
@herjarsa
herjarsa changed the base branch from release/v3.8.50 to main August 10, 2026 14:01
@herjarsa

Copy link
Copy Markdown
Contributor Author

Re-opened with rebased commits

This PR was closed and re-opened with a fresh rebased branch \herjarsa:rebase/9816-chat-admission-queue.

The original 3 commits have been cherry-picked onto current \main:

  • \925caf7 fix(admission): queue heavyweight chat requests before 503 busy\
  • \5441321 docs(env): register OMNIROUTE_CHAT_ADMISSION_QUEUE_MS in .env.example\
  • \7826450 i18n(vi): complete Vietnamese locale key parity\

Resolutions applied (all took the PR version via --theirs):

  • \docs/guides/TROUBLESHOOTING.md\ — PR version (admission-queue troubleshooting entries)
  • \src/shared/middleware/chatBodyAdmission.ts\ — PR version (file was new on PR side; conflict came from unrelated edits to other shared middleware on \main)
  • \ ests/unit/chat-body-admission.test.ts\ — PR version
  • \src/i18n/messages/vi.json\ — PR version (Vietnamese locale parity)

Diff stats: 7 files changed, 1014 insertions(+), 78 deletions(-)

@herjarsa

Copy link
Copy Markdown
Contributor Author

Closing to re-open as a clean PR with the rebased commits. The original PR carried 1812 accumulated commits (141k additions) against an old main; the rebased version has only the 3 intended commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.