Merged
Conversation
Ultraworked with Sisyphus (oh-my-opencode).
Add @global RedisModule with ioredis provider (retry strategy, graceful disconnect), Redis constants (REDIS_CLIENT token, key prefixes), and RedisSessionRepository for session CRUD with TTL and stale SET member pruning. Add RedisSessionData interface to session DTO. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Rewrite SessionManagerService to use Redis-backed metadata with local McpServer cache. Add await to async session calls in McpController. Wire RedisSessionRepository into McpModule. Import RedisModule in AppModule. Update .env.example with required Redis vars. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add Redis 7-alpine service to production (docker-compose.yml) and staging (docker-compose.staging.yml) with persistent volumes, Redis env vars, depends_on, and MCP_SESSION_TTL configuration. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add unit tests for RedisSessionRepository (mock ioredis, CRUD, stale pruning, stats) and SessionManagerService (mock repository + factory, session lifecycle, cache miss recovery). Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add deploy.sh (unified VPS deploy with backup/rollback for prod+staging). Rewrite AGENT.md with full architecture, module map, data flow, and conventions. Create AGENTS.md (hierarchical subagent knowledge). Create docs/DEPLOYMENT.md (ops runbook). Move redis-plan.md to docs/feature-plan/. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Convert all CRLF line endings to LF across the codebase for consistency. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Update tool-registry to extract Bearer token from extra.authInfo instead of extra.sessionId. Add optional existingSessionId parameter to session-manager for SDK-managed session IDs. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…ansport Replace hand-rolled JSON-RPC dispatch with official @modelcontextprotocol/sdk StreamableHTTPServerTransport. Adds POST/GET/DELETE handlers for full MCP Streamable HTTP spec compliance, enabling n8n and other SDK-compliant clients to connect. Remove McpProtocolHandlerService from module (superseded by SDK transport). Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add DELETE to CORS methods, mcp-session-id to allowed/exposed headers for SDK transport. Fix staging docker-compose PORT default from 3001 to 3002 to match host mapping. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Remove TESTING_RESOURCES.md and docs/IMPLEMENTATION-SUMMARY.md that are no longer relevant after the StreamableHTTPServerTransport refactor. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
WalkthroughThis pull request refactors the MCP controller architecture from per-request protocol handler design to streaming HTTP transport-based session management. Changes include CORS and configuration updates, removal of documentation files, session manager enhancements with optional session ID support, and conversion of tool registry authorization from session ID to Bearer token format. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant McpController
participant SessionManager
participant ServerFactory
participant Transport
participant Redis
Note over Client,Redis: OLD FLOW: Per-Request Protocol Handler
Client->>McpController: POST /mcp/:projectApiKey (handleMcpRequest)
McpController->>ServerFactory: Create new server per request
McpController->>Transport: Protocol handler creates transport
Transport->>ServerFactory: Handle request
ServerFactory->>Redis: Lookup/store session
Transport-->>Client: Response
Note over Transport: Transport discarded after response
Note over Client,Redis: NEW FLOW: Persistent Transport Sessions
Client->>McpController: POST /mcp/:projectApiKey (initialize)
McpController->>McpController: validateAuth (JWT extraction)
McpController->>SessionManager: createSession with existingSessionId
SessionManager->>Redis: Persist session metadata
McpController->>ServerFactory: Create server
ServerFactory->>Transport: Create StreamableHTTPServerTransport
Transport->>McpController: Register transport instance
Transport-->>Client: Session established with mcp-session-id
Client->>McpController: GET /mcp/:projectApiKey (with mcp-session-id)
McpController->>McpController: Lookup transport by session ID
Transport->>ServerFactory: Route to existing server
ServerFactory-->>Client: Streaming response (SSE-like)
Client->>McpController: DELETE /mcp/:projectApiKey (with mcp-session-id)
McpController->>Transport: Close transport
Transport->>SessionManager: Cleanup session
SessionManager->>Redis: Remove session
Transport-->>Client: 200 OK (cleanup complete)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (2)
docker-compose.staging.yml (1)
8-12: Decouple host port and app port variables to avoid override mismatches.Using one
PORTvariable for both host binding and in-container app port can break routing if someone overrides it (container mapping target stays fixed unless updated too). Consider separate variables (HOST_PORT,APP_PORT) for safer staging overrides.Suggested compose tweak
services: iot-cloud-mcp-staging: @@ ports: - - '${PORT:-3002}:3002' + - '${HOST_PORT:-3002}:${APP_PORT:-3002}' @@ - PORT: ${PORT:-3002} + PORT: ${APP_PORT:-3002}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose.staging.yml` around lines 8 - 12, Decouple the host-to-container port binding by introducing HOST_PORT and APP_PORT instead of reusing PORT: change the port mapping that currently reads '- '${PORT:-3002}:3002'' to use HOST_PORT and APP_PORT (e.g. host binding '${HOST_PORT:-3002}:${APP_PORT:-3002}' or at minimum '${HOST_PORT:-3002}:3002'), and update the environment entry that currently reads 'PORT: ${PORT:-3002}' to use APP_PORT (e.g. 'PORT: ${APP_PORT:-3002}' or add 'APP_PORT: ${APP_PORT:-3002}') so the container's internal app port and the host binding are independent (refer to the existing symbols PORT, '- '${PORT:-3002}:3002'', and 'PORT: ${PORT:-3002}').src/tools/services/tool-registry.service.ts (1)
51-51: Extract Bearer header composition into a single helper.The same authorization expression is duplicated across all tool registrations, which makes future auth changes easy to miss in one or more tools.
♻️ Suggested refactor
+ const buildAuthorization = (extra: { authInfo?: { token?: string } }) => + extra.authInfo?.token ? `Bearer ${extra.authInfo.token}` : ''; + // Register fetchUser tool mcpServer.registerTool( FETCH_USER_TOOL.name, @@ async (params: Record<string, unknown>, extra) => { return this.toolExecutor.executeTool(FETCH_USER_TOOL.name, params, { - authorization: extra.authInfo?.token ? `Bearer ${extra.authInfo.token}` : '', + authorization: buildAuthorization(extra), projectApiKey, meta: extra as Record<string, unknown>, }); }, );Also applies to: 67-67, 83-83, 99-99, 115-115, 131-131, 147-147, 163-163, 179-179, 195-195, 211-211, 227-227, 243-243, 259-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/services/tool-registry.service.ts` at line 51, The repeated inline expression `extra.authInfo?.token ? \`Bearer ${extra.authInfo.token}\` : ''` used for the authorization header across registrations should be extracted into a single helper function (e.g., `composeBearerToken(extra)` or `getAuthorizationHeader(extra: ExtraType)`) inside ToolRegistryService (or nearby in src/tools/services/tool-registry.service.ts); implement the helper to return the exact header string and replace every inline occurrence (the authorization property used in each tool registration) with a call to that helper so all registrations (previously at the duplicated lines) use the centralized function.
🤖 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/mcp/mcp.controller.ts`:
- Around line 82-87: The code currently calls decodeJwt(token) and trusts
decoded.sub as the authenticated identity (decoded, userId) without
cryptographic verification; change this to perform full JWT verification
(signature, expiry/exp, issuer/iss and audience/aud) before using sub by
replacing decodeJwt with a verification call (e.g., verifyJwt or jwt.verify) or
invoking your AuthService.verifyToken, check returned payload is valid and not
expired, and only then extract userId from payload.sub; also handle verification
errors explicitly and reject tokens that fail signature or claim checks to
prevent impersonation.
- Around line 163-170: The code creates two different McpServer instances for
the same session (one passed to sessionManager.createSession and another later
used to connect the transport), which can desync server state; modify
onsessioninitialized so you create a single server via
serverFactory.createServer(projectApiKey) and reuse that same McpServer instance
for both sessionManager.createSession(...) and the transport connection (where
transport.connect/transport.setServer or equivalent is called), removing the
second serverFactory.createServer call and any duplicate server creation; update
references to that single variable in onsessioninitialized and any helper
methods so the session store and active transport share the exact same McpServer
object.
- Around line 143-145: When reusing or accepting a transport/session ID in the
MCP controller (places using mcpSessionId, this.transports, and
this.sessionProjectMap — e.g., the reuse check at the start of the POST handler
and the analogous checks in the GET and DELETE flows), require that
this.sessionProjectMap.get(mcpSessionId) === projectApiKey before allowing
reuse; if the mapping is absent or mismatched, treat the session as invalid and
do not return or reuse the transport. Also ensure that when creating a new
session you set this.sessionProjectMap.set(newSessionId, projectApiKey) so
future lookups enforce project binding. This change should be applied to the
checks around mcpSessionId at the three mentioned code sites.
- Around line 148-152: Add a proper module augmentation for Node's
IncomingMessage so you can attach auth without unsafe casts: create/update a
.d.ts types file that declares module 'http' { interface IncomingMessage {
auth?: AuthInfo } } then remove all occurrences of the double-cast (e.g., the
assignments and calls around req.auth and transport.handleRequest in
mcp.controller.ts) and use req.auth = authInfo and pass req directly to
transport.handleRequest and other call sites (refer to IncomingMessage, auth,
AuthInfo, and transport.handleRequest to locate and update all instances).
In `@src/mcp/services/session-manager.service.ts`:
- Around line 48-50: The createSession method currently trusts existingSessionId
blindly; validate existingSessionId in createSession (in
session-manager.service.ts) before using it as sessionId: only accept it if it
is a non-empty string and passes a UUID-v4 validation (use the uuid library's
validate/isUuid helper or a strict regex); if validation fails, discard it and
generate a new id via uuidv4(); optionally emit a warning log when an invalid
existingSessionId is provided to aid debugging.
---
Nitpick comments:
In `@docker-compose.staging.yml`:
- Around line 8-12: Decouple the host-to-container port binding by introducing
HOST_PORT and APP_PORT instead of reusing PORT: change the port mapping that
currently reads '- '${PORT:-3002}:3002'' to use HOST_PORT and APP_PORT (e.g.
host binding '${HOST_PORT:-3002}:${APP_PORT:-3002}' or at minimum
'${HOST_PORT:-3002}:3002'), and update the environment entry that currently
reads 'PORT: ${PORT:-3002}' to use APP_PORT (e.g. 'PORT: ${APP_PORT:-3002}' or
add 'APP_PORT: ${APP_PORT:-3002}') so the container's internal app port and the
host binding are independent (refer to the existing symbols PORT, '-
'${PORT:-3002}:3002'', and 'PORT: ${PORT:-3002}').
In `@src/tools/services/tool-registry.service.ts`:
- Line 51: The repeated inline expression `extra.authInfo?.token ? \`Bearer
${extra.authInfo.token}\` : ''` used for the authorization header across
registrations should be extracted into a single helper function (e.g.,
`composeBearerToken(extra)` or `getAuthorizationHeader(extra: ExtraType)`)
inside ToolRegistryService (or nearby in
src/tools/services/tool-registry.service.ts); implement the helper to return the
exact header string and replace every inline occurrence (the authorization
property used in each tool registration) with a call to that helper so all
registrations (previously at the duplicated lines) use the centralized function.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
TESTING_RESOURCES.mddocker-compose.staging.ymldocs/IMPLEMENTATION-SUMMARY.mdsrc/main.tssrc/mcp/mcp.controller.tssrc/mcp/mcp.module.tssrc/mcp/services/session-manager.service.tssrc/tools/services/tool-registry.service.ts
💤 Files with no reviewable changes (2)
- TESTING_RESOURCES.md
- docs/IMPLEMENTATION-SUMMARY.md
Comment on lines
+82
to
+87
| const decoded = decodeJwt(token); | ||
| if (!decoded || !(decoded.sub as string)) { | ||
| throw new Error('Invalid token payload'); | ||
| } | ||
| const userId = decoded.sub as string; | ||
| this.logger.debug(`Token decoded - UserId: ${userId}`); |
There was a problem hiding this comment.
Do not treat decoded JWT payload as authenticated identity.
Line 82 only decodes token payload and then trusts sub (Line 86). Without cryptographic verification (signature, issuer, audience, expiry), forged tokens can impersonate any user.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/mcp/mcp.controller.ts` around lines 82 - 87, The code currently calls
decodeJwt(token) and trusts decoded.sub as the authenticated identity (decoded,
userId) without cryptographic verification; change this to perform full JWT
verification (signature, expiry/exp, issuer/iss and audience/aud) before using
sub by replacing decodeJwt with a verification call (e.g., verifyJwt or
jwt.verify) or invoking your AuthService.verifyToken, check returned payload is
valid and not expired, and only then extract userId from payload.sub; also
handle verification errors explicitly and reject tokens that fail signature or
claim checks to prevent impersonation.
Comment on lines
+143
to
+145
| if (mcpSessionId && this.transports.has(mcpSessionId)) { | ||
| const transport = this.transports.get(mcpSessionId)!; | ||
| this.logger.debug(`Reusing transport for session: ${mcpSessionId}`); |
There was a problem hiding this comment.
Enforce project binding for every session ID lookup.
At Line 143, Line 239, and Line 284, validation only checks transport existence. A valid session ID from another project can be reused against a different :projectApiKey. Enforce this.sessionProjectMap.get(sessionId) === projectApiKey before allowing POST/GET/DELETE flows.
🔒 Suggested fix
+ private isSessionBoundToProject(sessionId: string, projectApiKey: string): boolean {
+ return this.sessionProjectMap.get(sessionId) === projectApiKey;
+ }
+
// Existing session — reuse transport
- if (mcpSessionId && this.transports.has(mcpSessionId)) {
+ if (
+ mcpSessionId &&
+ this.transports.has(mcpSessionId) &&
+ this.isSessionBoundToProject(mcpSessionId, projectApiKey)
+ ) {
@@
- if (!mcpSessionId || !this.transports.has(mcpSessionId)) {
+ if (
+ !mcpSessionId ||
+ !this.transports.has(mcpSessionId) ||
+ !this.isSessionBoundToProject(mcpSessionId, projectApiKey)
+ ) {
@@
- if (!mcpSessionId || !this.transports.has(mcpSessionId)) {
+ if (
+ !mcpSessionId ||
+ !this.transports.has(mcpSessionId) ||
+ !this.isSessionBoundToProject(mcpSessionId, projectApiKey)
+ ) {Also applies to: 239-247, 284-292
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/mcp/mcp.controller.ts` around lines 143 - 145, When reusing or accepting
a transport/session ID in the MCP controller (places using mcpSessionId,
this.transports, and this.sessionProjectMap — e.g., the reuse check at the start
of the POST handler and the analogous checks in the GET and DELETE flows),
require that this.sessionProjectMap.get(mcpSessionId) === projectApiKey before
allowing reuse; if the mapping is absent or mismatched, treat the session as
invalid and do not return or reuse the transport. Also ensure that when creating
a new session you set this.sessionProjectMap.set(newSessionId, projectApiKey) so
future lookups enforce project binding. This change should be applied to the
checks around mcpSessionId at the three mentioned code sites.
Comment on lines
+148
to
+152
| (req as unknown as IncomingMessage & { auth?: AuthInfo }).auth = authInfo; | ||
| await transport.handleRequest( | ||
| req as unknown as IncomingMessage & { auth?: AuthInfo }, | ||
| res as unknown as ServerResponse, | ||
| req.body, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Run the provided verification script
rg -n --type=ts '\bas\s+unknown\s+as\b' src/mcp/mcp.controller.tsRepository: dadadadas111/iot-cloud-mcp
Length of output: 752
🏁 Script executed:
#!/bin/bash
# Read the controller file with context around each assertion cluster
echo "=== Lines 140-160 (around 148-151) ==="
sed -n '140,160p' src/mcp/mcp.controller.ts
echo -e "\n=== Lines 180-200 (around 190-193) ==="
sed -n '180,200p' src/mcp/mcp.controller.ts
echo -e "\n=== Lines 240-260 (around 252-255) ==="
sed -n '240,260p' src/mcp/mcp.controller.ts
echo -e "\n=== Lines 290-310 (around 298-299) ==="
sed -n '290,310p' src/mcp/mcp.controller.ts
echo -e "\n=== File imports and type definitions (top 60 lines) ==="
head -60 src/mcp/mcp.controller.tsRepository: dadadadas111/iot-cloud-mcp
Length of output: 4886
🏁 Script executed:
#!/bin/bash
# Search for existing module augmentations of IncomingMessage
rg -n 'declare\s+module\s+.*http' --type=ts src/
echo "---"
# Search for interface augmentations of IncomingMessage
rg -n 'interface\s+IncomingMessage' --type=ts src/
echo "---"
# Check if AuthInfo type is already being extended anywhere
rg -n 'AuthInfo' --type=ts src/ | head -20Repository: dadadadas111/iot-cloud-mcp
Length of output: 1034
🏁 Script executed:
#!/bin/bash
# Check if there are any type definition files or interfaces related to StreamableHTTPServerTransport
fd -e d.ts -e ts --path-separator '/' . node_modules/@modelcontextprotocol/sdk/server/ 2>/dev/null | head -10
# Alternative: search for how handleRequest is defined in types
rg -n 'handleRequest' --type=ts src/mcp/ -A 3 | head -30Repository: dadadadas111/iot-cloud-mcp
Length of output: 1522
Remove as unknown as casts by augmenting the IncomingMessage interface type.
The double-casts on lines 148, 150–151, 190, 192–193, 252, 254–255, and 297–299 bypass TypeScript's type system to attach the auth property to incoming requests. Instead of unsafe assertions, use module augmentation to properly extend IncomingMessage:
Create or update a type definitions file with:
declare module 'http' {
interface IncomingMessage {
auth?: AuthInfo;
}
}This eliminates all unsafe casts and aligns with the coding guideline: "Never use as any, @ts-ignore, or type assertions that circumvent TypeScript's type system."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/mcp/mcp.controller.ts` around lines 148 - 152, Add a proper module
augmentation for Node's IncomingMessage so you can attach auth without unsafe
casts: create/update a .d.ts types file that declares module 'http' { interface
IncomingMessage { auth?: AuthInfo } } then remove all occurrences of the
double-cast (e.g., the assignments and calls around req.auth and
transport.handleRequest in mcp.controller.ts) and use req.auth = authInfo and
pass req directly to transport.handleRequest and other call sites (refer to
IncomingMessage, auth, AuthInfo, and transport.handleRequest to locate and
update all instances).
Comment on lines
+163
to
+170
| onsessioninitialized: async (sessionId: string) => { | ||
| // Store transport and project mapping | ||
| this.transports.set(sessionId, transport); | ||
| this.sessionProjectMap.set(sessionId, projectApiKey); | ||
|
|
||
| // Persist session to Redis | ||
| const server = this.serverFactory.createServer(projectApiKey); | ||
| await this.sessionManager.createSession(projectApiKey, userId, server, sessionId); |
There was a problem hiding this comment.
Create only one McpServer per initialized session.
Line 169 creates a server for createSession, while Line 176 creates another server and connects that one to transport. This can desync cached session server state from the active transport server.
🧩 Suggested fix
- const transport = new StreamableHTTPServerTransport({
+ const server = this.serverFactory.createServer(projectApiKey);
+ const transport = new StreamableHTTPServerTransport({
sessionIdGenerator: () => randomUUID(),
onsessioninitialized: async (sessionId: string) => {
@@
- const server = this.serverFactory.createServer(projectApiKey);
await this.sessionManager.createSession(projectApiKey, userId, server, sessionId);
this.logger.log(`Session initialized - SessionId: ${sessionId}, UserId: ${userId}`);
},
});
@@
- const server = this.serverFactory.createServer(projectApiKey);
await server.connect(transport);Also applies to: 176-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/mcp/mcp.controller.ts` around lines 163 - 170, The code creates two
different McpServer instances for the same session (one passed to
sessionManager.createSession and another later used to connect the transport),
which can desync server state; modify onsessioninitialized so you create a
single server via serverFactory.createServer(projectApiKey) and reuse that same
McpServer instance for both sessionManager.createSession(...) and the transport
connection (where transport.connect/transport.setServer or equivalent is
called), removing the second serverFactory.createServer call and any duplicate
server creation; update references to that single variable in
onsessioninitialized and any helper methods so the session store and active
transport share the exact same McpServer object.
Comment on lines
+48
to
50
| async createSession(projectApiKey: string, userId: string, server: McpServer, existingSessionId?: string): Promise<string> { | ||
| const sessionId = existingSessionId || uuidv4(); | ||
| const now = new Date().toISOString(); |
There was a problem hiding this comment.
Validate existingSessionId before persisting.
Line 49 currently accepts any non-undefined value path, so malformed IDs can enter Redis/cache keyspace and make session behavior harder to reason about.
🛡️ Suggested fix
+ private static readonly UUID_V4_REGEX =
+ /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
+
async createSession(projectApiKey: string, userId: string, server: McpServer, existingSessionId?: string): Promise<string> {
- const sessionId = existingSessionId || uuidv4();
+ if (existingSessionId !== undefined && !SessionManagerService.UUID_V4_REGEX.test(existingSessionId)) {
+ throw new Error('Invalid existingSessionId format');
+ }
+ const sessionId = existingSessionId ?? uuidv4();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/mcp/services/session-manager.service.ts` around lines 48 - 50, The
createSession method currently trusts existingSessionId blindly; validate
existingSessionId in createSession (in session-manager.service.ts) before using
it as sessionId: only accept it if it is a non-empty string and passes a UUID-v4
validation (use the uuid library's validate/isUuid helper or a strict regex); if
validation fails, discard it and generate a new id via uuidv4(); optionally emit
a warning log when an invalid existingSessionId is provided to aid debugging.
This was referenced Mar 9, 2026
Merged
Open
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
1 participant
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 by CodeRabbit
New Features
Chores