Merge pull request #1488 from eyaltoledano/next

Crunchyman-ralph · web-flow · commit adcbba925232 · 2025-12-04T18:31:26.000+01:00
diff --git a/.changeset/nine-lilies-repair.md b/.changeset/nine-lilies-repair.md
@@ -0,0 +1,5 @@
+---
+"task-master-ai": patch
+---
+
+Improve auth-related error handling
diff --git a/apps/cli/src/utils/error-handler.ts b/apps/cli/src/utils/error-handler.ts
@@ -3,6 +3,11 @@
  * Provides consistent error formatting and debug mode detection
  */
 
+import {
+	AuthenticationError,
+	isSupabaseAuthError,
+	AUTH_ERROR_MESSAGES
+} from '@tm/core';
 import chalk from 'chalk';
 
 /**
@@ -36,6 +41,28 @@ export function displayError(
 		const sanitized = error.getSanitizedDetails();
 		console.error(chalk.red(`\n${sanitized.message}`));
 
+		// Show stack trace in debug mode or if forced
+		if ((isDebugMode() || options.forceStack) && error.stack) {
+			console.error(chalk.gray('\nStack trace:'));
+			console.error(chalk.gray(error.stack));
+		}
+	} else if (error instanceof AuthenticationError) {
+		// Handle AuthenticationError with clean message (no "Error:" prefix)
+		console.error(chalk.red(`\n${error.message}`));
+
+		// Show stack trace in debug mode or if forced
+		if ((isDebugMode() || options.forceStack) && error.stack) {
+			console.error(chalk.gray('\nStack trace:'));
+			console.error(chalk.gray(error.stack));
+		}
+	} else if (isSupabaseAuthError(error)) {
+		// Handle raw Supabase auth errors with user-friendly messages
+		const code = error.code;
+		const userMessage = code
+			? AUTH_ERROR_MESSAGES[code] || error.message
+			: error.message;
+		console.error(chalk.red(`\n${userMessage}`));
+
 		// Show stack trace in debug mode or if forced
 		if ((isDebugMode() || options.forceStack) && error.stack) {
 			console.error(chalk.gray('\nStack trace:'));
diff --git a/packages/tm-core/src/index.ts b/packages/tm-core/src/index.ts
@@ -96,6 +96,13 @@ export {
 	type LocalOnlyCommand
 } from './modules/auth/index.js';
 
+// Auth error utilities (shared with CLI)
+export {
+	isSupabaseAuthError,
+	AUTH_ERROR_MESSAGES,
+	isRecoverableStaleSessionError
+} from './modules/auth/index.js';
+
 // Brief types
 export type { Brief } from './modules/briefs/types.js';
 export type { TagWithStats } from './modules/briefs/services/brief-service.js';
diff --git a/packages/tm-core/src/modules/auth/index.ts b/packages/tm-core/src/modules/auth/index.ts
@@ -38,3 +38,12 @@ export {
 	LOCAL_ONLY_COMMANDS,
 	type LocalOnlyCommand
 } from './constants.js';
+
+// Auth error utilities (shared with CLI)
+export {
+	isSupabaseAuthError,
+	AUTH_ERROR_MESSAGES,
+	RECOVERABLE_STALE_SESSION_ERRORS,
+	isRecoverableStaleSessionError,
+	toAuthenticationError
+} from './utils/index.js';
diff --git a/packages/tm-core/src/modules/auth/utils/auth-error-utils.ts b/packages/tm-core/src/modules/auth/utils/auth-error-utils.ts
@@ -0,0 +1,93 @@
+/**
+ * Shared authentication error utilities
+ * These utilities are used by both tm-core (Supabase client) and CLI (error handler)
+ */
+
+import { isAuthError, type AuthError } from '@supabase/supabase-js';
+import { AuthenticationError } from '../types.js';
+
+/**
+ * Check if an error is a Supabase auth error.
+ * Uses Supabase's public isAuthError helper for stable identification.
+ */
+export function isSupabaseAuthError(
+	error: unknown
+): error is AuthError & { code?: string } {
+	return isAuthError(error);
+}
+
+/**
+ * User-friendly error messages for common Supabase auth error codes
+ * Note: refresh_token_not_found and refresh_token_already_used are expected
+ * during MFA flows and should not trigger these messages in that context.
+ */
+export const AUTH_ERROR_MESSAGES: Record<string, string> = {
+	refresh_token_not_found:
+		'Your session has expired. Please log in again with: task-master login',
+	refresh_token_already_used:
+		'Your session has expired (token was already used). Please log in again with: task-master login',
+	invalid_refresh_token:
+		'Your session has expired (invalid token). Please log in again with: task-master login',
+	session_expired:
+		'Your session has expired. Please log in again with: task-master login',
+	user_not_found:
+		'User account not found. Please log in again with: task-master login',
+	invalid_credentials:
+		'Invalid credentials. Please log in again with: task-master login'
+};
+
+/**
+ * Error codes caused by stale sessions that can be recovered from
+ * by clearing the session storage and retrying.
+ *
+ * These errors occur when there's a stale session with an invalid refresh token
+ * and Supabase tries to use it during authentication. The fix is to clear
+ * the stale session and retry the operation.
+ */
+export const RECOVERABLE_STALE_SESSION_ERRORS = [
+	'refresh_token_not_found',
+	'refresh_token_already_used'
+] as const;
+
+/**
+ * Check if an error is caused by a stale session and can be recovered
+ * by clearing the session storage and retrying.
+ */
+export function isRecoverableStaleSessionError(error: unknown): boolean {
+	if (!isSupabaseAuthError(error)) return false;
+	return RECOVERABLE_STALE_SESSION_ERRORS.includes(
+		(error.code || '') as (typeof RECOVERABLE_STALE_SESSION_ERRORS)[number]
+	);
+}
+
+/**
+ * Convert a Supabase auth error to a user-friendly AuthenticationError
+ */
+export function toAuthenticationError(
+	error: AuthError,
+	defaultMessage: string
+): AuthenticationError {
+	const code = error.code;
+	const userMessage = code
+		? AUTH_ERROR_MESSAGES[code] || `${defaultMessage}: ${error.message}`
+		: `${defaultMessage}: ${error.message}`;
+
+	// Map Supabase error codes to our AuthErrorCode
+	let authErrorCode:
+		| 'REFRESH_FAILED'
+		| 'NOT_AUTHENTICATED'
+		| 'INVALID_CREDENTIALS' = 'REFRESH_FAILED';
+	if (
+		code === 'refresh_token_not_found' ||
+		code === 'refresh_token_already_used' ||
+		code === 'invalid_refresh_token' ||
+		code === 'session_expired' ||
+		code === 'user_not_found'
+	) {
+		authErrorCode = 'NOT_AUTHENTICATED';
+	} else if (code === 'invalid_credentials') {
+		authErrorCode = 'INVALID_CREDENTIALS';
+	}
+
+	return new AuthenticationError(userMessage, authErrorCode, error);
+}
diff --git a/packages/tm-core/src/modules/auth/utils/index.ts b/packages/tm-core/src/modules/auth/utils/index.ts
@@ -0,0 +1,10 @@
+/**
+ * Auth utilities exports
+ */
+export {
+	isSupabaseAuthError,
+	AUTH_ERROR_MESSAGES,
+	RECOVERABLE_STALE_SESSION_ERRORS,
+	isRecoverableStaleSessionError,
+	toAuthenticationError
+} from './auth-error-utils.js';
diff --git a/packages/tm-core/src/modules/integration/clients/supabase-client.ts b/packages/tm-core/src/modules/integration/clients/supabase-client.ts
@@ -11,6 +11,11 @@ import {
 import { getLogger } from '../../../common/logger/index.js';
 import { SupabaseSessionStorage } from '../../auth/services/supabase-session-storage.js';
 import { AuthenticationError } from '../../auth/types.js';
+import {
+	isSupabaseAuthError,
+	isRecoverableStaleSessionError,
+	toAuthenticationError
+} from '../../auth/utils/index.js';
 
 export class SupabaseAuthClient {
 	private static instance: SupabaseAuthClient | null = null;
@@ -98,7 +103,10 @@ export class SupabaseAuthClient {
 			} = await client.auth.getSession();
 
 			if (error) {
-				this.logger.warn('Failed to restore session:', error);
+				// MFA-expected errors are normal during auth flows - don't log warnings
+				if (!isRecoverableStaleSessionError(error)) {
+					this.logger.warn('Failed to restore session:', error);
+				}
 				return null;
 			}
 
@@ -108,7 +116,14 @@ export class SupabaseAuthClient {
 
 			return session;
 		} catch (error) {
-			this.logger.error('Error initializing session:', error);
+			// MFA-expected errors (refresh_token_not_found, etc.) are normal during auth flows
+			if (isRecoverableStaleSessionError(error)) {
+				this.logger.debug('Session not available (expected during MFA flow)');
+			} else if (isSupabaseAuthError(error)) {
+				this.logger.warn('Session expired or invalid');
+			} else {
+				this.logger.error('Error initializing session:', error);
+			}
 			return null;
 		}
 	}
@@ -213,13 +228,23 @@ export class SupabaseAuthClient {
 			} = await client.auth.getSession();
 
 			if (error) {
-				this.logger.warn('Failed to get session:', error);
+				// MFA-expected errors are normal during auth flows - don't log warnings
+				if (!isRecoverableStaleSessionError(error)) {
+					this.logger.warn('Failed to get session:', error);
+				}
 				return null;
 			}
 
 			return session;
 		} catch (error) {
-			this.logger.error('Error getting session:', error);
+			// MFA-expected errors (refresh_token_not_found, etc.) are normal during auth flows
+			if (isRecoverableStaleSessionError(error)) {
+				this.logger.debug('Session not available (expected during MFA flow)');
+			} else if (isSupabaseAuthError(error)) {
+				this.logger.warn('Session expired or invalid');
+			} else {
+				this.logger.error('Error getting session:', error);
+			}
 			return null;
 		}
 	}
