fix(worker): refresh OAuth tokens in backend permission sync flow - #1000

Merged
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664
Mar 13, 2026
Merged

fix(worker): refresh OAuth tokens in backend permission sync flow#1000
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • OAuth token refresh was previously only triggered from the Next.js jwt callback, meaning tokens could expire between user visits and cause account-driven permission sync jobs to fail with auth errors.
  • Moves token refresh logic to packages/backend/src/ee/tokenRefresh.ts and calls ensureFreshAccountToken from accountPermissionSyncer before using an account's access token.
  • Adds tokenRefreshErrorMessage to the Account DB model — set on refresh failure, cleared on successful re-authentication — and surfaces it in the linked accounts UI so users know to re-authenticate.

Test plan

  • Verify that an expired OAuth token is automatically refreshed when a permission sync job runs
  • Verify that when refresh fails, tokenRefreshErrorMessage is set on the account and the "Token refresh failed — please reconnect" message appears in the linked accounts UI
  • Verify that successfully re-authenticating clears tokenRefreshErrorMessage and removes the error from the UI
  • Verify existing permission sync flows (GitHub, GitLab, Bitbucket Cloud, Bitbucket Server) still work correctly when tokens are valid

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Re-authentication prompts appear when OAuth token refresh fails to help restore account access.
    • UI refreshes automatically after successful permission refresh to show up-to-date linked account status.
  • Bug Fixes

    • Token refresh handling consolidated to backend for more reliable expiration handling and clearer per-account error reporting.
    • Backend now records token refresh error messages on accounts to aid recovery.

Token refresh was previously only triggered from the Next.js jwt callback,
meaning tokens could expire between user visits and cause account-driven
permission sync jobs to fail silently.
Move refresh logic to packages/backend/src/ee/tokenRefresh.ts and call it
from accountPermissionSyncer before using an account's access token. On
refresh failure, tokenRefreshErrorMessage is set on the Account record and
surfaced in the linked accounts UI so users know to re-authenticate.
Also adds a DB migration for the tokenRefreshErrorMessage field and wires
the signIn event to clear it on successful re-authentication.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Centralizes OAuth token refresh into a backend function (ensureFreshAccountToken(account, db)), persists token refresh errors on the Account record, and updates frontend flows to read/clear per-account token refresh errors instead of session-based provider errors.

Changes

Cohort / File(s)Summary
Database Schema
packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql, packages/db/prisma/schema.prisma
Adds optional tokenRefreshErrorMessage TEXT column/field to Account to persist OAuth refresh errors.
Backend Token Refresh & Syncer
packages/backend/src/ee/tokenRefresh.ts, packages/backend/src/ee/accountPermissionSyncer.ts
Introduces ensureFreshAccountToken(account, db) that validates, decrypts, refreshes tokens, updates DB, and records errors. accountPermissionSyncer now delegates token freshness to this function and removes local decrypt/validation logic.
Frontend Auth Changes
packages/web/src/auth.ts
Removes linkedAccountErrors session/JWT typings and runtime token-refresh logic; clears tokenRefreshErrorMessage on successful sign-in.
SSO UI / Actions
packages/web/src/ee/features/sso/actions.ts, packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
Switches linked-account error source to account.tokenRefreshErrorMessage; triggers router.refresh() after permission refresh to re-fetch UI data.
Adapter / Types
packages/web/src/lib/encryptedPrismaAdapter.ts
Extends encryptAccountData() input shape to accept optional tokenRefreshErrorMessage.
Changelog
CHANGELOG.md
Adds entry describing backend token refresh relocation and error surfacing changes.

Sequence Diagram

sequenceDiagram
participant Syncer as AccountPermissionSyncer
participant Refresh as ensureFreshAccountToken
participant Provider as OAuth Provider
participant DB as Database
participant UI as Frontend UI
Syncer->>Refresh: ensureFreshAccountToken(account, db)
Refresh->>Refresh: Validate access_token presence
Refresh->>Refresh: Check expiry (with buffer)
alt Token valid
Refresh->>Refresh: Decrypt & return access_token
Refresh-->>Syncer: access_token
else Token expired/near expiry
Refresh->>Refresh: Ensure refresh_token exists
Refresh->>Provider: refreshOAuthToken(request with provider creds)
alt Refresh successful
Provider-->>Refresh: token response
Refresh->>DB: Update Account (access_token, refresh_token?, expires_at), clear tokenRefreshErrorMessage
DB-->>Refresh: OK
Refresh-->>Syncer: new access_token
else Refresh failed
Provider-->>Refresh: Error
Refresh->>DB: set tokenRefreshErrorMessage on Account
DB-->>Refresh: OK
Refresh-->>Syncer: throw/error
end
end
Syncer->>Syncer: Continue permission sync using token
UI->>DB: Fetch Account (includes tokenRefreshErrorMessage)
DB-->>UI: Account with tokenRefreshErrorMessage
UI->>UI: Surface message / prompt re-authentication
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • msukkari
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe PR title clearly and accurately describes the main change: moving OAuth token refresh logic into the backend permission sync flow, which is the core objective of the changeset.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch brendan/fix-token-refresh-in-permission-sync-SOU-664
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/web/src/ee/features/sso/actions.ts (1)

54-60: ⚠️ Potential issue | 🟡 Minor

Don't send the raw refresh failure text to the client.

linkedAccountProviderCard.tsx only checks linkedAccount.error for truthiness, so returning account.tokenRefreshErrorMessage here just leaks backend diagnostics to the browser. Collapse it to a boolean or sentinel string instead.

Possible minimal change
- error: account.tokenRefreshErrorMessage ?? undefined,+ error: account.tokenRefreshErrorMessage ? "token_refresh_failed" : undefined,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/ee/features/sso/actions.ts` around lines 54 - 60, The code
currently exposes backend refresh failure text by assigning
account.tokenRefreshErrorMessage to the result.error field in the object pushed
by the result.push in actions.ts; change this to emit a non-sensitive sentinel
or boolean (e.g. error: !!account.tokenRefreshErrorMessage or error:
"REFRESH_FAILED") instead of the raw string so linkedAccountProviderCard.tsx can
still check truthiness without leaking diagnostics; update the object property
where provider/isLinked/accountId/providerAccountId/isAccountLinking are set to
use the boolean/sentinel and ensure undefined is used when there was no error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/ee/tokenRefresh.ts`:
- Around line 123-136: The account writes in db.account.update (used when
writing encryptOAuthToken(refreshResponse.access_token) / refresh_token /
expires_at) and in setTokenRefreshError are vulnerable to races because they
unconditionally overwrite from the job-start snapshot; to fix, serialize or use
compare-and-set: either acquire a per-account mutex (e.g., Redis lock keyed by
account.id) around the token refresh flow to ensure only one refresh runs for an
account, or change the updates to conditional updates that include the snapshot
you read (e.g., use updateMany/update with a where that includes the account.id
plus a snapshot column like account.updatedAt or the previous
access_token/refresh_token value read earlier) so the write only applies if the
stored values match the snapshot; apply the same pattern to the
setTokenRefreshError path.
---
Outside diff comments:
In `@packages/web/src/ee/features/sso/actions.ts`:
- Around line 54-60: The code currently exposes backend refresh failure text by
assigning account.tokenRefreshErrorMessage to the result.error field in the
object pushed by the result.push in actions.ts; change this to emit a
non-sensitive sentinel or boolean (e.g. error:
!!account.tokenRefreshErrorMessage or error: "REFRESH_FAILED") instead of the
raw string so linkedAccountProviderCard.tsx can still check truthiness without
leaking diagnostics; update the object property where
provider/isLinked/accountId/providerAccountId/isAccountLinking are set to use
the boolean/sentinel and ensure undefined is used when there was no error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 29444165-81cc-4596-86a9-2b25d4674472

📥 Commits

Reviewing files that changed from the base of the PR and between a0d4658 and a9252bd.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • packages/backend/src/ee/accountPermissionSyncer.ts
  • packages/backend/src/ee/tokenRefresh.ts
  • packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql
  • packages/db/prisma/schema.prisma
  • packages/web/src/auth.ts
  • packages/web/src/ee/features/sso/actions.ts
  • packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
  • packages/web/src/lib/encryptedPrismaAdapter.ts

Comment threadpackages/backend/src/ee/tokenRefresh.ts
@brendan-kellam
brendan-kellam enabled auto-merge (squash) March 13, 2026 01:28
@brendan-kellam
brendan-kellam merged commit c0b39e6 into mainMar 13, 2026
10 checks passed
@brendan-kellam
brendan-kellam deleted the brendan/fix-token-refresh-in-permission-sync-SOU-664 branch March 13, 2026 01:28
@github-actionsgithub-actionsBot mentioned this pull request Mar 13, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(worker): refresh OAuth tokens in backend permission sync flow - #1000

