Fix Fish installer setup - #1891
Conversation
Entire-Checkpoint: 01KZ43R2HRW3QW0KZ68N7JRHGS
There was a problem hiding this comment.
Pull request overview
This pull request fixes the Fish install experience for the Bash-based install.sh flow by correctly detecting the invoking shell (instead of relying on login $SHELL), using Fish-native PATH setup guidance, and ensuring post-install actions (notably shell completion setup) can run on first-time installs. It also updates completion targeting to respect XDG_CONFIG_HOME and adds regression coverage for the installer behaviors.
Changes:
- Add parent-shell detection in
scripts/install.shand present Fish-specific PATH setup usingfish_add_path, while preserving existing Bash/Zsh messaging. - Run
entire curl-bash-post-installusing the absolute installed binary path (so first-time installs can still execute post-install steps before PATH is updated). - Update Fish shell completion targeting to honor
ENTIRE_INSTALLER_SHELLandXDG_CONFIG_HOME, with new/expanded tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/install.sh | Detect invoking shell via parent process, pass it to post-install via env, and show Fish-native PATH instructions. |
| scripts/install_test.go | Add non-Windows regression tests covering Fish-first-install behavior, helper functions, and “piped to bash” execution. |
| README.md | Clarify that the installer is Bash-only and Fish users should still pipe to bash. |
| cmd/entire/cli/setup.go | Prefer ENTIRE_INSTALLER_SHELL over $SHELL and resolve Fish rc file under XDG_CONFIG_HOME when set. |
| cmd/entire/cli/setup_test.go | Extend completion-target tests for ENTIRE_INSTALLER_SHELL override and Fish XDG_CONFIG_HOME behavior. |
Suppressed comments (2)
scripts/install.sh:100
- Fish PATH instructions should reuse the derived install directory display value for consistency with the actual install location.
fish)
# fish_add_path updates this Fish session and persists the path for
# future sessions, so no config-file edit or restart is required.
echo -e " ${BOLD}fish_add_path \"\$HOME/.local/bin\"${NC}"
echo ""
scripts/install.sh:125
- In the unknown-shell fallback, the Fish example should also use the same derived install directory display value instead of hard-coding
$HOME/.local/bin.
*)
echo " Fish:"
echo -e " ${BOLD}fish_add_path \"\$HOME/.local/bin\"${NC}"
echo ""
| show_path_setup() { | ||
| local shell_name="$1" | ||
| local install_dir="$2" | ||
| local shell_config="" | ||
|
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 41a2f77. Configure here.
| if [[ -z "$path_binary" ]]; then | ||
| # First-time install: ~/.local/bin likely isn't on their PATH yet. | ||
| show_path_setup "$shell_name" "$install_dir" | ||
| exit 0 |
There was a problem hiding this comment.
Completion runs before PATH setup
Medium Severity
First-time installs now run curl-bash-post-install before PATH instructions. If completion is accepted, the rc line calls entire before that directory is on PATH. For Bash/Zsh the later export PATH=... append ends up after the completion line, so the next shell start invokes entire too early. The completion “Restart your shell” note also encourages restarting before running fish_add_path.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 41a2f77. Configure here.
| if command -v ps &> /dev/null; then | ||
| parent_command="$(ps -p "$PPID" -o comm= 2>/dev/null || true)" | ||
| shell_name="$(normalize_shell_name "$parent_command")" | ||
| fi |
There was a problem hiding this comment.
Untrimmed ps shell detection
Low Severity
normalize_shell_name exact-matches bash|fish|zsh on raw ps -o comm= output without trimming whitespace. Padded ps values like fish with spaces fail the match, so detection falls back to $SHELL and can again mis-identify Fish users whose login shell differs.
Reviewed by Cursor Bugbot for commit 41a2f77. Configure here.
Entire-Checkpoint: 01KZ45AF70QAAAQF7G9Y9GVDJM
|
Thanks for looking into supporting Fish shell @ashtom. 🙌🏻 I tested it from Fish on macOS (darwin/arm64): curl -fsSL https://raw.githubusercontent.com/entireio/cli/fix/install-fish-support/scripts/install.sh | bash
==> Installing Entire CLI...
==> Detected platform: darwin/arm64
==> Fetching latest stable version...
==> Installing version: 0.9.0
==> Downloading entire_darwin_arm64.tar.gz...
==> Downloading checksums...
==> Verifying checksum...
==> Checksum verified
==> Extracting...
==> Installing to /Users/thibaudgg/.local/bin...
==> Directory ready
==> Entire CLI installed to /Users/thibaudgg/.local/bin/entire
==> Running post-install actions...
✓ Shell completion added to /Users/thibaudgg/.zshrc
Restart your shell to activateInstall succeeded (v0.9.0), but post-install wrote the completion to the wrong place: I’m running Fish as my daily shell. The completion should have targeted Fish ( My Happy to re-test once the binary side is also updated / available. |
Summary
$SHELLfish_add_pathflow and preserve XDG-aware completion configbashRoot cause
The installer was Bash-only, but its Fish handoff relied solely on
$SHELLand appendedfish_add_pathto a hard-coded~/.config/fish/config.fish. That could select the wrong shell, ignoreXDG_CONFIG_HOME, leave the current Fish session without the new path, and skip post-install actions during the normal first install.User impact
Fish users now receive a command that updates the current session and persists for future sessions. Completion setup follows the XDG config location, while Bash and Zsh behavior remains unchanged.
Validation
mise run checkNote
Low Risk
Changes are limited to install scripting, post-install shell completion targeting, and tests; no auth or runtime CLI behavior for existing installs.
Overview
Fish and first-time installs no longer rely on login
$SHELLor editingconfig.fish. The Bash installer infers the parent shell (psonPPID, then$SHELL), passes it asENTIRE_INSTALLER_SHELLintoentire curl-bash-post-install, and shows Fish-specific PATH help viafish_add_path(no restart). Zsh/Bash PATH messaging is unchanged; unknown shells get both Fish and POSIX instructions.CLI post-install (
shellCompletionTarget) prefersENTIRE_INSTALLER_SHELLover$SHELLand resolves Fish’s rc file underXDG_CONFIG_HOMEwhen set.Flow change: post-install runs even when
~/.local/binis not on PATH yet (absolute binary path), then PATH setup is shown and the script exits—so shell completion can run on first install.Docs note that Fish users should still pipe the installer to
bash. Tests cover installer helpers, piped-to-bash execution, Fish end-to-end, and expandedTestShellCompletionTarget.Reviewed by Cursor Bugbot for commit 41a2f77. Configure here.