From f9dcfcbbefd80b086e46405c5fe31ff2d30b5d0f Mon Sep 17 00:00:00 2001 From: Roberto Cano <3525807+robercano@users.noreply.github.com> Date: Tue, 14 Jul 2026 09:55:50 +0200 Subject: [PATCH 1/2] fix(loop): bake resolved node/claude PATH into pr-loop.service unit (#107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .claude/scripts/arm-loop.sh | 21 +++++++++ .claude/scripts/loop-daemon.sh | 8 +++- .claude/scripts/loop-daemon.test.sh | 47 +++++++++++++++++++ .../skills/setup/templates/pr-loop.service | 3 +- .claude/systemd/pr-loop.service | 3 +- 5 files changed, 78 insertions(+), 4 deletions(-) diff --git a/.claude/scripts/arm-loop.sh b/.claude/scripts/arm-loop.sh index 3c3e9d7..72a0ca4 100755 --- a/.claude/scripts/arm-loop.sh +++ b/.claude/scripts/arm-loop.sh @@ -111,6 +111,26 @@ fi rc_name="${rc_name:-$repo_slug-planner}" claude_dir="$(dirname "$claude_bin")" +# Same rationale as claude_bin above, plus issue #107: the installed +# pr-loop.service unit (the loop daemon itself, NOT claude-rc) previously got +# NO baked PATH at all and ran under systemd's minimal PATH — which has `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 fallback (loop-daemon.sh) kicked in. Bake the +# resolved node/claude dirs in here too so the unit starts with a working PATH +# from the first tick, with the nvm-sourcing fallback staying as a safety net +# for installs that predate this change or use fnm/volta/system node. +node_bin="$(command -v node || true)" +if [ -z "$node_bin" ]; then + echo "arm-loop.sh: 'node' not found on PATH — run this from a real terminal where \`node\` works." >&2 + exit 1 +fi +node_dir="$(dirname "$node_bin")" + +# Compose the baked PATH: node_dir, claude_dir, then the standard system dirs +# — deduped, since under nvm node_dir and claude_dir are frequently identical. +baked_path="$(printf '%s\n' "$node_dir" "$claude_dir" "/usr/local/sbin" "/usr/local/bin" "/usr/sbin" "/usr/bin" "/sbin" "/bin" | awk '!seen[$0]++' | paste -sd: -)" + units_dir="$HOME/.config/systemd/user" mkdir -p "$units_dir" @@ -129,6 +149,7 @@ claude_rc_dst="$units_dir/claude-rc-$repo_slug.service" sed -e "s#__WORKDIR__#$repo_root#g" \ -e "s#__REPO_SLUG__#$repo_slug#g" \ -e "s#__GATES_ENV__#$gates_env#g" \ + -e "s#__PATH__#$baked_path#g" \ "$pr_loop_src" > "$pr_loop_dst" sed -e "s#__WORKDIR__#$repo_root#g" \ diff --git a/.claude/scripts/loop-daemon.sh b/.claude/scripts/loop-daemon.sh index 4823f27..8c79187 100644 --- a/.claude/scripts/loop-daemon.sh +++ b/.claude/scripts/loop-daemon.sh @@ -206,8 +206,12 @@ main() { # under the service silently skip merges and tick records while polling # still works — a deadlock, since the only path that DID source nvm # (run_driver) is unreachable while an unmergeable PR keeps advance away. - ensure_claude_on_path \ - || log "warning: 'claude' not resolvable at startup (nor via nvm) — node-dependent tick steps and driver spawns will fail until PATH provides it" + if ! ensure_claude_on_path; then + log "warning: 'claude' not resolvable at startup (nor via nvm) — node-dependent tick steps and driver spawns will fail until PATH provides it" + # append_ledger is pure bash (no node needed) — record the env-error even + # though write_tick_record itself can't run without node (issue #107). + append_ledger "unknown" "" "startup" "$(date -u +%FT%TZ)" "result=env-error" + fi log "starting (LOOP_MODEL=${LOOP_MODEL:-sonnet} GATES_FILE=${GATES_FILE:-} LOOP_DRIVER_TIMEOUT=${LOOP_DRIVER_TIMEOUT:-90m})" local iterations=0 local max_iterations="${LOOP_DAEMON_MAX_ITERATIONS:-0}" diff --git a/.claude/scripts/loop-daemon.test.sh b/.claude/scripts/loop-daemon.test.sh index 0e2869e..47a5da8 100644 --- a/.claude/scripts/loop-daemon.test.sh +++ b/.claude/scripts/loop-daemon.test.sh @@ -102,6 +102,15 @@ run_daemon_once() { ( cd "$1" && PATH="$1/bin:/usr/bin:/bin" LOOP_DAEMON_MAX_ITERATIONS=1 LOOP_DAEMON_SLEEP_FAST=0 LOOP_DAEMON_SLEEP_WATCH=0 LOOP_DAEMON_SLEEP_IDLE=0 LOOP_DAEMON_SLEEP_FALLBACK=0 bash .claude/scripts/loop-daemon.sh ) } +run_daemon_once_stripped_path() { + # $1=fixture root $2=NVM_DIR to expose; like run_daemon_once but with a PATH + # that has NEITHER node NOR claude (mirroring pr-loop.service's minimal + # systemd PATH before issue #107's baked-PATH fix), to exercise main()'s + # startup ensure_claude_on_path nvm fallback instead of the fixture's own + # bin/ dir. + ( cd "$1" && PATH="/usr/bin:/bin" NVM_DIR="$2" LOOP_DAEMON_MAX_ITERATIONS=1 LOOP_DAEMON_SLEEP_FAST=0 LOOP_DAEMON_SLEEP_WATCH=0 LOOP_DAEMON_SLEEP_IDLE=0 LOOP_DAEMON_SLEEP_FALLBACK=0 bash .claude/scripts/loop-daemon.sh ) +} + # --------------------------------------------------------------------------- # 1. action=none: fake loop-event.sh reports nothing actionable. Assert the # ledger file is never created/written and no marker any stub would leave @@ -224,6 +233,44 @@ check "scenario 5 (timeout): ledger records result=timeout rc=124" bash -c ' grep -Eq "^pid=[0-9]+ session=unknown verdict=advance issue=3 ts=[0-9T:Z-]+ result=timeout rc=124$" "$1" ' _ "$ledger5" +# --------------------------------------------------------------------------- +# 6. Startup PATH resolution regression guard (issue #107): the daemon's own +# PATH lacks BOTH node and claude (mirroring pr-loop.service's minimal +# systemd PATH before this issue's baked-PATH fix), but a FAKE nvm install +# is reachable via NVM_DIR. Assert main()'s startup ensure_claude_on_path +# call (the 4bf7dbb hotfix) resolves node/claude onto PATH BEFORE run_once +# spawns loop-event.sh, so a node-dependent tick step (stood in here by the +# fake loop-event.sh itself checking `command -v node`/`command -v claude`) +# sees them already resolved in the child. +# --------------------------------------------------------------------------- +dir6="$work/scenario6" +fake_nvm_dir="$work/scenario6-nvm" +mkdir -p "$fake_nvm_dir/bin" +cat > "$fake_nvm_dir/nvm.sh" < "$fake_nvm_dir/bin/node" +chmod +x "$fake_nvm_dir/bin/node" +printf '#!/usr/bin/env bash\nexit 0\n' > "$fake_nvm_dir/bin/claude" +chmod +x "$fake_nvm_dir/bin/claude" + +new_fixture scenario6 "#!/usr/bin/env bash +if command -v node >/dev/null 2>&1 && command -v claude >/dev/null 2>&1; then + : > '$dir6/node-resolved.marker' +else + : > '$dir6/node-missing.marker' +fi +echo 'cadence=IDLE cron=*/15 * * * *' +echo 'loop-event: action=none' +exit 0" >/dev/null +run_daemon_once_stripped_path "$dir6" "$fake_nvm_dir" >/dev/null 2>&1 +check "scenario 6 (startup PATH resolution): node+claude resolved in child before run_once" [ -f "$dir6/node-resolved.marker" ] +check "scenario 6: no node-missing marker was left (node/claude never resolved)" [ ! -f "$dir6/node-missing.marker" ] + echo "" if [ "$fail" -eq 0 ]; then echo "loop-daemon.test.sh: PASS ($ok checks)" diff --git a/.claude/skills/setup/templates/pr-loop.service b/.claude/skills/setup/templates/pr-loop.service index 5a994c2..05d704e 100644 --- a/.claude/skills/setup/templates/pr-loop.service +++ b/.claude/skills/setup/templates/pr-loop.service @@ -3,7 +3,7 @@ # ~/.config/systemd/user/pr-loop-.service by # `.claude/scripts/arm-loop.sh`, which substitutes the __PLACEHOLDER__ tokens # below for this checkout (WorkingDirectory, repo slug, optional GATES_FILE -# env line) and runs `systemctl --user enable --now`. +# env line, baked runtime PATH) and runs `systemctl --user enable --now`. # # DO NOT hand-edit the INSTALLED copy under ~/.config/systemd/user/ — it will # be silently overwritten the next time arm-loop.sh runs. Edit THIS checked-in @@ -19,6 +19,7 @@ Wants=network-online.target Type=simple WorkingDirectory=__WORKDIR__ __GATES_ENV__ +Environment=PATH=__PATH__ ExecStart=/usr/bin/env bash __WORKDIR__/.claude/scripts/loop-daemon.sh Restart=always RestartSec=15 diff --git a/.claude/systemd/pr-loop.service b/.claude/systemd/pr-loop.service index 5a994c2..05d704e 100644 --- a/.claude/systemd/pr-loop.service +++ b/.claude/systemd/pr-loop.service @@ -3,7 +3,7 @@ # ~/.config/systemd/user/pr-loop-.service by # `.claude/scripts/arm-loop.sh`, which substitutes the __PLACEHOLDER__ tokens # below for this checkout (WorkingDirectory, repo slug, optional GATES_FILE -# env line) and runs `systemctl --user enable --now`. +# env line, baked runtime PATH) and runs `systemctl --user enable --now`. # # DO NOT hand-edit the INSTALLED copy under ~/.config/systemd/user/ — it will # be silently overwritten the next time arm-loop.sh runs. Edit THIS checked-in @@ -19,6 +19,7 @@ Wants=network-online.target Type=simple WorkingDirectory=__WORKDIR__ __GATES_ENV__ +Environment=PATH=__PATH__ ExecStart=/usr/bin/env bash __WORKDIR__/.claude/scripts/loop-daemon.sh Restart=always RestartSec=15 From 978341c1a04319c0aa57a44fcf05f0f1cebf6297 Mon Sep 17 00:00:00 2001 From: Roberto Cano <3525807+robercano@users.noreply.github.com> Date: Wed, 15 Jul 2026 10:16:35 +0200 Subject: [PATCH 2/2] test(loop): make daemon scenarios 1/2 deterministic under claude-less CI (#107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .claude/scripts/loop-daemon.test.sh | 32 +++++++++++++++++++---------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/.claude/scripts/loop-daemon.test.sh b/.claude/scripts/loop-daemon.test.sh index 47a5da8..2f510ae 100644 --- a/.claude/scripts/loop-daemon.test.sh +++ b/.claude/scripts/loop-daemon.test.sh @@ -98,8 +98,11 @@ fake_bin() { } run_daemon_once() { - # $1=fixture root; runs loop-daemon.sh for exactly one iteration. - ( cd "$1" && PATH="$1/bin:/usr/bin:/bin" LOOP_DAEMON_MAX_ITERATIONS=1 LOOP_DAEMON_SLEEP_FAST=0 LOOP_DAEMON_SLEEP_WATCH=0 LOOP_DAEMON_SLEEP_IDLE=0 LOOP_DAEMON_SLEEP_FALLBACK=0 bash .claude/scripts/loop-daemon.sh ) + # $1=fixture root; runs loop-daemon.sh for exactly one iteration. NVM_DIR + # points inside the fixture (nothing there) so ensure_claude_on_path's nvm + # fallback can never resolve the HOST's ~/.nvm — otherwise scenarios without + # a claude stub pass on a dev box with nvm but fail on CI runners without it. + ( cd "$1" && PATH="$1/bin:/usr/bin:/bin" NVM_DIR="$1/no-such-nvm" LOOP_DAEMON_MAX_ITERATIONS=1 LOOP_DAEMON_SLEEP_FAST=0 LOOP_DAEMON_SLEEP_WATCH=0 LOOP_DAEMON_SLEEP_IDLE=0 LOOP_DAEMON_SLEEP_FALLBACK=0 bash .claude/scripts/loop-daemon.sh ) } run_daemon_once_stripped_path() { @@ -112,30 +115,37 @@ run_daemon_once_stripped_path() { } # --------------------------------------------------------------------------- -# 1. action=none: fake loop-event.sh reports nothing actionable. Assert the -# ledger file is never created/written and no marker any stub would leave -# behind exists — i.e. ZERO drivers spawned. Note there is deliberately NO -# 'claude' stub installed for this scenario either: if loop-daemon.sh ever -# tried to spawn one on action=none, the whole run would blow up with -# "command not found" instead of quietly passing. +# 1. action=none: fake loop-event.sh reports nothing actionable. Assert no +# DRIVER ledger line is written — i.e. ZERO drivers spawned. Note there is +# deliberately NO 'claude' stub installed for this scenario: if +# loop-daemon.sh ever tried to spawn one on action=none, the whole run +# would blow up with "command not found" instead of quietly passing. With +# claude unresolvable, main()'s startup check (issue #107) writes exactly +# one 'verdict=startup result=env-error' line — the ONLY line allowed here. # --------------------------------------------------------------------------- dir1="$(new_fixture scenario1 '#!/usr/bin/env bash echo "cadence=IDLE cron=*/15 * * * *" echo "loop-event: action=none" exit 0')" run_daemon_once "$dir1" >/dev/null 2>&1 -check "scenario 1 (action=none): no ledger file was created" [ ! -f "$dir1/.claude/state/loop-runs.log" ] +ledger1="$dir1/.claude/state/loop-runs.log" +check "scenario 1 (action=none): startup env-error is the ONLY ledger line" bash -c ' + [ "$(wc -l < "$1" 2>/dev/null || echo 0)" -eq 1 ] && grep -q "verdict=startup .*result=env-error" "$1"' _ "$ledger1" +check "scenario 1: no driver ledger line was written" bash -c '! grep -q "verdict=advance" "$1"' _ "$ledger1" # --------------------------------------------------------------------------- # 2. Broken tick (loop-event.sh exits non-zero): must not spawn a driver -# either, same as action=none. +# either, same as action=none (same startup env-error caveat as scenario 1). # --------------------------------------------------------------------------- dir2="$(new_fixture scenario2 '#!/usr/bin/env bash echo "cadence=WATCH cron=*/5 * * * *" echo "some diagnostic on a broken tick" >&2 exit 1')" run_daemon_once "$dir2" >/dev/null 2>&1 -check "scenario 2 (broken tick): no ledger file was created" [ ! -f "$dir2/.claude/state/loop-runs.log" ] +ledger2="$dir2/.claude/state/loop-runs.log" +check "scenario 2 (broken tick): startup env-error is the ONLY ledger line" bash -c ' + [ "$(wc -l < "$1" 2>/dev/null || echo 0)" -eq 1 ] && grep -q "verdict=startup .*result=env-error" "$1"' _ "$ledger2" +check "scenario 2: no driver ledger line was written" bash -c '! grep -q "verdict=advance" "$1"' _ "$ledger2" # --------------------------------------------------------------------------- # 3. action=advance issue=N: fake claude/setsid/timeout stubs record they ran