fix: listener race condition, stale callback ref, dead code cleanup

4 files changed, 77 insertions, 66 deletions

diff --git a/src/components/TaskList.tsx b/src/components/TaskList.tsx
index 0f87e0d..6ecce42 100644
--- a/src/components/TaskList.tsx
+++ b/src/components/TaskList.tsx
@@ -272,7 +272,7 @@ export function TaskList() {
                 }}
                 title="Remove task"
               >
-                x
+                ×
               </button>
             </div>
           );
diff --git a/src/components/TimerView.tsx b/src/components/TimerView.tsx
index b56e6a8..973f09f 100644
--- a/src/components/TimerView.tsx
+++ b/src/components/TimerView.tsx
@@ -122,7 +122,6 @@ export function TimerView() {
               lineHeight: 1,
               color: 'var(--fg-1)',
               letterSpacing: '-0.02em',
-              fontVariantNumeric: 'tabular-nums',
             }}
           >
             {formatTime(remainingSecs)}
diff --git a/src/hooks/useTimerEvents.ts b/src/hooks/useTimerEvents.ts
index c1f289a..aebc780 100644
--- a/src/hooks/useTimerEvents.ts
+++ b/src/hooks/useTimerEvents.ts
@@ -1,4 +1,4 @@
-import { useEffect } from 'react';
+import { useEffect, useRef } from 'react';
 import { listen } from '@tauri-apps/api/event';
 import { invoke } from '@tauri-apps/api/core';
 import { useTimerStore, TimerTickPayload } from '../store/timerStore';
@@ -29,17 +29,20 @@ export function useTimerEvents(
   const setRunning = useTimerStore((s) => s.setRunning);
   const fetchTasks = useTaskStore((s) => s.fetchTasks);
 
+  const onCompletedRef = useRef(onCompleted);
   useEffect(() => {
-    // Bootstrap initial state from backend
-    invoke<{
-      phase: TimerTickPayload['phase'];
-      remaining_secs: number;
-      total_secs: number;
-      running: boolean;
-      session_count: number;
-      current_task_id: string | null;
-    }>('get_timer_status')
-      .then((status) => {
+    onCompletedRef.current = onCompleted;
+  }, [onCompleted]);
+
+  useEffect(() => {
+    let cancelled = false;
+    let unlisteners: Array<() => void> = [];
+
+    async function setup() {
+      // Bootstrap initial state from backend
+      try {
+        const status = await invoke<TimerStatus>('get_timer_status');
+        if (cancelled) return;
         setTimerTick({
           phase: status.phase,
           remaining_secs: status.remaining_secs,
@@ -48,52 +51,58 @@ export function useTimerEvents(
           current_task_id: status.current_task_id,
         });
         setRunning(status.running);
-      })
-      .catch((e) => console.error('get_timer_status error:', e));
+      } catch (e) {
+        console.error('Failed to get timer status:', e);
+      }
 
-    let unlistenTick: (() => void) | null = null;
-    let unlistenCompleted: (() => void) | null = null;
-    let unlistenPhaseChanged: (() => void) | null = null;
+      if (cancelled) return;
 
-    listen<TimerTickPayload>('timer-tick', (event) => {
-      setTimerTick(event.payload);
-      setRunning(true);
-    }).then((fn) => { unlistenTick = fn; });
+      // Register all listeners atomically
+      try {
+        const [unlistenTick, unlistenCompleted, unlistenPhaseChanged] = await Promise.all([
+          listen<TimerTickPayload>('timer-tick', (event) => {
+            setTimerTick(event.payload);
+            setRunning(true);
+          }),
+          listen<CompletedPayload>('timer-completed', async (event) => {
+            setRunning(false);
+            onCompletedRef.current(event.payload.task_id ?? null);
+            await fetchTasks();
+          }),
+          listen<PhaseChangedPayload>('timer-phase-changed', async (_event) => {
+            try {
+              const status = await invoke<TimerStatus>('get_timer_status');
+              setTimerTick({
+                phase: status.phase,
+                remaining_secs: status.remaining_secs,
+                total_secs: status.total_secs,
+                session_count: status.session_count,
+                current_task_id: status.current_task_id,
+              });
+            } catch (e) {
+              console.error('Failed to re-sync timer status:', e);
+            }
+          }),
+        ]);
 
-    listen<CompletedPayload>('timer-completed', (event) => {
-      setRunning(false);
-      onCompleted(event.payload.task_id);
-      // Refresh tasks since remaining_sessions may have changed
-      fetchTasks();
-    }).then((fn) => { unlistenCompleted = fn; });
+        if (cancelled) {
+          unlistenTick();
+          unlistenCompleted();
+          unlistenPhaseChanged();
+          return;
+        }
 
-    listen<PhaseChangedPayload>('timer-phase-changed', async (event) => {
-      setRunning(false);
-      try {
-        const status = await invoke<TimerStatus>('get_timer_status');
-        setTimerTick({
-          phase: status.phase,
-          remaining_secs: status.remaining_secs,
-          total_secs: status.total_secs,
-          session_count: status.session_count,
-          current_task_id: status.current_task_id,
-        });
+        unlisteners = [unlistenTick, unlistenCompleted, unlistenPhaseChanged];
       } catch (e) {
-        // Fallback: at least update the phase from the event payload
-        setTimerTick({
-          phase: event.payload.phase,
-          remaining_secs: useTimerStore.getState().remainingSecs,
-          total_secs: useTimerStore.getState().totalSecs,
-          session_count: event.payload.session_count,
-          current_task_id: useTimerStore.getState().currentTaskId,
-        });
+        console.error('Failed to register timer listeners:', e);
       }
-    }).then((fn) => { unlistenPhaseChanged = fn; });
+    }
+
+    setup();
 
     return () => {
-      unlistenTick?.();
-      unlistenCompleted?.();
-      unlistenPhaseChanged?.();
+      cancelled = true;
+      unlisteners.forEach((fn) => fn());
     };
-  }, [setTimerTick, setRunning, fetchTasks, onCompleted]);
+  }, [setTimerTick, setRunning, fetchTasks]); // onCompleted excluded — updated via ref
 }
