Repository navigation
feat(server): bound HTTP request bodies and active responses - #2162
AlexStocks wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
hidb4ai
left a comment
There was a problem hiding this comment.
HTTP admission can release capacity before the outward response finishes; see the inline finding.
| configured_middleware.insert( | ||
| 0, | ||
| Middleware( | ||
| HttpResourceLimitsMiddleware, |
There was a problem hiding this comment.
[P2] Hold admission through the outer response send
create_app() subsequently prepends the attach_request_id and negotiate_dream_contract BaseHTTPMiddleware layers, so this limiter observes completion on their internal response channel before the response has finished sending. With max_concurrent_requests=1, blocking the external ASGI send on the /health/live response body still lets four subsequent requests return 200; they should return 503 while that response is blocked. Placing the same limiter outside those response-forwarding layers makes all four return 503. Slow clients can therefore retain unfinished responses beyond the configured cap. Keep admission held through the final outward response send, and add a blocked-send regression through create_server_app().
There was a problem hiding this comment.
Fixed in 26ea7ba. Admission now wraps the response-forwarding middleware and retains capacity while the outward ASGI send is blocked. A create_server_app() regression reproduces 200 before the fix, then verifies four 503 responses without body reads and recovery after the first send finishes. The existing cleanup test now waits for outward completion rather than its inner send. Current master was merged while retaining its mandatory stateless MCP authentication; related Linux admission/MCP/authentication suites passed 40 tests.
Merge current master, retain its mandatory stateless MCP authentication, and place HTTP admission outside response-forwarding middleware. A blocked external send now keeps the only slot reserved. Wait for actual outward completion in the existing cleanup regression instead of an inner send. Remove the superseded stateful opt-in and document the current transport. Constraint: Do not restore cross-request authentication context retention. Scope-risk: Limits remain process-local and long responses retain capacity. Tested: Blocked-send RED returned 200 instead of 503; all 16 admission tests passed after repair; Linux HTTP/MCP/authentication suites 40 passed; complete Linux Python 3.11 Ty; changed-file pre-commit.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Which issue or RFC does this PR close?
Refs #2084 (R7). Preserve the mandatory stateless MCP transport introduced by merged #2134.
Rationale for this change
Incoming bodies could consume unbounded resources before domain validation, and active external requests had no process-wide admission bound. The Server now rejects excess work before downstream parsing and reserves capacity through the actual outward response send.
What changes are included in this PR?
POWERCONTEXT_SERVER_HTTP_MAX_REQUEST_BODY_BYTES, default 16 MiB: reject oversized declared or actual streamed bodies with413/request_too_large, stopping at the first excess chunk.POWERCONTEXT_SERVER_HTTP_MAX_CONCURRENT_REQUESTS, default 64 per process: return503/server_overloadedandRetry-After: 1immediately without reading rejected bodies.Are there any user-facing changes?
Requests can receive 413 or 503 before reaching the route. Both limits must be positive. Lower body caps may reject otherwise valid domain input. The default admits the current 5 MiB Skill archive's 6,990,508-character base64 field plus bounded metadata.
Limits are per process. Sixty-four 16 MiB bodies already represent 1 GiB before parsing overhead; replay briefly copies each accepted body once. Long responses retain capacity until sent. Request-rate quotas, upload timeouts, connection limits and aggregate limits across processes remain deployment responsibilities.
MCP retains the existing stateless authentication behavior: no transport session ID or resumable cross-request SSE stream. Persisted Scope, Memory and Handoff data are unaffected.
How was this change tested?
create_server_app(): block the outward/health/livebody with concurrency one; before the fix subsequent requests return 200 instead of 503. After the fix all four are rejected without body reads, and capacity recovers after send completion. All 16 admission tests pass on Windows.Native Windows Ty has existing POSIX typing diagnostics; Linux typing is the reported full check. No real host UI or OceanBase multi-node acceptance is claimed.
AI usage statement
Codex assisted implementation, review and the reported validation.