From c18adc90c33b45577422de4929edb9034e854941 Mon Sep 17 00:00:00 2001
From: Tam Nhu Tran <kaitran.ntt@gmail.com>
Date: Wed, 18 Feb 2026 04:18:57 +0700
Subject: [PATCH] fix(ui): harden cliproxy panel and proxy edge handling

- make iframe auto-login/reload flow race-safe and lint-compliant

- normalize API base joins and replace conflict sentinels with typed errors

- tighten proxy port validation and defer cursor preset apply until models load

- add ui api-client unit coverage for path and conflict helpers
---
 tests/unit/ui-api-client.test.ts              | 32 +++++++
 .../cliproxy/control-panel-embed.tsx          | 94 ++++++++++---------
 .../config-form/use-copilot-config-form.ts    |  3 +-
 ui/src/hooks/use-copilot.ts                   |  4 +-
 ui/src/hooks/use-cursor.ts                    |  4 +-
 ui/src/lib/api-client.ts                      | 76 ++++++++++++++-
 ui/src/pages/cursor.tsx                       | 19 +++-
 .../pages/settings/sections/proxy/index.tsx   | 37 ++++++--
 .../sections/proxy/local-proxy-card.tsx       |  5 +-
 9 files changed, 210 insertions(+), 64 deletions(-)
 create mode 100644 tests/unit/ui-api-client.test.ts

diff --git a/tests/unit/ui-api-client.test.ts b/tests/unit/ui-api-client.test.ts
new file mode 100644
index 00000000..c5edc307
--- /dev/null
+++ b/tests/unit/ui-api-client.test.ts
@@ -0,0 +1,32 @@
+import { describe, expect, it } from 'bun:test';
+import {
+  API_BASE_URL,
+  API_CONFLICT_ERROR_CODE,
+  ApiConflictError,
+  isApiConflictError,
+  withApiBase,
+} from '../../ui/src/lib/api-client';
+
+describe('ui api-client helpers', () => {
+  it('normalizes relative paths with API base prefix', () => {
+    expect(withApiBase('/cliproxy/status')).toBe('/api/cliproxy/status');
+    expect(withApiBase('cliproxy/status')).toBe('/api/cliproxy/status');
+  });
+
+  it('preserves paths that already include API base', () => {
+    expect(withApiBase('/api/cliproxy/status')).toBe('/api/cliproxy/status');
+    expect(withApiBase('/api')).toBe('/api');
+  });
+
+  it('handles empty and absolute URLs safely', () => {
+    expect(withApiBase('')).toBe(API_BASE_URL);
+    expect(withApiBase('https://example.com/api')).toBe('https://example.com/api');
+  });
+
+  it('identifies typed API conflict errors', () => {
+    const conflict = new ApiConflictError('conflict');
+    expect(conflict.code).toBe(API_CONFLICT_ERROR_CODE);
+    expect(isApiConflictError(conflict)).toBe(true);
+    expect(isApiConflictError(new Error('plain'))).toBe(false);
+  });
+});
diff --git a/ui/src/components/cliproxy/control-panel-embed.tsx b/ui/src/components/cliproxy/control-panel-embed.tsx
index e29ea502..a92125b0 100644
--- a/ui/src/components/cliproxy/control-panel-embed.tsx
+++ b/ui/src/components/cliproxy/control-panel-embed.tsx
@@ -9,7 +9,7 @@
 import { useState, useEffect, useRef, useCallback, useMemo } from 'react';
 import { RefreshCw, AlertCircle, Key, X, Gauge, Globe, Settings } from 'lucide-react';
 import { useQuery } from '@tanstack/react-query';
-import { api } from '@/lib/api-client';
+import { api, withApiBase } from '@/lib/api-client';
 import type { CliproxyServerConfig } from '@/lib/api-client';
 import { CLIPROXY_DEFAULT_PORT } from '@/lib/preset-utils';
 
