fix(terminal): use detected shell for execa and vscode terminal creation (#321)

[F915]

Related GitHub Issue

Closes: #321

Description

Zoo Code correctly detects the user's configured default shell (e.g., PowerShell) and tells the AI to generate commands using that shell's syntax, but the Inline Terminal actually executed commands in cmd.exe. This caused:

• PowerShell-specific commands (like Write-Host) to fail with "not recognized" errors
• Non-ASCII output garbling on systems with non-ASCII locales — cmd.exe encodes output using the system codepage (e.g., CP936 for zh-CN) rather than UTF-8, making characters unreadable. Running commands in the user's configured shell (PowerShell) makes it easier and safer to configure UTF-8 encoding, though explicit configuration may still be needed.

This PR connects the existing getShell() function to both terminal creation paths so the detected shell flows through to execution.

Core changes (2 files):

1. ExecaTerminalProcess.ts — Changed shell: true to shell: getShell(). The detected shell from VSCode's terminal.integrated.defaultProfile is now passed to execa subprocess execution.

2. Terminal.ts — Added shellPath: getShell() to vscode.window.createTerminal(). Also added iconPath: new vscode.ThemeIcon("rocket") for terminal icon consistency.

Test updates (2 files — assertions to match the new behavior):

3. ExecaTerminalProcess.spec.ts — Assertions updated from shell: true to shell: "/mock/fallback-shell" using vitest.spyOn pattern.

4. TerminalRegistry.spec.ts — Assertions updated to include the new shellPath parameter and corrected iconPath matching for ThemeIcon instances.

Key design decision: getShell() already validates against a built-in allowlist of ~80 approved shell paths and falls back to a safe default. No additional validation layer was needed at the call sites.

Roadmap alignment: Reliability First — "Guarantee smooth operation across all locales and platforms."

Test Procedure

Unit tests:

cd src
pnpm test -- --run integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts integrations/terminal/__tests__/TerminalRegistry.spec.ts

Type checking:

pnpm check-types

Manual verification (Windows with PowerShell configured):

Prerequisites: PowerShell configured with UTF-8 encoding.

• Run Write-Host "test" in Zoo Code Inline Terminal
  • Before: 'Write-Host' is not recognized as an internal or external command
  • After: test
• Run echo "中文测试" in Zoo Code Inline Terminal
  • Before: garbled output (cmd.exe uses system codepage, e.g., CP936 for zh-CN)
  • After: 中文测试 (PowerShell with UTF-8 configured)

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Testing: New and/or updated tests have been added to cover my changes (if applicable).
• Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Video

N/A — backend fix with no visible UI changes.

Documentation Updates

• Yes, documentation updates are required.

The shell integration docs need the following updates:

1. "Use Inline Terminal" section — clarify that Inline Terminal now uses the shell detected from VSCode's terminal.integrated.defaultProfile setting, rather than always defaulting to cmd.exe (Windows) or sh (Unix)
2. Inline Terminal vs VSCode Terminal comparison table — update the shell selection row to reflect that both modes now respect the user's configured default shell
3. Add Windows troubleshooting entry — document that on non-ASCII locales (zh-CN, ja-JP, ko-KR, etc.), cmd.exe encodes output using the system codepage rather than UTF-8. Running in the configured shell (e.g., PowerShell) enables easier UTF-8 configuration, though explicit setup is still required.

These updates will ensure the documentation accurately describes the current behavior rather than the pre-fix state.

Additional Notes

None.

Get in Touch

Discord: Yon (yon.sinc)

Summary by CodeRabbit

• Bug Fixes
  • Terminal initialization now consistently uses the resolved system shell as the fallback, improving terminal launch and process behavior across environments.
• Tests
  • Updated terminal tests to reflect the resolved shell behavior and to restore mocks for cleaner test isolation and more reliable assertions.

[coderabbitai]

📝 Walkthrough

Walkthrough

Two terminal integration files now explicitly invoke getShell() to select shell paths instead of relying on defaults or boolean true. ExecaTerminalProcess uses getShell() as a fallback when BaseTerminal.getExecaShellPath() is unavailable, and Terminal sets this value on VSCode terminal creation. Tests updated to expect string shell paths.

Changes

Explicit shell path selection via getShell()

Layer / File(s)	Summary
ExecaTerminalProcess shell fallback src/integrations/terminal/ExecaTerminalProcess.ts, src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts	getShell is imported and used as the fallback when BaseTerminal.getExecaShellPath() is not provided, replacing the true boolean. Test assertions and spies updated to expect the resolved shell path/string.
Terminal shell path initialization src/integrations/terminal/Terminal.ts, src/integrations/terminal/__tests__/TerminalRegistry.spec.ts	getShell is imported and explicitly passed as shellPath when creating a new VSCode terminal instance; TerminalRegistry tests mock getShell() and assert the resolved shellPath and tightened iconPath expectations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

• [BUG] Windows: shell mismatch between AI prompt generation and Inline Terminal execution #321: Both ExecaTerminalProcess and Terminal changes implement the same shell-selection pattern (BaseTerminal.getExecaShellPath() || getShell() and shellPath: getShell()) to fix shell configuration.
• [BUG] System prompt on windows says shell is cmd.exe but if powershell is used via VS Code terminal provider when inline terminal is off #82: The main changes directly address the same code-level shell-selection mismatch by switching ExecaTerminalProcess to use getExecaShellPath() || getShell() and explicitly using getShell() for VS Code terminal creation.

Poem

🐰 I hopped through paths both near and far,
I sniffed each shell to find the star,
getShell() calls make launches bright,
No guesswork now when terminals light,
A hopping cheer for code done right!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(terminal): use detected shell for execa and vscode terminal creation (#321)' clearly and specifically summarizes the main change: using the detected shell for both terminal creation paths, directly tied to issue #321.
Linked Issues check	✅ Passed	The PR successfully implements all coding requirements from issue #321: uses getShell() in both ExecaTerminalProcess.ts and Terminal.ts, preserves getExecaShellPath() priority, includes updated unit tests, and addresses the Windows shell mismatch problem.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #321: ExecaTerminalProcess.ts, Terminal.ts, and their test files; modifications consistently target the shell detection integration with no unrelated code changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The PR description is comprehensive and complete. It includes the related issue, detailed explanation of the problem and solution, test procedures, pre-submission checklist completion, documentation updates, and Discord contact information.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f5fb7a1-993e-4d1d-987f-4466eb7afe09

📥 Commits

Reviewing files that changed from the base of the PR and between b761a0a and dc4a451.

📒 Files selected for processing (3)

• src/integrations/terminal/ExecaTerminalProcess.ts
• src/integrations/terminal/Terminal.ts
• src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts

[coderabbitai]

🧹 Nitpick comments (1)

src/integrations/terminal/__tests__/TerminalRegistry.spec.ts (1)

39-40: ⚡ Quick win

Assert getShell() invocation to prevent hardcoded-shell regressions.

You already assert the resulting shellPath, but adding expect(shellUtils.getShell).toHaveBeenCalled() makes this test fail if production code hardcodes the same string instead of calling the resolver.

Suggested test hardening

 		it("creates terminal with PAGER set appropriately for platform", () => {
 			TerminalRegistry.createTerminal("/test/path", "vscode")
+			expect(shellUtils.getShell).toHaveBeenCalledTimes(1)

 			expect(mockCreateTerminal).toHaveBeenCalledWith({
 				cwd: "/test/path",
 				name: "Roo Code",
 				iconPath: expect.objectContaining({ id: expect.any(String) }),
 				env: {
 					PAGER,
 					ROO_ACTIVE: "true",
 					VTE_VERSION: "0",
 					PROMPT_EOL_MARK: "",
 				},
 				shellPath: "/mock/fallback-shell",
 			})
 		})

Also applies to: 60-61, 83-84, 107-108, 130-131

🤖 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/__tests__/TerminalRegistry.spec.ts` around lines 39
- 40, Add assertions that the shell resolver is actually invoked: after each
vi.spyOn(shellUtils, "getShell").mockReturnValue("/mock/fallback-shell") in the
TerminalRegistry.spec tests, add expect(shellUtils.getShell).toHaveBeenCalled()
(or toHaveBeenCalledTimes(1)) to ensure production code calls
shellUtils.getShell rather than hardcoding the path; apply this to the
occurrences around the tests that set up mocked shells (the blocks using
vi.spyOn(shellUtils, "getShell") and asserting shellPath).

🤖 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.

Nitpick comments:
In `@src/integrations/terminal/__tests__/TerminalRegistry.spec.ts`:
- Around line 39-40: Add assertions that the shell resolver is actually invoked:
after each vi.spyOn(shellUtils,
"getShell").mockReturnValue("/mock/fallback-shell") in the TerminalRegistry.spec
tests, add expect(shellUtils.getShell).toHaveBeenCalled() (or
toHaveBeenCalledTimes(1)) to ensure production code calls shellUtils.getShell
rather than hardcoding the path; apply this to the occurrences around the tests
that set up mocked shells (the blocks using vi.spyOn(shellUtils, "getShell") and
asserting shellPath).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 11da761d-d24d-4448-bec2-2eb4363d25fa

📥 Commits

Reviewing files that changed from the base of the PR and between 9205427 and 3ae0a88.

📒 Files selected for processing (1)

• src/integrations/terminal/__tests__/TerminalRegistry.spec.ts

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/integrations/terminal/ExecaTerminalProcess.ts	66.66%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!