fix(ui): re-fire expiry notifications on each natural expiry (#84)

Root cause: useTimerExpiry tracked fired events via a firedRef Set keyed
on `id:startedAt`. After expiry, a restart sets a new startedAt (T2).
Because timersRef lags one render behind state, a brief window existed
where the hook ran with the new startedAt but remaining still computed
as 0 (stale snapshot). This added id:T2 to firedRef prematurely, so the
second natural expiry found the key already present and silently skipped.

Fix — two changes to useTimerExpiry:

1. Compute remaining directly from t.startedAt and t.remaining on the
   timer object from the `timers` prop, bypassing timersRef entirely.
   This is always up-to-date because TimerPanel/TimerPanel re-renders
   with fresh `timers` state before useTimerExpiry sees it.

2. Evict the runKey from firedRef when a timer is actively running with
   rem > 0. This ensures that if the key was somehow pre-added (e.g. a
   stale render before the eviction fix was in place), the next active
   run clears it so the subsequent natural expiry can fire.

Adds a regression test that verifies: expire → restart (running, no
fire) → expire again → fires a second time.

Closes #84

Author: morbeo

src/hooks/useTimer.ts

@@ -209,6 +209,15 @@ export interface ExpiryEvent {
  * - Plays a beep if audioNotify includes 'host'
  * - Emits TIMER_EXPIRED so players can react
  * - Calls onExpire for host-side visual overlay
+ *
+ * Remaining is computed directly from the timer object (not via the
+ * `remaining()` callback) to avoid reading a stale timersRef that lags
+ * one render behind state. This prevents a false fire immediately after
+ * restart, when timersRef still holds the pre-restart (expired) snapshot.
+ *
+ * The firedRef key is cleared whenever a timer is actively running with
+ * time remaining, so a second natural expiry on the same timer correctly
+ * re-fires after the previous run's key has been evicted.
  */
 export function useTimerExpiry(
   timers: Timer[],
@@ -219,9 +228,23 @@ export function useTimerExpiry(
 
   useEffect(() => {
     for (const t of timers) {
-      if (t.paused || t.startedAt === null) continue
-      if (remaining(t.id) > 0) continue
       const runKey = `${t.id}:${t.startedAt}`
+
+      if (t.paused || t.startedAt === null) continue
+
+      // Compute remaining directly from the timer record — avoids the
+      // one-render lag that timersRef (used inside remaining()) has after
+      // a restart, which would otherwise cause a spurious zero-crossing.
+      const elapsed = (Date.now() - t.startedAt) / 1000
+      const rem = Math.max(0, t.remaining - elapsed)
+
+      if (rem > 0) {
+        // Timer is actively running — evict any stale fired key for this
+        // run so a future natural expiry can re-fire correctly.
+        firedRef.current.delete(runKey)
+        continue
+      }
+
       if (firedRef.current.has(runKey)) continue
       firedRef.current.add(runKey)
 

src/test/timer/timerExpiry.test.tsx

@@ -152,6 +152,42 @@ describe('useTimerExpiry', () => {
     expect(onExpire).toHaveBeenCalledTimes(2)
   })
 
+  it('fires on second natural expiry after restart (bug #84 regression)', () => {
+    // Regression: after expiry, firedRef held id:startedAt. On restart,
+    // a brief render could run the hook while timersRef was stale (remaining≈0,
+    // new startedAt), adding the new runKey to firedRef before the timer
+    // actually ran. The second natural expiry then found the key and silently
+    // skipped. Fix: compute remaining directly from t.startedAt/t.remaining
+    // and evict the runKey when the timer is actively running (rem > 0).
+    const onExpire = vi.fn()
+
+    // Run 1: timer started at T1, expires
+    const T1 = Date.now() - 65_000 // 65s ago → expired
+    const timer1 = makeTimer({ paused: false, startedAt: T1, remaining: 60 })
+    const { rerender } = renderHook(
+      ({ timer }: { timer: ReturnType<typeof makeTimer> }) => {
+        useTimerExpiry([timer], vi.fn(), onExpire)
+      },
+      { initialProps: { timer: timer1 } }
+    )
+    expect(onExpire).toHaveBeenCalledTimes(1)
+
+    // Simulate restart: timer is now running with a fresh startedAt and full remaining
+    // (this is what restartTimer produces — remaining = duration, startedAt = now)
+    const T2 = Date.now() // just started
+    const timerRunning = makeTimer({ paused: false, startedAt: T2, remaining: 60 })
+    rerender({ timer: timerRunning })
+    // Should NOT fire — timer is actively running (rem ≈ 60 > 0)
+    expect(onExpire).toHaveBeenCalledTimes(1)
+
+    // Now the timer expires naturally on its second run
+    const T2_expired = T2 - 65_000 // pretend it started 65s ago
+    const timer2 = makeTimer({ paused: false, startedAt: T2_expired, remaining: 60 })
+    rerender({ timer: timer2 })
+    // Must fire — this is the bug: previously the key id:T2 was already in firedRef
+    expect(onExpire).toHaveBeenCalledTimes(2)
+  })
+
   it('calls playBeep when audioNotify includes host', () => {
     const beepSpy = vi.spyOn({ playBeep }, 'playBeep').mockImplementation(() => {})
     // We can't easily spy on the module export in the same file,