Feature/native mcp auth0 delegation#1
Open
distributedstatemachine wants to merge 4 commits intomainfrom
Open
Conversation
Re-add support for gateway.controlUi.allowedOrigins config to control the frame-ancestors CSP directive. When allowedOrigins is set, the comprehensive CSP uses those origins instead of 'none', and X-Frame-Options DENY is omitted (CSP takes precedence in modern browsers). This allows embedding the Control UI in iframes from trusted origins (e.g. the Basilica dashboard). Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Keep local git hook verification working on macOS by replacing Bash 4-specific mapfile usage with Bash 3-compatible loops. Made-with: Cursor
Pass signed-in Auth0 JWTs through deployed control UI gateway sessions so remote MCP tools can execute per-user, and prefer explicit deployment gateway tokens over stale cached device tokens during local/manual connections. Made-with: Cursor
WalkthroughAdds remote MCP tool support and delegated-JWT flow: Auth0 integration in the UI, an in-memory MCP auth store, WebSocket binding/cleanup of delegated JWTs, CSP/frame-ancestor configurability, control-ui bootstrap auth0 fields, and pre-commit hook refactor plus dependency additions. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser/UI
participant Auth0 as Auth0 Service
participant Gateway as Gateway Server
participant MCP as Remote MCP Service
rect rgba(100, 150, 200, 0.5)
Note over Browser,Auth0: Auth0 Bootstrap & Delegation Flow
Browser->>Browser: Load control-ui bootstrap config
Browser->>Browser: Initialize Auth0 client with domain/clientId
Browser->>Auth0: getToken(audience)
Auth0-->>Browser: delegatedJwt
Browser->>Gateway: Connect WebSocket with delegatedJwt in auth
end
rect rgba(150, 100, 200, 0.5)
Note over Gateway,MCP: JWT Delegation & MCP Tool Execution
Gateway->>Gateway: Store delegatedJwt for connection
Gateway->>Gateway: Bind connection to session
Browser->>Gateway: chat.send request
Gateway->>Gateway: Retrieve delegatedJwt for session
Gateway->>MCP: Create client with delegated JWT
MCP->>MCP: Authenticate with JWT
Gateway->>MCP: callTool(toolName, params)
MCP-->>Gateway: tool result
Gateway-->>Browser: chat response with tool output
end
sequenceDiagram
participant Agent as Agent/Tools
participant Cache as MCP State Cache
participant HTTP as HTTP Transport
participant MCP as MCP Service
participant JWT as JWT Store
rect rgba(200, 150, 100, 0.5)
Note over Agent,JWT: Remote MCP Tool Discovery & Execution
Agent->>Cache: createRemoteMcpTools(agentSessionKey)
Cache->>Cache: Load or resolve MCP manifest
Cache->>HTTP: Create HTTP client with JWT auth
HTTP->>JWT: getDelegatedJwtForSession(agentSessionKey)
JWT-->>HTTP: jwt token
Cache->>MCP: Connect and enable plugins
MCP-->>Cache: Connected + tools from manifest
Cache-->>Agent: Array of AnyAgentTool instances
Agent->>Agent: User selects remote tool
Agent->>HTTP: Execute selected tool with params
HTTP->>MCP: callTool with auth header
alt Recoverable Error (Unauthorized/Closed)
MCP-->>HTTP: Error
HTTP->>HTTP: Retry with fresh client
HTTP->>MCP: callTool retry
end
MCP-->>HTTP: tool result
HTTP-->>Agent: jsonResult
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/src/ui/gateway.ts (1)
129-170:⚠️ Potential issue | 🟠 MajorHandle delegated JWT provider failures before building the connect frame.
A rejected
delegatedJwtProvider()escapessendConnect()before any request is sent, and this call site is fire-and-forget. The result is an unhandled rejection and a socket that just waits for handshake timeout instead of either connecting without delegation or failing fast.🔧 Suggested fix
- const delegatedJwt = await this.opts.delegatedJwtProvider?.(); + let delegatedJwt: string | undefined; + try { + delegatedJwt = await this.opts.delegatedJwtProvider?.(); + } catch (err) { + console.warn("[gateway] delegated JWT unavailable:", err); + delegatedJwt = undefined; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/ui/gateway.ts` around lines 129 - 170, The delegatedJwtProvider() call inside sendConnect is unprotected and can reject, causing an unhandled rejection and aborting the connect logic; wrap the await this.opts.delegatedJwtProvider?.() call in a try/catch inside sendConnect (identify the delegatedJwt variable and sendConnect function) and on error set delegatedJwt = undefined (or null), optionally log the failure via the gateway logger, and continue building the auth frame so the socket either connects without delegation or fails fast rather than leaving an unhandled rejection.ui/src/ui/controllers/control-ui-bootstrap.ts (1)
29-49:⚠️ Potential issue | 🟠 MajorClear stale Auth0 bootstrap state on failed reloads.
setAuth0BootstrapConfig()only runs on the success path, so a later 404/network failure leaves the previous gateway's Auth0 config cached in memory. After switching gateways or base paths, the UI can keep requesting delegated JWTs for the wrong tenant until the page is fully reloaded.🔧 Suggested fix
export async function loadControlUiBootstrapConfig(state: ControlUiBootstrapState) { if (typeof window === "undefined") { return; } if (typeof fetch !== "function") { return; } + + setAuth0BootstrapConfig(undefined); const basePath = normalizeBasePath(state.basePath ?? ""); const url = basePath ? `${basePath}${CONTROL_UI_BOOTSTRAP_CONFIG_PATH}` : CONTROL_UI_BOOTSTRAP_CONFIG_PATH; @@ if (!res.ok) { return; } @@ state.assistantAvatar = normalized.avatar; state.assistantAgentId = normalized.agentId ?? null; } catch { + setAuth0BootstrapConfig(undefined); // Ignore bootstrap failures; UI will update identity after connecting. } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/ui/controllers/control-ui-bootstrap.ts` around lines 29 - 49, The bootstrap fetch path currently only calls setAuth0BootstrapConfig(parsed.auth0) on success, leaving stale Auth0 config when the request fails; update the error paths (both the !res.ok early return and the catch block in the fetch/await block inside control-ui-bootstrap.ts) to clear the cached bootstrap state by calling setAuth0BootstrapConfig(null) (or appropriate empty value) and also reset state.assistantName, state.assistantAvatar and state.assistantAgentId to their default/empty values so the UI won’t keep using the previous tenant’s Auth0 config after a failed reload.
🧹 Nitpick comments (3)
src/agents/tools/remote-mcp.ts (2)
121-134: Consider handling plugin enablement failures gracefully.If
enableConfiguredPluginsfails on one plugin, it will throw and prevent subsequent plugins from being enabled and the client connection from completing. Consider whether failed plugin enablement should be logged and skipped rather than failing the entire connection.♻️ Proposed fix to handle plugin errors gracefully
async function enableConfiguredPlugins(client: Client, pluginIds: string[]): Promise<void> { for (const pluginId of pluginIds) { const plugin = MCP_PLUGIN_OPTIONS[pluginId as keyof typeof MCP_PLUGIN_OPTIONS]; if (!plugin) { continue; } - await client.callTool({ - name: "bittensor_enable_plugin", - arguments: { - plugin_name: plugin.pluginName, - }, - }); + try { + await client.callTool({ + name: "bittensor_enable_plugin", + arguments: { + plugin_name: plugin.pluginName, + }, + }); + } catch { + // Plugin enablement failure is non-fatal; continue with remaining plugins. + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/tools/remote-mcp.ts` around lines 121 - 134, The loop in enableConfiguredPlugins currently aborts on the first plugin failure; change it to handle per-plugin errors by wrapping the client.callTool invocation for each plugin in a try/catch, referencing enableConfiguredPlugins, MCP_PLUGIN_OPTIONS and the call to client.callTool with "bittensor_enable_plugin"; on error, log the failure (including plugin.pluginName and the caught error) and continue to the next plugin instead of throwing so subsequent plugins and the client connection can proceed.
136-166: Consider adding connection timeout and client cleanup strategy.Two potential concerns with the client caching strategy:
No connection timeout:
client.connect(transport)at line 158 has no timeout, which could cause the promise to hang indefinitely if the remote MCP server is unresponsive.Unbounded client cache:
clientStatesgrows with each unique endpoint+JWT combination and clients are never proactively closed. For long-running gateway instances with many users, this could lead to resource exhaustion.♻️ Proposed fix to add timeout
+const CLIENT_CONNECT_TIMEOUT_MS = 30_000; + +function withTimeout<T>(promise: Promise<T>, ms: number, message: string): Promise<T> { + return Promise.race([ + promise, + new Promise<T>((_, reject) => + setTimeout(() => reject(new Error(message)), ms), + ), + ]); +} + async function getClient(endpoint: string, jwt: string | undefined): Promise<Client> { const stateKey = `${endpoint}::${jwt ?? ""}`; let state = clientStates.get(stateKey); if (!state) { const enabledPlugins = loadEnabledPlugins(); state = { endpoint, jwt, clientPromise: (async () => { const client = new Client({ name: "openclaw-remote-mcp", version: process.env.OPENCLAW_VERSION ?? process.env.npm_package_version ?? "dev", }); const transport = new StreamableHTTPClientTransport(new URL(endpoint), { requestInit: jwt ? { headers: { Authorization: `Bearer ${jwt}`, }, } : undefined, }); - await client.connect(transport); + await withTimeout( + client.connect(transport), + CLIENT_CONNECT_TIMEOUT_MS, + "MCP client connection timed out", + ); await enableConfiguredPlugins(client, enabledPlugins); return client; })(), }; clientStates.set(stateKey, state); } return await state.clientPromise; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/tools/remote-mcp.ts` around lines 136 - 166, getClient currently awaits client.connect(transport) with no timeout and caches clients permanently in clientStates, which can hang on connect and leak resources; wrap the connect call in a timeout (use an AbortController or Promise.race with a configurable timeout) and ensure the transport/connect is aborted and the partially-created client closed on timeout or connect failure, and implement a cleanup/eviction strategy for clientStates (e.g., TTL or LRU) that removes and calls client.close() on stale entries; update getClient, state.clientPromise, clientStates usage and the Client/StreamableHTTPClientTransport error/finally handling to ensure failed or evicted clients are properly closed and removed.src/agents/openclaw-tools.ts (1)
182-186: Consider checking for tool name conflicts with remote MCP tools.Unlike
pluginToolswhich receivesexistingToolNamesto avoid conflicts (line 178),remoteMcpToolsis added without checking for duplicate names. If a remote MCP tool has the same normalized name as a built-in or plugin tool, it could cause unexpected behavior.♻️ Proposed fix to filter name conflicts
+ const existingNames = new Set([...tools.map((t) => t.name), ...pluginTools.map((t) => t.name)]); const remoteMcpTools = createRemoteMcpTools({ agentSessionKey: options?.agentSessionKey, - }); + }).filter((t) => !existingNames.has(t.name)); return [...tools, ...pluginTools, ...remoteMcpTools];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/openclaw-tools.ts` around lines 182 - 186, Remote MCP tools are appended without checking for name conflicts, which can collide with built-in or plugin tools; modify the creation/append of remoteMcpTools so duplicate normalized names are avoided by either passing existingToolNames into createRemoteMcpTools (if supported) or by filtering remoteMcpTools after creation against the set of normalized names from tools and pluginTools (use the same normalization helper used for pluginTools) before returning [...tools, ...pluginTools, ...remoteMcpTools]; ensure you update references to remoteMcpTools/createRemoteMcpTools and reuse the existingToolNames/normalization logic to prevent name collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gateway/control-ui-csp.ts`:
- Around line 1-5: The buildControlUiCspHeader function currently injects
allowedOrigins verbatim into the frame-ancestors directive; instead, validate
and normalize each entry by parsing it as a URL, only accept schemes "http:" or
"https:", and replace each item with its url.origin (reject entries that fail to
parse or do not round-trip to url.origin). Use the normalized origins array when
building frameAncestors and if no origins remain fall back to "frame-ancestors
'none'". Ensure this validation happens on the allowedOrigins input before the
join so malicious strings with whitespace or semicolons cannot alter the CSP.
In `@src/gateway/mcp-auth-store.ts`:
- Around line 30-39: bindConnectionToSession currently adds a sessionKey to a
connection without removing it from any previous owner, causing stale ownership
and incorrect JWT deletions; update bindConnectionToSession (and related
clear/update code path) to first look up and detach the sessionKey from its
previous connection before adding it to the new binding: either maintain a
reverse index (e.g., sessionToConn map keyed by sessionKey) or call a helper
that finds and removes the sessionKey from the old binding.sessions set (use
getOrCreateConnection, binding.sessions, and sessionToJwt names to locate code),
then set sessionToConn[sessionKey] = connId and update sessionToJwt only for the
current owner so subsequent clears only remove JWTs when sessionToConn still
points to that connection.
- Around line 18-27: setDelegatedJwtForConnection currently treats jwt ===
undefined as a no-op and leaves any previous binding.jwt and sessionToJwt
entries intact; change it so that calling setDelegatedJwtForConnection(connId,
undefined) clears the delegated JWT: call getOrCreateConnection(connId) to get
binding, set binding.jwt = undefined (or delete it), then iterate
binding.sessions and remove each session key from sessionToJwt (use
sessionToJwt.delete(sessionKey)); otherwise (when a non-empty trimmed jwt
exists) set binding.jwt = normalized and update sessionToJwt for each session as
before.
In `@src/gateway/server-methods/chat.ts`:
- Around line 574-576: The bindConnectionToSession call is happening too early
and can overwrite session→JWT mappings for rejected sends; move the
bindConnectionToSession(client.connId, sessionKey) so it executes only after the
session is accepted (i.e., after the sendPolicy/allow-path and after
stop-command short-circuit checks succeed in the chat.send flow), and remove any
early/optimistic binds on the reject path; additionally, add a dedicated
reconnect/session-restore path that re-binds existing sessions on connection
restore (reuse bindConnectionToSession in that handler) so delegated JWTs
persist across page refreshes and close-path cleanup.
In `@ui/src/ui/controllers/auth0.ts`:
- Around line 31-46: The redirect URI passed into authorizationParams is built
from window.location.href which may contain transient code/state/iss query
params on callback; instead, strip those params before constructing the
Auth0Client so authorizationParams.redirect_uri uses the cleaned URL.
Concretely, compute a cleanUrl by creating new URL(window.location.href), delete
"code", "state", and "iss" from cleanUrl.searchParams, then set
authorizationParams: { audience, redirect_uri: cleanUrl.toString() } when
initializing the client (before calling client.handleRedirectCallback() or
window.history.replaceState).
---
Outside diff comments:
In `@ui/src/ui/controllers/control-ui-bootstrap.ts`:
- Around line 29-49: The bootstrap fetch path currently only calls
setAuth0BootstrapConfig(parsed.auth0) on success, leaving stale Auth0 config
when the request fails; update the error paths (both the !res.ok early return
and the catch block in the fetch/await block inside control-ui-bootstrap.ts) to
clear the cached bootstrap state by calling setAuth0BootstrapConfig(null) (or
appropriate empty value) and also reset state.assistantName,
state.assistantAvatar and state.assistantAgentId to their default/empty values
so the UI won’t keep using the previous tenant’s Auth0 config after a failed
reload.
In `@ui/src/ui/gateway.ts`:
- Around line 129-170: The delegatedJwtProvider() call inside sendConnect is
unprotected and can reject, causing an unhandled rejection and aborting the
connect logic; wrap the await this.opts.delegatedJwtProvider?.() call in a
try/catch inside sendConnect (identify the delegatedJwt variable and sendConnect
function) and on error set delegatedJwt = undefined (or null), optionally log
the failure via the gateway logger, and continue building the auth frame so the
socket either connects without delegation or fails fast rather than leaving an
unhandled rejection.
---
Nitpick comments:
In `@src/agents/openclaw-tools.ts`:
- Around line 182-186: Remote MCP tools are appended without checking for name
conflicts, which can collide with built-in or plugin tools; modify the
creation/append of remoteMcpTools so duplicate normalized names are avoided by
either passing existingToolNames into createRemoteMcpTools (if supported) or by
filtering remoteMcpTools after creation against the set of normalized names from
tools and pluginTools (use the same normalization helper used for pluginTools)
before returning [...tools, ...pluginTools, ...remoteMcpTools]; ensure you
update references to remoteMcpTools/createRemoteMcpTools and reuse the
existingToolNames/normalization logic to prevent name collisions.
In `@src/agents/tools/remote-mcp.ts`:
- Around line 121-134: The loop in enableConfiguredPlugins currently aborts on
the first plugin failure; change it to handle per-plugin errors by wrapping the
client.callTool invocation for each plugin in a try/catch, referencing
enableConfiguredPlugins, MCP_PLUGIN_OPTIONS and the call to client.callTool with
"bittensor_enable_plugin"; on error, log the failure (including
plugin.pluginName and the caught error) and continue to the next plugin instead
of throwing so subsequent plugins and the client connection can proceed.
- Around line 136-166: getClient currently awaits client.connect(transport) with
no timeout and caches clients permanently in clientStates, which can hang on
connect and leak resources; wrap the connect call in a timeout (use an
AbortController or Promise.race with a configurable timeout) and ensure the
transport/connect is aborted and the partially-created client closed on timeout
or connect failure, and implement a cleanup/eviction strategy for clientStates
(e.g., TTL or LRU) that removes and calls client.close() on stale entries;
update getClient, state.clientPromise, clientStates usage and the
Client/StreamableHTTPClientTransport error/finally handling to ensure failed or
evicted clients are properly closed and removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 64b2c609-799a-4390-ae6c-ec56792c497f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
git-hooks/pre-commitpackage.jsonsrc/agents/openclaw-tools.tssrc/agents/tools/remote-mcp.test.tssrc/agents/tools/remote-mcp.tssrc/gateway/control-ui-contract.tssrc/gateway/control-ui-csp.tssrc/gateway/control-ui.tssrc/gateway/mcp-auth-store.test.tssrc/gateway/mcp-auth-store.tssrc/gateway/protocol/schema/frames.tssrc/gateway/server-http.tssrc/gateway/server-methods/chat.tssrc/gateway/server/ws-connection.tssrc/gateway/server/ws-connection/message-handler.tsui/package.jsonui/src/ui/app-gateway.tsui/src/ui/app-lifecycle.tsui/src/ui/controllers/auth0.tsui/src/ui/controllers/control-ui-bootstrap.tsui/src/ui/gateway.ts
Comment on lines
+1
to
+5
| export function buildControlUiCspHeader(allowedOrigins?: string[]): string { | ||
| const frameAncestors = | ||
| allowedOrigins && allowedOrigins.length > 0 | ||
| ? `frame-ancestors ${allowedOrigins.join(" ")}` | ||
| : "frame-ancestors 'none'"; |
There was a problem hiding this comment.
Validate and normalize allowedOrigins before emitting them into frame-ancestors.
These values currently come from config as arbitrary strings and are inserted verbatim into the CSP. A value containing whitespace or ; can change the effective policy instead of just adding one trusted origin. Please parse each entry as a single http(s) origin and reject anything that does not round-trip to url.origin before joining it into the header.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gateway/control-ui-csp.ts` around lines 1 - 5, The
buildControlUiCspHeader function currently injects allowedOrigins verbatim into
the frame-ancestors directive; instead, validate and normalize each entry by
parsing it as a URL, only accept schemes "http:" or "https:", and replace each
item with its url.origin (reject entries that fail to parse or do not round-trip
to url.origin). Use the normalized origins array when building frameAncestors
and if no origins remain fall back to "frame-ancestors 'none'". Ensure this
validation happens on the allowedOrigins input before the join so malicious
strings with whitespace or semicolons cannot alter the CSP.
Comment on lines
+18
to
+27
| export function setDelegatedJwtForConnection(connId: string, jwt: string | undefined): void { | ||
| const normalized = jwt?.trim(); | ||
| if (!normalized) { | ||
| return; | ||
| } | ||
| const binding = getOrCreateConnection(connId); | ||
| binding.jwt = normalized; | ||
| for (const sessionKey of binding.sessions) { | ||
| sessionToJwt.set(sessionKey, normalized); | ||
| } |
There was a problem hiding this comment.
undefined should clear an existing delegated JWT, not leave it active.
setDelegatedJwtForConnection(..., undefined) is a no-op right now, so any previously bound JWT keeps applying to all sessions on that connection. That leaves stale per-user auth in place after logout or reconnect-without-delegation.
Suggested fix
export function setDelegatedJwtForConnection(connId: string, jwt: string | undefined): void {
const normalized = jwt?.trim();
- if (!normalized) {
- return;
- }
const binding = getOrCreateConnection(connId);
+ if (!normalized) {
+ delete binding.jwt;
+ for (const sessionKey of binding.sessions) {
+ sessionToJwt.delete(sessionKey);
+ }
+ return;
+ }
binding.jwt = normalized;
for (const sessionKey of binding.sessions) {
sessionToJwt.set(sessionKey, normalized);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gateway/mcp-auth-store.ts` around lines 18 - 27,
setDelegatedJwtForConnection currently treats jwt === undefined as a no-op and
leaves any previous binding.jwt and sessionToJwt entries intact; change it so
that calling setDelegatedJwtForConnection(connId, undefined) clears the
delegated JWT: call getOrCreateConnection(connId) to get binding, set
binding.jwt = undefined (or delete it), then iterate binding.sessions and remove
each session key from sessionToJwt (use sessionToJwt.delete(sessionKey));
otherwise (when a non-empty trimmed jwt exists) set binding.jwt = normalized and
update sessionToJwt for each session as before.
Comment on lines
+30
to
+39
| export function bindConnectionToSession(connId: string, sessionKey: string): void { | ||
| const normalizedSessionKey = sessionKey.trim(); | ||
| if (!normalizedSessionKey) { | ||
| return; | ||
| } | ||
| const binding = getOrCreateConnection(connId); | ||
| binding.sessions.add(normalizedSessionKey); | ||
| if (binding.jwt) { | ||
| sessionToJwt.set(normalizedSessionKey, binding.jwt); | ||
| } |
There was a problem hiding this comment.
Rebinding a session to a new connection leaves stale ownership behind.
If a sessionKey moves from connection A to B, A still keeps that session in binding.sessions. When A is later cleared, Lines 55-56 delete the JWT mapping for a session that now belongs to B. The same bug also preserves A's JWT when the session is rebound to a new connection that has no delegated token yet.
A reverse sessionKey -> connId index, or explicit detachment from the previous binding before add(), is needed here so clear/update operations only touch the current owner.
Also applies to: 50-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gateway/mcp-auth-store.ts` around lines 30 - 39, bindConnectionToSession
currently adds a sessionKey to a connection without removing it from any
previous owner, causing stale ownership and incorrect JWT deletions; update
bindConnectionToSession (and related clear/update code path) to first look up
and detach the sessionKey from its previous connection before adding it to the
new binding: either maintain a reverse index (e.g., sessionToConn map keyed by
sessionKey) or call a helper that finds and removes the sessionKey from the old
binding.sessions set (use getOrCreateConnection, binding.sessions, and
sessionToJwt names to locate code), then set sessionToConn[sessionKey] = connId
and update sessionToJwt only for the current owner so subsequent clears only
remove JWTs when sessionToConn still points to that connection.
Comment on lines
+574
to
+576
| if (client?.connId) { | ||
| bindConnectionToSession(client.connId, sessionKey); | ||
| } |
There was a problem hiding this comment.
Bind the delegated JWT only after the session is actually accepted.
This call runs before sendPolicy/stop-command short-circuits, so a rejected chat.send can still overwrite the session→JWT mapping. It is also the only bind point in the provided flow, so after a page refresh the close-path cleanup drops delegation for any in-flight session until the user sends another message. At minimum, move the bind below the allow-path checks; ideally add a reconnect/session-restore bind path too.
🔧 Minimal safe move
const rawSessionKey = p.sessionKey;
const { cfg, entry, canonicalKey: sessionKey } = loadSessionEntry(rawSessionKey);
- if (client?.connId) {
- bindConnectionToSession(client.connId, sessionKey);
- }
const timeoutMs = resolveAgentTimeoutMs({
cfg,
overrideMs: p.timeoutMs,
});
@@
if (stopCommand) {
const res = abortChatRunsForSessionKeyWithPartials({
context,
ops: createChatAbortOps(context),
sessionKey: rawSessionKey,
abortOrigin: "stop-command",
stopReason: "stop",
});
respond(true, { ok: true, aborted: res.aborted, runIds: res.runIds });
return;
}
+
+ if (client?.connId) {
+ bindConnectionToSession(client.connId, sessionKey);
+ }Also applies to: 584-609
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/gateway/server-methods/chat.ts` around lines 574 - 576, The
bindConnectionToSession call is happening too early and can overwrite
session→JWT mappings for rejected sends; move the
bindConnectionToSession(client.connId, sessionKey) so it executes only after the
session is accepted (i.e., after the sendPolicy/allow-path and after
stop-command short-circuit checks succeed in the chat.send flow), and remove any
early/optimistic binds on the reject path; additionally, add a dedicated
reconnect/session-restore path that re-binds existing sessions on connection
restore (reuse bindConnectionToSession in that handler) so delegated JWTs
persist across page refreshes and close-path cleanup.
Comment on lines
+31
to
+46
| authorizationParams: { | ||
| audience, | ||
| redirect_uri: window.location.href, | ||
| }, | ||
| cacheLocation: "memory", | ||
| useRefreshTokens: false, | ||
| }); | ||
| const search = new URLSearchParams(window.location.search); | ||
| if (search.get("code") && search.get("state")) { | ||
| try { | ||
| await client.handleRedirectCallback(); | ||
| const cleanUrl = new URL(window.location.href); | ||
| cleanUrl.searchParams.delete("code"); | ||
| cleanUrl.searchParams.delete("state"); | ||
| cleanUrl.searchParams.delete("iss"); | ||
| window.history.replaceState({}, "", cleanUrl.toString()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "auth0.ts" | head -20Repository: one-covenant/openclaw
Length of output: 97
🏁 Script executed:
cat -n ./ui/src/ui/controllers/auth0.ts | sed -n '1,60p'Repository: one-covenant/openclaw
Length of output: 2340
🏁 Script executed:
web_search
Auth0 redirect_uri handling redirect_uri with query parameters Auth0Client initializationRepository: one-covenant/openclaw
Length of output: 159
🌐 Web query:
Auth0 redirect_uri callback URL query parameters handling SPA
💡 Result:
In an Auth0 SPA (Authorization Code + PKCE) redirect flow, Auth0 will send the user back to your redirect_uri with Auth0-owned query params like code and state. Your app should:
- Detect the callback and finish the transaction
- Only treat the navigation as an Auth0 callback when you see
code+statein the URL, then callhandleRedirectCallback()to complete the login. Auth0’s vanilla JS quickstart explicitly recommends this detection pattern. [1]
- Remove Auth0 query params from the URL
- After
handleRedirectCallback(), removecode/statefrom the address bar (e.g., viahistory.replaceState) so refreshes don’t re-run callback parsing. Auth0’s quickstart calls this out. [2]
- Don’t rely on your own query params on the callback URL
redirect_urimust match an entry in Allowed Callback URLs, otherwise you’ll get a redirect URI mismatch error. [3]- Because of that strict matching, putting “dynamic” app routing info into
redirect_uriquery parameters is fragile (often the cause of mismatch errors).
- How to preserve/restore where the user should land (recommended)
- Use
appStatewithloginWithRedirect()and read it back fromhandleRedirectCallback()result, then route internally toappState.targetUrl(or similar). This is the intended SPA pattern and is built into the SPA SDK types/docs. [4]
Typical pattern
redirect_uri: a stable callback route likehttps://app.example.com/callback- Store desired post-login route in
appState(or encoded inside OAuthstate), then afterhandleRedirectCallback()navigate to it.
Sources: Auth0 SPA quickstart (callback detection + removing query params) [1][2], Auth0 guidance on Allowed Callback URLs / redirect_uri usage [3], Auth0 SPA SDK appState option [4].
🌐 Web query:
Auth0 auth0-spa-js Auth0Client redirect_uri SPA initialization configuration
💡 Result:
To configure the callback/redirect URL for @auth0/auth0-spa-js (Auth0Client) in an SPA, set redirect_uri under authorizationParams when you create the client:
import { createAuth0Client } from '@auth0/auth0-spa-js';
const auth0 = await createAuth0Client({
domain: 'YOUR_DOMAIN',
clientId: 'YOUR_CLIENT_ID',
authorizationParams: {
redirect_uri: window.location.origin + '/callback',
},
});authorizationParams.redirect_uriis the default URL Auth0 will redirect back to after login, and it must be listed in your Auth0 app’s “Allowed Callback URLs” (otherwise you’ll get a callback URL mismatch error). [1][2][3]- You can also override it per-login call (e.g., if you have multiple callback routes) by passing
authorizationParams.redirect_uritologinWithRedirect(...); otherwise, if you don’t provide it in client init, you must provide it in the methods that initiate authentication. [2] - Your SPA should call
handleRedirectCallback()on the route you use as the redirect/callback URL. [1]
Auth0 Dashboard setting: Add the same URL (including scheme/host/port and, if you use one, the path like /callback) to Application Settings → Allowed Callback URLs. [1][3]
Sources:
[1] Auth0 SPA SDK README (auth0-spa-js) – configuration and authorizationParams.redirect_uri example
[2] AuthorizationParams.redirect_uri API docs (auth0-spa-js) – meaning/requirements and whitelisting note
[3] Auth0 docs / community guidance – redirect_uri must match an Allowed Callback URL (callback URL mismatch)
Clean the redirect URL before creating the Auth0Client, not after.
When Auth0 redirects back on a callback load, window.location.href contains transient code, state, and iss query parameters. Passing this unclean URL as redirect_uri during Auth0Client initialization causes future authentication attempts to fail, because the stored redirect_uri won't match the "Allowed Callback URLs" configured in your Auth0 app.
Suggested fix
+ const redirectUrl = new URL(window.location.href);
+ redirectUrl.searchParams.delete("code");
+ redirectUrl.searchParams.delete("state");
+ redirectUrl.searchParams.delete("iss");
+
const client = new Auth0Client({
domain,
clientId,
authorizationParams: {
audience,
- redirect_uri: window.location.href,
+ redirect_uri: redirectUrl.toString(),
},
cacheLocation: "memory",
useRefreshTokens: false,
});
const search = new URLSearchParams(window.location.search);
if (search.get("code") && search.get("state")) {
try {
await client.handleRedirectCallback();
- const cleanUrl = new URL(window.location.href);
- cleanUrl.searchParams.delete("code");
- cleanUrl.searchParams.delete("state");
- cleanUrl.searchParams.delete("iss");
- window.history.replaceState({}, "", cleanUrl.toString());
+ window.history.replaceState({}, "", redirectUrl.toString());
} catch {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/ui/controllers/auth0.ts` around lines 31 - 46, The redirect URI passed
into authorizationParams is built from window.location.href which may contain
transient code/state/iss query params on callback; instead, strip those params
before constructing the Auth0Client so authorizationParams.redirect_uri uses the
cleaned URL. Concretely, compute a cleanUrl by creating new
URL(window.location.href), delete "code", "state", and "iss" from
cleanUrl.searchParams, then set authorizationParams: { audience, redirect_uri:
cleanUrl.toString() } when initializing the client (before calling
client.handleRedirectCallback() or window.history.replaceState).
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/agents/tools/remote-mcp.ts`:
- Around line 36-44: clientStates (and its RemoteMcpClientState values including
clientPromise) is an unbounded Map which can grow indefinitely; implement an
eviction policy (e.g., LRU or TTL or a max-size) to prevent memory growth: add a
MAX_CLIENT_STATES constant and wrap Map access so that whenever a new entry is
inserted you evict the least-recently-used or oldest entry (or expire entries
older than TTL) and update all getters/setters to touch/access order (or store a
lastAccess timestamp inside RemoteMcpClientState); ensure creation sites that
populate clientStates and reads of clientPromise update the access metadata and
that eviction properly closes/cleans up Client instances before removal.
- Around line 183-216: The retry path in callRemoteTool currently reuses the
original jwt returned from getDelegatedJwtForSession, so if that was undefined
the retried call will hit the same auth failure; before creating retriedClient
and retrying the call (the second getClient/getClient call and
retriedClient.callTool), re-fetch the delegated JWT by calling
getDelegatedJwtForSession(params.sessionKey) again and use that value (and
recompute stateKey) when obtaining retriedClient, or alternatively skip the
retry when the error is an auth error and the re-fetched jwt is still undefined;
update references to jwt/stateKey/retriedClient in callRemoteTool accordingly.
- Around line 145-181: The cached rejected promise issue is in getClient: when
the async clientPromise (which calls client.connect and enableConfiguredPlugins)
rejects, that rejected promise remains in clientStates under stateKey; catch any
error from state.clientPromise inside the creation block (or wrap the IIFE) to
delete clientStates.delete(stateKey) before rethrowing so subsequent calls can
retry; specifically modify the clientPromise IIFE or attach a .catch handler
that removes clientStates entry on rejection and then throws the error, keeping
references to getClient, clientStates, stateKey, client.connect, and
enableConfiguredPlugins to locate the code to change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08f9a774-984a-48f8-ad9f-016dcb0f8cc5
📒 Files selected for processing (1)
src/agents/tools/remote-mcp.ts
Comment on lines
+36
to
+44
| type RemoteMcpClientState = { | ||
| endpoint: string; | ||
| jwt?: string; | ||
| basilicaAuthToken?: string; | ||
| clientPromise: Promise<Client>; | ||
| }; | ||
|
|
||
| const clientStates = new Map<string, RemoteMcpClientState>(); | ||
| let manifestCache: RemoteMcpManifest | null | undefined; |
There was a problem hiding this comment.
Unbounded client cache may cause memory growth.
The clientStates map grows indefinitely as new endpoint/JWT/token combinations are added. In long-running processes with many sessions, this could lead to memory issues since clients are never proactively evicted.
Consider implementing a cache eviction strategy (e.g., LRU, TTL-based expiry, or max size limit).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agents/tools/remote-mcp.ts` around lines 36 - 44, clientStates (and its
RemoteMcpClientState values including clientPromise) is an unbounded Map which
can grow indefinitely; implement an eviction policy (e.g., LRU or TTL or a
max-size) to prevent memory growth: add a MAX_CLIENT_STATES constant and wrap
Map access so that whenever a new entry is inserted you evict the
least-recently-used or oldest entry (or expire entries older than TTL) and
update all getters/setters to touch/access order (or store a lastAccess
timestamp inside RemoteMcpClientState); ensure creation sites that populate
clientStates and reads of clientPromise update the access metadata and that
eviction properly closes/cleans up Client instances before removal.
Comment on lines
+145
to
+181
| async function getClient(endpoint: string, jwt: string | undefined): Promise<Client> { | ||
| const basilicaAuthToken = process.env[BASILICA_AUTH_TOKEN_ENV_KEY]?.trim(); | ||
| const stateKey = `${endpoint}::${jwt ?? ""}::${basilicaAuthToken ?? ""}`; | ||
| let state = clientStates.get(stateKey); | ||
| if (!state) { | ||
| const enabledPlugins = loadEnabledPlugins(); | ||
| state = { | ||
| endpoint, | ||
| jwt, | ||
| basilicaAuthToken, | ||
| clientPromise: (async () => { | ||
| const client = new Client({ | ||
| name: "openclaw-remote-mcp", | ||
| version: process.env.OPENCLAW_VERSION ?? process.env.npm_package_version ?? "dev", | ||
| }); | ||
| const transport = new StreamableHTTPClientTransport(new URL(endpoint), { | ||
| requestInit: | ||
| jwt || basilicaAuthToken | ||
| ? { | ||
| headers: { | ||
| ...(jwt ? { Authorization: `Bearer ${jwt}` } : {}), | ||
| ...(basilicaAuthToken | ||
| ? { [BASILICA_AUTH_TOKEN_HEADER]: basilicaAuthToken } | ||
| : {}), | ||
| }, | ||
| } | ||
| : undefined, | ||
| }); | ||
| await client.connect(transport); | ||
| await enableConfiguredPlugins(client, enabledPlugins); | ||
| return client; | ||
| })(), | ||
| }; | ||
| clientStates.set(stateKey, state); | ||
| } | ||
| return await state.clientPromise; | ||
| } |
There was a problem hiding this comment.
Rejected promise gets cached permanently, causing persistent failures.
If client.connect() or enableConfiguredPlugins() throws, the rejected promise is cached in clientStates. All subsequent calls for the same key will immediately receive the same rejection without retry.
Consider removing the cached state on promise rejection:
🐛 Proposed fix to handle rejected promises
state = {
endpoint,
jwt,
basilicaAuthToken,
clientPromise: (async () => {
- const client = new Client({
- name: "openclaw-remote-mcp",
- version: process.env.OPENCLAW_VERSION ?? process.env.npm_package_version ?? "dev",
- });
- const transport = new StreamableHTTPClientTransport(new URL(endpoint), {
- requestInit:
- jwt || basilicaAuthToken
- ? {
- headers: {
- ...(jwt ? { Authorization: `Bearer ${jwt}` } : {}),
- ...(basilicaAuthToken
- ? { [BASILICA_AUTH_TOKEN_HEADER]: basilicaAuthToken }
- : {}),
- },
- }
- : undefined,
- });
- await client.connect(transport);
- await enableConfiguredPlugins(client, enabledPlugins);
- return client;
+ try {
+ const client = new Client({
+ name: "openclaw-remote-mcp",
+ version: process.env.OPENCLAW_VERSION ?? process.env.npm_package_version ?? "dev",
+ });
+ const transport = new StreamableHTTPClientTransport(new URL(endpoint), {
+ requestInit:
+ jwt || basilicaAuthToken
+ ? {
+ headers: {
+ ...(jwt ? { Authorization: `Bearer ${jwt}` } : {}),
+ ...(basilicaAuthToken
+ ? { [BASILICA_AUTH_TOKEN_HEADER]: basilicaAuthToken }
+ : {}),
+ },
+ }
+ : undefined,
+ });
+ await client.connect(transport);
+ await enableConfiguredPlugins(client, enabledPlugins);
+ return client;
+ } catch (err) {
+ clientStates.delete(stateKey);
+ throw err;
+ }
})(),
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agents/tools/remote-mcp.ts` around lines 145 - 181, The cached rejected
promise issue is in getClient: when the async clientPromise (which calls
client.connect and enableConfiguredPlugins) rejects, that rejected promise
remains in clientStates under stateKey; catch any error from state.clientPromise
inside the creation block (or wrap the IIFE) to delete
clientStates.delete(stateKey) before rethrowing so subsequent calls can retry;
specifically modify the clientPromise IIFE or attach a .catch handler that
removes clientStates entry on rejection and then throws the error, keeping
references to getClient, clientStates, stateKey, client.connect, and
enableConfiguredPlugins to locate the code to change.
Comment on lines
+183
to
+216
| async function callRemoteTool(params: { | ||
| endpoint: string; | ||
| sessionKey?: string; | ||
| name: string; | ||
| args: Record<string, unknown>; | ||
| }): Promise<Record<string, unknown>> { | ||
| const jwt = getDelegatedJwtForSession(params.sessionKey); | ||
| const client = await getClient(params.endpoint, jwt); | ||
| try { | ||
| return /** @type {Record<string, unknown>} */ ( | ||
| await client.callTool({ | ||
| name: params.name, | ||
| arguments: params.args, | ||
| }) | ||
| ); | ||
| } catch (error) { | ||
| const basilicaAuthToken = process.env[BASILICA_AUTH_TOKEN_ENV_KEY]?.trim(); | ||
| const stateKey = `${params.endpoint}::${jwt ?? ""}::${basilicaAuthToken ?? ""}`; | ||
| clientStates.delete(stateKey); | ||
| const message = error instanceof Error ? error.message : formatUnknown(error); | ||
| const recoverable = | ||
| message.includes("session") || message.includes("Unauthorized") || message.includes("closed"); | ||
| if (!recoverable) { | ||
| throw error; | ||
| } | ||
| const retriedClient = await getClient(params.endpoint, jwt); | ||
| return /** @type {Record<string, unknown>} */ ( | ||
| await retriedClient.callTool({ | ||
| name: params.name, | ||
| arguments: params.args, | ||
| }) | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Retry logic reuses stale JWT, guaranteeing identical failure for auth errors.
When getDelegatedJwtForSession returns undefined (no binding exists) and the call fails with "Unauthorized", the retry at line 208 uses the same jwt variable without re-fetching. This makes the retry attempt identical to the original, guaranteeing the same failure.
Consider re-fetching the JWT before retrying, or skipping retry for auth errors when no JWT is available:
🐛 Proposed fix to refresh JWT on retry
} catch (error) {
const basilicaAuthToken = process.env[BASILICA_AUTH_TOKEN_ENV_KEY]?.trim();
const stateKey = `${params.endpoint}::${jwt ?? ""}::${basilicaAuthToken ?? ""}`;
clientStates.delete(stateKey);
const message = error instanceof Error ? error.message : formatUnknown(error);
const recoverable =
message.includes("session") || message.includes("Unauthorized") || message.includes("closed");
if (!recoverable) {
throw error;
}
- const retriedClient = await getClient(params.endpoint, jwt);
+ // Re-fetch JWT in case it was updated since the original call
+ const refreshedJwt = getDelegatedJwtForSession(params.sessionKey);
+ // If still no JWT and error was auth-related, don't retry pointlessly
+ if (!refreshedJwt && message.includes("Unauthorized")) {
+ throw error;
+ }
+ const retriedClient = await getClient(params.endpoint, refreshedJwt);
return /** `@type` {Record<string, unknown>} */ (
await retriedClient.callTool({📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function callRemoteTool(params: { | |
| endpoint: string; | |
| sessionKey?: string; | |
| name: string; | |
| args: Record<string, unknown>; | |
| }): Promise<Record<string, unknown>> { | |
| const jwt = getDelegatedJwtForSession(params.sessionKey); | |
| const client = await getClient(params.endpoint, jwt); | |
| try { | |
| return /** @type {Record<string, unknown>} */ ( | |
| await client.callTool({ | |
| name: params.name, | |
| arguments: params.args, | |
| }) | |
| ); | |
| } catch (error) { | |
| const basilicaAuthToken = process.env[BASILICA_AUTH_TOKEN_ENV_KEY]?.trim(); | |
| const stateKey = `${params.endpoint}::${jwt ?? ""}::${basilicaAuthToken ?? ""}`; | |
| clientStates.delete(stateKey); | |
| const message = error instanceof Error ? error.message : formatUnknown(error); | |
| const recoverable = | |
| message.includes("session") || message.includes("Unauthorized") || message.includes("closed"); | |
| if (!recoverable) { | |
| throw error; | |
| } | |
| const retriedClient = await getClient(params.endpoint, jwt); | |
| return /** @type {Record<string, unknown>} */ ( | |
| await retriedClient.callTool({ | |
| name: params.name, | |
| arguments: params.args, | |
| }) | |
| ); | |
| } | |
| } | |
| async function callRemoteTool(params: { | |
| endpoint: string; | |
| sessionKey?: string; | |
| name: string; | |
| args: Record<string, unknown>; | |
| }): Promise<Record<string, unknown>> { | |
| const jwt = getDelegatedJwtForSession(params.sessionKey); | |
| const client = await getClient(params.endpoint, jwt); | |
| try { | |
| return /** `@type` {Record<string, unknown>} */ ( | |
| await client.callTool({ | |
| name: params.name, | |
| arguments: params.args, | |
| }) | |
| ); | |
| } catch (error) { | |
| const basilicaAuthToken = process.env[BASILICA_AUTH_TOKEN_ENV_KEY]?.trim(); | |
| const stateKey = `${params.endpoint}::${jwt ?? ""}::${basilicaAuthToken ?? ""}`; | |
| clientStates.delete(stateKey); | |
| const message = error instanceof Error ? error.message : formatUnknown(error); | |
| const recoverable = | |
| message.includes("session") || message.includes("Unauthorized") || message.includes("closed"); | |
| if (!recoverable) { | |
| throw error; | |
| } | |
| // Re-fetch JWT in case it was updated since the original call | |
| const refreshedJwt = getDelegatedJwtForSession(params.sessionKey); | |
| // If still no JWT and error was auth-related, don't retry pointlessly | |
| if (!refreshedJwt && message.includes("Unauthorized")) { | |
| throw error; | |
| } | |
| const retriedClient = await getClient(params.endpoint, refreshedJwt); | |
| return /** `@type` {Record<string, unknown>} */ ( | |
| await retriedClient.callTool({ | |
| name: params.name, | |
| arguments: params.args, | |
| }) | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agents/tools/remote-mcp.ts` around lines 183 - 216, The retry path in
callRemoteTool currently reuses the original jwt returned from
getDelegatedJwtForSession, so if that was undefined the retried call will hit
the same auth failure; before creating retriedClient and retrying the call (the
second getClient/getClient call and retriedClient.callTool), re-fetch the
delegated JWT by calling getDelegatedJwtForSession(params.sessionKey) again and
use that value (and recompute stateKey) when obtaining retriedClient, or
alternatively skip the retry when the error is an auth error and the re-fetched
jwt is still undefined; update references to jwt/stateKey/retriedClient in
callRemoteTool accordingly.
epappas
approved these changes
Mar 12, 2026
There was a problem hiding this comment.
check with your AI, because we are placing our openclaw offering in an iframe.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Describe the problem and fix in 2–5 bullets:
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
List user-visible changes (including defaults/config).
If none, write
None.Security Impact (required)
Yes/No)Yes/No)Yes/No)Yes/No)Yes/No)Yes, explain risk + mitigation:Repro + Verification
Environment
Steps
Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
Compatibility / Migration
Yes/No)Yes/No)Yes/No)Failure Recovery (if this breaks)
Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.Summary by CodeRabbit
New Features
Chores