fix(server): bound inbound request concurrency

[amondnet]

Summary

axum::serve had no concurrency limit, so the number of in-flight inbound client requests was unbounded. Inbound auth is opt-in, so on an unauthenticated deployment this was reachable by anyone who could connect — and per-path memory bounds (the gzip semaphore from #256) can't be reasoned about without a ceiling on concurrent stream count.

Scope of the guarantee. This bounds in-flight request count, not bytes. Each admitted request may still buffer a body up to MAX_REQUEST_BODY_BYTES (64 MiB), and a permit is held for as long as the request is in flight — with no inbound request timeout, a client trickling a body holds its slot. So max_concurrent_requests makes the ceiling finite where it previously was not, but is not on its own a resident-memory bound. Surfaced by review; documented in the shared-gateway guide rather than overclaimed. Follow-ups below.

• Add [server] max_concurrent_requests (default 1024, 0 disables). At 0 no layer is installed at all, so the previous behavior is preserved exactly.
• Shed, never queue. An over-limit request gets an immediate 503 with Retry-After: 1, in the Anthropic error shape (overloaded_error) — or the OpenAI Responses envelope on the five inbound Codex-endpoint paths, per the AGENTS.md error-shape rule. Queueing was rejected deliberately: queued requests keep their buffered bodies resident, which defeats the memory bound that motivates the issue.
• The permit is held until the response body ends, not until the handler returns. PermitBody wraps the response body and delegates poll_frame / is_end_stream / size_hint, releasing the permit at end-of-stream or on drop (client disconnect). Releasing at the response head would leave concurrent SSE streams effectively unbounded, which is the exact quantity this issue is about. Delegating every frame — rather than into_data_stream() — keeps HTTP trailers intact.
• / and /health are merged outside the layer. Shedding liveness probes under load would make a load balancer evict an instance that is still serving work.
• The limit is fixed at boot (the semaphore is a router layer). A reload accepts a changed value but logs a warn! and requires a restart, matching how server.bind and [sentry] already behave.

Closes #260.

Milestone / spec

No milestone or frozen behavior spec applies — this is a focused cross-cutting server bound, not a new subsystem. docs/config-reload.md §4 gained the third restart-only field; docs/running.md documents the key and the liveness exemption.

Design decisions

Two knobs the issue raised were settled explicitly rather than left implicit:

Question	Choice	Why
Shed or queue?	Shed (503)	Queued requests hold buffered bodies resident; a queue without a depth cap just relocates the unbounded growth.
Permit scope on a stream?	Until body EOS/drop	The issue's rationale is concurrent-stream memory; releasing at the response head would not bound it.
Default value?	1024, 0 = unlimited	Pre-1.0 is the window to set a safer default. An off-by-default knob only protects operators who already know to set it, and the issue's concern is unauthenticated deployments. 1024 is far above any single-user deployment, so it is not a practical behavior change. Mirrors the existing sse_keepalive_seconds pattern (numeric, 0 disables) in the same struct.

HTTP/2 max_concurrent_streams is a separate knob and is not in this PR: axum::serve does not expose hyper's h2 builder, so it requires replacing the accept loop while preserving ConnectInfo propagation and shutdown semantics. Tracked as #268.

Checklist

• cargo build passes (cargo clippy --all-targets --all-features builds every target)
• cargo test --all-features --workspace passes: 1287 tests across 17 targets, 0 failed, 1 ignored
• cargo clippy --all-targets --all-features -- -D warnings clean
• cargo fmt --all --check clean
• Source files stay focused — src/concurrency.rs is 575 lines, but only 141 are production code; the other 434 are its test module, in line with the rest of the tree (config.rs is 5277)
• English only; matches surrounding style
• Frozen spec in docs/ updated if this change deviates from it
• User-facing docs updated — README.md, docs/running.md, docs/config-reload.md, both config examples, and the config reference in English + ko/ja/zh-cn (wiki/ untouched, it is generated)
• Any new GitHub Action is pinned to a full commit SHA (none added)

Notes for reviewers

The streaming permit is the part worth scrutinising. PermitBody::poll_frame releases the permit on Poll::Ready(None) or Poll::Ready(Some(Err(_))); Drop covers the disconnect path. The error arm was a real leak found late: a body terminating with an error held its permit until the wrapper happened to be dropped. Eight review passes read this function without catching it — only writing a test that observes the error path did. Both routes have their own test, and the EOS test is deliberately written to be load-bearing:

let (_parts, mut body) = first.into_parts();
while poll_fn(|cx| Pin::new(&mut body).poll_frame(cx)).await.is_some() {}

An earlier version used to_bytes(body, ..), which takes the body by value — so it released the permit through Drop and would still have passed with the EOS branch deleted. This was verified by mutation: with self.permit.take() removed, the to_bytes version passes and the version above fails 503 != 200.

Other things to look at closely:

• overloaded_error is Anthropic's type for 529, and this returns 503. Chosen because it is the most semantically accurate of the documented types and clients that special-case it back off correctly; api_error is the alternative if you would rather the type track the status convention.
• is_codex_path matches the five codex paths exactly, reading codex_endpoint::PATHS / codex_analytics::PATHS — the same constants build_router registers from, so a new Codex route cannot get the wrong error shape. These replaced a hand-maintained copy that the envelope test also iterated, which made that test tautological. When [server.codex_endpoint] is disabled those paths would otherwise 404, so an over-limit request to them returns an OpenAI-shaped 503 for a route that is not registered. Harmless, but intentional rather than overlooked.
• http-body was promoted to a direct dependency solely so PermitBody can name Frame/SizeHint. No pin-project is needed — axum::body::Body is Unpin, so Pin::new(&mut self.inner) suffices.
• The default 404 fallback is not behind the limiter. Because .merge(liveness_router) runs after .layer(...), the merged router's fallback ends up outside the gate — verified by probe: on a saturated router /routes returns 503 while /does-not-exist returns 404. An earlier revision of this description claimed the opposite; this is the corrected reading.

---

Summary by cubic

Bound inbound request concurrency process‑wide to cap in‑flight requests. Over‑cap requests return 503 with Retry‑After; streaming keeps a slot until the body ends; Codex routes keep the OpenAI error shape.

• New Features

  • New [server] max_concurrent_requests (default 1024; 0 disables). Fixed at startup; reload logs a warning and requires restart. / and /health bypass the gate.
  • Shed, never queue: immediate 503 with Retry-After: 1; Codex and analytics paths use the OpenAI error envelope.
  • Router and limiter read Codex path lists from shared PATHS constants (codex_endpoint, codex_analytics) so route registration and error shape stay in sync.
  • Permit held through response‑body completion (incl. SSE); released on end‑of‑stream, error, or drop. Also releases when the body reports end‑of‑stream even if no final poll occurs, but only after a ready frame; never on Poll::Pending. Trailers are preserved.
  • Validate the limit at load against tokio::sync::Semaphore::MAX_PERMITS; reject out‑of‑range values in Config::validate.
  • New metric shunt.requests_shed counts rejected requests; the rejection is logged at debug!.
  • Perf: remove a hot‑path refcount by moving the limiter Arc into try_acquire_owned instead of cloning per request.

• Dependencies

  • Added http-body to delegate frames while holding permits.
  • Added dev‑dependency http-body-util for test helpers.

^{Written for commit 6f4b954. Summary will update on new commits.}

Review follow-ups (commit 025c000)

A review pass over this diff surfaced findings that landed as a second commit:

• max_concurrent_requests was unvalidated. tokio::sync::Semaphore::new asserts above Semaphore::MAX_PERMITS, so an out-of-range value passed shunt check and then panicked at boot. Config::validate now rejects it. The bound is platform-dependent (usize::MAX >> 3 ≈ 5.4e8 on 32-bit), so the guard also stops the accepted range varying by build target. The test pins the constant against tokio's own MAX_PERMITS so a dependency bump cannot silently drift it.
• A saturated gateway was invisible. Shed requests never reach a handler, so they are absent from record_proxied_request and from request spans, and the rejection is logged only at debug! while the default filter is shunt=info. Added a shunt.requests_shed counter.
• Docs corrected and extended: the shared-gateway guide now documents the limit and its scope, reference/troubleshooting gained a 503 overloaded_error row, and docs/config-reload.md lost a stale "three fields" count that under-reported the actual restart-only set. All site changes are in en/ko/ja/zh-cn.

Known limitations, not addressed here

Confirmed during review, deliberately out of scope for this PR — each needs its own design rather than being bolted onto the admission gate:

1. Slow request bodies pin permits. proxy/failover.rs buffers the body before check_inbound_auth, so even with [server.auth] configured an unauthenticated client can hold permits by trickling bodies. There is no inbound request timeout, and a naive global one would break legitimate long SSE streams.
2. The byte budget scales with the cap. 1024 × 64 MiB is a large aggregate. A byte-weighted admission budget (or early Content-Length rejection) is the real fix; lowering the request count is the wrong lever.
3. Pre-admission slow-loris. axum::serve has no connection cap or header-read timeout, so connections can consume resources before ever reaching this middleware. Same class as the HTTP/2 knob in feat(server): expose HTTP/2 max_concurrent_streams #268.

[gemini-code-assist]

Code Review

This pull request introduces an inbound concurrency limiter to bound in-flight requests using a new max_concurrent_requests configuration setting, shedding excess requests with a 503 status while exempting liveness routes. The code review feedback recommends two performance and robustness improvements in src/concurrency.rs: avoiding an unnecessary Arc clone on the request hot path when acquiring semaphore permits, and ensuring the concurrency permit is released immediately if a streaming response encounters an error.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[codspeed-hq]

Merging this PR will not alter performance

✅ 72 untouched benchmarks

---

_{Comparing amondnet/fix-server-no-concurrency-limit-on-inbound-reque (6f4b954) with main (b84a3bd)}

[greptile-apps]

Greptile Summary

This PR adds boot-time inbound request concurrency admission control.

• Adds server.max_concurrent_requests, validation, restart-only reload warnings, and immediate overload shedding with a dedicated metric.
• Holds admission permits through response-body completion or drop while exempting liveness endpoints.
• Preserves protocol-specific error envelopes for Codex routes and documents the behavior across configuration examples and localized guides.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/concurrency.rs	Implements immediate admission shedding and retains permits through response-body termination, with focused lifecycle tests.
src/server.rs	Installs the boot-time gate around application routes while merging root and health liveness routes outside it.
src/config.rs	Adds the concurrency setting, its default, and validation against Tokio semaphore limits.
src/reload.rs	Detects changes to the boot-fixed limit and warns that a restart is required.
src/codex_endpoint.rs	Centralizes inbound Codex response paths for registration and overload-envelope classification.
src/codex_analytics.rs	Centralizes Codex analytics paths alongside the response endpoint path set.
src/metrics.rs	Adds a counter for requests shed by the concurrency gate.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Router
    participant Gate as Concurrency gate
    participant Handler
    participant Body as Response body

    Client->>Router: Request
    alt Root or health endpoint
        Router->>Handler: Bypass gate
        Handler-->>Client: Liveness response
    else Limited route with capacity
        Router->>Gate: Try to acquire permit
        Gate->>Handler: Admit request
        Handler-->>Body: Produce response
        Body-->>Client: Stream frames
        Body->>Gate: Release permit on EOS, error, or drop
    else Limited route is saturated
        Gate-->>Client: 503 + Retry-After: 1
    end

Loading

_{Reviews (4): Last reviewed commit: "fix(server): scope the end-of-stream rel..." | Re-trigger Greptile}

[amondnet]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces an inbound concurrency limiter (max_concurrent_requests, default 1024) to bound the number of concurrent in-flight requests and prevent memory exhaustion. Excess requests are shed immediately with a 503 Service Unavailable and a Retry-After: 1 header, while / and /health routes bypass this limit to remain available for liveness probes. The feedback suggests releasing the concurrency permit immediately when the last frame is polled or when is_end_stream() becomes true, rather than waiting for a subsequent poll or drop, ensuring resources are freed as early as possible.

[amondnet]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces an inbound concurrency limiter (max_concurrent_requests, defaulting to 1024) to bound in-flight requests and shed excess traffic immediately with a 503 Service Unavailable response and a Retry-After: 1 header. It adds the concurrency module, integrates it as a middleware layer in the router (exempting / and /health), introduces a shunt.requests_shed metric, and updates the documentation across multiple locales. Feedback on the implementation highlights a correctness issue in PermitBody::poll_frame where the concurrency permit could be prematurely released if is_end_stream() returns true while poll_frame yields Poll::Pending.

[amondnet]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces an inbound concurrency limiter to bound the number of in-flight requests (defaulting to 1024) and prevent memory exhaustion from buffered request bodies. It implements a ConcurrencyLimit middleware using a Tokio semaphore, wrapping response bodies in a custom PermitBody to hold permits until stream completion or client disconnection. Liveness probes (/ and /health) bypass this limit to ensure the gateway remains responsive under load. The PR also adds configuration options, extensive unit/integration tests, Prometheus/OpenTelemetry metrics (shunt.requests_shed), and comprehensive multi-language documentation. No review comments were provided, so there is no additional feedback to address.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud