fix(frontend): respect hasCompletedOnboarding from ~/.claude.json (#1537)

* fix(frontend): respect hasCompletedOnboarding from ~/.claude.json (ACS-395) Users who have completed onboarding in Claude Code (hasCompletedOnboarding: true in ~/.claude.json) were forced to go through Auto-Claude's onboarding wizard again because Auto-Claude didn't read this field during settings migration. Root cause: The migrateOnboardingCompleted() function in settings-store.ts only checked globalClaudeOAuthToken and autoBuildPath, not ~/.claude.json. Solution: Added IPC handler to read ~/.claude.json and check hasCompletedOnboarding field. The migration now respects Claude Code's onboarding status before falling back to existing user detection logic. Changes: - Added SETTINGS_CLAUDE_CODE_GET_ONBOARDING_STATUS IPC channel - Added IPC handler to read ~/.claude.json and return hasCompletedOnboarding - Updated migrateOnboardingCompleted() to be async and call new handler - Added browser mock for new API method - Added comprehensive tests (7 test cases covering edge cases) Test cases: - File doesn't exist → returns false - hasCompletedOnboarding: true → returns true - hasCompletedOnboarding: false → returns false - Field missing → returns false - Malformed JSON → returns false (error handling) - Other Claude Code fields present → works correctly - Read errors → handled gracefully * test: improve error handling test to exercise actual error path Previously the error handling test only tested the "file doesn't exist" path (existsSync returns false), not the actual read/parse error path. This change overrides both existsSync and readFileSync mocks to: - Make the file appear to exist (existsSync returns true) - Throw a permission error when attempting to read (readFileSync throws) This ensures the catch block in the IPC handler is actually tested. Fixes feedback from CodeRabbit AI review. * test: remove dead cleanup code and inherit real IPC constants Fixes issues from CodeRabbitAI and Auto Claude PR Review: 1. Remove unused fs imports (unlinkSync, existsSync) - these are only used in dead cleanup code that doesn't work because fs is mocked. 2. Remove dead cleanup code in beforeEach and afterEach - the existsSync and unlinkSync calls are ineffective since fs is mocked and mockFiles.clear() already handles cleanup. 3. Inherit real IPC_CHANNELS constants via vi.importActual instead of hardcoding values. This ensures the mock stays in sync with the source of truth (shared/constants/ipc.ts) and prevents silent test failures if channel names are changed. --------- Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com> Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
2026-01-27 11:01:03 +02:00
parent 94d941333b
commit 1860c2c432
7 changed files with 356 additions and 2 deletions
@@ -0,0 +1,279 @@
+/**
+ * Tests for settings onboarding migration logic
+ *
+ * Tests the SETTINGS_CLAUDE_CODE_GET_ONBOARDING_STATUS handler which
+ * reads ~/.claude.json to check if Claude Code onboarding is complete.
+ */
+
+import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest';
+import { join } from 'path';
+import { tmpdir } from 'os';
+
+// Store registered IPC handlers so we can call them directly
+type IpcHandler = (event: unknown, ...args: unknown[]) => Promise<unknown>;
+const registeredHandlers: Map<string, IpcHandler> = new Map();
+
+// Mock electron app
+const mockHomeDir = tmpdir();
+vi.mock('electron', () => ({
+  ipcMain: {
+    handle: vi.fn((channel: string, handler: IpcHandler) => {
+      registeredHandlers.set(channel, handler);
+    }),
+  },
+  app: {
+    getPath: vi.fn((_pathName: string) => mockHomeDir),
+    getAppPath: vi.fn(() => mockHomeDir),
+  },
+  dialog: {
+    showOpenDialog: vi.fn(),
+  },
+  shell: {
+    openExternal: vi.fn(),
+  },
+}));
+
+// Mock @electron-toolkit/utils
+vi.mock('@electron-toolkit/utils', () => ({
+  is: {
+    dev: true,
+  },
+}));
+
+// Mock fs
+const mockFiles: Map<string, string> = new Map();
+vi.mock('fs', () => ({
+  existsSync: vi.fn((path: string) => mockFiles.has(path)),
+  readFileSync: vi.fn((path: string) => {
+    const content = mockFiles.get(path);
+    if (content === undefined) {
+      const error = new Error(`ENOENT: no such file or directory, open '${path}'`) as NodeJS.ErrnoException;
+      error.code = 'ENOENT';
+      throw error;
+    }
+    return content;
+  }),
+  writeFileSync: vi.fn(),
+  mkdirSync: vi.fn(),
+  statSync: vi.fn(),
+}));
+
+// Mock node:child_process
+vi.mock('node:child_process', () => ({
+  execFile: vi.fn(),
+  execFileSync: vi.fn(),
+}));
+
+// Mock other dependencies - inherit real constants to keep them in sync
+vi.mock('../../shared/constants', async () => {
+  const actual = await vi.importActual<typeof import('../../shared/constants')>('../../shared/constants');
+  return {
+    ...actual,
+  };
+});
+
+vi.mock('../cli-tool-manager', () => ({
+  configureTools: vi.fn(),
+  getToolInfo: vi.fn(),
+  isPathFromWrongPlatform: vi.fn(() => false),
+  preWarmToolCache: vi.fn(),
+}));
+
+vi.mock('../settings-utils', () => ({
+  readSettingsFile: vi.fn(() => ({})),
+  writeSettingsFile: vi.fn(),
+  getSettingsPath: vi.fn(() => join(mockHomeDir, 'settings.json')),
+}));
+
+vi.mock('../agent', () => ({
+  AgentManager: vi.fn(),
+}));
+
+vi.mock('./utils', () => ({
+  parseEnvFile: vi.fn(() => ({})),
+}));
+
+vi.mock('../app-updater', () => ({
+  setUpdateChannel: vi.fn(),
+  setUpdateChannelWithDowngradeCheck: vi.fn(() => Promise.resolve()),
+}));
+
+import { IPC_CHANNELS } from '../../shared/constants';
+
+describe('SETTINGS_CLAUDE_CODE_GET_ONBOARDING_STATUS handler', () => {
+  let onboardingStatusHandler: IpcHandler;
+  const claudeJsonPath = join(mockHomeDir, '.claude.json');
+
+  beforeEach(async () => {
+    vi.clearAllMocks();
+    registeredHandlers.clear();
+    mockFiles.clear();
+
+    // Reset module cache to get fresh state
+    vi.resetModules();
+
+    // Re-import to re-register handlers
+    const { registerSettingsHandlers } = await import('../ipc-handlers/settings-handlers');
+    // Mock agentManager and getMainWindow
+    const mockAgentManager = {};
+    const mockGetMainWindow = vi.fn(() => null);
+    registerSettingsHandlers(mockAgentManager as never, mockGetMainWindow);
+
+    // Get the handler
+    const handler = registeredHandlers.get(IPC_CHANNELS.SETTINGS_CLAUDE_CODE_GET_ONBOARDING_STATUS);
+    if (!handler) {
+      throw new Error('SETTINGS_CLAUDE_CODE_GET_ONBOARDING_STATUS handler not registered');
+    }
+    onboardingStatusHandler = handler;
+  });
+
+  afterEach(() => {
+    // Cleanup handled via mockFiles.clear()
+    mockFiles.clear();
+  });
+
+  describe('when ~/.claude.json does not exist', () => {
+    test('should return hasCompletedOnboarding: false', async () => {
+      const result = await onboardingStatusHandler({}, null) as {
+        success: boolean;
+        data?: { hasCompletedOnboarding: boolean };
+        error?: string;
+      };
+
+      expect(result.success).toBe(true);
+      expect(result.data?.hasCompletedOnboarding).toBe(false);
+    });
+  });
+
+  describe('when ~/.claude.json exists', () => {
+    describe('with hasCompletedOnboarding: true', () => {
+      beforeEach(() => {
+        const content = JSON.stringify({ hasCompletedOnboarding: true });
+        mockFiles.set(claudeJsonPath, content);
+      });
+
+      test('should return hasCompletedOnboarding: true', async () => {
+        const result = await onboardingStatusHandler({}, null) as {
+          success: boolean;
+          data?: { hasCompletedOnboarding: boolean };
+        };
+
+        expect(result.success).toBe(true);
+        expect(result.data?.hasCompletedOnboarding).toBe(true);
+      });
+    });
+
+    describe('with hasCompletedOnboarding: false', () => {
+      beforeEach(() => {
+        const content = JSON.stringify({ hasCompletedOnboarding: false });
+        mockFiles.set(claudeJsonPath, content);
+      });
+
+      test('should return hasCompletedOnboarding: false', async () => {
+        const result = await onboardingStatusHandler({}, null) as {
+          success: boolean;
+          data?: { hasCompletedOnboarding: boolean };
+        };
+
+        expect(result.success).toBe(true);
+        expect(result.data?.hasCompletedOnboarding).toBe(false);
+      });
+    });
+
+    describe('without hasCompletedOnboarding field', () => {
+      beforeEach(() => {
+        const content = JSON.stringify({ someOtherField: 'value' });
+        mockFiles.set(claudeJsonPath, content);
+      });
+
+      test('should return hasCompletedOnboarding: false', async () => {
+        const result = await onboardingStatusHandler({}, null) as {
+          success: boolean;
+          data?: { hasCompletedOnboarding: boolean };
+        };
+
+        expect(result.success).toBe(true);
+        expect(result.data?.hasCompletedOnboarding).toBe(false);
+      });
+    });
+
+    describe('with malformed JSON', () => {
+      beforeEach(() => {
+        mockFiles.set(claudeJsonPath, '{ invalid json }');
+      });
+
+      test('should return hasCompletedOnboarding: false (error handling)', async () => {
+        const result = await onboardingStatusHandler({}, null) as {
+          success: boolean;
+          data?: { hasCompletedOnboarding: boolean };
+        };
+
+        expect(result.success).toBe(true);
+        expect(result.data?.hasCompletedOnboarding).toBe(false);
+      });
+    });
+
+    describe('with other Claude Code fields present', () => {
+      beforeEach(() => {
+        const content = JSON.stringify({
+          hasCompletedOnboarding: true,
+          oauthAccount: {
+            emailAddress: 'user@example.com',
+            accessToken: 'dummy-token',
+          },
+          lastChecked: '2024-01-15T10:30:00Z',
+        });
+        mockFiles.set(claudeJsonPath, content);
+      });
+
+      test('should return hasCompletedOnboarding: true', async () => {
+        const result = await onboardingStatusHandler({}, null) as {
+          success: boolean;
+          data?: { hasCompletedOnboarding: boolean };
+        };
+
+        expect(result.success).toBe(true);
+        expect(result.data?.hasCompletedOnboarding).toBe(true);
+      });
+    });
+  });
+
+  describe('error handling', () => {
+    test('should handle read errors gracefully', async () => {
+      // Get the mocked functions (vitest stores the implementation)
+      const { existsSync, readFileSync } = await import('fs');
+
+      // Save original mock implementations to restore after test
+      const originalExistsSync = (existsSync as any).getMockImplementation();
+      const originalReadFileSync = (readFileSync as any).getMockImplementation();
+
+      // Override existsSync to make file appear to exist
+      (existsSync as any).mockImplementation((path: string) => {
+        if (path === claudeJsonPath) {
+          return true; // File appears to exist
+        }
+        return originalExistsSync ? originalExistsSync(path) : false;
+      });
+
+      // Override readFileSync to throw error for our specific file
+      (readFileSync as any).mockImplementation((path: string) => {
+        if (path === claudeJsonPath) {
+          throw new Error('EACCES: permission denied, open \'' + path + '\'');
+        }
+        return originalReadFileSync ? originalReadFileSync(path) : '';
+      });
+
+      const result = await onboardingStatusHandler({}, null) as {
+        success: boolean;
+        data?: { hasCompletedOnboarding: boolean };
+      };
+
+      expect(result.success).toBe(true);
+      expect(result.data?.hasCompletedOnboarding).toBe(false);
+
+      // Restore original mocks
+      (existsSync as any).mockImplementation(originalExistsSync);
+      (readFileSync as any).mockImplementation(originalReadFileSync);
+    });
+  });
+});
@@ -280,6 +280,48 @@ export function registerSettingsHandlers(
    }
  );

+  /**
+   * Read ~/.claude.json to check if Claude Code onboarding is complete.
+   * This allows Auto-Claude to respect Claude Code's onboarding status and
+   * avoid showing the onboarding wizard to users who have already completed it.
+   */
+  ipcMain.handle(
+    IPC_CHANNELS.SETTINGS_CLAUDE_CODE_GET_ONBOARDING_STATUS,
+    async (): Promise<IPCResult<{ hasCompletedOnboarding: boolean }>> => {
+      try {
+        const homeDir = app.getPath('home');
+        const claudeJsonPath = path.join(homeDir, '.claude.json');
+
+        // If file doesn't exist, user hasn't completed Claude Code onboarding
+        if (!existsSync(claudeJsonPath)) {
+          return {
+            success: true,
+            data: { hasCompletedOnboarding: false }
+          };
+        }
+
+        const content = readFileSync(claudeJsonPath, 'utf-8');
+        const claudeConfig = JSON.parse(content);
+
+        // Check for hasCompletedOnboarding field
+        const hasCompletedOnboarding = claudeConfig.hasCompletedOnboarding === true;
+
+        return {
+          success: true,
+          data: { hasCompletedOnboarding }
+        };
+      } catch (error) {
+        // On error (parse error, read error, etc.), log and return false
+        // This ensures we don't block onboarding due to corrupted .claude.json
+        console.warn('[SETTINGS_CLAUDE_CODE_GET_ONBOARDING_STATUS] Error reading ~/.claude.json:', error);
+        return {
+          success: true,
+          data: { hasCompletedOnboarding: false }
+        };
+      }
+    }
+  );
+
  // ============================================
  // Dialog Operations
  // ============================================
@@ -21,6 +21,9 @@ export interface SettingsAPI {
    claude: ToolDetectionResult;
  }>>;

+  // Claude Code onboarding status
+  getClaudeCodeOnboardingStatus: () => Promise<IPCResult<{ hasCompletedOnboarding: boolean }>>;
+
  // App Info
  getAppVersion: () => Promise<string>;

@@ -52,6 +55,10 @@ export const createSettingsAPI = (): SettingsAPI => ({
  }>> =>
    ipcRenderer.invoke(IPC_CHANNELS.SETTINGS_GET_CLI_TOOLS_INFO),

+  // Claude Code onboarding status
+  getClaudeCodeOnboardingStatus: (): Promise<IPCResult<{ hasCompletedOnboarding: boolean }>> =>
+    ipcRenderer.invoke(IPC_CHANNELS.SETTINGS_CLAUDE_CODE_GET_ONBOARDING_STATUS),
+
  // App Info
  getAppVersion: (): Promise<string> =>
    ipcRenderer.invoke(IPC_CHANNELS.APP_VERSION),
@@ -30,6 +30,12 @@ export const settingsMock = {
    }
  }),

+  // Claude Code onboarding status (mock - always returns false in browser mode)
+  getClaudeCodeOnboardingStatus: async () => ({
+    success: true,
+    data: { hasCompletedOnboarding: false }
+  }),
+
  // App Info
  getAppVersion: async () => '0.1.0-browser',

@@ -299,13 +299,29 @@ export const useSettingsStore = create<SettingsState>((set) => ({
 * Check if settings need migration for onboardingCompleted flag.
 * Existing users (with tokens or projects configured) should have
 * onboardingCompleted set to true to skip the onboarding wizard.
+ *
+ * This function now also checks Claude Code's ~/.claude.json for
+ * hasCompletedOnboarding to respect Claude Code's onboarding status.
 */
-function migrateOnboardingCompleted(settings: AppSettings): AppSettings {
+async function migrateOnboardingCompleted(settings: AppSettings): Promise<AppSettings> {
  // Only migrate if onboardingCompleted is undefined (not explicitly set)
  if (settings.onboardingCompleted !== undefined) {
    return settings;
  }

+  // NEW: Check ~/.claude.json for hasCompletedOnboarding
+  // This allows Auto-Claude to respect Claude Code's onboarding status
+  try {
+    const claudeCodeResult = await window.electronAPI.getClaudeCodeOnboardingStatus();
+    if (claudeCodeResult.success && claudeCodeResult.data?.hasCompletedOnboarding) {
+      // Claude Code says onboarding is complete, respect that
+      return { ...settings, onboardingCompleted: true };
+    }
+  } catch (error) {
+    // If checking Claude Code onboarding fails, log and continue with existing logic
+    console.warn('[settings-store] Failed to check Claude Code onboarding status:', error);
+  }
+
  // Check for signs of an existing user:
  // - Has a Claude OAuth token configured
  // - Has the auto-build source path configured
@@ -334,7 +350,8 @@ export async function loadSettings(): Promise<void> {
    const result = await window.electronAPI.getSettings();
    if (result.success && result.data) {
      // Apply migration for onboardingCompleted flag
-      const migratedSettings = migrateOnboardingCompleted(result.data);
+      // This is now async since it needs to read ~/.claude.json
+      const migratedSettings = await migrateOnboardingCompleted(result.data);
      store.setSettings(migratedSettings);

      // If migration changed the settings, persist them
@@ -142,6 +142,7 @@ export const IPC_CHANNELS = {
  SETTINGS_GET: 'settings:get',
  SETTINGS_SAVE: 'settings:save',
  SETTINGS_GET_CLI_TOOLS_INFO: 'settings:getCliToolsInfo',
+  SETTINGS_CLAUDE_CODE_GET_ONBOARDING_STATUS: 'settings:claudeCode:getOnboardingStatus',  // Check hasCompletedOnboarding from ~/.claude.json

  // API Profile management (custom Anthropic-compatible endpoints)
  PROFILES_GET: 'profiles:get',
@@ -347,6 +347,8 @@ export interface ElectronAPI {
    gh: import('./cli').ToolDetectionResult;
    claude: import('./cli').ToolDetectionResult;
  }>>;
+  /** Check if Claude Code onboarding is complete (reads ~/.claude.json) */
+  getClaudeCodeOnboardingStatus: () => Promise<IPCResult<{ hasCompletedOnboarding: boolean }>>;

  // API Profile management (custom Anthropic-compatible endpoints)
  getAPIProfiles: () => Promise<IPCResult<ProfilesFile>>;