Account linking#53
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a self‑service account-merge feature (frontend + backend + email), modularizes MCP tooling into register* modules with caching and tests, introduces linked‑accounts UI and PIN input, updates DB schema and admin tooling/scripts, and adds route guards and centralized account‑linking error handling. Changes
sequenceDiagram
autonumber
actor User
participant Frontend as Web Frontend
participant Server as Workers API
participant DB as D1 Database
participant Email as Email Service
User->>Frontend: Submit target email (initiate)
Frontend->>Server: POST /api/accounts/merge/initiate { targetEmail }
activate Server
Server->>DB: lookup users & linked accounts
Server->>DB: create verification record (mergeToken + code + expiry)
Server->>Email: send verification email (HTML/text)
Email-->>User: deliver code
Server-->>Frontend: { mergeToken, preview }
deactivate Server
User->>Frontend: Enter code (verify)
Frontend->>Server: POST /api/accounts/merge/verify { mergeToken, code }
activate Server
Server->>DB: validate code, mark verified
Server-->>Frontend: success
deactivate Server
User->>Frontend: Confirm merge (complete)
Frontend->>Server: POST /api/accounts/merge/complete { mergeToken }
activate Server
Server->>DB: validate token & verified
Server->>DB: transfer non-duplicate accounts, reassign projects/memberships/media
Server->>DB: merge subscriptions, delete secondary user, cleanup
Server-->>Frontend: success + mergedProviders
deactivate Server
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
corates | 89c60f3 | Dec 15 2025, 09:41 PM |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/workers/src/routes/admin.js (1)
106-116: Duplicated search filter logic between count query and main query.The search filter logic in the count query (lines 106-116) doesn't include all the same fields as the main query (lines 93-103). The main query filters on
name,displayName, andusername, but the count query only filters onname. This will cause pagination to report incorrect totals when users match ondisplayNameorusername.// Get total count for pagination const [totalResult] = await db .select({ count: count() }) .from(user) .where( search ? or( like(sql`lower(${user.email})`, `%${search.toLowerCase()}%`), like(sql`lower(${user.name})`, `%${search.toLowerCase()}%`), + like(sql`lower(${user.displayName})`, `%${search.toLowerCase()}%`), + like(sql`lower(${user.username})`, `%${search.toLowerCase()}%`), ) : undefined, );Consider extracting the search condition into a shared helper to avoid duplication:
const buildSearchCondition = (searchTerm) => { if (!searchTerm) return undefined; const pattern = `%${searchTerm.toLowerCase()}%`; return or( like(sql`lower(${user.email})`, pattern), like(sql`lower(${user.name})`, pattern), like(sql`lower(${user.displayName})`, pattern), like(sql`lower(${user.username})`, pattern), ); };
🧹 Nitpick comments (12)
packages/mcp/server.js (1)
227-286: Well-structured tool implementation.The
better_auth_docstool is cleanly implemented with good error handling for both HTTP errors and exceptions. The descriptive parameter documentation and helpful guidance in response messages are nice touches.Optional consideration: The
fetchcall has no timeout, which could cause the tool to hang if the external server is unresponsive. Consider adding anAbortControllerwith a timeout:async ({ path: docPath }) => { try { // Build the URL - if no path provided, fetch the index const url = docPath ? `${BETTER_AUTH_BASE_URL}/llms.txt/${docPath}` : `${BETTER_AUTH_BASE_URL}/llms.txt`; - const response = await fetch(url); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); + + const response = await fetch(url, { signal: controller.signal }); + clearTimeout(timeoutId);packages/web/src/components/admin-ui/UserTable.jsx (1)
28-33: Consider centralizing provider metadata
PROVIDER_INFOis likely to be reused across the account-linking UI (settings, merge dialogs, etc.). To avoid drift when adding/removing providers, consider moving this map into a shared constants/module (e.g.,authProviders.js) and importing it here and in related components.As per coding guidelines, favor shared config over repeating hardcoded values.
packages/workers/migrations/0001_init.sql (1)
70-70: Consider adding an index forunlinkedAtif querying unlinked accounts.The new
unlinkedAtcolumn supports soft-delete/grace period semantics for account unlinking. If your backend queries for accounts whereunlinkedAt IS NOT NULLor filters by unlink timestamp ranges, consider adding an index:CREATE INDEX idx_account_unlinkedAt ON account(unlinkedAt);Only add this if you anticipate frequent queries filtering or ordering by this column.
Based on coding guidelines: all migrations should go in this single file (0001_init.sql).
packages/web/src/components/profile-ui/AccountProviderCard.jsx (1)
37-49: Consider using user's locale for date formatting.The date is hardcoded to
'en-US'locale. Per coding guidelines, prefer using config files rather than hardcoding values. Consider using the user's browser locale or a configurable app-wide setting.const linkedDate = createMemo(() => { if (!props.account.createdAt) return null; const date = new Date( typeof props.account.createdAt === 'number' ? props.account.createdAt * 1000 // Unix timestamp in seconds : props.account.createdAt, ); - return date.toLocaleDateString('en-US', { + return date.toLocaleDateString(undefined, { // Uses browser's locale month: 'short', day: 'numeric', year: 'numeric', }); });packages/web/src/components/profile-ui/MergeAccountsDialog.jsx (1)
1-384: Consider extracting step components if this grows further.At ~384 lines, this component exceeds the 200-300 line guideline. However, since it's a cohesive multi-step wizard with clear separation between handlers and UI steps, the current structure is acceptable. If more steps or complexity are added, consider extracting each step into separate components under a
MergeAccountsDialog/folder.packages/web/src/components/profile-ui/LinkedAccountsSection.jsx (1)
90-95: Consider a more robust approach than setTimeout.The 100ms delay is a workaround for Zag dialog initialization timing. While this works, it could cause flaky behavior if initialization takes longer. Consider using
queueMicrotaskor ensuring the Dialog component handles this internally.- setTimeout(() => { - setShowMergeDialog(true); - }, 100); + queueMicrotask(() => { + setShowMergeDialog(true); + });packages/workers/src/routes/account-merge.js (2)
43-45: Consider using crypto.getRandomValues for code generation.
Math.random()is not cryptographically secure. While this is a verification code with short expiry, usingcrypto.getRandomValueswould be more robust.function generateCode() { - return Math.floor(100000 + Math.random() * 900000).toString(); + const array = new Uint32Array(1); + crypto.getRandomValues(array); + return String(100000 + (array[0] % 900000)).padStart(6, '0'); }
265-383: Acknowledge partial merge risk due to lack of transactions.The code correctly notes that D1 doesn't support transactions. If a failure occurs mid-merge (e.g., after moving accounts but before deleting the secondary user), the database will be in an inconsistent state. Consider:
- Adding idempotency markers to allow retry/recovery
- Ordering operations from least to most destructive (current order is reasonable)
- Adding a status field to track merge progress
For now, ensure monitoring/alerting is in place for the
[AccountMerge] Error during merge:log message.packages/web/src/api/account-merge.js (1)
8-8: Solid API surface; consider a small fetch helper + defensive JSON parsingThe overall structure looks good: consistent base URL, credentialed requests, and clear error messages returned as thrown
Errors. Route paths and payload shapes align with the described backend flow.Two optional improvements you might consider:
Factor out common fetch/error logic
All five functions repeat the same pattern (fetch→response.json()→!response.okguard). A tiny helper would reduce duplication and keep future behavior changes (e.g., headers, error mapping) in one place:+async function request(path, options = {}) { + const response = await fetch(`${API_BASE}${path}`, { + credentials: 'include', + headers: { 'Content-Type': 'application/json', ...(options.headers || {}) }, + ...options, + }); + + // Be defensive if a route ever returns non‑JSON or 204 + let data; + try { + data = await response.json(); + } catch { + data = {}; + } + + if (!response.ok) { + throw new Error(data.error || 'Request failed'); + } + + return data; +} + export async function initiateMerge(targetEmail) { - const response = await fetch(`${API_BASE}/api/accounts/merge/initiate`, { - method: 'POST', - credentials: 'include', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ targetEmail }), - }); - - const data = await response.json(); - - if (!response.ok) { - throw new Error(data.error || 'Failed to initiate merge'); - } - - return data; + return request('/api/accounts/merge/initiate', { + method: 'POST', + body: JSON.stringify({ targetEmail }), + }); }You could similarly adapt
verifyMergeCode,completeMerge,getMergeStatus, andcancelMergeto userequest.Clarify expectations around JSON responses
Right now every function assumesresponse.json()will succeed, even on error. If any endpoint ever returns non‑JSON (e.g., empty 204, HTML error), these will throw before reaching your!response.okbranch. If the backend always returns JSON this is fine; otherwise the defensivetry/catchpattern above will keep errors surfaced as cleanErrors.These are quality-of-life improvements rather than blockers; behavior is otherwise sound.
Also applies to: 15-30, 38-53, 60-75, 82-98, 105-120
ACCOUNT_LINKING_PLAN.md (3)
61-94: Add languages to fenced code blocks to satisfy markdownlintSeveral of the diagram blocks use bare triple backticks without a language, which triggers
MD040inmarkdownlint-cli2. Since these are ASCII flow diagrams rather than code, you can tag them with a neutral language liketextto keep rendering the same while appeasing the linter.For example, for one of the flow diagrams:
-``` +```text ┌─────────────────────────────────────────────────────────────────┐ │ FLOW DIAGRAM │ ... -``` +```Applying this pattern to the diagram-style fences around the flows (e.g., roughly lines 61–94, 131–160, 165–175, 197–221, 248–290, 323–363, 399–435, 451–480, 495–531, 554–584, 595–624, 639–656, 812–846, and 1147–1162) should clear the MD040 warnings without changing the document semantics.
Also applies to: 131-160, 165-175, 197-221, 248-290, 323-363, 399-435, 451-480, 495-531, 554-584, 595-624, 639-656, 787-795, 812-846, 1147-1162
235-238: Replace bare ORCID URL with a markdown link to satisfy MD034The ORCID profile reference is currently a bare URL, which
markdownlintflags (MD034). Converting it to a proper markdown link keeps the text readable and resolves the warning:-- Consider linking to ORCID profile: `https://orcid.org/0000-0001-2345-6789` +- Consider linking to the ORCID profile: +- [https://orcid.org/0000-0001-2345-6789](https://orcid.org/0000-0001-2345-6789)You can pick any link text you prefer; the key is avoiding the bare URL form.
1411-1450: Update implementation checklist to match current implementation statusThe checklist at the end still marks several items as unchecked (e.g., creating
LinkedAccountsSection.jsx,AccountProviderCard.jsx, andaccount-linking-errors.js), but this PR appears to implement those artifacts already. Keeping the checklist in sync with reality will prevent confusion for future maintainers using this document as a source of truth.Recommend reviewing the checklist and checking off items that this PR actually delivers (or adjusting wording if the implementation differs slightly from the original plan).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
.github/copilot-instructions.md(2 hunks)ACCOUNT_LINKING_PLAN.md(1 hunks)docs/better-auth/llms.txt(0 hunks)mcp.json(1 hunks)package.json(1 hunks)packages/mcp/server.js(2 hunks)packages/web/src/Routes.jsx(1 hunks)packages/web/src/api/account-merge.js(1 hunks)packages/web/src/components/admin-ui/UserTable.jsx(5 hunks)packages/web/src/components/auth-ui/AuthGuard.jsx(1 hunks)packages/web/src/components/auth-ui/AuthLayout.jsx(1 hunks)packages/web/src/components/profile-ui/AccountProviderCard.jsx(1 hunks)packages/web/src/components/profile-ui/LinkedAccountsSection.jsx(1 hunks)packages/web/src/components/profile-ui/MergeAccountsDialog.jsx(1 hunks)packages/web/src/components/profile-ui/SettingsPage.jsx(2 hunks)packages/web/src/components/zag/PinInput.jsx(1 hunks)packages/web/src/lib/account-linking-errors.js(1 hunks)packages/workers/migrations/0001_init.sql(1 hunks)packages/workers/package.json(1 hunks)packages/workers/scripts/make-admin-local.mjs(1 hunks)packages/workers/src/auth/config.js(1 hunks)packages/workers/src/auth/emailTemplates.js(1 hunks)packages/workers/src/index.js(2 hunks)packages/workers/src/routes/account-merge.js(1 hunks)packages/workers/src/routes/admin.js(3 hunks)
💤 Files with no reviewable changes (1)
- docs/better-auth/llms.txt
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
**/*.{js,jsx,ts,tsx}: For UI icons, use thesolid-iconslibrary or SVGs only. Do not use emojis.
Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Prefer using config files rather than hardcoding values
Each file should handle one coherent responsibility
Group related components in subdirectories with anindex.jsbarrel export
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/web/src/components/auth-ui/AuthLayout.jsxpackages/workers/src/auth/config.jspackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/SettingsPage.jsxpackages/workers/src/routes/account-merge.jspackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/zag/PinInput.jsxpackages/workers/src/index.jspackages/web/src/components/profile-ui/MergeAccountsDialog.jsxpackages/web/src/components/admin-ui/UserTable.jsxpackages/workers/src/auth/emailTemplates.jspackages/web/src/lib/account-linking-errors.jspackages/workers/src/routes/admin.jspackages/web/src/api/account-merge.jspackages/web/src/Routes.jsxpackages/mcp/server.js
**/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/components/**/*.{jsx,tsx}: Keep component files small, focused, and modular, aiming for ~200-300 lines max per component file
When a component exceeds ~300 lines, extract sub-components into a folder (e.g.,ComponentName/withindex.jsxand helper components)
Move complex logic from component files into separate utility files or primitives
In SolidJS components, access props directly from the props object or wrap them in a function instead of destructuring to maintain reactivity
UsecreateMemoto compute derived values based on props or state in SolidJS components
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/web/src/components/auth-ui/AuthLayout.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/SettingsPage.jsxpackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/zag/PinInput.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsxpackages/web/src/components/admin-ui/UserTable.jsx
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursorrules)
Use Solid's
createStorefor complex state or state objects in SolidJS components
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/web/src/components/auth-ui/AuthLayout.jsxpackages/workers/src/auth/config.jspackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/SettingsPage.jsxpackages/workers/src/routes/account-merge.jspackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/zag/PinInput.jsxpackages/workers/src/index.jspackages/web/src/components/profile-ui/MergeAccountsDialog.jsxpackages/web/src/components/admin-ui/UserTable.jsxpackages/workers/src/auth/emailTemplates.jspackages/web/src/lib/account-linking-errors.jspackages/workers/src/routes/admin.jspackages/web/src/api/account-merge.jspackages/web/src/Routes.jsxpackages/mcp/server.js
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{jsx,tsx}: Use responsive design principles for UI components
Keep files small, focused, and modular. Extract sub-modules into a folder (e.g.,ComponentName/withindex.jsxand helper components). Move complex logic into separate utility files or primitives. Split large forms into section components.
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/web/src/components/auth-ui/AuthLayout.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/SettingsPage.jsxpackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/zag/PinInput.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsxpackages/web/src/components/admin-ui/UserTable.jsxpackages/web/src/Routes.jsx
packages/web/src/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/components/**/*.{jsx,tsx}: When implementing UI components, use zag.js from the Zag.js documentation at /docs/zag
Reuse existing Zag components before creating new ones or using plain HTML inputs. Existing components include: Avatar, Checkbox, Collapsible, Dialog, FileUpload, FloatingPanel, PasswordInput, Splitter, Switch, Tabs, Toast, and Tooltip
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/web/src/components/auth-ui/AuthLayout.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/SettingsPage.jsxpackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/zag/PinInput.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsxpackages/web/src/components/admin-ui/UserTable.jsx
packages/web/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/**/*.{jsx,tsx}: Do NOT prop-drill application state. Shared or cross-feature state must live in external stores underpackages/web/src/stores/or relative to the component file. Import stores directly where needed instead of passing values through multiple components.
Components should receive at most 1–5 props, and only for local configuration, not shared state
If a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context (when scoped to a feature)
Do not destructure props in SolidJS components as it breaks reactivity. Instead, access props directly from the props object (e.g.,props.name) or wrap them in a function (e.g.,() => props.name)
UsecreateMemofor derived values based on props or state to ensure reactive updates in SolidJS
Use Solid'screateStorefor complex state objects in SolidJS for better performance and reactivity
Components should be lean and focused. Do not implement business logic; move that into stores, utilities, or primitives
Never have a component act as a 'God component' coordinating multiple large concerns
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/web/src/components/auth-ui/AuthLayout.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/SettingsPage.jsxpackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/zag/PinInput.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsxpackages/web/src/components/admin-ui/UserTable.jsxpackages/web/src/Routes.jsx
packages/web/src/components/zag/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place new zag components into
packages/web/src/components/zag/*files
Files:
packages/web/src/components/zag/PinInput.jsx
packages/workers/migrations/0001_init.sql
📄 CodeRabbit inference engine (.cursorrules)
All database migrations should go in a single file:
packages/workers/migrations/0001_init.sqlWhen adding new tables or schema changes, edit the existing 0001_init.sql file directly rather than creating new migration files
Files:
packages/workers/migrations/0001_init.sql
packages/workers/migrations/*.sql
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All database migrations should go in a single file:
packages/workers/migrations/0001_init.sql. Do NOT create separate migration files.
Files:
packages/workers/migrations/0001_init.sql
🧠 Learnings (18)
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Components should be lean and focused. Do not implement business logic; move that into stores, utilities, or primitives
Applied to files:
packages/web/src/components/auth-ui/AuthLayout.jsx
📚 Learning: 2025-12-15T16:38:07.301Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-15T16:38:07.301Z
Learning: Applies to **/components/**/*.{jsx,tsx} : Move complex logic from component files into separate utility files or primitives
Applied to files:
packages/web/src/components/auth-ui/AuthLayout.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Do not destructure props in SolidJS components as it breaks reactivity. Instead, access props directly from the props object (e.g., `props.name`) or wrap them in a function (e.g., `() => props.name`)
Applied to files:
packages/web/src/components/auth-ui/AuthLayout.jsxpackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/zag/PinInput.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Use Solid's `createStore` for complex state objects in SolidJS for better performance and reactivity
Applied to files:
packages/web/src/components/auth-ui/AuthLayout.jsxpackages/web/src/components/auth-ui/AuthGuard.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Reuse existing Zag components before creating new ones or using plain HTML inputs. Existing components include: Avatar, Checkbox, Collapsible, Dialog, FileUpload, FloatingPanel, PasswordInput, Splitter, Switch, Tabs, Toast, and Tooltip
Applied to files:
packages/web/src/components/profile-ui/SettingsPage.jsxpackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/zag/PinInput.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsxpackages/web/src/components/admin-ui/UserTable.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Use `createMemo` for derived values based on props or state to ensure reactive updates in SolidJS
Applied to files:
packages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : If a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context (when scoped to a feature)
Applied to files:
packages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/zag/PinInput.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
📚 Learning: 2025-12-15T16:38:07.301Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-15T16:38:07.301Z
Learning: Applies to **/*.{jsx,tsx,js,ts} : Use Solid's `createStore` for complex state or state objects in SolidJS components
Applied to files:
packages/web/src/components/auth-ui/AuthGuard.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : When implementing UI components, use zag.js from the Zag.js documentation at /docs/zag
Applied to files:
packages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/zag/PinInput.jsx.github/copilot-instructions.md
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/components/zag/**/*.{jsx,tsx} : Place new zag components into `packages/web/src/components/zag/*` files
Applied to files:
packages/web/src/components/zag/PinInput.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : For UI icons, use the `solid-icons` library or SVGs only. Do not use emojis.
Applied to files:
packages/web/src/components/admin-ui/UserTable.jsx
📚 Learning: 2025-12-15T16:38:07.301Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-15T16:38:07.301Z
Learning: Reuse existing Zag components: Checkbox, Collapsible, Dialog, FileUpload, PasswordInput, Splitter, Switch, Tabs, Toast, and Tooltip from `packages/web/src/components/zag/`
Applied to files:
packages/web/src/components/admin-ui/UserTable.jsx.github/copilot-instructions.md
📚 Learning: 2025-12-15T16:38:07.301Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-15T16:38:07.301Z
Learning: When implementing UI components, use zag.js and reference the Zag.js documentation in `docs/zag/llms.txt`
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/workers/migrations/*.sql : All database migrations should go in a single file: `packages/workers/migrations/0001_init.sql`. Do NOT create separate migration files.
Applied to files:
.github/copilot-instructions.mdpackages/workers/migrations/0001_init.sql
📚 Learning: 2025-12-15T16:38:07.301Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-15T16:38:07.301Z
Learning: Applies to packages/workers/migrations/0001_init.sql : All database migrations should go in a single file: `packages/workers/migrations/0001_init.sql`
Applied to files:
.github/copilot-instructions.mdpackages/workers/migrations/0001_init.sql
📚 Learning: 2025-12-15T16:38:07.301Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-15T16:38:07.301Z
Learning: Do NOT create separate migration files (e.g., 0002_xxx.sql) since this project is not yet in production
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/workers/migrations/0001_init.sql : When adding new tables or schema changes, edit the existing 0001_init.sql file directly rather than creating new migration files
Applied to files:
packages/workers/migrations/0001_init.sql
📚 Learning: 2025-12-15T16:38:07.301Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-15T16:38:07.301Z
Learning: When adding new tables or schema changes, edit the existing 0001_init.sql file directly
Applied to files:
packages/workers/migrations/0001_init.sql
🧬 Code graph analysis (14)
packages/web/src/components/profile-ui/AccountProviderCard.jsx (1)
packages/web/src/components/zag/Tooltip.jsx (1)
Tooltip(5-37)
packages/web/src/components/profile-ui/LinkedAccountsSection.jsx (2)
packages/web/src/lib/account-linking-errors.js (4)
error(65-65)parseOAuthError(64-101)message(94-94)getLinkErrorMessage(48-53)packages/web/src/components/profile-ui/AccountProviderCard.jsx (1)
AccountProviderCard(25-131)
packages/web/src/components/profile-ui/SettingsPage.jsx (2)
packages/web/src/components/profile-ui/LinkedAccountsSection.jsx (1)
LinkedAccountsSection(54-353)packages/web/src/components/profile-ui/GoogleDriveSettings.jsx (1)
GoogleDriveSettings(14-109)
packages/workers/src/routes/account-merge.js (4)
packages/workers/src/middleware/auth.js (1)
requireAuth(34-53)packages/workers/src/db/client.js (1)
createDb(9-11)packages/workers/src/auth/email.js (1)
createEmailService(21-110)packages/workers/src/auth/emailTemplates.js (2)
getAccountMergeEmailHtml(151-177)getAccountMergeEmailText(179-194)
packages/web/src/components/auth-ui/AuthGuard.jsx (3)
packages/web/src/components/Navbar.jsx (1)
Navbar(8-189)packages/web/src/components/HomePage.jsx (1)
HomePage(19-221)packages/web/src/api/better-auth-store.js (1)
createBetterAuthStore(12-668)
packages/web/src/components/zag/PinInput.jsx (2)
packages/web/src/components/zag/PasswordInput.jsx (8)
props(8-8)PasswordInput(6-56)passwordInput(22-22)props(7-7)props(9-9)props(12-12)password(18-18)e(40-40)packages/web/src/components/zag/Checkbox.jsx (1)
Checkbox(22-83)
packages/workers/src/index.js (1)
packages/workers/src/routes/account-merge.js (1)
accountMergeRoutes(32-32)
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx (3)
packages/web/src/api/account-merge.js (4)
initiateMerge(15-30)verifyMergeCode(38-53)completeMerge(60-75)cancelMerge(105-120)packages/web/src/components/zag/Dialog.jsx (3)
Dialog(18-92)details(30-30)details(123-123)packages/web/src/components/zag/PinInput.jsx (1)
PinInput(5-54)
packages/web/src/components/admin-ui/UserTable.jsx (1)
packages/web/src/components/zag/Tooltip.jsx (1)
Tooltip(5-37)
packages/workers/scripts/make-admin-local.mjs (4)
packages/workers/src/index.js (1)
result(55-55)packages/workers/src/db/schema.js (2)
user(103-103)user(37-37)packages/workers/src/routes/users.js (1)
c(184-253)packages/workers/src/routes/members.js (2)
c(112-237)c(243-300)
packages/workers/src/auth/emailTemplates.js (2)
packages/workers/src/auth/templates.js (3)
getEmailVerificationSuccessPage(5-104)getEmailVerificationErrorPage(196-284)getEmailVerificationFailurePage(106-194)packages/workers/src/auth/email.js (1)
sendMagicLink(91-101)
packages/workers/src/routes/admin.js (1)
packages/workers/src/db/schema.js (3)
account(42-58)account(42-58)user(103-103)
packages/web/src/api/account-merge.js (2)
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx (2)
targetEmail(37-37)mergeToken(39-39)packages/workers/src/routes/account-merge.js (1)
mergeToken(99-99)
packages/web/src/Routes.jsx (2)
packages/web/src/config/api.js (2)
BASEPATH(13-13)BASEPATH(13-13)packages/web/src/components/auth-ui/AuthGuard.jsx (1)
AuthGuard(5-69)
🪛 LanguageTool
ACCOUNT_LINKING_PLAN.md
[uncategorized] ~101-~101: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... | Full page redirect | | 3...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~1252-~1252: The words ‘explain’ and ‘explanation’ are quite similar. Consider replacing ‘explain’ with a different word.
Context: ... 2025" - Disabled unlink buttons should explain why: aria-describedby pointing to exp...
(VERB_NOUN_SENT_LEVEL_REP)
🪛 markdownlint-cli2 (0.18.1)
ACCOUNT_LINKING_PLAN.md
61-61: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
131-131: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
164-164: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
196-196: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
225-225: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
247-247: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
323-323: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
398-398: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
450-450: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
494-494: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
553-553: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
594-594: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
638-638: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
787-787: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
812-812: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1146-1146: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1251-1251: Bare URL used
(MD034, no-bare-urls)
1311-1311: Bare URL used
(MD034, no-bare-urls)
⏰ 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). (1)
- GitHub Check: Workers Builds: corates
🔇 Additional comments (37)
mcp.json (1)
1-1: Empty configuration file - is this intentional?This file contains only an empty JSON object. If this is a placeholder for MCP configuration, consider adding a comment in the PR description or removing it until actual configuration is needed.
.github/copilot-instructions.md (2)
34-34: LGTM!Clear instruction to use the MCP tool for Better-Auth documentation, aligning with the new
better_auth_docstool added to the MCP server.
68-68: LGTM!Good addition to centralize documentation access through the Corates MCP.
packages/mcp/server.js (1)
69-69: LGTM!Version bump to 1.3.0 is appropriate for the new
better_auth_docstool addition.packages/web/src/components/admin-ui/UserTable.jsx (2)
15-16: Good use of icon + Zag Tooltip primitivesImporting
FiMailfor the fallback and reusing the shared ZagTooltipcomponent is consistent with the icon and component guidelines for this repo.Based on learnings, this correctly reuses existing Zag components and solid-icons.
Also applies to: 26-27
148-150: Table header + fallback colspan update look consistentAdding the “Providers” header and updating the “No users found” row to
colspan='6'correctly keeps the header/body structure aligned with the new column.Also applies to: 167-167
package.json (1)
30-31: LGTM!The new local admin script follows the existing pattern and properly delegates to the workers package.
packages/web/src/components/auth-ui/AuthLayout.jsx (1)
1-7: LGTM! Excellent separation of concerns.The simplified
AuthLayoutnow focuses solely on layout presentation, delegating authentication logic toAuthGuardat the route level. This follows the Single Responsibility Principle and makes the component more maintainable.As per coding guidelines: Components should be lean and focused, moving business logic into stores, utilities, or primitives.
packages/web/src/lib/account-linking-errors.js (2)
48-53: Message fallback logic is correct.The
getLinkErrorMessagefunction always returns a string (never null), using a generic fallback message for unknown error codes. This is appropriate for this helper function.The
parseOAuthErrorfunction on line 99 correctly returnsmessage ?? nullto preserve the null values for silent errors (like user cancellations) that are explicitly set tonullin theLINK_ERROR_MESSAGESmap.
64-101: Robust OAuth error parsing implementation.The error parsing logic properly normalizes various error code formats, maps them to standardized internal codes, and handles silent errors appropriately. The comprehensive error mappings cover common OAuth and Better Auth scenarios.
packages/workers/package.json (2)
19-19: LGTM!The new local admin script follows the existing pattern for the production script.
30-30: The review comment incorrectly attributes Tailwind CSS breaking changes to Zod. The listed changes (e.g.,text-3xl!,bg-blue-500/20,bg-(--brand-color)) are Tailwind CSS syntax updates, not Zod schema validation library changes.The codebase's Zod 4.1.13 usage is compatible: it uses the
errorparameter in.enum(),.refine()withmessage,.safeParse(), and.optional()—all standard Zod 4 patterns. No deprecated APIs (.flatten(),.format(),.nonstrict()) are present.No compatibility verification is needed for this dependency upgrade.
Likely an incorrect or invalid review comment.
packages/workers/src/index.js (2)
28-28: LGTM!Import follows the established pattern for route modules.
174-176: Account merge routes are properly protected with authentication middleware.The
/api/accounts/mergeendpoints applyrequireAuthmiddleware at the router level (accountMergeRoutes.use('*', requireAuth)), ensuring all routes—/initiate,/verify,/complete, and/cancel—require authentication. Each route handler additionally retrieves the authenticated user viagetAuth(c), confirming the user context is available and protected.packages/web/src/components/profile-ui/SettingsPage.jsx (4)
20-20: LGTM!Import follows proper module conventions.
147-149: Good placement for account linking section.Positioning the
LinkedAccountsSectionbefore the Integrations section creates a logical flow: authentication providers → third-party integrations → security settings.
167-170: Smart UX enhancement withdata-sectionattribute.The
data-section="security"attribute enables the LinkedAccountsSection to scroll users directly to the password/credential section when they need to add a password before unlinking accounts. This creates a seamless user experience.
150-160: Clear section naming and description.Renaming from "Connected Services" to "Integrations" with an explanatory description provides better clarity about the purpose of this section (third-party services) vs. the new Linked Accounts section (authentication providers).
packages/workers/src/auth/config.js (1)
137-148: Account unlinking safeguards are properly enforced.The configuration's
allowUnlinkingAll: truesetting is safe because:
Last account protection: Better Auth prevents unlinking a user's final sign-in method. The codebase enforces this with
canUnlink()checks (preventing the unlink button from displaying) and returnsCANNOT_UNLINK_ONLY_ACCOUNTerror if attempted. The comment about magic link fallback is correct—email verification is an additional fallback.
allowDifferentEmails: trueis an intentional feature with proper safeguards: email mismatches triggerEMAIL_DOESNT_MATCHerrors, and the UI requires an active session (allowDifferentEmailsrequires prior authentication per the comment).No action required—the implementation includes sufficient protections.
packages/workers/src/routes/admin.js (2)
121-151: LGTM!The linked accounts fetching logic correctly handles the empty array edge case and efficiently batches the query for all users in the result set. The reduce operation for grouping accounts by userId is clean and appropriate.
208-224: LGTM!The linked accounts query for the user details endpoint is clean and correctly ordered by
createdAtdescending. The response structure with the newaccountsfield aligns with the admin UI requirements.packages/web/src/components/zag/PinInput.jsx (2)
35-53: LGTM with minor suggestions.The component implementation follows the Zag.js patterns. A few optional improvements:
- The
requiredattribute on each input may be redundant since Zag'sgetInputPropsalready handles this based on the machine config.- Consider adding
focus-visible:variants for better keyboard navigation accessibility.These are minor and don't block the implementation.
20-31: Refactor to use Zag's recommendedsplitPropspattern for reactive prop handling.The current configuration passes evaluated prop values (
required()) instead of accessors. Zag.js documentation recommends usingsplitPropsto separate machine-driven props from local props, then passing accessors directly:-import { createMemo, createUniqueId, mergeProps } from 'solid-js'; +import { createMemo, createUniqueId, splitProps } from 'solid-js'; export default function PinInput(props) { - const merged = mergeProps( - { - required: true, - otp: true, - autoComplete: 'one-time-code', - }, - props, - ); - - const required = () => merged.required; - const otp = () => merged.otp; - const autoComplete = () => merged.autoComplete; - const isError = () => merged.isError; + const [machineProps, local] = splitProps( + props, + ['required', 'otp', 'autoComplete', 'onInput', 'onComplete'] + ); + const isError = () => local.isError; - const service = useMachine(pinInput.machine, () => ({ - id: createUniqueId(), - required: required(), - autoComplete: autoComplete(), - otp: otp(), + const service = useMachine(pinInput.machine, { + id: createUniqueId(), + required: machineProps.required ?? true, + autoComplete: machineProps.autoComplete ?? 'one-time-code', + otp: machineProps.otp ?? true, onValueChange(value) { - merged.onInput?.(value.valueAsString); + machineProps.onInput?.(value.valueAsString); }, onValueComplete(details) { - merged.onComplete?.(details.valueAsString); + machineProps.onComplete?.(details.valueAsString); }, - })); + });This ensures the machine receives reactive values and can observe prop changes, matching the library's recommended pattern.
⛔ Skipped due to learnings
Learnt from: CR Repo: InfinityBowman/corates PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-12-15T16:38:32.106Z Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Reuse existing Zag components before creating new ones or using plain HTML inputs. Existing components include: Avatar, Checkbox, Collapsible, Dialog, FileUpload, FloatingPanel, PasswordInput, Splitter, Switch, Tabs, Toast, and TooltipLearnt from: CR Repo: InfinityBowman/corates PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-12-15T16:38:32.106Z Learning: Applies to packages/web/src/**/*.{jsx,tsx} : If a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context (when scoped to a feature)Learnt from: CR Repo: InfinityBowman/corates PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-12-15T16:38:32.106Z Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : When implementing UI components, use zag.js from the Zag.js documentation at /docs/zagLearnt from: CR Repo: InfinityBowman/corates PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-12-15T16:38:32.106Z Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Do not destructure props in SolidJS components as it breaks reactivity. Instead, access props directly from the props object (e.g., `props.name`) or wrap them in a function (e.g., `() => props.name`)Learnt from: CR Repo: InfinityBowman/corates PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-12-15T16:38:32.106Z Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Components should receive at most 1–5 props, and only for local configuration, not shared stateLearnt from: CR Repo: InfinityBowman/corates PR: 0 File: .cursorrules:0-0 Timestamp: 2025-12-15T16:38:07.301Z Learning: Applies to **/components/**/*.{jsx,tsx} : In SolidJS components, access props directly from the props object or wrap them in a function instead of destructuring to maintain reactivityLearnt from: CR Repo: InfinityBowman/corates PR: 0 File: .cursorrules:0-0 Timestamp: 2025-12-15T16:38:07.301Z Learning: Components should receive at most 1–5 props for local configuration only, not shared stateLearnt from: CR Repo: InfinityBowman/corates PR: 0 File: .cursorrules:0-0 Timestamp: 2025-12-15T16:38:07.301Z Learning: Reuse existing Zag components: Checkbox, Collapsible, Dialog, FileUpload, PasswordInput, Splitter, Switch, Tabs, Toast, and Tooltip from `packages/web/src/components/zag/`packages/web/src/Routes.jsx (1)
27-33: LGTM!The auth routes are correctly wrapped with
component={AuthGuard}, which allows the router to pass nested routes as children. This pattern correctly gates the sign-in/sign-up flows.packages/web/src/components/profile-ui/AccountProviderCard.jsx (1)
61-129: LGTM!The component demonstrates good practices:
- Proper use of
createMemofor derived values per coding guidelines- Props accessed directly without destructuring to maintain SolidJS reactivity
- Reuses existing Zag
Tooltipcomponent as recommended- Good accessibility with
aria-labelon interactive elements- Clean conditional rendering with
Showcomponentspackages/web/src/components/auth-ui/AuthGuard.jsx (2)
10-17: LGTM!The initial load tracking pattern is clean and correctly distinguishes between the first auth state resolution and subsequent background refetches. This prevents showing loading spinners on every auth state check.
56-67: LGTM!The loading indicator and content gating using
Showcomponents follows SolidJS patterns. The spinner animation is clean.packages/workers/scripts/make-admin-local.mjs (2)
63-65: Escaping approach is adequate for this local-only script.The
sqlStringfunction properly escapes single quotes. For a local development utility this is sufficient.
124-189: LGTM!The main function has good safety checks: refusing to proceed with multiple matched users, checking if already admin, and verifying the update. Error handling is appropriate for a CLI tool.
packages/workers/src/auth/emailTemplates.js (2)
151-177: LGTM!The HTML template follows the established pattern of other templates in this file, with consistent styling and proper parameterization. The security warning about ignoring unexpected requests is appropriate.
179-194: LGTM!The plain text template provides a clear fallback matching the HTML version's content and formatting conventions.
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx (2)
33-55: LGTM!Good use of SolidJS patterns: signals for local state,
createEffectfor reset on dialog open, and props accessed directly without destructuring to maintain reactivity. State is appropriately scoped to this component.
80-102: Good double-submission guard.The
if (loading()) return;check at Line 82 prevents concurrent submissions. The fallback fromcodeFromPinInputtoverificationCode()signal properly handles both theonCompletecallback and manual button click paths.packages/web/src/components/profile-ui/LinkedAccountsSection.jsx (3)
54-56: LGTM!Good use of
createResourcefor data fetching with automatic loading/error states. Therefetchfunction is properly used after mutations.
193-197: LGTM!The
canUnlinklogic properly prevents users from locking themselves out - they can only unlink if they have another linked account or a verified email (for magic link sign-in). Good use ofcreateMemofor this derived value.
240-250: LGTM!Good use of the
Fordirective for rendering the accounts list, with proper passing of props toAccountProviderCard. The reactive values (canUnlink(),unlinkingId()) are correctly evaluated at render time.packages/workers/src/routes/account-merge.js (1)
392-414: LGTM!The cancel route is appropriately permissive (allows cancel without token since it's scoped to the current user's requests) and idempotent (always returns success). This is good practice for cancellation endpoints.
| <Route path='/' component={() => <AuthGuard redirect='dashboard' />}> | ||
| <Route path='/profile' component={ProfilePage} /> | ||
| <Route path='/settings' component={SettingsPage} /> | ||
| <Route path='/admin' component={AdminDashboard} /> | ||
| <Route path='/settings/billing' component={BillingPage} /> | ||
| </Route> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the Routes.jsx file to see the full context
cd packages/web/src && wc -l Routes.jsx && head -60 Routes.jsxRepository: InfinityBowman/corates
Length of output: 2903
🏁 Script executed:
# Find the AuthGuard component
find packages/web/src -name "*AuthGuard*" -o -name "*authGuard*" | head -20Repository: InfinityBowman/corates
Length of output: 115
🏁 Script executed:
# Search for AuthGuard import/usage patterns
rg -n "AuthGuard" packages/web/src --type jsx --type tsxRepository: InfinityBowman/corates
Length of output: 93
🏁 Script executed:
# Read AuthGuard component
cat -n packages/web/src/components/auth-ui/AuthGuard.jsxRepository: InfinityBowman/corates
Length of output: 2897
🏁 Script executed:
# Also check how AuthGuard is used on line 27 (the working pattern)
sed -n '27,34p' packages/web/src/Routes.jsxRepository: InfinityBowman/corates
Length of output: 457
Protected routes won't render - AuthGuard doesn't receive children.
The inline arrow function () => <AuthGuard redirect='dashboard' /> ignores the children prop passed by SolidJS Router. Nested routes need to be forwarded to AuthGuard via props.children for them to render. The profile, settings, admin, and billing routes on lines 41-44 are currently inaccessible.
Use the same pattern as the working auth routes on line 27:
- <Route path='/' component={() => <AuthGuard redirect='dashboard' />}>
+ <Route path='/' component={props => <AuthGuard redirect='dashboard'>{props.children}</AuthGuard>}>
<Route path='/profile' component={ProfilePage} />
<Route path='/settings' component={SettingsPage} />
<Route path='/admin' component={AdminDashboard} />
<Route path='/settings/billing' component={BillingPage} />
</Route>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Route path='/' component={() => <AuthGuard redirect='dashboard' />}> | |
| <Route path='/profile' component={ProfilePage} /> | |
| <Route path='/settings' component={SettingsPage} /> | |
| <Route path='/admin' component={AdminDashboard} /> | |
| <Route path='/settings/billing' component={BillingPage} /> | |
| </Route> | |
| <Route path='/' component={props => <AuthGuard redirect='dashboard'>{props.children}</AuthGuard>}> | |
| <Route path='/profile' component={ProfilePage} /> | |
| <Route path='/settings' component={SettingsPage} /> | |
| <Route path='/admin' component={AdminDashboard} /> | |
| <Route path='/settings/billing' component={BillingPage} /> | |
| </Route> |
🤖 Prompt for AI Agents
In packages/web/src/Routes.jsx around lines 40 to 45, the inline component for
the protected Route returns <AuthGuard redirect='dashboard' /> without
forwarding nested route children, so the nested profile/settings/admin/billing
routes never render; change the Route to accept props and pass props.children
into AuthGuard (use the same pattern as the working auth routes on line 27) so
AuthGuard wraps and renders its child routes.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/workers/scripts/make-admin-prod.mjs (1)
5-6: Update header comment to clarify ADMIN_EMAIL fallback behavior.The header comment at lines 5–6 refers to "default email" but doesn't explain what that default is. Since the code now uses
process.env.ADMIN_EMAILas the fallback (line 143), the comment should clarify this instead of leaving it vague.Additionally, the usage text at line 126 mentions setting ADMIN_EMAIL but doesn't explicitly state that this value is used when
Also confirmed that the old ADMIN_EMAILS (plural) key is fully retired—only ADMIN_EMAIL (singular) is referenced in the codebase.
♻️ Duplicate comments (1)
packages/workers/src/routes/account-merge.js (1)
180-185: Rate limiting implementation addresses previous concern.The per-token rate limiting (5 attempts per 15 minutes) adequately protects against brute-force attacks on the 6-digit verification code.
🧹 Nitpick comments (9)
packages/web/src/components/profile-ui/AccountProviderCard.jsx (2)
37-49: Avoid hard-codingen-USfor linked date formattingIf you care about i18n, consider using the browser locale (e.g.
navigator.language) or accepting alocale/formatOptionsprop instead of hard-coding'en-US', so the date renders naturally for non–US users.
106-128: Validate unlink UI logic and hardenaria-labelfallback
- With
Show when={props.canUnlink}and innerShow when={!isCredential()}, credential providers get no unlink button and no tooltip explanation when they can’t be unlinked. Confirm that this “no UI at all for credentials” state is intentional.- If
props.provider?.nameis missing, the button’s aria-label becomes “Unlink undefined account”. Prefer a safer fallback.- aria-label={`Unlink ${props.provider?.name} account`} + aria-label={`Unlink ${(props.provider?.name || props.account.providerId)} account`}packages/web/src/lib/account-linking-errors.js (1)
64-98: Minor: Redundant nullish coalescing on line 96.The expression
message ?? nullis redundant sincegetLinkErrorMessagealready returns either a string ornull(for silent errors). The?? nulldoesn't change the result.return { code, - message: message ?? null, + message, };packages/workers/src/routes/account-merge.js (2)
44-46: Consider using crypto for code generation.
Math.random()is not cryptographically secure. While the rate limiting (5 attempts) and short expiry (15 minutes) provide adequate protection, usingcrypto.getRandomValues()would be more robust.function generateCode() { - return Math.floor(100000 + Math.random() * 900000).toString(); + const array = new Uint32Array(1); + crypto.getRandomValues(array); + return (100000 + (array[0] % 900000)).toString(); }
288-291: Acknowledge data consistency risk without transactions.The comment correctly notes D1's lack of transaction support. A partial merge failure could leave data in an inconsistent state. Consider adding a more defensive approach: either implement idempotent operations that can be retried, or add a cleanup mechanism for failed merges.
For production reliability, consider:
- Adding a
mergeStatusfield to track progress through each step- Implementing a retry/resume mechanism for failed merges
- Logging each successful step for debugging partial failures
packages/workers/.env.example (1)
22-22: ADMIN_EMAIL name matches scripts; consider adding trailing newline for linter.The ADMIN_EMAIL key aligns with the new local/prod admin scripts that read
process.env.ADMIN_EMAIL, so the rename from the plural form looks correct. dotenv-linter is only flagging the missing blank line at EOF; you can optionally add a trailing newline to silence it, but behavior is otherwise fine.packages/workers/scripts/make-admin-local.mjs (1)
1-189: Local admin script looks solid; consider sharing helpers with prod and clarifying JSON/"no rows" handling.This script mirrors the prod admin tool well: argument parsing, email normalization/validation, SQL construction, and D1 execution all look correct for the local DB case, and the ADMIN_EMAIL fallback is consistent with the prod script.
Two optional refinements you might consider:
Share common helpers with make-admin-prod.mjs.
parseArgs,normalizeEmail,isValidEmail,sqlString,runWranglerD1Execute, andextractRowsFromWranglerJsonare nearly identical between the scripts (differing only in DB config and the--yesflag in prod). Extracting these into a shared module (e.g.,./admin-utils.mjs) would reduce duplication and keep behavior in sync.Differentiate JSON parse failures from "no rows found".
WhenJSON.parsethrows inextractRowsFromWranglerJson, you return{ rows: null, raw }, and the caller treats!before.rowsas "No user found". This conflates "Wrangler output changed / wasn't JSON" with a legitimate no-match. A simple check forrows === nullwith a distinct error message (or logging the raw output) would make failures easier to diagnose.The current implementation is safe and functionally correct for local admin operations.
packages/web/src/components/auth-ui/AuthGuard.jsx (1)
33-41: Spinner UI is duplicated with AuthLayout – consider a shared loader component.The loading block here is essentially the same as the one in
AuthLayout.jsx. To keep things DRY and make future styling changes easier, consider extracting a small<AuthLoadingSpinner />(or similar) component and reusing it in both guards.packages/web/src/components/auth-ui/AuthLayout.jsx (1)
22-51: AuthLayout guest guard and visibility logic are solid; only minor DRY opportunity.The redirect effect plus
showContentlogic correctly:
- Keeps logged‑in users off signin/signup/check‑email,
- Allows
/complete-profileand/reset-passwordfor both logged‑in and guests as intended, and- Avoids doing any navigation from within render.
The only minor nit is that the loading spinner + layout here duplicate the markup in
ProtectedGuard.jsx; extracting a shared<AuthLoadingSpinner />(or similar) used by both guards would make future changes to the auth loading UI simpler.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.prettierignore(1 hunks)packages/web/src/Routes.jsx(2 hunks)packages/web/src/components/admin-ui/UserTable.jsx(5 hunks)packages/web/src/components/auth-ui/AuthGuard.jsx(1 hunks)packages/web/src/components/auth-ui/AuthLayout.jsx(4 hunks)packages/web/src/components/profile-ui/AccountProviderCard.jsx(1 hunks)packages/web/src/lib/account-linking-errors.js(1 hunks)packages/workers/.env.example(1 hunks)packages/workers/scripts/make-admin-local.mjs(1 hunks)packages/workers/scripts/make-admin-prod.mjs(3 hunks)packages/workers/src/routes/account-merge.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .prettierignore
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web/src/components/admin-ui/UserTable.jsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
**/*.{js,jsx,ts,tsx}: For UI icons, use thesolid-iconslibrary or SVGs only. Do not use emojis.
Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Prefer using config files rather than hardcoding values
Each file should handle one coherent responsibility
Group related components in subdirectories with anindex.jsbarrel export
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/workers/src/routes/account-merge.jspackages/web/src/lib/account-linking-errors.jspackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/Routes.jsxpackages/web/src/components/auth-ui/AuthLayout.jsx
**/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/components/**/*.{jsx,tsx}: Keep component files small, focused, and modular, aiming for ~200-300 lines max per component file
When a component exceeds ~300 lines, extract sub-components into a folder (e.g.,ComponentName/withindex.jsxand helper components)
Move complex logic from component files into separate utility files or primitives
In SolidJS components, access props directly from the props object or wrap them in a function instead of destructuring to maintain reactivity
UsecreateMemoto compute derived values based on props or state in SolidJS components
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/auth-ui/AuthLayout.jsx
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursorrules)
Use Solid's
createStorefor complex state or state objects in SolidJS components
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/workers/src/routes/account-merge.jspackages/web/src/lib/account-linking-errors.jspackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/Routes.jsxpackages/web/src/components/auth-ui/AuthLayout.jsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{jsx,tsx}: Use responsive design principles for UI components
Keep files small, focused, and modular. Extract sub-modules into a folder (e.g.,ComponentName/withindex.jsxand helper components). Move complex logic into separate utility files or primitives. Split large forms into section components.
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/Routes.jsxpackages/web/src/components/auth-ui/AuthLayout.jsx
packages/web/src/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/components/**/*.{jsx,tsx}: When implementing UI components, use zag.js from the Zag.js documentation at /docs/zag
Reuse existing Zag components before creating new ones or using plain HTML inputs. Existing components include: Avatar, Checkbox, Collapsible, Dialog, FileUpload, FloatingPanel, PasswordInput, Splitter, Switch, Tabs, Toast, and Tooltip
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/components/auth-ui/AuthLayout.jsx
packages/web/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/**/*.{jsx,tsx}: Do NOT prop-drill application state. Shared or cross-feature state must live in external stores underpackages/web/src/stores/or relative to the component file. Import stores directly where needed instead of passing values through multiple components.
Components should receive at most 1–5 props, and only for local configuration, not shared state
If a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context (when scoped to a feature)
Do not destructure props in SolidJS components as it breaks reactivity. Instead, access props directly from the props object (e.g.,props.name) or wrap them in a function (e.g.,() => props.name)
UsecreateMemofor derived values based on props or state to ensure reactive updates in SolidJS
Use Solid'screateStorefor complex state objects in SolidJS for better performance and reactivity
Components should be lean and focused. Do not implement business logic; move that into stores, utilities, or primitives
Never have a component act as a 'God component' coordinating multiple large concerns
Files:
packages/web/src/components/profile-ui/AccountProviderCard.jsxpackages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/Routes.jsxpackages/web/src/components/auth-ui/AuthLayout.jsx
🧠 Learnings (4)
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Use Solid's `createStore` for complex state objects in SolidJS for better performance and reactivity
Applied to files:
packages/web/src/components/auth-ui/AuthGuard.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Do not destructure props in SolidJS components as it breaks reactivity. Instead, access props directly from the props object (e.g., `props.name`) or wrap them in a function (e.g., `() => props.name`)
Applied to files:
packages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/Routes.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : If a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context (when scoped to a feature)
Applied to files:
packages/web/src/components/auth-ui/AuthGuard.jsxpackages/web/src/Routes.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Use `createMemo` for derived values based on props or state to ensure reactive updates in SolidJS
Applied to files:
packages/web/src/components/auth-ui/AuthGuard.jsx
🧬 Code graph analysis (6)
packages/web/src/components/profile-ui/AccountProviderCard.jsx (1)
packages/web/src/components/zag/Tooltip.jsx (1)
Tooltip(5-37)
packages/workers/scripts/make-admin-prod.mjs (1)
packages/workers/scripts/make-admin-local.mjs (4)
args(15-18)args(68-68)args(125-125)
packages/workers/src/routes/account-merge.js (5)
packages/workers/src/middleware/auth.js (1)
requireAuth(34-53)packages/workers/src/db/client.js (1)
createDb(9-11)packages/workers/src/auth/email.js (1)
createEmailService(21-110)packages/workers/src/auth/emailTemplates.js (2)
getAccountMergeEmailHtml(151-177)getAccountMergeEmailText(179-194)packages/workers/src/middleware/rateLimit.js (1)
rateLimit(51-114)
packages/workers/scripts/make-admin-local.mjs (1)
packages/workers/scripts/make-admin-prod.mjs (12)
args(15-19)args(75-75)args(134-134)i(23-23)stderr(85-85)stdout(86-86)message(87-87)selectSql(157-159)updateSql(161-163)beforeJson(173-173)before(174-174)
packages/web/src/components/auth-ui/AuthGuard.jsx (2)
packages/web/src/components/auth-ui/AuthLayout.jsx (2)
useBetterAuth(10-10)navigate(11-11)packages/web/src/components/Navbar.jsx (1)
Navbar(8-189)
packages/web/src/Routes.jsx (2)
packages/web/src/components/auth-ui/AuthGuard.jsx (1)
ProtectedGuard(9-44)packages/web/src/Layout.jsx (1)
MainLayout(10-41)
🪛 dotenv-linter (4.0.0)
packages/workers/.env.example
[warning] 22-22: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
⏰ 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). (2)
- GitHub Check: Workers Builds: corates
- GitHub Check: Workers Builds: corates-workers-prod
🔇 Additional comments (9)
packages/web/src/components/profile-ui/AccountProviderCard.jsx (2)
17-23: ORCID formatter correctly handles checksum and preformatted IDsThe helper cleanly no-ops when hyphens are already present and the regex now supports 15 digits plus a final digit/X/x, which covers the ORCID checksum case without over-formatting invalid inputs.
8-11: > Likely an incorrect or invalid review comment.packages/web/src/lib/account-linking-errors.js (2)
8-41: LGTM! Well-structured error message mapping.The error code mapping is comprehensive, covering OAuth errors, session issues, network errors, and provider configuration problems. The use of
nullvalues for silent errors (user-cancelled scenarios) is a good pattern for UX.
48-53: LGTM!The function correctly handles both defined error codes (including
nullsilent errors) and unknown codes with a sensible fallback.packages/workers/src/routes/account-merge.js (4)
1-36: LGTM! Clean module setup with appropriate imports.The imports are well-organized, including authentication middleware, rate limiting, and email services. Using
requireAuthmiddleware on all routes is the correct security approach.
95-96: Useeqfor identifier cleanup to avoid unintended deletions.Using
likewith a pattern that ends with%could potentially match unintended records if the identifier format changes. Since the identifier format ismerge:{userId}:{targetId}, using a more precise match would be safer.// Delete any existing merge requests for this user - await db.delete(verification).where(like(verification.identifier, `merge:${currentUser.id}:%`)); + await db.delete(verification).where(sql`${verification.identifier} LIKE ${'merge:' + currentUser.id + ':%'}`);Actually, the current
likeusage is appropriate here since you want to delete all merge requests initiated by this user regardless of target. The pattern is safe given the structured identifier format.
130-139: Email failure handling properly implemented.The rollback of the verification record on email failure and returning 502 status correctly addresses the previous review concern about email failures.
427-434: Verify intentional behavior for token-less cancellation.Line 431 allows cancellation without providing a
mergeToken(!mergeTokenis truthy when token is missing). This enables users to cancel their own merge requests from the UI without tracking the token, which may be intentional for UX. Since the query is scoped tocurrentUser.id, this is secure.Confirm this is the intended behavior for the cancel flow.
packages/web/src/Routes.jsx (1)
20-44: ProtectedGuard wiring for nested routes looks correct.Importing
ProtectedGuardand using it directly as thecomponentfor the nested/route ensures that the/profile,/settings,/admin, and/settings/billingroutes render as children and are properly gated by auth, while still sharingLayout. This fixes the prior “children not forwarded” pattern and looks good.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/web/src/components/auth-ui/ProtectedGuard.jsx (1)
5-8: Clarify hardcoded redirect behavior in documentation.The doc comment states "Redirects guests to dashboard" but doesn't clarify that this path is hardcoded. This mirrors a similar documentation issue flagged in the related
AuthGuard.jsxcomponent. Consider either:
- Updating the comment to explicitly state "Redirects guests to
/dashboard" to match the current implementation, or- Adding a
redirectToprop (with/dashboardas default) if you anticipate reusing this guard with different redirect targets.Given the current usage in
Routes.jsx, this is primarily a documentation clarity issue.🤖 Optional: Add redirectTo prop for flexibility
If you'd like to make this configurable:
/** * ProtectedGuard - For authenticated pages (profile, settings, admin, etc.) - * Redirects guests to dashboard + * Redirects guests to the specified path (default: /dashboard) */ -export default function ProtectedGuard(props) { +export default function ProtectedGuard(props) { + const redirectTo = () => props.redirectTo || '/dashboard'; const { isLoggedIn, authLoading } = useBetterAuth(); const navigate = useNavigate(); // ... createEffect(() => { if (!authLoading() && !isLoggedIn()) { - navigate('/dashboard', { replace: true }); + navigate(redirectTo(), { replace: true }); } });
🧹 Nitpick comments (4)
packages/web/src/components/auth-ui/ProtectedGuard.jsx (1)
15-26: Consider simplifying auth state tracking logic.The current implementation uses two separate effects and an
initialLoadCompletesignal to coordinate auth state and redirect logic. While this works correctly, you might consider consolidating the logic into a single effect or moving this coordination into a primitive/hook for better reusability.However, the current separation of concerns (tracking load completion vs. handling redirects) is clear and maintainable, so this is purely optional.
🤖 Alternative: Single effect approach
- const [initialLoadComplete, setInitialLoadComplete] = createSignal(false); - - createEffect(() => { - if (!authLoading()) { - setInitialLoadComplete(true); - } - }); - - // Redirect non-logged-in users to dashboard + // Track initial load and redirect non-logged-in users createEffect(() => { if (!authLoading() && !isLoggedIn()) { navigate('/dashboard', { replace: true }); } }); - const showLoading = () => !initialLoadComplete() && authLoading(); - const showContent = () => initialLoadComplete() && isLoggedIn(); + const showLoading = () => authLoading(); + const showContent = () => !authLoading() && isLoggedIn();This simplification assumes the redirect happens fast enough that we don't need separate tracking of "initial load complete."
packages/workers/src/__tests__/admin.test.js (2)
67-84: Consider adding unique constraints to prevent duplicate account links.The
accounttable lacks unique constraints on provider-account combinations. Typically, OAuth account tables include constraints likeUNIQUE(providerId, accountId)orUNIQUE(userId, providerId)to prevent duplicate account linkings at the database level.Apply this diff to add a unique constraint:
password TEXT, createdAt INTEGER DEFAULT (unixepoch()), updatedAt INTEGER DEFAULT (unixepoch()), - FOREIGN KEY(userId) REFERENCES user(id) ON DELETE CASCADE + FOREIGN KEY(userId) REFERENCES user(id) ON DELETE CASCADE, + UNIQUE(userId, providerId, accountId) )
163-189: Consider adding aseedAccounthelper for future test maintainability.While the
accounttable has been added to the schema, there's no correspondingseedAccounthelper function like those provided for other tables (seedUser,seedProject,seedSession). Adding this helper now would make it easier to write account-related tests in the future.Example implementation to add after the
seedSessionfunction:async function seedAccount({ id, userId, accountId, providerId, accessToken = null, refreshToken = null, scope = null, createdAt, updatedAt }) { await env.DB.prepare( `INSERT INTO account (id, userId, accountId, providerId, accessToken, refreshToken, scope, createdAt, updatedAt) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)` ) .bind(id, userId, accountId, providerId, accessToken, refreshToken, scope, createdAt, updatedAt) .run(); }packages/workers/src/routes/account-merge.js (1)
44-46: Use crypto-secure randomness for verification codes
generateCoderelies onMath.random, which is not cryptographically secure. For verification codes, prefer a CSPRNG to avoid predictable codes, even with rate limiting in place.Example refactor:
-function generateCode() { - return Math.floor(100000 + Math.random() * 900000).toString(); -} +function generateCode() { + const array = new Uint32Array(1); + crypto.getRandomValues(array); + // 0–899999 mapped to 100000–999999 + const code = 100000 + (array[0] % 900000); + return code.toString(); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/web/src/Routes.jsx(2 hunks)packages/web/src/components/auth-ui/ProtectedGuard.jsx(1 hunks)packages/workers/src/__tests__/admin.test.js(2 hunks)packages/workers/src/routes/account-merge.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web/src/Routes.jsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
**/*.{js,jsx,ts,tsx}: For UI icons, use thesolid-iconslibrary or SVGs only. Do not use emojis.
Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Prefer using config files rather than hardcoding values
Each file should handle one coherent responsibility
Group related components in subdirectories with anindex.jsbarrel export
Files:
packages/web/src/components/auth-ui/ProtectedGuard.jsxpackages/workers/src/__tests__/admin.test.jspackages/workers/src/routes/account-merge.js
**/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/components/**/*.{jsx,tsx}: Keep component files small, focused, and modular, aiming for ~200-300 lines max per component file
When a component exceeds ~300 lines, extract sub-components into a folder (e.g.,ComponentName/withindex.jsxand helper components)
Move complex logic from component files into separate utility files or primitives
In SolidJS components, access props directly from the props object or wrap them in a function instead of destructuring to maintain reactivity
UsecreateMemoto compute derived values based on props or state in SolidJS components
Files:
packages/web/src/components/auth-ui/ProtectedGuard.jsx
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursorrules)
Use Solid's
createStorefor complex state or state objects in SolidJS components
Files:
packages/web/src/components/auth-ui/ProtectedGuard.jsxpackages/workers/src/__tests__/admin.test.jspackages/workers/src/routes/account-merge.js
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{jsx,tsx}: Use responsive design principles for UI components
Keep files small, focused, and modular. Extract sub-modules into a folder (e.g.,ComponentName/withindex.jsxand helper components). Move complex logic into separate utility files or primitives. Split large forms into section components.
Files:
packages/web/src/components/auth-ui/ProtectedGuard.jsx
packages/web/src/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/components/**/*.{jsx,tsx}: When implementing UI components, use zag.js from the Zag.js documentation at /docs/zag
Reuse existing Zag components before creating new ones or using plain HTML inputs. Existing components include: Avatar, Checkbox, Collapsible, Dialog, FileUpload, FloatingPanel, PasswordInput, Splitter, Switch, Tabs, Toast, and Tooltip
Files:
packages/web/src/components/auth-ui/ProtectedGuard.jsx
packages/web/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/**/*.{jsx,tsx}: Do NOT prop-drill application state. Shared or cross-feature state must live in external stores underpackages/web/src/stores/or relative to the component file. Import stores directly where needed instead of passing values through multiple components.
Components should receive at most 1–5 props, and only for local configuration, not shared state
If a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context (when scoped to a feature)
Do not destructure props in SolidJS components as it breaks reactivity. Instead, access props directly from the props object (e.g.,props.name) or wrap them in a function (e.g.,() => props.name)
UsecreateMemofor derived values based on props or state to ensure reactive updates in SolidJS
Use Solid'screateStorefor complex state objects in SolidJS for better performance and reactivity
Components should be lean and focused. Do not implement business logic; move that into stores, utilities, or primitives
Never have a component act as a 'God component' coordinating multiple large concerns
Files:
packages/web/src/components/auth-ui/ProtectedGuard.jsx
🧠 Learnings (4)
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Do not destructure props in SolidJS components as it breaks reactivity. Instead, access props directly from the props object (e.g., `props.name`) or wrap them in a function (e.g., `() => props.name`)
Applied to files:
packages/web/src/components/auth-ui/ProtectedGuard.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Use Solid's `createStore` for complex state objects in SolidJS for better performance and reactivity
Applied to files:
packages/web/src/components/auth-ui/ProtectedGuard.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Use `createMemo` for derived values based on props or state to ensure reactive updates in SolidJS
Applied to files:
packages/web/src/components/auth-ui/ProtectedGuard.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : If a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context (when scoped to a feature)
Applied to files:
packages/web/src/components/auth-ui/ProtectedGuard.jsx
🧬 Code graph analysis (1)
packages/workers/src/__tests__/admin.test.js (1)
packages/workers/src/db/schema.js (1)
user(103-103)
⏰ 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). (2)
- GitHub Check: Workers Builds: corates
- GitHub Check: Workers Builds: corates-workers-prod
🔇 Additional comments (3)
packages/workers/src/__tests__/admin.test.js (1)
41-41: LGTM!The
accounttable drop is correctly positioned before theusertable to respect foreign key constraints.packages/workers/src/routes/account-merge.js (2)
352-383: Subscription merge logic looks consistent and resolves earlierstripeCustomerIdedge caseThe updated subscription merge section now:
- Compares tiers purely via
TIER_PRIORITY, regardless ofstripeCustomerId.- Moves a higher-priority secondary subscription to the primary user (deleting the primary’s old sub if present).
- Deletes the secondary subscription when the primary is same-or-better.
This aligns with the intention to handle gifts/manual upgrades and avoids discarding a better plan just because it lacks a Stripe customer ID. The use of
updatedAt: new Date()on transfer is also good.
415-437: Cancel endpoint behavior is straightforward and idempotentThe
/cancelhandler:
- Scopes cancellations to the authenticated user via the
merge:${currentUser.id}:%identifier pattern.- Optionally validates
mergeTokenwhen provided, but still allows “cancel any active merge for me” when omitted.- Safely no-ops when there is no merge request.
This is simple, idempotent, and avoids leaking information about other users’ merges.
| const showLoading = () => !initialLoadComplete() && authLoading(); | ||
| const showContent = () => initialLoadComplete() && isLoggedIn(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use createMemo for derived values.
The showLoading and showContent functions compute derived values from reactive signals. Per SolidJS best practices and project guidelines, these should use createMemo to explicitly memoize computations and make reactive dependencies clearer.
Based on learnings, derived values based on props or state should use createMemo for reactive updates.
Apply this diff:
+ const showLoading = createMemo(() => !initialLoadComplete() && authLoading());
+ const showContent = createMemo(() => initialLoadComplete() && isLoggedIn());
- const showLoading = () => !initialLoadComplete() && authLoading();
- const showContent = () => initialLoadComplete() && isLoggedIn();
return (
<>
- <Show when={showLoading()}>
+ <Show when={showLoading()}>
<div class='w-full max-w-md sm:max-w-xl flex items-center justify-center'>
<div class='text-center'>
<div class='animate-spin rounded-full h-12 w-12 border-b-2 border-blue-600 mx-auto' />
<p class='mt-4 text-gray-600'>Loading...</p>
</div>
</div>
</Show>
- <Show when={showContent()}>{props.children}</Show>
+ <Show when={showContent()}>{props.children}</Show>
</>
);Note: The imports already include createMemo would need to be added:
-import { createEffect, Show, createSignal } from 'solid-js';
+import { createEffect, Show, createSignal, createMemo } from 'solid-js';| await run(` | ||
| CREATE TABLE account ( | ||
| id TEXT PRIMARY KEY, | ||
| userId TEXT NOT NULL, | ||
| accountId TEXT NOT NULL, | ||
| providerId TEXT NOT NULL, | ||
| accessToken TEXT, | ||
| refreshToken TEXT, | ||
| accessTokenExpiresAt INTEGER, | ||
| refreshTokenExpiresAt INTEGER, | ||
| scope TEXT, | ||
| idToken TEXT, | ||
| password TEXT, | ||
| createdAt INTEGER DEFAULT (unixepoch()), | ||
| updatedAt INTEGER DEFAULT (unixepoch()), | ||
| FOREIGN KEY(userId) REFERENCES user(id) ON DELETE CASCADE | ||
| ) | ||
| `); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if unlinkedAt field exists in migration file
rg -n "unlinkedAt" packages/workers/migrations/0001_init.sqlRepository: InfinityBowman/corates
Length of output: 144
🏁 Script executed:
#!/bin/bash
# Get the full account table schema from migration file
rg -A 20 "CREATE TABLE account" packages/workers/migrations/0001_init.sqlRepository: InfinityBowman/corates
Length of output: 687
🏁 Script executed:
#!/bin/bash
# Get the full test account table schema (lines 67-84 context)
sed -n '60,90p' packages/workers/src/__tests__/admin.test.jsRepository: InfinityBowman/corates
Length of output: 829
Apply the unlinkedAt field to match the migration schema.
The test schema is missing the unlinkedAt field defined in packages/workers/migrations/0001_init.sql (line 70). This discrepancy causes the test environment to diverge from the actual database schema, potentially allowing tests to miss issues related to soft-delete handling.
scope TEXT,
idToken TEXT,
password TEXT,
+ unlinkedAt INTEGER DEFAULT NULL,
createdAt INTEGER DEFAULT (unixepoch()),
updatedAt INTEGER DEFAULT (unixepoch()),
FOREIGN KEY(userId) REFERENCES user(id) ON DELETE CASCADE📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await run(` | |
| CREATE TABLE account ( | |
| id TEXT PRIMARY KEY, | |
| userId TEXT NOT NULL, | |
| accountId TEXT NOT NULL, | |
| providerId TEXT NOT NULL, | |
| accessToken TEXT, | |
| refreshToken TEXT, | |
| accessTokenExpiresAt INTEGER, | |
| refreshTokenExpiresAt INTEGER, | |
| scope TEXT, | |
| idToken TEXT, | |
| password TEXT, | |
| createdAt INTEGER DEFAULT (unixepoch()), | |
| updatedAt INTEGER DEFAULT (unixepoch()), | |
| FOREIGN KEY(userId) REFERENCES user(id) ON DELETE CASCADE | |
| ) | |
| `); | |
| await run(` | |
| CREATE TABLE account ( | |
| id TEXT PRIMARY KEY, | |
| userId TEXT NOT NULL, | |
| accountId TEXT NOT NULL, | |
| providerId TEXT NOT NULL, | |
| accessToken TEXT, | |
| refreshToken TEXT, | |
| accessTokenExpiresAt INTEGER, | |
| refreshTokenExpiresAt INTEGER, | |
| scope TEXT, | |
| idToken TEXT, | |
| password TEXT, | |
| unlinkedAt INTEGER DEFAULT NULL, | |
| createdAt INTEGER DEFAULT (unixepoch()), | |
| updatedAt INTEGER DEFAULT (unixepoch()), | |
| FOREIGN KEY(userId) REFERENCES user(id) ON DELETE CASCADE | |
| ) | |
| `); |
🤖 Prompt for AI Agents
In packages/workers/src/__tests__/admin.test.js around lines 67 to 84, the
CREATE TABLE account test schema is missing the unlinkedAt column from the real
migration; update the CREATE TABLE account block to include the unlinkedAt field
(match the migration by adding an unlinkedAt column of the same type and
default, e.g., unlinkedAt INTEGER DEFAULT NULL) inside the column list before
the createdAt/updatedAt columns so the test DB schema matches
packages/workers/migrations/0001_init.sql.
…hing/transactions
…orates into account-linking
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/workers/src/routes/account-merge.js (2)
61-161: Add rate limiting to prevent merge email spam.The
/initiateendpoint lacks rate limiting. An authenticated user can trigger unlimited merge emails to arbitrary addresses by repeatedly calling this endpoint with different target emails. Consider applying rate limiting keyed bycurrentUser.id:+// Rate limiter for merge initiation (3 attempts per hour per user) +const mergeInitiateRateLimiter = rateLimit({ + limit: 3, + windowMs: 60 * 60 * 1000, // 1 hour + keyPrefix: 'merge-initiate', + keyGenerator: c => c.get('user')?.id || 'unknown', +}); + -accountMergeRoutes.post('/initiate', async c => { +accountMergeRoutes.post('/initiate', mergeInitiateRateLimiter, async c => {Additionally, returning
targetProvidersin the preview (lines 156-159) before email verification allows probing which providers are linked to arbitrary emails. Consider deferring this information until after verification.
178-191: Add type validation formergeTokenandcodeto prevent runtime errors.If a malicious client sends a non-string value (e.g.,
{"code": 123}), callingcode.trim()on line 220 will throw a TypeError, resulting in a 500 error instead of a clean 400.accountMergeRoutes.post('/verify', async c => { const { user: currentUser } = getAuth(c); const { mergeToken, code } = await c.req.json(); - if (!mergeToken || !code) { + if (typeof mergeToken !== 'string' || typeof code !== 'string') { return c.json({ error: 'Merge token and code are required' }, 400); } + + const trimmedToken = mergeToken.trim(); + const trimmedCode = code.trim(); + + if (!trimmedToken || !trimmedCode) { + return c.json({ error: 'Merge token and code are required' }, 400); + } // Apply rate limiting keyed by mergeToken - c.set('mergeTokenKey', mergeToken); + c.set('mergeTokenKey', trimmedToken);Then use
trimmedTokenandtrimmedCodein subsequent comparisons.
🧹 Nitpick comments (11)
packages/web/src/lib/account-linking-errors.js (2)
72-84: Consider removing redundant self-mappings.Some entries in
errorMappingsmap codes to themselves (e.g.,ACCOUNT_ALREADY_LINKED: 'ACCOUNT_ALREADY_LINKED'). These are redundant since unmapped codes are used as-is. Removing them would reduce noise:const errorMappings = { ACCESS_DENIED: 'OAUTH_CANCELLED', USER_CANCELLED: 'OAUTH_CANCELLED', - ACCOUNT_ALREADY_LINKED: 'ACCOUNT_ALREADY_LINKED', ACCOUNT_EXISTS: 'ACCOUNT_ALREADY_LINKED', ACCOUNT_ALREADY_LINKED_TO_DIFFERENT_USER: 'ACCOUNT_ALREADY_LINKED_TO_DIFFERENT_USER', - EMAIL_DOESNT_MATCH: 'EMAIL_DOESNT_MATCH', - STATE_MISMATCH: 'STATE_MISMATCH', - STATE_NOT_FOUND: 'STATE_NOT_FOUND', - INVALID_CALLBACK_REQUEST: 'INVALID_CALLBACK_REQUEST', - OAUTH_PROVIDER_NOT_FOUND: 'OAUTH_PROVIDER_NOT_FOUND', - UNABLE_TO_LINK_ACCOUNT: 'UNABLE_TO_LINK_ACCOUNT', };
48-53: Use the existingUNKNOWNconstant for the fallback message.The fallback message on line 52 duplicates
LINK_ERROR_MESSAGES.UNKNOWN. Using the constant ensures consistency if the message is updated later.export function getLinkErrorMessage(code) { if (code in LINK_ERROR_MESSAGES) { return LINK_ERROR_MESSAGES[code]; } - return 'An unexpected error occurred. Please try again.'; + return LINK_ERROR_MESSAGES.UNKNOWN; }packages/mcp/__tests__/icons.test.js (1)
9-41: Consider extracting the manifest path to a shared constant.The manifest path is constructed identically on lines 11, 17, and 33. While test isolation is good, extracting this to a module-level constant would reduce duplication.
You could apply this pattern:
+const manifestPath = path.join(__dirname, '..', 'icon-manifest.json'); + describe('icons tool', () => { it('should have icon manifest file', async () => { - const manifestPath = path.join(__dirname, '..', 'icon-manifest.json'); const stat = await fs.stat(manifestPath);packages/mcp/__tests__/better-auth.test.js (1)
1-32: Consider importing BETTER_AUTH_BASE_URL from the production code.The constant on line 3 duplicates the one defined in
packages/mcp/tools/better-auth.js. Importing it from the production module would ensure consistency if the URL changes.Example:
import { describe, it, expect } from 'vitest'; - -const BETTER_AUTH_BASE_URL = 'https://www.better-auth.com'; +import { BETTER_AUTH_BASE_URL } from '../tools/better-auth.js';Note: This would require exporting the constant from
tools/better-auth.js.packages/mcp/__tests__/drizzle.test.js (1)
1-31: Consider importing DRIZZLE_DOCS_URL from the production code.The constant on line 3 duplicates the one in
packages/mcp/tools/drizzle.js. Importing it would ensure consistency.Example:
import { describe, it, expect } from 'vitest'; - -const DRIZZLE_DOCS_URL = 'https://orm.drizzle.team/llms-full.txt'; +import { DRIZZLE_DOCS_URL } from '../tools/drizzle.js';Note: This would require exporting the constant from
tools/drizzle.js.packages/mcp/__tests__/zag.test.js (2)
3-3: Consider importingZAG_DOCS_URLfrom the source module.The URL constant is duplicated here and in
packages/mcp/tools/zag.js. Export it from the source module to keep them in sync.-const ZAG_DOCS_URL = 'https://zagjs.com/llms-solid.txt'; +import { ZAG_DOCS_URL } from '../tools/zag.js';This requires exporting
ZAG_DOCS_URLfrompackages/mcp/tools/zag.js.
6-36: Reduce redundant fetches by caching the response inbeforeAll.Each test case independently fetches the same remote URL, resulting in 4 network requests per test run. This increases test execution time and potential flakiness from network issues.
+let fetchedText; + describe('zag tool', () => { + beforeAll(async () => { + const response = await fetch(ZAG_DOCS_URL); + expect(response.ok).toBe(true); + fetchedText = await response.text(); + }); + it('should fetch zag docs from remote URL', async () => { - const response = await fetch(ZAG_DOCS_URL); - expect(response.ok).toBe(true); - - const text = await response.text(); - expect(text.length).toBeGreaterThan(1000); + expect(fetchedText.length).toBeGreaterThan(1000); }); it('should contain ## Resources markers for parsing', async () => { - const response = await fetch(ZAG_DOCS_URL); - const text = await response.text(); - - expect(text).toContain('## Resources'); + expect(fetchedText).toContain('## Resources'); }); // ... apply similar pattern to remaining testspackages/mcp/tools/icons.js (1)
12-18: Add error handling for manifest loading failures.If
icon-manifest.jsonis missing or malformed, the unhandled exception will crash the tool. Consider graceful error handling with a user-friendly message.async function loadManifest() { if (!manifestCache) { const manifestPath = path.join(__dirname, '..', 'icon-manifest.json'); - manifestCache = JSON.parse(await fs.readFile(manifestPath, 'utf8')); + try { + manifestCache = JSON.parse(await fs.readFile(manifestPath, 'utf8')); + } catch (err) { + throw new Error(`Failed to load icon manifest: ${err.message}`); + } } return manifestCache; }packages/mcp/tools/zag.js (1)
30-41: Consider documenting the magic values for maintainability.The lookahead limit of 10 lines and the excluded package names (
'solid','i') are undocumented. Adding brief comments would help future maintainers understand why these values were chosen.+ // Look ahead for npm link (within next 10 lines - typical spacing in doc format) for (let i = 0; i < lines.length; i++) { if (lines[i] === '## Resources') { - // Look ahead for npm link (within next 10 lines) for (let j = i + 1; j < Math.min(i + 10, lines.length); j++) { const npmMatch = lines[j].match(/@zag-js\/([a-z-]+)/); + // Skip 'solid' (framework binding) and 'i' (false positive from npm link formatting) if (npmMatch && npmMatch[1] !== 'solid' && npmMatch[1] !== 'i') {packages/mcp/tools/drizzle.js (1)
6-18: Opportunity to extract shared caching logic.The cache pattern (module-level cache variable + TTL check + fetch + store) is duplicated across
better-auth.js,drizzle.js, andzag.js. Consider extracting a reusablecreateCachedFetcher(url, parser)utility.This is a nice-to-have refactor that could reduce duplication and ensure consistent caching behavior across all doc tools.
packages/mcp/tools/local-docs.js (1)
32-34: Silent error handling may hide configuration issues.Returning an empty array on any error (including permission issues or invalid
docsRootpath) makes debugging harder. Consider logging the error or distinguishing between "no docs found" and "error reading directory".} catch { - return []; + // Log for debugging but return empty to avoid breaking the tool + console.error('Failed to list local docs:', docsRoot); + return []; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.github/copilot-instructions.md(2 hunks).vscode/mcp.json(1 hunks)packages/mcp/__tests__/better-auth.test.js(1 hunks)packages/mcp/__tests__/drizzle.test.js(1 hunks)packages/mcp/__tests__/icons.test.js(1 hunks)packages/mcp/__tests__/zag.test.js(1 hunks)packages/mcp/constants.js(1 hunks)packages/mcp/package.json(1 hunks)packages/mcp/server.js(1 hunks)packages/mcp/tools/better-auth.js(1 hunks)packages/mcp/tools/drizzle.js(1 hunks)packages/mcp/tools/icons.js(1 hunks)packages/mcp/tools/lint.js(1 hunks)packages/mcp/tools/local-docs.js(1 hunks)packages/mcp/tools/zag.js(1 hunks)packages/mcp/vitest.config.js(1 hunks)packages/web/src/lib/account-linking-errors.js(1 hunks)packages/workers/src/routes/account-merge.js(1 hunks)packages/workers/src/routes/admin.js(5 hunks)packages/workers/src/routes/users.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/copilot-instructions.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
**/*.{js,jsx,ts,tsx}: For UI icons, use thesolid-iconslibrary or SVGs only. Do not use emojis.
Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Prefer using config files rather than hardcoding values
Each file should handle one coherent responsibility
Group related components in subdirectories with anindex.jsbarrel export
Files:
packages/mcp/constants.jspackages/mcp/__tests__/better-auth.test.jspackages/mcp/tools/better-auth.jspackages/mcp/__tests__/icons.test.jspackages/mcp/tools/zag.jspackages/mcp/__tests__/drizzle.test.jspackages/mcp/tools/icons.jspackages/mcp/tools/drizzle.jspackages/workers/src/routes/admin.jspackages/mcp/tools/lint.jspackages/workers/src/routes/users.jspackages/mcp/tools/local-docs.jspackages/mcp/vitest.config.jspackages/web/src/lib/account-linking-errors.jspackages/workers/src/routes/account-merge.jspackages/mcp/__tests__/zag.test.jspackages/mcp/server.js
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursorrules)
Use Solid's
createStorefor complex state or state objects in SolidJS components
Files:
packages/mcp/constants.jspackages/mcp/__tests__/better-auth.test.jspackages/mcp/tools/better-auth.jspackages/mcp/__tests__/icons.test.jspackages/mcp/tools/zag.jspackages/mcp/__tests__/drizzle.test.jspackages/mcp/tools/icons.jspackages/mcp/tools/drizzle.jspackages/workers/src/routes/admin.jspackages/mcp/tools/lint.jspackages/workers/src/routes/users.jspackages/mcp/tools/local-docs.jspackages/mcp/vitest.config.jspackages/web/src/lib/account-linking-errors.jspackages/workers/src/routes/account-merge.jspackages/mcp/__tests__/zag.test.jspackages/mcp/server.js
🧠 Learnings (3)
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : When implementing UI components, use zag.js from the Zag.js documentation at /docs/zag
Applied to files:
packages/mcp/tools/zag.jspackages/mcp/__tests__/zag.test.js
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/components/zag/**/*.{jsx,tsx} : Place new zag components into `packages/web/src/components/zag/*` files
Applied to files:
packages/mcp/tools/zag.jspackages/mcp/__tests__/zag.test.js
📚 Learning: 2025-12-15T16:38:07.301Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-15T16:38:07.301Z
Learning: When implementing UI components, use zag.js and reference the Zag.js documentation in `docs/zag/llms.txt`
Applied to files:
packages/mcp/tools/zag.jspackages/mcp/__tests__/zag.test.js
🧬 Code graph analysis (12)
packages/mcp/__tests__/better-auth.test.js (1)
packages/mcp/tools/better-auth.js (3)
BETTER_AUTH_BASE_URL(4-4)response(23-23)text(28-28)
packages/mcp/tools/better-auth.js (1)
packages/mcp/constants.js (2)
CACHE_TTL(1-1)CACHE_TTL(1-1)
packages/mcp/__tests__/icons.test.js (3)
packages/mcp/tools/icons.js (3)
__filename(6-6)__dirname(7-7)manifestPath(14-14)packages/mcp/iconSearch.js (1)
loadManifest(9-15)packages/mcp/scrape-icons.js (1)
main(62-78)
packages/mcp/__tests__/drizzle.test.js (1)
packages/mcp/tools/drizzle.js (2)
DRIZZLE_DOCS_URL(4-4)response(20-20)
packages/mcp/tools/icons.js (2)
packages/mcp/iconSearch.js (2)
loadManifest(9-15)console(36-38)packages/mcp/scrape-icons.js (1)
main(62-78)
packages/mcp/tools/drizzle.js (2)
packages/mcp/__tests__/drizzle.test.js (1)
DRIZZLE_DOCS_URL(3-3)packages/mcp/constants.js (2)
CACHE_TTL(1-1)CACHE_TTL(1-1)
packages/workers/src/routes/admin.js (1)
packages/workers/src/db/schema.js (5)
account(42-58)account(42-58)user(5-24)user(5-24)user(103-103)
packages/mcp/tools/lint.js (2)
packages/mcp/server.js (2)
server(26-29)repoRoot(22-22)packages/web/src/lib/account-linking-errors.js (1)
error(65-65)
packages/workers/src/routes/users.js (1)
packages/workers/src/db/schema.js (13)
projectMembers(85-95)projectMembers(85-95)projects(73-82)projects(73-82)verification(61-68)verification(61-68)account(42-58)account(42-58)session(27-39)session(27-39)user(5-24)user(5-24)user(103-103)
packages/mcp/tools/local-docs.js (1)
packages/mcp/server.js (2)
docsRoot(23-23)server(26-29)
packages/workers/src/routes/account-merge.js (5)
packages/workers/src/middleware/auth.js (1)
requireAuth(34-53)packages/workers/src/db/client.js (1)
createDb(9-11)packages/workers/src/auth/email.js (1)
createEmailService(21-110)packages/workers/src/auth/emailTemplates.js (2)
getAccountMergeEmailHtml(151-177)getAccountMergeEmailText(179-194)packages/workers/src/middleware/rateLimit.js (1)
rateLimit(51-114)
packages/mcp/__tests__/zag.test.js (1)
packages/mcp/tools/zag.js (2)
ZAG_DOCS_URL(4-4)response(20-20)
⏰ 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). (1)
- GitHub Check: Workers Builds: corates
🔇 Additional comments (24)
packages/workers/src/routes/admin.js (2)
121-145: LGTM - Good approach for fetching linked accounts.Using
sql.joinfor theINclause and grouping by userId is efficient. TheaccountsMappattern avoids N+1 queries.
248-260: Good use of atomic batch for ban operation.Atomically updating the ban status and invalidating sessions ensures the user can't continue using existing sessions after being banned.
packages/workers/src/routes/account-merge.js (3)
48-52: LGTM - Secure code generation using crypto.Using
crypto.getRandomValuesinstead ofMath.randomis the correct approach for security-sensitive codes.
358-429: Well-structured atomic batch for account merge.The batch operation handles all aspects of the merge comprehensively:
- Account deduplication and transfer
- Project ownership and membership transfer
- Subscription tier-based merging
- Media file ownership transfer
- Cleanup of secondary user and verification record
The atomic batch ensures data consistency if any operation fails.
449-471: LGTM - Cancel endpoint handles both explicit and implicit cancellation.The logic allows cancelling either by providing a specific
mergeTokenor by omitting it to cancel any pending merge for the current user. This provides flexibility for the UI..vscode/mcp.json (1)
7-11: LGTM!The Stripe MCP server configuration is correctly structured for HTTP-based external service integration.
packages/mcp/constants.js (1)
1-1: LGTM!Centralized cache TTL constant with clear calculation and documentation.
packages/mcp/__tests__/icons.test.js (1)
1-8: LGTM!Standard Vitest test setup with proper ESM module path handling.
packages/mcp/vitest.config.js (1)
1-9: LGTM!The Vitest configuration appropriately enables globals and sets a reasonable 10-second timeout for tests making network requests.
packages/mcp/package.json (3)
3-3: Major version bump to 2.0.0.The major version bump appropriately signals breaking changes from the modular tool registration refactor mentioned in the PR summary.
12-13: LGTM!The test scripts and Vitest devDependency properly support the new test infrastructure introduced in this PR.
Also applies to: 19-20
15-18: Zod version 4.1.13 is valid and stable. The version exists on npm, was published on 2025-11-24, and is marked as the current latest stable release. No action required.Likely an incorrect or invalid review comment.
packages/mcp/tools/lint.js (2)
1-16: LGTM!The tool registration follows the modular pattern established in this PR. Command construction is safe since
fixis a boolean parameter validated by Zod.
17-41: LGTM!Comprehensive error handling that captures both success and failure cases. The 4MB buffer size is appropriate for lint output, and the consistent content structure makes the tool output predictable.
packages/mcp/tools/icons.js (2)
20-60: LGTM!The
search_iconstool implementation is clean: case-insensitive search, configurable limit with early termination, and appropriate response formatting.
62-79: LGTM!The
icon-librariesresource properly lists available libraries with their icon counts.packages/mcp/tools/better-auth.js (2)
7-17: Global TTL clears entire cache on expiration.The current design uses a single
cacheTimefor all entries. When TTL expires, the entire cache is cleared even if some entries were cached more recently. This is acceptable for infrequently changing docs but worth noting.
33-71: LGTM!The tool registration is well-structured with clear documentation, proper error handling, and consistent response formatting.
packages/mcp/tools/zag.js (2)
149-152: Bidirectional partial match may produce unexpected results.The condition
c.includes(normalizedName) || normalizedName.includes(c)means searching for "a" would match any component containing "a", but also any component whose name is contained in "a". Consider if bidirectional matching is intentional.For example, searching for "tabs" would match "tabs" (intended) but the reverse condition
normalizedName.includes(c)seems unnecessary unless short component names exist.
114-187: LGTM!The tool registration provides good UX with exact match, partial match, and helpful error messages guiding users to the index.
packages/mcp/tools/drizzle.js (1)
100-168: LGTM!The tool implementation is well-structured with proper index generation, exact/partial matching, and informative error responses.
packages/mcp/tools/local-docs.js (2)
5-10: Good security practice with input validation.The
assertSafeDocNamefunction properly prevents path traversal attacks by restricting characters to alphanumeric, dot, underscore, and dash.
65-91: Good defense-in-depth with allowlist validation.The tool validates the doc name format with
assertSafeDocName, then verifies against the discovered allowlist before reading. This prevents accessing arbitrary files even with a valid-looking name.packages/mcp/server.js (1)
12-37: Clean modular refactor.The shift from inline tool definitions to modular registrations improves maintainability and testability. Each tool module is now self-contained with its own caching, parsing, and error handling.
| // Delete all user data atomically using batch transaction | ||
| // Order matters for foreign key constraints | ||
| await db.batch([ | ||
| db.delete(projectMembers).where(eq(projectMembers.userId, userId)), | ||
| db.delete(projects).where(eq(projects.createdBy, userId)), | ||
| db.delete(verification).where(eq(verification.identifier, currentUser.email)), | ||
| db.delete(account).where(eq(account.userId, userId)), | ||
| db.delete(session).where(eq(session.userId, userId)), | ||
| db.delete(user).where(eq(user.id, userId)), | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find and verify the files exist
echo "=== Checking files ==="
ls -la packages/workers/src/routes/users.js 2>/dev/null && echo "✓ users.js found" || echo "✗ users.js not found"
ls -la packages/workers/src/routes/admin.js 2>/dev/null && echo "✓ admin.js found" || echo "✗ admin.js not found"
# Get the actual code at specified lines in users.js
echo ""
echo "=== users.js lines 151-160 ==="
sed -n '151,160p' packages/workers/src/routes/users.js
# Get more context around line 151 in users.js
echo ""
echo "=== users.js lines 145-165 (more context) ==="
sed -n '145,165p' packages/workers/src/routes/users.js
# Get the actual code at specified lines in admin.js
echo ""
echo "=== admin.js lines 387-394 ==="
sed -n '387,394p' packages/workers/src/routes/admin.js 2>/dev/null || echo "Line range might differ"
# Get more context around potential delete operation in admin.js
echo ""
echo "=== Searching for DELETE /users/:userId in admin.js ==="
rg -n "DELETE.*users" packages/workers/src/routes/admin.js -A 5 -B 2Repository: InfinityBowman/corates
Length of output: 2741
🏁 Script executed:
#!/bin/bash
# Now let's verify the schema to see the cascade delete configuration
echo "=== Finding schema files ==="
find packages/workers -name "*.ts" -o -name "*.js" | grep -i schema | head -10
# Look for table definitions
echo ""
echo "=== Searching for table schemas ==="
rg -l "verification\|projectMembers\|projects\|account\|session\|user" packages/workers/src --type ts --type js | grep -i schema
# Let's check common schema locations
echo ""
echo "=== Checking db schema locations ==="
ls -la packages/workers/src/db/ 2>/dev/null || echo "db/ not found"
ls -la packages/workers/src/schema/ 2>/dev/null || echo "schema/ not found"
find packages/workers -name "schema.ts" -o -name "schema.js" -o -name "*schema*" 2>/dev/null | head -20Repository: InfinityBowman/corates
Length of output: 551
🏁 Script executed:
#!/bin/bash
# Read the schema file to verify cascade delete configurations
echo "=== Schema file content ==="
cat packages/workers/src/db/schema.jsRepository: InfinityBowman/corates
Length of output: 6251
Batch deletion is good, but note inconsistencies between endpoints and missing tables.
The batch transaction in users.js correctly uses atomic operations with proper foreign key ordering. However, there are significant inconsistencies with the admin endpoint's deletion logic in admin.js (lines 387-394):
- users.js deletes: projectMembers, projects, verification, account, session, user
- admin.js deletes: projectMembers, projects, session, user (missing: verification, account)
This means admin deletion of a user leaves orphaned verification and account records. Additionally, both endpoints are missing deletions for:
twoFactor(hasonDelete: 'cascade'constraint)subscriptions(hasonDelete: 'cascade'constraint)
While cascade deletes in the schema will handle these automatically, explicit deletion provides defensive redundancy and ensures intentional cleanup. The verification table deletion in users.js is correctly done by identifier since it lacks a direct userId foreign key.
Align admin.js deletion with users.js, and consider adding twoFactor and subscriptions to both for complete data removal.
🤖 Prompt for AI Agents
In packages/workers/src/routes/users.js around lines 151 to 160 and in
packages/workers/src/routes/admin.js around lines 387 to 394, the admin delete
flow is missing verification and account deletions and both endpoints omit
twoFactor and subscriptions; update admin.js to match users.js by adding
deletions for verification (filter by identifier), account (by userId), and also
add explicit deletions for twoFactor and subscriptions in both users.js and
admin.js, preserving the current foreign-key-aware ordering so deletes occur
before the user row is removed.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx (2)
146-152: UsecreateMemofor derived values from props.Both
providerNameandisOpenare derived values based on props. Per SolidJS best practices and coding guidelines, these should usecreateMemoto ensure proper reactive tracking and updates.Apply this diff:
- const providerName = () => { + const providerName = createMemo(() => { const names = { google: 'Google', orcid: 'ORCID' }; return names[props.conflictProvider] || props.conflictProvider; - }; + }); // Preserve reactivity for open prop - const isOpen = () => props.open; + const isOpen = createMemo(() => props.open);As per coding guidelines: "Use
createMemofor derived values based on props or state to ensure reactive updates in SolidJS."
154-387: Consider extracting step panels if the component grows further.The Dialog structure and step implementations are well-organized with proper conditional rendering and user feedback. At 388 lines, this component slightly exceeds the 200-300 line guideline, but remains cohesive with a single clear responsibility. If additional features are added, consider extracting individual step panels into separate sub-components.
Note: The plain HTML email input (lines 206-214) is appropriate since no generic TextInput Zag component exists in the documented list.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/web/src/api/account-merge.js(1 hunks)packages/web/src/components/profile-ui/MergeAccountsDialog.jsx(1 hunks)packages/workers/src/routes/account-merge.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web/src/api/account-merge.js
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
**/*.{js,jsx,ts,tsx}: For UI icons, use thesolid-iconslibrary or SVGs only. Do not use emojis.
Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Prefer using config files rather than hardcoding values
Each file should handle one coherent responsibility
Group related components in subdirectories with anindex.jsbarrel export
Files:
packages/workers/src/routes/account-merge.jspackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursorrules)
Use Solid's
createStorefor complex state or state objects in SolidJS components
Files:
packages/workers/src/routes/account-merge.jspackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
**/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/components/**/*.{jsx,tsx}: Keep component files small, focused, and modular, aiming for ~200-300 lines max per component file
When a component exceeds ~300 lines, extract sub-components into a folder (e.g.,ComponentName/withindex.jsxand helper components)
Move complex logic from component files into separate utility files or primitives
In SolidJS components, access props directly from the props object or wrap them in a function instead of destructuring to maintain reactivity
UsecreateMemoto compute derived values based on props or state in SolidJS components
Files:
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{jsx,tsx}: Use responsive design principles for UI components
Keep files small, focused, and modular. Extract sub-modules into a folder (e.g.,ComponentName/withindex.jsxand helper components). Move complex logic into separate utility files or primitives. Split large forms into section components.
Files:
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/web/src/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/components/**/*.{jsx,tsx}: When implementing UI components, use zag.js from the Zag.js documentation at /docs/zag
Reuse existing Zag components before creating new ones or using plain HTML inputs. Existing components include: Avatar, Checkbox, Collapsible, Dialog, FileUpload, FloatingPanel, PasswordInput, Splitter, Switch, Tabs, Toast, and Tooltip
Files:
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/web/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/**/*.{jsx,tsx}: Do NOT prop-drill application state. Shared or cross-feature state must live in external stores underpackages/web/src/stores/or relative to the component file. Import stores directly where needed instead of passing values through multiple components.
Components should receive at most 1–5 props, and only for local configuration, not shared state
If a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context (when scoped to a feature)
Do not destructure props in SolidJS components as it breaks reactivity. Instead, access props directly from the props object (e.g.,props.name) or wrap them in a function (e.g.,() => props.name)
UsecreateMemofor derived values based on props or state to ensure reactive updates in SolidJS
Use Solid'screateStorefor complex state objects in SolidJS for better performance and reactivity
Components should be lean and focused. Do not implement business logic; move that into stores, utilities, or primitives
Never have a component act as a 'God component' coordinating multiple large concerns
Files:
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx
🧠 Learnings (3)
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Reuse existing Zag components before creating new ones or using plain HTML inputs. Existing components include: Avatar, Checkbox, Collapsible, Dialog, FileUpload, FloatingPanel, PasswordInput, Splitter, Switch, Tabs, Toast, and Tooltip
Applied to files:
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : If a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context (when scoped to a feature)
Applied to files:
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx
📚 Learning: 2025-12-15T16:38:32.106Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-15T16:38:32.106Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Use `createMemo` for derived values based on props or state to ensure reactive updates in SolidJS
Applied to files:
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx
🧬 Code graph analysis (1)
packages/workers/src/routes/account-merge.js (5)
packages/workers/src/middleware/auth.js (1)
requireAuth(34-53)packages/workers/src/middleware/rateLimit.js (1)
rateLimit(51-114)packages/workers/src/db/client.js (1)
createDb(9-11)packages/workers/src/auth/email.js (1)
createEmailService(21-110)packages/workers/src/auth/emailTemplates.js (2)
getAccountMergeEmailHtml(151-177)getAccountMergeEmailText(179-194)
⏰ 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). (2)
- GitHub Check: Workers Builds: corates
- GitHub Check: Workers Builds: corates-workers-prod
🔇 Additional comments (8)
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx (3)
1-31: LGTM!The imports and constants are well-organized. The component correctly uses existing Zag components (Dialog, PinInput) and icons from solid-icons rather than emojis.
33-55: LGTM!Props are correctly accessed without destructuring to maintain SolidJS reactivity, and local state management with
createSignalis appropriate. ThecreateEffectproperly resets state when the dialog opens.
57-144: LGTM!All handler functions demonstrate proper error handling, loading state management, and user feedback through toasts. The use of optional chaining for callbacks and the dual-parameter approach in
handleVerifyCodeshow thoughtful design.packages/workers/src/routes/account-merge.js (5)
1-60: LGTM!The setup is solid with proper middleware, cryptographically secure code generation using
crypto.getRandomValues, and rate limiting configured for the initiate endpoint.
69-176: LGTM!The
/initiateendpoint properly addresses previous review concerns: email failures now roll back the verification record and return an error response (lines 151-160), rate limiting is applied per user+email (lines 84-88), andtargetProvidersare deferred until after code verification for security (line 173 comment). Type validation and self-merge prevention are correctly implemented.
178-283: LGTM!The
/verifyendpoint successfully addresses all previous review concerns: rate limiting prevents brute force attacks (lines 179-184, 210-215), type validation ensures safe string handling (lines 201-208), and provider information is now properly returned after verification (lines 264-282).
334-480: LGTM!The merge logic is comprehensive and correctly handles all data migration scenarios: accounts, projects, memberships, subscriptions, and media files. The subscription merge logic now properly uses only tier priority without the
stripeCustomerIdrequirement, and the atomic batch transaction ensures data integrity. Error handling is appropriate.
482-511: LGTM!The
/cancelendpoint safely handles cleanup of pending merge requests. The lenient token matching (line 505) allows users to cancel without providing a token, which is acceptable for this non-destructive operation since it's already scoped to the current user.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/workers/src/routes/admin.js (1)
130-161: Provider aggregation works correctly; considerinArrayfor cleaner code.The logic correctly fetches linked accounts and attaches provider arrays to each user. The manual SQL IN-clause construction (lines 142-146) is functional but could be simplified using Drizzle's
inArrayoperator.Optional refactoring to use
inArray:+import { eq, desc, sql, like, or, count, inArray } from 'drizzle-orm'; if (userIds.length > 0) { const accounts = await db .select({ userId: account.userId, providerId: account.providerId, }) .from(account) - .where( - sql`${account.userId} IN (${sql.join( - userIds.map(id => sql`${id}`), - sql`, `, - )})`, - ); + .where(inArray(account.userId, userIds));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/mcp/__tests__/icons.test.js(1 hunks)packages/workers/src/routes/account-merge.js(1 hunks)packages/workers/src/routes/admin.js(5 hunks)packages/workers/src/routes/users.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/workers/src/routes/account-merge.js
- packages/mcp/tests/icons.test.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
**/*.{js,jsx,ts,tsx}: For UI icons, use thesolid-iconslibrary or SVGs only. Do not use emojis.
Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Prefer using config files rather than hardcoding values
Each file should handle one coherent responsibility
Group related components in subdirectories with anindex.jsbarrel export
Files:
packages/workers/src/routes/admin.jspackages/workers/src/routes/users.js
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursorrules)
Use Solid's
createStorefor complex state or state objects in SolidJS components
Files:
packages/workers/src/routes/admin.jspackages/workers/src/routes/users.js
🧬 Code graph analysis (1)
packages/workers/src/routes/admin.js (1)
packages/workers/src/db/schema.js (12)
account(42-58)account(42-58)user(5-24)user(5-24)session(27-39)session(27-39)subscriptions(109-124)subscriptions(109-124)twoFactor(127-136)twoFactor(127-136)verification(61-68)verification(61-68)
⏰ 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). (2)
- GitHub Check: Workers Builds: corates-workers-prod
- GitHub Check: Workers Builds: corates
🔇 Additional comments (5)
packages/workers/src/routes/users.js (2)
8-17: LGTM: Comprehensive imports for atomic deletion.The extended imports correctly include
twoFactorandsubscriptions, enabling complete user data cleanup in the batch deletion operation below.
160-171: Excellent atomic deletion, but note inconsistency with admin endpoint.The batch transaction correctly ensures all-or-nothing deletion with proper foreign key ordering. However, line 169 uses
eq(verification.identifier, currentUser.email)while the admin endpoint (admin.js line 414) useslike(verification.identifier, \%${userToDelete.email}%`)`.The
eqapproach here is more precise and safer — it matches the exact email without wildcards that could accidentally match similar addresses (e.g., pattern "%user@example.com%" would also match "otheruser@example.com").The admin endpoint should adopt this
eqpattern for consistency and correctness.packages/workers/src/routes/admin.js (3)
8-17: LGTM: Complete schema imports for enhanced admin operations.The extended imports correctly support both the new provider/account aggregation features and the comprehensive atomic deletion operations.
217-227: LGTM: Clean account fetching for user details.The linked accounts query correctly retrieves and orders account provider information for the user detail endpoint.
257-269: Excellent use of atomic batch for ban operation.The batch transaction correctly ensures the user ban and session invalidation happen atomically, preventing inconsistent states where a user might be marked as banned but still have active sessions.
2 participants
Summary by CodeRabbit
New Features
UI
Backend
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.