v0.6.0 — Modes, drawing canvas, bookmarks, editor & command palette#6
v0.6.0 — Modes, drawing canvas, bookmarks, editor & command palette#6devohmycode merged 2 commits intomasterfrom
Conversation
Introduce rich-editor and bookmark features plus theme/palette support. Adds many new UI components (SuperEditor, BookmarkPanel, BookmarkReader, EditorFileList, PalettePicker, GradientText, ShinyText, SpotlightCard, etc.), services for bookmarks and editor documents, and new Supabase migrations for bookmarks and editor_documents. Update index.html to handle an 'amoled' theme and persist selected palette, and adjust App/Auth/feeds/ui helpers to integrate these features. package.json updated to include editor-related dependencies (e.g. Tiptap, Headless UI) and other supporting libraries.
Introduce five integrated app modes (Flux, Bookmark, Note, Editor, Draw) and wire up a command palette + shortcuts overlay. Adds new UI components (CommandPalette, ShortcutsOverlay, SuperDraw, updates to SuperEditor, NotePanel, SourcePanel and related components), a useCommands hook for keyboard/command registration, and services for notes and Pandoc export. Includes a Supabase migration to support notes, updates to App.tsx and the Tauri backend, README/AGENTS documentation updates, and package.json dependency changes to support new features.
Review Summary by Qodov0.6.0 — Multi-mode integration with SuperDraw, SuperEditor, SuperBookmark, command palette, and color themes
WalkthroughsDescription• **5 integrated modes** with icon tab bar and Ctrl+1–5 shortcuts: SuperFlux (RSS), SuperBookmark, SuperNote, SuperEditor, SuperDraw • **SuperDraw** — custom canvas drawing tool with 10 tools (select, hand, rect, ellipse, diamond, line, arrow, freehand, text, eraser), color/fill picker, stroke width, font size selector, dark/light toggle, zoom/pan, undo/redo, selection with resize handles, PNG export • **SuperEditor** — rich text document editor with TipTap, folder organization, Pandoc export (PDF, DOCX, HTML), emoji suggestions, and task lists • **SuperBookmark** — web bookmark manager with metadata extraction, reader mode with article extraction, and dual view modes (cards/list) • **Command palette** (Ctrl+K) with fuzzy search, keyboard shortcuts overlay (?), and 20+ registered commands • **Mode tab bar** (icon-only) in the source panel with mode switching and palette picker integration • **Markdown rendering** in sticky notes (post-it board + card view) with resizable sticky notes via react-markdown • **Color palette system** with 10 customizable themes (Amber, Ocean, Forest, Sunset, Lavender, Rosewood, Mint, Neon, Slate) and AMOLED dark theme • **Supabase migrations** for bookmarks, editor documents, and notes with RLS policies and optimized indexes • **Tauri Pandoc integration** for document import/export with base64 encoding and temporary file handling • **Remote deletion detection** in sync service to prevent orphaned feed/item re-syncing • **Feed reordering** via drag-and-drop with programmatic panel width control • **New UI components**: ShinyText, GradientText, SpotlightCard, PalettePicker, EditorFileList, BookmarkReader, CommandPalette, ShortcutsOverlay • **Documentation** updated with feature overview, keyboard shortcuts, architecture patterns, and agent guidance Diagramflowchart LR
A["App.tsx<br/>Mode Switching"] -->|Ctrl+1-5| B["5 Modes"]
B --> C["SuperFlux<br/>RSS Reader"]
B --> D["SuperBookmark<br/>Web Manager"]
B --> E["SuperNote<br/>Sticky Board"]
B --> F["SuperEditor<br/>Rich Text"]
B --> G["SuperDraw<br/>Canvas Tool"]
A -->|Ctrl+K| H["Command Palette<br/>Fuzzy Search"]
A -->|?| I["Shortcuts Overlay"]
J["SourcePanel"] -->|Mode Tabs| A
J -->|Palette Picker| K["10 Color Themes"]
F -->|Pandoc| L["Export DOCX/PDF"]
D -->|Article Extract| M["Bookmark Reader"]
E -->|Markdown| N["Sticky Notes"]
O["Supabase"] -->|Sync| P["Bookmarks<br/>Documents<br/>Notes"]
Q["syncService"] -->|Orphan Detection| O
File Changes1. src/themes/palettes.ts
|
Code Review by Qodo
1. Unsanitized article.content HTML
|
📝 WalkthroughWalkthroughThis PR introduces a comprehensive productivity suite expansion adding rich-text editor (SuperEditor with TipTap), drawing canvas (SuperDraw), bookmark management (SuperBookmark), enhanced notes with Markdown rendering and sticky board, command palette system, Pandoc document conversion, improved theme system with AMOLED and palette support, and extensive state synchronization for editor documents, bookmarks, and notes via Supabase migrations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant SuperEditor
participant PandocService
participant EditorDocService
participant Supabase
User->>App: Switch to Editor mode
App->>SuperEditor: Load active doc
User->>SuperEditor: Import DOCX file
SuperEditor->>PandocService: importWithPandoc(file)
alt Desktop (Tauri)
PandocService->>PandocService: Encode file to base64
PandocService->>App: invoke pandoc_import
App-->>PandocService: Return HTML
else Web
PandocService->>PandocService: Use Mammoth for DOCX
end
PandocService-->>SuperEditor: HTML content
SuperEditor->>App: onUpdateContent(html)
App->>EditorDocService: updateEditorDocContent(userId, docId, html)
EditorDocService->>Supabase: Update editor_documents
Supabase-->>EditorDocService: Success
EditorDocService-->>App: Confirm
App->>User: Editor displays imported content
User->>SuperEditor: Export to PDF
SuperEditor->>PandocService: exportWithPandoc(html, 'pdf')
alt Desktop (Tauri)
PandocService->>App: invoke pandoc_export
App-->>PandocService: Return base64 PDF
PandocService->>PandocService: Convert to Blob
else Web
PandocService-->>User: Error - unavailable
end
PandocService-->>SuperEditor: Blob
SuperEditor->>User: Download PDF
sequenceDiagram
participant User
participant App
participant BookmarkPanel
participant BookmarkService
participant ArticleExtractor as ArticleExtractor<br/>(extractArticle)
participant Supabase
User->>App: Login
App->>BookmarkService: fetchBookmarks(userId)
BookmarkService->>Supabase: SELECT from bookmarks
Supabase-->>BookmarkService: Bookmark array
BookmarkService-->>App: Display bookmarks
User->>BookmarkPanel: Click bookmark
BookmarkPanel->>App: onSelectBookmark(id)
App->>BookmarkReader: Pass bookmark
BookmarkReader->>ArticleExtractor: extractArticle(url)
ArticleExtractor-->>BookmarkReader: {title, content, image, date}
BookmarkReader->>BookmarkService: toggleBookmarkRead(userId, id, true)
BookmarkService->>Supabase: UPDATE is_read
Supabase-->>BookmarkService: Success
BookmarkReader->>User: Display article in reader mode
User->>BookmarkPanel: Toggle read status
BookmarkPanel->>BookmarkService: toggleBookmarkRead(userId, id, isRead)
BookmarkService->>Supabase: UPDATE is_read
Supabase-->>BookmarkPanel: Confirm
BookmarkPanel->>User: Update UI optimistically
sequenceDiagram
participant User
participant App
participant CommandPalette
participant useCommands Hook
participant Command Action
User->>App: Press Ctrl+K
App->>useCommands Hook: Global keydown listener
useCommands Hook->>useCommands Hook: Match Ctrl+K
useCommands Hook->>App: openPalette()
App->>CommandPalette: isOpen=true, commands=ref
CommandPalette->>User: Show palette, focus input
User->>CommandPalette: Type search query
CommandPalette->>CommandPalette: Fuzzy match against commands
CommandPalette->>User: Display filtered results
User->>CommandPalette: Press Enter on selected command
CommandPalette->>App: onClose()
App->>useCommands Hook: closePalette()
useCommands Hook->>useCommands Hook: Find matching command
useCommands Hook->>Command Action: Execute action
Command Action->>App: Update state/trigger flow
App->>User: Handle command (navigate, toggle panel, etc.)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| <div | ||
| className="bk-reader-body" | ||
| dangerouslySetInnerHTML={{ __html: article.content }} | ||
| /> |
There was a problem hiding this comment.
1. Unsanitized article.content html 📘 Rule violation ⛨ Security
article.content from external pages is rendered via dangerouslySetInnerHTML without sanitization, enabling XSS if the extracted content contains scripts or malicious markup. This violates the requirement to validate/sanitize external inputs before rendering.
Agent Prompt
## Issue description
`article.content` (external, untrusted HTML) is injected into the DOM with `dangerouslySetInnerHTML` without sanitization, creating an XSS risk.
## Issue Context
`extractArticle(bookmark.url)` fetches/parses content from arbitrary websites, so `article.content` must be treated as untrusted.
## Fix Focus Areas
- src/components/BookmarkReader.tsx[216-219]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| } catch (err: any) { | ||
| setImportError(err?.message || 'Erreur lors de l\'import'); | ||
| } | ||
| e.target.value = ''; | ||
| }, [editor, doc, onUpdateContent]); | ||
|
|
||
| const handleExport = useCallback(async (format: 'docx' | 'pdf') => { | ||
| if (!editor) return; | ||
| setImportError(null); | ||
| try { | ||
| const html = editor.getHTML(); | ||
| const blob = await exportWithPandoc(html, format); | ||
| const title = doc?.title || 'supereditor-document'; | ||
| const url = URL.createObjectURL(blob); | ||
| const a = document.createElement('a'); | ||
| a.href = url; | ||
| a.download = `${title.replace(/[^a-zA-Z0-9-_ ]/g, '')}.${format}`; | ||
| a.click(); | ||
| URL.revokeObjectURL(url); | ||
| } catch (err: any) { | ||
| setImportError(err?.message || 'Erreur lors de l\'export'); | ||
| } | ||
| }, [editor, doc?.title]); | ||
|
|
||
| // Keyboard shortcuts for file menu | ||
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if (!editor) return; | ||
| if (e.ctrlKey && e.key === 'n') { e.preventDefault(); handleNew(); } | ||
| if (e.ctrlKey && e.key === 'o') { e.preventDefault(); handleOpen(); } | ||
| if (e.ctrlKey && e.key === 's') { e.preventDefault(); handleDownload(); } | ||
| }; | ||
| window.addEventListener('keydown', handleKeyDown); | ||
| return () => window.removeEventListener('keydown', handleKeyDown); | ||
| }, [editor, handleNew, handleOpen, handleDownload]); | ||
|
|
||
| if (!editor) return null; | ||
|
|
||
| const chars = editor.storage.characterCount?.characters?.() ?? 0; | ||
| const words = editor.storage.characterCount?.words?.() ?? 0; | ||
|
|
||
| return ( | ||
| <div className="super-editor-root"> | ||
| <input | ||
| ref={fileInputRef} | ||
| type="file" | ||
| accept=".html,.htm,.txt,.md" | ||
| style={{ display: 'none' }} | ||
| onChange={handleFileChange} | ||
| /> | ||
| <input | ||
| ref={importInputRef} | ||
| type="file" | ||
| accept=".docx,.pdf" | ||
| style={{ display: 'none' }} | ||
| onChange={handleImportChange} | ||
| /> | ||
| {/* Toolbar */} | ||
| <div className="super-editor-toolbar"> | ||
| <div className="super-editor-toolbar-row"> | ||
| <FileMenu onNew={handleNew} onOpen={handleOpen} onDownload={handleDownload} onImport={handleImport} onExport={handleExport} /> | ||
| <ToolSep /> | ||
| <ToolBtn icon={Bold} label="Gras" active={editor.isActive('bold')} onClick={() => editor.chain().focus().toggleBold().run()} /> | ||
| <ToolBtn icon={Italic} label="Italique" active={editor.isActive('italic')} onClick={() => editor.chain().focus().toggleItalic().run()} /> | ||
| <ToolBtn icon={UnderlineIcon} label="Souligné" active={editor.isActive('underline')} onClick={() => editor.chain().focus().toggleUnderline().run()} /> | ||
| <ToolBtn icon={Strikethrough} label="Barré" active={editor.isActive('strike')} onClick={() => editor.chain().focus().toggleStrike().run()} /> | ||
| <ToolBtn icon={Code} label="Code" active={editor.isActive('code')} onClick={() => editor.chain().focus().toggleCode().run()} /> | ||
|
|
||
| <ToolSep /> | ||
|
|
||
| <ToolBtn icon={Heading1} label="H1" active={editor.isActive('heading', { level: 1 })} onClick={() => editor.chain().focus().toggleHeading({ level: 1 }).run()} /> | ||
| <ToolBtn icon={Heading2} label="H2" active={editor.isActive('heading', { level: 2 })} onClick={() => editor.chain().focus().toggleHeading({ level: 2 }).run()} /> | ||
| <ToolBtn icon={Heading3} label="H3" active={editor.isActive('heading', { level: 3 })} onClick={() => editor.chain().focus().toggleHeading({ level: 3 }).run()} /> | ||
| <ToolBtn icon={Heading4} label="H4" active={editor.isActive('heading', { level: 4 })} onClick={() => editor.chain().focus().toggleHeading({ level: 4 }).run()} /> | ||
|
|
||
| <ToolSep /> | ||
|
|
||
| <ToolBtn icon={AlignLeft} label="Gauche" active={editor.isActive({ textAlign: 'left' })} onClick={() => editor.chain().focus().setTextAlign('left').run()} /> | ||
| <ToolBtn icon={AlignCenter} label="Centre" active={editor.isActive({ textAlign: 'center' })} onClick={() => editor.chain().focus().setTextAlign('center').run()} /> | ||
| <ToolBtn icon={AlignRight} label="Droite" active={editor.isActive({ textAlign: 'right' })} onClick={() => editor.chain().focus().setTextAlign('right').run()} /> | ||
| <ToolBtn icon={AlignJustify} label="Justifié" active={editor.isActive({ textAlign: 'justify' })} onClick={() => editor.chain().focus().setTextAlign('justify').run()} /> | ||
|
|
||
| <ToolSep /> | ||
|
|
||
| <ToolBtn icon={List} label="Liste à puces" active={editor.isActive('bulletList')} onClick={() => editor.chain().focus().toggleBulletList().run()} /> | ||
| <ToolBtn icon={ListOrdered} label="Liste numérotée" active={editor.isActive('orderedList')} onClick={() => editor.chain().focus().toggleOrderedList().run()} /> | ||
| <ToolBtn icon={ListTodo} label="Liste de tâches" active={editor.isActive('taskList')} onClick={() => editor.chain().focus().toggleTaskList().run()} /> | ||
| <ToolBtn icon={Quote} label="Citation" active={editor.isActive('blockquote')} onClick={() => editor.chain().focus().toggleBlockquote().run()} /> | ||
| <ToolBtn icon={FileCode} label="Bloc de code" active={editor.isActive('codeBlock')} onClick={() => editor.chain().focus().toggleCodeBlock().run()} /> | ||
| <ToolBtn icon={Minus} label="Séparateur" onClick={() => editor.chain().focus().setHorizontalRule().run()} /> | ||
|
|
||
| <ToolSep /> | ||
|
|
||
| <ToolBtn icon={LinkIcon} label="Lien" active={editor.isActive('link')} onClick={() => { | ||
| const url = window.prompt('URL du lien'); | ||
| if (url) editor.chain().focus().extendMarkRange('link').setLink({ href: url }).run(); | ||
| }} /> | ||
| <ToolBtn icon={Unlink} label="Supprimer lien" onClick={() => editor.chain().focus().unsetLink().run()} /> | ||
| <ToolBtn icon={ImageIcon} label="Image" onClick={() => { | ||
| const url = window.prompt('URL de l\'image'); | ||
| if (url) editor.chain().focus().setImage({ src: url }).run(); | ||
| }} /> | ||
|
|
||
| <ToolSep /> | ||
|
|
||
| <ToolBtn icon={SubscriptIcon} label="Indice" active={editor.isActive('subscript')} onClick={() => editor.chain().focus().toggleSubscript().run()} /> | ||
| <ToolBtn icon={SuperscriptIcon} label="Exposant" active={editor.isActive('superscript')} onClick={() => editor.chain().focus().toggleSuperscript().run()} /> | ||
|
|
||
| <ToolSep /> | ||
|
|
||
| <ToolBtn icon={Undo2} label="Annuler" disabled={!editor.can().undo()} onClick={() => editor.chain().focus().undo().run()} /> | ||
| <ToolBtn icon={Redo2} label="Rétablir" disabled={!editor.can().redo()} onClick={() => editor.chain().focus().redo().run()} /> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Editor content with drag handle */} | ||
| <div className="super-editor-content-wrapper"> | ||
| <DragHandle editor={editor}> | ||
| <div className="super-editor-drag-handle"> | ||
| <GripVertical size={14} /> | ||
| </div> | ||
| </DragHandle> | ||
| <EditorContent editor={editor} className="super-editor-content" /> | ||
| </div> | ||
|
|
||
| {/* Import/Export error toast */} | ||
| {importError && ( | ||
| <div className="super-editor-toast" onClick={() => setImportError(null)}> | ||
| <span>⚠ {importError}</span> | ||
| </div> |
There was a problem hiding this comment.
2. importerror exposes raw error 📘 Rule violation ⛨ Security
The UI displays err.message directly to users via the import/export toast, which can leak internal details (e.g., stack/context, file paths, backend error text). User-facing errors should be generic while detailed errors go only to internal logs.
Agent Prompt
## Issue description
The SuperEditor import/export flow surfaces `err.message` directly to users, potentially leaking internal details.
## Issue Context
Compliance requires generic user-facing errors; detailed diagnostics should go to internal logs only.
## Fix Focus Areas
- src/components/SuperEditor.tsx[344-346]
- src/components/SuperEditor.tsx[469-473]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export async function fetchNotes(userId: string): Promise<NoteRow[]> { | ||
| console.log('[notes] isSupabaseConfigured:', isSupabaseConfigured, 'userId:', userId); | ||
| if (!isSupabaseConfigured) { console.warn('[notes] Supabase not configured'); return []; } | ||
|
|
||
| const { data, error, status } = await supabase | ||
| .from('notes') | ||
| .select('*') | ||
| .eq('user_id', userId) | ||
| .order('updated_at', { ascending: false }); | ||
|
|
||
| console.log('[notes] fetch response — status:', status, 'data:', data?.length, 'error:', error); | ||
|
|
||
| if (error) { | ||
| console.error('[notes] fetch error:', error); | ||
| return []; | ||
| } | ||
|
|
||
| return data ?? []; |
There was a problem hiding this comment.
3. fetchnotes logs userid 📘 Rule violation ⛨ Security
New console logs include userId and full error objects, which can expose sensitive identifiers and potentially sensitive backend details in logs. Logs are also unstructured, reducing audit/debug usefulness and violating secure logging requirements.
Agent Prompt
## Issue description
`fetchNotes()` logs sensitive identifiers (`userId`) and raw error objects to the console, and the log format is unstructured.
## Issue Context
Compliance requires logs to avoid PII/sensitive data and be structured for auditing/monitoring.
## Fix Focus Areas
- src/services/noteService.ts[19-33]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| fn pandoc_import(base64_data: String, filename: String) -> Result<String, String> { | ||
| let bytes = STANDARD.decode(&base64_data) | ||
| .map_err(|e| format!("base64 decode error: {e}"))?; | ||
|
|
||
| let tmp_dir = std::env::temp_dir().join("superflux_pandoc"); | ||
| std::fs::create_dir_all(&tmp_dir) | ||
| .map_err(|e| format!("Failed to create temp dir: {e}"))?; | ||
|
|
||
| let input_path = tmp_dir.join(&filename); | ||
| std::fs::write(&input_path, &bytes) | ||
| .map_err(|e| format!("Failed to write temp file: {e}"))?; |
There was a problem hiding this comment.
4. Pandoc path traversal 🐞 Bug ⛨ Security
pandoc_import writes a temp file using tmp_dir.join(filename) where filename is provided by the frontend. Absolute paths or '..' segments can escape the temp directory, enabling arbitrary file overwrite on the user’s machine.
Agent Prompt
### Issue description
`pandoc_import` uses a user-provided `filename` to build a filesystem path (`tmp_dir.join(&filename)`), allowing path traversal / absolute-path override.
### Issue Context
Frontend passes `file.name` to the backend.
### Fix Focus Areas
- src-tauri/src/lib.rs[645-674]
- src/services/pandocService.ts[15-26]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
♻️ Duplicate comments (1)
src/components/SettingsModal.tsx (1)
78-83: Duplicated color-mapping logic — already noted inmain.tsxreview.Same amoled/dark/light branching; same refactor recommendation applies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SettingsModal.tsx` around lines 78 - 83, The color-mapping logic that computes isAmoled/isDark and r,g,b is duplicated; extract it into a reusable helper (e.g., a function named getThemeRGB or mapThemeToRGB) and replace the inlined block in SettingsModal (the isAmoled/isDark checks and r,g,b assignments) with a call to that helper; ensure the helper accepts the document element or reads document.documentElement and returns an {r,g,b} object so main.tsx and SettingsModal.tsx can both call the same function to avoid duplication.
🟡 Minor comments (11)
AGENTS.md-12-25 (1)
12-25:⚠️ Potential issue | 🟡 MinorAdd fenced code block languages (markdownlint MD040).
Line 12 and Line 21 code fences should declare a language (e.g.,
bash) to satisfy linting.📝 Proposed fix
-``` +```bash npm run dev # Full Tauri dev mode (Vite + Rust backend) npm run dev:app # Frontend only (Vite on port 5173, no Tauri) npm run dev:proxy # CORS proxy server on port 3001 (for browser-only dev)@@
-+bash
npm run build # Full Tauri build (frontend + Rust → installer)
npm run build:frontend # TypeScript check + Vite build only
npm run lint # ESLint on all .ts/.tsx files🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 12 - 25, Add explicit language identifiers to the fenced code blocks in AGENTS.md: update the first code block (the dev scripts block shown with npm run dev/dev:app/dev:proxy) to use ```bash and update the second code block (the Build & Lint block with npm run build/build:frontend/npm run lint) to use ```bash so the markdown linter MD040 is satisfied; edit the two fenced blocks in AGENTS.md accordingly.src/components/SpotlightCard.tsx-39-41 (1)
39-41:⚠️ Potential issue | 🟡 MinorAvoid initial top-left spotlight flash on hover.
Line 40 sets opacity before pointer position is initialized for the current hover event, so the first frame can render at
(0,0).💡 Proposed fix
- const handleMouseEnter = () => { - setOpacity(0.6); - }; + const handleMouseEnter: React.MouseEventHandler<HTMLDivElement> = e => { + if (divRef.current) { + const rect = divRef.current.getBoundingClientRect(); + setPosition({ x: e.clientX - rect.left, y: e.clientY - rect.top }); + } + setOpacity(0.6); + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SpotlightCard.tsx` around lines 39 - 41, The initial opacity is being set in handleMouseEnter before the pointer coordinates are initialized, causing a flash at (0,0); modify handleMouseEnter to accept the mouse event, derive and set the spotlight coordinates (e.g., update spotlightX/spotlightY or call setSpotlightPosition using event.clientX/Y or relative element coordinates) first, then setOpacity(0.6) (or set opacity inside requestAnimationFrame after coordinates are set) so the first rendered frame uses the correct pointer position; reference handleMouseEnter and setOpacity (and the spotlight coordinate state setters) when making the change.src/services/pandocService.ts-29-33 (1)
29-33:⚠️ Potential issue | 🟡 MinorCase-sensitive file extension check may reject
.DOCXfiles.
file.name.endsWith('.docx')won't match uppercase or mixed-case extensions (e.g.,.DOCX,.Docx), which some Windows users may have. Consider normalizing the name.🔧 Proposed fix
- if (file.name.endsWith('.docx')) { + if (file.name.toLowerCase().endsWith('.docx')) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/pandocService.ts` around lines 29 - 33, The extension check `file.name.endsWith('.docx')` is case-sensitive and will miss `.DOCX`/`.Docx`; update the condition in pandocService.ts to perform a case-insensitive check (e.g., normalize with `file.name.toLowerCase()` or use a case-insensitive regex) before calling `mammoth.convertToHtml({ arrayBuffer: buffer })`, so any capitalization of `.docx` is accepted.supabase/migrations/20250225000006_editor_documents.sql-5-14 (1)
5-14:⚠️ Potential issue | 🟡 Minor
created_atandupdated_atlack NOT NULL constraints.Both columns default to
now()but aren't declaredNOT NULL, so an explicitINSERT ... (created_at) VALUES (NULL)would succeed. If these should always be populated, addNOT NULL.🔧 Suggested fix
- created_at timestamptz default now(), - updated_at timestamptz default now(), + created_at timestamptz not null default now(), + updated_at timestamptz not null default now(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20250225000006_editor_documents.sql` around lines 5 - 14, The created_at and updated_at columns in the editor_documents table are missing NOT NULL and therefore can be set to NULL despite having defaults; alter the CREATE TABLE definition for editor_documents to declare created_at timestamptz NOT NULL default now() and updated_at timestamptz NOT NULL default now() so those columns (created_at, updated_at) cannot be inserted as NULL.src/components/SuperDraw.tsx-357-368 (1)
357-368:⚠️ Potential issue | 🟡 MinorResize can produce negative
w/hwhen handles are dragged past each other.When dragging the
nwhandle far past thesecorner (or vice-versa),wandhcan become negative. This will causehitTest,paintSel, andgetHandlesto return incorrect results since they assume non-negative dimensions. Consider normalizing after resize:🔧 Suggested normalization
return { ...el, x, y, w, h }; + // After computing x, y, w, h: + const nx = w < 0 ? x + w : x; + const ny = h < 0 ? y + h : y; + return { ...el, x: nx, y: ny, w: Math.abs(w), h: Math.abs(h) };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SuperDraw.tsx` around lines 357 - 368, The resize branch in the setEls mapping can produce negative w/h when handles cross; normalize width/height and adjust x/y after computing new values so downstream code (hitTest, paintSel, getHandles) always sees non-negative dimensions: inside the cur.type === 'resize' case in the setEls callback (the closure that maps el => { ... }), after computing x, y, w, h apply normalization (if w < 0 then x += w; w = Math.abs(w); if h < 0 then y += h; h = Math.abs(h)) and return the normalized { ...el, x, y, w, h } so other functions can assume non-negative sizes.src/components/BookmarkReader.tsx-103-103 (1)
103-103:⚠️ Potential issue | 🟡 Minor
new URL(bookmark.url)can throw on malformed URLs.If
bookmark.urlis invalid, this will throw and crash the component during render.Proposed fix
- <span className="bk-reader-site">{bookmark.site_name || new URL(bookmark.url).hostname}</span> + <span className="bk-reader-site">{bookmark.site_name || (() => { try { return new URL(bookmark.url).hostname; } catch { return bookmark.url; } })()}</span>Or extract a helper:
function safeHostname(url: string): string { try { return new URL(url).hostname; } catch { return url; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/BookmarkReader.tsx` at line 103, The render uses new URL(bookmark.url) which can throw for malformed URLs; update BookmarkReader.tsx to avoid throwing by replacing the inline new URL call with a safe helper (e.g., safeHostname) that wraps new URL(bookmark.url) in a try/catch and returns a fallback (like the original url or empty string) on error, and use that helper where you currently reference bookmark.site_name || new URL(bookmark.url).hostname.src/components/GradientText.tsx-66-74 (1)
66-74:⚠️ Potential issue | 🟡 Minor
diagonaldirection produces the samebackgroundPositionashorizontal.The
elsebranch (line 72) is${p}% 50%, identical to thehorizontalbranch. For a diagonal motion the vertical component should also vary.🐛 Proposed fix
if (direction === 'horizontal') { return `${p}% 50%`; } else if (direction === 'vertical') { return `50% ${p}%`; } else { - return `${p}% 50%`; + return `${p}% ${p}%`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/GradientText.tsx` around lines 66 - 74, The backgroundPosition useTransform handler incorrectly treats the "diagonal" case the same as "horizontal"; update the else branch in the backgroundPosition calculation (inside the useTransform for progress) so that when direction === 'diagonal' both X and Y components vary (e.g., use p for both axes) instead of returning the horizontal value; adjust the conditional to explicitly handle 'diagonal' or change the default return to compute `${p}% ${p}%` so diagonal motion changes both coordinates.src/hooks/useCommands.ts-32-43 (1)
32-43:⚠️ Potential issue | 🟡 Minor
meta-only keybinds silently never match;targetKeyternary is a no-op.
Line 35: The ternary chain always returns
kb.keyunchanged — it can be simplified toconst targetKey = kb.key;.Line 38 merges
ctrlKey || metaKeyinto a single check againstkb.ctrl, butkb.metais never checked. A command registered withmeta: true, ctrl: falsewill never match because pressing Meta makes(e.ctrlKey || e.metaKey)true while!!kb.ctrlis false. This is fine today since all shortcuts useCtrl+…, but it's a latent bug if anyone registers aMeta-only binding.🛡️ Proposed fix
function matchesKeybind(e: KeyboardEvent, kb: KeyBind): boolean { const key = e.key.toLowerCase(); - // Handle special cases - const targetKey = kb.key === ',' ? ',' : kb.key === '/' ? '/' : kb.key === '?' ? '?' : kb.key; - - if (key !== targetKey) return false; - if (!!kb.ctrl !== (e.ctrlKey || e.metaKey)) return false; + if (key !== kb.key) return false; + const wantCtrlOrMeta = !!kb.ctrl || !!kb.meta; + const hasCtrlOrMeta = e.ctrlKey || e.metaKey; + if (wantCtrlOrMeta !== hasCtrlOrMeta) return false; if (!!kb.alt !== e.altKey) return false; if (!!kb.shift !== e.shiftKey) return false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useCommands.ts` around lines 32 - 43, The matchesKeybind function incorrectly builds targetKey with a no-op ternary and conflates Ctrl and Meta into a single check, so meta-only bindings never match; simplify targetKey to use kb.key directly and change the modifier checks to test e.ctrlKey and e.metaKey separately against kb.ctrl and kb.meta (i.e., verify !!kb.ctrl === e.ctrlKey, !!kb.meta === e.metaKey, and !!kb.alt === e.altKey, !!kb.shift === e.shiftKey) inside matchesKeybind so Meta-only, Ctrl-only, and combined modifiers all match correctly.src/components/SuperEditor.tsx-372-375 (1)
372-375:⚠️ Potential issue | 🟡 MinorShortcut handling excludes macOS Cmd keys.
At Lines 372-374, only
Ctrlis handled.Cmd+N/O/Swon’t work on macOS.Proposed fix
useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { if (!editor) return; - if (e.ctrlKey && e.key === 'n') { e.preventDefault(); handleNew(); } - if (e.ctrlKey && e.key === 'o') { e.preventDefault(); handleOpen(); } - if (e.ctrlKey && e.key === 's') { e.preventDefault(); handleDownload(); } + const mod = e.ctrlKey || e.metaKey; + const key = e.key.toLowerCase(); + if (mod && key === 'n') { e.preventDefault(); handleNew(); } + if (mod && key === 'o') { e.preventDefault(); handleOpen(); } + if (mod && key === 's') { e.preventDefault(); handleDownload(); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SuperEditor.tsx` around lines 372 - 375, The shortcut checks only look for Ctrl keys, so macOS Command shortcuts won't work; update the keydown handler in SuperEditor.tsx to accept either e.ctrlKey or e.metaKey (e.g., use if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'n') { e.preventDefault(); handleNew(); } and similarly for handleOpen() and handleDownload()), ensuring you normalize e.key to lowercase and still call preventDefault().src/components/SourcePanel.tsx-885-905 (1)
885-905:⚠️ Potential issue | 🟡 Minor
+action is inconsistent in draw mode (wrong title and no action path).In draw mode, the tooltip falls back to bookmark wording, and click handling has no draw branch.
Proposed fix
<button className="footer-btn footer-btn-add" title={ brandMode === 'flux' ? 'Ajouter un flux' : brandMode === 'note' ? 'Nouvelle note' : brandMode === 'editor' ? 'Nouveau document' : - 'Ajouter un bookmark' + brandMode === 'bookmark' ? 'Ajouter un bookmark' : + 'Aucune action rapide en mode dessin' } + disabled={brandMode === 'draw'} onClick={() => { if (brandMode === 'flux') { @@ } else if (brandMode === 'bookmark') { setBookmarkUrlOpen(prev => !prev); setTimeout(() => bookmarkUrlRef.current?.focus(), 50); + } else if (brandMode === 'draw') { + return; } }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SourcePanel.tsx` around lines 885 - 905, The tooltip/title and click handler need a draw branch: update the title expression (the title prop using brandMode) to include brandMode === 'draw' with the correct label (e.g., "Nouveau dessin" or whatever the UI copy requires) instead of falling back to bookmark text, and add a matching branch in the onClick handler for brandMode === 'draw' that invokes the draw action (e.g., open the draw modal/state — reference the same mechanism you use for other modals such as setIsAddModalOpen or an existing draw-specific setter or callback). Ensure you reference brandMode in both places and use the existing modal/callback helpers (setIsAddModalOpen, onAddNote, onAddDoc, setBookmarkUrlOpen, bookmarkUrlRef) to implement the draw behavior consistently with the other branches.src/components/NoteStickyBoard.tsx-141-146 (1)
141-146:⚠️ Potential issue | 🟡 MinorPersist final resize dimensions from pointer coordinates, not potentially stale state.
At Line 145,
onResizeEnd(size.w, size.h)can save stale values if the lastsetSizefrom Line 135-138 hasn’t committed yet.Proposed fix
+ const clampSize = useCallback((w: number, h: number) => ({ + w: Math.max(MIN_STICKY_W, Math.min(MAX_STICKY_W, w)), + h: Math.max(MIN_STICKY_H, Math.min(MAX_STICKY_H, h)), + }), []); + const handleResizePointerMove = useCallback((e: React.PointerEvent) => { if (!isResizing) return; const dx = e.clientX - resizeStart.current.mouseX; const dy = e.clientY - resizeStart.current.mouseY; - setSize({ - w: Math.max(MIN_STICKY_W, Math.min(MAX_STICKY_W, resizeStart.current.w + dx)), - h: Math.max(MIN_STICKY_H, Math.min(MAX_STICKY_H, resizeStart.current.h + dy)), - }); - }, [isResizing]); + setSize(clampSize(resizeStart.current.w + dx, resizeStart.current.h + dy)); + }, [isResizing, clampSize]); const handleResizePointerUp = useCallback((e: React.PointerEvent) => { if (!isResizing) return; (e.target as HTMLElement).releasePointerCapture(e.pointerId); + const dx = e.clientX - resizeStart.current.mouseX; + const dy = e.clientY - resizeStart.current.mouseY; + const next = clampSize(resizeStart.current.w + dx, resizeStart.current.h + dy); + setSize(next); setIsResizing(false); - onResizeEnd(size.w, size.h); - }, [isResizing, size, onResizeEnd]); + onResizeEnd(next.w, next.h); + }, [isResizing, onResizeEnd, clampSize]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/NoteStickyBoard.tsx` around lines 141 - 146, handleResizePointerUp currently calls onResizeEnd(size.w, size.h) which can read stale React state; update the handler to derive the final dimensions from the pointer event or a ref that is updated during pointer moves. Concretely, either compute final width/height from the pointer event (e.g., using e.clientX/e.clientY and the note element's getBoundingClientRect) or maintain a lastSizeRef that you update alongside setSize inside the pointer move handler, then call onResizeEnd(lastSizeRef.current.w, lastSizeRef.current.h) in handleResizePointerUp; keep the existing calls to releasePointerCapture and setIsResizing(false) in handleResizePointerUp.
🧹 Nitpick comments (19)
src/services/rssService.ts (1)
359-360: LGTM — correct early-return guard.The
!feed.urlshort-circuit already coversnull,undefined, and''; the additionalfeed.url.trim() === ''clause extends coverage to whitespace-only strings, which is a sensible defensive touch. Placement before any network I/O is correct.One optional simplification — the two conditions can be collapsed:
🔧 Optional simplification
- if (!feed.url || feed.url.trim() === '') return []; + if (!feed.url?.trim()) return [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/rssService.ts` around lines 359 - 360, Collapse the two early-return checks into a single null/empty/whitespace guard to simplify the logic: replace the separate "!feed.url || feed.url.trim() === ''" checks with a single expression that trims safely (e.g., using optional chaining or a null-safe check) and returns early when the result is falsy; update the guard around the feed processing code where "feed.url" is validated so that whitespace-only URLs are still rejected while avoiding redundant checks.src/hooks/useResizablePanels.ts (1)
99-103: Pass-through setter wrapper can be simplified.Line 99–Line 101 only proxies
setWidths; either returnsetWidthsdirectly or add clamping/normalization here to enforce panel invariants.♻️ Minimal simplification
- const setWidthsOverride = useCallback((newWidths: [number, number, number]) => { - setWidths(newWidths); - }, []); - - return { widths, setWidths: setWidthsOverride, handleMouseDown, containerRef }; + return { widths, setWidths, handleMouseDown, containerRef };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useResizablePanels.ts` around lines 99 - 103, The small wrapper setWidthsOverride in useResizablePanels simply proxies setWidths; either remove it and return setWidths directly in the return object, or implement normalization/clamping inside setWidthsOverride (e.g., clamp each value and enforce any invariants like min/max or total sum) before calling setWidths; update the returned object to use the chosen function name (keep containerRef, handleMouseDown and widths as-is) and ensure setWidthsOverride references the same tuple type [number, number, number].src/components/FeedPanel.tsx (1)
334-342: Consider whether a continuously-animating gradient on the panel title is desired.
GradientTextruns arequestAnimationFrameloop for its gradient animation. Since this title is always mounted while the panel is open, it will consume rAF cycles continuously. If the animation is meant to be subtle/decorative, consider addingpauseOnHover={false}or limiting the animation duration to avoid unnecessary battery/CPU usage on long sessions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/FeedPanel.tsx` around lines 334 - 342, The GradientText component used for the panel title continuously runs rAF; to avoid unnecessary CPU/battery use, update the FeedPanel usage of GradientText (in the JSX block rendering {title}) to either disable continuous animation by passing pauseOnHover={false} or reduce the animation runtime by increasing animationSpeed (or both) so the gradient is less taxing; modify the props on the GradientText instance (the component named GradientText in this file) accordingly to limit or pause the animation for long-lived mounts.src/components/SuperDraw.tsx (1)
175-177: Assigning toR.currentduring render is flagged by React 19's ref rules.This is a known pattern for syncing state to an imperative ref, but React's strict mode and the
react-hooks/refsrule now explicitly flag it. The recommended approach is to use a layout effect:♻️ Proposed fix
const R = useRef({ els, cam, sel }); - R.current = { els, cam, sel }; + // Sync after render, before paint + useEffect(() => { + R.current = { els, cam, sel }; + });Since
paintalready runs after state changes via the repaintuseEffect, using a no-depsuseEffecthere keeps the ref in sync without violating the render-purity rule.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SuperDraw.tsx` around lines 175 - 177, The ref R (created via useRef({ els, cam, sel }) and currently reassigned with R.current = { els, cam, sel } during render) must not be mutated during render; move the R.current update into a no-deps effect so the ref mirrors state after render. Replace the direct assignment with a useEffect (or useLayoutEffect if synchronous update before paint is required) that sets R.current = { els, cam, sel }, ensuring existing consumers like paint and the repaint useEffect read from R.current as before.src/services/pandocService.ts (1)
61-67: Performance concern: O(n²) string concatenation for large files.For large DOCX/PDF files (potentially tens of MB), building
binaryvia+=in a loop creates a new string on every iteration. Consider using a chunk-based approach or the built-inUint8Array→base64 path available in modern runtimes.♻️ Suggested improvement
function uint8ToBase64(bytes: Uint8Array): string { - let binary = ''; - for (let i = 0; i < bytes.length; i++) { - binary += String.fromCharCode(bytes[i]); - } - return btoa(binary); + const CHUNK = 0x8000; + const parts: string[] = []; + for (let i = 0; i < bytes.length; i += CHUNK) { + parts.push(String.fromCharCode(...bytes.subarray(i, i + CHUNK))); + } + return btoa(parts.join('')); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/pandocService.ts` around lines 61 - 67, The uint8ToBase64 function builds a binary string via repeated += causing O(n²) work for large files; replace the loop with a direct binary->base64 conversion appropriate to the runtime (e.g., use Buffer.from(bytes).toString('base64') in Node or a chunked approach that uses String.fromCharCode on slices to build a single binary string before calling btoa in browsers). Update the uint8ToBase64 implementation to use the Buffer-based conversion or a chunked String.fromCharCode approach to avoid per-byte concatenation and improve performance.src/main.tsx (1)
15-20: Duplicated color-mapping logic betweenmain.tsxandSettingsModal.tsx.Lines 15–20 here and lines 78–83 in
SettingsModal.tsxcontain identical AMOLED/dark/light color mapping. Consider extracting a shared helper (e.g.,getEffectColors()) to avoid divergence if one is updated without the other.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.tsx` around lines 15 - 20, Extract the duplicated AMOLED/dark/light color mapping into a single exported helper named getEffectColors() that returns an object { r, g, b } by checking document.documentElement.classList.contains('amoled') and 'dark' (use the same logic currently in main.tsx/SettingsModal.tsx); then replace the inline logic in both main.tsx and SettingsModal.tsx with calls to getEffectColors() and use the returned r,g,b values, keeping the helper in a shared module (e.g., utils or helpers) so both files import it and avoid future divergence.src/components/ShortcutsOverlay.tsx (1)
34-56: Consider adding basic accessibility attributes to the overlay.The modal-like overlay lacks
role,aria-modal, andaria-labelattributes. This would improve screen reader support.♻️ Suggested improvement
- <div className="shortcuts-backdrop" onClick={onClose}> - <div className="shortcuts-overlay" onClick={e => e.stopPropagation()}> + <div className="shortcuts-backdrop" onClick={onClose} role="presentation"> + <div className="shortcuts-overlay" role="dialog" aria-modal="true" aria-label="Raccourcis clavier" onClick={e => e.stopPropagation()}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ShortcutsOverlay.tsx` around lines 34 - 56, Add basic ARIA attributes to the overlay rendered by the ShortcutsOverlay component: set role="dialog" and aria-modal="true" on the element with class "shortcuts-overlay" (or the main container acting as the modal) and provide an accessible name via aria-label or aria-labelledby (pointing to the h2 with class "shortcuts-title"); ensure the backdrop element with class "shortcuts-backdrop" still handles onClick={onClose} and the close button remains focusable so screen readers can discover and dismiss the dialog.index.html (1)
14-16: AMOLED theme does not add thedarkclass — fragile for downstream consumers.When
theme === 'amoled', only theamoledclass is added. Any CSS rule or JS check that targets.darkalone will miss AMOLED mode. The JS code inmain.tsxandSettingsModal.tsxalready works around this withisAmoled || isDarkchecks, but CSS selectors and any future code would need to duplicate selectors (.dark, .amoled).Consider also adding
darkwhenamoledis set, so AMOLED is treated as a dark-mode superset:♻️ Suggested change
if (t === 'amoled') { + document.documentElement.classList.add('dark'); document.documentElement.classList.add('amoled'); } else if (t === 'dark' || (!t && window.matchMedia('(prefers-color-scheme: dark)').matches)) {This way,
.darkCSS rules apply universally and.amoledcan override with pure-black backgrounds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.html` around lines 14 - 16, When theme is set to 'amoled' ensure the document root gets both classes so dark-mode rules still apply: in the block that currently handles t === 'amoled' (where document.documentElement.classList.add('amoled') is called) also add the 'dark' class (i.e., add both 'amoled' and 'dark'); likewise ensure the theme-switching logic that removes or toggles classes keeps 'dark' and 'amoled' consistent (remove both when switching away from amoled and only remove 'amoled' when switching to dark) so downstream CSS/JS that targets '.dark' continues to work.src/components/PalettePicker.tsx (2)
38-40: HoistisDarkcomputation out of the.map()loop.
isDarkreads fromdocument.documentElement.classListwhich doesn't change between iterations. Computing it once before the map avoids redundant DOM reads on every palette item.Same applies to the
PalettePickerInlinecomponent at line 74.Proposed fix (PalettePicker)
+ const isDark = document.documentElement.classList.contains('dark') || document.documentElement.classList.contains('amoled'); {palettes.map(p => { - const isDark = document.documentElement.classList.contains('dark') || document.documentElement.classList.contains('amoled'); const colors = isDark ? p.dark : p.light;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/PalettePicker.tsx` around lines 38 - 40, Hoist the DOM read for theme detection so you compute isDark once per render instead of inside each iteration: move const isDark = document.documentElement.classList.contains('dark') || document.documentElement.classList.contains('amoled') out of the palettes.map callback in the PalettePicker component and do the same in the PalettePickerInline component (the isDark computation referenced around line 74) so both components read document.documentElement.classList only once and then use that const inside their map callbacks to select p.dark or p.light.
62-95: Palette item rendering is duplicated betweenPalettePickerandPalettePickerInline.The two components share nearly identical rendering logic for palette items. Consider extracting a shared
PaletteItemor aPaletteListsub-component to reduce duplication, if this surface area is expected to evolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/PalettePicker.tsx` around lines 62 - 95, PalettePicker and PalettePickerInline duplicate the same palette item rendering; refactor by extracting a shared sub-component (e.g., PaletteItem or PaletteList) that renders a single palette button and accepts props like palette (Palette), isActive (boolean), onSelect ((palette: Palette) => void) and current theme detection logic; replace the inline map in both PalettePicker and PalettePickerInline to call the new component and forward applyPalette/handleSelect logic (keep existing functions like PalettePickerInline, handleSelect, applyPalette, palettes, currentId) so behavior and state updates remain unchanged.src/components/BookmarkReader.tsx (1)
68-79: Retry handler lacks cancellation — stale state updates possible.Unlike the main
useEffectfetch (which sets acancelledflag on cleanup),handleRetryfires an uncancellable promise. If the bookmark changes or the component unmounts mid-retry,setArticle/setStatuswill update stale state. React 19 won't warn, but the user could briefly see the wrong article's content flash.Consider reusing the effect-based fetch by resetting
prevUrlRef.currentand forcing a re-trigger via a counter state, or adding a similar cancellation ref.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/BookmarkReader.tsx` around lines 68 - 79, The retry handler handleRetry currently starts an uncancellable extractArticle(bookmark.url) promise which can call setArticle/setStatus after the bookmark changed or component unmounted; change it to either (A) reuse the existing effect-based fetch by incrementing a retry counter state (e.g. retryCount) and resetting prevUrlRef.current so the useEffect that contains the cancelled flag will run and handle cleanup, or (B) add a cancellation ref (like cancelledRef) and check it in the .then/.catch callbacks (and set cancelledRef on unmount) so setArticle/setStatus are skipped when cancelled; update references to prevUrlRef, handleRetry, extractArticle, setArticle and setStatus accordingly.src/services/syncService.ts (1)
352-352: Long compound filter condition hurts readability.This single line packs 4 conditions. Consider extracting to a named predicate for clarity.
Proposed refactor
- const itemsToPush = mergedItems.filter(i => !remoteItemIdSet.has(i.id) && !(i.url && remoteItemUrlSet.has(i.url)) && !deletedItemIds.has(i.id) && !deletedFeedIds.has(i.feedId)); + const itemsToPush = mergedItems.filter(i => { + if (remoteItemIdSet.has(i.id)) return false; // already on remote + if (i.url && remoteItemUrlSet.has(i.url)) return false; // URL match on remote + if (deletedItemIds.has(i.id)) return false; // deleted remotely + if (deletedFeedIds.has(i.feedId)) return false; // parent feed deleted remotely + return true; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/syncService.ts` at line 352, The filter on mergedItems assigned to itemsToPush is a long compound condition that hurts readability; extract the predicate into a well-named function (e.g., isPushableItem) and use it in mergedItems.filter to make intent clear. Specifically, implement a function that accepts an item and returns the boolean composed of the four checks against remoteItemIdSet, remoteItemUrlSet, deletedItemIds, and deletedFeedIds, then replace the inline arrow predicate in the itemsToPush assignment with a call to that function (referencing itemsToPush, mergedItems, and the sets remoteItemIdSet/remoteItemUrlSet/deletedItemIds/deletedFeedIds).src/components/CommandPalette.tsx (1)
92-143: MutableflatIdxcounter in render body is functional but fragile.Using a
let flatIdx = 0that gets incremented inside nested.map()callbacks relies on synchronous render execution. It works correctly in React's current model, but if the rendering logic were ever refactored (e.g., extracted into sub-components), the counter would break.An alternative is to pre-compute a
commandToFlatIndexmap fromflatList:const cmdIndexMap = useMemo(() => new Map(flatList.map((c, i) => [c.id, i])), [flatList]);This is a minor nit — the current approach works fine.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/CommandPalette.tsx` around lines 92 - 143, The render uses a mutable let flatIdx incremented inside nested .map callbacks (flatIdx) which is fragile; replace this by precomputing a stable mapping from command id to flat index (e.g., compute cmdIndexMap from flatList with useMemo) and then look up idx = cmdIndexMap.get(cmd.id) when rendering the grouped entries, leaving selectedIndex, setSelectedIndex, grouped, flatList, and cmd.id references intact; ensure the map is memoized on flatList so indices stay consistent across renders and remove the mutable flatIdx variable.src/components/NotePanel.tsx (1)
124-130: Consider truncating content before Markdown parsing in card view.Each card renders the full
note.contentthroughreact-markdown. For a grid with many notes or notes with large content, this could cause noticeable rendering lag since every card incurs a full Markdown parse.Consider passing a truncated preview (e.g., first 200 chars) to
<Markdown>in card view, reserving full rendering for the editor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/NotePanel.tsx` around lines 124 - 130, NotePanel currently sends the full note.content into the <Markdown> renderer for each card, causing heavy parsing; change the card rendering to pass a truncated preview instead (e.g., first 200 chars with ellipsis) so Markdown receives preview text in the card view and full content is only rendered in the editor; implement this by adding a small helper (e.g., truncatePreview) or inline logic and use it where note.content is passed to <Markdown> in the NotePanel card rendering block to replace note.content with the preview variable.src/services/editorDocService.ts (2)
3-10:EditorDocRowomitsuser_id— intentional?The Supabase query uses
.select('*'), which returnsuser_id, but theEditorDocRowinterface doesn't include it. This is fine if you deliberately want to keep user IDs out of the UI layer, but if any caller ever needs to inspect the owner, the type won't reflect the actual shape. A quickuser_id?: stringor a comment explaining the omission would clarify intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/editorDocService.ts` around lines 3 - 10, EditorDocRow currently omits the user_id field returned by Supabase SELECT '*'; update the EditorDocRow interface to include user_id?: string (or user_id: string if required) or add a comment on the EditorDocRow declaration explaining the omission so the type matches the actual payload from the DB and callers can access the owner when needed; locate the EditorDocRow interface in src/services/editorDocService.ts and adjust the type or add the explanatory comment accordingly.
93-111:updateEditorDocMetawill send an empty payload if called with{}.If
metais{}(neithertitlenorfolderprovided), Supabase receives an empty update which may no-op or error depending on the version. A guard would make this defensive.🛡️ Proposed guard
export async function updateEditorDocMeta( userId: string, docId: string, meta: { title?: string; folder?: string | null } ): Promise<boolean> { if (!isSupabaseConfigured) return false; + if (!meta.title && meta.folder === undefined) return true; // nothing to update const { error } = await supabase🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/editorDocService.ts` around lines 93 - 111, The function updateEditorDocMeta currently calls supabase.update even when meta is an empty object; add a guard at the top of updateEditorDocMeta to detect an empty payload (e.g., meta is falsy or Object.keys(meta).length === 0) and short-circuit (return true or another no-op indicator) so you never call supabase.from('editor_documents').update(meta) with an empty object; update the function body around the meta parameter check to skip the database call when there's nothing to update.src/components/EditorFileList.tsx (2)
155-158: Sorting is recomputed on every render — consideruseMemo.
sortedandrootDocsare derived on each render. For a large doc list this is unnecessary work; wrapping inuseMemokeyed ondocsavoids repeated allocations.♻️ Proposed change
- const sorted = [...docs].sort((a, b) => - new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime() - ); - const rootDocs = sorted.filter(d => !d.folder); + const sorted = useMemo( + () => [...docs].sort((a, b) => new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime()), + [docs] + ); + const rootDocs = useMemo(() => sorted.filter(d => !d.folder), [sorted]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/EditorFileList.tsx` around lines 155 - 158, The sorting and filtering of docs are recomputed on every render (variables sorted and rootDocs in EditorFileList), causing unnecessary allocations; wrap the creation of sorted and rootDocs in a useMemo keyed on docs so the sort and filter run only when docs changes (reference the sorted and rootDocs variables inside the useMemo callback and return both values for use in the component). Ensure you import useMemo from React and replace direct declarations of sorted and rootDocs with the memoized result.
16-38: Exporting utility functions alongside the component breaks React Fast Refresh.
loadEditorDocs,saveEditorDocs,loadEditorFolders, andsaveEditorFoldersare non-component exports that cause the Fast Refresh plugin to skip HMR for this entire module during development. Consider moving these four helpers to a dedicated file (e.g.,editorDocStorage.ts).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/EditorFileList.tsx` around lines 16 - 38, Exported non-component helpers loadEditorDocs, saveEditorDocs, loadEditorFolders, and saveEditorFolders in the EditorFileList module prevent React Fast Refresh from working; move these four functions into a separate module (e.g., a new editor storage module), export them from there, and update EditorFileList to import and use those functions instead of exporting them itself; ensure function signatures and STORAGE_KEY/FOLDERS_KEY references remain unchanged and update any tests/imports accordingly.src/components/BookmarkPanel.tsx (1)
46-58: Optimistic UI updates are not rolled back on service failure.
handleRemoveremoves the bookmark from state beforeremoveBookmarkresolves. If the network call fails, the user loses the item from the UI without it being deleted server-side (same forhandleToggleRead). Consider reverting the optimistic state on failure or at least notifying the user.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/BookmarkPanel.tsx` around lines 46 - 58, The optimistic updates in handleRemove and handleToggleRead update state before the async service calls and do not revert on failure; capture the previous bookmarks state (e.g., const prev = bookmarks or from setState functional previous), perform the optimistic setBookmarks change, then await removeBookmark(user.id, id) or toggleBookmarkRead(user.id, id, isRead) inside a try/catch, and on catch restore the previous bookmarks with setBookmarks(prev) and surface an error (e.g., call an existing notify/showError function or set an error state) so the UI is rolled back when the network call fails; modify handleRemove and handleToggleRead to follow this pattern and reference setBookmarks, removeBookmark, toggleBookmarkRead, and user.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
AGENTS.mdREADME.mdindex.htmlpackage.jsonsrc-tauri/src/lib.rssrc/App.tsxsrc/components/BookmarkPanel.tsxsrc/components/BookmarkReader.tsxsrc/components/CommandPalette.tsxsrc/components/EditorFileList.tsxsrc/components/FeedPanel.tsxsrc/components/GradientText.tsxsrc/components/NotePanel.tsxsrc/components/NoteStickyBoard.tsxsrc/components/PalettePicker.tsxsrc/components/SettingsModal.tsxsrc/components/ShinyText.tsxsrc/components/ShortcutsOverlay.tsxsrc/components/SourcePanel.tsxsrc/components/SpotlightCard.tsxsrc/components/SuperDraw.tsxsrc/components/SuperEditor.tsxsrc/components/ui/animated-theme-toggler.tsxsrc/contexts/AuthContext.tsxsrc/hooks/useCommands.tssrc/hooks/useFeedStore.tssrc/hooks/useResizablePanels.tssrc/index.csssrc/main.tsxsrc/services/bookmarkService.tssrc/services/editorDocService.tssrc/services/noteService.tssrc/services/pandocService.tssrc/services/rssService.tssrc/services/syncService.tssrc/themes/palettes.tssupabase/migrations/20250225000005_bookmarks.sqlsupabase/migrations/20250225000006_editor_documents.sqlsupabase/migrations/20250225000007_notes.sql
| fn pandoc_import(base64_data: String, filename: String) -> Result<String, String> { | ||
| let bytes = STANDARD.decode(&base64_data) | ||
| .map_err(|e| format!("base64 decode error: {e}"))?; | ||
|
|
||
| let tmp_dir = std::env::temp_dir().join("superflux_pandoc"); | ||
| std::fs::create_dir_all(&tmp_dir) | ||
| .map_err(|e| format!("Failed to create temp dir: {e}"))?; | ||
|
|
||
| let input_path = tmp_dir.join(&filename); | ||
| std::fs::write(&input_path, &bytes) | ||
| .map_err(|e| format!("Failed to write temp file: {e}"))?; |
There was a problem hiding this comment.
Sanitize filename before constructing temp paths (path traversal risk).
Line 654 joins untrusted filename directly. Absolute paths or .. segments can escape the intended temp directory and overwrite arbitrary files.
🔐 Proposed fix
+ let safe_name = std::path::Path::new(&filename)
+ .file_name()
+ .and_then(|n| n.to_str())
+ .ok_or("Invalid filename")?;
- let input_path = tmp_dir.join(&filename);
+ let input_path = tmp_dir.join(safe_name);📝 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.
| fn pandoc_import(base64_data: String, filename: String) -> Result<String, String> { | |
| let bytes = STANDARD.decode(&base64_data) | |
| .map_err(|e| format!("base64 decode error: {e}"))?; | |
| let tmp_dir = std::env::temp_dir().join("superflux_pandoc"); | |
| std::fs::create_dir_all(&tmp_dir) | |
| .map_err(|e| format!("Failed to create temp dir: {e}"))?; | |
| let input_path = tmp_dir.join(&filename); | |
| std::fs::write(&input_path, &bytes) | |
| .map_err(|e| format!("Failed to write temp file: {e}"))?; | |
| fn pandoc_import(base64_data: String, filename: String) -> Result<String, String> { | |
| let bytes = STANDARD.decode(&base64_data) | |
| .map_err(|e| format!("base64 decode error: {e}"))?; | |
| let tmp_dir = std::env::temp_dir().join("superflux_pandoc"); | |
| std::fs::create_dir_all(&tmp_dir) | |
| .map_err(|e| format!("Failed to create temp dir: {e}"))?; | |
| let safe_name = std::path::Path::new(&filename) | |
| .file_name() | |
| .and_then(|n| n.to_str()) | |
| .ok_or("Invalid filename")?; | |
| let input_path = tmp_dir.join(safe_name); | |
| std::fs::write(&input_path, &bytes) | |
| .map_err(|e| format!("Failed to write temp file: {e}"))?; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/lib.rs` around lines 646 - 656, The pandoc_import function
currently joins untrusted filename directly into tmp_dir (input_path =
tmp_dir.join(&filename)), allowing absolute paths or ".." segments to escape the
temp directory; fix this by validating and sanitizing filename before joining:
reject or strip absolute path prefixes, ensure you only use the path's basename
(e.g., Path::new(&filename).file_name()), forbid or normalize "."/".." segments,
fall back to a safe generated filename (UUID or timestamp) if the basename is
missing or empty, and then construct input_path = tmp_dir.join(sanitized_name);
update error handling to report invalid filenames from pandoc_import.
| let tmp_dir = std::env::temp_dir().join("superflux_pandoc"); | ||
| std::fs::create_dir_all(&tmp_dir) | ||
| .map_err(|e| format!("Failed to create temp dir: {e}"))?; | ||
|
|
||
| let input_path = tmp_dir.join("export_input.html"); | ||
| let ext = match format.as_str() { | ||
| "docx" => "docx", | ||
| "pdf" => "pdf", | ||
| other => return Err(format!("Unsupported format: {other}")), | ||
| }; | ||
| let output_path = tmp_dir.join(format!("export_output.{ext}")); | ||
|
|
There was a problem hiding this comment.
Use per-request temp files to prevent collisions and data leaks.
Line 682 and Line 688 use deterministic filenames in a shared temp directory. Concurrent exports can clobber each other and return incorrect/cross-user output.
🧪 Proposed fix (unique request directory)
+ let request_id = std::time::SystemTime::now()
+ .duration_since(std::time::UNIX_EPOCH)
+ .map_err(|e| format!("clock error: {e}"))?
+ .as_nanos();
+ let request_dir = tmp_dir.join(format!("job-{request_id}"));
+ std::fs::create_dir_all(&request_dir)
+ .map_err(|e| format!("Failed to create request temp dir: {e}"))?;
-
- let input_path = tmp_dir.join("export_input.html");
+ let input_path = request_dir.join("export_input.html");
@@
- let output_path = tmp_dir.join(format!("export_output.{ext}"));
+ let output_path = request_dir.join(format!("export_output.{ext}"));
@@
- let _ = std::fs::remove_file(&output_path);
+ let _ = std::fs::remove_file(&output_path);
+ let _ = std::fs::remove_dir_all(&request_dir);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/lib.rs` around lines 678 - 689, The current code creates
deterministic temp files (tmp_dir, input_path, output_path) which can collide
across concurrent requests; change to create a unique per-request temp directory
or file names (e.g., by using a UUID or tempfile::Builder) and use that
directory for input_path and output_path, ensuring you still call
std::fs::create_dir_all (or let tempfile create the dir) and clean up after use;
update references to tmp_dir, input_path, and output_path in the export function
to use the newly generated unique path.
| const contentSaveTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null); | ||
| const noteSaveTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null); |
There was a problem hiding this comment.
Single debounce timer per entity type can drop remote writes across different notes/docs.
If the user edits doc A then doc B quickly, the timer for A is canceled by B. Same issue for notes. This risks unsynced backend state.
Proposed fix
- const contentSaveTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
- const noteSaveTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+ const contentSaveTimersRef = useRef(new Map<string, ReturnType<typeof setTimeout>>());
+ const noteSaveTimersRef = useRef(new Map<string, ReturnType<typeof setTimeout>>());
@@
- if (contentSaveTimerRef.current) clearTimeout(contentSaveTimerRef.current);
- contentSaveTimerRef.current = setTimeout(() => {
+ const prevTimer = contentSaveTimersRef.current.get(id);
+ if (prevTimer) clearTimeout(prevTimer);
+ const timer = setTimeout(() => {
updateEditorDocContent(user.id, id, content);
+ contentSaveTimersRef.current.delete(id);
}, 1000);
+ contentSaveTimersRef.current.set(id, timer);
@@
- if (noteSaveTimerRef.current) clearTimeout(noteSaveTimerRef.current);
- noteSaveTimerRef.current = setTimeout(() => {
+ const prevTimer = noteSaveTimersRef.current.get(noteId);
+ if (prevTimer) clearTimeout(prevTimer);
+ const timer = setTimeout(() => {
updateNoteContent(user.id, noteId, updates.content!);
+ noteSaveTimersRef.current.delete(noteId);
}, 1000);
+ noteSaveTimersRef.current.set(noteId, timer);
}
@@
+ useEffect(() => {
+ return () => {
+ contentSaveTimersRef.current.forEach(clearTimeout);
+ noteSaveTimersRef.current.forEach(clearTimeout);
+ };
+ }, []);Also applies to: 227-231, 324-327
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.tsx` around lines 157 - 158, The current single refs
contentSaveTimerRef and noteSaveTimerRef cause timers to be overwritten when
switching documents/notes, dropping pending saves; change both to maps keyed by
entity id (e.g., contentSaveTimerRef: useRef<Record<string, ReturnType<typeof
setTimeout> | null>> and noteSaveTimerRef similarly) and update the debounce
logic (where save timers are set/cleared) to clear/set timers using the current
doc/note id instead of a single shared ref; update all usages (including the
other occurrences around the indicated sections) to use the id-keyed timer
lookup so each document/note has its own debounce timer and remote writes are
not lost.
| const handleRenameNoteFolder = useCallback((oldName: string, newName: string) => { | ||
| setNoteFolders(prev => prev.map(f => f === oldName ? newName : f)); | ||
| setNotes(prev => prev.map(n => n.folder === oldName ? { ...n, folder: newName } : n)); | ||
| setNotes(prev => { | ||
| const updated = prev.map(n => n.folder === oldName ? { ...n, folder: newName } : n); | ||
| // Sync folder rename to Supabase for affected notes | ||
| if (user) { | ||
| updated.filter(n => n.folder === newName).forEach(n => { | ||
| updateNoteMeta(user.id, n.id, { folder: newName }); | ||
| }); | ||
| } | ||
| return updated; | ||
| }); | ||
| if (selectedNoteFolder === oldName) setSelectedNoteFolder(newName); | ||
| }, [selectedNoteFolder]); | ||
| }, [selectedNoteFolder, user]); | ||
|
|
||
| const handleDeleteNoteFolder = useCallback((name: string) => { | ||
| setNoteFolders(prev => prev.filter(f => f !== name)); | ||
| setNotes(prev => prev.map(n => n.folder === name ? { ...n, folder: undefined } : n)); | ||
| setNotes(prev => { | ||
| const updated = prev.map(n => n.folder === name ? { ...n, folder: undefined } : n); | ||
| // Sync folder removal to Supabase for affected notes | ||
| if (user) { | ||
| updated.filter(n => !n.folder).forEach(n => { | ||
| updateNoteMeta(user.id, n.id, { folder: null }); | ||
| }); | ||
| } | ||
| return updated; | ||
| }); | ||
| if (selectedNoteFolder === name) setSelectedNoteFolder(null); | ||
| }, [selectedNoteFolder]); | ||
| }, [selectedNoteFolder, user]); |
There was a problem hiding this comment.
Folder rename/delete sync updates too many notes in Supabase.
Current logic updates every note matching the post-update condition (e.g., all root notes on delete), not only notes affected by this folder operation.
Proposed fix
const handleRenameNoteFolder = useCallback((oldName: string, newName: string) => {
+ const affectedIds = notes.filter(n => n.folder === oldName).map(n => n.id);
setNoteFolders(prev => prev.map(f => f === oldName ? newName : f));
- setNotes(prev => {
- const updated = prev.map(n => n.folder === oldName ? { ...n, folder: newName } : n);
- // Sync folder rename to Supabase for affected notes
- if (user) {
- updated.filter(n => n.folder === newName).forEach(n => {
- updateNoteMeta(user.id, n.id, { folder: newName });
- });
- }
- return updated;
- });
+ setNotes(prev => prev.map(n => n.folder === oldName ? { ...n, folder: newName } : n));
+ if (user) affectedIds.forEach(id => updateNoteMeta(user.id, id, { folder: newName }));
if (selectedNoteFolder === oldName) setSelectedNoteFolder(newName);
- }, [selectedNoteFolder, user]);
+ }, [selectedNoteFolder, user, notes]);
const handleDeleteNoteFolder = useCallback((name: string) => {
+ const affectedIds = notes.filter(n => n.folder === name).map(n => n.id);
setNoteFolders(prev => prev.filter(f => f !== name));
- setNotes(prev => {
- const updated = prev.map(n => n.folder === name ? { ...n, folder: undefined } : n);
- // Sync folder removal to Supabase for affected notes
- if (user) {
- updated.filter(n => !n.folder).forEach(n => {
- updateNoteMeta(user.id, n.id, { folder: null });
- });
- }
- return updated;
- });
+ setNotes(prev => prev.map(n => n.folder === name ? { ...n, folder: undefined } : n));
+ if (user) affectedIds.forEach(id => updateNoteMeta(user.id, id, { folder: null }));
if (selectedNoteFolder === name) setSelectedNoteFolder(null);
- }, [selectedNoteFolder, user]);
+ }, [selectedNoteFolder, user, notes]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.tsx` around lines 350 - 378, In handleRenameNoteFolder and
handleDeleteNoteFolder the Supabase sync filters use the post-update condition
(checking n.folder === newName or !n.folder) which matches too many notes;
instead capture the affected notes from the previous state before mapping and
only call updateNoteMeta for those (e.g., inside setNotes(prev => { const
affected = prev.filter(n => n.folder === oldName) ...; /* map to updated */; if
(user) affected.forEach(n => updateNoteMeta(user.id, n.id, { folder: newName }))
})). Do the analogous change in handleDeleteNoteFolder (capture prev.filter(n =>
n.folder === name) and update those IDs with folder: null). This ensures
updateNoteMeta is only called for notes that actually had the target folder.
| <div className="bk-compact-item__text"> | ||
| <h3 className="bk-compact-item__title">{bk.title}</h3> | ||
| <div className="bk-compact-item__meta"> | ||
| <span className="bk-compact-item__site">{bk.site_name || new URL(bk.url).hostname}</span> |
There was a problem hiding this comment.
new URL(bk.url) can throw on malformed URLs, crashing the render.
If any bookmark has an invalid url value, new URL(bk.url).hostname will throw a TypeError, which will propagate through React's render and crash the component. The same pattern also appears at line 218.
🛡️ Proposed fix — extract a safe helper
+function safeHostname(url: string): string {
+ try { return new URL(url).hostname; }
+ catch { return url; }
+}
+
function formatDate(dateStr: string): string {Then replace both occurrences:
-<span className="bk-compact-item__site">{bk.site_name || new URL(bk.url).hostname}</span>
+<span className="bk-compact-item__site">{bk.site_name || safeHostname(bk.url)}</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/BookmarkPanel.tsx` at line 159, The render uses new
URL(bk.url).hostname directly (in the BookmarkPanel component inside the span
with class bk-compact-item__site and the other occurrence later) which can throw
for malformed URLs; add a small safe helper function (e.g., getHostnameSafe or
parseHostname) that accepts a string, returns the hostname on success or a
sensible fallback (empty string or the original input) on failure, implemented
with a try/catch and guards for null/undefined, then replace both occurrences of
new URL(bk.url).hostname with a call to that helper and keep the existing
bk.site_name || ... fallback logic intact.
| if (event.key === 'ArrowUp') { | ||
| setSelectedIndex(i => (i + props.items.length - 1) % props.items.length); | ||
| return true; | ||
| } | ||
| if (event.key === 'ArrowDown') { | ||
| setSelectedIndex(i => (i + 1) % props.items.length); | ||
| return true; | ||
| } | ||
| if (event.key === 'Enter') { | ||
| const item = props.items[selectedIndex]; | ||
| if (item) props.command(item); |
There was a problem hiding this comment.
Emoji keyboard navigation is inconsistent with rendered list (and breaks on empty list).
You navigate over props.items.length but render only props.items.slice(0, 12). This lets selection move to hidden entries; with zero items it also risks invalid modulo math.
Proposed fix
const EmojiList = forwardRef<
{ onKeyDown: (props: { event: KeyboardEvent }) => boolean },
{ items: EmojiItem[]; command: (item: EmojiItem) => void }
>((props, ref) => {
const [selectedIndex, setSelectedIndex] = useState(0);
+ const visibleItems = props.items.slice(0, 12);
+ const visibleCount = visibleItems.length;
useEffect(() => { setSelectedIndex(0); }, [props.items]);
useImperativeHandle(ref, () => ({
onKeyDown: ({ event }: { event: KeyboardEvent }) => {
+ if (!visibleCount) return false;
if (event.key === 'ArrowUp') {
- setSelectedIndex(i => (i + props.items.length - 1) % props.items.length);
+ setSelectedIndex(i => (i + visibleCount - 1) % visibleCount);
return true;
}
if (event.key === 'ArrowDown') {
- setSelectedIndex(i => (i + 1) % props.items.length);
+ setSelectedIndex(i => (i + 1) % visibleCount);
return true;
}
if (event.key === 'Enter') {
- const item = props.items[selectedIndex];
+ const item = visibleItems[selectedIndex];
if (item) props.command(item);
return true;
}
return false;
},
- }));
+ }), [visibleCount, visibleItems, selectedIndex, props.command]);
- if (!props.items.length) {
+ if (!visibleCount) {
return (
<div className="super-editor-emoji-popup">
<span className="super-editor-emoji-empty">Aucun emoji</span>
</div>
);
}
return (
<div className="super-editor-emoji-popup">
- {props.items.slice(0, 12).map((item, index) => (
+ {visibleItems.map((item, index) => (Also applies to: 82-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/SuperEditor.tsx` around lines 55 - 65, The keyboard navigation
uses props.items.length while the rendered list is props.items.slice(0, 12),
causing selection of non-visible items and breaking on empty lists; update the
handlers in SuperEditor (where setSelectedIndex and the Enter logic are used) to
compute and use a visibleItems constant (e.g., const visibleItems =
props.items.slice(0, 12)) and use visibleItems.length for the modulo arithmetic
and bounds checks, guard against visibleItems.length === 0 before doing modulo
or indexing, and apply the same change to the other handlers referenced around
the 82-83 region so all keyboard navigation and Enter selection operate on the
rendered subset only.
| document.documentElement.classList.remove("dark", "amoled") | ||
| if (theme === "dark") { | ||
| document.documentElement.classList.add("dark") | ||
| } else if (theme === "amoled") { | ||
| document.documentElement.classList.add("amoled") | ||
| } |
There was a problem hiding this comment.
Clear legacy sepia class during theme apply.
Line 18 removes only dark/amoled. Upgraded users with a stale sepia class can end up with conflicting theme classes.
🧹 Proposed fix
- document.documentElement.classList.remove("dark", "amoled")
+ document.documentElement.classList.remove("dark", "amoled", "sepia")📝 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.
| document.documentElement.classList.remove("dark", "amoled") | |
| if (theme === "dark") { | |
| document.documentElement.classList.add("dark") | |
| } else if (theme === "amoled") { | |
| document.documentElement.classList.add("amoled") | |
| } | |
| document.documentElement.classList.remove("dark", "amoled", "sepia") | |
| if (theme === "dark") { | |
| document.documentElement.classList.add("dark") | |
| } else if (theme === "amoled") { | |
| document.documentElement.classList.add("amoled") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/animated-theme-toggler.tsx` around lines 18 - 23, The
theme-apply block in animated-theme-toggler.tsx currently removes only "dark"
and "amoled" but leaves a stale "sepia" class; update the class removal to also
remove "sepia" so theme switches don’t leave conflicting classes. Locate the
document.documentElement.classList.remove call in the theme application code
(the block that checks theme === "dark" / "amoled") and include "sepia" in the
list of classes to remove before adding the new theme class.
| export function generateBookmarkId(url: string): string { | ||
| // Simple hash for desktop (sync compatible with Chrome extension's bk- prefix) | ||
| let hash = 0; | ||
| for (let i = 0; i < url.length; i++) { | ||
| const chr = url.charCodeAt(i); | ||
| hash = ((hash << 5) - hash) + chr; | ||
| hash |= 0; | ||
| } | ||
| return `bk-${Math.abs(hash).toString(16).padStart(16, '0').slice(0, 16)}`; |
There was a problem hiding this comment.
generateBookmarkId has a 32-bit hash — collision risk with upsert semantics.
The hash is a 32-bit integer (via hash |= 0), producing at most ~4 billion unique IDs. Combined with the upsert in addBookmark (line 45-48, onConflict: 'id,user_id'), two different URLs that collide will silently overwrite each other. With a few thousand bookmarks per user, the birthday-problem probability becomes non-trivial.
Consider using crypto.randomUUID() or at minimum a longer hash (e.g., SHA-256 truncated) to reduce collision likelihood. If sync compatibility with the Chrome extension is the constraint, the extension's ID generation should also be upgraded.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/bookmarkService.ts` around lines 94 - 102, The current
generateBookmarkId function uses a 32-bit integer hash which risks collisions
that can cause silent overwrites when addBookmark uses upsert (onConflict:
'id,user_id'); replace generateBookmarkId with a collision‑resistant ID strategy
such as using crypto.randomUUID() for new IDs (or compute a SHA‑256 of the URL
and truncate to a safe length) so IDs are far less likely to collide, and update
any Chrome extension counterpart to use the same scheme for sync compatibility;
ensure addBookmark's upsert behavior remains unchanged but now operates on
globally-unique IDs.
| console.log('[notes] isSupabaseConfigured:', isSupabaseConfigured, 'userId:', userId); | ||
| if (!isSupabaseConfigured) { console.warn('[notes] Supabase not configured'); return []; } |
There was a problem hiding this comment.
Remove user-identifying debug logs from note service.
Line 20 logs userId directly. This is sensitive identifier data and should not be emitted in normal client logs.
Proposed fix
export async function fetchNotes(userId: string): Promise<NoteRow[]> {
- console.log('[notes] isSupabaseConfigured:', isSupabaseConfigured, 'userId:', userId);
if (!isSupabaseConfigured) { console.warn('[notes] Supabase not configured'); return []; }
@@
- console.log('[notes] fetch response — status:', status, 'data:', data?.length, 'error:', error);
-
if (error) {
console.error('[notes] fetch error:', error);
return [];
}Also applies to: 29-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/noteService.ts` around lines 20 - 21, The debug log in
noteService.ts prints a sensitive user identifier (userId) via console.log (and
similar logs around lines 29-33); remove the userId from those debug statements
in functions that interact with Supabase (e.g., where isSupabaseConfigured and
userId are logged), replacing them with non-identifying info such as a boolean
or redacted value (e.g., "user present" or "<redacted>") or remove the console
logs entirely and, if necessary, emit only context-level logs via the app logger
(e.g., processLogger) without any PII; ensure all occurrences in the get/list
note-related functions are updated.
| if not exists (select 1 from pg_policies where tablename = 'notes' and policyname = 'Users can update own notes') then | ||
| create policy "Users can update own notes" on notes for update using (auth.uid() = user_id); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all migration files and check for UPDATE policies
fd -e sql . supabase/migrations/ --exec basename {} \; | sortRepository: devohmycode/SuperFlux
Length of output: 278
🏁 Script executed:
#!/bin/bash
# Check the specific notes migration file for UPDATE policies
cat supabase/migrations/20250225000007_notes.sqlRepository: devohmycode/SuperFlux
Length of output: 2601
🏁 Script executed:
#!/bin/bash
# Search for all UPDATE policies across migration files to identify the pattern
rg "create policy.*update" -i supabase/migrations/ -A 1 -B 1Repository: devohmycode/SuperFlux
Length of output: 2515
UPDATE policies across multiple migrations missing WITH CHECK clause — user_id could be reassigned.
The UPDATE policies in this migration (and several others: bookmarks, editor_documents, profiles, user_settings, feeds, feed_items) only specify USING (auth.uid() = user_id), which restricts which rows can be targeted. Without a WITH CHECK clause, users can update the user_id column to a different value, effectively transferring ownership of records to other users. This violates RLS intent.
The INSERT policies correctly use WITH CHECK (auth.uid() = user_id) — UPDATE policies need the same.
🔒 Fix for notes migration
- create policy "Users can update own notes" on notes for update using (auth.uid() = user_id);
+ create policy "Users can update own notes" on notes for update using (auth.uid() = user_id) with check (auth.uid() = user_id);Apply the same pattern across all other affected migrations.
📝 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.
| if not exists (select 1 from pg_policies where tablename = 'notes' and policyname = 'Users can update own notes') then | |
| create policy "Users can update own notes" on notes for update using (auth.uid() = user_id); | |
| if not exists (select 1 from pg_policies where tablename = 'notes' and policyname = 'Users can update own notes') then | |
| create policy "Users can update own notes" on notes for update using (auth.uid() = user_id) with check (auth.uid() = user_id); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@supabase/migrations/20250225000007_notes.sql` around lines 38 - 39, The
UPDATE policy creation for "Users can update own notes" on table notes currently
only uses USING (auth.uid() = user_id) which allows changing user_id; modify the
CREATE POLICY statement to include WITH CHECK (auth.uid() = user_id) so updates
are only valid when the resulting row still belongs to the same auth.uid(), and
apply the same change to the analogous UPDATE policy lines for bookmarks,
editor_documents, profiles, user_settings, feeds, and feed_items migrations to
prevent ownership reassignment.
Summary
Ctrl+1–5shortcuts: SuperFlux (RSS), SuperBookmark, SuperNote, SuperEditor, SuperDrawCtrl+K) with search, keyboard shortcuts overlay (?)Ctrl+Kbadgereact-markdownChanged files (41 files, +22k / -6k lines)
src/App.tsx— mode switching, brand transitions, panel routingsrc/components/SuperDraw.tsx— new canvas drawing componentsrc/components/SuperEditor.tsx— new document editorsrc/components/BookmarkPanel.tsx/BookmarkReader.tsx— bookmark managementsrc/components/CommandPalette.tsx/ShortcutsOverlay.tsx— palette & helpsrc/components/SourcePanel.tsx— mode tab bar, brand switch propsrc/components/NoteStickyBoard.tsx/NotePanel.tsx— Markdown renderingsrc/services/— bookmarkService, noteService, editorDocService, pandocServicesrc/index.css— mode tabs, sticky markdown, SuperDraw stylessupabase/migrations/— 3 new migration filesTest plan
Ctrl+1–5Ctrl+K, test search and shortcuts🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation