fix: expand tilde in configDir before fs calls when adding account#1892
fix: expand tilde in configDir before fs calls when adding account#1892brandon-claps wants to merge 1 commit intoAndyMik90:developfrom
Conversation
Node.js fs functions (existsSync, mkdirSync) do not perform shell tilde
expansion. When the renderer sends configDir as "~/.claude-profiles/name",
the main process was passing the unexpanded path directly to existsSync and
mkdirSync, causing an EACCES or ENOENT error. The catch block returned
{ success: false } with no visible feedback in the UI, making the Add
button appear to do nothing.
Fix: expand the tilde via expandHomePath() before the fs calls and update
profile.configDir to the resolved path so saveProfile receives the
absolute path consistently.
Also add an else branch in AccountSettings.tsx for when saveClaudeProfile
returns { success: false }, so the error toast is shown to the user
instead of silently swallowing the failure.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Brandon Claps seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Summary of ChangesHello @brandon-claps, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses two critical issues related to adding new Claude profiles: incorrect handling of home directory paths and a lack of user feedback on failure. It ensures that profile configuration directories are correctly created by explicitly expanding tilde paths before file system operations and provides immediate visual feedback to the user if the profile creation process encounters an error. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughEnhances Claude profile creation by expanding home directory paths in configuration directory handling and adding explicit error feedback UI when profile saves fail. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a critical bug where tilde paths in configDir were not expanded before fs calls, leading to EACCES/ENOENT errors or incorrect directory creation. It also improves user feedback by displaying a destructive toast message when adding a Claude profile fails. The changes correctly expand the home path and ensure error messages are surfaced to the user, enhancing both robustness and user experience.
| import { debugLog, } from '../../shared/utils/debug-logger'; | ||
| import { migrateSession } from '../claude-profile/session-utils'; | ||
| import { createProfileDirectory } from '../claude-profile/profile-utils'; | ||
| import { createProfileDirectory, expandHomePath } from '../claude-profile/profile-utils'; |
There was a problem hiding this comment.
The expandHomePath utility is correctly imported here. This is a good practice to centralize path manipulation logic.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/frontend/src/main/ipc-handlers/terminal-handlers.ts`:
- Around line 164-171: The handler currently uses synchronous fs functions
(existsSync and mkdirSync) which block the Electron main process and also
performs a redundant TOCTOU check; replace the sync calls by importing and
calling the promise-based mkdir from 'fs/promises' and await it (use await
mkdir(resolvedConfigDir, { recursive: true })); remove the existsSync guard
entirely, keep the expandHomePath call and assignment to profile.configDir, and
ensure any import reference to mkdirSync/existsSync is removed or replaced so
only the async mkdir is used.
- Line 168: Replace the dynamic in-function import of fs with a top-level static
import: add a top-level import that brings in mkdirSync and existsSync from the
built-in fs module, then remove the inline "const { mkdirSync, existsSync } =
await import('fs');" statement in terminal-handlers.ts (where that const
appears) so the code uses the top-level symbols directly (update any references
in the surrounding function if needed).
| // Ensure config directory exists (expand ~ before passing to Node.js fs functions, | ||
| // which do not perform shell tilde expansion) | ||
| const resolvedConfigDir = expandHomePath(profile.configDir); | ||
| profile.configDir = resolvedConfigDir; | ||
| const { mkdirSync, existsSync } = await import('fs'); | ||
| if (!existsSync(profile.configDir)) { | ||
| mkdirSync(profile.configDir, { recursive: true }); | ||
| if (!existsSync(resolvedConfigDir)) { | ||
| mkdirSync(resolvedConfigDir, { recursive: true }); | ||
| } |
There was a problem hiding this comment.
Synchronous existsSync/mkdirSync blocks the Electron main process; existsSync check is also a redundant TOCTOU
This is an async IPC handler; mkdirSync and existsSync are synchronous and will block the main process event loop during any I/O wait. The existsSync check before mkdirSync({ recursive: true }) is also a TOCTOU race — mkdirSync with recursive: true is already idempotent (silently succeeds when the directory exists), making the guard redundant.
Prefer fs/promises.mkdir:
⚡ Proposed fix: use async mkdir
- const { mkdirSync, existsSync } = await import('fs');
- if (!existsSync(profile.configDir)) {
- mkdirSync(profile.configDir, { recursive: true });
- }
+ const { mkdir } = await import('fs/promises');
+ await mkdir(profile.configDir, { recursive: true });📝 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.
| // Ensure config directory exists (expand ~ before passing to Node.js fs functions, | |
| // which do not perform shell tilde expansion) | |
| const resolvedConfigDir = expandHomePath(profile.configDir); | |
| profile.configDir = resolvedConfigDir; | |
| const { mkdirSync, existsSync } = await import('fs'); | |
| if (!existsSync(profile.configDir)) { | |
| mkdirSync(profile.configDir, { recursive: true }); | |
| if (!existsSync(resolvedConfigDir)) { | |
| mkdirSync(resolvedConfigDir, { recursive: true }); | |
| } | |
| // Ensure config directory exists (expand ~ before passing to Node.js fs functions, | |
| // which do not perform shell tilde expansion) | |
| const resolvedConfigDir = expandHomePath(profile.configDir); | |
| profile.configDir = resolvedConfigDir; | |
| const { mkdir } = await import('fs/promises'); | |
| await mkdir(resolvedConfigDir, { recursive: true }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/frontend/src/main/ipc-handlers/terminal-handlers.ts` around lines 164 -
171, The handler currently uses synchronous fs functions (existsSync and
mkdirSync) which block the Electron main process and also performs a redundant
TOCTOU check; replace the sync calls by importing and calling the promise-based
mkdir from 'fs/promises' and await it (use await mkdir(resolvedConfigDir, {
recursive: true })); remove the existsSync guard entirely, keep the
expandHomePath call and assignment to profile.configDir, and ensure any import
reference to mkdirSync/existsSync is removed or replaced so only the async mkdir
is used.
| // which do not perform shell tilde expansion) | ||
| const resolvedConfigDir = expandHomePath(profile.configDir); | ||
| profile.configDir = resolvedConfigDir; | ||
| const { mkdirSync, existsSync } = await import('fs'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Replace dynamic import('fs') with a static top-level import
fs is a Node.js built-in always present in the Electron main process; a dynamic await import('fs') adds unnecessary overhead on every call and obscures the module's dependencies.
♻️ Proposed refactor
At the top of the file, add:
+import { existsSync, mkdirSync } from 'fs';Then remove the inline dynamic import:
- const { mkdirSync, existsSync } = await import('fs');📝 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.
| const { mkdirSync, existsSync } = await import('fs'); | |
| import { existsSync, mkdirSync } from 'fs'; | |
| // ... rest of file ... | |
| // Dynamic import removed - fs is now imported at the top |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/frontend/src/main/ipc-handlers/terminal-handlers.ts` at line 168,
Replace the dynamic in-function import of fs with a top-level static import: add
a top-level import that brings in mkdirSync and existsSync from the built-in fs
module, then remove the inline "const { mkdirSync, existsSync } = await
import('fs');" statement in terminal-handlers.ts (where that const appears) so
the code uses the top-level symbols directly (update any references in the
surrounding function if needed).
3 participants
Summary
existsSync/mkdirSyncin theCLAUDE_PROFILE_SAVEIPC handler were called with the unexpanded tilde path (e.g.~/.claude-profiles/name). Node.jsfsfunctions do not perform shell tilde expansion, so this either throwsEACCES/ENOENTor creates a directory at the wrong literal path.{ success: false }but the renderer had noelsebranch to surface the error, so clicking Add appeared to do nothing with zero feedback.Changes
apps/frontend/src/main/ipc-handlers/terminal-handlers.tsexpandHomePathfromprofile-utils(already exported)profile.configDirviaexpandHomePath()before passing toexistsSync/mkdirSyncprofile.configDirto the resolved absolute path sosaveProfilereceives a consistent valueapps/frontend/src/renderer/components/settings/AccountSettings.tsxelse if (!result.success)branch inhandleAddClaudeProfileto show a destructive toast whensaveClaudeProfilereturns{ success: false }, so errors are surfaced instead of swallowed silentlyTest plan
~/.claude-profiles/<name>/directory is created at the correct absolute path🤖 Generated with Claude Code
Summary by CodeRabbit