reconcille duplicate tables#654
Conversation
✅ Deploy Preview for agent-native-meeting-notes ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-design ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-images canceled.
|
✅ Deploy Preview for agent-native-scheduling ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-voice ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Deploy Preview for nutritrack-daily-calories canceled.
|
✅ Deploy Preview for agent-native-starter canceled.
|
✅ Deploy Preview for agent-native-dispatch canceled.
|
✅ Deploy Preview for agent-native-mail canceled.
|
✅ Deploy Preview for agent-native-forms canceled.
|
✅ Deploy Preview for agent-native-content canceled.
|
✅ Deploy Preview for agent-native-calendar canceled.
|
✅ Deploy Preview for agent-native-slides canceled.
|
✅ Deploy Preview for agent-native-recruiting canceled.
|
✅ Deploy Preview for agent-native-macros canceled.
|
✅ Deploy Preview for agent-native-videos canceled.
|
✅ Deploy Preview for agent-native-issues canceled.
|
There was a problem hiding this comment.
Builder reviewed your changes — looks good ✅
Review Details
Code Review Summary
This PR consolidates duplicate org table schemas, migrating from better-auth's parallel organization/member/invitation tables to the framework's organizations/org_members/org_invitations tables. The refactoring is architecturally sound and reduces complexity, but critical bugs in role mapping and data migration will cause complete org system failure on deploy.
Critical Blocking Issues
🔴 Role hierarchy completely broken — Both the org creator path (create-organization.ts) and the migration backfill (server/plugins/db.ts lines 269 & 758) assign "admin" role instead of "owner". This means no organizations will have any "owner" role users. The framework handlers explicitly check role === "owner" for critical operations (remove-member, update-member-role), which will never succeed. Workspace owners and new org creators will be indistinguishable from admins, violating the entire hierarchy. Found by all 3 code-review agents.
🔴 Existing org memberships disappear on deploy — The PR removes the fallback to better-auth's member table in recordings.ts (lines 73-79) without any migration that copies existing organization/member/invitation data into the framework tables. After deploy, getOrganizationRoleForEmail() will return null, causing org-gated actions to fail with access denied for all existing users.
🔴 Admin invites silently downgraded to member — The migration in db.ts (lines 1020-1049) copies old invites rows but doesn't preserve the role field. When invitations are accepted, the code checks if (inv.role === "admin") which is always false (role is null), silently downgrading all admin invites to member on acceptance.
Data Integrity Issues
🟡 Email case mismatch in invite-member.ts (line 107) — Email is lowercased for queries (line 80) but inserted with original case (line 107). UNIQUE(org_id, email) constraint is case-sensitive, allowing duplicate invitations for the same email with different case variations.
🟡 Email case mismatch in accept-invite.ts — Email is lowercased for existence check but inserted with original case. Can create duplicate org_members rows for the same email with different case variations.
🟡 Email case mismatch in context.ts createOrganization — Function stores email parameter as-is without normalizing to lowercase, but all queries use LOWER(). Can bypass UNIQUE constraints and create duplicate rows.
🟡 Session org context broken after auth plugin removal (better-auth-instance.ts line 821) — Dropping Better Auth's organization() plugin leaves session.orgId unset, but poll events and custom routes still depend on it. Org-scoped updates will silently lose their organization context.
🟡 Role hierarchy weakened in update-member-role.ts — The new action only protects "owner" rows, letting any admin promote members to admin or demote peer admins. This bypasses the framework's owner/admin distinction.
Risk Assessment: Standard (org/auth refactoring) | Browser Testing: Deferred until code issues are fixed and re-tested.
|
thanks @shomix ! |
No description provided.