can now merge orcid accounts, fix lint issues#136
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
corates | a0d9027 | Commit Preview URL | Dec 24 2025, 06:05 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| 🔵 In progress View logs |
corates | 0c8ecd6 | Dec 24 2025, 06:03 PM |
📝 WalkthroughWalkthroughThis PR adds ORCID ID support for account merging alongside existing email-based flows, introduces centralized API error handling via Changes
Sequence DiagramsequenceDiagram
actor User
participant Browser
participant OAuthProvider
participant LinkedAccountsSection
participant MergeDialog as MergeAccountsDialog
participant API as packages/web/api
participant WorkersAPI as packages/workers<br/>account-merge
participant DB as Database
User->>Browser: Click link ORCID account
Browser->>LinkedAccountsSection: Initiate OAuth flow
LinkedAccountsSection->>Browser: Store provider in sessionStorage
LinkedAccountsSection->>OAuthProvider: Redirect to OAuth provider
User->>OAuthProvider: Authorize
OAuthProvider->>Browser: Callback with auth code
Browser->>LinkedAccountsSection: Handle OAuth callback
LinkedAccountsSection->>LinkedAccountsSection: Detect provider from<br/>URL / sessionStorage / path
alt Account Already Linked
LinkedAccountsSection->>MergeDialog: Show with detected provider
User->>MergeDialog: Enter target email or ORCID
alt Email Path
MergeDialog->>API: initiateMerge(email, null)
API->>WorkersAPI: POST /account-merge/initiate<br/>{targetEmail: "..."}
else ORCID Path
MergeDialog->>API: initiateMerge(null, orcidId)
API->>WorkersAPI: POST /account-merge/initiate<br/>{targetOrcidId: "..."}
end
WorkersAPI->>WorkersAPI: Normalize ORCID<br/>(remove hyphens, `@orcid.org`)
alt Email Lookup
WorkersAPI->>DB: Query user by email
else ORCID Lookup
WorkersAPI->>DB: Query account by<br/>providerId='orcid'<br/>+ normalized accountId
DB->>WorkersAPI: Return linked account
WorkersAPI->>DB: Query user from<br/>account.userId
end
DB->>WorkersAPI: Return target user
WorkersAPI->>API: Return verification code
API->>MergeDialog: Show code input field
User->>MergeDialog: Enter verification code
MergeDialog->>API: completeMerge(code)
API->>WorkersAPI: POST /account-merge/complete<br/>{code: "..."}
WorkersAPI->>DB: Merge accounts &<br/>link new provider
DB->>WorkersAPI: Success
WorkersAPI->>API: Success response
API->>MergeDialog: Update state
Browser->>Browser: Redirect to success page
end
LinkedAccountsSection->>LinkedAccountsSection: Clear sessionStorage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/workers/scripts/reset-db-prod.mjs (3)
4-6: Update documentation to match implementation.The header comment claims this script will "clear R2 bucket," but the R2 bucket operations are commented out (lines 106-125). Update the documentation to accurately reflect that only the D1 database is reset.
Proposed fix
/** - * Reset production D1 database, clear R2 bucket, and redeploy workers + * Reset production D1 database and redeploy workers * * Usage: node scripts/reset-db-prod.mjs */
82-83: Update console message to match actual operations.The message claims the script is "Resetting Production D1 Database & R2," but R2 operations are commented out. This misleads operators about what the script actually does.
Proposed fix
console.log('=========================================='); - console.log(' Resetting Production D1 Database & R2'); + console.log(' Resetting Production D1 Database'); console.log('==========================================');
142-142: Update success message to reflect actual operations.The success message claims "R2 cleared," but no R2 operations are performed since they're commented out.
Proposed fix
- console.log(' Database reset, R2 cleared, and workers deployed!'); + console.log(' Database reset and workers deployed!');packages/ui/src/zag/PinInput.jsx (1)
84-84: Remove the redundant named export on line 84.The default export is already being re-exported as
PinInputthrough the barrel export inpackages/ui/src/zag/index.js(line 19), making the named export statement redundant. All existing consumers correctly import from@corates/uiand are unaffected. Removeexport { PinInputComponent as PinInput };to keep the export pattern clean and consistent with other components in the zag directory.
🧹 Nitpick comments (7)
packages/workers/scripts/reset-db-prod.mjs (1)
106-125: Consider removing commented-out code.This large block of commented-out R2 bucket operations adds clutter and maintenance burden. If these operations are no longer needed, remove the code entirely. If you plan to re-enable them later, consider adding a brief comment explaining why they're temporarily disabled or track the work in an issue.
packages/web/src/components/profile-ui/LinkedAccountsSection.jsx (1)
76-102: Good fallback chain for provider detection, but consider the default value.The multi-source fallback (URL → sessionStorage → path extraction) is a robust approach for recovering provider context across OAuth redirects. However, the hardcoded fallback to
'google'on line 94 may be misleading if the user was actually trying to link a different provider (e.g., ORCID).Consider showing a generic error message when the provider cannot be determined, or log a warning to help debug such cases:
- // Fallback to google if still not found - provider = provider || 'google'; + // Fallback to generic handling if provider cannot be determined + if (!provider) { + console.warn('Could not determine OAuth provider from URL, sessionStorage, or path'); + provider = 'google'; // Default fallback + }packages/workers/scripts/create-test-orcid-user.mjs (3)
82-93: ORCID normalization logic is duplicated.The
normalizeOrcidIdandisValidOrcidIdfunctions duplicate logic frompackages/workers/src/routes/account-merge.js. For a test script this is acceptable, but consider extracting to@corates/sharedif ORCID utilities are needed elsewhere.
250-254: Non-atomic user and account creation.The user and account are created in separate
runWranglerD1Executecalls. If the account insertion fails, an orphaned user record remains. For a local test script this is acceptable, but you could combine both SQL statements in a single command for atomicity.Suggested improvement
- // Create user - runWranglerD1Execute({ command: createUserSql, json: false }); - - // Create account - runWranglerD1Execute({ command: createAccountSql, json: false }); + // Create user and account atomically + runWranglerD1Execute({ + command: `${createUserSql} ${createAccountSql}`, + json: false + });
245-248: Redundant logging of already-normalized ORCID ID.At line 168,
orcidIdis already normalized vianormalizeOrcidId(). Line 248 logs bothorcidIdandnormalizeOrcidId(orcidId), which will be identical. Consider showing the original input vs. normalized output instead.packages/workers/src/routes/account-merge.js (1)
306-322: Additional DB query for ORCID ID in email path.When initiating via email, an extra query fetches the target's ORCID account for the response. This could be combined with the accounts query on lines 240-249, but for an infrequent merge operation, the impact is negligible.
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx (1)
63-79: ORCID input normalization and validation helpers.These functions duplicate logic from the workers route. Consider extracting to
@corates/sharedif ORCID utilities are needed across packages. For now, keeping them local to the component is acceptable.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
eslint.config.jspackage.jsonpackages/ui/package.jsonpackages/ui/src/index.d.tspackages/ui/src/zag/PinInput.jsxpackages/ui/src/zag/Select.jsxpackages/web/package.jsonpackages/web/src/api/account-merge.jspackages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/components/checklist-ui/pdf/pdfRenderer.jspackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsxpackages/workers/package.jsonpackages/workers/scripts/create-test-orcid-user.mjspackages/workers/scripts/reset-db-prod.mjspackages/workers/src/middleware/rateLimit.jspackages/workers/src/routes/account-merge.js
💤 Files with no reviewable changes (1)
- packages/web/src/components/checklist-ui/pdf/pdfRenderer.js
🧰 Additional context used
📓 Path-based instructions (18)
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use emojis in code, comments, documentation, or commit messages
NEVER use emojis anywhere in code, comments, documentation, plan files, or commit messages. This includes unicode symbols. For UI icons, use solid-icons library or SVGs only.
Files:
eslint.config.jspackages/workers/scripts/create-test-orcid-user.mjspackages/workers/src/middleware/rateLimit.jspackages/workers/src/routes/account-merge.jspackages/ui/package.jsonpackage.jsonpackages/workers/package.jsonpackages/ui/src/zag/PinInput.jsxpackages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/api/account-merge.jspackages/ui/src/index.d.tspackages/ui/src/zag/Select.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsxpackages/web/package.jsonpackages/workers/scripts/reset-db-prod.mjs
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features in JavaScript/TypeScript code
Comments should explain why something is being done, not narrate what the code does. Avoid comments that repeat variable names or describe obvious code behavior.
Files:
eslint.config.jspackages/workers/src/middleware/rateLimit.jspackages/workers/src/routes/account-merge.jspackages/ui/src/zag/PinInput.jsxpackages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/api/account-merge.jspackages/ui/src/index.d.tspackages/ui/src/zag/Select.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/workers/src/**/*.{js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/workers/src/**/*.{js,ts}: Use Zod for schema and input validation on the backend
Use Drizzle ORM for database interactions and migrations
Use Better-Auth for authentication and user management
packages/workers/src/**/*.{js,ts}: Use Zod for schema and input validation on the backend
Use Better-Auth for authentication and user management
packages/workers/src/**/*.{js,ts}: Always usecreateDomainErrorfrom@corates/sharedfor error handling in backend routes with predefined error constants (PROJECT_ERRORS, AUTH_ERRORS, VALIDATION_ERRORS, SYSTEM_ERRORS, USER_ERRORS)
Wrap database operations in try-catch blocks and return domain errors usingcreateDomainError(SYSTEM_ERRORS.DB_ERROR, {...})instead of throwing raw errors
Use validation middleware withvalidateRequest(schema)to handle validation errors automatically rather than manually creating validation errors in routes
Files:
packages/workers/src/middleware/rateLimit.jspackages/workers/src/routes/account-merge.js
packages/{web,workers}/src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
packages/{web,workers}/src/**/*.{js,ts,jsx,tsx}: Never throw string literals; always throw Error objects or return domain errors from API routes
Use error utility functions likeisErrorCodefrom@corates/sharedor@/lib/error-utils.jsto check specific error types instead of manual string comparisons
Files:
packages/workers/src/middleware/rateLimit.jspackages/workers/src/routes/account-merge.jspackages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/api/account-merge.jspackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/workers/src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/workers.mdc)
ALWAYS use
db.batch()for multiple related database operations to ensure atomicity in Drizzle transactions. Single independent operations do not need batch.
Files:
packages/workers/src/middleware/rateLimit.jspackages/workers/src/routes/account-merge.js
packages/workers/src/routes/**/*.js
📄 CodeRabbit inference engine (.cursor/rules/api-routes.mdc)
packages/workers/src/routes/**/*.js: Always usevalidateRequestmiddleware for request body validation in API routes, passing appropriate schema from validation config
UsevalidateQueryParamsmiddleware for validating query string parameters with Zod schemas
Always create database client from environment usingcreateDb(c.env.DB)in route handlers
Usedb.batch()for related database operations that must be atomic (succeed or fail together)
Always use Drizzle ORM for database queries; never use raw SQL
Always usecreateDomainErrorfrom@corates/sharedfor error responses, with appropriate error constants (PROJECT_ERRORS, AUTH_ERRORS, VALIDATION_ERRORS, SYSTEM_ERRORS, USER_ERRORS)
Never create error objects manually or throw error strings; use error constants from@corates/shared
Order middleware chain in routes as: authentication (requireAuth), authorization (requireEntitlement,requireQuota), validation (validateRequest,validateQueryParams), then route handler
UsegetAuth(c)to retrieve authenticated user information in route handlers
Usec.get('validatedBody')to access request body data after validation middleware
Usec.get('validatedQuery')to access query parameters after validation middleware
Files:
packages/workers/src/routes/account-merge.js
packages/workers/src/routes/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/workers.mdc)
ALWAYS validate request bodies using
validateRequestmiddleware from validation config. Do not perform manual validation.
Files:
packages/workers/src/routes/account-merge.js
packages/{web,ui}/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
Group related components in subdirectories with barrel exports
Files:
packages/ui/src/zag/PinInput.jsxpackages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/ui/src/zag/Select.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/{web,ui}/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
packages/{web,ui}/**/*.{js,jsx,ts,tsx}: Import UI components from '@corates/ui' package instead of local component files. Do not import Ark UI components from local paths like '@/components/zag/' or 'packages/web/src/components/zag/'
Always use 'solid-icons' library for icons. Never use emoji characters or text as icon replacements. Import from specific icon sets like 'solid-icons/bi', 'solid-icons/fi', 'solid-icons/ai', etc.
Files:
packages/ui/src/zag/PinInput.jsxpackages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/api/account-merge.jspackages/ui/src/index.d.tspackages/ui/src/zag/Select.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/{web,ui}/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Use Tailwind CSS classes for styling components
Files:
packages/ui/src/zag/PinInput.jsxpackages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/ui/src/zag/Select.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/web/src/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/**/*.{js,jsx,ts,tsx}: For UI icons, use thesolid-iconslibrary or SVGs only. Do not use emojis
Ensure browser compatibility for all frontend code (Safari is usually problematic)
Keep files small, focused, and modular. If a file exceeds a high number of lines, consider refactoring by extracting sub-modules into a folder with index.jsx and helper components, moving complex logic into separate utility files or primitives, or splitting large forms into section components
Do NOT prop-drill application state. Shared or cross-feature state must live in external stores under packages/web/src/stores/ or relative to the component file
UsecreateMemofor derived values to ensure they update reactivelyUse import aliases from jsconfig.json instead of relative paths
Files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/api/account-merge.jspackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/web/src/components/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/components/**/*.{js,jsx,ts,tsx}: Use responsive design principles for UI components
Group related components in subdirectories with an index.js barrel export
Use Zag.js for UI components and design system
Zag component exist inpackages/web/src/components/zag/*and should be reused. Check the README.md in that folder for a list of existing components before adding new components and when debugging
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
Do not destructure props in SolidJS components as it breaks reactivity. Instead, access props directly from the props object or wrap them in a function to ensure they are always up-to-date
Components should be lean and focused. They should 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/checklist-ui/SplitScreenLayout.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/{web,landing}/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
packages/{web,landing}/src/**/*.{jsx,tsx}: Use Ark UI components from @corates/ui package, not local component implementations
Use solid-icons library (e.g., solid-icons/bi, solid-icons/fi) for icon imports
Files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/web/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
packages/web/src/**/*.{jsx,tsx}: In SolidJS, do NOT prop-drill application state. Import stores directly where needed instead.
In SolidJS, do NOT destructure props. Access props.field directly or wrap in a function: () => props.field
In SolidJS components, components should receive at most 1-5 props (local config only, not shared state)
Files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/web/src/**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
In SolidJS, use createMemo for derived values
Files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/api/account-merge.jspackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/web/src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
packages/web/src/**/*.{js,ts,jsx,tsx}: Always usehandleFetchErrorfrom@/lib/error-utils.jsfor fetch calls in frontend code with options like{ showToast: true }for error handling
UsecreateFormErrorSignalsfrom@/lib/form-errors.jsfor form validation error handling with field-level and global error management
Files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/api/account-merge.jspackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
{packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursor/rules/solidjs.mdc)
{packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts}: Never destructure props in SolidJS components - destructuring breaks reactivity. Access props directly (e.g.,props.name) or wrap in a function (e.g.,const name = () => props.name) to maintain reactivity.
Import stores directly in components rather than prop-drilling store data through component hierarchies.
Use separate read and write patterns for stores: import the store directly for reading data (e.g.,projectStore.getProjectList()) and import action stores separately for writing (e.g.,projectActionsStore.createProject()).
UsecreateSignalfrom solid-js for managing simple reactive values. Prefer derived state with signals or memo over effects when possible.
UsecreateStorefrom solid-js/store for managing complex objects and arrays that require granular reactivity, enabling fine-grained updates where only affected parts re-render.
UsecreateMemofrom solid-js for derived values that depend on reactive state, ensuring computed values update only when their dependencies change.
Always clean up effects that create subscriptions or timers using theonCleanupfunction from solid-js. Use effects sparingly, only when derived values won't work well.
Keep components lean and focused on rendering. Move business logic to stores (for shared state and operations), primitives (for reusable hooks/logic), or utilities (for pure functions).
Use theShowcomponent from solid-js for conditional rendering instead of JavaScript ternary operators or logical AND operators.
Use theForcomponent from solid-js for rendering lists. It provides better performance and keying compared to JavaScript's map function in JSX.
When manipulating children in wrapper components, use thechildrenhelper from solid-js to ensure proper reactivity and handling of child elements.
Files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/api/account-merge.jspackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
packages/web/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Use import aliases from 'packages/web/jsconfig.json' instead of relative paths. Aliases include: '@/' (src/), '@components/' (src/components/), '@auth-ui/' (src/components/auth-ui/), '@checklist-ui/' (src/components/checklist-ui/), '@project-ui/' (src/components/project-ui/), '@routes/' (src/routes/), '@primitives/' (src/primitives/), '@api/' (src/api/), '@config/' (src/config/), and '@lib/' (src/lib/)
Files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/src/api/account-merge.jspackages/web/src/components/profile-ui/LinkedAccountsSection.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
🧠 Learnings (23)
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/workers/src/**/*.{js,ts} : Use Drizzle ORM for database interactions and migrations
Applied to files:
packages/workers/scripts/create-test-orcid-user.mjspackages/workers/src/routes/account-merge.jspackages/workers/package.json
📚 Learning: 2025-12-24T17:22:31.782Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/api-routes.mdc:0-0
Timestamp: 2025-12-24T17:22:31.782Z
Learning: Applies to packages/workers/src/routes/**/*.js : Always use Drizzle ORM for database queries; never use raw SQL
Applied to files:
packages/workers/src/routes/account-merge.js
📚 Learning: 2025-12-24T17:22:48.927Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-24T17:22:48.927Z
Learning: Applies to packages/workers/**/*.{ts,sql} : Use Drizzle ORM for ALL database interactions and migrations
Applied to files:
packages/workers/src/routes/account-merge.jspackages/workers/package.json
📚 Learning: 2025-12-24T17:23:17.309Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-12-24T17:23:17.309Z
Learning: Applies to packages/web/**/*.{js,jsx,ts,tsx} : Use import aliases from 'packages/web/jsconfig.json' instead of relative paths. Aliases include: '@/*' (src/*), 'components/*' (src/components/*), 'auth-ui/*' (src/components/auth-ui/*), 'checklist-ui/*' (src/components/checklist-ui/*), 'project-ui/*' (src/components/project-ui/*), 'routes/*' (src/routes/*), 'primitives/*' (src/primitives/*), 'api/*' (src/api/*), 'config/*' (src/config/*), and 'lib/*' (src/lib/*)
Applied to files:
packages/ui/package.jsonpackages/web/package.json
📚 Learning: 2025-12-24T17:22:48.927Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-24T17:22:48.927Z
Learning: Applies to packages/{web,landing}/src/**/*.{jsx,tsx} : Use solid-icons library (e.g., solid-icons/bi, solid-icons/fi) for icon imports
Applied to files:
packages/ui/package.jsonpackages/web/package.json
📚 Learning: 2025-12-24T17:23:17.309Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-12-24T17:23:17.309Z
Learning: Applies to packages/{web,ui}/**/*.{js,jsx,ts,tsx} : Always use 'solid-icons' library for icons. Never use emoji characters or text as icon replacements. Import from specific icon sets like 'solid-icons/bi', 'solid-icons/fi', 'solid-icons/ai', etc.
Applied to files:
packages/ui/package.jsonpackages/web/package.json
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/workers/src/**/*.{js,ts} : Use Better-Auth for authentication and user management
Applied to files:
packages/workers/package.jsonpackages/web/package.json
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/workers/src/**/*.{js,ts} : Use Zod for schema and input validation on the backend
Applied to files:
packages/workers/package.json
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : 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
Applied to files:
packages/ui/src/zag/PinInput.jsx
📚 Learning: 2025-12-24T17:22:48.927Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-24T17:22:48.927Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : In SolidJS components, components should receive at most 1-5 props (local config only, not shared state)
Applied to files:
packages/ui/src/zag/PinInput.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Use responsive design principles for UI components
Applied to files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Do not destructure props in SolidJS components as it breaks reactivity. Instead, access props directly from the props object or wrap them in a function to ensure they are always up-to-date
Applied to files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
📚 Learning: 2025-12-24T17:23:17.309Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-12-24T17:23:17.309Z
Learning: Applies to packages/{web,ui}/**/*.{jsx,tsx} : Use Tailwind CSS classes for styling components
Applied to files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Never destructure props in SolidJS components - destructuring breaks reactivity. Access props directly (e.g., `props.name`) or wrap in a function (e.g., `const name = () => props.name`) to maintain reactivity.
Applied to files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : When manipulating children in wrapper components, use the `children` helper from solid-js to ensure proper reactivity and handling of child elements.
Applied to files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
📚 Learning: 2025-12-24T17:22:48.927Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-24T17:22:48.927Z
Learning: Applies to packages/{web,landing}/src/**/*.{jsx,tsx} : Use Ark UI components from corates/ui package, not local component implementations
Applied to files:
packages/web/src/components/checklist-ui/SplitScreenLayout.jsxpackages/web/package.json
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Use `createMemo` from solid-js for derived values that depend on reactive state, ensuring computed values update only when their dependencies change.
Applied to files:
packages/ui/src/zag/Select.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/**/*.{js,jsx,ts,tsx} : Use `createMemo` for derived values to ensure they update reactively
Applied to files:
packages/ui/src/zag/Select.jsx
📚 Learning: 2025-12-24T17:22:48.927Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-24T17:22:48.927Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,js,ts} : In SolidJS, use createMemo for derived values
Applied to files:
packages/ui/src/zag/Select.jsxpackages/web/src/components/profile-ui/MergeAccountsDialog.jsx
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Use `createSignal` from solid-js for managing simple reactive values. Prefer derived state with signals or memo over effects when possible.
Applied to files:
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Use the `Show` component from solid-js for conditional rendering instead of JavaScript ternary operators or logical AND operators.
Applied to files:
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Always clean up effects that create subscriptions or timers using the `onCleanup` function from solid-js. Use effects sparingly, only when derived values won't work well.
Applied to files:
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx
📚 Learning: 2025-12-24T17:23:17.309Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-12-24T17:23:17.309Z
Learning: Applies to packages/{web,ui}/**/*.{js,jsx,ts,tsx} : Import UI components from 'corates/ui' package instead of local component files. Do not import Ark UI components from local paths like '@/components/zag/' or 'packages/web/src/components/zag/'
Applied to files:
packages/web/package.json
🧬 Code graph analysis (5)
packages/workers/scripts/create-test-orcid-user.mjs (5)
packages/workers/scripts/reset-db-prod.mjs (7)
args(42-53)args(59-70)args(76-76)result(20-25)stderr(28-28)stdout(29-29)message(30-30)packages/workers/src/routes/account-merge.js (2)
normalized(78-78)now(626-626)packages/ui/src/zag/Select.jsx (1)
value(27-27)packages/workers/src/__tests__/helpers.js (1)
json(219-226)packages/workers/src/middleware/rateLimit.js (1)
now(17-17)
packages/workers/src/routes/account-merge.js (3)
packages/workers/scripts/create-test-orcid-user.mjs (3)
orcidId(168-168)normalized(85-85)normalized(90-90)packages/workers/src/index.js (1)
c(204-204)packages/workers/src/db/client.js (1)
createDb(9-11)
packages/web/src/api/account-merge.js (2)
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx (2)
targetEmail(35-35)targetOrcidId(36-36)packages/web/src/lib/error-utils.js (1)
parseApiError(23-37)
packages/web/src/components/profile-ui/LinkedAccountsSection.jsx (1)
packages/web/src/primitives/useProject/connection.js (1)
provider(33-33)
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx (3)
packages/workers/src/routes/account-merge.js (2)
normalized(78-78)normalizedOrcidId(171-171)packages/web/src/components/profile-ui/AccountProviderCard.jsx (1)
isOrcid(52-52)packages/web/src/api/account-merge.js (1)
initiateMerge(18-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: corates
- GitHub Check: Workers Builds: corates-workers-prod
🔇 Additional comments (24)
packages/web/src/components/checklist-ui/SplitScreenLayout.jsx (2)
108-123: LGTM - Lint fix for CSS property naming.The change from
willChangeto'will-change'correctly uses the standard CSS property name. This maintains functionality while improving code consistency.
148-169: LGTM - Consistent CSS property naming.The change from
willChangeto'will-change'is consistent with the first panel and correctly applies the lint fix across both panels.package.json (1)
47-47: LGTM - Patch version bump for TypeScript ESLint parser.Minor maintenance update that should include only bug fixes.
packages/web/package.json (1)
22-22: Consider the implications of loosening version constraints on an auth library.Changing from an exact pin (
1.4.5) to a caret range (^1.4.5) allows automatic adoption of minor and patch updates. While this reduces maintenance overhead, authentication libraries are security-critical - unexpected behavior changes could affect user sign-in flows.If this change is intentional to pick up security patches automatically, that's reasonable. Otherwise, consider using a tilde (
~1.4.5) to limit to patch updates only, or keeping the exact pin for maximum stability.eslint.config.js (1)
33-34: LGTM - Valid browser globals added.
requestIdleCallbackandcancelIdleCallbackare standard browser APIs. Note thatrequestIdleCallbackis not supported in Safari - ensure any code using these APIs includes a fallback or polyfill for Safari compatibility (as mentioned in coding guidelines about Safari being problematic).packages/workers/src/middleware/rateLimit.js (1)
61-65: LGTM - Rate limiting bypass for non-production environments.Disabling rate limiting in development simplifies local testing. The check correctly targets non-production environments only, and the early return pattern is clean.
packages/ui/src/index.d.ts (1)
185-185: LGTM - Parameter naming convention applied to type definitions.The underscore prefix convention aligns with the ESLint
argsIgnorePattern: '^_'rule, indicating these parameters may be intentionally unused in implementations.Also applies to: 225-225
packages/ui/src/zag/Select.jsx (1)
71-82: LGTM - Good use of per-item memoization.The
createMemoinside theIndexcallback correctly provides per-item reactive memoization for the disabled state. This is idiomatic SolidJS and aligns with the coding guidelines for usingcreateMemofor derived values.packages/web/src/components/profile-ui/LinkedAccountsSection.jsx (2)
140-153: Good use of sessionStorage for preserving provider context across OAuth redirects.Storing the linking provider before the OAuth redirect and cleaning it up on error is a solid pattern. This ensures the provider can be recovered even if the callback URL doesn't include it.
312-338: LGTM - Dev-only testing UI is well-guarded.The
import.meta.env.DEVcheck correctly gates this testing UI to development builds only. This is useful for manually testing merge flows without triggering actual OAuth redirects.packages/ui/package.json (1)
36-36: jsdom 27.x upgrade is compatible and properly handled by the test setup.jsdom 27.x does include breaking changes (PointerEvent instead of MouseEvent for click(), user-agent stylesheet changes, VirtualConsole API changes). However, the test setup in
src/__tests__/setup.jsalready includes necessary mocks for jsdom 27.x compatibility (PointerEvent polyfill, getComputedStyle mock). vitest 4.0.16 is compatible with jsdom 27.x, and the project's Node version requirement (≥24.0.0) satisfies jsdom 27's minimum (Node 20+).packages/ui/src/zag/PinInput.jsx (3)
21-28: Good use ofsplitPropsfor prop handling.Proper SolidJS pattern to separate local props from those passed to
PinInput.Root, maintaining reactivity while allowing prop forwarding.
54-54: Correct use ofcreateMemofor derived array.The
inputIndicesmemoization ensures the array is only recomputed whencount()changes, following SolidJS best practices per coding guidelines.
74-78: Dynamic input rendering withIndexcomponent.Good use of
Indexfor rendering the dynamic number of inputs. This is the correct SolidJS pattern for rendering arrays where items have stable indices.packages/workers/src/routes/account-merge.js (3)
69-99: Well-implemented ORCID normalization and formatting utilities.The
normalizeOrcidIdandformatOrcidIdfunctions correctly handle:
- Removal of
@orcid.orgsuffix- Hyphen stripping/adding
- Checksum character 'X' in the last position
190-205: Complex ORCID lookup using Drizzle'ssqltemplate.The
sqltemplate literal is Drizzle's mechanism for complex expressions that can't be expressed in the type-safe API. The parameterization via${normalizedOrcidId}prevents SQL injection. This is acceptable per the coding guidelines as it's not raw SQL string concatenation.
145-153: Early self-merge check for email path.This early email-based self-merge check provides better UX by failing fast before rate limiting is applied. The duplicate check on lines 228-237 (ID-based) catches the ORCID path. This redundancy is intentional for UX optimization.
packages/web/src/components/profile-ui/MergeAccountsDialog.jsx (2)
57-58: Good use ofcreateMemofor ORCID conflict detection.The memoization ensures
isOrcidConflictis only recomputed whenprops.conflictProviderchanges, following SolidJS best practices.
280-280: Input type changed totextfor ORCID compatibility.Good change -
type='email'would reject ORCID ID format. Usingtexttype allows both email and ORCID inputs.packages/web/src/api/account-merge.js (3)
8-8: Centralized error handling withparseApiError.Good refactor to use
parseApiErrorconsistently across all API functions, ensuring uniform error handling and better error messages for consumers.
18-26: UpdatedinitiateMergesignature for ORCID support.Clean implementation that:
- Accepts either
targetEmailortargetOrcidId- Validates that at least one is provided
- Throws proper
Errorobject (not string) per coding guidelines
35-41: Correct error handling pattern.The pattern of checking
response.ok, parsing errors withparseApiError, and then parsing successful responses is consistent and correct. The thrown error object will be caught byhandleErrorin the component layer.packages/workers/scripts/create-test-orcid-user.mjs (1)
11-13: Missing explicitdotenvdependency.The script imports
dotenvbut it's not listed inpackage.jsondependencies. This may work if it's a transitive dependency, but for reliability, consider adding it explicitly to devDependencies or using a built-in approach likenode --env-file=.env.packages/workers/package.json (1)
29-43: Verify dependency version compatibility and test integration.Ensure the following dependency updates are compatible:
@cloudflare/vitest-pool-workers:^0.11.1drizzle-kit:^0.31.8better-auth:^1.4.9Confirm tests pass with these versions and there are no breaking changes.
| <p class='font-medium'>Verification code sent</p> | ||
| <p class='mt-1'> | ||
| Check <strong>{targetEmail()}</strong> for a 6-digit code. | ||
| Check{' '} | ||
| <strong>{targetOrcidId() ? formatOrcidId(targetOrcidId()) : targetEmail()}</strong>{' '} | ||
| for a 6-digit code. | ||
| </p> |
There was a problem hiding this comment.
Clarify verification code destination message.
When merging via ORCID, the verification code is sent to the email associated with that ORCID account, not the ORCID ID itself. The message "Check {targetOrcidId} for a 6-digit code" may confuse users since they can't receive codes via ORCID.
Consider adjusting the message to indicate the code was sent to the email associated with the ORCID account:
Suggested fix
<p class='mt-1'>
- Check{' '}
- <strong>{targetOrcidId() ? formatOrcidId(targetOrcidId()) : targetEmail()}</strong>{' '}
- for a 6-digit code.
+ {targetOrcidId() ? (
+ <>A 6-digit code has been sent to the email linked to ORCID <strong>{formatOrcidId(targetOrcidId())}</strong>.</>
+ ) : (
+ <>Check <strong>{targetEmail()}</strong> for a 6-digit code.</>
+ )}
</p>📝 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.
| <p class='font-medium'>Verification code sent</p> | |
| <p class='mt-1'> | |
| Check <strong>{targetEmail()}</strong> for a 6-digit code. | |
| Check{' '} | |
| <strong>{targetOrcidId() ? formatOrcidId(targetOrcidId()) : targetEmail()}</strong>{' '} | |
| for a 6-digit code. | |
| </p> | |
| <p class='font-medium'>Verification code sent</p> | |
| <p class='mt-1'> | |
| {targetOrcidId() ? ( | |
| <>A 6-digit code has been sent to the email linked to ORCID <strong>{formatOrcidId(targetOrcidId())}</strong>.</> | |
| ) : ( | |
| <>Check <strong>{targetEmail()}</strong> for a 6-digit code.</> | |
| )} | |
| </p> |
🤖 Prompt for AI Agents
In packages/web/src/components/profile-ui/MergeAccountsDialog.jsx around lines
323 to 328, the UI text currently says "Check {targetOrcidId} for a 6-digit
code" which implies the code is sent to an ORCID ID; change the message to
indicate the code was sent to the email associated with the ORCID account by
using targetEmail() (or a conditional that renders the target email when
targetOrcidId() is present) so it reads something like "Check the email
associated with this ORCID ({targetEmail()}) for a 6-digit code" or simply
"Check {targetEmail()} for a 6-digit code" to avoid user confusion.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.