feat: add fish wrapper functions for local scripts and move yek to cargo globals - #1358
Conversation
- Add StopFailure hook that detects rate limit errors and runs cswap --switch to rotate to the next Claude account - Add claude-swap to uv global tools (pyproject.toml) - Hook reads error/error_details from stdin, only triggers on rate_limit/429/overloaded/quota patterns
…rgo globals - Add fish functions wrapping sync-local-binaries, install-cargo-globals, install-npm-globals, install-uv-globals, update-local-binaries, llm-update - Register abbreviations: slb, icg, ing, iug, ulb, llmu - Move yek from custom GitHub release installer to Cargo.toml (yek 0.22.1) - Remove yek module, shim scripts, and related specs - Add auto-switch.sh coverage entry and spec
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (28)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR removes the Home Manager Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mesa DescriptionTL;DRAdded fish wrapper functions for frequently used local scripts, streamlined What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements an automatic Claude account switcher for rate-limit handling via a new hook script and the claude-swap utility. It also replaces the custom yek module with a direct dependency and adds several Fish shell functions for managing global packages and local binaries. Reviewers suggested improving the hook script's robustness by checking for jq and using herestrings, and recommended directing error output to stderr in the new Fish functions.
| if ! command -v cswap &>/dev/null; then | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
The script relies on jq to parse the hook input. It is safer to check for its availability alongside cswap to avoid silent failures if jq is missing in the environment.
| if ! command -v cswap &>/dev/null; then | |
| exit 0 | |
| fi | |
| if ! command -v cswap &>/dev/null || ! command -v jq &>/dev/null; then | |
| exit 0 | |
| fi |
| ERROR=$(echo "$INPUT" | jq -r '.error // empty' 2>/dev/null) || true | ||
| ERROR_DETAILS=$(echo "$INPUT" | jq -r '.error_details // empty' 2>/dev/null) || true |
There was a problem hiding this comment.
Using herestrings (<<< "$INPUT") is generally preferred over echo "$INPUT" | jq as it avoids a subshell and is safer if the input starts with a hyphen (which echo might interpret as a flag).
| ERROR=$(echo "$INPUT" | jq -r '.error // empty' 2>/dev/null) || true | |
| ERROR_DETAILS=$(echo "$INPUT" | jq -r '.error_details // empty' 2>/dev/null) || true | |
| ERROR=$(jq -r '.error // empty' 2>/dev/null <<< "$INPUT") || true | |
| ERROR_DETAILS=$(jq -r '.error_details // empty' 2>/dev/null <<< "$INPUT") || true |
| function _install_cargo_globals_function --description "Install global Cargo packages" | ||
| set -l script "$HOME/dotfiles/home-manager/modules/cargo-globals/install-cargo-globals.sh" | ||
| if not test -f "$script" | ||
| echo "install-cargo-globals.sh not found at $script" |
There was a problem hiding this comment.
Error messages should be directed to stderr (>&2) to distinguish them from regular command output. This applies to the other new fish functions as well.
echo "install-cargo-globals.sh not found at $script" >&2
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds fish-shell convenience wrappers for several local maintenance scripts, introduces an auto-switch hook for Claude rate-limit failures, and simplifies installation of yek by using Cargo globals instead of a custom GitHub-release installer.
Changes:
- Add fish functions + abbreviations wrapping 6 local scripts (
slb,icg,ing,iug,ulb,llmu) and fish tests for missing-script behavior - Add
auto-switch.shhook + spec + coverage entry; addclaude-swapdependency - Remove the custom
yekNix module + installer/shim scripts and related specs; addyek = "0.22.1"toCargo.toml
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/yek_spec.sh | Removes specs for the deleted custom yek installer + wrapper scripts |
| spec/yek_shim_spec.sh | Removes specs for the deleted yek shim |
| spec/install_yek_shim_spec.sh | Removes specs for the deleted install-yek shim |
| spec/fish/_update_local_binaries_function_test.fish | Adds fish test for _update_local_binaries_function missing-script behavior |
| spec/fish/_sync_local_binaries_function_test.fish | Adds fish test for _sync_local_binaries_function missing-script behavior |
| spec/fish/_llm_update_function_test.fish | Adds fish test for _llm_update_function missing-script behavior |
| spec/fish/_install_uv_globals_function_test.fish | Adds fish test for _install_uv_globals_function missing-script behavior |
| spec/fish/_install_npm_globals_function_test.fish | Adds fish test for _install_npm_globals_function missing-script behavior |
| spec/fish/_install_cargo_globals_function_test.fish | Adds fish test for _install_cargo_globals_function missing-script behavior |
| spec/coverage_spec.sh | Updates coverage list/spec presence checks: adds auto-switch.sh, removes yek scripts |
| spec/auto_switch_spec.sh | Adds a spec for auto-switch.sh “no cswap => exit 0” behavior |
| pyproject.toml | Adds claude-swap dependency used by the new hook |
| home-manager/programs/fish/functions/_update_local_binaries_function.fish | Adds fish wrapper to run update-local-binaries.sh |
| home-manager/programs/fish/functions/_sync_local_binaries_function.fish | Adds fish wrapper to run sync-local-binaries.sh |
| home-manager/programs/fish/functions/_llm_update_function.fish | Adds fish wrapper to run llm-update.sh |
| home-manager/programs/fish/functions/_install_uv_globals_function.fish | Adds fish wrapper to run install-uv-globals.sh |
| home-manager/programs/fish/functions/_install_npm_globals_function.fish | Adds fish wrapper to run install-npm-globals.sh |
| home-manager/programs/fish/functions/_install_cargo_globals_function.fish | Adds fish wrapper to run install-cargo-globals.sh |
| home-manager/programs/fish/default.nix | Wires new fish functions into abbreviations and function list |
| home-manager/modules/yek/yek.sh | Deletes the custom yek wrapper (now installed via Cargo globals) |
| home-manager/modules/yek/yek-shim.sh | Deletes the custom yek shim |
| home-manager/modules/yek/install-yek.sh | Deletes the GitHub-release installer script for yek |
| home-manager/modules/yek/install-yek-shim.sh | Deletes the custom install-yek shim |
| home-manager/modules/yek/default.nix | Deletes the custom Nix module that installed/activated yek |
| home-manager/modules/default.nix | Removes the yek module from the modules list |
| config/claude/settings.json | Adds StopFailure hook to run auto-switch.sh |
| config/claude/hooks/auto-switch.sh | Adds hook that switches accounts on rate-limit-like errors |
| Cargo.toml | Adds yek = "0.22.1" as a Cargo global dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fi | ||
|
|
||
| # Require at least 2 managed accounts | ||
| ACCOUNT_COUNT=$(cswap --list 2>/dev/null | grep -c '^\s*[0-9]' || echo 0) |
There was a problem hiding this comment.
grep -c '^\s*[0-9]' is not doing what it looks like: \s isn’t a portable whitespace escape in grep BRE/ERE, so it will typically be treated as a literal s. Also, grep -c prints 0 even when it exits 1 (no matches); combined with set -o pipefail, the || echo 0 branch can run and append a second 0, producing a non-numeric ACCOUNT_COUNT (e.g., 0\n0) and breaking the -lt comparison. Use a portable whitespace class (e.g. [[:space:]]) and avoid appending output on “no matches” (e.g. || true), ensuring the command substitution always evaluates to a single integer.
| ACCOUNT_COUNT=$(cswap --list 2>/dev/null | grep -c '^\s*[0-9]' || echo 0) | |
| ACCOUNT_COUNT=$(cswap --list 2>/dev/null | grep -c '^[[:space:]]*[0-9]' || true) |
| case "${ERROR}${ERROR_DETAILS}" in | ||
| *rate_limit*|*rate-limit*|*rate\ limit*|*overloaded*|*too_many_requests*|*429*|*quota*|*capacity*) | ||
| echo "[$(date)] Auto-switching Claude account due to rate limit" >&2 | ||
| cswap --switch 2>&1 | head -5 >&2 |
There was a problem hiding this comment.
With set -euo pipefail, a failing cswap --switch (or a broken pipe from head) can cause this hook itself to exit non-zero, which is risky for a “best effort” remediation hook triggered during failures. Consider making the switch attempt explicitly non-fatal (e.g., append || true, or temporarily disable -e/pipefail around the pipeline) so the hook doesn’t introduce additional failure modes.
| cswap --switch 2>&1 | head -5 >&2 | |
| cswap --switch 2>&1 | head -5 >&2 || true |
| # Mock command so cswap is not found | ||
| command() { return 1; } |
There was a problem hiding this comment.
The command() { return 1; } definition on line 9 is not used by the When run ... invocation (which runs in a separate bash -c and redefines command again). Removing the outer definition avoids confusion and keeps the spec focused on the behavior under test.
| # Mock command so cswap is not found | |
| command() { return 1; } |
Summary
slb,icg,ing,iug,ulb,llmuCargo.toml(yek = "0.22.1") - yek is on crates.io, no need for a custom download scriptTest plan
make shell-lintpasses (0 errors)make shell-testpasses (914 shellspec examples, 265 fish tests)Summary by cubic
Add fish wrapper functions for six local scripts with short abbreviations, and add an auto-switch for Claude accounts on rate limits via
claude-swap. Moveyekto a Cargo-managed dependency and remove the custom installer and related tests.New Features
slb(sync-local-binaries),icg(install-cargo-globals),ing(install-npm-globals),iug(install-uv-globals),ulb(update-local-binaries),llmu(llm-update).$HOME/.claude/hooks/auto-switch.shto callcswap --switchwhen 429/rate-limit patterns are detected (requirescswapand ≥2 accounts).Refactors
yekinstaller with Cargo depyek = "0.22.1"; removehome-manager/modules/yekshims and specs.claude-swap>=1.1.5topyproject.toml; update coverage and add specs for the new hook and fish wrappers.Written for commit b2f5e1f. Summary will update on new commits.