@@ -24,7 +24,8 @@ interface ControlPanelEmbedProps {
 
 export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanelEmbedProps) {
   const iframeRef = useRef<HTMLIFrameElement>(null);
-  const [isLoading, setIsLoading] = useState(true);
+  const [loadedUrl, setLoadedUrl] = useState<string | null>(null);
+  const [iframeRevision, setIframeRevision] = useState(0);
   const [error, setError] = useState<string | null>(null);
   const [isConnected, setIsConnected] = useState(false);
   const [showLoginHint, setShowLoginHint] = useState(true);
@@ -40,7 +41,7 @@ export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanel
   const { data: authTokens } = useQuery<AuthTokensResponse>({
     queryKey: ['auth-tokens-raw'],
     queryFn: async () => {
-      const response = await fetch('/api/settings/auth/tokens/raw');
+      const response = await fetch(withApiBase('/settings/auth/tokens/raw'));
       if (!response.ok) throw new Error('Failed to fetch auth tokens');
       return response.json();
     },
@@ -60,8 +61,8 @@ export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanel
 
     if (remote?.enabled && remote?.host) {
       const protocol = remote.protocol || 'http';
-      // Use port from config, or default based on protocol (443 for https, 80 for http)
-      const remotePort = remote.port || (protocol === 'https' ? 443 : 80);
+      // Use port from config, or default based on protocol (443 for https, 8317 for http)
+      const remotePort = remote.port || (protocol === 'https' ? 443 : CLIPROXY_DEFAULT_PORT);
       // Only include port in URL if it's non-standard
       const portSuffix =
         (protocol === 'https' && remotePort === 443) || (protocol === 'http' && remotePort === 80)
@@ -89,6 +90,9 @@ export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanel
     };
   }, [cliproxyConfig, authTokens, port]);
 
+  const iframeLoaded = loadedUrl === managementUrl;
+  const isLoading = !iframeLoaded;
+
   // Check if CLIProxy is running
   useEffect(() => {
     const controller = new AbortController();
@@ -130,48 +134,53 @@ export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanel
     return () => controller.abort();
   }, [checkUrl, isRemote, displayHost]);
 
-  // Handle iframe load - attempt to auto-login via postMessage
-  const handleIframeLoad = useCallback(() => {
-    setIsLoading(false);
-
-    // Try to inject credentials via postMessage
-    // The management.html needs to listen for this message
-    // If it doesn't support it, user will see the login page
-    if (iframeRef.current?.contentWindow && authToken) {
-      try {
-        // Derive apiBase from checkUrl (remove trailing slash)
-        const apiBase = checkUrl.replace(/\/$/, '');
-
-        // Security: Validate iframe src matches target origin before sending credentials
-        const iframeSrc = iframeRef.current.src;
-        if (!iframeSrc.startsWith(apiBase)) {
-          console.warn('[ControlPanelEmbed] Iframe origin mismatch, skipping postMessage');
-          return;
-        }
-
-        // Send credentials to iframe
-        iframeRef.current.contentWindow.postMessage(
-          {
-            type: 'ccs-auto-login',
-            apiBase,
-            managementKey: authToken,
-          },
-          apiBase
-        );
-      } catch (e) {
-        // Cross-origin restriction - expected if not same origin
-        console.debug('[ControlPanelEmbed] postMessage failed - cross-origin:', e);
-      }
+  const postAutoLoginCredentials = useCallback(() => {
+    // Auto-login can only run when iframe has loaded and authToken is available.
+    if (!iframeLoaded || !iframeRef.current?.contentWindow || !authToken) {
+      return;
     }
-  }, [checkUrl, authToken]);
+
+    try {
+      // Derive apiBase from checkUrl (remove trailing slash)
+      const apiBase = checkUrl.replace(/\/$/, '');
+
+      // Security: Validate iframe src matches target origin before sending credentials
+      const iframeSrc = iframeRef.current.src;
+      if (!iframeSrc.startsWith(apiBase)) {
+        console.warn('[ControlPanelEmbed] Iframe origin mismatch, skipping postMessage');
+        return;
+      }
+
+      // Send credentials to iframe
+      iframeRef.current.contentWindow.postMessage(
+        {
+          type: 'ccs-auto-login',
+          apiBase,
+          managementKey: authToken,
+        },
+        apiBase
+      );
+    } catch (e) {
+      // Cross-origin restriction - expected if not same origin
+      console.debug('[ControlPanelEmbed] postMessage failed - cross-origin:', e);
+    }
+  }, [authToken, checkUrl, iframeLoaded]);
+
+  // Retry auto-login when token/checkUrl arrive after iframe onLoad.
+  useEffect(() => {
+    postAutoLoginCredentials();
+  }, [postAutoLoginCredentials]);
+
+  // Handle iframe load - mark ready then let effect post credentials.
+  const handleIframeLoad = useCallback(() => {
+    setLoadedUrl(managementUrl);
+  }, [managementUrl]);
 
   const handleRefresh = () => {
-    setIsLoading(true);
+    setLoadedUrl(null);
+    setIframeRevision((value) => value + 1);
     setError(null);
     setIsConnected(false);
-    if (iframeRef.current) {
-      iframeRef.current.src = managementUrl;
-    }
   };
 
   // Show error state if CLIProxy is not running
@@ -264,6 +273,7 @@ export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanel
 
       {/* Iframe */}
       <iframe
+        key={`${managementUrl}:${iframeRevision}`}
         ref={iframeRef}
         src={managementUrl}
         className="flex-1 w-full border-0"
diff --git a/ui/src/components/copilot/config-form/use-copilot-config-form.ts b/ui/src/components/copilot/config-form/use-copilot-config-form.ts
index 1c91cc8b..e507d8a4 100644
--- a/ui/src/components/copilot/config-form/use-copilot-config-form.ts
+++ b/ui/src/components/copilot/config-form/use-copilot-config-form.ts
@@ -5,6 +5,7 @@
 
 import { useState, useMemo, useCallback } from 'react';
 import { useCopilot } from '@/hooks/use-copilot';
+import { isApiConflictError } from '@/lib/api-client';
 import { toast } from 'sonner';
 import type { ModelPreset } from './types';
 
@@ -171,7 +172,7 @@ export function useCopilotConfigForm() {
       setLocalOverrides({});
       setRawJsonEdits(null);
     } catch (error) {
-      if ((error as Error).message === 'CONFLICT') {
+      if (isApiConflictError(error)) {
         setConflictDialog(true);
       } else {
         toast.error('Failed to save settings');
diff --git a/ui/src/hooks/use-copilot.ts b/ui/src/hooks/use-copilot.ts
index 06e8c84d..dca6f58c 100644
--- a/ui/src/hooks/use-copilot.ts
+++ b/ui/src/hooks/use-copilot.ts
@@ -6,7 +6,7 @@
 
 import { useMemo } from 'react';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
-import { withApiBase } from '@/lib/api-client';
+import { ApiConflictError, withApiBase } from '@/lib/api-client';
 
 // Types
 export interface CopilotStatus {
@@ -121,7 +121,7 @@ async function saveCopilotRawSettings(data: {
     headers: { 'Content-Type': 'application/json' },
     body: JSON.stringify(data),
   });
-  if (res.status === 409) throw new Error('CONFLICT');
+  if (res.status === 409) throw new ApiConflictError('Copilot raw settings changed externally');
   if (!res.ok) throw new Error('Failed to save copilot raw settings');
   return res.json();
 }
diff --git a/ui/src/hooks/use-cursor.ts b/ui/src/hooks/use-cursor.ts
index 2beca36f..8e34c6a7 100644
--- a/ui/src/hooks/use-cursor.ts
+++ b/ui/src/hooks/use-cursor.ts
@@ -6,7 +6,7 @@
 
 import { useMemo } from 'react';
 import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query';
-import { withApiBase } from '@/lib/api-client';
+import { ApiConflictError, withApiBase } from '@/lib/api-client';
 
 export interface CursorStatus {
   enabled: boolean;
@@ -102,7 +102,7 @@ async function saveCursorRawSettings(data: {
     headers: { 'Content-Type': 'application/json' },
     body: JSON.stringify(data),
   });
-  if (res.status === 409) throw new Error('CONFLICT');
+  if (res.status === 409) throw new ApiConflictError('Cursor raw settings changed externally');
   if (!res.ok) throw new Error('Failed to save cursor raw settings');
   return res.json();
 }
diff --git a/ui/src/lib/api-client.ts b/ui/src/lib/api-client.ts
index f3550685..405fa210 100644
--- a/ui/src/lib/api-client.ts
+++ b/ui/src/lib/api-client.ts
@@ -6,9 +6,60 @@
 import type { CLIProxyProvider } from './provider-config';
 
 export const API_BASE_URL = '/api';
+export const API_CONFLICT_ERROR_CODE = 'CONFLICT';
+
+export class ApiConflictError extends Error {
+  readonly code = API_CONFLICT_ERROR_CODE;
+
+  constructor(message = 'Resource modified externally') {
+    super(message);
+    this.name = 'ApiConflictError';
+  }
+}
+
+export function isApiConflictError(error: unknown): error is Error & { code: string } {
+  return (
+    error instanceof Error &&
+    'code' in error &&
+    (error as { code?: unknown }).code === API_CONFLICT_ERROR_CODE
+  );
+}
 
 export function withApiBase(path: string): string {
-  return `${API_BASE_URL}${path}`;
+  if (!path) {
+    return API_BASE_URL;
+  }
+
+  if (/^https?:\/\//i.test(path)) {
+    return path;
+  }
+
+  if (path === API_BASE_URL || path.startsWith(`${API_BASE_URL}/`)) {
+    return path;
+  }
+
+  return `${API_BASE_URL}${path.startsWith('/') ? path : `/${path}`}`;
+}
+
+async function parseErrorMessage(response: Response): Promise<string> {
+  const fallbackMessage = `Request failed (${response.status}${response.statusText ? ` ${response.statusText}` : ''})`;
+  const bodyText = await response.text();
+  if (!bodyText) {
+    return fallbackMessage;
+  }
+
+  try {
+    const parsed = JSON.parse(bodyText) as { error?: string; message?: string };
+    if (parsed.error?.trim()) {
+      return parsed.error;
+    }
+    if (parsed.message?.trim()) {
+      return parsed.message;
+    }
+    return fallbackMessage;
+  } catch {
+    return bodyText.trim() || fallbackMessage;
+  }
 }
 
 async function request<T>(url: string, options?: RequestInit): Promise<T> {
@@ -18,11 +69,28 @@ async function request<T>(url: string, options?: RequestInit): Promise<T> {
   });
 
   if (!res.ok) {
-    const error = await res.json().catch(() => ({ error: 'Unknown error' }));
-    throw new Error(error.error || res.statusText);
+    throw new Error(await parseErrorMessage(res));
   }
 
-  return res.json();
+  if (res.status === 204) {
+    return undefined as T;
+  }
+
+  const contentType = res.headers.get('content-type')?.toLowerCase() ?? '';
+  if (contentType.includes('application/json')) {
+    return (await res.json()) as T;
+  }
+
+  const bodyText = await res.text();
+  if (!bodyText) {
+    return undefined as T;
+  }
+
+  try {
+    return JSON.parse(bodyText) as T;
+  } catch {
+    return bodyText as T;
+  }
 }
 
 // Types
diff --git a/ui/src/pages/cursor.tsx b/ui/src/pages/cursor.tsx
index ec2da394..fcc0e08e 100644
--- a/ui/src/pages/cursor.tsx
+++ b/ui/src/pages/cursor.tsx
@@ -25,6 +25,7 @@ import {
 import { cn } from '@/lib/utils';
 import { useCursor } from '@/hooks/use-cursor';
 import { DEFAULT_CURSOR_PORT } from '@/lib/default-ports';
+import { isApiConflictError } from '@/lib/api-client';
 import { Button } from '@/components/ui/button';
 import { Input } from '@/components/ui/input';
 import { Label } from '@/components/ui/label';
@@ -391,6 +392,16 @@ export function CursorPage() {
   };
 
   const applyPreset = (preset: 'codex53' | 'claude46' | 'gemini3') => {
+    if (modelsLoading) {
+      toast.error('Models are still loading. Please wait before applying a preset.');
+      return;
+    }
+
+    if (models.length === 0) {
+      toast.error('No models available yet. Start the daemon and refresh first.');
+      return;
+    }
+
     const fallbackModel = effectiveModel || currentModel || models[0]?.id || 'gpt-5.3-codex';
     const codex53 = pickModelByAliases(
       models,
@@ -560,11 +571,10 @@ export function CursorPage() {
       }
       return true;
     } catch (error) {
-      const message = (error as Error).message || 'Failed to save raw settings';
-      if (message === 'CONFLICT') {
+      if (isApiConflictError(error)) {
         toast.error('Raw settings changed externally. Refresh and retry.');
       } else {
-        toast.error(message);
+        toast.error((error as Error).message || 'Failed to save raw settings');
       }
       return false;
     }
@@ -868,6 +878,7 @@ export function CursorPage() {
                                 size="sm"
                                 className="text-xs h-7 gap-1"
                                 onClick={() => applyPreset('codex53')}
+                                disabled={modelsLoading || models.length === 0}
                                 title="OpenAI-only mapping: GPT-5.3 Codex / Codex Max / GPT-5 Mini"
                               >
                                 <Zap className="w-3 h-3" />
@@ -878,6 +889,7 @@ export function CursorPage() {
                                 size="sm"
                                 className="text-xs h-7 gap-1"
                                 onClick={() => applyPreset('claude46')}
+                                disabled={modelsLoading || models.length === 0}
                                 title="Claude-first mapping: Opus 4.6 / Sonnet 4.5 / Haiku 4.5"
                               >
                                 <Zap className="w-3 h-3" />
@@ -888,6 +900,7 @@ export function CursorPage() {
                                 size="sm"
                                 className="text-xs h-7 gap-1"
                                 onClick={() => applyPreset('gemini3')}
+                                disabled={modelsLoading || models.length === 0}
                                 title="Gemini-first mapping: Gemini 3 Pro + Gemini 3 Flash"
                               >
                                 <Zap className="w-3 h-3" />
diff --git a/ui/src/pages/settings/sections/proxy/index.tsx b/ui/src/pages/settings/sections/proxy/index.tsx
index e81062c1..5972f1fa 100644
--- a/ui/src/pages/settings/sections/proxy/index.tsx
+++ b/ui/src/pages/settings/sections/proxy/index.tsx
@@ -25,6 +25,7 @@ import { RemoteProxyCard } from './remote-proxy-card';
 import { ProxyStatusWidget } from '@/components/monitoring/proxy-status-widget';
 import { api } from '@/lib/api-client';
 import { CLIPROXY_DEFAULT_PORT } from '@/lib/preset-utils';
+import { toast } from 'sonner';
 
 /** LocalStorage key for debug mode preference */
 const DEBUG_MODE_KEY = 'ccs_debug_mode';
@@ -184,12 +185,24 @@ export default function ProxySection() {
   };
 
   const savePort = () => {
-    const portStr = editedPort ?? displayPort;
-    const port = portStr === '' ? undefined : parseInt(portStr, 10);
-    const effectivePort = port && !isNaN(port) && port > 0 ? port : undefined;
+    const portStr = (editedPort ?? displayPort).trim();
+    if (portStr === '') {
+      if (config.remote.port !== undefined) {
+        saveConfig({ remote: { ...remoteConfig, port: undefined } });
+      }
+      setEditedPort(null);
+      return;
+    }
 
-    if (effectivePort !== config.remote.port) {
-      saveConfig({ remote: { ...remoteConfig, port: effectivePort } });
+    const parsedPort = Number(portStr);
+    if (!Number.isInteger(parsedPort) || parsedPort < 1 || parsedPort > 65535) {
+      toast.error('Port must be an integer between 1 and 65535, or empty for default');
+      setEditedPort(null);
+      return;
+    }
+
+    if (parsedPort !== config.remote.port) {
+      saveConfig({ remote: { ...remoteConfig, port: parsedPort } });
     }
     setEditedPort(null);
   };
@@ -211,9 +224,17 @@ export default function ProxySection() {
   };
 
   const saveLocalPort = () => {
-    const port = parseInt(editedLocalPort ?? displayLocalPort, 10);
-    if (!isNaN(port) && port !== config.local.port) {
-      saveConfig({ local: { ...config.local, port } });
+    const localPortStr = (editedLocalPort ?? displayLocalPort).trim();
+    const parsedPort = localPortStr === '' ? CLIPROXY_DEFAULT_PORT : Number(localPortStr);
+
+    if (!Number.isInteger(parsedPort) || parsedPort < 1 || parsedPort > 65535) {
+      toast.error('Local port must be an integer between 1 and 65535');
+      setEditedLocalPort(null);
+      return;
+    }
+
+    if (parsedPort !== config.local.port) {
+      saveConfig({ local: { ...config.local, port: parsedPort } });
     }
     setEditedLocalPort(null);
   };
diff --git a/ui/src/pages/settings/sections/proxy/local-proxy-card.tsx b/ui/src/pages/settings/sections/proxy/local-proxy-card.tsx
index e40fe168..9f942f4c 100644
--- a/ui/src/pages/settings/sections/proxy/local-proxy-card.tsx
+++ b/ui/src/pages/settings/sections/proxy/local-proxy-card.tsx
@@ -35,9 +35,10 @@ export function LocalProxyCard({
         <div className="flex items-center gap-2">
           <label className="text-sm text-muted-foreground">Port</label>
           <Input
-            type="number"
+            type="text"
+            inputMode="numeric"
             value={displayLocalPort}
-            onChange={(e) => setEditedLocalPort(e.target.value)}
+            onChange={(e) => setEditedLocalPort(e.target.value.replace(/\D/g, ''))}
             onBlur={onSaveLocalPort}
             placeholder={`${CLIPROXY_DEFAULT_PORT}`}
             className="font-mono max-w-32"