fix(scripts): find Bun when Herdr invokes a plugin action - #52
Merged
Conversation
`herdr plugin action invoke update` fails with "bun not found on PATH". Herdr spawns plugin actions with a minimal environment: no login shell, so nothing has sourced the line Bun's installer adds to your profile, and ~/.bun/bin is not on PATH. collie-ctl.sh resolved Bun with `command -v bun` alone, so every action needing it died. What makes this easy to miss is how it fails rather than that it fails. The systemd unit is written with an ABSOLUTE ExecStart, so the running bridge is unaffected — only plugin-invoked actions break. And `update` pulls BEFORE it builds, so a failed run still fast-forwards the checkout: you end up with new code on disk, the OLD web/dist still being served, and both the manifest and /api/config reporting the new version. A half-succeeded deploy is indistinguishable from a working one unless you read the plugin log. It failed this way four times on my deployment before I noticed. Bun is now resolved from PATH first, then $BUN_INSTALL, ~/.bun/bin, ~/.local/bin, /usr/local/bin, /opt/homebrew/bin and /usr/bin, and its directory is prepended to PATH rather than only remembered in $BUN — this script shells out to a bare `bun` for the Tailscale ownership probe, and `bun run build` spawns children that expect to find it themselves. An empty result stays non-fatal, so the existing "bun not found" messages still report and exit. Verified in a reproduction of the failing environment (`env -i` with PATH=/usr/bin:/bin), and end to end on a real deployment: the same action that had been failing now completes, because `update` re-execs the just-pulled script and so repairs itself on the first run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… tests Review fixes on top of the Bun resolution fix. `command -v` reports a shell function or alias as a bare word, and the plugin .env is sourced before we resolve — so a `bun()` left in there resolved to `bun`, whose dirname is `.`. Prepending that handed every later `git` / `systemctl` / `tailscale` a cwd-relative lookup. Restrict the prepend to absolute paths; an empty or bare result now falls through to the existing guards unchanged. The control script's resolution had no coverage at all — the lifecycle suite stubs `BUN=/bin/true` everywhere — which is why a missing Bun could fail four deploys unnoticed. Three tests: resolution reaching into $HOME when PATH holds no bun (and $BUN_INSTALL outranking it), the directory reaching children on PATH, the CWD never doing so, and the empty case still reporting and exiting non-zero. They fail against the unpatched script with the original `error: bun not found on PATH`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
|
Shipped in 0.20.2. Added the absolute-path guard on the PATH prepend ( |
konpyl
added a commit
to konpyl/collie
that referenced
this pull request
Jul 30, 2026
Cherry-pick of upstream 4841e37, which the maintainer wrote reviewing the Bun resolution fix this fork contributed (merged as PR AltanS#52). It is a real bug in code this fork has been running since 0.23.1 and deployed again at 0.27.0. `command -v` reports a shell function or alias as a BARE WORD, and the plugin's .env is sourced before we resolve — so a `bun()` left in there resolves to `bun`, whose dirname is `.`. My version prepended that unconditionally, handing every later `git` / `systemctl` / `tailscale` in the script a cwd-relative lookup. Reproduced before applying the fix: bun() { :; } command -v bun -> 'bun' dirname -> '.' old: PATH now starts with . new: PATH now starts with /home/kon/.bun/bin The prepend is now restricted to absolute paths; an empty or bare result falls through to the existing "bun not found" guards unchanged. It also brings the three tests the resolution never had — the lifecycle suite stubs BUN=/bin/true everywhere, which is exactly why a missing Bun could fail four deploys unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
darieldatoon
added a commit
to darieldatoon/collie
that referenced
this pull request
Jul 30, 2026
Closes AltanS#55. ARCHITECTURE.md §3 already describes the macOS deployment as a "launchd agent … starts at login, restarts on failure, survives Herdr restarts". No agent existed: `launchd` appeared exactly once in the repo, in that sentence. On macOS `have_systemd()` is false, so `cmd_start` fell through to the unsupervised `nohup` + pidfile branch — nothing restarted a crashed bridge and nothing brought it back after a reboot, on a host the README lists as supported. From a phone that is indistinguishable from a sleeping host, because the `tailscale serve` mapping keeps resolving. `cmd_start`, `cmd_stop`, `cmd_uninstall` and `print_status_banner` now branch systemd → launchd → nohup. `write_agent()` mirrors `write_unit()` key for key: RunAtLoad for WantedBy=default.target, KeepAlive/SuccessfulExit=false for Restart=on-failure, ThrottleInterval for RestartSec. StartLimitIntervalSec has no analogue — launchd has no start limit to lift. NoNewPrivileges and PrivateTmp are not representable, so the macOS agent is less confined than the Linux unit; that is called out in the comment rather than left to be discovered. The plist carries paths only, never config values. launchd has no EnvironmentFile=, so the obvious port bakes the sourced .env into EnvironmentVariables — but .env is mode 600 and may hold COLLIE_VAPID_PRIVATE, while a LaunchAgent plist has to stay readable. Baking would copy a Web Push signing key into a readable file, and would freeze config the README documents as re-read on restart. Instead the agent re-enters the script as the internal `_exec-bridge` step, which sources .env itself and `exec`s Bun so launchd supervises the bridge pid directly rather than a wrapper. HERDR_PLUGIN_CONFIG_DIR is passed in the plist so resolve_config_dir() short-circuits: at login the Herdr server may not be up, and shelling out to `herdr plugin config-dir` would hang or resolve the fallback dir. `start` boots the label out before bootstrapping, so it is idempotent — bootstrap on a loaded label is an error, and quietly starting a second bridge is the failure this branch removes. `stop` boots the job out of the domain, so KeepAlive cannot resurrect what an operator just stopped. Interpolated paths go through xml_escape: a checkout path containing `&` would otherwise emit a plist launchd refuses to parse. Tests: the suite gained a fake `launchctl` on the scratch PATH for every case, because without it a macOS run bootstrapped a real job into the developer's own gui/<uid> domain, pointed at a temp dir the suite then deleted — it crash-looped after the tests reported success. The lifecycle test stubs have_launchd rather than trusting `uname`, since CI is ubuntu-latest and a test that silently skips its own branch is worse than no test. Two assertions carry the weight: a .env seeded with a fake COLLIE_VAPID_PRIVATE must leave no trace in the plist, and the plist must survive `plutil -lint` — one launchd cannot parse means the agent silently never starts, which no substring check would notice. The lint is guarded on `command -v plutil`, so it no-ops on the ubuntu runner and covers every macOS dev machine. The banner's launchd line is covered separately, because the lifecycle test stubs print_status_banner and therefore read none of it — a first cut printed the pid twice and every lifecycle assertion still passed. The serve-failure test now pins have_launchd off, as the nohup fallback is what it exists to cover. Verified on macOS 26.5.2 (arm64) under the system bash 3.2 the plist actually invokes, against a real herdr herd: start bootstraps and serves, a second start leaves exactly one bridge, SIGKILL is recovered in ~1s, `stop` boots the job out and it stays down, and the generated plist round-trips through `plutil` with a config dir containing `&` and `<`. Version deliberately left at 0.20.2 with no CHANGELOG entry, matching AltanS#52 — the release commit is the maintainer's to cut. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
darieldatoon
added a commit
to darieldatoon/collie
that referenced
this pull request
Jul 30, 2026
Closes AltanS#55. ARCHITECTURE.md §3 already describes the macOS deployment as a "launchd agent … starts at login, restarts on failure, survives Herdr restarts". No agent existed: `launchd` appeared exactly once in the repo, in that sentence. On macOS `have_systemd()` is false, so `cmd_start` fell through to the unsupervised `nohup` + pidfile branch — nothing restarted a crashed bridge and nothing brought it back after a reboot, on a host the README lists as supported. From a phone that is indistinguishable from a sleeping host, because the `tailscale serve` mapping keeps resolving. `cmd_start`, `cmd_stop`, `cmd_uninstall` and `print_status_banner` now branch systemd → launchd → nohup. `write_agent()` mirrors `write_unit()` key for key: RunAtLoad for WantedBy=default.target, KeepAlive/SuccessfulExit=false for Restart=on-failure, ThrottleInterval for RestartSec. StartLimitIntervalSec has no analogue — launchd has no start limit to lift. NoNewPrivileges and PrivateTmp are not representable, so the macOS agent is less confined than the Linux unit; that is called out in the comment rather than left to be discovered. The plist carries paths only, never config values. launchd has no EnvironmentFile=, so the obvious port bakes the sourced .env into EnvironmentVariables — but .env is mode 600 and may hold COLLIE_VAPID_PRIVATE, while a LaunchAgent plist has to stay readable. Baking would copy a Web Push signing key into a readable file, and would freeze config the README documents as re-read on restart. Instead the agent re-enters the script as the internal `_exec-bridge` step, which sources .env itself and `exec`s Bun so launchd supervises the bridge pid directly rather than a wrapper. HERDR_PLUGIN_CONFIG_DIR is passed in the plist so resolve_config_dir() short-circuits: at login the Herdr server may not be up, and shelling out to `herdr plugin config-dir` would hang or resolve the fallback dir. `start` boots the label out before bootstrapping, so it is idempotent — bootstrap on a loaded label is an error, and quietly starting a second bridge is the failure this branch removes. `stop` pairs `disable` with `bootout`: bootout alone stops only the current process, and because LaunchAgents load at login, RunAtLoad would then resurrect a bridge the operator had deliberately stopped — `disable` is what makes `stop` persist, matching `systemctl --user disable --now`. `start` re-`enable`s, so a stopped agent is not wedged. macOS installs predating this change are running the nohup fallback, whose bridge still owns the port when the updated script first bootstraps an agent, so `start` and `stop` release it first. Only a numeric, non-system pid that `ps` still shows running our own `bridge/index.ts` is signalled: the pidfile outlives its process (SIGKILL, a panic, a reboot) and the OS recycles pids, and this runs on the `start` path, so a stale record must not kill an unrelated process during a routine start. Interpolated paths go through xml_escape: a checkout path containing `&` would otherwise emit a plist launchd refuses to parse. Tests: the suite gained a fake `launchctl` on the scratch PATH for every case, because without it a macOS run bootstrapped a real job into the developer's own gui/<uid> domain, pointed at a temp dir the suite then deleted — it crash-looped after the tests reported success. The lifecycle test stubs have_launchd rather than trusting `uname`, since CI is ubuntu-latest and a test that silently skips its own branch is worse than no test. Its load-bearing assertions are negative: a .env seeded with a fake COLLIE_VAPID_PRIVATE must leave no trace in the plist; a pidfile naming a recycled pid must be discarded without being signalled while a pid still holding the bridge is; and the plist must survive `plutil -lint`, since one launchd cannot parse means the agent silently never starts and no substring check would notice. The lint is guarded on `command -v plutil`, so it no-ops on the ubuntu runner and covers every macOS dev machine. The banner's launchd line is covered separately, because the lifecycle test stubs print_status_banner and therefore reads none of it — a first cut printed the pid twice and every lifecycle assertion still passed. The serve-failure test now pins have_launchd off, as the nohup fallback is what it exists to cover. Verified on macOS 26.5.2 (arm64) under the system bash 3.2 the plist actually invokes, against a real herdr herd: start bootstraps and serves, a second start leaves exactly one bridge, SIGKILL is recovered in ~1s, `disable` + `bootout` genuinely blocks a re-bootstrap until `enable` (so `stop` survives a re-login), the ownership guard matches a live bridge's command line and not `launchd(1)`, and the generated plist round-trips through `plutil` with a config dir containing `&` and `<`. Version deliberately left at 0.20.2 with no CHANGELOG entry, matching AltanS#52 — the release commit is the maintainer's to cut. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
darieldatoon
added a commit
to darieldatoon/collie
that referenced
this pull request
Jul 30, 2026
Closes AltanS#55. ARCHITECTURE.md §3 already describes the macOS deployment as a "launchd agent … starts at login, restarts on failure, survives Herdr restarts". No agent existed: `launchd` appeared exactly once in the repo, in that sentence. On macOS `have_systemd()` is false, so `cmd_start` fell through to the unsupervised `nohup` + pidfile branch — nothing restarted a crashed bridge and nothing brought it back after a reboot, on a host the README lists as supported. From a phone that is indistinguishable from a sleeping host, because the `tailscale serve` mapping keeps resolving. `cmd_start`, `cmd_stop`, `cmd_uninstall` and `print_status_banner` now branch systemd → launchd → nohup. `write_agent()` mirrors `write_unit()` key for key: RunAtLoad for WantedBy=default.target, KeepAlive/SuccessfulExit=false for Restart=on-failure, ThrottleInterval for RestartSec. StartLimitIntervalSec has no analogue — launchd has no start limit to lift. NoNewPrivileges and PrivateTmp are not representable, so the macOS agent is less confined than the Linux unit; that is called out in the comment rather than left to be discovered. The plist carries paths only, never config values. launchd has no EnvironmentFile=, so the obvious port bakes the sourced .env into EnvironmentVariables — but .env is mode 600 and may hold COLLIE_VAPID_PRIVATE, while a LaunchAgent plist has to stay readable. Baking would copy a Web Push signing key into a readable file, and would freeze config the README documents as re-read on restart. Instead the agent re-enters the script as the internal `_exec-bridge` step, which sources .env itself and `exec`s Bun so launchd supervises the bridge pid directly rather than a wrapper. HERDR_PLUGIN_CONFIG_DIR is passed in the plist so resolve_config_dir() short-circuits: at login the Herdr server may not be up, and shelling out to `herdr plugin config-dir` would hang or resolve the fallback dir. `start` boots the label out before bootstrapping, so it is idempotent — bootstrap on a loaded label is an error, and quietly starting a second bridge is the failure this branch removes. `stop` pairs `disable` with `bootout`: bootout alone stops only the current process, and because LaunchAgents load at login, RunAtLoad would then resurrect a bridge the operator had deliberately stopped — `disable` is what makes `stop` persist, matching `systemctl --user disable --now`. `start` re-`enable`s, so a stopped agent is not wedged. That `disable` is a record in launchd's per-user database, and it outlives the plist, so `uninstall` clears it — otherwise the inverse of `start` would leave a label a later reinstall inherits as disabled. It removes the plist first, since an enabled label whose plist is still on disk is one login away from loading again. `enable` is the only supported inverse: it resets the state rather than deleting the record, so a benign "enabled" entry can remain (deleting that needs root). macOS installs predating this change are running the nohup fallback, whose bridge still owns the port when the updated script first bootstraps an agent, so `start` and `stop` release it first. Only a numeric, non-system pid that `ps` still shows running our own `bridge/index.ts` is signalled: the pidfile outlives its process (SIGKILL, a panic, a reboot) and the OS recycles pids, and this runs on the `start` path, so a stale record must not kill an unrelated process during a routine start. Interpolated paths go through xml_escape: a checkout path containing `&` would otherwise emit a plist launchd refuses to parse. Tests: the suite gained a fake `launchctl` on the scratch PATH for every case, because without it a macOS run bootstrapped a real job into the developer's own gui/<uid> domain, pointed at a temp dir the suite then deleted — it crash-looped after the tests reported success. The lifecycle test stubs have_launchd rather than trusting `uname`, since CI is ubuntu-latest and a test that silently skips its own branch is worse than no test. Its load-bearing assertions are negative: a .env seeded with a fake COLLIE_VAPID_PRIVATE must leave no trace in the plist; a pidfile naming a recycled pid must be discarded without being signalled while a pid still holding the bridge is; and the plist must survive `plutil -lint`, since one launchd cannot parse means the agent silently never starts and no substring check would notice. The lint is guarded on `command -v plutil`, so it no-ops on the ubuntu runner and covers every macOS dev machine. The uninstall assertions run against a truncated call log, because `start` already records an `enable` and asserting on the whole log would pass either way. The banner's launchd line is covered separately, because the lifecycle test stubs print_status_banner and therefore reads none of it — a first cut printed the pid twice and every lifecycle assertion still passed. The serve-failure test now pins have_launchd off, as the nohup fallback is what it exists to cover. Verified on macOS 26.5.2 (arm64) under the system bash 3.2 the plist actually invokes, against a real herdr herd: start bootstraps and serves, a second start leaves exactly one bridge, SIGKILL is recovered in ~1s, `disable` + `bootout` genuinely blocks a re-bootstrap until `enable` (so `stop` survives a re-login), a start → stop → uninstall cycle against real `launchctl` moves the override enabled → disabled → enabled with the plist gone and the job unloaded, the ownership guard matches a live bridge's command line and not `launchd(1)`, and the generated plist round-trips through `plutil` with a config dir containing `&` and `<`. Version deliberately left at 0.20.2 with no CHANGELOG entry, matching AltanS#52 — the release commit is the maintainer's to cut. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
darieldatoon
added a commit
to darieldatoon/collie
that referenced
this pull request
Jul 30, 2026
Closes AltanS#55. ARCHITECTURE.md §3 already describes the macOS deployment as a "launchd agent … starts at login, restarts on failure" — and `launchd` appeared exactly once in the repo, in that sentence. On macOS `have_systemd()` is false, so `cmd_start` fell through to the unsupervised nohup + pidfile branch: nothing restarted a crashed bridge and nothing brought it back after a reboot, on a host the README lists as supported. From a phone that is indistinguishable from a sleeping host, because the `tailscale serve` mapping keeps resolving. `cmd_start`, `cmd_stop`, `cmd_uninstall` and `print_status_banner` now branch systemd → launchd → nohup, with `write_agent()` mirroring `write_unit()` key for key — mapping in the comment there. No ProcessType: Background throttles CPU and I/O, which the unit never does to the bridge. Two decisions worth stating here. The plist holds paths only: launchd has no `EnvironmentFile=`, and .env is mode 600 and may hold COLLIE_VAPID_PRIVATE, so baking config into a readable plist would copy a Web Push signing key out of a protected file — the agent re-enters this script as `_exec-bridge` and sources .env itself. And `stop` pairs `disable` with `bootout`, because bootout alone leaves RunAtLoad to resurrect a bridge the operator stopped; `start` re-enables, and `uninstall` clears that override so it stays the true inverse of `start`. Installs predating this still own the port through the nohup fallback, so `start` and `stop` release it — signalling only a pid `ps` still shows running our own bridge, since the pidfile outlives its process and pids get recycled. Tests: a fake `launchctl` on the scratch PATH for every case. Without it a macOS run bootstrapped a real job into the developer's own gui/<uid> domain, aimed at a temp dir the suite then deleted, and it crash-looped after the tests reported success. `have_launchd` is stubbed rather than trusted, since CI is ubuntu-latest. The assertions that matter are negative: a seeded COLLIE_VAPID_PRIVATE leaves no trace in the plist, a recycled pid is discarded rather than signalled, and the plist survives `plutil -lint` (guarded on the binary, so it no-ops on CI). The banner is covered separately — a first cut printed the pid twice and every other assertion still passed. Verified on macOS 26.5.2 under the system bash 3.2 the plist invokes, against a real herd: start serves and is idempotent, SIGKILL recovers in ~1s, disable + bootout blocks a re-bootstrap until enable, and start → stop → uninstall moves the override enabled → disabled → enabled with the plist gone. Not covered: an actual logout/login cycle. Version left at 0.20.2 with no CHANGELOG entry, matching AltanS#52 — the release commit is yours to cut. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
herdr plugin action invoke updatefails witherror: bun not found on PATH.Herdr spawns plugin actions with a minimal environment — no login shell, so nothing has sourced the line Bun's installer adds to your profile and
~/.bun/binisn't onPATH.collie-ctl.sh:47resolves Bun withcommand -v bunalone, so every action that needs it dies.Why this is worth fixing rather than documenting
It's not that it fails, it's how:
The systemd unit is written with an absolute
ExecStart, so the running bridge is unaffected. Only plugin-invoked actions break —update,restart,start.updatepulls before it builds:So a failed run still fast-forwards the checkout. You end up with new code on disk, the old
web/diststill being served, and both the manifest and/api/configreporting the new version. A half-succeeded deploy is indistinguishable from a working one unless you go read the plugin log.On my deployment it failed this way four times before I noticed — I only caught it because I checked the built bundle rather than the version string.
The change
Resolve Bun from
PATHfirst, then$BUN_INSTALL,~/.bun/bin,~/.local/bin,/usr/local/bin,/opt/homebrew/bin,/usr/bin.Its directory is prepended to
PATH, not just stored in$BUN: the script shells out to a barebunfor the Tailscale ownership probe (collie-ctl.sh:94), andbun run buildspawns children that expect to find it themselves.An empty result stays non-fatal — the existing
[ -n "$BUN" ] || { echo "error: bun not found on PATH"; exit 1; }guards still report and exit, so a host genuinely without Bun gets the same message as before.Verification
env -i HOME=$HOME PATH=/usr/bin:/bin: resolves and puts Bun onPATH.bash -nclean;bun run test(474 tests incl. thecollie-ctllifecycle suite) passes on top ofmain.cmd_updatere-execs the just-pulled script.One commit, one file, no version bump or CHANGELOG entry — this comes from a fork with its own numbering lane, so slotting it into your release is yours to do.
🤖 Generated with Claude Code