[codex] ExternalTool abstraction layer

[aboimpinto]

This is the second stacked PR on top of #2290.

Scope:

• extract hardcoded subprocess/tool spawning into ExternalTool
• update TUI call sites to use the shared abstraction
• keep the shell-dispatcher branch as the base

Validation:

• cargo fmt --all
• cargo check -p codewhale-tui --all-features --locked

This PR is intentionally stacked on fix/1779-windows-shell-dispatch and is not based on main.

Paulo Aboim Pinto

Greptile Summary

This PR introduces two cross-cutting abstractions: ExternalTool (a trait unifying subprocess resolution and command construction for git, gh, rustc, cargo, python, node) and ShellDispatcher (a singleton that detects the user's shell at startup and builds correctly-structured Command objects for every platform/shell variant). All direct Command::new(\"git\") / Command::new(\"gh\") call sites across the TUI crate are migrated to these new types.

• ExternalTool trait (dependencies.rs): each concrete tool holds its own static OnceLock cache and exposes both sync command() and async tokio_command() factory methods.
• ShellDispatcher (shell_dispatcher.rs): detects shell kind from $SHELL / PATH probing; build_command handles per-shell quoting (PowerShell, cmd, POSIX); run_foreground saves/restores crossterm raw mode.
• Call-site cleanup: ~20 files replace hardcoded Command::new(...) with ExternalTool::command() or convenience wrappers.

Confidence Score: 3/5

The abstraction layer is sound and production migrations are correct, but two Cmd-shell tests assert on get_args() which does not capture raw_arg() content on Windows.

The Cmd-shell tests would panic on Windows CI because build_command routes the payload through raw_arg(), which is invisible to get_args(). The rest of the migration is correct and well-tested on Unix paths.

crates/tui/src/shell_dispatcher.rs — the Cmd-shell unit tests need platform guards or alternative assertion strategies.

Important Files Changed

Filename	Overview
crates/tui/src/dependencies.rs	New ExternalTool trait with per-type OnceLock caching; concrete implementations for Git, Gh, RustC, Cargo, Python, Node with comprehensive unit tests.
crates/tui/src/shell_dispatcher.rs	New ShellDispatcher abstraction for shell detection and command building; Cmd shell tests use get_args() which misses raw_arg() content on Windows.
crates/tui/src/tools/tasks.rs	TaskGateRunTool restored to /bin/sh -lc login-shell invocation; PrAttemptPreflightTool uses spawn_blocking+sync command() instead of the purpose-built tokio_command().
crates/tui/src/tools/git.rs	Migrated to ExternalTool::Git; -c core.quotepath=false flag preserved with regression test restored.
crates/tui/src/core/engine/tool_catalog.rs	Rewrote should_default_defer_tool with mode-aware !matches!() block; adds tests for previously-deferred core tools.
crates/tui/src/utils.rs	New open_url() helper consolidates platform-specific URL opening using spawn() fire-and-forget on all platforms.
crates/tui/src/sandbox/mod.rs	CommandSpec::shell delegates to ShellDispatcher; display_command extended with PowerShell and custom-shell branches.

Comments Outside Diff (3)

1. crates/tui/src/tools/git.rs, line 66-77 (link)

   Unicode filenames silently regressed

   The refactor drops -c core.quotepath=false that was previously prepended to every git status invocation. Without that flag, git uses its default core.quotepath=true setting, which encodes non-ASCII filenames as octal escape sequences. Users with CJK, Cyrillic, Arabic, or emoji paths in their workspace will see "\344\270\255\346\226\207..." instead of the real path. The test that explicitly verified this (git_status_reports_unquoted_unicode_paths) was also removed in this PR — the fix and its verification vanished together. The args should prepend -c core.quotepath=false before extending with the user-provided args.

   

2. crates/tui/src/tools/git.rs, line 74-81 (link)

   The -c core.quotepath=false flag that forced git to emit raw UTF-8 filenames was removed here. Without it, git's default core.quotepath=true setting encodes non-ASCII path components as octal escape sequences (e.g. "\344\270\255\346\226\207..." for CJK paths). Any user with Unicode filenames will see garbled output. The test that exercised this behaviour (git_status_reports_unquoted_unicode_paths) was also deleted in this PR.

   

3. crates/tui/src/shell_dispatcher.rs, line 1936-1943 (link)

   Cmd-shell tests will fail on Windows due to raw_arg invisibility

   build_command for ShellKind::Cmd uses cmd.raw_arg(shell_command) on Windows (gated #[cfg(windows)]), but get_args() only returns args added via arg()/args() — it does not surface raw_arg content. So on a real Windows CI run:

   args == ["/C"] // length 1, not 2; args.contains(&"echo hello") == false

   Both cmd_build_command_uses_c_flag (asserts args.contains(&"echo hello")) and build_command_quotes_spaces_for_cmd (asserts args.len() == 2 and args[1].starts_with("git ")) would panic. Since this PR is explicitly stacked on the Windows shell-dispatch fix branch, broken Windows tests are a regression. Both tests should be gated with #[cfg(not(windows))], or the assertions should use an alternative that captures raw_arg content.

   

_{Reviews (7): Last reviewed commit: "fix(tui): keep core native tools active ..." | Re-trigger Greptile}

[gemini-code-assist]

Code Review

This pull request introduces a unified ExternalTool trait to abstract external command execution (such as git, gh, cargo, python, node, and rustc) and a ShellDispatcher to manage shell-specific execution details. While these changes centralize subprocess handling, several critical regressions were identified in the review. Specifically, eager clipboard initialization on startup and synchronous waiting on external clipboard commands can block or hang the TUI event loop, particularly on headless Linux/WSL2 hosts or Windows. Additionally, replacing lossy UTF-8 decoding with strict read_line in the Python REPL makes it fragile to non-UTF-8 output, removing -c core.quotepath=false breaks unicode filename support in git status, removing the wl-copy fallback breaks Linux Wayland clipboard support, removing the always_load parameter breaks user-configured tool loading, and using blocking .status() for URL opening can freeze the TUI thread.

[greptile-apps]

Want your agent to iterate on Greptile's feedback? Try greploops.