From 17e2c2171f790f055f872576bd9db43e5b1f8497 Mon Sep 17 00:00:00 2001 From: Reagan Hsu Date: Tue, 5 May 2026 20:10:56 -0700 Subject: [PATCH] Move settings into the hub page Settings was still modeled as a separate Electron window, which made provider focus, section spacing, and app-level navigation harder to keep coherent. The hub now owns settings as a scrollable in-window page with anchored tabs, Application first, Browser Sync split out, and provider loading skeletons that preserve row height. Constraint: Preserve unrelated dirty session persistence work in the worktree Rejected: Keep settings:browsercode:focus-provider IPC | settings focus now travels in the open-settings payload Rejected: Push current main directly | main already had an unrelated local commit ahead of origin/main Confidence: high Scope-risk: moderate Directive: Keep SettingsSectionId, SETTINGS_TABS, and rendered settings sections in the same order Tested: git diff --cached --check; task typecheck; npm run test -- ConnectionsPane BrowserCodeModelPicker EnginePicker SettingsPane; task lint Not-tested: Live Electron visual pass --- app/src/main/index.ts | 71 +-- app/src/main/settings/SettingsWindow.ts | 146 ----- app/src/preload/pill.ts | 7 +- app/src/preload/shell.ts | 20 +- app/src/renderer/globals.d.ts | 3 +- .../renderer/hub/BrowserCodeModelPicker.tsx | 63 +- app/src/renderer/hub/ConnectionsPane.tsx | 302 +++++++--- app/src/renderer/hub/HubApp.tsx | 127 +++-- app/src/renderer/hub/SettingsPane.tsx | 182 ++++-- app/src/renderer/hub/hub.css | 538 +++++++++--------- app/src/renderer/hub/keybindings.ts | 9 +- app/src/renderer/pill/pill.css | 71 +++ app/src/renderer/shared/CookieBrowser.css | 2 +- app/tests/a11y/axe-audit.spec.ts | 19 +- .../unit/hub/BrowserCodeModelPicker.spec.tsx | 57 +- app/tests/unit/hub/ConnectionsPane.spec.tsx | 160 ++++++ app/tests/unit/hub/SettingsPane.spec.tsx | 11 +- .../unit/settings/SettingsWindow.test.ts | 186 ------ app/tests/visual/capture.spec.ts | 153 ++--- app/tests/visual/review.html | 10 +- 20 files changed, 1154 insertions(+), 983 deletions(-) delete mode 100644 app/src/main/settings/SettingsWindow.ts create mode 100644 app/tests/unit/hub/ConnectionsPane.spec.tsx delete mode 100644 app/tests/unit/settings/SettingsWindow.test.ts diff --git a/app/src/main/index.ts b/app/src/main/index.ts index cf77d5b4..731b2edf 100644 --- a/app/src/main/index.ts +++ b/app/src/main/index.ts @@ -4,7 +4,7 @@ * Browser modules (tabs, bookmarks, history, downloads, extensions, * permissions, profiles, etc.) have been removed in the nuclear pivot. * Only the core infrastructure remains: shell window, pill, HL engine, - * OAuth/identity, settings window, updater, hotkeys. + * OAuth/identity, settings page routing, updater, hotkeys. */ import { config as loadDotEnv } from 'dotenv'; @@ -107,8 +107,6 @@ import { forwardAgentEvent } from './pill'; // Session management import { SessionManager } from './sessions/SessionManager'; import { BrowserPool } from './sessions/BrowserPool'; -// Settings window (no browser-feature IPC handlers) -import { openSettingsWindow, closeSettingsWindow, getSettingsWindow } from './settings/SettingsWindow'; // Channels (WhatsApp) import { WhatsAppAdapter } from './channels/WhatsAppAdapter'; import { ChannelRouter } from './channels/ChannelRouter'; @@ -205,6 +203,27 @@ const accountStore = new AccountStore(); const whatsAppAdapter = new WhatsAppAdapter(); const channelRouter = new ChannelRouter(sessionManager, whatsAppAdapter); +type SettingsOpenPayload = { + focusBrowserCodeProvider?: string; +}; + +function normalizeSettingsOpenPayload(payload: unknown): SettingsOpenPayload | undefined { + if (!payload || typeof payload !== 'object') return undefined; + const rawProvider = (payload as { focusBrowserCodeProvider?: unknown }).focusBrowserCodeProvider; + if (typeof rawProvider !== 'string') return undefined; + const providerId = rawProvider.trim(); + if (!providerId || providerId.length > 80) return undefined; + return { focusBrowserCodeProvider: providerId }; +} + +function openSettingsInShell(payload?: SettingsOpenPayload): void { + if (!shellWindow || shellWindow.isDestroyed()) return; + shellWindow.show(); + shellWindow.focus(); + shellWindow.webContents.send('open-settings', payload); +} + + // --------------------------------------------------------------------------- // Shell window factory // --------------------------------------------------------------------------- @@ -1377,23 +1396,12 @@ app.whenReady().then(async () => { }); // --------------------------------------------------------------------------- - // Settings window IPC + // Settings page IPC // --------------------------------------------------------------------------- - ipcMain.handle('settings:open', (_e, payload?: { focusBrowserCodeProvider?: string }) => { + ipcMain.handle('settings:open', (_e, rawPayload?: unknown) => { + const payload = normalizeSettingsOpenPayload(rawPayload); mainLogger.info('main.settings:open', { focusBrowserCodeProvider: payload?.focusBrowserCodeProvider }); - if (shellWindow && !shellWindow.isDestroyed()) { - shellWindow.show(); - shellWindow.focus(); - shellWindow.webContents.send('open-settings'); - if (payload?.focusBrowserCodeProvider) { - const providerId = payload.focusBrowserCodeProvider; - setTimeout(() => { - if (shellWindow && !shellWindow.isDestroyed()) { - shellWindow.webContents.send('settings:browsercode:focus-provider', { providerId }); - } - }, 150); - } - } + openSettingsInShell(payload); }); ipcMain.handle('settings:app:get-info', () => { @@ -1438,29 +1446,13 @@ app.whenReady().then(async () => { hidePill(); }); - ipcMain.handle('pill:open-settings', (_e, payload?: { focusBrowserCodeProvider?: string }) => { + ipcMain.handle('pill:open-settings', (_e, rawPayload?: unknown) => { + const payload = normalizeSettingsOpenPayload(rawPayload); mainLogger.info('main.pill:open-settings', { focusBrowserCodeProvider: payload?.focusBrowserCodeProvider }); - if (shellWindow && !shellWindow.isDestroyed()) { - shellWindow.show(); - shellWindow.focus(); - shellWindow.webContents.send('open-settings'); - if (payload?.focusBrowserCodeProvider) { - const providerId = payload.focusBrowserCodeProvider; - setTimeout(() => { - if (shellWindow && !shellWindow.isDestroyed()) { - shellWindow.webContents.send('settings:browsercode:focus-provider', { providerId }); - } - }, 150); - } - } + openSettingsInShell(payload); hidePill(); }); - ipcMain.handle('settings:close', () => { - mainLogger.info('main.settings:close'); - closeSettingsWindow(); - }); - // --------------------------------------------------------------------------- // Application menu // --------------------------------------------------------------------------- @@ -1583,10 +1575,7 @@ function buildApplicationMenu(): void { accelerator: 'CmdOrCtrl+,', click: () => { mainLogger.debug('menu.openSettings'); - if (shellWindow && !shellWindow.isDestroyed()) { - shellWindow.webContents.send('open-settings'); - shellWindow.focus(); - } + openSettingsInShell(); }, }, { type: 'separator' }, diff --git a/app/src/main/settings/SettingsWindow.ts b/app/src/main/settings/SettingsWindow.ts deleted file mode 100644 index f50a75cc..00000000 --- a/app/src/main/settings/SettingsWindow.ts +++ /dev/null @@ -1,146 +0,0 @@ -/** - * SettingsWindow.ts — creates and manages the Settings BrowserWindow. - * - * Spec (Track 5): - * width: 720, height: 560, resizable: false, titleBarStyle: 'hiddenInset' - * Warm theme: data-theme="onboarding" for visual continuity - * Preload: src/preload/settings.ts (built as settings.js in .vite/build/) - * Renderer: settings/settings.html (served by the settings Vite entry) - * - * Follows path invariant rules from memory: - * - preload: path.join(__dirname, 'settings.js') - * - loadURL: full path from project root - * - HTML script src: relative ./index.tsx - * - * D2 logging: window lifecycle events. - */ - -import path from 'node:path'; -import { BrowserWindow } from 'electron'; -import { mainLogger, rendererLogger } from '../logger'; - -// --------------------------------------------------------------------------- -// Forge VitePlugin globals (injected at build time) -// --------------------------------------------------------------------------- - -declare const SETTINGS_VITE_DEV_SERVER_URL: string | undefined; -declare const SETTINGS_VITE_NAME: string | undefined; - -// --------------------------------------------------------------------------- -// Singleton reference -// --------------------------------------------------------------------------- - -let settingsWindow: BrowserWindow | null = null; - -// --------------------------------------------------------------------------- -// Public API -// --------------------------------------------------------------------------- - -/** - * Open (or focus) the Settings window. - * Returns the BrowserWindow instance. - */ -export function openSettingsWindow(): BrowserWindow { - // If already open, focus and return - if (settingsWindow && !settingsWindow.isDestroyed()) { - mainLogger.info('SettingsWindow.focus', { windowId: settingsWindow.id }); - settingsWindow.focus(); - return settingsWindow; - } - - mainLogger.info('SettingsWindow.create'); - - const preloadPath = path.join(__dirname, 'settings.js'); - - settingsWindow = new BrowserWindow({ - width: 720, - height: 560, - resizable: false, - titleBarStyle: 'hiddenInset', - show: false, - backgroundColor: '#1a1a1f', // Match --color-bg-base (onboarding theme) - webPreferences: { - preload: preloadPath, - contextIsolation: true, - nodeIntegration: false, - sandbox: true, - }, - }); - - // Show window once renderer is painted (avoids white flash) - settingsWindow.once('ready-to-show', () => { - if (!settingsWindow || settingsWindow.isDestroyed()) return; - settingsWindow.show(); - settingsWindow.focus(); - const [x, y] = settingsWindow.getPosition(); - const [w, h] = settingsWindow.getSize(); - mainLogger.info('SettingsWindow.readyToShow', { - windowId: settingsWindow.id, - position: { x, y }, - size: { w, h }, - }); - }); - - settingsWindow.on('closed', () => { - mainLogger.info('SettingsWindow.closed'); - settingsWindow = null; - }); - - settingsWindow.webContents.on('did-fail-load', (_e, code, desc, url) => { - mainLogger.error('SettingsWindow.did-fail-load', { code, desc, url }); - }); - - settingsWindow.webContents.on('did-finish-load', () => { - mainLogger.info('SettingsWindow.did-finish-load', { - url: settingsWindow?.webContents.getURL(), - }); - }); - - settingsWindow.webContents.on('console-message', (_e, level, message, line, source) => { - rendererLogger.info('renderer.console', { window: 'settings', level, source, line, message }); - }); - - // Load the settings renderer - if (typeof SETTINGS_VITE_DEV_SERVER_URL !== 'undefined' && SETTINGS_VITE_DEV_SERVER_URL) { - const url = `${SETTINGS_VITE_DEV_SERVER_URL}/src/renderer/settings/settings.html`; - mainLogger.debug('SettingsWindow.loadURL', { url }); - void settingsWindow.loadURL(url); - } else { - // Production: load from built file - const name = typeof SETTINGS_VITE_NAME !== 'undefined' ? SETTINGS_VITE_NAME : 'settings'; - const filePath = path.join( - __dirname, - `../../renderer/${name}/settings.html`, - ); - mainLogger.debug('SettingsWindow.loadFile', { filePath }); - void settingsWindow.loadFile(filePath); - } - - mainLogger.info('SettingsWindow.create.ok', { - windowId: settingsWindow.id, - width: 720, - height: 560, - }); - - return settingsWindow; -} - -/** - * Get the current settings window reference (or null if not open). - */ -export function getSettingsWindow(): BrowserWindow | null { - if (settingsWindow && !settingsWindow.isDestroyed()) { - return settingsWindow; - } - return null; -} - -/** - * Close the settings window if open. - */ -export function closeSettingsWindow(): void { - if (settingsWindow && !settingsWindow.isDestroyed()) { - mainLogger.info('SettingsWindow.closeRequested'); - settingsWindow.close(); - } -} diff --git a/app/src/preload/pill.ts b/app/src/preload/pill.ts index 99a9d3e3..de668214 100644 --- a/app/src/preload/pill.ts +++ b/app/src/preload/pill.ts @@ -114,7 +114,7 @@ contextBridge.exposeInMainWorld('pillAPI', { }, /** - * Open the settings window. + * Open the settings page in the hub window. */ openSettings: (): void => { log.info('preload.pill.openSettings', { message: 'Invoking pill:open-settings' }); @@ -266,11 +266,6 @@ contextBridge.exposeInMainWorld('electronAPI', { log.info('preload.pill.electronAPI.settings.open', { focusBrowserCodeProvider: payload?.focusBrowserCodeProvider }); return ipcRenderer.invoke('pill:open-settings', payload); }, - onFocusBrowserCodeProvider: (handler: (providerId: string) => void): (() => void) => { - const listener = (_e: unknown, payload: { providerId: string }) => handler(payload.providerId); - ipcRenderer.on('settings:browsercode:focus-provider', listener); - return () => ipcRenderer.removeListener('settings:browsercode:focus-provider', listener); - }, browserCode: { getStatus: (): Promise<{ keys: Record; diff --git a/app/src/preload/shell.ts b/app/src/preload/shell.ts index 8628e746..95e6eb43 100644 --- a/app/src/preload/shell.ts +++ b/app/src/preload/shell.ts @@ -8,6 +8,17 @@ import { } from '../shared/session-schemas'; import type { AgentSession, HlEvent, TabInfo, BrowserPoolStats } from '../shared/session-schemas'; +type SettingsOpenPayload = { focusBrowserCodeProvider?: string }; + +function normalizeSettingsOpenPayload(raw: unknown): SettingsOpenPayload | undefined { + if (!raw || typeof raw !== 'object') return undefined; + const rawProvider = (raw as { focusBrowserCodeProvider?: unknown }).focusBrowserCodeProvider; + const providerId = typeof rawProvider === 'string' ? rawProvider.trim() : ''; + return providerId.length > 0 && providerId.length <= 80 + ? { focusBrowserCodeProvider: providerId } + : undefined; +} + contextBridge.exposeInMainWorld('electronAPI', { shell: { platform: process.platform, @@ -50,11 +61,6 @@ contextBridge.exposeInMainWorld('electronAPI', { }, settings: { open: (payload?: { focusBrowserCodeProvider?: string }): Promise => ipcRenderer.invoke('settings:open', payload), - onFocusBrowserCodeProvider: (handler: (providerId: string) => void): (() => void) => { - const listener = (_e: unknown, payload: { providerId: string }) => handler(payload.providerId); - ipcRenderer.on('settings:browsercode:focus-provider', listener); - return () => ipcRenderer.removeListener('settings:browsercode:focus-provider', listener); - }, apiKey: { getMasked: (): Promise<{ present: boolean; masked: string | null }> => ipcRenderer.invoke('settings:api-key:get-masked'), @@ -365,8 +371,8 @@ contextBridge.exposeInMainWorld('electronAPI', { ipcRenderer.on('session-output-term', handler); return () => ipcRenderer.removeListener('session-output-term', handler); }, - openSettings: (cb: () => void): (() => void) => { - const handler = () => cb(); + openSettings: (cb: (payload?: SettingsOpenPayload) => void): (() => void) => { + const handler = (_event: unknown, rawPayload?: unknown) => cb(normalizeSettingsOpenPayload(rawPayload)); ipcRenderer.on('open-settings', handler); return () => ipcRenderer.removeListener('open-settings', handler); }, diff --git a/app/src/renderer/globals.d.ts b/app/src/renderer/globals.d.ts index 7c5f3dba..88053d09 100644 --- a/app/src/renderer/globals.d.ts +++ b/app/src/renderer/globals.d.ts @@ -138,7 +138,7 @@ interface ElectronOnAPI { sessionBrowserGone: (cb: (id: string) => void) => () => void; sessionOutput: (cb: (id: string, event: import('./hub/types').HlEvent) => void) => () => void; sessionOutputTerm: (cb: (id: string, bytes: string) => void) => () => void; - openSettings?: (cb: () => void) => () => void; + openSettings?: (cb: (payload?: { focusBrowserCodeProvider?: string }) => void) => () => void; zoomChanged?: (cb: (factor: number) => void) => () => void; whatsappQr?: (cb: (dataUrl: string) => void) => () => void; channelStatus?: (cb: (channelId: string, status: string, detail?: string) => void) => () => void; @@ -294,7 +294,6 @@ interface ElectronSettingsAppAPI { interface ElectronSettingsAPI { open?: (payload?: { focusBrowserCodeProvider?: string }) => Promise; - onFocusBrowserCodeProvider?: (handler: (providerId: string) => void) => () => void; apiKey: ElectronSettingsApiKeyAPI; claudeCode?: ElectronSettingsClaudeCodeAPI; openaiKey?: ElectronSettingsOpenAiKeyAPI; diff --git a/app/src/renderer/hub/BrowserCodeModelPicker.tsx b/app/src/renderer/hub/BrowserCodeModelPicker.tsx index 608843ce..7d6300e6 100644 --- a/app/src/renderer/hub/BrowserCodeModelPicker.tsx +++ b/app/src/renderer/hub/BrowserCodeModelPicker.tsx @@ -61,12 +61,27 @@ function modelLabel(providers: BrowserCodeProvider[], modelId: string | undefine return modelId.includes('/') ? modelId.split('/').pop() ?? modelId : modelId; } +function BrowserCodeSkeletonRows({ count = 3 }: { count?: number }): React.ReactElement { + return ( + <> + {Array.from({ length: count }, (_, index) => ( + + ))} + + ); +} + export function BrowserCodeModelPicker({ visible, compact = false, onOpenChange, }: BrowserCodeModelPickerProps): React.ReactElement | null { const [status, setStatus] = useState({ keys: {}, active: null, providers: [] }); + const [loaded, setLoaded] = useState(false); const [open, setOpen] = useState(false); const [drilledProviderId, setDrilledProviderId] = useState(null); const [savingModel, setSavingModel] = useState(null); @@ -79,7 +94,10 @@ export function BrowserCodeModelPicker({ const refresh = useCallback(async () => { const api = window.electronAPI?.settings?.browserCode; - if (!api) return; + if (!api) { + setLoaded(true); + return; + } try { const next = await api.getStatus(); setStatus(next); @@ -93,6 +111,8 @@ export function BrowserCodeModelPicker({ const message = (err as Error).message; setError(message); console.warn('[BrowserCodeModelPicker] status.failed', { error: message }); + } finally { + setLoaded(true); } }, []); @@ -123,6 +143,7 @@ export function BrowserCodeModelPicker({ const currentModelLabel = modelLabel(status.providers, currentModel); const hasAnyKey = Object.keys(status.keys).length > 0; const canSwitchModels = hasAnyKey && status.installed?.installed !== false; + const loadingStatus = !loaded && status.providers.length === 0; const selectModel = useCallback(async (providerId: string, model: string) => { const api = window.electronAPI?.settings?.browserCode; @@ -158,17 +179,31 @@ export function BrowserCodeModelPicker({ type="button" className="browsercode-model-picker__toggle" onClick={(e) => { e.stopPropagation(); setOpen((v) => !v); }} - title={hasAnyKey ? `BrowserCode model: ${currentModelLabel}` : 'Set up BrowserCode model'} + title={loadingStatus ? 'Loading BrowserCode model' : hasAnyKey ? `BrowserCode model: ${currentModelLabel}` : 'Set up BrowserCode model'} aria-haspopup="menu" aria-expanded={open} + aria-busy={loadingStatus} > - {currentProvider && } - {currentModelLabel} + {loadingStatus ? ( + <> +