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

1 files changed, 59 insertions, 50 deletions

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
 }