Skip to content

fix(desktop): resolve Doctor install shell and command detection on Windows#1854

Merged
wpfleger96 merged 5 commits into
mainfrom
duncan/windows-doctor-install
Jul 14, 2026
Merged

fix(desktop): resolve Doctor install shell and command detection on Windows#1854
wpfleger96 merged 5 commits into
mainfrom
duncan/windows-doctor-install

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Doctor menu installs on Windows fail with failed to spawn shell: The system cannot find the path specified. (os error 3) for all three runtimes (Goose, Claude Code, Codex). Five phases addressed:

  • Phase A — install shell selection: install_shell_command() hardcoded /bin/zsh / /bin/bash, which don't exist on Windows. Now resolves Git Bash via resolve_bash_path() — a bash-requiring variant that skips BUZZ_SHELL (since install commands use bash-only -l -c syntax) and falls through to GIT_BASH → PATH scan → derive-from-git → well-known → registry. Adds CREATE_NO_WINDOW on Windows, and fixes the timeout-kill path to use taskkill instead of the unix-only libc::kill. A BUZZ_SHELL=pwsh user gets a green Doctor prereq (agents work fine) but installs use the Git Bash fallback chain.

  • Phase B — per-OS install commands: Claude and Codex get cli_install_commands_windows delegating to their official install.ps1 via powershell.exe -NoProfile -ExecutionPolicy Bypass -Command "irm <url> | iex". Goose unchanged (its script is already Windows-aware). cli_install_commands_for_os() accessor selects the right commands per platform.

  • Phase C — Windows command resolution: resolve_command_uncached now tries .cmd and .bat candidates on Windows so npm-generated shims (e.g. codex-acp.cmd in %APPDATA%\npm) are discoverable. common_binary_paths() includes %APPDATA%\npm and %LOCALAPPDATA%\Programs\OpenAI\Codex\bin. login_shell_candidates() falls back to Git Bash (via resolve_bash_path(), skipping non-bash BUZZ_SHELL) instead of the hardcoded /bin/zsh, /bin/bash. fetch_login_shell_path_inner() returns None on Windows to prevent POSIX-format PATH from leaking into native-process consumers.

  • Phase D — BUZZ_SHELL contract split: Two named resolvers with distinct contracts. resolve_git_bash_path() honors BUZZ_SHELL (any executable: cmd, pwsh) — called by discover_git_bash() for the Doctor readiness gate; git_bash_available() inlines the same chain with overrides for its distinct parameter needs. resolve_bash_path() skips BUZZ_SHELL entirely and requires a bash-compatible shell — called by resolve_install_shell() and login_shell_candidates(). Two regression tests in git_bash.rs prove both contracts hold simultaneously.

  • Phase E — deterministic Windows test isolation: resolve_git_bash_inner takes an explicit check_registry toggle; resolve_git_bash_no_registry test helper disables the ambient HKLM/HKCU\SOFTWARE\GitForWindows lookup so test_no_git_bash_resolved_returns_none passes deterministically on CI runners with Git for Windows installed. test_cmd_shim_resolves_from_path calls resolve_command_uncached through PATH mutation under lock_path_mutex (replacing the prior tautological assertion). install_shell_from pure seam enables direct testing of the OptionResult error mapping against GIT_BASH_INSTALL_HINT.

Files changed

  • desktop/src-tauri/src/commands/agent_discovery.rsinstall_shell_commandResult, resolve_install_shell() delegates to install_shell_from() seam, Windows CREATE_NO_WINDOW + taskkill timeout kill, tests
  • desktop/src-tauri/src/managed_agents/discovery.rscli_install_commands_windows field, cli_install_commands_for_os(), .cmd/.bat resolution, login_shell_candidates() calls resolve_bash_path(), POSIX PATH guard
  • desktop/src-tauri/src/managed_agents/discovery/tests.rs — deterministic .cmd resolver test, registry-disabled no-shell test, install_shell_from error-mapping tests
  • desktop/src-tauri/src/managed_agents/git_bash.rsresolve_git_bash_path() (readiness, called by discover_git_bash) + resolve_bash_path() (install/login-shell), resolve_git_bash_inner with check_registry toggle, resolve_git_bash_no_registry test helper, GIT_BASH_INSTALL_HINT, BUZZ_SHELL bypass regression tests
  • desktop/src-tauri/src/managed_agents/mod.rspub(crate) mod git_bash visibility
  • desktop/scripts/check-file-sizes.mjs — file-size overrides

Accepted residual

  • If the resolved bash lacks curl (e.g. bare Cygwin), the goose installer fails with a clear curl error.
  • BUZZ_SHELL=pwsh + no Git Bash anywhere = prereq green but installs fail with the Doctor hint. Correct semantic per design: install scripts are bash/PowerShell, not arbitrary shells.

…indows

install_shell_command() hardcoded /bin/zsh / /bin/bash, which don't exist
on Windows — every Doctor install failed with os error 3. Additionally,
Claude/Codex install.sh scripts hard-exit on MINGW/MSYS/CYGWIN, and
resolve_command only tried .exe extensions (missing npm .cmd shims).

Phase A: install_shell_command now resolves Git Bash via the same
resolver chain Doctor's prereq uses (resolve_git_bash_path), adds
CREATE_NO_WINDOW on Windows, and fixes the timeout-kill path to use
taskkill instead of the unix-only libc::kill.

Phase B: Claude and Codex get Windows-specific cli_install_commands
delegating to their official install.ps1 via powershell.exe. Goose
unchanged (its script is already Windows-aware).

