Coder - #389
Conversation
… for log creation
…4.6 configuration
…pencode configuration
…pencode configuration
…ctory configuration
…eview in configuration files
…ku in configuration files
…ning in configuration files
|
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 CLIProxyAPI integration infrastructure alongside PostgreSQL 18 support. It adds Nix modules for configuration management, a macOS launchd service with startup script, model provider configurations (CLIProxyAPI, AMP, factory), and augments shell PATH variables across bash, fish, and zsh. Homebrew packages for cliproxyapi, coder, and postgresql@18 are added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (16)
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 primarily focuses on integrating and configuring 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;DRSet up CLIProxyAPI as a local service and wired Codex, OpenCode, AMP, and Factory to use http://localhost:8317/v1 for unified model access. Also added brew packages and Postgres 18 PATH updates needed to run the service. What changed?
Description generated by Mesa. Update settings |
![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 configurations for several new tools, including amp, cliproxyapi, and factory, and sets up cliproxyapi as a local service on macOS. It also updates existing tool configurations (codex, opencode) to use the new proxy service. The changes are well-structured, but I have a few suggestions to improve robustness and maintainability, particularly regarding the new cliproxyapi service and shell configurations. My feedback includes suggestions for improving the service logging, simplifying the startup script, and avoiding redundancy in shell setup.
| # - "your-api-key-2" | ||
|
|
||
| # Enable debug logging | ||
| debug: true |
There was a problem hiding this comment.
debug: true is enabled, which can lead to verbose logging and might expose sensitive information in logs. For a long-running service, it's generally recommended to set this to false and only enable it when actively troubleshooting.
debug: false| fish_add_path -p /nix/var/nix/profiles/default/bin | ||
| fish_add_path -p ~/.foundry/bin | ||
| fish_add_path -p /opt/homebrew/bin | ||
| fish_add_path -p /opt/homebrew/opt/postgresql@18/bin |
There was a problem hiding this comment.
This path is already added on line 20 within loginShellInit. Since fish_add_path typically modifies fish_user_paths, which is a universal variable that persists across shell sessions, adding the path again in interactiveShellInit is redundant. You can safely remove this line.
| StandardOutPath = "/tmp/cliproxyapi.log"; | ||
| StandardErrorPath = "/tmp/cliproxyapi.error.log"; |
There was a problem hiding this comment.
Log files are being written to /tmp, which is an ephemeral directory. Its contents may be cleared on system reboot. For persistent logging, consider using a more permanent location, such as ~/Library/Logs/cliproxyapi/ on macOS. Note that you would need to provide an absolute path in the launchd configuration as it does not expand ~.
| if [ -x /opt/homebrew/bin/cliproxyapi ]; then | ||
| exec /opt/homebrew/bin/cliproxyapi -config "$CONFIG" "$@" | ||
| elif [ -x /usr/local/bin/cliproxyapi ]; then | ||
| exec /usr/local/bin/cliproxyapi -config "$CONFIG" "$@" | ||
| else | ||
| echo 'cliproxyapi binary not found; install it with "brew install cliproxyapi"' >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
The script hardcodes paths to find the cliproxyapi binary. A more robust approach is to rely on the PATH environment variable, which is already set in the launchd service definition. You can use command -v to check if the binary exists in the PATH and then execute it directly.
| if [ -x /opt/homebrew/bin/cliproxyapi ]; then | |
| exec /opt/homebrew/bin/cliproxyapi -config "$CONFIG" "$@" | |
| elif [ -x /usr/local/bin/cliproxyapi ]; then | |
| exec /usr/local/bin/cliproxyapi -config "$CONFIG" "$@" | |
| else | |
| echo 'cliproxyapi binary not found; install it with "brew install cliproxyapi"' >&2 | |
| exit 1 | |
| fi | |
| if command -v cliproxyapi >/dev/null; then | |
| exec cliproxyapi -config "$CONFIG" "$@" | |
| else | |
| echo 'cliproxyapi binary not found; install it with "brew install cliproxyapi"' >&2 | |
| exit 1 | |
| fi |
![mesa-dot-dev[bot]](https://avatars.githubusercontent.com/in/1050077?s=60&v=4){kind=small}
There was a problem hiding this comment.
Performed full review of 67c78fd...c68684c
Analysis
-
Insecure Secrets Management - The implementation uses plain .env files and sed-based template substitution instead of the repository's established agenix secret management system, creating security vulnerabilities and inconsistent patterns.
-
Configuration Fragmentation - "localhost:8317" is hardcoded across 5 different config files, creating maintenance challenges when ports need to change. A centralized configuration approach is missing.
-
Platform Limitations - The cliproxyapi service is Darwin/macOS-only with no Linux/NixOS equivalent, limiting cross-platform compatibility despite the repository supporting multiple platforms.
-
PostgreSQL Version Conflict Risk - Both "postgresql" and "postgresql@18" are installed via Homebrew without clear documentation about which should be used, potentially causing version conflicts or PATH ordering issues.
-
Missing Service Dependencies - There's no declaration that tools depend on cliproxyapi being running, which could lead to cryptic connection errors if the proxy service fails.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
16 files reviewed | 6 comments | Edit Agent Settings • Read Docs
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "amp.url": "http://localhost:8317" | |||
There was a problem hiding this comment.
Hardcoded localhost:8317. If cliproxyapi port changes, this and 4 other config files need manual updates (codex/config.toml, factory/config.json, opencode/opencode.jsonc). Consider extracting this to a shared Nix variable to maintain DRY principles: proxyPort = 8317; then reference it across configs.
Agent: 🏛 Architecture
| @@ -0,0 +1,19 @@ | |||
| { pkgs, ... }: | |||
| { | |||
| launchd.agents.cliproxyapi = pkgs.lib.mkIf pkgs.stdenv.isDarwin { | |||
There was a problem hiding this comment.
Service is Darwin-only (mkIf pkgs.stdenv.isDarwin) but the repository supports multiple platforms (hosts/darwin, hosts/linux, hosts/nixos). Consider adding a Linux systemd equivalent or documenting why this service is macOS-exclusive.
Agent: 🏛 Architecture
| api-keys: | ||
| - "__OPENROUTER_API_KEY__" | ||
| models: | ||
| - name: "z-ai/glm-4.6" |
There was a problem hiding this comment.
Placeholder syntax OPENROUTER_API_KEY gets replaced by sed in start.sh, but if replacement fails, this literal string remains in the config. Consider using cliproxyapi's native environment variable support (if available) or the agenix secret management system already established in this repository for more robust secret injection.
Agent: 🏛 Architecture
| "pinentry-mac" | ||
| "pnpm" | ||
| "postgresql" | ||
| "postgresql@18" |
There was a problem hiding this comment.
Both "postgresql" (line 37) and "postgresql@18" are now installed. This could cause version conflicts or PATH ordering issues. Document which version should be used, and consider removing the older "postgresql" if version 18 supersedes it.
Agent: 🏛 Architecture
| CONFIG_DIR="$HOME/.cli-proxy-api" | ||
| TEMPLATE="$CONFIG_DIR/config.template.yaml" | ||
| CONFIG="$CONFIG_DIR/config.yaml" | ||
| ENV_FILE="$HOME/dotfiles/.env" |
There was a problem hiding this comment.
Hardcoded path assumes 'dotfiles' directory exists in $HOME, which may not be true on all systems. This repository uses agenix for secret management (see named-hosts/galactica/secrets.nix). Consider migrating to agenix: age.secrets.openrouter-api-key.file = ./keys/openrouter.age; for consistent secret handling across the codebase.
Agent: 🏛 Architecture
|
|
||
| # Generate config from template with secrets injected | ||
| if [ -f "$TEMPLATE" ]; then | ||
| sed "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" "$TEMPLATE" >"$CONFIG" |
There was a problem hiding this comment.
Silent failure risk: If OPENROUTER_API_KEY is unset or empty, sed creates a config with an empty string, leading to subtle runtime failures. Add validation before sed replacement: if [ -z "${OPENROUTER_API_KEY:-}" ]; then echo "Error: OPENROUTER_API_KEY not set" >&2; exit 1; fi
Agent: 🏛 Architecture
There was a problem hiding this comment.
Pull request overview
This PR adds integration for CLIProxyAPI, a local proxy service for AI model APIs, along with configurations for multiple AI coding tools. It also updates PostgreSQL to version 18 and adds support for various AI models including hypothetical future versions.
Key changes:
- Adds cliproxyapi as a new launchd service with automatic startup and configuration templating
- Integrates cliproxyapi with multiple coding tools (opencode, factory, codex, amp)
- Updates PostgreSQL from default version to version 18 with PATH configurations across all shells
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| nix-darwin/config/homebrew.nix | Adds cliproxyapi, coder, and postgresql@18 brew packages |
| home-manager/services/default.nix | Registers cliproxyapi service module |
| home-manager/services/cliproxyapi/start.sh | Service startup script that handles config templating and environment setup |
| home-manager/services/cliproxyapi/default.nix | Launchd agent configuration for cliproxyapi service |
| home-manager/programs/zsh/default.nix | Adds postgresql@18 to PATH |
| home-manager/programs/fish/default.nix | Adds postgresql@18 to PATH in both login and interactive shells |
| home-manager/programs/bash/default.nix | Adds postgresql@18 to PATH |
| config/opencode/opencode.jsonc | Adds cliproxyapi provider with model configurations, removes plugin |
| config/factory/default.nix | Creates symlink for factory config |
| config/factory/config.json | Configures custom models pointing to cliproxyapi endpoint |
| config/default.nix | Registers amp, cliproxyapi, and factory config modules |
| config/codex/config.toml | Adds cliproxyapi model provider and kimi-k2 profile |
| config/cliproxyapi/default.nix | Creates symlink for cliproxyapi config template |
| config/cliproxyapi/config.yaml | Main configuration for cliproxyapi service with API settings |
| config/amp/settings.json | Configures amp URL to use cliproxyapi |
| config/amp/default.nix | Creates symlink for amp settings |
Comments suppressed due to low confidence (1)
config/default.nix:14
- The module imports are not in alphabetical order. The list has "./amp" and "./cliproxyapi" before "./codex", "./crush", and "./claude", but then "./claude" appears after "./crush". Consider sorting the entire list alphabetically: amp, claude, cliproxyapi, codex, crush, direnv, factory, ghostty, hammerspoon, karabiner, opencode, serena, starship.
./amp
./cliproxyapi
./codex
./crush
./claude
./direnv
./factory
./ghostty
./hammerspoon
./karabiner
./opencode
./serena
./starship
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "base_url": "http://127.0.0.1:8317/v1", | ||
| "api_key": "sk-dummy", | ||
| "provider": "openai" | ||
| }, | ||
| { | ||
| "model": "gpt-5.1", | ||
| "base_url": "http://127.0.0.1:8317/v1", | ||
| "api_key": "sk-dummy", | ||
| "provider": "openai" | ||
| }, | ||
| { | ||
| "model": "gpt-5.1-codex", | ||
| "base_url": "http://127.0.0.1:8317/v1", | ||
| "api_key": "sk-dummy", | ||
| "provider": "openai" | ||
| }, | ||
| { | ||
| "model": "gpt-5.1-codex-max", | ||
| "base_url": "http://127.0.0.1:8317/v1", | ||
| "api_key": "sk-dummy", | ||
| "provider": "openai" | ||
| }, | ||
| { | ||
| "model": "claude-opus-4-5-20251101", | ||
| "base_url": "http://127.0.0.1:8317/v1", | ||
| "api_key": "sk-dummy", | ||
| "provider": "anthropic" | ||
| }, | ||
| { | ||
| "model": "claude-haiku-4-5-20251001", | ||
| "base_url": "http://127.0.0.1:8317/v1", |
There was a problem hiding this comment.
The base_url uses http://127.0.0.1:8317/v1 while other configuration files (opencode.jsonc, codex/config.toml) use http://localhost:8317/v1. For consistency, consider using localhost instead of 127.0.0.1 across all configuration files.
| "base_url": "http://127.0.0.1:8317/v1", | |
| "api_key": "sk-dummy", | |
| "provider": "openai" | |
| }, | |
| { | |
| "model": "gpt-5.1", | |
| "base_url": "http://127.0.0.1:8317/v1", | |
| "api_key": "sk-dummy", | |
| "provider": "openai" | |
| }, | |
| { | |
| "model": "gpt-5.1-codex", | |
| "base_url": "http://127.0.0.1:8317/v1", | |
| "api_key": "sk-dummy", | |
| "provider": "openai" | |
| }, | |
| { | |
| "model": "gpt-5.1-codex-max", | |
| "base_url": "http://127.0.0.1:8317/v1", | |
| "api_key": "sk-dummy", | |
| "provider": "openai" | |
| }, | |
| { | |
| "model": "claude-opus-4-5-20251101", | |
| "base_url": "http://127.0.0.1:8317/v1", | |
| "api_key": "sk-dummy", | |
| "provider": "anthropic" | |
| }, | |
| { | |
| "model": "claude-haiku-4-5-20251001", | |
| "base_url": "http://127.0.0.1:8317/v1", | |
| "base_url": "http://localhost:8317/v1", | |
| "api_key": "sk-dummy", | |
| "provider": "openai" | |
| }, | |
| { | |
| "model": "gpt-5.1", | |
| "base_url": "http://localhost:8317/v1", | |
| "api_key": "sk-dummy", | |
| "provider": "openai" | |
| }, | |
| { | |
| "model": "gpt-5.1-codex", | |
| "base_url": "http://localhost:8317/v1", | |
| "api_key": "sk-dummy", | |
| "provider": "openai" | |
| }, | |
| { | |
| "model": "gpt-5.1-codex-max", | |
| "base_url": "http://localhost:8317/v1", | |
| "api_key": "sk-dummy", | |
| "provider": "openai" | |
| }, | |
| { | |
| "model": "claude-opus-4-5-20251101", | |
| "base_url": "http://localhost:8317/v1", | |
| "api_key": "sk-dummy", | |
| "provider": "anthropic" | |
| }, | |
| { | |
| "model": "claude-haiku-4-5-20251001", | |
| "base_url": "http://localhost:8317/v1", |
| sed "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" "$TEMPLATE" >"$CONFIG" | ||
| fi | ||
|
|
||
| # Change to config dir so logs are created there |
There was a problem hiding this comment.
The script attempts to cd "$CONFIG_DIR" without first verifying the directory exists. If ~/.cli-proxy-api doesn't exist, the script will fail. Consider adding a check: mkdir -p "$CONFIG_DIR" before line 24, or add error handling to verify the directory exists.
| # Change to config dir so logs are created there | |
| # Change to config dir so logs are created there | |
| mkdir -p "$CONFIG_DIR" |
| if [ -f "$TEMPLATE" ]; then | ||
| sed "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" "$TEMPLATE" >"$CONFIG" | ||
| fi |
There was a problem hiding this comment.
The config generation on line 19-21 only runs if $TEMPLATE exists, but the cliproxyapi command on lines 34 and 36 always passes -config "$CONFIG". If the template doesn't exist, the config file won't be created, potentially causing cliproxyapi to fail. Consider either: 1) Making template generation mandatory with error handling, or 2) Only passing -config flag if the config file was successfully generated.
| if [ -f "$TEMPLATE" ]; then | |
| sed "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" "$TEMPLATE" >"$CONFIG" | |
| fi | |
| if [ ! -f "$TEMPLATE" ]; then | |
| echo "Config template '$TEMPLATE' not found. Cannot generate config file." >&2 | |
| exit 1 | |
| fi | |
| sed "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" "$TEMPLATE" >"$CONFIG" |
|
|
||
| # Generate config from template with secrets injected | ||
| if [ -f "$TEMPLATE" ]; then | ||
| sed "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" "$TEMPLATE" >"$CONFIG" |
There was a problem hiding this comment.
The sed substitution doesn't escape the OPENROUTER_API_KEY value, which could cause issues if the API key contains special characters like |, &, or /. Consider using a more robust method like envsubst or escaping the value before substitution. For example: envsubst < "$TEMPLATE" > "$CONFIG" with export OPENROUTER_API_KEY set before calling it.
| sed "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" "$TEMPLATE" >"$CONFIG" | |
| export OPENROUTER_API_KEY | |
| envsubst < "$TEMPLATE" > "$CONFIG" |
| "${./start.sh}" | ||
| ]; | ||
| Environment = { | ||
| PATH = "${pkgs.lib.makeBinPath [ pkgs.gnused ]}:/opt/homebrew/bin:/usr/local/bin"; |
There was a problem hiding this comment.
The PATH includes hardcoded /opt/homebrew/bin:/usr/local/bin, but the script itself already checks both locations for the binary (lines 33-36 in start.sh). Since the script uses absolute paths when exec'ing cliproxyapi, these hardcoded PATH entries may be unnecessary. Consider whether they're needed for other commands in the script (like sed).
| PATH = "${pkgs.lib.makeBinPath [ pkgs.gnused ]}:/opt/homebrew/bin:/usr/local/bin"; | |
| PATH = "${pkgs.lib.makeBinPath [ pkgs.gnused ]}"; |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Summary by cubic
Set up CLIProxyAPI as a local service and wired Codex, OpenCode, AMP, and Factory to use http://localhost:8317/v1 for unified model access. Also added brew packages and Postgres 18 PATH updates needed to run the service.
New Features
Dependencies
Written for commit ad39434. Summary will update automatically on new commits.