Merged
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664
Mar 13, 2026
Merged

fix(worker): refresh OAuth tokens in backend permission sync flow#1000
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • OAuth token refresh was previously only triggered from the Next.js jwt callback, meaning tokens could expire between user visits and cause account-driven permission sync jobs to fail with auth errors.
  • Moves token refresh logic to packages/backend/src/ee/tokenRefresh.ts and calls ensureFreshAccountToken from accountPermissionSyncer before using an account's access token.
  • Adds tokenRefreshErrorMessage to the Account DB model — set on refresh failure, cleared on successful re-authentication — and surfaces it in the linked accounts UI so users know to re-authenticate.

Test plan

  • Verify that an expired OAuth token is automatically refreshed when a permission sync job runs
  • Verify that when refresh fails, tokenRefreshErrorMessage is set on the account and the "Token refresh failed — please reconnect" message appears in the linked accounts UI
  • Verify that successfully re-authenticating clears tokenRefreshErrorMessage and removes the error from the UI
  • Verify existing permission sync flows (GitHub, GitLab, Bitbucket Cloud, Bitbucket Server) still work correctly when tokens are valid

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Re-authentication prompts appear when OAuth token refresh fails to help restore account access.
    • UI refreshes automatically after successful permission refresh to show up-to-date linked account status.
  • Bug Fixes

    • Token refresh handling consolidated to backend for more reliable expiration handling and clearer per-account error reporting.
    • Backend now records token refresh error messages on accounts to aid recovery.

Token refresh was previously only triggered from the Next.js jwt callback,
meaning tokens could expire between user visits and cause account-driven
permission sync jobs to fail silently.
Move refresh logic to packages/backend/src/ee/tokenRefresh.ts and call it
from accountPermissionSyncer before using an account's access token. On
refresh failure, tokenRefreshErrorMessage is set on the Account record and
surfaced in the linked accounts UI so users know to re-authenticate.
Also adds a DB migration for the tokenRefreshErrorMessage field and wires
the signIn event to clear it on successful re-authentication.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Centralizes OAuth token refresh into a backend function (ensureFreshAccountToken(account, db)), persists token refresh errors on the Account record, and updates frontend flows to read/clear per-account token refresh errors instead of session-based provider errors.

Changes

Cohort / File(s)Summary
Database Schema
packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql, packages/db/prisma/schema.prisma
Adds optional tokenRefreshErrorMessage TEXT column/field to Account to persist OAuth refresh errors.
Backend Token Refresh & Syncer
packages/backend/src/ee/tokenRefresh.ts, packages/backend/src/ee/accountPermissionSyncer.ts
Introduces ensureFreshAccountToken(account, db) that validates, decrypts, refreshes tokens, updates DB, and records errors. accountPermissionSyncer now delegates token freshness to this function and removes local decrypt/validation logic.
Frontend Auth Changes
packages/web/src/auth.ts
Removes linkedAccountErrors session/JWT typings and runtime token-refresh logic; clears tokenRefreshErrorMessage on successful sign-in.
SSO UI / Actions
packages/web/src/ee/features/sso/actions.ts, packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
Switches linked-account error source to account.tokenRefreshErrorMessage; triggers router.refresh() after permission refresh to re-fetch UI data.
Adapter / Types
packages/web/src/lib/encryptedPrismaAdapter.ts
Extends encryptAccountData() input shape to accept optional tokenRefreshErrorMessage.
Changelog
CHANGELOG.md
Adds entry describing backend token refresh relocation and error surfacing changes.

Sequence Diagram

sequenceDiagram
participant Syncer as AccountPermissionSyncer
participant Refresh as ensureFreshAccountToken
participant Provider as OAuth Provider
participant DB as Database
participant UI as Frontend UI
Syncer->>Refresh: ensureFreshAccountToken(account, db)
Refresh->>Refresh: Validate access_token presence
Refresh->>Refresh: Check expiry (with buffer)
alt Token valid
Refresh->>Refresh: Decrypt & return access_token
Refresh-->>Syncer: access_token
else Token expired/near expiry
Refresh->>Refresh: Ensure refresh_token exists
Refresh->>Provider: refreshOAuthToken(request with provider creds)
alt Refresh successful
Provider-->>Refresh: token response
Refresh->>DB: Update Account (access_token, refresh_token?, expires_at), clear tokenRefreshErrorMessage
DB-->>Refresh: OK
Refresh-->>Syncer: new access_token
else Refresh failed
Provider-->>Refresh: Error
Refresh->>DB: set tokenRefreshErrorMessage on Account
DB-->>Refresh: OK
Refresh-->>Syncer: throw/error
end
end
Syncer->>Syncer: Continue permission sync using token
UI->>DB: Fetch Account (includes tokenRefreshErrorMessage)
DB-->>UI: Account with tokenRefreshErrorMessage
UI->>UI: Surface message / prompt re-authentication
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • msukkari
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe PR title clearly and accurately describes the main change: moving OAuth token refresh logic into the backend permission sync flow, which is the core objective of the changeset.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch brendan/fix-token-refresh-in-permission-sync-SOU-664
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/web/src/ee/features/sso/actions.ts (1)

54-60: ⚠️ Potential issue | 🟡 Minor

Don't send the raw refresh failure text to the client.

linkedAccountProviderCard.tsx only checks linkedAccount.error for truthiness, so returning account.tokenRefreshErrorMessage here just leaks backend diagnostics to the browser. Collapse it to a boolean or sentinel string instead.

Possible minimal change
- error: account.tokenRefreshErrorMessage ?? undefined,+ error: account.tokenRefreshErrorMessage ? "token_refresh_failed" : undefined,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/ee/features/sso/actions.ts` around lines 54 - 60, The code
currently exposes backend refresh failure text by assigning
account.tokenRefreshErrorMessage to the result.error field in the object pushed
by the result.push in actions.ts; change this to emit a non-sensitive sentinel
or boolean (e.g. error: !!account.tokenRefreshErrorMessage or error:
"REFRESH_FAILED") instead of the raw string so linkedAccountProviderCard.tsx can
still check truthiness without leaking diagnostics; update the object property
where provider/isLinked/accountId/providerAccountId/isAccountLinking are set to
use the boolean/sentinel and ensure undefined is used when there was no error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/ee/tokenRefresh.ts`:
- Around line 123-136: The account writes in db.account.update (used when
writing encryptOAuthToken(refreshResponse.access_token) / refresh_token /
expires_at) and in setTokenRefreshError are vulnerable to races because they
unconditionally overwrite from the job-start snapshot; to fix, serialize or use
compare-and-set: either acquire a per-account mutex (e.g., Redis lock keyed by
account.id) around the token refresh flow to ensure only one refresh runs for an
account, or change the updates to conditional updates that include the snapshot
you read (e.g., use updateMany/update with a where that includes the account.id
plus a snapshot column like account.updatedAt or the previous
access_token/refresh_token value read earlier) so the write only applies if the
stored values match the snapshot; apply the same pattern to the
setTokenRefreshError path.
---
Outside diff comments:
In `@packages/web/src/ee/features/sso/actions.ts`:
- Around line 54-60: The code currently exposes backend refresh failure text by
assigning account.tokenRefreshErrorMessage to the result.error field in the
object pushed by the result.push in actions.ts; change this to emit a
non-sensitive sentinel or boolean (e.g. error:
!!account.tokenRefreshErrorMessage or error: "REFRESH_FAILED") instead of the
raw string so linkedAccountProviderCard.tsx can still check truthiness without
leaking diagnostics; update the object property where
provider/isLinked/accountId/providerAccountId/isAccountLinking are set to use
the boolean/sentinel and ensure undefined is used when there was no error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 29444165-81cc-4596-86a9-2b25d4674472

📥 Commits

Reviewing files that changed from the base of the PR and between a0d4658 and a9252bd.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • packages/backend/src/ee/accountPermissionSyncer.ts
  • packages/backend/src/ee/tokenRefresh.ts
  • packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql
  • packages/db/prisma/schema.prisma
  • packages/web/src/auth.ts
  • packages/web/src/ee/features/sso/actions.ts
  • packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
  • packages/web/src/lib/encryptedPrismaAdapter.ts

Comment threadpackages/backend/src/ee/tokenRefresh.ts
@brendan-kellam
brendan-kellam enabled auto-merge (squash) March 13, 2026 01:28
@brendan-kellam
brendan-kellam merged commit c0b39e6 into mainMar 13, 2026
10 checks passed
@brendan-kellam
brendan-kellam deleted the brendan/fix-token-refresh-in-permission-sync-SOU-664 branch March 13, 2026 01:28
@github-actionsgithub-actionsBot mentioned this pull request Mar 13, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(worker): refresh OAuth tokens in backend permission sync flow - #1000

