Make backup wrapper self-contained#281
Conversation
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.
|
@coderabbitai review |
|
@codex review |
✅ Actions performedReview triggered.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbffe40d5a
ℹ️ 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".
|
|
||
| cp "$src" "$dst" | ||
| sed \ | ||
| -e "s|__BRAINLAYER_DIR__|$BRAINLAYER_DIR|g" \ |
There was a problem hiding this comment.
Escape BRAINLAYER_DIR before sed replacement
When the repository path contains sed replacement metacharacters such as & (for example a checkout under ~/R&D/brainlayer), this substitution writes a corrupted path into the installed wrapper because & expands to the matched placeholder in the replacement. The resulting PYTHONPATH points at a non-existent directory, so manual or launchd backup runs can fail to import brainlayer; escape the replacement string or use a safer templating approach before writing the wrapper.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bbffe40. Configure here.
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b4011a640
ℹ️ 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".
| cp "$src" "$dst" | ||
| escaped_brainlayer_dir="$(printf '%s' "$BRAINLAYER_DIR" | sed 's/[\\&|]/\\&/g')" | ||
| sed \ | ||
| -e "s|__BRAINLAYER_DIR_VALUE__|$escaped_brainlayer_dir|g" \ |
There was a problem hiding this comment.
Quote the embedded repository path for the shell
When the checkout path contains shell syntax characters such as ", `, or $(, this writes the raw path into the generated backup-daily.sh assignment; sed escaping only protects the sed replacement, not the shell source that will be parsed later. For example a repo under a directory containing " produces an unterminated BRAINLAYER_DIR="${BRAINLAYER_DIR:-...}" line, so manual and launchd backup runs fail before importing brainlayer; use shell-safe quoting/escaping for the value before embedding it.
Useful? React with 👍 / 👎.

Summary
scripts/launchd/backup-daily.shsetPYTHONPATHitself using installed BrainLayer repo path__BRAINLAYER_DIR__into the copied wrapperWhy
Manual execution of
~/.local/lib/brainlayer/backup-daily.shfailed after PR #280 becausePYTHONPATHwas only provided by the LaunchAgent plist. The wrapper needs to work both from launchd and direct manual restore/proof runs.Verification
python3 -m pytest tests/test_backup_daily.py -q->4 passedbash -n scripts/launchd/backup-daily.sh scripts/launchd/install.sh-> passed1827 passed, 9 skipped, 75 deselected, 1 xfailed; MCP registration3 passed; eval/hook routing32 passed; Bun1 pass; regression shell passNote
Low Risk
Low risk shell-script change that only affects the launchd-installed daily backup wrapper and its install-time templating; main risk is mis-resolving
BRAINLAYER_DIRor bad escaping causing backups to fail to run.Overview
Makes
backup-daily.shself-contained by resolvingBRAINLAYER_DIR(env or install-time placeholder, with a fallback) and exportingPYTHONPATHto includesrcbefore runningbrainlayer.backup_daily.Updates the launchd installer to generate the installed backup wrapper via
sedsubstitution of__BRAINLAYER_DIR_VALUE__(with escaping) instead of copying verbatim, and extendstest_backup_daily.pyto assert the placeholder/substitution plumbing and wrapperPYTHONPATHsetup are present.Reviewed by Cursor Bugbot for commit 2b4011a. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Make backup wrapper self-contained by templating
BRAINLAYER_DIRandPYTHONPATHat install time__BRAINLAYER_DIR_VALUE__placeholder and setsPYTHONPATHto include$BRAINLAYER_DIR/src, rather than relying on the environment at runtime.BRAINLAYER_DIRvalue (with special characters escaped) viasedwhen copying the script, instead of a verbatimcp.PYTHONPATHare present in the wrapper and that the installer references the substitution logic.Macroscope summarized 2b4011a.