fix(connection): fix 6 correctness bugs in auth/connect/diagnostics flow (#131)

1. buildRequestHeaders: UTF-8-encode Basic-auth credentials before btoa()
   so non-ASCII usernames/passwords don't throw (Hermes' btoa is Latin1-only
   and the throw was an unhandled rejection that hung the connect spinner).

2. diagnostics classify(): check root.ok (server reachable) before
   !internet.ok, so a reachable-but-failing server (e.g. wrong auth) is no
   longer misdiagnosed as "no internet" just because the public-internet
   probe also failed (captive portal, Tailscale-only network, etc).

3. sdk.ts createClient: strip trailing slashes from baseUrl once, so a
   trailing-slash URL from Advanced mode / Edit screen doesn't produce a
   double slash on every request path.

4. add.tsx / [id].tsx: wrap addConnection/updateConnection in try/catch so
   a SecureStore failure after a successful test resets the spinner and
   shows an alert instead of hanging forever. Adds
   connection.shared.alerts.saveFailedTitle/saveFailedMessage (en + zh-Hans).

5. add.tsx / [id].tsx: build the diagnostics probe's auth with buildAuth()
   instead of a hand-rolled expression, so the probe reproduces the real
   request's credentials (previously Quick Connect's password-only case
   sent no auth to the probe at all).

6. add.tsx handleQuickConnect: stop sending the shared `username` state,
   which could carry a stray value typed earlier in Advanced mode and
   silently override the "opencode" default after "Back to Quick".


Claude-Session: https://claude.ai/code/session_01T12AhSnQVrSxNnvwfCx2z6

Co-authored-by: engineer <engineer@macbookpro.lan>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

app/connection/[id].tsx

@@ -19,6 +19,7 @@ import type { ConnectionType } from "../../src/lib/types"
 import { probeConnection, shareReport } from "../../src/lib/diagnostics"
 import { captureDiagnostic } from "../../src/lib/sentry"
 import { parseUrl } from "../../src/lib/diagnostics-classify"
+import { buildAuth } from "../../src/lib/auth"
 
 // labelKey (not literal text): this is a module-level constant evaluated
 // before i18next is guaranteed ready, so the label is resolved with t() at
@@ -101,10 +102,7 @@ export default function EditConnectionScreen() {
     }
 
     // Failed: run active diagnostics, capture to Sentry, offer a shareable report.
-    const report = await probeConnection(
-      url.trim(),
-      username.trim() && password ? { username: username.trim(), password } : undefined,
-    )
+    const report = await probeConnection(url.trim(), buildAuth(username, password))
     captureDiagnostic(report)
     setIsTesting(false)
 
@@ -136,6 +134,7 @@ export default function EditConnectionScreen() {
     }
 
     setIsSaving(true)
