fix: guard worktrunk init and globals - #758
Conversation
shunkakinoki
added
the
bug
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdates home-manager configurations to add defensive guards around optional tool initialization (Worktrunk/wt command) across multiple shells, while improving PATH and environment variable exposure for Rust and Bun development tools. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing touches🧪 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 |
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! 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 consistency of the development environment setup. It achieves this by making Highlights
Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Mesa DescriptionTL;DRGuarded What changed?
Description generated by Mesa. Update settings |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![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 introduces several good improvements for environment setup and robustness. It guards the worktrunk shell initialization across bash, fish, and zsh to prevent errors when the command is not present. It also ensures the stable rust toolchain is installed and set as the default during activation, and correctly exports BUN_INSTALL for consistent bun paths. The changes are logical and well-implemented. I have one suggestion to improve the PATH configuration for Rust to better align with the use of rustup and avoid potential toolchain conflicts.
| export PATH=${pkgs.rustup}/bin:${pkgs.cargo}/bin:${pkgs.rustc}/bin:${pkgs.dasel}/bin:${pkgs.jq}/bin:${pkgs.gcc}/bin:${pkgs.pkg-config}/bin:$PATH | ||
| export CARGO_HOME="$HOME/.cargo" |
There was a problem hiding this comment.
Since you're using rustup to manage toolchains, it's better to rely on it to provide the correct cargo and rustc binaries via its shims in $CARGO_HOME/bin. Explicitly adding ${pkgs.cargo}/bin and ${pkgs.rustc}/bin to the PATH can lead to conflicts between the nix-provided packages and the rustup-managed toolchain.
I suggest defining CARGO_HOME first and then adding its bin directory to the PATH, removing the direct dependencies on pkgs.cargo and pkgs.rustc from the PATH for this activation script.
export CARGO_HOME="$HOME/.cargo"
export PATH="$CARGO_HOME/bin:${pkgs.rustup}/bin:${pkgs.dasel}/bin:${pkgs.jq}/bin:${pkgs.gcc}/bin:${pkgs.pkg-config}/bin:$PATH
There was a problem hiding this comment.
Pull request overview
This PR hardens shell initialization and global tool setup in the home-manager configuration, aiming to prevent failures when optional tools aren’t installed and to make bun/cargo globals more consistent across shells.
Changes:
- Guard
wt config shell init …so shells don’t error whenwtis absent (bash/zsh), and move fish init into interactive context after PATH setup. - Export
BUN_INSTALLviahome.sessionVariablesto standardize bun global paths. - Add rustup “stable” toolchain install/default steps to the cargo globals activation.
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 |
|---|---|
| home-manager/programs/zsh/default.nix | Wrap worktrunk init in a wt presence check to avoid shell startup errors. |
| home-manager/programs/fish/default.nix | Move worktrunk init to interactiveShellInit and guard it with type -q wt. |
| home-manager/programs/bash/default.nix | Wrap worktrunk init in a wt presence check to avoid shell startup errors. |
| home-manager/modules/npm-globals/default.nix | Add BUN_INSTALL to home.sessionVariables for consistent bun globals. |
| home-manager/modules/cargo-globals/default.nix | Modify activation PATH and add rustup stable toolchain install/default steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export PATH=${pkgs.rustup}/bin:${pkgs.cargo}/bin:${pkgs.rustc}/bin:${pkgs.dasel}/bin:${pkgs.jq}/bin:${pkgs.gcc}/bin:${pkgs.pkg-config}/bin:$PATH | ||
| export CARGO_HOME="$HOME/.cargo" | ||
| $DRY_RUN_CMD ${pkgs.rustup}/bin/rustup toolchain install stable | ||
| $DRY_RUN_CMD ${pkgs.rustup}/bin/rustup default stable | ||
| export PKG_CONFIG_PATH="${pkgs.openssl.dev}/lib/pkgconfig${libiconvPkgConfigPath}''${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}" |
There was a problem hiding this comment.
The activation script installs/sets a rustup toolchain (rustup toolchain install stable / rustup default stable), but the subsequent installs are performed with ${pkgs.cargo}/bin/cargo + ${pkgs.rustc}/bin/rustc from the PATH. That means these rustup commands don’t actually ensure the toolchain used for cargo install, while still introducing network/stateful side effects and a potential home-manager switch failure when offline.
Consider either (a) removing the rustup toolchain/default steps, or (b) explicitly running cargo via rustup (e.g., rustup run stable cargo … / setting RUSTUP_TOOLCHAIN=stable) and guarding the toolchain install so it’s idempotent and doesn’t hard-fail when the toolchain can’t be fetched.
Changes\n- Guard worktrunk shell init across fish/bash/zsh\n- Ensure rustup stable toolchain for cargo globals\n- Export BUN_INSTALL via home session vars\n\n## Technical Details\n- Move worktrunk init to fish interactive shell after PATH setup\n- Run rustup toolchain install/default in cargo globals activation\n- Set BUN_INSTALL for consistent bun global paths\n\n## Testing\n- make build && make switch\n\nGenerated with GitHub Copilot CLI by GPT-5
Summary by cubic
Guarded Worktrunk shell initialization and hardened global toolchains to prevent shell errors and ensure consistent paths. Adds rustup stable toolchain for cargo globals and sets BUN_INSTALL for predictable bun installs.
Written for commit 5cbc1db. Summary will update on new commits.