diff --git a/packages/tm-core/src/auth/auth-manager.test.ts b/packages/tm-core/src/auth/auth-manager.test.ts new file mode 100644 index 00000000..fc9e9ba0 --- /dev/null +++ b/packages/tm-core/src/auth/auth-manager.test.ts @@ -0,0 +1,82 @@ +/** + * Tests for AuthManager singleton behavior + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { AuthManager } from './auth-manager'; + +// Mock the logger to verify warnings +vi.mock('../logger', () => ({ + getLogger: () => ({ + warn: vi.fn(), + info: vi.fn(), + debug: vi.fn(), + error: vi.fn() + }) +})); + +describe('AuthManager Singleton', () => { + beforeEach(() => { + // Reset singleton before each test + AuthManager.resetInstance(); + }); + + it('should return the same instance on multiple calls', () => { + const instance1 = AuthManager.getInstance(); + const instance2 = AuthManager.getInstance(); + + expect(instance1).toBe(instance2); + }); + + it('should use config on first call', () => { + const config = { + baseUrl: 'https://test.auth.com', + configDir: '/test/config', + configFile: '/test/config/auth.json' + }; + + const instance = AuthManager.getInstance(config); + expect(instance).toBeDefined(); + }); + + it('should warn when config is provided after initialization', () => { + const logger = vi.mocked(require('../logger').getLogger()); + + // First call with config + AuthManager.getInstance({ baseUrl: 'https://first.auth.com' }); + + // Second call with different config + AuthManager.getInstance({ baseUrl: 'https://second.auth.com' }); + + // Verify warning was logged + expect(logger.warn).toHaveBeenCalledWith( + 'getInstance called with config after initialization; config is ignored.' + ); + }); + + it('should not warn when no config is provided after initialization', () => { + const logger = vi.mocked(require('../logger').getLogger()); + + // First call with config + AuthManager.getInstance({ configDir: '/test/config' }); + + // Second call without config + AuthManager.getInstance(); + + // Verify no warning was logged + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('should allow resetting the instance', () => { + const instance1 = AuthManager.getInstance(); + + // Reset the instance + AuthManager.resetInstance(); + + // Get new instance + const instance2 = AuthManager.getInstance(); + + // They should be different instances + expect(instance1).not.toBe(instance2); + }); +}); \ No newline at end of file diff --git a/packages/tm-core/src/auth/auth-manager.ts b/packages/tm-core/src/auth/auth-manager.ts index a87705a0..3242a3fd 100644 --- a/packages/tm-core/src/auth/auth-manager.ts +++ b/packages/tm-core/src/auth/auth-manager.ts @@ -142,22 +142,4 @@ export class AuthManager { isAuthenticated(): boolean { return this.credentialStore.hasValidCredentials(); } - - /** - * Get authorization headers - */ - getAuthHeaders(): Record { - const authData = this.getCredentials(); - - if (!authData) { - throw new AuthenticationError( - 'Not authenticated. Please authenticate first.', - 'NOT_AUTHENTICATED' - ); - } - - return { - Authorization: `Bearer ${authData.token}` - }; - } } diff --git a/packages/tm-core/src/auth/credential-store.ts b/packages/tm-core/src/auth/credential-store.ts index f55e0300..d9218d8e 100644 --- a/packages/tm-core/src/auth/credential-store.ts +++ b/packages/tm-core/src/auth/credential-store.ts @@ -3,6 +3,7 @@ */ import fs from 'fs'; +import path from 'path'; import { AuthCredentials, AuthenticationError, AuthConfig } from './types'; import { getAuthConfig } from './config'; import { getLogger } from '../logger'; @@ -28,10 +29,25 @@ export class CredentialStore { fs.readFileSync(this.config.configFile, 'utf-8') ) as AuthCredentials; - // Check if token is expired + // Normalize/migrate timestamps to numeric (handles both string ISO dates and numbers) + const expiresAtMs = + typeof authData.expiresAt === 'number' + ? authData.expiresAt + : authData.expiresAt + ? Date.parse(authData.expiresAt as unknown as string) + : undefined; + + // Update the authData with normalized timestamp + if (expiresAtMs !== undefined) { + authData.expiresAt = expiresAtMs; + } + + // Check if token is expired (API keys never expire) + const isApiKey = authData.tokenType === 'api_key'; if ( - authData.expiresAt && - new Date(authData.expiresAt) < new Date() && + !isApiKey && + expiresAtMs && + expiresAtMs < Date.now() && !options?.allowExpired ) { this.logger.warn('Authentication token has expired'); @@ -43,6 +59,21 @@ export class CredentialStore { this.logger.error( `Failed to read auth credentials: ${(error as Error).message}` ); + + // Quarantine corrupt file to prevent repeated errors + try { + if (fs.existsSync(this.config.configFile)) { + const corruptFile = `${this.config.configFile}.corrupt-${Date.now()}`; + fs.renameSync(this.config.configFile, corruptFile); + this.logger.warn(`Quarantined corrupt auth file to: ${corruptFile}`); + } + } catch (quarantineError) { + // If we can't quarantine, log but don't throw + this.logger.debug( + `Could not quarantine corrupt file: ${(quarantineError as Error).message}` + ); + } + return null; } } @@ -106,4 +137,48 @@ export class CredentialStore { getConfig(): AuthConfig { return { ...this.config }; } + + /** + * Clean up old corrupt auth files + * Removes corrupt files older than the specified age + */ + cleanupCorruptFiles(maxAgeMs: number = 7 * 24 * 60 * 60 * 1000): void { + try { + const dir = path.dirname(this.config.configFile); + const baseName = path.basename(this.config.configFile); + const corruptPattern = new RegExp(`^${baseName}\\.corrupt-\\d+$`); + + if (!fs.existsSync(dir)) { + return; + } + + const files = fs.readdirSync(dir); + const now = Date.now(); + + for (const file of files) { + if (corruptPattern.test(file)) { + const filePath = path.join(dir, file); + try { + const stats = fs.statSync(filePath); + const age = now - stats.mtimeMs; + + if (age > maxAgeMs) { + fs.unlinkSync(filePath); + this.logger.debug(`Cleaned up old corrupt file: ${file}`); + } + } catch (error) { + // Ignore errors for individual file cleanup + this.logger.debug( + `Could not clean up corrupt file ${file}: ${(error as Error).message}` + ); + } + } + } + } catch (error) { + // Log but don't throw - this is a cleanup operation + this.logger.debug( + `Error during corrupt file cleanup: ${(error as Error).message}` + ); + } + } } diff --git a/packages/tm-core/src/config/config-manager.ts b/packages/tm-core/src/config/config-manager.ts index 318c5a7f..51867b63 100644 --- a/packages/tm-core/src/config/config-manager.ts +++ b/packages/tm-core/src/config/config-manager.ts @@ -6,7 +6,10 @@ * maintainability, testability, and separation of concerns. */ -import type { PartialConfiguration } from '../interfaces/configuration.interface.js'; +import type { + PartialConfiguration, + RuntimeStorageConfig +} from '../interfaces/configuration.interface.js'; import { ConfigLoader } from './services/config-loader.service.js'; import { ConfigMerger, @@ -134,12 +137,7 @@ export class ConfigManager { /** * Get storage configuration */ - getStorageConfig(): { - type: 'file' | 'api' | 'auto'; - apiEndpoint?: string; - apiAccessToken?: string; - apiConfigured: boolean; - } { + getStorageConfig(): RuntimeStorageConfig { const storage = this.config.storage; // Return the configured type (including 'auto') diff --git a/packages/tm-core/src/interfaces/configuration.interface.ts b/packages/tm-core/src/interfaces/configuration.interface.ts index ca215447..b41e5717 100644 --- a/packages/tm-core/src/interfaces/configuration.interface.ts +++ b/packages/tm-core/src/interfaces/configuration.interface.ts @@ -74,9 +74,38 @@ export interface TagSettings { } /** - * Storage and persistence settings + * Storage type options + * - 'file': Local file system storage + * - 'api': Remote API storage (Hamster integration) + * - 'auto': Automatically detect based on auth status */ -export interface StorageSettings { +export type StorageType = 'file' | 'api' | 'auto'; + +/** + * Runtime storage configuration used for storage backend selection + * This is what getStorageConfig() returns and what StorageFactory expects + */ +export interface RuntimeStorageConfig { + /** Storage backend type */ + type: StorageType; + /** API endpoint for API storage (Hamster integration) */ + apiEndpoint?: string; + /** Access token for API authentication */ + apiAccessToken?: string; + /** + * Indicates whether API is configured (has endpoint or token) + * @computed Derived automatically from presence of apiEndpoint or apiAccessToken + * @internal Should not be set manually - computed by ConfigManager + */ + apiConfigured: boolean; +} + +/** + * Storage and persistence settings + * Extended storage settings including file operation preferences + */ +export interface StorageSettings + extends Omit { /** Storage backend type - 'auto' detects based on auth status */ type: 'file' | 'api' | 'auto'; /** Base path for file storage */ @@ -85,7 +114,11 @@ export interface StorageSettings { apiEndpoint?: string; /** Access token for API authentication */ apiAccessToken?: string; - /** Indicates whether API is configured (has endpoint or token) */ + /** + * Indicates whether API is configured (has endpoint or token) + * @computed Derived automatically from presence of apiEndpoint or apiAccessToken + * @internal Should not be set manually in user config - computed by ConfigManager + */ apiConfigured?: boolean; /** Enable automatic backups */ enableBackup: boolean; diff --git a/packages/tm-core/src/services/task-service.ts b/packages/tm-core/src/services/task-service.ts index b65df172..d607777f 100644 --- a/packages/tm-core/src/services/task-service.ts +++ b/packages/tm-core/src/services/task-service.ts @@ -64,8 +64,8 @@ export class TaskService { const storageConfig = this.configManager.getStorageConfig(); const projectRoot = this.configManager.getProjectRoot(); - this.storage = StorageFactory.create( - { storage: storageConfig } as any, + this.storage = StorageFactory.createFromStorageConfig( + storageConfig, projectRoot ); diff --git a/packages/tm-core/src/storage/storage-factory.ts b/packages/tm-core/src/storage/storage-factory.ts index 3b1d551d..60546d3c 100644 --- a/packages/tm-core/src/storage/storage-factory.ts +++ b/packages/tm-core/src/storage/storage-factory.ts @@ -3,8 +3,11 @@ */ import type { IStorage } from '../interfaces/storage.interface.js'; -import type { IConfiguration } from '../interfaces/configuration.interface.js'; -import { FileStorage } from './file-storage'; +import type { + IConfiguration, + RuntimeStorageConfig +} from '../interfaces/configuration.interface.js'; +import { FileStorage } from './file-storage/index.js'; import { ApiStorage } from './api-storage.js'; import { ERROR_CODES, TaskMasterError } from '../errors/task-master-error.js'; import { AuthManager } from '../auth/auth-manager.js'; @@ -14,6 +17,24 @@ import { getLogger } from '../logger/index.js'; * Factory for creating storage implementations based on configuration */ export class StorageFactory { + /** + * Create a storage implementation from runtime storage config + * This is the preferred method when you have a RuntimeStorageConfig + * @param storageConfig - Runtime storage configuration + * @param projectPath - Project root path (for file storage) + * @returns Storage implementation + */ + static createFromStorageConfig( + storageConfig: RuntimeStorageConfig, + projectPath: string + ): IStorage { + // Wrap the storage config in the expected format + return StorageFactory.create( + { storage: storageConfig } as Partial, + projectPath + ); + } + /** * Create a storage implementation based on configuration * @param config - Configuration object @@ -189,6 +210,11 @@ export class StorageFactory { // File storage doesn't require additional config break; + case 'auto': + // Auto storage is valid - it will determine the actual type at runtime + // No specific validation needed as it will fall back to file if API not configured + break; + default: errors.push(`Unknown storage type: ${storageType}`); }