Merged
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664
Mar 13, 2026
Merged

fix(worker): refresh OAuth tokens in backend permission sync flow#1000
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • OAuth token refresh was previously only triggered from the Next.js jwt callback, meaning tokens could expire between user visits and cause account-driven permission sync jobs to fail with auth errors.
  • Moves token refresh logic to packages/backend/src/ee/tokenRefresh.ts and calls ensureFreshAccountToken from accountPermissionSyncer before using an account's access token.
  • Adds tokenRefreshErrorMessage to the Account DB model — set on refresh failure, cleared on successful re-authentication — and surfaces it in the linked accounts UI so users know to re-authenticate.

Test plan

  • Verify that an expired OAuth token is automatically refreshed when a permission sync job runs
  • Verify that when refresh fails, tokenRefreshErrorMessage is set on the account and the "Token refresh failed — please reconnect" message appears in the linked accounts UI
  • Verify that successfully re-authenticating clears tokenRefreshErrorMessage and removes the error from the UI
  • Verify existing permission sync flows (GitHub, GitLab, Bitbucket Cloud, Bitbucket Server) still work correctly when tokens are valid

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Re-authentication prompts appear when OAuth token refresh fails to help restore account access.
    • UI refreshes automatically after successful permission refresh to show up-to-date linked account status.
  • Bug Fixes

    • Token refresh handling consolidated to backend for more reliable expiration handling and clearer per-account error reporting.
    • Backend now records token refresh error messages on accounts to aid recovery.

Token refresh was previously only triggered from the Next.js jwt callback,
meaning tokens could expire between user visits and cause account-driven
permission sync jobs to fail silently.
Move refresh logic to packages/backend/src/ee/tokenRefresh.ts and call it
from accountPermissionSyncer before using an account's access token. On
refresh failure, tokenRefreshErrorMessage is set on the Account record and
surfaced in the linked accounts UI so users know to re-authenticate.
Also adds a DB migration for the tokenRefreshErrorMessage field and wires
the signIn event to clear it on successful re-authentication.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Centralizes OAuth token refresh into a backend function (ensureFreshAccountToken(account, db)), persists token refresh errors on the Account record, and updates frontend flows to read/clear per-account token refresh errors instead of session-based provider errors.

Changes

Cohort / File(s)Summary
Database Schema
packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql, packages/db/prisma/schema.prisma
Adds optional tokenRefreshErrorMessage TEXT column/field to Account to persist OAuth refresh errors.
Backend Token Refresh & Syncer
packages/backend/src/ee/tokenRefresh.ts, packages/backend/src/ee/accountPermissionSyncer.ts
Introduces ensureFreshAccountToken(account, db) that validates, decrypts, refreshes tokens, updates DB, and records errors. accountPermissionSyncer now delegates token freshness to this function and removes local decrypt/validation logic.
Frontend Auth Changes
packages/web/src/auth.ts
Removes linkedAccountErrors session/JWT typings and runtime token-refresh logic; clears tokenRefreshErrorMessage on successful sign-in.
SSO UI / Actions
packages/web/src/ee/features/sso/actions.ts, packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
Switches linked-account error source to account.tokenRefreshErrorMessage; triggers router.refresh() after permission refresh to re-fetch UI data.
Adapter / Types
packages/web/src/lib/encryptedPrismaAdapter.ts
Extends encryptAccountData() input shape to accept optional tokenRefreshErrorMessage.
Changelog
CHANGELOG.md
Adds entry describing backend token refresh relocation and error surfacing changes.

Sequence Diagram

sequenceDiagram
participant Syncer as AccountPermissionSyncer
participant Refresh as ensureFreshAccountToken
participant Provider as OAuth Provider
participant DB as Database
participant UI as Frontend UI
Syncer->>Refresh: ensureFreshAccountToken(account, db)
Refresh->>Refresh: Validate access_token presence
Refresh->>Refresh: Check expiry (with buffer)
alt Token valid
Refresh->>Refresh: Decrypt & return access_token
Refresh-->>Syncer: access_token
else Token expired/near expiry
Refresh->>Refresh: Ensure refresh_token exists
Refresh->>Provider: refreshOAuthToken(request with provider creds)
alt Refresh successful
Provider-->>Refresh: token response
Refresh->>DB: Update Account (access_token, refresh_token?, expires_at), clear tokenRefreshErrorMessage
DB-->>Refresh: OK
Refresh-->>Syncer: new access_token
else Refresh failed
Provider-->>Refresh: Error
Refresh->>DB: set tokenRefreshErrorMessage on Account
DB-->>Refresh: OK
Refresh-->>Syncer: throw/error
end
end
Syncer->>Syncer: Continue permission sync using token
UI->>DB: Fetch Account (includes tokenRefreshErrorMessage)
DB-->>UI: Account with tokenRefreshErrorMessage
UI->>UI: Surface message / prompt re-authentication
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • msukkari
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe PR title clearly and accurately describes the main change: moving OAuth token refresh logic into the backend permission sync flow, which is the core objective of the changeset.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch brendan/fix-token-refresh-in-permission-sync-SOU-664
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/web/src/ee/features/sso/actions.ts (1)

54-60: ⚠️ Potential issue | 🟡 Minor

Don't send the raw refresh failure text to the client.

linkedAccountProviderCard.tsx only checks linkedAccount.error for truthiness, so returning account.tokenRefreshErrorMessage here just leaks backend diagnostics to the browser. Collapse it to a boolean or sentinel string instead.

Possible minimal change
- error: account.tokenRefreshErrorMessage ?? undefined,+ error: account.tokenRefreshErrorMessage ? "token_refresh_failed" : undefined,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/ee/features/sso/actions.ts` around lines 54 - 60, The code
currently exposes backend refresh failure text by assigning
account.tokenRefreshErrorMessage to the result.error field in the object pushed
by the result.push in actions.ts; change this to emit a non-sensitive sentinel
or boolean (e.g. error: !!account.tokenRefreshErrorMessage or error:
"REFRESH_FAILED") instead of the raw string so linkedAccountProviderCard.tsx can
still check truthiness without leaking diagnostics; update the object property
where provider/isLinked/accountId/providerAccountId/isAccountLinking are set to
use the boolean/sentinel and ensure undefined is used when there was no error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/ee/tokenRefresh.ts`:
- Around line 123-136: The account writes in db.account.update (used when
writing encryptOAuthToken(refreshResponse.access_token) / refresh_token /
expires_at) and in setTokenRefreshError are vulnerable to races because they
unconditionally overwrite from the job-start snapshot; to fix, serialize or use
compare-and-set: either acquire a per-account mutex (e.g., Redis lock keyed by
account.id) around the token refresh flow to ensure only one refresh runs for an
account, or change the updates to conditional updates that include the snapshot
you read (e.g., use updateMany/update with a where that includes the account.id
plus a snapshot column like account.updatedAt or the previous
access_token/refresh_token value read earlier) so the write only applies if the
stored values match the snapshot; apply the same pattern to the
setTokenRefreshError path.
---
Outside diff comments:
In `@packages/web/src/ee/features/sso/actions.ts`:
- Around line 54-60: The code currently exposes backend refresh failure text by
assigning account.tokenRefreshErrorMessage to the result.error field in the
object pushed by the result.push in actions.ts; change this to emit a
non-sensitive sentinel or boolean (e.g. error:
!!account.tokenRefreshErrorMessage or error: "REFRESH_FAILED") instead of the
raw string so linkedAccountProviderCard.tsx can still check truthiness without
leaking diagnostics; update the object property where
provider/isLinked/accountId/providerAccountId/isAccountLinking are set to use
the boolean/sentinel and ensure undefined is used when there was no error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 29444165-81cc-4596-86a9-2b25d4674472

📥 Commits

Reviewing files that changed from the base of the PR and between a0d4658 and a9252bd.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • packages/backend/src/ee/accountPermissionSyncer.ts
  • packages/backend/src/ee/tokenRefresh.ts
  • packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql
  • packages/db/prisma/schema.prisma
  • packages/web/src/auth.ts
  • packages/web/src/ee/features/sso/actions.ts
  • packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
  • packages/web/src/lib/encryptedPrismaAdapter.ts

Comment threadpackages/backend/src/ee/tokenRefresh.ts
@brendan-kellam
brendan-kellam enabled auto-merge (squash) March 13, 2026 01:28
@brendan-kellam
brendan-kellam merged commit c0b39e6 into mainMar 13, 2026
10 checks passed
@brendan-kellam
brendan-kellam deleted the brendan/fix-token-refresh-in-permission-sync-SOU-664 branch March 13, 2026 01:28
@github-actionsgithub-actionsBot mentioned this pull request Mar 13, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(worker): refresh OAuth tokens in backend permission sync flow - #1000

Merged
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664
Mar 13, 2026
Merged

