From ca490a9f4e96dd2da7e6c76b466328ca4aa4dc6c Mon Sep 17 00:00:00 2001
From: kaitranntt <kaitran.ntt@gmail.com>
Date: Wed, 21 Jan 2026 17:52:16 -0500
Subject: [PATCH] fix(cliproxy): handle edge cases in thinking validation

- model-catalog: add null guard and trim for modelId lookup
- thinking-validator: add 100 char length limit, accept "8192.0" format
- cliproxy-executor: error on empty --thinking= value
- use-thinking-config: handle HTML error pages, return cleanup fn
---
 src/cliproxy/cliproxy-executor.ts              |  7 +++++++
 src/cliproxy/model-catalog.ts                  |  4 ++--
 src/cliproxy/thinking-validator.ts             | 18 +++++++++++++++++-
 .../settings/hooks/use-thinking-config.ts      | 17 ++++++++++++++++-
 4 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/src/cliproxy/cliproxy-executor.ts b/src/cliproxy/cliproxy-executor.ts
index 942efe82..f63ff81b 100644
--- a/src/cliproxy/cliproxy-executor.ts
+++ b/src/cliproxy/cliproxy-executor.ts
@@ -337,6 +337,13 @@ export async function execClaudeWithCLIProxy(
   const thinkingEqArg = argsWithoutProxy.find((arg) => arg.startsWith('--thinking='));
   if (thinkingEqArg) {
     const val = thinkingEqArg.substring('--thinking='.length);
+    // Handle empty value after equals (--thinking=)
+    if (!val || val.trim() === '') {
+      console.error(fail('--thinking requires a value'));
+      console.error('    Examples: --thinking=low, --thinking=8192, --thinking=off');
+      console.error('    Levels: minimal, low, medium, high, xhigh, auto');
+      process.exit(1);
+    }
     // Parse as number if numeric, otherwise keep as string (level name)
     const numVal = parseInt(val, 10);
     thinkingOverride = !isNaN(numVal) ? numVal : val;
diff --git a/src/cliproxy/model-catalog.ts b/src/cliproxy/model-catalog.ts
index b4c17898..78c86d38 100644
--- a/src/cliproxy/model-catalog.ts
+++ b/src/cliproxy/model-catalog.ts
@@ -188,8 +188,8 @@ export function getProviderCatalog(provider: CLIProxyProvider): ProviderCatalog
  */
 export function findModel(provider: CLIProxyProvider, modelId: string): ModelEntry | undefined {
   const catalog = MODEL_CATALOG[provider];
-  if (!catalog) return undefined;
-  const normalizedId = modelId.toLowerCase();
+  if (!catalog || !modelId) return undefined;
+  const normalizedId = modelId.trim().toLowerCase();
   return catalog.models.find((m) => m.id.toLowerCase() === normalizedId);
 }
 
diff --git a/src/cliproxy/thinking-validator.ts b/src/cliproxy/thinking-validator.ts
index c99e424e..29a2a7d3 100644
--- a/src/cliproxy/thinking-validator.ts
+++ b/src/cliproxy/thinking-validator.ts
@@ -139,6 +139,15 @@ export function validateThinking(
 ): ThinkingValidationResult {
   const thinking = getModelThinkingSupport(provider, modelId);
 
+  // Handle string length limit to prevent performance issues
+  if (typeof value === 'string' && value.length > 100) {
+    return {
+      valid: false,
+      value: 'off',
+      warning: 'Thinking value too long (max 100 chars). Using "off".',
+    };
+  }
+
   // Handle empty string explicitly
   if (typeof value === 'string' && value.trim() === '') {
     return {
@@ -221,7 +230,14 @@ function validateBudgetThinking(
     } else {
       // Strict number parsing: reject partial parses like "123abc"
       const parsed = Number(value);
-      if (isNaN(parsed) || !Number.isFinite(parsed) || value.trim() !== String(parsed)) {
+      const trimmed = value.trim();
+      // Accept trailing .0 for whole numbers (e.g., "8192.0" → 8192)
+      const isWholeWithDecimal = Number.isInteger(parsed) && trimmed === `${parsed}.0`;
+      if (
+        isNaN(parsed) ||
+        !Number.isFinite(parsed) ||
+        (trimmed !== String(parsed) && !isWholeWithDecimal)
+      ) {
         // Not a valid number, try to find closest level
         const closest = findClosestLevel(value, Object.keys(THINKING_LEVEL_BUDGETS));
         if (closest) {
diff --git a/ui/src/pages/settings/hooks/use-thinking-config.ts b/ui/src/pages/settings/hooks/use-thinking-config.ts
index 1c686ff6..4decf2f9 100644
--- a/ui/src/pages/settings/hooks/use-thinking-config.ts
+++ b/ui/src/pages/settings/hooks/use-thinking-config.ts
@@ -37,7 +37,14 @@ export function useThinkingConfig() {
       setError(null);
       const res = await fetch('/api/thinking', { signal: controller.signal });
       clearTimeout(timeoutId);
-      if (!res.ok) throw new Error('Failed to load Thinking config');
+      if (!res.ok) {
+        // Handle HTML error pages gracefully
+        const contentType = res.headers.get('content-type');
+        if (contentType?.includes('text/html')) {
+          throw new Error(`Server error (${res.status})`);
+        }
+        throw new Error('Failed to load Thinking config');
+      }
       const data = await res.json();
       setConfig(data.config || DEFAULT_THINKING_CONFIG);
       // W4: Store lastModified for optimistic locking
@@ -53,6 +60,9 @@ export function useThinkingConfig() {
     } finally {
       setLoading(false);
     }
+
+    // Return cleanup function for AbortController
+    return () => controller.abort();
   }, []);
 
   const saveConfig = useCallback(
@@ -84,6 +94,11 @@ export function useThinkingConfig() {
         clearTimeout(timeoutId);
 
         if (!res.ok) {
+          // Handle HTML error pages gracefully
+          const contentType = res.headers.get('content-type');
+          if (contentType?.includes('text/html')) {
+            throw new Error(`Server error (${res.status})`);
+          }
           const data = await res.json();
           // W4: Handle conflict (409) with user-friendly message
           if (res.status === 409) {