feat(terminal): add VS Code integrated-terminal shell override#277
Open
proyectoauraorg wants to merge 20 commits into
Open
feat(terminal): add VS Code integrated-terminal shell override#277proyectoauraorg wants to merge 20 commits into
proyectoauraorg wants to merge 20 commits into
Conversation
proyectoauraorg
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a selectable VS Code terminal profile for the inline terminal: schema and message types, profile storage and resolution, extension↔webview plumbing for discovery and persistence, settings UI with profile dropdown and native picker, tests for backend/frontend behavior, and translations across locales. ChangesTerminal profile selection feature
Sequence Diagram(s)sequenceDiagram
participant SettingsView
participant TerminalSettings
participant Extension
participant Terminal
participant VSCodeConfig
SettingsView->>TerminalSettings: pass terminalProfile prop
TerminalSettings->>Extension: postMessage(type: "requestTerminalProfiles")
Extension->>VSCodeConfig: read terminal.integrated.profiles.{windows,osx,linux}
VSCodeConfig-->>Extension: profile lists
Extension-->>TerminalSettings: postMessage(type: "terminalProfiles", profiles)
User->>TerminalSettings: select profile
TerminalSettings->>Extension: setCachedStateField("terminalProfile", name|undefined)
Extension->>Terminal: Terminal.setTerminalProfile(name|undefined)
Terminal->>VSCodeConfig: resolve configured profile -> shellPath/shellArgs/env
Terminal->>Terminal: create inline terminal with resolved options
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![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)
webview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsx (1)
129-135: ⚡ Quick winAdd a save-path regression for “Default” profile clearing
This test stops at
setCachedStateField. It won’t catch serialization drops whenSettingsViewpostsupdateSettings. Add one integration assertion that clicking Save emits a clearable value forterminalProfile(not an omitted field).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsx` around lines 129 - 135, The test currently only asserts setCachedStateField when selecting the "__default__" option but doesn’t verify what SettingsView posts on Save, allowing serialization regressions to omit the field; after firing the existing click on screen.getByTestId("option-__default__"), simulate clicking the Save control (e.g. the Save button used by SettingsView) and add an assertion that the mocked updateSettings/postMessage handler was called with a payload that explicitly includes terminalProfile: null (i.e. a clearable value) rather than omitting terminalProfile, referencing the existing setup and setCachedStateField helpers and the SettingsView updateSettings/postMessage entry point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/integrations/terminal/BaseTerminal.ts`:
- Around line 306-308: The setter currently validates using profile.trim() but
stores the original untrimmed string; update BaseTerminal.setTerminalProfile to
persist a normalized value (use const normalized = profile?.trim();) and set
BaseTerminal.terminalProfile to normalized if normalized.length > 0, otherwise
undefined, so stored profile keys match lookups (reference:
BaseTerminal.setTerminalProfile and BaseTerminal.terminalProfile).
In `@src/integrations/terminal/Terminal.ts`:
- Around line 269-272: Terminal.getProfileShell() currently picks
profile.path[0] without checking filesystem existence; change it to iterate the
array when profile.path is an array and choose the first path that exists (use
fs.existsSync or the project's async existence helper) before passing to
createTerminal, while preserving fallback when no candidate exists; update unit
tests that assume index-0 selection for terminal.integrated.profiles.*.path[] to
assert selection of the first existing candidate.
In `@webview-ui/src/components/settings/SettingsView.tsx`:
- Line 400: When resetting settings to Default in SettingsView, don't leave
terminalProfile as undefined (which gets omitted during serialization);
explicitly assign a clear value such as an empty string (e.g. terminalProfile:
'') or a "Default" sentinel when building the settings object (the code paths
around terminalProfile in SettingsView / the reset handler), so the saved JSON
contains a persistent, clear value instead of omitting the key.
---
Nitpick comments:
In
`@webview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsx`:
- Around line 129-135: The test currently only asserts setCachedStateField when
selecting the "__default__" option but doesn’t verify what SettingsView posts on
Save, allowing serialization regressions to omit the field; after firing the
existing click on screen.getByTestId("option-__default__"), simulate clicking
the Save control (e.g. the Save button used by SettingsView) and add an
assertion that the mocked updateSettings/postMessage handler was called with a
payload that explicitly includes terminalProfile: null (i.e. a clearable value)
rather than omitting terminalProfile, referencing the existing setup and
setCachedStateField helpers and the SettingsView updateSettings/postMessage
entry point.
🪄 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 Plus
Run ID: db59c226-79a8-4714-b752-3545454c95be
📒 Files selected for processing (30)
packages/types/src/global-settings.tspackages/types/src/vscode-extension-host.tssrc/core/webview/ClineProvider.tssrc/core/webview/webviewMessageHandler.tssrc/integrations/terminal/BaseTerminal.tssrc/integrations/terminal/Terminal.tssrc/integrations/terminal/__tests__/TerminalProfile.spec.tswebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/components/settings/TerminalSettings.tsxwebview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsxwebview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsxwebview-ui/src/context/ExtensionStateContext.tsxwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
edelauna
reviewed
May 24, 2026
Contributor
edelauna
left a comment
edelauna
left a comment
There was a problem hiding this comment.
Nice this a good first pass - could you add a screenshot / screen recording of the terminal in action?
Contributor
Author
Thanks for the review! Here's the terminal profile selector in action: The new Inline terminal profile dropdown (Settings → Terminal → Advanced) set to zsh. 
I'm also working through your inline comments — the allowlisted profile-name request, fuller VS Code profile preservation, the copy clarification, and the updateSettings / startup-hydration tests. Will push those in a follow-up. 🦘 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/integrations/terminal/Terminal.ts (1)
280-283:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve
path[]using the first existing candidate, not index0.Current logic always picks
profile.path[0]. If that path is missing but a later candidate exists, profile resolution fails unnecessarily.In VS Code terminal profile settings (`terminal.integrated.profiles.windows|osx|linux`), when `path` is an array, does VS Code select the first existing executable path candidate?Proposed fix
+import { existsSync } from "fs" import * as vscode from "vscode" import pWaitFor from "p-wait-for" @@ - const pathValue = Array.isArray(profile.path) ? profile.path[0] : profile.path + const candidates = Array.isArray(profile.path) ? profile.path : [profile.path] + const firstNonEmpty = candidates.find((p): p is string => typeof p === "string" && p.length > 0) + const pathValue = + candidates.find((p): p is string => typeof p === "string" && p.length > 0 && existsSync(p)) ?? firstNonEmpty🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/terminal/Terminal.ts` around lines 280 - 283, The code in Terminal.ts currently picks profile.path[0] unconditionally which fails if that candidate doesn't exist; update the resolution for profile.path (used to compute pathValue passed to createTerminal) to iterate candidates and pick the first existing executable path (e.g., check each string in profile.path with fs.existsSync or fs.promises.access) and fall back to the original string when profile.path is not an array; ensure you update the variable referenced as pathValue and preserve existing behavior when no candidate exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/webview/webviewMessageHandler.ts`:
- Around line 1523-1538: The code currently iterates ["windows","osx","linux"]
and aggregates terminal profile names across all platforms; change it to only
read profiles for the active runtime platform so we don't surface non-resolvable
names. Detect the active platform (e.g. using process.platform and map
"win32"→"windows","darwin"→"osx","linux"→"linux"), then call
vscode.workspace.getConfiguration("terminal.integrated.profiles").get<Record<string,
unknown>>(mappedPlatform) and add keys from that single object into the existing
names Set before calling provider.postMessageToWebview({ type:
"terminalProfiles", profiles: Array.from(names).sort() }).
---
Duplicate comments:
In `@src/integrations/terminal/Terminal.ts`:
- Around line 280-283: The code in Terminal.ts currently picks profile.path[0]
unconditionally which fails if that candidate doesn't exist; update the
resolution for profile.path (used to compute pathValue passed to createTerminal)
to iterate candidates and pick the first existing executable path (e.g., check
each string in profile.path with fs.existsSync or fs.promises.access) and fall
back to the original string when profile.path is not an array; ensure you update
the variable referenced as pathValue and preserve existing behavior when no
candidate exists.
🪄 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 Plus
Run ID: f6372979-a8cc-46a4-bc65-be490c0584dc
📒 Files selected for processing (10)
packages/types/src/vscode-extension-host.tssrc/core/webview/__tests__/ClineProvider.spec.tssrc/core/webview/__tests__/webviewMessageHandler.spec.tssrc/core/webview/webviewMessageHandler.tssrc/integrations/terminal/Terminal.tssrc/integrations/terminal/__tests__/TerminalProfile.spec.tswebview-ui/src/components/settings/TerminalSettings.tsxwebview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsxwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.json
✅ Files skipped from review due to trivial changes (1)
- webview-ui/src/i18n/locales/es/settings.json
proyectoauraorg
added a commit
to proyectoauraorg/Zoo-Code
that referenced
this pull request
May 25, 2026
…o-Code-Org#119) VS Code selects the first terminal-profile path candidate that exists on disk; mirror that instead of always taking index 0, falling back to the first non-empty candidate when none exist. Addresses CodeRabbit review on Zoo-Code-Org#277.
proyectoauraorg
added a commit
to proyectoauraorg/Zoo-Code
that referenced
this pull request
May 26, 2026
…o-Code-Org#119) VS Code selects the first terminal-profile path candidate that exists on disk; mirror that instead of always taking index 0, falling back to the first non-empty candidate when none exist. Addresses CodeRabbit review on Zoo-Code-Org#277.
proyectoauraorg
force-pushed
the
feat/119-inline-terminal-profile
branch
from
92f45be to
cb3734f
Compare
Contributor
Author
|
Thanks for the review @edelauna! Responses: Screenshot/Recording: Will add a screen recording showing the terminal profile switching in action — targeting EOD. Code comments:
|
edelauna
reviewed
May 26, 2026
| </div> | ||
| </div> | ||
| <div className="flex flex-col gap-3 pl-3 border-l-2 border-vscode-button-background"> | ||
| <SearchableSetting |
Contributor
There was a problem hiding this comment.
Should this block be wrapped in {terminalShellIntegrationDisabled && (...)} so the dropdown is hidden when the user has turned off the inline terminal? The other inline-terminal-only settings (inheritEnv, shellIntegrationTimeout) disappear under the !terminalShellIntegrationDisabled guard at line 213, but the profile picker stays visible and editable even when it has no effect.
proyectoauraorg
added a commit
to proyectoauraorg/Zoo-Code
that referenced
this pull request
May 27, 2026
The terminal-profile dropdown only affects inline execution, which is active when shell integration is disabled. It previously stayed visible and editable even when inline mode was off, where it has no effect. Guard it with terminalShellIntegrationDisabled (defaulting to shown, matching the checkbox), mirroring the inline-only settings below. Addresses PR Zoo-Code-Org#277 review (edelauna).
Contributor
Author
|
Done — the profile picker is now guarded by |
edelauna
reviewed
May 27, 2026
Contributor
edelauna
left a comment
edelauna
left a comment
There was a problem hiding this comment.
could more questions, also lets close out the coderabbit feedback please
Comment on lines
+1515
to
+1545
| case "requestTerminalProfiles": { | ||
| // Allowlisted request: read VS Code's terminal profiles server-side and | ||
| // return only the sanitized profile names. The terminal profile dropdown | ||
| // only needs names, so this avoids routing it through the generic | ||
| // `getVSCodeSetting` handler (which reads any key the webview supplies). | ||
| try { | ||
| const names = new Set<string>() | ||
|
|
||
| for (const platform of ["windows", "osx", "linux"] as const) { | ||
| const profiles = vscode.workspace | ||
| .getConfiguration("terminal.integrated.profiles") | ||
| .get<Record<string, unknown>>(platform) | ||
|
|
||
| if (profiles && typeof profiles === "object") { | ||
| for (const name of Object.keys(profiles)) { | ||
| names.add(name) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| await provider.postMessageToWebview({ | ||
| type: "terminalProfiles", | ||
| profiles: Array.from(names).sort(), | ||
| }) | ||
| } catch (error) { | ||
| console.error("Failed to get terminal profiles:", error) | ||
| await provider.postMessageToWebview({ type: "terminalProfiles", profiles: [] }) | ||
| } | ||
|
|
||
| break | ||
| } |
Contributor
There was a problem hiding this comment.
The updateSettings -> Terminal.setTerminalProfile path has tests, but does this handler itself have any? The dedup+sort logic, the catch fallback (empty-array response), and edge cases like an all-null profiles object all seem untested -- worth adding a case to the webviewMessageHandler - terminalProfile suite that sends requestTerminalProfiles and asserts on what gets posted back.
| integration is disabled. Hide it otherwise so it isn't shown but ineffective | ||
| (mirrors the inline-only settings guarded below). Defaults to shown, matching | ||
| the checkbox's `?? true`. See PR #277 review. */} | ||
| {(terminalShellIntegrationDisabled ?? true) && ( |
Contributor
Author
|
Conflicts resolved and branch rebased on latest main. CI is passing. Ready for re-review. 🙏 |
|
Nice jobs!! ZooCode is purely unusable for my enviroment until this PR is merged, I am looking forward to this feature so I can switch from the buggy vscode termianl, Thanks for all of you!! 
|
edelauna
changed the title
Contributor
|
Ok I pushed an refactor update to get this PR on track since it was mixing 2 concerns. I think for this PR it would be great to specify the profile to use when integrated terminal is off. I updated the title and description, as well as a follow up for how to more specifically close issue #119 On windows this PR allows specifying the default terminal to use, created an issue in our docs repository to show how to overwrite the default profile and select a terminal by setting powershell or gir in 
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/core/webview/__tests__/webviewMessageHandler.spec.ts (2)
868-898: 💤 Low valueConsider adding afterEach for consistency.
While the manual
mockRestore()calls work correctly, adding anafterEach(() => { vi.restoreAllMocks() })block would align with the cleanup pattern used in other test suites (e.g., lines 913-915) and provide extra protection against test pollution.♻️ Proposed consistency improvement
describe("webviewMessageHandler - terminalProfile", () => { beforeEach(() => { vi.clearAllMocks() }) + + afterEach(() => { + vi.restoreAllMocks() + }) it("bridges a saved terminalProfile from updateSettings into the process-wide terminal state", async () => { - const setTerminalProfileSpy = vi.spyOn(Terminal, "setTerminalProfile").mockImplementation(() => {}) + vi.spyOn(Terminal, "setTerminalProfile").mockImplementation(() => {}) await webviewMessageHandler(mockClineProvider, { type: "updateSettings", updatedSettings: { terminalProfile: "Git Bash" }, }) - expect(setTerminalProfileSpy).toHaveBeenCalledWith("Git Bash") - - setTerminalProfileSpy.mockRestore() + expect(Terminal.setTerminalProfile).toHaveBeenCalledWith("Git Bash") }) it("clears the terminal profile when updateSettings sends undefined", async () => { - const setTerminalProfileSpy = vi.spyOn(Terminal, "setTerminalProfile").mockImplementation(() => {}) + vi.spyOn(Terminal, "setTerminalProfile").mockImplementation(() => {}) await webviewMessageHandler(mockClineProvider, { type: "updateSettings", updatedSettings: { terminalProfile: undefined }, }) - expect(setTerminalProfileSpy).toHaveBeenCalledWith(undefined) - - setTerminalProfileSpy.mockRestore() + expect(Terminal.setTerminalProfile).toHaveBeenCalledWith(undefined) }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/webview/__tests__/webviewMessageHandler.spec.ts` around lines 868 - 898, The tests for webviewMessageHandler that spy on Terminal.setTerminalProfile lack a global teardown; add an afterEach that calls vi.restoreAllMocks() so spies are reliably cleaned between tests (in the describe block containing the tests that call webviewMessageHandler and spy on Terminal.setTerminalProfile) to match other suites and prevent test pollution.
900-1019: ⚡ Quick winConsider mocking Terminal.getPlatformProfileKey() for test stability.
The handler calls
Terminal.getPlatformProfileKey()to determine which platform-specific configuration to read (line 1527 in the handler), but this method is not mocked in the tests. While the tests currently work becausegetConfigurationis mocked, explicitly mockinggetPlatformProfileKey()would:
- Make tests platform-independent
- Make the test intent clearer
- Prevent unexpected failures if the method's behavior changes
♻️ Proposed improvement
describe("webviewMessageHandler - requestTerminalProfiles", () => { beforeEach(() => { vi.clearAllMocks() + vi.spyOn(Terminal, "getPlatformProfileKey").mockReturnValue("windows") vi.spyOn(Terminal, "resolveProfilePath").mockImplementation((profilePath) => { const candidates = Array.isArray(profilePath) ? profilePath : [profilePath] const value = candidates.find( (candidate) => typeof candidate === "string" && candidate.trim().length > 0 && !candidate.includes("missing"), ) return typeof value === "string" ? value.trim() : undefined }) })Then update the test expectations to use the mocked platform key:
it("posts sorted path-resolvable profile names for the active platform", async () => { const mockGet = vi.fn().mockReturnValue({ "Git Bash": { path: "C:\\Git\\bin\\bash.exe" }, bash: { path: "/bin/bash" }, PowerShell: { source: "PowerShell" }, // source-only — must be excluded }) vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any) await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" }) + expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith("terminal.integrated.profiles") + expect(mockGet).toHaveBeenCalledWith("windows") expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({ type: "terminalProfiles", profiles: ["Git Bash", "bash"], }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/webview/__tests__/webviewMessageHandler.spec.ts` around lines 900 - 1019, Tests call Terminal.getPlatformProfileKey() indirectly via webviewMessageHandler but never mock it, making behavior platform-dependent; mock Terminal.getPlatformProfileKey() in the test setup (e.g., in beforeEach) to return a stable platform key string that matches the keys you use in your mocked vscode.workspace.getConfiguration().get(...) and update any expectations if they rely on a platform-specific key; specifically reference mocking Terminal.getPlatformProfileKey and ensure the mocked value aligns with the profile object names used in the mockGet return values and the webviewMessageHandler test assertions.webview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsx (1)
120-133: 💤 Low valueConsider testing profile switching for stronger validation.
The test clicks the already-selected profile ("Git Bash"). To verify that switching between profiles works correctly, consider clicking a different option like "zsh" instead.
💡 Optional enhancement to test profile switching
it("populates the dropdown from received profile names and selecting one sets the profile", () => { const { setCachedStateField } = setup("Git Bash") act(() => { window.dispatchEvent( new MessageEvent("message", { data: { type: "terminalProfiles", profiles: ["Git Bash", "zsh"] }, }), ) }) - fireEvent.click(screen.getByTestId("option-Git Bash")) - expect(setCachedStateField).toHaveBeenCalledWith("terminalProfile", "Git Bash") + fireEvent.click(screen.getByTestId("option-zsh")) + expect(setCachedStateField).toHaveBeenCalledWith("terminalProfile", "zsh") })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsx` around lines 120 - 133, The test in TerminalSettings.profile.spec.tsx currently clicks the already-selected "Git Bash"; update the test (inside the same it block using setup and setCachedStateField) to simulate selecting a different profile by dispatching the same MessageEvent with profiles ["Git Bash", "zsh"], then fireEvent.click on screen.getByTestId("option-zsh") instead of "option-Git Bash" and assert that setCachedStateField was called with ("terminalProfile", "zsh") to verify profile switching; keep the existing act wrapper around the window.dispatchEvent and reuse the setup helper and setCachedStateField mock.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@webview-ui/src/i18n/locales/ca/settings.json`:
- Around line 743-748: Update the "description" value for the terminal override
keys (the JSON entry with "label", "description", "overrideLabel",
"configureButton", "noProfiles") to mirror the English clarification: state
explicitly that this override only selects the shell binary/path/args/env
exposed by VS Code and does not enable full shell integration (so inline
execution still bypasses shell integration, prompts, and plugins); modify the
existing Catalan "description" string to include a short clause to that effect
and keep the existing <0>Més informació</0> link marker.
In `@webview-ui/src/i18n/locales/de/settings.json`:
- Around line 743-748: Update the German translation for the shell override so
it clarifies that this setting only selects the shell binary/path/arguments and
environment, not the full VS Code shell integration (no
prompts/plugins/interactive integration); modify the "description" value (and if
helpful, the "overrideLabel" or nearby text keys such as "label" /
"configureButton") to mirror the English clarification about inline mode
bypassing shell integration and explicitly state that only the executable/path
and args/env are used.
In `@webview-ui/src/i18n/locales/hi/settings.json`:
- Around line 743-748: Update the Hindi translation to clarify that selecting a
VS Code profile only chooses the shell binary/path/args/env and does not apply
full shell integration (prompts, plugins, or other in-shell behaviors); edit the
"description" string (and if needed "overrideLabel") in this locale entry (keys:
"description", "overrideLabel") to mirror the English clarification used for the
inline-terminal copy by appending a short sentence like "यह केवल शेल बाइनरी
(पथ/आर्ग/पर्यावरण) का चयन करता है — शेल इंटीग्रेशन/प्रॉम्प्ट/प्लगइन्स लागू नहीं
होते।" so users understand it does not enable full shell integration.
In `@webview-ui/src/i18n/locales/ja/settings.json`:
- Around line 743-748: Update the Japanese "description" (and optionally add a
short clarification near "overrideLabel" or "configureButton") so it mirrors the
English note: explicitly state that selecting a VS Code profile here only
chooses the shell binary/arguments/environment (path-based profile) and does not
enable full shell integration, prompts, or plugins for inline mode; edit the
"description" string used in settings.json to append a concise clarifying
sentence that the override only picks the shell executable and args, not full
shell integration.
---
Nitpick comments:
In `@src/core/webview/__tests__/webviewMessageHandler.spec.ts`:
- Around line 868-898: The tests for webviewMessageHandler that spy on
Terminal.setTerminalProfile lack a global teardown; add an afterEach that calls
vi.restoreAllMocks() so spies are reliably cleaned between tests (in the
describe block containing the tests that call webviewMessageHandler and spy on
Terminal.setTerminalProfile) to match other suites and prevent test pollution.
- Around line 900-1019: Tests call Terminal.getPlatformProfileKey() indirectly
via webviewMessageHandler but never mock it, making behavior platform-dependent;
mock Terminal.getPlatformProfileKey() in the test setup (e.g., in beforeEach) to
return a stable platform key string that matches the keys you use in your mocked
vscode.workspace.getConfiguration().get(...) and update any expectations if they
rely on a platform-specific key; specifically reference mocking
Terminal.getPlatformProfileKey and ensure the mocked value aligns with the
profile object names used in the mockGet return values and the
webviewMessageHandler test assertions.
In
`@webview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsx`:
- Around line 120-133: The test in TerminalSettings.profile.spec.tsx currently
clicks the already-selected "Git Bash"; update the test (inside the same it
block using setup and setCachedStateField) to simulate selecting a different
profile by dispatching the same MessageEvent with profiles ["Git Bash", "zsh"],
then fireEvent.click on screen.getByTestId("option-zsh") instead of "option-Git
Bash" and assert that setCachedStateField was called with ("terminalProfile",
"zsh") to verify profile switching; keep the existing act wrapper around the
window.dispatchEvent and reuse the setup helper and setCachedStateField mock.
🪄 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 Plus
Run ID: 549b96ae-c96e-493e-beab-b28dcfa67273
📒 Files selected for processing (28)
packages/types/src/vscode-extension-host.tssrc/core/webview/__tests__/webviewMessageHandler.spec.tssrc/core/webview/webviewMessageHandler.tssrc/integrations/terminal/BaseTerminal.tssrc/integrations/terminal/Terminal.tssrc/integrations/terminal/__tests__/TerminalProfile.spec.tswebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/components/settings/TerminalSettings.tsxwebview-ui/src/components/settings/__tests__/TerminalSettings.profile.spec.tsxwebview-ui/src/context/ExtensionStateContext.tsxwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
✅ Files skipped from review due to trivial changes (4)
- webview-ui/src/i18n/locales/ko/settings.json
- webview-ui/src/i18n/locales/id/settings.json
- webview-ui/src/i18n/locales/es/settings.json
- webview-ui/src/i18n/locales/fr/settings.json
🚧 Files skipped from review as they are similar to previous changes (8)
- webview-ui/src/i18n/locales/vi/settings.json
- src/core/webview/webviewMessageHandler.ts
- src/integrations/terminal/BaseTerminal.ts
- webview-ui/src/i18n/locales/it/settings.json
- webview-ui/src/components/settings/SettingsView.tsx
- src/integrations/terminal/tests/TerminalProfile.spec.ts
- webview-ui/src/components/settings/TerminalSettings.tsx
- src/integrations/terminal/Terminal.ts
Comment on lines
+743
to
+748
| "label": "Zoo Code टर्मिनल ओवरराइड", | ||
| "default": "VS Code डिफ़ॉल्ट प्रोफ़ाइल उपयोग करें (अनुशंसित)", | ||
| "description": "डिफ़ॉल्ट रूप से Zoo Code VS Code में कॉन्फ़िगर की गई शेल का उपयोग करता है। VS Code द्वारा प्रदर्शित पथ-आधारित शेल प्रोफ़ाइल चुनने के लिए ओवरराइड चुनें। केवल-स्रोत प्रोफ़ाइल (जैसे, अंतर्निर्मित PowerShell प्रविष्टि) यहाँ सूचीबद्ध नहीं किए जा सकते। <0>अधिक जानें</0>", | ||
| "overrideLabel": "Zoo Code के लिए शेल ओवरराइड करें", | ||
| "configureButton": "VS Code में डिफ़ॉल्ट प्रोफ़ाइल चुनें", | ||
| "noProfiles": "(terminal.integrated.profiles में कोई पथ-आधारित प्रोफ़ाइल नहीं मिली)" |
Contributor
There was a problem hiding this comment.
Clarify that this only selects the shell binary, not full shell integration.
This translation still reads as if inline execution uses the selected VS Code profile wholesale, but the nearby inline-terminal copy says inline mode bypasses shell integration/prompts/plugins. Please mirror the English clarification here so users understand this override only picks the shell path/args/env.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/i18n/locales/hi/settings.json` around lines 743 - 748, Update
the Hindi translation to clarify that selecting a VS Code profile only chooses
the shell binary/path/args/env and does not apply full shell integration
(prompts, plugins, or other in-shell behaviors); edit the "description" string
(and if needed "overrideLabel") in this locale entry (keys: "description",
"overrideLabel") to mirror the English clarification used for the
inline-terminal copy by appending a short sentence like "यह केवल शेल बाइनरी
(पथ/आर्ग/पर्यावरण) का चयन करता है — शेल इंटीग्रेशन/प्रॉम्प्ट/प्लगइन्स लागू नहीं
होते।" so users understand it does not enable full shell integration.
49283d9 to
a19a6b5
Compare
Add a 'terminalProfile' setting that lets users choose which VS Code terminal profile the inline terminal uses. On Windows the default cmd/PowerShell shell may use a non-UTF-8 code page (e.g. GBK) and garble output; selecting a UTF-8 profile such as Git Bash resolves this. The setting reuses VS Code's terminal profile concept: when set, the profile name is resolved against terminal.integrated.profiles.<platform> to derive shellPath/shellArgs for createTerminal. When empty/unset the default terminal behavior is preserved unchanged. Adds backend unit tests for profile resolution and a webview test for the settings dropdown wiring.
Zoo-Code-Org#119) - Route profile names through a dedicated allowlisted `requestTerminalProfiles` message instead of the generic `getVSCodeSetting` (which reads any key the webview supplies); the extension reads the profiles and returns only names. - Preserve the profile's `env` (sanitized to string/null; null unsets a var), merged onto the base env in createTerminal. - Clarify the setting copy (en + es) vs the 'Use Inline Terminal' description. - Add tests: updateSettings->setTerminalProfile bridge, resolveWebviewView startup hydration, and profile env preservation/sanitization.
…o-Code-Org#119) VS Code selects the first terminal-profile path candidate that exists on disk; mirror that instead of always taking index 0, falling back to the first non-empty candidate when none exist. Addresses CodeRabbit review on Zoo-Code-Org#277.
The terminal-profile dropdown only affects inline execution, which is active when shell integration is disabled. It previously stayed visible and editable even when inline mode was off, where it has no effect. Guard it with terminalShellIntegrationDisabled (defaulting to shown, matching the checkbox), mirroring the inline-only settings below. Addresses PR Zoo-Code-Org#277 review (edelauna).
…al, filter source-only profiles
…ait; add cmd.exe fast-path
…before submission
…ched profile on picker open
edelauna
force-pushed
the
feat/119-inline-terminal-profile
branch
from
4a3107f to
7be20a2
Compare
Contributor
Author
|
Thanks for taking the wheel on this @edelauna — splitting the two concerns and narrowing to "specify the profile to use when the integrated terminal is off" is the right call, and the refactor reads cleanly. Pulled your 17 commits and went through them locally:
Verified on the refactored branch locally:
Fully aligned with the direction. Glad to help with the docs follow-up (Zoo-Code-Docs#4) or the more specific #119 close-out — just say the word. 🦓 |
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
3 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.
Summary
Adds an optional Zoo Code shell override for commands executed through VS Code's integrated terminal.
By default, Zoo Code continues to use VS Code's configured terminal profile. Users can opt into a Zoo-specific shell from resolvable, path-based VS Code profiles. This setting does not apply to inline execution, which uses the Execa provider.
Key behavior
terminalProfilesetting. Leaving it unset preserves VS Code's default behavior.shellPath,shellArgs, and sanitizedenvvalues passed tovscode.window.createTerminal.cmd.exe, which cannot provide the shell integration required by this execution path.ZDOTDIRinjection while an override is active so VS Code can manage shell integration for the selected profile.Tests
Adds coverage for profile resolution and filtering, settings persistence and UI states, terminal recreation after profile changes,
cmd.exefallback, shell-integration startup handling, safe retry boundaries, and platform test-helper exit codes.Scope
This PR does not close #119. That issue concerns inline execution through
ExecaTerminalProcessand requires a separate Execa-specific shell selection design.Documentation follow-up: Zoo-Code-Org/Zoo-Code-Docs#4