Merge pull request #68 from VariableThe/fix/audit-findings

fix: address audit findings (ESC logic, IPC types, update UX, mutex safety, ESLint)

Author: VariableThe

AUDIT_LOG.md

@@ -2,6 +2,25 @@
 
 This log tracks all significant changes, updates, and versions in the PaperCache project.
 
+## 2026-06-27
+**Change:** fix: address audit findings — ESC logic, IPC types, update UX, mutex safety, ESLint (PR #68)
+
+**Details/Why:**
+Full-pass code-quality audit surfaced several correctness bugs and robustness issues that were fixed in a single PR:
+
+1. **ESC key logic** (`useGlobalHotkey.ts`) — The window was being hidden before checking whether any overlay (graph, timer panel, reminders) was open. Restructured to dismiss overlays in priority order; window hides only as final fallback.
+2. **IPC `void` types** (`types.d.ts`, `api.ts`) — `openExternal`, `openFile`, `setLaunchAtStartup`, `quitApp` were declared as returning `void` but the underlying `invoke()` is async. Changed to `Promise<void>`; `setLaunchAtStartup` now returns its promise instead of fire-and-forget.
+3. **Silent auto-restart** (`system.rs`) — App was calling `app.restart()` immediately after an update download with no user notice. Now runs download in a background Tokio task, emits `update-ready` to the frontend (which shows a toast), waits 3 seconds, then restarts.
+4. **Mutex `unwrap()` panics** (`shortcuts.rs`) — Two `.unwrap()` calls on `Mutex::lock()` replaced with `.map_err(|e| e.to_string())?` for graceful error propagation.
+5. **Dead-end Pause button** (`TimersPage.tsx`) — Removed the ⏸ button since `resumeTimer` is an unimplemented stub; a button with no un-pause path was worse UX than no button.
+6. **macOS `AppHandle` memory leak** (`macos.rs`) — Added a `SAFETY` comment documenting why `Box::into_raw` is intentionally not paired with `Box::from_raw`.
+7. **ESLint warnings** (`GraphView.tsx`) — Introduced a `ForceGraphInstance` interface replacing the `any` ref type; snapshotted `fgRef.current` inside effect body to fix stale-ref cleanup warning.
+8. **`react`/`react-dom` dependency category** (`package.json`) — Moved from `devDependencies` to `dependencies`.
+
+**Files changed:** `src/hooks/useGlobalHotkey.ts`, `src/types.d.ts`, `src/api.ts`, `src/App.tsx`, `src-tauri/src/commands/system.rs`, `src-tauri/src/commands/shortcuts.rs`, `src-tauri/src/macos.rs`, `src/components/TimersPage.tsx`, `src/GraphView.tsx`, `package.json`, `CHANGELOG.md`.
+
+---
+
 ## 2026-06-25
 **Change:** fix: switch autostart from LaunchAgent to LoginItem (AppleScript)
 

CHANGELOG.md

@@ -14,6 +14,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Shortcut key pill "+" centered between key caps**: The `+` separator in the shortcut display was grouped inside the same `<span>` as the key cap to its left with `justify-content: space-evenly`, making it appear visually attached to the left pill. Restructured to a flat flex layout with `gap` so the `+` is independently centered with equal spacing on both sides.
 - **Login-item toggle stays in sync with macOS System Settings**: The launch-at-startup toggle only read from `localStorage`, so removing PaperCache from System Settings left the toggle permanently stuck in the checked state. Fixed by adding a `get_launch_at_startup` Tauri command that queries the actual OS login-item state via `app.autolaunch().is_enabled()`, and syncing the toggle with the real OS state on every Settings mount.
 - **Scrollbars hidden on Windows/Linux in Settings and editor**: `.settings-content` and `.editor-container` had `overflow-y: auto`/`overflow: auto` but no scrollbar-hiding rules. macOS overlay scrollbars auto-hide, but Windows/Linux show persistent scrollbars. Added `scrollbar-width: none` (Firefox), `-ms-overflow-style: none` (IE/Edge legacy), and `::-webkit-scrollbar { display: none }` (Chrome/Safari/Edge Chromium) to both containers.
+- **ESC no longer hides the window when an overlay is open**: The global hotkey handler was calling `window.hide()` before checking whether the graph view, timer panel, or reminders panel was open. Pressing Escape now correctly dismisses the top-most overlay first; the window only hides as a last resort.
+- **IPC error handling for shell/file commands**: `openExternal`, `openFile`, `setLaunchAtStartup`, and `quitApp` now properly propagate backend errors to the caller instead of silently discarding the Promise.
+- **Update notification before restart**: When an auto-update is ready, PaperCache now shows a toast ("PaperCache updated — restarting in 3 seconds…") for 3 seconds before restarting, so users are never caught off-guard.
+- **Pause button removed from timer panel**: The ⏸ pause button had no corresponding resume path (backend not implemented). Removed to avoid a dead-end UX; the close/remove button remains.
 
 ## [v0.5.3] - 2026-06-24
 

package.json

@@ -40,6 +40,8 @@
     "@types/d3-force": "^3.0.10",
     "d3-force": "^3.0.0",
     "expr-eval": "^2.0.2",
+    "react": "^19.2.6",
+    "react-dom": "^19.2.6",
     "react-force-graph-3d": "^1.29.1",
     "three": "^0.184.0"
   },
