fix(clawdbot): inject cliproxy apiKey via activation script - #615
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
📝 WalkthroughSummary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a new home-manager activation hook that reads a cliproxy API key from a file and injects it into the clawdbot.json configuration on Kyber hosts, following the established remote-token injection pattern. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
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 |
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 addresses a critical configuration gap in 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
|
The upstream nix-clawdbot doesn't pass apiKeyFile through configOverrides, causing the cliproxy provider to be loaded without authentication. Added an activation script to inject the apiKey from the key file into the generated clawdbot.json config (similar to the gateway token injection). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Mesa DescriptionTL;DRAdd activation script to inject cliproxy apiKey into clawdbot.json. The upstream nix-clawdbot doesn't pass What changed?Added Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Code Review
This pull request adds a home-manager activation script to inject the cliproxy API key into clawdbot.json, addressing an upstream limitation. The approach is sound. My review includes a suggestion to make the script more robust by checking for a non-empty key file, using mktemp for safer file operations, and ensuring accurate success/error reporting.
| if [ -f "$KEY_FILE" ] && [ -f "$CONFIG_FILE" ]; then | ||
| KEY=$(${pkgs.coreutils}/bin/cat "$KEY_FILE" | ${pkgs.coreutils}/bin/tr -d '\n') | ||
| # Inject apiKey into models.providers.cliproxy | ||
| ${pkgs.jq}/bin/jq --arg key "$KEY" \ | ||
| '.models.providers.cliproxy.apiKey = $key' \ | ||
| "$CONFIG_FILE" > "$CONFIG_FILE.tmp" && \ | ||
| ${pkgs.coreutils}/bin/mv "$CONFIG_FILE.tmp" "$CONFIG_FILE" | ||
| echo "Injected cliproxy API key into clawdbot config" | ||
| fi |
There was a problem hiding this comment.
This script block can be made more robust and correct. Currently, it reports success even if the mv command fails, and it doesn't handle empty key files. I suggest the following improvements:
- Check for non-empty key file: Use
[ -s "$KEY_FILE" ]instead of[ -f "$KEY_FILE" ]to prevent injecting an empty API key ifextract-secrets.shdoesn't find a key. - Use
mktemp: Create a secure temporary file to avoid issues with stale or conflicting temporary files. - Correct Error Handling: Ensure the success message is only printed if the entire operation succeeds, and provide an error message otherwise. This also includes cleaning up the temporary file on failure.
if [ -s "$KEY_FILE" ] && [ -f "$CONFIG_FILE" ]; then
KEY=$(${pkgs.coreutils}/bin/cat "$KEY_FILE" | ${pkgs.coreutils}/bin/tr -d '\n')
TMP_FILE=$(${pkgs.coreutils}/bin/mktemp)
# Inject apiKey into models.providers.cliproxy
if ${pkgs.jq}/bin/jq --arg key "$KEY" \
'.models.providers.cliproxy.apiKey = $key' \
"$CONFIG_FILE" > "$TMP_FILE" && \
${pkgs.coreutils}/bin/mv "$TMP_FILE" "$CONFIG_FILE"; then
echo "Injected cliproxy API key into clawdbot config"
else
echo "Error: Failed to inject cliproxy API key into clawdbot config." >&2
${pkgs.coreutils}/bin/rm -f "$TMP_FILE"
fi
fi
There was a problem hiding this comment.
Pull request overview
This pull request adds an activation script to inject the cliproxy API key into the clawdbot configuration file, working around the limitation that the upstream nix-clawdbot package doesn't support apiKeyFile in configOverrides.
Changes:
- Added
home.activation.clawdbotCliproxyKeyactivation script that reads the API key from~/.config/clawdbot/cliproxy-keyand injects it into the generated clawdbot.json config at the path.models.providers.cliproxy.apiKey
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@home-manager/modules/clawdbot/default.nix`:
- Around line 53-61: The script currently reads KEY from KEY_FILE then
unconditionally runs jq to set .models.providers.cliproxy.apiKey and echoes
success; fix this by validating the KEY is non-empty (test -n "$KEY" after KEY
is assigned) and aborting/ skipping injection if empty, and ensure the echo
"Injected cliproxy API key into clawdbot config" only runs when the jq command
and subsequent mv both succeed (i.e., chain the jq -> mv pipeline and only print
on its success). Use the existing KEY_FILE, CONFIG_FILE, KEY variable and the
jq/mv pipeline to implement these checks so the config is not overwritten with
an empty key and the success log reflects true success.
🧹 Nitpick comments (1)
home-manager/modules/clawdbot/default.nix (1)
41-66: Keephome.activation.*entries alphabetically ordered.With the new entry, the activation keys are no longer sorted. Consider ordering them as
clawdbotCliproxyKey,clawdbotRemoteToken,clawdbotSecretsto comply with the Nix style guideline. As per coding guidelines, ...
| if [ -f "$KEY_FILE" ] && [ -f "$CONFIG_FILE" ]; then | ||
| KEY=$(${pkgs.coreutils}/bin/cat "$KEY_FILE" | ${pkgs.coreutils}/bin/tr -d '\n') | ||
| # Inject apiKey into models.providers.cliproxy | ||
| ${pkgs.jq}/bin/jq --arg key "$KEY" \ | ||
| '.models.providers.cliproxy.apiKey = $key' \ | ||
| "$CONFIG_FILE" > "$CONFIG_FILE.tmp" && \ | ||
| ${pkgs.coreutils}/bin/mv "$CONFIG_FILE.tmp" "$CONFIG_FILE" | ||
| echo "Injected cliproxy API key into clawdbot config" | ||
| fi |
There was a problem hiding this comment.
Guard against empty key + avoid false “Injected” logs.
If the key file exists but is empty (or jq fails), the script will overwrite the config with an empty apiKey and still print success, which can silently break authentication. Consider requiring a non-empty key and chaining the success log to the jq/mv pipeline.
🔧 Suggested fix
- if [ -f "$KEY_FILE" ] && [ -f "$CONFIG_FILE" ]; then
- KEY=$(${pkgs.coreutils}/bin/cat "$KEY_FILE" | ${pkgs.coreutils}/bin/tr -d '\n')
- # Inject apiKey into models.providers.cliproxy
- ${pkgs.jq}/bin/jq --arg key "$KEY" \
- '.models.providers.cliproxy.apiKey = $key' \
- "$CONFIG_FILE" > "$CONFIG_FILE.tmp" && \
- ${pkgs.coreutils}/bin/mv "$CONFIG_FILE.tmp" "$CONFIG_FILE"
- echo "Injected cliproxy API key into clawdbot config"
- fi
+ if [ -f "$KEY_FILE" ] && [ -f "$CONFIG_FILE" ]; then
+ KEY=$(${pkgs.coreutils}/bin/cat "$KEY_FILE" | ${pkgs.coreutils}/bin/tr -d '\n')
+ if [ -n "$KEY" ]; then
+ # Inject apiKey into models.providers.cliproxy
+ ${pkgs.jq}/bin/jq --arg key "$KEY" \
+ '.models.providers.cliproxy.apiKey = $key' \
+ "$CONFIG_FILE" > "$CONFIG_FILE.tmp" && \
+ ${pkgs.coreutils}/bin/mv "$CONFIG_FILE.tmp" "$CONFIG_FILE" && \
+ echo "Injected cliproxy API key into clawdbot config"
+ else
+ echo "cliproxy key is empty; skipping injection"
+ fi
+ 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.
| if [ -f "$KEY_FILE" ] && [ -f "$CONFIG_FILE" ]; then | |
| KEY=$(${pkgs.coreutils}/bin/cat "$KEY_FILE" | ${pkgs.coreutils}/bin/tr -d '\n') | |
| # Inject apiKey into models.providers.cliproxy | |
| ${pkgs.jq}/bin/jq --arg key "$KEY" \ | |
| '.models.providers.cliproxy.apiKey = $key' \ | |
| "$CONFIG_FILE" > "$CONFIG_FILE.tmp" && \ | |
| ${pkgs.coreutils}/bin/mv "$CONFIG_FILE.tmp" "$CONFIG_FILE" | |
| echo "Injected cliproxy API key into clawdbot config" | |
| fi | |
| if [ -f "$KEY_FILE" ] && [ -f "$CONFIG_FILE" ]; then | |
| KEY=$(${pkgs.coreutils}/bin/cat "$KEY_FILE" | ${pkgs.coreutils}/bin/tr -d '\n') | |
| if [ -n "$KEY" ]; then | |
| # Inject apiKey into models.providers.cliproxy | |
| ${pkgs.jq}/bin/jq --arg key "$KEY" \ | |
| '.models.providers.cliproxy.apiKey = $key' \ | |
| "$CONFIG_FILE" > "$CONFIG_FILE.tmp" && \ | |
| ${pkgs.coreutils}/bin/mv "$CONFIG_FILE.tmp" "$CONFIG_FILE" && \ | |
| echo "Injected cliproxy API key into clawdbot config" | |
| else | |
| echo "cliproxy key is empty; skipping injection" | |
| fi | |
| fi |
🤖 Prompt for AI Agents
In `@home-manager/modules/clawdbot/default.nix` around lines 53 - 61, The script
currently reads KEY from KEY_FILE then unconditionally runs jq to set
.models.providers.cliproxy.apiKey and echoes success; fix this by validating the
KEY is non-empty (test -n "$KEY" after KEY is assigned) and aborting/ skipping
injection if empty, and ensure the echo "Injected cliproxy API key into clawdbot
config" only runs when the jq command and subsequent mv both succeed (i.e.,
chain the jq -> mv pipeline and only print on its success). Use the existing
KEY_FILE, CONFIG_FILE, KEY variable and the jq/mv pipeline to implement these
checks so the config is not overwritten with an empty key and the success log
reflects true success.
Summary
apiKeyFilethroughconfigOverridesProblem
The cliproxy provider was being loaded without authentication because
apiKeyFilewasn't being passed through to the generated config. This causedclawdbot models listto show 0 cliproxy models and sub-agents to fall back to the default Anthropic model.Solution
Added
home.activation.clawdbotCliproxyKeythat injects the API key from~/.config/clawdbot/cliproxy-keyinto the JSON config at.models.providers.cliproxy.apiKey.Test plan
~/.clawdbot/clawdbot.json🤖 Generated with Claude Code
Summary by cubic
Inject cliproxy API key into clawdbot.json during activation so the cliproxy provider loads with auth. Fixes missing cliproxy models and prevents sub-agents from falling back to the default Anthropic model.
Written for commit b97909e. Summary will update on new commits.