Loop daemon: bake resolved node/claude PATH into pr-loop.service unit - #117
Merged
Conversation
…107) The installed pr-loop.service (the loop daemon itself) got no baked PATH at all and ran under systemd's minimal PATH — gh but neither node nor claude — silently stalling node-dependent tick steps (loop-census.sh, merge-ready.sh, write_tick_record) until the daemon's own runtime ensure_claude_on_path nvm fallback kicked in. arm-loop.sh now resolves node's dir (erroring loudly if unresolvable, matching the existing claude check) and bakes a deduped PATH (node dir, claude dir, standard system dirs) into the unit via a new __PATH__ placeholder in both pr-loop.service template copies. The nvm-sourcing fallback in loop-daemon.sh stays as a safety net for installs that predate this change or use fnm/volta/system node. Also: when startup env resolution fails, main() now appends an env-error ledger line (pure bash, no node needed) alongside the existing warning log, so a degraded start is visible in the run ledger. Adds a loop-daemon.test.sh regression scenario (6) that strips node/claude from the daemon's PATH but exposes a fake nvm install via NVM_DIR, asserting the startup ensure_claude_on_path call resolves them onto PATH before run_once spawns the first tick's child process. Item 3 (empty cadence under the degraded env) traced to the same root cause as this fix — loop-census.sh needs node to emit its cadence= line — no separate threading bug found, so no code change was needed there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
robercano
approved these changes
Jul 15, 2026
… CI (#107) CI failed where local passed: run_daemon_once strips claude off PATH but left NVM_DIR uncontrolled, so on a dev box ensure_claude_on_path resolved the HOST's ~/.nvm (no env-error ledger line) while CI runners — with no nvm at all — hit main()'s new startup env-error path, creating the ledger file that scenarios 1/2 asserted must not exist. Pin NVM_DIR inside the fixture so the host's nvm can never leak in, and assert the real invariant: the startup 'verdict=startup result=env-error' line is the ONLY ledger line and no driver (verdict=advance) line is ever written. This also gives the #107 env-error ledger feature its own deterministic coverage (21 -> 23 checks). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
robercano
approved these changes
Jul 15, 2026
robercano
added a commit
that referenced
this pull request
Jul 15, 2026
… autostart tiers in setup (#119) Field evidence from 2026-07-14/15 (5 of 6 drivers killed mid-flight by daemon cgroup kills — service restarts, overnight VM downtime, wsl shutdowns) showed three doc gaps: - USAGE daemon section oversold setsid: a process group is not a cgroup, so any daemon stop kills in-flight drivers with no ledger line. Document the caveat, the check-before-restart procedure, and #119 as the fix. - Failure contract: the in_flight wedge fix is now classify-then-recover (empty -> delete, publishable -> push + open PR as proven by #107 -> PR #117, dirty -> wip-commit and never delete), plus a new row for the daemon-killed-mid-driver fingerprint. New WSL2 caveat: autostart protects the queue, not a driver in flight. - Setup skill step 4 now offers the verified unattended autostart tier (AtStartup + S4U + vmIdleTimeout=-1, per USAGE) above the logon-only schtasks variant, and states the mid-driver caveat either way. Co-Authored-By: Claude Fable 5 <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.
Closes #107
What
The installed
pr-loop.service(the loop daemon itself) got no baked PATH and ran under systemd's minimal PATH —ghbut neithernodenorclaude— silently stalling node-dependent tick steps (loop-census.sh,merge-ready.sh,write_tick_record) until the daemon's runtime nvm fallback kicked in.__PATH__placeholder in bothpr-loop.servicetemplate copies.loop-daemon.shstays as a safety net for installs that predate this change or use fnm/volta/system node.main()now appends an env-error ledger line (pure bash, no node needed), so a degraded start is visible in the run ledger.Item 3 of the issue (empty cadence under the degraded env) traced to this same root cause —
loop-census.shneeds node to emit itscadence=line — no separate bug found, no extra code change needed.Tests
loop-daemon.test.shgains regression scenario 6: strips node/claude from the daemon's PATH but exposes a fake nvm install viaNVM_DIR, asserting the startupensure_claude_on_pathcall resolves them onto PATH beforerun_oncespawns the first tick's child. Suite green: PASS (21 checks), re-verified 2026-07-15.Review
Correctness (opus) + tests (sonnet) lenses both approved on 2026-07-14; the driver was killed by a mid-flight service restart before it could push/open this PR (see #111). Branch recovered and published manually.
🤖 Generated with Claude Code