diff --git a/apps/server/src/lib/codex-auth.ts b/apps/server/src/lib/codex-auth.ts new file mode 100644 index 00000000..965885bc --- /dev/null +++ b/apps/server/src/lib/codex-auth.ts @@ -0,0 +1,98 @@ +/** + * Shared utility for checking Codex CLI authentication status + * + * Uses 'codex login status' command to verify authentication. + * Never assumes authenticated - only returns true if CLI confirms. + */ + +import { spawnProcess, getCodexAuthPath } from '@automaker/platform'; +import { findCodexCliPath } from '@automaker/platform'; +import * as fs from 'fs'; + +const CODEX_COMMAND = 'codex'; +const OPENAI_API_KEY_ENV = 'OPENAI_API_KEY'; + +export interface CodexAuthCheckResult { + authenticated: boolean; + method: 'api_key_env' | 'cli_authenticated' | 'none'; +} + +/** + * Check Codex authentication status using 'codex login status' command + * + * @param cliPath Optional CLI path. If not provided, will attempt to find it. + * @returns Authentication status and method + */ +export async function checkCodexAuthentication( + cliPath?: string | null +): Promise { + console.log('[CodexAuth] checkCodexAuthentication called with cliPath:', cliPath); + + const resolvedCliPath = cliPath || (await findCodexCliPath()); + const hasApiKey = !!process.env[OPENAI_API_KEY_ENV]; + + console.log('[CodexAuth] resolvedCliPath:', resolvedCliPath); + console.log('[CodexAuth] hasApiKey:', hasApiKey); + + // Debug: Check auth file + const authFilePath = getCodexAuthPath(); + console.log('[CodexAuth] Auth file path:', authFilePath); + try { + const authFileExists = fs.existsSync(authFilePath); + console.log('[CodexAuth] Auth file exists:', authFileExists); + if (authFileExists) { + const authContent = fs.readFileSync(authFilePath, 'utf-8'); + console.log('[CodexAuth] Auth file content:', authContent.substring(0, 500)); // First 500 chars + } + } catch (error) { + console.log('[CodexAuth] Error reading auth file:', error); + } + + // If CLI is not installed, cannot be authenticated + if (!resolvedCliPath) { + console.log('[CodexAuth] No CLI path found, returning not authenticated'); + return { authenticated: false, method: 'none' }; + } + + try { + console.log('[CodexAuth] Running: ' + resolvedCliPath + ' login status'); + const result = await spawnProcess({ + command: resolvedCliPath || CODEX_COMMAND, + args: ['login', 'status'], + cwd: process.cwd(), + env: { + ...process.env, + TERM: 'dumb', // Avoid interactive output + }, + }); + + console.log('[CodexAuth] Command result:'); + console.log('[CodexAuth] exitCode:', result.exitCode); + console.log('[CodexAuth] stdout:', JSON.stringify(result.stdout)); + console.log('[CodexAuth] stderr:', JSON.stringify(result.stderr)); + + // Check both stdout and stderr for "logged in" - Codex CLI outputs to stderr + const combinedOutput = (result.stdout + result.stderr).toLowerCase(); + const isLoggedIn = combinedOutput.includes('logged in'); + console.log('[CodexAuth] isLoggedIn (contains "logged in" in stdout or stderr):', isLoggedIn); + + if (result.exitCode === 0 && isLoggedIn) { + // Determine auth method based on what we know + const method = hasApiKey ? 'api_key_env' : 'cli_authenticated'; + console.log('[CodexAuth] Authenticated! method:', method); + return { authenticated: true, method }; + } + + console.log( + '[CodexAuth] Not authenticated. exitCode:', + result.exitCode, + 'isLoggedIn:', + isLoggedIn + ); + } catch (error) { + console.log('[CodexAuth] Error running command:', error); + } + + console.log('[CodexAuth] Returning not authenticated'); + return { authenticated: false, method: 'none' }; +} diff --git a/apps/server/src/providers/codex-provider.ts b/apps/server/src/providers/codex-provider.ts index f4a071d0..dffc850f 100644 --- a/apps/server/src/providers/codex-provider.ts +++ b/apps/server/src/providers/codex-provider.ts @@ -15,6 +15,7 @@ import { getDataDirectory, getCodexConfigDir, } from '@automaker/platform'; +import { checkCodexAuthentication } from '../lib/codex-auth.js'; import { formatHistoryAsText, extractTextFromContent, @@ -963,11 +964,21 @@ export class CodexProvider extends BaseProvider { } async detectInstallation(): Promise { + console.log('[CodexProvider.detectInstallation] Starting...'); + const cliPath = await findCodexCliPath(); const hasApiKey = !!process.env[OPENAI_API_KEY_ENV]; const authIndicators = await getCodexAuthIndicators(); const installed = !!cliPath; + console.log('[CodexProvider.detectInstallation] cliPath:', cliPath); + console.log('[CodexProvider.detectInstallation] hasApiKey:', hasApiKey); + console.log( + '[CodexProvider.detectInstallation] authIndicators:', + JSON.stringify(authIndicators) + ); + console.log('[CodexProvider.detectInstallation] installed:', installed); + let version = ''; if (installed) { try { @@ -977,19 +988,29 @@ export class CodexProvider extends BaseProvider { cwd: process.cwd(), }); version = result.stdout.trim(); - } catch { + console.log('[CodexProvider.detectInstallation] version:', version); + } catch (error) { + console.log('[CodexProvider.detectInstallation] Error getting version:', error); version = ''; } } - return { + // Determine auth status - always verify with CLI, never assume authenticated + console.log('[CodexProvider.detectInstallation] Calling checkCodexAuthentication...'); + const authCheck = await checkCodexAuthentication(cliPath); + console.log('[CodexProvider.detectInstallation] authCheck result:', JSON.stringify(authCheck)); + const authenticated = authCheck.authenticated; + + const result = { installed, path: cliPath || undefined, version: version || undefined, - method: 'cli', + method: 'cli' as const, // Installation method hasApiKey, - authenticated: authIndicators.hasOAuthToken || authIndicators.hasApiKey || hasApiKey, + authenticated, }; + console.log('[CodexProvider.detectInstallation] Final result:', JSON.stringify(result)); + return result; } getAvailableModels(): ModelDefinition[] { @@ -1001,94 +1022,68 @@ export class CodexProvider extends BaseProvider { * Check authentication status for Codex CLI */ async checkAuth(): Promise { + console.log('[CodexProvider.checkAuth] Starting auth check...'); + const cliPath = await findCodexCliPath(); const hasApiKey = !!process.env[OPENAI_API_KEY_ENV]; const authIndicators = await getCodexAuthIndicators(); + console.log('[CodexProvider.checkAuth] cliPath:', cliPath); + console.log('[CodexProvider.checkAuth] hasApiKey:', hasApiKey); + console.log('[CodexProvider.checkAuth] authIndicators:', JSON.stringify(authIndicators)); + // Check for API key in environment if (hasApiKey) { + console.log('[CodexProvider.checkAuth] Has API key, returning authenticated'); return { authenticated: true, method: 'api_key' }; } // Check for OAuth/token from Codex CLI if (authIndicators.hasOAuthToken || authIndicators.hasApiKey) { + console.log( + '[CodexProvider.checkAuth] Has OAuth token or API key in auth file, returning authenticated' + ); return { authenticated: true, method: 'oauth' }; } - // CLI is installed but not authenticated + // CLI is installed but not authenticated via indicators - try CLI command + console.log('[CodexProvider.checkAuth] No indicators found, trying CLI command...'); if (cliPath) { try { + // Try 'codex login status' first (same as checkCodexAuthentication) + console.log('[CodexProvider.checkAuth] Running: ' + cliPath + ' login status'); const result = await spawnProcess({ command: cliPath || CODEX_COMMAND, - args: ['auth', 'status', '--json'], + args: ['login', 'status'], cwd: process.cwd(), + env: { + ...process.env, + TERM: 'dumb', + }, }); - // If auth command succeeds, we're authenticated - if (result.exitCode === 0) { + console.log('[CodexProvider.checkAuth] login status result:'); + console.log('[CodexProvider.checkAuth] exitCode:', result.exitCode); + console.log('[CodexProvider.checkAuth] stdout:', JSON.stringify(result.stdout)); + console.log('[CodexProvider.checkAuth] stderr:', JSON.stringify(result.stderr)); + + // Check both stdout and stderr - Codex CLI outputs to stderr + const combinedOutput = (result.stdout + result.stderr).toLowerCase(); + const isLoggedIn = combinedOutput.includes('logged in'); + console.log('[CodexProvider.checkAuth] isLoggedIn:', isLoggedIn); + + if (result.exitCode === 0 && isLoggedIn) { + console.log('[CodexProvider.checkAuth] CLI says logged in, returning authenticated'); return { authenticated: true, method: 'oauth' }; } - } catch { - // Auth command failed, not authenticated + } catch (error) { + console.log('[CodexProvider.checkAuth] Error running login status:', error); } } + console.log('[CodexProvider.checkAuth] Not authenticated'); return { authenticated: false, method: 'none' }; } - /** - * Deduplicate text blocks in Codex assistant messages - * - * Codex can send: - * 1. Duplicate consecutive text blocks (same text twice in a row) - * 2. A final accumulated block containing ALL previous text - * - * This method filters out these duplicates to prevent UI stuttering. - */ - private deduplicateTextBlocks( - content: Array<{ type: string; text?: string }>, - lastTextBlock: string, - accumulatedText: string - ): { content: Array<{ type: string; text?: string }>; lastBlock: string; accumulated: string } { - const filtered: Array<{ type: string; text?: string }> = []; - let newLastBlock = lastTextBlock; - let newAccumulated = accumulatedText; - - for (const block of content) { - if (block.type !== 'text' || !block.text) { - filtered.push(block); - continue; - } - - const text = block.text; - - // Skip empty text - if (!text.trim()) continue; - - // Skip duplicate consecutive text blocks - if (text === newLastBlock) { - continue; - } - - // Skip final accumulated text block - // Codex sends one large block containing ALL previous text at the end - if (newAccumulated.length > 100 && text.length > newAccumulated.length * 0.8) { - const normalizedAccum = newAccumulated.replace(/\s+/g, ' ').trim(); - const normalizedNew = text.replace(/\s+/g, ' ').trim(); - if (normalizedNew.includes(normalizedAccum.slice(0, 100))) { - // This is the final accumulated block, skip it - continue; - } - } - - // This is a valid new text block - newLastBlock = text; - newAccumulated += text; - filtered.push(block); - } - - return { content: filtered, lastBlock: newLastBlock, accumulated: newAccumulated }; - } - /** * Get the detected CLI path (public accessor for status endpoints) */ diff --git a/apps/server/src/routes/claude/index.ts b/apps/server/src/routes/claude/index.ts index 239499f9..20816bbc 100644 --- a/apps/server/src/routes/claude/index.ts +++ b/apps/server/src/routes/claude/index.ts @@ -13,7 +13,10 @@ export function createClaudeRoutes(service: ClaudeUsageService): Router { // Check if Claude CLI is available first const isAvailable = await service.isAvailable(); if (!isAvailable) { - res.status(503).json({ + // IMPORTANT: This endpoint is behind Automaker session auth already. + // Use a 200 + error payload for Claude CLI issues so the UI doesn't + // interpret it as an invalid Automaker session (401/403 triggers logout). + res.status(200).json({ error: 'Claude CLI not found', message: "Please install Claude Code CLI and run 'claude login' to authenticate", }); @@ -26,12 +29,13 @@ export function createClaudeRoutes(service: ClaudeUsageService): Router { const message = error instanceof Error ? error.message : 'Unknown error'; if (message.includes('Authentication required') || message.includes('token_expired')) { - res.status(401).json({ + // Do NOT use 401/403 here: that status code is reserved for Automaker session auth. + res.status(200).json({ error: 'Authentication required', message: "Please run 'claude login' to authenticate", }); } else if (message.includes('timed out')) { - res.status(504).json({ + res.status(200).json({ error: 'Command timed out', message: 'The Claude CLI took too long to respond', }); diff --git a/apps/server/src/routes/codex/index.ts b/apps/server/src/routes/codex/index.ts index 34412256..4a2db951 100644 --- a/apps/server/src/routes/codex/index.ts +++ b/apps/server/src/routes/codex/index.ts @@ -13,7 +13,10 @@ export function createCodexRoutes(service: CodexUsageService): Router { // Check if Codex CLI is available first const isAvailable = await service.isAvailable(); if (!isAvailable) { - res.status(503).json({ + // IMPORTANT: This endpoint is behind Automaker session auth already. + // Use a 200 + error payload for Codex CLI issues so the UI doesn't + // interpret it as an invalid Automaker session (401/403 triggers logout). + res.status(200).json({ error: 'Codex CLI not found', message: "Please install Codex CLI and run 'codex login' to authenticate", }); @@ -26,18 +29,19 @@ export function createCodexRoutes(service: CodexUsageService): Router { const message = error instanceof Error ? error.message : 'Unknown error'; if (message.includes('not authenticated') || message.includes('login')) { - res.status(401).json({ + // Do NOT use 401/403 here: that status code is reserved for Automaker session auth. + res.status(200).json({ error: 'Authentication required', message: "Please run 'codex login' to authenticate", }); } else if (message.includes('not available') || message.includes('does not provide')) { // This is the expected case - Codex doesn't provide usage stats - res.status(503).json({ + res.status(200).json({ error: 'Usage statistics not available', message: message, }); } else if (message.includes('timed out')) { - res.status(504).json({ + res.status(200).json({ error: 'Command timed out', message: 'The Codex CLI took too long to respond', }); diff --git a/apps/server/src/routes/setup/routes/codex-status.ts b/apps/server/src/routes/setup/routes/codex-status.ts index fee782da..84f2c3f4 100644 --- a/apps/server/src/routes/setup/routes/codex-status.ts +++ b/apps/server/src/routes/setup/routes/codex-status.ts @@ -19,6 +19,12 @@ export function createCodexStatusHandler() { const provider = new CodexProvider(); const status = await provider.detectInstallation(); + // Derive auth method from authenticated status and API key presence + let authMethod = 'none'; + if (status.authenticated) { + authMethod = status.hasApiKey ? 'api_key_env' : 'cli_authenticated'; + } + res.json({ success: true, installed: status.installed, @@ -26,7 +32,7 @@ export function createCodexStatusHandler() { path: status.path || null, auth: { authenticated: status.authenticated || false, - method: status.method || 'cli', + method: authMethod, hasApiKey: status.hasApiKey || false, }, installCommand, diff --git a/apps/server/src/services/codex-usage-service.ts b/apps/server/src/services/codex-usage-service.ts index 3697f5c9..6af12880 100644 --- a/apps/server/src/services/codex-usage-service.ts +++ b/apps/server/src/services/codex-usage-service.ts @@ -1,5 +1,6 @@ -import { spawn } from 'child_process'; import * as os from 'os'; +import { findCodexCliPath } from '@automaker/platform'; +import { checkCodexAuthentication } from '../lib/codex-auth.js'; export interface CodexRateLimitWindow { limit: number; @@ -40,21 +41,16 @@ export interface CodexUsageData { export class CodexUsageService { private codexBinary = 'codex'; private isWindows = os.platform() === 'win32'; + private cachedCliPath: string | null = null; /** * Check if Codex CLI is available on the system */ async isAvailable(): Promise { - return new Promise((resolve) => { - const checkCmd = this.isWindows ? 'where' : 'which'; - const proc = spawn(checkCmd, [this.codexBinary]); - proc.on('close', (code) => { - resolve(code === 0); - }); - proc.on('error', () => { - resolve(false); - }); - }); + // Prefer our platform-aware resolver over `which/where` because the server + // process PATH may not include npm global bins (nvm/fnm/volta/pnpm). + this.cachedCliPath = await findCodexCliPath(); + return Boolean(this.cachedCliPath); } /** @@ -84,29 +80,9 @@ export class CodexUsageService { * Check if Codex is authenticated */ private async checkAuthentication(): Promise { - return new Promise((resolve) => { - const proc = spawn(this.codexBinary, ['login', 'status'], { - env: { - ...process.env, - TERM: 'dumb', // Avoid interactive output - }, - }); - - let output = ''; - - proc.stdout.on('data', (data) => { - output += data.toString(); - }); - - proc.on('close', (code) => { - // Check if output indicates logged in - const isLoggedIn = output.toLowerCase().includes('logged in'); - resolve(code === 0 && isLoggedIn); - }); - - proc.on('error', () => { - resolve(false); - }); - }); + // Use the cached CLI path if available, otherwise fall back to finding it + const cliPath = this.cachedCliPath || (await findCodexCliPath()); + const authCheck = await checkCodexAuthentication(cliPath); + return authCheck.authenticated; } } diff --git a/apps/ui/src/components/views/logged-out-view.tsx b/apps/ui/src/components/views/logged-out-view.tsx index 26ec649c..3239a9bd 100644 --- a/apps/ui/src/components/views/logged-out-view.tsx +++ b/apps/ui/src/components/views/logged-out-view.tsx @@ -1,6 +1,6 @@ import { useNavigate } from '@tanstack/react-router'; import { Button } from '@/components/ui/button'; -import { LogOut, RefreshCcw } from 'lucide-react'; +import { LogOut } from 'lucide-react'; export function LoggedOutView() { const navigate = useNavigate(); @@ -22,10 +22,6 @@ export function LoggedOutView() { - diff --git a/apps/ui/src/components/views/settings-view.tsx b/apps/ui/src/components/views/settings-view.tsx index 659e0911..c57ca13d 100644 --- a/apps/ui/src/components/views/settings-view.tsx +++ b/apps/ui/src/components/views/settings-view.tsx @@ -2,7 +2,7 @@ import { useState } from 'react'; import { useAppStore } from '@/store/app-store'; import { useSetupStore } from '@/store/setup-store'; -import { useSettingsView } from './settings-view/hooks'; +import { useSettingsView, type SettingsViewId } from './settings-view/hooks'; import { NAV_ITEMS } from './settings-view/config/navigation'; import { SettingsHeader } from './settings-view/components/settings-header'; import { KeyboardMapDialog } from './settings-view/components/keyboard-map-dialog'; @@ -18,7 +18,7 @@ import { FeatureDefaultsSection } from './settings-view/feature-defaults/feature import { DangerZoneSection } from './settings-view/danger-zone/danger-zone-section'; import { AccountSection } from './settings-view/account'; import { SecuritySection } from './settings-view/security'; -import { ProviderTabs } from './settings-view/providers'; +import { ClaudeSettingsTab, CursorSettingsTab, CodexSettingsTab } from './settings-view/providers'; import { MCPServersSection } from './settings-view/mcp-servers'; import { PromptCustomizationSection } from './settings-view/prompts'; import type { Project as SettingsProject, Theme } from './settings-view/shared/types'; @@ -88,15 +88,30 @@ export function SettingsView() { // Use settings view navigation hook const { activeView, navigateTo } = useSettingsView(); + // Handle navigation - if navigating to 'providers', default to 'claude-provider' + const handleNavigate = (viewId: SettingsViewId) => { + if (viewId === 'providers') { + navigateTo('claude-provider'); + } else { + navigateTo(viewId); + } + }; + const [showDeleteDialog, setShowDeleteDialog] = useState(false); const [showKeyboardMapDialog, setShowKeyboardMapDialog] = useState(false); // Render the active section based on current view const renderActiveSection = () => { switch (activeView) { + case 'claude-provider': + return ; + case 'cursor-provider': + return ; + case 'codex-provider': + return ; case 'providers': - case 'claude': // Backwards compatibility - return ; + case 'claude': // Backwards compatibility - redirect to claude-provider + return ; case 'mcp-servers': return ; case 'prompts': @@ -181,7 +196,7 @@ export function SettingsView() { navItems={NAV_ITEMS} activeSection={activeView} currentProject={currentProject} - onNavigate={navigateTo} + onNavigate={handleNavigate} /> {/* Content Panel - Shows only the active section */} diff --git a/apps/ui/src/components/views/settings-view/cli-status/codex-cli-status.tsx b/apps/ui/src/components/views/settings-view/cli-status/codex-cli-status.tsx index 3e267a72..fb7af414 100644 --- a/apps/ui/src/components/views/settings-view/cli-status/codex-cli-status.tsx +++ b/apps/ui/src/components/views/settings-view/cli-status/codex-cli-status.tsx @@ -1,24 +1,237 @@ +import { Button } from '@/components/ui/button'; +import { CheckCircle2, AlertCircle, RefreshCw, XCircle } from 'lucide-react'; +import { cn } from '@/lib/utils'; import type { CliStatus } from '../shared/types'; -import { CliStatusCard } from './cli-status-card'; +import type { CodexAuthStatus } from '@/store/setup-store'; import { OpenAIIcon } from '@/components/ui/provider-icon'; interface CliStatusProps { status: CliStatus | null; + authStatus?: CodexAuthStatus | null; isChecking: boolean; onRefresh: () => void; } -export function CodexCliStatus({ status, isChecking, onRefresh }: CliStatusProps) { +function getAuthMethodLabel(method: string): string { + switch (method) { + case 'api_key': + return 'API Key'; + case 'api_key_env': + return 'API Key (Environment)'; + case 'cli_authenticated': + case 'oauth': + return 'CLI Authentication'; + default: + return method || 'Unknown'; + } +} + +function SkeletonPulse({ className }: { className?: string }) { + return
; +} + +function CodexCliStatusSkeleton() { return ( - +
+
+
+
+ + +
+ +
+
+ +
+
+
+ {/* Installation status skeleton */} +
+ +
+ + + +
+
+ {/* Auth status skeleton */} +
+ +
+ + +
+
+
+
+ ); +} + +export function CodexCliStatus({ status, authStatus, isChecking, onRefresh }: CliStatusProps) { + if (!status) return ; + + return ( +
+
+
+
+
+ +
+

Codex CLI

+
+ +
+

+ Codex CLI powers OpenAI models for coding and automation workflows. +

+
+
+ {status.success && status.status === 'installed' ? ( +
+
+
+ +
+
+

Codex CLI Installed

+
+ {status.method && ( +

+ Method: {status.method} +

+ )} + {status.version && ( +

+ Version: {status.version} +

+ )} + {status.path && ( +

+ Path: {status.path} +

+ )} +
+
+
+ {/* Authentication Status */} + {authStatus?.authenticated ? ( +
+
+ +
+
+

Authenticated

+
+

+ Method:{' '} + {getAuthMethodLabel(authStatus.method)} +

+
+
+
+ ) : ( +
+
+ +
+
+

Not Authenticated

+

+ Run codex login{' '} + or set an API key to authenticate. +

+
+
+ )} + + {status.recommendation && ( +

{status.recommendation}

+ )} +
+ ) : ( +
+
+
+ +
+
+

Codex CLI Not Detected

+

+ {status.recommendation || + 'Install Codex CLI to unlock OpenAI models with tool support.'} +

+
+
+ {status.installCommands && ( +
+

Installation Commands:

+
+ {status.installCommands.npm && ( +
+

+ npm +

+ + {status.installCommands.npm} + +
+ )} + {status.installCommands.macos && ( +
+

+ macOS/Linux +

+ + {status.installCommands.macos} + +
+ )} + {status.installCommands.windows && ( +
+

+ Windows (PowerShell) +

+ + {status.installCommands.windows} + +
+ )} +
+
+ )} +
+ )} +
+
); } diff --git a/apps/ui/src/components/views/settings-view/components/settings-navigation.tsx b/apps/ui/src/components/views/settings-view/components/settings-navigation.tsx index 0028eac7..fd3b4f07 100644 --- a/apps/ui/src/components/views/settings-view/components/settings-navigation.tsx +++ b/apps/ui/src/components/views/settings-view/components/settings-navigation.tsx @@ -57,6 +57,85 @@ function NavButton({ ); } +function NavItemWithSubItems({ + item, + activeSection, + onNavigate, +}: { + item: NavigationItem; + activeSection: SettingsViewId; + onNavigate: (sectionId: SettingsViewId) => void; +}) { + const hasActiveSubItem = item.subItems?.some((subItem) => subItem.id === activeSection) ?? false; + const isParentActive = item.id === activeSection; + const Icon = item.icon; + + return ( +
+ {/* Parent item - non-clickable label */} +
+ + {item.label} +
+ {/* Sub-items - always displayed */} + {item.subItems && ( +
+ {item.subItems.map((subItem) => { + const SubIcon = subItem.icon; + const isSubActive = subItem.id === activeSection; + return ( + + ); + })} +
+ )} +
+ ); +} + export function SettingsNavigation({ activeSection, currentProject, @@ -78,14 +157,23 @@ export function SettingsNavigation({ {/* Global Settings Items */}
- {GLOBAL_NAV_ITEMS.map((item) => ( - - ))} + {GLOBAL_NAV_ITEMS.map((item) => + item.subItems ? ( + + ) : ( + + ) + )}
{/* Project Settings - only show when a project is selected */} diff --git a/apps/ui/src/components/views/settings-view/config/navigation.ts b/apps/ui/src/components/views/settings-view/config/navigation.ts index 5e17c1fd..391e5f34 100644 --- a/apps/ui/src/components/views/settings-view/config/navigation.ts +++ b/apps/ui/src/components/views/settings-view/config/navigation.ts @@ -1,3 +1,4 @@ +import React from 'react'; import type { LucideIcon } from 'lucide-react'; import { Key, @@ -14,12 +15,14 @@ import { User, Shield, } from 'lucide-react'; +import { AnthropicIcon, CursorIcon, OpenAIIcon } from '@/components/ui/provider-icon'; import type { SettingsViewId } from '../hooks/use-settings-view'; export interface NavigationItem { id: SettingsViewId; label: string; - icon: LucideIcon; + icon: LucideIcon | React.ComponentType<{ className?: string }>; + subItems?: NavigationItem[]; } export interface NavigationGroup { @@ -30,7 +33,16 @@ export interface NavigationGroup { // Global settings - always visible export const GLOBAL_NAV_ITEMS: NavigationItem[] = [ { id: 'api-keys', label: 'API Keys', icon: Key }, - { id: 'providers', label: 'AI Providers', icon: Bot }, + { + id: 'providers', + label: 'AI Providers', + icon: Bot, + subItems: [ + { id: 'claude-provider', label: 'Claude', icon: AnthropicIcon }, + { id: 'cursor-provider', label: 'Cursor', icon: CursorIcon }, + { id: 'codex-provider', label: 'Codex', icon: OpenAIIcon }, + ], + }, { id: 'mcp-servers', label: 'MCP Servers', icon: Plug }, { id: 'prompts', label: 'Prompt Customization', icon: MessageSquareText }, { id: 'model-defaults', label: 'Model Defaults', icon: Workflow }, diff --git a/apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts b/apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts index 8755f2a1..f18ce832 100644 --- a/apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts +++ b/apps/ui/src/components/views/settings-view/hooks/use-settings-view.ts @@ -4,6 +4,9 @@ export type SettingsViewId = | 'api-keys' | 'claude' | 'providers' + | 'claude-provider' + | 'cursor-provider' + | 'codex-provider' | 'mcp-servers' | 'prompts' | 'model-defaults' diff --git a/apps/ui/src/components/views/settings-view/providers/codex-settings-tab.tsx b/apps/ui/src/components/views/settings-view/providers/codex-settings-tab.tsx index 0f8efdc1..e1dccedd 100644 --- a/apps/ui/src/components/views/settings-view/providers/codex-settings-tab.tsx +++ b/apps/ui/src/components/views/settings-view/providers/codex-settings-tab.tsx @@ -54,7 +54,7 @@ export function CodexSettingsTab() { } : null); - // Load Codex CLI status on mount + // Load Codex CLI status and auth status on mount useEffect(() => { const checkCodexStatus = async () => { const api = getElectronAPI(); @@ -158,11 +158,13 @@ export function CodexSettingsTab() { ); const showUsageTracking = codexAuthStatus?.authenticated ?? false; + const authStatusToDisplay = codexAuthStatus; return (
diff --git a/apps/ui/src/hooks/use-settings-sync.ts b/apps/ui/src/hooks/use-settings-sync.ts index 0f9514a9..0f645703 100644 --- a/apps/ui/src/hooks/use-settings-sync.ts +++ b/apps/ui/src/hooks/use-settings-sync.ts @@ -17,6 +17,7 @@ import { getHttpApiClient, waitForApiKeyInit } from '@/lib/http-api-client'; import { setItem } from '@/lib/storage'; import { useAppStore, type ThemeMode, THEME_STORAGE_KEY } from '@/store/app-store'; import { useSetupStore } from '@/store/setup-store'; +import { useAuthStore } from '@/store/auth-store'; import { waitForMigrationComplete } from './use-settings-migration'; import type { GlobalSettings } from '@automaker/types'; @@ -90,6 +91,9 @@ export function useSettingsSync(): SettingsSyncState { syncing: false, }); + const isAuthenticated = useAuthStore((s) => s.isAuthenticated); + const authChecked = useAuthStore((s) => s.authChecked); + const syncTimeoutRef = useRef | null>(null); const lastSyncedRef = useRef(''); const isInitializedRef = useRef(false); @@ -160,6 +164,9 @@ export function useSettingsSync(): SettingsSyncState { // Initialize sync - WAIT for migration to complete first useEffect(() => { + // Don't initialize syncing until we know auth status and are authenticated. + // Prevents accidental overwrites when the app boots before settings are hydrated. + if (!authChecked || !isAuthenticated) return; if (isInitializedRef.current) return; isInitializedRef.current = true; @@ -204,7 +211,7 @@ export function useSettingsSync(): SettingsSyncState { } initializeSync(); - }, []); + }, [authChecked, isAuthenticated]); // Subscribe to store changes and sync to server useEffect(() => { diff --git a/apps/ui/src/routes/__root.tsx b/apps/ui/src/routes/__root.tsx index d98470ec..faab81fa 100644 --- a/apps/ui/src/routes/__root.tsx +++ b/apps/ui/src/routes/__root.tsx @@ -251,44 +251,67 @@ function RootLayoutContent() { } if (isValid) { - // 2. Check Settings if valid + // 2. Load settings (and hydrate stores) before marking auth as checked. + // This prevents useSettingsSync from pushing default/empty state to the server + // when the backend is still starting up or temporarily unavailable. const api = getHttpApiClient(); try { - const settingsResult = await api.settings.getGlobal(); - if (settingsResult.success && settingsResult.settings) { - // Perform migration from localStorage if needed (first-time migration) - // This checks if localStorage has projects/data that server doesn't have - // and merges them before hydrating the store - const { settings: finalSettings, migrated } = await performSettingsMigration( - settingsResult.settings as unknown as Parameters[0] - ); + const maxAttempts = 8; + const baseDelayMs = 250; + let lastError: unknown = null; - if (migrated) { - logger.info('Settings migration from localStorage completed'); + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + try { + const settingsResult = await api.settings.getGlobal(); + if (settingsResult.success && settingsResult.settings) { + const { settings: finalSettings, migrated } = await performSettingsMigration( + settingsResult.settings as unknown as Parameters< + typeof performSettingsMigration + >[0] + ); + + if (migrated) { + logger.info('Settings migration from localStorage completed'); + } + + // Hydrate store with the final settings (merged if migration occurred) + hydrateStoreFromSettings(finalSettings); + + // Signal that settings hydration is complete so useSettingsSync can start + signalMigrationComplete(); + + // Mark auth as checked only after settings hydration succeeded. + useAuthStore + .getState() + .setAuthState({ isAuthenticated: true, authChecked: true }); + return; + } + + lastError = settingsResult; + } catch (error) { + lastError = error; } - // Hydrate store with the final settings (merged if migration occurred) - hydrateStoreFromSettings(finalSettings); - - // Signal that settings hydration is complete so useSettingsSync can start - signalMigrationComplete(); - - // Redirect based on setup status happens in the routing effect below - // but we can also hint navigation here if needed. - // The routing effect (lines 273+) is robust enough. + const delayMs = Math.min(1500, baseDelayMs * attempt); + logger.warn( + `Settings not ready (attempt ${attempt}/${maxAttempts}); retrying in ${delayMs}ms...`, + lastError + ); + await new Promise((resolve) => setTimeout(resolve, delayMs)); } + + throw lastError ?? new Error('Failed to load settings'); } catch (error) { logger.error('Failed to fetch settings after valid session:', error); - // If settings fail, we might still be authenticated but can't determine setup status. - // We should probably treat as authenticated but setup unknown? - // Or fail safe to logged-out/error? - // Existing logic relies on setupComplete which defaults to false/true based on env. - // Let's assume we proceed as authenticated. - // Still signal migration complete so sync can start (will sync current store state) + // If we can't load settings, we must NOT start syncing defaults to the server. + // Treat as not authenticated for now (backend likely unavailable) and unblock sync hook. + useAuthStore.getState().setAuthState({ isAuthenticated: false, authChecked: true }); signalMigrationComplete(); + if (location.pathname !== '/logged-out' && location.pathname !== '/login') { + navigate({ to: '/logged-out' }); + } + return; } - - useAuthStore.getState().setAuthState({ isAuthenticated: true, authChecked: true }); } else { // Session is invalid or expired - treat as not authenticated useAuthStore.getState().setAuthState({ isAuthenticated: false, authChecked: true }); diff --git a/apps/ui/tests/settings/settings-startup-sync-race.spec.ts b/apps/ui/tests/settings/settings-startup-sync-race.spec.ts new file mode 100644 index 00000000..b9c51cc6 --- /dev/null +++ b/apps/ui/tests/settings/settings-startup-sync-race.spec.ts @@ -0,0 +1,107 @@ +/** + * Settings Startup Race Regression Test + * + * Repro (historical bug): + * - UI verifies session successfully + * - Initial GET /api/settings/global fails transiently (backend still starting) + * - UI unblocks settings sync anyway and can push default empty state to server + * - Server persists projects: [] (and other defaults), wiping settings.json + * + * This test forces the first few /api/settings/global requests to fail and asserts that + * the server-side settings.json is NOT overwritten while the UI is waiting to hydrate. + */ + +import { test, expect } from '@playwright/test'; +import * as fs from 'fs'; +import * as path from 'path'; +import { authenticateForTests } from '../utils'; + +const SETTINGS_PATH = path.resolve(process.cwd(), '../server/data/settings.json'); +const WORKSPACE_ROOT = path.resolve(process.cwd(), '../..'); +const FIXTURE_PROJECT_PATH = path.join(WORKSPACE_ROOT, 'test/fixtures/projectA'); + +test.describe('Settings startup sync race', () => { + let originalSettingsJson: string; + + test.beforeAll(() => { + originalSettingsJson = fs.readFileSync(SETTINGS_PATH, 'utf-8'); + + const settings = JSON.parse(originalSettingsJson) as Record; + settings.projects = [ + { + id: `e2e-project-${Date.now()}`, + name: 'E2E Project (settings race)', + path: FIXTURE_PROJECT_PATH, + lastOpened: new Date().toISOString(), + theme: 'dark', + }, + ]; + + fs.writeFileSync(SETTINGS_PATH, JSON.stringify(settings, null, 2)); + }); + + test.afterAll(() => { + // Restore original settings.json to avoid polluting other tests/dev state + fs.writeFileSync(SETTINGS_PATH, originalSettingsJson); + }); + + test('does not overwrite projects when /api/settings/global is temporarily unavailable', async ({ + page, + }) => { + // Gate the real settings request so we can assert file contents before allowing hydration. + let requestCount = 0; + let allowSettingsRequestResolve: (() => void) | null = null; + const allowSettingsRequest = new Promise((resolve) => { + allowSettingsRequestResolve = resolve; + }); + + let sawThreeFailuresResolve: (() => void) | null = null; + const sawThreeFailures = new Promise((resolve) => { + sawThreeFailuresResolve = resolve; + }); + + await page.route('**/api/settings/global', async (route) => { + requestCount++; + if (requestCount <= 3) { + if (requestCount === 3) { + sawThreeFailuresResolve?.(); + } + await route.abort('failed'); + return; + } + // Keep the 4th+ request pending until the test explicitly allows it. + await allowSettingsRequest; + await route.continue(); + }); + + // Ensure we are authenticated (session cookie) before loading the app. + await authenticateForTests(page); + await page.goto('/'); + + // Wait until we have forced a few failures. + await sawThreeFailures; + + // At this point, the UI should NOT have written defaults back to the server. + const settingsAfterFailures = JSON.parse(fs.readFileSync(SETTINGS_PATH, 'utf-8')) as { + projects?: Array<{ path?: string }>; + }; + expect(settingsAfterFailures.projects?.length).toBeGreaterThan(0); + expect(settingsAfterFailures.projects?.[0]?.path).toBe(FIXTURE_PROJECT_PATH); + + // Allow the settings request to succeed so the app can hydrate and proceed. + allowSettingsRequestResolve?.(); + + // App should eventually render a main view after settings hydration. + await page + .locator('[data-testid="welcome-view"], [data-testid="board-view"]') + .first() + .waitFor({ state: 'visible', timeout: 30000 }); + + // Verify settings.json still contains the project after hydration completes. + const settingsAfterHydration = JSON.parse(fs.readFileSync(SETTINGS_PATH, 'utf-8')) as { + projects?: Array<{ path?: string }>; + }; + expect(settingsAfterHydration.projects?.length).toBeGreaterThan(0); + expect(settingsAfterHydration.projects?.[0]?.path).toBe(FIXTURE_PROJECT_PATH); + }); +});