feature: open in terminal from manage workspace#7877
Open
prateek-bruno wants to merge 7 commits intousebruno:mainfrom
Open
feature: open in terminal from manage workspace#7877prateek-bruno wants to merge 7 commits intousebruno:mainfrom
prateek-bruno wants to merge 7 commits intousebruno:mainfrom
Conversation
prateek-bruno
requested review from
bijin-bruno,
helloanoop,
lohit-bruno,
naman-bruno and
sid-bruno
as code owners
Contributor
WalkthroughAdds an "Open in Terminal" action to ManageWorkspace's actions menu that calls Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "ManageWorkspace UI"
participant Dispatch as "App Dispatch"
participant Devtools as "Devtools / Terminal"
UI->>Dispatch: openDevtoolsAndSwitchToTerminal(dispatch, workspace.pathname)
Dispatch->>Devtools: open/focus devtools and start terminal at workspace.pathname
Devtools-->>UI: terminal session visible with workspace path
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.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: 2
🧹 Nitpick comments (1)
packages/bruno-app/src/components/ManageWorkspace/index.js (1)
173-175: Add a stable test id to the new trigger.The new Playwright flow still has to target
.more-actions-btn, which is brittle. Add adata-testidhere so the E2E spec can use a stable locator instead of a styling class. As per coding guidelines, Add data-testid to testable elements for Playwright.🧪 Proposed fix
- <button className="more-actions-btn"> + <button + className="more-actions-btn" + data-testid="workspace-actions-button" + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/ManageWorkspace/index.js` around lines 173 - 175, Add a stable test id to the new actions trigger button so Playwright can target it instead of the styling class; update the button element rendered in ManageWorkspace (the JSX that currently uses className="more-actions-btn" and IconDots) to include a data-testid attribute (e.g., data-testid="workspace-more-actions-btn") and ensure the prop is added to that button element so E2E tests can use this stable locator.
🤖 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/components/ManageWorkspace/index.js`:
- Around line 158-170: The actions menu is being shown for the default workspace
when workspace.pathname exists; change the outer conditional so MenuDropdown is
only rendered when !isDefault, and inside MenuDropdown keep the terminal action
gated by workspace.pathname (i.e., render MenuDropdown only when !isDefault, and
inside its items include open-in-terminal via openDevtoolsAndSwitchToTerminal
only if workspace.pathname is truthy while still including rename/remove via
handleRenameClick and handleCloseClick).
In `@tests/workspace/manage-workspace/manage-workspace.spec.ts`:
- Around line 19-58: The test body must ensure the Electron app is always torn
down: wrap the sequence after creating app (the call to launchElectronApp) in a
try/finally, moving the existing closeElectronApp(app) into the finally block;
ensure app is checked for truthiness before calling closeElectronApp to avoid
calling it if launchElectronApp failed. This impacts the test function that
calls launchElectronApp and closeElectronApp (the "should open terminal from the
workspace actions menu" test), so locate that test, introduce try { ... }
finally { if (app) await closeElectronApp(app); } around the steps to guarantee
cleanup.
---
Nitpick comments:
In `@packages/bruno-app/src/components/ManageWorkspace/index.js`:
- Around line 173-175: Add a stable test id to the new actions trigger button so
Playwright can target it instead of the styling class; update the button element
rendered in ManageWorkspace (the JSX that currently uses
className="more-actions-btn" and IconDots) to include a data-testid attribute
(e.g., data-testid="workspace-more-actions-btn") and ensure the prop is added to
that button element so E2E tests can use this stable locator.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 17427256-b02c-4021-b4d1-6099d44971f6
📒 Files selected for processing (2)
packages/bruno-app/src/components/ManageWorkspace/index.jstests/workspace/manage-workspace/manage-workspace.spec.ts
…rminal' for non default workspace
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-app/src/components/ManageWorkspace/index.js (1)
157-163:⚠️ Potential issue | 🟡 MinorOnly show “Open in Terminal” when the workspace has a path.
Right now this item still renders for non-default workspaces with no
pathname, which makes it open an unscoped terminal instead of a workspace-scoped one. Gate the entry onworkspace.pathnameso the action only appears when it can actually open the workspace folder.🔧 Suggested fix
{!isDefault && ( <MenuDropdown placement="bottom-end" items={[ - { id: 'open-in-terminal', label: 'Open in Terminal', onClick: () => openDevtoolsAndSwitchToTerminal(dispatch, workspace.pathname) }, + ...(workspace.pathname + ? [{ id: 'open-in-terminal', label: 'Open in Terminal', onClick: () => openDevtoolsAndSwitchToTerminal(dispatch, workspace.pathname) }] + : []), { id: 'rename', label: 'Rename', onClick: () => handleRenameClick(workspace) }, { id: 'remove', label: 'Remove', onClick: () => handleCloseClick(workspace) } ]} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/ManageWorkspace/index.js` around lines 157 - 163, The "Open in Terminal" menu item is shown for non-default workspaces even when workspace.pathname is falsy, causing an unscoped terminal to open; update the MenuDropdown items array (in the JSX where MenuDropdown is rendered) to include the "Open in Terminal" entry only when workspace.pathname is truthy—i.e. gate the item that calls openDevtoolsAndSwitchToTerminal(dispatch, workspace.pathname) on workspace.pathname so it only appears when a workspace path exists, leaving rename (handleRenameClick) and remove (handleCloseClick) items unchanged.
♻️ Duplicate comments (1)
tests/workspace/manage-workspace/manage-workspace.spec.ts (1)
22-26:⚠️ Potential issue | 🟡 MinorKeep
firstWindow()inside the cleanup guard.The
finallyblock only protects the code afterfirstWindow()succeeds. If window creation fails,closeElectronApp(app)is skipped and the Electron app can leak into later specs.🧹 Suggested fix
const app = await launchElectronApp({ initUserDataPath, templateVars: { wsLocation } }); - const page = await app.firstWindow(); - try { + const page = await app.firstWindow(); await page.locator('[data-app-state="loaded"]').waitFor({ timeout: 30000 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/workspace/manage-workspace/manage-workspace.spec.ts` around lines 22 - 26, The call to firstWindow() is outside the try/finally so if window creation throws the finally doesn't run and the Electron process (app) can leak; to fix, ensure the Electron app is always cleaned up by moving the call to firstWindow() inside the try block (or guard in finally) and ensure closeElectronApp(app) runs only if app is defined — update the flow around launchElectronApp, firstWindow, and closeElectronApp so app is created, firstWindow is awaited inside the try, and the finally closes app when non-null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/bruno-app/src/components/ManageWorkspace/index.js`:
- Around line 157-163: The "Open in Terminal" menu item is shown for non-default
workspaces even when workspace.pathname is falsy, causing an unscoped terminal
to open; update the MenuDropdown items array (in the JSX where MenuDropdown is
rendered) to include the "Open in Terminal" entry only when workspace.pathname
is truthy—i.e. gate the item that calls
openDevtoolsAndSwitchToTerminal(dispatch, workspace.pathname) on
workspace.pathname so it only appears when a workspace path exists, leaving
rename (handleRenameClick) and remove (handleCloseClick) items unchanged.
---
Duplicate comments:
In `@tests/workspace/manage-workspace/manage-workspace.spec.ts`:
- Around line 22-26: The call to firstWindow() is outside the try/finally so if
window creation throws the finally doesn't run and the Electron process (app)
can leak; to fix, ensure the Electron app is always cleaned up by moving the
call to firstWindow() inside the try block (or guard in finally) and ensure
closeElectronApp(app) runs only if app is defined — update the flow around
launchElectronApp, firstWindow, and closeElectronApp so app is created,
firstWindow is awaited inside the try, and the finally closes app when non-null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5a82f2a-a00b-46d8-9391-41cc6973c187
📒 Files selected for processing (2)
packages/bruno-app/src/components/ManageWorkspace/index.jstests/workspace/manage-workspace/manage-workspace.spec.ts
gopu-bruno
approved these changes
May 7, 2026
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
Jira
Fixes #6770
Adds "Open in Terminal" in "Manage Workspace" for non-default workspaces.
Screen.Recording.2026-04-29.at.2.00.07.PM.mov
Contribution Checklist:
Summary by CodeRabbit
New Features
Improvements
Tests