Skip to content

fix: resolve architecture rule violations from AGENTS.md #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
VariableThe merged 1 commit into from
Jun 20, 2026
Merged

fix: resolve architecture rule violations from AGENTS.md #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions electron/preload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,22 @@ contextBridge.exposeInMainWorld('electronAPI', {
openExternal: (url: string) => ipcRenderer.send('open-external', url),
openFile: (path: string) => ipcRenderer.send('open-file', path),
onSwipeGesture: (callback: (direction: string) => void) => {
ipcRenderer.on('swipe-gesture', (_event, direction) => callback(direction))
const handler = (_event: any, direction: string) => callback(direction)
ipcRenderer.on('swipe-gesture', handler)
return () => ipcRenderer.removeListener('swipe-gesture', handler)
},
setLaunchAtStartup: (value: boolean) => ipcRenderer.send('set-launch-startup', value),
updateGlobalShortcut: (action: string, oldShortcut: string, newShortcut: string) =>
ipcRenderer.send('update-global-shortcut', { action, oldShortcut, newShortcut }),
onTriggerNewNote: (callback: () => void) => {
ipcRenderer.on('trigger-new-note', () => callback())
const handler = () => callback()
ipcRenderer.on('trigger-new-note', handler)
return () => ipcRenderer.removeListener('trigger-new-note', handler)
},
onTriggerTasks: (callback: () => void) => {
ipcRenderer.on('trigger-tasks', () => callback())
const handler = () => callback()
ipcRenderer.on('trigger-tasks', handler)
return () => ipcRenderer.removeListener('trigger-tasks', handler)
Comment on lines 25 to +33

@coderabbitai coderabbitai Bot Jun 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Unsubscribe contract is implemented but not consumed by current renderer callers.

onTriggerNewNote/onTriggerTasks now return disposers, but src/hooks/useGlobalHotkey.ts (Lines 124-138 in provided context) does not capture or invoke them in useEffect cleanup, so listeners can still accumulate.

Suggested caller-side fix (outside this file) 
// src/hooks/useGlobalHotkey.ts (inside the relevant useEffect)
+ let offNewNote: (() => void) | undefined
+ let offTasks: (() => void) | undefined

if (window.electronAPI.onTriggerNewNote) {
-  window.electronAPI.onTriggerNewNote(() => {
+  offNewNote = window.electronAPI.onTriggerNewNote(() => {
     // ...
   })
}

if (window.electronAPI.onTriggerTasks) {
-  window.electronAPI.onTriggerTasks(() => {
+  offTasks = window.electronAPI.onTriggerTasks(() => {
     // ...
   })
}

+ return () => {
+   offNewNote?.()
+   offTasks?.()
+ }

As per coding guidelines, trace context across related files to ensure cross-file contracts are actually enforced at usage sites.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/preload.ts` around lines 25 - 33, The preload.ts file correctly
implements the disposer pattern by having onTriggerNewNote and onTriggerTasks
return cleanup functions that call ipcRenderer.removeListener. However, this
unsubscribe contract is only effective if callers actually invoke the returned
disposer functions. While the implementation in this file is correct, ensure the
disposer pattern is being properly consumed by verifying that callers in
useGlobalHotkey.ts are capturing the return values from onTriggerNewNote and
onTriggerTasks and invoking them in useEffect cleanup functions to prevent
listener accumulation.

},
safeStorageEncrypt: (val: string) => ipcRenderer.invoke('safe-storage-encrypt', val),
safeStorageDecrypt: (val: string) => ipcRenderer.invoke('safe-storage-decrypt', val),
Expand Down
24 changes: 11 additions & 13 deletions src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,17 @@ import { getSecure } from './lib/safeStorage'
import { MathEvaluator } from './lib/editor/MathEvaluator'

function App() {
const {
notes,
setNotes,
currentNoteIndex,
setCurrentNoteIndex,
zoomLevel,
showGraphView,
setShowGraphView,
showRemindersView,
setShowRemindersView,
showNoteSearch,
setShowMainActionMenu,
} = useAppStore()
const notes = useAppStore((state) => state.notes)
const setNotes = useAppStore((state) => state.setNotes)
const currentNoteIndex = useAppStore((state) => state.currentNoteIndex)
const setCurrentNoteIndex = useAppStore((state) => state.setCurrentNoteIndex)
const zoomLevel = useAppStore((state) => state.zoomLevel)
const showGraphView = useAppStore((state) => state.showGraphView)
const setShowGraphView = useAppStore((state) => state.setShowGraphView)
const showRemindersView = useAppStore((state) => state.showRemindersView)
const setShowRemindersView = useAppStore((state) => state.setShowRemindersView)
const showNoteSearch = useAppStore((state) => state.showNoteSearch)
const setShowMainActionMenu = useAppStore((state) => state.setShowMainActionMenu)

const {
themePreset,
Expand Down
14 changes: 6 additions & 8 deletions src/components/MainActionMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@ import { useAppStore } from '../store/useAppStore'
import { useSettingsStore } from '../store/useSettingsStore'

export function MainActionMenu() {
const {
notes,
currentNoteIndex,
showMainActionMenu,
setShowNoteSearch,
setShowGraphView,
setShowRemindersView,
} = useAppStore()
const notes = useAppStore((state) => state.notes)
const currentNoteIndex = useAppStore((state) => state.currentNoteIndex)
const showMainActionMenu = useAppStore((state) => state.showMainActionMenu)
const setShowNoteSearch = useAppStore((state) => state.setShowNoteSearch)
const setShowGraphView = useAppStore((state) => state.setShowGraphView)
const setShowRemindersView = useAppStore((state) => state.setShowRemindersView)

const { bgType, bgColor, textColor, fontFamily } = useSettingsStore()

Expand Down
30 changes: 14 additions & 16 deletions src/components/NoteSearch.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,20 @@ import { useAppStore } from '../store/useAppStore'
import { getFolderColor } from '../utils'

export function NoteSearch() {
const {
notes,
setNotes,
currentNoteIndex,
setCurrentNoteIndex,
showNoteSearch,
setShowNoteSearch,
noteSearchQuery,
setNoteSearchQuery,
searchSelectedIndex,
setSearchSelectedIndex,
showNoteActionMenu,
setShowNoteActionMenu,
actionMenuIndex,
setActionMenuIndex,
} = useAppStore()
const notes = useAppStore((state) => state.notes)
const setNotes = useAppStore((state) => state.setNotes)
const currentNoteIndex = useAppStore((state) => state.currentNoteIndex)
const setCurrentNoteIndex = useAppStore((state) => state.setCurrentNoteIndex)
const showNoteSearch = useAppStore((state) => state.showNoteSearch)
const setShowNoteSearch = useAppStore((state) => state.setShowNoteSearch)
const noteSearchQuery = useAppStore((state) => state.noteSearchQuery)
const setNoteSearchQuery = useAppStore((state) => state.setNoteSearchQuery)
const searchSelectedIndex = useAppStore((state) => state.searchSelectedIndex)
const setSearchSelectedIndex = useAppStore((state) => state.setSearchSelectedIndex)
const showNoteActionMenu = useAppStore((state) => state.showNoteActionMenu)
const setShowNoteActionMenu = useAppStore((state) => state.setShowNoteActionMenu)
const actionMenuIndex = useAppStore((state) => state.actionMenuIndex)
const setActionMenuIndex = useAppStore((state) => state.setActionMenuIndex)

if (!showNoteSearch) return null

Expand Down
16 changes: 7 additions & 9 deletions src/components/NoteTitleBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { useAppStore } from '../store/useAppStore'

export function NoteTitleBar() {
const {
notes,
setNotes,
currentNoteIndex,
isRenaming,
setIsRenaming,
renameValue,
setRenameValue,
} = useAppStore()
const notes = useAppStore((state) => state.notes)
const setNotes = useAppStore((state) => state.setNotes)
const currentNoteIndex = useAppStore((state) => state.currentNoteIndex)
const isRenaming = useAppStore((state) => state.isRenaming)
const setIsRenaming = useAppStore((state) => state.setIsRenaming)
const renameValue = useAppStore((state) => state.renameValue)
const setRenameValue = useAppStore((state) => state.setRenameValue)

const activeNote = notes[currentNoteIndex] || { id: 'note.md', content: '', mtime: 0 }

Expand Down Expand Up @@ -37,7 +35,7 @@
prev.map((n) => (n.id === activeNote.id ? { ...n, content: newContent } : n))
)
} catch (e) {
console.error('Failed to save note', e)

Check warning on line 38 in src/components/NoteTitleBar.tsx

View workflow job for this annotation

GitHub Actions / ci 

Unexpected console statement
}
} else {
const parts = activeNote.id.split('/')
Expand All @@ -52,7 +50,7 @@
setNotes((prev) => prev.map((n) => (n.id === activeNote.id ? { ...n, id: newId } : n)))
}
} catch (e) {
console.error('Failed to rename note', e)

Check warning on line 53 in src/components/NoteTitleBar.tsx

View workflow job for this annotation

GitHub Actions / ci 

Unexpected console statement
}
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/components/RemindersPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,15 @@ export const RemindersPage: React.FC<RemindersPageProps> = ({
const [now, setNow] = React.useState(() => Date.now())

React.useEffect(() => {
const timer = setInterval(() => setNow(Date.now()), 10000) // update every 10s for better responsiveness
return () => clearInterval(timer)
let timeoutId: ReturnType<typeof setTimeout>
const scheduleNext = () => {
timeoutId = setTimeout(() => {
setNow(Date.now())
scheduleNext()
}, 10000) // update every 10s for better responsiveness
}
scheduleNext()
return () => clearTimeout(timeoutId)
Comment on lines +71 to +79

@coderabbitai coderabbitai Bot Jun 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Prevent post-unmount re-scheduling in the recursive timer.

Line 73 can still schedule the next timeout after unmount if teardown happens while the callback is in flight, which leaves a dangling timer. Add a cancellation flag and nullable timeout guard in cleanup.

Suggested patch 
 React.useEffect(() => {
-  let timeoutId: ReturnType<typeof setTimeout>
+  let timeoutId: ReturnType<typeof setTimeout> | null = null
+  let cancelled = false
   const scheduleNext = () => {
+    if (cancelled) return
     timeoutId = setTimeout(() => {
+      if (cancelled) return
       setNow(Date.now())
       scheduleNext()
     }, 10000) // update every 10s for better responsiveness
   }
   scheduleNext()
-  return () => clearTimeout(timeoutId)
+  return () => {
+    cancelled = true
+    if (timeoutId) clearTimeout(timeoutId)
+  }
 }, [])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let timeoutId: ReturnType<typeof setTimeout>
const scheduleNext = () => {
timeoutId = setTimeout(() => {
setNow(Date.now())
scheduleNext()
}, 10000) // update every 10s for better responsiveness
}
scheduleNext()
return () => clearTimeout(timeoutId)
let timeoutId: ReturnType<typeof setTimeout> | null = null
let cancelled = false
const scheduleNext = () => {
if (cancelled) return
timeoutId = setTimeout(() => {
if (cancelled) return
setNow(Date.now())
scheduleNext()
}, 10000) // update every 10s for better responsiveness
}
scheduleNext()
return () => {
cancelled = true
if (timeoutId) clearTimeout(timeoutId)
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/RemindersPage.tsx` around lines 71 - 79, The recursive
scheduling in the scheduleNext function can create dangling timers after unmount
if the callback fires while teardown is in progress. Add a boolean flag
initialized to true (like isMounted) to track component mount state, set it to
false in the cleanup function return statement, and check this flag at the
beginning of the setTimeout callback before calling scheduleNext() again. This
prevents scheduling new timeouts after the component has unmounted.

}, [])

return (
Expand Down
26 changes: 12 additions & 14 deletions src/hooks/useGlobalHotkey.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,18 @@ import { useEffect } from 'react'
import { useAppStore } from '../store/useAppStore'

export function useGlobalHotkey() {
const {
showMainActionMenu,
showNoteSearch,
showGraphView,
setShowMainActionMenu,
setShowNoteSearch,
setShowGraphView,
setShowRemindersView,
setZoomLevel,
setNotes,
setCurrentNoteIndex,
setNoteSearchQuery,
setSearchSelectedIndex,
} = useAppStore()
const showMainActionMenu = useAppStore((state) => state.showMainActionMenu)
const showNoteSearch = useAppStore((state) => state.showNoteSearch)
const showGraphView = useAppStore((state) => state.showGraphView)
const setShowMainActionMenu = useAppStore((state) => state.setShowMainActionMenu)
const setShowNoteSearch = useAppStore((state) => state.setShowNoteSearch)
const setShowGraphView = useAppStore((state) => state.setShowGraphView)
const setShowRemindersView = useAppStore((state) => state.setShowRemindersView)
const setZoomLevel = useAppStore((state) => state.setZoomLevel)
const setNotes = useAppStore((state) => state.setNotes)
const setCurrentNoteIndex = useAppStore((state) => state.setCurrentNoteIndex)
const setNoteSearchQuery = useAppStore((state) => state.setNoteSearchQuery)
const setSearchSelectedIndex = useAppStore((state) => state.setSearchSelectedIndex)

useEffect(() => {
const handleGlobalKeyDown = async (e: KeyboardEvent) => {
Expand Down
5 changes: 4 additions & 1 deletion src/hooks/useNoteStorage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import { useAppStore } from '../store/useAppStore'
import type { Note } from '../store/useAppStore'

export function useNoteStorage() {
const { notes, setNotes, currentNoteIndex, setCurrentNoteIndex } = useAppStore()
const notes = useAppStore((state) => state.notes)
const setNotes = useAppStore((state) => state.setNotes)
const currentNoteIndex = useAppStore((state) => state.currentNoteIndex)
const setCurrentNoteIndex = useAppStore((state) => state.setCurrentNoteIndex)

// Load notes initially
useEffect(() => {
Expand Down
6 changes: 3 additions & 3 deletions src/types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ export interface ElectronAPI {
quitApp: () => void
openExternal: (url: string) => void
openFile: (path: string) => void
onSwipeGesture: (callback: (direction: string) => void) => void
onSwipeGesture: (callback: (direction: string) => void) => () => void
setLaunchAtStartup: (value: boolean) => void
updateGlobalShortcut: (action: string, oldShortcut: string, newShortcut: string) => void
onTriggerNewNote: (callback: () => void) => void
onTriggerTasks: (callback: () => void) => void
onTriggerNewNote: (callback: () => void) => () => void
onTriggerTasks: (callback: () => void) => () => void
safeStorageEncrypt: (val: string) => Promise<string>
safeStorageDecrypt: (val: string) => Promise<string>
onPowerSuspend: (callback: () => void) => () => void
Expand Down
Loading