Skip to content

feat: add usage statistics persistence for cliproxyapi - #674

Merged
shunkakinoki merged 2 commits into
mainfrom
feat/cliproxyapi-usage-persistence
Jan 26, 2026
Merged

feat: add usage statistics persistence for cliproxyapi#674
shunkakinoki merged 2 commits into
mainfrom
feat/cliproxyapi-usage-persistence

Conversation

@shunkakinoki

@shunkakinoki shunkakinoki commented Jan 26, 2026

Copy link
Copy Markdown
Owner

Changes

  • Added automatic usage statistics backup on service exit
  • Added automatic usage statistics restore on service start
  • Moved Docker pull logic to start.sh from Makefile
  • Added trap handlers for graceful shutdown with usage export

Technical Details

  • Implemented usage_export() function to save usage stats to JSON file
  • Implemented usage_import() function to restore usage stats from JSON file
  • Added wait_for_management() to ensure management API is ready before import
  • Set up EXIT and TERM/INT traps to export usage on service termination

Testing

  • Manual testing on Linux with Docker
  • Manual testing on macOS with Homebrew binary

Generated with opencode by glm-4.7


Summary by cubic

Persist cliproxyapi usage statistics across restarts by auto-exporting on shutdown and restoring on startup. Moved Docker image pull into start.sh and added graceful shutdown traps.

  • New Features

    • Auto export usage to JSON on exit; auto restore on start via the management API.
    • Waits for the management API to be ready before importing.
    • Works for Docker and macOS binaries; uses MANAGEMENT_KEY for auth.
  • Refactors

    • Moved Docker pull logic from the Makefile to start.sh.
    • Updated README to note automatic usage backup/restore.

Written for commit 00d610e. Summary will update on new commits.

Copilot AI review requested due to automatic review settings January 26, 2026 12:43
@mesa-dot-dev

mesa-dot-dev Bot commented Jan 26, 2026

Copy link
Copy Markdown

You do not have enough credits to review this pull request. Please purchase more credits to continue.

@shunkakinoki
shunkakinoki enabled auto-merge (squash) January 26, 2026 12:44
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @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 enhances the cliproxyapi service by introducing robust usage statistics persistence. It ensures that usage data is automatically saved when the service shuts down and restored upon startup, preventing data loss. Additionally, it refactors the Docker image pulling process into the service's startup script and implements graceful shutdown procedures to maintain data integrity.

Highlights

  • Usage Statistics Persistence: Implemented automatic backup of usage statistics on service exit and restore on service start for the cliproxyapi service.
  • Docker Pull Logic Refactor: Moved the Docker image pull operation for cliproxyapi from the Makefile to the start.sh script, centralizing startup logic.
  • Graceful Shutdown: Added trap handlers in start.sh to ensure usage statistics are exported during graceful service termination (EXIT, TERM, INT signals).
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai

coderabbitai Bot commented Jan 26, 2026

Copy link
Copy Markdown

Note

Other AI code review bot(s) detected

CodeRabbit 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.

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Automatic usage statistics import/export during service start and exit
  • Improvements

    • Refactored startup flow for Linux and macOS to run the service in the background with coordinated shutdown
    • Waits for management endpoint readiness before importing usage data
    • Removed automatic Docker image pull during service restart
  • Documentation

    • Updated service README and usage guidance to reflect new startup and usage-data behavior

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Adds usage-statistics import/export and management polling to the cliproxyapi startup script; reworks background startup and coordinated shutdown for containers/binaries. Removes docker pull logic from the Makefile's systemctl-cliproxyapi target. Documentation updated to reflect service-oriented startup and usage handling.

Changes

Cohort / File(s) Summary
Build configuration
Makefile
Removed the docker pull block from the systemctl-cliproxyapi target so the service restart proceeds without attempting an image pull
Service startup & docs
home-manager/services/cliproxyapi/README.md, home-manager/services/cliproxyapi/scripts/start.sh
Added MANAGEMENT_URL / MANAGEMENT_KEY / USAGE_EXPORT_FILE, usage_import/export functions, wait_for_management polling, EXIT/TERM/INT traps and child PID tracking; Linux Docker and macOS Homebrew flows run background processes, perform management import/export and coordinated shutdown; README text updated to reflect service start and usage behavior

