fix(install): skip shell config for uninstalled shells

[AnnatarHe]

Summary

• Guard zsh/bash PATH and hook-sourcing blocks behind [ -f ~/.zshrc ] / [ -f ~/.bashrc ] checks so missing shells are silently skipped
• Guard fish blocks behind command_exists fish instead of checking config file existence, avoiding creation of fish config dirs on non-fish systems
• Soften ~/.shelltime/daemon and ~/.shelltime/hooks mkdir failures from fatal errors to warnings, allowing partial installation to succeed

Test plan

• Run bash ./install.bash on a machine without fish installed — should complete without errors
• Run bash ./install.bash on a machine without zsh — should complete without errors
• Run bash ./install.bash on a machine with all shells — all configs should be updated as before
• Verify ~/.shelltime/bin mkdir failure still aborts (essential directory)

🤖 Generated with Claude Code

---

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the robustness and user experience of the installation script by making it more resilient to environments where not all target shells (Zsh, Bash, Fish) are installed or configured. It achieves this by introducing conditional logic for applying shell-specific configurations and by softening error handling for non-critical directory creations, ensuring a smoother installation process across diverse system setups.

Highlights

• Conditional Shell Configuration: Implemented checks to ensure shell-specific configurations (PATH and hook sourcing) are only applied if the respective shell's configuration file exists (for Zsh and Bash) or if the shell command is available (for Fish).
• Improved Error Handling: Converted fatal errors for creating ~/.shelltime/daemon and ~/.shelltime/hooks directories into warnings, allowing the installation to proceed partially even if these non-essential directories cannot be created.
• Fish Configuration Logic Refinement: Modified the Fish shell configuration logic to use command_exists fish instead of checking for config file existence, preventing the creation of unnecessary Fish configuration directories on systems without Fish.

Changelog

• install.bash
  • Modified error handling for ~/.shelltime/daemon directory creation from a fatal error to a warning.
  • Added a file existence check ([ -f "$HOME/.zshrc" ]) before attempting to modify the Zsh configuration.
  • Updated the Fish configuration logic to check for the fish command's existence (command_exists fish) instead of relying on config file presence.
  • Added a file existence check ([ -f "$HOME/.bashrc" ]) before attempting to modify the Bash configuration.
  • Modified error handling for ~/.shelltime/hooks directory creation from a fatal error to a warning.
  • Introduced file existence checks for .zshrc, config.fish, and .bashrc before adding source lines to them.

Activity

• No human activity (comments, reviews, etc.) has been recorded on this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request significantly improves the robustness of the installation script. It correctly adds checks to ensure shell configuration files exist before attempting to modify them, preventing errors on systems where certain shells are not installed. The change to use command_exists fish is a smart way to handle the fish shell without creating unnecessary configuration files. Downgrading directory creation failures to warnings for non-essential directories also makes the installation process more resilient. My feedback includes a suggestion to improve consistency in one of the new checks.

[gemini-code-assist]

For consistency with the fish PATH setup logic (lines 149-160) and the PR description ("Guard fish blocks behind command_exists fish"), it would be clearer to use command_exists fish here as well. While the current file existence check works because the PATH setup section earlier in the script creates config.fish if fish is installed, using the same type of check improves readability and makes the intent more explicit.

Suggested change

	if [ -f "$HOME/.config/fish/config.fish" ]; then
	add_source_to_config "$HOME/.config/fish/config.fish" "${hooks_path}/fish.fish"
	fi
	if command_exists fish; then
	add_source_to_config "$HOME/.config/fish/config.fish" "${hooks_path}/fish.fish"
	fi

[claude]

Code Review

Overall this is a good fix for a real bug — the original script would error out (or silently corrupt output) when shell config files didn't exist. The approach is sound. A few issues worth addressing before merging:

---

Bug: hooks_path mkdir failure is no longer safe to ignore

File: install.bash, around line 196

if [ ! -d "$hooks_path" ]; then
    mkdir -p "$hooks_path"
    if [ $? -ne 0 ]; then
        echo "Warning: Failed to create $hooks_path directory. Shell hooks may be unavailable."
    fi
fi

