feat: update Clawdbot and Codex configurations - #633
Conversation
…talled package checks
…S bundle attributes and adjust launch command
… and function descriptions
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
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 significantly refines the system's configuration and tooling, focusing on improved application deployment, shell environment management, and build process automation. It addresses issues with macOS application bundle integrity, standardizes command execution for the Codex tool, and expands the build system's capabilities to include Go projects. These changes aim to enhance the reliability and consistency of the development environment. Highlights
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
|
Mesa DescriptionTL;DRUpdated Clawdbot and Codex configurations, including improved macOS bundle handling, What changed?
Description generated by Mesa. Update settings |
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR introduces multiple coordinated enhancements: updates Codex profile naming conventions (glm-4.7-flash → glm-4-7-flash) with corresponding function invocation changes, adds Go module support to the local binaries build script, enhances cargo globals installation with version caching via INSTALLED_MAP, improves macOS application bundle preservation for Clawdbot, and adds conditional Homebrew initialization for Fish and Zsh shells. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
![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 updates and fixes across configurations and scripts. Key changes include updating Clawdbot installation to use ditto for better macOS app handling, switching Codex functions to use the exec subcommand, and adding Homebrew environment loading for bash, fish, and zsh shells. The cargo global installation script is improved to handle more version formats and to more efficiently check for installed packages. Additionally, the local binary update script now supports building Go projects.
My review focuses on improving maintainability and performance by reducing code duplication in shell configurations and optimizing script logic. I've suggested refactoring in a few shell scripts to make them more efficient and easier to read.
| CRATE=$(echo "$pkg" | cut -d'@' -f1) | ||
| VERSION=$(echo "$pkg" | cut -d'@' -f2) |
There was a problem hiding this comment.
For better performance and to follow shell scripting best practices, consider using shell parameter expansion to extract the crate name and version instead of forking cut twice in a loop. This avoids creating subshells for each dependency.
| CRATE=$(echo "$pkg" | cut -d'@' -f1) | |
| VERSION=$(echo "$pkg" | cut -d'@' -f2) | |
| CRATE=${pkg%@*} | |
| VERSION=${pkg#*@} |
| # OpenSSL for cargo builds on Linux (available in login shells) | ||
| if [ "$(uname)" = "Linux" ]; then | ||
| export XDG_RUNTIME_DIR="/run/user/$(id -u)" | ||
| export PKG_CONFIG_PATH="${pkgs.openssl.dev}/lib/pkgconfig''${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}" | ||
| export OPENSSL_DIR="${pkgs.openssl.dev}" | ||
| export OPENSSL_LIB_DIR="${pkgs.openssl.out}/lib" | ||
| export OPENSSL_INCLUDE_DIR="${pkgs.openssl.dev}/include" | ||
| fi |
There was a problem hiding this comment.
This block of code, which sets up OpenSSL environment variables for Linux, is a duplicate of the logic already present in bashrcExtra (lines 39-44). Since profileExtra now sources ~/.bashrc (lines 82-84), these settings will be applied for login shells anyway. To avoid redundancy and improve maintainability, this duplicated block can be removed.
| if test -f /opt/homebrew/bin/brew | ||
| eval "$(/opt/homebrew/bin/brew shellenv)" | ||
| end |
There was a problem hiding this comment.
The Homebrew environment setup is duplicated in both loginShellInit and interactiveShellInit. Since a login shell is typically also interactive, this can lead to eval "$(/opt/homebrew/bin/brew shellenv)" being executed twice, which is unnecessary and can slightly slow down shell startup.
To avoid this duplication, you could move this block to shellInit (around line 25), which is sourced only once for all shell sessions. This would ensure the Homebrew environment is set up correctly and efficiently for all shell types.
| if [ -d "$repo_dir/cmd/$repo_name" ]; then | ||
| if (cd "$repo_dir" && go build "./cmd/$repo_name" 2>&1); then | ||
| return 0 | ||
| else | ||
| return 1 | ||
| fi | ||
| else | ||
| if (cd "$repo_dir" && go build 2>&1); then | ||
| return 0 | ||
| else | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
The logic for building a Go project can be refactored to be more concise and less repetitive. You can determine the build path first and then have a single go build command.
| if [ -d "$repo_dir/cmd/$repo_name" ]; then | |
| if (cd "$repo_dir" && go build "./cmd/$repo_name" 2>&1); then | |
| return 0 | |
| else | |
| return 1 | |
| fi | |
| else | |
| if (cd "$repo_dir" && go build 2>&1); then | |
| return 0 | |
| else | |
| return 1 | |
| fi | |
| local build_path="." | |
| if [ -d "$repo_dir/cmd/$repo_name" ]; then | |
| build_path="./cmd/$repo_name" | |
| fi | |
| if (cd "$repo_dir" && go build "$build_path" 2>&1); then | |
| return 0 | |
| else | |
| return 1 | |
| fi |
There was a problem hiding this comment.
Pull request overview
This PR updates several configurations related to Clawdbot installation, Codex command usage, and shell environment setup. The changes focus on improving macOS bundle handling, adding Go build support, and standardizing Homebrew environment loading across shells.
Changes:
- Improved macOS bundle handling for Clawdbot.app with ditto/cp -pR to preserve bundle attributes
- Updated Codex commands to use 'exec' subcommand and fixed glm-4-7-flash model naming
- Added Homebrew environment loading to bash, fish, and zsh shells
- Enhanced cargo-globals script with table-style dependency version support
- Added Go build support to local binaries update script
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/test_helpers.sh | New test helper functions for mock binaries and Nix script preprocessing |
| spec/spec_helper.sh | Updated to import new test_helpers instead of custom_matcher |
| spec/update_local_binaries_spec.sh | Added tests for Go build support with go.mod detection |
| spec/cargo_globals_spec.sh | Added tests for table-style dependency versions and installed version map |
| scripts/update-local-binaries.sh | Added Go build support with cmd/{repo_name} pattern detection |
| overlays/default.nix | Changed cp -R to cp -pR for Clawdbot.app installation |
| nix-darwin/default.nix | Changed from cp -R to ditto for proper macOS bundle attribute preservation |
| home-manager/modules/clawdbot/default.nix | Updated launchd to use 'open' command instead of direct binary execution |
| config/codex/config.toml | Renamed profile from glm-4.7-flash to glm-4-7-flash |
| home-manager/programs/fish/functions/_coxel_function.fish | Updated to use 'codex exec' and corrected model name |
| home-manager/programs/fish/functions/_coxelh_function.fish | Updated to use 'codex exec' and corrected model name |
| home-manager/programs/fish/functions/_coxe_function.fish | Updated to use 'codex exec' |
| home-manager/programs/fish/functions/_coxeh_function.fish | Updated to use 'codex exec' |
| home-manager/programs/fish/default.nix | Added Homebrew environment loading in loginShellInit and interactiveShellInit |
| home-manager/programs/bash/default.nix | Added Homebrew loading and OpenSSL configuration in profileExtra |
| home-manager/programs/zsh/default.nix | Added Homebrew loading in envExtra |
| home-manager/modules/cargo-globals/install-cargo-globals.sh | Enhanced to support table-style dependency versions and use associative array for installed packages |
| .local-binaries.txt | Added multiclaude binary entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| exit 1 | ||
| fi | ||
| cp -R "$app_path" "$out/Applications/Clawdbot.app" | ||
| cp -pR "$app_path" "$out/Applications/Clawdbot.app" |
There was a problem hiding this comment.
While cp -pR preserves permissions and timestamps, it doesn't preserve macOS extended attributes and resource forks which are critical for Bundle.module lookups. Consider using ditto here as well (similar to the fix in nix-darwin/default.nix) or using rsync -a --xattrs to ensure all macOS bundle attributes are preserved.
| cp -pR "$app_path" "$out/Applications/Clawdbot.app" | |
| ditto "$app_path" "$out/Applications/Clawdbot.app" |
| elif [ -f "$repo_dir/go.mod" ]; then | ||
| # Go project: build ./cmd/{repo_name} if it exists, otherwise build root | ||
| if [ -d "$repo_dir/cmd/$repo_name" ]; then | ||
| if (cd "$repo_dir" && go build "./cmd/$repo_name" 2>&1); then |
There was a problem hiding this comment.
The go build command for cmd/{repo_name} pattern doesn't specify an output location, which means the binary will be placed in the cmd/{repo_name} directory rather than at the repository root where other build systems typically place their outputs. Consider using 'go build -o {repo_name} ./cmd/{repo_name}' to place the output binary at the repository root for consistency with Rust and Make builds.
| if (cd "$repo_dir" && go build "./cmd/$repo_name" 2>&1); then | |
| if (cd "$repo_dir" && go build -o "$repo_name" "./cmd/$repo_name" 2>&1); then |
| Label = "com.clawdbot.app"; | ||
| ProgramArguments = [ | ||
| "/Applications/Clawdbot.app/Contents/MacOS/Clawdbot" | ||
| "open" |
There was a problem hiding this comment.
Using 'open' as a direct command in ProgramArguments may not work as expected in launchd. The 'open' command typically requires a full path. Consider using '/usr/bin/open' instead of just 'open' to ensure launchd can find and execute the command reliably.
| "open" | |
| "/usr/bin/open" |
| if test -f /opt/homebrew/bin/brew | ||
| eval "$(/opt/homebrew/bin/brew shellenv)" | ||
| end | ||
|
|
There was a problem hiding this comment.
The Homebrew environment loading is duplicated in both loginShellInit and interactiveShellInit. This could lead to unnecessary overhead and potential issues. The brew shellenv should typically only be loaded once, preferably in loginShellInit.
| if test -f /opt/homebrew/bin/brew | |
| eval "$(/opt/homebrew/bin/brew shellenv)" | |
| end |
|
|
||
| [profiles.glm-4.7-flash] | ||
| [profiles.glm-4-7-flash] | ||
| model = "zai-org/glm-4.7-flash" |
There was a problem hiding this comment.
The profile name was changed to 'glm-4-7-flash' (with hyphens), but the model name itself is still 'zai-org/glm-4.7-flash' (with a dot). This inconsistency could cause confusion. Consider updating the model name to use hyphens consistently, i.e., 'zai-org/glm-4-7-flash'.
| model = "zai-org/glm-4.7-flash" | |
| model = "zai-org/glm-4-7-flash" |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Changes
Technical Details
Testing
Generated with opencode by claude-3-7-sonnet
Summary by cubic
Improved Clawdbot macOS app handling, switched Codex functions to use exec, and loaded Homebrew in shells to make tools work reliably. Added Go build support and more robust cargo global version parsing.
New Features
Bug Fixes
Written for commit 421d357. Summary will update on new commits.