From 2ad4776a76a071cb1201a7095686860923a97a64 Mon Sep 17 00:00:00 2001
From: Jamie Pine <ijamespine@me.com>
Date: Mon, 16 Mar 2026 13:06:57 -0700
Subject: [PATCH] fix review feedback: restart race, listener cleanup, stable
 keys, accessibility

---
 .../components/ServerTab/ChangelogPage.tsx    | 16 ++---
 app/src/components/ServerTab/GeneralPage.tsx  |  7 ++
 app/src/components/ServerTab/GpuPage.tsx      | 65 ++++++++-----------
 app/src/components/ServerTab/LogsPage.tsx     |  2 +-
 app/src/components/ServerTab/SettingRow.tsx   |  2 +-
 app/src/components/ui/toaster.tsx             |  2 +-
 app/src/components/ui/toggle.tsx              |  1 +
 app/src/stores/logStore.ts                    |  6 +-
 tauri/src/platform/lifecycle.ts               | 19 ++++--
 9 files changed, 66 insertions(+), 54 deletions(-)

diff --git a/app/src/components/ServerTab/ChangelogPage.tsx b/app/src/components/ServerTab/ChangelogPage.tsx
index 9582d0c2..afb32b47 100644
--- a/app/src/components/ServerTab/ChangelogPage.tsx
+++ b/app/src/components/ServerTab/ChangelogPage.tsx
@@ -57,8 +57,8 @@ function renderMarkdown(md: string): React.ReactNode[] {
       }
       elements.push(
         <ul key={elements.length} className="space-y-1 my-2">
-          {items.map((item) => (
-            <li key={item} className="text-sm text-muted-foreground flex gap-2">
+          {items.map((item, idx) => (
+            <li key={idx} className="text-sm text-muted-foreground flex gap-2">
               <span className="text-muted-foreground/50 select-none shrink-0">&bull;</span>
               <span>{inlineMarkdown(item)}</span>
             </li>
@@ -96,9 +96,9 @@ function renderTable(tableLines: string[], keyBase: number): React.ReactNode {
       <table className="text-sm w-full">
         <thead>
           <tr className="border-b">
-            {headers.map((h) => (
+            {headers.map((h, hIdx) => (
               <th
-                key={h}
+                key={hIdx}
                 className="text-left py-1.5 pr-4 text-muted-foreground font-medium text-xs"
               >
                 {inlineMarkdown(h)}
@@ -107,10 +107,10 @@ function renderTable(tableLines: string[], keyBase: number): React.ReactNode {
           </tr>
         </thead>
         <tbody>
-          {rows.map((row) => (
-            <tr key={row.join('|')} className="border-b border-border/50">
-              {row.map((cell) => (
-                <td key={cell} className="py-1.5 pr-4 text-muted-foreground">
+          {rows.map((row, rowIdx) => (
+            <tr key={rowIdx} className="border-b border-border/50">
+              {row.map((cell, cellIdx) => (
+                <td key={cellIdx} className="py-1.5 pr-4 text-muted-foreground">
                   {inlineMarkdown(cell)}
                 </td>
               ))}
diff --git a/app/src/components/ServerTab/GeneralPage.tsx b/app/src/components/ServerTab/GeneralPage.tsx
index 51b5ed74..ccf117b1 100644
--- a/app/src/components/ServerTab/GeneralPage.tsx
+++ b/app/src/components/ServerTab/GeneralPage.tsx
@@ -133,6 +133,13 @@ export function GeneralPage() {
                 setKeepServerRunningOnClose(checked);
                 platform.lifecycle.setKeepServerRunning(checked).catch((error) => {
                   console.error('Failed to sync setting to Rust:', error);
+                  setKeepServerRunningOnClose(!checked);
+                  toast({
+                    title: 'Failed to update setting',
+                    description: 'Could not sync setting to backend.',
+                    variant: 'destructive',
+                  });
+                  return;
                 });
                 toast({
                   title: 'Setting updated',
diff --git a/app/src/components/ServerTab/GpuPage.tsx b/app/src/components/ServerTab/GpuPage.tsx
index f0f38681..0ebe1d37 100644
--- a/app/src/components/ServerTab/GpuPage.tsx
+++ b/app/src/components/ServerTab/GpuPage.tsx
@@ -171,17 +171,21 @@ export function GpuPage() {
     };
   }, [cudaDownloading, serverUrl, refetchCudaStatus]);
 
+  const clearHealthPolling = useCallback(() => {
+    if (healthPollRef.current) {
+      clearInterval(healthPollRef.current);
+      healthPollRef.current = null;
+    }
+  }, []);
+
   const startHealthPolling = useCallback(() => {
-    if (healthPollRef.current) return;
+    clearHealthPolling();
 
     healthPollRef.current = setInterval(async () => {
       try {
         const result = await apiClient.getHealth();
         if (result.status === 'healthy') {
-          if (healthPollRef.current) {
-            clearInterval(healthPollRef.current);
-            healthPollRef.current = null;
-          }
+          clearHealthPolling();
           setRestartPhase('ready');
           queryClient.invalidateQueries();
           setTimeout(() => setRestartPhase('idle'), 2000);
@@ -190,7 +194,23 @@ export function GpuPage() {
         // Server still down, keep polling
       }
     }, 1000);
-  }, [queryClient]);
+  }, [queryClient, clearHealthPolling]);
+
+  const restartServerWithPolling = useCallback(
+    async (errorMessage: string) => {
+      setRestartPhase('stopping');
+      try {
+        await platform.lifecycle.restartServer();
+        setRestartPhase('waiting');
+        startHealthPolling();
+      } catch (e: unknown) {
+        clearHealthPolling();
+        setRestartPhase('idle');
+        throw new Error(e instanceof Error ? e.message : errorMessage);
+      }
+    },
+    [platform, startHealthPolling, clearHealthPolling],
+  );
 
   const handleDownload = async () => {
     setError(null);
@@ -209,24 +229,9 @@ export function GpuPage() {
 
   const handleRestart = async () => {
     setError(null);
-    setRestartPhase('stopping');
     try {
-      setRestartPhase('waiting');
-      startHealthPolling();
-      await platform.lifecycle.restartServer();
-      if (healthPollRef.current) {
-        clearInterval(healthPollRef.current);
-        healthPollRef.current = null;
-      }
-      setRestartPhase('ready');
-      queryClient.invalidateQueries();
-      setTimeout(() => setRestartPhase('idle'), 2000);
+      await restartServerWithPolling('Restart failed');
     } catch (e: unknown) {
-      setRestartPhase('idle');
-      if (healthPollRef.current) {
-        clearInterval(healthPollRef.current);
-        healthPollRef.current = null;
-      }
       setError(e instanceof Error ? e.message : 'Restart failed');
     }
   };
@@ -236,22 +241,8 @@ export function GpuPage() {
     setRestartPhase('stopping');
     try {
       await apiClient.deleteCudaBackend();
-      setRestartPhase('waiting');
-      startHealthPolling();
-      await platform.lifecycle.restartServer();
-      if (healthPollRef.current) {
-        clearInterval(healthPollRef.current);
-        healthPollRef.current = null;
-      }
-      setRestartPhase('ready');
-      queryClient.invalidateQueries();
-      setTimeout(() => setRestartPhase('idle'), 2000);
+      await restartServerWithPolling('Failed to switch to CPU');
     } catch (e: unknown) {
-      setRestartPhase('idle');
-      if (healthPollRef.current) {
-        clearInterval(healthPollRef.current);
-        healthPollRef.current = null;
-      }
       setError(e instanceof Error ? e.message : 'Failed to switch to CPU');
       refetchCudaStatus();
     }
diff --git a/app/src/components/ServerTab/LogsPage.tsx b/app/src/components/ServerTab/LogsPage.tsx
index b7cf85ae..68b8f317 100644
--- a/app/src/components/ServerTab/LogsPage.tsx
+++ b/app/src/components/ServerTab/LogsPage.tsx
@@ -96,7 +96,7 @@ export function LogsPage() {
             )}
           </div>
         ) : (
-          entries.map((entry, i) => <LogLine key={`${entry.timestamp}-${i}`} entry={entry} />)
+          entries.map((entry) => <LogLine key={entry.id} entry={entry} />)
         )}
       </div>
     </div>
diff --git a/app/src/components/ServerTab/SettingRow.tsx b/app/src/components/ServerTab/SettingRow.tsx
index f2a54f58..f4b42faa 100644
--- a/app/src/components/ServerTab/SettingRow.tsx
+++ b/app/src/components/ServerTab/SettingRow.tsx
@@ -48,7 +48,7 @@ export function SettingRow({
         <div className="min-w-0">
           <label
             htmlFor={htmlFor}
-            className="text-sm font-medium leading-none cursor-pointer select-none"
+            className={`text-sm font-medium leading-none select-none ${htmlFor ? 'cursor-pointer' : ''}`}
           >
             {title}
           </label>
diff --git a/app/src/components/ui/toaster.tsx b/app/src/components/ui/toaster.tsx
index 7332623b..3c1d2978 100644
--- a/app/src/components/ui/toaster.tsx
+++ b/app/src/components/ui/toaster.tsx
@@ -25,7 +25,7 @@ export function Toaster() {
           <ToastClose />
         </Toast>
       ))}
-      <ToastViewport className={isPlayerOpen ? 'sm:bottom-32' : ''} />
+      <ToastViewport className={isPlayerOpen ? 'sm:bottom-44' : ''} />
     </ToastProvider>
   );
 }
diff --git a/app/src/components/ui/toggle.tsx b/app/src/components/ui/toggle.tsx
index 8259695e..3db31a81 100644
--- a/app/src/components/ui/toggle.tsx
+++ b/app/src/components/ui/toggle.tsx
@@ -26,6 +26,7 @@ const Toggle = React.forwardRef<HTMLButtonElement, ToggleProps>(
         }}
         className={cn(
           'relative inline-flex h-5 w-9 shrink-0 items-center rounded-full transition-colors',
+          'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2',
           checked ? 'bg-accent' : 'bg-muted-foreground/25',
           disabled ? 'opacity-50 cursor-not-allowed' : 'cursor-pointer',
           className,
diff --git a/app/src/stores/logStore.ts b/app/src/stores/logStore.ts
index 2f369d86..833f3088 100644
--- a/app/src/stores/logStore.ts
+++ b/app/src/stores/logStore.ts
@@ -3,7 +3,10 @@ import type { ServerLogEntry } from '@/platform/types';
 
 const MAX_LOG_ENTRIES = 2000;
 
+let nextLogEntryId = 0;
+
 export interface LogEntry extends ServerLogEntry {
+  id: number;
   timestamp: number;
 }
 
@@ -17,9 +20,8 @@ export const useLogStore = create<LogStore>((set) => ({
   entries: [],
   addEntry: (entry) =>
     set((state) => {
-      const newEntry: LogEntry = { ...entry, timestamp: Date.now() };
+      const newEntry: LogEntry = { ...entry, id: nextLogEntryId++, timestamp: Date.now() };
       const entries = [...state.entries, newEntry];
-      // Cap buffer size
       if (entries.length > MAX_LOG_ENTRIES) {
         return { entries: entries.slice(entries.length - MAX_LOG_ENTRIES) };
       }
diff --git a/tauri/src/platform/lifecycle.ts b/tauri/src/platform/lifecycle.ts
index ad20fa0a..b20da778 100644
--- a/tauri/src/platform/lifecycle.ts
+++ b/tauri/src/platform/lifecycle.ts
@@ -88,16 +88,27 @@ class TauriLifecycle implements PlatformLifecycle {
   }
 
   subscribeToServerLogs(callback: (entry: ServerLogEntry) => void): () => void {
+    let disposed = false;
     let unlisten: (() => void) | null = null;
 
-    listen<ServerLogEntry>('server-log', (event) => {
+    void listen<ServerLogEntry>('server-log', (event) => {
       callback(event.payload);
-    }).then((fn) => {
-      unlisten = fn;
-    });
+    })
+      .then((fn) => {
+        if (disposed) {
+          fn();
+          return;
+        }
+        unlisten = fn;
+      })
+      .catch((error) => {
+        console.error('Failed to subscribe to server logs:', error);
+      });
 
     return () => {
+      disposed = true;
       unlisten?.();
+      unlisten = null;
     };
   }
 }