Compare commits

...

4 Commits

Author SHA1 Message Date
Ralph Khreish
08d3720f92 chore: fix integration test 2025-10-13 21:38:41 +02:00
Ralph Khreish
0a9ec7f44b chore: fix format 2025-10-13 21:18:46 +02:00
Ralph Khreish
8ee37d6812 chore: apply requested coderabbit changes 2025-10-13 15:59:22 +02:00
github-actions[bot]
9986b9838b chore: rc version bump 2025-10-12 17:27:02 +00:00
7 changed files with 255 additions and 30 deletions

View File

@@ -1,11 +1,11 @@
{ {
"mode": "exit", "mode": "pre",
"tag": "rc", "tag": "rc",
"initialVersions": { "initialVersions": {
"task-master-ai": "0.28.0", "task-master-ai": "0.29.0-rc.0",
"@tm/cli": "", "@tm/cli": "",
"docs": "0.0.5", "docs": "0.0.5",
"extension": "0.25.5", "extension": "0.25.6-rc.0",
"@tm/ai-sdk-provider-grok-cli": "", "@tm/ai-sdk-provider-grok-cli": "",
"@tm/build-config": "", "@tm/build-config": "",
"@tm/claude-code-plugin": "0.0.1", "@tm/claude-code-plugin": "0.0.1",
@@ -16,6 +16,7 @@
"mean-planes-wave", "mean-planes-wave",
"nice-ways-hope", "nice-ways-hope",
"plain-falcons-serve", "plain-falcons-serve",
"silent-bushes-grow",
"smart-owls-relax" "smart-owls-relax"
] ]
} }

View File

