Andreas/LibreChat
1
0
Fork 0
mirror of https://github.com/danny-avila/LibreChat.git synced 2025-12-16 16:30:15 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

⚠️ fix: OAuth Error and Token Expiry Detection and Reporting Improvements (#10922)

Browse source 
* fix: create new flows on invalid_grant errors

* chore: fix failing test

* chore: keep isOAuthError test function in sync with implementation

* test: add tests for OAuth error detection on invalid grant errors

* test: add tests for creating new flows when token expires

* test: add test for flow clean up prior to creation

* refactor: consolidate token expiration handling in FlowStateManager

- Removed the old token expiration checks and replaced them with a new method, `isTokenExpired`, to streamline the logic.
- Introduced `normalizeExpirationTimestamp` to handle timestamp normalization for both seconds and milliseconds.
- Updated tests to ensure proper functionality of flow management with token expiration scenarios.

* fix: conditionally setup cleanup handlers in FlowStateManager

- Updated the FlowStateManager constructor to only call setupCleanupHandlers if the ci parameter is not set, improving flexibility in flow management.

* chore: enhance OAuth token refresh logging

- Introduced a new method, `processRefreshResponse`, to streamline the processing of token refresh responses from the OAuth server.
- Improved logging to provide detailed information about token refresh operations, including whether new tokens were received and if the refresh token was rotated.
- Updated existing token handling logic to utilize the new method, ensuring consistency and clarity in token management.

* chore: enhance logging for MCP server reinitialization

- Updated the logging in the reinitMCPServer function to provide more detailed information about the response, including success status, OAuth requirements, presence of the OAuth URL, and the count of tools involved. This improves the clarity and usefulness of logs for debugging purposes.

---------

Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
Dustin Healy 2025-12-12 10:51:28 -08:00 • committed by GitHub
parent ef96ce2b4b
commit abeaab6e17
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 1191 additions and 419 deletions
Download patch file Download diff file Expand all files Collapse all files

7
api/server/services/Tools/mcp.js
View file

@ -127,7 +127,12 @@ async function reinitMCPServer({
oauthUrl,
tools,
};
logger.debug(`[MCP Reinitialize] Response for ${serverName}:`, result);
logger.debug(`[MCP Reinitialize] Response for ${serverName}:`, {
success: result.success,
oauthRequired: result.oauthRequired,
oauthUrl: result.oauthUrl ? 'present' : null,
toolsCount: tools?.length ?? 0,
});
return result;
} catch (error) {
logger.error(

394
packages/api/src/flow/manager.spec.ts
View file

@ -1,394 +0,0 @@
import { Keyv } from 'keyv';
import { FlowStateManager } from './manager';
import { FlowState } from './types';
/** Mock class without extending Keyv */
class MockKeyv {
private store: Map<string, FlowState<string>>;
constructor() {
this.store = new Map();
}
async get(key: string): Promise<FlowState<string> | undefined> {
return this.store.get(key);
}
// eslint-disable-next-line @typescript-eslint/no-unused-vars
async set(key: string, value: FlowState<string>, _ttl?: number): Promise<true> {
this.store.set(key, value);
return true;
}
async delete(key: string): Promise<boolean> {
return this.store.delete(key);
}
}
describe('FlowStateManager', () => {
let flowManager: FlowStateManager<string>;
let store: MockKeyv;
beforeEach(() => {
store = new MockKeyv();
// Type assertion here since we know our mock implements the necessary methods
flowManager = new FlowStateManager(store as unknown as Keyv, { ttl: 30000, ci: true });
});
afterEach(() => {
jest.clearAllMocks();
});
describe('Concurrency Tests', () => {
it('should handle concurrent flow creation and return same result', async () => {
const flowId = 'test-flow';
const type = 'test-type';
// Start two concurrent flow creations
const flow1Promise = flowManager.createFlowWithHandler(flowId, type, async () => {
await new Promise((resolve) => setTimeout(resolve, 100));
return 'result';
});
const flow2Promise = flowManager.createFlowWithHandler(flowId, type, async () => {
await new Promise((resolve) => setTimeout(resolve, 50));
return 'different-result';
});
// Both should resolve to the same result from the first handler
const [result1, result2] = await Promise.all([flow1Promise, flow2Promise]);
expect(result1).toBe('result');
expect(result2).toBe('result');
});
it('should handle flow timeout correctly', async () => {
const flowId = 'timeout-flow';
const type = 'test-type';
// Create flow with very short TTL
const shortTtlManager = new FlowStateManager(store as unknown as Keyv, {
ttl: 100,
ci: true,
});
const flowPromise = shortTtlManager.createFlow(flowId, type);
await expect(flowPromise).rejects.toThrow('test-type flow timed out');
});
it('should maintain flow state consistency under high concurrency', async () => {
const flowId = 'concurrent-flow';
const type = 'test-type';
// Create multiple concurrent operations
const operations = [];
for (let i = 0; i < 10; i++) {
operations.push(
flowManager.createFlowWithHandler(flowId, type, async () => {
await new Promise((resolve) => setTimeout(resolve, Math.random() * 50));
return `result-${i}`;
}),
);
}
// All operations should resolve to the same result
const results = await Promise.all(operations);
const firstResult = results[0];
results.forEach((result: string) => {
expect(result).toBe(firstResult);
});
});
it('should handle race conditions in flow completion', async () => {
const flowId = 'test-flow';
const type = 'test-type';
// Create initial flow
const flowPromise = flowManager.createFlow(flowId, type);
// Increase delay to ensure flow is properly created
await new Promise((resolve) => setTimeout(resolve, 500));
// Complete the flow
await flowManager.completeFlow(flowId, type, 'result1');
const result = await flowPromise;
expect(result).toBe('result1');
}, 15000);
it('should handle concurrent flow monitoring', async () => {
const flowId = 'test-flow';
const type = 'test-type';
// Create initial flow
const flowPromise = flowManager.createFlow(flowId, type);
// Increase delay
await new Promise((resolve) => setTimeout(resolve, 500));
// Complete the flow
await flowManager.completeFlow(flowId, type, 'success');
const result = await flowPromise;
expect(result).toBe('success');
}, 15000);
it('should handle concurrent success and failure attempts', async () => {
const flowId = 'race-flow';
const type = 'test-type';
const flowPromise = flowManager.createFlow(flowId, type);
// Increase delay
await new Promise((resolve) => setTimeout(resolve, 500));
// Fail the flow
await flowManager.failFlow(flowId, type, new Error('failure'));
await expect(flowPromise).rejects.toThrow('failure');
}, 15000);
});
describe('deleteFlow', () => {
const flowId = 'test-flow-123';
const type = 'test-type';
const flowKey = `${type}:${flowId}`;
it('deletes an existing flow', async () => {
await store.set(flowKey, { type, status: 'PENDING', metadata: {}, createdAt: Date.now() });
expect(await store.get(flowKey)).toBeDefined();
const result = await flowManager.deleteFlow(flowId, type);
expect(result).toBe(true);
expect(await store.get(flowKey)).toBeUndefined();
});
it('returns false if the deletion errors', async () => {
jest.spyOn(store, 'delete').mockRejectedValue(new Error('Deletion failed'));
const result = await flowManager.deleteFlow(flowId, type);
expect(result).toBe(false);
});
it('does nothing if the flow does not exist', async () => {
expect(await store.get(flowKey)).toBeUndefined();
const result = await flowManager.deleteFlow(flowId, type);
expect(result).toBe(true);
});
});
describe('isFlowStale', () => {
const flowId = 'test-flow-stale';
const type = 'test-type';
const flowKey = `${type}:${flowId}`;
it('returns not stale for non-existent flow', async () => {
const result = await flowManager.isFlowStale(flowId, type);
expect(result).toEqual({
isStale: false,
age: 0,
});
});
it('returns not stale for PENDING flow regardless of age', async () => {
const oldTimestamp = Date.now() - 10 * 60 * 1000; // 10 minutes ago
await store.set(flowKey, {
type,
status: 'PENDING',
metadata: {},
createdAt: oldTimestamp,
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result).toEqual({
isStale: false,
age: 0,
status: 'PENDING',
});
});
it('returns not stale for recently COMPLETED flow', async () => {
const recentTimestamp = Date.now() - 30 * 1000; // 30 seconds ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 60 * 1000,
completedAt: recentTimestamp,
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(false);
expect(result.status).toBe('COMPLETED');
expect(result.age).toBeGreaterThan(0);
expect(result.age).toBeLessThan(60 * 1000);
});
it('returns stale for old COMPLETED flow', async () => {
const oldTimestamp = Date.now() - 5 * 60 * 1000; // 5 minutes ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 10 * 60 * 1000,
completedAt: oldTimestamp,
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(true);
expect(result.status).toBe('COMPLETED');
expect(result.age).toBeGreaterThan(2 * 60 * 1000);
});
it('returns not stale for recently FAILED flow', async () => {
const recentTimestamp = Date.now() - 30 * 1000; // 30 seconds ago
await store.set(flowKey, {
type,
status: 'FAILED',
metadata: {},
createdAt: Date.now() - 60 * 1000,
failedAt: recentTimestamp,
error: 'Test error',
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(false);
expect(result.status).toBe('FAILED');
expect(result.age).toBeGreaterThan(0);
expect(result.age).toBeLessThan(60 * 1000);
});
it('returns stale for old FAILED flow', async () => {
const oldTimestamp = Date.now() - 5 * 60 * 1000; // 5 minutes ago
await store.set(flowKey, {
type,
status: 'FAILED',
metadata: {},
createdAt: Date.now() - 10 * 60 * 1000,
failedAt: oldTimestamp,
error: 'Test error',
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(true);
expect(result.status).toBe('FAILED');
expect(result.age).toBeGreaterThan(2 * 60 * 1000);
});
it('uses custom stale threshold', async () => {
const timestamp = Date.now() - 90 * 1000; // 90 seconds ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 2 * 60 * 1000,
completedAt: timestamp,
});
// 90 seconds old, threshold 60 seconds = stale
const result1 = await flowManager.isFlowStale(flowId, type, 60 * 1000);
expect(result1.isStale).toBe(true);
// 90 seconds old, threshold 120 seconds = not stale
const result2 = await flowManager.isFlowStale(flowId, type, 120 * 1000);
expect(result2.isStale).toBe(false);
});
it('uses default threshold of 2 minutes when not specified', async () => {
const timestamp = Date.now() - 3 * 60 * 1000; // 3 minutes ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5 * 60 * 1000,
completedAt: timestamp,
});
// Should use default 2 minute threshold
const result = await flowManager.isFlowStale(flowId, type);
expect(result.isStale).toBe(true);
expect(result.age).toBeGreaterThan(2 * 60 * 1000);
});
it('falls back to createdAt when completedAt/failedAt are not present', async () => {
const createdTimestamp = Date.now() - 5 * 60 * 1000; // 5 minutes ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: createdTimestamp,
// No completedAt or failedAt
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(true);
expect(result.status).toBe('COMPLETED');
expect(result.age).toBeGreaterThan(2 * 60 * 1000);
});
it('handles flow with no timestamps', async () => {
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
// No timestamps at all
} as FlowState<string>);
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(false);
expect(result.age).toBe(0);
expect(result.status).toBe('COMPLETED');
});
it('prefers completedAt over createdAt for age calculation', async () => {
const createdTimestamp = Date.now() - 10 * 60 * 1000; // 10 minutes ago
const completedTimestamp = Date.now() - 30 * 1000; // 30 seconds ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: createdTimestamp,
completedAt: completedTimestamp,
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
// Should use completedAt (30s) not createdAt (10m)
expect(result.isStale).toBe(false);
expect(result.age).toBeLessThan(60 * 1000);
});
it('prefers failedAt over createdAt for age calculation', async () => {
const createdTimestamp = Date.now() - 10 * 60 * 1000; // 10 minutes ago
const failedTimestamp = Date.now() - 30 * 1000; // 30 seconds ago
await store.set(flowKey, {
type,
status: 'FAILED',
metadata: {},
createdAt: createdTimestamp,
failedAt: failedTimestamp,
error: 'Test error',
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
// Should use failedAt (30s) not createdAt (10m)
expect(result.isStale).toBe(false);
expect(result.age).toBeLessThan(60 * 1000);
});
});
});

990
packages/api/src/flow/manager.test.ts Normal file
View file

@ -0,0 +1,990 @@
import { Keyv } from 'keyv';
import { FlowStateManager } from './manager';
import { FlowState } from './types';
jest.mock('@librechat/data-schemas', () => ({
...jest.requireActual('@librechat/data-schemas'),
logger: {
info: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
debug: jest.fn(),
},
}));
/** Mock class without extending Keyv */
class MockKeyv<T = string> {
private store: Map<string, FlowState<T>>;
constructor() {
this.store = new Map();
}
async get(key: string): Promise<FlowState<T> | undefined> {
return this.store.get(key);
}
// eslint-disable-next-line @typescript-eslint/no-unused-vars
async set(key: string, value: FlowState<T>, _ttl?: number): Promise<true> {
this.store.set(key, value);
return true;
}
async delete(key: string): Promise<boolean> {
return this.store.delete(key);
}
}
describe('FlowStateManager', () => {
let flowManager: FlowStateManager<string>;
let store: MockKeyv;
beforeEach(() => {
store = new MockKeyv();
// Type assertion here since we know our mock implements the necessary methods
flowManager = new FlowStateManager(store as unknown as Keyv, { ttl: 30000, ci: true });
});
afterEach(() => {
jest.clearAllMocks();
});
describe('Concurrency Tests', () => {
it('should handle concurrent flow creation and return same result', async () => {
const flowId = 'test-flow';
const type = 'test-type';
// Start two concurrent flow creations
const flow1Promise = flowManager.createFlowWithHandler(flowId, type, async () => {
await new Promise((resolve) => setTimeout(resolve, 100));
return 'result';
});
const flow2Promise = flowManager.createFlowWithHandler(flowId, type, async () => {
await new Promise((resolve) => setTimeout(resolve, 50));
return 'different-result';
});
// Both should resolve to the same result from the first handler
const [result1, result2] = await Promise.all([flow1Promise, flow2Promise]);
expect(result1).toBe('result');
expect(result2).toBe('result');
});
it('should handle flow timeout correctly', async () => {
const flowId = 'timeout-flow';
const type = 'test-type';
// Create flow with very short TTL
const shortTtlManager = new FlowStateManager(store as unknown as Keyv, {
ttl: 100,
ci: true,
});
const flowPromise = shortTtlManager.createFlow(flowId, type);
await expect(flowPromise).rejects.toThrow('test-type flow timed out');
});
it('should maintain flow state consistency under high concurrency', async () => {
const flowId = 'concurrent-flow';
const type = 'test-type';
// Create multiple concurrent operations
const operations = [];
for (let i = 0; i < 10; i++) {
operations.push(
flowManager.createFlowWithHandler(flowId, type, async () => {
await new Promise((resolve) => setTimeout(resolve, Math.random() * 50));
return `result-${i}`;
}),
);
}
// All operations should resolve to the same result
const results = await Promise.all(operations);
const firstResult = results[0];
results.forEach((result: string) => {
expect(result).toBe(firstResult);
});
});
it('should handle race conditions in flow completion', async () => {
const flowId = 'test-flow';
const type = 'test-type';
// Create initial flow
const flowPromise = flowManager.createFlow(flowId, type);
// Increase delay to ensure flow is properly created
await new Promise((resolve) => setTimeout(resolve, 500));
// Complete the flow
await flowManager.completeFlow(flowId, type, 'result1');
const result = await flowPromise;
expect(result).toBe('result1');
}, 15000);
it('should handle concurrent flow monitoring', async () => {
const flowId = 'test-flow';
const type = 'test-type';
// Create initial flow
const flowPromise = flowManager.createFlow(flowId, type);
// Increase delay
await new Promise((resolve) => setTimeout(resolve, 500));
// Complete the flow
await flowManager.completeFlow(flowId, type, 'success');
const result = await flowPromise;
expect(result).toBe('success');
}, 15000);
it('should handle concurrent success and failure attempts', async () => {
const flowId = 'race-flow';
const type = 'test-type';
const flowPromise = flowManager.createFlow(flowId, type);
// Increase delay
await new Promise((resolve) => setTimeout(resolve, 500));
// Fail the flow
await flowManager.failFlow(flowId, type, new Error('failure'));
await expect(flowPromise).rejects.toThrow('failure');
}, 15000);
});
describe('deleteFlow', () => {
const flowId = 'test-flow-123';
const type = 'test-type';
const flowKey = `${type}:${flowId}`;
it('deletes an existing flow', async () => {
await store.set(flowKey, { type, status: 'PENDING', metadata: {}, createdAt: Date.now() });
expect(await store.get(flowKey)).toBeDefined();
const result = await flowManager.deleteFlow(flowId, type);
expect(result).toBe(true);
expect(await store.get(flowKey)).toBeUndefined();
});
it('returns false if the deletion errors', async () => {
jest.spyOn(store, 'delete').mockRejectedValue(new Error('Deletion failed'));
const result = await flowManager.deleteFlow(flowId, type);
expect(result).toBe(false);
});
it('does nothing if the flow does not exist', async () => {
expect(await store.get(flowKey)).toBeUndefined();
const result = await flowManager.deleteFlow(flowId, type);
expect(result).toBe(true);
});
});
describe('createFlowWithHandler - token expiration', () => {
const flowId = 'token-flow';
const type = 'mcp_get_tokens';
const flowKey = `${type}:${flowId}`;
type TokenResult = {
access_token: string;
refresh_token: string;
expires_at?: number;
};
let tokenFlowManager: FlowStateManager<TokenResult>;
let tokenStore: MockKeyv<TokenResult>;
beforeEach(() => {
tokenStore = new MockKeyv<TokenResult>();
tokenFlowManager = new FlowStateManager(tokenStore as unknown as Keyv, {
ttl: 30000,
ci: true,
});
});
it('should execute handler when existing flow has expired token', async () => {
const expiredTokenResult: TokenResult = {
access_token: 'expired_token',
refresh_token: 'refresh_token',
expires_at: Date.now() - 1000, // Expired 1 second ago
};
// Create flow with expired token
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
completedAt: Date.now() - 4000,
result: expiredTokenResult,
} as FlowState<TokenResult>);
const newTokenResult: TokenResult = {
access_token: 'new_token',
refresh_token: 'new_refresh',
expires_at: Date.now() + 3600000,
};
const handlerSpy = jest.fn().mockResolvedValue(newTokenResult);
const result = await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
// Handler should be called because token is expired
expect(handlerSpy).toHaveBeenCalled();
expect(result).toEqual(newTokenResult);
});
it('should reuse existing flow when token is still valid', async () => {
const validTokenResult: TokenResult = {
access_token: 'valid_token',
refresh_token: 'refresh_token',
expires_at: Date.now() + 3600000, // Expires in 1 hour
};
// Create flow with valid token
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
completedAt: Date.now() - 4000,
result: validTokenResult,
} as FlowState<TokenResult>);
const handlerSpy = jest.fn().mockResolvedValue({
access_token: 'new_token',
refresh_token: 'new_refresh',
expires_at: Date.now() + 3600000,
});
const resultPromise = tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
// Complete the monitored flow
await new Promise((resolve) => setTimeout(resolve, 500));
const result = await resultPromise;
// Handler should NOT be called because token is still valid
expect(handlerSpy).not.toHaveBeenCalled();
expect(result).toEqual(validTokenResult);
}, 15000);
it('should reuse existing flow when no expires_at field exists', async () => {
const tokenResultWithoutExpiry: TokenResult = {
access_token: 'token_without_expiry',
refresh_token: 'refresh_token',
// No expires_at field - handles flows without expiration metadata
};
// Create flow without expires_at
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
completedAt: Date.now() - 4000,
result: tokenResultWithoutExpiry,
} as FlowState<TokenResult>);
const handlerSpy = jest.fn().mockResolvedValue({
access_token: 'new_token',
refresh_token: 'new_refresh',
expires_at: Date.now() + 3600000,
});
const resultPromise = tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
// Wait for flow monitoring
await new Promise((resolve) => setTimeout(resolve, 500));
const result = await resultPromise;
// Handler should NOT be called - flows without expires_at are treated as valid/non-expired
expect(handlerSpy).not.toHaveBeenCalled();
expect(result).toEqual(tokenResultWithoutExpiry);
}, 15000);
it('should treat NaN expires_at as non-expired and reuse existing flow', async () => {
const tokenResultWithNaN: TokenResult = {
access_token: 'token_with_nan',
refresh_token: 'refresh_token',
expires_at: NaN,
};
// Create flow with NaN expires_at
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
completedAt: Date.now() - 4000,
result: tokenResultWithNaN,
} as FlowState<TokenResult>);
const handlerSpy = jest.fn().mockResolvedValue({
access_token: 'new_token',
refresh_token: 'new_refresh',
expires_at: Date.now() + 3600000,
});
const resultPromise = tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
// Wait for flow monitoring
await new Promise((resolve) => setTimeout(resolve, 500));
const result = await resultPromise;
// Handler should NOT be called - NaN is treated as invalid expiration (non-expired)
expect(handlerSpy).not.toHaveBeenCalled();
expect(result).toEqual(tokenResultWithNaN);
}, 15000);
it('should handle expires_at in seconds format (Unix timestamp)', async () => {
const expiredSecondsTimestamp = Math.floor(Date.now() / 1000) - 60;
const expiredTokenResult: TokenResult = {
access_token: 'expired_token_seconds',
refresh_token: 'refresh_token',
expires_at: expiredSecondsTimestamp,
};
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
completedAt: Date.now() - 4000,
result: expiredTokenResult,
} as FlowState<TokenResult>);
const newTokenResult: TokenResult = {
access_token: 'new_token',
refresh_token: 'new_refresh',
expires_at: Date.now() + 3600000,
};
const handlerSpy = jest.fn().mockResolvedValue(newTokenResult);
const result = await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
// Handler SHOULD be called - token in seconds format is expired
expect(handlerSpy).toHaveBeenCalled();
expect(result).toEqual(newTokenResult);
});
it('should reuse flow when expires_at in seconds format is still valid', async () => {
const validSecondsTimestamp = Math.floor(Date.now() / 1000) + 3600;
const validTokenResult: TokenResult = {
access_token: 'valid_token_seconds',
refresh_token: 'refresh_token',
expires_at: validSecondsTimestamp,
};
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
completedAt: Date.now() - 4000,
result: validTokenResult,
} as FlowState<TokenResult>);
const handlerSpy = jest.fn().mockResolvedValue({
access_token: 'new_token',
refresh_token: 'new_refresh',
expires_at: Date.now() + 3600000,
});
const resultPromise = tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
await new Promise((resolve) => setTimeout(resolve, 500));
const result = await resultPromise;
// Handler should NOT be called - seconds-format token is still valid
expect(handlerSpy).not.toHaveBeenCalled();
expect(result).toEqual(validTokenResult);
}, 15000);
});
describe('Timestamp normalization', () => {
const SECONDS_THRESHOLD = 1e10;
const flowId = 'normalization-test';
const type = 'timestamp_test';
const flowKey = `${type}:${flowId}`;
type TokenResult = {
access_token: string;
expires_at: number;
};
let tokenFlowManager: FlowStateManager<TokenResult>;
let tokenStore: MockKeyv<TokenResult>;
beforeEach(() => {
tokenStore = new MockKeyv<TokenResult>();
tokenFlowManager = new FlowStateManager(tokenStore as unknown as Keyv, {
ttl: 30000,
ci: true,
});
});
describe('Seconds format detection (values < 1e10)', () => {
it('should normalize current Unix timestamp in seconds (Dec 2024: ~1734000000)', async () => {
const currentSeconds = Math.floor(Date.now() / 1000);
const expiredSecondsAgo = currentSeconds - 60;
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'test', expires_at: expiredSecondsAgo },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
expect(handlerSpy).toHaveBeenCalled();
});
it('should handle Sept 2001 timestamp in seconds (1000000000)', async () => {
const sept2001Seconds = 1000000000;
expect(sept2001Seconds).toBeLessThan(SECONDS_THRESHOLD);
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'test', expires_at: sept2001Seconds },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
expect(handlerSpy).toHaveBeenCalled();
});
it('should handle year 2000 timestamp in seconds (946684800)', async () => {
const year2000Seconds = 946684800;
expect(year2000Seconds).toBeLessThan(SECONDS_THRESHOLD);
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'test', expires_at: year2000Seconds },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
expect(handlerSpy).toHaveBeenCalled();
});
it('should handle May 2033 timestamp in seconds (2000000000) - future but still seconds format', async () => {
const may2033Seconds = 2000000000;
expect(may2033Seconds).toBeLessThan(SECONDS_THRESHOLD);
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'test', expires_at: may2033Seconds },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
const resultPromise = tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
await new Promise((resolve) => setTimeout(resolve, 500));
await resultPromise;
expect(handlerSpy).not.toHaveBeenCalled();
}, 15000);
it('should handle edge case: just below threshold (9999999999 - ~Nov 2286 in seconds)', async () => {
const justBelowThreshold = 9999999999;
expect(justBelowThreshold).toBeLessThan(SECONDS_THRESHOLD);
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'test', expires_at: justBelowThreshold },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
const resultPromise = tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
await new Promise((resolve) => setTimeout(resolve, 500));
await resultPromise;
expect(handlerSpy).not.toHaveBeenCalled();
}, 15000);
});
describe('Milliseconds format detection (values >= 1e10)', () => {
it('should recognize current timestamp in milliseconds (Dec 2024: ~1734000000000)', async () => {
const currentMs = Date.now();
const expiredMsAgo = currentMs - 60000;
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'test', expires_at: expiredMsAgo },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
expect(handlerSpy).toHaveBeenCalled();
});
it('should handle Sept 2001 timestamp in milliseconds (1000000000000)', async () => {
const sept2001Ms = 1000000000000;
expect(sept2001Ms).toBeGreaterThanOrEqual(SECONDS_THRESHOLD);
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'test', expires_at: sept2001Ms },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
expect(handlerSpy).toHaveBeenCalled();
});
it('should handle edge case: exactly at threshold (10000000000 - ~April 1970 in ms)', async () => {
const exactlyAtThreshold = 10000000000;
expect(exactlyAtThreshold).toEqual(SECONDS_THRESHOLD);
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'test', expires_at: exactlyAtThreshold },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
expect(handlerSpy).toHaveBeenCalled();
});
it('should handle edge case: just above threshold (10000000001)', async () => {
const justAboveThreshold = 10000000001;
expect(justAboveThreshold).toBeGreaterThan(SECONDS_THRESHOLD);
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'test', expires_at: justAboveThreshold },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
expect(handlerSpy).toHaveBeenCalled();
});
it('should handle future timestamp in milliseconds (2000000000000 - May 2033)', async () => {
const may2033Ms = 2000000000000;
expect(may2033Ms).toBeGreaterThanOrEqual(SECONDS_THRESHOLD);
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'test', expires_at: may2033Ms },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
const resultPromise = tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
await new Promise((resolve) => setTimeout(resolve, 500));
await resultPromise;
expect(handlerSpy).not.toHaveBeenCalled();
}, 15000);
});
describe('Real-world OAuth provider timestamp formats', () => {
it('should handle Google/MCP OAuth style (milliseconds, current + expires_in * 1000)', async () => {
const expiresIn = 3600;
const googleStyleExpiry = Date.now() + expiresIn * 1000;
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'google_token', expires_at: googleStyleExpiry },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 7200000 });
const resultPromise = tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
await new Promise((resolve) => setTimeout(resolve, 500));
await resultPromise;
expect(handlerSpy).not.toHaveBeenCalled();
}, 15000);
it('should handle OIDC style (seconds, Unix epoch)', async () => {
const oidcStyleExpiry = Math.floor(Date.now() / 1000) + 3600;
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'oidc_token', expires_at: oidcStyleExpiry },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 7200000 });
const resultPromise = tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
await new Promise((resolve) => setTimeout(resolve, 500));
await resultPromise;
expect(handlerSpy).not.toHaveBeenCalled();
}, 15000);
it('should handle expired Google/MCP OAuth style token', async () => {
const expiredGoogleStyle = Date.now() - 60000;
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 70000,
result: { access_token: 'expired_google', expires_at: expiredGoogleStyle },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
expect(handlerSpy).toHaveBeenCalled();
});
it('should handle expired OIDC style token', async () => {
const expiredOidcStyle = Math.floor(Date.now() / 1000) - 60;
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 70000,
result: { access_token: 'expired_oidc', expires_at: expiredOidcStyle },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
expect(handlerSpy).toHaveBeenCalled();
});
});
describe('Threshold boundary validation', () => {
/**
* The threshold (1e10 = 10 billion) was chosen because:
* - In SECONDS: 10 billion seconds from epoch = ~year 2286
* - In MILLISECONDS: 10 billion ms from epoch = ~April 1970
*
* So any reasonable timestamp:
* - Less than 10 billion -> must be seconds (we haven't reached year 2286 yet)
* - Greater than or equal to 10 billion -> must be milliseconds (we're past April 1970)
*/
it('should correctly identify threshold represents ~April 1970 in milliseconds', () => {
const thresholdAsDate = new Date(SECONDS_THRESHOLD);
expect(thresholdAsDate.getFullYear()).toBe(1970);
expect(thresholdAsDate.getMonth()).toBe(3);
});
it('should correctly identify threshold represents ~year 2286 in seconds', () => {
const thresholdAsSecondsDate = new Date(SECONDS_THRESHOLD * 1000);
expect(thresholdAsSecondsDate.getFullYear()).toBe(2286);
});
it('should handle token expiring exactly at threshold boundary', async () => {
await tokenStore.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5000,
result: { access_token: 'test', expires_at: SECONDS_THRESHOLD },
} as FlowState<TokenResult>);
const handlerSpy = jest
.fn()
.mockResolvedValue({ access_token: 'new', expires_at: Date.now() + 3600000 });
await tokenFlowManager.createFlowWithHandler(flowId, type, handlerSpy);
expect(handlerSpy).toHaveBeenCalled();
});
});
});
describe('isFlowStale', () => {
const flowId = 'test-flow-stale';
const type = 'test-type';
const flowKey = `${type}:${flowId}`;
it('returns not stale for non-existent flow', async () => {
const result = await flowManager.isFlowStale(flowId, type);
expect(result).toEqual({
isStale: false,
age: 0,
});
});
it('returns not stale for PENDING flow regardless of age', async () => {
const oldTimestamp = Date.now() - 10 * 60 * 1000; // 10 minutes ago
await store.set(flowKey, {
type,
status: 'PENDING',
metadata: {},
createdAt: oldTimestamp,
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result).toEqual({
isStale: false,
age: 0,
status: 'PENDING',
});
});
it('returns not stale for recently COMPLETED flow', async () => {
const recentTimestamp = Date.now() - 30 * 1000; // 30 seconds ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 60 * 1000,
completedAt: recentTimestamp,
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(false);
expect(result.status).toBe('COMPLETED');
expect(result.age).toBeGreaterThan(0);
expect(result.age).toBeLessThan(60 * 1000);
});
it('returns stale for old COMPLETED flow', async () => {
const oldTimestamp = Date.now() - 5 * 60 * 1000; // 5 minutes ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 10 * 60 * 1000,
completedAt: oldTimestamp,
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(true);
expect(result.status).toBe('COMPLETED');
expect(result.age).toBeGreaterThan(2 * 60 * 1000);
});
it('returns not stale for recently FAILED flow', async () => {
const recentTimestamp = Date.now() - 30 * 1000; // 30 seconds ago
await store.set(flowKey, {
type,
status: 'FAILED',
metadata: {},
createdAt: Date.now() - 60 * 1000,
failedAt: recentTimestamp,
error: 'Test error',
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(false);
expect(result.status).toBe('FAILED');
expect(result.age).toBeGreaterThan(0);
expect(result.age).toBeLessThan(60 * 1000);
});
it('returns stale for old FAILED flow', async () => {
const oldTimestamp = Date.now() - 5 * 60 * 1000; // 5 minutes ago
await store.set(flowKey, {
type,
status: 'FAILED',
metadata: {},
createdAt: Date.now() - 10 * 60 * 1000,
failedAt: oldTimestamp,
error: 'Test error',
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(true);
expect(result.status).toBe('FAILED');
expect(result.age).toBeGreaterThan(2 * 60 * 1000);
});
it('uses custom stale threshold', async () => {
const timestamp = Date.now() - 90 * 1000; // 90 seconds ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 2 * 60 * 1000,
completedAt: timestamp,
});
// 90 seconds old, threshold 60 seconds = stale
const result1 = await flowManager.isFlowStale(flowId, type, 60 * 1000);
expect(result1.isStale).toBe(true);
// 90 seconds old, threshold 120 seconds = not stale
const result2 = await flowManager.isFlowStale(flowId, type, 120 * 1000);
expect(result2.isStale).toBe(false);
});
it('uses default threshold of 2 minutes when not specified', async () => {
const timestamp = Date.now() - 3 * 60 * 1000; // 3 minutes ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: Date.now() - 5 * 60 * 1000,
completedAt: timestamp,
});
// Should use default 2 minute threshold
const result = await flowManager.isFlowStale(flowId, type);
expect(result.isStale).toBe(true);
expect(result.age).toBeGreaterThan(2 * 60 * 1000);
});
it('falls back to createdAt when completedAt/failedAt are not present', async () => {
const createdTimestamp = Date.now() - 5 * 60 * 1000; // 5 minutes ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: createdTimestamp,
// No completedAt or failedAt
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(true);
expect(result.status).toBe('COMPLETED');
expect(result.age).toBeGreaterThan(2 * 60 * 1000);
});
it('handles flow with no timestamps', async () => {
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
// No timestamps at all
} as FlowState<string>);
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
expect(result.isStale).toBe(false);
expect(result.age).toBe(0);
expect(result.status).toBe('COMPLETED');
});
it('prefers completedAt over createdAt for age calculation', async () => {
const createdTimestamp = Date.now() - 10 * 60 * 1000; // 10 minutes ago
const completedTimestamp = Date.now() - 30 * 1000; // 30 seconds ago
await store.set(flowKey, {
type,
status: 'COMPLETED',
metadata: {},
createdAt: createdTimestamp,
completedAt: completedTimestamp,
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
// Should use completedAt (30s) not createdAt (10m)
expect(result.isStale).toBe(false);
expect(result.age).toBeLessThan(60 * 1000);
});
it('prefers failedAt over createdAt for age calculation', async () => {
const createdTimestamp = Date.now() - 10 * 60 * 1000; // 10 minutes ago
const failedTimestamp = Date.now() - 30 * 1000; // 30 seconds ago
await store.set(flowKey, {
type,
status: 'FAILED',
metadata: {},
createdAt: createdTimestamp,
failedAt: failedTimestamp,
error: 'Test error',
});
const result = await flowManager.isFlowStale(flowId, type, 2 * 60 * 1000);
// Should use failedAt (30s) not createdAt (10m)
expect(result.isStale).toBe(false);
expect(result.age).toBeLessThan(60 * 1000);
});
});
});

