fix(core): fix OAuth token access per user - #242
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThe PR updates OAuth token expiry types, user-scoped OAuth account lookup, callback handling, refresh checks, device persistence, and extensive stateful, shared utility, and Elysia integration tests. ChangesOAuth stateful flow
Shared and integration validation
Device persistence
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
packages/prisma/src/adapter.ts (1)
341-346: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve the existing device type when
input.typeis omitted.
stripNullishValues(input)removes omitted fields, but this assignment addstype: "UNKNOWN"back to every partial update. A call that updates only another field will change an existing device type toUNKNOWN.Only convert and write
typewheninput.typeis defined.Proposed fix
- type: toDeviceType(input.type ?? "unknown"), + type: input.type === undefined ? undefined : toDeviceType(input.type),🤖 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/prisma/src/adapter.ts` around lines 341 - 346, Update the device update data construction in the client.device.update flow so type is included only when input.type is defined; otherwise preserve the existing type by relying on stripNullishValues(input). Continue converting provided values with toDeviceType, without defaulting omitted types to "unknown".
🧹 Nitpick comments (1)
packages/core/src/session/stateful.ts (1)
746-748: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a cross-user regression test for the account lookup.
For a session belonging to User A, an OAuth account for the same provider belonging to User B must not be returned. Test both the matching-account and missing-account paths.
🤖 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/core/src/session/stateful.ts` around lines 746 - 748, Add a regression test covering the account lookup around getAccountsByUserId and getOAuthAccount: create sessions for two users where User B has the same OAuth provider, then verify User A’s matching-account path returns only User A’s account and the missing-account path returns no account. Ensure the assertions confirm the lookup is scoped to the session user rather than provider alone.
🤖 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.
Inline comments:
In `@packages/core/src/`@types/session.ts:
- Around line 349-352: Complete the OAuth expiry-contract migration across
packages/core/src/@types/session.ts lines 349-352 and
packages/core/src/shared/utils.ts line 209: update every token payload producer
to populate accessTokenExpiresAt before making it required in the session type,
then update the refresh/expiry logic in the relevant utility function to compare
against accessTokenExpiresAt while preserving the existing guard semantics for
the refresh-token requirement.
---
Outside diff comments:
In `@packages/prisma/src/adapter.ts`:
- Around line 341-346: Update the device update data construction in the
client.device.update flow so type is included only when input.type is defined;
otherwise preserve the existing type by relying on stripNullishValues(input).
Continue converting provided values with toDeviceType, without defaulting
omitted types to "unknown".
---
Nitpick comments:
In `@packages/core/src/session/stateful.ts`:
- Around line 746-748: Add a regression test covering the account lookup around
getAccountsByUserId and getOAuthAccount: create sessions for two users where
User B has the same OAuth provider, then verify User A’s matching-account path
returns only User A’s account and the missing-account path returns no account.
Ensure the assertions confirm the lookup is scoped to the session user rather
than provider alone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ce85035a-7755-4abc-8a8e-a6fc219f8384
📒 Files selected for processing (4)
packages/core/src/@types/session.tspackages/core/src/session/stateful.tspackages/core/src/shared/utils.tspackages/prisma/src/adapter.ts
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/session/stateful.ts (2)
1318-1333: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReactivate the account status when an existing OAuth account re-authenticates.
In the existing-account branch,
updateOAuthTokenspersists the new tokens, but nothing callsupdateAccountStatusto set the account back to"active". If the account was previously unlinked (viarevokeToken), a fresh, successful OAuth callback still leaves its status as"unlinked". Downstream,isProviderConnectedchecksaccount.status === "active", so it would keep reporting the account as disconnected even though valid tokens now exist.Update the account status to
"active"alongside the token update in this branch.🤖 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/core/src/session/stateful.ts` around lines 1318 - 1333, The existing-account branch updates OAuth tokens without restoring the account status. In the flow around getAccountByProvider and updateOAuthTokens, also call updateAccountStatus for account.id with status "active" after successful re-authentication, preserving the existing token persistence behavior.
894-1002: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftNo change needed for
revokeTokenaccount lookup.
getOAuthAccountaccepts an OAuth account ID rather than a provider slug; the unlinked provider slug must be resolved first, as ingetProviderTokens, and its matchingAccountEntity.idmust be passed togetOAuthAccountbefore callingrevokeProviderToken,updateOAuthTokens, orupdateAccountStatus.🤖 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/core/src/session/stateful.ts` around lines 894 - 1002, Update revokeToken to resolve the unlinked provider slug from oauthId before calling getOAuthAccount, following the lookup used by getProviderTokens. Pass the matching AccountEntity.id to getOAuthAccount, then use that resolved account for revokeProviderToken and updateAccountStatus (and related OAuth account updates) while preserving existing behavior.
🧹 Nitpick comments (2)
packages/core/test/shared/assert.test.ts (1)
419-449: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the last three case descriptions unique, and drop the redundant template literal.
The description "invalid pattern with wildcard in the middle of the domain" repeats at lines 420, 432, and 438. Vitest then reports three tests with the same name, so a failure does not identify the case. Line 446 also wraps
descriptionin a template literal without interpolation.♻️ Proposed rename and simplification
{ - description: "invalid pattern with wildcard in the middle of the domain", + description: "invalid pattern with wildcard inside the domain label", url: "https://example.com", trustedOrigins: ["https://exa*mple.com"], expected: false, }, @@ { - description: "invalid pattern with wildcard in the middle of the domain", + description: "invalid pattern with wildcard as the middle label", url: "https://example.com", trustedOrigins: ["https://example.*.com"], expected: false, }, { - description: "invalid pattern with wildcard in the middle of the domain", + description: "invalid pattern with wildcard as the middle label and a subdomain URL", url: "https://api.example.com", trustedOrigins: ["https://api.*.com"], expected: false, }, ] for (const { description, url, trustedOrigins, expected } of testCases) { - test(`${description}`, () => { + test(description, () => { expect(isTrustedOrigin(url, trustedOrigins)).toBe(expected) }) }🤖 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/core/test/shared/assert.test.ts` around lines 419 - 449, Update the duplicate descriptions in the final three test cases within the testCases array so each case name uniquely identifies its wildcard placement or URL, and simplify the test name assertion in the test loop by passing description directly instead of wrapping it in an unnecessary template literal.packages/core/test/shared/secure.test.ts (1)
5-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the verifier length and the literal method value.
This test checks determinism only.
expect(expected.method).toBe(method)compares two results of the same function, so it passes even if the method changes from"S256". The test name claims validation of the verifier and the challenge, but neither is checked directly.
createPKCE(packages/core/src/shared/crypto.ts:29-38) requires a verifier length of 43 to 128 and returns"S256". Assert both.💚 Proposed added assertions
test("generates a valid code verifier and code challenge", async () => { const { codeVerifier, codeChallenge, method } = await createPKCE() + expect(codeVerifier.length).toBeGreaterThanOrEqual(43) + expect(codeVerifier.length).toBeLessThanOrEqual(128) + expect(method).toBe("S256") + expect(codeChallenge).not.toBe(codeVerifier) + const expected = await createPKCE(codeVerifier) expect(expected.codeChallenge).toBe(codeChallenge) expect(expected.method).toBe(method) })🤖 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/core/test/shared/secure.test.ts` around lines 5 - 11, Update the test around createPKCE to directly assert that codeVerifier has a length between 43 and 128 characters and that method equals the literal "S256"; retain the existing codeChallenge determinism assertion.
🤖 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.
Inline comments:
In `@packages/core/src/session/stateful.ts`:
- Around line 746-748: Update the account lookup in getProviderTokens to match
only accounts whose provider equals oauthId and whose status is "active",
consistent with isProviderConnected. Preserve the existing OAuth account
retrieval flow for active matches and return no account for disconnected or
unlinked accounts.
In `@packages/core/test/presets.ts`:
- Around line 54-62: Update the oauthTokens fixture to satisfy the
OAuthTokenPayload contract by replacing the deprecated expiresAt field with
accessTokenExpiresAt, while preserving the existing expiration value and other
token fields.
In `@packages/core/test/shared/request.test.ts`:
- Around line 77-94: Update the fetch stub in the “fetch resolving at timeout
boundary succeeds” test to observe the provided AbortSignal and reject when it
is aborted, while still resolving at the 1000ms timer otherwise. Keep the
existing fetchAsync call and boundary assertion so the test verifies the
intended timer-ordering contract.
In `@packages/elysia/test/stateful/index.test.ts`:
- Around line 202-208: Update the OAuth callback test around the request to
first create an authenticated session cookie for the existing user, include that
cookie in the callback request headers, and use the callback session cookie when
verifying the linked Google and GitHub accounts. Preserve the differing mocked
provider emails so the assertions validate session-based linking rather than
email matching.
---
Outside diff comments:
In `@packages/core/src/session/stateful.ts`:
- Around line 1318-1333: The existing-account branch updates OAuth tokens
without restoring the account status. In the flow around getAccountByProvider
and updateOAuthTokens, also call updateAccountStatus for account.id with status
"active" after successful re-authentication, preserving the existing token
persistence behavior.
- Around line 894-1002: Update revokeToken to resolve the unlinked provider slug
from oauthId before calling getOAuthAccount, following the lookup used by
getProviderTokens. Pass the matching AccountEntity.id to getOAuthAccount, then
use that resolved account for revokeProviderToken and updateAccountStatus (and
related OAuth account updates) while preserving existing behavior.
---
Nitpick comments:
In `@packages/core/test/shared/assert.test.ts`:
- Around line 419-449: Update the duplicate descriptions in the final three test
cases within the testCases array so each case name uniquely identifies its
wildcard placement or URL, and simplify the test name assertion in the test loop
by passing description directly instead of wrapping it in an unnecessary
template literal.
In `@packages/core/test/shared/secure.test.ts`:
- Around line 5-11: Update the test around createPKCE to directly assert that
codeVerifier has a length between 43 and 128 characters and that method equals
the literal "S256"; retain the existing codeChallenge determinism assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2038872d-ca03-4c89-ab1c-4678aa66c4fd
📒 Files selected for processing (14)
packages/core/src/session/stateful.tspackages/core/test/actions/callback/stateful.test.tspackages/core/test/actions/providers/tokens/tokens/stateful.test.tspackages/core/test/actions/providers/user/refresh/stateful.test.tspackages/core/test/api/stateful/getAccessToken.test.tspackages/core/test/api/stateful/getProviderTokens.test.tspackages/core/test/api/stateful/refreshUserInfo.test.tspackages/core/test/presets.tspackages/core/test/shared/assert.test.tspackages/core/test/shared/request.test.tspackages/core/test/shared/secure.test.tspackages/elysia/test/stateful/app.tspackages/elysia/test/stateful/index.test.tspackages/elysia/vitest.config.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/session/stateful.ts (2)
1318-1333: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReactivate the account status when an existing OAuth account re-authenticates.
In the existing-account branch,
updateOAuthTokenspersists the new tokens, but nothing callsupdateAccountStatusto set the account back to"active". If the account was previously unlinked (viarevokeToken), a fresh, successful OAuth callback still leaves its status as"unlinked". Downstream,isProviderConnectedchecksaccount.status === "active", so it would keep reporting the account as disconnected even though valid tokens now exist.Update the account status to
"active"alongside the token update in this branch.🤖 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/core/src/session/stateful.ts` around lines 1318 - 1333, The existing-account branch updates OAuth tokens without restoring the account status. In the flow around getAccountByProvider and updateOAuthTokens, also call updateAccountStatus for account.id with status "active" after successful re-authentication, preserving the existing token persistence behavior.
894-1002: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftNo change needed for
revokeTokenaccount lookup.
getOAuthAccountaccepts an OAuth account ID rather than a provider slug; the unlinked provider slug must be resolved first, as ingetProviderTokens, and its matchingAccountEntity.idmust be passed togetOAuthAccountbefore callingrevokeProviderToken,updateOAuthTokens, orupdateAccountStatus.🤖 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/core/src/session/stateful.ts` around lines 894 - 1002, Update revokeToken to resolve the unlinked provider slug from oauthId before calling getOAuthAccount, following the lookup used by getProviderTokens. Pass the matching AccountEntity.id to getOAuthAccount, then use that resolved account for revokeProviderToken and updateAccountStatus (and related OAuth account updates) while preserving existing behavior.
🧹 Nitpick comments (2)
packages/core/test/shared/assert.test.ts (1)
419-449: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the last three case descriptions unique, and drop the redundant template literal.
The description "invalid pattern with wildcard in the middle of the domain" repeats at lines 420, 432, and 438. Vitest then reports three tests with the same name, so a failure does not identify the case. Line 446 also wraps
descriptionin a template literal without interpolation.♻️ Proposed rename and simplification
{ - description: "invalid pattern with wildcard in the middle of the domain", + description: "invalid pattern with wildcard inside the domain label", url: "https://example.com", trustedOrigins: ["https://exa*mple.com"], expected: false, }, @@ { - description: "invalid pattern with wildcard in the middle of the domain", + description: "invalid pattern with wildcard as the middle label", url: "https://example.com", trustedOrigins: ["https://example.*.com"], expected: false, }, { - description: "invalid pattern with wildcard in the middle of the domain", + description: "invalid pattern with wildcard as the middle label and a subdomain URL", url: "https://api.example.com", trustedOrigins: ["https://api.*.com"], expected: false, }, ] for (const { description, url, trustedOrigins, expected } of testCases) { - test(`${description}`, () => { + test(description, () => { expect(isTrustedOrigin(url, trustedOrigins)).toBe(expected) }) }🤖 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/core/test/shared/assert.test.ts` around lines 419 - 449, Update the duplicate descriptions in the final three test cases within the testCases array so each case name uniquely identifies its wildcard placement or URL, and simplify the test name assertion in the test loop by passing description directly instead of wrapping it in an unnecessary template literal.packages/core/test/shared/secure.test.ts (1)
5-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the verifier length and the literal method value.
This test checks determinism only.
expect(expected.method).toBe(method)compares two results of the same function, so it passes even if the method changes from"S256". The test name claims validation of the verifier and the challenge, but neither is checked directly.
createPKCE(packages/core/src/shared/crypto.ts:29-38) requires a verifier length of 43 to 128 and returns"S256". Assert both.💚 Proposed added assertions
test("generates a valid code verifier and code challenge", async () => { const { codeVerifier, codeChallenge, method } = await createPKCE() + expect(codeVerifier.length).toBeGreaterThanOrEqual(43) + expect(codeVerifier.length).toBeLessThanOrEqual(128) + expect(method).toBe("S256") + expect(codeChallenge).not.toBe(codeVerifier) + const expected = await createPKCE(codeVerifier) expect(expected.codeChallenge).toBe(codeChallenge) expect(expected.method).toBe(method) })🤖 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/core/test/shared/secure.test.ts` around lines 5 - 11, Update the test around createPKCE to directly assert that codeVerifier has a length between 43 and 128 characters and that method equals the literal "S256"; retain the existing codeChallenge determinism assertion.
🤖 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.
Inline comments:
In `@packages/core/src/session/stateful.ts`:
- Around line 746-748: Update the account lookup in getProviderTokens to match
only accounts whose provider equals oauthId and whose status is "active",
consistent with isProviderConnected. Preserve the existing OAuth account
retrieval flow for active matches and return no account for disconnected or
unlinked accounts.
In `@packages/core/test/presets.ts`:
- Around line 54-62: Update the oauthTokens fixture to satisfy the
OAuthTokenPayload contract by replacing the deprecated expiresAt field with
accessTokenExpiresAt, while preserving the existing expiration value and other
token fields.
In `@packages/core/test/shared/request.test.ts`:
- Around line 77-94: Update the fetch stub in the “fetch resolving at timeout
boundary succeeds” test to observe the provided AbortSignal and reject when it
is aborted, while still resolving at the 1000ms timer otherwise. Keep the
existing fetchAsync call and boundary assertion so the test verifies the
intended timer-ordering contract.
In `@packages/elysia/test/stateful/index.test.ts`:
- Around line 202-208: Update the OAuth callback test around the request to
first create an authenticated session cookie for the existing user, include that
cookie in the callback request headers, and use the callback session cookie when
verifying the linked Google and GitHub accounts. Preserve the differing mocked
provider emails so the assertions validate session-based linking rather than
email matching.
---
Outside diff comments:
In `@packages/core/src/session/stateful.ts`:
- Around line 1318-1333: The existing-account branch updates OAuth tokens
without restoring the account status. In the flow around getAccountByProvider
and updateOAuthTokens, also call updateAccountStatus for account.id with status
"active" after successful re-authentication, preserving the existing token
persistence behavior.
- Around line 894-1002: Update revokeToken to resolve the unlinked provider slug
from oauthId before calling getOAuthAccount, following the lookup used by
getProviderTokens. Pass the matching AccountEntity.id to getOAuthAccount, then
use that resolved account for revokeProviderToken and updateAccountStatus (and
related OAuth account updates) while preserving existing behavior.
---
Nitpick comments:
In `@packages/core/test/shared/assert.test.ts`:
- Around line 419-449: Update the duplicate descriptions in the final three test
cases within the testCases array so each case name uniquely identifies its
wildcard placement or URL, and simplify the test name assertion in the test loop
by passing description directly instead of wrapping it in an unnecessary
template literal.
In `@packages/core/test/shared/secure.test.ts`:
- Around line 5-11: Update the test around createPKCE to directly assert that
codeVerifier has a length between 43 and 128 characters and that method equals
the literal "S256"; retain the existing codeChallenge determinism assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2038872d-ca03-4c89-ab1c-4678aa66c4fd
📒 Files selected for processing (14)
packages/core/src/session/stateful.tspackages/core/test/actions/callback/stateful.test.tspackages/core/test/actions/providers/tokens/tokens/stateful.test.tspackages/core/test/actions/providers/user/refresh/stateful.test.tspackages/core/test/api/stateful/getAccessToken.test.tspackages/core/test/api/stateful/getProviderTokens.test.tspackages/core/test/api/stateful/refreshUserInfo.test.tspackages/core/test/presets.tspackages/core/test/shared/assert.test.tspackages/core/test/shared/request.test.tspackages/core/test/shared/secure.test.tspackages/elysia/test/stateful/app.tspackages/elysia/test/stateful/index.test.tspackages/elysia/vitest.config.ts
🛑 Comments failed to post (1)
packages/core/test/shared/request.test.ts (1)
77-94: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
This test does not verify the boundary behavior that its name states.
fetchAsyncregisters the abort timer beforefetchregisters the inner timer.vi.advanceTimersByTime(1000)therefore callscontroller.abort()first. The stubbed fetch ignoressignal, so it resolves regardless. The assertion passes for the wrong reason, and a signal-aware fetch would reject at the same boundary.Make the stub honor the signal. The test then documents the actual contract at the boundary.
💚 Proposed signal-aware stub
vi.stubGlobal( "fetch", - () => - new Promise((resolve) => { - setTimeout(() => resolve("OK"), 1000) + (_: unknown, { signal }: RequestInit = {}) => + new Promise((resolve, reject) => { + signal?.addEventListener("abort", () => + reject(new DOMException("Aborted Request", "AbortError")) + ) + setTimeout(() => resolve("OK"), 999) }) )📝 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.test("fetch resolving at timeout boundary succeeds", async () => { vi.useFakeTimers() vi.stubGlobal( "fetch", (_: unknown, { signal }: RequestInit = {}) => new Promise((resolve, reject) => { signal?.addEventListener("abort", () => reject(new DOMException("Aborted Request", "AbortError")) ) setTimeout(() => resolve("OK"), 999) }) ) const promise = fetchAsync("https://example.com/timeout", {}, 1000) vi.advanceTimersByTime(1000) await expect(promise).resolves.toBe("OK") vi.useRealTimers() })🤖 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/core/test/shared/request.test.ts` around lines 77 - 94, Update the fetch stub in the “fetch resolving at timeout boundary succeeds” test to observe the provided AbortSignal and reject when it is aborted, while still resolving at the 1000ms timer otherwise. Keep the existing fetchAsync call and boundary assertion so the test verifies the intended timer-ordering contract.
1 participant
Description
This pull request fixes an issue in the OAuth account token retrieval flow and prevents unnecessary token refresh attempts when expiration or refresh token metadata is unavailable.
Previously, OAuth account tokens were not always retrieved correctly for the authenticated user. In some cases, requesting provider tokens could incorrectly invalidate the current session. This issue has been fixed by ensuring that the OAuth account is first resolved for the authenticated user and then validated against the requested provider before any token operations are performed.
Additionally, the token refresh logic has been improved to avoid refresh attempts when the required expiration or refresh token information is missing.
Key Changes
accessTokenExpiresAtfield to explicitly represent access token expiration.expiresAtfield in favor ofaccessTokenExpiresAt.@coderabbitai ignore