@@ -1,5 +1,11 @@
# task-master-ai # task-master-ai
## 0.29.0-rc.1
### Patch Changes
- [#1299](https://github.com/eyaltoledano/claude-task-master/pull/1299) [`a6c5152`](https://github.com/eyaltoledano/claude-task-master/commit/a6c5152f20edd8717cf1aea34e7c178b1261aa99) Thanks [@Crunchyman-ralph](https://github.com/Crunchyman-ralph)! - Improve refresh token when authenticating
## 0.29.0-rc.0 ## 0.29.0-rc.0
### Minor Changes ### Minor Changes

View File

@@ -1,6 +1,6 @@
{ {
"name": "task-master-ai", "name": "task-master-ai",
"version": "0.29.0-rc.0", "version": "0.29.0-rc.1",
"description": "A task management system for ambitious AI-driven development that doesn't overwhelm and confuse Cursor.", "description": "A task management system for ambitious AI-driven development that doesn't overwhelm and confuse Cursor.",
"main": "index.js", "main": "index.js",
"type": "module", "type": "module",

View File

@@ -21,11 +21,16 @@ const CredentialStoreSpy = vi.fn();
vi.mock('./credential-store.js', () => { vi.mock('./credential-store.js', () => {
return { return {
CredentialStore: class { CredentialStore: class {
static getInstance(config?: any) {
return new (this as any)(config);
}
static resetInstance() {
// Mock reset instance method
}
constructor(config: any) { constructor(config: any) {
CredentialStoreSpy(config); CredentialStoreSpy(config);
this.getCredentials = vi.fn(() => null);
} }
getCredentials() { getCredentials(_options?: any) {
return null; return null;
} }
saveCredentials() {} saveCredentials() {}
@@ -85,7 +90,7 @@ describe('AuthManager Singleton', () => {
expect(instance1).toBe(instance2); expect(instance1).toBe(instance2);
}); });
it('should use config on first call', () => { it('should use config on first call', async () => {
const config = { const config = {
baseUrl: 'https://test.auth.com', baseUrl: 'https://test.auth.com',
configDir: '/test/config', configDir: '/test/config',
@@ -101,7 +106,7 @@ describe('AuthManager Singleton', () => {
// Verify the config is passed to internal components through observable behavior // Verify the config is passed to internal components through observable behavior
// getCredentials would look in the configured file path // getCredentials would look in the configured file path
const credentials = instance.getCredentials(); const credentials = await instance.getCredentials();
expect(credentials).toBeNull(); // File doesn't exist, but config was propagated correctly expect(credentials).toBeNull(); // File doesn't exist, but config was propagated correctly
}); });

View File

@@ -30,6 +30,7 @@ export class AuthManager {
private supabaseClient: SupabaseAuthClient; private supabaseClient: SupabaseAuthClient;
private organizationService?: OrganizationService; private organizationService?: OrganizationService;
private logger = getLogger('AuthManager'); private logger = getLogger('AuthManager');
private refreshPromise: Promise<AuthCredentials> | null = null;
private constructor(config?: Partial<AuthConfig>) { private constructor(config?: Partial<AuthConfig>) {
this.credentialStore = CredentialStore.getInstance(config); this.credentialStore = CredentialStore.getInstance(config);
@@ -37,7 +38,10 @@ export class AuthManager {
this.oauthService = new OAuthService(this.credentialStore, config); this.oauthService = new OAuthService(this.credentialStore, config);
// Initialize Supabase client with session restoration // Initialize Supabase client with session restoration
this.initializeSupabaseSession(); // Fire-and-forget with catch handler to prevent unhandled rejections
this.initializeSupabaseSession().catch(() => {
// Errors are already logged in initializeSupabaseSession
});
} }
/** /**
@@ -82,38 +86,49 @@ export class AuthManager {
* Automatically refreshes the token if expired * Automatically refreshes the token if expired
*/ */
async getCredentials(): Promise<AuthCredentials | null> { async getCredentials(): Promise<AuthCredentials | null> {
const credentials = this.credentialStore.getCredentials(); const credentials = this.credentialStore.getCredentials({
allowExpired: true
});
// If credentials exist but are expired, try to refresh
if (!credentials) { if (!credentials) {
const expiredCredentials = this.credentialStore.getCredentials({ return null;
allowExpired: true }
});
// Check if we have any credentials at all // Check if credentials are expired (with 30-second clock skew buffer)
if (!expiredCredentials) { const CLOCK_SKEW_MS = 30_000;
// No credentials found const isExpired = credentials.expiresAt
return null; ? new Date(credentials.expiresAt).getTime() <= Date.now() + CLOCK_SKEW_MS
: false;
// If expired and we have a refresh token, attempt refresh
if (isExpired && credentials.refreshToken) {
// Return existing refresh promise if one is in progress
if (this.refreshPromise) {
try {
return await this.refreshPromise;
} catch {
return null;
}
} }
// Check if refresh token is available
if (!expiredCredentials.refreshToken) {
this.logger.warn(
'Token expired but no refresh token available. Please re-authenticate.'
);
return null;
}
// Attempt refresh
try { try {
this.logger.info('Token expired, attempting automatic refresh...'); this.logger.info('Token expired, attempting automatic refresh...');
return await this.refreshToken(); this.refreshPromise = this.refreshToken();
const result = await this.refreshPromise;
return result;
} catch (error) { } catch (error) {
this.logger.warn('Automatic token refresh failed:', error); this.logger.warn('Automatic token refresh failed:', error);
return null; return null;
} finally {
this.refreshPromise = null;
} }
} }
// Return null if expired and no refresh token
if (isExpired) {
return null;
}
return credentials; return credentials;
} }

View File

@@ -0,0 +1,176 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import fs from 'fs';
import os from 'os';
import path from 'path';
import type { Session } from '@supabase/supabase-js';
import { AuthManager } from '../../src/auth/auth-manager';
import { CredentialStore } from '../../src/auth/credential-store';
import type { AuthCredentials } from '../../src/auth/types';
describe('AuthManager Token Refresh', () => {
let authManager: AuthManager;
let credentialStore: CredentialStore;
let tmpDir: string;
let authFile: string;
beforeEach(() => {
// Reset singletons
AuthManager.resetInstance();
CredentialStore.resetInstance();
// Create temporary directory for test isolation
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tm-auth-refresh-'));
authFile = path.join(tmpDir, 'auth.json');
// Initialize AuthManager with test config (this will create CredentialStore internally)
authManager = AuthManager.getInstance({
configDir: tmpDir,
configFile: authFile
});
// Get the CredentialStore instance that AuthManager created
credentialStore = CredentialStore.getInstance();
credentialStore.clearCredentials();
});
afterEach(() => {
// Clean up
try {
credentialStore.clearCredentials();
} catch {
// Ignore cleanup errors
}
AuthManager.resetInstance();
CredentialStore.resetInstance();
vi.restoreAllMocks();
// Remove temporary directory
if (tmpDir && fs.existsSync(tmpDir)) {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
it('should not make concurrent refresh requests', async () => {
// Set up expired credentials with refresh token
const expiredCredentials: AuthCredentials = {
token: 'expired_access_token',
refreshToken: 'valid_refresh_token',
userId: 'test-user-id',
email: 'test@example.com',
expiresAt: new Date(Date.now() - 1000).toISOString(), // Expired 1 second ago
savedAt: new Date().toISOString()
};
credentialStore.saveCredentials(expiredCredentials);
// Mock the refreshToken method to track calls
const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken');
const mockSession: Session = {
access_token: 'new_access_token',
refresh_token: 'new_refresh_token',
expires_at: Math.floor(Date.now() / 1000) + 3600,
user: {
id: 'test-user-id',
email: 'test@example.com',
app_metadata: {},
user_metadata: {},
aud: 'authenticated',
created_at: new Date().toISOString()
}
};
refreshTokenSpy.mockResolvedValue({
token: mockSession.access_token,
refreshToken: mockSession.refresh_token,
userId: mockSession.user.id,
email: mockSession.user.email,
expiresAt: new Date(mockSession.expires_at! * 1000).toISOString(),
savedAt: new Date().toISOString()
});
// Make multiple concurrent calls to getCredentials
const promises = [
authManager.getCredentials(),
authManager.getCredentials(),
authManager.getCredentials()
];
const results = await Promise.all(promises);
// Verify all calls returned the same new credentials
expect(results[0]?.token).toBe('new_access_token');
expect(results[1]?.token).toBe('new_access_token');
expect(results[2]?.token).toBe('new_access_token');
// Verify refreshToken was only called once, not three times
expect(refreshTokenSpy).toHaveBeenCalledTimes(1);
});
it('should return valid credentials without attempting refresh', async () => {
// Set up valid (non-expired) credentials
const validCredentials: AuthCredentials = {
token: 'valid_access_token',
refreshToken: 'valid_refresh_token',
userId: 'test-user-id',
email: 'test@example.com',
expiresAt: new Date(Date.now() + 3600000).toISOString(), // Expires in 1 hour
savedAt: new Date().toISOString()
};
credentialStore.saveCredentials(validCredentials);
// Spy on refreshToken to ensure it's not called
const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken');
const credentials = await authManager.getCredentials();
expect(credentials?.token).toBe('valid_access_token');
expect(refreshTokenSpy).not.toHaveBeenCalled();
});
it('should return null if credentials are expired with no refresh token', async () => {
// Set up expired credentials WITHOUT refresh token
const expiredCredentials: AuthCredentials = {
token: 'expired_access_token',
refreshToken: undefined,
userId: 'test-user-id',
email: 'test@example.com',
expiresAt: new Date(Date.now() - 1000).toISOString(), // Expired 1 second ago
savedAt: new Date().toISOString()
};
credentialStore.saveCredentials(expiredCredentials);
const credentials = await authManager.getCredentials();
expect(credentials).toBeNull();
});
it('should return null if no credentials exist', async () => {
const credentials = await authManager.getCredentials();
expect(credentials).toBeNull();
});
it('should handle refresh failures gracefully', async () => {
// Set up expired credentials with refresh token
const expiredCredentials: AuthCredentials = {
token: 'expired_access_token',
refreshToken: 'invalid_refresh_token',
userId: 'test-user-id',
email: 'test@example.com',
expiresAt: new Date(Date.now() - 1000).toISOString(),
savedAt: new Date().toISOString()
};
credentialStore.saveCredentials(expiredCredentials);
// Mock refreshToken to throw an error
const refreshTokenSpy = vi.spyOn(authManager as any, 'refreshToken');
refreshTokenSpy.mockRejectedValue(new Error('Refresh failed'));
const credentials = await authManager.getCredentials();
expect(credentials).toBeNull();
expect(refreshTokenSpy).toHaveBeenCalledTimes(1);
});
});

View File

@@ -6,6 +6,9 @@
*/ */
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import fs from 'fs';
import os from 'os';
import path from 'path';
import type { Session } from '@supabase/supabase-js'; import type { Session } from '@supabase/supabase-js';
import { AuthManager } from '../../src/auth/auth-manager'; import { AuthManager } from '../../src/auth/auth-manager';
import { CredentialStore } from '../../src/auth/credential-store'; import { CredentialStore } from '../../src/auth/credential-store';
@@ -14,6 +17,8 @@ import type { AuthCredentials } from '../../src/auth/types';
describe('AuthManager - Token Auto-Refresh Integration', () => { describe('AuthManager - Token Auto-Refresh Integration', () => {
let authManager: AuthManager; let authManager: AuthManager;
let credentialStore: CredentialStore; let credentialStore: CredentialStore;
let tmpDir: string;
let authFile: string;
// Mock Supabase session that will be returned on refresh // Mock Supabase session that will be returned on refresh
const mockRefreshedSession: Session = { const mockRefreshedSession: Session = {
@@ -34,10 +39,21 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
}; };
beforeEach(() => { beforeEach(() => {
// Reset AuthManager singleton // Reset singletons
AuthManager.resetInstance(); AuthManager.resetInstance();
CredentialStore.resetInstance();
// Clear any existing credentials // Create temporary directory for test isolation
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tm-auth-integration-'));
authFile = path.join(tmpDir, 'auth.json');
// Initialize AuthManager with test config (this will create CredentialStore internally)
authManager = AuthManager.getInstance({
configDir: tmpDir,
configFile: authFile
});
// Get the CredentialStore instance that AuthManager created
credentialStore = CredentialStore.getInstance(); credentialStore = CredentialStore.getInstance();
credentialStore.clearCredentials(); credentialStore.clearCredentials();
}); });
@@ -50,7 +66,13 @@ describe('AuthManager - Token Auto-Refresh Integration', () => {
// Ignore cleanup errors // Ignore cleanup errors
} }
AuthManager.resetInstance(); AuthManager.resetInstance();
CredentialStore.resetInstance();
vi.restoreAllMocks(); vi.restoreAllMocks();
// Remove temporary directory
if (tmpDir && fs.existsSync(tmpDir)) {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
}); });
describe('Expired Token Detection', () => { describe('Expired Token Detection', () => {