session routes api created#7
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds session management and Stream video integration: new Mongoose Session model, six session API endpoints and controllers (create, list active, list recent, get, join, end), Stream SDK client export and dependency, route mounted at /api/sessions, and a non-functional comment added to inngest syncUser. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as rgba(50,115,220,0.5)
participant API as rgba(80,200,120,0.5)
participant DB as rgba(200,120,80,0.5)
participant Stream as rgba(180,60,200,0.5)
participant Chat as rgba(200,180,60,0.5)
Client->>API: POST /api/sessions (createSession)
API->>DB: create Session document (host, problem, difficulty)
API->>Stream: streamClient.createCall(...) -> returns callId
Stream-->>API: callId
API->>Chat: chatClient.createChannel(...) -> returns channelId
Chat-->>API: channelId
API->>DB: update Session with callId & channelId
API-->>Client: 201 Created (Session with callId)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/package.json`:
- Line 16: The dependency `@stream-io/node-sdk` is pinned at ^0.7.12; update
package.json to the newer release (e.g., ^0.7.44) for bug fixes and
improvements, then run your package manager (npm/yarn/pnpm) to refresh
node_modules and update the lockfile; afterwards run the test suite and any
integration checks that exercise code paths using the Stream client (look for
usage sites of `@stream-io/node-sdk` in the codebase) to verify there are no
breaking changes and revert or pin if compatibility issues occur.
In `@backend/src/controllers/sessionController.js`:
- Around line 138-147: The current flow marks session.status = "completed" and
calls await session.save() before deleting Stream resources, which can leave
orphaned resources if streamClient.video.call(...).delete() or
chatClient.channel(...).delete() fail; change the flow to delete external
resources first (use streamClient.video.call("default", session.callId).delete({
hard: true }) and chatClient.channel("messaging", session.callId).delete())
inside a try block, and only on success set session.status = "completed" and
await session.save(); if deletion fails, catch the error, log it, and avoid
updating the session (or schedule a retry/cleanup job), or alternatively perform
session.save() and on failure enqueue the callId for a cleanup worker—use the
function/method names call.delete, channel.delete, session.save, session.status
to locate and implement the fix.
- Line 66: Fix the typo in the inline comment in sessionController.js: replace
the misspelled comment "//where user is eigther host or pwrticipant" with the
corrected text "//where user is either host or participant" so the comment for
the session/user handling logic is accurate.
- Around line 103-109: The current join flow uses Session.findById and a
separate check on session.participant which is a race condition; replace the
read-then-write with an atomic update using Session.findOneAndUpdate (or
findOneAndUpdate with { _id: id, participant: { $exists: false } } or
participant: null) to set participant to userId only when no participant exists,
check the returned document to determine success, and respond with 404/400
appropriately; update the logic around Session.findById/session.participant and
session.save to use the atomic findOneAndUpdate call instead.
In `@backend/src/lib/inngest.js`:
- Around line 30-31: Fix the typo in the inline TODO comment inside
backend/src/lib/inngest.js: change "Send a Welcome email ro the user in future"
to "Send a welcome email to the user in the future" (or similar correct wording)
so the TODO reads clearly; update the comment near the existing TODO line in the
file (no code changes other than the comment).
In `@backend/src/models/Session.js`:
- Around line 5-8: Schema field name in Session.js is misspelled as "problemn"
causing mismatch with sessionController.js which expects "problem"; rename the
schema property from problemn to problem in the Session model so that the
Mongoose schema key matches the request body and creation logic in
sessionController.js and persists the value correctly.
- Around line 19-23: The schema field name and controller usage are mismatched:
the Session schema defines participant (singular) while sessionController
functions getMyRecentSessions and getSessionById query/populate participants
(plural); fix by either (A) if multiple participants are intended,
rename/replace the schema field participant with participants: [{ type:
mongoose.Schema.Types.ObjectId, ref: "User" }] (default to [] or null as
appropriate) so populate and queries in getMyRecentSessions and getSessionById
work, or (B) if only a single participant is intended, update sessionController
(in getMyRecentSessions and getSessionById) to query and populate participant
(singular) instead of participants; ensure the ref stays "User" and adjust any
query operators (e.g., $in vs equality) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 91182a7e-4ef1-456b-8901-c268bba46e5a
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
backend/package.jsonbackend/src/controllers/sessionController.jsbackend/src/lib/inngest.jsbackend/src/lib/stream.jsbackend/src/models/Session.jsbackend/src/routes/sessionRoute.jsbackend/src/server.js
| "license": "ISC", | ||
| "dependencies": { | ||
| "@clerk/express": "^1.7.41", | ||
| "@stream-io/node-sdk": "^0.7.12", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest version and security advisories for `@stream-io/node-sdk`
# Get latest version from npm
echo "Latest version on npm:"
curl -s https://registry.npmjs.org/@stream-io/node-sdk/latest | jq '.version'
# Check for security advisories
echo -e "\nSecurity advisories:"
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@stream-io/node-sdk") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'Repository: realACO/CodeMeet
Length of output: 274
🏁 Script executed:
# Check npm registry for vulnerability info
curl -s "https://registry.npmjs.org/@stream-io/node-sdk" | jq '.time | keys | .[-5:]'
# Check if there's a security advisory file or known issues
curl -s "https://api.github.com/repos/GetStream/stream-node-sdk/releases" | jq '.[] | select(.tag_name | contains("0.7")) | {tag_name, published_at}' | head -30Repository: realACO/CodeMeet
Length of output: 208
🌐 Web query:
@stream-io/node-sdk security vulnerabilities 0.7.12 changelog
💡 Result:
@stream-io/node-sdk v0.7.12 changelog (stream-node repo)
Release 0.7.12 (2025-10-16) includes one bug fix:
- “cutoff time on req should be string” (
#189) (commit55ccb83) [1]
Known / reported security vulnerabilities for v0.7.12
- No known vulnerabilities were found/reported for
@stream-io/node-sdk(including v0.7.12) by Aikido Intel at the time of writing. [2]
Sources: [1] https://github.com/GetStream/stream-node/releases/tag/v0.7.12, [2] https://intel.aikido.dev/packages/npm/%40stream-io/node-sdk
Consider upgrading @stream-io/node-sdk to the latest version.
The dependency is pinned to version ^0.7.12. While this version is stable with no known security vulnerabilities, the latest available version is 0.7.44—32 minor versions ahead. Consider upgrading to benefit from recent bug fixes and improvements unless there's a specific compatibility requirement for 0.7.12.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/package.json` at line 16, The dependency `@stream-io/node-sdk` is
pinned at ^0.7.12; update package.json to the newer release (e.g., ^0.7.44) for
bug fixes and improvements, then run your package manager (npm/yarn/pnpm) to
refresh node_modules and update the lockfile; afterwards run the test suite and
any integration checks that exercise code paths using the Stream client (look
for usage sites of `@stream-io/node-sdk` in the codebase) to verify there are no
breaking changes and revert or pin if compatibility issues occur.
| const session = await Session.findById(id); | ||
| if (!session) return res.status(404).json({ msg: "Session not found" }); | ||
| //check if session is already full | ||
| if (session.participant) | ||
| return res.status(400).json({ msg: "Session is already full" }); | ||
| session.participant = userId; | ||
| await session.save(); |
There was a problem hiding this comment.
Race condition when joining session.
The check-then-set on session.participant is not atomic. Two concurrent requests could both pass the if (session.participant) check before either saves, resulting in the second participant overwriting the first.
Consider using findOneAndUpdate with the condition in the query to make it atomic:
🔒 Proposed atomic update
- const session = await Session.findById(id);
- if (!session) return res.status(404).json({ msg: "Session not found" });
- //check if session is already full
- if (session.participant)
- return res.status(400).json({ msg: "Session is already full" });
- session.participant = userId;
- await session.save();
+ const session = await Session.findOneAndUpdate(
+ { _id: id, participant: null },
+ { participant: userId },
+ { new: true }
+ );
+ if (!session) {
+ const exists = await Session.findById(id);
+ if (!exists) return res.status(404).json({ msg: "Session not found" });
+ return res.status(400).json({ msg: "Session is already full" });
+ }📝 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.
| const session = await Session.findById(id); | |
| if (!session) return res.status(404).json({ msg: "Session not found" }); | |
| //check if session is already full | |
| if (session.participant) | |
| return res.status(400).json({ msg: "Session is already full" }); | |
| session.participant = userId; | |
| await session.save(); | |
| const session = await Session.findOneAndUpdate( | |
| { _id: id, participant: null }, | |
| { participant: userId }, | |
| { new: true } | |
| ); | |
| if (!session) { | |
| const exists = await Session.findById(id); | |
| if (!exists) return res.status(404).json({ msg: "Session not found" }); | |
| return res.status(400).json({ msg: "Session is already full" }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/controllers/sessionController.js` around lines 103 - 109, The
current join flow uses Session.findById and a separate check on
session.participant which is a race condition; replace the read-then-write with
an atomic update using Session.findOneAndUpdate (or findOneAndUpdate with { _id:
id, participant: { $exists: false } } or participant: null) to set participant
to userId only when no participant exists, check the returned document to
determine success, and respond with 404/400 appropriately; update the logic
around Session.findById/session.participant and session.save to use the atomic
findOneAndUpdate call instead.
| session.status = "completed"; | ||
| await session.save(); | ||
|
|
||
| //delete stream video call | ||
| const call = streamClient.video.call("default", session.callId); | ||
| await call.delete({ hard: true }); | ||
|
|
||
| //delete stream chat channel | ||
| const channel = chatClient.channel("messaging", session.callId); | ||
| await channel.delete(); |
There was a problem hiding this comment.
Partial failure risk when ending session.
If session.save() succeeds but call.delete() or channel.delete() fails, the session is marked completed but Stream resources remain orphaned. Consider:
- Deleting external resources first, then updating DB
- Or implementing a cleanup mechanism for orphaned resources
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/controllers/sessionController.js` around lines 138 - 147, The
current flow marks session.status = "completed" and calls await session.save()
before deleting Stream resources, which can leave orphaned resources if
streamClient.video.call(...).delete() or chatClient.channel(...).delete() fail;
change the flow to delete external resources first (use
streamClient.video.call("default", session.callId).delete({ hard: true }) and
chatClient.channel("messaging", session.callId).delete()) inside a try block,
and only on success set session.status = "completed" and await session.save();
if deletion fails, catch the error, log it, and avoid updating the session (or
schedule a retry/cleanup job), or alternatively perform session.save() and on
failure enqueue the callId for a cleanup worker—use the function/method names
call.delete, channel.delete, session.save, session.status to locate and
implement the fix.
|
|
||
| //Send a Welcome email ro the user in future |
There was a problem hiding this comment.
Typo in TODO comment.
The comment has a typo: "ro" should be "to".
- //Send a Welcome email ro the user in future
+ //Send a Welcome email to the user in future📝 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.
| //Send a Welcome email ro the user in future | |
| //Send a Welcome email to the user in future |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/lib/inngest.js` around lines 30 - 31, Fix the typo in the inline
TODO comment inside backend/src/lib/inngest.js: change "Send a Welcome email ro
the user in future" to "Send a welcome email to the user in the future" (or
similar correct wording) so the TODO reads clearly; update the comment near the
existing TODO line in the file (no code changes other than the comment).
| participant: { | ||
| type: mongoose.Schema.Types.ObjectId, | ||
| ref: "User", | ||
| default: null, | ||
| }, |
There was a problem hiding this comment.
Field name mismatch: participant vs participants.
The schema defines participant (singular), but sessionController.js queries and populates participants (plural) at lines 69 and 86. This will cause:
getMyRecentSessionsto never find sessions where the user was a participantgetSessionByIdpopulate onparticipantsto return nothing
Either rename the schema field to participants (if supporting multiple participants) or fix the controller to use participant.
🐛 Option 1: Fix schema to match controller (if multiple participants intended)
- participant: {
+ participants: {
type: mongoose.Schema.Types.ObjectId,
ref: "User",
default: null,
},🐛 Option 2: Fix controller to match schema (if single participant intended)
In sessionController.js, update the queries:
// Line 69 in getMyRecentSessions
- $or: [{ host: userId }, { participants: userId }],
+ $or: [{ host: userId }, { participant: userId }],
// Line 86 in getSessionById
- .populate("participants", "name email profileImage clerkId");
+ .populate("participant", "name email profileImage clerkId");📝 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.
| participant: { | |
| type: mongoose.Schema.Types.ObjectId, | |
| ref: "User", | |
| default: null, | |
| }, | |
| participants: { | |
| type: mongoose.Schema.Types.ObjectId, | |
| ref: "User", | |
| default: null, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/models/Session.js` around lines 19 - 23, The schema field name
and controller usage are mismatched: the Session schema defines participant
(singular) while sessionController functions getMyRecentSessions and
getSessionById query/populate participants (plural); fix by either (A) if
multiple participants are intended, rename/replace the schema field participant
with participants: [{ type: mongoose.Schema.Types.ObjectId, ref: "User" }]
(default to [] or null as appropriate) so populate and queries in
getMyRecentSessions and getSessionById work, or (B) if only a single participant
is intended, update sessionController (in getMyRecentSessions and
getSessionById) to query and populate participant (singular) instead of
participants; ensure the ref stays "User" and adjust any query operators (e.g.,
$in vs equality) accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/src/controllers/sessionController.js (1)
67-72: Consider adding.populate()for consistency.Unlike
getActiveSessionsandgetSessionById, this endpoint doesn't populatehostandparticipantreferences. If the client needs user details for recent sessions, consider adding:const sessions = await Session.find({ status: "completed", $or: [{ host: userId }, { participant: userId }], }) + .populate("host", "name profileImage email clerkId") + .populate("participant", "name profileImage email clerkId") .sort({ createdAt: -1 }) .limit(20);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/controllers/sessionController.js` around lines 67 - 72, The recent-sessions query using Session.find(...) that constructs the sessions constant should be updated to populate the host and participant references just like getActiveSessions and getSessionById do; modify the Session.find(...).sort(...).limit(20) chain to include .populate('host').populate('participant') (or the same populate paths used in getActiveSessions/getSessionById) so the returned sessions include user details for clients expecting populated host/participant objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/controllers/sessionController.js`:
- Around line 103-108: Add two validations in the join flow after fetching the
Session via Session.findById: ensure the request userId is not the session host
(e.g., compare userId to session.host or session.hostId) and return a 400/403
with a clear message if they try to join their own session; also check
session.status (or similar field) and reject joins when status === "completed"
(or not "active") with a 400 and message indicating the session is not joinable.
Keep the existing check for session.participant and only assign
session.participant = userId after both new validations pass.
- Around line 19-42: The code creates a Session via Session.create and then
calls streamClient.video.call(...).getOrCreate and channel.create without
rollback, risking orphaned resources if later calls fail; fix by wrapping the
multi-resource creation in a DB transaction (use Mongoose transaction around
Session.create) or at minimum add a try/catch that performs compensating
deletes: if streamClient.video.call(...).getOrCreate succeeds but channel.create
fails, call the Stream API to delete the created call and delete the Session
document (Session.deleteOne or session.remove), and likewise if stream creation
fails after Session.create delete the Session; ensure cleanup paths reference
the created session._id, callId, and channel to reliably remove partial
resources.
---
Nitpick comments:
In `@backend/src/controllers/sessionController.js`:
- Around line 67-72: The recent-sessions query using Session.find(...) that
constructs the sessions constant should be updated to populate the host and
participant references just like getActiveSessions and getSessionById do; modify
the Session.find(...).sort(...).limit(20) chain to include
.populate('host').populate('participant') (or the same populate paths used in
getActiveSessions/getSessionById) so the returned sessions include user details
for clients expecting populated host/participant objects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7352925e-2467-43d1-9813-f812a04e5dd4
📒 Files selected for processing (1)
backend/src/controllers/sessionController.js
| const session = await Session.create({ | ||
| problem, | ||
| difficulty, | ||
| host: userId, | ||
| callId, | ||
| }); | ||
|
|
||
| //create stream video call | ||
| await streamClient.video.call("default", callId).getOrCreate({ | ||
| data: { | ||
| created_by_id: clerkId, | ||
| custom: { problem, difficulty, sessionId: session._id.toString() }, | ||
| }, | ||
| }); | ||
|
|
||
| //chat messaging | ||
| const channel = chatClient.channel("messaging", callId, { | ||
| name: `${problem} Session`, | ||
| created_by_id: clerkId, | ||
| members: [clerkId], | ||
| }); | ||
|
|
||
| await channel.create(); | ||
|
|
There was a problem hiding this comment.
Partial failure risk when creating session.
If streamClient.video.call(...).getOrCreate() fails after Session.create() succeeds, an orphaned DB record remains. Similarly, if channel.create() fails, both the DB record and Stream video call are orphaned with no cleanup.
Consider wrapping in a transaction or implementing cleanup on failure:
🛡️ Proposed cleanup on failure
//create session in db
const session = await Session.create({
problem,
difficulty,
host: userId,
callId,
});
- //create stream video call
- await streamClient.video.call("default", callId).getOrCreate({
- data: {
- created_by_id: clerkId,
- custom: { problem, difficulty, sessionId: session._id.toString() },
- },
- });
-
- //chat messaging
- const channel = chatClient.channel("messaging", callId, {
- name: `${problem} Session`,
- created_by_id: clerkId,
- members: [clerkId],
- });
-
- await channel.create();
+ try {
+ //create stream video call
+ await streamClient.video.call("default", callId).getOrCreate({
+ data: {
+ created_by_id: clerkId,
+ custom: { problem, difficulty, sessionId: session._id.toString() },
+ },
+ });
+
+ //chat messaging
+ const channel = chatClient.channel("messaging", callId, {
+ name: `${problem} Session`,
+ created_by_id: clerkId,
+ members: [clerkId],
+ });
+
+ await channel.create();
+ } catch (externalError) {
+ // Cleanup: remove the orphaned session
+ await Session.findByIdAndDelete(session._id);
+ throw externalError;
+ }📝 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.
| const session = await Session.create({ | |
| problem, | |
| difficulty, | |
| host: userId, | |
| callId, | |
| }); | |
| //create stream video call | |
| await streamClient.video.call("default", callId).getOrCreate({ | |
| data: { | |
| created_by_id: clerkId, | |
| custom: { problem, difficulty, sessionId: session._id.toString() }, | |
| }, | |
| }); | |
| //chat messaging | |
| const channel = chatClient.channel("messaging", callId, { | |
| name: `${problem} Session`, | |
| created_by_id: clerkId, | |
| members: [clerkId], | |
| }); | |
| await channel.create(); | |
| const session = await Session.create({ | |
| problem, | |
| difficulty, | |
| host: userId, | |
| callId, | |
| }); | |
| try { | |
| //create stream video call | |
| await streamClient.video.call("default", callId).getOrCreate({ | |
| data: { | |
| created_by_id: clerkId, | |
| custom: { problem, difficulty, sessionId: session._id.toString() }, | |
| }, | |
| }); | |
| //chat messaging | |
| const channel = chatClient.channel("messaging", callId, { | |
| name: `${problem} Session`, | |
| created_by_id: clerkId, | |
| members: [clerkId], | |
| }); | |
| await channel.create(); | |
| } catch (externalError) { | |
| // Cleanup: remove the orphaned session | |
| await Session.findByIdAndDelete(session._id); | |
| throw externalError; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/controllers/sessionController.js` around lines 19 - 42, The code
creates a Session via Session.create and then calls
streamClient.video.call(...).getOrCreate and channel.create without rollback,
risking orphaned resources if later calls fail; fix by wrapping the
multi-resource creation in a DB transaction (use Mongoose transaction around
Session.create) or at minimum add a try/catch that performs compensating
deletes: if streamClient.video.call(...).getOrCreate succeeds but channel.create
fails, call the Stream API to delete the created call and delete the Session
document (Session.deleteOne or session.remove), and likewise if stream creation
fails after Session.create delete the Session; ensure cleanup paths reference
the created session._id, callId, and channel to reliably remove partial
resources.
| const session = await Session.findById(id); | ||
| if (!session) return res.status(404).json({ msg: "Session not found" }); | ||
| //check if session is already full | ||
| if (session.participant) | ||
| return res.status(400).json({ msg: "Session is already full" }); | ||
| session.participant = userId; |
There was a problem hiding this comment.
Missing validation: user can join own session or join non-active sessions.
Two validation gaps:
- A host can join their own session as a participant
- Users can join sessions that are already
completed
🛡️ Proposed validation
const session = await Session.findById(id);
if (!session) return res.status(404).json({ msg: "Session not found" });
+
+ //check if session is active
+ if (session.status !== "active")
+ return res.status(400).json({ msg: "Session is not active" });
+
+ //prevent host from joining as participant
+ if (session.host.toString() === userId.toString())
+ return res.status(400).json({ msg: "Host cannot join as participant" });
+
//check if session is already full
if (session.participant)
return res.status(400).json({ msg: "Session is already full" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/controllers/sessionController.js` around lines 103 - 108, Add two
validations in the join flow after fetching the Session via Session.findById:
ensure the request userId is not the session host (e.g., compare userId to
session.host or session.hostId) and return a 400/403 with a clear message if
they try to join their own session; also check session.status (or similar field)
and reject joins when status === "completed" (or not "active") with a 400 and
message indicating the session is not joinable. Keep the existing check for
session.participant and only assign session.participant = userId after both new
validations pass.
Summary by CodeRabbit
New Features
Chores