Sequence Diagram(s)

sequenceDiagram
    participant Script as start.sh (script)
    participant Host as Docker / Homebrew (host runtime)
    participant Management as Management Endpoint

    Script->>Host: Launch container or binary in background
    Note over Script: Record child PID
    Script->>Management: Poll readiness (wait_for_management)
    loop Poll until ready or timeout
        Management-->>Script: Health/ready response
    end
    Script->>Management: POST usage data (usage_import)
    Management-->>Script: Acknowledge import
    Script->>Host: Wait for child process to exit
    Host-->>Script: Child process exits (status)
    Script->>Management: GET/export usage data (usage_export)
    Management-->>Script: Return usage payload (saved to file)
    Script->>Script: Cleanup and exit with child status
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I burrowed in scripts at the break of day,
Backed up the numbers, then hopped away.
No more pulls in Makefile, just a clean restart,
Management listens while I do my part.
Background paws tidy, the service sleeps tight.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: add usage statistics persistence for cliproxyapi' accurately summarizes the main change: adding automatic usage statistics backup and restore functionality.
Description check ✅ Passed The description provides detailed information about the changes, technical implementation, and testing performed, all directly related to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/cliproxyapi-usage-persistence

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mesa-dot-dev

mesa-dot-dev Bot commented Jan 26, 2026

Copy link
Copy Markdown

Mesa Description

TL;DR

Persisted cliproxyapi usage statistics across restarts by implementing automatic export on shutdown and import on startup, alongside moving Docker pull logic and adding graceful shutdown traps.

What changed?

  • Makefile: Removed docker pull operation from the systemctl-cliproxyapi target.
  • home-manager/services/cliproxyapi/README.md: Updated to clarify start.sh's role in service management and noted the automatic backup and restoration of usage statistics.
  • home-manager/services/cliproxyapi/scripts/start.sh:
    • Introduced usage_import, usage_export, and wait_for_management functions, along with new environment variables, to interact with the management API.
    • Implemented usage data import on service startup.
    • Added trap handlers to gracefully export usage data on script exit.
    • Configured the service to run in the background and wait for its management API to be ready.
    • Incorporated Docker image pull logic for Docker installations before service startup.

Description generated by Mesa. Update settings

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a valuable feature for persisting usage statistics across service restarts. The approach of using trap handlers for graceful shutdown and curl for interacting with the management API is sound. The refactoring of the Docker pull logic into start.sh is also a good improvement for centralizing service startup logic.

My review has identified a few issues in the start.sh script. There is a critical issue in how the management API key is handled, which could lead to authentication failures. I've also found a high-severity issue with the EXIT trap that would cause it to fail on every normal service exit. Additionally, there is an opportunity to improve maintainability by refactoring duplicated code blocks. Addressing these points will make the implementation more robust and easier to maintain.

Comment on lines 30 to +31
MANAGEMENT_PASSWORD="${CLIPROXY_MANAGEMENT_PASSWORD:-}"
MANAGEMENT_KEY="${CLIPROXY_MANAGEMENT_PASSWORD:-${CLIPROXY_MANAGEMENT_KEY:-}}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There's a potential bug in how MANAGEMENT_PASSWORD and MANAGEMENT_KEY are derived. If CLIPROXY_MANAGEMENT_KEY is set but CLIPROXY_MANAGEMENT_PASSWORD is not, MANAGEMENT_PASSWORD will be empty. This empty password is then used to generate config.yaml (via sed on line 79) and passed as an environment variable to the Docker container. However, MANAGEMENT_KEY will hold the value of CLIPROXY_MANAGEMENT_KEY, and this script will use it to make API calls, which will fail due to the password mismatch.

To fix this, the service must be configured with the correct key, and the script must use that same key. The suggested change unifies the key derivation. Note: For this to work completely, you must also update the sed command on line 79 to use ${MANAGEMENT_PASSWORD} instead of ${CLIPROXY_MANAGEMENT_PASSWORD:-}.

