From 2b3dc2670e34cfd6dfbb0eb33ec42bee5579aa32 Mon Sep 17 00:00:00 2001 From: Roberto Cano Date: Thu, 6 Aug 2026 17:45:04 +0200 Subject: [PATCH] fix(harness): sync arm-loop.sh setup template to v7 + guard against drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit arm-loop.sh lives twice: .claude/scripts/arm-loop.sh (what runs from the installed plugin cache) and .claude/skills/setup/templates/arm-loop.sh (what scaffold.sh writes into a consumer repo and sync.sh re-stamps from). Nothing kept them in sync — release.sh does not copy one onto the other and no check compared them — so they drifted: scripts/ was v7 while the template was still v6. Consequence downstream: sync.sh stamps consumers from the TEMPLATE, so every already-onboarded repo is reported "up to date" at v6 forever while the plugin's own runtime copy is v7. Observed in reDeploy, where /orchestrator:sync printed "up to date: .claude/scripts/arm-loop.sh already v6" against a v7 plugin. The skew is not cosmetic. v7 added --stop-after-days (issue #95), which writes .claude/state/loop-arming.json — the file loop-tick.sh reads to decide whether the loop has passed its self-disarm horizon. A consumer stamped at v6 arms a loop whose tick script (resolved from the plugin cache, so v7) expects state the v6 arming script never writes. Changes: - templates/arm-loop.sh := scripts/arm-loop.sh (now byte-identical, v7, executable bit preserved). - New .claude/scripts/managed-template-parity.test.sh: for every templates/ file with a same-named scripts/ twin, assert byte-identity and matching executable bit, so this cannot silently recur. Auto-discovered by checks.sh do_test() (.claude/scripts/*.test.sh) — no self/ changes needed. Verified it fails on the drifted tree and passes on the fixed one. No version bump here: release.sh owns plugin.json/marketplace.json versioning as part of a milestone-gated cut, so the fix reaches installed consumers on the next release. (CONTRIBUTING.md still says to bump plugin.json in the same PR as a managed-file change — that predates release.sh and now conflicts; flagged in the PR body rather than acted on unilaterally.) Gates: build + lint pass. test has 4 pre-existing failures in this sandbox (cockpit, loop-census, loop-tick, pr-feedback — all network/port-bound); verified byte-identical failure sets on pristine main and on this branch. Co-Authored-By: Claude Opus 5 --- .../scripts/managed-template-parity.test.sh | 88 +++++++++++++++ .claude/skills/setup/templates/arm-loop.sh | 101 +++++++++++++++++- 2 files changed, 185 insertions(+), 4 deletions(-) create mode 100755 .claude/scripts/managed-template-parity.test.sh diff --git a/.claude/scripts/managed-template-parity.test.sh b/.claude/scripts/managed-template-parity.test.sh new file mode 100755 index 0000000..b3d78df --- /dev/null +++ b/.claude/scripts/managed-template-parity.test.sh @@ -0,0 +1,88 @@ +#!/usr/bin/env bash +# managed-template-parity.test.sh — offline guard against the two hand-maintained +# copies of a managed script drifting apart. +# +# The problem this exists to catch: +# Some managed files live TWICE in this repo — once under .claude/scripts/ (the +# copy that actually executes when a user runs it out of the installed plugin +# cache) and once under .claude/skills/setup/templates/ (the copy scaffold.sh +# writes into a consumer repo and sync.sh re-stamps from). Nothing kept the two +# in sync: release.sh does not copy one onto the other, and no check compared +# them. They silently drifted — `arm-loop.sh` shipped as v7 under scripts/ while +# the template was still v6, so `/orchestrator:sync` reported every consumer +# "up to date" at v6 forever while the plugin's own runtime copy was v7. +# +# That skew is not cosmetic: arm-loop v7 added --stop-after-days (issue #95), +# which writes .claude/state/loop-arming.json — the file loop-tick.sh reads to +# decide whether the loop has passed its self-disarm horizon. A consumer stamped +# at v6 arms a loop whose tick script expects state the arming script never +# writes. +# +# Asserts, for every templates/ file that has a same-named .claude/scripts/ twin: +# 1. the two are byte-identical (so the @orchestrator-managed marker version, +# and everything else, necessarily agrees); +# 2. the executable bit matches, since scaffold.sh copies the template with its +# mode preserved and an armed loop runs it directly. +# +# Pure filesystem comparison — no gh, no network, no temp dirs, no mutation. +# Exit 0 on success, non-zero if any pair has drifted. Runnable bare: +# bash .claude/scripts/managed-template-parity.test.sh +set -uo pipefail + +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +repo_root="$(cd "$script_dir/.." && pwd)" +templates_dir="$repo_root/skills/setup/templates" +scripts_dir="$repo_root/scripts" + +fail=0 +ok=0 +check() { + local desc="$1"; shift + if "$@"; then + ok=$((ok + 1)) + echo "ok - $desc" + else + fail=1 + echo "FAIL - $desc" + fi +} + +if [ ! -d "$templates_dir" ]; then + echo "managed-template-parity: missing $templates_dir" >&2 + exit 1 +fi + +pairs=0 +for tpl in "$templates_dir"/*; do + [ -f "$tpl" ] || continue + base="$(basename "$tpl")" + twin="$scripts_dir/$base" + # Only files that exist in BOTH places are dual-maintained. Templates with no + # scripts/ twin (gates.json, gates.yml, CLAUDE.md, the .service units, …) are + # scaffold-only by design and are deliberately not compared here. + [ -f "$twin" ] || continue + pairs=$((pairs + 1)) + + check "$base: templates/ copy is byte-identical to scripts/ copy" \ + diff -q "$twin" "$tpl" + + tpl_x=no; [ -x "$tpl" ] && tpl_x=yes + twin_x=no; [ -x "$twin" ] && twin_x=yes + check "$base: executable bit matches (scripts=$twin_x templates=$tpl_x)" \ + [ "$tpl_x" = "$twin_x" ] +done + +# Guard the guard: if a refactor ever moves these files apart, this test must not +# quietly pass by comparing nothing at all. +check "found at least one dual-maintained template/script pair (got $pairs)" \ + [ "$pairs" -ge 1 ] + +echo +if [ "$fail" -eq 0 ]; then + echo "managed-template-parity.test.sh: all $ok assertion(s) passed ($pairs pair(s) compared)" +else + echo "managed-template-parity.test.sh: FAILURES — a managed script and its setup template have drifted." >&2 + echo " Fix: copy the canonical .claude/scripts/ over .claude/skills/setup/templates/" >&2 + echo " (keeping the higher @orchestrator-managed marker version), then re-run this test." >&2 +fi +exit "$fail" diff --git a/.claude/skills/setup/templates/arm-loop.sh b/.claude/skills/setup/templates/arm-loop.sh index 4906e88..7be3fbf 100755 --- a/.claude/skills/setup/templates/arm-loop.sh +++ b/.claude/skills/setup/templates/arm-loop.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# @orchestrator-managed arm-loop v6 +# @orchestrator-managed arm-loop v7 # arm-loop.sh — installs the cron-less PR-loop as systemd (user) units # (issue #102). Templated + re-stamped by `/orchestrator:setup`/`sync`; do # not hand-edit the copy scaffold.sh wrote into this repo if you want future @@ -13,10 +13,10 @@ # then recreate). # # Usage: -# bash .claude/scripts/arm-loop.sh [--gates-file ] [--permission-mode ] [--capacity N] [--rc-name ] [--spawn ] +# bash .claude/scripts/arm-loop.sh [--gates-file ] [--permission-mode ] [--capacity N] [--rc-name ] [--spawn ] [--stop-after-days N] # # --gates-file passed to pr-loop.service as GATES_FILE (e.g. -# .claude/self/gates.json for the self-hosted +# self/gates.json for the self-hosted # loop). Omit for the default project adapter. # --permission-mode passed to `claude remote-control --permission-mode`. # Defaults to permissions.defaultMode in @@ -30,6 +30,16 @@ # --spawn remote-control spawn mode: same-dir (default) or # worktree. Passed explicitly so the server never # blocks on its interactive first-run question. +# --stop-after-days N self-disarm horizon (issue #95): loop-tick.sh +# refuses every advance/feedback dispatch once +# armed_at + N days has passed, until re-armed. +# Defaults to budget.stop_after_days in the +# adapter picked by --gates-file (or the default +# .claude/gates.json when --gates-file is +# omitted), else 7. Every re-arm rewrites +# .claude/state/loop-arming.json fresh -- +# clearing any prior expiry AND the one-time +# "disarmed" notification guard. set -euo pipefail gates_file="" @@ -37,6 +47,7 @@ permission_mode="" capacity="8" rc_name="" spawn_mode="same-dir" +stop_after_days="" while [ "$#" -gt 0 ]; do case "$1" in --gates-file) gates_file="${2:?--gates-file needs a value}"; shift 2 ;; @@ -49,8 +60,10 @@ while [ "$#" -gt 0 ]; do --spawn) spawn_mode="${2:?--spawn needs a value}"; shift 2 ;; --spawn=*) spawn_mode="${1#--spawn=}"; shift ;; --capacity=*) capacity="${1#--capacity=}"; shift ;; + --stop-after-days) stop_after_days="${2:?--stop-after-days needs a value}"; shift 2 ;; + --stop-after-days=*) stop_after_days="${1#--stop-after-days=}"; shift ;; -h|--help) - sed -n '2,32p' "$0" + sed -n '2,42p' "$0" exit 0 ;; *) echo "arm-loop.sh: unknown argument '$1'" >&2; exit 2 ;; @@ -96,6 +109,46 @@ if [ -n "$gates_file" ]; then gates_env="Environment=GATES_FILE=$gates_file" fi +# --- spend-ceiling arming state (issue #95) --------------------------------- +# Resolve the stop-after horizon: --stop-after-days wins; else +# budget.stop_after_days from the SAME adapter the armed daemon will read +# (gates_file, defaulting to .claude/gates.json); else 7. Always WRITE a +# fresh .claude/state/loop-arming.json on every arm/re-arm -- this is what +# clears a prior expiry and the one-time "disarmed" notification guard. +if [ -z "$stop_after_days" ]; then + adapter_for_stop_after="${gates_file:-.claude/gates.json}" + case "$adapter_for_stop_after" in + /*) ;; + *) adapter_for_stop_after="$repo_root/$adapter_for_stop_after" ;; + esac + stop_after_days="$(node -e ' + try { + const g = require(process.argv[1]); + const d = g && g.budget && g.budget.stop_after_days; + if (Number.isFinite(d) && d > 0) { console.log(d); process.exit(0); } + } catch (e) {} + ' "$adapter_for_stop_after" 2>/dev/null || true)" + stop_after_days="${stop_after_days:-7}" +fi +case "$stop_after_days" in + ''|*[!0-9.]*) echo "arm-loop.sh: --stop-after-days must be a positive number (got '$stop_after_days')" >&2; exit 2 ;; +esac + +arming_state_dir="$repo_root/.claude/state" +mkdir -p "$arming_state_dir" +arm_now="$(date -u +%FT%TZ)" +node -e ' + const fs = require("fs"); + const now = process.argv[2]; + const days = parseFloat(process.argv[3]); + const expires = new Date(Date.parse(now) + days * 86400000).toISOString(); + fs.writeFileSync(process.argv[1], JSON.stringify({ + armed_at: now, expires_at: expires, stop_after_days: days, + notified_expired: false, notice_issue: null, + }, null, 2) + "\n"); +' "$arming_state_dir/loop-arming.json" "$arm_now" "$stop_after_days" +echo "arm-loop.sh: armed until $(node -e 'const j=require(process.argv[1]);console.log(j.expires_at)' "$arming_state_dir/loop-arming.json") (stop_after_days=$stop_after_days) -- .claude/state/loop-arming.json" + # Absolute claude path, resolved HERE — this script runs in a real terminal # with the user's full environment, while the installed unit runs under # systemd's minimal PATH (gh but no nvm-provisioned node/claude). A bare @@ -111,6 +164,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 +202,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" \ @@ -144,6 +218,25 @@ sed -e "s#__WORKDIR__#$repo_root#g" \ echo "arm-loop.sh: wrote $pr_loop_dst" echo "arm-loop.sh: wrote $claude_rc_dst" +# Guard against template/script skew (issue #130): if either sed block above +# is missing a substitution for a placeholder the template still contains +# (e.g. a new __FOO__ added to the .service template without a matching -e +# here), the installed unit silently keeps the literal token and systemd +# fails it at the NEXT boot with an opaque status=127 -- long after this +# script has exited 0. Fail loudly, right here, instead. +for dst in "$pr_loop_dst" "$claude_rc_dst"; do + # Scan only directive (non-comment) lines: the template header comments carry + # the literal doc token __PLACEHOLDER__, which is not a sed target and must + # not false-positive. A REAL leftover lives in a directive line. `|| true` + # keeps the no-leftover healthy path from aborting under `set -euo pipefail` + # (grep exits 1 on no match). Fail loudly on genuine skew (issue #130). + leftover="$(grep -v '^[[:space:]]*#' "$dst" | grep -o '__[A-Z_]*__' | sort -u | tr '\n' ' ' || true)" + if [ -n "$leftover" ]; then + echo "arm-loop.sh: unsubstituted placeholder(s) leaked into $dst: ${leftover}-- the sed block that generated this file is missing a substitution (issue #130); fix arm-loop.sh before re-running." >&2 + exit 1 + fi +done + systemctl --user daemon-reload # pr-loop: enable --now on purpose (NOT restart) — never kill a daemon that # may have a driver in flight; a re-arm only rewrites its unit file, and the