Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a nullable Project.ownerTeamId, a migration to convert managedProjectIds into Team and TeamMember records and assign ownerTeamId, and broad refactors so project ownership, provision/transfer flows, seeds, UI, libs, and tests use owner_team_id (teams) instead of per-user serverMetadata. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant API as Transfer Verify Handler
participant Teams as Teams CRUD
participant DB as Global Prisma
U->>API: POST /transfer/confirm { code, project_id }
API->>DB: find project by project_id (global)
DB-->>API: project (ownerTeamId maybe null)
alt project not found or already transferred
API-->>U: 400 error
else
API->>Teams: adminCreate(team for transferee)
Teams-->>API: Team { id }
API->>DB: update project.ownerTeamId = Team.id
DB-->>API: OK
API-->>U: 200 { project_id }
end
sequenceDiagram
participant U as User (client)
participant API as Create Project Endpoint
participant Ten as Tenancy / Membership
participant DB as Prisma
U->>API: POST /projects { display_name, teamId }
API->>Ten: ensureTeamMembershipExists(tenancyId, teamId, userId)
Ten-->>API: OK
API->>DB: create Project { ownerTeamId: teamId, ... }
DB-->>API: Project { owner_team_id }
API-->>U: 201 Project { owner_team_id: teamId }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR implements a major architectural shift from individual user-based project ownership to team-based project ownership in the Stack Auth system. The changes introduce an ownerTeamId field throughout the system, replacing the previous approach where project ownership was tracked through user metadata (managedProjectIds arrays).
Key changes include:
Database Schema: Added an optional ownerTeamId UUID field to the Project model, with a comprehensive migration that creates personal teams for existing users who manage projects and assigns their projects to these teams.
API Layer: Updated all project CRUD operations to work with team-based ownership. The listManagedProjectIds function now queries team memberships instead of user metadata. Project creation endpoints across different integrations (Neon, custom) now use ownerTeamId: null for externally provisioned projects.
Frontend Components: Project cards now display the owner team's name, and the team switcher component gained styling customization capabilities to support different contexts.
Type Definitions: Added ownerTeamId: string | null to the AdminProject type and updated CRUD schemas to include the owner_team_id field for read operations.
Transfer Handlers: Refactored project transfer confirmation handlers to create personal teams for users and assign transferred projects to those teams, rather than updating user metadata.
Testing: Comprehensively updated E2E tests to expect owner_team_id fields in responses and verify proper team-based ownership behavior.
This change enables better collaboration by allowing multiple team members to manage projects collectively, provides clearer permission boundaries, and creates a foundation for more sophisticated team-based features. The migration preserves existing functionality by creating personal teams for current project owners, ensuring backward compatibility during the transition.
Confidence score: 3/5
- This PR introduces significant architectural changes that affect core ownership and permission systems across the entire codebase
- Score reflects the complexity of the changes and potential for subtle permission or access control issues during the transition period
- Pay close attention to the database migration file and the
listManagedProjectIdsfunction implementation
18 files reviewed, 6 comments
|
✨ No issues found! Your code is sparkling clean! ✨ 🗒️ View all ignored comments in this repo
Need help? Join our Discord for support! |
Documentation Changes Required
Please ensure these changes are reflected in the relevant documentation file. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
apps/backend/prisma/seed.ts (1)
39-61: Potential race condition: Team referenced before creationThe internal project is created with
ownerTeamId: internalTeamIdat line 40, but the corresponding team is only created later at lines 95-103. If there's a foreign key constraint between projects and teams, this could cause the project creation to fail.Consider reordering the operations to create the team before assigning it as the owner.
🧹 Nitpick comments (4)
apps/backend/prisma/schema.prisma (1)
22-22: Consider adding referential integrity for ownerTeamId.The addition of
ownerTeamId String? @db.Uuidcorrectly implements team-based ownership. However, consider adding a foreign key relationship to the Team model to ensure referential integrity, even though the composite primary key structure may make this complex.If referential integrity is desired, you could consider adding a relation like:
ownerTeam Team? @relation(fields: [ownerTeamId], references: [teamId])However, this might require additional constraints to handle the tenancy relationship properly.
apps/dashboard/src/components/project-card.tsx (1)
9-10: Consider safer fallback for ownerTeamId.The current implementation uses
project.ownerTeamId ?? ""as a fallback, but this may result in callinguseTeam("")which could be problematic. Consider using a more explicit check:- const user = useUser({ "or": "redirect", projectIdMustMatch: "internal" }); - const team = user.useTeam(project.ownerTeamId ?? ""); + const user = useUser({ "or": "redirect", projectIdMustMatch: "internal" }); + const team = project.ownerTeamId ? user.useTeam(project.ownerTeamId) : null;apps/backend/src/app/api/latest/integrations/custom/projects/transfer/confirm/verification-code-handler.tsx (1)
59-79: Consider extracting common team creation and ownership logic.The team creation and project ownership assignment logic is identical between this handler and the neon integration handler, indicating potential code duplication.
Consider creating a shared utility function for project transfer team creation:
// In a shared utilities file async function createProjectOwnershipTeam(user: User, tenancy: Tenancy) { return await teamsCrudHandlers.adminCreate({ data: { display_name: user.display_name ? `${user.display_name}'s Team` : user.primary_email ? `${user.primary_email}'s Team` : "Personal Team", creator_user_id: 'me', }, tenancy, user, }); } async function assignProjectToTeam(projectId: string, teamId: string) { return await globalPrismaClient.project.update({ where: { id: projectId }, data: { ownerTeamId: teamId }, }); }apps/backend/src/lib/projects.tsx (1)
14-38: Consider optimizing with a single join queryThe current implementation makes two separate database queries - first to fetch teams, then to fetch projects. This could be optimized into a single query with a join for better performance.
Consider refactoring to use a single query:
const projectIds = await globalPrismaClient.project.findMany({ where: { ownerTeam: { tenancyId: internalTenancy.id, teamMembers: { some: { projectUserId: projectUser.id, } } } }, select: { id: true, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/backend/prisma/migrations/20250806171211_add_team_based_project_ownership/migration.sql(1 hunks)apps/backend/prisma/schema.prisma(1 hunks)apps/backend/prisma/seed.ts(11 hunks)apps/backend/src/app/api/latest/integrations/custom/projects/provision/route.tsx(1 hunks)apps/backend/src/app/api/latest/integrations/custom/projects/transfer/confirm/verification-code-handler.tsx(2 hunks)apps/backend/src/app/api/latest/integrations/neon/projects/provision/route.tsx(1 hunks)apps/backend/src/app/api/latest/integrations/neon/projects/transfer/confirm/verification-code-handler.tsx(2 hunks)apps/backend/src/app/api/latest/internal/failed-emails-digest/crud.tsx(1 hunks)apps/backend/src/app/api/latest/internal/projects/crud.tsx(2 hunks)apps/backend/src/app/api/latest/internal/projects/current/crud.tsx(0 hunks)apps/backend/src/lib/projects.tsx(4 hunks)apps/backend/src/route-handlers/smart-request.tsx(1 hunks)apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client.tsx(3 hunks)apps/dashboard/src/components/project-card.tsx(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/projects.test.ts(19 hunks)packages/stack-shared/src/interface/crud/projects.ts(1 hunks)packages/template/src/components/selected-team-switcher.tsx(4 hunks)packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts(1 hunks)packages/template/src/lib/stack-app/projects/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/backend/src/app/api/latest/internal/projects/current/crud.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/backend/src/app/api/latest/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/backend/src/app/api/latest/**/*: Main API routes are located in /apps/backend/src/app/api/latest
The project uses a custom route handler system in the backend for consistent API responses
Files:
apps/backend/src/app/api/latest/integrations/custom/projects/provision/route.tsxapps/backend/src/app/api/latest/integrations/neon/projects/provision/route.tsxapps/backend/src/app/api/latest/internal/projects/crud.tsxapps/backend/src/app/api/latest/integrations/custom/projects/transfer/confirm/verification-code-handler.tsxapps/backend/src/app/api/latest/integrations/neon/projects/transfer/confirm/verification-code-handler.tsxapps/backend/src/app/api/latest/internal/failed-emails-digest/crud.tsx
apps/backend/prisma/schema.prisma
📄 CodeRabbit Inference Engine (CLAUDE.md)
Database models use Prisma
Files:
apps/backend/prisma/schema.prisma
🧠 Learnings (2)
📚 Learning: 2025-08-04T22:25:51.260Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T22:25:51.260Z
Learning: Applies to apps/backend/src/app/api/latest/**/* : The project uses a custom route handler system in the backend for consistent API responses
Applied to files:
apps/backend/src/app/api/latest/integrations/custom/projects/provision/route.tsxapps/backend/src/app/api/latest/integrations/neon/projects/provision/route.tsx
📚 Learning: 2025-08-04T22:25:51.260Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T22:25:51.260Z
Learning: Applies to apps/backend/prisma/schema.prisma : Database models use Prisma
Applied to files:
apps/backend/prisma/schema.prismaapps/backend/prisma/migrations/20250806171211_add_team_based_project_ownership/migration.sqlapps/backend/src/app/api/latest/integrations/neon/projects/transfer/confirm/verification-code-handler.tsxapps/backend/prisma/seed.ts
🧬 Code Graph Analysis (5)
apps/backend/src/route-handlers/smart-request.tsx (1)
apps/backend/src/lib/projects.tsx (1)
listManagedProjectIds(14-38)
apps/dashboard/src/components/project-card.tsx (3)
packages/template/src/lib/stack-app/projects/index.ts (1)
AdminProject(15-35)apps/dashboard/src/hooks/use-from-now.tsx (1)
useFromNow(4-22)packages/stack-shared/src/utils/urls.tsx (1)
urlString(172-174)
packages/template/src/components/selected-team-switcher.tsx (2)
packages/stack-ui/src/components/ui/select.tsx (1)
SelectTrigger(160-160)packages/stack-ui/src/index.ts (1)
cn(53-53)
apps/backend/src/app/api/latest/integrations/neon/projects/transfer/confirm/verification-code-handler.tsx (2)
apps/backend/src/app/api/latest/teams/crud.tsx (1)
teamsCrudHandlers(29-245)apps/backend/src/prisma-client.tsx (1)
globalPrismaClient(31-31)
apps/backend/src/lib/projects.tsx (3)
packages/stack-shared/src/interface/crud/users.ts (1)
UsersCrud(103-103)apps/backend/src/lib/tenancies.tsx (2)
getSoleTenancyFromProjectBranch(59-66)DEFAULT_BRANCH_ID(17-17)apps/backend/src/prisma-client.tsx (2)
getPrismaClientForTenancy(50-52)globalPrismaClient(31-31)
⏰ 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). (9)
- GitHub Check: lint_and_build (latest)
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: all-good
- GitHub Check: restart-dev-and-test
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: Security Check
🔇 Additional comments (13)
apps/backend/src/app/api/latest/integrations/custom/projects/provision/route.tsx (1)
32-32: LGTM! Team-based ownership correctly implemented.The change from user-based ownership to team-based ownership is correctly implemented. Setting
ownerTeamId: nullduring provisioning is appropriate as no specific team is assigned at this stage, consistent with other integration provisioning routes.packages/template/src/lib/stack-app/projects/index.ts (1)
21-21: LGTM! Type definition correctly updated for team-based ownership.The addition of
ownerTeamId: string | nullto the AdminProject type is appropriate and aligns with the database schema changes. The field is correctly marked as readonly and nullable to match the expected usage pattern.apps/backend/src/route-handlers/smart-request.tsx (1)
219-219: Critical fix: Correctly awaiting async listManagedProjectIds.This
awaitaddition is essential. ThelistManagedProjectIdsfunction was changed from synchronous to asynchronous as part of the team-based ownership migration. Without thisawait, the function would return a Promise object instead of an array, causingallProjects.includes(options.projectId)to always return false and break admin authentication.packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts (1)
116-116: LGTM! Clean field mapping for team ownership.The addition of
ownerTeamIdfield mapping fromdata.owner_team_idis consistent with the team-based ownership model and follows the established camelCase convention for the AdminProject interface.apps/backend/src/app/api/latest/integrations/neon/projects/provision/route.tsx (1)
36-36: LGTM! Correct transition to team-based ownership.The change from
ownerIds: []toownerTeamId: nullproperly aligns with the new team-based ownership model. Usingnullis appropriate for the Neon integration context where no specific team is pre-assigned during provisioning.packages/template/src/components/selected-team-switcher.tsx (1)
5-5: LGTM! Good styling flexibility enhancement.The addition of the optional
triggerClassNameprop and its proper integration using thecnutility provides excellent styling flexibility while maintaining backward compatibility. This enhancement supports the component's usage across different UI contexts.Also applies to: 37-37, 122-122
apps/dashboard/src/components/project-card.tsx (1)
18-18: LGTM! Clean team display integration.The addition of team display name in the CardDescription properly shows project ownership information and uses safe optional chaining.
apps/backend/src/app/api/latest/internal/projects/crud.tsx (2)
27-27: LGTM! Team-based ownership implementation is correct.The change from owner packs to using the user's
selected_team_idaligns with the team-based ownership migration. The null check withthrowErrensures data integrity.
48-48: Correct async handling.The addition of
awaitis necessary sincelistManagedProjectIdsis now asynchronous to support team-based project queries.apps/backend/prisma/migrations/20250806171211_add_team_based_project_ownership/migration.sql (1)
51-51: Potential SQL injection vulnerability in team name generation.The string concatenation in the display name could be vulnerable to SQL injection if user display names contain malicious SQL.
Use proper parameterization for the team display name:
- COALESCE(user_record."displayName", 'User') || '''s Team', + COALESCE(user_record."displayName", 'User') || E'\'s Team',Or better yet, use
format()function:- COALESCE(user_record."displayName", 'User') || '''s Team', + format('%s''s Team', COALESCE(user_record."displayName", 'User')),Likely an incorrect or invalid review comment.
apps/backend/src/app/api/latest/integrations/neon/projects/transfer/confirm/verification-code-handler.tsx (2)
58-69: Team creation logic is well-structured.The team creation with proper fallbacks for display name generation is well-implemented. The use of
teamsCrudHandlers.adminCreateensures consistent team creation logic.
71-78: Correct team ownership assignment.The project ownership update properly assigns the newly created team as the owner using the global Prisma client.
apps/backend/src/app/api/latest/internal/failed-emails-digest/crud.tsx (1)
31-36: Consider optionalownerTeamIdin your JOINsThe Prisma schema (
apps/backend/prisma/schema.prisma) declaresownerTeamIdasString?, so it can beNULL. UsingINNER JOINonTeam(and subsequently onTeamMember/ContactChannel) will drop any projects without anownerTeamId.Please confirm whether excluding those projects is intended. If you need to include projects lacking an owner team, consider switching to
LEFT JOINfor theTeamtable (and adjust downstream joins or addWHEREfilters accordingly).• File:
apps/backend/src/app/api/latest/internal/failed-emails-digest/crud.tsx
• Lines: 31–36Original snippet:
INNER JOIN "Team" team ON team."teamId" = p."ownerTeamId" AND team."tenancyId" = t.id INNER JOIN "TeamMember" tm ON tm."teamId" = team."teamId" AND tm."projectUserId" = pu."projectUserId" AND tm."tenancyId" = t.id INNER JOIN "ContactChannel" cc ON tm."projectUserId" = cc."projectUserId"
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/e2e/tests/backend/endpoints/api/v1/auth/password/sign-up.test.ts (1)
228-230: Address TODO items for team creation configuration tests.These TODO items indicate missing test coverage for different team creation configurations. Consider implementing these tests or creating issues to track them.
Would you like me to help implement these missing test cases or create issues to track this work?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/backend/prisma/migrations/20250806171211_add_team_based_project_ownership/migration.sql(1 hunks)apps/backend/src/app/api/latest/internal/failed-emails-digest/crud.tsx(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/anonymous/sign-up.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/token.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/password/sign-in.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/password/sign-up.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/contact-channels/contact-channels.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/provision.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth-providers.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/current.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/provision.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/failed-emails-digest.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/projects.test.ts(11 hunks)apps/e2e/tests/backend/endpoints/api/v1/project-permissions.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/team-invitations.test.ts(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/team-memberships.test.ts(8 hunks)apps/e2e/tests/backend/endpoints/api/v1/team-permissions.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/teams.test.ts(6 hunks)apps/e2e/tests/backend/endpoints/api/v1/users.test.ts(19 hunks)packages/stack-shared/src/interface/crud/projects.ts(1 hunks)
✅ Files skipped from review due to trivial changes (9)
- apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth-providers.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/project-permissions.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/team-permissions.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/provision.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/current.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/internal/projects.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/contact-channels/contact-channels.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/teams.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/team-memberships.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/stack-shared/src/interface/crud/projects.ts
- apps/backend/src/app/api/latest/internal/failed-emails-digest/crud.tsx
- apps/backend/prisma/migrations/20250806171211_add_team_based_project_ownership/migration.sql
⏰ 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). (8)
- GitHub Check: restart-dev-and-test
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: all-good
- GitHub Check: lint_and_build (latest)
- GitHub Check: Security Check
🔇 Additional comments (13)
apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/provision.test.ts (1)
85-85: LGTM! Test snapshot correctly updated for team ownership migration.The addition of
"owner_team_id": nullto the test snapshot properly reflects the new project schema. The null value is appropriate for projects provisioned via the Neon integration that don't have explicit team ownership assigned.apps/e2e/tests/backend/endpoints/api/v1/auth/anonymous/sign-up.test.ts (1)
27-34: LGTM! Team-based ownership correctly implemented for anonymous users.The test properly validates that anonymous users now receive a
selected_teamobject with appropriate metadata and a correspondingselected_team_id. This aligns with the broader migration to team-based project ownership.apps/e2e/tests/backend/endpoints/api/v1/auth/password/sign-in.test.ts (1)
39-46: LGTM! Consistent team assignment for password authentication.The test correctly validates the new team-based user model. The
selected_teamobject structure andselected_team_idfield are properly populated, maintaining consistency with other authentication methods.apps/e2e/tests/backend/endpoints/api/v1/auth/password/sign-up.test.ts (1)
50-57: LGTM! Password sign-up properly implements team assignment.The test correctly validates that new users signing up with passwords receive appropriate team assignments following the established pattern.
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/token.test.ts (1)
57-64: LGTM! OAuth authentication properly integrated with team model.The test correctly validates that OAuth users receive proper team assignments, maintaining consistency across all authentication methods in the system.
apps/e2e/tests/backend/endpoints/api/v1/internal/failed-emails-digest.test.ts (1)
332-363: LGTM! Properly implements team-based ownership for multiple owners scenario.The refactored approach correctly:
- Captures user IDs explicitly for team management
- Uses the team-memberships API to establish ownership relationships
- Validates team membership creation with proper assertions
This is a cleaner implementation than the previous user metadata patching approach and aligns with the new team-based ownership model.
apps/e2e/tests/backend/endpoints/api/v1/team-invitations.test.ts (2)
99-107: LGTM! Test correctly reflects personal team creation.The addition of the personal team object in the expected response aligns with the new team-based ownership model. The team structure includes all necessary metadata fields and follows the expected naming pattern.
149-150: LGTM! Assertion updated for new team structure.The updated assertion correctly expects 2 teams instead of 1, accounting for both the explicitly created team and the personal team that's now automatically generated for users.
apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (5)
147-154: LGTM! User response updated for team-based model.The selected_team object correctly includes all necessary metadata fields for client access, replacing the previous null value to reflect the new personal team assignment.
186-194: LGTM! Consistent team structure in user responses.The selected_team structure is consistently applied across different test scenarios, maintaining the same metadata pattern.
270-278: LGTM! Server and client responses aligned.The selected_team object maintains consistency between client and server access patterns while including appropriate fields for each access level.
803-812: LGTM! Server response includes additional metadata.The server access response correctly includes
created_at_millisandserver_metadatafields in the selected_team object, which is appropriate for server-level access.
743-774: LGTM! Selected team functionality properly tested.The test case comprehensively validates the selected team update functionality, including switching between teams and setting to null. This ensures the new team selection feature works correctly.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/backend/prisma/migrations/20250806171211_add_team_based_project_ownership/migration.sql (2)
82-89: Still missing existence check before updating projects.
The prior review comment suggesting a guard clause for nonexistent project IDs has not been addressed.
76-76: Insert stores'TRUE'instead of boolean TRUE inTeamMember.isSelected.This will silently coerce or fail depending on DB settings and violates the column’s type.
- 'TRUE', + TRUE,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/backend/prisma/migrations/20250806171211_add_team_based_project_ownership/migration.sql(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-04T22:25:51.260Z
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T22:25:51.260Z
Learning: Applies to apps/backend/prisma/schema.prisma : Database models use Prisma
Applied to files:
apps/backend/prisma/migrations/20250806171211_add_team_based_project_ownership/migration.sql
⏰ 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). (10)
- GitHub Check: restart-dev-and-test
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: all-good
- GitHub Check: lint_and_build (latest)
- GitHub Check: check_prisma_migrations (22.x)
- GitHub Check: docker
- GitHub Check: Security Check
|
Claude finished @BilalG1's task —— View job Comprehensive Code Review: Team-Based Project OwnershipThis is a significant architectural refactoring that introduces team-based project ownership, replacing the previous user-based ownership model. The changes are well-architected overall, but there are several important issues that need to be addressed. 🚨 Critical Issues1. Database Migration Safety ConcernsFile:
Recommendations: -- Fix boolean comparisons
AND cc."isPrimary" = TRUE -- not 'TRUE'
-- Add foreign key constraint and index
ALTER TABLE "Project"
ADD COLUMN "ownerTeamId" UUID,
ADD CONSTRAINT "Project_ownerTeamId_fkey"
FOREIGN KEY ("ownerTeamId") REFERENCES "Team"("teamId") ON DELETE SET NULL;
CREATE INDEX IF NOT EXISTS "idx_Project_ownerTeamId" ON "Project" ("ownerTeamId");
-- Add project existence check
IF NOT EXISTS (SELECT 1 FROM "Project" WHERE "id" = project_id_text) THEN
RAISE WARNING 'Project % does not exist, skipping', project_id_text;
CONTINUE;
END IF;2. UI Form Integration IssueFile: The Fix Required: // Add to form schema
ownerTeamId: yup.string().nullable(),
// Replace current team selection with proper form field
<FormField
control={form.control}
name="ownerTeamId"
render={({ field }) => (
<div className="flex flex-col gap-2">
<Label>Team</Label>
<SelectedTeamSwitcher
triggerClassName="max-w-full"
onChange={field.onChange}
/>
<FormMessage />
</div>
)}
/>
|
|
Claude finished @BilalG1's task —— View job Pull Request Review: Team-Based Project OwnershipReview Checklist:
Comprehensive Code Review: Team-Based Project OwnershipThis is a significant architectural refactoring that introduces team-based project ownership, replacing the previous user-based ownership model. The changes are well-architected overall, but there are several important issues that need to be addressed. 🚨 Critical Issues1. Database Migration Safety ConcernsFile:
Recommendations: -- Fix boolean comparisons
AND cc."isPrimary" = TRUE -- not 'TRUE'
-- Add foreign key constraint and index
ALTER TABLE "Project"
ADD COLUMN "ownerTeamId" UUID,
ADD CONSTRAINT "Project_ownerTeamId_fkey"
FOREIGN KEY ("ownerTeamId") REFERENCES "Team"("teamId") ON DELETE SET NULL;
CREATE INDEX IF NOT EXISTS "idx_Project_ownerTeamId" ON "Project" ("ownerTeamId");
-- Add project existence check
IF NOT EXISTS (SELECT 1 FROM "Project" WHERE "id" = project_id_text) THEN
RAISE WARNING 'Project % does not exist, skipping', project_id_text;
CONTINUE;
END IF;2. UI Form Integration IssueFile: The Fix Required: // Add to form schema
ownerTeamId: yup.string().nullable(),
// Replace current team selection with proper form field
<FormField
control={form.control}
name="ownerTeamId"
render={({ field }) => (
<div className="flex flex-col gap-2">
<Label>Team</Label>
<SelectedTeamSwitcher
triggerClassName="max-w-full"
onChange={field.onChange}
/>
<FormMessage />
</div>
)}
/>
|
|
Claude finished @BilalG1's task —— View job Pull Request Review: Team-Based Project OwnershipReview Checklist:
Comprehensive Code Review: Team-Based Project OwnershipThis is a significant architectural refactoring that introduces team-based project ownership, replacing the previous user-based ownership model. The changes are well-architected overall, but there are several important issues that need to be addressed. 🚨 Critical Issues1. Database Migration Safety ConcernsFile:
Recommendations: -- Fix boolean comparisons
AND cc."isPrimary" = TRUE -- not 'TRUE'
-- Add foreign key constraint and index
ALTER TABLE "Project"
ADD COLUMN "ownerTeamId" UUID,
ADD CONSTRAINT "Project_ownerTeamId_fkey"
FOREIGN KEY ("ownerTeamId") REFERENCES "Team"("teamId") ON DELETE SET NULL;
CREATE INDEX IF NOT EXISTS "idx_Project_ownerTeamId" ON "Project" ("ownerTeamId");
-- Add project existence check
IF NOT EXISTS (SELECT 1 FROM "Project" WHERE "id" = project_id_text) THEN
RAISE WARNING 'Project % does not exist, skipping', project_id_text;
CONTINUE;
END IF;2. UI Form Integration IssueFile: The Fix Required: // Add to form schema
ownerTeamId: yup.string().nullable(),
// Replace current team selection with proper form field
<FormField
control={form.control}
name="ownerTeamId"
render={({ field }) => (
<div className="flex flex-col gap-2">
<Label>Team</Label>
<SelectedTeamSwitcher
triggerClassName="max-w-full"
onChange={field.onChange}
/>
<FormMessage />
</div>
)}
/>
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/current.test.ts (1)
1-177: Add missingownerTeamIdto consumerProjecttypeThe API now returns
owner_team_id, but the client-facingProjecttype inpackages/template/src/lib/stack-app/projects/index.tsdoesn’t include it. Update the type (and any JSON-to-TS mapping) to prevent drift:Impacted file:
- packages/template/src/lib/stack-app/projects/index.ts
Proposed diff:
export type Project = { readonly id: string, readonly displayName: string, + /** Maps from API’s `owner_team_id` */ + readonly ownerTeamId: string, readonly config: ProjectConfig, };Also verify your fetch/deserialize logic maps the JSON
owner_team_id→ownerTeamId.apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (1)
744-776: Fix inconsistent initial expectation for selected_team_id and drop the “set to null” stepElsewhere in this file, selected_team and selected_team_id are now non-null after sign-in. This test still expects null initially and later sets selected_team_id to null, which contradicts the new semantics and will likely fail.
Apply the following minimal update: assert a non-null initial selected_team_id and remove the final “set to null” block.
it("should be able to update selected team", async ({ expect }) => { - await Project.createAndSwitch({ config: { magic_link_enabled: true } }); + await Project.createAndSwitch({ config: { magic_link_enabled: true } }); await Auth.Otp.signIn(); const { teamId: team1Id } = await Team.createWithCurrentAsCreator({}); const { teamId: team2Id } = await Team.createWithCurrentAsCreator({}); const response1 = await niceBackendFetch("/api/v1/users/me", { accessType: "client", }); - expect(response1.body.selected_team_id).toEqual(null); + // With team-based semantics, the user should already have a selected team (e.g., personal team). + expect(typeof response1.body.selected_team_id).toBe("string"); const response2 = await niceBackendFetch("/api/v1/users/me", { accessType: "client", method: "PATCH", body: { selected_team_id: team1Id, }, }); expect(response2.body.selected_team_id).toEqual(team1Id); const response3 = await niceBackendFetch("/api/v1/users/me", { accessType: "client", method: "PATCH", body: { selected_team_id: team2Id, }, }); expect(response3.body.selected_team_id).toEqual(team2Id); - const response4 = await niceBackendFetch("/api/v1/users/me", { - accessType: "client", - method: "PATCH", - body: { - selected_team_id: null, - }, - }); - expect(response4.body.selected_team_id).toEqual(null);If clearing the selection to null is still a supported scenario, consider moving that into a separate, explicitly named test and aligning expectations with the intended behavior (e.g., error or fallback to personal team).
♻️ Duplicate comments (4)
packages/template/src/components/selected-team-switcher.tsx (1)
71-90: Handle nullable team correctly and drop unsafe casts in onChangeCurrent code force-casts team as Team in multiple places. When AllowNull is true, this can cause runtime errors (e.g., navigating with null or calling setSelectedTeam(null)). Narrow the value and eliminate the casts.
Apply this diff:
onChange={async (team) => { - if (props.onChange) { - props.onChange(team as Team); - } + props.onChange?.(team); // Skip actual navigation/updates in mock mode if (props.mockUser) return; - if (!props.noUpdateSelectedTeam) { - await user?.setSelectedTeam(team as Team); - } - if (props.urlMap) { - navigate(props.urlMap(team as Team)); - } + // Only proceed when a non-null team is selected + if (team == null) return; + if (!props.noUpdateSelectedTeam) { + await user?.setSelectedTeam(team); + } + if (props.urlMap) { + navigate(props.urlMap(team)); + } }}apps/backend/src/lib/projects.tsx (1)
76-90: Verify all create call sites now provide owner_team_idThe create signature now requires owner_team_id. Some routes (e.g., Neon provisioning) may still miss it.
Run:
#!/bin/bash set -euo pipefail echo "== Call sites for createOrUpdateProjectWithLegacyConfig ==" rg -n -C2 'createOrUpdateProjectWithLegacyConfig\(' echo echo "== Instances passing type: \"create\" without owner_team_id ==" rg -n -C3 'createOrUpdateProjectWithLegacyConfig\(\s*\{\s*type:\s*"create".*?\}\s*\)' | sed -n '1,220p'If the provisioning route is still missing owner_team_id, add it (null or a UUID) to unblock the create path.
apps/e2e/tests/backend/endpoints/api/v1/projects.test.ts (2)
1406-1409: User token incorrectly used as admin tokenThis is using a user access token obtained from
Auth.Otp.signIn()as an admin token, which is incorrect. User tokens should not authenticate admin endpoints.- await Auth.Otp.signIn(); - const adminAccessToken = backendContext.value.userAuth?.accessToken; - const { projectId } = await Project.create(); + await Auth.Otp.signIn(); + const { projectId, adminAccessToken } = await Project.createAndGetAdminToken();
1451-1453: User token incorrectly used as admin token (second instance)Similar issue - using user auth token instead of proper admin token.
- await Auth.Otp.signIn(); - const user1Auth = backendContext.value.userAuth; - const { projectId } = await Project.create(); + await Auth.Otp.signIn(); + const user1Auth = backendContext.value.userAuth; + const { projectId, adminAccessToken: user1AdminToken } = await Project.createAndGetAdminToken();
🧹 Nitpick comments (11)
packages/template/src/components/selected-team-switcher.tsx (3)
6-6: Avoid double Suspense boundaries; TeamSwitcher already wraps itself with SuspensePer the referenced snippet, TeamSwitcher renders its own Suspense boundary with a fallback. Wrapping SelectedTeamSwitcher in an additional Suspense duplicates the loading UI unnecessarily.
Apply this diff:
-import { Suspense, useEffect } from "react"; +import { useEffect } from "react";export function SelectedTeamSwitcher<AllowNull extends boolean = false>(props: SelectedTeamSwitcherProps<AllowNull>) { - return <Suspense fallback={<Fallback />}> - <Inner {...props} /> - </Suspense>; + return <Inner {...props} />; }If you take this route, the local Fallback component (Lines 42-44) becomes unused; consider removing it as well (see separate nit).
Also applies to: 36-40
16-24: Propagate allowNull to selectedTeam’s type (aligns consumer ergonomics with onChange/urlMap)selectedTeam is currently typed as Team regardless of allowNull. For consistency with the conditional types on urlMap and onChange—and to let consumers pass null when AllowNull is true—make selectedTeam conditional as well.
type SelectedTeamSwitcherProps<AllowNull extends boolean = false> = { urlMap?: (team: AllowNull extends true ? Team | null : Team) => string, - selectedTeam?: Team, + selectedTeam?: AllowNull extends true ? Team | null : Team, noUpdateSelectedTeam?: boolean, allowNull?: AllowNull, nullLabel?: string, onChange?: (team: AllowNull extends true ? Team | null : Team) => void, triggerClassName?: string,Note: This is a type-only change that improves correctness and DX. It doesn’t alter runtime behavior.
42-44: If outer Suspense is removed, drop the now-unused Fallback componentFollow-up to the Suspense refactor: once the additional Suspense wrapper is removed, this Fallback will be unused and can be safely deleted.
apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (2)
519-528: Remove unused variable in TOTP testThe local variable secret is declared but never used.
- const secret = generateSecureRandomString(32); const response = await niceBackendFetch("/api/v1/users/me", { accessType: "client", method: "PATCH", body: { totp_secret_base64: "ZXhhbXBsZSB2YWx1ZQ==", }, });
1900-1934: Optional: reduce snapshot brittleness by asserting shapes for selected_teamThere are many repeated inline snapshots that include the same selected_team shape. Consider using expect.objectContaining or a helper matcher to reduce duplication and flakiness.
Example pattern for these assertions:
expect(response.body).toEqual(expect.objectContaining({ selected_team_id: expect.any(String), selected_team: expect.objectContaining({ id: expect.any(String), display_name: expect.any(String), client_metadata: null, client_read_only_metadata: null, profile_image_url: null, }), }));If you prefer to keep snapshots for readability, extracting a reusable helper like expectUserWithSelectedTeam(response.body) will still cut down duplication.
apps/backend/src/lib/projects.tsx (4)
15-39: Tighten query: select only teamId to reduce data transferThe first query fetches full team rows but only teamId is used. Select just teamId to reduce payload and CPU.
- const teams = await internalPrisma.team.findMany({ + const teams = await internalPrisma.team.findMany({ where: { tenancyId: internalTenancy.id, teamMembers: { some: { projectUserId: projectUser.id, } } }, + select: { teamId: true }, }); const projectIds = await globalPrismaClient.project.findMany({ where: { ownerTeamId: { in: teams.map((team) => team.teamId), }, }, select: { id: true, }, });
63-66: Harden created_at handling; avoid string concatenation with "Z"Depending on the driver, row.createdAt may already be a Date. Concatenating "Z" to a Date can produce NaN. Prefer safe parsing.
- created_at_millis: new Date(row.createdAt + "Z").getTime(), + created_at_millis: (row.createdAt instanceof Date + ? row.createdAt + : new Date(row.createdAt)).getTime(),
137-147: Avoid passing undefined fields to Prisma updateWhen fields are omitted in options.data, you currently pass undefined, which Prisma may ignore but is brittle. Build the update payload with filterUndefined to only set explicitly provided fields and to support the “null -> empty string” behavior for description.
- project = await tx.project.update({ - where: { - id: projectFound.id, - }, - data: { - displayName: options.data.display_name, - description: options.data.description === null ? "" : options.data.description, - isProductionMode: options.data.is_production_mode, - logoUrl, - fullLogoUrl, - }, - }); + const updateData = filterUndefined({ + displayName: options.data.display_name, + description: options.data.description === null ? "" : options.data.description, + isProductionMode: options.data.is_production_mode, + logoUrl, + fullLogoUrl, + }); + project = await tx.project.update({ + where: { id: projectFound.id }, + data: updateData, + });
28-37: Add an index on Project.ownerTeamIdI didn’t find any index on
ownerTeamIdin your Prisma schema or migrations. To keep lookups byownerTeamIdperformant at scale, please add one. For example:// in apps/backend/prisma/schema.prisma model Project { id String @id @default(uuid()) // … - ownerTeamId String? @db.Uuid + ownerTeamId String? @db.Uuid @index // … }Then run:
npx prisma migrate dev --name add_ownerTeamId_indexLet me know if you’d like me to draft the migration script.
apps/e2e/tests/backend/endpoints/api/v1/projects.test.ts (2)
1387-1387: Consider using a more descriptive test nameThe test name could better reflect that it verifies team ownership alignment rather than user ownership.
-it("makes sure users own the correct projects after creating a project", async ({ expect }) => { +it("verifies project owner_team_id matches creator's selected team", async ({ expect }) => {
1404-1404: Update test name to reflect new behaviorThe test name should be updated to better describe the actual behavior being tested.
-it("removes a deleted project from a user", async ({ expect }) => { +it("returns CURRENT_PROJECT_NOT_FOUND after project deletion", async ({ expect }) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
apps/backend/prisma/schema.prisma(1 hunks)apps/backend/src/lib/projects.tsx(4 hunks)apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client.tsx(6 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/provision.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth-providers.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/current.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/provision.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/projects.test.ts(11 hunks)apps/e2e/tests/backend/endpoints/api/v1/project-permissions.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/projects.test.ts(21 hunks)apps/e2e/tests/backend/endpoints/api/v1/team-permissions.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/users.test.ts(20 hunks)examples/demo/src/app/gmail/page.tsx(2 hunks)packages/stack-shared/src/interface/crud/projects.ts(2 hunks)packages/template/src/components/selected-team-switcher.tsx(3 hunks)packages/template/src/components/team-switcher.tsx(1 hunks)packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts(1 hunks)packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts(2 hunks)packages/template/src/lib/stack-app/projects/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/demo/src/app/gmail/page.tsx
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/stack-shared/src/interface/crud/projects.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/provision.test.ts
- packages/template/src/components/team-switcher.tsx
- apps/e2e/tests/backend/endpoints/api/v1/team-permissions.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/project-permissions.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/provision.test.ts
- packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
- apps/backend/prisma/schema.prisma
- apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth-providers.test.ts
- packages/template/src/lib/stack-app/projects/index.ts
- packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
- apps/e2e/tests/backend/endpoints/api/v1/internal/projects.test.ts
- apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client.tsx
🧰 Additional context used
🧬 Code Graph Analysis (4)
apps/e2e/tests/backend/endpoints/api/v1/projects.test.ts (1)
apps/e2e/tests/backend/backend-helpers.ts (3)
backendContext(34-56)InternalProjectKeys(75-80)niceBackendFetch(107-165)
apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (1)
packages/template/src/lib/stack-app/projects/index.ts (1)
Project(9-13)
packages/template/src/components/selected-team-switcher.tsx (2)
packages/template/src/components/team-switcher.tsx (1)
TeamSwitcher(49-53)packages/template/src/index.ts (1)
TeamSwitcher(24-24)
apps/backend/src/lib/projects.tsx (2)
apps/backend/src/lib/tenancies.tsx (2)
getSoleTenancyFromProjectBranch(59-66)DEFAULT_BRANCH_ID(17-17)apps/backend/src/prisma-client.tsx (2)
getPrismaClientForTenancy(51-53)globalPrismaClient(31-31)
⏰ 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). (7)
- GitHub Check: restart-dev-and-test
- GitHub Check: all-good
- GitHub Check: setup-tests
- GitHub Check: lint_and_build (latest)
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: Security Check
🔇 Additional comments (13)
packages/template/src/components/selected-team-switcher.tsx (3)
23-23: Prop threading for triggerClassName looks goodNew prop is defined and passed through to TeamSwitcher. No issues.
8-8: Switch to TeamSwitcher import looks correctImporting the new TeamSwitcher component aligns with the refactor. No concerns.
73-73: No changes needed: TeamSwitcher expectsteampropConfirmed that
TeamSwitcherPropsdefines the controlled value asteam, so the existingteam={props.selectedTeam}is correct.
apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/current.test.ts (1)
46-47: Snapshots correctly include owner_team_idThe updated snapshots asserting owner_team_id look consistent across read and patch flows. This aligns with the new team-based ownership model.
Also applies to: 101-102, 172-173
apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (1)
147-155: LGTM: selected_team shape added to client responsesThe added selected_team object and selected_team_id fields look consistent with the new model. Good differentiation between client and server surface areas elsewhere in the file.
Also applies to: 186-194, 270-278
apps/backend/src/lib/projects.tsx (1)
101-116: LGTM: create path sets ownerTeamId and persists tenancyCreate branch correctly sets ownerTeamId and provisions tenancy. Matches the PR’s ownership shift.
apps/e2e/tests/backend/endpoints/api/v1/projects.test.ts (7)
43-43: Team creation is now enabled in the internal projectThe
client_team_creation_enabledfield is nowtruefor the internal project (line 43), whereas it'sfalsein other project configurations (lines 80, 131, 179, etc.). This change appears intentional as part of the team-based ownership migration but worth noting that it enables different behavior for internal vs. regular projects.
104-104: LGTM! owner_team_id consistently added to all project responsesThe addition of
owner_team_idfield to all project responses is consistent throughout the test file, properly reflecting the team-based ownership model introduced in this PR.Also applies to: 154-154, 209-209, 270-270, 325-325, 423-423, 483-483, 529-529, 575-575, 635-635, 808-808, 861-861, 918-918, 970-970, 1037-1037, 1104-1104
1389-1401: Good transition to team-based ownership verificationThe test correctly verifies that the project's
owner_team_idmatches the creator user'sselected_team.id, properly validating the team-based ownership model. The use of internal endpoints with admin access is appropriate for this verification.
1410-1413: Good validation of project listing with internal client endpointThe test correctly uses the internal endpoint with client access to list projects, which is appropriate for the authenticated user context.
1430-1446: Appropriate error handling after project deletionThe test correctly verifies that accessing a deleted project returns
CURRENT_PROJECT_NOT_FOUNDerror, properly validating the deletion behavior in the team-based ownership model.
1467-1474: Good isolation test for project deletionThe test correctly verifies that deleting one user's project doesn't affect another user's projects, properly validating the team-based ownership isolation.
1495-1499: Project.createAndSwitch() no longer returns adminAccessTokenBased on the AI summary,
Project.createAndSwitch()now returnsvoidinstead of returning an object withadminAccessToken. The test correctly adapts to this change by not expecting a return value.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (12)
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts (1)
1143-1145: Breaking change: createProject now requires teamId — ensure all call sites are updatedThis is a deliberate API break. Make sure every invocation passes a teamId. Prior review already flagged the e2e helper needing an update.
Find call sites and check for teamId being supplied:
#!/bin/bash # List all createProject calls with surrounding context rg -n -C3 -S '\.createProject\(' -g '!**/node_modules/**' # Heuristic: calls that construct an object literal without a teamId key rg -n -U -P '\.createProject\(\s*\{(?:(?!teamId\b).)*\}\s*\)' -g '!**/node_modules/**' --type=ts --type=tsxapps/e2e/tests/backend/endpoints/api/v1/users.test.ts (9)
309-316: Same invariant applies here tooAdd the id equality assertion after this response as well.
853-861: Same id equality invariant for this server responseRepeat the equality assertion after this snapshot too.
1687-1696: Same invariant for updated user (by id) responseAdd the id equality assertion for consistency across server responses.
1727-1735: And again after the follow-up GETRepeat the id equality assertion after the follow-up read.
1775-1783: Server “me” update: id equality assertionSame suggestion applies here.
1823-1831: Password update flow: assert id equality tooAdd the equality check here for completeness.
1920-1928: Metadata update flow: id equality assertionSame as above.
1980-1988: Primary email update flow: id equality assertionSame as above.
2046-2054: Primary email removal flow: id equality assertionSame as above.
apps/backend/src/lib/projects.tsx (2)
15-18: Correct choice: tenancy-aware access using internalPrismaUsing getSoleTenancyFromProjectBranch + getPrismaClientForTenancy is the right pattern here, consistent with prior guidance.
112-113: Missingowner_team_idin Neon provisioning handlerThe Neon provisioning route at
apps/backend/src/app/api/latest/integrations/neon/projects/provision/route.tsx
invokescreateOrUpdateProjectWithLegacyConfig({ type: 'create', data: { display_name, description } })without providingowner_team_id. Since the signature now requiresowner_team_id(ornull), this will break at runtime.• apps/backend/src/app/api/latest/integrations/neon/projects/provision/route.tsx (around line 44):
data: { display_name: req.body.display_name, description: "Created with Neon", + owner_team_id: /* appropriate team ID or null */, }Please add the missing
owner_team_idhere. Also verify that other “create” call sites (e.g. internal projects CRUD which spreadsdata, custom integration which already setsowner_team_id: null) correctly supply this field.
🧹 Nitpick comments (12)
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts (1)
1865-1874: Add a defensive runtime guard forteamIdEven though the converter already maps
teamId→owner_team_id(confirmed in packages/template/src/lib/stack-app/projects/index.ts:114), it’s safer to guard against missing values at runtime in JS call sites:• File: packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
• Method:_createProjectSuggested diff:
protected async _createProject( session: InternalSession, - newProject: AdminProjectUpdateOptions & { displayName: string, teamId: string }, + newProject: AdminProjectUpdateOptions & { displayName: string, teamId: string }, ): Promise<AdminOwnedProject> { this._ensureInternalProject(); + if (!newProject.teamId) { + throw new StackAssertionError("teamId is required to create a project"); + } const crud = await this._interface.createProject( adminProjectCreateOptionsToCrud(newProject), session, ); // …apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (6)
186-194: Add an explicit invariant: selected_team_id should equal selected_team.idSmall strengthening: assert that the id in the embedded object matches the top-level selected_team_id. This catches drift if the API ever mismatches fields.
You can append this right after the snapshot:
expect(response.body.selected_team_id).toEqual(response.body.selected_team.id);
270-278: Repeat the id equality invariant after PATCHSame suggestion here to assert selected_team_id === selected_team.id after update. It makes the intent explicit and guards against subtle regressions.
expect(response1.body.selected_team_id).toEqual(response1.body.selected_team.id);
517-520: Remove unused variable “secret”The local variable ‘secret’ is declared but never used in this test.
- const secret = generateSecureRandomString(32);
744-776: Selected team update test: assert selected_team object and add negative casesGreat addition testing the full set→switch→clear flow. Two improvements:
- Assert the embedded selected_team object is present/absent consistently and that its id matches selected_team_id.
- Consider negative cases: setting a team the user isn’t a member of, a team from another project, or an invalid UUID.
Inline assertions you can add now:
const response2 = await niceBackendFetch("/api/v1/users/me", { accessType: "client", method: "PATCH", body: { selected_team_id: team1Id, }, }); expect(response2.body.selected_team_id).toEqual(team1Id); + expect(response2.body.selected_team).toBeDefined(); + expect(response2.body.selected_team.id).toEqual(team1Id); const response3 = await niceBackendFetch("/api/v1/users/me", { accessType: "client", method: "PATCH", body: { selected_team_id: team2Id, }, }); expect(response3.body.selected_team_id).toEqual(team2Id); + expect(response3.body.selected_team).toBeDefined(); + expect(response3.body.selected_team.id).toEqual(team2Id); const response4 = await niceBackendFetch("/api/v1/users/me", { accessType: "client", method: "PATCH", body: { selected_team_id: null, }, }); expect(response4.body.selected_team_id).toEqual(null); + expect(response4.body.selected_team).toEqual(null);If you want, I can draft follow-up tests for:
- selected_team_id set to a random UUID (expect 400/404 with a specific error code)
- selected_team_id set to a team from a different project (expect 400/403/404)
- selected_team_id set to a team where the user isn’t a member (expect 403/404)
Tell me which error semantics you intend (e.g., TEAM_NOT_FOUND vs. INSUFFICIENT_TEAM_ACCESS), and I’ll tailor the assertions.
805-815: Server “me” response: add id equality assertionServer path adds created_at_millis and server_metadata; also assert that selected_team_id equals the embedded id.
expect(response.body.selected_team_id).toEqual(response.body.selected_team.id);
987-996: Reading a user by id: assert selected_team_id === selected_team.idStrengthen with the equality assertion here as well.
expect(response.body.selected_team_id).toEqual(response.body.selected_team.id);apps/e2e/tests/backend/endpoints/api/v1/internal/projects.test.ts (2)
483-524: Add explicit assertions for owner_team_id format and stability post-updateInline snapshots confirm presence, but it’s useful to assert:
- It’s a valid UUID string at runtime, and
- It remains stable across PATCH and subsequent GET for the same project.
You can append to this test:
const uuidRe = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; // after patchResponse assertion expect(typeof patchResponse.body.owner_team_id).toBe("string"); expect(patchResponse.body.owner_team_id).toMatch(uuidRe); // after response (GET) assertion expect(response.body.owner_team_id).toBe(patchResponse.body.owner_team_id);
386-432: Also assert the created owner_team_id matches the listed itemIn this test, you can capture the created project’s owner_team_id and assert it appears in the list (helps catch cross-tenant listing issues):
await Auth.Otp.signIn(); const { createProjectResponse } = await Project.create(); const createdOwnerTeamId = createProjectResponse.body.owner_team_id; const response = await niceBackendFetch("/api/v1/internal/projects", { accessType: "client" }); expect(response.body.items.length).toBeGreaterThan(0); expect(response.body.items.some(p => p.owner_team_id === createdOwnerTeamId)).toBe(true);apps/backend/src/lib/projects.tsx (3)
18-37: Optimize query pattern and short-circuit when no team membershipsMinor improvements:
- Select only the needed field from teams.
- Early return if there are no memberships to avoid the second query.
- Reuse a computed teamIds array for clarity.
Apply:
const teams = await internalPrisma.team.findMany({ where: { tenancyId: internalTenancy.id, teamMembers: { some: { projectUserId: projectUser.id, } } }, - }); - const projectIds = await globalPrismaClient.project.findMany({ + select: { teamId: true }, + }); + if (teams.length === 0) { + return []; + } + const teamIds = teams.map((t) => t.teamId); + const projectIds = await globalPrismaClient.project.findMany({ where: { ownerTeamId: { - in: teams.map((team) => team.teamId), + in: teamIds, }, }, select: { id: true, }, });
112-113: Optional: validate non-null owner_team_id for internal create flowsIf internal flows must always attach a team, consider rejecting null here (leave null only for external/provisioning contexts that can set it later).
28-33: Add missing index on Project.ownerTeamIdI didn’t find any existing index on
ownerTeamIdin your Prisma schema or SQL migrations. Without it, querying projects by team will full-scan the table at scale.• In
apps/backend/prisma/schema.prisma, update theProjectmodel:model Project { id String @id @default(uuid()) ownerTeamId String? @db.Uuid // … other fields … @@index([ownerTeamId]) // ← add this }• Then run
npx prisma migrate dev(or your standard migration workflow) to generate a new migration that creates the index.This will ensure
SELECT … WHERE ownerTeamId IN (…)remains performant as your dataset grows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
apps/backend/prisma/schema.prisma(1 hunks)apps/backend/src/lib/projects.tsx(4 hunks)apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client.tsx(6 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/provision.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth-providers.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/current.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/provision.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/projects.test.ts(11 hunks)apps/e2e/tests/backend/endpoints/api/v1/project-permissions.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/projects.test.ts(21 hunks)apps/e2e/tests/backend/endpoints/api/v1/team-permissions.test.ts(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/users.test.ts(20 hunks)examples/demo/src/app/gmail/page.tsx(2 hunks)packages/stack-shared/src/interface/crud/projects.ts(2 hunks)packages/template/src/components/selected-team-switcher.tsx(3 hunks)packages/template/src/components/team-switcher.tsx(1 hunks)packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts(1 hunks)packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts(2 hunks)packages/template/src/lib/stack-app/projects/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- examples/demo/src/app/gmail/page.tsx
- apps/e2e/tests/backend/endpoints/api/v1/integrations/custom/projects/provision.test.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/e2e/tests/backend/endpoints/api/v1/project-permissions.test.ts
- apps/backend/prisma/schema.prisma
- apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/provision.test.ts
- packages/template/src/components/team-switcher.tsx
- apps/e2e/tests/backend/endpoints/api/v1/team-permissions.test.ts
- packages/template/src/lib/stack-app/projects/index.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/projects/current.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/integrations/neon/oauth-providers.test.ts
- packages/stack-shared/src/interface/crud/projects.ts
- apps/e2e/tests/backend/endpoints/api/v1/projects.test.ts
- apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/new-project/page-client.tsx
- packages/template/src/components/selected-team-switcher.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts (1)
packages/template/src/lib/stack-app/projects/index.ts (2)
AdminProjectUpdateOptions(44-51)AdminOwnedProject(40-42)
⏰ 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). (6)
- GitHub Check: restart-dev-and-test
- GitHub Check: all-good
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: Security Check
🔇 Additional comments (7)
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts (1)
116-116: No action needed for ownerTeamId nullability
AdminProject.ownerTeamId is declared asstring | null, matching the backend’sowner_team_idwhich can benull. The assignmentownerTeamId: data.owner_team_id,is therefore type-safe and requires no change.
apps/e2e/tests/backend/endpoints/api/v1/users.test.ts (1)
147-155: Client “me” response: selected_team shape looks correctGood assertion of the client-visible selected_team fields; server-only fields are excluded as expected.
apps/e2e/tests/backend/endpoints/api/v1/internal/projects.test.ts (1)
99-100: LGTM: owner_team_id is asserted across create/list/update flowsGood coverage making the ownership explicit in all response shapes. This will prevent regressions where the field drops out of any pathway.
Also applies to: 152-153, 208-209, 255-256, 316-317, 379-380, 425-426, 477-478, 518-519, 569-570, 623-624
apps/backend/src/lib/projects.tsx (4)
65-66: Expose owner_team_id in project readsThe projection correctly maps ownerTeamId -> owner_team_id for API consumers.
82-83: Create path type now requires owner_team_id: correct for team ownership modelGood type-level enforcement. This prevents accidental reintroduction of user-based ownership.
227-253: Sensible defaults for RBAC in createPreseeding team_member/team_admin permissions and default mappings on create avoids broken team flows out of the box.
31-32: No changes needed:teamIdis the correct Team identifier
The Prisma schema’sTeammodel definesteamId(alongsidetenancyId) as its primary key, and there is no standaloneidfield. Usingteam.teamIdin the IN clause is therefore correct—no adjustments required.
2 participants
Important
Introduces team-based project ownership, refactoring existing user-based model, and updates UI, backend, and tests to support this feature.
This description was created by
for 0f6f12b. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements
Tests