chore: address concerns

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);
+	});
+});

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<string, string> {
-		const authData = this.getCredentials();
-
-		if (!authData) {
-			throw new AuthenticationError(
-				'Not authenticated. Please authenticate first.',
-				'NOT_AUTHENTICATED'
-			);
-		}
-
-		return {
-			Authorization: `Bearer ${authData.token}`
-		};
-	}
 }

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}`
+			);
+		}
+	}
 }

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')

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<RuntimeStorageConfig, 'apiConfigured'> {
 	/** 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;

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
 		);
 

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<IConfiguration>,
+			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}`);
 		}