Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

update slack config to support refresh token rotation, since the first refresh would succeed and future ones intermittently, depending on whether or not the refresh token was rotated.

Type of Change

  • Bug fix

Testing

Tested manually via agent tool and standalone block

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Oct 15, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Oct 15, 2025 6:44pm

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Summary

This PR fixes an intermittent Slack OAuth refresh token issue caused by Slack's refresh token rotation mechanism. When Slack rotates refresh tokens, concurrent refresh attempts fail because the old refresh token becomes invalid after the first successful use.

Key Changes:

  • Added supportsRefreshTokenRotation: true flag to Slack's OAuth configuration in oauth.ts:861
  • Implemented fallback logic in refreshTokenIfNeeded() that checks the database for a valid token when the refresh fails, handling the race condition where another concurrent request already succeeded
  • Updated domain reference from sim.dev to sim.ai in changelog RSS feed
  • Added previewContextValues prop to ChannelSelectorInput component for better preview mode support with context from parent components
  • Refactored channel selector in tool input to use wrapper component pattern and removed unused ToolInputErrorBoundary class

The core fix addresses the race condition gracefully by checking if another request has already refreshed the token successfully before throwing an error.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are well-thought-out and address a specific race condition in Slack's OAuth refresh token rotation. The fallback logic is defensive and properly handles the edge case. All other changes are minor improvements (domain update, component refactoring) that don't introduce risk.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/lib/oauth/oauth.ts 5/5 Added supportsRefreshTokenRotation: true to Slack OAuth config to enable refresh token rotation handling
apps/sim/app/api/auth/oauth/utils.ts 5/5 Added fallback logic to check DB for valid token when refresh fails due to concurrent request race condition

Sequence Diagram

sequenceDiagram
    participant Client
    participant refreshTokenIfNeeded
    participant refreshOAuthToken
    participant SlackAPI
    participant Database
    
    Client->>refreshTokenIfNeeded: Request with expired token
    refreshTokenIfNeeded->>refreshTokenIfNeeded: Check if token expired
    
    alt Token needs refresh
        refreshTokenIfNeeded->>refreshOAuthToken: Call with refresh token
        refreshOAuthToken->>SlackAPI: POST /oauth.v2.access
        
        alt Refresh succeeds
            SlackAPI-->>refreshOAuthToken: New access token + new refresh token
            refreshOAuthToken-->>refreshTokenIfNeeded: Return tokens
            refreshTokenIfNeeded->>Database: Update access token & refresh token
            Database-->>refreshTokenIfNeeded: Success
            refreshTokenIfNeeded-->>Client: Return new access token
        else Refresh fails (race condition)
            SlackAPI-->>refreshOAuthToken: Error (refresh token already used)
            refreshOAuthToken-->>refreshTokenIfNeeded: Throw error
            refreshTokenIfNeeded->>refreshTokenIfNeeded: Log warning about concurrent request
            refreshTokenIfNeeded->>Database: getCredential() - fetch fresh credential
            Database-->>refreshTokenIfNeeded: Return latest credential
            refreshTokenIfNeeded->>refreshTokenIfNeeded: Check if token still valid
            alt Token valid from concurrent refresh
                refreshTokenIfNeeded-->>Client: Return valid token from DB
            else Token still invalid
                refreshTokenIfNeeded-->>Client: Throw error
            end
        end
    else Token still valid
        refreshTokenIfNeeded-->>Client: Return existing token
    end
Loading

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit 4cceb22 into staging Oct 15, 2025
9 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/slack branch October 15, 2025 18:54
Sign up for free to 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.

2 participants