fix(cliproxyapi): enhance auth recovery and atomic swap processes - #498
Conversation
- Added a new activation script to ensure the authentication cache for cliproxyapi is populated immediately after a home-manager switch. This prevents issues with missing auth files during the first CLI invocation post-rebuild. chore(package): update dependencies - Added @mariozechner/pi-coding-agent to package.json dependencies.
… multiple sources
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe changes enhance the cliproxyapi service startup flow by implementing multi-source authentication recovery (CCS, macOS dotfiles, R2 backup) before atomic swap operations, adding a Home Manager activation hook for early auth cache hydration, and introducing a new coding agent dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant HM as Home Manager
participant Act as Activation Hook
participant Start as start.sh
participant Auth as Auth Sources
participant OS as ObjectStore
HM->>Act: Trigger hydrateCliproxyAuths
Act->>Start: Execute backupAuthScript
Start->>Auth: Check TMP_AUTH_DIR
alt TMP_AUTH_DIR empty
Start->>Auth: Recover from CCS_AUTH_DIR
Auth-->>Start: Copy files (if exist)
Start->>Auth: Recover from dotfiles backup
Auth-->>Start: Merge files (ignore-existing)
Start->>Auth: Recover from R2 backup
Auth-->>Start: Sync files via S3
end
alt Files recovered
Start->>OS: Perform atomic swap to objectstore/auths/
OS-->>Start: Swap complete
else No files recovered
Start->>OS: Preserve existing cache
OS-->>Start: Cache retained
end
Start->>OS: Bootstrap fallback if needed
OS-->>Start: Auth ready
Start-->>Act: Script completes (tolerant)
Act-->>HM: Activation proceeds
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
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 significantly improves the reliability and resilience of the 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;DREnhanced What changed?
Description generated by Mesa. Update settings |
![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 e35dac3...61a9dee
Analysis
-
Error handling relies on "silent failure" patterns (
|| true) throughout recovery processes, making authentication failures difficult to detect and debug while allowing the system to operate in a degraded state without indication. -
Significant code duplication exists between initial sync operations and recovery logic (lines 46-66 and 70-90), violating DRY principles and creating maintenance challenges if sync logic changes.
-
The recovery process lacks proper logging or visibility into failure states, which complicates troubleshooting authentication issues in production.
-
An apparently unrelated dependency (@mariozechner/pi-coding-agent) was added to package.json without clear connection to the PR's authentication recovery enhancements.
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
0 files reviewed | 3 comments | Edit Agent Settings • Read Docs
| echo "⚠️ Temp auth dir empty; attempting recovery from CCS/dotfiles/R2 backup" >&2 | ||
| # CCS -> TMP | ||
| if [ -d "$CCS_AUTH_DIR" ] && [ -n "$(ls -A "$CCS_AUTH_DIR" 2>/dev/null)" ]; then | ||
| @rsync@ -a "$CCS_AUTH_DIR/" "$TMP_AUTH_DIR/" |
There was a problem hiding this comment.
The recovery logic duplicates the sync operations from earlier in the script (lines 46-66). This creates maintenance burden and potential for divergence. If the initial sync succeeds but creates an empty temp directory (edge case), this recovery will re-run the same operations. Consider extracting the sync logic into a reusable function to avoid duplication and ensure consistency.
Agent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#498
File: home-manager/services/cliproxyapi/scripts/start.sh#L74
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
The recovery logic duplicates the sync operations from earlier in the script (lines 46-66). This creates maintenance burden and potential for divergence. If the initial sync succeeds but creates an empty temp directory (edge case), this recovery will re-run the same operations. Consider extracting the sync logic into a reusable function to avoid duplication and ensure consistency.
| # so first CLI invocation after a rebuild doesn't hit missing auth files. | ||
| home.activation = lib.optionalAttrs (lib ? hm && lib.hm ? dag) { | ||
| hydrateCliproxyAuths = lib.hm.dag.entryAfter [ "writeBoundary" ] '' | ||
| ${pkgs.bash}/bin/bash ${backupAuthScript} || true |
There was a problem hiding this comment.
Using || true to silence failures in home.activation could mask genuine errors during auth cache hydration. If the backup auth script fails (e.g., due to permission issues, missing dependencies, or network problems), the user won't be notified and may experience authentication failures later. Consider logging failures to a file or implementing a more graceful degradation strategy that at least notifies the user of hydration issues.
Agent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#498
File: home-manager/services/cliproxyapi/default.nix#L35
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
Using `|| true` to silence failures in home.activation could mask genuine errors during auth cache hydration. If the backup auth script fails (e.g., due to permission issues, missing dependencies, or network problems), the user won't be notified and may experience authentication failures later. Consider logging failures to a file or implementing a more graceful degradation strategy that at least notifies the user of hydration issues.
| @rsync@ -a --ignore-existing "$DOTFILES_AUTH_DIR/" "$TMP_AUTH_DIR/" | ||
| fi | ||
| # R2 backup -> TMP | ||
| if [ -n "${OBJECTSTORE_ENDPOINT:-}" ] && [ -n "${OBJECTSTORE_ACCESS_KEY:-}" ]; then |
There was a problem hiding this comment.
This R2 backup recovery silently fails with || true, which means if the recovery itself encounters an error (network timeout, authentication failure), the script will continue without notification. Combined with the outer check at line 71 that only verifies if temp dir is empty (not WHY it's empty), you could have a scenario where all recovery attempts fail silently, leaving the system in an unknown state. Consider accumulating recovery attempt results and logging them for debugging.
Agent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#498
File: home-manager/services/cliproxyapi/scripts/start.sh#L81
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
This R2 backup recovery silently fails with `|| true`, which means if the recovery itself encounters an error (network timeout, authentication failure), the script will continue without notification. Combined with the outer check at line 71 that only verifies if temp dir is empty (not WHY it's empty), you could have a scenario where all recovery attempts fail silently, leaving the system in an unknown state. Consider accumulating recovery attempt results and logging them for debugging.
There was a problem hiding this comment.
Pull request overview
This PR enhances the cliproxyapi authentication recovery mechanisms by adding fallback logic when the temporary auth directory is empty, and introduces automatic auth cache hydration after home-manager configuration switches. Additionally, it adds the @mariozechner/pi-coding-agent package as a new dependency.
- Added recovery logic to attempt syncing from multiple sources (CCS, dotfiles, R2 backup) when the temp auth directory is empty
- Implemented optional auth cache hydration via home-manager activation hooks
- Added new npm dependency for pi-coding-agent tooling
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds @mariozechner/pi-coding-agent@^0.33.0 dependency |
| home-manager/services/cliproxyapi/scripts/start.sh | Implements recovery logic when temp auth dir is empty, with fallback syncs from CCS/dotfiles/R2 |
| home-manager/services/cliproxyapi/default.nix | Adds home.activation hook to hydrate auth cache after home-manager switches |
| home-manager/services/cliproxyapi/README.md | Updates documentation to describe the enhanced atomic swap with recovery process |
| bun.lock | Updates lock file to include new pi-coding-agent package and its dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # If TMP is empty, attempt recovery from CCS/dotfiles/R2 backup; otherwise keep cache. | ||
| if [ -z "$(ls -A "$TMP_AUTH_DIR" 2>/dev/null)" ]; then | ||
| echo "⚠️ Temp auth dir empty; attempting recovery from CCS/dotfiles/R2 backup" >&2 | ||
| # CCS -> TMP | ||
| if [ -d "$CCS_AUTH_DIR" ] && [ -n "$(ls -A "$CCS_AUTH_DIR" 2>/dev/null)" ]; then | ||
| @rsync@ -a "$CCS_AUTH_DIR/" "$TMP_AUTH_DIR/" | ||
| fi | ||
| # dotfiles (macOS) -> TMP | ||
| if [ "$(uname)" = "Darwin" ] && [ -d "$DOTFILES_AUTH_DIR" ] && [ -n "$(ls -A "$DOTFILES_AUTH_DIR" 2>/dev/null)" ]; then | ||
| @rsync@ -a --ignore-existing "$DOTFILES_AUTH_DIR/" "$TMP_AUTH_DIR/" | ||
| fi | ||
| # R2 backup -> TMP | ||
| if [ -n "${OBJECTSTORE_ENDPOINT:-}" ] && [ -n "${OBJECTSTORE_ACCESS_KEY:-}" ]; then | ||
| AWS_ACCESS_KEY_ID="${OBJECTSTORE_ACCESS_KEY}" \ | ||
| AWS_SECRET_ACCESS_KEY="${OBJECTSTORE_SECRET_KEY}" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="${OBJECTSTORE_ENDPOINT}" \ | ||
| --no-progress \ | ||
| "s3://cliproxyapi/backup/auths/" \ | ||
| "$TMP_AUTH_DIR/" 2>/dev/null || true | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The recovery logic in lines 70-90 is largely redundant with the sync operations that already occurred in lines 57-66 and 63-66. The CCS sync (lines 63-66) and dotfiles sync (lines 57-60) already populated TMP_AUTH_DIR before this check. The recovery block will re-sync from the same sources that were just synced, which is unnecessary.
Consider removing the redundant recovery block, or restructure the logic so that the initial sync operations (lines 57-66) are conditional and only happen when TMP_AUTH_DIR is not empty after the R2 sync.
| # If TMP is empty, attempt recovery from CCS/dotfiles/R2 backup; otherwise keep cache. | |
| if [ -z "$(ls -A "$TMP_AUTH_DIR" 2>/dev/null)" ]; then | |
| echo "⚠️ Temp auth dir empty; attempting recovery from CCS/dotfiles/R2 backup" >&2 | |
| # CCS -> TMP | |
| if [ -d "$CCS_AUTH_DIR" ] && [ -n "$(ls -A "$CCS_AUTH_DIR" 2>/dev/null)" ]; then | |
| @rsync@ -a "$CCS_AUTH_DIR/" "$TMP_AUTH_DIR/" | |
| fi | |
| # dotfiles (macOS) -> TMP | |
| if [ "$(uname)" = "Darwin" ] && [ -d "$DOTFILES_AUTH_DIR" ] && [ -n "$(ls -A "$DOTFILES_AUTH_DIR" 2>/dev/null)" ]; then | |
| @rsync@ -a --ignore-existing "$DOTFILES_AUTH_DIR/" "$TMP_AUTH_DIR/" | |
| fi | |
| # R2 backup -> TMP | |
| if [ -n "${OBJECTSTORE_ENDPOINT:-}" ] && [ -n "${OBJECTSTORE_ACCESS_KEY:-}" ]; then | |
| AWS_ACCESS_KEY_ID="${OBJECTSTORE_ACCESS_KEY}" \ | |
| AWS_SECRET_ACCESS_KEY="${OBJECTSTORE_SECRET_KEY}" \ | |
| @aws@ s3 sync \ | |
| --endpoint-url="${OBJECTSTORE_ENDPOINT}" \ | |
| --no-progress \ | |
| "s3://cliproxyapi/backup/auths/" \ | |
| "$TMP_AUTH_DIR/" 2>/dev/null || true | |
| fi | |
| fi | |
| # If TMP is empty, preserve existing cache; later bootstrap may copy from dotfiles. |
| home.activation = lib.optionalAttrs (lib ? hm && lib.hm ? dag) { | ||
| hydrateCliproxyAuths = lib.hm.dag.entryAfter [ "writeBoundary" ] '' | ||
| ${pkgs.bash}/bin/bash ${backupAuthScript} || true | ||
| ''; | ||
| }; |
There was a problem hiding this comment.
The condition lib ? hm && lib.hm ? dag checks if lib has an hm attribute and if lib.hm has a dag attribute. However, lib.hm.dag is not a standard attribute in the Nix home-manager library structure. The home-manager DAG functions are typically available directly via lib.hm.dag when using home-manager modules.
This condition may never evaluate to true, causing the activation hook to never be registered. Consider verifying the correct way to check for home-manager DAG availability, or simply remove the conditional wrapper if this module is always used with home-manager.
| home.activation = lib.optionalAttrs (lib ? hm && lib.hm ? dag) { | |
| hydrateCliproxyAuths = lib.hm.dag.entryAfter [ "writeBoundary" ] '' | |
| ${pkgs.bash}/bin/bash ${backupAuthScript} || true | |
| ''; | |
| }; | |
| home.activation.hydrateCliproxyAuths = lib.hm.dag.entryAfter [ "writeBoundary" ] '' | |
| ${pkgs.bash}/bin/bash ${backupAuthScript} || true | |
| ''; |
| fi | ||
| # dotfiles (macOS) -> TMP | ||
| if [ "$(uname)" = "Darwin" ] && [ -d "$DOTFILES_AUTH_DIR" ] && [ -n "$(ls -A "$DOTFILES_AUTH_DIR" 2>/dev/null)" ]; then | ||
| @rsync@ -a --ignore-existing "$DOTFILES_AUTH_DIR/" "$TMP_AUTH_DIR/" |
There was a problem hiding this comment.
The --ignore-existing flag is used here but not in line 74 below. This inconsistency could lead to different behavior in the recovery path versus the normal sync path. In line 74 (inside the recovery block), files from dotfiles would overwrite existing files in TMP_AUTH_DIR, while in line 78 they would be skipped.
Consider using consistent rsync flags in both locations to ensure predictable behavior, or document why the behavior should differ between the two code paths.
| rm -rf "$AUTH_DIR.old" | ||
| else | ||
| echo "⚠️ Temp auth dir empty; preserving existing auth cache" >&2 | ||
| echo "⚠️ No auth files recovered; preserving existing auth cache" >&2 |
There was a problem hiding this comment.
The warning message has been updated from "Temp auth dir empty; preserving existing auth cache" to "No auth files recovered; preserving existing auth cache". However, this message is displayed even when recovery wasn't attempted at all (i.e., when the temp dir had files from the initial sync but they were successfully swapped).
The message should more accurately reflect whether recovery was actually attempted. Consider rephrasing to make it clear this is only shown when the temp directory remains empty after all sync and recovery attempts.
| echo "⚠️ No auth files recovered; preserving existing auth cache" >&2 | |
| echo "⚠️ Temp auth dir still empty after sync/recovery checks; preserving existing auth cache" >&2 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61a9deed42
ℹ️ 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".
| home.activation = lib.optionalAttrs (lib ? hm && lib.hm ? dag) { | ||
| hydrateCliproxyAuths = lib.hm.dag.entryAfter [ "writeBoundary" ] '' | ||
| ${pkgs.bash}/bin/bash ${backupAuthScript} || true |
There was a problem hiding this comment.
The new activation hook runs backupAuthScript directly, but that script does not source ~/dotfiles/.env (unlike backup-and-recover.sh). In the common setup where OBJECTSTORE_* only comes from .env (as start.sh assumes), this hook will skip the R2 pull and won’t hydrate the cache, so the “first CLI after rebuild” can still fail. Consider invoking backup-and-recover.sh or sourcing .env here so the intended hydration actually runs with credentials.
Useful? React with 👍 / 👎.
![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 enhances the authentication recovery mechanism for cliproxyapi. The changes introduce a recovery step if the temporary auth directory is empty and add a home-manager activation hook to hydrate the cache on system updates.
The core logic change is in home-manager/services/cliproxyapi/scripts/start.sh. While the intent to make auth recovery more robust is good, the implementation introduces redundant operations. The new recovery block repeats sync operations that have already been attempted. I've left a comment with a suggestion to refactor this for clarity and efficiency. Additionally, the error handling in the script could be improved to aid debugging by not swallowing all error messages.
The documentation in README.md has been updated to reflect the new logic, but it will need to be adjusted again if the script logic is refactored. The other changes in package.json, bun.lock, and default.nix look good.
| if [ -z "$(ls -A "$TMP_AUTH_DIR" 2>/dev/null)" ]; then | ||
| echo "⚠️ Temp auth dir empty; attempting recovery from CCS/dotfiles/R2 backup" >&2 | ||
| # CCS -> TMP | ||
| if [ -d "$CCS_AUTH_DIR" ] && [ -n "$(ls -A "$CCS_AUTH_DIR" 2>/dev/null)" ]; then | ||
| @rsync@ -a "$CCS_AUTH_DIR/" "$TMP_AUTH_DIR/" | ||
| fi | ||
| # dotfiles (macOS) -> TMP | ||
| if [ "$(uname)" = "Darwin" ] && [ -d "$DOTFILES_AUTH_DIR" ] && [ -n "$(ls -A "$DOTFILES_AUTH_DIR" 2>/dev/null)" ]; then | ||
| @rsync@ -a --ignore-existing "$DOTFILES_AUTH_DIR/" "$TMP_AUTH_DIR/" | ||
| fi | ||
| # R2 backup -> TMP | ||
| if [ -n "${OBJECTSTORE_ENDPOINT:-}" ] && [ -n "${OBJECTSTORE_ACCESS_KEY:-}" ]; then | ||
| AWS_ACCESS_KEY_ID="${OBJECTSTORE_ACCESS_KEY}" \ | ||
| AWS_SECRET_ACCESS_KEY="${OBJECTSTORE_SECRET_KEY}" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="${OBJECTSTORE_ENDPOINT}" \ | ||
| --no-progress \ | ||
| "s3://cliproxyapi/backup/auths/" \ | ||
| "$TMP_AUTH_DIR/" 2>/dev/null || true | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The recovery logic in this block appears to be redundant. The script has already attempted to sync from CCS, dotfiles, and the R2 backup in lines 40-66. If the temporary directory is empty at this point, it's because all those sources were empty or inaccessible. Re-running a subset of the same sync operations here will not yield a different result and makes the script harder to understand.
A cleaner approach would be to consolidate all sync/recovery logic into a single sequence of operations, then perform the atomic swap. The current implementation attempts recovery by repeating steps that have already been performed.
Also, redirecting stderr to /dev/null and using || true on the aws s3 sync command (line 88) completely swallows any errors. This can make it very difficult to debug issues with R2 connectivity or permissions. It would be better to log the error, even if the script is allowed to continue.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="home-manager/services/cliproxyapi/default.nix">
<violation number="1" location="home-manager/services/cliproxyapi/default.nix:35">
P2: The activation hook invokes the backup script without sourcing the `.env` file that provides `OBJECTSTORE_*` credentials. Since these environment variables are required for R2 connectivity, the intended cache hydration will silently skip the R2 pull, potentially leaving the auth cache empty after a rebuild. Consider sourcing `~/dotfiles/.env` before running the script, or invoking a wrapper that includes the environment setup.</violation>
</file>
<file name="home-manager/services/cliproxyapi/scripts/start.sh">
<violation number="1" location="home-manager/services/cliproxyapi/scripts/start.sh:88">
P2: The R2 backup recovery silently swallows all errors by redirecting stderr to `/dev/null` and using `|| true`. If recovery fails due to network issues, authentication problems, or other errors, the script continues without any indication of failure. This makes it very difficult to debug issues when auth recovery doesn't work as expected. Consider logging the error output while still allowing the script to continue.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| # so first CLI invocation after a rebuild doesn't hit missing auth files. | ||
| home.activation = lib.optionalAttrs (lib ? hm && lib.hm ? dag) { | ||
| hydrateCliproxyAuths = lib.hm.dag.entryAfter [ "writeBoundary" ] '' | ||
| ${pkgs.bash}/bin/bash ${backupAuthScript} || true |
There was a problem hiding this comment.
P2: The activation hook invokes the backup script without sourcing the .env file that provides OBJECTSTORE_* credentials. Since these environment variables are required for R2 connectivity, the intended cache hydration will silently skip the R2 pull, potentially leaving the auth cache empty after a rebuild. Consider sourcing ~/dotfiles/.env before running the script, or invoking a wrapper that includes the environment setup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/cliproxyapi/default.nix, line 35:
<comment>The activation hook invokes the backup script without sourcing the `.env` file that provides `OBJECTSTORE_*` credentials. Since these environment variables are required for R2 connectivity, the intended cache hydration will silently skip the R2 pull, potentially leaving the auth cache empty after a rebuild. Consider sourcing `~/dotfiles/.env` before running the script, or invoking a wrapper that includes the environment setup.</comment>
<file context>
@@ -28,6 +28,14 @@ let
+ # so first CLI invocation after a rebuild doesn't hit missing auth files.
+ home.activation = lib.optionalAttrs (lib ? hm && lib.hm ? dag) {
+ hydrateCliproxyAuths = lib.hm.dag.entryAfter [ "writeBoundary" ] ''
+ ${pkgs.bash}/bin/bash ${backupAuthScript} || true
+ '';
+ };
</file context>
| --endpoint-url="${OBJECTSTORE_ENDPOINT}" \ | ||
| --no-progress \ | ||
| "s3://cliproxyapi/backup/auths/" \ | ||
| "$TMP_AUTH_DIR/" 2>/dev/null || true |
There was a problem hiding this comment.
P2: The R2 backup recovery silently swallows all errors by redirecting stderr to /dev/null and using || true. If recovery fails due to network issues, authentication problems, or other errors, the script continues without any indication of failure. This makes it very difficult to debug issues when auth recovery doesn't work as expected. Consider logging the error output while still allowing the script to continue.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/cliproxyapi/scripts/start.sh, line 88:
<comment>The R2 backup recovery silently swallows all errors by redirecting stderr to `/dev/null` and using `|| true`. If recovery fails due to network issues, authentication problems, or other errors, the script continues without any indication of failure. This makes it very difficult to debug issues when auth recovery doesn't work as expected. Consider logging the error output while still allowing the script to continue.</comment>
<file context>
@@ -65,17 +65,38 @@ if [ -n "${OBJECTSTORE_ENDPOINT:-}" ] && [ -n "${OBJECTSTORE_ACCESS_KEY:-}" ]; t
+ --endpoint-url="${OBJECTSTORE_ENDPOINT}" \
+ --no-progress \
+ "s3://cliproxyapi/backup/auths/" \
+ "$TMP_AUTH_DIR/" 2>/dev/null || true
+ fi
+ fi
</file context>
Summary
Changes
Testing
Summary by cubic
Improved cliproxyapi auth cache robustness with recovery from multiple sources and a safe atomic swap, plus automatic cache hydration after home-manager switch to prevent missing auths on first run.
Bug Fixes
Dependencies
Written for commit 61a9dee. Summary will update on new commits.