feat(shell_dispatcher): isolate shell dispatcher layer

[aboimpinto]

This is the first PR in the split stack for the former #1848 work.

Scope:

• ShellDispatcher abstraction and shell selection logic
• Foreground shell execution routed through the dispatcher
• Sandbox command construction updated to use the dispatcher
• TUI shell execution integration kept separate from ExternalTool and the pluggable registry layers

This PR intentionally stays narrow and does not include the later plugin/registry security surface called out in #1848. Those follow-up concerns stay in the later stack layers.

Follow-up PRs will add the ExternalTool layer and then the pluggable tool registry on top of this base.

Greptile Summary

This PR introduces a ShellDispatcher abstraction layer that centralises shell detection and command construction, replacing scattered Command::new(\"sh\") / Command::new(\"cmd\") calls across eval.rs, sandbox/mod.rs, and tools/shell.rs. Detection at startup picks the right shell (Bash, Pwsh, WindowsPowerShell, Cmd, or a POSIX Custom path) from $SHELL or probe-based fallbacks, and all callers route through global_dispatcher().

• Shell detection results in a ShellKind enum with platform-aware binary(), command_flag(), and needs_command_flag() helpers; build_command / build_command_parts encapsulate per-shell quoting rules including Windows raw_arg for cmd.exe /C.
• Raw-mode save/restore guards are added in run_foreground, execute_sync_sandboxed, and execute_interactive_sandboxed so the TUI terminal state is correctly restored after child processes exit (issue [Windows] Terminal input breaks after exec_shell runs a batch script that spawns long-lived child processes #1690).
• sandbox/mod.rs now delegates shell selection to the dispatcher and layers the UTF-8 encoding prefix (chcp 65001 / [Console]::OutputEncoding) per shell family instead of unconditionally prepending the cmd-only wrapper.

Confidence Score: 5/5

Safe to merge; the dispatcher correctly handles all shell families and the raw-mode guards match the fix already applied to run_foreground.

The core detection, command-building, and raw-mode save/restore logic all look correct. The two findings are a redundant branch in display_command and a cfg-gate inconsistency on is_powershell(), neither of which affects runtime behaviour.

crates/tui/src/sandbox/mod.rs (redundant display_command branch) and crates/tui/src/shell_dispatcher.rs (is_powershell cfg gate).

Important Files Changed

Filename	Overview
crates/tui/src/shell_dispatcher.rs	New file; implements ShellKind enum, ShellDispatcher with detect/build/run_foreground, and a LazyLock global. Detection and builders look correct; raw-mode guard properly conditional. is_powershell() is cfg(test)-only which creates an asymmetry with the non-gated needs_command_flag().
crates/tui/src/sandbox/mod.rs	Shell command construction updated to use dispatcher; Windows path correctly splits chcp/PowerShell encoding wrappers by shell family. display_command has a redundant first branch for sh/bash that is entirely subsumed by the second branch.
crates/tui/src/tools/shell.rs	Added log_exec calls and raw-mode guards (SyncRawModeGuard / InteractiveRawModeGuard) that correctly check is_raw_mode_enabled() before disabling, matching the fix applied to run_foreground.
crates/tui/src/eval.rs	exec_shell simplified to delegate to global_dispatcher().run_foreground(); EvalShellPlatform and related helpers moved to cfg(test) since they are now only needed in tests.
crates/tui/src/tools/shell/tests.rs	Tests updated to branch on the detected shell kind; PowerShell-specific command strings added. The issue_1691 round-trip test now runs push_shell_args unconditionally (was windows-only), which is safe since push_shell_args behaves correctly on both platforms.
crates/tui/tests/eval_harness.rs	Adds shell_dispatcher module path include so the integration test binary can resolve crate::shell_dispatcher references from eval.rs.
crates/tui/src/prompts.rs	Shell name in environment block now sourced from global_dispatcher().kind().binary() instead of $SHELL directly; behaviour is equivalent but consistent with the dispatcher's view of the shell.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[global_dispatcher] -->|LazyLock, once| B[ShellDispatcher::detect]
    B --> C{Platform}
    C -->|Windows| D{$SHELL set?}
    D -->|contains bash| E[ShellKind::Bash]
    D -->|contains pwsh| F[ShellKind::Pwsh]
    D -->|contains powershell| G[ShellKind::WindowsPowerShell]
    D -->|not set / no match| H{pwsh.exe on PATH?}
    H -->|yes| F
    H -->|no| I{powershell.exe on PATH?}
    I -->|yes| G
    I -->|no| J[ShellKind::Cmd]
    C -->|Unix| K{$SHELL set?}
    K -->|contains bash| E
    K -->|contains pwsh| F
    K -->|contains powershell| G
    K -->|other path| L[ShellKind::Custom]
    K -->|not set| M[ShellKind::Sh]
    E & F & G & J & L & M --> N[ShellDispatcher]
    N -->|build_command| O[std::process::Command]
    N -->|build_command_parts| P[program + args Vec]
    N -->|run_foreground| Q[raw-mode guard + output]
    P --> R[CommandSpec::shell in sandbox]
    Q --> S[exec_shell in eval.rs]

Loading

Comments Outside Diff (2)

1. crates/tui/src/sandbox/mod.rs, line 155-186 (link)

   display_command regressed for bash and custom POSIX shells

   The first branch only matches program == "sh", but after this PR the detected shell on most Linux/macOS machines is ShellKind::Bash (program bash) or ShellKind::Custom (program /bin/zsh, /usr/bin/fish, etc.). None of those match the "sh" literal, so the else-branch fires and returns bash -c echo hello (or /bin/zsh -c echo hello) everywhere a bare echo hello was previously shown. The PowerShell and cmd branches were extended to cover their new variants, but the POSIX branch was not widened to cover bash or any -c-flag shell.

   

2. crates/tui/src/sandbox/mod.rs, line 159-190 (link)

   display_command still only matches "sh", breaking bash/custom POSIX shells

   The first branch checks self.program == "sh" (exact match). After this PR, Unix detection yields ShellKind::Bash (program "bash") or ShellKind::Custom (program /bin/zsh, /usr/bin/fish, etc.) on virtually every Linux/macOS machine where $SHELL is set. Both cases fall through to the else-branch and return bash -c echo hello instead of echo hello.

   Two tests will fail on any standard CI runner (GitHub Actions Ubuntu/macOS sets SHELL=/bin/bash):

   • test_command_spec_shell asserts display_command() == "echo hello" but gets bash -c echo hello
   • test_command_spec_shell_quoted_arg_not_split (line 649) asserts spec.program == "sh" but receives "bash"

   The POSIX branch needs to be widened to cover "bash" and any -c-flag shell, just as the cmd/PowerShell branches were already extended for their new variants.

   

_{Reviews (6): Last reviewed commit: "fix(tui): use raw args for cmd payloads" | Re-trigger Greptile}

[gemini-code-assist]

Code Review

This pull request introduces a new ShellDispatcher module to centralize and abstract shell detection and command execution across Windows and Unix platforms. It replaces scattered, platform-specific shell invocation logic and ensures terminal raw-mode is correctly saved and restored during foreground execution. The review feedback highlights several cross-platform issues: the $SHELL environment variable detection should be restricted to non-Windows platforms to avoid issues with Unix-style paths, Windows-only helper functions should be conditionally compiled to prevent dead code warnings on Unix, and Windows-specific tests should be marked with #[cfg(windows)] to avoid test failures on Unix systems.

[aboimpinto]

Addressed the macOS CI failure and the platform-gating review comments in this push:

• SHELL detection is now Unix-only.
• ind_exe / �inary_on_path are Windows-only.
• Windows-only tests are gated with #[cfg(windows)].
• The sandbox shell wrapper no longer trips the unused-variable warning on non-Windows builds.

I also resolved the review threads that this commit directly covers.

Paulo Aboim Pinto