fix(worker): refresh OAuth tokens in backend permission sync flow#1000
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • OAuth token refresh was previously only triggered from the Next.js jwt callback, meaning tokens could expire between user visits and cause account-driven permission sync jobs to fail with auth errors.
  • Moves token refresh logic to packages/backend/src/ee/tokenRefresh.ts and calls ensureFreshAccountToken from accountPermissionSyncer before using an account's access token.
  • Adds tokenRefreshErrorMessage to the Account DB model — set on refresh failure, cleared on successful re-authentication — and surfaces it in the linked accounts UI so users know to re-authenticate.

Test plan

  • Verify that an expired OAuth token is automatically refreshed when a permission sync job runs
  • Verify that when refresh fails, tokenRefreshErrorMessage is set on the account and the "Token refresh failed — please reconnect" message appears in the linked accounts UI
  • Verify that successfully re-authenticating clears tokenRefreshErrorMessage and removes the error from the UI
  • Verify existing permission sync flows (GitHub, GitLab, Bitbucket Cloud, Bitbucket Server) still work correctly when tokens are valid

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Re-authentication prompts appear when OAuth token refresh fails to help restore account access.
    • UI refreshes automatically after successful permission refresh to show up-to-date linked account status.
  • Bug Fixes

    • Token refresh handling consolidated to backend for more reliable expiration handling and clearer per-account error reporting.
    • Backend now records token refresh error messages on accounts to aid recovery.

Token refresh was previously only triggered from the Next.js jwt callback,
meaning tokens could expire between user visits and cause account-driven
permission sync jobs to fail silently.
Move refresh logic to packages/backend/src/ee/tokenRefresh.ts and call it
from accountPermissionSyncer before using an account's access token. On
refresh failure, tokenRefreshErrorMessage is set on the Account record and
surfaced in the linked accounts UI so users know to re-authenticate.
Also adds a DB migration for the tokenRefreshErrorMessage field and wires
the signIn event to clear it on successful re-authentication.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Centralizes OAuth token refresh into a backend function (ensureFreshAccountToken(account, db)), persists token refresh errors on the Account record, and updates frontend flows to read/clear per-account token refresh errors instead of session-based provider errors.

Changes

Cohort / File(s)Summary
Database Schema
packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql, packages/db/prisma/schema.prisma
Adds optional tokenRefreshErrorMessage TEXT column/field to Account to persist OAuth refresh errors.
Backend Token Refresh & Syncer
packages/backend/src/ee/tokenRefresh.ts, packages/backend/src/ee/accountPermissionSyncer.ts
Introduces ensureFreshAccountToken(account, db) that validates, decrypts, refreshes tokens, updates DB, and records errors. accountPermissionSyncer now delegates token freshness to this function and removes local decrypt/validation logic.
Frontend Auth Changes
packages/web/src/auth.ts
Removes linkedAccountErrors session/JWT typings and runtime token-refresh logic; clears tokenRefreshErrorMessage on successful sign-in.
SSO UI / Actions
packages/web/src/ee/features/sso/actions.ts, packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
Switches linked-account error source to account.tokenRefreshErrorMessage; triggers router.refresh() after permission refresh to re-fetch UI data.
Adapter / Types
packages/web/src/lib/encryptedPrismaAdapter.ts
Extends encryptAccountData() input shape to accept optional tokenRefreshErrorMessage.
Changelog
CHANGELOG.md
Adds entry describing backend token refresh relocation and error surfacing changes.

Sequence Diagram

sequenceDiagram
participant Syncer as AccountPermissionSyncer
participant Refresh as ensureFreshAccountToken
participant Provider as OAuth Provider
participant DB as Database
participant UI as Frontend UI
Syncer->>Refresh: ensureFreshAccountToken(account, db)
Refresh->>Refresh: Validate access_token presence
Refresh->>Refresh: Check expiry (with buffer)
alt Token valid
Refresh->>Refresh: Decrypt & return access_token
Refresh-->>Syncer: access_token
else Token expired/near expiry
Refresh->>Refresh: Ensure refresh_token exists
Refresh->>Provider: refreshOAuthToken(request with provider creds)
alt Refresh successful
Provider-->>Refresh: token response
Refresh->>DB: Update Account (access_token, refresh_token?, expires_at), clear tokenRefreshErrorMessage
DB-->>Refresh: OK
Refresh-->>Syncer: new access_token
else Refresh failed
Provider-->>Refresh: Error
Refresh->>DB: set tokenRefreshErrorMessage on Account
DB-->>Refresh: OK
Refresh-->>Syncer: throw/error
end
end
Syncer->>Syncer: Continue permission sync using token
UI->>DB: Fetch Account (includes tokenRefreshErrorMessage)
DB-->>UI: Account with tokenRefreshErrorMessage
UI->>UI: Surface message / prompt re-authentication
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • msukkari
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe PR title clearly and accurately describes the main change: moving OAuth token refresh logic into the backend permission sync flow, which is the core objective of the changeset.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch brendan/fix-token-refresh-in-permission-sync-SOU-664
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/web/src/ee/features/sso/actions.ts (1)

54-60: ⚠️ Potential issue | 🟡 Minor

Don't send the raw refresh failure text to the client.

linkedAccountProviderCard.tsx only checks linkedAccount.error for truthiness, so returning account.tokenRefreshErrorMessage here just leaks backend diagnostics to the browser. Collapse it to a boolean or sentinel string instead.

Possible minimal change
- error: account.tokenRefreshErrorMessage ?? undefined,+ error: account.tokenRefreshErrorMessage ? "token_refresh_failed" : undefined,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/ee/features/sso/actions.ts` around lines 54 - 60, The code
currently exposes backend refresh failure text by assigning
account.tokenRefreshErrorMessage to the result.error field in the object pushed
by the result.push in actions.ts; change this to emit a non-sensitive sentinel
or boolean (e.g. error: !!account.tokenRefreshErrorMessage or error:
"REFRESH_FAILED") instead of the raw string so linkedAccountProviderCard.tsx can
still check truthiness without leaking diagnostics; update the object property
where provider/isLinked/accountId/providerAccountId/isAccountLinking are set to
use the boolean/sentinel and ensure undefined is used when there was no error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/ee/tokenRefresh.ts`:
- Around line 123-136: The account writes in db.account.update (used when
writing encryptOAuthToken(refreshResponse.access_token) / refresh_token /
expires_at) and in setTokenRefreshError are vulnerable to races because they
unconditionally overwrite from the job-start snapshot; to fix, serialize or use
compare-and-set: either acquire a per-account mutex (e.g., Redis lock keyed by
account.id) around the token refresh flow to ensure only one refresh runs for an
account, or change the updates to conditional updates that include the snapshot
you read (e.g., use updateMany/update with a where that includes the account.id
plus a snapshot column like account.updatedAt or the previous
access_token/refresh_token value read earlier) so the write only applies if the
stored values match the snapshot; apply the same pattern to the
setTokenRefreshError path.
---
Outside diff comments:
In `@packages/web/src/ee/features/sso/actions.ts`:
- Around line 54-60: The code currently exposes backend refresh failure text by
assigning account.tokenRefreshErrorMessage to the result.error field in the
object pushed by the result.push in actions.ts; change this to emit a
non-sensitive sentinel or boolean (e.g. error:
!!account.tokenRefreshErrorMessage or error: "REFRESH_FAILED") instead of the
raw string so linkedAccountProviderCard.tsx can still check truthiness without
leaking diagnostics; update the object property where
provider/isLinked/accountId/providerAccountId/isAccountLinking are set to use
the boolean/sentinel and ensure undefined is used when there was no error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 29444165-81cc-4596-86a9-2b25d4674472

📥 Commits

Reviewing files that changed from the base of the PR and between a0d4658 and a9252bd.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • packages/backend/src/ee/accountPermissionSyncer.ts
  • packages/backend/src/ee/tokenRefresh.ts
  • packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql
  • packages/db/prisma/schema.prisma
  • packages/web/src/auth.ts
  • packages/web/src/ee/features/sso/actions.ts
  • packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
  • packages/web/src/lib/encryptedPrismaAdapter.ts

Comment threadpackages/backend/src/ee/tokenRefresh.ts
@brendan-kellam
brendan-kellam enabled auto-merge (squash) March 13, 2026 01:28
@brendan-kellam
brendan-kellam merged commit c0b39e6 into mainMar 13, 2026
10 checks passed
@brendan-kellam
brendan-kellam deleted the brendan/fix-token-refresh-in-permission-sync-SOU-664 branch March 13, 2026 01:28
@github-actionsgithub-actionsBot mentioned this pull request Mar 13, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(worker): refresh OAuth tokens in backend permission sync flow - #1000

Merged
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664
Mar 13, 2026
Merged

fix(worker): refresh OAuth tokens in backend permission sync flow#1000
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • OAuth token refresh was previously only triggered from the Next.js jwt callback, meaning tokens could expire between user visits and cause account-driven permission sync jobs to fail with auth errors.
  • Moves token refresh logic to packages/backend/src/ee/tokenRefresh.ts and calls ensureFreshAccountToken from accountPermissionSyncer before using an account's access token.
  • Adds tokenRefreshErrorMessage to the Account DB model — set on refresh failure, cleared on successful re-authentication — and surfaces it in the linked accounts UI so users know to re-authenticate.

Test plan

  • Verify that an expired OAuth token is automatically refreshed when a permission sync job runs
  • Verify that when refresh fails, tokenRefreshErrorMessage is set on the account and the "Token refresh failed — please reconnect" message appears in the linked accounts UI
  • Verify that successfully re-authenticating clears tokenRefreshErrorMessage and removes the error from the UI
  • Verify existing permission sync flows (GitHub, GitLab, Bitbucket Cloud, Bitbucket Server) still work correctly when tokens are valid

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Re-authentication prompts appear when OAuth token refresh fails to help restore account access.
    • UI refreshes automatically after successful permission refresh to show up-to-date linked account status.
  • Bug Fixes

    • Token refresh handling consolidated to backend for more reliable expiration handling and clearer per-account error reporting.
    • Backend now records token refresh error messages on accounts to aid recovery.

Token refresh was previously only triggered from the Next.js jwt callback,
meaning tokens could expire between user visits and cause account-driven
permission sync jobs to fail silently.
Move refresh logic to packages/backend/src/ee/tokenRefresh.ts and call it
from accountPermissionSyncer before using an account's access token. On
refresh failure, tokenRefreshErrorMessage is set on the Account record and
surfaced in the linked accounts UI so users know to re-authenticate.
Also adds a DB migration for the tokenRefreshErrorMessage field and wires
the signIn event to clear it on successful re-authentication.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Centralizes OAuth token refresh into a backend function (ensureFreshAccountToken(account, db)), persists token refresh errors on the Account record, and updates frontend flows to read/clear per-account token refresh errors instead of session-based provider errors.

Changes

Cohort / File(s)Summary
Database Schema
packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql, packages/db/prisma/schema.prisma
Adds optional tokenRefreshErrorMessage TEXT column/field to Account to persist OAuth refresh errors.
Backend Token Refresh & Syncer
packages/backend/src/ee/tokenRefresh.ts, packages/backend/src/ee/accountPermissionSyncer.ts
Introduces ensureFreshAccountToken(account, db) that validates, decrypts, refreshes tokens, updates DB, and records errors. accountPermissionSyncer now delegates token freshness to this function and removes local decrypt/validation logic.
Frontend Auth Changes
packages/web/src/auth.ts
Removes linkedAccountErrors session/JWT typings and runtime token-refresh logic; clears tokenRefreshErrorMessage on successful sign-in.
SSO UI / Actions
packages/web/src/ee/features/sso/actions.ts, packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
Switches linked-account error source to account.tokenRefreshErrorMessage; triggers router.refresh() after permission refresh to re-fetch UI data.
Adapter / Types
packages/web/src/lib/encryptedPrismaAdapter.ts
Extends encryptAccountData() input shape to accept optional tokenRefreshErrorMessage.
Changelog
CHANGELOG.md
Adds entry describing backend token refresh relocation and error surfacing changes.

Sequence Diagram

sequenceDiagram
participant Syncer as AccountPermissionSyncer
participant Refresh as ensureFreshAccountToken
participant Provider as OAuth Provider
participant DB as Database
participant UI as Frontend UI
Syncer->>Refresh: ensureFreshAccountToken(account, db)
Refresh->>Refresh: Validate access_token presence
Refresh->>Refresh: Check expiry (with buffer)
alt Token valid
Refresh->>Refresh: Decrypt & return access_token
Refresh-->>Syncer: access_token
else Token expired/near expiry
Refresh->>Refresh: Ensure refresh_token exists
Refresh->>Provider: refreshOAuthToken(request with provider creds)
alt Refresh successful
Provider-->>Refresh: token response
Refresh->>DB: Update Account (access_token, refresh_token?, expires_at), clear tokenRefreshErrorMessage
DB-->>Refresh: OK
Refresh-->>Syncer: new access_token
else Refresh failed
Provider-->>Refresh: Error
Refresh->>DB: set tokenRefreshErrorMessage on Account
DB-->>Refresh: OK
Refresh-->>Syncer: throw/error
end
end
Syncer->>Syncer: Continue permission sync using token
UI->>DB: Fetch Account (includes tokenRefreshErrorMessage)
DB-->>UI: Account with tokenRefreshErrorMessage
UI->>UI: Surface message / prompt re-authentication
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • msukkari
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe PR title clearly and accurately describes the main change: moving OAuth token refresh logic into the backend permission sync flow, which is the core objective of the changeset.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch brendan/fix-token-refresh-in-permission-sync-SOU-664
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/web/src/ee/features/sso/actions.ts (1)

54-60: ⚠️ Potential issue | 🟡 Minor

Don't send the raw refresh failure text to the client.

linkedAccountProviderCard.tsx only checks linkedAccount.error for truthiness, so returning account.tokenRefreshErrorMessage here just leaks backend diagnostics to the browser. Collapse it to a boolean or sentinel string instead.

Possible minimal change
- error: account.tokenRefreshErrorMessage ?? undefined,+ error: account.tokenRefreshErrorMessage ? "token_refresh_failed" : undefined,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/ee/features/sso/actions.ts` around lines 54 - 60, The code
currently exposes backend refresh failure text by assigning
account.tokenRefreshErrorMessage to the result.error field in the object pushed
by the result.push in actions.ts; change this to emit a non-sensitive sentinel
or boolean (e.g. error: !!account.tokenRefreshErrorMessage or error:
"REFRESH_FAILED") instead of the raw string so linkedAccountProviderCard.tsx can
still check truthiness without leaking diagnostics; update the object property
where provider/isLinked/accountId/providerAccountId/isAccountLinking are set to
use the boolean/sentinel and ensure undefined is used when there was no error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/ee/tokenRefresh.ts`:
- Around line 123-136: The account writes in db.account.update (used when
writing encryptOAuthToken(refreshResponse.access_token) / refresh_token /
expires_at) and in setTokenRefreshError are vulnerable to races because they
unconditionally overwrite from the job-start snapshot; to fix, serialize or use
compare-and-set: either acquire a per-account mutex (e.g., Redis lock keyed by
account.id) around the token refresh flow to ensure only one refresh runs for an
account, or change the updates to conditional updates that include the snapshot
you read (e.g., use updateMany/update with a where that includes the account.id
plus a snapshot column like account.updatedAt or the previous
access_token/refresh_token value read earlier) so the write only applies if the
stored values match the snapshot; apply the same pattern to the
setTokenRefreshError path.
---
Outside diff comments:
In `@packages/web/src/ee/features/sso/actions.ts`:
- Around line 54-60: The code currently exposes backend refresh failure text by
assigning account.tokenRefreshErrorMessage to the result.error field in the
object pushed by the result.push in actions.ts; change this to emit a
non-sensitive sentinel or boolean (e.g. error:
!!account.tokenRefreshErrorMessage or error: "REFRESH_FAILED") instead of the
raw string so linkedAccountProviderCard.tsx can still check truthiness without
leaking diagnostics; update the object property where
provider/isLinked/accountId/providerAccountId/isAccountLinking are set to use
the boolean/sentinel and ensure undefined is used when there was no error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 29444165-81cc-4596-86a9-2b25d4674472

📥 Commits

Reviewing files that changed from the base of the PR and between a0d4658 and a9252bd.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • packages/backend/src/ee/accountPermissionSyncer.ts
  • packages/backend/src/ee/tokenRefresh.ts
  • packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql
  • packages/db/prisma/schema.prisma
  • packages/web/src/auth.ts
  • packages/web/src/ee/features/sso/actions.ts
  • packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
  • packages/web/src/lib/encryptedPrismaAdapter.ts

Comment threadpackages/backend/src/ee/tokenRefresh.ts
@brendan-kellam
brendan-kellam enabled auto-merge (squash) March 13, 2026 01:28
@brendan-kellam
brendan-kellam merged commit c0b39e6 into mainMar 13, 2026
10 checks passed
@brendan-kellam
brendan-kellam deleted the brendan/fix-token-refresh-in-permission-sync-SOU-664 branch March 13, 2026 01:28
@github-actionsgithub-actionsBot mentioned this pull request Mar 13, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(worker): refresh OAuth tokens in backend permission sync flow - #1000

Merged
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664
Mar 13, 2026
Merged

