feat: update clawdbot and pi configurations - #675
Conversation
|
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 focuses on enhancing the configuration of AI models and improving the reliability of local development environment setup. It updates model definitions for agent interactions and standardizes API key usage across configurations. Additionally, it significantly upgrades a critical shell script to prevent issues arising from uncommitted changes during repository updates, ensuring a smoother and more consistent development workflow. 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 What changed?
Description generated by Mesa. Update settings |
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughConfiguration files update the default agent model to use cliproxy-based providers with fallback options and reference an environment variable for API key. A shell script adds conditional logic to handle dirty working trees by performing a reset before pulling remote updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
The pull request introduces updates to configuration files for clawdbot and pi models, and enhances the update-local-binaries.sh script. The configuration changes correctly update model references and improve API key handling by using placeholders, which is a good practice. The script update adds robustness by handling dirty Git working trees, which is a significant improvement for ensuring clean updates.
| if (cd "$repo_dir" && [ -n "$(git status --porcelain)" ]); then | ||
| log_warn " Working tree dirty; resetting to remote (destructive)..." | ||
| if ! ( | ||
| cd "$repo_dir" && | ||
| git fetch --prune 2>&1 && | ||
| upstream_ref="$(git rev-parse --abbrev-ref --symbolic-full-name "@{u}" 2>/dev/null || true)" && | ||
| if [ -n "$upstream_ref" ]; then | ||
| git reset --hard "$upstream_ref" 2>&1 | ||
| else | ||
| git reset --hard HEAD 2>&1 | ||
| fi && | ||
| git clean -fd 2>&1 | ||
| ); then | ||
| log_error " Reset to remote failed" | ||
| FAILURES+=("$repo_name (reset failed)") | ||
| return 1 |
There was a problem hiding this comment.
The script now performs a destructive reset (git reset --hard and git clean -fd) if the working tree is dirty. While this ensures a clean update, it can lead to data loss for users with uncommitted local changes. It might be beneficial to add an option to skip this destructive step or prompt the user before proceeding, especially since the log_warn message is just a warning and doesn't require user confirmation. Consider making this behavior configurable or adding an interactive prompt.
| if (cd "$repo_dir" && [ -n "$(git status --porcelain)" ]); then | ||
| log_warn " Working tree dirty; resetting to remote (destructive)..." | ||
| if ! ( | ||
| cd "$repo_dir" && | ||
| git fetch --prune 2>&1 && | ||
| upstream_ref="$(git rev-parse --abbrev-ref --symbolic-full-name "@{u}" 2>/dev/null || true)" && | ||
| if [ -n "$upstream_ref" ]; then | ||
| git reset --hard "$upstream_ref" 2>&1 | ||
| else | ||
| git reset --hard HEAD 2>&1 | ||
| fi && | ||
| git clean -fd 2>&1 | ||
| ); then | ||
| log_error " Reset to remote failed" | ||
| FAILURES+=("$repo_name (reset failed)") | ||
| return 1 | ||
| fi | ||
|
|
||
| if ! (cd "$repo_dir" && git pull 2>&1); then | ||
| log_error " Git pull failed" | ||
| FAILURES+=("$repo_name (git pull failed)") | ||
| return 1 | ||
| fi | ||
| else | ||
| if ! (cd "$repo_dir" && git pull 2>&1); then | ||
| log_error " Git pull failed" | ||
| FAILURES+=("$repo_name (git pull failed)") | ||
| return 1 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The git pull command is duplicated in both the if and else branches of the dirty working tree check. This can be refactored to reduce duplication and improve readability by moving the git pull to a single block after the dirty check and reset logic.
if (cd "$repo_dir" && [ -n "$(git status --porcelain)" ]); then
log_warn " Working tree dirty; resetting to remote (destructive)..."
if ! (
cd "$repo_dir" &&
git fetch --prune 2>&1 &&
upstream_ref="$(git rev-parse --abbrev-ref --symbolic-full-name "@{u}" 2>/dev/null || true)" &&
if [ -n "$upstream_ref" ]; then
git reset --hard "$upstream_ref" 2>&1
else
git reset --hard HEAD 2>&1
fi &&
git clean -fd 2>&1
); then
log_error " Reset to remote failed"
FAILURES+=("$repo_name (reset failed)")
return 1
fi
fi
# Attempt git pull after handling dirty state
if ! (cd "$repo_dir" && git pull 2>&1); then
log_error " Git pull failed"
FAILURES+=("$repo_name (git pull failed)")
return 1
fi![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/update-local-binaries.sh`:
- Around line 164-191: The current logic in the block that runs inside the
repo_dir performs destructive operations (git reset --hard, git clean -fd)
whenever the working tree is dirty; add an explicit opt-in guard (e.g., an env
var like ALLOW_DESTRUCTIVE_RESET or a CLI flag) before performing the
reset/clean in the function that checks repo_dir, and if the opt-in is not set,
skip the destructive steps, log a clear warning via log_warn/log_error, append
the repo to FAILURES (e.g., "$repo_name (dirty, reset skipped)") and return
failure; ensure both the dirty-branch (where reset/clean currently runs) and the
non-dirty branch (git pull) respect this guard so no destructive action runs
without the opt-in.
🧹 Nitpick comments (1)
config/clawdbot/clawdbot.template.json (1)
294-298: Verify default model IDs match provider definitions.
primaryusescliproxy/claude-opus-4-5, but the provider list definesclaude-opus-4-5-20251101. If aliases aren’t supported, the default won’t resolve and fallback won’t engage. Please confirm alias support or switch to the exact model ID. Also, consider ordering the keys alphabetically (fallbacksbeforeprimary) for config consistency. As per coding guidelines, ...🔧 Example update if aliases aren’t supported
- "primary": "cliproxy/claude-opus-4-5", - "fallbacks": [ - "cliproxy/glm-4.7" - ] + "fallbacks": [ + "cliproxy/glm-4.7" + ], + "primary": "cliproxy/claude-opus-4-5-20251101"
| if (cd "$repo_dir" && [ -n "$(git status --porcelain)" ]); then | ||
| log_warn " Working tree dirty; resetting to remote (destructive)..." | ||
| if ! ( | ||
| cd "$repo_dir" && | ||
| git fetch --prune 2>&1 && | ||
| upstream_ref="$(git rev-parse --abbrev-ref --symbolic-full-name "@{u}" 2>/dev/null || true)" && | ||
| if [ -n "$upstream_ref" ]; then | ||
| git reset --hard "$upstream_ref" 2>&1 | ||
| else | ||
| git reset --hard HEAD 2>&1 | ||
| fi && | ||
| git clean -fd 2>&1 | ||
| ); then | ||
| log_error " Reset to remote failed" | ||
| FAILURES+=("$repo_name (reset failed)") | ||
| return 1 | ||
| fi | ||
|
|
||
| if ! (cd "$repo_dir" && git pull 2>&1); then | ||
| log_error " Git pull failed" | ||
| FAILURES+=("$repo_name (git pull failed)") | ||
| return 1 | ||
| fi | ||
| else | ||
| if ! (cd "$repo_dir" && git pull 2>&1); then | ||
| log_error " Git pull failed" | ||
| FAILURES+=("$repo_name (git pull failed)") | ||
| return 1 |
There was a problem hiding this comment.
Add an explicit opt‑in before destructive reset/clean.
This change now discards uncommitted changes and untracked files automatically when a repo is dirty. That’s a data‑loss risk for anyone who runs the script without realizing there are local changes. Please gate the reset behind an explicit flag/env var (or prompt) and fail otherwise.
🛡️ Suggested guard
- if (cd "$repo_dir" && [ -n "$(git status --porcelain)" ]); then
- log_warn " Working tree dirty; resetting to remote (destructive)..."
+ if (cd "$repo_dir" && [ -n "$(git status --porcelain)" ]); then
+ if [ "${UPDATE_LOCAL_BINARIES_FORCE:-}" != "1" ]; then
+ log_warn " Working tree dirty; skipping reset (set UPDATE_LOCAL_BINARIES_FORCE=1 to discard changes)"
+ FAILURES+=("$repo_name (dirty tree)")
+ return 1
+ fi
+ log_warn " Working tree dirty; resetting to remote (destructive)..."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (cd "$repo_dir" && [ -n "$(git status --porcelain)" ]); then | |
| log_warn " Working tree dirty; resetting to remote (destructive)..." | |
| if ! ( | |
| cd "$repo_dir" && | |
| git fetch --prune 2>&1 && | |
| upstream_ref="$(git rev-parse --abbrev-ref --symbolic-full-name "@{u}" 2>/dev/null || true)" && | |
| if [ -n "$upstream_ref" ]; then | |
| git reset --hard "$upstream_ref" 2>&1 | |
| else | |
| git reset --hard HEAD 2>&1 | |
| fi && | |
| git clean -fd 2>&1 | |
| ); then | |
| log_error " Reset to remote failed" | |
| FAILURES+=("$repo_name (reset failed)") | |
| return 1 | |
| fi | |
| if ! (cd "$repo_dir" && git pull 2>&1); then | |
| log_error " Git pull failed" | |
| FAILURES+=("$repo_name (git pull failed)") | |
| return 1 | |
| fi | |
| else | |
| if ! (cd "$repo_dir" && git pull 2>&1); then | |
| log_error " Git pull failed" | |
| FAILURES+=("$repo_name (git pull failed)") | |
| return 1 | |
| if (cd "$repo_dir" && [ -n "$(git status --porcelain)" ]); then | |
| if [ "${UPDATE_LOCAL_BINARIES_FORCE:-}" != "1" ]; then | |
| log_warn " Working tree dirty; skipping reset (set UPDATE_LOCAL_BINARIES_FORCE=1 to discard changes)" | |
| FAILURES+=("$repo_name (dirty tree)") | |
| return 1 | |
| fi | |
| log_warn " Working tree dirty; resetting to remote (destructive)..." | |
| if ! ( | |
| cd "$repo_dir" && | |
| git fetch --prune 2>&1 && | |
| upstream_ref="$(git rev-parse --abbrev-ref --symbolic-full-name "@{u}" 2>/dev/null || true)" && | |
| if [ -n "$upstream_ref" ]; then | |
| git reset --hard "$upstream_ref" 2>&1 | |
| else | |
| git reset --hard HEAD 2>&1 | |
| fi && | |
| git clean -fd 2>&1 | |
| ); then | |
| log_error " Reset to remote failed" | |
| FAILURES+=("$repo_name (reset failed)") | |
| return 1 | |
| fi | |
| if ! (cd "$repo_dir" && git pull 2>&1); then | |
| log_error " Git pull failed" | |
| FAILURES+=("$repo_name (git pull failed)") | |
| return 1 | |
| fi | |
| else | |
| if ! (cd "$repo_dir" && git pull 2>&1); then | |
| log_error " Git pull failed" | |
| FAILURES+=("$repo_name (git pull failed)") | |
| return 1 | |
| fi |
🤖 Prompt for AI Agents
In `@scripts/update-local-binaries.sh` around lines 164 - 191, The current logic
in the block that runs inside the repo_dir performs destructive operations (git
reset --hard, git clean -fd) whenever the working tree is dirty; add an explicit
opt-in guard (e.g., an env var like ALLOW_DESTRUCTIVE_RESET or a CLI flag)
before performing the reset/clean in the function that checks repo_dir, and if
the opt-in is not set, skip the destructive steps, log a clear warning via
log_warn/log_error, append the repo to FAILURES (e.g., "$repo_name (dirty, reset
skipped)") and return failure; ensure both the dirty-branch (where reset/clean
currently runs) and the non-dirty branch (git pull) respect this guard so no
destructive action runs without the opt-in.
There was a problem hiding this comment.
Pull request overview
Updates local tooling and bot/agent configurations to route model usage through CLIProxy and improve resilience.
Changes:
- Updated
clawdbotdefault model selection to use thecliproxyprovider and added a fallback model list. - Updated Pi agent provider configuration to use
CLIPROXY_API_KEYinstead of a hardcoded dummy value. - Enhanced the local binaries update script to hard-reset/clean repos when the working tree is dirty before pulling.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scripts/update-local-binaries.sh | Adds a “dirty tree” path that fetches, hard-resets, cleans, and then pulls. |
| config/pi/models.json | Switches cli-proxy-api provider apiKey value from dummy to CLIPROXY_API_KEY. |
| config/clawdbot/clawdbot.template.json | Changes default agent model to cliproxy/... and introduces model fallbacks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (cd "$repo_dir" && [ -n "$(git status --porcelain)" ]); then | ||
| log_warn " Working tree dirty; resetting to remote (destructive)..." | ||
| if ! ( | ||
| cd "$repo_dir" && | ||
| git fetch --prune 2>&1 && | ||
| upstream_ref="$(git rev-parse --abbrev-ref --symbolic-full-name "@{u}" 2>/dev/null || true)" && | ||
| if [ -n "$upstream_ref" ]; then | ||
| git reset --hard "$upstream_ref" 2>&1 | ||
| else | ||
| git reset --hard HEAD 2>&1 | ||
| fi && | ||
| git clean -fd 2>&1 | ||
| ); then |
There was a problem hiding this comment.
This new “dirty tree → fetch/reset/clean” behavior is significant and currently isn’t covered by the existing shellspec tests for this script (they only grep for git pull). Add a test using a temporary git repo to verify the intended behavior (e.g., dirty tree with upstream gets reset/cleaned, and dirty tree without upstream does not silently discard changes).
| "defaults": { | ||
| "model": { | ||
| "primary": "anthropic/claude-opus-4-5" | ||
| "primary": "cliproxy/claude-opus-4-5", |
There was a problem hiding this comment.
primary references cliproxy/claude-opus-4-5, but the cliproxy provider model list in this same template only defines claude-opus-4-5-20251101 and claude-opus-4-5-thinking (no claude-opus-4-5). Unless the runtime does aliasing, this will fail model resolution. Update primary (and any related defaults) to use an ID that actually exists in the provider list.
| "primary": "cliproxy/claude-opus-4-5", | |
| "primary": "cliproxy/claude-opus-4-5-20251101", |
| if (cd "$repo_dir" && [ -n "$(git status --porcelain)" ]); then | ||
| log_warn " Working tree dirty; resetting to remote (destructive)..." | ||
| if ! ( | ||
| cd "$repo_dir" && | ||
| git fetch --prune 2>&1 && | ||
| upstream_ref="$(git rev-parse --abbrev-ref --symbolic-full-name "@{u}" 2>/dev/null || true)" && | ||
| if [ -n "$upstream_ref" ]; then | ||
| git reset --hard "$upstream_ref" 2>&1 | ||
| else | ||
| git reset --hard HEAD 2>&1 | ||
| fi && |
There was a problem hiding this comment.
The dirty-working-tree path performs a destructive reset/clean even when the repo has no upstream configured (it falls back to git reset --hard HEAD). In that case the script can delete local changes but still fail on the subsequent git pull, and the log message “resetting to remote” becomes inaccurate. Consider only doing the hard reset when an upstream exists; otherwise abort with an explicit error (or require an opt-in flag/env var) before discarding changes.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Changes
Technical Details
Testing
Generated with opencode by glm-4.7
Summary by cubic
Switch clawdbot to use cliproxy with a fallback model, and update PI auth to use CLIPROXY_API_KEY. Harden the local binaries update script to reset dirty repos before pulling for more reliable updates.
Refactors
Migration
Written for commit 37340a5. Summary will update on new commits.