fix(shell): default to PowerShell on native Windows without the wrapper#49
Open
MilkClouds wants to merge 1 commit into
Open
fix(shell): default to PowerShell on native Windows without the wrapper#49MilkClouds wants to merge 1 commit into
MilkClouds wants to merge 1 commit into
Conversation
Running the bare `agf` binary on Windows (no `agf setup`) crashed with `Error: program not found`. With `AGF_SHELL` unset, `from_env` assumed POSIX on every OS, so resume/new/open generated `cd ... && claude` and the delivery path tried to exec it via `sh -c`. Windows has no `sh`, so the spawn failed with Rust's "program not found" io error. Infer the default shell from the OS when `AGF_SHELL` is absent: POSIX off Windows or under a Git Bash / MSYS2 layer (`MSYSTEM`); otherwise native Windows, which needs PowerShell. A present `SHELL` is a POSIX-layer signal (native Windows shells don't export it, e.g. Cygwin), so its basename is classified through `from_name` — keeping the powershell/pwsh names in one place and treating any non-PowerShell shell as POSIX. The wrapper still sets `AGF_SHELL` explicitly, so the installed path is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6a3a4f8 to
e4a4b83
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Running the bare
agfbinary on Windows — withoutagf setuphaving installed the wrapper — crashes on any Resume / New Session / Open:Cause
deliver_commandis designed to exec the chosen action directly when no wrapper is present (the wrapper is only strictly needed to persist a barecd). Butfrom_envreturnedPosixwheneverAGF_SHELLwas unset — on every OS. So on native Windows it generatedcd '...' && claudeand exec'd it viash -c; Windows has nosh, so the spawn failed with Rust's WindowsErrorKind::NotFound, whose message is literallyprogram not found. POSIX hosts were unaffected (shexists), so the intended no-wrapper behavior already worked everywhere except native Windows.Why
default_shellWith
AGF_SHELLabsent there was no shell decision at all — just a blanket POSIX assumption. Fixing the crash means making that case an OS-aware choice (native Windows → PowerShell; Git Bash / MSYS2 / Cygwin → POSIX; else POSIX), which requires readingMSYSTEM/SHELLand must be unit-testable without mutating process-global env. So it's factored into a small pure functiondefault_shell(is_windows, msystem, shell), kept separate fromfrom_name(an explicit user-supplied token) to avoid conflating "told us" with "inferred."Fix
from_envhonorsAGF_SHELLwhen set, else defers todefault_shell. On Windows a presentSHELLsignals a POSIX layer (native shells don't export it); its basename is classified throughfrom_nameso the powershell/pwsh names stay in one place and any other shell is POSIX. The wrapper still setsAGF_SHELL=powershell, so the installed path is unchanged — this only fixes the no-wrapper fallback.program not found)¹ Persisting a bare
cdstill requires the wrapper (a child process can't change the parent shell's directory) — unchanged, and this path already hints to runagf setup.Tests
Pure unit tests for
default_shell: native Windows → PowerShell,MSYSTEM/unixSHELL→ POSIX, PowerShell-only-for-pwsh, and basename-not-path-substring matching.