Dev#91
Conversation
Feature/drag drop
Fix/system hardening
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
.github/workflows/release.yml (1)
101-102:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent
pnpm/action-setupversion across jobs.The
version_and_tagjob uses@v4(Line 30) while thebuildjob here uses@v3. Align them to@v4for consistent pnpm resolution behavior, and double-check that a version is resolvable (see thedevelopment.ymlnote about the droppedwith.version).🔧 Proposed fix
- name: Setup pnpm - uses: pnpm/action-setup@v3 + uses: pnpm/action-setup@v4🤖 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 @.github/workflows/release.yml around lines 101 - 102, The pnpm setup action in the build job is using pnpm/action-setup@v3 while the version_and_tag job uses `@v4`; update the build job's action reference from pnpm/action-setup@v3 to pnpm/action-setup@v4 (ensure any with.version inputs are compatible or removed per development.yml notes) so both jobs use the same pnpm/action-setup@v4 behavior; locate the build job's "Setup pnpm" step and make the change.apps/main-processor/src/index.ts (2)
87-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame unhandled-rejection risk in
second-instance.
sendFilePathToRenderer(filePath)at Line 92 is also unawaited and uncaught. Add a.catchto log failures consistently with theopen-filehandler.🛡️ Proposed fix
mainWindow.focus(); - sendFilePathToRenderer(filePath); + void sendFilePathToRenderer(filePath).catch((error) => { + console.error('Failed to open file:', error); + });🤖 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 `@apps/main-processor/src/index.ts` around lines 87 - 95, The call to sendFilePathToRenderer(filePath) in the second-instance handler is unawaited and unhandled; update the branch that runs when mainWindow exists (the block referencing mainWindow, mainWindow.restore(), mainWindow.focus()) to call sendFilePathToRenderer(filePath).catch(...) and log any errors with the same logger used in the open-file handler so failures are consistently reported; leave the pendingFilePath assignment in the else branch unchanged.
61-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle rejection from async
sendFilePathToRenderer.
sendFilePathToRendereris now async andresolveMarkdownFilePathcan reject (invalid path, disallowed extension, path escapes root). Here it's invoked fire-and-forget without a.catch, so a rejection becomes an unhandled promise rejection. Theopen-filehandler at Line 76 already guards this; apply the same here.🛡️ Proposed fix
mainWindow.webContents.once('did-finish-load', () => { const cliFilePath = parseFilePathFromArgv(process.argv); const filePathToOpen = pendingFilePath ?? cliFilePath; if (filePathToOpen) { - sendFilePathToRenderer(filePathToOpen); + void sendFilePathToRenderer(filePathToOpen).catch((error) => { + console.error('Failed to open file:', error); + }); } pendingFilePath = null; });🤖 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 `@apps/main-processor/src/index.ts` around lines 61 - 68, sendFilePathToRenderer is async and can reject (via resolveMarkdownFilePath); update the did-finish-load handler to handle rejections the same way the "open-file" handler does: make the callback async (or explicitly attach .catch) and wrap the sendFilePathToRenderer(filePathToOpen) call in try/catch (or .catch) to log the error and show the same invalid-file handling used by the open-file handler, then still set pendingFilePath = null. Target the did-finish-load callback and the sendFilePathToRenderer call for the change.apps/main-processor/src/ipc.ts (1)
73-79:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent resolver usage for UNWATCH_FILE
WATCH_FILEusesresolveWatchedMarkdownPath(restricts toallowedMarkdownFiles/allowedFolderRoots), butUNWATCH_FILEusesresolveMarkdownFilePathwithout the allowed-root checks, so UNWATCH_FILE accepts paths that WATCH_FILE would reject.- If the goal is to only unwatch files that were previously allowed/watched, align
UNWATCH_FILEto useresolveWatchedMarkdownPath(or enforce the same allowlist checks).🤖 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 `@apps/main-processor/src/ipc.ts` around lines 73 - 79, UNWATCH_FILE currently calls resolveMarkdownFilePath which bypasses the allowlist checks used by WATCH_FILE; update the handler for IPC_CONSTANTS.UNWATCH_FILE to use the same restricted resolver (resolveWatchedMarkdownPath) or otherwise perform the same allowed-root/allowed-file checks before calling unWatchFile so only files that could have been watched are accepted; keep validateSender and the async flow, just replace resolveMarkdownFilePath with resolveWatchedMarkdownPath (or add explicit allowlist validation) to ensure consistent behavior with WATCH_FILE.apps/renderer/src/context/ThemeProvider.tsx (1)
11-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInitial OS theme is not applied on first launch.
getSystemTheme()is only invoked inside thematchMediachangehandler, so when there is no savedapp-themethe app stays on the default'github-light'until the user actually toggles their OS theme. For the OS-theme-detection feature, the initial mount should fall back to the system theme when nothing is persisted.🐛 Proposed fix
useEffect(()=>{ const saved=localStorage.getItem('app-theme'); if(saved && APPTHEMES.includes(saved as Theme)){ setTheme(saved as Theme); + } else { + setThemeState(getSystemTheme()); } },[])🤖 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 `@apps/renderer/src/context/ThemeProvider.tsx` around lines 11 - 27, The initial mount effect that reads localStorage should fall back to the system theme when nothing is persisted: in the first useEffect (the one reading localStorage 'app-theme') call getSystemTheme() and setThemeState(getSystemTheme()) when saved is falsy instead of leaving the default; ensure you keep the existing APPTHEMES check and only use setTheme(saved as Theme) when a valid saved theme exists, otherwise call setThemeState(getSystemTheme()) so the app applies the OS theme immediately on first launch (update the effect surrounding localStorage.getItem, setTheme, and setThemeState accordingly).apps/renderer/src/components/FileTree.tsx (1)
9-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
role="group"is placed on the label row, not around the grouped items.The
role="group"andaria-labelsit on the folder headerdiv(chevron + name only). The child entries are rendered in the siblingdivat Lines 19-29, so the announced group has no member items. Move the grouping role to the container that actually wraps the children so assistive tech associates the label with the folder contents.♻️ Suggested placement
<div className="mb-1"> <div - role='group' - aria-label={`${node.name} folder`} style={{ paddingLeft }} className="flex items-center gap-2 py-2 text-[11px] font-semibold uppercase tracking-wider text-text-muted" > <Icons.ChevronRight size={12} /> <span className="truncate">{node.name}</span> </div> - <div className="space-y-0.5"> + <div role="group" aria-label={`${node.name} folder`} className="space-y-0.5"> {node.children?.map((child) => (🤖 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 `@apps/renderer/src/components/FileTree.tsx` around lines 9 - 17, The folder header currently applies role="group" and aria-label to the header div (the chevron + <span>{node.name}</span>) so the announced group has no member items; move the role="group" and aria-label={`${node.name} folder`} from that header div into the parent/container that actually wraps the child entries (the element rendered by the FileTree component that contains the children entries for node), ensuring the group role surrounds the child items so assistive tech associates the label with the folder contents; update the header div to be a plain button/label element and keep accessibility on the wrapping container (reference: node, FileTree component, header div with Icons.ChevronRight and span).apps/renderer/src/hooks/useShortcuts.ts (1)
28-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDuplicate
mod + Shift + Fcondition makes the focus-mode branch dead code.Both blocks test
mod && e.shiftKey && e.key.toLowerCase() === 'f'. The first (Lines 28–32) returns after callingonOpenFolderSearch, so the second block (Lines 34–38) callingonToggleFocusModeis unreachable. If folder search replaced focus mode on this shortcut, remove the stale block; otherwise pick a distinct key for one of them. NoteonToggleFocusModeis then unused here and can be dropped from the destructure and dependency array.🐛 Remove the unreachable block
if (mod && e.shiftKey && e.key.toLowerCase() === 'f') { e.preventDefault(); onOpenFolderSearch(); return; } - - if (mod && e.shiftKey && e.key.toLowerCase() === 'f') { - e.preventDefault(); - onToggleFocusMode(); - return; - }🤖 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 `@apps/renderer/src/hooks/useShortcuts.ts` around lines 28 - 38, The shortcut handler in useShortcuts uses the same condition (mod && e.shiftKey && e.key.toLowerCase() === 'f') twice so the second branch calling onToggleFocusMode is dead code; fix by either removing the stale onToggleFocusMode branch (the second if block) if folder search should own mod+Shift+F, or change one branch to a distinct key combination (and update its condition accordingly), and then remove onToggleFocusMode from the hook's destructure and the dependency array if you delete it; ensure you update any related tests or callers that rely on onToggleFocusMode.
🧹 Nitpick comments (19)
apps/renderer/src/components/SettingsPanel.tsx (2)
16-43: 🏗️ Heavy liftModal has no focus trap; keyboard focus can escape the dialog.
The dialog sets
role="dialog"/aria-modal="true"and focuses the section on open, but nothing constrains Tab/Shift+Tab to elements inside the modal. Keyboard users can tab into the obscured background content while the modal is up. Consider trapping focus within the dialog (cycle from last→first focusable and vice-versa) whileisOpen.🤖 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 `@apps/renderer/src/components/SettingsPanel.tsx` around lines 16 - 43, The dialog currently focuses dialogRef on open but does not trap Tab/Shift+Tab, so keyboard focus can escape; add a focus-trap in the SettingsPanel by registering a keydown handler (inside the existing useEffect that watches isOpen) that intercepts Tab/Shift+Tab, finds all focusable elements inside the section referenced by dialogRef, and when Tab would move past the last element move focus to the first (and when Shift+Tab at the first move to the last); ensure this handler is added/removed alongside the Escape handler (or merge into the same onKeyDown) and use dialogRef, isOpen, and onClose to control activation and cleanup.
86-97: 💤 Low valueRedundant labeling on the Custom CSS textarea.
The textarea is labeled via
aria-labelledby="custom-css-title"(the sr-only span on Line 96), while a visible "Custom CSS"<span>on Line 87 isn't programmatically associated. Pointingaria-labelledbyat the visible label (give Line 87 an id) is simpler and keeps the visible/accessible names in sync.🤖 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 `@apps/renderer/src/components/SettingsPanel.tsx` around lines 86 - 97, The Custom CSS textarea in SettingsPanel has an aria-labelledby pointing to the hidden sr-only span instead of the visible label; give the visible label span (the one with text "Custom CSS") a unique id and update the textarea's aria-labelledby to reference that id, and remove the redundant sr-only span so the programmatic and visible labels stay in sync (update references in the JSX for the label span and the textarea)..github/workflows/development.yml (2)
47-48: 💤 Low valueConsider
persist-credentials: falseon this read-only checkout.The security job only scans the tree and never pushes, so disabling credential persistence reduces token exposure surface (zizmor
artipacked).♻️ Suggested hardening
- name: Checkout uses: actions/checkout@v4 + with: + persist-credentials: false🤖 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 @.github/workflows/development.yml around lines 47 - 48, The Checkout step currently uses actions/checkout@v4 with default credential persistence; for a read-only security scan, set persist-credentials: false on the "Checkout" step to avoid storing the GITHUB_TOKEN in the working directory and reduce token exposure—update the step named "Checkout" to include persist-credentials: false alongside the uses: actions/checkout@v4 attribute.
23-23: ⚡ Quick winConfirm pnpm version resolution without
with.versionin CI
pnpm/action-setup@v4in.github/workflows/development.yml(setup pnpm steps at lines 23 and 50-51) will resolve the pnpm version from the rootpackage.jsonpackageManagerfield (pnpm@10.33.0), so removingwith.versionshould not break setup.Optional hardening: consider adding
persist-credentials: falseonactions/checkout@v4steps if your security linter requests it.🤖 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 @.github/workflows/development.yml at line 23, CI currently uses pnpm/action-setup@v4 without an explicit with.version and that's fine because the action will resolve pnpm from the repository package.json packageManager field (e.g., pnpm@10.33.0); verify that your root package.json has the correct packageManager entry and you can safely remove any with.version keys from the setup steps (`uses: pnpm/action-setup@v4`) or leave them if you prefer pinning. As an optional hardening step, add persist-credentials: false to the actions/checkout@v4 steps to satisfy security linters.docs/src/components/Homepage/hero.tsx (1)
75-77: ⚡ Quick winAdd
aria-labelto the video element for screen reader accessibility.The video element should include an
aria-labeldescribing its purpose or content, even though it's decorative. This improves accessibility for users with screen readers.♿ Proposed accessibility improvement
- <video autoPlay loop muted playsInline preload="metadata" className="w-full rounded-xl"> + <video autoPlay loop muted playsInline preload="metadata" className="w-full rounded-xl" aria-label="Markdown Reader product demonstration video"> <source src={videoUrl} type="video/mp4" /> </video>🤖 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 `@docs/src/components/Homepage/hero.tsx` around lines 75 - 77, The video element in the hero component (the <video> using source src={videoUrl} and className="w-full rounded-xl") lacks an accessibility label; add an aria-label attribute to that video element with a brief descriptive string (e.g., "Product demo video showing feature X" or "Background decorative video") so screen readers can identify its purpose while keeping autoplay, loop, muted behavior unchanged.apps/main-processor/__tests__/folder.test.ts (1)
7-7: 💤 Low valueAvoid an
asynccallback ondescribe.
describecallbacks are collected synchronously; the returned promise is not awaited, so any async work here (none currently, but it invites it) would not be reliably sequenced before the tests run. Dropasyncand keep async logic inside theitblocks.♻️ Proposed change
-describe('folder reader testing', async () => { +describe('folder reader testing', () => {🤖 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 `@apps/main-processor/__tests__/folder.test.ts` at line 7, The describe callback in the test suite is declared async which is unnecessary and unsafe because describe is executed synchronously; remove the async keyword from the describe(...) callback (the describe call that wraps the suite) and ensure any asynchronous setup is moved into individual it/test hooks or beforeEach/afterEach as needed (e.g., convert async work into async functions inside the it blocks or use beforeEach/afterEach async functions) so that describe remains synchronous and async operations are properly awaited.apps/main-processor/src/utils/helper/ipc-path-resolver.ts (1)
1-1: 💤 Low valueUse the
node:protocol for thepathimport.
node:fs/promisesis already used on Line 2; for consistency and explicit core-module resolution, import fromnode:path.-import path from 'path'; +import path from 'node:path';🤖 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 `@apps/main-processor/src/utils/helper/ipc-path-resolver.ts` at line 1, Update the core path import to use the node: protocol for consistency with other core imports; replace the existing import of path (the module referenced by the top-level import statement) so that the module is imported as node:path (keep the same imported identifier `path`) to match `node:fs/promises` usage and ensure explicit core-module resolution.apps/renderer/src/renderer/toc.ts (1)
5-9: 💤 Low valueRename
html→markdownand update the comment.The implementation now tokenizes Markdown via
lexer(), but the parameter and comment still say HTML, which is misleading after this change.♻️ Suggested rename
-// extracts table of content from HTML string -export function extractTOC(html: string): TOCType[] { +// extracts table of contents from a Markdown string +export function extractTOC(markdown: string): TOCType[] { const items: TOCType[] = []; const idCount = new Map<string, number>(); - const tokens = lexer(html); + const tokens = lexer(markdown);🤖 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 `@apps/renderer/src/renderer/toc.ts` around lines 5 - 9, Rename the parameter and comment to reflect Markdown: change extractTOC(html: string) and its leading comment to use "markdown" instead of "html" (e.g., extractTOC(markdown: string): TOCType[] and update the comment to "extracts table of content from Markdown string"); update all uses inside the function (e.g., the call to lexer(html) → lexer(markdown)) and any other references to the old variable name so the function, comment and internal calls consistently refer to "markdown".apps/renderer/src/utils/helpers/heading-helper.ts (1)
29-32: 💤 Low value
parseInlinereturn type assumes synchronous parsing.
parseInline(token.text) as stringis safe only while the defaultmarkedinstance stays synchronous; ifparseInlineis ever called withasync: true, it returns aPromise<string>andstripHtmlwould receive"[object Promise]". Consider passing{ async: false }explicitly to make the contract intentional.🤖 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 `@apps/renderer/src/utils/helpers/heading-helper.ts` around lines 29 - 32, The headingText function assumes parseInline is synchronous; update the call in headingText to explicitly request synchronous parsing by calling parseInline(token.text, { async: false }) and then pass the resulting string into stripHtml (e.g., const inlineHtml = parseInline(token.text, { async: false }) as string; return stripHtml(inlineHtml).trim();). This makes the contract explicit for parseInline and prevents passing a Promise (which would stringify to "[object Promise]") into stripHtml; keep the return type of headingText as string.apps/renderer/src/hooks/useExport.ts (1)
6-31: ⚡ Quick winAdd error handling around the export calls.
window.api.export*can reject (disk full, permission denied, conversion error). Currently failures surface as unhandled rejections with no user feedback. Wrap in try/catch and report via the toast/error channel.The three handlers are also near-identical; consider extracting a small helper to remove duplication:
♻️ Sketch
const runExport = useCallback( async (defaultExt: 'html' | 'pdf' | 'docx', fn: (html: string, css: string, out: string) => Promise<unknown>) => { if (!activeTab) return; const outPath = await window.api.showSaveDialog({ defaultExt }); if (!outPath) return; try { await fn(activeTab.html, exportCss, outPath); } catch (err) { // surface to user via toast/error state } }, [activeTab], );🤖 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 `@apps/renderer/src/hooks/useExport.ts` around lines 6 - 31, The three export handlers (exportHtml, exportPdf, exportDocx) call window.api.exportHTML/exportPDF/exportDOCX without error handling and duplicate the same flow; wrap the export call in a try/catch that reports failures to the user via your toast/error channel (e.g., show a toast with the error message) and avoid unhandled rejections, and refactor the repeated logic into a single helper (e.g., runExport) that accepts the defaultExt and the specific export function, uses activeTab and exportCss, runs showSaveDialog, and executes the export inside the try/catch so each handler just calls runExport with the appropriate parameters.apps/main-processor/src/export/inlineImage.ts (1)
23-25: 💤 Low valueNo-op replacement in the catch branch.
output.replace(fullMatch, fullMatch)rewrites the first occurrence with itself, which has no effect. The intent (leave the originalsrcunchanged on failure) is already satisfied by doing nothing.♻️ Proposed cleanup
- } catch { - output = output.replace(fullMatch, fullMatch); - } + } catch { + // leave the original src unchanged on failure + }🤖 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 `@apps/main-processor/src/export/inlineImage.ts` around lines 23 - 25, In the catch block remove the no-op replacement (output = output.replace(fullMatch, fullMatch)); — instead leave the original output unchanged by either making the catch empty or logging a concise warning; locate the try/catch around the inline image processing that uses variables fullMatch and output (in inlineImage.ts) and delete the replace call so failures simply preserve the original src.apps/main-processor/src/export/exportDocx.ts (1)
19-27: 💤 Low valueDefensive fallback cast may hide unexpected return types.
The
elsebranch castsresulttoUint8Arraywithout verification. Ifhtml-to-docxreturns an unexpected type (e.g.,stringorobject),Buffer.from()may silently produce garbage or throw a confusing error.Consider adding explicit type checking or logging for unexpected types:
Proposed improvement
} else if (typeof Blob !== 'undefined' && result instanceof Blob) { buffer = Buffer.from(await result.arrayBuffer()); + } else if (result instanceof Uint8Array) { + buffer = Buffer.from(result); } else { - buffer = Buffer.from(result as unknown as Uint8Array); + throw new Error(`Unexpected result type from html-to-docx: ${typeof result}`); }🤖 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 `@apps/main-processor/src/export/exportDocx.ts` around lines 19 - 27, The fallback branch in exportDocx.ts blindly casts result to Uint8Array which can hide unexpected return types; update the conversion logic in the block handling result (the Buffer.isBuffer / ArrayBuffer / Blob branches) to explicitly handle known types: check for string (use Buffer.from(string,'utf8')), check for Uint8Array or ArrayBuffer (use Buffer.from), and if none match log/throw a clear TypeError that includes typeof/result.constructor to fail fast and aid debugging rather than silently producing garbage.apps/main-processor/src/file.ts (1)
38-57: 💤 Low valueTimeout resolves silently when watcher fails to become ready.
If chokidar never emits
ready(e.g., due to permission issues that don't trigger an error), the 1-second timeout resolves successfully and the code proceeds as if the watcher is operational. Consider logging a warning or tracking this state to aid debugging file-watch issues in production.🤖 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 `@apps/main-processor/src/file.ts` around lines 38 - 57, The timeout handler currently calls resolve() silently when chokidar never emits 'ready'; change that handler so it creates a descriptive TimeoutError (e.g., "watcher ready timed out for {filePath}"), call onError?.(timeoutError) and/or log a warning, perform the same cleanup via unWatchFile(filePath).catch(() => {}), then resolve or reject depending on desired behavior (recommend resolve but mark/log the timeout) so the code no longer proceeds without any warning; update the timeout callback in the Promise that references watcher, unWatchFile, onError, and filePath accordingly.electron-builder.ts (1)
20-28: 💤 Low valueRedundant
extraResourcesentry.The first entry (
from: 'assets', to: 'assets') already bundlesunsigned-install-notice.txtinto resources, andnsis.licensereferences the source path directly. The second mapping toUNSIGNED_INSTALL_NOTICE.txtis only needed if something consumes that exact top-level resource name at runtime — otherwise it can be dropped.🤖 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-builder.ts` around lines 20 - 28, The extraResources array currently contains a redundant mapping for 'assets/unsigned-install-notice.txt' -> 'UNSIGNED_INSTALL_NOTICE.txt'; remove that second object from extraResources so only the directory mapping (from: 'assets', to: 'assets') remains, and ensure any code that references nsis.license or other runtime consumers points to the bundled path 'assets/unsigned-install-notice.txt' (or adjust those references to the top-level name if you intentionally need 'UNSIGNED_INSTALL_NOTICE.txt'); keep the symbol names extraResources and nsis.license in mind when making the change.apps/renderer/src/components/ReaderToolbar.tsx (1)
46-50: 💤 Low valueHard-coded dark-theme list will silently break if new dark themes are added.
The Sun/Moon decision enumerates
github-dark,dracula,nord. Any future dark theme defaults to the Moon (light) icon. Consider centralizing aDARK_THEMESset intheme-constantsand reusing it here so the toggle stays in sync.🤖 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 `@apps/renderer/src/components/ReaderToolbar.tsx` around lines 46 - 50, ReaderToolbar currently hard-codes dark-theme names when choosing Icons.Sun vs Icons.Moon, which will break if themes change; update the component (ReaderToolbar) to import and use a centralized DARK_THEMES (or similar) set from theme-constants instead of enumerating 'github-dark', 'dracula', 'nord', and then use DARK_THEMES.has(theme) (or equivalent) to decide between Icons.Sun and Icons.Moon so the toggle stays in sync with the canonical theme list.packages/shared-constants/src/theme-constants.ts (2)
12-19: ⚖️ Poor tradeoffFont-size/width constants fragmented across modules.
These numeric constants overlap with the
FONT_SIZE/WIDTH_MAPvalues consumed inuseSettings.ts(via../types/component-types) and the literals insettings-default.ts(e.g.fontSize: 16). Three independent definitions of the same domain values can drift. Consider consolidating to one source.🤖 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 `@packages/shared-constants/src/theme-constants.ts` around lines 12 - 19, Consolidate the font-size and width domain values into a single canonical source by making the constants in theme-constants (MIN_FONT_SIZE, MAX_FONT_SIZE, DEFAULT_FONT_SIZE, FONT_SIZE_INCREMENT, NARROW_WIDTH, DEFAULT_WIDTH, WIDE_WIDTH) the single truth; update consumers (the FONT_SIZE and WIDTH_MAP usages referenced in ../types/component-types, the logic in useSettings.ts, and the literals in settings-default.ts) to import and use these exported symbols instead of duplicating literals or separate maps, and remove or replace any duplicate definitions so all modules reference theme-constants.
1-10: ⚖️ Poor tradeoffMake
THEMESthe single source of truth forTheme.
packages/shared-types/src/settings-type.tshard-codes aThemestring union that duplicatespackages/shared-constants/src/theme-constants.ts(THEMES+ derivedTheme). DeriveThemeinsettings-type.tsfrom the exportedTHEMESvalue in@package/shared-constants(note:shared-typescurrently has no imports/dependency onshared-constants, so you’ll need to add the dependency/import first).🤖 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 `@packages/shared-constants/src/theme-constants.ts` around lines 1 - 10, Add a dependency from packages/shared-types to packages/shared-constants and import the THEMES constant into packages/shared-types/src/settings-type.ts, then replace the hard-coded Theme string union with a derived type: export type Theme = (typeof THEMES)[number]; reference the exported THEMES constant and ensure any existing Theme type declaration in settings-type.ts is removed/updated to use that derived type.apps/renderer/src/hooks/useFolderSearch.ts (1)
19-40: ⚡ Quick winConsider debouncing folder search to avoid per-keystroke IPC.
searchFolderis wired toonQueryChangeinApp.tsx, so every keystroke triggers awindow.api.searchFolderround-trip. The stale-response guard prevents incorrect results, but on large folders this still issues a burst of redundant traversals. A short debounce (e.g. 150–250ms) would reduce main-process load while keeping responsiveness.🤖 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 `@apps/renderer/src/hooks/useFolderSearch.ts` around lines 19 - 40, searchFolder in useFolderSearch is called on every keystroke (via onQueryChange) causing excessive IPC; debounce the search to batch rapid input. Wrap the existing async searchFolder logic in a debounced function (150–250ms) or use a useDebouncedCallback inside the hook so requests to window.api.searchFolder(folderPath, query) only fire after the pause, while preserving the current requestId stale-response guard, setFolderQuery behavior, and setIsSearchingFolder handling; ensure cancellation/cleanup on unmount by clearing the debounce timer or cancelling the debounced callback so no out-of-date calls run.apps/renderer/src/types/hook-types.ts (1)
76-76: 💤 Low valueTypo in public contract member name:
getHiglightedHtml.The interface member is misspelled (should be
getHighlightedHtml). It matches the currentuseSearchimplementation so it's not breaking, but renaming the contract and its usages now avoids cementing the typo into the public type surface.🤖 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 `@apps/renderer/src/types/hook-types.ts` at line 76, Rename the misspelled interface member getHiglightedHtml to getHighlightedHtml in the hook-types interface and update every usage to match (including the useSearch implementation and any callers/exports that reference getHiglightedHtml) so the public contract and implementation use the correct spelling; search for the symbol getHiglightedHtml, rename it to getHighlightedHtml, update type annotations and implementations (e.g., the function assigned in useSearch) and run type checks to ensure no remaining references remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7309821c-4f48-4a6a-b4eb-a2de1ff7bc0a
⛔ Files ignored due to path filters (5)
apps/renderer/public/icons/app-icon.svgis excluded by!**/*.svgdocs/static/img/docusaurus-social-card.jpgis excluded by!**/*.jpgdocs/static/img/markdown-social-card.pngis excluded by!**/*.pngdocs/static/video/hero.mp4is excluded by!**/*.mp4pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (140)
.coderabbit.yaml.github/workflows/development.yml.github/workflows/production.yml.github/workflows/release.yml.gitignore.husky/pre-push.prettierignoreREADME.mdapps/main-processor/__tests__/cli.test.tsapps/main-processor/__tests__/docx.test.tsapps/main-processor/__tests__/export.test.tsapps/main-processor/__tests__/file.test.tsapps/main-processor/__tests__/folder-search.test.tsapps/main-processor/__tests__/folder.test.tsapps/main-processor/__tests__/ipc.test.tsapps/main-processor/__tests__/menu.test.tsapps/main-processor/__tests__/recent.test.tsapps/main-processor/__tests__/settings.test.tsapps/main-processor/package.jsonapps/main-processor/setup.tsapps/main-processor/src/cli.tsapps/main-processor/src/export/buildDocument.tsapps/main-processor/src/export/exportDocx.tsapps/main-processor/src/export/exportHtml.tsapps/main-processor/src/export/exportPdf.tsapps/main-processor/src/export/getImage.tsapps/main-processor/src/export/inlineImage.tsapps/main-processor/src/export/sanitizeCss.tsapps/main-processor/src/file.tsapps/main-processor/src/folder-search.tsapps/main-processor/src/folder.tsapps/main-processor/src/index.tsapps/main-processor/src/ipc.tsapps/main-processor/src/menu.tsapps/main-processor/src/recent/getUniqueRecentFile.tsapps/main-processor/src/settings/get-settings.tsapps/main-processor/src/settings/save-settings.tsapps/main-processor/src/types/watch-file-types.tsapps/main-processor/src/updater.tsapps/main-processor/src/utils/constants/export-constants.tsapps/main-processor/src/utils/constants/folder-constants.tsapps/main-processor/src/utils/constants/ipc-validation.tsapps/main-processor/src/utils/constants/path-constants.tsapps/main-processor/src/utils/constants/setting-constants.tsapps/main-processor/src/utils/constants/window-constants.tsapps/main-processor/src/utils/helper/folder-search-helper.tsapps/main-processor/src/utils/helper/html-template.tsapps/main-processor/src/utils/helper/ipc-path-resolver.tsapps/main-processor/src/utils/helper/menu-helper.tsapps/main-processor/src/utils/helper/path-helper.tsapps/main-processor/src/utils/helper/setting-helper.tsapps/main-processor/src/utils/ipc-validation.tsapps/main-processor/tests/ipc.test.tsapps/main-processor/vitest.config.tsapps/preload/src/index.tsapps/preload/src/utils/menu-event-helper.tsapps/renderer/__tests__/components/Sidebar.test.tsxapps/renderer/__tests__/components/StatusBar.test.tsxapps/renderer/__tests__/components/TabBar.test.tsxapps/renderer/__tests__/components/Welcome.test.tsxapps/renderer/__tests__/hooks/useCollapsibleToc.test.tsapps/renderer/__tests__/hooks/useSearch.test.tsapps/renderer/__tests__/hooks/useSettings.test.tsapps/renderer/__tests__/renderer/callout.test.tsapps/renderer/__tests__/renderer/drag-drop.test.tsapps/renderer/__tests__/renderer/katex.test.tsapps/renderer/__tests__/renderer/markdown.test.tsapps/renderer/__tests__/renderer/mermaid.test.tsapps/renderer/__tests__/renderer/sanitize.test.tsapps/renderer/__tests__/renderer/sikhi.test.tsapps/renderer/__tests__/renderer/toc.test.tsapps/renderer/__tests__/store/tabStore.test.tsapps/renderer/index.htmlapps/renderer/setup.tsapps/renderer/src/App.tsxapps/renderer/src/components/DragDrop.tsxapps/renderer/src/components/Error.tsxapps/renderer/src/components/ErrorBoundary.tsxapps/renderer/src/components/FileBrowser.tsxapps/renderer/src/components/FileTree.tsxapps/renderer/src/components/Loading.tsxapps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/components/SearchBar.tsxapps/renderer/src/components/SettingsPanel.tsxapps/renderer/src/components/Sidebar.tsxapps/renderer/src/components/StatusBar.tsxapps/renderer/src/components/TabBar.tsxapps/renderer/src/components/Toast.tsxapps/renderer/src/components/UpdateBanner.tsxapps/renderer/src/config/marked.tsapps/renderer/src/context/ThemeProvider.tsxapps/renderer/src/hooks/useCollapsibleToc.tsapps/renderer/src/hooks/useDragDrop.tsapps/renderer/src/hooks/useExport.tsapps/renderer/src/hooks/useFile.tsapps/renderer/src/hooks/useFileActions.tsapps/renderer/src/hooks/useFilePersistence.tsapps/renderer/src/hooks/useFolderSearch.tsapps/renderer/src/hooks/useLayout.tsapps/renderer/src/hooks/useMenuEvents.tsapps/renderer/src/hooks/useOpenFilePath.tsapps/renderer/src/hooks/useSearch.tsapps/renderer/src/hooks/useSettings.tsapps/renderer/src/hooks/useShortcuts.tsapps/renderer/src/hooks/useTabNavigation.tsapps/renderer/src/main.tsxapps/renderer/src/renderer/drag-drop.tsapps/renderer/src/renderer/shiki.tsapps/renderer/src/renderer/toc.tsapps/renderer/src/store/tabStore.tsapps/renderer/src/styles/export.cssapps/renderer/src/types/component-types.tsapps/renderer/src/types/hook-types.tsapps/renderer/src/utils/constants/icon-contants.tsxapps/renderer/src/utils/helpers/heading-helper.tsapps/renderer/src/utils/helpers/sidebar-helper.tsapps/renderer/src/utils/helpers/tab-helper.tsxapps/renderer/src/utils/helpers/theme-helper.tsassets/unsigned-install-notice.txtdocs/docs/architecture.mddocs/docs/ipc-api.mddocs/docusaurus.config.tsdocs/sidebars.tsdocs/src/components/Homepage/hero.tsxdocs/versioned_docs/version-1.0.0/architecture.mddocs/versioned_docs/version-1.0.0/ipc-api.mddocs/versioned_sidebars/version-1.0.0-sidebars.jsonelectron-builder.tspackage.jsonpackages/shared-constants/src/index.tspackages/shared-constants/src/ipc-constants.tspackages/shared-constants/src/menu-constants.tspackages/shared-constants/src/path-constants.tspackages/shared-constants/src/theme-constants.tspackages/shared-types/package.jsonpackages/shared-types/src/index.tspackages/shared-types/src/markdown-type.tspackages/shared-types/src/search-type.tspackages/shared-types/src/settings-default.tspackages/shared-types/src/settings-type.ts
💤 Files with no reviewable changes (2)
- apps/main-processor/tests/ipc.test.ts
- apps/main-processor/src/utils/ipc-validation.ts
| it('it should create a non-empty DOCX file', () => { | ||
| const stats = statSync(docxPath); | ||
|
|
||
| expect(stats.size).toBeGreaterThan(100); | ||
| }); |
There was a problem hiding this comment.
Test depends on file produced by a previous test.
statSync(docxPath) relies on the first test having created the file. If the first test fails or the suite is reordered/filtered (e.g. it.only), this test throws ENOENT instead of giving a clear failure. Consider folding the size check into the creation test, or recreating the file in this test.
🤖 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 `@apps/main-processor/__tests__/docx.test.ts` around lines 28 - 32, The test
currently calls statSync(docxPath) which depends on a previous test having
created the file and will throw ENOENT if the file is missing; update this test
to be self-contained by either invoking the same creation routine used in the
creation test (call the function that produces the DOCX before asserting) or by
merging the size assertion into the DOCX creation test; reference the docxPath
constant and replace the standalone statSync(docxPath) call with a creation step
followed by statSync(docxPath) and the expect(stats.size).toBeGreaterThan(100)
assertion so the test no longer relies on test order.
| EXPORT: 'Export', | ||
| EXPORT_HTML: 'Export as HTML', | ||
| EXPORT_PDF: 'Export as PDF', | ||
| EXPORT_DOCX: 'Export as Docx', |
There was a problem hiding this comment.
Inconsistent label casing.
EXPORT_DOCX: 'Export as Docx' is inconsistent with the sibling 'Export as PDF' / 'Export as HTML'. Use 'Export as DOCX' for a consistent, user-facing menu label.
✏️ Proposed fix
- EXPORT_DOCX: 'Export as Docx',
+ EXPORT_DOCX: 'Export as DOCX',🤖 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 `@packages/shared-constants/src/menu-constants.ts` at line 11, The menu label
for EXPORT_DOCX is inconsistent with its siblings; update the value of the
EXPORT_DOCX constant in menu-constants.ts from 'Export as Docx' to 'Export as
DOCX' so the user-facing label uses the same uppercase extension style as
'Export as PDF' and 'Export as HTML'.
…g the main window
Fix/system hardening
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/renderer/src/utils/helpers/heading-helper.ts (1)
36-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDecode HTML entities before building TOC text and ids.
parseInline(token.text)escapes entities first, so a heading like## A & BbecomesA & Bhere.getHeadingId()then slugs that toa-amp-b, while the rendered heading ID is still derived from the raw text and staysa-b, so the sidebar TOC link points at the wrong anchor.💡 Proposed fix
export function headingText(token: Tokens.Heading): string { const inlineHtml = parseInline(token.text) as string; - return stripHtml(inlineHtml).trim(); + return decodeHtml(stripHtml(inlineHtml)).trim(); }🤖 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 `@apps/renderer/src/utils/helpers/heading-helper.ts` around lines 36 - 38, headingText() is using parseInline(token.text) which returns escaped HTML entities, causing mismatched TOC IDs vs rendered headings; update headingText (and ensure getHeadingId uses the same source) to decode HTML entities after parseInline and before stripHtml/slugging so "A & B" becomes "A & B"; use your project's HTML-decoding utility (or DOMParser/textarea/unescape helper) on the parsed inline string, then call stripHtml(...).trim() and pass that decoded string to getHeadingId to keep TOC links consistent with rendered heading IDs.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@apps/renderer/src/utils/helpers/heading-helper.ts`:
- Around line 23-27: The heading renderer currently escapes the stripped text
and loses inline markdown while headingText builds plain text differently,
causing TOC ID drift; update heading({ text, depth }, registry) to render the
inner HTML from the inline tokens via this.parser.parseInline(tokens) (i.e., use
the parser’s inline-rendered HTML for the hN content) and compute the id by
decoding entities from the same plain source passed to getHeadingId so both
paths normalize identically; in practice use the parser to produce safe HTML for
the element body and run decodeHtml(...) (or the same decode path used by
headingText) on the plain-result before calling getHeadingId (keeping
getHeadingId usage intact) to keep TOC slugs and emitted IDs synchronized.
---
Outside diff comments:
In `@apps/renderer/src/utils/helpers/heading-helper.ts`:
- Around line 36-38: headingText() is using parseInline(token.text) which
returns escaped HTML entities, causing mismatched TOC IDs vs rendered headings;
update headingText (and ensure getHeadingId uses the same source) to decode HTML
entities after parseInline and before stripHtml/slugging so "A & B" becomes
"A & B"; use your project's HTML-decoding utility (or
DOMParser/textarea/unescape helper) on the parsed inline string, then call
stripHtml(...).trim() and pass that decoded string to getHeadingId to keep TOC
links consistent with rendered heading IDs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0c16d786-ac72-45e6-a54d-ac359bb6f055
📒 Files selected for processing (20)
.coderabbit.yamlapps/main-processor/src/export/exportPdf.tsapps/main-processor/src/index.tsapps/main-processor/src/ipc.tsapps/main-processor/src/menu.tsapps/main-processor/src/register-menu.tsapps/main-processor/src/settings/get-settings.tsapps/main-processor/src/updater.tsapps/main-processor/src/utils/constants/ipc-validation.tsapps/renderer/index.htmlapps/renderer/src/App.tsxapps/renderer/src/config/marked.tsapps/renderer/src/hooks/useFileActions.tsapps/renderer/src/hooks/useFilePersistence.tsapps/renderer/src/renderer/markdown.tsapps/renderer/src/renderer/shiki.tsapps/renderer/src/renderer/toc.tsapps/renderer/src/utils/helpers/heading-helper.tsdocs/docs/architecture.mddocs/versioned_docs/version-1.0.0/architecture.md
💤 Files with no reviewable changes (2)
- docs/docs/architecture.md
- docs/versioned_docs/version-1.0.0/architecture.md
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/main-processor/src/updater.ts
- apps/renderer/src/renderer/shiki.ts
- apps/main-processor/src/menu.ts
- apps/main-processor/src/export/exportPdf.ts
- apps/renderer/index.html
- apps/renderer/src/hooks/useFileActions.ts
- apps/renderer/src/hooks/useFilePersistence.ts
- apps/main-processor/src/settings/get-settings.ts
- apps/main-processor/src/index.ts
- apps/renderer/src/App.tsx
- apps/main-processor/src/ipc.ts
- .coderabbit.yaml
| export function heading({ text, depth }: HeadingProps, registry: Map<string, number>) { | ||
| const plainText = stripHtml(text); | ||
| const id = getHeadingId(plainText, registry); | ||
| const safeText = escapeHtml(plainText); | ||
| return `<h${depth} id="${id}">${safeText}</h${depth}>\n`; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Official Marked docs for custom heading renderers: should renderer.headingrender heading content viathis.parser.parseInline(tokens) to preserve inline markdown formatting?
💡 Result:
Yes. In Marked’s official extensibility docs, the renderer.heading override example renders the heading’s inline content by calling this.parser.parseInline(tokens) (where tokens is the heading token’s inline token array), specifically so inline markdown inside the heading (e.g., **bold**, links, code spans) is preserved in the generated HTML. [1]
Source
- [1] Marked Documentation – “Extending Marked” / Renderer example (
heading({ tokens, depth }) { const text = this.parser.parseInline(tokens); ... }) (marked.js.org)
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and show the relevant sections (small excerpts)
FILE="apps/renderer/src/utils/helpers/heading-helper.ts"
if [ -f "$FILE" ]; then
echo "== $FILE =="
wc -l "$FILE"
sed -n '1,120p' "$FILE" | nl -ba | sed -n '1,120p'
else
echo "Missing $FILE"
fi
# Find where heading-helper's functions are used (especially for TOC)
rg -n "headingHelper|headingText|getHeadingId|createHeadingRegistry|decodeHtml|heading\(" apps/renderer/src -S || true
rg -n "toc|table of contents|TOC" apps/renderer/src -S || true
rg -n "getHeadingId\(" -S . || trueRepository: mindfiredigital/markdown-reader
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/renderer/src/utils/helpers/heading-helper.ts"
if [ -f "$FILE" ]; then
echo "== $FILE =="
wc -l "$FILE"
sed -n '1,120p' "$FILE" | cat -n
else
echo "Missing $FILE"
fi
echo "== usages in apps/renderer/src =="
rg -n "heading-helper|headingText|getHeadingId|createHeadingRegistry|decodeHtml|renderer\.heading|heading\(" apps/renderer/src -S || true
echo "== TOC related =="
rg -n "(toc|table of contents|TOC)" apps/renderer/src -S || true
echo "== getHeadingId references across repo =="
rg -n "getHeadingId\(" -S . || trueRepository: mindfiredigital/markdown-reader
Length of output: 9849
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' apps/renderer/src/config/marked.ts | cat -n
echo "----"
sed -n '1,120p' apps/renderer/src/renderer/toc.ts | cat -n
echo "----"
sed -n '1,120p' apps/renderer/src/renderer/markdown.ts | cat -nRepository: mindfiredigital/markdown-reader
Length of output: 3821
🏁 Script executed:
sed -n '1,120p' apps/renderer/src/hooks/useFile.ts | cat -nRepository: mindfiredigital/markdown-reader
Length of output: 2538
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print the HeadingProps type definition
rg -n "export (type|interface) HeadingProps|HeadingProps" apps/renderer/src/types/component-types.ts apps/renderer/src -S || true
# Show around the match
MATCH_FILE="apps/renderer/src/types/component-types.ts"
if [ -f "$MATCH_FILE" ]; then
# Print entire file if small enough; otherwise print around likely area
wc -l "$MATCH_FILE"
sed -n '1,220p' "$MATCH_FILE" | cat -n
fiRepository: mindfiredigital/markdown-reader
Length of output: 5948
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json =="
ls -la package.json apps/renderer/package.json 2>/dev/null || true
cat package.json 2>/dev/null | head -n 120 || true
cat apps/renderer/package.json 2>/dev/null | head -n 120 || true
echo "== marked version from lockfile (if present) =="
ls -ლა package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || true
rg -n "\"marked\"\\s*:" package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || true
rg -n "marked@" pnpm-lock.yaml yarn.lock 2>/dev/null || true
echo "== check for marked package availability =="
node -e "try{const m=require('marked'); console.log('marked loaded', typeof m, Object.keys(m).slice(0,5));}catch(e){console.error('marked require failed:', e.message); process.exit(0)}"Repository: mindfiredigital/markdown-reader
Length of output: 3763
🌐 Web query:
Marked v18 renderer.heading props shape { tokens, depth } or { text, depth } and recommended usage to preserve inline markdown
💡 Result:
In Marked v18, renderer methods receive the token object itself rather than a list of individual parameters [1][2]. For a heading renderer, the expected prop shape is { tokens, depth, raw } (where tokens is an array of inline tokens) [3][4][1]. Recommended usage to preserve inline markdown involves using the provided tokens and the parser's built-in methods [1][5]. Specifically, you should use this.parser.parseInline(tokens) to convert the heading's inline tokens into HTML, which ensures that markdown formatting (like bold, italics, or links) within the heading text is correctly processed [3][1][5]. Example of correct implementation: renderer: { heading({ tokens, depth }) { // Correctly parse the inline tokens to preserve markdown formatting const text = this.parser.parseInline(tokens); return <h${depth}>${text}</h${depth}>\n; } } Key points for v18 renderers: - Token Object: The renderer receives a single token object of type Tokens.Heading [4][1]. - Parser Access: The renderer context (this) provides access to the parser, specifically this.parser.parseInline for inline elements and this.parser.parse for block elements [3][5]. - Avoiding Deprecation: Ensure you are accepting the token object instead of positional arguments, as older signatures may no longer be supported or are deprecated [1][2].
Citations:
- 1: Linking back to the source markedjs/marked#3591
- 2: feat: Send token objects to renderers markedjs/marked#3291
- 3: https://github.com/markedjs/marked/blob/v18.0.0/docs/USING_PRO.md
- 4: https://github.com/markedjs/marked/blob/0ccf4769/src/Renderer.ts
- 5: https://github.com/markedjs/marked/blob/0ccf4769/docs/USING_PRO.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/renderer/src/utils/constants/regex-constants.ts"
if [ -f "$FILE" ]; then
wc -l "$FILE"
sed -n '1,200p' "$FILE" | cat -n
else
echo "Missing $FILE"
fiRepository: mindfiredigital/markdown-reader
Length of output: 1267
🌐 Web query:
Marked v18 Tokens.Heading properties: what is token.text and what is renderer.heading props.text vs tokens
💡 Result:
In Marked v18, the distinction between token properties and renderer arguments centers on how the library handles raw markdown versus parsed content [1][2]. Token.text The text property on a Heading token (Tokens.Heading) contains the raw markdown text of the heading, excluding the leading hashes (#) and surrounding whitespace that define the heading level [1][3]. It represents the content that needs to be further parsed to handle inline markdown elements like bold, italic, or links [2]. Renderer.heading Props In recent versions of Marked, the renderer methods (including heading) receive a single token object as an argument, rather than a list of individual parameters [2][4]. Heading Renderer Signature: The heading renderer expects a Tokens.Heading object, which contains: - tokens: An array of parsed inline tokens generated from the heading text [1][3]. This should be passed to this.parser.parseInline(tokens) to convert the heading's inner markdown into HTML [2]. - depth: The numeric level of the heading (e.g., 1 for h1, 2 for h2) [1][3]. - raw: The original, full markdown string that produced the token [2]. - text: The raw text content of the heading [1]. Key Difference When customizing the renderer, you should use token.tokens to render the content to HTML, as it allows Marked to correctly process inline elements [2]. Using token.text directly would treat the heading content as plain text, bypassing the parser and failing to render inline markdown (like formatting) correctly [2].
Citations:
- 1: https://github.com/markedjs/marked/blob/0ccf4769/src/Tokens.ts
- 2: Linking back to the source markedjs/marked#3591
- 3: https://github.com/markedjs/marked/blob/0ccf4769/src/Tokenizer.ts
- 4: feat: Send token objects to renderers markedjs/marked#3291
Fix heading rendering to preserve inline markdown and keep TOC IDs in sync
apps/renderer/src/utils/helpers/heading-helper.ts:heading()renders<h*>fromstripHtml(text)+escapeHtml(...), which outputs escaped raw heading content and drops inline markdown formatting (e.g.,*emphasis*,code,[link](...)). Update the Markedrenderer.headingimplementation to generate the heading HTML from the heading’s inline tokens viathis.parser.parseInline(tokens)(as Marked’s extension API intends), rather than escaping the raw text.apps/renderer/src/utils/helpers/heading-helper.ts:headingText()runsparseInline(token.text)and strips tags, but it never decodes HTML entities, sogetHeadingId()can produce different slugs vs the IDs emitted byheading()(the file’sdecodeHtml()is currently unused). Decode entities before slug generation, or normalize both code paths from the same plain-text source so TOC anchors don’t drift.
🤖 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 `@apps/renderer/src/utils/helpers/heading-helper.ts` around lines 23 - 27, The
heading renderer currently escapes the stripped text and loses inline markdown
while headingText builds plain text differently, causing TOC ID drift; update
heading({ text, depth }, registry) to render the inner HTML from the inline
tokens via this.parser.parseInline(tokens) (i.e., use the parser’s
inline-rendered HTML for the hN content) and compute the id by decoding entities
from the same plain source passed to getHeadingId so both paths normalize
identically; in practice use the parser to produce safe HTML for the element
body and run decodeHtml(...) (or the same decode path used by headingText) on
the plain-result before calling getHeadingId (keeping getHeadingId usage intact)
to keep TOC slugs and emitted IDs synchronized.
mind-murtaza
left a comment
mind-murtaza
left a comment
There was a problem hiding this comment.
Overall, this looks like a solid update with several good improvements around IPC validation, file handling, Shiki initialization, and TOC generation. However, I found a few items that should be addressed before merging.
1. Path Traversal / Arbitrary File Read in Export
File: apps/main-processor/src/export/inlineImage.ts
The export flow reads image paths directly from rendered HTML and inlines them without validating the resolved path. This could allow a crafted markdown document to reference arbitrary local files, which would then be embedded into exported PDF/HTML/DOCX outputs.
Recommendation: Validate image paths before reading them and ensure they resolve within approved directories (e.g., markdown workspace roots). Using realpath() and verifying the resolved path is contained within an allowed root would provide stronger protection.
2. CSS Sanitization Coverage
File: apps/main-processor/src/utils/constants/export-constants.ts
The current CSS sanitization rules cover several legacy injection vectors but do not block external resource loading through @import or remote url(...) references.
Recommendation: Extend the sanitization patterns to block:
@importurl(http://...)url(https://...)- protocol-relative URLs (
url(//...))
This will reduce the risk of exported HTML loading remote resources when opened in a browser.
3. Marked Instance Recreation
File: apps/renderer/src/config/marked.ts
The previous implementation cached the Marked instance, whereas the new implementation creates a fresh instance and re-registers extensions on every render. This introduces unnecessary allocations and setup work for large documents.
Recommendation: Consider extracting reusable configuration and caching the shared portions of the setup to avoid repeated initialization overhead.
4. Theme Selection via Array Index
File: apps/renderer/src/config/marked.ts
Using THEMES[1] creates a hidden dependency on array ordering and can silently break if the list changes in the future.
Recommendation: Use an explicit theme constant (e.g. 'github-dark') or introduce a named default theme constant.
5. Unbounded customCss Length
File: apps/main-processor/src/utils/helper/setting-helper.ts
customCss currently accepts strings of any size. Extremely large values could negatively impact export performance and increase processing overhead.
Recommendation: Introduce a reasonable maximum size limit consistent with the validation approach used for other settings.
Minor / Follow-up Items
- Remove the redundant guard inside the debounce callback in
useFilePersistence.ts. - Add an
isMountedcheck inside the scroll persistence timeout callback. - Consider removing the silent
realpath()fallback infolder.tssince the current flow already resolves paths earlier. - Tighten
validateSender()by restricting acceptedfile://origins to known renderer locations. - Consider moving the
ErrorBoundaryhigher in the component tree to provide broader crash protection.
Overall, I think the merge is very close. The export-related security concerns should be resolved before merging, while the remaining items can be addressed either in this PR or as follow-up improvements.
2 participants
Description
This PR raised to merge dev into main
Type of Change
Checklist
Summary by CodeRabbit
New Features
Improvements
Documentation