@@ -241,10 +266,8 @@ export class SupabaseAuthClient {
 
 			if (error) {
 				this.logger.error('Failed to refresh session:', error);
-				throw new AuthenticationError(
-					`Failed to refresh session: ${error.message}`,
-					'REFRESH_FAILED'
-				);
+				// Use user-friendly error message for known Supabase auth errors
+				throw toAuthenticationError(error, 'Failed to refresh session');
 			}
 
 			if (session) {
@@ -257,6 +280,11 @@ export class SupabaseAuthClient {
 				throw error;
 			}
 
+			// Handle raw Supabase auth errors that might be thrown
+			if (isSupabaseAuthError(error)) {
+				throw toAuthenticationError(error, 'Session refresh failed');
+			}
+
 			throw new AuthenticationError(
 				`Failed to refresh session: ${(error as Error).message}`,
 				'REFRESH_FAILED'
@@ -341,12 +369,36 @@ export class SupabaseAuthClient {
 		}
 	}
 
+	/**
+	 * Handle recoverable stale session errors by clearing storage and retrying.
+	 * Returns the result of the retry if applicable, or null if no retry was attempted.
+	 */
+	private async handleRecoverableError(
+		error: unknown,
+		isRetry: boolean,
+		retryFn: () => Promise<Session>
+	): Promise<Session | null> {
+		if (!isRetry && isRecoverableStaleSessionError(error)) {
+			this.logger.debug(
+				'MFA-expected error during token verification, clearing stale session and retrying'
+			);
+			await this.sessionStorage.clear();
+			return retryFn();
+		}
+		return null;
+	}
+
 	/**
 	 * Verify a one-time token and create a session
 	 * Used for CLI authentication with pre-generated tokens
+	 *
+	 * Note: If MFA is enabled and there's a stale session, Supabase might throw
+	 * refresh_token_not_found errors. We handle this by clearing the stale session
+	 * and retrying once.
 	 */
-	async verifyOneTimeCode(token: string): Promise<Session> {
+	async verifyOneTimeCode(token: string, isRetry = false): Promise<Session> {
 		const client = this.getClient();
+		const retryFn = () => this.verifyOneTimeCode(token, true);
 
 		try {
 			this.logger.info('Verifying authentication token...');
@@ -359,11 +411,18 @@ export class SupabaseAuthClient {
 			});
 
 			if (error) {
-				this.logger.error('Failed to verify token:', error);
-				throw new AuthenticationError(
-					`Failed to verify token: ${error.message}`,
-					'INVALID_CODE'
+				// If this is an MFA-expected error (like refresh_token_not_found),
+				// it might be due to a stale session interfering. Clear and retry once.
+				const retryResult = await this.handleRecoverableError(
+					error,
+					isRetry,
+					retryFn
 				);
+				if (retryResult) return retryResult;
+
+				this.logger.error('Failed to verify token:', error);
+				// Use user-friendly error message for known Supabase auth errors
+				throw toAuthenticationError(error, 'Failed to verify token');
 			}
 
 			if (!data?.session) {
@@ -380,6 +439,17 @@ export class SupabaseAuthClient {
 				throw error;
 			}
 
+			// Handle raw Supabase auth errors that might be thrown
+			if (isSupabaseAuthError(error)) {
+				const retryResult = await this.handleRecoverableError(
+					error,
+					isRetry,
+					retryFn
+				);
+				if (retryResult) return retryResult;
+				throw toAuthenticationError(error, 'Token verification failed');
+			}
+
 			throw new AuthenticationError(
 				`Token verification failed: ${(error as Error).message}`,
 				'CODE_AUTH_FAILED'

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +"task-master-ai": patch
 +---
++
 +Improve auth-related error handling