feat(scripts): accept profile name as argument to restart/dev scripts#426
Conversation
Previously the positional argument only accepted known provider names (anthropic, copilot, ollama…) and rejected anything else. Now if the argument is not a recognised provider it is treated as an ACECLAW_PROFILE value, so named profiles such as `claude-acn` or `claude` can be passed directly: aceclaw-restart claude-acn aceclaw-restart claude aceclaw-restart copilot (unchanged — still sets ACECLAW_PROVIDER) Applies the same logic to dev.sh and the Windows aceclaw-restart.cmd generated by install.sh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 12 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe changes modify provider and profile handling across dev.sh, install.sh, and restart.sh. Instead of rejecting invalid provider strings, the scripts now conditionally export them as ACECLAW_PROFILE when they don't match valid providers. Windows batch argument handling is also adjusted. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Add config.default.json with pre-configured claude and copilot profiles (no tokens). Bundle it in the release tarball via build.gradle.kts. install.sh copies it to ~/.aceclaw/config.json only when no config exists, so existing installs are never touched. Profiles included: claude, copilot, copilot-sonnet, copilot-haiku. The claude profile relies on OAuth auto-discovery from ~/.claude/.credentials so no credentials need to be added manually. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New users had no indication of how to authenticate after install. The success message now explains three options (Claude CLI OAuth, API key, GitHub Copilot) and shows account-switching examples. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65495cff27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| *" $PROVIDER "*) ;; | ||
| *) echo "Invalid provider: $PROVIDER"; echo "Valid: $VALID_PROVIDERS"; exit 1 ;; | ||
| *" $PROVIDER "*) export ACECLAW_PROVIDER="$PROVIDER" ;; | ||
| *) export ACECLAW_PROFILE="$PROVIDER" ;; |
There was a problem hiding this comment.
Keep invalid provider typos from silently proceeding
This fallback now treats any unrecognized argument as ACECLAW_PROFILE, so a typo like aceclaw-restart copilto no longer fails fast and instead starts with whichever provider remains in config. Because profile lookup is non-fatal (applyProfile only warns and returns), users can think they switched providers when they did not, which is a behavior regression from the previous explicit validation error.
Useful? React with 👍 / 👎.
| *" $PROVIDER "*) ;; | ||
| *) echo "Invalid provider: $PROVIDER"; echo "Valid: $VALID_PROVIDERS"; exit 1 ;; | ||
| *" $PROVIDER "*) export ACECLAW_PROVIDER="$PROVIDER"; echo "Provider: $PROVIDER" ;; | ||
| *) export ACECLAW_PROFILE="$PROVIDER"; echo "Profile: $PROVIDER" ;; |
There was a problem hiding this comment.
Fail fast on unknown dev.sh arguments
Unknown tokens (including mistyped flags) are parsed into PROVIDER, and this branch now exports them as ACECLAW_PROFILE instead of erroring. For example, ./dev.sh --basline will no longer stop; it continues with an unintended benchmark path and a bogus profile, so required benchmark checks can be skipped silently.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev.sh`:
- Around line 125-130: The script sets ACECLAW_PROVIDER or ACECLAW_PROFILE based
on PROVIDER but leaves the opposite env var unchanged, which can cause the
daemon to combine a stale value; update the branch in the PROVIDER handling (the
case matching against VALID_PROVIDERS) to explicitly unset the other
variable—when exporting ACECLAW_PROVIDER set ACECLAW_PROFILE="" (or unset
ACECLAW_PROFILE), and when exporting ACECLAW_PROFILE set ACECLAW_PROVIDER="" (or
unset ACECLAW_PROVIDER) so only the intended env var remains.
In `@install.sh`:
- Around line 214-216: The Windows wrapper currently treats every argument as
ACECLAW_PROFILE; instead set ACECLAW_PROVIDER from the first argument and
ACECLAW_PROFILE from the second so commands like "aceclaw-restart.cmd copilot"
set ACECLAW_PROVIDER=copilot. Update the logic around
ACECLAW_PROFILE/ACECLAW_PROVIDER assignment (the lines that reference "%~1" and
set "ACECLAW_PROFILE") to: if %~1 is not empty set ACECLAW_PROVIDER=%~1, and if
%~2 is not empty set ACECLAW_PROFILE=%~2; keep the subsequent calls to
"%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat" and the daemon stop invocation
unchanged so the environment variables are honored when calling aceclaw-cli.bat
and when invoking daemon stop.
In `@restart.sh`:
- Around line 63-68: The current branch sets ACECLAW_PROVIDER or ACECLAW_PROFILE
from the incoming PROVIDER but leaves the other env variable possibly stale;
update the logic in restart.sh where PROVIDER is validated against
VALID_PROVIDERS so that when you export ACECLAW_PROVIDER="$PROVIDER" you also
unset/export an empty ACECLAW_PROFILE, and when you export
ACECLAW_PROFILE="$PROVIDER" you also unset/export an empty ACECLAW_PROVIDER,
ensuring the daemon won't be influenced by a previously set counterpart
variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 974d15bd-9a45-4d50-b261-9714d5b8070a
📒 Files selected for processing (3)
dev.shinstall.shrestart.sh
| # Validate and set provider or profile via env if specified | ||
| if [ -n "$PROVIDER" ]; then | ||
| case " $VALID_PROVIDERS " in | ||
| *" $PROVIDER "*) ;; | ||
| *) echo "Invalid provider: $PROVIDER"; echo "Valid: $VALID_PROVIDERS"; exit 1 ;; | ||
| *" $PROVIDER "*) export ACECLAW_PROVIDER="$PROVIDER"; echo "Provider: $PROVIDER" ;; | ||
| *) export ACECLAW_PROFILE="$PROVIDER"; echo "Profile: $PROVIDER" ;; | ||
| esac |
There was a problem hiding this comment.
Clear the opposite env var when an argument is explicit.
If the caller already has ACECLAW_PROVIDER or ACECLAW_PROFILE in the environment, this now exports only one side and leaves the other active. Per aceclaw-daemon/src/main/java/dev/aceclaw/daemon/AceClawConfig.java:296-324, the daemon can then combine a stale env value with the requested argument.
🐛 Proposed fix
# Validate and set provider or profile via env if specified
if [ -n "$PROVIDER" ]; then
case " $VALID_PROVIDERS " in
- *" $PROVIDER "*) export ACECLAW_PROVIDER="$PROVIDER"; echo "Provider: $PROVIDER" ;;
- *) export ACECLAW_PROFILE="$PROVIDER"; echo "Profile: $PROVIDER" ;;
+ *" $PROVIDER "*) unset ACECLAW_PROFILE; export ACECLAW_PROVIDER="$PROVIDER"; echo "Provider: $PROVIDER" ;;
+ *) unset ACECLAW_PROVIDER; export ACECLAW_PROFILE="$PROVIDER"; echo "Profile: $PROVIDER" ;;
esac
fi📝 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.
| # Validate and set provider or profile via env if specified | |
| if [ -n "$PROVIDER" ]; then | |
| case " $VALID_PROVIDERS " in | |
| *" $PROVIDER "*) ;; | |
| *) echo "Invalid provider: $PROVIDER"; echo "Valid: $VALID_PROVIDERS"; exit 1 ;; | |
| *" $PROVIDER "*) export ACECLAW_PROVIDER="$PROVIDER"; echo "Provider: $PROVIDER" ;; | |
| *) export ACECLAW_PROFILE="$PROVIDER"; echo "Profile: $PROVIDER" ;; | |
| esac | |
| # Validate and set provider or profile via env if specified | |
| if [ -n "$PROVIDER" ]; then | |
| case " $VALID_PROVIDERS " in | |
| *" $PROVIDER "*) unset ACECLAW_PROFILE; export ACECLAW_PROVIDER="$PROVIDER"; echo "Provider: $PROVIDER" ;; | |
| *) unset ACECLAW_PROVIDER; export ACECLAW_PROFILE="$PROVIDER"; echo "Profile: $PROVIDER" ;; | |
| esac | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dev.sh` around lines 125 - 130, The script sets ACECLAW_PROVIDER or
ACECLAW_PROFILE based on PROVIDER but leaves the opposite env var unchanged,
which can cause the daemon to combine a stale value; update the branch in the
PROVIDER handling (the case matching against VALID_PROVIDERS) to explicitly
unset the other variable—when exporting ACECLAW_PROVIDER set ACECLAW_PROFILE=""
(or unset ACECLAW_PROFILE), and when exporting ACECLAW_PROFILE set
ACECLAW_PROVIDER="" (or unset ACECLAW_PROVIDER) so only the intended env var
remains.
| if not "%~1"=="" set "ACECLAW_PROFILE=%~1" | ||
| call "%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat" daemon stop 2>nul | ||
| call "%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat" %* | ||
| call "%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat" |
There was a problem hiding this comment.
Mirror the provider/profile split in the Windows wrapper.
Line 214 treats every argument as ACECLAW_PROFILE, so aceclaw-restart.cmd copilot no longer sets ACECLAW_PROVIDER=copilot. That breaks the PR’s “provider path unchanged” behavior on Windows.
🐛 Proposed fix
`@echo` off
+setlocal
set ACECLAW_BENCH_MODE=none
-if not "%~1"=="" set "ACECLAW_PROFILE=%~1"
+set "ACECLAW_ARG=%~1"
+set "ACECLAW_ARG_IS_PROVIDER="
+if not "%ACECLAW_ARG%"=="" (
+ if /I "%ACECLAW_ARG%"=="anthropic" set "ACECLAW_ARG_IS_PROVIDER=1"
+ if /I "%ACECLAW_ARG%"=="openai" set "ACECLAW_ARG_IS_PROVIDER=1"
+ if /I "%ACECLAW_ARG%"=="openai-codex" set "ACECLAW_ARG_IS_PROVIDER=1"
+ if /I "%ACECLAW_ARG%"=="ollama" set "ACECLAW_ARG_IS_PROVIDER=1"
+ if /I "%ACECLAW_ARG%"=="copilot" set "ACECLAW_ARG_IS_PROVIDER=1"
+ if /I "%ACECLAW_ARG%"=="groq" set "ACECLAW_ARG_IS_PROVIDER=1"
+ if defined ACECLAW_ARG_IS_PROVIDER (
+ set "ACECLAW_PROFILE="
+ set "ACECLAW_PROVIDER=%ACECLAW_ARG%"
+ ) else (
+ set "ACECLAW_PROVIDER="
+ set "ACECLAW_PROFILE=%ACECLAW_ARG%"
+ )
+)
call "%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat" daemon stop 2>nul
call "%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat"📝 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 not "%~1"=="" set "ACECLAW_PROFILE=%~1" | |
| call "%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat" daemon stop 2>nul | |
| call "%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat" %* | |
| call "%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat" | |
| `@echo` off | |
| setlocal | |
| set ACECLAW_BENCH_MODE=none | |
| set "ACECLAW_ARG=%~1" | |
| set "ACECLAW_ARG_IS_PROVIDER=" | |
| if not "%ACECLAW_ARG%"=="" ( | |
| if /I "%ACECLAW_ARG%"=="anthropic" set "ACECLAW_ARG_IS_PROVIDER=1" | |
| if /I "%ACECLAW_ARG%"=="openai" set "ACECLAW_ARG_IS_PROVIDER=1" | |
| if /I "%ACECLAW_ARG%"=="openai-codex" set "ACECLAW_ARG_IS_PROVIDER=1" | |
| if /I "%ACECLAW_ARG%"=="ollama" set "ACECLAW_ARG_IS_PROVIDER=1" | |
| if /I "%ACECLAW_ARG%"=="copilot" set "ACECLAW_ARG_IS_PROVIDER=1" | |
| if /I "%ACECLAW_ARG%"=="groq" set "ACECLAW_ARG_IS_PROVIDER=1" | |
| if defined ACECLAW_ARG_IS_PROVIDER ( | |
| set "ACECLAW_PROFILE=" | |
| set "ACECLAW_PROVIDER=%ACECLAW_ARG%" | |
| ) else ( | |
| set "ACECLAW_PROVIDER=" | |
| set "ACECLAW_PROFILE=%ACECLAW_ARG%" | |
| ) | |
| ) | |
| call "%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat" daemon stop 2>nul | |
| call "%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@install.sh` around lines 214 - 216, The Windows wrapper currently treats
every argument as ACECLAW_PROFILE; instead set ACECLAW_PROVIDER from the first
argument and ACECLAW_PROFILE from the second so commands like
"aceclaw-restart.cmd copilot" set ACECLAW_PROVIDER=copilot. Update the logic
around ACECLAW_PROFILE/ACECLAW_PROVIDER assignment (the lines that reference
"%~1" and set "ACECLAW_PROFILE") to: if %~1 is not empty set
ACECLAW_PROVIDER=%~1, and if %~2 is not empty set ACECLAW_PROFILE=%~2; keep the
subsequent calls to "%USERPROFILE%\.aceclaw\bin\aceclaw-cli.bat" and the daemon
stop invocation unchanged so the environment variables are honored when calling
aceclaw-cli.bat and when invoking daemon stop.
| # Validate and set provider or profile via env if specified | ||
| if [ -n "$PROVIDER" ]; then | ||
| case " $VALID_PROVIDERS " in | ||
| *" $PROVIDER "*) ;; | ||
| *) echo "Invalid provider: $PROVIDER"; echo "Valid: $VALID_PROVIDERS"; exit 1 ;; | ||
| *" $PROVIDER "*) export ACECLAW_PROVIDER="$PROVIDER" ;; | ||
| *) export ACECLAW_PROFILE="$PROVIDER" ;; | ||
| esac |
There was a problem hiding this comment.
Avoid mixing the requested argument with stale env config.
A pre-existing ACECLAW_PROVIDER or ACECLAW_PROFILE can still affect this restart after Line 66 or Line 67 exports the new value. Since the daemon applies ACECLAW_PROFILE and then lets ACECLAW_PROVIDER override provider, clear the counterpart when the user passes an explicit argument.
🐛 Proposed fix
# Validate and set provider or profile via env if specified
if [ -n "$PROVIDER" ]; then
case " $VALID_PROVIDERS " in
- *" $PROVIDER "*) export ACECLAW_PROVIDER="$PROVIDER" ;;
- *) export ACECLAW_PROFILE="$PROVIDER" ;;
+ *" $PROVIDER "*) unset ACECLAW_PROFILE; export ACECLAW_PROVIDER="$PROVIDER" ;;
+ *) unset ACECLAW_PROVIDER; export ACECLAW_PROFILE="$PROVIDER" ;;
esac
fi📝 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.
| # Validate and set provider or profile via env if specified | |
| if [ -n "$PROVIDER" ]; then | |
| case " $VALID_PROVIDERS " in | |
| *" $PROVIDER "*) ;; | |
| *) echo "Invalid provider: $PROVIDER"; echo "Valid: $VALID_PROVIDERS"; exit 1 ;; | |
| *" $PROVIDER "*) export ACECLAW_PROVIDER="$PROVIDER" ;; | |
| *) export ACECLAW_PROFILE="$PROVIDER" ;; | |
| esac | |
| # Validate and set provider or profile via env if specified | |
| if [ -n "$PROVIDER" ]; then | |
| case " $VALID_PROVIDERS " in | |
| *" $PROVIDER "*) unset ACECLAW_PROFILE; export ACECLAW_PROVIDER="$PROVIDER" ;; | |
| *) unset ACECLAW_PROVIDER; export ACECLAW_PROFILE="$PROVIDER" ;; | |
| esac | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@restart.sh` around lines 63 - 68, The current branch sets ACECLAW_PROVIDER or
ACECLAW_PROFILE from the incoming PROVIDER but leaves the other env variable
possibly stale; update the logic in restart.sh where PROVIDER is validated
against VALID_PROVIDERS so that when you export ACECLAW_PROVIDER="$PROVIDER" you
also unset/export an empty ACECLAW_PROFILE, and when you export
ACECLAW_PROFILE="$PROVIDER" you also unset/export an empty ACECLAW_PROVIDER,
ensuring the daemon won't be influenced by a previously set counterpart
variable.
- config.default.json: default claude profile to sonnet-4-6 (not opus, too expensive for new users); align copilot-sonnet to 4.6 - restart.sh: add echo for provider and profile selection (was silent, unlike dev.sh) - dev.sh: clarify message when arg is treated as profile name, not a built-in provider Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
restart.sh/dev.sh: if the positional argument matches a known provider (anthropic,copilot,ollama…) it setsACECLAW_PROVIDERas before; otherwise it falls through toACECLAW_PROFILE, enabling named profiles to be passed directlyinstall.sh: Windowsaceclaw-restart.cmdupdated with the same behaviour (setsACECLAW_PROFILEfrom%1)Usage after this change
Test plan
aceclaw-restart claude-acnstarts daemon with ACN OAuth profileaceclaw-restart claudestarts daemon with personal Claude profileaceclaw-restart copilotstill works (provider path unchanged)aceclaw-restartwith no args still usesdefaultProfiledev.sh claude-acnprintsProfile: claude-acnand starts correctly🤖 Generated with Claude Code
Summary by CodeRabbit