@@ -73,8 +75,6 @@
     "jsdom": "^29.1.1",
     "lint-staged": "^17.0.7",
     "prettier": "^3.8.3",
-    "react": "^19.2.6",
-    "react-dom": "^19.2.6",
     "typescript": "~6.0.2",
     "typescript-eslint": "^8.59.2",
     "vite": "^8.0.12",

src-tauri/src/commands/shortcuts.rs

@@ -59,7 +59,7 @@ pub fn update_global_shortcut(
 
     // Update state
     let state = app.state::<GlobalShortcutState>();
-    let mut map = state.shortcuts.lock().unwrap();
+    let mut map = state.shortcuts.lock().map_err(|e| e.to_string())?;
     map.insert(action, new_shortcut);
 
     Ok(())
@@ -75,7 +75,7 @@ pub fn pause_shortcuts(app: AppHandle) -> Result<(), String> {
 #[tauri::command]
 pub fn resume_shortcuts(app: AppHandle) -> Result<(), String> {
     let state = app.state::<GlobalShortcutState>();
-    let map = state.shortcuts.lock().unwrap();
+    let map = state.shortcuts.lock().map_err(|e| e.to_string())?;
 
     for (action, shortcut_str) in map.iter() {
         if let Ok(shortcut) = shortcut_str.parse::<Shortcut>() {

src-tauri/src/commands/system.rs

@@ -1,4 +1,4 @@
-use tauri::{AppHandle, Manager, WebviewWindow};
+use tauri::{AppHandle, Emitter, Manager, WebviewWindow};
 use tauri_plugin_opener::OpenerExt;
 use tauri_plugin_window_state::{AppHandleExt, StateFlags};
 
@@ -114,11 +114,16 @@ pub async fn check_for_updates(app: tauri::AppHandle) -> Result<(), String> {
     use tauri_plugin_updater::UpdaterExt;
     let updater = app.updater().map_err(|e| e.to_string())?;
 
-    // We handle the update automatically if one is available
     if let Some(update) = updater.check().await.map_err(|e| e.to_string())? {
-        // Here we could emit an event to the frontend or just download and install it
-        let _ = update.download_and_install(|_, _| {}, || {}).await;
-        app.restart();
+        // Run the download + install + restart in the background so the command
+        // returns immediately. The "update-ready" event gives the frontend 3 seconds
+        // to show a toast before the process restarts.
+        tokio::spawn(async move {
+            let _ = update.download_and_install(|_, _| {}, || {}).await;
+            let _ = app.emit("update-ready", ());
+            tokio::time::sleep(tokio::time::Duration::from_secs(3)).await;
+            app.restart();
+        });
     }
     Ok(())
 }

src-tauri/src/macos.rs

@@ -65,6 +65,11 @@ pub fn setup_power_monitor(app_handle: AppHandle) {
 
         let delegate: id = msg_send![delegate_class, new];
 
+        // SAFETY: We intentionally leak the AppHandle here. The NSNotificationCenter
+        // observer (delegate) is registered for the lifetime of the process and must
+        // always have a valid pointer to the handle. Freeing the box would invalidate
+        // the pointer stored in the Objective-C ivar, causing a use-after-free.
+        // This is a deliberate, bounded leak (one pointer per process lifetime).
         let app_box = Box::new(app_handle);
         let ptr = Box::into_raw(app_box) as *mut c_void;
         (*delegate).set_ivar("app_handle", ptr);

src/App.tsx

@@ -57,6 +57,17 @@ function App() {
     window.electronAPI.isHyprland().then((isHyp) => {
       useAppStore.getState().setIsHyprland(isHyp)
     })
+
+    // Show a toast before the app auto-restarts for an update
+    const disposeUpdateReady = window.electronAPI.onUpdateReady(() => {
+      useAppStore.getState().addToast({
+        message: '✨ PaperCache updated — restarting in 3 seconds…',
+        type: 'info',
+      })
+    })
+    return () => {
+      disposeUpdateReady()
+    }
   }, [])
 
   // Auto-dismiss toasts after 5 seconds

src/GraphView.tsx

@@ -46,6 +46,17 @@ function buildFolderCentroids(folderNames: string[]): Map<string, { cx: number;
   return centroids
 }
 
+// Minimal typing for the react-force-graph-3d instance (library ships no declarations)
+interface ForceGraphInstance {
+  controls: () => Record<string, unknown> | null
+  cameraPosition: (pos: { x: number; y: number; z: number }) => void
+  zoomToFit: (duration: number, padding: number) => void
+  graphData: () => { nodes: GraphNode[]; links: GraphLink[] } | null
+  d3Force: (name: string, force?: unknown) => unknown
+  scene: () => THREE.Scene
+  nodeThreeObject: unknown
+}
+
 export default function GraphView({
   notes,
   onClose,
@@ -54,13 +65,13 @@ export default function GraphView({
   bgColor,
   accentColor,
 }: GraphViewProps) {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  const fgRef = useRef<any>(null)
+  const fgRef = useRef<ForceGraphInstance | null>(null)
 
   const draggedNodesRef = useRef<Set<string>>(new Set())
 
   useEffect(() => {
     let raf: number
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
     let ctrls: any = null
     const setup = () => {
       const fg = fgRef.current
@@ -89,8 +100,10 @@ export default function GraphView({
   }, [])
 
   useEffect(() => {
+    // Snapshot ref at effect-run time so the cleanup reads a stable value
+    // (avoids the react-hooks/exhaustive-deps stale-ref warning)
+    const fg = fgRef.current
     return () => {
-      const fg = fgRef.current
       if (!fg) return
       const data = fg.graphData()
       if (!data || !data.nodes) return

src/api.ts

@@ -39,9 +39,7 @@ export const tauriApi: ElectronAPI = {
     return () => {}
   },
   getLaunchAtStartup: () => invoke('get_launch_at_startup'),
-  setLaunchAtStartup: (value) => {
-    invoke('set_launch_at_startup', { enabled: value })
-  },
+  setLaunchAtStartup: (value) => invoke('set_launch_at_startup', { enabled: value }),
   updateGlobalShortcut: (action, oldShortcut, newShortcut) =>
     invoke('update_global_shortcut', { action, oldShortcut, newShortcut }),
   onTriggerNewNote: (callback) => {
@@ -84,4 +82,13 @@ export const tauriApi: ElectronAPI = {
   },
   pauseShortcuts: () => invoke('pause_shortcuts') as unknown as void,
   resumeShortcuts: () => invoke('resume_shortcuts') as unknown as void,
+  onUpdateReady: (callback) => {
+    let unlisten: (() => void) | undefined
+    listen('update-ready', () => callback()).then((fn) => {
+      unlisten = fn
+    })
+    return () => {
+      if (unlisten) unlisten()
+    }
+  },
 }

src/components/TimersPage.tsx

@@ -18,7 +18,6 @@ import { listen } from '@tauri-apps/api/event'
 interface TimerItemProps {
   timer: Timer
   onRemove: (id: string) => void
-  onPause: (id: string) => void
 }
 
 function formatTime(ms: number): string {
@@ -32,7 +31,7 @@ function formatTime(ms: number): string {
   return `${String(m).padStart(2, '0')}:${String(s).padStart(2, '0')}`
 }
 
-function TimerItem({ timer, onRemove, onPause }: TimerItemProps) {
+function TimerItem({ timer, onRemove }: TimerItemProps) {
   const tickTimer = useTimerStore((s) => s.tickTimer)
   const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null)
 
@@ -75,11 +74,6 @@ function TimerItem({ timer, onRemove, onPause }: TimerItemProps) {
       <div className="timer-header">
         <span className="timer-label">{timer.label || 'Timer'}</span>
         <div className="timer-controls">
-          {timer.status === 'running' && (
-            <button className="timer-btn" onClick={() => onPause(timer.id)} title="Pause">
-              ⏸
-            </button>
-          )}
           <button
             className="timer-btn timer-btn-remove"
             onClick={() => onRemove(timer.id)}
@@ -119,7 +113,6 @@ export function TimersPage({ onClose }: TimersPageProps) {
   const timers = useTimerStore((s) => s.timers)
   const addTimer = useTimerStore((s) => s.addTimer)
   const removeTimer = useTimerStore((s) => s.removeTimer)
-  const pauseTimer = useTimerStore((s) => s.pauseTimer)
   const completeTimer = useTimerStore((s) => s.completeTimer)
   const addToast = useAppStore((s) => s.addToast)
 
@@ -284,9 +277,7 @@ export function TimersPage({ onClose }: TimersPageProps) {
               </p>
             </div>
           ) : (
-            timers.map((t) => (
-              <TimerItem key={t.id} timer={t} onRemove={handleRemove} onPause={pauseTimer} />
-            ))
+            timers.map((t) => <TimerItem key={t.id} timer={t} onRemove={handleRemove} />)
           )}
         </div>
       </div>