Phase C: resolve_command_uncached now tries .cmd/.bat candidates on
Windows so npm-generated shims are discoverable. common_binary_paths
includes %APPDATA%\npm and %LOCALAPPDATA%\Programs\OpenAI\Codex\bin.
run_in_login_shell falls back to Git Bash on Windows instead of the
hardcoded /bin/zsh, /bin/bash.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96 wpfleger96 requested a review from a team as a code owner July 14, 2026 14:56

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking Windows shell-contract issue: the new install/login-shell path reuses the Doctor resolver, but that resolver intentionally treats BUZZ_SHELL=cmd and BUZZ_SHELL=pwsh as valid. These install commands are bash syntax and are always invoked with -l -c, so a user with either documented override still gets a broken Doctor install path. Please make the install/login-shell resolver require a bash-compatible executable (or bypass a non-bash BUZZ_SHELL and continue to Git Bash), with a Windows regression for BUZZ_SHELL=cmd/pwsh. The rest of the platform-specific command/shim work looks sound, and current CI is green.

Comment thread desktop/src-tauri/src/managed_agents/git_bash.rs
…vioral tests

fetch_login_shell_path_inner() returns None on Windows so Git Bash's
colon-delimited PATH never reaches agent_models, build_augmented_path,
or cli_probe consumers that split on semicolons. resolve_git_bash()
made pub(crate) for cross-module test access.

Five cfg(windows) behavioral tests added:
- command_basenames produces .exe/.cmd/.bat candidates
- cli_install_commands_for_os selects PowerShell installers
- login_shell_path returns None (regression lock)
- .cmd shim resolves from well-known dirs
- empty env yields no Git Bash + Doctor hint is non-empty

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…solution

BUZZ_SHELL intentionally accepts any executable (cmd, pwsh) for the MCP
child's readiness gate, but install_shell_command and login_shell_candidates
invoke the result with bash-only -l -c syntax. A BUZZ_SHELL=cmd/pwsh
config broke every Doctor install and command discovery path.

Add resolve_bash_path() that passes None for shell_override, skipping
BUZZ_SHELL and falling through to GIT_BASH → PATH scan → derive-from-git
→ well-known → registry. The existing resolve_git_bash_path() keeps its
any-shell semantics for Doctor/readiness. Two regression tests prove both
contracts hold simultaneously (readiness accepts pwsh/cmd; install skips
them and finds bash).
discover_git_bash() inlined the same GitBashEnv::from_process() +
resolve_git_bash() call that resolve_git_bash_path() wraps. Replace
with a call to resolve_git_bash_path() so the "two named resolvers,
two contracts" story is true in code: resolve_git_bash_path() has a
real caller (discover_git_bash), resolve_bash_path() has its two
(resolve_install_shell, login_shell_candidates).

Fix the #[allow(dead_code)] comment to name the actual caller.
…ell error

resolve_git_bash_no_registry skips the ambient HKLM/HKCU registry lookup
so test_no_git_bash_resolved_returns_none passes deterministically on CI
runners with Git for Windows installed. test_cmd_shim_resolves_from_path
now calls the real resolve_command_uncached via PATH mutation under
lock_path_mutex. install_shell_from seam enables pure testing of the
Option→Result error mapping against GIT_BASH_INSTALL_HINT.
@wpfleger96 wpfleger96 merged commit 1aec7ea into main Jul 14, 2026
25 checks passed
@wpfleger96 wpfleger96 deleted the duncan/windows-doctor-install branch July 14, 2026 17:05
brow pushed a commit that referenced this pull request Jul 14, 2026
…image-paste

* origin/main:
  refactor(clients): standardize product naming on community (#1858)
  fix(desktop): retain live rows after window exhaustion (#1810)
  Add private product feedback sidecar (#1857)
  fix(desktop): resolve Doctor install shell and command detection on Windows (#1854)
  fix(desktop): navigate to channel from inbox thread header (#1847)
  feat(desktop): surface needsRestart badge on live UI surfaces (#1853)
  fix(desktop): prevent menu-to-dialog UI lockups (#1839)
  feat(relay): add durable community archival (#1834)
  feat(desktop): add conversation-style DM composer (#1768)
  feat(push): add public APNs gateway (#1770)
  feat(relay): add atomic community ownership transfer (#1845)
  fix(desktop): stabilize agent identity restore (#1831)
  fix(buzz-acp): distinguish idle vs hard-cap timeout, dead-letter hard kills immediately (#1844)
  fix(observer): align scroll-anchor ids with transcript display-block keys (#1849)
  [codex] Add view activity label to agent popover (#1748)
  ci(desktop): surface flaky E2E tests instead of retry-masking them (#1838)
  fix(desktop): treat channel creator as member before 39002 provisioning (#1830)
  fix(mobile): mirror app bar title padding when actions are empty (#1832)

Co-authored-by: npub1shglkdhngx3hrnhf4gf8vhpqdrmeludctechdvpwd3988zzs7ncq2cmtxu <85d1fb36f341a371cee9aa12765c2068f79ff1b85e7176b02e6c4a738850f4f0@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: npub1shglkdhngx3hrnhf4gf8vhpqdrmeludctechdvpwd3988zzs7ncq2cmtxu <85d1fb36f341a371cee9aa12765c2068f79ff1b85e7176b02e6c4a738850f4f0@sprout-oss.stage.blox.sqprod.co>
tlongwell-block pushed a commit that referenced this pull request Jul 14, 2026
* origin/main:
  Add private product feedback sidecar (#1857)
  fix(desktop): resolve Doctor install shell and command detection on Windows (#1854)

Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co>

# Conflicts:
#	crates/buzz-db/src/migration.rs
tlongwell-block pushed a commit that referenced this pull request Jul 14, 2026
* origin/main:
  Add private product feedback sidecar (#1857)
  fix(desktop): resolve Doctor install shell and command detection on Windows (#1854)

Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co>

# Conflicts:
#	crates/buzz-db/src/migration.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants