fix: use global bun paperclipai with pino override - #1353
Conversation
Propagate overrides from dotfiles package.json to ~/.bun/install/global/ so the global binary resolves pino@9.14.0 correctly.
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 58 seconds. ⌛ 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: CHILL 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 |
Mesa DescriptionTL;DRFixes a What changed?
Description generated by Mesa. Update settings |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="home-manager/modules/npm-globals/install-npm-globals.sh">
<violation number="1" location="home-manager/modules/npm-globals/install-npm-globals.sh:58">
P2: The new jq expression overwrites existing global `overrides` instead of merging, which can silently drop previously configured override rules.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if [ -n "$OVERRIDES" ]; then | ||
| GLOBAL_PKG="${HOME}/.bun/install/global/package.json" | ||
| if [ -f "$GLOBAL_PKG" ]; then | ||
| jq --argjson overrides "$OVERRIDES" '.overrides = $overrides' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && |
There was a problem hiding this comment.
P2: The new jq expression overwrites existing global overrides instead of merging, which can silently drop previously configured override rules.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/modules/npm-globals/install-npm-globals.sh, line 58:
<comment>The new jq expression overwrites existing global `overrides` instead of merging, which can silently drop previously configured override rules.</comment>
<file context>
@@ -49,4 +49,17 @@ if [ -n "$DEPS" ]; then
+if [ -n "$OVERRIDES" ]; then
+ GLOBAL_PKG="${HOME}/.bun/install/global/package.json"
+ if [ -f "$GLOBAL_PKG" ]; then
+ jq --argjson overrides "$OVERRIDES" '.overrides = $overrides' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" &&
+ mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG"
+ (cd "${HOME}/.bun/install/global" && bun install 2>/dev/null || true)
</file context>
| jq --argjson overrides "$OVERRIDES" '.overrides = $overrides' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| jq --argjson overrides "$OVERRIDES" '.overrides = ((.overrides // {}) + $overrides)' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && |
![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 implements dependency overrides for global Bun installations and updates the paperclipai service to use the Bun-managed binary path. The review feedback recommends ensuring the shell script handles file manipulation failures gracefully to maintain consistency and suggests quoting the service execution path as a best practice for systemd units.
| jq --argjson overrides "$OVERRIDES" '.overrides = $overrides' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | ||
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" |
There was a problem hiding this comment.
The jq and mv commands should handle failures gracefully to remain consistent with the error handling pattern used elsewhere in this script. This ensures that a failure in patching the global package.json (due to permissions or an invalid global manifest) does not halt the entire home-manager activation process.
| jq --argjson overrides "$OVERRIDES" '.overrides = $overrides' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | |
| jq --argjson overrides "$OVERRIDES" '.overrides = $overrides' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" || true |
References
- Maintain consistency with established patterns for writing scripts that are extracted from Nix expressions.
| Service = { | ||
| Type = "simple"; | ||
| ExecStart = "${homeDir}/dotfiles/node_modules/.bin/paperclipai run --no-repair"; | ||
| ExecStart = "${homeDir}/.bun/bin/paperclipai run --no-repair"; |
There was a problem hiding this comment.
Wrapping the ExecStart path in double quotes is a best practice for systemd units to ensure the command is correctly parsed if the path contains spaces, even if unlikely in this specific environment.
ExecStart = "\"${homeDir}/.bun/bin/paperclipai\" run --no-repair";
There was a problem hiding this comment.
Pull request overview
Updates Paperclip’s systemd user service to run the globally-installed Bun paperclipai binary and enhances the npm-globals installer to propagate dependency overrides into Bun’s global install to avoid pino-http/pino incompatibilities.
Changes:
- Switch Paperclip systemd user service
ExecStartto~/.bun/bin/paperclipai. - Extend
install-npm-globals.shto copyoverridesfrom~/dotfiles/package.jsoninto Bun’s globalpackage.jsonand re-runbun installthere.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| home-manager/modules/paperclip/default.nix | Points the Paperclip service at the Bun global paperclipai binary. |
| home-manager/modules/npm-globals/install-npm-globals.sh | Applies dotfiles overrides to Bun global install and re-installs to enforce versions (e.g., pino). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| jq --argjson overrides "$OVERRIDES" '.overrides = $overrides' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | ||
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" |
There was a problem hiding this comment.
The PR description says the dotfiles overrides are merged into the global package.json, but this jq filter replaces the entire .overrides object. If the global file already has overrides (or Bun adds some in the future), they will be lost. Consider merging instead of overwriting (e.g., combine existing .overrides with the new overrides).
| jq --argjson overrides "$OVERRIDES" '.overrides = $overrides' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | ||
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | ||
| (cd "${HOME}/.bun/install/global" && bun install 2>/dev/null || true) | ||
| echo "Applied dependency overrides to global install" |
There was a problem hiding this comment.
This block runs under set -e, but the jq ... > tmp && mv ... pipeline isn't protected by || true / error handling. If jq fails (e.g., corrupted global package.json), the whole activation will abort, unlike the earlier best-effort bun install --global ... || true. Consider making the override-application step best-effort as well (or emit an explicit error and continue).
| jq --argjson overrides "$OVERRIDES" '.overrides = $overrides' "$GLOBAL_PKG" >"${GLOBAL_PKG}.tmp" && | |
| mv "${GLOBAL_PKG}.tmp" "$GLOBAL_PKG" | |
| (cd "${HOME}/.bun/install/global" && bun install 2>/dev/null || true) | |
| echo "Applied dependency overrides to global install" | |
| GLOBAL_PKG_TMP="${GLOBAL_PKG}.tmp" | |
| if jq --argjson overrides "$OVERRIDES" '.overrides = $overrides' "$GLOBAL_PKG" >"$GLOBAL_PKG_TMP" && | |
| mv "$GLOBAL_PKG_TMP" "$GLOBAL_PKG"; then | |
| (cd "${HOME}/.bun/install/global" && bun install 2>/dev/null || true) | |
| echo "Applied dependency overrides to global install" | |
| else | |
| rm -f "$GLOBAL_PKG_TMP" | |
| echo "Warning: failed to apply dependency overrides to global install, continuing" | |
| fi |
* feat: add paperclip service (#1342) * feat: add paperclip service - Systemd service on kyber: runs `paperclipai run` via bun, depends on docker-postgres - Config via builtins.toJSON: external postgres on kyber, embedded on macOS - Setup script creates paperclip database on docker-postgres - Makefile target: `make systemctl-paperclip` * fix: add shellcheck disable and shell test coverage for paperclip * fix: correct shellcheck disable directive syntax * refactor: use config.template.json pattern for paperclip * refactor: rename setup.sh to hydrate.sh for paperclip * test: add auto-switch hook tests and update coverage spec * fix: remove EnvironmentFile from paperclip service (#1344) * fix: paperclip authenticated mode and required config fields (#1345) * fix: remove EnvironmentFile from paperclip service * fix: use authenticated mode on kyber, add required meta/logging fields * fix: add allowedHostnames for paperclip.shunkakinoki.com * fix: format config template json * fix: use trust auth for docker-postgres (#1346) POSTGRES_HOST_AUTH_METHOD=trust removes password auth for all connections. The password kept going out of sync after crash recovery, causing paperclip and other services to fail with 'password authentication failed'. Safe since postgres is only reachable from the host. * fix: use k8s postgres via DATABASE_URL, remove docker-postgres dep, add authenticated mode (#1347) * fix: use bun runtime for paperclip (pino-http node crash), k8s postgres via DATABASE_URL (#1348) * fix: use nix-profile bun path for paperclip service (#1350) * fix: add caret prefix to paperclipai dependency (#1349) * fix: use extra-substituters to avoid untrusted user warnings (#1351) * fix: run paperclip from cloned repo via pnpm dev:once (#1352) * fix: run paperclip from cloned repo via pnpm dev:once The global bun install flattens pino@10 + pino-http@10.5 together, but pino-http needs pino@9. The repo lockfile resolves this correctly with nested dependencies. Running from the repo avoids the crash. * fix: use bun run server/src/index.ts instead of pnpm dev:once * fix: pin pino@9.14.0 override, run paperclipai from dotfiles node_modules The bun flat hoisting was resolving pino@10 which is incompatible with pino-http@10.5. Pinning pino to 9.14.0 via overrides matches the paperclip repo's lockfile resolution and fixes the crash. * fix: use global bun paperclipai with pino override (#1353) * fix: use global ~/.bun/bin/paperclipai with pino override Propagate overrides from dotfiles package.json to ~/.bun/install/global/ so the global binary resolves pino@9.14.0 correctly. * test: add tests for npm-globals dependency overrides * fix: resolve GitHub Actions failures and code review issues - Fix non-portable \s regex to [[:space:]] in auto-switch.sh (shfmt compat) - Add jq dependency check alongside cswap - Use printf instead of echo for safer output - Fix claude-swap version from >=1.1.5 (non-existent) to >=0.7.1 - Add auto-switch.sh to Nix deployment config (default.nix) - Sort covered_scripts list alphabetically in coverage_spec.sh https://claude.ai/code/session_012GyQBesQGF1asTfKebWyLM --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
~/.bun/bin/paperclipai(global install) for the systemd serviceoverridesfrom~/dotfiles/package.jsonto~/.bun/install/global/package.jsonduring npm-globals installpino@9.14.0in the global install, fixing the pino-http crashHow it works
install-npm-globals.shrunsbun install --globalas beforeoverridesfrom dotfilespackage.json~/.bun/install/global/package.jsonbun installin the global dir to apply the overrideTested
10/10 requests returned 200 with
~/.bun/bin/paperclipaiafter applying the override.Summary by cubic
Switch the Paperclip service to the global
~/.bun/bin/paperclipaiand forcepino@9.14.0via overrides to stop thepino-httpcrash.overridesfrom~/dotfiles/package.jsonto~/.bun/install/global/package.json, runbun installto apply them, and add tests to confirm the overrides are applied in the global dir.ExecStartto use~/.bun/bin/paperclipai.Written for commit cd91d04. Summary will update on new commits.