fix(worker): refresh OAuth tokens in backend permission sync flow#1000
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • OAuth token refresh was previously only triggered from the Next.js jwt callback, meaning tokens could expire between user visits and cause account-driven permission sync jobs to fail with auth errors.
  • Moves token refresh logic to packages/backend/src/ee/tokenRefresh.ts and calls ensureFreshAccountToken from accountPermissionSyncer before using an account's access token.
  • Adds tokenRefreshErrorMessage to the Account DB model — set on refresh failure, cleared on successful re-authentication — and surfaces it in the linked accounts UI so users know to re-authenticate.

Test plan

  • Verify that an expired OAuth token is automatically refreshed when a permission sync job runs
  • Verify that when refresh fails, tokenRefreshErrorMessage is set on the account and the "Token refresh failed — please reconnect" message appears in the linked accounts UI
  • Verify that successfully re-authenticating clears tokenRefreshErrorMessage and removes the error from the UI
  • Verify existing permission sync flows (GitHub, GitLab, Bitbucket Cloud, Bitbucket Server) still work correctly when tokens are valid

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Re-authentication prompts appear when OAuth token refresh fails to help restore account access.
    • UI refreshes automatically after successful permission refresh to show up-to-date linked account status.
  • Bug Fixes

    • Token refresh handling consolidated to backend for more reliable expiration handling and clearer per-account error reporting.
    • Backend now records token refresh error messages on accounts to aid recovery.

Token refresh was previously only triggered from the Next.js jwt callback,
meaning tokens could expire between user visits and cause account-driven
permission sync jobs to fail silently.
Move refresh logic to packages/backend/src/ee/tokenRefresh.ts and call it
from accountPermissionSyncer before using an account's access token. On
refresh failure, tokenRefreshErrorMessage is set on the Account record and
surfaced in the linked accounts UI so users know to re-authenticate.
Also adds a DB migration for the tokenRefreshErrorMessage field and wires
the signIn event to clear it on successful re-authentication.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Centralizes OAuth token refresh into a backend function (ensureFreshAccountToken(account, db)), persists token refresh errors on the Account record, and updates frontend flows to read/clear per-account token refresh errors instead of session-based provider errors.

Changes

Cohort / File(s)Summary
Database Schema
packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql, packages/db/prisma/schema.prisma
Adds optional tokenRefreshErrorMessage TEXT column/field to Account to persist OAuth refresh errors.
Backend Token Refresh & Syncer
packages/backend/src/ee/tokenRefresh.ts, packages/backend/src/ee/accountPermissionSyncer.ts
Introduces ensureFreshAccountToken(account, db) that validates, decrypts, refreshes tokens, updates DB, and records errors. accountPermissionSyncer now delegates token freshness to this function and removes local decrypt/validation logic.
Frontend Auth Changes
packages/web/src/auth.ts
Removes linkedAccountErrors session/JWT typings and runtime token-refresh logic; clears tokenRefreshErrorMessage on successful sign-in.
SSO UI / Actions
packages/web/src/ee/features/sso/actions.ts, packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
Switches linked-account error source to account.tokenRefreshErrorMessage; triggers router.refresh() after permission refresh to re-fetch UI data.
Adapter / Types
packages/web/src/lib/encryptedPrismaAdapter.ts
Extends encryptAccountData() input shape to accept optional tokenRefreshErrorMessage.
Changelog
CHANGELOG.md
Adds entry describing backend token refresh relocation and error surfacing changes.

Sequence Diagram

sequenceDiagram
participant Syncer as AccountPermissionSyncer
participant Refresh as ensureFreshAccountToken
participant Provider as OAuth Provider
participant DB as Database
participant UI as Frontend UI
Syncer->>Refresh: ensureFreshAccountToken(account, db)
Refresh->>Refresh: Validate access_token presence
Refresh->>Refresh: Check expiry (with buffer)
alt Token valid
Refresh->>Refresh: Decrypt & return access_token
Refresh-->>Syncer: access_token
else Token expired/near expiry
Refresh->>Refresh: Ensure refresh_token exists
Refresh->>Provider: refreshOAuthToken(request with provider creds)
alt Refresh successful
Provider-->>Refresh: token response
Refresh->>DB: Update Account (access_token, refresh_token?, expires_at), clear tokenRefreshErrorMessage
DB-->>Refresh: OK
Refresh-->>Syncer: new access_token
else Refresh failed
Provider-->>Refresh: Error
Refresh->>DB: set tokenRefreshErrorMessage on Account
DB-->>Refresh: OK
Refresh-->>Syncer: throw/error
end
end
Syncer->>Syncer: Continue permission sync using token
UI->>DB: Fetch Account (includes tokenRefreshErrorMessage)
DB-->>UI: Account with tokenRefreshErrorMessage
UI->>UI: Surface message / prompt re-authentication
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • msukkari
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe PR title clearly and accurately describes the main change: moving OAuth token refresh logic into the backend permission sync flow, which is the core objective of the changeset.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch brendan/fix-token-refresh-in-permission-sync-SOU-664
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/web/src/ee/features/sso/actions.ts (1)

54-60: ⚠️ Potential issue | 🟡 Minor

Don't send the raw refresh failure text to the client.

linkedAccountProviderCard.tsx only checks linkedAccount.error for truthiness, so returning account.tokenRefreshErrorMessage here just leaks backend diagnostics to the browser. Collapse it to a boolean or sentinel string instead.

Possible minimal change
- error: account.tokenRefreshErrorMessage ?? undefined,+ error: account.tokenRefreshErrorMessage ? "token_refresh_failed" : undefined,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/ee/features/sso/actions.ts` around lines 54 - 60, The code
currently exposes backend refresh failure text by assigning
account.tokenRefreshErrorMessage to the result.error field in the object pushed
by the result.push in actions.ts; change this to emit a non-sensitive sentinel
or boolean (e.g. error: !!account.tokenRefreshErrorMessage or error:
"REFRESH_FAILED") instead of the raw string so linkedAccountProviderCard.tsx can
still check truthiness without leaking diagnostics; update the object property
where provider/isLinked/accountId/providerAccountId/isAccountLinking are set to
use the boolean/sentinel and ensure undefined is used when there was no error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/ee/tokenRefresh.ts`:
- Around line 123-136: The account writes in db.account.update (used when
writing encryptOAuthToken(refreshResponse.access_token) / refresh_token /
expires_at) and in setTokenRefreshError are vulnerable to races because they
unconditionally overwrite from the job-start snapshot; to fix, serialize or use
compare-and-set: either acquire a per-account mutex (e.g., Redis lock keyed by
account.id) around the token refresh flow to ensure only one refresh runs for an
account, or change the updates to conditional updates that include the snapshot
you read (e.g., use updateMany/update with a where that includes the account.id
plus a snapshot column like account.updatedAt or the previous
access_token/refresh_token value read earlier) so the write only applies if the
stored values match the snapshot; apply the same pattern to the
setTokenRefreshError path.
---
Outside diff comments:
In `@packages/web/src/ee/features/sso/actions.ts`:
- Around line 54-60: The code currently exposes backend refresh failure text by
assigning account.tokenRefreshErrorMessage to the result.error field in the
object pushed by the result.push in actions.ts; change this to emit a
non-sensitive sentinel or boolean (e.g. error:
!!account.tokenRefreshErrorMessage or error: "REFRESH_FAILED") instead of the
raw string so linkedAccountProviderCard.tsx can still check truthiness without
leaking diagnostics; update the object property where
provider/isLinked/accountId/providerAccountId/isAccountLinking are set to use
the boolean/sentinel and ensure undefined is used when there was no error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 29444165-81cc-4596-86a9-2b25d4674472

📥 Commits

Reviewing files that changed from the base of the PR and between a0d4658 and a9252bd.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • packages/backend/src/ee/accountPermissionSyncer.ts
  • packages/backend/src/ee/tokenRefresh.ts
  • packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql
  • packages/db/prisma/schema.prisma
  • packages/web/src/auth.ts
  • packages/web/src/ee/features/sso/actions.ts
  • packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
  • packages/web/src/lib/encryptedPrismaAdapter.ts

Comment threadpackages/backend/src/ee/tokenRefresh.ts
@brendan-kellam
brendan-kellam enabled auto-merge (squash) March 13, 2026 01:28
@brendan-kellam
brendan-kellam merged commit c0b39e6 into mainMar 13, 2026
10 checks passed
@brendan-kellam
brendan-kellam deleted the brendan/fix-token-refresh-in-permission-sync-SOU-664 branch March 13, 2026 01:28
@github-actionsgithub-actionsBot mentioned this pull request Mar 13, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(worker): refresh OAuth tokens in backend permission sync flow - #1000

Merged
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664
Mar 13, 2026
Merged

