chore/twitter-google-account-linking#701
Conversation
Frederick-88
changed the title
📝 WalkthroughWalkthroughThe Turnkey wallet package now publicly exports strategy types and defines dedicated OAuth, OAuth2, and provider response types. OAuth login and confirmation flows use the updated contracts, validate required credentials, and return typed results. A new method resolves the active organization and user before retrieving linked OAuth providers. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/wallets/wallet-turnkey/src/strategy/turnkey/turnkey.ts (1)
428-436: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
confirmOAuth2should returnexisting_account_detectedtoo.
TurnkeyOAuth2Responsealready includes that status, and the payload carriescredentialBundleandorganizationId, but this branch currently falls through tologinWithSession. Add it to the early return and widenTurnkeyOAuth2ConfirmResponseto includeTurnkeyExistingAccountDetectedResponse.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/wallets/wallet-turnkey/src/strategy/turnkey/turnkey.ts` around lines 428 - 436, Update confirmOAuth2 to return early when response.data.status is either link_required or existing_account_detected, and widen TurnkeyOAuth2ConfirmResponse to include TurnkeyExistingAccountDetectedResponse so the returned payload is correctly typed.
🧹 Nitpick comments (1)
packages/wallets/wallet-turnkey/src/strategy/turnkey/turnkey.ts (1)
463-476: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated org/user resolution across
linkOAuthProviderandgetCurrentOauthProviders.The
getWhoami→getUserssingle-user fallback block (Lines 463-476 and 497-510) is identical. Extract a private helper to keep the resolution logic in one place.♻️ Suggested helper
+ private async resolveCurrentUserId(organizationId: string): Promise<string> { + const indexedDbClient = await this.getIndexedDbClient() + const user = await indexedDbClient.getWhoami({ organizationId }) + + if (user?.userId) { + return user.userId + } + + const { users } = await indexedDbClient.getUsers({ organizationId }) + + if (users.length !== 1) { + throw new WalletException( + new Error('Unable to resolve current Turnkey user.'), + ) + } + + return users[0].userId + }Both methods can then call
const userId = await this.resolveCurrentUserId(organizationId)after theorganizationIdguard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/wallets/wallet-turnkey/src/strategy/turnkey/turnkey.ts` around lines 463 - 476, Extract the duplicated getWhoami-to-getUsers fallback from linkOAuthProvider and getCurrentOauthProviders into a private resolveCurrentUserId(organizationId) helper. Preserve the existing single-user validation and WalletException behavior, then have both methods call the helper after validating organizationId.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/wallets/wallet-turnkey/src/strategy/turnkey/turnkey.ts`:
- Around line 428-436: Update confirmOAuth2 to return early when
response.data.status is either link_required or existing_account_detected, and
widen TurnkeyOAuth2ConfirmResponse to include
TurnkeyExistingAccountDetectedResponse so the returned payload is correctly
typed.
---
Nitpick comments:
In `@packages/wallets/wallet-turnkey/src/strategy/turnkey/turnkey.ts`:
- Around line 463-476: Extract the duplicated getWhoami-to-getUsers fallback
from linkOAuthProvider and getCurrentOauthProviders into a private
resolveCurrentUserId(organizationId) helper. Preserve the existing single-user
validation and WalletException behavior, then have both methods call the helper
after validating organizationId.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a81e7b17-54d7-4557-ae9f-89e28dc42c31
📒 Files selected for processing (4)
packages/wallets/wallet-turnkey/src/index.tspackages/wallets/wallet-turnkey/src/strategy/turnkey/oauth.tspackages/wallets/wallet-turnkey/src/strategy/turnkey/turnkey.tspackages/wallets/wallet-turnkey/src/strategy/types.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/wallets/wallet-turnkey/src/strategy/turnkey/turnkey.ts (1)
437-468: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
getOauthProvidersisn’t aTurnkeyIndexedDbClientmethod — this call won’t type-check against@turnkey/sdk-browser5.16.1. Use the supported Turnkey/API client path for fetching OAuth providers instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/wallets/wallet-turnkey/src/strategy/turnkey/turnkey.ts` around lines 437 - 468, The getCurrentOauthProviders method calls unsupported getOauthProviders on TurnkeyIndexedDbClient. Replace this call with the supported Turnkey/API client method for retrieving OAuth providers, while preserving organizationId and resolved userId handling and returning the provider list.
🧹 Nitpick comments (1)
packages/wallets/wallet-turnkey/src/strategy/types.ts (1)
63-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNear-identical type name is error-prone:
TurnkeyOauthProvidervsTurnkeyOAuthProvider.This new object type differs from the existing provider identifier type
TurnkeyOAuthProvider(used as a parameter type inconfirmOAuth/confirmOAuth2/oauthLogin) only by the casing of "OAuth" vs "Oauth". They represent completely different concepts (a linked-provider record vs a provider selector), so this collision is very easy to mix up and hard to catch in review. Consider a clearer name such asTurnkeyLinkedOauthProvider.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/wallets/wallet-turnkey/src/strategy/types.ts` around lines 63 - 69, Rename the new linked-provider object type TurnkeyOauthProvider to a distinct name such as TurnkeyLinkedOauthProvider, updating all imports, exports, annotations, and references while leaving the existing TurnkeyOAuthProvider selector type unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/wallets/wallet-turnkey/src/strategy/turnkey/turnkey.ts`:
- Around line 437-468: The getCurrentOauthProviders method calls unsupported
getOauthProviders on TurnkeyIndexedDbClient. Replace this call with the
supported Turnkey/API client method for retrieving OAuth providers, while
preserving organizationId and resolved userId handling and returning the
provider list.
---
Nitpick comments:
In `@packages/wallets/wallet-turnkey/src/strategy/types.ts`:
- Around line 63-69: Rename the new linked-provider object type
TurnkeyOauthProvider to a distinct name such as TurnkeyLinkedOauthProvider,
updating all imports, exports, annotations, and references while leaving the
existing TurnkeyOAuthProvider selector type unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c1c17990-cbc0-4df3-8805-dbc109f29539
📒 Files selected for processing (3)
packages/wallets/wallet-turnkey/src/strategy/turnkey/oauth.tspackages/wallets/wallet-turnkey/src/strategy/turnkey/turnkey.tspackages/wallets/wallet-turnkey/src/strategy/types.ts
1 participant
Summary by CodeRabbit