+    try {
       await updateConnection(
         connection.id,
         {
@@ -158,6 +157,13 @@ export default function EditConnectionScreen() {
       }
       setIsSaving(false)
       router.back()
+    } catch {
+      setIsSaving(false)
+      Alert.alert(
+        t("connection.shared.alerts.saveFailedTitle"),
+        t("connection.shared.alerts.saveFailedMessage"),
+      )
+    }
   }
 
   const handleDelete = () => {

app/connection/add.tsx

@@ -19,6 +19,7 @@ import type { ConnectionType } from "../../src/lib/types"
 import { probeConnection, shareReport } from "../../src/lib/diagnostics"
 import { captureDiagnostic } from "../../src/lib/sentry"
 import { parseUrl } from "../../src/lib/diagnostics-classify"
+import { buildAuth } from "../../src/lib/auth"
 import { AnalyticsEvent, track } from "../../src/lib/analytics"
 import { submitWaitlistSignup, buildWaitlistMailtoUrl } from "../../src/lib/waitlist"
 
@@ -75,14 +76,17 @@ export default function AddConnectionScreen() {
     track(AnalyticsEvent.ConnectionFormSubmitted, { mode: "quick" })
     setIsConnecting(true)
 
-    // Test connection first
+    // Test connection first. Quick Connect has no username field, so the
+    // connection is intentionally saved without one — buildAuth() defaults
+    // it to "opencode" wherever auth is built. Sending the `username` state
+    // here would leak a value typed earlier in Advanced mode (issue: Back to
+    // Quick silently overriding the default).
     const result = await testConnection(
       {
         id: "",
         name: name || t("connection.shared.namePlaceholder"),
         type: "local",
         url: serverUrl,
-        username: username.trim() || undefined,
       },
       "onboarding",
       password || undefined,
@@ -90,23 +94,27 @@ export default function AddConnectionScreen() {
 
     if (result.ok) {
       // Save and go back
+      try {
         await addConnection(
           {
             name: name.trim() || t("connection.shared.namePlaceholder"),
             type: "local",
             url: serverUrl,
-          username: username.trim() || undefined,
           },
           password || undefined,
         )
         setIsConnecting(false)
         router.back()
+      } catch {
+        setIsConnecting(false)
+        Alert.alert(
+          t("connection.shared.alerts.saveFailedTitle"),
+          t("connection.shared.alerts.saveFailedMessage"),
+        )
+      }
     } else {
       // Failed: run active diagnostics, capture to Sentry, offer a shareable report.
-      const report = await probeConnection(
-        serverUrl,
-        username.trim() && password ? { username: username.trim(), password } : undefined,
-      )
+      const report = await probeConnection(serverUrl, buildAuth(undefined, password))
       captureDiagnostic(report)
       setIsConnecting(false)
       Alert.alert(
@@ -160,6 +168,7 @@ export default function AddConnectionScreen() {
     )
 
     if (result.ok) {
+      try {
         await addConnection(
           {
             name: name.trim(),
@@ -172,16 +181,20 @@ export default function AddConnectionScreen() {
         )
         setIsConnecting(false)
         router.back()
+      } catch {
+        setIsConnecting(false)
+        Alert.alert(
+          t("connection.shared.alerts.saveFailedTitle"),
+          t("connection.shared.alerts.saveFailedMessage"),
+        )
+      }
       return
     }
 
     // Failed: same "Connection Failed" alert as Quick Connect — run active
     // diagnostics, capture to Sentry, and offer a shareable report instead of
     // silently persisting an unreachable/unauthorized connection.
-    const report = await probeConnection(
-      url.trim(),
-      username.trim() && password ? { username: username.trim(), password } : undefined,
-    )
+    const report = await probeConnection(url.trim(), buildAuth(username, password))
     captureDiagnostic(report)
     setIsConnecting(false)
     Alert.alert(

src/lib/diagnostics-classify.test.ts

@@ -110,6 +110,19 @@ test("internet up, root unreachable, no timeout -> server-unreachable", () => {
   assert.equal(r.classification, "server-unreachable")
 })
 
+test("root reachable but internet probe down still classifies as health-failed, not no-internet", () => {
+  // Regression: the user's own server responded (proving the path to it
+  // works — e.g. captive portal or no WAN but Tailscale LAN still up), so
+  // this must not be misreported as "no internet".
+  const r = classify(
+    okUrl,
+    probe({ ok: false, error: "HTTP 401", status: 401 }), // health
+    probe({ ok: false }), // internet (down)
+    probe({ ok: true }), // root (reachable)
+  )
+  assert.equal(r.classification, "health-failed")
+})
+
 test("server-unreachable adds MagicDNS hint only for hostnames, not IPs", () => {
   const fail = { internet: probe({ ok: true }), root: probe({ ok: false }) }
   const hostR = classify(parseUrl("http://box.ts.net:8080"), probe({ error: "refused" }), fail.internet, fail.root)

src/lib/diagnostics-classify.ts

@@ -62,13 +62,17 @@ export function classify(
   if (isTls) {
     return { classification: "tls-error", summary: "TLS/certificate problem. Try http:// instead of https://, or fix the server certificate." }
   }
+  // The user's own server responded to *something* (even a 401/403/404) —
+  // that proves the path to the server works, so a failed public-internet
+  // probe (captive portal, no WAN but Tailscale LAN still up, etc.) must not
+  // override it and misreport a reachable server as "no internet".
+  if (root.ok) {
+    return { classification: "health-failed", summary: `Server is reachable but /global/health failed (HTTP ${health.status ?? "error"}). Likely wrong path, auth, or an old server version.` }
+  }
   if (!internet.ok) {
     return { classification: "no-internet", summary: "The device has no working internet/network at all (public check also failed). Check Wi-Fi/data and Tailscale (VPN) status." }
   }
   // Internet works, server does not.
-  if (root.ok) {
-    return { classification: "health-failed", summary: `Server is reachable but /global/health failed (HTTP ${health.status ?? "error"}). Likely wrong path, auth, or an old server version.` }
-  }
   if (isTimeout) {
     return { classification: "timeout", summary: "Connection to the server timed out (dropped, not refused). Likely a firewall, wrong port, or Tailscale ACL blocking the device." }
   }

src/lib/headers.test.ts

@@ -57,3 +57,31 @@ test("empty-string directory is treated as absent (falsy)", () => {
   const h = buildRequestHeaders({ directory: "" })
   assert.equal("x-opencode-directory" in h, false)
 })
+
+// Hermes' `btoa` is Latin1-only and throws a RangeError on non-ASCII input.
+// The auth header must be built with a UTF-8-safe base64 helper so a
+// non-ASCII username/password never throws (which otherwise surfaces as an
+// unhandled rejection and a stuck connect spinner).
+
+test("ASCII credentials produce the exact same header as plain btoa (no behavior change)", () => {
+  const h = buildRequestHeaders({ auth: { username: "alice", password: "s3cret" } })
+  assert.equal(h["Authorization"], `Basic ${btoa("alice:s3cret")}`)
+})
+
+test("non-ASCII username does not throw and round-trips back to the credentials", () => {
+  const auth = { username: "usér", password: "s3cret" }
+  assert.doesNotThrow(() => buildRequestHeaders({ auth }))
+  const h = buildRequestHeaders({ auth })
+  const b64 = h["Authorization"].replace("Basic ", "")
+  const decoded = decodeURIComponent(escape(atob(b64)))
+  assert.equal(decoded, `${auth.username}:${auth.password}`)
+})
+
+test("non-ASCII password does not throw and round-trips back to the credentials", () => {
+  const auth = { username: "admin", password: "pässwörd123" }
+  assert.doesNotThrow(() => buildRequestHeaders({ auth }))
+  const h = buildRequestHeaders({ auth })
+  const b64 = h["Authorization"].replace("Basic ", "")
+  const decoded = decodeURIComponent(escape(atob(b64)))
+  assert.equal(decoded, `${auth.username}:${auth.password}`)
+})

src/lib/headers.ts

@@ -9,6 +9,17 @@ export interface HeaderConfig {
   auth?: { username: string; password: string }
 }
 
+// `btoa` is Latin1-only and throws a range error on any character outside
+// the Latin1 byte range (e.g. a non-ASCII username/password). UTF-8-encode
+// first so arbitrary Unicode credentials survive - this is the standard
+// browser idiom for UTF-8-safe base64, and matches RFC 7617 (Basic auth
+// credentials are UTF-8 before being base64-encoded). ASCII input is
+// byte-identical to plain `btoa` since encodeURIComponent/unescape
+// round-trip it unchanged.
+function toBase64Utf8(str: string): string {
+  return btoa(unescape(encodeURIComponent(str)))
+}
+
 export function buildRequestHeaders(config: HeaderConfig): Record<string, string> {
   const headers: Record<string, string> = {
     "Content-Type": "application/json",
@@ -25,7 +36,7 @@ export function buildRequestHeaders(config: HeaderConfig): Record<string, string
   }
 
   if (config.auth) {
-    const credentials = btoa(`${config.auth.username}:${config.auth.password}`)
+    const credentials = toBase64Utf8(`${config.auth.username}:${config.auth.password}`)
     headers["Authorization"] = `Basic ${credentials}`
   }
 

src/lib/i18n/en.json

@@ -146,7 +146,9 @@
         "invalidUrlTitle": "Invalid URL",
         "invalidUrlMessage": "Enter a full URL including http:// or https://, e.g. http://192.168.1.100:4096",
         "connectionFailedTitle": "Connection Failed",
-        "unknownError": "Unknown error"
+        "unknownError": "Unknown error",
+        "saveFailedTitle": "Couldn't Save Connection",
+        "saveFailedMessage": "The connection test succeeded, but saving it failed. Please try again."
       }
     },
     "add": {

src/lib/i18n/zh-Hans.json

@@ -146,7 +146,9 @@
         "invalidUrlTitle": "地址无效",
         "invalidUrlMessage": "请输入包含 http:// 或 https:// 的完整地址，例如 http://192.168.1.100:4096",
         "connectionFailedTitle": "连接失败",
-        "unknownError": "未知错误"
+        "unknownError": "未知错误",
+        "saveFailedTitle": "无法保存连接",
+        "saveFailedMessage": "连接测试成功，但保存失败，请重试。"
       }
     },
     "add": {

src/lib/sdk.ts

@@ -245,6 +245,13 @@ async function fetchWithTimeout(url: string, options: RequestInit = {}, timeoutM
 }
 
 export function createClient(config: ClientConfig) {
+  // Normalize once: a trailing slash on baseUrl (e.g. pasted into Advanced
+  // mode or the Edit screen) would otherwise survive into every
+  // `${config.baseUrl}${path}` concatenation below as a double slash, which
+  // every request then fails against (while the diagnostics probe, which
+  // reconstructs a clean URL, reports "works now"). A bare URL with no
+  // trailing slash is untouched.
+  config = { ...config, baseUrl: config.baseUrl.replace(/\/+$/, "") }
   return {
     global: {
       // `timeoutMs` overrides the default REQUEST_TIMEOUT_MS — used by the