chore: refactoring and tests (#12629)

4 files changed, 266 insertions, 30 deletions

diff --git a/packages/app/src/utils/persist.test.ts b/packages/app/src/utils/persist.test.ts
new file mode 100644
index 000000000..5be68f3b0
--- /dev/null
+++ b/packages/app/src/utils/persist.test.ts
@@ -0,0 +1,102 @@
+import { beforeAll, beforeEach, describe, expect, mock, test } from "bun:test"
+
+type PersistTestingType = typeof import("./persist").PersistTesting
+
+class MemoryStorage implements Storage {
+  private values = new Map<string, string>()
+  readonly events: string[] = []
+  readonly calls = { get: 0, set: 0, remove: 0 }
+
+  clear() {
+    this.values.clear()
+  }
+
+  get length() {
+    return this.values.size
+  }
+
+  key(index: number) {
+    return Array.from(this.values.keys())[index] ?? null
+  }
+
+  getItem(key: string) {
+    this.calls.get += 1
+    this.events.push(`get:${key}`)
+    if (key.startsWith("opencode.throw")) throw new Error("storage get failed")
+    return this.values.get(key) ?? null
+  }
+
+  setItem(key: string, value: string) {
+    this.calls.set += 1
+    this.events.push(`set:${key}`)
+    if (key.startsWith("opencode.quota")) throw new DOMException("quota", "QuotaExceededError")
+    if (key.startsWith("opencode.throw")) throw new Error("storage set failed")
+    this.values.set(key, value)
+  }
+
+  removeItem(key: string) {
+    this.calls.remove += 1
+    this.events.push(`remove:${key}`)
+    if (key.startsWith("opencode.throw")) throw new Error("storage remove failed")
+    this.values.delete(key)
+  }
+}
+
+const storage = new MemoryStorage()
+
+let persistTesting: PersistTestingType
+
+beforeAll(async () => {
+  mock.module("@/context/platform", () => ({
+    usePlatform: () => ({ platform: "web" }),
+  }))
+
+  const mod = await import("./persist")
+  persistTesting = mod.PersistTesting
+})
+
+beforeEach(() => {
+  storage.clear()
+  storage.events.length = 0
+  storage.calls.get = 0
+  storage.calls.set = 0
+  storage.calls.remove = 0
+  Object.defineProperty(globalThis, "localStorage", {
+    value: storage,
+    configurable: true,
+  })
+})
+
+describe("persist localStorage resilience", () => {
+  test("does not cache values as persisted when quota write and eviction fail", () => {
+    const storageApi = persistTesting.localStorageWithPrefix("opencode.quota.scope")
+    storageApi.setItem("value", '{"value":1}')
+
+    expect(storage.getItem("opencode.quota.scope:value")).toBeNull()
+    expect(storageApi.getItem("value")).toBeNull()
+  })
+
+  test("disables only the failing scope when storage throws", () => {
+    const bad = persistTesting.localStorageWithPrefix("opencode.throw.scope")
+    bad.setItem("value", '{"value":1}')
+
+    const before = storage.calls.set
+    bad.setItem("value", '{"value":2}')
+    expect(storage.calls.set).toBe(before)
+    expect(bad.getItem("value")).toBeNull()
+
+    const healthy = persistTesting.localStorageWithPrefix("opencode.safe.scope")
+    healthy.setItem("value", '{"value":3}')
+    expect(storage.getItem("opencode.safe.scope:value")).toBe('{"value":3}')
+  })
+
+  test("failing fallback scope does not poison direct storage scope", () => {
+    const broken = persistTesting.localStorageWithPrefix("opencode.throw.scope2")
+    broken.setItem("value", '{"value":1}')
+
+    const direct = persistTesting.localStorageDirect()
+    direct.setItem("direct-value", '{"value":5}')
+
+    expect(storage.getItem("direct-value")).toBe('{"value":5}')
+  })
+})
diff --git a/packages/app/src/utils/persist.ts b/packages/app/src/utils/persist.ts
index 0ca3abad0..329a406dd 100644
--- a/packages/app/src/utils/persist.ts
+++ b/packages/app/src/utils/persist.ts
@@ -17,7 +17,7 @@ type PersistTarget = {
 const LEGACY_STORAGE = "default.dat"
 const GLOBAL_STORAGE = "opencode.global.dat"
 const LOCAL_PREFIX = "opencode."
-const fallback = { disabled: false }
+const fallback = new Map<string, boolean>()
 
 const CACHE_MAX_ENTRIES = 500
 const CACHE_MAX_BYTES = 8 * 1024 * 1024
@@ -65,6 +65,14 @@ function cacheGet(key: string) {
   return entry.value
 }
 
+function fallbackDisabled(scope: string) {
+  return fallback.get(scope) === true
+}
+
+function fallbackSet(scope: string) {
+  fallback.set(scope, true)
+}
+
 function quota(error: unknown) {
   if (error instanceof DOMException) {
     if (error.name === "QuotaExceededError") return true
@@ -142,7 +150,6 @@ function write(storage: Storage, key: string, value: string) {
   }
 
   const ok = evict(storage, key, value)
-  if (!ok) cacheSet(key, value)
   return ok
 }
 
@@ -196,18 +203,19 @@ function workspaceStorage(dir: string) {
 
 function localStorageWithPrefix(prefix: string): SyncStorage {
   const base = `${prefix}:`
+  const scope = `prefix:${prefix}`
   const item = (key: string) => base + key
   return {
     getItem: (key) => {
       const name = item(key)
       const cached = cacheGet(name)
-      if (fallback.disabled && cached !== undefined) return cached
+      if (fallbackDisabled(scope)) return cached ?? null
 
       const stored = (() => {
         try {
           return localStorage.getItem(name)
         } catch {
-          fallback.disabled = true
+          fallbackSet(scope)
           return null
         }
       })()
@@ -217,40 +225,40 @@ function localStorageWithPrefix(prefix: string): SyncStorage {
     },
     setItem: (key, value) => {
       const name = item(key)
-      cacheSet(name, value)
-      if (fallback.disabled) return
+      if (fallbackDisabled(scope)) return
       try {
         if (write(localStorage, name, value)) return
       } catch {
-        fallback.disabled = true
+        fallbackSet(scope)
         return
       }
-      fallback.disabled = true
+      fallbackSet(scope)
     },
     removeItem: (key) => {
       const name = item(key)
       cacheDelete(name)
-      if (fallback.disabled) return
+      if (fallbackDisabled(scope)) return
       try {
         localStorage.removeItem(name)
       } catch {
-        fallback.disabled = true
+        fallbackSet(scope)
       }
     },
   }
 }
 
 function localStorageDirect(): SyncStorage {
+  const scope = "direct"
   return {
     getItem: (key) => {
       const cached = cacheGet(key)
-      if (fallback.disabled && cached !== undefined) return cached
+      if (fallbackDisabled(scope)) return cached ?? null
 
       const stored = (() => {
         try {
           return localStorage.getItem(key)
         } catch {
-          fallback.disabled = true
+          fallbackSet(scope)
           return null
         }
       })()
@@ -259,28 +267,32 @@ function localStorageDirect(): SyncStorage {
       return stored
     },
     setItem: (key, value) => {
-      cacheSet(key, value)
-      if (fallback.disabled) return
+      if (fallbackDisabled(scope)) return
       try {
         if (write(localStorage, key, value)) return
       } catch {
-        fallback.disabled = true
+        fallbackSet(scope)
         return
       }
-      fallback.disabled = true
+      fallbackSet(scope)
     },
     removeItem: (key) => {
       cacheDelete(key)
-      if (fallback.disabled) return
+      if (fallbackDisabled(scope)) return
       try {
         localStorage.removeItem(key)
       } catch {
-        fallback.disabled = true
+        fallbackSet(scope)
       }
     },
   }
 }
 
+export const PersistTesting = {
+  localStorageDirect,
+  localStorageWithPrefix,
+}
+
 export const Persist = {
   global(key: string, legacy?: string[]): PersistTarget {
     return { storage: GLOBAL_STORAGE, key, legacy }
diff --git a/packages/app/src/utils/server-health.test.ts b/packages/app/src/utils/server-health.test.ts
index 34c86685a..26bda070a 100644
--- a/packages/app/src/utils/server-health.test.ts
+++ b/packages/app/src/utils/server-health.test.ts
@@ -1,6 +1,12 @@
 import { describe, expect, test } from "bun:test"
 import { checkServerHealth } from "./server-health"
 
+function abortFromInput(input: RequestInfo | URL, init?: RequestInit) {
+  if (init?.signal) return init.signal
+  if (input instanceof Request) return input.signal
+  return undefined
+}
+
 describe("checkServerHealth", () => {
   test("returns healthy response with version", async () => {
     const fetch = (async () =>
@@ -24,10 +30,40 @@ describe("checkServerHealth", () => {
     expect(result).toEqual({ healthy: false })
   })
 
+  test("uses timeout fallback when AbortSignal.timeout is unavailable", async () => {
+    const timeout = Object.getOwnPropertyDescriptor(AbortSignal, "timeout")
+    Object.defineProperty(AbortSignal, "timeout", {
+      configurable: true,
+      value: undefined,
+    })
+
+    let aborted = false
+    const fetch = ((input: RequestInfo | URL, init?: RequestInit) =>
+      new Promise<Response>((_resolve, reject) => {
+        const signal = abortFromInput(input, init)
+        signal?.addEventListener(
+          "abort",
+          () => {
+            aborted = true
+            reject(new DOMException("Aborted", "AbortError"))
+          },
+          { once: true },
+        )
+      })) as unknown as typeof globalThis.fetch
+
+    const result = await checkServerHealth("http://localhost:4096", fetch, { timeoutMs: 10 }).finally(() => {
+      if (timeout) Object.defineProperty(AbortSignal, "timeout", timeout)
+      if (!timeout) Reflect.deleteProperty(AbortSignal, "timeout")
+    })
+
+    expect(aborted).toBe(true)
+    expect(result).toEqual({ healthy: false })
+  })
+
   test("uses provided abort signal", async () => {
     let signal: AbortSignal | undefined
     const fetch = (async (input: RequestInfo | URL, init?: RequestInit) => {
-      signal = init?.signal ?? (input instanceof Request ? input.signal : undefined)
+      signal = abortFromInput(input, init)
       return new Response(JSON.stringify({ healthy: true, version: "1.2.3" }), {
         status: 200,
         headers: { "content-type": "application/json" },
@@ -39,4 +75,40 @@ describe("checkServerHealth", () => {
 
     expect(signal).toBe(abort.signal)
   })
+
+  test("retries transient failures and eventually succeeds", async () => {
+    let count = 0
+    const fetch = (async () => {
+      count += 1
+      if (count < 3) throw new TypeError("network")
+      return new Response(JSON.stringify({ healthy: true, version: "1.2.3" }), {
+        status: 200,
+        headers: { "content-type": "application/json" },
+      })
+    }) as unknown as typeof globalThis.fetch
+
+    const result = await checkServerHealth("http://localhost:4096", fetch, {
+      retryCount: 2,
+      retryDelayMs: 1,
+    })
+
+    expect(count).toBe(3)
+    expect(result).toEqual({ healthy: true, version: "1.2.3" })
+  })
+
+  test("returns unhealthy when retries are exhausted", async () => {
+    let count = 0
+    const fetch = (async () => {
+      count += 1
+      throw new TypeError("network")
+    }) as unknown as typeof globalThis.fetch
+
+    const result = await checkServerHealth("http://localhost:4096", fetch, {
+      retryCount: 2,
+      retryDelayMs: 1,
+    })
+
+    expect(count).toBe(3)
+    expect(result).toEqual({ healthy: false })
+  })
 })
diff --git a/packages/app/src/utils/server-health.ts b/packages/app/src/utils/server-health.ts
index ab33460b2..929826d0d 100644
--- a/packages/app/src/utils/server-health.ts
+++ b/packages/app/src/utils/server-health.ts
@@ -5,10 +5,50 @@ export type ServerHealth = { healthy: boolean; version?: string }
 interface CheckServerHealthOptions {
   timeoutMs?: number
   signal?: AbortSignal
+  retryCount?: number
+  retryDelayMs?: number
 }
 
+const defaultTimeoutMs = 3000
+const defaultRetryCount = 2
+const defaultRetryDelayMs = 100
+
 function timeoutSignal(timeoutMs: number) {
-  return (AbortSignal as unknown as { timeout?: (ms: number) => AbortSignal }).timeout?.(timeoutMs)
+  const timeout = (AbortSignal as unknown as { timeout?: (ms: number) => AbortSignal }).timeout
+  if (timeout) {
+    try {
+      return { signal: timeout.call(AbortSignal, timeoutMs), clear: undefined as (() => void) | undefined }
+    } catch {}
+  }
+  const controller = new AbortController()
+  const timer = setTimeout(() => controller.abort(), timeoutMs)
+  return { signal: controller.signal, clear: () => clearTimeout(timer) }
+}
+
+function wait(ms: number, signal?: AbortSignal) {
+  return new Promise<void>((resolve, reject) => {
+    if (signal?.aborted) {
+      reject(new DOMException("Aborted", "AbortError"))
+      return
+    }
+    const timer = setTimeout(() => {
+      signal?.removeEventListener("abort", onAbort)
+      resolve()
+    }, ms)
+    const onAbort = () => {
+      clearTimeout(timer)
+      reject(new DOMException("Aborted", "AbortError"))
+    }
+    signal?.addEventListener("abort", onAbort, { once: true })
+  })
+}
+
+function retryable(error: unknown, signal?: AbortSignal) {
+  if (signal?.aborted) return false
+  if (!(error instanceof Error)) return false
+  if (error.name === "AbortError" || error.name === "TimeoutError") return false
+  if (error instanceof TypeError) return true
+  return /network|fetch|econnreset|econnrefused|enotfound|timedout/i.test(error.message)
 }
 
 export async function checkServerHealth(
@@ -16,14 +56,24 @@ export async function checkServerHealth(
   fetch: typeof globalThis.fetch,
   opts?: CheckServerHealthOptions,
 ): Promise<ServerHealth> {
-  const signal = opts?.signal ?? timeoutSignal(opts?.timeoutMs ?? 3000)
-  const sdk = createOpencodeClient({
-    baseUrl: url,
-    fetch,
-    signal,
-  })
-  return sdk.global
-    .health()
-    .then((x) => ({ healthy: x.data?.healthy === true, version: x.data?.version }))
-    .catch(() => ({ healthy: false }))
+  const timeout = opts?.signal ? undefined : timeoutSignal(opts?.timeoutMs ?? defaultTimeoutMs)
+  const signal = opts?.signal ?? timeout?.signal
+  const retryCount = opts?.retryCount ?? defaultRetryCount
+  const retryDelayMs = opts?.retryDelayMs ?? defaultRetryDelayMs
+  const next = (count: number, error: unknown) => {
+    if (count >= retryCount || !retryable(error, signal)) return Promise.resolve({ healthy: false } as const)
+    return wait(retryDelayMs * (count + 1), signal)
+      .then(() => attempt(count + 1))
+      .catch(() => ({ healthy: false }))
+  }
+  const attempt = (count: number): Promise<ServerHealth> =>
+    createOpencodeClient({
+      baseUrl: url,
+      fetch,
+      signal,
+    })
+      .global.health()
+      .then((x) => (x.error ? next(count, x.error) : { healthy: x.data?.healthy === true, version: x.data?.version }))
+      .catch((error) => next(count, error))
+  return attempt(0).finally(() => timeout?.clear?.())
 }