"Last active at" column on users and sessions#1081
Conversation
Older cmux preview screenshots (latest comment is below)Preview Screenshots⏳ Preview screenshots are being captured... Workspace and dev browser links will appear here once the preview environment is ready. Generated by cmux preview system |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TokenSvc as TokenService
participant TenancyDB as Tenancy-Prisma
participant GlobalDB as Global-Prisma
participant Events as SessionActivityLogger
Client->>TokenSvc: Refresh access using refresh token
TokenSvc->>TenancyDB: Validate refresh token & load session
TenancyDB-->>TokenSvc: Session + refresh token record
TokenSvc->>TenancyDB: Update ProjectUser.lastActiveAt
TenancyDB-->>TokenSvc: OK
TokenSvc->>TenancyDB: Update ProjectUserRefreshToken.lastActiveAt (+ IP info)
TenancyDB-->>TokenSvc: OK
TokenSvc->>Events: Log SessionActivity event
Events-->>TokenSvc: Logged
TokenSvc->>GlobalDB: Read/write any global state (sessions listing, etc.)
TokenSvc-->>Client: Return new access token
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
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 |
Greptile SummaryThis PR adds Key Changes
Implementation DetailsThe migration uses a batched approach (1000 rows at a time) with conditional repetition to safely handle large tables. It backfills existing data from the event log by joining on The tracking happens in Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant API
participant TokenHandler
participant Database
participant EventLogger
Client->>API: Authentication Request
API->>TokenHandler: Validate refresh token
TokenHandler->>Database: Fetch user
Database-->>TokenHandler: User data
par Parallel Updates
TokenHandler->>Database: Update user lastActiveAt
TokenHandler->>Database: Update session lastActiveAt
end
TokenHandler->>EventLogger: Log session activity
EventLogger->>Database: Store activity event
TokenHandler-->>API: New access token
API-->>Client: Auth response
Client->>API: Metrics request
API->>Database: Query by lastActiveAt
Database-->>API: Active users
API-->>Client: Metrics response
|
Greptile's behavior is changing!From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
There was a problem hiding this comment.
Pull request overview
This PR adds lastActiveAt columns to the ProjectUser and ProjectUserRefreshToken database tables to track when users and sessions were last active. Instead of querying the Event table on every request, the system now maintains dedicated timestamp columns that are updated whenever a refresh token is used.
Key changes:
- Database columns are updated in real-time when access tokens are generated from refresh tokens
- Removed complex event-based queries in favor of direct column reads
- Updated snapshot tests to reflect the new named snapshot format
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/backend/prisma/schema.prisma | Added lastActiveAt column with default value to ProjectUser and ProjectUserRefreshToken models |
| apps/backend/prisma/migrations/20260101000000_add_last_active_at_columns/migration.sql | Migration that adds columns, backfills data from Event table, then sets columns to NOT NULL |
| apps/backend/src/lib/tokens.tsx | Updates lastActiveAt on both user and session when generating access tokens from refresh tokens |
| apps/backend/src/app/api/latest/users/crud.tsx | Removed complex event-based getUsersLastActiveAtMillis functions; now reads lastActiveAt directly from the database model |
| apps/backend/src/app/api/latest/team-member-profiles/crud.tsx | Updated to use simplified userPrismaToCrud without passing lastActiveAtMillis parameter |
| apps/backend/src/app/api/latest/internal/metrics/route.tsx | Simplified recently active users query to use direct orderBy lastActiveAt instead of complex event-based CTE |
| apps/backend/src/app/api/latest/auth/sessions/crud.tsx | Removed event-based session activity queries; now reads lastActiveAt directly from ProjectUserRefreshToken |
| apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts | Changed snapshot matcher to use named snapshot for better maintainability |
| apps/e2e/tests/backend/endpoints/api/v1/snapshots/internal-metrics.test.ts.snap | Updated snapshot with new name and removed duplicate user entries from the test data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/backend/prisma/schema.prisma (1)
173-175: Consider adding an index forlastActiveAtonProjectUser.The metrics route queries
ProjectUserwithorderBy: { lastActiveAt: 'desc' }. Without an index, this could become slow as the user base grows. The existing indices follow a pattern for sort fields (e.g.,createdAt,displayName).🔎 Suggested index addition
// indices for sorting and filtering @@index([tenancyId, displayName(sort: Asc)], name: "ProjectUser_displayName_asc") @@index([tenancyId, displayName(sort: Desc)], name: "ProjectUser_displayName_desc") @@index([tenancyId, createdAt(sort: Asc)], name: "ProjectUser_createdAt_asc") @@index([tenancyId, createdAt(sort: Desc)], name: "ProjectUser_createdAt_desc") + @@index([tenancyId, lastActiveAt(sort: Desc)], name: "ProjectUser_lastActiveAt_desc")apps/backend/src/lib/tokens.tsx (1)
189-199: Consider iflogEventneeds to be awaited.The
logEventcall is awaited, adding latency to the token refresh flow. If the event is used only for analytics/metrics (as the comment suggests), consider making it non-blocking to improve token refresh performance.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/e2e/tests/backend/endpoints/api/v1/__snapshots__/internal-metrics.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
apps/backend/prisma/migrations/20260101000000_add_last_active_at_columns/migration.sqlapps/backend/prisma/schema.prismaapps/backend/src/app/api/latest/auth/sessions/crud.tsxapps/backend/src/app/api/latest/internal/metrics/route.tsxapps/backend/src/app/api/latest/team-member-profiles/crud.tsxapps/backend/src/app/api/latest/users/crud.tsxapps/backend/src/lib/tokens.tsxapps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Always add new E2E tests when changing the API or SDK interface
For blocking alerts and errors, never use toast; use alerts instead as they are less easily missed by the user
NEVER try-catch-all, NEVER void a promise, and NEVER .catch(console.error); use loading indicators and async callbacks instead, or use runAsynchronously/runAsynchronouslyWithAlert for error handling
Use ES6 maps instead of records wherever you can
Files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.tsapps/backend/src/app/api/latest/team-member-profiles/crud.tsxapps/backend/src/app/api/latest/internal/metrics/route.tsxapps/backend/src/lib/tokens.tsxapps/backend/src/app/api/latest/auth/sessions/crud.tsxapps/backend/src/app/api/latest/users/crud.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer .toMatchInlineSnapshot over other selectors in tests when possible; check snapshot-serializer.ts for formatting details
Files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts
**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,css}: Keep hover/click transitions snappy and fast; avoid fade-in delays on hover. Apply transitions after action completion instead, like smooth fade-out when hover ends
Use hover-exit transitions instead of hover-enter transitions; for example, use 'transition-colors hover:transition-none' instead of fade-in on hover
Files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.tsapps/backend/src/app/api/latest/team-member-profiles/crud.tsxapps/backend/src/app/api/latest/internal/metrics/route.tsxapps/backend/src/lib/tokens.tsxapps/backend/src/app/api/latest/auth/sessions/crud.tsxapps/backend/src/app/api/latest/users/crud.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
NEVER use Next.js dynamic functions if you can avoid them; prefer using client components and hooks like usePathname instead of await params to keep pages static
Files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.tsapps/backend/src/app/api/latest/team-member-profiles/crud.tsxapps/backend/src/app/api/latest/internal/metrics/route.tsxapps/backend/src/lib/tokens.tsxapps/backend/src/app/api/latest/auth/sessions/crud.tsxapps/backend/src/app/api/latest/users/crud.tsx
{.env*,**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (AGENTS.md)
Prefix environment variables with STACK_ (or NEXT_PUBLIC_STACK_ if public) so changes are picked up by Turborepo and improves readability
Files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.tsapps/backend/src/app/api/latest/team-member-profiles/crud.tsxapps/backend/src/app/api/latest/internal/metrics/route.tsxapps/backend/src/lib/tokens.tsxapps/backend/src/app/api/latest/auth/sessions/crud.tsxapps/backend/src/app/api/latest/users/crud.tsx
apps/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always add new E2E tests when changing API or SDK interface; err on the side of creating too many tests due to the critical nature of the industry
Files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts
apps/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Check existing apps for inspiration when implementing new apps or pages; update apps-frontend.tsx and apps-config.ts to add new apps
Files:
apps/backend/src/app/api/latest/team-member-profiles/crud.tsxapps/backend/src/app/api/latest/internal/metrics/route.tsxapps/backend/src/lib/tokens.tsxapps/backend/src/app/api/latest/auth/sessions/crud.tsxapps/backend/src/app/api/latest/users/crud.tsx
🧠 Learnings (4)
📚 Learning: 2025-12-04T18:03:49.984Z
Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T18:03:49.984Z
Learning: Applies to **/*.test.{ts,tsx} : Prefer .toMatchInlineSnapshot over other selectors in tests when possible; check snapshot-serializer.ts for formatting details
Applied to files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts
📚 Learning: 2025-12-04T18:03:49.984Z
Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T18:03:49.984Z
Learning: Applies to apps/e2e/**/*.{ts,tsx} : Always add new E2E tests when changing API or SDK interface; err on the side of creating too many tests due to the critical nature of the industry
Applied to files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts
📚 Learning: 2025-12-04T18:03:49.984Z
Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T18:03:49.984Z
Learning: Applies to **/*.{ts,tsx} : Always add new E2E tests when changing the API or SDK interface
Applied to files:
apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts
📚 Learning: 2025-12-17T01:23:15.483Z
Learnt from: N2D4
Repo: stack-auth/stack-auth PR: 1032
File: apps/backend/src/app/api/latest/analytics/query/route.tsx:20-20
Timestamp: 2025-12-17T01:23:15.483Z
Learning: In apps/backend/src/app/api/latest/analytics/query/route.tsx, the include_all_branches field controls which ClickHouse user type is used: when true, use "admin" authType for access to all branches; when false (default), use "external" authType for limited/filtered branch access.
Applied to files:
apps/backend/src/app/api/latest/internal/metrics/route.tsx
🧬 Code graph analysis (3)
apps/backend/src/app/api/latest/team-member-profiles/crud.tsx (1)
apps/backend/src/app/api/latest/users/crud.tsx (2)
userFullInclude(27-46)userPrismaToCrud(86-128)
apps/backend/src/app/api/latest/internal/metrics/route.tsx (1)
apps/backend/src/app/api/latest/users/crud.tsx (2)
userFullInclude(27-46)userPrismaToCrud(86-128)
apps/backend/src/lib/tokens.tsx (1)
apps/backend/src/prisma-client.tsx (1)
getPrismaClientForTenancy(67-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: Vercel Agent Review
- GitHub Check: Cursor Bugbot
- GitHub Check: build (22.x)
- GitHub Check: E2E Tests (Node 22.x, Freestyle mock)
- GitHub Check: restart-dev-and-test
- GitHub Check: E2E Tests (Node 22.x, Freestyle prod)
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: all-good
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: setup-tests
- GitHub Check: setup-tests-with-custom-base-port
- GitHub Check: build (22.x)
- GitHub Check: lint_and_build (latest)
- GitHub Check: docker
🔇 Additional comments (14)
apps/backend/prisma/schema.prisma (1)
434-436: LGTM!The
lastActiveAtfield addition toProjectUserRefreshTokenis correctly configured with@default(now()), matching the migration'sDEFAULT NOW()constraint.apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts (1)
68-68: Named snapshot is acceptable, though inline snapshots are preferred per guidelines.The named snapshot
metrics_result_with_usersimproves test clarity. If the response structure is manageable, consider usingtoMatchInlineSnapshot()instead per coding guidelines. However, if the metrics response is too large/dynamic, the named snapshot approach is reasonable.apps/backend/src/app/api/latest/internal/metrics/route.tsx (1)
195-210: LGTM!The refactored
loadRecentlyActiveUserscorrectly uses the newlastActiveAtfield for ordering, simplifying the previous event-based approach. The filtering logic and mapping are consistent with other user queries in the codebase.apps/backend/src/app/api/latest/auth/sessions/crud.tsx (1)
20-25: LGTM! Good security addition.The client access guard correctly ensures that clients can only list sessions for their own user, preventing unauthorized access to other users' session data.
apps/backend/prisma/migrations/20260101000000_add_last_active_at_columns/migration.sql (2)
77-83: LGTM!The final step correctly enforces
NOT NULLand setsDEFAULT NOW()after the backfill completes, ensuring data integrity for existing and future rows.
13-41: Well-structured batched migration with appropriate fallback and performance considerations addressed.The backfill logic correctly:
- Limits batches to 1000 rows for manageable execution
- Falls back to
createdAtwhen no activity events exist- Uses the repeat sentinel for iterative processing
The JSON path queries on
Event.dataare well-supported by the existing GIN index withJsonbPathOpsdefined in the schema, so performance should be appropriate for large Event tables.apps/backend/src/app/api/latest/team-member-profiles/crud.tsx (2)
15-23: LGTM! Clean simplification.The
prismaToCrudfunction is correctly simplified to derivelastActiveAtMillisinternally viauserPrismaToCrud, which now handles the conversion fromprisma.projectUser.lastActiveAt. This aligns with the broader refactor to use the persistedlastActiveAtfield.
82-84: LGTM!All call sites correctly use the simplified single-parameter
prismaToCrudfunction.apps/backend/src/app/api/latest/users/crud.tsx (6)
6-6: LGTM: Import cleanup is consistent with refactoring.The removal of
getPrismaSchemaForTenancyimport and simplification to just theTenancytype aligns well with eliminating the external lastActiveAt computation logic.Also applies to: 9-9
452-452: LGTM: Call site correctly updated.The call to
userPrismaToCrudhas been correctly updated to pass only the Prisma payload, consistent with the new function signature.
601-601: LGTM: User creation correctly returns mapped result.The call to
userPrismaToCrudis correctly updated. However, ensure that newly created users have theirlastActiveAtfield set (either by database default or application logic) to avoid null reference errors.
945-945: LGTM: Update operation correctly returns mapped result.The call to
userPrismaToCrudis correctly updated and consistent with the new signature.
86-89: E2E tests already comprehensively coverlast_active_at_millisin user CRUD operations. The field appears in test snapshots across all list, read, create, and update operations inapps/e2e/tests/backend/endpoints/api/v1/users.test.ts, and is intentionally stripped from snapshots (persnapshot-serializer.tsline 44) along with other timestamp fields likesigned_up_at_millisandcreated_at_millis. No new tests are needed.Likely an incorrect or invalid review comment.
86-89: No action needed—lastActiveAtfield is non-nullable.The
lastActiveAtfield in the Prisma schema is defined asDateTime @default(now())without a nullable marker, ensuring it is always populated at the database level. TypeScript's type system reflects this asDate(notDate | null), making the.getTime()call safe. The suggested fallback tocreatedAtis unnecessary.Likely an incorrect or invalid review comment.
Older cmux preview screenshots (latest comment is below)Preview ScreenshotsOpen Workspace (1 hr expiry) · Open Dev Browser (1 hr expiry) · Open Diff Heatmap Screenshot capture was skipped.
Generated by cmux preview system |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/backend/prisma/schema.prisma (2)
173-175: Add an index onlastActiveAtfor query performance.The new
lastActiveAtfield will be used for sorting and filtering (metrics endpoint, session listings), but there's no index defined. Similar tocreatedAt(lines 207-208), consider adding indices to support efficient queries.📊 Suggested index definitions
Add these indices after line 208:
@@index([tenancyId, createdAt(sort: Desc)], name: "ProjectUser_createdAt_desc") + @@index([tenancyId, lastActiveAt(sort: Asc)], name: "ProjectUser_lastActiveAt_asc") + @@index([tenancyId, lastActiveAt(sort: Desc)], name: "ProjectUser_lastActiveAt_desc") }
434-436: Consider adding an index onlastActiveAtfor session queries.Session listings use
projectUserRefreshToken.lastActiveAtfor ordering. Depending on the volume of refresh tokens per tenancy, an index could improve query performance.📊 Suggested index definition
Add this index after line 442:
@@id([tenancyId, id]) + @@index([tenancyId, lastActiveAt(sort: Desc)], name: "ProjectUserRefreshToken_lastActiveAt_desc") }apps/backend/src/app/api/latest/users/crud.tsx (2)
86-89: Consider defensive coding forlastActiveAtaccess.While the schema enforces
lastActiveAtas non-nullable with a default, adding an explicit null check with a clear error message aligns with the coding guideline to "code defensively; prefer?? throwErr(...)over non-null assertions."🛡️ Defensive coding suggestion
export const userPrismaToCrud = ( prisma: Prisma.ProjectUserGetPayload<{ include: typeof userFullInclude }>, ): UsersCrud["Admin"]["Read"] => { - const lastActiveAtMillis = prisma.lastActiveAt.getTime(); + const lastActiveAtMillis = (prisma.lastActiveAt ?? throwErr( + new StackAssertionError("User lastActiveAt should never be null after migration", { userId: prisma.projectUserId }) + )).getTime(); const selectedTeamMembers = prisma.teamMembers;
330-330: Add defensive null check for raw SQLlastActiveAtparsing.The raw SQL path lacks Prisma's type safety. If
row.lastActiveAtis unexpectedly null (e.g., migration failure), the current code producesNaNsilently. Consider adding an explicit check.🛡️ Defensive parsing suggestion
- last_active_at_millis: new Date(row.lastActiveAt + "Z").getTime(), + last_active_at_millis: row.lastActiveAt + ? new Date(row.lastActiveAt + "Z").getTime() + : throwErr(new StackAssertionError("User lastActiveAt should never be null", { userId: row.projectUserId })), is_anonymous: row.isAnonymous,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/backend/prisma/schema.prismaapps/backend/src/app/api/latest/users/crud.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{tsx,ts,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
For blocking alerts and errors, never use
toast; instead, use alerts as toasts are easily missed by the user
Files:
apps/backend/src/app/api/latest/users/crud.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{tsx,css}: Keep hover/click animations snappy and fast; don't delay actions with pre-transitions (e.g., no fade-in on button hover) as it makes UI feel sluggish; instead apply transitions after the action like smooth fade-out when hover ends
When creating hover transitions, avoid hover-enter transitions and use only hover-exit transitions (e.g.,transition-colors hover:transition-none)
Files:
apps/backend/src/app/api/latest/users/crud.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
NEVER use Next.js dynamic functions if avoidable; prefer using client components instead to keep pages static (e.g., use
usePathnameinstead ofawait params)
Files:
apps/backend/src/app/api/latest/users/crud.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: NEVER try-catch-all, NEVER void a promise, and NEVER use .catch(console.error) or similar; use loading indicators instead; if asynchronous handling is necessary, userunAsynchronouslyorrunAsynchronouslyWithAlertinstead
Use ES6 maps instead of records wherever possible
Files:
apps/backend/src/app/api/latest/users/crud.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Code defensively; prefer?? throwErr(...)over non-null assertions with good error messages explicitly stating violated assumptions
Avoid theanytype; when necessary, leave a comment explaining why it's used, why the type system fails, and how errors would be caught at compile-, test-, or runtime
Files:
apps/backend/src/app/api/latest/users/crud.tsx
🧠 Learnings (1)
📚 Learning: 2026-01-07T00:55:19.871Z
Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-07T00:55:19.871Z
Learning: The project uses PostgreSQL with Prisma ORM for database management; database models are located in `/apps/backend/src`
Applied to files:
apps/backend/prisma/schema.prisma
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Cursor Bugbot
- GitHub Check: docker
- GitHub Check: lint_and_build (latest)
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: E2E Tests (Node 22.x, Freestyle mock)
- GitHub Check: E2E Tests (Node 22.x, Freestyle prod)
- GitHub Check: all-good
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: setup-tests
- GitHub Check: setup-tests-with-custom-base-port
- GitHub Check: restart-dev-and-test
🔇 Additional comments (1)
apps/backend/src/app/api/latest/users/crud.tsx (1)
453-453: LGTM! SimplifieduserPrismaToCrudcalls.The refactoring consistently removes the external
lastActiveAtMillisparameter and relies on the database column directly via Prisma, which correctly simplifies the codebase and ensures data consistency.Also applies to: 602-602, 946-946
Older cmux preview screenshots (latest comment is below)Preview Screenshots⏳ Preview screenshots are being captured... Workspace and dev browser links will appear here once the preview environment is ready. Generated by cmux preview system |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/backend/src/app/api/latest/auth/sessions/crud.tsx:
- Around line 40-51: The mapping over refreshTokenObjs uses
geoInfoSchema.validateSync which will throw on malformed lastActiveAtIpInfo and
abort the whole operation; change the logic in the items mapping so IP info is
validated safely (e.g., use non-throwing validation or wrap validateSync in a
try/catch) and on failure fall back to undefined or a sanitized default before
returning the session object; update the block that constructs items
(references: refreshTokenObjs.map, geoInfoSchema, lastActiveAtIpInfo,
last_used_at_end_user_ip_info, auth.refreshTokenId) so a single bad IP blob
won’t break the entire sessions list.
🧹 Nitpick comments (1)
apps/backend/src/lib/tokens.tsx (1)
170-194: Consider handling potential record-not-found errors defensively.If the
ProjectUserorProjectUserRefreshTokenrecord is deleted between the user validation (lines 147-160) and these updates, Prisma will throw aP2025(record not found) error. While unlikely, this could cause a 500 error instead of a graceful invalid-token response.Consider wrapping these updates to handle this edge case, or at minimum, catching
P2025errors and returningnull(invalid token):♻️ Suggested defensive handling
+ import { Prisma } from '@prisma/client'; + // ... in the function: + try { await Promise.all([ prisma.projectUser.update({ where: { tenancyId_projectUserId: { tenancyId: options.tenancy.id, projectUserId: options.refreshTokenObj.projectUserId, }, }, data: { lastActiveAt: now, }, }), globalPrismaClient.projectUserRefreshToken.update({ where: { tenancyId_id: { tenancyId: options.tenancy.id, id: options.refreshTokenObj.id, }, }, data: { lastActiveAt: now, lastActiveAtIpInfo: ipInfo, }, }), ]); + } catch (error) { + if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2025') { + // Record was deleted between validation and update + return null; + } + throw error; + }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/backend/prisma/migrations/20260101000000_add_last_active_at_columns/migration.sqlapps/backend/prisma/schema.prismaapps/backend/src/app/api/latest/auth/sessions/crud.tsxapps/backend/src/lib/tokens.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/backend/prisma/schema.prisma
- apps/backend/prisma/migrations/20260101000000_add_last_active_at_columns/migration.sql
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{tsx,ts,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
For blocking alerts and errors, never use
toast; instead, use alerts as toasts are easily missed by the user
Files:
apps/backend/src/lib/tokens.tsxapps/backend/src/app/api/latest/auth/sessions/crud.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{tsx,css}: Keep hover/click animations snappy and fast; don't delay actions with pre-transitions (e.g., no fade-in on button hover) as it makes UI feel sluggish; instead apply transitions after the action like smooth fade-out when hover ends
When creating hover transitions, avoid hover-enter transitions and use only hover-exit transitions (e.g.,transition-colors hover:transition-none)
Files:
apps/backend/src/lib/tokens.tsxapps/backend/src/app/api/latest/auth/sessions/crud.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
NEVER use Next.js dynamic functions if avoidable; prefer using client components instead to keep pages static (e.g., use
usePathnameinstead ofawait params)
Files:
apps/backend/src/lib/tokens.tsxapps/backend/src/app/api/latest/auth/sessions/crud.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: NEVER try-catch-all, NEVER void a promise, and NEVER use .catch(console.error) or similar; use loading indicators instead; if asynchronous handling is necessary, userunAsynchronouslyorrunAsynchronouslyWithAlertinstead
Use ES6 maps instead of records wherever possible
Files:
apps/backend/src/lib/tokens.tsxapps/backend/src/app/api/latest/auth/sessions/crud.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Code defensively; prefer?? throwErr(...)over non-null assertions with good error messages explicitly stating violated assumptions
Avoid theanytype; when necessary, leave a comment explaining why it's used, why the type system fails, and how errors would be caught at compile-, test-, or runtime
Files:
apps/backend/src/lib/tokens.tsxapps/backend/src/app/api/latest/auth/sessions/crud.tsx
🧠 Learnings (2)
📚 Learning: 2025-10-11T04:13:19.308Z
Learnt from: N2D4
Repo: stack-auth/stack-auth PR: 943
File: examples/convex/app/action/page.tsx:23-28
Timestamp: 2025-10-11T04:13:19.308Z
Learning: In the stack-auth codebase, use `runAsynchronouslyWithAlert` from `stackframe/stack-shared/dist/utils/promises` for async button click handlers and form submissions instead of manual try/catch blocks. This utility automatically handles errors and shows alerts to users.
Applied to files:
apps/backend/src/lib/tokens.tsx
📚 Learning: 2026-01-07T00:55:19.871Z
Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-07T00:55:19.871Z
Learning: The project uses PostgreSQL with Prisma ORM for database management; database models are located in `/apps/backend/src`
Applied to files:
apps/backend/src/app/api/latest/auth/sessions/crud.tsx
🧬 Code graph analysis (1)
apps/backend/src/lib/tokens.tsx (2)
apps/backend/src/prisma-client.tsx (1)
getPrismaClientForTenancy(67-69)apps/backend/src/lib/end-users.tsx (1)
getEndUserInfo(63-134)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Vercel Agent Review
- GitHub Check: Cursor Bugbot
- GitHub Check: lint_and_build (latest)
- GitHub Check: E2E Tests (Node 22.x, Freestyle prod)
- GitHub Check: E2E Tests (Node 22.x, Freestyle mock)
- GitHub Check: setup-tests
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: restart-dev-and-test-with-custom-base-port
- GitHub Check: all-good
- GitHub Check: setup-tests-with-custom-base-port
- GitHub Check: build (22.x)
🔇 Additional comments (5)
apps/backend/src/lib/tokens.tsx (3)
2-2: LGTM!The new imports are correctly added and properly utilized in the activity tracking logic below.
Also applies to: 14-14
196-206: LGTM!The event logging is correctly awaited and appropriately placed after the DB updates. The comment clearly explains the purpose of maintaining the event for metrics and geo tracking.
166-168: LGTM!The IP info extraction correctly handles all cases of the discriminated union returned by
getEndUserInfo(), properly differentiating between spoofed and exact info while gracefully handling null.apps/backend/src/app/api/latest/auth/sessions/crud.tsx (2)
21-26: LGTM! Good security improvement.The client authorization guard correctly prevents clients from listing other users' sessions, ensuring proper access control.
57-80: LGTM!The delete handler correctly validates permissions, prevents deletion of the current session, and uses proper error handling.
Preview Screenshots⏳ Preview screenshots are being captured... Workspace and dev browser links will appear here once the preview environment is ready. Generated by cmux preview system |
Note
Introduces persistent activity tracking for users and sessions and switches APIs/metrics to read from these fields instead of ad‑hoc event queries.
lastActiveAttoProjectUser,lastActiveAtandlastActiveAtIpInfotoProjectUserRefreshToken; backfills from$user-activity/$session-activityevents; sets NOT NULL withNOW()defaultslast_used_atand validatedlast_used_at_end_user_ip_infofrom stored fieldslastActiveAt, stores end-user IP info, and logs a$session-activityeventrecently_activenow ordered byProjectUser.lastActiveAt; removes event-based lookupslastActiveAtdirectly; removes helper queriesWritten by Cursor Bugbot for commit a237f88. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.