diff --git a/src/store/taskStore.ts b/src/store/taskStore.ts
index dccd17d..94386ac 100644
--- a/src/store/taskStore.ts
+++ b/src/store/taskStore.ts
@@ -18,7 +18,6 @@ interface TaskStore {
   updateTask: (id: string, remainingSessions?: number, completed?: boolean) => Promise<void>;
   deleteTask: (id: string) => Promise<void>;
   setCurrentTask: (id: string | null) => Promise<void>;
-  refreshTask: (task: Task) => void;
 }
 
 export const useTaskStore = create<TaskStore>((set) => ({
@@ -37,11 +36,16 @@ export const useTaskStore = create<TaskStore>((set) => ({
   },
 
   addTask: async (name, totalSessions) => {
-    const task = await invoke<Task>('add_task', {
-      name,
-      totalSessions,
-    });
-    set((state) => ({ tasks: [...state.tasks, task] }));
+    try {
+      const task = await invoke<Task>('add_task', {
+        name,
+        totalSessions,
+      });
+      set((state) => ({ tasks: [...state.tasks, task] }));
+    } catch (e) {
+      console.error('addTask error:', e);
+      throw e;
+    }
   },
 
   updateTask: async (id, remainingSessions, completed) => {
@@ -64,17 +68,16 @@ export const useTaskStore = create<TaskStore>((set) => ({
   },
 
   deleteTask: async (id) => {
-    await invoke('delete_task', { id });
-    set((state) => ({ tasks: state.tasks.filter((t) => t.id !== id) }));
+    try {
+      await invoke('delete_task', { id });
+      set((state) => ({ tasks: state.tasks.filter((t) => t.id !== id) }));
+    } catch (e) {
+      console.error('deleteTask error:', e);
+      throw e;
+    }
   },
 
   setCurrentTask: async (id) => {
     await invoke('set_current_task', { taskId: id });
   },
-
-  refreshTask: (updatedTask) => {
-    set((state) => ({
-      tasks: state.tasks.map((t) => (t.id === updatedTask.id ? updatedTask : t)),
-    }));
-  },
 }));