Expire idle Streamable HTTP sessions by default and cap concurrent sessions - #3395
Expire idle Streamable HTTP sessions by default and cap concurrent sessions#3395maxisbey wants to merge 3 commits into
Conversation
📚 Documentation preview
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also examined two candidate issues and ruled them out: holding _session_creation_lock while serving the entire opening request (including reading the POST body) serializes concurrent session opens, but that scope is pre-existing behavior, not widened by this PR; and the new docs/migration.md section is a deliberate record of the changed idle-timeout/max-sessions defaults rather than a stray addition.
Extended reasoning...
The inline findings cover the cancellation-window leak around task_group.start(run_server) (src/mcp/server/streamable_http_manager.py:354), the idle-expiry race in src/mcp/server/streamable_http.py:490, and the stateless-mode missing try/finally around terminate(). Separately, I checked whether serving the opening request inside _session_creation_lock (the _send_and_report_status call at src/mcp/server/streamable_http_manager.py:362, which awaits the request body) was a regression — the old code already called handle_request inside the same lock, so this PR does not widen that critical section, and the docs/migration.md addition documents this PR's own default changes, which is the kind of entry a behavior-changing PR legitimately adds. Neither warranted a posted finding; recording them here so a human reviewer knows they were looked at.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
src/mcp/server/streamable_http_manager.py— pre-existing:_handle_stateless_requeststill runshandle_requestthenterminate()with no try/finally, so when the ASGI task is cancelled (client disconnects mid-request — the exact trigger the PR handles for stateful opens)terminate()is skipped andrun_stateless_serverblocks forever inserve_connectionon a read stream onlyterminate()closes, leaking one task+transport per aborted request in the manager's task group until shutdown, exactly as the base does. The PR added this hardening only to the stateful path (try/finally + shieldedterminate(), lines 360-370). Mirror it here: wrap lines 256-259 in try/finally and callterminate()underanyio.CancelScope(shield=True).Extended reasoning...
Path: stateless mode,
_handle_stateless_request(src/mcp/server/streamable_http_manager.py:253-259). It startsrun_stateless_serverin the manager's long-livedself._task_group; that task sits inserve_connectionreading the transport's read stream, which is closed only byhttp_transport.terminate()(streamable_http.py:839-865) —connect()'s own finally can't help because it runs only afterserve_connectionreturns (circular). Trigger: the client disconnects while its POST is being served and the ASGI server cancels the request task (the PR's own testtest_opening_request_that_is_cancelled_leaves_no_sessionmodels exactly this for the stateful path).await http_transport.handle_request(scope, receive, send)at line 256 raises CancelledError, so line 259'sterminate()never runs; the read stream stays open andrun_stateless_serverblocks forever. No safeguard applies: the new idle timeout is not wired in stateless mode (transport is built withoutidle_timeoutat lines 211-216, soidle_scopeis None), andmax_sessionsonly gates the stateful branch — so leakVerification: pre-existing — The mechanism is real, and the diff leaves the stateless path with exactly the gap it just closed on the stateful path. Code path (src/mcp/server/streamable_http_manager.py:253-259, byte-identical to base commit 6705402 —
git diffhas no hunk in_handle_stateless_request): ```python await self._task_group.start(run_stateless_server) # Handle the HTTP request and return the respo
| self._requests_in_flight += 1 | ||
| self.idle_scope.deadline = math.inf | ||
| try: | ||
| await self._handle_request(scope, receive, send) | ||
| finally: | ||
| self._requests_in_flight -= 1 | ||
| if not self._requests_in_flight: | ||
| self.idle_scope.deadline = anyio.current_time() + self._idle_timeout |
There was a problem hiding this comment.
🟡 Race at idle expiry: once the idle deadline fires, idle_scope.cancel_called is set and setting deadline = math.inf here is a no-op (anyio's setter skips cancelled scopes), yet the transport still accepts the request — _terminated only becomes True after run_server unwinds through serve_connection's connection-cleanup await and calls terminate(). A request landing in that window is dispatched into a session with no reader: writer.send() blocks until terminate() closes the stream, so the client gets a 500 (JSON mode) or a 200 SSE stream that ends with no response (SSE mode) instead of the clean expired-session 404 — a path that now exists by default since session_idle_timeout defaults to 1800 (base default was None; the opt-in base had the same race). Fix: have…
Extended reasoning...
Path: session idle deadline reached with no request in flight. anyio timer fires -> idle_scope.cancel_called = True, cancellation delivered to the serve_loop task. Unwinding is not atomic: serve_loop -> serve_connection's finally: await aclose_shielded(connection) (runs per-connection exit-stack user cleanup, an await) -> back in run_server, with idle_scope: exits catching the cancel -> only THEN does run_server's finally run _forget_session(new_session_id) and await http_transport.terminate() (streamable_http_manager.py:343-349). Throughout that window the manager's existing-session branch (streamable_http_manager.py:270-282) still finds the transport in _server_instances and calls transport.handle_request. handle_request (streamable_http.py:483-490) increments _requests_in_flight and sets self.idle_scope.deadline = math.inf, but anyio's deadline setter does nothing once cancel_called is set, so this cannot revive the session; _handle_request then sees self._terminated still False (line 502) and dispatches. For a POST, writer.send(session_message) (line 620/6
Verification: nit — The race is real: after the idle timer fires, idle_scope.cancel_called is True but the session is still in _server_instances (removed only at streamable_http_manager.py:347 in run_server's finally) and _terminated is still False (set at line 349 via terminate()). Unwinding contains await checkpoints (runner.py:467 finally: await aclose_shielded(connection)), so an HTTP request task c
There was a problem hiding this comment.
i didn't read what claude said, but this does look like a race to me
| # Assert task group is not None for type checking | ||
| assert self._task_group is not None | ||
| # Start the server task | ||
| await self._task_group.start(run_server) |
There was a problem hiding this comment.
🟣 If await self._task_group.start(run_server) raises — the opening request's task is cancelled at that checkpoint, or connect() fails before task_status.started() — the session registered at lines 309-311 is never removed: the established try/finally that discards provisional sessions only begins at line 360, and run_server's own finally (347) is skipped because it never reached its try. The ghost entry then permanently occupies a max_sessions slot and answers 500s (its idle scope is never entered, so the reaper can never fire), so repeated occurrences walk the manager toward 503 for every new client — the exact leak the PR's not-established cleanup was added to prevent. Fix: cover registration through start() with the same _forget_session + shielded terminate() cleanup.
Extended reasoning...
Path: _handle_stateful_request new-session branch registers the transport in _server_instances/_session_owners (309-311) before await self._task_group.start(run_server) (354). anyio's TaskGroup.start is a checkpoint: if the calling (ASGI/host) task is cancelled while awaiting it, anyio cancels the not-yet-started run_server (cancellation lands inside connect() before task_status.started(), so run_server's try/finally at 319-349 — which calls _forget_session — is never entered) and re-raises CancelledError in the caller before the try at line 361, so the manager-side cleanup (365-370) never runs either. The same holds if connect() raises before started(): start() re-raises the exception at 354. Result: _server_instances[new_session_id] holds a transport whose streams were closed by connect()'s teardown and whose idle_scope was never entered (a CancelScope's deadline only arms on __enter__), so the default 30-min reaper cannot fire and _forget_session is never called — the entry lives until manager shutdown, each occurrence consuming one of the `max_se
Verification: pre-existing — The gap is mechanically real: src/mcp/server/streamable_http_manager.py:309-311 registers the session (self._server_instances[http_transport.mcp_session_id] = http_transport) before await self._task_group.start(run_server) at line 354. If the host ASGI task is cancelled at that checkpoint, anyio cancels the not-yet-started child; run_server calls task_status.started() at l
The session manager kept a session's registry entry after the client
ended it with DELETE (the per-session task's cleanup skipped terminated
transports), and a request without a session ID that was refused
(anything but a valid initialize: wrong Accept, malformed JSON, a
non-initialize message, GET/DELETE) still left a registered transport
with a running server task behind it.
Now the manager drops the entry as soon as the transport is terminated,
the per-session task forgets the session and terminates its transport
however the loop ended, and a provisional session whose opening request
was answered with an error is discarded before the request returns. A
follow-up request on a deleted session is answered by the manager
("Session not found", 404) rather than by the dead transport.
`session_idle_timeout` was opt-in (default None), so at stock settings a stateful session that its client never deleted stayed registered, with its server task and streams, until the process exited. The docstring already recommended 1800 seconds; make that the default (DEFAULT_SESSION_IDLE_TIMEOUT) so sessions nobody is using are reclaimed after 30 minutes. `None` keeps the previous behaviour. "Idle" is now measured from the moment the session's last in-flight request completes rather than from the arrival of the last request: the transport takes an `idle_timeout` and owns the countdown, holding it while any request (an open GET stream included) is being served and restarting it when the last one finishes. A connected client, or a call that runs longer than the timeout, therefore never loses its session; a client that goes quiet with no stream open gets 404 on its next request and initializes again, as the spec describes. The timeout is simply unused in stateless mode, which keeps no sessions, so constructing a stateless manager with a timeout no longer raises.
c60175b to
30c3971
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/mcp/server/streamable_http.py">
<violation number="1" location="src/mcp/server/streamable_http.py:206">
P2: When `idle_timeout` is `NaN`, the constructor accepts it and the idle scope receives a `NaN` deadline, so session expiration becomes undefined instead of failing fast. Reject non-finite timeout values alongside non-positive values.</violation>
<violation number="2" location="src/mcp/server/streamable_http.py:484">
P2: There is a race window between the idle deadline firing (idle_scope.cancel_called becomes True) and run_server's finally actually calling terminate(). A request that arrives in that window increments _requests_in_flight and sets `self.idle_scope.deadline = math.inf`, but anyio's deadline setter is a no-op once cancel_called is set, so the session cannot be revived. Since `_terminated` is still False, the request is dispatched into a session whose loop is already unwinding, and `writer.send()` blocks until terminate() eventually runs — producing a 500 (JSON mode) or a silently-ended SSE stream instead of the intended clean 404 for an expired session. Guard handle_request against `idle_scope.cancel_called` (or check `_terminated`/loop status) before dispatching, or have this in-flight bump made atomic with the cancellation check.</violation>
</file>
<file name="tests/server/test_streamable_http_manager.py">
<violation number="1" location="tests/server/test_streamable_http_manager.py:638">
P3: These refusal/cancel/failure tests verify only that the session registry empties (`_server_instances == {}`, `_session_owners == {}`), but the PR's claim is that no transport is left behind either. The manager registers the provisional transport and starts its `run_server` task before serving the request, then forgets+terminates it in a finally; if `terminate()` were dropped, the background task would keep running while the dicts are already empty, and all three tests would still pass. Capture the created transport(s) (as `test_stateless_requests_memory_cleanup` does) and assert `transport.is_terminated` after the refused/cancelled/failed open.</violation>
</file>
<file name="src/mcp/server/streamable_http_manager.py">
<violation number="1" location="src/mcp/server/streamable_http_manager.py:349">
P2: During manager shutdown, this `terminate()` await runs inside the cancelled task-group scope and can be interrupted before closing the transport streams. Shield the transport cleanup so session shutdown actually completes.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| """ | ||
| if mcp_session_id is not None and not SESSION_ID_PATTERN.fullmatch(mcp_session_id): | ||
| raise ValueError("Session ID must only contain visible ASCII characters (0x21-0x7E)") | ||
| if idle_timeout is not None and idle_timeout <= 0: |
There was a problem hiding this comment.
P2: When idle_timeout is NaN, the constructor accepts it and the idle scope receives a NaN deadline, so session expiration becomes undefined instead of failing fast. Reject non-finite timeout values alongside non-positive values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/streamable_http.py, line 206:
<comment>When `idle_timeout` is `NaN`, the constructor accepts it and the idle scope receives a `NaN` deadline, so session expiration becomes undefined instead of failing fast. Reject non-finite timeout values alongside non-positive values.</comment>
<file context>
@@ -187,12 +189,22 @@ def __init__(
"""
if mcp_session_id is not None and not SESSION_ID_PATTERN.fullmatch(mcp_session_id):
raise ValueError("Session ID must only contain visible ASCII characters (0x21-0x7E)")
+ if idle_timeout is not None and idle_timeout <= 0:
+ raise ValueError("idle_timeout must be a positive number of seconds")
</file context>
| if idle_timeout is not None and idle_timeout <= 0: | |
| if idle_timeout is not None and (not math.isfinite(idle_timeout) or idle_timeout <= 0): |
| # the idle countdown is suspended while any is being served and | ||
| # restarts when the last one completes. | ||
| self._requests_in_flight += 1 | ||
| self.idle_scope.deadline = math.inf |
There was a problem hiding this comment.
P2: There is a race window between the idle deadline firing (idle_scope.cancel_called becomes True) and run_server's finally actually calling terminate(). A request that arrives in that window increments _requests_in_flight and sets self.idle_scope.deadline = math.inf, but anyio's deadline setter is a no-op once cancel_called is set, so the session cannot be revived. Since _terminated is still False, the request is dispatched into a session whose loop is already unwinding, and writer.send() blocks until terminate() eventually runs — producing a 500 (JSON mode) or a silently-ended SSE stream instead of the intended clean 404 for an expired session. Guard handle_request against idle_scope.cancel_called (or check _terminated/loop status) before dispatching, or have this in-flight bump made atomic with the cancellation check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/streamable_http.py, line 484:
<comment>There is a race window between the idle deadline firing (idle_scope.cancel_called becomes True) and run_server's finally actually calling terminate(). A request that arrives in that window increments _requests_in_flight and sets `self.idle_scope.deadline = math.inf`, but anyio's deadline setter is a no-op once cancel_called is set, so the session cannot be revived. Since `_terminated` is still False, the request is dispatched into a session whose loop is already unwinding, and `writer.send()` blocks until terminate() eventually runs — producing a 500 (JSON mode) or a silently-ended SSE stream instead of the intended clean 404 for an expired session. Guard handle_request against `idle_scope.cancel_called` (or check `_terminated`/loop status) before dispatching, or have this in-flight bump made atomic with the cancellation check.</comment>
<file context>
@@ -458,6 +473,23 @@ async def _clean_up_memory_streams(self, request_id: RequestId) -> None:
+ # the idle countdown is suspended while any is being served and
+ # restarts when the last one completes.
+ self._requests_in_flight += 1
+ self.idle_scope.deadline = math.inf
+ try:
+ await self._handle_request(scope, receive, send)
</file context>
| async with manager.run(): | ||
| response_start, _ = await _call(manager, _request_scope(), _INITIALIZE_BODY) | ||
| assert response_start["status"] == 200 | ||
| assert manager._server_instances == {} |
There was a problem hiding this comment.
P3: These refusal/cancel/failure tests verify only that the session registry empties (_server_instances == {}, _session_owners == {}), but the PR's claim is that no transport is left behind either. The manager registers the provisional transport and starts its run_server task before serving the request, then forgets+terminates it in a finally; if terminate() were dropped, the background task would keep running while the dicts are already empty, and all three tests would still pass. Capture the created transport(s) (as test_stateless_requests_memory_cleanup does) and assert transport.is_terminated after the refused/cancelled/failed open.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/server/test_streamable_http_manager.py, line 638:
<comment>These refusal/cancel/failure tests verify only that the session registry empties (`_server_instances == {}`, `_session_owners == {}`), but the PR's claim is that no transport is left behind either. The manager registers the provisional transport and starts its `run_server` task before serving the request, then forgets+terminates it in a finally; if `terminate()` were dropped, the background task would keep running while the dicts are already empty, and all three tests would still pass. Capture the created transport(s) (as `test_stateless_requests_memory_cleanup` does) and assert `transport.is_terminated` after the refused/cancelled/failed open.</comment>
<file context>
@@ -503,9 +628,150 @@ def test_session_idle_timeout_rejects_non_positive():
+ async with manager.run():
+ response_start, _ = await _call(manager, _request_scope(), _INITIALIZE_BODY)
+ assert response_start["status"] == 200
+ assert manager._server_instances == {}
+
+
</file context>
… on the server factories Add `max_sessions` (DEFAULT_MAX_SESSIONS = 10_000, `None` for no limit) to StreamableHTTPSessionManager: while that many stateful sessions are open, a request that would open another is answered 503 with a JSON-RPC error body and nothing is allocated; existing sessions are untouched and room frees up as they end or expire. This matches the Ruby SDK's defaults (the C# SDK uses the same 10 000 figure). `session_idle_timeout` and `max_sessions` are accepted by `Server.streamable_http_app()`, `MCPServer.streamable_http_app()`, `run_streamable_http_async()` and `run(transport="streamable-http")`, the same way `max_request_body_size` is, so applications can tune or disable them without reaching into `session_manager` after the fact. Docs: run/index.md options list, run/legacy-clients.md session cost, troubleshooting.md.
30c3971 to
ae2daca
Compare
There was a problem hiding this comment.
Beyond the inline comment, I also looked at whether holding _session_creation_lock across the opening request's entire handle_request (src/mcp/server/streamable_http_manager.py:292-370) lets one slow initialize stall all new-session creation, now including the 503 capacity check — the lock already spanned handle_request before this change, so it is pre-existing behavior rather than something this PR introduces.
Extended reasoning...
This run confirmed one inline finding (the 503 "Too many open sessions" response reusing _error_response's hardcoded INVALID_REQUEST JSON-RPC code) and separately investigated whether the session-creation lock being held across the whole opening request — including the awaited handle_request at src/mcp/server/streamable_http_manager.py:362 — is a serialization bottleneck introduced by this PR. Comparing against the base commit shows the pre-PR code held the same lock across the opening request's handle_request as well, so the behavior predates this change and was ruled out as a finding rather than silently dropped. Earlier review threads from prior runs (idle-expiry race, registration-before-start() cleanup, migration.md scope) remain visible on the PR; the migration.md edits are gone from the current revision, and the remaining threads have author replies whose substance should be judged in those threads rather than restated here.
| async with self._session_creation_lock: | ||
| if self.max_sessions is not None and len(self._server_instances) >= self.max_sessions: | ||
| logger.warning("Refusing to open a new session: %d sessions are already open", self.max_sessions) | ||
| await _error_response("Too many open sessions", 503)(scope, receive, send) |
There was a problem hiding this comment.
🟡 nit: The new 503 "Too many open sessions" response reuses _error_response, which hardcodes JSON-RPC code INVALID_REQUEST (-32600) — a client-error code meaning the request object is malformed — for what is a transient server-capacity condition. The SDK client (client/streamable_http.py:348-357) forwards that body verbatim, so callers get MCPError code -32600, the same code as "Session not found"/"Session terminated", and reconnect-on-invalid-request logic (as docs/troubleshooting.md teaches) will hot-loop re-initializing against a full server instead of backing off. Give _error_response a code parameter and use a server-error code (e.g. INTERNAL_ERROR or a -32000-range code) for the 503, ideally with a Retry-After header.
Extended reasoning...
Path: manager at capacity -> src/mcp/server/streamable_http_manager.py:293-296 answers the opening request with _error_response("Too many open sessions", 503); _error_response (lines 384-389) builds JSONRPCError(... error=ErrorData(code=INVALID_REQUEST, ...)) unconditionally — INVALID_REQUEST (-32600) per JSON-RPC 2.0 means "the JSON sent is not a valid Request object", but the refused initialize is perfectly valid. Consequence: the SDK client's POST handler (src/mcp/client/streamable_http.py:342-357) sees status >= 400 with an application/json JSON-RPC error body and delivers that error to the caller as-is, so a client gets MCPError with code -32600 and message "Too many open sessions". By code alone this is indistinguishable from the 404 "Session not found" (-32600) that the same PR documents as "reconnect and initialize again" (docs/troubleshooting.md), so any client that branches on the code (rather than string-matching the message) treats capacity exhaustion as a dead session and immediately re-initializes, which at capacity yields another 503/-32600 — a retry lo
Verification: nit. The candidate's mechanics are accurate. The new capacity path at /home/claude/python-sdk/src/mcp/server/streamable_http_manager.py:295 (await _error_response("Too many open sessions", 503)(scope, receive, send)) goes through the shared helper at lines 384-386, which unconditionally builds JSONRPCError(jsonrpc="2.0", id=None, error=ErrorData(code=INVALID_REQUEST, message=message)). So
1 participant
Fixes #2455, fixes #3228, fixes #3300
Stateful Streamable HTTP sessions now have a lifecycle the server owns: a session is forgotten as soon as it ends, sessions with nothing in flight expire after
session_idle_timeout(default 30 minutes), and a manager holds at mostmax_sessions(default 10 000) at a time. Both settings are accepted bystreamable_http_app(),run_streamable_http_async()andrun(transport="streamable-http"), next tomax_request_body_size.Motivation and Context
session_idle_timeouthas existed onStreamableHTTPSessionManagersince 1.27 but defaulted toNoneand wasn't reachable fromMCPServerorServer.streamable_http_app()(#2455), so at stock settings a session whose client never sentDELETEstayed registered, with its server task and streams, until the process exited. The docstring already recommended 1800 seconds; this makes it the default, which is also what the Ruby SDK ships (the C# SDK measures idleness the same in-flight-aware way, with a longer window and a 10 000-session limit).Three related bookkeeping fixes ride along because the default only makes sense with them:
DELETEnow removes the session's entry immediately (the per-session cleanup used to skip terminated transports, Cleanly terminated streamable-HTTP sessions are never deregistered: the DELETE path skips its own cleanup #3300), and the per-session task forgets and terminates the session however its loop ended.initialize: wrongAccept, malformed JSON, a non-initialize message, GET/DELETE) no longer leaves a registered session and task behind (Rejected streamable-HTTP requests leave live sessions behind: the session is registered before the request is validated #3228).max_sessionslimits how many stateful sessions one manager holds at a time: while that many are open, a request that would open another gets503with a JSON-RPC error body; existing sessions are unaffected and room frees up as they end or expire. Nothing is evicted to make room.How Has This Been Tested?
New tests in
tests/server/test_streamable_http_manager.py,tests/shared/test_streamable_http.py,tests/server/test_streamable_http_router.pyandtests/docs_src/: a deleted session is forgotten and its ID answers 404 "Session not found"; each refused opening-request shape leaves no session, and neither does an opening request whose handler raises or is cancelled; the defaults; atools/callparked past the timeout, an open GET stream, and a request completing under an open GET stream all keep the session, after which it expires and answers 404; sessionmax_sessions + 1gets 503 and a slot frees as soon as a session is deleted; non-positive values are rejected; the factories forward both settings; the transport can be constructed outside an event loop and creates its idle scope onconnect(). The interaction test forDELETEasserts the new contract. Full suite (100 % coverage, strict-no-cover), pyright and ruff pass locally; also exercised end to end againststreamable_http_app()under uvicorn with the SDK client and raw HTTP.Breaking Changes
No signature loses anything and every new parameter has a default (keyword-only on the factories), but two defaults are now active where previously there was no limit:
session_idle_timeoutdefaults to1800instead ofNone. A stateful session with no request in flight for 30 minutes (no open GET stream, no running call) is terminated; the client's next request gets404and it has to initialize again, as the spec describes. Clients that keep the GET stream open (the SDK clients do) or have a call running are unaffected. Passsession_idle_timeout=Nonefor the previous behaviour.max_sessionsdefaults to10_000. Deployments that expect more than 10 000 concurrent stateful sessions in one process should raise it or passNone.Smaller observable differences: constructing a stateless manager with a timeout no longer raises (the value is unused there); a request on a deleted session is answered by the manager (
404, "Session not found") rather than by the terminated transport;StreamableHTTPServerTransportaccepts an optionalidle_timeoutand createsidle_scopeitself when it is set.Types of changes
Checklist
help wanted, or I'm a maintainer)Additional context
docs/run/index.md,docs/run/legacy-clients.mdand theSession not foundentry indocs/troubleshooting.mddescribe the defaults and how to turn them off. Stateless mode and the 2026-07-28 request path keep no sessions and are unaffected. Thanks to @shaun0927 (#2457) and @sainikhiljuluri (#3229) for the earlier PRs in this area, which this supersedes.AI Disclaimer