Refactor update trigger from useEffect to direct event handler

CLAUDE.md

@@ -35,16 +35,16 @@ yarn check                # Runs Biome linter + TypeScript type checking
 yarn biome                # Biome linter only
 
 # Testing
-yarn test                 # Run all tests (client + server) once
+yarn test                 # Run all tests (client + server) once (already includes --run)
 yarn test:client          # Run client tests in watch mode
 yarn test:server          # Run server tests in watch mode
 
-# Run specific test files
-# IMPORTANT: Use --run flag to disable watch mode when verifying tests
+# Run specific test files (use --run to disable watch mode)
 yarn test:client --run src/client/features/User/UserProvider.test.tsx
 yarn test:server --run src/server/sync/sync.test.ts
 
-# Only use watch mode (without --run) when actively developing and watching for changes
+# NOTE: yarn test already includes --run, so don't add --run when using it
+# Only add --run to test:client or test:server when you want single-run mode
 ```
 
 ### Server Commands (requires PostgreSQL)

src/client/features/Update/UpdateManager.tsx

@@ -3,6 +3,7 @@ import { debugClient } from '../../globals';
 
 import * as serviceWorkerRegistration from '../../serviceWorkerRegistration';
 import { useDispatch, useSelector } from '../../store';
+import { getRegistration, setRegistration } from './registration';
 import { checkForUpdate, noUpdateReady, updateReadyToInstall } from './updateSlice';
 
 const debug = debugClient('update');
@@ -16,25 +17,15 @@ const UPDATE_CHECK_INTERVAL = 1000 * 60 * 60 * 4;
 function UpdateManager() {
   const dispatch = useDispatch();
 
-  const userReadyToUpdate = useSelector((state) => state.update.userReadyToUpdate);
-  const waitingToInstall = useSelector((state) => state.update.waitingToInstall);
   const lastChecked = useSelector((state) => state.update.lastChecked);
 
-  const [registration, setRegistration] = useState(
-    undefined as unknown as ServiceWorkerRegistration,
-  );
-
-  useEffect(() => {
-    if (registration && waitingToInstall && userReadyToUpdate) {
-      debug('prepped update is set to install');
-      // registration.update(); // maybe to get freshest freshest?
-      registration.waiting?.postMessage({ type: 'SKIP_WAITING' });
-    }
-  }, [registration, userReadyToUpdate, waitingToInstall]);
+  // Track when registration becomes available for the update check effect
+  const [registrationReady, setRegistrationReady] = useState(false);
 
   useEffect(() => {
     const updateCheck = async () => {
-      if (registration && !lastChecked) {
+      const registration = getRegistration();
+      if (registrationReady && registration && !lastChecked) {
         debug('checking for updates');
         const reg = (await registration.update()) as unknown as ServiceWorkerRegistration;
         if (reg?.waiting) {
@@ -45,7 +36,7 @@ function UpdateManager() {
       }
     };
     updateCheck();
-  }, [dispatch, lastChecked, registration]);
+  }, [dispatch, lastChecked, registrationReady]);
 
   useEffect(() => {
     const initializeServiceWorker = () => {
@@ -64,6 +55,7 @@ function UpdateManager() {
           debug('service worker registered successfully');
           dispatch(noUpdateReady());
           setRegistration(reg);
+          setRegistrationReady(true);
 
           setInterval(() => {
             dispatch(checkForUpdate());

src/client/features/Update/UpdateMenuItem.tsx

@@ -1,14 +1,13 @@
 import GetAppIcon from '@mui/icons-material/GetApp';
 import { Badge, ListItemIcon, MenuItem, Typography } from '@mui/material';
-import { useDispatch, useSelector } from '../../store';
-import { userReadyToUpdate } from './updateSlice';
+import { useSelector } from '../../store';
+import { triggerUpdate } from './registration';
 
 function UpdateMenuItem(props: { onClick: () => void }) {
-  const dispatch = useDispatch();
   const updateNeeded = useSelector((state) => state.update.waitingToInstall);
 
   function handleOnClick() {
-    dispatch(userReadyToUpdate());
+    triggerUpdate();
     props.onClick();
   }
 

src/client/features/Update/UpdatePanel.tsx

@@ -2,7 +2,8 @@ import { Button, ButtonGroup, CircularProgress, Tooltip } from '@mui/material';
 import { Fragment } from 'react';
 import RelativeTime from '../../components/RelativeTime';
 import { useDispatch, useSelector } from '../../store';
-import { checkForUpdate, userReadyToUpdate } from './updateSlice';
+import { triggerUpdate } from './registration';
+import { checkForUpdate } from './updateSlice';
 
 function UpdatePanel() {
   const dispatch = useDispatch();
@@ -14,7 +15,7 @@ function UpdatePanel() {
     dispatch(checkForUpdate());
   };
   const updateNow = () => {
-    dispatch(userReadyToUpdate());
+    triggerUpdate();
   };
 
   return (

src/client/features/Update/registration.ts

@@ -0,0 +1,20 @@
+// Store the service worker registration outside of React/Redux since it's not serializable
+// Similar pattern to how we store the PouchDB handle in src/client/db/index.ts
+
+let registration: ServiceWorkerRegistration | undefined;
+
+export function setRegistration(reg: ServiceWorkerRegistration): void {
+  registration = reg;
+}
+
+export function getRegistration(): ServiceWorkerRegistration | undefined {
+  return registration;
+}
+
+/**
+ * Trigger the waiting service worker to skip waiting and activate.
+ * Call this directly from click handlers when user wants to update.
+ */
+export function triggerUpdate(): void {
+  registration?.waiting?.postMessage({ type: 'SKIP_WAITING' });
+}

src/client/features/Update/updateSlice.ts

@@ -2,20 +2,15 @@ import { createSlice } from '@reduxjs/toolkit';
 
 type UpdateState = {
   waitingToInstall: boolean;
-  userReadyToUpdate: boolean;
   lastChecked?: number;
 };
 
 export const updateSlice = createSlice({
   name: 'update',
   initialState: {
     waitingToInstall: false,
-    userReadyToUpdate: false,
   } as UpdateState,
   reducers: {
-    userReadyToUpdate: (state) => {
-      state.userReadyToUpdate = true;
-    },
     // TODO: we are using redux as a primitive message passing system. This is probably wrong
     // (eg by wiping lastChecked we trigger the Update component to check for an update)
 
@@ -32,6 +27,5 @@ export const updateSlice = createSlice({
   },
 });
 
-export const { noUpdateReady, updateReadyToInstall, userReadyToUpdate, checkForUpdate } =
-  updateSlice.actions;
+export const { noUpdateReady, updateReadyToInstall, checkForUpdate } = updateSlice.actions;
 export default updateSlice.reducer;