workspace renaming with path update#7437
Merged
bijin-bruno merged 4 commits intousebruno:mainfrom Mar 11, 2026
Merged
Conversation
naman-bruno
requested review from
bijin-bruno,
helloanoop and
lohit-bruno
as code owners
Contributor
WalkthroughAdds path-aware workspace renaming: renderer dispatches Changes
Sequence DiagramsequenceDiagram
participant Redux as Redux Action (renderer)
participant IPC as IPC (main)
participant WsConfig as workspace-config (main util)
participant FS as File System
Redux->>IPC: renderer:rename-workspace(path, newName)
IPC->>WsConfig: renameWorkspace(path, newName)
WsConfig->>FS: check target folder exists / sanitize name
alt conflict detected
WsConfig-->>IPC: throw/error
else target available
WsConfig->>FS: rename folder (if name changes)
WsConfig->>WsConfig: update workspace.yml (updateWorkspaceName)
alt folder moved
WsConfig-->>IPC: { newWorkspacePath }
else no path change
WsConfig-->>IPC: { newWorkspacePath: null }
end
end
alt newWorkspacePath returned
IPC->>IPC: update file watcher for new path
IPC->>IPC: refresh lastOpenedWorkspaces entries
IPC-->>Redux: { newWorkspacePath }
Redux->>Redux: remove old workspace\ncreate new workspace (new UID/path/name)\nreactivate if old was active
else in-place rename
IPC-->>Redux: { success: true }
Redux->>Redux: update workspace name in place
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/bruno-electron/src/ipc/workspace.js (1)
271-291: Handler exposes partial failure risk.The sequence of operations (rename → watcher update → lastOpenedWorkspaces update) isn't transactional. If the app crashes after
renameWorkspacesucceeds but beforelastOpenedWorkspaces.add()completes, the persisted store will reference the old (now non-existent) path.The existing
renderer:get-last-opened-workspaceshandler (lines 245-268) does prune invalid paths on startup, which mitigates this for the next session. This is acceptable for a desktop app but worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/src/ipc/workspace.js` around lines 271 - 291, After renameWorkspace succeeds, persist the updated lastOpenedWorkspaces immediately (call lastOpenedWorkspaces.remove(workspacePath) and lastOpenedWorkspaces.add(result.newWorkspacePath)) before touching workspaceWatcher so the stored paths are updated even if the app crashes; then perform watcher updates inside a try/catch (wrap workspaceWatcher.removeWatcher and workspaceWatcher.add) and log but do not rethrow watcher errors; only return { success: true, newWorkspacePath } after the store update succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js`:
- Around line 537-538: The client currently always uses
generateUidBasedOnHash(result.newWorkspacePath) to create newWorkspaceUid, which
diverges from Electron's getWorkspaceUid behavior; change the logic in the
workspace rename/create flow (where newWorkspaceUid is assigned in actions.js)
to mirror Electron: if result.newWorkspacePath equals the default workspace
path, call defaultWorkspaceManager.getDefaultWorkspaceUid() (or the equivalent
helper that returns the default UID) and use that UID; otherwise fall back to
generateUidBasedOnHash(result.newWorkspacePath). Ensure you reference and use
the same default workspace-path check used by getWorkspaceUid so both sides
produce identical UIDs.
In `@packages/bruno-electron/src/utils/workspace-config.js`:
- Around line 584-585: The rename of the workspace folder
(fs.renameSync(workspacePath, newWorkspacePath)) is not rolled back if
updateWorkspaceName(newWorkspacePath, newName) throws; wrap the YAML update in a
try/catch so that on failure you attempt to restore the original folder name
(fs.renameSync(newWorkspacePath, workspacePath)) and then rethrow or propagate
the error. Ensure you catch and handle any error from the rollback attempt too
(log or aggregate) so failures during rollback don't silence the original error;
reference fs.renameSync, workspacePath, newWorkspacePath, and
updateWorkspaceName to locate and implement the change.
In `@packages/bruno-electron/tests/utils/workspace-config.spec.js`:
- Around line 152-163: The test assumes a case-insensitive filesystem; detect
filesystem behavior at runtime and adjust the assertions accordingly: add a
small helper (inline in the spec) that uses fs to create a temp dir/file via
createWorkspace or fs.mkdtemp and checks whether a differently-cased path refers
to the same inode (or whether fs.existsSync with changed case returns true) to
determine case-sensitivity, then in the test for renameWorkspace use that helper
to either expect result.newWorkspacePath toBeNull and
getWorkspaceName(lowerPath) toBe('MyWorkspace') on case-insensitive systems, or
expect result.newWorkspacePath toEqual the actual renamed path (and verify the
folder rename) on case-sensitive systems; reference the existing
createWorkspace, renameWorkspace, and getWorkspaceName symbols to locate where
to update the test.
---
Nitpick comments:
In `@packages/bruno-electron/src/ipc/workspace.js`:
- Around line 271-291: After renameWorkspace succeeds, persist the updated
lastOpenedWorkspaces immediately (call
lastOpenedWorkspaces.remove(workspacePath) and
lastOpenedWorkspaces.add(result.newWorkspacePath)) before touching
workspaceWatcher so the stored paths are updated even if the app crashes; then
perform watcher updates inside a try/catch (wrap workspaceWatcher.removeWatcher
and workspaceWatcher.add) and log but do not rethrow watcher errors; only return
{ success: true, newWorkspacePath } after the store update succeeds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b96db8a6-7545-42c9-8aa9-7493540b7277
📒 Files selected for processing (4)
packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.jspackages/bruno-electron/src/ipc/workspace.jspackages/bruno-electron/src/utils/workspace-config.jspackages/bruno-electron/tests/utils/workspace-config.spec.js
packages/bruno-app/src/providers/ReduxStore/slices/workspaces/actions.js
Show resolved
Hide resolved
Contributor
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 `@packages/bruno-electron/tests/utils/workspace-config.spec.js`:
- Around line 124-130: Update the test so it proves the config write actually
happened by seeding a workspace whose folder basename already equals the
sanitized target but whose workspace.yml info.name is different; call
renameWorkspace(workspacePath, 'Untitled Workspace'), assert
result.newWorkspacePath is null and the folder still exists, and then verify the
workspace.yml (via getWorkspaceName(workspacePath)) now contains 'Untitled
Workspace' (i.e. change the initial info.name to something else before calling
renameWorkspace so the assertion fails if the config-write step is skipped).
- Around line 176-202: The test should assert that renameWorkspace preserves all
fields that renameWorkspace rewrites in workspace.yml, not just info.name, one
collection and docs; update the spec in workspace-config.spec.js to load the
parsed config and add assertions for other top-level keys that could be
rewritten (e.g., opencollection, info.type, specs, activeEnvironmentUid if
present, full collections entries including path and any extra collection
fields, and any other metadata expected to remain unchanged) while still
checking that info.name equals 'My Project'—use the existing renameWorkspace
result and newConfigPath to locate the file and verify each field that
renameWorkspace is responsible for preserving.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3aca6511-027f-4533-a29d-9d5ccbe467fb
📒 Files selected for processing (2)
packages/bruno-electron/src/utils/workspace-config.jspackages/bruno-electron/tests/utils/workspace-config.spec.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bruno-electron/src/utils/workspace-config.js
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/bruno-electron/tests/utils/workspace-config.spec.js (1)
156-167:⚠️ Potential issue | 🟠 MajorMake this case-only rename test filesystem-aware.
This assertion is still OS-dependent: on case-sensitive filesystems,
myworkspaceandMyWorkspaceare different paths, sorenameWorkspace()can legitimately return a new path instead ofnull. Please branch the expectation based on detected filesystem case-sensitivity, or skip this scenario where that guarantee does not hold.Suggested adjustment
test('handles case-only rename by updating config without renaming folder', async () => { const lowerPath = createWorkspace('myworkspace', 'myworkspace'); const result = await renameWorkspace(lowerPath, 'MyWorkspace'); - expect(result.newWorkspacePath).toBeNull(); - expect(fs.existsSync(lowerPath)).toBe(true); - expect(getWorkspaceName(lowerPath)).toBe('MyWorkspace'); + const probe = path.join(parentDir, 'CaseProbe'); + fs.mkdirSync(probe); + const isCaseInsensitive = fs.existsSync(path.join(parentDir, 'caseprobe')); + fs.rmSync(probe, { recursive: true, force: true }); + + if (isCaseInsensitive) { + expect(result.newWorkspacePath).toBeNull(); + expect(fs.existsSync(lowerPath)).toBe(true); + expect(getWorkspaceName(lowerPath)).toBe('MyWorkspace'); + return; + } + + const renamedPath = path.join(parentDir, 'MyWorkspace'); + expect(result.newWorkspacePath).toBe(renamedPath); + expect(fs.existsSync(renamedPath)).toBe(true); + expect(getWorkspaceName(renamedPath)).toBe('MyWorkspace'); });As per coding guidelines, "Ensure tests are deterministic and reproducible. No randomness, timing dependencies, or environment-specific assumptions without explicit control" and "
**/*: Bruno is a cross-platform Electron desktop app that runs on macOS, Windows, and Linux. Ensure that all code is OS-agnostic".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-electron/tests/utils/workspace-config.spec.js` around lines 156 - 167, The test 'handles case-only rename by updating config without renaming folder' assumes case-insensitive filesystems; make it filesystem-aware by detecting whether the fs is case-sensitive (e.g., create a temp file/folder with a mixed-case duplicate and check if paths are distinct) before asserting expectations for renameWorkspace; if filesystem is case-insensitive assert result.newWorkspacePath === null and that fs.existsSync(lowerPath) is true and getWorkspaceName(lowerPath) === 'MyWorkspace', otherwise assert result.newWorkspacePath equals the new path returned (not null) and that both the old and new paths exist and getWorkspaceName(result.newWorkspacePath) === 'MyWorkspace' (or skip the case-only branch when detection indicates sensitivity). Use the existing helpers createWorkspace, renameWorkspace, getWorkspaceName and fs.existsSync to implement this branching logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/bruno-electron/tests/utils/workspace-config.spec.js`:
- Around line 156-167: The test 'handles case-only rename by updating config
without renaming folder' assumes case-insensitive filesystems; make it
filesystem-aware by detecting whether the fs is case-sensitive (e.g., create a
temp file/folder with a mixed-case duplicate and check if paths are distinct)
before asserting expectations for renameWorkspace; if filesystem is
case-insensitive assert result.newWorkspacePath === null and that
fs.existsSync(lowerPath) is true and getWorkspaceName(lowerPath) ===
'MyWorkspace', otherwise assert result.newWorkspacePath equals the new path
returned (not null) and that both the old and new paths exist and
getWorkspaceName(result.newWorkspacePath) === 'MyWorkspace' (or skip the
case-only branch when detection indicates sensitivity). Use the existing helpers
createWorkspace, renameWorkspace, getWorkspaceName and fs.existsSync to
implement this branching logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba54065c-8799-40f1-a779-7864ee31f2f8
📒 Files selected for processing (1)
packages/bruno-electron/tests/utils/workspace-config.spec.js
bijin-bruno
approved these changes
Mar 11, 2026
naman-bruno
added a commit
that referenced
this pull request
Mar 12, 2026
This reverts commit e7c2c7c.
bijin-bruno
pushed a commit
that referenced
this pull request
Mar 12, 2026
6 tasks
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
2 participants
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.
Description
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
New Features
Tests