feat(vault): add local file editing and terminal split view#42
Open
Vonder85 wants to merge 7 commits into
Open
feat(vault): add local file editing and terminal split view#42Vonder85 wants to merge 7 commits into
Vonder85 wants to merge 7 commits into
Conversation
- Add readLocalFile/writeLocalFile IPC handlers for reading/writing local files - Add binary file detection to prevent opening non-text files - Add 'Open File' button in Vault to edit local files directly - Add file editor split view in terminal panels (vertical layout) - FileEditorPanel component with write/preview tabs and Cmd+S save - Add error handling around vault DB initialization in main.ts
|
@Vonder85 is attempting to deploy a commit to the Charlie Rabiller's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds Electron IPC handlers for local file read/write and clipboard-image saving; exposes them via preload types; introduces a dockable FileEditorPanel with paste-to-image support; integrates per-agent opened-file state and editor split into terminal panels; adds local-file edit/save flows in the Vault UI. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as "React Component"
participant Preload as "Preload / electronAPI"
participant Main as "Electron Main"
participant FS as "File System"
User->>UI: Click "Open File"
UI->>Preload: electronAPI.vault.readLocalFile(filePath)
Preload->>Main: IPC vault:readLocalFile
Main->>FS: resolve path, stat, size/null-byte checks, read bytes
FS-->>Main: file bytes
Main-->>Preload: {content, filename, filePath}
Preload-->>UI: {content, filename, filePath}
User->>UI: Paste image
UI->>Preload: electronAPI.vault.saveClipboardImage({imageDataUrl, targetDir})
Preload->>Main: IPC vault:saveClipboardImage
Main->>FS: decode base64, ensure dir, write timestamped file
FS-->>Main: {filePath, filename}
Main-->>Preload: {success, filePath, filename}
Preload-->>UI: {success, filePath, filename}
UI->>UI: Insert markdown image link
User->>UI: Click Save
UI->>Preload: electronAPI.vault.writeLocalFile({filePath, content})
Preload->>Main: IPC vault:writeLocalFile
Main->>FS: write UTF-8 file
FS-->>Main: {success}
Main-->>Preload: {success, filePath}
Preload-->>UI: {success}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
src/components/VaultView/index.tsx (1)
319-322: Redundant state update.Line 320 updates
localFilecontent, but line 322 immediately sets it tonull. The intermediate update is wasted.Proposed simplification
if (result.success) { - setLocalFile(prev => prev ? { ...prev, content: data.content } : null); setViewMode('list'); setLocalFile(null); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/VaultView/index.tsx` around lines 319 - 322, The code currently calls setLocalFile twice when result.success is true—first updating the content via setLocalFile(prev => prev ? { ...prev, content: data.content } : null) and then immediately calling setLocalFile(null), so the intermediate update is wasted; remove the content-update call and only call setViewMode('list') and setLocalFile(null) (or alternatively remove the final setLocalFile(null) if you intend to keep the updated localFile), updating the block around result.success to use only the intended single setLocalFile call while keeping setViewMode('list').src/components/TerminalsView/components/TerminalPanel.tsx (1)
98-129: Event listeners could leak on unmount during drag.If the component unmounts while the user is mid-drag, the
mousemoveandmouseuplisteners remain attached todocument. Consider cleaning up inuseEffectreturn.Proposed fix: cleanup on unmount
+ // Track listeners for cleanup + const listenersRef = useRef<{ move?: (e: MouseEvent) => void; up?: () => void }>({}); + + useEffect(() => { + return () => { + // Cleanup any dangling listeners on unmount + if (listenersRef.current.move) { + document.removeEventListener('mousemove', listenersRef.current.move); + } + if (listenersRef.current.up) { + document.removeEventListener('mouseup', listenersRef.current.up); + } + }; + }, []); const handleResizeStart = useCallback((e: React.MouseEvent) => { e.preventDefault(); e.stopPropagation(); isDraggingRef.current = true; const onMouseMove = (ev: MouseEvent) => { // ... existing logic }; const onMouseUp = () => { isDraggingRef.current = false; document.removeEventListener('mousemove', onMouseMove); document.removeEventListener('mouseup', onMouseUp); + listenersRef.current = {}; }; + listenersRef.current = { move: onMouseMove, up: onMouseUp }; document.addEventListener('mousemove', onMouseMove); document.addEventListener('mouseup', onMouseUp); }, [isVertical, dockPosition]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TerminalsView/components/TerminalPanel.tsx` around lines 98 - 129, The drag listeners added in handleResizeStart (onMouseMove/onMouseUp) can leak if the component unmounts mid-drag; make the handlers stable and ensure cleanup on unmount: extract the onMouseMove and onMouseUp functions into refs (e.g. onMouseMoveRef/onMouseUpRef) or module-scoped callbacks that handle isDraggingRef, splitContainerRef and setEditorRatio, have handleResizeStart assign/use those stable handlers when calling document.addEventListener, and add a useEffect with a cleanup that removes document listeners using the same handler refs and sets isDraggingRef.current = false on unmount.electron/handlers/vault-handlers.ts (1)
378-388: Add path validation to IPC handlers for defense-in-depth.While file paths currently come from user dialog selections, the IPC handlers accept arbitrary paths from the renderer without validation. If the renderer were compromised (XSS, supply chain attack), these handlers could be exploited. Per Electron security best practices, validate paths in the main process independently.
Consider validating that
writeLocalFileandsaveClipboardImageonly accept paths derived from previously-opened files, and normalize/resolve all paths to prevent traversal attacks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/handlers/vault-handlers.ts` around lines 378 - 388, The IPC handler for 'vault:writeLocalFile' (and similarly saveClipboardImage) accepts arbitrary renderer-supplied paths; add defense-in-depth by validating and normalizing paths in the main process: resolve and realpath the incoming params.filePath (use path.resolve/path.normalize and fs.realpathSync) and reject any path that is not explicitly present in a maintained allowlist/Set of previously-opened real paths (populate that allowlist when files are opened/selected earlier), returning { success: false, error: 'invalid path' } if the check fails; also ensure traversal is prevented by comparing the realpath against allowed entries and log rejected attempts for auditing.src/components/TerminalsView/components/FileEditorPanel.tsx (2)
183-186: Preview mode doesn't render markdown.The preview tab displays raw content in a
<pre>tag, whileDocumentEditorusesSimpleMarkdownfor proper rendering. Since the component supports pasting images as markdown syntax, users might expect markdown preview here as well.💡 Suggested improvement
Consider importing and using
SimpleMarkdownfor consistency:+import { SimpleMarkdown } from '@/components/VaultView/components/MarkdownRenderer'; // In the preview section: ) : ( <div className="flex-1 overflow-y-auto p-3 text-xs text-foreground bg-[`#1a1a2e`]"> - <pre className="whitespace-pre-wrap font-mono">{content}</pre> + <SimpleMarkdown content={content} /> </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TerminalsView/components/FileEditorPanel.tsx` around lines 183 - 186, The preview pane in FileEditorPanel currently renders raw text inside a <pre> (using the content variable) instead of parsing markdown; update FileEditorPanel to import and use the existing SimpleMarkdown component (same used by DocumentEditor) to render content in the preview tab so pasted image/markdown syntax renders correctly; ensure you replace the <pre> block that references content with SimpleMarkdown (or the wrapper used by DocumentEditor) and pass the content string and any needed props (e.g., className or image handlers) so styling and image rendering remain consistent.
164-170: No confirmation when closing with unsaved changes.The close button calls
onClose()directly without checking thesavedstate. Users could accidentally lose unsaved work. Consider adding a confirmation dialog or handling this in the parent component.💡 Suggested approach
Either handle it locally:
<button - onClick={onClose} + onClick={() => { + if (!saved && !confirm('You have unsaved changes. Close anyway?')) return; + onClose(); + }} className="p-0.5 hover:bg-primary/10 transition-colors text-muted-foreground hover:text-red-400" title="Close file" >Or pass the
savedstate to the parent and let it handle the confirmation logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TerminalsView/components/FileEditorPanel.tsx` around lines 164 - 170, The close button in FileEditorPanel calls onClose() directly and can discard unsaved edits; modify the click handler for the button that currently calls onClose to first check the component's saved state (the local saved variable/prop) and if saved === false prompt the user (e.g., window.confirm or a modal) and only call onClose when the user confirms; alternatively change the onClose signature to accept the saved flag (e.g., onClose(saved)) and let the parent show the confirmation—update the button's onClick to implement one of these approaches and reference FileEditorPanel and the onClose prop when making the change.src/components/VaultView/components/DocumentEditor.tsx (1)
243-261: Missing error handling for FileReader and IPC failures.If the FileReader fails or the IPC call returns an error, the user receives no feedback. Consider adding
reader.onerrorhandling and checkingresult.error.💡 Suggested improvement
const reader = new FileReader(); + reader.onerror = () => { + console.error('Failed to read clipboard image'); + }; reader.onload = async () => { const dataUrl = reader.result as string; if (!window.electronAPI?.vault?.saveClipboardImage) return; // For local files, save to same directory; for vault docs, use vault attachments const targetDir = isLocalFile && localFile ? localFile.filePath.split('/').slice(0, -1).join('/') : undefined; const result = await window.electronAPI.vault.saveClipboardImage({ imageDataUrl: dataUrl, targetDir: targetDir || undefined, }); - if (result.success && result.filePath && result.filename) { + if (result.error) { + console.error('Failed to save clipboard image:', result.error); + return; + } + if (result.success && result.filePath && result.filename) { insertTextAtCursor(``); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/VaultView/components/DocumentEditor.tsx` around lines 243 - 261, Add robust error handling around the FileReader and the IPC call in the clipboard image save flow: attach reader.onerror to handle and notify on read failures (log error and show a user-facing notification), wrap the await window.electronAPI.vault.saveClipboardImage call in try/catch to handle thrown exceptions, and after the call check result.success and result.error (or missing filePath/filename) to log and notify the user when the IPC returns an error instead of silently doing nothing; update the block around reader.onload, reader.onerror, and the saveClipboardImage call (referencing reader, reader.onload, reader.onerror, and window.electronAPI.vault.saveClipboardImage and insertTextAtCursor) to implement these checks and notifications.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@electron/handlers/vault-handlers.ts`:
- Around line 361-364: The code opens a file descriptor (fd) with fs.openSync
and then calls fs.readSync, but if fs.readSync throws the fd will not be closed;
wrap the open/read/close sequence in a try/finally: call
fs.openSync(resolvedPath, 'r') before the try, perform fs.readSync(...) inside
the try, and always call fs.closeSync(fd) in the finally block (or only if fd is
defined) to guarantee the descriptor is closed even on errors; reference the
variables fd, resolvedPath, buffer, stats.size and the functions
fs.openSync/fs.readSync/fs.closeSync when making the change.
- Around line 390-418: The handler for 'vault:saveClipboardImage' accepts
params.targetDir from the renderer and must validate it before writing; update
the ipcMain.handle implementation to resolve params.targetDir with path.resolve
and ensure the resulting saveDir is inside an allowed base (e.g., VAULT_DIR or
path.join(VAULT_DIR, 'attachments')) by checking path.relative(allowedBase,
resolvedDir) does not start with '..' and that resolvedDir begins with the
allowed base; if the check fails, return an error and do not write files; keep
the existing behavior for creating directories (fs.mkdirSync) and writing via
fs.writeFileSync when validation passes, referencing the same symbols
(params.targetDir, saveDir, VAULT_DIR, filename).
In `@electron/main.ts`:
- Around line 397-410: If initVaultDb() throws, the app currently logs the error
but still calls registerVaultHandlers(), which leads to runtime failures when
handlers call getVaultDb(); change the flow so that registerVaultHandlers({
getMainWindow }) is only invoked when initVaultDb() succeeds (i.e., wrap
registerVaultHandlers behind the successful initVaultDb path or return a
boolean/throw up on failure), and also set or expose a simple health flag (e.g.,
vaultInitialized = true/false) or a getVaultHealth() API so the UI can detect
vault unavailability instead of assuming handlers are functional; reference
initVaultDb, registerVaultHandlers, and getVaultDb when making the change.
In `@src/components/TerminalsView/components/FileEditorPanel.tsx`:
- Line 61: The current dirPath calculation (const dirPath =
filePath.split('/').slice(0, -1).join('/')) is not cross-platform; normalize and
derive the directory using the Node path utilities instead. Replace that
splitting logic in FileEditorPanel (where dirPath is computed) with
path.dirname(filePath) after importing path from 'path' (or, if running in a
browser environment without Node path, normalize by replacing backslashes with
forward slashes before splitting). This ensures Windows backslashes are handled
correctly and yields a correct directory path across platforms.
- Around line 73-75: The insertion logic uses the captured content variable and
can clobber concurrent user edits; change setContent to use a functional updater
so it computes the new text from the latest state: replace
setContent(newContent) with setContent(prev => prev.slice(0, start) + mdImage +
prev.slice(end)); keep setSaved(false) as-is. Apply the same pattern in the
analogous handler in DocumentEditor.tsx (handleTextareaPaste).
In `@src/components/TerminalsView/index.tsx`:
- Around line 280-295: handleSaveFile currently only logs errors to the console
when window.electronAPI.vault.writeLocalFile fails; update handleSaveFile to
surface failures to the user by invoking the app's user-facing
notification/toast function (e.g., showToast/notifyError) in both the
!result.success branch and the catch block, include a clear message that the
save failed and the error text (use file.filePath and err/result.error), and
keep the existing console.error for debugging; reference the handleSaveFile
function and window.electronAPI.vault.writeLocalFile call when applying the
change.
In `@src/components/VaultView/components/DocumentEditor.tsx`:
- Around line 248-250: The code derives targetDir by splitting
localFile.filePath on '/' which breaks on Windows; change the logic so targetDir
is computed using a cross-platform dirname function (e.g., Node's path.dirname)
instead of manual splitting: when isLocalFile && localFile, call
path.dirname(localFile.filePath) (invoke via an IPC helper if this renderer
can't import Node's path) or, if you must remain pure-frontend, normalize
separators first (replace backslashes with slashes) before extracting the
directory from localFile.filePath; update the targetDir assignment accordingly
to use path.dirname(localFile.filePath) or the normalized fallback.
---
Nitpick comments:
In `@electron/handlers/vault-handlers.ts`:
- Around line 378-388: The IPC handler for 'vault:writeLocalFile' (and similarly
saveClipboardImage) accepts arbitrary renderer-supplied paths; add
defense-in-depth by validating and normalizing paths in the main process:
resolve and realpath the incoming params.filePath (use
path.resolve/path.normalize and fs.realpathSync) and reject any path that is not
explicitly present in a maintained allowlist/Set of previously-opened real paths
(populate that allowlist when files are opened/selected earlier), returning {
success: false, error: 'invalid path' } if the check fails; also ensure
traversal is prevented by comparing the realpath against allowed entries and log
rejected attempts for auditing.
In `@src/components/TerminalsView/components/FileEditorPanel.tsx`:
- Around line 183-186: The preview pane in FileEditorPanel currently renders raw
text inside a <pre> (using the content variable) instead of parsing markdown;
update FileEditorPanel to import and use the existing SimpleMarkdown component
(same used by DocumentEditor) to render content in the preview tab so pasted
image/markdown syntax renders correctly; ensure you replace the <pre> block that
references content with SimpleMarkdown (or the wrapper used by DocumentEditor)
and pass the content string and any needed props (e.g., className or image
handlers) so styling and image rendering remain consistent.
- Around line 164-170: The close button in FileEditorPanel calls onClose()
directly and can discard unsaved edits; modify the click handler for the button
that currently calls onClose to first check the component's saved state (the
local saved variable/prop) and if saved === false prompt the user (e.g.,
window.confirm or a modal) and only call onClose when the user confirms;
alternatively change the onClose signature to accept the saved flag (e.g.,
onClose(saved)) and let the parent show the confirmation—update the button's
onClick to implement one of these approaches and reference FileEditorPanel and
the onClose prop when making the change.
In `@src/components/TerminalsView/components/TerminalPanel.tsx`:
- Around line 98-129: The drag listeners added in handleResizeStart
(onMouseMove/onMouseUp) can leak if the component unmounts mid-drag; make the
handlers stable and ensure cleanup on unmount: extract the onMouseMove and
onMouseUp functions into refs (e.g. onMouseMoveRef/onMouseUpRef) or
module-scoped callbacks that handle isDraggingRef, splitContainerRef and
setEditorRatio, have handleResizeStart assign/use those stable handlers when
calling document.addEventListener, and add a useEffect with a cleanup that
removes document listeners using the same handler refs and sets
isDraggingRef.current = false on unmount.
In `@src/components/VaultView/components/DocumentEditor.tsx`:
- Around line 243-261: Add robust error handling around the FileReader and the
IPC call in the clipboard image save flow: attach reader.onerror to handle and
notify on read failures (log error and show a user-facing notification), wrap
the await window.electronAPI.vault.saveClipboardImage call in try/catch to
handle thrown exceptions, and after the call check result.success and
result.error (or missing filePath/filename) to log and notify the user when the
IPC returns an error instead of silently doing nothing; update the block around
reader.onload, reader.onerror, and the saveClipboardImage call (referencing
reader, reader.onload, reader.onerror, and
window.electronAPI.vault.saveClipboardImage and insertTextAtCursor) to implement
these checks and notifications.
In `@src/components/VaultView/index.tsx`:
- Around line 319-322: The code currently calls setLocalFile twice when
result.success is true—first updating the content via setLocalFile(prev => prev
? { ...prev, content: data.content } : null) and then immediately calling
setLocalFile(null), so the intermediate update is wasted; remove the
content-update call and only call setViewMode('list') and setLocalFile(null) (or
alternatively remove the final setLocalFile(null) if you intend to keep the
updated localFile), updating the block around result.success to use only the
intended single setLocalFile call while keeping setViewMode('list').
🪄 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: 523199eb-fa47-4f5c-9a33-15506296273e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
electron/handlers/vault-handlers.tselectron/main.tselectron/preload.tssrc/components/TerminalsView/components/FileEditorPanel.tsxsrc/components/TerminalsView/components/TerminalGrid.tsxsrc/components/TerminalsView/components/TerminalPanel.tsxsrc/components/TerminalsView/components/TerminalPanelHeader.tsxsrc/components/TerminalsView/index.tsxsrc/components/VaultView/components/DocumentEditor.tsxsrc/components/VaultView/index.tsxsrc/types/electron.d.ts
Moved openedFiles from local useState in TerminalsView to a dedicated Zustand store (src/store/openedFiles.ts) so linked files survive when navigating away from the dashboard and coming back.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/TerminalsView/index.tsx (1)
271-285:⚠️ Potential issue | 🟠 MajorSurface save failures to users, not only console logs.
At Line 280 and Line 283, failures are still silent from a user perspective, which can lead to mistaken assumptions that content was saved.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TerminalsView/index.tsx` around lines 271 - 285, The handleSaveFile callback currently only logs write failures to console; update it to surface errors to the user by showing a visible notification or inline UI error when window.electronAPI.vault.writeLocalFile returns success: false or throws. Locate handleSaveFile and after the failure branches (where it currently calls console.error at the result.error check and the catch block) invoke the app's existing user-facing notifier (e.g., showToast, notifyError, or set a component state to render an error) with a descriptive message including the file.filePath and the error text from result.error or caught err so users see save failures immediately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/TerminalsView/index.tsx`:
- Around line 271-286: The save handler handleSaveFile writes the file but
doesn't update the openedFiles store, causing stale content on remount; after a
successful write (when window.electronAPI.vault.writeLocalFile returns
result.success true), update the openedFiles entry for the given agentId with
the new content (and any updated metadata like lastSaved timestamp) so the
editor state is in sync—modify the openedFiles map/object used by the component
(or call the appropriate setter/dispatch that maintains openedFiles) to replace
file.content with the passed content immediately after the successful write.
---
Duplicate comments:
In `@src/components/TerminalsView/index.tsx`:
- Around line 271-285: The handleSaveFile callback currently only logs write
failures to console; update it to surface errors to the user by showing a
visible notification or inline UI error when
window.electronAPI.vault.writeLocalFile returns success: false or throws. Locate
handleSaveFile and after the failure branches (where it currently calls
console.error at the result.error check and the catch block) invoke the app's
existing user-facing notifier (e.g., showToast, notifyError, or set a component
state to render an error) with a descriptive message including the file.filePath
and the error text from result.error or caught err so users see save failures
immediately.
🪄 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: 6bbc9bc0-d53a-48f0-ab7a-2e54df535305
📒 Files selected for processing (2)
src/components/TerminalsView/index.tsxsrc/store/openedFiles.ts
- Fix file descriptor leak in readLocalFile (try/finally around readSync) - Validate targetDir in saveClipboardImage to prevent arbitrary dir writes - Skip vault handler registration if DB init fails - Fix cross-platform path splitting in FileEditorPanel and DocumentEditor - Fix stale closure in FileEditorPanel handlePaste (functional state update) - Alert user on save file failure instead of silently swallowing errors
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
electron/handlers/vault-handlers.ts (1)
407-414:⚠️ Potential issue | 🟠 Major
startsWith(homeDir)is still a boundary-check bypass.A sibling path like
/Users/alice-backupstill passesresolvedTarget.startsWith('/Users/alice'), so this validation can write outside the intended home subtree. Usepath.relative(homeDir, resolvedTarget)and reject../absolute results instead.🔒 Safer directory validation
if (params.targetDir) { const resolvedTarget = path.resolve(params.targetDir); // Only allow targetDir under the user's home directory const homeDir = require('os').homedir(); - if (!resolvedTarget.startsWith(homeDir)) { + const relativeTarget = path.relative(homeDir, resolvedTarget); + if (relativeTarget.startsWith('..') || path.isAbsolute(relativeTarget)) { return { success: false, error: 'Target directory is outside the home directory' }; } saveDir = resolvedTarget; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/handlers/vault-handlers.ts` around lines 407 - 414, The current home-directory check using resolvedTarget.startsWith(homeDir) is unsafe; replace it by computing const rel = path.relative(homeDir, resolvedTarget) and reject if rel === '' || rel.startsWith('..') || path.isAbsolute(rel') (i.e., treat any relative path that goes up or is absolute as outside the home tree). Update the validation around resolvedTarget/homeDir in the vault handlers so only targets with a non-empty rel that do not begin with '..' are accepted, returning the same error object when rejected.src/components/TerminalsView/index.tsx (1)
271-288:⚠️ Potential issue | 🟠 MajorSync the opened-file store after a successful save.
The file is written to disk, but
openedFilesstill keeps the oldcontent. Closing/reopening or remounting the split editor can rehydrate stale text from Zustand.💾 Minimal fix
const handleSaveFile = useCallback(async (agentId: string, content: string) => { if (!isElectron() || !window.electronAPI?.vault) return; const file = openedFiles.get(agentId); if (!file) return; try { const result = await window.electronAPI.vault.writeLocalFile({ filePath: file.filePath, content, }); if (!result.success) { console.error('Failed to save file:', result.error); alert(`Failed to save file: ${result.error || 'Unknown error'}`); + return; } + setOpenedFile(agentId, { ...file, content }); } catch (err) { console.error('Failed to save file:', err); alert(`Failed to save file: ${String(err)}`); } - }, [openedFiles]); + }, [openedFiles, setOpenedFile]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TerminalsView/index.tsx` around lines 271 - 288, handleSaveFile writes the file to disk but does not update the in-memory openedFiles store, causing stale content to persist in the split editor; after a successful write (result.success true) update the openedFiles entry for the given agentId with the new content (the same place where openedFiles.get(agentId) is used) so the Zustand store reflects the saved text; locate the handleSaveFile function and after the successful vault.writeLocalFile call, call the existing setter/update method you use for openedFiles (or replace the map entry) to set file.content = content and persist that change to the store.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@electron/handlers/vault-handlers.ts`:
- Around line 346-386: The vault:readLocalFile and vault:writeLocalFile handlers
currently accept any renderer-supplied path (after path.resolve), enabling
arbitrary file access; instead, enforce that reads/writes only operate on
dialog-approved paths by implementing an allowlist/token mechanism: when the
file picker/dialog grants access, register the resolved path (or a short-lived
access token mapped to that resolved path) in a main-process store (e.g.,
approvedPaths map keyed by token or path); then in
ipcMain.handle('vault:readLocalFile', ...) and
ipcMain.handle('vault:writeLocalFile', ...) validate the incoming filePath
against that store (or require the matching token) and reject any requests not
present or expired, returning an error like "Access denied"—apply the same check
for both handlers and ensure approved entries are removed/expire after use or
timeout.
In `@electron/main.ts`:
- Around line 406-415: The current gating of registerVaultHandlers({
getMainWindow }) behind vaultReady prevents DB-independent IPCs
(vault:readLocalFile, vault:writeLocalFile, vault:saveClipboardImage) from
registering when DB init fails; change the registration so DB-independent
handlers are always registered and only DB-backed document/folder handlers that
call getVaultDb() are guarded by vaultReady. Concretely, split or refactor
registerVaultHandlers into two logical paths (e.g.,
registerVaultLocalFileHandlers and registerVaultDbHandlers, or add an options
flag to registerVaultHandlers) and call the local-file registration
unconditionally while only invoking the DB-backed registration when vaultReady
is true; ensure references to getVaultDb(), vault:readLocalFile,
vault:writeLocalFile, and vault:saveClipboardImage are used to separate the code
paths.
In `@src/components/VaultView/components/DocumentEditor.tsx`:
- Around line 242-259: The async paste flow uses reader.onload awaiting
saveClipboardImage and then calls insertTextAtCursor using a stale snapshot of
editor state; change this to read and use the latest editor/selection via a ref
or functional updater (same pattern used in the split file editor) so the
insertion happens relative to current cursor/selection instead of the pre-await
snapshot. Specifically, update the clipboard handler around reader.onload /
saveClipboardImage to capture a live editorRef or provide insertTextAtCursor
with a callback that resolves current selection/position at insertion time
(rather than relying on variables like isLocalFile, localFile, or any
closure-captured content), ensuring the markdown image string produced from
result.filename/result.filePath is inserted at the current cursor/selection
after the async save completes.
---
Duplicate comments:
In `@electron/handlers/vault-handlers.ts`:
- Around line 407-414: The current home-directory check using
resolvedTarget.startsWith(homeDir) is unsafe; replace it by computing const rel
= path.relative(homeDir, resolvedTarget) and reject if rel === '' ||
rel.startsWith('..') || path.isAbsolute(rel') (i.e., treat any relative path
that goes up or is absolute as outside the home tree). Update the validation
around resolvedTarget/homeDir in the vault handlers so only targets with a
non-empty rel that do not begin with '..' are accepted, returning the same error
object when rejected.
In `@src/components/TerminalsView/index.tsx`:
- Around line 271-288: handleSaveFile writes the file to disk but does not
update the in-memory openedFiles store, causing stale content to persist in the
split editor; after a successful write (result.success true) update the
openedFiles entry for the given agentId with the new content (the same place
where openedFiles.get(agentId) is used) so the Zustand store reflects the saved
text; locate the handleSaveFile function and after the successful
vault.writeLocalFile call, call the existing setter/update method you use for
openedFiles (or replace the map entry) to set file.content = content and persist
that change to the store.
🪄 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: 624c85cd-58ef-4fec-a262-45f5057e63d4
📒 Files selected for processing (5)
electron/handlers/vault-handlers.tselectron/main.tssrc/components/TerminalsView/components/FileEditorPanel.tsxsrc/components/TerminalsView/index.tsxsrc/components/VaultView/components/DocumentEditor.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/TerminalsView/components/FileEditorPanel.tsx
- Constrain readLocalFile/writeLocalFile to user's home directory - Split local-file handlers into registerLocalFileHandlers() so they work even when vault DB init fails - Fix stale closure in DocumentEditor insertTextAtCursor (functional state update)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
electron/handlers/vault-handlers.ts (1)
356-356: Consider hoistingosimport to top level.The
require('os')call is repeated inline in three places. While functional, importingosat the top of the file would be more idiomatic and slightly more efficient.Proposed refactor
import { ipcMain, BrowserWindow } from 'electron'; import * as fs from 'fs'; import * as path from 'path'; +import * as os from 'os'; import { v4 as uuidv4 } from 'uuid';Then replace
require('os').homedir()withos.homedir()in the three locations.Also applies to: 397-397, 426-426
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/handlers/vault-handlers.ts` at line 356, Hoist the OS module import to the top of the file (e.g., add "const os = require('os')" or "import os from 'os'") and replace inline calls of require('os').homedir() with os.homedir() where used (the three occurrences that set homeDir / call homedir in this file). This removes repeated require calls and centralizes the OS import for the functions that reference homeDir.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/VaultView/components/DocumentEditor.tsx`:
- Around line 243-266: Wrap the await
window.electronAPI.vault.saveClipboardImage call inside a try/catch within the
reader.onload handler in DocumentEditor.tsx (where saveClipboardImage,
insertTextAtCursor, isLocalFile and localFile are used); on exception or when
result.success is false, log the error and surface user feedback (e.g., call an
existing showNotification/toast or fall back to console.error) and avoid calling
insertTextAtCursor, and ensure any returned error message from the IPC result is
included in the notification so the user knows the paste failed.
---
Nitpick comments:
In `@electron/handlers/vault-handlers.ts`:
- Line 356: Hoist the OS module import to the top of the file (e.g., add "const
os = require('os')" or "import os from 'os'") and replace inline calls of
require('os').homedir() with os.homedir() where used (the three occurrences that
set homeDir / call homedir in this file). This removes repeated require calls
and centralizes the OS import for the functions that reference homeDir.
🪄 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: fa783c2b-ef84-48ef-a62c-7bfed95e755e
📒 Files selected for processing (4)
electron/handlers/vault-handlers.tselectron/main.tssrc/components/TerminalsView/index.tsxsrc/components/VaultView/components/DocumentEditor.tsx
Comment on lines
+243
to
+266
| const reader = new FileReader(); | ||
| reader.onload = async () => { | ||
| const dataUrl = reader.result as string; | ||
| if (!window.electronAPI?.vault?.saveClipboardImage) return; | ||
|
|
||
| // For local files, save to same directory; for vault docs, use vault attachments | ||
| const targetDir = isLocalFile && localFile | ||
| ? localFile.filePath.replace(/\\/g, '/').split('/').slice(0, -1).join('/') | ||
| : undefined; | ||
|
|
||
| const result = await window.electronAPI.vault.saveClipboardImage({ | ||
| imageDataUrl: dataUrl, | ||
| targetDir: targetDir || undefined, | ||
| }); | ||
|
|
||
| if (result.success && result.filePath && result.filename) { | ||
| insertTextAtCursor(``); | ||
| } | ||
| }; | ||
| reader.readAsDataURL(blob); | ||
| return; | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add error handling for clipboard image save failures.
The async paste flow lacks a try/catch around saveClipboardImage, and silent failures provide no user feedback. If the IPC call throws or returns an error, the user won't know the paste failed.
Proposed fix
reader.onload = async () => {
const dataUrl = reader.result as string;
if (!window.electronAPI?.vault?.saveClipboardImage) return;
// For local files, save to same directory; for vault docs, use vault attachments
const targetDir = isLocalFile && localFile
? localFile.filePath.replace(/\\/g, '/').split('/').slice(0, -1).join('/')
: undefined;
+ try {
const result = await window.electronAPI.vault.saveClipboardImage({
imageDataUrl: dataUrl,
targetDir: targetDir || undefined,
});
if (result.success && result.filePath && result.filename) {
insertTextAtCursor(``);
+ } else if (result.error) {
+ console.error('Failed to save pasted image:', result.error);
}
+ } catch (err) {
+ console.error('Failed to save pasted image:', err);
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const reader = new FileReader(); | |
| reader.onload = async () => { | |
| const dataUrl = reader.result as string; | |
| if (!window.electronAPI?.vault?.saveClipboardImage) return; | |
| // For local files, save to same directory; for vault docs, use vault attachments | |
| const targetDir = isLocalFile && localFile | |
| ? localFile.filePath.replace(/\\/g, '/').split('/').slice(0, -1).join('/') | |
| : undefined; | |
| const result = await window.electronAPI.vault.saveClipboardImage({ | |
| imageDataUrl: dataUrl, | |
| targetDir: targetDir || undefined, | |
| }); | |
| if (result.success && result.filePath && result.filename) { | |
| insertTextAtCursor(``); | |
| } | |
| }; | |
| reader.readAsDataURL(blob); | |
| return; | |
| } | |
| } | |
| }; | |
| const reader = new FileReader(); | |
| reader.onload = async () => { | |
| const dataUrl = reader.result as string; | |
| if (!window.electronAPI?.vault?.saveClipboardImage) return; | |
| // For local files, save to same directory; for vault docs, use vault attachments | |
| const targetDir = isLocalFile && localFile | |
| ? localFile.filePath.replace(/\\/g, '/').split('/').slice(0, -1).join('/') | |
| : undefined; | |
| try { | |
| const result = await window.electronAPI.vault.saveClipboardImage({ | |
| imageDataUrl: dataUrl, | |
| targetDir: targetDir || undefined, | |
| }); | |
| if (result.success && result.filePath && result.filename) { | |
| insertTextAtCursor(``); | |
| } else if (result.error) { | |
| console.error('Failed to save pasted image:', result.error); | |
| } | |
| } catch (err) { | |
| console.error('Failed to save pasted image:', err); | |
| } | |
| }; | |
| reader.readAsDataURL(blob); | |
| return; | |
| } | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/VaultView/components/DocumentEditor.tsx` around lines 243 -
266, Wrap the await window.electronAPI.vault.saveClipboardImage call inside a
try/catch within the reader.onload handler in DocumentEditor.tsx (where
saveClipboardImage, insertTextAtCursor, isLocalFile and localFile are used); on
exception or when result.success is false, log the error and surface user
feedback (e.g., call an existing showNotification/toast or fall back to
console.error) and avoid calling insertTextAtCursor, and ensure any returned
error message from the IPC result is included in the notification so the user
knows the paste failed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit