chore: implement requested changes

This commit is contained in:
Ralph Khreish
2025-09-04 23:07:18 +02:00
parent 538d745023
commit 37a8955494
4 changed files with 78 additions and 17 deletions

View File

@@ -19,10 +19,10 @@
"build:packages": "npm run build:core && npm run build:cli", "build:packages": "npm run build:core && npm run build:cli",
"build:core": "cd packages/tm-core && npm run build", "build:core": "cd packages/tm-core && npm run build",
"build:cli": "cd apps/cli && npm run build", "build:cli": "cd apps/cli && npm run build",
"test": "node --experimental-vm-modules node_modules/.bin/jest", "test": "vitest run",
"test:fails": "node --experimental-vm-modules node_modules/.bin/jest --onlyFailures", "test:fails": "vitest run --changed",
"test:watch": "node --experimental-vm-modules node_modules/.bin/jest --watch", "test:watch": "vitest",
"test:coverage": "node --experimental-vm-modules node_modules/.bin/jest --coverage", "test:coverage": "vitest run --coverage",
"test:e2e": "./tests/e2e/run_e2e.sh", "test:e2e": "./tests/e2e/run_e2e.sh",
"test:e2e-report": "./tests/e2e/run_e2e.sh --analyze-log", "test:e2e-report": "./tests/e2e/run_e2e.sh --analyze-log",
"postpack": "chmod +x dist/task-master.js dist/mcp-server.js", "postpack": "chmod +x dist/task-master.js dist/mcp-server.js",

View File

@@ -3,9 +3,8 @@
*/ */
import { describe, it, expect, beforeEach, vi } from 'vitest'; import { describe, it, expect, beforeEach, vi } from 'vitest';
import { AuthManager } from './auth-manager.js';
// Mock the logger to verify warnings // Mock the logger to verify warnings (must be hoisted before SUT import)
const mockLogger = { const mockLogger = {
warn: vi.fn(), warn: vi.fn(),
info: vi.fn(), info: vi.fn(),
@@ -17,11 +16,66 @@ vi.mock('../logger/index.js', () => ({
getLogger: () => mockLogger getLogger: () => mockLogger
})); }));
// Spy on CredentialStore constructor to verify config propagation
const CredentialStoreSpy = vi.fn();
vi.mock('./credential-store.js', () => {
return {
CredentialStore: class {
constructor(config: any) {
CredentialStoreSpy(config);
this.getCredentials = vi.fn(() => null);
}
getCredentials() {
return null;
}
saveCredentials() {}
clearCredentials() {}
hasValidCredentials() {
return false;
}
}
};
});
// Mock OAuthService to avoid side effects
vi.mock('./oauth-service.js', () => {
return {
OAuthService: class {
constructor() {}
authenticate() {
return Promise.resolve({});
}
getAuthorizationUrl() {
return null;
}
}
};
});
// Mock SupabaseAuthClient to avoid side effects
vi.mock('../clients/supabase-client.js', () => {
return {
SupabaseAuthClient: class {
constructor() {}
refreshSession() {
return Promise.resolve({});
}
signOut() {
return Promise.resolve();
}
}
};
});
// Import SUT after mocks
import { AuthManager } from './auth-manager.js';
describe('AuthManager Singleton', () => { describe('AuthManager Singleton', () => {
beforeEach(() => { beforeEach(() => {
// Reset singleton before each test // Reset singleton before each test
AuthManager.resetInstance(); AuthManager.resetInstance();
vi.clearAllMocks(); vi.clearAllMocks();
CredentialStoreSpy.mockClear();
}); });
it('should return the same instance on multiple calls', () => { it('should return the same instance on multiple calls', () => {
@@ -41,11 +95,14 @@ describe('AuthManager Singleton', () => {
const instance = AuthManager.getInstance(config); const instance = AuthManager.getInstance(config);
expect(instance).toBeDefined(); expect(instance).toBeDefined();
// Verify the config is passed to internal components // Assert that CredentialStore was constructed with the provided config
// This would be observable when attempting operations that use the config expect(CredentialStoreSpy).toHaveBeenCalledTimes(1);
// For example, getCredentials would look in the configured file path expect(CredentialStoreSpy).toHaveBeenCalledWith(config);
// Verify the config is passed to internal components through observable behavior
// getCredentials would look in the configured file path
const credentials = instance.getCredentials(); const credentials = instance.getCredentials();
expect(credentials).toBeNull(); // File doesn't exist, but it should check the right path expect(credentials).toBeNull(); // File doesn't exist, but config was propagated correctly
}); });
it('should warn when config is provided after initialization', () => { it('should warn when config is provided after initialization', () => {
@@ -60,7 +117,7 @@ describe('AuthManager Singleton', () => {
// Verify warning was logged // Verify warning was logged
expect(mockLogger.warn).toHaveBeenCalledWith( expect(mockLogger.warn).toHaveBeenCalledWith(
'getInstance called with config after initialization; config is ignored.' expect.stringMatching(/config.*after initialization.*ignored/i)
); );
}); });

View File

@@ -17,7 +17,7 @@ import { getLogger } from '../logger/index.js';
* Authentication manager class * Authentication manager class
*/ */
export class AuthManager { export class AuthManager {
private static instance: AuthManager; private static instance: AuthManager | null = null;
private credentialStore: CredentialStore; private credentialStore: CredentialStore;
private oauthService: OAuthService; private oauthService: OAuthService;
private supabaseClient: SupabaseAuthClient; private supabaseClient: SupabaseAuthClient;
@@ -41,14 +41,14 @@ export class AuthManager {
'getInstance called with config after initialization; config is ignored.' 'getInstance called with config after initialization; config is ignored.'
); );
} }
return AuthManager.instance!; return AuthManager.instance;
} }
/** /**
* Reset the singleton instance (useful for testing) * Reset the singleton instance (useful for testing)
*/ */
static resetInstance(): void { static resetInstance(): void {
AuthManager.instance = null as any; AuthManager.instance = null;
} }
/** /**
@@ -120,7 +120,7 @@ export class AuthManager {
await this.supabaseClient.signOut(); await this.supabaseClient.signOut();
} catch (error) { } catch (error) {
// Log but don't throw - we still want to clear local credentials // Log but don't throw - we still want to clear local credentials
console.warn('Failed to sign out from Supabase:', error); getLogger('AuthManager').warn('Failed to sign out from Supabase:', error);
} }
// Always clear local credentials (removes auth.json file) // Always clear local credentials (removes auth.json file)

View File

@@ -57,13 +57,17 @@ export class StorageFactory {
case 'api': case 'api':
if (!StorageFactory.isHamsterAvailable(config)) { if (!StorageFactory.isHamsterAvailable(config)) {
const missing: string[] = [];
if (!config.storage?.apiEndpoint) missing.push('apiEndpoint');
if (!config.storage?.apiAccessToken) missing.push('apiAccessToken');
// Check if authenticated via AuthManager // Check if authenticated via AuthManager
const authManager = AuthManager.getInstance(); const authManager = AuthManager.getInstance();
if (!authManager.isAuthenticated()) { if (!authManager.isAuthenticated()) {
throw new TaskMasterError( throw new TaskMasterError(
'API storage configured but not authenticated. Run: tm auth login', `API storage not fully configured (${missing.join(', ') || 'credentials missing'}). Run: tm auth login, or set the missing field(s).`,
ERROR_CODES.MISSING_CONFIGURATION, ERROR_CODES.MISSING_CONFIGURATION,
{ storageType: 'api' } { storageType: 'api', missing }
); );
} }
// Use auth token from AuthManager // Use auth token from AuthManager