fix(auth): await profile manager initialization before auth check (#1010)

* fix(auth): await profile manager initialization before auth check Fixes race condition where hasValidAuth() was called before the ClaudeProfileManager finished async initialization from disk. The getClaudeProfileManager() returns a singleton immediately with default profile data (no OAuth token). When hasValidAuth() runs before initialization completes, it returns false even when valid credentials exist. Changed all pre-flight auth checks to use await initializeClaudeProfileManager() which ensures initialization completes via promise caching. Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com> * fix(auth): add error handling for profile manager initialization Prevents unhandled promise rejections when initializeClaudeProfileManager() throws due to filesystem errors (permissions, disk full, corrupt JSON). The ipcMain.on handler for TASK_START doesn't await promises, so unhandled rejections could crash the main process. Wrapped all await initializeClaudeProfileManager() calls in try-catch blocks. Found via automated code review. Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com> * test: mock initializeClaudeProfileManager in subprocess tests The test mock was only mocking getClaudeProfileManager, but now we also use initializeClaudeProfileManager which wasn't mocked, causing test failures. Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com> * fix(auth): add try-catch for initializeClaudeProfileManager in remaining handlers Addresses PR review feedback - TASK_UPDATE_STATUS and TASK_RECOVER_STUCK handlers were missing try-catch blocks for initializeClaudeProfileManager(), inconsistent with TASK_START handler. If initialization fails, users now get specific file permissions guidance instead of generic error messages. Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com> * refactor(auth): extract profile manager initialization into helper Extract the repeated initializeClaudeProfileManager() + try/catch pattern into a helper function ensureProfileManagerInitialized() that returns a discriminated union for type-safe error handling. This reduces code duplication across TASK_START, TASK_UPDATE_STATUS, and TASK_RECOVER_STUCK handlers while preserving context-specific error handling behavior. The helper returns: - { success: true, profileManager } on success - { success: false, error } on failure Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com> * fix(auth): improve error details and allow retry after transient failures Two improvements to profile manager initialization: 1. Include actual error details in failure response for better debugging. Previously, only a generic message was returned to users, making it hard to diagnose the root cause. Now the error message is appended. 2. Reset cached promise on failure to allow retries after transient errors. Previously, if initialize() failed (e.g., EACCES, ENOSPC), the rejected promise was cached forever, requiring app restart to recover. Now the cached promise is reset on failure, allowing subsequent calls to retry. Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com> --------- Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com> Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com> Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
2026-01-13 22:47:57 +02:00
parent 88277f843f
commit c8374bc104
4 changed files with 105 additions and 15 deletions
@@ -39,11 +39,14 @@ vi.mock('child_process', async (importOriginal) => {
 });

 // Mock claude-profile-manager to bypass auth checks in tests
+const mockProfileManager = {
+  hasValidAuth: () => true,
+  getActiveProfile: () => ({ profileId: 'default', profileName: 'Default' })
+};
+
 vi.mock('../../main/claude-profile-manager', () => ({
-  getClaudeProfileManager: () => ({
-    hasValidAuth: () => true,
-    getActiveProfile: () => ({ profileId: 'default', profileName: 'Default' })
-  })
+  getClaudeProfileManager: () => mockProfileManager,
+  initializeClaudeProfileManager: () => Promise.resolve(mockProfileManager)
 }));

 // Mock validatePythonPath to allow test paths (security validation is tested separately)
@@ -5,7 +5,7 @@ import { AgentState } from './agent-state';
 import { AgentEvents } from './agent-events';
 import { AgentProcessManager } from './agent-process';
 import { AgentQueueManager } from './agent-queue';
-import { getClaudeProfileManager } from '../claude-profile-manager';
+import { getClaudeProfileManager, initializeClaudeProfileManager } from '../claude-profile-manager';
 import {
  SpecCreationMetadata,
  TaskExecutionOptions,
@@ -96,7 +96,15 @@ export class AgentManager extends EventEmitter {
    baseBranch?: string
  ): Promise<void> {
    // Pre-flight auth check: Verify active profile has valid authentication
-    const profileManager = getClaudeProfileManager();
+    // Ensure profile manager is initialized to prevent race condition
+    let profileManager;
+    try {
+      profileManager = await initializeClaudeProfileManager();
+    } catch (error) {
+      console.error('[AgentManager] Failed to initialize profile manager:', error);
+      this.emit('error', taskId, 'Failed to initialize profile manager. Please check file permissions and disk space.');
+      return;
+    }
    if (!profileManager.hasValidAuth()) {
      this.emit('error', taskId, 'Claude authentication required. Please authenticate in Settings > Claude Profiles before starting tasks.');
      return;
@@ -174,7 +182,15 @@ export class AgentManager extends EventEmitter {
    options: TaskExecutionOptions = {}
  ): Promise<void> {
    // Pre-flight auth check: Verify active profile has valid authentication
-    const profileManager = getClaudeProfileManager();
+    // Ensure profile manager is initialized to prevent race condition
+    let profileManager;
+    try {
+      profileManager = await initializeClaudeProfileManager();
+    } catch (error) {
+      console.error('[AgentManager] Failed to initialize profile manager:', error);
+      this.emit('error', taskId, 'Failed to initialize profile manager. Please check file permissions and disk space.');
+      return;
+    }
    if (!profileManager.hasValidAuth()) {
      this.emit('error', taskId, 'Claude authentication required. Please authenticate in Settings > Claude Profiles before starting tasks.');
      return;
@@ -568,6 +568,7 @@ export function getClaudeProfileManager(): ClaudeProfileManager {
 * Initialize and get the singleton Claude profile manager instance (async)
 * This ensures the profile manager is fully initialized before use.
 * Uses promise caching to prevent concurrent initialization.
+ * The cached promise is reset on failure to allow retries after transient errors.
 */
 export async function initializeClaudeProfileManager(): Promise<ClaudeProfileManager> {
  if (!profileManager) {
@@ -581,9 +582,16 @@ export async function initializeClaudeProfileManager(): Promise<ClaudeProfileMan

  // If initialization is in progress, wait for it (promise caching)
  if (!initPromise) {
-    initPromise = profileManager.initialize().then(() => {
-      return profileManager!;
-    });
+    initPromise = profileManager.initialize()
+      .then(() => {
+        return profileManager!;
+      })
+      .catch((error) => {
+        // Reset cached promise on failure so retries can succeed
+        // This allows recovery from transient errors (e.g., disk full, permission issues)
+        initPromise = null;
+        throw error;
+      });
  }

  return initPromise;
@@ -9,7 +9,7 @@ import { AgentManager } from '../../agent';
 import { fileWatcher } from '../../file-watcher';
 import { findTaskAndProject } from './shared';
 import { checkGitStatus } from '../../project-initializer';
-import { getClaudeProfileManager } from '../../claude-profile-manager';
+import { initializeClaudeProfileManager, type ClaudeProfileManager } from '../../claude-profile-manager';
 import {
  getPlanPath,
  persistPlanStatus,
@@ -74,6 +74,30 @@ function checkSubtasksCompletion(plan: Record<string, unknown> | null): {
  return { allSubtasks, completedCount, totalCount, allCompleted };
 }

+/**
+ * Helper function to ensure profile manager is initialized.
+ * Returns a discriminated union for type-safe error handling.
+ *
+ * @returns Success with profile manager, or failure with error message
+ */
+async function ensureProfileManagerInitialized(): Promise<
+  | { success: true; profileManager: ClaudeProfileManager }
+  | { success: false; error: string }
+> {
+  try {
+    const profileManager = await initializeClaudeProfileManager();
+    return { success: true, profileManager };
+  } catch (error) {
+    console.error('[ensureProfileManagerInitialized] Failed to initialize:', error);
+    // Include actual error details for debugging while providing actionable guidance
+    const errorMessage = error instanceof Error ? error.message : String(error);
+    return {
+      success: false,
+      error: `Failed to initialize profile manager. Please check file permissions and disk space. (${errorMessage})`
+    };
+  }
+}
+
 /**
 * Register task execution handlers (start, stop, review, status management, recovery)
 */
@@ -86,7 +110,7 @@ export function registerTaskExecutionHandlers(
   */
  ipcMain.on(
    IPC_CHANNELS.TASK_START,
-    (_, taskId: string, _options?: TaskStartOptions) => {
+    async (_, taskId: string, _options?: TaskStartOptions) => {
      console.warn('[TASK_START] Received request for taskId:', taskId);
      const mainWindow = getMainWindow();
      if (!mainWindow) {
@@ -94,6 +118,19 @@ export function registerTaskExecutionHandlers(
        return;
      }

+      // Ensure profile manager is initialized before checking auth
+      // This prevents race condition where auth check runs before profile data loads from disk
+      const initResult = await ensureProfileManagerInitialized();
+      if (!initResult.success) {
+        mainWindow.webContents.send(
+          IPC_CHANNELS.TASK_ERROR,
+          taskId,
+          initResult.error
+        );
+        return;
+      }
+      const profileManager = initResult.profileManager;
+
      // Find task and project
      const { task, project } = findTaskAndProject(taskId);

@@ -129,7 +166,6 @@ export function registerTaskExecutionHandlers(
      }

      // Check authentication - Claude requires valid auth to run tasks
-      const profileManager = getClaudeProfileManager();
      if (!profileManager.hasValidAuth()) {
        console.warn('[TASK_START] No valid authentication for active profile');
        mainWindow.webContents.send(
@@ -665,7 +701,19 @@ export function registerTaskExecutionHandlers(
          }

          // Check authentication before auto-starting
-          const profileManager = getClaudeProfileManager();
+          // Ensure profile manager is initialized to prevent race condition
+          const initResult = await ensureProfileManagerInitialized();
+          if (!initResult.success) {
+            if (mainWindow) {
+              mainWindow.webContents.send(
+                IPC_CHANNELS.TASK_ERROR,
+                taskId,
+                initResult.error
+              );
+            }
+            return { success: false, error: initResult.error };
+          }
+          const profileManager = initResult.profileManager;
          if (!profileManager.hasValidAuth()) {
            console.warn('[TASK_UPDATE_STATUS] No valid authentication for active profile');
            if (mainWindow) {
@@ -996,7 +1044,22 @@ export function registerTaskExecutionHandlers(
          }

          // Check authentication before auto-restarting
-          const profileManager = getClaudeProfileManager();
+          // Ensure profile manager is initialized to prevent race condition
+          const initResult = await ensureProfileManagerInitialized();
+          if (!initResult.success) {
+            // Recovery succeeded but we can't restart without profile manager
+            return {
+              success: true,
+              data: {
+                taskId,
+                recovered: true,
+                newStatus,
+                message: `Task recovered but cannot restart: ${initResult.error}`,
+                autoRestarted: false
+              }
+            };
+          }
+          const profileManager = initResult.profileManager;
          if (!profileManager.hasValidAuth()) {
            console.warn('[Recovery] Auth check failed, cannot auto-restart task');
            // Recovery succeeded but we can't restart without auth