Suggested change
MANAGEMENT_PASSWORD="${CLIPROXY_MANAGEMENT_PASSWORD:-}"
MANAGEMENT_KEY="${CLIPROXY_MANAGEMENT_PASSWORD:-${CLIPROXY_MANAGEMENT_KEY:-}}"
MANAGEMENT_KEY="${CLIPROXY_MANAGEMENT_KEY:-${CLIPROXY_MANAGEMENT_PASSWORD:-}}"
MANAGEMENT_PASSWORD="${MANAGEMENT_KEY}"

}

child_pid=""
trap 'usage_export' EXIT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The EXIT trap as currently implemented will cause issues. The EXIT trap is executed when the script exits. In the normal flow of this script, it exits after the child process (the cliproxyapi service) has terminated. At that point, the service is no longer running, so the usage_export call will fail to connect to the management API.

This trap is redundant because the TERM/INT trap already handles exporting usage statistics before terminating the service in graceful shutdown scenarios. For abnormal exits (like the service crashing), this EXIT trap wouldn't be able to save the stats before the service is gone.

I recommend removing the EXIT trap to avoid failed API calls on every normal service exit.

Comment on lines +202 to +206
child_pid=$!
wait_for_management || true
usage_import
wait "$child_pid"
exit $?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block of logic for setting the child_pid, waiting for the management API, importing usage, and waiting for the process is duplicated for the macOS execution paths (lines 212-216 and 219-223). This code repetition makes the script harder to maintain.

To improve this, you could extract the common logic into a helper function. For example, you could define this function before the platform-specific if blocks:

run_and_wait() {
  child_pid=$!
  wait_for_management || true
  usage_import
  wait "$child_pid"
  exit $?
}