fix(worker): refresh OAuth tokens in backend permission sync flow#1000
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • OAuth token refresh was previously only triggered from the Next.js jwt callback, meaning tokens could expire between user visits and cause account-driven permission sync jobs to fail with auth errors.
  • Moves token refresh logic to packages/backend/src/ee/tokenRefresh.ts and calls ensureFreshAccountToken from accountPermissionSyncer before using an account's access token.
  • Adds tokenRefreshErrorMessage to the Account DB model — set on refresh failure, cleared on successful re-authentication — and surfaces it in the linked accounts UI so users know to re-authenticate.

Test plan

  • Verify that an expired OAuth token is automatically refreshed when a permission sync job runs
  • Verify that when refresh fails, tokenRefreshErrorMessage is set on the account and the "Token refresh failed — please reconnect" message appears in the linked accounts UI
  • Verify that successfully re-authenticating clears tokenRefreshErrorMessage and removes the error from the UI
  • Verify existing permission sync flows (GitHub, GitLab, Bitbucket Cloud, Bitbucket Server) still work correctly when tokens are valid

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Re-authentication prompts appear when OAuth token refresh fails to help restore account access.
    • UI refreshes automatically after successful permission refresh to show up-to-date linked account status.
  • Bug Fixes

    • Token refresh handling consolidated to backend for more reliable expiration handling and clearer per-account error reporting.
    • Backend now records token refresh error messages on accounts to aid recovery.

Token refresh was previously only triggered from the Next.js jwt callback,
meaning tokens could expire between user visits and cause account-driven
permission sync jobs to fail silently.
Move refresh logic to packages/backend/src/ee/tokenRefresh.ts and call it
from accountPermissionSyncer before using an account's access token. On
refresh failure, tokenRefreshErrorMessage is set on the Account record and
surfaced in the linked accounts UI so users know to re-authenticate.
Also adds a DB migration for the tokenRefreshErrorMessage field and wires
the signIn event to clear it on successful re-authentication.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Centralizes OAuth token refresh into a backend function (ensureFreshAccountToken(account, db)), persists token refresh errors on the Account record, and updates frontend flows to read/clear per-account token refresh errors instead of session-based provider errors.

Changes

Cohort / File(s)Summary
Database Schema
packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql, packages/db/prisma/schema.prisma
Adds optional tokenRefreshErrorMessage TEXT column/field to Account to persist OAuth refresh errors.
Backend Token Refresh & Syncer
packages/backend/src/ee/tokenRefresh.ts, packages/backend/src/ee/accountPermissionSyncer.ts
Introduces ensureFreshAccountToken(account, db) that validates, decrypts, refreshes tokens, updates DB, and records errors. accountPermissionSyncer now delegates token freshness to this function and removes local decrypt/validation logic.
Frontend Auth Changes
packages/web/src/auth.ts
Removes linkedAccountErrors session/JWT typings and runtime token-refresh logic; clears tokenRefreshErrorMessage on successful sign-in.
SSO UI / Actions
packages/web/src/ee/features/sso/actions.ts, packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
Switches linked-account error source to account.tokenRefreshErrorMessage; triggers router.refresh() after permission refresh to re-fetch UI data.
Adapter / Types
packages/web/src/lib/encryptedPrismaAdapter.ts
Extends encryptAccountData() input shape to accept optional tokenRefreshErrorMessage.
Changelog
CHANGELOG.md
Adds entry describing backend token refresh relocation and error surfacing changes.

Sequence Diagram

sequenceDiagram
participant Syncer as AccountPermissionSyncer
participant Refresh as ensureFreshAccountToken
participant Provider as OAuth Provider
participant DB as Database
participant UI as Frontend UI
Syncer->>Refresh: ensureFreshAccountToken(account, db)
Refresh->>Refresh: Validate access_token presence
Refresh->>Refresh: Check expiry (with buffer)
alt Token valid
Refresh->>Refresh: Decrypt & return access_token
Refresh-->>Syncer: access_token
else Token expired/near expiry
Refresh->>Refresh: Ensure refresh_token exists
Refresh->>Provider: refreshOAuthToken(request with provider creds)
alt Refresh successful
Provider-->>Refresh: token response
Refresh->>DB: Update Account (access_token, refresh_token?, expires_at), clear tokenRefreshErrorMessage
DB-->>Refresh: OK
Refresh-->>Syncer: new access_token
else Refresh failed
Provider-->>Refresh: Error
Refresh->>DB: set tokenRefreshErrorMessage on Account
DB-->>Refresh: OK
Refresh-->>Syncer: throw/error
end
end
Syncer->>Syncer: Continue permission sync using token
UI->>DB: Fetch Account (includes tokenRefreshErrorMessage)
DB-->>UI: Account with tokenRefreshErrorMessage
UI->>UI: Surface message / prompt re-authentication
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • msukkari
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe PR title clearly and accurately describes the main change: moving OAuth token refresh logic into the backend permission sync flow, which is the core objective of the changeset.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch brendan/fix-token-refresh-in-permission-sync-SOU-664
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/web/src/ee/features/sso/actions.ts (1)

54-60: ⚠️ Potential issue | 🟡 Minor

Don't send the raw refresh failure text to the client.

linkedAccountProviderCard.tsx only checks linkedAccount.error for truthiness, so returning account.tokenRefreshErrorMessage here just leaks backend diagnostics to the browser. Collapse it to a boolean or sentinel string instead.

Possible minimal change
- error: account.tokenRefreshErrorMessage ?? undefined,+ error: account.tokenRefreshErrorMessage ? "token_refresh_failed" : undefined,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/ee/features/sso/actions.ts` around lines 54 - 60, The code
currently exposes backend refresh failure text by assigning
account.tokenRefreshErrorMessage to the result.error field in the object pushed
by the result.push in actions.ts; change this to emit a non-sensitive sentinel
or boolean (e.g. error: !!account.tokenRefreshErrorMessage or error:
"REFRESH_FAILED") instead of the raw string so linkedAccountProviderCard.tsx can
still check truthiness without leaking diagnostics; update the object property
where provider/isLinked/accountId/providerAccountId/isAccountLinking are set to
use the boolean/sentinel and ensure undefined is used when there was no error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/ee/tokenRefresh.ts`:
- Around line 123-136: The account writes in db.account.update (used when
writing encryptOAuthToken(refreshResponse.access_token) / refresh_token /
expires_at) and in setTokenRefreshError are vulnerable to races because they
unconditionally overwrite from the job-start snapshot; to fix, serialize or use
compare-and-set: either acquire a per-account mutex (e.g., Redis lock keyed by
account.id) around the token refresh flow to ensure only one refresh runs for an
account, or change the updates to conditional updates that include the snapshot
you read (e.g., use updateMany/update with a where that includes the account.id
plus a snapshot column like account.updatedAt or the previous
access_token/refresh_token value read earlier) so the write only applies if the
stored values match the snapshot; apply the same pattern to the
setTokenRefreshError path.
---
Outside diff comments:
In `@packages/web/src/ee/features/sso/actions.ts`:
- Around line 54-60: The code currently exposes backend refresh failure text by
assigning account.tokenRefreshErrorMessage to the result.error field in the
object pushed by the result.push in actions.ts; change this to emit a
non-sensitive sentinel or boolean (e.g. error:
!!account.tokenRefreshErrorMessage or error: "REFRESH_FAILED") instead of the
raw string so linkedAccountProviderCard.tsx can still check truthiness without
leaking diagnostics; update the object property where
provider/isLinked/accountId/providerAccountId/isAccountLinking are set to use
the boolean/sentinel and ensure undefined is used when there was no error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 29444165-81cc-4596-86a9-2b25d4674472

📥 Commits

Reviewing files that changed from the base of the PR and between a0d4658 and a9252bd.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • packages/backend/src/ee/accountPermissionSyncer.ts
  • packages/backend/src/ee/tokenRefresh.ts
  • packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql
  • packages/db/prisma/schema.prisma
  • packages/web/src/auth.ts
  • packages/web/src/ee/features/sso/actions.ts
  • packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
  • packages/web/src/lib/encryptedPrismaAdapter.ts

Comment threadpackages/backend/src/ee/tokenRefresh.ts
@brendan-kellam
brendan-kellam enabled auto-merge (squash) March 13, 2026 01:28
@brendan-kellam
brendan-kellam merged commit c0b39e6 into mainMar 13, 2026
10 checks passed
@brendan-kellam
brendan-kellam deleted the brendan/fix-token-refresh-in-permission-sync-SOU-664 branch March 13, 2026 01:28
@github-actionsgithub-actionsBot mentioned this pull request Mar 13, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@brendan-kellam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(worker): refresh OAuth tokens in backend permission sync flow - #1000

Merged
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664
Mar 13, 2026
Merged

fix(worker): refresh OAuth tokens in backend permission sync flow#1000
brendan-kellam merged 2 commits into
mainfrom
brendan/fix-token-refresh-in-permission-sync-SOU-664

Conversation

@brendan-kellam

@brendan-kellambrendan-kellam commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • OAuth token refresh was previously only triggered from the Next.js jwt callback, meaning tokens could expire between user visits and cause account-driven permission sync jobs to fail with auth errors.
  • Moves token refresh logic to packages/backend/src/ee/tokenRefresh.ts and calls ensureFreshAccountToken from accountPermissionSyncer before using an account's access token.
  • Adds tokenRefreshErrorMessage to the Account DB model — set on refresh failure, cleared on successful re-authentication — and surfaces it in the linked accounts UI so users know to re-authenticate.

Test plan

  • Verify that an expired OAuth token is automatically refreshed when a permission sync job runs
  • Verify that when refresh fails, tokenRefreshErrorMessage is set on the account and the "Token refresh failed — please reconnect" message appears in the linked accounts UI
  • Verify that successfully re-authenticating clears tokenRefreshErrorMessage and removes the error from the UI
  • Verify existing permission sync flows (GitHub, GitLab, Bitbucket Cloud, Bitbucket Server) still work correctly when tokens are valid

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Re-authentication prompts appear when OAuth token refresh fails to help restore account access.
    • UI refreshes automatically after successful permission refresh to show up-to-date linked account status.
  • Bug Fixes

    • Token refresh handling consolidated to backend for more reliable expiration handling and clearer per-account error reporting.
    • Backend now records token refresh error messages on accounts to aid recovery.

Token refresh was previously only triggered from the Next.js jwt callback,
meaning tokens could expire between user visits and cause account-driven
permission sync jobs to fail silently.
Move refresh logic to packages/backend/src/ee/tokenRefresh.ts and call it
from accountPermissionSyncer before using an account's access token. On
refresh failure, tokenRefreshErrorMessage is set on the Account record and
surfaced in the linked accounts UI so users know to re-authenticate.
Also adds a DB migration for the tokenRefreshErrorMessage field and wires
the signIn event to clear it on successful re-authentication.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Centralizes OAuth token refresh into a backend function (ensureFreshAccountToken(account, db)), persists token refresh errors on the Account record, and updates frontend flows to read/clear per-account token refresh errors instead of session-based provider errors.

Changes

Cohort / File(s)Summary
Database Schema
packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql, packages/db/prisma/schema.prisma
Adds optional tokenRefreshErrorMessage TEXT column/field to Account to persist OAuth refresh errors.
Backend Token Refresh & Syncer
packages/backend/src/ee/tokenRefresh.ts, packages/backend/src/ee/accountPermissionSyncer.ts
Introduces ensureFreshAccountToken(account, db) that validates, decrypts, refreshes tokens, updates DB, and records errors. accountPermissionSyncer now delegates token freshness to this function and removes local decrypt/validation logic.
Frontend Auth Changes
packages/web/src/auth.ts
Removes linkedAccountErrors session/JWT typings and runtime token-refresh logic; clears tokenRefreshErrorMessage on successful sign-in.
SSO UI / Actions
packages/web/src/ee/features/sso/actions.ts, packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
Switches linked-account error source to account.tokenRefreshErrorMessage; triggers router.refresh() after permission refresh to re-fetch UI data.
Adapter / Types
packages/web/src/lib/encryptedPrismaAdapter.ts
Extends encryptAccountData() input shape to accept optional tokenRefreshErrorMessage.
Changelog
CHANGELOG.md
Adds entry describing backend token refresh relocation and error surfacing changes.

Sequence Diagram

sequenceDiagram
participant Syncer as AccountPermissionSyncer
participant Refresh as ensureFreshAccountToken
participant Provider as OAuth Provider
participant DB as Database
participant UI as Frontend UI
Syncer->>Refresh: ensureFreshAccountToken(account, db)
Refresh->>Refresh: Validate access_token presence
Refresh->>Refresh: Check expiry (with buffer)
alt Token valid
Refresh->>Refresh: Decrypt & return access_token
Refresh-->>Syncer: access_token
else Token expired/near expiry
Refresh->>Refresh: Ensure refresh_token exists
Refresh->>Provider: refreshOAuthToken(request with provider creds)
alt Refresh successful
Provider-->>Refresh: token response
Refresh->>DB: Update Account (access_token, refresh_token?, expires_at), clear tokenRefreshErrorMessage
DB-->>Refresh: OK
Refresh-->>Syncer: new access_token
else Refresh failed
Provider-->>Refresh: Error
Refresh->>DB: set tokenRefreshErrorMessage on Account
DB-->>Refresh: OK
Refresh-->>Syncer: throw/error
end
end
Syncer->>Syncer: Continue permission sync using token
UI->>DB: Fetch Account (includes tokenRefreshErrorMessage)
DB-->>UI: Account with tokenRefreshErrorMessage
UI->>UI: Surface message / prompt re-authentication
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • msukkari
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe PR title clearly and accurately describes the main change: moving OAuth token refresh logic into the backend permission sync flow, which is the core objective of the changeset.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch brendan/fix-token-refresh-in-permission-sync-SOU-664
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/web/src/ee/features/sso/actions.ts (1)

54-60: ⚠️ Potential issue | 🟡 Minor

Don't send the raw refresh failure text to the client.

linkedAccountProviderCard.tsx only checks linkedAccount.error for truthiness, so returning account.tokenRefreshErrorMessage here just leaks backend diagnostics to the browser. Collapse it to a boolean or sentinel string instead.

Possible minimal change
- error: account.tokenRefreshErrorMessage ?? undefined,+ error: account.tokenRefreshErrorMessage ? "token_refresh_failed" : undefined,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/ee/features/sso/actions.ts` around lines 54 - 60, The code
currently exposes backend refresh failure text by assigning
account.tokenRefreshErrorMessage to the result.error field in the object pushed
by the result.push in actions.ts; change this to emit a non-sensitive sentinel
or boolean (e.g. error: !!account.tokenRefreshErrorMessage or error:
"REFRESH_FAILED") instead of the raw string so linkedAccountProviderCard.tsx can
still check truthiness without leaking diagnostics; update the object property
where provider/isLinked/accountId/providerAccountId/isAccountLinking are set to
use the boolean/sentinel and ensure undefined is used when there was no error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/ee/tokenRefresh.ts`:
- Around line 123-136: The account writes in db.account.update (used when
writing encryptOAuthToken(refreshResponse.access_token) / refresh_token /
expires_at) and in setTokenRefreshError are vulnerable to races because they
unconditionally overwrite from the job-start snapshot; to fix, serialize or use
compare-and-set: either acquire a per-account mutex (e.g., Redis lock keyed by
account.id) around the token refresh flow to ensure only one refresh runs for an
account, or change the updates to conditional updates that include the snapshot
you read (e.g., use updateMany/update with a where that includes the account.id
plus a snapshot column like account.updatedAt or the previous
access_token/refresh_token value read earlier) so the write only applies if the
stored values match the snapshot; apply the same pattern to the
setTokenRefreshError path.
---
Outside diff comments:
In `@packages/web/src/ee/features/sso/actions.ts`:
- Around line 54-60: The code currently exposes backend refresh failure text by
assigning account.tokenRefreshErrorMessage to the result.error field in the
object pushed by the result.push in actions.ts; change this to emit a
non-sensitive sentinel or boolean (e.g. error:
!!account.tokenRefreshErrorMessage or error: "REFRESH_FAILED") instead of the
raw string so linkedAccountProviderCard.tsx can still check truthiness without
leaking diagnostics; update the object property where
provider/isLinked/accountId/providerAccountId/isAccountLinking are set to use
the boolean/sentinel and ensure undefined is used when there was no error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 29444165-81cc-4596-86a9-2b25d4674472

📥 Commits

Reviewing files that changed from the base of the PR and between a0d4658 and a9252bd.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • packages/backend/src/ee/accountPermissionSyncer.ts
  • packages/backend/src/ee/tokenRefresh.ts
  • packages/db/prisma/migrations/20260313002214_add_account_token_refresh_error_message/migration.sql
  • packages/db/prisma/schema.prisma
  • packages/web/src/auth.ts
  • packages/web/src/ee/features/sso/actions.ts
  • packages/web/src/ee/features/sso/components/linkedAccountProviderCard.tsx
  • packages/web/src/lib/encryptedPrismaAdapter.ts

Comment threadpackages/backend/src/ee/tokenRefresh.ts
@brendan-kellam
brendan-kellam enabled auto-merge (squash) March 13, 2026 01:28
@brendan-kellam
brendan-kellam merged commit c0b39e6 into mainMar 13, 2026
10 checks passed
@brendan-kellam
brendan-kellam deleted the brendan/fix-token-refresh-in-permission-sync-SOU-664 branch March 13, 2026 01:28
@github-actionsgithub-actionsBot mentioned this pull request Mar 13, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@brendan-kellam