56
packages/api/src/flow/manager.ts
View file

@ -21,7 +21,10 @@ export class FlowStateManager<T = unknown> {
this.ttl = ttl;
this.keyv = store;
this.intervals = new Set();
this.setupCleanupHandlers();
if (!ci) {
this.setupCleanupHandlers();
}
}
private setupCleanupHandlers() {
@ -42,6 +45,49 @@ export class FlowStateManager<T = unknown> {
return `${type}:${flowId}`;
}
/**
* Normalizes an expiration timestamp to milliseconds.
* Detects whether the input is in seconds or milliseconds based on magnitude.
* Timestamps below 10 billion are assumed to be in seconds (valid until ~2286).
* @param timestamp - The expiration timestamp (in seconds or milliseconds)
* @returns The timestamp normalized to milliseconds
*/
private normalizeExpirationTimestamp(timestamp: number): number {
const SECONDS_THRESHOLD = 1e10;
if (timestamp < SECONDS_THRESHOLD) {
return timestamp * 1000;
}
return timestamp;
}
/**
* Checks if a flow's token has expired based on its expires_at field
* @param flowState - The flow state to check
* @returns true if the token has expired, false otherwise (including if no expires_at exists)
*/
private isTokenExpired(flowState: FlowState<T> | undefined): boolean {
if (!flowState?.result) {
return false;
}
if (typeof flowState.result !== 'object') {
return false;
}
if (!('expires_at' in flowState.result)) {
return false;
}
const expiresAt = (flowState.result as { expires_at: unknown }).expires_at;
if (typeof expiresAt !== 'number' || !Number.isFinite(expiresAt)) {
return false;
}
const normalizedExpiresAt = this.normalizeExpirationTimestamp(expiresAt);
return normalizedExpiresAt < Date.now();
}
/**
* Creates a new flow and waits for its completion
*/
@ -272,16 +318,16 @@ export class FlowStateManager<T = unknown> {
): Promise<T> {
const flowKey = this.getFlowKey(flowId, type);
let existingState = (await this.keyv.get(flowKey)) as FlowState<T> | undefined;
if (existingState) {
logger.debug(`[${flowKey}] Flow already exists`);
if (existingState && !this.isTokenExpired(existingState)) {
logger.debug(`[${flowKey}] Flow already exists with valid token`);
return this.monitorFlow(flowKey, type, signal);
}
await new Promise((resolve) => setTimeout(resolve, 250));
existingState = (await this.keyv.get(flowKey)) as FlowState<T> | undefined;
if (existingState) {
logger.debug(`[${flowKey}] Flow exists on 2nd check`);
if (existingState && !this.isTokenExpired(existingState)) {
logger.debug(`[${flowKey}] Flow exists on 2nd check with valid token`);
return this.monitorFlow(flowKey, type, signal);
}

4
packages/api/src/mcp/MCPConnectionFactory.ts
View file

@ -167,6 +167,10 @@ export class MCPConnectionFactory {
config?.oauth,
);
// Delete any existing flow state to ensure we start fresh
// This prevents stale codeVerifier issues when re-authenticating
await this.flowManager!.deleteFlow(flowId, 'mcp_oauth');
// Create the flow state so the OAuth callback can find it
// We spawn this in the background without waiting for it
this.flowManager!.createFlow(flowId, 'mcp_oauth', flowMetadata).catch(() => {

12
packages/api/src/mcp/__tests__/MCPConnection.test.ts
View file

@ -70,6 +70,10 @@ describe('MCPConnection Error Detection', () => {
if (message.includes('401') || message.includes('non-200 status code (401)')) {
return true;
}
// Check for invalid_grant (OAuth servers return this for expired/revoked grants)
if (message.includes('invalid_grant')) {
return true;
}
// Check for invalid_token (OAuth servers return this for expired/revoked tokens)
if (message.includes('invalid_token')) {
return true;
@ -159,6 +163,14 @@ describe('MCPConnection Error Detection', () => {
const error = { message: 'The access token is invalid_token or expired' };
expect(isOAuthError(error)).toBe(true);
});
it('should detect OAuth error for invalid_grant', () => {
const error = {
message:
'Streamable HTTP error: Error POSTing to endpoint: {"error":"invalid_grant","error_description":"The provided authorization grant is invalid, expired, or revoked"}',
};
expect(isOAuthError(error)).toBe(true);
});
});
describe('error type differentiation', () => {

73
packages/api/src/mcp/__tests__/MCPConnectionFactory.test.ts
View file

@ -49,6 +49,7 @@ describe('MCPConnectionFactory', () => {
createFlow: jest.fn(),
createFlowWithHandler: jest.fn(),
getFlowState: jest.fn(),
deleteFlow: jest.fn().mockResolvedValue(true),
} as unknown as jest.Mocked<FlowStateManager<MCPOAuthTokens | null>>;
mockConnectionInstance = {
@ -266,6 +267,78 @@ describe('MCPConnectionFactory', () => {
}),
);
});
it('should delete existing flow before creating new OAuth flow to prevent stale codeVerifier', async () => {
const basicOptions = {
serverName: 'test-server',
serverConfig: mockServerConfig,
user: mockUser,
};
const oauthOptions = {
user: mockUser,
useOAuth: true,
returnOnOAuth: true,
oauthStart: jest.fn(),
flowManager: mockFlowManager,
};
const mockFlowData = {
authorizationUrl: 'https://auth.example.com',
flowId: 'user123:test-server',
flowMetadata: {
serverName: 'test-server',
userId: 'user123',
serverUrl: 'https://api.example.com',
state: 'test-state',
codeVerifier: 'new-code-verifier-xyz',
clientInfo: { client_id: 'test-client' },
metadata: {
authorization_endpoint: 'https://auth.example.com/authorize',
token_endpoint: 'https://auth.example.com/token',
issuer: 'https://api.example.com',
},
},
};
mockMCPOAuthHandler.initiateOAuthFlow.mockResolvedValue(mockFlowData);
mockFlowManager.deleteFlow.mockResolvedValue(true);
mockFlowManager.createFlow.mockRejectedValue(new Error('Timeout expected'));
mockConnectionInstance.isConnected.mockResolvedValue(false);
let oauthRequiredHandler: (data: Record<string, unknown>) => Promise<void>;
mockConnectionInstance.on.mockImplementation((event, handler) => {
if (event === 'oauthRequired') {
oauthRequiredHandler = handler as (data: Record<string, unknown>) => Promise<void>;
}
return mockConnectionInstance;
});
try {
await MCPConnectionFactory.create(basicOptions, oauthOptions);
} catch {
// Expected to fail due to connection not established
}
await oauthRequiredHandler!({ serverUrl: 'https://api.example.com' });
// Verify deleteFlow was called with correct parameters
expect(mockFlowManager.deleteFlow).toHaveBeenCalledWith('user123:test-server', 'mcp_oauth');
// Verify deleteFlow was called before createFlow
const deleteCallOrder = mockFlowManager.deleteFlow.mock.invocationCallOrder[0];
const createCallOrder = mockFlowManager.createFlow.mock.invocationCallOrder[0];
expect(deleteCallOrder).toBeLessThan(createCallOrder);
// Verify createFlow was called with fresh metadata
expect(mockFlowManager.createFlow).toHaveBeenCalledWith(
'user123:test-server',
'mcp_oauth',
expect.objectContaining({
codeVerifier: 'new-code-verifier-xyz',
}),
);
});
});
describe('connection retry logic', () => {

6
packages/api/src/mcp/connection.ts
View file

@ -652,7 +652,7 @@ export class MCPConnection extends EventEmitter {
}
// Check if it's an OAuth authentication error
if (errorCode === 401 || errorCode === 403) {
if (this.isOAuthError(error)) {
logger.warn(`${this.getLogPrefix()} OAuth authentication error detected`);
this.emit('oauthError', error);
}
@ -815,6 +815,10 @@ export class MCPConnection extends EventEmitter {
if (message.includes('invalid_token')) {
return true;
}
// Check for invalid_grant (OAuth servers return this for expired/revoked grants)
if (message.includes('invalid_grant')) {
return true;
}
// Check for authentication required
if (message.includes('authentication required') || message.includes('unauthorized')) {
return true;

54
packages/api/src/mcp/oauth/handler.ts
View file

@ -558,6 +558,39 @@ export class MCPOAuthHandler {
: `${baseUrl}/api/mcp/oauth/callback`;
}
/**
* Processes and logs a token refresh response from an OAuth server.
* Normalizes the response to MCPOAuthTokens format and logs debug info about refresh token rotation.
*/
private static processRefreshResponse(
tokens: Record<string, unknown>,
serverName: string,
source: string,
): MCPOAuthTokens {
const hasNewRefreshToken = !!tokens.refresh_token;
logger.debug(`[MCPOAuth] Token refresh response (${source})`, {
serverName,
has_new_access_token: !!tokens.access_token,
has_new_refresh_token: hasNewRefreshToken,
refresh_token_rotated: hasNewRefreshToken,
expires_in: tokens.expires_in,
});
if (!hasNewRefreshToken) {
logger.debug(
`[MCPOAuth] OAuth server did not return new refresh_token for ${serverName} - existing refresh token remains valid (normal for non-rotating providers)`,
);
}
return {
...tokens,
obtained_at: Date.now(),
expires_at:
typeof tokens.expires_in === 'number' ? Date.now() + tokens.expires_in * 1000 : undefined,
} as MCPOAuthTokens;
}
/**
* Refreshes OAuth tokens using a refresh token
*/
@ -687,12 +720,7 @@ export class MCPOAuthHandler {
}
const tokens = await response.json();
return {
...tokens,
obtained_at: Date.now(),
expires_at: tokens.expires_in ? Date.now() + tokens.expires_in * 1000 : undefined,
};
return this.processRefreshResponse(tokens, metadata.serverName, 'stored client info');
}
// Fallback: If we have pre-configured OAuth settings, use them
@ -771,12 +799,7 @@ export class MCPOAuthHandler {
}
const tokens = await response.json();
return {
...tokens,
obtained_at: Date.now(),
expires_at: tokens.expires_in ? Date.now() + tokens.expires_in * 1000 : undefined,
};
return this.processRefreshResponse(tokens, metadata.serverName, 'pre-configured OAuth');
}
/** For auto-discovered OAuth, we need the server URL */
@ -828,12 +851,7 @@ export class MCPOAuthHandler {
}
const tokens = await response.json();
return {
...tokens,
obtained_at: Date.now(),
expires_at: tokens.expires_in ? Date.now() + tokens.expires_in * 1000 : undefined,
};
return this.processRefreshResponse(tokens, metadata.serverName, 'auto-discovered OAuth');
} catch (error) {
logger.error(`[MCPOAuth] Failed to refresh tokens for ${metadata.serverName}`, error);
throw error;

14
packages/api/src/mcp/oauth/tokens.ts
View file

@ -134,6 +134,9 @@ export class MCPTokenStorage {
// Store refresh token if available
if (tokens.refresh_token) {
logger.debug(
`${logPrefix} New refresh token received from OAuth server, will store/update`,
);
const encryptedRefreshToken = await encryptV2(tokens.refresh_token);
const extendedTokens = tokens as ExtendedOAuthTokens;
const refreshTokenExpiry = extendedTokens.refresh_token_expires_in
@ -173,6 +176,10 @@ export class MCPTokenStorage {
await createToken(refreshTokenData);
logger.debug(`${logPrefix} Created refresh token (no update methods available)`);
}
} else {
logger.debug(
`${logPrefix} No refresh token in response - OAuth server did not rotate refresh token (this is normal for some providers)`,
);
}
/** Store client information if provided */
@ -320,6 +327,13 @@ export class MCPTokenStorage {
const newTokens = await refreshTokens(decryptedRefreshToken, metadata);
logger.debug(`${logPrefix} Refresh completed`, {
has_new_access_token: !!newTokens.access_token,
has_new_refresh_token: !!newTokens.refresh_token,
refresh_token_will_be_rotated: !!newTokens.refresh_token,
expires_at: newTokens.expires_at,
});
// Store the refreshed tokens (handles both create and update)
// Pass existing token state to avoid duplicate DB calls
await this.storeTokens({