The change from exit 1 to a warning is risky here. Unlike the daemon directory, hooks_path is essential — the next ~50 lines download hook files into it (process_file "bash.bash" ...) and then add source lines to shell configs pointing to it. If the directory can't be created, those subsequent steps will either fail with confusing errors or silently write broken source paths into the user's shell configs.

Suggestion: Either keep exit 1 for the hooks directory (it's essential, unlike daemon), or set a flag and skip the hook-related steps:

HOOKS_OK=true
if [ ! -d "$hooks_path" ]; then
    mkdir -p "$hooks_path"
    if [ $? -ne 0 ]; then
        echo "Warning: Failed to create $hooks_path directory. Shell hooks will be skipped."
        HOOKS_OK=false
    fi
fi

# ... later ...
if $HOOKS_OK; then
    process_file "zsh.zsh" ...
    process_file "bash.bash" ...
    # etc.
fi

The daemon directory softening is fine since it's optional functionality.

---

Inconsistency: fish detection uses different strategy in two places

In the PATH-configuration block, fish is gated on command_exists fish:

if command_exists fish; then
    ...create config.fish if needed...
fi

But in the hook-sourcing block at the bottom, it's gated on file existence:

if [ -f "$HOME/.config/fish/config.fish" ]; then
    add_source_to_config "$HOME/.config/fish/config.fish" "${hooks_path}/fish.fish"
fi

In practice this works (the PATH block creates config.fish when fish is installed, so the file will exist by the time we reach the source block), but the inconsistency makes the logic harder to reason about. Using command_exists fish in both places would make the intent explicit and symmetric with the zsh/bash pattern.

---

Minor: zsh and bash configs are silently skipped but not created

Fish proactively creates its config dir/file when fish is installed. Zsh and bash skip silently if .zshrc/.bashrc don't exist. This is probably fine (it's unusual to have zsh installed with no .zshrc), but it could confuse users who run the installer on a fresh system and find shelltime not configured for their shell.

Consider logging a notice when skipping:

if [ -f "$HOME/.zshrc" ]; then
    ...configure zsh...
else
    echo "Note: ~/.zshrc not found, skipping zsh configuration."
fi

This way the user knows they need to create the file and re-run the installer.

---

Positive notes

• The command_exists fish guard in the PATH block is the right call — it prevents creating fish config dirs on non-fish systems, which was the original bug.
• The duplicate-entry guards (grep -q) are preserved correctly in all three shell blocks.
• The test plan is clear and covers the important cases.

---

Summary: The core logic is correct. The main concern is the hooks_path warning — that directory is load-bearing for the rest of the script and silently continuing without it will cause confusing downstream failures. The fish inconsistency is a minor readability issue. The rest looks good.

[devin-ai-integration]

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

[devin-ai-integration]

🔴 Removing exit on hooks_path creation failure allows broken source lines to be added to shell configs

When mkdir -p "$hooks_path" fails at line 198, the old code called exit 1, preventing any further damage. The new code prints a warning and continues. The script then proceeds to: (1) attempt mkdir -p again at install.bash:238 (which also fails), (2) call process_file at install.bash:245-252 where curl -o fails because the directory doesn't exist so no hook files are written, and (3) call add_source_to_config at install.bash:255-263 which adds source ~/.shelltime/hooks/zsh.zsh (and similar) lines to the user's shell config files — pointing to files that don't exist. This causes error messages on every new shell session (e.g., no such file or directory from zsh/bash source). In shells configured with set -e or setopt err_exit, this could abort shell initialization entirely.

Prompt for agents

In install.bash, after the warning at line 199 about failing to create the hooks_path directory, the script should either (a) exit 1 as before, or (b) set a flag (e.g., hooks_failed=true) and check that flag before the add_source_to_config calls at lines 255-263, skipping those calls if hooks directory creation failed. The simplest fix is to restore the exit 1 at line 199, but if the intent is to be graceful, then guard lines 238-263 with a check like: if [ -d "$hooks_path" ]; then ... fi, or add early returns/skips when the directory doesn't exist. The key invariant to maintain: never add source lines to shell configs unless the corresponding hook files actually exist.

---

Was this helpful? React with 👍 or 👎 to provide feedback.