fix: auto-refresh OAuth token before expiry#347
fix: auto-refresh OAuth token before expiry#347arvanus wants to merge 1 commit intozereight:mainfrom
Conversation
The OAuth token was only fetched once at server startup and never refreshed, causing 401 errors after expiry. This adds a proactive refresh timer that re-fetches the token before it expires. - Add initializeOAuthClient() that returns the GitLabOAuth instance - Add getTokenExpiresInMs() to check remaining token lifetime - Schedule refresh timer that runs 5 min before token expiry - Fall back to 30-min refresh interval if no expiry info available - Timer is unref'd so it doesn't prevent process exit Fixes zereight#346 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
167fc5c to
f7374ef
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes OAuth reliability in the GitLab MCP server by (1) making token storage path resolution cross-platform and (2) proactively refreshing OAuth access tokens before they expire, addressing runtime 401s caused by a static startup token.
Changes:
- Use
os.homedir()for default OAuth token storage path (Windows-safe). - Add
initializeOAuthClient()to return both theGitLabOAuthclient instance and the initial access token with a singlegetAccessToken()call. - Add a proactive refresh timer in
index.tsthat refreshes the OAuth token ~5 minutes before expiry andunref()s the timer.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| oauth.ts | Fixes default token path; adds token-expiry introspection and a new initialization API returning both client + token. |
| index.ts | Stores the OAuth client instance and schedules periodic proactive token refreshes to avoid expired-token 401s. |
Comments suppressed due to low confidence (1)
oauth.ts:637
initializeOAuthClient()introduces a new exported initialization surface (and is now used by the server). There aren’t tests covering its behavior (e.g., missing env vars throws, single getAccessToken call, returned client/token shape). Please add targeted tests since oauth.ts already has a dedicated test suite.
export async function initializeOAuthClient(gitlabUrl: string = "https://gitlab.com"): Promise<{ client: GitLabOAuth; accessToken: string }> {
const clientId = process.env.GITLAB_OAUTH_CLIENT_ID;
const clientSecret = process.env.GITLAB_OAUTH_CLIENT_SECRET;
const redirectUri = process.env.GITLAB_OAUTH_REDIRECT_URI || "http://127.0.0.1:8888/callback";
const tokenStoragePath = process.env.GITLAB_OAUTH_TOKEN_PATH;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const delay = expiresInMs != null | ||
| ? Math.max(expiresInMs - REFRESH_BUFFER_MS, 0) | ||
| : DEFAULT_REFRESH_INTERVAL_MS; |
There was a problem hiding this comment.
delay can become 0 when the token is already within the refresh buffer (or expired). If the refresh attempt keeps failing, the reschedule will run again immediately, causing a tight retry loop (CPU + log spam) with no backoff. Consider enforcing a minimum delay and/or adding exponential backoff on consecutive failures.
| try { | ||
| logger.info("Proactively refreshing OAuth token..."); | ||
| const freshToken = await oauthClient.getAccessToken(); | ||
| OAUTH_ACCESS_TOKEN = freshToken; | ||
| logger.info("OAuth token refreshed successfully"); | ||
| } catch (error) { | ||
| logger.error("Failed to refresh OAuth token:", error); | ||
| } |
There was a problem hiding this comment.
The refresh timer calls oauthClient.getAccessToken(), which (per oauth.ts) falls back to starting a new OAuth browser flow when refresh fails or no refresh_token is available. That means the “background refresh” can unexpectedly open a browser / wait for user interaction, which also contradicts the PR description of “log error and keep using last token”. Consider adding a non-interactive refresh method/option (refresh-only) for the timer path and only trigger the browser flow on an explicit/user-initiated action.
| } | ||
|
|
||
| const expiryTime = tokenData.created_at + tokenData.expires_in * 1000; | ||
| const remaining = expiryTime - Date.now(); |
There was a problem hiding this comment.
New public API getTokenExpiresInMs() isn’t covered by the existing OAuth tests. Adding unit tests for the null cases (no token / no expires_in) and for correct remaining-time calculation would help prevent regressions (especially around the 5-minute buffer behavior used by the server).
| const remaining = expiryTime - Date.now(); | |
| // Apply the same 5 minute buffer used in isTokenExpired() | |
| const bufferedExpiryTime = expiryTime - 5 * 60 * 1000; | |
| const remaining = bufferedExpiryTime - Date.now(); |
Summary
Two OAuth issues fixed:
OAUTH_ACCESS_TOKENwas fetched once at startup and never refreshed, causing 401 errors after expiry. Added a proactive refresh timer.process.env.HOMEis not set outside Git Bash on Windows, causing the token storage path to be relative. Replaced withos.homedir()which works on all platforms.Changes
oauth.tsos.homedir()instead ofprocess.env.HOME || ""for reliable cross-platform token storageinitializeOAuthClient()— returns theGitLabOAuthinstance + initial token in a singlegetAccessToken()call (no duplicate browser prompts)getTokenExpiresInMs()— public method to check remaining token lifetimeinitializeOAuth()for backward compatibilityindex.tsGitLabOAuthinstance inoauthClientscheduleOAuthTokenRefresh()— timer that refreshes the token 5 minutes before expiry (matching the buffer inisTokenExpired())unref()'d so it doesn't prevent process exitTest plan
npm run build— compiles without errorsnpm run test:mock— all mock tests passFixes #346
🤖 Generated with Claude Code