fix: use automation profile for terminal creation #7893
Conversation
- Add getAutomationShell() function to retrieve automation profile settings - Modify Terminal class to use automation profile when creating terminals - Add comprehensive tests for automation profile functionality - Respects terminal.integrated.automationProfile settings for all platforms Fixes #7892
dosubot
bot
added
size:L
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| const iconPath = new vscode.ThemeIcon("rocket") | ||
| this.terminal = terminal ?? vscode.window.createTerminal({ cwd, name: "Roo Code", iconPath, env }) | ||
|
|
||
| // Try to get the automation shell first, fall back to default behavior if not configured |
There was a problem hiding this comment.
Is this comment accurate? The fallback to default behavior actually happens in VSCode's createTerminal when shellPath is undefined, not in our getAutomationShell() function. Maybe we should clarify this to avoid confusion?
| this.terminal = terminal ?? vscode.window.createTerminal({ cwd, name: "Roo Code", iconPath, env }) | ||
|
|
||
| // Try to get the automation shell first, fall back to default behavior if not configured | ||
| const shellPath = getAutomationShell() || undefined |
There was a problem hiding this comment.
Could we simplify this to just shellPath: getAutomationShell() since getAutomationShell() already returns null? The || undefined conversion seems redundant unless VSCode's TerminalOptions specifically requires undefined over null.
| * This is used for automation tools and should respect the automationProfile settings. | ||
| * Falls back to the default shell if no automation profile is configured. | ||
| */ | ||
| export function getAutomationShell(): string | null { |
There was a problem hiding this comment.
This function works well, but could benefit from more detailed JSDoc comments explaining:
- When it returns null vs a string
- The precedence of automation profile over default profile
- The security validation that happens
This would help other developers understand the function's behavior without reading the implementation.
| } | ||
| }) | ||
|
|
||
| it("uses automation profile shell when configured", () => { |
There was a problem hiding this comment.
Should we add a test case for when getAutomationShell() throws an exception? It would be good to ensure the Terminal constructor handles this gracefully and falls back to default behavior.
| // -------------------------------------------------------------------------- | ||
| // Automation Shell Detection Tests | ||
| // -------------------------------------------------------------------------- | ||
| describe("Automation Shell Detection", () => { |
There was a problem hiding this comment.
The automation shell tests have great coverage! Though for consistency with the main shell detection tests above, we could consider reorganizing these into more structured platform-specific describe blocks. Not critical, but might improve readability.
hannesrudolph
added
the
Issue/PR - Triage
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
|
Closing this PR as the approach doesn't align with the proper solution. The fix should allow users to select a terminal profile from settings rather than automatically using the automation profile. See issue #7892 for the detailed solution. |
github-project-automation
bot
moved this from PR [Needs Prelim Review]
to Done
in Roo Code Roadmap
Description
This PR addresses Issue #7892 by implementing support for VSCode's automation profile settings when creating terminals. Previously, Roo would use the default terminal profile, which could be incompatible with automation tools. Now it respects the
terminal.integrated.automationProfilesettings.Changes
getAutomationShell()function to retrieve automation profile settings for all platforms (Windows, macOS, Linux)Terminalclass constructor to use the automation shell when availableTesting
How it works
When creating a new terminal, Roo will now:
terminal.integrated.automationProfile.{platform})This allows users to configure a different shell for automation tools without changing their default terminal shell.
Fixes #7892
Important
This PR adds support for using VSCode's automation profile settings in terminal creation, ensuring compatibility with automation tools and validating shell paths against an allowlist.
Terminalclass inTerminal.tsnow usesgetAutomationShell()to retrieve and use automation profile settings for terminal creation.getAutomationShell()inshell.tsto retrieve automation profile settings for Windows, macOS, and Linux.getShell()inshell.tsto include automation profile handling.TerminalRegistry.spec.tsandshell.spec.tsto cover automation profile functionality and shell path validation.This description was created by
for 0010aef. You can customize this summary. It will automatically update as commits are pushed.