Then, you can replace this block and the other duplicated blocks with a single call: run_and_wait.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@home-manager/services/cliproxyapi/scripts/start.sh`:
- Around line 179-181: The TERM/INT trap redundantly calls usage_export and then
the EXIT trap runs it again; update the TERM/INT trap to disable the EXIT trap
first (e.g., run trap - EXIT) before invoking usage_export and terminating the
child process so usage_export only runs once; reference the existing trap that
mentions usage_export, child_pid, and the TERM/INT trap and ensure you preserve
the kill/wait logic and redirecting errors while removing the duplicate EXIT
invocation.
- Around line 161-177: The readiness check in wait_for_management uses curl but
doesn't treat HTTP 4xx/5xx as failures; update the curl invocation inside
wait_for_management to include --fail so non-2xx responses cause curl to return
a non-zero exit code (keep existing -sS, -H "Authorization: Bearer
${MANAGEMENT_KEY}" and -o /dev/null options intact) so the loop will retry on
unauthorized or server errors.
🧹 Nitpick comments (1)
home-manager/services/cliproxyapi/scripts/start.sh (1)

210-223: Consider extracting duplicated startup logic.

The startup sequence (background exec, PID tracking, wait, import, wait, exit) is identical for both Homebrew paths. This could be refactored to reduce duplication, though it's a minor concern given the script's scope.

Optional refactor
+run_binary() {
+  local binary="$1"
+  shift
+  "$binary" -config "$CONFIG" "$@" &
+  child_pid=$!
+  wait_for_management || true
+  usage_import
+  wait "$child_pid"
+  exit $?
+}
+
 # macOS: Homebrew binary
 if [ -x /opt/homebrew/bin/cliproxyapi ]; then
-  /opt/homebrew/bin/cliproxyapi -config "$CONFIG" "$@" &
-  child_pid=$!
-  wait_for_management || true
-  usage_import
-  wait "$child_pid"
-  exit $?
+  run_binary /opt/homebrew/bin/cliproxyapi "$@"
 elif [ -x /usr/local/bin/cliproxyapi ]; then
-  /usr/local/bin/cliproxyapi -config "$CONFIG" "$@" &
-  child_pid=$!
-  wait_for_management || true
-  usage_import
-  wait "$child_pid"
-  exit $?
+  run_binary /usr/local/bin/cliproxyapi "$@"
 else

Comment on lines +161 to +177
wait_for_management() {
if [ -z "$MANAGEMENT_KEY" ]; then
return 0
fi
local attempts=60
local i
for i in $(seq 1 "$attempts"); do
if curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o /dev/null; then
return 0
fi
sleep 3
done
return 1
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add --fail to validate HTTP status in readiness check.

The curl command returns exit code 0 for any HTTP response, including 4xx/5xx errors. This means a 401 Unauthorized or 500 Server Error would be incorrectly treated as "ready."

Proposed fix
     if curl -sS \
+      --fail \
       -H "Authorization: Bearer ${MANAGEMENT_KEY}" \
       "${MANAGEMENT_URL}/usage/export" \
       -o /dev/null; then
📝 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.

Suggested change
wait_for_management() {
if [ -z "$MANAGEMENT_KEY" ]; then
return 0
fi
local attempts=60
local i
for i in $(seq 1 "$attempts"); do
if curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o /dev/null; then
return 0
fi
sleep 3
done
return 1
}
wait_for_management() {
if [ -z "$MANAGEMENT_KEY" ]; then
return 0
fi
local attempts=60
local i
for i in $(seq 1 "$attempts"); do
if curl -sS \
--fail \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o /dev/null; then
return 0
fi
sleep 3
done
return 1
}
🤖 Prompt for AI Agents
In `@home-manager/services/cliproxyapi/scripts/start.sh` around lines 161 - 177,
The readiness check in wait_for_management uses curl but doesn't treat HTTP
4xx/5xx as failures; update the curl invocation inside wait_for_management to
include --fail so non-2xx responses cause curl to return a non-zero exit code
(keep existing -sS, -H "Authorization: Bearer ${MANAGEMENT_KEY}" and -o
/dev/null options intact) so the loop will retry on unauthorized or server
errors.

Comment on lines +179 to +181
child_pid=""
trap 'usage_export' EXIT
trap 'usage_export; if [ -n "$child_pid" ]; then kill -TERM "$child_pid" 2>/dev/null || true; wait "$child_pid" 2>/dev/null || true; fi' TERM INT

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Double usage_export on signal termination.

When receiving TERM/INT, the signal trap runs usage_export, then when the script exits, the EXIT trap also runs usage_export again. This is redundant and the second call may fail if the service is already down.

Proposed fix: Clear EXIT trap after signal handler runs
 child_pid=""
 trap 'usage_export' EXIT
-trap 'usage_export; if [ -n "$child_pid" ]; then kill -TERM "$child_pid" 2>/dev/null || true; wait "$child_pid" 2>/dev/null || true; fi' TERM INT
+trap 'trap - EXIT; usage_export; if [ -n "$child_pid" ]; then kill -TERM "$child_pid" 2>/dev/null || true; wait "$child_pid" 2>/dev/null || true; fi' TERM INT
📝 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.

Suggested change
child_pid=""
trap 'usage_export' EXIT
trap 'usage_export; if [ -n "$child_pid" ]; then kill -TERM "$child_pid" 2>/dev/null || true; wait "$child_pid" 2>/dev/null || true; fi' TERM INT
child_pid=""
trap 'usage_export' EXIT
trap 'trap - EXIT; usage_export; if [ -n "$child_pid" ]; then kill -TERM "$child_pid" 2>/dev/null || true; wait "$child_pid" 2>/dev/null || true; fi' TERM INT
🤖 Prompt for AI Agents
In `@home-manager/services/cliproxyapi/scripts/start.sh` around lines 179 - 181,
The TERM/INT trap redundantly calls usage_export and then the EXIT trap runs it
again; update the TERM/INT trap to disable the EXIT trap first (e.g., run trap -
EXIT) before invoking usage_export and terminating the child process so
usage_export only runs once; reference the existing trap that mentions
usage_export, child_pid, and the TERM/INT trap and ensure you preserve the
kill/wait logic and redirecting errors while removing the duplicate EXIT
invocation.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds automatic persistence of cliproxyapi usage statistics by exporting on shutdown and importing on startup, and relocates Docker image pull logic from the Makefile into the service start script.

Changes:

  • Add usage stats export/import + management readiness wait + signal/exit traps in start.sh.
  • Update service docs to reflect new start behavior.
  • Remove Docker pull from systemctl-cliproxyapi Makefile target.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.

File Description
home-manager/services/cliproxyapi/scripts/start.sh Implements usage export/import persistence and moves Docker pull into startup flow.
home-manager/services/cliproxyapi/README.md Updates script purpose wording and notes automatic usage backup/restore.
Makefile Removes Docker pull step from the cliproxyapi systemctl target.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o "$USAGE_EXPORT_FILE" || true

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usage-export.json is written without explicitly restricting file permissions. Depending on the user’s umask, this file could be group/world-readable and may contain sensitive usage metadata. Consider setting a restrictive umask for the export write or chmod 600 the file after a successful export.

Suggested change
-o "$USAGE_EXPORT_FILE" || true
-o "$USAGE_EXPORT_FILE" && chmod 600 "$USAGE_EXPORT_FILE" || true

Copilot uses AI. Check for mistakes.
Comment on lines +138 to +181
usage_import() {
if [ -z "$MANAGEMENT_KEY" ] || [ ! -f "$USAGE_EXPORT_FILE" ]; then
return 0
fi
curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
-H "Content-Type: application/json" \
-X POST \
--data-binary @"$USAGE_EXPORT_FILE" \
"${MANAGEMENT_URL}/usage/import" >/dev/null || true
}

usage_export() {
if [ -z "$MANAGEMENT_KEY" ]; then
return 0
fi
mkdir -p "$(dirname "$USAGE_EXPORT_FILE")"
curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o "$USAGE_EXPORT_FILE" || true
}

wait_for_management() {
if [ -z "$MANAGEMENT_KEY" ]; then
return 0
fi
local attempts=60
local i
for i in $(seq 1 "$attempts"); do
if curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o /dev/null; then
return 0
fi
sleep 3
done
return 1
}

child_pid=""
trap 'usage_export' EXIT
trap 'usage_export; if [ -n "$child_pid" ]; then kill -TERM "$child_pid" 2>/dev/null || true; wait "$child_pid" 2>/dev/null || true; fi' TERM INT

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds non-trivial new startup/shutdown behavior (usage import/export, readiness polling, traps), but the existing ShellSpec coverage for start.sh doesn’t exercise these paths. Adding tests that mock curl and verify (1) no calls happen without a management key, (2) import happens after readiness, and (3) export runs on exit would help prevent regressions.

Copilot uses AI. Check for mistakes.
Comment thread Makefile
@@ -704,12 +704,6 @@ systemctl: systemctl-cliproxyapi systemctl-clawdbot systemctl-code-syncer system

.PHONY: systemctl-cliproxyapi
systemctl-cliproxyapi: ## Pull latest image and restart cliproxyapi systemd user service.

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The target description still says it pulls the latest image, but the pull logic was removed from this recipe. Update the help text to match current behavior (or restore the pull step if the description is still intended).

Suggested change
systemctl-cliproxyapi: ## Pull latest image and restart cliproxyapi systemd user service.
systemctl-cliproxyapi: ## Restart cliproxyapi systemd user service.

Copilot uses AI. Check for mistakes.
# OAuth login
cliproxyapi --claude-login

# Usage statistics backup/restore happens automatically on start/exit

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README now states usage statistics backup/restore happens automatically, but in start.sh it is a no-op unless a management key is configured (and the Management API/usage stats are enabled). Consider clarifying the prerequisites here (e.g., requires CLIPROXY_MANAGEMENT_PASSWORD/key and Management API enabled).

Suggested change
# Usage statistics backup/restore happens automatically on start/exit
# Usage statistics backup/restore happens automatically on start/exit
# (requires Management API with usage stats enabled and CLIPROXY_MANAGEMENT_PASSWORD/management key configured)

Copilot uses AI. Check for mistakes.
Comment on lines +168 to +172
if curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o /dev/null; then
return 0

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait_for_management treats any HTTP response as success because curl returns exit code 0 for 4xx/5xx unless --fail/--fail-with-body is used. This can mark the management API as “ready” even when the key is invalid (e.g., 401), and then usage_import/usage_export will operate against an unauthorized endpoint. Consider checking the HTTP status explicitly (or using --fail-with-body) so only a 2xx response is treated as ready.

Copilot uses AI. Check for mistakes.
Comment on lines +155 to +158
curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o "$USAGE_EXPORT_FILE" || true

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usage_export writes directly to usage-export.json even when the request fails (and without curl --fail), which risks overwriting a good backup with an error page/JSON error response. A safer pattern is to require a successful HTTP status and write to a temp file, then atomically move it into place on success.

Suggested change
curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o "$USAGE_EXPORT_FILE" || true
local tmp_file="${USAGE_EXPORT_FILE}.tmp.$$"
if curl -sS --fail \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o "$tmp_file"; then
mv "$tmp_file" "$USAGE_EXPORT_FILE"
else
rm -f "$tmp_file" || true
fi

Copilot uses AI. Check for mistakes.
Comment on lines +142 to +158
curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
-H "Content-Type: application/json" \
-X POST \
--data-binary @"$USAGE_EXPORT_FILE" \
"${MANAGEMENT_URL}/usage/import" >/dev/null || true
}

usage_export() {
if [ -z "$MANAGEMENT_KEY" ]; then
return 0
fi
mkdir -p "$(dirname "$USAGE_EXPORT_FILE")"
curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o "$USAGE_EXPORT_FILE" || true

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All curl calls here have no connect/overall timeouts. Since usage_export runs in EXIT/TERM/INT traps, a stalled TCP connect (e.g., misconfigured CLIPROXY_MANAGEMENT_URL) can block shutdown indefinitely. Consider adding --connect-timeout and --max-time (or similar) to each curl invocation.

Copilot uses AI. Check for mistakes.
Comment on lines +142 to +170
curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
-H "Content-Type: application/json" \
-X POST \
--data-binary @"$USAGE_EXPORT_FILE" \
"${MANAGEMENT_URL}/usage/import" >/dev/null || true
}

usage_export() {
if [ -z "$MANAGEMENT_KEY" ]; then
return 0
fi
mkdir -p "$(dirname "$USAGE_EXPORT_FILE")"
curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \
-o "$USAGE_EXPORT_FILE" || true
}

wait_for_management() {
if [ -z "$MANAGEMENT_KEY" ]; then
return 0
fi
local attempts=60
local i
for i in $(seq 1 "$attempts"); do
if curl -sS \
-H "Authorization: Bearer ${MANAGEMENT_KEY}" \
"${MANAGEMENT_URL}/usage/export" \

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The curl invocations include the MANAGEMENT_KEY directly in an Authorization header on the process command line, which can leak this secret via ps//proc to other local users or monitoring tooling. An attacker with local access could capture the bearer token while these commands run and reuse it to call the management API with full privileges. To mitigate this, avoid placing secrets in command-line arguments (e.g., pass the token via environment variables, a config file with restricted permissions, or curl config, and ensure it is not exposed in process listings).

Copilot uses AI. Check for mistakes.
Comment on lines +184 to +201
if docker info >/dev/null 2>&1; then
echo "🔄 Pulling latest cliproxyapi image..."
docker pull eceasy/cli-proxy-api:latest || true
else
echo "⏭️ Skipping docker pull (docker not accessible)"
fi

docker rm -f cliproxyapi 2>/dev/null || true
mkdir -p "$CONFIG_DIR/logs"
exec docker run --rm \
docker run --rm \
--name cliproxyapi \
--network host \
--ulimit nofile=65536:65536 \
-v "$CONFIG:/CLIProxyAPI/config.yaml:ro" \
-v "$CONFIG_DIR:/root/.cli-proxy-api" \
-v "$CONFIG_DIR/logs:/CLIProxyAPI/logs" \
-e MANAGEMENT_PASSWORD="${MANAGEMENT_PASSWORD:-}" \
eceasy/cli-proxy-api:latest
eceasy/cli-proxy-api:latest &

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This script pulls and runs the Docker image eceasy/cli-proxy-api:latest using a mutable latest tag from Docker Hub, which creates a supply chain risk if the upstream image is compromised or the tag is retagged maliciously. Because the container is started with access to configuration, auth directories, and management credentials, a poisoned image would have full control over the service and associated secrets. To reduce this risk, pin the image to an immutable reference (such as a specific version tag or image digest) and update it explicitly when upgrading.

Copilot uses AI. Check for mistakes.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

@shunkakinoki
shunkakinoki merged commit 5cb1c46 into main Jan 26, 2026
29 of 30 checks passed
@shunkakinoki
shunkakinoki deleted the feat/cliproxyapi-usage-persistence branch January 26, 2026 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants