feat: allow selecting terminal shell on Windows

[mohamedmastouri-hue]

Fixes issue #123. Adds a setting to select terminal shell on Windows.

---

Note

Medium Risk
Changes how every terminal session picks its shell at spawn time; wrong resolution or incomplete settings wiring could break terminal startup on Windows.

Overview
Prepares terminal shell selection on Windows by resolving the preferred shell asynchronously when a PTY session starts, instead of reading it synchronously at manager construction time.

shellResolver is now an Effect (tests can still inject Effect.succeed(...)). resolveShellCandidates takes a resolved preferredShell string; startSession runs yield* shellResolver on each spawn so a user-configured shell can be read from settings at launch. Existing Windows fallback order (custom choice → pwsh.exe → Windows PowerShell → ComSpec / cmd) is unchanged.

The diff also adds a ServerSettingsService import (not wired in this hunk) and is largely formatting across Manager.ts.

^{Reviewed by Cursor Bugbot for commit 8150931. Bugbot is set up for automated code reviews on this repo. Configure here.}

Note

Add terminal shell selector for Windows in General settings

• Adds a WindowsTerminalShellRow component to the General settings panel, visible only on Windows, letting users pick their preferred terminal shell from a predefined list.
• Persists the selection as windowsTerminalShell in ServerSettings (defaults to empty string) and supports reset to default.
• Adds a "Terminal shell" entry to the settings search index.
• The TerminalManagerOptions.shellResolver type changes from a synchronous function to an Effect.Effect<string>, evaluated at session start to resolve the preferred shell.
• stopProcess no longer immediately marks a session as exited; cleanup is now deferred until the process exit event is observed via drainProcessEvents.

📊 Macroscope summarized 8150931. 4 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 86794a3a-9506-446e-940e-04b50b4c964e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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

[macroscopeapp]

Two Effect service convention issues in apps/server/src/terminal/Manager.ts: the new serverSettings.ts import uses a named service import (and is never used/wired), and the shellResolver option was switched to an Effect without updating existing callers.

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

At a service boundary the local service module should be imported as a namespace (ServerSettings.ServerSettingsService), not as a named import. This import is also currently unused — production make() never does yield* ServerSettings.ServerSettingsService, so the new windowsTerminalShell setting is never fed into shellResolver and the dependency is not reflected in make/layer requirements. Consider either wiring it up or dropping the import.

-import { ServerSettingsService } from "../serverSettings.ts";
+import * as ServerSettings from "../serverSettings.ts";

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

Changing this seam from () => string to an Effect leaves existing consumers on the old shape: apps/server/src/terminal/Manager.test.ts still declares shellResolver?: () => string (line 205) and passes thunks (lines 1287, 1344, 1482), which no longer type-check against this option. Consider updating those callers mechanically (e.g. shellResolver: Effect.succeed("/bin/zsh")) as part of this change.

^{Posted via Macroscope — Effect Service Conventions}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 8150931. Configure here.}

[cursor]

Terminal shell setting unused

High Severity

ServerSettingsService is imported but never used, and make() never passes a shellResolver that reads windowsTerminalShell. Production terminals always fall back to defaultShellResolver, so the Windows shell setting persists in UI/settings but has no effect on spawned PTYs.

Additional Locations (1)

• apps/server/src/terminal/Manager.ts#L54-L55

 

^{Reviewed by Cursor Bugbot for commit 8150931. Configure here.}

[macroscopeapp]

🟡 Medium

t3code/apps/web/src/components/settings/SettingsPanels.tsx

Line 654 in 8150931

settings.fontSizeTerminal,

changedSettingLabels reads settings.windowsTerminalShell but the useMemo dependency array omits it. When the user changes only the terminal-shell setting, the memo stays stale, so the restore-defaults control fails to include "Terminal shell" in the confirmation until another listed dependency changes. Add settings.windowsTerminalShell to the dependency array.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/web/src/components/settings/SettingsPanels.tsx around line 654:

`changedSettingLabels` reads `settings.windowsTerminalShell` but the `useMemo` dependency array omits it. When the user changes only the terminal-shell setting, the memo stays stale, so the restore-defaults control fails to include "Terminal shell" in the confirmation until another listed dependency changes. Add `settings.windowsTerminalShell` to the dependency array.

[macroscopeapp]

🟡 Medium settings/SettingsPanels.tsx:1887

WindowsTerminalShellRow gates visibility on navigator.platform (the client's browser OS), but the setting configures the connected server's terminal. A user on macOS/Linux browsing to a Windows server gets null and never sees the shell selector, and a user on Windows connected to a non-Windows server sees a control that doesn't apply. Visibility should be driven by the server platform, not the browser platform.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/web/src/components/settings/SettingsPanels.tsx around line 1887:

`WindowsTerminalShellRow` gates visibility on `navigator.platform` (the client's browser OS), but the setting configures the connected server's terminal. A user on macOS/Linux browsing to a Windows server gets `null` and never sees the shell selector, and a user on Windows connected to a non-Windows server sees a control that doesn't apply. Visibility should be driven by the server platform, not the browser platform.

[macroscopeapp]

Approvability

Verdict: Needs human review

2 blocking correctness issues found. This PR introduces a new terminal shell selection feature for Windows, which is a new user-facing capability requiring human review. Additionally, unresolved findings indicate the feature is incomplete - the shell setting is persisted in the UI but is not wired up to actually affect terminal spawning.

^{You can customize Macroscope's approvability policy. Learn more.}